The three branch-review findings, fixed 2026-08-30

• Source: findings 1, 2 and 11 of branch-review-0828.md, the origin/dev..feat/admin-and-merchant-portals review.
• Operator ruling: fix all three today.
• Evidence: every claim below is a run of the REAL hook against REAL PreToolUse payloads, reproducible with harness/probe-findings.mjs.

What was actually wrong?

One defect stopped the guard from running, and two made it lie in opposite directions.

#	Where	What it did
1	.claude/hooks/guard-pm-lane.sh	Committed mode 100644 while all seven siblings are 100755, so on a fresh clone the guard she called ALWAYS ON never executed
2	guard-pm-lane.mjs brace test	Unanchored /\s|$/, true for every input, so a word-initial { discarded the operand behind it and the braced form of a refused command was allowed
11a	guard-pm-lane.mjs tokenizer	A trailing # comment was tokenized as operands and judged as paths, so an in-lane command was refused over its own comment
11b	guard-pm-lane.mjs splitArgs	git commit -m"msg" read the whole token as an operand, so the commit MESSAGE was judged as a pathspec and refused on a path that does not exist

The root cause is the same for all three: none of them had a test.

• 195 tests passed while the hook could not execute at all, because every case in the suite spawns bash HOOK, and bash given a script by path ignores its execute bit. The suite could not see the defect it was standing on.
• Each fix below ships with cases in both directions, and the two cry-wolf fixes ship with a control proving the mechanism they loosen still fires.

How was each one proved?

The harness drives the hook, not the module, and every group carries a control.

• The reviewer's first attempt passed a bare string to decide(), which takes a payload, and every case came back ALLOWED including the control that should have blocked. A harness that cannot reproduce its own control has tested nothing, so each group here pairs the defect with a case whose answer was already correct before any change.
• Fixtures are throwaway repos and a throwaway HOME, so no result can come from whatever happens to be dirty in the real tree.

One control failed for a reason that was not the fix, and it was worth the detour.

• The case "an attached message still refuses when a blocked path is STAGED" went red after the fix. The cause was test ORDER: section 4 of the suite mutates the shared stagedBlocked fixture by committing the file it had staged, so by the time the new case ran there was nothing staged to refuse.
• The fix was a fixture of its own, not a looser assertion. A control that passes because of what another test did is not a control.
• The probe never saw it, because the probe builds its fixtures fresh. That is the argument for having both: the suite catches what a run misses, and the run catches what a suite's shared state hides.

Before and after, one run each, 22 cases.

Group	Case	Before	After
brace	rm -rf tooling/foo (control)	BLOCKED	BLOCKED
brace	rm -rf {tooling,apps}/foo	ALLOWED	BLOCKED
brace	rm -rf docs/projects/p/foo (control)	ALLOWED	ALLOWED
brace	mkdir -p docs/projects/p/{a,b}	ALLOWED	ALLOWED
brace	mkdir -p {docs/projects/p/a,docs/projects/p/b}	ALLOWED	ALLOWED
brace	mkdir -p {docs/projects/p/a,tooling/x}	ALLOWED	BLOCKED
brace	touch docs/projects/{p/a.md,../../tooling/x.md}	ALLOWED	BLOCKED
brace	{ rm -rf tooling/foo; } (control, brace GROUP)	BLOCKED	BLOCKED
brace	{ cat docs/projects/p/README.md; } (control)	ALLOWED	ALLOWED
comment	mkdir -p docs/projects/newproj # scaffold the project	BLOCKED	ALLOWED
comment	mkdir -p tooling/newthing # scaffold the thing (control)	BLOCKED	BLOCKED
comment	sed -i 's/a/b/' tooling/scripts/thing.mjs # tidy (control)	BLOCKED	BLOCKED
comment	touch tooling/x#1.md (control, # inside a word)	BLOCKED	BLOCKED
comment	echo "# heading" > docs/projects/p/x.md (control, quoted)	ALLOWED	ALLOWED
comment	echo hi # then newline rm -rf tooling/foo (control)	BLOCKED	BLOCKED
commit	git commit -m"docs(projects): the agenda"	BLOCKED	ALLOWED
commit	git commit -m "docs(projects): the agenda" (control)	ALLOWED	ALLOWED
commit	git commit -m'docs(projects): the agenda'	BLOCKED	ALLOWED
commit	git commit -m"chore: add a note"	BLOCKED	ALLOWED
commit	git commit -am "chore: sweep" (control, real -a)	BLOCKED	BLOCKED
commit	git commit -m"docs: x" with apps/ staged (control)	BLOCKED	BLOCKED
commit	git commit -m"a" tooling/scripts/thing.mjs (control)	BLOCKED	BLOCKED

• Before: 7 of 22 wrong, and every one of the 15 controls already right, which is what makes the 7 readable as defects rather than as a broken harness.
• After: 22 of 22 as expected.

What did the ratchet checks say?

DIRECTION: can the change hide a real violation?

Change	Answer
Anchoring the brace test	No. The word is now KEPT instead of discarded, and keeping a word can only add candidates
Expanding {a,b} before judging	No. One candidate becomes the several the shell would produce, and any one outside the lane refuses the whole command. A runaway expansion (over 64 arms) falls back to the literal, which inside the repo is default-deny
Dropping a # comment	Only the text bash itself never runs. The # is honoured only at a WORD START and only unquoted, which is bash's own rule, and the controls pin a mid-word #, a quoted #, and a real write sitting before the comment
Reading -m"msg" as a flag	This one needed the most care, and two controls exist because of it. The attached value must not swallow the NEXT word (git commit -m"a" tooling/x must still refuse tooling/x), and the message text must not be read as more short flags (-m"chore: add a note" must not be seen as -a and sweep the dirty tree). Both are cases in the suite

MEANING: would it fire on correct behaviour?

• Before these fixes, three shapes the PM lane exists to ALLOW were refused: scaffolding a project directory with a brace list, annotating a command with a comment, and committing with the message attached to -m. A gate that cries wolf gets switched off, which is why finding 11 sits beside a fail-open rather than below it.
• The fixes are the narrow ones. A brace list with one arm outside the lane still refuses, a comment cannot hide a write beside it, and a real -a still pulls in the dirty tree.

What is the file mode, in the index rather than on disk?

$ git ls-files -s .claude/hooks/
100755 ... block-pr-to-main.sh      100755 ... guard-git-branch.sh
100755 ... enforce-pr-draft.sh      100755 ... guard-inline-secrets.sh
100755 ... guard-backtick-body.sh   100755 ... guard-pm-lane.sh   (was 100644)
100755 ... guard-em-dash.sh         100755 ... guard-pr-ready.sh

• Set with git update-index --chmod=+x, so it lands in the INDEX. A local chmod alone leaves the committed mode untouched and the next clone unguarded.
• The test reads the mode out of git ls-files -s rather than off the filesystem, and loops over every hook in the directory, so the next hook added is covered without anyone remembering to come back.

Did a guard block this session?

The PM lane guard did not, and it could not have: this is a builder session, and it judges only a declared PM.

• CLAUDE_PM_SESSION is unset here, so guard-pm-lane.sh exits at its first line. That is the inversion from #975 working as designed.
• Worth stating rather than leaving implied: the 2026-08-27 session sent to fix this guard WAS blocked by it, which is the cleanest evidence that issue has. The inversion is why this one was not.

The em dash guard did, twice, and it is the same defect class this project was built to avoid.

• The blocked command was a heredoc writing THIS file plus a check searching the result for a literal em dash. The file contained none. The SEARCH EXPRESSION contained one, guard-em-dash.sh scans the raw command text, and the whole call was refused. The second refusal was the same command with the same mistake, because the check was retyped rather than escaped.
• That is #945 exactly: a guard that scans raw text cannot tell content from a pattern that matches content, so it refused a compliance check for itself. The PM lane guard's own header cites this incident as the reason it tokenizes rather than greps, and the em dash guard is still unchanged.
• No override was used and none was asked for. The check was rewritten to express the character as a codepoint escape, which satisfies the guard's actual condition (nothing written adds a U+2014) rather than evading it. Recorded here because a guard misfiring is data, and because the session that hit it was three fixes deep into the same failure mode in a sibling guard.

What went wrong in this session that was not a finding?

The first commit swept in 29 files another lane had staged.

• git add <my two paths> then a bare git commit takes the WHOLE INDEX, and the index is shared across every session in this checkout. Another lane had staged docs/projects/brainstorm-archive/** between this session's start and its first commit, and those files landed in commit 4e28fe3a under a message about a file mode.
• Nothing is lost: the files are committed and pushed on the branch. What is wrong is the message, which describes one file and carries thirty.
• It was not undone, because undoing it is git reset, which rule 17 puts behind an action-specific yes that this session does not have. Surfaced to the PM instead.
• The fix for the rest of the session is the pathspec form, git commit -- <paths>, which takes only what it names and leaves the shared index alone. That is the same reasoning the guard's own git commit branch already carries in a comment, which is where it should have been read first.