Order migration 0021 so it never drops a table with children

src/services/extensions/v2/README.md

@@ -104,15 +104,22 @@ guaranteeing a published row is never half-written. `extension_revisions`
 (renamed from `extension_submissions` in migration 0021) holds proposed
 content, always attached to a real extension row and cascading with it.
 
-Migration 0021 rebuilds `extensions`, `extension_revisions` and `developers`
-in one step, because SQLite cannot relax `NOT NULL`, add a `CHECK`, or add a
-foreign key in place. It also renames `extensions.author_id` to `developer_id`
-(nothing public depended on the old name — v1's response field is `author`
-either way) and replaces `developers.created_at`/`updated_at`'s placeholder
-1970 default with `CURRENT_TIMESTAMP`, which is what every writer already
-uses. Existing 1970 values are left alone: they are the only record those rows
-have, and a timestamp invented at migration time would look real without being
-so.
+Migration 0021 rebuilds `extensions` and replaces `extension_submissions` with
+`extension_revisions`, because SQLite cannot relax `NOT NULL`, add a `CHECK`, or
+add a foreign key in place. It also renames `extensions.author_id` to
+`developer_id` — nothing public depended on the old name, since v1's response
+field is `author` either way.
+
+**Ordering in 0021 is load-bearing.** It never drops a table that still has
+children, so `extension_submissions` is copied aside and dropped before
+`extensions` is rebuilt. Foreign keys cannot be relaxed to avoid this:
+`PRAGMA foreign_keys` is a no-op inside a transaction and wrangler wraps each
+migration file in one, while `PRAGMA defer_foreign_keys` does not help either —
+dropping a parent increments SQLite's deferred-violation counter per child row
+and nothing decrements it, so the commit fails even when the data is sound. The
+same constraint is why `developers` is not rebuilt: three tables reference it.
+`migrations.test.ts` applies the chain under those conditions so this cannot
+regress.
 
 Apply migrations **only from this repository**, from `db/migrations`, with `npm run db:migrate:extensions-v2:local` / `:remote`. The Extensions site has no D1 migration source.
 

src/services/extensions/v2/db/migrations/0021_restructure_extensions_revisions.sql

@@ -7,17 +7,26 @@
 -- the first approval), and extension_submissions becomes extension_revisions:
 -- one proposed content version, always attached to a real extension row.
 --
--- The tables are rebuilt rather than ALTERed because SQLite cannot relax NOT
--- NULL, add a CHECK, or add a foreign key in place. Two renames ride along,
--- since the rebuild is already paid for: extensions.author_id becomes
--- developer_id, and developers' created_at/updated_at lose the placeholder 1970
--- default that migration 0002 was forced to use and no writer ever produced.
+-- extensions is rebuilt rather than ALTERed because SQLite cannot relax NOT
+-- NULL, add a CHECK, or add a foreign key in place. author_id becomes
+-- developer_id on the way through, since the rebuild is already paid for.
 --
 -- Hand-written, not drizzle-kit-generated: the generated diff cannot infer the
 -- table rename or the backfills below non-interactively, so only the snapshot
 -- in meta/0021_snapshot.json comes from drizzle-kit. The end state is verified
 -- against schema.ts by test/services/extensions/v2/migrations.test.ts.
-PRAGMA foreign_keys=OFF;--> statement-breakpoint
+--
+-- The ordering below is load-bearing: nothing here drops a table that still
+-- has children, which is why extension_submissions is copied aside and dropped
+-- before extensions is rebuilt. Neither pragma can buy you out of this.
+-- foreign_keys=OFF is a no-op inside a transaction, and wrangler wraps each
+-- migration file in one - an earlier version of this file opened with it,
+-- passed locally where statements run outside a transaction, and failed the
+-- first remote apply. defer_foreign_keys is not a substitute either: DROP
+-- TABLE on a parent increments SQLite's deferred-violation counter once per
+-- child row and nothing ever decrements it, so COMMIT fails even when
+-- foreign_key_check is clean. It is also why developers is not rebuilt here -
+-- three tables reference it. See migrations.test.ts's applyAllAsD1().
 
 -- idx_extensions_id_nocase, created further down, is the constraint that stops
 -- a new lowercase id colliding with an adopted mixed-case one. A catalogue
@@ -48,6 +57,22 @@ GROUP BY LOWER(id) HAVING COUNT(*) > 1;--> statement-breakpoint
 
 DROP TABLE _extension_id_case_conflicts;--> statement-breakpoint
 
