[ac-pplx]

What merging does

Shared api code outside the substrate module stops asking "which substrate is this?" where it means "does this substrate support X?". SubstrateAdapter gets one required method, capabilities(), which returns a small Copy struct, SubstrateCapabilities. There is one constant per substrate, and the five api-crate branches this phase covers read their answers from it. Production behavior, wire formats, schema, metrics and configuration are unchanged: every field holds, for each substrate, the value the branch it replaces produced. Three behavioural tests cover the sites that had no coverage, and comments that misdescribed Direct's restore lifecycle are corrected.

This is phase 3, PR 1 of the plan to keep the Direct/Kubernetes split out of shared code. The cluster controller, the routability/redrive behavior, the row-level checks and making Substrate's variants private are left for later PRs.

The struct

crates/libs/api/src/controllers/substrate/mod.rs:

pub enum NodeAddressing { DirectEndpoint, SubstrateNodeId }

pub struct SubstrateCapabilities {
    pub caller_completes_claimed_restore: bool,
    pub node_addressing: NodeAddressing,
    pub registers_nodes_via_admin_api: bool,
    pub resolves_cold_create_shape: bool,
    pub supports_node_pool_placement: bool,
}

field	meaning	DIRECT	KUBERNETES
caller_completes_claimed_restore	Every restore-shaped wake (from suspended or restoring) claims restoring before its node call and leaves restoring → started to the caller's completion CAS. The caller can then fence a suspended row at completion as a newer suspend, and must shield the CAS against request cancellation.	false	true
node_addressing	Which NodeApiTarget field addresses a node; must match what the substrate's resolve_node_api_endpoint requires	DirectEndpoint	SubstrateNodeId
registers_nodes_via_admin_api	Nodes live in the admin node registry (create, update, delete, template pull and delete)	true	false
resolves_cold_create_shape	The control plane resolves a cold Firecracker create's shape: a fallthrough create takes a configured or policy pool shape, and otherwise vCPU and memory the request left unset come from the template's build shape	false	true
supports_node_pool_placement	Create requests may carry a node-pool placement pin	false	true

Why caller_completes_claimed_restore is false for Direct. Direct rows do become restoring, so the reason is not that they never do. Direct doesn't guarantee that every restore-shaped wake claims restoring first and leaves the completion to the caller:

• Its explicit restore, and a wake that falls back to it, go through restore_execution_with_candidates. That path claims restoring under its own lease (claim_direct_restore_lease) and writes started itself (complete_direct_restore_lease) before returning.
• A placed worker-local wake (wake_sandbox) doesn't touch lifecycle. It leaves the pre-wake lifecycle for the caller's completion CAS to advance: suspended, or restoring kept by an earlier lease restore that released its lease, timed out or failed to persist its receipt.

So on Direct, a suspended row at completion can be the wake's own baseline, which the completion must advance, not fence. The shield's doc and two lifecycle.rs comments said Direct "never flips the row out of suspended" or "doesn't touch lifecycle at all"; they now describe both Direct paths. A cancelled Direct wake leaves suspended, started, or a restoring row that a later restore reclaims once any lease expires.

• DirectSubstrate::capabilities() returns SubstrateCapabilities::DIRECT and KubernetesSubstrate::capabilities() returns SubstrateCapabilities::KUBERNETES.
• impl SubstrateAdapter for Substrate has a forwarding arm for capabilities(), which the new tests exercise (they call it on Substrate).
• The method has no default, so a substrate can't silently inherit an answer.
• Every field has a consumer in this PR.
• supports_node_pool_placement() used to be a trait method (default false, overridden to true by Kubernetes, forwarded by Substrate). Its one caller, the admin create route, converted cleanly, so the method folds into the struct. SandboxController::supports_node_pool_placement() becomes SandboxController::substrate_capabilities().

Site table

All paths are under crates/libs/api/src/.

