Skip to content

fix(runner): fast-fail spawn-thrash when no task completes#657

Merged
lwgray merged 1 commit into
developfrom
fix/spawn-thrash-fast-fail
May 25, 2026
Merged

fix(runner): fast-fail spawn-thrash when no task completes#657
lwgray merged 1 commit into
developfrom
fix/spawn-thrash-fast-fail

Conversation

@lwgray

@lwgray lwgray commented May 25, 2026

Copy link
Copy Markdown
Owner

What is this system, briefly

Marcus is a coordination platform for multi-agent software builds. The
experiment runner (the long-running script in
dev-tools/experiments/runners/spawn_agents.py) is the program that
actually drives a Marcus run: it polls Marcus every ~30 seconds and
spawns short-lived "ephemeral" agents — each does exactly one task
and exits — to fill the agent count Marcus asks for. This PR fixes
a failure mode in the runner that can quietly burn tens of dollars
per run.

Why (the user-visible problem)

When a task gets marked BLOCKED in Marcus — for example because an
agent's worktree merge to main failed with a real content conflict
that the auto-rebase recovery added in PR #656 cannot resolve — every
downstream task that depends on it stays unclaimed forever. Marcus
still reports desired_agent_count > in_flight_tasks, so the runner
keeps spawning agents that immediately receive "no task ready" and
exit. Each wasted agent costs roughly $0.50–$1.00 in tokens.

The existing safety net is a stall watchdog that fires only after
20 minutes of zero change to the (completed, in_progress, blocked)
task-count tuple. Under spawn-thrash, that tuple keeps flickering
(in_progress goes up by one when the agent claims, back down when
it exits) — so the watchdog never trips. Real recent incidents:

Run Spawn count before teardown Estimated waste
test71 (verify-snake-8) 49 agents over 20 minutes $25–$50
test73 (verify-snake-10) 38 agents before manual kill $19–$38

If this happened on a user's API key in a hosted Marcus deployment,
one bad project could rack up real money before anyone noticed.

What changed

New: SpawnThrashDetector (in dev-tools/experiments/runners/spawn_controller.py)

A second watchdog that runs alongside StallWatchdog. Each poll the
runner reports two numbers:

  • completed: cumulative tasks completed this run
  • to_spawn: number of agents the runner is spawning this poll
    (the output of compute_spawn_count)

The detector counts consecutive polls where to_spawn > 0 AND
completed did not increase
. When the count reaches thrash_polls
(default 5), the detector reports thrash and the runner tears the
session down with the new spawn_thrash exit reason.

Behavior details:

  • Any task completing during the streak resets the counter to zero.
  • A poll where the runner spawned nothing (to_spawn == 0) holds the
    counter where it is — a slow task is not thrash, but the thrash may
    resume once the runner spawns again, and we should not have to
    start counting from scratch.
  • The first observation only sets the baseline (we need a prior poll
    to compare completed against), so the detector never fires on
    the very first poll.
  • thrash_polls = 0 disables the detector entirely.

Wired into the runner main loop (spawn_agents.py)

The detector is constructed alongside the stall watchdog and consulted
every poll, immediately after the existing stall-watchdog check.

Config

A new top-level config key spawn_thrash_polls (default 5) is
read in ExperimentSpawner.__init__. At the default 30-second poll
cadence this is a 2.5-minute fast-fail. Worst-case waste under
the new threshold: ~5 agents per incident instead of the 30–50 we
have been seeing — a 6–10x improvement.

Why a separate detector (vs. tightening StallWatchdog)

StallWatchdog answers "has the whole run stopped making any kind of
progress?" — that has to be a slow signal because a long-running task
can legitimately leave the tuple flat for many minutes. Speeding it
up would cause false positives on slow tasks.

SpawnThrashDetector answers a sharper question — "is the runner
actively burning money on doomed agents?" — and can be detected
quickly without that risk because the signal includes the spawning
behavior itself, not just whether the counts moved.

Worked numerical example

At default spawn_thrash_polls = 5 and control_poll_seconds = 30:

  • Detection window: 5 polls × 30 s = 150 s = 2.5 minutes
  • Max spawns during detection: bounded by desired_agent_count;
    for a typical 10-agent run, ≤ 5 spawns (one or two per poll)
  • Max wasted cost: 5 spawns × $1/spawn = ~$5 per thrash incident
  • Compared to today: 30–50 spawns × $1/spawn = $30–$50 per incident

