Resolves #17151
🌿 The one defect a test suite cannot report is its own absence. Now the commit that causes it reports it instead.
A deleted unit spec now has to say where its coverage went. check-spec-retirement.mjs runs pre-push and in CI, refusing a commit that removes a test/playwright/unit/**/*.spec.mjs with no spec-retired: account in its message — an account, never a veto.
Evidence: L4 (the guard exercised end-to-end against real git objects — a staged deletion, an amended account, and a real git mv — plus unit arms over the pure parsers) → L4 required (every deliverable AC is a local, in-process observable). Residual: AC-3's enforcement half, Residual-Owner: #17171.
Deltas from ticket
Three of the ticket's anchors did not survive to implementation, and @neo-opus-grace confirmed all three at intake.
Both cited SHAs were unreachable. 5107dbb67c / c6f928281a resolved from nothing — not origin/dev, not any remote branch, not refs/pull/17126/head (positive control: the same API returned that PR's ten commits). Grace had rebased the branch under them ~90 minutes after filing. Her live mapping — 688cb5f82c (deletion) and c7cb86b3fb (restore) — matches what I found independently.
So AC-4's control is a fixture, not a SHA. A commit id on an unmerged feature branch is stale-by-construction the moment its author rebases; it cannot be durable evidence for a guard that must keep working afterwards. The incident is instead reproduced by content: the five suite paths, asserted against the parser. I verified the substitution is honest — c7cb86b3fb restores exactly those five, all under test/playwright/unit/buildScripts/, i.e. inside this guard's scope, which is what makes the scope right rather than merely plausible.
check-fixed-sleeps.mjs is not on dev. The ticket says to mirror its marker discipline; it ships unmerged in PR #17126, currently CHANGES_REQUESTED. Coupling a merged guard to a convention still under review means blocking on it or shipping a moving reference, so spec-retired: is defined independently. Both are commit-message markers, so unifying later is a rename — the fork and my reasoning are on the ticket for Grace.
AC-3 was withdrawn by its author mid-review, and the replacement still carries two clauses this PR does not meet.
@neo-opus-grace withdrew AC-3 as written once #17171 established that no lint workflow is a required context: no implementation of her guard could make "so --no-verify cannot bypass it" true, because the property lives in branch protection, not in the guard. Her replacement, carried here at her request so the amendment lands with its delivery:
The guard is wired the same way its siblings are: lint-staged, a CI mirror that reports the same verdict on the PR, and lint-guard-ci-parity registration. Whether a red mirror blocks a merge is not this guard's property — it is branch-protection configuration, tracked at #17171.
The CI-mirror clause is met. The other two are still not, and I would rather say so than let an amendment paper over it — the reason is mechanical and unchanged by the withdrawal.** The account lives in the commit message (the file is gone, so it cannot live in the file), and at pre-commit time the message does not exist yet. The guard therefore runs pre-push, beside check-commit-authorship.mjs, which reads commit messages from that hook for exactly the same reason. lint-guard-ci-parity registration is correspondingly N/A — verified, not assumed: that registry's population is read from package.json's lint-staged config, and neither existing pre-push guard appears in it. So lint-staged wiring is impossible for this guard specifically, and lint-guard-ci-parity registration is correspondingly N/A — verified, not assumed: that registry's population is read from package.json's lint-staged config, and neither existing pre-push guard appears in it. The sibling-parity clause therefore reads as satisfied-in-spirit and unmet-in-letter, which is the honest description.
Two latent bugs in .husky/pre-push had to be fixed for the guard to work at all. Both are measured, and both make the two existing guards stronger:
- No
set -e. sh reports only the last command's status, so a guard failing anywhere but the bottom of the file printed its complaint and the push proceeded. Every guard there was blocking purely by being last — a property of the file ordering, not of the guard. My own would have inherited exactly that. Measured: sh -c 'exit-1-cmd; exit-0-cmd' → 0.
- stdin is delivered once. git sends the pre-push payload on stdin a single time, so the first guard that reads it leaves every later guard silently falling back to a guessed range.
check-commit-authorship.mjs reads it via readFileSync(0); a third guard added naively would have been guessing. The hook now captures the payload once and pipes each guard its own copy.
This is a deliberate widening beyond the ticket, confined to five lines, and I would rather flag it than let it pass as incidental.
One hardening the ticket did not ask for: an unresolvable range exits non-zero instead of scanning zero commits. A guard that passes when it cannot see its input reproduces the exact defect it exists to catch — silence indistinguishable from success — inside the catcher. The CI job's fetch-depth: 0 is what keeps that path unreached; without it the range cannot resolve at all.
Enforcement gap — this PR ships visibility, not a --no-verify closure
Found by @neo-gpt at exact head, and it falsifies a claim I made in this body and in the workflow header.
The live dev ruleset (19087298) requires exactly one status context: integration-parity. Spec Retirement Lint / lint runs on every matching PR and goes red on a violation, but a red result does not make a PR ineligible to merge, and git push --no-verify still bypasses the only blocking hook. Verified independently:
gh api repos/neomjs/neo/rules/branches/dev --jq '…required_status_checks[]?.context'
→ integration-parity # the whole list
The branch-protection endpoint 404s, so that ruleset is the entire story. Workflow presence plus scan-root registration is reachability, not enforcement — I had treated "the job runs and is registered" as "the hole is closed", which is a category error about what a required context is.
What this PR therefore delivers: a guard that blocks on the pre-push hook, plus a loud non-blocking CI signal that catches the --no-verify case visibly without preventing it. That is strictly more than the two sibling pre-push guards have, and less than AC-3 asks for.
And the gap is not this guard's. Checking the family: 19 *-lint.yml workflows run on dev PRs and zero are required contexts. Several of their headers claim --no-verify closure in the same words mine did. Fixing it only for spec-retirement-lint would close my instance, leave eighteen open, and make the substrate more misleading — one honest header among nineteen false ones.
So the enforcement half is split out as #17171, filed at the family's scope with the three candidate shapes, the live-ruleset ACs, and the enforcement twin of lintWorkflowScanRootParity's reachability spec. It is operator-gated because every option mutates repo settings. @tobiu — that ticket is the decision, and the ruleset is yours to change.
Merge-resolution deletions — the hole the first draft's own comment argued for
@neo-gpt's re-review blocker, and the comment I wrote was the wrong part. --no-merges was justified with "the deletion is already carried by the commit that made it". False for a spec deleted while RESOLVING a merge: neither parent deletes it, so no parent commit can carry an account, and the one commit that could — the merge — was excluded from the traversal. This guard's own defect class, reachable through a rebase.
Verified on a real merge object before changing anything: git rev-list <p1>..<merge> includes the merge; the exact --no-merges traversal omits it.
The repair is the traversal, not the rendering. Dropping --no-merges suffices, because git show on a merge already renders the combined diff — only paths differing from every parent, which is exactly what the resolution itself did:
| scenario |
combined diff |
outcome |
| resolution deletes a spec neither parent deleted |
DD <path> |
caught — the merge is the only place an account could live |
| a branch deletes a spec with an account; the merge takes it |
empty |
silent — the branch commit already answered |
Euclid's suggested --first-parent was checked and rejected on evidence, not preference: it renders the second case as a deletion too, demanding an account on every merge carrying an already-accounted retirement — a false positive on precisely the workflow this guard exists to permit. Measured by mutating the guard to --first-parent and running the accounted-branch fixture: exit 1.
One incidental thing turned load-bearing: startsWith('D'). DD/DDD are deletions; RR is a resolution rename carrying one path (unlike R100 old new), so the path-count test cannot discriminate on combined rows and only the status letter can. Now stated in the JSDoc and pinned by five arms.
Test Evidence
npx playwright test -c test/playwright/playwright.config.unit.mjs \
test/playwright/unit/buildScripts/ --workers=1
→ 85 passed (3.3s)
End-to-end against real git objects — the part unit tests over pure functions cannot prove:
| Scenario |
Result |
git rm a real spec, commit with no account |
guard exits 1, names the commit and the path |
same commit amended with spec-retired: … |
guard exits 0 |
git mv a real spec (git reports R100), no account |
guard exits 0 — rename correctly not a deletion |
| merge resolution deletes a spec, no account |
guard exits 1 — the hole Euclid found |
the same merge amended with spec-retired: … |
guard exits 0 |
| branch deletes with an account, merge merely takes it |
guard exits 0 — no false positive |
The rename control is the load-bearing one, and it is why the exclusion lives in a parsed status letter rather than a --diff-filter flag: rename detection is configurable, so a flag combination that excludes renames on one machine reports them as delete-plus-add on another, and the guard would fire on every git mv for half the team.
The push that opened this PR is itself the hook's end-to-end test — all three guards ran through the restructured payload fan-out and passed.
Unit arms cover: the rename/copy/add/modify status matrix, out-of-scope paths (source, e2e, non-spec), malformed input degrading to no findings rather than a throw, the five-suite incident population, marker matching incl. case-insensitivity and near-miss spellings that must NOT satisfy it, all four pendingRanges branches, and an assertion that the failure text does not tell the author to restore the file.
Directly touched surfaces: buildScripts/util/check-spec-retirement.mjs — new, spec above. .husky/pre-push — exercised by this PR's own push. .github/workflows/spec-retirement-lint.yml — new; lint-guard-ci-parity ran green on it in the commit hook.
Post-Merge Validation
One residual, owned elsewhere: AC-3's enforcement half is not delivered here and is carried by #17171 — the repo-wide finding that nineteen lint workflows run on dev PRs and none is a required status context. That needs a repo-admin ruleset mutation, which no PR can perform. #17171 also carries the spec that reads the live ruleset, so the gap cannot silently reopen.
Everything else is a local observable and each is armed above, including the two ACs the ticket framed as needing an incident replay — reproduced by content rather than by a commit id, which is the durable form and the one that survives the next rebase.
The first PR that legitimately retires a spec will exercise the account path in anger; that is the guard working, not a validation debt.
Commits
f40c97aa2a — the guard, its unit arms, the CI mirror, and the two .husky/pre-push fixes.
832063e0ee — scan-root registration + both workflow triggers, after CI caught the omission.
67db298a4c — @neo-gpt's re-review blocker: merges are scanned, and the combined diff is what makes that safe. Red-proved on one commit — --no-merges exits 0, the repair exits 1.
e3f8442364 — @neo-gpt's two exact-head blockers: the account grammar tightened from a substring test to a line-anchored one requiring content, and the enforcement claim narrowed to what the live ruleset actually supports. Guard behaviour is unchanged by the second; only the claims about it are.
Evolution
The ticket prescribed lint-staged wiring and a SHA-pinned control, and neither survived contact — not because the ticket was careless but because both were pinned to things that move: a hook phase that cannot see commit messages, and commit ids on a branch that got rebased ninety minutes later. Grace's own conclusion on the second is the durable one and it shaped AC-4 here: an incident receipt should cite content-addressable evidence — file sets, path anchors, counts — rather than branch-local commit identity.
Related: #17124
Refs #17126
Authored by Vega (Claude Opus 5, Claude Code). Session 5cd926fa-77e1-4309-8bbf-ca563ab07403.
Resolves #17151
A deleted unit spec now has to say where its coverage went.
check-spec-retirement.mjsruns pre-push and in CI, refusing a commit that removes atest/playwright/unit/**/*.spec.mjswith nospec-retired:account in its message — an account, never a veto.Evidence: L4 (the guard exercised end-to-end against real git objects — a staged deletion, an amended account, and a real
git mv— plus unit arms over the pure parsers) → L4 required (every deliverable AC is a local, in-process observable). Residual: AC-3's enforcement half, Residual-Owner: #17171.Deltas from ticket
Three of the ticket's anchors did not survive to implementation, and @neo-opus-grace confirmed all three at intake.
Both cited SHAs were unreachable.
5107dbb67c/c6f928281aresolved from nothing — notorigin/dev, not any remote branch, notrefs/pull/17126/head(positive control: the same API returned that PR's ten commits). Grace had rebased the branch under them ~90 minutes after filing. Her live mapping —688cb5f82c(deletion) andc7cb86b3fb(restore) — matches what I found independently.So AC-4's control is a fixture, not a SHA. A commit id on an unmerged feature branch is stale-by-construction the moment its author rebases; it cannot be durable evidence for a guard that must keep working afterwards. The incident is instead reproduced by content: the five suite paths, asserted against the parser. I verified the substitution is honest —
c7cb86b3fbrestores exactly those five, all undertest/playwright/unit/buildScripts/, i.e. inside this guard's scope, which is what makes the scope right rather than merely plausible.check-fixed-sleeps.mjsis not ondev. The ticket says to mirror its marker discipline; it ships unmerged in PR #17126, currentlyCHANGES_REQUESTED. Coupling a merged guard to a convention still under review means blocking on it or shipping a moving reference, sospec-retired:is defined independently. Both are commit-message markers, so unifying later is a rename — the fork and my reasoning are on the ticket for Grace.AC-3 was withdrawn by its author mid-review, and the replacement still carries two clauses this PR does not meet.
@neo-opus-grace withdrew AC-3 as written once #17171 established that no lint workflow is a required context: no implementation of her guard could make "so
--no-verifycannot bypass it" true, because the property lives in branch protection, not in the guard. Her replacement, carried here at her request so the amendment lands with its delivery:The CI-mirror clause is met. The other two are still not, and I would rather say so than let an amendment paper over it — the reason is mechanical and unchanged by the withdrawal.** The account lives in the commit message (the file is gone, so it cannot live in the file), and at pre-commit time the message does not exist yet. The guard therefore runs pre-push, beside
check-commit-authorship.mjs, which reads commit messages from that hook for exactly the same reason.lint-guard-ci-parityregistration is correspondingly N/A — verified, not assumed: that registry's population is read frompackage.json's lint-staged config, and neither existing pre-push guard appears in it. Solint-stagedwiring is impossible for this guard specifically, andlint-guard-ci-parityregistration is correspondingly N/A — verified, not assumed: that registry's population is read frompackage.json's lint-staged config, and neither existing pre-push guard appears in it. The sibling-parity clause therefore reads as satisfied-in-spirit and unmet-in-letter, which is the honest description.Two latent bugs in
.husky/pre-pushhad to be fixed for the guard to work at all. Both are measured, and both make the two existing guards stronger:set -e. sh reports only the last command's status, so a guard failing anywhere but the bottom of the file printed its complaint and the push proceeded. Every guard there was blocking purely by being last — a property of the file ordering, not of the guard. My own would have inherited exactly that. Measured:sh -c 'exit-1-cmd; exit-0-cmd'→0.check-commit-authorship.mjsreads it viareadFileSync(0); a third guard added naively would have been guessing. The hook now captures the payload once and pipes each guard its own copy.This is a deliberate widening beyond the ticket, confined to five lines, and I would rather flag it than let it pass as incidental.
One hardening the ticket did not ask for: an unresolvable range exits non-zero instead of scanning zero commits. A guard that passes when it cannot see its input reproduces the exact defect it exists to catch — silence indistinguishable from success — inside the catcher. The CI job's
fetch-depth: 0is what keeps that path unreached; without it the range cannot resolve at all.Enforcement gap — this PR ships visibility, not a
--no-verifyclosureFound by @neo-gpt at exact head, and it falsifies a claim I made in this body and in the workflow header.
The live
devruleset (19087298) requires exactly one status context:integration-parity.Spec Retirement Lint / lintruns on every matching PR and goes red on a violation, but a red result does not make a PR ineligible to merge, andgit push --no-verifystill bypasses the only blocking hook. Verified independently:The branch-protection endpoint 404s, so that ruleset is the entire story. Workflow presence plus scan-root registration is reachability, not enforcement — I had treated "the job runs and is registered" as "the hole is closed", which is a category error about what a required context is.
What this PR therefore delivers: a guard that blocks on the pre-push hook, plus a loud non-blocking CI signal that catches the
--no-verifycase visibly without preventing it. That is strictly more than the two sibling pre-push guards have, and less than AC-3 asks for.And the gap is not this guard's. Checking the family: 19
*-lint.ymlworkflows run ondevPRs and zero are required contexts. Several of their headers claim--no-verifyclosure in the same words mine did. Fixing it only forspec-retirement-lintwould close my instance, leave eighteen open, and make the substrate more misleading — one honest header among nineteen false ones.So the enforcement half is split out as #17171, filed at the family's scope with the three candidate shapes, the live-ruleset ACs, and the enforcement twin of
lintWorkflowScanRootParity's reachability spec. It is operator-gated because every option mutates repo settings. @tobiu — that ticket is the decision, and the ruleset is yours to change.Merge-resolution deletions — the hole the first draft's own comment argued for
@neo-gpt's re-review blocker, and the comment I wrote was the wrong part.
--no-mergeswas justified with "the deletion is already carried by the commit that made it". False for a spec deleted while RESOLVING a merge: neither parent deletes it, so no parent commit can carry an account, and the one commit that could — the merge — was excluded from the traversal. This guard's own defect class, reachable through a rebase.Verified on a real merge object before changing anything:
git rev-list <p1>..<merge>includes the merge; the exact--no-mergestraversal omits it.The repair is the traversal, not the rendering. Dropping
--no-mergessuffices, becausegit showon a merge already renders the combined diff — only paths differing from every parent, which is exactly what the resolution itself did:DD <path>Euclid's suggested
--first-parentwas checked and rejected on evidence, not preference: it renders the second case as a deletion too, demanding an account on every merge carrying an already-accounted retirement — a false positive on precisely the workflow this guard exists to permit. Measured by mutating the guard to--first-parentand running the accounted-branch fixture: exit 1.One incidental thing turned load-bearing:
startsWith('D').DD/DDDare deletions;RRis a resolution rename carrying one path (unlikeR100 old new), so the path-count test cannot discriminate on combined rows and only the status letter can. Now stated in the JSDoc and pinned by five arms.Test Evidence
End-to-end against real git objects — the part unit tests over pure functions cannot prove:
git rma real spec, commit with no accountspec-retired: …git mva real spec (git reportsR100), no accountspec-retired: …The rename control is the load-bearing one, and it is why the exclusion lives in a parsed status letter rather than a
--diff-filterflag: rename detection is configurable, so a flag combination that excludes renames on one machine reports them as delete-plus-add on another, and the guard would fire on everygit mvfor half the team.The push that opened this PR is itself the hook's end-to-end test — all three guards ran through the restructured payload fan-out and passed.
Unit arms cover: the rename/copy/add/modify status matrix, out-of-scope paths (source, e2e, non-spec), malformed input degrading to no findings rather than a throw, the five-suite incident population, marker matching incl. case-insensitivity and near-miss spellings that must NOT satisfy it, all four
pendingRangesbranches, and an assertion that the failure text does not tell the author to restore the file.Directly touched surfaces:
buildScripts/util/check-spec-retirement.mjs— new, spec above..husky/pre-push— exercised by this PR's own push..github/workflows/spec-retirement-lint.yml— new;lint-guard-ci-parityran green on it in the commit hook.Post-Merge Validation
One residual, owned elsewhere: AC-3's enforcement half is not delivered here and is carried by #17171 — the repo-wide finding that nineteen lint workflows run on
devPRs and none is a required status context. That needs a repo-admin ruleset mutation, which no PR can perform. #17171 also carries the spec that reads the live ruleset, so the gap cannot silently reopen.Everything else is a local observable and each is armed above, including the two ACs the ticket framed as needing an incident replay — reproduced by content rather than by a commit id, which is the durable form and the one that survives the next rebase.
The first PR that legitimately retires a spec will exercise the account path in anger; that is the guard working, not a validation debt.
Commits
f40c97aa2a— the guard, its unit arms, the CI mirror, and the two.husky/pre-pushfixes.832063e0ee— scan-root registration + both workflow triggers, after CI caught the omission.67db298a4c— @neo-gpt's re-review blocker: merges are scanned, and the combined diff is what makes that safe. Red-proved on one commit —--no-mergesexits 0, the repair exits 1.e3f8442364— @neo-gpt's two exact-head blockers: the account grammar tightened from a substring test to a line-anchored one requiring content, and the enforcement claim narrowed to what the live ruleset actually supports. Guard behaviour is unchanged by the second; only the claims about it are.Evolution
The ticket prescribed
lint-stagedwiring and a SHA-pinned control, and neither survived contact — not because the ticket was careless but because both were pinned to things that move: a hook phase that cannot see commit messages, and commit ids on a branch that got rebased ninety minutes later. Grace's own conclusion on the second is the durable one and it shaped AC-4 here: an incident receipt should cite content-addressable evidence — file sets, path anchors, counts — rather than branch-local commit identity.Related: #17124 Refs #17126
Authored by Vega (Claude Opus 5, Claude Code). Session 5cd926fa-77e1-4309-8bbf-ca563ab07403.