[ac-pplx]

Adds characterization tests for the sandbox-manager Stop, Delete and Create RPC handlers in crate::service::grpc_core, so the upcoming lifecycle consolidation (one state-write helper, splitting the service struct) has a safety net at the RPC layer. Tests only: the only file changed is service/grpc_core/tests.rs, which was already wired in.

Before this change, the RPC layer of stop and delete_with_auto_lifecycle_expectation had no tests for its own branches. Nothing covered the blocking vs non-blocking split, the admission-time dirty ref and its x-space-skip-cluster-sync gate, the empty template filter, the stop-only live-runtime selection, the shutdown refusal, the auto-lifecycle expectation guards, or any of delete's pool-clone disposal branches. The lower layers (target resolution, admission, cleanup completion) already have their own tests, so these tests assert only what the RPC layer adds: return values, persisted rows, cleanup episodes, durable cleanup jobs, node dirty refs, and the status codes and metadata that callers match on.

Tests

Test	Production behavior pinned
forwarded_create_refuses_owner_and_network_policy_authority_before_template_lookup	create from a forwarded (node-api) peer, for every BootMode: asking for the template-manager owner → PermissionDenied; a Local network-policy authority → PermissionDenied; an unspecified authority → InvalidArgument "…is unspecified"; an unknown authority 99 → InvalidArgument "…99 is unknown". Two accepting controls: no network policy, and a Cluster authority, both pass the network-policy check and reach the barrier (below). No row is created. Replaces the two earlier forwarded-peer create tests and keeps their network-policy case.
core_rpcs_refuse_unauthenticated_and_unlisted_peers_without_mutation	create, stop, delete: no peer credentials → Unauthenticated. A node-ctl peer (not on the allow list) → PermissionDenied; for create this is pinned by the peer check's message, because the owner check refuses node-ctl too. No row, cleanup episode, cleanup job or dirty ref appears.
stop_selects_only_a_live_runtime_and_both_treat_an_empty_template_filter_as_none	stop selects a sandbox only when its runtime is live; delete selects it either way. An empty template_filter behaves like no filter. A different template selects nothing. An absent sandbox is an idempotent Ok with no ids. When a sandbox is selected, blocking cleanup completes (stopped, or the row is removed) and the sandbox has a dirty ref.
nonblocking_cleanup_returns_once_admitted_and_honors_skip_node_publish	A non-blocking stop or delete returns while node cleanup is still held, leaving the row stopping/deleting with a durable cleanup job. The RPC adds an admission-time dirty ref, and x-space-skip-cluster-sync skips it; the test checks the ref at return time, before node cleanup is released. Releasing node cleanup completes the job, after which the sandbox has a dirty ref with or without the header: the header defers the dirty ref rather than suppressing it. No current caller sends this header to Stop or Delete: its only sender is node-ctl, which both RPCs refuse.
template_manager_cleanup_requires_a_template_owned_sandbox	A template-manager peer may stop or delete only template-owned sandboxes. Otherwise it gets PermissionDenied and the row is untouched.
cleanup_rpcs_refuse_after_shutdown_without_mutation	After shutdown, stop and delete return Unavailable and leave the row untouched.
auto_lifecycle_delete_requires_one_canonical_id_and_the_expected_policy	Delete with an auto-lifecycle expectation returns Internal when given a template filter or a non-canonical (uppercase) id. A stale policy fingerprint returns Aborted carrying AUTO_LIFECYCLE_DELETE_CAS_LOST_METADATA (the reconciler matches on this key), and no receipt is created. The current policy deletes the row.
delete_of_a_pool_clone_disposal_requires_an_operator_after_quarantine	Delete's pool-clone disposal branch. While publication still owns the clone → FailedPrecondition. After quarantine, a template-manager or auto-lifecycle delete → FailedPrecondition, and a held cleanup fence → Unavailable. In every refusal the state, episode and jobs are unchanged. An operator delete re-arms the disposal and returns: non-blocking leaves a pending cleanup job, blocking removes the row and episode.

Create cases never reach template lookup

resolve_template_paths connects to the fixed template-manager socket, and a live template manager can pull artifacts and reconcile snapshots. These tests cannot redirect it without a production seam. So every create request is refused before that call, both on the correct path and under every mutation below.

Each create request starts from a barrier: an auto-lifecycle policy with an unknown authority, 98. Create parses that authority last before template lookup, and it refuses with InvalidArgument "policy authority proto value 98 is unknown", a message no case expects for its own refusal. Every case asserts both code and message, so reaching the barrier in place of the case's own refusal is a failure. Pre-lookup refusal order in create: authorize_peer, authorize_create_sandbox_owner, validate_create_request, network-policy authority parse plus require_local_authority_peer, auto-lifecycle authority parse plus require_local_authority_peer.

