Skip to content

fix(park): rung-2 un-pause root fix — vsock RX gate must not arm on a plain resume (+ outbox/park hardening) - #596

Merged
nikhilunni merged 4 commits into
mainfrom
fix/rung2-vsock-gate-root-cause
Jul 7, 2026
Merged

fix(park): rung-2 un-pause root fix — vsock RX gate must not arm on a plain resume (+ outbox/park hardening)#596
nikhilunni merged 4 commits into
mainfrom
fix/rung2-vsock-gate-root-cause

Conversation

@nikhilunni

@nikhilunni nikhilunni commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

The incident

PR #585 (ADR 0074 rung 2, parked-paused eviction) wedged every un-parked session in prod: the ascent's host.resume returned Ok in ~30ms, but the returning prompt never ran. The first fix attempt (#594) was built on a wrong theory and changed nothing observable.

Root cause (verified live + in code + by bookend)

Upstream Firecracker v1.16, inherited by our fork: Vmm::resume_vm() calls kick_virtio_devices() on every resume, and the vsock device's kick() unconditionally arms pending_event_ack — the RX gate that blocks all host→guest vsock delivery until the guest acks a TRANSPORT_RESET. Correct after a snapshot (prepare_save queued a reset to ack); fatal on a plain pause→resume (the rung-2 park/un-pause): nothing was queued, the guest has nothing to ack, the gate never clears. Prompts were "delivered" into an intact-but-gated connection (outbox rows: attempts=N, delivered, never acked), in-guest exec hung, and the FC event loop busy-spun at 100% of a core on the undeliverable backlog.

