Skip to content

Commit 7538f74

Browse files
committed
Silence ambiguous callers and scope transcript scan to this loop
Two blocking findings from the previous review: [P1] Round 7 dropped the `-n "$HOOK_SESSION_ID"` check from the cross-session guard to close its own P1. That inverted the failure mode: any wrapper call (e.g. rlcr-stop-gate.sh without --session-id) matched the guard forever once the marker was written, and the RLCR loop never resumed through the wrapper path. [P2] list_pending_background_task_ids scanned the entire session-wide Claude transcript. A long-running background Agent/Bash that started earlier in the same session -- before the current RLCR loop was created -- counted as "pending" for this loop. The short-circuit fired forever for a loop that had no in-scope pending work of its own. Fixes: * loop-codex-stop-hook.sh: - Add an "Ambiguous-Caller Marker Guard" before the cross- session guard. When bg-pending.marker is present AND HOOK_SESSION_ID is empty, exit 0 silently (no systemMessage, no on-disk mutation). The real Claude stop hook always has session_id populated and remains the only authoritative driver for parking and cleanup. - Restore `[[ -n "$HOOK_SESSION_ID" ]]` inside the cross- session guard. That branch now fires only when both session ids are non-empty and different. - Compute LOOP_START_TS via derive_loop_start_iso_ts once and pass it through every pending-tasks helper call. * loop-common.sh: - New derive_loop_start_iso_ts helper: parses the loop dir basename YYYY-MM-DD_HH-MM-SS and emits YYYY-MM-DDTHH:MM:SS.000Z for lexical comparison against transcript timestamps. - list_pending_background_task_ids gains an optional since_ts argument. Launch events are filtered by ($since_ts == "" or (.timestamp // "") == "" or (.timestamp // "") >= $since_ts). Empty since_ts preserves old scan-everything behavior; events without .timestamp remain included for fixture / older record compatibility. - has_pending_background_tasks and count_pending_background_tasks pass since_ts through unchanged. Regressions in tests/test-stop-hook-bg-allow.sh: AC-10c refixtured to avoid the AC-10 marker leaking into the wrapper ambiguous-caller branch. AC-19 rewritten: empty HOOK_SESSION_ID + marker -> silent ALLOW, marker and state preserved (inverts Round 7's "parked" expectation). AC-21 helper filters a pre-loop launch and keeps an in-loop launch. AC-21b derive_loop_start_iso_ts produces the expected ISO-8601 form. AC-21c end-to-end: pre-loop launch in transcript does not trigger the short-circuit; Codex runs. AC-22 wrapper without --session-id + no prior marker + pending bg -> writes marker, surfaces systemMessage. AC-22b wrapper without --session-id + prior marker -> silent ALLOW, marker and state preserved. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 37 passed, 0 failed - bash tests/run-all-tests.sh -> 1716 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 37 passed, 0 failed systemMessage wording unchanged on existing paths. Version stays at 1.16.0.
1 parent 7fb3038 commit 7538f74

3 files changed

Lines changed: 282 additions & 30 deletions

File tree

hooks/lib/loop-common.sh

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,31 @@ extract_transcript_path() {
267267
expand_leading_tilde "$raw"
268268
}
269269

270+
# Convert an RLCR loop dir basename to a lexically-comparable ISO-8601
271+
# timestamp suitable for filtering transcript events.
272+
#
273+
# The setup script creates loop dirs named `YYYY-MM-DD_HH-MM-SS`; real
274+
# Claude transcript events carry timestamps like `2026-04-16T13:19:26.819Z`.
275+
# String comparison works cleanly once we pad the loop boundary with
276+
# `.000Z` so sub-second transcript timestamps in the same second always
277+
# compare greater.
278+
#
279+
# Usage: derive_loop_start_iso_ts "$loop_dir"
280+
# Prints the ISO-8601 timestamp, or empty string when the basename does
281+
# not match the expected format.
282+
derive_loop_start_iso_ts() {
283+
local loop_dir="$1"
284+
local base
285+
base=$(basename "$loop_dir" 2>/dev/null || echo "")
286+
if [[ "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then
287+
printf '%sT%s:%s:%s.000Z' \
288+
"${BASH_REMATCH[1]}" \
289+
"${BASH_REMATCH[2]}" \
290+
"${BASH_REMATCH[3]}" \
291+
"${BASH_REMATCH[4]}"
292+
fi
293+
}
294+
270295
# Enumerate background-task ids that have been launched but not yet marked
271296
# completed in a Claude Code transcript.jsonl.
272297
#
@@ -290,14 +315,23 @@ extract_transcript_path() {
290315
#
291316
# pending := launched \ completed
292317
#
293-
# Usage: list_pending_background_task_ids "$transcript_path"
318+
# Optional second argument `since_ts` (ISO-8601 string, e.g. the value
319+
# returned by `derive_loop_start_iso_ts`): when provided, only launch
320+
# events whose top-level `.timestamp` field is >= `since_ts` count as
321+
# candidate launches. Events without a `.timestamp` are included (keeps
322+
# fixture transcripts and older record formats working). This keeps
323+
# pre-loop session-wide background work from pinning an RLCR loop that
324+
# has no pending work of its own.
325+
#
326+
# Usage: list_pending_background_task_ids "$transcript_path" [since_ts]
294327
# - Outputs one id per line on stdout (possibly empty).
295328
# - Returns 0 when the transcript is readable (including when there are
296329
# no pending tasks). Returns 1 when the transcript path is empty, not
297330
# a regular file, or jq is unavailable, so callers must treat non-zero
298331
# as "unknown -> do not short-circuit".
299332
list_pending_background_task_ids() {
300333
local transcript_path="$1"
334+
local since_ts="${2:-}"
301335

302336
# Normalize a leading tilde so direct callers (tests, ad-hoc scripts)
303337
# work correctly even when transcript_path was not routed through
@@ -312,8 +346,13 @@ list_pending_background_task_ids() {
312346
fi
313347

314348
local launched completed
315-
launched=$(jq -r '
349+
launched=$(jq -r --arg since_ts "$since_ts" '
316350
select(.toolUseResult != null)
351+
| select(
352+
($since_ts == ""
353+
or ((.timestamp // "") == "")
354+
or ((.timestamp // "") >= $since_ts))
355+
)
317356
| select(
318357
(.toolUseResult.isAsync == true and (.toolUseResult.agentId // "") != "")
319358
or ((.toolUseResult.backgroundTaskId // "") != "")
@@ -355,22 +394,24 @@ list_pending_background_task_ids() {
355394
# Returns 1 when no pending tasks are detected (including fail-closed cases
356395
# like missing transcript, non-file path, or jq unavailable).
357396
#
358-
# Usage: has_pending_background_tasks "$transcript_path"
397+
# Usage: has_pending_background_tasks "$transcript_path" [since_ts]
359398
has_pending_background_tasks() {
360399
local transcript_path="$1"
400+
local since_ts="${2:-}"
361401
local pending
362-
pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || return 1
402+
pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || return 1
363403
[[ -n "$pending" ]]
364404
}
365405

366406
# Prints the count of pending background tasks to stdout. Prints 0 for any
367407
# error case so callers can still format messages safely.
368408
#
369-
# Usage: count_pending_background_tasks "$transcript_path"
409+
# Usage: count_pending_background_tasks "$transcript_path" [since_ts]
370410
count_pending_background_tasks() {
371411
local transcript_path="$1"
412+
local since_ts="${2:-}"
372413
local pending
373-
pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || {
414+
pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || {
374415
echo 0
375416
return 0
376417
}

hooks/loop-codex-stop-hook.sh

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,27 @@ if [[ -z "$LOOP_DIR" ]]; then
6868
exit 0
6969
fi
7070

71+
# Shared state used by both guard blocks and the pending-tasks check below.
72+
# Loop-start boundary: derived from the loop dir basename (`YYYY-MM-DD_HH-MM-SS`).
73+
# Empty means derivation failed; helpers treat empty since_ts as no boundary.
74+
LOOP_START_TS=$(derive_loop_start_iso_ts "$LOOP_DIR")
75+
HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT")
76+
77+
# ========================================
78+
# Ambiguous-Caller Marker Guard
79+
# ========================================
80+
# If a bg-pending.marker is present but we have no session_id on this
81+
# hook invocation (typical of scripts/rlcr-stop-gate.sh invoked without
82+
# --session-id, or any other caller that doesn't forward session_id),
83+
# we cannot tell whether this caller owns the parked loop. Taking either
84+
# branch (foreign-session guard below, or same-session cleanup further
85+
# down) would be wrong in one of the two possible realities. Exit 0
86+
# silently: the real Claude hook will arrive with session_id populated
87+
# and drive parking / cleanup from an authoritative context.
88+
if [[ -f "$LOOP_DIR/bg-pending.marker" ]] && [[ -z "$HOOK_SESSION_ID" ]]; then
89+
exit 0
90+
fi
91+
7192
# ========================================
7293
# Cross-Session Parked-Loop Guard
7394
# ========================================
@@ -77,18 +98,17 @@ fi
7798
# transcript sees none of the foreign bg activity - so the only safe
7899
# response is to exit 0 with a distinct systemMessage and leave every
79100
# on-disk artifact (state file, stored session_id, marker) untouched.
80-
HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT")
101+
#
102+
# Both sides of the session-id comparison must be non-empty for this
103+
# branch to trigger: an empty HOOK_SESSION_ID has already exited above
104+
# via the ambiguous-caller guard, and an empty stored session_id keeps
105+
# the backward-compat "matches any" semantics from find_active_loop.
81106
if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then
82107
GUARD_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR")
83108
if [[ -n "$GUARD_STATE_FILE" ]]; then
84109
GUARD_STORED_SID=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$GUARD_STATE_FILE" 2>/dev/null | tr -d ' ')
85-
# Non-empty stored session_id that differs from the caller's session
86-
# (empty or not) means this is a foreign parked loop. Hook-input
87-
# schemas that omit session_id -- such as rlcr-stop-gate.sh invoked
88-
# without --session-id -- still get a mismatch here and take the
89-
# safe exit path. An empty stored session_id keeps the existing
90-
# backward-compat "matches any" semantics from find_active_loop.
91110
if [[ -n "$GUARD_STORED_SID" ]] \
111+
&& [[ -n "$HOOK_SESSION_ID" ]] \
92112
&& [[ "$GUARD_STORED_SID" != "$HOOK_SESSION_ID" ]]; then
93113
jq -n \
94114
'{systemMessage: "RLCR loop in this repo is parked by another Claude session waiting for background work. Stop allowed; your session leaves the loop untouched. If that session ended, run /humanize:cancel-rlcr-loop to clean up."}'
@@ -111,10 +131,14 @@ fi
111131
# untouched -- the next natural stop (after background work finishes) will
112132
# re-enter this hook with no pending tasks and run the normal flow.
113133
#
134+
# LOOP_START_TS confines the transcript scan to launches that actually
135+
# happened during this loop; earlier session-wide bg activity cannot pin
136+
# the loop.
137+
#
114138
# This check MUST run before any other gate (phase detection, state parsing,
115139
# branch / plan / git-clean / summary / max-iter checks, Codex review).
116-
if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then
117-
PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH")
140+
if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS"; then
141+
PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS")
118142
# Mark the loop as parked; allows the same session to resume later and
119143
# makes the cross-session guard above reachable if the user opens a
120144
# different Claude session in this repo before the bg task completes.
@@ -142,7 +166,7 @@ fi
142166
# The check uses a single fresh call so we capture both the exit code
143167
# and the emptiness without double-running jq.
144168
if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then
145-
if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" 2>/dev/null) \
169+
if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS" 2>/dev/null) \
146170
&& [[ -z "$PENDING_BG_CHECK" ]]; then
147171
rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true
148172
fi

0 commit comments

Comments
 (0)