Create case	Own refusal	If that refusal is removed
no credentials	authorize_peer	barrier
node-ctl peer	authorize_peer	owner check, with a different message
template-manager owner	owner check	barrier
local network policy	network require_local_authority_peer	barrier
unspecified or unknown network authority	network authority parse	network require_local_authority_peer if it parses as Local; barrier, with a different message, if it parses as Cluster
no network policy, cluster network policy	barrier (accepting controls)	the barrier is not mutated; a network check that refuses them refuses before lookup

Mutations

Each mutation was a scratch edit to grpc_core.rs (M33 edits cleanup.rs), run against the final test file and then reverted. All 33 made the listed test fail. For create mutations, the third column shows the refusal the failing case actually got.

Mutation	Failing tests	Caught by
M1: a bad network-policy authority parses as Local	forwarded create	PermissionDenied "authority=local requires a local operator peer…" in place of "…is unspecified"
M1c: a bad network-policy authority parses as Cluster	forwarded create	barrier message in place of "…is unspecified"
M3: drop the network-policy require_local_authority_peer	forwarded create	barrier InvalidArgument in place of PermissionDenied
M30: refuse every forwarded network-policy authority	forwarded create	"cluster network policy" gets PermissionDenied in place of the barrier
M32: run the network-policy check without a network policy	forwarded create	"no network policy" gets "…is unspecified" in place of the barrier
M4: skip authorize_create_sandbox_owner	forwarded create	barrier InvalidArgument in place of PermissionDenied
M5: create falls back to a root identity when credentials are missing	peers	barrier InvalidArgument in place of Unauthenticated
M31: Create's allow list admits node-ctl	peers	owner refusal "root@local is restricted to local privileged callers" in place of the peer refusal
M6: Stop's allow list admits node-ctl	peers, template-manager
M7: Delete's allow list admits node-ctl	peers, template-manager, pool clone
M8: stop passes running_only = false	selection
M9: stop stops ignoring an empty filter	selection
M10: delete stops ignoring an empty filter	selection
M27: delete of an absent sandbox returns NotFound	selection
M28: stop of an absent sandbox returns NotFound	selection
M11: stop's blocking branch is flipped	non-blocking, selection, template-manager
M12: delete's blocking branch is flipped	non-blocking, selection, template-manager, auto-lifecycle
M13: stop's publish gate is inverted	non-blocking
M14: delete's publish gate is inverted	non-blocking
M33: cleanup completion stops marking the sandbox dirty (cleanup.rs)	non-blocking
M15: stop passes false for the template-manager flag to both target resolution and admission	template-manager
M16: delete passes false for the template-manager flag to both target resolution and admission	template-manager
M17: drop stop's cleanup_request_permit	shutdown
M18: drop delete's cleanup_request_permit	shutdown
M19: drop the expectation template-filter guard	auto-lifecycle
M20: drop the expectation canonical-id guard	auto-lifecycle
M21: route the expectation through ordinary admission	auto-lifecycle
M22: drop the "publication still owns" guard	pool clone
M23: drop the template-manager operand of the quarantined-clone guard	pool clone
M24: drop the expectation operand of the quarantined-clone guard	pool clone
M25: drop the clone cleanup-fence check	pool clone
M26: skip the clone re-arm	pool clone
M29: flip the clone blocking branch	pool clone

For M15 and M16, target resolution and admission each refuse a template-manager peer on a non-template sandbox on their own. Removing the flag from only one layer is masked by the other, so these mutations remove it from both.

Coverage of the scoped functions

Measured with cargo-llvm-cov 0.9.1 on toolchain 1.98.0 over the full cargo nextest run -p pplx-space-sbx-mgr suite. The pinned toolchain's llvm-cov export segfaults on these binaries, so as a workaround the export used the nightly llvm-tools llvm-cov reader while profiles were merged with 1.98.0's llvm-profdata. A line counts as covered by LLVM's line rule, restricted to each function's source span in service/grpc_core.rs. "Before" uses the starting commit's test file. The tests that construct a service run in a child process (see Host isolation), and their profiles are collected.

Function	Before	After
stop	48/111 (43.2%)	90/111 (81.1%)
delete_with_auto_lifecycle_expectation	93/178 (52.2%)	159/178 (89.3%)
delete	7/7	7/7
finish_unpersisted_delete_cleanup	11/13	11/13
create (whole function)	32/267 (12.0%)	30/267 (11.2%)
create through template lookup	25/35	23/35

The starting commit's auto-lifecycle Local case covered create's auto-lifecycle require_local_authority_peer call and the line before it. This PR defers that case (see Follow-ups), so those two lines are no longer covered. The rest of create's uncovered pre-lookup lines are the lookup itself and lines reached only after every pre-lookup check passes.

Still uncovered in stop and delete: the unpersisted stop/delete path (stop lines 638–657, delete lines 850–865), the stop event trigger (line 605), and two race-only arms in clone disposal: the template recheck under the fence (line 783) and a failed re-arm (line 788).

