fix(hooks): the claim gate passed any commit whenever its own git read failed (BACKLOG #1383) - #686
Merged
Merged
Conversation
Collaborator
Author
Dequeued deliberately — it conflicts with #684, which is ahead of itNot a defect in this PR. It reported Both touch A PR can be Order: #684 lands, then this needs a rebase onto the new main, then it re-queues. The rebase is mechanical (ledger tail plus a SECURITY.md overlap) and the branch carries no allocated row of its own, so it is mine to take rather than its author's. Nothing about the fix itself has changed. The fail-open is still real and still confirmed at source — |
Collaborator
Author
Needs resolving by its author — the ledger gate refuses me, and I am not using the known bypass#684 landed, so this now conflicts with main. Its only conflicting path is I resolved it (main's five rows first, then #1383 appended; 394 items, no duplicates, zero conflict markers) and the commit was refused: That is correct and I reverted. An ordinary commit runs the gate, and the ownership check applies. #1383 belongs to your worktree. Why I did not reach for the sequencer insteadBecause it would have worked, and that is the problem. BACKLOG #1395 — filed and independently reproduced today — establishes that Its sibling finding closes the other door: A gate that has just refused me is not one to reach around with a known bypass, however available it is. The refusal is telling me where the commit has to be made. What is neededBring this onto current main from the worktree that allocated #1383, keeping both sides of the ledger tail — main's rows first, yours appended — and push. Then it lands. The fix itself is unchanged and still verified: |
Collaborator
Author
Conflicting again after the latest landings — and the resolve is yours, not mine
The ledger gate refuses me on a merge carrying a row I did not allocate — I tested that on this exact class earlier and got I could have replayed instead and it would have worked — BACKLOG #1395 measures 11 gate result lines on an ordinary commit and zero on a sequencer commit, and CI never checks ownership either ( What is neededFrom the worktree that allocated the row: bring the branch onto current main, keep both sides of the tail — main's rows first, yours appended — and push. Then it lands. Controls worth running: The structural note, because this is the fourth time todayOne file serialises every row-carrying branch. Three branches have now needed a resolve for this reason alone, none of them for a defect in the work. |
added 2 commits
August 30, 2026 07:17
…het the rest (BACKLOG #1383) THE MECHANISM, READ FROM SOURCE RATHER THAN RELAYED. _RELOCATED_TO_SECURITY maps 15 legacy [section] key spellings to their [security] replacements. _reject_relocated_keys RAISES ValueError and is called at settings.py:4588, BEFORE _desugar_security at :4592, so a legacy spelling never reaches the desugarer. Refusing is the whole behaviour, not a fallback -- worth checking rather than assuming, because ADR 0118 is described as desugaring. THE SEVERITY IS LOWER THAN THE ORIGINATING HANDOFF ASSERTED, AND THE ORIGINATOR WITHDREW THE CLAIM. It said at least one line INSTRUCTS an operator, causing a load refusal on first start. Measured, with the key list imported from the table so it cannot drift: 0 refused keys inside any fenced code block, 0 imperative instructions, 3 TOML-shaped prose citations, 19 descriptive mentions, against a positive control of 4 new-spelling hits. The line meant was :811, read with :810 it is declarative. SO THE RESIDUAL RISK IS NARROWER AND STILL REAL: nobody hits a load refusal, because nothing tells them to set the key. But a reader told "the documented org opt-out is X" may well write X, and X is refused. THREE FALSE-POSITIVE CLASSES, RECORDED BECAUSE MY OWN FIRST PASS RETURNED 40 -- looser than either earlier estimate. create_app(serve_ui=True) is a PYTHON KEYWORD ARGUMENT, not a TOML key; [approvals].enabled and [auth].oidc_enabled are DIFFERENT KEYS; and lines that CORRECTLY DOCUMENT THE REFUSAL are not defects. SECURITY.md:186 says the [diagnostics].audit_all_authz spelling "is refused at load" -- the one place the document is already right, and the originating scan counted it as a defect. The discriminator that separates config from API surface is one character: TOML booleans are lowercase. THE CLAIMS WERE MEASURED, NOT TRANSCRIBED. An approval settles whether a change may be made, never whether the words are true. require_sign_in defaults True and require_mfa_scope defaults "every_local_account" in source, so the three sentences stay true after the respelling. THE GATE IS A RATCHET, DELIBERATELY. 26 other documents carry 56 more citations and the agreed scope was docs/SECURITY.md only, so a repo-wide check would land red and get disabled while a one-file check would let the rest spread. Every affected file is pinned at its measured count, a file absent from the table must be at zero, and new files or new citations fail immediately. IT SELF-PRUNES, WHICH IS WHAT KEEPS A BASELINE FROM ROTTING INTO A SUPPRESSION LIST: fixing a file below its number fails until the number is lowered, and fixing it entirely fails until the row is deleted. BOTH DIRECTIONS PROVED BY MAKING IT FAIL ON PURPOSE -- a new doc carrying a citation fails, and removing a citation from a baselined doc fails the self-pruning test. Controls in the suite itself: the table must be populated (an empty import would make every check pass vacuously), a planted bad line must be caught, and the two negative controls -- the documented-refusal line and the Python kwarg -- must NOT be caught. Scope: docs/SECURITY.md + docs/BACKLOG.md + tests/test_docs_cite_no_refused_config_keys.py. Clearing the other 26 documents is separate and is not filed here.
…failed
THE GATE WAS DISARMED BY THE ONE CONDITION IT CANNOT DETECT, AND IT PRINTED NOTHING. _git returned
subprocess.stdout and never looked at returncode, so a failed read returned "" -- indistinguishable
from a genuinely empty diff. That flowed to _staged_paths() -> [] -> _touches_code([]), which is
False BY DESIGN so a message-only --amend is never blocked, and main() took its docs-only exit and
returned 0. A commit citing an UNCLAIMED item passed, silently.
RED-FIRST, BOTH VERSIONS RUN SIDE BY SIDE FROM A DIRECTORY GIT CANNOT ANSWER IN:
OLD (pre-fix) exit 0 PASSES the commit -- FAIL OPEN
NEW (fixed) exit 1 refuses
AND THE EMPTINESS IS THE FAILURE, NOT THE NORM, measured with a control on the same box: outside a
repository the staged-diff read exits 129 with empty stdout; inside the repository it exits 0. The
old code could not tell those apart, which is the whole defect.
FAIL CLOSED, AND ONLY FROM THE POINT WHERE A WRONG ANSWER BECOMES A SILENT PASS. Every exit above
that line is reached WITHOUT calling git, so a commit that does not cite a BACKLOG number is
untouched. Only a commit that DOES cite one, on a box where git cannot answer, is refused --
refusing costs a re-run, passing costs the duplicate build this gate exists to stop.
WHY FAIL-CLOSED IS SAFE HERE SPECIFICALLY, because it is not always: _git has THREE callers and all
of them are in this file. The sibling case that argued for care -- a board tool with sixty callers
written against a function that could not fail -- would trade a silent wrong number for a dead
board. A commit hook has no such surface.
LOW ODDS, AND THAT IS NOT A REASON TO SHRUG. git normally invokes this hook from inside a
repository. It fails on index.lock contention, a corrupt index, git missing from the hook's PATH, or
GIT_DIR oddities in a worktree -- rare states that arrive exactly when several sessions are
committing at once, which is when a duplicate-work gate matters most.
The refusal names the cause, the item, and the usual fixes, and says the check has not failed the
operator -- a gate that refuses without explaining is the next defect.
TWO TESTS, THE SECOND GUARDING THE FIRST: the regression drives the real hook from a directory git
cannot answer in and requires exit 1; its control requires that the SAME command inside a real repo
still refuses with the CLAIM message and NOT the read-failure one. Without the control, deleting the
staged-diff read entirely would satisfy the regression while breaking every real check, because a
refusal for the wrong reason reads identically to a refusal for the right one.
Found by playbooks-manager with a firing control; widened by the dispatcher, which measured the same
shape in the vault copy. THE VAULT HALF IS NOT FIXED HERE -- it is a separate repository.
Scope: scripts/hooks/claim_check.py + tests/test_claim_check.py.
wshallwshall
force-pushed
the
b1-1383-claimgate-landable
branch
from
August 30, 2026 12:18
52690a6 to
c0ed201
Compare
added 2 commits
August 30, 2026 07:32
# Conflicts: # docs/BACKLOG.md
Sixteen items landed on main since this branch was cut, several of them tail-appending docs/BACKLOG.md, so this conflicted append-vs-append at the file tail. Only that one path conflicted; everything else merged automatically. Resolved keeping BOTH sides in append order, this file's convention rather than numeric order, verified by reading main's existing tail rather than assumed. Checked with a set difference rather than a count, because a merge can fold two items' blocks into one and leave a correct-looking file short by one item while the totals still agree: main 412 branch 397 merged 413 expected 413 LOST [] ADDED [] only-on-branch preserved [1383] positive control 1401 present on main; negative control 999999 absent backlog_status_check: OK, 649 items, exit 0 Committed from the worktree the #1383 claim records, which satisfies the ledger gate on its path key. The claim's recorded branch key names b1-1375-install-gate-merge, which does not exist on origin at all, so the PR 703 branch fallback cannot help here -- the path key is the only route, and it is why this had to be committed from this worktree.
github-actions
Bot
added
the
ci-red
added 3 commits
September 1, 2026 16:30
…not stale test_the_baseline_is_exact_and_self_pruning failed on every runner -- ubuntu, windows-2022 and windows-2025 -- and the baseline was right. Two of its 27 rows name docs/testing/master-test-plan documents that are UNTRACKED: `git ls-files` does not report them and no checkout anywhere carries them. They were measured where they do exist, so their counts are correct; CI simply cannot open the files. Treating "not in this checkout" as "stale row" made the test unpassable while nothing about the corpus had drifted. Warn and skip, which is the call two sibling tests already make for their own withheld documents: test_threat_model_doc_drift.py raises ThreatModelDocUnenforced for docs/security/** and test_crit2_inline_doc_drift.py raises CoveragePlanUnenforced for docs/testing/**. Both say the assertion is inert in this run rather than failing. This adds BaselineRowUnenforced and names the two paths in _WITHHELD_FROM_PUBLIC_CHECKOUTS. THE EXEMPTION IS TWO NAMED PATHS, NOT A RULE ABOUT MISSING FILES. "Any absent file is fine" would have deleted the self-pruning property the baseline exists to have. Any other missing row is still a hard failure, a present row is still enforced exactly, and a floor of len(_BASELINE) - len(_WITHHELD) stops a baseline decaying to all-absent and passing having read nothing -- the vacuous green test_the_relocated_table_is_populated already guards upstream. test_every_withheld_row_is_still_a_baseline_row keeps the exemption from outliving its rows. Mutated three ways, each red disjoint from the others: a non-withheld row goes missing -> "docs/NO-SUCH-DOC.md: listed but missing" a present row's count is wrong -> "docs/CONNECTIONS.md: baseline says 8, now 9 -- lower it" an exempt path leaves _BASELINE -> "exempted paths that are no longer baseline rows: [...]" 286 passed locally, 2 warnings, both naming the withheld rows. ruff format and ruff check clean.
…nd pass) Main advanced while this branch waited -- PRs 718 and 723 landed -- so the docs/BACKLOG.md tail collided again. Append against append, both sides kept in APPEND order; this file's tail is not sorted by item number. main=413 branch=413 merged=414 expected=414 items on main missing from the merge: [] items appearing from nowhere : [] folded in from main : [1414] preserved on branch : [1383] positive control 1401 present: True negative control 999999: False backlog_status_check: OK, 414 items, each declaring exactly one status COMMITTED FROM THE WORKTREE THE ALLOCATION RECORDS. ledger_check.owns() keys on alloc/backlog/1383.json, which names MessageFoundry-b1-1067-repo-governance; that path still exists, so the ordinary path key applies and no recovery was needed. The earlier commit on this branch went through from elsewhere only because it touched no ledger file -- the gate reads what a commit ADDS, so a code-only commit never reached this check.
…ich is its whole population BACKLOG #1383's new gate scans docs/**/*.md for config keys the loader refuses. It was reachable only from the full test legs, and ci.yml's changes job sets code=false when every changed path matches noncode -- a regex whose first alternative is \.md$. So a pull request that edits ONLY documentation, the exact shape that can introduce one of these citations, skipped the legs entirely. The docs-only lane runs DOC_GUARDS instead, and this module was not in it. The gate existed and could not fail on its own subject. tests/test_doc_guards_lane.py:83 records the identical hole for a different module: a citation guard "was absent from DOC_GUARDS while TWO citation siblings were present, so the gate existed". Same class, second instance, so the fix is membership rather than another one-off. Added to DOC_GUARDS, which the lane test parses out of ci.yml rather than re-typing, so there is no second copy to drift. The minimal docs-only install supports it: sibling members test_crit2_inline_doc_drift.py and test_security_doc_rate_limits.py already import messagefoundry, and this module imports _RELOCATED_TO_SECURITY from config.settings the same way. The lane is now 20 modules. Also corrected a stale measurement in freshly written prose, which is the defect this branch's own review caught elsewhere tonight: the comment above _BASELINE said "27 documents carry 58 of these citations" while the dict directly beneath it sums to 59 over 27 rows. The table is the only authority for that number and the line now says so. The count predates the Lander's edits to this file -- commit 4e0ffc6 touched neither the comment nor any baseline row -- but it is wrong either way. 290 passed locally across the lane test and the ratchet, 2 warnings, both naming withheld rows.
wshallwshall
added
the
reviewed
github-actions
Bot
removed
the
reviewed
…d pass) PR 726 landed while this branch waited, so the docs/BACKLOG.md tail collided again. Append against append, both sides kept in APPEND order; this file's tail is not sorted by item number. main=414 branch=414 merged=415 expected=415 items on main missing from the merge: [] items appearing from nowhere : [] folded in from main : [1412] preserved on branch : [1383] positive control 1401 present: True negative control 999999: False backlog_status_check: OK, 415 items, each declaring exactly one status -- run before this message was written. COMMITTED FROM THE WORKTREE THE ALLOCATION RECORDS, and that is why it works on a detached HEAD. ledger_check.owns() tries the alloc record's worktree path FIRST and only falls back to its branch; this checkout IS the path alloc/backlog/1383.json names, so the path key matches and the detached HEAD never has to be considered. PR 734 hit the opposite case in the same minute: its worktree is not the recorded path and it was detached, so neither key could match -- a detached HEAD reports the literal string "HEAD" and can never equal a recorded branch name.
wshallwshall
added
the
reviewed
wshallwshall
enabled auto-merge
September 2, 2026 00:11
github-actions
Bot
removed
the
reviewed
wshallwshall
added
the
reviewed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits. The first is a fail-open in a gate.
The defect, confirmed at the source
_gitreturned.stdoutwithout checkingreturncode. So a failed git read was indistinguishable from an empty diff, and an empty diff takes the docs-only exit — meaning the claim gate passed any commit whenever its own read broke.Zero on main is the whole finding: the function had no way to tell success from failure.
This is the class the fleet hit three separate times today — a control that cannot observe its own failure. ADR 0158 Class 2, and here it is in a gate rather than in a report.
Lander checks
One thing this PR's own landing should not obscure
The branch was rescued from a
docs/BACKLOG.mdcollision by a rebase at about 17:4xZ. Per BACKLOG #1395 — filed and measured today —gitnever invokespre-commitfor a sequencer commit, so that rebase re-ran none of the eleven hooks. Reproduced independently: 11 gate result lines on an ordinary commit, 0 on a cherry-pick that really created a commit.So CI is the only gate this branch has passed since the rebase. That is probably fine and was true before anyone knew, but it should be said rather than assumed — and which of the eleven have a CI twin is still unmeasured.
Neither the author nor the lander has reviewed the 347 tests on their merits. The lander verified the fail-open at the source, that the merge is clean, and what a merge would touch.