perf(disk-hygiene): if-gate the PowerShell engine gate on the engine's file name by kyle-sexton · Pull Request #3880 · melodic-software/claude-code-plugins

kyle-sexton · 2026-09-06T21:34:06Z

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.

Umbrella disk-hygiene:clean — ceremony does not scale to stakes: audit of a real --execute run (13 live findings) #3347; siblings disk-hygiene: always-on engine gate and session belt emit identical denial text with opposite scopes #3348 and disk-hygiene: handoff-verify should accept --path inline, halving prompts on multi-path runs #3351 share this plugin's CHANGELOG and manifest (touched only this entry).
perf(hooks): dispatch the guardrails guards through one process and if-gate the path-specific hooks #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.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob

Generated by Claude Code

…s file name Closes #3349. Every PowerShell tool call in every session was still launching the destructive guard to be told it was irrelevant. On a warm interpreter cache that is four execve calls (bash -c, the launcher through its env shebang, bash, the interpreter), no fork, and a 106 KB module import, counted with strace -f; on this Linux host the hook process walls at p50 44 ms against a 2 ms bash -c floor (n = 20 per tool), about 22 spawn-equivalents, the cost class the issue measured as a 2.4 s median on Windows. The Bash entry has carried an if filter on the engine's file name since 0.21.4; the PowerShell entry now carries the same filter for its own tool, so the harness spawns nothing for a PowerShell call that does not name the engine. The 0.21.4 note that a PowerShell filter "must match every subcommand of a compound command" described allow rules, not if. The harness evaluates if through the tool's own permission matcher, and the PowerShell tool's parses the command AST and runs the hook when any statement, pipeline element or nested command matches (Claude Code 2.1.258, preparePermissionMatcher: some over every collected command, case-insensitive glob; an unparsable command runs the hook). A mixed line still reaches the guard and is still denied. No allow/deny decision changes for a call that reaches the guard, and no call the guard would have judged is skipped: the engine-gate mode defers every command that is not _engine_gate_relevant before any deletion spelling is consulted. A/B against a pristine origin/main export over 33 payloads (compound, CR LF, U+2028, BOM, zero-width and near-miss spellings) is identical on exit code, stdout and stderr, and every payload the filter skips is one the pristine gate deferred. The residual the filter cannot see, an engine reached by a link under another name, is the one the Bash lane has accepted since 0.21.4. Tests: the registration-shape test asserts both filters; two new tests assert that a skipped PowerShell call is one the gate defers and that every compound invocation shape is still relevant and still denied; the launcher suite gains a strace census (a warm launch creates no process and execs exactly bash and the interpreter). Each new test was shown to fail under a targeted mutation. README budget accounting and CHANGELOG updated; manifest 0.21.9 -> 0.21.10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob

…ual class The CHANGELOG and README described the filter's blind spot as "a symlink or hard link under another name." That undersells it: _engine_gate_relevant's marker-free branch identity-checks every word against the bundled engine via os.path.samefile, and its own docstring names Win32 8.3 short names (and trailing-dot/space and ADS-stream spellings) as the same alias class. A "python HYGIEN~1.PY apply" invocation reaches the engine with no literal "hygiene.py" text for the *hygiene.py* permission filter to match, so it is part of the same residual as a link. Reworded both surfaces to name the class (any spelling that reaches the engine without its own file name in the text) with links and 8.3 short names as examples, keeping both surfaces in agreement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob

