From 9730744d124ad0ab0ae9ef5877ddddc86fb176fa Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 13:35:00 -0700 Subject: [PATCH 01/13] Allow natural stop when background tasks still running Add a background-task short-circuit to the RLCR stop hook so it exits 0 with a user-facing systemMessage whenever the Claude Code transcript shows at least one pending background dispatch (Agent run_in_background, or Bash run_in_background) that has not yet produced a completion queue-operation notification. The short-circuit precedes every other gate (phase detection, state parse, branch consistency, plan integrity, git cleanliness, summary, BitLesson, max iterations, Codex review) and touches no on-disk state; when the background work finishes, the next natural stop re-enters the normal review flow. Supporting helpers in loop-common.sh: - extract_transcript_path mirrors extract_session_id - list_pending_background_task_ids parses the transcript jsonl and diffs launched ids against queue-operation completion task-ids - has_pending_background_tasks + count_pending_background_tasks wrap that list with fail-closed semantics when the transcript path is missing or jq is unavailable The rlcr-stop-gate wrapper had two latent bugs newly exposed by forwarding transcript_path end-to-end: select(length>0) on an object field collapsed the entire JSON when session_id was empty, and a wrapped response with no decision field was treated as Unexpected instead of ALLOW. Both paths are fixed alongside this change. tests/test-stop-hook-bg-allow.sh covers AC-1..AC-9 end-to-end (11 cases including two missing-path variants and one wrapper smoke test) and is registered in run-all-tests.sh. Full suite: 1690 passed, 0 failed. Branch targets dev; version stays at 1.16.0 as requested. --- hooks/lib/loop-common.sh | 96 +++++++ hooks/loop-codex-stop-hook.sh | 24 ++ scripts/rlcr-stop-gate.sh | 30 +- tests/run-all-tests.sh | 1 + tests/test-stop-hook-bg-allow.sh | 467 +++++++++++++++++++++++++++++++ 5 files changed, 613 insertions(+), 5 deletions(-) create mode 100755 tests/test-stop-hook-bg-allow.sh diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 8d013b71..a2119511 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -242,6 +242,102 @@ extract_session_id() { printf '%s' "$input" | jq -r '.session_id // empty' 2>/dev/null || echo "" } +# Extract transcript_path from hook JSON input +# Usage: extract_transcript_path "$json_input" +# Outputs the transcript_path to stdout, or empty string if not available +extract_transcript_path() { + local input="$1" + printf '%s' "$input" | jq -r '.transcript_path // empty' 2>/dev/null || echo "" +} + +# Enumerate background-task ids that have been launched but not yet marked +# completed in a Claude Code transcript.jsonl. +# +# Launch events (inspected in tool_result "user" messages): +# - Background subagent: toolUseResult.isAsync == true +# -> id is toolUseResult.agentId +# - Background shell: toolUseResult.backgroundTaskId non-empty +# -> id is toolUseResult.backgroundTaskId +# +# Completion events (inspected in "queue-operation" messages with +# operation == "enqueue" whose content contains a +# XML block): any ... value is treated as terminal +# regardless of the reported (completed, failed, killed, +# cancelled, interrupted, ...). +# +# pending := launched \ completed +# +# Usage: list_pending_background_task_ids "$transcript_path" +# - Outputs one id per line on stdout (possibly empty). +# - Returns 0 when the transcript is readable (including when there are +# no pending tasks). Returns 1 when the transcript path is empty, not +# a regular file, or jq is unavailable, so callers must treat non-zero +# as "unknown -> do not short-circuit". +list_pending_background_task_ids() { + local transcript_path="$1" + + if [[ -z "$transcript_path" ]] || [[ ! -f "$transcript_path" ]]; then + return 1 + fi + if ! command -v jq >/dev/null 2>&1; then + return 1 + fi + + local launched completed + launched=$(jq -r ' + select(.toolUseResult != null) + | select( + (.toolUseResult.isAsync == true and (.toolUseResult.agentId // "") != "") + or ((.toolUseResult.backgroundTaskId // "") != "") + ) + | (.toolUseResult.agentId // .toolUseResult.backgroundTaskId) + ' "$transcript_path" 2>/dev/null | sort -u) || return 1 + + completed=$(jq -r ' + select(.type == "queue-operation" and .operation == "enqueue") + | (.content // "" | tostring) + | select(contains("")) + ' "$transcript_path" 2>/dev/null \ + | grep -oE '[^<]+' \ + | sed -E 's|||g' \ + | sort -u) || completed="" + + # Emit launched ids that have no matching completion notification. + comm -23 \ + <(printf '%s\n' "$launched" | sed '/^$/d') \ + <(printf '%s\n' "$completed" | sed '/^$/d') +} + +# Returns 0 when the transcript shows at least one pending background task. +# Returns 1 when no pending tasks are detected (including fail-closed cases +# like missing transcript, non-file path, or jq unavailable). +# +# Usage: has_pending_background_tasks "$transcript_path" +has_pending_background_tasks() { + local transcript_path="$1" + local pending + pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || return 1 + [[ -n "$pending" ]] +} + +# Prints the count of pending background tasks to stdout. Prints 0 for any +# error case so callers can still format messages safely. +# +# Usage: count_pending_background_tasks "$transcript_path" +count_pending_background_tasks() { + local transcript_path="$1" + local pending + pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || { + echo 0 + return 0 + } + if [[ -z "$pending" ]]; then + echo 0 + else + printf '%s\n' "$pending" | sed '/^$/d' | wc -l | tr -d ' ' + fi +} + # Resolve the active state file for a loop directory # Checks for finalize-state.md first, then state.md # Usage: resolve_active_state_file "$loop_dir" diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index 570a4867..5b57a3cb 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -68,6 +68,30 @@ if [[ -z "$LOOP_DIR" ]]; then exit 0 fi +# ======================================== +# Early Exit: Pending Background Tasks +# ======================================== +# When the main Claude Code session has dispatched background work (Agent with +# run_in_background=true, or Bash with run_in_background=true) whose +# completion notifications have not yet arrived, the natural "stop" is simply +# "I am waiting for the background task". Running git/summary/BitLesson/Codex +# gates in that state wastes Codex tokens and produces low-signal reviews. +# +# Allow the stop (exit 0) and emit a user-visible systemMessage so nobody +# mistakes the pause for loop completion. The on-disk loop state is left +# untouched -- the next natural stop (after background work finishes) will +# re-enter this hook with no pending tasks and run the normal flow. +# +# This check MUST run before any other gate (phase detection, state parsing, +# branch / plan / git-clean / summary / max-iter checks, Codex review). +HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") +if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then + PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH") + jq -n --arg count "$PENDING_BG_COUNT" \ + '{systemMessage: ("RLCR loop active. " + $count + " background task(s) still running - stop allowed naturally; loop has NOT terminated and will resume on completion.")}' + exit 0 +fi + # ======================================== # Detect Loop Phase: Normal or Finalize # ======================================== diff --git a/scripts/rlcr-stop-gate.sh b/scripts/rlcr-stop-gate.sh index 09166100..1e928892 100755 --- a/scripts/rlcr-stop-gate.sh +++ b/scripts/rlcr-stop-gate.sh @@ -83,9 +83,15 @@ if ! command -v jq >/dev/null 2>&1; then exit 20 fi -# Build hook input JSON while omitting empty fields. -# Include standard Stop hook fields so the underlying hook sees the same schema -# as a real Claude Code Stop event (hook_event_name, stop_hook_active, cwd). +# Build hook input JSON. Include standard Stop hook fields so the underlying +# hook sees the same schema as a real Claude Code Stop event +# (hook_event_name, stop_hook_active, cwd). +# +# Empty session_id / transcript_path become explicit null instead of being +# filtered out; a `select(length > 0)` used as a plain object value collapses +# the entire enclosing object to empty whenever any selected field is empty, +# which would hide forwarded fields like transcript_path when only session_id +# is missing. HOOK_INPUT=$(jq -n \ --arg session_id "$SESSION_ID" \ --arg transcript_path "$TRANSCRIPT_PATH" \ @@ -99,8 +105,8 @@ HOOK_INPUT=$(jq -n \ model: $model, permission_mode: $permission_mode, last_assistant_message: null, - session_id: ($session_id | select(length > 0)), - transcript_path: ($transcript_path | select(length > 0)) + session_id: (if ($session_id | length) > 0 then $session_id else null end), + transcript_path: (if ($transcript_path | length) > 0 then $transcript_path else null end) }') # Capture hook exit code explicitly to map non-zero to exit 20 (wrapper error) @@ -140,6 +146,20 @@ if [[ "$DECISION" == "block" ]]; then exit 10 fi +# No decision field in the JSON: per Claude Code Stop-hook spec this means +# allow the stop. Surface any systemMessage so callers see the reason +# (e.g. "background task(s) still running"), then exit 0. +if [[ -z "$DECISION" ]]; then + if [[ "$PRINT_JSON" == "true" ]]; then + printf '%s\n' "$HOOK_OUTPUT" + elif [[ -n "$SYSTEM_MESSAGE" ]]; then + printf 'ALLOW: %s\n' "$SYSTEM_MESSAGE" + else + echo "ALLOW: stop gate passed." + fi + exit 0 +fi + echo "Error: Unexpected hook decision: ${DECISION:-}" >&2 printf '%s\n' "$HOOK_OUTPUT" >&2 exit 20 diff --git a/tests/run-all-tests.sh b/tests/run-all-tests.sh index b6ba6b24..a39d9ab1 100755 --- a/tests/run-all-tests.sh +++ b/tests/run-all-tests.sh @@ -68,6 +68,7 @@ TEST_SUITES=( "test-templates-comprehensive.sh" "test-plan-file-hooks.sh" "test-stop-hook-legacy-compat.sh" + "test-stop-hook-bg-allow.sh" "test-error-scenarios.sh" "test-ansi-parsing.sh" "test-allowlist-validators.sh" diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh new file mode 100755 index 00000000..c1e89853 --- /dev/null +++ b/tests/test-stop-hook-bg-allow.sh @@ -0,0 +1,467 @@ +#!/usr/bin/env bash +# +# Tests for the background-task short-circuit in loop-codex-stop-hook.sh. +# +# When the current Claude Code session has dispatched background work that has +# not yet completed (via Agent run_in_background=true or Bash +# run_in_background=true), the RLCR stop hook must exit 0 with a user-facing +# systemMessage instead of running any gate or Codex review. The on-disk loop +# state must remain unchanged, so that the next natural stop (after the +# background task finishes) re-enters the normal review flow. +# +# Acceptance criteria exercised here (see +# .humanize/rlcr/2026-04-16_13-19-26/goal-tracker.md for authoritative list): +# AC-1 no bg dispatches -> normal Codex flow +# AC-2 pending subagent -> exit 0 + systemMessage +# AC-3 pending shell -> exit 0 + systemMessage +# AC-4 subagent launch + complete -> normal Codex flow +# AC-5 2 subagents + 1 shell -> systemMessage mentions "3 background" +# AC-6 missing transcript path -> normal Codex flow (fail-closed) +# AC-7 no active loop -> exit 0, no systemMessage, no Codex +# AC-8 finalize phase pending bg -> exit 0 + systemMessage +# AC-9 via rlcr-stop-gate.sh -> exit 0 (wrapper ALLOW) +# + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +source "$SCRIPT_DIR/test-helpers.sh" + +STOP_HOOK="$PROJECT_ROOT/hooks/loop-codex-stop-hook.sh" +GATE_SCRIPT="$PROJECT_ROOT/scripts/rlcr-stop-gate.sh" + +setup_test_dir + +export XDG_CACHE_HOME="$TEST_DIR/.cache" +mkdir -p "$XDG_CACHE_HOME" + +# ---------------------------------------------------------------------- +# Mock codex CLI: records an invocation marker and prints canned feedback. +# ---------------------------------------------------------------------- +setup_mock_codex() { + mkdir -p "$TEST_DIR/bin" + cat > "$TEST_DIR/bin/codex" << 'EOF' +#!/usr/bin/env bash +if [[ -n "${MOCK_CODEX_MARKER:-}" ]]; then + : > "$MOCK_CODEX_MARKER" +fi +printf '%s\n' "${MOCK_CODEX_OUTPUT:-Mock review feedback}" +exit 0 +EOF + chmod +x "$TEST_DIR/bin/codex" + export PATH="$TEST_DIR/bin:$PATH" +} + +# ---------------------------------------------------------------------- +# Build a minimal "active loop" project that satisfies every gate the +# stop hook enforces BEFORE it calls Codex (so tests that want to reach +# the Codex review flow can pass cleanly when bg-pending is not expected). +# ---------------------------------------------------------------------- +create_full_fixture() { + local repo_dir="$1" + local finalize_phase="${2:-false}" + + init_test_git_repo "$repo_dir" + + printf 'plans/\n' > "$repo_dir/.gitignore" + git -C "$repo_dir" add .gitignore + git -C "$repo_dir" commit -q -m "Add test gitignore" + + mkdir -p "$repo_dir/plans" + cat > "$repo_dir/plans/test-plan.md" << 'EOF' +# Test Plan + +Exercise the background-task short-circuit. +EOF + + local branch base_commit loop_dir + branch=$(git -C "$repo_dir" rev-parse --abbrev-ref HEAD) + base_commit=$(git -C "$repo_dir" rev-parse HEAD) + loop_dir="$repo_dir/.humanize/rlcr/2026-03-01_00-00-00" + mkdir -p "$loop_dir" + + cp "$repo_dir/plans/test-plan.md" "$loop_dir/plan.md" + + local state_name="state.md" + if [[ "$finalize_phase" == "true" ]]; then + state_name="finalize-state.md" + fi + + cat > "$loop_dir/$state_name" << EOF +--- +current_round: 0 +max_iterations: 42 +codex_model: gpt-5.4 +codex_effort: high +codex_timeout: 60 +push_every_round: false +full_review_round: 5 +plan_file: "plans/test-plan.md" +plan_tracked: false +start_branch: $branch +base_branch: $branch +base_commit: $base_commit +review_started: false +ask_codex_question: false +agent_teams: false +--- +EOF + + local summary_name="round-0-summary.md" + if [[ "$finalize_phase" == "true" ]]; then + summary_name="finalize-summary.md" + fi + cat > "$loop_dir/$summary_name" << 'EOF' +# Summary + +Exercised the background-task short-circuit. +EOF + + cat > "$loop_dir/goal-tracker.md" << 'EOF' +# Goal Tracker +## IMMUTABLE SECTION +### Ultimate Goal +Exercise background-task short-circuit. +### Acceptance Criteria +- AC-1: Hook reaches Codex review when no bg tasks are pending. +## MUTABLE SECTION +### Plan Version: 1 (Updated: Round 0) +#### Active Tasks +| Task | Target AC | Status | Notes | +|------|-----------|--------|-------| +| Exercise stop hook | AC-1 | completed | - | +EOF + + # Echo the loop dir so callers can reach state artifacts. + echo "$loop_dir" +} + +# A project with no RLCR state file at all. +create_empty_project() { + local repo_dir="$1" + init_test_git_repo "$repo_dir" +} + +# ---------------------------------------------------------------------- +# Transcript fixture builders. +# Each prints a JSONL transcript to stdout. +# ---------------------------------------------------------------------- +emit_tool_use_assistant() { + local tool_use_id="$1" tool_name="$2" extra_input_json="$3" + local input_json="{\"run_in_background\":true${extra_input_json}}" + jq -c -n \ + --arg id "$tool_use_id" \ + --arg name "$tool_name" \ + --argjson input "$input_json" \ + '{ + type:"assistant", + message:{ + role:"assistant", + content:[ + {type:"tool_use", id:$id, name:$name, input:$input} + ] + } + }' +} + +emit_async_agent_launch_result() { + local tool_use_id="$1" agent_id="$2" + jq -c -n \ + --arg id "$tool_use_id" \ + --arg aid "$agent_id" \ + '{ + type:"user", + message:{ + role:"user", + content:[{tool_use_id:$id, type:"tool_result", + content:[{type:"text", text:"Async agent launched"}]}] + }, + toolUseResult:{isAsync:true, status:"async_launched", agentId:$aid} + }' +} + +emit_bg_shell_launch_result() { + local tool_use_id="$1" bg_task_id="$2" + jq -c -n \ + --arg id "$tool_use_id" \ + --arg bid "$bg_task_id" \ + '{ + type:"user", + message:{ + role:"user", + content:[{tool_use_id:$id, type:"tool_result", + content:[{type:"text", text:"Shell started in background"}]}] + }, + toolUseResult:{backgroundTaskId:$bid} + }' +} + +emit_task_completion_event() { + local task_id="$1" tool_use_id="$2" status="${3:-completed}" + local notif + notif=$(printf '\n%s\n%s\n%s\n' \ + "$task_id" "$tool_use_id" "$status") + jq -c -n --arg content "$notif" \ + '{type:"queue-operation", operation:"enqueue", content:$content}' +} + +write_transcript() { + local path="$1" + shift + : > "$path" + for line in "$@"; do + printf '%s\n' "$line" >> "$path" + done +} + +# ---------------------------------------------------------------------- +# Invoke the stop hook with a crafted hook input JSON. +# Sets RUN_EXIT_CODE, RUN_OUTPUT, RUN_MARKER. +# ---------------------------------------------------------------------- +run_stop_hook_with_input() { + local repo_dir="$1" hook_input_json="$2" + + RUN_MARKER="$repo_dir/codex-called.marker" + rm -f "$RUN_MARKER" + + set +e + RUN_OUTPUT=$( + cd "$repo_dir" + CLAUDE_PROJECT_DIR="$repo_dir" \ + MOCK_CODEX_MARKER="$RUN_MARKER" \ + MOCK_CODEX_OUTPUT="Mock review feedback" \ + "$STOP_HOOK" <<<"$hook_input_json" 2>&1 + ) + RUN_EXIT_CODE=$? + set -e +} + +assert_systemmessage_only() { + local test_name="$1" repo_dir="$2" state_file="$3" expected_count_regex="$4" + + local before_hash after_hash + before_hash=$(sha256sum "$state_file" 2>/dev/null | awk '{print $1}') + + if [[ "$RUN_EXIT_CODE" -ne 0 ]]; then + fail "$test_name" "exit 0 with systemMessage" \ + "exit $RUN_EXIT_CODE; output: $RUN_OUTPUT" + return + fi + if [[ -f "$RUN_MARKER" ]]; then + fail "$test_name" "Codex NOT invoked" \ + "marker present (Codex was called); output: $RUN_OUTPUT" + return + fi + local system_message + system_message=$(printf '%s' "$RUN_OUTPUT" | jq -r '.systemMessage // empty' 2>/dev/null || echo "") + if [[ -z "$system_message" ]]; then + fail "$test_name" "JSON output with systemMessage" \ + "no systemMessage in output: $RUN_OUTPUT" + return + fi + if [[ -n "$expected_count_regex" ]]; then + if ! printf '%s' "$system_message" | grep -Eq "$expected_count_regex"; then + fail "$test_name" \ + "systemMessage matches /$expected_count_regex/" \ + "got: $system_message" + return + fi + fi + after_hash=$(sha256sum "$state_file" 2>/dev/null | awk '{print $1}') + if [[ "$before_hash" != "$after_hash" ]]; then + fail "$test_name" "state file unchanged" \ + "hash changed ($before_hash -> $after_hash)" + return + fi + pass "$test_name" +} + +assert_reached_codex() { + local test_name="$1" + if [[ "$RUN_EXIT_CODE" -eq 0 ]] && [[ -f "$RUN_MARKER" ]]; then + pass "$test_name" + else + fail "$test_name" "exit 0 and Codex invoked (marker present)" \ + "exit $RUN_EXIT_CODE, marker=$(test -f "$RUN_MARKER" && echo present || echo missing); output: $RUN_OUTPUT" + fi +} + +setup_mock_codex + +# Transcripts live outside any test repo to avoid tripping git cleanliness +# gates in the stop hook. +TRANSCRIPTS_DIR="$TEST_DIR/transcripts" +mkdir -p "$TRANSCRIPTS_DIR" + +echo "==========================================" +echo "Stop Hook Background-Task Allow Tests" +echo "==========================================" +echo "" + +# ---------------- AC-1 ---------------- +echo "Test AC-1: No bg dispatches -> reaches Codex" +AC1_REPO="$TEST_DIR/ac1" +create_full_fixture "$AC1_REPO" > /dev/null +AC1_TRANSCRIPT="$TRANSCRIPTS_DIR/ac1.jsonl" +write_transcript "$AC1_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' + +AC1_INPUT=$(jq -c -n --arg tp "$AC1_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC1_REPO" "$AC1_INPUT" +assert_reached_codex "AC-1: transcript without bg dispatches proceeds to Codex review" + +# ---------------- AC-2 ---------------- +echo "Test AC-2: One pending background subagent -> exit 0 + systemMessage" +AC2_REPO="$TEST_DIR/ac2" +AC2_LOOP=$(create_full_fixture "$AC2_REPO") +AC2_STATE="$AC2_LOOP/state.md" +AC2_TRANSCRIPT="$TRANSCRIPTS_DIR/ac2.jsonl" +AC2_LINE_LAUNCH=$(emit_tool_use_assistant "toolu_A" "Agent" ',"description":"x","prompt":"x"') +AC2_LINE_RESULT=$(emit_async_agent_launch_result "toolu_A" "agent_pending_A") +write_transcript "$AC2_TRANSCRIPT" "$AC2_LINE_LAUNCH" "$AC2_LINE_RESULT" + +AC2_INPUT=$(jq -c -n --arg tp "$AC2_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC2_REPO" "$AC2_INPUT" +assert_systemmessage_only \ + "AC-2: pending subagent triggers exit 0 + systemMessage, state untouched" \ + "$AC2_REPO" "$AC2_STATE" "1 background task" + +# ---------------- AC-3 ---------------- +echo "Test AC-3: One pending background shell -> exit 0 + systemMessage" +AC3_REPO="$TEST_DIR/ac3" +AC3_LOOP=$(create_full_fixture "$AC3_REPO") +AC3_STATE="$AC3_LOOP/state.md" +AC3_TRANSCRIPT="$TRANSCRIPTS_DIR/ac3.jsonl" +AC3_LINE_LAUNCH=$(emit_tool_use_assistant "toolu_B" "Bash" ',"command":"sleep 30"') +AC3_LINE_RESULT=$(emit_bg_shell_launch_result "toolu_B" "shell_pending_B") +write_transcript "$AC3_TRANSCRIPT" "$AC3_LINE_LAUNCH" "$AC3_LINE_RESULT" + +AC3_INPUT=$(jq -c -n --arg tp "$AC3_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC3_REPO" "$AC3_INPUT" +assert_systemmessage_only \ + "AC-3: pending background shell triggers exit 0 + systemMessage" \ + "$AC3_REPO" "$AC3_STATE" "1 background task" + +# ---------------- AC-4 ---------------- +echo "Test AC-4: Launched subagent with completion notification -> reaches Codex" +AC4_REPO="$TEST_DIR/ac4" +create_full_fixture "$AC4_REPO" > /dev/null +AC4_TRANSCRIPT="$TRANSCRIPTS_DIR/ac4.jsonl" +AC4_LAUNCH=$(emit_tool_use_assistant "toolu_C" "Agent" ',"description":"x","prompt":"x"') +AC4_RESULT=$(emit_async_agent_launch_result "toolu_C" "agent_done_C") +AC4_COMPLETE=$(emit_task_completion_event "agent_done_C" "toolu_C" "completed") +write_transcript "$AC4_TRANSCRIPT" "$AC4_LAUNCH" "$AC4_RESULT" "$AC4_COMPLETE" + +AC4_INPUT=$(jq -c -n --arg tp "$AC4_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC4_REPO" "$AC4_INPUT" +assert_reached_codex "AC-4: subagent with matching completion notification proceeds to Codex review" + +# ---------------- AC-5 ---------------- +echo "Test AC-5: 2 pending subagents + 1 pending shell -> systemMessage mentions 3" +AC5_REPO="$TEST_DIR/ac5" +AC5_LOOP=$(create_full_fixture "$AC5_REPO") +AC5_STATE="$AC5_LOOP/state.md" +AC5_TRANSCRIPT="$TRANSCRIPTS_DIR/ac5.jsonl" +AC5_L1_LAUNCH=$(emit_tool_use_assistant "toolu_D1" "Agent" ',"description":"x","prompt":"x"') +AC5_L1_RESULT=$(emit_async_agent_launch_result "toolu_D1" "agent_pending_D1") +AC5_L2_LAUNCH=$(emit_tool_use_assistant "toolu_D2" "Agent" ',"description":"y","prompt":"y"') +AC5_L2_RESULT=$(emit_async_agent_launch_result "toolu_D2" "agent_pending_D2") +AC5_L3_LAUNCH=$(emit_tool_use_assistant "toolu_D3" "Bash" ',"command":"sleep 30"') +AC5_L3_RESULT=$(emit_bg_shell_launch_result "toolu_D3" "shell_pending_D3") +write_transcript "$AC5_TRANSCRIPT" \ + "$AC5_L1_LAUNCH" "$AC5_L1_RESULT" \ + "$AC5_L2_LAUNCH" "$AC5_L2_RESULT" \ + "$AC5_L3_LAUNCH" "$AC5_L3_RESULT" + +AC5_INPUT=$(jq -c -n --arg tp "$AC5_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC5_REPO" "$AC5_INPUT" +assert_systemmessage_only \ + "AC-5: 2 pending subagents + 1 pending shell -> systemMessage mentions '3 background task(s)'" \ + "$AC5_REPO" "$AC5_STATE" "3 background task\\(s\\)" + +# ---------------- AC-6 ---------------- +echo "Test AC-6: missing transcript path -> reaches Codex (fail-closed)" +AC6_REPO="$TEST_DIR/ac6" +create_full_fixture "$AC6_REPO" > /dev/null +AC6_INPUT=$(jq -c -n --arg tp "/nonexistent/file-$$.jsonl" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC6_REPO" "$AC6_INPUT" +assert_reached_codex "AC-6: missing transcript_path proceeds to Codex review (fail-closed)" + +# Also: empty transcript_path field +AC6B_REPO="$TEST_DIR/ac6b" +create_full_fixture "$AC6B_REPO" > /dev/null +AC6B_INPUT='{"transcript_path":""}' +run_stop_hook_with_input "$AC6B_REPO" "$AC6B_INPUT" +assert_reached_codex "AC-6b: empty transcript_path string proceeds to Codex review" + +# And: no transcript_path key at all +AC6C_REPO="$TEST_DIR/ac6c" +create_full_fixture "$AC6C_REPO" > /dev/null +AC6C_INPUT='{}' +run_stop_hook_with_input "$AC6C_REPO" "$AC6C_INPUT" +assert_reached_codex "AC-6c: hook input with no transcript_path proceeds to Codex review" + +# ---------------- AC-7 ---------------- +echo "Test AC-7: No active loop -> exit 0, no systemMessage, no Codex" +AC7_REPO="$TEST_DIR/ac7" +create_empty_project "$AC7_REPO" +AC7_TRANSCRIPT="$TRANSCRIPTS_DIR/ac7.jsonl" +AC7_LAUNCH=$(emit_tool_use_assistant "toolu_E" "Agent" ',"description":"x","prompt":"x"') +AC7_RESULT=$(emit_async_agent_launch_result "toolu_E" "agent_pending_E") +write_transcript "$AC7_TRANSCRIPT" "$AC7_LAUNCH" "$AC7_RESULT" +AC7_INPUT=$(jq -c -n --arg tp "$AC7_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC7_REPO" "$AC7_INPUT" + +AC7_SYS_MSG=$(printf '%s' "$RUN_OUTPUT" | jq -r '.systemMessage // empty' 2>/dev/null || echo "") +if [[ "$RUN_EXIT_CODE" -eq 0 ]] && [[ ! -f "$RUN_MARKER" ]] && [[ -z "$AC7_SYS_MSG" ]]; then + pass "AC-7: no active loop takes original exit-0 path without systemMessage" +else + fail "AC-7: no active loop takes original exit-0 path without systemMessage" \ + "exit 0, no Codex marker, no systemMessage" \ + "exit $RUN_EXIT_CODE, marker=$(test -f "$RUN_MARKER" && echo present || echo missing), systemMessage='$AC7_SYS_MSG'; output: $RUN_OUTPUT" +fi + +# ---------------- AC-8 ---------------- +echo "Test AC-8: Finalize phase + pending bg -> exit 0 + systemMessage" +AC8_REPO="$TEST_DIR/ac8" +AC8_LOOP=$(create_full_fixture "$AC8_REPO" true) +AC8_STATE="$AC8_LOOP/finalize-state.md" +AC8_TRANSCRIPT="$TRANSCRIPTS_DIR/ac8.jsonl" +AC8_LAUNCH=$(emit_tool_use_assistant "toolu_F" "Agent" ',"description":"x","prompt":"x"') +AC8_RESULT=$(emit_async_agent_launch_result "toolu_F" "agent_pending_F") +write_transcript "$AC8_TRANSCRIPT" "$AC8_LAUNCH" "$AC8_RESULT" +AC8_INPUT=$(jq -c -n --arg tp "$AC8_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC8_REPO" "$AC8_INPUT" +assert_systemmessage_only \ + "AC-8: finalize phase with pending bg task -> exit 0 + systemMessage" \ + "$AC8_REPO" "$AC8_STATE" "1 background task" + +# ---------------- AC-9 ---------------- +echo "Test AC-9: rlcr-stop-gate.sh forwards transcript_path to hook" +AC9_REPO="$TEST_DIR/ac9" +create_full_fixture "$AC9_REPO" > /dev/null +AC9_TRANSCRIPT="$TRANSCRIPTS_DIR/ac9.jsonl" +AC9_LAUNCH=$(emit_tool_use_assistant "toolu_G" "Agent" ',"description":"x","prompt":"x"') +AC9_RESULT=$(emit_async_agent_launch_result "toolu_G" "agent_pending_G") +write_transcript "$AC9_TRANSCRIPT" "$AC9_LAUNCH" "$AC9_RESULT" + +AC9_OUT="$AC9_REPO/gate-out.txt" +set +e +( + cd "$AC9_REPO" + "$GATE_SCRIPT" --transcript-path "$AC9_TRANSCRIPT" +) > "$AC9_OUT" 2>&1 +AC9_EXIT=$? +set -e + +if [[ "$AC9_EXIT" -eq 0 ]] && grep -q "^ALLOW:" "$AC9_OUT"; then + pass "AC-9: rlcr-stop-gate.sh exits 0 with ALLOW when bg tasks are pending" +else + AC9_BODY=$(cat "$AC9_OUT" 2>/dev/null || true) + fail "AC-9: rlcr-stop-gate.sh exits 0 with ALLOW when bg tasks are pending" \ + "exit 0 and output containing ALLOW:" \ + "exit $AC9_EXIT; output: $AC9_BODY" +fi + +print_test_summary "Stop Hook Background-Task Allow Test Summary" +exit $? From 3adf8ef8cdbcc6c2d055ebd4e33909fac38b4c6b Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 13:45:48 -0700 Subject: [PATCH 02/13] Expand '~' in transcript_path before background-task file check Round 0 shipped the natural-stop short-circuit using a literal `-f` check on the transcript path, which silently failed for the "~/.claude/projects//.jsonl" form Claude Code actually publishes. A real session landed on the fall-through path and still ran the full Codex review, defeating the feature. Add a small `expand_leading_tilde` helper in loop-common.sh (no eval, handles bare "~" and "~/..." only, leaves everything else verbatim) and apply it in both extract_transcript_path and list_pending_background_task_ids. The stop hook and the rlcr-stop-gate wrapper inherit the fix through the shared helpers without any call-site edits. Regression coverage in tests/test-stop-hook-bg-allow.sh: AC-10 hook input with transcript_path="~/...\" under $HOME still triggers the short-circuit (exit 0 + systemMessage). AC-10b direct list_pending_background_task_ids on the same tilde form returns the pending id, so a future regression in the helper cannot be masked by additional normalization in the hook itself. Under-$HOME fixture dir is cleaned alongside $TEST_DIR via an extended EXIT trap. Full suite: 1692 passed, 0 failed (+2 vs Round 0). Branch still targets dev; version stays at 1.16.0. --- hooks/lib/loop-common.sh | 28 +++++++++++++++++-- tests/test-stop-hook-bg-allow.sh | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index a2119511..3b210793 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -242,12 +242,29 @@ extract_session_id() { printf '%s' "$input" | jq -r '.session_id // empty' 2>/dev/null || echo "" } -# Extract transcript_path from hook JSON input +# Expand a leading "~" or "~/" in a path to "$HOME" without using eval. +# Only the bare "~" and "~/..." forms are expanded; "~user/..." and every +# other input (absolute path, relative path, empty string) is returned verbatim. +# +# Usage: expand_leading_tilde "$path" +# Prints the normalized path to stdout. +expand_leading_tilde() { + local path="$1" + case "$path" in + '~') printf '%s' "${HOME:-}" ;; + '~/'*) printf '%s/%s' "${HOME:-}" "${path#'~/'}" ;; + *) printf '%s' "$path" ;; + esac +} + +# Extract transcript_path from hook JSON input and expand any leading tilde. # Usage: extract_transcript_path "$json_input" -# Outputs the transcript_path to stdout, or empty string if not available +# Outputs the transcript_path to stdout, or empty string if not available. extract_transcript_path() { local input="$1" - printf '%s' "$input" | jq -r '.transcript_path // empty' 2>/dev/null || echo "" + local raw + raw=$(printf '%s' "$input" | jq -r '.transcript_path // empty' 2>/dev/null || echo "") + expand_leading_tilde "$raw" } # Enumerate background-task ids that have been launched but not yet marked @@ -276,6 +293,11 @@ extract_transcript_path() { list_pending_background_task_ids() { local transcript_path="$1" + # Normalize a leading tilde so direct callers (tests, ad-hoc scripts) + # work correctly even when transcript_path was not routed through + # extract_transcript_path. + transcript_path=$(expand_leading_tilde "$transcript_path") + if [[ -z "$transcript_path" ]] || [[ ! -f "$transcript_path" ]]; then return 1 fi diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index c1e89853..a8025f75 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -33,6 +33,16 @@ GATE_SCRIPT="$PROJECT_ROOT/scripts/rlcr-stop-gate.sh" setup_test_dir +# AC-10 needs a transcript living under $HOME so the hook input can use +# the "~/..." form. Create the fixture dir in the AC-10 block and extend +# the EXIT trap set by setup_test_dir to clean both directories on shutdown. +HOME_FIXTURE_DIR="" +cleanup_all() { + rm -rf "$TEST_DIR" + [[ -n "$HOME_FIXTURE_DIR" ]] && rm -rf "$HOME_FIXTURE_DIR" +} +trap cleanup_all EXIT + export XDG_CACHE_HOME="$TEST_DIR/.cache" mkdir -p "$XDG_CACHE_HOME" @@ -463,5 +473,43 @@ else "exit $AC9_EXIT; output: $AC9_BODY" fi +# ---------------- AC-10 ---------------- +# Regression: real sessions pass transcript_path as "~/.claude/projects/...". +# Without tilde expansion the file check `[[ -f "~/..." ]]` is always false, +# so the short-circuit silently misses pending background tasks. +echo "Test AC-10: '~/...' transcript path still triggers short-circuit" +AC10_REPO="$TEST_DIR/ac10" +AC10_LOOP=$(create_full_fixture "$AC10_REPO") +AC10_STATE="$AC10_LOOP/state.md" + +HOME_FIXTURE_DIR=$(mktemp -d "$HOME/.humanize-bg-allow-test-XXXXXX") +AC10_TRANSCRIPT="$HOME_FIXTURE_DIR/ac10.jsonl" +AC10_LAUNCH=$(emit_tool_use_assistant "toolu_H" "Agent" ',"description":"x","prompt":"x"') +AC10_RESULT=$(emit_async_agent_launch_result "toolu_H" "agent_pending_H") +write_transcript "$AC10_TRANSCRIPT" "$AC10_LAUNCH" "$AC10_RESULT" + +# Build the tilde-form string literally. Do NOT let the shell expand "~". +AC10_TILDE_PATH="~/${AC10_TRANSCRIPT#$HOME/}" +AC10_INPUT=$(jq -c -n --arg tp "$AC10_TILDE_PATH" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC10_REPO" "$AC10_INPUT" +assert_systemmessage_only \ + "AC-10: '~/'-prefixed transcript_path is expanded and short-circuits on pending bg" \ + "$AC10_REPO" "$AC10_STATE" "1 background task" + +# Also prove the helper works directly against a "~/..." argument. +# This avoids masking a helper regression behind the hook's own expansion. +AC10_HELPER_OUT=$( + cd "$AC10_REPO" + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + list_pending_background_task_ids "$AC10_TILDE_PATH" 2>/dev/null | sort -u +) +if printf '%s\n' "$AC10_HELPER_OUT" | grep -qx 'agent_pending_H'; then + pass "AC-10b: list_pending_background_task_ids expands '~/...' directly" +else + fail "AC-10b: list_pending_background_task_ids expands '~/...' directly" \ + "output containing 'agent_pending_H'" "$AC10_HELPER_OUT" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 38691dd2c860472c637519253fc82abe054a7505 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 13:55:27 -0700 Subject: [PATCH 03/13] Make tilde-path regressions portable and cover the wrapper too Two test-harness gaps flagged in the previous Codex review: 1. The tilde-path fixture was created with mktemp -d "$HOME/...", which aborts on sandboxed or read-only-HOME environments and forces callers to override HOME externally. 2. AC-9 with the literal "~/..." --transcript-path form was named as a target for the round but never made it into automation; the checked-in wrapper coverage still used an absolute path. Fix both inside tests/test-stop-hook-bg-allow.sh only -- no production code changes. The helper, hook, and wrapper already handle the tilde form correctly; the missing piece was portable, checked-in proof. - Introduce FAKE_HOME="$TEST_DIR/fake-home" rooted inside the existing test temp dir, so cleanup rides on the setup_test_dir EXIT trap. The previous cleanup_all / HOME_FIXTURE_DIR extension is removed. - Extend run_stop_hook_with_input with an optional 3rd argument that exports HOME inside the hook subshell when non-empty. Absent, behavior is verbatim. - Refactor AC-10 and AC-10b to point at "$FAKE_HOME/session-data/ac10.jsonl", and pass the tilde-form literal "~/session-data/ac10.jsonl" verbatim. The previous dynamic derivation "~/${AC10_TRANSCRIPT#$HOME/}" is gone. - Add AC-10c: exercises scripts/rlcr-stop-gate.sh with --transcript-path "~/..." under HOME=$FAKE_HOME and asserts exit 0 + "^ALLOW:" + "background task". Validation: - bash tests/test-stop-hook-bg-allow.sh -> 14 passed, 0 failed (baseline 13; +1 for AC-10c). - bash tests/run-all-tests.sh -> 1693 passed, 0 failed (baseline 1692; +1). - Portability: HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 14 passed, 0 failed. Branch still targets dev; version stays at 1.16.0. --- tests/test-stop-hook-bg-allow.sh | 72 +++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index a8025f75..408400a6 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -33,19 +33,18 @@ GATE_SCRIPT="$PROJECT_ROOT/scripts/rlcr-stop-gate.sh" setup_test_dir -# AC-10 needs a transcript living under $HOME so the hook input can use -# the "~/..." form. Create the fixture dir in the AC-10 block and extend -# the EXIT trap set by setup_test_dir to clean both directories on shutdown. -HOME_FIXTURE_DIR="" -cleanup_all() { - rm -rf "$TEST_DIR" - [[ -n "$HOME_FIXTURE_DIR" ]] && rm -rf "$HOME_FIXTURE_DIR" -} -trap cleanup_all EXIT - export XDG_CACHE_HOME="$TEST_DIR/.cache" mkdir -p "$XDG_CACHE_HOME" +# Fake HOME rooted inside $TEST_DIR so the tilde-path regressions (AC-10, +# AC-10b, AC-10c) do not write into the real user home. The hook, helper, +# and wrapper invocations that need tilde expansion run with HOME set to +# this directory; every other invocation keeps the real HOME. Cleanup is +# covered by the setup_test_dir EXIT trap because FAKE_HOME is under +# $TEST_DIR. +FAKE_HOME="$TEST_DIR/fake-home" +mkdir -p "$FAKE_HOME" + # ---------------------------------------------------------------------- # Mock codex CLI: records an invocation marker and prints canned feedback. # ---------------------------------------------------------------------- @@ -226,11 +225,14 @@ write_transcript() { } # ---------------------------------------------------------------------- -# Invoke the stop hook with a crafted hook input JSON. +# Invoke the stop hook with a crafted hook input JSON. The optional third +# argument overrides HOME for the hook invocation only, so tilde-path +# regressions can point at a fake HOME rooted under $TEST_DIR without +# leaking into the real user home. # Sets RUN_EXIT_CODE, RUN_OUTPUT, RUN_MARKER. # ---------------------------------------------------------------------- run_stop_hook_with_input() { - local repo_dir="$1" hook_input_json="$2" + local repo_dir="$1" hook_input_json="$2" home_override="${3:-}" RUN_MARKER="$repo_dir/codex-called.marker" rm -f "$RUN_MARKER" @@ -238,6 +240,7 @@ run_stop_hook_with_input() { set +e RUN_OUTPUT=$( cd "$repo_dir" + [[ -n "$home_override" ]] && export HOME="$home_override" CLAUDE_PROJECT_DIR="$repo_dir" \ MOCK_CODEX_MARKER="$RUN_MARKER" \ MOCK_CODEX_OUTPUT="Mock review feedback" \ @@ -473,33 +476,40 @@ else "exit $AC9_EXIT; output: $AC9_BODY" fi -# ---------------- AC-10 ---------------- +# ---------------- AC-10 / AC-10b / AC-10c ---------------- # Regression: real sessions pass transcript_path as "~/.claude/projects/...". # Without tilde expansion the file check `[[ -f "~/..." ]]` is always false, # so the short-circuit silently misses pending background tasks. +# +# The fixture lives under a fake HOME rooted inside $TEST_DIR so the tests +# remain portable on sandboxed or read-only-HOME environments. Only the +# specific hook / helper / wrapper invocations that need tilde expansion +# run with HOME=$FAKE_HOME; the rest of the suite keeps the real HOME. echo "Test AC-10: '~/...' transcript path still triggers short-circuit" AC10_REPO="$TEST_DIR/ac10" AC10_LOOP=$(create_full_fixture "$AC10_REPO") AC10_STATE="$AC10_LOOP/state.md" -HOME_FIXTURE_DIR=$(mktemp -d "$HOME/.humanize-bg-allow-test-XXXXXX") -AC10_TRANSCRIPT="$HOME_FIXTURE_DIR/ac10.jsonl" +mkdir -p "$FAKE_HOME/session-data" +AC10_TRANSCRIPT="$FAKE_HOME/session-data/ac10.jsonl" AC10_LAUNCH=$(emit_tool_use_assistant "toolu_H" "Agent" ',"description":"x","prompt":"x"') AC10_RESULT=$(emit_async_agent_launch_result "toolu_H" "agent_pending_H") write_transcript "$AC10_TRANSCRIPT" "$AC10_LAUNCH" "$AC10_RESULT" # Build the tilde-form string literally. Do NOT let the shell expand "~". -AC10_TILDE_PATH="~/${AC10_TRANSCRIPT#$HOME/}" +AC10_TILDE_PATH="~/session-data/ac10.jsonl" AC10_INPUT=$(jq -c -n --arg tp "$AC10_TILDE_PATH" '{transcript_path:$tp}') -run_stop_hook_with_input "$AC10_REPO" "$AC10_INPUT" +run_stop_hook_with_input "$AC10_REPO" "$AC10_INPUT" "$FAKE_HOME" assert_systemmessage_only \ "AC-10: '~/'-prefixed transcript_path is expanded and short-circuits on pending bg" \ "$AC10_REPO" "$AC10_STATE" "1 background task" -# Also prove the helper works directly against a "~/..." argument. -# This avoids masking a helper regression behind the hook's own expansion. +# Also prove the helper works directly against a "~/..." argument under a +# fake HOME. Avoids masking a helper regression behind the hook's own +# normalization. AC10_HELPER_OUT=$( cd "$AC10_REPO" + HOME="$FAKE_HOME" # shellcheck source=/dev/null source "$PROJECT_ROOT/hooks/lib/loop-common.sh" list_pending_background_task_ids "$AC10_TILDE_PATH" 2>/dev/null | sort -u @@ -511,5 +521,29 @@ else "output containing 'agent_pending_H'" "$AC10_HELPER_OUT" fi +# Verify the gate wrapper path with a tilde-form --transcript-path also +# reaches the short-circuit. AC-9 uses an absolute transcript path; this +# covers the same code path with a "~/..." form. +echo "Test AC-10c: rlcr-stop-gate.sh with '~/...' --transcript-path -> ALLOW" +AC10C_OUT="$TEST_DIR/ac10c-out.txt" +set +e +( + cd "$AC10_REPO" + HOME="$FAKE_HOME" "$GATE_SCRIPT" --transcript-path "$AC10_TILDE_PATH" +) > "$AC10C_OUT" 2>&1 +AC10C_EXIT=$? +set -e + +if [[ "$AC10C_EXIT" -eq 0 ]] \ + && grep -q "^ALLOW:" "$AC10C_OUT" \ + && grep -q "background task" "$AC10C_OUT"; then + pass "AC-10c: rlcr-stop-gate.sh expands '~/...' and emits ALLOW with systemMessage" +else + AC10C_BODY=$(cat "$AC10C_OUT" 2>/dev/null || true) + fail "AC-10c: rlcr-stop-gate.sh expands '~/...' and emits ALLOW with systemMessage" \ + "exit 0 + output containing ALLOW: and 'background task'" \ + "exit $AC10C_EXIT; output: $AC10C_BODY" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 6a1d931643975e2819abec200f6f362173636da7 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 14:13:38 -0700 Subject: [PATCH 04/13] Prevent RLCR loop orphaning when session dies mid-background Codex review P2 on the background-task short-circuit: allowing the natural stop at the early-exit site returned control to the user while state.md was still bound to the current session_id. If the user closed Claude before the background-task completion notification arrived, find_active_loop in any later session would reject the loop (stored session_id did not match), so the loop was stranded and recovery required manual cancellation. Resolve without touching state.md by introducing a narrow cross-session adoption signal: - Early-exit block now also runs `: > "$LOOP_DIR/bg-pending.marker"` before emitting the systemMessage JSON. Failure is tolerated so the short-circuit never blocks on a flaky filesystem. - find_active_loop's session-filter branch gains a second acceptance step: on a stored-vs-filter session_id mismatch, if the dir has `bg-pending.marker` AND an active state file, it is still accepted. Terminal loops with a stale marker are ignored. Three new regression cases in tests/test-stop-hook-bg-allow.sh: AC-11 session_id mismatch + marker + pending bg -> short-circuit fires; state.md stays byte-identical. AC-11b session_id mismatch + no marker -> hook takes the existing "no active loop" exit-0 path; confirms the marker is the only cross-session adoption signal. AC-11c same-session short-circuit really writes the marker, so AC-11 is grounded in real hook behavior rather than a synthetic setup. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 17 passed, 0 failed - bash tests/run-all-tests.sh -> 1696 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 17 passed, 0 failed No systemMessage wording change. No state.md mutation. Branch still targets dev; version stays at 1.16.0. --- hooks/lib/loop-common.sh | 12 +++ hooks/loop-codex-stop-hook.sh | 3 + tests/test-stop-hook-bg-allow.sh | 127 +++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 3b210793..f89c2eed 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -477,6 +477,18 @@ find_active_loop() { echo "" return fi + + # Session mismatch: adopt the loop only when it was explicitly parked + # for a background task (stop hook writes bg-pending.marker there). + if [[ -f "$trimmed_dir/bg-pending.marker" ]]; then + local active_state_bg + active_state_bg=$(resolve_active_state_file "$trimmed_dir") + if [[ -n "$active_state_bg" ]]; then + echo "$trimmed_dir" + return + fi + # Marker on a terminal loop is stale; ignore it and keep walking. + fi done < <(ls -1d "$loop_base_dir"/*/ 2>/dev/null | sort -r) echo "" diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index 5b57a3cb..ec8444e5 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -87,6 +87,9 @@ fi HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH") + # Mark the loop as parked; allows a fresh session to adopt it if this + # Claude window is closed before the background task finishes. + : > "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true jq -n --arg count "$PENDING_BG_COUNT" \ '{systemMessage: ("RLCR loop active. " + $count + " background task(s) still running - stop allowed naturally; loop has NOT terminated and will resume on completion.")}' exit 0 diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 408400a6..b9810246 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -545,5 +545,132 @@ else "exit $AC10C_EXIT; output: $AC10C_BODY" fi +# ---------------- AC-11 / AC-11b ---------------- +# Orphan prevention: when the short-circuit parks a loop waiting for a +# background task and the user closes that Claude session, a fresh +# session must still be able to pick up the loop. The short-circuit +# writes `bg-pending.marker` into the loop dir; find_active_loop +# accepts a stored-vs-filter session_id mismatch iff the marker is +# present. Without this cross-session adoption path, state.md would +# be stranded with the dead session_id and require manual cancel. +echo "Test AC-11: cross-session bg-pending.marker allows pickup" +AC11_REPO="$TEST_DIR/ac11" +AC11_LOOP=$(create_full_fixture "$AC11_REPO") +AC11_STATE="$AC11_LOOP/state.md" + +# Override state.md with an explicit stored session_id so find_active_loop +# sees a real mismatch when we later pass a different session_id. +AC11_BRANCH=$(git -C "$AC11_REPO" rev-parse --abbrev-ref HEAD) +AC11_BASE_COMMIT=$(git -C "$AC11_REPO" rev-parse HEAD) +cat > "$AC11_STATE" < "$AC11_LOOP/bg-pending.marker" + +AC11_TRANSCRIPT="$TRANSCRIPTS_DIR/ac11.jsonl" +AC11_LAUNCH=$(emit_tool_use_assistant "toolu_I" "Agent" ',"description":"x","prompt":"x"') +AC11_RESULT=$(emit_async_agent_launch_result "toolu_I" "agent_pending_I") +write_transcript "$AC11_TRANSCRIPT" "$AC11_LAUNCH" "$AC11_RESULT" + +AC11_INPUT=$(jq -c -n --arg tp "$AC11_TRANSCRIPT" \ + '{transcript_path:$tp, session_id:"session_beta"}') +run_stop_hook_with_input "$AC11_REPO" "$AC11_INPUT" +assert_systemmessage_only \ + "AC-11: cross-session bg-pending.marker allows pickup and short-circuit" \ + "$AC11_REPO" "$AC11_STATE" "1 background task" + +# Negative counterpart: same session mismatch but NO marker must still +# reject the loop (preserving the existing session-bound isolation when +# the loop was not explicitly parked). +echo "Test AC-11b: cross-session without marker is still rejected" +AC11B_REPO="$TEST_DIR/ac11b" +AC11B_LOOP=$(create_full_fixture "$AC11B_REPO") +AC11B_STATE="$AC11B_LOOP/state.md" +AC11B_BRANCH=$(git -C "$AC11B_REPO" rev-parse --abbrev-ref HEAD) +AC11B_BASE_COMMIT=$(git -C "$AC11B_REPO" rev-parse HEAD) +cat > "$AC11B_STATE" </dev/null || echo "") +if [[ "$RUN_EXIT_CODE" -eq 0 ]] && [[ ! -f "$RUN_MARKER" ]] && [[ -z "$AC11B_SYS_MSG" ]]; then + pass "AC-11b: cross-session without marker keeps existing isolation (no adoption)" +else + fail "AC-11b: cross-session without marker keeps existing isolation (no adoption)" \ + "exit 0, no Codex marker, no systemMessage" \ + "exit $RUN_EXIT_CODE, marker=$(test -f "$RUN_MARKER" && echo present || echo missing), systemMessage='$AC11B_SYS_MSG'; output: $RUN_OUTPUT" +fi + +# AC-11c: short-circuit should actually write bg-pending.marker so the +# adoption path in AC-11 is reachable from real usage (not only from +# synthetic test setup). +echo "Test AC-11c: short-circuit writes bg-pending.marker" +AC11C_REPO="$TEST_DIR/ac11c" +AC11C_LOOP=$(create_full_fixture "$AC11C_REPO") +AC11C_MARKER="$AC11C_LOOP/bg-pending.marker" +[[ -e "$AC11C_MARKER" ]] && rm -f "$AC11C_MARKER" + +AC11C_TRANSCRIPT="$TRANSCRIPTS_DIR/ac11c.jsonl" +AC11C_LAUNCH=$(emit_tool_use_assistant "toolu_K" "Agent" ',"description":"x","prompt":"x"') +AC11C_RESULT=$(emit_async_agent_launch_result "toolu_K" "agent_pending_K") +write_transcript "$AC11C_TRANSCRIPT" "$AC11C_LAUNCH" "$AC11C_RESULT" + +AC11C_INPUT=$(jq -c -n --arg tp "$AC11C_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC11C_REPO" "$AC11C_INPUT" +if [[ "$RUN_EXIT_CODE" -eq 0 ]] && [[ -f "$AC11C_MARKER" ]]; then + pass "AC-11c: short-circuit path writes bg-pending.marker into loop dir" +else + fail "AC-11c: short-circuit path writes bg-pending.marker into loop dir" \ + "exit 0 and bg-pending.marker present" \ + "exit $RUN_EXIT_CODE, marker=$(test -f "$AC11C_MARKER" && echo present || echo missing); output: $RUN_OUTPUT" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 69587a777fb5feb6c159af3d0d610321e27582ac Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 14:25:45 -0700 Subject: [PATCH 05/13] Prefer own-session match and clean marker on resume Codex review flagged two regressions from the previous round's bg-pending.marker change: [P1] find_active_loop could return the newest loop dir that had a marker before the scan reached an older dir whose stored session_id actually matched the caller. With multiple active RLCR loops in one repo that breaks session isolation and lets a stop hook attach to the wrong conversation. [P2] The short-circuit wrote bg-pending.marker but nothing cleared it when the next stop saw no pending background task. A later stop from a different session_id would keep being adopted through the stale marker long after the bg had resolved. Both fixes are in the files already touched by this feature: * find_active_loop's session-filter branch now makes exact stored-vs-filter match win over marker fallback. Marker candidates are recorded while the scan walks newest-to-oldest and only returned after the whole listing fails to yield an exact match. Zombie-loop protection still wins for the caller's own session. This preserves Round 3 orphan recovery while restoring isolation across concurrent sessions. * After has_pending_background_tasks returns false, the stop hook now handles the marker: if HOOK_SESSION_ID differs from the stored session_id in the active state file, it rewrites that line with portable sed -i.bak; then the marker is removed unconditionally. Rewrite failure is logged but non-fatal. Regressions in tests/test-stop-hook-bg-allow.sh: AC-12 find_active_loop returns the older exact-match dir over a newer foreign-session dir that has a marker. AC-12b find_active_loop does not touch a foreign session's marker. AC-13 Same-session resume with a stale marker + empty bg transcript clears the marker. AC-13b Same-session resume leaves state.md session_id unchanged. AC-14 Cross-session resume clears the marker. AC-14b Cross-session resume rewrites state.md session_id to the caller's session. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 23 passed, 0 failed - bash tests/run-all-tests.sh -> 1702 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 23 passed, 0 failed systemMessage wording unchanged. Version stays at 1.16.0. --- hooks/lib/loop-common.sh | 44 ++++++--- hooks/loop-codex-stop-hook.sh | 22 +++++ tests/test-stop-hook-bg-allow.sh | 156 +++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 14 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index f89c2eed..62af2c12 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -448,9 +448,18 @@ find_active_loop() { return fi - # Session filter: iterate newest-to-oldest, find the first dir belonging - # to this session (any state file), then check if it is still active. + # Session filter: iterate newest-to-oldest. + # + # The caller's own (exact stored session_id) match takes precedence over + # any marker-based adoption: with multiple active RLCR loops in the same + # repo, a newer dir parked by a different session must not be returned + # before an older dir that actually belongs to the caller. Marker + # candidates are recorded during the scan and only used as a fallback + # when no exact match is found anywhere. Zombie-loop protection + # (terminal newest for this session returns empty) still wins over + # marker fallback. local dir + local marker_candidate="" while IFS= read -r dir; do [[ -z "$dir" ]] && continue local trimmed_dir="${dir%/}" @@ -464,9 +473,9 @@ find_active_loop() { local stored_session_id stored_session_id=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/'"${FIELD_SESSION_ID}"': *//; p; } }' "$any_state" 2>/dev/null | tr -d ' ') - # Empty stored session_id matches any session (backward compat) + # Empty stored session_id matches any session (backward compat). if [[ -z "$stored_session_id" ]] || [[ "$stored_session_id" == "$filter_session_id" ]]; then - # This is the newest dir for this session -- only return if active + # Newest dir for this session -- only return if active. local active_state active_state=$(resolve_active_state_file "$trimmed_dir") if [[ -n "$active_state" ]]; then @@ -474,27 +483,34 @@ find_active_loop() { return fi # Session's newest loop is in terminal state; do not fall through + # to marker-based adoption either. echo "" return fi - # Session mismatch: adopt the loop only when it was explicitly parked - # for a background task (stop hook writes bg-pending.marker there). - if [[ -f "$trimmed_dir/bg-pending.marker" ]]; then - local active_state_bg - active_state_bg=$(resolve_active_state_file "$trimmed_dir") - if [[ -n "$active_state_bg" ]]; then - echo "$trimmed_dir" - return + # Session mismatch: stash the newest eligible marker candidate but + # keep walking in case an older dir is the caller's own session. + if [[ -z "$marker_candidate" ]] && [[ -f "$trimmed_dir/bg-pending.marker" ]]; then + local candidate_state + candidate_state=$(resolve_active_state_file "$trimmed_dir") + if [[ -n "$candidate_state" ]]; then + marker_candidate="$trimmed_dir" fi - # Marker on a terminal loop is stale; ignore it and keep walking. + # Marker on a terminal loop is stale; leave it alone. fi done < <(ls -1d "$loop_base_dir"/*/ 2>/dev/null | sort -r) + # No exact session match. Fall back to marker-based adoption if any -- + # this is the cross-session recovery path when a previous session parked + # the loop and then died before the background-task completion arrived. + if [[ -n "$marker_candidate" ]]; then + echo "$marker_candidate" + return + fi + echo "" } - # Extract current round number from state.md # Outputs the round number to stdout, defaults to 0 # Note: For full state parsing, use parse_state_file() instead diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index ec8444e5..d018e598 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -95,6 +95,28 @@ if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then exit 0 fi +# No pending background task. If a stale bg-pending.marker is lingering +# here, this stop is the resume point. When find_active_loop picked this +# dir up through the marker-fallback path (stored session_id differs from +# the current one), rewrite the stored session_id so future same-session +# stops use the exact-match path, then remove the marker so any later +# hook trigger from an unrelated session is rejected rather than adopted. +if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then + ADOPT_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") + if [[ -n "$ADOPT_STATE_FILE" ]] && [[ -n "$HOOK_SESSION_ID" ]]; then + STORED_SID_ADOPT=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$ADOPT_STATE_FILE" 2>/dev/null | tr -d ' ') + if [[ -n "$STORED_SID_ADOPT" ]] && [[ "$STORED_SID_ADOPT" != "$HOOK_SESSION_ID" ]]; then + # Portable in-place rewrite. Failure is logged but non-fatal: + # worst case the next stop re-adopts via the marker pathway. + if ! sed -i.bak -E "s|^(${FIELD_SESSION_ID}:).*$|\\1 $HOOK_SESSION_ID|" "$ADOPT_STATE_FILE" 2>/dev/null; then + echo "Warning: failed to adopt session_id in $ADOPT_STATE_FILE" >&2 + fi + rm -f "${ADOPT_STATE_FILE}.bak" 2>/dev/null || true + fi + fi + rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true +fi + # ======================================== # Detect Loop Phase: Normal or Finalize # ======================================== diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index b9810246..3dda7356 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -672,5 +672,161 @@ else "exit $RUN_EXIT_CODE, marker=$(test -f "$AC11C_MARKER" && echo present || echo missing); output: $RUN_OUTPUT" fi +# ---------------- AC-12 ---------------- +# Session isolation under multiple concurrent RLCR loops: when the caller's +# own exact-match dir exists in the listing, find_active_loop must return +# it even if a newer sibling dir (belonging to another session) also has a +# bg-pending.marker. The marker fallback is only for orphan recovery when +# no exact match exists. +echo "Test AC-12: find_active_loop prefers exact session match over marker" +AC12_BASE="$TEST_DIR/ac12-loops" +mkdir -p "$AC12_BASE/2026-03-02_00-00-00" +mkdir -p "$AC12_BASE/2026-03-01_00-00-00" + +cat > "$AC12_BASE/2026-03-02_00-00-00/state.md" <<'EOF_AC12_NEWER' +--- +current_round: 0 +max_iterations: 42 +codex_model: gpt-5.4 +codex_effort: high +session_id: session_foreign +--- +EOF_AC12_NEWER +: > "$AC12_BASE/2026-03-02_00-00-00/bg-pending.marker" + +cat > "$AC12_BASE/2026-03-01_00-00-00/state.md" <<'EOF_AC12_OLDER' +--- +current_round: 0 +max_iterations: 42 +codex_model: gpt-5.4 +codex_effort: high +session_id: session_home +--- +EOF_AC12_OLDER + +AC12_RESULT=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + find_active_loop "$AC12_BASE" "session_home" +) +if [[ "$AC12_RESULT" == "$AC12_BASE/2026-03-01_00-00-00" ]]; then + pass "AC-12: find_active_loop returns older exact-match dir over newer marker dir" +else + fail "AC-12: find_active_loop returns older exact-match dir over newer marker dir" \ + "$AC12_BASE/2026-03-01_00-00-00" "$AC12_RESULT" +fi + +if [[ -f "$AC12_BASE/2026-03-02_00-00-00/bg-pending.marker" ]]; then + pass "AC-12b: foreign session's marker untouched by find_active_loop scan" +else + fail "AC-12b: foreign session's marker untouched by find_active_loop scan" \ + "newer dir marker still present" "marker was removed" +fi + +# ---------------- AC-13 ---------------- +# Same-session resume after background completion: a stale marker from the +# previous short-circuit must be cleaned up on the next stop where no bg is +# pending. State.md session_id stays put because it already matches. +echo "Test AC-13: same-session resume removes stale bg-pending.marker" +AC13_REPO="$TEST_DIR/ac13" +AC13_LOOP=$(create_full_fixture "$AC13_REPO") +AC13_STATE="$AC13_LOOP/state.md" +AC13_BRANCH=$(git -C "$AC13_REPO" rev-parse --abbrev-ref HEAD) +AC13_BASE_COMMIT=$(git -C "$AC13_REPO" rev-parse HEAD) +cat > "$AC13_STATE" < "$AC13_LOOP/bg-pending.marker" + +AC13_TRANSCRIPT="$TRANSCRIPTS_DIR/ac13.jsonl" +write_transcript "$AC13_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' +AC13_INPUT=$(jq -c -n --arg tp "$AC13_TRANSCRIPT" \ + '{transcript_path:$tp, session_id:"session_home"}') +run_stop_hook_with_input "$AC13_REPO" "$AC13_INPUT" + +if [[ ! -f "$AC13_LOOP/bg-pending.marker" ]]; then + pass "AC-13: marker removed on non-short-circuit resume (same session)" +else + fail "AC-13: marker removed on non-short-circuit resume (same session)" \ + "marker absent" "marker still present" +fi + +if grep -q "^session_id: session_home$" "$AC13_STATE"; then + pass "AC-13b: same-session resume leaves state.md session_id unchanged" +else + fail "AC-13b: same-session resume leaves state.md session_id unchanged" \ + "session_id: session_home" "$(grep '^session_id:' "$AC13_STATE" || echo '(missing)')" +fi + +# ---------------- AC-14 ---------------- +# Cross-session resume: a different session walks in, finds the loop through +# the marker fallback, and the non-short-circuit path must rewrite the +# stored session_id so future same-session stops use the exact-match path +# instead of re-adopting via a stale marker. +echo "Test AC-14: cross-session resume rewrites session_id and removes marker" +AC14_REPO="$TEST_DIR/ac14" +AC14_LOOP=$(create_full_fixture "$AC14_REPO") +AC14_STATE="$AC14_LOOP/state.md" +AC14_BRANCH=$(git -C "$AC14_REPO" rev-parse --abbrev-ref HEAD) +AC14_BASE_COMMIT=$(git -C "$AC14_REPO" rev-parse HEAD) +cat > "$AC14_STATE" < "$AC14_LOOP/bg-pending.marker" + +AC14_TRANSCRIPT="$TRANSCRIPTS_DIR/ac14.jsonl" +write_transcript "$AC14_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' +AC14_INPUT=$(jq -c -n --arg tp "$AC14_TRANSCRIPT" \ + '{transcript_path:$tp, session_id:"session_home"}') +run_stop_hook_with_input "$AC14_REPO" "$AC14_INPUT" + +if [[ ! -f "$AC14_LOOP/bg-pending.marker" ]]; then + pass "AC-14: marker removed after cross-session resume" +else + fail "AC-14: marker removed after cross-session resume" \ + "marker absent" "marker still present" +fi + +if grep -q "^session_id: session_home$" "$AC14_STATE"; then + pass "AC-14b: state.md session_id rewritten to the current session on adoption" +else + fail "AC-14b: state.md session_id rewritten to the current session on adoption" \ + "session_id: session_home" "$(grep '^session_id:' "$AC14_STATE" || echo '(missing)')" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 05359199a442d3db994a3f4bcb9783190ba7c515 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 14:40:49 -0700 Subject: [PATCH 06/13] Recognise SDK task_notification completions and keep recovery marker Two Codex review findings on the previous round: [P1] The completion parser in list_pending_background_task_ids only understood the legacy queue-operation XML blocks. Current Claude Code transcripts emit background-task completions as SDKTaskNotificationMessage records: type: "system", subtype: "task_notification", task_id: ... Without SDK-format recognition, launched ids stayed pending forever and the short-circuit would fire on every stop. [P2] The non-short-circuit path cleared bg-pending.marker unconditionally whenever has_pending_background_tasks returned false. That helper is fail-closed, so it also returned false when transcript_path was missing or unreadable (e.g. rlcr-stop-gate.sh without --transcript-path). In that case the cleanup still deleted the marker and rewrote the stored session_id, breaking cross-session recovery exactly where transcript inspection is unavailable. Fixes, confined to the files already touched by this feature: * loop-common.sh: union the completion set from both SDK (type:system subtype:task_notification -> .task_id) and legacy (queue-operation XML) sources. Wrap the legacy branch's grep in `{ grep -oE ... || true; }` so its "no match -> exit 1" cannot combine with set -o pipefail to invalidate the SDK side of the union through the `|| completed=""` fallback. * loop-codex-stop-hook.sh: gate the non-short-circuit marker cleanup on `HOOK_TRANSCRIPT_PATH` being a readable regular file. When transcript inspection is unavailable, leave the marker AND the stored session_id untouched so cross-session recovery stays reachable. Regressions added in tests/test-stop-hook-bg-allow.sh: AC-15 helper treats an SDK task_notification as terminal. AC-16 helper unions SDK + legacy completion formats. AC-17 missing transcript_path key -> marker preserved. AC-17b same scenario -> stored session_id preserved. AC-17c transcript_path pointing at a non-existent file -> same guarantees. A small emit_sdk_task_notification helper was added to keep AC-15 and AC-16 declarative. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 28 passed, 0 failed - bash tests/run-all-tests.sh -> 1707 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 28 passed, 0 failed systemMessage wording unchanged. Version stays at 1.16.0. --- hooks/lib/loop-common.sh | 47 ++++++--- hooks/loop-codex-stop-hook.sh | 11 ++- tests/test-stop-hook-bg-allow.sh | 157 +++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+), 14 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 62af2c12..54944567 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -276,11 +276,17 @@ extract_transcript_path() { # - Background shell: toolUseResult.backgroundTaskId non-empty # -> id is toolUseResult.backgroundTaskId # -# Completion events (inspected in "queue-operation" messages with -# operation == "enqueue" whose content contains a -# XML block): any ... value is treated as terminal -# regardless of the reported (completed, failed, killed, -# cancelled, interrupted, ...). +# Completion events are recognised from two Claude Code transcript forms: +# +# 1. Structured SDK record +# (see SDKTaskNotificationMessage in docs/typescript.md): +# `type == "system"`, `subtype == "task_notification"`, +# `task_id` is the completed id. Any `status` value +# (completed, failed, stopped, ...) is treated as terminal. +# +# 2. Legacy queue-operation enqueue whose `content` embeds a +# `` XML block with `...`; +# kept for transcripts produced by older Claude Code versions. # # pending := launched \ completed # @@ -315,14 +321,29 @@ list_pending_background_task_ids() { | (.toolUseResult.agentId // .toolUseResult.backgroundTaskId) ' "$transcript_path" 2>/dev/null | sort -u) || return 1 - completed=$(jq -r ' - select(.type == "queue-operation" and .operation == "enqueue") - | (.content // "" | tostring) - | select(contains("")) - ' "$transcript_path" 2>/dev/null \ - | grep -oE '[^<]+' \ - | sed -E 's|||g' \ - | sort -u) || completed="" + # Union of both completion formats. Either source alone is enough to + # mark a launched id terminal. + # + # The `grep -oE || true` guard on the legacy branch keeps `set -o + # pipefail` from poisoning the combined pipeline when no legacy + # queue-operation records exist in the transcript (grep with `-o` + # exits 1 on no matches, which would otherwise wipe out any SDK + # task_notification results collected above). + completed=$( + { + jq -r ' + select(.type == "system" and .subtype == "task_notification") + | (.task_id // empty) + ' "$transcript_path" 2>/dev/null + jq -r ' + select(.type == "queue-operation" and .operation == "enqueue") + | (.content // "" | tostring) + | select(contains("")) + ' "$transcript_path" 2>/dev/null \ + | { grep -oE '[^<]+' || true; } \ + | sed -E 's|||g' + } | sort -u | sed '/^$/d' + ) || completed="" # Emit launched ids that have no matching completion notification. comm -23 \ diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index d018e598..d52fadbf 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -101,7 +101,16 @@ fi # the current one), rewrite the stored session_id so future same-session # stops use the exact-match path, then remove the marker so any later # hook trigger from an unrelated session is rejected rather than adopted. -if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then +# +# Guard: only perform the cleanup when we could actually inspect the +# transcript. `has_pending_background_tasks` is fail-closed and also +# returns false when the transcript is missing or unreadable (e.g. +# rlcr-stop-gate.sh invoked without --transcript-path). In that case the +# "no pending" signal is not authoritative, so the marker and the stored +# session_id must be preserved to keep cross-session recovery reachable. +if [[ -f "$LOOP_DIR/bg-pending.marker" ]] \ + && [[ -n "$HOOK_TRANSCRIPT_PATH" ]] \ + && [[ -f "$HOOK_TRANSCRIPT_PATH" ]]; then ADOPT_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") if [[ -n "$ADOPT_STATE_FILE" ]] && [[ -n "$HOOK_SESSION_ID" ]]; then STORED_SID_ADOPT=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$ADOPT_STATE_FILE" 2>/dev/null | tr -d ' ') diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 3dda7356..71ffb9e0 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -215,6 +215,12 @@ emit_task_completion_event() { '{type:"queue-operation", operation:"enqueue", content:$content}' } +emit_sdk_task_notification() { + local task_id="$1" tool_use_id="$2" status="${3:-completed}" + jq -c -n --arg tid "$task_id" --arg tu "$tool_use_id" --arg st "$status" \ + '{type:"system", subtype:"task_notification", task_id:$tid, tool_use_id:$tu, status:$st}' +} + write_transcript() { local path="$1" shift @@ -828,5 +834,156 @@ else "session_id: session_home" "$(grep '^session_id:' "$AC14_STATE" || echo '(missing)')" fi +# ---------------- AC-15 ---------------- +# Completion recognition: the current Claude Code transcript format emits +# background-task completion as +# type: "system", subtype: "task_notification", task_id: "..." +# The helper must recognise this form (not only the legacy queue-operation +# XML block) or launched tasks will stay "pending" forever. +echo "Test AC-15: task_notification system records mark launches completed" +AC15_TRANSCRIPT="$TRANSCRIPTS_DIR/ac15.jsonl" +AC15_LAUNCH=$(emit_tool_use_assistant "toolu_L" "Agent" ',"description":"x","prompt":"x"') +AC15_RESULT=$(emit_async_agent_launch_result "toolu_L" "agent_done_L") +AC15_NOTIF=$(emit_sdk_task_notification "agent_done_L" "toolu_L" "completed") +write_transcript "$AC15_TRANSCRIPT" "$AC15_LAUNCH" "$AC15_RESULT" "$AC15_NOTIF" + +AC15_PENDING=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + list_pending_background_task_ids "$AC15_TRANSCRIPT" 2>/dev/null +) +if [[ -z "$AC15_PENDING" ]]; then + pass "AC-15: task_notification completion removes the matching launch from pending" +else + fail "AC-15: task_notification completion removes the matching launch from pending" \ + "empty pending list" "got: $AC15_PENDING" +fi + +# ---------------- AC-16 ---------------- +# Completion recognition mixed formats: two launches, one completed via the +# legacy queue-operation XML block, the other via the current +# system/task_notification record. Union of both sources must resolve to +# an empty pending set. +echo "Test AC-16: helper unions legacy queue-operation and task_notification completions" +AC16_TRANSCRIPT="$TRANSCRIPTS_DIR/ac16.jsonl" +AC16_L1=$(emit_tool_use_assistant "toolu_M1" "Agent" ',"description":"x","prompt":"x"') +AC16_R1=$(emit_async_agent_launch_result "toolu_M1" "agent_legacy_M1") +AC16_C1=$(emit_task_completion_event "agent_legacy_M1" "toolu_M1" "completed") +AC16_L2=$(emit_tool_use_assistant "toolu_M2" "Agent" ',"description":"y","prompt":"y"') +AC16_R2=$(emit_async_agent_launch_result "toolu_M2" "agent_sdk_M2") +AC16_C2=$(emit_sdk_task_notification "agent_sdk_M2" "toolu_M2" "completed") +write_transcript "$AC16_TRANSCRIPT" \ + "$AC16_L1" "$AC16_R1" "$AC16_C1" \ + "$AC16_L2" "$AC16_R2" "$AC16_C2" + +AC16_PENDING=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + list_pending_background_task_ids "$AC16_TRANSCRIPT" 2>/dev/null +) +if [[ -z "$AC16_PENDING" ]]; then + pass "AC-16: mixed legacy+SDK completion records resolve to empty pending set" +else + fail "AC-16: mixed legacy+SDK completion records resolve to empty pending set" \ + "empty pending list" "got: $AC16_PENDING" +fi + +# ---------------- AC-17 ---------------- +# Marker preservation when completion cannot be verified: if +# transcript_path is missing or unreadable, has_pending_background_tasks +# fails closed (returns no pending). The non-short-circuit cleanup must NOT +# erase bg-pending.marker or rewrite session_id in that case, because the +# cross-session recovery signal is still needed. +echo "Test AC-17: missing transcript preserves bg-pending.marker and session_id" +AC17_REPO="$TEST_DIR/ac17" +AC17_LOOP=$(create_full_fixture "$AC17_REPO") +AC17_STATE="$AC17_LOOP/state.md" +AC17_BRANCH=$(git -C "$AC17_REPO" rev-parse --abbrev-ref HEAD) +AC17_BASE_COMMIT=$(git -C "$AC17_REPO" rev-parse HEAD) +cat > "$AC17_STATE" < "$AC17_LOOP/bg-pending.marker" + +# Hook input has NO transcript_path -> has_pending_background_tasks is +# fail-closed; cleanup path must leave marker and session_id intact. +AC17_INPUT='{"session_id":"session_home"}' +run_stop_hook_with_input "$AC17_REPO" "$AC17_INPUT" + +if [[ -f "$AC17_LOOP/bg-pending.marker" ]]; then + pass "AC-17: unreadable transcript preserves bg-pending.marker" +else + fail "AC-17: unreadable transcript preserves bg-pending.marker" \ + "marker still present" "marker was removed" +fi + +if grep -q "^session_id: session_foreign$" "$AC17_STATE"; then + pass "AC-17b: unreadable transcript leaves stored session_id untouched" +else + fail "AC-17b: unreadable transcript leaves stored session_id untouched" \ + "session_id: session_foreign" "$(grep '^session_id:' "$AC17_STATE" || echo '(missing)')" +fi + +# AC-17c: transcript_path is provided but points at a non-existent file +# (equally unreadable). Same guarantee: marker + stored session_id +# preserved. +echo "Test AC-17c: transcript_path pointing at non-existent file preserves marker" +AC17C_REPO="$TEST_DIR/ac17c" +AC17C_LOOP=$(create_full_fixture "$AC17C_REPO") +AC17C_STATE="$AC17C_LOOP/state.md" +AC17C_BRANCH=$(git -C "$AC17C_REPO" rev-parse --abbrev-ref HEAD) +AC17C_BASE_COMMIT=$(git -C "$AC17C_REPO" rev-parse HEAD) +cat > "$AC17C_STATE" < "$AC17C_LOOP/bg-pending.marker" + +AC17C_INPUT=$(jq -c -n --arg tp "$TRANSCRIPTS_DIR/never-written.jsonl" \ + '{transcript_path:$tp, session_id:"session_home"}') +run_stop_hook_with_input "$AC17C_REPO" "$AC17C_INPUT" + +if [[ -f "$AC17C_LOOP/bg-pending.marker" ]] \ + && grep -q "^session_id: session_foreign$" "$AC17C_STATE"; then + pass "AC-17c: missing-file transcript_path preserves marker and session_id" +else + fail "AC-17c: missing-file transcript_path preserves marker and session_id" \ + "marker present and session_id: session_foreign" \ + "marker=$(test -f "$AC17C_LOOP/bg-pending.marker" && echo present || echo missing); session_id=$(grep '^session_id:' "$AC17C_STATE" || echo '(missing)')" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 60e263504442255fcc4edf2ca89a2f805e8e77c1 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 14:56:17 -0700 Subject: [PATCH 07/13] Narrow cross-session adoption and block foreign-session hijack Three blocking findings from the previous review: [P1] The Round 5 non-short-circuit cleanup hijacked foreign parked loops: session B could rewrite a parked loop's stored session_id to B and delete the marker using B's own transcript, even though A's background task was still running. A was locked out of its own loop. [P2] Every caller of find_active_loop inherited the marker fallback, including loop-read-validator, loop-write- validator, loop-bash-validator, and loop-plan-file- validator. An unrelated session's validators started enforcing foreign parked-loop gates (notably the methodology-analysis phase) on ordinary writes and bash commands, breaking existing session isolation guarantees. [P2] The Round 4 sed -i.bak session_id rewrite injected HOOK_SESSION_ID into the replacement text unescaped. Session IDs containing `&` are valid per this repo's test-session-id.sh; adopting such an id corrupted state.md. Fixes, confined to files already touched by this feature: * loop-common.sh: find_active_loop gains a third positional parameter allow_bg_marker_fallback (default false). Both the inner marker_candidate record and the post-loop fallback return are gated on it. Only the stop hook opts in; validators keep the pre-Round-3 strict isolation. * loop-codex-stop-hook.sh: - Call find_active_loop with `true` to opt in. - Add a new "Cross-Session Parked-Loop Guard" block before the short-circuit: when bg-pending.marker is present AND the stored session_id differs from HOOK_SESSION_ID, emit a dedicated "parked by another Claude session" systemMessage and exit 0 without touching marker, state.md, or session_id. B's hook can never advance A's parked loop on B's transcript. - Simplify the non-short-circuit cleanup: the guard ensures only same-session cases reach here, so the cleanup is now just `rm -f bg-pending.marker` (still gated on transcript readability). The sed session_id rewrite is removed entirely, which also removes the unescaped-metacharacter issue. Regressions in tests/test-stop-hook-bg-allow.sh: AC-11 (rewritten) cross-session + marker -> "parked" systemMessage + marker preserved + state.md byte-identical. AC-14 (rewritten) anti-hijack: cross-session stop preserves bg-pending.marker. AC-14b (rewritten) cross-session stop leaves stored session_id intact. AC-18 NEW. find_active_loop default (no opt-in) ignores a foreign marker dir -> validators stay isolated. AC-18b NEW. find_active_loop with opt-in does return the marker dir (confirms the flag is wired). All other existing regressions continue to pass. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 30 passed, 0 failed - bash tests/run-all-tests.sh -> 1709 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 30 passed, 0 failed Original short-circuit systemMessage wording unchanged. Version stays at 1.16.0. --- hooks/lib/loop-common.sh | 26 +++++-- hooks/loop-codex-stop-hook.sh | 67 ++++++++++-------- tests/test-stop-hook-bg-allow.sh | 114 +++++++++++++++++++++++-------- 3 files changed, 143 insertions(+), 64 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 54944567..b8037614 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -442,10 +442,17 @@ resolve_any_state_file() { # Empty stored session_id matches any filter (backward compat for pre-session # state files). # +# Third parameter `allow_bg_marker_fallback` (default "false"): when "true", +# the session-filter branch also considers a mismatched-session dir that holds +# a `bg-pending.marker` file AND an active state file. Only the RLCR stop +# hook opts in to this; every other caller (read/write/bash/plan-file +# validators, ...) keeps strict session isolation. +# # Outputs the directory path to stdout, or empty string if none found find_active_loop() { local loop_base_dir="$1" local filter_session_id="${2:-}" + local allow_bg_marker_fallback="${3:-false}" if [[ ! -d "$loop_base_dir" ]]; then echo "" @@ -509,9 +516,13 @@ find_active_loop() { return fi - # Session mismatch: stash the newest eligible marker candidate but - # keep walking in case an older dir is the caller's own session. - if [[ -z "$marker_candidate" ]] && [[ -f "$trimmed_dir/bg-pending.marker" ]]; then + # Session mismatch. Only the stop hook opts in to marker-based + # adoption; validators and other callers keep strict isolation, so + # the candidate is only recorded when the caller explicitly allows + # it. + if [[ "$allow_bg_marker_fallback" == "true" ]] \ + && [[ -z "$marker_candidate" ]] \ + && [[ -f "$trimmed_dir/bg-pending.marker" ]]; then local candidate_state candidate_state=$(resolve_active_state_file "$trimmed_dir") if [[ -n "$candidate_state" ]]; then @@ -521,10 +532,11 @@ find_active_loop() { fi done < <(ls -1d "$loop_base_dir"/*/ 2>/dev/null | sort -r) - # No exact session match. Fall back to marker-based adoption if any -- - # this is the cross-session recovery path when a previous session parked - # the loop and then died before the background-task completion arrived. - if [[ -n "$marker_candidate" ]]; then + # No exact session match. Fall back to marker-based adoption only when + # the caller explicitly opted in -- the stop hook uses this to surface + # a "parked by another session" notice or to resume its own parked + # loop after a previous session died before the bg completion arrived. + if [[ "$allow_bg_marker_fallback" == "true" ]] && [[ -n "$marker_candidate" ]]; then echo "$marker_candidate" return fi diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index d52fadbf..58bf6c1e 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -61,13 +61,37 @@ GIT_TIMEOUT=30 # Extract session_id from hook input for session-aware loop filtering HOOK_SESSION_ID=$(extract_session_id "$HOOK_INPUT") -LOOP_DIR=$(find_active_loop "$LOOP_BASE_DIR" "$HOOK_SESSION_ID") +LOOP_DIR=$(find_active_loop "$LOOP_BASE_DIR" "$HOOK_SESSION_ID" true) # If no active loop (or session_id mismatch), allow exit if [[ -z "$LOOP_DIR" ]]; then exit 0 fi +# ======================================== +# Cross-Session Parked-Loop Guard +# ======================================== +# If find_active_loop handed this dir over via the marker fallback, the +# loop is parked by a different session waiting on a background task. The +# current session has no authority to inspect or advance that loop - its +# transcript sees none of the foreign bg activity - so the only safe +# response is to exit 0 with a distinct systemMessage and leave every +# on-disk artifact (state file, stored session_id, marker) untouched. +HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") +if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then + GUARD_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") + if [[ -n "$GUARD_STATE_FILE" ]]; then + GUARD_STORED_SID=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$GUARD_STATE_FILE" 2>/dev/null | tr -d ' ') + if [[ -n "$GUARD_STORED_SID" ]] \ + && [[ -n "$HOOK_SESSION_ID" ]] \ + && [[ "$GUARD_STORED_SID" != "$HOOK_SESSION_ID" ]]; then + jq -n \ + '{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."}' + exit 0 + fi + fi +fi + # ======================================== # Early Exit: Pending Background Tasks # ======================================== @@ -84,45 +108,32 @@ fi # # This check MUST run before any other gate (phase detection, state parsing, # branch / plan / git-clean / summary / max-iter checks, Codex review). -HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH") - # Mark the loop as parked; allows a fresh session to adopt it if this - # Claude window is closed before the background task finishes. + # Mark the loop as parked; allows the same session to resume later and + # makes the cross-session guard above reachable if the user opens a + # different Claude session in this repo before the bg task completes. : > "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true jq -n --arg count "$PENDING_BG_COUNT" \ '{systemMessage: ("RLCR loop active. " + $count + " background task(s) still running - stop allowed naturally; loop has NOT terminated and will resume on completion.")}' exit 0 fi -# No pending background task. If a stale bg-pending.marker is lingering -# here, this stop is the resume point. When find_active_loop picked this -# dir up through the marker-fallback path (stored session_id differs from -# the current one), rewrite the stored session_id so future same-session -# stops use the exact-match path, then remove the marker so any later -# hook trigger from an unrelated session is rejected rather than adopted. +# Same-session resume after background task finished: the cross-session +# guard above already exited for every foreign session, so reaching here +# with the marker present means the CURRENT session parked the loop and +# has now come back with a transcript showing no pending bg events. +# Remove the stale marker before the normal flow takes over. # -# Guard: only perform the cleanup when we could actually inspect the -# transcript. `has_pending_background_tasks` is fail-closed and also -# returns false when the transcript is missing or unreadable (e.g. -# rlcr-stop-gate.sh invoked without --transcript-path). In that case the -# "no pending" signal is not authoritative, so the marker and the stored -# session_id must be preserved to keep cross-session recovery reachable. +# Guard: only run when we could actually inspect the transcript. +# `has_pending_background_tasks` is fail-closed and also returns false +# when the transcript is missing or unreadable (e.g. rlcr-stop-gate.sh +# invoked without --transcript-path). In that case the "no pending" +# signal is not authoritative, so the marker stays in place to keep +# cross-session recovery reachable. if [[ -f "$LOOP_DIR/bg-pending.marker" ]] \ && [[ -n "$HOOK_TRANSCRIPT_PATH" ]] \ && [[ -f "$HOOK_TRANSCRIPT_PATH" ]]; then - ADOPT_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") - if [[ -n "$ADOPT_STATE_FILE" ]] && [[ -n "$HOOK_SESSION_ID" ]]; then - STORED_SID_ADOPT=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$ADOPT_STATE_FILE" 2>/dev/null | tr -d ' ') - if [[ -n "$STORED_SID_ADOPT" ]] && [[ "$STORED_SID_ADOPT" != "$HOOK_SESSION_ID" ]]; then - # Portable in-place rewrite. Failure is logged but non-fatal: - # worst case the next stop re-adopts via the marker pathway. - if ! sed -i.bak -E "s|^(${FIELD_SESSION_ID}:).*$|\\1 $HOOK_SESSION_ID|" "$ADOPT_STATE_FILE" 2>/dev/null; then - echo "Warning: failed to adopt session_id in $ADOPT_STATE_FILE" >&2 - fi - rm -f "${ADOPT_STATE_FILE}.bak" 2>/dev/null || true - fi - fi rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true fi diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 71ffb9e0..b42b9423 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -552,17 +552,17 @@ else fi # ---------------- AC-11 / AC-11b ---------------- -# Orphan prevention: when the short-circuit parks a loop waiting for a -# background task and the user closes that Claude session, a fresh -# session must still be able to pick up the loop. The short-circuit -# writes `bg-pending.marker` into the loop dir; find_active_loop -# accepts a stored-vs-filter session_id mismatch iff the marker is -# present. Without this cross-session adoption path, state.md would -# be stranded with the dead session_id and require manual cancel. -echo "Test AC-11: cross-session bg-pending.marker allows pickup" +# Cross-session parked-loop guard: when a loop in the repo carries the +# bg-pending.marker and its stored session_id does not match the caller, +# the stop hook must exit 0 with a dedicated "parked by another session" +# systemMessage and leave every on-disk artifact intact. The current +# session has no authority to advance or cleanup a foreign parked loop +# because its transcript cannot observe the other session's bg task. +echo "Test AC-11: cross-session bg-pending.marker emits 'parked' systemMessage" AC11_REPO="$TEST_DIR/ac11" AC11_LOOP=$(create_full_fixture "$AC11_REPO") AC11_STATE="$AC11_LOOP/state.md" +AC11_MARKER="$AC11_LOOP/bg-pending.marker" # Override state.md with an explicit stored session_id so find_active_loop # sees a real mismatch when we later pass a different session_id. @@ -588,11 +588,10 @@ agent_teams: false session_id: session_alpha --- EOF_AC11 +AC11_STATE_HASH_BEFORE=$(sha256sum "$AC11_STATE" | awk '{print $1}') -# Simulate the state left by a previous session that took the short-circuit -# and then died (Claude window closed). The marker is the public contract -# between the short-circuit path and cross-session pickup. -: > "$AC11_LOOP/bg-pending.marker" +# Simulate the state left by a previous session that took the short-circuit. +: > "$AC11_MARKER" AC11_TRANSCRIPT="$TRANSCRIPTS_DIR/ac11.jsonl" AC11_LAUNCH=$(emit_tool_use_assistant "toolu_I" "Agent" ',"description":"x","prompt":"x"') @@ -602,9 +601,19 @@ write_transcript "$AC11_TRANSCRIPT" "$AC11_LAUNCH" "$AC11_RESULT" AC11_INPUT=$(jq -c -n --arg tp "$AC11_TRANSCRIPT" \ '{transcript_path:$tp, session_id:"session_beta"}') run_stop_hook_with_input "$AC11_REPO" "$AC11_INPUT" -assert_systemmessage_only \ - "AC-11: cross-session bg-pending.marker allows pickup and short-circuit" \ - "$AC11_REPO" "$AC11_STATE" "1 background task" +AC11_SYS_MSG=$(printf '%s' "$RUN_OUTPUT" | jq -r '.systemMessage // empty' 2>/dev/null || echo "") +AC11_STATE_HASH_AFTER=$(sha256sum "$AC11_STATE" | awk '{print $1}') +if [[ "$RUN_EXIT_CODE" -eq 0 ]] \ + && [[ ! -f "$RUN_MARKER" ]] \ + && [[ -f "$AC11_MARKER" ]] \ + && [[ "$AC11_STATE_HASH_BEFORE" == "$AC11_STATE_HASH_AFTER" ]] \ + && printf '%s' "$AC11_SYS_MSG" | grep -qi "parked"; then + pass "AC-11: cross-session stop exits with 'parked' systemMessage; marker and session_id untouched" +else + fail "AC-11: cross-session stop exits with 'parked' systemMessage; marker and session_id untouched" \ + "exit 0 + systemMessage matches /parked/ + marker stays + state.md byte-identical + no Codex" \ + "exit $RUN_EXIT_CODE, codex_marker=$(test -f "$RUN_MARKER" && echo present || echo missing), bg_marker=$(test -f "$AC11_MARKER" && echo present || echo missing), state_unchanged=$([[ "$AC11_STATE_HASH_BEFORE" == "$AC11_STATE_HASH_AFTER" ]] && echo yes || echo no), systemMessage='$AC11_SYS_MSG'; output: $RUN_OUTPUT" +fi # Negative counterpart: same session mismatch but NO marker must still # reject the loop (preserving the existing session-bound isolation when @@ -782,14 +791,17 @@ else fi # ---------------- AC-14 ---------------- -# Cross-session resume: a different session walks in, finds the loop through -# the marker fallback, and the non-short-circuit path must rewrite the -# stored session_id so future same-session stops use the exact-match path -# instead of re-adopting via a stale marker. -echo "Test AC-14: cross-session resume rewrites session_id and removes marker" +# Anti-hijack: a different session walking in MUST NOT rewrite the stored +# session_id and MUST NOT delete bg-pending.marker, even when its own +# transcript shows no pending bg events. The foreign session's transcript +# cannot observe the parking session's bg activity, so nothing the new +# session sees is authoritative. The cross-session guard takes over +# instead. +echo "Test AC-14: cross-session stop preserves marker and stored session_id" AC14_REPO="$TEST_DIR/ac14" AC14_LOOP=$(create_full_fixture "$AC14_REPO") AC14_STATE="$AC14_LOOP/state.md" +AC14_MARKER="$AC14_LOOP/bg-pending.marker" AC14_BRANCH=$(git -C "$AC14_REPO" rev-parse --abbrev-ref HEAD) AC14_BASE_COMMIT=$(git -C "$AC14_REPO" rev-parse HEAD) cat > "$AC14_STATE" < "$AC14_LOOP/bg-pending.marker" +: > "$AC14_MARKER" AC14_TRANSCRIPT="$TRANSCRIPTS_DIR/ac14.jsonl" write_transcript "$AC14_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' @@ -820,18 +832,18 @@ AC14_INPUT=$(jq -c -n --arg tp "$AC14_TRANSCRIPT" \ '{transcript_path:$tp, session_id:"session_home"}') run_stop_hook_with_input "$AC14_REPO" "$AC14_INPUT" -if [[ ! -f "$AC14_LOOP/bg-pending.marker" ]]; then - pass "AC-14: marker removed after cross-session resume" +if [[ -f "$AC14_MARKER" ]]; then + pass "AC-14: cross-session stop preserves bg-pending.marker" else - fail "AC-14: marker removed after cross-session resume" \ - "marker absent" "marker still present" + fail "AC-14: cross-session stop preserves bg-pending.marker" \ + "marker still present" "marker was removed (foreign-session hijack)" fi -if grep -q "^session_id: session_home$" "$AC14_STATE"; then - pass "AC-14b: state.md session_id rewritten to the current session on adoption" +if grep -q "^session_id: session_foreign$" "$AC14_STATE"; then + pass "AC-14b: cross-session stop leaves stored session_id intact" else - fail "AC-14b: state.md session_id rewritten to the current session on adoption" \ - "session_id: session_home" "$(grep '^session_id:' "$AC14_STATE" || echo '(missing)')" + fail "AC-14b: cross-session stop leaves stored session_id intact" \ + "session_id: session_foreign" "$(grep '^session_id:' "$AC14_STATE" || echo '(missing)')" fi # ---------------- AC-15 ---------------- @@ -985,5 +997,49 @@ else "marker=$(test -f "$AC17C_LOOP/bg-pending.marker" && echo present || echo missing); session_id=$(grep '^session_id:' "$AC17C_STATE" || echo '(missing)')" fi +# ---------------- AC-18 ---------------- +# Validator isolation: find_active_loop's marker-based adoption is opt-in +# via its third positional argument. Default callers (read/write/bash/etc. +# validators) must continue to see strict session-id isolation; a parked +# loop for a different session must NOT become visible to them through a +# bg-pending.marker. +echo "Test AC-18: find_active_loop default invocation ignores foreign marker" +AC18_BASE="$TEST_DIR/ac18-loops" +mkdir -p "$AC18_BASE/2026-03-02_00-00-00" +cat > "$AC18_BASE/2026-03-02_00-00-00/state.md" <<'EOF_AC18' +--- +current_round: 0 +max_iterations: 42 +codex_model: gpt-5.4 +codex_effort: high +session_id: session_foreign +--- +EOF_AC18 +: > "$AC18_BASE/2026-03-02_00-00-00/bg-pending.marker" + +AC18_DEFAULT=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + find_active_loop "$AC18_BASE" "session_home" +) +if [[ -z "$AC18_DEFAULT" ]]; then + pass "AC-18: find_active_loop default (no opt-in) ignores foreign marker dir" +else + fail "AC-18: find_active_loop default (no opt-in) ignores foreign marker dir" \ + "empty result (validators stay isolated)" "got: $AC18_DEFAULT" +fi + +AC18_OPTIN=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + find_active_loop "$AC18_BASE" "session_home" true +) +if [[ "$AC18_OPTIN" == "$AC18_BASE/2026-03-02_00-00-00" ]]; then + pass "AC-18b: find_active_loop with opt-in does return the marker dir" +else + fail "AC-18b: find_active_loop with opt-in does return the marker dir" \ + "$AC18_BASE/2026-03-02_00-00-00" "$AC18_OPTIN" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 7fb303804e33037a786b3561dd1da9c330a804f3 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 15:07:27 -0700 Subject: [PATCH 08/13] Tighten cross-session guard and marker cleanup Two blocking findings from the previous review: [P1] The cross-session parked-loop guard required both a non-empty stored session_id AND a non-empty HOOK_SESSION_ID. Callers that reach the hook without a session_id field -- including scripts/rlcr-stop-gate.sh invoked without --session-id -- bypassed the guard, and a foreign parked loop fell through into the normal path. With any readable transcript, the later cleanup deleted bg-pending.marker and the hook started reviewing a loop it did not own. [P2] Non-short-circuit cleanup only verified that the transcript file existed. list_pending_background_task_ids is fail-closed on malformed or truncated transcripts, yet the cleanup still deleted the marker in that case. The parked-state signal was lost even though background completion was never verified. Fixes, limited to loop-codex-stop-hook.sh: * Cross-session guard: drop the `-n "$HOOK_SESSION_ID"` clause. A non-empty stored session_id that differs from the (possibly empty) hook session_id now triggers the "parked by another Claude session" exit path. Backward-compat semantics are preserved: an empty stored session_id still matches any caller, consistent with find_active_loop's existing rule. * Non-short-circuit cleanup: call list_pending_background_task_ids inline and check its exit code along with its output. The marker is removed only when the helper returned exit 0 AND produced an empty id list. Every fail-closed path (missing file, empty path, jq parse failure, truncation) now leaves the marker intact. No changes to hooks/lib/loop-common.sh; the helper already has the exit-code semantics we rely on. Regressions in tests/test-stop-hook-bg-allow.sh: AC-19 Hook input with NO session_id key + state.md session_id populated + bg-pending.marker -> exit 0 with "parked" systemMessage, marker preserved, state.md byte-identical. AC-20 Hook input pointing transcript_path at a deliberately malformed JSONL file + bg-pending.marker -> marker preserved. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 32 passed, 0 failed - bash tests/run-all-tests.sh -> 1711 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 32 passed, 0 failed systemMessage wording unchanged. Version stays at 1.16.0. --- hooks/loop-codex-stop-hook.sh | 33 +++++--- tests/test-stop-hook-bg-allow.sh | 138 +++++++++++++++++++++++++++++-- 2 files changed, 151 insertions(+), 20 deletions(-) diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index 58bf6c1e..d0d3af30 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -82,8 +82,13 @@ if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then GUARD_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") if [[ -n "$GUARD_STATE_FILE" ]]; then GUARD_STORED_SID=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$GUARD_STATE_FILE" 2>/dev/null | tr -d ' ') + # Non-empty stored session_id that differs from the caller's session + # (empty or not) means this is a foreign parked loop. Hook-input + # schemas that omit session_id -- such as rlcr-stop-gate.sh invoked + # without --session-id -- still get a mismatch here and take the + # safe exit path. An empty stored session_id keeps the existing + # backward-compat "matches any" semantics from find_active_loop. if [[ -n "$GUARD_STORED_SID" ]] \ - && [[ -n "$HOOK_SESSION_ID" ]] \ && [[ "$GUARD_STORED_SID" != "$HOOK_SESSION_ID" ]]; then jq -n \ '{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."}' @@ -125,16 +130,22 @@ fi # has now come back with a transcript showing no pending bg events. # Remove the stale marker before the normal flow takes over. # -# Guard: only run when we could actually inspect the transcript. -# `has_pending_background_tasks` is fail-closed and also returns false -# when the transcript is missing or unreadable (e.g. rlcr-stop-gate.sh -# invoked without --transcript-path). In that case the "no pending" -# signal is not authoritative, so the marker stays in place to keep -# cross-session recovery reachable. -if [[ -f "$LOOP_DIR/bg-pending.marker" ]] \ - && [[ -n "$HOOK_TRANSCRIPT_PATH" ]] \ - && [[ -f "$HOOK_TRANSCRIPT_PATH" ]]; then - rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true +# Two-part guard to make sure we never drop the parked-state signal +# without evidence: +# (a) list_pending_background_task_ids returned exit 0 -- the +# transcript was present, readable, AND parsed successfully. +# The helper is fail-closed on missing files, empty paths, +# jq parse failure, and truncation, so a non-zero exit blocks +# cleanup here even when the transcript "file" exists. +# (b) its output is empty -- proves "no pending" was authoritatively +# verified, not inferred from a failure. +# The check uses a single fresh call so we capture both the exit code +# and the emptiness without double-running jq. +if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then + if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" 2>/dev/null) \ + && [[ -z "$PENDING_BG_CHECK" ]]; then + rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true + fi fi # ======================================== diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index b42b9423..930d6ffc 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -11,15 +11,26 @@ # # Acceptance criteria exercised here (see # .humanize/rlcr/2026-04-16_13-19-26/goal-tracker.md for authoritative list): -# AC-1 no bg dispatches -> normal Codex flow -# AC-2 pending subagent -> exit 0 + systemMessage -# AC-3 pending shell -> exit 0 + systemMessage -# AC-4 subagent launch + complete -> normal Codex flow -# AC-5 2 subagents + 1 shell -> systemMessage mentions "3 background" -# AC-6 missing transcript path -> normal Codex flow (fail-closed) -# AC-7 no active loop -> exit 0, no systemMessage, no Codex -# AC-8 finalize phase pending bg -> exit 0 + systemMessage -# AC-9 via rlcr-stop-gate.sh -> exit 0 (wrapper ALLOW) +# AC-1 no bg dispatches -> normal Codex flow +# AC-2 pending subagent -> exit 0 + systemMessage +# AC-3 pending shell -> exit 0 + systemMessage +# AC-4 subagent launch + complete -> normal Codex flow +# AC-5 2 subagents + 1 shell -> systemMessage mentions "3 background" +# AC-6 missing transcript path -> normal Codex flow (fail-closed) +# AC-7 no active loop -> exit 0, no systemMessage, no Codex +# AC-8 finalize phase pending bg -> exit 0 + systemMessage +# AC-9 via rlcr-stop-gate.sh -> exit 0 (wrapper ALLOW) +# AC-10 tilde transcript path -> short-circuit fires +# AC-11 cross-session bg-pending.marker -> "parked" systemMessage, artifacts intact +# AC-12 find_active_loop prefers exact session -> returns older exact-match dir +# AC-13 same-session resume -> stale marker removed +# AC-14 cross-session stop with marker -> marker and stored session_id preserved +# AC-15 task_notification completion format -> marks launch completed +# AC-16 mixed legacy + SDK completions -> resolves to empty pending set +# AC-17 unreadable transcript with marker -> marker and session_id preserved +# AC-18 find_active_loop default ignores marker -> validators stay isolated +# AC-19 hook input omits session_id -> cross-session guard fires +# AC-20 malformed transcript with marker -> marker preserved (fail-closed) # set -euo pipefail @@ -1041,5 +1052,114 @@ else "$AC18_BASE/2026-03-02_00-00-00" "$AC18_OPTIN" fi +# ---------------- AC-19 ---------------- +# Empty-session caller must still be treated as "foreign" for a parked +# loop whose stored session_id is non-empty. Real trigger: callers such +# as scripts/rlcr-stop-gate.sh invoked without --session-id reach the +# hook with no session_id key at all. +echo "Test AC-19: cross-session guard fires when hook input omits session_id" +AC19_REPO="$TEST_DIR/ac19" +AC19_LOOP=$(create_full_fixture "$AC19_REPO") +AC19_STATE="$AC19_LOOP/state.md" +AC19_MARKER="$AC19_LOOP/bg-pending.marker" +AC19_BRANCH=$(git -C "$AC19_REPO" rev-parse --abbrev-ref HEAD) +AC19_BASE_COMMIT=$(git -C "$AC19_REPO" rev-parse HEAD) +cat > "$AC19_STATE" < "$AC19_MARKER" + +# Transcript exists and is a readable, well-formed minimal record. The +# guard must rely on the stored-vs-current session_id mismatch alone, +# not on transcript readability, to detect the foreign-session case. +AC19_TRANSCRIPT="$TRANSCRIPTS_DIR/ac19.jsonl" +write_transcript "$AC19_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' + +# Hook input without any session_id key (mirrors rlcr-stop-gate.sh +# invoked without --session-id). +AC19_INPUT=$(jq -c -n --arg tp "$AC19_TRANSCRIPT" '{transcript_path:$tp}') +run_stop_hook_with_input "$AC19_REPO" "$AC19_INPUT" +AC19_SYS_MSG=$(printf '%s' "$RUN_OUTPUT" | jq -r '.systemMessage // empty' 2>/dev/null || echo "") +AC19_STATE_HASH_AFTER=$(sha256sum "$AC19_STATE" | awk '{print $1}') +if [[ "$RUN_EXIT_CODE" -eq 0 ]] \ + && [[ ! -f "$RUN_MARKER" ]] \ + && [[ -f "$AC19_MARKER" ]] \ + && [[ "$AC19_STATE_HASH_BEFORE" == "$AC19_STATE_HASH_AFTER" ]] \ + && printf '%s' "$AC19_SYS_MSG" | grep -qi "parked"; then + pass "AC-19: empty hook session_id triggers 'parked' guard; marker and state preserved" +else + fail "AC-19: empty hook session_id triggers 'parked' guard; marker and state preserved" \ + "exit 0 + systemMessage matches /parked/ + marker stays + state.md byte-identical + no Codex" \ + "exit $RUN_EXIT_CODE, codex_marker=$(test -f "$RUN_MARKER" && echo present || echo missing), bg_marker=$(test -f "$AC19_MARKER" && echo present || echo missing), state_unchanged=$([[ "$AC19_STATE_HASH_BEFORE" == "$AC19_STATE_HASH_AFTER" ]] && echo yes || echo no), systemMessage='$AC19_SYS_MSG'; output: $RUN_OUTPUT" +fi + +# ---------------- AC-20 ---------------- +# Non-short-circuit cleanup must not drop bg-pending.marker when the +# transcript exists but cannot be parsed. The helper is fail-closed on +# malformed JSON; that failure must NOT be treated as "no pending". +echo "Test AC-20: malformed transcript preserves bg-pending.marker" +AC20_REPO="$TEST_DIR/ac20" +AC20_LOOP=$(create_full_fixture "$AC20_REPO") +AC20_STATE="$AC20_LOOP/state.md" +AC20_MARKER="$AC20_LOOP/bg-pending.marker" +AC20_BRANCH=$(git -C "$AC20_REPO" rev-parse --abbrev-ref HEAD) +AC20_BASE_COMMIT=$(git -C "$AC20_REPO" rev-parse HEAD) +cat > "$AC20_STATE" < "$AC20_MARKER" + +# Write a deliberately malformed transcript (truncated JSON object) so +# list_pending_background_task_ids's jq invocations fail the parse. +AC20_TRANSCRIPT="$TRANSCRIPTS_DIR/ac20.jsonl" +printf '%s\n' '{"type":"user","message":' > "$AC20_TRANSCRIPT" + +AC20_INPUT=$(jq -c -n --arg tp "$AC20_TRANSCRIPT" \ + '{transcript_path:$tp, session_id:"session_home"}') +run_stop_hook_with_input "$AC20_REPO" "$AC20_INPUT" + +if [[ -f "$AC20_MARKER" ]]; then + pass "AC-20: malformed transcript preserves bg-pending.marker" +else + fail "AC-20: malformed transcript preserves bg-pending.marker" \ + "marker still present (cleanup must not fire on fail-closed helper)" \ + "marker was removed" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 7538f742cd7af4c04391d18d236c39581e4e8a35 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 15:24:19 -0700 Subject: [PATCH 09/13] 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. --- hooks/lib/loop-common.sh | 53 +++++++- hooks/loop-codex-stop-hook.sh | 44 +++++-- tests/test-stop-hook-bg-allow.sh | 215 +++++++++++++++++++++++++++++-- 3 files changed, 282 insertions(+), 30 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index b8037614..2b71e778 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -267,6 +267,31 @@ extract_transcript_path() { expand_leading_tilde "$raw" } +# Convert an RLCR loop dir basename to a lexically-comparable ISO-8601 +# timestamp suitable for filtering transcript events. +# +# The setup script creates loop dirs named `YYYY-MM-DD_HH-MM-SS`; real +# Claude transcript events carry timestamps like `2026-04-16T13:19:26.819Z`. +# String comparison works cleanly once we pad the loop boundary with +# `.000Z` so sub-second transcript timestamps in the same second always +# compare greater. +# +# Usage: derive_loop_start_iso_ts "$loop_dir" +# Prints the ISO-8601 timestamp, or empty string when the basename does +# not match the expected format. +derive_loop_start_iso_ts() { + local loop_dir="$1" + local base + base=$(basename "$loop_dir" 2>/dev/null || echo "") + if [[ "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then + printf '%sT%s:%s:%s.000Z' \ + "${BASH_REMATCH[1]}" \ + "${BASH_REMATCH[2]}" \ + "${BASH_REMATCH[3]}" \ + "${BASH_REMATCH[4]}" + fi +} + # Enumerate background-task ids that have been launched but not yet marked # completed in a Claude Code transcript.jsonl. # @@ -290,7 +315,15 @@ extract_transcript_path() { # # pending := launched \ completed # -# Usage: list_pending_background_task_ids "$transcript_path" +# Optional second argument `since_ts` (ISO-8601 string, e.g. the value +# returned by `derive_loop_start_iso_ts`): when provided, only launch +# events whose top-level `.timestamp` field is >= `since_ts` count as +# candidate launches. Events without a `.timestamp` are included (keeps +# fixture transcripts and older record formats working). This keeps +# pre-loop session-wide background work from pinning an RLCR loop that +# has no pending work of its own. +# +# Usage: list_pending_background_task_ids "$transcript_path" [since_ts] # - Outputs one id per line on stdout (possibly empty). # - Returns 0 when the transcript is readable (including when there are # no pending tasks). Returns 1 when the transcript path is empty, not @@ -298,6 +331,7 @@ extract_transcript_path() { # as "unknown -> do not short-circuit". list_pending_background_task_ids() { local transcript_path="$1" + local since_ts="${2:-}" # Normalize a leading tilde so direct callers (tests, ad-hoc scripts) # work correctly even when transcript_path was not routed through @@ -312,8 +346,13 @@ list_pending_background_task_ids() { fi local launched completed - launched=$(jq -r ' + launched=$(jq -r --arg since_ts "$since_ts" ' select(.toolUseResult != null) + | select( + ($since_ts == "" + or ((.timestamp // "") == "") + or ((.timestamp // "") >= $since_ts)) + ) | select( (.toolUseResult.isAsync == true and (.toolUseResult.agentId // "") != "") or ((.toolUseResult.backgroundTaskId // "") != "") @@ -355,22 +394,24 @@ list_pending_background_task_ids() { # Returns 1 when no pending tasks are detected (including fail-closed cases # like missing transcript, non-file path, or jq unavailable). # -# Usage: has_pending_background_tasks "$transcript_path" +# Usage: has_pending_background_tasks "$transcript_path" [since_ts] has_pending_background_tasks() { local transcript_path="$1" + local since_ts="${2:-}" local pending - pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || return 1 + pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || return 1 [[ -n "$pending" ]] } # Prints the count of pending background tasks to stdout. Prints 0 for any # error case so callers can still format messages safely. # -# Usage: count_pending_background_tasks "$transcript_path" +# Usage: count_pending_background_tasks "$transcript_path" [since_ts] count_pending_background_tasks() { local transcript_path="$1" + local since_ts="${2:-}" local pending - pending=$(list_pending_background_task_ids "$transcript_path" 2>/dev/null) || { + pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || { echo 0 return 0 } diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index d0d3af30..405c82ef 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -68,6 +68,27 @@ if [[ -z "$LOOP_DIR" ]]; then exit 0 fi +# Shared state used by both guard blocks and the pending-tasks check below. +# Loop-start boundary: derived from the loop dir basename (`YYYY-MM-DD_HH-MM-SS`). +# Empty means derivation failed; helpers treat empty since_ts as no boundary. +LOOP_START_TS=$(derive_loop_start_iso_ts "$LOOP_DIR") +HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") + +# ======================================== +# Ambiguous-Caller Marker Guard +# ======================================== +# If a bg-pending.marker is present but we have no session_id on this +# hook invocation (typical of scripts/rlcr-stop-gate.sh invoked without +# --session-id, or any other caller that doesn't forward session_id), +# we cannot tell whether this caller owns the parked loop. Taking either +# branch (foreign-session guard below, or same-session cleanup further +# down) would be wrong in one of the two possible realities. Exit 0 +# silently: the real Claude hook will arrive with session_id populated +# and drive parking / cleanup from an authoritative context. +if [[ -f "$LOOP_DIR/bg-pending.marker" ]] && [[ -z "$HOOK_SESSION_ID" ]]; then + exit 0 +fi + # ======================================== # Cross-Session Parked-Loop Guard # ======================================== @@ -77,18 +98,17 @@ fi # transcript sees none of the foreign bg activity - so the only safe # response is to exit 0 with a distinct systemMessage and leave every # on-disk artifact (state file, stored session_id, marker) untouched. -HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") +# +# Both sides of the session-id comparison must be non-empty for this +# branch to trigger: an empty HOOK_SESSION_ID has already exited above +# via the ambiguous-caller guard, and an empty stored session_id keeps +# the backward-compat "matches any" semantics from find_active_loop. if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then GUARD_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") if [[ -n "$GUARD_STATE_FILE" ]]; then GUARD_STORED_SID=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$GUARD_STATE_FILE" 2>/dev/null | tr -d ' ') - # Non-empty stored session_id that differs from the caller's session - # (empty or not) means this is a foreign parked loop. Hook-input - # schemas that omit session_id -- such as rlcr-stop-gate.sh invoked - # without --session-id -- still get a mismatch here and take the - # safe exit path. An empty stored session_id keeps the existing - # backward-compat "matches any" semantics from find_active_loop. if [[ -n "$GUARD_STORED_SID" ]] \ + && [[ -n "$HOOK_SESSION_ID" ]] \ && [[ "$GUARD_STORED_SID" != "$HOOK_SESSION_ID" ]]; then jq -n \ '{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 # untouched -- the next natural stop (after background work finishes) will # re-enter this hook with no pending tasks and run the normal flow. # +# LOOP_START_TS confines the transcript scan to launches that actually +# happened during this loop; earlier session-wide bg activity cannot pin +# the loop. +# # This check MUST run before any other gate (phase detection, state parsing, # branch / plan / git-clean / summary / max-iter checks, Codex review). -if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH"; then - PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH") +if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS"; then + PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS") # Mark the loop as parked; allows the same session to resume later and # makes the cross-session guard above reachable if the user opens a # different Claude session in this repo before the bg task completes. @@ -142,7 +166,7 @@ fi # The check uses a single fresh call so we capture both the exit code # and the emptiness without double-running jq. if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then - if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" 2>/dev/null) \ + if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS" 2>/dev/null) \ && [[ -z "$PENDING_BG_CHECK" ]]; then rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true fi diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 930d6ffc..9fe2ba5b 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -541,12 +541,27 @@ fi # Verify the gate wrapper path with a tilde-form --transcript-path also # reaches the short-circuit. AC-9 uses an absolute transcript path; this # covers the same code path with a "~/..." form. +# +# Fresh fixture so the repo has no prior bg-pending.marker (AC-10 left +# one behind). The ambiguous-caller guard in the hook only silences the +# wrapper when a marker already exists; a clean repo falls through to +# the normal short-circuit so the systemMessage surfaces in the wrapper +# output. echo "Test AC-10c: rlcr-stop-gate.sh with '~/...' --transcript-path -> ALLOW" +AC10C_REPO="$TEST_DIR/ac10c" +create_full_fixture "$AC10C_REPO" > /dev/null +mkdir -p "$FAKE_HOME/session-data-c" +AC10C_TRANSCRIPT="$FAKE_HOME/session-data-c/ac10c.jsonl" +AC10C_LAUNCH=$(emit_tool_use_assistant "toolu_H2" "Agent" ',"description":"x","prompt":"x"') +AC10C_RESULT=$(emit_async_agent_launch_result "toolu_H2" "agent_pending_H2") +write_transcript "$AC10C_TRANSCRIPT" "$AC10C_LAUNCH" "$AC10C_RESULT" +AC10C_TILDE_PATH="~/session-data-c/ac10c.jsonl" + AC10C_OUT="$TEST_DIR/ac10c-out.txt" set +e ( - cd "$AC10_REPO" - HOME="$FAKE_HOME" "$GATE_SCRIPT" --transcript-path "$AC10_TILDE_PATH" + cd "$AC10C_REPO" + HOME="$FAKE_HOME" "$GATE_SCRIPT" --transcript-path "$AC10C_TILDE_PATH" ) > "$AC10C_OUT" 2>&1 AC10C_EXIT=$? set -e @@ -1053,11 +1068,14 @@ else fi # ---------------- AC-19 ---------------- -# Empty-session caller must still be treated as "foreign" for a parked -# loop whose stored session_id is non-empty. Real trigger: callers such -# as scripts/rlcr-stop-gate.sh invoked without --session-id reach the -# hook with no session_id key at all. -echo "Test AC-19: cross-session guard fires when hook input omits session_id" +# Empty-session caller + bg-pending.marker present: the caller might be +# the parked loop's owner invoking through a wrapper that didn't forward +# session_id, OR it might be a different session. The hook cannot tell +# them apart from the input, so the safe response is `exit 0` silently +# with no systemMessage and no on-disk mutation. The real Claude stop +# hook (which always has session_id populated) drives actual parking and +# cleanup. +echo "Test AC-19: ambiguous caller (empty session_id + marker) exits silently" AC19_REPO="$TEST_DIR/ac19" AC19_LOOP=$(create_full_fixture "$AC19_REPO") AC19_STATE="$AC19_LOOP/state.md" @@ -1087,9 +1105,6 @@ EOF_AC19 AC19_STATE_HASH_BEFORE=$(sha256sum "$AC19_STATE" | awk '{print $1}') : > "$AC19_MARKER" -# Transcript exists and is a readable, well-formed minimal record. The -# guard must rely on the stored-vs-current session_id mismatch alone, -# not on transcript readability, to detect the foreign-session case. AC19_TRANSCRIPT="$TRANSCRIPTS_DIR/ac19.jsonl" write_transcript "$AC19_TRANSCRIPT" '{"type":"user","message":{"role":"user","content":"hello"}}' @@ -1103,11 +1118,11 @@ if [[ "$RUN_EXIT_CODE" -eq 0 ]] \ && [[ ! -f "$RUN_MARKER" ]] \ && [[ -f "$AC19_MARKER" ]] \ && [[ "$AC19_STATE_HASH_BEFORE" == "$AC19_STATE_HASH_AFTER" ]] \ - && printf '%s' "$AC19_SYS_MSG" | grep -qi "parked"; then - pass "AC-19: empty hook session_id triggers 'parked' guard; marker and state preserved" + && [[ -z "$AC19_SYS_MSG" ]]; then + pass "AC-19: ambiguous caller exits silently; marker and state.md preserved" else - fail "AC-19: empty hook session_id triggers 'parked' guard; marker and state preserved" \ - "exit 0 + systemMessage matches /parked/ + marker stays + state.md byte-identical + no Codex" \ + fail "AC-19: ambiguous caller exits silently; marker and state.md preserved" \ + "exit 0 + no systemMessage + marker stays + state.md byte-identical + no Codex" \ "exit $RUN_EXIT_CODE, codex_marker=$(test -f "$RUN_MARKER" && echo present || echo missing), bg_marker=$(test -f "$AC19_MARKER" && echo present || echo missing), state_unchanged=$([[ "$AC19_STATE_HASH_BEFORE" == "$AC19_STATE_HASH_AFTER" ]] && echo yes || echo no), systemMessage='$AC19_SYS_MSG'; output: $RUN_OUTPUT" fi @@ -1161,5 +1176,177 @@ else "marker was removed" fi +# ---------------- AC-21 ---------------- +# Transcript scan boundary: the Claude transcript is session-wide and +# can contain background launches that predate the RLCR loop. The +# helper filters launch events by `.timestamp >= since_ts` (derived +# from the loop dir basename) so only launches made after the loop +# started count as pending. +echo "Test AC-21: pre-loop launches are filtered out by since_ts" +AC21_TRANSCRIPT="$TRANSCRIPTS_DIR/ac21.jsonl" + +# The loop boundary used throughout the suite's fixtures is +# 2026-03-01 00:00:00. Build two launches: one BEFORE that boundary +# (should be filtered) and one AFTER (should still count as pending). +AC21_PRE_LAUNCH=$(jq -c -n '{ + type:"user", + timestamp:"2026-02-28T10:00:00.000Z", + toolUseResult:{isAsync:true, agentId:"agent_pre_loop"} +}') +AC21_POST_LAUNCH=$(jq -c -n '{ + type:"user", + timestamp:"2026-03-01T10:00:00.000Z", + toolUseResult:{isAsync:true, agentId:"agent_in_loop"} +}') +write_transcript "$AC21_TRANSCRIPT" "$AC21_PRE_LAUNCH" "$AC21_POST_LAUNCH" + +AC21_SINCE="2026-03-01T00:00:00.000Z" +AC21_FILTERED=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + list_pending_background_task_ids "$AC21_TRANSCRIPT" "$AC21_SINCE" 2>/dev/null | sort -u +) +if [[ "$AC21_FILTERED" == "agent_in_loop" ]]; then + pass "AC-21: list_pending_background_task_ids filters launches before since_ts" +else + fail "AC-21: list_pending_background_task_ids filters launches before since_ts" \ + "only 'agent_in_loop' (pre-loop launch excluded)" "got: $AC21_FILTERED" +fi + +# AC-21b: confirm the derive helper produces the expected ISO-8601 form +# so real callers get a matching boundary. +AC21B_DERIVED=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + derive_loop_start_iso_ts "/tmp/.humanize/rlcr/2026-03-01_00-00-00" +) +if [[ "$AC21B_DERIVED" == "2026-03-01T00:00:00.000Z" ]]; then + pass "AC-21b: derive_loop_start_iso_ts emits ISO-8601 with .000Z suffix" +else + fail "AC-21b: derive_loop_start_iso_ts emits ISO-8601 with .000Z suffix" \ + "2026-03-01T00:00:00.000Z" "$AC21B_DERIVED" +fi + +# AC-21c: end-to-end through the stop hook. Pre-loop launch only -> hook +# must NOT short-circuit (no pending bg "belongs" to this loop). +echo "Test AC-21c: stop hook ignores pre-loop launches for this loop" +AC21C_REPO="$TEST_DIR/ac21c" +AC21C_LOOP=$(create_full_fixture "$AC21C_REPO") +AC21C_MARKER="$AC21C_LOOP/bg-pending.marker" +AC21C_TRANSCRIPT="$TRANSCRIPTS_DIR/ac21c.jsonl" +write_transcript "$AC21C_TRANSCRIPT" "$AC21_PRE_LAUNCH" +AC21C_INPUT=$(jq -c -n --arg tp "$AC21C_TRANSCRIPT" \ + '{transcript_path:$tp, session_id:"session_home"}') +run_stop_hook_with_input "$AC21C_REPO" "$AC21C_INPUT" + +# With the pre-loop launch filtered out, the transcript has no in-loop +# pending bg -> no short-circuit -> no marker written -> hook proceeds +# to the normal flow (which will call Codex in this fixture). +if [[ ! -f "$AC21C_MARKER" ]] && [[ -f "$RUN_MARKER" ]]; then + pass "AC-21c: pre-loop launch does not write bg-pending.marker; Codex runs" +else + fail "AC-21c: pre-loop launch does not write bg-pending.marker; Codex runs" \ + "no bg marker AND Codex invoked" \ + "bg_marker=$(test -f "$AC21C_MARKER" && echo present || echo missing); codex_marker=$(test -f "$RUN_MARKER" && echo present || echo missing)" +fi + +# ---------------- AC-22 ---------------- +# Wrapper without --session-id on a repo that has NO marker: should +# behave just like the normal same-session path, i.e. a pending bg in +# the transcript writes the marker and the wrapper output surfaces the +# "background task" systemMessage. This confirms the ambiguous-caller +# guard only fires on a pre-existing marker, not on every no-session +# call. +echo "Test AC-22: wrapper without session_id, no prior marker, pending bg -> ALLOW with systemMessage" +AC22_REPO="$TEST_DIR/ac22" +create_full_fixture "$AC22_REPO" > /dev/null +AC22_LOOP="$AC22_REPO/.humanize/rlcr/2026-03-01_00-00-00" +AC22_MARKER="$AC22_LOOP/bg-pending.marker" +AC22_TRANSCRIPT="$TRANSCRIPTS_DIR/ac22.jsonl" +AC22_LAUNCH=$(jq -c -n '{ + type:"user", + timestamp:"2026-03-01T10:00:00.000Z", + toolUseResult:{isAsync:true, agentId:"agent_wrapper_pending"} +}') +write_transcript "$AC22_TRANSCRIPT" "$AC22_LAUNCH" + +AC22_OUT="$TEST_DIR/ac22-out.txt" +set +e +( + cd "$AC22_REPO" + "$GATE_SCRIPT" --transcript-path "$AC22_TRANSCRIPT" +) > "$AC22_OUT" 2>&1 +AC22_EXIT=$? +set -e + +if [[ "$AC22_EXIT" -eq 0 ]] \ + && grep -q "^ALLOW:" "$AC22_OUT" \ + && grep -q "background task" "$AC22_OUT" \ + && [[ -f "$AC22_MARKER" ]]; then + pass "AC-22: wrapper without session_id + no prior marker + pending bg -> writes marker, surfaces systemMessage" +else + AC22_BODY=$(cat "$AC22_OUT" 2>/dev/null || true) + fail "AC-22: wrapper without session_id + no prior marker + pending bg -> writes marker, surfaces systemMessage" \ + "exit 0 + ALLOW + 'background task' + marker written" \ + "exit $AC22_EXIT; marker=$(test -f "$AC22_MARKER" && echo present || echo missing); output: $AC22_BODY" +fi + +# AC-22b: wrapper without --session-id on a repo that ALREADY has a +# marker (e.g. set up by a prior hook call). Must exit 0 silently -- no +# systemMessage, no state mutation. Mirrors the real scenario Codex +# flagged: rlcr-stop-gate.sh re-run by an unaware caller. +echo "Test AC-22b: wrapper without session_id, prior marker -> silent ALLOW" +AC22B_REPO="$TEST_DIR/ac22b" +AC22B_LOOP=$(create_full_fixture "$AC22B_REPO") +AC22B_STATE="$AC22B_LOOP/state.md" +AC22B_MARKER="$AC22B_LOOP/bg-pending.marker" +AC22B_BRANCH=$(git -C "$AC22B_REPO" rev-parse --abbrev-ref HEAD) +AC22B_BASE_COMMIT=$(git -C "$AC22B_REPO" rev-parse HEAD) +cat > "$AC22B_STATE" < "$AC22B_MARKER" + +AC22B_OUT="$TEST_DIR/ac22b-out.txt" +set +e +( + cd "$AC22B_REPO" + "$GATE_SCRIPT" +) > "$AC22B_OUT" 2>&1 +AC22B_EXIT=$? +set -e + +AC22B_STATE_HASH_AFTER=$(sha256sum "$AC22B_STATE" | awk '{print $1}') +if [[ "$AC22B_EXIT" -eq 0 ]] \ + && grep -q "^ALLOW:" "$AC22B_OUT" \ + && ! grep -qi "parked" "$AC22B_OUT" \ + && [[ -f "$AC22B_MARKER" ]] \ + && [[ "$AC22B_STATE_HASH_BEFORE" == "$AC22B_STATE_HASH_AFTER" ]]; then + pass "AC-22b: wrapper without session_id + existing marker -> silent ALLOW; marker and state preserved" +else + AC22B_BODY=$(cat "$AC22B_OUT" 2>/dev/null || true) + fail "AC-22b: wrapper without session_id + existing marker -> silent ALLOW; marker and state preserved" \ + "exit 0 + ALLOW: (no 'parked') + marker kept + state.md byte-identical" \ + "exit $AC22B_EXIT; marker=$(test -f "$AC22B_MARKER" && echo present || echo missing); state_unchanged=$([[ "$AC22B_STATE_HASH_BEFORE" == "$AC22B_STATE_HASH_AFTER" ]] && echo yes || echo no); output: $AC22B_BODY" +fi + print_test_summary "Stop Hook Background-Task Allow Test Summary" exit $? From 9c80698f1354d6a1fc4b762acb2328a4526eb00f Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 15:34:59 -0700 Subject: [PATCH 10/13] Normalize loop-start boundary into actual UTC setup-rlcr-loop.sh creates loop dirs named YYYY-MM-DD_HH-MM-SS with `date +%Y-%m-%d_%H-%M-%S` (no -u), so the basename is local wall clock. The previous derive_loop_start_iso_ts pasted `.000Z` on top of that basename, treating the local wall clock as if it were already UTC. Claude transcript events carry real UTC timestamps, so on any non-UTC machine the filter boundary was shifted by the local offset: west-of-UTC users inherited pre-loop background launches and stayed parked forever, east-of-UTC users missed in-loop background work and ran Codex review too early. Fix in hooks/lib/loop-common.sh: * derive_loop_start_iso_ts now does a two-step local -> epoch -> UTC conversion. Local wall clock is parsed into epoch seconds with `date -d` (GNU) or `date -j -f` (BSD/macOS), then the epoch is formatted in UTC as YYYY-MM-DDTHH:MM:SS.000Z with `date -u -d "@"` (GNU) or `date -u -r ` (BSD/macOS). Any failure yields an empty string, which disables the filter in callers -- same backward-compat behaviour as before. Regressions in tests/test-stop-hook-bg-allow.sh: AC-21b pinned to `export TZ=UTC` inside its subshell so the expected 2026-03-01T00:00:00.000Z is TZ-deterministic. AC-21d NEW. TZ=Asia/Tokyo + basename 2026-03-01_09-00-00 -> 2026-03-01T00:00:00.000Z (9am JST = 0am UTC). AC-21e NEW. TZ=America/Los_Angeles + basename 2026-03-01_00-00-00 -> 2026-03-01T08:00:00.000Z (0am PST = 8am UTC; March 1 is before DST starts on March 8, 2026). Validation: - bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - bash tests/run-all-tests.sh -> 1718 passed, 0 failed - TZ=America/Los_Angeles bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - TZ=Asia/Tokyo bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed systemMessage wording unchanged. Version stays at 1.16.0. --- hooks/lib/loop-common.sh | 52 +++++++++++++++++++++++--------- tests/test-stop-hook-bg-allow.sh | 39 ++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 17 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 2b71e778..7e18df9b 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -268,28 +268,52 @@ extract_transcript_path() { } # Convert an RLCR loop dir basename to a lexically-comparable ISO-8601 -# timestamp suitable for filtering transcript events. +# UTC timestamp suitable for filtering transcript events. # -# The setup script creates loop dirs named `YYYY-MM-DD_HH-MM-SS`; real -# Claude transcript events carry timestamps like `2026-04-16T13:19:26.819Z`. -# String comparison works cleanly once we pad the loop boundary with -# `.000Z` so sub-second transcript timestamps in the same second always -# compare greater. +# `setup-rlcr-loop.sh` creates loop dirs named `YYYY-MM-DD_HH-MM-SS` in +# the system's LOCAL wall clock (it calls `date +%Y-%m-%d_%H-%M-%S` +# without `-u`). Claude transcript events carry actual UTC timestamps +# like `2026-04-16T13:19:26.819Z`. To compare them correctly, this +# helper converts the local wall-clock parse back to a real UTC moment +# via a two-step: parse local -> epoch seconds -> format in UTC. +# +# The `.000Z` suffix keeps sub-second transcript timestamps in the same +# second compared greater via lexical string ordering. # # Usage: derive_loop_start_iso_ts "$loop_dir" -# Prints the ISO-8601 timestamp, or empty string when the basename does -# not match the expected format. +# Prints the ISO-8601 UTC timestamp, or empty string when the +# basename does not match the expected format or the local `date` +# binary cannot parse it. derive_loop_start_iso_ts() { local loop_dir="$1" local base base=$(basename "$loop_dir" 2>/dev/null || echo "") - if [[ "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then - printf '%sT%s:%s:%s.000Z' \ - "${BASH_REMATCH[1]}" \ - "${BASH_REMATCH[2]}" \ - "${BASH_REMATCH[3]}" \ - "${BASH_REMATCH[4]}" + if [[ ! "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then + return + fi + local local_datetime + local_datetime="${BASH_REMATCH[1]} ${BASH_REMATCH[2]}:${BASH_REMATCH[3]}:${BASH_REMATCH[4]}" + + # Local wall-clock -> epoch seconds. GNU `date -d` first, + # BSD/macOS `date -j -f ...` second. Both honour the caller's TZ + # for interpretation, matching setup-rlcr-loop.sh's behaviour at + # loop-dir creation time. + local epoch + epoch=$(date -d "$local_datetime" +%s 2>/dev/null) || epoch="" + if [[ -z "$epoch" ]]; then + epoch=$(date -j -f "%Y-%m-%d %H:%M:%S" "$local_datetime" +%s 2>/dev/null) || epoch="" + fi + if [[ -z "$epoch" ]]; then + return + fi + + # Epoch -> UTC ISO-8601. Try GNU then BSD. + local utc_iso + utc_iso=$(date -u -d "@$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" + if [[ -z "$utc_iso" ]]; then + utc_iso=$(date -u -r "$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" fi + printf '%s' "$utc_iso" } # Enumerate background-task ids that have been launched but not yet marked diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 9fe2ba5b..22d0a1dc 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -1214,19 +1214,52 @@ else fi # AC-21b: confirm the derive helper produces the expected ISO-8601 form -# so real callers get a matching boundary. +# under TZ=UTC, where local wall clock == UTC so no offset is applied. AC21B_DERIVED=$( # shellcheck source=/dev/null source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + export TZ="UTC" derive_loop_start_iso_ts "/tmp/.humanize/rlcr/2026-03-01_00-00-00" ) if [[ "$AC21B_DERIVED" == "2026-03-01T00:00:00.000Z" ]]; then - pass "AC-21b: derive_loop_start_iso_ts emits ISO-8601 with .000Z suffix" + pass "AC-21b: derive_loop_start_iso_ts under TZ=UTC preserves the wall-clock" else - fail "AC-21b: derive_loop_start_iso_ts emits ISO-8601 with .000Z suffix" \ + fail "AC-21b: derive_loop_start_iso_ts under TZ=UTC preserves the wall-clock" \ "2026-03-01T00:00:00.000Z" "$AC21B_DERIVED" fi +# AC-21d: setup-rlcr-loop.sh names the dir with local wall clock, so a +# non-UTC caller must see the boundary shifted into actual UTC. +# JST (UTC+9) example: 09:00 JST == 00:00 UTC. +AC21D_DERIVED=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + export TZ="Asia/Tokyo" + derive_loop_start_iso_ts "/tmp/.humanize/rlcr/2026-03-01_09-00-00" +) +if [[ "$AC21D_DERIVED" == "2026-03-01T00:00:00.000Z" ]]; then + pass "AC-21d: derive_loop_start_iso_ts converts JST wall-clock to correct UTC" +else + fail "AC-21d: derive_loop_start_iso_ts converts JST wall-clock to correct UTC" \ + "2026-03-01T00:00:00.000Z (9am JST = 0am UTC)" "$AC21D_DERIVED" +fi + +# AC-21e: PST (UTC-8) example. Pick March 1 which is still PST (DST +# does not start until March 8, 2026), so the offset is a fixed -8h: +# 00:00 PST == 08:00 UTC. +AC21E_DERIVED=$( + # shellcheck source=/dev/null + source "$PROJECT_ROOT/hooks/lib/loop-common.sh" + export TZ="America/Los_Angeles" + derive_loop_start_iso_ts "/tmp/.humanize/rlcr/2026-03-01_00-00-00" +) +if [[ "$AC21E_DERIVED" == "2026-03-01T08:00:00.000Z" ]]; then + pass "AC-21e: derive_loop_start_iso_ts converts PST wall-clock to correct UTC" +else + fail "AC-21e: derive_loop_start_iso_ts converts PST wall-clock to correct UTC" \ + "2026-03-01T08:00:00.000Z (0am PST = 8am UTC before DST)" "$AC21E_DERIVED" +fi + # AC-21c: end-to-end through the stop hook. Pre-loop launch only -> hook # must NOT short-circuit (no pending bg "belongs" to this loop). echo "Test AC-21c: stop hook ignores pre-loop launches for this loop" From 74671f783ea362744ec22b8eaa4f0b75a491715b Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 15:45:24 -0700 Subject: [PATCH 11/13] Pin rlcr-stop-gate project root at every wrapper testsite tests/test-stop-hook-bg-allow.sh invokes rlcr-stop-gate.sh at four spots (AC-9, AC-10c, AC-22, AC-22b). The wrapper resolves its project root as `${CLAUDE_PROJECT_DIR:-$(pwd)}`, giving the env var precedence over `cd`. When the outer runner exports CLAUDE_PROJECT_DIR (the normal case in hosted environments), those four tests were inspecting the outer repo instead of their per-test fixtures, falling through with "ALLOW: stop gate passed." and causing the suite to go red inside tests/run-all-tests.sh. Reproduced directly: CLAUDE_PROJECT_DIR=/tmp/outer-unrelated \ bash tests/test-stop-hook-bg-allow.sh ... FAIL: AC-10c FAIL: AC-22 Fix: pass `--project-root "$FIXTURE_REPO"` at every wrapper call site. The wrapper priority order is explicit flag > CLAUDE_PROJECT_DIR env > cwd, so the gate now pins deterministically to each fixture regardless of inherited environment. No product-code change. Validation: - bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - bash tests/run-all-tests.sh -> 1718 passed, 0 failed - CLAUDE_PROJECT_DIR=/tmp/outer-unrelated bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - HOME=/nonexistent/readonly bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - TZ=America/Los_Angeles bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - TZ=Asia/Tokyo bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed systemMessage wording unchanged. Version stays at 1.16.0. --- tests/test-stop-hook-bg-allow.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/test-stop-hook-bg-allow.sh b/tests/test-stop-hook-bg-allow.sh index 22d0a1dc..ab9fe571 100755 --- a/tests/test-stop-hook-bg-allow.sh +++ b/tests/test-stop-hook-bg-allow.sh @@ -476,10 +476,12 @@ AC9_RESULT=$(emit_async_agent_launch_result "toolu_G" "agent_pending_G") write_transcript "$AC9_TRANSCRIPT" "$AC9_LAUNCH" "$AC9_RESULT" AC9_OUT="$AC9_REPO/gate-out.txt" +# Pass --project-root explicitly so an inherited CLAUDE_PROJECT_DIR +# from the outer runner cannot redirect the gate to the outer repo. set +e ( cd "$AC9_REPO" - "$GATE_SCRIPT" --transcript-path "$AC9_TRANSCRIPT" + "$GATE_SCRIPT" --project-root "$AC9_REPO" --transcript-path "$AC9_TRANSCRIPT" ) > "$AC9_OUT" 2>&1 AC9_EXIT=$? set -e @@ -561,7 +563,9 @@ AC10C_OUT="$TEST_DIR/ac10c-out.txt" set +e ( cd "$AC10C_REPO" - HOME="$FAKE_HOME" "$GATE_SCRIPT" --transcript-path "$AC10C_TILDE_PATH" + HOME="$FAKE_HOME" "$GATE_SCRIPT" \ + --project-root "$AC10C_REPO" \ + --transcript-path "$AC10C_TILDE_PATH" ) > "$AC10C_OUT" 2>&1 AC10C_EXIT=$? set -e @@ -1307,7 +1311,7 @@ AC22_OUT="$TEST_DIR/ac22-out.txt" set +e ( cd "$AC22_REPO" - "$GATE_SCRIPT" --transcript-path "$AC22_TRANSCRIPT" + "$GATE_SCRIPT" --project-root "$AC22_REPO" --transcript-path "$AC22_TRANSCRIPT" ) > "$AC22_OUT" 2>&1 AC22_EXIT=$? set -e @@ -1362,7 +1366,7 @@ AC22B_OUT="$TEST_DIR/ac22b-out.txt" set +e ( cd "$AC22B_REPO" - "$GATE_SCRIPT" + "$GATE_SCRIPT" --project-root "$AC22B_REPO" ) > "$AC22B_OUT" 2>&1 AC22B_EXIT=$? set -e From 39f09c47d7089bbea4f3df75977ab42c5312bb66 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 15:55:02 -0700 Subject: [PATCH 12/13] Avoid duplicate jq pass in bg-pending short-circuit hooks/loop-codex-stop-hook.sh previously called has_pending_background_tasks AND count_pending_background_tasks back-to-back on the short-circuit path. Both wrap list_pending_background_task_ids, so every pending-bg stop was running jq over the transcript twice. Collapse into a single list_pending_background_task_ids call: capture the id list once, use its non-emptiness for the short-circuit decision, and derive the count with `wc -l`. The public helpers has_pending_background_tasks and count_pending_background_tasks stay for other callers. No behaviour change. systemMessage wording and exit codes identical. Validation (all six suites green): - bash tests/run-all-tests.sh -> 1718 passed, 0 failed - bash tests/test-stop-hook-bg-allow.sh -> 39 passed, 0 failed - HOME=/nonexistent/readonly ... -> 39 passed, 0 failed - TZ=America/Los_Angeles ... -> 39 passed, 0 failed - TZ=Asia/Tokyo ... -> 39 passed, 0 failed - CLAUDE_PROJECT_DIR=/tmp/outer-unrelated ... -> 39 passed, 0 failed Version stays at 1.16.0. --- hooks/loop-codex-stop-hook.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index 405c82ef..6779f94e 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -137,8 +137,9 @@ fi # # This check MUST run before any other gate (phase detection, state parsing, # branch / plan / git-clean / summary / max-iter checks, Codex review). -if has_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS"; then - PENDING_BG_COUNT=$(count_pending_background_tasks "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS") +PENDING_BG_IDS=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS" 2>/dev/null) || true +if [[ -n "$PENDING_BG_IDS" ]]; then + PENDING_BG_COUNT=$(printf '%s\n' "$PENDING_BG_IDS" | sed '/^$/d' | wc -l | tr -d ' ') # Mark the loop as parked; allows the same session to resume later and # makes the cross-session guard above reachable if the user opens a # different Claude session in this repo before the bg task completes. From 3711e5fd9059584c7bf98cf1d19ee02dcf5bef48 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 16:47:36 -0700 Subject: [PATCH 13/13] Extract bg-task helpers into dedicated lib and guard transcript_path survival Move the six background-task helpers (expand_leading_tilde, extract_transcript_path, derive_loop_start_iso_ts, list/has/count_pending_background_task[_ids]) and the four hook guard blocks (ambiguous caller, cross-session parked, pending-bg short-circuit, stale-marker cleanup) out of hooks/lib/loop-common.sh and hooks/loop-codex-stop-hook.sh into a new hooks/lib/loop-bg-tasks.sh. The stop hook now delegates to a single handle_bg_task_short_circuit entry point; loop-common.sh sources the new lib so every existing consumer continues to see the helpers transparently. Add a regression test to tests/test-stop-gate.sh that asserts transcript_path is still forwarded to the hook when session_id is empty, freezing the jq object-collapse fix that replaced plain select(length > 0) field values with explicit if/then/else nulls. --- hooks/lib/loop-bg-tasks.sh | 363 ++++++++++++++++++++++++++++++++++ hooks/lib/loop-common.sh | 220 ++------------------- hooks/loop-codex-stop-hook.sh | 115 ++--------- tests/test-stop-gate.sh | 75 +++++++ 4 files changed, 467 insertions(+), 306 deletions(-) create mode 100755 hooks/lib/loop-bg-tasks.sh diff --git a/hooks/lib/loop-bg-tasks.sh b/hooks/lib/loop-bg-tasks.sh new file mode 100755 index 00000000..116e9e36 --- /dev/null +++ b/hooks/lib/loop-bg-tasks.sh @@ -0,0 +1,363 @@ +#!/usr/bin/env bash +# +# Background-task helpers for the RLCR stop hook. +# +# Owns all logic that inspects the Claude Code transcript to decide +# whether the hook should short-circuit (the main session is still +# waiting on an asynchronous Agent/Bash dispatch), plus the four guard +# blocks that the stop hook runs before its normal gate logic: +# +# 1. Ambiguous-caller marker guard +# 2. Cross-session parked-loop guard +# 3. Early exit: pending background tasks +# 4. Same-session stale-marker cleanup +# +# Depends on loop-common.sh (FIELD_SESSION_ID, resolve_active_state_file) +# being sourced first. +# + +# Source guard. +[[ -n "${_LOOP_BG_TASKS_LOADED:-}" ]] && return 0 2>/dev/null || true +_LOOP_BG_TASKS_LOADED=1 + +# Expand a leading "~" or "~/" in a path to "$HOME" without using eval. +# Only the bare "~" and "~/..." forms are expanded; "~user/..." and every +# other input (absolute path, relative path, empty string) is returned verbatim. +# +# Usage: expand_leading_tilde "$path" +# Prints the normalized path to stdout. +expand_leading_tilde() { + local path="$1" + case "$path" in + '~') printf '%s' "${HOME:-}" ;; + '~/'*) printf '%s/%s' "${HOME:-}" "${path#'~/'}" ;; + *) printf '%s' "$path" ;; + esac +} + +# Extract transcript_path from hook JSON input and expand any leading tilde. +# Usage: extract_transcript_path "$json_input" +# Outputs the transcript_path to stdout, or empty string if not available. +extract_transcript_path() { + local input="$1" + local raw + raw=$(printf '%s' "$input" | jq -r '.transcript_path // empty' 2>/dev/null || echo "") + expand_leading_tilde "$raw" +} + +# Convert an RLCR loop dir basename to a lexically-comparable ISO-8601 +# UTC timestamp suitable for filtering transcript events. +# +# `setup-rlcr-loop.sh` creates loop dirs named `YYYY-MM-DD_HH-MM-SS` in +# the system's LOCAL wall clock (it calls `date +%Y-%m-%d_%H-%M-%S` +# without `-u`). Claude transcript events carry actual UTC timestamps +# like `2026-04-16T13:19:26.819Z`. To compare them correctly, this +# helper converts the local wall-clock parse back to a real UTC moment +# via a two-step: parse local -> epoch seconds -> format in UTC. +# +# The `.000Z` suffix keeps sub-second transcript timestamps in the same +# second compared greater via lexical string ordering. +# +# Usage: derive_loop_start_iso_ts "$loop_dir" +# Prints the ISO-8601 UTC timestamp, or empty string when the +# basename does not match the expected format or the local `date` +# binary cannot parse it. +derive_loop_start_iso_ts() { + local loop_dir="$1" + local base + base=$(basename "$loop_dir" 2>/dev/null || echo "") + if [[ ! "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then + return + fi + local local_datetime + local_datetime="${BASH_REMATCH[1]} ${BASH_REMATCH[2]}:${BASH_REMATCH[3]}:${BASH_REMATCH[4]}" + + # Local wall-clock -> epoch seconds. GNU `date -d` first, + # BSD/macOS `date -j -f ...` second. Both honour the caller's TZ + # for interpretation, matching setup-rlcr-loop.sh's behaviour at + # loop-dir creation time. + local epoch + epoch=$(date -d "$local_datetime" +%s 2>/dev/null) || epoch="" + if [[ -z "$epoch" ]]; then + epoch=$(date -j -f "%Y-%m-%d %H:%M:%S" "$local_datetime" +%s 2>/dev/null) || epoch="" + fi + if [[ -z "$epoch" ]]; then + return + fi + + # Epoch -> UTC ISO-8601. Try GNU then BSD. + local utc_iso + utc_iso=$(date -u -d "@$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" + if [[ -z "$utc_iso" ]]; then + utc_iso=$(date -u -r "$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" + fi + printf '%s' "$utc_iso" +} + +# Enumerate background-task ids that have been launched but not yet marked +# completed in a Claude Code transcript.jsonl. +# +# Launch events (inspected in tool_result "user" messages): +# - Background subagent: toolUseResult.isAsync == true +# -> id is toolUseResult.agentId +# - Background shell: toolUseResult.backgroundTaskId non-empty +# -> id is toolUseResult.backgroundTaskId +# +# Completion events are recognised from two Claude Code transcript forms: +# +# 1. Structured SDK record +# (see SDKTaskNotificationMessage in docs/typescript.md): +# `type == "system"`, `subtype == "task_notification"`, +# `task_id` is the completed id. Any `status` value +# (completed, failed, stopped, ...) is treated as terminal. +# +# 2. Legacy queue-operation enqueue whose `content` embeds a +# `` XML block with `...`; +# kept for transcripts produced by older Claude Code versions. +# +# pending := launched \ completed +# +# Optional second argument `since_ts` (ISO-8601 string, e.g. the value +# returned by `derive_loop_start_iso_ts`): when provided, only launch +# events whose top-level `.timestamp` field is >= `since_ts` count as +# candidate launches. Events without a `.timestamp` are included (keeps +# fixture transcripts and older record formats working). This keeps +# pre-loop session-wide background work from pinning an RLCR loop that +# has no pending work of its own. +# +# Usage: list_pending_background_task_ids "$transcript_path" [since_ts] +# - Outputs one id per line on stdout (possibly empty). +# - Returns 0 when the transcript is readable (including when there are +# no pending tasks). Returns 1 when the transcript path is empty, not +# a regular file, or jq is unavailable, so callers must treat non-zero +# as "unknown -> do not short-circuit". +list_pending_background_task_ids() { + local transcript_path="$1" + local since_ts="${2:-}" + + # Normalize a leading tilde so direct callers (tests, ad-hoc scripts) + # work correctly even when transcript_path was not routed through + # extract_transcript_path. + transcript_path=$(expand_leading_tilde "$transcript_path") + + if [[ -z "$transcript_path" ]] || [[ ! -f "$transcript_path" ]]; then + return 1 + fi + if ! command -v jq >/dev/null 2>&1; then + return 1 + fi + + local launched completed + launched=$(jq -r --arg since_ts "$since_ts" ' + select(.toolUseResult != null) + | select( + ($since_ts == "" + or ((.timestamp // "") == "") + or ((.timestamp // "") >= $since_ts)) + ) + | select( + (.toolUseResult.isAsync == true and (.toolUseResult.agentId // "") != "") + or ((.toolUseResult.backgroundTaskId // "") != "") + ) + | (.toolUseResult.agentId // .toolUseResult.backgroundTaskId) + ' "$transcript_path" 2>/dev/null | sort -u) || return 1 + + # Union of both completion formats. Either source alone is enough to + # mark a launched id terminal. + # + # The `grep -oE || true` guard on the legacy branch keeps `set -o + # pipefail` from poisoning the combined pipeline when no legacy + # queue-operation records exist in the transcript (grep with `-o` + # exits 1 on no matches, which would otherwise wipe out any SDK + # task_notification results collected above). + completed=$( + { + jq -r ' + select(.type == "system" and .subtype == "task_notification") + | (.task_id // empty) + ' "$transcript_path" 2>/dev/null + jq -r ' + select(.type == "queue-operation" and .operation == "enqueue") + | (.content // "" | tostring) + | select(contains("")) + ' "$transcript_path" 2>/dev/null \ + | { grep -oE '[^<]+' || true; } \ + | sed -E 's|||g' + } | sort -u | sed '/^$/d' + ) || completed="" + + # Emit launched ids that have no matching completion notification. + comm -23 \ + <(printf '%s\n' "$launched" | sed '/^$/d') \ + <(printf '%s\n' "$completed" | sed '/^$/d') +} + +# Returns 0 when the transcript shows at least one pending background task. +# Returns 1 when no pending tasks are detected (including fail-closed cases +# like missing transcript, non-file path, or jq unavailable). +# +# Usage: has_pending_background_tasks "$transcript_path" [since_ts] +has_pending_background_tasks() { + local transcript_path="$1" + local since_ts="${2:-}" + local pending + pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || return 1 + [[ -n "$pending" ]] +} + +# Prints the count of pending background tasks to stdout. Prints 0 for any +# error case so callers can still format messages safely. +# +# Usage: count_pending_background_tasks "$transcript_path" [since_ts] +count_pending_background_tasks() { + local transcript_path="$1" + local since_ts="${2:-}" + local pending + pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || { + echo 0 + return 0 + } + if [[ -z "$pending" ]]; then + echo 0 + else + printf '%s\n' "$pending" | sed '/^$/d' | wc -l | tr -d ' ' + fi +} + +# Single entry point for the stop hook: runs the four guard blocks +# (ambiguous-caller, cross-session parked, pending-bg short-circuit, +# same-session stale-marker cleanup) in order. When a guard decides to +# short-circuit the stop hook, it emits the appropriate JSON on stdout +# and `exit 0`s directly; the caller (sourcing the hook script) never +# returns. When no guard fires, this function returns 0 and the stop +# hook continues into its normal gate logic. +# +# Depends on FIELD_SESSION_ID and resolve_active_state_file from +# loop-common.sh. +# +# Usage: handle_bg_task_short_circuit "$LOOP_DIR" "$HOOK_INPUT" "$HOOK_SESSION_ID" +handle_bg_task_short_circuit() { + local loop_dir="$1" hook_input="$2" hook_session_id="$3" + + # Shared state used by the guard blocks below. + # Loop-start boundary: derived from the loop dir basename + # (`YYYY-MM-DD_HH-MM-SS`). Empty means derivation failed; helpers + # treat empty since_ts as no boundary. + local loop_start_ts transcript_path + loop_start_ts=$(derive_loop_start_iso_ts "$loop_dir") + transcript_path=$(extract_transcript_path "$hook_input") + + # ---------------------------------------- + # Ambiguous-Caller Marker Guard + # ---------------------------------------- + # If a bg-pending.marker is present but we have no session_id on + # this hook invocation (typical of scripts/rlcr-stop-gate.sh + # invoked without --session-id, or any other caller that doesn't + # forward session_id), we cannot tell whether this caller owns the + # parked loop. Taking either branch (foreign-session guard below, + # or same-session cleanup further down) would be wrong in one of + # the two possible realities. Exit 0 silently: the real Claude + # hook will arrive with session_id populated and drive parking / + # cleanup from an authoritative context. + if [[ -f "$loop_dir/bg-pending.marker" ]] && [[ -z "$hook_session_id" ]]; then + exit 0 + fi + + # ---------------------------------------- + # Cross-Session Parked-Loop Guard + # ---------------------------------------- + # If find_active_loop handed this dir over via the marker fallback, + # the loop is parked by a different session waiting on a background + # task. The current session has no authority to inspect or advance + # that loop - its transcript sees none of the foreign bg activity - + # so the only safe response is to exit 0 with a distinct + # systemMessage and leave every on-disk artifact (state file, + # stored session_id, marker) untouched. + # + # Both sides of the session-id comparison must be non-empty for + # this branch to trigger: an empty hook_session_id has already + # exited above via the ambiguous-caller guard, and an empty stored + # session_id keeps the backward-compat "matches any" semantics + # from find_active_loop. + if [[ -f "$loop_dir/bg-pending.marker" ]]; then + local guard_state_file guard_stored_sid + guard_state_file=$(resolve_active_state_file "$loop_dir") + if [[ -n "$guard_state_file" ]]; then + guard_stored_sid=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$guard_state_file" 2>/dev/null | tr -d ' ') + if [[ -n "$guard_stored_sid" ]] \ + && [[ -n "$hook_session_id" ]] \ + && [[ "$guard_stored_sid" != "$hook_session_id" ]]; then + jq -n \ + '{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."}' + exit 0 + fi + fi + fi + + # ---------------------------------------- + # Early Exit: Pending Background Tasks + # ---------------------------------------- + # When the main Claude Code session has dispatched background work + # (Agent with run_in_background=true, or Bash with + # run_in_background=true) whose completion notifications have not + # yet arrived, the natural "stop" is simply "I am waiting for the + # background task". Running git/summary/BitLesson/Codex gates in + # that state wastes Codex tokens and produces low-signal reviews. + # + # Allow the stop (exit 0) and emit a user-visible systemMessage so + # nobody mistakes the pause for loop completion. The on-disk loop + # state is left untouched -- the next natural stop (after + # background work finishes) will re-enter this hook with no + # pending tasks and run the normal flow. + # + # loop_start_ts confines the transcript scan to launches that + # actually happened during this loop; earlier session-wide bg + # activity cannot pin the loop. + # + # This check MUST run before any other gate (phase detection, + # state parsing, branch / plan / git-clean / summary / max-iter + # checks, Codex review). + local pending_bg_ids + pending_bg_ids=$(list_pending_background_task_ids "$transcript_path" "$loop_start_ts" 2>/dev/null) || true + if [[ -n "$pending_bg_ids" ]]; then + local pending_bg_count + pending_bg_count=$(printf '%s\n' "$pending_bg_ids" | sed '/^$/d' | wc -l | tr -d ' ') + # Mark the loop as parked; allows the same session to resume + # later and makes the cross-session guard above reachable if + # the user opens a different Claude session in this repo + # before the bg task completes. + : > "$loop_dir/bg-pending.marker" 2>/dev/null || true + jq -n --arg count "$pending_bg_count" \ + '{systemMessage: ("RLCR loop active. " + $count + " background task(s) still running - stop allowed naturally; loop has NOT terminated and will resume on completion.")}' + exit 0 + fi + + # ---------------------------------------- + # Same-Session Stale-Marker Cleanup + # ---------------------------------------- + # The cross-session guard above already exited for every foreign + # session, so reaching here with the marker present means the + # CURRENT session parked the loop and has now come back with a + # transcript showing no pending bg events. Remove the stale marker + # before the normal flow takes over. + # + # Two-part guard to make sure we never drop the parked-state + # signal without evidence: + # (a) list_pending_background_task_ids returned exit 0 -- the + # transcript was present, readable, AND parsed successfully. + # The helper is fail-closed on missing files, empty paths, + # jq parse failure, and truncation, so a non-zero exit + # blocks cleanup here even when the transcript "file" + # exists. + # (b) its output is empty -- proves "no pending" was + # authoritatively verified, not inferred from a failure. + # The check uses a single fresh call so we capture both the exit + # code and the emptiness without double-running jq. + if [[ -f "$loop_dir/bg-pending.marker" ]]; then + local pending_bg_check + if pending_bg_check=$(list_pending_background_task_ids "$transcript_path" "$loop_start_ts" 2>/dev/null) \ + && [[ -z "$pending_bg_check" ]]; then + rm -f "$loop_dir/bg-pending.marker" 2>/dev/null || true + fi + fi +} diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index 7e18df9b..374a5d30 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -242,209 +242,11 @@ extract_session_id() { printf '%s' "$input" | jq -r '.session_id // empty' 2>/dev/null || echo "" } -# Expand a leading "~" or "~/" in a path to "$HOME" without using eval. -# Only the bare "~" and "~/..." forms are expanded; "~user/..." and every -# other input (absolute path, relative path, empty string) is returned verbatim. -# -# Usage: expand_leading_tilde "$path" -# Prints the normalized path to stdout. -expand_leading_tilde() { - local path="$1" - case "$path" in - '~') printf '%s' "${HOME:-}" ;; - '~/'*) printf '%s/%s' "${HOME:-}" "${path#'~/'}" ;; - *) printf '%s' "$path" ;; - esac -} - -# Extract transcript_path from hook JSON input and expand any leading tilde. -# Usage: extract_transcript_path "$json_input" -# Outputs the transcript_path to stdout, or empty string if not available. -extract_transcript_path() { - local input="$1" - local raw - raw=$(printf '%s' "$input" | jq -r '.transcript_path // empty' 2>/dev/null || echo "") - expand_leading_tilde "$raw" -} - -# Convert an RLCR loop dir basename to a lexically-comparable ISO-8601 -# UTC timestamp suitable for filtering transcript events. -# -# `setup-rlcr-loop.sh` creates loop dirs named `YYYY-MM-DD_HH-MM-SS` in -# the system's LOCAL wall clock (it calls `date +%Y-%m-%d_%H-%M-%S` -# without `-u`). Claude transcript events carry actual UTC timestamps -# like `2026-04-16T13:19:26.819Z`. To compare them correctly, this -# helper converts the local wall-clock parse back to a real UTC moment -# via a two-step: parse local -> epoch seconds -> format in UTC. -# -# The `.000Z` suffix keeps sub-second transcript timestamps in the same -# second compared greater via lexical string ordering. -# -# Usage: derive_loop_start_iso_ts "$loop_dir" -# Prints the ISO-8601 UTC timestamp, or empty string when the -# basename does not match the expected format or the local `date` -# binary cannot parse it. -derive_loop_start_iso_ts() { - local loop_dir="$1" - local base - base=$(basename "$loop_dir" 2>/dev/null || echo "") - if [[ ! "$base" =~ ^([0-9]{4}-[0-9]{2}-[0-9]{2})_([0-9]{2})-([0-9]{2})-([0-9]{2})$ ]]; then - return - fi - local local_datetime - local_datetime="${BASH_REMATCH[1]} ${BASH_REMATCH[2]}:${BASH_REMATCH[3]}:${BASH_REMATCH[4]}" - - # Local wall-clock -> epoch seconds. GNU `date -d` first, - # BSD/macOS `date -j -f ...` second. Both honour the caller's TZ - # for interpretation, matching setup-rlcr-loop.sh's behaviour at - # loop-dir creation time. - local epoch - epoch=$(date -d "$local_datetime" +%s 2>/dev/null) || epoch="" - if [[ -z "$epoch" ]]; then - epoch=$(date -j -f "%Y-%m-%d %H:%M:%S" "$local_datetime" +%s 2>/dev/null) || epoch="" - fi - if [[ -z "$epoch" ]]; then - return - fi - - # Epoch -> UTC ISO-8601. Try GNU then BSD. - local utc_iso - utc_iso=$(date -u -d "@$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" - if [[ -z "$utc_iso" ]]; then - utc_iso=$(date -u -r "$epoch" "+%Y-%m-%dT%H:%M:%S.000Z" 2>/dev/null) || utc_iso="" - fi - printf '%s' "$utc_iso" -} - -# Enumerate background-task ids that have been launched but not yet marked -# completed in a Claude Code transcript.jsonl. -# -# Launch events (inspected in tool_result "user" messages): -# - Background subagent: toolUseResult.isAsync == true -# -> id is toolUseResult.agentId -# - Background shell: toolUseResult.backgroundTaskId non-empty -# -> id is toolUseResult.backgroundTaskId -# -# Completion events are recognised from two Claude Code transcript forms: -# -# 1. Structured SDK record -# (see SDKTaskNotificationMessage in docs/typescript.md): -# `type == "system"`, `subtype == "task_notification"`, -# `task_id` is the completed id. Any `status` value -# (completed, failed, stopped, ...) is treated as terminal. -# -# 2. Legacy queue-operation enqueue whose `content` embeds a -# `` XML block with `...`; -# kept for transcripts produced by older Claude Code versions. -# -# pending := launched \ completed -# -# Optional second argument `since_ts` (ISO-8601 string, e.g. the value -# returned by `derive_loop_start_iso_ts`): when provided, only launch -# events whose top-level `.timestamp` field is >= `since_ts` count as -# candidate launches. Events without a `.timestamp` are included (keeps -# fixture transcripts and older record formats working). This keeps -# pre-loop session-wide background work from pinning an RLCR loop that -# has no pending work of its own. -# -# Usage: list_pending_background_task_ids "$transcript_path" [since_ts] -# - Outputs one id per line on stdout (possibly empty). -# - Returns 0 when the transcript is readable (including when there are -# no pending tasks). Returns 1 when the transcript path is empty, not -# a regular file, or jq is unavailable, so callers must treat non-zero -# as "unknown -> do not short-circuit". -list_pending_background_task_ids() { - local transcript_path="$1" - local since_ts="${2:-}" - - # Normalize a leading tilde so direct callers (tests, ad-hoc scripts) - # work correctly even when transcript_path was not routed through - # extract_transcript_path. - transcript_path=$(expand_leading_tilde "$transcript_path") - - if [[ -z "$transcript_path" ]] || [[ ! -f "$transcript_path" ]]; then - return 1 - fi - if ! command -v jq >/dev/null 2>&1; then - return 1 - fi - - local launched completed - launched=$(jq -r --arg since_ts "$since_ts" ' - select(.toolUseResult != null) - | select( - ($since_ts == "" - or ((.timestamp // "") == "") - or ((.timestamp // "") >= $since_ts)) - ) - | select( - (.toolUseResult.isAsync == true and (.toolUseResult.agentId // "") != "") - or ((.toolUseResult.backgroundTaskId // "") != "") - ) - | (.toolUseResult.agentId // .toolUseResult.backgroundTaskId) - ' "$transcript_path" 2>/dev/null | sort -u) || return 1 - - # Union of both completion formats. Either source alone is enough to - # mark a launched id terminal. - # - # The `grep -oE || true` guard on the legacy branch keeps `set -o - # pipefail` from poisoning the combined pipeline when no legacy - # queue-operation records exist in the transcript (grep with `-o` - # exits 1 on no matches, which would otherwise wipe out any SDK - # task_notification results collected above). - completed=$( - { - jq -r ' - select(.type == "system" and .subtype == "task_notification") - | (.task_id // empty) - ' "$transcript_path" 2>/dev/null - jq -r ' - select(.type == "queue-operation" and .operation == "enqueue") - | (.content // "" | tostring) - | select(contains("")) - ' "$transcript_path" 2>/dev/null \ - | { grep -oE '[^<]+' || true; } \ - | sed -E 's|||g' - } | sort -u | sed '/^$/d' - ) || completed="" - - # Emit launched ids that have no matching completion notification. - comm -23 \ - <(printf '%s\n' "$launched" | sed '/^$/d') \ - <(printf '%s\n' "$completed" | sed '/^$/d') -} - -# Returns 0 when the transcript shows at least one pending background task. -# Returns 1 when no pending tasks are detected (including fail-closed cases -# like missing transcript, non-file path, or jq unavailable). -# -# Usage: has_pending_background_tasks "$transcript_path" [since_ts] -has_pending_background_tasks() { - local transcript_path="$1" - local since_ts="${2:-}" - local pending - pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || return 1 - [[ -n "$pending" ]] -} - -# Prints the count of pending background tasks to stdout. Prints 0 for any -# error case so callers can still format messages safely. -# -# Usage: count_pending_background_tasks "$transcript_path" [since_ts] -count_pending_background_tasks() { - local transcript_path="$1" - local since_ts="${2:-}" - local pending - pending=$(list_pending_background_task_ids "$transcript_path" "$since_ts" 2>/dev/null) || { - echo 0 - return 0 - } - if [[ -z "$pending" ]]; then - echo 0 - else - printf '%s\n' "$pending" | sed '/^$/d' | wc -l | tr -d ' ' - fi -} +# Background-task helpers (expand_leading_tilde, extract_transcript_path, +# derive_loop_start_iso_ts, list/has/count_pending_background_task[_ids], +# handle_bg_task_short_circuit) live in loop-bg-tasks.sh and are sourced +# at the bottom of this file so every existing consumer of loop-common.sh +# continues to get them transparently. # Resolve the active state file for a loop directory # Checks for finalize-state.md first, then state.md @@ -1712,3 +1514,15 @@ end_loop() { return 1 fi } + +# Source background-task helpers. Sourced at the bottom so every function +# above is available to callers that only need loop-common.sh, while bg-aware +# callers (the stop hook, the test suite) still get the bg helpers via a +# single source of loop-common.sh. +# +# _LOOP_COMMON_DIR is set here instead of at the top of the file because +# loop-bg-tasks.sh lives in the same directory as this file and we want to +# locate it regardless of how loop-common.sh was sourced. +_LOOP_COMMON_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +# shellcheck source=loop-bg-tasks.sh +source "$_LOOP_COMMON_DIR/loop-bg-tasks.sh" diff --git a/hooks/loop-codex-stop-hook.sh b/hooks/loop-codex-stop-hook.sh index 6779f94e..f3a821d5 100755 --- a/hooks/loop-codex-stop-hook.sh +++ b/hooks/loop-codex-stop-hook.sh @@ -68,110 +68,19 @@ if [[ -z "$LOOP_DIR" ]]; then exit 0 fi -# Shared state used by both guard blocks and the pending-tasks check below. -# Loop-start boundary: derived from the loop dir basename (`YYYY-MM-DD_HH-MM-SS`). -# Empty means derivation failed; helpers treat empty since_ts as no boundary. -LOOP_START_TS=$(derive_loop_start_iso_ts "$LOOP_DIR") -HOOK_TRANSCRIPT_PATH=$(extract_transcript_path "$HOOK_INPUT") - -# ======================================== -# Ambiguous-Caller Marker Guard -# ======================================== -# If a bg-pending.marker is present but we have no session_id on this -# hook invocation (typical of scripts/rlcr-stop-gate.sh invoked without -# --session-id, or any other caller that doesn't forward session_id), -# we cannot tell whether this caller owns the parked loop. Taking either -# branch (foreign-session guard below, or same-session cleanup further -# down) would be wrong in one of the two possible realities. Exit 0 -# silently: the real Claude hook will arrive with session_id populated -# and drive parking / cleanup from an authoritative context. -if [[ -f "$LOOP_DIR/bg-pending.marker" ]] && [[ -z "$HOOK_SESSION_ID" ]]; then - exit 0 -fi - -# ======================================== -# Cross-Session Parked-Loop Guard # ======================================== -# If find_active_loop handed this dir over via the marker fallback, the -# loop is parked by a different session waiting on a background task. The -# current session has no authority to inspect or advance that loop - its -# transcript sees none of the foreign bg activity - so the only safe -# response is to exit 0 with a distinct systemMessage and leave every -# on-disk artifact (state file, stored session_id, marker) untouched. -# -# Both sides of the session-id comparison must be non-empty for this -# branch to trigger: an empty HOOK_SESSION_ID has already exited above -# via the ambiguous-caller guard, and an empty stored session_id keeps -# the backward-compat "matches any" semantics from find_active_loop. -if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then - GUARD_STATE_FILE=$(resolve_active_state_file "$LOOP_DIR") - if [[ -n "$GUARD_STATE_FILE" ]]; then - GUARD_STORED_SID=$(sed -n '/^---$/,/^---$/{ /^'"${FIELD_SESSION_ID}"':/{ s/^'"${FIELD_SESSION_ID}"': *//; p; } }' "$GUARD_STATE_FILE" 2>/dev/null | tr -d ' ') - if [[ -n "$GUARD_STORED_SID" ]] \ - && [[ -n "$HOOK_SESSION_ID" ]] \ - && [[ "$GUARD_STORED_SID" != "$HOOK_SESSION_ID" ]]; then - jq -n \ - '{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."}' - exit 0 - fi - fi -fi - -# ======================================== -# Early Exit: Pending Background Tasks -# ======================================== -# When the main Claude Code session has dispatched background work (Agent with -# run_in_background=true, or Bash with run_in_background=true) whose -# completion notifications have not yet arrived, the natural "stop" is simply -# "I am waiting for the background task". Running git/summary/BitLesson/Codex -# gates in that state wastes Codex tokens and produces low-signal reviews. -# -# Allow the stop (exit 0) and emit a user-visible systemMessage so nobody -# mistakes the pause for loop completion. The on-disk loop state is left -# untouched -- the next natural stop (after background work finishes) will -# re-enter this hook with no pending tasks and run the normal flow. -# -# LOOP_START_TS confines the transcript scan to launches that actually -# happened during this loop; earlier session-wide bg activity cannot pin -# the loop. -# -# This check MUST run before any other gate (phase detection, state parsing, -# branch / plan / git-clean / summary / max-iter checks, Codex review). -PENDING_BG_IDS=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS" 2>/dev/null) || true -if [[ -n "$PENDING_BG_IDS" ]]; then - PENDING_BG_COUNT=$(printf '%s\n' "$PENDING_BG_IDS" | sed '/^$/d' | wc -l | tr -d ' ') - # Mark the loop as parked; allows the same session to resume later and - # makes the cross-session guard above reachable if the user opens a - # different Claude session in this repo before the bg task completes. - : > "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true - jq -n --arg count "$PENDING_BG_COUNT" \ - '{systemMessage: ("RLCR loop active. " + $count + " background task(s) still running - stop allowed naturally; loop has NOT terminated and will resume on completion.")}' - exit 0 -fi - -# Same-session resume after background task finished: the cross-session -# guard above already exited for every foreign session, so reaching here -# with the marker present means the CURRENT session parked the loop and -# has now come back with a transcript showing no pending bg events. -# Remove the stale marker before the normal flow takes over. -# -# Two-part guard to make sure we never drop the parked-state signal -# without evidence: -# (a) list_pending_background_task_ids returned exit 0 -- the -# transcript was present, readable, AND parsed successfully. -# The helper is fail-closed on missing files, empty paths, -# jq parse failure, and truncation, so a non-zero exit blocks -# cleanup here even when the transcript "file" exists. -# (b) its output is empty -- proves "no pending" was authoritatively -# verified, not inferred from a failure. -# The check uses a single fresh call so we capture both the exit code -# and the emptiness without double-running jq. -if [[ -f "$LOOP_DIR/bg-pending.marker" ]]; then - if PENDING_BG_CHECK=$(list_pending_background_task_ids "$HOOK_TRANSCRIPT_PATH" "$LOOP_START_TS" 2>/dev/null) \ - && [[ -z "$PENDING_BG_CHECK" ]]; then - rm -f "$LOOP_DIR/bg-pending.marker" 2>/dev/null || true - fi -fi +# Background-Task Guards +# ======================================== +# Delegates to handle_bg_task_short_circuit (hooks/lib/loop-bg-tasks.sh), +# which runs four cohesive guards in order: +# 1. Ambiguous-caller marker guard (no session_id + marker present) +# 2. Cross-session parked-loop guard (foreign session walking in) +# 3. Pending-bg short-circuit (this session has async work in flight) +# 4. Same-session stale-marker cleanup (bg work just finished) +# When any guard short-circuits, it emits the appropriate JSON on stdout +# and `exit 0`s directly; we never return from that call. When no guard +# fires we continue into the normal gate logic below. +handle_bg_task_short_circuit "$LOOP_DIR" "$HOOK_INPUT" "$HOOK_SESSION_ID" # ======================================== # Detect Loop Phase: Normal or Finalize diff --git a/tests/test-stop-gate.sh b/tests/test-stop-gate.sh index 32f2c3ac..612c68e2 100755 --- a/tests/test-stop-gate.sh +++ b/tests/test-stop-gate.sh @@ -203,5 +203,80 @@ else fail "rlcr-stop-gate reports ALLOW when no active loop" "output containing ALLOW:" "$OUTPUT5" fi +# Test 6: Empty session_id must NOT drop transcript_path from the hook +# input JSON (regression: a `select(length > 0)` used as a plain object +# value would collapse the whole enclosing object to empty whenever any +# selected field was empty, wiping forwarded fields like transcript_path +# even though only session_id was missing). The fix replaces the plain +# select with explicit if/then/else so each field independently becomes +# null on empty input. +T6_DIR="$TEST_DIR/t6" +mkdir -p "$T6_DIR/bin" + +# Mock hook that echoes the raw stdin it received, so we can inspect the +# JSON rlcr-stop-gate.sh builds without depending on the real hook's +# pending-bg logic. +cat > "$T6_DIR/bin/loop-codex-stop-hook.sh" <<'MOCK_HOOK_EOF' +#!/usr/bin/env bash +set -euo pipefail +INPUT="$(cat)" +# Emit a JSON block so the gate wrapper walks the non-"allow on empty" +# branch. We set decision:"block" AND include a recognizable reason the +# test can grep for. +printf '%s\n' "$INPUT" > "${MOCK_HOOK_INPUT_LOG:-/dev/null}" +printf '%s\n' '{"decision":"block","reason":"mock-hook","systemMessage":"mock"}' +MOCK_HOOK_EOF +chmod +x "$T6_DIR/bin/loop-codex-stop-hook.sh" + +# Layout expected by rlcr-stop-gate.sh: HUMANIZE_ROOT/hooks/loop-codex-stop-hook.sh. +# We stage a fake plugin root pointing at the mock hook and copy the gate +# wrapper next to it so the relative resolution resolves to the mock. +mkdir -p "$T6_DIR/plugin/scripts" "$T6_DIR/plugin/hooks" +cp "$T6_DIR/bin/loop-codex-stop-hook.sh" "$T6_DIR/plugin/hooks/loop-codex-stop-hook.sh" +cp "$GATE_SCRIPT" "$T6_DIR/plugin/scripts/rlcr-stop-gate.sh" +chmod +x "$T6_DIR/plugin/scripts/rlcr-stop-gate.sh" + +T6_INPUT_LOG="$T6_DIR/hook-input.json" +T6_TRANSCRIPT="$T6_DIR/fake-transcript.jsonl" +: > "$T6_TRANSCRIPT" + +set +e +( + cd "$T6_DIR" + MOCK_HOOK_INPUT_LOG="$T6_INPUT_LOG" \ + "$T6_DIR/plugin/scripts/rlcr-stop-gate.sh" \ + --transcript-path "$T6_TRANSCRIPT" \ + --json +) > "$T6_DIR/out.txt" 2>&1 +EXIT6=$? +set -e + +if [[ ! -f "$T6_INPUT_LOG" ]]; then + fail "rlcr-stop-gate forwards transcript_path when session_id is empty" \ + "mock hook to capture hook input JSON" \ + "captured input log missing; gate output: $(cat "$T6_DIR/out.txt" 2>/dev/null || true)" +else + T6_TRANSCRIPT_SEEN=$(jq -r '.transcript_path // "__MISSING__"' "$T6_INPUT_LOG" 2>/dev/null || echo "__PARSE_ERROR__") + T6_SESSION_SEEN=$(jq -r '.session_id | if . == null then "__NULL__" else . end' "$T6_INPUT_LOG" 2>/dev/null || echo "__PARSE_ERROR__") + if [[ "$T6_TRANSCRIPT_SEEN" == "$T6_TRANSCRIPT" ]] && [[ "$T6_SESSION_SEEN" == "__NULL__" ]]; then + pass "rlcr-stop-gate forwards transcript_path when session_id is empty (jq object-collapse fix)" + else + fail "rlcr-stop-gate forwards transcript_path when session_id is empty (jq object-collapse fix)" \ + "transcript_path=$T6_TRANSCRIPT, session_id=__NULL__" \ + "transcript_path=$T6_TRANSCRIPT_SEEN, session_id=$T6_SESSION_SEEN; raw: $(cat "$T6_INPUT_LOG" 2>/dev/null || true)" + fi +fi + +# Exit 10 because the mock hook always returns decision:"block"; ensure +# the wrapper reached the decision branch rather than exiting 20 +# (wrapper error) or 0 (bogus ALLOW from lost transcript_path). +if [[ "$EXIT6" -eq 10 ]]; then + pass "rlcr-stop-gate reaches decision branch with empty session_id + real transcript_path" +else + T6_BODY=$(cat "$T6_DIR/out.txt" 2>/dev/null || true) + fail "rlcr-stop-gate reaches decision branch with empty session_id + real transcript_path" \ + "exit 10 (mock hook returns block)" "exit $EXIT6; output: $T6_BODY" +fi + print_test_summary "RLCR Stop Gate Wrapper Test Summary" exit $?