[ac-pplx]

Includes #299. The template-manager resolver seam was reviewed as #299 and merged into this branch, so this PR lands both seams together.

• Every sandbox-manager GetTemplate goes through an injected TemplateResolver. Production uses HostTemplateManager, with the same client construction, error mapping and requests.
• sbx-mgr: inject the template-manager resolver #299's description has the full call-site inventory, mutation proofs and validation.

Summary

Every node-manager client sandbox-manager builds now comes from one injected NodeManagerConnector. Production keeps today's exact construction. Tests can reach an in-process node-manager instead of the host's fixed /space/run/node-manager/node-manager.sock.

The seam

crates/libs/sbx/mgr/server/src/node_manager_transport.rs adds:

• NodeManagerTransport, a trait with connect(), connect_once() and lazy(). These mirror the three pplx-space-node-mgr-client constructors.
• NodeManagerConnector, a cloneable handle around Arc<dyn NodeManagerTransport>.
• NodeManagerConnector::host(), the default. Each method calls the client-crate function of the same name, pplx_space_node_mgr_client::{connect, connect_once, lazy}.

The connector is a new field, SandboxManagerServiceOptions::node_manager, defaulting to NodeManagerConnector::host(). SandboxManagerService::new_with_config hands it to every subsystem it builds:

• the watchdog's sidecar and composed-health loops;
• the snapshot scheduler (CheckpointContext::node_manager);
• the snapshot-upload worker and its NodeManagerSourceInspector;
• the snapshot reaper (GcOwnership::node_manager);
• the template-policy gateway caller client.

Two pieces of process-wide state now follow the connector:

• Capability cache. The retained-source inspection capability answer is cached per connector and shared by its clones. host() always returns the same instance, so production keeps exactly one cache, with today's 30 s / 5 s lifetimes.
• Immediate-reclaim backlog. Each backlog entry carries the connector that submitted it. The worker that drains the backlog deletes every entry through that entry's own node-manager.

Service handlers reach it through self.node_manager. Free functions take it as a parameter: btrfs_via_nodemgr, firecracker_via_nodemgr, runtime_reconciliation, snapshot_encryption, rootfs_encryption, the restore pipeline in snapshot_restore, snapshot_upload::submit_prepared_snapshot, and staging reclaim. run() builds one host connector and uses it for both the takeover-health and startup-recovery probes (connect_once) and the service options.

Production wiring is unchanged

• Same constructor at every site. Each call site calls the connector method named after the client function it called before: connect where it dialed with the default retry policy, connect_once in the two startup probes, and lazy where it used the lazy channel.
• Same lazy channel. host().lazy() returns the client crate's process-wide OnceLock channel, so every lazy caller still shares one reconnecting channel.
• Same timing. No call site moved: each still dials at the same point, with the same timeout wrappers.
• Same errors and logs. Each site keeps its own map_err text, so error messages and log lines are identical.
• No wire, schema, metric or configuration change.
• The trait is async_trait, so each connect future is boxed. That is the only runtime difference.

Call-site inventory, production code in crates/libs/sbx/mgr/server/src:

	before (7a41941)	after
pplx_space_node_mgr_client::connect() / connect_once / lazy()	30 sites	3, all inside HostSocket in node_manager_transport.rs
tail_support::connect_node() wrapper	24 call sites	24, now connect_node(&self.node_manager)

Before, by owner. Every site is routed through the connector:

• Service handlers. 24 connect_node() calls across these modules: cleanup ×3, create ×4, kill ×2, start ×2, grpc_spaced_files ×2, grpc_spaced_process_snapshots ×3, auth_lookup, grpc_core, pause_resume, suspend, suspend_snapshot, artifact_backup, grpc_net_gateway and grpc_state_snapshot.
• Other service sites. Direct dials in grpc_state_snapshot (×3 connect), recovery_cross_node (connect) and grpc_pool_wait_clone::connect_clone_node (connect). Lazy clients in spaced_bridge, the cache_cleanup orphan observer, the recovery inventory reconciler and gateway_caller_client in service/mod.rs. Also the connect inside tail_support::connect_node itself.
• btrfs-via-node-manager. encode_rootfs_send, encode_snapshot_rootfs_send, receive_instance, delete_sandbox_instance and materialize_rootfs_send (connect); inspect_send_source and the inspection capability probe (lazy).
• Firecracker state. firecracker_via_nodemgr::get_firecracker_vm_state (lazy), used by the watchdog health probe, runtime reconciliation and stop-flush.
• Subsystems. snapshot_encryption ×2, rootfs_encryption ×2, snapshot_scheduler ×2, watchdog ×2 (spaced-host restart, net-gateway relaunch), snapshot_gc ×1 (connect), and recovery/staging_cleanup::reclaim_one (lazy).
• Startup. run.rs ×2 (connect_once, takeover health and startup recovery).

Clients that are passed in as a NodeManagerGrpcClient all come from one of the routed sites above, so they are covered: the cleanup dispatch helpers, pause_with_reconciliation, the status readers, SnapshotPages, the endpoint-proof capture and the orphan-execution observer.

Call sites left unrouted

None in production code.

• Existing tests that build options with ..Default::default() still get NodeManagerConnector::host(), exactly as before. Only the tests below were converted.
• run/test_receiver.rs builds its own test client and is unchanged.

Tests converted

FakeNodeManager (node_manager_transport/fake.rs, test-only) serves scripted unary RPCs on a socket in a test-owned tempfile directory. It refuses every other method with Unimplemented and records the method names it receives.

• fatal_kill_signal_failure_still_schedules_admitted_cleanup. Moved from service/tests.rs to the new service/tests/node_manager_transport_tests.rs, because sbx-mgr: keep quarantined rows through inventory and bound recovery that can never succeed #270 (open) and tests: keep three tests off host state they don't own #278 (since merged) edit service/tests.rs. Its test ID becomes service::tests::node_manager_transport_tests::fatal_kill_signal_failure_still_schedules_admitted_cleanup. Before, it passed only because the host had no node-manager to dial. Now node-manager refuses StopProcess, and the test asserts:

  • the caller receives that refusal;
  • exactly one StopProcess was sent, fenced to the observed execution;
  • the admitted cleanup still sends one CleanupSandbox for the same execution, with the filesystem kept, immediate_kill, and return after slot release;
  • the row ends Stopped;
  • node-manager saw exactly StopProcess, then CleanupSandbox.

  A live host node-manager cannot satisfy these assertions.

• blocking_stop_follows_a_retryable_attempt_to_durable_completion (service/cleanup_executor/tests.rs). Previously it avoided node-manager through the cleanup executor's test dispatch hook. Now it drives the real CleanupSandbox dispatch against a fake that answers Unavailable once and then succeeds. It asserts both requests (fence, keep_filesystem, immediate_kill, force, return after slot release), that the fake received no other call, and that the row ends Stopped.

• immediate_reclaim_backlog_uses_each_entrys_own_node_manager (new, service/recovery/staging_cleanup/tests.rs). The test holds both reclaim slots, then submits two entries through two different fakes. They are regular files, so the local removal fails even for root. A third submission through a third fake drains the backlog. Each fake receives exactly its own entry's DeleteCacheArtifact.