…ners (#3878)  No linked issue ## Summary Drop leftover process creations on the hottest Bash paths in this marketplace: the shared hook library every always-on hook sources, the always-on formatter Write/Edit paths (typos, ruff, biome, bash, powershell, go, actionlint, eol, markdown), the always-on desktop-notification Notification path, always-on guardrails verifiers, and CI scanners that used to spawn once per file, per plugin, or per allowlist entry. ## Fix GNU Bash runs command substitution in a subshell even for builtins (Command Substitution, [Bash Reference Manual](https://www.gnu.org/software/bash/manual/html_node/Command-Execution-Environment.html); [Greg's Wiki](https://mywiki.wooledge.org/CommandSubstitution)). Cygwin's `fork` is a non-copy-on-write Win32 `CreateProcess` ([Cygwin User's Guide, Process Creation](https://ftp.cygwin.com/cygwin-ug-net/highlights.html)): "fork will almost certainly always be inefficient under Win32." ### Shared hook library (`lib/hook-utils.sh`, synced to 17 carriers, patch bump) Same `_to` / in-process pattern as #3838, #3732, and #3678: - `hook::json_escape_to` deletes residual C0 bytes with parameter expansion instead of `printf | tr -d` - `hook::emit_channels` writes through `_to` instead of `$(hook::json_escape …)` - Fractional `read -t` landed in bash-4.0-alpha (CHANGES). `hook::read_supports_fractional_timeout` is `BASH_VERSINFO`; no TMPDIR probe file - `hook::notice_once` reads the marker with `read`, creates the directory only when missing, and prunes stale markers once per process - `hook::bash_parse_segments` walks `${cmd:i:1}` instead of `read -N1` from a process substitution, and `$'…'` bodies decode through `ansi_c_decode_to` (`printf -v`) - `hook::repo_root_to` / `hook::repo_relative_path_to` write in this shell so callers skip a leftover capture around git or builtins-only work Isolation `$(source …)` forks are unchanged (#3685). ### typos-format (always-on Write|Edit|NotebookEdit) - Basename via `${FILE##*/}` (plus a backslash trim), not `basename(1)` - `repo_root_to` / `repo_relative_path_to` instead of capture subshells - Directory existence check instead of `$(cd && pwd)` - `command -v typos` is no longer captured; the later exec looks the name up on PATH ### Remaining always-on formatters (ruff, biome, bash, powershell, go, actionlint, eol, markdown) Same leftover class as typos-format, now applied to every always-on formatter that still captured `_to` helpers or spawned `basename` / leftover `cd && pwd`: - `FILE_BASE` is `${FILE##*/}` (and a backslash trim) - `repo_root_to` / `repo_relative_path_to` write in-process - `$(cd && pwd)` canonicalize is an existence check on the path git already answered (ruff, biome, bash-format EditorConfig walk) - Nested `$(normalize_path "$(physical_path …)")` in powershell-format uses the `_to` forms - `command -v ruff|biome|goimports` is no longer captured - markdown-format keeps physical `pwd -P` containment and config discovery; leftover helper-capture and membership dirname on the root-resolution path are gone ### desktop-notification (always-on Notification) - Field extract fuses into `hook::buffer_stdin_to` so completeness and `.notification_type` / `.message` share one jq process - C0 stripping is parameter expansion, not `printf | tr` - `repo_root_to` writes in-process; OSC 9 / BEL use `printf -v`; `terminalSequence` uses `json_escape_jq_to` - `uname` stays so tests can PATH-stub Darwin; git for `repo_root` stays ### guardrails verifiers (always-on PostToolUse / PreToolUse) - `skill-reference-verify`, `stale-path-verify`, and `cli-flag-verify` call `repo_root_to` / `repo_relative_path_to` in-process - `hardcoded-path-check` and `secret-pattern-detection` use `normalize_path_to` instead of leftover `$(hook::normalize_path)` captures - Isolation `$(source …)` forks are unchanged (#3685) ### CI scanners - Orphaned-fixture scan: one `*.test.*` index, cached `evals.json` `files[]`, in-shell ERE escape. Unquoted `\\` matches one backslash (a quoted `'\\'` arm is two chars and leaves `\b` as a word boundary) - Purged-em-dash scan: one `git ls-files -z` with every `:(glob)` pathspec; in-process component-wise attribution so `*` cannot cross `/`. `--list` stdout is byte-identical to origin/main - Cross-plugin source drift: one `find plugins` plus one `sha256sum` of 2+ cluster paths. Discover stdout is byte-identical to origin/main - Discriminating-test-skips / silent-skips: one awk per corpus (`FNR` + `FILENAME`; mawk has no `ENDFILE`) - Hook-exec-form: one jq over every `hooks.json` and one over every `plugin.json` (`input_filename` attributes rows). Unreadable `hooks.json` still fails closed via per-file fallback; unreadable manifests are still skipped Hook-specific leftover-fork work already in flight (#3873, #3872, #3871, #3870, #3869, #3851, #3849, #3779, #3880, #3886) is out of scope here. ## Verification Independent census re-derived spawn counts from `84adf87b` vs `cdb93f61` without inheriting implementer figures. Kernel census `strace -f -e trace=clone,clone3,fork,vfork,execve`; counter over duration; 3 identical trials. **always-on formatters** (this revision vs `84adf87b`): | Hook | clones before | clones after | execve before | execve after | |---|---|---|---|---| | ruff-format no-config skip | 14 | 10 | 4 | 4 | | powershell-format no-settings skip | 23 | 16 | 7 | 7 | | bash-format no-EditorConfig (ShellCheck finding) | 17 | 13 | 6 (1 `basename`) | 5 (0 `basename`) | **guardrails** (this revision): | Hook | clones before | clones after | execve | |---|---|---|---| | skill-reference-verify Write, no skill refs | 18 | 17 | 8 unchanged | | secret-pattern-detection clean Write | 10 | 8 | 4 unchanged | Secret-pattern absolute counts are with `CLAUDE_PLUGIN_ROOT` set (Claude Code always sets it). Without that env the leftover `PLUGIN_ROOT=$(cd … && pwd)` fallback adds one clone on both sides (11→9); the drop of 2 is the same. **CI scanners** (successful execve, exclude ENOENT; earlier commits on this PR): | Gate | origin/main or prior HEAD | HEAD | |---|---|---| | purged-em-dashes `--list` | 478 | 9 | | cross-plugin-source-drift `--check` | 181 | 4 | | discriminating-test-skips | 316 (awk 312) | 5 (awk 1) | | silent-skips | 120 (awk 118) | 4 (awk 2) | | hook-exec-form `--check` | 196 execve, jq 96, tr 96, clones 292 | 7 execve, jq 2, tr 0, clones 9 | `--list` / discover stdout for the two listing gates is byte-identical to origin/main. **Local `scripts/affected-tests.sh --run`:** 153 shell suites passed or were skipped; 14 NOT RUN python/mjs ecosystems (exit 3, expected on this runner). No `FAIL`. Including: `lib/hook-utils.test.sh` PASS=323; bash-format PASS=54; eol-normalizer PASS=54; markdown-format PASS=174; powershell-format PASS=17; cli-flag-verify PASS=92; hardcoded-path-check PASS=118; secret-pattern-detection PASS=86; skill-reference-verify PASS=140; stale-path-verify PASS=108. ruff/biome/go/actionlint behavioral cases skipped here (binaries absent); skip-path and source pins still ran. `session-event-log.test.sh` PASS=53 isolated under the fan-out. **CI on `cdb93f61`:** lint, hook-utils, test-linux (0–3), test-windows, changes, ci-status, and managed-files-guard all succeeded. https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676378 https://github.com/melodic-software/claude-code-plugins/actions/runs/34066676488 ## Related Refs #3838, #3732, #3678, #1979, #3488, #2891. Same leftover-fork class as open PRs #3849 / #3851 / #3869 / #3873 / #3872 / #3871 / #3870 / #3779 / #3880 / #3886 (those stay hook-specific). N/A for a dedicated issue.  <div><a href="https://cursor.com/agents/bc-fdfdc962-be1b-4c9c-9833-3aec57852330?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-fdfdc962-be1b-4c9c-9833-3aec57852330&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

kyle-sexton · 2026-09-07T00:20:24Z

Merge-ordering note for the merge lane, from the review of this PR.

Open PR #3783 (branch claude/3352-scan-verbosity) carries 0.22.0 on this same plugin, while this PR takes 0.21.10 against main's 0.21.9. There is no version collision, but whichever of the two merges second will need a rebase, or scripts/check-changelog-parity.sh --check-order will fail at that point.

Recording it here rather than in the body because the body edit was interrupted; the substance is unchanged.

Generated by Claude Code

chatgpt-codex-connector · 2026-09-07T00:20:39Z

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-07T00:24:41.539772Z	`5eaba36`	Draft marked ready

ℹ️ 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-07T00:20:47Z

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

Code review — `/review:code-review`

Gather context (PR body, diff via gh pr diff 3880, changed files)
Read hooks/hooks.json, destructive_guard.py (_engine_gate_relevant, _carries_marker, powershell_decision), and the new tests in test_hygiene.py / run-python-hook.test.sh
Check README/CHANGELOG for stale references to the old (unfiltered) PowerShell registration
Static verification (JSON structure, Python logic trace, regex trace against every listed test case)

Findings

No high-signal correctness or maintainability issues found. This is a narrowly-scoped, mechanical change (one "if": "PowerShell(*hygiene.py*)" line in hooks/hooks.json) with unusually thorough accompanying test coverage, and the logic traces out correctly:

Every "skipped" example in test_powershell_if_filter_skips_only_calls_the_gate_would_defer (test_hygiene.py:6461-6476) correctly evaluates to False under _carries_marker's tokenization rules (zero-width space and space-split spellings both break the hygiene.py token, so they're already outside _engine_gate_relevant, not newly excluded by the filter).
Every "admitted" example in test_powershell_if_filter_admits_every_engine_invocation_shape (test_hygiene.py:6478-6506) — ;, |, &&, newline, CRLF, U+2028, the call operator, nested pwsh -Command, tab/upper-case spellings — retains the hygiene.py token as a standalone word after _MARKER_TOKEN_SPLIT's tokenization, so _engine_gate_relevant and the new _powershell_if_admits reference both agree, and powershell_decision denies unconditionally for all of them (it checks _engine_gate_relevant before consulting the enabled flag).
No stale prose survives the swap: I grepped README.md and CHANGELOG.md's current-version text for the old "PowerShell carries no filter / must match every subcommand" rationale and found it only in the historical (correctly-immutable) [0.21.4] changelog entry, not restated as current guidance.
hooks/hooks.json stays valid JSON with the added key; the two engine-gate entries are now symmetric per tool.