Where to look in the code first

File Purpose
dev-tools/experiments/runners/spawn_controller.py New SpawnThrashDetector class (pure decision logic, no I/O)
dev-tools/experiments/runners/spawn_agents.py Wires the detector into the polling loop; adds spawn_thrash_polls config key
tests/unit/experiments/test_spawn_controller.py 7 new unit tests in TestSpawnThrashDetector

Glossary

Term Meaning
Ephemeral agent A worker process that claims one Marcus task, completes it, and exits. The runner spawns a new one for the next task instead of reusing the process.
Spawn-thrash The runner keeps spawning ephemeral agents that immediately exit because no task is actually claimable. Identified by completed flat while to_spawn > 0.
BLOCKED task A Marcus task marked unworkable (e.g., a merge to main failed). Downstream tasks that depend on it stay unclaimed.
Stall watchdog The existing 20-minute timer that tears down a run when (completed, in_progress, blocked) stops changing. Does NOT catch spawn-thrash because the tuple keeps flickering.
to_spawn The output of compute_spawn_count(desired, in_flight, unclaimed) — how many agents the runner will start this poll.

How to verify it works

  1. Run the new unit tests:
    python -m pytest tests/unit/experiments/test_spawn_controller.py -v
    Expected: all 21 tests pass, including 7 in TestSpawnThrashDetector.
  2. Run the spawn_agents unit suite to confirm no regression:
    python -m pytest tests/unit/experiments/test_spawn_agents.py -q
    Expected: 90 tests pass.
  3. Mypy:
    python -m mypy dev-tools/experiments/runners/spawn_controller.py \
                   dev-tools/experiments/runners/spawn_agents.py
    Expected: Success: no issues found in 2 source files.
  4. Optional end-to-end (when one is convenient): run a project that
    reliably produces a real content merge conflict. The runner should
    emit SPAWN-THRASH: spawned agents for 5 polls with no task completing and tear down after ~2.5 minutes instead of waiting
    20 minutes.

Test plan

  • Unit: SpawnThrashDetector fires after N idle-spawn polls
  • Unit: completion resets the counter
  • Unit: to_spawn == 0 holds the counter without incrementing
  • Unit: thrash_polls == 0 disables the detector
  • Unit: first observation never fires (baseline-only)
  • Unit: post-fire re-arm via completion
  • Unit: realistic test73 trace (completed plateau at 4)
  • All 90 existing spawn_agents unit tests still pass
  • Mypy clean on both modified files
  • Manual: trigger a real BLOCKED-task scenario, observe spawn_thrash teardown in ~2.5 min

Related

🤖 Generated with Claude Code

PROBLEM
The experiment runner is a long-lived controller that polls Marcus
every ~30 seconds and spawns ephemeral, one-task agents to fill the
"desired agent count" Marcus reports. When a task gets marked BLOCKED
(for example because a worktree merge to main failed with a real
content conflict that auto-rebase cannot resolve), every downstream
task that depends on it stays unclaimed forever. Marcus still reports
desired > in_flight, so the runner keeps spawning agents that
immediately get "no task ready" and exit. Each wasted agent costs
roughly $0.50–$1.00 in tokens.

The existing safety net is a stall watchdog that fires only after 20
minutes of zero change to (completed, in_progress, blocked). That tuple
keeps flickering during spawn-thrash (in_progress goes up by one when
the agent claims, back down when it exits), so the watchdog stays
quiet. Real recent incidents:

- test71 (verify-snake-8): 49 agents spawned over ~20 min
- test73 (verify-snake-10): 38 agents spawned before manual kill

CHANGE
Add a new SpawnThrashDetector alongside StallWatchdog in
spawn_controller.py. Each poll the runner feeds it two numbers:
  - completed: cumulative tasks completed this run
  - to_spawn: number of agents spawned this poll
If completed is unchanged AND to_spawn > 0 for `thrash_polls` polls
in a row, the detector reports thrash and the runner tears the
session down ("spawn_thrash" exit reason). Any completion resets the
counter. A poll with to_spawn == 0 holds the counter without
incrementing it (a slow task is not thrash, but the thrash may resume
once the runner spawns again).

Default threshold is 5 polls — at the 30s poll cadence that is a 2.5
minute fast-fail. Configurable via `spawn_thrash_polls` in
config.yaml; 0 disables. Worst case waste under the new threshold:
~5 agents per incident instead of the 30–50 we have been seeing.

