Skip to content

perf(context-guard): skip the zone resolver when the snapshot is unchanged - #4193

Merged
kyle-sexton merged 13 commits into
mainfrom
context-guard-zone-crossing-fast-path-on
Sep 16, 2026
Merged

kyle-sexton merged 13 commits into
mainfrom
context-guard-zone-crossing-fast-path-on

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

No related issue: handoff-inbox item 20260913-034032 under program item 20260915-153000 (spawn budget per tool call); no GitHub issue was filed.

Stacked on #4185 (base branch guardrails-run-guards-in-process-no-subs): the envelope parse rides on that PR's hook::jq_fields builtin parser in the synced lib. GitHub retargets it to main when #4185 merges.

Summary

context-guard's zone-crossing-inject.sh fires on every PostToolBatch and UserPromptSubmit. Each fire spawned a jq for the envelope and then the zone resolver (its own bash plus a jq) even when nothing the resolver reads had changed since the last fire, which is the common case: the statusline snapshot is rewritten only when the statusline renders.

Fix

  • An unchanged-input skip. A $STATE_DIR/$SESSION.seen mark records the inputs behind the last completed resolve in two ways: its mtime, stamped with a redirection and compared with -nt, and one flags line (z=<0|1> c=<0|1>) recording whether zones.json and the compaction marker existed when that resolve ran, read back with builtin read. The fire exits before starting a process only when the per-session snapshot, zones.json and the compaction marker are all no newer than the mark AND both existence flags still match the current -e results; a mark with no readable flags line never takes the skip. The mark moves only after the resolve persisted its markers, so a resolver failure, an unknown reading and a failed marker write are each retried next fire. A missing snapshot is never skippable. Skipping can only choose silence: no arrangement of timestamps or existence changes can manufacture an injection the full path would not have made.
  • The envelope parse is size-branched. Under 64 KiB it goes through hook::jq_fields' builtin parser (zero spawns); above it keeps the single here-string jq (2 creations), because the helper's oversize fallback reads through a process substitution and measured 4. Plain routing through the helper would have made the large-payload path 9 to 11 creations; the branch is what keeps every cell at or below before. The 65536 literal mirrors the helper's private proof ceiling and is documented at the site.
  • STATE_DIR resolution moves ahead of the resolver, so a session with no state root exits one process earlier.

context-guard is bumped 0.7.64 to 0.7.65 with the numbers in the CHANGELOG, and the README gains a "Skipping the resolve when nothing moved" subsection carrying the table.

Verification

Job-object census (n=5, identical across reps; subject floor 3 = bash -c, env, bash):

Fire Payload Creations before Creations after
first (resolves) small 11 9
repeat, nothing moved small 9 3
snapshot rewritten small 9 7
first 150 KB 11 11
repeat, nothing moved 150 KB 9 5
snapshot rewritten 150 KB 9 9

No cell is worse than before. Small repeat fire wall p50 1448 ms to 237 ms (re-run 365 ms; the host is bimodal). On Windows a resolve costs 4 creations rather than 2 because bash "$RESOLVER" hits the bin\bash.exe wrapper, which re-spawns usr\bin\bash.

