From 3116fcf97ae4673aa68849398e2f571d23e51866 Mon Sep 17 00:00:00 2001 From: Florian DAVID <150798857+fdaviddpt@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:29:48 +0200 Subject: [PATCH 1/5] Follow-up investigation for #1029: the ~67-process whole-hook total is cold-start-only, not a per-call cost #1011/#1028's own CI measurement (strace -f -e trace=clone,clone3,fork,vfork, whole-hook total for post-tool-hook.sh's no-save path, minimum of 9 runs) is 67 on HEAD, down from v0.43.0's 69 and v0.42.4's 68 -- but still on the order of several dozen processes for one no-save PostToolUse call, per #1029. Traced to the test's own fixture: tests/test_post_tool_total_processes_1011.py creates a brand-new tmp_path (HOME, PROJECT, REMEMBER_DIR, TMPDIR all freshly created) for every one of the n repeated runs, so every measured run takes the slow/cold path -- _remember_env_cache_load misses, so resolve-paths.sh, detect-tools.sh, bootstrap-dirs.sh and log.sh all get sourced, pulling in lib-memory-dir.sh's worktree-detection forks, _config_load's jq read, and several mktemp+mv atomic cache publishes. On the warm path -- any call after the first in a real session, the common case once a project has run once -- _remember_env_cache_load succeeds and none of those four files are sourced at all; the only forks left are _pt_scan_lines's own tail+wc pair, which the code's own comment directly above the wc call already documents as unavoidable without reintroducing #1017's quadratic bash-expansion regression. No source change lands in this commit: every candidate #1029 named (bootstrap-dirs.sh's directory creation, the mktemp+mv atomic-write pattern, the jq config read, _pt_scan_lines's two forks) traced to either already-gated cold-path-only work (#230/#666), work inside the byte-pinned lib-memory-dir.sh (tests/test_case_divergence_298.py forbids touching it), or a fork already proven, in-repo, unavoidable without a worse regression. Forcing a cut here would mean either reaching into the pinned file or reintroducing one of #1013/#1017/#1021's own fixed regressions, neither of which is a safe trade for a performance-only, non-blocking issue. tests/test_post_tool_total_processes_1011.py now asserts an absolute budget (69 -- the measured 67 plus 2 of slack) for both the below-threshold and cooldown paths, in addition to the existing relative tag-comparisons, so a real future reduction or regression on this cold path is still caught even on a CI leg where neither tagged comparison tree can be materialized (no network reaching origin). Adds a changelog fragment documenting the corrected attribution with the CI-measured totals. Co-Authored-By: Claude Sonnet 5 --- changelog.d/1029.fixed.md | 3 ++ tests/test_post_tool_total_processes_1011.py | 48 ++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 changelog.d/1029.fixed.md diff --git a/changelog.d/1029.fixed.md b/changelog.d/1029.fixed.md new file mode 100644 index 00000000..4aee366f --- /dev/null +++ b/changelog.d/1029.fixed.md @@ -0,0 +1,3 @@ +- #1029 follow-up investigation (no behaviour change): the ~67-process whole-hook total CI measured for `post-tool-hook.sh`'s no-save path (#1011/#1028; CI-measured strace totals: `v0.42.4` = 68, `v0.43.0` = 69, HEAD = 67) is cold-start-only, not a per-call cost. `tests/test_post_tool_total_processes_1011.py`'s own fixture creates a brand-new project (fresh `HOME`/`REMEMBER_DIR`/`TMPDIR`) for every measured run, so every one of those 67 processes comes from the slow path a project only takes once: `_remember_env_cache_load` misses, so `resolve-paths.sh`, `detect-tools.sh`, `bootstrap-dirs.sh` and `log.sh` all get sourced, pulling in `lib-memory-dir.sh`'s worktree-detection forks, `_config_load`'s `jq` read, and several `mktemp`+`mv` atomic cache publishes. On the warm path -- every call after the first in a real session, once a project has run once -- `_remember_env_cache_load` succeeds and none of those four files are sourced at all; the only forks left are `_pt_scan_lines`'s own `tail`+`wc` pair, already minimal and already documented (in-code, directly above the `wc` call) as unavoidable without reintroducing #1017's quadratic bash-expansion regression. + + No further cut landed: every candidate this issue named (`bootstrap-dirs.sh`'s directory creation, the `mktemp`+`mv` atomic-write pattern, the `jq` config read, `_pt_scan_lines`'s two forks) traced to either already-gated cold-path-only work (#230/#666), work inside the byte-pinned `lib-memory-dir.sh` (`tests/test_case_divergence_298.py` forbids touching it), or a fork this repo's own prior rounds already proved unavoidable without a worse regression. `tests/test_post_tool_total_processes_1011.py` now also asserts an absolute budget (69, i.e. the measured 67 plus 2 of slack) for both the below-threshold and cooldown paths, independent of whether either tagged comparison tree can be materialized on a given CI leg, so a future regression on this cold path is still caught even when the relative tag-comparison assertions get skipped for lack of network. diff --git a/tests/test_post_tool_total_processes_1011.py b/tests/test_post_tool_total_processes_1011.py index aeef7845..f2631158 100644 --- a/tests/test_post_tool_total_processes_1011.py +++ b/tests/test_post_tool_total_processes_1011.py @@ -48,6 +48,31 @@ number nobody re-derived. `_pt_scan_lines` and its call site are a few processes out of a much larger total; this file no longer pretends it is measuring their contribution alone. + +#1029 (follow-up investigation): this file's own fixture -- a brand-new +tmp_path (HOME, PROJECT, REMEMBER_DIR, TMPDIR all freshly created) for every +single one of the `n` repeated runs -- measures ONLY the cold-start cost: no +prior invocation has ever published lib-env-cache.sh's session cache, the +SESSION_DIR cache, or log.sh's cfg-flatten cache for that tree, so every run +takes the full slow path (resolve-paths.sh -> detect-tools.sh -> +bootstrap-dirs.sh -> log.sh, lib-memory-dir.sh's worktree-detection forks, +`_config_load`'s jq read, the mktemp+mv cache publishes). On the WARM path -- +any call after the first in a real session, which is the common case once a +project has run once -- `_remember_env_cache_load` (post-tool-hook.sh:317) +succeeds and skips sourcing all four of those files entirely; the only two +forks left on that path are `_pt_scan_lines`'s own `tail`+`wc` (lines +~959/989), and the comment immediately above that `wc` call already +documents why collapsing it further reintroduces #1017's quadratic +bash-expansion regression. No further cut landed in this round: every +candidate the issue named (bootstrap-dirs.sh's mkdir, the mktemp+mv +atomic-write pattern, the jq config read, the two `_pt_scan_lines` forks) +was traced to either (a) cold-path-only work already gated behind existence +checks (#230/#666), (b) work inside the byte-pinned lib-memory-dir.sh +(tests/test_case_divergence_298.py), or (c) a fork already proven, in-repo, +unavoidable without reintroducing a worse regression. The absolute-budget +assertions below exist so a REAL future reduction (or regression) on this +cold path is still caught, without this test depending on a cut this round +did not find a safe way to make. """ from __future__ import annotations @@ -220,6 +245,21 @@ def test_below_threshold_path_total_forks_vs_tagged_baselines(tmp_path): head_min, head_all = _min_forks_for_tree( tmp_path, "head", HOOK, REPO_ROOT, n=n) + # #1029: an ABSOLUTE budget, independent of whether either tagged tree can + # be materialized here (no network reaching origin skips BOTH relative + # assertions below, leaving nothing that can fail -- see the skip branch + # a few lines down). HEAD measured 67 on every one of 9 runs across every + # ubuntu/Python CI leg as of #1028; +2 is slack for ordinary noise + # (scheduler jitter, background reaping), not room for a new fork class + # to creep back in unnoticed the way #1011 round 1 did. + assert head_min <= 69, ( + f"below-threshold path, whole-hook strace total -- HEAD min={head_min} " + f"all={sorted(head_all)} -- exceeds the absolute budget of 69 (67 " + f"measured as of #1028, +2 slack); this must hold even when neither " + f"tagged baseline tree below can be materialized, so a regression is " + f"never silently unchecked just because this CI leg has no network" + ) + results = {"HEAD": (head_min, head_all)} skipped = [] for tag in ("v0.42.4", "v0.43.0"): @@ -299,3 +339,11 @@ def test_cooldown_path_forks_fewer_than_below_threshold_path(tmp_path): f"no longer being skipped under cooldown, or this counting method " f"cannot see the decrease it exists to detect" ) + # #1029: absolute budget for the cooldown path too, same reasoning as the + # below-threshold path's own budget above -- a relative check alone + # ("fewer than below-threshold") would still pass if BOTH paths silently + # grew by the same amount. + assert cool_min <= 69, ( + f"cooldown path, whole-hook strace total -- min={cool_min} " + f"all={sorted(cool_all)} -- exceeds the absolute budget of 69" + ) From 00f26b7c73a17cd0cf5367d53e85a4f7ebd4e621 Mon Sep 17 00:00:00 2001 From: Florian DAVID <150798857+fdaviddpt@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:43:47 +0200 Subject: [PATCH 2/5] Fix #1029: port the fork-free stdin-field extractor to post-tool-hook.sh, removing three unconditional subshell forks per call Self-review (Explore + oss:auditor review of commit 3116fcf) found a real, low-risk cut this round's investigation missed: _stdin_json_string in scripts/post-tool-hook.sh was called through X=$(...) command substitution at three sites (REMEMBER_HOOK_CWD, STDIN_SESSION_ID, STDIN_TRANSCRIPT_PATH), all above the _REMEMBER_FAST if/else, so all three ran unconditionally on EVERY invocation of this hook -- cold-start and warm path alike -- each forking a subshell even though the function itself is pure parameter expansion with no external exec at all. session-start-hook.sh already carries a fork-free twin of this exact extractor, _stdin_json_string_into (#665, part of #660), written into VARNAME via printf -v instead of printed through a subshell. Ports that same function (renamed in place, same matching algorithm: same first-occurrence scan, same #344 nested-key gap, same #829 double-backslash collapse) into post-tool-hook.sh and switches all three call sites to it. No behaviour change: tests/test_hook_cwd_end_to_end_411.py, tests/test_codex_stdin_session_id_468.py and tests/test_session_end_hook_345.py, which all exercise this extraction end-to-end, pass unchanged, along with 47 other post-tool-hook.sh tests run locally (test_post_tool_* family, test_hot_path_cost_pin_330, test_pt_scan_lines_wall_clock_1017, test_post_tool_fast_path_350) and tests/test_release_branch_check_900.py's byte-budget pin. tests/test_locale_ranges_695.py's five post-tool-hook.sh line pins shift by +4 for the lines this change adds above them. Tightens tests/test_post_tool_total_processes_1011.py's new absolute-budget assertions (added in 3116fcf) from <69 to <67 -- strictly below #1028's own prior total, reflecting this cut -- for both the below-threshold and cooldown paths; the exact new total is reasoned, not re-measured locally (this file's own strace gate needs Linux, unavailable here), and this pull request's own CI run will confirm it. Updates the changelog fragment and this file's own module docstring to describe the fix landed rather than "no further cut was found." Co-Authored-By: Claude Sonnet 5 --- changelog.d/1029.fixed.md | 6 +- scripts/post-tool-hook.sh | 20 +++--- tests/test_locale_ranges_695.py | 10 +-- tests/test_post_tool_total_processes_1011.py | 76 ++++++++++++-------- 4 files changed, 67 insertions(+), 45 deletions(-) diff --git a/changelog.d/1029.fixed.md b/changelog.d/1029.fixed.md index 4aee366f..02cd0539 100644 --- a/changelog.d/1029.fixed.md +++ b/changelog.d/1029.fixed.md @@ -1,3 +1,5 @@ -- #1029 follow-up investigation (no behaviour change): the ~67-process whole-hook total CI measured for `post-tool-hook.sh`'s no-save path (#1011/#1028; CI-measured strace totals: `v0.42.4` = 68, `v0.43.0` = 69, HEAD = 67) is cold-start-only, not a per-call cost. `tests/test_post_tool_total_processes_1011.py`'s own fixture creates a brand-new project (fresh `HOME`/`REMEMBER_DIR`/`TMPDIR`) for every measured run, so every one of those 67 processes comes from the slow path a project only takes once: `_remember_env_cache_load` misses, so `resolve-paths.sh`, `detect-tools.sh`, `bootstrap-dirs.sh` and `log.sh` all get sourced, pulling in `lib-memory-dir.sh`'s worktree-detection forks, `_config_load`'s `jq` read, and several `mktemp`+`mv` atomic cache publishes. On the warm path -- every call after the first in a real session, once a project has run once -- `_remember_env_cache_load` succeeds and none of those four files are sourced at all; the only forks left are `_pt_scan_lines`'s own `tail`+`wc` pair, already minimal and already documented (in-code, directly above the `wc` call) as unavoidable without reintroducing #1017's quadratic bash-expansion regression. +- #1029: three unconditional per-call forks removed from `post-tool-hook.sh`, on top of CI-measured strace totals this range already tracks (`v0.42.4` = 68, `v0.43.0` = 69, prior HEAD per #1028 = 67). `_stdin_json_string` (`cwd`, `session_id`, `transcript_path`) was called through `X=$(...)` command substitution at three sites above the fast/slow-path split, each forking a subshell even though the function itself is pure parameter expansion -- the same cost class `lib-clock.sh`'s own `_remember_date_into` fix already removed elsewhere in this hook. Ported to the fork-free `_stdin_json_string_into` form `session-start-hook.sh` already used (#665, part of #660): identical matching algorithm (same first-occurrence scan, same #344 gap, same #829 double-backslash collapse), no behaviour change, three fewer forks on every invocation of this hook, cold-start and warm path alike. - No further cut landed: every candidate this issue named (`bootstrap-dirs.sh`'s directory creation, the `mktemp`+`mv` atomic-write pattern, the `jq` config read, `_pt_scan_lines`'s two forks) traced to either already-gated cold-path-only work (#230/#666), work inside the byte-pinned `lib-memory-dir.sh` (`tests/test_case_divergence_298.py` forbids touching it), or a fork this repo's own prior rounds already proved unavoidable without a worse regression. `tests/test_post_tool_total_processes_1011.py` now also asserts an absolute budget (69, i.e. the measured 67 plus 2 of slack) for both the below-threshold and cooldown paths, independent of whether either tagged comparison tree can be materialized on a given CI leg, so a future regression on this cold path is still caught even when the relative tag-comparison assertions get skipped for lack of network. + `tests/test_post_tool_total_processes_1011.py`'s own fixture measures the cold-start path only (a brand-new project with no warmed caches), which is why #1011/#1028's own strace totals land near 67-69 rather than the much lower warm-path count a real session's later tool calls pay once a project has run once. On that warm path, the only forks left after this cut are `_pt_scan_lines`'s own `tail`+`wc` pair, already documented in-code as unavoidable without reintroducing #1017's quadratic bash-expansion regression -- every other candidate this issue named (`bootstrap-dirs.sh`'s directory creation, the `jq` config read, the `mktemp`+`mv` atomic-write patterns) traced to cold-path-only work already gated behind existence checks (#230/#666) or work inside the byte-pinned `lib-memory-dir.sh` (`tests/test_case_divergence_298.py` forbids touching it). + + `tests/test_post_tool_total_processes_1011.py` now asserts an absolute budget (strictly below 67, this round's own prior total) for both the below-threshold and cooldown paths, in addition to the existing relative tag-comparisons, so a regression is caught even on a no-network CI leg where neither tagged baseline tree can be materialized. The exact new total is reasoned, not re-measured locally (this file's own strace gate needs Linux), and will be confirmed by this pull request's own CI run. diff --git a/scripts/post-tool-hook.sh b/scripts/post-tool-hook.sh index 3a9ffdb6..432d1b63 100755 --- a/scripts/post-tool-hook.sh +++ b/scripts/post-tool-hook.sh @@ -206,8 +206,12 @@ fi # -- same extractor mechanism, same finding (on every # host checked, `tool_input` is positioned AFTER the top-level `cwd` field, # so this stays the safe nested-after case), not repeated here. -_stdin_json_string() { - local field="$1" raw="$2" rest prefix value dq +# 1029: VARNAME is the new first argument, written via printf -v instead +# of printed, so a call site forks no subshell -- same shape as +# session-start-hook.sh own _stdin_json_string_into, 665/660. +_stdin_json_string_into() { + local _sjsi_var="$1" field="$2" raw="$3" rest prefix value dq + printf -v "$_sjsi_var" '%s' "" # The double quote is held in `dq` (octal 042) rather than written # backslash-escaped: the plugin directory's scanner mis-tracks an # escaped quote (#898 round 8). Same patterns, same quoting of $field. @@ -222,12 +226,12 @@ _stdin_json_string() { # Codex arrives as `C:\\work\\proj` otherwise (#829). value=${value//\\\\/\\} [ -n "$value" ] || return 1 - printf '%s' "$value" + printf -v "$_sjsi_var" '%s' "$value" } # The double quote, for the last-save.json scope check and the sanitiser # log line below -- held in a variable rather than backslash-escaped, the -# same reason as dq in _stdin_json_string (#898 round 8). +# same reason as _sjsi_dq in _stdin_json_string_into (#898 round 8). printf -v _pt_dq '\042' # ── The cwd the host handed us (#411, #444) ───────────────────────────── @@ -247,7 +251,7 @@ printf -v _pt_dq '\042' # not. Precedence is CLAUDE_PROJECT_DIR, then this, then the existing # .claude/remember layout derivation, then the existing failure -- resolved # entirely inside resolve-paths.sh, unchanged by this hook. -REMEMBER_HOOK_CWD=$(_stdin_json_string cwd "$HOOK_STDIN" 2>/dev/null) || REMEMBER_HOOK_CWD="" +_stdin_json_string_into REMEMBER_HOOK_CWD cwd "$HOOK_STDIN" 2>/dev/null if [[ "$REMEMBER_HOOK_CWD" == *$'\n'* ]] \ || [[ "$REMEMBER_HOOK_CWD" == *$'\r'* ]]; then REMEMBER_HOOK_CWD="" @@ -418,10 +422,10 @@ fi # --- Which session is this invocation FOR? (#212) --- # HOOK_STDIN was already captured above, ahead of path resolution (#444, so # the same payload's `cwd` can feed resolve-paths.sh's REMEMBER_HOOK_CWD -# fallback), and _stdin_json_string already defined there. session_id is +# fallback), and _stdin_json_string_into already defined there. session_id is # extracted from that same capture rather than reading stdin a second time — # it is a single stream and a second read here would see EOF. -STDIN_SESSION_ID=$(_stdin_json_string session_id "$HOOK_STDIN" 2>/dev/null) || STDIN_SESSION_ID="" +_stdin_json_string_into STDIN_SESSION_ID session_id "$HOOK_STDIN" 2>/dev/null # stdin is not more trustworthy than a basename. The id becomes both a path # component under capture-alive.d/ and a transcript filename, so it faces the # same guard the basename-derived id now faces too (#620) -- both are @@ -466,7 +470,7 @@ fi # return is rejected outright; whether the value actually names a file is # the `[ -f ]` check below, not a character allowlist -- a transcript path # legitimately contains slashes and dots. -STDIN_TRANSCRIPT_PATH=$(_stdin_json_string transcript_path "$HOOK_STDIN" 2>/dev/null) || STDIN_TRANSCRIPT_PATH="" +_stdin_json_string_into STDIN_TRANSCRIPT_PATH transcript_path "$HOOK_STDIN" 2>/dev/null if [[ "$STDIN_TRANSCRIPT_PATH" == *$'\r'* ]]; then STDIN_TRANSCRIPT_PATH="" fi diff --git a/tests/test_locale_ranges_695.py b/tests/test_locale_ranges_695.py index 097ddc03..9f7d5f54 100644 --- a/tests/test_locale_ranges_695.py +++ b/tests/test_locale_ranges_695.py @@ -199,7 +199,7 @@ def _is_protected(text: str, line_no: int, line: str) -> bool: # from the scan, so that one libc's behaviour stays visible and can be # re-checked rather than quietly assumed forever. ("scripts/agy-stop-hook.sh", 159): _CASE, - ("scripts/post-tool-hook.sh", 449): _CASE, + ("scripts/post-tool-hook.sh", 453): _CASE, # Lines shifted by #913's SESSION_DIR cache sidecar, inserted above # these three (session-dir-cache does not touch SESSION_ID or add a new # bracket range of its own -- these are the same three pre-existing @@ -215,8 +215,8 @@ def _is_protected(text: str, line_no: int, line: str) -> bool: # unchanged rewrite landed between the first and the other two), and once # more by #1011's own self-review round (a stale-comment fix above the # glob-walk, plus a longer comment on the capture-alive in-place write). - ("scripts/post-tool-hook.sh", 767): _CASE, - ("scripts/post-tool-hook.sh", 810): _CASE, + ("scripts/post-tool-hook.sh", 771): _CASE, + ("scripts/post-tool-hook.sh", 814): _CASE, # The #913 transcript-scan sidecar's own key: a filename component, same # [A-Za-z0-9._-]+ charset as SESSION_ID right above, never locale text -- # measured the same way, same reason. Shifted +27 (957->984, 1059->1086) @@ -235,8 +235,8 @@ def _is_protected(text: str, line_no: int, line: str) -> bool: # class this whole round exists to remove), and +4 for the comment # above `_nl_count` that now describes the herestring instead of the # pipe it replaced. - ("scripts/post-tool-hook.sh", 1001): _CASE, - ("scripts/post-tool-hook.sh", 1103): _CASE, + ("scripts/post-tool-hook.sh", 1005): _CASE, + ("scripts/post-tool-hook.sh", 1107): _CASE, ("scripts/session-end-hook.sh", 201): _CASE, ("scripts/session-end-hook.sh", 209): _CASE, ("scripts/session-start-hook.sh", 289): _CASE, diff --git a/tests/test_post_tool_total_processes_1011.py b/tests/test_post_tool_total_processes_1011.py index f2631158..bd3442d5 100644 --- a/tests/test_post_tool_total_processes_1011.py +++ b/tests/test_post_tool_total_processes_1011.py @@ -49,30 +49,37 @@ processes out of a much larger total; this file no longer pretends it is measuring their contribution alone. -#1029 (follow-up investigation): this file's own fixture -- a brand-new -tmp_path (HOME, PROJECT, REMEMBER_DIR, TMPDIR all freshly created) for every -single one of the `n` repeated runs -- measures ONLY the cold-start cost: no -prior invocation has ever published lib-env-cache.sh's session cache, the -SESSION_DIR cache, or log.sh's cfg-flatten cache for that tree, so every run -takes the full slow path (resolve-paths.sh -> detect-tools.sh -> -bootstrap-dirs.sh -> log.sh, lib-memory-dir.sh's worktree-detection forks, -`_config_load`'s jq read, the mktemp+mv cache publishes). On the WARM path -- -any call after the first in a real session, which is the common case once a -project has run once -- `_remember_env_cache_load` (post-tool-hook.sh:317) -succeeds and skips sourcing all four of those files entirely; the only two -forks left on that path are `_pt_scan_lines`'s own `tail`+`wc` (lines -~959/989), and the comment immediately above that `wc` call already -documents why collapsing it further reintroduces #1017's quadratic -bash-expansion regression. No further cut landed in this round: every +#1029 (follow-up investigation and fix): this file's own fixture -- a +brand-new tmp_path (HOME, PROJECT, REMEMBER_DIR, TMPDIR all freshly created) +for every single one of the `n` repeated runs -- measures ONLY the +cold-start cost: no prior invocation has ever published lib-env-cache.sh's +session cache, the SESSION_DIR cache, or log.sh's cfg-flatten cache for that +tree, so every run takes the full slow path (resolve-paths.sh -> +detect-tools.sh -> bootstrap-dirs.sh -> log.sh, lib-memory-dir.sh's +worktree-detection forks, `_config_load`'s jq read, the mktemp+mv cache +publishes). On the WARM path -- any call after the first in a real session, +which is the common case once a project has run once -- the fast-path review +of this round (self-review finding) located a real cut that ran UNCONDITIONALLY +on BOTH paths, not cold-start-only: `_stdin_json_string` was called through +`X=$(...)` command substitution at three sites (REMEMBER_HOOK_CWD, +STDIN_SESSION_ID, STDIN_TRANSCRIPT_PATH), all above the `_REMEMBER_FAST` +if/else, each forking a subshell even though the function itself is pure +parameter expansion -- the same cost class lib-clock.sh's own +`_remember_date_into` comment documents. Ported to the fork-free +`_stdin_json_string_into` form session-start-hook.sh already used (#665, +part of #660) -- same matching algorithm, no behaviour change, three fewer +forks on EVERY invocation of this hook. What remains on the warm path after +this cut: `_pt_scan_lines`'s own `tail`+`wc` (lines ~959/989), which the +comment immediately above that `wc` call documents as unavoidable without +reintroducing #1017's quadratic bash-expansion regression. Every OTHER candidate the issue named (bootstrap-dirs.sh's mkdir, the mktemp+mv -atomic-write pattern, the jq config read, the two `_pt_scan_lines` forks) -was traced to either (a) cold-path-only work already gated behind existence -checks (#230/#666), (b) work inside the byte-pinned lib-memory-dir.sh -(tests/test_case_divergence_298.py), or (c) a fork already proven, in-repo, -unavoidable without reintroducing a worse regression. The absolute-budget -assertions below exist so a REAL future reduction (or regression) on this -cold path is still caught, without this test depending on a cut this round -did not find a safe way to make. +atomic-write pattern, the jq config read) traced to either (a) cold-path-only +work already gated behind existence checks (#230/#666), or (b) work inside +the byte-pinned lib-memory-dir.sh (tests/test_case_divergence_298.py). The +absolute-budget assertions below are tightened to reflect this cut (REASONED +near the new total, not re-measured locally -- this file's own strace gate +needs Linux, not available here) and exist so a future reduction or +regression on this cold path is still caught even on a no-network CI leg. """ from __future__ import annotations @@ -252,12 +259,18 @@ def test_below_threshold_path_total_forks_vs_tagged_baselines(tmp_path): # ubuntu/Python CI leg as of #1028; +2 is slack for ordinary noise # (scheduler jitter, background reaping), not room for a new fork class # to creep back in unnoticed the way #1011 round 1 did. - assert head_min <= 69, ( + assert head_min < 67, ( f"below-threshold path, whole-hook strace total -- HEAD min={head_min} " - f"all={sorted(head_all)} -- exceeds the absolute budget of 69 (67 " - f"measured as of #1028, +2 slack); this must hold even when neither " - f"tagged baseline tree below can be materialized, so a regression is " - f"never silently unchecked just because this CI leg has no network" + f"all={sorted(head_all)} -- exceeds the absolute budget of 67 (this " + f"round's own cut -- three unconditional per-call forks removed from " + f"_stdin_json_string, #1029 -- is REASONED to land near 64, not " + f"measured locally since this file's own strace gate requires Linux; " + f"67 is #1028's own prior total, so this budget is strictly tighter " + f"than before even if the reasoned estimate is off, and still never " + f"<=69 -- see the below comment for why that bound was wrong); this " + f"must hold even when neither tagged baseline tree below can be " + f"materialized, so a regression is never silently unchecked just " + f"because this CI leg has no network" ) results = {"HEAD": (head_min, head_all)} @@ -343,7 +356,10 @@ def test_cooldown_path_forks_fewer_than_below_threshold_path(tmp_path): # below-threshold path's own budget above -- a relative check alone # ("fewer than below-threshold") would still pass if BOTH paths silently # grew by the same amount. - assert cool_min <= 69, ( + assert cool_min < 67, ( f"cooldown path, whole-hook strace total -- min={cool_min} " - f"all={sorted(cool_all)} -- exceeds the absolute budget of 69" + f"all={sorted(cool_all)} -- exceeds the absolute budget of 67 " + f"(tightened for #1029's own cut, same three unconditional forks " + f"removed on this path too; see the below-threshold test's own " + f"budget comment for the full reasoning)" ) From 933c8cf87da08d6090a79af4059c4e043a7145dc Mon Sep 17 00:00:00 2001 From: Florian DAVID <150798857+fdaviddpt@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:15:04 +0200 Subject: [PATCH 3/5] Fix #447 tests: follow post-tool-hook.sh's extractor rename to _stdin_json_string_into 00f26b7 ported the fork-free printf-v extractor into post-tool-hook.sh, renaming _stdin_json_string to _stdin_json_string_into there (same shape as session-start-hook.sh's own #665/#660 port). The #447 characterization tests still looked for the old name by string match, so all 7 post-tool/* parametrized cases failed collection-time assertions on every CI leg ("_stdin_json_string not found in scripts/post-tool-hook.sh"), not a behaviour regression. Confirmed the new extractor keeps the same first-occurrence scan, the same documented nested-before/after gap, and the same #829 double-backslash collapse by reading its body directly (post-tool-hook.sh:212-230): identical mechanism to _stdin_json_string, just printf -v into a named variable instead of printed and captured via command substitution. No product fix needed -- only the test's CASES table, which already supported the "_into" calling convention via session-start-hook.sh's own rows in the same table. Co-Authored-By: Claude Sonnet 5 --- tests/test_stdin_extractor_top_level_wins_447.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_stdin_extractor_top_level_wins_447.py b/tests/test_stdin_extractor_top_level_wins_447.py index 8ff2d517..03b38b67 100644 --- a/tests/test_stdin_extractor_top_level_wins_447.py +++ b/tests/test_stdin_extractor_top_level_wins_447.py @@ -158,8 +158,8 @@ def _nested_before(key: str, top: str, nested: str) -> str: pytest.param("scripts/session-start-hook.sh", "_stdin_json_string_into", True, "session_id", id="session-start/session_id"), pytest.param("scripts/session-end-hook.sh", "_stdin_json_string", True, "cwd", id="session-end/cwd"), pytest.param("scripts/session-end-hook.sh", "_stdin_json_string", True, "session_id", id="session-end/session_id"), - pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string", True, "cwd", id="post-tool/cwd"), - pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string", True, "session_id", id="post-tool/session_id"), + pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string_into", True, "cwd", id="post-tool/cwd"), + pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string_into", True, "session_id", id="post-tool/session_id"), pytest.param("scripts/user-prompt-hook.sh", "_stdin_cwd", False, "cwd", id="user-prompt/cwd"), ] From ebc5fcae233b81f5a4e48419007c5e1fb9d2893c Mon Sep 17 00:00:00 2001 From: Florian DAVID <150798857+fdaviddpt@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:32:12 +0200 Subject: [PATCH 4/5] Fix #829's post-tool EXTRACTORS row to match the same extractor rename The previous commit fixed test_stdin_extractor_top_level_wins_447.py's own CASES table but missed this file's separate EXTRACTORS table, which also names post-tool-hook.sh's extractor by the old "_stdin_json_string" / keyed=True / into=False shape. CI's first re-run surfaced the gap directly: test_windows_cwd_is_decoded[post-tool], test_unc_prefix_decodes_pairwise[post-tool] and test_posix_cwd_is_unchanged[post-tool] all failed the same "_stdin_json_string not found in scripts/post-tool-hook.sh" assertion. Same fix as before: the function is now _stdin_json_string_into (printf -v, no subshell), so this row flips to into=True, matching session-start-hook.sh's own "session-start/into" row in the same table. Co-Authored-By: Claude Sonnet 5 --- tests/test_stdin_json_backslash_decode_829.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_stdin_json_backslash_decode_829.py b/tests/test_stdin_json_backslash_decode_829.py index 7dbde442..bb622519 100644 --- a/tests/test_stdin_json_backslash_decode_829.py +++ b/tests/test_stdin_json_backslash_decode_829.py @@ -55,7 +55,7 @@ def _call_into(script: str, func_name: str, keyed: bool, key: str, raw: str): EXTRACTORS = [ pytest.param("scripts/session-start-hook.sh", "_stdin_json_string_into", True, True, id="session-start/into"), pytest.param("scripts/session-end-hook.sh", "_stdin_json_string", True, False, id="session-end"), - pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string", True, False, id="post-tool"), + pytest.param("scripts/post-tool-hook.sh", "_stdin_json_string_into", True, True, id="post-tool"), pytest.param("scripts/user-prompt-hook.sh", "_stdin_cwd", False, False, id="user-prompt"), pytest.param("scripts/user-prompt-hook.sh", "_stdin_cwd_into", False, True, id="user-prompt/into"), ] From e868ae322f95232df2fa453f2248b71446549335 Mon Sep 17 00:00:00 2001 From: Florian DAVID <150798857+fdaviddpt@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:02:38 +0200 Subject: [PATCH 5/5] Fix #1029 review: _sjsi_-prefix every local in the ported extractor printf -v resolves against the innermost local, so the port left a latent collision: a future call site passing a VARNAME of value, dq, rest, raw, field or prefix would write the function own local, not the caller. session-start-hook.sh copy already prefixes every local for this reason. Same line count, so the locale-range pins do not move. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/post-tool-hook.sh | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/scripts/post-tool-hook.sh b/scripts/post-tool-hook.sh index 432d1b63..359e21ca 100755 --- a/scripts/post-tool-hook.sh +++ b/scripts/post-tool-hook.sh @@ -210,23 +210,23 @@ fi # of printed, so a call site forks no subshell -- same shape as # session-start-hook.sh own _stdin_json_string_into, 665/660. _stdin_json_string_into() { - local _sjsi_var="$1" field="$2" raw="$3" rest prefix value dq + local _sjsi_var="$1" _sjsi_field="$2" _sjsi_raw="$3" _sjsi_rest _sjsi_prefix _sjsi_value _sjsi_dq printf -v "$_sjsi_var" '%s' "" - # The double quote is held in `dq` (octal 042) rather than written - # backslash-escaped: the plugin directory's scanner mis-tracks an - # escaped quote (#898 round 8). Same patterns, same quoting of $field. - printf -v dq '\042' - [[ "$raw" == *"$dq$field$dq"* ]] || return 1 - rest=${raw#*"$dq"$field"$dq"} - prefix=${rest%%"$dq"*} - if [[ "$prefix" == *[!:[:space:]]* ]]; then return 1; fi - value=${rest#*"$dq"} - value=${value%%"$dq"*} + # Every local is _sjsi_-prefixed, as in session-start-hook.sh: printf -v + # resolves against the innermost local, so an unprefixed `value` here + # would swallow a caller's VARNAME of the same name. + printf -v _sjsi_dq '\042' + [[ "$_sjsi_raw" == *"$_sjsi_dq$_sjsi_field$_sjsi_dq"* ]] || return 1 + _sjsi_rest=${_sjsi_raw#*"$_sjsi_dq"$_sjsi_field"$_sjsi_dq"} + _sjsi_prefix=${_sjsi_rest%%"$_sjsi_dq"*} + if [[ "$_sjsi_prefix" == *[!:[:space:]]* ]]; then return 1; fi + _sjsi_value=${_sjsi_rest#*"$_sjsi_dq"} + _sjsi_value=${_sjsi_value%%"$_sjsi_dq"*} # A JSON encoder writes each backslash as `\\` -- a Windows `cwd` from # Codex arrives as `C:\\work\\proj` otherwise (#829). - value=${value//\\\\/\\} - [ -n "$value" ] || return 1 - printf -v "$_sjsi_var" '%s' "$value" + _sjsi_value=${_sjsi_value//\\\\/\\} + [ -n "$_sjsi_value" ] || return 1 + printf -v "$_sjsi_var" '%s' "$_sjsi_value" } # The double quote, for the last-save.json scope check and the sanitiser