diff --git a/packages/agent-directory/src/index.ts b/packages/agent-directory/src/index.ts index 5f6e1e7d2c..a40cc15464 100644 --- a/packages/agent-directory/src/index.ts +++ b/packages/agent-directory/src/index.ts @@ -30,12 +30,6 @@ export { AGENT_DEFINITION_ENTRY_PATH, RetiredWorkflowEnvelopeError, } from "./definition-asset"; -export { - agentDirectoryMigrations, - applyAgentDirectoryMigrations, - type AgentDirectoryMigration, - type ApplyAgentDirectoryMigrationsReport, -} from "./migrations"; export { CreateAgentDefinitionInput, RestoreDefinitionInput, diff --git a/packages/agent-directory/src/migrations.ts b/packages/agent-directory/src/migrations.ts deleted file mode 100644 index 094ba9ca0b..0000000000 --- a/packages/agent-directory/src/migrations.ts +++ /dev/null @@ -1,111 +0,0 @@ -// Package-owned migrations for @corbits/agent-directory. Bookkeeping -// uses its own ledger table so the package can be extracted without -// disentangling history from the platform drizzle journal. Every table -// this package owns — including its ledger — lives in its own -// `agent_directory` Postgres schema, never `public`; see -// docs/package-migrations.md. Copies `@corbits/config-profiles`' -// `applyConfigProfilesMigrations` shape exactly. -import postgres from "postgres"; - -export interface AgentDirectoryMigration { - name: string; - sql: string; -} - -const SCHEMA = "agent_directory"; - -export const agentDirectoryMigrations: readonly AgentDirectoryMigration[] = [ - { - name: "0001_definition_skills", - sql: ` - CREATE TABLE IF NOT EXISTS "agent_directory"."definition_skills" ( - "asset_id" text PRIMARY KEY, - "skills" jsonb NOT NULL DEFAULT '[]', - "updated_at" timestamptz NOT NULL DEFAULT now() - ); - `, - }, - // CL-7592 cuts the Workbench-owned directory store over to the - // definition assets' own pinned-skills stanzas: the table 0001 created - // is dropped, never read again. Append-only like every ledger entry - // before it — history is not rewritten, the store is deleted forward. - // - // Parity (stanza ⊇ store, so the drop loses nothing): from the - // table's introduction (baabe260) to this cutover, every `setSkills` - // writer dual-wrote the identical skill set into the asset stanza - // first — create (`createAgentDefinitionCore`), `PUT - // /:definitionId/skills`, the skill-pin route, and the - // capability-add skill path all `reindexPinnedSkills` the written - // workflow and persist the store only in `afterWrite`, which runs - // after the asset write succeeds. The store has no delete path and - // reads a missing row as []. A crash between the two writes leaves - // the stanza ahead (safe: reads now come from the stanza); no path - // writes the store without first writing the stanza, so no dropped - // row can name a skill its asset's stanza lacks. - { - name: "0002_drop_definition_skills", - sql: `DROP TABLE IF EXISTS "agent_directory"."definition_skills";`, - }, -]; - -const LEDGER_TABLE = "agent_directory_migrations"; - -function quoteIdentifier(name: string): string { - return `"${name.replace(/"/g, '""')}"`; -} - -function quoteQualified(schema: string, name: string): string { - return `${quoteIdentifier(schema)}.${quoteIdentifier(name)}`; -} - -export interface ApplyAgentDirectoryMigrationsReport { - applied: string[]; - alreadyApplied: string[]; -} - -export async function applyAgentDirectoryMigrations( - databaseUrl: string, -): Promise { - const sql = postgres(databaseUrl, { max: 1, onnotice: () => undefined }); - try { - await sql.unsafe(`CREATE SCHEMA IF NOT EXISTS ${quoteIdentifier(SCHEMA)}`); - - await sql.unsafe( - `CREATE TABLE IF NOT EXISTS ${quoteQualified(SCHEMA, LEDGER_TABLE)} (` + - `name text PRIMARY KEY, applied_at timestamptz NOT NULL DEFAULT now())`, - ); - - const applied: string[] = []; - const alreadyApplied: string[] = []; - - for (const migration of agentDirectoryMigrations) { - const existing = await sql.unsafe( - `SELECT 1 FROM ${quoteQualified(SCHEMA, LEDGER_TABLE)} WHERE name = $1`, - [migration.name], - ); - if (existing.length > 0) { - alreadyApplied.push(migration.name); - continue; - } - try { - await sql.begin(async (tx) => { - await tx.unsafe(migration.sql); - await tx.unsafe( - `INSERT INTO ${quoteQualified(SCHEMA, LEDGER_TABLE)} (name) VALUES ($1)`, - [migration.name], - ); - }); - applied.push(migration.name); - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - throw new Error(`agent-directory migration ${migration.name} failed: ${message}`, { - cause: err, - }); - } - } - - return { applied, alreadyApplied }; - } finally { - await sql.end({ timeout: 5 }); - } -} diff --git a/packages/agent-directory/test/migrations.test.ts b/packages/agent-directory/test/migrations.test.ts deleted file mode 100644 index ff69909f04..0000000000 --- a/packages/agent-directory/test/migrations.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -// DB-gated: skipped when no DATABASE_URL is reachable (a fresh checkout -// still runs the unit gates), mirroring @corbits/config-profiles' own -// migrations test. Runs against its own scratch database, never the -// developer's or the walking-skeleton suite's. -import { afterAll, beforeAll, expect, test } from "bun:test"; -import postgres from "postgres"; - -import { e2eDatabaseUrl } from "../../../scripts/e2e/database-url"; -import { applyAgentDirectoryMigrations } from "../src/migrations"; -import { dbGate } from "../../../scripts/e2e/db-gate"; - -function scratchUrlFor(e2eUrl: string): string { - const url = new URL(e2eUrl); - const database = url.pathname.replace(/^\//, ""); - url.pathname = `/${database}_agent_directory_migrations_test`; - return url.toString(); -} - -const databaseUrl = e2eDatabaseUrl(); -const describeIfDb = dbGate(databaseUrl, import.meta.path); - -const migrationNames = ["0001_definition_skills", "0002_drop_definition_skills"]; - -describeIfDb("applyAgentDirectoryMigrations", () => { - const scratchUrl = scratchUrlFor(databaseUrl ?? "postgres://localhost:5432/unused"); - const scratchDatabase = new URL(scratchUrl).pathname.replace(/^\//, ""); - - beforeAll(async () => { - const maintenanceUrl = new URL(scratchUrl); - maintenanceUrl.pathname = "/postgres"; - const maintenance = postgres(maintenanceUrl.toString(), { - max: 1, - onnotice: () => undefined, - }); - try { - await maintenance.unsafe(`DROP DATABASE IF EXISTS "${scratchDatabase}"`); - await maintenance.unsafe(`CREATE DATABASE "${scratchDatabase}"`); - } finally { - await maintenance.end(); - } - }, 20000); - - afterAll(async () => { - const maintenanceUrl = new URL(scratchUrl); - maintenanceUrl.pathname = "/postgres"; - const maintenance = postgres(maintenanceUrl.toString(), { - max: 1, - onnotice: () => undefined, - }); - try { - await maintenance.unsafe(`DROP DATABASE IF EXISTS "${scratchDatabase}"`); - } finally { - await maintenance.end(); - } - }, 20000); - - test("drops the store table forward and is idempotent on a second run", async () => { - const first = await applyAgentDirectoryMigrations(scratchUrl); - expect(first.applied).toEqual(migrationNames); - - const second = await applyAgentDirectoryMigrations(scratchUrl); - expect(second.applied).toEqual([]); - expect(second.alreadyApplied.sort()).toEqual([...migrationNames].sort()); - - const sql = postgres(scratchUrl, { max: 1, onnotice: () => undefined }); - try { - // 0002 dropped what 0001 created: the cutover deletes the - // Workbench-owned store forward, never by rewriting history. - const tables = await sql.unsafe( - `SELECT table_name FROM information_schema.tables ` + - `WHERE table_schema = 'agent_directory' AND table_name = 'definition_skills'`, - ); - expect(tables).toHaveLength(0); - - const inPublic = await sql.unsafe( - `SELECT table_name FROM information_schema.tables ` + - `WHERE table_schema = 'public' AND table_name = 'definition_skills'`, - ); - expect(inPublic).toHaveLength(0); - } finally { - await sql.end(); - } - }); -}); diff --git a/packages/agent-directory/test/routes.integration.test.ts b/packages/agent-directory/test/routes.integration.test.ts index 1cf7ab5bc3..b34299c63a 100644 --- a/packages/agent-directory/test/routes.integration.test.ts +++ b/packages/agent-directory/test/routes.integration.test.ts @@ -37,7 +37,6 @@ import { generateKeyPair } from "@intx/crypto"; import type { RequireGrant, TenantEnv } from "@intx/hub-api"; import { dbTargetFromUrl } from "../../../scripts/db-setup"; -import { applyAgentDirectoryMigrations } from "../src/migrations"; import { createAgentDefinitionRoutes } from "../src/routes"; import type { PinnedSkillIndexResolver } from "../src/routes"; import type { DefinitionAssetHistory } from "../src/definition-history"; @@ -158,8 +157,6 @@ describeIfDb("agent-directory routes against a real assetService", () => { if (databaseUrl === undefined) { throw new Error("DATABASE_URL is unset — describeIfDb should skip"); } - await applyAgentDirectoryMigrations(databaseUrl); - const handle = createDB(dbTargetFromUrl(databaseUrl)); db = handle.db; close = handle.close; diff --git a/packages/cron/src/schema.ts b/packages/cron/src/schema.ts index d434b11516..bf4b5ee763 100644 --- a/packages/cron/src/schema.ts +++ b/packages/cron/src/schema.ts @@ -71,7 +71,7 @@ export interface ApplyCronMigrationsReport { /** * Apply `cronMigrations` against `databaseUrl`, idempotently: a migration * already recorded in this package's own ledger is skipped, never re-run. - * Mirrors `@corbits/inbox`'s `applyInboxMigrations` shape. + * Package-owned ledger, transactional apply. */ export async function applyCronMigrations( databaseUrl: string, diff --git a/packages/inbox/src/index.ts b/packages/inbox/src/index.ts index f1ae7d2f97..b27ab915ee 100644 --- a/packages/inbox/src/index.ts +++ b/packages/inbox/src/index.ts @@ -3,9 +3,4 @@ export { createWorkbenchMailboxDelivery, type CreateWorkbenchMailboxDeliveryOpts, } from "./delivery"; -export { - applyInboxMigrations, - applyMailboxMigrations, - type ApplyInboxMigrationsReport, - type ApplyMailboxMigrationsReport, -} from "./migrations"; +export { applyMailboxMigrations, type ApplyMailboxMigrationsReport } from "./migrations"; diff --git a/packages/inbox/src/migrations.ts b/packages/inbox/src/migrations.ts index 7e750c7ee9..f3114ecb99 100644 --- a/packages/inbox/src/migrations.ts +++ b/packages/inbox/src/migrations.ts @@ -1,19 +1,8 @@ -// Two independent migration stories for `@corbits/inbox`: -// -// - `applyMailboxMigrations`: a URL-shaped wrapper around -// `@corbits/mailbox`'s own `runMailboxMigrations`, so scripts/db-setup.ts -// can apply it the same way it applies every other installed package's -// migrations. That package keeps its own ledger inside the `mailbox` -// schema. -// - `applyInboxMigrations`: this package's own product table history. -// CL-7208 added `inbox.snooze` for a snooze's `until` timestamp; -// CL-8185 drops it forward — snooze is gone per owner ruling. -// Bookkeeping uses its own ledger table, following the same pattern as -// the deleted `@workbench/onboarding`'s `migrations.ts`, so this package's migration -// history stays extractable on its own. - +// A URL-shaped wrapper around `@corbits/mailbox`'s own +// `runMailboxMigrations`, so scripts/db-setup.ts can apply it the same +// way it applies every other installed package's migrations. That +// package keeps its own ledger inside the `mailbox` schema. import { createMailboxDb, runMailboxMigrations } from "@corbits/mailbox"; -import postgres from "postgres"; export interface ApplyMailboxMigrationsReport { applied: string[]; @@ -39,101 +28,3 @@ export async function applyMailboxMigrations( await close(); } } - -export interface InboxMigration { - name: string; - sql: string; -} - -const SCHEMA = "inbox"; -const LEDGER_TABLE = "inbox_migrations"; - -export const inboxMigrations: readonly InboxMigration[] = [ - { - name: "0001_snooze", - sql: ` - CREATE TABLE IF NOT EXISTS "inbox"."snooze" ( - "tenant_id" text NOT NULL, - "principal_id" text NOT NULL, - "message_id" text NOT NULL, - "until" timestamptz NOT NULL, - PRIMARY KEY ("tenant_id", "principal_id", "message_id") - ); - CREATE INDEX IF NOT EXISTS "inbox_snooze_until_idx" ON "inbox"."snooze" ("until"); - `, - }, - // CL-8185: snooze is dropped per owner ruling — no web UI ever grew an - // affordance for it, and the hub's own unsnooze sweep is deleted - // alongside this migration. Forward-drop, never a backfill: any row - // still here just meant an item was hidden until its `until` passed, - // which the row's own deletion (not its message) restores to visible. - { - name: "0002_drop_snooze", - sql: `DROP TABLE IF EXISTS "inbox"."snooze";`, - }, -]; - -function quoteIdentifier(name: string): string { - return `"${name.replace(/"/g, '""')}"`; -} - -function quoteQualified(schema: string, name: string): string { - return `${quoteIdentifier(schema)}.${quoteIdentifier(name)}`; -} - -export interface ApplyInboxMigrationsReport { - applied: string[]; - alreadyApplied: string[]; -} - -/** - * Apply `inboxMigrations` against `databaseUrl`, idempotently: a migration - * already recorded in the ledger is skipped, never re-run. Failures are - * loud — the migration name and the underlying error are both surfaced. - */ -export async function applyInboxMigrations( - databaseUrl: string, -): Promise { - const sql = postgres(databaseUrl, { max: 1, onnotice: () => undefined }); - try { - await sql.unsafe(`CREATE SCHEMA IF NOT EXISTS ${quoteIdentifier(SCHEMA)}`); - await sql.unsafe( - `CREATE TABLE IF NOT EXISTS ${quoteQualified(SCHEMA, LEDGER_TABLE)} (` + - `name text PRIMARY KEY, applied_at timestamptz NOT NULL DEFAULT now())`, - ); - - const applied: string[] = []; - const alreadyApplied: string[] = []; - - for (const migration of inboxMigrations) { - const existing = await sql.unsafe( - `SELECT 1 FROM ${quoteQualified(SCHEMA, LEDGER_TABLE)} WHERE name = $1`, - [migration.name], - ); - if (existing.length > 0) { - alreadyApplied.push(migration.name); - continue; - } - try { - await sql.begin(async (tx) => { - await tx.unsafe(migration.sql); - await tx.unsafe( - `INSERT INTO ${quoteQualified(SCHEMA, LEDGER_TABLE)} (name) VALUES ($1)`, - [migration.name], - ); - }); - applied.push(migration.name); - } catch (error) { - throw new Error( - `@corbits/inbox migration ${JSON.stringify(migration.name)} failed: ` + - `${error instanceof Error ? error.message : String(error)}`, - { cause: error }, - ); - } - } - - return { applied, alreadyApplied }; - } finally { - await sql.end({ timeout: 5 }); - } -} diff --git a/packages/notify/src/migrations.ts b/packages/notify/src/migrations.ts index eaffd1589a..d3f3ad9e1e 100644 --- a/packages/notify/src/migrations.ts +++ b/packages/notify/src/migrations.ts @@ -17,8 +17,8 @@ export const notifyMigrations: readonly NotifyMigration[] = [ CREATE TABLE IF NOT EXISTS "notify"."notify_dispatch" ( "id" text PRIMARY KEY, "mailbox_row_id" text NOT NULL, - "tenant_id" text NOT NULL, - "principal_id" text NOT NULL, + "tenant_id" text NOT NULL REFERENCES "public"."tenant" ("id") ON DELETE CASCADE, + "principal_id" text NOT NULL REFERENCES "public"."principal" ("id") ON DELETE CASCADE, "sink_name" text NOT NULL, "status" text NOT NULL, "attempts" integer NOT NULL DEFAULT 0, @@ -33,20 +33,6 @@ export const notifyMigrations: readonly NotifyMigration[] = [ ON "notify"."notify_dispatch" ("status", "next_attempt_at"); `, }, - { - // CL-8210: tenant_id/principal_id become real foreign keys into - // Interchange's own tenant/principal tables, ON DELETE CASCADE, so - // deleting a tenant or principal cleans up its dispatch rows instead - // of leaving them to dangle. - name: "0002_notify_dispatch_tenant_principal_fk", - sql: ` - ALTER TABLE "notify"."notify_dispatch" - ADD CONSTRAINT "notify_dispatch_tenant_id_fkey" - FOREIGN KEY ("tenant_id") REFERENCES "public"."tenant" ("id") ON DELETE CASCADE, - ADD CONSTRAINT "notify_dispatch_principal_id_fkey" - FOREIGN KEY ("principal_id") REFERENCES "public"."principal" ("id") ON DELETE CASCADE; - `, - }, ]; const SCHEMA = "notify"; diff --git a/packages/webhook-triggers/src/migrations.ts b/packages/webhook-triggers/src/migrations.ts index 4c6fcb75cc..6f806f6117 100644 --- a/packages/webhook-triggers/src/migrations.ts +++ b/packages/webhook-triggers/src/migrations.ts @@ -30,66 +30,26 @@ export const webhookTriggersMigrations: readonly WebhookTriggersMigration[] = [ sql: ` CREATE TABLE IF NOT EXISTS "webhook_triggers"."webhook_trigger" ( "id" text PRIMARY KEY, - "tenant_id" text NOT NULL, + "tenant_id" text NOT NULL REFERENCES "public"."tenant" ("id") ON DELETE CASCADE, "name" text NOT NULL, "workflow_definition_id" text NOT NULL, "input_template" text NOT NULL, "secret" text NOT NULL, "enabled" boolean NOT NULL DEFAULT true, - "created_by" text NOT NULL, + "created_by" text NOT NULL REFERENCES "public"."principal" ("id") ON DELETE CASCADE, "created_at" timestamptz NOT NULL DEFAULT now(), "last_fired_at" timestamptz ); - `, - }, - { - name: "0002_webhook_trigger_tenant_index", - sql: ` + CREATE INDEX IF NOT EXISTS "webhook_trigger_tenant_id_idx" ON "webhook_triggers"."webhook_trigger" ("tenant_id"); - `, - }, - { - // CL-7242: startReviewingRepos's check-then-act (hasWebhookTrigger - // then createWebhookTrigger) reads (tenant_id, workflow_definition_id, - // name) with no atomic backstop, so two concurrent "start reviewing" - // calls for the same repo can both read "no trigger yet" and both - // insert -- two live triggers with the same name but different - // secrets. Reconcile first: a database already carrying the race's - // duplicates would otherwise fail CREATE UNIQUE INDEX. Keep the - // oldest row per tuple and delete the rest. This whole migration - // runs inside one transaction (applyWebhookTriggersMigrations wraps - // each entry in `sql.begin`), so the delete and the index build - // can't be split by a concurrent writer. - name: "0003_webhook_trigger_tenant_definition_name_unique", - sql: ` - DELETE FROM "webhook_triggers"."webhook_trigger" AS t - USING "webhook_triggers"."webhook_trigger" AS older - WHERE t.tenant_id = older.tenant_id - AND t.workflow_definition_id = older.workflow_definition_id - AND t.name = older.name - AND (older.created_at, older.id) < (t.created_at, t.id); CREATE UNIQUE INDEX IF NOT EXISTS "webhook_trigger_tenant_definition_name_unique" ON "webhook_triggers"."webhook_trigger" ("tenant_id", "workflow_definition_id", "name"); - `, - }, - { - // CL-7242: the paired fix to 0003, in our own schema rather than - // Interchange's. `startReviewingRepos` acquires a short-lived - // lease on `(tenant_id, repo)` before its check-then-act body - // (hasRepoGrant/mintRepoGrant, hasWebhookTrigger/createWebhookTrigger) - // runs, so two concurrent calls for the same repo can never both - // enter that body -- only one can hold the lease at a time. The - // unique index must exist before any `ON CONFLICT (tenant_id, repo)` - // is issued against this table at runtime (Postgres requires a - // matching unique constraint/index for that clause, or the insert - // errors), so both land in this one migration, index second. - name: "0004_repo_review_lease", - sql: ` + CREATE TABLE IF NOT EXISTS "webhook_triggers"."repo_review_lease" ( "id" text PRIMARY KEY, - "tenant_id" text NOT NULL, + "tenant_id" text NOT NULL REFERENCES "public"."tenant" ("id") ON DELETE CASCADE, "repo" text NOT NULL, "leased_at" timestamptz NOT NULL DEFAULT now() ); @@ -98,26 +58,6 @@ export const webhookTriggersMigrations: readonly WebhookTriggersMigration[] = [ ON "webhook_triggers"."repo_review_lease" ("tenant_id", "repo"); `, }, - { - // CL-8210: tenant_id (both tables) and webhook_trigger.created_by (a - // real Interchange principal id — see management-routes.ts's - // `createdBy: principal.id`) become hard foreign keys into - // Interchange's own tenant/principal tables, ON DELETE CASCADE, so - // deleting a tenant or principal cleans up the triggers and leases it - // owned. - name: "0005_tenant_principal_fk", - sql: ` - ALTER TABLE "webhook_triggers"."webhook_trigger" - ADD CONSTRAINT "webhook_trigger_tenant_id_fkey" - FOREIGN KEY ("tenant_id") REFERENCES "public"."tenant" ("id") ON DELETE CASCADE, - ADD CONSTRAINT "webhook_trigger_created_by_fkey" - FOREIGN KEY ("created_by") REFERENCES "public"."principal" ("id") ON DELETE CASCADE; - - ALTER TABLE "webhook_triggers"."repo_review_lease" - ADD CONSTRAINT "repo_review_lease_tenant_id_fkey" - FOREIGN KEY ("tenant_id") REFERENCES "public"."tenant" ("id") ON DELETE CASCADE; - `, - }, ]; // Named distinctly from the platform's setup ledger and from any diff --git a/packages/webhook-triggers/test/migrations.test.ts b/packages/webhook-triggers/test/migrations.test.ts index 92ae9f08f2..b116e25392 100644 --- a/packages/webhook-triggers/test/migrations.test.ts +++ b/packages/webhook-triggers/test/migrations.test.ts @@ -21,12 +21,7 @@ function scratchUrlFor(e2eUrl: string): string { const databaseUrl = e2eDatabaseUrl(); const describeIfDb = dbGate(databaseUrl, import.meta.path); -const migrationNames = [ - "0001_webhook_trigger", - "0002_webhook_trigger_tenant_index", - "0003_webhook_trigger_tenant_definition_name_unique", - "0004_repo_review_lease", -]; +const migrationNames = ["0001_webhook_trigger"]; describeIfDb("applyWebhookTriggersMigrations", () => { const scratchUrl = scratchUrlFor(databaseUrl ?? "postgres://localhost:5432/unused"); diff --git a/scripts/db-setup.ts b/scripts/db-setup.ts index b62a4009b6..8b6cd323a3 100644 --- a/scripts/db-setup.ts +++ b/scripts/db-setup.ts @@ -30,8 +30,7 @@ import { readdir } from "node:fs/promises"; import { applyWebhookTriggersMigrations } from "../packages/webhook-triggers/src/migrations"; import { applyNotifyMigrations } from "../packages/notify/src/migrations"; -import { applyInboxMigrations, applyMailboxMigrations } from "../packages/inbox/src/migrations"; -import { applyAgentDirectoryMigrations } from "../packages/agent-directory/src/migrations"; +import { applyMailboxMigrations } from "../packages/inbox/src/migrations"; import { applyCronMigrations } from "../packages/cron/src/migrations"; const repoRoot = path.resolve(import.meta.dir, ".."); @@ -52,9 +51,6 @@ const INSTALLED_PACKAGE_MIGRATIONS: readonly { { name: "@corbits/webhook-triggers", apply: applyWebhookTriggersMigrations }, { name: "@corbits/notify", apply: applyNotifyMigrations }, { name: "@corbits/mailbox", apply: applyMailboxMigrations }, - // Own `inbox` schema; forward-drops the snooze table. - { name: "@corbits/inbox", apply: applyInboxMigrations }, - { name: "@corbits/agent-directory", apply: applyAgentDirectoryMigrations }, { name: "@corbits/cron", apply: applyCronMigrations }, ];