Uh oh!
There was an error while loading. Please reload this page.
fix(closing-ref): re-run when the title changes, and let a child PR be truthful (backend#2556) - #360
Conversation
…e truthful (backend#2556, backend#2616) Two defects in one gate, fixed together because either alone leaves the other's remedy unusable. BACKEND#2556 -- THE GATE READS THE TITLE AND NEVER RE-RUNS WHEN IT CHANGES. `closing-ref` decides its verdict from the PR title and body; the callers triggered on `[opened, reopened, ready_for_review, converted_to_draft]`, and `edited` is the only event GitHub fires when either field changes. So the two inputs the gate reads were the two inputs that could change without re-running it. That is a bypass, not a gap: open a PR titled `chore: tidy up`, the gate records NOTHING_NAMED and goes green, then retitle it to `fix(1234): ...` with nothing linked -- no event fires, the green stands. It also made the remediation unusable in the other direction: 20 sync PRs had to be cleared with 20 manual `gh run rerun` calls (backend#2555). Same one-word fix, same reason, as `fr-gate-caller.yml` (backend#1945). A CONSEQUENCE THAT HAD TO BE GUARDED, not discovered later. Every trigger this workflow had was reachable only on an OPEN PR, so `set-status` writing Status unconditionally was always correct. `edited` is not: GitHub fires it when a MERGED PR's title or body is edited, and this is the one board writer in the fleet that does not compare pipeline rank. Without the new guard, fixing a typo in a shipped PR's description would drag its card from `Prod` back to `Code review`. Both jobs are now `if: ...state == 'open'`. BACKEND#2616 -- ONE SATISFYING FORM, AND A CHILD PR CANNOT TRUTHFULLY USE IT. The checker read `closingIssuesReferences` and nothing else, and that field is populated only by a closing keyword. So a PR whose title named a ticket had exactly one satisfying form: promise to close it. For this org's most common shape -- a parent ticket with several child PRs titled `type(NNNN): ...`, none of which finishes it -- the only remedies were a FALSE `Closes`, or deleting the number from the title, which passes the check by removing the traceability the check exists to enforce. Four PRs in one day took the second (.github#349, #350, #352, #353) and #354 wrote that workaround into the canon. A declared NON-CLOSING body reference now satisfies the title, reported as its own state (`MENTIONED`) so a green run still says which promise was made. The closing link stays the stronger form, and `WRONG_REPO` is decided BEFORE any body reference is consulted -- a truthful `Part of tracebloc/backend#304` must not mask a `Closes#304` that closes `.github#304` on merge. DERIVED, NOT RESTATED (rule 1). The non-closing vocabulary is PARSED out of `org-standards.md`, which already declares the org's partial-work form. Add `Refs <owner>/<repo>#N` to the canon and the gate accepts it with no code change. The derivation fails closed in both directions: an unreadable canon and a canon declaring no non-closing form are separate cannot-tells (exit 2), never a silent reversion to closing-only -- reverting is the defect itself. GitHub's closing keywords are the one list here that cannot be derived from this org, so they are named, cited, and used only to subtract. The canon and CLAUDE.md carried two statements this change makes false -- that the check fails unless the body carries `Closes`, and to keep the number out of the subject. Both are rewritten; classify() reports IN_SYNC. EVIDENCE closing-ref-gate-selftest 156 assertions, all passed (was 109) closing-ref-gate-mutations 53 mutations, 0 stale, 0 uncaught (was 36) ruff / actionlint / shellcheck / house-rules clean standards-sync selftest 60 checks, 0 failed The mutation harness now rewrites the two WORKFLOW files as well as the checker: `edited` and the open-state guards are guarantees declared in YAML, and rule 5 does not exempt a guard for living in a different language. Part of tracebloc/backend#2556 Part of tracebloc/backend#2616 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lling the suite The multi-word-keyword mutation was scored UNCAUGHT for the right reason: it made `evaluate` raise, which killed the suite mid-run, and the harness refuses to count a broken harness as coverage. `evaluate` grew a path that can raise for a NEW reason (the derivation from org-standards.md coming back empty), so the positive cases now go through a wrapper, exactly as `value` already does for every other call. Part of tracebloc/backend#2616 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ead of crashing `value` returns the exception TEXT on a refusal, and a string is iterable -- so a case that walked the derived list iterated characters and raised IndexError on the space in "RAISED Unreadable: ...". Normalised once. Found by the multi-word-keyword mutation, which is the job: it was scored UNCAUGHT for breaking the harness rather than being caught by a case. Part of tracebloc/backend#2616 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Not a verdict yet — five checks still running. One thing you can fix while they do:
the title cites backend#9999999, which doesn't exist (Could not resolve to an issue or pull request with the number of 9999999). The body is correct — Closes tracebloc/backend#2556 and Closes tracebloc/backend#2616 both link properly — so this is title-only and the gate itself will pass, since it reads GitHub's computed closingIssuesReferences and not the subject line. It's just going to read oddly forever on the one PR whose subject is ticket-reference correctness.
On the substance, both findings look right to me and the second one is the sharper:
"The only remedies were a false Closes, or deleting the number from the title, which passes the check by removing the traceability the check exists to enforce." That's the shape worth naming — a gate satisfiable by deleting its own subject. Four PRs in one day going red for correctly saying Part of is the measurement that makes it a defect rather than a preference.
And #2556 being a bypass rather than a gap is the right classification: chore: tidy up records NOTHING_NAMED and goes green, then a retitle to fix(1234): … fires no event and the green stands. Twenty manual gh run rerun calls to clear the sync batch is the same bug pointing the other way.
Worth saying explicitly since I approved it: this supersedes the advice in #354, which I reviewed two days ago. That was the docs workaround written after those four red PRs; fixing the gate is the better answer and reverting the advice with it is correct, not a regression.
I'll pick this up for a verdict once the checks land.
saadqbal
left a comment
There was a problem hiding this comment.
Title fixed — (backend#2556) now, and the head is unchanged at bcf026e8, which is a fair demonstration of the bug this PR closes: the correction is invisible in the SHA, so nothing that keys on the head would notice it. Re-reviewing meant re-reading the title, not re-reading a diff.
Verified the substance rather than resting on my earlier comment:
edited is the whole of the #2556 fix and the reasoning is right — it is the only event GitHub fires when a title or body changes, so without it the gate's verdict outlives the thing it judged. Same one-word change as fr-gate-caller.yml under backend#1945, which is the right precedent to reuse rather than invent a mechanism.
The second-order consequence is the part I'd have gone looking for, and you found it first. Adding edited means GitHub also fires on a merged PR's title or body edit, and this job would then write Code review to a card long past it. Before edited, every trigger implied an open PR, so the unconditional write was always correct — the new event breaks an invariant the old ones held implicitly. That is exactly the class of thing that ships.
And guarding on state == 'open' rather than action != 'edited' is the better fix for both stated reasons. The event-name guard would have kept the demotion out and disabled the board write for a draft toggled by an edit — and it would encode the accident of which event exposed the bug rather than the rule. "The rule is that this job describes an open PR's review state" is the invariant; guarding on it means the next trigger someone adds is covered without a second thought.
The #2616 half stands as I read it before: closingIssuesReferences being populated only by a closing keyword left a child PR choosing between a false Closes and deleting the number — "which passes the check by removing the traceability the check exists to enforce." Four PRs red in one day for correctly saying Part of is the measurement that makes it a defect.
Green, no threads, 411 lines of selftest and 113 of mutations behind it. 👍
Closes tracebloc/backend#2556
Closes tracebloc/backend#2616
Two defects in one gate, fixed together because either alone leaves the other's remedy unusable: #2616 widens what satisfies the check, and #2556 is what lets a PR act on that by editing its body.
Verified first, because three tickets in this queue turned out already fixed
Measured on
origin/developbefore writing anything:set-pr-status-caller.ymlomitseditedtypes: [opened, reopened, ready_for_review, converted_to_draft]closing-ref-gate.pyreads onlyclosingIssuesReferencesbodyfield and no other link source616daa0(#314, the original gate) /2017a1c(#349, an unrelated mint scope)Neither was fixed.
a28adf2(#354, merged today) is the docs workaround for #2616 — it wrote "keep the number out of the subject" into the canon after four PRs in one day (#349, #350, #352, #353) went red for correctly sayingPart of. This PR fixes the gate and reverts that advice.backend#2556 — the gate reads the title and never re-runs when the title changes
closing-refdecides its verdict from the PR title and body;editedis the only event GitHub fires when either changes. So the two inputs the gate reads were the two inputs that could change without re-running it.That is a bypass, not a gap: open a PR titled
chore: tidy up, the gate recordsNOTHING_NAMEDand goes green, then retitle it tofix(1234): …with nothing linked. No event fires, the green stands. It also made remediation unusable in the other direction — 20 sync PRs had to be cleared with 20 manualgh run reruncalls (backend#2555).Fixed with the same one-word change, and the same reasoning comment, as
fr-gate-caller.ymlunder backend#1945 (fd31ef4/ .github#338).A consequence that had to be guarded, not discovered later
Every trigger this workflow had was reachable only on an open PR, so
set-statuswriting Status unconditionally was always correct.editedis not: GitHub fires it when a merged PR's title or body is edited, and this is the one board writer in the fleet that does not compare pipeline rank (advance-deploy-env.ymldoes).Without a guard, fixing a typo in a shipped PR's description would drag its card from
Prodback toCode review. Both jobs are nowif: …pull_request.state == 'open', and both conditions are mutation-pinned.backend#2616 — one satisfying form, and a child PR cannot truthfully use it
closingIssuesReferencesis populated only by a closing keyword, so a PR whose title named a ticket had exactly one satisfying form: promise to close it. For this org's most common shape — a parent ticket with several child PRs titledtype(NNNN): …, none of which finishes it — the only remedies were a falseCloses, or deleting the number from the title, which passes the check by removing the traceability the check exists to enforce.A declared non-closing body reference now satisfies the title, reported as its own state (
MENTIONED) so a green run still says which promise was made:That is
.github#356, the PR #2616 was filed from, passing truthfully.WRONG_REPOis unchanged and is decided before any body reference is consulted — a truthfulPart of tracebloc/backend#304must not mask aCloses #304that closes.github#304on merge. A title naming a ticket referenced no way at all still fails; that was the real defect and it still fires.Derived, not restated
The non-closing vocabulary is parsed out of
org-standards.md, which already declares the org's partial-work form. AddRefs <owner>/<repo>#Nto the canon and the gate accepts it with no code change. The derivation fails closed in both directions — an unreadable canon and a canon declaring no non-closing form are separate cannot-tells (exit 2), never a silent reversion to closing-only, because reverting is the defect itself.GitHub's closing keywords are the one list here that cannot be derived from anything in this org, so they are named, cited, and used only to subtract. A mutation empties them and the suite reddens.
Docs updated in the same PR
The canon and
CLAUDE.mdcarried two statements this change makes false — that the check fails unless the body carriesCloses, and to keep the number out of the subject. Both rewritten;standards-sync.classify()reportsIN_SYNC. The 18 other repos'CLAUDE.mdcopies will drift untilstandards-sync.ymlruns, which is its normal remediation path.Evidence
Two of the new mutations were genuinely instructive rather than decorative: the multi-word-keyword mutation was first scored UNCAUGHT because it killed the suite instead of reddening a case (
valuereturns the exception text, and iterating that string raisedIndexError). Both follow-up commits exist because the harness refused to count a broken harness as coverage.The mutation harness now rewrites the two workflow files as well as the checker.
editedand the open-state guards are guarantees declared in YAML, and rule 5 does not exempt a guard for living in a different language.The fleet still needs the one-word caller change — measured, not assumed
set-pr-status.ymlis a reusable inrepo-inventory.yml, not acopy(onlyadd-to-kanban.ymlis byte-compared), andcaller-drift.pymatches onuses:content. So this PR does not put the fleet into drift, and.githubdid not need to be sequenced last.caller_inputsmodelscode-quality.ymlonly — nothing in the org asserts caller trigger types today.Measured 2026-08-27 across all 20 repos carrying the caller (filename is
set-pr-status.ymlin 18 of them,set-pr-status-caller.ymlhere, andrelease-train's lives onmain): 20 of 20 are missingedited. This PR fixes.github's own. The other 19 need the identical one-word change to their own copies before the bypass is closed for them — the same 19-PR shape backend#1945 used.Also worth naming: callers pin
@main, andquality-refdefaults to"main", so the checker change takes effect fleet-wide only after the normaldevelop → staging → mainpromotion. Nothing here shortcuts that.Live proof of backend#2556, run on this PR
editedis on this PR's head branch, and GitHub takes apull_requestworkflow definition from the PR's own ref — so the fix was testable before merging. #2556 asks for exactly this test: retitle a PR and require the check to re-run.Set PR Statusruns on this branch, allpull_request, zero pushes after the first:opened— the only run the old trigger set would have producededited(GitHub processing the body's cross-references)editededited— title changed to(backend#9999999), a ticket this PR links nowhereedited— title restoredTwo deliberate title edits, two new runs, no push. Under
[opened, reopened, ready_for_review, converted_to_draft], runs 2–5 do not exist — that is the bypass: the verdict from run 1 would have stood over both retitles.Worth recording, because it pins down what the fix had to be: the checker reads the title live from the API, not from the event payload, so a queued run self-corrects to the current title. The verdict therefore tracks the title by construction, and the only thing ever missing was an event to trigger a run at all. That is precisely what #2556 diagnosed, and the trigger is the whole fix.
Run 4 was still queued on GitHub's backlog when the title was restored, so its red was never observed live. The red path is covered by the selftest and by the mutation that strips
editedfrom the caller.Deliberately not done
set-status / closing-refwas not armed as a required context. It is required nowhere (#2616 measuredactionlint,gate,quality / *,selftestson.github/develop), and arming a gate is a separate decision that belongs after the compliance backlog — rule 4, and the reusable's own header says so.caller-drift.py; that is real work, not a line here.#Nthis gate deliberately refuses in a title. The explicit declared keyword is the whole signal.🤖 Generated with Claude Code