Skip to content

Hard host disk budget for the chunk cache: periodic enforcement, single evictor, pin-aware arithmetic, dedicated volume - #557

Merged
nikhilunni merged 11 commits into
mainfrom
feat/chunk-cache-disk-budget
Jul 6, 2026
Merged

nikhilunni merged 11 commits into
mainfrom
feat/chunk-cache-disk-budget

Conversation

@nikhilunni

@nikhilunni nikhilunni commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes #528

Summary

Implements ADR 0067 (docs/adr/0067-chunk-cache-disk-budget.md): a disk-derived absolute chunk-cache budget by default, periodic enforcement independent of populate traffic, a single evictor per host (the UFFD handler's second eviction policy is deleted), pin-aware budget arithmetic with an overflow alarm, a kubelet-eviction-headroom gauge, and a chart-level dedicated-volume option with a host-agent boot gate.

Commits (one logical change each):

  1. f2968bb0 — engram-chunk-store: disk-derived default budget (min(60% of disk, 80%), ~179 GB on the 298.1 GB prod disk) + pin accounting (engram_chunk_cache_{pinned,budget}_bytes, engram_chunk_cache_pins_over_budget) + ChunkCache::spawn_sweeper + ChunkCacheConfig::eviction_enabled.
  2. 0ac3f55e — engram-uffd-handler: single evictor — deletes --cache-budget-bytes/DEFAULT_CACHE_BUDGET_BYTES outright (no compat shim), builds its cache with eviction_enabled: false.
  3. 2cd69096 — engram-host-agent: spawns the periodic sweeper; emits engram_host_disk_headroom_to_kubelet_bytes and engram_host_base_memfile_bytes.
  4. b7ba7f96 — engram-host-agent: ENGRAM_WORK_DIR_REQUIRE_MOUNTPOINT boot gate (st_dev comparison vs /).
  5. 21f01b2e — deploy: helm chart wiring (storage.{chunkCacheDiskFraction,kubeletEvictPct,dedicatedDevice}, node-prep format+mount step).
  6. ca3ea1d7 — docs: ADR 0067 (Accepted, with the commit chain and two noted divergences).
  7. 45ad6a00 — docs: fixes a README table cell that was already stale before this change.

Acceptance criteria

  • Budget enforced without traffic (spawn_sweeper_enforces_with_zero_populate_traffic). Update (post-review): the original sweeper enforced starting at t=0 (tokio::time::interval's immediate first tick), which raced the image-prefetch supervisor's pin-set reconcile on a cold start and would have evicted boot-staged base chunks pin-blind on the very first rollout (review finding 2, [CONFIRMED]). Fixed — the sweeper now consumes the immediate tick and enforces starting at t=interval instead; new regression test spawn_sweeper_does_not_sweep_at_t_zero.
  • Default derivation formula table incl. the 298.1 GB → ~179 GB prod case (default_budget_bytes_prod_298gb_case + 4 more formula-table tests)
  • Pins are a floor (never unlinked; unpinned still evicts) — pins_over_budget_never_unlinks_pinned_but_still_evicts_unpinned. Caveat: I verify the behavior the engram_chunk_cache_pins_over_budget/_pinned_bytes gauges report, not the gauge values themselves via readback — the metrics crate's Gauge type has no get() accessor and this repo has no existing test-scoped-recorder pattern. Noted as a divergence in the ADR.
  • Single evictor (eviction_disabled_cache_never_unlinks_under_pressure + uffd-handler from_blob_built_cache_has_eviction_disabled constructor test)
  • Metrics present: engram_chunk_cache_{size,fs_free,pinned,budget}_bytes, engram_chunk_cache_pins_over_budget, engram_host_disk_headroom_to_kubelet_bytes, engram_host_base_memfile_bytes — all wired and gauge-emitting in code; not verified via a live /metrics scrape in this PR (that's the issue's own step 6, post-merge prod rollout).
  • Mountpoint gate (require_work_dir_mountpoint_or_exit, 4 unit tests). Update (post-review): the originally-shipped gate compared work_dir against the host-agent container's OWN / — always the pod's image overlayfs, always a distinct device from any hostPath mount — so in the exact K8s DaemonSet this was built for, it was vacuous: it returned Ok unconditionally regardless of whether storage.dedicatedDevice's mount had actually landed (review finding 1, [CONFIRMED]). Fixed — new ENGRAM_HOST_ROOT_REF_PATH env var (default /, bare-metal unaffected) that the chart now points at a read-only hostPath mount of the NODE's / (/mnt/host-root), added to the DaemonSet only when storage.dedicatedDevice is set. Also fixed the gate's unit tests, which implicitly assumed the test tempdir shares /'s filesystem — true on ubuntu CI runners / macOS APFS firmlinks, false on any Linux box with /tmp on tmpfs (review finding 3, [PLAUSIBLE], confirmed real) — they now construct both sides of the comparison explicitly.
  • needs-prod-verification: ENGRAM_KUBELET_EVICT_PCT default of 10. Per explicit instruction for this issue, this ships as an env-configurable default with a documented TODO (in util.rs and values.yaml) rather than a guessed-and-asserted-verified number — it has not been cross-checked against the actual nodepool/kubelet flags in the engrams-internal deploy repo (which this agent has no access to). GKE's documented nodefs.available < 10% is consistent with the 611v incident's own arithmetic, but a cluster can override it via --eviction-hard/--system-reserved.
  • needs-prod-verification: the 14-day post-deploy prod deltas (zero kubelet ephemeral-storage evictions, write_local ENOENT class → 0, node disk ≤ ~80%) — explicitly out of scope for this PR per the issue's own plan (step 6, post-merge, engrams-prod-ops).
  • N/A: prod rollout + verification, engrams-internal nodepool device provisioning for storage.dedicatedDevice — out of scope for this OSS repo/PR.

Post-review fixup commits (deep review pass before undrafting)

A deep review (gh api repos/cortexapps/engrams/pulls/557/reviews) found 4 findings; all addressed. See the PR comment with the finding→commit checklist for details.

Stale anchors found (issue verified against f6602259; branch HEAD was 42ed9bb2, 4 commits later)

All file:line anchors the issue cited were re-verified against 42ed9bb2 and still held — no drift found. The 298.1 GB prod-disk example, the NO_CEILING/DEFAULT_FREE_FLOOR_PCT constants, the UFFD handler's DEFAULT_CACHE_BUDGET_BYTES/--cache-budget-bytes surface, and the FC backend's spawn_uffd_handler (confirmed passing no budget flag) all matched exactly as described.

Deviations from the issue text

  • Commit slicing: the issue suggested 5 slices (chunk-store budget/pins → sweeper → uffd-handler → host-agent gauges → helm). I merged "budget/pins" and "sweeper" into one chunk-store commit (f2968bb0) because ChunkCacheConfig::eviction_enabled and the budget-doc updates touch the exact same struct-literal call sites across the crate — splitting would have meant editing the same lines twice for no independent-revert value. Still one logical unit ("chunk-store's full budget mechanics"); every other slice matches the suggestion.
  • headroom_frac in default_budget_bytes: the issue's pure-function sketch left headroom_frac's source unspecified. I resolve it as the same resolved free-space-floor fraction (resolve_free_floor_pct) rather than a second independent knob — one fewer number to keep in sync with the floor's own kubelet-line rationale. Recorded in the ADR.
  • Gauge-value test assertions: pin-overflow/no-pin-overflow tests assert behavior (pinned survives, unpinned evicts), not the literal gauge float via a metrics-recorder readback (no existing pattern in this codebase for that; metrics::Gauge has no get()). Recorded in the ADR.
  • Added ChunkCache::eviction_enabled() (diagnostic accessor, mirrors is_pinned/pinned_count) — not explicitly requested but needed to unit-test the uffd-handler's single-evictor constructor invariant without reaching into private fields cross-crate.
  • Fixed a README.md table cell (storage-substrate table) that claimed the old-and-already-wrong "default 200 GiB" ceiling — stale before this PR, now doubly stale after it; fixed in the same change since it's the exact topic.

Pre-existing, unrelated test failures (confirmed via baseline)

3 engram-host-agent tests fail identically on pristine main @ 42ed9bb (verified via a throwaway git worktree add --detach at that commit, cargo nextest run -p engram-host-agent --lib --no-fail-fast, before any of this PR's changes existed):

  • disk_daemon::backend::tests::flush_write_throughs_chunks_into_local_cache
  • pooled_backend::tests::migration_fetch_rejects_unlisted_hash_and_bad_export_id
  • pooled_backend::tests::materialize_chunked_rootfs_uses_chunk_cache_when_present

All three are blob not found / write-through failures unrelated to ChunkCacheConfig, evict_to_budget, or anything else this PR touches — environment-specific on this dev machine, not a regression. cargo nextest run --workspace --no-fail-fast on this branch: 1367/1370 passed, exactly these 3 failed, 99 skipped — identical failure set, count-for-count, before and after this PR's changes. Not fixed here (out of scope for #528); flagging for a follow-up.

just check state

  • cargo fmt --all -- --check: clean.
  • cargo clippy --workspace --all-targets -- -D warnings: clean.
  • cargo hakari verify: clean.
  • cargo nextest run --workspace: fails only on the 3 pre-existing failures above (fail-fast stops there; --no-fail-fast confirms nothing else regresses — see above).
  • Linux cross-check (nix develop -c cargo clippy --target aarch64-unknown-linux-musl -p engram-uffd-handler -p engram-sandbox-firecracker -p engram-host-agent --all-targets -- -D warnings): clean. Caught and fixed one real bug this way — image_prefetch.rs's new memfile-bytes gauge loop originally held a HashMap::values() borrow across an .await, which is invisible to macOS clippy (no UFFD/MemfilePin there) but a hard Send-future compile error on Linux (MemfilePin wraps a non-Sync raw pointer). Fixed by collecting owned paths before the loop.
  • Deploy YAML: helm template both value shapes (storage.dedicatedDevice set/unset) + yaml.safe_load-validated the output — clean, correct nsenter-step ordering confirmed (dedicated-volume mount before the base-shm tmpfs mount).
  • Did not run just vz-test (macOS VZ codesigned tests) — the sweeper/gauges live entirely above the SandboxBackend seam in host-agent's main.rs (shared construction path for FC/VZ/Process), so there's no VZ-specific code path this PR adds; flagging in case a reviewer wants it run explicitly.
  • No new migration/SQL — confirmed, .sqlx/ untouched.
  • No new FC/NBD test — confirmed, .github/workflows/ci.yml untouched (none needed per the issue's own Testing & CI section).

Landing order: #541 -> #536 -> #533 -> #527p1 -> #528 -> #530 -> #537 -> #540 -> #539 -> #526 -> #531 -> #538 -> #529 -> #535. Note: #538 (enable-fleet-prewarm) stacks on this branch and will be rebased onto it once this PR is reviewed.

nikhilunni and others added 7 commits July 1, 2026 22:11
… periodic sweeper (ADR 0067)

Closes the structural half of the chunk-cache disk-exhaustion loop
(issue #528): the cache had eviction machinery, but no default byte
ceiling (NO_CEILING), no enforcement independent of populate traffic,
and no accounting for what pins make unevictable.

- ChunkCacheConfig::from_env_or_default now derives a real absolute
  ceiling from the disk backing `root` by default:
  min(fs_total * ENGRAM_CHUNK_CACHE_DISK_FRACTION [0.60],
      fs_total * (1 - free_floor_frac)) — ~179 GB on the 298.1 GB prod
  disk that grew to an unbounded 182.2 GB. ENGRAM_CHUNK_CACHE_BUDGET_BYTES
  still wins outright as an operator override; a filesystem-probe
  failure still fails soft to NO_CEILING.
- evict_to_budget now computes pinned_bytes every sweep and emits
  engram_chunk_cache_{pinned,budget}_bytes plus
  engram_chunk_cache_pins_over_budget (+ a rate-limited-by-sweep-
  interval error log) when pins alone exceed the budget. Pins are a
  floor, never auto-released under pressure.
- ChunkCacheConfig::eviction_enabled (default true) lets a cache
  populate without ever evicting — the single-evictor primitive the
  UFFD handler needs (wired in the next commit).
- ChunkCache::spawn_sweeper(interval) runs evict_to_budget on a timer
  independent of populate traffic (ENGRAM_CHUNK_CACHE_SWEEP_INTERVAL_SECS,
  default 60s, 0 disables) — closes the "host under disk pressure with
  no writes enforces nothing" gap.

Every existing ChunkCacheConfig struct-literal site (tests across
engram-chunk-store, engram-sandbox-firecracker, engram-host-agent)
picks up the new eviction_enabled field.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
… cache (ADR 0067)

The UFFD handler ran a second, pin-blind LRU eviction policy over the
same cache_root the host-agent pins into: a stale fixed 200 GiB
--cache-budget-bytes default with zero pins. Under disk pressure this
sweep preferentially evicted the exact pinned base-image chunks the
host-agent was protecting (eviction is oldest-populate-first, and
host-boot-staged base chunks are the oldest).

Clean break: delete DEFAULT_CACHE_BUDGET_BYTES, the --cache-budget-bytes
flag, and its plumbing through ChunkedMemoryBackend::from_blob(_with_
session_json) — zero external users, no compat shim. The handler now
builds its ChunkCache with eviction_enabled: false (ADR 0067, previous
commit): it still populates (write-through of faulted chunks stays,
that's the locality win) but never unlinks. The host-agent, which holds
the pin set, is the one process per host that evicts.

Adds ChunkCache::eviction_enabled() (diagnostic accessor, mirrors
is_pinned/pinned_count) and a constructor test asserting every
from_blob-built cache has eviction disabled.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
…se-memfile gauges (ADR 0067)

Host-agent side of the disk-budget invariant:

- main.rs spawns chunk_cache.spawn_sweeper() right after cache
  construction (ENGRAM_CHUNK_CACHE_SWEEP_INTERVAL_SECS, default 60s),
  held for the process lifetime like the existing _base_shm_gc handle —
  this is the ONE eviction policy per host now that the UFFD handler's
  is disabled (previous commit).
- util.rs: UtilizationProbe now resolves ENGRAM_KUBELET_EVICT_PCT once
  at construction and emits engram_host_disk_headroom_to_kubelet_bytes
  (fs_free - fs_total * kubelet_evict_frac) every heartbeat tick,
  sharing the existing statvfs probe. This alarms on TOTAL disk
  pressure (snapshots, memfiles, OCI cache, anything on the mount), not
  just the chunk cache's slice — it's meant to fire before the kubelet
  acts, since the eviction itself is the churn amplifier (orphaned
  local cache -> cold GCS resume path).

  ENGRAM_KUBELET_EVICT_PCT defaults to 10 (GKE's documented
  nodefs.available < 10% and consistent with the 611v incident's
  arithmetic), but per explicit instruction for this issue this default
  is a PLACEHOLDER — it has NOT been cross-checked against the actual
  nodepool/kubelet config in the engrams-internal deploy repo. See the
  TODO on KUBELET_EVICT_PCT_ENV_VAR.
- image_prefetch.rs: the reconcile tick now gauges
  engram_host_base_memfile_bytes (summed on-disk size of every tracked
  base memfile) — unevictable disk in the same "floor the budget can't
  touch" category as pinned chunk bytes. Collects owned paths before
  awaiting metadata() — MemfileState::pin holds a raw, non-Sync pointer,
  so borrowing the map across an await breaks Send on the Linux target
  (caught via the required aarch64-unknown-linux-musl clippy cross-check;
  invisible to macOS clippy since MemfilePin never materializes there).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
Boot-time guard for the chart's paired storage.dedicatedDevice +
ENGRAM_WORK_DIR_REQUIRE_MOUNTPOINT=true knob: when set, refuse to
start unless work_dir resolves to a distinct filesystem from / (st_dev
comparison, walking up to the nearest existing ancestor since work_dir
may not exist yet on a fresh host). Runs before anything touches
work_dir or the coordinator registration.

Guards the base-shm-startup-race failure class: a rolled pod starting
before node-prep's dedicated-volume mount is visible would otherwise
silently write the chunk cache/snapshots/memfiles onto the boot disk,
defeating the whole point of the dedicated volume (moving that load
off the kubelet's nodefs signal). No-op when the env var is unset —
zero behavior change until a chart opts in (helm wiring in a follow-up
commit).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
…ed volume (ADR 0067)

engram-host-fleet chart changes (component 4 of the disk-budget issue):

- values.yaml: storage.chunkCacheDiskFraction (overrides the 0.60
  default fraction), storage.kubeletEvictPct (default 10 — flagged
  needs-prod-verification, see the TODO), storage.dedicatedDevice
  ("" = today's hostPath-on-boot-disk behavior).
- configmap.yaml: emits ENGRAM_CHUNK_CACHE_DISK_FRACTION when set,
  ENGRAM_KUBELET_EVICT_PCT always (has a sane default), and
  ENGRAM_WORK_DIR_REQUIRE_MOUNTPOINT=true when dedicatedDevice is set
  (picked up automatically by host-agent.daemonset.yaml's existing
  envFrom: configMapRef — no per-var daemonset wiring needed).
- node-prep.daemonset.yaml: idempotent format (blkid-gated, never
  reformats a device with an existing filesystem) + mount step for
  dedicatedDevice, in the host mount namespace via the same nsenter
  pattern as the base-shm tmpfs step. Ordered BEFORE that tmpfs step so,
  when both are set, the tmpfs mounts inside the dedicated volume's tree
  (uffdBaseDir normally lives under workDirHostPath).

Rendered + yaml.safe_load-validated both value shapes (dedicatedDevice
set/unset) — deploy YAML is outside `just check`.

Updates the stale storage.chunkCacheBudgetBytes comment ("host-agent
default (200 GiB)") to describe the new disk-derived default.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
Bookend for the disk-derived budget + pin arithmetic + single evictor +
dedicated-volume work (issue #528, commits f2968bb/0ac3f55e/2cd69096/
b7ba7f9/21f01b2e). Status Accepted with the commit chain; records the
budget formula, the pins-as-floor decision, the single-evictor
relationship to epic-substrate-single-writer, the accounting-vs-bytes
framing of the dedicated volume, the ENGRAM_KUBELET_EVICT_PCT
needs-prod-verification caveat, and two divergences from the issue's
sketch (headroom_frac reuses the floor's resolved fraction rather than
a second knob; gauge-value assertions are behavioral, not
metrics-recorder-based).

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
The storage-substrate table said the cache was "bounded by
ENGRAM_CHUNK_CACHE_BUDGET_BYTES (default 200 GiB)" — already wrong
before this change (the actual default was NO_CEILING; 200 GiB was the
UFFD handler's separate, now-deleted default) and definitely wrong
after it (the default is now disk-derived, ADR 0067). Fixed while
touching this exact topic.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc

@nikhilunni nikhilunni left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review of PR #557 against issue #528 / ADR 0067. Overall: a faithful, well-tested implementation of all four components (disk-derived budget + pin arithmetic, periodic sweeper, single evictor with the UFFD flag surface deleted outright, headroom gauge + helm dedicated-volume wiring), with honest disclosure of deviations. Verdict: solid — but the ENGRAM_WORK_DIR_REQUIRE_MOUNTPOINT boot gate is vacuous in the exact K8s deployment it was built for (finding 1) and the sweeper's immediate first tick opens a cold-boot pin race that will fire fleet-wide on the very first rollout (finding 2). Both are worth fixing before undrafting.

CI

Green. All lanes pass (CI Gate pass; buf/orchestrator/web/e2e-bake lanes path-skipped as expected for a Rust+helm change).

Issue compliance

Strong. All four components landed as specified: (1) default_budget_bytes pure function + ENGRAM_CHUNK_CACHE_DISK_FRACTION knob + explicit-override precedence + NO_CEILING fail-soft + pinned/budget/pins-over-budget gauges and the error! alarm in evict_to_budget; (2) spawn_sweeper with ENGRAM_CHUNK_CACHE_SWEEP_INTERVAL_SECS (default 60, 0 disables), held-handle pattern in host-agent main; (3) eviction_enabled flag, UFFD handler builds eviction-disabled caches, DEFAULT_CACHE_BUDGET_BYTES/--cache-budget-bytes/plumbing deleted per the retire list with no compat shim; (4) kubelet-headroom gauge in the utilization sampler (env resolved once at construction), HOST_BASE_MEMFILE_BYTES gauge-only per the do-not-touch, helm storage.{dedicatedDevice,kubeletEvictPct,chunkCacheDiskFraction}, node-prep idempotent format+mount ordered before base-shm. ADR 0067 authored with the 0060-collision note, divergences recorded, flipped Accepted with the commit chain. The kubelet-pct default is left as a documented needs-prod-verification TODO per the issue's own unverified list; the gauge-value readback deviation is disclosed and reasonable. New unit tests run in existing nextest lanes (verified: the Linux lane covers them; the uffd-handler test compiles under the Linux target); no ci.yml change needed, correctly.

One acceptance criterion does NOT hold as stated in the shipped deployment shape: "with work_dir on the root filesystem the host-agent exits nonzero" is only true un-containerized. In the K8s DaemonSet the check is structurally vacuous (finding 1) — the unit tests validate bare-metal semantics and mask the gap. The coordinator's mirror of the stale 200 GiB comment also survived the doc sweep (finding 4).

Findings

  1. crates/engram-host-agent/src/main.rs:754 [CONFIRMED] — the mountpoint gate compares work_dir against the container's /, which is the pod's overlayfs; the two devices ALWAYS differ, so the gate is vacuous in the K8s DaemonSet it was built for. The host-agent runs as an ordinary (privileged) container: / is the image overlayfs, and work_dir (/var/lib/engram) is a hostPath volume whose st_dev is the backing filesystem's — boot-disk ext4 or the dedicated device, either way never the overlayfs device. Failure scenario: storage.dedicatedDevice is set, node-prep's mount step fails (its failure is echo-swallowed in node-prep.daemonset.yaml with the message "host-agent will refuse to start" — now false) or a rolled pod starts before the mount lands; require_work_dir_mountpoint_or_exit returns Ok and the host-agent comes up silently shadow-writing the boot disk — the base-shm-startup-race failure class ADR 0067 says this gate blocks. Suggested fix: a container-aware check — e.g. have node-prep drop a sentinel file on the mounted filesystem that the gate requires to exist, or compare work_dir's st_dev against a known boot-disk reference path mounted into the pod. The unit tests only exercise the non-containerized semantics; rework them alongside.

  2. crates/engram-host-agent/src/main.rs:479 [CONFIRMED] — the sweeper's first tick fires immediately at boot, before the image-prefetch supervisor re-establishes the pin set, so a restarted host-agent with an over-budget cache evicts exactly the boot-staged base-image chunks that were pinned in the prior life. tokio::time::interval's first tick completes immediately; spawn_sweeper is called at main.rs:479, well before image_prefetch::spawn_supervisor (lib.rs:998, on the serve/registration path, and its reconcile needs coordinator RPC before it can pin). Pins are in-memory only, so the t=0 sweep runs pin-blind; eviction is oldest-mtime-first, and the oldest chunks are the boot-staged base manifests. This is not just theoretical: on the FIRST rollout of this PR, prod caches sit at ~182 GB against the ~179 GB derived budget, so every host will trip this on upgrade — evicting ~3 GB of base chunks, flipping readiness, and re-fetching from GCS: the same hazard class the single-evictor change closes. Suggested fix: consume one tick before the loop (or sleep one interval first), or better, spawn the sweeper after the first prefetch reconcile completes; the populate-path debounced sweep still bounds growth in the meantime.

  3. crates/engram-host-agent/src/main.rs:939 [PLAUSIBLE] — mountpoint_gate_rejects_same_filesystem_as_root_when_required (and the walk-up/case-insensitivity tests) assume the test tempdir shares /'s filesystem — a host-layout assumption, not a property of the code under test. Green today: ubuntu-2404 runners keep /tmp on the root fs, and on macOS the APFS firmlink illusion gives / and /var/folders the same st_dev (verified locally). But any Linux box with /tmp on tmpfs (Fedora/Arch default; nextest honors TMPDIR) sees different devices, the gate correctly returns Ok, and assert!(result.is_err()) fails red for a non-bug. Confirmable by running with TMPDIR=/dev/shm/.... Since finding 1 forces a rework of the gate's mechanism anyway, make the rewritten tests construct both sides of the comparison explicitly (or stat-assert the premise and skip).

  4. crates/engram-coordinator/src/main.rs:690 [CONFIRMED] — stale mirror comment: "Budget defaults to 200 GiB; smaller hosts ... override via ENGRAM_CHUNK_CACHE_BUDGET_BYTES". (Not in this diff, so no inline anchor.) The PR fixed the equivalent stale claim in README.md but missed this call site, whose from_env_or_default now (a) derives min(60%, 80%) of the disk and (b) create_dir_alls the cache root as a side effect. Repo convention is no dangling references to old behavior; also worth a line in the PR body noting the coordinator cache silently gained the disk-derived budget.

Automated deep review (core-ops batch); findings verified against the branch — treat PLAUSIBLE items as questions.

}
};

if work_dev == root_dev {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONFIRMED] Gate is vacuous in the K8s DaemonSet. This compares work_dir's st_dev against the container's / — the pod's image overlayfs. work_dir is a hostPath mount (boot-disk ext4 or the dedicated device), so the two devices ALWAYS differ and the gate passes even when the dedicated volume never mounted. Node-prep's mount failure is echo-swallowed with "host-agent will refuse to start" — which this makes false: the pod comes up silently shadow-writing the boot disk, the exact base-shm-startup-race class ADR 0067 says this blocks. Fix: container-aware check — e.g. node-prep drops a sentinel file on the mounted fs that the gate requires, or compare against a boot-disk reference path mounted into the pod.

// UFFD handler shares this directory but builds its own cache with
// eviction disabled; see engram-uffd-handler). Held for the process
// lifetime, same pattern as `_base_shm_gc` below.
let _chunk_cache_sweeper = chunk_cache.spawn_sweeper(std::time::Duration::from_secs(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CONFIRMED] First tick fires immediately, before the pin set exists. tokio::time::interval's first tick completes at t=0, and this is spawned well before image_prefetch::spawn_supervisor (lib.rs:998) can reconcile + pin (needs coordinator RPC). Pins are in-memory only, so a restarted host-agent with an over-budget cache sweeps pin-blind and unlinks the oldest-mtime chunks — precisely the boot-staged base-image chunks pinned in the prior life. On the FIRST rollout of this PR prod caches sit at ~182 GB vs the ~179 GB derived budget, so every host trips this on upgrade: readiness flap + GCS re-stage. Fix: consume one tick before the loop / sleep one interval first, or spawn the sweeper after the initial prefetch reconcile.

Comment thread crates/engram-host-agent/src/main.rs Outdated
let _g = env_guard();
std::env::set_var(WORK_DIR_REQUIRE_MOUNTPOINT_ENV_VAR, "true");
let tmp = tempfile::tempdir().unwrap();
let result = require_work_dir_mountpoint_or_exit(tmp.path());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[PLAUSIBLE] Host-layout assumption in the test. This asserts the tempdir shares /'s filesystem — true on ubuntu runners (/tmp on rootfs) and on macOS (APFS firmlinks give / and /var/folders the same st_dev; verified), but false on any Linux box with /tmp on tmpfs (Fedora/Arch default; nextest honors TMPDIR): the gate correctly returns Ok and assert!(result.is_err()) fails red for a non-bug. Since the gate mechanism needs a rework anyway (see the st_dev-vs-container-root comment above), make the new tests construct both sides of the comparison explicitly, or stat-assert the premise and skip.

nikhilunni and others added 4 commits July 3, 2026 13:32
…ng 2)

tokio::time::interval's first tick fires immediately, but at t=0 a
freshly-started host-agent hasn't re-established its in-memory pin set
yet (image_prefetch's reconcile needs a coordinator RPC round trip
after registration). An immediate sweep on a restarted host that's
already over budget ran pin-blind and evicted the oldest-mtime
chunks — exactly the boot-staged base-image chunks pinned in the prior
life — flapping readiness and re-fetching from GCS. Confirmed against
prod: caches currently sit ~182 GB against the ~179 GB derived budget,
so every host would trip this on the first rollout of ADR 0067.

spawn_sweeper now consumes the interval's immediate tick before
entering the loop, so the first real sweep lands at t=interval instead
of t=0. The populate-path debounced sweep still bounds growth from
writes in the meantime. Adds a regression test
(spawn_sweeper_does_not_sweep_at_t_zero) asserting the over-budget
chunk survives past t=0 but is evicted once the first interval elapses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
… disk, not the container's own overlayfs (PR #557 review findings 1, 3)

The gate compared work_dir's st_dev against the host-agent
container's OWN `/`, which is always the pod's ephemeral image
overlayfs — a distinct device from EVERY hostPath mount by
construction. So in the K8s DaemonSet this was built for, the gate
reported "distinct filesystem" (and returned Ok) unconditionally,
whether or not storage.dedicatedDevice's mount had actually landed.
Node-prep's mount failure is only echo-swallowed with "host-agent will
refuse to start" — which was false: the pod could come up silently
shadow-writing the boot disk, the exact base-shm-startup-race class
this gate exists to block.

Introduces ENGRAM_HOST_ROOT_REF_PATH: the boot-disk reference path the
gate compares work_dir against, defaulting to `/` (unaffected on bare
metal / dev / VZ). The chart now sets it to a read-only hostPath mount
of the NODE's `/` at /mnt/host-root, added to the host-agent DaemonSet
only when storage.dedicatedDevice is configured — so the comparison is
boot-disk-vs-work_dir, not overlayfs-vs-work_dir.

Also fixes finding 3: the gate's unit tests implicitly assumed the
test tempdir shares `/`'s filesystem — true on ubuntu CI runners and
macOS's APFS firmlinks, false on any Linux box with /tmp on tmpfs
(e.g. Fedora/Arch defaults; nextest honors TMPDIR), which would make
the "rejects" assertions fail red for a non-bug. The tests now
construct both sides of the comparison explicitly under the same
tempdir root via ENGRAM_HOST_ROOT_REF_PATH, deterministic regardless
of host layout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
…ew finding 4)

The --mode=all wiring comment still claimed "Budget defaults to 200
GiB; smaller hosts ... override via ENGRAM_CHUNK_CACHE_BUDGET_BYTES" —
the PR fixed the equivalent stale claim in README.md but missed this
call site. from_env_or_default now derives a disk-sized default
(min(60% of the disk, 80%)) and, as a side effect, create_dir_all's
the cache root to probe disk size. Updated to describe the current
behavior and both override knobs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
…gs 1, 2, 3, 4)

Updates the periodic-enforcement and dedicated-volume sections to
describe the shipped mechanics (skip-first-tick sweeper,
ENGRAM_HOST_ROOT_REF_PATH-based mountpoint gate) and adds a
"Post-review fixes" section summarizing the two real bugs a deep
review pass found before undrafting plus the stale coordinator
comment, so the ADR stays the accurate record of what actually
shipped.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4WkZddtVw8djsWz2RCcQc
@nikhilunni

Copy link
Copy Markdown
Contributor Author

Deep-review fixup pass

Addressed every finding from the deep review (gh api repos/cortexapps/engrams/pulls/557/reviews, review id 4618556166, + its 3 inline comments). All 4 were real; none disputed.

# Severity File:line Finding Commit
1 CONFIRMED crates/engram-host-agent/src/main.rs:754 Mountpoint gate compared work_dir against the container's own / (the pod's overlayfs) — always a distinct device from any hostPath mount, so the gate was vacuous in the K8s DaemonSet it was built for 5ca872cd
2 CONFIRMED crates/engram-host-agent/src/main.rs:479 Sweeper's first tick fired at t=0, before the image-prefetch supervisor could re-establish the pin set — a cold-started, over-budget host would evict boot-staged base chunks pin-blind on the very first rollout d4f7aef1
3 PLAUSIBLE crates/engram-host-agent/src/main.rs:939 Gate's unit tests implicitly assumed the test tempdir shares /'s filesystem — true on ubuntu CI/macOS, false on any Linux box with /tmp on tmpfs. Confirmed real; fixed alongside finding 1's mechanism rework 5ca872cd
4 CONFIRMED crates/engram-coordinator/src/main.rs:690 Stale "budget defaults to 200 GiB" comment on the coordinator's --mode=all chunk-cache wiring, missed by the README fix already in this PR 87308e88

ADR 0067 updated to describe the shipped (fixed) mechanics + a "Post-review fixes" section: 7f25a315.

Fix summaries

  • Finding 1 (+3): introduced ENGRAM_HOST_ROOT_REF_PATH — the boot-disk reference path the gate compares work_dir against, defaulting to / (bare-metal unaffected). The chart now points it at a read-only hostPath mount of the node's / (/mnt/host-root), added to the host-agent DaemonSet only when storage.dedicatedDevice is set. Reworked the gate's unit tests to construct both sides of the comparison explicitly under a shared tempdir root instead of relying on the real /.
  • Finding 2: ChunkCache::spawn_sweeper now consumes the interval's immediate t=0 tick before entering the loop, so the first real sweep lands at t=interval. Added regression test spawn_sweeper_does_not_sweep_at_t_zero.
  • Finding 4: updated the coordinator's --mode=all comment to describe the disk-derived default + create_dir_all side effect.

Validation

  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings, cargo hakari verify: all clean.
  • cargo nextest run --workspace: same 3 pre-existing, already-documented-in-this-PR failures as baseline main (disk_daemon::backend::tests::flush_write_throughs_chunks_into_local_cache, pooled_backend::tests::migration_fetch_rejects_unlisted_hash_and_bad_export_id, pooled_backend::tests::materialize_chunked_rootfs_uses_chunk_cache_when_present) — nothing new broke; new/changed tests (spawn_sweeper_does_not_sweep_at_t_zero, the 3 reworked mountpoint-gate tests) all pass.
  • Linux cross-check: nix develop -c cargo clippy --target aarch64-unknown-linux-musl -p engram-host-agent -p engram-chunk-store -p engram-coordinator --all-targets -- -D warnings — clean.
  • helm template both storage.dedicatedDevice shapes (set/unset) + yaml.safe_load-validated — the new host-root volume/mount and ENGRAM_HOST_ROOT_REF_PATH only render when dedicatedDevice is set; default (unset) render is byte-identical in that regard (no new keys leak in).

PR body's acceptance-criteria checklist updated in place to record findings 1/2 against their items (still checked — now actually true, not vacuously true).

@nikhilunni
nikhilunni marked this pull request as ready for review July 6, 2026 04:32
@nikhilunni
nikhilunni merged commit 7cc144c into main Jul 6, 2026
17 checks passed
@nikhilunni
nikhilunni deleted the feat/chunk-cache-disk-budget branch July 6, 2026 04:33
nikhilunni added a commit that referenced this pull request Jul 6, 2026
… 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
nikhilunni added a commit that referenced this pull request Jul 6, 2026
–#566) (#590)

* fix(ci): stop test-e2e-stack from force-skipping on Rust-only changes

needs: included web/lint/test-linux/cross-musl-linux purely as a
cost-saving DAG dep, but GitHub Actions skips a job whenever ANY needed
job was skipped — overriding this job's own if:. Those jobs each gate
on their own path flags (test_web, test_rust, test_cross), so a
Rust-only PR that doesn't touch web/host-binary paths skipped one of
them and silently force-skipped the entire e2e-stack lane, even though
its own path-gate (needs.detect.outputs.e2e) said it should run.

Trim needs: to just the jobs whose artifacts this lane actually
downloads (detect's e2e flag, artifacts-e2e-stack's release binaries,
bake-harness-claude-artifact's harness tree), add !cancelled() so a
skip among them no longer force-skips this job, and check each
remaining need's .result explicitly so this job doesn't attempt to run
against artifacts that never got produced. The dropped lanes
(lint/test-linux/web/cross-musl-linux) are still independently
required via ci-gate's own needs:, so overall gate strength is
unchanged.

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

* fix(coordinator): placement exclusion visibility on all NoCapacity paths

exclusion_summary + PLACEMENT_EXCLUDED_TOTAL were only emitted from
pick_for_session (resume/evac) — the wire-skew incident that motivated
this actually broke the queue-scanner's create path, which had zero
visibility into why every host was excluded. api/sessions.rs's create
path and queue_scanner.rs's resume precheck were silent too.

Add placement::log_empty_candidates(meta, ctx, origin) as the one
shared helper every empty-candidate-set call site invokes; it emits
the same exclusion_summary-driven counter + warn as before, now
labeled with origin ∈ {create, queue_create, queue_resume_precheck,
resume} so each path is distinguishable. Reword ADR 0068 to describe
the corrected coverage instead of claiming pick_for_session alone
covered the incident.

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

* fix(host-agent,postgres,coordinator): correction-pass items C-G

Four independent review-fix findings from the core-ops batch:

- eviction_finalize.rs::publish_disk_manifest: mirror run_memory_leg's
  idempotent VersionConflict arm. The first attempt always targets the
  same deterministic version derived from base_manifest, so a conflict
  on THAT SPECIFIC attempt can only mean a prior crash-redrive already
  published this exact content — treat as success instead of bumping
  to latest+1 and republishing under a version no caller will ever
  reference. Later, non-deterministic bumps in the same retry loop keep
  the generic race-retry behavior.

- engram-postgres retry_enable_job: also NULL capture_phase/warm_stage/
  warm_stage_started_at/warm_stages/output_tail (migration 0079) so a
  retried capture doesn't have the UI rendering the previous attempt's
  stage/tail as if it were live. Extended the existing live-PG test
  (progress_state_failure_and_retry_round_trip) to stamp capture
  progress before failing the job and assert it's cleared after retry.

- pooled_backend.rs run_warm_hook: the terminal violation_failure send
  is the one CaptureProgress event that actually matters (failing
  stage + output tail) — warn on TrySendError::Full there instead of
  silently dropping it. Bump the production progress channel
  (grpc_server.rs build_base_snapshot) from 64 to 256 so a burst of
  routine progress/keepalive traffic is far less likely to crowd out
  that terminal frame in the first place.

- lib.rs: extract stages_images_gate(has_chunk_store, has_chunk_cache)
  as the one pure source of truth for whether the image-prefetch
  supervisor spawns (and thus whether the heartbeat's `stages_images`
  field is true) — a debug_assert ties the derived bool to the actual
  match's spawn condition so they can't silently drift apart. Unit
  test covers all 4 branches.

- session_boot.rs / metrics.rs: add COORD_BOOT_OVERLAP_SECONDS, timing
  the tokio::join! between the restore leg and the env/egress leg
  (which can round-trip an external mint-mode connector) — a window
  neither coord_prepare nor coord_finalize covers, so a slow env/egress
  leg was previously invisible to both.

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

* docs: fix ADR/comment drift batch (item H) + renumber 0067 chunk-cache ADR to 0070

Seven independent doc-accuracy fixes surfaced in the core-ops batch review:

- engram-sandbox-firecracker: restore_in_jail's leg_setup comment and
  net.rs's netns_name_for doc both described netns entry via `ip netns
  exec`; the actual mechanism (confirmed against spawn_firecracker) is
  a direct exec + pre_exec setns(2) closure. Reworded both to name it.

- ADR 0020: the P4 residual-spawns paragraph described the teardown
  veth-A delete as "conditional on the netns delete having failed" —
  that's the reverted pre-fixup design. The shipped code
  (net.rs::teardown_netns) does an unconditional, synchronous netlink
  delete of veth-A BEFORE allocator.free, precisely because netns
  teardown is asynchronous and a conditional gate would race
  allocator.free, reopening the double-bound-/30 hazard. Reworded to
  match.

- ADR 0034: the recovery-ladder comment named a separate `InstallHostCa
  (idempotent)` step between wait-ready and SpawnHarness; #554 folded
  InstallHostCa into SpawnHarness, so that step no longer exists
  standalone. Reworded to `wait-ready → SpawnHarness (installs CA +
  reattaches/respawns)`.

- ADR 0048: the Invariants bullet still said FIFO head-of-line blocking
  is global, contradicting the queue-fairness update earlier in the
  same doc (issue #537) that scoped it per `(mem_budget_mib,
  cpu_budget_vcpus)` fit class. Reworded to match the per-class scope.

- ADR 0069: `WIRE_VERSION 7 → 8` should read `8 → 9` — renumbered at
  merge since main had already taken 8 for #563. Confirmed against the
  current `engram_protocol::WIRE_VERSION = 9`.

- ADR 0067 duplicate: docs/adr/ had both
  0067-browser-stack-reliability-and-portable-bundle.md AND
  0067-chunk-cache-disk-budget.md. Renumbered the newer chunk-cache one
  (issue #528, #557) to 0070 (0068/0069 already taken) — renamed the
  file, updated its header + added a numbering note, and swept every
  `ADR 0067` reference in the repo that means the chunk-cache topic
  (engram-chunk-store, engram-host-agent, engram-uffd-handler, the
  host-fleet helm chart, README.md) to 0070. The two references that
  mean the browser-stack ADR (docs/adr/0027 and 0065, both explicitly
  paired with "issue #569") were left pointing at 0067, matching that
  ADR's own issue number.

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

* style: cargo fmt fixups on the placement/enable-jobs correction commits

Whitespace-only: cargo fmt wrapping log_empty_candidates's signature and
one assert_eq! in the extended live-PG test.

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

* test(host-agent): stage-deadline warm-hook watchdog test (T1, issue #539/#563)

Mirrors warm_hook_stall_fails_capture_with_stage_and_tail but proves the
OTHER watchdog arm: a stage exceeding its own deadline_secs is killed at
the budget even while the hook keeps emitting heartbeats fast enough
that the stall detector never fires. Asserts CaptureFailureKind::
WarmStageDeadline, the failing stage name, and the output tail.

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

* test(host-agent): trace diagnostics for the flaky NBD survivor test (T2, #582)

survivor_reconfigure_resumes_parked_io is a known-flaky CI test with no
diagnostics on failure. Add the same tracing_subscriber init
two_host_live_teleport.rs already uses (env-filter + try_init, safe
under repeat init) so the next natural CI failure captures
reattach()/serve_loop diagnostics. No timeout/assert changed.

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

* test(coordinator): persist-before-reconcile heartbeat regression (T3, issue #531/#564)

Proves the ADR 0068 early-return: a heartbeat whose touch_host_heartbeat
persist fails must 5xx AND must never reach reconcile_host for that
tick. Calls the real heartbeat handler directly (constructible without a
full axum server) with a MiniMeta fail-flag
(fail_next_heartbeat_persist), and asserts on a new
reconcile_probe_calls counter (an override of
list_active_sandbox_assignments_on_host, the entry point
Reconciler::reconcile_with_deps hits on every tick it actually runs) —
not just "no flip happened", which MiniMeta's no-op default
apply_missing_sandbox_strikes would satisfy either way regardless of
whether reconcile ran.

host_http.rs's local build_state_for_session now also returns the
Arc<MiniMeta> (mirroring state::tests::build_state_for_session) so the
new test can drive the fail flag and read the probe counter; updated
its four existing call sites.

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

* test(coordinator): prestage flip + straggler against live host rows (T4, issue #538/#565)

enable_scanner::run_once/advance_one/eval_prestage are pub(crate) or
module-private (unreachable from this external tests/ binary), and
advance_one unconditionally restarts every job from
fetch_and_seal_manifest (a real OCI registry fetch) regardless of state
— infra this live-PG-only lane doesn't wire (this file's own header note
already scopes the full pipeline exercise to the FC e2e suite).

So this drives the REAL fenced MetadataStore prestage surface
(begin_enable_job_prestage / set_enable_job_prestage_hosts /
list_active_hosts / touch_host_heartbeat) against two real polls of real
host rows: one host flips from un-staged to staged between polls, a
second never stages. Classification reuses the actual pub
placement::host_is_schedulable predicate (eval_prestage's own
unreachable logic reimplemented in ~10 lines), then sequences
prestaging -> ready exactly as advance_one's tail does. Asserts the
flipping host lands `staged` and the straggler lands `timed_out` in
prestage_hosts while the job still reaches `ready`.

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

* test(fc): probe_sandbox map-vs-process desync coverage (T5, issue #531/#564)

FirecrackerBackend::probe_sandbox is the ground-truth check
reconcile::flip_missing uses to avoid mistaking "not yet reattached" for
"actually gone" (the fbd3794c incident shape). Its implementation only
touches the in-memory sandbox map and an on-disk manifest + /proc read —
no jailer, no KVM ioctl — so this needs no real Firecracker VM: a
FirecrackerBackend that never created/reattached the sandbox (map has no
entry) plus a hand-written manifest pointing at this test process's own
real pid/start_time_jiffies/comm (genuinely alive for the test's
duration) is the cheapest honest construction of the desync arm.

Two tests: (1) known_to_backend=false while process_alive=true — the
desync reconcile must not treat as "gone", and (2) once the manifest is
also gone (mirrors destroy()'s manifest delete), the probe returns the
honest negative on both fields.

Wired into ci.yml's test-firecracker lane's unprivileged --test list
alongside this crate's other #[ignore]'d tests, though — unlike its
siblings — it needs neither /dev/kvm nor the firecracker binary; that's
documented in the test file's module doc comment.

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

* test(coordinator): no-op resume enqueue must not fire placement_changed (T6, issue #537/#559)

Negative twin of notify_placement_changed_fires_at_every_site.
enqueue_session_resume is gated on status='idle'; resume_origin_
enqueue_requires_idle already pins the row-level no-op, but nothing
pinned the NOTIFY side. Same LISTEN harness, but asserts a bounded
absence of any placement_changed notification after the no-op call
instead of waiting for an expected payload.

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

* test(host-agent): disk-leg idempotent VersionConflict redrive (T7, correction-pass item C)

publish_disk_manifest's redrive arm treats a VersionConflict at the
FIRST-attempt deterministic ref (base_manifest.next_version()) as
idempotent success (a prior crash-redrive already published this exact
content), not an error or a bump-and-retry. No fake store needed: a real
ChunkStore genuinely returns VersionConflict when a manifest already
exists at that (manifest_id, version) key, so this seeds that conflict
directly and asserts the redrive returns the already-published ref.

The memory leg's mirror-image arm (inside run_memory_leg) is left
untested: unlike the disk leg it isn't factored into a standalone
callable, and exercising it needs a full EvictionFinalizeRecord +
EvictionFinalizer + a real binary memory.diff file dirty_ranges can
parse + a matching prev-manifest — meaningfully more scaffolding than
the ~50-line budget. Noted in a doc comment as a follow-up refactor
(extract a publish_memory_manifest helper mirroring publish_disk_manifest).

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

* docs: correct ADR references across code and docs (audit sweep)

A line-by-line audit of ADR cross-references found five root-cause
clusters of drift and fixes each:

1. grpc_app never got its 0039→0051 sweep: ADR 0051 documents the
   coordinator's app-gRPC surface being renumbered from a draft 0039,
   but the sweep never touched crates/engram-coordinator/src/grpc_app/*
   or its grpc_app/grpc_smoke integration tests. Retargeted all of
   them; left the genuine ADR 0039 (memfile/checkpoint) references in
   idle_evictor, image_prefetch, pooled_backend, and placement.rs
   untouched.
2. Session profiles landed as ADR 0053, but proto comments, generated
   TS bindings, and orchestrator/web source + tests kept citing the
   working number 0052 (the persistent-streaming-harness ADR) from
   before the final renumber. Fixed the two source protos and
   regenerated the four gen mirrors via `just gen-proto` (never
   hand-edited), plus every orchestrator/web/docs citation; left the
   genuine 0052 (streaming harness) citations alone.
3. The disk-daemon util.rs kubelet-headroom gauge cites ADR 0067, but
   this branch's own 0067→0070 sweep (commit c6828c1) missed
   util.rs:176.
4. Two unrelated 0037 mentions dangle: ADR 0037 was never merged (its
   findings were folded into ADR 0052), so two_host_live_teleport.rs
   and ADR 0034's addendum now annotate the reference instead of
   pointing at a nonexistent doc; ADR 0052 itself gets one clarifying
   parenthetical at its first mention.
5. Two ADR numbers each had two files: 0061 (NBD read concurrency vs.
   VZ erofs skills) and 0042 (substrate survey vs. its evidence
   companion). Renumbered the younger of each pair to the next free
   numbers, 0071 and 0072, and retargeted the code comments + doc
   cross-reference that meant the renumbered ADR.

Also fixed a handful of standalone wrong-number citations (metrics.rs
boot-latency target, flush_scheduler.rs's idle-evictor and
session_bindings backreferences, and doc citations in 0015, 0016,
0060, 0064, and memory-substrate-and-restore.md) turned up by the same
audit.

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

* fix(enable-jobs): retry also resets chunks_done/chunks_total (review completeness)

The retry_enable_job stale-progress clear (this PR) covered capture_phase/
warm_stage/warm_stage_started_at/warm_stages/output_tail but not the
chunk-progress counters, which are the SAME live-progress class the UI
renders. The progress checkpoint writes chunks_done absolutely, so a
prior attempt's value read as live on a fresh retry until its first chunk
event (or forever on a pre-chunk failure). Reset them to 0/NULL; live-PG
test extended to assert it (they were 100/625 before the retry).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jJi2vqAaxt3Q5UKxbe4Gx

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hard host disk budget for the chunk cache: periodic enforcement, single evictor, pin-aware arithmetic, dedicated volume

1 participant