site	before	after	Direct	Kubernetes	why it is the same
controllers/sandbox/lifecycle.rs PublishedSuspendFence (Wake/ImplicitWake and Restore arms)	match substrate { Direct => DirectRestoreBaseline, Kubernetes => Enforced }	for_capabilities(caps): if caps.caller_completes_claimed_restore { Enforced } else { DirectRestoreBaseline }	false → DirectRestoreBaseline	true → Enforced	same variant for each substrate
controllers/sandbox/wake_completion.rs shield_applies (same two arms)	restore_shaped && match substrate { Direct => false, Kubernetes => true }	restore_shaped && caps.caller_completes_claimed_restore	false	true	same bool for each substrate
fork_operation_executor.rs copy-target completeness	copy_node_endpoint_requirement(&substrate): Direct => DirectEndpoint, Kubernetes => SubstrateNodeId (private CopyNodeEndpointRequirement)	substrate.capabilities().node_addressing (NodeAddressing, same two variants)	DirectEndpoint	SubstrateNodeId	same variant; the private enum and helper are removed
routes/admin/node_maintenance.rs create_node, update_node, delete_node, pull_node_template, delete_node_template	require_direct_substrate: managed_by() != "space" → 501	require_admin_node_registry: !caps.registers_nodes_via_admin_api → the same 501 and message	"space" → allowed / true	"k8s" → 501 / false	managed_by() is "space" exactly for Direct
controllers/sandbox/queries.rs resolve_cold_create_resources	ColdCreateShapeInput { managed_by }, managed_by != "k8s" → requested shape	ColdCreateShapeInput { capabilities }, !caps.resolves_cold_create_shape → requested shape	"space" → early return / false	"k8s" → pool or template shape / true	managed_by() is "k8s" exactly for Kubernetes
routes/admin/sandboxes.rs create_sandbox	sandbox_controller.supports_node_pool_placement()	sandbox_controller.substrate_capabilities().supports_node_pool_placement	false (trait default)	true (override)	same values as the removed method

Left as they are, deliberately:

• routes/admin/node_maintenance.rs update_node's node.managed_by == NodeManagedBy::Space tests the row, not the running substrate.

• routes/session_auth.rs ConsoleSubstrate::from_managed_by (the console /me label) needs a typed substrate kind, and none is reachable here without one of three changes, each rejected:

  • parsing managed_by()'s string, which adds a fallible step that the current total mapping doesn't have;
  • changing managed_by()'s return type, which this PR must not do;
  • copying the kind into the capability struct, which would create a second source for the same label.

  It becomes a one-line From once the sibling SubstrateKind work lands and managed_by() returns it.

Coverage by mutation

For each field, I flipped its value in one substrate's constant, ran cargo nextest run --locked -p pplx-space-api --all-features --retries 0 --no-fail-fast (Postgres and Redis started, SPACE_CI_REQUIRE_TEST_DEPS=1), recorded what failed, then reverted. The unmutated baseline in the same scratch copy passed 1734/1734.

field	flip	failing test	coverage
caller_completes_claimed_restore	KUBERNETES true → false	controllers::sandbox::lifecycle::wake_completion_postgres_tests::wake_completion_fence_follows_the_running_substrate (new)	the field's value and PublishedSuspendFence::for_capabilities are covered; uncovered: the shield_applies consumer and the two arms' call sites, see below
caller_completes_claimed_restore	DIRECT false → true	same (new)	covered
node_addressing	KUBERNETES → DirectEndpoint	none	uncovered, see below
node_addressing	DIRECT → SubstrateNodeId	none	uncovered
registers_nodes_via_admin_api	KUBERNETES false → true	routes::admin::node_maintenance::tests::node_registry_delete_follows_the_running_substrate (new)	covered
registers_nodes_via_admin_api	DIRECT true → false	same (new)	covered
resolves_cold_create_shape	KUBERNETES true → false	controllers::sandbox::queries::create_postgres_tests::postgres_kubernetes_create_fills_unset_resources_from_the_template_shape (existing)	covered
resolves_cold_create_shape	DIRECT false → true	same (existing)	covered
supports_node_pool_placement	KUBERNETES true → false	routes::admin::sandboxes::tests::admin_pinned_create_follows_the_running_substrate (new)	covered
supports_node_pool_placement	DIRECT false → true	same (new)	covered

The first registers_nodes_via_admin_api/KUBERNETES run could not start its Postgres container (Postgres did not become ready within 30 seconds) because the Docker disk was full at that moment (see Validation). That was ENVIRONMENTAL: after the prune, the rerun failed the new test as expected.