WHY A SEPARATE DETECTOR
StallWatchdog answers "did the whole run wedge?" — that signal must
stay slow because a long-running task can legitimately leave the
tuple flat for minutes. SpawnThrashDetector answers a sharper
question — "is the runner actively burning money on doomed agents?"
— which can be detected quickly without false-positives on slow
tasks.

TESTS
Seven new unit tests in tests/unit/experiments/test_spawn_controller.py
cover: fires after N idle-spawn polls, completion resets, to_spawn=0
holds the counter, thrash_polls=0 disables, first-poll baseline never
fires, post-fire re-arm, and a realistic test73 trace. All 21
spawn_controller tests pass; all 90 spawn_agents tests pass.

This unblocks issue #206 (file-level baton arbitration), which the
user wants gated on spawn-thrash being fixed first so that a
coordination bug there cannot cost tens of dollars per run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 25, 2026

Copy link
Copy Markdown

Claude finished @lwgray's task —— View job


Code Review Complete ✅

Overall Assessment: This is a well-designed and implemented solution to an expensive failure mode. The code quality is high with excellent documentation, comprehensive testing, and clean separation of concerns.

🟢 Strengths

Architecture & Design

  • Clean separation: Pure decision logic in SpawnThrashDetector with no I/O dependencies makes testing straightforward
  • Single responsibility: Focused detector for spawn-thrash vs. broader StallWatchdog - good design choice
  • Well-documented rationale: Clear explanation of why separate detector vs. tightening existing watchdog

Implementation Quality

  • Robust state management: Proper baseline initialization, counter reset logic, and edge case handling
  • Defensive programming: Disabled detector when thrash_polls = 0, graceful handling of first observation
  • Clear naming: idle_spawn_polls, observe() method names are intuitive
  • Good integration: Minimal changes to existing runner loop

Test Coverage

  • Comprehensive scenarios: 7 focused unit tests covering all key behaviors
  • Real-world validation: test_realistic_thrash_scenario_test73() uses actual incident data
  • Edge cases covered: First observation, disabled detector, post-fire re-arm
  • Clear test names: Each test clearly describes what it verifies

🟡 Minor Considerations

Performance

  • Minimal overhead: Simple integer comparisons per poll - negligible impact
  • Memory efficient: Only stores last completed count and counter state

Configuration

  • Reasonable defaults: 5 polls (2.5 min) balances fast-fail vs. false positives
  • Good configurability: Can disable via spawn_thrash_polls = 0

Error Handling

  • Existing patterns: Leverages existing teardown mechanism and exception handling
  • Graceful degradation: Detector disable doesn't break runner functionality

🟢 Security & Safety

  • No security concerns: Read-only state tracking with no external dependencies
  • Safe failure mode: Detector failure would just revert to old 20-minute timeout
  • No injection risks: Integer-only inputs with bounds checking

📋 Code Quality Details

spawn_controller.py:166-261

  • Excellent docstring: Clear explanation of failure mode and detection strategy
  • Type hints: Proper Optional typing and return types
  • Logic clarity: observe() method flow is easy to follow

spawn_agents.py:336,1895-1896,2015-2023

  • Minimal integration: Clean addition without disrupting existing flow
  • Consistent logging: Error message follows existing patterns
  • Proper config reading: Uses existing config patterns with sensible default

test_spawn_controller.py:152-256

  • Test organization: Well-structured test class with clear scenarios
  • Realistic data: test73 scenario uses real incident numbers
  • Boundary testing: Covers first observation, disabled detector, etc.

💰 Business Impact

  • Significant cost savings: 6-10x reduction in waste (from $30-50 to ~$5 per incident)
  • Fast feedback: 2.5 min vs. 20 min detection improves developer experience
  • Production safety: Prevents runaway costs in hosted deployments

Recommendation

APPROVE - This is a solid implementation that solves a real problem with minimal risk. The code is well-tested, clearly documented, and follows existing patterns. The cost savings potential justifies the added complexity.

Pre-merge checklist (as noted in PR):

  • Run unit tests to verify no regression
  • Mypy type checking passes
  • Optional: End-to-end test with real BLOCKED scenario

@lwgray

lwgray commented May 25, 2026

Copy link
Copy Markdown
Owner Author

@codex

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lwgray lwgray merged commit b2fc752 into develop May 25, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant