Uh oh!
There was an error while loading. Please reload this page.
fix(2555): the sync PR title stops naming an epic it must not close - #345
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
This is the right fix and the right root cause. _ensure_pr() having no coverage at all is the real answer to "why it shipped" — the selftest was thorough about the splice and never looked at the PR being opened, so the title was free to contradict a required gate indefinitely.
Importing parse_title from the real closing-ref-gate.py rather than restating a regex is the part I'd point at: the generator and the gate can't drift apart now, and the mutation anchor (the pre-fix title is seen as naming a ticket) makes check 1 non-vacuous rather than passing because parse_title went blind. Failing closed if the import can't be loaded is the right third leg.
One gap, in the check whose name is broader than its assertion. Check 2 is titled "the body keeps traceability WITHOUT a closing keyword" but asserts:
"backend#1602"inbodyand"Closes"notinbodyGitHub honours three keyword families, not one: close/closes/closed, fix/fixes/fixed, resolve/resolves/resolved. So a future edit to Fixes tracebloc/backend#1602 or Resolves tracebloc/backend#1602 passes this test and closes the epic on the first of nineteen merges — which is the exact disaster this PR exists to prevent, arriving through the door the test leaves open.
The fix is the same move you already made for the title: derive the keyword set rather than name one of it. Cheapest version is a tuple of the three stems checked case-insensitively; better, if the gate ever grows a CLOSING_KEYWORDS constant, import that the way you import parse_title. Worth noting the gate's own prose already discusses Closes as one keyword ("linked by a closing keyword"), so the concept exists — it just isn't exported yet.
Holding on bugbot / review, selftests and Cursor Bugbot, all pending. Ping me when they land.
…st Closes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 26, 2026
You're right, and the gap was worse than "broader name than assertion" — it was the exact failure mode this PR exists to prevent, reachable in one word. Fixed in On importing it rather than writing it: I looked, and Your suggestion also caught a bug in my own anchor, which is worth naming because it changes what the check proves. My first version asserted Mutation-proved against the real generator by rewriting its body line to each family in turn: The pre-fix assertion would have caught only the third. One thing your review surfaced that I've filed separately rather than folded in here: backend#2556 — |
LukasWodka
commented
Aug 26, 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 1661a49. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
1661a499 covers all three families, and the reason you give for writing the tuple rather than importing it is the right one — closing-ref-gate.py genuinely has no such constant, because it delegates to GitHub's computed closingIssuesReferences and never scans text. Leaving "if it ever grows one, import it the way parse_title is imported and delete this tuple" is better than either alternative.
One more, same shape one layer in.closing_keyword_in matches on f"{k} " in low — keyword followed by a space:
returnnext((kforkinCLOSING_KEYWORDSiff"{k} "inlow), None)So Fixes: tracebloc/backend#1602 doesn't match: low contains fixes:, not fixes . The colon form is one people write, and if GitHub honours it — which I can't test from here, but I wouldn't bet the epic on it not doing — the assertion passes and the first of nineteen merges closes #1602 anyway. That's the same door, still ajar by one character.
I'd match the separator rather than assume it: keyword followed by space or colon, e.g. re.search(rf"\b{k}\b\s*:?\s", low) or simply testing both f"{k} " and f"{k}:". Cheap, and it removes the need to be right about GitHub's parser — which is the better property for a check whose false negative costs an epic.
Worth saying that the check is otherwise the right shape now, and the failure mode it guards is worth this much fussing over precisely because nothing else would catch it: the epic closes, the other eighteen PRs lose their tracking ticket, and the only symptom is a closed issue nobody closed on purpose.
Held on one pending check. Ping me and I'll approve — this is the last thing I have.
Closes tracebloc/backend#2555
Summary
create-prsopened every sync PR titleddocs(claude): sync org-standards block (backend#1602).closing-ref-gate.pyrequires every ticket a title names to appear in that PR'sclosingIssuesReferences— and these PRs must not close #1602, because one is opened per repo and the first to merge would close the epic for all of them. The body saysPart of tracebloc/backend#1602on purpose.So the remediation path generated PRs its own required gate refuses. Observed on today's dispatch, on every repo it touched:
The title is the only defect — the body was already right. The commit-message template at
standards-sync.py:271keeps its(backend#1602); commit messages are not parsed by the gate, and there the reference is pure provenance.Why it shipped
_ensure_pr()had no selftest coverage whatsoever. The offline selftest is thorough about the splice and its fail-closed paths and never looked at the PR being opened, so the title was free to contradict a required gate indefinitely.Type
fix
Test plan
30 checks, 0 failed— 3 new, all in_ensure_pr:backend#1602without a closing keywordCheck 1 derives its rule by importing
parse_titlefrom the realclosing-ref-gate.pyrather than restating a regex, so the generator and the gate cannot drift apart — the failure mode this repo has hit twice by re-implementing a rule inline.Check 3 exists because a vacuous assertion and a live one are indistinguishable in a green log. Mutation-proved both directions:
Follow-up (not in this PR)
The ~19 PRs already opened need retitling by hand —
_ensure_prreturns early when an open PR exists, so re-dispatching will not correct them. I am doing that separately.Checklist
developNote
Low Risk
Automation-only change to PR title text and offline selftests; no runtime product or security paths affected.
Overview
Org-standards sync remediation was opening PRs whose titles included
(backend#1602), which closing-ref-gate treats as naming a ticket that must appear inclosingIssuesReferences. Those PRs intentionally do not close the shared epic—one PR per repo—so the gate blocked merge across the fleet.standards-sync.pynow creates PRs with titledocs(claude): sync the org-standards block(no ticket in the title). The body still usesPart of tracebloc/backend#…for traceability without a closing link. Commit messages keep the(backend#…)reference; only PR titles changed.standards-sync-selftest.pyadds_ensure_prcoverage: imports the real gate’sparse_title, asserts the generated title names no ticket, asserts the body mentions the epic without close/fix/resolve keywords, and includes mutation anchors so regressions fail visibly.Reviewed by Cursor Bugbot for commit 1661a49. Bugbot is set up for automated code reviews on this repo. Configure here.