fix(artifacts): tighten adopt shape for nullability and CHECKs

Empty-ledger adopt only checked column presence and types, so a schema
missing tenant_id NOT NULL or the 0003 version/size CHECKs could still be
stamped. Validate those invariants too, document the bar, and clean up
migration tests that left half-applied state for later runs.

Author: TheGreatAxios

ARCHITECTURE.md

@@ -213,6 +213,10 @@ every boot of every replica.
 - A later ledgered migration sets `artifact.tenant_id NOT NULL` and adds the
   version/size CHECKs. If null-tenant rows still exist, that migration raises
   before altering the column so the operator can clean them up first.
+- Empty ledger + pre-existing package objects fails closed
+  (`MigrationAdoptError`). `{ adopt: true }` records checksums without re-DDL
+  only after shape validation: tables, column types, required nullability, and
+  the named CHECK constraints. Column presence alone is not enough.
 
 **The package owns its own Postgres schema.** Every table, index and the ledger
 live in `artifacts`, created by the runner and qualified in every

packages/artifacts/README.md

@@ -226,6 +226,12 @@ install multi-table triggers for that alignment (see ARCHITECTURE.md).
 replica: concurrent cold starts serialize on a transaction-scoped advisory lock, and a
 re-run prints nothing.
 
+If the ledger is empty but package tables already exist (restored dump, dropped
+ledger), the runner fails closed with `MigrationAdoptError`. Operators who have
+confirmed the live schema may pass `{ adopt: true }` to record checksums without
+re-running DDL. Adopt validates tables, column types, `artifact.tenant_id NOT NULL`,
+and the named version/size CHECK constraints — not a columns-only glance.
+
 Event timestamps are `timestamptz` so list keyset cursors and date filters stay
 stable under a non-UTC session `TimeZone`. A ledgered retype migration converts
 legacy zoneless columns with `USING col AT TIME ZONE 'UTC'` (existing walls were

packages/artifacts/src/artifacts.test.ts

@@ -74,7 +74,7 @@ describe("create", () => {
       files: { "index.html": "<p>hi</p>" },
     });
   });
-test("rejects oversize title and content before insert", async () => {
+  test("rejects oversize title and content before insert", async () => {
     const db = await testDb();
     await expect(
       db.transaction((tx) =>

packages/artifacts/src/migrations.test.ts

@@ -415,6 +415,68 @@ describe("migrations", () => {
     expect(again.length).toBe(MIGRATIONS.length);
   });
 
+  test("empty ledger + columns without 0003 CHECKs refuses adopt", async () => {
+    assertDestructiveArtifactTestsAllowed(DATABASE_URL);
+    await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`);
+    // Honest migrate, drop ledger, then strip a row-local CHECK so columns and
+    // types still match but 0003 invariants are gone — adopt must not stamp.
+    await runArtifactMigrations(db);
+    await db.execute(
+      sql`DROP TABLE ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)}`,
+    );
+    await db.execute(sql`
+      ALTER TABLE ${sql.identifier(SCHEMA)}."artifact"
+        DROP CONSTRAINT "artifact_version_gte_1"
+    `);
+
+    await expect(
+      runArtifactMigrations(db, { adopt: true }),
+    ).rejects.toThrow(MigrationAdoptError);
+    await expect(
+      runArtifactMigrations(db, { adopt: true }),
+    ).rejects.toThrow(/artifact_version_gte_1/);
+
+    const ledgerTables = await db.execute<{ n: number }>(sql`
+      SELECT count(*)::int AS n FROM information_schema.tables
+      WHERE table_schema = ${SCHEMA} AND table_name = ${LEDGER}
+    `);
+    expect(ledgerTables[0]!.n).toBe(0);
+
+    // Later tests call runArtifactMigrations without a reset; restore a
+    // fully-ledgered schema so they are not stranded on an empty ledger.
+    await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`);
+    await runArtifactMigrations(db);
+  });
+
+  test("empty ledger + nullable tenant_id refuses adopt", async () => {
+    assertDestructiveArtifactTestsAllowed(DATABASE_URL);
+    await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`);
+    await runArtifactMigrations(db);
+    await db.execute(
+      sql`DROP TABLE ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)}`,
+    );
+    await db.execute(sql`
+      ALTER TABLE ${sql.identifier(SCHEMA)}."artifact"
+        ALTER COLUMN "tenant_id" DROP NOT NULL
+    `);
+
+    await expect(
+      runArtifactMigrations(db, { adopt: true }),
+    ).rejects.toThrow(MigrationAdoptError);
+    await expect(
+      runArtifactMigrations(db, { adopt: true }),
+    ).rejects.toThrow(/tenant_id.*NOT NULL/i);
+
+    const ledgerTables = await db.execute<{ n: number }>(sql`
+      SELECT count(*)::int AS n FROM information_schema.tables
+      WHERE table_schema = ${SCHEMA} AND table_name = ${LEDGER}
+    `);
+    expect(ledgerTables[0]!.n).toBe(0);
+
+    await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`);
+    await runArtifactMigrations(db);
+  });
+
   /**
    * DB invariants: tenant_id is required on every artifact row; version and size
    * stay non-negative. Principal↔tenant alignment is host-owned (resolvePrincipal)
@@ -567,5 +629,10 @@ describe("migrations", () => {
       "0001_artifacts",
       "0002_timestamptz",
     ]);
+
+    // Full suite (and re-runs of this file) must not inherit null-tenant rows
+    // and a half-applied ledger.
+    await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`);
+    await runArtifactMigrations(db);
   });
 });

packages/artifacts/src/migrations.ts

@@ -249,65 +249,87 @@ export class MigrationAdoptError extends Error {
  * `adopt` is an operator escape hatch for the rare case where package tables
  * already exist (restored dump, manual DDL, ledger dropped) and the operator
  * has confirmed they match the expected shape. It is never set by default and
- * must not be passed on ordinary boots.
+ * must not be passed on ordinary boots. Adopt validates tables, column types,
+ * required nullability (`artifact.tenant_id`), and the named CHECK constraints
+ * from `0003_schema_invariants` before writing ledger rows — it does not
+ * re-run DDL.
  */
 export type RunArtifactMigrationsOptions = {
   adopt?: boolean;
 };
 
 /** Package-owned tables (excluding the ledger) and the columns each must have. */
