Fix sim run-dir discovery races under concurrent suites

tests/bounce-state-simulation.sh

@@ -121,15 +121,22 @@ DOC
 # ---------------------------------------------------------------------------
 # Scenario 1: co-evolve-bouncer --vanilla --bounce-only -> conforming state
 # ---------------------------------------------------------------------------
+# Policy (b): co-evolve-bouncer.sh already supports CO_EVOLVE_RUNS_DIR (v1.4,
+# added for the npm MCP embedder) to redirect run artifacts away from its own
+# runs/ dir. Pointing it at this sim's own $TEST_DIR makes discovery within
+# that private root race-free by construction — no other concurrent suite can
+# ever write into it — instead of racing on `ls -dt` against the shared repo
+# runs/ dir that every concurrent suite's default invocation also writes to.
+CO_EVOLVE_RUNS_DIR="$TEST_DIR/co-evolve-runs"
 TOTAL=$((TOTAL + 1))
 s1_doc="$TEST_DIR/s1-doc.md"
 cp "$DOC_SRC" "$s1_doc"
 rc=0
-BOUNCE_STUB_COUNTER="$TEST_DIR/s1-count" PATH="$TEST_DIR/bin:$PATH" \
+BOUNCE_STUB_COUNTER="$TEST_DIR/s1-count" PATH="$TEST_DIR/bin:$PATH" CO_EVOLVE_RUNS_DIR="$CO_EVOLVE_RUNS_DIR" \
   bash "$REPO_ROOT/co-evolve-bouncer.sh" --vanilla --bounce-only "$s1_doc" \
   > "$TEST_DIR/s1-stdout.log" 2> "$TEST_DIR/s1-stderr.log" || rc=$?
 
-s1_run_dir=$(ls -dt "$REPO_ROOT"/runs/co-evolve-* 2>/dev/null | head -1)
+s1_run_dir=$(ls -dt "$CO_EVOLVE_RUNS_DIR"/co-evolve-* 2>/dev/null | head -1)
 s1_state="$s1_run_dir/state.json"
 if [[ "$rc" -eq 0 && -f "$s1_state" ]] \
    && jq -e '.schema == "bounce-state/1.0"
@@ -193,10 +200,10 @@ cp "$TEST_DIR/authbin/claude" "$TEST_DIR/authbin/codex"
 s4_doc="$TEST_DIR/s4-doc.md"
 cp "$DOC_SRC" "$s4_doc"
 rc=0
-PATH="$TEST_DIR/authbin:$PATH" \
+PATH="$TEST_DIR/authbin:$PATH" CO_EVOLVE_RUNS_DIR="$CO_EVOLVE_RUNS_DIR" \
   bash "$REPO_ROOT/co-evolve-bouncer.sh" --vanilla --bounce-only "$s4_doc" \
   > /dev/null 2>&1 || rc=$?
-s4_run_dir=$(ls -dt "$REPO_ROOT"/runs/co-evolve-* 2>/dev/null | head -1)
+s4_run_dir=$(ls -dt "$CO_EVOLVE_RUNS_DIR"/co-evolve-* 2>/dev/null | head -1)
 s4_state="$s4_run_dir/state.json"
 if [[ "$rc" -ne 0 && -f "$s4_state" ]] \
    && jq -e '.status == "aborted" and (.passes | length) == 0' "$s4_state" >/dev/null; then
@@ -209,14 +216,34 @@ rm -rf "$s4_run_dir"
 # ---------------------------------------------------------------------------
 # Scenario 5: agent-bouncer -> conforming state + normalized artifact names
 # ---------------------------------------------------------------------------
+# Policy (c): agent-bouncer.sh hardcodes RUNS_DIR="${REPO_ROOT}/runs" with no
+# env override (unlike co-evolve-bouncer.sh's CO_EVOLVE_RUNS_DIR), so it always
+# writes into the shared repo runs/ dir that a concurrent suite's own bouncer
+# runs also write into. Attributable snapshot-diff instead of `ls -dt | head -1`:
+# the run dir label derives from the input doc's basename (agent-bouncer.sh's
+# filename fallback — the stubbed LLM naming output never passes its kebab-case
+# regex), so a PID-tagged doc name makes OUR dir identifiable. List before,
+# list after, take the one new entry carrying our tag; a concurrent suite's S5
+# dirs carry a different PID and never match, so zero or multiple MATCHING
+# entries indicates a real bug — fail loudly.
 TOTAL=$((TOTAL + 1))
-s5_doc="$TEST_DIR/s5-doc.md"
+s5_doc="$TEST_DIR/s5-doc-$$.md"
 cp "$DOC_SRC" "$s5_doc"
+s5_before=$(ls -d "$REPO_ROOT"/runs/bouncer-s5-doc-$$-* 2>/dev/null || true)
 rc=0
 BOUNCE_STUB_COUNTER="$TEST_DIR/s5-count" PATH="$TEST_DIR/bin:$PATH" \
   bash "$REPO_ROOT/agent-bouncer/agent-bouncer.sh" "$s5_doc" 2 claude claude \
   > "$TEST_DIR/s5-stdout.log" 2>&1 || rc=$?
