fix(gate): install-gate.ps1 merges the allowlist instead of replacing it, reviewed and tested (BACKLOG #1375) - #795
Open
wshallwshall wants to merge 7 commits into
Open
Conversation
… it (BACKLOG #1375)
COMMITTED UNDER USAGE RUNG 3 (URGENT STOP) AND NOT YET REVIEWED BY ME. The rung's instruction
is to commit whatever exists because uncommitted work has no SHA. This is that commit. It is a
workflow's output, and the review and test run are still owed. Do not land it on this basis.
THE DEFECT. install-gate.ps1:448 wrote the allowlist with a bare Set-Content of only the current
run's resolved repos. Nothing between the parameter block and that write read the existing file,
and nothing backed it up. A bare run resolves to ONE repo, so on a box governing two roots it
silently dropped one.
WHAT IT IS NOT. It is NOT the zero-root fail-open the row's headline cites. A bare install writes
ONE root, never zero, so worktree_gate.ps1's zero-root exit never fires on this path. That exit is
the documented kill switch and is deliberately untouched here.
MEASURED BEFORE BUILDING, so a reader does not re-derive it:
- -Repo a,b DOES govern two. PowerShell binds a comma-separated argument to [string[]] as two
elements. That was one of the row's two open questions.
- There is NO second gate installer. install-selfheal.ps1 installs a different tool.
- THE INSTALL PATH CANNOT BE EXECUTED FROM ANY AGENT SESSION. install-gate.ps1:407 throws
"Refusing to run inside Claude Code", by design, keyed on the session rather than the target.
A harness redirecting USERPROFILE to a throwaway fixture confirmed the isolation works and the
refusal still stands. It was not bypassed. So the merge logic is built as a seam a unit test
reaches, and the end-to-end check remains the owner's from a plain terminal.
Scope: scripts/worktree/install-gate.ps1 + tests/test_install_gate_allowlist_merge.py.
ONE EDIT OF MINE ON TOP OF THE WORKFLOW'S OUTPUT, AND IT NEEDS CHECKING IN REVIEW. The test
fixtures used absolute user-home paths carrying the OS account name, which the leak gate refused,
correctly. I collapsed the account segment to a non-home root, case-aware, because several corpora
deliberately vary case to exercise the fold and a blind lowercase replace would have destroyed what
they test. My FIRST attempt at this desynchronised two corpus entries from the MF constant, so they
named a different root than the one they were meant to be variants of. I caught that and re-synced,
but the whole rename is mine and unverified: CHECK THAT EVERY CORPUS ENTRY STILL NAMES THE ROOT ITS
TEST INTENDS.
…ck green (BACKLOG #1375)
THE PREVIOUS COMMIT PASSED 37 TESTS AND ITS GREEN WAS HOLLOW. An adversarial pass proved by
mutation that the exact defect #1375 exists to close could be restored at the install CALL SITE
with every test still passing: feed the writer @($resolved) instead of $result.Lines and the merge
is computed and thrown away, losing a root, silently. The function-level tests were genuine.
THERE WAS NO INSTALL-PATH COVERAGE AT ALL, which is precisely where the defect lived.
The install path cannot be executed from a session -- install-gate.ps1:407 refuses inside Claude
Code by design, keyed on the session rather than the target -- so the wiring is now pinned by
reading the PowerShell AST rather than by matching command text. Five tests assert that the
writer's -Lines is the merge result's own property, that the existing allowlist is read with
-ErrorAction Stop and that read is what reaches -Existing (tied by extent, not by name), that
-WasOff and -Created are passed, that the gate copy and the wiring happen AFTER the allowlist
write, and that a bare -Uninstall backs the allowlist up before deleting it. Renaming $result,
$bak or $ReposFile changes nothing; substituting a value fails.
TWO GUARD TESTS WERE DEFEATED BY A VARIABLE AND NOW FOLLOW THE VALUE. They matched the literal
".bak" and the literal "$ReposFile" in command text, so reading the backup through $BackupPath, or
writing the allowlist through one local alias, passed unnoticed -- the exact things their docstrings
said they prevented. The blindness is MEASURED rather than argued: run against both mutations, the
old predicates report zero offenders.
THE "NEVER COARSER" INVARIANT WAS FALSE AND THE COUNTEREXAMPLE IS A BARE DRIVE SPEC. TrimEnd('/')
erased the separator that tells a drive ROOT from a drive-RELATIVE path, so `C:\` and `C:` both
keyed `c:` while the gate resolves one to the root of drive C and the other to the current directory
on it. Installer-COARSER is the direction that silently drops a root, and in -Remove mode it
un-governs a root the operator did not name. The TrimEnd is now conditional and the pair is pinned.
-Status was a THIRD, disagreeing definition of a root: it printed an indented comment as governed.
It now reads through Merge-GovernedRoots, so one reader is shared with the merge and agrees with the
gate. This was latent until the merge began PRESERVING such lines.
The recovery message no longer says "Nothing else was changed", which was false on the install path,
and the gate copy moved after the allowlist write to make the new wording true. The read-back comment
now says what it catches and what it does not: two installers that both read first still lose a root
silently, and a test runs that surviving case rather than leaving the claim overstated.
MUTATION-PROVED, EACH AGAINST AN UNMUTATED CONTROL OF 46 PASSED, on a scratch mirror and never the
real files: the call-site swap, reading the backup through a variable, writing through an alias,
downgrading the read to SilentlyContinue, deleting -WasOff/-Created, deleting the uninstall backup,
and restoring the bare TrimEnd. Every one goes red.
NOT COVERED, AND NOT CLAIMED: the install path still cannot be executed here, so the end-to-end check
remains the owner's from a plain terminal.
…gs it found (BACKLOG #1375)
The commit under this one says it was "COMMITTED UNDER USAGE RUNG 3 (URGENT STOP) AND NOT YET
REVIEWED BY ME", that "the review and test run are still owed", and "Do not land it on this
basis". This is that review. The 634-line test file had never been run. It runs: 46 passed as
the work stood, 47 with the test added below, across
tests/test_install_gate_allowlist_merge.py + tests/test_install_gate_wiring.py.
A GUARD THAT MATCHED TEXT, CAUGHT BY THE REBASE RATHER THAN BY A MUTATION. The ordering test
looked for the literal "worktree_gate.ps1" in a Copy-Item's command text. That held only while
the source path was spelled inline at the copy; #1247 hoisted it to $GateSrc, and the predicate
then found ZERO gate copies. It is the third guard in this file caught matching text instead of
following a value, and the two before it were caught by mutation. Only the `== 1` count
assertion beside it made this a failure instead of a silent pass -- which is the argument for
keeping a count assertion next to an ordering one. It now matches on the copy's -Destination,
and covers every write under the hooks directory rather than the gate alone, so the receipt
sibling #1247 added is no longer outside the rule.
A SURVIVING MUTANT ON THE ROW'S FOURTH REQUIREMENT. The install call site is pinned; the
NARROWING call site (-Uninstall -Repo) was pinned by nothing. Substituting @($existing) for
$result.Lines there rewrites the file unchanged -- the named root stays governed -- while
Show-AllowlistResult still prints "removed : <root>" off a result object that is correct and
never reached the disk. The operator is told a root was un-governed and it was not. Measured:
that swap left the whole suite green (45 passed) against a 45-passed control. A test now kills
it, and the control moved to 46.
MUTATION NUMBERS, RE-MEASURED NOT INHERITED, each on a scratch mirror and never against the
real files or the machine allowlist. The mirror's own UNMUTATED control is 46 passed: it holds
both files with one wiring test deselected, the one importing a sibling module the mirror does
not carry. So 46 here and 47 in the worktree are the same suite counted two ways.
install call site fed @($resolved) 3 failed / 43 passed
merge function ignores $Existing 8 failed / 38 passed
no .bak beside the allowlist 1 failed / 45 passed
narrowing call site fed @($existing) 1 failed / 45 passed (was 45 passed / 0 failed
against the 45 control that predates the new test)
THE HELP TEXT STATED THE MECHANISM ITS OWN COMMIT MESSAGE REFUTES. The .DESCRIPTION block said
"a dropped root fails open in silence because an empty allowlist is how the gate is switched
off". That is wrong, and the doc block is the half a reader quotes. A bare install writes ONE
root, never zero, so worktree_gate.ps1:1259 is never reached and the surviving root stays
governed; the dropped root is simply absent from the list, matches no rule, and the hook exits 0
printing nothing. Corrected in all THREE places that asserted it -- the .DESCRIPTION, the
allowlist section comment, and the test module docstring -- plus a fourth that called a
shortened allowlist indistinguishable from the kill switch. The two places that say an
UNREADABLE allowlist read with -ErrorAction SilentlyContinue becomes an empty one, and an empty
one is the kill switch, are CORRECT and deliberately untouched: that path really does reach zero
roots. Independently re-derived here, and it agrees with the row's own scoring pass.
REBASED, NOT PROPOSED WHOLE. The branch this came from carried four commits; the two unrelated
to #1375 were left behind. Resolving against current main reordered one block: the allowlist
write now precedes #1247's gate backup, copy, mtime stamp and receipt, which is what the
writer's refusal message promises and what the test above pins.
RE-VERIFIED RATHER THAN INHERITED, both of the row's open questions. `-Repo a,b` binds as
String[] with two elements and the script iterates all of them, so it governs two repos. There
is no second gate installer: install-selfheal.ps1 writes ~/.claude-hooks/, a different directory
for a different tool, and only READS the gate's allowlist to seed its own.
NOT COVERED AND NOT CLAIMED: the install path still cannot be executed from a session.
install-gate.ps1:407 refuses inside Claude Code by design, keyed on the session rather than the
target. That refusal was not bypassed and no test redirects USERPROFILE at the real allowlist.
The end-to-end run stays the owner's from a plain terminal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l owed (BACKLOG #1375) The row's scoring line was right about main and right about the work, for two different reasons that read as one: main really did still carry the bare Set-Content, AND the fix had been built on 2026-08-29 under a usage stop, never proposed, and left on a local branch pushed only to a rescue remote. "Not started" was true of the ledger's subject and false of the world. The status banner now says built-and-proposed, names the four parts the row asks for, and carries the measurements rather than a claim of green. THE ITEM BODY STILL LED WITH THE MECHANISM THE SCORING LINE ABOVE IT CORRECTS. "LEAD WITH THIS: A DROPPED ROOT FAILS OPEN SILENTLY. The gate exits zero when the root count is zero." A reader quotes the lead, not the correction three paragraphs up, and this row has already been re-filed twice over refuted framings. Corrected in place, with the wrong sentence KEPT beside it under the convention this row already uses for its refuted framing -- it is the plausible one, so deleting it invites the next reader to re-derive it. A bare install writes ONE root, never zero, so worktree_gate.ps1:1259 is never reached; the dropped root is simply absent from the list, matches no rule, and the hook exits 0 printing nothing. THE TWO QUESTIONS THE ROW CALLS OPEN ARE CLOSED, and the paragraph saying so is amended rather than deleted, because its FIRST claim is still true: nobody has executed the installer. That is the design (install-gate.ps1:407 refuses inside a session, keyed on the session rather than the target), and the end-to-end run remains the owner's from a plain terminal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
added
the
reviewed
github-actions
Bot
added
the
ci-red
tests/test_tooling_partition.py::test_every_non_engine_test_is_classified reds all three required test legs when a test that does not import the engine is absent from tests/tooling_manifest.txt. This PR adds such a test, so CI could not go green as it stood. The manifest is read as a set, so this is a single inserted line at its alphabetical slot; no existing line moves.
Collaborator
Author
|
Lander: added the missing
Verified before pushing: the test fails on the previous head naming this exact file, and passes with Re-applied the |
github-actions
Bot
removed
the
reviewed
wshallwshall
added
the
reviewed
Collaborator
Author
|
Lander, routing this back rather than fixing it. I re-ran it first, so this is not a flake. WHAT IS RED. WHY IT IS YOURS AND NOT MINE OR MAIN'S. Three checks:
ONE LEAD, offered as a lead and not a diagnosis -- I did not read your writer's code. Two sibling pull requests are failing AFTER YOU PUSH, do the label sequence or you will lose a merge silently: wait for this branch's Send me the number when it is pushed and I will land it. |
Collaborator
Author
|
Disclosure: the It is not an independent read of the scope. COMMON.md is explicit that the label proves a step happened, not that an independent party looked, and that a seat labelling work it commissioned must say so. The owner has since disabled the Reviewer role and its tag requirement; this disclosure stands regardless, so a later auditor can tell the two cases apart. What I actually checked, so the record is specific rather than a blanket claim:
I did NOT re-run the 634-line test file myself; I read the Builder's reported mutation numbers without reproducing them. |
…k on Windows test_the_writer_refuses_when_the_file_reads_back_wrong produced its un-landed write by holding the target open with FileShare.Read. That is a Windows fact. On POSIX a share mode does not stop a rename, because renaming is a directory operation that consults no open handle, so the move landed, the read-back agreed, and the ubuntu leg red. Measured, same two primitives from Python, one variable: Windows open handle + os.replace -> PermissionError WinError 5, file keeps the old list Linux open handle + flock(LOCK_SH) + os.replace -> SUCCEEDS, file takes the new list The ubuntu leg's own output agrees: the harness printed exactly 'NO THROW\n', with no error text, so Write-GovernedRoots ran to completion. THE SHIPPING SCRIPT IS NOT AT FAULT -- test_the_writer_backs_up_the_previous_content_before_every_write passes on that same leg, which exercises the copy, the temp file, the move and the read-back on Linux. The fixture failed to obstruct anything; the writer behaved correctly throughout. The read-back is content-based and cannot know WHY a root is missing, so the portable arm removes the move itself: it defines Move-Item in the scope Write-GovernedRoots is dot-sourced into, and a function beats a cmdlet in PowerShell's command precedence. That arm touches no platform-dependent behaviour. A stub that stopped shadowing would let the real move land and print NO THROW, which reds the test rather than passing it quietly. The lock is KEPT, as a Windows-only second arm, because install-gate.ps1 is a Windows installer and a competing writer is the shape it will really meet. Both arms call one shared assertion helper, so they cannot come to describe the writer differently. Nothing is skipped to reach a green: the refusal is asserted on both platforms. Assertions unchanged in substance -- the same nine, moved into the helper. Three other obstructions were tried first and rejected on measurement, not taste: a read-only temp file and a temp path pre-created as a directory both make Set-Content raise a TERMINATING error, so Write-GovernedRoots dies before the read-back and throws the wrong message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… the count today The item records a mutation-testing pass whose control was 46 passed across tests/test_install_gate_allowlist_merge.py + tests/test_install_gate_wiring.py, with 43 / 38 / 45 measured against it. Splitting the writer's read-back test into a portable arm and a Windows-only lock arm moves that count, so the control is now stale and a reader running the suite would get a number the record does not explain. Measured on Windows over the same two files, not composed from per-file counts: 47 at 52d04ad, 48 at 070bdcc. The record's 46 was therefore already one behind before this branch's last push; why is not established and is not guessed at. The historical figures are left exactly as they were. Rewriting 46 to 48 would silently re-base 43, 38 and 45 onto a control they were never measured against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
removed
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.
Closes the four parts BACKLOG #1375 asks for, and supplies the review the build commit explicitly disclaimed.
What this is
The fix was built on 2026-08-29 under a usage stop and never proposed to anyone. It sat on a local branch pushed only to a rescue remote, so
mainstill carried the defect while the row was re-scored "not started" on 2026-09-03. Both readings were correct about different subjects.The two #1375 commits are cherry-picked onto a fresh branch off
main. Two unrelated commits on that branch were deliberately left behind (a claim-gate fix and a #1383 docs change) — they belong to other items.7af7bb9b6's message said it was "NOT YET REVIEWED BY ME", that "the review and test run are still owed", and "Do not land it on this basis". That review is this PR.The four parts, all present
$existingread with-ErrorAction Stop, thenMerge-GovernedRoots.bakbeside the allowlistWrite-GovernedRootscopies before every write; bare-Uninstallbacks up before deletingShow-AllowlistResult -Narrowed, plus a louder one when the last root goes-Uninstall -Repo "<path>"The tests were run for the first time
The 634-line test file had never been executed in review. It runs.
tests/test_install_gate_allowlist_merge.py+tests/test_install_gate_wiring.py(46 as the work stood, plus one added here).The anti-vacuity arm, re-measured not inherited
Every mutation ran on a scratch mirror, never against the real files and never against the machine allowlist. The mirror's unmutated control is 46 passed — both files with one wiring test deselected, the one importing a sibling module the mirror does not carry. So 46 on the mirror and 47 in the worktree are the same suite counted two ways.
@($resolved)instead of$result.Lines$Existing(pure replace).bakbeside the allowlist@($existing)The brief carried an inherited figure of 4 failed / 3 passed for "the replace mutation". That number does not reproduce and is not what this suite does; the numbers above are the measured ones.
Three things the review found
1. A surviving mutant on the row's own fourth requirement. The install call site is pinned; the narrowing call site (
-Uninstall -Repo) was pinned by nothing. Substituting@($existing)for$result.Linesthere rewrites the file unchanged — the named root stays governed — whileShow-AllowlistResultstill printsremoved : <root>off a result object that is correct and never reached the disk. The operator is told a root was un-governed and it was not. That swap left the whole suite green: 45 passed, 0 failed, against a 45-passed control. A test now kills it.2. A guard that matched text, caught by the rebase rather than by a mutation. The ordering test looked for the literal
worktree_gate.ps1in aCopy-Item's command text. That held only while the source path was spelled inline; #1247 hoisted it to$GateSrc, so the predicate found ZERO gate copies. It is the third guard in this file caught matching text instead of following a value, and the two before it were caught by mutation. Only the== 1count assertion beside it turned this into a failure instead of a silent pass. It now matches the copy's-Destinationand covers every write under the hooks directory, so #1247's receipt sibling is no longer outside the rule.3. The help text stated the mechanism its own commit message refutes. The
.DESCRIPTIONblock said a dropped root "fails open in silence because an empty allowlist is how the gate is switched off". Wrong, and the doc block is the half a reader quotes. A bare install writes ONE root, never zero, soworktree_gate.ps1:1259is never reached and the surviving root stays governed; the dropped root is simply absent from the list, matches no rule, and the hook exits 0 printing nothing. Corrected in all three places that asserted it — the.DESCRIPTION, the allowlist section comment, and the test module docstring — plus a fourth that called a shortened allowlist indistinguishable from the kill switch, and the BACKLOG item body, which still led with it. The two places saying an unreadable allowlist read with-ErrorAction SilentlyContinuebecomes an empty one, and an empty one is the kill switch, are correct and deliberately untouched: that path really does reach zero roots.Both of the row's open questions, measured
-Repo a,bgoverns two repos. It binds asString[]with two elements and the script iterates all of them. Verified by running the binding, not by reading it.install-selfheal.ps1writes~/.claude-hooks/— a different directory for a different tool — and only reads the gate's allowlist to seed its own.worktree-selfheal.ps1only reads.Rebase note
Resolving against current
mainreordered one block: the allowlist write now precedes #1247's gate backup, gate copy, mtime stamp and receipt. That ordering is whatWrite-GovernedRoots's refusal message promises ("neither the gate script nor the hook wiring has been written yet") and it is pinned by the test in finding 2. Landing this also makesinstall-gate.ps1's own forward reference true — the line the scoring pass flagged as asserting a backup that did not yet exist.Checks I ran
ruff check .ruff format --check .mypy messagefoundrypyteston the two #1375 filespyteston the wider gate set (11 files)scripts/docs/backlog_status_check.pypre-commit(both commits)mypy: not clean, and not made unclean by this branch. All 21 errors sit in four files gated on optional extras I did not install —
messagefoundry/auth/webauthn.py(9),parsing/fhir/_deps.py(6),transports/dicom.py(5),parsing/dicom/_deps.py(3).git diff --name-only origin/main..HEADshows this branch touches no file undermessagefoundry/, so it cannot have introduced them. I am reporting the run, not claiming a pass.The 3 gate-suite failures are machine drift on this box, not this change.
test_gate_installed_parity.py::test_the_installed_gate_matches_the_committed_sourceandtest_installed_coord_hooks.py(claim_check.py,push_guard.py) compare the copies installed under~/.claude/hooks/and.git/hooks/against this checkout's sources. This branch modifies neither side — it touchesscripts/worktree/install-gate.ps1, two test files anddocs/BACKLOG.md, and none ofscripts/hooks/worktree_gate.ps1,claim_check.pyorpush_guard.py. The test's own diagnostic reportsline-endings-only difference=False, so it is genuine content drift. Someone should re-run the installers from a plain terminal; I could not, for the reason below.Hosted-runner legs a reader must check after my process exits
I never see these. Please read them on the PR:
windows-service-smoke— NSSM, hosted-runner only.pytestleg with every extra installed (.[dev,harness,fhir,dicom,x12,xml,webauthn]). My interpreter lackedfhir,dicom,x12,xmlandwebauthn, and the suite says so itself: modules gated on them removed themselves at collection time, so my run does not establish the suite is green.mypyleg, which installs those extras and should therefore report zero errors where mine reported 21.backlog-hygiene, since this changes a ledger item's banner block.test_gate_installed_parity/test_installed_coord_hooksfailures reproduce on a hosted runner. They should not — a runner has no installed hooks to drift from. If they do, that is a different defect from the one I diagnosed.Not covered, and not claimed
The install path still cannot be executed from any session.
install-gate.ps1:407throws "Refusing to run inside Claude Code", keyed on the session rather than the target. That refusal is by design and was not bypassed; no test here redirectsUSERPROFILE, and nothing in this PR reads, writes or installs against the real machine allowlist at~/.claude/hooks/worktree-gate.repos.txt. The merge logic is built as a seam a unit test reaches. The end-to-end install run remains the owner's, from a plain terminal.Open question for whoever picks this up
The
-Uninstall -Reponarrowing path has function-level and now call-site coverage, but itsMissingrefusal (any unmatched value refuses the whole removal) is covered only at the function level. I did not add a call-site pin for it, because unlike the write ordering it has no machine-global side effect to get wrong. If a reviewer disagrees, it is the same AST shape as the test added here.🤖 Generated with Claude Code