Repository navigation
Bind the last substrate literals and forbid new ones with archcheck - #611
Merged
Merged
Conversation
ac-pplx
force-pushed
the
ac/substrate-literals-rule
branch
2 times, most recently
from
October 17, 2026 06:22
711ff14 to
0a96a29
Compare
Control-plane Postgres `sandboxes.managed_by` and `nodes.managed_by` hold the substrate that owns a row, 'k8s' or 'space'. After #373, #383 and #506 the remaining production literals were the shared routability fragment, the Kubernetes adapter's builder lookup and the Direct route-identity match, plus Direct's create-target claim and release, which landed on main since. They now bind `SubstrateKind` or match its parsed variant, and a new archcheck instance keeps new literals out of production code. No schema, wire format, metric or configuration changes, and no behavior change. - common-svc: `routable_execution_or_non_k8s(alias, kubernetes_placeholder)` writes `{alias}.managed_by <> {placeholder}` instead of `<> 'k8s'`. Each caller binds `SubstrateKind::Kubernetes` at the placeholder it numbers after its own parameters: the transient sweeper's aged-transient list and restoring converge (`$5`), internal-api's restored-state convergence (`$4`) and the api's wake-completion CAS (`$2`). - api Kubernetes adapter: `builder_client_for_node` binds `n.managed_by = $2`. - api Direct: `claim_create_target` and `release_create_target` bind `managed_by = $3` (`Space`). - api Direct: the route identity's `substrate_kind` text is parsed as `SubstrateKind` in `route_identity_substrate_kind`, which maps exactly 'space' and 'k8s' as before and rejects any other text with the same error. - ARCHITECTURE.toml: `pattern_ratchet.substrate_literals` catches the common literal forms in production code: SQL comparisons of `managed_by` with 'k8s'/'space' (=, <>, !=, IS [NOT] DISTINCT FROM, [NOT] IN, = ANY, same-line CASE ... WHEN; any case, alias, prefix, quoting, cast or wrapping call), and Rust comparisons with the label (raw strings included), a `NODE_MANAGED_BY_*` const or a variant's `.as_str()`/`.to_string()`, matches on the label string, and match arms on "k8s"/"space" beside a substrate name. Its comment lists what it misses and its false positives. It allows test-support modules and fixtures that archcheck's filename-based test rule counts as production. On the parent commit it matches the seven lines this commit converts; on this commit it matches nothing, so it has no baseline table. no_crossing, listed_no_growth and stale_entries are required; no_new_entries and no_raised_caps are advisory, so a deliberate exception is a reviewed baseline entry. - Tests: Postgres-backed twins for each converted statement. New Kubernetes builder-lookup and Direct create-target tests, a unit table for the route-identity labels, and an otherwise identical 'space' row added to the sweeper's converge, internal-api's restored convergence and the wake-completion redrive tests. Test callers of the sweeper's shared builders bind the extra parameter. - Docs: `docs/rust/space-conventions.md`, the substrate AGENTS.md and the space-rust-development skill say every crate's production SQL binds the value; in Rust a decision about a row's substrate calls a `SubstrateKind` method named for the question, a fact about the running substrate reads `capabilities()`, and code never compares the label string; and the archcheck rule catches the common literal forms. Plugin version 0.2.33 -> 0.2.34. Converted statements' text changes, so their sqlx statement-cache keys and query signatures change; statements below an added `.bind` move to new `sql!` locators.
ac-pplx
force-pushed
the
ac/substrate-literals-rule
branch
from
October 17, 2026 06:22
0a96a29 to
9a18529
Compare
ac-pplx
marked this pull request as ready for review
October 17, 2026 06:46
kumarpallav-pplx
approved these changes
Oct 17, 2026
ac-pplx
added a commit
that referenced
this pull request
Oct 19, 2026
…ate comments #611 and #602 each added the same row-decision bullet to docs/rust/space-conventions.md, so it appeared twice; keep the copy with the examples. Correct two comments #602 added: - `wakes_after_failed_control_connect`: Direct's access path wakes a suspended or restoring sandbox (`DirectSubstrate::access_runtime_client`), not every "dormant" sandbox as the Kubernetes bullet defines it. - `REGISTRY_SUBSTRATE`: every registry write binds it and `verify` proceeds only for a live node of it; the registry's reads return nodes of every substrate.
ac-pplx
added a commit
that referenced
this pull request
Oct 19, 2026
…comments #611 and #602 each added the same row-decision bullet to docs/rust/space-conventions.md, so it appeared twice; keep the copy with the examples. Correct comments #602 added: - `wakes_after_failed_control_connect`: Direct's access path wakes a suspended or restoring sandbox (`DirectSubstrate::access_runtime_client`), not every "dormant" sandbox as the Kubernetes bullet defines it. - `REGISTRY_SUBSTRATE`: every registry write to `nodes`/`node_direct` binds it and `verify` proceeds only for a live node of it; the registry's reads, and the template pull and delete routes, act on nodes of every substrate. - The registry's `substrate_scope` test module header, which said the registry reads only Direct nodes, now describes what it tests: writes and `verify`.
meefs
pushed a commit
to meefs/SPACE
that referenced
this pull request
Oct 20, 2026
…comments (perplexityai#657) perplexityai#611 and perplexityai#602 each added the same row-decision bullet to docs/rust/space-conventions.md, so it appeared twice; keep the copy with the examples. Correct comments perplexityai#602 added: - `wakes_after_failed_control_connect`: Direct's access path wakes a suspended or restoring sandbox (`DirectSubstrate::access_runtime_client`), not every "dormant" sandbox as the Kubernetes bullet defines it. - `REGISTRY_SUBSTRATE`: every registry write to `nodes`/`node_direct` binds it and `verify` proceeds only for a live node of it; the registry's reads, and the template pull and delete routes, act on nodes of every substrate. - The registry's `substrate_scope` test module header, which said the registry reads only Direct nodes, now describes what it tests: writes and `verify`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of containing the Direct/Kubernetes substrate split, last PR: the same method as #373 (cluster controller), #383 (internal-api, trigger queue, sandbox store) and #506 (api crate). Based on
main(9af70ec). One commit.What merging does
Control-plane Postgres
sandboxes.managed_byandnodes.managed_byhold the substrate that owns a row,'k8s'or'space'. After #373, #383 and #506, these production sites still spelled it:routable_execution_or_non_k8sfragment (crates/libs/common/svc/src/sql/sandbox_routability.rs), which wrote{alias}.managed_by <> 'k8s'into four statements in three crates;n.managed_by = 'k8s');"space" =>/"k8s" =>);managed_by = 'space'), which Preserve authenticated scope in cached template lookups #217 added while this PR was open.They now bind
SubstrateKindor match its parsed variant. A new archcheck instance,pattern_ratchet.substrate_literals, catches the common forms of a new literal in production code.no_crossing,listed_no_growthandstale_entriesarerequired.no_new_entriesandno_raised_capsareadvisory, so a deliberate exception goes in as a reviewed baseline entry, as in the sibling instances.There are no schema, wire format, metric or configuration changes, and no behavior change.
SubstrateKindencodes as the column's text, so a boundSubstrateKind::Kubernetesmatches exactly what'k8s'did.Converted sites
Each new placeholder is numbered after the statement's existing parameters, and its
.bindcomes last.cluster/controller/server/src/transient_lifecycle_sweeper/store.rslist_aged_transient_query(routable_executionselect-list fact)<> 'k8s'→routable_execution_or_non_k8s("sandboxes", "$5"),$5=Kubernetesconverge_restoring_query(converge CAS)"$5",$5=Kubernetesinternal/api/server/src/published_state.rspublish_sandbox_statesrestored-state convergenceroutable_execution_or_non_k8s("s", "$4"),$4=Kubernetesapi/src/controllers/sandbox/wake_completion.rsfenced_completion_cas(cancelled-wake redrive)"$2",$2=Kubernetesapi/src/controllers/substrate/kubernetes/adapter/substrate_adapter.rsbuilder_client_for_nodelookupn.managed_by = 'k8s'→$2=Kubernetesapi/src/controllers/substrate/direct.rsclaim_create_target(#217)managed_by = 'space'→$3=Spacerelease_create_target(#217)managed_by = 'space'→$3=Spaceapi/src/controllers/substrate/direct.rsload_active_direct_route_identity'ssubstrate_kindmatchmatch substrate_kind.as_str() { "space" => Direct, "k8s" => Kubernetes, other => Err }→route_identity_substrate_kind(&str) -> Option<SandboxSubstrateKind>, which parsesSubstrateKindand matches the variantroutable_execution_or_non_k8s(alias, kubernetes_placeholder), as cluster-controller: bind the substrate value instead of writing it in SQL #373 did forrow_state_guards. Every caller is listed above (rg routable_execution_or_non_k8sfinds no others). Its doc comment, which explained why the literal was left as debt, now says what the argument is. The fragment stays a fragment. Inlining it into four statements would give the routability contract four copies, which is what the shared definition exists to prevent.NodeManagedBy::parseis an exact match on"space"and"k8s", with no trimming or case folding. Any other text, including"SPACE"," k8s"and"", still returns the sameSubstrateError::Failedwith the stored text in the message.{substrate_kind:?}formats theStringexactly as{other:?}formatted the&str. The mapping moved into a small free function so a unit table can pin every input. Postgres can't produce the'k8s'or unknown arms, becausesandbox_direct_route_identities_substrate_kind_checklimits the column to'space'.publisher_node_for_identity(template_releases_store.rs,n.managed_by = 'k8s'). This PR converted it, with a relabelled-builder case intenant_publication_rejects_invalid_grantsthat failed its predicate-removal mutation. Revert "Enforce tenant publication grants after preparation rollout (#458)" #606 then reverted Enforce tenant publication grants after preparation rollout #458, so the rebase drops both: those two files are identical tomain.managed_by()label ("k8s") and Direct's ("space") are labels the adapters return, not comparisons; the rule doesn't match them.Not converted, by design
crates/libs/sbx/mgr/server/src/route_activation_claims.rssubstrate_kind = 'k8s': node-local sbx-mgr SQLite, not control-planemanaged_by(sbx-mgr'sNodeManagedBy/route substrate is a separate, node-side notion). The rule's SQL patterns namemanaged_byonly.crates/libs/node/mgr/server/src/slot_pool/liveness.rscomponent == "space": a component name, not a substrate value.#[cfg(test)]support modules and fixtures keep writing literal rows (see the rule'sallow).The rule:
[rules.pattern_ratchet.substrate_literals]ARCHITECTURE.tomlonly, as the archcheck README's "Adding an instance" describes. The patterns, by group:(The
# S1… labels are only for this table;ARCHITECTURE.tomlhas the patterns without them.)managed_bycompared with'k8s'/'space'by=,<>,!=orIS [NOT] DISTINCT FROM, on either side (S1, S2). The column may have an alias, a prefix (node_managed_by) or quoting (n."managed_by"), a::textcast or a wrapping call (lower(…)). "Either side" applies to these binary comparisons only. For lists,managed_by [NOT] IN (…)or= ANY(…)with a literal in the list counts (S3; one level of nested call, as in(lower($1), 'k8s')), and so does'k8s' [NOT] IN (…)namingmanaged_by(S4).CASE managed_by WHEN '…'counts on one line (S5).SET managed_by = '…'counts too.managed_byorsubstratecompared with"k8s"/"space", raw strings included. Forms:==/!=either side,Some("…"),.eq,.eq_ignore_ascii_caseandmatches!.NODE_MANAGED_BY_*const or with aSubstrateKind/NodeManagedByvariant's.as_str()/.to_string(). Those were the likeliest way around a flagged== "k8s"; the consts stay public in this PR.matchon a*managed_by/substrate_kindlabel string (.as_str()/.as_deref()/.as_ref()). M2: amatchon the adapter'smanaged_by()."k8s"/"space"("k8s" =>,Some("k8s") =>,("k8s", x) =>,"k8s" | … =>,"k8s" if … =>) only when a substrate name (substrate,Substrate,ManagedBy,managed_by) is on the same line. That keeps the arm patterns off the non-substrate"k8s"/"space"vocabularies: auth mode, the metricsbackendtag, the nftables table, the cgroup component, the lease namespace and the runtime class.allow: archcheck's test files are filename-based (tests.rs,*_tests.rs,tests/), so#[cfg(test)]support modules count as production. The allow list names them explicitly:**/*_tests/**,**/test_support.rs,**/test_support/**,**/*_test_support.rs,**/test_fixtures.rs,**/*_test_fixture.rs,**/scripted_test_doubles.rsandcrates/libs/db-test-fixtures/**. Fixtures are not converted. I checked each*_tests/directory undersrc/(published_state_tests,adapter_characterization_tests,k8s_store_postgres_tests): all are test modules.//and///lines don't count (archcheck's code-line definition).no_crossing,listed_no_growthandstale_entriesarerequired.no_new_entriesandno_raised_capsareadvisory, likekubernetes_access,capability_identity,rootfs_copyandnode_manager_commands: a deliberate exception is a reviewed baseline entry, not a widerallow. There is no baseline table, since production code has no matches.VALUESlist, split across lines, built withformat!, or held in a non-.rsfile;if let "k8s" = …andlet … elsepatterns;["space", "k8s"].contains(…);row.get::<String, _>("managed_by") == "k8s");|m| m == "k8s");eq_ignore_ascii_case("K8S"));match &*x.managed_by,&x.managed_by[..]or.as_deref().unwrap_or(…), and so its arms;matchline, or with any of;{}=:between the literal and=>(a guard with==,("k8s", NodeRole::Builder), a struct pattern).//or/* */comment, in#[doc = "…"], in a SQL--comment, or in an ordinary Rust string such as an error message;.as_ref()/.as_deref()match on a typedOption<SubstrateKind>/Option<NodeManagedBy>namedsubstrate_kindor*managed_by.What the rule matches, before and after
substrate_literalsresultc1187fb(this task's starting commit) with thisARCHITECTURE.tomlno_crossing(required) findings, 5 lines:direct.rs1026–1028 (the route-identitymatch substrate_kind.as_str() {header and its two arms),substrate_adapter.rs1902,sandbox_routability.rs61. Without theallowlist, the test-support modulepublished_state_tests/lifecycle_writers.rsadds 3.17cd654(mainwhile #458 was in)direct.rs3119/3142 (#217) andtemplate_releases_store.rs150 (#458)9af70ec(this PR's base) with thisARCHITECTURE.tomlno_crossing(required) findings, 7 lines:direct.rs5 (1026–1028, 3107, 3130),substrate_adapter.rs1902,sandbox_routability.rs61. That is the 6 converted lines plus the header of the converted route-identity match, which the new M1 pattern catches.cargo archexits 1.cargo archexits 0.The rule caught #217's and #458's new literals as soon as I rebased, which is the ratchet working. The only
'k8s'/'space'left in production.rsSQL (outside comments, test files and the allowed support modules) is sbx-mgr's node-localsubstrate_kind = 'k8s'.Synthetic probe. A scratch production file (
crates/libs/common/svc/src/sql/zz_probe.rs, deleted afterwards) held 100 lines: 52 positives, 35 look-alikes, the 11 listed misses and the 2 listed false positives. A script checked every line against every pattern (rg, the same regex engine). Every positive matched its own pattern; no look-alike or listed miss matched anything; both listed false positives matched. archcheck counted exactly 54 lines (52 + 2).n.managed_by = 'k8s';MANAGED_BY<>'space';!= 'k8s';IS DISTINCT FROM 'space';SET managed_by = 'space';managed_by::text = 'k8s';lower(managed_by) = 'k8s';n.node_managed_by = 'k8s';n."managed_by" = 'k8s'managed_by = $2;pool.managed_by = 'autoscaler';role = 'k8s''k8s' = s.managed_by;'space' <> lower(s.managed_by);'k8s' = n."managed_by"'k8s' = s.roleIN ('space', 'k8s');not in ('k8s');IN (lower($1), 'k8s');= ANY(ARRAY['k8s']);"managed_by" IN ('k8s')IN ($2, $3);IN ($2) AND kind = $3;= ANY($1)'k8s' IN (n.managed_by);'space' NOT IN (managed_by, other)'k8s' IN ($1);'k8s' IN (n.role)CASE s.managed_by WHEN 'k8s'CASE s.managed_by WHEN $1;CASE s.role WHEN 'k8s'row.managed_by == "k8s";.as_str() != "space";node_managed_by == "k8s";.as_deref() == Some("k8s");s.substrate.managed_by() == "k8s";substrate_kind == "space";== r"k8s";== r#"space"#component == "space";mode == "k8s";mode == r"k8s";managed_by == SubstrateKind::Kubernetes"k8s" == managed_by;Some("k8s") == row.managed_by.as_deref();r#"k8s"# == row.managed_by"k8s" == modemanaged_by.eq_ignore_ascii_case("k8s");managed_by.eq("space");managed_by.eq(r"space")mode.eq_ignore_ascii_case("k8s")matches!(row.managed_by.as_str(), "k8s");matches!(row.managed_by.as_deref(), Some("k8s" | "space"))matches!(row.managed_by.parse::<SubstrateKind>(), Ok(SubstrateKind::Kubernetes));matches!(mode, "k8s")row.managed_by == NODE_MANAGED_BY_K8S;managed_by != types::NODE_MANAGED_BY_SPACE;.as_str() == SubstrateKind::Kubernetes.as_str();label == NodeManagedBy::Space.to_string()NODE_MANAGED_BY_K8S => Ok(Self::Kubernetes),(the parser);.bind(SubstrateKind::Kubernetes.as_str());kind == SubstrateKind::Kubernetes;pub const NODE_MANAGED_BY_K8S: &str = "k8s";NODE_MANAGED_BY_K8S == row.managed_by;SubstrateKind::Kubernetes.as_str() != managed_bylet substrate_kind = match substrate_kind.as_str() {(main'sdirect.rs1026);match row.managed_by.as_deref() {;match row.node_managed_by.as_ref() {match row.managed_by.parse::<SubstrateKind>() {;match row.managed_by {;match row.managed_by.as_str().parse::<SubstrateKind>() {;match mode.as_str() {;match substrate_outcome.as_ref() {;match self.substrate.as_ref() {;match substrate_capabilities.as_ref() {match self.substrate.managed_by() {"space" => proto::SandboxSubstrateKind::Direct,;Some("k8s") => SubstrateKind::Kubernetes,;("k8s", true) => NodeManagedBy::Kubernetes,;"k8s" | "kubernetes" => Substrate::from_kubernetes(k),;"k8s" if strict => Substrate::from_kubernetes(k),"k8s" => AuthMode::K8s,;"space" => Table::Space,;"k8s" => 1,match managed_by { "k8s" => 1, _ => 0 }NodeManagedBy::Kubernetes => "k8s",; one-linematch k { NodeManagedBy::Space => "space", NodeManagedBy::Kubernetes => "k8s" }if let "k8s" = row.managed_by.as_str() {;let "k8s" = row.managed_by.as_str() else { … };["space", "k8s"].contains(&row.managed_by.as_str());row.get::<String, _>("managed_by") == "k8s";|m: &str| m == "k8s";row.managed_by.eq_ignore_ascii_case("K8S");match &*row.managed_by {;match &row.managed_by[..] {;match row.managed_by.as_deref().unwrap_or("space") {;("k8s", NodeRole::Builder) => SubstrateKind::Kubernetes,;"k8s" if role == "builder" => SubstrateKind::Kubernetes,.match substrate_kind.as_ref() {on a typedOption<SubstrateKind>(M1), and an error messageformat!("expected managed_by = 'k8s' for {id}")(S1).Precision across the whole tree. I ran the patterns over every
.rsfile, tests included. On this head they match 24 code lines, all in test files (C1/C2 match nothing anywhere, tests included) and all substrate values (fixtureSET managed_by = …writes andmanaged_by == "k8s"branches). The lines they don't match that mentionmanaged_byorsubstrate_kindwith a literal are inserts, binds, assignments and assertions. The exception is sbx-mgr's node-localsubstrate_kind = 'k8s', which is out of scope as above.Evidence
A Postgres-backed test per converted statement
Each test shows a row of the statement's substrate on one side of the predicate and an otherwise identical row of the other substrate on the other. For the fragment, the discriminating case is a row without a routable execution: the
'k8s'row must not converge, and the'space'row must, because only Kubernetes has the routability contract. All run on the shared nextest Postgres withSPACE_CI_REQUIRE_TEST_DEPS=1.cluster-controller transient_lifecycle_sweeper::tests::postgres_backed::r17_candidate_select_resolves_routable_execution_fact: an unroutable'k8s'restoring row listsroutable_execution = false; a'space'restoring row with no execution liststrue…::r18_converge_cas_requires_routable_execution_at_flip_timecreatinggeneration with no pod IP) now has an identical'space'twin. The Kubernetes sweeper's CAS claims nothing for the'k8s'row; the Direct sweeper's CAS converges the twin tostarted.internal-api tests/published_state.rs publishing_owned_started_sandbox_leaves_unroutable_restoring_lifecycle'space'restoring row with the samecreatingexecution, publishedstartedin the same census, converges; the'k8s'row staysrestoringapi controllers::sandbox::wake_completion::tests::redrive_cas_refuses_unroutable_execution_at_flip_time'k8s'(redriveAbandoned, row staysrestoring) and an identical'space'row (Completed,started). Theinsert_k8s_sandboxhelper now delegates to amanaged_by-parameterised one.api controllers::substrate::kubernetes::tests::substrate_scope::builder_lookup_reads_only_kubernetes_nodesnode_k8srow and a ready builder node-api pod in the cache. The'space'one is refused as "not registered"; the'k8s'one gets past the lookup and dials a local listener. Bounded at 5 s, with the dial in anAbortOnDropHandle(the pattern of #506's D12).api controllers::substrate::direct::tests::substrate_scope::create_target_claims_only_direct_rowscreating, unplaced sandboxes that differ only inmanaged_by. The claim sets the'space'row's target and refuses the'k8s'row withConflict, leaving it NULL. With both rows given the same target, the release clears only the'space'row.api controllers::substrate::direct::tests::substrate_scope::route_identity_substrate_kind_reads_only_the_exact_labels"space"→ Direct,"k8s"→ Kubernetes;"direct","kubernetes","SPACE","K8S"," k8s","space ",""→ noneTest plumbing: the sweeper tests that run
list_aged_transient_query/converge_restoring_querydirectly bind the extra$5(6 call sites), andsandbox_routability/tests.rspasses the new argument.Predicate removal
Each mutation changes one production site; then I ran the owning package's whole suite (
cargo nextest run --locked -p <package> --retries 0 --no-fail-fast,SPACE_CI_REQUIRE_TEST_DEPS=1,--all-featuresfor api) and reverted. P1a/P1b first ran without--all-featuresfor internal-api and were rerun with it, with the same results; that crate's publication tests compile only with a feature. For a fragment call site there are two mutations, each made through the placeholder argument:"$n OR TRUE": the predicate accepts every row."$n AND FALSE": the substrate exemption is dropped and the fragment reduces to the routabilityEXISTS."Before" is this PR's production code with
main's tests plus the bind plumbing. "After" is this head.$5 OR TRUEr17_candidate_select_resolves_routable_execution_fact$5 AND FALSE$5 OR TRUEr18_converge_cas_requires_routable_execution_at_flip_time$5 AND FALSEr18…(new twin), plus existingr10_converge_cas_respects_restore_leaseandlifecycle_writer_tests::converge_cas_moves_only_restoring_rowsr18)$4 OR TRUEpublishing_owned_started_sandbox_leaves_unroutable_restoring_lifecycle$4 AND FALSEpublishing_owned_started…(new twin), plus existingpublished_state_tests::node_publication_converges_only_from_its_source_lifecycles$2 OR TRUEredrive_cas_refuses_unroutable_execution_at_flip_time$2 AND FALSEredrive_cas_refuses…(new twin), plus existinglifecycle::characterization_tests::redrive_completes_only_a_restoring_row(n.managed_by = $2 OR TRUE)builder_lookup_reads_only_kubernetes_nodesSpace→Kubernetesroute_identity_substrate_kind_reads_only_the_exact_labels, plus existingdirect::tests::wake_noop_without_receipt_preserves_current_direct_identityandingress::tests::stored_ingress_access_resumes_paused_execution_before_reporting_readyDirectroute_identity_substrate_kind_reads_only_the_exact_labels(managed_by = $3 OR TRUE)create_target_claims_only_direct_rows(managed_by = $3 OR TRUE)On
main's tests, 9 of the 13 mutations failed a test; on this PR's tests all 13 do. (For #458's dropped lookup, the mutation was also passed → fails.) Each extended test now fails both mutations of its statement on its own.Plan cache: literal vs bound
Setup. A scratch database migrated from the test template on the repository's test image (PostgreSQL 16.15), with synthetic, deliberately skewed data:
'space'; Kubernetes nodes withnode_k8srows; a builder every 50th);'space'; 30% deleted, 40% started, 15% suspended, 5% restoring, 5% stopped, 3% failed, 2% pausing), across 60 orgs, one of which holds 40%;creatingwith no IP; and a retired generation for a third;ANALYZE.Each sequence ran on one connection with
plan_cache_mode = auto:PREPAREonce with the parameter types sqlx sends, then 30 ×EXPLAIN (ANALYZE, TIMING OFF) EXECUTE(writes each inBEGIN … ROLLBACK, so the data stays fixed), thenpg_prepared_statements. "Literal" is the statement text onmain, "bound" this PR's.Which statements. The fragment's Kubernetes-path callers:
The builder lookup is also measured.
D13/D14 (Direct-only) are primary-key updates. Their generic plans are identical apart from the filter text:
(managed_by = $3)for(managed_by = 'space'::text)under the sameid = $1scan. On a Direct deployment's databasesandboxes.managed_byis effectively single-valued, so 1/ndistinct equals the literal's frequency anyway (#506's argument).random()order), so it already replanned onmain.EXPLAIN (GENERIC_PLAN)) are identical, costs and row estimates included, for all five Kubernetes-path statements. The only difference is the filter text,(managed_by <> $n)instead of(managed_by <> 'k8s'::text), in P1, C1 and W1, and(managed_by = $2)instead of= 'k8s'in K1.id-driven index scan (Index Scan using sandboxes_id_org_id_key … Index Cond: (id = input.sandbox_id)for P1), ORed with the routabilityEXISTS, so its estimate can't change the access path.nodes.managed_by(idx_nodes_managed_by,idx_nodes_role (managed_by, role),idx_sandboxes_managed_by) are full, not partial, and a<>filter can't use them. So a generic plan has no index it could lose.No converted statement is worse bound, so no literal is kept as an exception. These are synthetic numbers on one PostgreSQL 16.15 instance; a production database's statistics decide the actual margins.
Characterization tests pass unchanged
The characterization suites pass in every run below:
adapter_characterization_tests(57) andlifecycle::characterization_tests(16, plus the 11 Kubernetes ones api: run the shared lifecycle tests on Kubernetes too #603 added), from api: characterize Kubernetes substrate suspend, pooled preparation and restore hydration (tests only) #273, tests: pin publication convergence and other lifecycle behavior from #272 #290, api: pin the pooled-setup error arms #273 called unreachable #326 and api: let tests inject the Kubernetes readiness and suspend budgets #330;lifecycle_writer_tests, from cluster-controller: characterize the lifecycle writers (tests only) #323;published_state_tests::lifecycle_writers(internal-api: characterize node publication's lifecycle writers (tests only) #324) andpublished_state_lifecycle_race_tests(internal-api: recheck the failed projection's guards under the row lock #337).None of their files, nor
lifecycle_writer_test_support.rsorscripted_test_doubles.rs, changes.Docs agents read
docs/rust/space-conventions.md, "Substrate values are bound parameters":managed_by()to decide behavior (api, cluster-controller: make the Substrate variants private to the substrate module #607's rule; the substrateAGENTS.mdnames the one SQL-scope exception). It sayspattern_ratchet.substrate_literalscatches the common forms, not all, and points at its comment for the gaps.SubstrateKindmethod named for the question; repeating a statement's fixed scope compares against the namedSubstrateKindconst the statement binds; matching a variant is only for mapping it to a value; a fact about the running substrate readscapabilities(); never compare the label string. It is Ask SubstrateKind row-level questions instead of matching the substrate #602's bullet word for word, except that it says "namedSubstrateKindconst" and leaves out Ask SubstrateKind row-level questions instead of matching the substrate #602's examples (proves_execution_absence,restores_through_pods,REGISTRY_SUBSTRATE), which don't exist until Ask SubstrateKind row-level questions instead of matching the substrate #602 merges.AGENTS.md. Itsspace-conventions.mdhunk is anchored on the "direct.rshas about a dozen" text this PR deletes, so whichever lands second gets a conflict there. Keep this PR's paragraph and Ask SubstrateKind row-level questions instead of matching the substrate #602's bullet with "namedSubstrateKindconst". With that one-word change, Ask SubstrateKind row-level questions instead of matching the substrate #602's bullet is a superset of this PR's bullet, which is dropped. Ask SubstrateKind row-level questions instead of matching the substrate #602'sAGENTS.mdbullet goes above the "Scoping a query" bullet this PR edits and doesn't overlap it.direct.rshas about a dozen" and the fragment-as-debt text are gone.SandboxRecord.managed_by/AutoLifecycleExecution.managed_byfields (stillStringonmain).row_state_guardssentence now also names the routability fragment as an existing fragment that takes its placeholder, not a pattern for new code.crates/libs/api/src/controllers/substrate/AGENTS.md: "Existing literals, such as the sharedroutable_execution_or_non_k8sfragment, are debt" → the archcheck rule catches the common literal forms in production code, and its comment lists what it misses. The rest of api, cluster-controller: make the Substrate variants private to the substrate module #607's new text in that bullet is kept.crates/libs/cluster/controller/server/AGENTS.md: unchanged. Onmainit already says to scope withmanaged_by = $n, never a literal, bound fromsubstrate.managed_by()or a fixedSubstrateKind(the Kubernetes arm ofrun_reconcilers). No text anywhere still tells a Kubernetes-only loop to writemanaged_by = 'k8s'.space-rust-developmentskill: its "remaining raw-literal sites … are existing debt" sentence is replaced. In Rust, a decision about a row's substrate calls theSubstrateKindmethod named for the question, a fact about the running substrate readscapabilities(), and code never compares the label string; it points at the conventions subsection. archcheck's rule catches the common literal forms in production code. The plugin version goes from 0.2.33 to 0.2.34 inplugin.yamland bothplugin.jsonfiles (.agents/plugins/AGENTS.md).claude plugin validatepasses; its one warning, a missingauthor, is also onmain.Decisions made unattended
routable_execution_or_non_k8sa fragment that takes the placeholderrow_state_guardsas an existing fragment, not a pattern.SubstrateKind::Kuberneteseverywhere$1 = 'space'and$5 = 'k8s', as the literal did.required, so the rebased head would otherwise fail its own check. Each conversion got a twin test and a mutation, as the others did. #458's was dropped again when #606 reverted it.otelmodeSPACE_KIND_TELEMETRY_MODE: otel(kind-ci.yml). My first runs used the harness default (datadog), which skips two otel-only scenarios, so the later runs match CI.route_identity_substrate_kindindirect.rs'k8s'and unknown arms unreachable through Postgres, so only a unit table can pin that the parse is identical for every input. The extraction is 16 lines besideparse_direct_route_identity. The call site keeps the same error and message.managed_byonly; Rust patterns namemanaged_by/substrate_kindvalues and match armssubstrate_kindis a node-local notion the plan keeps separate. Matching any= 'k8s'would flag it, and Kubernetes-y strings in unrelated SQL.no_crossing,listed_no_growth,stale_entriesrequired;no_new_entries,no_raised_capsadvisory; no baseline tablemain. The two advisory modes give a false positive a reviewed path (a baseline entry) instead of a silentallowwidening, as in every sibling instance (review round 1)."k8s"/"space"arm counted, including non-substrate vocabularies. The cost is an arm whose substrate name is only on thematchline; M1/M2 catch thatmatchheader when it matches the label string, which is how such an arm arises.#[cfg(test)]support naming conventions (*_tests/,test_support,*_test_support.rs,test_fixtures.rs,*_test_fixture.rs,scripted_test_doubles.rs, thedb-test-fixturescrate). Narrower globs would turn the next fixture into a required failure. Generic names (fake,transport,compatibility) are not allowed.AGENTS.mdfor the cluster controller unchangedkubectlto127.0.0.1failed. Scoped to the task's own Kind network and removed at each teardown.Locator note
The converted statements' text changes (a placeholder replaces the literal), so their sqlx statement-cache keys and query signatures change. Statements below an added
.bindline move to new/*module_path:line*/locators. Module paths don't change.Overlaps
ARCHITECTURE.toml(both added instances; both kept) and the substrateAGENTS.mdbullet (api, cluster-controller: make the Substrate variants private to the substrate module #607's new wording kept, with this PR's sentence about literals).git range-diffshows no other change to this commit.SubstrateKindmethods) is open. It editsdocs/rust/space-conventions.md(10 lines) and the substrateAGENTS.md(6 lines). See "Reconciling with Ask SubstrateKind row-level questions instead of matching the substrate #602" above for the trivial resolution.Review round 1 → changes
no_new_entries/no_raised_capsrequired, unlike the sibling instances)advisory. The instance comment says a deliberate exception is a reviewed baseline entry.no_crossing,listed_no_growthandstale_entriesstayrequired."k8s"/"space"arm; comments)////* */,#[doc], SQL--comments, and a non-substrate vocabulary sharing a line with a substrate name. No production line matches on this head; onmainonly the converted ones do.matches!(…managed_by…, "k8s"),.as_deref() == Some("k8s"),Some("k8s") =>,"k8s" if … =>,("k8s", true) =>,eq/eq_ignore_ascii_case("k8s"), prefixed names,IN (lower($1), 'k8s'),= ANY(ARRAY['k8s']), same-lineCASE managed_by WHEN 'k8s',managed_by::text = 'k8s',lower(managed_by) = 'k8s', and amatchon the label string. Still missed, and listed in the comment:VALUESlists, split lines,format!, non-.rsSQL, an arm with its substrate name only on thematchline, and a guard containing=/::. Probe table above.space-conventions.md, the skill and the substrateAGENTS.mdsay the rule catches the common forms. They now use #602's and #607's conventions (SubstrateKindmethod for a row decision, the named const for a fixed scope, variant matching only for mapping,capabilities()for the running substrate, no parse of the runningmanaged_by(), never the label string). "Parse the value and match the variant" is gone. The text is shaped so #602 reconciles trivially.$1) would change which rows the fragment exempts.This round changes
ARCHITECTURE.toml, docs and one doc comment; no Rust code. So nextest and Kind weren't rerun; their rows below are from38caefa, whose code is identical.Review round 2 → changes
== NODE_MANAGED_BY_K8S/_SPACE(any path prefix) and== SubstrateKind::…/NodeManagedBy::….as_str()/.to_string(), either side. No hits anywhere on this head, tests included. The parser armNODE_MANAGED_BY_K8S => …and.bind(SubstrateKind::Kubernetes.as_str())don't match. The consts stay public. The conventions bullet now says "the namedSubstrateKindconst the statement binds".*managed_by/substrate_kind.match substrate_outcome.as_ref() {,match self.substrate.as_ref() {andmatch substrate_capabilities.as_ref() {no longer match.main'smatch substrate_kind.as_str() {still does..as_ref()/.as_deref()on a typedOption<SubstrateKind>/Option<NodeManagedBy>is listed as a false positive.'k8s' IN (n.managed_by)and raw strings missed"managed_by", S4 handles'k8s' [NOT] IN (…managed_by…), and R1–R3 acceptr"…"/r#"…"#. The comment and body say "either side" applies to the binary comparisons only.if let/let … else,[...].contains(…), a column read by name, a neutral binding, a case-folded literal, the&*/[..]/.unwrap_or(…)headers and their arms, and any;{}=:between an arm's literal and=>. Added false positive: SQL-shaped text in an ordinary Rust string. Every one is in the probe.Validation
Head
9a18529onmain9af70ec. Rounds 1 and 2 changed config, docs and one doc comment only. Before them, head38caefa. The commit was rebased six times asmainmoved:c1187fb→125c08f→04c03e6→17cd654→25cfbf8→202ef6b→9af70ec. The rebase onto17cd654added the #217/#458 conversions; the one onto25cfbf8dropped #458's after its revert. Each row names the heads it ran on. Rounds 1 and 2 changed no Rust code apart from one doc comment, so nextest and Kind weren't rerun after38caefa. Kind ran last on77f5a3b, this commit on25cfbf8.git range-diffshows this head differs from it only in the two conflict resolutions with #607 (ARCHITECTURE.toml, the substrateAGENTS.md), so the production diff Kind validated is identical. The last rebase, onto #603 (tests only), left the commit unchanged (range-diff:=). #607's own production changes (Substrate variants private), #610 (CI) and #603 (tests) are not covered by these Kind runs.cargo fmt --all -- --check9a18529,711ff14and38caefa)cargo clippy --workspace --all-targets --all-features --locked -- -D warnings9a18529,711ff14and38caefa)cargo clippy -p pplx-space-api --no-default-features -- -D warnings(Direct-only build;direct.rschanged)--all-targetsit reports one unused import incontrollers/sandbox/mod.rs(pooled_prepare_error_is_reclaimable) that is only unused in test builds withoutkubernetes. That file is untouched: PRE-EXISTING.pre-commit run gitleaks --from-ref origin/main --to-ref HEAD9a18529,711ff14and38caefa)space-public-reviewon the staged change and this textcaches.rs)cargo arch --base-baseline <main's baseline> --base-architecture <main's ARCHITECTURE.toml>, and--format rdjsonas CI runs it9a18529. 90 advisory and 0 required findings, the same set asmain. Onmainwith thisARCHITECTURE.toml: 3 requiredsubstrate_literalsfindings (the 7 lines above). Growth in three files already over advisorylisted_no_growthcaps:direct.rs3981 → 3992 (route_identity_substrate_kindand two.bindlines),substrate_adapter.rs2506 → 2508,published_state.rs3806 → 3807.cargo nextest run --locked -p pplx-space-api --all-features -p pplx-space-cluster-controller-server -p pplx-space-internal-api-server -p pplx-space-common-svc --run-ignored all --retries 0,SPACE_CI_REQUIRE_TEST_DEPS=1, three runs38caefa: 3478/3478 passed, 0 skipped, in each of runs 1, 2 and 3. Every run includes the characterization tests above, #603's 11 new Kubernetes lifecycle characterization tests and the six new or extended tests. Earlier heads, three runs each:d8d95ba3453/3453,58523c83453/3453,5efa6db3480/3480,77f5a3b3456/3456,eb0d1f93457/3457.claude plugin validate .agents/plugins/pplx-spaceauthor, as onmain)cargo nextest run --locked -p pplx-space-archcheck, includestests/repository.rsagainst thisARCHITECTURE.toml)9a18529Kind
docker builder prune -affreed 20.4 GB on the device anddocker image prune -f0 B, leaving 35.1 GB. (2) This worktree'starget/, removed three times after the cargo validation for the pushed head had finished, freed 86.6, 78.0 and 35.0 GB on the home volume (see the builder-storage item below). No later step was needed.127.0.0.1port DNAT for user-defined bridges but not the matching-o <bridge> -m addrtype --src-type LOCAL -j MASQUERADEit has fordocker0. Sokubectlto the Kind apiserver on127.0.0.1:<port>failed ("no route to host", then connection refused), and the first bring-up failed incheck-object-store-transition.sh. Recreating thekindnetwork didn't add the rule. For each run I added that one rule for the task's own Kind bridge withiptables -t nat, and removed it at teardown. Nothing else on the host changed.77f5a3b)smoke-kind-e2e.sh(BOOTSTRAP_TIMEOUT=1800,DATA_PLANE_URL,SPACE_KIND_TELEMETRY_MODE=otelas CI sets) createdspace-local(control plane created 04:25 UTC) and started Tilt from this checkout. The api, internal-api and cluster-controller services logversion: 77f5a3bc77f6. Their deployed binaries match the artifact image's by sha256 (cc8be0aa…,212223a4…,cd12fa30…) before and after the suites. None containsmanaged_by <> 'k8s',managed_by = 'k8s'ormanaged_by = 'space'. The api binary contains the builder lookup'sn.managed_by = $2and the create-targetmanaged_by = $3 AND lifecycle = 'creating'.smoke-kind-e2e.sh(start)smoke-kind-typed.sh all(unsharded, CI's env:otel, admin token file, cold-receipts dir, host dm targets loaded)smoke-kind-e2e.sh(end)teardown.shon each task-owned cluster; the iptables rule removed; scratch Postgres containers and scratch worktrees removedTyped-suite failure on the final run, rerun once: PRE-EXISTING.
local.btrfs_send_restore_pulls_parent_template: "template-manager counted no successful on-demand pull of the parent on the worker; the build may have come from prefetch". The rerun failed the same way. On a fresh otel stack built from pristinemain17cd654(no commit of this PR), the same scenario fails the same way. It passed in both otel runs at58523c8(base04c03e6), so it arrived with one ofmain's commits between04c03e6and17cd654; I did not bisect which. This PR touches neither template-manager nor the pull path.Earlier Kind runs
d8d95ba(base125c08f, harness default telemetrydatadog), fresh cluster, provenance as above (version: d8d95babec4e): e2e 7/7, typed ECW 78/78 and native 23/23, e2e 7/7. Indatadogmode the suite skipslocal.btrfs_send_restore_pulls_parent_templateandlocal.telemetry_reaches_local_backends, so the later runs used CI'sotel.58523c8(base04c03e6): first adatadogrun (e2e 7/7, ECW 76/76, native 24/24, e2e 7/7). Then two freshotelruns:INFRA_ERROR.local.ecw_template_build_from_oci_bootshit "storage admission requires 55834574848 available bytes but found 51032117248": the builder's storage is on the home volume, which this worktree'starget/had filled. I tore down, removedtarget/(Disk step 2) and reran on a fresh cluster.5efa6db(base17cd654, while Enforce tenant publication grants after preparation rollout #458 was onmain), freshotelcluster: e2e 7/7, ECW 75/78, native 24/24, e2e 7/7. The three failures were all PRE-EXISTING: each reproduced on the pristinemain17cd654stack.local.ecw_template_build_from_oci_bootsandlocal.private_template_build_isolationwere refused at registration with "publisher node UID is awaiting inventory synchronization". That is Enforce tenant publication grants after preparation rollout #458's new check, and the Kind builders'node_k8s.k8s_node_uidwas NULL on both stacks. That message means the lookup found the builder row, so itsmanaged_bypredicate matched. Run against the live Kind database with the builder's identity,main's literal statement and this PR's bound one returned the same row. Both pass again on77f5a3b, after Revert "Enforce tenant publication grants after preparation rollout (#458)" #606's revert.local.btrfs_send_restore_pulls_parent_template: the failure above.🤖 Generated with Claude Code