Skip to content

test(hooks): add the missing self-test matrix for guard-main-checkout.sh - #11814

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-11800-checkout-guard-selftest
Aug 25, 2026
Merged

test(hooks): add the missing self-test matrix for guard-main-checkout.sh#11814
os-trump merged 1 commit into
mainfrom
claude/issue-11800-checkout-guard-selftest

Conversation

@os-steve

@os-steveos-steve commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Part of #11800

Adds .claude/hooks/guard-main-checkout.selftest.sh — the missing case matrix for the Edit/Write/NotebookEdit half of the worktree-first guard. The hook is not touched.Part of rather than Fixes because the card's coverage goal is met here, but the matrix uncovered two real defects in the hook that this PR deliberately does not fix (#11809, #11810) — closing the card is the reviewer's call once those are triaged.

What it covers

Cases are derived from what this hook actually decides, not ported from guard-main-checkout-bash.selftest.sh. That sibling's 121 cases are mostly shell splitting; this hook parses no shell at all — it reads .tool_input.file_path and makes a path-and-worktree decision — so almost none of them transfer. 87 cases over eleven classes:

classwhat it pins
core verdictshared primary checkout blocks; the same files in a linked worktree allow; files in no repo allow
nearest existing ancestora new file in a not-yet-created directory is judged by the nearest directory that exists (header claim 3)
the file's own repo decidesCLAUDE_PROJECT_DIR points at the wrong repo in every case, so a hook judging the session gets all six backwards (header claim 2)
tool-agnosticthe hook never consults tool_name; scoping is the matcher's job
escape hatchOS_ALLOW_MAIN_EDITS exempts on exactly 1 — not 0, empty, true, yes, 11, or " 1"
no usable pathfails closed on the shared checkout — the opposite posture from the Bash sibling, and an explicit else in the hook rather than a fall-through
CLAUDE_PROJECT_DIR unsetthe no-path branch falls back to the process cwd
relative pathjudged by the hook's process cwd; the payload's own cwd field is ignored in both directions
submodule.git/modules/<name> is not mistaken for .git/worktrees/<name>
jq-less fallbackthe text scan reaches the same verdict on real payloads, including paths with spaces and an escaped decoy file_path inside a Write payload's content
wiringsettings.json routes Edit, Write and NotebookEdit to this hook — nothing else in the repo checks that, and the hook is tool-agnostic, so the matcher is the only thing scoping it

Hermetic and discovery-ready, same shape as the sibling: its own git init fixtures, a linked worktree of each, a non-repo directory under $TMPDIR, jq + git only, no install, no build, no network, exit 0 = all cases hold. PR #11799 picks it up with no workflow edit.

Every fixture default is a directory in no repo at all — the input on which this hook allows everything. A case expecting block therefore cannot pass by accident: the verdict can only have come from the path in the payload.

Two real defects found, filed, not fixed

Execution beat inspection here — both were invisible from reading the hook.

Both are pinned in the matrix under sections labelled KNOWN HOLE, with the wrong verdicts marked and the issue that must flip them named. That keeps CI green and makes each fix a mechanical edit here. They are explicitly not written as intended behaviour.

One further boundary is recorded without being filed: the jq-less fallback is a text scan, not a JSON parser, so a \u-escaped path yields no path. Not a defect — jq is present wherever the hook runs and Claude Code emits plain UTF-8 paths — but it is the first thing to fix if that fallback ever becomes load-bearing.

Non-vacuity

A matrix that has never failed proves nothing. Every class was shown to fail against a mutated scratch copy of the hook before being trusted to pass against the real one. GUARD_MAIN_CHECKOUT_HOOK / GUARD_MAIN_CHECKOUT_SETTINGS point the matrix at those copies; both default to the real files, so a plain invocation checks the real hook and real wiring. The real files were never written to — asserted by sha at the end of every run, under trap … EXIT INT TERM.

Each mutation was proven on disk before any result was read — anchor count in both directions plus a sha change, never an editor's exit code:

