Skip to content

Commit d2fae66

Browse files
dmealingclaude
andcommitted
docs(#241): implementation plan — D1 FK-cascade auto-fix
6-task TDD plan: export sqlite primitives; FK-graph + topo-sort + cycle detection; thread actualSchema through emit; the cascade emitter + renderD1 dispatch (refuse cycles only); libSQL real-engine gate (all topologies + #226 gap + cycle); docs. Closes #226's residual gap via the actual∪expected FK graph. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c97e642 commit d2fae66

1 file changed

Lines changed: 161 additions & 0 deletions

File tree

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
# D1 FK-cascade auto-fix (#241) Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Replace #226's *refuse* with a cascade that generates a correct, appliable D1 migration when a foreign-key-referenced table is rebuilt — and close #226's residual under-refuse gap. Refuse only multi-table FK cycles.
6+
7+
**Architecture:** New `emit/d1-cascade.ts` holds a reverse FK-graph (over the *union* of the actual and expected schemas), a topological sort with cycle detection, and a cascade emitter that reuses `emit/sqlite.ts` primitives. `renderD1` dispatches to the cascade for an acyclic referenced-rebuild and throws a cycle-refusal otherwise. `EmitOptions` gains `actualSchema`, threaded from both `meta migrate` emit call sites. Local SQLite + Postgres are untouched; the no-referenced-rebuild path stays byte-identical.
8+
9+
**Tech Stack:** TypeScript (ESM, `.js` specifiers), `bun:test`, libSQL (raw `libsql.createClient`, one-transaction model = remote D1) for the gate.
10+
11+
**Design spec:** `docs/superpowers/specs/2026-07-28-d1-fk-cascade-design.md`
12+
13+
**The validated cascade recipe** (spike-proven 10/10; all statements inside D1's implicit transaction):
14+
```sql
15+
PRAGMA defer_foreign_keys = ON;
16+
CREATE TABLE __f_<t> ( <final schema; FK cols → __f_<ref> if ref∈A else <ref> ); -- for each t in A
17+
INSERT INTO __f_<t> ( <carry cols> ) SELECT <mapped cols> FROM <t>; -- for each t in A
18+
DROP TABLE <t>; -- REFERRERS-FIRST (reverse-topological)
19+
ALTER TABLE __f_<t> RENAME TO <t>; -- PARENTS-FIRST (topological)
20+
```
21+
Referrers-first DROP + parents-first RENAME are load-bearing. Forward-ref FKs in CREATE are fine (SQLite resolves FK targets lazily).
22+
23+
## Global Constraints
24+
25+
- All changes in `server/typescript/packages/migrate-ts` + a 2-line `server/typescript/packages/cli` change (+ docs/CHANGELOG). TS-only; no other-port impact.
26+
- **Byte-identical** D1 output for any migration that does NOT rebuild a referenced table (the cascade path is entered only when `recreatedTables` intersects the FK-referenced set).
27+
- No `any`; ESM `.js` import specifiers.
28+
- The affected-set + ordering FK graph is the **union of the actual and expected** FK edges over `A`. When `actualSchema` is absent, fall back to expected-only AND **refuse** any referenced rebuild not provably safe (never emit an unproven cascade).
29+
- Multi-table cycles (len ≥ 2) are refused; self-loops are handled by the temp-name trick (excluded from the cycle check).
30+
- Run tests scoped: `cd server/typescript/packages/migrate-ts && bun test <file>`. Never bare repo-root `bun test`.
31+
- Unrelated untracked `.serena/` — never stage it. Stage only each task's files; never `git add -A`. Commit to `main`.
32+
33+
---
34+
35+
### Task 1: Export the SQLite rebuild primitives (read-only extraction)
36+
37+
**Files:** Modify `src/emit/sqlite.ts`; Test `test/unit/sqlite-primitives-export.test.ts`
38+
39+
The cascade reuses SQLite's `renderCreateTable(t: TableDescriptor): string` (sqlite.ts:257), `renderCreateIndex(table: string, ix: IndexDescriptor): string` (sqlite.ts:367), and the column carry/remap logic currently inline in `renderRecreate` (sqlite.ts:132-192, lines 137-156: the renames map, addedNames set, `carryColumns`/`insertCols`/`selectCols`). This task exposes them WITHOUT changing SQLite behavior.
40+
41+
- [ ] **Step 1 (test):** Write `test/unit/sqlite-primitives-export.test.ts` importing `{ renderCreateTable, renderCreateIndex, computeCarryColumns }` from `../../src/emit/sqlite.js`; assert `renderCreateTable({name:"t",columns:[{name:"id",sqlType:{kind:"integer",bits:64},nullable:false}],indexes:[],foreignKeys:[],primaryKey:["id"],checks:[]})` contains `CREATE TABLE "t"`; assert `computeCarryColumns` (see Step 3 signature) maps a rename correctly.
42+
- [ ] **Step 2:** Run — FAIL (not exported / `computeCarryColumns` undefined).
43+
- [ ] **Step 3 (impl):** Add `export` to `renderCreateTable` and `renderCreateIndex`. Extract the carry/remap block from `renderRecreate` into an exported pure helper and call it from `renderRecreate` (so SQLite output is unchanged):
44+
```ts
45+
export interface CarryColumns { insertCols: string[]; selectCols: string[]; }
46+
/** newTable columns not newly-added, mapped to their old-name SELECT source (for renames). */
47+
export function computeCarryColumns(tableChanges: Change[], newTable: TableDescriptor): CarryColumns {
48+
const renames = new Map<string, string>();
49+
for (const c of tableChanges) if (c.kind === "rename-column") renames.set(c.from, c.to);
50+
const addedNames = new Set<string>();
51+
for (const c of tableChanges) if (c.kind === "add-column") addedNames.add(c.column.name);
52+
const renamesReverse = new Map<string, string>();
53+
for (const [from, to] of renames) renamesReverse.set(to, from);
54+
const carry = newTable.columns.filter((c) => !addedNames.has(c.name));
55+
return { insertCols: carry.map((c) => c.name), selectCols: carry.map((c) => renamesReverse.get(c.name) ?? c.name) };
56+
}
57+
```
58+
Replace the inline block in `renderRecreate` with a call to it.
59+
- [ ] **Step 4:** Run the new test AND `test/` sqlite/recreate suites (`bun test test/unit/sqlite-primitives-export.test.ts` + any `*recreate*`/`*sqlite*` unit test) — all PASS, SQLite output unchanged.
60+
- [ ] **Step 5:** Commit `refactor(#241): export SQLite rebuild primitives for D1 cascade reuse`.
61+
62+
---
63+
64+
### Task 2: FK-graph + topological sort + cycle detection
65+
66+
**Files:** Create `src/emit/fk-graph.ts`; Test `test/unit/fk-graph.test.ts`
67+
68+
Pure functions, no I/O. This is the graph core the cascade orders by.
69+
70+
**Interfaces (produce):**
71+
- `buildFkEdges(schema: SchemaSnapshot): Map<string, Set<string>>` — child→parent edges (`table` → set of `refTable`), self-loops included.
72+
- `unionEdges(a, b): Map<string,Set<string>>` — merge two edge maps.
73+
- `affectedSet(recreated: ReadonlySet<string>, edges: Map<string,Set<string>>): Set<string>` — `recreated` plus all transitive *referrers* (walk edges backwards: any table with an edge into the set).
74+
- `topoOrder(nodes: Set<string>, edges): { order: string[]; cycle: string[] | null }` — Kahn's algorithm over `nodes` (edges child→parent; ignore self-loops and edges leaving `nodes`). `order` = parents-first (a table appears after all tables it references within `nodes`). `cycle` = a multi-node cycle's members if the sort can't complete, else `null`.
75+
76+
- [ ] **Step 1 (test):** cover: `buildFkEdges` from a `{child→parent}` schema; a self-loop table maps to itself; `affectedSet` pulls a grandchild in via transitive referrer walk (`g→c→p`, recreated={p} → {p,c,g}); `topoOrder` returns parents-first for `g→c→p` (`[p,c,g]`) and self-loops don't block; `topoOrder` on `A↔B` returns `cycle=[A,B]`. Use the same `table()` fixture helper style as `test/unit/d1-fk-refuse.test.ts`.
77+
- [ ] **Step 2:** Run — FAIL (module missing).
78+
- [ ] **Step 3 (impl):** Implement the four functions. `affectedSet`: BFS/worklist adding any `x` where `edges.get(x)` intersects the current set (referrer walk), until fixpoint. `topoOrder`: Kahn over `nodes`, treating an edge `child→parent` as "parent must come before child"; drop self-loops (`x→x`) and edges whose target ∉ `nodes`; if nodes remain when the queue empties, return them as `cycle`.
79+
- [ ] **Step 4:** Run — PASS.
80+
- [ ] **Step 5:** Commit `feat(#241): FK-graph, affected-set, and topological sort with cycle detection`.
81+
82+
---
83+
84+
### Task 3: Thread `actualSchema` through `emit` + the CLI call sites
85+
86+
**Files:** Modify `src/emit/index.ts`; Modify `server/typescript/packages/cli/src/commands/migrate.ts`; Test `test/unit/emit-actualschema-passthrough.test.ts`
87+
88+
**Interfaces:** `EmitOptions` gains `actualSchema?: SchemaSnapshot`; `emit` passes it to `renderD1` as a 4th arg `renderD1(changes, expectedSchema, actualMeta, actualSchema)`.
89+
90+
- [ ] **Step 1 (test):** Assert `emit(changes, { dialect:"d1", expectedSchema, actualSchema })` forwards `actualSchema` — simplest: a d1 referenced-rebuild that would refuse WITHOUT actualSchema but (after Task 4) cascades WITH it; for THIS task, assert the option is accepted and a plain non-referenced d1 emit is byte-identical whether or not `actualSchema` is passed.
91+
- [ ] **Step 2:** Run — FAIL (type error: `actualSchema` not on `EmitOptions`) — or write the test to first fail on behavior.
92+
- [ ] **Step 3 (impl):** Add `actualSchema?: SchemaSnapshot` to `EmitOptions` (doc-comment: used by the d1 cascade to build the actual∪expected FK graph). In `emit`, `case "d1": return renderD1(opts.changes... , opts.expectedSchema, opts.actualMeta, opts.actualSchema);` (extend `renderD1`'s signature to accept the optional 4th arg; unused until Task 4). In `cli/src/commands/migrate.ts`, at the live-introspection emit call (~440) add `actualSchema: actual`, and at the offline emit call (~833) add `actualSchema: snapshot`. Both `actual` and `snapshot` are `SchemaSnapshot` in scope there.
93+
- [ ] **Step 4:** Run the new test + `test/unit/emit-d1.test.ts` + `emit-d1-refuse.test.ts` — PASS (still byte-identical; refuse still fires for a referenced rebuild since Task 4 isn't in yet).
94+
- [ ] **Step 5:** Commit `feat(#241): thread actualSchema through emit() to the d1 emitter`.
95+
96+
---
97+
98+
### Task 4: The cascade emitter + `renderD1` dispatch (the meat)
99+
100+
**Files:** Create `src/emit/d1-cascade.ts`; Modify `src/emit/d1.ts`; Modify `src/emit/d1-fk-refuse.ts` (repurpose the error message for cycles); Test `test/unit/d1-cascade.test.ts`
101+
102+
**`d1-cascade.ts` — `emitD1Cascade(changes, expectedSchema, actualSchema | undefined, recreatedTables): { up: string; downWarning: string } | { refuseCycle: string[] }`:**
103+
1. `edges = unionEdges(buildFkEdges(expectedSchema), actualSchema ? buildFkEdges(actualSchema) : emptyEdges)`.
104+
2. `A = affectedSet(recreatedTables, edges)`.
105+
3. `{ order, cycle } = topoOrder(A, edges)`. If `cycle` → return `{ refuseCycle: cycle }`.
106+
4. Build statements in the validated order:
107+
- `PRAGMA defer_foreign_keys = ON;`
108+
- For each `t` in `A` (any order): a `CREATE TABLE __f_<t>` from `t`'s **expected** `TableDescriptor` (via `renderCreateTable`) BUT with the temp name `__f_<t>` and every FK whose `refTable ∈ A` rewritten to `__f_<refTable>`. (Build a modified `TableDescriptor` clone: `name:"__f_"+t`, `foreignKeys` mapped.) Indexes are recreated AFTER rename (step below), matching `renderRecreate`.
109+
- For each `t` in `A`: `INSERT INTO __f_<t> (insertCols) SELECT selectCols FROM <t>` using `computeCarryColumns(changesForTable(t), expectedTable(t))`. For a table in `A` but NOT in `recreatedTables` (pulled in as a referrer, no changes), `changesForTable` is `[]` → carry = all expected columns by name.
110+
- `DROP TABLE <t>` for each `t` in **reverse `order`** (referrers-first).
111+
- `ALTER TABLE __f_<t> RENAME TO <t>` for each `t` in **`order`** (parents-first).
112+
- After each rename, recreate that table's indexes (`renderCreateIndex(t, ix)` for `ix` in the expected descriptor's `indexes`).
113+
5. `downWarning` = the same best-effort `-- WARNING` block `renderRecreate` emits.
114+
115+
Notes: reuse `renderCreateTable`/`renderCreateIndex`/`computeCarryColumns` from Task 1. `changesForTable(t)` = `changes.filter(c => changeTable(c) === t)` — you may need to export `changeTable` from sqlite.ts too (add to Task 1 if so).
116+
117+
**`d1.ts` (`renderD1`) dispatch:** when `sqliteResult.recreatedTables` intersects the FK-referenced set (existing `findReferencedRebuilds` on `expectedSchema` OR the actual graph):
118+
- If `actualSchema` is undefined AND there is a referenced rebuild → refuse (as #226 does today) — never emit an unproven cascade.
119+
- Else call `emitD1Cascade(...)`. On `{ refuseCycle }` → throw the (repurposed) error naming the cycle. On success → the cascade `up` REPLACES the recreate portion of the sqlite output. Simplest correct integration: when a cascade is needed, emit the cascade for the affected-set tables and let renderSqlite handle all OTHER (non-A) changes; splice by emitting cascade statements first, then the non-recreate, non-A changes via the existing native path. (Detail for the implementer: the cleanest approach is to compute the cascade statements for `A`, then append the sqlite-native emit of all changes whose table ∉ `A` — reuse `renderSqlite` filtered, or emit the cascade and the remaining `renderUpNative` changes. Verify against the gate that a mixed migration converges.)
120+
- Run the result through `applyD1SafetyPass`; do NOT emit the `PRAGMA foreign_keys=OFF/ON` bracket on the cascade path.
121+
122+
**`d1-fk-refuse.ts`:** repurpose `D1ReferencedTableRebuildError` (or add `D1CyclicForeignKeyError`) so the thrown message now describes the **cycle** case (naming the cycle members + the hand-write workaround), since acyclic referenced rebuilds no longer throw.
123+
124+
- [ ] **Step 1 (test):** `test/unit/d1-cascade.test.ts` — build `changes` + expected/actual schemas via literal fixtures (mirror `emit-d1-refuse.test.ts`). Assert: (a) a referenced-parent rebuild with `actualSchema` produces cascade SQL containing `defer_foreign_keys`, `__f_parent`, referrers-first `DROP`, parents-first `RENAME`, and NO `foreign_keys = OFF`; (b) a multi-table cycle throws the cycle error; (c) a self-ref table produces a single `__f_t` referencing `__f_t`; (d) `actualSchema` absent → still refuses; (e) a non-referenced rebuild → byte-identical to the pre-#241 sqlite-through-safety-pass output.
125+
- [ ] **Step 2:** Run — FAIL.
126+
- [ ] **Step 3 (impl):** Implement `emitD1Cascade` + wire `renderD1` + the error repurpose.
127+
- [ ] **Step 4:** Run `test/unit/d1-cascade.test.ts` + `emit-d1.test.ts` + `emit-d1-refuse.test.ts` (update the latter: acyclic referenced rebuilds now CASCADE, not throw — the throw tests move to the cycle case) — PASS.
128+
- [ ] **Step 5:** Commit `feat(#241): D1 FK-cascade emitter — rebuild referenced tables; refuse only cycles`.
129+
130+
---
131+
132+
### Task 5: libSQL real-engine gate (per topology + gap + cycle)
133+
134+
**Files:** Modify/extend `test/integration/d1-referenced-rebuild.test.ts` (or a new `test/integration/d1-cascade.test.ts`)
135+
136+
Reuse the #226 harness: raw `libsql.createClient` (from `@libsql/kysely-libsql`), `foreign_keys=ON` at the connection, the whole emitted batch run inside ONE `client.transaction("write")` (models remote D1). For each scenario: seed schema+data, emit the D1 migration (`emit({dialect:"d1", expectedSchema, actualSchema, ...})`), apply in one transaction, then assert **applies cleanly, row data intact, FK re-enforced (a bad insert rejected), and a re-diff is EMPTY**.
137+
138+
- [ ] **Step 1 (tests):** scenarios — single parent + populated child; transitive `g→c→p`; multiple children; self-referential; **the #226 gap** (one migration that rebuilds a parent AND drops a child's FK to it — must apply cleanly, proving the actual-graph ordering); a multi-table cycle → `emit` throws (unit-level assertion, no apply); a no-referenced-rebuild migration → D1 `up` byte-identical to sqlite-through-safety-pass. Use the full pipeline (`buildExpectedSchema → introspectSqlite → diff → emit → apply → re-diff`) for the convergence assertions, mirroring `test/integration/sqlite-fk-convergence.test.ts` + the #226 gate.
139+
- [ ] **Step 2:** Run — expect PASS once Task 4 is in (this task is the gate for Task 4). If a convergence assertion fails, the defect is in Task 2/4; fix there and re-run.
140+
- [ ] **Step 3:** Run the full migrate-ts suite once: `bun test` (from the migrate-ts dir; libSQL is file-based, no Docker for these — but the suite also has Postgres Testcontainers tests, so this may be slow; scope to the d1/sqlite/emit tests if Docker is unavailable and note it).
141+
- [ ] **Step 4:** Commit `test(#241): real-engine cascade gate — all topologies + #226 gap + cycle refuse`.
142+
143+
---
144+
145+
### Task 6: Docs + CHANGELOG + close #241
146+
147+
**Files:** Modify `docs/features/migrations-and-drift.md`; Modify `CHANGELOG.md`
148+
149+
- [ ] **Step 1:** In `migrations-and-drift.md`, update the D1 limitation subsection: the emitter now **auto-generates a cascade** to rebuild an FK-referenced table on D1; **remove the "Known limitation of the current refusal" paragraph** (the residual gap is closed); the only remaining hand-write case is a **multi-table FK cycle** (still refused, with guidance).
150+
- [ ] **Step 2:** `CHANGELOG.md` under `## [Unreleased]` (create if absent): `### Added — D1 auto-cascade for rebuilding foreign-key-referenced tables (#241)` — npm-only (`migrate-ts`); describe the cascade, that it closes #226's residual gap, and that multi-table cycles are still refused.
151+
- [ ] **Step 3:** Commit `docs(#241): document the D1 FK-cascade; remove the refusal known-limitation`.
152+
153+
---
154+
155+
## Self-Review
156+
157+
**Spec coverage:** cascade algorithm → Task 4; affected-set/topo/cycle → Task 2; actual∪expected graph + plumbing → Task 3 + Task 4; reuse sqlite primitives → Task 1; refuse cycles only → Task 4; gate per topology + gap + cycle + byte-identical → Task 5; docs/remove-known-limitation → Task 6. Closes #226 residual gap via the union graph (Task 4 uses `actualSchema`).
158+
159+
**Placeholder scan:** Task 4's integration paragraph gives a named approach ("cascade for A + native emit for non-A changes") with the convergence gate as the acceptance test — a concrete strategy with a verification, not a TODO. The emitter's exact statement assembly is specified as the validated recipe + the primitives to call; the implementer assembles it and the Task 5 gate proves it.
160+
161+
**Type consistency:** `buildFkEdges`/`unionEdges`/`affectedSet`/`topoOrder` (Task 2) are consumed by `emitD1Cascade` (Task 4) with matching signatures. `computeCarryColumns`/`renderCreateTable`/`renderCreateIndex` (Task 1) reused in Task 4. `EmitOptions.actualSchema?: SchemaSnapshot` (Task 3) consumed by `renderD1`→`emitD1Cascade` (Task 4). `SchemaSnapshot`/`TableDescriptor`/`FkDescriptor`/`Change` are the existing `types.ts` shapes.

0 commit comments

Comments
 (0)