feat(offload): workflow pane offload & resume (RFC §5)

[lavaman131]

Summary

Implements RFC §5: Workflow Pane Offload & Resume end-to-end — keeps long-running workflows from accumulating idle agent CLIs by killing each tmux pane + agent process as soon as its stage completes (or as soon as the user navigates away from it), and respawns the pane via <provider> --resume <id> when the user attaches again.

What's in this PR

• OffloadManager: state machine (alive/offloaded/resuming), per-name op queue for idempotency, schema-versioned metadata.json, telemetry sink, and tmux/provider seams. (af03f358, a25e029b, 2a166b96, a78c8dba)
• Per-stage offload + chrome-tab semantics: offloadSession() fires from the executor as each stage callback resolves, and from a focus-poll branch in SessionGraphPanel when the user navigates away from a pane. The eligibility check refuses to offload the user's currently-focused pane. (29003593)
• Resume args fix for opencode/copilot: --port 0 / --ui-server --port 0 mirrors the original spawn, otherwise the resumed CLI never binds a TCP port and waitForServer times out. (a19d7fb1)
• Claude SessionStart hook fix: matcher widened from startup to startup|resume so the claude-ready/<id> marker fires on claude --resume and waitForClaudeReady doesn't hang. (c673f62f)
• Status rendering: offloaded (◌) and resuming… (◐) statuses with theme-aware colors and header badges, behind a single-source STATUS_TABLE. (0cd45f33, 72c23679)
• Toast UI (RFC §5.5): top-right ToastStack component with severity (info/warning/error), auto-dismiss timer (unref()'d), and a kind-aware store API. (d8469d8e)
• Schema-guard tests for empty agentSessionId across all three providers. (4c942544)
• Defer-render fix for OrchestratorPanel: skips the initial null-manager render so useOffloadManager doesn't throw before attachOffloadManager fires. (a2cef007)
• Test coverage gaps closed for shellQuote and getProductionTelemetrySink so pre-push test:coverage hits the 0.85 threshold. (e56d4300, ca6a2546)
• Ralph reviewer fix: switches the Claude reviewer stage from agent: "reviewer" to systemPrompt.append, since the SDK silently disables outputFormat when both are passed and the loop never converges. (705e5465)
• MCP env hardening: neutralize host git config on the ast-grep MCP server entry across all subagent configs. (6c30f9c7)
• hello-world example: add .opencode config so the example picks up the same MCP wiring as the workspace. (8146e013)

Test plan

• bun lint and bun typecheck — pass
• bun test --coverage packages tests — 2535 tests, 0 fail, all coverage thresholds met
• Manual: run a multi-stage workflow with claude/opencode/copilot, navigate away from each completed pane, observe the chrome-tab-style offload (status badge flips to offloaded ◌)
• Manual: re-attach to an offloaded pane, observe resuming… then alive with the same agent session id
• Manual: ralph workflow with claude — verify the reviewer loop converges (structured output reaches hasActionableFindings)
• Manual: verify offload/resume telemetry events land in ~/.atomic/sessions/<runId>/telemetry.jsonl

[claude]

PR Review — feat(offload): workflow pane offload & resume

This is a substantial, well-architected feature with strong test coverage (28+ new test files, 2535 tests passing per the PR description). The state machine, schema-versioned metadata, telemetry, and lint guards are all thoughtful design choices. Below are findings, ordered by impact.

Overall

• Strong: per-stageDir mutex on persistResume, schema-version pinning, telemetry constants centralized, idempotent op-queue, the dedicated lint-offload-await.ts rule, and the rationale-rich docstrings (especially around the Stop/SessionStart hook semantics and the --port 0 / --ui-server resume args).
• Solid: test coverage targets the right seams — Bug 1 (cascade isolation via Error identity), Bug 2 (null-safe schema guard), claudeOffloadCleanup, doResume rollback, the buildResumeArgs trio.
• One overall concern: the in-process OffloadManager state is not persisted across an atomic restart (in-memory sessions Map is lost). Resume only works within a live orchestrator process. That seems intentional given the RFC scope, but it deserves a clear ""out of scope"" note in offload-manager.ts so future readers don't assume durability.

---

Bugs / correctness

1. doResume rollback can leave the tmux window alive while state says ""offloaded"" — packages/atomic-sdk/src/runtime/offload-manager.ts:489-499

   If windowCreated is true and the post-create rollback killWindow itself fails, telemetry emits RESUME_ROLLBACK_FAILED and sess.state is set to ""offloaded"". A subsequent requestResume will go through doResume again and call tmux.createWindow against a window name that already exists, which tmux new-window rejects. The user gets stuck in a permanent ""resume failed"" loop with no automated recovery. Consider either: (a) marking the session as terminally error after a failed rollback, (b) detecting the existing window in createWindow and reusing it, or (c) at minimum surfacing a toast guiding the user to manually kill the wedged window.

2. offloadSession is awaited inside the stage's success path — packages/atomic-sdk/src/runtime/executor.ts:2231

   await shared.offloadManager.offloadSession(name) runs before completedRegistry.set(name, result) and resolveDone(). A slow killOnePane (e.g. tmux contention, claude marker fs.rm I/O) delays dependent stages from observing this stage's done promise, serializing parallel branches. Not a correctness bug, but it eliminates parallelism gains during workflow execution. Consider firing offload via void ... .catch(...) so dependents resolve immediately, with offload completing in the background — the per-name op queue already coalesces with onWorkflowCompletion's sweep.

3. Eligibility check reads panelStore.activeAgentId without ordering guarantees — packages/atomic-sdk/src/runtime/offload-manager.ts:420-426 and packages/atomic-sdk/src/components/session-graph-panel.tsx:419-450

   The focus-leave logic in session-graph-panel.tsx updates setViewMode first, then calls offloadSession(prevName). That works for the chrome-tab path. But the per-stage offload at executor.ts:2231 fires from a different thread of control entirely, and there's no guarantee that activeAgentId reflects the currently-displayed window — tmux focus changes are detected by a 500 ms poll, not by an immediate event. Window: a stage completes, offloadSession fires, but the user has not yet been polled into being the ""active"" focus → the eligibility check refuses to offload because some other pane is registered as focused. Stage 0 from a long-running poll cycle could silently keep the pane open. Worth adding a comment explaining this is best-effort, and verifying the onWorkflowCompletion sweep is the safety net.

4. buildResumeCommand for Claude omits --session-id but original spawn includes it — packages/atomic-sdk/src/providers/claude.ts:451-1465

   Original spawn (line 442-450) is claude <chatFlags> --settings <path> --session-id <UUID> 'Read prompt'. Resume is claude --resume <UUID> <chatFlags> --settings <path>. That's correct if --resume <UUID> substitutes for --session-id <UUID> in Claude Code's CLI. Worth confirming the Claude CLI accepts both interchangeably and that --resume doesn't expect a fresh session-id; otherwise the resumed CLI may run in a slightly different mode than the original. (Tests cover the argv shape but not the CLI's interpretation.)

