diff --git a/.changeset/24925-plan-names-ownership-ceremony.md b/.changeset/24925-plan-names-ownership-ceremony.md new file mode 100644 index 00000000000..63de7d91fee --- /dev/null +++ b/.changeset/24925-plan-names-ownership-ceremony.md @@ -0,0 +1,15 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate plan` names the organization-ownership ceremony, not `--allow-destructive`, for the `organization_id` drops `os migrate apply` refuses (#24925) + +Clause-②: no + +On a database the organization-ownership ceremony has not verified, such as a 17.x one before the ceremony, `os migrate apply --allow-destructive` refuses every run that would drop an `organization_id` column, and drops nothing. No flag stands that refusal down. `os migrate plan` still listed each of those columns as `"os migrate apply --allow-destructive" to drop it.`, closed with `(add --allow-destructive for drops / tightenings)`, and did not mention the ceremony. + +- **Now.** Each such drop is still listed and counted as destructive drift. Its line names the ceremony as its remedy: `… (orphaned) — a drop the organization-ownership ceremony owns, and it has not verified on this database: the ceremony drops it itself (see below).` +- **The footer.** It offers `--allow-destructive` for nothing while such a drop is listed, because `apply` refuses the whole run. It still offers `os migrate apply` when there is other work that command performs without the flag. Then it names the ceremony's two commands: `os migrate organization-ownership --out PLAN`, then `os migrate organization-ownership --apply --plan-file PLAN --backup-taken`. It also counts the other destructive changes that wait for the ceremony. +- **Unchanged.** On a database whose ceremony has verified, or one with no `organization_id` drop, the plan reads as before, byte for byte, and on the second it reads no marker. The `--json` document, the exit code and `os migrate apply` are unchanged. + +**What to do.** Nothing. On a 17.x database, follow the plan's footer: run the ceremony, then re-run `os migrate plan`. diff --git a/packages/cli/src/commands/migrate/plan.apply-limit.test.ts b/packages/cli/src/commands/migrate/plan.apply-limit.test.ts index aab69f52b18..b8bc9ce7f60 100644 --- a/packages/cli/src/commands/migrate/plan.apply-limit.test.ts +++ b/packages/cli/src/commands/migrate/plan.apply-limit.test.ts @@ -29,10 +29,33 @@ * * `bootSchemaStack` is stubbed, so this runs without a database; the same * cases run against a real `sqld` in `apply.remote-face.integration.test.ts`. + * + * ## #24925 — a drop `apply` refuses: the ceremony's `organization_id` drops + * + * On a 17.x database before the organization-ownership ceremony, the plan + * listed each `organization_id` column 18.x no longer declares with the + * differ's `--allow-destructive` remedy and the same footer, while `os migrate + * apply` refuses the whole `--allow-destructive` run until the ceremony has + * verified (`utils/organization-ownership-drop-gate.ts`, ADR-0131 D10 item 5). + * The plan asks the drop gate's own two functions, through the stack's + * `objectql` service as `apply` does: + * + * 5. Unverified (no marker row, an unverified row, or no `sys_migration` + * table to read): each such drop is listed and counted with the ceremony + * as its remedy, every other entry keeps its message, and the footer offers + * `--allow-destructive` for nothing and names the ceremony's two commands. + * Beside a driver's declared limit, each keeps its own remedy. + * 6. CONTROL: a verified marker, or no `organization_id` drop, gives the plan + * as it always was; with no such drop the marker is not even read. + * 7. The `--json` document is unchanged and reads no marker. + * + * The marker read is the real `readOrganizationOwnershipMarker` over a fake + * engine. */ import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { fileURLToPath } from 'node:url'; +import { ORGANIZATION_OWNERSHIP_MIGRATION_ID } from '@objectstack/spec/system'; const boot = vi.hoisted(() => vi.fn(async (_opts: Record): Promise => { throw new Error('the database was opened'); @@ -135,7 +158,13 @@ async function run(argv: string[]): Promise<{ payload: any; lines: string[]; exi if (cb) cb(); return true; }) as typeof process.stdout.write); - vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as typeof process.stderr.write); + // Its callback runs too, or the command's exit waits out its drain timeout + // (`utils/one-shot-exit.ts`, 2 s) on every case. + vi.spyOn(process.stderr, 'write').mockImplementation(((_chunk: unknown, ...rest: unknown[]) => { + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stderr.write); const capture = (...parts: unknown[]) => { said.push(parts.map(String).join(' ')); }; vi.spyOn(console, 'log').mockImplementation(capture); vi.spyOn(console, 'warn').mockImplementation(capture); @@ -233,3 +262,208 @@ describe('CONTROL: a driver that declares nothing reads exactly as before', () = expect(footer(lines)).toEqual([' Apply with: os migrate apply (add --allow-destructive for drops / tightenings)']); }, CASE_TIMEOUT_MS); }); + +// ── #24925: the organization-ownership ceremony's organization_id drops ────── + +/** An orphaned column as the shared differ reports it, with the message it writes. */ +function orphan(table: string, column: string): Entry { + return { + kind: 'unmapped_column', remoteName: table, table, column, severity: 'warning', category: 'destructive', + op: { type: 'drop_column', table, column }, + message: `${table}.${column}: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it.`, + }; +} + +/** Two `organization_id` drops a 17.x database carries before the ceremony. */ +const ORG_JOB = orphan('sys_job', 'organization_id'); +const ORG_SECRET = orphan('sys_secret', 'organization_id'); +/** A destructive drop the ceremony does not own. */ +const LEGACY = orphan('sys_sso_provider', 'created_by'); +/** A safe entry, which `os migrate apply` performs without --allow-destructive. */ +const INDEX: Entry = { + kind: 'missing_index', remoteName: 'sys_job', table: 'sys_job', severity: 'info', category: 'safe', + op: { type: 'create_index', table: 'sys_job', createIndexName: 'idx_sys_job_created_at' }, + message: `sys_job: metadata declares index 'idx_sys_job_created_at' (created_at) but the database has no such index — run "os migrate apply" to create it.`, +}; + +/** The line each held drop is listed with: the finding, then the ceremony. */ +const heldLine = (e: Entry) => + ` ${e.table}.${e.column}: column exists in the database but not in metadata (orphaned) — a drop the ` + + 'organization-ownership ceremony owns, and it has not verified on this database: the ceremony drops it itself (see below).'; + +const CEREMONY_STEPS = + 'Run "os migrate organization-ownership --out PLAN" to write and review its plan, take a backup, then ' + + '"os migrate organization-ownership --apply --plan-file PLAN --backup-taken"; it drops these columns itself.'; +const ceremonyLine = (drops: number, waiting: number) => + ` ⚠ ${drops} organization_id drop(s) above are the organization-ownership ceremony's, and it has not verified on ` + + 'this database, so "os migrate apply --allow-destructive" refuses every run until it has' + + (waiting > 0 ? ` (the other ${waiting} destructive change(s) above wait for it too)` : '') + + `. ${CEREMONY_STEPS}`; + +const MARKED_AT = '2026-10-20T00:00:00.000Z'; +/** The ceremony's `sys_migration` row as `readDataMigrationFlag` reads it. */ +const VERIFIED = { id: ORGANIZATION_OWNERSHIP_MIGRATION_ID, last_run_at: MARKED_AT, verified_at: MARKED_AT, blocking: 0 }; +const UNVERIFIED = { id: ORGANIZATION_OWNERSHIP_MIGRATION_ID, last_run_at: MARKED_AT, verified_at: null, blocking: 2 }; + +/** + * The `objectql` service as the marker read meets it: `sys_migration` is + * registered, and the row read answers `row` (or throws it, as a read of a + * table the database does not have does). + */ +function engineAnswering(row: Record | null | Error) { + return { + getObject: (name: string) => (name === 'sys_migration' ? { name } : undefined), + findOne: vi.fn(async (object: string, options: { where?: { id?: string } }) => { + expect(object).toBe('sys_migration'); + expect(options.where?.id).toBe(ORGANIZATION_OWNERSHIP_MIGRATION_ID); + if (row instanceof Error) throw row; + return row; + }), + }; +} + +/** A booted local stack whose kernel serves `engine` as `objectql`; `limited` as {@link stackWith}'s. */ +function ceremonyStackWith( + drift: Entry[], + engine: ReturnType, + limited: ReadonlySet = new Set(), +) { + const getService = vi.fn((name: string) => { + if (name === 'objectql') return engine; + throw new Error(`no ${name} service in this pin`); + }); + const { stack } = stackWith(drift, limited); + return { + getService, + stack: { + ...stack, + driver: { config: { url: 'postgres://db.example/app' } }, + dbLabel: 'postgres://db.example/app', + schemaSync: 'deferred', + kernel: { getService }, + }, + }; +} + +/** The line the plan prints under the entry acting on `target`: its finding and its remedy. */ +function lineUnder(lines: string[], target: string): string { + const at = lines.findIndex((l) => l.includes(` ${target} [`)); + expect(at, `no ${target} entry in:\n${lines.join('\n')}`).toBeGreaterThanOrEqual(0); + return lines[at + 1]!; +} + +/** Everything the plan prints below its summary line, but the blank lines and its timer. */ +function footerUnder(lines: string[]): string[] { + const at = lines.findIndex((l) => /\d+ change\(s\): \d+ safe/.test(l)); + expect(at, lines.join('\n')).toBeGreaterThanOrEqual(0); + return lines.slice(at + 1).filter((l) => l.trim() !== '' && !/^\s+\d+ms$/.test(l)); +} + +describe('#24925 — a database the organization-ownership ceremony has not verified', () => { + it('each organization_id drop names the ceremony, never --allow-destructive; the footer offers --allow-destructive for nothing and names both ceremony commands', async () => { + const engine = engineAnswering(null); + const { stack, getService } = ceremonyStackWith([INDEX, ORG_JOB, LEGACY, ORG_SECRET], engine); + boot.mockResolvedValueOnce(stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + + expect(lineUnder(lines, 'sys_job.organization_id')).toBe(heldLine(ORG_JOB)); + expect(lineUnder(lines, 'sys_secret.organization_id')).toBe(heldLine(ORG_SECRET)); + // The ceremony does not own this drop: it keeps the differ's message. + expect(lineUnder(lines, 'sys_sso_provider.created_by')).toBe(` ${LEGACY.message}`); + expect(lineUnder(lines, 'sys_job [idx_sys_job_created_at]')).toBe(` ${INDEX.message}`); + // Still drift: listed and counted, never dropped from the plan. + expect(lines.join('\n')).toContain('4 change(s): 1 safe, 0 needs-confirm, 3 destructive'); + + expect(footerUnder(lines)).toEqual([' Apply with: os migrate apply', ceremonyLine(2, 1)]); + // Asked once, of the service `apply`'s drop gate asks. + expect(getService.mock.calls).toEqual([['objectql']]); + expect(engine.findOne).toHaveBeenCalledTimes(1); + }, CASE_TIMEOUT_MS); + + it('when apply can perform nothing listed before the ceremony, there is no apply line, only the ceremony', async () => { + boot.mockResolvedValueOnce(ceremonyStackWith([ORG_JOB], engineAnswering(null)).stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expect(lineUnder(lines, 'sys_job.organization_id')).toBe(heldLine(ORG_JOB)); + expect(footerUnder(lines)).toEqual([ceremonyLine(1, 0)]); + }, CASE_TIMEOUT_MS); + + it('an unverified marker row holds the drops too', async () => { + boot.mockResolvedValueOnce(ceremonyStackWith([ORG_JOB, LEGACY], engineAnswering(UNVERIFIED)).stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expect(lineUnder(lines, 'sys_job.organization_id')).toBe(heldLine(ORG_JOB)); + expect(footerUnder(lines)).toEqual([ceremonyLine(1, 1)]); + }, CASE_TIMEOUT_MS); + + it('a database with no sys_migration table to read answers "not verified": the read fails, the plan does not', async () => { + const engine = engineAnswering(new Error('SQLITE_ERROR: no such table: sys_migration')); + boot.mockResolvedValueOnce(ceremonyStackWith([ORG_JOB], engine).stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expect(engine.findOne).toHaveBeenCalledTimes(1); + expect(lineUnder(lines, 'sys_job.organization_id')).toBe(heldLine(ORG_JOB)); + expect(footerUnder(lines)).toEqual([ceremonyLine(1, 0)]); + }, CASE_TIMEOUT_MS); + + it('beside a driver\'s declared apply limit, each keeps its own remedy, and the limit\'s count is its own', async () => { + boot.mockResolvedValueOnce( + ceremonyStackWith([ORG_JOB, TIGHTEN, INDEX], engineAnswering(null), new Set(['tighten_not_null'])).stack, + ); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expect(lineUnder(lines, 'sys_job.organization_id')).toBe(heldLine(ORG_JOB)); + expect(entryLine(lines, 'tighten_not_null')).toContain(`"os migrate apply" cannot apply it on this database: ${LIMIT}.`); + expect(entryLine(lines, 'tighten_not_null')).not.toContain('--allow-destructive'); + expect(footerUnder(lines)).toEqual([ + ' Apply with: os migrate apply', + ceremonyLine(1, 0), + ' ⚠ 1 change(s) above cannot be applied on this database by "os migrate apply" (each says why), so they ' + + 'remain drift after it runs.', + ]); + }, CASE_TIMEOUT_MS); + + it('--json: the document is the report, unchanged — each message as the differ wrote it, and no marker is read', async () => { + const { stack, getService } = ceremonyStackWith([INDEX, ORG_JOB, LEGACY], engineAnswering(null)); + boot.mockResolvedValueOnce(stack); + const { payload, exitCode } = await run(['--json']); + expect(exitCode).toBe(0); + expect(Object.keys(payload).sort()).toEqual( + ['changes', 'database', 'databaseSource', 'duration', 'managedTables', 'pending', 'total', 'unmanagedTables'], + ); + expect(payload.changes.map((c: Entry) => c.message)).toEqual([INDEX.message, ORG_JOB.message, LEGACY.message]); + expect(getService).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); +}); + +describe('#24925 CONTROL: the plan as it always was', () => { + /** What the plan prints for `drift` when nothing holds an entry back: each message, and the one footer. */ + function expectTodaysPlan(lines: string[], drift: Entry[]): void { + for (const e of drift) { + const target = e.column ? `${e.table}.${e.column}` : `${e.table} [${String(e.op.createIndexName)}]`; + expect(lineUnder(lines, target)).toBe(` ${e.message}`); + } + expect(lines.join('\n')).not.toContain('organization-ownership'); + expect(footerUnder(lines)).toEqual([' Apply with: os migrate apply (add --allow-destructive for drops / tightenings)']); + } + + it('a verified ceremony: every organization_id drop keeps its message, and the footer is the one it always was', async () => { + const engine = engineAnswering(VERIFIED); + boot.mockResolvedValueOnce(ceremonyStackWith([INDEX, ORG_JOB, LEGACY, ORG_SECRET], engine).stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expectTodaysPlan(lines, [INDEX, ORG_JOB, LEGACY, ORG_SECRET]); + // …and the marker WAS read: this control is not passing for want of a read. + expect(engine.findOne).toHaveBeenCalledTimes(1); + }, CASE_TIMEOUT_MS); + + it('no organization_id drop: the plan is the one it always was, and the marker is never read', async () => { + const { stack, getService } = ceremonyStackWith([INDEX, LEGACY], engineAnswering(null)); + boot.mockResolvedValueOnce(stack); + const { lines, exitCode } = await run([]); + expect(exitCode, lines.join('\n')).toBe(0); + expect(getService).not.toHaveBeenCalled(); + expectTodaysPlan(lines, [INDEX, LEGACY]); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index e3011dcc785..03cb80830b1 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -2,6 +2,7 @@ import { Command, Flags } from '@oclif/core'; import chalk from 'chalk'; +import type { ManagedDriftEntry } from '@objectstack/driver-sql'; import { printHeader, printSuccess, @@ -39,6 +40,7 @@ import { } from '../../utils/unmanaged-tables.js'; import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import { describeDriverSqliteFile } from '../../utils/connection-display.js'; +import { ceremonyOrganizationColumnDrops, readOrganizationOwnershipMarker } from '../../utils/organization-ownership-drop-gate.js'; import { resolveTenancyPosture, collectGlobalUniques, @@ -83,6 +85,18 @@ import { * driver that declares nothing (every database but a Turso REMOTE one) gets * the plan it always got, byte for byte. The `--json` document is the report * and is unchanged. + * + * [#24925, ADR-0131 D10 item 5] Nor a remedy the apply refuses. On a database + * the organization-ownership ceremony has not verified (a 17.x one before the + * ceremony), `os migrate apply --allow-destructive` refuses every run that + * would drop an `organization_id` column, the whole run, and no flag stands + * that down (`utils/organization-ownership-drop-gate.ts`). Each such drop is + * listed and counted as drift with the ceremony as its remedy, and the footer + * offers `--allow-destructive` for nothing while one is listed: it names the + * ceremony's two commands instead. Which drops are held, and whether the + * ceremony verified, are read through the two functions `apply` asks, never a + * second list. A verified database, or one with no such drop, gets the plan it + * always got, byte for byte; the `--json` document is unchanged. */ export default class MigratePlan extends Command { static override description = @@ -405,34 +419,48 @@ export default class MigratePlan extends Command { // but a drop (it runs no SQLite table rebuild). Empty for a driver that // declares nothing, and then everything below reads as it always has. const limits = collectDriftApplyLimits(stack, drift); + // [#24925] The `organization_id` drops `os migrate apply` refuses here + // until the organization-ownership ceremony has verified. Empty on a + // verified database and on one with no such drop, and then everything + // below reads as it always has. + const held = await ceremonyHeldDrops(stack, drift); await renderPendingSchemaWork(pending); - if (limits.size === 0) { + if (limits.size === 0 && held.size === 0) { renderPlan(drift); } else { // Still listed, still counted: it IS drift. Only the remedy changes, // from a command that cannot perform it here to the reason it cannot. - renderPlan(drift, await describeWithApplyLimits(limits)); + const byLimit = limits.size === 0 ? (d: ManagedDriftEntry) => d.message : await describeWithApplyLimits(limits); + renderPlan(drift, held.size === 0 ? byLimit : await describeWithCeremonyHolds(held, byLimit)); } if (pending.length > 0) printInfo(await summarizePendingSchemaWork(pending)); printInfo(summarize(drift)); - if (limits.size === 0) { + if (limits.size === 0 && held.size === 0) { console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') + chalk.dim(' (add --allow-destructive for drops / tightenings)')); } else { // `--allow-destructive` only for a destructive entry this database can // apply; no apply line at all when it can apply nothing listed here. - const appliable = drift.filter((d) => !limits.has(d)); + const appliable = drift.filter((d) => !limits.has(d) && !held.has(d)); const appliableDestructive = appliable.filter((d) => d.category === 'destructive').length; - if (appliable.length > 0 || pending.length > 0) { - console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') + (appliableDestructive > 0 + // [#24925] While the ceremony holds a drop, `apply` refuses every + // --allow-destructive run, so it is offered for nothing: what `apply` + // performs now is the rest, and the destructive changes wait. + const appliableNow = held.size === 0 ? appliable : appliable.filter((d) => d.category !== 'destructive'); + if (appliableNow.length > 0 || pending.length > 0) { + console.log(chalk.dim(' Apply with: ') + chalk.white('os migrate apply') + (held.size === 0 && appliableDestructive > 0 ? chalk.dim(` (add --allow-destructive for the ${appliableDestructive} destructive change(s) it can apply on this database)`) : '')); } - printWarning( - `${limits.size} change(s) above cannot be applied on this database by "os migrate apply" (each says why), ` - + 'so they remain drift after it runs.', - ); + if (held.size > 0) printWarning(describeCeremonyHold(held.size, appliableDestructive)); + const limited = [...limits.keys()].filter((d) => !held.has(d)).length; + if (limited > 0) { + printWarning( + `${limited} change(s) above cannot be applied on this database by "os migrate apply" (each says why), ` + + 'so they remain drift after it runs.', + ); + } } console.log(chalk.dim(` ${timer.display()}`)); console.log(''); @@ -445,3 +473,57 @@ export default class MigratePlan extends Command { } } } + +/** + * [#24925] The entries `os migrate apply` refuses on this database: each + * `organization_id` drop, while the organization-ownership ceremony has not + * verified. Answered by the two functions `apply`'s drop gate asks, through + * the same `objectql` service, so the plan and the refusal cannot disagree: + * `ceremonyOrganizationColumnDrops`, asked of one entry at a time, says which + * entries; `readOrganizationOwnershipMarker` says whether the ceremony + * verified, and answers "not verified" for a database with no `sys_migration` + * table or row. The marker is read only when such a drop is listed, so every + * other plan reads nothing it did not read before. + */ +async function ceremonyHeldDrops( + stack: { kernel: unknown }, + drift: readonly ManagedDriftEntry[], +): Promise> { + const drops = drift.filter((d) => ceremonyOrganizationColumnDrops([d]).length > 0); + if (drops.length === 0) return new Set(); + const kernel = stack.kernel as { getService?: (n: string) => unknown }; + const engine = kernel.getService?.call(kernel, 'objectql'); + const marker = await readOrganizationOwnershipMarker(engine as never); + return marker.verified ? new Set() : new Set(drops); +} + +/** + * [#24925] The {@link renderPlan} line for a held drop: the finding without + * its `--allow-destructive` sentence, then the ceremony as its remedy. Every + * other entry is described by `otherwise`. The driver package is imported at + * the point of use, never statically (`schema-migrate.lazy-driver-import.test.ts`). + */ +async function describeWithCeremonyHolds( + held: ReadonlySet, + otherwise: (d: ManagedDriftEntry) => string, +): Promise<(d: ManagedDriftEntry) => string> { + const { driftFindingWithoutRemedy } = await import('@objectstack/driver-sql'); + return (d) => (held.has(d) + ? `${driftFindingWithoutRemedy(d)} — a drop the organization-ownership ceremony owns, and it has not verified ` + + 'on this database: the ceremony drops it itself (see below).' + : otherwise(d)); +} + +/** + * [#24925] The footer line for the held drops: why `--allow-destructive` is + * offered for nothing, and the ceremony's two commands, as `os migrate apply`'s + * refusal names them. `waiting` counts the other destructive entries that run + * could perform once the ceremony has verified. + */ +function describeCeremonyHold(drops: number, waiting: number): string { + return `${drops} organization_id drop(s) above are the organization-ownership ceremony's, and it has not verified ` + + 'on this database, so "os migrate apply --allow-destructive" refuses every run until it has' + + (waiting > 0 ? ` (the other ${waiting} destructive change(s) above wait for it too)` : '') + + '. Run "os migrate organization-ownership --out PLAN" to write and review its plan, take a backup, then ' + + '"os migrate organization-ownership --apply --plan-file PLAN --backup-taken"; it drops these columns itself.'; +}