fix(metadata): bind the published migrations to the driver surface IDataDriver declares, and pin it with a real driver

.changeset/migration-driver-exec-surface.md

@@ -0,0 +1,61 @@
+---
+"@objectstack/metadata": patch
+---
+
+fix(metadata): every migration in `@objectstack/metadata/migrations` refused every driver this repo ships (#14023)
+
+All four helpers exported from `@objectstack/metadata/migrations` guarded on —
+and drove through — `driver.raw(sql, bindings?)`. **No data driver in this repo
+defines `raw`.** `SqlDriver` keeps its knex handle `protected` and declares no
+`raw` member, and `SqliteWasmDriver` inherits that; the only `raw(` member
+anywhere outside a test double is an HTTP harness in `packages/verify` whose
+signature is `(path, init)`. So an operator who passed their platform driver was
+refused by all four:
+
+```
+migrateSysNotificationToEvent({ driver, data })  ->  { status: 'error', migrated: 0 }
+```
+
+The failure was quiet in the shape that matters. `migrateSysNotificationToEvent`
+*returns* `{ status: 'error' }` rather than throwing, and the message blamed the
+caller's driver for lacking a method instead of saying the migration had not
+run — so someone following the ADR-0030 cut-over runbook, which names this call
+as the supported way to preserve users' existing bell notifications, would read
+it as a problem with their own driver.
+
+It was not only an operator-facing path. `DatabaseLoader` calls
+`migrateProjectIdToEnvironmentId(driver)` on bootstrap with a real driver, at
+two call sites, each wrapped in a catch — so the v5.0 `project_id` ->
+`environment_id` forward migration threw and was swallowed on every boot.
+
+The four helpers now resolve their raw-SQL entry point through one shared
+resolver (`src/migrations/driver-exec.ts`) that tries `execute` first and falls
+back to `raw`. `execute` goes first because it is the surface the contract
+declares: `IDataDriver` (`@objectstack/spec/contracts`) declares
+`execute(command, parameters?, options?)` **non-optionally**, with bound
+parameters as the second positional argument — exactly the shape `raw(sql,
+bindings?)` was being called in — and has never declared `raw`. `raw` is kept as
+a fallback so a host or third-party driver that does define it keeps working;
+nothing that worked before stops working, and the refusal now fires only for a
+driver offering neither surface.
+
+Two sibling directories already resolved both surfaces instead of assuming one,
+in opposite orders (`metadata-protocol`'s `partial-index-probe` tries `raw`
+first, its `seed-tenancy-backfill` tries `execute` first, and `protocol.ts`'s
+`ensureOverlayIndex` is a third). One operation with three implementations and
+two behaviours resolves to the declaration-bound side, which is why this
+directory adopts `execute`-first uniformly rather than copying either precedent.
+
+The refusal message now names both surfaces. It keeps the properties pinned
+after the doubled-sentence defect: the remedy is stated exactly once, the
+sentences stay separated, and a conforming driver is still named.
+
+Tests: every pre-existing case in this directory built its own double carrying a
+`raw` method — including the case asserting the guard fires — so the suite
+pinned the guard's wording while never exercising a driver the platform ships.
+Swapping `raw` for `execute` in the helpers and in the doubles would have moved
+that hole rather than closed it. A new `real-driver-exec-surface.test.ts` drives
+all four migrations through a real `SqliteWasmDriver` against real in-process
+SQLite, asserting the physical schema rather than the returned status, and pins
+the surface reality the file exists for: the real driver has no `raw` and does
+have `execute`.

packages/metadata/src/loaders/database-loader.test.ts

@@ -619,20 +619,27 @@ describe('DatabaseLoader schema-sync failure reporting (#4728)', () => {
     });
 
     it('still runs the post-sync migrations (the table exists, so they apply)', async () => {
+      // #14023 — this used to bolt a `raw` method onto the mock through an
+      // `as unknown as { raw: unknown }` cast, because that was the only
+      // surface the migration accepted. The cast was the tell: it reached PAST
+      // the declared contract. `IDataDriver` declares `execute` non-optionally
+      // and has never declared `raw`, which is why `createMockDriver` already
+      // carries `execute` and needed no cast to carry it. The migration now
+      // drives the declared surface, so this case observes the mock's own
+      // `execute` and the cast is gone.
       const driver = createMockDriver();
       driver.syncSchema = vi.fn().mockRejectedValue(alreadyExists());
-      const raw = vi.fn().mockResolvedValue(undefined);
-      (driver as unknown as { raw: unknown }).raw = raw;
+      const execute = driver.execute as ReturnType<typeof vi.fn>;
       const loader = new DatabaseLoader({ driver });
 
       await loader.list('object');
 
       // The `project_id` → `environment_id` forward migration still runs; it
       // probes the column list before touching anything.
-      expect(raw).toHaveBeenCalled();
-      expect(raw.mock.calls.some(([sql]) => /table_info|information_schema/i.test(String(sql)))).toBe(
-        true,
-      );
+      expect(execute).toHaveBeenCalled();
+      expect(
+        execute.mock.calls.some(([sql]) => /table_info|information_schema/i.test(String(sql))),
+      ).toBe(true);
     });
 
     /**
@@ -646,15 +653,20 @@ describe('DatabaseLoader schema-sync failure reporting (#4728)', () => {
     it('issues NO overlay-index DDL — this package is not a producer of that name', async () => {
       const driver = createMockDriver();
       driver.syncSchema = vi.fn().mockRejectedValue(alreadyExists());
-      const raw = vi.fn().mockResolvedValue(undefined);
-      (driver as unknown as { raw: unknown }).raw = raw;
+      const execute = driver.execute as ReturnType<typeof vi.fn>;
       const loader = new DatabaseLoader({ driver });
 
       await loader.list('object');
 
-      const overlayDdl = raw.mock.calls
+      // Non-vacuity FIRST (#14023). This assertion is "no statement matched a
+      // pattern", which a run that issued NO statements at all satisfies just
+      // as well — and that is exactly the state this file was in while the
+      // migration refused every driver. Observe that SQL really flowed before
+      // reading anything into the absence of that one statement.
+      expect(execute, 'nothing ran — the emptiness below would prove nothing').toHaveBeenCalled();
+      const overlayDdl = execute.mock.calls
         .map(([sql]) => String(sql))
-        .filter((sql) => /idx_sys_metadata_overlay_active/i.test(sql));
+        .filter((sql: string) => /idx_sys_metadata_overlay_active/i.test(sql));
       expect(overlayDdl).toEqual([]);
     });
   });

packages/metadata/src/migrations/driver-exec.ts

@@ -0,0 +1,110 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * How the migrations in this directory obtain a raw-SQL entry point.
+ *
+ * Every helper here used to guard on — and drive through — `driver.raw(sql,
+ * bindings?)`. **No data driver in this repo defines `raw`.** Measured on
+ * `origin/main`, the only `raw(` member anywhere outside a test double is
+ * `packages/verify/src/harness.ts`, an HTTP harness whose signature is
+ * `(path, init)`. `SqlDriver` keeps its knex handle `protected`, so
+ * `driver.raw` is `undefined` there too, and `SqliteWasmDriver` inherits that.
+ * The result was a published, operator-documented migration path that refused
+ * every driver the platform ships — quietly, because
+ * `migrateSysNotificationToEvent` *returns* `{ status: 'error' }` rather than
+ * throwing, and the message blamed the operator's driver instead of saying the
+ * migration did not run.
+ *
+ * ## Why `execute` is tried FIRST
+ *
+ * `IDataDriver` (`@objectstack/spec/contracts`, `data-driver.ts`) declares
+ *
+ * ```ts
+ * execute(command: unknown, parameters?: unknown[], options?: DriverOptions): Promise<unknown>;
+ * ```
+ *
+ * — **non-optional**, with bound parameters as the second POSITIONAL argument,
+ * which is the exact shape `raw(sql, bindings?)` was being called in. `raw` has
+ * never appeared on that interface. So `execute` is not merely the surface the
+ * shipped drivers happen to have; it is the only raw-execution surface the
+ * contract guarantees at all, and a driver that satisfies `IDataDriver` always
+ * has it. Trying it first is therefore the order that matches the declaration.
+ *
+ * ⚠️ `IDataEngine.execute?(command, options?)` (`data-engine.ts`) is a DIFFERENT
+ * member on a different interface — its second parameter is an options bag, not
+ * bindings. These helpers take an `IDataDriver`, so `data-driver.ts` governs.
+ * Do not reason about this call from the engine declaration.
+ *
+ * ## Prior art, and why the order had to be chosen rather than copied
+ *
+ * `packages/metadata-protocol/src/migrations/` already resolves both surfaces
+ * instead of assuming one — twice, and **in opposite orders**:
+ * `partial-index-probe.ts` tries `raw` first, `seed-tenancy-backfill.ts` tries
+ * `execute` first. `metadata-protocol/src/protocol.ts` (`ensureOverlayIndex`)
+ * is a third, raw-first. One operation with three implementations and two
+ * behaviours resolves to the declaration-bound side, so this directory adopts
+ * `execute`-first uniformly across all four of its members.
+ *
+ * `raw` is kept as a fallback rather than dropped: nothing in this repo defines
+ * it, but a host or a third-party driver may, and removing a surface that
+ * currently works is not what this repair is for. The refusal below therefore
+ * fires only for a driver that has NEITHER.
+ *
+ * ## Known limitation, deliberately not papered over here
+ *
+ * Two shipped drivers satisfy `typeof driver.execute === 'function'` without
+ * being able to run SQL: `MemoryDriver.execute` logs a warning and returns
+ * `null` for every command, and `MongoDbDriver.execute` returns a string
+ * command back verbatim. Both are selected by the probe below and then answer
+ * every column probe with "absent", so a migration reports `not_applicable` /
+ * `table_missing` instead of refusing. `IDataDriver` exposes no capability flag
+ * that would separate "implements the escape hatch" from "can run SQL"
+ * (`DriverCapabilities` has no such member), so distinguishing them is a
+ * contract question, not something to guess at with a driver-name sniff.
+ * Filed separately.
+ */
+
+import type { IDataDriver } from '@objectstack/spec/contracts';
+
+/**
+ * A raw-SQL entry point resolved off a driver. `bindings` are passed
+ * positionally, matching `IDataDriver.execute`'s declared `parameters`.
+ */
+export type DriverExec = (sql: string, bindings?: readonly unknown[]) => Promise<any>;
+
+/**
+ * Resolve the raw-SQL entry point of `driver`, or `undefined` when it offers
+ * neither surface.
+ *
+ * Callers that must refuse should pair this with {@link driverExecRefusal} so
+ * every member of this directory states the same remedy.
+ */
+export function resolveDriverExec(driver: IDataDriver | null | undefined): DriverExec | undefined {
+    const candidate = driver as any;
+    if (!candidate) return undefined;
+    // Declared surface first — see the header.
+    if (typeof candidate.execute === 'function') {
+        return (sql, bindings) => candidate.execute(sql, bindings ? [...bindings] : []);
+    }
+    if (typeof candidate.raw === 'function') {
+        return (sql, bindings) => candidate.raw(sql, bindings ? [...bindings] : []);
+    }
+    return undefined;
+}
+
+/**
+ * The single refusal sentence used by every migration in this directory, for a
+ * driver that offers neither surface.
+ *
+ * Assembled in one place because the wording carries pinned properties: the
+ * remedy is stated exactly ONCE (a guard here once concatenated its instruction
+ * twice), the two sentences stay separated rather than running together, and a
+ * conforming driver is named so the operator has something to act on.
+ */
+export function driverExecRefusal(helper: string): string {
+    return (
+        `${helper}: driver must expose an .execute(sql, bindings?) or .raw(sql, bindings?) method. ` +
+        'SqlDriver (better-sqlite3/knex) exposes .execute(), as does its SqliteWasmDriver subclass; ' +
+        'cloud-side TursoDriver also conforms.'
+    );
+}

packages/metadata/src/migrations/drop-projection-tables.ts

@@ -19,6 +19,8 @@
 
 import type { IDataDriver } from '@objectstack/spec/contracts';
 
+import { driverExecRefusal, resolveDriverExec } from './driver-exec.js';
+
 const DEPRECATED_TABLES = [
     'sys_object',
     'sys_view',
@@ -36,19 +38,21 @@ export interface DropProjectionResult {
 /**
  * Drop the deprecated per-type metadata projection tables.
  *
- * @param driver  An `IDataDriver` with `driver.raw(sql, bindings?)` access.
+ * @param driver  An `IDataDriver`. Raw SQL is issued through the surface
+ *                `IDataDriver` declares — `execute(sql, bindings?)` — falling
+ *                back to `raw(sql, bindings?)`; see `./driver-exec.ts`.
  * @returns       Per-table results.
  */
 export async function dropProjectionTables(driver: IDataDriver): Promise<DropProjectionResult[]> {
-    const driverAny = driver as any;
-    if (typeof driverAny.raw !== 'function') {
-        throw new Error('dropProjectionTables: driver must expose a raw(sql) method');
+    const exec = resolveDriverExec(driver);
+    if (!exec) {
+        throw new Error(driverExecRefusal('dropProjectionTables'));
     }
 
     const results: DropProjectionResult[] = [];
     for (const table of DEPRECATED_TABLES) {
         try {
-            await driverAny.raw(`DROP TABLE IF EXISTS ${table}`);
+            await exec(`DROP TABLE IF EXISTS ${table}`);
             results.push({ table, status: 'dropped' });
         } catch (error) {
             results.push({

packages/metadata/src/migrations/migrate-env-id-to-project-id.ts

@@ -21,6 +21,8 @@
 
 import type { IDataDriver } from '@objectstack/spec/contracts';
 
+import { type DriverExec, driverExecRefusal, resolveDriverExec } from './driver-exec.js';
+
 const AFFECTED_TABLES = [
     'sys_metadata',
     'sys_metadata_history',
@@ -35,27 +37,26 @@ export interface MigrationResult {
 /**
  * Rename `env_id` → `project_id` on all metadata tables.
  *
- * @param driver  An IDataDriver with access to the target database.
- *                Must expose a raw query method: `driver.raw(sql, bindings?)`.
+ * @param driver  An IDataDriver with access to the target database. Raw SQL is
+ *                issued through the surface `IDataDriver` declares —
+ *                `execute(sql, bindings?)` — falling back to
+ *                `raw(sql, bindings?)`; see `./driver-exec.ts`.
  * @returns       Per-table migration results.
  */
 export async function migrateEnvIdToProjectId(driver: IDataDriver): Promise<MigrationResult[]> {
-    const driverAny = driver as any;
+    const exec = resolveDriverExec(driver);
 
-    if (typeof driverAny.raw !== 'function') {
-        throw new Error(
-            'migrateEnvIdToProjectId: driver must expose a .raw(sql, bindings?) method. ' +
-            'SqlDriver (better-sqlite3/knex) supports this; cloud-side TursoDriver also conforms.'
-        );
+    if (!exec) {
+        throw new Error(driverExecRefusal('migrateEnvIdToProjectId'));
     }
 
     const results: MigrationResult[] = [];
 
     for (const table of AFFECTED_TABLES) {
         try {
             // Detect dialect: SQLite uses PRAGMA, others use information_schema.
-            const hasColumn = await _columnExists(driverAny, table, 'env_id');
-            const alreadyMigrated = await _columnExists(driverAny, table, 'project_id');
+            const hasColumn = await _columnExists(exec, table, 'env_id');
+            const alreadyMigrated = await _columnExists(exec, table, 'project_id');
 
             if (alreadyMigrated && !hasColumn) {
                 results.push({ table, status: 'already_done' });
@@ -69,7 +70,7 @@ export async function migrateEnvIdToProjectId(driver: IDataDriver): Promise<Migr
             }
 
             // Perform the rename.  SQLite ≥ 3.25.0 supports ALTER TABLE RENAME COLUMN.
-            await driverAny.raw(`ALTER TABLE "${table}" RENAME COLUMN env_id TO project_id`);
+            await exec(`ALTER TABLE "${table}" RENAME COLUMN env_id TO project_id`);
 
             results.push({ table, status: 'renamed' });
         } catch (err: any) {
@@ -84,18 +85,18 @@ export async function migrateEnvIdToProjectId(driver: IDataDriver): Promise<Migr
 // Internal helpers
 // ---------------------------------------------------------------------------
 
-async function _columnExists(driver: any, table: string, column: string): Promise<boolean> {
+async function _columnExists(exec: DriverExec, table: string, column: string): Promise<boolean> {
     try {
         // SQLite: PRAGMA table_info returns rows with `name` column.
-        const rows: any[] = await driver.raw(`PRAGMA table_info("${table}")`);
+        const rows: any[] = await exec(`PRAGMA table_info("${table}")`);
         if (Array.isArray(rows) && rows.length > 0) {
             // knex wraps PRAGMA result; handle both `rows` and `rows[0]` shapes.
             const list: any[] = Array.isArray(rows[0]) ? rows[0] : rows;
             return list.some((r: any) => r?.name === column);
         }
 
         // Fallback for non-SQLite: query information_schema.
-        const result: any[] = await driver.raw(
+        const result: any[] = await exec(
             `SELECT column_name FROM information_schema.columns WHERE table_name = ? AND column_name = ?`,
             [table, column]
         );