Uncovered consumers of caller_completes_claimed_restore. The new test calls PublishedSuspendFence::for_capabilities directly, and no test calls shield_applies. Reverting shield_applies, or either arm's (Wake/ImplicitWake, Restore) fence selection, to a literal would still pass the suite. Observing them means cancelling a wake after dispatch on each substrate and checking whether the redrive ran. No existing fixture does that, and this PR adds none.

node_addressing stays uncovered. The read happens inside the fork executor's artifact-copy step. Observing it means driving a fork operation to that step, where a source row with an incomplete active-node target either reuses the target or falls back to select_ready_worker_node_api_target. No existing fixture reaches that point on both substrates without node RPCs. The active_copy_node_target_is_complete unit tests pin each variant's rule.

New tests

Each runs against a real DirectSubstrate and, under #[cfg(feature = "kubernetes")], the store-backed Kubernetes test substrate (kubernetes::tests::substrate_with_store), wrapped in Substrate. Each asserts an observable outcome, not a capability value. They use only the repository's Postgres test fixture, and the Kubernetes substrate's client is the existing inert test client. The two admin tests run in a child test process (enter_test_process) that sets SPACE_K8S_CONTROL_PLANE=false, so AppState::from_pool builds Direct whatever the host environment says and never reaches a real cluster. The admin-create test also sets SPACE_ALLOW_SANDBOX_PLACEMENT=true there.

• wake_completion_fence_follows_the_running_substrate: picks the fence through PublishedSuspendFence::for_capabilities(substrate.capabilities()) and runs complete_wake_lifecycle on a suspended row. Kubernetes leaves the row suspended; Direct advances it to started.
• node_registry_delete_follows_the_running_substrate: calls the delete_node handler for a nonexistent node as the managing-org admin. Direct reaches the registry lookup (404 Node not found); Kubernetes gets the unchanged 501 and message.
• admin_pinned_create_follows_the_running_substrate: with placement enabled, calls the admin create_sandbox handler with a node_pool pin. Direct gets 400 (sandbox node-pool placement requires the Kubernetes substrate) and nothing is queued; Kubernetes gets 202, and the queued public_operations payload carries the pin.

Existing tests are moved, never rewritten to pass. They change only where a signature changed, and their assertions are untouched:

• The cold-create shape test passes SubstrateCapabilities::KUBERNETES / DIRECT (bound to local k8s / space) where it passed "k8s" / "space".
• The fork copy-target tests name NodeAddressing::….
• The two hand-written SubstrateAdapter doubles in create_postgres_tests.rs implement capabilities().

