service@1 DB migration capability (Team Deployments Phase 2A) [draft]

[Koh0920]

DB migration capability (service@1)

Scope: Add an app-schema migration capability to the service@1 dependency contract (ato#1071 §4 state_migration_capability) — NOT new [dependencies.*] manifest grammar. The orchestrator drives migrations via the provider; the CLI bakes no SQL logic.
Base branch: main
Dependent PR: pairs with the ato-postgres migration-runner hook PR.
Changed migrations: n/a (Rust).
Security boundary: fail-closed — a migration failure makes wait_for_ready fail, so start_one returns Err and the dependent app never starts.
Test commands and results: cargo build -p capsule -p cli finished; cargo test -p capsule --lib migration 3/3, --lib contract 226/226 (existing manifests parse unchanged); cargo test -p cli --lib migration 6/6; cargo clippy -p capsule -p cli --all-targets -- -D warnings clean. Full cargo test --workspace + cargo clippy --workspace is a pre-merge gate (running/CI).
Staging verification: n/a (runner-side; validated with the ato-postgres provider).
Known limitations: single consumer migrations/ dir maps to every db-capable provider (fine for single-DB Phase 2A); a migration failure surfaces as a readiness timeout (specific error in redacted provider logs).
Not yet proven: end-to-end migration apply against a live ato-postgres deployment on a runner.
Production deployment status: n/a (client/runner binary).

---

Common status (Phase 2A, ato#1073)

• Staging ato-api version: 8caf3e94
• Staging migrations: 0109–0115 (applied to ato-store-db-stg)
• ato-api deployment suites: 46/46 passing
• PWA tests: 936/936 passing
• TypeScript: tsc clean
• Production: not deployed
• Live-runner durable-volume E2E: pending

The control-plane implementation and mocked runner integration are complete.
The final live-runner execution and durable-volume persistence leg remains an explicit merge gate.

Draft — do not merge until full cargo test --workspace CI and the live-runner durable-volume E2E pass. ato-api/ato-pwa main merges auto-deploy production; hold until explicit production approval.

---

Phase 2A PR set (9 PRs)

1. ato-api feat(ato-cli): sweep stale import preview sessions at startup #311 — RFC + schema
2. ato-api feat(ato-cli): guard active import workspaces from deletion #312 — ES256 Principal Assertion + JWKS + proxy security
3. ato-api feat(ato-cli): Windows process-tree stop for import preview sessions #313 — Shared Deployment core + team_shared proxy
4. ato-api feat(ato-cli): guard active import workspaces from deletion #314 — Audit + Purge + Export
5. ato-api feat(ato-cli): best-effort Windows root taskkill for import preview #315 — Runner-session launch integration
6. ato-pwa feat(ato-desktop): sidebar B1 — widen rail to 72px + Arc bg/border (gpui-html / AODD) #162 — Team Library UI (existing)
7. ato-pwa desktop: spike — verify NSWindow child-window behavior across Spaces and fullscreen #168 — Shared Deployment UI (stacked on feat(ato-desktop): sidebar B1 — widen rail to 72px + Arc bg/border (gpui-html / AODD) #162)
8. ato service@1 DB migration capability (Team Deployments Phase 2A) [draft] #1083 — service@1 DB migration capability
9. ato-postgres fix(ato-cli): accept canonical capsule:// URLs in ato app latest #2 — migration runner + __ato_migrations hook

[Koh0920]

Automated review (Claude Code)

Verdict: changes-requested (direction is sound; base branch + two correctness gaps must be settled before it leaves draft)

What it does
Adds an optional [contracts."service@1".migrations] block (enabled / format="sql" / tracking_table) to provider-side ContractSpec, defaulting to absent so existing manifests parse and re-serialize unchanged. At dep-start time, if the provider advertises the capability and the plan supplies a migrations dir for that alias, start_one injects ATO_MIGRATIONS_DIR / ATO_MIGRATIONS_TRACKING_TABLE / ATO_MIGRATIONS_FORMAT into the provider process before spawn; the provider owns all SQL. The run pipeline populates that map by convention — if <workspace_root>/migrations/ exists, it is offered to every declared dep, with the provider-side flag as the only real gate.

Findings

• blocker — base branch. PR targets main. CLAUDE.md §Branching: main = "latest stable release only (every commit = a published vX.Y.Z tag)", and the base-branch table routes "0.7 new feature / experiment" to nightly. Head is 0 ahead of main, 54 behind nightly — this is a 0.7 service@1 contract change landing on the stable line, and it inverts the mandated oldest→newest flow. Retarget to nightly and rebase. Note the trade-off: rust-ci.yml:8-12 only fires on main/dev, so retargeting loses the workspace cargo test/clippy gate the PR body relies on — that gate becomes manual.

• blocker — fail-open on version skew; nothing ever confirms migrations ran. The capsule crate deliberately has no deny_unknown_fields (documented as policy at crates/capsule/src/foundation/types/ready_state.rs:12). So an ato-postgres capsule advertising enabled = true parses cleanly on any already-released 0.7.x CLI, which has no migrations field, injects no env, starts the provider normally, passes readiness — and the consumer app boots against an un-migrated schema with no warning. The mirror case (new CLI, old provider that ignores the env) is identical. The PR's "fail-closed" claim only covers migrations ran and failed; it does not cover migrations never ran, which is the more likely production failure. Needs a positive acknowledgement: a provider-written receipt in state_dir checked after readiness, or a lock-time error when the consumer ships migrations/ and the resolved provider doesn't advertise the capability.

• major — a failed migration is only visible as a generic ready timeout. crates/cli/src/application/dependency_runtime/ready.rs:71-108: wait_for_ready polls the probe and never calls child.try_wait(). A provider exiting non-zero from a bad migration isn't noticed; the loop burns the full ready_probe_timeout (contract value, else 30s from run.rs:1057) and reports Timeout, with the SQL error only in the redacted provider log. The PR body concedes this, but this PR is what makes migration failure a routine failure mode, so the gap becomes load-bearing. Mirror problem: migrations now spend the readiness budget, so a legitimately slow migration (index build) trips a timeout that previously only had to cover boot — there is no separate migration timeout. And on that path start_one returns Err with child merely dropped (Rust Child doesn't kill on drop), leaving a half-migrated postgres up until the orphan sweep. A try_wait() in the loop is cheap and fixes the diagnostic.

• major — the consumer has no say in which dep receives its SQL. crates/cli/src/application/pipeline/phases/run.rs:1037-1047 maps <workspace_root>/migrations/ onto every alias in providers_for_run; the sole gate is provider-declared enabled (orchestrator.rs:825). So "these SQL files belong to this database" is decided unilaterally by the provider capsule, and two db-capable providers both get the same SQL. Listed as a known limitation, but it's grammar-shaped, not scheduling-shaped: RFC §7.3 is built on consumer-declared [dependencies.*.parameters] being what tells a provider what to do for this consumer. A consumer-side opt-in ([dependencies.db] migrations = "migrations/") resolves the ambiguity and makes the convention explicit. Also worth weighing: migrations/ at workspace root is a near-universal convention (sqlx, golang-migrate, Django, Rails) — a capsule already shipping one for its own runner gets it double-applied the day its provider opts in.

• minor — normative spec not amended. docs/rfcs/accepted/CAPSULE_DEPENDENCY_CONTRACTS.md:289-329 (§7.1) enumerates the [contracts."service@1"] sub-blocks (parameters / credentials / identity_exports / runtime_exports / state). This adds a sixth and touches zero docs. Line 658 states outright that v1 provides no automatic migration and points provider migration logic at [provision]. The app-schema vs. state-format distinction is a legitimate reading, but it belongs in the spec, not only in Rust doc comments. Relatedly, the cited ato#1071 §4 and "phase2a RFC §1.2" have no in-repo document, so a reviewer can't check the diff against the contract it claims to implement.

• minor — [provision] is the RFC's designated seam and is unimplemented. orchestrator.rs:35 lists [provision] execution as deferred out of MVP. RFC §10.2 step 5 runs provision before target start, so Ato reads its exit code directly — precisely the diagnostic missing in finding 3. Choosing env-into-the-run-target is a reasonable workaround for an unbuilt phase, but the PR should say so explicitly, since the RFC names [provision] for exactly this job.

• minor — lossy path conversion. migration_provider_env returns Vec<(String, String)> and builds the dir via source.display().to_string(), so a non-UTF-8 path silently becomes U+FFFD and the provider is handed a directory that doesn't exist. Command::env takes AsRef<OsStr>; returning OsString (or passing the Path through) avoids the class entirely.

• minor — the risky half is untested. The 4 migration_provider_env tests and 3 serde-default tests are real and would fail if broken (no empty-filter hazard; they assert on values, not just non-panic). But nothing covers run.rs:1037-1047 — the migrations/ discovery, the empty-when-absent branch, or the fan-out to every alias — which is where findings 2 and 4 live.

Checked and clean (worth stating, since these are the usual suspects here): adding the block does not move any recorded digest. compute_instance_hash (crates/capsule/src/foundation/dependency_contracts/lock.rs:731-750) folds only {resolved, contract-id string, parameters} through serde_jcs, so ContractSpec.migrations never enters instance_hash and provider state dirs don't relocate when ato-postgres opts in. skip_serializing_if = "Option::is_none" keeps re-serialized manifests byte-identical. enabled defaults to false, so a bare [migrations] is off rather than on. migration_sources and providers are both keyed by alias (orchestrator.rs:360-371), so no key mismatch. Migration env is injected after env_clear() and after target_block.env, so it can't be shadowed by provider static env.

Staleness/mergeability
Head 89b0f30f sits exactly 1 commit ahead of origin/main and 54 behind origin/nightly; merge-base with main is 9c0ea0c0 (v0.7.9 release merge). GitHub reports MERGEABLE / BLOCKED (draft). Against main it applies cleanly today. Against nightly it will need a real rebase — orchestrator.rs and run.rs are both hot files over those 54 commits, and every OrchestratorInput literal gains a field, so expect struct-literal conflicts in the orchestrator test module specifically. Rebase onto nightly before further review; the diff is small enough that re-deriving it there is cheap.