[ac-pplx]

Phase 3's last step: Substrate's Direct and Kubernetes variants are now private to crates/libs/api/src/controllers/substrate/, so a new match on them in shared code (tests included) fails to compile. The cluster controller's leader-election choice, the one remaining "is this Kubernetes?" check, now reads a capability. Behavior-preserving: no change to production behavior, wire formats, schema, metrics or configuration.

What changed

• Substrate is now pub struct Substrate { inner: Inner } over a private enum Inner { Direct(..), #[cfg(feature = "kubernetes")] Kubernetes(..) } (substrate/mod.rs:1701, :1706). Only controllers::substrate and its submodules can name Inner or read inner.
• Constructors Substrate::from_direct(DirectSubstrate) and Substrate::from_kubernetes(KubernetesSubstrate) (mod.rs:1714, :1722) replace Substrate::Direct(Box::new(..)) / Substrate::Kubernetes(Box::new(..)). Building a Substrate is not a branch, so the constructors stay pub; the cluster controller's tests are in another crate.
• Inside the module, the 8 inherent methods and the 42 arms of impl SubstrateAdapter for Substrate now match &self.inner (Inner::Direct(..) / Inner::Kubernetes(..)) instead of self. No arm's body changed.
• Shared code already used the phase 3 API: capabilities() for static facts, SubstrateAdapter/inherent methods for behavior, as_kubernetes() for Kubernetes-only state. api: ask the substrate for capabilities instead of matching on it #368 and api, cluster-controller: reach Kubernetes-only state through one accessor #386 converted every production variant match outside the module (api: reject node drain and cordon on the Direct substrate #505 later added supports_node_drain the same way), so with the variants private the compiler flagged only construction sites (below).
• New capability contends_for_leader_lease (mod.rs:1074; DIRECT false, KUBERNETES true). The cluster controller's lib.rs:150 was match substrate.as_kubernetes() { Some(_) if le_config.enabled => … }: it discarded the payload and built its lease client from the environment, so it only asked which substrate was running. It now reads if le_config.contends(substrate.capabilities()). LeaderElectionConfig::contends (leader_election/mod.rs:439) is self.enabled && capabilities.contends_for_leader_lease, the same decision as before, and both arm bodies are unchanged. A table test, contention_follows_the_substrate_and_the_kill_switch, covers both substrates with election on and off.
• Two archcheck pattern_ratchet instances (config only, ARCHITECTURE.toml, next to direct_tables; modes as rootfs_copy's: no_crossing, listed_no_growth, stale_entries required, no_new_entries/no_raised_caps advisory).
  • kubernetes_access counts every \bas_kubernetes\s*\( call outside crates/libs/api/src/controllers/substrate/** against architecture-baseline.toml. Today that is one call, reconcilers.rs = 1, which binds and uses the payload. A new caller in any form (wrapped chain, if let Some(_), matches!, .is_some_and(, .map(..).is_some()) needs a visible, reviewed baseline entry.
  • capability_identity rejects \bSubstrateCapabilities\s*::\s*(?:DIRECT|KUBERNETES)\b in production code outside direct.rs and kubernetes/adapter/substrate_adapter.rs. SubstrateCapabilities derives PartialEq and its consts are public, so capabilities() == SubstrateCapabilities::KUBERNETES would be another identity check. Tests are outside archcheck's inventory and still compare with the consts.
• SubstrateAdapter::managed_by() still names the running substrate publicly, for SQL row scope and wire labels. The Substrate doc comment says so rather than claiming shared code never learns which substrate is running.
• Docs: the substrate AGENTS.md now says the variants are private (Inner in mod.rs), how shared code builds a Substrate, how to add a capability (field with a doc comment, set in both DIRECT and KUBERNETES, read through capabilities()), that as_kubernetes() must bind and use its payload (Some(_), .is_some() and .is_none() are identity checks that need a capability instead), and that the running managed_by() is never parsed to decide behavior, apart from the sanctioned SQL-scope early return (failed_active_execution_scope, transient_lifecycle_sweeper/store.rs). Its test guidance names the constructors, and the "existing substrate matches are debt" note is gone because none remain. The api crate's AGENTS.md points at capabilities() / as_kubernetes() instead of Substrate::Direct. docs/architecture.md's substrate overview names the adapters, not the variants, so it is unchanged.

Site table (this PR)

Line numbers before are at this PR's base, 17cd654. The replacement is on the same logical line.

file:line (before)	before	replaced by	why
api/src/controllers/substrate/mod.rs:1685	pub enum Substrate { Direct(Box<..>), Kubernetes(Box<..>) }	pub struct Substrate { inner: Inner } + private enum Inner	variants matchable/constructible only inside the module
substrate/mod.rs:1737, :1745 (from_env_with_lifecycle)	Self::Kubernetes(Box::new(s)), Self::Direct(Box::new(DirectSubstrate::new(..)))	Self::from_kubernetes(s), Self::from_direct(..)	construction through the constructors
substrate/mod.rs:1697–1906 (8 inherent methods: as_kubernetes, live_node_names, try_resolve_node_api_endpoint, reserve_/persist_rollback_route_identity, readiness, eligible_start_node_count, set_node_schedulability)	match self { Self::Direct(..) .. }	match &self.inner { Inner::Direct(..) .. }	in-module forwarding; behavior unchanged
substrate/mod.rs:2290–2945 (impl SubstrateAdapter for Substrate, 42 methods)	match self { Self::Direct(s) => s.op(..), Self::Kubernetes(s) => s.op(..) }	same arms over &self.inner	in-module forwarding; behavior unchanged
cluster/controller/server/src/lib.rs:150	match substrate.as_kubernetes() { Some(_) if le_config.enabled => …, _ => … }	if le_config.contends(substrate.capabilities()) { … } else { … }	identity check → new contends_for_leader_lease capability
api/src/test_support.rs:62	Substrate::Direct(Box::new(DirectSubstrate::new(..)))	Substrate::from_direct(..)	test fixture construction
api/src/routes/public/sandboxes/admission_test_fixture.rs:166	Substrate::Kubernetes(Box::new(substrate))	Substrate::from_kubernetes(substrate)	test fixture construction
api/src/controllers/sandbox/lifecycle/tests.rs:216,290,543,751,842,924	Substrate::Direct(Box::new(..))	Substrate::from_direct(..)	test construction
api/src/controllers/sandbox/lifecycle/wake_completion_postgres_tests.rs:63,68,211	Substrate::Direct(Box::new(..)), Substrate::Kubernetes(Box::new(..)) ×2	from_direct, from_kubernetes	test construction (#597's fence table; its content is otherwise unchanged)
api/src/controllers/sandbox/{ingress/tests.rs:288, queries/create_postgres_tests.rs:499, spaced/tests.rs:244, updates/postgres_tests.rs:75, updates/transport_tests.rs:262}	Substrate::Direct(Box::new(..))	Substrate::from_direct(..)	test construction
api/src/controllers/template/tests.rs:191,399,678	Substrate::Direct(Box::new(..))	Substrate::from_direct(..)	test construction
api/src/public_sandbox_operations/tests.rs:69,154,765	Direct, Kubernetes, Direct	from_direct, from_kubernetes, from_direct	test construction
api/src/routes/admin/{node_maintenance/tests.rs:168, nodes/drain_handler_tests.rs:70, nodes/tests.rs:2079, sandboxes/tests.rs:578}	Substrate::Kubernetes(Box::new(..))	Substrate::from_kubernetes(..)	test construction
api/src/routes/e2b/tests.rs:344,388,4023	Substrate::Kubernetes(Box::new(..))	Substrate::from_kubernetes(..)	test construction
substrate/tests.rs:671,675,714,734 (714/734 are #597's node_addressing table), substrate/runtime_rpc/tests.rs:14, substrate/direct/tests.rs:2754,2956	Substrate::Direct/Kubernetes(Box::new(..))	constructors	in-module test construction; the constructors keep one spelling across the tree
substrate/kubernetes/tests.rs (18 sites: 3047, 3093, 5610, 5820, 5941, 5970, 6013, 6070, 6135, 6199, 6773, 9415, 9479, 9557, 9616, 9624, 9680, 9687), kubernetes/tests/substrate_scope.rs:37,44, kubernetes/adapter_characterization_tests/hydration_and_suspend.rs:348	Substrate::Direct/Kubernetes(Box::new(..))	constructors	in-module test construction
cluster/controller/server/src/warm_pool/tests.rs:159,211,887,981,1340	Substrate::Direct(Box::new(DirectSubstrate::new(..)))	Substrate::from_direct(..)	test construction (another crate)
cluster/controller/server/src/public_operation_dispatcher/tests.rs:697,810	Substrate::Direct(Box::new(..))	Substrate::from_direct(..)	test construction
cluster/controller/server/src/transient_lifecycle_sweeper/lifecycle_writer_tests.rs:55	Substrate::Direct(Box::new(..))	Substrate::from_direct(..)	test construction

65 construction sites in total: 57 in the api crate, 8 in the cluster controller. None of them matched a variant. Which tests use Direct as a fixture is phase 4's job; this PR changes only how they build it.

The phase 0 inventory's 33 checks

git show origin/ac/direct-substrate-inventory:reports/direct-substrate-inventory/ lists 33 category-d rows outside the module: 14 decision sites (7 Substrate::{Direct,Kubernetes} matches plus 7 string/typed comparisons). None of the 7 variant matches remains, and none of the 7 comparisons matches the Substrate enum.

inventory row(s)	was	now (at this head)	removed by
sandbox/lifecycle.rs:366–368	Substrate::Direct(_) => DirectRestoreBaseline, Kubernetes(_) => Enforced	capabilities.caller_completes_claimed_restore (lifecycle.rs:367)	#368
sandbox/wake_completion.rs:95–97	shield_applies matched the variant	capabilities.caller_completes_claimed_restore (wake_completion.rs:101)	#368
sandbox/wake_completion.rs:256–258	Direct(_) => Ok(true), Kubernetes(kube) => hydrate	SubstrateAdapter::ensure_routable_execution (wake_completion.rs:259)	#386
fork_operation_executor.rs:61–63	CopyNodeEndpointRequirement by variant	capabilities().node_addressing (fork_operation_executor.rs:794)	#368
cluster/controller/server/src/lib.rs:145	Substrate::Kubernetes(kube) if le_config.enabled	#386 made it as_kubernetes() with Some(_), still an identity check; this PR makes it the contends_for_leader_lease capability (lib.rs:150)	#386, this PR
cluster/controller/server/src/reconcilers.rs:391,394	Direct(_) => skip, Kubernetes(kube) => spawn	substrate.as_kubernetes() (reconcilers.rs:393)	#386
transient_lifecycle_sweeper/store.rs:952–954	Direct(_) => Routable, Kubernetes(kube) => hydrate	SubstrateAdapter::ensure_routable_execution (store.rs:992)	#386
sandbox/queries.rs:606	managed_by != "k8s"	capabilities.resolves_cold_create_shape (queries.rs:626)	#368
routes/admin/node_maintenance.rs:431–432 + 5 callers	require_direct_substrate: managed_by() != "space"	require_admin_node_registry: capabilities().registers_nodes_via_admin_api (node_maintenance.rs:198)	#368
sandbox/spaced.rs:75	row managed_by == "k8s"	typed SubstrateKind parse of the row (spaced.rs:76)	#506; row-level, the SubstrateKind sibling task's scope
routes/session_auth.rs:31–33	"k8s" => K8s	typed SubstrateKind parse (session_auth.rs:31). It maps the running substrate (ConsoleSubstrate::from_managed_by(state.sandbox_controller.managed_by()), session_auth.rs:143) to the console's wire label: a deliberate wire-mapping exception, not a row-level check	#506
routes/admin/node_maintenance.rs:1210	node.managed_by == NodeManagedBy::Space	unchanged (node_maintenance.rs:554)	row-level, sibling scope
cluster/trigger-queue/src/lib.rs:265	target.managed_by == NodeManagedBy::Kubernetes	unchanged (lib.rs:266)	row-level, sibling scope
sandbox/store/src/lib.rs:485	self.managed_by == "k8s"	typed SubstrateKind parse (lib.rs:521)	#383; row-level, sibling scope

Not in the inventory: transient_lifecycle_sweeper/store.rs:1199,1231,1262 call failed_active_execution_scope(self.substrate.managed_by()) (store.rs:153, added by #373 after the inventory). It reads the running substrate's label to choose a fixed SQL scope, or to return early: the sanctioned SQL-scope exception named in the substrate AGENTS.md.

Of the remaining rows, spaced.rs, node_maintenance.rs:554, trigger-queue and the sandbox store check a row's managed_by. session_auth.rs and the sweeper's scope helper read the running substrate's managed_by(), one for a wire label and one for SQL scope. None names a Substrate variant, so making the variants private cannot reach them. SQL literals, sbx-mgr and SubstrateKind row methods are other tasks' scope and untouched here.

Integration with open PRs

Whichever lands second fixes these:

• api: run the shared lifecycle tests on Kubernetes too #603 adds crates/libs/api/src/test_support/kubernetes.rs, which builds Substrate::Kubernetes(Box::new(substrate)), and conflicts textually with this PR in public_sandbox_operations/tests.rs. If api: run the shared lifecycle tests on Kubernetes too #603 lands first, that line becomes Substrate::from_kubernetes(substrate), and the helper whose doc calls it "the one place tests … name a Substrate variant" is inlined or deleted.
• Ask SubstrateKind row-level questions instead of matching the substrate #602 keeps the substrate AGENTS.md bullet this PR rewrites ("Don't match Substrate::Direct … Matching Substrate::Kubernetes(k) … is fine") and appends a SubstrateKind bullet. The merged file keeps this PR's bullets and Ask SubstrateKind row-level questions instead of matching the substrate #602's new bullet, and drops the stale text. Ask SubstrateKind row-level questions instead of matching the substrate #602's wording becomes "mapping a SubstrateKind variant to a value … may match it", since a bare "variant" is ambiguous next to "Substrate's variants are private".

Compile-fail proof

A scratch crates/libs/api/src/substrate_match_probe.rs (declared from lib.rs, outside the module; not committed), checked with cargo check -p pplx-space-api --all-features:

use crate::controllers::substrate::Substrate;

pub fn is_direct(substrate: &Substrate) -> bool {
    match substrate {
        Substrate::Direct(..) => true,
        _ => false,
    }
}

error[E0599]: no associated function or constant named `Direct` found for struct `substrate::Substrate` in the current scope
    --> crates/libs/api/src/substrate_match_probe.rs:5:20
     |
   5 |         Substrate::Direct(..) => true,
     |                    ^^^^^^ associated function or constant not found in `substrate::Substrate`
     |
    ::: crates/libs/api/src/controllers/substrate/mod.rs:1701:1
     |
1701 | pub struct Substrate {
     | -------------------- associated function or constant `Direct` not found for this struct

Reaching for the inner enum instead (use crate::controllers::substrate::{Inner, Substrate}; matches!(&substrate.inner, Inner::Direct(_))) fails with error[E0603]: enum Inner is private and error[E0616]: field innerof structsubstrate::Substrate is private. The repository does not use trybuild, so there is no compile-fail test.

Review round 1: finding → change

finding	change
MAJOR: lib.rs:150 used as_kubernetes() as Some(_), a pure identity check; as_kubernetes().is_none() was an escape hatch	new SubstrateCapabilities::contends_for_leader_lease (doc comment; DIRECT false, KUBERNETES true), read through LeaderElectionConfig::contends; same decision; table test; mutation-checked
MAJOR: AGENTS.md rule for as_kubernetes() / managed_by()	as_kubernetes() must bind and use its payload; Some(_)/.is_some()/.is_none() need a capability; never parse the running managed_by() to decide behavior, except failed_active_execution_scope
MAJOR: "never which substrate it is" overclaimed	Substrate doc and this body now say managed_by() still names it for SQL row scope and wire labels
MAJOR (optional): archcheck ratchet	(replaced in round 2 by kubernetes_access and capability_identity) pattern_ratchet.kubernetes_identity in ARCHITECTURE.toml, its own table next to direct_tables, no baseline, required; a probe line makes cargo arch exit 1
MAJOR: brownout smoke	run on a fresh Kind cluster (below)
MINOR: inventory table misclassified running-substrate reads	session_auth.rs marked as a running-substrate wire-label mapping; sweeper's failed_active_execution_scope (#373) added as the sanctioned SQL-scope exception; summary sentence rewritten
MINOR: #603 / #602 integration	neither has merged; noted under "Integration with open PRs"
NIT: stale "Substrate enum" comments	reconcilers.rs:380 → "Substrate", transient_lifecycle_sweeper/store.rs:1 → "the substrate"

Review round 2: finding → change

finding	change
MINOR: line-pattern kubernetes_identity misses wrapped chains, .is_some_and(, matches!, if let Some(_), .map(..).is_some(); a trailing comment could fail an unrelated PR	replaced with kubernetes_access, a call-count ratchet with a baseline (reconcilers.rs = 1), plus capability_identity for the const comparison
MINOR: substrate AGENTS.md contradicted itself on early returns	failed_active_execution_scope is the only managed_by() parse; new fixed-scope SQL returns early by reading a capability or reusing that helper, never a new parse; the ratchet sentence says exactly what each instance counts
LOW: call-site gap at lib.rs:150	noted below
NIT: "match arm" comment in lib.rs	now "branch"

Validation

Round 2 (config, docs, a comment) at head 3112b47, base 17cd654. No Kind, per the review.

check	result
cargo fmt --all -- --check, workspace clippy -D warnings, pre-commit (gitleaks)	pass
cargo arch on the tree	exit 0, 0 required findings; kubernetes_access matches equal the baseline (reconcilers.rs 1), capability_identity matches only the allowed files
cargo arch --base-baseline <main's> --base-architecture <main's>	0 required; the seeded reconcilers.rs entry shows as advisory no_new_entries, as the archcheck README expects for a new instance
scratch: wrapped chain s\n.as_kubernetes()\n.is_some_and(..) in cluster-controller lib.rs	kubernetes_access.no_crossing (required), exit 1
scratch: if let Some(_) = s.as_kubernetes() in lib.rs	kubernetes_access.no_crossing (required), exit 1
scratch: s.capabilities() == SubstrateCapabilities::KUBERNETES in lib.rs	capability_identity.no_crossing (required), exit 1
scratch: a second call, matches!(s.as_kubernetes(), Some(_)), in the baselined reconcilers.rs	kubernetes_access.listed_no_growth (required): "grew from 1 to 2", exit 1
cargo nextest run -p pplx-space-archcheck	55/55
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run -p pplx-space-cluster-controller-server --all-features --run-ignored all --retries 0	1009/1009

Round 1 at head b3ab233 (same code, with the line-pattern rule):

check	head	result
cargo fmt --all -- --check	b3ab233	pass
cargo clippy --workspace --locked --all-targets --all-features -- -D warnings	b3ab233	pass
cargo check -p pplx-space-api --no-default-features	b3ab233	pass, no warnings
cargo build -p pplx-space-api-svc	b3ab233	pass
pre-commit (gitleaks)	b3ab233	pass
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-api -p pplx-space-cluster-controller-server --all-targets --all-features --run-ignored all --ignore-default-filter --retries 0 ×3	b3ab233	3/3 runs: 2842 passed (api 1833, cluster-controller-server 1009 including the new test), 0 failed, 0 skipped; characterization tests unchanged
mutation: DIRECT.contends_for_leader_lease = true	b3ab233	contention_follows_the_substrate_and_the_kill_switch fails ("direct substrate, election enabled: true")
mutation: KUBERNETES.contends_for_leader_lease = false	b3ab233	same test fails ("kubernetes substrate, election enabled: true"); restored code passes
cargo arch + cargo nextest run -p pplx-space-archcheck	b3ab233	0 required findings, kubernetes_identity matches nothing; archcheck tests 55/55. A scratch s.as_kubernetes().is_none() in reconcilers.rs gave pattern_ratchet.kubernetes_identity.no_crossing (required) and exit 1
compile-fail probe (above)	b3ab233	E0599 as shown; E0603/E0616 for Inner/inner
Kind round C: fresh cluster (kind get clusters empty → bootstrap.sh), SPACE_KIND_TELEMETRY_MODE=otel; API and cluster-controller log git_sha=b3ab233b9087; SPACE_K8S_CONTROL_PLANE=true; both controller replicas log "contending for leadership" and one "elected leader"	b3ab233	—
↳ smoke-kind-e2e.sh (SKIP_BOOTSTRAP=1)	b3ab233	7/7 pass
↳ local.leader_election_brownout_keeps_one_leader (the typed port of the brownout smoke from #593; the shell script no longer exists), run by name	b3ab233	pass, no cleanup failures, about 26 min
↳ smoke-kind-e2e.sh again	b3ab233	7/7 pass
typed suite	—	not rerun: the rebase onto 17cd654 pulled in no substrate changes (#590 template pins, #217 cached template lookups)

Earlier rounds, on the head before this review (same variant change, without the capability):

round	head	result
cargo matrix, nextest ×3	2540563, 4f4d4c0, 3ac9e5b	all pass; 2841/2841 each run
Kind A, fresh, otel	4f4d4c0	e2e ×2 pass; typed all ecw 78/78, native 24/24; fork pass
Kind B, fresh, otel	2540563	e2e ×2 pass; fork pass; typed ecw 75 pass + 2 fail + 1 infra error, native 24/24. The 2 failures (local.ecw_template_build_from_oci_boots, local.private_template_build_isolation: "publisher node UID is awaiting inventory synchronization") also failed Main CI's Kind shards at a0f5dc2 and come from #458's template-publication path, which main has since reverted (#606); the infra error (object-store port-forward) passed on rerun

Notes and decisions

• Name Inner, not SubstrateInner. archcheck's file_size rule caps substrate/mod.rs. With SubstrateInner:: the longer arms re-wrapped and the file grew by 59 code lines; with Inner:: it grows by 17, which is just the new struct and constructors (the new capability field adds 8 more). The Inner type is private, and the Substrate doc comment explains it. cargo arch reports no new findings for the touched files. The stale cap on mod.rs (2771) already existed on main and is left for the baseline owner.

• No #[cfg(test)] escape hatch. Tests outside the module use the same constructors and have no access to Inner.

• Call-site gap (pre-existing, no fix). Changing lib.rs:150 back to if le_config.enabled passes every unit test: the table test covers LeaderElectionConfig::contends, not its call site. No CI job runs the cluster controller on Direct, and election defaults to enabled.

• reconcilers.rs:393 keeps as_kubernetes(). Its Some(kube) arm binds and uses the payload (client, caches, pod store), which is the sanctioned use.

• One pre-existing warning. cargo check -p pplx-space-api --no-default-features --tests warns that the re-export pooled_prepare_error_is_reclaimable (controllers/sandbox/mod.rs:1427) is unused, because only kubernetes-gated tests use it. This PR doesn't touch that file, and CI does not build that configuration. The plain --no-default-features lib build is warning-free.

• Kind harness notes. (0) Round C: the first brownout attempt ran without SKIP_BOOTSTRAP=1, so its "bootstrap kind stack" phase started Tilt. Tilt replaced the Postgres pod (data kept on its PVC) and re-created the space-db-migrate Job, which hit the race below. I stopped that attempt and Tilt; the control-plane deployments were still the bootstrap's b3ab233 pods. The brownout and the second e2e then ran with SKIP_BOOTSTRAP=1, as CI's typed step does. (1) In rounds A and the 3ac9e5b round, starting Tilt over a bootstrap.sh-deployed stack re-created Postgres and the space-db-migrate Job together; the Job exhausted its backoff on "connection refused" and every dependent resource waited. A tilt trigger space-db-migrate once Postgres was up let the bring-up finish. This is a harness race from my bootstrap-then-Tilt sequence; CI's flow starts Tilt from the e2e bring-up. (2) Round B ran against the bootstrap.sh-deployed stack with SKIP_BOOTSTRAP=1 and no Tilt, because the Docker host had 11 GB free after the bring-up (under the 15 GB floor) and cleanup is not permitted while a cluster runs. Tilt's rebuilds would have added more. (3) local.retention_claim_finish_forward reported an INFRA_ERROR when the object-store port-forward lost its pod after an object-store pod replacement, then passed once the forward recovered.

AI assistance: this change was prepared with Claude Code.

🤖 Generated with Claude Code