Crossing messages are byte-identical, asserted in the suite against a control session driven through the same zone sequence with no skipped fire. Suite: zone-crossing-inject.test.sh 78/1 to 97/2, where both failures are strace: no usable trace on this host (Git Bash's cygwin strace rejects -e trace=; the second is the new strace block, not a regression); zone-gate 26/0, post-compact-mark 18/0. shellcheck clean; check-changelog-parity.sh --check-bump origin/main and --check green.

Residuals: the strace pins (steady fire 0 creations / 1 execve; resolving fire 2 / 3) are set by reasoning and verified only on CI's Linux lane. The one miss window is the resolve itself: a snapshot written between the resolver's read and the stamp is marked seen and its crossing is reported one fire late, never lost, since the statusline rewrites the snapshot on its next render; on a filesystem or bash build that compares mtimes at whole-second granularity the window is up to one second. The README still carries the older "0.7.49 brought it to 3" paragraph; the new subsection supersedes it.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 5 commits September 15, 2026 13:51
…ields without jq

run-guards.sh sources its guards into its own shell instead of one
command-substitution subshell per guard: `exit` is a dispatcher function
that records the guard's status and runs the next guard from inside the
call, stdout documents are collected through hook::emit_document, and a
guard that dies of a hard error hands its status to the abort boundary's
new chain slot (_GAB_CONTINUE), which runs the guards still owed in one
subshell. Per-invocation analysis state (the alias memo) is reset before
each guard, so a later guard's alias walk is not answered by an earlier
guard's memo.

lib/hook-utils.sh answers a well-formed payload's plain-string fields
with its builtin JSON parser (hook::_fast_fields) and runs jq only for a
shape it cannot prove; hook::jq_fields_uncached names the same body for
the dispatcher's cache, hook::emit_document is the one stdout path, and
hook::extract_bash_subject_to is the in-shell telemetry subject. The 17
carrying plugins take the synced copy with a patch bump.

Measured on Windows 11 + Git Bash with an exact job-object census of the
harness's own invocation: the Bash-lane chain went from 23 process
creations to 3 and 880 ms to 285 ms isolated p50; the PowerShell lane
from 100 to 80 creations and 3.3 s to 2.7 s. Decisions are byte-identical
(rc, stdout, stderr) against 0.33.11 over the perf baseline's 17-command
corpus in both tool modes, over 653 commands harvested from the guard
suites on the Bash lane and 200 on the PowerShell lane, and over the
Write, Edit and drive-root-tmp lanes.

Handoff item: 20260913-034031 (program 20260915-153000).

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
hook::extract_bash_subject_to assigns SUBJECT through a nameref, which
shellcheck 0.11 cannot follow; CI's lint lane failed SC2154 on the two
guards that read it in emit_tel. Declaring the variable empty first is
the idiom block-hook-bypass already uses. block-no-verify 256/0 and
flag-commit-pr-skill-bypass 35/0 unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CI's machine-specific-paths hygiene check refuses a Windows user path
in a fixture. The value is opaque to the parser under test; hook-utils
504/0 unchanged.

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

zone-crossing-inject.sh reads three files the world outside it can move: the
per-session snapshot, the optional zones.json, and the compaction marker. When
none is newer than the `.seen` mark left by the last COMPLETED resolve, the fire
would repeat that resolve's decision, which is already persisted, so it exits
before starting a process. The mark is stamped with a redirection and compared
with `-nt`, both builtins, and it moves only after both markers persist, so a
resolver failure, an `unknown` reading and a failed marker write are each
retried rather than skipped.

The envelope parse had to become free for the skip to mean anything, and it is
free only on the payloads hook::jq_fields can prove: within its ceiling the
builtin parser answers both fields with no process, above it the single
here-string jq stays, because the helper's oversize fallback reads through a
process substitution and costs four process creations against that jq's two.
That builtin parser arrives with the guardrails-run-guards-in-process-no-subs
merge (PR #4185's lib/hook-utils.sh sync), which this fast path depends on.

Process creations under a Windows job object, 5 reps, identical across reps;
the subject's own floor is 3 (the `-c` shell, env, the shebang's shell):

  small envelope   first 11 -> 9   repeat 9 -> 3   rewritten 9 -> 7
  150 KB batch     first 11 -> 11  repeat 9 -> 5   rewritten 9 -> 9

No cell is worse than before. Median wall for the small repeat fire, on a host
whose timings are bimodal, 1,448 ms -> 237 ms.

The one failure mode: a snapshot written DURING a resolve is marked as seen and
its crossing waits for the next statusline render, so the window is the resolve
rather than an mtime tick. A missed crossing is late, never lost, and a spurious
one is impossible, because skipping only ever chooses silence.

The suite gains the skip cases (silent repeat, byte-identical crossing after a
skipped fire against a control session, a newer zones.json, a resolver failure
leaving the mark untouched) and splits the per-batch budget in two: the steady
fire now spawns nothing, and a resolving fire spawns the resolver alone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 16, 2026 00:08
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 16, 2026

Copy link
Copy Markdown

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-16T00:11:07.480047Z a6fb54e 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

claude Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, changed files)
  • Review changed files for security-relevant issues
  • Report findings or confirm none found

No security issues found.

