perf(context-guard): cut the zone-crossing fire from 8 process creations to 3 - #3779
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Independent review, and the corrections it produced — flipped to readyPosted by the autonomous worker lane ( A fresh-context reviewer audited the change with the producer's rationale withheld. The code was confirmed correct and is untouched by the follow-up commit — the corrections were all to the surrounding claims, which had drifted from the measurements. Confirmed by independent reproduction
Four claims corrected
Gates at baseline or better: Generated by Claude Code |
|
Claude finished @kyle-sexton's task in 4m 18s —— View job Code review
Reviewed the three redirection-placement rewrites ( One finding posted inline on Variable-shadowing hazard in the new Everything else checked out:
No blocking findings. The one inline note is a maintainability/latent-bug flag on new shared-library code, not something that affects this PR's behavior today. |
|
Claude finished @kyle-sexton's task in 3m 4s —— View job Security review (
|
|
Last security-reviewed head: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Lane Generated by Claude Code |
|
Lane Three conflicts, all from
The
Verification: No merge was attempted, and this is not only the environment's doing. This PR's own body states that #3520's first acceptance criterion is not met: the branch reaches 4 Separately, no lane instance could merge this anyway: GraphQL here serves only a pinned subset of PR-review operations, Generated by Claude Code |
|
Lane The conflictOne file, Main's
All four Why I stopped thereThis PR's own body declares that #3520's first acceptance criterion is not met: it asks for no more than 2 external process creations per fire, and this branch reaches 4. The body is explicit that the remaining 4 -> 2 work is blocked on #3740, and it offers the operator a choice between accepting a partial close and splitting the remainder into a follow-up. I am not overriding that. Resolving a changelog collision does not close an acceptance criterion, and an author who has disclosed an incompleteness this precisely should not have a lane quietly merge past it. The body is also candid on two related points that survive this merge unchanged: the So the decision this PR needs is unchanged and belongs to an operator:
Worth noting for that decision: #3740 is itself escalated and not merging — it carries 55 conflicts and a 17-plugin changelog fan-out against current Lane action: advanced (conflict cleared, branch current with Generated by Claude Code |
…destination printf -v "$__cg_dest" resolves the destination name against this function's own scope, so a caller passing `input` or `chunk` got its variable left unset while the function still returned 0 — success with no value and no error. No call site hits it today: zone-crossing-inject.sh passes INPUT and the printing wrapper passes __cg_buf. But the header comment invites new callers to adopt the _to form, and `input`/`chunk` are the names such a caller reaches for first, so the hazard sat in front of the next caller rather than behind this one. The internal locals become __cg_input / __cg_chunk, the prefix convention lib/hook-utils.sh already uses for __hu_. A prefix reserves a namespace rather than abolishing the hazard, so the three names that remain internal are now refused loudly with rc 2 and a stderr line instead of failing silently. __cg_buf stays usable because the wrapper passes it. zone-crossing-inject.test.sh pins both halves: five caller-chosen destination names fill correctly, three reserved names are refused with rc 2, and the wrapper still returns the payload. Verified discriminating: the new cases fail five ways against the pre-fix reader, including rc=0 with empty stderr on the reserved names, which is the exact silent-success mode reported. Reported by an automated review on PR #3779. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
…ners (#3878) <!-- CURSOR_AGENT_PR_BODY_BEGIN --> No linked issue ## Summary Drop leftover process creations on the hottest Bash paths in this marketplace: the shared hook library every always-on hook sources, the always-on formatter Write/Edit paths (typos, ruff, biome, bash, powershell, go, actionlint, eol, markdown), the always-on desktop-notification Notification path, always-on guardrails verifiers, and CI scanners that used to spawn once per file, per plugin, or per allowlist entry. ## Fix GNU Bash runs command substitution in a subshell even for builtins (Command Substitution, [Bash Reference Manual](https://www.gnu.org/software/bash/manual/html_node/Command-Execution-Environment.html); [Greg's Wiki](https://mywiki.wooledge.org/CommandSubstitution)). Cygwin's `fork` is a non-copy-on-write Win32 `CreateProcess` ([Cygwin User's Guide, Process Creation](https://ftp.cygwin.com/cygwin-ug-net/highlights.html)): "fork will almost certainly always be inefficient under Win32." ### Shared hook library (`lib/hook-utils.sh`, synced to 17 carriers, patch bump) Same `_to` / in-process pattern as #3838, #3732, and #3678: - `hook::json_escape_to` deletes residual C0 bytes with parameter expansion instead of `printf | tr -d` - `hook::emit_channels` writes through `_to` instead of `$(hook::json_escape …)` - Fractional `read -t` landed in bash-4.0-alpha (CHANGES). `hook::read_supports_fractional_timeout` is `BASH_VERSINFO`; no TMPDIR probe file - `hook::notice_once` reads the marker with `read`, creates the directory only when missing, and prunes stale markers once per process - `hook::bash_parse_segments` walks `${cmd:i:1}` instead of `read -N1` from a process substitution, and `$'…'` bodies decode through `ansi_c_decode_to` (`printf -v`) - `hook::repo_root_to` / `hook::repo_relative_path_to` write in this shell so callers skip a leftover capture around git or builtins-only work Isolation `$(source …)` forks are unchanged (#3685). ### typos-format (always-on Write|Edit|NotebookEdit) - Basename via `${FILE##*/}` (plus a backslash trim), not `basename(1)` - `repo_root_to` / `repo_relative_path_to` instead of capture subshells - Directory existence check instead of `$(cd && pwd)` - `command -v typos` is no longer captured; the later exec looks the name up on PATH ### Remaining always-on formatters (ruff, biome, bash, powershell, go, actionlint, eol, markdown) Same leftover class as typos-format, now applied to every always-on formatter that still captured `_to` helpers or spawned `basename` / leftover `cd && pwd`: - `FILE_BASE` is `${FILE##*/}` (and a backslash trim) - `repo_root_to` / `repo_relative_path_to` write in-process - `$(cd && pwd)` canonicalize is an existence check on the path git already answered (ruff, biome, bash-format EditorConfig walk) - Nested `$(normalize_path "$(physical_path …)")` in powershell-format uses the `_to` forms - `command -v ruff|biome|goimports` is no longer captured - markdown-format keeps physical `pwd -P` containment and config discovery; leftover helper-capture and membership dirname on the root-resolution path are gone ### desktop-notification (always-on Notification) - Field extract fuses into `hook::buffer_stdin_to` so completeness and `.notification_type` / `.message` share one jq process - C0 stripping is parameter expansion, not `printf | tr` - `repo_root_to` writes in-process; OSC 9 / BEL use `printf -v`; `terminalSequence` uses `json_escape_jq_to` - `uname` stays so tests can PATH-stub Darwin; git for `repo_root` stays ### guardrails verifiers (always-on PostToolUse / PreToolUse) - `skill-reference-verify`, `stale-path-verify`, and `cli-flag-verify` call `repo_root_to` / `repo_relative_path_to` in-process - `hardcoded-path-check` and `secret-pattern-detection` use `normalize_path_to` instead of leftover `$(hook::normalize_path)` captures - Isolation `$(source …)` forks are unchanged (#3685) ### CI scanners - Orphaned-fixture scan: one `*.test.*` index, cached `evals.json` `files[]`, in-shell ERE escape. Unquoted `\\` matches one backslash (a quoted `'\\'` arm is two chars and leaves `\b` as a word boundary) - Purged-em-dash scan: one `git ls-files -z` with every `:(glob)` pathspec; in-process component-wise attribution so `*` cannot cross `/`. `--list` stdout is byte-identical to origin/main - Cross-plugin source drift: one `find plugins` plus one `sha256sum` of 2+ cluster paths. Discover stdout is byte-identical to origin/main - Discriminating-test-skips / silent-skips: one awk per corpus (`FNR` + `FILENAME`; mawk has no `ENDFILE`) - Hook-exec-form: one jq over every `hooks.json` and one over every `plugin.json` (`input_filename` attributes rows). Unreadable `hooks.json` still fails closed via per-file fallback; unreadable manifests are still skipped Hook-specific leftover-fork work already in flight (#3873, #3872, #3871, #3870, #3869, #3851, #3849, #3779, #3880, #3886) is out of scope here. ## Verification Independent census re-derived spawn counts from `84adf87b` vs `cdb93f61` without inheriting implementer figures. Kernel census `strace -f -e trace=clone,clone3,fork,vfork,execve`; counter over duration; 3 identical trials. **always-on formatters** (this revision vs `84adf87b`): | Hook | clones before | clones after | execve before | execve after | |---|---|---|---|---| | ruff-format no-config skip | 14 | 10 | 4 | 4 | | powershell-format no-settings skip | 23 | 16 | 7 | 7 | | bash-format no-EditorConfig (ShellCheck finding) | 17 | 13 | 6 (1 `basename`) | 5 (0 `basename`) | **guardrails** (this revision): | Hook | clones before | clones after | execve | |---|---|---|---| | skill-reference-verify Write, no skill refs | 18 | 17 | 8 unchanged | | secret-pattern-detection clean Write | 10 | 8 | 4 unchanged | Secret-pattern absolute counts are with `CLAUDE_PLUGIN_ROOT` set (Claude Code always sets it). Without that env the leftover `PLUGIN_ROOT=$(cd … && pwd)` fallback adds one clone on both sides (11→9); the drop of 2 is the same. **CI scanners** (successful execve, exclude ENOENT; earlier commits on this PR): | Gate | origin/main or prior HEAD | HEAD | |---|---|---| | purged-em-dashes `--list` | 478 | 9 | | cross-plugin-source-drift `--check` | 181 | 4 | | discriminating-test-skips | 316 (awk 312) | 5 (awk 1) | | silent-skips | 120 (awk 118) | 4 (awk 2) | | hook-exec-form `--check` | 196 execve, jq 96, tr 96, clones 292 | 7 execve, jq 2, tr 0, clones 9 | `--list` / discover stdout for the two listing gates is byte-identical to origin/main. **Local `scripts/affected-tests.sh --run`:** 153 shell suites passed or were skipped; 14 NOT RUN python/mjs ecosystems (exit 3, expected on this runner). No `FAIL`. Including: `lib/hook-utils.test.sh` PASS=323; bash-format PASS=54; eol-normalizer PASS=54; markdown-format PASS=174; powershell-format PASS=17; cli-flag-verify PASS=92; hardcoded-path-check PASS=118; secret-pattern-detection PASS=86; skill-reference-verify PASS=140; stale-path-verify PASS=108. ruff/biome/go/actionlint behavioral cases skipped here (binaries absent); skip-path and source pins still ran. `session-event-log.test.sh` PASS=53 isolated under the fan-out. **CI on `cdb93f61`:** lint, hook-utils, test-linux (0–3), test-windows, changes, ci-status, and managed-files-guard all succeeded. https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676378 https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676488 ## Related Refs #3838, #3732, #3678, #1979, #3488, #2891. Same leftover-fork class as open PRs #3849 / #3851 / #3869 / #3873 / #3872 / #3871 / #3870 / #3779 / #3880 / #3886 (those stay hook-specific). N/A for a dedicated issue. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-fdfdc962-be1b-4c9c-9833-3aec57852330?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-fdfdc962-be1b-4c9c-9833-3aec57852330&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
The Stop hook fires on every turn of every session, gated or not. Its interactive default path (no gate footprint in any settings file) cost 4 process creations and 2 launches (grep, uname); it now costs 1 and 1 (uname), the managed-settings platform primitive. An enabled lane's first unsignaled stop went from 48 creations and 18 launches to 10 and 5, a signaled stop from 44/16 to 9/4, an armed lane's stop from 60/20 to 11/5. Counted with strace -f -e trace=clone,clone3,fork,vfork,execve from a staged install; process counts are the proxy because a spawn is ~1 ms on this Linux host against 180-2,841 ms on the #3508 Windows hosts. The cost was in how the work was written, not what it was: every $( ) capture of a lib helper that is only parameter expansion has a _to <var> form; the five per-field printf | jq | tr payload reads are one hook::jq_fields pass; the three per-key settings reads are one jq per settings file, read once and answered from memory; the arm record is read in one jq pass instead of five; the grep -q settings scan is a builtin read; the sentinel escape (sed) and match (grep -E) are the shell's own; uname runs once per stop with its redirection on the enclosing group; the telemetry data object is assembled from its closed vocabulary. uname -s stays the platform primitive, hook::buffer_stdin and its validation pass belong to the synced shared library (untouched, as are all 17 copies), and the block decision is still one jq. Verdicts are unchanged: 95 old-versus-new scenarios compared byte for byte on rc, stdout and marker/ledger side effects. One disclosed divergence, a configured sentinel that itself holds a newline: grep read that newline as a pattern separator and authorized on either half; the shell match authorizes only the whole token standing alone. No launcher writes such a token; the suite pins the choice. The suite pins the budget by trace: exactly 1 creation and 1 launch on the default path, ceilings of 10 and 5 on the enabled block path (room left for the shared library's share), and no dirname, tr, sed, grep, cksum or date on either. Moving one redirection back inside its substitution fails it (2 vs 1; 11 vs 10). Closes #3515. Parent #3508; the in-file approach follows #3779. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
…acement (#3871) Closes #3509 ## Summary `pr-body-linkage-gate.sh` timed out on **every** recorded run in the measurement window — 423 timeouts against a 15 s ceiling, the worst blocked count in the #3508 campaign. A gate that is killed before it renders a verdict protects nothing, so its cost is part of its contract. Its MCP-surface sibling `pr-linkage-mcp-gate.sh` and the validator both share is fixed here too. **The parent's stated cause does not hold, and this PR does not act on it.** #3508 attributes the cost to per-field `jq` forks needing a new shared helper in `lib/hook-utils.sh`. Shard #3520 (PR #3779) established the real mechanism, and merged PR #3788 fixed 34 hook scripts across 17 plugins while touching `lib/hook-utils.sh` **zero times**. This PR follows that precedent: **`lib/hook-utils.sh` and every `plugins/*/hooks/hook-utils.sh` copy are untouched** (unmerged PRs #3740 and #3838 own that file). The fix is entirely in-file. The mechanism, re-verified on this host before any edit was made: | Form | clone-family | `execve` | | --- | --- | --- | | `V=$(jq . f)` | 1 | 1 | | `V=$(jq . f 2>/dev/null)` | **2** | 1 | | `{ V=$(jq . f); } 2>/dev/null` | **1** | 1 | | `V=$(printf \| jq \| tr)` | 4 | 2 | | `V=$(cat -- f)` | 1 | 1 | | `V=$(<f)` | **0** | **0** | | `V=$(shellfunc)` | 1 | 0 | | out-variable call | **0** | 0 | | `read < <(printf …)` | 1 | 0 | Bash elides the extra fork and execs in the command substitution's own subshell only when the command carries no redirection of its own. A `2>/dev/null`, a `<<<`, or a pipeline inside the substitution defeats that. ## Fix Four shapes, all in-file: 1. **Per-field `jq` batched into one process.** Both gates read payload fields through `printf '%s' "$INPUT" | jq -r … 2>/dev/null | tr -d '\r'`, once per field — 4 clones and 2 execs each, **five times over** on the MCP surface, all asking about one buffered string. One `hook::jq_fields` call (the library's existing batched reader, called not changed) answers every field, and CR-strips exactly as the `tr` did. 2. **Redirection hoisted onto the enclosing group.** `ORIGIN=$(git … 2>/dev/null || true)` became `{ ORIGIN=$(git …) || ORIGIN=""; } 2>/dev/null`. The group holds exactly one command, so nothing beyond that git call is silenced. 3. **The shared validator's helpers write into a caller-named variable.** `strip_html_comments`, `mask_markdown_code`, `section_content` and `trim` were each read through `$(…)` over a `< <(printf …)` line reader: **12 forks and zero extra `execve` per judged body** — pure process-creation latency. They now use `printf -v` and an in-shell line split. `linkage::chomp_to` reproduces the trailing-newline strip that command substitution performed, which is the one thing a naive out-variable conversion gets wrong. 4. **`$(<file)` for the `--body-file` read, `printf -v '%q'` for wrapper re-quoting, `hook::json_str_object_to` for the telemetry envelope** — replacing a `cat`, a `printf` substitution, and a `jq -n` that bash can do itself. The `$(dirname …)` source line was already fixed in both gates by #3771. `pr-body-linkage-gate.sh` and `pr-linkage-validator.sh` now invoke **no external command of their own at all**; every remaining spawn on their path belongs to `lib/hook-utils.sh`. `pr-linkage-mcp-gate.sh` keeps three, each justified: the origin-remote scope guard, the `jq -e` defer-guard on a repo's own `settings.json` (behind a `[[ -f ]]` probe), and a carriage-return fallback described below. ### Where the MCP reader's behaviour could change, and what it does about each `hook::jq_fields` CR-strips every value it returns; the validator only strips a CR at end of line. A body with a **mid-line** CR would therefore be judged against different text — and stripping is the **permissive** direction: `## Sum<CR>mary` becomes a section that was previously missing, turning a BLOCK into a silent ALLOW. The batch therefore also reports whether the raw body holds a CR at all, and the MCP body is re-read losslessly with its own `jq` only in that case, which no real payload hits. Review of the first revision found two field shapes where that batched reader itself flipped a per-field DENY to ALLOW; both are fixed in the second commit (`fe3fd235`), and the differential below was re-run over them: - **The CR probe was not type-safe.** `contains("\r")` errors on a non-string body, one erroring filter fails the whole batch, and a failed batch exited 0 — so a body of `5`, `true`, `{"a":1}` or `["x"]`, which the per-field reader rendered as text and blocked, was allowed. The probe now goes through `tostring` first, so it is total over every JSON type. A batch that **still** fails (a `tool_input` or payload root that is not an object) falls back to the per-field reads it replaced instead of allowing outright, so a batch failure is now exactly as fail-closed as the per-field reader was: a determinable bad body still blocks. That fallback is unreachable for any object `tool_input`, so the fenced spawn counts do not move. - **Trailing newlines survived on the exact-match fields.** `$( )` chomped them from every per-field read, so `"owner": "acme-corp\n"` or a tool name with a trailing newline matched the guards and was gated; `hook::jq_fields` keeps the newline and both slipped past. `TOOL`, `HOOK_CWD`, `T_OWNER`, `T_REPO` and `BODY` are now chomped in-shell, byte-identical to their `$(jq -r …)` form whenever they carry no CR. **One accepted stricter change remains.** A CR *inside* `tool_name`, `owner` or `repo` used to stay in the value, so it never matched and the call was allowed; `hook::jq_fields` strips it, the value matches, and the body is judged. Neither GitHub nor the MCP server produces such a value; the stricter direction is kept and named in the CHANGELOG rather than asserted away. ## Verification **Behaviour, proven by differential rather than argued.** - **End-to-end, 154 payloads (85 Bash, 69 MCP), exit code, stdout and stderr compared byte-for-byte** between the merge-base gates and these, after `fe3fd235`. Bash surface: every body-flag spelling (`--body`, `-b`, `--body=`, `--body-file`, `-F`, attached forms), stdin and substitution heredocs, multiple-heredoc and unterminated cases, missing body files, `--repo`/`-R` and `cd` escapes, `env -S` / `env` / `sudo` wrappers, `gh.exe` and `./gh`, CRLF and mid-line-CR bodies, NUL bytes, NBSP/BOM/ZWSP/U+2028/U+2029, `--fill`/`--web`, absent and non-string `command`, trailing-newline and CR `cwd`. MCP surface: the tool x owner x body matrix, absent/empty/null body, non-string bodies (`5`, `true`, `{"a":1}`, `["x"]`, `false`), absent and non-object `tool_input`, NUL and CR in `owner`, trailing newline and CR in `owner`/`repo`/`tool_name`/`cwd`. **Bash gate: 85/85 verdict-identical. MCP gate: 67/69 verdict-identical; the remaining 2 are the CR-in-`owner` and CR-in-`tool_name` cases above, ALLOW to DENY.** Before `fe3fd235` the same run showed 7 DENY-to-ALLOW mismatches (the four non-string bodies, `owner\n`, `repo\n`, `tool_name\n`); all 7 are gone and no new one appeared. Bash's own "ignored null byte" warning line, whose text carries the script path, is excluded from the byte comparison. Denies: 69 at the merge base, 71 here. - **Validator differential, 425 bodies**, hand cases plus a seeded fuzz corpus over heading/fence/comment/CR/NBSP/backtick-run tokens. **0 mismatches.** - **Trailing-newline invariance** proved separately, because `hook::jq_fields` preserves trailing newlines where `$(printf | jq)` stripped them: 24 validator cases and 9 hook cases, 0 mismatches; the MCP gate's own fields are now chomped as well. - Contract suites: `pr-body-linkage-gate.test.sh` **146/146**, `pr-linkage-mcp-gate.test.sh` **37/37** (nine new cases: the four non-string bodies block, `owner\n` / `repo\n` / `tool_name\n` still gate, CR-in-owner gates as the accepted stricter case, a string `tool_input` takes the fallback and allows). **Cost, measured with `strace -f -e trace=clone,clone3,fork,vfork,execve`** — not an xtrace command count, which reads source positions rather than kernel spawns (#3520 measured xtrace at 2 against 8 real spawns on one script). Telemetry sink off; re-measured after `fe3fd235`, unchanged: | Path | clones before | clones after | `execve` before | `execve` after | | --- | --- | --- | --- | --- | | Bash gate, a `gh` call with no `pr` | 8 | **7** | 3 | **2** | | Bash gate, `gh pr create` with a body (ALLOW) | 28 | **11** | 6 | **3** | | Bash gate, `gh pr create` with a body (BLOCK) | 28 | **11** | 6 | **3** | | MCP gate, unrelated tool | 7 | **7** | 2 | **2** | | MCP gate, create (ALLOW) | 37 | **11** | 9 | **4** | | MCP gate, create (BLOCK) | 37 | **11** | 9 | **4** | Two components: the validator refactor alone removes **12 clones with `execve` unchanged** — that half is pure latency, no work removed. The rest is genuinely duplicated work removed: six `jq` processes re-parsing one buffered payload, plus two `tr` calls deleting a byte class bash rewrites in place. **No wall-clock figure is claimed.** This is a Linux host where a spawn costs ~3-5 ms; the campaign's host measures 0.3-0.9 s and is bimodal at 501 concurrent processes. A timing here would say nothing about there, so the process count is reported as the proxy, per #3508's own correction. **New gate: `hooks/pr-linkage-spawn-budget.test.sh`.** Ceilings are the measured counts with **no headroom**, per `hook-budget.md` rule 2. It refuses to report a pass it has not earned: - a self-check first proves the harness can distinguish `$(cmd 2>/dev/null)` from `{ …; } 2>/dev/null` at the kernel level, and **SKIPs** rather than passing if it cannot (no ptrace, no strace); - three mutants must each raise the count above the ceiling or **the suite fails itself**: a redirect moved back inside a substitution (11 -> 12 clones), one field split back out of the batch (11 -> 14 clones, 4 -> 5 execve), a validator helper re-forking (11 -> 13 clones); - it asserts both gates still exit 2 on a failing body, so a budget of zero spawns cannot pass as a no-op. Pointed at HEAD's hooks the new suite reports **13 failures**; against this branch, 23/23 pass. **Gates run** (re-run after `fe3fd235`). `scripts/affected-tests.sh --run`: 151 shell suites pass, 14 selected suites belong to ecosystems the runner does not execute (reported NOT RUN, not skipped), one pre-existing failure noted below; `scripts/check-changelog-parity.sh` in all four modes (`--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`) all rc=0; `shellcheck -x` and `shfmt -d` clean on all five scripts; `markdownlint-cli2` and `editorconfig-checker` clean on the changed files; `check-shell-portability.sh`, `check-purged-em-dashes.sh`, `check-silent-skips.sh`, `check-discriminating-test-skips.sh`, `check-killswitch-hoist.sh`, `check-hook-exec-form.sh`, `check-fixture-git-isolation.sh` all rc=0 on the first revision. Manifest bumped 0.55.58 -> **0.55.60** with the matching CHANGELOG entry (see Related for why not 0.55.59), and the README carries the measured share per `hook-budget.md` rule 1. **Pre-existing failures, not from this branch** (both reproduce on a clean `HEAD` checkout, and this branch touches no file either reads): `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (2 cases, its own xtrace-based budget probe) and `plugins/session-flow/scripts/tests/test_save_point.py::test_new_origin_falls_back_to_directory_name`. ### Acceptance criteria: two are not fully met, stated plainly | # | Criterion | Status | | --- | --- | --- | | 1 | No more than 2 external spawns on the common path (own shell + at most one `jq`) | **NOT MET — over by one.** The common path is now the hook's own shell, one `jq -e .`, and one batched `jq`. The batched `jq` is the criterion's allowance; the `jq -e .` is `hook::buffer_stdin`'s payload validation inside `lib/hook-utils.sh`, which this PR is fenced off from. Removing it is that library's change to make, in #3740/#3838. | | 2 | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **MET.** None appears in any of the three scripts; `cat` is gone too. | | 3 | Matcher or early guard exits before any spawn for non-matching invocations | **PARTIALLY MET.** The `if: Bash(*gh *)` filter (pre-existing) removes the hook process entirely for non-`gh` calls, and the two in-hook guards short-circuit before any repo I/O. They cannot run before `hook::buffer_stdin`, whose `jq` is the same library spawn as row 1 — the hook must read stdin before it can know what it is looking at. | | 4 | Existing behavioural tests still pass; the guard still blocks what it blocked before | **MET, with one named exception.** 183 contract cases plus the 154-payload differential: every merge-base DENY is reproduced, and two merge-base ALLOWs (a CR inside `owner` or `tool_name`) are now DENY, the stricter direction, recorded in the CHANGELOG. | | 5 | Under 2 s for a single run on a Windows host | **NOT VERIFIED.** No Windows host available; the process-count proxy above is offered instead, and no timing figure is invented. | ## Related - Parent: #3508 (Windows process-creation tax). Its stated cause — per-field `jq` needing a new shared `hook-utils.sh` helper — is not what this PR acts on; see Summary. - Precedent: #3520 / PR #3779 (established redirection placement as the real mechanism) and merged PR #3788 (34 scripts, 17 plugins, `lib/hook-utils.sh` untouched). - **Version and overlap with #3838.** `main` is at `source-control` 0.55.58. Open, ready PR #3838 (`cursor/shell-script-perf-phase1-bb5b`) bumps this plugin to **0.55.59** with its own `## [0.55.59]` heading, and edits **both gate files this PR touches** (`pr-body-linkage-gate.sh`, `pr-linkage-mcp-gate.sh`, introducing `hook::buffer_stdin_to`) plus the three worktree gates and the synced `hook-utils.sh`. This PR therefore takes **0.55.60**, verified against `main`, #3838, #3774 (0.55.58, no bump) and #3740 (tops at 0.55.54). Whichever of #3838 and this PR lands second needs a **rebase of the two gate files, not just a rebump**; that is the merge lane's call, flagged here so it is not a surprise. Sibling #3510 is in flight against this plugin and takes the next version after this one. - Fenced off: unmerged PRs #3740 and #3838 own `lib/hook-utils.sh`; #3510 owns `worktree-add-containment-gate.sh`, `worktree-add-claim-gate.sh` and `worktree-create-gate.sh`, which share this plugin's `CHANGELOG.md` and manifest. Neither set is touched here. - Prior art acknowledged: #1403 / PR #1385, whose revival bar (four contract-test regressions, two failing open) is what the deny-preserving differential above is aimed at. - Budget authority: `docs/conventions/hook-budget/README.md` (#1809), surfaced by `.claude/rules/hook-budget.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
… off substitutions The Stop-event unsurfaced-failure detector timed out 113 times in the #3508 window at a 21.9 s average, roughly 44x the 500 ms the hook-budget convention gives the whole always-on per-turn set. The parent issue blames per-field jq forks needing a shared helper; that diagnosis is wrong here, as shard #3520 and PR #3788 established twice. The cost is redirection placement. Bash runs the command of a command substitution in the substitution's own subshell and skips the extra fork ONLY when that command carries no redirection of its own. `$(wc -c <file 2>/dev/null)` is two processes for one byte count; `$(cat -- file 2>/dev/null)` is two for a file read; `{ V=$(cmd); } 2>/dev/null` is one. Those forks are invisible to `bash -x`, which reads command positions, which is why the campaign kept mis-attributing the cost to the exec count. Six sites in hook-failure-audit.sh change and no shared library does: - `wc` names the transcript instead of redirecting it in, and `read` drops the filename column along with the padding some wc builds add. - The `read_window` helper is gone. Under the tail cap grep opens the transcript itself rather than being fed by `cat` through a function call that was a second subshell on top of the substitution's own. - The marker read is `$(<file)`, which forks nothing and execs nothing. - The marker write strips carriage returns in the shell instead of piping through `tr`. - The marker directory is probed with `-d` before `mkdir -p` spawns. - The two payload fields come from one `hook::jq_fields` pass instead of two `hook::jq_field` calls. Over the tail cap the `tail | sed | grep` pipeline and its redirect placement are untouched: a pipeline element forks either way, and hoisting the redirect onto a group would newly silence sed and grep for no saving. The `printf | jq` feeding the structural selection also stays a pipeline, because a here-string at or above the pipe capacity deadlocks before jq is exec'd and that payload routinely clears it; it is off the common path regardless. Common path (a turn with no failure recorded, under the cap): 18 process creations and 6 execs before, 9 and 4 after. Over the cap: 19 and 7 before, 12 and 6 after. A turn that warns: 34 and 17 before, 24 and 14 after. Measured with `strace -ff -e trace=clone,clone3,fork,vfork,execve`. No wall-clock figure is claimed: this Linux host says nothing about the Windows spawn tax the budget binds to, and on the #3508 host one creation costs 180-2,841 ms. Behaviour is unchanged, which for this hook is the whole point: it exists to surface failures that otherwise pass unnoticed, so a hoisted `2>/dev/null` could silence exactly the diagnostic it is for. Every group holds one command, and the one stream now silenced that was not before is grep's, which `cat`'s own redirect already discarded. Pre- and post-change stdout, stderr, exit codes and marker contents were compared byte for byte across ten scenarios: first warn, dedup, a new failing registration re-warning while the warned one stays muted, all three classification branches, the tail cap, a clean transcript, a missing transcript, the kill switch, and no data directory. Identical. The contract test gains an strace budget assertion on both counts, mutation- checked: moving either silenced redirect back inside its substitution adds a fork with no new exec, leaves every behavioural assertion green, and trips the creation ceiling. The README states the measured share per hook-budget Rule 1, as a process count, and says plainly that the Windows parallel-wall figure is still owed. Two acceptance criteria are not met and are called out in the PR: the hook cannot exit before any spawn, since reading the transcript is how it learns there is nothing to report, and the 500 ms parallel-wall figure for the three-plugin per-turn set is cross-plugin and Windows-only. Refs #3508. Precedent #3779, #3788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
…ons to 3
The plugin's own hook-cost accounting reported a 3-process steady
PostToolBatch fire, but it counted commands in command position, which
counts jq and bash INVOCATIONS rather than processes. Bash elides the
extra fork inside `$(...)` and execs in the substitution's own subshell
only when that command carries no redirection of its own; a
`2>/dev/null`, a `<<<`, or a pipeline written inside the substitution
defeats the elision and forks twice for one program. A fork that never
execs never reaches a command position, so the existing budget test
could not see any of it. Under `strace -f` the fire was creating 8
processes, not 3.
Every redirection on this path moves onto an enclosing `{ ...; }` group,
and the stdin payload is assigned in-process rather than captured
through a command substitution. Measured on the steady non-crossing
path: 8 process creations to 3, with program launches unchanged at 4 —
the same jq, bash and jq still run over the same inputs, so only the
fork overhead around them is gone. On the hosts in #3508 a process
creation costs 180-2,841 ms (median 1,108 ms) against about 1% user CPU,
which is the whole of the available saving there.
`scripts/context-zone.sh` carries two of the three sites, so the
PreToolUse zone gate and the PostCompact marker inherit the resolver's
share. `lib/hook-utils.sh` is untouched.
Behaviour is unchanged: no decision, emitted text, exit code or state
file differs, and the group redirect suppresses exactly the stream the
inner one did while still propagating the command's exit status.
Tests: a process-creation budget asserted under `strace` at exactly 3,
with program launches pinned at 4 so a fork saving cannot be mistaken
for work removed; verified non-vacuous by reverting one redirect, which
takes the count to 4 and fails. Plus behaviour tests for the new
redirection placement — a malformed zones.json drives the resolver's
only stderr path on this route and pins that the notice reaches neither
of the hook's streams, that stdout stays one parseable JSON document,
and that shipped default bands still resolve and inject; an unparseable
payload pins that the payload pass's nonzero status still propagates out
of its enclosing group.
Refs #3508
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
The optimization is unchanged. Three statements around it were wrong or missing, and this corrects them. The PostCompact marker does not benefit. post-compact-mark.sh never calls scripts/context-zone.sh, and it still reads its payload through cg::read_payload inside a command substitution, so its process count is untouched. Verified by reading the file: the resolver appears nowhere in it, and line 50 is still INPUT=$(cg::read_payload). The changelog and the README now name the two hooks that do benefit: the zone-crossing hook on both its PostToolBatch and UserPromptSubmit routes, and the PreToolUse zone gate, which calls the same resolver. The here-string is not free, and was documented as if it were. Replacing printf '%s' "$INPUT" | jq with jq fed by <<<"$INPUT" removes two process creations, but bash 5.1+ delivers a here-string through the pipe buffer only while it fits: at or above 64KiB it writes the string to /tmp/sh-thd.* and hands jq that descriptor. Measured here on bash 5.2.21 under strace: 60,000 bytes opens no file, 65,536 opens one twice. The pipeline it replaced never touched disk. Output is byte-identical, but a PostToolBatch payload carrying every serialized tool result routinely clears 64KiB, and the target platform runs Defender real-time protection, which scans temp-file writes. Disclosed in the README's hook-cost accounting per the hook-budget rule, in the changelog, and at the call site, replacing a comment that claimed the rewrite "changes nothing else". The reason for keeping the drain loop was stale. It said piping stdin straight to jq would remove another process; after the _to conversion the loop is read builtins and costs zero. What piping would actually avoid is the here-string above. The decision stands on its real merit: payload.sh's bounded read -t 5 caps a stalled pipe at five seconds instead of blocking to the harness timeout, which is the symptom #3508 is about. Refs #3520 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
…destination printf -v "$__cg_dest" resolves the destination name against this function's own scope, so a caller passing `input` or `chunk` got its variable left unset while the function still returned 0 — success with no value and no error. No call site hits it today: zone-crossing-inject.sh passes INPUT and the printing wrapper passes __cg_buf. But the header comment invites new callers to adopt the _to form, and `input`/`chunk` are the names such a caller reaches for first, so the hazard sat in front of the next caller rather than behind this one. The internal locals become __cg_input / __cg_chunk, the prefix convention lib/hook-utils.sh already uses for __hu_. A prefix reserves a namespace rather than abolishing the hazard, so the three names that remain internal are now refused loudly with rc 2 and a stderr line instead of failing silently. __cg_buf stays usable because the wrapper passes it. zone-crossing-inject.test.sh pins both halves: five caller-chosen destination names fill correctly, three reserved names are refused with rc 2, and the wrapper still returns the payload. Verified discriminating: the new cases fail five ways against the pre-fix reader, including rc=0 with empty stderr on the reserved names, which is the exact silent-success mode reported. Reported by an automated review on PR #3779. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
4472da8 to
6236608
Compare
…ach (#3873) Closes #3510 ## Summary The three `source-control` worktree gates spawned four processes to read each hook payload field. The parent campaign (#3508) blames per-field `jq` forks needing a shared helper; that diagnosis is wrong and this is the fourth PR to show it. Part of the cost is a **fork that never execs**, part is a `| tr -d '\r'` stage behind every read, and all of it is in-file: > `$(cmd)` = 1 process. `$(cmd 2>/dev/null)` = 2. `$(a | b | c)` = 4. `{ v=$(cmd); } 2>/dev/null` = 1. `$(printf '%s' "$X" | cmd)` = 3. Measured on this branch with `strace -f -e trace=clone,clone3,fork,vfork,execve`, confirming the mechanism before touching anything. `lib/hook-utils.sh` and its 17 synced plugin copies are **untouched**. Reported at **418 timeouts**, worst hook in the set after #3511. A scout counted roughly 7 in-substitution redirect/pipe sites; the actual count is 8: three in `worktree-add-containment-gate.sh`, four in `worktree-add-claim-gate.sh`, one in `worktree-create-gate.sh`. **`worktree-create-gate.sh` was partly clean already.** It carries the fixed `${BASH_SOURCE[0]%/*}` form (from #3788), and `hooks.json` registers it under `WorktreeCreate` at timeout 60, firing once per worktree creation. It is *not* on the `PreToolUse:Bash` path that produced the 418 timeouts, so none of those were its. Its two field reads were still costing 19 process creations, so they are fixed here, but the win is bookkeeping, not the timeout. **Review fix, second revision.** The first revision of this PR fed the whole payload to `jq` and `sed` by **here-string** (`{ V=$(jq …); } <<<"$INPUT"`), which is the exact form `lib/hook-utils.sh` forbids at `hook::json_complete` ("`printf | jq`, never `jq <<< "$1"`"). The reason is #1587 (`dfda6ec3`): bash fills a here-string's pipe itself, so a payload at or above the pipe capacity, **65536 bytes, traced hanging bash indefinitely on Git Bash**, blocks the shell before `jq` is ever exec'd. On these hooks that hang is the 15 s timeout, and on a containment gate that is a stall **and** a fail-open, the failure mode this campaign exists to remove. It does not reproduce on Linux bash 5.2 (the temp-file fallback engages at 65536 to 70000 bytes, verified in this session), which is how it slipped through. This revision adopts the library's own form and re-measures; the numbers below are the honest new ones, and several are worse than the first revision's. ## Fix In-file only, all mechanical: 1. **Both Bash gates' payload reads** (2 sites in containment, 3 in claim): `$(printf '%s' "$INPUT" | jq … 2>/dev/null | tr -d '\r')` becomes `$(printf '%s' "$INPUT" | jq … 2>/dev/null)` plus `V="${V//$'\r'/}"`. The feed is **kept as `printf '%s' "$INPUT" | jq`**, byte-for-byte the feed line inside `hook::jq_field`, the form the library prescribes for a hook payload; the jq program text is unchanged (`// empty`, no `gsub`), which is what keeps every differential below byte-identical to `main` on a non-string `.cwd`. Only the `tr` stage goes. `hook::jq_field` itself was measured too: it costs 4 creations to the inline form's 3 and adds a string-only `gsub` that changes the non-string-field behaviour, so it is not used. A here-string was measured at 1 creation and is not used either, for the reason above. 2. **`git_unlocated`**: `2>/dev/null` moves off the `git` command onto the enclosing subshell, so bash execs git in that subshell instead of forking for it. `unset` writes nothing to stderr, so the same stream is silenced and no other. 3. **`configured_root`**: `git config --get-all | tail -n 1 | tr -d '\r'` becomes one `git` plus `${r##*$'\n'}` and `${r//$'\r'/}`, on a single-command group (no payload involved). Multi-value last-wins verified A/B. 4. **Claim gate's stderr temp file**: `mktemp` + `2>"$err_file"` + `rm -f` where the file was **never read**. Replaced with `2>/dev/null` on a one-command group. This also removes the `|| continue` that silently skipped a claim whenever `TMPDIR` was unwritable, a fail-open now gone (`TMPDIR=/nonexistent` reproduced the lost claim on `main`). `claim_rc=$?` stays outside the group so it is still the helper's status. 5. **Create gate's `json_field`** becomes a `_to` form (the convention `hook-utils.sh` documents on its own `_to` helpers), dropping the two substitutions that wrapped each read; each rung is its own `printf '%s' "$payload" | jq` or `printf '%s' "$payload" | sed` pipeline. The jq program text is byte-for-byte `hook::jq_field`'s, `gsub` included, on purpose: `gsub` is string-only, so a numeric or object `.name` fails jq and falls through to the string-shaped `sed` rung, which yields nothing and refuses. 6. Nothing else. Every remaining `jq`, `git` and `sed` call is the same call with the same arguments. The two `<<<` left in the Bash gates are `IFS='/' read -r -a segs <<<"$rest"`, a builtin fed one path string, pre-existing on `main`; bash fills no pipe for a builtin, so that is not the hazard. ### Counts, per file, before → after Process creations (clone/clone3/fork/vfork) and `execve`, counted separately, re-measured after the review fix. No wall-clock figure: this Linux host is nothing like the campaign's contended Windows hosts, so a time here would be misleading. | Hook | Path | creations | `execve` | | --- | --- | --- | --- | | `worktree-add-containment-gate.sh` | a `worktree` command that is not an `add` (the hot path) | 8 → **7** | 3 → **2** | | | an `add` outside every repository (allow) | 22 → **18** | 7 → **5** | | | an `add` into a working tree (**block**) | 21 → **18** | 7 → **5** | | | an `add` into a `.git` directory (**block**) | 27 → **22** | 9 → **7** | | | `git -C <repo> worktree add sub/x` (**block**, names the root) | 26 → **20** | 10 → **6** | | | dynamic / post-`cd` / `echo git worktree add` (allow) | 13 → **11** | 5 → **3** | | `worktree-add-claim-gate.sh` | a `worktree` command that is not an `add` | 8 → **7** | 3 → **2** | | | a parsed `add` target | 28 → **22** | 11 → **6** | | `worktree-create-gate.sh` | payload with no `.name` (before the helper runs) | 19 → **13** | 6 → **4** | | | disabled by the kill switch | 0 → **0** | 0 → **0** | For the record, the first revision's here-string counts were 5 / 14 / 14 / 18 / 16 / 7 / 5 / 16 / 7 creations on those rows. The `printf | jq` form gives back exactly 2 creations per field read (3 where a here-string was 1), and no `execve`: the `execve` column is unchanged from the first revision. **`execve` dropping is not removed work.** Every drop is a named program replaced by a bash builtin or removed as dead code: | Program | Replacement | | --- | --- | | `tr -d '\r'` (×6 across the two Bash gates, 3 + 3; ×1 in the create gate; 7 removed) | `${v//$'\r'/}` | | `tail -n 1` (containment `configured_root`) | `${r##*$'\n'}` | | `head -n 1` (create gate fallback rung) | `${v%%$'\n'*}` | | `mktemp` + `rm` (claim gate) | removed; the temp file was written and deleted, never read | Four of the seven creations left on the hot path belong to `lib/hook-utils.sh` (the `hook::buffer_stdin` substitution and `hook::json_complete`'s `printf | jq -e .`), which is fenced off here. The gate's own share is the one `printf | jq` field read: 3 creations, 1 `execve`. ## Verification **Behaviour is the risk** on these gates: a swallowed non-zero status turns a block into a silent allow, which here means an unclaimed or out-of-tree worktree gets created. So every deny path and its near misses were A/B'd on **rc, full stdout and full stderr** against a pristine `git archive origin/main` copy of the plugin, re-run in full after the review fix: - **Containment, 52 probes**: 11 payloads × 4 root-resolution environments (nothing configured, plugin option, plugin data dir, `melodic.worktreeroot` git key) = 44, plus the kill switch, an empty stdin, a malformed payload, four non-string `.tool_input.command` values (number, object, `null`, `true`), and a non-string `.cwd`. Payloads: out-of-containment target, target inside a working tree, target inside a `.git` directory, `git -C`-composed relative target, valid external creation, `..`-escape out of the repo, dynamic `$HOME` target, post-`cd` target, `echo git worktree add`, an unrelated command, a CR-bearing path. **Byte-identical, all 52.** - **`configured_root` precedence** separately: git key beats plugin option beats data dir; multi-value `--get-all` last-wins; single value. Identical. - **Claim gate, real state**: a freshly-added unlocked worktree (claimed, correct `additionalContext`), a worktree already carrying **another session's** live claim (helper rc 4, not rewritten, reason string preserved verbatim), a target that was never created (nothing claimed), and a payload with no `session_id`. Identical, including the emitted JSON, modulo fixture path and timestamp. - **Create gate, 18 probes** including the disabled path, empty stdin, illegal branch name, missing root, non-repository cwd, unexpanded `${user_config}` placeholder, `name` 4242 / object / `null` with jq present, and a **jq-absent PATH** exercising the `sed` fallback rung with `name` 4242, `null` and a string. Identical apart from fixture paths and a fixture commit SHA. - **Existing suites**: `worktree-add-containment-gate.test.sh` (41 cases), `worktree-add-claim-gate.test.sh` (24), `worktree-create-gate.test.sh` (37). All pass. ### Permissive normalization The sibling shard's finding, that CR stripping can be the *permissive* direction, was checked here in both directions, with CR, BOM, zero-width space, U+2028, U+2029, NBSP, and trailing newline in the target path and in `.cwd`. **A/B identical on all 15 probes.** No batching was introduced, so neither of the two flip mechanisms is reachable: each field keeps its own `$(…)` and its own `// empty`, and the only string-only jq filter in the diff is the create gate's **pre-existing** `gsub("\r";"")`, kept byte-for-byte on purpose (see Fix 5). `name:4242`, `name:{"a":1}` and `name:null` are all still refused with the identical message, with jq present and absent. ### Pre-existing CR containment bypass (not fixed here; predates this work) Recorded precisely enough to act on without rediscovery, because a security finding should not live only in a PR body. **This PR does not fix it, and it is present identically on `origin/main`**; this lane does not file work items. - **Payload shape.** A Bash tool call whose command is `git worktree add ..<CR>/../outside/x` from a cwd inside a repository, where `<CR>` is the raw byte 0x0D immediately after a `..` segment. On the wire the harness JSON-escapes it, so the payload reads `"command":"git worktree add ..\r/../outside/x"`; that is the real `PreToolUse:Bash` payload path, nothing downstream re-parses the command, and the hook's tokenizer keeps the word whole. - **What the hook does.** `worktree-add-containment-gate.sh` strips every CR from the command before resolving, so it sees `../../outside/x`, resolves it to a path outside the repository, and returns rc 0 (allow). Plain `../outside/x` from the same cwd is blocked (rc 2, target `<repo>/outside/x`); `a<CR>b/../../outside/x` is also blocked, since a CR mid-segment leaves the nearest-existing-ancestor walk on the repository. The `..`-adjacent position is the one that flips. - **What git then does.** git does not strip the CR: `..<CR>` is an ordinary directory name, so `..<CR>/../outside/x` resolves to `<cwd>/outside/x`, and `git worktree add` creates it there, **inside the repository** (`<repo>/sub/outside/x`, rc 0, confirmed by `git worktree list`; a literal `..<CR>` directory is left in `<repo>/sub`). From the repository root, `..<CR>/../.git/wt` lands **inside `.git/`** (`<repo>/.git/wt`, created, rc 0). - **`worktree-add-claim-gate.sh`** (PostToolUse) returns rc 0 on the same command as well; it has no containment role, so nothing after the PreToolUse gate catches it. - **CR-only.** Quoting does not matter: unquoted, double-quoted and single-quoted CR words all allow in the hook and all create the directory in git. TAB is not in the class: an unquoted `..<TAB>/../outside/x` splits into two words for the shell and for the hook's tokenizer alike, and git then fails (rc 128, nothing created); a quoted `"..<TAB>/../outside/x"` stays one word, the hook does not strip TAB, resolves it lexically to `<repo>/sub/outside/x`, and **blocks**. Only CR is stripped before resolution, so only CR diverges from what git will do. - **Severity, the reviewer's read: low.** The gate is documented best-effort (the header declares fail-open on anything it cannot resolve statically) and already allows any `$VAR`-carrying target, so `git worktree add $PWD/x` reaches the same place with no CR at all. The CR shape grants nothing that `$PWD/x` does not. It is still a real fail-open of the class #3871 named, in shipped code, and the fix belongs with whoever owns the strip (strip CR only from the ends of the word, or resolve the un-stripped word and compare). ### New test `plugins/source-control/hooks/worktree-gates-spawn-budget.test.sh` (15 assertions) holds the counts as ceilings. It uses **strace, not xtrace and not a PATH shim**: both are blind to a fork that never execs. Upper bounds rather than equalities, so a later library change that removes more work does not fail it. **The ceilings cannot see a here-string regression**, because a here-string *lowers* the count; the suite's header says so, and three new cases grep each gate for `<<<` on `$INPUT` or `$payload` so that regression fails the suite anyway. It skips as a suite where `strace` is absent or cannot ptrace, rather than asserting on empty trace output. **Proven non-vacuous against seven mutants**, one per change, each reverted alone in a scratch copy and re-run after the review fix. A restored `tr` stage costs exactly +1 creation and +1 `execve` per field, which the exact ceilings catch: | Mutant | Suite | Cases it fails | | --- | --- | --- | | control (no mutation) | pass | none | | M1 containment `COMMAND` read, `tr` restored | fail | all 3 containment cases | | M2 containment `HOOK_CWD` read, `tr` restored | fail | 2 containment cases | | M3 containment `git_unlocated` | fail | 2 containment cases | | M4 containment `configured_root` | fail | the block case | | M5 claim's three field reads, `tr` restored | fail | both claim cases | | M6 claim `err_file` temp | fail | the parsed-target case | | M7 create `json_field_to` rungs (back to `hook::jq_field` + `sed \| head \| tr`) | fail | the create case | ### Gates | Gate | Result | | --- | --- | | `scripts/affected-tests.sh --run` | **exit 0**, all 4 selected suites pass; no unmapped file | | `check-changelog-parity.sh --check` | pass | | `check-changelog-parity.sh --check-order` | pass (91 changelogs, no duplicate heading) | | `check-changelog-parity.sh --check-bump origin/main` | pass | | `check-changelog-parity.sh --check-preserved origin/main` | pass (234 headings compared) | | `shellcheck -x` on all four scripts | clean | | `shfmt -d` (EditorConfig-driven) | clean | | `markdownlint-cli2` on README + CHANGELOG | 0 issues | | `check-killswitch-hoist.sh` | pass (31 hooks) | | `check-silent-skips.sh` | pass | | `check-discriminating-test-skips.sh` | pass | | `check-hook-exec-form.sh` | pass | | `check-fixture-git-isolation.sh` | pass (128 isolated) | ### Acceptance criteria, stated plainly | Criterion | Status | | --- | --- | | No more than 2 external spawns on the common path (own shell + at most one `jq`) | **Met for this hook's own share**: the gate execs exactly one `jq` of its own (3 creations, because the payload rides in on `printf \| jq` rather than a here-string, by library rule). **Not met literally**: a second `jq -e .` remains, and it belongs to `hook::json_complete` inside `lib/hook-utils.sh`, which is fenced off from this change (17 synced copies; unmerged #3740 and #3838 both target it). | | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **Met.** `tr`, `tail` and `head` are gone; `dirname` went in #3788. The one remaining `sed` is the create gate's jq-absent fallback rung, off the hot path, and is deliberately kept. | | Early exit before any spawn for non-matching invocations | **Already met on `main`** by the `if: Bash(*worktree*)` registration filter (#3621) plus each hook's own jq-free regex pre-filter. Unchanged here. | | Existing behavioural tests pass; the guard still blocks what it blocked | **Met.** 102 existing cases plus the A/B above. | | Under 2 s for a single run on a Windows host | **NOT VERIFIED.** No Windows host available. The budget doc's ceiling is stated as parallel wall time and this host's spawn cost is nothing like the campaign's, so a figure from here would mislead. Process creations are reported instead, which is the quantity that maps to the tax. | ### Reproduction ```bash strace -f -qq -e trace=clone,clone3,fork,vfork,execve -o t.txt \ bash plugins/source-control/hooks/worktree-add-containment-gate.sh <payload.json grep -cE '(clone3?|v?fork)\(' t.txt # creations grep -cE 'execve\(' t.txt # minus 1 for the traced program itself ``` ## Related - **#3508**: parent (Windows process-creation tax). Its stated cause, per-field `jq` forks needing a shared helper, is disproved again here: this PR touches `lib/hook-utils.sh` zero times. - **#1587** (`dfda6ec3`): the here-string deadlock trace this revision defers to; `lib/hook-utils.sh:1380` is the rule. - **#3779** (issue #3520): the precedent that first established the redirect-placement mechanism. - **#3788** (merged): fixed 34 scripts with zero `lib/hook-utils.sh` edits, and is where these three gates got their `${BASH_SOURCE[0]%/*}` form. - **#3871** (issue #3509): **sibling sharing this plugin's version chain**, and the source of the permissive-normalization class checked above. Its files (`pr-body-linkage-gate.sh`, `pr-linkage-mcp-gate.sh`, `pr-linkage-validator.sh`) are untouched here and its changelog entry is preserved verbatim. Its README hunk sits ~40 lines above this PR's, in the `pr-body-linkage-gate` section. - **#3838**: **overlaps this PR's files.** It edits all three worktree gates, replacing `INPUT=$(hook::buffer_stdin)` with the new `hook::buffer_stdin_to` form. The hunks do not overlap this PR's (its edits are on the `buffer_stdin` line; this PR's start below it), and a three-way merge was clean, but **the merge lane should expect a rebase rather than only a rebump**. The two changes are complementary: `buffer_stdin_to` removes one more creation from the same hot path, which is exactly why this PR's budget test asserts upper bounds rather than equalities. - **Version chain.** `main` is `0.55.58`; #3838 takes `0.55.59`; #3871 takes `0.55.59` and is being renumbered to `0.55.60`; **this PR takes `0.55.61`**, verified against `main` and every open `source-control` PR (#3774 and #3740 both claim versions at or below `main` and are stale, so they do not contend for 59 to 61). - **#3740**: unmerged `lib/hook-utils.sh` change; fenced off here, as is #3838's copy of it. - **#1403 / #1385**: prior art whose revival must first clear four contract-test regressions, two of which failed open. Not revived here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Keep the zone-crossing process-count work current so CI can land. merge-tree against origin/main was clean. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
typos flags "unparseable"; the rest of this plugin already writes "unparsable". The process-count sentence on the purged README used an em dash; rewrite it with a comma. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…3870) Closes #3515 > [!IMPORTANT] > **Which of #3515's acceptance criteria this PR meets, and which it does not.** > > | Criterion | Status | > |---|---| > | No more than 2 external process spawns on the common path (own shell + at most one `jq`) | **Met for the common path**, which is every interactive Stop: the hook's shell plus one `uname -s`. The one spawn is `uname`, not `jq`; it is the managed-settings platform primitive the trust design rests on (`$OSTYPE` is a variable a repo env block can set) and was left alone on purpose. **Not met for an enabled lane's stop**: that path launches 5 programs (4 `jq`, 1 `uname`), down from 18. One of those `jq` is in the fenced shared library's `hook::buffer_stdin` (its payload validation pass); the other three read three different inputs (payload, settings file, block decision). | > | `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` replaced with builtins on the hot path | **Met.** None launches on either traced path; the suite pins their absence. `cksum` remains on the marker-consumption path only (it keys the ledger, and changing the key would orphan existing ledger entries). | > | An early guard exits before any spawn for non-matching invocations | **Partially met.** The payload-free pre-filter exits before stdin and before `jq`, but it spawns `uname` to know which fixed managed-settings path to test. Removing it would mean testing all three platform paths, one of which is cwd-relative on Linux, so it stays. | > | Existing behavioural tests pass; the guard blocks what it blocked before | **Met**, with one disclosed divergence in the stricter direction (below). | > | Measured on a Windows host, a single run completes in under 2 s | **Not measured.** This runner is Linux; wall-clock is not the right proxy here and no figure is invented. The hook-budget convention's per-turn 500 ms bar (the corrected criterion from the issue comments) also still needs the Windows measurement. | > > `Closes #3515` is kept because the `pr-issue-linkage` gate wants a native closing keyword. A reviewer may prefer to reopen #3515 on merge for the Windows measurement and the enabled-lane count, or split those into a follow-up. ## Summary `hooks/lane-stop-gate.sh` is the autonomy plugin's `Stop` hook. It fires on every turn of every session, gated or not, and #3515 recorded it at a 27.6 s average with 73 timeouts against its 15 s budget on the #3508 hosts. It carried the largest in-file fork signature of the campaign: about 12 redirections and 11 pipes written inside command substitutions, plus a `$( )` capture around every lib helper that is nothing but parameter expansion. Following #3779's diagnosis rather than #3508's original framing: the cost was redirection placement and per-helper subshells, not per-field `jq` needing a shared helper. `lib/hook-utils.sh` and all 17 `plugins/*/hooks/hook-utils.sh` copies are untouched (`git diff origin/main --name-only` shows only the six autonomy files). The one shared-library facility used, `hook::jq_fields`, already existed on `main`. ## Fix All in `plugins/autonomy/hooks/lane-stop-gate.sh` and `lane-stop-gate-lib.sh`: | Site | Before | After | |---|---|---| | Lib path helpers (`gate_data_dir`, `gate_trusted_data_dir`, `gate_arm_record_path`, `gate_arm_claim_path`, `gate_user_settings_file`) | `v=$(gate_x)`: one fork each, for a function that only expands parameters | `gate_x_to <var>` forms via `printf -v`; print forms delegate, so `lane-stop-gate-arm.sh` and the lib-level tests read exactly what they read before | | Pre-filter settings scan | `grep -q lane_stop_gate "$f" 2>/dev/null` per file | builtin NUL-chunk `read` + substring test (`gate_file_mentions`), with `2>/dev/null` written **before** the input redirection so an existing-but-unreadable settings file stays as silent as `grep` was (bash applies redirections left to right; see case 51) | | Managed-files list | `done < <(gate_managed_settings_files)` with `$(uname -s 2>/dev/null)` inside: 3 creations per call, called up to 4 times per stop | `gate_managed_settings_files_load` fills an array in-process; `{ platform=$(uname -s); } 2>/dev/null` is 1 creation; loaded once per stop and reused by option resolution | | Payload fields (`EVENT`, `SESSION_ID`, `CWD`, `STOP_ACTIVE`, `LAST`) | five `printf \| jq \| tr` pipelines, 3-4 creations each | one `hook::jq_fields` pass (3 creations: process substitution, printf writer, jq); values chomped of trailing newlines so each reads byte-for-byte as the old `$( )` capture did | | Settings options (3 keys x managed files + user file) | one `$(jq … <file 2>/dev/null)` per key per file, each behind two more captures (8 creations per key) | `gate_settings_options_to <file> <key>...`: one jq per file for every key, NUL-separated through a process substitution with the redirections on the enclosing group (1 creation per file); resolved once, answered from memory (`gate_option_to`) | | Arm record | `jq -ec .` validation + `printf \| jq` per field + `printf \| jq` per option: 8 creations plus 6 on lookup | one jq pass over the record file yielding armed_at, session_id, sentinel, marker, compact json (1 creation); `EPOCHSECONDS` for the TTL clock with `date` as the pre-5.0 fallback | | Sentinel escape and match | `$(printf \| sed …)` then `grep -qE … <<<"$LAST"` | shell loop over the same fifteen metacharacters; `[[ $LAST =~ (^\|\n)[[:space:]]*TOKEN[[:space:]]*(\n\|$) ]]`, which agrees with grep's per-line verdict on every message (argument in the code comment) | | Telemetry data object | `$(jq -nc --arg … 2>/dev/null)` on every evaluated stop | assembled in the shell from the closed vocabulary: identical bytes | | Marker ledger | `$(printf \| cksum \| tr -cd '0-9')`, `$(stat … \|\| stat … \|\| printf '')`, `$(dirname …)` | `{ key=$(cksum); } < <(printf '%s' "$path")` with a parameter-expansion digit filter (same key as before, verified), per-rung `{ …; } 2>/dev/null` stat groups, `${ledger%/*}` | | Post-nudge branch read | `$(git … 2>/dev/null \| tr -d '\000-\037')` | `{ BRANCH=$(git …); } 2>/dev/null` + `${BRANCH//[[:cntrl:]]/}` (a superset strip; git refuses every such byte in a ref name, and `lane::notify` strips C0 again) | | `gate_resolve_plugin_name` (unanchored installs) | `$(jq … <"$manifest" 2>/dev/null)` | group-hoisted redirections | Left alone on purpose: `uname -s` as the platform primitive; `hook::buffer_stdin` (shared library; its `$( )` capture, read-slice probe and `printf | jq -e` validation are 4 of the enabled path's remaining 10 creations); the `gate_arm_owned` subshell (one fork, and the cleanest scope for `umask`/`noclobber`); the final `jq -nc` block decision. **Disclosed divergence (one degenerate config):** a configured sentinel that itself holds a newline. `grep -E` read that newline as a pattern separator and authorized a stop on any line matching either half of the token. The shell match treats the token as one pattern, so only the whole token standing alone authorizes, which is what the block reason instructs the agent to emit. No shipped launcher writes such a token. Pinned by new case 50 and named in the 0.22.30 changelog entry. Group redirections were checked for over-suppression: every `{ …; } 2>/dev/null` here wraps exactly one command, and the two process-substitution loops wrap a `read` builtin (no stderr) plus the one jq whose stderr the old code already suppressed. ## Verification **Before/after, measured here** (`strace -f -e trace=clone,clone3,fork,vfork,execve`, staged `plugins/cache/<m>/<n>/<v>/hooks` install, hook launched by the harness so its own shell is not in the count): | Path | Creations before | After | Launches (`execve`) before | After | |---|---|---|---|---| | Default: no gate footprint anywhere (every interactive `Stop`) | 4 | **1** | 2 (`grep`, `uname`) | **1** (`uname`) | | Enabled by user settings, first stop, no signal (block) | 48 | **10** | 18 | **5** (`jq`, `jq`, `uname`, `jq`, `jq`) | | Enabled, sentinel on its own line (allow) | 44 | 9 | 16 | 4 | | Enabled, second stop after the nudge (allow, notify muted) | 54 | 10 | 21 | 5 | | Enabled, marker present (allow, consume) | 51 | 12 | 19 | 6 | | Env-only enable claim (once-per-session notice) | 31 | 22 | 11 | 8 (remainder is `hook::notice_once` in the shared library) | | Armed by the launcher, first stop (block) | 60 | 11 | 20 | 5 | Launches did not stay flat, and that is expected here: unlike #3779, this issue explicitly asks for the `grep`/`sed`/`tr`/`dirname` helpers to become builtins, so those launches are gone; every `jq` that reads a distinct input is still launched, and the payload/settings/arm-record `jq` calls that were batched read the same inputs once instead of several times. **Wall-clock is not measured and no figure is invented.** A spawn is about 1 ms on this Linux runner; #3508 measures 180 to 2,841 ms per spawn (median 1,108 ms at 501 concurrent processes) on the affected Windows hosts. Process creations by trace are the drift-immune proxy #3508's corrected criteria ask for. The plugin README now carries the table above under a "Hook cost" heading, per hook-budget Rule 1, and says the Windows wall-clock share still needs measuring. **Proof the verdicts did not change.** - All 89 pre-existing assertions in `lane-stop-gate.test.sh` pass unmodified; the suite is now 103 with the additions below (102 where `chmod 000` denies nothing and case 51 skips visibly). - A black-box differential harness (scratch, not committed) ran the `origin/main` hook and this branch's hook over 95 scenarios from identical fresh staged installs, comparing rc, stdout, and marker/ledger side effects: every settings shape the reader distinguishes (boolean/string/number/null values, `options` as string/array, `pluginConfigs` as array/string, null entry, other marketplace, malformed, two documents), sentinel edge cases (CRLF, tabs, blank lines, inline mention, substring, trailing text, 120 KB messages with early and no sentinel, invalid UTF-8 around the token, metacharacter sentinels, empty sentinel), marker absolute/relative/consumed/stale-ledger/undeletable, env-only claims, and the arm record's every parse and claim path (null/false/number/string/empty/malformed, missing or non-numeric or float `armed_at`, expired, legacy `session_id` match/mismatch/numeric, taken/empty/directory claim files, missing session id). **93 identical; the 2 differences are the two halves of the disclosed newline-sentinel case.** - The ledger key derivation and the sentinel escape were also compared directly against the old pipelines on sample inputs: identical. **New regression tests, verified non-vacuous.** - Case 49: `gate_settings_options_to` answers three keys from one pass with the single-key verdicts (including the trailing-newline chomp and the all-or-nothing `options`-not-an-object case). - Case 50: a newline-bearing sentinel authorizes only as a whole block (the disclosed divergence, pinned). - Case 51: an existing-but-unreadable settings file produces **no stderr** and does not change the verdict. Pins the redirection order in `gate_file_mentions` (commit `fd0e7578`): the first draft wrote `done <"$1" 2>/dev/null`, which attempts the open before stderr is silenced and prints `Permission denied` per turn where `grep -q … 2>/dev/null` was silent. Run as an unprivileged user the case passes on the fixed tree and fails with that exact line on the old ordering; where `chmod 000` denies nothing (root, or a filesystem without POSIX modes) it skips visibly rather than passing vacuously. - Trace budget: exactly 1 creation and 1 launch (`uname`) on the default path; ceilings of 10 creations and 5 launches on the enabled block path (a ceiling so a shared-library saving in #3740/#3838 lowers the count without failing here, while a regression in this plugin's files raises it and does); and no `dirname`, `tr`, `sed`, `grep`, `cksum` or `date` on either path. Skips where `strace` is unavailable; the CI Linux lane does not skip. Re-measured rather than assumed after the `fd0e7578` redirection swap: both pins hold, and the enabled path's four `jq` are still `jq -e .` (`hook::buffer_stdin`), the `jq -j` payload pass, the `jq -j` settings pass and the `jq -nc` block decision, in that order with `uname` third. - Mutation 1 (move the `uname` redirection back inside its substitution): `FAIL: default path creates 2 processes, budget is 1` and `FAIL: enabled block path creates 11 processes, ceiling is 10`. Mutation 2 (put the `sed` escape pipeline back): 13 creations, 6 launches, and the named-helper check all fail. Both on scratch copies; the committed tree passes. **Commands run in the foreground on the pushed head `fd0e7578`, with actual results** (the `lane-notify` / lane-launcher / `check-shell-portability` lines are from the `1ca98cbf` run; the second commit touches neither those files nor their inputs, and the full `affected-tests.sh --run` below was re-run on `fd0e7578`): ``` bash plugins/autonomy/hooks/lane-stop-gate.test.sh -> PASS=103 FAIL=0 (unprivileged) -> PASS=102 FAIL=0 as root, case 51 skips visibly bash plugins/autonomy/hooks/lane-notify.test.sh -> rc=0 bash plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh -> rc=0 (calls lane-stop-gate-arm.sh) bash plugins/claude-ops/skills/lanes/scripts/restart-consumer.test.sh -> rc=0 bash scripts/affected-tests.sh --explain -> the R4 transitive walk from lane-stop-gate.sh fans out to the whole shell corpus (170 suites); plugin.json, CHANGELOG.md, README.md recorded as no-suite (non-shell lanes) bash scripts/affected-tests.sh --run (foreground) -> rc=1: one failing suite, plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh ("process budget: the trace probe actually counted something ... measured -1, the pid-stamped PS4 did not reach the traced shell"). Reproduced identically on a pristine `git archive origin/main` export: pre-existing and environmental, not from this change. Every other selected shell suite passed. shellcheck <3 changed .sh files> -> clean shfmt -d <3 changed .sh files> -> clean bash scripts/check-shell-portability.sh origin/main -> No unexcused GNU-only constructs in 3 shell file(s). node_modules/.bin/markdownlint-cli2 README.md CHANGELOG.md -> 0 issues typos <5 changed files> -> clean scripts/check-hook-exec-form.sh, check-killswitch-hoist.sh, check-silent-skips.sh, check-discriminating-test-skips.sh, check-hook-wiring-liveness.sh, check-purged-em-dashes.sh -> all rc=0 bash scripts/check-changelog-parity.sh --check -> rc=0 bash scripts/check-changelog-parity.sh --check-order -> rc=0 bash scripts/check-changelog-parity.sh --check-bump origin/main -> rc=0 bash scripts/check-changelog-parity.sh --check-preserved origin/main -> rc=0 (78 headings compared) ``` Manifest bumped 0.22.29 to 0.22.30 with a matching changelog entry. Not mine and pre-existing: `test_save_point.py::test_new_origin_falls_back_to_directory_name` (Python, in the NOT RUN ecosystem list) and the `cache-content-check.test.sh` trace-probe failure above. ## Related - Parent campaign: #3508 (Windows process-creation tax). Its corrected criteria ask for spawn counts before and after by trace, and the hook-budget bar; both are reported above. - Precedent for the in-file approach: #3779 (context-guard shard #3520), which located the cost in redirection placement rather than per-field `jq`, and merged PR #3788, which applied it across 34 hooks without touching `lib/hook-utils.sh`. - #3740 and #3838 own `lib/hook-utils.sh` and its 17 synced copies; this PR does not touch them. The enabled path's remaining shared-library share (4 of 10 creations) is theirs; the trace test's ceiling leaves it room. - Prior art #1403 / #1385 (`hook::jq_fields`, `strip_quoted_spans`, the 65-spawn census). `hook::jq_fields` is used here as it already exists on `main`; nothing from #1385 is revived. - Budget authority: `docs/conventions/hook-budget/README.md`, surfaced by `.claude/rules/hook-budget.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…ing them when no telemetry sink is set (#3913) Closes #3862 ## Summary Guard decisions left the process only through `HOOK_TELEMETRY_SINK`, which is inert unless an environment variable names an executable. On an ordinary install every decision was discarded as it was made, so "why was this denied", "has it denied this all along", and "did the guard run at all" had no evidence to answer from. New `plugins/disk-hygiene/lib/guard_decision_log.py` appends one JSON object per line to `<CLAUDE_PLUGIN_DATA>/guard-decisions/decisions.jsonl`, default on, no configuration. `destructive_guard.py` records every branch that reaches a verdict; `guard_launch_monitor.py` records the did-not-run state the guard structurally cannot write about itself. Version taken: **0.23.0**. Verified free against current `main` (`6db96637d`, 0.21.9) and against the head of every open PR at the moment of opening: #3880 claims 0.21.10, #3783 claims 0.22.0, and #3851/#3887/#3900/#3779 leave the manifest at 0.21.9. 0.23.0 is strictly greater than all of them, so it cannot collide under any merge order; the 0.22.x gap is what `--check-order` explicitly reads as correctly ordered. ## Fix **Where the record lives, and why it survives.** Under the plugin's own persistent data root, resolved by the guard's existing `resolve_authorized_data_root()` (the `--authorized-data-root` / `--plugin-root` / `CLAUDE_PLUGIN_DATA` ladder) beside the run records already kept there. Because this plugin is the one that deletes things, the filename was checked against the engine's own discovery hints in `reference/baseline-policy.json`: `decisions.jsonl` and `decisions.previous.jsonl` match none of the 14 bundled name globs (`*.tmp`, `tmp-*`, `tmp_*`, `scratch*`, `*.lock`, `__pycache__`, `*.partial`, `*.crdownload`, `*.tmp.*`, `.claude.json.tmp.*`, `temp_git_*`, `.pulumi-write-test-*`, `.DS_Store`, `Thumbs.db`), so the plugin's own hints never nominate its audit trail. Appending directly rather than writing a temp file and renaming is deliberate for the same reason: an atomic-write staging name would land on `*.tmp.*`, which is a bundled hint. **What is recorded.** `schema_version`, `timestamp` (UTC, milliseconds), `hook`, `decision`, `rule`, `tool`, `mode`, `command`, `reason`. `decision` is `allow` / `ask` / `deny` / `none` (ran, issued no `permissionDecision`) / `not-run`. `rule` names the branch that fired, so `kill-switch-disabled-apply` is distinguishable from `not-exact-engine-command`: two different answers to "why". `command` is the input that drove it and `reason` is the exact text the host was given, so the record and the host cannot disagree. Both are clipped to 400 characters, which keeps it a record of the decision rather than a copy of the payload and keeps every line short enough that concurrent hook processes appending to the same file do not interleave. **Bounded, enforced.** The live file rotates to `decisions.previous.jsonl` at 1 MiB via `os.replace`, so the record occupies at most about 2 MiB forever with no operator pruning. The bound is checked from the offset the append already returns (`handle.tell()` in append mode), so enforcing it costs no extra syscall. **Cost.** Measured with `strace -f -e trace=clone,clone3,fork,vfork,execve,openat,write` against a detached worktree of `origin/main` at `6db96637d`, five invocations per arm plus a warm-path detail run: | Path | Before | After | | --- | --- | --- | | defer (a Bash command not naming the engine, the always-on branch) | 1 `execve`, 1 `clone3` | 1 `execve`, 1 `clone3`, 0 record syscalls | | decision (deny), warm data root | 1 `execve`, 1 `clone3` | 1 `execve`, 1 `clone3`, 1 `openat` + 1 `write` | | decision (deny), first write of an install | 1 `execve`, 1 `clone3` | plus 1 failed `openat` and 1 `mkdir` | The single `clone3` is `CLONE_THREAD`, the existing watchdog thread, not a process. **The process and exec census is unchanged on every path.** The plugin-level defer branch, which is what this always-on hook takes for work unrelated to disk-hygiene, writes nothing at all and is byte-for-byte the path it was. Wall clock over 40 invocations per arm, alternated twice, moved inside run-to-run noise on this host (52 to 58 ms both before and after, the sign of the difference changing between repetitions), which is why the syscall census rather than a duration is the figure cited. **Failure behavior: the verdict never changes.** Two boundaries, both load-bearing. `guard_decision_log.record` returns a bool and catches `BaseException` around the whole write. `_record_decision` in the guard wraps its own call, because the data root and mode are resolved in the argument list, outside `record`'s protection, and one of its call sites is `main`'s own `except BaseException` handler, where a raise would reach the interpreter's default handler: exit 1, which PreToolUse treats as non-blocking, so the command the guard just denied would run. Every record call is made after the verdict has been emitted, and its result is discarded. **What is deliberately not recorded.** The plugin-level defer (hot path, and not a decision anyone reconstructs later). The watchdog expiry path: that callback runs while the main thread is presumed wedged inside a filesystem call and stays syscall-free for exactly that reason, so a write there could hang on the same filesystem. Both are stated in the README rather than left implicit. **Adjacency, stayed out of.** #3861 (the guard's fail-open when no interpreter resolves) is `needs-human`. This change touches the guard's decision branches but not interpreter resolution, and adds no new fail-open path; the `not-run` record makes the fail-open class more visible after the fact without adjudicating it. **Escape hatch.** `DISK_HYGIENE_GUARD_DECISION_LOG` set to `0` / `off` / `false` / `no` turns the record off. Opt-out, not opt-in: any other value, including an absent one, records. ## Verification All runs local and in the foreground; draft CI is not cited as test evidence. - `bash scripts/affected-tests.sh --run --shard N/4`: shards 0, 1, 2 exit **3** (success, with `NOT RUN` non-shell ecosystems), 0 `FAIL` lines each. Shard 3 exits 1 with exactly three `FAIL` lines, all from `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (`process budget: the trace probe actually counted something`, `process budget: a one-install report costs at most 26 process creations`). **Reproduced unchanged on a clean detached worktree of `origin/main` at `6db96637d`**: same suite, same 2 cases, exit 1. Pre-existing, not this change. Every changed file maps to at least one suite; the three docs/manifest files resolve through the recorded no-suite allowlist. - Direct suites: `hygiene.test.sh` RC=0 (350 cases), `guard_launch_monitor.test.sh` RC=0 (28 cases), `run-python-hook.test.sh` RC=0, `test_guard_decision_log.py` + `test_hook_telemetry.py` RC=0 (15 new cases). - All four parity modes green: `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`. - `scripts/run-ruff.sh check plugins/disk-hygiene`: all checks passed. `format --check`: my four touched/new Python files are clean. Five files remain unformatted in this plugin (`killswitch_config.py`, `hygiene.py`, `guard_launch_monitor.py:137`, `test_hygiene.py:2373`, `test_kill_switch_probe.py:96`); all five are identically unformatted on `origin/main`, so none is introduced here. - Gates run clean: `check-purged-em-dashes.sh`, `check-drive-root-litter.sh`, `check-silent-skips.sh`, `check-discriminating-test-skips.sh`, `check-fixture-git-isolation.sh`, `check-hook-exec-form.sh`, `check-killswitch-hoist.sh`, all RC=0. New test file committed `100755` (verified with `git ls-tree`), the new library `100644` matching its sibling `hook_telemetry.py`. - No shell files changed, so shellcheck and shfmt have nothing to say about this diff. `lib/hook-utils.sh` untouched. **Mutation proof (the new assertions discriminate).** Nine mutations applied one group at a time, each reverted: | Mutation | Caught by | | --- | --- | | rotation call disabled | 3 lib cases (`rotates_at_the_bound`, `discards_only_the_generation_before_last`, `a_failing_rotation...`) | | `_clip` returns text unchanged | `long_command_and_reason_are_truncated` | | `enabled()` hardcoded True | 5 lib subtests + `the_record_can_be_turned_off_without_changing_a_verdict` | | deny rule string collapsed onto the kill-switch rule | `denied_engine_command_is_recorded_with_its_rule_and_input` | | a record added on the defer path | `engine_gate_defer_records_nothing` | | `_record_decision`'s try/except removed | `a_broken_decision_record_never_changes_a_verdict` (record raises), `record_decision_swallows_a_failure_in_data_root_resolution`, and the **pre-existing** `every_call_graph_function_failure_denies_at_exit_2_never_1` for both `resolve_mode` and `resolve_authorized_data_root` | | `_record_not_run` call removed from the monitor | 3 monitor cases | The write-failure proof is `test_a_broken_decision_record_never_changes_a_verdict`, which drives all five verdict shapes (`allow`, `ask`, `deny`-by-authority, `deny`-by-kill-switch, and the no-output defer) three times: unsabotaged, against a data root whose parent is a regular file (a real filesystem `OSError` on both the append and the `mkdir` behind it), and with `record` raising `RuntimeError`. All three runs produce the identical verdict list, and the unwritable root is asserted to still not exist afterwards. `test_a_write_failure_leaves_the_deny_exit_status_untouched` pins the same thing end to end through `main`. **Hermeticity fix included.** `run_guard_engine_gate` previously passed `SCRIPT_DIR / "data-root"` as the authorized data root. With records being written, that would have littered the checkout on every test run, so it now takes a per-test temp path, and `GuardTests.setUp` pops an inherited `CLAUDE_PLUGIN_DATA` so a developer's real plugin data directory is never written to by the suite. ## Related - Closes #3862; parent #3347 finding F12. - Sibling #3861 (guard fail-open when no interpreter resolves) is `needs-human` and deliberately untouched; the `not-run` record is what makes that class visible after the fact. - Sibling finding on a false denial: `rule` plus `command` plus `reason` is what answers it from the record instead of by reproduction. - Hook budget convention: `docs/conventions/hook-budget/README.md`, `.claude/rules/hook-budget.md`. Measured share stated in the plugin README's trust-surface record. - Version contention checked against open PRs #3783 (0.22.0) and #3880 (0.21.10). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
… off substitutions The Stop-event unsurfaced-failure detector timed out 113 times in the #3508 window at a 21.9 s average, roughly 44x the 500 ms the hook-budget convention gives the whole always-on per-turn set. The parent issue blames per-field jq forks needing a shared helper; that diagnosis is wrong here, as shard #3520 and PR #3788 established twice. The cost is redirection placement. Bash runs the command of a command substitution in the substitution's own subshell and skips the extra fork ONLY when that command carries no redirection of its own. `$(wc -c <file 2>/dev/null)` is two processes for one byte count; `$(cat -- file 2>/dev/null)` is two for a file read; `{ V=$(cmd); } 2>/dev/null` is one. Those forks are invisible to `bash -x`, which reads command positions, which is why the campaign kept mis-attributing the cost to the exec count. Six sites in hook-failure-audit.sh change and no shared library does: - `wc` names the transcript instead of redirecting it in, and `read` drops the filename column along with the padding some wc builds add. - The `read_window` helper is gone. Under the tail cap grep opens the transcript itself rather than being fed by `cat` through a function call that was a second subshell on top of the substitution's own. - The marker read is `$(<file)`, which forks nothing and execs nothing. - The marker write strips carriage returns in the shell instead of piping through `tr`. - The marker directory is probed with `-d` before `mkdir -p` spawns. - The two payload fields come from one `hook::jq_fields` pass instead of two `hook::jq_field` calls. Over the tail cap the `tail | sed | grep` pipeline and its redirect placement are untouched: a pipeline element forks either way, and hoisting the redirect onto a group would newly silence sed and grep for no saving. The `printf | jq` feeding the structural selection also stays a pipeline, because a here-string at or above the pipe capacity deadlocks before jq is exec'd and that payload routinely clears it; it is off the common path regardless. Common path (a turn with no failure recorded, under the cap): 18 process creations and 6 execs before, 9 and 4 after. Over the cap: 19 and 7 before, 12 and 6 after. A turn that warns: 34 and 17 before, 24 and 14 after. Measured with `strace -ff -e trace=clone,clone3,fork,vfork,execve`. No wall-clock figure is claimed: this Linux host says nothing about the Windows spawn tax the budget binds to, and on the #3508 host one creation costs 180-2,841 ms. Behaviour is unchanged, which for this hook is the whole point: it exists to surface failures that otherwise pass unnoticed, so a hoisted `2>/dev/null` could silence exactly the diagnostic it is for. Every group holds one command, and the one stream now silenced that was not before is grep's, which `cat`'s own redirect already discarded. Pre- and post-change stdout, stderr, exit codes and marker contents were compared byte for byte across ten scenarios: first warn, dedup, a new failing registration re-warning while the warned one stays muted, all three classification branches, the tail cap, a clean transcript, a missing transcript, the kill switch, and no data directory. Identical. The contract test gains an strace budget assertion on both counts, mutation- checked: moving either silenced redirect back inside its substitution adds a fork with no new exec, leaves every behavioural assertion green, and trips the creation ceiling. The README states the measured share per hook-budget Rule 1, as a process count, and says plainly that the Windows parallel-wall figure is still owed. Two acceptance criteria are not met and are called out in the PR: the hook cannot exit before any spawn, since reading the transcript is how it learns there is nothing to report, and the 500 ms parallel-wall figure for the three-plugin per-turn set is cross-plugin and Windows-only. Refs #3508. Precedent #3779, #3788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
…3513) (#3869) Closes #3513 ## Summary `block-hook-bypass.sh` was the largest process creator of any guard in the plugin on a benign Bash call: 7 creations, 0 execs. Every exec-based census (the PATH-shim spawn census, `run-guards.test.sh`'s dirname/sed pin, an xtrace command-position count) reads a fork-without-exec as free, which is why the issue's numbers and the "per-field jq" cause never matched what the kernel does. Six of the seven were in this file; the seventh is `$(hook::buffer_stdin)`. Measured with `strace -f -e trace=clone,clone3,fork,vfork,execve` on the real dispatched path (`run-guards.sh block-hook-bypass.sh`), `HOOK_TELEMETRY_SINK` unset, guard share = count minus a no-op guard dispatched the same way, three identical repeats: | Path | creations before -> after | execve before -> after | |---|---|---| | Guard share, benign `git status --short` | 7 -> 1 | 0 -> 0 | | Guard share, blocked `echo hi > notes.md` | 10 -> 5 | 1 -> 1 | | Whole Bash dispatcher, benign | 36 -> 30 | 3 -> 3 | | Guard alone under the dispatcher, wall p50 / p95 (n=20, interleaved) | 27.4 / 29.2 ms -> 24.3 / 27.5 ms | | | Whole Bash dispatcher, wall p50 / p95 (n=20, interleaved) | 51.6 / 60.4 ms -> 48.2 / 49.6 ms | | The execve column does not move: this is latency, not removed work. **On the issue's "at most two spawns" line.** The one creation left in the guard's own share is `$(hook::buffer_stdin)`. Its fork-free form (`hook::buffer_stdin_to <var>`) belongs to `lib/hook-utils.sh`, which is fenced by unmerged #3740 and #3838, so it is not touched here. Whether #3513 counts as met depends on what one counts: the guard's in-file contribution is now 1 creation and 0 execs, and the dispatcher's own `$(source …)` isolation fork is the dispatcher's, not this file's. `Closes` here means the in-file work is done; the remaining fork is the hook-utils item. ## Fix Five in-file creations removed, one left in place: 1. `:159` eager `SUBJECT=$(hook::extract_bash_subject …)` at file scope. Only `emit_tel` read it, and `emit_tel` is gated on the start stamp and the opt-in sink. The subject is now derived inside `emit_tel` behind both gates. Verdicts never read it. 2. `:478` `EXECUTABLE=$(strip_literals "$COMMAND")`. Now `strip_literals_to EXECUTABLE "$COMMAND"`, assigning through a nameref; every trailing newline is stripped, as `$(…)` stripped them. 3. `:474`, `:1305`, `:1330`, `:1403` the four `done < <(printf '%s\n' …)` loops. One fork-free splitter, `split_lines_to`, does a sentinel-prefixed `IFS=$'\n'` word split under `set -f` (globbing restored to whatever the caller had). The sentinel prefix is what keeps blank lines and runs of newlines from collapsing, and a leading or trailing empty line from disappearing, so the array is exactly what `read` delivered, which `strip_literals` needs to keep its heredoc and open-quote state aligned with physical lines. `NORMALIZED_SEGMENTS` becomes an array filled once in `normalize_segments`; the three per-segment scans iterate it. `return` and `continue 2` inside those loops reach the same scopes as before because neither loop shape runs its body in a subshell. A here-string was not an option: at 65536-65663 bytes bash blocks forever writing it into the pipe (documented in `lib/path-detection/hardcoded-path-patterns.sh`). 4. `:112` `INPUT=$(hook::buffer_stdin)` is left as is. It is the fenced hook-utils item, and #3740 modifies this exact rc-handling block; the diff stays off it so the two do not fight at merge. Manifest 0.32.10 -> **0.32.12**, CHANGELOG entry, README "Hook budget accounting" entry with the method and table above. 0.32.11 is taken by #3849 (issue #3511), which bumps the same manifest off the same 0.32.10 base; this branch takes the next free number so the two do not collide on `plugins/guardrails/CHANGELOG.md`. Two documentation corrections carried in the same renumber commit: - The CHANGELOG entry and the new README row said "the 602-case contract suite passes". With the pins this change set adds the suite is 611 cases, which is what it reports. - The comment moved onto `emit_tel` carried two em dashes over from the file-scope comment it replaced. `.claude/rules/vendor-docs-are-not-style.md` bars them from this repo's instruction surfaces, and `check-purged-em-dashes.sh` scans markdown only, so nothing caught them. They are parentheses now. No other comment is touched, and no guard logic changed. ## Verification - **Deny paths still deny, compared against `origin/main`.** 244 paired runs (61 commands x Bash/PowerShell payloads x standalone/dispatched, plus a 70 KiB single-line command and a 3000-line command on each side) agree on exit code and first stderr line. The corpus covers every documented deny form (cat/echo/printf redirects, no-space forms, heredoc openers with a trailing redirect, `2>&1`/`>&2` before the file, prefix modifiers `command`/`exec --`/`FOO=bar`, group and `if` headers, leading redirects, discard-then-real-file `> /dev/null > real.txt`, python write indicators, staged `mv`/`cp`, quoted operands with embedded separators, `\;` and backslash-newline escapes, `ec""ho` / `ec"xy"ho` splices) and the allow forms (dup-only redirects, producer-scoped `bash x.sh > out && echo done`, quoted prose in commit and PR bodies, `command -v`, comments, blank-line-only and newline-padded commands, glob characters). - `split_lines_to` checked directly against `while IFS= read -r` over `printf '%s\n'` on: empty, single line, trailing newline, leading newline, runs of blank lines, whitespace-only lines, glob characters, the sentinel byte itself at line starts and ends, IFS characters, trailing backslashes, UTF-8, a 70 KiB line, 3001 lines. Identical arrays in every case; `set -f` preserved when the caller had it and restored off when it did not. - **New strace-based budget test** in `block-hook-bypass.test.sh`: pins the guard's benign share at exactly 1 creation and 0 execve on two benign payloads (single segment; multi-line, multi-segment), and that `echo > file` still exits 2 under the tracer. Skips visibly on a host without a working strace. It runs the real dispatched path, not the standalone guard, and subtracts a no-op guard so dispatcher forks do not leak into the pin. **Mutation-checked**: unmutated control 9/9; each of the four changes reverted alone (eager SUBJECT restored; splitter through `mapfile < <(…)`; strip through `$(…)`; segments through `mapfile < <(…)`) fails the pin on both payloads. - **The census was re-taken after the renumber commit**, since a documentation-only change must not move it, and it does not: whole Bash dispatcher on the benign payload reads 36 creations at `origin/main` against 30 on this branch, 3/3 identical repeats, execve flat at 3 on both; the blocked-path guard share is 5 creations and 1 execve; the contract suite's exact benign-share pin (1 creation, 0 execve) passes. - `scripts/affected-tests.sh --run`: 5 suites selected (`block-hook-bypass.test.sh` 611/611, `flag-commit-pr-skill-bypass.test.sh`, `require-jq-posture.test.sh`, `run-guards.test.sh`, `scripts/check-shell-portability.test.sh`), all passed, exit 0; the manifest, CHANGELOG and README map to the recorded no-suite allowlist. - `scripts/check-changelog-parity.sh` **all four modes** the script defines, on the committed tree: `--check` (pass), `--check-order` (pass, 91 changelogs read newest-first with no duplicate versions), `--check-bump origin/main` (pass), `--check-preserved origin/main` (pass, 155 headings preserved). `--check-order` is the mode that catches a doubled `## [<version>]` after a bad conflict resolution, which is the failure the renumber exists to avoid; an earlier revision of this PR ran only the other three. - `shellcheck -x` clean on the guard and the test; `shfmt -d` no diff; markdownlint-cli2 0 issues; editorconfig-checker clean; `check-purged-em-dashes.sh` clean; ai-slop detector 0 findings on the README and CHANGELOG. - Pre-existing and not touched: `test_save_point.py::test_new_origin_falls_back_to_directory_name`. ## Related - #3508 parent campaign. Its stated cause (per-field `jq` forks needing a shared helper) does not describe this guard: since #3788 the dispatcher primes the fields once, and this guard contributed 0 execs before this change. - #3779 precedent (#3520): an xtrace command-position count read 2 where the kernel made 8; the same instrument gap is why the shim-based budget test could not see any of these seven. - #3740 adjacent: modifies this script's `buffer_stdin` rc handling. This diff stays off that block. #3838 is the other fence on `lib/hook-utils.sh`. - #3849 (issue #3511) holds guardrails 0.32.11; this PR now takes 0.32.12, so the two no longer collide. - Credit note for the record: this file's `dirname` exec died in `a5f5e799` (#3781); #3788 never touched this file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
… off substitutions The Stop-event unsurfaced-failure detector timed out 113 times in the #3508 window at a 21.9 s average, roughly 44x the 500 ms the hook-budget convention gives the whole always-on per-turn set. The parent issue blames per-field jq forks needing a shared helper; that diagnosis is wrong here, as shard #3520 and PR #3788 established twice. The cost is redirection placement. Bash runs the command of a command substitution in the substitution's own subshell and skips the extra fork ONLY when that command carries no redirection of its own. `$(wc -c <file 2>/dev/null)` is two processes for one byte count; `$(cat -- file 2>/dev/null)` is two for a file read; `{ V=$(cmd); } 2>/dev/null` is one. Those forks are invisible to `bash -x`, which reads command positions, which is why the campaign kept mis-attributing the cost to the exec count. Six sites in hook-failure-audit.sh change and no shared library does: - `wc` names the transcript instead of redirecting it in, and `read` drops the filename column along with the padding some wc builds add. - The `read_window` helper is gone. Under the tail cap grep opens the transcript itself rather than being fed by `cat` through a function call that was a second subshell on top of the substitution's own. - The marker read is `$(<file)`, which forks nothing and execs nothing. - The marker write strips carriage returns in the shell instead of piping through `tr`. - The marker directory is probed with `-d` before `mkdir -p` spawns. - The two payload fields come from one `hook::jq_fields` pass instead of two `hook::jq_field` calls. Over the tail cap the `tail | sed | grep` pipeline and its redirect placement are untouched: a pipeline element forks either way, and hoisting the redirect onto a group would newly silence sed and grep for no saving. The `printf | jq` feeding the structural selection also stays a pipeline, because a here-string at or above the pipe capacity deadlocks before jq is exec'd and that payload routinely clears it; it is off the common path regardless. Common path (a turn with no failure recorded, under the cap): 18 process creations and 6 execs before, 9 and 4 after. Over the cap: 19 and 7 before, 12 and 6 after. A turn that warns: 34 and 17 before, 24 and 14 after. Measured with `strace -ff -e trace=clone,clone3,fork,vfork,execve`. No wall-clock figure is claimed: this Linux host says nothing about the Windows spawn tax the budget binds to, and on the #3508 host one creation costs 180-2,841 ms. Behaviour is unchanged, which for this hook is the whole point: it exists to surface failures that otherwise pass unnoticed, so a hoisted `2>/dev/null` could silence exactly the diagnostic it is for. Every group holds one command, and the one stream now silenced that was not before is grep's, which `cat`'s own redirect already discarded. Pre- and post-change stdout, stderr, exit codes and marker contents were compared byte for byte across ten scenarios: first warn, dedup, a new failing registration re-warning while the warned one stays muted, all three classification branches, the tail cap, a clean transcript, a missing transcript, the kill switch, and no data directory. Identical. The contract test gains an strace budget assertion on both counts, mutation- checked: moving either silenced redirect back inside its substitution adds a fork with no new exec, leaves every behavioural assertion green, and trips the creation ceiling. The README states the measured share per hook-budget Rule 1, as a process count, and says plainly that the Windows parallel-wall figure is still owed. Two acceptance criteria are not met and are called out in the PR: the hook cannot exit before any spawn, since reading the transcript is how it learns there is nothing to report, and the 500 ms parallel-wall figure for the three-plugin per-turn set is cross-plugin and Windows-only. Refs #3508. Precedent #3779, #3788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
…stitutions (#3849) Closes #3511 ## Summary `plugins/guardrails/hooks/block-convention-violation.sh` carried three `2>/dev/null` redirects **inside** command substitutions. GNU Bash execs the body of a command substitution in the substitution's own subshell, instead of forking a second time, only when that body carries no redirection of its own (Command Substitution, Bash Reference Manual; https://mywiki.wooledge.org/CommandSubstitution). So `v=$(cmd 2>/dev/null)` costs two process creations to run one program where `{ v=$(cmd); } 2>/dev/null` costs one. On the Windows Git Bash hosts #3508 measures, a fork is a full process creation at 0.3-0.9 s. **The parent's stated cause does not apply to this file.** #3508 attributes the cost to per-field `jq` forks needing a shared helper. This guard already batches all three payload fields through `hook::jq_fields`, already derives its own directory with `${BASH_SOURCE[0]%/*}` instead of `$(dirname …)`, and since #3788 is `source`d in-process by `run-guards.sh`, which buffers stdin and primes the jq fields once for the whole batch. What was left is redirection placement, the same cause shard #3520 established in PR #3779 and PR #3788 fixed across 34 scripts without touching `lib/hook-utils.sh`. This PR touches no shared library and no dispatcher: the change is in-file. ## Fix Three redirects hoisted onto single-command groups: | Site | Reached on | |---|---| | `bash "$RESOLVER" …` convention resolver (two calls) | first commit or `gh pr create` after a cache miss | | `git rev-parse --absolute-git-dir` sequencer probe | every stdin-form commit | | `git config --get alias.<sub>` alias probe | every non-builtin git subcommand, so this one is on the per-tool-call path | Each group holds **exactly one command**, so the group's status is still that command's. That matters here specifically because this is a *gate*: hoisting `2>/dev/null` over a multi-command group can swallow a non-zero status and turn a block into a silent allow. `|| conv_val=""` and `|| return 1` fire on exactly the failures they fired on before. ## Verification **Process counts, from the kernel** (`strace -f -e trace=clone,clone3,fork,vfork,execve`, `HOOK_TELEMETRY_SINK` unset). A PATH shim cannot see a fork that never execs, and `bash -x` prints one line whether a command costs one process or two, so neither existing counter in this repo can measure this. Guard invoked directly: | Scenario | creations before | after | execve before | after | |---|---|---|---|---| | `echo hello` | 11 | 11 | 3 | 3 | | `git status --short` | 11 | 11 | 3 | 3 | | `git wibble --x` (alias probe) | 14 | **13** | 4 | 4 | | stdin-form commit, cold cache | 34 | **31** | 16 | 16 | | stdin-form commit, warm cache | 19 | **18** | 5 | 5 | Guard's incremental cost inside `run-guards.sh`, the way it actually runs (full Bash matcher guard list, measured with and without this guard in the list, warm convention cache): | Scenario | guard's added creations before | after | guard's added execve | |---|---|---|---| | `git status --short` | 3 | 3 | 0 | | `git wibble --x` | 6 | **5** | 1 | | stdin-form commit | 11 | **10** | 2 | `execve` is unchanged everywhere, which is the evidence this removes latency rather than removing work. Wall-clock is not the claim: this Linux host's spawn floor is nothing like the contended Windows host in #3508, and inventing a millisecond figure from it would mislead. **Deny paths still deny.** All three touched sites were exercised against a fixture repo carrying a tracked `subject_pattern`: - violating commit subject blocked (exit 2), conforming allowed (exit 0) — proves the resolver site still returns a pattern - violating `gh pr create --title` blocked (exit 2) - `git qc` where `alias.qc = commit` blocked (exit 2) — proves the alias-probe site still resolves the alias, which fails **open** if it breaks - commit during an in-progress merge (`MERGE_HEAD` present) allowed (exit 0) — proves the sequencer-probe site still detects the sequencer, which is the exemption the probe exists for **New test, mutation-checked.** `block-convention-violation.test.sh` gains a per-site kernel-trace budget assertion: for each external command this file starts, the process that execs it must have a parent that itself execve'd. A parent that never execs and has exactly one child is the wasted fork. Each site must appear in the trace, so a scenario that stops reaching a site fails rather than passing by absence. Moving each redirect back inside its substitution, one at a time, failed exactly its own assertion and no other (3 runs, `PASS=80 FAIL=1` each). The block skips loudly, printing `skip: strace is unavailable…`, where `strace` is absent or not permitted, so it is a Linux-CI guard and not a Windows one. **Gates.** `scripts/affected-tests.sh --run` exit 0, all 5 selected suites pass (`block-convention-violation.test.sh` PASS=81 FAIL=0, `run-guards.test.sh` PASS=101, `require-jq-posture.test.sh`, `lib/resolve-convention-pattern.test.sh`, `commit-msg-convention.test.sh` PASS=15). `shellcheck -x` clean, `shfmt -d` clean, `check-shell-portability.sh --paths` clean, `check-killswitch-hoist.sh` clean, `check-changelog-parity.sh` clean in all four modes (`--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`). Manifest 0.32.10 → 0.32.11 with the matching CHANGELOG entry. The prettier warning on `plugins/guardrails/CHANGELOG.md` is pre-existing at `origin/main` and untouched. ### Acceptance criteria not fully met — stated plainly | Criterion from #3511 | Status | |---|---| | Subprocess `grep`/`sed`/`cut`/`tr`/`basename`/`dirname` on the hot path replaced with builtins | **Already true before this PR.** None of them appear on any path of this file. | | Early guard clause exits before any spawn for non-matching invocations | **Already true before this PR** (#3820 deferred the convention load). `git status --short` and `echo hello` add **zero** execs from this guard. | | Existing behavioural tests pass; the guard blocks what it blocked before | **Met.** | | No more than 2 external process spawns on its common path (own shell plus at most one `jq`) | **Not met as literally written, and not achievable in-file.** Run standalone the guard costs 3 execs on the common path: 2 `jq` and the telemetry sink, all inside `lib/hook-utils.sh`. In the batch it actually runs in, it adds **0** execs to the common path and 3 pure forks, and those forks are the dispatcher's per-guard `$(source …)` isolation fork plus `hook-utils` internals. Both files are out of scope here: `hook-utils.sh` is synced across 17 plugin copies with unmerged work in flight (#3740, #3838), and the isolation fork is #3685. | | A single run completes in under 2 s on a Windows host | **Unverified.** No Windows host in this session. The comment thread on #3511 also supersedes this figure with the repo's own budget (`docs/conventions/hook-budget/README.md`: <= 1 s typical / <= 2 s worst case per tool call, parallel wall). | This PR does not on its own bring the guard under the hook budget on the host in #3508. It removes the cost this file owns and can fix without touching a fenced shared library. ## Related - Parent: #3508 (Windows process-creation tax), whose stated cause is corrected above - Precedent for the diagnosis: #3520, PR #3779 (found redirection placement is the real cost), PR #3788 (fixed 34 scripts across 17 plugins, `lib/hook-utils.sh` untouched) - Prior work on this file: #3820 (deferred the convention load off the benign path) - Out of scope and named above: #3685 (per-guard isolation fork), #3740 and #3838 (unmerged `hook-utils.sh` work) - Siblings sharing this plugin's manifest and CHANGELOG: #3521, #3529 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ Co-authored-by: Claude <noreply@anthropic.com>
… off substitutions (#3851) Closes #3512 ## Summary The Stop-event unsurfaced-failure detector (`plugins/claude-ops/hooks/hook-failure-audit.sh`) timed out 113 times in the #3508 window at a 21.9 s average, roughly 44x the 500 ms the hook-budget convention gives the whole always-on per-turn set. **The parent issue's stated cause is wrong for this hook.** #3508 blames per-field `jq` forks needing a new shared helper. Shard #3520 (PR #3779) established that the real cost is *redirection placement*, and merged PR #3788 fixed 34 hook scripts across 17 plugins while touching `lib/hook-utils.sh` zero times. This shard applies the same diagnosis. **No shared library is touched** — the fence around `lib/hook-utils.sh` and its 17 synced copies (unmerged #3740, #3838) holds. The mechanism: bash runs the command of a command substitution in the substitution's own subshell and skips the extra fork **only when that command carries no redirection of its own**. `$(wc -c <file 2>/dev/null)` is two processes for one byte count; `$(cat -- file 2>/dev/null)` is two for a file read; `{ V=$(cmd); } 2>/dev/null` is one; `$(<file)` is zero. Those forks are invisible to `bash -x`, which reads command positions — which is why the campaign kept mis-attributing the cost to the exec count. ## Fix Six in-file sites, all in `hook-failure-audit.sh`: | Site | Before | After | |---|---|---| | Transcript size | `SIZE=$(wc -c <"$TRANSCRIPT" 2>/dev/null)` | `wc` names the file, `read` drops the filename column and any padding, `2>/dev/null` rides a single-command group | | Pre-filter, under the cap | `read_window \| grep -F …`, where `read_window` was `cat -- file 2>/dev/null` | `{ RECORDS=$(grep -F … -- "$TRANSCRIPT"); } 2>/dev/null` — grep opens the transcript itself; the helper function was a second subshell on top of the substitution's own | | Marker read | `$(cat -- "$MARKER" 2>/dev/null)` | `$(<"$MARKER")` — no subshell, no exec | | Marker write | `jq … 2>/dev/null \| tr -d '\r' >>"$MARKER"` | `$(jq …)` then in-shell `${VAR//$'\r'/}` with a builtin `printf` append | | Marker directory | `mkdir -p "$MARKER_DIR" 2>/dev/null` every warned turn | `[[ -d … ]] ||` first; `mkdir -p` on an existing directory was a whole process to reach the same no-op | | Payload fields | two `hook::jq_field` calls | one `hook::jq_fields` call (the batching helper the library already ships, used by 10 other hooks) | The CHANGELOG entry originally said "Five sites" over that same list of six; it now says six. Deliberately **not** changed, both documented in the file: - **Over the tail cap**, `tail | sed '1d' | grep` and its `2>/dev/null` on `tail` are untouched. A pipeline element forks either way, so hoisting the redirect onto a group would newly silence `sed` and `grep` for no saving, and merging `sed` into `grep` would change the matching semantics. - **`printf '%s' "$RECORDS" | jq -cRs`** stays a pipeline rather than becoming a here-string, and the in-file comment now states the grounds as they actually stand. `hook::jq_field` in the shared library documents this hazard and refuses the here-string form for it: bash fills a here-string's pipe itself, so a payload at or above the pipe capacity can block before `jq` is exec'd. The trace behind that note (#1587: 65536 bytes hung indefinitely while 65000 returned at once) comes from this repo's **Windows Git Bash** hosts. It does **not** reproduce on Linux bash 5.2 — re-checked here at 65535, 65536, 65537, 200 kB and 2 MB, each returning immediately, including under an unwritable `TMPDIR`. The call keeps the library's conservative form anyway rather than bet the hazard is Linux-only: the forgone saving is one fork on the warning path only, since a turn with no failure record exits before that line. Earlier revisions of this PR and of the comment asserted the deadlock as universal fact, which is more than is known. ## Verification **Process counts**, measured with `strace -ff -qq -e trace=clone,clone3,fork,vfork,execve` (`-ff` so no syscall line is split across an `<unfinished>`/`<resumed>` pair), successful `execve` only, the harness's own top-level `bash <hook>` exec excluded: | Path | Creations before | after | execs before | after | |---|---|---|---|---| | No failure recorded, under the tail cap (the common case) | 18 | **9** | 6 | **4** | | No failure recorded, over the tail cap | 19 | **12** | 7 | **6** | | A turn that warns | 34 | **24** | 17 | **14** | The remaining 9 creations on the common path are 4 execs (`wc`, `grep`, and two `jq` passes inside the synced `hook-utils.sh`) plus subshell forks inside that same fenced library. The `execve` drop is one batched `jq` and two removed helper processes — removed *processes*, not removed work: every input is still read and every record still classified. **No wall-clock figure is claimed.** This Linux host says nothing about the Windows spawn tax the budget binds to, and on the #3508 host one process creation costs 180-2,841 ms (median 1,108 ms at 501 concurrent). The process count is the honest proxy and the README says so. **Behaviour is unchanged, proven, not asserted.** This hook's whole job is surfacing failures that otherwise pass unnoticed, so a carelessly hoisted `2>/dev/null` could silence exactly the diagnostic it exists to emit. Every hoisted group holds one command. The only stream now silenced that was not before is `grep`'s own stderr, and `cat`'s own `2>/dev/null` already discarded that same stream on the same path. Pre- and post-change stdout, stderr, exit codes **and marker-file contents** were compared byte for byte across ten scenarios — first warn, dedup, a new failing registration re-warning while the already-warned one stays muted, all three classification branches (launch / ambiguous / completed), the tail cap, a clean transcript, a missing transcript, the kill switch, and no data directory. Identical. **Budget test, mutation-checked.** `hook-failure-audit.test.sh` gains an strace-based assertion on both the creation and exec ceilings, skipping cleanly where strace is unavailable or not permitted. Non-vacuity was demonstrated, not assumed: moving either silenced redirect back inside its substitution — `SIZE=$(wc -c <"$TRANSCRIPT" 2>/dev/null)`, or `RECORDS=$(grep -F … -- "$TRANSCRIPT" 2>/dev/null)` — pushes creations from 9 to 10 with **no change to the exec count**, leaves all 93 behavioural assertions green, and fails the budget assertion. That is precisely the fork xtrace cannot see. **Gates, all foreground:** | Gate | Result | |---|---| | `scripts/affected-tests.sh --run` | exit 0, both selected suites pass (`hook-failure-audit.test.sh` 94/94, `audit-session-id.test.sh` 27/27) | | `shellcheck -x` on both changed shell files | clean | | `shfmt -d -i 2 -ci` | clean | | `scripts/check-changelog-parity.sh` `--check` / `--check-order` / `--check-bump origin/main` / `--check-preserved origin/main` | all four pass | | `markdownlint-cli2` on the changed markdown | clean | | `scripts/check-purged-em-dashes.sh` | pass | | `scripts/check-shell-portability.sh origin/main` | pass | | `scripts/check-silent-skips.sh`, `check-killswitch-hoist.sh`, `check-hook-exec-form.sh`, `check-hook-wiring-liveness.sh`, `check-cross-plugin-source-drift.sh`, `validate-plugins.sh` | pass | The wording-only follow-up commit re-ran the test suites, all four parity modes, `shellcheck -x`, `shfmt -d -i 2 -ci` and `markdownlint-cli2`: unchanged, including the 9-creation / 4-exec budget assertions, which is the expected result for a comment-and-prose change. `test_save_point.py::test_new_origin_falls_back_to_directory_name` is pre-existing and not in this diff's selection. ### Acceptance criteria not fully met Two of the issue's criteria are **not** satisfied, and no amount of in-file work would satisfy them: - **"A turn with no hook failures exits before any external process spawn."** Not met, and not reachable. The hook learns there is nothing to report *by reading the transcript*; there is no cheaper oracle. The floor without touching the fenced `hook-utils.sh` or abandoning the O(cap) tail bound is one `wc`, one `grep`, and the library's own payload parse — which is what this PR reaches. If the criterion is to be met literally it needs a different design (a marker written by the failing hook, or consolidation with #3515/#3516), not a further micro-optimization here. - **"The always-on per-turn set stays within <= 500 ms parallel wall."** Not measured. That figure is cross-plugin (this hook shares the budget with #3515 `autonomy` and #3516 `disk-hygiene`) and binds to Windows Git Bash, which this runner is not. The README records the figure as owed rather than implying it was taken. A third is met in a different form than specified: the issue asks for a **PATH shim** spawn census. This uses **strace** instead, which is strictly stronger for this defect — a PATH shim sees only programs that are `exec`'d, and the whole cost here is subshell forks that never exec anything. ## Related - Parent: #3508 (the campaign; its stated cause is corrected above for this hook) - Precedent: #3520 / PR #3779 (established redirection placement as the real cost), PR #3788 (34 scripts, 17 plugins, zero library edits) - Siblings on the same 500 ms per-turn budget: #3515 (`autonomy`), #3516 (`disk-hygiene`) - Fenced, deliberately untouched: #3740, #3838 (`lib/hook-utils.sh` and its 17 synced copies); the here-string note above cites `hook::jq_field` there but changes nothing in it - **Adjacency checked, no edit-surface overlap:** open PR #3769 adds correlation keys (`session_id`, `prompt_id`, `tool_use_id`, `agent_id`) to the hook-telemetry envelope spine inside `hook::emit_telemetry`. It does **not** touch `hook-failure-audit.sh`. This diff leaves that hook's `SESSION_ID` extraction and its `data.session_id` envelope key exactly as they are, so nothing here collides with the key migration. The two PRs both bump `plugins/claude-ops/.claude-plugin/plugin.json` and prepend to its CHANGELOG, so whichever lands second takes a trivial version/heading rebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…it (#3529) (#3872) Closes #3529 ## Summary `block-dangerous-git.sh` is the guard with the highest external-command call-site count in the marketplace, but on the dispatched path it executes nothing: its cost is forks that never exec, which a PATH shim cannot see. On every Bash and PowerShell call the guard created three such processes of its own. This PR removes the one that was this file's to remove on the common path, and three more on the alias and lease paths, in-file only. `lib/hook-utils.sh`, its per-plugin copies, and `run-guards.sh` are untouched. ## Fix - **Common path (every Bash/PowerShell call):** the eager `SUBJECT=$(hook::extract_bash_subject ...)` at file scope fed a telemetry envelope that is off by default and that the verdict never reads. It is now derived inside `emit_tel`, behind the start-stamp and sink gates. - **Lease path (`--force-with-lease=<ref>:<hex>`):** `out="$(git ... rev-parse --show-object-format 2>&1)"` cost two creations for one exec, because bash execs a substitution's body in the substitution's own subshell only when the body carries no redirection of its own. The `2>&1` cannot move onto an outer group here (git's stderr is the diagnostic the block message quotes, and an outer `2>&1` would send it to the hook's stdout), so the body is now `exec git ...`: the subshell becomes git. A missing git still lands in the same `*` error branch, with bash's own "not found" text captured, and the wording of that captured text changes: see the stderr note under Verification. - **`!` alias reparse:** one `$(printf '%q')` per trailing argument is now `printf -v`; `$(effective_dir ...)` around a builtins-only function is now `effective_dir_to`, a nameref assignment (the default base is read before the nameref is written, so the caller's scratch variable then assigns `HOOK_EFFECTIVE_BASE`). - `$(dirname ...)` was already gone from this file before this PR (`${BASH_SOURCE[0]%/*}`); the scout's note was stale. The PowerShell lane's `$(cd "$_HOOK_SELF/.." && pwd)` is left alone: it runs only when `CLAUDE_PLUGIN_ROOT` is unset, which Claude Code never leaves unset. - Guardrails **0.32.13** (`main` is 0.32.10; #3849 takes 0.32.11 and #3869 takes 0.32.12, re-verified against `origin/main` and the open PR list immediately before opening this). README hook-budget accounting entry added per hook-budget Rule 1. ## Verification **Kernel census** (`strace -f -e trace=clone,clone3,fork,vfork,execve`; guard share = `run-guards.sh block-dangerous-git.sh` minus a no-op guard dispatched the same way; this repository as cwd, `HOOK_TELEMETRY_SINK` unset, `CLAUDE_PROJECT_DIR` empty; three identical repeats). strace rather than a PATH shim or xtrace because the subject is a fork that never execs, which neither of those can see. execve reported separately: it does not move anywhere, which is the evidence this is latency, not removed work. | Scenario | creations before → after | execve before → after | |---|---|---| | Guard share, benign `git status --short` / `echo hello` | 3 → 2 | 0 → 0 | | Guard share, blocked `git push --force origin main` / `git reset --hard` | 3 → 2 | 0 → 0 | | Guard share, lease `--force-with-lease=main:<40-hex>` | 5 → 3 | 1 → 1 | | Guard share, `!` alias no trailing args | 5 → 3 | 0 → 0 | | Guard share, `!` alias with three trailing args | 8 → 3 | 0 → 0 | | Guard share, `!` alias whose body carries a lease | 7 → 4 | 1 → 1 | | Guard share, PowerShell `git status` | 15 → 14 | 3 → 3 | | Standalone `bash block-dangerous-git.sh`, benign | 9 → 8 | 3 → 3 | | Whole Bash dispatcher line from hooks.json, benign | 35 → 34 | 3 → 3 | **Deny paths still deny.** A/B against a pristine `origin/main` export on exit code and stderr: 190 paired runs, 0 differences. 87 Bash commands (every form the guard matches: `--force`, `-f`, `+refspec`, `--mirror`, bare/`=ref`/`=ref:movable`/`=ref:<40-hex>`/`=ref:<64-hex>`/`--force-if-includes`/`--no-force-with-lease`/`--dry-run` lease spellings, `reset --hard`/`--h`, `clean -f/-fd/-fdx/--force`, `checkout .`/`:/`/`-f`/`--pathspec-from-file`/exclude-only, `restore .`, `switch --discard-changes`; their near-miss safe variants `reset --keep`, `clean -n`, `checkout -- file`, `restore --staged .`, `branch -D`, `filter-branch`, quoted text; `!` and inline aliases, `bash -c`/`sh -c`/`env`/`sudo` wrappers, multi-segment lines) and 8 PowerShell commands, standalone and dispatched, across a SHA-1 repo, a SHA-256 repo, a non-repo and a nonexistent `-C` dir (probe-failure path). Verdict tally on the new tree: 51 blocked / 36 allowed Bash, 5 / 3 PowerShell. **One stderr string is not identical, and it is not covered by those 190 runs.** Every run above had `git` on `PATH`. On a `PATH` carrying no `git` the lookup now fails inside the `exec` rather than around it, so the diagnostic the block message quotes changes wording: - before: `<hook>: line 341: git: command not found` - after: `<hook>: line 363: exec: git: not found` Measured directly on both trees, running the real hook against a `PATH` shadow that excludes `git` and nothing else: both exit **2**, both emit the same `BLOCKED: ... hash format could not be determined (...)` frame and the same remedy line, and the probe's own status is **127** on both, so the same `*` branch runs and the push is blocked either way. The claim this section previously made, "full stderr", overstated that: verdicts and stderr agree everywhere the 190 runs reached, and on the git-absent probe the only thing that moves is the wording of a quoted error message on a path that still denies. (Related, same mechanism: a `git` on `PATH` that is present but not executable is rc **126** on both trees, and the `exec` form appends a second `cannot execute: Permission denied` line.) The 0.32.13 CHANGELOG entry and the README hook-budget entry state the same qualification. **Permissive-normalization check.** Field reads are not touched by this PR. Probed anyway: a CR mid-token (`git push --for\rce origin main`) and after the token are both blocked, identically on standalone and dispatched paths and on both trees, so the CR strip already lives in the library's field read rather than in the dispatcher cache, and for this guard it acts in the restrictive direction. BOM-prefixed `git`, a zero-width space glued to `--force`, and U+2028 glued to `--force` are allowed on both trees (the bytes make the token something bash would hand to git verbatim, and git rejects it); no divergence introduced. **Contract suite.** `block-dangerous-git.test.sh` 479 → 492, all passing (re-run at the tip of this branch: `PASS=492 FAIL=0`, strace pins included). The 13 new assertions are strace-based pins: benign share exactly 2 creations / 0 execve; blocked share equals benign; `!` alias reparse is benign + 1 (the shared parser's re-entry); trailing alias arguments add 0; lease probe is benign + 1 creation and exactly 1 execve; and a per-process check that the `git rev-parse --show-object-format` exec's parent itself exec'd. Skips visibly where strace is absent. **Non-vacuity proven by mutation**, each change reverted alone: eager subject → benign pin fails (3); `exec` removed → lease pins fail (4, `extra-fork`); `$(printf)` restored → trailing-args pin fails (6); `$(effective_dir)` restored → alias pin fails (4). **Gates.** `scripts/affected-tests.sh --run`: every selected suite passes except `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (2 of 24, PS4 trace probe), which fails identically on a clean `origin/main` export, so it is pre-existing and host-specific, not this change. `check-changelog-parity.sh` `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`: all four pass. `check-killswitch-hoist.sh` passes. `check-purged-em-dashes.sh` passes. shellcheck and shfmt clean on both shell files; markdownlint-cli2 clean on CHANGELOG and README. **Acceptance criteria in #3529, stated plainly.** (1) Spawn count before/after: reported above, by kernel trace, a superset of the PATH-shim count the issue asked for. (2) "No more than 2 external process spawns on the common path": this guard's own share is now 0 execs and 2 forks; the two forks are `$(hook::buffer_stdin)` and the shared parser's `< <(printf ...)`, both `lib/hook-utils.sh` (#3740, #3838) and outside this PR's fence, so that line is not closed from inside this file. (3) "A non-`git` command exits before any spawn": this guard spawns nothing of its own on any command; the dispatcher's two `jq` execs are batch-wide since #3788 and not attributable to this guard. (4) Behavioural tests pass and every form blocked before is still blocked (190/190 A/B on verdict). (5) Wall-clock alongside concurrent-process count: not reported. This Linux host's spawn floor is under a millisecond, so a timing here would not transfer to the Windows spawn tax the parent describes; the process count is the durable figure. ## Related - Refs #3508 (parent). Its "per-field jq" diagnosis does not apply to this guard: since #3788 the dispatcher buffers stdin and primes jq once per batch, so no individual guard contributes a jq spawn; the remaining cost here was forks that never exec. - Precedent for the mechanism and the strace pin: #3779 (context-guard), #3849 (guardrails 0.32.11), #3869 (guardrails 0.32.12), #3851, #3870, #3871. - Version-chain siblings: #3849, #3869 (entries kept verbatim; this PR adds only 0.32.13). - Out of fence, left for their owners: `$(hook::buffer_stdin)` fork-free form in #3740 / #3838; the PowerShell lane's 14 creations live in `lib/powershell/ps-command.sh`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Closes #3520
Important
#3520's first acceptance criterion is NOT met by this PR. It asks for "no more than 2 external
process spawns on its common path (its own shell plus at most one
jq)". This branch runs 4execveon that path: the hook's own shell, onejqfor the envelope fields,bashforscripts/context-zone.sh, and onejqinside the resolver. What this PR reduces is processcreations (
clone/fork/vfork), 8 to 3; the program launches are deliberately unchanged, andthe branch does not get to 2.
Two reasons it stops at 4, both structural rather than oversight:
bash+jqinto the hook's ownjqneeds the sharedlib/hook-utils.shhelper, which fix(hook-utils): a hook payload cut short at EOF is a loud allow; a stall stays a block #3740 has in flight and which this shard is fenced off from.Re-implementing the band lookup inside the hook instead would fork the single band authority
that
context-zone.shexists to be.payload.sh's drain loop is kept on purpose (see the Fix section). It costs no process, butkeeping it means the payload is re-fed to
jqrather thanjqreading the hook's stdindirectly, which is what forces the here-string disclosed below.
Closes #3520is retained only because the repo'spr-issue-linkagegate requires a nativeclosing keyword and
Refsfails it. A reviewer may reasonably prefer to reopen #3520 on merge,or split the remaining 4 → 2 work into a follow-up blocked on #3740, rather than accept a partial
close. No argument is made here for accepting it; the figure is stated so the decision is made on
the real number.
Summary
zone-crossing-inject.shfires onPostToolBatchandUserPromptSubmit, so it draws on both the per-tool-call and the per-turn ceiling indocs/conventions/hook-budget/README.md. It was reported at a 73.0 s average with 10 timeouts.The plugin's own hook-cost accounting already claimed a 3-process steady fire. That claim was measured by counting commands in command position, which counts
jqandbashinvocations, not processes. Understrace -fthe same path was creating 8 processes.The gap is a bash detail, not a script-logic problem. Bash elides the extra fork inside
$(...)and execs the command in the substitution's own subshell — but only when that command carries no redirection of its own. A2>/dev/null, a<<<, or a pipeline written inside the substitution defeats the elision, so bash forks the subshell and forks again to run the command. A fork that never execs never reaches a command position, which is exactly why the existing budget test read 2 and passed while the kernel was making 8.Measured on this Linux host,
V=$(jq . f)costs 1 process creation;V=$(jq . f 2>/dev/null)costs 2;V=$(printf %s "$X" | jq .)costs 3. Hoisting the redirection onto an enclosing group restores the elision:{ V=$(jq . f); } 2>/dev/nullis back to 1.This confirms the campaign's stated cause holds for this script, but locates it more precisely than "per-field
jqforks": there was never more than onejqper pass here. The cost was redirection placement around calls that were already batched. Notably, nohook::jq_fieldsbatching was needed, so the fencedlib/hook-utils.shis untouched.Fix
Three call sites move their redirection onto an enclosing
{ ...; }group, and the stdin payload is assigned in-process instead of captured through a command substitution:hooks/payload.sh,hooks/zone-crossing-inject.shINPUT=$(cg::read_payload)— a subshell to move a string between two copies of the same shellcg::read_payload_toassigns viaprintf -v;cg::read_payloadstays and delegates, so there is one drain loophooks/zone-crossing-inject.sh$(printf '%s' "$INPUT" | jq … 2>/dev/null)— 3 processes{ FIELDS=$(jq …); } 2>/dev/null <<<"$INPUT"— 1hooks/zone-crossing-inject.sh$(bash "$RESOLVER" … 2>/dev/null)— 2{ zone=$(bash "$RESOLVER" …); } 2>/dev/null— 1scripts/context-zone.sh$(jq … "$snap" 2>/dev/null)— 2zones.jsonpassscripts/context-zone.sh$(jq … "$zones" 2>/dev/null)— 2Which hooks actually benefit
Two, not three:
zone-crossing-inject.shitself, on both of its routes (PostToolBatchandUserPromptSubmit).PreToolUsezone gate,hooks/zone-gate.sh. It callsscripts/context-zone.shat line 91 and so inherits the resolver's internal saving (its own resolver call still carries an inner redirect and is not touched here).The
PostCompactmarker does NOT benefit, and an earlier revision of this body and ofCHANGELOG.mdwrongly said it did.hooks/post-compact-mark.shnever callsscripts/context-zone.sh— the resolver appears nowhere in the file — and it still reads its payload withINPUT=$(cg::read_payload)at line 50, a command substitution. Its process count is unchanged by this PR. Both the changelog entry and the README now say so explicitly.Disclosed cost: a temp file on payloads over 64KiB
The saving is not free, and the charge is disk rather than CPU. Two of the five removed process creations come from replacing
printf '%s' "$INPUT" | jqwithjqfed by<<<"$INPUT", and a here-string is not a pipe. Bash 5.1+ delivers one through the pipe buffer only while it fits; at or above 64KiB it writes the string to a temp file (/tmp/sh-thd.*) and handsjqthat descriptor. Measured here on bash 5.2.21 understrace -f -e trace=openat:/tmp/sh-thd.*opensThe pipeline this replaced never touched disk at any size. The extracted fields are byte-identical either way, so no output changes. But a
PostToolBatchpayload carries every serialized tool result and routinely clears 64KiB, so a large fire now performs a temp-file write and read it did not perform before. That lands on the platform this work is for: the #3508 hosts run Defender real-time protection, which scans temp-file writes, and the 0.4.8 measurement already recorded in the plugin README attributes 22.0 s on that platform to it.The trade taken is one process creation saved on every fire against disk I/O on the fires that exceed the buffer, on hosts where a process creation costs 180–2,841 ms. Disclosed in three places: the plugin README's hook-cost accounting (per hook-budget Rule 1 and
.claude/rules/hook-budget.md), the0.7.44changelog entry, and the code comment at the call site — which previously claimed the group rewrite "changes nothing else".Why the drain loop is kept
payload.sh's drain loop is kept, not bypassed. An earlier revision of this body justified that by saying piping stdin straight tojq"would have removed another process". That is no longer true and has been corrected: after thecg::read_payload_toconversion the loop isreadbuiltins only and costs zero processes. What piping stdin straight tojqwould actually avoid is the here-string temp file above.The decision stands on its real merit: the loop's bounded
read -t 5caps a stalled pipe at five seconds instead of letting it block to the harness timeout, which is the very symptom this campaign is about. Disk I/O on oversized payloads is the smaller of the two costs.lib/hook-utils.shand everyplugins/*/hooks/hook-utils.shcopy are untouched — verified in CI-visible form bygit diff origin/main --name-only.Verification
Before/after, measured here (
strace -f -e trace=clone,clone3,fork,vfork,execve, steady non-crossing path, primed state dir):execve)origin/mainProgram launches are unchanged at 4 — the same
jq,bashandjqstill run over the same inputs. That is the evidence this is a latency fix and not a work-removal fix: only the fork overhead around the calls is gone. It is also why #3520's 2-spawn criterion is unmet; see the note at the top.Wall-clock is not measurable on this host, and I am not inventing a figure. The cost in #3508 is Windows process-creation tax (180–2,841 ms per spawn, median 1,108 ms at 501 concurrent processes, against ~1% user CPU). This runner is Linux, where a spawn is ~1 ms, so the whole effect is below noise. The proxy reported is the drift-immune criterion #3508 itself specifies: process-creation count by trace. At the #3508 median spawn cost, 5 fewer process creations is on the order of 5.5 s per fire; at that issue's floor, ~0.9 s. Both need confirming on the affected Windows host before any acceptance criterion can be signed off — this PR does not claim that measurement, and it does not net out the temp-file I/O disclosed above.
Proof the behaviour did not change:
zone-crossing-inject.test.sh, 81 incontext-zone.test.sh), including partial-write recovery, the armed-rank hysteresis, fail-open paths, and the malformed-zones.jsonstderr notices.execvecount and targets are byte-identical before and after.zones.jsonis the one input that drives the resolver's stderr on this route: it pins that the notice reaches neither of the hook's streams, that stdout stays one parseable JSON document, and that shipped default bands still resolve and inject. An unparseable payload pins that the payload pass's nonzero status still propagates out of its new enclosing group rather than being absorbed by it.New regression test, verified non-vacuous.
strace -fpins process creations at exactly 3 and program launches at exactly 4, so a fork saving cannot be confused with work removed. Mutating a copy of the tree to move one redirect back inside its substitution takes the count to 4 and fails withsteady path creates 4 processes, budget is 3, while the old command-position budget passes silently. The assertion skips wherestraceis unavailable (containers, macOS), and the pre-existing command-position budget still runs there.Commands run in the foreground on the current head, with actual results:
Manifest bumped 0.7.43 → 0.7.44 with a matching changelog entry; the accuracy corrections land inside that same unreleased entry, so no further bump was needed (
--check-bumpand--check-preservedboth confirm).Not mine and pre-existing:
test_save_point.py::test_new_origin_falls_back_to_directory_namefails identically on pristineorigin/main.Related
hook::buffer_stdincall sites, so it was fixable in-file without serializing behind the sharedlib/hook-utils.shwork.lib/hook-utils.shchanges in flight; this PR deliberately does not touch that file or any of its 17 synced copies. It is also the blocker on the remaining 4 → 2execvework described at the top of this body.hook::jq_fields,strip_quoted_spans, the 65-spawn census). Not revived here and not needed: the batching those introduce was already present in this hook, and the remaining cost was redirection placement rather than call count.context-guardfile.docs/conventions/hook-budget/README.md, surfaced by.claude/rules/hook-budget.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob