kagent-dev
diff --git a/‎.claude/skills/kagent-dev/references/database-migrations.md‎
Lines changed: 89 additions & 18 deletions b/‎.claude/skills/kagent-dev/references/database-migrations.md‎
Lines changed: 89 additions & 18 deletions
@@ -30,6 +30,8 @@ go/core/internal/database/
 
 Migrations manage two independent tracks — `core` and `vector` — and roll back both if either fails. The `--database-vector-enabled` flag (default `true`) controls whether the vector track runs.
 
+Migration files are append-only and immutable once merged (see [One Linear History](#one-linear-history)).
+
 ## sqlc Workflow
 
 When you add or change a SQL query:
@@ -48,26 +50,64 @@ A CI check (`.github/workflows/sqlc-generate-check.yaml`) fails the PR if `gen/`
 - `:many` — returns a slice
 - `:exec` — returns only error (use for INSERT/UPDATE/DELETE that don't need the result)
 
+Because the hand-written queries are the source of truth for what code reads, sqlc's generated output makes "no current code reads this column" greppable — the check behind a contraction sign-off (see [Backward compatibility and contraction](#backward-compatibility-and-contraction)).
+
 ## Writing Migrations
 
-### Backward-compatible schema changes
+### Backward compatibility and contraction
 
-During a rolling deploy, old pods will be reading and writing a schema that has already been upgraded. **Every migration must be backward-compatible with the previous version's code.**
+During a rolling deploy — and after a version rollback — old pods read and write a schema a newer release has already migrated. **The default for every migration is backward-compatible: nothing a prior release's code reads or writes may stop working.** "Additive-only" is the usual shorthand, but it is imprecise — some additive DDL still breaks old code. State the rule by *effect*, not by DDL shape.
 
 | Change | Old code behavior | Safe? |
 |--------|------------------|-------|
 | Add nullable column | SELECT ignores it; INSERT omits it (goes NULL) | ✅ |
 | Add column with `DEFAULT x` | INSERT omits it; DB fills default | ✅ |
-| Add NOT NULL column **without** default | Old INSERT missing the column → error | ❌ |
 | Add index | Invisible to application code | ✅ |
-| Add foreign key | Old INSERT may fail constraint | ❌ |
-| Drop/rename column old code references | Old SELECT/INSERT errors | ❌ |
-| Change compatible type (e.g. `int` → `bigint`) | Usually fine | ⚠️ |
+| Widen a compatible type (e.g. `int` → `bigint`) | Usually fine | ⚠️ |
+| Add NOT NULL column **without** default | Old INSERT missing the column → error | ❌ |
+| New constraint on a shipped table (FK / `UNIQUE` / `CHECK`) | Old writer violates it → error | ❌ |
+| Narrow a column type | Existing/old-code value may no longer fit | ❌ |
+| Drop or rename a column/table old code uses | Old SELECT/INSERT errors | ❌ |
+| Rewrite stored rows into a new format | Old reader can't parse the new format | ❌ |
+
+This is exactly what makes a rollback safe: when an operator redeploys the previous release against a database the newer release already migrated, the old code's queries still run because no contraction has shipped (see [Rollback and ahead-schema tolerance](#rollback-and-ahead-schema-tolerance)).
+
+The last ❌ row is easy to miss: a migration — **or an out-of-band tool** — that rewrites existing rows breaks old readers the same way a `DROP COLUMN` does, with no DDL for static analysis to catch. A data rewrite is a contraction regardless of its SQL.
+
+**Contractions are not banned — they are windowed.** Anything in the ❌ rows is a *contraction*. Forever-backward-compatible is not tenable (dead weight accumulates without bound), so a breaking change is split across minor versions such that no supported rollback target ever lands on code that needs the removed structure:
+
+1. **Minor `X.Y` (expand):** add the new column/table (nullable or with default). Old code still works.
+2. **Minor `X.Y` (deploy):** ship the code that uses the new structure.
+3. **Minor `X.(Y+1)` (contract):** drop the old column/table — safe because the furthest supported rollback from `X.(Y+1)` lands on minor `X.Y`, which already uses the new structure.
+
+The **rollback window** is how far back a rollback is supported: **one minor back**. From `Major.Minor.Patch`, an operator may roll back to an earlier release in the current minor, or to the previous minor — the previous minor is the furthest-back supported target. A contraction is therefore safe to merge only once its replacement shipped at or before the previous minor, so no supported rollback can land on code that predates it.
+
+**Destructive changes must be declared, not silent.** An intentional contraction is allowed only with explicit reviewer sign-off confirming (1) the replacement shipped in the prior release and (2) no current code still reads the old structure — sqlc makes that second point checkable for Postgres, since the generated queries are greppable. Pre-rule contractions already in history are grandfathered; the rule binds going forward.
+
+> **Enforcement.** *Target — not yet enforced*: a static check extending `cross_track_test.go` will block undeclared destructive DDL at merge (see [Static Analysis Enforcement](#static-analysis-enforcement)).
+
+### Schema-agnostic SQL
+
+**Migration SQL must not name a schema.** The schema a migration lands in is chosen by the *connection* (its `search_path` / `current_schema`), not the file, so the same migration files apply into whatever schema the connection selects.
+
+Forbidden in any migration file:
+
+- `CREATE SCHEMA` / `DROP SCHEMA`
+- schema-qualified DDL (`CREATE TABLE foo.bar`, `ALTER TABLE foo.bar ...`)
+- `SET search_path`
+- `ALTER ... SET SCHEMA`
+
+```sql
+-- ❌ pins every install to one hard-coded schema
+CREATE TABLE myschema.eval_set (...);
+
+-- ✅ lands in whatever schema the connection selects
+CREATE TABLE IF NOT EXISTS eval_set (...);
+```
+
+Schema is a deploy-time choice, fixed by the connection rather than the migration file. A hard-coded schema breaks any deployment that runs the track in a different schema (e.g. a connection that sets `?search_path=<schema>`). The core and vector migrations comply today (verified by inspection until the lint lands).
 
-**Expand-then-contract pattern for schema changes:**
-1. **Version N (Expand)**: add the new column/table (nullable or with default); old code still works
-2. **Version N (Deploy)**: ship new code that uses the new structure
-3. **Version N+1 (Contract)**: drop the old column/table once version N is fully deployed and no pods run version N-1
+> **Enforcement.** A static lint test rejecting the forbidden patterns across all migration files (*Target — not yet enforced*; see [Static Analysis Enforcement](#static-analysis-enforcement)).
 
 ### Idempotency and cross-track safety
 
@@ -93,6 +133,18 @@ Files must follow `NNNNNN_description.up.sql` / `NNNNNN_description.down.sql` wi
 
 Every `.up.sql` must have a corresponding `.down.sql` that exactly reverses it. Down migrations are used for rollbacks and by automatic rollback on migration failure. They must be **idempotent** — the two-track rollback logic (roll back core if vector fails) may call them more than once in failure scenarios.
 
+A down file that never runs is a down file you cannot trust. There are no up-only migrations — a working down has shipped with every migration since the golang-migrate adoption. Exercising every migration up → down → up against the real migration set, to prove the reversal rather than assume it, is a *Target — not yet enforced* (see [Upgrade and rollback testing](#upgrade-and-rollback-testing)).
+
+## One Linear History
+
+Migrations form a single, append-only sequence. Two rules keep it that way.
+
+**Immutable from merge.** A migration file is never edited once it merges to `main` — not merely once it is released. The next build picks it up as soon as it merges, so editing it would diverge the schema state of any database that already applied it. A bug in a migration is fixed by a **new** compensating migration, never by editing the original in place.
+
+**Sequence numbers are claimed at merge.** The 6-digit number is allocated when the migration lands on `main`. A feature branch carrying a draft migration **renumbers** before merging if `main` has moved on, so the sequence never forks or collides.
+
+A backward-compatible migration may ship in the **same PR** as the code that uses it — the migration is additive, so old code tolerates the new schema during the rollout.
+
 ## Multi-Instance Safety
 
 ### How the advisory lock works
@@ -110,10 +162,7 @@ This is safe. The only risk is if the winning controller crashes mid-migration (
 
 ### Dirty state recovery
 
-If the controller crashes mid-migration, the migration runner records the version as `dirty = true` in the tracking table. The next startup detects dirty state and calls `rollbackToVersion`, which:
-1. Calls `mg.Force(version - 1)` to clear the dirty flag.
-2. Runs the down migration to restore the previous clean state.
-3. Re-runs the failed up migration.
+If the controller crashes mid-migration, golang-migrate leaves the tracking table marked `dirty = true`. On the next startup `Up` refuses to run against a dirty database, so the runner clears the flag: `mg.Force(version - 1)` resets the tracking table to the last clean version. The process then exits with the error, and the failed migration is re-applied on a **subsequent** startup once the database is clean — so recovery from a transient failure spans restarts rather than completing in a single run.
 
 **Requirement**: down migrations must be idempotent and correctly reverse their up migration. A missing or broken down migration requires manual recovery.
 
@@ -127,6 +176,18 @@ For backward-compatible migrations a rolling update is safe:
 
 For a migration that is **not** backward-compatible, restructure it using the expand-then-contract pattern (add new column/table in version N, ship code that uses it, drop the old column in version N+1).
 
+## Rollback and ahead-schema tolerance
+
+Two distinct events both get called "rollback."
+
+**A migration fails mid-upgrade.** The runner reverts the in-flight migration automatically and the process exits, leaving the database at the last clean version (see [Dirty state recovery](#dirty-state-recovery)). This has always worked.
+
+**A version rollback after a successful upgrade.** A regression turns up and the operator redeploys the previous release against a database the newer release already migrated forward. The runner **tolerates a database ahead of it** — it sees a higher-than-known version, accepts it, and starts.
+
+Tolerating an ahead database is only safe because of the [backward-compatibility window](#backward-compatibility-and-contraction): inside the window no contraction has shipped, so the old code's queries run against the newer schema by construction. The schema simply stays expanded until the operator re-upgrades. The server does **no** version arithmetic at startup — staying within the supported rollback window (one minor back) is the operator's responsibility, not a runtime check.
+
+Down migrations are off this routine path. They are still authored and still run — by the failure-revert above, and for a deliberate schema reversal (run from the newer release, which ships the down files) — but a routine in-window version rollback touches no down file.
+
 ## Static Analysis Enforcement
 
 The policies above are enforced by static analysis tests in `go/core/pkg/migrations/cross_track_test.go`. These run against the embedded SQL files — no database required.
@@ -135,21 +196,31 @@ The policies above are enforced by static analysis tests in `go/core/pkg/migrati
 |------|-----------------|
 | `TestNoCrossTrackDDL` | No track may `ALTER TABLE` or `CREATE INDEX ON` a table owned by another track |
 | `TestMigrationGuards` | Up migrations must use `IF NOT EXISTS` on all `CREATE`/`ADD COLUMN`; down migrations must use `IF EXISTS` on all `DROP` statements |
+| Contraction guard *(target)* | Blocks undeclared destructive DDL — `DROP`/`RENAME` of shipped objects, type narrowing, new constraints on shipped tables (see [Backward compatibility and contraction](#backward-compatibility-and-contraction)) |
+| Schema-agnostic lint *(target)* | Rejects `CREATE SCHEMA`, schema-qualified DDL, `SET search_path`, and `ALTER ... SET SCHEMA` (see [Schema-agnostic SQL](#schema-agnostic-sql)) |
 
 **Adding a new track**: add the track directory name to the `tracks` slice in each test so the new track is covered by the same checks.
 
 These tests catch policy violations at PR time without needing a running database. They complement the integration tests in `runner_test.go`, which verify the runner's rollback and concurrency behavior against a real Postgres instance.
 
+## Upgrade and rollback testing
+
+Static analysis covers file *content*; round-trip tests cover *behavior* against a real Postgres. Beyond `runner_test.go` (rollback and concurrency), two release-to-release tests make the rollback promise real. Both are *Target — not yet enforced*.
+
+**Previous-minor round-trip.** Seed a database at the previous minor's latest release with representative data, apply migrations up to `HEAD`, and assert the schema matches a clean `HEAD` install and the data survives; then reverse to the previous minor and assert the schema matches a clean previous-minor install and the data survives. This exercises every changed down file rather than only reviewing it.
+
+**Query-level backward compatibility.** Run the previous minor's database test suite against a `HEAD`-migrated schema, proving old code's queries run against the newer schema — the exact property [ahead-schema tolerance](#rollback-and-ahead-schema-tolerance) relies on.
+
 ## Downstream Extension Model
 
-The migration layer is designed for downstream consumers to extend with their own migrations alongside OSS. The extension points are:
+The migration layer is designed for downstream consumers to extend with their own migrations. The extension points are:
 
 1. **SQL files as the contract.** The migration files in `go/core/pkg/migrations/core/` and `vector/` are the stable interface. Downstream consumers sync these files into their own repos and build their own migration runners. Don't move or reorganize migration file paths without considering downstream impact.
 
-2. **`MigrationRunner` DI callback.** Downstream consumers pass a custom `MigrationRunner` to `app.Start` to take full ownership of the migration process — running OSS migrations alongside their own in whatever order they need. The signature `func(ctx context.Context, url string, vectorEnabled bool) error` is stable.
+2. **`MigrationRunner` DI callback.** Downstream consumers pass a custom `MigrationRunner` to `app.Start` to take full ownership of the migration process — running the core and vector migrations alongside their own in whatever order they need. The signature `func(ctx context.Context, url string, vectorEnabled bool) error` is stable.
 
 3. **Vector track stays separate.** The vector track is conditionally applied and has its own tracking table. Downstream extensions should not modify vector-owned tables (enforced by `TestNoCrossTrackDDL`).
 
-### What this means for OSS development
+### What this means for development
 
-- **Migration immutability is cross-repo.** Once a migration file is merged and tagged, downstream consumers may have synced it. Modifying it breaks their tracking table state.
+- **Migration immutability is cross-repo.** [Immutability](#one-linear-history) binds from the moment a migration merges to `main`, not from release: downstream consumers may have synced a merged file before it ships. Modifying it breaks their tracking-table state.