perf(autonomy): cut lane-stop-gate process creations on every Stop - #3870
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 fixes it produced — flipped to readyPosted by the autonomous worker lane ( The behaviour change is a genuine fix, and the old code was the bugThe review built the case from staged old and new installs. With sentinel The old behaviour could not have been load-bearing: it authorized on half-tokens the block reason never asked for, a newline in a sentinel is malformed config, and no shipped launcher writes one. Every other class tried — default token, CR, tabs, blank lines, inline and substring and trailing forms, a metacharacter sentinel Fail-safe direction: the new code errs toward "fails to stop" (it blocks once, then the existing one-nudge-then-allow path releases it). The old code erred toward "stops when it should not", which is the direction the gate exists to prevent.
|
|
Claude finished @kyle-sexton's task in 3m 28s —— View job Security review (
|
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd0e7578de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude finished @kyle-sexton's task in 7m 2s —— View job Code review complete
SummaryReviewed the process-creation reduction in 1 finding, posted as 3 inline comments (same root cause, three sites): A redirection-order bug —
Functionally these still fail safe (no verdict change — I traced that a failed group redirection skips the enclosed Minor, not filed as a finding: the README's new "Hook cost" table has 6 rows; the PR description's verification table has 7 (missing the "Env-only enable claim" row). Doc-only, no action needed unless you want the table to match. Verification claims in the PR body (differential harness, strace ceilings, |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…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
`gate_file_mentions` replaced the pre-filter's `grep -q lane_stop_gate "$f" 2>/dev/null` with a builtin NUL-chunk read, but wrote the loop's redirections as `done <"$1" 2>/dev/null`. Bash applies a command's redirections left to right, so the open was attempted before stderr was silenced: a settings file that exists but this hook may not read printed "Permission denied" where `grep` had been silent. The gate runs on every Stop of every session, so that is per-turn noise, and it reaches the user even though the harness ignores stderr from a hook that exits 0. Swapping the two redirections is the whole fix. The verdict is unchanged in both directions -- an unreadable file still yields no chunk and returns 1, contributing no verdict, so the gate stays off and the stop is allowed -- and only stderr discriminates, so new case 51 asserts stderr is empty while re-asserting the verdict. Verified by running the suite as an unprivileged user, where the case runs for real: it passes on the fixed tree and fails with the "Permission denied" line on a tree with the old ordering. Where chmod 000 denies nothing (root, or a filesystem without POSIX modes) the case skips visibly rather than passing vacuously. The strace budget is unchanged, re-measured rather than assumed: the default path is still exactly 1 creation and 1 launch (`uname`), and the enabled block path still 10 creations and 5 launches (`jq -e .` from the shared library's `hook::buffer_stdin`, the `jq -j` payload pass, `uname`, the `jq -j` settings pass, the `jq -nc` block decision). Suite is PASS=103 FAIL=0 unprivileged, PASS=102 with the visible skip. Refs #3515. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Trust EPOCHSECONDS only when Bash 5.0+ provides it; on older bash an inherited value is an ordinary variable a repo env block can set and would choose the arm-record TTL verdict. Preserve CR in last_assistant_message so LANE-STOP\r-OK cannot become LANE-STOP-OK. Silence stderr before the input open on the remaining group redirections (gate_resolve_plugin_name, gate_settings_options_to, arm-record load). Suite pins the pre-5.0 EPOCHSECONDS path, the embedded-CR token, and unreadable-file stderr silence at each remaining site. Refs #3515. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
fd0e757 to
ba2b0a2
Compare
Case 54 left the user settings file from the CR-token cases in place, so the hook still enabled from settings and blocked even though the arm record was unreadable. Drop the leftover settings file first. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Keep the lane-stop-gate work current so auto-merge can land once lint is green. No content conflicts: merge-tree against origin/main was clean. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
The NUL-split payload loop lived in a group, not a function, so `local f` was invalid (SC2168) and the group's 2>/dev/null swallowed the diagnostic. Use `_gate_pf` at script scope and unset it after the read. Case 53 now initializes `got` before the nameref fill (SC2154) and isolates the EPOCHSECONDS spoof to its subshell (SC2030/SC2031). Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.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 #3515
Important
Which of #3515's acceptance criteria this PR meets, and which it does not.
jq)uname -s. The one spawn isuname, notjq; it is the managed-settings platform primitive the trust design rests on ($OSTYPEis 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 (4jq, 1uname), down from 18. One of thosejqis in the fenced shared library'shook::buffer_stdin(its payload validation pass); the other three read three different inputs (payload, settings file, block decision).grep/sed/cut/tr/basename/dirnamereplaced with builtins on the hot pathcksumremains on the marker-consumption path only (it keys the ledger, and changing the key would orphan existing ledger entries).jq, but it spawnsunameto 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.Closes #3515is kept because thepr-issue-linkagegate 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.shis the autonomy plugin'sStophook. 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
jqneeding a shared helper.lib/hook-utils.shand all 17plugins/*/hooks/hook-utils.shcopies are untouched (git diff origin/main --name-onlyshows only the six autonomy files). The one shared-library facility used,hook::jq_fields, already existed onmain.Fix
All in
plugins/autonomy/hooks/lane-stop-gate.shandlane-stop-gate-lib.sh: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 parametersgate_x_to <var>forms viaprintf -v; print forms delegate, solane-stop-gate-arm.shand the lib-level tests read exactly what they read beforegrep -q lane_stop_gate "$f" 2>/dev/nullper fileread+ substring test (gate_file_mentions), with2>/dev/nullwritten before the input redirection so an existing-but-unreadable settings file stays as silent asgrepwas (bash applies redirections left to right; see case 51)done < <(gate_managed_settings_files)with$(uname -s 2>/dev/null)inside: 3 creations per call, called up to 4 times per stopgate_managed_settings_files_loadfills an array in-process;{ platform=$(uname -s); } 2>/dev/nullis 1 creation; loaded once per stop and reused by option resolutionEVENT,SESSION_ID,CWD,STOP_ACTIVE,LAST)printf | jq | trpipelines, 3-4 creations eachhook::jq_fieldspass (3 creations: process substitution, printf writer, jq); values chomped of trailing newlines so each reads byte-for-byte as the old$( )capture did$(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)jq -ec .validation +printf | jqper field +printf | jqper option: 8 creations plus 6 on lookupEPOCHSECONDSfor the TTL clock withdateas the pre-5.0 fallback$(printf | sed …)thengrep -qE … <<<"$LAST"[[ $LAST =~ (^|\n)[[:space:]]*TOKEN[[:space:]]*(\n|$) ]], which agrees with grep's per-line verdict on every message (argument in the code comment)$(jq -nc --arg … 2>/dev/null)on every evaluated stop$(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/nullstat groups,${ledger%/*}$(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, andlane::notifystrips C0 again)gate_resolve_plugin_name(unanchored installs)$(jq … <"$manifest" 2>/dev/null)Left alone on purpose:
uname -sas the platform primitive;hook::buffer_stdin(shared library; its$( )capture, read-slice probe andprintf | jq -evalidation are 4 of the enabled path's remaining 10 creations); thegate_arm_ownedsubshell (one fork, and the cleanest scope forumask/noclobber); the finaljq -ncblock decision.Disclosed divergence (one degenerate config): a configured sentinel that itself holds a newline.
grep -Eread 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/nullhere wraps exactly one command, and the two process-substitution loops wrap areadbuiltin (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, stagedplugins/cache/<m>/<n>/<v>/hooksinstall, hook launched by the harness so its own shell is not in the count):execve) beforeStop)grep,uname)uname)jq,jq,uname,jq,jq)hook::notice_oncein the shared library)Launches did not stay flat, and that is expected here: unlike #3779, this issue explicitly asks for the
grep/sed/tr/dirnamehelpers to become builtins, so those launches are gone; everyjqthat reads a distinct input is still launched, and the payload/settings/arm-recordjqcalls 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.
lane-stop-gate.test.shpass unmodified; the suite is now 103 with the additions below (102 wherechmod 000denies nothing and case 51 skips visibly).origin/mainhook 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,optionsas string/array,pluginConfigsas 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 floatarmed_at, expired, legacysession_idmatch/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.New regression tests, verified non-vacuous.
gate_settings_options_toanswers three keys from one pass with the single-key verdicts (including the trailing-newline chomp and the all-or-nothingoptions-not-an-object case).gate_file_mentions(commitfd0e7578): the first draft wrotedone <"$1" 2>/dev/null, which attempts the open before stderr is silenced and printsPermission deniedper turn wheregrep -q … 2>/dev/nullwas silent. Run as an unprivileged user the case passes on the fixed tree and fails with that exact line on the old ordering; wherechmod 000denies nothing (root, or a filesystem without POSIX modes) it skips visibly rather than passing vacuously.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 fix(hook-utils): a hook payload cut short at EOF is a loud allow; a stall stays a block #3740/perf(hooks): fuse stdin jq completeness with field extract #3838 lowers the count without failing here, while a regression in this plugin's files raises it and does); and nodirname,tr,sed,grep,cksumordateon either path. Skips wherestraceis unavailable; the CI Linux lane does not skip. Re-measured rather than assumed after thefd0e7578redirection swap: both pins hold, and the enabled path's fourjqare stilljq -e .(hook::buffer_stdin), thejq -jpayload pass, thejq -jsettings pass and thejq -ncblock decision, in that order withunamethird.unameredirection back inside its substitution):FAIL: default path creates 2 processes, budget is 1andFAIL: enabled block path creates 11 processes, ceiling is 10. Mutation 2 (put thesedescape 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 (thelane-notify/ lane-launcher /check-shell-portabilitylines are from the1ca98cbfrun; the second commit touches neither those files nor their inputs, and the fullaffected-tests.sh --runbelow was re-run onfd0e7578):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 thecache-content-check.test.shtrace-probe failure above.Related
jq, and merged PR perf(hooks): cut leftover Bash parse and dirname tax on the hot path #3788, which applied it across 34 hooks without touchinglib/hook-utils.sh.lib/hook-utils.shand 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.hook::jq_fields,strip_quoted_spans, the 65-spawn census).hook::jq_fieldsis used here as it already exists onmain; nothing from fix(guardrails): restore PreToolUse enforcement — guards were killed at their timeout #1385 is revived.docs/conventions/hook-budget/README.md, surfaced by.claude/rules/hook-budget.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Generated by Claude Code