+-- A dangling developer reference would be caught by the real foreign key on
+-- the rebuilt table, but as a bare "FOREIGN KEY constraint failed" from the
+-- middle of the copy. Check it here, before anything is copied, so the failure
+-- names what is wrong and points at the rows.
+CREATE TABLE _unresolved_references (
+  kind TEXT NOT NULL,
+  row_id TEXT NOT NULL,
+  CONSTRAINT extension_references_must_resolve CHECK (1 = 0)
+);--> statement-breakpoint
+
+INSERT INTO _unresolved_references (kind, row_id)
+SELECT 'extension.developer_id', e.id FROM extensions e
+WHERE NOT EXISTS (SELECT 1 FROM developers d WHERE d.id = e.author_id);--> statement-breakpoint
+
+DROP TABLE _unresolved_references;--> statement-breakpoint
+
 -- A submission naming a developer that does not exist cannot become an
 -- extension row: developer_id is NOT NULL with a foreign key. Such a
 -- submission is already unapprovable today - the pre-0021 approve() only ever
@@ -104,59 +129,12 @@ WHERE LOWER(COALESCE(extension_id, json_extract(payload, '$.extension.id')))
 
 DROP TABLE _reserved_submission_targets;--> statement-breakpoint
 
--- developers first, while every table that references it is still the old one:
--- the drop-and-rename re-parses every schema, and doing it with a referrer
--- pointing at a dropped table is the case that errors.
-CREATE TABLE `__new_developers` (
-	`id` text PRIMARY KEY NOT NULL,
-	`type` text NOT NULL,
-	`name` text NOT NULL,
-	`url` text,
-	`owner_user_id` text,
-	`approved_at` text,
-	`created_at` text DEFAULT CURRENT_TIMESTAMP NOT NULL,
-	`updated_at` text DEFAULT CURRENT_TIMESTAMP NOT NULL,
-	`avatar_url` text,
-	`contact_email` text,
-	`ownership_epoch` integer DEFAULT 1 NOT NULL,
-	`content_revision` integer DEFAULT 1 NOT NULL,
-	`approved_revision` integer,
-	`approved_by` text,
-	`github_org_verified` integer,
-	`github_verification_note` text,
-	`github_verified_at` text,
-	`github_url_verified` integer,
-	`url_check_cooldown_until` text,
-	FOREIGN KEY (`owner_user_id`) REFERENCES `users`(`id`) ON UPDATE no action ON DELETE no action,
-	CONSTRAINT "developers_ownership_epoch_check" CHECK("__new_developers"."ownership_epoch" >= 1),
-	CONSTRAINT "developers_content_revision_check" CHECK("__new_developers"."content_revision" >= 1),
-	CONSTRAINT "developers_github_org_verified_check" CHECK("__new_developers"."github_org_verified" IN (0, 1)),
-	CONSTRAINT "developers_github_url_verified_check" CHECK("__new_developers"."github_url_verified" = 1)
-);--> statement-breakpoint
-
--- Existing 1970 values stay. They are wrong, but they are the only record
--- those rows have, and a timestamp invented here would look real without
--- being so.
-INSERT INTO `__new_developers` (
-  id, type, name, url, owner_user_id, approved_at, created_at, updated_at,
-  avatar_url, contact_email, ownership_epoch, content_revision,
-  approved_revision, approved_by, github_org_verified,
-  github_verification_note, github_verified_at, github_url_verified,
-  url_check_cooldown_until
-)
-SELECT
-  id, type, name, url, owner_user_id, approved_at, created_at, updated_at,
-  avatar_url, contact_email, ownership_epoch, content_revision,
-  approved_revision, approved_by, github_org_verified,
-  github_verification_note, github_verified_at, github_url_verified,
-  url_check_cooldown_until
-FROM `developers`;--> statement-breakpoint
-
-DROP TABLE `developers`;--> statement-breakpoint
-ALTER TABLE `__new_developers` RENAME TO `developers`;--> statement-breakpoint
+-- extension_submissions is the only table referencing extensions, so it goes
+-- first. AS SELECT rather than a declared table: it carries no constraints
+-- across, so the holding table survives the rebuild it spans.
+CREATE TABLE `_submissions_backup` AS SELECT * FROM `extension_submissions`;--> statement-breakpoint
 