One thing I could not independently verify in this sandbox: the new kernel-level spawn census in run-python-hook.test.sh (strace -f -e trace=clone,clone3,fork,vfork,execve, asserting 0 creations / 2 execs on a warm launch) — this review environment's sandbox categorically refuses to invoke strace ("runs its argument as a command — cannot be statically analyzed"), and also blocks running the test suite itself or py_compile on the modified test file. I traced the shell logic by hand (the grep -v unfinished / = [1-9][0-9]*$ split-syscall accounting looks correct, and strace -f is documented to prefix every line with the PID from the start of tracing, not only after an actual fork occurs, so the ^[0-9]+ +execve\( anchor should hold even in the zero-creations case) but did not execute it. The PR body's own "Acceptance criteria ... not met" section already discloses that the harness-side skip isn't verified end-to-end under a real PowerShell tool call, which is consistent with what I can confirm from the diff alone — flagging this as author-claimed/unverified from my side rather than independently confirmed.

No blocking issues. The fix (plugins/disk-hygiene/hooks/hooks.json line 24) is behavior-preserving for every call that reaches the guard and correctly scoped to the class of calls the PR claims it skips.

claude · 2026-09-07T00:21:09Z

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

`/review:security-review` — PR #3880

Gather context (diff, changed files)
Run security review per plugins/review/skills/security-review/SKILL.md
Post findings

