Fix #1029: remove three unconditional subshell forks per post-tool-hook.sh call

changelog.d/1029.fixed.md

@@ -0,0 +1,5 @@
+- #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.
+
+  `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.

scripts/post-tool-hook.sh

@@ -206,28 +206,32 @@ 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
-    # 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"*}
+# 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" _sjsi_field="$2" _sjsi_raw="$3" _sjsi_rest _sjsi_prefix _sjsi_value _sjsi_dq
+    printf -v "$_sjsi_var" '%s' ""
+    # 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 '%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
 # 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

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,

tests/test_post_tool_total_processes_1011.py

@@ -48,6 +48,38 @@
 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 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) 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
@@ -220,6 +252,27 @@ 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 < 67, (
+        f"below-threshold path, whole-hook strace total -- HEAD min={head_min} "
+        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)}
     skipped = []
     for tag in ("v0.42.4", "v0.43.0"):
@@ -299,3 +352,14 @@ 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 < 67, (
+        f"cooldown path, whole-hook strace total -- min={cool_min} "
+        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)"
+    )

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"),
 ]
 

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"),
 ]