feat(2449): a closed citation in an exemption reason is a finding (backend#2449) - #329
feat(2449): a closed citation in an exemption reason is a finding (backend#2449)#329LukasWodka wants to merge 1 commit into
Conversation
…ckend#2449) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Nice, careful PR 👍 — approving. The scoping decision is the right one: build mechanism (1) because it's checkable from the citation alone, and say plainly why (2) needs a schema change and (3) needs prose understood well enough to know which half is falsifiable. A checker that only does the tractable third of a ticket, and names the other two thirds, beats one that guesses at all three.
The distinctions I went looking for are all made. A merged PR isn't a finding — "the one closed thing whose terminal state is SUCCESS" — while a closed-unmerged one is, because the reason is leaning on something that never landed. Cannot-tell is a finding about the check rather than a pass. And zero citations is itself a finding, which is the guard I'd have asked for: it's the only thing standing between a silently-broken regex and a permanently green audit.
I also checked whether the new EXEMPT list reintroduces the problem one level up — a list of grandfathered rows whose reasons can go stale is exactly what this PR exists to catch. stale_exemptions() covers it: a row that reopened or was edited out of the inventory reddens. And the two rows marked UNREMEDIATED as the ticket's own case 1, with "do not let the row read as permission", keep the decision-versus-to-do distinction that #328 established rather than blurring back into it.
Small thing I appreciated: no tally in the list comment, for the stated reason that mint-scope.py shipped one that drifted. That's the right lesson from it — the run prints len(_exempt()) and nothing else claims a number.
Closes tracebloc/backend#2449
The gap
repo-inventory.ymlexemption entries carry a free-prosereason.caller-drift.pyenforces staleness on the entry (
caller-drift.py:2309— an exemption with a callerpresent is a finding) and nothing reads the reason. So a reason can become false and
stay false indefinitely while the audit stays green, and that sentence is what the next
person reads to decide whether the exemption still applies.
Measured false twice on 2026-08-24:
customer_priority_bump_caller_missingsaid the wiringwas "sequenced behind" backend#1408, closed COMPLETED on 2026-08-06 — three repos sat behind
it for eighteen days; and
rfcs'advance-deploy-env.ymlreason said the repo has nodevelop, which it has.What this adds — mechanism (1) only, deliberately
scripts/reason-citations.py: where a reason cites a ticket, fail when that ticket isdead. The reason may still be valid — a closed citation is not proof of anything — but it
is exactly the case a human has to re-read, and it is checkable from the citation alone.
That alone would have caught case 1 the day #1408 closed.
The ticket's other two sketches are not built here, on purpose:
repo-inventory.ymland a schema decision, not a checker.understood well enough to know which half is falsifiable, and the wrong answer either
invents findings or teaches people to phrase reasons so the check cannot see them.
Splitting them keeps this one small enough to be obviously correct.
Design
Derived, never restated. The citations come from parsing the inventory — every
exempt:string, both
divergent:spellings, and every anchor body inshared_reasons:(an anchordefined and not yet aliased is scanned too, which is the state a new reason is in on the PR
that introduces it). There is no hand-written list of which reason cites which ticket.
Comments are out of scope by construction: the scan runs over parsed YAML, so the ~40 ticket
numbers in the file's header are not treated as justifications.
Issue vs pull request is read, not assumed. GitHub answers both in one field:
A merged PR is the one closed thing whose terminal state is success; reasons cite PRs as
provenance ("remediated under model-zoo#115") and that stays true forever. Flagging them
would have made 10 of the 25 live citations findings for describing history correctly.
Which repo a citation means — stated, not assumed.
owner/repo#Nas written;repo#Ntakes its owner from the inventory's own
org:; a bare#Nresolves against<org>/<source_repo>— the repo the inventory lives in, which is what GitHub itself rendersa bare
#Nas. Both values are read from the inventory; neither is typed into the script.It does not assume
backend— that guess was the defect fixed in.github#314— and itdoes not stay quiet either: when
GITHUB_REPOSITORYcontradictssource_repothe premise ofthe bare rule is false and a bare citation becomes "cannot tell" rather than a guess.
Fail closed, and "cannot tell" is a finding about the check (exit 2, never a pass):
unreadable or unparseable inventory, missing
org:/source_repo:, a failingghcall, anon-JSON response, a citation the API will not resolve (404/403/rate limit), an unrecognised
state, and zero citations found. Zero matters most: the premise is that reasons cite
tickets, so finding none means the matcher broke. Exit 1 is a real finding about the
inventory; the two are separated so a red run says whether to fix a reason or fix the script.
One GraphQL call covers every citation — this runs in a required context on every PR, and N
sequential API calls is N chances for a blip to fail a merge closed. A partial read (null
node +
errors[]at exit 0, which is what GitHub actually returns) is refused, not scored.What it found against the live inventory
Twelve dead citations, all closed issues. They are seeded into
EXEMPT— the same shapeand the same reason as
mint-scope.py's: this arms green rather than landing a red gatein a required context that nobody can merge past. Per backend#2449 they are not fixed
here — re-reading twelve reasons is its own work and would bury the mechanism in a prose
diff.
repo-inventory.ymlis untouched by this PR.Two of the twelve are the ticket's own case 1, found on the first run, and are marked
UNREMEDIATED rather than explained away — both lean forward on a ticket that is shut:
wip_limit_check_has_no_callers: "the decision to wire it up or deleteit is backend#1415 follow-up work". Closed COMPLETED.
blocked_gate_rollout_pending: "the callers follow in the rollout PR(backend#1729)". Closed COMPLETED, so the rollout it is waiting on is tracked by nothing
open.
The other ten (backend#1276, #1408, #1420, #1563, #1752, #1816, #1975, #1976, #1979, #2243)
are cited in the past tense, as the ticket under which something already landed or was
decided. That distinction is a judgement about prose, which is why it is a note to whoever
burns these down and not something the script tries to infer.
The other half of the map is what stops it becoming cover: a row that is no longer a finding
— reopened, or edited out of the inventory — is itself reported.
Wiring
A step in the existing
selftests.yml, besidemint-scope, not a new workflow.selftestsis already a required context on develop/staging/main, so this arms with nobranch-protection edit; a new context would sit unrequired until somebody armed it. A new
reusable would also need its own
repo-inventory.ymlrow in every repo, serialising againstevery other guarded-file PR in this single-writer repo.
The token is minted read-only (
permission-issues: read,permission-pull-requests: read)and is org-wide on purpose — the opposite of
add-to-kanban.yml's narrowing, because thereasons cite tickets in seven different repos and a repo-scoped token would resolve none of
them. It fails closed on a missing secret, which also means a fork PR fails rather than
reporting clean from a read it could not make.
Verification
scripts/tests/reason-citations-selftest.py— 33 cases, all green. Hermetic: fixtureinventories plus a throwaway
ghfirst onPATH, so the subprocess call, the exit-codepath and the JSON decoding are covered rather than being the part nobody tests. Covers open
issue (pass), closed issue (finding), unreadable issue (finding), malformed citation, zero
citations (finding), unreadable inventory (finding), plus merged/open/abandoned PRs, all
three repo-resolution rules, both
divergent:spellings, an unaliased anchor, a ticketnumber in a comment, and both halves of the exemption map.
scripts/tests/reason-citations-mutations.py— 23 mutations, 0 stale, 0 uncaught.Breaking the closed-issue detection (
("Issue", "CLOSED")added toLIVE) reddens threecases; restoring it greens them. Every anchor is asserted to match exactly once, so an
inert mutation is a failure rather than a silent pass — and that caught a real one: the
first draft of the "cannot tell reads as OPEN" mutation inserted its line above the
raiseinstead of replacing it, and the harness reported UNCAUGHT, correctly.make checkgreen;make mutationsgreen (58 s in CI today, +~15 s for this runner);actionlintandyaml.safe_loadclean onselftests.yml;ruffclean viamake lint.make selftests-covergreen — the new suite and the new mutation runner are both wired.🤖 Generated with Claude Code
Note
Medium Risk
Adds a required
selftestsstep that mints an org-wide (read-only issues/PRs) App token and can fail merges on citation/API errors. Existing dead citations are seeded intoEXEMPTso the gate lands green.Overview
Makes stale ticket citations in
repo-inventory.ymlexemption reasons a required finding (backend#2449). Closed issues and PRs closed without merging fail; open issues and merged PRs do not.scripts/reason-citations.pyparses reasons fromexempt, bothdivergentspellings, andshared_reasons(including unaliased anchors). Citations are resolved from the inventory’sorg/source_repo(not hardcodedbackend). Fail-closed on unreadable inventory, unresolvable API reads, unknown states, and zero citations found. Twelve already-dead citations are seeded inEXEMPT(stale rows are also findings); the inventory file is unchanged.Wired into the existing
selftestsjob: fixture suite inmake check, mutations inmake mutations, live audit via a read-only org-wide App token (forks fail closed). Hermetic selftest plus 23 mutation cases pin the matcher and fail-closed paths.Reviewed by Cursor Bugbot for commit e226259. Bugbot is set up for automated code reviews on this repo. Configure here.