Scope: hooks/hooks.json, hooks/run-python-hook.test.sh, skills/clean/scripts/test_hygiene.py, README.md, CHANGELOG.md, plugin.json — all in plugins/disk-hygiene. No .github/workflows changes, so no Actions-hardening surface (zizmor's lane).

This diff touches a security-relevant control (the destructive-guard's PowerShell activation condition), so it clears the skip gate. I read destructive_guard.py (unchanged by this PR) to check whether the new if filter can skip the hook for a command _engine_gate_relevant would still have gated.

The one real question here: _engine_gate_relevant can return True via filesystem identity alone (_same_file_as_bundled/samefile) for a command that never contains the literal substring hygiene.py at all — e.g. a bare PATH alias or symlink to the bundled engine invoked under a different name (destructive_guard.py#L509-L530). The new "if": "PowerShell(*hygiene.py*)" filter (hooks.json#L24) is a text glob, so it cannot see that identity match and will now skip launching the guard entirely for that case, whereas before (no PowerShell filter) the guard always launched and would have caught it.

That is a real narrowing of coverage, but it is not new or unique to this PR: it's the exact residual the Bash(*hygiene.py*) filter has carried since 0.21.4 (_within_plugin_cache_family/samefile branch applies identically to both tools), and this PR explicitly extends the same accepted trade-off to PowerShell rather than introducing it. It's disclosed in both the CHANGELOG entry and the README hook-budget section, and the guard's own docstring notes the belt/skill-scoped guard and the engine's own preview/approval-token containment still answer for a command smuggled past this plugin-level gate. Given it's a known, documented, symmetric-with-Bash trade-off rather than a silent new gap, I'm not flagging it as a finding — it's the intended shape of the change, not a defect in it.

One thing worth naming plainly rather than as a finding: the correctness of when the new filter fires rests on a reverse-engineered read of the closed-source PowerShell if matcher (Claude Code 2.1.258's preparePermissionMatcher), not an end-to-end test — the PR body says as much under "not met." If that harness behavior differs from what's assumed (e.g., an unparsable/obfuscated compound statement resolving to "no match" instead of "run the hook"), the gate would silently narrow further than intended. This isn't something I can verify statically either, and the PR is already transparent about it, so I'm surfacing it as context for the merge decision rather than as a blocking issue.

No injection, secrets-handling, or authorization findings. The if filter is a static JSON string (no interpolation), the new shell test (run-python-hook.test.sh) only touches mktemp-scoped fixture paths under trusted control, and the new Python tests are test-only additions with no runtime guard logic changes (destructive_guard.py is untouched, consistent with the PR's "byte-identical" claim).

Verdict: no security issues to flag. The change is a disclosed extension of an existing accepted risk trade-off (Bash → PowerShell parity), not a new vulnerability.
· branch claude/3349-hygiene-gate-perf

github-actions · 2026-09-07T00:24:19Z

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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5eaba367b6

ℹ️ 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-07T00:26:25Z

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

…ing them when no telemetry sink is set (#3913) Closes #3862 ## Summary Guard decisions left the process only through `HOOK_TELEMETRY_SINK`, which is inert unless an environment variable names an executable. On an ordinary install every decision was discarded as it was made, so "why was this denied", "has it denied this all along", and "did the guard run at all" had no evidence to answer from. New `plugins/disk-hygiene/lib/guard_decision_log.py` appends one JSON object per line to `<CLAUDE_PLUGIN_DATA>/guard-decisions/decisions.jsonl`, default on, no configuration. `destructive_guard.py` records every branch that reaches a verdict; `guard_launch_monitor.py` records the did-not-run state the guard structurally cannot write about itself. Version taken: **0.23.0**. Verified free against current `main` (`6db96637d`, 0.21.9) and against the head of every open PR at the moment of opening: #3880 claims 0.21.10, #3783 claims 0.22.0, and #3851/#3887/#3900/#3779 leave the manifest at 0.21.9. 0.23.0 is strictly greater than all of them, so it cannot collide under any merge order; the 0.22.x gap is what `--check-order` explicitly reads as correctly ordered. ## Fix **Where the record lives, and why it survives.** Under the plugin's own persistent data root, resolved by the guard's existing `resolve_authorized_data_root()` (the `--authorized-data-root` / `--plugin-root` / `CLAUDE_PLUGIN_DATA` ladder) beside the run records already kept there. Because this plugin is the one that deletes things, the filename was checked against the engine's own discovery hints in `reference/baseline-policy.json`: `decisions.jsonl` and `decisions.previous.jsonl` match none of the 14 bundled name globs (`*.tmp`, `tmp-*`, `tmp_*`, `scratch*`, `*.lock`, `__pycache__`, `*.partial`, `*.crdownload`, `*.tmp.*`, `.claude.json.tmp.*`, `temp_git_*`, `.pulumi-write-test-*`, `.DS_Store`, `Thumbs.db`), so the plugin's own hints never nominate its audit trail. Appending directly rather than writing a temp file and renaming is deliberate for the same reason: an atomic-write staging name would land on `*.tmp.*`, which is a bundled hint. **What is recorded.** `schema_version`, `timestamp` (UTC, milliseconds), `hook`, `decision`, `rule`, `tool`, `mode`, `command`, `reason`. `decision` is `allow` / `ask` / `deny` / `none` (ran, issued no `permissionDecision`) / `not-run`. `rule` names the branch that fired, so `kill-switch-disabled-apply` is distinguishable from `not-exact-engine-command`: two different answers to "why". `command` is the input that drove it and `reason` is the exact text the host was given, so the record and the host cannot disagree. Both are clipped to 400 characters, which keeps it a record of the decision rather than a copy of the payload and keeps every line short enough that concurrent hook processes appending to the same file do not interleave. **Bounded, enforced.** The live file rotates to `decisions.previous.jsonl` at 1 MiB via `os.replace`, so the record occupies at most about 2 MiB forever with no operator pruning. The bound is checked from the offset the append already returns (`handle.tell()` in append mode), so enforcing it costs no extra syscall. **Cost.** Measured with `strace -f -e trace=clone,clone3,fork,vfork,execve,openat,write` against a detached worktree of `origin/main` at `6db96637d`, five invocations per arm plus a warm-path detail run: | Path | Before | After | | --- | --- | --- | | defer (a Bash command not naming the engine, the always-on branch) | 1 `execve`, 1 `clone3` | 1 `execve`, 1 `clone3`, 0 record syscalls | | decision (deny), warm data root | 1 `execve`, 1 `clone3` | 1 `execve`, 1 `clone3`, 1 `openat` + 1 `write` | | decision (deny), first write of an install | 1 `execve`, 1 `clone3` | plus 1 failed `openat` and 1 `mkdir` | The single `clone3` is `CLONE_THREAD`, the existing watchdog thread, not a process. **The process and exec census is unchanged on every path.** The plugin-level defer branch, which is what this always-on hook takes for work unrelated to disk-hygiene, writes nothing at all and is byte-for-byte the path it was. Wall clock over 40 invocations per arm, alternated twice, moved inside run-to-run noise on this host (52 to 58 ms both before and after, the sign of the difference changing between repetitions), which is why the syscall census rather than a duration is the figure cited. **Failure behavior: the verdict never changes.** Two boundaries, both load-bearing. `guard_decision_log.record` returns a bool and catches `BaseException` around the whole write. `_record_decision` in the guard wraps its own call, because the data root and mode are resolved in the argument list, outside `record`'s protection, and one of its call sites is `main`'s own `except BaseException` handler, where a raise would reach the interpreter's default handler: exit 1, which PreToolUse treats as non-blocking, so the command the guard just denied would run. Every record call is made after the verdict has been emitted, and its result is discarded. **What is deliberately not recorded.** The plugin-level defer (hot path, and not a decision anyone reconstructs later). The watchdog expiry path: that callback runs while the main thread is presumed wedged inside a filesystem call and stays syscall-free for exactly that reason, so a write there could hang on the same filesystem. Both are stated in the README rather than left implicit. **Adjacency, stayed out of.** #3861 (the guard's fail-open when no interpreter resolves) is `needs-human`. This change touches the guard's decision branches but not interpreter resolution, and adds no new fail-open path; the `not-run` record makes the fail-open class more visible after the fact without adjudicating it. **Escape hatch.** `DISK_HYGIENE_GUARD_DECISION_LOG` set to `0` / `off` / `false` / `no` turns the record off. Opt-out, not opt-in: any other value, including an absent one, records. ## Verification All runs local and in the foreground; draft CI is not cited as test evidence. - `bash scripts/affected-tests.sh --run --shard N/4`: shards 0, 1, 2 exit **3** (success, with `NOT RUN` non-shell ecosystems), 0 `FAIL` lines each. Shard 3 exits 1 with exactly three `FAIL` lines, all from `plugins/claude-ops/skills/plugins/scripts/cache-content-check.test.sh` (`process budget: the trace probe actually counted something`, `process budget: a one-install report costs at most 26 process creations`). **Reproduced unchanged on a clean detached worktree of `origin/main` at `6db96637d`**: same suite, same 2 cases, exit 1. Pre-existing, not this change. Every changed file maps to at least one suite; the three docs/manifest files resolve through the recorded no-suite allowlist. - Direct suites: `hygiene.test.sh` RC=0 (350 cases), `guard_launch_monitor.test.sh` RC=0 (28 cases), `run-python-hook.test.sh` RC=0, `test_guard_decision_log.py` + `test_hook_telemetry.py` RC=0 (15 new cases). - All four parity modes green: `--check`, `--check-order`, `--check-bump origin/main`, `--check-preserved origin/main`. - `scripts/run-ruff.sh check plugins/disk-hygiene`: all checks passed. `format --check`: my four touched/new Python files are clean. Five files remain unformatted in this plugin (`killswitch_config.py`, `hygiene.py`, `guard_launch_monitor.py:137`, `test_hygiene.py:2373`, `test_kill_switch_probe.py:96`); all five are identically unformatted on `origin/main`, so none is introduced here. - Gates run clean: `check-purged-em-dashes.sh`, `check-drive-root-litter.sh`, `check-silent-skips.sh`, `check-discriminating-test-skips.sh`, `check-fixture-git-isolation.sh`, `check-hook-exec-form.sh`, `check-killswitch-hoist.sh`, all RC=0. New test file committed `100755` (verified with `git ls-tree`), the new library `100644` matching its sibling `hook_telemetry.py`. - No shell files changed, so shellcheck and shfmt have nothing to say about this diff. `lib/hook-utils.sh` untouched. **Mutation proof (the new assertions discriminate).** Nine mutations applied one group at a time, each reverted: | Mutation | Caught by | | --- | --- | | rotation call disabled | 3 lib cases (`rotates_at_the_bound`, `discards_only_the_generation_before_last`, `a_failing_rotation...`) | | `_clip` returns text unchanged | `long_command_and_reason_are_truncated` | | `enabled()` hardcoded True | 5 lib subtests + `the_record_can_be_turned_off_without_changing_a_verdict` | | deny rule string collapsed onto the kill-switch rule | `denied_engine_command_is_recorded_with_its_rule_and_input` | | a record added on the defer path | `engine_gate_defer_records_nothing` | | `_record_decision`'s try/except removed | `a_broken_decision_record_never_changes_a_verdict` (record raises), `record_decision_swallows_a_failure_in_data_root_resolution`, and the **pre-existing** `every_call_graph_function_failure_denies_at_exit_2_never_1` for both `resolve_mode` and `resolve_authorized_data_root` | | `_record_not_run` call removed from the monitor | 3 monitor cases | The write-failure proof is `test_a_broken_decision_record_never_changes_a_verdict`, which drives all five verdict shapes (`allow`, `ask`, `deny`-by-authority, `deny`-by-kill-switch, and the no-output defer) three times: unsabotaged, against a data root whose parent is a regular file (a real filesystem `OSError` on both the append and the `mkdir` behind it), and with `record` raising `RuntimeError`. All three runs produce the identical verdict list, and the unwritable root is asserted to still not exist afterwards. `test_a_write_failure_leaves_the_deny_exit_status_untouched` pins the same thing end to end through `main`. **Hermeticity fix included.** `run_guard_engine_gate` previously passed `SCRIPT_DIR / "data-root"` as the authorized data root. With records being written, that would have littered the checkout on every test run, so it now takes a per-test temp path, and `GuardTests.setUp` pops an inherited `CLAUDE_PLUGIN_DATA` so a developer's real plugin data directory is never written to by the suite. ## Related - Closes #3862; parent #3347 finding F12. - Sibling #3861 (guard fail-open when no interpreter resolves) is `needs-human` and deliberately untouched; the `not-run` record is what makes that class visible after the fact. - Sibling finding on a false denial: `rule` plus `command` plus `reason` is what answers it from the record instead of by reproduction. - Hook budget convention: `docs/conventions/hook-budget/README.md`, `.claude/rules/hook-budget.md`. Measured share stated in the plugin README's trust-surface record. - Version contention checked against open PRs #3783 (0.22.0) and #3880 (0.21.10). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob --- _Generated by [Claude Code](https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

Keep the PowerShell engine-gate if-filters and preserve guarding when the script path is in a variable. The matcher evaluates collected command nodes, so `$script = '.../hygiene.py'; python $script` missed PowerShell(*hygiene.py*). Sibling filters PowerShell(*python*$*) and PowerShell(*& $*) keep those invocations on the guard. Bump disk-hygiene to 0.23.1 above main's 0.23.0. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

… test PowerShell now has several matchers (literal path plus variable-based invocations), so the watchdog ceiling test cannot assume exactly two PreToolUse registrations. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

GNU grep '\\b' word boundaries fail the shell-portability gate. strace syscall lines already carry a literal '(' after the name. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

…ply path already has (#3919) Closes #3859 ## Summary The apply path already processes nested entries bottom-up, so removing a directory and the now-empty directory that contained it happens in one pass. `handoff-verify` lacked those semantics, so a container emptied by removing its last child was not recognized as newly removable in the same round. ## Fix - Share the apply lane's `removal_sort_key` via `emptied_container_order`. One decreasing-depth pass names the inventoried directories the settled (`clear`/`gone`) removals empty. Each is then run through the same categorical checks and reported under `emptied_containers`. Verification still mutates nothing. The approved paths' `clear`/`not_clear` counters (and the CLI exit code) are unchanged. - Compare only the surplus (live children the snapshot did not record) when checking a container. Apply refuses to `rmdir` only when `scandir` still finds an occupant; missing inventoried children are the verify-one-delete-one sequence progressing, not drift. A replaced inventoried child fails its own approved-path verdict and stays out of the settled set. Rebased onto origin/main (not onto #3880). Version is `0.23.2`, stacked above main `0.23.0` so #3880 can keep `0.23.1`. `hooks.json` is untouched; `test_hygiene.py` edits stay in `HandoffVerifyTests` (plus a ruff wrap in `HygieneTests`) so #3880's PowerShell matcher additions in `GuardTests` can still merge. ## Verification - `python3 -m unittest` `HandoffVerifyTests`: 37 tests, including the one-round cascade, verify-one-delete-one, apply/verify agreement, and 60-level termination - `plugins/disk-hygiene/skills/clean/scripts/hygiene.test.sh`: 368 tests OK - `scripts/run-ruff.sh check` and `format --check` on the changed Python - all four `check-changelog-parity.sh` modes vs origin/main - `scripts/affected-tests.sh --run` (the one `block-hook-bypass` FAIL is the suite's symlink case when the worktree itself lives under `/tmp`, reproduced on origin/main, not this change) - mutation: restoring exact child-set equality kills `test_container_survives_the_verify_one_delete_one_sequence`; making `emptied_container_order` never qualify kills the cascade and apply agreement tests ## Remaining work None. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

claude added 2 commits September 6, 2026 21:33

cursor Bot mentioned this pull request Sep 6, 2026

perf: drop leftover forks in hook-utils, always-on hooks, and CI scanners #3878

Merged

kyle-sexton marked this pull request as ready for review September 7, 2026 00:20

chatgpt-codex-connector Bot reviewed Sep 7, 2026

View reviewed changes

Comment thread plugins/disk-hygiene/hooks/hooks.json

This was referenced Sep 7, 2026

feat(disk-hygiene): record guard decisions locally instead of discarding them when no telemetry sink is set #3913

Merged

fix(disk-hygiene): give handoff-verify the container semantics the apply path already has #3919

Merged

cursoragent and others added 2 commits September 7, 2026 14:36

kyle-sexton enabled auto-merge (squash) September 7, 2026 15:25

fix(disk-hygiene): use POSIX ERE for the strace spawn census

973da03

GNU grep '\\b' word boundaries fail the shell-portability gate. strace syscall lines already carry a literal '(' after the name. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>

kyle-sexton merged commit 84a653e into main Sep 7, 2026
12 checks passed

kyle-sexton deleted the claude/3349-hygiene-gate-perf branch September 7, 2026 16:05

Navigation Menu

Uh oh!

perf(disk-hygiene): if-gate the PowerShell engine gate on the engine's file name - #3880

perf(disk-hygiene): if-gate the PowerShell engine gate on the engine's file name#3880
kyle-sexton merged 5 commits into
mainfrom
claude/3349-hygiene-gate-perf

kyle-sexton commented Sep 6, 2026

Uh oh!

kyle-sexton commented Sep 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kyle-sexton commented Sep 6, 2026

Summary

Fix

Verification

Related

Uh oh!

kyle-sexton commented Sep 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

claude Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review — /review:code-review

Findings

Uh oh!

claude Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

/review:security-review — PR #3880

Uh oh!

github-actions Bot commented Sep 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Sep 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chatgpt-codex-connector Bot commented Sep 7, 2026 •

edited

Loading

claude Bot commented Sep 7, 2026 •

edited

Loading

Code review — `/review:code-review`

claude Bot commented Sep 7, 2026 •

edited

Loading

`/review:security-review` — PR #3880