fix: recover Instant sessions across runtime loss#1660
Open
simple-agent-manager[bot] wants to merge 8 commits into
Open
fix: recover Instant sessions across runtime loss#1660simple-agent-manager[bot] wants to merge 8 commits into
simple-agent-manager[bot] wants to merge 8 commits into
Conversation
Address all CRITICAL/HIGH findings from the eight-reviewer pass on the
Instant runtime recovery state machine:
- ensureAwake() honors a concurrent explicit stop landing in the
post-classification window ('stopping' guard) and stops the already
started container when a stop crosses a wake, instead of leaking it
until sleepAfter
- vm-agent no longer burns the LoadSession restore identity before a
restore attempt succeeds, serializes concurrent RestoreAgent calls,
stops orphaned agent processes on selection failure, and tolerates a
browser-reconnect session registration during restore
- HOME snapshots exclude credential-bearing paths (.ssh, .aws, .netrc,
.npmrc, .config/gh, .claude/.credentials.json, .codex/auth.json)
while keeping harness transcript state; staged oversized files can no
longer bypass the WIP bundle size budget via the index tree
- destructive error/failed callbacks from a superseded container
generation are rejected by a freshness guard
(INSTANT_STALE_CALLBACK_MARGIN_MS, fail-open), and the resume route
re-fetches instead of clobbering the DO-written manual-retry state
- stuck-task resumable short-circuit is gated on cf-container runtime so
VM workspaces stuck in recovery on a dead node are conclusively
reconciled again
- recovery UI: failed resumes reset the working indicator, successful
sends clear stale delivery banners, RUNTIME_STOPPED terminates the
composer via the existing terminated presentation, auto/manual resume
is re-entrancy guarded, and failed deliveries keep the composer text
- Miniflare workers pool binds a VM_AGENT_CONTAINER test double that
reuses the real lifecycle inspection + classifier (real Container
class cannot boot without ctx.container), making the stale-heartbeat
and new three-actor sweep tests discriminating
- additive partial index idx_tasks_workspace_id (migration 0097) for
the recovery-exhaustion task lookup hot path
Every fix ships with a regression test verified to fail on the pre-fix
code. Deferred items are tracked backlog tasks (stale non-destructive
callbacks; DO-RPC request cancellation, evidence PR #1544; SANDBOX_*
env fallback cleanup).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Comment-only change (no behavior): vm-agent-container.ts already delegates to four extracted modules; the remainder is the mutex-guarded lifecycle state machine kept reviewable as one unit per rule 45. node-agent.ts is marginally over with one cohesive cross-boundary timeout concern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gate 3 evidence: strict idle wake with restored HOME/index/worktree/ harness phrase and recorded restore_status; two real rollout kills (idle-state and mid-active-work) classified as recovery, task never failed, in-flight prompt never replayed, same-chat follow-ups restored in under a minute; staging test resources cleaned up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Fixes the remaining production reliability failure where an Instant (Cloudflare Container) session appears to lose its work after the user steps away for ~20–36 minutes: they return to the same chat, send a follow-up, and either nothing answers or the task/workspace appears gone — despite a valid snapshot and preserved transcript.
Production evidence (2026-07-16 incidents, correlated across D1, DO state, snapshots, and deploy timelines) showed three unsynchronized lifecycle owners destroying recoverable work:
VmAgentContainer.onStop()mapped every unexpected stop (including CF deployment rollouts) to terminalerror, andresolveLiveAgentSessionForChat()'s dead-node precheck then rejected follow-ups before the DO could restore. The web client swallowed the failed/prompt, making outcomes look random.sleepingworkspaces as conclusively dead (workspace_sleeping) and destroyed them (found by staging gate 1).task_acp_session_not_live), failed the task, and deleted the workspace/node (found by staging gate 2).What ships
VmAgentContainer(persisted typed recovery record; mutex-serialized wake/restore; idempotentonStop/onError; bounded attempts viaCF_CONTAINER_RECOVERY_MAX_ATTEMPTS): unexpected loss of an established runtime becomes generic recoverable state — never auto-labeled "rollout", never terminal on first sight. Explicit stop stays terminal, including when it races an in-flight wake (the raced wake now also stops the already-started container instead of leaking it untilsleepAfter).LoadSession— noNewSessionmasquerading as resume. Restore identity is retained across transient failures; concurrent restores are serialized.inspectLifecycle(); sleep/wake/restore/recovery is inconclusive/resumable, explicit stop/expired/error is terminal — a sleeping Instant session can no longer be destroyed by a stale secondary replica. VM/devcontainer behavior is unchanged (resumable short-circuit is gated onnode_runtime === 'cf-container'; VMrecoveryrows keep conclusive dead-node detection).RUNTIME_STOPPEDflows into the terminated-session presentation, stale error banners clear on later success, and a failed resume no longer strands "Agent is working…"..ssh,.aws,.netrc,.npmrc,.config/gh,.claude/.credentials.json,.codex/auth.json) — restore reinjects fresh credentials, so nothing depended on snapshotted secrets; destructive callbacks from a superseded container generation are rejected by a freshness guard (INSTANT_STALE_CALLBACK_MARGIN_MS, fail-open).Full production evidence, the lifecycle contract diagram, both failed-gate records, and the control-loop I/O budget live in
tasks/active/2026-07-21-instant-runtime-recovery-state-machine.md.Control-loop budget (rule 47)
Candidate selection is unchanged in both loops. Per stale Instant ACP candidate the alarm adds one indexed D1 lookup + one read-only
inspectLifecycle()DO RPC (three storage keys, no container start); per running Instant stuck-task candidate the sweep substitutes the same RPC for stale prechecks. Both bounded byTASK_LIVENESS_PROBE_TIMEOUT_MS(5s background default); deferred stale alarms reschedule no sooner than one detection window; every deferred candidate escapes via resumed heartbeat, explicit stop/destroy, terminal error/expiry, or configured idle cleanup. Two-sweep zombie-prevention tests included. New additive partial indexidx_tasks_workspace_id(migration 0097) covers the recovery-exhaustion task lookup.Validation
pnpm lint(7/7)pnpm typecheck(16/16)pnpm test(19/19 turbo tasks; API 449 files / 6,322 tests; one transient parallel-load flake did not reproduce across three subsequent full runs)vet+test+-raceforinternal/server+internal/acp(one pre-existing docker-dependent test,TestBootstrapLifecycle_SessionsUseDetectedUser, fails locally for lack of a Docker daemon — environmental, unrelated, covered by CI);quality:migration-safety,quality:do-migration-safety, migration ordering, wrangler-bindings all green. Local Miniflare workers pool cannot run in this workspace (workerd 1.20260329.1SIGSEGV on startup — recorded gap); the new/changedtests/workers/**suites are validated by CI on this PR.Staging Verification (REQUIRED for all code changes — merge-blocking)
31737ae39(deploy jobs green first try; one transient networkidle smoke flake passed on rerun); runs 29875487605 / 29876306236 / 29877444911 deployed the final comments-only head5bc143f8cfully green including the 12-test smoke suiteStaging Verification Evidence
Gate 3 (this head) PASSED — full record in
tasks/active/2026-07-21-instant-runtime-recovery-state-machine.md§ "Third Exact-Head Staging Gate". Highlights, all from a genuine UI-created Instant session (project01KY3D007EA6JNCCHSK5YCFEYK, chat60d82ff6, nonce HPPQ0F):availablenon-degraded snapshot and survived ~8 min of sleep across the 5-minute reconciliation sweep (exactly where the two prior gates were destroyed); the same-chat follow-up restored everything in ~30s — exact porcelain, HOME contents, and the phrase (real LoadSession).session_snapshots.restore_status='restored'/restored_atwritten — the fields that were NULL in every production incident.recoveryworkspace and restored in ~40s, no replay.activity=prompting), the rollout killed the container at 23:44:59 — workspace+agent session classifiedrecovery, the sanitized "Instant session interrupted; restoring the last safe checkpoint…" disposition persisted (un-clobbered), task stayedin_progressacross 8 minutes of sweeps, the in-flight prompt was never replayed, and the same-chat follow-up restored phrase + exact porcelain in ~28s. The post-checkpoint tick log honestly reported missing (last-safe-checkpoint semantics), while the restored harness retained pre-checkpoint conversation memory — LoadSession continuity across two kills.Prior-head gates 1 (
680db591d, failed usefully) and 2 (4b461c051, idle wake passed; exposed destroyer #3) remain recorded in the task file as evidence.UI Compliance Checklist (Required for UI changes)
min-w-0 flex-1 break-words [overflow-wrap:anywhere], flex-column stacking with composer)role="status"waking banner,role="alert"error banner, keyboard-reachable Dismiss)RUNTIME_STOPPED; existingElapsedTimereused in the waking banner)instant-runtime-recovery-audit.spec.ts: 30 tests green across mobile (375×667) + desktop (1280×800) + themes, covering normal/waking/degraded/interrupted/stopped scenarios with long unbroken-string stress data andassertNoOverflow; screenshots in.codex/tmp/playwright-screenshots/End-to-End Verification (Required for multi-component changes)
ProjectData.alarm()+ realrecoverStuckTasks()+ VM_AGENT_CONTAINER lifecycle double reusing the real classifier; DO recovery tests driveensureAwake()→ wake → restore → D1 reconciliation with dynamic state-mutating mocks)Data Flow Trace
Untested Gaps
Local Miniflare workers-pool suites cannot execute in this workspace (workerd SIGSEGV) — CI on this PR is the automated gate for
tests/workers/**. A real Cloudflare container rollout cannot be simulated locally at all — that is exactly what staging gate 3 (forced replacement while work is active on this exact head) verifies live; evidence appended before merge.Post-Mortem (Required for bug fix PRs)
What broke
A user could leave a valid Instant conversation for ~20–36 minutes, return, send a follow-up, and get no answer — or find the task/workspace destroyed — despite a safe snapshot and preserved transcript.
Root cause
Runtime liveness had multiple unsynchronized owners:
VmAgentContainer.onStop()terminalized every unexpected stop; dead-D1 prechecks blocked the runtime owner's recovery path; the ProjectData ACP alarm treated a sleeping container's intentionally absent heartbeat as terminal; the stuck-task sweep trusted that stale replica and deleted recoverable work.Class of bug
Cross-control-plane lifecycle races where a stale replica is treated as authoritative terminal state and destructive reconciliation runs before the runtime owner can classify recovery.
Why it wasn't caught
Tests exercised each state store in isolation; no regression combined a sleeping/recovering runtime owner with a stale ACP heartbeat and a later cleanup sweep. The first live verifier also accepted prompt text and negative explanations as success.
Process fix included in this PR
.claude/rules/02-quality-gates.md— multi-control-plane lifecycle regression requirement: stale-secondary-state tests plus one shared liveness classifier across timeout/cleanup paths. The exact-head verifier methodology (role-aware persisted transcript evidence, observable active work, strict no-replay assertions) is recorded in the task file.Post-mortem file
tasks/active/2026-07-21-instant-runtime-recovery-state-machine.md(Post-Mortem + both failed-gate records)Specialist Review Evidence (Required for agent-authored PRs)
31737ae39(probe-timeout doc description,resolveRuntimeSettingsparsing tests); pre-existingSANDBOX_*fallback → backlog task31737ae39stoppingguard) + HIGH leaked-container-on-stop-crossed-wake + HIGH resume clobber fixed in31737ae39;idx_tasks_workspace_idadded; DO-RPC abort deferred with evidence (PR #1544) → backlog task31737ae39; restore-registration race confirmed real and fixedINSTANT_STALE_CALLBACK_MARGIN_MS, fail-open); non-destructive stale surfaces → backlog taskVM_AGENT_CONTAINERbound via real-classifier test double; sweep tests through realrecoverStuckTasks()with explicitnode_runtime); legacy-bundle, launch-transition, andApiClientError-code coverage addedRUNTIME_STOPPED) + resume re-entrancy guard + composer-text retention, each with discriminating testsrecoveryrestored to VM live set) with cross-runtime regression testsAll discrimination checks (fix reverted → test red → fix restored) are recorded per suite. Deferred MEDIUM/LOW items are tracked in three backlog tasks added by this PR (rule 42 compliant).
Exceptions (If any)
Agent Preflight (Required)
Classification
External References
Cloudflare Containers docs (container-class, platform-details/architecture, platform-details/rollouts) — cited in the task file;
@cloudflare/containers@0.3.7dist sources read directly to verifyContainerconstructor behavior (ctx.containerrequirement) for the workers-pool binding decision; production D1/DO/snapshot records queried via the read-only CF API as primary evidence.Codebase Impact Analysis
apps/api(VmAgentContainer DO + recovery/lifecycle/runtime modules, ProjectData heartbeat policy, stuck-task sweep, chat resolver, resume route, activity/task callbacks, node-agent service, schema + migration 0097),apps/web(project-message-view recovery UI),packages/shared(session/workspace status types),packages/vm-agent(snapshot/restore, WIP bundle encoding, RestoreAgent/LoadSession selection),scripts/deploy(env passthrough), docs (configuration.md),.claude/rules/02.Documentation & Specs
apps/www/src/content/docs/docs/reference/configuration.md(CF_CONTAINER_* +INSTANT_STALE_CALLBACK_MARGIN_MS+ probe-timeout description),apps/api/.env.example,.claude/skills/env-reference/SKILL.md,.claude/rules/02-quality-gates.md(process fix), task file as the durable post-mortem record.Constitution & Risk Check
Principle XI verified by constitution-validator (PASS): all new knobs are env-configurable with named defaults (
CF_CONTAINER_RECOVERY_MAX_ATTEMPTS,INSTANT_STALE_CALLBACK_MARGIN_MS, reusedTASK_LIVENESS_PROBE_TIMEOUT_MS/CF_CONTAINER_WAKE_TIMEOUT_MS), documented in all four reference surfaces and threaded through the deploy passthrough. Key risks: DO concurrency (rule 45 — mutex-serialized with discriminating overlap tests), control-loop I/O budget (rule 47 — stated above), migration safety (additive index only, gates green), credential handling (snapshot exclusions + fresh reinjection; no secrets in logs).