-CREATE UNIQUE INDEX `idx_developers_owner_unique` ON `developers` (`owner_user_id`);--> statement-breakpoint
-CREATE INDEX `idx_developers_approved` ON `developers` (`approved_at`);--> statement-breakpoint
+DROP TABLE `extension_submissions`;--> statement-breakpoint
 
 CREATE TABLE `__new_extensions` (
 	`id` text PRIMARY KEY NOT NULL,
@@ -216,7 +194,7 @@ SELECT
   target.target_id,
   (
     SELECT s.developer_id
-    FROM extension_submissions s
+    FROM _submissions_backup s
     WHERE LOWER(COALESCE(s.extension_id, json_extract(s.payload, '$.extension.id'))) = target.target_id
     ORDER BY s.created_at DESC, s.id DESC
     LIMIT 1
@@ -227,7 +205,7 @@ SELECT
 FROM (
   SELECT DISTINCT
     LOWER(COALESCE(extension_id, json_extract(payload, '$.extension.id'))) AS target_id
-  FROM extension_submissions
+  FROM _submissions_backup
 ) AS target
 WHERE target.target_id IS NOT NULL
   AND NOT EXISTS (
@@ -288,7 +266,7 @@ SELECT
   s.created_at,
   s.reviewed_at,
   s.ownership_epoch
-FROM extension_submissions s
+FROM _submissions_backup s
 JOIN extensions e
   ON LOWER(e.id) = LOWER(COALESCE(s.extension_id, json_extract(s.payload, '$.extension.id')))
 -- A payload without an extension object cannot become a revision. This has
@@ -327,28 +305,4 @@ CREATE INDEX `idx_extension_revisions_extension_page` ON `extension_revisions` (
 CREATE INDEX `idx_extension_revisions_submitter_page` ON `extension_revisions` (`submitted_by`,"created_at" desc,"id" desc);--> statement-breakpoint
 CREATE INDEX `idx_extension_revisions_queue_page` ON `extension_revisions` (`status`,`created_at`,`id`);--> statement-breakpoint
 
-DROP TABLE `extension_submissions`;--> statement-breakpoint
-
--- The rebuilds above run with foreign_keys=OFF, which means SQLite does not
--- re-validate the copied rows against the new declarations - a pre-existing
--- extension pointing at a developer that no longer exists would be carried
--- through silently, and every read would then have to defend against it
--- forever. Fail the deploy instead, and let the reads assume the join always
--- matches. Same CHECK-on-a-scratch-table trick as migration 0020, for the
--- same reason: SQLite has no RAISE() outside a trigger.
-CREATE TABLE _unresolved_references (
-  kind TEXT NOT NULL,
-  row_id TEXT NOT NULL,
-  CONSTRAINT extension_references_must_resolve CHECK (1 = 0)
-);--> statement-breakpoint
-
-INSERT INTO _unresolved_references (kind, row_id)
-SELECT 'extension.developer_id', e.id FROM extensions e
-WHERE NOT EXISTS (SELECT 1 FROM developers d WHERE d.id = e.developer_id)
-UNION ALL
-SELECT 'revision.extension_id', r.id FROM extension_revisions r
-WHERE NOT EXISTS (SELECT 1 FROM extensions e WHERE e.id = r.extension_id);--> statement-breakpoint
-
-DROP TABLE _unresolved_references;--> statement-breakpoint
-
-PRAGMA foreign_keys=ON;
+DROP TABLE `_submissions_backup`;--> statement-breakpoint

src/services/extensions/v2/db/migrations/meta/0021_snapshot.json

@@ -394,15 +394,15 @@
           "primaryKey": false,
           "notNull": true,
           "autoincrement": false,
-          "default": "CURRENT_TIMESTAMP"
+          "default": "'1970-01-01T00:00:00.000Z'"
         },
         "updated_at": {
           "name": "updated_at",
           "type": "text",
           "primaryKey": false,
           "notNull": true,
           "autoincrement": false,
-          "default": "CURRENT_TIMESTAMP"
+          "default": "'1970-01-01T00:00:00.000Z'"
         },
         "avatar_url": {
           "name": "avatar_url",

src/services/extensions/v2/db/schema.ts

@@ -125,16 +125,18 @@ export const developers = sqliteTable(
     url: text("url"),
     ownerUserId: text("owner_user_id").references(() => users.id),
     approvedAt: text("approved_at"),
-    // Migration 0002 could only give these a constant default (SQLite rejects
-    // non-constant ALTER TABLE ADD COLUMN defaults), so they carried a
-    // placeholder 1970 epoch that no write ever produced. 0021 rebuilds the
-    // table and replaces it with the value every writer already uses.
-    createdAt: text("created_at")
-      .notNull()
-      .default(sql`CURRENT_TIMESTAMP`),
-    updatedAt: text("updated_at")
-      .notNull()
-      .default(sql`CURRENT_TIMESTAMP`),
+    // Placeholder default from migration 0002 (SQLite rejects non-constant
+    // ALTER TABLE ADD COLUMN defaults). Every write sets this explicitly (see
+    // db/developer-profiles.ts) - the literal default is never actually read,
+    // but it is part of the real column definition, so it is kept here for
+    // baseline-diff fidelity against the existing database.
+    //
+    // Replacing it needs a table rebuild, which this table cannot have: three
+    // tables reference it and a parent cannot be dropped with foreign keys
+    // enforced (see migration 0021's header). Rebuilding all three children
+    // too is a lot of risk for a default nothing reads.
+    createdAt: text("created_at").notNull().default("1970-01-01T00:00:00.000Z"),
+    updatedAt: text("updated_at").notNull().default("1970-01-01T00:00:00.000Z"),
     avatarUrl: text("avatar_url"),
     contactEmail: text("contact_email"),
     ownershipEpoch: integer("ownership_epoch").notNull().default(1),