Host isolation

• Environment: every test that constructs a service first runs itself in a child process via crate::service::tests::enter_route_test_process. The child clears EPPO_SDK_KEY, SPACE_FLAGD_ENABLED, SPACE_NODE_ID, SPACE_NODE_ROLE and the route-fence override, so service construction cannot start a feature-flag provider. Other environment variables are inherited.

• Service construction: services are built from fully explicit options, never SandboxManagerServiceOptions::default(), whose node identity falls back to reading /space/etc/node-id. Instead:

  • the node id is fixed;
  • the host-control and credential-manager sockets are absent paths under a test-owned temporary directory;
  • runtime liveness reads the test-owned run directory, the same directory stop's liveness check uses.

  Construction then reads environment variables, and resolves the six peer account names (space-nodeapi, space-tmplmgr, space-nodectl, space-sbxmgr, dev, deploy) through the host user database (getpwnam, which can read /etc/passwd via NSS). Both are read-only, and it reads no SPACE host files or sockets.

• Create: no case and no listed mutation reaches resolve_template_paths. The refusal that bars each is in the tables above.

• Node cleanup: stop and delete reach node cleanup only through the cleanup executor's existing #[cfg(test)] dispatch, so the guest flush and node-manager calls are never reached. Each fixture's dispatch waits for permits the test grants. The runtime-liveness probe reads a test-owned temporary run directory. The executor task runs on the service's task tracker and is drained at the end of every case.

• Non-destructive reads of fixed paths:

  • Every admitted stop or ordinary delete removes watchdog monitoring (an admitted clone-disposal delete returns before that step). That stats the fixed sidecar pid path /space/run/firecracker/sandboxes/<id>/space-netgtw-svc.pid for the fresh sandbox id. Nothing exists there, and with no committed gateway pid nothing is probed.
  • The "absent sandbox" case stats the fixed sandbox cache path of a fresh UUID, which does not exist.

• Successful stops and deletes notify the process-global dirty-outbox Notify, which simply records a permit (no worker waits on it in tests).

• Waits: every wait on cleanup is bounded by 30 s. That covers blocking RPC calls, the non-blocking calls, the clone-disposal delete, wait_cleanup_job, the wait for the completion dirty ref, and the fixture's shutdown drain. These bounds also cover correct runs: the executor wakes on notify with a one-second tick fallback, so the bound leaves room for a loaded CI runner. A regression that blocks on held node cleanup fails instead of hanging.

Findings

• Clone-disposal delete ignores x-space-skip-cluster-sync for its dirty ref. The ordinary delete path adds the admission-time dirty ref only when node publish is allowed (grpc_core.rs:901–903). The pool-clone disposal branch calls mark_sandbox_dirty unconditionally (grpc_core.rs:819). To reproduce:

  1. Insert an armed pool clone (a Creating row with a DeletePoolClone cleanup episode, as in insert_armed_pool_clone).
  2. Quarantine it with crate::db::mark_quarantined_from_state.
  3. With no dispatch permit granted, call Delete { blocking: false } from root with x-space-skip-cluster-sync: true.

  The call returns the id, and crate::db::list_node_dirty_refs already contains it. Today this only marks the sandbox dirty earlier than cleanup completion would, which marks it dirty regardless. No caller is affected: no current caller sends this header to Delete. The tests do not assert this behavior either way.

Follow-ups

• Needs a template-resolver seam (deferred). resolve_template_paths connects to the fixed template-manager socket, so these create cases have no later pre-lookup barrier and are deferred:
  • the auto-lifecycle Local authority from a forwarded peer (PermissionDenied);
  • an unspecified or unknown auto-lifecycle authority (InvalidArgument);
  • a Cluster auto-lifecycle accepting control;
  • everything after template lookup: intent resolution, pool adoption, requested-id reuse, create_with_id.
• Needs a seam: unpersisted stop/delete. When admission.is_unpersisted_for_sandbox(..) is true, the cleanup goes through run_cleanup_after_persistence_failure. That path flushes the guest and connects to node-manager directly, bypassing the executor's test dispatch.
• Needs a seam: clone-disposal races. The template recheck under the fence (check_cleanup_selection) and the Aborted "clone disposal hold changed" result each require a concurrent writer between target resolution and the fenced re-arm. Testing them needs an interleaving hook.
• The stop event trigger is not covered: these tests deliberately don't assert on events or other telemetry.
• The skip-header completion wait polls with yield_now() until the dirty ref appears. It ends within microseconds on a correct run, but under a regression it spins one core until the 30 s bound. A short async sleep between checks would be gentler.

Validation

