From 0fc3ae257558d01c1493ab3e4afb91a4e08167b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 08:09:09 +0000 Subject: [PATCH 1/3] feat(metadata-core): export dialectOfDriver, the one driver-dialect resolver Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude --- .../metadata-core/src/legacy-organization-row.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/metadata-core/src/legacy-organization-row.ts b/packages/metadata-core/src/legacy-organization-row.ts index 3034a6f17b7..17c876ee186 100644 --- a/packages/metadata-core/src/legacy-organization-row.ts +++ b/packages/metadata-core/src/legacy-organization-row.ts @@ -102,8 +102,18 @@ function dialectOfName(name: string): Dialect | null { * The driver's dialect, `'none'` when it is not a SQL driver at all, or * `'unresolved'` when it is one whose dialect this read cannot name. * `dialectName` first: `SqliteWasmDriver`'s `config.client` is a class. + * + * Exported as the ONE resolver for every raw-SQL reader that must pick a + * dialect from a driver it was handed: this read, and the + * organization-ownership ceremony (`os migrate organization-ownership`), which + * refuses on anything but the three dialects instead of guessing one. A + * `dialectName` string is the whole answer, recognised or not; a `config.client` + * is read only when there is no `dialectName`, and only as a string — a client + * CLASS names no spelling, so it is `'unresolved'`, never a default. */ -function dialectOfDriver(driver: RawDriver | null | undefined): Dialect | 'none' | 'unresolved' { +export function dialectOfDriver( + driver: { dialectName?: unknown; config?: { client?: unknown } } | null | undefined, +): Dialect | 'none' | 'unresolved' { if (!driver) return 'none'; if (typeof driver.dialectName === 'string') return dialectOfName(driver.dialectName) ?? 'unresolved'; const client = driver.config?.client; From 91084b5d5e6ef572321e11175cc15e0fae180df8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 08:21:51 +0000 Subject: [PATCH 2/3] fix(cli): the organization-ownership ceremony resolves its dialect once, through metadata-core's dialectOfDriver, and refuses an unresolved one before any statement Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude --- .changeset/23558-ceremony-dialect-resolver.md | 27 +++ .../organization-ownership.dialect.test.ts | 224 ++++++++++++++++++ .../migrate/organization-ownership.ts | 22 +- ...p-apply.served-catalog.integration.test.ts | 1 + .../organization-ownership-apply.test.ts | 1 + .../src/utils/organization-ownership-apply.ts | 6 +- ...ership-host-supplement.integration.test.ts | 2 +- ...ization-ownership-plan.integration.test.ts | 11 +- .../src/utils/organization-ownership-plan.ts | 71 ++++-- .../cli/src/utils/unmanaged-tables.test.ts | 37 +++ packages/cli/src/utils/unmanaged-tables.ts | 12 +- .../src/legacy-organization-row.test.ts | 18 +- 12 files changed, 397 insertions(+), 35 deletions(-) create mode 100644 .changeset/23558-ceremony-dialect-resolver.md create mode 100644 packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts diff --git a/.changeset/23558-ceremony-dialect-resolver.md b/.changeset/23558-ceremony-dialect-resolver.md new file mode 100644 index 00000000000..189120d1853 --- /dev/null +++ b/.changeset/23558-ceremony-dialect-resolver.md @@ -0,0 +1,27 @@ +--- +'@objectstack/cli': patch +'@objectstack/metadata-core': minor +--- + +fix(cli): `os migrate organization-ownership` reads its SQL dialect through the driver resolver it shares with metadata-core, `dialectName` first, and refuses an unresolved dialect, naming the driver, before any statement runs + +Clause-②: yes (widening) + +**Who this is for.** An operator who runs the organization-ownership ceremony (plan or `--apply`) on a database whose driver's knex `config.client` is not a client string the ceremony recognises: a client CLASS (knex's bring-your-own-dialect hatch, and `SqliteWasmDriver`'s own config), or an unrecognised spelling. + +**FROM.** Both arms took the dialect from `String(config.client)` alone. + +- A client class was stringified, so on `SqliteWasmDriver` (a `wasm-sqlite://` or `*.wasm.db` database, or the development SQLite fallback) the ceremony refused a SQLite database it can migrate, with `driver-unsupported` and the class's source text as the driver's name. +- The apply step and its template lookup carried a `'sqlite'` default for an unresolved dialect, unreachable today only because the plan, which runs first, refused before them. + +**TO.** The dialect is resolved once, from the stack's driver, before either arm runs a statement. It uses the same resolver metadata-core's legacy-row reader uses: the driver's `dialectName` first, then a recognised `config.client` string. + +- `SqliteWasmDriver` (`dialectName: 'sqlite'`) runs the ceremony as SQLite. +- A client class with no recognised `dialectName`, an unrecognised client string, `dialectName: 'unknown'` (what `SqlDriver` answers for either), and a driver with no dialect at all are refused: `driver-unsupported`, exit 1, no statement run, no plan file written. The refusal names the driver (its name, its `dialectName`, its client string or client class name). +- No `'sqlite'` default remains: the plan and the apply take the resolved dialect and never re-derive it. +- The plan file's `client` field records the client only when it is a string. +- `os migrate plan`'s unmanaged-table sweep reads its catalog dialect through the same resolver. On `SqliteWasmDriver` it now reads the catalog instead of reporting the sweep unreadable. + +Every in-repo driver, measured, resolves as before or better: `SqlDriver` on better-sqlite3, pg and mysql2, and `TursoDriver` (remote and file), are unchanged. `SqliteWasmDriver` moves from refused to SQLite. + +**metadata-core.** `dialectOfDriver(driver)` is exported. It is the one resolver behind `readLegacyOrganizationRows`: `'sqlite' | 'postgres' | 'mysql'`, `'none'` for a driver that is not a SQL driver, or `'unresolved'`. A `dialectName` string is the whole answer, recognised or not. A `config.client` is read only when there is no `dialectName`, and only as a string. diff --git a/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts new file mode 100644 index 00000000000..86bf689a197 --- /dev/null +++ b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate organization-ownership` reads its SQL dialect off the stack's + * driver ONCE, before the first statement, through the resolver it shares + * with `@objectstack/metadata-core` (`ceremonyDialectOf` → `dialectOfDriver`): + * the driver's `dialectName` first, then a recognised `config.client` string. + * On anything else both arms refuse, naming the driver, with exit 1 and no + * statement run; neither arm reads an unresolved dialect as SQLite. + * + * The boot is replaced by a stack whose driver records every statement and + * answers each with no rows; the apply step is replaced by a recorder (its own + * suite pins what it does with the executor it is handed). The driver shapes + * are the ones each in-repo driver presents, measured on the drivers + * themselves: `SqliteWasmDriver`'s knex `client` is a dialect CLASS and its + * `dialectName` is `'sqlite'`; `SqlDriver` answers `dialectName: 'unknown'` + * for a client it does not recognise, a client class included. + */ + +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot })); +// The siblings' occupancy stub: no case here passes --database-url, so the real +// probe would read whatever database the package's own directory resolves to. +vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ + OCCUPANCY_STOP_HINT: 'occupancy hint', + probeMigrationTarget: vi.fn(async () => ({ status: 'free' })), +})); + +const apply = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/organization-ownership-apply.js', async (importOriginal) => ({ + ...(await importOriginal()), + applyOrganizationOwnership: apply, +})); + +type Command = (typeof import('./organization-ownership.js'))['default']; +let MigrateOrganizationOwnership: Command; + +// The command's module graph is large, and oclif reads the package's config on +// the first run: both are paid outside the 5 s default (CASE_TIMEOUT_MS). +beforeAll(async () => { + ({ default: MigrateOrganizationOwnership } = await import('./organization-ownership.js')); +}, 60_000); + +const CASE_TIMEOUT_MS = 60_000; +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +/** knex's bring-your-own-dialect hatch: a Client constructor as `config.client`. */ +class Client_PG {} +class Client_WasmSqlite {} + +interface DriverShape { + name?: string; + dialectName?: string; + config?: { client?: unknown }; +} + +/** Refused: no dialect the ceremony can write statements for. */ +const REFUSED: Array<[string, DriverShape]> = [ + ['a client class with no dialectName', { name: 'com.example.driver', config: { client: Client_PG } }], + ['a client class, as SqlDriver answers it (dialectName unknown)', { name: 'com.objectstack.driver.sql', dialectName: 'unknown', config: { client: Client_PG } }], + ['an unrecognised client string', { name: 'com.objectstack.driver.sql', config: { client: 'oracledb' } }], + ['dialectName unknown beside a recognised client string', { name: 'com.objectstack.driver.sql', dialectName: 'unknown', config: { client: 'pg' } }], + ['a driver with neither a dialectName nor a client', { name: 'com.example.driver' }], +]; + +/** Proceeds: each in-repo driver's measured shape, and the first statement its dialect's catalog read. */ +const PROCEEDS: Array<[string, DriverShape, RegExp]> = [ + ['SqlDriver on better-sqlite3', { name: 'com.objectstack.driver.sql', dialectName: 'sqlite', config: { client: 'better-sqlite3' } }, /FROM sqlite_master/], + ['SqliteWasmDriver (a client class, dialectName sqlite)', { name: 'com.objectstack.driver.sqlite-wasm', dialectName: 'sqlite', config: { client: Client_WasmSqlite } }, /FROM sqlite_master/], + ['SqlDriver on pg', { name: 'com.objectstack.driver.sql', dialectName: 'postgres', config: { client: 'pg' } }, /current_schemas\(false\)/], + ['SqlDriver on mysql2', { name: 'com.objectstack.driver.sql', dialectName: 'mysql', config: { client: 'mysql2' } }, /table_schema = DATABASE\(\)/], + ['TursoDriver', { name: 'com.objectstack.driver.turso', dialectName: 'sqlite', config: { client: 'better-sqlite3' } }, /FROM sqlite_master/], + ['a driver with no dialectName and a recognised client string', { name: 'com.example.driver', config: { client: 'postgresql' } }, /current_schemas\(false\)/], +]; + +let dir: string; +/** Every statement the driver was handed, in order. */ +let statements: string[]; + +function stackOver(shape: DriverShape) { + return { + driver: { + ...shape, + execute: async (sql: string) => { + statements.push(sql); + return []; + }, + }, + dbLabel: 'fixture.db', + kernel: { + getService: (name: string) => + name === 'objectql' + ? { registry: { getItem: () => undefined, listItems: () => [], isPackageDisabled: () => false } } + : name === 'metadata' + ? { get: async () => undefined, list: async () => [] } + : undefined, + }, + tableAbsent: () => false, + composition: { securityPlugin: { composed: false, reason: 'no-secret' } }, + shutdown: async () => {}, + }; +} + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'os-ownership-dialect-')); + statements = []; + apply.mockImplementation(async () => ({ + kind: 'organization-ownership-apply', + generatedAt: '2026-10-15T00:00:00.000Z', + steps: [], + tables: [], + complete: true, + blocking: 0, + advisory: 0, + })); +}); + +afterEach(() => { + boot.mockReset(); + apply.mockReset(); + rmSync(dir, { recursive: true, force: true }); +}); + +/** A plan file as a plan run writes it (the fields the apply reads). */ +function writePlan(): string { + const file = join(dir, 'reviewed-plan.json'); + writeFileSync(file, JSON.stringify({ + kind: 'organization-ownership-plan', + ceremony: 'ADR-0131 D10', + generatedAt: '2026-10-15T00:00:00.000Z', + database: 'fixture.db', + inventoryDigest: 'f'.repeat(64), + posture: 'single', + })); + return file; +} + +async function runJson(argv: string[]): Promise<{ payload: Record; exitCode: unknown }> { + const chunks: string[] = []; + const savedExit = process.exitCode; + vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, ...rest: unknown[]) => { + chunks.push(String(chunk)); + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stdout.write); + vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as typeof process.stderr.write); + try { + await MigrateOrganizationOwnership.run(argv, { root: CLI_ROOT }); + } catch (error) { + if ((error as { oclif?: unknown }).oclif === undefined) throw error; + } + const exitCode = process.exitCode; + process.exitCode = savedExit; + vi.restoreAllMocks(); + return { payload: JSON.parse(chunks.join('').trim()) as Record, exitCode }; +} + +describe('an unresolved dialect is refused, naming the driver, before any statement runs', () => { + for (const [label, shape] of REFUSED) { + it(`plan: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const target = join(dir, 'plan.json'); + const { payload, exitCode } = await runJson(['--out', target, '--json']); + expect(payload).toMatchObject({ error: 'plan_refused', reason: 'driver-unsupported' }); + expect(String(payload.detail)).toContain(shape.name!); + expect(exitCode).toBe(1); + expect(statements).toEqual([]); + expect(existsSync(target)).toBe(false); + }, CASE_TIMEOUT_MS); + + it(`--apply: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const { payload, exitCode } = await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'driver-unsupported' }); + expect(String(payload.detail)).toContain(shape.name!); + expect(exitCode).toBe(1); + expect(statements).toEqual([]); + expect(apply).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); + } + + it('a client class is named, never printed as its source', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.example.driver', config: { client: Client_PG } })); + const { payload } = await runJson(['--out', join(dir, 'plan.json'), '--json']); + expect(String(payload.detail)).toContain('client class Client_PG'); + expect(String(payload.detail)).not.toContain('class Client_PG {'); + }, CASE_TIMEOUT_MS); +}); + +describe('a recognised dialect proceeds, in that dialect', () => { + for (const [label, shape, catalog] of PROCEEDS) { + it(`plan: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const { payload } = await runJson(['--out', join(dir, 'plan.json'), '--json']); + // The driver answers no tables, so the plan stops at its catalog read, + // past the dialect, which the catalog statement it issued names. + expect(payload).toMatchObject({ error: 'plan_refused', reason: 'not-an-objectstack-database' }); + expect(statements[0]).toMatch(catalog); + }, CASE_TIMEOUT_MS); + } + + it('--apply hands the apply step the dialect it resolved, and the client string only as a record', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.objectstack.driver.sqlite-wasm', dialectName: 'sqlite', config: { client: Client_WasmSqlite } })); + await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(apply).toHaveBeenCalledTimes(1); + const { executor } = apply.mock.calls[0]![0] as { executor: Record }; + expect(executor.dialect).toBe('sqlite'); + expect(executor).not.toHaveProperty('client'); + }, CASE_TIMEOUT_MS); + + it('--apply on pg: the executor speaks postgres', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.objectstack.driver.sql', dialectName: 'postgres', config: { client: 'pg' } })); + await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(apply).toHaveBeenCalledTimes(1); + expect(apply.mock.calls[0]![0]).toMatchObject({ executor: { dialect: 'postgres', client: 'pg' } }); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index 2c19326067e..1750f1b56aa 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -28,7 +28,7 @@ import { OrganizationOwnershipPlanRefusal, columnListSql, columnNameOf, - dialectOf, + ceremonyDialectOf, type OrganizationOwnershipPlan, } from '../../utils/organization-ownership-plan.js'; import { @@ -87,6 +87,12 @@ async function confirm(question: string): Promise { * ObjectStack one) refuses the whole plan, naming it — exit 1 and no file. ⛔ * Never a table reported as empty. * + * The SQL dialect is read once, off the stack's driver, before either arm + * runs a statement (`ceremonyDialectOf`, the resolver `@objectstack/metadata-core` + * shares): `dialectName` first, then a recognised `config.client` string. A + * driver whose dialect that cannot name is refused, naming the driver, exit 1 + * — ⛔ never read as SQLite. + * * ## Why a subcommand, not a flag on `os migrate` * * Bare `os migrate` is the schema-drift plan (#2186) and stays exactly that. @@ -346,11 +352,15 @@ export default class MigrateOrganizationOwnership extends Command { ); return; } + // The dialect, once, before the first statement: an unresolved one is + // refused here, naming the driver, and never read as SQLite. + const dialect = ceremonyDialectOf(stack.driver); const { normalizeRows } = await import('@objectstack/metadata-protocol'); const client = (stack.driver.config as { client?: unknown } | undefined)?.client; plan = await buildOrganizationOwnershipPlan({ reader: { - client: client === undefined ? undefined : String(client), + dialect, + ...(typeof client === 'string' ? { client } : {}), query: async (sql, params) => normalizeRows(await exec(sql, params ? [...params] : [])), }, posture, @@ -517,6 +527,9 @@ export default class MigrateOrganizationOwnership extends Command { ); return; } + // The dialect, once, before the first statement: an unresolved one is + // refused here, naming the driver, and never read as SQLite. + const dialect = ceremonyDialectOf(stack.driver); if (stack.tableAbsent('sys_migration')) { await refuse( { reason: 'ledger-absent', table: 'sys_migration', detail: 'the deployment ledger the ceremony checkpoints into does not exist' }, @@ -542,7 +555,8 @@ export default class MigrateOrganizationOwnership extends Command { }); const client = (stack.driver.config as { client?: unknown } | undefined)?.client; const executor: ApplyExecutor = { - client: client === undefined ? undefined : String(client), + dialect, + ...(typeof client === 'string' ? { client } : {}), query: async (sql, params) => normalizeRows(await exec(sql, params ? [...params] : [])), execute: async (sql, params) => { await exec(sql, params ? [...params] : []); @@ -619,7 +633,7 @@ export default class MigrateOrganizationOwnership extends Command { // overwrite), then what the registry serves for the bundle member. find: async (name, locale) => { // Once column-drops has run, every row is the environment's (C7b). - const scoped = (await executor.query(columnListSql(dialectOf(executor.client) ?? 'sqlite'), ['sys_metadata'])) + const scoped = (await executor.query(columnListSql(executor.dialect), ['sys_metadata'])) .some((row) => columnNameOf(row) === 'organization_id'); const stored = await executor.query( `SELECT metadata FROM sys_metadata WHERE type = ? AND name = ? AND state = ?${scoped ? ' AND organization_id IS NULL' : ''} ` + diff --git a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts index 578ff41b425..cdbce401a72 100644 --- a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts @@ -136,6 +136,7 @@ async function ceremony(posture: string, db: Database.Database): Promise db.prepare(sql).all(...(params as unknown[])) as Record[], execute: async (sql, params = []) => { db.prepare(sql).run(...(params as unknown[])); }, diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index 7b3db21b643..241fc1b4710 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -57,6 +57,7 @@ let writes: string[]; function executor(opts: { failOnceOn?: RegExp } = {}): ApplyExecutor { let failed = false; return { + dialect: 'sqlite', client: 'better-sqlite3', async query(sql, params = []) { return db.prepare(sql).all(...(params as unknown[])) as Record[]; diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 2f84f96aa8b..8d08e2fd8d7 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -24,7 +24,6 @@ import { SEEDED_BY, TEMPLATE_OBJECT, columnNameOf, - dialectOf, predicateSql, Sql, type AttributionSink, @@ -2559,11 +2558,12 @@ function refuseUnreviewedChoices(plan: OrganizationOwnershipPlan, opts: ApplyOpt * the report. */ export async function applyOrganizationOwnership(opts: ApplyOptions): Promise { - const dialect = dialectOf(opts.executor.client); const now = opts.now ?? (() => new Date()); const run: Run = { opts, - sql: new Sql(opts.executor, dialect ?? 'sqlite'), + // The executor's dialect was resolved from the driver before this call + // (`ceremonyDialectOf`, which refuses an unresolved one): never a default. + sql: new Sql(opts.executor, opts.executor.dialect), plan: undefined as unknown as OrganizationOwnershipPlan, columns: new Map(), attributed: new Map(), diff --git a/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts b/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts index 5f62929516d..16dc039b7ac 100644 --- a/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts @@ -75,7 +75,7 @@ function open(name: string, ddl: string): { db: Database.Database; file: string; return { db, file, - reader: { client: 'better-sqlite3', query: async (sql, params = []) => db.prepare(sql).all(...(params as unknown[])) as Record[] }, + reader: { dialect: 'sqlite', client: 'better-sqlite3', query: async (sql, params = []) => db.prepare(sql).all(...(params as unknown[])) as Record[] }, }; } diff --git a/packages/cli/src/utils/organization-ownership-plan.integration.test.ts b/packages/cli/src/utils/organization-ownership-plan.integration.test.ts index 906fce74fd0..5cc5b1e5024 100644 --- a/packages/cli/src/utils/organization-ownership-plan.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-plan.integration.test.ts @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { buildOrganizationOwnershipPlan, + ceremonyDialectOf, OrganizationOwnershipPlanRefusal, type OrganizationOwnershipPlan, type PlanReader, @@ -28,9 +29,10 @@ let db: Database.Database; /** Every statement the plan issued, in order. */ let statements: string[]; -function reader(opts: { client?: string; failOn?: RegExp } = {}): PlanReader { +function reader(opts: { failOn?: RegExp } = {}): PlanReader { return { - client: opts.client ?? 'better-sqlite3', + dialect: 'sqlite', + client: 'better-sqlite3', async query(sql, params = []) { statements.push(sql); if (opts.failOn?.test(sql)) throw new Error('SQLITE_IOERR: disk I/O error'); @@ -249,9 +251,10 @@ describe('ADR-0131 D10 preflight — it refuses a table it cannot enumerate, nam expect(r.message).toContain('managed_by'); }); - it('a driver with no catalog statement', async () => { - const r = await refusal(plan('isolated', reader({ client: 'oracledb' }))); + it('a driver with no catalog statement: its dialect is refused before the plan issues one', async () => { + const r = await refusal((async () => plan('isolated', { ...reader(), dialect: ceremonyDialectOf({ config: { client: 'oracledb' } }) }))()); expect(r.reason).toBe('driver-unsupported'); + expect(statements).toEqual([]); }); it('a database that is not an ObjectStack one (the wrong --database-url)', async () => { diff --git a/packages/cli/src/utils/organization-ownership-plan.ts b/packages/cli/src/utils/organization-ownership-plan.ts index 2e0d1b3e161..f61f623555c 100644 --- a/packages/cli/src/utils/organization-ownership-plan.ts +++ b/packages/cli/src/utils/organization-ownership-plan.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import { emailTemplateDefinitionFromRow } from '@objectstack/core'; +import { dialectOfDriver } from '@objectstack/metadata-core'; import { hasPlatformObjectPrefix } from '@objectstack/spec/system'; import { APPLICATION_OBJECT_FATE, @@ -54,8 +55,14 @@ export type PlanDialect = 'sqlite' | 'postgres' | 'mysql'; /** The one seam the plan reads through. Rows already normalized across dialects. */ export interface PlanReader { - /** The knex client spelling of the connected driver. */ - client: string | undefined; + /** + * The dialect every statement is written in: resolved ONCE from the stack's + * driver by {@link ceremonyDialectOf}, before the first statement, and never + * re-derived or defaulted here. + */ + dialect: PlanDialect; + /** The knex client spelling, when the driver's `config.client` is a string: recorded in the plan file, never read for the dialect. */ + client?: string; query(sql: string, params?: readonly unknown[]): Promise[]>; } @@ -412,12 +419,42 @@ const DEFAULT_ORGANIZATION_SLUG = 'default'; /** A name the plan may interpolate as a quoted identifier. */ export const SAFE_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/; -export function dialectOf(client: string | undefined): PlanDialect | null { - const c = String(client ?? '').toLowerCase(); - if (['sqlite3', 'sqlite', 'better-sqlite3'].includes(c)) return 'sqlite'; - if (['postgres', 'pg', 'postgresql', 'pgnative'].includes(c)) return 'postgres'; - if (['mysql', 'mysql2'].includes(c)) return 'mysql'; - return null; +/** + * The ceremony's SQL dialect, read off the stack's driver once, before the + * first statement, or a `driver-unsupported` refusal that names the driver. + * + * The one resolver `@objectstack/metadata-core`'s legacy-row reader uses + * (`dialectOfDriver`): the driver's `dialectName` first (the only answer that + * names `SqliteWasmDriver`, whose knex `client` is a dialect CLASS), then a + * recognised `config.client` string. Everything else is refused, never read + * as SQLite: a client class with no `dialectName`, an unrecognised spelling, + * `dialectName: 'unknown'`, or a driver with no SQL dialect at all. A + * statement written for the wrong dialect either fails mid-ceremony or runs + * against the database with the wrong quoting. + */ +export function ceremonyDialectOf(driver: unknown): PlanDialect { + const named = driver as { dialectName?: unknown; config?: { client?: unknown } } | null | undefined; + const dialect = dialectOfDriver(named); + if (dialect === 'sqlite' || dialect === 'postgres' || dialect === 'mysql') return dialect; + throw new OrganizationOwnershipPlanRefusal( + 'driver-unsupported', + `The connected driver (${describeDriver(driver)}) names no SQL dialect the ceremony can write statements for, so ` + + 'none was run. The ceremony supports SQLite, PostgreSQL and MySQL, read from the driver\'s dialectName, or from ' + + 'its config.client when that is a recognised client string; a client class is read only through dialectName.', + ); +} + +/** The driver, as the refusal names it: its name, its `dialectName`, its client. A client class is named, never printed. */ +function describeDriver(driver: unknown): string { + const d = driver as { name?: unknown; dialectName?: unknown; config?: { client?: unknown } } | null | undefined; + const parts = [typeof d?.name === 'string' && d.name ? d.name : 'unnamed driver']; + if (typeof d?.dialectName === 'string') parts.push(`dialectName ${JSON.stringify(d.dialectName)}`); + const client = d?.config?.client; + if (typeof client === 'string') parts.push(`client ${JSON.stringify(client)}`); + else if (typeof client === 'function') parts.push(`client class ${client.name || '(anonymous)'}`); + else if (client === undefined || client === null) parts.push('no client'); + else parts.push(`client of type ${typeof client}`); + return parts.join(', '); } /** sha256 over the inventory, so a plan names the table it was computed from. */ @@ -567,12 +604,12 @@ interface Catalog { } async function readCatalog(sql: Sql, reader: PlanReader, inventoried: ReadonlySet): Promise { - const listSql = physicalTableListSql(reader.client); + const listSql = physicalTableListSql(sql.dialect); if (!listSql) { throw new OrganizationOwnershipPlanRefusal( 'driver-unsupported', - `The connected driver (${reader.client ?? 'unknown client'}) has no catalog statement this plan can run, so ` + - 'no table can be enumerated. The ceremony supports SQLite, PostgreSQL and MySQL.', + `The ${sql.dialect} dialect has no catalog statement this plan can run, so no table can be enumerated. ` + + 'The ceremony supports SQLite, PostgreSQL and MySQL.', ); } let rows: Record[]; @@ -1074,15 +1111,9 @@ export async function buildOrganizationOwnershipPlan(opts: { onAttribution?: AttributionSink; }): Promise { const inventory = opts.inventory ?? ORGANIZATION_OWNERSHIP_INVENTORY; - const dialect = dialectOf(opts.reader.client); - if (!dialect) { - throw new OrganizationOwnershipPlanRefusal( - 'driver-unsupported', - `The connected driver (${opts.reader.client ?? 'unknown client'}) has no catalog statement this plan can run, so ` + - 'no table can be enumerated. The ceremony supports SQLite, PostgreSQL and MySQL.', - ); - } - const sql = new Sql(opts.reader, dialect); + // The reader's dialect was resolved from the driver before this call + // (`ceremonyDialectOf`, which refuses an unresolved one). + const sql = new Sql(opts.reader, opts.reader.dialect); const byObject = new Map(inventory.map((e) => [e.object, e])); const catalog = await readCatalog(sql, opts.reader, new Set(byObject.keys())); diff --git a/packages/cli/src/utils/unmanaged-tables.test.ts b/packages/cli/src/utils/unmanaged-tables.test.ts index c1b757a4520..de58f3d75d8 100644 --- a/packages/cli/src/utils/unmanaged-tables.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.test.ts @@ -366,6 +366,43 @@ describe('collectUnmanagedTables — every no-answer path is unreadable, not emp }); }); +/** + * The dialect comes from the one resolver the organization-ownership ceremony + * shares (`@objectstack/metadata-core`'s `dialectOfDriver`): `dialectName` + * first, then a recognised client string. The shapes are the drivers' own, + * measured: `SqliteWasmDriver`'s knex `client` is a dialect CLASS and its + * `dialectName` is `'sqlite'`; `SqlDriver` answers `'unknown'` for a client + * class it does not recognise. + */ +describe('collectUnmanagedTables — the dialect is read through dialectName first', () => { + const statements: string[] = []; + const driverOf = (shape: { dialectName?: string; client: unknown }) => ({ + managedObjectFields: new Map(), + ...(shape.dialectName !== undefined ? { dialectName: shape.dialectName } : {}), + config: { client: shape.client }, + execute: async (sql: string) => { + statements.push(sql); + return rows('sys_user'); + }, + }); + const run = (driver: unknown) => + collectUnmanagedTables({ drivers: [driver], declaredObjects: [], composition: MIRRORED, normalize }); + + it('reads SqliteWasmDriver (a client class, dialectName sqlite) with the SQLite catalog statement', async () => { + statements.length = 0; + const report = await run(driverOf({ dialectName: 'sqlite', client: class Client_WasmSqlite {} })); + expect(report).toMatchObject({ status: 'read', physicalTables: 1 }); + expect(statements).toEqual([physicalTableListSql('sqlite')]); + }); + + it('a client class whose dialectName is unknown stays unreadable, and nothing is run', async () => { + statements.length = 0; + const report = await run(driverOf({ dialectName: 'unknown', client: class Client_PG {} })); + expect(report.status).toBe('unreadable'); + expect(statements).toEqual([]); + }); +}); + /** * [#22580] Every database the plan covers. The plan diffs the primary and, * where the serving boot keeps one, the `telemetry` sibling; a `read` answer diff --git a/packages/cli/src/utils/unmanaged-tables.ts b/packages/cli/src/utils/unmanaged-tables.ts index 79269b2509d..8d6bc4bb173 100644 --- a/packages/cli/src/utils/unmanaged-tables.ts +++ b/packages/cli/src/utils/unmanaged-tables.ts @@ -113,6 +113,7 @@ * byte-identical to a reader. */ +import { dialectOfDriver } from '@objectstack/metadata-core'; import { PLATFORM_OBJECT_PREFIXES, StorageNameMapping, hasPlatformObjectPrefix } from '@objectstack/spec/system'; // The result-set/no-answer predicate `os migrate duplicates` already publishes // (#10677). Imported rather than re-spelled: a second copy would drift, and the @@ -370,9 +371,16 @@ async function readPhysicalTableNames( return { detail: 'the planned driver exposes no raw SQL seam, so its table catalog could not be read' }; } - const client = (driver as { config?: { client?: unknown } } | null | undefined)?.config?.client; + const named = driver as { dialectName?: unknown; config?: { client?: unknown } } | null | undefined; + const client = named?.config?.client; const clientName = typeof client === 'string' ? client : ''; - const sql = physicalTableListSql(clientName); + // The one driver-dialect resolver (`@objectstack/metadata-core`), shared + // with the organization-ownership ceremony: `dialectName` first, so + // `SqliteWasmDriver`, whose knex `client` is a dialect CLASS, is read as the + // SQLite it is; then a recognised client string. Anything else stays + // unreadable, never a guessed catalog statement. + const dialect = dialectOfDriver(named); + const sql = dialect === 'sqlite' || dialect === 'postgres' || dialect === 'mysql' ? physicalTableListSql(dialect) : null; if (sql === null) { return { detail: `the connected dialect (${clientName || 'unnamed client'}) is not one this sweep enumerates tables on ` diff --git a/packages/metadata-core/src/legacy-organization-row.test.ts b/packages/metadata-core/src/legacy-organization-row.test.ts index 99ab2b31764..56655f25196 100644 --- a/packages/metadata-core/src/legacy-organization-row.test.ts +++ b/packages/metadata-core/src/legacy-organization-row.test.ts @@ -9,7 +9,7 @@ */ import { describe, expect, it } from 'vitest'; -import { LegacyOrganizationRowsUnreadable, readLegacyOrganizationRows } from './legacy-organization-row.js'; +import { dialectOfDriver, LegacyOrganizationRowsUnreadable, readLegacyOrganizationRows } from './legacy-organization-row.js'; /** A SQL driver double: the column list and the legacy select, by statement. */ function sqlDriver(opts: { @@ -151,3 +151,19 @@ describe('readLegacyOrganizationRows — the presence check is the driver\'s cat await expect(readLegacyOrganizationRows(catalogDriver([]))).rejects.toThrow(/column list came back empty/); }); }); + +describe('dialectOfDriver — the one driver-dialect resolver, shared with the organization-ownership ceremony', () => { + it.each([ + ['dialectName first: SqliteWasmDriver\'s client is a dialect class', { dialectName: 'sqlite', config: { client: class Client_WasmSqlite {} } }, 'sqlite'], + ['dialectName postgres', { dialectName: 'postgres', config: { client: 'pg' } }, 'postgres'], + ['dialectName mysql', { dialectName: 'mysql', config: { client: 'mysql2' } }, 'mysql'], + ['no dialectName: a recognised client string', { config: { client: 'postgresql' } }, 'postgres'], + ['dialectName is the whole answer: unknown beside a recognised client', { dialectName: 'unknown', config: { client: 'pg' } }, 'unresolved'], + ['a client class with no dialectName', { config: { client: class Client_PG {} } }, 'unresolved'], + ['an unrecognised client string', { config: { client: 'oracledb' } }, 'unresolved'], + ['no dialectName and no client (memory, MongoDB)', {}, 'none'], + ['no driver', null, 'none'], + ])('%s', (_label, driver, expected) => { + expect(dialectOfDriver(driver)).toBe(expected); + }); +}); From f9f69beda50087c8e15e1adc3243bb68256f5094 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 08:29:29 +0000 Subject: [PATCH 3/3] fix(cli): keep the unmanaged-table sweep as it was, and state the ceremony change as measured The sweep is os migrate plan's, not the ceremony's, and the schema boot never plans SqliteWasmDriver, so the change had no reach there. Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude --- .changeset/23558-ceremony-dialect-resolver.md | 28 ++++++++------ .../organization-ownership.dialect.test.ts | 6 +++ .../cli/src/utils/unmanaged-tables.test.ts | 37 ------------------- packages/cli/src/utils/unmanaged-tables.ts | 12 +----- 4 files changed, 24 insertions(+), 59 deletions(-) diff --git a/.changeset/23558-ceremony-dialect-resolver.md b/.changeset/23558-ceremony-dialect-resolver.md index 189120d1853..452e3d06075 100644 --- a/.changeset/23558-ceremony-dialect-resolver.md +++ b/.changeset/23558-ceremony-dialect-resolver.md @@ -3,25 +3,29 @@ '@objectstack/metadata-core': minor --- -fix(cli): `os migrate organization-ownership` reads its SQL dialect through the driver resolver it shares with metadata-core, `dialectName` first, and refuses an unresolved dialect, naming the driver, before any statement runs +fix(cli): `os migrate organization-ownership` reads its SQL dialect once, through the driver resolver it shares with metadata-core, and refuses an unresolved dialect before any statement, naming the driver, with no SQLite default left Clause-②: yes (widening) -**Who this is for.** An operator who runs the organization-ownership ceremony (plan or `--apply`) on a database whose driver's knex `config.client` is not a client string the ceremony recognises: a client CLASS (knex's bring-your-own-dialect hatch, and `SqliteWasmDriver`'s own config), or an unrecognised spelling. +**Who this is for.** An operator who runs the organization-ownership ceremony (plan or `--apply`) on a PostgreSQL or MySQL database whose driver's knex `config.client` is a client CLASS (knex's bring-your-own-dialect hatch) or a client string the ceremony does not recognise. -**FROM.** Both arms took the dialect from `String(config.client)` alone. +**FROM.** Both arms took the dialect from `String(config.client)` alone, with a reader of their own. -- A client class was stringified, so on `SqliteWasmDriver` (a `wasm-sqlite://` or `*.wasm.db` database, or the development SQLite fallback) the ceremony refused a SQLite database it can migrate, with `driver-unsupported` and the class's source text as the driver's name. -- The apply step and its template lookup carried a `'sqlite'` default for an unresolved dialect, unreachable today only because the plan, which runs first, refused before them. +- Such a run was refused (`driver-unsupported`, exit 1) by the plan step, which runs first. A client class was stringified, so the refusal printed the class's source text in place of the driver's name. +- The apply step and its template lookup each carried a `'sqlite'` default for an unresolved dialect. Neither was reachable, only because the plan step had already refused. -**TO.** The dialect is resolved once, from the stack's driver, before either arm runs a statement. It uses the same resolver metadata-core's legacy-row reader uses: the driver's `dialectName` first, then a recognised `config.client` string. +**TO.** The dialect is resolved once, from the stack's driver, before either arm runs a statement. The resolver is the one metadata-core's legacy-row reader uses: the driver's `dialectName` first, then a recognised `config.client` string. -- `SqliteWasmDriver` (`dialectName: 'sqlite'`) runs the ceremony as SQLite. -- A client class with no recognised `dialectName`, an unrecognised client string, `dialectName: 'unknown'` (what `SqlDriver` answers for either), and a driver with no dialect at all are refused: `driver-unsupported`, exit 1, no statement run, no plan file written. The refusal names the driver (its name, its `dialectName`, its client string or client class name). -- No `'sqlite'` default remains: the plan and the apply take the resolved dialect and never re-derive it. +- Refused: a client class with no recognised `dialectName`, an unrecognised client string, `dialectName: 'unknown'` (what `SqlDriver` answers for either), and a driver with no dialect at all. Each gets `driver-unsupported`, exit 1, with no statement run and no plan file written. The refusal names the driver: its name, its `dialectName`, and its client string or client class name. +- No `'sqlite'` default remains. The plan and the apply take the resolved dialect and never re-derive it. - The plan file's `client` field records the client only when it is a string. -- `os migrate plan`'s unmanaged-table sweep reads its catalog dialect through the same resolver. On `SqliteWasmDriver` it now reads the catalog instead of reporting the sweep unreadable. -Every in-repo driver, measured, resolves as before or better: `SqlDriver` on better-sqlite3, pg and mysql2, and `TursoDriver` (remote and file), are unchanged. `SqliteWasmDriver` moves from refused to SQLite. +Measured on each in-repo SQL driver: `SqlDriver` on better-sqlite3, pg and mysql2, and `TursoDriver`, resolve to the dialect they did before. -**metadata-core.** `dialectOfDriver(driver)` is exported. It is the one resolver behind `readLegacyOrganizationRows`: `'sqlite' | 'postgres' | 'mysql'`, `'none'` for a driver that is not a SQL driver, or `'unresolved'`. A `dialectName` string is the whole answer, recognised or not. A `config.client` is read only when there is no `dialectName`, and only as a string. +**metadata-core.** `dialectOfDriver(driver)` is exported. It is the one resolver behind `readLegacyOrganizationRows`, and answers: + +- `'sqlite' | 'postgres' | 'mysql'`; +- `'none'` for a driver that is not a SQL driver; +- `'unresolved'` otherwise. + +A `dialectName` string is the whole answer, recognised or not. A `config.client` is read only when there is no `dialectName`, and only as a string. diff --git a/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts index 86bf689a197..7e0f8266b1b 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts @@ -15,6 +15,12 @@ * themselves: `SqliteWasmDriver`'s knex `client` is a dialect CLASS and its * `dialectName` is `'sqlite'`; `SqlDriver` answers `dialectName: 'unknown'` * for a client it does not recognise, a client class included. + * + * The `SqliteWasmDriver` row pins the resolver's answer for that SHAPE. The + * command's boot does not hand that driver over today (the schema boot looks + * a SQL driver up by service names that do not include it, so a + * `wasm-sqlite://` run is refused earlier, as having no SQL driver at all); + * the row keeps the answer right for the day it does. */ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; diff --git a/packages/cli/src/utils/unmanaged-tables.test.ts b/packages/cli/src/utils/unmanaged-tables.test.ts index de58f3d75d8..c1b757a4520 100644 --- a/packages/cli/src/utils/unmanaged-tables.test.ts +++ b/packages/cli/src/utils/unmanaged-tables.test.ts @@ -366,43 +366,6 @@ describe('collectUnmanagedTables — every no-answer path is unreadable, not emp }); }); -/** - * The dialect comes from the one resolver the organization-ownership ceremony - * shares (`@objectstack/metadata-core`'s `dialectOfDriver`): `dialectName` - * first, then a recognised client string. The shapes are the drivers' own, - * measured: `SqliteWasmDriver`'s knex `client` is a dialect CLASS and its - * `dialectName` is `'sqlite'`; `SqlDriver` answers `'unknown'` for a client - * class it does not recognise. - */ -describe('collectUnmanagedTables — the dialect is read through dialectName first', () => { - const statements: string[] = []; - const driverOf = (shape: { dialectName?: string; client: unknown }) => ({ - managedObjectFields: new Map(), - ...(shape.dialectName !== undefined ? { dialectName: shape.dialectName } : {}), - config: { client: shape.client }, - execute: async (sql: string) => { - statements.push(sql); - return rows('sys_user'); - }, - }); - const run = (driver: unknown) => - collectUnmanagedTables({ drivers: [driver], declaredObjects: [], composition: MIRRORED, normalize }); - - it('reads SqliteWasmDriver (a client class, dialectName sqlite) with the SQLite catalog statement', async () => { - statements.length = 0; - const report = await run(driverOf({ dialectName: 'sqlite', client: class Client_WasmSqlite {} })); - expect(report).toMatchObject({ status: 'read', physicalTables: 1 }); - expect(statements).toEqual([physicalTableListSql('sqlite')]); - }); - - it('a client class whose dialectName is unknown stays unreadable, and nothing is run', async () => { - statements.length = 0; - const report = await run(driverOf({ dialectName: 'unknown', client: class Client_PG {} })); - expect(report.status).toBe('unreadable'); - expect(statements).toEqual([]); - }); -}); - /** * [#22580] Every database the plan covers. The plan diffs the primary and, * where the serving boot keeps one, the `telemetry` sibling; a `read` answer diff --git a/packages/cli/src/utils/unmanaged-tables.ts b/packages/cli/src/utils/unmanaged-tables.ts index 8d6bc4bb173..79269b2509d 100644 --- a/packages/cli/src/utils/unmanaged-tables.ts +++ b/packages/cli/src/utils/unmanaged-tables.ts @@ -113,7 +113,6 @@ * byte-identical to a reader. */ -import { dialectOfDriver } from '@objectstack/metadata-core'; import { PLATFORM_OBJECT_PREFIXES, StorageNameMapping, hasPlatformObjectPrefix } from '@objectstack/spec/system'; // The result-set/no-answer predicate `os migrate duplicates` already publishes // (#10677). Imported rather than re-spelled: a second copy would drift, and the @@ -371,16 +370,9 @@ async function readPhysicalTableNames( return { detail: 'the planned driver exposes no raw SQL seam, so its table catalog could not be read' }; } - const named = driver as { dialectName?: unknown; config?: { client?: unknown } } | null | undefined; - const client = named?.config?.client; + const client = (driver as { config?: { client?: unknown } } | null | undefined)?.config?.client; const clientName = typeof client === 'string' ? client : ''; - // The one driver-dialect resolver (`@objectstack/metadata-core`), shared - // with the organization-ownership ceremony: `dialectName` first, so - // `SqliteWasmDriver`, whose knex `client` is a dialect CLASS, is read as the - // SQLite it is; then a recognised client string. Anything else stays - // unreadable, never a guessed catalog statement. - const dialect = dialectOfDriver(named); - const sql = dialect === 'sqlite' || dialect === 'postgres' || dialect === 'mysql' ? physicalTableListSql(dialect) : null; + const sql = physicalTableListSql(clientName); if (sql === null) { return { detail: `the connected dialect (${clientName || 'unnamed client'}) is not one this sweep enumerates tables on `