-const EXPECTED_OWNED_SHAPE: Readonly<
-  Record<string, readonly { name: string; udt: string }[]>
-> = {
-  artifact: [
-    { name: "id", udt: "text" },
-    { name: "tenant_id", udt: "text" },
-    { name: "principal_id", udt: "text" },
-    { name: "owner_principal_id", udt: "text" },
-    { name: "kind", udt: "text" },
-    { name: "title", udt: "text" },
-    { name: "content", udt: "text" },
-    { name: "source", udt: "jsonb" },
-    { name: "version", udt: "int4" },
-    { name: "archived_at", udt: "timestamptz" },
-    { name: "created_at", udt: "timestamptz" },
-    { name: "updated_at", udt: "timestamptz" },
-  ],
-  artifact_version: [
-    { name: "id", udt: "text" },
-    { name: "artifact_id", udt: "text" },
-    { name: "version", udt: "int4" },
-    { name: "title", udt: "text" },
-    { name: "content", udt: "text" },
-    { name: "author_id", udt: "text" },
-    { name: "created_at", udt: "timestamptz" },
-  ],
-  upload: [
-    { name: "id", udt: "text" },
-    { name: "tenant_id", udt: "text" },
-    { name: "principal_id", udt: "text" },
-    { name: "filename", udt: "text" },
-    { name: "mime_type", udt: "text" },
-    { name: "content", udt: "bytea" },
-    { name: "size", udt: "int4" },
-    { name: "created_at", udt: "timestamptz" },
-  ],
-  mail_attachment_ref: [
-    { name: "id", udt: "text" },
-    { name: "tenant_id", udt: "text" },
-    { name: "principal_id", udt: "text" },
-    { name: "instance_id", udt: "text" },
-    { name: "mail_id", udt: "text" },
-    { name: "artifact_id", udt: "text" },
-    { name: "name", udt: "text" },
-    { name: "mime_type", udt: "text" },
-    { name: "size", udt: "int4" },
-    { name: "created_at", udt: "timestamptz" },
-  ],
+type ExpectedColumn = {
+  name: string;
+  udt: string;
+  /** When set, live `is_nullable` must be `NO`. */
+  notNull?: true;
 };
 
-async function listOwnedTables(tx: ArtifactTx): Promise<string[]> {
+const EXPECTED_OWNED_SHAPE: Readonly<Record<string, readonly ExpectedColumn[]>> =
+  {
+    artifact: [
+      { name: "id", udt: "text" },
+      { name: "tenant_id", udt: "text", notNull: true },
+      { name: "principal_id", udt: "text" },
+      { name: "owner_principal_id", udt: "text" },
+      { name: "kind", udt: "text" },
+      { name: "title", udt: "text" },
+      { name: "content", udt: "text" },
+      { name: "source", udt: "jsonb" },
+      { name: "version", udt: "int4" },
+      { name: "archived_at", udt: "timestamptz" },
+      { name: "created_at", udt: "timestamptz" },
+      { name: "updated_at", udt: "timestamptz" },
+    ],
+    artifact_version: [
+      { name: "id", udt: "text" },
+      { name: "artifact_id", udt: "text" },
+      { name: "version", udt: "int4" },
+      { name: "title", udt: "text" },
+      { name: "content", udt: "text" },
+      { name: "author_id", udt: "text" },
+      { name: "created_at", udt: "timestamptz" },
+    ],
+    upload: [
+      { name: "id", udt: "text" },
+      { name: "tenant_id", udt: "text" },
+      { name: "principal_id", udt: "text" },
+      { name: "filename", udt: "text" },
+      { name: "mime_type", udt: "text" },
+      { name: "content", udt: "bytea" },
+      { name: "size", udt: "int4" },
+      { name: "created_at", udt: "timestamptz" },
+    ],
+    mail_attachment_ref: [
+      { name: "id", udt: "text" },
+      { name: "tenant_id", udt: "text" },
+      { name: "principal_id", udt: "text" },
+      { name: "instance_id", udt: "text" },
+      { name: "mail_id", udt: "text" },
+      { name: "artifact_id", udt: "text" },
+      { name: "name", udt: "text" },
+      { name: "mime_type", udt: "text" },
+      { name: "size", udt: "int4" },
+      { name: "created_at", udt: "timestamptz" },
+    ],
+  };
 
+/**
+ * Row-local CHECKs applied by `0003_schema_invariants`. Adopt must see these
+ * names so stamping 0003 without the constraints cannot pass shape validation.
+ */
+const EXPECTED_CHECK_CONSTRAINTS: ReadonlyArray<{
+  table: string;
+  name: string;
+}> = [
+  { table: "artifact", name: "artifact_version_gte_1" },
+  { table: "artifact_version", name: "artifact_version_version_gte_1" },
+  { table: "upload", name: "upload_size_gte_0" },
+  { table: "mail_attachment_ref", name: "mail_attachment_ref_size_gte_0" },
+];
+
+async function listOwnedTables(tx: ArtifactTx): Promise<string[]> {
   const rows = await tx.execute<{ table_name: string }>(sql`
     SELECT table_name FROM information_schema.tables
     WHERE table_schema = ${ARTIFACTS_SCHEMA}
@@ -319,11 +341,11 @@ async function listOwnedTables(tx: ArtifactTx): Promise<string[]> {
 }
 
 /**
- * Compare live catalogue columns against the shape the migrations create.
- * Returns a human-readable mismatch list (empty when compatible).
+ * Compare live catalogue against the shape the migrations create: tables,
+ * column types, required nullability, and named CHECK constraints. Returns a
+ * human-readable mismatch list (empty when compatible).
  */
 async function shapeMismatches(tx: ArtifactTx): Promise<string[]> {
-
   const owned = await listOwnedTables(tx);
   const expectedTables = Object.keys(EXPECTED_OWNED_SHAPE).sort();
   const mismatches: string[] = [];
@@ -347,38 +369,80 @@ async function shapeMismatches(tx: ArtifactTx): Promise<string[]> {
     table_name: string;
     column_name: string;
     udt_name: string;
+    is_nullable: string;
   }>(sql`
-    SELECT table_name, column_name, udt_name
+    SELECT table_name, column_name, udt_name, is_nullable
     FROM information_schema.columns
     WHERE table_schema = ${ARTIFACTS_SCHEMA}
       AND table_name <> ${LEDGER_TABLE}
   `);
 
-  const byTable = new Map<string, Map<string, string>>();
+  const byTable = new Map<string, Map<string, { udt: string; nullable: string }>>();
   for (const col of columns) {
     let cols = byTable.get(col.table_name);
     if (!cols) {
       cols = new Map();
       byTable.set(col.table_name, cols);
     }
-    cols.set(col.column_name, col.udt_name);
+    cols.set(col.column_name, {
+      udt: col.udt_name,
+      nullable: col.is_nullable,
+    });
   }
 
   for (const table of expectedTables) {
     const expectedCols = EXPECTED_OWNED_SHAPE[table]!;
     const live = byTable.get(table) ?? new Map();
-    for (const { name, udt } of expectedCols) {
-      const liveUdt = live.get(name);
-      if (liveUdt === undefined) {
-        mismatches.push(`missing column ${ARTIFACTS_SCHEMA}.${table}.${name}`);
-      } else if (liveUdt !== udt) {
+    for (const expected of expectedCols) {
+      const liveCol = live.get(expected.name);
+      if (liveCol === undefined) {
         mismatches.push(
-          `column ${ARTIFACTS_SCHEMA}.${table}.${name} has type ${liveUdt}, expected ${udt}`,
+          `missing column ${ARTIFACTS_SCHEMA}.${table}.${expected.name}`,
+        );
+      } else if (liveCol.udt !== expected.udt) {
+        mismatches.push(
+          `column ${ARTIFACTS_SCHEMA}.${table}.${expected.name} has type ${liveCol.udt}, expected ${expected.udt}`,
+        );
+      } else if (expected.notNull && liveCol.nullable !== "NO") {
+        mismatches.push(
+          `column ${ARTIFACTS_SCHEMA}.${table}.${expected.name} is nullable, expected NOT NULL`,
         );
       }
     }
   }
 
+  if (mismatches.length > 0) return mismatches;
+
+  // Named CHECKs from 0003 — without these, adopt would stamp the ledger over a
+  // schema that never gained the row-local invariants.
+  const checks = await tx.execute<{ table_name: string; constraint_name: string }>(
+    sql`
+      SELECT c.relname AS table_name, con.conname AS constraint_name
+      FROM pg_catalog.pg_constraint con
+      JOIN pg_catalog.pg_class c ON c.oid = con.conrelid
+      JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
+      WHERE n.nspname = ${ARTIFACTS_SCHEMA}
+        AND con.contype = 'c'
+    `,
+  );
+  const checkByTable = new Map<string, Set<string>>();
+  for (const row of checks) {
+    let names = checkByTable.get(row.table_name);
+    if (!names) {
+      names = new Set();
+      checkByTable.set(row.table_name, names);
+    }
+    names.add(row.constraint_name);
+  }
+  for (const { table, name } of EXPECTED_CHECK_CONSTRAINTS) {
+    const live = checkByTable.get(table);
+    if (!live?.has(name)) {
+      mismatches.push(
+        `missing CHECK constraint ${name} on ${ARTIFACTS_SCHEMA}.${table}`,
+      );
+    }
+  }
+
   return mismatches;
 }
 
@@ -394,9 +458,10 @@ async function shapeMismatches(tx: ArtifactTx): Promise<string[]> {
  *
  * When the ledger is empty but package-owned objects already exist, the runner
  * fails closed with {@link MigrationAdoptError} unless `{ adopt: true }` is
- * passed and the live shape matches what the migrations would create. That
- * path records checksums without re-running DDL. Ledger checksum drift still
- * throws {@link MigrationChecksumError}.
+ * passed and the live shape matches what the migrations would create (tables,
+ * column types, required nullability, and named CHECK constraints). That path
+ * records checksums without re-running DDL. Ledger checksum drift still throws
+ * {@link MigrationChecksumError}.
  */
 export async function runArtifactMigrations(
   db: ArtifactDb,