[ac-pplx]

What this changes

Node-manager's service constructor built two process-boundary dependencies itself: the runtime-supervisor client (the production Unix socket) and the memory-admission sources (/proc/meminfo and the Firecracker cgroup base). Only those production endpoints could back them. This PR moves both into ServiceSettings, the options struct the constructor already takes. ServiceSettings already carried the other two dependencies, the runtime paths and the lifecycle limiter.

The unit-test constructor (service_for_unit_tests) now defaults all four to test-owned values. Tests that reached a host dependency now inject what they need, and UpdateCgroup gains unit coverage.

The seams and their production wiring

Dependency	Production construction before	Production construction after
Runtime-supervisor client	RuntimeSupervisorLazyClient::new() inside NodeManagerService::with_memory_admission_config_and_task_tracker	RuntimeSupervisorLazyClient::new() in ServiceSettings::from_env; the constructor moves it into the service
Memory-admission sources	MemoryAdmissionSources::default() inside the constructor	MemoryAdmissionSources::default() in ServiceSettings::from_env
Runtime paths	ServiceSettings::from_env (SPACE_FIRECRACKER_* or the fixed defaults)	unchanged
Lifecycle limiter	LifecycleLimiter::from_env() in ServiceSettings::from_env, given the task tracker by the constructor	unchanged

Production builds the same values as before:

• The client is the same lazily connected client: same socket, connect_supervisor_proven_live dial with its health round-trip, same retry policy and connect timeout.
• RuntimeSupervisorLazyClient::new() only allocates. It opens no connection and logs nothing, so building it at the start of run rather than after the pool opens changes no behavior, ordering or log line.
• The memory sources are the same Default value.

There is no change to wire formats, schema, metrics, configuration or log lines. ServiceSettings derives Debug, so the client gets a field-less Debug impl. The startup log keeps listing explicit fields, not a Debug dump.

Every place the service reads these dependencies goes through the injected field. That was true before and still is:

Field	Read sites
runtime_supervisor	control.rs update_cgroup → UpdateCgroup; runtime.rs launch_inner → LaunchSidecar, stop_process_inner → StopProcess; launch_prepare.rs launch_sandbox_inner → LaunchFirecracker; cleanup_helpers.rs stop_sandbox_runtime_with_supervisor → StopSandboxRuntime; cleanup_inspect.rs cleanup_sandbox_phases → BindExecution, RetireExecution; execution_fence.rs bind_execution_for_allocation → BindExecution
memory_admission_sources	runtime.rs allocate_new_lease → MemoryAdmissionConfig::observe_active_leases
runtime_paths	service.rs refresh_runtime_inventory
lifecycle_limiter	acquire in control.rs, filesystem_work.rs, runtime.rs, launch_prepare.rs, cleanup_inspect.rs, snapshots.rs, encrypted_volumes.rs; close in cleanup_inspect.rs

RuntimePaths::default() returned the fixed host paths, and only tests used it, so this PR removes it. ServiceSettings::from_env builds its paths explicitly, as it did before.

Unit-test defaults

service_for_unit_tests used the production supervisor client, RuntimePaths::default(), MemoryAdmissionSources::default() and LifecycleLimiter::from_env(). It now uses:

• Supervisor: an in-process supervisor reached over an in-memory stream (runtime_supervisor_client::tests::refusing_client).
  • The connection passes the production liveness check.
  • Every RPC fails at once with Unimplemented.
  • That is not a connect failure, so the client does not spend its 15-second restart budget retrying.
  • A scripted variant, scripted_in_process_client, serves the existing stub supervisor the same way, with no socket for the test to own.
• Runtime paths and memory sources: paths under a vacant temporary directory (a unique tempdir name, created and removed). Discovery and admission both fail closed until a test installs its own fixture (installed_runtime() or MemoryAdmissionSources::new).
  • Memory is not scripted with an ample reading by default. A test that reaches admission without scripting it is refused (Unavailable) before it claims a slot.
  • That matches the refusing supervisor, and no existing test needed an implicit reading.
• Limiter: the compiled-in limits (lifecycle_limiter::tests::default_limiter), whatever SPACE_NODEMGR_LIMIT_* the runner sets.

