Fix sim run-dir discovery races under concurrent suites - #44
Merged
Conversation
ls -dt runs/dev-review-* | head -1 picked whichever concurrent suite's run dir was newest, so two suites running in one checkout could each grab the other's state.json (wrong pre/post_execute_sha, missing keys, false Robustness failures). Pass --run-dir explicitly to every runner invocation in these three sims instead of rediscovering "newest" after the fact; run dirs live under each sim's own $TEST_DIR so the existing EXIT trap cleans them up (dropped the now-redundant rm -rf calls). Also folds observability-lifecycle-simulation.sh's mid-run poller to watch the known --run-dir path directly instead of re-scanning runs/ on every iteration, removing a second race inside the loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
S1/S4 (co-evolve-bouncer.sh) rediscovered "newest runs/co-evolve-*", racing against any concurrent suite whose own bouncer runs also land in the shared repo runs/ dir. Point CO_EVOLVE_RUNS_DIR (already supported since v1.4 for the npm MCP embedder) at this sim's own $TEST_DIR so discovery happens in a private root nothing else writes into. S5 (agent-bouncer.sh) has no equivalent runs-root override, so it still writes into the shared runs/ dir; switched to snapshot-diff (list before, list after, take the one new entry) instead of "newest wins". Guard the zero-new-dirs count with `|| true` since grep -c's exit 1 on no match would otherwise trip set -e inside the command substitution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plain snapshot-diff still false-failed when two concurrent suites ran S5 in the same window: each saw the other's new bouncer-* dir, counted two new entries, and took the loud-failure path (proven by a concurrent quick x 2 run — both sides failed S5 and leaked both dirs). agent-bouncer.sh's run label derives from the input doc's basename, so tagging the doc name with $$ makes our dir identifiable: scope the before/after listing to bouncer-s5-doc-<pid>-* and a sibling suite's dirs (different PID) can never match. Zero or multiple matching entries now genuinely means a bug, and cleanup works again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task #10 of the workflow-upgrade run (plan @ e6db065). Eliminates the newest-mtime run-dir race that produced false suite failures when two runs shared a checkout (proven 2026-07-05: disjoint false-failure sets, all-solo-green).
What changed (3 commits: b750017, 505c996, c1100ee)
tests/observability-lifecycle-simulation.sh(:140-287),:231-383),tests/token-capture-simulation.sh(tests/preset-expansion-simulation.sh(~:257-705) — every runner invocation pins--run-dir(pre-existing dev-review.sh support); no morels -dt | head -1; the observability mid-run poller watches the pinned path.tests/bounce-state-simulation.sh(~:121-256) — S1/S4 use the runner's pre-existingCO_EVOLVE_RUNS_DIRprivate root; S5 (agent-bouncer, no runs-root override) uses PID-tagged snapshot-diff — plain diff still false-failed when both suites' S5 windows collided, which the concurrent proof caught and the tag fixed.Verification
run-all.sh --quickin one checkout →25/25/25/25, zero FAIL lines (this exact scenario failed before the fix).run-all.sh28/28 in 635s; each changed sim green individually; pr-emitter 12/12, code-proposer 16/16 at final HEAD.grep -rn "ls -dt.*runs/" tests/ evals/→ zero active sites.dev-review-status.sh:75,136retains its intentional default-to-latest UX (override already sim-tested).Noted, not done: auth-gate + reliability sims each leak one
runs/co-evolve-tmp-*dir per run (pre-existing, not a race) — chip filed.🤖 Generated with Claude Code