-s5_run_dir=$(ls -dt "$REPO_ROOT"/runs/bouncer-* 2>/dev/null | head -1)
+s5_after=$(ls -d "$REPO_ROOT"/runs/bouncer-s5-doc-$$-* 2>/dev/null || true)
+s5_new=$(comm -13 <(printf '%s\n' "$s5_before" | sort) <(printf '%s\n' "$s5_after" | sort))
+s5_run_dir=""
+# grep -c exits 1 on zero matches, which would trip `set -e` inside a command
+# substitution (zero-new-dirs is a real failure mode here, e.g. the invocation
+# died before creating one) — `|| true` keeps that a value, not a script kill.
+s5_new_count=$(printf '%s\n' "$s5_new" | grep -c . || true)
+if [[ "$s5_new_count" -eq 1 ]]; then
+  s5_run_dir="$s5_new"
+fi
 s5_state="$s5_run_dir/state.json"
 if [[ "$rc" -eq 0 && -f "$s5_state" ]] \
    && jq -e '.schema == "bounce-state/1.0"

tests/observability-lifecycle-simulation.sh

@@ -137,26 +137,28 @@ harmless line to a tracked file so the run produces a diff for verify to score.
 - None identified.
 PLAN
 
-latest_run_dir() { ls -dt "$REPO_ROOT"/runs/dev-review-* 2>/dev/null | head -1; }
+# Explicit per-scenario run dirs (policy a) — avoids the `ls -dt runs/dev-review-*`
+# newest-mtime race that cross-reads another concurrent suite's run dir. Each
+# scenario passes --run-dir under this sim's own $TEST_DIR, so cleanup rides the
+# existing TEST_DIR EXIT trap instead of a per-scenario rm -rf.
 
 # ===========================================================================
 # Scenario 1: full lifecycle — current_phase=null at EOF, runner_pid present,
 # pre/post SHA 40-hex.
 # ===========================================================================
 TOTAL=$((TOTAL + 1))
 repo=$(make_scratch_repo s1)
-before=$(latest_run_dir || true)
+run1="$TEST_DIR/run-s1"
 (
   unset CLAUDE_MODEL CLAUDE_EFFORT CODEX_MODEL CODEX_REASONING_EFFORT
   unset COMPOSER_MODEL COMPOSER_EFFORT EXECUTOR_MODEL EXECUTOR_EFFORT VERIFIER_MODEL VERIFIER_EFFORT
   export PATH="$TEST_DIR/bin:$PATH"
   bash "$RUNNER" --skip-plan --plan "$PLAN_FIXTURE" --bounces 0 --verify --verifier codex \
-    --workdir "$repo" --timeout 60 -- "obs lifecycle probe"
+    --run-dir "$run1" --workdir "$repo" --timeout 60 -- "obs lifecycle probe"
 ) > "$TEST_DIR/s1.out" 2>&1 || true
-run1=$(latest_run_dir || true)
 s1_state="$run1/state.json"
 s1_ok=true
-if [[ -z "$run1" || "$run1" == "$before" || ! -f "$s1_state" ]]; then
+if [[ ! -f "$s1_state" ]]; then
   s1_ok=false
 else
   jq -e '.current_phase == null' "$s1_state" >/dev/null 2>&1 || { s1_ok=false; echo "  current_phase not null" >&2; }
@@ -172,28 +174,25 @@ else
   [[ -f "$s1_state" ]] && cat "$s1_state" >&2
   cat "$TEST_DIR/s1.out" >&2
 fi
-[[ -n "$run1" && "$run1" != "$before" ]] && rm -rf "$run1"
 
 # ===========================================================================
 # Scenario 2: --parent-run lands in .orchestration.parent_run_id.
 # ===========================================================================
 TOTAL=$((TOTAL + 1))
 repo=$(make_scratch_repo s2)
-before=$(latest_run_dir || true)
+run2="$TEST_DIR/run-s2"
 (
   export PATH="$TEST_DIR/bin:$PATH"
   bash "$RUNNER" --skip-plan --plan "$PLAN_FIXTURE" --bounces 0 --verify --verifier codex \
-    --parent-run "abc-123" --workdir "$repo" --timeout 60 -- "obs parent-run probe"
+    --parent-run "abc-123" --run-dir "$run2" --workdir "$repo" --timeout 60 -- "obs parent-run probe"
 ) > "$TEST_DIR/s2.out" 2>&1 || true
