Repository navigation
fix(scheduler): agents survive VM reboot — bridge JSONL session fallback + TMUX-presence shim - #2460
Conversation
…ack + TMUX-presence shim
The oldest genie problem: after a VM reboot (or pm2 restart of Genie),
no agents auto-resumed. Reference instance: 33 agents, 20 classified
`eager_invoke` by the boot pass, `recovery_completed: resumed_agents: 0`
on every restart. The recovery infrastructure existed (`recoverOnStartup`
→ `runBootPass` → `runAgentRecoveryPass` → `attemptAgentResume` →
`defaultResumeAgent`), but two coupled defects silently killed it:
1. **runAgentRecoveryPass over-filtered.** The filter at line 1126 was
`state != 'suspended/done' && currentSessionId`. But `currentSessionId`
comes from the `executors.claude_session_id` join, which is null for
most agents on a fresh boot — yet `genie status`'s classifier
(`shouldResume`) resolves the session via a BROADER `getResumeSessionId`
path: DB happy path → JSONL on-disk fallback (`~/.claude/projects/...`).
So bootPass eager-invoked 20 agents whose JSONLs sat right there on
disk, but the recovery pass filtered them out before the dead-pane
check could see them. Fix: drop the `currentSessionId` filter clause,
resolve inside the loop via `getResumeSessionId` for dead-pane
survivors only (cheap), and enrich `worker.currentSessionId` in-place
so attemptAgentResume can issue `claude --resume <uuid>`. SDK / non-tmux
agents naturally fall out at attemptAgentResume's `no_session_id`
early-exit when neither source yields a UUID.
2. **defaultResumeAgent shelled without TMUX.** `genie agent resume <id>`
errors with `Cannot resume agent "<id>": no team context (template,
agent record, env, or session). Pass --team or set GENIE_TEAM, or run
inside a registered tmux session.` for `dir:`-prefixed agents whose
`team` is NULL (29/33 on the reference instance). The scheduler is a
pm2 child (never inside tmux) and shelled the CLI bare → exit 1 →
execSync threw → bare `catch {}` swallowed everything → silent
`agent_resume_failed`. The CLI's check is just env-presence (value
isn't validated). Fix: pass a synthetic `TMUX` sentinel when the
parent process has none, and surface previously-invisible CLI errors
behind `GENIE_DEBUG_RESUME=1` instead of dropping them on the floor.
Validated on the reference instance (33 agents, post-fix Genie restart):
recovery_completed → **resumed_agents: 17, failed_agents: 0** (was 0/0)
26 live tmux panes on the canonical /tmp/tmux-1000/genie socket (was 2)
20 agents in `spawning`/`idle` state in DB
`genie ls` shows real registered agents with running panes
biome + check:fast green.
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 improves the agent recovery process in the scheduler daemon. Key changes include adding a TMUX environment variable shim to bypass CLI session checks, relaxing the initial worker filter in the recovery pass to include agents without a session ID, and implementing a fallback mechanism to resolve session IDs from disk. The review feedback suggests further refining the worker filter to exclude agents in the 'archived' state to avoid unnecessary liveness probes and disk scans for inert sessions.
| // pass it through; SDK / non-tmux agents naturally fall out at | ||
| // attemptAgentResume's `no_session_id` early-exit when neither source | ||
| // has a UUID. | ||
| const resumable = workers.filter((w) => w.state !== 'suspended' && w.state !== 'done'); |
There was a problem hiding this comment.
The filter for resumable workers should also exclude agents in the archived state. While archived is not currently in the AgentState type union, it is a valid state in the database (used by archiveExhaustedZombies) and represents agents that are explicitly marked as non-resumable and inert. Excluding them here avoids unnecessary liveness probes and disk scans for sessions that will ultimately be skipped by attemptAgentResume later in the loop.
const resumable = workers.filter((w) => w.state !== 'suspended' && w.state !== 'done' && (w.state as string) !== 'archived');![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: 71b7d4bc41
ℹ️ 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".
| const hasTmuxPane = typeof worker.paneId === 'string' && /^%\d+$/.test(worker.paneId); | ||
| const alive = hasTmuxPane ? await deps.isPaneAlive(worker.paneId) : false; |
There was a problem hiding this comment.
This pass now treats any worker whose paneId is not a tmux pane id as dead (alive = false), and because the prefilter was widened to state-only, SDK/inline workers can enter this path even when they are healthy. In sweep mode (turn-aware is default-on), an idle non-tmux worker will go through handleDeadPane and can be terminalized as clean_exit_unverified, corrupting live executor state simply because it has no %<n> pane. Before this commit, many of these rows were filtered out by currentSessionId, so this introduces false terminalization/auto-resume churn for non-tmux transports.
Useful? React with 👍 / 👎.
CI failure on PR #2460 caught by state-machine.invariants.test.ts (invariant 3): only src/lib/should-resume.ts and the definition site may import getResumeSessionId() — every consumer must read the ShouldResumeResult instead. My fix imported the raw helper directly in scheduler-daemon.ts, breaking the chokepoint contract. Switch the dynamic import to `shouldResume` and read `decision.sessionId`. Same semantics for the enrichment path (DB happy path → JSONL fallback), respects the architectural contract, and incidentally gives the recovery pass access to `decision.resume` if a future change wants to short-circuit auto_resume_disabled / assignment_closed cases here too. Validated: - bun test src/__tests__/state-machine.invariants.test.ts → 1 pass / 0 fail - bun run typecheck → clean - biome check src/lib/scheduler-daemon.ts → clean Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three bot-reviewed correctness fixes on the v4.260520.1 recovery work. HIGH-1: codex P1 + gemini M on PR #2461. shouldResume returns sessionId for assignment_closed and auto_resume_disabled rows (for genie status display), not only for the resumable ok case. Previous code gated enrichment on decision?.sessionId alone, so a task agent whose latest assignment was done/failed would be re-invoked on every reboot — JSONL on disk, sessionId resurfaced, currentSessionId enriched, attemptAgentResume re-spawned. Fix: gate on decision.resume && decision.sessionId. HIGH-2: codex P1 on PR #2460. Dropping the currentSessionId clause widened the prefilter to state-only, so SDK/inline workers (paneId = synthetic id like 'sdk', not %N) entered the loop. With hasTmuxPane=false, alive was false and the worker fell into handleDeadPane. In sweep mode with turn-aware enabled, an idle SDK worker hits terminalizeCleanExitUnverified — a state-mutating write that corrupts live executor state for healthy non-tmux workers. Fix: prefilter on paneId shape (^%\d+$ only). Mirrors the canonical transport gate in countActiveWorkers. Also adds archived to excluded states (gemini M on #2460). MEDIUM: CodeRabbit major on PR #2461. .catch(() => null) on shouldResume converted transient resolver/DB failures to decision=null, the worker fell through unenriched, attemptAgentResume counted it as no_session_id and incremented the retry budget. After 3 transient blips a healthy worker's auto_resume flipped to false. Fix: let shouldResume throw — the outer try/catch routes through handleRecoveryProbeError or increments failed. LOW polish: hoisted shouldResume import outside loop (gemini M on #2461). Added archived to excluded states. Validated: - invariant 3 in state-machine.invariants.test.ts: pass - bun test --test-name-pattern recovery: 10 pass / 0 fail - bun run typecheck: clean - biome check src/lib/scheduler-daemon.ts: clean Ignored CodeRabbit major "SHA drift on package.json" on #2461 — false positive, Binary SHA Drift workflow is not running on this PR. 17/17 CI green on #2461. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The oldest genie problem, fixed: agents survive VM reboot
After a VM reboot (or pm2 restart of
Genie), no agents auto-resumed — even when the boot pass classified them eligible. Reference instance: 33 agents, 20eager_invoke,recovery_completed: resumed_agents: 0every restart. The recovery chain existed (recoverOnStartup → runBootPass → runAgentRecoveryPass → attemptAgentResume → defaultResumeAgent) but two coupled defects silently killed it.Defect 1 —
runAgentRecoveryPassover-filteredLine 1126 filter was
state ∉ {suspended, done} && currentSessionId. ButcurrentSessionIdis theexecutors.claude_session_idjoin, which is null for most agents on a fresh boot.genie status's classifier (shouldResume) resolves the session via the BROADERgetResumeSessionIdpath: DB happy path → JSONL on-disk fallback (~/.claude/projects/<encoded-cwd>/<uuid>.jsonl). Boot pass eager-invoked 20 agents whose JSONLs sat right there on disk; recovery dropped them before the dead-pane check could see them.Fix: drop the
currentSessionIdfilter clause. Resolve inside the loop viagetResumeSessionIdfor dead-pane survivors only (cheap), enrichworker.currentSessionIdin-place soattemptAgentResumecan issueclaude --resume <uuid>. SDK / non-tmux agents naturally fall out atattemptAgentResume's existingno_session_idearly-exit when neither source yields a UUID.Defect 2 —
defaultResumeAgentshelled without TMUXgenie agent resume <id>errors withCannot resume agent "<id>": no team context (template, agent record, env, or session). Pass --team or set GENIE_TEAM, or run inside a registered tmux session.fordir:-prefixed agents whoseteamis NULL (29/33 on the reference instance — the common case post-reboot when the registry rebuilds from scaffolds). The scheduler is a pm2 child (never inside tmux) and shelled the CLI bare → exit 1 →execSyncthrew → barecatch {}swallowed everything → silentagent_resume_failed.The CLI's check is just env-presence (value isn't validated). Fix: pass a synthetic
TMUXsentinel when the parent process has none, and surface previously-invisible CLI errors behindGENIE_DEBUG_RESUME=1instead of dropping them on the floor.Validated on the reference instance, post-fix Genie restart
Every Genie supervisor restart now auto-resumes the eligible bucket. After a real VM reboot (where tmux panes are gone and all agents become "dead-pane" via the same code path), they all flow through the now-working recovery → spawn
claude --resume <stored sessionId>→ land on the canonical genie socket. TheUPGRADING-pgserve-v3.mdrunbook is now the fallback for the ~12 truly-orphan agents (no_session_id), not the primary path.biome + check:fast (typecheck, lint, dead-code, skills, wishes, emit-discipline) green.
🤖 Generated with Claude Code