Skip to content

fix: recover Instant sessions across runtime loss#1660

Open
simple-agent-manager[bot] wants to merge 8 commits into
mainfrom
sam/diagnose-fix-remaining-production-0dvhf5
Open

fix: recover Instant sessions across runtime loss#1660
simple-agent-manager[bot] wants to merge 8 commits into
mainfrom
sam/diagnose-fix-remaining-production-0dvhf5

Conversation

@simple-agent-manager

@simple-agent-manager simple-agent-manager Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

DO NOT MERGE without separate explicit authorization (task 01KY27979PPENEWEM0PH0DVHF5 constraint). All gates are complete: CI fully green, specialist reviews addressed, and the exact-head staging gate (strict idle wake + two real rollout replacements, including one mid-active-work) PASSED — evidence below and in the task file.

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:

  1. VmAgentContainer.onStop() mapped every unexpected stop (including CF deployment rollouts) to terminal error, and resolveLiveAgentSessionForChat()'s dead-node precheck then rejected follow-ups before the DO could restore. The web client swallowed the failed /prompt, making outcomes look random.
  2. The stuck-task reconciler classified legitimately sleeping workspaces as conclusively dead (workspace_sleeping) and destroyed them (found by staging gate 1).
  3. The ProjectData stale-heartbeat alarm terminalized sleeping containers' ACP rows (heartbeats are intentionally absent while sleeping); the stuck-task sweep then saw the terminal replica (task_acp_session_not_live), failed the task, and deleted the workspace/node (found by staging gate 2).

