Repository navigation
Host RAM ledger: charge base-shm, parked residents, and running VMs under one allocatable_mib budget - #561
Conversation
…AM ledger Issue #540 P1: extend `GuestMemoryStats` with `parked_pss_bytes` / `parked_sampled` and bucket the Firecracker backend's smaps_rollup sum by a new per-sandbox `parked: bool` flag on `LiveSandbox` (always false today; no backend transition sets it yet — epic-parking-ladder owns that). This is the seam the host RAM ledger (next commit) uses to add back only reservation-backed VM PSS into `allocatable_mib`, never a parked resident's. No `SandboxBackend` trait method is added for park/unpark (a default no-op verb would be a silent-Ok trap); only a `pub(crate)`, test-visible setter on the FC backend. Adds a Linux-only unit test proving the bucketing itself (same real pid sampled into both buckets, split not summed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
… one snapshot, one allocatable_mib Issue #540: unify the scheduler's and pressure-evictor's three unreconciled views of host RAM into one per-tick ledger. - New `engram-host-agent::ram_ledger`: `RamLedgerSnapshot` charges every MiB to a named bucket (running-VM PSS, parked-paused PSS, base-shm tmpfs residency + pending prewarm charges) and derives `allocatable_mib = MemAvailable + Σ non-parked VM PSS − pending base-shm charges` — parked residents are NEVER added back, closing the double-count the parking ladder would otherwise introduce. `RamLedger` is the long-lived pending- charge registry `image_prefetch` writes to before a base-shm prewarm and settles after, so the charge lands within one heartbeat tick of prewarm start instead of minutes later when the multi-GiB write finishes. Adds a tmpfs-headroom pre-check (the 2026-06-28 ENOSPC incident class) that skips the write attempt with a counter, without touching the existing warn-and-continue-then-lazy-backstop failure posture. - `UtilizationProbe::sample` no longer computes `allocatable_mib` itself — it now derives `HostUtilization`'s memory fields from the heartbeat tick's `RamLedgerSnapshot`, deleting the stale "mlock'd base-memfile residency" formula comment along with it. - The idle-evictor's pressure gate reads the SAME snapshot (via a `tokio::sync::watch` channel) instead of taking its own private `/proc/meminfo` sample (`mem_pressure_check`, deleted; the pure `mem_pressure_from` core is unchanged and re-pointed). `HOST_MEM_FREE_PCT` moves to the heartbeat's single emission site so it's no longer gated on pressure-aware mode being on. - Wire + PG (migration 0077, additive/serde-defaulted): `HostUtilization` gains `base_shm_mib` / `base_shm_pending_mib` / `parked_pss_mib` / `running_pss_mib`; the first three (pending is transient, already folded into `allocatable_mib`) persist to new `hosts` columns and surface on the fleet view (`api/hosts.rs` HostView, `fleet.proto`, the web Fleet page). - New `engram_host_ram_ledger_mib{category=...}` / `engram_host_ram_allocatable_mib` / `engram_host_base_shm_tmpfs_{total,used}_mib` / `engram_base_shm_prewarm_skipped_total` gauges, all emitted from the single per-tick snapshot. - Tests: ram_ledger unit tests (parked exclusion, pending lifecycle, sparse st_blocks accounting, non-Linux zeros); a placement fixture proving a parked-excluded host never gets double-counted as free; a heartbeat JSON round-trip + old-host-defaults-to-zero test for the new wire fields; a live-Postgres round-trip proving migration 0077 + the row mapping agree. Pre-existing, unrelated: 3 `engram-host-agent` chunk-cache write-through test failures (disk_daemon::backend, pooled_backend) reproduce on unmodified main and are tracked as issue #552. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
Dated addendum (not a rewrite): explains why the implementation-note-1 formula (allocatable = MemAvailable + Σ guest-resident PSS) silently assumed resident ⇔ reserved, the two gaps that broke (unattributed base-shm, the parking-ladder double-count), the amended formula, and the no-sharing- discount stance backed by the 2026-07-01 rss≈pss prod datum. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
|
The latest Buf updates on your PR. Results from workflow CI / buf (pull_request).
|
nikhilunni
left a comment
There was a problem hiding this comment.
Deep review of the RAM-ledger implementation (issue #540). Overall: a faithful, high-quality implementation — the ledger module matches the specced snapshot shape and derivation exactly, the parked/running PSS split lands behind a per-sandbox flag with no trait method (guardrail honored), prewarm pending-charging + the tmpfs headroom pre-check work as specced, and the wire/PG/fleet-view surfacing plus the ADR 0046 addendum are all in place. Verdict: solid, mergeable after the migration-number collision is resolved; one transient accounting bug and two acceptance-checklist boxes that the code doesn't actually satisfy are worth a pass before undrafting.
CI
Green — all lanes pass (fmt+clippy, linux, macOS, firecracker, e2e suite + teleport, orchestrator, web, buf, CI Gate).
Issue compliance
Implementation plan steps 1–9 are all present and faithful: ram_ledger.rs matches the specced struct/derivation (parked never added back; pending subtracted); GuestMemoryStats extended with no trait method (the only setter is a cfg(test) pub(crate) fn on the FC backend); UtilizationProbe::sample retired its guest_pss_mib param with no compat shim; the heartbeat builds one snapshot, publishes on a watch channel, and is the single gauge-emission site (HOST_MEM_FREE_PCT moved there); mem_pressure_check deleted with the pure core kept; prewarm registers-before/settles-both-arms with the headroom pre-check; wire (4 serde-defaulted fields + round-trip + old-host default tests), PG (3 columns per the issue's own spec — the pending-field deviation is explicitly documented in the migration, row.rs, and the PR body), fleet view (totality destructure, fleet.proto +3, regenerated TS in both orchestrator/ and web/), and the dated ADR 0046 addendum. New tests are genuinely wired into CI: the live-PG round-trip rides placement_reservation_live_pg, already in ci.yml's --test list; the FC bucketing test is a plain Linux unit test.
Three gaps against the issue's own checklist:
- The migration-number check verified the on-disk high-water mark (0076) but not the issue's explicit "and any in-flight branches" clause — five sibling open PRs also claim 0077 (finding 1).
- Two acceptance boxes are checked that the code does not satisfy: "VZ/Process/non-Linux — gauges not emitted" (they are emitted, as zeros — finding 3) and the one-source-of-truth criterion's "assertable via the watch channel in a unit test" (no such test exists; the box is checked citing construction + grep — finding 4).
No scope creep; the deviations that exist are declared. (I also checked the finder's claim that the two-host placement fixture is "tautological" — dropped it: the issue's acceptance section explicitly requested exactly that fixture, and the test's comment honestly states placement only ever sees the final allocatable_mib.)
Findings
-
deploy/migrations/0077_host_ram_ledger.sql:19 — [CONFIRMED] migration number 0077 collides with five sibling open PRs. #560 (
0077_drop_guest_ready_state.sql), #563 (0077_enable_job_capture_progress.sql), #564 (0077_host_capabilities.sql), #565 (0077_enable_job_prestage.sql), and #566 (0077_session_selected_skills.sql) each also add a 0077 migration, and this PR's own landing order places #530/#537 (→ PR #560 et al.) before #540. Failure: whichever lands second gives sqlx a duplicate migration version → coordinator crash-loops at boot (or the live-PG CI lane fails at apply). Fix: renumber at merge time to the then-current high-water mark + 1, and coordinate the number across the in-flight stack (the issue's unverified-claims list called this out explicitly). -
crates/engram-host-agent/src/ram_ledger.rs:86 — [CONFIRMED] the written fraction of an in-flight prewarm is double-subtracted from
allocatable_mibfor the whole write window.register_pending_base_shmcharges the FULL non-hole byte total before the write and nothing decrements it untilsettle_pendingafter the write completes — butMemAvailableis already dropping as the pwrites land tmpfs pages. Concretely: a ~19 GiB dev-brain prewarm at 90% written has MemAvailable down ~17 GiB and the full 19 GiB pending charge still subtracted, soallocatable_mibunder-reports by ~17 GiB on a 64 GiB host for minutes — placement rejects sessions that genuinely fit. The direction is conservative (never over-places), but the field's own doc says "charges not yet on the tmpfs". Fix: insample(), net each pending charge against that file's measuredst_blocks(e.g.pending.saturating_sub(file_blocks_bytes)), or settle incrementally. -
crates/engram-host-agent/src/lib.rs:1176 — [CONFIRMED] ledger gauges are emitted unconditionally, contradicting the checked acceptance box. The issue's criterion (and the PR body's
[x]) says "VZ/Process/non-Linux: … gauges not emitted", but everyengram_host_ram_ledger_mib{...}/ allocatable / tmpfs gauge is set every tick regardless ofram_snapshot.measured— an unmeasured host carries a full family of permanently-zero series (onlyHOST_MEM_FREE_PCTis gated). Either gate the emission block onram_snapshot.measuredor un-check/correct the acceptance claim. -
crates/engram-host-agent/src/lib.rs:1175 — [CONFIRMED] the one-source-of-truth watch-channel unit test the issue asked for doesn't exist. The criterion says the heartbeat/pressure-gate coupling is "assertable via the watch channel in a unit test"; the PR body checks the box citing construction + grep instead. The coupling lives in one large inline tokio task — a refactor that re-orders
send_replaceafterutil_probe.sample, or samples twice, would silently reintroduce the two-views divergence with nothing failing. A small test (publish a snapshot on a watch channel, assertborrow()equals whatsample()was fed) would pin it; otherwise declare the deviation rather than checking the box. -
crates/engram-host-agent/src/lib.rs:1190 — [CONFIRMED]
HOST_BASE_SHM_TMPFS_USED_MIBduplicates thebase_shmcategory gauge instead of reporting statfs used. It's set fromram_snapshot.base_shm_mib(the read_dir st_blocks walk) — byte-identical toengram_host_ram_ledger_mib{category="base_shm"}. Tmpfs consumption invisible to the flat-file walk (an unlinked-but-open file from abase_shm_gcrace, a stray subdir/temp file) counts against the 32 GiB cap and the headroom pre-check (both statfs-based) but not this gauge — i.e. it under-reports exactly during an ENOSPC-class incident.tmpfs_stat_mibalready has statfs in hand; reportf_blocks − f_bfreethere instead. -
crates/engram-core/src/traits/sandbox.rs:94 — [CONFIRMED] no
parked_rss_bytes, so a parked sandbox's RSS is measured then discarded.read_smaps_rollup_pss_rssreturns both, but the parked branch keeps only PSS — once the ladder parks a 12 GiB resident, the Σpss/Σrss sharing signal structurally excludes it, and the ADR addendum's own rule ("density math that wants a sharing credit must first observe Σpss/Σrss < 1.0") can never be evaluated for parked residents — the exact population density math cares about. This PR is the cheap moment to add the one defaulted field (a later addition re-touchesGuestMemoryStats, the FC split, and the read loop). The issue's spec also omitted it, so PLAUSIBLE-as-scope, but the discard itself is in the code.
Automated deep review (core-ops batch); findings verified against the branch — treat PLAUSIBLE items as questions.
| -- Additive + serde-defaulted: pre-0077 hosts (and non-Linux dev backends) | ||
| -- report 0 for all three, same posture as every other util_* column. | ||
|
|
||
| ALTER TABLE hosts ADD COLUMN IF NOT EXISTS util_base_shm_mib BIGINT NOT NULL DEFAULT 0; |
There was a problem hiding this comment.
[CONFIRMED] 0077 collides with five sibling open PRs — #560, #563, #564, #565, #566 each also add a 0077_*.sql, and the PR body's landing order puts several of them before this one. sqlx errors on a duplicate version → coordinator crash-loops at boot. Renumber at merge time to the then-current high-water mark + 1 (the issue's unverified-claims list explicitly said to check in-flight branches).
There was a problem hiding this comment.
Fixed in 6db9843: renamed the migration file 0077 -> 0078 and every code/doc reference to it (coordinator hosts.rs, engram-postgres row.rs, fleet.proto + regenerated TS in both web/ and orchestrator/, ADR 0046 addendum, the live-PG test comment). Per the batch owner's collision resolution, 0077 stays with #560.
| /// been re-scanned) yet. Subtracted from `allocatable_mib` so | ||
| /// placement sees the charge within one heartbeat tick of prewarm | ||
| /// start rather than minutes later when the write completes. | ||
| pub base_shm_pending_mib: u64, |
There was a problem hiding this comment.
[CONFIRMED] transient double-charge during the prewarm window — the full registered charge stays here for the entire multi-minute write while MemAvailable is already dropping as pwrites land tmpfs pages, so the written fraction is subtracted twice from allocatable_mib (a ~19 GiB dev-brain prewarm at 90% written under-reports by ~17 GiB). Direction is conservative, but the doc says "not yet on the tmpfs". Fix: in sample(), net each pending charge against that file's current st_blocks bytes.
There was a problem hiding this comment.
Fixed in 8aade45: RamLedger::register_pending_base_shm now takes the target file path alongside the expected byte count, and sample() nets each pending charge against that specific file's own st_blocks (via a new file_allocated_bytes helper) instead of holding the full charge outstanding for the whole write window. New regression test pending_charge_nets_against_bytes_already_written writes partial bytes into the target file mid-charge and asserts the pending figure shrinks accordingly.
| &guest_mem.unwrap_or_default(), | ||
| ); | ||
| ram_ledger_tx_for_heartbeat.send_replace(ram_snapshot); | ||
| ::metrics::gauge!(crate::metrics::HOST_RAM_LEDGER_MIB, "category" => "running_vms") |
There was a problem hiding this comment.
[CONFIRMED] gauges emitted unconditionally, contradicting the checked acceptance box — the issue (and the PR body's [x]) says non-Linux/unmeasured hosts get "gauges not emitted", but this whole block runs every tick regardless of ram_snapshot.measured (only HOST_MEM_FREE_PCT is gated). Either gate the block on measured or correct the acceptance claim.
There was a problem hiding this comment.
Fixed in 8aade45: the whole HOST_RAM_LEDGER_MIB/HOST_RAM_ALLOCATABLE_MIB/HOST_BASE_SHM_TMPFS_* gauge block is now gated on ram_snapshot.measured, so VZ/Process/non-Linux hosts stop emitting a permanent-zero series. HOST_MEM_FREE_PCT was already correctly gated via free_pct()'s own None return. PR body's acceptance box corrected to describe the actual (now-true) behavior.
| base_shm_dir_for_heartbeat.as_deref(), | ||
| &guest_mem.unwrap_or_default(), | ||
| ); | ||
| ram_ledger_tx_for_heartbeat.send_replace(ram_snapshot); |
There was a problem hiding this comment.
[CONFIRMED] the issue's one-source-of-truth criterion says this coupling is "assertable via the watch channel in a unit test" — no such test exists (the PR body checks the box citing construction + grep). A refactor that re-orders send_replace after util_probe.sample, or samples twice, would silently reintroduce the two-views divergence with nothing failing. A small publish-then-borrow() assertion test would pin it; otherwise declare the deviation.
There was a problem hiding this comment.
Fixed in 8aade45: added ram_ledger_snapshot_round_trips_through_watch_channel — publishes a RamLedgerSnapshot on a tokio::sync::watch channel and asserts two independent readers (borrow_and_update) observe byte-identical values to what was published, pinning the one-source-of-truth mechanism the heartbeat and pressure gate rely on. PR body's acceptance box corrected to cite the test instead of construction + grep.
| .set(ram_snapshot.allocatable_mib() as f64); | ||
| ::metrics::gauge!(crate::metrics::HOST_BASE_SHM_TMPFS_TOTAL_MIB) | ||
| .set(ram_snapshot.base_shm_tmpfs_total_mib as f64); | ||
| ::metrics::gauge!(crate::metrics::HOST_BASE_SHM_TMPFS_USED_MIB) |
There was a problem hiding this comment.
[CONFIRMED] byte-identical duplicate of the base_shm category gauge — "tmpfs used" should be statfs used (f_blocks − f_bfree, which tmpfs_stat_mib already has in hand), not the read_dir st_blocks walk. An unlinked-but-open file or stray subdir occupies tmpfs (counts against the cap and the headroom pre-check, both statfs-based) but is invisible here — so this gauge under-reports exactly during the ENOSPC-class incident it exists to debug.
There was a problem hiding this comment.
Fixed in 8aade45: tmpfs_stat_mib now also does a real statfs read (f_blocks - f_bfree) and RamLedgerSnapshot carries a new base_shm_tmpfs_used_mib field for it; HOST_BASE_SHM_TMPFS_USED_MIB now reads that field instead of base_shm_mib (the st_blocks dir-walk). base_shm_mib/the base_shm category gauge is unchanged (still the per-file attribution walk) — the two are now genuinely distinct signals.
| /// the flag yet). Never added back into `allocatable_mib` — see | ||
| /// [`GuestMemoryStats::pss_bytes`]. | ||
| pub parked_pss_bytes: u64, | ||
| /// How many parked sandboxes were successfully sampled. |
There was a problem hiding this comment.
[CONFIRMED] no parked_rss_bytes — a parked sandbox's RSS is measured by read_smaps_rollup_pss_rss and then discarded. Once the ladder parks a resident VM, the Σpss/Σrss sharing signal structurally excludes it, so the ADR addendum's "must first observe Σpss/Σrss < 1.0" rule can never be evaluated for parked residents. This PR is the cheap moment to add the one defaulted field (a later addition re-touches this struct, the FC split, and the read loop). Spec also omitted it, so treat as a question.
There was a problem hiding this comment.
Fixed in c788990: added parked_rss_bytes to GuestMemoryStats, populated it in the FC backend's guest_memory_stats() alongside parked_pss_bytes (same smaps_rollup read), and exposed both via new gauge-only metrics (engram_sandbox_guest_parked_pss_bytes/_rss_bytes) so the Σpss/Σrss ratio is actually observable for parked residents. Took the fix now rather than deferring — agreed it's cheap and avoids re-touching this struct + the FC split + the read loop later; no follow-up issue needed.
…lision) Six PRs in the 2026-07 core-ops overhaul batch all added a migration numbered 0077 from the same main base. Land-queue assignment resolves the collision: #560 keeps 0077, #561->0078, #563->0079, #564 (this PR)->0080, #565->0081, #566->0082. Renames deploy/migrations/0077_host_capabilities.sql to 0080_host_capabilities.sql and updates the "(migration 0077)" comment references in crates/engram-core/src/types/{host,snapshot}.rs, crates/engram-postgres/src/row.rs, and docs/adr/0068-capability-vector- readiness.md to match. No SQL content changes — the migration is not yet applied anywhere, so this is a pure rename, not an edit to an already-applied (checksum-immutable) migration. Also appends a "Post-review fixes (PR #564)" section to the ADR documenting the six review findings fixed on this branch (NotApplicable substrate gating, the two unstamped record_snapshot sites, the misleading sessions.rs comment, the OnceLock transient-failure cache, the exclusion_summary doc-order fix, and the web fleet-view plumbing), plus the CI musl-lane CFLAGS fix, per this repo's ADR-bookend convention (update between phases with divergences found).
6 PRs in the core-ops-overhaul batch each added a migration numbered 0077. Land-queue assignment gives #563 (this PR) 0079: #560 keeps 0077, #561->0078, #564->0080, #565->0081, #566->0082. No content change; the migration hasn't landed on main yet so renumbering is safe (applied migrations are checksum-immutable, but this one isn't applied anywhere).
Six sibling open PRs in this batch (#560, #561, #563-#566) all added a migration numbered 0077. Land-queue assignment gives #560 0077 and this PR 0078, per the batch owner's collision resolution. Renumbers the migration file and every code/doc reference to it (coordinator, PG row mapping, fleet.proto + regenerated TS, ADR 0046 addendum, the live-PG round-trip test's comment). Review: #561 (comment)
…asured, real statfs tmpfs-used, pin watch-channel invariant (findings 2, 3, 4, 5) - finding 2: RamLedger::register_pending_base_shm now stores the target file path alongside the expected byte count. sample() nets each pending charge against that file's own st_blocks (file_allocated_bytes) instead of holding the full charge outstanding for the whole write window — the fix for the transient double-charge where a ~19 GiB dev-brain prewarm at 90% written was under-reporting allocatable_mib by ~17 GiB (MemAvailable already dropping AND the full charge still subtracted). New regression test pending_charge_nets_against_bytes_already_written proves the charge shrinks as bytes actually land. - finding 3: the whole HOST_RAM_LEDGER_MIB / HOST_RAM_ALLOCATABLE_MIB / HOST_BASE_SHM_TMPFS_* gauge-emission block is now gated on ram_snapshot.measured, so VZ/Process/non-Linux hosts (and a genuine /proc/meminfo parse failure) stop reporting a permanently-zero gauge family that reads as "this host has 0 MiB of everything" instead of "unmeasured" — matches the PR's own acceptance criterion. - finding 5: HOST_BASE_SHM_TMPFS_USED_MIB was byte-identical to the base_shm category gauge (a read_dir/st_blocks walk over known files). tmpfs_stat_mib now also does a real statfs read (f_blocks - f_bfree) and RamLedgerSnapshot carries a new base_shm_tmpfs_used_mib field for it, so an unlinked-but-open file or stray subdir the dir walk can't see is no longer invisible to the gauge that exists specifically to debug ENOSPC-class tmpfs incidents. - finding 4: added ram_ledger_snapshot_round_trips_through_watch_channel, publishing a RamLedgerSnapshot on a tokio::sync::watch channel and asserting two independent readers observe byte-identical values to what was published — pins the "one source of truth" mechanism the heartbeat and pressure gate rely on. Review threads: #561 (comment) (finding 2) #561 (comment) (finding 3) #561 (comment) (finding 4) #561 (comment) (finding 5)
…st PSS (finding 6) GuestMemoryStats::parked_pss_bytes was measured via the same smaps_rollup read that returns both PSS and RSS, but the RSS half was discarded for parked sandboxes. Once the parking ladder lands, this would structurally block ever evaluating the sharing-credit rule (Sigma_pss/Sigma_rss < 1.0, ADR 0046) for parked residents -- exactly the population the density math cares about. Cheap to add now (one defaulted field mirroring the existing pss_bytes/rss_bytes pair) versus re-touching this struct, the FC split, and the read loop later. Adds parked_rss_bytes to GuestMemoryStats, populates it in the FC backend's guest_memory_stats() alongside parked_pss_bytes, and exposes both via new gauge-only metrics (engram_sandbox_guest_parked_pss_bytes / _rss_bytes) so the ratio is actually observable -- never folded into allocatable_mib, same posture as the existing non-parked pair. Extended guest_memory_stats_buckets_parked_pss_separately to assert parked RSS is measured too. The issue's own spec omitted this field (review flagged it PLAUSIBLE-as-scope), but the discard was real and the fix is small and isolated -- taken now rather than deferred. Review: #561 (comment)
Post-review fixup passAddressed every finding from the deep review (all 6 marked [CONFIRMED]) in scoped commits, and replied to each inline thread with the fix commit. Also did an honesty pass on the PR body's acceptance-criteria checklist.
Acceptance-criteria honesty passEdited the PR body: corrected the "VZ/Process/non-Linux — gauges not emitted" box (now literally true post-finding-3) and the "one source of truth ... assertable via the watch channel in a unit test" box (now backed by the new test from finding 4), and added a "Post-review fixups" section summarizing all six.
|
# Conflicts: # crates/engram-sandbox-firecracker/src/lib.rs
…rough test flush_write_throughs_chunks_into_local_cache (added on main in 6766a5b, inherited via this merge) asserted the flushed chunk is resident in the local cache right after flush(), but the cache's default 20%-free-space floor is governed by a real statvfs(2) of the host disk — on any dev machine over ~80% used, the very first write_local's debounced sweep evicts the chunk it just wrote, failing the assertion regardless of the write-through logic (which is correct; confirmed by tracing the actual cache.put() call). two_host_drain_wave.rs / migration_source.rs / two_host_live_teleport.rs already work around this by pinning ENGRAM_CHUNK_CACHE_FREE_FLOOR_PCT for the test process (nextest gives each test its own process); apply the same fix here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
…idency tests Same root cause as the previous commit: migration_fetch_rejects_unlisted_hash_and_bad_export_id and materialize_chunked_rootfs_uses_chunk_cache_when_present both populate a ChunkCache directly and then assert the cache still serves those chunks after the backing blob store is gone — but on a dev disk over the cache's default 80%-full floor, the debounced sweep on the very first write evicts the chunk before either test can observe it. Pin ENGRAM_CHUNK_CACHE_FREE_FLOOR_PCT low for these tests, matching the existing two_host_drain_wave.rs / migration_source.rs / two_host_live_teleport.rs workaround. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
|
Merge conflict resolved against |
# Conflicts: # crates/engram-host-agent/src/util.rs
|
Merge conflict against |
…s, stall detection, persisted output tail (OSS half) (#563) * feat(host-agent): warm-hook progress protocol + stall/stage-deadline watchdog (#539) Pure, clock-injected building blocks for hardening the capture-time [warm] hook: parse_progress_line() for the `::engram-warm:: event=...` sentinel grammar, an OutputTail 16 KiB ring buffer, and WarmWatchdog — a state machine that fires on stall (only once a hook has emitted its first progress line — "conforming"), a stage's declared deadline_secs, or the existing global WarmConfig timeout (global timeout always has supremacy). No I/O yet; run_warm_hook wiring lands in the next commit. Also: SandboxError gains a CaptureFailed(CaptureFailure) variant (kind/stage/tail/message) so a warm-hook failure can carry structured diagnosis instead of collapsing to a string, plus the shared CaptureProgress/WarmStageRecord/CaptureFailureKind types in engram-core that will cross the coord<->host boundary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(host-agent,coordinator,protocol): stream capture progress into enable_jobs (#539) Wires the warm-hook progress protocol + watchdog (previous commit) into the actual capture path, end to end: - BuildBaseSnapshot becomes a server-streaming RPC (wire v8, clean break — no dual-decode ladder): the host emits CaptureProgress at least every 30s (host keepalive) for phase=boot/warm/snapshot, plus a structured CaptureFailed terminal frame instead of a flat error string. SandboxBackend::build_base_snapshot and HostClient::build_base_snapshot both gain a `progress: mpsc::Sender<CaptureProgress>` parameter. - PooledBackend::run_warm_hook now drives exec_stream through a tokio::select! loop against WarmWatchdog::next_deadline(), demuxing stdout lines for the ::engram-warm:: protocol, feeding stall/stage/ global-timeout checks, and forwarding live progress. Every failure path returns SandboxError::CaptureFailed{kind, stage, tail, message} — the "(see host logs for stderr)" dead end is gone. New engram_warm_hook_stage_seconds / engram_warm_hook_failures_total metrics record from the same call sites. - Migration 0077 adds capture_phase/warm_stage/warm_stage_started_at/ warm_stages/output_tail to enable_jobs; new fenced, lease-renewing MetadataStore::update_enable_job_capture_progress. - enable_scanner.rs deletes the blind capture-lease-renewal ticker — each progress write now renews the claim — and classify_capture_error maps CaptureFailureKind::WarmExecTransport (the only kind a stream transport death produces) to the retryable Pipeline path; every other kind stays NonRetryable (bail-fast), same as before. - App-gRPC EnableJob gains capture_phase/warm_stage/ warm_stage_started_at/output_tail (fields 11-14); TS bindings regenerated via `just gen-proto`. Tests: 23 pure warm_progress unit tests (parser/OutputTail/watchdog, no sleeps) + 2 new PooledBackend-level tests (mock exec_stream: a stalled hook fails within a test-shrunk ENGRAM_WARM_STALL_SECS with stage+tail carried; a two-stage hook that exits 0 succeeds with ordered CaptureProgress + closed stage history) + a new live-PG test (capture-progress fencing, lease renewal, and survival through record_enable_job_failure) — all pass locally against a live Postgres. `enable_jobs_live_pg` is already in ci.yml's Postgres-gated ignored-test list, so the new test runs there with no workflow change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(web): surface live warm-hook capture progress on the enable-job panel (#539) Extends the ADR 0036 enable-job progress UI: while an enable is `capturing`, the row now shows the live capture_phase/warm_stage badge instead of going dark for the whole [warm]-hook duration (previously 10-33 min of "capturing canonical snapshot" with zero further signal). A failed job additionally renders its failing warm_stage and the hook's output_tail (last 16 KiB of combined stdout+stderr) inline — no host-log access needed to see why a dev-brain-style enable failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * docs(warm-hooks): author the [warm]-hook contract; correct the stale VZ no-op claim (#539) New docs/warm-hooks.md: what a [warm] hook may assume (agentd-ready guest, capture_env-merged exec env, egress per [warm.network], no per-session secrets, no TTY, fresh VM per attempt so side effects must be idempotent under replay), the three deadline budgets (stage/stall/ global) and their precedence, the ::engram-warm:: progress-line grammar, and a worked example. Also corrects two stale claims in WarmConfig's rustdoc (image.rs): "the hook is a no-op on VZ" doesn't match any code gate (VZ opts out of build_base_snapshot entirely via the trait default, same as Process — there's no VZ-specific gate on the hook itself), and the "no network" hermetic claim is now only the default — [warm.network] (already shipped) lets an image opt into capture-time egress. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * style: cargo fmt the new capture-progress live-PG test (#539) Follow-up to the previous commits' capture_progress_is_fenced_renews_lease_and_survives_failure test — the multi-import use statement needed rustfmt's wrap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * test(protocol): bump the wire_version_pinned golden to 8 (#539) wire_golden.rs deliberately tripwires WIRE_VERSION bumps so a payload shape change without a version bump (or vice versa) is a conscious decision. The streaming BuildBaseSnapshot change bumped v7->v8; no existing bincode-golden type's shape changed (the new CaptureProgress/ WarmStageRecord/CaptureFailureKind types aren't in this golden corpus), so only the pinned constant needed updating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * chore(migrations): renumber 0077 -> 0079 (batch land-queue collision) 6 PRs in the core-ops-overhaul batch each added a migration numbered 0077. Land-queue assignment gives #563 (this PR) 0079: #560 keeps 0077, #561->0078, #564->0080, #565->0081, #566->0082. No content change; the migration hasn't landed on main yet so renumbering is safe (applied migrations are checksum-immutable, but this one isn't applied anywhere). * fix(host-agent): update warm_hook_capture.rs callsites for the 4-arg build_base_snapshot (review: CI-red root cause) This PR added a 4th parameter (mpsc::Sender<CaptureProgress>) to SandboxBackend::build_base_snapshot but missed the pre-existing FC integration test crates/engram-host-agent/tests/warm_hook_capture.rs, which still called it with 3 args at 3 callsites -> E0061, failing fmt+clippy+check, tests (linux), and tests (firecracker) in CI. Wire a throwaway mpsc channel (matching the pattern already used by the PooledBackend unit tests in pooled_backend.rs) at each callsite. * fix(protocol): unwrap_or -> unwrap_or_else in parse_capture_failure_kind (finding 2) unwrap_or's argument is evaluated eagerly, so the tracing::warn! block ran on EVERY call to parse_capture_failure_kind -- including a correctly-parsed, perfectly ordinary kind like warm_exit_non_zero -- logging a bogus "unrecognized CaptureFailureKind on the wire" warning on the coordinator for every capture failure, not just genuinely unrecognized ones. unwrap_or_else defers the closure to the None case only. * fix(coordinator): classify connect-time Unavailable/WireSkew as retryable in capture (finding 4) If the capture host rolls between pick_capture_host and the build_base_snapshot RPC, the connect-time failure surfaced as SandboxError::Unavailable/WireSkew, which fell into the generic ApiError::Internal arm -> classify_capture_error bails NonRetryable on attempt 1. The identical failure one second later, mid-stream, already retries via CaptureFailureKind::WarmExecTransport. Route Unavailable/WireSkew to ApiError::Unavailable, which classify_capture_error already retries via the attempts budget -- matching this PR's whole point of making transport deaths retryable. * fix(postgres): use .map_err(col_err) for warm_stages/capture_phase columns (finding 7) warm_stages_from_row and capture_phase_from_row silently swallowed a try_get Err into a default (Vec::new()/None), unlike their three sibling new columns in enable_job_from_row (warm_stage, warm_stage_started_at, output_tail) which already use the file's dominant .map_err(col_err) convention. All current queries select all five columns, so this was dead code today -- but a future SELECT/RETURNING that forgets one of these two columns (the exact copy-paste hazard of this hand-maintained list) would decode silently with empty progress instead of a loud ColumnNotFound. Keep only the genuine SQL-NULL handling (Some/None on the Option<...> try_get); a try_get Err now propagates via col_err like everywhere else. * test(host-agent): drop dead Probe.weak scaffolding in the two-stage warm-hook test (finding 8) warm_hook_two_stages_then_exit_zero_succeeds_with_ordered_progress's Probe.weak was set but never read -- TwoStageMock never upgrades it, unlike the sibling test's Probe (~L7192), whose snapshot() does dereference it. Dead scaffolding copied from that pattern; delete the field, its OnceLock::new()/set() calls, and the now-unused OnceLock/Weak import. * style: rustfmt the warm_hook_capture.rs build_base_snapshot callsites cargo fmt wraps the 4-arg calls across multiple lines (over the line-length limit as single-line). Follow-up to the CI-red fix commit, split out per repo convention (new commit, not an amend). * fix(host-agent): cap pending_stdout to prevent unbounded growth (finding 5) pending_stdout only drained on \n. A hook that streams newline-free output (gradle rich-console \r redraws, binary noise) grew this Vec unbounded for the hook's entire 10-33 min runtime -- the OutputTail tail buffer is capped at 16 KiB, this wasn't -- and the per-event iter().position(b'\n') re-walk of the growing buffer on every chunk was quadratic. Cap it at OutputTail::DEFAULT_CAP_BYTES: past that with no newline in sight, it can't be a valid (short) ::engram-warm:: line, so drop it. Added warm_hook_newline_free_noise_does_not_wedge_progress_parsing to prove the cap doesn't hang/OOM on 64 KiB of noise and doesn't wedge parsing of a real progress line arriving right after. * fix(host-agent,core): stop misclassifying an early signal-kill Exit(None) as WarmGlobalTimeout (finding 3) Exit(None) was unconditionally labeled WarmGlobalTimeout, but None just means the child died to a signal -- agentd's timeout SIGKILL is only one producer. A guest-OOM-killed hook (one of the failure causes WarmConfig's own docs name) at minute 2 of a 55-minute budget also reports Exit(None), and previously the row recorded kind=warm_global_timeout -- steering an operator to raise timeout_secs instead of fixing memory. Add CaptureFailureKind::WarmKilled (wire-safe: CaptureFailed.kind is a plain proto string, not a bincode-positional enum) and gate the WarmGlobalTimeout label on started.elapsed() (plus 2s scheduling slack) actually reaching warm.timeout(); an early Exit(None) now classifies as the distinct, unattributed WarmKilled. Added warm_hook_early_signal_kill_is_not_misclassified_as_global_timeout to lock in the new classification. * fix(core,protocol): add the missing Vec<WarmStageRecord> wire golden, fix the wire-corruption bug it caught (finding 6) Finding 6: no golden pinned the CaptureProgress.warm_stages_bincode payload (Vec<WarmStageRecord>), the only bincode-in-proto-bytes type this PR introduced without one. Adding it (two records: one closed, one still OPEN with ended_at: None -- the normal shape of a live, in-progress capture) immediately failed round-trip decode with Io(UnexpectedEof), not a golden mismatch. Root cause, confirmed with a minimal bincode 1.3.3 repro outside this crate: WarmStageRecord::ended_at carried #[serde(skip_serializing_if = "Option::is_none")]. That attribute is silently corrupting for a bincode-positional (non-self-describing) payload: on encode, the derived Serialize omits the field's bytes entirely when None (no placeholder), but the derived Deserialize unconditionally reads it next in sequence -- desyncing every subsequent field/element. Since `outcome` follows `ended_at` and isn't the last field, EVERY warm_stages_bincode frame carrying an open stage (i.e. essentially every live capture, which is the entire point of this PR's streaming progress) would fail to decode on the coordinator side via decode_bincode's `?`, silently breaking live progress in production. Fix: drop skip_serializing_if (keep #[serde(default)] for the JSON `enable_jobs.warm_stages` leg, which still tolerates a missing key). Also pinned WarmStageOutcome's variant indices (Running=0, Done=1, Failed=2) per the suite's append-only-enum convention. * fix(host-agent): widen the >=30s keepalive to span the whole capture, not just the warm hook (finding 1) The >=30s keepalive lived only inside run_warm_hook. build_base_snapshot's boot phase sent exactly one Boot frame and the snapshot phase exactly one Snapshot frame -- no further progress traffic during either leg, even though both can run many minutes (a slow cold boot/materialize; pause/flush/chunk memory + upload a multi-GB state blob to GCS). enable_scanner.rs deleted the blind lease-renewal ticker on the premise that build_base_snapshot streams CaptureProgress at least every 30s for the WHOLE capture -- so silence during boot or snapshot let claimed_at go stale past lease_secs, and a peer coordinator could re-claim and spawn a second concurrent capture: exactly the regression the deleted ticker prevented, and a direct miss on issue #539's "no duplicate concurrent capture" acceptance criterion. Add spawn_leg_keepalive + a KeepaliveGuard (aborts its ticker task on drop, including through an early `?`-return) and wrap the boot leg (from VM create to wait_agent_ready) and the snapshot leg (from warm-hook-done to snapshot() returning) in it, each resending its last progress frame every ENGRAM_CAPTURE_KEEPALIVE_SECS (default 30s, same default as run_warm_hook's own warm-phase keepalive, which now shares the same env override for consistency and test-shrinkability). New tests: leg_keepalive_resends_until_dropped (the mechanism itself, via a 1s test-shrunk interval) and slow_boot_keeps_emitting_boot_phase_progress (an integration-level proof that a 2.5s wait_agent_ready with no [warm] hook still emits >=2 Boot-phase CaptureProgress events, isolating the boot leg from run_warm_hook's already-tested keepalive). * style: rustfmt the WireSkew match arm in enabled_images.rs Follow-up to the finding-4 commit; cargo fmt reflows the destructured WireSkew pattern across multiple lines. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…t literal Linux-only field (#[cfg(target_os = "linux")], from #561's RAM-ledger PSS bucketing) was missing from a LiveSandbox literal in a test added by an earlier review-fixup pass on this branch, predating #561's field. Invisible to macOS clippy/check (the field doesn't exist there); broke CI's Linux lane with E0063 missing-field.
…ness counters (scoped: span parenting + DaemonSet OTLP + counters) (#562) * fix(coordinator): re-parent detached lifecycle spawns onto the request span (#526) tokio::spawn severs the ambient tracing context, so every #210/#213 cancellation-hardened pipeline (session create boot, manual snapshot capture, resume, evict_local) became a new orphaned trace root instead of a child of the request span — the "every prod trace is spans=1" symptom from the telemetry-restoration evidence pass. Wrap each spawned future in `.instrument(tracing::Span::current())`: span context only, task lifetime (and the detach-for-cancellation-safety property) is unchanged. Adds a capturing tracing_subscriber::Layer unit test proving the instrumented shape parents correctly and the bare tokio::spawn shape (pre-fix) orphans — a regression guard. Part of the #526 telemetry-restoration scope (span parenting only; host DaemonSet OTLP export and durable counters land in follow-up commits on this branch). * fix(coordinator): give scanner-driven lifecycle pipelines an explicit root span (#526) Per the #526 implementation plan step 1's audit of remaining lifecycle spawns: idle_evictor, queue_scanner, evac_resumer, preemption_drain, and live_migration all run detached, scanner-driven pipeline bodies with no request span to inherit — their internal spans would otherwise export as disconnected roots sharing no correlating attribute. - idle_evictor::scanner_advance_one, queue_scanner::boot_placed_create, evac_resumer::advance_one, preemption_drain::drain_session, live_migration::migrate_session_live: `#[tracing::instrument]` gives each pipeline an explicit root carrying session_id (and sandbox_id / host_id where relevant) — correlation by attribute, not a fabricated parent, per the issue's explicit guardrail. - idle_evictor's D5 finalize task and live_migration's dest-restore / drain-finalize spawns detach via bare tokio::spawn mid-pipeline (the same context-severing shape as the 4 core anchors fixed in the prior commit) — re-parented with `.instrument(Span::current())` so they stitch under the pipeline's own root instead of orphaning a second time. The dest-restore spawn matters specifically because `dest.restore()` is a gRPC call carrying the TraceparentInjector interceptor — an unparented spawn there sends an empty traceparent and orphans the host-side restore spans too. No lifecycle behavior change: `#[tracing::instrument]` and `.instrument()` affect span context only, not task scheduling or cancellation semantics. * feat(deploy): host-fleet OTLP export + PodMonitoring (#526) The K8s host-fleet DaemonSet (ADR 0044) never got a collector or an OTLP endpoint when it replaced the GCE/packer fleet (which carried an otelcol systemd unit) — host-side spans (chunk.fetch, host.restore_base_for_session, restore.*) have exported nowhere since the migration, and fleet Prometheus counters die with every pod churn (no PodMonitoring template existed). - deploy/helm/engram-host-fleet/values.yaml: new `otel` block (mirrors the engram/coord chart's shape exactly: enabled, endpoint, collector.{enabled,image,configMap,resources}), default enabled=false so the OSS chart stays cloud-agnostic and inert; new `metrics.podMonitoring` block, default disabled. - templates/host-agent.daemonset.yaml: gated otelcol-contrib NATIVE sidecar (init container, restartPolicy: Always, NO readinessProbe — the standing guardrail from the coord collector incident that 502'd prod: a crashing collector must never gate the workload) + OTEL_EXPORTER_OTLP_ENDPOINT on the host-agent container when otel.enabled. Because the pod is hostNetwork, the sidecar also serves the uffd-handler (inherits env at spawn, no env_clear). - templates/podmonitoring.yaml (new): GMP PodMonitoring gated on metrics.podMonitoring.enabled, mirroring the coord chart's — samples land in Cloud Monitoring on every scrape, so pod-churn counter resets become ordinary rate()/increase() resets instead of losing the fleet-wide percentile. - deploy/otel/collector-gcp.yaml: rewrite the stale header — the packer/systemd deployment leg is gone (the ADR 0044 migration ported the workload, not the collector); documents the two real deployments (coord sidecar + host-fleet sidecar) and the host-fleet's hostNetwork/node-SA ADC gotcha for the internal-repo follow-up. Deliberately excludes the guest->collector firewall pinhole (net.rs) and ENGRAM_GUEST_OTEL_ENDPOINT wiring — that workstream is prototype-first and out of scope per the issue; the values block has no `guestEndpoint` field yet so there's nothing to wire to a nonexistent pinhole. Validated: `helm lint` + `helm template` with otel/podMonitoring on and off, output parsed with yaml.safe_load. * feat(chunk-store): NVMe fetch-latency histogram + GCS-fill effectiveness counters (#526) engram_chunk_fetch_seconds only had a blobstorage arm (the cold-tier miss path) — the fast tier's own latency was unmeasured, so "the local NVMe cache got slow" (saturation, ext4 fragmentation) had no signal. Time the hit-path read_if_present and record it under tier="nvme". Also add engram_chunk_fill_total{source="gcs"} + engram_chunk_fill_bytes_total{source="gcs"} next to the existing blobstorage bytes counter in the leader-persist arm — the baseline half of the peer-vs-GCS fill split (the peer="..." half lands at the host-agent's MigrationFetch destination pull loop in a follow-up commit); this is the meter epic-gcs-free-resume's "GCS-free by policy" claim will read. Adds a round-trip unit test (miss populates, hit returns the same hash-verified bytes without re-invoking the fetcher) proving the added timing is pure instrumentation — no change to what `get` returns. * feat(prefault+resume): per-mechanism effectiveness counters (#526) The resume-prefault mechanism went silently inert three separate, undetected ways (d0e5ecf, cf6e4d3, 7c2a722) because the only detector was manual archaeology weeks later — no counter said "prefault installed 0 chunks across N resumes". This closes that gap, plus the same-host/cross-host resume split and the peer half of the peer-vs-GCS chunk-fill counter pair. - engram-uffd-handler/src/runtime.rs: new `PrefaultStats` (serde) + `write_prefault_stats`/`prefault_stats_path` (atomic temp+rename, same pattern as the existing working-set-trace.json dump). `prefault_from_trace` times itself and writes the stats file at the end regardless of success/failure (a `?`-propagated error still leaves a record of what installed before failing, not a total absence that would masquerade as a crashed handler). - engram-uffd-handler/src/main.rs: the no-trace half — writes `trace_loaded: false` synchronously, before the listener even binds, so "no trace requested" stays distinguishable from "handler died before writing anything" on the host-agent's read side. - engram-core: new `SandboxBackend::prefault_stats_path` (default None, mirrors `working_set_trace_path`); engram-sandbox-firecracker implements it as the per-jail sibling file. - engram-host-agent/src/pooled_backend.rs: `restore()` reads the stats file post-restore, classifies into `PrefaultOutcome` (Replayed{installed,skipped} / NoTrace / StatsMissing — file absence IS the alarm), and emits `engram_resume_prefault_total{outcome=...}` + `engram_resume_prefault_chunks_total{result=...}` (crate::metrics::{RESUME_PREFAULT_TOTAL,RESUME_PREFAULT_CHUNKS_TOTAL}). Also emits `engram_chunk_fill_total{source="peer"}` + `_bytes_total` at the migration_prestage chunk-write site (the peer half of the chunk-store commit's GCS half). - engram-coordinator: `engram_session_resume_total{placement= same_host|cross_host|unknown_prior_host}` emitted once resume placement resolves in `resume_from_fc_snapshot`, comparing the chosen host against the snapshot record's capturing host (`resume_placement_label`, pure + unit-tested). Convergence note (documented at each new constant): `prefault- admission-control`, landing in the same overhaul, reuses this exact `engram_resume_prefault_*` counter namespace against a superset gate-file schema — this commit does not fork a second jail-dir file or a parallel `engram_prefault_*` namespace. Tests: PrefaultStats serialize/write/read round-trip (pure, no uffd) + the no-trace shape; classify_prefault_outcome covers all three labels plus absent/corrupt/superset-schema input; resume_placement_label covers all three placement labels. Cross-checked engram-uffd-handler, engram-sandbox-firecracker, engram-host-agent, engram-core on aarch64-unknown-linux-musl via `nix develop -c cargo clippy` (Linux- only runtime.rs is invisible to macOS clippy) — clean, no warnings. Incidentally fixed a pre-existing dangling doc-comment fragment in pooled_backend.rs (ADR 0021 P1.5 leftover) that this change's insertion point newly tripped `clippy::empty_line_after_doc_comments` against. * docs(adr-0019): amendment — Phase 0 telemetry regressed silently; #526 restores it Records why the 173-span prod-validated waterfall this ADR documents had quietly collapsed to zero host-side spans + childless-root coord traces (ADR 0044's K8s host-fleet migration dropped the collector + OTLP endpoint; the #210/#213 cancellation-hardening spawns severed span context) and what #526 restores. Also records the New-mechanism rule the issue's Solution section calls for: any optimization/ recovery-path PR must ship an effectiveness counter and name its zero-rate alarm — the prefault mechanism went inert three times undetected because that discipline didn't exist yet. * fix(sandbox-firecracker): prefault_stats_path returns None without a live uffd handler Review finding 2 [CONFIRMED]: `prefault_stats_path` returned `Some` for every sandbox, but `RestoreMode::File` restores (the config default, and the documented ENGRAM_FC_RESTORE_MODE=file fleet knob) never spawn a uffd-handler process, so nothing could ever write `prefault-stats.json`. The host-agent read that absence as `stats_missing` — the handler-died alarm — on every single File-mode resume, a fleet-wide false positive by construction. Key the answer off the live sandbox's `uffd_pid` (populated for RestoreMode::Uffd restores AND pidfd-reattached Uffd sandboxes, ADR 0044 K2) instead of static backend config, mirroring the trait contract's `None` = "no per-sandbox prefault detector" used by VZ/Process. Adds a regression test covering: no live entry, live without a handler, live with one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(chunk-store): don't count a GCS fill when write_local failed; name fill metrics via constants Review finding 4 [CONFIRMED]: `engram_chunk_fill_total{source="gcs"}` / `engram_chunk_fill_bytes_total{source="gcs"}` incremented unconditionally after a blobstorage fetch, even when the immediately-preceding `write_local` call failed (ENOSPC/perms — the exact disk-pressure class #526 itself cites). A fetch whose local persist failed did NOT fill the cache; counting it anyway masked precisely the "warming ran but reads still miss" state the write_local warning next to it exists to catch, and broke `rate(fill{gcs})` vs `rate(fetch{blobstorage})` as a persist-failure detector. Gate the two fill increments on `write_local`'s outcome; the `engram_chunk_cache_bytes_total` volume counter stays unconditional (it measures the fetch, not the fill). Review finding 7 [CONFIRMED] (chunk-store half): the fill metric names were inline string literals, unlike this same PR's `RESUME_PREFAULT_*` constants. Add `CHUNK_FILL_TOTAL` / `CHUNK_FILL_BYTES_TOTAL` here, doc-commented as the shared contract with the peer half (`engram-host-agent::pooled_backend`, next commit) — a typo in either literal would otherwise silently fork the series. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(host-agent): count pull_chunks_from_source's peer fills via the fill-metric constants Review finding 3 [CONFIRMED]: `pull_chunks_from_source` (the synchronous pre-resume divergence pull, ADR 0045 C1 — hundreds of chunks/MB per its own comments) lands chunks into the local cache via `put_no_evict` same as `migration_prestage`'s loop, but left them uncounted against `engram_chunk_fill_total{source="peer"}`. On any warm migration resume this systematically undercounted peer-fill volume — the baseline meter epic-gcs-free-resume's "GCS-free by policy" claim depends on. Factor a shared `count_peer_chunk_fill` helper (both loops now call it) and route it through the previous commit's `engram_chunk_store::cache::CHUNK_FILL_TOTAL` / `CHUNK_FILL_BYTES_TOTAL` constants instead of a second copy of the inline literals — finding 7's host-agent half: a typo forking the peer vs. gcs metric name is now a compile error, not a silent series split. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(host-agent,uffd-handler): fix the prefault-stats race, add restore-span attributes, record peer-fill fields Combined commit for three tightly-coupled findings against the same restore()-time prefault-effectiveness-detector code path (finding 5 extends finding 1's fix in place; finding 6 extends finding 5's span with more fields) — split three ways below for traceability. Review finding 1 [CONFIRMED, the flagship bug]: `restore()` read `prefault-stats.json` the instant `restore_with` returned — i.e. the instant FC resumes vCPUs — while `prefault_from_trace` writes that file only when its deliberately-concurrent background thread (ADR 0043 P1) finishes, seconds later. Healthy replayed resumes chronically misclassified as `stats_missing`, inverting the detector's purpose and breaking the issue's own acceptance criterion (`engram_resume_prefault_total{outcome=replayed} >= 1` after one evict->resume). Fix: detach the read into a `tokio::spawn`ed, bounded poll (`read_prefault_stats_with_retry`, 250ms cadence / 20s timeout) that never blocks `restore()`'s return — the same off-critical-path guardrail ADR 0043 P1 already established for the write side. A file that still isn't there after the timeout is a real `stats_missing` alarm, just no longer a false one on the first instant. Covered by `prefault_stats_retry_picks_up_a_late_write` (late-arriving file is picked up) and `prefault_stats_retry_times_out_to_stats_missing` (genuine absence still alarms). Review finding 5 [CONFIRMED]: issue step (d)1's "record the numbers as attributes on the restore span" was skipped (log line only), undisclosed in the PR's deviations section. Recording directly onto the RPC's `host.restore` span isn't safe once the read is detached (holding that span's handle open across the poll would inflate its reported duration in Cloud Trace by however long the poll takes) — so this gives the detached task its own `resume.prefault_stats` span, correlated by `sandbox_id`, matching this same PR's existing "detached work gets its own span, not a fake/held-open parent" convention (scanner-driven pipelines, `restore.prefetch_memory_bg`). `outcome`/`installed`/`skipped` recorded via pre-declared `tracing::field::Empty` fields. Review finding 6 [CONFIRMED]: issue step (d)3's "uffd-handler live peer page-serves are recorded in the same per-jail stats file" was skipped (no fields in `PrefaultStats`), undisclosed. Fixed rather than deferred (the reviewer's "cheap to fix now" held up): `engram-uffd-handler` captures the peer drain snapshot (`pulled`/`alt_sourced`/`zero_chunks` from `DrainStats`, plus live fault count from `Peer::fault_stats()`) and patches it onto `prefault-stats.json` via a pure, tested read-modify- write (`patch_peer_fields`) at the very end of `run_listener`'s background thread — strictly after `prefault_from_trace`'s own write, so it can never be clobbered, and a no-prior-writer case (handler died before anything wrote) is left alone rather than fabricating a file that would mask finding 1's alarm. `engram-host-agent` parses these independently of the counter-outcome contract (`parse_peer_fill_snapshot`) and records them on the same `resume.prefault_stats` span — span attributes only for now, per the issue's own scoping ("epic-gcs-free-resume owns promoting them" to counters). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(sandbox-firecracker): update finding-2 test for the guest_ip -> guest_endpoints rename main renamed LiveSandbox::guest_ip -> guest_endpoints (#555, merged in the previous commit) after this PR's base — update the prefault_stats_path_none_without_a_uffd_handler regression test's struct literal to match. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(sandbox-firecracker): add missing parked field to LiveSandbox test literal Linux-only field (#[cfg(target_os = "linux")], from #561's RAM-ledger PSS bucketing) was missing from a LiveSandbox literal in a test added by an earlier review-fixup pass on this branch, predating #561's field. Invisible to macOS clippy/check (the field doesn't exist there); broke CI's Linux lane with E0063 missing-field. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(coordinator,host-agent): typed capability-vector host readiness + probe-before-host_lost (ADR 0068)
Replaces the registered-boolean host-readiness model (and its four
incident-driven point-fix bandages: the gRPC gate, the base-shm tmpfs
withhold, the NBD silent downgrade, the invisible wire-skew drop) with a
self-verified `HostCapabilities` vector, probed by the host-agent at
startup and every heartbeat, persisted to `hosts.capabilities` (migration
0077), and gated on directly by the placement filter
(`host_meets_capabilities`). NoCapacity picks now name every host's
exclusion reason (`engram_placement_excluded_total{reason}` +
`placement::exclusion_summary`), closing the "no capacity with free
hosts" mystery mode; the fleet view surfaces `failing_capabilities` /
`fc_snapshot_version` / `capabilities_schema` (fleet.proto fields 24-26).
Also adds probe-before-host_lost: a new `HostService.ProbeSandbox` RPC +
`HostClient::probe_sandbox` (no default `Ok`), with a ground-truth
FirecrackerBackend override (persisted per-sandbox manifest pid identity,
independent of the in-memory sandbox map). `reconcile::flip_missing`
probes before flipping a session to `host_lost`, rescuing the fbd3794c
incident shape (a provably-alive VM flapped host_lost/resumed 8x in 12
min while the host was reachable). The heartbeat handler now persists
before running reconcile, so a 5xx'd heartbeat can never drive flips.
See docs/adr/0068-capability-vector-readiness.md for the full design,
deviations from the issue's literal 3-PR plan, and acceptance-criteria
status.
Closes #531
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
* fix(coordinator): treat NotApplicable substrate caps as passing (review finding 1)
host_meets_capabilities() failed a required base_shm_tmpfs/uffd_minor_shmem/nbd
capability on NotApplicable exactly like Failed/Unknown. An ADR 0022
File-backend FC host honestly reports NotApplicable for these three (the
UFFD substrate was never configured, so there's nothing to probe) — but it
can still legitimately serve memory-manifest restores via the File-backend
path. The old gate made a File-mode fleet 100% NoCapacity for every
memory-manifest placement, foreclosing the ADR 0022 canary/flip.
Now only Failed (probe ran, broke) and Unknown (never probed despite
schema >= 1) withhold a required substrate capability; NotApplicable passes
alongside Ok. Split the old single test into one asserting Failed/Unknown
still reject, and a new one asserting NotApplicable passes.
PR #560 (issue #530, dead-code purge) was checked and confirmed NOT to have
deleted RestoreMode::File (item a explicitly "NOT DONE" per its PR body), so
this finding is still live and needed fixing here rather than being made
moot by landing order.
Review: PR #564, finding 1 (crates/engram-coordinator/src/placement.rs:124)
* docs(coordinator): fix exclusion_summary reason-order doc (review finding 5)
The doc comment on exclusion_summary() stated the first-match order as
excluded → stale → cordoned → wire_skew → cap → digest_not_ready → no_fit,
omitting not_ready entirely. The actual code (and its own unit test,
exclusion_summary_names_the_first_matching_reason) checks
excluded → not_ready → cordoned → wire_skew → stale → cap → digest_not_ready
→ no_fit. Fix the doc to match so an operator/reader doesn't misread which
gate fires first.
Review: PR #564, finding 5 (crates/engram-coordinator/src/placement.rs:280)
* fix(coordinator): stamp fc_snapshot_version at all record_snapshot sites (review findings 2, 3)
Issue #531 step 11 asked to stamp snapshots.fc_snapshot_version wherever
record_snapshot is called with a known host. The idle-evictor pipeline and
checkpoint-advert sites did this, but two more call sites had the host in
hand and still recorded None:
- api/snapshot.rs::snapshot_core (API-initiated session snapshots):
host_id is resolved just above; now does the idle-evictor's best-effort
fc_snapshot_version_for_host lookup before building the SnapshotRecord.
- api/enabled_images.rs::capture_and_record_base_snapshot (base-template
capture): host_id: Some(host_id) was already in the same struct
literal; same lookup added.
Leaving these unstamped meant an API-initiated snapshot or a fresh base
capture during a SNAPSHOT_VERSION cutover (#532) could resume unconstrained
onto a mismatched host — the exact corruption class the PR's acceptance
criterion claims closed.
Also rewrites the misleading comment at api/sessions.rs (create path):
it justified fc_snapshot_version: None by claiming pairing "only matters
for RESTORING a previously-captured snapshot... not for booting from the
base template" — but a create IS an FC restore of the base snapshot (ADR
0020, no warm pool). The real reason creates stay unconstrained is that
`PreparedBoot` assembly doesn't have the base row's recorded version
threaded through (`enabled.base_snapshot_memory_manifest` only, no
fc_snapshot_version column) — now genuinely deferred as a scoped follow-up
rather than misdescribed as inapplicable.
Review: PR #564, findings 2 (api/snapshot.rs:350), 3 (api/sessions.rs:659),
and the enabled_images.rs:520 inline comment (same cluster as finding 2).
* fix(host-agent): don't permanently cache a transient fc_snapshot_version probe failure (review finding 4)
FC_SNAPSHOT_VERSION's OnceLock cached the probe's Option<String> result
unconditionally, including None. A None here (once firecracker_bin is
Some, i.e. we're actually on the FC backend) can only mean the
`firecracker --snapshot-version` subprocess spawn or exit failed —
possibly transiently (fork EAGAIN under boot-time load, the binary
momentarily missing mid-thin-layer-bake). Locking that in meant every
snapshot the host captures stays fc_snapshot_version-NULL and every
restore onto it stays version-unconstrained until process restart,
invisibly (None also legitimately means "not FC", so nothing downstream
could tell the two apart).
Now only a successful probe populates the OnceLock; a failed probe
re-spawns (one cheap failed subprocess call) on the next heartbeat tick
and self-heals once the transient condition clears. Tightened the
existing test to assert two consecutive calls with a missing binary both
re-probe and return None, rather than tolerating "a prior test may have
already cached something" as before.
Review: PR #564, finding 4 (crates/engram-host-agent/src/capabilities.rs:297)
* fix(web): plumb fc_snapshot_version + capabilities_schema to the fleet view (review finding 6)
Issue #531 step 13 asked for a one-glance skew display in the fleet view.
failing_capabilities made it end to end, but fc_snapshot_version and
capabilities_schema (proto HostView fields 25/26) stopped at the generated
bindings — dropped by protoHostToLegacy, absent from types.ts and
Fleet.tsx. An operator chasing a cap:fc_snapshot_version NoCapacity
exclusion had no way to see any host's reported snapshot version, and a
schema-0 (never-reported, soft-pass) host was visually indistinguishable
from a probed-healthy one.
- lib/types.ts: add fc_snapshot_version/capabilities_schema to HostView.
- hooks/useHosts.ts: map the two proto fields through in protoHostToLegacy.
- pages/Fleet.tsx: render the host's fc_snapshot_version inline with the
other per-host stats, and a "caps unreported" badge when
capabilities_schema === 0 (mirrors the existing failing_capabilities
badge).
- operator-health.test.ts: thread the two new required HostView fields
through the test's host() factory.
Review: PR #564, finding 6 (web/src/hooks/useHosts.ts:21)
* ci: hoist musl-lane CFLAGS/BINDGEN env to job level (review CI finding)
cross-compile linux-musl artifacts was failing: the new target-gated
userfaultfd = "0.9" dep in engram-host-agent (ADR 0068's UFFD MINOR
capability probe) pulls userfaultfd-sys into the host-agent musl build for
the first time. Its build script runs bindgen against a wrapper header
that pulls in <linux/types.h>, which needs the kernel UAPI include paths
pointed at explicitly for the musl target. The job already solved this for
engram-uffd-handler's own step (CFLAGS_x86_64_unknown_linux_musl +
BINDGEN_EXTRA_CLANG_ARGS), but scoped it to that one step, so the
host-agent step ran with CFLAGS=None and failed the same way uffd-handler
used to.
Hoist both env vars to the cross-musl-linux job level so every step in the
lane gets them — not just step-scoped onto the crate that happened to need
it first. Any future crate in this lane that grows the same transitive
dependency is covered for free instead of needing its own copy-paste fix.
Validated: `python3 -c "import yaml; yaml.safe_load(...)"` parses clean;
`actionlint .github/workflows/ci.yml` reports only pre-existing findings
(the blacksmith-* self-hosted runner labels it doesn't recognize, and two
pre-existing shellcheck style nits elsewhere in the file) — nothing new
from this change.
* chore(migrations): renumber 0077_host_capabilities to 0080 (batch collision)
Six PRs in the 2026-07 core-ops overhaul batch all added a migration
numbered 0077 from the same main base. Land-queue assignment resolves the
collision: #560 keeps 0077, #561->0078, #563->0079, #564 (this PR)->0080,
#565->0081, #566->0082.
Renames deploy/migrations/0077_host_capabilities.sql to
0080_host_capabilities.sql and updates the "(migration 0077)" comment
references in crates/engram-core/src/types/{host,snapshot}.rs,
crates/engram-postgres/src/row.rs, and docs/adr/0068-capability-vector-
readiness.md to match. No SQL content changes — the migration is not yet
applied anywhere, so this is a pure rename, not an edit to an
already-applied (checksum-immutable) migration.
Also appends a "Post-review fixes (PR #564)" section to the ADR
documenting the six review findings fixed on this branch (NotApplicable
substrate gating, the two unstamped record_snapshot sites, the misleading
sessions.rs comment, the OnceLock transient-failure cache, the
exclusion_summary doc-order fix, and the web fleet-view plumbing), plus
the CI musl-lane CFLAGS fix, per this repo's ADR-bookend convention
(update between phases with divergences found).
* docs: update migration-0077 comment references to 0080 + ADR post-review notes
Follow-up to 453db0a (the migration file rename) — that commit only
staged the renamed .sql file because a stray invalid pathspec in the same
git add invocation aborted staging of these four files. Completes the
0077->0080 reference update in the code comments
(engram-core/src/types/{host,snapshot}.rs, engram-postgres/src/row.rs) and
lands the ADR's "Post-review fixes (PR #564)" section + NotApplicable
semantics correction that were part of the same intended commit.
* fix(coordinator): match ProbeBackend mock's guest_ip signature to the tightened Ipv4Addr type (post-merge fixup)
* debug(placement): log want/have on fc_snapshot_version gate mismatch
The e2e teleport lane deterministically excludes the only candidate host
with cap:fc_snapshot_version even though both hosts run the identical
firecracker binary. The exclusion summary carries only the static reason,
so the two compared strings are invisible. Log them at the mismatch site.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
* fix(capabilities): parse only the first line of firecracker --snapshot-version
The fork's binary appends a timestamped exit-log line to stdout after
the version. Stamping the whole trimmed stdout made every probe string
unique per invocation, so the snapshot's capture-time version never
equaled any host's heartbeat-reported version and the placement gate
deterministically excluded every candidate on cross-host restore — the
e2e teleport lane failed with cap:fc_snapshot_version on all 18 evac
retries. Parse the first non-empty line only, via a pure helper with a
regression test (the OnceLock success cache makes the probe itself
untestable in-process).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… stages_images (this PR) with capabilities/RAM-ledger/capture-progress (main #557/#559/#561/#562/#563/#564) Conflicts across ~37 files were purely additive (both sides adding a sibling field/const/column at the same insertion point) except: - crates/engram-postgres/src/lib.rs: touch_host_heartbeat's UPDATE grew to $21 placeholders (main's RAM-ledger + capabilities columns at $17-$20, this branch's stages_images renumbered to $21); SQL SET clause and .bind() call order kept in lockstep. - crates/engram-protocol/proto/.../image.proto: both sides claimed field 11 on EnableJob; renumbered this branch's prestage_hosts to 15 (next free after main's capture_phase/warm_stage/warm_stage_started_at/ warm_stages/output_tail at 11-14). Regenerated TS bindings via `buf generate` instead of hand-merging orchestrator/web gen output. - crates/engram-host-agent/src/{util.rs,main.rs} and crates/engram-chunk-store/src/cache.rs: this branch carried a pre-#557-follow-up snapshot of shared disk-budget code (the UtilizationProbe RAM-ledger signature, the mountpoint gate's ENGRAM_HOST_ROOT_REF_PATH fix, the sweeper's t=0-tick skip); took main's superseding versions wholesale since HEAD had nothing unique left in those blocks (post-merge diff against main is byte-identical). - crates/engram-coordinator/src/queue_scanner.rs: git's line-based merge silently mis-nested the `PlaceOutcome::ImageGone` match arm outside its enclosing `match place_create(...)` block (no conflict markers, but invalid syntax) — caught by `cargo fmt --check`; moved it back inside as the last arm. - Several HostRecord/HostHeartbeat struct literals in files git never flagged as conflicting (enable_scanner.rs, enable_jobs_live_pg.rs, placement_reservation_live_pg.rs) were missing the sibling branch's new field entirely, since only one side ever touched those literals; audited every HostRecord/HostHeartbeat literal in the tree and added the missing field. - docs/adr/0067-chunk-cache-disk-budget.md (add/add): took main's fully-accepted version (includes the Post-review-fixes section for the two bugs above) — this branch's copy was a stale duplicate from before that ADR's own follow-up commits landed. Migration 0081_enable_job_prestage.sql was already correctly numbered after main's 0080_host_capabilities.sql; no renumbering needed. Verified: cargo fmt --check, cargo clippy --workspace --all-targets (0 warnings), cargo hakari verify, cargo nextest run across engram-{coordinator,host-agent,chunk-store,protocol,core,postgres} (904 passed, 104 live-PG skipped), musl cross-clippy, web pnpm build (tsc+vite), orchestrator bun typecheck, helm template with storage.dedicatedDevice set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
… overlapped boot legs + prompt-over-wire (#566) * feat(coord): migration 0077 — session.selected_skills + fleet_catalog_changed NOTIFY trigger Part of issue #535 (create-as-a-plan). Groundwork for two later commits: - sessions.selected_skills (TEXT[]) persists a create's dynamic-mount selection so the queue scanner's boot re-prepare can reconstruct it (ADR 0055 TODO(P1-D): queued creates currently boot with base skills only, since the queue row never carried the selection). - notify_fleet_catalog_changed() + a trigger on hosts scoped to current_bundles changes (guarded by IS DISTINCT FROM, so it stays quiet across the few-seconds heartbeat UPDATE and only fires on an actual host-roll stamp change) backs the coordinator's boot-bundle cache invalidation, added in a follow-up commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(coord): per-enabled-image boot bundle cache (issue #535 a) Combines the issue's steps 2+3 into one commit (the NOTIFY listener's invalidation calls need the cache type to exist first; splitting them would leave a dead intermediate compile state). - New `boot_bundle` module: `BootBundleCache` read-through caches, per enabled image, the parsed manifest + fetched base-snapshot record + resolved memory/vcpu budgets (today re-derived on every create), plus the fleet's baked bundle-name→sha catalog (today re-scanned via `list_active_hosts` up to three times per create). Both are TTL'd (30s belt-and-braces against a dropped PgListener notification). - `engram-postgres`: `upsert_enabled_image` / `soft_delete_enabled_image` now fire `pg_notify('enabled_image_changed', image_uri)` inside their existing transaction (delivered iff it commits); `delete_enabled_image` fires it best-effort after, mirroring `org_secret_changed`. - `pg_listener`: subscribes `enabled_image_changed` (invalidates one cache entry) and `fleet_catalog_changed` (invalidates the whole catalog — see migration 0077's trigger, landed in the prior commit). - `prepare_from_grpc` / `prepare_from_row` / `prepare_inner` / `fleet_bundle_catalog` rewired onto the cache: the strict (non-soft-deleted) vs. tolerant (`_any`) split moves to the two call sites (the cache always fills via the tolerant view), and `boot_on_reserved_host`'s per-create `get_snapshot` is gone — the snapshot record now rides `BootInputs.base_snapshot` from the bundle. Net: a warm-cache create now does zero `toml::from_str` calls and zero extra `list_active_hosts` scans beyond the one placement still needs (`candidates_for` — deliberately kept, per the issue's "conscious divergence": placement needs a heartbeat-fresh host view). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(coord): one-transaction session write-set (issue #535 b) Collapses `reserve_placement` + `enqueue_session_create` + the boot/ enqueue paths' separate satellite-write chains into a single `MetadataStore::reserve_and_persist_create` call whose Postgres impl commits the ENTIRE write-set — the row (placed or queued) plus every satellite (sealed secrets, capabilities, integration policy, harness, selected skills) — in ONE `FOR UPDATE` transaction, before any host RPC. - `engram-core`: new `SessionCreateWriteSet` / `CreateDisposition` + `MetadataStore::reserve_and_persist_create` (replaces `reserve_ placement` + `enqueue_session_create`) and `transition_session_created` (a slim `pending → created` + `sandbox_id` UPDATE, replacing `create_ session_created`'s INSERT-or-UPDATE upsert — the row is now guaranteed to already exist). `Session` gains `selected_skills: Vec<String>`, fixing the ADR 0055 TODO(P1-D) gap: a queued create's boot re-prepare can now reconstruct its dynamic-mount selection instead of silently dropping to base skills. - `engram-postgres`: the transactional impl (extends `reserve_placement`'s FOR-UPDATE body); `get_session`/`list_active_sessions`/ `list_queued_sessions_fifo` project the new column. - `engram-coordinator`: `boot_prepared` seals secrets (KEK, pure crypto — has no place inside the DB transaction) and serializes the policy BEFORE calling `reserve_and_persist_create`, then dispatches on `CreateDisposition` — `enqueue_create` as a separate function is gone, its Queued-disposition handling folds into `boot_prepared`. `boot_on_reserved_host` now does exactly ONE write of its own (`transition_session_created`, since the sandbox doesn't exist until the restore RPC returns) — the FK-ordering bug class (a satellite write racing the row's own insert; the ADR 0051 forge-token regression) is dead by construction, not by "call it after the row" convention. - Every other `MetadataStore` impl (9 test/mock fixtures across 6 crates) updated: the 2 that exercise the real create path (coordinator HTTP + gRPC integration tests) got honest in-memory equivalents; the rest mirror their pre-existing `unreachable!()`/`unimplemented!()` convention for unexercised trait surface. - New Postgres-level tests (`placement_reservation_live_pg.rs`) proving the write-set's atomicity: a single call commits every satellite together, and a forced mid-transaction failure (duplicate session_id) leaves NOTHING from that attempt — not even satellites that would have followed the failing statement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(coord): overlap the independent boot legs with the restore RPC (issue #535 c) `boot_on_reserved_host` no longer serializes the restore RPC behind the env/egress work (or vice versa) — the two are independent (neither touches the other's inputs) and now run concurrently via `tokio::join!`: - Restore leg: `restore_base_on_host` (the ~0.4-0.7s VM-side work). - Env/egress leg: the per-spawn forge/upload broker-token mint (`inject_harness_env`) + the integration policy's Plane-B injection resolution (`resolve_inject_entries`, which can round-trip an external mint-provider API for a mint-mode connector) — this is where that external round trip moves OFF the serial tail. Both only need `session_id`/`image_ref`/`integration_policy`, not the sandbox; the broker-token FK has been satisfiable since `reserve_and_persist_ create` committed the row, well before this function runs. `build_egress_policy` splits accordingly: `resolve_inject_entries` + `build_observe_entries` (sandbox-independent, now called from the overlapped leg) stay as-is; the renamed `assemble_egress_policy` is the remaining sandbox-dependent half (`guest_ip` + final assembly). Also parallelizes `resolve_policy_secrets`' per-secret `SecretStore` round trips (order-insensitive — no secret depends on another) via `futures::future::join_all`, replacing the one-at-a-time loop. `transition_session_created` + the removal of `boot_on_reserved_host`'s satellite writes already landed in the prior commit (they're the same underlying change as the one-transaction write-set — splitting them would have left a dead intermediate compile state), so this commit is scoped to the actual leg-overlap + secret-resolution parallelization. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(coord,harness): prompt over the wire (issue #535 d) The initial prompt no longer rides `ENGRAM_INITIAL_PROMPT` env — every prompt, first or follow-up, is now a harness-protocol `Prompt` frame: - `api/prompt.rs`: factored the echo-then-forward core out of `send_prompt_core` into `deliver_prompt(state, session_id, sandbox_id, prompt_id, text)` — the user-echo-first ordering (load-bearing for web rendering) and the self-healing `deliver_with_reattach` forward, shared by every caller. - `session_boot::boot_on_reserved_host`: after the Active flip, mints a server-side `prompt_id` and calls `deliver_prompt` for the initial prompt — replacing the synthetic `prompt_id: None` event. Delivery failure past the reattach budget is `BootError::Started` (terminal, consistent with a `start_agent` failure): a session that can't receive the prompt that created it is broken. - `resolve_harness` no longer takes an `initial_prompt` param or inserts `ENGRAM_INITIAL_PROMPT`; `git grep ENGRAM_INITIAL_PROMPT` now returns nothing. - `engram-harness-claude`: `run_engine` drops the `initial_prompt` param — the pending queue starts empty and the first prompt arrives via `HarnessCommand::Prompt` like every other one. Updated the 8 unit tests that seeded an initial prompt through the deleted parameter to instead send it via `cmd_tx` post-spawn (and to expect the leading `Idle` the engine now emits before any prompt arrives, since the env-seeded fast-path — "start turn 1 with no leading Idle" — no longer exists). - `engram-host-agent/tests/e2e_harness.rs`: `capture_sink` now also returns a command sender so `drive_harness` can push the initial prompt as a wire frame instead of an env var — a hand-rolled minimal stand-in for `HarnessHub` (this test drives `SandboxBackend` directly, no coordinator/hub in the loop). The queued path unifies for free: `SessionCreateWriteSet::queue_prompt` (landed in the write-set commit) is already the durable prompt, and `prepare_from_row` threads it into the identical `boot_on_reserved_host` delivery path — no separate queued-prompt spelling. Deviation: did not extend `e2e_stack.rs`'s create-with-prompt test (`e2e_claude_with_bogus_key_surfaces_anthropic_auth_error`) to assert `prompt_id` threads onto `run_started` — it's quarantined (#403, excluded from the gating e2e lane) and requires a live `ENGRAM_E2E_GRPC_ADDR` stack this environment doesn't have, so the change is unverifiable here. The same property (RunStarted.prompt_id matches the delivered prompt_id) is covered by engram-harness-claude's `queue_holds_edits_and_consumes_type_ahead` unit test instead. Note: engram-harness-claude's test module is `#[cfg(target_os = "linux")]`-gated and e2e_harness.rs is Linux+KVM+FC+Docker+sudo-gated — neither compiles or runs on this macOS dev machine; both are verified by careful reading + will run for real in CI's Linux/FC lanes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * feat(coord): coord_prepare/coord_finalize phase metrics + doc pass (issue #535) Adds the two phase labels the issue's acceptance criteria need to turn "the coordinator serial tail is ~1s" from an estimate into a measurement, on the existing `engram_session_boot_seconds` histogram (no new metric names): - `coord_prepare`: `create_session_core` entry through `reserve_and_ persist_create`'s commit — the serial coordinator-side prefix ahead of the (now-concurrent, host-side) restore work. Recorded on the Placed path only. - `coord_finalize`: the restore RPC returning through the `created → active` transition — the coordinator-owned tail after the host hands back a live sandbox. Success path only. `total` minus (`coord_prepare` + `coord_finalize`) is the actual host-side restore RPC wall time — the split this issue's evidence section was missing. Doc pass: `session_boot.rs`'s module header now describes the (a)-(d) pipeline shape instead of the pre-refactor procedure; the FK-ordering guard comment in `prepare_inner` (the anchor the issue tracked as `sessions.rs:1197-1203`, drifted slightly by the time this landed) rewritten to describe the current dead-by-construction invariant instead of the historical hazard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc * fix(coord): guard transition_session_created on status=pending (finding #2) The UPDATE that flips a session row to `created` + binds `sandbox_id` had no status guard and ignored `rows_affected`, so it returned `Ok(())` even when the row was deleted (DeleteSession) or requeued (stale-pending scanner) while the restore RPC that precedes this call was in flight. That silently binds a live sandbox onto a gone/inconsistent row instead of hitting the existing `Err` teardown arm in session_boot.rs, which destroys the now-orphaned sandbox. Add `AND status = 'pending'` to the WHERE clause and return MetaError::NotFound on rows_affected() == 0, matching the convention used elsewhere in this file (e.g. delete_registry_credential). * fix(coord): check rows_affected on the transition_session_created guard (finding #2 cont'd) Completes the previous commit: the WHERE clause guard alone silently swallowed a lost race (0 rows matched) as Ok(()) unless rows_affected() is actually checked. This was split out of the prior commit by mistake during hunk staging; closing the gap here. * docs(coord): fix stale FOR UPDATE lock-duration comment (finding #3) "Held only for the pick + insert below (sub-ms)" stopped being true once reserve_and_persist_create's satellite writes (sealed-secrets insert, per-capability insert loop, integration-policy upsert) moved inside the same transaction as the FOR UPDATE host-row lock (issue #535 (b)) — the lock is now held until tx.commit() at the end of the function, across all of that. Correct the comment so the next reader doesn't under-estimate placement-lock contention on a many-capability create burst. * refactor(coord): delete dead upsert_session_secrets/set_session_harness (finding #4) Both are zero-caller writers left behind by reserve_and_persist_create subsuming the old satellite-write paths: the sealed-secrets bytea and the harness selection now ride the one-transaction write-set / row INSERT directly (engram-postgres/src/lib.rs's reserve_and_persist_create), not these standalone upserts. Confirmed zero callers workspace-wide (including orchestrator/ and web/) before deleting — only the trait declarations, the PostgresStore impls, and 10 mock impls referenced them. Per the repo's clean-break convention, retire both from the trait, the PG impl, and every mock rather than leaving them as an orphaned, unused re-entry point for the FK-ordering/partial-write bug class this PR set out to kill. get_session_secrets/delete_session_secrets and get_session_harness are untouched — those remain live (read/delete) call sites. * fix(coord): split INSERT/UPDATE migration triggers to fix invalid WHEN-OLD DDL (finding #1) migration 0077's `hosts_notify_fleet_catalog_changed` trigger's WHEN clause referenced OLD on an AFTER INSERT OR UPDATE trigger. Postgres rejects this at CREATE TRIGGER time ("INSERT trigger's WHEN condition cannot reference OLD values") — OLD doesn't exist on INSERT and the restriction is static, not runtime, so the `OLD IS NULL` guard didn't help. This is the exact error CI hit ("while executing migration 77") and would crash-loop every coordinator replica at boot on merge, since migrations run at coordinator startup. Split into two triggers: an INSERT trigger with no WHEN clause (a new host's first bundle stamp always counts as a "change"), and an UPDATE trigger with `WHEN (OLD.current_bundles IS DISTINCT FROM NEW.current_bundles)`. Also updates the pg_listener.rs comment describing the guard now that it only applies to the UPDATE leg. * chore(migrations): renumber 0077 -> 0082 (batch land-queue collision) Six PRs in this land batch each added a migration numbered 0077. Land-queue assignment: #560 keeps 0077, #561->0078, #563->0079, #564->0080, #565->0081, this PR (#566)->0082. Pure rename plus updating the two in-repo comments that named the migration by number; no SQL content change. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Closes #540
Summary
One host-side RAM ledger (
engram-host-agent::ram_ledger) replacing thethree unreconciled views of host RAM: kernel
MemAvailable, theUtilizationProbeformula (silently assumed resident ⇔ reserved), and theidle-evictor's own private
/proc/meminforead. Every heartbeat tick nowbuilds one
RamLedgerSnapshot, charges every MiB to a named bucket(running-VM PSS, parked-paused PSS, base-shm tmpfs residency + pending
prewarm charges), and derives
allocatable_mibfrom it once — theheartbeat and the pressure gate both read the same snapshot (via a
tokio::sync::watchchannel), so they can never disagree.Verified against commit f660225 (issue-authoring time); worktree HEAD
was 42ed9bb — every file:line anchor cited in the issue was re-checked
against HEAD before implementation; all held (line numbers drifted by only
a few lines in places, noted below, no anchor was wrong).
What changed
crates/engram-host-agent/src/ram_ledger.rs:RamLedgerSnapshot(
allocatable_mib(),free_pct()) +RamLedger(the pending-base-shm-charge registry).
allocatable = MemAvailable + Σ non-parked VM PSS − pending base-shm charges. Parked PSS is never added back — the fixfor the double-count the parking ladder would otherwise introduce.
engram-core::traits::sandbox::GuestMemoryStatsgainsparked_pss_bytes/parked_rss_bytes/parked_sampled; theFirecracker backend's
guest_memory_stats()buckets its smaps_rollup sumby a new per-sandbox
parked: boolflag onLiveSandbox(alwaysfalsetoday — no backend transition sets it yet). No
SandboxBackendtraitmethod added for park/unpark, per the issue's explicit guardrail — only
a
pub(crate), test-visible setter on the FC backend.UtilizationProbe::sampleno longer computesallocatable_mibitself; it projects the heartbeat tick's
RamLedgerSnapshotintoHostUtilization's memory fields. The stale "mlock'd base-memfileresidency" formula comment is deleted (that machinery is generation-
purge's, Generation purge: delete the dead memory-path, migration, and compat-stub generations #530, to actually delete).
its own private
/proc/meminfosample.mem_pressure_checkdeleted (thepure
mem_pressure_fromcore is unchanged, tests re-pointed at snapshotmath — no change needed, they already only exercised the pure core).
HOST_MEM_FREE_PCTmoved to the heartbeat's single emission site (waspreviously only set inside the pressure-aware branch).
image_prefetch.rs): registers themanifest's non-hole byte total + target path in the ledger before
prewarm_base_shmwrites, settles in both the success and failure arms.RamLedger::samplenets each pending charge against that specific file'sown
st_blocks(not just the raw promised total), so the charge shrinksas bytes actually land instead of double-subtracting the written
fraction for the whole write window (see Post-review fixups below).
Added a tmpfs-headroom pre-check (the 2026-06-28
pwrite ... No space left on deviceincident class) that skips thewrite with a counter
(
engram_base_shm_prewarm_skipped_total{reason="tmpfs_headroom"}) —the existing warn-and-continue-then-lazy-backstop failure posture for the
write itself is untouched.
0078_host_ram_ledger.sql— renumbered from0077 post-review to resolve a 6-way collision with sibling in-flight PRs
in the same land-queue batch):
HostUtilizationgainsbase_shm_mib,base_shm_pending_mib,parked_pss_mib,running_pss_mib(all#[serde(default)]). Only the first, third, and fourth persist to PG(
base_shm_pending_mibis transient host-local state already folded intothe persisted
allocatable_mib— deliberately not its own column, notedin the migration). Surfaced on the fleet view:
api/hosts.rsHostView,fleet.protoHostView(+3 fields),grpc_app/convert.rs's totality-guard destructure, and the web Fleet page (a small
ram: X GiB running · Y GiB parked · Z GiB base-shmreadout, hidden until non-zero).engram-host-operator'sHostLoadconversion is a partial (non-exhaustive) destructure and needed no change — verified.
engram_host_ram_ledger_mib{category=...},engram_host_ram_allocatable_mib,engram_host_base_shm_tmpfs_{total,used}_mib(now gated onmeasured, and_used_mibis a realstatfsread, not a duplicate ofthe
base_shmcategory gauge — see Post-review fixups),engram_base_shm_prewarm_skipped_total,engram_sandbox_guest_parked_{pss,rss}_bytes.formula, the resident⇔reserved invariant, and the no-sharing-discount
stance (backed by the 2026-07-01 prod rss≈pss datum).
Post-review fixups (2026-07-03)
A deep-review pass (review)
found one real accounting bug and two acceptance-criteria boxes that were
checked but not actually true. All six findings were addressed on this
branch:
batch also claimed migration number 0077. Renumbered this PR's migration
to 0078 per the batch owner's collision resolution (0077 stays with
Generation purge: delete the dead memory-path, migration, and compat-stub generations #560).
register_pending_base_shmcharged thefull registered prewarm total for the entire multi-minute write window
while
MemAvailablewas already dropping as pwrites landed — a ~19 GiBprewarm at 90% written was under-reporting
allocatable_mibby ~17 GiB.Fixed: the pending charge is now netted against the target file's own
st_blockson every sample, so it shrinks in step with the actualwrite. New regression test:
pending_charge_nets_against_bytes_already_written.set every tick regardless of
ram_snapshot.measured, contradicting thechecked "gauges not emitted" acceptance box for VZ/Process/non-Linux
hosts. Now gated on
measured.acceptance box was checked citing construction + grep, but no test
actually exercised the watch-channel mechanism. Added
ram_ledger_snapshot_round_trips_through_watch_channel.HOST_BASE_SHM_TMPFS_USED_MIBwas a duplicate. It read the samest_blocks-over-known-files walk as thebase_shmcategory gauge,instead of the tmpfs mount's actual
statfsusage — invisible to anunlinked-but-open file or stray subdir, exactly the case this gauge
exists to catch during an ENOSPC-class incident. Now a real
f_blocks − f_bfreestatfs read.parked_rss_bytesdiscarded.read_smaps_rollup_pss_rssmeasuresboth PSS and RSS for a parked sandbox, but only PSS was kept — the
Σpss/Σrsssharing-density signal (ADR 0046's own rule) could never beevaluated for parked residents once the parking ladder lands. The
issue's own spec omitted this field, but it was cheap to add now
(mirrors the existing non-parked pair) rather than re-touching this
struct, the FC split, and the read loop later. Added, populated, and
exposed via new gauge-only metrics.
Acceptance criteria
parked_pss_is_never_added_back(ram_ledger.rs) + coordinator placement fixture
parked_resident_on_one_host_never_gets_placed_on(two-host,12 GiB parked on one, a 24 GiB session lands on the other).
ledger_pending_charge_registers_and_settles+pending_base_shm_charge_lowers_allocatable+pending_charge_nets_against_bytes_already_written(added post-review — proves the charge nets against bytes actually written,
closing the transient double-charge bug); live end-to-end (real
prewarm write) is out of scope for a unit test (needs a real tmpfs +
chunk store), covered by the pending-registry contract instead.
host has measured at least once (gated on
measuredpost-review;see finding 3 above); fleet view renders the new columns
(
web/src/pages/Fleet.tsx).same
RamLedgerSnapshotvia the watch channel; grep confirms no/proc/meminforead outsideram_ledger.rs(the pre-existingone-time
resource.rs::read_total_memory_mibstartup capacity seedis a distinct, unrelated figure —
HostCapacityReport.total_mib,not
allocatable_mib/pressure — left untouched, out of scope).Now backed by an actual unit test,
ram_ledger_snapshot_round_trips_through_watch_channel(addedpost-review — publishes a snapshot on a
tokio::sync::watchchanneland asserts two independent readers observe byte-identical values),
not just construction + grep.
Σpss/Σrssgauges are unchanged as the only sharing signal. Parkedresidents'
Σpss/Σrssis now actually observable too(
parked_rss_bytes, added post-review — see finding 6 above).in-flight prewarm,
allocatable_mibis the same formula, justre-derived through the ledger;
placement_reservation_live_pg/queue_scanner_live_pg/admin_evac_live_pgall green againstreal Postgres (ran locally,
just db-up).measured=false,allocatable_mib=0,gauges genuinely not emitted (fixed post-review — the ledger gauge
block is now gated on
ram_snapshot.measured; previously this boxwas checked but the gauges were in fact emitted as a permanent-zero
series on every tick regardless of platform), matches the existing
unmeasured-host posture; cross-compiled clean (
nix develop -c cargo clippy --target aarch64-unknown-linux-musl -p engram-sandbox-firecracker -p engram-host-agent --all-targets, zerowarnings) since the smaps/statfs walks and the
parkedfield arecfg(target_os = "linux")-gated (macOS clippy can't see them, perrepo convention).
Deviations / notes
base_shm_pending_mibhas no PG column. The issue's PG section listsexactly 3 new columns (
util_base_shm_mib,util_parked_pss_mib,util_running_pss_mib) even though the wire section lists 4 newHostUtilizationfields. This is intentional, not a miss: the pendingcharge is transient host-local state, already folded into the persisted
allocatable_mib, and visible directly via the host's own/metrics—documented in the migration and in
row.rs.f660225 to 42ed9bb) but every claim held; no anchor was substantively
wrong.
HostViewdoescarry a utilization message (
fleet.proto), extended per the issue'scontingency.
engram-host-agentchunk-cache write-throughtest failures (
disk_daemon::backend,pooled_backend) — filed asRegression on main: chunk write-through cache not populated (3 test failures post-#522) #552, reproduce on unmodified
main, not touched by this change.collision with sibling in-flight PRs in the same land-queue batch (see
Post-review fixups, finding 1).
just checkfmt clean, clippy clean (
-D warnings, full workspace + Linux/muslcross-compile for the touched Linux-only-gated crates),
cargo hakari verifyclean,cargo nextest run --workspace --no-fail-fastgreen — thepre-existing #552 chunk-cache failures are unrelated to this change
(different files, reproduce identically on unmodified
main).Landing order: #541 -> #536 -> #533 -> #527p1 -> #528 -> #530 -> #537 -> #540 -> #539 -> #526 -> #531 -> #538 -> #529 -> #535
🤖 Generated with Claude Code