From 7417c3c469dbcaf50722bf22be471aeddbde7ca0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:29:07 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): resolve the raw-SQL driver seam through one execute-first helper (#14083) Three sites resolved a raw-SQL entry point off a driver in two different orders: `migrations/partial-index-probe.ts` and `protocol.ts`'s `ensureOverlayIndex` tried `raw` first, `migrations/seed-tenancy-backfill.ts` tried `execute` first. They now share `migrations/driver-exec.ts`, which tries `execute` first and keeps `raw` as the fallback. `execute` goes first because `IDataDriver` declares it non-optionally and has never declared `raw`, so it is the only raw-execution surface the contract guarantees. Same reasoning and same order as `@objectstack/metadata`'s `migrations/driver-exec.ts`; the two headers cross-reference each other. No behaviour change on any shipped driver: none defines `raw`, so that limb was unreachable and `execute` already ran at all three sites. The flip matters for a host or third-party driver defining BOTH, which previously took `raw` at two sites and `execute` at the third. `raw` is kept. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .../metadata-protocol-driver-exec-order.md | 45 ++++++ .../src/migrations/driver-exec.test.ts | 118 ++++++++++++++ .../src/migrations/driver-exec.ts | 153 ++++++++++++++++++ .../src/migrations/partial-index-probe.ts | 16 +- .../src/migrations/seed-tenancy-backfill.ts | 28 ++-- .../sys-setting-identity-index.test.ts | 6 +- .../view-definition-active-index.test.ts | 21 ++- packages/metadata-protocol/src/protocol.ts | 42 +++-- 8 files changed, 390 insertions(+), 39 deletions(-) create mode 100644 .changeset/metadata-protocol-driver-exec-order.md create mode 100644 packages/metadata-protocol/src/migrations/driver-exec.test.ts create mode 100644 packages/metadata-protocol/src/migrations/driver-exec.ts diff --git a/.changeset/metadata-protocol-driver-exec-order.md b/.changeset/metadata-protocol-driver-exec-order.md new file mode 100644 index 0000000000..9a84cb2674 --- /dev/null +++ b/.changeset/metadata-protocol-driver-exec-order.md @@ -0,0 +1,45 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): resolve the raw-SQL driver seam through one `execute`-first helper (#14083) + +Three sites in this package resolved a raw-SQL entry point off a driver, in two +different orders: `migrations/partial-index-probe.ts` and `protocol.ts`'s +`ensureOverlayIndex` tried `raw` first, while `migrations/seed-tenancy-backfill.ts` +tried `execute` first. They now share one helper, `migrations/driver-exec.ts`, +which tries `execute` first and keeps `raw` as the fallback. + +`execute` goes first because `IDataDriver` (`@objectstack/spec/contracts`, +`data-driver.ts`) declares it **non-optionally** and has never declared `raw`. +It is therefore the only raw-execution surface the contract guarantees, and any +driver satisfying the interface has it. This is the same reasoning, and the same +order, that `@objectstack/metadata`'s `migrations/driver-exec.ts` adopted; the +two modules are twins and their headers cross-reference each other. + +**No behaviour change on any driver this repo ships.** No data driver here +defines `raw` — `InMemoryDriver`, `MongoDBDriver` and `SqlDriver` each declare +`execute` and none declares `raw`, and `SqliteWasmDriver` and `TursoDriver` +extend `SqlDriver` — so the `raw` limb was unreachable and `execute` was already +what ran at all three sites. The flip matters for a host or third-party driver +that defines BOTH surfaces: such a driver used to be driven through `raw` at two +sites and `execute` at the third, the same operation taking two paths in one +process. It is now driven through `execute` everywhere. + +`raw` is deliberately **kept**: nothing that worked before stops working. + +Two smaller consequences of routing all three through one helper: + +- Bindings are now passed positionally to whichever surface is selected. The + `raw` fallback in `seed-tenancy-backfill.ts` previously dropped its `params` + argument entirely, which was invisible only because that limb is unreachable + on every shipped driver. +- The capability predicate each site spelled for itself (`canRunSql` / `canRun` + / an inline check) is now defined as the resolution succeeding, so the + predicate and the selection cannot drift apart. + +⚠️ Unchanged and explicitly not addressed here: `typeof driver.execute === 'function'` +cannot distinguish "declares the surface" from "can actually run SQL", and two +shipped drivers satisfy the declaration while executing nothing. That is a +capability-declaration question tracked separately; this change aligns the ORDER +only and does not endorse the probe. diff --git a/packages/metadata-protocol/src/migrations/driver-exec.test.ts b/packages/metadata-protocol/src/migrations/driver-exec.test.ts new file mode 100644 index 0000000000..626b65f3ef --- /dev/null +++ b/packages/metadata-protocol/src/migrations/driver-exec.test.ts @@ -0,0 +1,118 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The order pin for this package's raw-SQL seam resolution. + * + * Three sites used to resolve a raw-SQL entry point off a driver in TWO + * different orders (`partial-index-probe.ts` and `protocol.ts`'s + * `ensureOverlayIndex` tried `raw` first, `seed-tenancy-backfill.ts` tried + * `execute` first). They now share `./driver-exec.ts`, which tries the surface + * `IDataDriver` DECLARES — `execute`, non-optional — and keeps `raw` as the + * fallback for a host or third-party driver that defines it. + * + * What this file pins is the SELECTION, across all four driver shapes that can + * reach the resolver: + * + * | shape | selected | + * |:-------------|:----------| + * | execute only | `execute` | + * | raw only | `raw` | + * | BOTH | `execute` | + * | neither | undefined | + * + * The `both` row is the one with teeth. On every driver this repo ships the + * `raw` limb is unreachable — no data driver defines `raw` — so a test using a + * realistic double would pass under EITHER order and pin nothing. Only a double + * offering both surfaces can tell the two orders apart, which is why the row + * exists and why it is asserted on the call arguments rather than on a return + * value. + * + * ⚠️ This pin is about the ORDER only. `typeof driver.execute === 'function'` + * cannot tell "declares the surface" from "can actually run SQL" — two shipped + * drivers satisfy the declaration and execute nothing — and nothing here should + * be read as endorsing that probe. See `./driver-exec.ts`'s header. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { driverCanRunSql, resolveDriverExec } from './driver-exec.js'; + +describe('resolveDriverExec — surface selection', () => { + it('selects execute() on a driver that offers only execute', async () => { + const execute = vi.fn(async () => 'ran'); + const exec = resolveDriverExec({ execute } as any); + + expect(exec).toBeTypeOf('function'); + await exec!('SELECT 1'); + expect(execute).toHaveBeenCalledWith('SELECT 1', []); + }); + + it('selects raw() on a driver that offers only raw', async () => { + const raw = vi.fn(async () => 'ran'); + const exec = resolveDriverExec({ raw } as any); + + expect(exec).toBeTypeOf('function'); + await exec!('SELECT 1'); + expect(raw).toHaveBeenCalledWith('SELECT 1', []); + }); + + it('selects execute() — NOT raw() — on a driver that offers BOTH', async () => { + // The row that separates the two orders. Under the pre-alignment + // raw-first order this expectation is exactly inverted, so a regression + // to `raw` first fails here and nowhere else. + const raw = vi.fn(async () => 'raw'); + const execute = vi.fn(async () => 'execute'); + + await resolveDriverExec({ raw, execute } as any)!('SELECT 1'); + + expect(execute).toHaveBeenCalledWith('SELECT 1', []); + expect(raw).not.toHaveBeenCalled(); + }); + + it('returns undefined on a driver that offers neither', () => { + expect(resolveDriverExec({} as any)).toBeUndefined(); + expect(resolveDriverExec(null)).toBeUndefined(); + expect(resolveDriverExec(undefined)).toBeUndefined(); + // A non-callable member of the right NAME must not satisfy the probe. + expect(resolveDriverExec({ execute: 'yes', raw: 42 } as any)).toBeUndefined(); + }); + + it('passes bindings positionally to whichever surface is selected', async () => { + // `IDataDriver.execute(command, parameters?)` takes bindings as the + // second POSITIONAL argument. The `raw` limb is held to the same call + // shape: before the alignment this package's `raw` fallback in + // `seed-tenancy-backfill.ts` dropped its `params` argument on the floor, + // which was invisible only because that limb is unreachable today. + const execute = vi.fn(async () => undefined); + await resolveDriverExec({ execute } as any)!('SELECT ?', ['a']); + expect(execute).toHaveBeenCalledWith('SELECT ?', ['a']); + + const raw = vi.fn(async () => undefined); + await resolveDriverExec({ raw } as any)!('SELECT ?', ['b']); + expect(raw).toHaveBeenCalledWith('SELECT ?', ['b']); + }); +}); + +describe('driverCanRunSql', () => { + it('agrees with resolveDriverExec on all four shapes', () => { + const shapes: Array<[string, unknown]> = [ + ['execute only', { execute: async () => undefined }], + ['raw only', { raw: async () => undefined }], + ['both', { execute: async () => undefined, raw: async () => undefined }], + ['neither', {}], + ['null', null], + ]; + + // The predicate is DEFINED as the resolution succeeding; this pins that + // the two cannot drift into disagreeing about which drivers count. + for (const [label, driver] of shapes) { + expect( + driverCanRunSql(driver), + `${label}: predicate must match resolution`, + ).toBe(resolveDriverExec(driver as any) !== undefined); + } + + expect(driverCanRunSql({ execute: async () => undefined })).toBe(true); + expect(driverCanRunSql({})).toBe(false); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/driver-exec.ts b/packages/metadata-protocol/src/migrations/driver-exec.ts new file mode 100644 index 0000000000..ca09238f31 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/driver-exec.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * How this package obtains a raw-SQL entry point from a driver — one + * definition, because it used to be three and they did not agree. + * + * ## The divergence this module closes + * + * Three sites in `@objectstack/metadata-protocol` resolved a raw-SQL seam off a + * driver, in TWO different orders: + * + * | site | order | + * |:--------------------------------------|:-------------------| + * | `migrations/partial-index-probe.ts` | `raw`, then `execute` | + * | `migrations/seed-tenancy-backfill.ts` | `execute`, then `raw` | + * | `protocol.ts` (`ensureOverlayIndex`) | `raw`, then `execute` | + * + * On the drivers this repo ships the two orders pick the same surface, so the + * split produced no measurable difference (see "Why this was not urgent" + * below). It still had to be closed: a host or third-party driver defining BOTH + * surfaces would have been driven through `raw` at two of those sites and + * `execute` at the third — the same operation taking two different paths in one + * process — and the dead limb read as the preferred one to anybody maintaining + * the raw-first sites. + * + * ## Why `execute` is tried FIRST + * + * `IDataDriver` (`@objectstack/spec/contracts`, `data-driver.ts`) declares + * + * ```ts + * execute(command: unknown, parameters?: unknown[], options?: DriverOptions): Promise; + * ``` + * + * — **non-optional**. `raw` has never appeared on that interface at all. So + * `execute` is not merely the surface the shipped drivers happen to have; it is + * the only raw-execution surface the contract guarantees, and any driver + * satisfying `IDataDriver` has it. Trying it first is the order that matches the + * declaration. + * + * This is the 2026-08-07 meta-criterion — one operation, several + * implementations, inconsistent behaviour, decide by the declaration-bound side + * — applied a second time. The first application is the precedent this module + * follows: `packages/metadata/src/migrations/driver-exec.ts`, which converted + * `@objectstack/metadata`'s migrations to `execute`-first for exactly this + * reason. ⚠️ That module and this one are TWINS and must stay in step; its + * header carries the longer argument, including the `raw(sql, bindings?)` call + * shape that made `execute`'s positional `parameters` the matching surface. + * + * `execute`-first also happens to be the order `seed-tenancy-backfill.ts` + * already argued for on independent grounds: `execute(sql, params)` carries + * bound parameters and `raw(sql)`, as this package was calling it, did not. + * + * ⚠️ `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 resolve an `IDataDriver`, so `data-driver.ts` governs. + * Do not reason about this call from the engine declaration. + * + * ## Why `raw` is KEPT + * + * Nothing in this repo defines `raw` on a data driver, but a host or a + * third-party driver may, and removing a surface that currently works is not + * what this alignment is for. The fallback stays; only the ORDER changed. + * Callers that must refuse should treat `undefined` as "neither surface". + * + * ## Why this is a twin rather than an import + * + * `@objectstack/metadata` is already a declared dependency of this package, and + * `resolveDriverExec` could have been imported instead of restated. It is not, + * for two reasons: + * + * - `driver-exec.ts` is INTERNAL to `metadata`'s migrations directory — it is + * not re-exported from `@objectstack/metadata/migrations`. Importing it would + * mean widening that package's published surface to serve three call sites in + * a sibling package. + * - The only subpath that could carry it is the `./migrations` barrel, and + * `ensureOverlayIndex` — one of the three callers — runs on EVERY boot. + * Pulling a migrations barrel onto the boot path to save ten lines is the + * wrong trade. + * + * ⚠️ The comment in `protocol.ts` that used to justify keeping this logic local + * cited a circular dependency ("metadata already depends on objectql"). That + * reason is STALE and was not the one acted on here: `@objectstack/metadata` + * does not depend on `@objectstack/objectql`; `objectql` depends on both. The + * two bullets above are the live reasons. + * + * ## Known limitation, deliberately not papered over here + * + * `typeof driver.execute === 'function'` separates "declares the surface" from + * "does not declare it" — it does NOT separate either from "can actually run + * SQL". Two shipped drivers satisfy the non-optional `execute` declaration and + * then execute nothing: `InMemoryDriver.execute` logs and returns `null` for + * every command, and `MongoDBDriver.execute` hands the command back. Both are + * selected by the resolver below and then answer every probe with "absent", so a + * migration reports "not applicable" instead of refusing. `IDataDriver` exposes + * no capability flag that would tell the two apart (`DriverCapabilities` has no + * such member), so distinguishing them is a contract question rather than + * something to guess at with a driver-name sniff. Filed separately and tracked + * on the capability-declaration surface; this module inherits the limitation and + * does not add to it. Nothing below should be read as an endorsement of the + * probe — only as agreement about its ORDER. + * + * ## Why this was not urgent + * + * Measured on the tree this module landed on: no data driver in this repo + * defines `raw`. `InMemoryDriver`, `MongoDBDriver` and `SqlDriver` each declare + * `execute` and none declares `raw`; `SqliteWasmDriver` and `TursoDriver` extend + * `SqlDriver` and inherit the same. The only `raw(` members anywhere are two + * test doubles and `packages/verify/src/harness.ts`, an HTTP harness whose + * signature is `(path, init)` and which is not a data driver. So on every + * shipped driver the `raw` limb is unreachable and the flip changes no observed + * behaviour today — which is precisely why it was safe to do before a + * third-party driver made it a live defect. + */ + +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; + +/** + * Whether `driver` offers either raw-SQL surface. + * + * Exactly `resolveDriverExec(driver) !== undefined`, and defined in terms of it + * so the predicate and the resolution can never disagree about which drivers + * count — the three call sites previously spelled this test three times, once + * per site, alongside three copies of the resolution. + */ +export function driverCanRunSql(driver: unknown): boolean { + return resolveDriverExec(driver as IDataDriver | null | undefined) !== undefined; +} + +/** + * Resolve the raw-SQL entry point of `driver`, or `undefined` when it offers + * neither surface. + * + * Order: declared surface (`execute`) first, `raw` as the fallback — see the + * header for why, and do not flip it back without reading that argument. + */ +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; +} diff --git a/packages/metadata-protocol/src/migrations/partial-index-probe.ts b/packages/metadata-protocol/src/migrations/partial-index-probe.ts index f4fbb2018c..2a657fdddb 100644 --- a/packages/metadata-protocol/src/migrations/partial-index-probe.ts +++ b/packages/metadata-protocol/src/migrations/partial-index-probe.ts @@ -39,7 +39,15 @@ import { isUniqueViolationError } from '@objectstack/types'; -/** Raw-SQL seam. Mirrors `ensureOverlayIndex`: `raw()` first, `execute()` second. */ +import { driverCanRunSql, resolveDriverExec } from './driver-exec.js'; + +/** + * Raw-SQL seam. The surface is resolved by `./driver-exec.ts`: `execute()` + * first — the member `IDataDriver` declares non-optionally — then `raw()` as + * the fallback for a host or third-party driver that defines it. That module's + * header carries the argument; this order is shared with `ensureOverlayIndex` + * and `seed-tenancy-backfill.ts`, which used to disagree with it. + */ export type IndexExec = (sql: string) => Promise; /** @@ -75,8 +83,7 @@ export function resolveIndexExecForTable(engine: unknown, table: string): IndexE return undefined; } }; - const canRunSql = (d: any): boolean => - !!d && (typeof d.raw === 'function' || typeof d.execute === 'function'); + const canRunSql = (d: any): boolean => driverCanRunSql(d); let driver: any = attempt(() => engineAny?.getDriverForObject?.(table)); if (!canRunSql(driver)) driver = attempt(() => engineAny?.driver); @@ -91,8 +98,7 @@ export function resolveIndexExecForTable(engine: unknown, table: string): IndexE } } if (!canRunSql(driver)) return undefined; - if (typeof driver.raw === 'function') return (sql: string) => driver.raw(sql); - return (sql: string) => driver.execute(sql); + return resolveDriverExec(driver); } /** diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index f0b4e3ae3f..f4d619b79a 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -140,6 +140,7 @@ import { resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; import { DATA_MIGRATION_FLAG_OBJECT, type DataMigrationFlag } from '@objectstack/spec/system'; import type { IndexMigrationLogger } from './partial-index-probe.js'; +import { driverCanRunSql, resolveDriverExec } from './driver-exec.js'; /** The driver-private counter table (`SqlDriver.SEQUENCES_TABLE`). */ export const SEQUENCES_TABLE = '_objectstack_sequences'; @@ -298,10 +299,13 @@ export interface SeedTenancySeam { /** * Resolve a row-returning raw-SQL seam, together with the dialect it speaks. * - * `execute` is probed BEFORE `raw` — the opposite order to - * `resolveIndexExecForTable` — because `execute(sql, params)` carries bound - * parameters and `raw(sql)` does not. Every probe is individually guarded so - * this returns `undefined` rather than throwing into a boot hook. + * `execute` is probed BEFORE `raw`, via `./driver-exec.ts`. This module argued + * for that order first — `execute(sql, params)` carries bound parameters and + * `raw(sql)`, as this package was calling it, did not — and it is now the order + * `resolveIndexExecForTable` and `ensureOverlayIndex` use too, on the stronger + * ground that `IDataDriver` declares `execute` non-optionally and has never + * declared `raw`. See that module's header. Every probe is individually guarded + * so this returns `undefined` rather than throwing into a boot hook. * * Unlike its sibling this does not ask `getDriverForObject`: the target table is * `_objectstack_sequences`, which is driver-private and not a registered object, @@ -322,8 +326,7 @@ export function resolveSeedTenancySeam(engine: unknown): SeedTenancySeam | undef return undefined; } }; - const canRun = (d: any): boolean => - !!d && (typeof d.execute === 'function' || typeof d.raw === 'function'); + const canRun = (d: any): boolean => driverCanRunSql(d); let driver: any = attempt(() => engineAny?.driver); if (!canRun(driver)) driver = attempt(() => engineAny?.getDefaultDriver?.()); @@ -344,14 +347,11 @@ export function resolveSeedTenancySeam(engine: unknown): SeedTenancySeam | undef // timestamps), never as raw SQL against a table this module would then have // to spell for three dialects. const ledger = resolveSeedTenancyLedger(engine); - if (typeof driver.execute === 'function') { - return { - exec: (sql: string, params?: unknown[]) => driver.execute(sql, params ?? []), - client, - ledger, - }; - } - return { exec: (sql: string) => driver.raw(sql), client, ledger }; + const exec = resolveDriverExec(driver); + // `canRun` above is defined AS this resolution succeeding, so `exec` is + // present here; the guard is for the type, not for a reachable state. + if (!exec) return undefined; + return { exec, client, ledger }; } /** diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts index 612c7edac3..2d7521fa5d 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts @@ -562,7 +562,11 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { await resolved?.('SELECT 1'); expect(engine.getDriverForObject).toHaveBeenCalledWith(SYS_SETTING_TABLE); - expect(owner.raw).toHaveBeenCalledWith('SELECT 1'); + // Both doubles offer only `raw`, so the fallback limb is the one + // that runs and the OWNER-vs-default question this case exists for + // is unaffected by the execute-first flip. Bindings are now passed + // positionally to `raw` too (`./driver-exec.ts`), hence the `[]`. + expect(owner.raw).toHaveBeenCalledWith('SELECT 1', []); expect(engine.driver.raw).not.toHaveBeenCalled(); }); diff --git a/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts b/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts index 1db243327d..a4c35ba2f9 100644 --- a/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts +++ b/packages/metadata-protocol/src/migrations/view-definition-active-index.test.ts @@ -563,16 +563,29 @@ describe('sys_view_definition active-row uniqueness (#5839) on a NULL-safe key ( } }); - it('resolveIndexExec prefers raw(), falls back to execute(), else undefined', async () => { + /** + * The order flipped: this used to assert `raw()` first. `IDataDriver` + * declares `execute` non-optionally and has never declared `raw`, so the + * declared surface is the one tried first; `raw` stays as the fallback for a + * host or third-party driver that defines it. See `./driver-exec.ts`, and + * `driver-exec.test.ts` for the four-shape pin. The assertion below is the + * half that actually changed — a driver offering BOTH surfaces. + */ + it('resolveIndexExec prefers execute(), falls back to raw(), else undefined', async () => { const raw = vi.fn(async () => undefined); const execute = vi.fn(async () => undefined); await resolveIndexExec({ driver: { raw, execute } })!('SELECT 1'); - expect(raw).toHaveBeenCalledWith('SELECT 1'); - expect(execute).not.toHaveBeenCalled(); + expect(execute).toHaveBeenCalledWith('SELECT 1', []); + expect(raw).not.toHaveBeenCalled(); + execute.mockClear(); await resolveIndexExec({ driver: { execute } })!('SELECT 2'); - expect(execute).toHaveBeenCalledWith('SELECT 2'); + expect(execute).toHaveBeenCalledWith('SELECT 2', []); + + // The fallback limb still resolves for a driver that has only `raw`. + await resolveIndexExec({ driver: { raw } })!('SELECT 3'); + expect(raw).toHaveBeenCalledWith('SELECT 3', []); // getDriver() and the drivers Map, the two other shapes the paradigm walks. expect(resolveIndexExec({ getDriver: () => ({ raw }) })).toBeTypeOf('function'); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 5ce4ef8afe..5e75b9816b 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -27,6 +27,7 @@ import type { RuntimeAuthoringIssue } from './runtime-authoring-gate.js'; // [#6418] `sys_metadata`'s overlay-uniqueness indexes: probe-first DDL plus the // ADR-0120 D4 reporting that replaced this file's empty `catch` blocks. import { ensureMetadataOverlayIndexes } from './migrations/overlay-index.js'; +import { driverCanRunSql, resolveDriverExec } from './migrations/driver-exec.js'; import { SysMetadataRepository, type SysMetadataEngine } from './sys-metadata-repository.js'; import { metaOverlayCacheTtlMs, @@ -5202,9 +5203,23 @@ export class ObjectStackProtocolImplementation implements * See that module's header for why the order is now probe-first and why the * dialect fallback must stay NON-unique. * - * Kept in this package (rather than imported from - * `@objectstack/metadata/migrations`) to avoid a circular dependency: - * metadata already depends on objectql. + * The raw-SQL surface is resolved by `./migrations/driver-exec.ts`: + * `execute()` first — the member `IDataDriver` declares non-optionally — + * then `raw()` as the fallback for a host or third-party driver that + * defines it. This method used to try `raw` first, disagreeing with + * `seed-tenancy-backfill.ts`; that module's header carries the argument for + * the order and the measurement showing the flip changes nothing on any + * driver this repo ships. + * + * Kept in this package rather than imported from + * `@objectstack/metadata/migrations`. ⚠️ The reason recorded here used to be + * a circular dependency ("metadata already depends on objectql") — that is + * STALE: `@objectstack/metadata` does not depend on `@objectstack/objectql`, + * and `@objectstack/metadata` is already a declared dependency of this + * package. The live reasons are that `driver-exec.ts` is internal to + * `metadata`'s migrations directory rather than part of its published + * surface, and that this method runs on EVERY boot, so importing the + * `./migrations` barrel would put it on the boot path. */ private overlayIndexEnsured = false; private async ensureOverlayIndex(): Promise { @@ -5215,25 +5230,22 @@ export class ObjectStackProtocolImplementation implements let driver: any = engineAny?.driver ?? engineAny?.getDriver?.(); if (!driver && engineAny?.drivers instanceof Map) { for (const candidate of engineAny.drivers.values()) { - if ( - candidate && - (typeof (candidate as any).raw === 'function' || - typeof (candidate as any).execute === 'function') - ) { + if (driverCanRunSql(candidate)) { driver = candidate; break; } } } if (!driver) return; + const resolved = resolveDriverExec(driver); + // The refusal stays INSIDE the seam rather than becoming an early + // return: a driver taken from `engineAny?.driver` is not + // capability-checked above, and when it offers neither surface it is + // the migration that classifies and reports the failure — an early + // return here would put that back into the silent `catch` below. const exec = async (sql: string): Promise => { - if (typeof (driver as any).raw === 'function') { - await (driver as any).raw(sql); - } else if (typeof (driver as any).execute === 'function') { - await (driver as any).execute(sql); - } else { - throw new Error('driver has neither raw nor execute'); - } + if (!resolved) throw new Error('driver has neither raw nor execute'); + await resolved(sql); }; // `console` satisfies the logger surface structurally; this class // carries no injected logger, and its own diagnostics go to From 6b45e0c4040ca2df690ecbd98861bb11e67de786 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 06:14:11 +0000 Subject: [PATCH 2/2] docs(permissions): re-anchor the isSystem census row this branch shifted by one line The import added at `packages/metadata-protocol/src/protocol.ts:30` shifts every line below it by +1, including the `if (context?.isSystem) return data;` elevation read in `stripReadonlyForInsert` (1736 -> 1737). The census page still anchored 1736, so `check-system-context-census` reported both halves of one rot: `[site-without-a-row]` at 1737 and `[anchor-is-not-a-read-site]` at 1736. Repaired with the gate's own `--fix`, which re-pointed exactly one anchor and added or deleted no row. Pure line rot: the population is unchanged at 109 elevation read sites. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 7b3dc4d498..0077e8b65e 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -112,7 +112,7 @@ that silently does not happen. | 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10787` | | 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:10949` | | 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9680` | -| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1736` | +| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1737` | | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9717`, `readonly-strict-errors.ts:66` | | 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5705` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3574`, `:3584`, `:3611` |