From eccf8d0285c7d8160e1cdb73dc0326da852011ba Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:00:22 +0000 Subject: [PATCH 1/6] wip: declaration boot write guard --- packages/cli/src/utils/schema-migrate.ts | 12 + .../cli/src/utils/schema-migration-plugins.ts | 333 +++++++++++++++++- 2 files changed, 342 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 83af49adf1..cf25edc4d0 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -350,6 +350,18 @@ export async function bootSchemaStack( } await runtime.start(); + // #13332 — the kernel bootstrap is over, and with it the window the + // declaration boot's write guard covers. `composeForDeclarations` suppresses + // a host plugin's `start()`, but `kernel.ts` fires `kernel:ready`, + // `kernel:bootstrapped` and `kernel:listening` unconditionally afterwards, so + // a hook REGISTERED from `init()` runs on a plan; the guard refuses those + // writes at the driver instead of at a list of phase names. Everything from + // this line on is work the command was ASKED for — `apply`'s confirmed DDL + // flush, the #13028 coverage pass — so the guard comes off here and reports + // whatever it refused, which the plan prints and `--json` carries. + const refusalNote = composition.writeGuard?.disarm() ?? null; + if (refusalNote) composition.notes.push(refusalNote); + const driver = findSqlDriver(kernel); // #13028 — the composed host declared its objects in `init()`; the pass that diff --git a/packages/cli/src/utils/schema-migration-plugins.ts b/packages/cli/src/utils/schema-migration-plugins.ts index 242e3fe2b0..ed279262f5 100644 --- a/packages/cli/src/utils/schema-migration-plugins.ts +++ b/packages/cli/src/utils/schema-migration-plugins.ts @@ -79,6 +79,30 @@ import { isAppPluginLike } from './graft-runtime-hooks.js'; * SUPPRESSED ({@link composeForDeclarations}). Same measurement with the * suppression in place: the same 16 managed tables, and **0** insert attempts. * + * ## …and why suppressing `start()` was never the whole guarantee (#13332) + * + * That suppression was scoped to the shape of the ONE plugin that had been + * measured. `composeForDeclarations` overrides `start` and nothing else, while + * `packages/core/src/kernel.ts` fires `kernel:ready` (Phase 3), + * `kernel:bootstrapped` (Phase 3.5) and `kernel:listening` (Phase 4) + * unconditionally after the suppressed start pass. A writing hook REGISTERED + * from `init()` survives on all three — so "a plan writes nothing" was a + * property of plugins that happen to seed from `start()`, an unwritten + * convention nothing checked. Measured downstream: a control plane whose + * plugin created and updated `sys_ai_model` rows from an `init()`-registered + * `kernel:ready` hook, on the `apply=false` run that is its mandatory human + * review gate before a production schema apply. + * + * The fix refuses the WRITE rather than the hook + * ({@link createDeclarationBootWriteGuard}): for the length of the kernel + * bootstrap, the row-write members of the data-driver contract are refused on + * every `driver.*` instance the kernel publishes. Phase-agnostic by + * construction — a fourth phase is covered on the day it ships — and + * read/log-only hooks still run, which is what an operator reading a plan + * before a production apply needs them to do. Neutralising `init()`-registered + * hooks instead would have been neither necessary (a log-only hook violates + * nothing) nor sufficient (a write arriving by any other path still lands). + * * ⚠️ **The residue, stated rather than hidden:** a host plugin that registers * its objects in `start()` instead of `init()` is invisible to this * composition — its tables stay out of the plan. That is the same class of @@ -87,6 +111,15 @@ import { isAppPluginLike } from './graft-runtime-hooks.js'; * out loud what it did, so a missing table is diagnosable instead of being * indistinguishable from "in sync". * + * ⚠️ **The write guard's own residue, likewise stated:** it covers the driver + * contract's row writes, which is where every measured instance of this defect + * landed and the only surface a plugin is supposed to write through. It does + * NOT cover a driver's raw escape hatches (`driver-sql`'s `execute()` and + * `getKnex()`), DDL (held back by `deferSchemaDdl`, and FLUSHED on purpose by + * `apply`), writes a plugin makes outside the database entirely, or work a + * hook defers past the end of the bootstrap. Each is named here so a future + * reader can tell a deliberate boundary from an oversight. + * * `PlatformObjectsPlugin` is deliberately NOT suppressed: it is platform * infrastructure this CLI already boots fully under the sibling DATA * subcommands ({@link ./data-migration-plugins.js}), which are dry-run-by-default @@ -144,6 +177,13 @@ async function suppressedStart(): Promise { * * `destroy()` is forwarded: it is the symmetric teardown of `init()`, and a * plugin that connected something during Phase 1 must still be able to close it. + * + * ⚠️ **This suppression is not, on its own, the "writes nothing" guarantee** + * (#13332). `init()` runs, and every hook it registers fires on the phases + * `kernel.ts` triggers unconditionally after the suppressed start pass. What + * makes the sentence true is {@link createDeclarationBootWriteGuard}, which + * refuses the write itself; this Proxy keeps Phase 2 out of a dry run, which is + * a different and narrower job. */ export function composeForDeclarations(plugin: T): T { return new Proxy(plugin, { @@ -159,6 +199,279 @@ export function composeForDeclarations(plugin: T): T { }) as T; } +/** + * The row-write surface of the data-driver contract + * ({@link @objectstack/spec/contracts.IDataSourceDriver}, declared in + * `packages/spec/src/contracts/data-driver.ts`) — what + * {@link createDeclarationBootWriteGuard} refuses. + * + * Derived from the CONTRACT rather than from a survey of which plugins write + * today, and that is the point of the choice. A list of lifecycle phase names + * goes stale silently — this card started as "`kernel:ready` fires after the + * suppressed start pass" and was three phases before a line was written + * (`kernel:ready`, `kernel:bootstrapped`, `kernel:listening`), and a fourth + * would re-open the hole with nothing turning red. A list of contract members + * goes stale LOUDLY: adding a write to `IDataSourceDriver` is a spec diff, and + * every driver in the repo has to implement it. + * + * DDL is deliberately absent. `deferSchemaDdl` already holds create-table / + * add-column back for this boot, and `os migrate apply` FLUSHES exactly that, + * once, after the operator confirms the plan — guarding it here would refuse + * the one write these commands exist to make. + */ +const DRIVER_ROW_WRITE_METHODS = [ + 'create', + 'update', + 'upsert', + 'delete', + 'bulkCreate', + 'bulkUpdate', + 'bulkDelete', + 'updateMany', + 'deleteMany', +] as const; + +/** One refused write, as the plan reports it. */ +export interface RefusedDeclarationWrite { + /** The `driver.*` kernel service the call was made on. */ + driver: string; + /** The contract method the caller reached for. */ + method: string; + /** The object named in the call, or `(unknown)` when the call carried none. */ + object: string; + /** How many times this exact driver/method/object triple was refused. */ + count: number; +} + +/** + * The declaration boot's write guard — the mechanism behind the sentence + * `buildSchemaMigrationPlugins` prints, "a plan writes nothing" (#13332). + * + * ## Why the guard sits at the DRIVER, not at the plugin + * + * {@link composeForDeclarations} suppresses a host plugin's `start()`, and that + * is where every seeder this repo had measured wrote from. It does NOT touch + * `init()`, and `packages/core/src/kernel.ts` fires `kernel:ready`, + * `kernel:bootstrapped` and `kernel:listening` unconditionally after the + * suppressed start pass. A hook REGISTERED from `init()` therefore runs on a + * plan, on all three — measured downstream as `driver.create` / + * `driver.update` against `sys_ai_model` on the `apply=false` run that is a + * control plane's mandatory human review gate. + * + * Neutralising `init()`-registered hooks instead would enforce a proxy for the + * guarantee, and the proxy is neither necessary nor sufficient: a log-only + * hook violates nothing yet would go silent exactly when an operator is + * reading the plan before a production apply, while a write reaching the + * driver by any other path still lands. The guarantee is about WRITES, so the + * refusal belongs where a write happens. One choke point, phase-agnostic: + * a phase added to the kernel tomorrow is covered on the day it ships. + * + * ## Why the driver INSTANCE, and not the `driver.*` service entry + * + * The instance is shared. `ObjectQLPlugin.init()` walks the kernel's + * `driver.*` services and hands each one to the engine, which keys its + * registry by `driver.name` and DISCARDS a second instance under a name it + * already holds. So a wrapper registered in place of the service would be + * refused by the engine and every `objectql`-mediated write would go straight + * to the raw driver. Guarding the object itself covers both callers — the + * plugin that resolves `driver.*` directly and the engine that writes through + * it — because there is only ever one object. + * + * ## What a refusal does, and why it does not throw + * + * `context.trigger()` dispatches boot hooks PROPAGATING + * (`packages/core/src/hook-dispatch.ts`): a handler that throws aborts the + * bootstrap. Throwing here would turn "your plugin wrote during a dry run" + * into "you cannot get a plan at all" — on the command whose whole job is to + * be read before a production apply. So a refused write returns a benign, + * contract-shaped value, and the run says so out loud: one `console.warn` per + * driver/method/object triple, plus a line in the composition `notes` the plan + * prints and the `--json` payload carries. Nothing is hidden; the operator + * gets both the plan and the list of writes their stack attempted. + */ +export interface DeclarationBootWriteGuard { + /** + * Compose this into the boot's plugin list. It arms in Phase 1 — ordered + * after `DefaultDatasourcePlugin`, which is what registers `driver.*` — and + * re-scans in Phase 2 so a driver registered by a later `init()` is covered + * before any hook phase can fire. + */ + readonly plugin: unknown; + /** Every refusal recorded so far. */ + readonly refusals: readonly RefusedDeclarationWrite[]; + /** + * Restore every guarded driver to the methods it had, and return the line + * for {@link SchemaMigrationComposition.notes} — or `null` when there is + * nothing to report, so a boot in which nothing tried to write renders + * byte-identically to before this existed. + * + * Called once, by `bootSchemaStack`, the moment the kernel bootstrap + * returns: everything after that point is work the command was asked for + * (`apply`'s confirmed DDL flush, the #13028 coverage pass), and it must not + * meet a guard meant for the boot. + */ + disarm(): string | null; +} + +/** What a refused call hands back — shaped like the contract's return value. */ +function refusalValue(method: string, args: readonly unknown[]): unknown { + const copy = (v: unknown): Record => + (v && typeof v === 'object' ? { ...(v as Record) } : {}); + switch (method) { + // Echo the caller's own payload rather than inventing an identity: a + // fabricated id is a second untruth, and a caller that reads one back + // gets nothing from a database that was never written. + case 'create': + case 'upsert': + return copy(args[1]); + case 'update': + return { ...copy(args[2]), id: args[1] }; + case 'delete': + return false; // the contract's "not found" + case 'bulkCreate': + return Array.isArray(args[1]) ? args[1].map(copy) : []; + case 'bulkUpdate': + return Array.isArray(args[1]) + ? args[1].map((u: any) => ({ ...copy(u?.data), id: u?.id })) + : []; + case 'updateMany': + case 'deleteMany': + return 0; // rows affected + case 'bulkDelete': + default: + return undefined; + } +} + +/** + * Build the guard. See {@link DeclarationBootWriteGuard} for the seam and the + * reasoning; this function only assembles it. + */ +export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { + const refusals = new Map(); + /** Guarded instance -> method -> the own descriptor it had, `undefined` when it had none. */ + const armed = new Map>(); + /** Instances a runtime refused to let us guard — reported, never swallowed. */ + const unguardable = new Set(); + const warned = new Set(); + + const refuse = (driverName: string, method: string) => + async function refusedWrite(...args: unknown[]): Promise { + const object = typeof args[0] === 'string' ? args[0] : '(unknown)'; + const key = `${driverName}|${method}|${object}`; + const seen = refusals.get(key); + if (seen) seen.count += 1; + else refusals.set(key, { driver: driverName, method, object, count: 1 }); + if (!warned.has(key)) { + warned.add(key); + // eslint-disable-next-line no-console + console.warn( + `[migrate] ⚠ Refused ${method}() on ${object} via ${driverName}: this is a declaration ` + + 'boot, which writes nothing. The plugin that issued it registers a writing hook ' + + 'outside start() — move the write into start(), or out of the boot path.', + ); + } + return refusalValue(method, args); + }; + + const armDriver = (serviceName: string, driver: unknown): void => { + if (!driver || typeof driver !== 'object') return; + const target = driver as Record; + let saved = armed.get(target); + if (!saved) { + saved = new Map(); + armed.set(target, saved); + } + for (const method of DRIVER_ROW_WRITE_METHODS) { + if (saved.has(method)) continue; // already guarded + if (typeof target[method] !== 'function') continue; // optional member this driver lacks + const original = Object.getOwnPropertyDescriptor(target, method); + try { + Object.defineProperty(target, method, { + value: refuse(serviceName, method), + writable: true, + configurable: true, + // Prototype methods are non-enumerable; an own property that shadows + // one must not start showing up in `for…in` / spread. + enumerable: original?.enumerable ?? false, + }); + } catch { + // A frozen or non-configurable driver. The guarantee cannot be made + // for it, and saying so beats a silent hole. + unguardable.add(`${serviceName}.${method}`); + continue; + } + saved.set(method, original); + } + }; + + const scan = (ctx: any): void => { + const services: Map | undefined = ctx?.getServices?.(); + if (!services || typeof services.entries !== 'function') return; + for (const [name, service] of services.entries()) { + if (typeof name === 'string' && name.startsWith('driver.')) armDriver(name, service); + } + }; + + const plugin = { + name: 'com.objectstack.cli.declaration-boot-write-guard', + version: '1.0.0', + /** + * Ordering, not optionality — the same reason `DeferSchemaDdlPlugin` + * declares it. `resolvePluginOrder` is a DFS in registration order, so + * naming the plugin that registers `driver.*` puts our `init()` after it + * and (because this guard is composed ahead of every host plugin) before + * any host `init()` can reach a driver. `optionalDependencies` rather than + * `dependencies` so a stack assembled without that plugin still boots. + */ + optionalDependencies: ['com.objectstack.runtime.default-datasource'], + init: async (ctx: any): Promise => { scan(ctx); }, + /** + * Re-scan in Phase 2. Host `start()`s are suppressed and the hook phases + * are still ahead, so this is the last point before any host code can run + * — and it catches a driver registered by an `init()` ordered after ours. + */ + start: async (ctx: any): Promise => { scan(ctx); }, + }; + + const describe = (): string | null => { + const parts: string[] = []; + if (refusals.size > 0) { + const total = [...refusals.values()].reduce((n, r) => n + r.count, 0); + const detail = [...refusals.values()] + .map((r) => `${r.method}() on ${r.object}${r.count > 1 ? ` x${r.count}` : ''}`) + .join(', '); + parts.push( + `Refused ${total} write(s) during the declaration boot — a plan writes nothing: ${detail}. ` + + 'A plugin in this stack registers a writing hook outside start(); the plan below is ' + + 'unaffected, but that write WOULD have landed on a served boot of the same stack.', + ); + } + if (unguardable.size > 0) { + parts.push( + `Could NOT guard ${[...unguardable].sort().join(', ')} — the driver refused the override, ` + + 'so writes through those members were NOT suppressed on this run.', + ); + } + return parts.length > 0 ? parts.join(' ') : null; + }; + + return { + plugin, + get refusals(): readonly RefusedDeclarationWrite[] { return [...refusals.values()]; }, + disarm(): string | null { + for (const [target, saved] of armed) { + for (const [method, original] of saved) { + if (original) Object.defineProperty(target, method, original); + else delete (target as Record)[method]; + } + } + armed.clear(); + return describe(); + }, + }; +} + /** Whether `plugins` already carries a `PlatformObjectsPlugin` — `serve` 5c's test. */ function hasPlatformObjects(plugins: readonly unknown[]): boolean { return plugins.some( @@ -244,6 +557,14 @@ export interface SchemaMigrationComposition { * run renders byte-identically to before any of this existed. */ coverage: SchemaMigrationCoverage | null; + /** + * The declaration boot's write guard (#13332), when this composition armed + * one — `undefined` on a boot that composed nothing, so an artifact-less, + * config-less run is untouched. `bootSchemaStack` calls + * {@link DeclarationBootWriteGuard.disarm} the moment the kernel bootstrap + * returns and appends the line it hands back to {@link notes}. + */ + writeGuard?: DeclarationBootWriteGuard; } const NOTHING_COMPOSED: SchemaMigrationComposition = Object.freeze({ @@ -281,7 +602,12 @@ export async function buildSchemaMigrationPlugins(opts: { // artifact-less, config-less run byte-identical to the one before this card. if (!hostConfigPath && !hasArtifactApp) return NOTHING_COMPOSED; - const plugins: unknown[] = []; + // #13332 — armed FIRST, so its `init()` is ordered ahead of every host + // plugin's: `resolvePluginOrder` is a DFS in registration order, and a host + // `init()` that writes directly is only refused if the guard is already on + // the driver by the time it runs. See {@link DeclarationBootWriteGuard}. + const writeGuard = createDeclarationBootWriteGuard(); + const plugins: unknown[] = [writeGuard.plugin]; const notes: string[] = []; let hostConfigLoaded = false; let hostConfigError: string | null = null; @@ -316,7 +642,8 @@ export async function buildSchemaMigrationPlugins(opts: { notes.push( `Composed the host stack from ${path.relative(cwd, hostConfigPath) || hostConfigPath}: ` + `${hostPlugins.length} plugin(s), registered for their declarations only ` - + '(init runs, start does not — a plan writes nothing).', + + '(init runs, start does not), with row writes refused at the driver for the ' + + 'whole boot — a plan writes nothing.', ); } catch (error: any) { // Loud, and on stderr in both modes (`--json` reserves stdout, and the @@ -359,7 +686,7 @@ export async function buildSchemaMigrationPlugins(opts: { notes.push('Composed PlatformObjectsPlugin (the platform floor `os serve` composes unconditionally).'); } - return { plugins, hostConfigPath, hostConfigLoaded, hostConfigError, notes, coverage: null }; + return { plugins, hostConfigPath, hostConfigLoaded, hostConfigError, notes, coverage: null, writeGuard }; } /** From ffb94857d81c31deb1a706bd7a81dc78bde8f556 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:06:15 +0000 Subject: [PATCH 2/6] wip: tests --- ...grate.host-composition.integration.test.ts | 216 ++++++++++- ...ugins.declaration-boot-write-guard.test.ts | 353 ++++++++++++++++++ 2 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts diff --git a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts index a892908a22..e1a909884c 100644 --- a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, writeFileSync, appendFileSync, readFileSync, symlinkSync, rmSync } from 'node:fs'; import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; @@ -494,3 +494,217 @@ describe('a host that brings its OWN ObjectQL engine (#13028 — cloud\'s measur } }, 60_000); }); + +/** + * #13332 — the same guarantee, end to end, against a real SQL driver. + * + * The unit half + * (`schema-migration-plugins.declaration-boot-write-guard.test.ts`) pins the + * mechanism on a recording driver. This half proves the property the operator + * actually depends on: `os migrate plan`'s boot, with a host plugin that + * registers a writing hook from `init()`, leaves the DATABASE unchanged — on a + * database whose tables already exist, which is the condition under which the + * measured inserts SUCCEED instead of failing. + * + * The positive control comes first and is load-bearing. The identical plugin, + * on a boot that composes no host stack (so no declaration composition and no + * write guard), lands its rows. Without that leg the assertion below would be + * green over a fixture that could not have written. + */ +describe('a plan writes nothing even when the host writes from init() (#13332)', () => { + let dir: string; + let dbFile: string; + let hookLog: string; + const savedEnv: Record = {}; + + const PHASES = ['kernel:ready', 'kernel:bootstrapped', 'kernel:listening'] as const; + + /** + * cloud's measured shape, as a plugin this file can hand to either boot: the + * writing hooks are registered from `init()`, so `composeForDeclarations`'s + * `start()` suppression never sees them, and they fire on each of the three + * phases `kernel.ts` triggers unconditionally after the suppressed pass. + * + * The driver is found by scanning `driver.*` — the same surface + * `ObjectQLPlugin`'s discovery loop reads — rather than by naming one, so the + * fixture does not depend on what the standalone stack calls its default. + */ + const initWritingPlugin = (tag: string): any => ({ + name: `com.example.writes-from-init.${tag}`, + version: '1.0.0', + init: async (ctx: any) => { + for (const phase of PHASES) { + ctx.hook(phase, async () => { + appendFileSync(hookLog, `${tag}|log-only|${phase}\n`); + }); + ctx.hook(phase, async () => { + const services: Map = ctx.getServices(); + const entry = [...services.entries()].find(([n]) => n.startsWith('driver.')); + if (!entry) return; + await entry[1].create('sys_metadata', { + id: `os13332-${tag}-${phase}`, + name: `os13332-${tag}-${phase}`, + type: 'os13332_probe', + }); + appendFileSync(hookLog, `${tag}|write|${phase}\n`); + }); + } + }, + }); + + const probeRows = async (driver: any): Promise => { + const rows: any = await driver.knex('sys_metadata') + .where({ type: 'os13332_probe' }) + .count({ c: '*' }); + return Number((Array.isArray(rows) ? rows[0] : rows)?.c ?? -1); + }; + + beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-13332-')); + dbFile = join(dir, 'control.db'); + hookLog = join(dir, 'hooks.log'); + writeFileSync(hookLog, ''); + + // A host config carrying the SAME plugin shape, so the composed path is + // exercised as an operator would hit it — the plugin comes out of + // `objectstack.config.ts`, through `composeForDeclarations`. + writeFileSync( + join(dir, 'objectstack.config.ts'), + [ + "import { appendFileSync } from 'node:fs';", + '', + `const LOG = ${JSON.stringify(hookLog)};`, + "const PHASES = ['kernel:ready', 'kernel:bootstrapped', 'kernel:listening'];", + '', + 'export default {', + ' plugins: [{', + " name: 'com.example.host-writes-from-init',", + " version: '1.0.0',", + ' init: async (ctx: any) => {', + ' for (const phase of PHASES) {', + " ctx.hook(phase, async () => { appendFileSync(LOG, `host|log-only|${phase}\\n`); });", + ' ctx.hook(phase, async () => {', + ' const entry = [...ctx.getServices().entries()]', + " .find(([n]: [string, unknown]) => n.startsWith('driver.'));", + ' if (!entry) return;', + " await entry[1].create('sys_metadata', {", + ' id: `os13332-host-${phase}`,', + ' name: `os13332-host-${phase}`,', + " type: 'os13332_probe',", + ' });', + " appendFileSync(LOG, `host|write|${phase}\\n`);", + ' });', + ' }', + ' },', + ' }],', + '};', + '', + ].join('\n'), + ); + + savedEnv.NODE_ENV = process.env.NODE_ENV; + savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH; + process.env.NODE_ENV = 'production'; + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + + // Materialize the tables the way `os migrate apply` does. The measured + // defect is precisely that on a database whose tables EXIST the inserts + // succeed rather than fail, so the cases below must not run against an + // empty schema. + const boot = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + composeHostStack: true, + projectRoot: dir, + }); + try { + await boot.flushSchemaDdl(); + } finally { + await boot.shutdown(); + } + writeFileSync(hookLog, ''); + }, 60_000); + + afterAll(() => { + if (savedEnv.NODE_ENV === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = savedEnv.NODE_ENV; + if (savedEnv.OS_ARTIFACT_PATH === undefined) delete process.env.OS_ARTIFACT_PATH; + else process.env.OS_ARTIFACT_PATH = savedEnv.OS_ARTIFACT_PATH; + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('POSITIVE CONTROL: the same plugin lands three rows on a boot with no declaration composition', async () => { + const stack = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + // No host composition ⇒ no declaration wrapper and no write guard. This + // is the leg that proves the fixture can write at all. + composeHostStack: false, + extraPlugins: [initWritingPlugin('control')], + projectRoot: dir, + }); + try { + expect(await probeRows(stack.driver)).toBe(3); + const log = readFileSync(hookLog, 'utf8'); + for (const phase of PHASES) expect(log).toContain(`control|write|${phase}`); + } finally { + await stack.shutdown(); + } + }, 60_000); + + it('THE FIX: the declaration boot lands none of them — from the host config or from anywhere else', async () => { + const before = await (async () => { + const s = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + composeHostStack: false, + projectRoot: dir, + }); + try { return await probeRows(s.driver); } finally { await s.shutdown(); } + })(); + // The control's three rows are still there — this case measures a DELTA, + // not an empty table, so a fixture that silently stopped writing cannot + // pass it. + expect(before).toBe(3); + + writeFileSync(hookLog, ''); + const stack = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + composeHostStack: true, + // Both routes at once: the host config's own plugin (composed through + // `composeForDeclarations`) and one handed straight to the kernel. The + // guard sits at the driver, so neither reaches the database. + extraPlugins: [initWritingPlugin('extra')], + projectRoot: dir, + }); + try { + expect(await probeRows(stack.driver)).toBe(before); + + const log = readFileSync(hookLog, 'utf8'); + + // The property (b) was chosen for: the hooks RAN — the log-only ones + // included — on the path an operator reads before a production apply. + for (const phase of PHASES) { + expect(log).toContain(`host|log-only|${phase}`); + expect(log).toContain(`extra|log-only|${phase}`); + // …and the writing hooks got all the way to their `create()` call, + // which returned instead of throwing: the line after it was reached. + expect(log).toContain(`host|write|${phase}`); + expect(log).toContain(`extra|write|${phase}`); + } + + // The refusals are REPORTED, not swallowed — this is the line the plan + // prints and `--json` carries. + const notes = stack.composition.notes.join(' '); + expect(notes).toContain('Refused 6 write(s) during the declaration boot'); + expect(notes).toContain('create() on sys_metadata'); + } finally { + await stack.shutdown(); + } + }, 60_000); +}); diff --git a/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts new file mode 100644 index 0000000000..6d6b43d55c --- /dev/null +++ b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts @@ -0,0 +1,353 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { + composeForDeclarations, + createDeclarationBootWriteGuard, +} from './schema-migration-plugins.js'; + +/** + * #13332 — the declaration boot writes nothing, as a property of the + * MECHANISM. + * + * `composeForDeclarations` suppresses a host plugin's `start()` and nothing + * else, while `packages/core/src/kernel.ts` fires three phases unconditionally + * after the suppressed start pass: + * + * ``` + * :402 Phase 3 trigger('kernel:ready') + * :404 Phase 3.5 trigger('kernel:bootstrapped') + * :416 Phase 4 trigger('kernel:listening') + * ``` + * + * A writing hook REGISTERED from `init()` survives the suppression on all + * three. This file boots a REAL `ObjectKernel` — the same class + * `bootSchemaStack` boots — with a recording driver, and pins both directions: + * + * - the POSITIVE CONTROL first, because a green "no write" over a fixture + * that could not have written proves nothing. The same fixture plugin, on a + * boot composed the way a SERVED boot composes it, writes on every one of + * the three phases; + * - the declaration boot then refuses every one of those writes at the driver + * — while the log-only hooks the same plugin registered still run, which is + * the property this shape was chosen for over neutralising `init()` hooks. + */ + +/** The row-write members of the data-driver contract, as the fixture exercises them. */ +interface RecordedWrite { + method: string; + object: string; +} + +/** + * A driver with the row-write surface of `IDataSourceDriver`, recording what it + * was asked to do. Methods live on the PROTOTYPE, like every real driver's, so + * the guard is exercised against the shape it actually meets: an own property + * shadowing a prototype method, restored by `delete` rather than by rewrite. + */ +class RecordingDriver { + name = 'recording'; + version = '1.0.0'; + writes: RecordedWrite[] = []; + + async create(object: string, data: Record): Promise> { + this.writes.push({ method: 'create', object }); + return { id: 'generated', ...data }; + } + + async update(object: string, id: string, data: Record): Promise> { + this.writes.push({ method: 'update', object }); + return { ...data, id }; + } + + async upsert(object: string, data: Record): Promise> { + this.writes.push({ method: 'upsert', object }); + return { id: 'generated', ...data }; + } + + async delete(object: string, _id: string): Promise { + this.writes.push({ method: 'delete', object }); + return true; + } + + async bulkCreate(object: string, rows: Record[]): Promise[]> { + this.writes.push({ method: 'bulkCreate', object }); + return rows; + } + + async bulkUpdate( + object: string, + updates: Array<{ id: string; data: Record }>, + ): Promise[]> { + this.writes.push({ method: 'bulkUpdate', object }); + return updates.map((u) => ({ ...u.data, id: u.id })); + } + + async bulkDelete(object: string, _ids: string[]): Promise { + this.writes.push({ method: 'bulkDelete', object }); + } + + async updateMany(object: string): Promise { + this.writes.push({ method: 'updateMany', object }); + return 1; + } + + async deleteMany(object: string): Promise { + this.writes.push({ method: 'deleteMany', object }); + return 1; + } + + /** A read, to prove the guard leaves the read half of the contract alone. */ + async find(_object: string): Promise[]> { + return []; + } +} + +/** + * Stands in for `DefaultDatasourcePlugin`: connects a driver in `init()` and + * publishes it as `driver.`, which is the surface `ObjectQLPlugin`'s + * discovery loop and `os migrate`'s `findSqlDriver` both read. + */ +function datasourcePlugin(driver: RecordingDriver): Plugin { + return { + name: 'com.objectstack.runtime.default-datasource', + version: '1.0.0', + init: async (ctx: PluginContext) => { + ctx.registerService(`driver.${driver.name}`, driver); + }, + }; +} + +/** What the fixture host plugin recorded about its own hooks having run. */ +interface HookLog { + ran: string[]; +} + +/** + * cloud's measured shape: a host plugin that registers its WRITING hook from + * `init()` — on each of the three phases the kernel fires unconditionally + * after the suppressed start pass — plus a log-only hook on each, the + * `control-plane-email-guard` shape that must keep running. + * + * It also seeds from `start()`, the shape `composeForDeclarations` already + * suppressed, so one fixture covers both halves. + */ +function writingHostPlugin(log: HookLog): Plugin { + const PHASES = ['kernel:ready', 'kernel:bootstrapped', 'kernel:listening'] as const; + return { + name: 'com.example.writes-from-init', + version: '1.0.0', + init: async (ctx: PluginContext) => { + for (const phase of PHASES) { + ctx.hook(phase, async () => { + log.ran.push(`log-only:${phase}`); + }); + ctx.hook(phase, async () => { + const driver = ctx.getService('driver.recording'); + await driver.create('sys_ai_model', { name: `from-${phase}` }); + log.ran.push(`write:${phase}`); + }); + } + }, + start: async (ctx: PluginContext) => { + const driver = ctx.getService('driver.recording'); + await driver.create('sys_permission_set', { name: 'from-start' }); + log.ran.push('write:start'); + }, + }; +} + +const newKernel = () => new ObjectKernel({ + logger: { level: 'error' }, + gracefulShutdown: false, + skipSystemValidation: true, +}); + +describe('the declaration boot writes nothing (#13332)', () => { + it('POSITIVE CONTROL: the same fixture writes on every phase when nothing suppresses it', async () => { + // A served boot: the host plugin composed as-is, no declaration wrapper and + // no write guard. Without this leg, the assertion below would be green over + // a fixture that could not have written in the first place. + const driver = new RecordingDriver(); + const log: HookLog = { ran: [] }; + const kernel = newKernel(); + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(writingHostPlugin(log)); + await kernel.bootstrap(); + await kernel.shutdown(); + + expect(driver.writes.map((w) => w.method + ':' + w.object)).toEqual([ + 'create:sys_permission_set', // start() + 'create:sys_ai_model', // kernel:ready + 'create:sys_ai_model', // kernel:bootstrapped + 'create:sys_ai_model', // kernel:listening + ]); + expect(log.ran).toContain('write:kernel:ready'); + expect(log.ran).toContain('write:kernel:bootstrapped'); + expect(log.ran).toContain('write:kernel:listening'); + }); + + it('THE DEFECT: suppressing start() alone leaves all three phases writing', async () => { + // The state of the world before this card: `composeForDeclarations` and + // nothing else. `start()`'s seed is gone; the three `init()`-registered + // hooks are untouched. This is the shape the guarantee was measured + // against, so it is pinned rather than described. + const driver = new RecordingDriver(); + const log: HookLog = { ran: [] }; + const kernel = newKernel(); + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(composeForDeclarations(writingHostPlugin(log))); + await kernel.bootstrap(); + await kernel.shutdown(); + + expect(log.ran).not.toContain('write:start'); + expect(driver.writes.map((w) => w.object)).toEqual([ + 'sys_ai_model', + 'sys_ai_model', + 'sys_ai_model', + ]); + }); + + it('THE FIX: the guard refuses every one of them, and the log-only hooks still run', async () => { + const driver = new RecordingDriver(); + const log: HookLog = { ran: [] }; + const guard = createDeclarationBootWriteGuard(); + const kernel = newKernel(); + + // The composition order `buildSchemaMigrationPlugins` produces: the guard + // ahead of every host plugin, so its `init()` is ordered first. + await kernel.use(datasourcePlugin(driver)); + await kernel.use(guard.plugin as Plugin); + await kernel.use(composeForDeclarations(writingHostPlugin(log))); + await kernel.bootstrap(); + + // The whole point: nothing reached the driver. + expect(driver.writes).toEqual([]); + + // …and the hooks themselves still RAN. This is what separates suppressing + // the WRITE from neutralising the HOOK: a read/log-only hook keeps working + // on the path an operator reads before a production apply. + expect(log.ran).toEqual([ + 'log-only:kernel:ready', + 'write:kernel:ready', + 'log-only:kernel:bootstrapped', + 'write:kernel:bootstrapped', + 'log-only:kernel:listening', + 'write:kernel:listening', + ]); + + // The refusal is reported, per phase, rather than swallowed. + const note = guard.disarm(); + expect(note).toContain('Refused 3 write(s)'); + expect(note).toContain('create() on sys_ai_model x3'); + + await kernel.shutdown(); + }); + + it('covers the whole row-write contract, not just create()', async () => { + const driver = new RecordingDriver(); + const guard = createDeclarationBootWriteGuard(); + const kernel = newKernel(); + + const exerciser: Plugin = { + name: 'com.example.exercises-the-contract', + version: '1.0.0', + init: async (ctx: PluginContext) => { + ctx.hook('kernel:bootstrapped', async () => { + const d = ctx.getService('driver.recording'); + await d.create('t', {}); + await d.update('t', 'id1', {}); + await d.upsert('t', {}); + await d.delete('t', 'id1'); + await d.bulkCreate('t', [{}]); + await d.bulkUpdate('t', [{ id: 'id1', data: {} }]); + await d.bulkDelete('t', ['id1']); + await d.updateMany('t'); + await d.deleteMany('t'); + // A read, which must go straight through. + await d.find('t'); + }); + }, + }; + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(guard.plugin as Plugin); + await kernel.use(composeForDeclarations(exerciser)); + await kernel.bootstrap(); + + expect(driver.writes).toEqual([]); + expect(guard.refusals.map((r) => r.method).sort()).toEqual([ + 'bulkCreate', 'bulkDelete', 'bulkUpdate', + 'create', 'delete', 'deleteMany', + 'update', 'updateMany', 'upsert', + ]); + + guard.disarm(); + await kernel.shutdown(); + }); + + it('a refused call hands back a contract-shaped value instead of throwing', async () => { + // `context.trigger()` dispatches boot hooks PROPAGATING, so a throwing + // refusal would abort the bootstrap and leave the operator with no plan at + // all on the command whose job is to be read before a production apply. + const driver = new RecordingDriver(); + const guard = createDeclarationBootWriteGuard(); + const kernel = newKernel(); + const seen: unknown[] = []; + + const reader: Plugin = { + name: 'com.example.reads-back', + version: '1.0.0', + init: async (ctx: PluginContext) => { + ctx.hook('kernel:ready', async () => { + const d = ctx.getService('driver.recording'); + seen.push(await d.create('sys_thing', { name: 'x' })); + seen.push(await d.update('sys_thing', 'id1', { name: 'y' })); + seen.push(await d.delete('sys_thing', 'id1')); + seen.push(await d.deleteMany('sys_thing')); + }); + }, + }; + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(guard.plugin as Plugin); + await kernel.use(composeForDeclarations(reader)); + await kernel.bootstrap(); + + expect(seen).toEqual([{ name: 'x' }, { name: 'y', id: 'id1' }, false, 0]); + guard.disarm(); + await kernel.shutdown(); + }); + + it('disarm() gives the driver its own methods back, byte for byte', async () => { + // `apply` flushes the DDL the operator confirmed AFTER the bootstrap, and + // the #13028 coverage pass runs there too. A guard left on would refuse the + // one write these commands exist to make. + const driver = new RecordingDriver(); + const guard = createDeclarationBootWriteGuard(); + const kernel = newKernel(); + + const before = Object.getPrototypeOf(driver).create; + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(guard.plugin as Plugin); + await kernel.bootstrap(); + + expect(Object.getOwnPropertyDescriptor(driver, 'create')).toBeDefined(); + expect(guard.disarm()).toBeNull(); // nothing was refused — no note at all + + // The shadowing own property is GONE, not overwritten with a copy: the + // instance is back to resolving through its prototype. + expect(Object.getOwnPropertyDescriptor(driver, 'create')).toBeUndefined(); + expect(driver.create).toBe(before); + + await driver.create('sys_thing', {}); + expect(driver.writes).toEqual([{ method: 'create', object: 'sys_thing' }]); + + await kernel.shutdown(); + }); +}); From 8dbfe2cd592df9841b12f4f66db891e742a2e71c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:22:04 +0000 Subject: [PATCH 3/6] wip: guard + tests green --- .../utils/schema-migration-plugins.test.ts | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/utils/schema-migration-plugins.test.ts b/packages/cli/src/utils/schema-migration-plugins.test.ts index df567cafe9..b52336ea82 100644 --- a/packages/cli/src/utils/schema-migration-plugins.test.ts +++ b/packages/cli/src/utils/schema-migration-plugins.test.ts @@ -25,6 +25,9 @@ import { * be in (no host at all / a host that loads / a host that does not). */ +/** The declaration boot's write guard (#13332) — first in every composed list. */ +const WRITE_GUARD = 'com.objectstack.cli.declaration-boot-write-guard'; + const dirs: string[] = []; function tempProject(): string { const dir = mkdtempSync(join(tmpdir(), 'os-12938-unit-')); @@ -137,15 +140,19 @@ describe('buildSchemaMigrationPlugins', () => { basePlugins: [artifactApp], cwd: tempProject(), }); - expect(out.plugins).toHaveLength(1); - expect((out.plugins[0] as any)?.name).toBe('com.objectstack.platform-objects'); + // The write guard leads every composed list (#13332) — see the ordering + // assertion in the host-config case below for why the position matters. + expect(out.plugins.map((p: any) => p?.name)).toEqual([ + WRITE_GUARD, + 'com.objectstack.platform-objects', + ]); // A host that already brought one gets nothing added — `serve` 5c's rule. const already = await buildSchemaMigrationPlugins({ basePlugins: [artifactApp, { name: 'com.objectstack.platform-objects' }], cwd: tempProject(), }); - expect(already.plugins).toEqual([]); + expect(already.plugins.map((p: any) => p?.name)).toEqual([WRITE_GUARD]); }); it('composes a host config\'s plugins for their declarations only', async () => { @@ -166,8 +173,16 @@ describe('buildSchemaMigrationPlugins', () => { const out = await buildSchemaMigrationPlugins({ basePlugins: [], cwd: dir }); expect(out.hostConfigPath).toBe(join(dir, 'objectstack.config.ts')); expect(out.hostConfigLoaded).toBe(true); - // The host plugin, plus the platform floor. - expect(out.plugins).toHaveLength(2); + // The write guard, the host plugin, then the platform floor — and the + // guard's POSITION is load-bearing, not cosmetic (#13332): + // `resolvePluginOrder` is a DFS in registration order, so a host `init()` + // that writes directly is only refused if the guard's `init()` already + // ran. Composed last, it would arm after the write it exists to refuse. + expect(out.plugins.map((p: any) => p?.name)).toEqual([ + WRITE_GUARD, + 'com.example.demo', + 'com.objectstack.platform-objects', + ]); const demo = out.plugins.find((p: any) => p?.name === 'com.example.demo') as any; expect(demo, 'the host plugin must be composed').toBeDefined(); From 75f12db2b0034de28f760c31d83c29b7c1863864 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:22:30 +0000 Subject: [PATCH 4/6] changeset --- .../declaration-boot-write-suppression.md | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 .changeset/declaration-boot-write-suppression.md diff --git a/.changeset/declaration-boot-write-suppression.md b/.changeset/declaration-boot-write-suppression.md new file mode 100644 index 0000000000..22e7346096 --- /dev/null +++ b/.changeset/declaration-boot-write-suppression.md @@ -0,0 +1,53 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): make `os migrate plan`'s "writes nothing" a property of the mechanism, not of an unwritten host convention (#13332) + +`composeForDeclarations` documented the plan path's guarantee in its own words — +*"(init runs, start does not — a plan writes nothing)"* — and implemented it as a +Proxy whose only override is `start`. `packages/core/src/kernel.ts` then fires +three phases unconditionally after the suppressed start pass: `kernel:ready` +(Phase 3), `kernel:bootstrapped` (Phase 3.5) and `kernel:listening` (Phase 4). A +writing hook **registered from `init()`** survives the suppression and executes +on all three. The guarantee was therefore a property of plugins that happen to +seed from `start()` — the shape of the one plugin that had been measured — and +not of the plan path. + +Measured, twice. The module header records 14 `Insert operation failed` rows +against `sys_permission_set` from a deferred `plan` boot, and notes that on a +database whose tables already exist those inserts **succeed**: a command +documented as writing nothing seeds rows into an operator's production control +plane. Downstream, a control plane hit exactly this on the `apply=false` run +that is its mandatory human review gate before a production schema apply +(`driver.create` / `driver.update` on `sys_ai_model`, from an +`init()`-registered `kernel:ready` hook). + +**What changed.** For the length of the kernel bootstrap, `os migrate plan` / +`os migrate apply` now refuse the row-write members of the data-driver contract +(`create`, `update`, `upsert`, `delete`, `bulkCreate`, `bulkUpdate`, +`bulkDelete`, `updateMany`, `deleteMany`) on every `driver.*` instance the +kernel publishes. The refusal sits at the driver, not at a list of lifecycle +phase names: it is phase-agnostic (a phase added tomorrow is covered on the day +it ships), it covers writes that arrive through the ObjectQL engine as well as +direct `driver.*` calls (the engine holds the same instance), and read/log-only +hooks still run — which is what an operator reading a plan before a production +apply needs them to do. A refused write returns a contract-shaped value rather +than throwing (boot hooks dispatch propagating, so throwing would abort the +bootstrap and leave the operator with no plan at all), and every refusal is +reported: one warning on stderr per driver/method/object triple, plus a line in +the composition notes the plan prints and `--json` carries. + +The guard is disarmed the moment the bootstrap returns, so `os migrate apply`'s +confirmed DDL flush and the coverage measurement are untouched. + +**Who this affects.** A host whose plugins write during a `plan`/`apply` boot +from anywhere other than a suppressed `start()`. Those writes previously landed +and now do not; the run says so. A host that already wrote nothing during the +boot sees no change at all — no note is emitted when nothing is refused. + +Boundaries stated rather than hidden: the guard does not cover a driver's raw +escape hatches (`driver-sql`'s `execute()` / `getKnex()`), DDL (already held +back by `deferSchemaDdl`, and flushed on purpose by `apply`), writes a plugin +makes outside the database, or work a hook defers past the end of the +bootstrap. From c472f59e8d0af0e689c76a032d8802b1430cf8e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:30:52 +0000 Subject: [PATCH 5/6] test: make the 13332 integration fixture independent of the fix --- ...grate.host-composition.integration.test.ts | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts index e1a909884c..8f780f1a41 100644 --- a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts @@ -565,6 +565,32 @@ describe('a plan writes nothing even when the host writes from init() (#13332)', hookLog = join(dir, 'hooks.log'); writeFileSync(hookLog, ''); + savedEnv.NODE_ENV = process.env.NODE_ENV; + savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH; + process.env.NODE_ENV = 'production'; + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + + // Materialize `sys_metadata` FIRST, with no host config on disk yet. The + // measured defect is precisely that on a database whose tables EXIST the + // inserts succeed rather than fail, so neither case below may run against + // an empty schema — and the fixture must not depend on the fix to build + // itself: with the guard ablated, a writing hook against a table that does + // not exist yet THROWS, and boot hooks dispatch propagating, so the whole + // bootstrap dies. Setting the schema up before the writer exists keeps an + // ablation landing on the assertions below instead of on this hook. + const boot = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + composeHostStack: false, + projectRoot: dir, + }); + try { + await boot.flushSchemaDdl(); + } finally { + await boot.shutdown(); + } + // A host config carrying the SAME plugin shape, so the composed path is // exercised as an operator would hit it — the plugin comes out of // `objectstack.config.ts`, through `composeForDeclarations`. @@ -602,27 +628,6 @@ describe('a plan writes nothing even when the host writes from init() (#13332)', ].join('\n'), ); - savedEnv.NODE_ENV = process.env.NODE_ENV; - savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH; - process.env.NODE_ENV = 'production'; - process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); - - // Materialize the tables the way `os migrate apply` does. The measured - // defect is precisely that on a database whose tables EXIST the inserts - // succeed rather than fail, so the cases below must not run against an - // empty schema. - const boot = await bootSchemaStack({ - jsonOutput: false, - databaseUrl: `file:${dbFile}`, - deferSchemaDdl: true, - composeHostStack: true, - projectRoot: dir, - }); - try { - await boot.flushSchemaDdl(); - } finally { - await boot.shutdown(); - } writeFileSync(hookLog, ''); }, 60_000); From 02a6dd1dc1191359571083ac16ed511380bb14f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:05:19 +0000 Subject: [PATCH 6/6] =?UTF-8?q?fix(cli):=20R1-R4=20from=20at-tier=20contra?= =?UTF-8?q?ct=20review=20=E2=80=94=20report=20the=20contract's=20execute()?= =?UTF-8?q?=20escape=20hatch,=20correct=20the=20census?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1: execute() is a REQUIRED member of IDataDriver (packages/spec/src/ contracts/data-driver.ts:108, 'Raw Execution (Escape Hatch)'), not a driver-sql extension, and the guard did not touch it: in one guarded boot create() was refused and reported while execute("INSERT ...") landed a row silently under a printed 'a plan writes nothing'. Chosen: REPORT, not cover — a raw command is unknown by contract ('SQL string, shell command, or API payload') and SQL text cannot be classified read-vs-write reliably (a SELECT can quote INSERT in a literal; a CTE can write), while the framework's own boot work (ensureOverlayIndex's index DDL) runs through this seam, so a refusal would either break legitimate calls or rest on a guess. A boot-window execute() is now forwarded, counted per driver, warned once per driver on stderr, and named in the composition notes — and no note claims the plan wrote nothing on such a run: the flat claim is an OUTCOME, printed by disarm() only when it held. R2: 'DDL already held back by deferSchemaDdl' was false for dropTable / rotateShards — they run assertSchemaMutable (a schemaMode/dialect gate, not a deferral check) and execute immediately. The census now states the split and names them a genuinely open boundary. R3: drivers the engine holds for a non-default datasource are never published as driver.* (the only registration is default-datasource-plugin.ts) and are invisible to the guard's scan — now stated in the census, module header and changeset. R4: ObjectQLPlugin.start (plugin.ts:469/:628), not init(), walks the driver.* services; the interface is IDataDriver, not IDataSourceDriver (no such symbol exists in spec). Corrected in docblocks, tests and the changeset. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk --- .../declaration-boot-write-suppression.md | 62 +++-- ...grate.host-composition.integration.test.ts | 77 +++++- ...ugins.declaration-boot-write-guard.test.ts | 78 ++++++- .../cli/src/utils/schema-migration-plugins.ts | 220 +++++++++++++++--- 4 files changed, 375 insertions(+), 62 deletions(-) diff --git a/.changeset/declaration-boot-write-suppression.md b/.changeset/declaration-boot-write-suppression.md index 22e7346096..b5130a7e9b 100644 --- a/.changeset/declaration-boot-write-suppression.md +++ b/.changeset/declaration-boot-write-suppression.md @@ -25,29 +25,49 @@ that is its mandatory human review gate before a production schema apply **What changed.** For the length of the kernel bootstrap, `os migrate plan` / `os migrate apply` now refuse the row-write members of the data-driver contract -(`create`, `update`, `upsert`, `delete`, `bulkCreate`, `bulkUpdate`, -`bulkDelete`, `updateMany`, `deleteMany`) on every `driver.*` instance the -kernel publishes. The refusal sits at the driver, not at a list of lifecycle -phase names: it is phase-agnostic (a phase added tomorrow is covered on the day -it ships), it covers writes that arrive through the ObjectQL engine as well as -direct `driver.*` calls (the engine holds the same instance), and read/log-only -hooks still run — which is what an operator reading a plan before a production -apply needs them to do. A refused write returns a contract-shaped value rather -than throwing (boot hooks dispatch propagating, so throwing would abort the -bootstrap and leave the operator with no plan at all), and every refusal is -reported: one warning on stderr per driver/method/object triple, plus a line in -the composition notes the plan prints and `--json` carries. +(`IDataDriver`: `create`, `update`, `upsert`, `delete`, `bulkCreate`, +`bulkUpdate`, `bulkDelete`, `updateMany`, `deleteMany`) on every `driver.*` +instance the kernel publishes. The refusal sits at the driver, not at a list of +lifecycle phase names: it is phase-agnostic (a phase added tomorrow is covered +on the day it ships), it covers writes that arrive through the ObjectQL engine +as well as direct `driver.*` calls (the engine holds the same instance), and +read/log-only hooks still run — which is what an operator reading a plan before +a production apply needs them to do. A refused write returns a contract-shaped +value rather than throwing (boot hooks dispatch propagating, so throwing would +abort the bootstrap and leave the operator with no plan at all), and every +refusal is reported: one warning on stderr per driver/method/object triple, +plus a line in the composition notes the plan prints and `--json` carries. + +The contract's raw-execution escape hatch — `IDataDriver.execute()`, a required +member on every driver — is FORWARDED and REPORTED rather than refused: a raw +command is `unknown` by contract ("SQL string, shell command, or API payload"), +and SQL text cannot be classified as read-vs-write reliably, so refusing would +break boot-legitimate reads and the framework's own index DDL on a guess. A +boot-window `execute()` is counted per driver, warned once per driver on +stderr, and named in the composition notes — and on such a run the notes do +NOT claim the plan wrote nothing, because the guard cannot verify it. The guard is disarmed the moment the bootstrap returns, so `os migrate apply`'s confirmed DDL flush and the coverage measurement are untouched. **Who this affects.** A host whose plugins write during a `plan`/`apply` boot -from anywhere other than a suppressed `start()`. Those writes previously landed -and now do not; the run says so. A host that already wrote nothing during the -boot sees no change at all — no note is emitted when nothing is refused. - -Boundaries stated rather than hidden: the guard does not cover a driver's raw -escape hatches (`driver-sql`'s `execute()` / `getKnex()`), DDL (already held -back by `deferSchemaDdl`, and flushed on purpose by `apply`), writes a plugin -makes outside the database, or work a hook defers past the end of the -bootstrap. +from anywhere other than a suppressed `start()`. Contract row writes previously +landed and now do not; the run says so. A host whose plugins call raw +`execute()` during the boot keeps its behaviour (the call is forwarded) and +now sees it reported. A host that did neither sees no change at all — no +disarm note is emitted when nothing was refused and no raw command went +through. + +Boundaries stated rather than hidden: `execute()` is reported, never refused +(above); `getKnex()` (a driver-sql extension, genuinely off-contract) is not +intercepted. DDL splits: `deferSchemaDdl` holds back the +`initObjects`/`syncSchema` path (flushed on purpose by `apply` once the +operator confirms), while `dropTable`/`rotateShards` are NOT held back by that +deferral — they are gated only by `assertSchemaMutable` +(schemaMode/dialect) and stay a genuinely open boundary during the boot. +Drivers the engine holds for a NON-default datasource are never published as +`driver.*` (`DatasourceConnectionService.connect()` hands them to +`engine.registerDriver` directly), so they are invisible to the guard's scan +and objectql-mediated writes to objects bound to them would land. The guard +also does not cover writes a plugin makes outside the database, or work a hook +defers past the end of the bootstrap. diff --git a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts index 8f780f1a41..d71daf1e11 100644 --- a/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts @@ -704,12 +704,85 @@ describe('a plan writes nothing even when the host writes from init() (#13332)', } // The refusals are REPORTED, not swallowed — this is the line the plan - // prints and `--json` carries. + // prints and `--json` carries. No raw execute() went through on this + // boot, so the outcome claim HELD and is printed with the report. const notes = stack.composition.notes.join(' '); - expect(notes).toContain('Refused 6 write(s) during the declaration boot'); + expect(notes).toContain('Refused 6 write(s) during the declaration boot — a plan writes nothing'); expect(notes).toContain('create() on sys_metadata'); } finally { await stack.shutdown(); } }, 60_000); + + it('R1 (#14053): a raw execute() is FORWARDED — the row lands — and the run reports it instead of claiming it wrote nothing', async () => { + // The at-tier review's own control shape, pinned: in one guarded boot, a + // hook issues a contract write (refused — the in-run control) and a raw + // `execute("INSERT …")`. `execute()` is a REQUIRED member of `IDataDriver` + // (`packages/spec/src/contracts/data-driver.ts`, "Raw Execution (Escape + // Hatch)"), and the guard cannot classify a raw command as read-vs-write, + // so the row LANDS — that is the documented behaviour, not the defect. + // The defect was the SILENT half: before this case's fix, the same run + // printed "a plan writes nothing" and a refusal list that looked + // complete. Now the notes name the forwarded call and drop the claim. + const rawWritingPlugin: any = { + name: 'com.example.raw-execute-from-init', + version: '1.0.0', + init: async (ctx: any) => { + ctx.hook('kernel:ready', async () => { + const entry = [...ctx.getServices().entries()] + .find(([n]: [string, unknown]) => n.startsWith('driver.')); + if (!entry) return; + const driver = entry[1]; + // In-run control: the guarded surface refuses this one. + await driver.create('sys_metadata', { + id: 'os14053-create-probe', + name: 'os14053-create-probe', + type: 'os14053_create_probe', + }); + // The escape hatch: forwarded, so this one LANDS. + await driver.execute( + "INSERT INTO sys_metadata (id, name, type) VALUES " + + "('os14053-exec-probe', 'os14053-exec-probe', 'os14053_exec_probe')", + ); + }); + }, + }; + + const stack = await bootSchemaStack({ + jsonOutput: false, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + composeHostStack: true, + extraPlugins: [rawWritingPlugin], + projectRoot: dir, + }); + try { + const countByType = async (type: string) => { + const rows: any = await (stack.driver as any).knex('sys_metadata') + .where({ type }).count({ c: '*' }); + return Number((Array.isArray(rows) ? rows[0] : rows)?.c ?? -1); + }; + // The control half: the contract write was refused. + expect(await countByType('os14053_create_probe')).toBe(0); + // The escape hatch half: the raw INSERT landed — forwarded on purpose. + expect(await countByType('os14053_exec_probe')).toBe(1); + + // …and the run SAYS so. The refusal line drops the flat claim (the + // colon directly after "boot" is the dropped phrase), the forwarded + // call is named with its count, and no note in the run claims the + // plan wrote nothing. 4 refusals: the host config's plugin on three + // phases, plus this fixture's in-run control. + const notes = stack.composition.notes.join(' '); + expect(notes).toContain('Refused 4 write(s) during the declaration boot:'); + expect(notes).toContain('Raw execute() was called 1 time(s) during the declaration boot'); + expect(notes).not.toContain('a plan writes nothing'); + + // The guard's structural surface carries it too, for `--json` consumers. + expect(stack.composition.writeGuard?.rawExecutions).toEqual([ + expect.objectContaining({ count: 1 }), + ]); + } finally { + await stack.shutdown(); + } + }, 60_000); }); diff --git a/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts index 6d6b43d55c..fa29300844 100644 --- a/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts +++ b/packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts @@ -42,7 +42,7 @@ interface RecordedWrite { } /** - * A driver with the row-write surface of `IDataSourceDriver`, recording what it + * A driver with the row-write surface of `IDataDriver`, recording what it * was asked to do. Methods live on the PROTOTYPE, like every real driver's, so * the guard is exercised against the shape it actually meets: an own property * shadowing a prototype method, restored by `delete` rather than by rewrite. @@ -51,6 +51,13 @@ class RecordingDriver { name = 'recording'; version = '1.0.0'; writes: RecordedWrite[] = []; + /** Raw commands `execute()` actually RAN — the escape hatch is forwarded, not refused. */ + executed: unknown[] = []; + + async execute(command: unknown): Promise { + this.executed.push(command); + return { ran: command }; + } async create(object: string, data: Record): Promise> { this.writes.push({ method: 'create', object }); @@ -240,14 +247,70 @@ describe('the declaration boot writes nothing (#13332)', () => { 'write:kernel:listening', ]); - // The refusal is reported, per phase, rather than swallowed. + // The refusal is reported, per phase, rather than swallowed — and with no + // raw execute() forwarded this run, the outcome claim HELD and is printed. const note = guard.disarm(); expect(note).toContain('Refused 3 write(s)'); + expect(note).toContain('a plan writes nothing'); expect(note).toContain('create() on sys_ai_model x3'); await kernel.shutdown(); }); + it('execute() — the contract\'s raw escape hatch — is FORWARDED and REPORTED, never silent (#14053 R1)', async () => { + // `execute()` is a REQUIRED member of `IDataDriver` + // (`packages/spec/src/contracts/data-driver.ts`, "Raw Execution (Escape + // Hatch)") — not a driver-sql extension. The guard cannot classify a raw + // command as read-vs-write (the contract admits any native shape), so it + // must not refuse — but the one unacceptable outcome is the SILENT one: a + // raw write landing while the run claims "a plan writes nothing". This + // pins all three properties: forwarded, counted, and the claim dropped. + const driver = new RecordingDriver(); + const guard = createDeclarationBootWriteGuard(); + const kernel = newKernel(); + const seen: unknown[] = []; + const RAW = "INSERT INTO sys_metadata (id) VALUES ('landed')"; + + const rawCaller: Plugin = { + name: 'com.example.calls-execute', + version: '1.0.0', + init: async (ctx: PluginContext) => { + ctx.hook('kernel:ready', async () => { + const d = ctx.getService('driver.recording'); + seen.push(await d.execute(RAW)); + // An in-run control: the guarded surface still refuses. + await d.create('sys_thing', { name: 'refused' }); + }); + }, + }; + + await kernel.use(datasourcePlugin(driver)); + await kernel.use(guard.plugin as Plugin); + await kernel.use(composeForDeclarations(rawCaller)); + await kernel.bootstrap(); + + // FORWARDED: the real driver ran the raw command, and its own return + // value came back to the caller. + expect(driver.executed).toEqual([RAW]); + expect(seen).toEqual([{ ran: RAW }]); + // …while the contract surface was refused in the same boot. + expect(driver.writes).toEqual([]); + + // COUNTED, per driver, on the guard's own surface. + expect(guard.rawExecutions).toEqual([{ driver: 'driver.recording', count: 1 }]); + + // REPORTED — and the flat claim is DROPPED: neither the refusal half nor + // any other part of the note may print "a plan writes nothing" over a run + // in which a raw command went through unclassified. + const note = guard.disarm(); + expect(note).toContain('Refused 1 write(s) during the declaration boot:'); + expect(note).toContain('Raw execute() was called 1 time(s) during the declaration boot'); + expect(note).toContain('1 via driver.recording'); + expect(note).not.toContain('a plan writes nothing'); + + await kernel.shutdown(); + }); + it('covers the whole row-write contract, not just create()', async () => { const driver = new RecordingDriver(); const guard = createDeclarationBootWriteGuard(); @@ -332,18 +395,25 @@ describe('the declaration boot writes nothing (#13332)', () => { const kernel = newKernel(); const before = Object.getPrototypeOf(driver).create; + const beforeExecute = Object.getPrototypeOf(driver).execute; await kernel.use(datasourcePlugin(driver)); await kernel.use(guard.plugin as Plugin); await kernel.bootstrap(); expect(Object.getOwnPropertyDescriptor(driver, 'create')).toBeDefined(); - expect(guard.disarm()).toBeNull(); // nothing was refused — no note at all + expect(Object.getOwnPropertyDescriptor(driver, 'execute')).toBeDefined(); + // Nothing was refused and no raw execute() was CALLED (the forwarding + // shadow was installed, but installation alone is not an event) — so no + // note at all, and a quiet boot renders byte-identically to before. + expect(guard.disarm()).toBeNull(); - // The shadowing own property is GONE, not overwritten with a copy: the + // The shadowing own properties are GONE, not overwritten with a copy: the // instance is back to resolving through its prototype. expect(Object.getOwnPropertyDescriptor(driver, 'create')).toBeUndefined(); expect(driver.create).toBe(before); + expect(Object.getOwnPropertyDescriptor(driver, 'execute')).toBeUndefined(); + expect(driver.execute).toBe(beforeExecute); await driver.create('sys_thing', {}); expect(driver.writes).toEqual([{ method: 'create', object: 'sys_thing' }]); diff --git a/packages/cli/src/utils/schema-migration-plugins.ts b/packages/cli/src/utils/schema-migration-plugins.ts index ed279262f5..cfa3fcf02e 100644 --- a/packages/cli/src/utils/schema-migration-plugins.ts +++ b/packages/cli/src/utils/schema-migration-plugins.ts @@ -111,14 +111,45 @@ import { isAppPluginLike } from './graft-runtime-hooks.js'; * out loud what it did, so a missing table is diagnosable instead of being * indistinguishable from "in sync". * - * ⚠️ **The write guard's own residue, likewise stated:** it covers the driver - * contract's row writes, which is where every measured instance of this defect - * landed and the only surface a plugin is supposed to write through. It does - * NOT cover a driver's raw escape hatches (`driver-sql`'s `execute()` and - * `getKnex()`), DDL (held back by `deferSchemaDdl`, and FLUSHED on purpose by - * `apply`), writes a plugin makes outside the database entirely, or work a - * hook defers past the end of the bootstrap. Each is named here so a future - * reader can tell a deliberate boundary from an oversight. + * ⚠️ **The write guard's own residue, likewise stated** — each boundary named + * here so a future reader can tell a deliberate one from an oversight: + * + * - **`execute()` — the contract's own raw-execution escape hatch.** A + * REQUIRED member of `IDataDriver` (`packages/spec/src/contracts/` + * `data-driver.ts`, under "Raw Execution (Escape Hatch)") — on every + * driver, not a driver-sql extension. It is NOT refused, because a raw + * command is opaque to this guard: the contract admits "SQL string, shell + * command, or API payload", and classifying SQL text as read-vs-write is + * unreliable in both directions (a `SELECT` can quote the word `INSERT` in + * a literal; a CTE can write) — while the framework's own boot-legitimate + * work runs through this very seam (`metadata-protocol`'s + * `ensureOverlayIndex` issues its index DDL here). So a boot-window + * `execute()` is FORWARDED and REPORTED instead: one stderr warning per + * driver, a line in the composition notes, and the notes stop claiming + * "a plan writes nothing" for that run. `getKnex()` (a driver-sql + * extension, genuinely off-contract) is not intercepted; it was not the + * path any measured instance of this defect took. + * - **DDL — and its members split.** `deferSchemaDdl` holds back the + * `initObjects`/`syncSchema` path, which `os migrate apply` FLUSHES on + * purpose once the operator confirms. `dropTable` (and driver-sql's + * `rotateShards`) are NOT held back by that deferral: they run + * `assertSchemaMutable` — a schemaMode/dialect gate, not a deferral check — + * and then execute immediately, so a hook calling `driver.dropTable(...)` + * on a managed datasource during a declaration boot executes, today as + * before this guard. A genuinely open boundary, stated. + * - **Engine-held drivers for non-default datasources.** The guard's scan + * covers the `driver.*` services the kernel publishes, and the only such + * registration repo-wide is the DEFAULT datasource's + * (`packages/runtime/src/default-datasource-plugin.ts`). + * `DatasourceConnectionService.connect()` hands every OTHER datasource's + * driver straight to `engine.registerDriver`, never through `driver.*` — + * so a host stack that connects a second datasource during a composed boot + * holds an engine-side driver this guard cannot see, and objectql-mediated + * writes to objects bound to it would land. + * - writes a plugin makes outside the database entirely (filesystem, + * network). + * - work a hook defers past the end of the bootstrap; the guard covers the + * boot window. * * `PlatformObjectsPlugin` is deliberately NOT suppressed: it is platform * infrastructure this CLI already boots fully under the sibling DATA @@ -201,7 +232,7 @@ export function composeForDeclarations(plugin: T): T { /** * The row-write surface of the data-driver contract - * ({@link @objectstack/spec/contracts.IDataSourceDriver}, declared in + * ({@link @objectstack/spec/contracts.IDataDriver}, declared in * `packages/spec/src/contracts/data-driver.ts`) — what * {@link createDeclarationBootWriteGuard} refuses. * @@ -211,13 +242,18 @@ export function composeForDeclarations(plugin: T): T { * suppressed start pass" and was three phases before a line was written * (`kernel:ready`, `kernel:bootstrapped`, `kernel:listening`), and a fourth * would re-open the hole with nothing turning red. A list of contract members - * goes stale LOUDLY: adding a write to `IDataSourceDriver` is a spec diff, and + * goes stale LOUDLY: adding a write to `IDataDriver` is a spec diff, and * every driver in the repo has to implement it. * - * DDL is deliberately absent. `deferSchemaDdl` already holds create-table / - * add-column back for this boot, and `os migrate apply` FLUSHES exactly that, - * once, after the operator confirms the plan — guarding it here would refuse - * the one write these commands exist to make. + * Two contract members that can write are deliberately NOT in this list, and + * both are stated in the module header's residue census rather than silently + * excluded: `execute()` — the contract's raw-execution escape hatch, which is + * forwarded and REPORTED ({@link DRIVER_RAW_EXECUTION_METHODS}) because a raw + * command cannot be classified as read-vs-write reliably — and DDL, whose + * members split: `deferSchemaDdl` holds back the `initObjects`/`syncSchema` + * path (which `os migrate apply` FLUSHES once the operator confirms — guarding + * it here would refuse the one write these commands exist to make), while + * `dropTable`/`rotateShards` are NOT deferred and stay a stated open boundary. */ const DRIVER_ROW_WRITE_METHODS = [ 'create', @@ -231,6 +267,26 @@ const DRIVER_ROW_WRITE_METHODS = [ 'deleteMany', ] as const; +/** + * The contract's raw-execution escape hatch — `IDataDriver.execute()`, a + * REQUIRED member on every driver (`packages/spec/src/contracts/` + * `data-driver.ts`, "Raw Execution (Escape Hatch)"). + * + * FORWARDED and REPORTED during the boot window, never refused, and the + * reason is stated because it was weighed (#14053 review, R1): the command is + * `unknown` by contract — "SQL string, shell command, or API payload" — and + * classifying SQL text as read-vs-write is unreliable in both directions (a + * `SELECT` can quote the word `INSERT` inside a literal; a CTE can write), so + * a refusal would either break boot-legitimate reads and the framework's own + * index DDL (`ensureOverlayIndex` runs through this seam) or rest on a guess. + * What must never happen instead is the SILENT half: an `execute()` that + * landed a write while the run printed an unqualified "a plan writes + * nothing". So every boot-window call is counted per driver, warned once per + * driver on stderr, and named in the composition notes — which drop the + * "writes nothing" claim for that run ({@link DeclarationBootWriteGuard}). + */ +const DRIVER_RAW_EXECUTION_METHODS = ['execute'] as const; + /** One refused write, as the plan reports it. */ export interface RefusedDeclarationWrite { /** The `driver.*` kernel service the call was made on. */ @@ -244,8 +300,27 @@ export interface RefusedDeclarationWrite { } /** - * The declaration boot's write guard — the mechanism behind the sentence - * `buildSchemaMigrationPlugins` prints, "a plan writes nothing" (#13332). + * One driver's boot-window `execute()` traffic, as the plan reports it. + * Forwarded, not refused — see {@link DRIVER_RAW_EXECUTION_METHODS} for why — + * and counted per driver rather than per statement: the command is `unknown` + * by contract, so there is no object name to key on and echoing raw command + * text into an operator-facing note would leak whatever the caller inlined. + */ +export interface ForwardedRawExecution { + /** The `driver.*` kernel service the call was made on. */ + driver: string; + /** How many `execute()` calls this driver saw during the guarded window. */ + count: number; +} + +/** + * The declaration boot's write guard — the mechanism behind the sentence the + * plan's notes print when the boot is over and it held, "a plan writes + * nothing" (#13332). The sentence is an OUTCOME, so {@link disarm} owns it: + * it is claimed when every write the boot attempted was refused, and dropped + * when a raw `execute()` was forwarded (#14053 review, R1 — a claim printed + * over a write the guard let through is the defect this module exists to + * close). * * ## Why the guard sits at the DRIVER, not at the plugin * @@ -268,14 +343,16 @@ export interface RefusedDeclarationWrite { * * ## Why the driver INSTANCE, and not the `driver.*` service entry * - * The instance is shared. `ObjectQLPlugin.init()` walks the kernel's - * `driver.*` services and hands each one to the engine, which keys its - * registry by `driver.name` and DISCARDS a second instance under a name it + * The instance is shared. `ObjectQLPlugin.start()` walks the kernel's + * `driver.*` services (`packages/objectql/src/plugin.ts` — the discovery loop + * lives in `start`, not `init`) and hands each one to the engine, which keys + * its registry by `driver.name` and DISCARDS a second instance under a name it * already holds. So a wrapper registered in place of the service would be * refused by the engine and every `objectql`-mediated write would go straight * to the raw driver. Guarding the object itself covers both callers — the * plugin that resolves `driver.*` directly and the engine that writes through - * it — because there is only ever one object. + * it — because there is only ever one object, and the engine's write path is a + * call-time property lookup on it. * * ## What a refusal does, and why it does not throw * @@ -299,6 +376,13 @@ export interface DeclarationBootWriteGuard { readonly plugin: unknown; /** Every refusal recorded so far. */ readonly refusals: readonly RefusedDeclarationWrite[]; + /** + * Every boot-window `execute()` call seen so far, per driver — forwarded + * and reported rather than refused ({@link DRIVER_RAW_EXECUTION_METHODS}). + * Non-empty means the run's notes must not (and do not) claim an + * unqualified "a plan writes nothing". + */ + readonly rawExecutions: readonly ForwardedRawExecution[]; /** * Restore every guarded driver to the methods it had, and return the line * for {@link SchemaMigrationComposition.notes} — or `null` when there is @@ -349,11 +433,13 @@ function refusalValue(method: string, args: readonly unknown[]): unknown { */ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { const refusals = new Map(); + const rawExecutions = new Map(); /** Guarded instance -> method -> the own descriptor it had, `undefined` when it had none. */ const armed = new Map>(); /** Instances a runtime refused to let us guard — reported, never swallowed. */ const unguardable = new Set(); const warned = new Set(); + const warnedExec = new Set(); const refuse = (driverName: string, method: string) => async function refusedWrite(...args: unknown[]): Promise { @@ -366,29 +452,59 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { warned.add(key); // eslint-disable-next-line no-console console.warn( - `[migrate] ⚠ Refused ${method}() on ${object} via ${driverName}: this is a declaration ` - + 'boot, which writes nothing. The plugin that issued it registers a writing hook ' + `[migrate] ⚠ Refused ${method}() on ${object} via ${driverName}: a declaration ` + + 'boot refuses row writes. The plugin that issued it registers a writing hook ' + 'outside start() — move the write into start(), or out of the boot path.', ); } return refusalValue(method, args); }; + /** + * The escape hatch is forwarded, never refused — see + * {@link DRIVER_RAW_EXECUTION_METHODS} for the weighed reason — but it is + * COUNTED and SAID: the one unacceptable outcome is a write landing through + * `execute()` while the run prints an unqualified "a plan writes nothing". + */ + const forwardRawExecution = ( + driverName: string, + original: (...args: unknown[]) => unknown, + target: object, + ) => + function forwardedExecute(this: unknown, ...args: unknown[]): unknown { + const seen = rawExecutions.get(driverName); + if (seen) seen.count += 1; + else rawExecutions.set(driverName, { driver: driverName, count: 1 }); + if (!warnedExec.has(driverName)) { + warnedExec.add(driverName); + // eslint-disable-next-line no-console + console.warn( + `[migrate] ⚠ Raw execute() called via ${driverName} during the declaration boot. ` + + 'A raw command cannot be classified as read or write, so it was FORWARDED, not ' + + 'refused — if it wrote, this boot wrote, and the plan\'s notes say so. Issue row ' + + 'writes through the contract methods (which a declaration boot refuses and ' + + 'reports), or move raw commands out of the boot path.', + ); + } + return Reflect.apply(original, this ?? target, args); + }; + const armDriver = (serviceName: string, driver: unknown): void => { if (!driver || typeof driver !== 'object') return; const target = driver as Record; - let saved = armed.get(target); - if (!saved) { - saved = new Map(); - armed.set(target, saved); + let existing = armed.get(target); + if (!existing) { + existing = new Map(); + armed.set(target, existing); } - for (const method of DRIVER_ROW_WRITE_METHODS) { - if (saved.has(method)) continue; // already guarded - if (typeof target[method] !== 'function') continue; // optional member this driver lacks + const saved = existing; + const shadow = (method: string, value: (...args: unknown[]) => unknown): void => { + if (saved.has(method)) return; // already guarded + if (typeof target[method] !== 'function') return; // member this driver lacks const original = Object.getOwnPropertyDescriptor(target, method); try { Object.defineProperty(target, method, { - value: refuse(serviceName, method), + value, writable: true, configurable: true, // Prototype methods are non-enumerable; an own property that shadows @@ -399,9 +515,17 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { // A frozen or non-configurable driver. The guarantee cannot be made // for it, and saying so beats a silent hole. unguardable.add(`${serviceName}.${method}`); - continue; + return; } saved.set(method, original); + }; + for (const method of DRIVER_ROW_WRITE_METHODS) { + shadow(method, refuse(serviceName, method)); + } + for (const method of DRIVER_RAW_EXECUTION_METHODS) { + const original = target[method]; + if (typeof original !== 'function') continue; + shadow(method, forwardRawExecution(serviceName, original as (...args: unknown[]) => unknown, target)); } }; @@ -436,21 +560,42 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { const describe = (): string | null => { const parts: string[] = []; + // The flat claim is made only when it is TRUE of this run: with any raw + // execute() forwarded, whether the boot wrote is not this guard's to + // assert, and the sentence must not print over a write it let through + // (#14053 review, R1). + const writesNothingHeld = rawExecutions.size === 0; if (refusals.size > 0) { const total = [...refusals.values()].reduce((n, r) => n + r.count, 0); const detail = [...refusals.values()] .map((r) => `${r.method}() on ${r.object}${r.count > 1 ? ` x${r.count}` : ''}`) .join(', '); parts.push( - `Refused ${total} write(s) during the declaration boot — a plan writes nothing: ${detail}. ` + `Refused ${total} write(s) during the declaration boot` + + `${writesNothingHeld ? ' — a plan writes nothing' : ''}: ${detail}. ` + 'A plugin in this stack registers a writing hook outside start(); the plan below is ' + 'unaffected, but that write WOULD have landed on a served boot of the same stack.', ); } + if (rawExecutions.size > 0) { + const total = [...rawExecutions.values()].reduce((n, r) => n + r.count, 0); + const detail = [...rawExecutions.values()] + .map((r) => `${r.count} via ${r.driver}`) + .join(', '); + parts.push( + `Raw execute() was called ${total} time(s) during the declaration boot (${detail}) and ` + + 'FORWARDED, not refused: a raw command is opaque to this guard — the contract admits ' + + 'any native shape, and SQL text cannot be classified as read-vs-write reliably — so ' + + 'whether this boot wrote is NOT verified, and this run does NOT claim to have ' + + 'written nothing. If those commands only read, nothing was written; if one wrote, it ' + + 'landed. Issue row writes through the contract methods (refused and reported here), ' + + 'or move raw commands out of the boot path.', + ); + } if (unguardable.size > 0) { parts.push( `Could NOT guard ${[...unguardable].sort().join(', ')} — the driver refused the override, ` - + 'so writes through those members were NOT suppressed on this run.', + + 'so calls through those members were neither refused nor reported on this run.', ); } return parts.length > 0 ? parts.join(' ') : null; @@ -459,6 +604,7 @@ export function createDeclarationBootWriteGuard(): DeclarationBootWriteGuard { return { plugin, get refusals(): readonly RefusedDeclarationWrite[] { return [...refusals.values()]; }, + get rawExecutions(): readonly ForwardedRawExecution[] { return [...rawExecutions.values()]; }, disarm(): string | null { for (const [target, saved] of armed) { for (const [method, original] of saved) { @@ -639,11 +785,15 @@ export async function buildSchemaMigrationPlugins(opts: { } hostConfigLoaded = true; + // The mechanism, not the outcome: whether "a plan writes nothing" HELD + // is only known once the boot is over, so that sentence belongs to the + // guard's disarm() note — which drops it if a raw execute() went + // through (#14053 review, R1) — and must not be pre-claimed here. notes.push( `Composed the host stack from ${path.relative(cwd, hostConfigPath) || hostConfigPath}: ` + `${hostPlugins.length} plugin(s), registered for their declarations only ` - + '(init runs, start does not), with row writes refused at the driver for the ' - + 'whole boot — a plan writes nothing.', + + '(init runs, start does not), with the contract\'s row writes refused at the ' + + 'driver for the whole boot.', ); } catch (error: any) { // Loud, and on stderr in both modes (`--json` reserves stdout, and the