5. Per-stage and offload cleanup overlap for Claude — packages/atomic-sdk/src/runtime/executor.ts:2208 then :2231

   cleanupProvider → clearClaudeSession(paneId) already unlinks pid, ready, inflight markers. Then offloadSession → claudeOffloadCleanup(agentSessionId) does it again (and adds claude-stop). The cleanup is idempotent (ENOENT-tolerant), so this is correct — but it's two cleanup paths doing overlapping work. Worth either consolidating into a single helper or adding a comment in killOnePane noting the redundancy is intentional and idempotent.

Style / maintainability

6. _testOnlyGetOrStartOp is exported as a public symbol — packages/atomic-sdk/src/runtime/offload-manager.ts:324

   The _testOnly prefix doesn't actually prevent it being part of the package API surface — anyone can import it. Consider moving it to a separate internal/ module or the test file, or guard with if (process.env.NODE_ENV !== ""test"") throw.

7. SPAWN_ENV_PREFIX_ALLOW deserves an inline rationale comment — packages/atomic-sdk/src/runtime/offload-manager.ts:33

   Allowing every OPENCODE_* and COPILOT_* env var to persist to disk is broad. Some upstream projects use those prefixes for tokens (e.g. COPILOT_API_KEY could land in disk if it ever exists). The suffix-deny _(API_KEY|...)$ catches the obvious cases, but a comment noting ""prefix allowlist is broader than denylist by design — relies on suffix-deny + audit"" would help future maintainers.

8. session-graph-panel.tsx 500 ms tmux poll is forking subprocesses on every tick — packages/atomic-sdk/src/components/session-graph-panel.tsx:400-454

   tmux display-message spawns a subprocess synchronously every 500 ms for the entire orchestrator lifetime. On a large workflow this is hundreds of forks. Tmux supports wait-for and hooks (window-active, client-active) that could push this rather than polling. Not a regression vs. existing behavior — the same tick already runs unrelated work — but worth noting if you're tuning idle-orchestrator CPU later.

9. Empty in-memory state on register vs. eligible-for-offload race — packages/atomic-sdk/src/runtime/offload-manager.ts:514-541

   registerSession uses getOrStartOp(\register:${name}`, ...)to queue the persist, butoffloadSessionkeys on plainname. They don't coalesce, and the per-stageDir persistResume mutex is what actually serializes disk writes. The in-memory state mutation (sessions.set(input.name, ...)) happens *synchronously* before the await, which is correct. Adding a brief comment to registerSession explaining the dual-key choice (""register:""` prefix to allow concurrent register+offload via separate op slots, mutex still serializing disk) would help — it took a few read passes to confirm the design is sound.

Tests

10. Missing test: rollback failure leaves window-create succeeding for next attempt

    The doResume-rollback.test.ts covers rollback emission of RESUME_ROLLBACK_FAILED but I don't see a test for the follow-up resume attempt that would hit the ""window already exists"" path described in finding add agent instructions #1. Worth adding one.

11. Missing integration test: parallel stages where one offloads while another registers in the same stageDir is covered indirectly by the cascade test, but an explicit ""register A, register B, offload A, register C"" sequencing test against a real mkdtemp would catch any future regressions in the dual-key op queue.

Security

• Token denylist (SPAWN_ENV_SUFFIX_DENY = /_(API_KEY|AUTH_TOKEN|SECRET|TOKEN|PASSWORD)$/i) is solid. The 0o600 mode on the metadata .tmp file before rename is correct. Tokens stay in-memory at resume time (re-injected from sess.spawnEnv), as documented.
• One small note: telemetry events include runId, name, agent, and error messages — make sure error message strings (e.g. from createWindow failing) cannot contain credentials before they land in ~/.atomic/sessions/<runId>/telemetry.jsonl. Stack traces from the providers are usually fine; but if a future provider error includes URL with embedded basic-auth, that would persist. Probably defer to a follow-up.

Conformance with CLAUDE.md

• All Bun-only conventions followed (Bun.write, Bun.spawn, no node/npm).
• No any/unknown types in the new public APIs (good).
• New deps: none.
• No new *.md files created beyond the spec (correct).

Bottom line

This is well-architected and the test coverage is thorough. The most actionable item is finding #1 (rollback wedge) — that has a real chance of stranding users. #2 (await-blocking parallelism) and #3 (focus race in eligibility) are worth a second look but may be acceptable given the RFC's scope. Everything else is polish.

— Review by Claude Code (claude-opus-4-7)