[zibi-pplx]

Summary

Runtime inventory no longer requires a jailer or memory-state format for every installation. Add managed-only CH discovery and runtime-owned resident state observation. External diagnostic files remain ineligible for execution. Firecracker retains its implementation and historical metadata reader. Unknown observations remain errors, never guessed lifecycle states.

This introduces the control seam; the guarded boot consumer and authenticated connections arrive in #930. No protobuf, SQL, default-policy or runnable-environment changes.

Validation

Combined CPU-stack validation at #960: 35 focused Linux tests passed for containment filesystem contracts, control-peer/protocol handling, authenticated-store publication/removal, install/retry/removal across both directory layouts, and fenced crash recovery with saturated storage capacity. The production-permission registration test, shared-inode output test, and saturated-recovery test failed before their respective fixes and passed afterward.

The actual fresh-process launcher passed a disposable privileged Linux probe with the production sidecar-directory mode and console ownership: PID 1, dropped credentials, detached host mount tree, private devices, inherited root-only listener, and refusal of a substituted device directory. This does not qualify KVM, cgroup limits or a CH guest boot.

Affected-package Linux ARM64 all-target/all-feature Clippy passed with warnings denied; workspace rustfmt passed. Ruff passed and both embedded smoke programs compile. Live guest and repeated crash/manager-restart qualification has not been repeated and remains required.

Severity and risk

Runtime observation changes can affect recovery. CH remains unlaunchable until the compiled environment and complete launch integration land; this PR alone does not enable it.

Stack

CPU execution sequence: #925 → #926 → #928 → #929 → #930 → #931 → #960. Deploy a supervisor binary containing #930 before node-manager and runtime-manager. #929 alone is not an enablement boundary. Drain CH executions before rolling back below #930; retain a supervisor containing #930 while any CH execution exists. Enable CH explicitly only after the complete sequence is deployed. Firecracker/E0 remains the default. GPU and Windows are separate work.

AI assistance: Codex drafted code, tests and this description.

[the-gigi-pplx]

Review — 7 findings

Medium

1. crates/libs/node/mgr/server/src/service/snapshots.rs:207 — GetVmState for live Firecracker VMs now reads metadata.json first and fails closed, and nothing in the stack tests that dispatch.
   When the file is absent, it falls back to Firecracker (runtime_control.rs:16, snapshots.rs:210-213). But a file that is present and refused fails the probe of a healthy Firecracker VM. Refusal covers a non-regular file, nlink > 1, more than 1 MiB, a parse error, schema_version != 2, and an id mismatch (runtime_control.rs:17-46). sbx-mgr maps that failure to Unobservable, and interrupted-pause recovery then quarantines the sandbox (sbx/mgr/server/src/service/recovery.rs:476-479). Capture already failed closed on the same file, so the policy is consistent. What's missing is a test pinning the three cases: absent → Firecracker socket, CH metadata → CH socket, corrupt or mismatched → error. The only GetVmState test is the authorization matrix (service/tests.rs:293), and no later PR in the stack adds one.

Low

2. crates/libs/node/mgr/server/src/service/snapshots.rs:218 — The 250 ms probe deadline covers only control.state(). The new metadata read runs before it as an unadmitted spawn_blocking (off_runtime, cleanup_helpers.rs:80-106). Callers stay bounded by sbx-mgr's 500 ms RPC timeout, but under filesystem stalls node-manager can pile up blocked threads, and PR node: launch and restore Cloud Hypervisor through runtime adapters #930 adds a second unbounded read on the CH path. Consider off_runtime_probe, or putting the read inside the deadline.
3. crates/libs/node/runtime/core/src/control.rs:31 — RuntimeControl::pause/resume have no callers here, and Firecracker pause and capture still build firecracker.sock paths by hand (snapshots.rs:275, :362). PR node: launch and restore Cloud Hypervisor through runtime adapters #930 removes these methods from the trait, so either drop them here or note that the trait is transitional.
4. crates/libs/node/runtime/core/src/control.rs:69 — The Firecracker branch of socket_path re-derives node_fs::jail_root (common/core/src/node_fs.rs:53-56). It also puts CH under the Firecracker-named jail directory (…/firecracker/jails/cloud-hypervisor/<id>/root). Per-sandbox isolation holds, but the layout now has two derivations that can drift.
5. crates/libs/node/runtime/core/src/control/cloud_hypervisor.rs:86 — The doc says the enum lists the upstream v54 VM states, but upstream v54.0 also has BreakPoint (vmm/src/vm.rs, only reachable with the non-default guest_debug feature). Rejecting it as unknown is right, and tests.rs:11 covers it. Only the doc should change.
6. crates/libs/node/runtime/core/src/runtime_inventory.rs:970 — Discovery watches default/bin/firecracker and default/bin/jailer for every root, CH roots included. CH directory discovery yields nothing today, so this has no functional effect, but the watch list should come from the driver (release.binaries), as managed.rs now does.
7. crates/libs/node/runtime/core/src/runtime_inventory/cloud_hypervisor.rs:60 — compare_versions returns a semver ordering next to a comment saying replacement preference is "never inferred". Its only caller, the discover() sort, never sees CH entries, so it's unreachable. Either drop the ordering or reword the comment.

Two things are fixed later in the stack, so I'm not filing them here. First, with a CH environment allowed, launch_prepare.rs:314 fails a CH resolution with the misleading "missing jailer" error; PR #930 replaces this with explicit runtime dispatch. Second, recovery.rs:473 / startup_resident.rs:192 say "Firecracker reported NotStarted" for a CH Shutdown; PR #930 corrects the strings.

Verified against 33c83cf91be7. I traced each finding through the later stack PRs, and checked the upstream Cloud Hypervisor v54.0 VmState enum against its source. I read the code; I didn't run the tests.

[the-gigi-pplx]

Approved. 7 non-blocking findings in the review comment above.

[zibi-pplx]

@the-gigi-pplx Reviewed these against the complete seven-PR CPU sequence. The accepted fixes are applied and absorbed locally, not pushed at the author’s request; the GitHub heads do not contain them yet. The later GPU/Windows drafts were left untouched.

• Added regression coverage for missing metadata (historical Firecracker), valid Firecracker/CH identity, malformed data and a foreign sandbox ID. Present but invalid identity still fails closed.
• Put metadata, endpoint attestation and state probing under the same deadline; filesystem work uses bounded probe admission, retained until blocking work actually finishes.
• Reused node_fs::jail_root, corrected the v54 state comment, restricted Firecracker default-binary watches to Firecracker, and clarified that diagnostic version ordering is not replacement policy.
• The final stack uses the generic pause-state operation; the unused transitional trait methods are already gone.

These integration fixes are absorbed in #930, where the final dispatch and endpoint checks live. Validation: the metadata-selection regression and targeted runtime tests passed on Linux. This does not replace live endpoint qualification.