Evidence chain: stuck prod outbox rows → coord DEBUG logs (7 clean forwards, zero NotFound — #594's arm never even ran) → host logs (hub connection alive the whole window) → live repro (parked session; host_exec hung; /proc sampling showed idle vCPUs + a spinning FC main thread) → git show v1.16.0 confirming the upstream mechanism.

The fix

  1. Fork (fix(vsock): kick() must not arm the RX gate on a plain vm resume firecracker#14, merged; submodule bumped here): kick() signals only when the gate is already armed and never arms it itself; prepare_save (in-process, diff-checkpoint source resume) and restore() (cross-snapshot, re-armed from the saved virtio_state.activated flag) own the arming. Snapshot byte-format unchanged.
  2. Revert fix(park): rung-2 un-pause lets the alive harness self-reattach, not start_agent #594's outbox self-reattach window — wrong premise (the connection never drops, so there is no self-reattach to await; and the rung-2 path never hits NotFound at all) and dead code (outbox_defer never bumped attempts, so the start_agent fallback was unreachable — a genuine desync would have hung forever). NotFound → immediate reattach again.
  3. outbox_defer increments attempts — failure_backoff actually grows now, and investigations see real attempt counts.
  4. Terminal sessions return Gone (410) from ensure_active — the outbox driver drops their rows via its Terminal arm instead of deferring every ~16s forever (3 such rows were live-spinning the prod driver).
  5. Park status unification — the park branch transitions the admin EvictIdle path's Active entry to Evicting (parked-paused ⇒ Evicting + park_rung=2, always); the ascent clears park_rung the moment the un-pause lands (Active→Active conflicts by design); evict_idle_core reports the true outcome instead of hardcoded "idle".
  6. Headroom check reads PG sessions.host_id — the in-memory host_registry lookup made parking a per-replica coin flip (reproduced: identical EvictIdle calls parked on one attempt, captured on the next).

Verification

  • Fork unit suite on the KVM dev VM: 67 passed (2 pre-existing environment failures identical at base).
  • Bookend — new pause_resume_vsock FC integration test (fork binary via ENGRAM_FC_FORK_BIN, wired into ci.yml's FC lane): base fork binary FAILS ("exec dial blocked after plain pause→resume" — the prod wedge reproduced in a clean VM), fixed binary PASSES in 16s.
  • just check green (1484/1484); outbox_live_pg extended + green against live PG; park unit tests updated for the PG-resolved headroom lookup.
  • ADR 0074 divergence log carries the full incident record.

Post-merge: bake-images' build-firecracker publishes the fork artifact for the new gitlink SHA; deploy rolls coord + host fleet; then the prod park→un-pause benchmark (target: sub-second prompt→run_started vs the 18s evict baseline) before the train continues.

🤖 Generated with Claude Code

https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

nikhilunni and others added 4 commits July 6, 2026 19:20
@nikhilunni @claude
fix(outbox): revert the #594 self-reattach window; defers bump attemp…
d0f6459 
…ts; terminal sessions drop their rows

Three outbox-delivery corrections from the rung-2 root-cause pass:

- Revert #594's NotFound await-in-guest-self-reattach window. Its
  premise was disproven twice over: an ADR 0074 rung-2 un-pause keeps
  the harness vsock connection INTACT (delivery succeeds; the NotFound
  arm never runs there), and a genuine harness-unbound desync has
  nothing to wait for (the harness only re-dials on a dropped link or
  agentd's SIGUSR1). It was also unreachable-fallback dead code -- see
  next point -- so a real desync deferred forever instead of
  self-healing via start_agent. NotFound now reattaches immediately
  again (e35ed1f).

- outbox_defer increments attempts. Only mark_delivered bumped it, so a
  row failing BEFORE the forward (ensure_active error, NotFound) sat at
  the floor failure_backoff forever and read attempts=0 in every
  investigation -- and #594's attempts<3 gate was always true.

- ensure_active maps Completed/Failed to Gone (410), not Conflict (409):
  a 409 reads as retry-later to every caller, and the outbox driver
  deferred a completed session's un-acked rows every backoff tick
  forever (observed live: 3 rows spinning the prod driver for hours).
  Gone routes them to the driver's Terminal drop arm and gives
  exec/upload/relay callers an honest 410.

Verified: coordinator unit suite + outbox_live_pg (extended with the
defer-bumps-attempts property) against live PG.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
@nikhilunni @claude
fix(park): parked-paused uniformly means Evicting; headroom resolves …
afd5f28 
…via PG; ascent clears park_rung on un-pause

Three ADR 0074 rung-2 hardening fixes found during the root-cause pass:

- The park branch now transitions an Active entry (the admin EvictIdle
  path) to Evicting after the pause lands, so park_rung=2 uniformly
  implies Evicting -- the admin path used to leave an ACTIVE session
  advertised over a frozen VM. On bookkeeping failure the VM is
  un-paused and the pipeline falls through to the full eviction. The
  ensure_active Active-arm un-park stays as the backstop for the crash
  window between the pause and the transition.

- try_cancel_nominated_eviction clears park_rung the moment the
  un-pause lands, not in the transition's Ok arm: the backstop case is
  already Active, and Active->Active is a same-state Conflict -- tying
  the clear to the Ok arm left park_rung=2 advertised forever over a
  running VM (observed live on prod).

- host_has_memory_headroom resolves the host via PG sessions.host_id
  instead of the in-memory host_registry: the registry is per-replica,
  so the RPC landing on the pod without the cached bind failed closed
  and silently degraded every park into a full eviction -- a coin flip
  in a 2-replica deployment (observed live: identical EvictIdle calls
  parked on one attempt and captured on the next).

- evict_idle_core reports the pipeline's actual outcome (parked /
  skipped / idle) instead of a hardcoded "idle".

Verified: coordinator unit suite (park tests updated to stamp the mock
session's host_id for the PG-resolved lookup).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
@nikhilunni @claude
test(fc): pause->resume vsock-delivery regression test + ADR 0074 inc…
8a55137 
…ident record

The rung-2 un-pause black-hole was a VMM bug: upstream FC v1.16
(inherited by the fork) arms the vsock RX-delivery gate
(pending_event_ack) from kick() on EVERY resume_vm, but a plain
pause->resume queues no TRANSPORT_RESET for the guest to ack, so the
gate never clears -- all host->guest vsock delivery black-holes and the
FC event loop spins at 100% CPU. Fixed in the fork
(cortexapps/firecracker#14): kick() signals only when the gate is
already armed; prepare_save (in-process) and restore() (cross-snapshot,
from the saved activation flag) own the arming.

The new test proves the property end-to-end on the fork binary
(ENGRAM_FC_FORK_BIN, same gating + artifact plumbing as
stock_fork_snapshot_compat): boot a real microVM with the in-guest
agent, exec over vsock, plain pause+resume, exec again within a bounded
budget. Wired into ci.yml's FC-lane --test list. Bookend on the KVM dev
VM: base fork binary FAILS ("exec dial blocked after plain
pause->resume", pinned by the outer timeout -- the prod wedge
reproduced), fixed binary PASSES in 16s.

ADR 0074's divergence log gains the full incident record (the two wrong
theories, the actual mechanism, the engrams-side hardening, and the
file-upstream follow-up).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
@nikhilunni @claude
chore(fc-fork): bump third_party/firecracker — vsock RX gate must not…
fa8a625 
… arm on a plain resume

Picks up cortexapps/firecracker#14: kick() signals the vsock evq (and
replays the TXQ notification) only when pending_event_ack is ALREADY
armed, and never arms it itself; prepare_save and restore() (from the
saved activation flag) own the arming. A plain PATCH /vm
Paused->Resumed cycle no longer closes the RX-delivery gate with
nothing for the guest to ack -- the ADR 0074 rung-2 un-pause black-hole
(and the latent ADR 0045 pause/resume + migration-abort variants).
Snapshot byte-format unchanged (stock<->fork compat preserved).

bake-images' build-firecracker publishes the fork artifact keyed by
this gitlink SHA; the FC lane's pause_resume_vsock regression test
exercises it via ENGRAM_FC_FORK_BIN.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped⏩ skipped✅ passedJul 7, 2026, 2:26 AM

@nikhilunni
nikhilunni merged commit 29aa0e4 into main Jul 7, 2026
18 checks passed
@nikhilunni
nikhilunni deleted the fix/rung2-vsock-gate-root-cause branch July 7, 2026 02:39
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(substrate): ADR 0075/0076 — interim single-writer chunk-cache + …
f845653 
…substrated design (#547)

Rebased onto main past #585/#590-#596; ADRs renumbered 0072/0073 ->
0075/0076 (main's 0072 = substrate-survey-evidence, 0073 =
binding-epoch). See PR #583 for the full description.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(substrate): ADR 0075/0076 — interim single-writer chunk-cache + …
68375d6 
…substrated design (#547)

Rebased onto main past #585/#590-#596; ADRs renumbered 0072/0073 ->
0075/0076 (main's 0072 = substrate-survey-evidence, 0073 =
binding-epoch). See PR #583 for the full description.

Review fix folded in: the client replays `Hello` on every fresh dial
(with_conn), not just at startup — the server's session-chunk pin set
and proto check are per-connection state, and the designed-for
reconnect (a rolled host-agent's successor, holding an empty pin set)
is exactly when silently skipping the replay would leave the handler
unpinned for the rest of the sandbox's life.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(substrate): ADR 0075/0076 — interim single-writer chunk-cache + …
5bee563 
…substrated design (#547)

Rebased onto main past #585/#590-#596; ADRs renumbered 0072/0073 ->
0075/0076 (main's 0072 = substrate-survey-evidence, 0073 =
binding-epoch). See PR #583 for the full description.

Review fix folded in: the client replays `Hello` on every fresh dial
(with_conn), not just at startup — the server's session-chunk pin set
and proto check are per-connection state, and the designed-for
reconnect (a rolled host-agent's successor, holding an empty pin set)
is exactly when silently skipping the replay would leave the handler
unpinned for the rest of the sandbox's life.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(substrate): ADR 0075/0076 — interim single-writer chunk-cache + …
43c87b9 
…substrated design (#547) (#583)

Rebased onto main past #585/#590-#596; ADRs renumbered 0072/0073 ->
0075/0076 (main's 0072 = substrate-survey-evidence, 0073 =
binding-epoch). See PR #583 for the full description.

Review fix folded in: the client replays `Hello` on every fresh dial
(with_conn), not just at startup — the server's session-chunk pin set
and proto check are per-connection state, and the designed-for
reconnect (a rolled host-agent's successor, holding an empty pin set)
is exactly when silently skipping the replay would leave the handler
unpinned for the rest of the sandbox's life.


Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(snapshots): ADR 0077 — transactional snapshots, durable_head, Ru…
630cf00 
…ntimeSpec, Revive (#544)

Rebased onto main past #583/#585/#590-#596. Renumbered at land:
ADR 0074 -> 0077 (main's 0074 = parking ladder); migrations
0087-0089 -> 0088-0090 (main's applied high-water = 0087). The
queue_prompt column references died in the rebase (#592 removed it).

Five pre-merge review findings fixed (see the ADR's divergence log):
- get_session projection: the selected_skills removal left a missing
  comma aliasing live_disk_manifest_version AS park_rung — every
  session read un-parked (frozen-VM-advertised-Active on the rung-2
  ascent) and live_disk_manifest read None. Live-PG round-trip test
  pins the projection now.
- fork-at-attach minted an UNPUBLISHED (uuid, v0) into manifest_ref;
  sessions evicted before their first flush were unevictable and
  zero-dirty captures unresumable. Split: fork_identity is adopted at
  v1 by the FIRST publish; manifest_ref stays the resolvable base
  until then (+ regression test).
- durable_head advanced on recoverable=false rows; now gated, and a
  demote of the current head re-points to the newest recoverable.
- prepare_from_row swallowed RuntimeSpec read errors into "no skills";
  now propagates (scanner/resume retry is the recovery).
- the Idle->Created harness-failed park emitted no StatusChanged and
  swallowed transition failures; now emits + propagates.

Accepted (zero users): no selected_skills backfill into
session_runtime_specs; pre-deploy sessions lose their skill selection
on the next re-prepare.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(snapshots): ADR 0077 — transactional snapshots, durable_head, Ru…
3ee7c6e 
…ntimeSpec, Revive (#544)

Rebased onto main past #583/#585/#590-#596. Renumbered at land:
ADR 0074 -> 0077 (main's 0074 = parking ladder); migrations
0087-0089 -> 0088-0090 (main's applied high-water = 0087). The
queue_prompt column references died in the rebase (#592 removed it).

Five pre-merge review findings fixed (see the ADR's divergence log):
- get_session projection: the selected_skills removal left a missing
  comma aliasing live_disk_manifest_version AS park_rung — every
  session read un-parked (frozen-VM-advertised-Active on the rung-2
  ascent) and live_disk_manifest read None. Live-PG round-trip test
  pins the projection now.
- fork-at-attach minted an UNPUBLISHED (uuid, v0) into manifest_ref;
  sessions evicted before their first flush were unevictable and
  zero-dirty captures unresumable. Split: fork_identity is adopted at
  v1 by the FIRST publish; manifest_ref stays the resolvable base
  until then (+ regression test).
- durable_head advanced on recoverable=false rows; now gated, and a
  demote of the current head re-points to the newest recoverable.
- prepare_from_row swallowed RuntimeSpec read errors into "no skills";
  now propagates (scanner/resume retry is the recovery).
- the Idle->Created harness-failed park emitted no StatusChanged and
  swallowed transition failures; now emits + propagates.

Accepted (zero users): no selected_skills backfill into
session_runtime_specs; pre-deploy sessions lose their skill selection
on the next re-prepare.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx
nikhilunni added a commit that referenced this pull request Jul 7, 2026
@nikhilunni @claude
feat(snapshots): ADR 0077 — transactional snapshots, durable_head, Ru…
6a37340 
…ntimeSpec, Revive (#544) (#584)

Rebased onto main past #583/#585/#590-#596. Renumbered at land:
ADR 0074 -> 0077 (main's 0074 = parking ladder); migrations
0087-0089 -> 0088-0090 (main's applied high-water = 0087). The
queue_prompt column references died in the rebase (#592 removed it).

Five pre-merge review findings fixed (see the ADR's divergence log):
- get_session projection: the selected_skills removal left a missing
  comma aliasing live_disk_manifest_version AS park_rung — every
  session read un-parked (frozen-VM-advertised-Active on the rung-2
  ascent) and live_disk_manifest read None. Live-PG round-trip test
  pins the projection now.
- fork-at-attach minted an UNPUBLISHED (uuid, v0) into manifest_ref;
  sessions evicted before their first flush were unevictable and
  zero-dirty captures unresumable. Split: fork_identity is adopted at
  v1 by the FIRST publish; manifest_ref stays the resolvable base
  until then (+ regression test).
- durable_head advanced on recoverable=false rows; now gated, and a
  demote of the current head re-points to the newest recoverable.
- prepare_from_row swallowed RuntimeSpec read errors into "no skills";
  now propagates (scanner/resume retry is the recovery).
- the Idle->Created harness-failed park emitted no StatusChanged and
  swallowed transition failures; now emits + propagates.

Accepted (zero users): no selected_skills backfill into
session_runtime_specs; pre-deploy sessions lose their skill selection
on the next re-prepare.


Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@nikhilunni nikhilunni mentioned this pull request Jul 7, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nikhilunni