Repository navigation
fix(scheduler): mark unresumable + exhausted agents as terminal (stop re-processing) - #1195
Conversation
… re-processing)
Two related bugs, same architectural shape — terminal-state agents that the
scheduler kept re-processing every cycle.
Bug A: Rows in `state='error', auto_resume=true, claude_session_id=null`
were dropped by the resumable filter (no session id to resume), but stayed
in `auto_resume=true` forever — misleading `genie ls` ("auto-resume: on")
and cluttering the worker list. Observed live on felipe's machine: 9 wedged
rows (8 genie-docs directory placeholders + 2 omni workers that died before
capturing a Claude session).
Fix: new `reconcileUnresumable` pass wired into `startAgentResumeTimer`
between `reconcileDeadPaneZombies` and `runAgentRecoveryPass`. Flips those
rows to `auto_resume=false` and logs `agent_marked_unresumable`.
Bug B: The `attempts >= maxAttempts` early-exit in `attemptAgentResume`
logged `agent_resume_exhausted` and returned, but did NOT persist
`auto_resume=false`. Next scheduler tick (60s later) re-entered the same
branch and re-logged the same event — the agent was never re-attempted,
yet the log stayed polluted with redundant exhaustion events forever.
Fix: both exhaustion branches (pre-attempt early-exit + post-attempt final
failure) now call `deps.updateAgent(agentId, { autoResume: false })` before
logging. The next cycle's `attemptAgentResume` short-circuits at the
`autoResume === false` check with `agent_resume_skipped` (debug level),
ending the every-60s spam.
Prior art: PR #1181 (zombie concurrency cap), PR #1187 (counter
persistence). This closes the "terminal states should be terminal" theme.
Staged follow-ups (out of scope):
- dir:-row state mutation root cause (separate /trace)
- omni session-capture gap (separate /trace)
Regression tests (4 new + 1 updated in scheduler-daemon.test.ts):
- reconcileUnresumable flips null-session error rows to auto_resume=false
- reconcileUnresumable leaves valid-session + already-disabled + healthy rows untouched
- attemptAgentResume exhaustion branch persists auto_resume=false + logs event
- Subsequent runAgentRecoveryPass tick does not re-log agent_resume_exhausted
- Existing `exhaustion check fires on re-entry` test updated to assert the new
terminal-flag write (autoResume=false, no counter mutation on early-exit)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 addresses issues with agents getting stuck in terminal states by ensuring that agents in an error state without a session ID, or those that have exhausted their retry budget, are correctly marked with autoResume=false. This prevents redundant logging and ensures the UI accurately reflects the agent's status. The feedback suggests optimizing the reconciliation process by performing database updates in parallel and ensuring the new reconciliation pass is also executed during the daemon's startup recovery sequence.
| for (const worker of workers) { | ||
| // Terminal boundary condition: error-state, auto-resume on, no session id. | ||
| // `!worker.claudeSessionId` matches both `null` (DB) and `undefined` | ||
| // (listWorkers mapping for missing column). | ||
| if (worker.state !== 'error') continue; | ||
| if (worker.autoResume === false) continue; | ||
| if (worker.claudeSessionId) continue; | ||
|
|
||
| try { | ||
| await deps.updateAgent(worker.id, { autoResume: false }); | ||
| deps.log({ | ||
| timestamp: deps.now().toISOString(), | ||
| level: 'warn', | ||
| event: 'agent_marked_unresumable', | ||
| agent_id: worker.id, | ||
| reason: 'no_session_id', | ||
| state: worker.state, | ||
| }); | ||
| flipped++; | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| deps.log({ | ||
| timestamp: deps.now().toISOString(), | ||
| level: 'warn', | ||
| event: 'agent_marked_unresumable_failed', | ||
| agent_id: worker.id, | ||
| error: message, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation processes unresumable agents sequentially using await inside a loop. If a systemic issue causes a large number of agents to become wedged, these sequential database updates could significantly delay the completion of the reconciliation pass and the subsequent recovery pass. Consider using Promise.all to perform these updates in parallel for better efficiency, as each agent update is independent.
| // null session ids stayed in `auto_resume=true` forever, misleading | ||
| // `genie ls` and cluttering the worker list. | ||
| await reconcileDeadPaneZombies(d); | ||
| await reconcileUnresumable(d); |
There was a problem hiding this comment.
While reconcileUnresumable is correctly added to the periodic timer, it appears to be missing from the recoverOnStartup function. This means that after a daemon restart, wedged agents will continue to show as 'auto-resume: on' in the UI until the first timer tick (default 60s). Adding this pass to recoverOnStartup would ensure the state is reconciled immediately upon startup, providing a more accurate UI experience.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7624e609fd
ℹ️ 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".
| // `attempts >= maxAttempts` branch and re-log `agent_resume_exhausted` every | ||
| // 60s forever. | ||
| if (newAttempts >= maxAttempts) { | ||
| await deps.updateAgent(agentId, { autoResume: false }); |
There was a problem hiding this comment.
Persisting autoResume: false on exhaustion here makes the disablement sticky across manual recovery: genie agent resume re-registers the worker using the existing agent.autoResume value, so a successfully resumed agent stays opted out of future scheduler retries. Because the CLI resume path only resets resumeAttempts (not autoResume), an agent that exhausts once can silently lose auto-recovery forever unless the flag is manually edited in storage; this is a behavioral regression from before this commit.
Useful? React with 👍 / 👎.
…y spam Two retry-forever spam bugs in the observability loops — both follow the pattern we've been closing all session: recoverable-looking error on a fast-polling path with no circuit breaker → same log line every 30s. Bug A — filewatch FK violation (session-filewatch.ts) Orphan subagent JSONLs (parent session not in PG sessions table, e.g. SDK agents) made ingestFileFull raise FK constraint 23503. The catch block logged but did NOT advance offsetCache, so every subsequent fs.watch event retried the same file at offset 0 → same FK error → same log. Forever. Fix: detect FK (code 23503 or "foreign key constraint" in message), pin offsetCache to Infinity AND add the session to an unrecoverable set so future events short-circuit before touching ingest. Non-FK errors (connection reset, deadlock) still preserve retry semantics. Bug B — inbox-watcher missing workingDir (inbox-watcher.ts) checkInboxes iterates teams with unread messages. If workingDir is missing from a team's config, it warned every poll cycle — same 3-4 teams re-warning every 30s forever (live-observed on felipe). Fix: mirror the existing spawnFailures rate-limit — new noWorkingDirWarned Map warns once per team, silences for 1 hour, then re-fires (so configs that get populated upstream naturally recover). Pattern continuity with #1187/#1192/#1195 — same shape, same fix class: observability path sees recoverable-looking condition → no circuit breaker → floods logs. Staged follow-up (out of scope): separate /trace needed upstream on why teams register without workingDir (team-config creation bug). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two related bugs, one cohesive PR — both variants of terminal-state agents the scheduler kept re-processing every cycle.
state='error', auto_resume=true, claude_session_id=nullare permanently wedged — the resumable filter drops them (no session id), yet they keepauto_resume=trueforever. 9 such rows observed live on felipe's machine (8genie-docsdir-placeholders + 2 omni workers that died pre-session-capture).genie lsmisleadingly showedauto-resume: on+0/3forever.attempts >= maxAttemptsearly-exit inattemptAgentResumeloggedagent_resume_exhaustedand returned, but did NOT persistauto_resume=false. Next tick (60s) re-entered the same branch → redundant event spam for every exhausted agent, every minute, forever.Both fixes apply the same terminal-boundary invariant: set
auto_resume=falseat the point the scheduler concludes the agent is unrecoverable. Shared primitive (deps.updateAgent), shared filter semantics (theautoResume === falseearly-skip already exists inattemptAgentResume).Files touched
src/lib/scheduler-daemon.ts— new exportedreconcileUnresumable(deps), wired intostartAgentResumeTimerbetween the existing zombie GC and the resume pass. Both exhaustion branches inattemptAgentResumenowawait deps.updateAgent(id, { autoResume: false })before logging.src/lib/scheduler-daemon.test.ts— 4 new regression tests + 1 existing test updated to assert the new terminal-flag write.Prior art
claudeSessionIdpredicate)--no-reset-attempts, made exhaustion actually reachable)This is the final cleanup pass closing the "terminal states should be terminal" theme.
Staged follow-ups (explicitly out of scope)
dir:registry rows end up instate='error'— separate/trace(tui-spawn--team is requiredfailure path)claudeSessionId— separate/traceTest plan
bun run typecheck— 0 errorsbun run check— 2668 pass / 0 failreconcileUnresumableflips null-session error rows + logsagent_marked_unresumablereconcileUnresumableleaves valid-session / already-disabled / healthy rows untouchedauto_resume=falsealongside theagent_resume_exhaustedlog eventrunAgentRecoveryPasstick does NOT re-logagent_resume_exhaustedfor the same agentexhaustion check fires on re-entryupdated: now asserts exactly 1updateAgentcall withautoResume: false+ no counter mutationgenie ls | grep -E "genie-docs|omni"should flip toauto-resume: offwithin oneleaseRecoveryIntervalMscycle;tail -30 ~/.genie/logs/scheduler.log | grep agent_resume_exhausted | wc -lshould show each agent ONLY ONCE🤖 Generated with Claude Code