Skip to content

pr_review.py Reads unresolved=0 With an Open qodo Thread, Because qodo Posts Under a Second Login the Allowlist Does Not Know #1404

Closed

Description

@ptr727

scripts/pr_review.py tracks one qodo identity:

QODO_LOGIN = "qodo-code-review"

qodo posts under at least two. Measured across this fleet's own recent pull request history:

Repository Thread author logins seen
Utilities coderabbitai, qodo-code-review
PlexCleaner qodo-free-for-open-source-projects
ProjectTemplate copilot-pull-request-reviewer

unresolved is computed as:

unresolved = [t for t in threads if not t["isResolved"] and thread_author(t) in KNOWN_REVIEWERS]

so a thread opened by the second spelling is counted in threads= and dropped from
unresolved=.

Observed, on ptr727/PlexCleaner#954

threads=1 unresolved=0 suppressed=0 (on_head=0 earlier=0) merge=BLOCKED checks=17/18

The one thread is open. GraphQL reports isResolved: false for it, authored by
qodo-free-for-open-source-projects, carrying a Medium-severity finding. status read it as
zero unresolved, and wait exited 0 on the same reading.

wait returning clean is the part that matters. The digest is what a Merge Gate decision is
taken from, so this reports a pull request as having nothing outstanding while a finding sits
open on it, unanswered.

This is the regression #915 already fixed once

The constant carries its own warning:

# An open thread blocks a ruleset-gated merge whoever opened it, and `status`'s `unresolved=0` once silently hid a CodeRabbit/qodo thread that did block one (PR #915, ptr727/ProjectTemplate).
# Login spellings are read off this repository's own history (`gh pr view --json reviews,comments`) rather than guessed.

The generalization from "Copilot's own threads" to "every tracked reviewer's" held. What did not
hold is the membership test underneath it, because the spellings were read off this
repository's
history, and this repository has only ever seen Copilot. The two spellings that
matter appear in Utilities and PlexCleaner, which that reading never covered.

Why an allowlist keeps failing this way

Each new spelling is silent by construction: the thread lands, the count ignores it, and the digest
looks the same as a genuinely clean one. Nothing in the output distinguishes "no unresolved
threads" from "unresolved threads by an author I do not recognize", which is the property that
makes it a gate defect rather than a cosmetic miscount.

Two directions, both worth weighing rather than patching the one spelling and moving on:

  1. Count every unresolved thread, and attribute rather than filter. An open thread blocks the
    merge whoever opened it, which the comment above already says. The login set would then decide
    only how a thread is labelled in the breakdown, never whether it is counted.
  2. Keep the allowlist, and make an unrecognized author loud. Emit a distinct marker
    (unknown_authors=<login>) so the digest cannot read clean while carrying a thread it
    declined to count.

Direction 1 matches what the gate is for. Direction 2 is the smaller change and still removes the
silence. Adding qodo-free-for-open-source-projects to the tuple fixes today's instance and
leaves the mechanism exactly as fragile.

Found during the #1399 fleet resync pilot, on the review of ptr727/PlexCleaner#954.

Activity

  1. ptr727 commented on Sep 7, 2026

    @ptr727
    OwnerAuthor

    Disposition, decided 2026-09-07: direction 1, count every unresolved thread and attribute rather than filter. Recorded here rather than implemented.

    The login set decides only how a thread is labelled in the breakdown, never whether it is counted. An open thread blocks a ruleset-gated merge whoever opened it, which the constant's own comment already says, so the count follows that rather than an allowlist.

    Direction 2, keeping the allowlist and emitting an unknown_authors=<login> marker, was weighed and not taken. It removes the silence without removing the fragility, and the mechanism would go on needing a new spelling added every time a reviewer changes one.

    Worth carrying into the implementation: the wait exit codes are keyed off unresolved, so widening what that counts changes what wait blocks on. Every caller of those codes needs re-checking rather than assuming the widening is transparent.

    Live example of the gap, observed while driving #1417 on 2026-09-07: status reported threads=2 unresolved=0, and both issue-level bot comments on that pull request came from authors the digest does not count at all. Neither carried a finding on that occasion, qodo being billing-blocked and CodeRabbit having auto-skipped a repository under ten stars, but nothing in the digest distinguished that from a finding it declined to count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions