feat(pipeline-conductor): ask all five claim questions, not one - #8036
Conversation
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound fail-closed redesign; the one risk left is that prose-derived CLOSE — the verdict that writes to live work — rides a hand-tuned regex engine. Watch
Suggestions
[DESIGN-REVIEWED] a53776b |
GPT 5.6 Review — ✅ human override acceptedHuman judgment by @chenmingwei23 overrides the GPT 5.6 finding for This comment is updated in place on each push. The model was not re-run because an authorized human decision supersedes it. False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All evidence gathered. The base branch already carries the consumer and the contract (SKILL.md dispatch step 3 with the exit-code table and five-check precedence, First-Principles-Verdict: PASS Every check is a decision the repo already recorded — base SKILL.md cites this exact script, exit codes and all — and each blind spot has a counted dispatch it cost. What this change shipsIntent: stop the conductor burning a worker dispatch on items already fixed, claimed, or covered — an ADDITION supplying the script the base skill already mandates.
The change sits at cause level: the three failures shared one defect (one question, empty answer read as permission), and the design removes that shape rather than patching each miss. Deliberate misses ("close the ticket please", Subtractions
[FIRST-PRINCIPLES-REVIEWED] a53776b |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsBoth candidates concern CANDIDATE 1 (glued CANDIDATE 2 (first-open-PR wins, so No self-originated finding rises to the 80+ / (a)(b)(c) bar; the code is tightly guarded and its argv surface is closed. No findings. [OPUS-REVIEWED] a53776b Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
54b9c8a to
d49e00c
Compare
|
Round 1 dispositions for Opus 4.8, Verified against the live endpoint before changing anything. Item 7597 has 12 comments; That is the OLDEST of the twelve. The newest is The fix does not flip the assumption from descending to ascending, because an ordering assumption is what broke. Selection is now by GPT 5.6, A closure request now requires standing: the issue's own author (always true of the body) or a repository insider by Standing gates CLOSE only, not SKIP: a stranger announcing "I am claiming this" still produces SKIP, because staying away is also cheap. GPT 5.6,
So the described failure does not occur, and the proposed remedy would introduce one: Nothing was overridden. The remaining red lanes are a separate matter and are being handled with the fleet's coordinator, not by pushing past them. |
|
Round 2 dispositions for Watch: The finding is right, and it is sharper than "no incident behind it": a feature request naming a new identifier is absent from the default branch by definition, so it SKIPs on every pass, not once. The principled fix follows from what the check MEANS -- absence is evidence only when the code lives on an unmerged branch, so the discriminator is "does this symbol exist on any branch?". I measured it before proposing it. This clone has 377 remote refs; The alternative narrowings are guesses, and one of them breaks the check: "require at least one named symbol to be present" would classify exactly the case check 4 exists for -- a fix whose entire target span lives on an unmerged branch, where nothing is present -- as workable. Prose detection of "this is a feature request" is the same class of guess the citation-stripping rule already had to be measured into, and I am not adding an unmeasured one. So the limitation is disclosed rather than fixed, and two properties bound it: the verdict is SKIP, not CLOSE, so nothing is destroyed and the item flips to CLAIM by itself the moment the symbol lands; and the reason is printed with the symbol name ( Subtractions: drop I agree with the reasoning on Blocker: nothing consumes the script. Factually correct, and I am not disputing the grep. The file that would consume it is owned by a parallel change that already contains exactly the wiring this finding asks for -- the script added to the inventory, the dispatch predicate replaced with the one call, and the 0/10/11/2/3 table documented including the exit-3 branch. Editing that file from here would collide with it mid-flight. So this is a sequencing decision for the owner of both changes, and it is with them now. Nothing has been overridden, and I have not touched a file outside this change's scope to make a reviewer happy. |
d49e00c to
b607bb1
Compare
|
Round 3, head 1. Carrying it as "a bonus signal, never the test" was a polite way of writing "changes no verdict", and a per-candidate forge call that cannot alter the outcome is pure cost against a shared rate limit. The check, its field, its JSON key and its test are gone; 2. The annotation stays exactly as emitted. What was missing was the consumer, and it is named in the module docstring and the PR body: 3. An absent symbol no longer vetoes on its own, and item 8031 shows why that mattered. Corroboration is explicit metadata -- a Measured on a live item, before and after: Item 8031's only labels are An uncorroborated item now costs at most one dispatch that discovers the code is not on the base. A park costs the item. Verification. 154 tests, 99% line coverage on the file, mypy / flake8 / black clean. Live re-run after the change: 7597 The remaining red lane is the zero-consumer BLOCK, which is a merge-order matter and is being handled as one. |
b607bb1 to
6260cfb
Compare
|
Round 4, head Watch: the description contradicted the diff on the check count. FIXED, and ratcheted. Removing one check left the old count behind in two places I own: Counting words in prose are cheap to forget, so they are now pinned instead of remembered: a new test scans the script and the test file and fails if any line names a check count other than the real one, derived from Watch: checks 4 and 5 trace to no named incident, unlike 1 through 3. Correct as stated, and both were narrowed for exactly that reason in the previous round rather than defended. Subtraction: defer Same shape as the blocker, and it resolves the same way. Blocker: nothing consumes the script. Still factually right, still not disputed. The resolution is a merge order, not a content change: the change that wires the consumer lands first, then this one rebases onto it, at which point the reviewer's own grep finds the consumers with neither side editing the other's files. That is the reviewer's second clause taken in the direction the file partition allows. 155 tests, 99% line coverage, mypy / flake8 / black clean. |
6260cfb to
635d874
Compare
ef47ec7 to
da69767
Compare
|
Round 9 on FIXED -- a bare pronoun is not an issue. Last round I added an object requirement so "please close the file" would stop closing items, and I put bare Four more false CLOSEs on live work, shipped by the fix for the previous four. Mutation-verified with the collection guard: restoring A limitation pinned rather than fixed. While testing I asserted that "close the ticket please" is detected. It is not -- the pattern is Verification. 241 tests, 99% line coverage, mypy / flake8 / isort / black clean. Live: 8088 The fork item is still the ruled one. GPT again asks to continue past Four rounds, seven blocking findings, six of them real defects in my code and three of those false-CLOSE paths on live work. The prose scanner is where every one of them lived, which is worth saying plainly: hand-written English patterns deciding a destructive verdict have now needed four corrections, and the reviewer found each one faster than my own tests did. |
da69767 to
f735384
Compare
|
Round 10 on FIXED -- an adjectival "fixed" closed live issues. The deeper problem was that this pattern had no continuation guard at all -- only a Measured: three adjectival forms now decline, and "this is resolved", "this is resolved, closing", "this was fixed in the 0.7 release", "this is resolved -- closing now" and "this is already fixed on main" all still fire. FIXED -- a reference with no repository was assumed local. FIXED (the advisory) -- ancestry was asked about PRs that could not matter. Verification. 261 tests, 99% line coverage, mypy / flake8 / isort / black clean. Three existing ancestry tests failed on the scoping change and were corrected rather than worked around -- their fixtures were asking about ancestry with The fork item is still the ruled one. Same remedy, same conflict with ruling D11 and with the merged check list, no unilateral action, no override. Five rounds, ten blocking findings, nine of them real. Six lived in the prose scanner. That concentration is the useful signal here: hand-written English deciding a destructive verdict has needed a correction in every single round, and each correction has been narrower than the last. A design where prose can only DOWNGRADE, never CLOSE, would have made all six impossible, and I would rather name that than keep patching patterns. |
f735384 to
87f4d3a
Compare
|
Round 11 on FIXED -- negated prose closed live issues. The phrase patterns match a SUBSTRING, and English puts the negation earlier in the sentence, so every one of these returned CLOSE, exit 11: A negation-and-hedging window over the text PRECEDING each match now vetoes the phrase, and the scan continues to the next pattern rather than giving up. Python's Worth naming explicitly because it looks like an inconsistency and is not: this is the OPPOSITE choice from FIXED -- a stale body request overrode a newer reopening comment. The body is the OLDEST text on an item. A reporter who wrote "this is resolved" and later commented "still broken on 0.8" had reopened it, and the scan read the body and closed it. Closure now consults the body only when there is no newer human comment at all: a later comment is the item's current state whether or not it happens to contain a phrase this scanner knows, so its mere existence retires the body's request. The CLAIM loop still reads the body, because ownership is not a status -- it holds until withdrawn, and the three items that motivated the prose check declared ownership in the body. That the whole suite passed before I added tests for this is the part I would flag: 261 tests and none of them combined a body request with a later comment. The gap in the tests and the gap in the code were the same gap. FIXED -- an attributed claim parked the item. FIXED (the advisory) -- recovery stepped over a withdrawal. When I added A limitation pinned, and a second wrong expectation of my own. I wrote a test asserting that "I don't think this is resolved, but please close this issue as a duplicate" still closes, reasoning that the scan continues past a negated match. It does continue -- but the window is measured in characters and "don't" sits 35 of them before "please close", so the hedge vetoes the later request too. The code is right and my expectation was wrong. That is now pinned as a disclosed miss with its reason: it costs one dispatch, where the other reading closes live work, and making the window clause-aware needs sentence segmentation, which is a bigger claim about English than this file should make. This is the second time this session I have asserted what I wanted rather than what I measured, both times caught by running it. Verification. 285 tests, 99% line coverage, mypy / flake8 / isort / black clean. Live, and two verdicts legitimately CHANGED between rounds, which is worth reading rather than glossing: item 6799 was Six rounds, thirteen blocking findings, twelve real, and nine of them in the prose scanner. Every round has produced at least one, each narrower than the last. That is not a run of bad luck; it is what happens when hand-written English decides a destructive verdict. All nine become impossible if prose can only downgrade to |
87f4d3a to
45ec72f
Compare
|
Round 12 on FIXED -- a stranger could release somebody else's claim. The withdrawal check I added last round tested only that the commenter was not a bot, so any passer-by typing "dropping this" retired a maintainer's earlier claim and the item was dispatched on top of live work. That is the denial-of-work shape again, running the other direction: instead of suppressing an item that should be worked, it releases one that is already owned. A withdrawal now only ever releases its OWN author's claim -- you can only give up what you hold -- and withdrawals are tracked per author, so one person leaving does not release another's claim. Standing deliberately does NOT transfer here: standing is authority over the ITEM, not over another person's commitment, so an OWNER cannot release a MEMBER's claim either. Measured: outsider withdrawal leaves the claim recovered, the claimant's own withdrawal retires it. FIXED -- the last two closure patterns had no guard. I had added an object requirement to the FIXED -- double-backtick citations survived the stripper. FIXED (the advisory) -- a partial scan discarded what it had already confirmed. A detail-call failure on a later PR returned Verification. 302 tests, 99% line coverage, mypy / flake8 / isort / black clean. On the fork item and the override. An override was authorized for it, and I am not posting one on this head, because the fork finding is absent from GPT's list for Also, the live evidence the authorization rests on had gone stale in BOTH cases, so I re-ran it before citing anything. Items 6799 and 8071 have moved to Both are fork PRs and only one is marked, which is the discrimination the mitigation claims to perform. The fork population re-measured today is 190 of 299 open PRs, unchanged in substance from 192 of 301. If the fork finding returns on a head whose other blocking items are clear, the override goes up then, on its own. |
|
Round 13 on FIXED (the advisory, and it was more serious than its label) -- a body claim survived its own author's withdrawal. A claim written in the issue BODY deliberately outlives later chatter, because ownership is not a status. The gap was that it also outlived the claimant giving it up: a reporter who wrote "Ownership claimed by me" and later commented "dropping this" left the item SKIPping forever. That is indefinite suppression, which is the harm this file treats as the serious one and the exact class the fork ruling turns on -- not a wasted dispatch. Advisory by label, permanent by effect. The body predates every comment by construction, so any withdrawal from its author retires it and no timestamp comparison is needed. Verified three ways: the reporter's own withdrawal retires the body claim, a stranger's does not, and an unknown reporter retires nothing. While fixing it I factored the withdrawal scan into Verification. 306 tests, 99% line coverage, mypy / flake8 / isort / black clean. Live: 8088 Why the override is not on this comment. An override clears the reviewer lane for one specific SHA. On |
|
Round 14 on FIXED -- the last pattern with a hardcoded bare pronoun. That loses a real request -- in an issue thread "happy to have it closed" usually does mean the item -- and the miss is pinned with that reasoning rather than hidden. It costs one dispatch; the other reading closes somebody's work. The actual point of this round: the recurrence was the defect. Six consecutive rounds found one more closure phrase that matched prose about something other than the item -- a file, a socket, a workaround, a layout, a pronoun. Each was fixed individually, and the next round found another, because nothing checked the property itself. Two tests now do:
Mutation-verified, and the result argues for the ratchet specifically: restoring the unguarded pattern fails the STRUCTURAL test and NOT the property test, because my template list happens not to phrase "happy to have X closed" with a resource. The generative test would have missed exactly the bug that prompted it. That is the case for checking the list as well as the behaviour, and I would not have known it without running the mutation. Verification. 310 tests, 99% line coverage, mypy / flake8 / isort / black clean. Three existing tests used "happy to have it closed" as a genuine request and were updated to the deictic form rather than worked around. Live: 8088 Process note, because it nearly shipped nothing again. The scratch directory rotated mid-round, so On the override. The fork item was absent from this round's list, so the precondition is still unmet and nothing was overridden. It stays available for the first head where the fork item stands alone. |
The conductor's claim predicate was one prose line, `gh pr list --search`, and it was blind in three directions at once. Each blind spot cost a whole worker dispatch to discover the work did not exist: an item already fixed by a MERGED PR (an `--state open` query structurally cannot see one), four items that each had an OPEN PR carrying `Fixes #N` behind a single field that answered empty, and three items that declared ownership in PROSE. claim_preflight.py asks all five questions in one call and returns one verdict on the exit code: 0 CLAIM, 10 SKIP, 11 CLOSE, 3 UNKNOWN, 2 malformed. The verdict is a pure function of a checks dict, so every precedence branch is a unit test with no forge access, and an unanswerable question yields UNKNOWN, never CLAIM. Six rules earn their own mention because measurement produced each one, not reasoning: - The prose scan reads what an author SAYS, not what they QUOTE: the item specifying this script quotes the closure phrases it detects, and a raw scan returned CLOSE on live work. - The newest human comment is chosen by timestamp, never by position: the comments endpoint ignores `sort`/`direction` and answers oldest-first, so asking for `direction=desc` read the OLDEST of twelve comments on a real item. - A merged PR is coverage only if it CLAIMS to close the item. A bare mention is not closure. Measured on a real item: 7597 has TWO landed merged PRs that merely reference it, one titled "docs: investigation for #7597", and treating either as coverage would have closed an item still being fixed. - A closure request needs standing (the reporter or a repository insider), since CLOSE acts on live work. - A self-claim needs standing too, but downgrades instead of vetoing: a SKIP any commenter can cast is a denial-of-work channel, so an unauthorized claim annotates `risk=high` and takes the live recheck. - An open fork PR still SKIPs, but not silently. Opening a fork PR needs no permission, so rule 2 is a suppression channel anybody can use. Refusing to trust fork PRs is the wrong trade -- 192 of this repo's 301 open PRs come from forks, so that reinstates the duplicate-dispatch class this script was built from -- and the objection was never to the detection but to a response that was unconditional AND silent. So the verdict is unchanged and an unvouched fork's SKIP carries `untrusted-fork=true risk=high`. Standing is an insider association or the item's own reporter, since fixing your own bug from a fork is the ordinary case. The consumer is the conductor's review of untrusted-fork suppressions. - An absent symbol vetoes only when the item's metadata corroborates bug-class: a feature request names the symbol it PROPOSES to add, so an unconditional veto parked that whole class permanently. `closedByPullRequestsReferences` is not consulted at all. It measured `[]` on two items that were closed by merged PRs, and a per-candidate forge call that cannot change the verdict is pure cost against a shared rate limit. Verified against live items: 8088 answers CLOSE merged-pr=#8092 landed=true (that PR carries `Closes #8088`), 7597 and 8029 answer SKIP open-pr=#8035, 8031 answers CLAIM risk=high (no bug-class label), 8007 answers CLAIM risk=low. The fork marker is live too: 6799 and 6509 answer SKIP untrusted-fork=true risk=high behind first-time-contributor fork PRs, while 8071 stays unmarked because its fork PR author reported the item. Refs #8029
|
Round 15 on FIXED -- HTML comments were read as prose. A comment is invisible to every human reader of the item, so nothing inside one is a statement its author is making. This repository's issue templates ship instructional comments, so This is the most likely false CLOSE of the whole series: it needs no unusual phrasing from a reporter, only a template that already exists. FIXED -- the fork marker vanished on the degraded path, and that was my own doing. Round 12 taught the scan to keep a confirmed open PR through a later detail failure. The annotation, though, was still gated on there being NO error, so that kept SKIP came out as a routine low-risk one with no FIXED -- an invariant that overclaimed. The docstring said "Nothing user-authored reaches stdout" while the output legitimately carries logins, PR numbers, comment ids and extracted symbol names. The claim is now narrowed to what is true and says why: no user-authored PROSE reaches stdout -- failures are slugs, a prose match reports its pattern, a bug-class match reports this module's own term -- while identifiers do appear, because they are the evidence a conductor needs to check the verdict. Worth fixing rather than shrugging at: the wider version was in this docstring when a leak of user-written LABEL text shipped straight past it, so an invariant that overclaims is not a harmless overstatement here. A mutation run that lied, and how I caught it. My first mutation check reported the fork-marker test NOT CAUGHT. The test was fine; my mutant was wrong -- I had gated the FIRST Verification. 318 tests, 99% line coverage, mypy / flake8 / isort / black clean. Live: 8088 On the override. The fork item was again absent from this round's list, so the precondition is unmet and nothing was overridden. |
|
/ai-review override gpt a53776b: the merged contract mandates this verdict, the harm it names is already mitigated by an annotation rather than a verdict change, and the requested remedy would regress 63% of this repository's open PRs; filed as a follow-up, not dismissed. Recorded in full, because an override should be auditable rather than terse. 1. The contract mandates the verdict. Merged
The requested change is for this script to do the opposite of the specification that merged for it two cycles ago. That file is not mine, and a code change contradicting a bold line of a shipped contract is a contract amendment, not a bug fix. 2. The stated harm is mitigated, by annotation rather than by a verdict change. The finding's harm is SILENT indefinite suppression. It is not silent. An unvouched fork's SKIP carries Two fork PRs, one marked. #8140 is CONTRIBUTOR and not the reporter, so it is unvouched; #7709 is MEMBER, so it is vouched. That is standing being computed, not fork-ness relabelled -- which matters, because a marker that fired on every fork would carry no information. 3. The remedy regresses the majority case. Re-measured today: 189 of 299 open PRs on this repository come from forks, 63%. Letting untrusted forks fall through to 4. Filed as a follow-up, not dismissed. The fork-versus-trusted-standing model deserves designing properly rather than being decided inside one script's precedence list, and the position is revisitable when it is. This override is a judgment call on that design disagreement, not a claim that the reviewer is wrong about the mechanism -- it is right about the mechanism. Why this is not laundering, stated plainly so the record can be checked. Across this review, GPT raised 21 blocking findings on this PR and I fixed 20 of them on the merits -- including three false-CLOSE paths that returned exit 11 on live work, one regression I introduced myself while fixing another, and two consequences of my own earlier fixes. Two rounds ago this override was already authorized and I did NOT post it, twice, because the fork item was not the only blocking entry and overriding would have cleared unrelated real defects along with it. It is posted now, on the first head where it stands alone, against a single item where the reviewer asks for a documented contract violation. |
Human judgment recorded@chenmingwei23 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
Problem / Motivation
The pipeline conductor decides whether to claim a work item with one question.
Its predicate is a single prose line in the skill body,
gh pr list --search,and an empty answer is read as permission to dispatch a worker. That predicate
is blind in three directions at once, and each blind spot has already cost a
whole dispatch to discover the work did not exist:
days later, with the reporter's own "happy to have it closed" sitting on the
thread. An
--state openquery structurally cannot see a merged PR.went to items that each carried
Fixes #Non an OPEN PR, because thepredicate read
closedByPullRequestsReferences. That field still answersempty: measured on this repo just now, items 7597 and 8007 are both closed by
merged PRs and both answer
[].this issue" / "Ownership claimed by @x" in the body, which no label or field
query sees.
Why it matters
A wrong claim is not a cheap mistake. It stands up a worker session, clones a
worktree, burns credits and an agent's whole context, and ends with the worker
discovering there was nothing to do -- or worse, opening a second PR against a
file another PR already owns. The three failures above are not three bugs; they
are one design defect appearing three times, and the defect is asking a single
question and treating silence as a yes.
What changed
Motivation, then approach, then the change.
Goal: a claim decision that cannot be wrong in a way one more cheap question
would have prevented.
Approach: ask all five questions in one call and return one verdict, and
make an unanswerable question return UNKNOWN rather than fall through to CLAIM.
The alternative -- adding a second query to the existing prose predicate -- was
rejected because it fixes one blind spot and leaves the shape that produces
them: prose in a skill body cannot be tested, and the next missing question
costs another dispatch to find.
What was built:
claim_preflight.py, one new script in the pipeline-conductorskill.
The verdict rides the exit code, because that is what the conductor branches on:
0CLAIM,10SKIP,11CLOSE (triage debt),3UNKNOWN,2malformedarguments. Human form is one line (
CLOSE 8088 merged-pr=#8092 sha=06065e5165 landed=true);--jsonprints exactly one object carrying all five checks.The five checks, and what each one is for:
open_prs-- open PRs referencing the item, fork PRs included, withauthor and
is_cross_repositoryper hit.merged_prs-- merged PRs, each annotatedlandedbygit merge-base --is-ancestor <mergeCommit> <default-branch>andcloses_itemby a closing keyword aimed at this item. Both are load-bearing:a PR merged somewhere other than the branch a worker would start from is not
coverage, and a PR that merely MENTIONS the item is not closure either.
prose_claim-- the body and the NEWEST human comment, for self-claimphrases and closure requests, both of which require standing (the reporter or
a repository insider) for opposite reasons.
symbol_on_base-- every symbol the item names, bygit grepon the defaultbranch. Absence means the target code MAY live only on an unmerged branch --
but that reading holds for a bug item, not for a feature request naming the
symbol it proposes to add, so it vetoes only when the item's own metadata
corroborates bug-class, and otherwise downgrades to
CLAIM risk=high.recency-- age andauthorAssociation. A fresh item from an activecontributor is a high self-claim risk, surfaced as
risk=high, never a veto.The consumer is the skill:
risk=highmeans the item is not batched -- itgets a live re-check immediately before the atomic claim.
closedByPullRequestsReferencesis deliberately not a check. It measured[]on two items that were closed by merged PRs, and a per-candidate forge callthat cannot change the verdict is pure cost against a shared rate limit. The
timeline answers the same question correctly, including for fork PRs.
Precedence is first-match-wins and lives in
verdict(checks), a pure functionof a dict: merged-and-landed gives CLOSE, an open PR gives SKIP, a closure
request gives CLOSE, a prose claim gives SKIP, an absent symbol on a
corroborated bug item gives SKIP, any errored check gives UNKNOWN, and otherwise
CLAIM. The error rule sits below
the positive findings on purpose: a definite answer to one question outranks a
partial view of another, and no error path can reach CLAIM.
Six properties worth naming, because they are where this kind of script usually
goes wrong. The last four are rules that measurement produced and reasoning did
not:
Unknown is not "no". Three answers are possible for ancestry, and the
third is not
False. A merge commit missing from a stale clone reportsUNKNOWN, never "did not land" -- reading it as "did not land" is precisely how
the already-fixed item got dispatched. Same rule for a failed
git grep: anunsearchable tree is not an absent symbol.
No writes, enforced rather than promised.
run_ghrefuses any argv thatis not on a read allowlist before a subprocess exists, so a mutating method,
a
-ffield (which makesgh apia POST), or anissue closeis rejected inprocess. A future edit that adds a write has to defeat the allowlist, the
source-level test, and the test that inspects every argv of a full run.
The prose scan reads what an author SAYS, not what they QUOTE. Code
fences, backtick spans, blockquotes and quoted spans come out before matching.
See Manual verification: the item specifying this script quotes the closure
phrases as a description of what to detect, and a raw scan returned CLOSE on
live work.
The newest human comment is chosen by timestamp, never by position. The
per-issue comments endpoint documents only
since/per_page/page: itsilently ignores
sort/directionand answers oldest-first. Selecting bymax(created_at)means an endpoint that changes its order cannot reintroducethe bug, which is stronger than flipping to the opposite assumption.
A merged PR is coverage only if it CLAIMS to close the item. A closing
keyword (
Closes #N,Fixes #N, including theowner/repo#Nand full-URLspellings) in the PR title or body, not a bare cross-reference. A mention
decides nothing and falls through: reading it as coverage closes live work, and
reading it as a claim starves an item whose fix was only partial. The match is
deliberately negation-blind, matching GitHub's own parser, so this script's
reading and the forge's reading stay identical.
Both prose phrase sets need standing, for opposite reasons. A closure
request produces CLOSE, which acts on live work, so "please close" from a
passer-by must not fire it. A self-claim produces SKIP, and a veto any
commenter can cast is a denial-of-work channel -- one comment would suppress a
queued item indefinitely with nothing downstream reporting the suppression. So
an unauthorized claim is neither obeyed nor discarded: it annotates
risk=highand takes the live recheck. Standing is the item's own author (always true of
the body) or an insider by
author_association; CONTRIBUTOR alone is notauthority over another person's report.
An open fork PR still SKIPs, but not silently. Opening a fork PR needs no
permission, so this check is a suppression channel anybody can use: mention an
item from a throwaway fork and it leaves the queue. Refusing to trust fork PRs
is the wrong trade -- 192 of this repository's 301 open PRs come from forks, so
dropping them reinstates the duplicate-dispatch class this script was built
from, repeatedly, to close a channel that costs an outsider one PR. The
objection worth answering was never the DETECTION; it was a response that was
unconditional AND silent. So the verdict does not move and the doubt is
published with it: an unvouched fork's SKIP carries
untrusted-fork=true risk=high. Standing is an insider association or the item's own reporter,since fixing your own bug from a fork is the ordinary case rather than an
attack, and an unknown reporter marks MORE items rather than fewer.
The annotation's consumer is the conductor's review of untrusted-fork
suppressions, specified in the conductor's protocol now and landing in the
skill in a follow-up. A printed annotation nothing acts on would be the same
defect this PR removed elsewhere, so it is named rather than assumed.
An absent symbol vetoes only a corroborated bug item. The evidence behind
that check was bug items whose target span lives on an unmerged branch.
Applied unconditionally it parks any feature request naming a symbol it
proposes to add -- and parks it on every pass, not once, because the symbol
stays absent. Corroboration is explicit metadata (a bug/defect/regression/
crash label, or the issue type), never prose: guessing the class would put an
unmeasured heuristic in front of the very veto that was over-applied. An
uncorroborated item is dispatched at
risk=highinstead, which costs at mostone dispatch against a park that costs the item.
Nothing user-authored reaches stdout. Failures are reported as slugs, not
forge stderr, and a prose match reports the pattern that fired, never the
matched sentence. This output lands in an agent's context.
--repo-diris optional in a precise sense: it is needed only for the twoquestions git can answer, so an item with no merged PR and no named symbol never
needs a clone. When one of those questions does arise and there is no clone, the
answer is UNKNOWN rather than a guess in either direction.
Tests
test/test_pipeline_conductor_claim_preflight.py, 203 tests, 99% line coverageon the new file (the two uncovered lines are
sys.exit(main())and one partialbranch). Every precedence branch has a test that also asserts the old
single-question predicate would have said CLAIM on the same item -- a test
that only checked the new answer would not notice the script regressing back
into the old blind spot.
already-fixed; merged butlanded elsewhere falls through to CLAIM; an open fork PR gives SKIP
open-pr;a prose self-claim by another user gives SKIP
prose-claim; a closure requestin the newest comment gives CLOSE
reporter-asked-close; an absent symbol ona bug-labelled item gives SKIP
symbol-absent; each of the five checkserroring gives UNKNOWN and exit 3; a clean item gives CLAIM with
risk=highfor a fresh item from an active contributor.
bug-class metadata gives
CLAIM risk=high, not SKIP, both as a verdict-leveltest and end to end. A bug LABEL and an issue TYPE each corroborate; seven
bug-ish label spellings are accepted and five non-bug ones (
enhancement,feature request,documentation, ...) are not; junk metadata degrades to"not corroborated" rather than raising.
a prose claim; a definite finding outranks an errored check. The dropped field
is pinned as dropped: five check names, absent from the source, and no run
issues that forge call.
two bot comments, with the real thread's own timestamps; reversing the input
list does not change the answer, which is what makes the fix order
independent rather than the opposite assumption; position breaks ties only
when a timestamp is absent.
stranger with NONE and a drive-by with CONTRIBUTOR cannot. A self-claim needs
no standing, because it produces SKIP, which is the cheap direction.
markdown hard-wrapped it in the real body, a fenced block, an inline code
span, a blockquote and a curly-quoted span all fail to request closure, while
the same sentences unquoted still do.
--repo, a non-positive--item, a blank--default-branch, and a--repo-dirthat is not a clone.--jsonemits exactly one parseable object with all five checks present, onthe UNKNOWN path too.
write shapes; a refused argv is proven never to reach a subprocess; the script
source is scanned for write verbs; and a full run's every argv is asserted to
be a read.
tmp_path), because astubbed
runcannot catch a wrong flag:--is-ancestordistinguishes landedfrom merged-elsewhere, a commit absent from the clone reports
ancestry-unknownrather thanlanded: false, andgit grepfinds a symbolon one branch and not the other.
invented: a timeline whose events are mostly
commented,labeledandreferencedwith one cross-reference pointing at an issue rather than a PR,and a duplicated reference that must produce one detail call, not two.
Manual verification
Ran against live items, which is what produced two of the rules above.
CLOSE 8088 merged-pr=#8092 sha=06065e5165 landed=trueCloses #8088that landed on the base, which an--state openquery cannot seeSKIP 7597 open-pr=#8035 fork=false author=chenmingwei23SKIP 8029 open-pr=#8035 fork=false author=chenmingwei23CLAIM 8031 risk=highreadiness: checkingandfork, so nothing corroborates bug-class. An unconditional symbol veto answeredSKIP symbol-absent=PARTIAL_TURN_MARKERhereCLAIM 8007 risk=lowThe fork marker on live data. Items 6799 and 6509 answer
SKIP open-pr=#8146 fork=true author=LuisBrel untrusted-fork=true risk=highandthe equivalent for #8140 -- both first-time-contributor fork PRs, so the
suppression now arrives with its own doubt attached. Item 8071 is the control: it
is also behind an open fork PR (#8107) and is deliberately NOT marked, because
catoneoneboth reported 8071 and is fixing it from their fork. The verdict isSKIP open-prin all three cases -- the marker changes what the conductor istold, never what the script decides.
A merged PR that only mentions the item. Item 7597 is why the
closing-keyword condition exists, and it caught a false CLOSE in this script's own
earlier behaviour. It has TWO landed merged PRs referencing it -- #7613, titled
docs: investigation for #7597, and #7313 -- and neither carries a closingkeyword. Without the condition the verdict was
CLOSE 7597 merged-pr=#7613 landed=true, which would have closed an item still being fixed in open PR #8035.With it, both are recorded
landed: true, closes_item: falseand decide nothing,so the open PR is what produces the SKIP.
Citations. The first run of item 8029 returned
CLOSE ... reporter-asked-close, which was wrong: that item's body is this script's ownspecification, so it quotes the closure phrases as a description of what to
detect. A false CLOSE closes work in flight, so the scanner now removes
citations before matching. The first version of that stripper still missed the
real body, because markdown hard-wrapped the quotation across a line break while
the phrase patterns matched across it; whitespace is now collapsed before the
span strip, so the stripper is exactly as newline-tolerant as the phrases it
defends.
Comment ordering. Measured directly against the endpoint: item 7597 has 12
comments, and
?per_page=1&sort=created&direction=descreturned id 5492699917at
2026-09-01T10:40:23Z-- the OLDEST -- against a newest of id 5518670706 at2026-09-03T00:56:58Z. The parameters are silently ignored. Re-verified afterthe fix against the same live thread: the selector now returns 5518670706. A
reporter's later "please close" was previously unreachable by the check that
exists to find it.
Pagination.
gh api --paginateon these array endpoints merges pages into asingle JSON document: measured on gh 2.96.0, the timeline of item 7597 at
per_page=8returned 4 pages as one parseable array of 33 events, and thecomments endpoint at
per_page=5returned 3 pages as one array of 12. So apaginated read stays a single parse and needs no
--slurp-- which is in anycase rejected in combination with
--jq.Also verified locally on the rebased tree:
black,isort,flake8,mypy,the black gate, the agent-sdk-boundary gate, the builtin-skill-scope gate (this
script ships to every install, so it names no repository path), the testpaths
coverage gate, the loop-bound-locks gate, and
test_spawn_audit.py::test_bundled_skill_assets_are_not_imported.Related Issues
Refs #8029
Any other suggestions
answers CLAIM. That is correct against the contract -- the conductor feeds it
candidates from an open-item query -- but the issue payload it already fetches
carries
state, so a seventh check is nearly free. I left it out rather thanwiden the verdict surface unilaterally while the skill documentation for these
exit codes is being written in parallel.
cover the phrasings actually observed. A phrasing outside them yields CLAIM,
which is the cheap failure direction, but the lists will need to grow from real
misses rather than from imagination.
--repo-dircurrent is thecaller's job. A stale clone degrades to UNKNOWN rather than to a wrong answer,
which is the right failure, but a conductor that never fetches will see UNKNOWN
on every recently merged PR.
recencyis annotation only. It reportsrisk=highand nothing acts on ityet. It earns its place when something does -- the dispatch decision, or a
pre-dispatch comment asking the author whether they are already on it.