• send_source_inspection_is_cached_per_connector (new module btrfs_via_nodemgr/capability_cache_tests.rs, because Defer btrfs-send capture by default in sandbox-manager #287 edits btrfs_via_nodemgr/tests.rs). Two fakes give opposing capability answers. Each connector gets its own answer twice, from a single probe.

• Two tests with incidental node-manager fallbacks. immediate_reclaim_removes_ordinary_entries_and_leaves_the_rest_in_the_lane and confirmed_node_pause_reports_paused_and_carries_the_execution_fence now pass a fake connector. A DeleteCacheArtifact or Firecracker-state fallback in those tests can no longer reach the host.

Mutation proof

Each mutation was applied, the named test was run with --retries 0, and the file was restored. Node-manager was the fake throughout.

#	Mutation	Test	Result
A2	kill: StopProcess drops the execution fence	fatal kill	killed
A5	kill: the scheduled cleanup drops the filesystem (keep_filesystem: false)	fatal kill	killed
A6	kill: the admitted cleanup completes to Created, not Stopped	fatal kill	killed
A1	kill: at the StopProcess dispatch's Err arm, return the error instead of continuing	fatal kill	survived. On this persisted, non-blocking path, admission has already stored the cleanup job and started the executor, so the cleanup still runs. The test therefore does not pin three things the early return skips. It skips the unpersisted-admission branch (run_cleanup_after_persistence_failure), which this test never reaches. It moves watchdog and snapshot-scheduler deregistration from the handler to the cleanup. And for blocking kills, it skips the wait for cleanup to complete (run_admitted_cleanup_to_completion / persist_and_retry_cleanup_failure), so a blocking caller would get the error before cleanup finishes. All three remain uncovered
B1	CleanupSandbox inverts immediate_kill	blocking stop	killed
B2	A retryable attempt fails the durable job permanently	blocking stop	killed
A3 / B3	Seam misconfigured: the fake does not answer StopProcess / CleanupSandbox	each	killed
A4 / B4	Seam misconfigured: the connector dials a test-owned socket nothing serves	each	killed
R1	Reclaim: the draining worker deletes every backlog entry through its own connector	backlog connectors	killed
R2	Seam misconfigured: the first entry's fake does not answer DeleteCacheArtifact	backlog connectors	killed
K1	Capability cache shared by every connector in the process	per-connector cache	killed
K2	Capability answer not cached	per-connector cache	killed

The other required seam check, reverting to the default host connector, was not run. It would dial the fixed host socket, which the host-safety rule forbids even on a mutation's regression path. A4 and B4 prove the same dependence without contacting the host.

Host safety

• The converted tests build their service with every field explicit: the fake connector, host-control and credential-manager socket paths nothing binds, inside a test-owned temporary directory, no live runtime, and a fixed node ID.
• Every node-manager call reaches the fake. On a regression path, an unexpected RPC is refused with Unimplemented before any destructive step.
• One read-only probe remains outside this seam: watchdog.remove_sandbox checks exists() on /space/run/sandboxes/<fresh id>/space-netgtw-svc.pid. The ID is new, so nothing exists there.

Overlaps with open PRs

Rebased onto a8b5df6, which merged #269, #271, #273, #275, #277, #278 and #302. Those PRs change no sbx-mgr production code, so the call-site inventory above, taken at 7a41941, still holds. Open PRs were re-checked against this head with git merge-tree, plus a scan of each diff's added lines for the APIs this PR changes and for SandboxManagerServiceOptions literals:

• sbx-mgr: characterize stop, delete and create core RPC paths (tests only) #271 (merged). Its isolated_service in service/grpc_core/tests.rs lists every SandboxManagerServiceOptions field, so this PR now adds node_manager there. The value is node_manager_transport::fake::unbound(&scratch.join("absent-node-manager.sock")): a connector on a socket inside the test's scratch directory that nothing binds, next to the absent host-control and credential-manager sockets it already uses. Any node-manager call from those tests fails closed. fake::unbound is new in node_manager_transport/fake.rs and shares the fake's lazy-client construction. sbx-mgr: characterize stop, delete and create core RPC paths (tests only) #271's tests are otherwise unchanged. Their stop and delete cases still reach node cleanup only through the executor's test dispatch hook.
• tests: keep three tests off host state they don't own #278 (merged). The rebase was clean. This PR adds one node_manager: line to runtime_liveness_options in service/tests.rs, the function tests: keep three tests off host state they don't own #278 edited.
• docs: localized AGENTS.md for sbx-mgr, node-mgr and the api crate #275 (merged). No file overlap. The sbx-mgr AGENTS.md it added tells tests to give the code under test a test-served node-manager; this connector is how they do it.
• node-mgr: validate LoadSnapshot requests before staging host state #269, 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, Stamp effective TLS interception mode on sandbox create events #302 (merged). No overlap.
• No other SandboxManagerServiceOptions literal on main lists every field, and no open PR adds one.
• sbx-mgr: keep quarantined rows through inventory and bound recovery that can never succeed #270. Its only conflict, in cache_cleanup/periodic/tests.rs, is also present against main. This PR's service/tests.rs deletion is far from sbx-mgr: keep quarantined rows through inventory and bound recovery that can never succeed #270's hunks.
• Quarantine sandboxes after sustained guest-agent probe failure #110. It already conflicts with main in db.rs and watchdog.rs. This PR adds one more conflict hunk in watchdog.rs, because both rewrite the probe_composed_health call in composed_health_loop's spawned probe. Keeping Quarantine sandboxes after sustained guest-agent probe failure #110's version of that line breaks compilation: it needs the let node_manager = node_manager.clone(); before pending.spawn and the extra &node_manager argument.
• docs: localized AGENTS.md for sbx-mgr, node-mgr and the api crate #275. No file overlap. Its proposed sbx-mgr AGENTS.md says connect()/lazy() sites cannot be redirected; with this PR they can.
• node-mgr: validate LoadSnapshot requests before staging host state #269, 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 overlap.
• Other open PRs touching sbx-mgr.
  • Defer btrfs-send capture by default in sandbox-manager #287 edits btrfs_via_nodemgr.rs. This PR adds the connector as the last parameter there, so Defer btrfs-send capture by default in sandbox-manager #287's hunk context is untouched, and the merge is clean.
  • Pace failed teardown drivers without holding worker capacity #285, Recover teardown of restored snapshots with read-only metadata #281 and Approve bounded publication of validated retained suspends #283 merge cleanly.
  • sbx-mgr: keep teardown polls pending while a delete is in flight #201's conflict in grpc_lifecycle.rs is also present against main.
  • Preserve authenticated scope in cached template lookups #217 merges cleanly with this PR; its conflicts are with sbx-mgr: inject the template-manager resolver #299.
  • fix(sandbox): wait before retrying failed activations #3, [api] restore full snapshots only onto CPU-compatible hosts, behind a default-off gate #176, [sbx-mgr] record the capture host CPU on snapshots and publish it as a node label #203, Add an opt-in Firecracker balloon device per template #212 and [api] stop retrying restore pods whose snapshot the host CPU cannot load #221 each conflict with main already. This PR adds no conflict hunk to any of them, and their added lines call none of the APIs it changes.

Validation

Check	Command / evidence	Result
fmt	cargo fmt --all -- --check, at 56a295d	pass
clippy	cargo clippy --workspace --locked --all-targets --all-features -- -D warnings, at 56a295d	pass
repo guards	check-rust-error-policy.sh, generate-env-contract.sh --check, source_guards/env_test_hook_boundary.py, at 56a295d	pass
repo guards (pre-existing failures)	check_no_inline_rust_test_modules.py (two inline modules in observability) and source_guards/thin_service_mains.py, at 56a295d	output identical to main a8b5df6; files not touched here
pre-commit	gitleaks hook on the commit	pass
unit tests ×2 after rebase	SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-sbx-mgr --retries 0, two consecutive runs at 56a295d (nextest's Postgres setup script passed)	1568/1568 passed each run (1 #[ignore] test not selected); 0 dependency-skip markers
unit tests ×3 before rebase	same command, three consecutive runs at the pre-rebase head a314315	1562/1562 passed each run
mutation proof	see the table above	13 killed; A1 survives (uncovered behaviors stated)
Kind cluster	one new space-local cluster per attempt on the current harness, built from #299's head b196857	up
deployed binaries	space-sbxmgr-svc sha256 8e9ce23a…7090 in both worker space-sandbox-manager pods, before and after the suites, equals the staged artifact built from this checkout; the synced build source for sbx-mgr is identical to b196857; the binary contains the node_manager_transport and template_resolver module paths	proven
Tilt quiet	tilt get uiresources: 42/42 ok, none pending or in error	quiet
smoke-kind-e2e.sh	BOOTSTRAP_TIMEOUT=1800 DATA_PLANE_URL=http://127.0.0.1:8090	PASS on both clusters
smoke-kind-typed.sh all, attempt 1	SKIP_BOOTSTRAP=1, SPACE_SMOKE_KIND_ADMIN_TOKEN_FILE and SPACE_SMOKE_KIND_COLD_RECEIPTS set as in kind-ci.yml, unsharded	ENVIRONMENTAL. 58 scenarios passed, then local.template_cache_lru_eviction_reclaims failed and the following scenarios were inconclusive. Docker's data disk (49 GB) reached 100%, so the Kind nodes' overlays could not write: os error 5 under /space/cache and Postgres could not write init file. A rerun on that cluster could not start sandbox pods (pod_never_ready), so the cluster was torn down
disk	docker builder prune -af and docker image prune -f only	freed 20,668,022,784 bytes after attempt 1 and 6,930,202,624 bytes before the final typed run
smoke-kind-typed.sh all, rerun on a new cluster	same environment	ECW 76 passed, native 23 passed; 0 warned, 0 failed, 0 inconclusive (includes ecw.cold_create_environment)

The mutation proof and Kind evidence come from the pre-rebase heads: a314315, and #299's head b196857, which contained a314315 unchanged. They were not re-run after the rebase. Since then, the only changes to this PR are the fake::unbound test helper and the new node_manager field in #271's test isolated_service, both test-only; main's merged PRs change no sbx-mgr production code.

🤖 Generated with Claude Code

[the-gigi-pplx]

Review — 2 findings

No blocking findings. The production change is a mechanical threading of one connector, and I couldn't find a site it misses: at this head the only pplx_space_node_mgr_client::{connect, connect_once, lazy} calls left in crates/libs/sbx/mgr/server/src outside tests are the three inside HostSocket (node_manager_transport.rs:36-44). The two remaining NodeManagerConnector::host() calls outside Default and run() sit in SandboxWatchdog::new_for_tests (watchdog.rs:816, :829), which is #[cfg(test)].

Low

1. service/recovery/staging_cleanup/tests.rs:661: the new backlog test and the existing immediate_reclaim_removes_ordinary_entries_and_leaves_the_rest_in_the_lane both take every permit of the process-wide IMMEDIATE_RECLAIM_SLOTS and share the static backlog.
   Under nextest each test gets its own process, so CI is unaffected. Under plain cargo test they run as threads in one process and collide. I reproduced it with cargo test -p pplx-space-sbx-mgr --lib immediate_reclaim: panicked at staging_cleanup/tests.rs:583:56: called Result::unwrap() on an Err value: NoPermits. Both pass with --test-threads=1. Before this PR only one test touched these statics. A module-level static RECLAIM_TEST_LOCK: tokio::sync::Mutex<()> held by both tests would close it.
2. service/tests/node_manager_transport_tests.rs:3: the module doc says the service is built "with every host dependency redirected". At this head that isn't true for template-manager, because a service built by service_with_node_manager still resolves templates through pplx_space_template_mgr_client::connect() (create_support::resolve_template_paths) and the host lazy() channel (network_policy::fetch_template_default_network_policy). It also isn't true for the watchdog's pid exists() probe, which the PR body already names. PR sbx-mgr: inject the template-manager resolver #299 closes the template-manager gap in substance, and its body lists updating this doc as a follow-up. Name the template-manager exception here, or narrow the sentence to node-manager, host-control and credential-manager.

Verified against e1e992711a833d87ed68e49f588246c8df1e05e8. Reproduced locally: ran the converted and new tests (fatal_kill_signal_failure_still_schedules_admitted_cleanup, blocking_stop_follows_a_retryable_attempt_to_durable_completion, immediate_reclaim_backlog_uses_each_entrys_own_node_manager, send_source_inspection_is_cached_per_connector, both incidental-fallback tests), and all pass. Re-ran two of the PR's mutations myself, and both were killed:

• R1: the draining worker deletes every backlog entry through its own connector. The test fails, with all three deletes landing on the drainer's fake.
• K1: a process-wide static capability cache in place of the per-connector one. It fails at capability_cache_tests.rs:29.

git range-diff shows this head is patch-identical to the 56a295d the description validates. I read the rest of the production diff without running it. I didn't run a Kind smoke or the full sbx-mgr suite.

[the-gigi-pplx]

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