[ac-pplx]

Summary

The substrate AGENTS.md told contributors to implement new lifecycle operations for Kubernetes only, and to return SubstrateError::Unsupported on Direct unless examples/first_sandbox.py used the call. It also asked for a required SubstrateAdapter method for each new operation. Both substrates are supported, so that text left Direct further behind with each feature, grew the trait, and invited Kubernetes-only repair loops that give Direct no failure exit.

This replaces "Direct Substrate Contract" with "Substrate contract" in crates/libs/api/src/controllers/substrate/AGENTS.md:

• Both substrates are supported. Regressing either one is a regression. Direct is the planned base for pod-less hosting on Kubernetes nodes.
• Adapters answer node and execution questions only. Placement, admission, warm-pool policy, lifecycle transitions, leases, generations, retries, repair, drain and evacuation are core logic. Core logic belongs once, in shared code such as controllers/sandbox/ and pplx-space-sandbox-store. The section names the core logic still in the adapters as debt:
  • Direct's worker and restore-candidate retries, and its restore lease;
  • Kubernetes' restore leases, started recovery and redrive;
  • the Kubernetes-only pplx-space-placement machine.
• New adapter methods are for infrastructure actions only, and land for both substrates.
  • Every Unsupported needs a row in the gap table that names an owner and the change that removes it.
  • A change to Direct updates the direct.rs module doc, and the PR that closes a gap deletes its row.
  • The old rule that "a change to Direct behavior belongs in its own PR" is removed on purpose: new operations now land on both substrates in one PR.
• New sandbox- and node-state reconcilers run for both substrates. The existing Kubernetes-only ones are debt.
• Capabilities state infrastructure facts, not disabled features. Unsupported trait defaults and supports_node_drain, a capability for a missing feature, are debt, not precedent.

The archcheck-backed rules are kept, condensed, and say where their ratchets stop: kubernetes_access cannot tell a presence test from a use, substrate_literals catches only the common forms, and direct_tables is advisory. The required-method rule keeps its reason: a forgotten forwarding arm gives both substrates the trait default. It holds until dispatch is generated.

The tests section:

• asks for both substrates only where the path differs by substrate;
• names the shared Direct helper (crate::test_support::direct_substrate with a SimWorkerHarness worker);
• states that only the try_pg_test_db* fixtures skip, and only when SPACE_TEST_DEPS_OPTIONAL is set.

The direct.rs module doc's list of what Direct does not do becomes a gap table (refused or missing / effect today / owner / removed by) with the same gaps; each existing row's owner is the PR author until it is reassigned.

The resolves_cold_create_shape doc comment in mod.rs no longer says Direct refuses a one-shot create with custom vCPU or memory. That refusal is gone; Direct sends custom vCPU or memory on its separate start call.

Links to the renamed heading are updated in crates/libs/api/AGENTS.md, crates/libs/cluster/controller/server/AGENTS.md and crates/libs/internal/api/server/AGENTS.md.

No Rust behavior changes: only doc comments change in direct.rs and mod.rs.

Validation

All at this head:

• Relative links and anchors in the four changed .md files: 2 links, 0 broken. The #substrate-contract anchor resolves.
• Old heading: rg -n "direct-substrate-contract|Direct Substrate Contract|Direct substrate contract" finds no matches.
• Formatting: cargo fmt -p pplx-space-api --check passes.
• Docs: RUSTDOCFLAGS="-D rustdoc::broken_intra_doc_links" cargo doc -p pplx-space-api --no-deps --document-private-items reports only errors already on main: 10 unresolved links and one run function/module ambiguity in lib.rs. None are in direct.rs or in the edited mod.rs lines.
• Architecture: cargo arch exits 0 with 113 advisory findings and 0 required. It ran without base baselines, so the no_new_entries and no_raised_caps checks were skipped.

Instruction-chain budget (bytes; root AGENTS.md plus each chain, against a 32,768 B limit):

Chain	Base	Head
root AGENTS.md	6,466	6,466
root + crates/libs/api + …/controllers/substrate	16,299	17,750
root + crates/libs/api	8,397	8,439
root + crates/libs/cluster/controller/server	9,100	9,180
root + crates/libs/internal/api/server	9,396	9,382

The contract section is 6,220 B, against 5,450 B for the old "Direct Substrate Contract" section. It grew on purpose: it now says where core logic lives, what each archcheck ratchet does not catch, and the upkeep rule for the direct.rs module doc. The largest chain is still well under the limit.

No tests were run because there is no behavior change.

Severity and risk

Severity: moderate. These are agent and contributor instructions, and the old text steered new work away from Direct.

Risk: documentation only; nothing at runtime changes. Open PRs and an RFC that cite the old heading or the old own-PR rule will need to rebase their references. If this conflicts with #1157 on the as_kubernetes() sentence, whichever lands second rebases. To roll back, revert the commit.

Not covered

Tracked as follow-ups:

• Sorting each Kubernetes-only loop in run_reconcilers into sandbox/node-state loops or Kubernetes-object loops, and naming them in the gap row.
• The trigger wording in the space-rust-development skill (a plugin change that needs a version bump).
• Whether supports_node_pool_placement and resolves_cold_create_shape are feature-off capabilities.
• The crate doc of pplx-space-placement.
• Tightening when a new operation may ship as Unsupported with a gap row, once the Direct lane runs as a required check. For now a row must name an owner and the change that removes it.

AI assistance: An AI agent (Claude Code) drafted all of the documentation and doc-comment changes and this description.

[github-actions]

Thanks for opening this pull request! A SPACE maintainer will take a look.

CONTRIBUTING.md explains how we review outside contributions.

[ac-pplx]

Heads-up for open work that cites the substrate contract this PR replaces.

• @the-gigi-pplx: RFC [docs] add RFCs for per-client admission independent of the substrate #1072 proposes live_sandboxes_for_client and label_adopted_execution as new required SubstrateAdapter methods, citing the old rule ("new operation = required method, Direct returns Unsupported"). Under the new contract, behavior lives above the adapter, and a new adapter method is only for an infrastructure action the existing ones can't express, implemented for both substrates. Could the RFC count live sandboxes per client from a substrate-neutral source (the sandbox and execution rows, or the node registry) rather than the sandbox-pod reflector, and keep the adopted-pod label inside the Kubernetes pool module rather than on the trait? If one of them has to stay one-sided for now, it needs a gap-table row with an owner and the change that removes it.
• [space] fence every placement writer against the provider-terminate path #559 (delete_created_execution) and api: decide admission pool claims for the create's owner; collect refused claims promptly #168 (abandon_admitted_pool_claim) add required methods under the old rule. They can land as written; generated dispatch, planned after they merge, absorbs them.
• The required-method rule itself stays until dispatch is generated, so nothing changes for PRs that only extend existing methods.

[the-gigi-pplx]

Review — 2 findings

Low

1. crates/libs/api/src/controllers/substrate/AGENTS.md (Tests, last paragraph) — the Postgres skip rule is stated more narrowly than the code applies it.
   "Only the optional fixtures (try_pg_test_db and its variants) skip instead, and only when the run sets SPACE_TEST_DEPS_OPTIONAL" misses two cases in TestDepsPolicy::from_values (crates/libs/db-test-fixtures/src/lib.rs). SPACE_CI_REQUIRE_TEST_DEPS set to anything but empty or 0 requires the dependency even when SPACE_TEST_DEPS_OPTIONAL=1. Set to empty or 0, it makes the run optional without SPACE_TEST_DEPS_OPTIONAL at all (the LEGACY_OPT_OUT rows in db-test-fixtures/src/tests.rs). Also, the fixtures aren't the only skippers: a test that starts its own container skips through report_unavailable_dependency, as docs/rust/space-conventions.md "Service-dependency tests" says. Since that section is already linked, I'd drop the restatement and keep the one sentence that matters here ("a skip is not evidence the test passed"), or match the doc's wording.

2. crates/libs/api/src/controllers/substrate/AGENTS.md (as_kubernetes() bullet) — capability_identity "flags either const named in production code outside the two capabilities() implementations" is narrower than the rule.
   The allowlist in ARCHITECTURE.toml ([rules.pattern_ratchet.capability_identity]) is the two whole files, direct.rs and kubernetes/adapter/substrate_adapter.rs. So a comparison against SubstrateCapabilities::DIRECT anywhere else in either file goes unflagged. The old text named the files. Since this section's stated job is to say where each ratchet stops, I'd say "outside direct.rs and kubernetes/adapter/substrate_adapter.rs", and note that review covers the rest of those two files.

Checked and holds:

• use_one_shot_create (direct.rs:234-247) routes custom vCPU or memory to create plus start, so the resolves_cold_create_shape doc edit is right. No other Direct path refuses a custom shape.
• Every Unsupported Direct returns today has a gap row: node pool (direct.rs:3604), fork-disk restore (ensure_direct_create_supported), the pool-config trait defaults, and set_node_schedulability (mod.rs:2022, covered by the drain row). select_ready_worker_node_api_target's Unsupported default is overridden by both substrates.
• The template pull and delete routes return 501 on Kubernetes through the registers_nodes_via_admin_api capability, not SubstrateError::Unsupported, so they don't need a Kubernetes gap row under the new rule.
• crate::test_support::direct_substrate and SimWorkerHarness exist and are used as cited in characterization_tests.rs, and direct/tests.rs uses try_pg_test_db_with_max_connections.
• The kubernetes_access claim that a new caller needs a baseline entry holds: no_crossing is required.
• The gap table's "see below" rows match the "Kubernetes-only reconcilers" section.

Not checked: I didn't run the doc build or cargo arch. The PR's CI is green at this head.

Verified against d1eb9c422e4a7ac4d518ab098d73808142bf1afe. Reproduced locally: read the archcheck rule config and the test-deps policy table at this head; no commands beyond reading were needed for these findings.

[the-gigi-pplx]

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

[zibi-pplx]

The new contract is clear. I have one suggestion about how the gap table can be checked.

• [low] Gap rows have no link to tracked work
  crates/libs/api/src/controllers/substrate/direct.rs:23-36, crates/libs/api/src/controllers/substrate/AGENTS.md:68: the contract says a row without "an owner and a removal plan fails review". In practice, each "Removed by" cell is a sentence describing a future change, such as "Those loops running for both substrates through the adapter". All ten rows name the same owner. A reviewer can't tell from the row whether that work is scheduled, and the PR that closes a gap has nothing to reference when it deletes the row.
  Suggestion: link a tracking issue in each "Removed by" cell, or in a new column. The AGENTS.md rule could then require a linked issue, not just a description of the plan.