perf(hooks): dispatch the guardrails guards through one process and if-gate the path-specific hooks by kyle-sexton · Pull Request #3621 · melodic-software/claude-code-plugins

kyle-sexton · 2026-09-02T03:14:15Z

No related issue: performance work driven by the dotfiles hook fan-out harness; the earlier spawn-reduction tracker #1403 is already closed.

Summary

Every Bash tool call was paying for eleven always-on hook processes and every Write for eighteen, most of them guards or formatters that exit immediately because the payload is not theirs. On Windows Git Bash the process spawn is the cost, not the classifier, so the fan-out itself was the typing lag.

Fix

guardrails: a dispatcher, hooks/run-guards.sh, is now the registered command for each always-on event. It reads stdin once, extracts the payload fields with one jq process, and sources each guard in turn inside that one bash process, so its exit, trap and BASH_SOURCE behave as they do standalone. Exit code: 2 if any guard blocks, and every guard still runs; several guards' additionalContext documents are merged into the one JSON document a hook process may emit. Every guard keeps its script, contract test, kill switch and telemetry envelope, and hooks.json still names each one by file name. Eight processes per Bash call become one; three per Write PreToolUse become one; three per Write PostToolUse become one.
if filters on the hooks that only ever act on a specific file or command class, each a superset of the hook's own first check: the six extension formatters (ruff-format, bash-format, biome-format, go-format, powershell-format, actionlint), instruction-placement's index-drift (Edit(**/.claude/rules/*.md)), source-control's pr-body-linkage-gate (Bash(gh *)) and the two worktree gates (Bash(*worktree*)), and disk-hygiene's engine gate (a filter on the engine's file name; see that plugin's CHANGELOG).
Versions bumped on every changed plugin, with CHANGELOG and README entries, and the guardrails README carries the budget accounting.

context-guard is unchanged: if is only evaluated on tool events, so its PostToolBatch and UserPromptSubmit hooks cannot carry one, and zone-gate keys on a plugin option rather than a payload field.

Verification

plugins/guardrails/hooks/run-guards.test.sh (new, 54 assertions): stdin re-served with the same rc, jq cache hit and miss paths, NUL payload bypasses the cache, block wins and later guards still run, exit aggregation, single emitter passes through verbatim, several emitters merge into one document, --lib preload, unknown guard reported, and a real guard blocking identically alone and dispatched.
Baseline-versus-installed evidence runs for all 13 dispatched guards and every gated hook with a payload matching its real trigger (a --no-verify commit, a force push, cat > file, a multi-line -m, a /tmp write, an exported MSYS_NO_PATHCONV, an AWS key in a Write, a gh pr create with a bad body, a nested git worktree add, a .py / .sh / .ts / .go / .ps1 / workflow / rules-file write): same exit code and same stderr in every case; the only stdout delta is the intended one, three advisory documents merged into one.
The fleet harness (dotfiles/common/measure-claude-hook-fanout.sh, unchanged, 3 runs) against the installed cache: PreToolUse:Bash fires 11 -> 1, PostToolUse:Bash 1 -> 0, PreToolUse:Write 5 -> 3, PostToolUse:Write 13 -> 4; UserPromptSubmit and Stop unchanged at 1 and 4.
scripts/affected-tests.sh --base main --run: the guardrails, source-control, disk-hygiene and formatter suites pass; three prose-audit suites in untouched plugins (ai-slop, claude-config, code-tidying) report failures on this Windows host that do not reference the changed files.

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T03:18:34.987430Z	`a423e2c`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-09-02T03:14:49Z

Claude finished @kyle-sexton's task in 7m 17s —— View job

Claude finished the code review

Gather context (PR diff, CLAUDE.md/AGENTS.md conventions)
Understand the request (invoke /review:code-review)
Review guardrails/hooks/run-guards.sh dispatcher
Review guardrails/hooks/run-guards.test.sh
Review hooks.json changes (dispatcher registration + if gates) across all plugins
Review CHANGELOG/README/version bumps
Post findings

Summary

Reviewed the dispatcher (plugins/guardrails/hooks/run-guards.sh) and the if-gate additions across all nine changed plugins against the PR's own stated design rule ("each [if filter] a superset of the hook's own first check"). The dispatcher itself is careful, well-tested code — the stdin re-serving, jq caching, per-guard subshell isolation, and exit/stdout aggregation all check out against run-guards.test.sh's 54 assertions and the guards' own contract tests. Left 3 inline comments:

plugins/source-control/hooks/hooks.json:10 — the new "if": "Bash(gh *)" filter on pr-body-linkage-gate.sh anchors at the start of the command, but the guard's own applicability check (and its own test suite) is explicitly designed to catch gh pr create/gh pr edit after a leading cd/pushd/wrapper. This looks like it silently disables the PR-body compliance gate for exactly the compound-command shapes it was built to catch.
plugins/disk-hygiene/hooks/hooks.json:10 — the new "if": "Bash(*hygiene.py*)" filter only names the Bash tool, but the matcher above it is Bash|PowerShell and destructive_guard.py has an explicit tool_name == "PowerShell" branch. If if is tool-scoped the way Bash permission rules are (per this repo's own permission-rule-hygiene doc), PowerShell-issued commands referencing the hygiene engine would no longer reach this destructive-operation guard at all.
plugins/guardrails/CHANGELOG.md:21 — this entry claims the per-Bash-call dispatcher change lands "under 1 s," but the fuller accounting added in the same PR (README.md, same commit) reports ≈1,220 ms and says explicitly it's "still above the convention's ≤ 1 s typical ceiling." The two documents disagree about whether the stated hook-budget rule is satisfied.

Findings 1 and 2 are worth resolving (or explicitly confirming Claude Code's if-matching semantics don't work the way the rest of the codebase's own documentation suggests) before merge, since both are compliance/guard gates rather than formatters. The other if additions (six formatter extension filters, the two worktree gates, instruction-placement's rules-index filter) checked out as exact matches or correct supersets of their guards' own internal checks.

Everything else — version bumps, CHANGELOG/README entries per plugin, the hooks.json wiring test that pins every dispatched guard by file name — looked consistent with the PR's stated changes.

claude · 2026-09-02T03:14:57Z

Claude finished @kyle-sexton's task in 6m 29s —— View job

Security review (`/review:security-review`)

Gather context (PR perf(hooks): dispatch the guardrails guards through one process and if-gate the path-specific hooks #3621 body, changed files, diff vs origin/main)
Read run-guards.sh dispatcher, hook-utils.sh trust-boundary helpers, and the new if filters in every touched hooks.json
Post findings

Reviewed only the files changed in this PR (git diff origin/main...HEAD), per the security-review lane's scope. Focus: the new guard dispatcher (plugins/guardrails/hooks/run-guards.sh) and the if filters added to eleven hooks.json files, since both change whether/how a security-relevant hook runs.

Finding (IMPORTANT) — posted inline on `run-guards.sh#L65-L68`

The dispatcher shadows the dirname builtin with a fast-path shell function, intended (per its own comment) only for the dirname "${BASH_SOURCE[0]}" shape every guard's source line uses. Because it's a plain function defined before any guard is sourced, it actually overrides dirname for every call any dispatched guard makes in its subshell, and it isn't a faithful dirname:

dirname "/foo" → returns "" instead of "/"
dirname "/a/b//" → returns "/a/b" instead of "/a" (only strips one trailing slash)

That override sits underneath hook::in_git_working_tree "$(dirname "$file_physical")" (hook-utils.sh#L573), the CLAUDE_PROJECT_DIR-unset fallback inside hook::read_file_path — the scope gate cli-flag-verify.sh, skill-reference-verify.sh, and stale-path-verify.sh all rely on when dispatched. A dirname divergence there flips the git-working-tree scope check the wrong way and lets the guard fail-open (silently skip a file it should have verified) for a root-level or trailing-slash-heavy path.

Current blast radius is narrow — only advisory PostToolUse "verify" guards, only in the no-CLAUDE_PROJECT_DIR branch, not a blocking secrets/injection guard. The concern is architectural: the shadow is invisible to any guard that sources it, so any future guard added to this dispatch list that calls dirname on a tool_input-derived path outside the one shape this was built for silently inherits the bug. Full details and a suggested fix (replicate GNU dirname's trailing-slash stripping, or scope the fast path to a differently-named helper used only at the BASH_SOURCE[0] site) are in the inline comment.

Also checked, no issue

New if filters as bypass surfaces (source-control's Bash(gh *) / Bash(*worktree*), disk-hygiene's Bash(*hygiene.py*)): each is documented in the corresponding README/CHANGELOG as a superset of the gated script's own relevance check, and Claude Code's if matcher itself evaluates each subcommand of a compound command (falling open to "run the hook" when it can't tell what a command expands to) — so a chained command like cd x && gh pr create … isn't a bypass. pr-body-linkage-gate.sh's own header additionally documents this as a policy gate, not a security guard, with the actual enforcement authority sitting in the pr-issue-linkage.yml CI check — consistent with this repo's .claude/rules/pr-body-contract.md.
Guard names / --lib args passed to run-guards.sh: all static, sourced from hooks.json, never derived from tool input — no injection surface.
stdin/jq caching in the dispatcher: NUL-bearing payloads correctly bypass the cache (RUN_GUARDS_PRIMED only set when HOOK_JQ_FIELDS_NUL == 0), matching each guard's own NUL handling; exit-code aggregation keeps 2 (block) sticky once any guard blocks, and every guard still runs after a block, so no guard's verdict is silently dropped.
Guard isolation across the dispatcher's single process: each guard is sourced inside a $(...) command substitution, which bash always forks as a subshell — so one guard's variable/trap/set changes can't leak into the next guard or into run-guards.sh itself.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a423e2c050

ℹ️ 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".

github-actions · 2026-09-02T03:21:50Z

Last security-reviewed head: 4ac5dffcd8cc8f688dd27e2e4e4d2941b86cc182. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

github-actions · 2026-09-02T03:22:31Z

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

…ts, shellcheck info findings) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

claude · 2026-09-02T03:55:03Z

Claude finished @kyle-sexton's task in 5m 3s —— View job

Code review — PR #3621 (`f0c5ffc`)

Gather context (PR diff vs origin/main, prior review threads, AGENTS.md/CLAUDE.md conventions)
Review run-guards.sh dispatcher against prior findings (executable bit, dirname shadow, jq-missing merge path)
Review if-gate additions across all changed hooks.json files
Review CHANGELOG/README/version-bump consistency
Post findings

Summary

This pass re-verifies the PR at its current head, f0c5ffc. The only commit since the prior review round (f0c5ffc) fixed the dispatcher's missing executable bit and added shellcheck disables — the other findings raised earlier are unchanged in the code and still hold. I posted 4 inline comments; verified each directly against the guards' own contract tests/source rather than re-asserting the earlier claims.

Inline findings posted:

plugins/source-control/hooks/hooks.json:10 — "if": "Bash(gh *)" is anchored to the start of the command, so GH_TOKEN=x gh pr create, env GH_TOKEN=x gh …, and sudo gh … never launch the hook process, even though pr-body-linkage-gate.sh's own unanchored check and its own test suite (pr-body-linkage-gate.test.sh:278-300) are explicitly built to catch exactly those forms. This silently drops enforcement of a required-CI-check compliance gate.
plugins/guardrails/hooks/run-guards.sh:66 — the dirname shadow still diverges from GNU dirname (dirname "/foo" → "" instead of "/") and still applies to every dirname call inside every dispatched guard's subshell, not only the one call site it was written for. Only a shellcheck-disable comment was added since the earlier review round; the divergence itself is unchanged.
plugins/guardrails/CHANGELOG.md:21 — claims the dispatcher change lands "under 1 s," but README.md's own budget accounting (same commit) reports ≈1,220 ms and says outright it's "still above the convention's ≤ 1 s typical ceiling." The two documents disagree.
plugins/disk-hygiene/hooks/hooks.json:10 — "if": "Bash(*hygiene.py*)" only names the Bash tool though the matcher is Bash|PowerShell, and destructive_guard.py:1475 explicitly branches on tool_name == "PowerShell". The same open question has a larger blast radius elsewhere in this PR: all six formatter plugins (ruff-format, bash-format, biome-format, go-format, powershell-format, actionlint) register their if filters as Edit(*.ext) on a Write|Edit matcher — if if's tool prefix requires an exact match against the tool that fired, none of them would run on a brand-new file created via Write, only on edits to existing files. This turns on one unverified premise about Claude Code's if-matching semantics; worth confirming before merge since it fails silently either way.

Also checked, no new issue beyond what's already tracked:

The jq-missing merge fallback in run-guards.sh:181-198 (already flagged inline by Codex on this PR) — confirmed real: several guards can independently emit a jq-missing "degraded" notice via additionalContext, and the merge step that would combine them itself depends on jq, so on a jq-less host the fallback prints multiple JSON documents sequentially, which is invalid as a single hook response. Lower severity — an edge case gated on jq being entirely absent — but still open.
Guard isolation (subshell per guard via command substitution), stdin re-serving/caching including the NUL-bypass path, and exit-code aggregation all check out against run-guards.test.sh's 54 assertions.
The Bash(*worktree*) gates in source-control and the Edit(**/.claude/rules/*.md) gate in instruction-placement are correct supersets of their guards' own checks.
Version bumps and CHANGELOG/README entries are present and consistent across all nine other changed plugins.
· branch perf/hook-fanout-consolidation

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

claude · 2026-09-02T03:56:10Z

Claude finished @kyle-sexton's task in 2m 57s —— View job

Security review (`/review:security-review`)

Gather context (PR perf(hooks): dispatch the guardrails guards through one process and if-gate the path-specific hooks #3621 body, prior review threads, changed files at head 09f58f3)
Diff the two fix-up commits since the last security pass (a423e2c..09f58f3) to see what actually changed
Re-verify the previously flagged dirname shadow finding against current head
Check the disk-hygiene if-filter tool-scoping concern (raised by the code-review lane, not yet assessed by this security lane)
Post findings

This PR was already security-reviewed at a423e2c (see the earlier claude comment above). Since then, two commits landed: f0c5ffc and 09f58f3, touching only plugins/guardrails/hooks/run-guards.sh and its test. I diffed exactly those two commits and re-checked what's still open at the current head (09f58f3).

Fixed since the last pass — no action needed

run-guards.sh was previously committed as mode 100644 (Codex's P1 finding: the registered dispatcher wasn't executable, which would fail every dispatched guard — including the security-relevant ones — with exit 126 on POSIX). f0c5ffc sets the executable bit (old mode 100644 / new mode 100755), confirmed via git diff a423e2c..09f58f3 --stat. This closes a fail-open surface across the whole guard fleet.

Still open at head `09f58f3` — carried forward

dirname shadow in the dispatcher, IMPORTANT — plugins/guardrails/hooks/run-guards.sh#L66-L69. The only change here since the last review is a # shellcheck disable=SC2329 comment (f0c5ffc) — the function body is unchanged. The shell-wide dirname() override still diverges from GNU dirname (dirname "/foo" → "" instead of "/"; dirname "/a/b//" → "/a/b" instead of "/a") and still applies to every guard sourced in the dispatcher's subshell, not just the one BASH_SOURCE[0] call site it was written for. This can flip hook::in_git_working_tree's scope check the wrong way for a root-level or trailing-slash-heavy path and silently skip a verify guard (cli-flag-verify, skill-reference-verify, stale-path-verify) that should have run. Full detail already posted inline on the prior commit; unresolved.
disk-hygiene's if filter likely excludes PowerShell entirely, IMPORTANT (new to this pass) — posted inline on plugins/disk-hygiene/hooks/hooks.json:10. The matcher covers Bash|PowerShell, but "if": "Bash(*hygiene.py*)" names only the Bash tool. If if filters are tool-scoped the way this repo's own Bash permission rules are (docs/conventions/permission-rule-hygiene/README.md), a PowerShell-tool call never satisfies a Bash(...) pattern at all, so the destructive-operation kill-switch/data-root guard's hook process is never launched for any PowerShell command referencing the hygiene engine — and, because the if filter prevents the launch rather than the guard failing at runtime, this would be invisible to the plugin's own guard_launch_monitor.py Stop-event detector (which only catches guards that launched and then failed). The earlier claude security-review pass on this PR checked the compound-Bash-command bypass angle for this same filter and found no issue there — it did not evaluate the multi-tool matcher-vs-single-tool-if question, which is what this finding is about.

Both findings existed before the last two commits and are unchanged by them — this pass adds the tool-scoping angle on disk-hygiene as new ground, and re-confirms the dirname shadow is still live at 09f58f3. No new security issues found in the diff between a423e2c and 09f58f3 itself (comment/mode/shellcheck-suppression only).

Out of scope for this lane (already correctly triaged as policy/code-review, not security): the source-control Bash(gh *) anchoring question — the earlier security pass already assessed this as a policy gate backed by the real pr-issue-linkage.yml CI check, consistent with .claude/rules/pr-body-contract.md; that's /review:code-review territory, not this lane's.

claude · 2026-09-02T04:00:54Z

Claude finished @kyle-sexton's task in 3m 22s —— View job

Claude code review (`/review:code-review`)

Gather context (PR diff, CLAUDE.md/AGENTS.md conventions)
Understand the request (invoke /review:code-review)
Review dispatcher (run-guards.sh) and its test
Review hooks.json if-gate changes across all plugins
Review CHANGELOG/README/version bumps
Post findings

Summary

This is the code-review lane; security findings are scoped to the separate claude-security-review lane already running on this PR (which flagged the dirname shadow in run-guards.sh — not repeated here). Reviewed the diff at HEAD 09f58f3 against origin/main. The two most recent commits (f0c5ffc, 09f58f3) only fix hygiene-lane findings (exec bit, shellcheck waivers) — they don't touch the four correctness/documentation issues below, which were raised on an earlier commit in this PR's history and remain present at this HEAD. Posted as fresh inline comments since they anchor to lines still unresolved in the current diff:

plugins/source-control/hooks/hooks.json:10 — "if": "Bash(gh *)" is anchored to the start of the command, so it misses cd x && gh pr create ...-shaped commands that pr-body-linkage-gate.sh's own unanchored applicability check and test suite are explicitly built to catch. This silently drops the PR-body compliance gate for exactly the compound-command shapes it exists to catch.
plugins/disk-hygiene/hooks/hooks.json:10 — the matcher covers Bash|PowerShell, but the new if filter only names Bash. destructive_guard.py has an explicit PowerShell branch; if if is tool-scoped (as Bash(...)/PowerShell(...) are elsewhere in this repo's own conventions), PowerShell-issued commands referencing the hygiene engine lose this destructive-operation gate entirely.
plugins/guardrails/hooks/run-guards.sh:184 — on a host without jq, the multi-guard merge step fails silently (2>/dev/null) and falls back to printing every guard's JSON document concatenated, which is invalid as a single hook response. run-guards.test.sh hard-requires jq to run, so this branch is untested.
plugins/guardrails/CHANGELOG.md:21 — claims the dispatcher lands "under 1 s," but the README's own fuller accounting (added in the same commit) reports ≈1,220 ms and says explicitly it's still above the hook-budget convention's ≤1 s ceiling. The two docs disagree.

Everything else checked out: the dispatcher's stdin/jq caching and subshell isolation match run-guards.test.sh's assertions, the six formatter extension if filters and the worktree/rules-index filters are correct supersets of their guards' own checks, and version bumps/CHANGELOG/README entries are consistent per plugin.

A pattern beyond a bare command name still spawns the process on a command containing a substitution; the harness reports it as RAN(best-effort). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

claude · 2026-09-02T06:45:32Z

Claude finished @kyle-sexton's task in 2m 45s —— View job

Claude code review (`/review:code-review`)

Gather context (PR diff vs origin/main, prior review threads, AGENTS.md/CLAUDE.md conventions)
Review dispatcher (run-guards.sh) and its test at current HEAD
Review hooks.json if-gate changes across all plugins at current HEAD
Review CHANGELOG/README/version bumps
Post findings

Summary

This pass reviews the PR at its current head, 121c222. The only commit since the last review round (121c222, "docs(hooks): state the best-effort residual of the Bash if filters") is doc-only — it touches plugins/disk-hygiene/README.md and plugins/source-control/README.md, adding a caveat that a command containing $(), a backtick, or $VAR still spawns the gated process because the if filter can't see what the substitution expands to. That's an accurate, narrow addition and consistent with how a prior security-review pass on this PR already characterized if's fallback-open behavior for compound commands.

It does not address any of the four correctness/documentation findings raised repeatedly on earlier commits in this PR's history, and none of them have changed in the code since. Re-posted as fresh inline comments anchored to the current HEAD:

plugins/source-control/hooks/hooks.json:10 — "if": "Bash(gh *)" is anchored to the start of the command, so GH_TOKEN=x gh pr create, env GH_TOKEN=x gh …, and sudo gh … never launch pr-body-linkage-gate.sh, even though the guard's own unanchored check and test suite (pr-body-linkage-gate.test.sh:278-300) are built to catch exactly those forms. Notably, the new README addendum documents a different residual gap (substitution-bearing commands) and says nothing about this prefix/wrapper gap, which could read as if the residual is now fully characterized when it isn't.
plugins/disk-hygiene/hooks/hooks.json:10 — the matcher is Bash|PowerShell, but the if filter only names Bash. destructive_guard.py explicitly branches on tool_name == "PowerShell"; if if's tool prefix is scoped the way this repo's own Bash permission-rule docs describe, PowerShell-issued commands referencing the hygiene engine lose this destructive-operation gate entirely. Unconfirmed against Claude Code's actual if-matching semantics for a multi-tool matcher.
plugins/guardrails/hooks/run-guards.sh:66 — the dirname shadow still diverges from GNU dirname for root-level and trailing-slash-heavy paths, and still applies shell-wide to every dispatched guard's subshell rather than only the one BASH_SOURCE[0] call site it was written for.
plugins/guardrails/hooks/run-guards.sh:184 — the multi-guard jq-merge fallback, on a host without jq, prints every guard's JSON document concatenated, which is invalid as a single hook response; untested (run-guards.test.sh hard-requires jq).
plugins/guardrails/CHANGELOG.md:21 — still claims the dispatcher lands "under 1 s," while README.md's own accounting (same commit) reports ≈1,220 ms and says explicitly it's still above the 1 s ceiling. The two documents disagree.

Everything else checked out as in prior passes: the dispatcher's stdin/jq caching, subshell isolation, and exit-code aggregation match run-guards.test.sh's 54 assertions; the six formatter extension if filters and the two worktree gates are correct supersets of their guards' own checks; version bumps and CHANGELOG/README entries are consistent across all nine other changed plugins; the executable-bit and shellcheck fixups from earlier fixup commits remain in place.

…t plugins Second merge-forward. Since the last one, main claimed the exact version numbers this branch had bumped to in eight plugins (actionlint 0.8.28, bash-format 0.7.29, biome-format 0.6.28, disk-hygiene 0.21.3, go-format 0.3.31, powershell-format 0.7.31, ruff-format 0.6.29, source-control 0.55.40), so `check-changelog-parity.sh --check-bump origin/main` failed and a plugin cache pointed at the branch reported "already at the latest version". Each of those eight is rebumped one patch above main's value and its CHANGELOG entry retitled to match; main's entry keeps its own heading and text verbatim below. guardrails (0.31.0) and instruction-placement (0.11.19) were still above main and are unchanged. The dispatcher and the `if` filters in all ten hooks.json files are byte-identical to the pre-merge head. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

claude · 2026-09-02T20:15:12Z

Claude finished @kyle-sexton's task in 44s —— View job

Security review (`/review:security-review`)

Gather context (PR body, prior review threads, HEAD SHA cd5aefb)
Diff since last security-reviewed commit (4d22596) to see what actually changed
Re-check open findings against current head
Post findings

Result: skip gate — no new review needed

git diff origin/main...HEAD --stat at this head confirms the PR diff is still exactly 42 files / 647 insertions / 89 deletions — identical in scope to the last security-reviewed pass, matching the PR description precisely.

The only commit since the last security review (4d22596) is cd5aefb, "Merge origin/main into perf/hook-fanout-consolidation and rebump eight plugins." git diff 4d22596..cd5aefb --stat touches ~200 files, but all of it is unrelated main history (other plugins' CHANGELOGs/READMEs, a references → reference directory rename across several plugins, machine-health audit catalog tweaks, knowledge extraction script changes, check-purged-em-dashes.sh, etc.) plus routine version rebumps — none of it is new content in this PR's diff, since origin/main already contains those commits.

Directly confirmed the previously-flagged files are byte-identical to what was already reviewed:

git diff 4d22596..cd5aefb -- plugins/guardrails/hooks/run-guards.sh \
  plugins/guardrails/hooks/run-guards.test.sh \
  plugins/source-control/hooks/hooks.json \
  plugins/disk-hygiene/hooks/hooks.json \
  plugins/guardrails/hooks/hooks.json

→ no output (zero diff).

So there is no new security-relevant surface to review at cd5aefb. For continuity, the two IMPORTANT findings from the prior passes on this PR are still unresolved in the code at this head (not re-posting as new findings — carrying forward, consistent with how this lane has tracked them across every prior sync):

plugins/guardrails/hooks/run-guards.sh — the dirname() shell function still shadows the builtin for every call any dispatched guard makes in its subshell (not only the one BASH_SOURCE[0] call site it's justified for), and still diverges from GNU dirname for root-level/trailing-slash paths (dirname "/foo" → "" instead of "/"). This can flip hook::in_git_working_tree's scope check the wrong way and fail-open (silently skip) a verify guard for a root-level or trailing-slash-heavy path. Line 66
plugins/disk-hygiene/hooks/hooks.json — the matcher is Bash|PowerShell but the new "if": "Bash(*hygiene.py*)" filter only names the Bash tool; unconfirmed whether Claude Code's if-matching silently drops PowerShell-tool coverage for this destructive-operation kill-switch. Line 10

No new findings from this pass.
· branch perf/hook-fanout-consolidation

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

… the dispatcher - source-control: widen the linkage gate's filter to Bash(*gh *) so wrapped calls (env, sudo, bash -c) launch the gate; README caveat states what the filter covers and what it still cannot see. - disk-hygiene: register the engine gate once per tool. The Bash entry keeps its if filter; a separate PowerShell entry carries none, since an if filter is tool-scoped and PowerShell compound-command matching would skip the kill-switch guard silently. - guardrails: the dispatcher no longer defines a function named dirname (it was inherited by every sourced guard and diverged from GNU); the helper is run_guards::script_dir, used only for the dispatcher's own path. Without jq several guard documents are never concatenated: the blocking one wins, else the first, and the rest go to stderr with a run-guards: dropped prefix. Tests cover both. - guardrails CHANGELOG states the README's measured figures (about 2,450 ms to about 1,220 ms) and that the set remains above the 1 s ceiling. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

The guard tests that read hooks.json asserted exactly one PreToolUse registration of the engine gate; there are now two, one per tool, with the same argv and timeout. A new test locks that shape: the Bash entry carries the if filter, the PowerShell entry carries none. The README's Stop-detector sentence no longer counts it as the "second" hook entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

claude · 2026-09-02T21:59:12Z

Claude finished @kyle-sexton's task in 2m 3s —— View job

Security review (`/review:security-review`)

Gather context (PR body, prior review threads, HEAD SHA 27e3730)
Diff since last security-reviewed commit (cd5aefb) to see what actually changed
Re-check open findings against current head
Post findings

Result: previously-flagged findings resolved, no new issues

Two commits landed since the last security-reviewed head (cd5aefb): 87931d6 ("fix(hooks): address the PR #3621 review threads on the if filters and the dispatcher") and 27e3730 ("test(disk-hygiene): pin the engine gate's per-tool registration shape"). Diffed exactly those two commits (git diff cd5aefb..27e3730) against the files this lane and the code-review lane had flagged across every prior pass on this PR.

Both previously-open IMPORTANT security findings are fixed in the code, not just documented:

dirname shadow — plugins/guardrails/hooks/run-guards.sh#L58-L74. The fast-path helper is no longer named dirname — it's run_guards::script_dir, used only at the dispatcher's own two BASH_SOURCE[0] call sites. A dispatched guard's own dirname calls now resolve to the real external command again, closing the fail-open path in hook::in_git_working_tree's scope check that cli-flag-verify.sh/skill-reference-verify.sh/stale-path-verify.sh rely on. Verified with a new test (run-guards.test.sh "dirname inside a dispatched guard is the external command") that asserts type -t dirname is file (not a function) and dirname /foo returns /, matching GNU semantics.
disk-hygiene's if filter dropping PowerShell coverage — plugins/disk-hygiene/hooks/hooks.json#L5-L23. The single Bash|PowerShell matcher carrying a Bash(...)-only filter is now split: a Bash entry keeps the if filter, and a separate PowerShell entry has none, so PowerShell calls unconditionally reach the destructive-operation kill-switch/data-root gate again (same coverage as before the optimization, at the cost of the interpreter start it was trying to save on that path). Locked in by a new regression test, test_engine_gate_is_registered_once_per_tool, which asserts exactly {Bash, PowerShell} matchers exist, the Bash entry's if starts with Bash(, and the PowerShell entry carries no if at all — this would fail loudly if the coverage regressed again.

Also fixed, tracked by the code-review lane rather than this one but confirmed while diffing: the gh anchoring gap (Bash(gh *) → Bash(*gh *), now catching env GH_TOKEN=x gh …/sudo gh …/wrapper forms) and the jq-less merge fallback (run_guards::emit_one now picks the single document carrying a block/deny decision, else ask, else first — never concatenates multiple JSON documents onto stdout — with dropped documents echoed to stderr). Exit-code aggregation (RC, set in the guard-run loop before any of this document-selection logic) is unchanged and unaffected either way, so which document is chosen for display never changes whether a blocking guard's block actually takes effect.

No new security-relevant issues found in this diff. The new run_guards::emit_one document-selection regex is a plain substring/pattern match against each guard's JSON text rather than a structural JSON check, so a guard whose advisory text happened to literally contain "decision":"block" as prose could in principle be mis-prioritized for display ahead of an actual blocking guard's document — but this only affects which message is shown to the user in the narrow jq-unavailable-or-merge-failed path, never the exit code that actually blocks the tool call, so it doesn't clear this lane's bar for a security finding.

· branch perf/hook-fanout-consolidation

…s (MD038) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ejected settings gate (#3662) No related issue: phase 3 of the hook performance program tracked in #3623; that tracking issue stays open until phase 8. ## Summary Phase 3 of the hook performance program (`if` gates, matcher scoping, explicit timeouts). The two advisory context-guard rows (`zone-crossing-inject.sh` on UserPromptSubmit and PostToolBatch) drop from a 60-second to a 15-second timeout, because a stalled hook on the prompt path holds up the turn and a late advisory nudge is cheap to lose; the `zone-gate.sh` and PostCompact rows keep 60 seconds because a shortened timeout on a gate is fail-open. Every always-on row marketplace-wide is asserted to carry an explicit timeout. The planned `if` gate on the context-budget settings checkpoint was evaluated with a live `claude --debug-file` probe and rejected: on this Windows host Claude Code's `if` file rules do not match an absolute path outside the working directory under any anchoring form (`~/`, `//`, drive-letter, `//**/`), so a gate would silently drop the user-global and managed-settings checks. The row stays unconditioned and the README records the evidence. The claude-ops InstructionsLoaded row stays unscoped for a documented reason (scoping below the documented `load_reason` set would disable a user-facing opt-in). ## Fix - `plugins/context-guard/hooks/hooks.json`: two advisory rows at `timeout: 15`; version 0.7.31, CHANGELOG entry, README sentence. - `plugins/context-budget/README.md`: the rejected-gate record (no functional change, no bump). - `plugins/claude-ops/README.md`: a per-row "why it earns its spawn" column on the audit-hook table and the InstructionsLoaded reason (no functional change, no bump). ## Verification - `for f in plugins/*/hooks/hooks.json` with `jq`: no row without an explicit `timeout`; every row `type: command`. - Live probe (fresh `claude -p --debug-file` sessions, project-level stub hook with sixteen `if` spellings): every rule logged `Skipping hook due to if condition ... not matching` on writes to the user-global and managed-settings paths while the unconditioned row returned `ask`; only an in-cwd settings file matched. - `scripts/affected-tests.sh --run`: exit 0 (every changed file is a recorded no-suite class). markdownlint: 0 issues. No em dashes in added lines. - A fresh-context verifier checked eight binary criteria against the diff: all PASS. - context-guard 0.7.31 delivered to the local plugin cache at this branch head and asserted by `installed_plugins.json` `gitCommitSha`. ## Related - Tracking issue #3623 - PR #3621 (guardrails dispatcher, carries the Bash `if` residual notes) - Plan: `docs/topics/hook-performance-levers/PLAN.md` on branch `perf/hook-performance-program` 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…and renumber instruction-placement instruction-placement 0.11.19 -> 0.11.20: #3666 landed on main with 0.11.19. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude · 2026-09-02T22:29:49Z

Claude finished @kyle-sexton's task in 2m 48s —— View job

Security review (`/review:security-review`)

Gather context (PR body, prior review threads, HEAD SHA 4ac5dff)
Diff since last security-reviewed commit (27e3730) to see what actually changed
Re-check previously-flagged findings against current head
Post findings

Result: skip gate — no new review needed

This head (4ac5dffcd8cc8f688dd27e2e4e4d2941b86cc182) already has a security review that still applies.

gh pr diff 3621 --name-only at this head returns the same 42-file scope as every prior pass, and the full diff content is byte-identical to what was reviewed and confirmed clean at 27e3730/cd5aefb. The commits since the last security-reviewed head (bba16ae #3666 instruction-placement, 6539303 #3662 context-guard timeouts, f4eb65b docs, plus two merge commits) are all unrelated main history pulled in by merges — none of it touches this PR's own changed files, since origin/main already contains those commits independently of this branch.

Directly re-verified the two previously-IMPORTANT findings are still fixed in the code at this head, not just documented:

dirname shadow — plugins/guardrails/hooks/run-guards.sh#L58-L74. The fast-path helper is run_guards::script_dir, not dirname, used only at the dispatcher's own two BASH_SOURCE[0] call sites — a dispatched guard's own dirname calls resolve to the real external command.
disk-hygiene's PowerShell coverage — plugins/disk-hygiene/hooks/hooks.json#L5-L23. Registered once per tool: a Bash matcher entry keeps the if filter, a separate PowerShell matcher entry carries none, so PowerShell calls unconditionally reach the destructive-operation kill-switch/data-root gate. Locked in by test_engine_gate_is_registered_once_per_tool in plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py.

Also re-confirmed present and unchanged: the gh anchoring fix (Bash(gh *) → Bash(*gh *) in plugins/source-control/hooks/hooks.json) and the jq-less merge fallback (run_guards::emit_one picks the document carrying a block/deny/ask decision rather than concatenating multiple JSON documents onto stdout).

No new security-relevant surface in this diff, and no new findings from this pass.
· branch perf/hook-fanout-consolidation

…3621 landed Every conflicted file belongs to #3621's plugins and takes main's text; this branch's own changes stay confined to the three Write formatters. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…dispatches Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ks (#3675) No related issue: phase 4c of the hook performance program tracked in #3623; that tracking issue stays open until phase 8. Stacked on PR #3621 (its base); retarget to `main` once #3621 merges. ## Summary Every Write or Edit of a Markdown file inside a repository fires typos-format, eol-normalizer and markdown-format. On a file with nothing to fix they cost 36 / 41 / 42 spawn-equivalents at a 59 ms spawn floor (16 / 27 / 21 exec'd processes plus about 30 command-substitution forks each). This PR removes the processes that ran before each hook knew the file was relevant, with every decision, message and rewrite byte-identical: sum 118.9 to 79.5 spawn-equivalents, exec'd processes 64 to 38, and each hook's OWN externals on the benign path down to 1 / 1 / 4. ## Fix - All three: `dirname`/`basename` to parameter expansion; the payload read once. - eol-normalizer: one `git check-attr eol text` instead of two, `tr` pipelines to substitution, the NUL sniff to one `read`; the library splits `normalize_eol_plan` from `normalize_eol_apply`, so a file that is already normalized never reaches the rewrite guard (`mktemp`, `cp`, `cmp`, `rm`). The one observable change, disclosed in CHANGELOG and README: such a file is no longer opened for writing, so its mtime is untouched. 27 execs to 11. - typos-format: skip the notebook-normalising `jq` when the raw payload carries `file_path`. 16 to 13. - markdown-format: the existing raw-payload extension check stays the short-circuit; directory walks no longer shell out per level. 21 to 14. - Each suite gains a traced case pinning a ceiling and an allowlist on the hook's own exec'd externals (library frames excluded by a `PS4` carrying `${FUNCNAME[0]}`), so a new spawn on the benign path fails on Linux and Windows alike. - Versions typos-format 0.6.35, eol-normalizer 0.6.28, markdown-format 0.11.38 (each one above `main`, which had claimed the next numbers), CHANGELOG entries and dated README accounting tables with exec and fork counts. ## Verification - Suites: typos-format 137/137, eol-normalizer 47/47, markdown-format 156 pass with the same 4 PATH-shape cases that fail on the base on this Windows host. - 21 decision comparisons against the base (7 cases x 3 hooks: benign `.md` Write and Edit, `.txt`, a misspelling, CRLF, a lint defect, a file outside the repo): stdout, stderr, exit code and file bytes identical; the eol-normalizer mtime difference appears only where nothing was rewritten. - A fresh-context verifier passed all seven criteria, re-tracing the own-frame externals independently (1 / 1 / 4) and matching the README exec totals exactly. - `check-changelog-parity.sh --check` and `--check-preserved` exit 0; shellcheck and markdownlint clean; no em dashes in added lines (byte check). `--check-bump origin/main` fails only on the base branch's own collisions (PR #3621 needs its next merge-forward). ## Related - Tracking issue #3623; base PR #3621; the `hook-utils.sh` share (8 to 10 execs per hook) is the library phase. - Plan: `docs/topics/hook-performance-levers/PLAN.md` on branch `perf/hook-performance-program` 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…landed Every conflicted file outside guardrails takes main's text; guardrails keeps its 0.31.1 entry above main's 0.31.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ect skills manual-only (#3677) No related issue: phase 6 of the hook-performance program, tracking issue #3623 ## Summary Phase 6 of the hook-performance program (skill listing budget). The Claude Code skills doc states that the combined `description` and `when_to_use` text is truncated at 1,536 characters in the skill listing and that the shared listing budget scales at 1% of the model's context window. This PR rewrites the ten largest listing entries in the marketplace to spend fewer characters while keeping every single-quoted trigger phrase and every `Not for` / `Skip when` / sibling disambiguation verbatim, and records the outcome of the manual-only (`disable-model-invocation: true`) sweep over side-effect skills. Baseline finding: the over-cap clause of the program ruling was already satisfied. The largest entry was 1,526 characters, so zero entries exceeded the 1,536 cap before this PR and zero exceed it after. The substance here is the aggregate reduction. ## Fix Ten `SKILL.md` descriptions tightened (old chars, new chars): | Skill | Before | After | |---|---|---| | claude-ops:audit-performance | 1526 | 1356 | | claude-ops:audit-skill-visibility | 1511 | 1391 | | claude-config:audit-prompting-postures | 1316 | 1228 | | work-items:decompose | 1266 | 1251 | | skill-quality:check | 1248 | 1019 | | improvement:find | 1245 | 1212 | | claude-ops:audit-native-overlap | 1236 | 1153 | | code-tidying:dissolve-comments | 1194 | 1173 | | overengineering:audit | 1192 | 1177 | | architecture:improve | 1165 | 1154 | What was cut: mechanism narration and parenthetical enumerations (the suspect labels in audit-performance, the twenty-five-check list in check, the verdict enumeration in audit-native-overlap, the budget-drop explanation in audit-skill-visibility). What was kept: every quoted trigger phrase, every `/plugin:skill` reference inside the disambiguation sentences (those references are the citation edges the manual-only rule reads), and the argument names. No description grew. Manual-only sweep: no skill gains `disable-model-invocation: true`. The rule applied was "a skill cited as `/<plugin>:<skill>` by any external file (SKILL.md, agent, plugin README, or skill spoke) is not a candidate." Every listing-eligible side-effect skill has at least one external citing file. Three side-effect skills are cited only by their own plugin README (overengineering:realign, coupling:reduce, code-tidying:dissolve-comments); they would be candidates under a stricter presence-gate reading and are listed for the program lead to re-rule. Deferred (plugin has an open PR): source-control:babysit-loop sits in the top ten at 1,196 characters and was not touched. instruction-placement:realign is a side-effect skill in a no-touch plugin. Eight plugins get a patch bump and a `## [<version>]` CHANGELOG entry: claude-ops 0.41.8, claude-config 0.40.32, work-items 0.39.53, skill-quality 0.20.11, improvement 0.1.7, code-tidying 0.15.4, overengineering 0.3.6, architecture 0.6.8. ## Verification Listing-budget script, before (`bash plugins/skill-quality/scripts/check-listing-budget.sh plugins/*/skills`): ```text Shared listing-budget estimate over 182 listing-eligible skill(s) across 74 root(s): aggregate: 136209 chars budget: 8000 chars (documented default (SLASH_COMMAND_TOOL_CHAR_BUDGET fallback)) ``` After: ```text Shared listing-budget estimate over 182 listing-eligible skill(s) across 74 root(s): aggregate: 135424 chars budget: 8000 chars (documented default (SLASH_COMMAND_TOOL_CHAR_BUDGET fallback)) ``` Aggregate reduction: 785 characters (0.58%). Over-cap entries (combined description plus when_to_use over 1,536): 0 before, 0 after, computed per skill from frontmatter across all 243 SKILL.md files (182 listing-eligible). Phase 0 recorded 178 eligible skills at 132,552 characters; the fleet grew by four skills between phase 0 and this baseline. `check-skill.sh` (with `CHECK_SKILL_SKILLS_ROOT` set per plugin and `CHECK_SKILL_SKIP_MARKDOWNLINT=1`, matching the repo gate): PASS on nine of ten, each reporting "all N base-ref trigger phrase(s) preserved" for check 3. audit-skill-visibility reports every trigger phrase preserved but FAILs on its bundled script test, `scripts/audit_skill_visibility.test.sh`: all 96 Python unit tests pass and the shell fixture then errors with `no installed skills resolved from '<mktemp dir>/cfg'`. That fixture does not read SKILL.md and the only change to that skill is the description line, so this is a Windows-host fixture-path issue, not a regression from this PR; CI runs the suite on Linux. `bash scripts/check-changelog-parity.sh --check-bump origin/main`: exit 0 after re-fetching origin/main. `bash scripts/check-purged-em-dashes.sh`: exit 0 (86 declared paths, 105 files scanned, no em dashes). Em-dash line counts per touched file did not grow versus HEAD; architecture:improve dropped from 2 lines to 1. `bash scripts/affected-tests.sh --explain`: no suites selected. Every changed file (SKILL.md, CHANGELOG.md, plugin.json) is a recorded no-suite class covered by a non-shell CI lane. ## Related - Tracking issue #3623 (hook-performance program) - Merged program PRs: #3621, #3662, #3666, #3668 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…guards (#3676) No related issue: phase 4a of the hook performance program tracked in #3623; that tracking issue stays open until phase 8. Stacked on PR #3621 (its base); retarget to `main` once #3621 merges. ## Summary The guardrails dispatcher runs eight Bash guards per Bash call and three verifier guards per Write. Profiled on this Windows host, `skill-reference-verify.sh` built a plugin index with two `jq` per manifest (about 150 `jq` spawns, 11.4 s) before knowing whether the written content cited any skill, `block-convention-violation.sh` forked the convention resolver twice per Bash call, and several guards spent `dirname`, `basename`, `git rev-parse` and `jq` on paths the dispatcher had already primed. This PR removes those spawns with every decision byte-identical. ## Fix - `skill-reference-verify.sh`: the plugin index is built only after the content is known to reference a skill; on a benign in-repo Markdown write the PostToolUse dispatch drops from 155 `jq` execs and 151 manifest reads to 7 and 1. - `block-convention-violation.sh`: the two resolved patterns are cached per repo root in plugin data, keyed on the convention file's path and mtime, so the resolver forks once per convention change; contract cases prove the cached pattern equals the forked form and that an mtime change re-forks. - Benign-path spawn cuts across the Write guards and verifiers (parameter expansion for `dirname`/`basename`, primed fields instead of re-run `jq`/`git`). - Version 0.31.1 with a CHANGELOG entry and a dated README accounting section (two paired passes, spawn-equivalents with the floor stated; the first pass's PreToolUse Write credit is retracted in place, since that path did not move). ## Verification - 12 interleaved trials, base versus head, telemetry sink unset: PostToolUse:Write on an in-repo `.md` 18200 to 3841 ms (389.7 to 82.2 spawn-equivalents); PreToolUse:Bash benign 2494 to 2042 ms (55.8 to 45.7). With the sink set: 19435 to 5146 ms and 3969 to 3467 ms. - Decision equivalence against the base on nine payloads (benign Bash, `$()`, two MUST-fire git payloads, `git commit`, in-repo PostToolUse and PreToolUse Write, a secret-bearing Write, a synthetic-repo convention violation and a conforming subject): stdout, stderr and exit code identical, cold and warm. - Suites: 15 of 17 green; the two failing on this Windows host (`block-windows-drive-tmp.test.sh`, `hardcoded-path-check.test.sh`) fail the identical case sets on the base. - A fresh-context verifier passed all eight criteria, re-tracing the dispatch itself (base 155 `jq` execs, head 7). - `check-changelog-parity.sh --check` and `--check-preserved` exit 0; shellcheck and markdownlint clean; no em dashes in added lines (byte check). ## Related - Tracking issue #3623; base PR #3621. Residual: `hook::emit_telemetry` inside the synced `hook-utils.sh` is about 1.4 s (33.8 spawn-equivalents) of the Bash wall when a telemetry sink is set; that is the library phase. - Plan: `docs/topics/hook-performance-levers/PLAN.md` on branch `perf/hook-performance-program` 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…h builtins (#3678) No related issue: phase 4b of the hook-performance program, tracking issue #3623 ## Summary With `HOOK_TELEMETRY_SINK` wired, every guard that reached `hook::emit_telemetry` paid two jq processes, a `mktemp` and an `rm` per run, and `hook::read_file_path` cost a jq plus four `realpath` and two `cygpath` processes on every Write and Edit. On Windows Git Bash each spawn is tens of milliseconds; the phase 4a profile put the telemetry cost at about 1.2 s of the 2.5 s Bash dispatcher wall across five guards. Both functions now do their work with shell builtins and hand anything they cannot prove to the unchanged jq path. ## Fix `hook::emit_telemetry` - The envelope is assembled in the shell: `hook::json_escape_jq_to` escapes the string fields exactly as jq does (backslash, quote, `\b \f \n \r \t`, other C0 bytes and DEL as `\u00xx`, non-ASCII verbatim) and `hook::json_compact_to` compacts the caller's data object exactly as `jq -c` does when that can be proven. jq's own output (pretty or compact), and the compact literal fallbacks the hooks carry, all qualify. - Anything not proven (a `\u` or `\/` escape, a raw control byte, a fraction, an exponent, a 16-digit integer, a non-object, invalid JSON) goes to the jq path, which now runs `jq -nc` so both paths write the same bytes. jq's absence is fail-open only there. - One deliberate change in the emitted bytes, flagged for review: the old filter ran `jq -n` without `-c`, so today's envelope is jq's pretty-printed multi-line document (CRLF line endings with a Windows jq). The new envelope is one compact line. The sink contract is one JSON document on stdin; the repo's own sink appends a byte-identical record for both forms (proof below), and a compact line is also valid JSONL for a sink that appends raw envelopes. The hook-telemetry convention README now states the compact-line shape and that sinks must parse JSON rather than line layout. - No `mktemp`, no `rm`, no temp file on the builtin path. The timestamp uses `printf -v '%()T'` with a `date -u` fallback when printf binds nothing. - Test 2 (`jq absent`) is rewritten: it asserted "returns 0 with no output" because the envelope needed jq; it now asserts the envelope is delivered with an empty PATH and with a failing `jq` shadow that is never invoked. `hook::read_file_path` - stdin is read into the shell (NUL-safe) and `.tool_input.file_path` is taken by `hook::_fast_file_path_to` when it can prove jq's answer: the payload is one JSON value by the grammar (every string escape one jq accepts, no raw control bytes), exactly one string in the whole payload decodes to `tool_input` and exactly one to `file_path`, `tool_input` is a direct member of the root object whose value is a flat object (no nested object or array), and `file_path` is a plain string inside it. Key strings are compared after decoding escapes. A top-level `file_path`, a `tool_input` nested elsewhere, two keys, a non-string value, a payload over 64 KiB, a NUL byte, or a non-ASCII `\u` escape all fall back to the unchanged jq filter. - No per-character scanning of the payload: the only whole-payload operations are literal substring replacement, containment tests, IFS splitting and offset slicing (bash regex and `%%` on a 60 KB payload cost more than the jq process they would replace on this host). - The membership comparison resolves the file, the project root and every temp-root candidate with one batched `realpath` (`hook::_physical_prime`) into a per-process cache of plain indexed arrays (Bash 3.2 safe), and `hook::under_temp_root` reads the cache. The file's entry is forgotten after use so a later call in the same process re-resolves it. - The git branch uses `hook::dirname_to` (builtin dirname with the `.` and `/` fallbacks). - Every existing helper keeps its printing form; `_to <var>` variants store into a variable so the hot path forks nothing it does not need. No check was removed. `hook::json_escape` (a different tool: it drops residual control bytes, which is right for a notice) is unchanged. ## Verification Differential harness under `$TEMP/phase4b/` (`emit-diff.sh`, `rfp-diff.sh`, `exec-count.sh`, `rfp-time.sh`): the OLD lib from `git show origin/main:lib/hook-utils.sh` and the NEW lib run in separate bash processes on the same inputs. ### emit_telemetry (28 cases, 0 diffs, 9 take the jq fallback) Columns: `document` is `jq -c .` of old vs new; `compact` is the NEW raw bytes vs `jq -c .` of the OLD (the builtin escaper and compactor reproduce jq's compact rendering byte for byte); `sink row` is the record `.claude/hooks/hook-telemetry-sink.sh` appends to `hook-events.jsonl` for old vs new. `timestamp` and `duration_ms` are masked (nondeterministic across two processes) and the timestamp shape is asserted separately. | # | case | new path | document | compact | sink row | note | |---|---|---|---|---|---|---| | 1 | plain | builtin | identical | identical | identical | | | 2 | empty data {} | builtin | identical | identical | identical | | | 3 | nested arrays and objects | builtin | identical | identical | identical | | | 4 | strings with quotes | builtin | identical | identical | identical | | | 5 | strings with backslashes | builtin | identical | identical | identical | | | 6 | newlines and tabs | builtin | identical | identical | identical | | | 7 | control chars (ESC, 0x01, DEL) | jq-fallback | identical | identical | identical | | | 8 | 35 KB blob | builtin | identical | identical | identical | | | 9 | non-ASCII | builtin | identical | identical | identical | | | 10 | status with a quote | builtin | identical | identical | identical | | | 11 | empty hook_id | builtin | identical | identical | identical | | | 12 | pretty jq -n data (guard shape) | builtin | identical | identical | identical | | | 13 | compact literal fallback | builtin | identical | identical | identical | | | 14 | hand-written escaped slash | jq-fallback | identical | identical | identical | | | 15 | hand-written unicode escape | jq-fallback | identical | identical | identical | | | 16 | data not an object (array) | jq-fallback | identical | identical | no-record | sink rejects the envelope on both sides (`data.tool` lookup on an array) | | 17 | numbers (jq-rendered) | jq-fallback | identical | identical | identical | | | 18 | numbers (hand-written literals) | jq-fallback | identical | identical | identical | | | 19 | integers only | builtin | identical | identical | identical | | | 20 | hook_event with control chars | builtin | identical | identical | no-record | sink writes no record on both sides | | 21 | all string fields non-ASCII | builtin | identical | identical | identical | | | 22 | invalid JSON data | jq-fallback | both-empty | both-empty | both-empty | no envelope on either side | | 23 | data with trailing newline | builtin | identical | identical | identical | | | 24 | findings with messages (markdownlint shape) | builtin | identical | identical | identical | | | 25 | raw DEL inside data string | jq-fallback | identical | identical | identical | | | 26 | pretty data with raw tab indent | builtin | identical | identical | identical | | | 27 | subject with a Windows path (guard shape) | builtin | identical | identical | identical | | | 28 | empty data string | jq-fallback | identical | identical | identical | | ### read_file_path (42 cases: 41 identical, 1 intended difference, 7 take the jq fallback) Same stdin, same environment, separate processes; stdout and return code compared. | # | case | new path | old rc/out | new rc/out | verdict | |---|---|---|---|---|---| | 1 | normal Write, in-project (compact) | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 2 | Edit shape | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 3 | pretty payload (jq -n fixture shape) | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 4 | path with spaces | builtin | 0/PROJ/with space.md | 0/PROJ/with space.md | identical | | 5 | path with escaped quote (no such file) | builtin | 1/ | 1/ | identical | | 6 | Windows drive path, backslashes | builtin | 1/ | 1/ | identical | | 7 | Windows drive path, forward slashes | builtin | 1/ | 1/ | identical | | 8 | path with A escape | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 9 | path with é escape (non-ASCII) | jq-fallback | 0/PROJ/é.md | 0/PROJ/é.md | identical | | 10 | path with raw non-ASCII | builtin | 0/PROJ/é.md | 0/PROJ/é.md | identical | | 11 | path with \r | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 12 | missing file | builtin | 1/ | 1/ | identical | | 13 | file outside project | builtin | 1/ | 1/ | identical | | 14 | temp file, project outside temp | builtin | 1/ | 1/ | identical | | 15 | project under temp | builtin | 0/HOMEPROJ/scratch/tmpfile.md | 0/HOMEPROJ/scratch/tmpfile.md | identical | | 16 | in-project file, project outside temp | builtin | 0/HOMEPROJ/inside.md | 0/HOMEPROJ/inside.md | identical | | 17 | symlinked file escaping the project | builtin | 1/ | 1/ | identical | | 18 | two file_path keys at different depths | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 19 | tool_input with a nested object | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 20 | tool_input with a nested array of objects | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 21 | file_path at top level only | builtin | 1/ | 1/ | identical | | 22 | 70 KiB payload | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 23 | 40 KiB payload (under the cap) | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 24 | no CLAUDE_PROJECT_DIR, inside a git tree | builtin | 0/GITREPO/tracked.md | 0/GITREPO/tracked.md | identical | | 25 | no CLAUDE_PROJECT_DIR, outside any git tree | builtin | 1/ | 1/ | identical | | 26 | no CLAUDE_PROJECT_DIR, git absent | builtin | 1/ | 1/ | identical | | 27 | empty file_path | builtin | 1/ | 1/ | identical | | 28 | file_path is a number | builtin | 1/ | 1/ | identical | | 29 | no file_path at all (Bash payload) | builtin | 1/ | 1/ | identical | | 30 | tool_input nested in another object | builtin | 1/ | 1/ | identical | | 31 | truncated after tool_input | builtin | 1/ | 1/ | identical | | 32 | payload with a trailing NUL byte | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 33 | key spelled with \u escapes | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 34 | duplicate file_path key inside tool_input | jq-fallback | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 35 | root is an array | builtin | 1/ | 1/ | identical | | 36 | empty stdin | builtin | 1/ | 1/ | identical | | 37 | invalid JSON (bad literal) | builtin | 1/ | 1/ | identical | | 38 | invalid escape in an unrelated string | builtin | 1/ | 1/ | identical | | 39 | bad \u hex in an unrelated string | builtin | 1/ | 1/ | identical | | 40 | project dir with trailing slash | builtin | 0/PROJ/plain.md | 0/PROJ/plain.md | identical | | 41 | prefix sibling of the project | builtin | 1/ | 1/ | identical | | 42 | jq absent, in-project Write | builtin | 1/ | 0/PROJ/plain.md | intended difference | Cases 38 and 39 are payloads jq rejects outright (an invalid escape anywhere in the text); the skeleton pass validates every escape in every string, so the fast path never proves a value from a text jq would refuse. Case 42 is the one behavior change on this function and it is intended: with jq absent the old reader could never extract a path (the jq filter failed, so every file was skipped); the builtin path needs no jq, so a well-formed payload is admitted. Every hook in this marketplace gates on `hook::require_jq` or `hook::require_jq_blocking` before reaching the reader, so no shipped hook observes it. ### Exec count and wall time: typos-format PostToolUse Write, sink wired `plugins/typos-format/hooks/typos-format.sh` run from a scratch copy of the plugin dir with the OLD and NEW lib swapped in (the carrier copy was never edited), on an in-repo scratch `.md`, `HOOK_TELEMETRY_SINK` pointed at a file sink, `CLAUDE_PROJECT_DIR` set. External programs counted from a `PS4='+X ' bash -x` trace; wall is the median of 5 runs. | lib | external execs | wall median (ms) | 5 runs (ms) | envelopes delivered | |---|---|---|---|---| | old (origin/main) | 24 | 1130 | 1169, 1130, 1099, 1100, 1307 | 6 | | new | 17 | 861 | 994, 782, 839, 885, 861 | 6 | A second run of the same harness on the final lib, on a busier host: old 24 execs / 1390 ms median, new 17 execs / 994 ms median. By program, old: jq 7, realpath 4, cygpath 4, tr 3, rm 1, mktemp 1, git 1, basename 1, sink 1, typos 1. New: jq 5, cygpath 4, tr 3, realpath 1, git 1, basename 1, sink 1, typos 1. The remaining jq processes belong to the hook itself (`buffer_stdin` validation, tool_name, `build_data_json`, findings), not to the lib; the four cygpath are `hook::repo_relative_path` and `expand_8dot3` on an 8.3-spelled temp candidate, both out of scope here. `hook::read_file_path` alone (median of 7, `CLAUDE_PROJECT_DIR` set, in-project file): | payload | old (ms) | new (ms) | |---|---|---| | 102 bytes (plain Write) | 507 | 118 | | 40 KB content | 567 | 160 | | 70 KB content (jq fallback) | 538 | 243 | ### Suites (one at a time, this Windows host) | suite | result | |---|---| | `lib/hook-utils.test.sh` | PASS=284 FAIL=0 (was 224 on main; new cases: 14b escaper corpus, 14c compactor corpus including invalid escapes, 3b envelope bytes and fallback, 12g fast-path parity, top-level key, invalid escape elsewhere, 64 KiB threshold both sides, 12h dirname_to, Test 2 rewritten) | | `.claude/hooks/hook-telemetry-sink.test.sh` | PASS (2 checks), exit 0 | | `lib/rewrite-guard.test.sh` | 16 passed, 0 failed | | `plugins/guardrails/hooks/run-guards.test.sh` | PASS=66 FAIL=0 | | `plugins/guardrails/hooks/block-windows-drive-tmp.test.sh` | PASS=199 FAIL=15, the 15 are the known host-only `/usr/bin/mkdir /tmp/x` cases on this Windows machine; the telemetry assertions (now read through jq) pass | | `plugins/markdown-format/hooks/markdown-format.test.sh` | PASS=162 FAIL=4, the 4 are the known host-only PATH-shape cases; the unwired path asserts one jq spawn (the stdin probe) now that file_path is read with builtins | First CI run failed on exactly those two suites (they grepped the pretty-envelope spelling and counted two jq spawns) plus a typos hit on two deliberate JSON-typo test literals; all three are fixed in the last commit. `scripts/affected-tests.sh --explain` selects 142 suites for this change (not run on this host per the shared-host discipline; CI runs everything). ### Gates - `shellcheck --rcfile .shellcheckrc lib/hook-utils.sh lib/hook-utils.test.sh`: clean - `bash scripts/check-shell-portability.sh origin/main`: exit 0 (three `\b` sites annotated `portability-ok`, they are bash ANSI-C backspace bytes, not GNU grep word boundaries; one `realpath` multi-operand site annotated) - `bash scripts/sync-hook-utils.sh --check`: all 17 copies match - `bash scripts/sync-hook-utils.sh --check-bump origin/main`: exit 0 (origin/main re-fetched at 9f07fb5 before the bump) - `bash scripts/check-changelog-parity.sh --check-bump origin/main`: exit 0 - `markdownlint-cli2 plugins/*/CHANGELOG.md`: 0 issues - em dash count did not grow in any touched file (lib 141 to 140, test 158 to 158) - Bash 3.2: no associative arrays, no `${var,,}` outside the existing OSTYPE guard, `%()T` guarded by a `date -u` fallback ## Related - #3623 (tracking issue, hook-performance program) - Merged: #3621, #3662, #3666, #3668 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

No related issue: phase 8 of the hook-performance program, tracking issue #3623 ## Summary The hook-performance program landed nine PRs today (#3621, #3662, #3666, #3668, #3671, #3675, #3676, #3677, #3678, plus dotfiles #620). This PR records the outcome where the convention says it lives: the hook-budget convention gains a dated reference-figures section in spawn-equivalents with the harness sha256, and the four plugins that lacked a hook budget accounting row gain one under Requirements. Documentation only; no hook, script or registration changes. ## Fix - `docs/conventions/hook-budget/README.md`: a "Reference figures (2026-09-02, after the hook-performance program)" section with the harness identity, the before-and-after spawn-equivalents per surface (before at S = 33 ms, after at S = 18 ms on `main` `5e3d749cb`), the reference-host conversion, and a plain reading against the budget table: per-turn rows meet the 500 ms ceiling; per-tool-call rows sit at 1.4 to 1.9 s against the 1 s typical ceiling, with the guardrails dispatcher named as the whole of the remainder. - `plugins/typos-format/README.md`, `plugins/eol-normalizer/README.md`, `plugins/markdown-format/README.md`, `plugins/context-guard/README.md`: a "Hook budget accounting" section each, carrying the measured rows their CHANGELOG entries already state (36.3 to 26.0, 41.0 to 21.5, 41.6 to 32.0 spawn-equivalents; 11 to 2, 9 to 4 and 6 to 1 processes for context-guard), what changed, and the residual. guardrails and rate-limit-guard already carried one. - Version bumps with a documentation-only CHANGELOG entry: typos-format 0.6.37, eol-normalizer 0.6.30, markdown-format 0.11.40, context-guard 0.7.36. ## Verification - Final harness run on the installed cache at `main` `5e3d749cb`, `--runs 3`, S = 18 ms, valid, quiet host; every measured plugin's cache directory byte-compared against `origin/main` (0 stale files in 17); `enabledPlugins` unchanged against the pre-program snapshot; 52 `hooks.json` entries listed, every one `type: command`, no `async` row. The per-event block, the STATED CHECK and the reading against goal (B) are in the program's PLAN.md and DEVIATIONS.md on `perf/hook-performance-program`. - Per event (ms, slowest hook): PreToolUse:Bash 2,475 before to 1,599 after; PostToolBatch 1,254 to 282; UserPromptSubmit 975 to 297; in-repo PostToolUse:Write 13,225 to 1,949; in-repo PostToolUse:Edit 17,192 to 3,048. - `markdownlint-cli2` 0 issues on the five files; em dash counts unchanged; `scripts/check-changelog-parity.sh --check-bump origin/main` exit 0; `scripts/affected-tests.sh --explain` selects no suites (every changed file is a recorded no-suite class covered by a non-shell CI lane). ## Related - #3623 (tracking issue) - Merged today: #3621, #3662, #3666, #3668, #3671, #3675, #3676, #3677, #3678; dotfiles #620 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

…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>

…s file name (#3880) Closes #3349 ## Summary Every PowerShell tool call in every session was still launching the disk-hygiene destructive guard to be told it was irrelevant. The Bash entry in `hooks/hooks.json` has carried `"if": "Bash(*hygiene.py*)"` since 0.21.4; the PowerShell entry carried no filter, on the 0.21.4 rationale that "a PowerShell filter must match every subcommand of a compound command and would skip this kill-switch guard silently on a mixed line". Where the cost actually goes (measured, not assumed): on a warm interpreter cache the hook spends **0 process forks** and **4 `execve` calls** (`bash -c`, the launcher through its `#!/usr/bin/env bash` shebang, bash, python3) plus a 106 KB module import, counted with `strace -f -e trace=clone,clone3,fork,vfork,execve` on the real dispatched command string; the one creation is the guard's own watchdog thread. Cold path adds `chmod`, `mv` and a probe interpreter (6 creations, 7 `execve`). On this Linux host the hook process walls at p50 44 ms (n = 20 per tool, min 43, max 58) against a `bash -c :` floor of 2 ms, about 22 spawn-equivalents. That is a spawn-chain-and-interpreter-start cost, not anything the guard computes, which is why the issue's suggested fixes 1 and 2 (lazy regex table, cached `MIN_PYTHON`) would not have bought it back: fix 2 already landed in 0.21.0, and regex compilation is microseconds. The only fix that removes the cost is not spawning. The 0.21.4 rationale carried allow-rule semantics over to `if`, where they do not apply. Verified in the installed Claude Code 2.1.258 binary: the hook `if` evaluator resolves the rule through the tool's own `preparePermissionMatcher`, and the PowerShell tool's parses the command AST, collects every statement's commands plus nested commands, and returns `some(...)`, a case-insensitive whitespace-normalised glob match against each `name args` text; an unparsable command runs the hook. The hooks reference documents `"if": "PowerShell(Remove-Item *)"` as the PowerShell spelling. So `Get-Date; python hygiene.py` still runs the hook. ## Fix - `hooks/hooks.json`: the PowerShell entry carries `"if": "PowerShell(*hygiene.py*)"`, the same content as the Bash entry for its own tool. No change to any Python; the guard is byte-identical. - `test_hygiene.py`: `test_engine_gate_is_registered_once_per_tool` now asserts both filters (it previously asserted the PowerShell entry had none, with the stale rationale in its docstring). Two new tests: `test_powershell_if_filter_skips_only_calls_the_gate_would_defer` (a skipped PowerShell call, including `Remove-Item`, `.NET Delete`, zero-width and split spellings, is one `_engine_gate_relevant` rejects, so the gate deferred it before any deletion spelling was consulted) and `test_powershell_if_filter_admits_every_engine_invocation_shape` (`;`, `|`, `&&`, newline, CR LF, U+2028, call operator, nested `pwsh -Command`, tab and upper-case forms are all admitted by a reference of the 2.1.258 matcher, all relevant, and all still denied by `powershell_decision`). - `run-python-hook.test.sh`: a kernel-level spawn census (`strace -f`, skipped where strace is absent, ptrace is refused, or python3 is a shim script): a warm launch creates no process and execs exactly bash and the interpreter. - README hooks paragraph and hook-budget accounting record the before and after census; CHANGELOG `[0.21.10]`; manifest 0.21.9 -> 0.21.10. Why this is behaviour-identical for every call that reaches the guard, and lossless for every call that no longer does: `_decide` in engine-gate mode returns before any PowerShell mutation regex when `_engine_gate_relevant` is false, and relevance requires the engine's file name as a token or a separator-bearing word that is the same file as the bundled engine. The second case (a symlink or hard link under another name) is the residual the filter cannot see; the Bash lane has accepted it since 0.21.4 and it is stated in the CHANGELOG, along with a comment naming the engine, which the AST-based matcher assigns to no command. ## Verification - A/B against a pristine `git archive origin/main` export, running both trees through the registered command string (`bash -c`, payload on stdin): 33 payloads (benign, deletion spellings, engine invocations in every compound form, CR LF, U+2028, BOM, zero-width, near-misses like `test_hygiene.py` and `hygiene.pyc`, three Bash lane controls), identical on exit code, stdout and stderr, and every payload the predicate skips is one the pristine gate deferred (rc 0, no output). - Mutation checks, each on a tree copy: dropping the PowerShell `if` fails the shape test (`'PowerShell(*hygiene.py*)' != None`); reintroducing a `$(dirname ...)` fork in the launcher fails the census (creations 0 became 1); removing `casefold()` from `_carries_marker` fails the admitted test on the upper-case case; widening it to `startswith("hygiene")` fails the skipped test twice. - `scripts/affected-tests.sh --run`: 330 suites selected by the transitive basename closure; every shell suite passes except `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh`, whose two "process budget" failures reproduce identically on the clean `origin/main` export (pre-existing, not this change). The 13 NOT RUN suites from other ecosystems were run from their own lanes: `test_hygiene` 340 OK, `test_guard_launch_monitor` 23 OK, `test_hook_telemetry` 3 OK, the nine other Python suites OK, the `.mjs` suite 1/1. - Pinned ruff (`scripts/run-ruff.sh check plugins/disk-hygiene`) clean; ShellCheck and shfmt clean on the extended suite; markdownlint clean on README and CHANGELOG; `check-purged-em-dashes.sh` clean; ai-slop detector 0 findings on both prose files; `check-changelog-parity.sh` green in `--check`, `--check-order`, `--check-bump origin/main` and `--check-preserved origin/main`. - Measurement after: a PowerShell call that does not name the engine now costs this plugin 0 creations and 0 `execve`, because the harness never spawns the command; a call that names it pays the unchanged 4 `execve` / 0 forks and is judged unchanged. `execve` for the admitted path is unchanged, which is the evidence this is latency removed by not launching, not work removed from the guard. Acceptance criteria from the triage brief that are **not** met, stated plainly: - The fresh measurement was taken on Linux (n = 20 per tool, spawn floor as the control), not on a Windows/Git Bash host, and there is no Windows wall-clock figure here; the strace census is the host-independent number. No wall-clock figure for Windows is claimed. - The harness-side skip is asserted through a reference implementation of the 2.1.258 PowerShell matcher and the extracted evaluator, not through an end-to-end PowerShell tool call under Claude Code, which this host cannot run. - The issue thread has not been given a supersession comment for the stale 2,427 ms figure; the numbers above are in this PR only. ## Related - Umbrella #3347; siblings #3348 and #3351 share this plugin's CHANGELOG and manifest (touched only this entry). - #3621 (0.21.4) introduced the Bash-lane filter and the per-tool split this completes. - `.claude/rules/hook-budget.md` and `docs/conventions/hook-budget/README.md` (rule 1: measured share stated in the plugin README). 🤖 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>

kyle-sexton and others added 2 commits September 1, 2026 23:00

perf(hooks): dispatch the guardrails guards through one process and i…

4d15818

…f-gate the path-specific hooks Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

docs(hooks): record the dispatcher and the if gates in the plugin REA…

a423e2c

…DMEs and changelogs Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP

chatgpt-codex-connector Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread plugins/guardrails/hooks/run-guards.sh

Comment thread plugins/source-control/hooks/hooks.json Outdated

Comment thread plugins/guardrails/hooks/run-guards.sh Outdated