perf(guardrails): cut the benign-path spawns inside the dispatcher's guards - #3676
Merged
Merged
Conversation
…f-gate the path-specific hooks Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…DMEs and changelogs Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…ts, shellcheck info findings) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
… file Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
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
Main re-bumped seven of this branch's ten plugins to the very numbers the branch had already claimed, and moved source-control and disk-hygiene past them, so the branch could not merge and its versions were ambiguous. Conflict resolution keeps main's content and re-lands this branch's changes on top: - Ten plugin manifests take main's file wholesale (main re-encoded the em dash in several descriptions as \u2014) and carry a version one patch above main's: ruff-format 0.6.29, bash-format 0.7.29, biome-format 0.6.28, go-format 0.3.31, powershell-format 0.7.31, actionlint 0.8.28, instruction-placement 0.11.19 (0.11.18 is claimed by another open branch), source-control 0.55.40, disk-hygiene 0.21.3. guardrails keeps 0.31.0, still above main's 0.30.4. - Ten changelogs carry this branch's entry retitled to that number and placed above every entry main added, with main's entries intact. actionlint, bash-format and instruction-placement manifests auto-merged at the colliding number and are re-bumped here explicitly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…reference exists
skill-reference-verify built its plugin name -> directory index from every
plugin manifest before it had looked at whether the written content cites a
skill at all. Two jq processes per manifest, 76 manifests in this marketplace:
152 jq plus 149 tr spawns on a PostToolUse:Write of a file with no references,
measured at 11,430 ms (272 spawn-equivalents against a 42 ms spawn floor) on
Windows Git Bash. That was the entire PostToolUse:Write cost of this plugin.
Nothing below the reference scan reads the index, and the scan finds its
candidates with no plugin knowledge, so the build moves into a function called
once, only when the scan produced a candidate. The plugins-root and manifest
gates stay exactly where they were: their exit paths deliberately skip
telemetry, and moving them would change that.
The repo-root anchor also stops forking: ${FILE%/*} with a slashless fallback
replaces $(dirname "$FILE"), one fewer subshell per Write and Edit.
Tests count the manifest reads through a jq shim rather than asserting the
source text: zero manifest reads for a write that cites nothing, a non-zero
count when it cites something, and exactly two per manifest when it cites two
references, which is what proves the index is built once.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
block-convention-violation forked the convention resolver twice on every Bash and PowerShell tool call, because the resolver answers one key per call. On Windows Git Bash that measured 423 ms, 10.1 spawn-equivalents against a 42 ms spawn floor, paid on every command the agent runs, to re-derive an answer that changes only when the convention files change. The resolved pair is now cached per repo root under the plugin data directory and invalidated by mtime against every file the resolver reads: the team markdown, the well-known neutral YAML, and an explicit convention_source target when one is declared. Freshness is the `-nt` builtin rather than a stat process, so a warm run spawns nothing at all. The resolver itself is untouched and stays the only authority for what a pattern is; only its answer is stored. Equal mtimes read as not-newer, so a same-tick write re-resolves instead of serving a stale pattern. The entry records the root it was resolved for and ends with a terminator line, so neither a filename collision between two roots nor a truncated write can quietly disable the gate. With no plugin data directory the gate simply does not cache. Known residual, recorded in the file: the resolver's well-known rung honours its YAML only when git reports it tracked, and tracked status can change with no mtime change. Such a change is picked up when a dependency is next written. Probing it would cost the git spawn this cache exists to remove. Tests compare the cached patterns against the forked patterns value by value, not decision by decision, count resolver forks through a shim to prove a warm run makes none, and drive a real convention change through touch -d so the invalidation is a genuine mtime move rather than two writes inside one tick. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…es on ruled-out work Three more always-on guards did expensive setup before checking whether the payload could ever produce a finding. stale-path-verify listed the entire git index on every Write and Edit. A write that cites no inline-code token has no candidate for that list to answer about, so the warm now happens only when the token scan produced one. It still happens in the same shell and still precedes the candidate loop, which is what the subshell-assignment fix it carries requires. cli-flag-verify ran its fragment pipeline, three processes, on content that cannot contain a flag. Every candidate it can produce is a (bin, chain, flag) triple and a flag token always carries a dash, so content with no dash anywhere is now skipped ahead of the scan. The Edit reconstruction takes the same gate, since its anchor is a flag token from the hunk. secret-pattern-detection, cli-flag-verify and stale-path-verify each resolved the repository root through a $(dirname ...) subshell, a fork per call on Windows Git Bash, and now use parameter expansion with a slashless fallback. Nothing that is reported changes: the skipped work is the scan, never the report, and telemetry and the exit paths are reached exactly as before. Version bumped to 0.31.1 with the accounting recorded in the CHANGELOG. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
t.jsonl is the transcript_path a Phase 4a profiling payload names. It is measurement scaffolding and was never meant to be tracked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
… residual The README's budget accounting gains the 0.31.1 measurement beside 0.31.0's. 0.31.0 cut the number of hook processes; this cut what each guard spends inside one, and the per-guard slice table states both. The method paragraph says why the two trees are measured paired and alternating rather than one after the other: the host is shared and its spawn floor moved from 42 ms to over 100 ms while the work was done, so a sequential comparison would have booked that drift as the change. It also records that the Write sample names a file inside the repository, because every Write and verifier guard early-exits on an outside path and an out-of-tree sample measures a no-op. The residual is stated with numbers rather than left implied. PreToolUse Bash is still 52.6 spawn-equivalents against the fleet target of 8, and nearly all of what remains is telemetry inside the synced hook library, one subshell fork per guard, and the classification each guard exists to do. Three further cuts were measured and deliberately not made; each is named with what it would have returned. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…ale back-reference Four corrections, all to prose that describes the 0.31.1 measurement rather than to any behaviour. The manifest count was 76 in three places. The glob the index actually walks is `plugins/*/.claude-plugin/plugin.json`, which returns 74 on this tree and on the base; the ~150 index spawns the same comment states are 2 times 74, so the count was the figure that drifted, not the spawn total. The CHANGELOG cited 339.8 falling to 21.6 spawn-equivalents for the PostToolUse Write path. 339.8 comes from the superseded standalone before run, whose per-trial cold plugin-data directory measured every cache cold, and 21.6 appears in no recorded run at all. The README already states that the paired columns are the comparison of record, so the CHANGELOG now quotes them: 368.4 to 79.3, and 12.8 to 3.0 for the convention gate in place of a before-only 10.1. The CHANGELOG said three guards dropped the `$(dirname ...)` subshell. Four did: cli-flag-verify, secret-pattern-detection, skill-reference-verify and stale-path-verify. One added comment in stale-path-verify cited an issue number a future reader of the file cannot see. The sentence above it already carries that reference from before this branch; the new sentence now names the fix instead of the ticket. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…olUse Write gain The 0.31.1 accounting table credited the PreToolUse Write path with 10.1 spawn-equivalents. A second paired pass, run on a much quieter host with the spawn floor back at 42 to 47 ms rather than 85.5 and 101, does not find it: 12 interleaved trials per tree put that delta at -0.9 with the telemetry sink unset, -3.2 with it set, and -1.9 with it set and CLAUDE_PROJECT_DIR unset. The row was drift rather than a result. Two of the three guards on that path are untouched by this phase and moved 1.1 and 0.9 in the first pass, the same size. And the single line the phase changed in secret-pattern-detection sits inside emit_tel, on the branch taken only when CLAUDE_PROJECT_DIR is empty, so a session that sets it never executes the change at all; the unset row was measured for exactly that reason and shows no gain either. The other two rows do reproduce, and the paragraph now states the reproduction figures beside the originals. The telemetry sentence said the sink-unset Bash path measured roughly half its cost. Measured, it is 45.7 spawn-equivalents against 79.5, so telemetry is about two fifths of the wall and 4.2 per guard across the eight. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
Claude finished @kyle-sexton's task in 7m 18s —— View job Claude code review
This is a well-scoped, well-tested perf change: four guards now skip expensive setup (plugin-manifest indexing, Two findings posted inline, both narrow:
Everything else — the |
Contributor
|
Claude finished @kyle-sexton's task in 3m 23s —— View job Security review complete
Scope: diff No blocking security findings. This is a spawn-reduction refactor of existing guard logic; none of the changes cross a new trust boundary, and the new caching layer is scoped to already-trusted local paths. What I checked
No secrets, credentials, or unsafe |
kyle-sexton
added a commit
that referenced
this pull request
Sep 2, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
Contributor
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baacee5d97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…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
Merge origin/perf/hook-fanout-consolidation (cd5aefb) into perf/guard-hot-path. The merge was clean and needed no resolution: the base landed the guardrails dispatcher as 0.31.0 and this branch's 0.31.1 entry already sits above it in plugins/guardrails/CHANGELOG.md, so the manifest version line and the changelog top region both merged without a conflict. Verified after the merge: scripts/check-changelog-parity.sh exits 0 for both --check-bump origin/main and --check, no conflict markers remain in the tree, and the two guardrails hook suites pass (run-guards.test.sh 54/54, skill-reference-verify.test.sh 120/120). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
Contributor
|
Claude finished @kyle-sexton's task in 1m 17s —— View job Claude code review
Skip gate hit: condition 4 (this head already has a successful review from this lane that still applies). The head moved from
This lane already completed a full review of this exact code at No new inline comments from this run. |
Contributor
|
Claude finished @kyle-sexton's task in 45s —— View job Claude security review
Skip gate hit: this head's security review already applies. The commits since the last security-reviewed head ( Per |
kyle-sexton
changed the base branch from
perf/hook-fanout-consolidation
to
main
September 2, 2026 22:29
…nce and anchor root-level paths as dirname did Three review findings on #3676. The convention-pattern cache tested freshness with `cache -nt dep` alone, which is true against a missing file, so an entry warmed while the team markdown, the well-known YAML, or an explicit `convention_source` target existed kept enforcing a policy the team had since deleted, where the resolver answers no enforcement. The entry now records each dependency with its existence at warm time, and a recorded-as-present file that is missing, or a recorded-as-absent file that now exists (even with an older mtime), reads as a miss. The warm path still forks the resolver zero times; both transitions are suite cases that fail against the previous hook. The parameter expansion that replaced the `dirname` fork in four guards produced an empty string for a root-level `/file.md`, which hook::repo_root read as `.` (the process CWD) rather than `/`. The empty case now yields `/`. secret-pattern-detection proves it end to end through a `git` shim recording the `-C` anchor; the three PostToolUse guards cannot reach that shape end to end (the file must exist and `/` is not writable), so their suites lift the seam from the hook source and hold it to dirname's answer for each path shape. cli-flag-verify's flag-shape pre-gate now has contract coverage: a `sed` shim proves no-dash content spawns no scan process on both the Write and the bare-bin Edit path, and dash-bearing content is held to the identical UNKNOWN_FLAG line and count the suite already pinned. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP
…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>
kyle-sexton
enabled auto-merge (squash)
September 2, 2026 23:23
Contributor
|
Claude finished @kyle-sexton's task in 2m 18s —— View job Claude code review
Skip gate: partially applies. Since the last review of this lane (head I traced all three fixes against the findings they claim to close:
All three fixes read as correct and directly test the failure mode each finding described; I didn't find a new gap they introduce. The CHANGELOG got three matching entries under the existing unreleased Not independently verified by me: I don't have Bash execution available in this run (test-script invocations were blocked pending approval), so I read the suite additions rather than running No new findings to report. |
Contributor
|
Claude finished @kyle-sexton's task in 2m 1s —— View job Claude security review
Skip gate not applicable — reviewed the delta since the last security-reviewed head. Last security-reviewed head was
No new security-relevant surface was introduced by this fix — it's a targeted correctness/enforcement-integrity patch to logic already in scope. I did not find any new issue in this delta. I wasn't able to execute the shell test suites in this sandbox (Bash tool required interactive approval that wasn't available), so this is a static trace of the diff, not a suite run — the diff-level reasoning holds regardless, and the added test cases described above are structured to fail against the pre-fix code if the logic were wrong. The other 10 files (README/CHANGELOG/version bump in the merge) carry only documentation and unrelated upstream content pulled in by |
kyle-sexton
added a commit
that referenced
this pull request
Sep 2, 2026
… main Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 3, 2026
kyle-sexton
added a commit
that referenced
this pull request
Sep 3, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
mainonce #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.shbuilt a plugin index with twojqper manifest (about 150jqspawns, 11.4 s) before knowing whether the written content cited any skill,block-convention-violation.shforked the convention resolver twice per Bash call, and several guards spentdirname,basename,git rev-parseandjqon 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 155jqexecs 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.dirname/basename, primed fields instead of re-runjq/git).Verification
.md18200 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.$(), 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.block-windows-drive-tmp.test.sh,hardcoded-path-check.test.sh) fail the identical case sets on the base.jqexecs, head 7).check-changelog-parity.sh --checkand--check-preservedexit 0; shellcheck and markdownlint clean; no em dashes in added lines (byte check).Related
hook::emit_telemetryinside the syncedhook-utils.shis about 1.4 s (33.8 spawn-equivalents) of the Bash wall when a telemetry sink is set; that is the library phase.docs/topics/hook-performance-levers/PLAN.mdon branchperf/hook-performance-program🤖 Generated with Claude Code
https://claude.ai/code/session_011boRpMmkKhritwmR4ZPMvP