-run2=$(latest_run_dir || true)
-if [[ -n "$run2" && "$run2" != "$before" && -f "$run2/state.json" ]] \
+if [[ -f "$run2/state.json" ]] \
    && jq -e '.orchestration.parent_run_id == "abc-123"' "$run2/state.json" >/dev/null 2>&1; then
   pass "S2: --parent-run abc-123 recorded in .orchestration.parent_run_id"
 else
   fail "S2: parent_run_id missing (run=$run2)"
   [[ -f "$run2/state.json" ]] && cat "$run2/state.json" >&2
 fi
-[[ -n "$run2" && "$run2" != "$before" ]] && rm -rf "$run2"
 
 # ===========================================================================
 # Scenario 3: --parent-run rejects a garbage token (path traversal / shell meta).
@@ -215,25 +214,25 @@ fi
 # ===========================================================================
 TOTAL=$((TOTAL + 1))
 repo=$(make_scratch_repo s4)
-before=$(latest_run_dir || true)
+run4="$TEST_DIR/run-s4"
 (
   export PATH="$TEST_DIR/bin:$PATH"
   export OBS_EXECUTE_SLEEP=4
   bash "$RUNNER" --skip-plan --plan "$PLAN_FIXTURE" --bounces 0 --verify --verifier codex \
-    --workdir "$repo" --timeout 60 -- "obs mid-run probe"
+    --run-dir "$run4" --workdir "$repo" --timeout 60 -- "obs mid-run probe"
 ) > "$TEST_DIR/s4.out" 2>&1 &
 runner_bg=$!
 
-# Poll for a run dir whose state.json reports current_phase.name == execute.
+# Poll the known run dir's state.json for current_phase.name == execute (no
+# newest-mtime discovery needed — the path is pinned by --run-dir above).
 captured=false
 captured_pid=""
 for _ in $(seq 1 60); do
-  rd=$(latest_run_dir || true)
-  if [[ -n "$rd" && "$rd" != "$before" && -f "$rd/state.json" ]]; then
-    name=$(jq -r '.current_phase.name // empty' "$rd/state.json" 2>/dev/null || true)
+  if [[ -f "$run4/state.json" ]]; then
+    name=$(jq -r '.current_phase.name // empty' "$run4/state.json" 2>/dev/null || true)
     if [[ "$name" == "execute" ]]; then
       captured=true
-      captured_pid=$(jq -r '.runner_pid // empty' "$rd/state.json" 2>/dev/null || true)
+      captured_pid=$(jq -r '.runner_pid // empty' "$run4/state.json" 2>/dev/null || true)
       break
     fi
   fi
@@ -246,8 +245,7 @@ s4_ok=true
 # runner_pid captured mid-run must be a positive integer.
 [[ "$captured_pid" =~ ^[0-9]+$ ]] || { s4_ok=false; echo "  captured runner_pid not numeric: $captured_pid" >&2; }
 # And the run must still terminate cleanly with current_phase cleared.
-run4=$(latest_run_dir || true)
-if [[ -n "$run4" && -f "$run4/state.json" ]]; then
+if [[ -f "$run4/state.json" ]]; then
   jq -e '.current_phase == null and .status == "completed"' "$run4/state.json" >/dev/null 2>&1 \
     || { s4_ok=false; echo "  post-run state not clean terminal" >&2; }
 fi
@@ -258,7 +256,6 @@ else
   [[ -f "$run4/state.json" ]] && cat "$run4/state.json" >&2
   cat "$TEST_DIR/s4.out" >&2
 fi
-[[ -n "$run4" && "$run4" != "$before" ]] && rm -rf "$run4"
 
 # ===========================================================================
 # Scenario 5: parity — a default skip-plan run with NO new flags still produces
@@ -267,21 +264,19 @@ fi
 # ===========================================================================
 TOTAL=$((TOTAL + 1))
 repo=$(make_scratch_repo s5)
-before=$(latest_run_dir || true)
+run5="$TEST_DIR/run-s5"
 (
   export PATH="$TEST_DIR/bin:$PATH"
   bash "$RUNNER" --skip-plan --plan "$PLAN_FIXTURE" --bounces 0 --verify --verifier codex \
-    --workdir "$repo" --timeout 60 -- "obs parity probe"
+    --run-dir "$run5" --workdir "$repo" --timeout 60 -- "obs parity probe"
 ) > "$TEST_DIR/s5.out" 2>&1 || true
-run5=$(latest_run_dir || true)
-if [[ -n "$run5" && "$run5" != "$before" && -f "$run5/state.json" ]] \
+if [[ -f "$run5/state.json" ]] \
    && jq -e '.status == "completed" and .current_phase == null' "$run5/state.json" >/dev/null 2>&1; then
   pass "S5: no-new-flags run reaches completed with current_phase cleared"
 else
   fail "S5: parity run not clean (run=$run5)"
   [[ -f "$run5/state.json" ]] && cat "$run5/state.json" >&2
 fi
-[[ -n "$run5" && "$run5" != "$before" ]] && rm -rf "$run5"
 
 # ---------------------------------------------------------------------------
 printf '%d/%d scenarios passed' "$PASSED" "$TOTAL"