This PR is a performance-only refactor of plugins/context-guard/hooks/zone-crossing-inject.sh (plus tests/docs/version bump). Reviewed for logic flaws, trust-boundary issues, injection surfaces, and unsafe credential/token handling:

  • Path construction stays behind the existing validation gate. SESSION is validated against ^[A-Za-z0-9_-]+$ (zone-crossing-inject.sh#L246) before it's used to build STATE_FILE, ARMED_FILE, and the new SEEN_FILE (#L262-L263), so the new .seen marker can't be used for path traversal via a crafted session_id.
  • New envelope-parse branch (hook::jq_fields vs. here-string jq) doesn't change the trust boundary. Both arms parse the same untrusted JSON payload the same way as before; the size branch on ${#INPUT} is a plain arithmetic comparison, not something an attacker can leverage.
  • The unchanged-input skip (.seen mark compared via -nt) is a pure availability/timing optimization, not an access-control check — it can only suppress an injection the full path would also have suppressed (per the PR's own stated invariant, "skipping can only choose silence"), never fabricate or leak one. The documented TOCTOU residual (a snapshot write landing mid-resolve delays a crossing report by one fire) is a functional edge case, not a security gap.
  • No new handling of secrets/tokens, no new external command construction from untrusted input, and no changes to the GitHub Actions workflow files in this PR.

@github-actions

Copy link
Copy Markdown
Contributor

Last security-reviewed head: a6fb54ea388f676ecbbf223983baa2a15cd7cfe2. 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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6fb54ea38

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

Comment thread plugins/context-guard/hooks/zone-crossing-inject.sh
@claude

claude Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

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


Code review — /review:code-review (CI lane)

  • Gather context (PR body, changed files, custom instructions)
  • Read plugins/context-guard/hooks/zone-crossing-inject.sh diff and full file
  • Cross-check hook::jq_fields / hook::_json_split (hook-utils.sh) for the size-ceiling and fallback semantics the diff relies on
  • Trace the unchanged-input skip (.seen mark, -nt comparisons, ordering vs. resolver/persist) for correctness
  • Read the test diff (zone-crossing-inject.test.sh) and CHANGELOG/README/plugin.json changes
  • Attempt to run shellcheck / the test suite locally
  • Post findings

Note on verification: this lane's Bash tool required interactive approval for every invocation (including shellcheck and running zone-crossing-inject.test.sh), which wasn't available in this run, so I could not independently execute either. Everything below is a static read of the diff plus hook-utils.sh's existing hook::jq_fields/hook::_fast_fields/hook::_json_split implementation (unchanged by this PR) that the new code depends on. The PR body's shellcheck/suite-pass/perf-table claims are author-reported and not verified by me.

Findings

1. Doc drift (pre-existing, not touched by this PR, but now directly contradicted): README still states the steady-path budget is 3 process creations.

plugins/context-guard/README.md lines 319–323 (unchanged by this diff):

"the steady PostToolBatch and UserPromptSubmit fire was creating 8 processes where those rows report 3; moving every redirection off the inside of a command substitution brings it to 3…"

The spawn-equivalents above are command-position counts. Re-measured as process creations under
`strace -f` (0.7.49), the steady PostToolBatch and UserPromptSubmit fire was creating 8 processes
where those rows report 3; moving every redirection off the inside of a command substitution
brings it to 3 with the program launches unchanged. See "Counting invocations is not counting
processes" above for why the two counts differ and what it costs on a slow-spawn host.

This PR adds a new "Skipping the resolve when nothing moved" subsection right above it (README.md lines 163–197) that correctly states the steady fire is now 0 process creations in the common case — but the older paragraph is left asserting 3 with no cross-reference or "superseded by" note. The PR description acknowledges this exact staleness under "Residuals" ("The README still carries the older '0.7.49 brought it to 3' paragraph; the new subsection supersedes it") but the fix wasn't applied to the file. A reader landing on that paragraph via search or a permalink (not the new subsection) will get a number the code no longer produces for the common case. Since the PR is already touching this README and already knows about the conflict, it's low-cost to close it here — e.g. append "superseded for the steady case by the 0.7.65 skip above" to the 0.7.49 paragraph, or fold the two together.

Fix this →

What I checked and did not find issues in

  • Skip-gate correctness (zone-crossing-inject.sh lines 305–310): the -nt/-e combination correctly forces a full resolve on the first-ever fire (no .seen file yet → -nt treats the snapshot as newer), never skips when the snapshot is missing, and the .seen mark is only stamped (line 479) after .zone/.armed persist successfully — so a resolver failure, an unknown reading, or a failed marker write all correctly fall through to being retried next fire, matching the stated contract and the new tests (13, 13a, 13b) that exercise exactly these paths.
  • Size-branch threshold (> 65536 vs. hook::_json_split's own <= 65536 proof ceiling in hook-utils.sh:796): the boundary is consistent — a payload of exactly 65536 bytes takes the hook::jq_fields builtin-parser arm, and the helper's own ceiling check independently agrees it's eligible, so there's no off-by-one gap or double-fallback at the boundary.
  • Ordering of STATE_DIR resolution ahead of the resolver: doesn't change behavior for the degraded/COMPACTED_FILE check, since COMPACTED_FILE is still gated on HOME being set exactly as before.
  • hook::require_jq still being unconditionally called after the parse: reasonable, since scripts/context-zone.sh (the resolver) has a hard jq dependency of its own, so gating the whole hook on jq's presence up front is still correct even though the envelope parse itself may not need jq for small payloads.
  • Test diff exercises the skip, the zones.json-moved case, the resolver-failure retry case, and re-derives both the xtrace (command-position) and strace (process-creation) budgets for the steady vs. resolving paths, including a byte-identical crossing-message check against a non-skipping control session.

No correctness or security issues found in the hook logic itself. Security-shaped questions are left to /review:security-review per this lane's scope split with the repo's claude-security-review workflow.
· branch context-guard-zone-crossing-fast-path-on

@github-actions

Copy link
Copy Markdown
Contributor

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

kyle-sexton and others added 3 commits September 15, 2026 21:26
…ty lint

The shell-portability lint reads the backslash-w in the previous spelling as a GNU-only regex class. The value is opaque to the parser under test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both surfaces are declared purged in scripts/em-dash-purged-paths.txt, so the lint lane treats one as a regression.

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

The unchanged-input skip compared three inputs with `-nt` alone, which only
ever sees an existing file becoming newer. Removing the zones.json override or
the compaction marker moves no mtime, so both read as unchanged and the fire
skipped, leaving the stale zone in place until some unrelated snapshot write.

The mark now also carries one line recording whether each of the two optional
inputs existed behind the last completed resolve, read back with the `read`
builtin, and the skip is taken only when the three `-nt` tests are false and
those flags still match the current `-e` results. A mark with no readable line,
an older build's stamp or a write that failed after truncating, never takes the
skip. Both flags are captured from what the resolve actually used, so an
override created while the resolver runs is not recorded as seen.

Every added read and write is a builtin, so both process budgets are unchanged:
the small steady repeat fire still measures 3 process creations under a Windows
job object, 5 reps identical, against the subject's own floor of 3.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton force-pushed the context-guard-zone-crossing-fast-path-on branch from 9748035 to e2c053b Compare September 16, 2026 03:00
kyle-sexton and others added 4 commits September 15, 2026 23:04
…ts key bound

hook::_fast_fields indexes the payload's key strings with an associative
array, which Bash added in 4.0, and it ran on every hook::jq_fields call.
On the 3.2 shell macOS ships, and which these hooks document support for,
`local -A` fails per call. hook::_fast_fields_supported is the predicate,
split out the way hook::read_supports_nchars is so a test can force the
below-floor branch on a modern host, and hook::jq_fields_uncached asks it
before entering the fast path. Below the floor jq answers, unchanged.

The index loop also skipped any string body longer than a fixed 60 bytes
before decoding it, while nothing capped the key names a caller may ask
for. Two wrong answers came out of that: a requested key longer than 60
characters was proven ABSENT while present, and a key of 11 or more
characters spelled with \u escapes (hook_event_name is 15, 90 escaped)
was missed the same way. The bound is now six times the longest requested
key name, the width of `\uXXXX` per identifier character, which is the
bound the header comment always described.

The suite gains four cases: the below-floor branch forced with the fast
path replaced by a tripwire, a present 70-character key, an absent one,
and hook_event_name spelled entirely in \u escapes. Each compares the
fast path against jq rather than against an expectation.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The chain-slot paragraph asks a function plugged into _GAB_CONTINUE to be
builtins only, never exit, and never touch the trap, and the slot's own
comment says it must not return. run-guards.sh's consumer does none of
that: run_guards::guard_died forks a subshell for the guards still owed,
spawns jq to merge their documents, and ends at `builtin exit`.

Say so. run-guards.sh is the one documented exception, and it is one
because it is the dispatcher finishing the run the process owes rather
than a hook doing exit-time work. The discipline is unchanged for
everyone else, and the slot comment now matches it: a chained function
that returns hands control back and the handler settles the status.

Comment only.

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-no-subs' into context-guard-zone-crossing-fast-path-on
Base automatically changed from guardrails-run-guards-in-process-no-subs to main September 16, 2026 12:29
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton merged commit 0eb2dfe into main Sep 16, 2026
12 checks passed
@kyle-sexton
kyle-sexton deleted the context-guard-zone-crossing-fast-path-on branch September 16, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant