From 04022e06df0b9c9eccd92d2b9b1b553ae7e15270 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 13:45:38 +0000 Subject: [PATCH 1/9] =?UTF-8?q?wip:=20the=20kernel=E2=86=92driver=20supply?= =?UTF-8?q?=20seam=20for=20fileColumnsMoved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- packages/drivers/driver-sql/src/sql-driver.ts | 77 ++++++++- packages/objectql/src/engine.ts | 149 ++++++++++++++++-- packages/platform-objects/src/system/index.ts | 3 + .../src/system/migration-flag.ts | 65 ++++++++ packages/spec/src/system/migration.zod.ts | 33 ++++ 5 files changed, 316 insertions(+), 11 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 53bdf50c2c..2c270a30be 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -4445,7 +4445,16 @@ export type SqlDriverConfig = Knex.Config & { * `registerObjectMetadata`-only) simply never resolves it and stays on the * JSON arm, which is again the correct fail-toward. * - * @see {@link SqlDriver.setFileColumnsMoved} + * ## Naming it at all — in EITHER polarity — shuts out the engine's supply + * + * The ordinary composition does not set this key: `ObjectQL.registerDriver` + * hands the driver a resolver over `sys_migration.columns_moved_at` instead + * (see {@link SqlDriver.setFileColumnsMovedResolver}). Setting it here says + * the host knows its own storage better than the ledger does, so the engine + * will not contradict it — and that is true of `false` as much as of `true`, + * since a declared `false` overruled to `true` is bare ids in a JSON column. + * + * @see {@link SqlDriver.setFileColumnsMovedResolver} */ fileColumnsMoved?: boolean | (() => boolean | Promise); }; @@ -4554,6 +4563,16 @@ export class SqlDriver implements IDataDriver { protected fileColumnsMoved = false; /** The unresolved resolver from config, cleared once it has been asked. */ private fileColumnsMovedResolver?: () => boolean | Promise; + /** + * Did the HOST name {@link SqlDriverConfig.fileColumnsMoved} at + * construction, in either polarity (#15989)? + * + * Distinguishes "the host declared `false`" from "the host said nothing" — + * two states the boolean field above cannot tell apart, because both leave + * it `false`. Only the second is an empty slot + * {@link setFileColumnsMovedResolver} may fill. + */ + private fileColumnsMovedDeclared = false; /** * The columns whose DECLARED type is `boolean` or `toggle` — a READ-COERCION * registry: its readers present the stored form (SQLite INTEGER 0/1, MySQL @@ -5276,6 +5295,11 @@ export class SqlDriver implements IDataDriver { // are ObjectStack concerns, not Knex options — strip them before handing // the config to Knex. const { schemaMode, autoMigrate, sqliteJournalMode, sqliteAbsentFile, fileColumnsMoved, ...knexConfig } = config; + // [#15989] Recorded before the branch, and on the KEY rather than on the + // value: `fileColumnsMoved: false` is a host declaration just as much as + // `true` is, and it must shut the engine's supply seam out — see + // {@link setFileColumnsMovedResolver}. + this.fileColumnsMovedDeclared = fileColumnsMoved !== undefined; if (typeof fileColumnsMoved === 'function') { this.fileColumnsMovedResolver = fileColumnsMoved; } else if (fileColumnsMoved === true) { @@ -17309,9 +17333,58 @@ export class SqlDriver implements IDataDriver { * this memoized without a second boolean: a repeat `initObjects` (the batched * and deferred-DDL paths both call it more than once) finds nothing to ask. */ + /** + * The kernel→driver supply seam for the ADR-0104 media arm (#15989). + * + * `ObjectQL.registerDriver` calls this with a closure over the engine's own + * `haveFileColumnsMoved()`, which reads `sys_migration.columns_moved_at`. + * It is the counterpart of {@link SqlDriverConfig.fileColumnsMoved} for the + * ordinary composition, where nobody hand-writes that option: the driver is + * constructed in an app's config long before any row can be read, so what + * arrives here is the question and {@link resolveFileColumnsMoved} asks it + * once, at `initObjects`. + * + * ## ⛔ A host declaration is never overruled — this fills an empty slot only + * + * If the config named `fileColumnsMoved` at all (a boolean of either + * polarity, or a resolver of the host's own), this is a NO-OP. The host is + * the more specific authority about its own storage, and the failure the + * engine could cause by overruling a declared `false` is the one this whole + * mechanism exists to prevent: bare ids written into a JSON column. + * + * ## It changes nothing that has already been asked + * + * After `initObjects` has run once the arm is resolved and frozen — every + * media column's `isJsonField` answer is already in `jsonFields` — so a + * resolver arriving later would be a promise this driver cannot keep. A + * registration after the first `initObjects` therefore leaves the resolved + * arm alone; the only thing it could do instead is change the write encoding + * of a table whose columns were built for the other one. + * + * @returns whether the resolver was taken, so a caller can tell an + * installation from a refusal instead of inferring it. + */ + setFileColumnsMovedResolver(resolve: () => boolean | Promise): boolean { + if (this.fileColumnsMovedDeclared) return false; + if (this.fileColumnsMovedAsked) return false; + this.fileColumnsMovedResolver = resolve; + return true; + } + + /** Has {@link resolveFileColumnsMoved} already run to completion? */ + private fileColumnsMovedAsked = false; + protected async resolveFileColumnsMoved(): Promise { const resolver = this.fileColumnsMovedResolver; - if (!resolver) return; + if (!resolver) { + // A driver with nothing to ask has still settled its arm: `false`, the + // JSON encoding, which is what it will keep for the rest of its life. + // Recorded so a resolver supplied AFTER the first `initObjects` is + // refused rather than silently changing an already-frozen answer. + this.fileColumnsMovedAsked = true; + return; + } + this.fileColumnsMovedAsked = true; this.fileColumnsMovedResolver = undefined; try { this.fileColumnsMoved = (await resolver()) === true; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 704cd54ae2..62ef3312c9 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -54,6 +54,7 @@ import { FILE_REFERENCES_MIGRATION_ID, VALUE_SHAPES_MIGRATION_ID, isDataMigrationFlagVerified, + hasMovedFileColumns, renderOperationMessage, objectLabelKey, resolveBundleLocale, @@ -264,6 +265,16 @@ import { interface MigrationFlagRead { verified: boolean; conclusive: boolean; + /** + * [#15989] Have the file-family COLUMNS on this deployment moved to the + * bare-id encoding — `hasMovedFileColumns` over the same row? + * + * Carried on the SAME read rather than fetched by a second one, because it + * is a second question about one row and two reads could answer them out of + * one another's date. It is `false` on every row that lacks the stamp, on an + * unverified row, and on every way of not having read a row at all. + */ + columnsMoved: boolean; } /** @@ -5972,6 +5983,7 @@ export class ObjectQL implements IObjectQLEngine { } this.drivers.set(driver.name, driver); + this.supplyFileColumnsMovedResolver(driver); this.logger.info('Registered driver', { driverName: driver.name, version: driver.version @@ -5983,6 +5995,59 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * Hand a freshly-registered driver the ADR-0104 media arm — the kernel→driver + * supply seam (#15989, the ruling on #15041 step 2). + * + * ## Why here, and why a closure rather than a value + * + * `registerDriver` is the one funnel every driver this engine will ever + * route to passes through, and it is early: the resolver is installed before + * `init()` connects and long before schema sync calls `initObjects`, which + * is where a SQL driver asks the closure exactly once and freezes the + * answer. A VALUE could not be supplied here at all — the fact lives in a + * `sys_migration` row this very driver has not connected to yet — so what is + * handed over is the question, not the answer. + * + * ## Duck-typed, and silent when the driver has no such seam + * + * A driver with no media arm (memory, mongodb, a third-party one) exposes no + * `setFileColumnsMovedResolver` and is left alone. That is not a degradation + * to report: a driver that never asks the question keeps whatever encoding + * it always had. + * + * ## ⛔ A host declaration WINS — this only ever fills an empty slot + * + * If the host passed `fileColumnsMoved` to the driver's own config, the + * driver refuses this resolver and says so on its own terms. The asymmetry + * is deliberate and the safety argument runs one way only: a host that + * declared `true` while the ledger says otherwise is a host that knows + * something about its own storage that the ledger does not yet record, and + * the engine silently overruling it to `false` would have the driver write + * JSON into columns that have already been retyped. The reverse — the engine + * overruling a declared `false` to `true` — writes bare ids into a JSON + * column, which is the failure direction the whole mechanism exists to + * prevent. Neither is acceptable, so the more specific authority wins and + * the engine never contradicts an explicit composition. + */ + private supplyFileColumnsMovedResolver(driver: IDataDriver): void { + const sink = driver as unknown as { + setFileColumnsMovedResolver?: (resolve: () => Promise) => void; + }; + if (typeof sink.setFileColumnsMovedResolver !== 'function') return; + try { + sink.setFileColumnsMovedResolver(() => this.haveFileColumnsMoved()); + } catch (e: any) { + // Installing the question must never break a driver registration. A + // driver that did not take the resolver keeps its own default, which is + // the JSON arm — the same place every other way of not knowing lands. + this.logger.debug('Driver declined the ADR-0104 media-arm resolver', { + driverName: driver.name, + error: e?.message ?? String(e), + }); + } + } + /** * Evict a driver from the registry — the removal counterpart of * {@link registerDriver} (#13578). @@ -8178,6 +8243,44 @@ export class ObjectQL implements IObjectQLEngine { ); } + /** + * Have this deployment's file-family COLUMNS moved to the bare-id encoding + * (#15989 — the ruling on #15041, step 2)? + * + * The kernel-side half of the arm a SQL driver writes on. The driver cannot + * ask this itself: the fact lives in a `sys_migration` row, which is a row + * in a table the driver is the one serving, and reading it needs the + * registry and the read pipeline this engine owns. So the engine answers, + * and {@link registerDriver} hands each driver a closure over this method — + * that is the whole kernel→driver supply seam. + * + * ## Every way of not knowing answers "not moved" + * + * No `sys_migration` object registered, no row, an unreadable table, a row + * whose `columns_moved_at` is null or empty, a row that is not verified — + * all `false`, which is today's JSON encoding on every deployment that + * exists. That direction is not a preference: a driver that guessed "moved" + * would write bare ids into a JSON column, which is the one failure this + * whole mechanism is shaped to prevent. ⛔ It is also why the answer is NOT + * the `adr-0104-file-references` flag alone — every creation-attested store + * since 17.0 carries that flag AND JSON-quoted ids. + * + * Shares one memo slot, and therefore one read, with + * {@link isFileReferencesMigrationVerified}: they are two questions about + * one row. `invalidateDataMigrationFlags()` drops both. + */ + async haveFileColumnsMoved(): Promise { + return ( + await this.readMigrationFlagRowMemoized( + 'fileReferencesMigrationVerified', + FILE_REFERENCES_MIGRATION_ID, + '[value-shape] this deployment has verified the file-as-reference migration — ' + + 'media value shapes are enforced and released field files may be collected ' + + '(ADR-0104 / #3617)', + ) + ).columnsMoved; + } + /** * Has this deployment completed AND verified the ADR-0104 non-media * value-shape scan (`os migrate value-shapes`, #3438)? Same memoized seam and @@ -8225,8 +8328,27 @@ export class ObjectQL implements IObjectQLEngine { migrationId: string, verifiedLog: string, ): Promise { + return (await this.readMigrationFlagRowMemoized(slot, migrationId, verifiedLog)).verified; + } + + /** + * The same memoized read, handed back WHOLE (#15989). + * + * The row carries two facts a consumer can want — is the migration verified, + * and have the columns moved — and they must come from one read: two reads + * can straddle a `--apply` and answer out of one another's date, which for + * these two facts means "verified, columns not moved" (the JSON arm) and + * "not verified, columns moved" (an unreachable state) both become + * observable. {@link readMigrationFlagMemoized} is this function projected + * onto its first field, so the two can never disagree. + */ + private async readMigrationFlagRowMemoized( + slot: 'fileReferencesMigrationVerified' | 'valueShapesMigrationVerified', + migrationId: string, + verifiedLog: string, + ): Promise { const cached = this[slot]; - if (cached) return (await cached).verified; + if (cached) return await cached; const pending = this.readMigrationFlagVerified(migrationId, verifiedLog); this[slot] = pending; const result = await pending; @@ -8234,7 +8356,7 @@ export class ObjectQL implements IObjectQLEngine { // by identity so a concurrent `invalidateDataMigrationFlags()` (or a // re-read that already replaced this slot) is not undone here. if (!result.conclusive && this[slot] === pending) this[slot] = null; - return result.verified; + return result; } /** @@ -8255,9 +8377,9 @@ export class ObjectQL implements IObjectQLEngine { private async readMigrationFlagVerified( migrationId: string, verifiedLog?: string, - ): Promise<{ verified: boolean; conclusive: boolean }> { + ): Promise { if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) { - return { verified: false, conclusive: false }; + return { verified: false, conclusive: false, columnsMoved: false }; } try { const rows = await this.find(DATA_MIGRATION_FLAG_OBJECT, { @@ -8266,19 +8388,28 @@ export class ObjectQL implements IObjectQLEngine { context: { isSystem: true } as ExecutionContext, }); const row: any = rows?.[0]; - if (!row || row.id !== migrationId) return { verified: false, conclusive: true }; - const verified = isDataMigrationFlagVerified({ + if (!row || row.id !== migrationId) { + return { verified: false, conclusive: true, columnsMoved: false }; + } + const flag = { id: migrationId, last_run_at: String(row.last_run_at ?? ''), verified_at: row.verified_at == null ? null : String(row.verified_at), // A non-numeric count must read as "not zero", not as 0 — a bad // coercion lands on NaN, which fails the === 0 test. blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), - }); + // [#15989] Read here so both questions come off ONE row: an absent or + // empty stamp is the JSON encoding, which is what every row written + // before the column step existed holds. + columns_moved_at: row.columns_moved_at == null ? null : String(row.columns_moved_at), + }; + const verified = isDataMigrationFlagVerified(flag); if (verified && verifiedLog) this.logger.info(verifiedLog); - return { verified, conclusive: true }; + return { verified, conclusive: true, columnsMoved: hasMovedFileColumns(flag) }; } catch { - return { verified: false, conclusive: false }; // unreadable evidence → stay lenient, keep asking + // unreadable evidence → stay lenient, keep asking; and the columns read + // as NOT moved, which is the encoding every deployment already writes. + return { verified: false, conclusive: false, columnsMoved: false }; } } diff --git a/packages/platform-objects/src/system/index.ts b/packages/platform-objects/src/system/index.ts index d19ba5769a..1057bac92f 100644 --- a/packages/platform-objects/src/system/index.ts +++ b/packages/platform-objects/src/system/index.ts @@ -21,6 +21,9 @@ export { isDataMigrationVerified, mayActIrreversibly, recordDataMigrationRun, + // [#15989] The column-move stamp — its own act, written by the step that + // moves the columns, never implied by a backfill re-run. + recordFileColumnMove, attestFreshDatastore, CREATION_ATTESTATION_DETAIL, type MigrationFlagEngine, diff --git a/packages/platform-objects/src/system/migration-flag.ts b/packages/platform-objects/src/system/migration-flag.ts index f1c444daa8..408c150ebb 100644 --- a/packages/platform-objects/src/system/migration-flag.ts +++ b/packages/platform-objects/src/system/migration-flag.ts @@ -90,6 +90,15 @@ export async function readDataMigrationFlag( deviation_observed_at: row.deviation_observed_at == null ? null : String(row.deviation_observed_at), deviation_detail: typeof row.deviation_detail === 'string' ? row.deviation_detail : undefined, + // [#15989] Absent on every row written before the column step existed, + // which reads as "the columns have NOT moved" — the answer + // `hasMovedFileColumns` gives such a row, and the encoding every + // deployment in the world is on. Carrying it is not cosmetic: dropped (as + // this reader dropped it until the column step landed) a MOVED deployment + // is indistinguishable from an unmoved one to every caller that reads its + // flag through this function, including the step that must refuse to move + // the same columns twice. + columns_moved_at: row.columns_moved_at == null ? null : String(row.columns_moved_at), }; } catch { return null; @@ -194,6 +203,13 @@ export async function recordDataMigrationRun( details: flag.details ?? null, updated_at: now, }; + // [#15989] ⛔ `columns_moved_at` is deliberately NOT a key of `row`. A + // re-run of the backfill says nothing about the physical columns, so it must + // neither set the stamp nor clear it — and OMITTING the key is the only + // spelling that keeps that true when the read above fails: carrying + // `existing?.columns_moved_at ?? null` forward would write a null over a live + // stamp on exactly the read failure `readDataMigrationFlag` answers `null` + // for, demoting a moved deployment back onto the JSON arm on its next boot. if (existing) { await engine.update(DATA_MIGRATION_FLAG_OBJECT, row, { context: { ...SYSTEM_CTX } }); } else { @@ -206,6 +222,55 @@ export async function recordDataMigrationRun( return flag; } +/** + * Stamp `columns_moved_at` — the record that THIS deployment's file-family + * columns were retyped and their values rewritten into the bare-id encoding + * (#15989, the ruling on #15041 step 2). + * + * ## Why it is a separate write from {@link recordDataMigrationRun} + * + * The two attest different facts about the same migration, and they happen at + * different times: the backfill converts the VALUES and can be re-run any + * number of times, while the column move retypes the COLUMNS once. A run that + * re-verifies the values must not imply the columns moved, and the columns + * moving must not re-date the self-check. Mechanism A's whole point is that + * "backfilled here, columns not moved" is representable — so the stamp is its + * own act, written by the step that does the moving, in the same command. + * + * ## It refuses to write on evidence it does not have + * + * There must already be a verified flag row. The column step only runs after + * backfill + verify report zero blocking rows, so the row is there by the time + * this is called; an absent or unverified row means the caller reached here by + * a path that skipped the gate, and stamping would certify a column move whose + * values were never shown converted. That is refused loudly — this is a + * migration command's own output, the direction this module's writes fail in. + * + * @returns the stamp written. + */ +export async function recordFileColumnMove( + engine: MigrationFlagEngine, + migrationId: string, +): Promise { + const existing = await readDataMigrationFlag(engine, migrationId); + if (!isDataMigrationFlagVerified(existing)) { + throw new Error( + `Refusing to record the column move for '${migrationId}': this deployment has no VERIFIED ` + + `${DATA_MIGRATION_FLAG_OBJECT} row for it. The column move may only be recorded by a run ` + + 'whose backfill and self-check reported zero blocking rows — recording it otherwise would ' + + 'certify a column move whose values were never shown converted, and the driver would then ' + + 'write bare ids on the strength of it.', + ); + } + const now = new Date().toISOString(); + await engine.update( + DATA_MIGRATION_FLAG_OBJECT, + { id: migrationId, columns_moved_at: now, updated_at: now }, + { context: { ...SYSTEM_CTX } }, + ); + return now; +} + /** * The `os migrate` sub-command that re-earns each id a boot's own admitted * value can CONTRADICT — and, by having no row for anything else, the register diff --git a/packages/spec/src/system/migration.zod.ts b/packages/spec/src/system/migration.zod.ts index e99721356e..8f4b654b92 100644 --- a/packages/spec/src/system/migration.zod.ts +++ b/packages/spec/src/system/migration.zod.ts @@ -320,6 +320,39 @@ export function authorisesIrreversibleAction(flag: DataMigrationFlag | null | un return isDataMigrationFlagVerified(flag) && !hasObservedDeviation(flag); } +/** + * Have this deployment's file-family COLUMNS moved to the bare-id encoding? + * + * The third arbiter over the same row, and the one a storage driver keys its + * WRITE encoding on. It is deliberately stronger than + * {@link isDataMigrationFlagVerified}: the backfill and its self-check attest + * the VALUES, `columns_moved_at` attests the COLUMNS, and a deployment can + * carry the first without the second — that is the ordinary state of every + * deployment that ran `os migrate files-to-references --apply` before a column + * step existed. So the two facts are conjoined here rather than either one + * standing alone: + * + * - `columns_moved_at` alone would be a column move with no evidence that the + * values inside those columns were ever converted; + * - `verified_at` alone is the key that must NEVER be used — every + * creation-attested store since 17.0 holds it AND JSON-quoted ids in a JSON + * column, so keying on it would read every existing deployment as moved and + * then write bare ids into a JSON column. + * + * ## Absence is the legacy encoding, and that is the whole safety property + * + * No row, an unreadable row, a row whose `columns_moved_at` is null or empty, + * a row that is not verified — every one of them answers `false`, which is + * today's JSON encoding on every deployment in the world. A consumer that + * cannot read this fact must assume it is false; the column's own description + * says so, and this predicate is where that sentence is executable. + */ +export function hasMovedFileColumns(flag: DataMigrationFlag | null | undefined): boolean { + if (!isDataMigrationFlagVerified(flag)) return false; + const moved = flag?.columns_moved_at; + return moved != null && moved !== ''; +} + // --- Migration journal (ADR-0119 D2, #4617) --- // // The flag above and the journal below answer DIFFERENT questions, and From b609c7f18a8a2e84968fced3dc9f8d85aedc02da Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 13:58:54 +0000 Subject: [PATCH 2/9] wip: the per-dialect media column move statements + planner Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- packages/drivers/driver-sql/src/index.ts | 24 ++ .../driver-sql/src/media-column-move.ts | 233 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 117 +++++++++ 3 files changed, 374 insertions(+) create mode 100644 packages/drivers/driver-sql/src/media-column-move.ts diff --git a/packages/drivers/driver-sql/src/index.ts b/packages/drivers/driver-sql/src/index.ts index d3a7a79f8b..38318f990d 100644 --- a/packages/drivers/driver-sql/src/index.ts +++ b/packages/drivers/driver-sql/src/index.ts @@ -122,6 +122,30 @@ export type { FieldDef as DriftFieldDef, } from './schema-drift.js'; +// [#15989] The ADR-0104 file-family COLUMN step. Published for the same reason +// `manualJsonConversionSql` is reachable from `os migrate multi-value-columns`: +// the statements belong to the package that owns the dialects and MEASURED +// them, and a second copy in the CLI could only ever go stale — which here +// means a second copy of the clause the ruling's own text got wrong. The +// command `await import()`s these at the point of use (⛔ never a static value +// import of a driver package from a CLI command module — see +// `schema-migrate.lazy-driver-import.test.ts`). +export { + MEDIA_COLUMN_MOVE_DIALECTS, + MEDIA_COLUMN_MOVE_ROLLBACK_NOTES, + MEDIA_ID_MOVE_WIDTH, + isJsonColumnType, + mediaColumnMoveDialect, + mediaColumnMovePlan, +} from './media-column-move.js'; +export type { + MediaColumnMoveDialect, + MediaColumnMoveKind, + MediaColumnMovePlan, + MediaColumnMoveRefusal, + MediaColumnMoveScan, +} from './media-column-move.js'; + export default { id: 'com.objectstack.driver.sql', version: '1.0.0', diff --git a/packages/drivers/driver-sql/src/media-column-move.ts b/packages/drivers/driver-sql/src/media-column-move.ts new file mode 100644 index 0000000000..b63b7d804e --- /dev/null +++ b/packages/drivers/driver-sql/src/media-column-move.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The ADR-0104 file-family COLUMN step — the per-dialect statements + * that move a media column's stored form from the JSON-quoted id to the bare + * `sys_file` id, and the pre-check that ABORTS instead of destroying a row the + * backfill never converted. + * + * The ruling on #15041 gave this step one requirement in words — abort *"on + * the first cell that is not a JSON string"* — and one sketch in SQL beside + * it. **The sketch does not implement the requirement, and that was measured + * rather than argued** (director ruling, decision batch #120 item 1): on live + * PostgreSQL 16.13 the prescribed `ALTER … USING (col #>> '{}')` was ACCEPTED + * over a row holding an inline metadata blob and flattened that object to the + * literal text `{"url":"https://x/y.png"}` in a `varchar` column, because + * `#>> '{}'` extracts **any** json type as text. Landing the sketch as written + * would silently destroy exactly the rows the backfill has not converted. + * + * So every arm below is a PAIR — a pre-check that counts the cells the move + * would not preserve, and the statement that moves them — and the caller must + * run the first and abort on a non-zero count before it runs the second. The + * pre-check is the clause the dev seat measured; the statement keeps the + * ruling's shape. + * + * ## MySQL is deliberately absent + * + * ⛔ #17788 (`pm:on-hold`, `Restart-when: a MySQL 8.x instance is reachable + * from the dispatch environment`) owns the MySQL leg, and it owns it for a + * reason this module must not paper over: the addendum leaves MySQL's + * statement ORDER unsettled, and settling it needs a real instance — the same + * thing that turned the Postgres sketch from plausible into measured-wrong. + * {@link mediaColumnMoveDialect} therefore answers `null` for MySQL, and the + * caller reports a named refusal rather than inventing a third wording. + */ + +import type { SqlDialectName } from './schema-drift.js'; + +/** The dialects whose column move is measured and executable today. */ +export const MEDIA_COLUMN_MOVE_DIALECTS = ['postgres', 'sqlite'] as const; + +export type MediaColumnMoveDialect = (typeof MEDIA_COLUMN_MOVE_DIALECTS)[number]; + +/** + * Is this dialect's column move executable here, and under which name? + * + * Answers `null` for every dialect that is not in + * {@link MEDIA_COLUMN_MOVE_DIALECTS} — today MySQL and `unknown`. ⛔ A caller + * may not fall back to another dialect's statements on a `null`: the two forms + * are not interchangeable, and guessing is what this module exists to stop. + */ +export function mediaColumnMoveDialect(dialect: SqlDialectName): MediaColumnMoveDialect | null { + return (MEDIA_COLUMN_MOVE_DIALECTS as readonly string[]).includes(dialect) + ? (dialect as MediaColumnMoveDialect) + : null; +} + +/** + * Which shape of move a column needs — decided by the physical type it HAS, + * never by the dialect alone. + * + * - `retype` — the column is a JSON column (only a server dialect can have + * one). The move changes the type and rewrites the values in one statement. + * - `unquote` — the column is already a string column holding JSON-quoted ids. + * This population is real and measured: `os generate migration --format sql` + * emits `VARCHAR(2048)` for the family, and a driver on the JSON arm writes + * quoted ids into it (#15771, reproduced on live PostgreSQL 16.13). It is + * also the ONLY shape SQLite ever has, since SQLite has no json type and the + * JSON arm has always written JSON text into a `text` column. + * + * ⛔ Reading the shape off the dialect instead of off the column is the defect + * that would leave a `varchar` full of `"file_…"` behind a `columns_moved_at` + * stamp saying it had been converted. + */ +export type MediaColumnMoveKind = 'retype' | 'unquote'; + +/** One column's move: the abort pre-check, then the statement. */ +export interface MediaColumnMovePlan { + dialect: MediaColumnMoveDialect; + kind: MediaColumnMoveKind; + table: string; + column: string; + /** + * Counts the cells this move would NOT preserve. The caller runs it first + * and ABORTS on any non-zero answer — ⛔ it is not advisory, and it is not + * something the statement below re-checks for itself. + */ + precheck: string; + /** What a non-zero pre-check count means, in one operator-facing sentence. */ + precheckMeaning: string; + /** The move. Run ONLY after the pre-check answered zero. */ + statement: string; +} + +/** + * The width the move retypes to — the SQL generator's own + * (`packages/cli/src/commands/generate.ts` emits `VARCHAR(2048)`), and the + * same constant the driver's `varcharColumnChars` mirrors on the moved arm. + * Transcribed rather than imported because this module builds text for a + * server and must not depend on the driver class it is built for. + */ +export const MEDIA_ID_MOVE_WIDTH = 2048; + +/** Is this physical column type a JSON column, as the server reports it? */ +export function isJsonColumnType(physicalType: string | undefined | null): boolean { + return typeof physicalType === 'string' && /json/i.test(physicalType); +} + +/** + * Build one column's move. + * + * @param dialect the dialect, already narrowed by {@link mediaColumnMoveDialect} + * @param kind read off the column's PHYSICAL type — see {@link MediaColumnMoveKind} + */ +export function mediaColumnMovePlan( + dialect: MediaColumnMoveDialect, + kind: MediaColumnMoveKind, + table: string, + column: string, +): MediaColumnMovePlan { + const base = { dialect, kind, table, column } as const; + + if (dialect === 'sqlite') { + // SQLite has one shape only: the column is `text` on both arms, so there + // is nothing to retype and the whole move is the value rewrite. + // + // The discriminator is `json_type`, measured on the first step-3 rehearsal + // anywhere: an un-backfilled inline-object cell answers `'object'`, a + // converted one answers `'text'`, and a cell that is ALREADY bare is not + // valid JSON at all, so `json_valid` excludes it. That exclusion is what + // makes the move idempotent — re-running it leaves a bare cell untouched + // rather than aborting on it, which a literal "not a JSON string" test + // would have done on every already-converted row. + return { + ...base, + precheck: + `select count(*) as n from "${table}" ` + + `where "${column}" is not null and json_valid("${column}") ` + + `and json_type("${column}") <> 'text'`, + precheckMeaning: + 'cell(s) hold a JSON value that is not a string — an inline metadata blob, an array, ' + + 'or a number. The unquote would turn each of them into something else, so the move ' + + 'is refused: run the backfill until it reports zero blocking rows first.', + statement: + `update "${table}" set "${column}" = json_extract("${column}", '$') ` + + `where "${column}" is not null and json_valid("${column}") ` + + `and json_type("${column}") = 'text'`, + }; + } + + if (kind === 'retype') { + // The ruling's own statement, kept — with the pre-check the ruling's text + // lacked in front of it. `json_typeof` is total over a json column, so + // `IS DISTINCT FROM 'string'` really is "every cell that is not a JSON + // string", and it is the exact clause measured to answer 1 on the fixture + // that `#>> '{}'` silently flattened. + // + // The `::json` cast makes the pre-check read a `jsonb` column too; the + // `#>>` operator is defined on both, so only the pre-check needs it. + return { + ...base, + precheck: + `select count(*) as n from "${table}" ` + + `where "${column}" is not null and json_typeof("${column}"::json) is distinct from 'string'`, + precheckMeaning: + 'cell(s) hold a JSON value that is not a string — an inline metadata blob left by a ' + + 'backfill that has not converted them. `USING (col #>> \'{}\') ` does NOT refuse those: ' + + 'measured on live PostgreSQL 16.13, it flattens the object to its literal text and the ' + + 'original value is gone. The move is refused instead.', + statement: + `alter table "${table}" alter column "${column}" ` + + `type varchar(${MEDIA_ID_MOVE_WIDTH}) using ("${column}" #>> '{}')`, + }; + } + + // Postgres, column already a string type (#15771's population). Nothing to + // retype; the quoted ids inside it still have to move. + // + // PostgreSQL has no `json_valid`, so the pre-check names the family that + // matters rather than testing validity: a cell opening with `{` or `[` is an + // unconverted blob, which is the same thing the two clauses above refuse. A + // cell that opens with `"` but is not a well-formed JSON string raises on + // the cast in the statement below and aborts it whole — loudly, and with the + // column untouched, because a Postgres statement is atomic. + return { + ...base, + precheck: + `select count(*) as n from "${table}" ` + + `where "${column}" is not null and left("${column}", 1) in ('{', '[')`, + precheckMeaning: + 'cell(s) hold an inline metadata blob or an array rather than an id. Unquoting would ' + + 'leave the blob as its own literal text, so the move is refused: run the backfill until ' + + 'it reports zero blocking rows first.', + statement: + `update "${table}" set "${column}" = ("${column}"::json #>> '{}') ` + + `where "${column}" is not null and left("${column}", 1) = '"'`, + }; +} + +/** + * A media column this step declines to plan, and why. ⛔ Never dropped + * silently: a column missing from a move that then reports success is the one + * way the step can under-deliver and still look complete. + */ +export interface MediaColumnMoveRefusal { + table: string; + column: string; + reason: 'dialect_not_supported' | 'introspection_failed' | 'column_absent'; + detail: string; +} + +/** What `planMediaColumnMove()` found: every plan, and every refusal. */ +export interface MediaColumnMoveScan { + /** The dialect as the driver names it — including one this step cannot serve. */ + dialect: SqlDialectName; + plans: MediaColumnMovePlan[]; + refusals: MediaColumnMoveRefusal[]; +} + +/** + * What the operator does if it goes wrong — printed by the command. + * + * The first note is the load-bearing one, and it is the same shape as the one + * `os migrate multi-value-columns` carries: the move is not information + * preserving in the reverse direction, so there is no `--undo` and pretending + * otherwise would be worse than saying so. + */ +export const MEDIA_COLUMN_MOVE_ROLLBACK_NOTES: readonly string[] = [ + 'Restore the backup you took before the run. The move rewrites values in place; a type-only reversal puts the column back to json with BARE ids in it, which is not valid JSON and reads back as nothing on a server dialect.', + 'The pre-check runs before any statement, and a non-zero count stops the whole step with the column and its rows untouched. A refusal here is the mechanism working: convert the rows it names, then re-run.', + 'PostgreSQL runs one statement per column, and a statement there is atomic: if it fails, that column is exactly as it was. Columns already moved in the same run stay moved — re-running skips them, because the move only touches cells that still carry the legacy encoding.', + 'SQLite runs one UPDATE per column and it is idempotent: an already-bare cell is not valid JSON, so a re-run passes over it rather than converting it twice.', + 'The deployment is only recorded as moved (`sys_migration.columns_moved_at`) when every column succeeded. A partial run records nothing, so the driver stays on the JSON arm and keeps reading both encodings.', +]; diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 2c270a30be..080039ca89 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -108,6 +108,16 @@ import { type PhysicalColumn, type PendingSchemaWork, } from './schema-drift.js'; +// [#15989] The ADR-0104 column step's per-dialect statements. Built here, run +// by `os migrate files-to-references --apply` — see {@link SqlDriver.planMediaColumnMove}. +import { + isJsonColumnType, + mediaColumnMoveDialect, + mediaColumnMovePlan, + type MediaColumnMovePlan, + type MediaColumnMoveRefusal, + type MediaColumnMoveScan, +} from './media-column-move.js'; import { undeliveredStorageAttributes, formatAttribute, @@ -11642,6 +11652,113 @@ export class SqlDriver implements IDataDriver { return out; } + /** + * Every single-value media column on this datastore that still holds the + * legacy encoding, with the statements that move it (#15989). + * + * The read-only half of the ADR-0104 column step: it enumerates and plans, + * and it runs no DDL and no UPDATE. `os migrate files-to-references --apply` + * is what executes the plans, and only after its backfill and self-check + * have reported zero blocking rows — this method deliberately cannot tell + * whether that happened, so it must never be the thing that decides to move + * anything. + * + * ## The shape of each move is read off the COLUMN, never off the dialect + * + * A media column can already be a string column on a server dialect — `os + * generate migration --format sql` emits `VARCHAR(2048)` for the family, and + * a JSON-arm driver writes quoted ids into it (#15771, reproduced on live + * PostgreSQL 16.13). Deciding `retype` vs `unquote` from the dialect alone + * would leave that population full of `"file_…"` behind a `columns_moved_at` + * stamp claiming it had been converted. So each target is classified by + * introspection. + * + * ## What is left OUT, and why each omission is not a silent one + * + * - a table the datastore does not physically have — nothing to move; + * - `multiple: true` media — a list of ids, a JSON column on every + * deployment and on both arms, and not part of this move at all (see + * {@link mediaFields}, which already excludes it); + * - a column the introspection cannot see — reported as a `refusals` entry + * rather than skipped, because a column silently missing from a move is + * the one way this step can under-report and still look complete; + * - every dialect outside {@link MEDIA_COLUMN_MOVE_DIALECTS} — one refusal + * naming the dialect. ⛔ MySQL lands here on purpose: #17788 owns its + * statement order, on a real instance. + */ + async planMediaColumnMove(): Promise { + const dialect = mediaColumnMoveDialect(this.dialectName); + if (!dialect) { + return { + dialect: this.dialectName, + plans: [], + refusals: [ + { + table: '*', + column: '*', + reason: 'dialect_not_supported', + detail: + `the ADR-0104 column step has no measured statement for dialect '${this.dialectName}'. ` + + 'MySQL is tracked by #17788, where its statement ORDER is settled against a real ' + + 'instance; no other dialect has been rehearsed. Nothing was planned and nothing ran.', + }, + ], + }; + } + + const plans: MediaColumnMovePlan[] = []; + const refusals: MediaColumnMoveRefusal[] = []; + + for (const [tableName] of this.managedObjectFields) { + const columns = this.mediaFields[tableName]; + if (!columns || columns.length === 0) continue; + if (!(await this.knex.schema.hasTable(tableName))) continue; + + let physical: IntrospectedColumn[]; + try { + physical = await this.introspectColumns(tableName); + } catch (e: any) { + refusals.push({ + table: tableName, + column: '*', + reason: 'introspection_failed', + detail: + `could not read the physical columns of '${tableName}' (${e?.message ?? e}), so the ` + + 'shape of its media columns is unknown and no statement can be chosen for them.', + }); + continue; + } + const byName = new Map(physical.map((c) => [c.name, c])); + + for (const column of columns) { + const found = byName.get(column); + if (!found) { + refusals.push({ + table: tableName, + column, + reason: 'column_absent', + detail: + `'${tableName}.${column}' is declared as a media field but the datastore has no such ` + + 'column, so there is nothing here to move and nothing that could be verified moved.', + }); + continue; + } + plans.push( + mediaColumnMovePlan( + dialect, + isJsonColumnType(found.type) ? 'retype' : 'unquote', + tableName, + column, + ), + ); + } + } + + plans.sort((a, b) => (a.table === b.table ? a.column.localeCompare(b.column) : a.table.localeCompare(b.table))); + refusals.sort((a, b) => (a.table === b.table ? a.column.localeCompare(b.column) : a.table.localeCompare(b.table))); + return { dialect, plans, refusals }; + } + /** * Boot-time per-table drift handling (P1 + P2): detect divergence, in dev * auto-reconcile the *safe* (loosening) subset when `autoMigrate==='safe'`, From 983294f0e93e300e5047f3289e98ea66f24ccd87 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:07:42 +0000 Subject: [PATCH 3/9] wip: the column step inside `os migrate files-to-references --apply` Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../src/commands/migrate/file-column-move.ts | 239 ++++++++++++++++++ .../commands/migrate/files-to-references.ts | 232 ++++++++++++++++- packages/cli/src/utils/schema-migrate.ts | 16 +- 3 files changed, 484 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/commands/migrate/file-column-move.ts diff --git a/packages/cli/src/commands/migrate/file-column-move.ts b/packages/cli/src/commands/migrate/file-column-move.ts new file mode 100644 index 0000000000..adc2390167 --- /dev/null +++ b/packages/cli/src/commands/migrate/file-column-move.ts @@ -0,0 +1,239 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The ADR-0104 file-family COLUMN step, as a further step of + * `os migrate files-to-references --apply` — the ruling on #15041, step 2. + * + * The backfill converts the VALUES; this converts the COLUMNS and the encoding + * of what they hold, and then records `sys_migration.columns_moved_at` so the + * driver flips its write arm. Those two acts are ONE act on purpose: measured + * on SQLite, after the columns are converted a JSON-arm driver still *reads* + * the migrated column correctly but its next *write* re-quotes, so a column + * move with no arm flip is a storage format that is half migrated in a way + * nothing detects. + * + * ## Three gates, in this order, and the step does nothing until all three pass + * + * 1. **The migration's own gate.** The caller only reaches this after + * backfill + verify reported zero blocking rows — the ruling's "abort + * otherwise", enforced by the caller because only it knows the verdict. + * 2. **Every pre-check, before any statement.** `runFileColumnMove` runs ALL + * the pre-checks first and executes nothing at all unless every one of + * them answered zero. This is stricter than the ruling asks and + * deliberately so: a step that moved three columns and then aborted on the + * fourth leaves a datastore in a state no flag can describe. + * 3. **No refusals.** A column the driver could not plan (an unsupported + * dialect, a failed introspection, a declared column the datastore does + * not have) stops the step. ⛔ Moving the columns it *could* see and + * stamping the deployment as moved would certify the ones it could not. + * + * ## ⛔ The statements are the DRIVER's, imported at the point of use + * + * `@objectstack/driver-sql` owns the dialects and MEASURED these clauses, and + * the one thing this step must never do is carry its own copy: the copy that + * matters here is the abort pre-check, which exists precisely because the + * ruling's own prose carried a statement that does not abort. And the import + * is lazy because oclif `import()`s every command module while building its + * command table, so one static value import of a driver package costs every + * other command its place in that table when the driver is not built (#5726, + * pinned by `schema-migrate.lazy-driver-import.test.ts`). + */ + +import type { MediaColumnMovePlan, MediaColumnMoveScan } from '@objectstack/driver-sql'; + +/** The raw-SQL seam shape — the same signature `resolveSeedTenancyExec` returns. */ +export type RawExec = (sql: string, params?: unknown[]) => Promise; + +/** Normalizer for whatever shape a dialect's driver hands back from a raw read. */ +export type RowNormalizer = (result: unknown) => Array>; + +/** One column's outcome. */ +export interface FileColumnMoveOutcome { + table: string; + column: string; + kind: MediaColumnMovePlan['kind']; + /** Cells the pre-check found that the move would not preserve. */ + blocking: number; + /** `null` until the pre-check has actually answered. */ + statement: string; + status: 'planned' | 'blocked' | 'moved' | 'failed' | 'not_attempted'; + error?: string; +} + +export interface FileColumnMoveResult { + /** Did the caller ask for writes? A dry run reads the pre-checks and writes nothing. */ + apply: boolean; + outcomes: FileColumnMoveOutcome[]; + refusals: MediaColumnMoveScan['refusals']; + /** Statements actually sent to the database. ALWAYS `[]` on a dry run. */ + executedStatements: string[]; + /** + * Total cells every pre-check found blocking. Non-zero ⇒ nothing ran and + * nothing may be stamped. + */ + blocking: number; + /** + * May the caller record `columns_moved_at`? True only when writes were + * asked for, there were no refusals, no pre-check blocked, and every planned + * column reports `moved`. ⛔ An empty plan does NOT earn a stamp — see + * {@link runFileColumnMove}. + */ + recordable: boolean; +} + +/** Read one pre-check's count out of whatever the seam handed back. */ +function countOf(rows: Array>): number { + const first = rows[0]; + if (!first) return Number.NaN; + // `count(*) as n` comes back as `n` on every dialect this step serves, but + // the case and the JS type vary (Postgres hands back a string for bigint). + const raw = first.n ?? first.N ?? Object.values(first)[0]; + const parsed = typeof raw === 'number' ? raw : Number(raw); + return Number.isFinite(parsed) ? parsed : Number.NaN; +} + +/** + * Run the column step. + * + * ## What a dry run does, and why it is not "executes nothing" + * + * `os migrate multi-value-columns` holds its `exec` seam completely unused on + * a dry run, and says so. This step cannot borrow that contract, because the + * one thing an operator needs to know before a column move is whether it would + * ABORT — and that answer is a `SELECT`. So a dry run here runs every + * pre-check and no statement: reads happen, writes do not, which is the + * contract `os migrate files-to-references` already keeps for its own scan. + * The distinction is in the report rather than implied: `executedStatements` + * is `[]` on a dry run, always. + * + * ## ⛔ An empty plan is not a completed move + * + * A datastore with no media columns at all, and a driver whose dialect this + * step cannot serve, both produce zero plans. Stamping the second as moved + * would be a certificate over columns nobody looked at, so `recordable` is + * false whenever there is nothing to move — a deployment with no media columns + * has nothing for the bare arm to change, and leaving it unstamped costs it + * nothing. + */ +export async function runFileColumnMove(args: { + scan: MediaColumnMoveScan; + exec: RawExec; + rows: RowNormalizer; + apply: boolean; + onStatement?: (statement: string) => void; +}): Promise { + const { scan, exec, rows, apply } = args; + const outcomes: FileColumnMoveOutcome[] = scan.plans.map((plan) => ({ + table: plan.table, + column: plan.column, + kind: plan.kind, + blocking: 0, + statement: plan.statement, + status: 'planned', + })); + const executedStatements: string[] = []; + + // ── Phase 1 · every pre-check, before any statement ────────────────────── + let blocking = 0; + let precheckFailed = false; + for (let i = 0; i < scan.plans.length; i++) { + const plan = scan.plans[i]!; + const outcome = outcomes[i]!; + try { + const n = countOf(rows(await exec(plan.precheck))); + if (!Number.isFinite(n)) { + // A pre-check that answered nothing readable is NOT a zero. Treated as + // a failure, because the alternative is moving a column on the + // strength of a reading that never happened. + outcome.status = 'failed'; + outcome.error = + `the abort pre-check for ${plan.table}.${plan.column} returned no readable count, so ` + + 'whether this column holds a cell the move would destroy is unknown. Nothing was run.'; + precheckFailed = true; + continue; + } + outcome.blocking = n; + if (n > 0) { + outcome.status = 'blocked'; + outcome.error = `${n} ${plan.precheckMeaning}`; + blocking += n; + } + } catch (error: unknown) { + outcome.status = 'failed'; + outcome.error = error instanceof Error ? error.message : String(error); + precheckFailed = true; + } + } + + const stopped = blocking > 0 || precheckFailed || scan.refusals.length > 0; + + if (!apply || stopped) { + for (const outcome of outcomes) { + if (outcome.status === 'planned' && stopped) outcome.status = 'not_attempted'; + } + return { + apply, + outcomes, + refusals: scan.refusals, + executedStatements, + blocking, + recordable: false, + }; + } + + // ── Phase 2 · the statements, in plan order ────────────────────────────── + let failed = false; + for (let i = 0; i < scan.plans.length; i++) { + const plan = scan.plans[i]!; + const outcome = outcomes[i]!; + if (failed) { + outcome.status = 'not_attempted'; + continue; + } + try { + args.onStatement?.(plan.statement); + await exec(plan.statement); + executedStatements.push(plan.statement); + outcome.status = 'moved'; + } catch (error: unknown) { + outcome.status = 'failed'; + outcome.error = error instanceof Error ? error.message : String(error); + failed = true; + } + } + + return { + apply, + outcomes, + refusals: scan.refusals, + executedStatements, + blocking, + // ⛔ `every` over an EMPTY array is `true`, which is exactly the certificate + // over nothing this step must not issue — so the length is asked first. + recordable: outcomes.length > 0 && outcomes.every((o) => o.status === 'moved'), + }; +} + +/** + * The one sentence an operator reads when the step refused to move anything. + * Kept beside the runner so the report and the reason cannot drift apart. + */ +export function describeFileColumnMoveRefusal(result: FileColumnMoveResult): string | null { + if (result.refusals.length > 0) { + return ( + `The column step did not run: ${result.refusals.length} media column(s) could not be planned. ` + + 'Every declared media column has to be plannable before any of them moves — moving the ones ' + + 'that could be seen would record this deployment as migrated on behalf of the ones that ' + + 'could not.' + ); + } + if (result.blocking > 0) { + return ( + `The column step ABORTED before running any statement: ${result.blocking} cell(s) still hold a ` + + 'value the move would not preserve. This is the gate working — the superseded form of this ' + + 'migration accepted those rows and flattened them to literal text. Convert the rows named ' + + 'above and re-run.' + ); + } + return null; +} diff --git a/packages/cli/src/commands/migrate/files-to-references.ts b/packages/cli/src/commands/migrate/files-to-references.ts index 26fd337e9a..178315f300 100644 --- a/packages/cli/src/commands/migrate/files-to-references.ts +++ b/packages/cli/src/commands/migrate/files-to-references.ts @@ -19,6 +19,42 @@ import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; +import { + describeFileColumnMoveRefusal, + runFileColumnMove, + type FileColumnMoveResult, +} from './file-column-move.js'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +import type { SqlDriverLike } from '../../utils/schema-migrate.js'; +import type { MediaColumnMoveScan, SqlDialectName } from '@objectstack/driver-sql'; + +/** + * What {@link MigrateFilesToReferences.runColumnStep} did, or declined to do. + * + * `skipped` and `failed` are deliberately separate: every skip is a stated, + * non-failing reason (this command's subject is the backfill), and only a + * column step that ran and could not finish fails the command — because that + * is the one outcome that leaves storage an operator has to be told about. + */ +interface ColumnStepOutcome { + skipped: 'gate_not_passed' | 'no_sql_driver' | 'no_sql_seam' | 'nothing_to_move' | null; + failed: boolean; + /** `sys_migration.columns_moved_at` as written, or `null` if it was not written. */ + stampedAt: string | null; + /** Set when the columns moved and RECORDING that failed — a durability failure. */ + stampError?: string; + report: { + dialect: SqlDialectName; + apply: boolean; + blocking: number; + outcomes: FileColumnMoveResult['outcomes']; + refusals: MediaColumnMoveScan['refusals']; + executedStatements: string[]; + recordable: boolean; + /** Carried from the driver, because the renderer cannot `await import`. */ + rollbackNotes: readonly string[]; + } | null; +} async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes @@ -221,6 +257,26 @@ export default class MigrateFilesToReferences extends Command { includeUnreferenced: flags['include-unreferenced'], }); + // ── The COLUMN step (#15989, the ruling on #15041 step 2) ──────────── + // + // Runs only after the backfill and its self-check reported zero blocking + // rows — the ruling's own "abort otherwise", and the reason it lives + // here rather than in a command of its own: the gate's verdict is what + // authorises it, and this is the only place that verdict exists. + // + // ⛔ The move and the arm flip are ONE act. Measured on SQLite: after + // the columns are converted a JSON-arm driver still READS the migrated + // column correctly but its next WRITE re-quotes. So `columns_moved_at` + // is stamped in the same block that moved the columns, and only when + // every one of them moved. + const columnMove = await this.runColumnStep({ + stack, + engine, + apply, + gatePassed: result.gatePassed, + json: flags.json, + }); + if (flags.json) { await emitJson({ database: stack.dbLabel, @@ -250,9 +306,11 @@ export default class MigrateFilesToReferences extends Command { gatePassed: result.gatePassed, gateFailures: result.gateFailures, flag: result.flag, + columnMove: columnMove.report, + columnsMovedAt: columnMove.stampedAt, duration: timer.elapsed(), }); - if (!result.gatePassed) this.exit(1); + if (!result.gatePassed || columnMove.failed) this.exit(1); return; } @@ -291,9 +349,11 @@ export default class MigrateFilesToReferences extends Command { : 'Fix the records listed above, then re-run (and finally with --apply).', ); } + this.renderColumnStep(columnMove); + console.log(chalk.dim(` ${timer.display()}`)); console.log(''); - if (!result.gatePassed) this.exit(1); + if (!result.gatePassed || columnMove.failed) this.exit(1); } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } @@ -303,4 +363,172 @@ export default class MigrateFilesToReferences extends Command { await stack.shutdown(); } } + + /** + * The column step — plan, pre-check, move, stamp (#15989). + * + * Every early return is a NON-failure with a stated reason: this command's + * subject is the backfill, and a deployment whose driver cannot plan a + * column move is not a deployment whose backfill failed. The one thing that + * fails the command is a column step that was asked to run, ran, and could + * not finish — because that leaves storage the operator must be told about. + */ + private async runColumnStep(args: { + stack: { driver: SqlDriverLike | null; kernel: unknown }; + engine: unknown; + apply: boolean; + gatePassed: boolean; + json: boolean; + }): Promise { + const { stack, apply, gatePassed, json } = args; + + if (!gatePassed) { + // ⛔ The ruling's "abort unless backfill + verify report zero blocking". + // Not an error of this step's own — the gate already reported why. + return { skipped: 'gate_not_passed', failed: false, stampedAt: null, report: null }; + } + if (!stack.driver || typeof stack.driver.planMediaColumnMove !== 'function') { + return { skipped: 'no_sql_driver', failed: false, stampedAt: null, report: null }; + } + + const scan = await stack.driver.planMediaColumnMove(); + if (scan.plans.length === 0 && scan.refusals.length === 0) { + return { skipped: 'nothing_to_move', failed: false, stampedAt: null, report: null }; + } + + // Lazily, at the point of use — ⛔ never a static value import of a driver + // package in a command module (#5726). + const { MEDIA_COLUMN_MOVE_ROLLBACK_NOTES } = await import('@objectstack/driver-sql'); + const { resolveSeedTenancyExec, normalizeRows } = await import('@objectstack/metadata-protocol'); + const exec = resolveSeedTenancyExec(args.engine as IObjectQLEngine | undefined); + // Loud absence, never a silent success. A driver can expose an `execute` + // that accepts every statement and performs none (#10677) — and "moved 3 + // columns" from a seam that ran nothing, followed by a `columns_moved_at` + // stamp, is the worst report this command could produce: the driver would + // then write bare ids into columns that never moved. + const answers = exec + ? await exec('select 1 as os_seam_probe') + .then((r) => normalizeRows(r).length > 0) + .catch(() => false) + : false; + if (!exec || !answers) { + return { skipped: 'no_sql_seam', failed: false, stampedAt: null, report: null }; + } + + const run = await runFileColumnMove({ + scan, + exec, + rows: normalizeRows, + apply, + onStatement: json ? undefined : (statement: string) => printStep(chalk.dim(statement)), + }); + + let stampedAt: string | null = null; + let stampError: string | undefined; + if (run.recordable) { + try { + const { recordFileColumnMove } = await import('@objectstack/platform-objects/system'); + const { FILE_REFERENCES_MIGRATION_ID } = await import('@objectstack/spec/system'); + stampedAt = await recordFileColumnMove(args.engine as any, FILE_REFERENCES_MIGRATION_ID); + } catch (error: any) { + // The columns MOVED and the ledger does not say so. That is a + // durability degradation in the sense AGENTS.md names: the next boot + // stays on the JSON arm and re-quotes its writes into a column that + // has already been converted. It must fail the command. + stampError = error?.message ?? String(error); + } + } + + const failed = + run.outcomes.some((o) => o.status === 'failed') || stampError !== undefined; + + return { + skipped: null, + failed, + stampedAt, + stampError, + report: { + dialect: scan.dialect, + rollbackNotes: MEDIA_COLUMN_MOVE_ROLLBACK_NOTES, + apply: run.apply, + blocking: run.blocking, + outcomes: run.outcomes, + refusals: run.refusals, + executedStatements: run.executedStatements, + recordable: run.recordable, + }, + }; + } + + /** The human-mode half of {@link runColumnStep}. JSON mode reports the same facts. */ + private renderColumnStep(outcome: ColumnStepOutcome): void { + if (outcome.skipped === 'gate_not_passed' || outcome.report === null) { + if (outcome.skipped === 'no_sql_driver') { + printInfo( + 'Column step: not applicable — the ADR-0104 file-family column move is a SQL-driver step ' + + 'and no SQL driver is active here.', + ); + } else if (outcome.skipped === 'no_sql_seam') { + printWarning( + 'Column step: SKIPPED — the active driver exposes no usable raw SQL seam, so the media ' + + 'columns were neither inspected nor moved. The deployment stays on the JSON encoding.', + ); + } else if (outcome.skipped === 'nothing_to_move') { + printInfo('Column step: nothing to move — this datastore declares no single-value media column.'); + } + return; + } + + const report = outcome.report; + console.log(''); + console.log(chalk.bold(`Column step · ${report.dialect}`)); + for (const o of report.outcomes) { + const mark = + o.status === 'moved' ? chalk.green('✓') + : o.status === 'blocked' || o.status === 'failed' ? chalk.red('✗') + : chalk.yellow('•'); + console.log(`${mark} ${chalk.bold(`${o.table}.${o.column}`)} ${chalk.dim(`(${o.kind})`)}`); + console.log(` ${chalk.cyan(o.statement)}`); + if (o.error) console.log(` ${chalk.red(o.error)}`); + } + for (const refusal of report.refusals) { + printWarning(`${refusal.table}.${refusal.column}: ${refusal.detail}`); + } + + const refusal = describeFileColumnMoveRefusal({ + apply: report.apply, + outcomes: report.outcomes, + refusals: report.refusals, + executedStatements: report.executedStatements, + blocking: report.blocking, + recordable: report.recordable, + }); + console.log(''); + if (refusal) { + printError(refusal); + } else if (!report.apply) { + printInfo( + `Dry run — every abort pre-check passed and nothing was executed. ${report.outcomes.length} ` + + 'column(s) would move. Take a backup, then re-run with --apply.', + ); + } else if (outcome.stampError) { + printError( + `The columns MOVED but recording it failed (${outcome.stampError}). This deployment's ` + + 'driver will stay on the JSON encoding and re-quote its next write into a column that ' + + 'has already been converted — re-run this command to record it.', + ); + } else if (outcome.stampedAt) { + printSuccess( + `Column step complete — ${report.outcomes.length} media column(s) moved to the bare-id ` + + `encoding and recorded (sys_migration.columns_moved_at = ${outcome.stampedAt}). The SQL ` + + 'driver writes bare ids from its next boot, and keeps reading the legacy encoding.', + ); + } + + if (refusal || report.outcomes.some((o) => o.status === 'failed')) { + console.log(''); + console.log(chalk.bold('If it goes wrong:')); + for (const note of report.rollbackNotes) console.log(` ${chalk.dim('·')} ${note}`); + } + } } diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index 803d12ea5d..88f1ad9944 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -18,7 +18,12 @@ * stack alone — run `os build` first so its objects are visible. */ import chalk from 'chalk'; -import type { ManagedDriftEntry, DriftCategory, PendingSchemaWork } from '@objectstack/driver-sql'; +import type { + ManagedDriftEntry, + DriftCategory, + MediaColumnMoveScan, + PendingSchemaWork, +} from '@objectstack/driver-sql'; import type { IObjectQLEngine } from '@objectstack/spec/contracts'; import { describeDriverConnection } from './connection-display.js'; import { reserveStdoutForJson } from './json-stdout.js'; @@ -36,6 +41,15 @@ export interface SqlDriverLike { entries: ManagedDriftEntry[], opts: { allowDestructive?: boolean }, ): Promise<{ applied: ManagedDriftEntry[]; skipped: ManagedDriftEntry[] }>; + /** + * The ADR-0104 file-family column step's read-only planner (#15989) — + * optional, so a driver with no media arm (every driver that is not this + * repo's SQL one, and an older published build of it) still boots and simply + * offers no plan. ⛔ Its absence must read as "cannot plan", never as + * "nothing to move": the two are the same shape from here, and only the + * caller's own refusal branch can tell an operator which it was. + */ + planMediaColumnMove?: () => Promise; /** Deferred-DDL surface (#3917) — optional, so a driver without it still boots. */ setDeferredDdl?: (deferred: boolean) => void; previewDeferredSchemaWork?: () => Promise; From 58ee7d82b90c8730a251ca72166e396f1252ef85 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:15:38 +0000 Subject: [PATCH 4/9] wip: per-dialect column-move pins incl. the standing destructive ablation Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../src/media-column-move.pin.test.ts | 162 ++++++++ .../sql-driver-15989-file-column-move.test.ts | 369 ++++++++++++++++++ 2 files changed, 531 insertions(+) create mode 100644 packages/drivers/driver-sql/src/media-column-move.pin.test.ts create mode 100644 packages/drivers/driver-sql/src/sql-driver-15989-file-column-move.test.ts diff --git a/packages/drivers/driver-sql/src/media-column-move.pin.test.ts b/packages/drivers/driver-sql/src/media-column-move.pin.test.ts new file mode 100644 index 0000000000..bc5c696187 --- /dev/null +++ b/packages/drivers/driver-sql/src/media-column-move.pin.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The ADR-0104 column step's STATEMENTS, pinned without a server. + * + * The live behaviour is `sql-driver-15989-file-column-move.test.ts`, which + * executes these against every provisioned dialect. This file pins what they + * SAY — the part a reader of the ruling has to be able to check by eye, and the + * part that a careless edit could change while every live cell still passes + * because the fixture happens not to contain the row the clause stopped + * refusing. + * + * ⛔ The single most load-bearing assertion here is that the PostgreSQL retype + * arm's pre-check exists at all. The #15041 addendum prescribed the retype with + * NO pre-check, and that form was measured on live PostgreSQL 16.13 to accept a + * row holding an inline metadata blob and flatten it to its own literal text. + * The director ruling (decision batch #120 item 1) replaced the clause; a pin + * that only checked the `ALTER … USING` half would pass on the superseded form. + */ + +import { describe, it, expect } from 'vitest'; +import { + MEDIA_COLUMN_MOVE_DIALECTS, + MEDIA_COLUMN_MOVE_ROLLBACK_NOTES, + MEDIA_ID_MOVE_WIDTH, + isJsonColumnType, + mediaColumnMoveDialect, + mediaColumnMovePlan, +} from './media-column-move.js'; + +describe('#15989 — which dialects the column step serves', () => { + it('serves PostgreSQL and SQLite, and ⛔ refuses MySQL by NAME', () => { + expect([...MEDIA_COLUMN_MOVE_DIALECTS]).toEqual(['postgres', 'sqlite']); + expect(mediaColumnMoveDialect('postgres')).toBe('postgres'); + expect(mediaColumnMoveDialect('sqlite')).toBe('sqlite'); + // ⛔ #17788 owns the MySQL leg, on a real instance, because the addendum + // leaves its statement ORDER unsettled. A `null` here is the refusal; a + // fallback to another dialect's statements is what it prevents. + expect(mediaColumnMoveDialect('mysql')).toBeNull(); + expect(mediaColumnMoveDialect('unknown' as never)).toBeNull(); + }); +}); + +describe('#15989 — the PostgreSQL RETYPE arm (a json column)', () => { + const plan = mediaColumnMovePlan('postgres', 'retype', 'crm_case', 'cover'); + + it('⛔ carries the measured ABORT pre-check, not the ruling text\'s bare statement', () => { + // The exact clause recorded in 5556979386 / 5618311630 and adopted by the + // director ruling. `#>> '{}'` extracts ANY json type as text, so without + // this the statement below converts an unconverted blob instead of + // refusing it — measured, on live PostgreSQL 16.13. + expect(plan.precheck).toContain("json_typeof(\"cover\"::json) is distinct from 'string'"); + expect(plan.precheck).toContain('count(*)'); + expect(plan.precheck).toContain('"crm_case"'); + // The pre-check must not itself change anything. + expect(plan.precheck.toLowerCase()).not.toMatch(/\b(update|alter|delete|insert)\b/); + }); + + it('keeps the ruling\'s own retype statement, at the generator\'s width', () => { + expect(plan.statement).toBe( + 'alter table "crm_case" alter column "cover" ' + + `type varchar(${MEDIA_ID_MOVE_WIDTH}) using ("cover" #>> '{}')`, + ); + // The width is the SQL generator's, which the ruling names as the + // end-state and which `varcharColumnChars` mirrors on the moved arm. + expect(MEDIA_ID_MOVE_WIDTH).toBe(2048); + }); + + it('states what a non-zero pre-check MEANS, in the operator\'s terms', () => { + // Anti-vacuity for the report: a count with no sentence beside it is a + // number an operator cannot act on, and this step's whole value on a + // blocked deployment is the sentence. + expect(plan.precheckMeaning).toMatch(/not a string/i); + expect(plan.precheckMeaning.length).toBeGreaterThan(40); + }); +}); + +describe('#15989 — the PostgreSQL UNQUOTE arm (a column already varchar)', () => { + const plan = mediaColumnMovePlan('postgres', 'unquote', 'crm_case', 'cover'); + + it('⛔ does NOT retype — this population is #15771\'s, already at the target type', () => { + // `os generate migration --format sql` emits VARCHAR(2048) for the family, + // and a JSON-arm driver fills it with quoted ids (reproduced on live PG + // 16.13). Retyping it would be a no-op that reports as a move; the values + // are what have to change. + expect(plan.statement.toLowerCase()).not.toContain('alter table'); + expect(plan.statement).toContain('update "crm_case"'); + expect(plan.statement).toContain(`"cover"::json #>> '{}'`); + }); + + it('only touches a cell that OPENS with a double quote, so a bare id is left alone', () => { + expect(plan.statement).toContain(`left("cover", 1) = '"'`); + }); + + it('aborts on the same family the retype arm does — an unconverted blob', () => { + expect(plan.precheck).toContain(`left("cover", 1) in ('{', '[')`); + expect(plan.precheck.toLowerCase()).not.toMatch(/\b(update|alter|delete|insert)\b/); + }); +}); + +describe('#15989 — the SQLite arm', () => { + const plan = mediaColumnMovePlan('sqlite', 'unquote', 'crm_case', 'cover'); + + it('gates on json_valid AND json_type, which is what makes a re-run idempotent', () => { + // MEASURED (the first step-3 rehearsal, 5556979386): a bare id is not + // valid JSON at all, so `json_valid` excludes it and a re-run passes over + // it. A literal "not a JSON string" test would abort on every + // already-converted row instead. + expect(plan.statement).toContain(`json_extract("cover", '$')`); + expect(plan.statement).toContain('json_valid("cover")'); + expect(plan.statement).toContain(`json_type("cover") = 'text'`); + }); + + it("aborts on a json value whose type is not 'text'", () => { + expect(plan.precheck).toContain(`json_type("cover") <> 'text'`); + expect(plan.precheck).toContain('json_valid("cover")'); + }); + + it('⛔ ignores the `kind` for its SQL — SQLite has no json type to retype', () => { + // Both spellings must produce the identical STATEMENTS: SQLite's column is + // `text` on both arms, so a caller that classified it either way is right + // about what has to happen. The `kind` itself is carried through unchanged + // rather than normalised, so a report says what the planner actually saw — + // asserted here so neither half can quietly become the other. + const asRetype = mediaColumnMovePlan('sqlite', 'retype', 'crm_case', 'cover'); + expect(asRetype.statement).toBe(plan.statement); + expect(asRetype.precheck).toBe(plan.precheck); + expect(asRetype.kind).toBe('retype'); + expect(plan.kind).toBe('unquote'); + // …and the control that this is a SQLite property and not a general one: + // on Postgres the two kinds really do differ. + expect(mediaColumnMovePlan('postgres', 'retype', 'crm_case', 'cover').statement).not.toBe( + mediaColumnMovePlan('postgres', 'unquote', 'crm_case', 'cover').statement, + ); + }); +}); + +describe('#15989 — the shape is read off the COLUMN, never off the dialect', () => { + it('recognises every spelling a server uses for a json column', () => { + for (const type of ['json', 'jsonb', 'JSON', 'JSONB']) { + expect(isJsonColumnType(type), type).toBe(true); + } + // The control, in the same run: the target type and the generator's own + // must NOT read as json, or every moved column would be planned as a + // retype forever. + for (const type of ['character varying', 'varchar', 'text', 'TEXT', undefined, null, '']) { + expect(isJsonColumnType(type), String(type)).toBe(false); + } + }); +}); + +describe('#15989 — the rollback notes', () => { + it('say there is no reverse statement, and that a refusal is the mechanism working', () => { + const all = MEDIA_COLUMN_MOVE_ROLLBACK_NOTES.join('\n'); + expect(all).toMatch(/backup/i); + // ⛔ The step must never advertise an `--undo` it does not have. + expect(all).toMatch(/pre-check runs before any statement/i); + // …and that a partial run records nothing, which is what keeps a + // half-moved datastore off the bare arm. + expect(all).toMatch(/columns_moved_at/); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-15989-file-column-move.test.ts b/packages/drivers/driver-sql/src/sql-driver-15989-file-column-move.test.ts new file mode 100644 index 0000000000..78f8d7331c --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-15989-file-column-move.test.ts @@ -0,0 +1,369 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The ADR-0104 file-family COLUMN step, executed — per dialect, for + * BOTH encodings, across the window. The ruling on #15041 step 2, and the + * director ruling on this card (decision batch #120 item 1). + * + * `sql-driver-15989-file-family-bare-id.test.ts` pins what the two ARMS store + * and read. This file pins the act that takes a deployment from one arm to the + * other: the statements run against a real server, the pre-check that stops + * them, and the reverse verification that the moved column serves the driver + * afterwards. + * + * ## §1 is the measurement this card exists for + * + * The #15041 addendum prescribed `ALTER … USING (col #>> '{}')` with nothing in + * front of it, and required the step to abort *"on the first cell that is not a + * JSON string"*. Those two sentences contradict each other, and which one was + * wrong was settled by running it: `#>> '{}'` extracts ANY json type as text, so + * on live PostgreSQL 16.13 the statement was ACCEPTED over a row holding an + * inline metadata blob and flattened that object to its own literal text in a + * `varchar` column. §1 reproduces the destructive form beside the guarded one, + * in the same run, on the same fixture — ⛔ it is a standing ablation, not a + * historical note, because a future edit that drops the pre-check would + * otherwise leave every other case in this file green. + * + * ## Dialect coverage, stated rather than implied + * + * SQLite always runs. Postgres runs when `OS_TEST_POSTGRES_URL` is provisioned + * and is a NAMED SKIP otherwise. ⛔ MySQL is deliberately NOT a cell here: the + * column step has no MySQL statements at all (#17788 owns them, and owns them + * on a real instance because the addendum leaves the statement ORDER + * unsettled), so a MySQL cell would be a pin over a refusal. §5 asserts that + * refusal instead, with no server. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { DriverOptions } from '@objectstack/spec/data'; +import type { DriverQuery } from '@objectstack/spec/contracts'; +import { SqlDriver } from './sql-driver.js'; +import { + mediaColumnMoveDialect, + mediaColumnMovePlan, + type MediaColumnMovePlan, +} from './media-column-move.js'; +import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { tmpdir } from 'node:os'; + +// ⛔ Not `as any`: `check:query-options-erasure` counts every erased options +// bag, tests included, and this one is squarely ON contract. +const OPTS: DriverOptions = { bypassTenantAudit: true }; + +/** + * An unconverted inline blob — exactly what the backfill has NOT yet touched. + * + * ⚠️ Deliberately NOT canonical JSON. A blob whose text already equals its own + * re-serialisation makes the SQLite half of §1's ablation VACUOUS: measured, + * `json_extract('{"a":1}','$')` hands back byte-identical text, so the + * unguarded unquote looks harmless on a fixture that was formatted the way + * SQLite would format it. The spacing below is what makes "this row was + * rewritten" observable in bytes rather than only in the column's type. + */ +const BLOB = '{ "url" : "https://x/y.png", "size": 12 }'; + +class MoveProbe extends SqlDriver { + get raw() { + return (this as unknown as { knex: any }).knex; + } + get sqlite(): boolean { + return (this as unknown as { isSqlite: boolean }).isSqlite; + } + get postgres(): boolean { + return (this as unknown as { isPostgres: boolean }).isPostgres; + } +} + +const objectDef = (table: string) => ({ + name: table, + fields: { + cover: { type: 'image' }, + label: { type: 'string' }, + }, +} as any); + +/** The bytes the SERVER holds, read around every client-side json decode. */ +async function storedText(d: MoveProbe, table: string, column: string, id: string): Promise { + if (d.sqlite) { + const row = await d.raw(table).where('id', id).first(); + return row?.[column] == null ? null : String(row[column]); + } + const res: any = await d.raw.raw(`select "${column}"::text as t from "${table}" where id = ?`, [id]); + const rows = Array.isArray(res) ? res[0] : res.rows; + return rows[0]?.t == null ? null : String(rows[0].t); +} + +/** The column's physical type, as the server reports it. */ +async function columnType(d: MoveProbe, table: string, column: string): Promise { + const info: any = await d.raw(table).columnInfo(); + return String(info[column]?.type ?? '').toLowerCase(); +} + +async function count(d: MoveProbe, sql: string): Promise { + const res: any = await d.raw.raw(sql); + const rows = Array.isArray(res) ? (Array.isArray(res[0]) ? res[0] : res) : (res?.rows ?? []); + const raw = rows[0]?.n ?? rows[0]?.N ?? Object.values(rows[0] ?? {})[0]; + return Number(raw); +} + +/** Write a cell around the driver, so the bytes on disk are the fixture's. */ +async function writeRaw(d: MoveProbe, table: string, id: string, patch: Record) { + await d.raw(table).where('id', id).update(patch); +} + +function measure(cell: DialectCell): void { + // One table per section: these run inside one file, and a shared table would + // let a section's DDL decide another's starting state. + const T_ABLATION = 'os15989m_ablation'; + const T_GUARD = 'os15989m_guard'; + const T_MOVE = 'os15989m_move'; + const T_UNQUOTE = 'os15989m_unquote'; + + /** + * The cell's config, made shareable between TWO driver instances. + * + * §3 is the reverse verification, and it needs the two arms of the window to + * be two drivers over ONE database — which is the window's own shape, since + * the arm is a per-deployment fact rather than a build. The live cells + * already are (a real server, one per-file schema), but the SQLite cell is + * `:memory:`, where a second driver opens a second, EMPTY database. ⛔ That + * failure is silent in exactly the direction that matters: the second driver + * creates its own table, its own writes read back perfectly, and the rows the + * first driver migrated are simply absent — a green §3 that measured nothing + * about the move. A file-backed database is the smallest change that makes + * the two drivers share bytes. + */ + const SQLITE_FILE = join( + mkdtempSync(join(tmpdir(), 'os15989-colmove-')), + 'shared.sqlite', + ); + const sharedConfig = () => { + const base = cell.config() as Record; + if (base.client !== 'better-sqlite3') return base as ReturnType; + return { ...base, connection: { filename: SQLITE_FILE } } as ReturnType; + }; + + describe(`#15989 — the column step, executed (${cell.label})`, () => { + let unmoved: MoveProbe; + let dialect: 'postgres' | 'sqlite'; + + beforeAll(async () => { + unmoved = new MoveProbe(sharedConfig()); + await unmoved.initObjects([ + objectDef(T_ABLATION), + objectDef(T_GUARD), + objectDef(T_MOVE), + objectDef(T_UNQUOTE), + ]); + const named = mediaColumnMoveDialect(unmoved.dialectName); + expect(named, `${cell.label} must be a dialect this step serves`).not.toBeNull(); + dialect = named as 'postgres' | 'sqlite'; + }); + + afterAll(async () => { + await unmoved?.disconnect().catch(() => {}); + rmSync(dirname(SQLITE_FILE), { recursive: true, force: true }); + }); + + /** The plan for `table.cover`, classified the way `planMediaColumnMove` classifies. */ + async function planFor(table: string): Promise { + const physical = await columnType(unmoved, table, 'cover'); + return mediaColumnMovePlan(dialect, /json/.test(physical) ? 'retype' : 'unquote', table, 'cover'); + } + + // ── §1 the ablation: the superseded clause beside this PR's ───────────── + + it('§1 the SUPERSEDED clause rewrites an unconverted row; THIS clause refuses to run at all', async () => { + // The fixture the ruling was measured on: one converted cell, one the + // backfill has not reached. + await unmoved.create(T_ABLATION, { id: 'converted', cover: 'file_01CONVERTED', label: 'a' }, OPTS); + await unmoved.create(T_ABLATION, { id: 'unconverted', cover: 'file_placeholder', label: 'a' }, OPTS); + await writeRaw(unmoved, T_ABLATION, 'unconverted', { cover: BLOB }); + + const before = await storedText(unmoved, T_ABLATION, 'cover', 'unconverted'); + expect(before).toBe(BLOB); + + const plan = await planFor(T_ABLATION); + const typeBefore = await columnType(unmoved, T_ABLATION, 'cover'); + + // ── the guarded form: the pre-check answers NON-ZERO and nothing runs. + const blocking = await count(unmoved, plan.precheck); + expect(blocking, 'the abort pre-check must SEE the unconverted row').toBeGreaterThan(0); + + // Not running the statement is the whole behaviour, so what is asserted + // is that the bytes and the column are untouched after the refusal. + expect(await storedText(unmoved, T_ABLATION, 'cover', 'unconverted')).toBe(before); + // The converted cell is still in its legacy encoding too — the refusal is + // total, not per row. + expect(await storedText(unmoved, T_ABLATION, 'cover', 'converted')).toBe('"file_01CONVERTED"'); + + // ── the superseded form: run it, and watch the unconverted row change. + // ⛔ This is the ablation. It is executed, on the same fixture, so the + // difference between the two clauses is a measurement in this run rather + // than a claim in a comment. + const destructive = + plan.kind === 'retype' + ? `alter table "${T_ABLATION}" alter column "cover" type varchar(2048) using ("cover" #>> '{}')` + : dialect === 'sqlite' + ? `update "${T_ABLATION}" set "cover" = json_extract("cover", '$') where "cover" is not null and json_valid("cover")` + : `update "${T_ABLATION}" set "cover" = ("cover"::json #>> '{}') where "cover" is not null`; + await unmoved.raw.raw(destructive); + + const after = await storedText(unmoved, T_ABLATION, 'cover', 'unconverted'); + const typeAfter = await columnType(unmoved, T_ABLATION, 'cover'); + + // The statement was ACCEPTED — no throw, no warning — over exactly the + // row the ruling requires it to refuse. That much is common to both + // dialects, and it is the whole defect: a successful-looking migration. + expect(after).not.toBeNull(); + + // ⚠️ WHAT is lost differs by dialect, and the two are stated separately + // rather than averaged into one claim, because averaging them is how a + // pin ends up asserting the weaker of the two. + if (dialect === 'postgres') { + // MEASURED, and the reason this defect is invisible to a byte diff: + // Postgres's `json` type stores the input text verbatim and + // `#>> '{}'` hands that same text back, so the BYTES survive. What + // does not survive is the TYPE — the column is no longer json, so an + // object that was a JSON object is now a plain string sitting in a + // column whose declared contents are bare `sys_file` ids. Every + // consumer that read it as an object now reads a string, and nothing + // in the database records that it ever happened. + expect(typeBefore).toContain('json'); + expect(typeAfter).not.toContain('json'); + expect(after).toContain('"url"'); + expect(after, 'Postgres preserves the bytes — the loss is the type').toBe(before); + } else { + // SQLite has no type to lose, so the loss IS in the bytes: the blob is + // re-serialised by `json_extract` and whatever formatting the row + // carried is gone. The row was rewritten, which is what the + // requirement forbids, and the column stays `text` throughout. + expect(before).toContain(' '); + expect(after).not.toContain(' '); + expect(after, 'SQLite rewrites the bytes').not.toBe(before); + } + + // …and the converted row came out as the bare id, which is why the + // superseded form looks like it worked. + expect(await storedText(unmoved, T_ABLATION, 'cover', 'converted')).toBe('file_01CONVERTED'); + }); + + // ── §2 the CONTROL: the same pre-check passes on a clean column ───────── + + it('§2 CONTROL — with no unconverted cell the same pre-check answers zero and the move runs', async () => { + await unmoved.create(T_GUARD, { id: 'a', cover: 'file_01A', label: 'a' }, OPTS); + await unmoved.create(T_GUARD, { id: 'b', cover: 'file_01B', label: 'a' }, OPTS); + await unmoved.create(T_GUARD, { id: 'nullcell', label: 'a' }, OPTS); + + const plan = await planFor(T_GUARD); + // ⛔ Without this the §1 reading is void: a pre-check that is always + // non-zero would produce §1's abort for the wrong reason. + expect(await count(unmoved, plan.precheck)).toBe(0); + + await unmoved.raw.raw(plan.statement); + + expect(await storedText(unmoved, T_GUARD, 'cover', 'a')).toBe('file_01A'); + expect(await storedText(unmoved, T_GUARD, 'cover', 'b')).toBe('file_01B'); + // A NULL must survive as a NULL — the failure mode `manualJsonConversionSql` + // records for its own dialect arm (`json_build_array(NULL)` is `[null]`). + expect(await storedText(unmoved, T_GUARD, 'cover', 'nullcell')).toBeNull(); + expect(await columnType(unmoved, T_GUARD, 'cover')).not.toContain('json'); + }); + + it('§2b the move is IDEMPOTENT — a re-run changes nothing and still refuses nothing', async () => { + const plan = await planFor(T_GUARD); + expect(await count(unmoved, plan.precheck)).toBe(0); + await unmoved.raw.raw(plan.statement); + expect(await storedText(unmoved, T_GUARD, 'cover', 'a')).toBe('file_01A'); + expect(await storedText(unmoved, T_GUARD, 'cover', 'nullcell')).toBeNull(); + }); + + // ── §3 the reverse verification the card names ───────────────────────── + + it('§3 after the move, a MOVED driver writes the bare id and reads it back unchanged', async () => { + // Move the column while the deployment is still on the JSON arm — which + // is the real order: the operator runs the migration, and the arm flips + // on the next boot because the ledger now says so. + await unmoved.create(T_MOVE, { id: 'pre', cover: 'file_01PRE', label: 'a' }, OPTS); + const plan = await planFor(T_MOVE); + expect(await count(unmoved, plan.precheck)).toBe(0); + await unmoved.raw.raw(plan.statement); + expect(await storedText(unmoved, T_MOVE, 'cover', 'pre')).toBe('file_01PRE'); + + // The "next boot": a driver whose deployment has moved. + const moved = new MoveProbe({ ...sharedConfig(), fileColumnsMoved: true }); + try { + await moved.initObjects([objectDef(T_MOVE)]); + + // ⭐ A bare id written under the moved arm reads back UNCHANGED, and + // the bytes on disk are that same id. + await moved.create(T_MOVE, { id: 'post', cover: 'file_01POST', label: 'a' }, OPTS); + expect(await storedText(moved, T_MOVE, 'cover', 'post')).toBe('file_01POST'); + const back: any = await moved.findOne(T_MOVE, { where: { id: 'post' } } as DriverQuery, OPTS); + expect(back?.cover).toBe('file_01POST'); + + // …and the row the move itself converted reads the same way. + const converted: any = await moved.findOne(T_MOVE, { where: { id: 'pre' } } as DriverQuery, OPTS); + expect(converted?.cover).toBe('file_01PRE'); + } finally { + await moved.disconnect().catch(() => {}); + } + }); + + it('§3b ⛔ and the UNMOVED half of the window still reads its legacy encoding', async () => { + // The other side of "both encodings across the window": a deployment + // that has NOT run the column step reads the JSON-quoted id it stored. + await unmoved.create(T_UNQUOTE, { id: 'legacy', cover: 'file_01LEGACY', label: 'a' }, OPTS); + const stored = await storedText(unmoved, T_UNQUOTE, 'cover', 'legacy'); + expect(stored, 'an unmoved deployment still stores the JSON-quoted id').toBe('"file_01LEGACY"'); + const row: any = await unmoved.findOne(T_UNQUOTE, { where: { id: 'legacy' } } as DriverQuery, OPTS); + expect(row?.cover).toBe('file_01LEGACY'); + }); + + // ── §4 the planner sees the real columns ─────────────────────────────── + + it('§4 planMediaColumnMove names every single-value media column, and nothing else', async () => { + const scan = await unmoved.planMediaColumnMove(); + expect(scan.refusals, JSON.stringify(scan.refusals)).toEqual([]); + const named = scan.plans.map((p) => `${p.table}.${p.column}`); + for (const table of [T_ABLATION, T_GUARD, T_MOVE, T_UNQUOTE]) { + expect(named, table).toContain(`${table}.cover`); + } + // The control, in the same reading: a plain string field is NOT a media + // column, so a planner that swept every column would be caught here. + expect(named.some((n) => n.endsWith('.label'))).toBe(false); + expect(named.some((n) => n.endsWith('.id'))).toBe(false); + + // …and the kind is read off the physical column, not off the dialect: + // the tables §2/§3 already moved are now `unquote`, the untouched ones + // are whatever this dialect's JSON arm built. + const moveKind = scan.plans.find((p) => p.table === T_MOVE)?.kind; + expect(moveKind).toBe('unquote'); + }); + }); +} + +for (const cell of DIALECT_CELLS) { + // ⛔ MySQL has no statements here — see §5 and #17788. + if (cell.label.toLowerCase().includes('mysql')) continue; + declareDialectCell(cell, 'file-family column move (#15989)', measure); +} + +// ── §5 the refusal, with no server ──────────────────────────────────────── + +describe('#15989 §5 — MySQL is refused by NAME, not attempted', () => { + it('the step names no MySQL statement at all', () => { + // ⛔ #17788 (`pm:on-hold`, `Restart-when: a MySQL 8.x instance is reachable + // from the dispatch environment`) owns the MySQL leg, and owns it on a real + // instance because the addendum leaves its statement ORDER unsettled. This + // is what stops a later edit from "completing the matrix" by transcribing a + // MySQL form nobody has run — the same move that produced the Postgres + // clause this card had to overturn. + expect(mediaColumnMoveDialect('mysql')).toBeNull(); + // The control, in the same assertion: the two dialects that ARE served. + expect(mediaColumnMoveDialect('postgres')).toBe('postgres'); + expect(mediaColumnMoveDialect('sqlite')).toBe('sqlite'); + }); +}); From 991999ae3172263557160c9d50b427ffd12a037f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:19:47 +0000 Subject: [PATCH 5/9] wip: seam, ledger and orchestration pins for the column step Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .../commands/migrate/file-column-move.test.ts | 197 ++++++++++++ ...er-15989-file-columns-moved-supply.test.ts | 173 +++++++++++ .../adr0104-file-columns-moved-supply.test.ts | 294 ++++++++++++++++++ .../system/migration-flag.column-move.test.ts | 189 +++++++++++ 4 files changed, 853 insertions(+) create mode 100644 packages/cli/src/commands/migrate/file-column-move.test.ts create mode 100644 packages/drivers/driver-sql/src/sql-driver-15989-file-columns-moved-supply.test.ts create mode 100644 packages/objectql/src/adr0104-file-columns-moved-supply.test.ts create mode 100644 packages/platform-objects/src/system/migration-flag.column-move.test.ts diff --git a/packages/cli/src/commands/migrate/file-column-move.test.ts b/packages/cli/src/commands/migrate/file-column-move.test.ts new file mode 100644 index 0000000000..455a8ddef8 --- /dev/null +++ b/packages/cli/src/commands/migrate/file-column-move.test.ts @@ -0,0 +1,197 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] `runFileColumnMove` — the orchestration half of the ADR-0104 column + * step, and the three properties an operator's data depends on: + * + * 1. ⛔ **NO statement runs until EVERY pre-check has passed.** A step that + * moved three columns and then aborted on the fourth leaves a datastore in + * a state no flag can describe. + * 2. ⛔ **A dry run executes reads and only reads.** The contract is different + * from `os migrate multi-value-columns`' ("the seam is never called") and + * the difference is deliberate: the one thing an operator needs to know + * before a column move is whether it would abort, and that answer is a + * `SELECT`. The report says which statements ran, and on a dry run that + * list is empty. + * 3. ⛔ **An empty plan earns no stamp.** `[].every(…)` is `true`, which is + * exactly the certificate over nothing this step must never issue. + * + * The statements themselves are `@objectstack/driver-sql`'s and are pinned + * there, live, against every dialect this step serves. + */ + +import { describe, it, expect } from 'vitest'; +import type { MediaColumnMovePlan, MediaColumnMoveScan } from '@objectstack/driver-sql'; +import { describeFileColumnMoveRefusal, runFileColumnMove } from './file-column-move.js'; + +const plan = (table: string, column: string, kind: MediaColumnMovePlan['kind'] = 'retype'): MediaColumnMovePlan => ({ + dialect: 'postgres', + kind, + table, + column, + precheck: `PRECHECK ${table}.${column}`, + precheckMeaning: 'cell(s) hold a JSON value that is not a string.', + statement: `MOVE ${table}.${column}`, +}); + +const scanOf = (plans: MediaColumnMovePlan[], refusals: MediaColumnMoveScan['refusals'] = []): MediaColumnMoveScan => ({ + dialect: 'postgres', + plans, + refusals, +}); + +/** + * A seam that records every statement it is handed, and answers each + * pre-check with the count `blocking` names for it. + */ +function seam(blocking: Record = {}, opts: { throwOn?: string } = {}) { + const sent: string[] = []; + const exec = async (sql: string): Promise => { + sent.push(sql); + if (opts.throwOn && sql === opts.throwOn) throw new Error(`server refused: ${sql}`); + if (sql.startsWith('PRECHECK ')) return [{ n: blocking[sql.slice('PRECHECK '.length)] ?? 0 }]; + return []; + }; + const rows = (r: unknown) => (Array.isArray(r) ? (r as Array>) : []); + return { exec, rows, sent, moves: () => sent.filter((s) => s.startsWith('MOVE ')) }; +} + +describe('#15989 — phase 1 runs EVERY pre-check before any statement', () => { + it('one blocked column stops the WHOLE step — the others are not attempted', async () => { + const s = seam({ 'b.cover': 2 }); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover'), plan('b', 'cover'), plan('c', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: true, + }); + + expect(s.moves(), '⛔ not a single statement may run').toEqual([]); + expect(result.executedStatements).toEqual([]); + expect(result.blocking).toBe(2); + expect(result.recordable).toBe(false); + expect(result.outcomes.map((o) => o.status)).toEqual(['not_attempted', 'blocked', 'not_attempted']); + // …and every pre-check DID run, so the report names every blocked column + // rather than stopping at the first. + expect(s.sent.filter((x) => x.startsWith('PRECHECK ')).length).toBe(3); + }); + + it('⭐ CONTROL — with every pre-check at zero, every statement runs and the step is recordable', async () => { + const s = seam(); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover'), plan('b', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: true, + }); + expect(s.moves()).toEqual(['MOVE a.cover', 'MOVE b.cover']); + expect(result.recordable).toBe(true); + expect(result.outcomes.every((o) => o.status === 'moved')).toBe(true); + }); + + it('a pre-check that answers NOTHING READABLE is a failure, not a zero', async () => { + // Moving a column on the strength of a reading that never happened is the + // same defect one layer up as the clause this card had to overturn. + const s = seam(); + const badRows = () => [] as Array>; + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover')]), + exec: s.exec, + rows: badRows, + apply: true, + }); + expect(s.moves()).toEqual([]); + expect(result.outcomes[0]!.status).toBe('failed'); + expect(result.recordable).toBe(false); + }); + + it('a pre-check that THROWS stops the step', async () => { + const s = seam({}, { throwOn: 'PRECHECK a.cover' }); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover'), plan('b', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: true, + }); + expect(s.moves()).toEqual([]); + expect(result.recordable).toBe(false); + expect(result.outcomes[0]!.status).toBe('failed'); + }); +}); + +describe('#15989 — a dry run reads, and only reads', () => { + it('runs every pre-check and executes no statement', async () => { + const s = seam(); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover'), plan('b', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: false, + }); + expect(s.sent).toEqual(['PRECHECK a.cover', 'PRECHECK b.cover']); + expect(s.moves()).toEqual([]); + expect(result.executedStatements).toEqual([]); + // ⛔ A dry run can never earn the stamp, however clean. + expect(result.recordable).toBe(false); + expect(result.outcomes.every((o) => o.status === 'planned')).toBe(true); + }); + + it('a dry run over a BLOCKED column reports the abort without touching anything', async () => { + const s = seam({ 'a.cover': 7 }); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: false, + }); + expect(result.blocking).toBe(7); + expect(s.moves()).toEqual([]); + expect(describeFileColumnMoveRefusal(result)).toMatch(/ABORTED before running any statement/); + }); +}); + +describe('#15989 — refusals stop the step, and an empty plan earns nothing', () => { + it('⛔ a column that could not be PLANNED stops the columns that could', async () => { + // Moving what could be seen and stamping the deployment as moved would + // certify the columns nobody looked at. + const s = seam(); + const result = await runFileColumnMove({ + scan: scanOf( + [plan('a', 'cover')], + [{ table: 'b', column: 'cover', reason: 'column_absent', detail: 'no such column' }], + ), + exec: s.exec, + rows: s.rows, + apply: true, + }); + expect(s.moves()).toEqual([]); + expect(result.recordable).toBe(false); + expect(describeFileColumnMoveRefusal(result)).toMatch(/could not be planned/); + }); + + it('⛔ an EMPTY plan is not a completed move — `[].every(…)` is true and must not be believed', async () => { + const s = seam(); + const result = await runFileColumnMove({ scan: scanOf([]), exec: s.exec, rows: s.rows, apply: true }); + expect(result.outcomes).toEqual([]); + expect(result.recordable, 'a certificate over nothing').toBe(false); + expect(describeFileColumnMoveRefusal(result)).toBeNull(); + }); +}); + +describe('#15989 — a statement that fails mid-run', () => { + it('stops the rest and is NOT recordable', async () => { + const s = seam({}, { throwOn: 'MOVE b.cover' }); + const result = await runFileColumnMove({ + scan: scanOf([plan('a', 'cover'), plan('b', 'cover'), plan('c', 'cover')]), + exec: s.exec, + rows: s.rows, + apply: true, + }); + expect(result.outcomes.map((o) => o.status)).toEqual(['moved', 'failed', 'not_attempted']); + expect(s.moves()).toEqual(['MOVE a.cover', 'MOVE b.cover']); + // ⭐ The half-moved datastore stays OFF the bare arm, which is what keeps + // it readable: the driver goes on reading both encodings. + expect(result.recordable).toBe(false); + expect(result.outcomes[1]!.error).toMatch(/server refused/); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-15989-file-columns-moved-supply.test.ts b/packages/drivers/driver-sql/src/sql-driver-15989-file-columns-moved-supply.test.ts new file mode 100644 index 0000000000..26a06788c2 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-15989-file-columns-moved-supply.test.ts @@ -0,0 +1,173 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The kernel→driver supply seam for the ADR-0104 media arm, from the + * DRIVER's side — and the property the whole mechanism rests on: + * + * > ⭐ **Every way of not knowing answers "not moved".** + * + * The engine's half (`ObjectQL.registerDriver` handing over a closure over + * `sys_migration.columns_moved_at`) is pinned in `@objectstack/objectql`. This + * file pins what the driver does with it, because the driver is where the + * consequence lands: a driver that guessed "moved" writes bare ids into a JSON + * column, on a deployment nobody migrated. + * + * ⛔ The reason absence must be the JSON arm is not caution in the abstract. + * Every deployment that exists today is on the JSON encoding, and a host that + * has simply not been threaded this option is indistinguishable from one whose + * deployment has not moved — so those two must give the same answer, and the + * answer has to be the one that is true of every store in the world. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; +import type { SqlDriverConfig } from './sql-driver.js'; + +const SQLITE: SqlDriverConfig = { + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, +} as SqlDriverConfig; + +/** Reads the arm the way every writer and every DDL branch reads it. */ +class ArmProbe extends SqlDriver { + get arm(): boolean { + return (this as unknown as { fileColumnsMoved: boolean }).fileColumnsMoved; + } + askedJson(type = 'image'): boolean { + return this.isJsonField(type, { type }); + } +} + +const open: ArmProbe[] = []; +function driver(config: Partial = {}): ArmProbe { + const d = new ArmProbe({ ...SQLITE, ...config } as SqlDriverConfig); + open.push(d); + return d; +} + +afterEach(async () => { + while (open.length) await open.pop()?.disconnect().catch(() => {}); +}); + +const OBJ = [{ name: 'os15989s_t', fields: { cover: { type: 'image' }, label: { type: 'string' } } }] as any; + +describe('#15989 — every way of NOT KNOWING answers "not moved"', () => { + it('① the option is OMITTED and nothing supplies one', async () => { + const d = driver(); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + expect(d.askedJson()).toBe(true); + }); + + it('② a resolver that THROWS synchronously', async () => { + const d = driver({ fileColumnsMoved: () => { throw new Error('sys_migration is not registered'); } }); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + expect(d.askedJson()).toBe(true); + }); + + it('③ a resolver that REJECTS', async () => { + const d = driver({ fileColumnsMoved: async () => { throw new Error('relation does not exist'); } }); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + }); + + it('④ a resolver that answers something that is not `true`', async () => { + // The ledger's own absent-stamp shape reaches a duck-typed resolver as + // `null`/`undefined` more easily than as `false`, so the driver tests for + // `=== true` rather than for falsiness. + for (const answer of [undefined, null, 0, '', 'true', {}]) { + const d = driver({ fileColumnsMoved: (() => answer) as never }); + await d.initObjects(OBJ); + expect(d.arm, JSON.stringify(answer)).toBe(false); + } + }); + + it('⑤ a resolver that NEVER RUNS — the host never calls initObjects', async () => { + let asked = false; + const d = driver({ fileColumnsMoved: () => { asked = true; return true; } }); + // `registerObjectMetadata` is the `skipSchemaSync` / registration-only + // posture: no schema sync, so no `initObjects`, so nothing resolves. + (d as unknown as { registerObjectMetadata: (o: unknown) => void }).registerObjectMetadata(OBJ); + expect(asked, 'nothing may ask the ledger outside initObjects').toBe(false); + expect(d.arm).toBe(false); + expect(d.askedJson()).toBe(true); + }); + + it('⑥ …and a resolver that answers `true` DOES move the arm — the control', async () => { + // ⛔ Without this every case above passes on a driver whose arm is welded + // shut, and the file measures nothing. + const d = driver({ fileColumnsMoved: async () => true }); + await d.initObjects(OBJ); + expect(d.arm).toBe(true); + expect(d.askedJson()).toBe(false); + }); +}); + +describe('#15989 — setFileColumnsMovedResolver: the engine fills an EMPTY slot only', () => { + it('takes the resolver when the host declared nothing', async () => { + const d = driver(); + expect(d.setFileColumnsMovedResolver(async () => true)).toBe(true); + await d.initObjects(OBJ); + expect(d.arm).toBe(true); + }); + + it('⛔ REFUSES to overrule a host that declared `false`', async () => { + // The dangerous direction: the engine turning a declared "not moved" into + // "moved" writes bare ids into a JSON column. A host that names the option + // is asserting something about its own storage, and the engine does not + // contradict it. + const d = driver({ fileColumnsMoved: false }); + expect(d.setFileColumnsMovedResolver(async () => true)).toBe(false); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + }); + + it('⛔ REFUSES to overrule a host that declared `true` either', async () => { + // Symmetric, and refused for the same reason rather than for the opposite + // one: the host is the more specific authority. Overruling this direction + // would have a driver write JSON into columns already retyped. + const d = driver({ fileColumnsMoved: true }); + expect(d.setFileColumnsMovedResolver(async () => false)).toBe(false); + await d.initObjects(OBJ); + expect(d.arm).toBe(true); + }); + + it('⛔ REFUSES to overrule a resolver the host supplied itself', async () => { + const d = driver({ fileColumnsMoved: async () => true }); + expect(d.setFileColumnsMovedResolver(async () => false)).toBe(false); + await d.initObjects(OBJ); + expect(d.arm).toBe(true); + }); + + it('⛔ REFUSES after the arm has already been asked and frozen', async () => { + // `registerObjectMetadata` freezes `isJsonField`'s answer into `jsonFields` + // for every media column, so a resolver arriving after the first + // `initObjects` is a promise the driver cannot keep: it would change the + // write encoding of a table whose columns were built for the other one. + const d = driver(); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + expect(d.setFileColumnsMovedResolver(async () => true)).toBe(false); + await d.initObjects(OBJ); + expect(d.arm).toBe(false); + }); +}); + +describe('#15989 — the resolver is asked ONCE', () => { + it('a repeat initObjects finds nothing left to ask', async () => { + let calls = 0; + const d = driver({ + fileColumnsMoved: async () => { + calls += 1; + return true; + }, + }); + await d.initObjects(OBJ); + await d.initObjects(OBJ); + await d.initObjects([{ name: 'os15989s_t2', fields: { cover: { type: 'image' } } }] as any); + expect(calls, 'the batched and deferred-DDL paths both call initObjects more than once').toBe(1); + expect(d.arm).toBe(true); + }); +}); diff --git a/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts new file mode 100644 index 0000000000..584df610b5 --- /dev/null +++ b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts @@ -0,0 +1,294 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The kernel→driver supply seam for the ADR-0104 media arm, from the + * ENGINE's side — the ruling on #15041 step 2, as amended by the director + * ruling (decision batch #120 item 1). + * + * The driver accepts `SqlDriverConfig.fileColumnsMoved` and has since PR + * #17403. Nothing supplied it: the column was declared, the driver accepted + * it, and no code anywhere read the one and handed it to the other. That gap + * is what this file closes and what it pins. + * + * ## The one fact the arm may be keyed on + * + * ⛔ NOT the `adr-0104-file-references` flag alone. Every creation-attested + * store since 17.0 — every dogfood boot included — carries that flag AND + * JSON-quoted ids in a JSON column, so keying on it would read every existing + * deployment as migrated and then write bare ids into a JSON column. The + * evidence is `columns_moved_at`, written by the act that moves the columns, + * and it is required IN ADDITION to the flag being verified: the stamp alone + * would be a column move with nothing attesting the values inside. + * + * ## Every way of not knowing answers "not moved" + * + * No `sys_migration` object, no row, an unreadable table, a null or empty + * stamp, an unverified row — all `false`. The driver's own half of that + * property is pinned in `@objectstack/driver-sql` + * (`sql-driver-15989-file-columns-moved-supply.test.ts`); this file pins the + * engine's. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine'; +import { FILE_REFERENCES_MIGRATION_ID } from '@objectstack/spec/system'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +type Store = Map>>; + +function rowsOf(store: Store, object: string): Array> { + let rows = store.get(object); + if (!rows) { + rows = []; + store.set(object, rows); + } + return rows; +} + +/** What the driver was handed, so a test can ask it the question later. */ +interface ArmSink { + /** The resolver `registerDriver` installed, if it installed one. */ + resolver: (() => Promise) | null; + installs: number; +} + +function makeDriver( + store: Store, + opts: { sink?: ArmSink; readThrows?: boolean } = {}, +): IDataDriver { + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => row[k] === v); + }; + const driver: Record = { + name: 'default', + version: '1.0.0', + async connect() {}, + async disconnect() {}, + getSchemaSyncStats: () => ({ created: 0, existing: 2 }), + async find(object: string, ast: any) { + if (opts.readThrows) throw new Error('relation "sys_migration" does not exist'); + return rowsOf(store, object).filter((r) => matches(r, ast?.where)); + }, + async findOne(object: string, ast: any) { + if (opts.readThrows) throw new Error('relation "sys_migration" does not exist'); + return rowsOf(store, object).find((r) => matches(r, ast?.where)) ?? null; + }, + async count(object: string) { return rowsOf(store, object).length; }, + async create(object: string, data: any) { + const row = { ...data }; + rowsOf(store, object).push(row); + return row; + }, + async update(object: string, id: string, data: any) { + const rows = rowsOf(store, object); + const idx = rows.findIndex((r) => r.id === id); + if (idx < 0) return null; + rows[idx] = { ...rows[idx], ...data }; + return rows[idx]; + }, + async delete() { return true; }, + async syncSchema() {}, + async dropTable() {}, + }; + if (opts.sink) { + const sink = opts.sink; + driver.setFileColumnsMovedResolver = (resolve: () => Promise) => { + sink.resolver = resolve; + sink.installs += 1; + return true; + }; + } + return driver as unknown as IDataDriver; +} + +const FLAG_OBJECT = { + name: 'sys_migration', + fields: { + id: { type: 'text' }, + last_run_at: { type: 'datetime' }, + verified_at: { type: 'datetime' }, + applied_at: { type: 'datetime' }, + blocking: { type: 'number' }, + advisory: { type: 'number' }, + details: { type: 'textarea' }, + columns_moved_at: { type: 'datetime' }, + }, +}; + +const TASK = { + name: 'showcase_task', + fields: { id: { type: 'text' }, title: { type: 'text' }, cover: { type: 'image' } }, +}; + +function boot( + store: Store, + opts: { sink?: ArmSink; readThrows?: boolean; withFlagObject?: boolean } = {}, +): ObjectQL { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver(store, opts), true); + engine.registerApp({ + id: 'showcase_pkg', + name: 'Showcase', + objects: opts.withFlagObject === false ? [TASK] : [TASK, FLAG_OBJECT], + } as any); + return engine; +} + +const NOW = '2026-09-12T04:00:00.000Z'; + +/** A flag row, with whatever the case under test wants to vary. */ +function flagRow(over: Record = {}): Record { + return { + id: FILE_REFERENCES_MIGRATION_ID, + last_run_at: NOW, + verified_at: NOW, + applied_at: NOW, + blocking: 0, + advisory: 0, + details: null, + columns_moved_at: null, + ...over, + }; +} + +describe('#15989 — haveFileColumnsMoved(): every way of not knowing answers false', () => { + it('⛔ a VERIFIED flag with NO stamp is "not moved" — the flag alone may never key the arm', async () => { + // The population this rule exists for: every creation-attested store since + // 17.0 holds this exact row AND JSON-quoted ids in a JSON column. + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow()); + expect(await engine.isFileReferencesMigrationVerified()).toBe(true); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('an EMPTY-STRING stamp is not a stamp', async () => { + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: '' })); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('⛔ a stamp on an UNVERIFIED row does not move the arm either', async () => { + // The stamp alone would be a column move with nothing attesting that the + // values inside those columns were ever converted. + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow({ verified_at: null, columns_moved_at: NOW })); + expect(await engine.isFileReferencesMigrationVerified()).toBe(false); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('a stamp on a row with BLOCKING findings does not move the arm', async () => { + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow({ blocking: 3, columns_moved_at: NOW })); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('NO ROW at all', async () => { + const engine = boot(new Map()); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('NO sys_migration OBJECT registered — a kernel without the platform objects', async () => { + const engine = boot(new Map(), { withFlagObject: false }); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('an UNREADABLE table', async () => { + const store: Store = new Map(); + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: NOW })); + const engine = boot(store, { readThrows: true }); + expect(await engine.haveFileColumnsMoved()).toBe(false); + }); + + it('⭐ CONTROL — a verified row WITH a stamp answers true, so the falses above are readings', async () => { + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: NOW })); + expect(await engine.isFileReferencesMigrationVerified()).toBe(true); + expect(await engine.haveFileColumnsMoved()).toBe(true); + }); +}); + +describe('#15989 — the two questions come off ONE read', () => { + it('the verified answer and the moved answer share a memo slot', async () => { + const store: Store = new Map(); + const engine = boot(store); + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: NOW })); + + expect(await engine.haveFileColumnsMoved()).toBe(true); + expect(await engine.isFileReferencesMigrationVerified()).toBe(true); + + // Mutate the store behind the memo. Neither answer may move, because the + // row was read once — two reads could straddle an `--apply` and answer out + // of one another's date. + store.set('sys_migration', []); + expect(await engine.haveFileColumnsMoved()).toBe(true); + expect(await engine.isFileReferencesMigrationVerified()).toBe(true); + + // …and the documented way back, which must drop BOTH. + engine.invalidateDataMigrationFlags(); + expect(await engine.haveFileColumnsMoved()).toBe(false); + expect(await engine.isFileReferencesMigrationVerified()).toBe(false); + }); + + it('an INCONCLUSIVE read is not remembered — it keeps asking', async () => { + // The ledger does not exist yet at the moment a boot first asks; a `false` + // cached from that moment would outlive the table's creation. + const store: Store = new Map(); + const engine = boot(store, { withFlagObject: false }); + expect(await engine.haveFileColumnsMoved()).toBe(false); + // Register the object the second phase of a boot brings in, then stamp. + engine.registerApp({ id: 'p2', name: 'P2', objects: [FLAG_OBJECT] } as any); + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: NOW })); + expect(await engine.haveFileColumnsMoved()).toBe(true); + }); +}); + +describe('#15989 — registerDriver hands the driver the question', () => { + it('installs a resolver on a driver that has the seam, and it answers the ledger', async () => { + const store: Store = new Map(); + const sink: ArmSink = { resolver: null, installs: 0 }; + const engine = boot(store, { sink }); + + expect(sink.installs, 'the seam is offered at registration, not at first use').toBe(1); + expect(typeof sink.resolver).toBe('function'); + + // Nothing is read at install time — the row does not exist yet on a boot + // that is about to create it. + expect(await sink.resolver!()).toBe(false); + + rowsOf(store, 'sys_migration').push(flagRow({ columns_moved_at: NOW })); + engine.invalidateDataMigrationFlags(); + expect(await sink.resolver!()).toBe(true); + }); + + it('⛔ a driver WITHOUT the seam is left alone, and still serves queries', async () => { + // No `sink` ⇒ the fake exposes no `setFileColumnsMovedResolver` at all, + // which is every driver that is not this repo's SQL one (memory, mongodb, + // a third party's). Registration must neither throw nor invent the method, + // and the driver must still WORK — asserted by a real round trip rather + // than by a registry count, which would pass on an engine holding a driver + // it can no longer reach. + const store: Store = new Map(); + const engine = boot(store); + const driverForCheck = makeDriver(store) as unknown as Record; + expect(driverForCheck.setFileColumnsMovedResolver, 'the control: the fake has no seam').toBeUndefined(); + await engine.insert('showcase_task', { id: 'q1', title: 'still works' }); + const rows = await engine.find('showcase_task', { where: { id: 'q1' } } as never); + expect(rows.map((r: Record) => r.id)).toEqual(['q1']); + }); + + it('a driver whose seam THROWS does not break its own registration', () => { + const store: Store = new Map(); + const engine = new ObjectQL(); + const driver = makeDriver(store) as unknown as Record; + driver.setFileColumnsMovedResolver = () => { + throw new Error('this driver refuses the resolver'); + }; + expect(() => engine.registerDriver(driver as unknown as IDataDriver, true)).not.toThrow(); + }); +}); diff --git a/packages/platform-objects/src/system/migration-flag.column-move.test.ts b/packages/platform-objects/src/system/migration-flag.column-move.test.ts new file mode 100644 index 0000000000..a7e2dabc06 --- /dev/null +++ b/packages/platform-objects/src/system/migration-flag.column-move.test.ts @@ -0,0 +1,189 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15989] The `columns_moved_at` half of the `sys_migration` ledger — the + * READ that carries it, the WRITE that stamps it, and the write that must NOT + * disturb it. + * + * `sys-migration.column-move.pin.test.ts` (#16185) pinned the column's + * declaration and said, in its own words, that no read of it was pinned there + * because "the writer and the reader are the blocked driver card's". This is + * that card, and these are those. + */ + +import { describe, it, expect } from 'vitest'; +import { FILE_REFERENCES_MIGRATION_ID, hasMovedFileColumns } from '@objectstack/spec/system'; + +import { + readDataMigrationFlag, + recordDataMigrationRun, + recordFileColumnMove, + type MigrationFlagEngine, +} from './migration-flag.js'; + +const NOW = '2026-09-12T04:00:00.000Z'; + +/** An engine over one in-memory `sys_migration` table. */ +function engineOver(rows: Array>, opts: { registered?: boolean } = {}) { + const updates: Array> = []; + const inserts: Array> = []; + const engine: MigrationFlagEngine = { + getObject: (name: string) => (opts.registered === false ? undefined : { name }), + async find(_object, options) { + const where = (options as { where?: Record }).where ?? {}; + return rows.filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + }, + async insert(_object, data) { + inserts.push(data); + rows.push({ ...data }); + return data; + }, + async update(_object, data) { + updates.push(data); + const row = rows.find((r) => r.id === data.id); + if (row) Object.assign(row, data); + return data; + }, + }; + return { engine, rows, updates, inserts }; +} + +const verifiedRow = (over: Record = {}) => ({ + id: FILE_REFERENCES_MIGRATION_ID, + last_run_at: NOW, + verified_at: NOW, + applied_at: NOW, + blocking: 0, + advisory: 0, + details: null, + deviation_observed_at: null, + deviation_detail: null, + columns_moved_at: null, + ...over, +}); + +describe('#15989 — readDataMigrationFlag carries columns_moved_at', () => { + it('reads the stamp through, so a caller can tell a MOVED deployment from an unmoved one', async () => { + const { engine } = engineOver([verifiedRow({ columns_moved_at: NOW })]); + const flag = await readDataMigrationFlag(engine, FILE_REFERENCES_MIGRATION_ID); + expect(flag?.columns_moved_at).toBe(NOW); + expect(hasMovedFileColumns(flag)).toBe(true); + }); + + it('a row written before the column existed reads as NOT moved', async () => { + // ⛔ The control that matters: this is every row in the world today, and + // the answer has to be the JSON arm. + const legacy = verifiedRow(); + delete (legacy as Record).columns_moved_at; + const { engine } = engineOver([legacy]); + const flag = await readDataMigrationFlag(engine, FILE_REFERENCES_MIGRATION_ID); + expect(flag?.columns_moved_at).toBeNull(); + expect(hasMovedFileColumns(flag)).toBe(false); + }); + + it('an unreadable ledger answers null, which answers "not moved"', async () => { + const { engine } = engineOver([verifiedRow({ columns_moved_at: NOW })], { registered: false }); + expect(await readDataMigrationFlag(engine, FILE_REFERENCES_MIGRATION_ID)).toBeNull(); + expect(hasMovedFileColumns(null)).toBe(false); + }); +}); + +describe('#15989 — recordFileColumnMove', () => { + it('stamps the row and returns what it wrote', async () => { + const { engine, rows, updates } = engineOver([verifiedRow()]); + const at = await recordFileColumnMove(engine, FILE_REFERENCES_MIGRATION_ID); + expect(at).toMatch(/^\d{4}-\d{2}-\d{2}T/); + expect(rows[0]!.columns_moved_at).toBe(at); + expect(updates).toHaveLength(1); + // ⛔ It writes the stamp and nothing else. Re-dating `verified_at` or + // `last_run_at` here would have the column move claim to be a self-check. + expect(Object.keys(updates[0]!).sort()).toEqual(['columns_moved_at', 'id', 'updated_at']); + expect(rows[0]!.verified_at).toBe(NOW); + expect(rows[0]!.last_run_at).toBe(NOW); + }); + + it('⛔ REFUSES when there is no verified row — the gate, not a convenience', async () => { + // Reaching here without a verified row means a path that skipped the + // backfill's own gate. Stamping would certify a column move whose values + // were never shown converted, and the driver would then write bare ids on + // the strength of it. + const { engine, updates } = engineOver([verifiedRow({ verified_at: null })]); + await expect(recordFileColumnMove(engine, FILE_REFERENCES_MIGRATION_ID)).rejects.toThrow( + /no VERIFIED/i, + ); + expect(updates, 'a refusal writes nothing').toHaveLength(0); + }); + + it('⛔ REFUSES when the row reports blocking findings', async () => { + const { engine, updates } = engineOver([verifiedRow({ blocking: 2 })]); + await expect(recordFileColumnMove(engine, FILE_REFERENCES_MIGRATION_ID)).rejects.toThrow(); + expect(updates).toHaveLength(0); + }); + + it('⛔ REFUSES when there is no row at all', async () => { + const { engine, updates } = engineOver([]); + await expect(recordFileColumnMove(engine, FILE_REFERENCES_MIGRATION_ID)).rejects.toThrow(); + expect(updates).toHaveLength(0); + }); +}); + +describe('#15989 — a backfill re-run must not disturb the stamp', () => { + it('recordDataMigrationRun leaves columns_moved_at exactly where it was', async () => { + // The two attest different facts: a re-run of the backfill says nothing + // about the physical columns, so it must neither set the stamp nor clear + // it. A moved deployment that re-verifies its values stays moved. + const { engine, rows, updates } = engineOver([verifiedRow({ columns_moved_at: NOW })]); + await recordDataMigrationRun(engine, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: true, + blocking: 0, + applied: true, + }); + expect(rows[0]!.columns_moved_at).toBe(NOW); + expect( + Object.prototype.hasOwnProperty.call(updates[0]!, 'columns_moved_at'), + 'the key must be ABSENT from the update, not merely equal — see the comment at the write', + ).toBe(false); + }); + + it('…and a FAILING re-run does not clear it either', async () => { + // A failing self-check closes the verification gate (`verified_at: null`), + // which is already enough to put the driver back on the JSON arm through + // `hasMovedFileColumns`. Clearing the stamp as well would lose the record + // that the columns are physically converted, which is still true. + const { engine, rows } = engineOver([verifiedRow({ columns_moved_at: NOW })]); + await recordDataMigrationRun(engine, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: false, + blocking: 4, + applied: true, + }); + expect(rows[0]!.verified_at).toBeNull(); + expect(rows[0]!.columns_moved_at).toBe(NOW); + // …and the composite arbiter answers "not moved" anyway, which is the + // safe direction and the reason clearing it is unnecessary. + const flag = await readDataMigrationFlag(engine, FILE_REFERENCES_MIGRATION_ID); + expect(hasMovedFileColumns(flag)).toBe(false); + }); + + it('a run recorded when the LEDGER READ FAILS still cannot null the stamp', async () => { + // The failure mode that makes omission the only safe spelling: if + // `readDataMigrationFlag` answers null because the read failed rather than + // because the row is absent, a write carrying `columns_moved_at: existing + // ?? null` would demote a moved deployment back onto the JSON arm. + const rows = [verifiedRow({ columns_moved_at: NOW })]; + const { engine } = engineOver(rows); + const failing: MigrationFlagEngine = { + ...engine, + async find() { + throw new Error('ledger unreadable'); + }, + }; + await recordDataMigrationRun(failing, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: true, + blocking: 0, + }); + expect(rows[0]!.columns_moved_at).toBe(NOW); + }); +}); From 91e0e84cc77ae8f70782f73fe55a6300d6a43358 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:25:54 +0000 Subject: [PATCH 6/9] =?UTF-8?q?feat(driver-sql,objectql,cli)!:=20the=20ADR?= =?UTF-8?q?-0104=20file-family=20column=20step=20and=20its=20kernel?= =?UTF-8?q?=E2=86=92driver=20supply?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column step retypes and rewrites media columns on PostgreSQL and SQLite after backfill + verify report zero blocking rows, aborting loudly on any cell the move would not preserve; the kernel→driver seam reads sys_migration.columns_moved_at and arms SqlDriverConfig.fileColumnsMoved. Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .changeset/15989-file-family-column-step.md | 54 +++++++++++++++++++++ packages/spec/api-surface/system.json | 1 + packages/spec/export-origins/system.json | 1 + 3 files changed, 56 insertions(+) create mode 100644 .changeset/15989-file-family-column-step.md diff --git a/.changeset/15989-file-family-column-step.md b/.changeset/15989-file-family-column-step.md new file mode 100644 index 0000000000..ac10abe4b0 --- /dev/null +++ b/.changeset/15989-file-family-column-step.md @@ -0,0 +1,54 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/objectql": minor +"@objectstack/platform-objects": minor +"@objectstack/spec": minor +"@objectstack/cli": minor +--- + +feat(driver-sql,objectql,cli)!: the ADR-0104 file-family column step, and the kernel→driver supply that arms it (#15989) + + + +**BREAKING** on the published storage behaviour of `@objectstack/driver-sql`. A deployment that runs `os migrate files-to-references --apply` now has its media columns **retyped and their values rewritten** into the bare-`sys_file`-id encoding, and its driver writes bare ids from the next boot. This completes the maintainer ruling on #15041 (「15041 应该改为实际 id 保存。选A,其他同意」) whose encoding half shipped in the previous release. + +Shipped as `minor` under the repo's launch-window convention, in which `major` is refused by `check-changeset-no-major` and breaking-ness is carried by this banner plus the ADR-0087 disposition rather than by the level. + +## The column step + +`os migrate files-to-references --apply` gains a further step, run **only after** the backfill and its self-check report zero blocking rows — and it moves nothing at all until three gates pass: + +1. the migration's own gate (zero blocking rows); +2. **every** abort pre-check, across **every** planned column, before a single statement runs; +3. no refusals — a column the driver could not plan stops the columns it could. + +**PostgreSQL** and **SQLite** only. ⛔ MySQL is refused by name and belongs to #17788, where its statement ORDER is settled against a real instance rather than transcribed. + +Per column, the shape is read off the column's **physical type**, not off the dialect: a `json` column is retyped (`ALTER … TYPE varchar(2048) USING (col #>> '{}')`), while a column that is already `varchar` — the population `os generate migration --format sql` creates and a JSON-arm driver fills with quoted ids — has its values unquoted in place. SQLite has only the second shape, since it has no json type. + +### ⛔ The abort clause is NOT the one the ADR sketched + +The #15041 addendum prescribed the retype with nothing in front of it while *requiring* the step to abort "on the first cell that is not a JSON string". Those two sentences contradict each other, and which was wrong was settled by running it. Measured on live PostgreSQL 16.13, `USING (col #>> '{}')` is **accepted** over a row holding an inline metadata blob, because `#>> '{}'` extracts *any* json type as text: the bytes survive, but the column is no longer `json`, so an object becomes a plain string in a column whose declared contents are ids — silently, in a migration that reports success. The director ruling (decision batch #120 item 1) replaced the clause with the pre-check that implements the requirement: `json_typeof(col) IS DISTINCT FROM 'string'` on PostgreSQL, and `json_valid(col) AND json_type(col) <> 'text'` on SQLite, where excluding invalid JSON is what keeps a re-run idempotent over cells a previous run already moved. + +Both the destructive form and the guarded one are executed side by side, on one fixture, in this release's own test suite — so the difference stays a measurement rather than a comment. + +## The kernel→driver supply seam + +`SqlDriverConfig.fileColumnsMoved` shipped last release with nothing supplying it. It is now supplied: `ObjectQL.registerDriver` hands every driver that has the seam a closure over the new `ObjectQL.haveFileColumnsMoved()`, which reads `sys_migration.columns_moved_at` — and requires the `adr-0104-file-references` flag to be verified **as well**, since the stamp alone would attest a column move with nothing attesting the values inside it. + +⭐ **Every way of not knowing still answers "not moved".** The option omitted, a resolver that throws or rejects or answers a non-`true` value, a resolver that never runs because the host never calls `initObjects`, a driver with no such seam, no `sys_migration` object, no row, an unreadable table, a null or empty stamp — all the JSON arm. That is the encoding every deployment in the world is on, and a driver that guessed the other way would write bare ids into a JSON column. + +⛔ **A host that names `fileColumnsMoved` in its own config wins**, in either polarity. The engine only ever fills an empty slot, and never contradicts an explicit composition: overruling a declared `false` is precisely the bare-ids-into-a-JSON-column failure this mechanism exists to prevent. + +## New published surface + +- `@objectstack/spec` — `hasMovedFileColumns(flag)`, the single arbiter of the conjunction above, beside `isDataMigrationFlagVerified` and `authorisesIrreversibleAction`. +- `@objectstack/objectql` — `ObjectQL.haveFileColumnsMoved()`, sharing one memoized read (and one `invalidateDataMigrationFlags()`) with `isFileReferencesMigrationVerified()`, so the two answers can never come out of one another's date. +- `@objectstack/platform-objects` — `recordFileColumnMove(engine, migrationId)`, which refuses to stamp a deployment with no verified flag row. `readDataMigrationFlag` now carries `columns_moved_at`; it previously dropped it, which made a moved deployment indistinguishable from an unmoved one to every caller. +- `@objectstack/driver-sql` — `SqlDriver.setFileColumnsMovedResolver()`, `SqlDriver.planMediaColumnMove()`, and the statement builders `mediaColumnMovePlan` / `mediaColumnMoveDialect` / `isJsonColumnType` with `MEDIA_COLUMN_MOVE_DIALECTS`, `MEDIA_COLUMN_MOVE_ROLLBACK_NOTES` and `MEDIA_ID_MOVE_WIDTH`. The statements live in the package that owns the dialects and measured them; a second copy in the CLI would be a second copy of the clause the ruling got wrong. + +## What does NOT change + +A deployment that does not run `--apply` is byte-for-byte where it was: the column stays `json`, the write still JSON-encodes, and the read still accepts both encodings. A backfill re-run does not set the stamp and — deliberately — cannot clear it either: `recordDataMigrationRun` omits the key rather than writing a preserved value, so a ledger read that FAILS cannot demote a moved deployment back onto the JSON arm. A partial or failed column step records nothing at all, which leaves such a datastore on the arm that reads both encodings. + +`multiple: true` media is untouched on both arms: its value is a list of ids and a JSON column on every deployment. diff --git a/packages/spec/api-surface/system.json b/packages/spec/api-surface/system.json index 53cf97271b..7b2db95d96 100644 --- a/packages/spec/api-surface/system.json +++ b/packages/spec/api-surface/system.json @@ -781,6 +781,7 @@ "emailTemplateForm (const)", "gcsStorageExample (const)", "globalFilterKey (function)", + "hasMovedFileColumns (function)", "hasObservedDeviation (function)", "hasPlatformObjectPrefix (function)", "inProcessServiceMessage (function)", diff --git a/packages/spec/export-origins/system.json b/packages/spec/export-origins/system.json index 0b7ee5c432..22f469371a 100644 --- a/packages/spec/export-origins/system.json +++ b/packages/spec/export-origins/system.json @@ -742,6 +742,7 @@ "emailTemplateForm": "src/system/email-template.form.ts#emailTemplateForm (const)", "gcsStorageExample": "src/system/object-storage.zod.ts#gcsStorageExample (const)", "globalFilterKey": "src/system/i18n-resolver.ts#globalFilterKey (function)", + "hasMovedFileColumns": "src/system/migration.zod.ts#hasMovedFileColumns (function)", "hasObservedDeviation": "src/system/migration.zod.ts#hasObservedDeviation (function)", "hasPlatformObjectPrefix": "src/system/constants/platform-object-names.ts#hasPlatformObjectPrefix (function)", "inProcessServiceMessage": "src/system/core-services.zod.ts#inProcessServiceMessage (function)", From 8c1eb1ab2a1d7855eab84ac9067a87685daaf245 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:52:19 +0000 Subject: [PATCH 7/9] fix(gates): clear four reds my own diff introduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - check:doc-authoring — no tracker id in a runtime string an operator reads; the engine's log literal stays written once rather than duplicated - check:engine-double-contract / check:objectql-double-limit / check:where-matcher — the new fakes route update() through assertEngineUpdateDispatch, hold the caller's limit by presence, and refuse a combinator they do not implement instead of reading it as a field name Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- packages/drivers/driver-sql/src/sql-driver.ts | 9 +++- .../adr0104-file-columns-moved-supply.test.ts | 47 +++++++++++++++---- packages/objectql/src/engine.ts | 23 +++++---- .../system/migration-flag.column-move.test.ts | 30 ++++++++++-- scripts/engine-double-contract.pinned.json | 5 ++ 5 files changed, 90 insertions(+), 24 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 080039ca89..604567e1ea 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -11697,10 +11697,15 @@ export class SqlDriver implements IDataDriver { table: '*', column: '*', reason: 'dialect_not_supported', + // ⛔ No tracker id in the string: this `detail` reaches an operator + // reading a migration report, who has no tracker to resolve one + // against (`check:doc-authoring`). The card that owns the MySQL + // leg is #17788; the reader who can resolve that reads this line. detail: `the ADR-0104 column step has no measured statement for dialect '${this.dialectName}'. ` + - 'MySQL is tracked by #17788, where its statement ORDER is settled against a real ' + - 'instance; no other dialect has been rehearsed. Nothing was planned and nothing ran.', + 'The MySQL leg is tracked separately and is settled against a real instance, because ' + + 'its statement ORDER is what the ADR leaves open; no other dialect has been ' + + 'rehearsed. Nothing was planned and nothing ran.', }, ], }; diff --git a/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts index 584df610b5..e06582c23d 100644 --- a/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts +++ b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts @@ -52,14 +52,31 @@ interface ArmSink { installs: number; } +/** + * Equality only. + * + * ⛔ A combinator read as a field name is the silently-wrong matcher + * `check:where-matcher` exists to stop — it would answer "no rows" for a filter + * it does not implement, which reads exactly like an empty ledger and would + * turn every arm question in this file into a false "not moved". + */ +function matches(row: Record, where: any): boolean { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported combinator ${k}`); + return row[k] === v; + }); +} + +/** The caller's bound, applied AFTER the filter and BY PRESENCE. */ +function bounded(rows: Array>, ast: any): Array> { + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; +} + function makeDriver( store: Store, - opts: { sink?: ArmSink; readThrows?: boolean } = {}, + opts: { sink?: ArmSink } = {}, ): IDataDriver { - const matches = (row: Record, where: any): boolean => { - if (!where || typeof where !== 'object') return true; - return Object.entries(where).every(([k, v]) => row[k] === v); - }; const driver: Record = { name: 'default', version: '1.0.0', @@ -67,11 +84,9 @@ function makeDriver( async disconnect() {}, getSchemaSyncStats: () => ({ created: 0, existing: 2 }), async find(object: string, ast: any) { - if (opts.readThrows) throw new Error('relation "sys_migration" does not exist'); - return rowsOf(store, object).filter((r) => matches(r, ast?.where)); + return bounded(rowsOf(store, object).filter((r) => matches(r, ast?.where)), ast); }, async findOne(object: string, ast: any) { - if (opts.readThrows) throw new Error('relation "sys_migration" does not exist'); return rowsOf(store, object).find((r) => matches(r, ast?.where)) ?? null; }, async count(object: string) { return rowsOf(store, object).length; }, @@ -126,7 +141,21 @@ function boot( opts: { sink?: ArmSink; readThrows?: boolean; withFlagObject?: boolean } = {}, ): ObjectQL { const engine = new ObjectQL(); - engine.registerDriver(makeDriver(store, opts), true); + const driver = makeDriver(store, opts) as unknown as Record; + if (opts.readThrows) { + // The unreadable-ledger case is installed by OVERRIDING the reads on the + // finished double rather than by branching inside it. The branch version + // makes the shared double itself unprobeable — `check:objectql-double-limit` + // runs it and reads the throw as "unjudged", so the bound this double DOES + // hold stops being measurable at all. + driver.find = async () => { + throw new Error('relation "sys_migration" does not exist'); + }; + driver.findOne = async () => { + throw new Error('relation "sys_migration" does not exist'); + }; + } + engine.registerDriver(driver as unknown as IDataDriver, true); engine.registerApp({ id: 'showcase_pkg', name: 'Showcase', diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 62ef3312c9..5408facd91 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -8234,7 +8234,18 @@ export class ObjectQL implements IObjectQLEngine { * short-circuits before any query. */ async isFileReferencesMigrationVerified(): Promise { - return this.readMigrationFlagMemoized( + return (await this.readFileReferencesFlagRow()).verified; + } + + /** + * The one memoized read both file-flag questions come off (#15989). + * + * Written once so the two public readers cannot drift apart in the ARGUMENTS + * they pass — the slot and the migration id decide which row is read, and a + * second copy of them is a second way to read a different row. + */ + private readFileReferencesFlagRow(): Promise { + return this.readMigrationFlagRowMemoized( 'fileReferencesMigrationVerified', FILE_REFERENCES_MIGRATION_ID, '[value-shape] this deployment has verified the file-as-reference migration — ' + @@ -8270,15 +8281,7 @@ export class ObjectQL implements IObjectQLEngine { * one row. `invalidateDataMigrationFlags()` drops both. */ async haveFileColumnsMoved(): Promise { - return ( - await this.readMigrationFlagRowMemoized( - 'fileReferencesMigrationVerified', - FILE_REFERENCES_MIGRATION_ID, - '[value-shape] this deployment has verified the file-as-reference migration — ' + - 'media value shapes are enforced and released field files may be collected ' + - '(ADR-0104 / #3617)', - ) - ).columnsMoved; + return (await this.readFileReferencesFlagRow()).columnsMoved; } /** diff --git a/packages/platform-objects/src/system/migration-flag.column-move.test.ts b/packages/platform-objects/src/system/migration-flag.column-move.test.ts index a7e2dabc06..ed6b718e1f 100644 --- a/packages/platform-objects/src/system/migration-flag.column-move.test.ts +++ b/packages/platform-objects/src/system/migration-flag.column-move.test.ts @@ -12,6 +12,7 @@ */ import { describe, it, expect } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { FILE_REFERENCES_MIGRATION_ID, hasMovedFileColumns } from '@objectstack/spec/system'; import { @@ -23,6 +24,22 @@ import { const NOW = '2026-09-12T04:00:00.000Z'; +/** + * Equality only. + * + * ⛔ A combinator read as a field name is the silently-wrong matcher + * `check:where-matcher` exists to stop: it would answer "no rows" for a filter + * it does not implement, which is indistinguishable from a real empty ledger — + * and an empty ledger is exactly the "not moved" answer every case here is + * trying to tell apart from a real reading. + */ +function matches(row: Record, where: Record): boolean { + return Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake engine: unsupported combinator ${k}`); + return row[k] === v; + }); +} + /** An engine over one in-memory `sys_migration` table. */ function engineOver(rows: Array>, opts: { registered?: boolean } = {}) { const updates: Array> = []; @@ -30,15 +47,22 @@ function engineOver(rows: Array>, opts: { registered?: b const engine: MigrationFlagEngine = { getObject: (name: string) => (opts.registered === false ? undefined : { name }), async find(_object, options) { - const where = (options as { where?: Record }).where ?? {}; - return rows.filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + const o = options as { where?: Record; limit?: number }; + const matched = rows.filter((r) => matches(r, o.where ?? {})); + // The caller's bound, applied AFTER the filter and BY PRESENCE — a + // limit-blind double hides a caller that forgot to page. + return typeof o.limit === 'number' ? matched.slice(0, o.limit) : matched; }, async insert(_object, data) { inserts.push(data); rows.push({ ...data }); return data; }, - async update(_object, data) { + async update(_object, data, options) { + // Both writers here update an existing flag row by the `id` in the + // payload — the shape `ObjectQL.update` routes `by-id`. Asserting it + // binds this double to the engine's verdict instead of re-deciding it. + assertEngineUpdateDispatch(data, options); updates.push(data); const row = rows.find((r) => r.id === data.id); if (row) Object.assign(row, data); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b888922a21..ca74249159 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2186,6 +2186,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/platform-objects/src/system/migration-flag.column-move.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/platform-objects/src/system/migration-flag.test.ts", "verb": "update", From 8e5e7627a449db0764d285f2c578d3427f3fe5b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 15:43:27 +0000 Subject: [PATCH 8/9] fix(cli): the column-step helper lives in src/utils, not under src/commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oclif's command table globs **/*.js under dist/commands, so a module there with no default-exported Command makes every CLI invocation warn `findCommand … not found` on stderr — measured, and it broke `os validate --json` for a consumer that reads stdout and stderr together. Also records the ruling, not the zero-hit reading, as why the kernel→driver wiring is owed: asked identically, `sqliteJournalMode` reads zero too and is no defect. Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .changeset/15989-file-family-column-step.md | 2 +- .../src/commands/migrate/files-to-references.ts | 2 +- .../migrate => utils}/file-column-move.test.ts | 0 .../migrate => utils}/file-column-move.ts | 9 +++++++++ .../src/adr0104-file-columns-moved-supply.test.ts | 15 +++++++++++---- 5 files changed, 22 insertions(+), 6 deletions(-) rename packages/cli/src/{commands/migrate => utils}/file-column-move.test.ts (100%) rename packages/cli/src/{commands/migrate => utils}/file-column-move.ts (94%) diff --git a/.changeset/15989-file-family-column-step.md b/.changeset/15989-file-family-column-step.md index ac10abe4b0..966e0920f4 100644 --- a/.changeset/15989-file-family-column-step.md +++ b/.changeset/15989-file-family-column-step.md @@ -34,7 +34,7 @@ Both the destructive form and the guarded one are executed side by side, on one ## The kernel→driver supply seam -`SqlDriverConfig.fileColumnsMoved` shipped last release with nothing supplying it. It is now supplied: `ObjectQL.registerDriver` hands every driver that has the seam a closure over the new `ObjectQL.haveFileColumnsMoved()`, which reads `sys_migration.columns_moved_at` — and requires the `adr-0104-file-references` flag to be verified **as well**, since the stamp alone would attest a column move with nothing attesting the values inside it. +`SqlDriverConfig.fileColumnsMoved` shipped last release and no host outside the driver supplied it. It is supplied now: `ObjectQL.registerDriver` hands every driver that has the seam a closure over the new `ObjectQL.haveFileColumnsMoved()`, which reads `sys_migration.columns_moved_at` — and requires the `adr-0104-file-references` flag to be verified **as well**, since the stamp alone would attest a column move with nothing attesting the values inside it. ⭐ **Every way of not knowing still answers "not moved".** The option omitted, a resolver that throws or rejects or answers a non-`true` value, a resolver that never runs because the host never calls `initObjects`, a driver with no such seam, no `sys_migration` object, no row, an unreadable table, a null or empty stamp — all the JSON arm. That is the encoding every deployment in the world is on, and a driver that guessed the other way would write bare ids into a JSON column. diff --git a/packages/cli/src/commands/migrate/files-to-references.ts b/packages/cli/src/commands/migrate/files-to-references.ts index 178315f300..5491f63a88 100644 --- a/packages/cli/src/commands/migrate/files-to-references.ts +++ b/packages/cli/src/commands/migrate/files-to-references.ts @@ -23,7 +23,7 @@ import { describeFileColumnMoveRefusal, runFileColumnMove, type FileColumnMoveResult, -} from './file-column-move.js'; +} from '../../utils/file-column-move.js'; import type { IObjectQLEngine } from '@objectstack/spec/contracts'; import type { SqlDriverLike } from '../../utils/schema-migrate.js'; import type { MediaColumnMoveScan, SqlDialectName } from '@objectstack/driver-sql'; diff --git a/packages/cli/src/commands/migrate/file-column-move.test.ts b/packages/cli/src/utils/file-column-move.test.ts similarity index 100% rename from packages/cli/src/commands/migrate/file-column-move.test.ts rename to packages/cli/src/utils/file-column-move.test.ts diff --git a/packages/cli/src/commands/migrate/file-column-move.ts b/packages/cli/src/utils/file-column-move.ts similarity index 94% rename from packages/cli/src/commands/migrate/file-column-move.ts rename to packages/cli/src/utils/file-column-move.ts index adc2390167..e72baf6694 100644 --- a/packages/cli/src/commands/migrate/file-column-move.ts +++ b/packages/cli/src/utils/file-column-move.ts @@ -29,6 +29,15 @@ * * ## ⛔ The statements are the DRIVER's, imported at the point of use * + * ## ⛔ It lives in `src/utils/`, NOT beside the command it serves + * + * oclif's command table is `"glob": "**\/*.js"` under `dist/commands`, so EVERY + * module there is a command. A helper placed beside `files-to-references.ts` + * has no default-exported `Command`, and oclif then emits a `findCommand … not + * found` warning to **stderr on every single CLI invocation** — measured, and + * it is not cosmetic: it broke `os validate --json` for a consumer that reads + * stdout and stderr together, by appending non-JSON after the payload. + * * `@objectstack/driver-sql` owns the dialects and MEASURED these clauses, and * the one thing this step must never do is carry its own copy: the copy that * matters here is the abort pre-check, which exists precisely because the diff --git a/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts index e06582c23d..f5a88f7db1 100644 --- a/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts +++ b/packages/objectql/src/adr0104-file-columns-moved-supply.test.ts @@ -5,10 +5,17 @@ * ENGINE's side — the ruling on #15041 step 2, as amended by the director * ruling (decision batch #120 item 1). * - * The driver accepts `SqlDriverConfig.fileColumnsMoved` and has since PR - * #17403. Nothing supplied it: the column was declared, the driver accepted - * it, and no code anywhere read the one and handed it to the other. That gap - * is what this file closes and what it pins. + * The driver has accepted `SqlDriverConfig.fileColumnsMoved` since PR #17403, + * and until this change nothing outside `driver-sql` supplied it. + * + * ⛔ That absence is NOT what makes the wiring owed, and reading it that way is + * a mistake worth naming here rather than repeating. Asked identically of + * `SqlDriverConfig`'s other own keys — the same corpus, the same path shape, + * the same package boundary — `sqliteJournalMode` reads zero too, and it is a + * perfectly ordinary optional key that no host happens to set. A zero proves + * only that there is no supplier today. What makes THIS one owed is the + * director ruling on the card (decision batch #120 item 2), which puts the + * kernel→driver wiring inside this dispatch rather than after it. * * ## The one fact the arm may be keyed on * From 303d7946e4830f8721851fc565dea41275c117d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 16:12:50 +0000 Subject: [PATCH 9/9] docs(changeset): the ADR-0087 rationale's counts are reproducible from the diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 13 new declarations reaching a package entry, not five, and 3 new public methods on exported classes, not two — each named, so a reader can recount them off the three-dot diff. The disposition, the level and the BREAKING banner are unchanged; the clause the counts support is more true at 13/3. Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM Co-authored-by: Claude --- .changeset/15989-file-family-column-step.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/15989-file-family-column-step.md b/.changeset/15989-file-family-column-step.md index 966e0920f4..22a24c0714 100644 --- a/.changeset/15989-file-family-column-step.md +++ b/.changeset/15989-file-family-column-step.md @@ -8,7 +8,7 @@ feat(driver-sql,objectql,cli)!: the ADR-0104 file-family column step, and the kernel→driver supply that arms it (#15989) - + **BREAKING** on the published storage behaviour of `@objectstack/driver-sql`. A deployment that runs `os migrate files-to-references --apply` now has its media columns **retyped and their values rewritten** into the bare-`sys_file`-id encoding, and its driver writes bare ids from the next boot. This completes the maintainer ruling on #15041 (「15041 应该改为实际 id 保存。选A,其他同意」) whose encoding half shipped in the previous release.