Repository navigation
fix(spawn): transition executor state from spawning to running after spawn - #1021
Conversation
…spawn Three fixes for agents/executors stuck permanently in 'spawning' state: 1. launchTmuxSpawn: after awaitAgentReadiness(), transition executor to 'running' and legacy worker to 'idle' — previously nothing triggered this transition, leaving all executors stuck in 'spawning' 2. reconcileStaleSpawns: add current_executor_id IS NULL guard so agent identity records (from findOrCreateAgent) are not falsely marked as errors — they track state through their executor, not the legacy state column 3. findOrCreateAgent: explicitly set state=NULL on identity records instead of inheriting the 'spawning' default, preventing them from being caught by any state-based reconciliation queries
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the agent registry to prevent reconcileStaleSpawns from erroneously flagging identity records by adding a current_executor_id IS NULL check and initializing records with a NULL state. It also adds state transitions in launchTmuxSpawn. Reviewers suggested extending these transitions to resumeAgent and launchInlineSpawn for consistency and updating the test case to explicitly set the agent state to 'spawning' to ensure the SQL guard is correctly tested.
| // Transition executor + legacy worker from 'spawning' to 'running' | ||
| if (ctx.executorId) { | ||
| await executorRegistry.updateExecutorState(ctx.executorId, 'running').catch(() => {}); | ||
| } | ||
| await registry.update(ctx.workerId, { state: 'idle' }).catch(() => {}); |
There was a problem hiding this comment.
The state transition logic added here is important for fixing executors stuck in the spawning state. However, this transition is currently missing from the resumeAgent and launchInlineSpawn functions. To ensure consistency and prevent executors from being stuck in spawning across all lifecycle paths, similar updates should be applied to those functions as well.
|
|
||
| // Backdate started_at so it would match the threshold | ||
| const sql = await getConnection(); | ||
| await sql`UPDATE agents SET started_at = now() - interval '10 seconds' WHERE id = ${agent.id}`; |
There was a problem hiding this comment.
To effectively test the current_executor_id IS NULL guard in reconcileStaleSpawns, the test should explicitly set the agent's state to 'spawning'. Currently, findOrCreateAgent sets the state to NULL, which already causes reconcileStaleSpawns to skip the record regardless of the current_executor_id value.
| await sql`UPDATE agents SET started_at = now() - interval '10 seconds' WHERE id = ${agent.id}`; | |
| await sql("UPDATE agents SET started_at = now() - interval '10 seconds', state = 'spawning' WHERE id = " + agent.id); |
… from concurrency cap Bug 1: reconcileStaleSpawns second pass (added in prior commit) now verified by updating the integration test to expect dead-pane agents ARE cleaned up. Bug 2: The concurrency cap in attemptAgentResume counted 'spawning' agents as active, causing zombie spawning agents to block all auto-resume. Adding 'spawning' to the exclusion list fixes this — spawning agents are not yet consuming compute and should not count toward the cap.
Summary
awaitAgentReadiness()inlaunchTmuxSpawn(), transition executor fromspawning→runningand legacy worker toidle. Previously nothing triggered this transition, leaving all executors permanently stuck inspawning.current_executor_id IS NULLtoreconcileStaleSpawns()query so agent identity records (fromfindOrCreateAgent) are not falsely marked aserrorafter 60s.state = NULLexplicitly infindOrCreateAgent()instead of inheriting thespawningdefault, preventing identity records from being caught by state-based reconciliation.Root Cause
The executor model (migration 012) introduced a split between agent identity records and executor records. Identity records default to
state='spawning'withpane_id=''.reconcileStaleSpawns()matched these after 60s and marked them as errors, triggering false[executor.error]events. Additionally, the spawn flow never transitioned executors fromspawningtorunningafter successful pane creation.Test plan
reconcileStaleSpawnstests still pass (74 tests in agent-registry)current_executor_idare excluded from reconciliation