The api characterization tests (#273, #290, #326) are untouched and pass.

Decisions made unattended

1. Constants live on the struct, as SubstrateCapabilities::DIRECT / KUBERNETES, rather than on each substrate type. Both answers sit side by side, and the cold-create test can name the Kubernetes answer without the kubernetes feature. KUBERNETES is pub and compiled unconditionally; it is plain data.
2. capabilities() is required and has no default. With a default, a substrate that forgot to override it would silently answer like the other one.
3. Field names describe what the consumers rely on: caller_completes_claimed_restore (re-derived from the fence and the shield, see above), and resolves_cold_create_shape (the gate covers both the pool shape and the template build shape).
4. ColdCreateShapeInput takes the whole SubstrateCapabilities, not just one bool, so its test reads resolve(k8s, …) rather than resolve(true, …).
5. require_direct_substrate is renamed require_admin_node_registry, because it now asks a capability question. The status (501) and the message are unchanged.
6. PublishedSuspendFence::DirectRestoreBaseline keeps its name: existing tests name it.
7. The console /me label is left as is, for the reasons above.
8. architecture-baseline.toml is not edited. cargo arch advises lowering fork_operation_executor.rs's cap, because this change shrinks that file. The finding is advisory, and the shared baseline is a conflict point for other PRs.

Overlaps with open PRs

• Record where a sandbox create's shape came from #237 (create shape source) conflicts textually in controllers/sandbox/queries.rs and create_postgres_tests.rs. Its new test builds ColdCreateShapeInput { managed_by: "k8s", … }. It also changes resolve_cold_create_resources to return a third value, Option<CreateShapeSource>, which flows into completion events and metrics. Whichever lands second must keep both: the capability-based selector (capabilities: SubstrateCapabilities, !capabilities.resolves_cold_create_shape) and Record where a sandbox create's shape came from #237's three-value return with its telemetry propagation.
• api: document the Direct substrate's contract for agents #357 (adds controllers/substrate/AGENTS.md). Its "For new work" bullet says "ask a capability such as supports_node_pool_placement". After this PR, that is a SubstrateCapabilities field. Whichever lands second should make the bullet say: add a field to SubstrateCapabilities, set it in both DIRECT and KUBERNETES, and read it via capabilities().
• PoC: claim BYOK warm sandboxes by switching Firecracker drives #336, Remove legacy Falcon policy integration after caller migration #191, api: decide admission pool claims for the create's owner; collect refused claims promptly #168, Refuse credential retirement while effective policies still bind it #166 also touch controllers/sandbox/queries.rs and/or create_postgres_tests.rs, but in other hunks. The two test doubles gain a method right after managed_by(); api: decide admission pool claims for the create's owner; collect refused claims promptly #168 edits NodeRpcStubbedKubernetes's impl elsewhere.
• Bound each TCP gRPC connect attempt and client TLS handshake #360, Bound reconnect recovery on unpublished local suspends #350, api: decide admission pool claims for the create's owner; collect refused claims promptly #168, fix(sandbox): wait before retrying failed activations #3 touch controllers/substrate/mod.rs in other hunks. This PR edits the trait head (where supports_node_pool_placement was) and the start of impl SubstrateAdapter for Substrate. api: decide admission pool claims for the create's owner; collect refused claims promptly #168 adds a trait method and its forwarding arm elsewhere.
• PoC: claim BYOK warm sandboxes by switching Firecracker drives #336, Bound reconnect recovery on unpublished local suspends #350 touch kubernetes/adapter/substrate_adapter.rs in other hunks.
• api: let tests inject the Kubernetes readiness and suspend budgets #330 (api Kubernetes readiness budget seam) landed after the original base. This PR is now rebased onto current main, which includes it (and internal-api: characterize node publication's lifecycle writers (tests only) #324, Keep scratch-build staging outside the shared cache #343, Run Kind's sandboxes in their own namespace, confined by NetworkPolicies #355, Run Kind's control plane highly available behind load-balanced entry #356, sbx-mgr: build every test service from explicit test options #322, sbx-mgr: test create after template lookup, unpersisted cleanup and endpoint capture on the seams (tests only) #327). No interaction.

Validation

check	head	result
cargo fmt --all -- --check; ci/scripts/run-rust-checks.sh fmt (env-contract and error-policy checks included)	final	pass
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings	final	pass
pre-commit (gitleaks), on the commit and over the commit range	final	pass
cargo nextest run --locked -p pplx-space-api --all-features --retries 0, SPACE_CI_REQUIRE_TEST_DEPS=1, three consecutive runs	final	1734/1734 passed each time; 0 dependency skips (the one skipped test is the opt-in benchmark_single_node_occupancy)
The three new tests with SPACE_K8S_CONTROL_PLANE=true in the parent environment	final	pass (the child process overrides it)
Mutation proofs of the two admin tests (registers_nodes_via_admin_api and supports_node_pool_placement, each flipped for each substrate)	final	each flip fails its test; baseline passes
Mutation runs for the other fields (table above)	previous	as tabled
cargo arch; source guards; scripts/check-rust-error-policy.sh	previous	cargo arch exit 0, advisory findings only. Guards pass, except thin_service_mains.py and check_no_inline_rust_test_modules.py, which fail only on files outside this diff
Kind: fresh cluster on the current harness (HA control plane, sandbox namespace), scripts/tests/smoke-kind-e2e.sh	previous	pass
Kind provenance	previous	The build sources were byte-identical to the checkout. Both space-api and both space-cluster-controller pods ran binaries hash-identical to the fresh artifact image.
scripts/tests/smoke-kind-typed.sh all (SPACE_SMOKE_KIND_COLD_RECEIPTS set as CI does)	previous	ECW 77 passed and 1 ENVIRONMENTAL failure; native 23/23 (native_contract.command_replay_credentials is IGNORED by design). The failure, local.telemetry_reaches_local_backends, happened because Tempo hit no space left on device after the devbox Docker disk filled. It passed on its one rerun on a second fresh cluster (baseline e2e passed again, same binary hashes).
Docker disk	previous	Only docker builder prune -af (12.92 GB and 8.64 GB freed) and docker image prune -f (0 B) were used.

"Final" is this head. "Previous" is the preceding head on the same base. The only differences between the two are comment wording and the two admin tests' child-process environment, so production binaries are unchanged and Kind was not rerun.

Full Kind validation

Run after review on a fresh Kind cluster at 6f1dc66 (the tip of #386, which includes #368 at 251e4c7). Deployed api, internal-api and cluster-controller binaries matched the build's hashes before and after the suites.

Suite	Result
smoke-kind-e2e.sh (start and end)	pass, pass
smoke-kind-typed.sh all (unsharded, CI's env)	ECW surface 76/78, native 23/23. The two non-passes (local.node_group_boundary_schedules, local.rate_limiters_enforce_and_fail_open) lost a request during the scenario's own api rollout; both passed on rerun and on main.
smoke-kind-sandbox-fork.py	pass
smoke-kind-leader-election-brownout.sh	pass, all three scenarios, including the apiserver outage
smoke-spaced-rest.sh, run-local-smokes.sh all	pass
smoke-space-m1-conformance.sh	fails identically on main (a malformed sandbox id returns 404, not 400); pre-existing, fixed separately

🤖 Generated with Claude Code

[zibi-pplx]

Approving. I traced all six sites, and each substrate gets exactly the behavior it had before:

• the wake fence and the cancellation shield;
• fork node addressing;
• the node-registry 501;
• the cold-create shape;
• admin placement.

Making capabilities() required with no default is the right call. The branch merges cleanly with main. None of the suggestions below blocks merging.

Suggestions

1. node_addressing duplicates what resolve_node_api_endpoint requires.
   • The field (substrate/mod.rs:2166) restates what each substrate's resolve_node_api_endpoint needs. direct.rs:4175 reads direct_endpoint; kubernetes/adapter/node_eligibility.rs:224 requires substrate_node_id.
   • Nothing ties the two together. As the PR notes, flipping the field fails no test.
   • Options: make it a trait method next to resolve_node_api_endpoint (e.g. node_api_target_is_complete), or add one test per substrate that resolves a target carrying only the declared field. Direct's resolve is pure.
2. caller_completes_claimed_restore names one protocol, but on Direct the two consumers are false for different reasons.
   • The worker-local wake keeps suspended as its baseline. The lease restore has already written started.
   • A related window, which predates this PR and is worth an issue:
     • In the Restore arm on Direct, complete_direct_restore_lease writes started.
     • complete_wake_lifecycle then runs with DirectRestoreBaseline ($2 = FALSE, lifecycle.rs:393-412).
     • So a suspend that fully finishes in between is overwritten back to started.
   • The window is narrow and not a regression. The rewritten comment above that update (lifecycle.rs:1884-1899) describes the Direct restore path but doesn't flag the window.
3. The new wake test mostly repeats an existing one.
   • wake_completion_fence_follows_the_running_substrate largely re-covers wake_completion_cas_outcome_for_every_prior_lifecycle (same file, :48).
   • It calls for_capabilities itself, so the Wake and Restore call sites stay uncovered.
   • Per the space-code-quality test policy, running [DIRECT, KUBERNETES] through for_capabilities inside the existing table test would be leaner.

Nits

• substrate/mod.rs:2174: "Without it, the requested shape goes to the node unchanged" isn't true for Direct one-shot creates. Those drop vcpu/memory, because sbx-mgr's CreateRequest has no shape fields. "the control plane passes the requested shape through unchanged" is accurate.
• wake_completion_postgres_tests.rs:71-72 still says "a direct restore keeps suspended as the baseline". That contradicts the corrected comments: it's the worker-local wake that keeps it.
• Missing from the "left for later" list: lifecycle.rs:1225 (delete_observed_sandbox) defaults a missing or unparseable node managed_by to NodeManagedBy::Space.
• The body says "five api-crate branches", but the site table lists six sites, covered by five fields.