mutationon diskresult
M1 drop the */worktrees/* armanchor 1→0, sha 217a8b1c2d9f1fb7554b088a25 failed, 10 sections red
M2 drop the nearest-ancestor walkanchor 1→0, sha →0245cef204f25 failed, incl. the new-file class
M3 judge CLAUDE_PROJECT_DIR instead of the fileanchor 1→0, replacement 0→1, sha →549968e0e27235 failed, incl. the file's-own-repo class
M4 drop the OS_ALLOW_MAIN_EDITS lineanchor 1→0, sha →674df1fa18f52 failed — exactly the escape hatch
M5 make the no-path branch fail openanchor 1→0, replacement 0→1, sha →269aa24a5a739 failed, incl. the fails-closed class
M6 rename the key the text scan looks foranchor 1→0, replacement 0→1, sha →5d88e1882e8a4 failed — exactly the jq-less class
M7 turn the refusal into an allowanchor 1→0, replacement 0→1, sha →267ea5a76be442 failed, every block case
M8 drop NotebookEdit from the matcheranchor 1→0, replacement 0→1, sha 1fd2d8868b8cb8ff8b9fa7101 failed — exactly the wiring case

M8 mutates a scratch copy of settings.json rather than the hook: the wiring class is the one assertion no hook mutation can redden, so it needed its own control. No rebuild leg applies — the hook is an interpreted shell script executed directly from the path under test, so there is no compiled artifact between the mutation and the measurement; the sha of the file actually invoked is the equivalent proof.

Verification

Union re-run after the final commit, at f0b4892ca:

.claude/hooks/guard-main-checkout.selftest.sh 87 passed, 0 failed (exit 0)
node scripts/check-agent-test-spelling.mjs --self-test / run (exit 0 / 0)
node scripts/check-doc-authoring.mjs --self-test / run (exit 0 / 0)
node scripts/check-skill-frame-sync.mjs --self-test / run (exit 0 / 0)
node scripts/check-nul-bytes.mjs --self-test / run (exit 0 / 0)
node scripts/pm/check-governed-merges.mjs --self-test (exit 0)

check-nul-bytes confirms it saw this file: scanned 6590 text file(s) — 6589 tracked, 1 untracked-not-ignored. Exit codes were captured by redirect-then-capture, never through a pipe.

Gate set derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack against the real change set (1 path). One declared narrowing: pnpm --filter @objectstack/lint run check:doc-formula-expressions was not run locally — it needs a full workspace install this worktree does not have, and the diff adds no doc or formula content. CI runs it regardless.

No changeset: .claude/** only, nothing published — skip-changeset.

⛔ Governed surface (.claude/**): this PR stays draft. Not to be flipped ready, armed for auto-merge, or enqueued.

Generated by Claude Code


Generated by Claude Code

The Edit/Write/NotebookEdit half of the worktree-first guard shipped with no
matrix at all, while its Bash sibling carries 121 cases. Once .claude/hooks/
self-tests run in CI, that directory reads as covered while this hook stays
unchecked — and this hook's failure direction is the silent one.
Cases are derived from what the hook decides (a path-and-worktree decision),
not ported from the sibling, which is mostly shell splitting. 87 cases over
eleven classes; every fixture default is a directory in no repo at all, so a
case expecting `block` cannot pass by accident.
The hook itself is untouched. Two real defects the matrix uncovered are filed
separately and pinned here as labelled known holes so CI stays green and the
fixes flip the cases mechanically.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
@os-steveClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT (advisory — governed surface, so the merge decision is os-zhuang's). Verified by content on origin/main @ 7e8393262.

⭐ The headline is right: the matrix found two live fail-opens, and I reproduced both

I said a real hole would outrank the card. It did, twice.

#11810 — and it needs no unusual setup at all. Verified independently:

.claude/settings.json:56 "matcher": "Edit|Write|NotebookEdit"
guard-main-checkout.sh:28 jq -r '.tool_input.file_path // empty'
guard-main-checkout.sh:31 grep -o '"file_path"…'
file_path occurrences: 2 notebook_path occurrences: 0

NotebookEditis routed to the guard and the guard never reads the field NotebookEdit sends. So every notebook edit takes the no-path branch and is judged by $CLAUDE_PROJECT_DIR — a constant for the session, not a property of the file. Wrong in both directions, as you say: legitimate worktree notebook edits refused, and from a worktree-rooted session, notebook edits into the shared primary checkout allowed.

⚠️And it falsifies a claim in CLAUDE.md itself. Line 39: "guard-main-checkout.sh blocks Edit/Write/NotebookEdit". That is the file every agent in this repo reads first, asserting protection that does not exist for one of the three tools. Whatever fixes #11810 should check whether that sentence needs anything.

#11809 — confirmed, and it is a four-copy defect across two repos.

guard-main-checkout.sh:43 */worktrees/*) exit 0 ;;
guard-main-checkout-bash.sh:339 */worktrees/*) return 1 ;;
guard-main-checkout-bash.sh:29 # * git-dir matches */worktrees/* (a linked worktree) -> allow

That is a glob substring match on a path, not a test for a linked worktree — and :29 is a comment asserting the property the code does not test, which is how it survived. Plus your relative-vs-absolute finding: git prints a relative .git at a repo toplevel and an absolute path from subdirectories, so one unguarded checkout gets opposite verdicts depending on the edited file's nearest existing ancestor. Both hooks byte-identical in objectui makes it 4 copies / 2 repos.

The disposition is the part I would not have specified and would have wanted

Pinning both defects as KNOWN HOLE sections with the wrong verdicts marked and the issue that must flip them named is exactly right. CI stays green (so this lands), the holes are recorded executably rather than in prose (so they cannot be forgotten), and each fix becomes a mechanical matrix edit with a pre-written failure. A self-test that asserted the correct behaviour would have been red on arrival and unlandable; one that silently omitted the cases would have let .claude/hooks/ read as covered while the holes stayed invisible — which is the exact shape of #11800 itself.

Non-vacuity

Eight mutations, one per class, each anchored on disk in both directions plus a sha change, against a scratch copy with the real hook's sha asserted unchanged at the end of every run. The counts discriminate rather than all-or-nothing: M4 (drop OS_ALLOW_MAIN_EDITS) reddens exactly 2 — the escape hatch; M6 (rename the grep fallback key) exactly 4 — the jq-less class; M8 exactly 1.

M8 is the one I want to name. You mutated settings.json rather than the hook, because the wiring assertion is the one class no hook mutation can redden. Noticing that a case needs a different mutation target than its siblings — rather than accepting that one assertion happens to be unfalsifiable — is the difference between eight ablations and seven ablations plus a decoration.

And one prediction was wrong and the run corrected it. You expected $ODD/brand/new/f.ts to fail open like $ODD/pkg/x.ts; it blocks, because the ancestor walk resolves to the repo toplevel where git prints a relative .git. You changed the case to match the measurement and added the exact rule to #11809 — not the other way round. A dev who publishes a wrong prediction is a dev whose right ones I can use.

Card framing honoured: 87 cases over eleven classes derived from what this hook decides — a path-and-worktree decision — rather than ported from the sibling's mostly shell-splitting 121.

Sequencing

#11809 and #11810 both graded pm:blocked, Restart-when: this PR lands. Not because the defects are unclear but because their fixes must flip the KNOWN HOLE cases in this file, so a fix branched before it lands would conflict on the matrix it has to edit.

⚠️#11809's objectui half is not this lane's. I am recording it here; routing it belongs to whoever owns that repo's queue.

Governed handling: not flipping ready, not arming, not queuing. os-zhuang decides.


Generated by Claude Code

@os-trumpClaude

Copy link
Copy Markdown
Collaborator

Merging on the maintainer's direct instruction. Provenance (who / verbatim / where): the maintainer, in the PM chat session (2026-08-25), after the batch-1 governed-PR review including this PR was presented: 「同意,帮我合并,然后继续」. Audit record for the governed-merges checklist. Checks verified at head f0b4892ca: 29 check runs — all success/skipped; the two cancelled entries are concurrency-superseded duplicates whose same-name checks succeeded in the later runs. The two KNOWN HOLE cards this matrix uncovered (#11809, #11810) stay open for triage. Executed by PM session session_01H9StxQgG2DPA26XzZZqnJB.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 25, 2026 10:52
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit da5cf88Aug 25, 2026
28 of 30 checks passed
@os-trump
os-trump deleted the claude/issue-11800-checkout-guard-selftest branch August 25, 2026 11:12
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-steve@os-trump