What ships

  • Recovery state machine in VmAgentContainer (persisted typed recovery record; mutex-serialized wake/restore; idempotent onStop/onError; bounded attempts via CF_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 until sleepAfter).
  • Real wake/restore on the resume path: stopped/error/recovery D1 rows reach the DO recovery path instead of failing a dead precheck; the route re-fetches after recovery and no longer clobbers the DO-written manual-retry disposition.
  • Exact-resume fidelity (vm-agent): git WIP bundles encode separate worktree+index commits (staged/unstaged/untracked state survives restore exactly; legacy single-ref bundles still restore); the snapshot manifest persists the underlying ACP session ID + agent type and restore requires exact LoadSession — no NewSession masquerading as resume. Restore identity is retained across transient failures; concurrent restores are serialized.
  • Shared lifecycle classifier across control loops: ProjectData heartbeat alarms and stuck-task reconciliation both consult the DO's sanitized 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 on node_runtime === 'cf-container'; VM recovery rows keep conclusive dead-node detection).
  • Never replay an ambiguous prompt: a request crossing a runtime loss is classified and returned as an explicit manual-retry disposition; the transcript keeps the message; the composer keeps the text.
  • Truthful recovery UI: waking/restoring progress (with elapsed time), degraded recovery that never claims resume, RUNTIME_STOPPED flows into the terminated-session presentation, stale error banners clear on later success, and a failed resume no longer strands "Agent is working…".
  • Security hardening: HOME snapshots exclude credential-bearing paths (.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 by TASK_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 index idx_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)
  • Additional validation: Go vet + test + -race for internal/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.1 SIGSEGV on startup — recorded gap); the new/changed tests/workers/** suites are validated by CI on this PR.
  • Control-loop candidate-selection review stated above (rule 47)

Staging Verification (REQUIRED for all code changes — merge-blocking)

  • Staging deployment green — run 29872844972 deployed 31737ae39 (deploy jobs green first try; one transient networkidle smoke flake passed on rerun); runs 29875487605 / 29876306236 / 29877444911 deployed the final comments-only head 5bc143f8c fully green including the 12-test smoke suite
  • Live app verified via Playwright — authenticated via token-login; full gate-3 flow driven through the real UI (project wizard, profile form, chat composer, session view)
  • Existing workflows confirmed working — dashboard/projects/settings navigation covered by the green smoke suite; project creation, profile creation, chat, and session surfaces exercised repeatedly by the gate driver with zero API errors
  • New feature/fix verified on staging — strict idle wake AND two real rollout replacements verified end-to-end (evidence below)
  • Infrastructure verification — vm-agent snapshot/restore exercised end-to-end by real Instant container boots, sleeps, kills, and restores; no cloud-init/DNS/TLS changes
  • Mobile and desktop verification notes — local Playwright audit (30 tests, mobile+desktop+themes) green; staging flow exercised at desktop viewport with screenshots

Staging 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 (project 01KY3D007EA6JNCCHSK5YCFEYK, chat 60d82ff6, nonce HPPQ0F):

  • Strict idle wake: unique HOME marker + unstaged/staged/untracked git markers + a never-on-disk harness phrase; container slept with an available non-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_at written — the fields that were NULL in every production incident.
  • Rollout kill while idle: a real deploy-driven rollout hit the sleeping container; nothing failed; a later follow-up woke the recovery workspace and restored in ~40s, no replay.
  • Rollout kill mid-active-work (decisive): with a delivery-confirmed foreground loop running (activity=prompting), the rollout killed the container at 23:44:59 — workspace+agent session classified recovery, the sanitized "Instant session interrupted; restoring the last safe checkpoint…" disposition persisted (un-clobbered), task stayed in_progress across 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.
  • Cleanup verified: both test projects, all workspaces, and the container node row removed (D1-verified zero remaining).

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)

  • Mobile-first layout verified (recovery banners: min-w-0 flex-1 break-words [overflow-wrap:anywhere], flex-column stacking with composer)
  • Accessibility checks completed (role="status" waking banner, role="alert" error banner, keyboard-reachable Dismiss)
  • Shared UI components used (existing terminated-session presentation reused for RUNTIME_STOPPED; existing ElapsedTime reused in the waking banner)
  • Playwright visual audit run locally — 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 and assertNoOverflow; screenshots in .codex/tmp/playwright-screenshots/

End-to-End Verification (Required for multi-component changes)

  • Data flow traced (below)
  • Capability tests exercise the happy path across boundaries (workers-pool three-actor test: real D1 + real ProjectData.alarm() + real recoverStuckTasks() + VM_AGENT_CONTAINER lifecycle double reusing the real classifier; DO recovery tests drive ensureAwake() → wake → restore → D1 reconciliation with dynamic state-mutating mocks)
  • Spec/doc assumptions verified against code (production D1/DO evidence in the task file; PR Recover Instant sessions across container rollouts #1615's rejected assumptions documented)
  • Remaining gap documented below

Data Flow Trace

1. User returns to chat, sends follow-up
   → apps/web/.../useSessionLifecycle.ts:handleSendFollowUp() → useConnectionRecovery.ts:resumeAndSend()
   → POST /api/workspaces/:id/agent-sessions/:sessionId/resume
2. Resume route admits recovery-state rows (no dead precheck block)
   → apps/api/src/routes/chat-workspace-resolver.ts:resolveLiveAgentSessionForChat() (cf-container branch)
   → apps/api/src/routes/workspaces/agent-sessions.ts → services/vm-agent-container.ts:resumeVmAgentContainer()
3. DO classifies + wakes exactly once
   → durable-objects/vm-agent-container.ts:ensureAwake() (wakeChain mutex, attempt bound, stop-race guards)
   → wakeFromSnapshot(): startRuntime → vm-agent /restore (fresh callback tokens + credentials)
4. vm-agent restores exact state
   → packages/vm-agent/internal/server/session_snapshot.go:restoreSessionSnapshot()
   → HOME tar (credentials excluded → reinjected), WIP bundle worktree+index refs, manifest ACP identity
   → internal/acp/session_host_selection.go:RestoreAgent() → exact LoadSession (no NewSession fallback)
5. DO commits running + reconciles D1
   → vm-agent-container-recovery.ts:persistRuntimeRecovered() (node/workspace/agent-session/task rows; route re-fetches, no clobber)
6. Prompt delivered once; UI clears error state
   → services/node-agent.ts (cf-container wake-timeout budget) → chat → transcript
While sleeping/recovering, both control loops defer:
   → project-data/runtime-heartbeat-policy.ts:shouldDeferRuntimeHeartbeatTimeout() and
     scheduled/stuck-tasks.ts:getTaskRuntimeLiveness() → VmAgentContainer.inspectLifecycle() → shared classifier

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)

  • All local reviewers completed and findings addressed before merge
  • No reviewer failed or is pending
Reviewer Status Outcome
constitution-validator PASS No CRITICAL/HIGH; polish applied in 31737ae39 (probe-timeout doc description, resolveRuntimeSettings parsing tests); pre-existing SANDBOX_* fallback → backlog task
doc-sync-validator PASS Env docs verified exact; rule-02 process fix confirmed present; citations + naming note applied in 31737ae39
cloudflare-specialist ADDRESSED CRITICAL stop-race (stopping guard) + HIGH leaked-container-on-stop-crossed-wake + HIGH resume clobber fixed in 31737ae39; idx_tasks_workspace_id added; DO-RPC abort deferred with evidence (PR #1544) → backlog task
go-specialist ADDRESSED CRITICAL restore-identity burn + HIGH concurrent-restore double-spawn + orphaned-process stop + staged-file size-budget bypass fixed in 31737ae39; restore-registration race confirmed real and fixed
security-auditor ADDRESSED CRITICAL credential-bearing HOME snapshots (pre-existing from #1562) fixed; HIGH stale-generation callback regression guarded (INSTANT_STALE_CALLBACK_MARGIN_MS, fail-open); non-destructive stale surfaces → backlog task
test-engineer ADDRESSED Both CRITICAL non-discriminating flagship tests fixed (VM_AGENT_CONTAINER bound via real-classifier test double; sweep tests through real recoverStuckTasks() with explicit node_runtime); legacy-bundle, launch-transition, and ApiClientError-code coverage added
ui-ux-specialist ADDRESSED 3 HIGH truthfulness bugs fixed (stuck "Agent is working…", sticky delivery banner, non-terminal RUNTIME_STOPPED) + resume re-entrancy guard + composer-text retention, each with discriminating tests
task-completion-validator ADDRESSED HIGH VM dead-detection regression fixed (runtime-gated resumable short-circuit + recovery restored to VM live set) with cross-runtime regression tests

All 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)

  • Scope: none

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • cross-component-change
  • business-logic-change
  • security-sensitive-change
  • ui-change
  • infra-change
  • docs-sync-change

External References

Cloudflare Containers docs (container-class, platform-details/architecture, platform-details/rollouts) — cited in the task file; @cloudflare/containers@0.3.7 dist sources read directly to verify Container constructor behavior (ctx.container requirement) 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, reused TASK_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).

raphaeltm and others added 6 commits July 21, 2026 19:07
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>
@codspeed-hq

codspeed-hq Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing sam/diagnose-fix-remaining-production-0dvhf5 (1e8e7d2) with main (0c13372)

Open in CodSpeed

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>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant