golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
5,867 Commits 1 Branch 0 Tags
ca8d9d4459feeaef716810005b3325e391d4ae92
Commit Graph

11 Commits

Author SHA1 Message Date
Tom Boucher
ca8d9d4459 fix(#4429): stop a large commit_types config blocking or bypassing the gate (#4723) 
* test(#4429): regression coverage for three defects in the commit hook

Failing-first coverage. Every conforming-subject row is red against the
unfixed hook, and each defect gets an explicit CONTROL row that reconstructs
the pre-fix form and asserts the defect reproduces -- without those, the
passing rows would pass with or without the fix.

1. SIGPIPE (the reported defect). The pre-fix first-line extraction used a
   `head -1` pipeline; once CONFIG_OUT exceeds the 64 KiB pipe buffer printf
   is killed and `set -euo pipefail` aborts the hook. That fix is already on
   next -- it landed incidentally in #4537, whose message never mentions
   #4429 -- and nothing in the tree would notice its removal.

2. regcomp. The commit-type alternation grew with the CONFIGURED list and
   exceeded bash's 64 KiB compiled-pattern cap. Boundary rows pin the cliff
   at 6051/6052, with controls on BOTH sides so limit-1 is not vacuous.

3. Ambient subprocess statuses (found by this change's security review).

Defects 1 and 2 cannot be separated: each configured type adds len+1 bytes to
CONFIG_OUT and len+1 to the alternation, so the smallest payload that
overflows the pipe (N=6059) already puts the alternation past the ceiling.

The SIGPIPE control accepts either SIGPIPE (141, Linux) or a reported write
error (macOS bash 3.2's builtin printf, exit 1). Asserting only the message
would go red on every CI lane, since the remote matrix is Linux-only.

Named to bucket with gsd-validate-commit-crash-policy.test.cjs, which covers
this same hook: lint-test-file-count derives a test's owning module from its
filename prefix, and `validate-commit-*` collided with the `validate` module,
already at its 2-file cap.

Harness note, learned from three vacuous control runs: hooks/lib/git-cmd.js
requires ../gsd-core/bin/lib/token-scanner.cjs relative to the hooks dir's
parent, so a copy in a bare tmpdir fails open and returns 0 for any input.
The layout symlinks gsd-core beside the copy, and every row that can prove it
asserts the run was substantive.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#4429): bound the commit-type regex and isolate subprocess statuses

Two fixes in the same file, both of the same shape: a value computed for one
purpose was being read as authority about something else.

1. The commit-type alternation could not be compiled.

COMMIT_TYPE_ALT joined every CONFIGURED type into one regex, so the pattern
grew without bound. bash caps a compiled pattern at 64 KiB. Bisected on bash
3.2.57 (this repo's macOS target): a 65504-byte alternation compiles, 65515
fails. `[[ =~ ]]` returns 2 on a compile failure, and `if !` cannot tell that
from "the subject does not conform" -- so the hook blocked a valid
`feat(auth): ...` with CONVENTIONAL_COMMITS_VIOLATION while printing `feat`
in its own valid_types.

Match the shape with a fixed-size pattern, capture the type, then test
membership against the COMMIT_TYPES array. The character class is exactly the
`^[a-z][a-z0-9-]*$` safe-token filter the config loader already applies, so it
captures every type that can legally reach COMMIT_TYPES and no token that
cannot. Review verified equivalence over 46 handcrafted plus 6000 randomized
adversarial subjects against a type list containing prefix-overlapping,
digit-bearing and trailing-hyphen types: zero divergences. The loop adds no
subprocess and no pipe, which is the hazard class #4429 is about.
COMMIT_TYPE_ALT is now unused and removed.

  types   pre-fix `feat(auth): ...`   fixed
  10      accept                      accept
  6051    accept                      accept
  6052    BLOCK                       accept
  20000   BLOCK                       accept

2. Subprocess statuses were inherited from the environment.

Each status is captured as `... || VAR=$?`, which assigns ONLY on the failure
branch; on success the variable kept whatever it already held, and
`${VAR:-0}` defaults only when unset or empty. So an EXPORTED CONFIG_STATUS,
CMD_STATUS or CLASSIFY_STATUS -- from a CI wrapper, a .envrc, or another hook
-- survived into the success path and was read as "the subprocess failed".
Since the hook fails OPEN on a genuine subprocess failure by design (#3838),
the result was a silent bypass. Measured: `CLASSIFY_STATUS=3 git commit -m
"nope: bad"` printed "validator disabled for this call" and exited 0.

The three are now initialised before use. The fail-open path is unchanged and
verified byte-identical to origin/next with a failing node.

hooks/dist/ is gitignored and rebuilt from hooks/ by scripts/build-hooks.js,
so there is no second copy to sync.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#4429): register the new suite with the conformance manifests

Both conformance-tier manifests embed the test-file list, so adding a test
file makes them stale. Regenerated with their own generators:

  node scripts/gen-platform-conformance-tier.cjs --write
  node scripts/gen-platform-conformance-tier.cjs --target macos --write

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#4429): pin both fail-open-prone controls to their named cause

Two rows in the ambient-status block asserted `status === 0`, which the hook
also returns when the harness layout is broken -- so either row could have
passed for entirely the wrong reason. This is the same vacuity trap the rest
of the suite already guards, applied inconsistently to the rows added last.

Measured, rather than reasoned about:

  genuine ambient bypass (pre-fix hook, CLASSIFY_STATUS=3)  rc=0, no CLASSIFIER_THREW
  orphaned layout (no gsd-core symlink)                     rc=0, CLASSIFIER_THREW
  genuine fail-open (node shim exits 3)                     rc=0, no CLASSIFIER_THREW

So assertSubstantive separates the intended cause from the harness failure in
both rows, and each now pins its pass to the cause it names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#4429): stop asserting a macOS-only regex cap on every platform

First verification run was RED: 45636/45638 passed, both failures in this new
suite on linux-node24. Cause is mine -- I measured the compiled-pattern ceiling
on macOS and encoded it as a cross-platform expectation.

Measured in the tester image itself:

  engine                            6051      6052      20000
  bash 3.2.57 / BSD libc (macOS)    compiles  rc 2      rc 2
  bash 5.2.15 / glibc  (Linux)      compiles  compiles  compiles (228943 B)

glibc has no reachable cap, so the regcomp defect cannot occur there and the
control asserting a block at 6052 was red for a behaviour the platform cannot
produce.

The control now calibrates at runtime: it runs the pre-fix form and, when this
engine compiled the alternation, it SKIPS with a message naming the reason
rather than asserting. Skipped out loud, never silently passed -- a green row
there would read as "the defect is covered" on a platform where it cannot
occur. Both branches verified: the capped branch asserts (macOS 17/17, zero
skipped), and the uncapped branch was exercised by forcing the payload to a
size that always compiles, producing a skip and not a failure.

Consequence stated rather than hidden: the remote matrix is Linux-only, so this
one control is skipped in CI and really runs only on a macOS workstation. The
rows that run everywhere are the ones carrying the regression weight -- the
shipped hook accepting a conforming commit at every payload size, the gate
still blocking unknown types, the SIGPIPE control, and all seven ambient-status
rows.

Note this also narrows the coupling claim: SIGPIPE and regcomp are coupled only
on a capped engine. On glibc the SIGPIPE defect is directly testable without
the regcomp fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(#4429): scope the regex-cap claim to the platform it applies to

The changeset told users the validator "built a regular expression bigger than
bash can compile" past ~6,000 configured types. That is false on Linux: glibc
compiled a 228,943-byte alternation without complaint, so a Linux reader would
have been misled about their own exposure. These are user-facing release notes,
so the claim is now scoped to macOS (bash 3.2 / BSD libc) and says explicitly
that glibc was never affected by this half.

The hook's own comment led with the same overstatement -- "bash caps a compiled
pattern at 64 KiB" -- before qualifying it. Reworded so the first clause states
what is actually true: the limit is a property of the platform's regex engine.

Text only; no behaviour change. Suite 17/17, eslint and lint:ci clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#4429): backfill changeset PR number (#4723)

* chore(#4429): backfill changeset PR number (#4723)

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-09-14 05:43:59 -04:00
Behruz Nassre Esfahani
137f3115a1 fix(#4492): index the suffix window instead of pattern-matching it (#4539) 
* fix(#4492): index the suffix window instead of pattern-matching it

`MSG_SUFFIX="${CMD#*"$MSG_MATCH"}"` is quadratic in the -m message. bash tries
every prefix length and compares the whole matched literal at each, and
MSG_MATCH is BASH_REMATCH[0] — the entire `-m "..."` — so the cost grows with
the thing being scanned. Measured on the real hook: 10.0s at 64KB, 22.0s at
96KB, 30.2s at 112KB, 40.1s at 128KB. `bash -x` with an EPOCHREALTIME PS4
attributes 10.116s of a 10.2s run to that one expansion, which computes an
empty string. Conforming and non-conforming cost the same, so this is the path
every commit takes, and Claude Code blocks on PreToolUse hooks.

MSG_PREFIX on the line above has already located the match, so the suffix is
arithmetic rather than a search. Same first-occurrence assumption both
expansions always made — MSG_MATCH is a literal substring of CMD by
construction. Equivalence checked across 480 comparisons on bash 3.2.57 and
5.3.15 under C, UTF-8 and SJIS locales, including multibyte text, repeated
matches, metacharacters and invalid bytes.

Three regression rows, all deliberately on the RESOLVE=1 path so they pin the
suffix scan alone and do not depend on the separate #4429 SIGPIPE fix:
non-conforming and conforming 112KB heredocs, plus a suffix-window row whose
padding sits before the heredoc opener's newline so the COMMAND is large while
the message stays small. Red against the true base — all three killed at the
10s bound with the head -1 sites still present — and green with only this
change.

Fixture sizes stay under Linux MAX_ARG_STRLEN (131072 on a 4KB-page kernel).
Above it execve fails, the classifier cannot launch and the hook fails open, so
a larger fixture measures the argument limit rather than the suffix scan; an
earlier 131225-byte draft passed on base AND head for exactly that reason.
Every row asserts empty stderr, which is what separates "validated" from
"failed open".

The bound is enforced by killing the process GROUP, not the direct child: the
hook spawns a node classifier that inherits stdout, so killing only bash can
leave the pipe open and `close` never arrives. `local/no-elapsed-assertion`
forbids asserting on elapsed time, and `{ timeout }` is inert on a synchronous
body, so the rows are async and the kill is the signal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZw5UhR474YLyE4knjHrte

* chore(#4492): add changeset

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZw5UhR474YLyE4knjHrte

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
 2026-09-09 05:48:17 +00:00
Tom Boucher
78013b3b74 fix(#4447): classify mixed structural+transient planning commits as a 5th arm (#4537) 
* fix(#4447): classify mixed structural+transient planning commits as a 5th arm

pr-branch.md's analyze_commits step computed only NON_PLANNING and
STRUCTURAL per commit -- never a total planning-file count -- so its four
classification arms assumed every planning-only commit was either wholly
structural or wholly non-structural. A commit touching both a structural
.planning/ path and a transient/other one matched no arm, and the
ambiguous prose let an LLM executing the workflow silently drop it,
breaking STATE.md's per-commit revision chain in default mode.

Adds an explicit PLANNING_COUNT variable and rewrites the four arms into
five, each with an exact computable condition. The new "mixed planning
commit" arm (structural + transient/other, no code) gets the same
treatment mixed code+planning commits already get: INCLUDE, relying on
create_pr_branch's existing universal per-commit filter to strip the
transient/other paths -- no new filtering logic needed.

tests/helpers/pr-branch-filter.cjs's classifyCommit already returned
'include' for this shape (no upper bound on its structural check); the
defect was entirely in the workflow's own prose spec, which is what an
executing agent actually reads. New tests pin both: classifyCommit's
already-correct behavior (tests 49-50), and a failing-first assertion
that analyze_commits computes an explicit planning-total signal (test
51, fails against the pre-fix text).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(#4447): address code-review findings on the mixed-planning arm

- Correct the mixed-planning arm's prose: create_pr_branch's universal
  filter only strips the TRANSIENT_DIRS subset, not the "other" bucket
  (config.json, intel/, etc.) -- that subset is preserved, not filtered,
  same as default mode already does for it on any commit.
- Fix the "Mixed planning commits" display line to use the same
  mode-conditional bracket form as "Structural planning commits" --
  it was hardcoding "included" even though the arm is EXCLUDE in strict
  mode, which would have misled a strict-mode user.
- Tighten test 51's regex from unanchored /PLANNING_COUNT=/ to
  /^PLANNING_COUNT=\$\(/m so it requires the real shell-assignment
  shape, not just the substring appearing anywhere in prose.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Emitted-Drift-Ack-Growth: pr-branch.md — growth is this PR's own #4447 fix (5th classification arm with explicit computable conditions), not incidental drift

* fix(#4447): bound test 51's regex quantifier (local/no-unbounded-quantifier)

lint:ci flagged the unbounded [\s\S]*? over readFileSync content as a
catastrophic-backtracking risk (CWE-1333 class). Bounded to {0,20000},
comfortably larger than the analyze_commits step's actual size.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(#4447): add changeset for the pr-branch mixed-planning classification fix

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: eliminate SIGPIPE race in gsd-validate-commit.sh subject/config extraction

Discovered while verifying an unrelated PR (#4447): tests/hooks-opt-in.test.cjs's
"a git-GENERATED subject is never measured against the supplied message
(round 7)" test intermittently got r.status===141 instead of the expected 2
for the --fixup=HEAD case, on a run where the identical code had passed
cleanly moments earlier -- confirming a timing race, not a deterministic
bug in the test's own assertions.

Root cause: gsd-validate-commit.sh runs under `set -euo pipefail` and
extracted the commit subject via `SUBJECT=$(echo "$MSG" | head -1)` (two
call sites) and the opt-in ENABLED flag via `$(printf '%s\n' "$CONFIG_OUT"
| head -1)`. `head -1` closes its read end as soon as it has one line; a
real commit message or multi-command-type CONFIG_OUT is multi-line, so the
writer can receive SIGPIPE (exit 128+13=141) if its write lands after that
close. Under pipefail this is NOT suppressed -- it aborts the whole hook
instead of the intended exit-2 rejection.

Fix: replace both patterns with pure bash parameter expansion
(`${VAR%%$'\n'*}`) -- zero subprocesses, zero pipe/race surface, and
behaviorally identical to `head -1` for single-line, multi-line, and
trailing-newline input (verified directly). The third similar pipe
(`tail -n +2` feeding a `while read` loop that drains to EOF) is a
different, race-free shape and was left alone.

Regression test is a static, by-construction assertion (per this repo's
policy against forcing scheduling races to reproduce deterministically):
the vulnerable pipe patterns must be absent from the shipped script, and
the parameter-expansion forms must be present.

This overrides one-concern-per-PR per CLAUDE.md's Defects & Warnings
policy -- a genuine defect discovered mid-work is fixed inline, not
deferred to a separate issue.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs: add changeset for the gsd-validate-commit.sh SIGPIPE race fix

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: harden SIGPIPE-race regression test against reformatted reintroduction

Code review found the test's original exact-string regexes would miss a
cosmetically-reworded reintroduction of the same dangerous head-1 pipe
(extra whitespace, an appended 2>/dev/null). Broadened to content-tolerant
but still $(...)-wrapped regexes (bounded quantifiers per
local/no-unbounded-quantifier) -- verified against both the current file
(no false positive, including the fix's own explanatory comments that
quote the bare unwrapped pattern in prose) and a synthetic reformatted
reintroduction (correctly caught).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(#4447): backfill changeset PR number

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-09-08 00:19:03 -04:00
Tom Boucher
4e1c449281 enh(#3811): add hooks.commit_types config surface to gsd-validate-commit (#4340) 
* enh(#3811): add hooks.commit_types config surface to gsd-validate-commit

Extends the opt-in Conventional Commits hook with a hooks.commit_types
config array that adds project-specific types to the 10 built-ins
without replacing them. Configured values pass a safe-token filter
before reaching the compiled regex, so a config entry can never alter
the pattern's structure. The regex alternation, the human-readable
error text, and a new typed valid_types JSON field all derive from one
list instead of the two hand-synced copies this replaces.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore(#3811): backfill changeset PR number

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-09-05 18:35:09 -04:00
Behruz Nassre Esfahani
4499933807 fix(#3802): resolve the heredoc body before validating the commit subject (#3816) 
* fix(#3802): resolve the heredoc body before validating the commit subject

With hooks.community: true, gsd-validate-commit.sh blocked EVERY heredoc-form
commit with CONVENTIONAL_COMMITS_VIOLATION regardless of the message, including
Claude Code's own documented idiom:

    git commit -m "$(cat <<'EOF'
    feat(auth): add login flow
    EOF
    )"

Reproduced before changing anything: conforming heredoc -> exit 2; plain
-m "feat(auth): add login flow" -> exit 0.

Root cause is the extraction regex `-m[[:space:]]+"([^"]+)"`. Bash `[^"]`
matches newlines, so the capture ran from the quote after -m to the FINAL quote
at `)"`, swallowing the whole span. `head -1` then returned the literal
`$(cat <<'EOF'` as the subject, which can never satisfy Conventional Commits.

Fixed by not answering a regex bug with another regex. hooks/lib/git-cmd.js
already exists because "a naive regex misses all three" invocation forms, and
extractBranchArgument is the established precedent for pulling an argument off a
git command line. extractCommitSubject joins it on the same tokenizeShellLike
seam — which, checked first, already returns the entire heredoc span as ONE
token, leaving only "resolve the body to its first line" as new logic.

Because the walk starts at the subcommand, `git -C <path> commit` and
env-prefixed invocations now extract correctly too — forms the raw string scan
never handled.

Deliberately unchanged, and pinned as such: a glued `-mfeat: x` and
`--message=...` still yield no message, exactly as the regex left them. The fix
stays scoped to the reported defect rather than widening on a true observation.

Two things I got wrong and corrected by measuring rather than reasoning:

  - I expected `git commit -m ""` to be blocked. Checked against the ORIGINAL
    hook: allowed before, allowed now, identical. The scanner drops the empty
    token so it takes the null path. My expectation was wrong, not the code.
  - That exposed a false comment I had just written, claiming the exit-status
    split prevents silently allowing `-m ""`. It does not. The split IS
    load-bearing, but for a heredoc whose body's first line is blank, which
    resolves to an empty subject and is correctly blocked. The comment now names
    the real case and records that `-m ""` is not it.

Tests at both layers: 9 unit rows on extractCommitSubject beside its sibling in
tests/worktree-safety.test.cjs, and 5 behavioral rows piping real PreToolUse
payloads through the hook in tests/hooks-opt-in.test.cjs. Replacing
firstLineOfMessageArg with a plain first-line return reds 8 of them across both
files. (A first mutation attempt silently no-opped and reported green — the
mutated body is echoed in the transcript for the run that counted.)

Out of scope, per the issue: the hooks.commit_types config surface, split off by
the maintainer as #3811 and explicitly sequenced after this.

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): confine the fix to heredoc resolution, closing four regressions

Codex review of the first attempt. It was right, and the finding is one my own
rules already name: a true observation is not a licence to widen the diff.

The first attempt replaced the shell's `-m` extraction with a token walk. That
looked like the better abstraction — this module exists precisely because a
naive regex misses invocation forms — but selecting WHICH argument is the
message was never the defect, and changing it regressed four forms that
upstream allowed, plus opened a bypass:

  - `git commit -- -m WIP`             -- introduces pathspecs; `-m` is a path
  - `git commit --amend && echo -m WIP` a later command's flag became the message
  - `git commit -m "" --allow-empty-message`  the shared scanner drops empty
                                        tokens, so the next flag became the
                                        message
  - `git commit -m WIP`                unquoted argument
  - `-m "WIP notes <<EOF\nfix: smuggled subject"` was ALLOWED — the opener was
    recognised unanchored, so validation skipped past the real, non-conforming
    subject. An enforcement bypass, not a misclassification.

Now confined to the actual defect. The shell's `-m` capture is restored byte for
byte, and only the subject-from-message step is delegated, to a PURE STRING
helper `resolveCommitSubject()` that never tokenizes. Verified as a differential
against the upstream hook run inside the real tree: the only behaviours that
change are the two intended heredoc rows (2 -> 0); all four forms above read
identical, and the bypass case blocks.

That differential also corrected my own control. An earlier comparison ran the
upstream hook from a scratch directory, where its `lib/` could not resolve
`../../gsd-core/bin/lib/token-scanner.cjs`, so the classifier failed open and
reported exit 0 for everything. That made a real regression look pre-existing.
Re-run inside the tree, `<<-"TAG"` (a double-quoted tag nested in the
double-quoted argument) is genuinely pre-existing — the capture truncates — and
is now recorded as a known limitation rather than silently "fixed".

Also fixed from the review:
  - `<<-` strips leading TABS from body lines; returning the raw line blocked a
    conforming message.
  - a non-identifier tag such as `END-MSG` is a valid bash word and was rejected.
  - an immediately-following terminator is an EMPTY message, not a subject.
  - a node/library failure now falls back to the previous `head -1` instead of
    skipping validation, so a broken extractor degrades to old behaviour rather
    than becoming a new silent-allow path.

Tests strengthened per the review: the opener-spelling rows now assert BOTH
directions per spelling, since "conforming passes" alone would also pass if the
resolver returned an empty subject for a spelling it failed to parse. Added
differential rows pinning the five previously-allowed forms, and a row for the
bypass. Dropped two rows whose comments claimed the raw scan could not handle
`-C`/env-prefix invocations — it could; the claim was wrong.

Replacing resolveCommitSubject with a plain first-line return reds 9 rows across
both files. (Mutant body echoed in the transcript; an earlier mutation attempt
on this branch silently no-opped and reported green.)

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): keep the installed hook runtime-neutral

`hooks/lib/git-cmd.js` ships into every runtime, including hermes and qwen,
where tests/install.test.cjs enforces that no Claude reference leaks into the
installed tree. My JSDoc named the idiom after the runtime that documents it.

Reworded to describe the SHAPE rather than the vendor; the runtime is still
named in the changeset, which feeds CHANGELOG.md where such references are
allowed, and in the tests, which are not installed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#3802): backfill changeset pr number

The fragment shipped with the documented `pr: 0` placeholder, which the
changeset lint treats as always-silent, because the number does not exist until
the PR is opened. Backfilled to 3816 now that it does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): close the truncated-capture hole, add the required test artifacts

Review round 1. Major 3 was the one that mattered, and it disproved a claim I
had stated in falsifiable form — the PR body said only two behaviours change;
the differential found five.

Major 3 — an embedded `"` truncates the `-m` capture, so the resolver received a
PREFIX of the real subject and the length gate measured the wrong string. Before
this fix the whole form was blocked outright, so the gate was unreachable; the
fix opened the path and then mismeasured it. A new enforcement hole, so it is
CLOSED here rather than declared.

Closed precisely rather than bluntly. A first attempt refused to resolve any body
with no terminator, which also blocked commits whose SUBJECT was intact and whose
quote sat further down the body — a false positive of its own. Truncation is only
fatal to the line it lands IN, and a captured line is complete exactly when
another line follows it, because the capture kept its newline. So an unterminated
body whose subject line is followed by more text stays measurable; only a subject
line running to the end of a truncated capture falls back to the opener, which
fails the format gate exactly as this form did before the fix.

Major 1 — fast-check property rows for the new parser, via the shared seeded
setup helper rather than requiring fast-check directly, per repo convention:
totality (a security property here, since an exception on this path fails OPEN),
idempotency, and that the result is always a single line drawn from the input —
the third catches a resolver that concatenated or trimmed while satisfying the
first two.

Major 2 — the 72-char gate is now exercised at {71, 72, 73} on the RESOLVED
heredoc subject, with the fixture length asserted so a mis-built fixture cannot
silently pass. 92 chars did not show which side of `> 72` the code sits on.

Minor 1 — leading blank body lines are skipped, as git's cleanup=whitespace does.
A conforming commit written that way was still blocked, which is the same defect
class #3802 reports.

Nit 1 — a backslash-escaped delimiter (`<<\EOF`) is now the same delimiter rather
than failing closed on a delimiter that includes the backslash.

Nit 5 — changeset trimmed from 2,208 chars of design note to the user-visible
change.

Mutation discipline, including a correction to my own: dropping the truncation
guard reds the unit rows, and the pre-review naive shape reds the hook-level row
too. My first mutant did NOT distinguish the hook row — removing the guard made
an empty slice and blocked for an unrelated reason, so the row passed and looked
proven. Only mutating to the actual pre-review shape showed it discriminates.

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): measure the subject as git does — strip trailing whitespace, split CRLF

git's cleanup=whitespace strips whitespace at BOTH ends of a line; the
resolver handled only the leading direction, so a 72-char subject with
trailing spaces measured 75 and stayed blocked — the defect class #3802
reports, surviving one round further (review of #3816, Major 2). The
resolved subject now drops trailing spaces and tabs; the plain non-
heredoc path is untouched, keeping the fix confined to heredoc
resolution. The length-gate boundary rows gain dirty fixtures: 72+3
trailing spaces passes, 73+1 stays blocked on LENGTH.

split('\n') left \r on every body line, so on CRLF input the delimiter
never matched: the truncation guard was inert, an empty message resolved
to 'EOF\r', and a real 72-char subject measured 73. Split on /\r?\n/
(Minor 3).

The three property tests never reached the parser — the pinned-seed
fc.string corpus contained no newline and no opener, so every property
reduced to f(s) === s (Major 1). The generator now constructs heredoc-
shaped input (all opener spellings, <<- tabs, optional terminator, CRLF)
and each property asserts a floor on inputs its corpus actually resolved.
All new rows proved failing-first against the pre-fix resolver.

Also records the unquoted-delimiter expansion limit as one JSDoc
sentence (Informational 5).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): close two recognition bypasses, pin the dquoted-delimiter limit

Codex whole-PR review found two enforcement bypasses in the resolver:

- The opener's path prefix was \S*, which accepted `id;/bin/cat` — the
  resolver then validated the heredoc BODY while bash runs `id` first
  and git's real subject is id's OUTPUT. The prefix is now a
  path-character class; any shell metacharacter fails recognition and
  the form falls back to the opener line and the format gate.

- The blank-line skip used JavaScript trim(), whose Unicode whitespace
  class skips lines git KEEPS: a NBSP first body line resolved to the
  SECOND line while git's real subject is the NBSP line (verified
  against git stripspace — the c2a0 bytes survive). Blank is now git's
  ASCII space/tab only; a Unicode-blank line is returned and fails the
  format gate, the same fail-closed direction git takes.

Both proven failing-first at resolver AND hook level. Also: the
<<"TAG" spelling is recorded as a documented limit — the -m capture
stops at the delimiter's own quote so the caller can never deliver it
(fail closed; widening the capture would change every embedded-quote
case) — with a hook-level row pinning the limit; and the derivation
property no longer accepts '' unconditionally, only for heredoc-shaped
input, so a conditional constant-'' regression can't satisfy the corpus
floor unnoticed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): recognition whitespace is ASCII, and '' answers to the generator

Codex round 2: the opener's \s accepted Unicode whitespace bash does
not split on — $(<NBSP>/bin/cat was recognized here while bash reads
<NBSP>/bin/cat as the executable NAME, so recognition claimed a
substitution that does not run cat. Every whitespace position in the
recognition is now [ \t], the same ASCII rule as the blank-line skip,
proven failing-first.

The derivation property's ''-acceptance now consults GENERATION-TIME
metadata: the heredoc generator records whether it built an empty
message (terminator reachable, all scanned lines ASCII-blank, <<- tab
stripping accounted for), and '' is accepted exactly then — a resolver
conditionally degrading to '' on non-empty heredocs now fails, closing
the residual round-1 permissiveness without re-deriving resolver logic.

The changeset no longer overstates the opener spellings: it names the
capture-deliverable set and the documented <<"EOF" limit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): nothing after the terminator escapes measurement

Round-3 BLOCKER: everything after the heredoc terminator was silently
discarded, so `-m "$(cat <<'EOF'\nfeat: ok\nEOF\n) <200 a's>"` —
one 200+ char real subject once bash substitutes — measured 8 chars and
dodged COMMIT_SUBJECT_TOO_LONG, a hole the base did not have. The
canonical idiom's tail is exactly one closing-paren line; any other tail
now falls back to the opener line and the format gate, the pre-fix
behaviour for the whole form. Proven failing-first at resolver and hook
level, including the glued-text and second-substitution variants.

Also from round 3: `cat<<'EOF'` (no space) is legal bash and now
resolves — the token before << is still literally cat; the env-prefixed
and option-terminated spellings join the JSDoc KNOWN LIMIT list instead
(fail closed, modelling bash prefix words is cost with no reported
user); the changeset states the embedded-quote truncation limit for the
message body, not just the <<"EOF" spelling; the dquoted unit and hook
rows now cross-reference each other; and the fast-check setup helper's
docstring no longer claims property-file exclusivity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): glued text outside the closing quote must not shrink the measurement

Codex on the round-3 guard: bash concatenates -m "$(…)"suffix into ONE
argument, but the capture holds only the quoted part — so the resolver
measured the heredoc body (8 chars) for a 200+ char real subject, a
net-new length-gate bypass the base did not have (base measured the
opener and blocked). When the closing quote is followed by anything but
whitespace or end-of-command, the hook now skips the resolver and keeps
the pre-fix first-line subject: the heredoc form fails the format gate
exactly as on base, and the plain single-line form keeps base behavior
unchanged — both pinned as differential rows, the glued-suffix row
proven failing-first against the unguarded script.

The property generator's ''-oracle now models the post-terminator guard
it previously predated: expectEmpty requires the FIRST reachable
terminator to be followed by the one canonical closing-paren line, so a
resolver regressing to '' on a non-canonical tail (e.g. a body line that
doubles as an early terminator) fails the derivation property instead of
being blessed by stale metadata.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: retrigger CI — the previous wave never started (Actions queue stall)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): only resolve a heredoc whose body bash does not rewrite

Round-4 review found two net-new enforcement bypasses: commands the base
hook blocked (exit 2) that this branch allowed (exit 0). Both reproduced as
a base-vs-head differential against the real hook, not inferred.

The predicate "may I resolve this?" was computed from the resolver's input
string alone, while two of its determinants live outside that string:

  1. WHICH -m quote arm produced the input. Inside -m '...' bash performs no
     command substitution, so $(cat <<'EOF' is literal text and git's real
     subject is the opener line. The resolver ran on both arms, so all four
     delimiter spellings went 2 -> 0 on the sq arm — reachable by the
     ordinary slip of typing ' for ". The hook now records MSG_QUOTE and
     gates the resolver on dq; sq keeps head -1, exact base parity.

  2. WHETHER the delimiter suppresses expansion. Only <<'D', <<"D" and <<\D
     do; a bare <<D is expanded by bash before git sees it. Resolving the
     literal dodged the format gate (feat: $UNSET_VAR reaches git as feat:)
     and the length gate (feat: ${LONG} reaches it at any length). The
     opener regex now separates the backslash-quoted and bare alternatives
     and refuses the bare one — the same fail-closed rule the metacharacter,
     truncation and post-terminator guards already follow.

A test row asserted exit 0 for a bare-delimiter body, so the suite defended
the second bypass and the fix could not land without editing a test that
read as intentional. That row and its two unit counterparts now assert the
block, per RULESET.TESTS.delete-bad-tests. Two unrelated rows used <<-EOF
to exercise tab stripping; they move to <<-'EOF' so each tests what it names.

Scoping the adjacency guard to the matched arm — required by the fix above —
also removes a spurious block (round-4 Minor 1): a double-quoted heredoc
whose body mentioned a glued single-quoted token tripped the sq arm.

The JSDoc claimed <<"EOF" was unreachable through the caller and that the
bare-delimiter gap was pre-existing. Round 4 disproved both; both corrected
here, along with the matching changeset sentence.

Verified: 7 bypass commands now block at head (was allow), the #3802 fix and
plain-form parity are unchanged across 8 control commands, hooks-opt-in 44/44,
worktree-safety 401/401, property-test non-vacuity 73/200 against a floor of
20, lint:ci exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3802): resolve only where the captured text is provably git's subject

Codex review of the full PR found two more inputs where the validated text
is not the subject git receives, both net-new bypasses (base 2 -> head 0),
plus one escalation of round-4 Minor 2. All reproduced here against the real
hook and confirmed against real commits before fixing.

BLOCKER — the matched -m need not be git's message. The capture is a search
over the whole command and the double-quoted arm runs first, so it could
select a -m that is not the subject at all. git concatenates multiple -m
values and takes the FIRST as the subject, so

    git commit -m 'WIP first' -m "$(cat <<'EOF' … )"

commits the subject `WIP first` while the hook validated the heredoc. Same
for an unquoted earlier -m, for a heredoc after `--` (a pathspec, not a
message), and for one belonging to a later `&& echo`. The mis-selection is
pre-existing; resolving it is what made it a bypass. The hook now resolves
only when nothing before the matched -m could have been an earlier message,
an end-of-options marker, or another command.

BLOCKER — cleanup mode is part of the predicate. The resolver skips leading
blank lines and strips trailing whitespace because git's DEFAULT
cleanup=whitespace does. Under --cleanup=verbatim git does neither, so a
72-char subject plus three trailing spaces is committed at 75 bytes while
the hook measured 72 — COMMIT_SUBJECT_TOO_LONG dodged. This one hides from
`git log --pretty=%s`, which strips trailing whitespace in its own output;
the raw commit object shows 75 vs 72. Any named mode other than whitespace,
in either the --cleanup= or -c commit.cleanup= form, now refuses to resolve.

MAJOR — recognition trusted any path ending in /cat, so a planted
`../evil/cat` printing `WIP injected` had its heredoc body validated while
git's real subject was `WIP injected`. Only a bare `cat` or an absolute path
is recognised now. A bare `cat` shadowed on PATH is a documented residual and
is not fixable from a string — nor a meaningful boundary, since planting an
executable already allows running git directly.

The changeset and the JSDoc both asserted that a `"` anywhere in the message
blocks. Measured false: a `"` on a later body line resolves fine, because the
subject completes before the truncation point; only a `"` in the subject line
blocks. The changeset also listed <<"EOF" as covered when it measures 2/2.
Both rewritten to claim only what is measured, and the residual false
positives are now named.

Verified: 4 + 2 + 3 new bypass commands now block, with non-vacuity controls
proving the default path still resolves; all round-4 maintainer blockers stay
closed; the #3802 fix and plain-form parity unchanged across 7 controls;
hooks-opt-in 47/47, worktree-safety 402/402, property non-vacuity 73/200
against a floor of 20, lint:ci exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3802): scope the cleanup-mode guard to the command outside the message

The guard scanned the whole $CMD for `--cleanup=` / `commit.cleanup=`,
and the heredoc BODY sits verbatim inside $CMD, so any conforming
message that merely MENTIONED the token was refused, fell back to the
opener line, and was blocked with CONVENTIONAL_COMMITS_VIOLATION. These
are ordinary English in this repository, whose own hooks and docs
discuss cleanup modes constantly. Reproduced against the real hook:
`fix: document commit.cleanup=strip behavior` blocked, the same message
without the token allowed (review of #3816, round 5 — BLOCKER).

Scoping to $MSG_PREFIX alone, as prescribed, would have reopened the
round-4 length-gate bypass the guard exists for: git accepts the flag on
EITHER side of -m, and `git commit -m "<heredoc>" --cleanup=verbatim` is
caught today only because the scan is command-wide. Measured, not
assumed. The scan now covers MSG_PREFIX + MSG_SUFFIX — the whole command
minus the one span that is message text — joined with a space so a token
cannot be forged across the seam.

Swept the guard class rather than the reported instance. The adjacency
guard does not share the defect: an in-body `-m "foo"bar` is refused by
the already-documented embedded-quote capture limit (any `"` in the
subject line truncates the capture), and an in-body `-m ` without quotes
resolves and is allowed. Deliberately untouched.

Both directions pinned failing-first: the three false-positive rows red
against the unscoped guard, and the trailing-flag row reds against
prefix-only scoping. Each mutation was echoed back to prove it landed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XogDtuuuGQEfsWaLSaZCLB

* fix(#3802): read commit options the way bash hands them to git

Round 6 reported the adjacency guard scanning all of $CMD for a glued
`-m "..."`, so a glued -m belonging to a chained-after command refused a
heredoc that was never truncated. Glue is a property of the ONE character
following the matched span, so that character is now the whole window.
Separators and redirections are excluded because bash does not
concatenate across them: in `-m "msg"&& echo hi` the argument ends at the
quote, so there is no truncated capture to defend against.

An independent full-PR pass then found three accept-direction defects
this PR had introduced in earlier rounds, each measured against a real
commit by reading the raw commit object — `git log --pretty=%s` strips
the trailing whitespace that makes the length wrong and hides it:

  --cle=verbatim         git accepts any unambiguous prefix of a long
                         option, so the mode was set by a token that is
                         not the literal --cleanup. 75-byte subject
                         recorded, 72 measured.
  -am 'WIP first'        git reads this as -a -m, so the real subject is
                         `WIP first` and the heredoc is only the second
                         message. The scan looked for a standalone -m.
  --clean""up=verbatim   bash removes quotes before git sees the
  -""m                   argument, so a spliced spelling is the same
                         option and matched no literal.

The two option-name scans now read their window with quote characters
removed, which is what bash does to it, and the cleanup class covers
git's abbreviations. The adjacency test deliberately keeps the raw text:
it asks about a literal character position, not an option name.

Narrowing the cleanup window to git's own command segment was tried and
reverted. `;`, `&` and `|` end a command only outside quotes, and this is
a substring scan, not a parse: an unconditional trim cut the window short
on `--author "a&b"`, and a quote-aware trim still cut it on `--author
a\&b`. Each hid a real trailing --cleanup=verbatim and accepted a 75-byte
subject. The resulting false positive — a --cleanup carried by a chained
command refuses the commit — is documented and pinned instead. Refusing a
commit git would take is recoverable; accepting an over-long subject is
not.

Sixteen rows in tests/hooks-opt-in.test.cjs. Seven mutations, including
both reverted narrowings, so no dead end can be reintroduced silently.

* fix(#3802): close six accept-direction bypasses in the resolve guards

Round 7's FIRST-MESSAGE GUARD Major does not reproduce. Measured against the
real hook in a complete tree at the reviewed head: the classifier gate runs
before any guard, so `git add -A && git commit …` (git->add stops on a
non-commit subcommand) and `cd dir && git commit …` (the first executable is
not git) exit 0 without a guard being evaluated. The control is the proof — a
subject the bare form blocks with CONVENTIONAL_COMMITS_VIOLATION exits 0 in
both chained forms, so the hook never validated them and cannot be
over-blocking them. The guard is unchanged; scoping this scan to $MSG_PREFIX
alone is what reopened the round-4 trailing-flag bypass.

The class was real, though, one shape further out: `FOO=bar; git commit …` IS
classified and then refused, because assignment detection is prefix-anchored
and the tokenizer does not split operators. Pinned as a counterexample and
disclosed rather than generalised away; narrowing it means changing
isGitSubcommand, the shared git-commit detector every gating hook uses, and it
fails closed.

Six accept-direction bypasses are fixed. Each let the hook resolve and ALLOW a
commit whose real subject the rules refuse; the three that turn on git's
recorded subject were confirmed against the RAW COMMIT OBJECT, since
`git log --pretty=%s` strips trailing whitespace and hid two of them:

  --cleanup=whitespace -m <72+spaces> --cleanup=verbatim  git kept 75 bytes
  -mWIP -m <heredoc>                                      git recorded `WIP`
  --mes=WIP -m <heredoc>                                  git recorded `WIP`
  -\m WIP -m <heredoc>                                    git recorded `WIP`
  git commit --amend --no-edit \n echo -m <heredoc>        echo's argument read
  --squash=HEAD -m <heredoc>                              `squash! …`

Causes: one BASH_REMATCH inspected only the FIRST cleanup directive while git
applies the last, so multiplicity now refuses rather than guesses at an
argument order a substring scan cannot recover; the option scan required a
trailing space or `=`, missing attached values and long-option abbreviations;
dequoting removed quotes but not the syntactic backslashes bash also removes;
the separator scan omitted newline; and --squash/--fixup have git compose the
subject, so the supplied message is not the subject at all. Every fix widens
refusal, the direction this file documents as recoverable.

The multiplicity count first broke the hook outright: the script runs under
`set -euo pipefail` and grep exits 1 when it matches nothing, which is the
common case, so every ordinary commit died at exit 1 with no verdict. Guarded,
and only caught because the probe runs the real hook rather than the scan.

Five new rows, all five proven red against the pre-fix hook, each carrying a
non-vacuity assertion that the canonical single-`-m` heredoc still resolves.
Changeset corrected on three counts: "all fail-closed" was wrong (persistent
commit.cleanup fails OPEN, as do the -C/-c/-F/-t message sources), "global
options are all walked through" was too broad, and the chained-before claim
now states what is measured.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018FUAVz49BghqxoJgwt7EW9

* fix(#3802): stop the separator and glue classes matching a literal backslash

Round 8's Major, with two corrections to its account.

`;`, `&` and `|` are metacharacters inside `[[ ]]`, so an inline bracket
class must escape each one. POSIX bracket expressions have no escape
mechanism of their own, so on bash 3.2 -- the system /bin/bash on macOS,
already a supported target here per the `declare -A` ban in
tests/install.test.cjs -- those backslashes reach the regex engine and add
a literal `\` to the class. bash 4+ consumes them, which is why this is
invisible on a modern bash. The hazard is specific to bracket
expressions: `\(` outside one is made literal correctly on every version,
and the subject validator and the `-m` capture classes were checked and
are unaffected.

The prescribed fix is not taken, because it does not parse. Inline
`[;&|]` is a bash SYNTAX ERROR on 3.2 and on 5.3 alike -- the backslashes
exist to get the metacharacters past the `[[ ]]` parser, so removing them
leaves an unparseable script. Each class is held in a variable and
expanded unquoted on the right of `=~` instead, which is a plain regex on
both versions.

One root cause, consequences in BOTH directions. The reported half is the
separator scan over-blocking. The half not reported is the accept
direction, and it is the more serious: the glue class is NEGATED, so on
bash 3.2 a backslash-glued suffix fell inside the exclusion and the hook
RESOLVED a heredoc it should have declined -- measured exit 0 on 3.2
against the unfixed hook, exit 2 everywhere else, with a letter-glued
control refused in all four cells.

The reported repro is not actually fixed by this, and the changeset says
so. A `\`-newline line continuation carries a literal newline, which the
round-7 separator guard refuses on every bash, so that shape stays
blocked with or without this change. Narrowing the newline guard is not
attempted: telling a continuation from a separator by substring scan is
the class that was tried twice in earlier rounds and reverted both times,
and an escaped backslash sitting immediately before a real newline is
indistinguishable from a continuation. Disclosed as a known fail-closed
limit instead.

Every new row runs under each bash on the machine. Against the unfixed
hook both bash 3.2 rows go red while all four bash 5.3 rows stay green --
written the ordinary way these rows would run under PATH bash, pass
against the broken hook, and prove nothing. Two non-vacuity controls per
interpreter prove the validator is reached rather than passing
everything. All 8 rows of the established differential harness are
byte-identical before and after on both versions: no regression, no new
refusal.

* fix(#3802): remove the $ of a dollar-quote from the option-name scans

Independent round-8 review, accept direction.

The option-name windows are dequoted so they match "the command as bash
hands it to git" -- round 6 removed quote characters, round 7 removed
syntactic backslashes. Both passes missed that bash has two further
quoting forms whose introducer is a `$`: `$'...'` and `$"..."`. Removing
the quote characters alone left that `$` stranded INSIDE the option name,
so `-$"m"` dequoted to `-$m` and matched no literal, while bash passed a
real `-m` to git.

Measured on bash 3.2.57 and 5.3.15 against a real repository: the hook
allowed

    git commit --allow-empty -$"m" WIP -m "$(cat <<'EOF'
    fix: a perfectly ordinary conforming subject
    EOF
    )"

with exit 0, and `git cat-file -p HEAD` recorded the subject `WIP`.

The comparison that establishes this is HEAD-internal, not a differential:
the same command spelled `-m WIP` is refused (exit 2). The merge-base
refuses EVERY heredoc form, including a perfectly conforming one, so its
exit 2 on this input says nothing about whether any guard fired -- it is
the absence of the feature, not a working check. The same miss covered
`$'m'`, spliced `--message`, `--cleanup`, `--squash` and `--fixup`.

An option NAME finished by a command substitution -- `--clean$(printf
up)=verbatim` -- is a different problem and gets its own guard: bash runs
a program to complete the name, so the argv git receives is not derivable
from this string at all, and resolution is refused rather than guessed.
The guard is scoped to the NAME: the class is a `-`-leading token whose
characters up to the substitution contain no `=`. A substitution
supplying a VALUE -- the ordinary `--author="$(git config user.name)"`,
spaced or glued, in either window -- is untouched and still resolves,
pinned in both directions. It is a SHAPE, not a segmentation of the
command line; segmenting was tried twice in earlier rounds and reverted
both times, and that reasoning stands.

Both new rows fail against the unfixed tree with their own assertions,
proven in a complete worktree at the previous head rather than a hook
copied out of its tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): recognise a canonical cat, not any absolute path ending in /cat

Independent round-8 review, accept direction.

Round 4 restricted heredoc-opener recognition to an absolute path, after a
relative `./cat` was measured being trusted to echo its stdin. It stopped
at "absolute", so any absolute path ENDING in `/cat` was still trusted --
the same claim the round-4 reasoning had rejected one spelling earlier.

Measured on bash 3.2.57 and 5.3.15 against a real commit: with an
executable at `/.../fake-cat/cat` printing `WIP injected`, the hook
validated the conforming heredoc body and allowed the commit (exit 0)
while `git cat-file -p HEAD` recorded the subject `WIP injected`. The
same command through `./cat` was already refused, which is the control
that shows this is the round-4 class one spelling out rather than a new
one.

Recognition is now the canonical system locations -- bare `cat`,
`/bin/cat`, `/usr/bin/cat` -- which is the only identity claim a string
can support. `/usr/local/bin` is deliberately excluded: it is
user-writable on ordinary machines, which is the plantable case this
guard exists for. Anything else falls back to the opener line and the
format gate: fail closed, exactly the pre-fix behaviour for the form.

The pre-existing residual is unchanged and still documented: a bare `cat`
shadowed earlier on PATH is indistinguishable here, and is not a
meaningful boundary -- anyone able to plant an executable on PATH can run
`git commit` directly. This hook stays an authoring guard, not a security
control.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): an option name carrying a shell expansion is unresolvable

Independent review, round 9, accept direction. Four more spellings, and a
change of strategy that is the actual point of this commit.

Rounds 6, 7 and 8 each tried to EMULATE what bash does to an argument
before git sees it -- round 6 removed quote characters, round 7 syntactic
backslashes, round 8 the `$` that introduces a dollar-quote -- and each
round review found another transform that had been missed. Round 9 found
four more. All measured on bash 3.2.57 and 5.3.15 against a real
repository, each with the plain spelling of the same command as its
control (refused, exit 2) and `git cat-file -p HEAD` for the subject git
actually recorded:

    -$'\155' WIP        hook 0, real subject `WIP`   ANSI-C octal -> m
    -$'\x6d' WIP        hook 0, real subject `WIP`   ANSI-C hex   -> m
    -`printf m` WIP     hook 0, real subject `WIP`   backtick substitution
    x= … -${x}m WIP     hook 0, real subject `WIP`   parameter expansion
    -? WIP              hook 0, real subject `WIP`   pathname expansion

and the same class through the cleanup guard, where git recorded a
75-character subject the length gate had measured as 72:

    --cle$'\141'nup=verbatim, --clean`printf up`=verbatim, --cle?nup=verbatim

The last two settle it. An option name finished by a PARAMETER expansion
depends on a variable's value at run time; one finished by a PATHNAME
expansion depends on the contents of the working directory. Neither is
derivable from the command string at any level of effort, so emulation
cannot be completed -- not "has not been completed yet". A fifth patch in
that direction would have the same shape as the previous four.

The rule is therefore no longer "normalise it and match the literal". It
is: an option NAME carrying a shell expansion or quoting construct is
UNRESOLVABLE, and unresolvable refuses. One rule covers every spelling
above and every spelling nobody has thought of yet, in the fail-closed
direction. The dequoting passes are kept rather than replaced: they still
normalise the deterministic removals, so the guards RECOGNISE
`--clean""up=` and `-\m` as the options they are instead of merely
refusing them, which keeps the existing rows meaningful.

Scope is unchanged and still pinned in both directions: the class is a
`-`-leading token whose characters up to the construct contain no `=`, so
a construct supplying a VALUE -- `--author="$(git config user.name)"`,
the backtick spelling, `--date="${NOW}"`, a glob character inside an
author string, a pathspec after `--` -- still resolves. Nine such forms
are asserted to pass beside the seven that must refuse.

The class is bracket-only and holds no backslash, per round 8: a POSIX
bracket expression has no escape mechanism, and a backslash written
inside one becomes a literal member on bash 3.2.

The new rows fail against the previous head with their own assertion
message, in a complete worktree with the lib built, not a copied hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* docs(#3802): disclose and pin the two spellings the round-9 class over-blocks

A scoped review of the round-9 class asked one question -- does it refuse a
conforming heredoc commit that the previous head accepted -- and found two
spellings that it does. Both measured on bash 3.2.57 and 5.3.15, previous
head 518d97b64 exit 0, current head exit 2:

    git commit -S$SIGNING_KEY -m <conforming heredoc>
    git commit -m <conforming heredoc> -- -*.txt

Disclosed and pinned rather than narrowed, for two reasons.

Narrowing is not available cheaply. Dropping the bare `$` member reopens
`-$xm`: with `xm=m` bash hands git a real `-m`, which is the parameter
expansion bypass the round-9 commit exists to close. Skipping tokens after
`--` means deciding where git's options end from a substring scan, which
is the class this file has already reverted twice for opening
accept-direction holes -- a `--` inside a quoted value (`--author "a -- b"`)
would truncate the window and hide a real trailing directive.

And the limits are narrower than they look, because in both cases the
spelling a developer actually reaches for still resolves:

    -S "$KEY"  and  --gpg-sign="$KEY"        resolve
    '-*.txt', "-*.txt", ':(exclude)-*.txt'   resolve

The pathspec one is worth stating precisely: a glob only reaches git AS a
pathspec when it is quoted, because an unquoted one is expanded by the
shell before git is executed. So the refused spelling is not passing a
glob to git at all, and the spellings that do are unaffected.

Refusing a commit git would take is the recoverable direction; accepting a
non-conforming subject is not. That is the trade this file already makes
everywhere else, and it is made explicitly here.

Nine rows pin the working spellings beside the three that refuse, so a
later narrowing cannot silently drop the cases that must keep working.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): join backslash-newline continuations before the resolve guards

Round 9's Major, with a correction to its diagnosis.

The cited bracket classes at :223 and :260 no longer exist -- round 8
moved both into SEP_CLASS and GLUE_CLASS, and a lone backslash before -m
resolves (exit 0) at the reviewed head on both bash 3.2.57 and 5.3.15.
What refuses the repro is the NEWLINE a `\`-continuation carries: round
7's separator guard reads any newline in a window as a command boundary,
and `git commit \` newline `  -m "$(cat <<'EOF' …` was refused for that
reason. Round 8 disclosed it as a fail-closed limit; round 9 calls the
idiom common and the limit a Major, and it is fixed here.

It was left as a limit because "is this newline a continuation" looked
like the segmentation question this file has reverted twice. It is not:
bash's rule is local and character-level. A newline preceded by an ODD
run of backslashes is a continuation and bash removes both; an EVEN run
(`\\` then newline) is a literal backslash followed by a real newline,
which IS a separator. Both scan windows are joined that way immediately
after they are cut from the command and before any dequote copy is
derived, in three bash-3.2-safe parameter expansions: every `\\` pair is
parked on \x01, any backslash-newline that remains is a lone one and is
removed, then the pairs are restored.

Measured on both bashes, both directions:

    git commit \<nl>  -m <heredoc>                 2 -> 0   the fix
    git commit \\<nl>  -m <heredoc>                2 -> 2   literal \ + real separator
    git commit<nl>  -m <heredoc>                   2 -> 2   bare newline
    -m <heredoc>\<nl>suffix                        2 -> 2   bash glues it; the glue guard sees it glued
    git commit … \<nl>  --allow-empty<nl>echo -m … 2 -> 2   the REAL newline still separates

The prescribed `[\;&|]` is not taken: a backslash written inside a
bracket expression becomes a literal member on bash 3.2, which is the
round-8 defect from the other side.

Rows run under each bash on the machine. The fix row fails against the
previous head in a complete worktree with the lib built; the four control
rows were measured against that same head and were already refused, so
they pin existing behaviour rather than the change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
 2026-09-05 03:51:43 +00:00
Tom Boucher
2ea5efc151 enhance(#3911): hooks declare their crash policy (#3960) 
* enhance(#3911): give hooks an exit seam that needs no build

ADR-3889 Phase 7 foundation. The 19 shipped enforcement hooks hold 91 of the
epic's 128 terminators and cannot reach `terminateNow` today.

The obvious route — requiring `gsd-core/bin/lib/cli-exit.cjs`, as
gsd-agent-isolation-guard.js already does for two other modules — is rejected.
That precedent carries its own warning (#3582): those files are tsc output,
gitignored and absent on a raw plugin-marketplace or git-clone install, so the
hook must first call ensureRuntimeBuild() to self-heal. Making the module a
hook needs IN ORDER TO TERMINATE depend on a build inverts the dependency, and
its failure mode is precisely the fail-open this phase exists to remove: a
guard that cannot terminate cannot deny. `lint-hooks-runtime-build-seam`
already encodes that concern, and Design B would have had to add an
ensureRuntimeBuild() call to all 19 hooks to satisfy it.

So `hooks/lib/` becomes a third emit location for cli-exit and a fifth for the
registry, preserving the invariant `src/cli-exit.cts`'s own header states: it
imports nothing but node:fs and its sibling registry, and the generator
dual-emits that sibling alongside each copy so a relative require resolves next
to whichever copy loaded it. Shipping needed no change — build-hooks.js already
declares HOOKS_SUBDIRS_TO_COPY = ['lib'].

Proven, not asserted: the two files are copied into an otherwise-empty tmpdir
and a child process requires them and terminates — PASS exits 0, HOOK_DENY
exits 2 with the payload on both stdout and stderr. That test fails the moment
the hooks copy gains a require reaching outside hooks/lib/.

Also fixed inline: the registry's fifth target let any `--write` test overwrite
the real committed hooks/lib/exit-code-registry.js, because the test helper
derived only three of the other output paths. It now redirects all five, and a
regression test asserts every committed artifact is byte-identical after a
redirected write.

Install-tree goldens pick up the two new shipped paths across 11 runtimes —
insertions only, no removals. lint:ci was green while they were stale, so this
was found by regenerating rather than by a gate.

Verification runs on the remote runner.

Refs #3911

* enhance(#3911): declare a crash policy, and migrate the write guard

Adds `hooks/lib/hook-exit.js` — the hook-facing vocabulary over `terminateNow`,
hand-written because the cli-exit copy beside it is generated:

  allow(payload)          exit 0
  deny(payload, stderr?)  exit 2
  crash(onCrash, payload) whichever the hook DECLARED

`crash()` takes the policy as a required argument with no default, which is the
whole mechanism: fail-open by accident stops being expressible. A hook must
name ALLOW or DENY at the call site, and an unrecognized value terminates
INTERNAL rather than guessing. Fail-open stays legal; fail-open by omission
does not.

`gsd-write-guard.js` is the first hook migrated, all 12 sites, and it exposed a
gap in the seam. `terminateNow`'s doc comment justified its fd-2 write by
citing this hook's `emitBlock` — but modeled it as sending the same bytes to
both streams, when `emitBlock` actually sends full JSON to stdout and only the
bare `reason` string to stderr, because Kimi's hook bus feeds stderr verbatim
back to the model. Migrating as written would have turned a readable sentence
into a JSON blob for Kimi-backed agents.

#3911 requires both "all 19 hooks terminate through terminateNow" and "no
hook's effective default changes". Those are jointly satisfiable only by
teaching the seam to carry a distinct stderr payload, so `terminateNow` gains
an optional third argument: omitted, behavior is byte-for-byte what it was; a
string is written raw, which is exactly the Kimi case. The doc comment's
inaccurate claim about emitBlock is corrected in place.

Proven rather than asserted: the pre-migration file is reconstructed from HEAD
and driven with the same catastrophic-shrink payload as the migrated one —
exit code, stdout and stderr all byte-identical.

Verification runs on the remote runner.

Refs #3911

* enhance(#3911): all 19 hooks terminate through the seam

Migrates the remaining 18 enforcement hooks onto allow/deny/crash. An AST walk
now reports zero `process.exit(` call sites across every `hooks/*.js` — down
from the 91 the census measured.

Each hook with an outer catch declares its policy once, at module top, with the
reason that policy is right for that specific guard: a read guard that cannot
scan must not block the read; a statusline that renders every prompt must
degrade rather than crash; an injection scanner must not retroactively block a
result already returned. Those sentences are the deliverable — they are what
turns fail-open-by-accident into fail-open-on-purpose. No hook's effective
default changed.

Wiring exposed two defects, both fixed here rather than noted.

A SECOND stdout/stderr-splitting site turned up in `gsd-workflow-guard.js`'s
`emitForceAddBlock`, matching the pattern already known from the write guard —
full JSON to stdout, bare reason to stderr for the Kimi bus. It uses the
`stderrPayload` argument added in the previous commit, which is now carrying
its second real caller rather than one special case.

More seriously, `terminateNow` emitted both streams inside ONE try, so a
payload that failed to serialize aborted before the stderr write ever ran. The
two windsurf guards write nothing to stdout on a block and only a reason string
to stderr, so `deny(undefined, reason)` exited 2 with EMPTY stderr — a deny
that silently loses its reason, which is the exact "fails with success" class
this epic exists to close. The streams are now emitted independently, each with
its own guard, and `undefined` means "nothing to write for this stream" rather
than an error. Regression tests inject a throwing write on one fd and assert
the other still receives its payload; they fail against the single-try version.

Byte-identity was proven per hook, not assumed: each pre-change file is
reconstructed from HEAD and driven side by side with the migrated one across
its normal path, its deny path, malformed stdin and empty stdin — exit code,
stdout and stderr compared.

Verification runs on the remote runner.

Refs #3911

* enhance(#3911): harden the three shell hooks, and pin every hook's policy

`gsd-phase-boundary.sh`, `gsd-session-state.sh` and `gsd-validate-commit.sh`
gain `set -euo pipefail`.

The expected hazard did not materialize, and that is worth recording: every
intentionally-non-zero command in all three is already the condition of an
`if`/`elif`, which `set -e` never fires on, and none of them reads a
possibly-unset variable or pipes through a grep that may legitimately match
nothing. No `|| true` guards were needed. Each hook was still checked
command-by-command before the flags went in rather than after.

Twenty-one before/after cases across the three hooks — disabled and enabled,
planning and non-planning, missing STATE.md, malformed JSON, the Kimi payload
shape, quoted and unquoted `-m`, valid and over-long Conventional Commits —
all match on exit code, stdout and stderr.

The hardening is shown to actually fire, not merely added: with a stubbed
`node` that fails at the JSON-emit step, phase-boundary and session-state go
from silently exiting 0 with empty stdout to failing visibly with the error
surfaced. No such case could be constructed for `gsd-validate-commit.sh`,
whose every statement already sits inside an if-condition — recorded as
unproven rather than claimed.

`tests/hooks-crash-policy.test.cjs` adds the per-hook coverage the issue asks
for, table-driven over all 19 hooks rather than 76 hand-written cases: normal
allow, deny where a deny path exists, crash-honors-the-declared-policy, and an
unclosed-stdin case — the one `process.exitCode` structurally cannot serve. The
deny assertions encode each hook's ACTUAL stream split rather than a uniform
shape, since four of the six deliberately differ. A drift guard enumerates
`hooks/*.js` and fails if a terminating hook is ever added without a row.

Writing those tests surfaced two hooks that emit a block decision in their JSON
body and exit 0. Both were checked rather than assumed, and neither is a
fails-with-success: `gsd-read-injection-scanner.js` is PostToolUse, where the
tool has already run and exit 2 has no meaning, and `gsd-cursor-subagent-start.js`
follows Cursor's JSON-body protocol. They are deliberately left alone — a
mechanical sweep to `deny()` would have broken exactly these two.

Verification runs on the remote runner.

Refs #3911

* fix(#3838): the commit validator says when it could not validate

#3911 claims to subsume #3838. Measurement said otherwise, so this closes it
for real rather than by assertion.

`set -euo pipefail`, added earlier on this branch, does NOT fix #3838: bash
exempts a command used as an `if` condition from `set -e`, and all three of the
hook's swallow-and-pass sites are exactly that shape. Verified against the
hardened hook with a node shim that fails only the classifier call — a
non-conforming commit still exited 0 with empty stdout AND empty stderr,
indistinguishable from "your commit conforms". That is the defect verbatim.

All three sites named in #3838 now capture the real exit status instead of
consuming it as a condition, and each distinguishes its genuine negative from
"could not run":

- the classifier: 0 = is a git commit, 1 = genuinely not one, anything else =
  could not classify. Its `node -e` now wraps the require and the call in
  try/catch and exits 3 on a throw, so a broken require chain can never be
  mistaken for `isGitSubcommand` legitimately returning false — which is the
  arm that matters, since `token-scanner.cjs` is a gitignored build artifact
  and a fresh checkout lands there.
- the opt-in config read and the JSON command extraction get the same
  treatment.

On "could not run" the hook emits a diagnostic to stderr naming which check
failed and why, then exits 0. The issue confirms this is safe — it is a
PreToolUse hook, so stderr does not disturb the JSON protocol — and ranks it
the smallest sufficient fix. The gate still fails open, but it can no longer do
so silently, which is the whole complaint: a validator that disables itself
quietly costs more than one that is absent, because it is trusted.

Both controls are unchanged and pinned by tests: a conforming commit still
passes silently, a non-conforming one still exits 2 with its existing block
payload. The defect test asserts stderr is non-empty and names the failure; it
fails against the pre-fix hook.

Verification runs on the remote runner.

Refs #3911, #3838

* docs(#3911): document the hook crash-policy contract

Reference and Explanation via a new docs/features fragment (FEATURES.md is
generated from it), INVENTORY rows for the three new hooks/lib files, and an
ARCHITECTURE note on the hooks section.

How-To: docs/how-to/declare-a-hook-crash-policy.md, indexed from docs/README.md
— a hook author now has to choose and declare a crash policy, which is more
than one step and crosses into which harness protocol their hook speaks. It
covers allow/deny/crash, writing an ON_CRASH reason that is actually useful,
when a deny needs a distinct stderr payload, the two hooks whose harness reads
a JSON-body decision and must NOT use deny(), and what to do when a check
cannot run at all — with #3838 as the worked example.

Refs #3911

* test(#3911): prove the seam actually ships, and stop hand-rolling temp cleanup

Two review findings.

The acceptance criterion 'hooks/dist/** stays in parity via the build seam
(lint:hooks-runtime-build-seam)' was misstated and unmet: that lint checks
something else — that a hook requiring a compiled gsd-core/bin/lib module also
calls ensureRuntimeBuild(). Nothing exercised that the three new hooks/lib
files reach hooks/dist/lib at all. That gap is not theoretical: #770 is a
recorded ship-blocking bug where a new hook never shipped because a copy list
missed it. The suite now builds dist through the repo's own ensureBuiltHooks(),
byte-compares each shipped copy against its source, and spawns a child that
requires the SHIPPED dist copy and denies — which is what catches a copy that
exists but cannot resolve its sibling registry.

gsd-validate-commit.sh hand-duplicated mktemp/run/rm three times; one idempotent
trap on EXIT replaces them, guarded so cleanup cannot alter the exit status.
Behavior-neutral across five cases, with temp-file counts taken before and
after each run.

Refs #3911

* fix(#3911): stage transitive hook lib requires, not just one level

The remote run returned 7 failures across 3 real causes.

The important one is a PRODUCTION bug this phase exposed rather than caused.
`writeCursorHooksJson` scanned each hook script for `./lib/X` requires exactly
one level deep and never re-scanned the lib files it staged for their own
sibling requires. Nothing had a transitive lib dependency before, so the gap
was invisible. Adding hook-exit.js -> cli-exit.js -> exit-code-registry.js
made real Cursor installs ship a bundle that dies at require time with
MODULE_NOT_FOUND. It now walks to a fixed point, and a real installed Cursor
hook runs to completion.

The staging harness in shared-hooks-dir-resolution hand-copied its fixture, so
the injection scanner crashed at require time and its exit-1 was being read as
a policy decision. Migrated to copyScriptWithDeps, which walks the require
graph — the repo's recorded rule for this class, since adding another
copyFileSync keeps it alive for the next person.

The missing-lib-source test in cursor-hook-workspace-roots hardcoded which lib
file it expected to be named in the abort message; the same throw now fires for
a different file first. Its assertion is unchanged in substance — staging still
must abort rather than ship a broken hook — only the name is no longer pinned.

The last one was my own test asserting an uppercase reason code. Measured
against origin/next: the pre-change hook emits the same lowercase
'config_unreadable', so the test was wrong, not the migration. Corrected to the
real value rather than making the code match the test.

Verification runs on the remote runner.

Refs #3911

* chore(#3911): regenerate the cursor install-tree golden

The staging fix means a Cursor install now correctly carries the two
transitive lib files it was silently missing. Additive only — no path was
removed. The golden diff is the evidence the packaging defect was real.

Refs #3911

* chore(#3911): backfill the changeset PR number

Refs #3911

* fix(#3911): a git probe that timed out is not a negative

A macOS CI lane failed three deny cases at 2084ms, 2112ms and 2177ms — just
past the 2000ms budget these hooks give their git probes. The three that passed
took 72ms, 595ms and 651ms. Under shard contention `git rev-parse` overruns,
the hook reads the non-zero result as "not a git repo", and allows with exit 0
and empty stdout AND empty stderr. Under load, the guards silently stop
guarding. That is ADR-3889's thesis exactly, sitting inside the security hooks
this phase is about.

The repo had already recognized the class in one place — gsd-cursor-subagent-start.js
fail-closed-denies on `git_timed_out` (#3045) — but nowhere else.

`hooks/lib/git-probe.js` classifies a probe's outcome, distinguishing a real
non-zero exit from ETIMEDOUT, a signal kill, and a spawn failure, rather than
folding all four into `status !== 0`. Three guards route their eight git probes
through it.

The resolution is the same shape #3838 took, and the same one that issue
endorsed as smallest-sufficient: fail open, but loudly. **No exit code changes
on any path** — a developer on a loaded machine is still not blocked, which
keeps #3911's declaration-pass contract intact for exit codes. What changes is
that the hook now says on stderr which probe could not answer, instead of
presenting silence as a clean verdict.

Scope was checked across every hooks/*.js, not just the three that failed:
gsd-agent-isolation-guard spawns no git; gsd-statusline's two probes gate only
a cosmetic display segment, not an allow/deny decision, and are left alone.

The C2 deny assertion was a real-race test — it demanded exit 2 while a slow
git legitimately yields 0. It now requires the hook to either deny, or allow
with a diagnostic naming the probe that could not run; a silent allow still
fails, so the assertion is not vacuous. A deterministic regression stubs git on
PATH to sleep past the budget rather than waiting for load to reproduce it.

Verification runs on the remote runner.

Refs #3911

* test(#3911): a PATH shim cannot intercept the hooks' git spawn on Windows

The deterministic timeout regression stubbed git on PATH and asserted the
guard reports rather than silently allows. It passes on Linux and macOS and
failed on Windows in 83ms and 176ms — the stub was never invoked at all.

Mechanism: the hooks call spawnSync('git', args) with no shell:true, so on
Windows CreateProcess resolves git.exe only and never a PATH .cmd shim. The
git.cmd branch could not have worked and is removed rather than left implying
a Windows path that does. Adding shell:true to the hooks to serve a test would
change product behavior and widen an injection surface, so the case is skipped
on win32 only, with the mechanism written into the skip reason so a future
reader does not 'fix' it that way.

Linux and macOS keep the coverage, and macOS is where the underlying fail-open
was actually caught.

Refs #3911

---------

Co-authored-by: sim <sim@local>
 2026-08-27 22:21:10 -04:00
Otavio Salvador
8ca86b5e24 fix: use #!/usr/bin/env bash in community .sh hooks for distro portability 
The three opt-in bash hooks (gsd-phase-boundary.sh, gsd-session-state.sh,
gsd-validate-commit.sh) shipped with #!/bin/bash, which fails on distros
that don't ship bash at /bin/bash (NixOS, minimal Alpine images, some
container runtimes). POSIX guarantees /bin/sh but not /bin/bash.

This is latent in the default install path because Claude Code wires the
hooks as `bash <path>` from settings.json (PATH-resolved — the script's
own shebang is read as a comment by bash). The fix matters when scripts
are run directly: tests, future installer changes, or manual debugging.

Changes:
- hooks/gsd-{phase-boundary,session-state,validate-commit}.sh: shebang
  switched to #!/usr/bin/env bash, matching the convention already used
  in scripts/*.sh.
- tests/bug-2136-sh-hook-version.test.cjs: assertion updated to expect
  the new shebang; comment updated to spell out the rationale.
- tests/bug-2979-hook-absolute-node.test.cjs: doc-comment updated — the
  prior wording cited "POSIX std PATH always has /bin" as the reason
  bare `bash` is OK. The actual reason is that bare `bash` is
  PATH-resolved, which is portable across distros that don't ship
  /bin/bash. POSIX std PATH guarantees /bin/sh, not /bin/bash.
- bin/install.js::buildHookCommand: comment block clarifying the same.
  No behavior change in this file — bare `bash` was already correct.
- .changeset/portable-bash-shebang-hooks.md: changeset entry.

Verified locally on NixOS:
- npm run build:hooks: hooks/dist/*.sh shebangs propagate correctly.
- node --test tests/bug-2136-*.cjs tests/bug-2979-*.cjs
  tests/bug-1817-*.cjs tests/bug-1834-*.cjs tests/bug-1906-*.cjs
  tests/bug-2557-*.cjs tests/bug-3017-*.cjs tests/security-scan.test.cjs
  tests/hooks-doc-parity.test.cjs: 126/126 pass.
- node scripts/run-tests.cjs (full suite): 6944 pass / 0 fail / 5 skip.
 2026-05-06 15:41:27 -04:00
Tom Boucher
7827e1ddee fix(#3129): replace bypassed bash regex with token-walk git-cmd.js classifier (#3141) 
* fix(#3129): replace bypassed bash regex with token-walk git-cmd.js classifier

Root cause: gsd-validate-commit.sh used:
  if [[ "$CMD" =~ ^git[[:space:]]+commit ]]
This regex silently bypasses Conventional Commits enforcement for:
  git -C /path commit -m ...     (working-directory prefix)
  GIT_AUTHOR_NAME=x git commit   (env-var prefix)
  /usr/bin/git commit -m ...     (full-path executable)

Fix: introduces hooks/lib/git-cmd.js with isGitSubcommand(cmd, sub) —
a token-walk classifier that handles all four forms by:
  1. Skipping leading VAR=VALUE env assignments
  2. Validating the git executable (basename check for full-path support)
  3. Consuming git global options (-C <path>, --git-dir=, -p, etc.)
  4. Checking the subcommand token

The hook delegates to this classifier via node shell-out. node is
already called twice in this hook (config check + JSON parse), so no
new runtime dependency.

This becomes the single source of truth for all hooks that gate on
git subcommands (pre-commit-review-gate, post-push-verify, etc.).

Regression test: 27 assertions — tokenize correctness, 12 must-match
cases (including all 3 bypass forms), 8 must-not-match cases, 3 source
checks. All are real behavioral tests, not string comparisons.
Suite: 7035/7035. Closes #3129.

* fix(lint+hook+changeset): allow-test-rule, fix HOOK_DIR quote injection, fix changeset pr+typo
 2026-05-05 15:02:15 -04:00
Tom Boucher
f55069ecbf test(#2974): migrate 8 test files to typed-IR assertions (#3016) 
* test(#2974): migrate 8 test files to typed-IR assertions

Replaces raw stdout/stderr substring matching with structured-field
assertions per CONTRIBUTING.md "Prohibited: Raw Text Matching on Test
Outputs". Adds shared infrastructure for typed error emission so this
pattern is the easy path going forward.

Shared infrastructure:
- core.cjs: ERROR_REASON frozen enum + setJsonErrorMode/getJsonErrorMode
- gsd-tools.cjs: --json-errors CLI flag, parsed before subcommand dispatch
- config.cjs: typed reasons at all 7 error sites
- graphify.cjs: GRAPHIFY_REASON enum + reason/timeout_ms in execGraphify result
- bin/install.js: pure buildSdkFailFastReport() IR builder + renderer
- hooks/gsd-session-state.sh, gsd-phase-boundary.sh: emit Claude Code
  hookSpecificOutput JSON envelope with typed state_present/config_mode/
  planning_modified/file_path fields (no-op when hooks.community is off)

Test migrations (all pass, 171 tests across the 8 files):
- bug-2649-sdk-fail-fast: assert on ir.reason / ir.context / ir.fix_command
- bug-2687-config-read-warning-parity: assert.equal stderr === ''
- bug-2796-arg-parsing-regression: assert on result.json.updated/.phase
- bug-2838-summary-rescue: parse rescue footer, assert mtime invariant
- bug-2943-config-get-context-window: parse JSON, assert ERROR_REASON.CONFIG_KEY_NOT_FOUND
- graphify: assert reason === GRAPHIFY_REASON.ENOENT/TIMEOUT
- hooks-opt-in: parse hookSpecificOutput, assert typed fields
- security-scan: reclassified as source-text-is-the-product (scan label
  output and CI workflow YAML ARE the deployed contract)

Verification: lint-no-source-grep clean (0 violations), full suite
6741/6741 pass.

Closes #2974

* test(#2974): address CR feedback — typed code field, robust idempotency

Two CodeRabbit findings on #3016 addressed:

1. tests/hooks-opt-in.test.cjs:355 (Minor, inline) —
   parsed.reason.includes('Conventional Commits') was still substring
   matching after the typed-IR migration. Fixed at the source: the
   gsd-validate-commit hook now emits a typed `code` field
   ('CONVENTIONAL_COMMITS_VIOLATION', 'COMMIT_SUBJECT_TOO_LONG')
   alongside the human-readable `reason`. Test asserts strictEqual
   on the code; the prose copy is no longer part of the test contract.

2. tests/bug-2838-summary-rescue-gitignored-planning.test.cjs:224-250
   (Outside-diff) — mtimeMs alone can stay unchanged on coarse-grained
   filesystems (HFS+, FAT) when two rewrites land within the same
   timestamp tick, falsely passing the idempotency assertion.
   Replaced with a full snapshot (mtimeMs, ctimeMs, size, ino, sha256
   of contents) compared via assert.deepStrictEqual — the hash
   catches any rewrite the timestamp would miss.

Verification: 30/30 pass on the two affected files; lint-no-source-grep
clean (0 violations across 368 test files).
 2026-05-02 09:27:23 -04:00
Tom Boucher
50f61bfd9a fix(hooks): complete stale-hooks false-positive fix — stamp .sh version headers + fix detector regex (#2224) 
* fix(hooks): stamp gsd-hook-version in .sh hooks and fix stale detection regex (#2136, #2206)

Three-part fix for the persistent "⚠ stale hooks — run /gsd-update" false
positive that appeared on every session after a fresh install.

Root cause: the stale-hook detector (gsd-check-update.js) could only match
the JS comment syntax // in its version regex — never the bash # syntax used
in .sh hooks. And the bash hooks had no version header at all, so they always
landed in the "unknown / stale" branch regardless.

Neither partial fix (PR #2207 regex only, PR #2215 install stamping only) was
sufficient alone:
  - Regex fix without install stamping: hooks install with literal
    "{{GSD_VERSION}}", the {{-guard silently skips them, bash hook staleness
    permanently undetectable after future updates.
  - Install stamping without regex fix: hooks are stamped correctly with
    "# gsd-hook-version: 1.36.0" but the detector's // regex can't read it;
    still falls to the unknown/stale branch on every session.

Fix:
  1. Add "# gsd-hook-version: {{GSD_VERSION}}" header to
     gsd-phase-boundary.sh, gsd-session-state.sh, gsd-validate-commit.sh
  2. Extend install.js (both bundled and Codex paths) to substitute
     {{GSD_VERSION}} in .sh files at install time (same as .js hooks)
  3. Extend gsd-check-update.js versionMatch regex to handle bash "#"
     comment syntax: /(?:\/\/|#) gsd-hook-version:\s*(.+)/

Tests: 11 new assertions across 5 describe blocks covering all three fix
parts independently plus an E2E install+detect round-trip. 3885/3885 pass.

Approach credit: PR #2207 (j2h4u / Maxim Brashenko) for the regex fix;
PR #2215 (nitsan2dots) for the install.js substitution approach.

Closes #2136, #2206, #2209, #2210, #2212

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(hooks): extract check-update worker to dedicated file, eliminating template-literal regex escaping

Move stale-hook detection logic from inline `node -e '<template literal>'` subprocess
to a standalone gsd-check-update-worker.js. Benefits:
- Regex is plain JS with no double-escaping (root cause of the (?:\\/\\/|#) confusion)
- Worker is independently testable and can be read directly by tests
- Uses execFileSync (array args) to satisfy security hook that blocks execSync
- MANAGED_HOOKS now includes gsd-check-update-worker.js itself

Update tests to read worker file instead of main hook for regex/configDir assertions.
All 3886 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-04-14 17:57:38 -04:00
Tibsfox
4157c7f20a feat(hooks): add opt-in community hooks for GSD projects 
Port 3 community hooks from gsd-skill-creator, gated behind hooks.community config flag. All hooks are registered on install but are no-ops unless the project config has hooks: { community: true }.

gsd-session-state.sh (SessionStart): outputs STATE.md head for orientation. gsd-validate-commit.sh (PreToolUse/Bash): blocks non-Conventional-Commits messages. gsd-phase-boundary.sh (PostToolUse/Write|Edit): warns when .planning/ files are modified.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-02 04:23:44 -07:00