Which tests relied on a host default. At the base commit I made each host default panic and ran the package's tests:

• Only cancelled_probe_retains_capacity_until_blocking_work_finishes reached one: it read the fixed runtime paths. It is converted below.
• No test reached the production supervisor socket or /proc/meminfo.

Tests converted

All mutation runs below used a tripwire build. It panics on the node-ctl dirty-mark dial, on read_pid_from_symlink (run directory), on a dial through RuntimeSupervisorLazyClient::new(), on a /proc/meminfo read and on runtime discovery under /space. No mutation's regression path tripped it.

At the base commit, the same tripwire fired in the four cases of blocked_cleanup_phase_times_out_before_stopping_the_runtime (node-ctl dial from the fixture's on-runtime allocation) and in cancelled_probe_retains_capacity_until_blocking_work_finishes (fixed runtime paths). Now they pass with no trip.

1. service::tests::cancelled_probe_retains_capacity_until_blocking_work_finishes (runtime assets).
   • Before: it read the node's fixed Firecracker installation and accepted any discovery failure except ResourceExhausted.
   • Now: it installs a Firecracker installation in a tempdir and requires cold discovery to succeed while the diagnostic lane is saturated.
   • Behavior mutation: route discovery through off_runtime_probe → fails with ResourceExhausted: node diagnostic capacity exhausted; retry.
   • Seam mutation: drop the runtime_paths override → fails with FailedPrecondition: kernel installation /tmp/space-node-mgr-unit-…/assets/default/kernel/vmlinux.bin is missing. That is the test-owned default, not a host path.
2. service::cleanup_inspect::tests::blocked_cleanup_phase_times_out_before_stopping_the_runtime::case_1_capacity (lifecycle limiter).
   • Before: it re-ran itself in a child process with SPACE_NODEMGR_LIMIT_CLEANUP_SANDBOX=1:1 to pin the limit.
   • Now: it injects a 1:1 cleanup limiter. It also holds the next phase's artifact lock, so a cleanup that ignores capacity stops there, before it reads run-directory pid links.
   • Its CleanupNode fixture now claims the lease off the Tokio runtime (allocate_test_lease), so allocation no longer dials node-ctl's fixed socket. This also covers the other CleanupNode users.
   • Behavior mutation: cleanup acquires the Release class instead of CleanupSandbox → fails (timed out waiting for artifact transfer ownership ≠ timed out waiting for cleanup capacity).
   • Seam mutation: drop the limiter override → the compiled 8:24 limit admits the cleanup → fails the same way.
3. service::control::tests::update_cgroup_forwards_current_execution_with_clamped_memory (supervisor; new).
   • UpdateCgroup had no unit coverage: reaching its supervisor call from a unit-test service dialed the production socket.
   • Through a scripted in-process supervisor, it pins two things. A stale execution is refused (FailedPrecondition) before the supervisor sees it. A current one is forwarded with its execution id and limits, with memory clamped to the node's per-sandbox cap.
   • Behavior mutations:
     • Drop the execution check → the stale request succeeds (unwrap_err on Ok).
     • Drop the clamp → the recorded memory_mib is 8192, not 4096.
   • Seam mutation: drop the scripted supervisor → the default refuses (Unimplemented … refuses UpdateCgroup).
4. service::control::tests::update_cgroup_reports_a_supervisor_refusal_with_its_code (supervisor default; new).
   • With the unit-test service's own refusing supervisor, UpdateCgroup keeps the supervisor's status code and prefixes the operation.
   • Behavior mutation: map every failure to Internal → fails.
   • Seam mutation: make the default supervisor accept UpdateCgroup → fails (unwrap_err on Ok).

concurrent_new_lease_admission_allows_only_one_reserved_cap now starts from the unit-test constructor. Before, it hand-built a service carrying the production supervisor client, the fixed runtime paths and the environment-read limiter. Its assertions are unchanged.

It is not host-clean: like every on-runtime allocation test, it still reaches the node-ctl dirty mark (below). So it is not claimed as a converted test.

installed_runtime() moved from launch_prepare/tests.rs to sit beside service_for_unit_tests, so both modules share it. pplx-space-test-env is no longer used by node-manager and is dropped from its dev-dependencies.

Call sites left unrouted

• node-ctl node-status dirty mark. node_state_sync::mark_node_status_dirty is a free function that slot_pool calls on allocation, release and config updates. On a Tokio runtime it spawns a dial to node-ctl's fixed socket.
  • Routing it needs a seam in NetworkPool, outside the service options this PR uses.
  • The tripwire panics at a test's first host contact, so these lists are lower bounds (first contact per test), not exact. The run on this branch lists at least 28 tests that still reach it: every_failed_stop_preserves_the_active_lease_and_reservation (its on-runtime pool.allocate, after it first trips on run-directory pid links), runtime::tests (allocate, release and stop-process tables), execution_fence::tests, three launch_prepare/service tests, cleanup_does_not_stop_a_lease_replaced_while_it_waited_for_the_gate, concurrent_new_lease_admission_allows_only_one_reserved_cap, network_pool_metrics::shutdown_stops_a_bounded_pass_of_wedged_probes and 12 slot_pool tests.
• Fixed host directories. RUN_DIR, CACHE_DIR, TEMPLATE_DIR and JAIL_DIR in service.rs, the host cgroup hierarchy read through pplx_space_node_runtime_core::cgroup (e.g. execution_fence's sandbox_is_quiescent, which execution_fence::tests preflight cases reach), and runtime_paths::RuntimePaths::firecracker() (jail layout) in snapshots.rs, are used throughout business logic. Routing them would put seams inside business logic.
  • Tests that still read fixed run-directory pid links (again first contact, so a lower bound): every_failed_stop_preserves_the_active_lease_and_reservation, inspect_sandbox_remains_available_when_diagnostics_are_saturated and cleanup_does_not_stop_a_lease_replaced_while_it_waited_for_the_gate, which reaches them before its expected adopt_stopping_lease refusal. The two spaced_host_peer_* tests read pid links from temporary run directories, so they don't touch the host.
• run.rs activate_runtime_supervisor_on_start activates the supervisor's systemd unit at startup. It is not a client call.
• The production constructor itself opens the host network pool (NetworkPool::open), so unit tests cannot call it. The production wiring is proven by the Kind runs below, not by a unit test.

Host safety

• New and converted tests use tempdirs, an in-memory supervisor and in-memory slot pools only. The tripwire runs above show their regression paths stay off node-ctl, the run directory, the production supervisor, /proc/meminfo and the fixed runtime paths.
• The vacant root is a unique name under the system temporary directory that nothing creates.

Follow-ups

• Re-run the tripwire in record-and-continue mode, logging every contact, to turn the lower-bound lists above into an exact inventory.
• Note in service_for_unit_tests' doc that an admission refusal from the unscripted memory source names /proc/meminfo whatever path was read, because NodeMemoryCapacityError::Read hard-codes it. Tests that reach admission should inject MemoryAdmissionSources::new.

Overlaps with open PRs

• node-mgr: validate LoadSnapshot requests before staging host state #269 (node-mgr snapshots.rs, snapshots/tests.rs) and tests: keep three tests off host state they don't own #278 (loop_devices/tests.rs): no shared files. node-mgr: validate LoadSnapshot requests before staging host state #269's added tests use none of these dependencies. Its existing snapshot tests already inject their own supervisor.
• docs: localized AGENTS.md for sbx-mgr, node-mgr and the api crate #275 adds crates/libs/node/mgr/server/AGENTS.md. Its sentence "Unit-test services must override runtime paths, memory-admission sources, the lifecycle limiter and the supervisor client wherever a test reaches them; their defaults read the host, env or the production supervisor socket" becomes stale once this merges. Whichever lands second should say the defaults are test-owned and refuse.
• [api] stop retrying restore pods whose snapshot the host CPU cannot load #221 and Add an opt-in Firecracker balloon device per template #212 also edit node-mgr snapshots.rs: no shared files.
• sbx-mgr: keep quarantined rows through inventory and bound recovery that can never succeed #270, sbx-mgr: characterize stop, delete and create core RPC paths (tests only) #271, api: characterize Kubernetes substrate suspend, pooled preparation and restore hydration (tests only) #273, api: release a Kubernetes create's claimed execution on every early exit #277: no shared files (sbx-mgr and api).

Validation

Head cebb5483360461ab7b4ead3d1641a0294178620c.

Check	Result
cargo fmt --all -- --check	pass
cargo clippy --workspace --locked --all-targets --all-features -- -D warnings	pass
check-rust-error-policy.sh, generate-env-contract.sh --check, rustfmt --check scripts/generate-env-contract.rs	pass
scripts/source_guards/*.py	env_test_hook_boundary, node_selector_extras_smoke, observability_vendor_boundary pass. thin_service_mains fails on crates/apps/vol/mgr/svc/src/main.rs, which this PR does not touch (pre-existing)
check_no_inline_rust_test_modules.py	flags two crates/libs/observability files this PR does not touch (pre-existing); nothing in node-mgr
pre-commit (Gitleaks)	pass
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-node-mgr --retries 0 ×3	454 passed, 9 ignored skipped, each run. The Postgres setup script ran, with 0 dependency-skip markers. .config/nextest.toml scopes the Redis setup to pplx-space-api; node-mgr has no Redis-backed tests
Mutation proofs (9)	all 9 fail the intended test; host-contact tripwire: 0 hits
Kind: fresh clusters	no cluster existed beforehand; every run below used a newly created cluster
Kind: binary provenance	space-nodemgr-svc sha256 03db7458…d156 matches across: the incremental artifact built after this commit, whose synced crates/ and proto/ are identical to this checkout; node-manager pods on all five clusters (all four pods on four of them; three of four on the first)
Kind: Tilt	41/41 resources ok, none pending, before the typed smoke
smoke-kind-e2e.sh	PASS (7/7) on each of the five fresh clusters
smoke-kind-typed.sh all, ECW pass	75/76 PASS, 1 ENVIRONMENTAL (details below)
smoke-kind-typed.sh all, native pass	23/23 PASS (7 + 8 + 8 over native 1/3, 2/3, 3/3; one fresh cluster each, as CI shards). native_contract.command_replay_credentials is IGNORED by design
CHANGE-INDUCED failures	none

ECW pass detail:

• First run (with SPACE_SMOKE_KIND_COLD_RECEIPTS and the admin token file set, as CI does): 49 passed, 0 failed, 27 inconclusive.
  • 26 of the 27 began once the host's Docker data disk filled; in-cluster Postgres reported No space left on device.
• Rerun of those 27 on a fresh cluster: 26 passed.
• local.ecw_template_build_from_oci_boots was inconclusive both times. Build-manager's storage admission requires 52 GiB free (requires 55834574848 available bytes), and this host offered 15–34 GB. ENVIRONMENTAL.

Native pass detail: inside all, the pass could not start because harness bootstrap failed on the full disk (ENVIRONMENTAL). It was then rerun as the three shards above.

🤖 Generated with Claude Code

[nickdujay-pplx]

Reviewed cebb548. Production still constructs the same lazy supervisor client and memory-admission sources; moving their construction into ServiceSettings does not introduce an eager connection or change the existing defaults. The test fixtures now isolate those dependencies, and the new UpdateCgroup coverage checks execution fencing, memory clamping, and error-code propagation.

Verified the CI test log: all 463 node-manager tests passed, including both new UpdateCgroup tests, the four cleanup-capacity/lock cases, and the runtime-discovery test. The full primary Rust run passed 8,873 tests with zero skips and zero dependency-skip markers. No blocking findings. The Kind runs described in the PR were not independently rerun.

[nickdujay-pplx]

Review summary for cebb54833604: approved in the review above.

The production constructor still supplies the same lazy supervisor client and memory source. I checked the changed dependency consumers and the new UpdateCgroup coverage for stale execution rejection, memory clamping, forwarding, and error-code preservation. The CI log confirms all 463 node-manager tests passed; the overall Rust run reported 8,873 passes with no skips. I did not independently rerun Kind.