Check	Result
cargo fmt --all --check	pass
cargo clippy --locked --all-targets --all-features -p pplx-space-sbx-mgr -- -D warnings	pass
cargo nextest run --locked -p pplx-space-sbx-mgr --retries 0 -E 'test(/grpc_core::tests/)'	11 passed
cargo nextest run --locked -p pplx-space-sbx-mgr --retries 0, 3 consecutive runs, SPACE_CI_REQUIRE_TEST_DEPS=1, repository Postgres setup script live	3/3 runs on the rebased head: 1528 passed, 1 skipped (an existing #[ignore] test that needs root)
Mutation sweep (33 scratch edits, each reverted)	33/33 detected; no mutation log contains a template-manager connection attempt. Run on the previous base; grpc_core.rs, the cleanup, create-support, cache-cleanup, activation-support and watchdog modules, service/mod.rs and service/tests.rs are identical on the current base
Gitleaks pre-commit hook; public-safety review of the staged diff	pass; no findings
Kind	not run (no production code changes)

AI assistance: these tests and this description were written with Claude Code.

🤖 Generated with Claude Code

[the-gigi-pplx]

Review — 4 findings

Medium

1. crates/libs/sbx/mgr/server/src/service/grpc_core/tests.rs (deleted create_rejects_local_auto_lifecycle_from_forwarded_service_peer) — the auto-lifecycle require_local_authority_peer check (grpc_core.rs:193) has no RPC-layer test left.
   On correct code, the old test never reached template lookup: a Local authority from a forwarded peer is refused at :193, before resolve_template_paths at :201. It was dropped because a mutation removing :193 would fall through to the host lookup. I deleted :193 and all 11 grpc_core tests still passed. This check is what stops a forwarded peer from persisting a Local auto-lifecycle policy, so I'd keep the test, built on isolated_service, and accept the lookup risk only under that mutation. The other option is to hold the deletion until the template-resolver seam lands. Today the body only says "deferred".

Low

2. grpc_core/tests.rs:190 and :445 — the !sandbox_exists asserts can't fail. Create writes no row before resolve_template_paths (grpc_core.rs:167-200), and the barrier stops every case before that point. So the table's "No row is created" pins nothing. I'd drop the asserts, or say in a comment that they hold by construction.
3. grpc_core/tests.rs:768-775 and :949-969 — these cases assert only the status code, but production returns that code from more than one branch. Internal covers both "requires one exact sandbox id" and "requires a canonical sandbox id" (grpc_core.rs:726, :731). FailedPrecondition covers both "publication still owns this sandbox" and "clone disposal hold requires an explicit operator delete" (:766, :772). A regression that sends a case into its sibling branch passes. Matching the message, as the create cases already do at :188, would pin the branch.
4. grpc_core/tests.rs:573 — the test hard-codes "x-space-skip-cluster-sync" instead of using SKIP_NODE_PUBLISH_METADATA (node_state_sync.rs:18). If the key is ever renamed, the test silently sends the wrong header, and the dirty-ref assertion fails with a misleading cause. Making the constant pub(crate) keeps the two tied.

It's tests-only. The only file is grpc_core/tests.rs, and the removed lines are the two old forwarded-create tests. The pool-clone delete branch marking the sandbox dirty regardless of publish_node_state (grpc_core.rs:819, versus the gate at :901) is pre-existing, and the body already lists it under Findings.

Verified against bf4c74f93f6ade8888dc972a6670651c4913f561. Reproduced: cargo test -p pplx-space-sbx-mgr --lib -- grpc_core::tests, 11/11 at head. I applied 10 production mutations, one at a time, and 9 were caught:

• stop's running-only selection
• both publish gates forced on
• the canonical-id guard dropped
• the quarantined-clone expectation guard dropped
• cleanup never marking the sandbox dirty
• the network-policy authority check dropped
• stop's cleanup permit dropped
• the clone cleanup-fence check dropped

The one that survived was the auto-lifecycle authority check (finding 1). Findings 2–4 are from reading.

[the-gigi-pplx]

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

[ac-pplx]

Thanks. On the four findings:

1. Auto-lifecycle Local authority from a forwarded peer. The template-resolver seam has landed as sbx-mgr: inject the template-manager resolver #299, stacked on sbx-mgr: inject the node-manager client transport and template-manager resolver #296. It adds grpc_core/create_template_lookup_tests.rs, which covers this case against a fake resolver, along with unspecified and unknown authorities and the Cluster and Local-from-local accepting controls. So the RPC-level test returns there, with no host-lookup risk under mutation.
2. !sandbox_exists asserts. Agreed, they hold by construction. Follow-up: drop them or comment them.
3. Status-only cases where one code comes from two branches. Good catch; matching the message would pin the branch. Follow-up.
4. SKIP_NODE_PUBLISH_METADATA. Agreed. Follow-up: make it pub(crate) and use it.

I'll fold items 2–4 into the next sbx-mgr test change, which moves these tests onto the new seams.