Uh oh!
There was an error while loading. Please reload this page.
ci(2364): a PR whose title names a ticket must link it (backend#2364) - #314
Conversation
Merging a fix never closed or advanced its ticket. `closingIssuesReferences` was 0 on 7 of 7 sampled merged PRs (release-train#109/#108, .github#304/#300, backend#2266, client#774, docs#131). The house convention puts the ticket in the PR TITLE; GitHub creates a closing link ONLY from a keyword in the BODY, so a title reference is inert. `kanban-closure-router.yml` fires, finds no linked issue, and correctly does nothing -- every kanban workflow green, every card unmoved. Adds a `closing-ref` job to the EXISTING `set-pr-status.yml` reusable: parse the real title, assert the real `closingIssuesReferences` contains what it names. Derived, not restated (rule 1): two live reads, no list of tickets, repos or authors. The four title forms are measured, not imagined. A bare `#N` outside parentheses is deliberately NOT read as a ticket -- backend#2309's `#2271` is prose about a PR, and scanning loose `#N` would redden a compliant PR. Fails closed (rule 3): a blank title, a GraphQL error, `pullRequest: null`, a null/ownerless node, or `totalCount > len(nodes)` all exit 2 as "cannot tell", never a pass and never a finding against the author. The truncation test is load-bearing beyond pagination -- a link to an issue the token cannot read comes back missing from `nodes` while `totalCount` still counts it, which is indistinguishable from "not linked". The cross-repo trap is its own verdict: `WRONG_REPO` is reported apart from `MISSING` because the remedies differ -- a bare `Closes#304` in `.github` links `.github#304`, closing the wrong issue on merge, and needs the line rewritten rather than added. Fixtures are measured bytes (the backend#2114 lesson), captured with `gh api graphql` and re-verified against the live API before commit. Tests: 102 selftest assertions; 34 mutations, 0 stale, 0 uncaught. The mutation harness edits the real gate and re-runs the real suite -- no inline copy of any rule (rule 9, .github#114/#115). Every anchor must match exactly once, which is the assertion that it actually applied. Refusals are asserted by their own message, never a catch-all (rule 10). The commit-type vocabulary is derived out of org-standards.md and the derivation fails closed if it finds nothing (rule 6). Arming: `closing-ref` is a required status check NOWHERE -- measured across 19 repos x develop/staging/main/master x both classic protection and rulesets -- so a finding blocks no merge (rule 4). Callers trigger on opened/reopened/ready_for_review/converted_to_draft, not `synchronize`, so the 13 open PRs that would report a finding are not reddened by a push. Touches no file in `conformance-gate.yml`'s GUARDED list, and needs no `repo-inventory.yml` row: the inventory tracks callers, one row per reusable, and `set-pr-status.yml` already has its rows. Closestracebloc/backend#2364 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 23, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1e99fbf. Configure here.
…heir place in the list Both sides added a runner to MUTATION_TARGETS and SELFTEST_TARGETS. Kept both. The naive resolution drops the line-continuation backslash and silently truncates the selftest list -- exactly the half-wiring `selftests-cover` exists to catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 24, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…364) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 24, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c97832a. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
saadqbal
left a comment
There was a problem hiding this comment.
Good gate, and I'd take it. One inline note at closing-ref-gate.py:156 — a latent fragility rather than a live defect, and I verified it isn't one before writing it up. Commenting rather than approving because that thread is mine and open.
The head commit's fix is the sharpest thing in the PR and worth naming explicitly: a remedy that advised Closes tracebloc/backend#N for every bare title number would, on a release-train ticket, link the wrong issue, turn this check green, and close the wrong ticket on merge. A remedy that manufactures the defect the gate exists to prevent is a much worse failure than no remedy, and dropping the or "backend" default in favour of telling the author to confirm the owning repo is the right answer. Keeping the explanation of why GitHub ignores a title reference — closing links come only from a keyword in the body — in the message the author actually reads is what makes this actionable rather than annoying.
Completeness is derived and fails closed.connections_missing_totalcount reads the real QUERY text so the self-check cannot drift from what's actually asked for, PAGE_CAP is likewise computed from the query rather than restated, and require_complete raises Unreadable when totalCount is absent or the page is short. The docstring on Unreadable is the line I'd point at: "Never a pass, never the author's fault." That's the correct two-sided posture for a gate — an unreadable state must not approve, and must not blame the person who opened the PR either.
The type class is deliberately loose in the right direction.[A-Za-z]+ rather than the declared feat|fix|docs|sec|ci|chore list, so a type the org adds later doesn't silently stop the scope being read — with the selftest walking the declared list out oforg-standards.md and asserting each member parses. That's the direction that can't go stale: the file holds no copy of the vocabulary, and the test fails if the real list grows a member this parser can't read.
The .github note is small, and it's only interesting because you already treated the leading dot as a requirement one pattern down — so the two patterns disagreeing is the kind of thing that reads as an oversight later even though it works today.
saqlainsyed007
left a comment
There was a problem hiding this comment.
Reviewed at high effort — approving.
The gate logic is correct and precisely scoped. classify handles the three cases right — a bare number matches any repo, a repo-qualified ref matches on repo (and owner if given), and the number-seen-but-repo-wrong case is WRONG_REPO vs MISSING. parse_title reads the four measured title forms and the scope/parenthetical overlap is de-duplicated. It fails closed on an unreadable title or a partial link graph (Unreadable), and correctly PASSES drafts and titles that name no ticket (precision over recall, stated). The rollout is opt-in per repo (if: inputs.closing-ref-check), so it isn't a fleet-wide surprise. The PR-template edits also fix a now-false note (closing keywords do fire on merge now that every train repo's default branch is develop).
I checked @saadqbal's open thread and agree with their own conclusion: it is not a live defect. SCOPE_REPO_RE's repo class [A-Za-z0-9][A-Za-z0-9._-]* can't match a leading dot, but a scope is always inside parentheses and PAREN_REPO_RE ([A-Za-z0-9.]) matches anywhere in the title, so a .github#N scope is still detected via that path. Worth aligning the two character classes as a one-char consistency nit so the requirement isn't stated for one pattern and silently relied-upon-elsewhere for the other — but non-blocking, and your call.
Green: all checks pass, mergeable, no open Bugbot threads.
…ps depending on a coincidence (backend#2364) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 24, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9b0e6d2. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
Approving on 9b0e6d2d. Fix verified in the source and by running it:
SCOPE_REPO_RE = ^(?:([A-Za-z0-9][A-Za-z0-9._-]*)/)?([A-Za-z0-9.][A-Za-z0-9._-]*)#(\d+)$
PAREN_REPO_RE = \(\s*(?:([A-Za-z0-9][A-Za-z0-9._-]*)/)?([A-Za-z0-9.][A-Za-z0-9._-]*)#(\d+)\s*\)
The two repo classes now agree, and parse_title reports the right provenance:
'ci(.github#300): arm the stale sweep' -> [('.github', 300, 'scope')] # was 'parenthetical'
'ci(backend#300): arm the stale sweep' -> [('backend', 300, 'scope')] # unchanged
'ci(300): bare' -> [(None, 300, 'scope')] # unchanged
So the label is accurate for the one input where it wasn't, and — the part I actually cared about — detection of a .github scope no longer depends on PAREN_REPO_RE happening to reach into the scope position. Narrowing that pattern later can't now silently stop the gate seeing tickets in the repo it lives in.
Reproducing my measurement before applying it, and quoting the before/after in the thread, is the right instinct — I'd have wanted that even if the answer had come back the other way.
Also nice that you added the selftest case; a fix whose only evidence is a reviewer's shell session is one refactor from regressing.
One thing I noticed and am explicitly not asking for: a title naming the same ticket twice at different qualification levels — ci(.github#300): … (tracebloc/.github#300) — yields two refs, because the dedupe key is (owner, repo, number) and "" ≠ "tracebloc" for the owner. So the remedy would name that ticket twice. It's cosmetic, and it behaved identically before this change (two refs, both labelled parenthetical), so my fix didn't introduce it. Mentioning it only in case you'd rather normalise the owner into the key while you're in here — not a request, and I wouldn't hold anything for it.
Uh oh!
There was an error while loading. Please reload this page.
…#320) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Merging a fix never closed or advanced its ticket. The board automation is present, wired and green — and structurally unable to act, because the input it needs is never produced.
closingIssuesReferenceswas 0 on 7 of 7 sampled merged PRs: release-train#109, release-train#108, .github#304, .github#300, backend#2266, client#774, docs#131.The house convention puts the ticket in the PR title (
fix(2302): summary). GitHub creates a closing link only from a keyword in the body. A title reference is inert, sokanban-closure-router.ymlfires, finds no linked issue, and correctly does nothing. Every kanban workflow reported success. They were right to.This adds a
closing-refjob that parses the real title and asserts the real link graph contains what it names.Related
Closes tracebloc/backend#2364
What it checks
Exactly one thing: title-to-link agreement. If the title names a ticket, the PR's link graph must contain that ticket.
It deliberately does not require that a PR have a ticket. A title naming none is reported and passes —
chore(global_model): delete dead TF FLOPS path(backend#2333) names no ticket and links backend#2288 correctly. Requiring a ticket per PR is a separate decision.Derived, not restated (rule 1). Two live reads — the real title, the real
closingIssuesReferences(the same graph the router consumes). No list of tickets, repos or authors anywhere in the file.The four title forms are measured, not imagined:
fix(2218):— bare number in the scopefix(#349):feat(kanban): … (backend#2348)— repo without ownerfix(hop): … (release-train#110)— same repoA bare
#Noutside parentheses is deliberately not read as a ticket. backend#2309 is titledtest(platform): pin the three early-close call sites #2271 rewrote—#2271is prose about a PR, and its real closing ref is backend#2308. Scanning loose#Nwould have reddened a compliant PR. Precision over recall.A bare number resolves against no repo:
fix(2218)in client-runtime means backend#2218,fix(90)in release-train means release-train#90. Both are real and measured, so inventing a repo for a bare number would be this file holding a mapping of its own. A bare number is satisfied by that number in any repo; nothing stronger is claimed.The cross-repo trap, as its own verdict
A bare
Closes #2302insiderelease-trainresolves againstrelease-train, notbackend. Two outcomes, reported separately because the remedies differ:MISSING— no such local issue, nothing links at all. A line has to be added.WRONG_REPO— a local issue with that number exists, so the PR links and on merge closes the wrong issue..githubis where this bites: its own issues are in the 300s and it references backend tickets in the 300s and 2000s alike, soCloses #304in.githublinks.github#304. A line has to be rewritten to the fullowner/repo#numform.Fails closed — "cannot tell" is its own reported state (rule 3)
Each exits 2. Never a pass, and never counted as a finding against the author, because these are malfunctions of the check:
errors[], orpullRequest: nulltotalCount > len(nodes)repository.nameWithOwneror no integernumbertotalCount(which would make the test above inert) — checked before any readThe truncation test is load-bearing for a reason beyond pagination: a link to an issue the token cannot read comes back missing from
nodeswhiletotalCountstill counts it. That is indistinguishable from "not linked" and would redden every compliant cross-repo PR. Hence the org-scoped App token, and hence this refusal if the read is ever partial — it blames the check, not the author.SOFT_FAILgoverns findings only; every malfunction above exits 2 either way. Same splitaction-pins-soft-failkeeps incode-quality.yml.Fixtures are the measured shape (the backend#2114 lesson)
#2114 encoded
login.endswith("[bot]")while the real API returns{"is_bot": true, "login": "app/dependabot"}— the test passed while the code could not fire. Every fixture here was captured withgh api graphqland re-verified against the live API before this commit:{"number":365,"title":"fix(2218): three age checks compared local wall-clock against UTC", "isDraft":false,"closingIssuesReferences":{"totalCount":1, "nodes":[{"number":2218,"repository":{"nameWithOwner":"tracebloc/backend"}}]}}Tests
102 selftest assertions; 34 mutations, 0 stale, 0 uncaught.
make checkgreen.scripts/closing-ref-gate.pyand re-runs the real suite. There is no inline copy of any rule; that shape drifted from production twice here (.github#114, feat(code-quality): opt-in black --check format job (diff-scoped) #115).classify()is the single function both the assertions and the mutations go through.expect_unreadabletakes the substring the refusal must contain, so a case named for a truncated page cannot pass on an unrelated exception. Two mutations are caught precisely by "refused, but for the wrong reason".org-standards.md(found:chore ci docs feat fix sec) and every declared type is asserted to parse; the derivation fails closed if it finds fewer than six. Independently written observed types (test refactor perf build style) are asserted too. Mutation coverage cannot see a vocabulary gap.Arming — not required anywhere (rule 4)
Measured, not assumed:
closing-ref/set-status/set-pr-statusis a required status check nowhere — 19 repos ×develop/staging/main/master× both protection systems (classic protection and rulesets, since a ruleset-only branch 404s on the classic API). So a finding is loud and blocks no merge.Blast radius on open PRs is near zero: callers trigger on
opened, reopened, ready_for_review, converted_to_draft— notsynchronize— so a push does not re-run this on an existing PR.Measured against every open PR in the fleet with the gate's own logic: 9 pass, 11 draft (exempt), 5 name no ticket, 13 would report a finding. Naming them exactly, as the ones that would go red:
.github#306,.github#311,backend#2083,backend#2128,client#785,client#791,client#798,e2e-test-agent#208,e2e-test-agent#209,e2e-test-agent#210,frontend-app#878,rfcs#44,rfcs#45Each is fixed by adding one line to the PR body — there is no backlog to clear and nothing to migrate. Making it a required context is a separate, later decision that belongs after that backlog, not in the change that introduces the check.
Stated limitation: there is no
editedtrigger, so after an author adds theClosesline the job does not re-run by itself — the check has to be re-run manually. The gate's own failure message says so. Addingeditedmeans touching 19 per-repo callers and belongs in its own PR.Placement — why a job in an existing reusable
Three constraints, and
set-pr-status.ymlis the only host satisfying all three:tracebloc/backend, and nearly every repo is private. This file already mints an org-scoped installation token (repository_selection: all) and all 19 callers passsecrets: inherit.code-quality.ymlcannot host it: it is deliberately secretless withcontents: read, and a called workflow may not hold more than its caller grants — addingpull-requests: readthere would fail all 16 callers at startup, with no jobs, before any could be updated.Code review— this file's trigger set by construction.repo-inventory.ymlrow for all 19 repos, and that file is guarded byconformance-gate.yml— a required check every other merge invalidates. A job in an existing reusable needs no row: the inventory tracks callers, one row per reusable, andset-pr-status.ymlalready has its rows.Guarded files touched: none. Not
repo-inventory.yml,scripts/caller-drift.py,scripts/tests/caller-drift-selftest.py,.github/workflows/caller-drift.yml, or.github/workflows/conformance-gate.yml— so this does not queue behind the org audit.closing-refis a separate job fromset-statusdeliberately: a finding must not stop the card being written, and a failed board write must not hide the finding. Steps inside one job short-circuit; jobs do not.Nothing from the PR is interpolated into a
run:block — the title is read from the API by the checker itself, so a title containing shell is data, never code.Also
The PR template's Related section and checklist now say the link is required when the title names a ticket, and spell the cross-repo form. The old note claiming closing keywords "do not fire on merge" predated the fleet's move to
developas the default branch and is corrected.Test plan
make checkgreen (ruff, shellcheck, house-rules, action-pins, mint-scope, actionlint, mutations-dry, all selftests)closing-ref-gate-selftest.py— 102 assertionsclosing-ref-gate-mutations.py— 34 mutations, 0 stale, 0 uncaught#2364andbackend#2364; body linkstracebloc/backend#2364Note
Medium Risk
Org-wide CI on 19 callers that mints an App token and reads cross-repo issue graphs. Not a required check, so it cannot block merges, but a logic bug could still close or fail to close the wrong tickets once authors follow its advice.
Overview
Title-named tickets must now be linked in the PR body. House titles put the ticket in the subject (
fix(2218): …); GitHub only createsclosingIssuesReferencesfrom a body keyword, sokanban-closure-routerwas green while cards never moved.A new
closing-refjob onset-pr-status.yml(same event as the board write, org-scoped App token withissues/pull_requestsread) parses the live title and asserts the live link graph. Drafts and titles that name no ticket pass. Missing vs wrong-repo (bareCloses #Nin the wrong repo) are separate findings. API/permission failures exit 2 and are not softened. The job is not a required check.The PR template now requires an owner-qualified
Closesline when the title names a ticket, and corrects the old note that closing keywords do not fire ondevelop. Selftest + mutation harness are wired intomake check.Reviewed by Cursor Bugbot for commit 9b0e6d2. Bugbot is set up for automated code reviews on this repo. Configure here.