From 420c3fe0938cdd64207b582e8aa55cc1d006510e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 01:01:51 +0000 Subject: [PATCH 1/2] fix(cli): migrate plan and apply name each drop gate's command, never --allow-destructive, for a drop apply refuses The sys_account.issuer and sys_comment.reactions predicates move out of apply.ts into utils/apply-drop-gates.ts beside the organization_id gate's, with one read per gate per run shared by the listing and the gate. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6 --- packages/cli/src/commands/migrate/apply.ts | 88 +++++-- packages/cli/src/commands/migrate/plan.ts | 97 ++------ packages/cli/src/utils/apply-drop-gates.ts | 272 +++++++++++++++++++++ 3 files changed, 359 insertions(+), 98 deletions(-) create mode 100644 packages/cli/src/utils/apply-drop-gates.ts diff --git a/packages/cli/src/commands/migrate/apply.ts b/packages/cli/src/commands/migrate/apply.ts index 5581f5db0a..2d4a066365 100644 --- a/packages/cli/src/commands/migrate/apply.ts +++ b/packages/cli/src/commands/migrate/apply.ts @@ -18,7 +18,6 @@ import { import { bootSchemaStack, collectDriftApplyLimits, - describeWithApplyLimits, describeDatabaseSource, describeTelemetryDatabase, renderPlan, @@ -37,12 +36,16 @@ import { } from '../../utils/schema-migration-plugins.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; +import { ceremonyOrganizationColumnDrops } from '../../utils/organization-ownership-drop-gate.js'; import { - COMMENT_OBJECT, - REACTIONS_COLUMN, - readCommentReactionsConversion, -} from '../../utils/comment-reactions-conversion.js'; -import { ceremonyOrganizationColumnDrops, readOrganizationOwnershipMarker } from '../../utils/organization-ownership-drop-gate.js'; + DropGateReads, + describeDriftEntries, + describeHeldDrops, + isAccountIssuerDrop, + isCommentReactionsDrop, + readHeldDrops, + type HeldDrops, +} from '../../utils/apply-drop-gates.js'; async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes @@ -101,6 +104,13 @@ function describeNotApplied(entry: ManagedDriftEntry): string { * The drift entries are applied as on any database, and **every entry the * driver did not apply is named** by table, column and kind; a destructive * one the operator confirmed makes the run PARTIAL and exit 1. + * + * [#24958] And it never offers `--allow-destructive` for a drop its own gates + * refuse here (`sys_account.issuer`, `sys_comment.reactions`, an + * `organization_id` drop ahead of the ceremony). The listing before the prompt + * names what each such drop waits for, the skip lines offer no re-run while + * one is held, and the gates' predicates and reads are the ones + * `utils/apply-drop-gates.ts` holds for `os migrate plan` too. */ export default class MigrateApply extends Command { static override description = @@ -337,23 +347,44 @@ export default class MigrateApply extends Command { // the --json document, `notApplied`, PARTIAL and the exit code never read // it. Empty for a driver that declares nothing, which reads as it always has. const limits = flags.json ? new Map() : collectDriftApplyLimits(stack, drift); + // [#24958] Each drop gate below reads at most once per run: the listing + // and the gate share that read, so they cannot answer differently. + const gateReads = new DropGateReads(); + // [#24958] The drops those gates refuse on this database, each with the + // gate that refuses it — read for the human face only, like `limits`: + // the --json document, the gates' rule and the exit code never read it. + // Empty when nothing is held, and then everything below reads as it + // always has. + const kernel = stack.kernel as { getService?: (n: string) => unknown }; + const gateHeld: HeldDrops = flags.json + ? new Map() + : await readHeldDrops(gateReads, () => kernel.getService?.call(kernel, 'objectql'), drift); + // A held-back entry no re-run with --allow-destructive could apply here: + // a driver's declared limit keeps it from any run. A gate-held entry is + // not one of these — it waits for the gate's own command. + const neverReRunnable = (d: ManagedDriftEntry) => limits.has(d) && !gateHeld.has(d); if (!flags.json) { // [#23925] An entry with a limit shows its finding and that limit, never - // --allow-destructive. + // --allow-destructive. [#24958] Nor does an entry a gate holds: it + // names what it waits for. await renderPendingSchemaWork(pending); - if (limits.size === 0) renderPlan(drift); - else renderPlan(drift, await describeWithApplyLimits(limits)); + renderPlan(drift, await describeDriftEntries(limits, gateHeld)); if (pending.length > 0) printInfo(await summarizePendingSchemaWork(pending)); printInfo(summarize(drift)); // [#23925] A re-run with --allow-destructive is offered only for the // held-back entries it could apply here; the rest are named as drift - // that re-run would leave as it is. - const reRunnable = deferred.filter((d) => !limits.has(d)); + // that re-run would leave as it is. [#24958] While a gate holds a + // drop, that re-run is refused whole, so it is offered for nothing: + // each gate's line names the command that moves the database on. + const reRunnable = deferred.filter((d) => !neverReRunnable(d)); const notReRunnable = deferred.length - reRunnable.length; if (reRunnable.length > 0) { - printWarning(`${reRunnable.length} destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).`); + printWarning(gateHeld.size === 0 + ? `${reRunnable.length} destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).` + : `${reRunnable.length} destructive change(s) will be SKIPPED, and a re-run with --allow-destructive cannot include them yet:`); } + for (const line of describeHeldDrops(gateHeld, drift, limits)) printWarning(line); if (notReRunnable > 0) { printWarning( `${notReRunnable} destructive change(s) remain drift that a re-run with --allow-destructive cannot apply ` @@ -440,12 +471,11 @@ export default class MigrateApply extends Command { // rows indistinguishable and let a sign-in resolve onto the wrong user's // account — silently. Hence a row-level pre-flight, run against the live // database at the moment it matters. - const dropsAccountIssuer = intended.some( - (d) => d.op?.type === 'drop_column' && d.table === 'sys_account' && d.op.column === 'issuer', - ); + // [#24958] Its predicate and read live in `utils/apply-drop-gates.ts`, + // where `os migrate plan` and the listing above ask them too. + const dropsAccountIssuer = intended.some(isAccountIssuerDrop); if (dropsAccountIssuer) { - const { probeAccountIdentityCollisions, formatAccountIdentityPreflightReport } = - await import('@objectstack/plugin-auth'); + const { formatAccountIdentityPreflightReport } = await import('@objectstack/plugin-auth'); const engine = (stack.kernel as { getService?: (n: string) => unknown }).getService?.call( stack.kernel, 'objectql', @@ -456,7 +486,7 @@ export default class MigrateApply extends Command { // caller gating on the status. Each emitted with 0 until this card. let preflight; try { - preflight = await probeAccountIdentityCollisions(engine as never); + preflight = await gateReads.accountIssuerPreflight(engine); } catch (e: any) { // A pre-flight that could not read is not a pre-flight that passed. const detail = e?.message ?? String(e); @@ -513,16 +543,16 @@ export default class MigrateApply extends Command { // converted, a member may delete their own reaction, and a live probe // would read that as unconverted (`utils/comment-reactions-conversion.ts` // says why). The boot drift report and the retirement's upgrade note name - // this very command as the remedy, so both arrive here. - const dropsCommentReactions = intended.some( - (d) => d.op?.type === 'drop_column' && d.table === COMMENT_OBJECT && d.op.column === REACTIONS_COLUMN, - ); + // this very command as the remedy, so both arrive here. [#24958] Its + // predicate and read live in `utils/apply-drop-gates.ts`, where + // `os migrate plan` and the listing above ask them too. + const dropsCommentReactions = intended.some(isCommentReactionsDrop); if (dropsCommentReactions) { const engine = (stack.kernel as { getService?: (n: string) => unknown }).getService?.call( stack.kernel, 'objectql', ); - const conversion = await readCommentReactionsConversion(engine as never); + const conversion = await gateReads.commentReactionsConversion(engine); if (!conversion.verified) { const detail = conversion.flag ? `The last run of "os migrate comment-reactions --apply" (${conversion.flag.last_run_at}) left ` @@ -569,7 +599,7 @@ export default class MigrateApply extends Command { stack.kernel, 'objectql', ); - const marker = await readOrganizationOwnershipMarker(engine as never); + const marker = await gateReads.organizationOwnershipMarker(engine); if (!marker.verified) { // The ceremony is the way on (18.x step 6): the refusal names its // steps, as the boot refusal does (`organizationOwnershipRefusalMessage`, @@ -688,12 +718,16 @@ export default class MigrateApply extends Command { printSuccess(`Applied ${applied.length} change(s).`); // [#23925] Split as the count before the prompt splits `deferred`: a re-run // is offered only for the held-back entries it could apply here, and the - // rest are named as drift it could not apply either. + // rest are named as drift it could not apply either. [#24958] While a + // gate holds a drop, no re-run is offered: the lines before the prompt + // name each gate's command. const heldBackEntries = skipped.filter((d) => held.has(d)); - const heldBack = heldBackEntries.filter((d) => !limits.has(d)).length; + const heldBack = heldBackEntries.filter((d) => !neverReRunnable(d)).length; const heldBackNotReRunnable = heldBackEntries.length - heldBack; if (heldBack > 0) { - printWarning(`Skipped ${heldBack} destructive change(s) (re-run with --allow-destructive to include them).`); + printWarning(gateHeld.size === 0 + ? `Skipped ${heldBack} destructive change(s) (re-run with --allow-destructive to include them).` + : `Skipped ${heldBack} destructive change(s); a re-run with --allow-destructive cannot include them yet (see above).`); } if (heldBackNotReRunnable > 0) { printWarning( diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 03cb80830b..5d66675a32 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -2,7 +2,6 @@ import { Command, Flags } from '@oclif/core'; import chalk from 'chalk'; -import type { ManagedDriftEntry } from '@objectstack/driver-sql'; import { printHeader, printSuccess, @@ -17,7 +16,6 @@ import { import { bootSchemaStack, collectDriftApplyLimits, - describeWithApplyLimits, describeDatabaseSource, describeTelemetryDatabase, distinctDatabaseDrivers, @@ -40,7 +38,12 @@ 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 { + DropGateReads, + describeDriftEntries, + describeHeldDrops, + readHeldDrops, +} from '../../utils/apply-drop-gates.js'; import { resolveTenancyPosture, collectGlobalUniques, @@ -97,6 +100,13 @@ import { * 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. + * + * [#24958] The same holds for every drop `apply` refuses, not only the + * ceremony's: `sys_account.issuer` while its retirement pre-flight does not + * pass, and `sys_comment.reactions` while no verified run of + * `os migrate comment-reactions --apply` is recorded. Each gate's predicate + * and read live in `utils/apply-drop-gates.ts`, which `apply` asks too; a held + * drop names what it waits for, and the footer names each gate's command. */ export default class MigratePlan extends Command { static override description = @@ -419,21 +429,20 @@ 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 + // [#24925, #24958] The drops `os migrate apply` refuses here, each with + // the gate that refuses it: an `organization_id` drop until the + // organization-ownership ceremony has verified, and the + // `sys_account.issuer` / `sys_comment.reactions` drops until their own + // preconditions hold. Read through the gates' own predicates and reads, + // never a second list. Empty when nothing is held, and then everything // below reads as it always has. - const held = await ceremonyHeldDrops(stack, drift); + const kernel = stack.kernel as { getService?: (n: string) => unknown }; + const held = await readHeldDrops(new DropGateReads(), () => kernel.getService?.call(kernel, 'objectql'), drift); await renderPendingSchemaWork(pending); - 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. - const byLimit = limits.size === 0 ? (d: ManagedDriftEntry) => d.message : await describeWithApplyLimits(limits); - renderPlan(drift, held.size === 0 ? byLimit : await describeWithCeremonyHolds(held, byLimit)); - } + // 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 describeDriftEntries(limits, held)); if (pending.length > 0) printInfo(await summarizePendingSchemaWork(pending)); printInfo(summarize(drift)); if (limits.size === 0 && held.size === 0) { @@ -444,7 +453,7 @@ export default class MigratePlan extends Command { // apply; no apply line at all when it can apply nothing listed here. const appliable = drift.filter((d) => !limits.has(d) && !held.has(d)); const appliableDestructive = appliable.filter((d) => d.category === 'destructive').length; - // [#24925] While the ceremony holds a drop, `apply` refuses every + // [#24925, #24958] While a gate 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'); @@ -453,7 +462,7 @@ export default class MigratePlan extends Command { ? chalk.dim(` (add --allow-destructive for the ${appliableDestructive} destructive change(s) it can apply on this database)`) : '')); } - if (held.size > 0) printWarning(describeCeremonyHold(held.size, appliableDestructive)); + for (const line of describeHeldDrops(held, drift, limits)) printWarning(line); const limited = [...limits.keys()].filter((d) => !held.has(d)).length; if (limited > 0) { printWarning( @@ -473,57 +482,3 @@ 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.'; -} diff --git a/packages/cli/src/utils/apply-drop-gates.ts b/packages/cli/src/utils/apply-drop-gates.ts new file mode 100644 index 0000000000..85168e7790 --- /dev/null +++ b/packages/cli/src/utils/apply-drop-gates.ts @@ -0,0 +1,272 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24958] `os migrate apply`'s drop gates, as every door that names a remedy + * for a drift entry reads them: the plan, and the listing and skip lines + * `apply` prints around its prompt. + * + * ## The shape this closes + * + * The shared differ ends every orphaned column's finding with + * `"os migrate apply --allow-destructive" to drop it.`, and `apply` offers a + * re-run with that flag for every destructive entry it held back. Three of + * those drops `apply` itself refuses on a database that has not met their + * precondition, and it refuses the WHOLE `--allow-destructive` run while one + * of them is in it, whatever flag is passed (ADR-0131 D10 item 5 for the + * ceremony's; the same rule for the other two, each gate's header in + * `commands/migrate/apply.ts`): + * + * - `sys_account.issuer`, while its retirement pre-flight + * (`probeAccountIdentityCollisions`, `@objectstack/plugin-auth`) finds a + * `(provider_id, account_id)` key held by more than one row, or cannot read + * the table; + * - `sys_comment.reactions`, while no verified run of + * `os migrate comment-reactions --apply` is recorded + * (`readCommentReactionsConversion`); + * - every `organization_id` drop, while the organization-ownership ceremony + * has not verified (`ceremonyOrganizationColumnDrops` / + * `readOrganizationOwnershipMarker`). + * + * A door that offered `--allow-destructive` for one of those drops sent the + * operator into a refusal. Each such drop is still listed and counted as + * drift; its line names what it waits for, and the footer names the command + * that moves the database on, never `--allow-destructive`. + * + * ## One source per gate + * + * Which entries a gate judges is the predicate `apply`'s gate asks — the two + * that lived inline in `apply.ts` live here now, and `apply` asks them from + * here — and whether it holds them is the very read that gate makes. There is + * no second inventory, dialect or table list. {@link DropGateReads} makes that + * read once per run, so the listing and the gate behind it can never answer + * differently, and a gate never pays its read twice. + * + * ⛔ A NEW DROP GATE IN `apply.ts` GETS A ROW IN {@link APPLY_DROP_GATES}, and + * one in the enumeration pin `test/migrate-drop-gate-remedies.pin.test.ts`, + * which discovers `apply`'s refusals from its source and fails until both + * exist. The refusal itself, its text and its `--json` document stay in + * `apply.ts`; nothing here refuses anything. + */ + +import type { ManagedDriftEntry } from '@objectstack/driver-sql'; +import { describeWithApplyLimits } from './schema-migrate.js'; +import { + COMMENT_OBJECT, + REACTIONS_COLUMN, + readCommentReactionsConversion, +} from './comment-reactions-conversion.js'; +import { ceremonyOrganizationColumnDrops, readOrganizationOwnershipMarker } from './organization-ownership-drop-gate.js'; + +/** The fields of a drift entry the gates' predicates read. */ +export type DropGateEntry = { table: string; op?: { type?: string; column?: string } }; + +/** The table, and the retired column on it, the `sys_account.issuer` gate guards. */ +export const ACCOUNT_OBJECT = 'sys_account'; +export const ISSUER_COLUMN = 'issuer'; + +/** [#17440] The entry the `sys_account.issuer` retirement pre-flight guards: the retired column's drop. */ +export function isAccountIssuerDrop(d: DropGateEntry): boolean { + return d.op?.type === 'drop_column' && d.table === ACCOUNT_OBJECT && d.op.column === ISSUER_COLUMN; +} + +/** [#23281] The entry that waits for the reactions conversion: the retired `sys_comment.reactions` column's drop. */ +export function isCommentReactionsDrop(d: DropGateEntry): boolean { + return d.op?.type === 'drop_column' && d.table === COMMENT_OBJECT && d.op.column === REACTIONS_COLUMN; +} + +/** [ADR-0131 D10 item 5] An `organization_id` drop the ceremony owns, as the drop gate's own function answers it. */ +export function isCeremonyOrganizationDrop(d: DropGateEntry): boolean { + return ceremonyOrganizationColumnDrops([d]).length > 0; +} + +/** The retirement pre-flight's report, as `@objectstack/plugin-auth` returns it. */ +type AccountIssuerPreflight = Awaited>; + +/** + * The reads `apply`'s drop gates make, each made at most once per run and + * shared by whoever asks first: the listing above the prompt, or the gate. + * A read that throws is remembered as thrown — the issuer pre-flight's + * refusal discipline depends on it (a read that fails is not a clean read). + * + * Every method takes the `objectql` service its caller resolved; a run has one + * stack, so the first caller's is the one read. + */ +export class DropGateReads { + private issuer: Promise | undefined; + private reactions: ReturnType | undefined; + private marker: ReturnType | undefined; + + /** `probeAccountIdentityCollisions` — throws when `sys_account` cannot be enumerated. */ + accountIssuerPreflight(engine: unknown): Promise { + this.issuer ??= (async () => { + const { probeAccountIdentityCollisions } = await import('@objectstack/plugin-auth'); + return probeAccountIdentityCollisions(engine as never); + })(); + return this.issuer; + } + + /** `readCommentReactionsConversion` — the recorded verdict of `os migrate comment-reactions --apply`. */ + commentReactionsConversion(engine: unknown): ReturnType { + this.reactions ??= readCommentReactionsConversion(engine as never); + return this.reactions; + } + + /** `readOrganizationOwnershipMarker` — the ceremony's verified marker. */ + organizationOwnershipMarker(engine: unknown): ReturnType { + this.marker ??= readOrganizationOwnershipMarker(engine as never); + return this.marker; + } +} + +/** What a gate read on this database, when it holds its drops. */ +interface Hold { + /** For the footer: the gate's own words for what it read. */ + why?: string; +} + +/** One of `apply`'s drop gates, as the doors that name a remedy read it. */ +export interface ApplyDropGate { + /** The drop it guards, as an operator reads it. */ + readonly id: 'sys_account.issuer' | 'sys_comment.reactions' | 'organization_id'; + /** Which entries it judges: the predicate `apply`'s gate asks. */ + readonly judges: (d: DropGateEntry) => boolean; + /** Whether it holds them on this database: the read `apply`'s gate makes, judged as it judges it. */ + readonly holds: (reads: DropGateReads, engine: unknown) => Promise; + /** The listing line's tail after the finding, for a held entry. */ + readonly entry: string; + /** The footer line: `held` entries, `waiting` other destructive entries the refusal holds back too. */ + readonly footer: (held: number, waiting: number, hold: Hold) => string; +} + +const andTheOthers = (waiting: number): string => + (waiting > 0 ? ` (the other ${waiting} destructive change(s) above wait for it too)` : ''); + +const REFUSES = '"os migrate apply --allow-destructive" refuses every run until'; + +/** + * `apply`'s drop gates, in the order `apply` asks them. ⛔ A new gate there is + * a new row here (see this file's header). + */ +export const APPLY_DROP_GATES: readonly ApplyDropGate[] = [ + { + id: 'sys_account.issuer', + judges: isAccountIssuerDrop, + // A pre-flight that could not read is not a pre-flight that passed: `apply` refuses both. + holds: async (reads, engine) => { + try { + const preflight = await reads.accountIssuerPreflight(engine); + return preflight.ok ? null : { why: `finds ${preflight.collisions.length} (provider_id, account_id) key(s) held by more than one row` }; + } catch { + return { why: 'could not read sys_account' }; + } + }, + entry: 'a drop that waits for its retirement pre-flight, "os migrate account-issuer", which does not pass on this database (see below).', + footer: (_held, waiting, hold) => + `sys_account.issuer above is the retired account issuer, and its retirement pre-flight ${hold.why} on this ` + + `database, so ${REFUSES} it passes${andTheOthers(waiting)}. Run "os migrate account-issuer" for its report, ` + + 'resolve what it lists, then re-run it as the post-check.', + }, + { + id: 'sys_comment.reactions', + judges: isCommentReactionsDrop, + holds: async (reads, engine) => ((await reads.commentReactionsConversion(engine)).verified ? null : {}), + entry: 'a drop that waits for "os migrate comment-reactions" to carry the reactions stored in it into ' + + 'sys_comment_reaction records, and no verified run is recorded on this database (see below).', + footer: (_held, waiting) => + 'sys_comment.reactions above holds the reactions stored before they became sys_comment_reaction records, and no ' + + `verified run of "os migrate comment-reactions --apply" is recorded on this database, so ${REFUSES} one is` + + `${andTheOthers(waiting)}. Run "os migrate comment-reactions" to preview, then "os migrate comment-reactions ` + + '--apply"; it carries them into records and records the run verified.', + }, + { + id: 'organization_id', + judges: isCeremonyOrganizationDrop, + holds: async (reads, engine) => ((await reads.organizationOwnershipMarker(engine)).verified ? null : {}), + entry: 'a drop the organization-ownership ceremony owns, and it has not verified on this database: the ceremony ' + + 'drops it itself (see below).', + footer: (held, waiting) => + `${held} organization_id drop(s) above are the organization-ownership ceremony's, and it has not verified on ` + + `this database, so ${REFUSES} it has${andTheOthers(waiting)}. 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.', + }, +]; + +/** A held entry: the gate that holds it, and what that gate read. */ +export interface HeldBy { + gate: ApplyDropGate; + hold: Hold; +} + +/** The entries `apply` refuses to drop on this database, each with the gate that holds it. */ +export type HeldDrops = ReadonlyMap; + +/** + * Ask each gate that judges an entry of `drift` whether it holds them here. + * A gate that judges none is never read, and neither is the `objectql` + * service when no gate judges anything: every other database reads nothing it + * did not read before. Empty when nothing is held, and then every door reads + * as it always has. + */ +export async function readHeldDrops( + reads: DropGateReads, + engineOf: () => unknown, + drift: readonly ManagedDriftEntry[], +): Promise { + const held = new Map(); + let engine: { value: unknown } | undefined; + for (const gate of APPLY_DROP_GATES) { + const judged = drift.filter((d) => gate.judges(d)); + if (judged.length === 0) continue; + engine ??= { value: engineOf() }; + const hold = await gate.holds(reads, engine.value); + if (hold) for (const d of judged) held.set(d, { gate, hold }); + } + return held; +} + +/** + * The `renderPlan` line under each entry, or `undefined` for the differ's own + * message on every one: a held entry's finding without its `--allow-destructive` + * sentence, then what its gate waits for; a driver's declared apply limit + * (`describeWithApplyLimits`) for the rest. Held wins where both apply, as it + * is the reason the run is refused. The driver package is imported at the + * point of use, never statically (`schema-migrate.lazy-driver-import.test.ts`). + */ +export async function describeDriftEntries( + limits: ReadonlyMap, + held: HeldDrops, +): Promise<((d: ManagedDriftEntry) => string) | undefined> { + if (limits.size === 0 && held.size === 0) return undefined; + const otherwise = limits.size === 0 ? (d: ManagedDriftEntry) => d.message : await describeWithApplyLimits(limits); + if (held.size === 0) return otherwise; + const { driftFindingWithoutRemedy } = await import('@objectstack/driver-sql'); + return (d) => { + const by = held.get(d); + return by ? `${driftFindingWithoutRemedy(d)} — ${by.gate.entry}` : otherwise(d); + }; +} + +/** + * One footer line per gate that holds a drop here, in `apply`'s order: what it + * holds, why `--allow-destructive` is refused, and the command that moves the + * database on. `waiting` counts, per gate, the other destructive entries a + * refused run holds back with it — every one but its own and those a + * driver's declared limit keeps from any run. + */ +export function describeHeldDrops( + held: HeldDrops, + drift: readonly ManagedDriftEntry[], + limits: ReadonlyMap, +): string[] { + const lines: string[] = []; + for (const gate of APPLY_DROP_GATES) { + const own = drift.filter((d) => held.get(d)?.gate === gate); + if (own.length === 0) continue; + const waiting = drift.filter( + (d) => d.category === 'destructive' && held.get(d)?.gate !== gate && (held.has(d) || !limits.has(d)), + ).length; + lines.push(gate.footer(own.length, waiting, held.get(own[0]!)!.hold)); + } + return lines; +} From 8bb7ff69ffa9c5d0106f0ef6891cb285007966c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 01:17:00 +0000 Subject: [PATCH 2/2] test(cli): enumeration pin for apply's drop gates, and the changeset The pin discovers apply's drop-gate refusals from its source and holds plan and apply to naming each gate's command for a held drop. The two gate refusal tests' fixtures spell the drift entry's finding as `message`, the key ManagedDriftEntry declares. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6 --- .../24958-migrate-drop-gate-remedies.md | 15 + .../apply.account-issuer-json-refusal.test.ts | 9 +- ...ply.organization-ownership-refusal.test.ts | 4 +- packages/cli/src/utils/apply-drop-gates.ts | 4 +- .../migrate-drop-gate-remedies.pin.test.ts | 501 ++++++++++++++++++ 5 files changed, 528 insertions(+), 5 deletions(-) create mode 100644 .changeset/24958-migrate-drop-gate-remedies.md create mode 100644 packages/cli/test/migrate-drop-gate-remedies.pin.test.ts diff --git a/.changeset/24958-migrate-drop-gate-remedies.md b/.changeset/24958-migrate-drop-gate-remedies.md new file mode 100644 index 0000000000..f577bea677 --- /dev/null +++ b/.changeset/24958-migrate-drop-gate-remedies.md @@ -0,0 +1,15 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate plan` and `os migrate apply` name the command each held drop waits for, never `--allow-destructive`, for every drop `apply` refuses (#24958) + +Clause-②: no + +`os migrate apply --allow-destructive` refuses the whole run, whatever flag is passed, while it would drop one of three columns before its precondition holds: `sys_account.issuer` while its retirement pre-flight finds `(provider_id, account_id)` keys held by more than one row (or cannot read `sys_account`), `sys_comment.reactions` while no verified run of `os migrate comment-reactions --apply` is recorded, and an `organization_id` column while the organization-ownership ceremony has not verified. Yet `os migrate plan` offered `--allow-destructive` for the first two, and `os migrate apply` without the flag listed all three with `"os migrate apply --allow-destructive" to drop it.` and closed with `N destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).` and `Skipped N destructive change(s) (re-run with --allow-destructive to include them).` That re-run was refused, exit 1. + +- **Now.** Each such drop is still listed and counted as destructive drift, in both commands. Its line names what it waits for: the ceremony, `os migrate comment-reactions`, or the retirement pre-flight `os migrate account-issuer`. +- **The footer.** One line per gate that holds a drop names that gate's commands and says `--allow-destructive` is refused until they have run. `os migrate plan` offers `--allow-destructive` for nothing while a drop is held. `os migrate apply` prints `N destructive change(s) will be SKIPPED, and a re-run with --allow-destructive cannot include them yet:` before those lines, and `Skipped N destructive change(s); a re-run with --allow-destructive cannot include them yet (see above).` after the apply. +- **Unchanged.** A database with no held drop prints what it printed before, byte for byte, in both commands. The `--json` documents, the exit codes and every refusal of `os migrate apply --allow-destructive` (its rule and its text) are unchanged. + +**What to do.** Nothing. Follow the footer: run the command each held drop names, then re-run `os migrate plan`. diff --git a/packages/cli/src/commands/migrate/apply.account-issuer-json-refusal.test.ts b/packages/cli/src/commands/migrate/apply.account-issuer-json-refusal.test.ts index fcd562cdad..4ceb00a499 100644 --- a/packages/cli/src/commands/migrate/apply.account-issuer-json-refusal.test.ts +++ b/packages/cli/src/commands/migrate/apply.account-issuer-json-refusal.test.ts @@ -69,7 +69,9 @@ const ISSUER_DROP = { table: 'sys_account', column: 'issuer', op: { type: 'drop_column', column: 'issuer' }, - description: 'sys_account.issuer: column exists in the database but not in metadata (orphaned)', + // The differ's `message` (`ManagedDriftEntry`), remedy sentence included: the + // listing above the gate reads it (#24958). + message: 'sys_account.issuer: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it.', }; const COLLISIONS = { @@ -188,7 +190,10 @@ describe('controls', () => { const { exit } = await runApply(ARGV); expect(exit).toBe(1); - // Refused by the pre-flight, not by anything earlier on the human face. + // Refused by the pre-flight, not by anything earlier on the human face: + // its refusal printed, and the pre-flight read once — the listing above + // the gate shares the gate's read (#24958). + expect(log.mock.calls.map((c: unknown[]) => c.map(String).join(' ')).some((l: string) => l.includes('Refusing to drop sys_account.issuer'))).toBe(true); expect(mocks.probeAccountIdentityCollisions).toHaveBeenCalledTimes(1); expect(applyMigrationEntries).not.toHaveBeenCalled(); }, RUN_TIMEOUT); diff --git a/packages/cli/src/commands/migrate/apply.organization-ownership-refusal.test.ts b/packages/cli/src/commands/migrate/apply.organization-ownership-refusal.test.ts index 08fa2c97e8..a08abf8e73 100644 --- a/packages/cli/src/commands/migrate/apply.organization-ownership-refusal.test.ts +++ b/packages/cli/src/commands/migrate/apply.organization-ownership-refusal.test.ts @@ -64,7 +64,9 @@ const ORGANIZATION_DROP = { table: 'sys_platform_setting', column: 'organization_id', op: { type: 'drop_column', column: 'organization_id' }, - description: 'sys_platform_setting.organization_id: column exists in the database but not in metadata (orphaned)', + // The differ's `message` (`ManagedDriftEntry`), remedy sentence included: the + // listing above the gate reads it (#24958). + message: 'sys_platform_setting.organization_id: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it.', }; /** The ceremony's two steps, as the refusal prescribes them. */ diff --git a/packages/cli/src/utils/apply-drop-gates.ts b/packages/cli/src/utils/apply-drop-gates.ts index 85168e7790..d93231fab0 100644 --- a/packages/cli/src/utils/apply-drop-gates.ts +++ b/packages/cli/src/utils/apply-drop-gates.ts @@ -170,10 +170,10 @@ export const APPLY_DROP_GATES: readonly ApplyDropGate[] = [ id: 'sys_comment.reactions', judges: isCommentReactionsDrop, holds: async (reads, engine) => ((await reads.commentReactionsConversion(engine)).verified ? null : {}), - entry: 'a drop that waits for "os migrate comment-reactions" to carry the reactions stored in it into ' + entry: 'a drop that waits for "os migrate comment-reactions" to carry any reactions stored in it into ' + 'sys_comment_reaction records, and no verified run is recorded on this database (see below).', footer: (_held, waiting) => - 'sys_comment.reactions above holds the reactions stored before they became sys_comment_reaction records, and no ' + 'sys_comment.reactions above is where reactions were stored before they became sys_comment_reaction records, and no ' + `verified run of "os migrate comment-reactions --apply" is recorded on this database, so ${REFUSES} one is` + `${andTheOthers(waiting)}. Run "os migrate comment-reactions" to preview, then "os migrate comment-reactions ` + '--apply"; it carries them into records and records the run verified.', diff --git a/packages/cli/test/migrate-drop-gate-remedies.pin.test.ts b/packages/cli/test/migrate-drop-gate-remedies.pin.test.ts new file mode 100644 index 0000000000..afce72ae41 --- /dev/null +++ b/packages/cli/test/migrate-drop-gate-remedies.pin.test.ts @@ -0,0 +1,501 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN — for every drop `os migrate apply` refuses on a database, `os migrate + * plan` and `apply`'s own listing and skip lines name the command that moves + * that database on, and never offer `--allow-destructive` for it (#24958). + * + * ## The defect + * + * Measured at the public door on `62bc12930`, the CLI binary against copies of + * a SQLite database a `create-objectstack@17.7.0` app had booted once: + * + * os migrate apply --yes + * → each organization_id drop listed with `"os migrate apply --allow-destructive" to drop it.` + * ⚠ 22 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them). + * ⚠ Skipped 22 destructive change(s) (re-run with --allow-destructive to include them). + * os migrate apply --allow-destructive --yes + * → ✗ Refusing to drop sys_comment.reactions: … Run "os migrate comment-reactions" to preview … exit 1 + * + * The plan offered `--allow-destructive` for `sys_comment.reactions` before + * the conversion had run, and for `sys_account.issuer` while its retirement + * pre-flight found colliding keys; `apply --allow-destructive` refused both, + * exit 1, naming `os migrate comment-reactions` and `os migrate account-issuer`. + * + * ## The population is discovered from `apply.ts` + * + * Each of `apply`'s drop gates answers its `--json` refusal with a + * `message: 'refused_…'`. The first case reads those from `apply.ts`'s source + * and requires every one to be claimed by a row of {@link GATES} (or by + * {@link NOT_A_DROP_GATE}), and the rows to be the gates + * `utils/apply-drop-gates.ts` reads, in the same order. + * + * ⛔ A NEW DROP GATE IN `apply.ts` ADDS A ROW TO {@link GATES}: the drift entry + * it guards, a database state it holds that entry on, one it releases it on, + * and the commands its refusal names. The discovery case fails, naming its + * refusal, until the row exists; the row's cases then fail until + * `APPLY_DROP_GATES` reads that gate for the doors. + * + * ## Per gate, both doors, each state + * + * - held, `plan`: the entry is still listed and counted, its line names what + * it waits for and never `--allow-destructive`, and the footer names every + * command of the gate's remedy and offers `--allow-destructive` for nothing; + * - held, `apply --yes`: the same listing line and footer, and no line offers + * a re-run with `--allow-destructive`, before the prompt or after the apply; + * - held, `apply --allow-destructive --yes`: the same listing line, then the + * gate's own refusal, exit 1, nothing written, naming the same commands — + * and the gate's read was made once, shared by the listing and the gate; + * - CONTROL, released: both doors print what they always printed — the + * differ's `--allow-destructive` remedy and the re-run offer. + * + * ## What runs for real + * + * Both commands' parse, listing and footer, `apply`'s gates, and every gate's + * own predicate and read: the reads go to a fake `objectql` service holding the + * database state (`sys_migration` rows, `sys_account` rows). Replaced: the boot, + * the occupancy probe and the one-shot exit. `unit` tier: nothing spawns and no + * kernel boots (`vitest-tiers.ts`). + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { Config } from '@oclif/core'; +import { isExitSignal } from '../src/utils/format.js'; +import { APPLY_DROP_GATES } from '../src/utils/apply-drop-gates.js'; + +// ⛔ `vi.hoisted`, never a value import of `bootSchemaStack`: a value import of +// that symbol files a test in the integration tier (`vitest-tiers.ts`). +const seams = vi.hoisted(() => ({ + bootSchemaStack: vi.fn(), + exitOneShotCommand: vi.fn(async (_code: number) => {}), +})); +vi.mock('../src/utils/schema-migrate.js', async (importOriginal) => ({ + ...(await importOriginal>()), + bootSchemaStack: seams.bootSchemaStack, +})); +vi.mock('../src/utils/migrate-occupancy-gate.js', () => ({ + OCCUPANCY_HINT: 'occupancy hint', + probeMigrationTarget: vi.fn(async () => ({ status: 'not_applicable' })), +})); +vi.mock('../src/utils/one-shot-exit.js', () => ({ exitOneShotCommand: seams.exitOneShotCommand })); + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PKG_ROOT = resolve(HERE, '..'); +const APPLY_SOURCE = resolve(HERE, '../src/commands/migrate/apply.ts'); + +// Paid at module scope — collection is unclocked; every case below only runs. +const config = await Config.load({ root: PKG_ROOT }); +const { default: MigratePlan } = await import('../src/commands/migrate/plan.js'); +const { default: MigrateApply } = await import('../src/commands/migrate/apply.js'); +// What the commands import at the point of use — the issuer gate's pre-flight +// (plugin-auth), the differ's remedy stripper (driver-sql), the plan's row +// normaliser and the flag reads — has its first load paid here, not inside a +// clocked case. +await Promise.all([ + import('@objectstack/plugin-auth'), + import('@objectstack/driver-sql'), + import('@objectstack/metadata-protocol'), + import('@objectstack/platform-objects/system'), +]); + +// --------------------------------------------------------------------------- +// The database a gate reads, behind a fake `objectql` service +// --------------------------------------------------------------------------- + +type Row = Record; +interface DatabaseState { + /** `sys_migration` rows by id. */ + flags?: Record; + /** `sys_account` rows, or an error its read throws. */ + accounts?: Row[] | Error; +} + +const AT = '2026-10-21T00:00:00.000Z'; +const verifiedFlag = (id: string): Row => ({ id, last_run_at: AT, verified_at: AT, blocking: 0 }); +const unverifiedFlag = (id: string): Row => ({ id, last_run_at: AT, verified_at: null, blocking: 3 }); +const REACTIONS_FLAG = 'sys-comment-reactions-to-records'; +const CEREMONY_FLAG = 'adr-0131-organization-ownership'; +const account = (id: string, issuer: string): Row => ({ + id, provider_id: 'oidc', account_id: 'sub_1', issuer, user_id: `usr_${id}`, +}); + +/** The `objectql` service the gates' reads meet; counts each read. */ +function engineOver(state: DatabaseState) { + return { + getObject: (name: string) => ({ name }), + findOne: vi.fn(async (object: string, options: { where?: { id?: string } }) => { + expect(object).toBe('sys_migration'); + return state.flags?.[options.where?.id ?? ''] ?? null; + }), + find: vi.fn(async (object: string, query: { where?: { id?: { $gt?: string } } }) => { + expect(object).toBe('sys_account'); + if (state.accounts instanceof Error) throw state.accounts; + const after = query.where?.id?.$gt; + return (state.accounts ?? []).filter((r) => after === undefined || String(r.id) > after); + }), + }; +} +type Engine = ReturnType; + +// --------------------------------------------------------------------------- +// The drift, as the shared differ reports it +// --------------------------------------------------------------------------- + +type Entry = { + kind: string; remoteName: string; table: string; column?: string; severity: string; + category: 'safe' | 'needs_confirm' | 'destructive'; op: Record & { type: string }; message: string; +}; + +const ALLOW_DESTRUCTIVE_REMEDY = ' — "os migrate apply --allow-destructive" to drop it.'; + +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)${ALLOW_DESTRUCTIVE_REMEDY}`, + }; +} +/** A destructive drop no gate guards. */ +const UNGATED = orphan('sys_sso_provider', 'created_by'); +/** A safe entry, which `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 roster — one row per drop gate `os migrate apply` has +// --------------------------------------------------------------------------- + +interface GateRow { + /** `APPLY_DROP_GATES[i].id`, in `apply`'s order. */ + id: string; + /** The `--json` refusal messages this gate answers with (discovered from `apply.ts`). */ + refusals: string[]; + /** The drift entry it guards. */ + entry: Entry; + /** + * Database states it holds that entry on, and the line its refusal starts + * with there. `names` is false for a refusal that names no command (the + * issuer pre-flight's unreadable table: its detail is the remedy). + */ + held: Array<{ state: string; db: DatabaseState; refusal: string; names?: false }>; + /** A database state it releases it on. */ + released: DatabaseState; + /** The commands its refusal names, which every door names for a held entry. */ + remedy: string[]; + /** How many reads of its verdict a held run makes, per the fake engine. */ + readsOf: (engine: Engine) => number; +} + +const flagReads = (id: string) => (engine: Engine) => engine.findOne.mock.calls.filter(([, o]) => o.where?.id === id).length; + +/** + * ⛔ A NEW DROP GATE IN `apply.ts` ADDS ITS ROW HERE (see this file's header). + */ +const GATES: GateRow[] = [ + { + id: 'sys_account.issuer', + refusals: ['refused_account_issuer_preflight_unreadable', 'refused_account_issuer_collisions'], + entry: orphan('sys_account', 'issuer'), + held: [ + { + state: 'two rows share one (provider_id, account_id) key', + db: { accounts: [account('a1', 'https://idp-a'), account('a2', 'https://idp-b')] }, + refusal: 'Refusing to drop sys_account.issuer: dropping it would make the rows above indistinguishable', + }, + { + state: 'sys_account cannot be read', + db: { accounts: new Error('SQLITE_ERROR: no such column: provider_id') }, + refusal: 'Refusing to drop sys_account.issuer: the retirement pre-flight could not read the table.', + names: false, + }, + ], + released: { accounts: [account('a1', 'https://idp-a')] }, + remedy: ['os migrate account-issuer'], + // The walk's first page carries no cursor: one per pre-flight. + readsOf: (engine) => engine.find.mock.calls.filter(([, q]) => q.where?.id?.$gt === undefined).length, + }, + { + id: 'sys_comment.reactions', + refusals: ['refused_comment_reactions_unconverted'], + entry: orphan('sys_comment', 'reactions'), + held: [ + { state: 'no conversion run recorded', db: {}, refusal: 'Refusing to drop sys_comment.reactions:' }, + { + state: 'an unverified conversion run recorded', + db: { flags: { [REACTIONS_FLAG]: unverifiedFlag(REACTIONS_FLAG) } }, + refusal: 'Refusing to drop sys_comment.reactions:', + }, + ], + released: { flags: { [REACTIONS_FLAG]: verifiedFlag(REACTIONS_FLAG) } }, + remedy: ['os migrate comment-reactions', 'os migrate comment-reactions --apply'], + readsOf: flagReads(REACTIONS_FLAG), + }, + { + id: 'organization_id', + refusals: ['refused_organization_ownership_unmigrated'], + entry: orphan('sys_job', 'organization_id'), + held: [ + { state: 'no ceremony run recorded', db: {}, refusal: 'Refusing this --allow-destructive run: it would drop organization_id' }, + { + state: 'an unverified ceremony run recorded', + db: { flags: { [CEREMONY_FLAG]: unverifiedFlag(CEREMONY_FLAG) } }, + refusal: 'Refusing this --allow-destructive run: it would drop organization_id', + }, + ], + released: { flags: { [CEREMONY_FLAG]: verifiedFlag(CEREMONY_FLAG) } }, + remedy: [ + 'os migrate organization-ownership --out PLAN', + 'os migrate organization-ownership --apply --plan-file PLAN --backup-taken', + ], + readsOf: flagReads(CEREMONY_FLAG), + }, +]; + +/** `apply`'s refusals that are not a drop gate's. */ +const NOT_A_DROP_GATE = ['refused_unloadable_host_config']; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +let flushSchemaDdl: ReturnType; +let applyMigrationEntries: ReturnType; + +/** Boot a stack whose drift is `drift` and whose `objectql` service reads `db`; answer the engine. */ +function bootOver(drift: Entry[], db: DatabaseState): Engine { + const engine = engineOver(db); + flushSchemaDdl = vi.fn(async () => []); + applyMigrationEntries = vi.fn(async (entries: Entry[], opts: { allowDestructive: boolean }) => ({ + applied: entries.filter((d) => d.category !== 'destructive' || opts.allowDestructive), + skipped: entries.filter((d) => d.category === 'destructive' && !opts.allowDestructive), + })); + seams.bootSchemaStack.mockResolvedValue({ + driver: { config: { url: 'postgres://db.example/app' } }, + drivers: [], + dbLabel: 'postgres://db.example/app', + dbSource: { kind: 'flag' }, + telemetryDatabase: null, + schemaSync: 'deferred', + composition: { + plugins: [], hostConfigPath: null, hostConfigLoaded: false, hostConfigError: null, + servedBoot: { mirrored: false, reason: 'not-composed' }, notes: [], coverage: null, + }, + detectManagedDrift: async () => drift, + driftApplyLimit: () => null, + pendingSchemaWork: [], + managedTableCount: 3, + nonObjectTables: [], + tableAbsent: () => false, + allObjects: () => [], + kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) }, + flushSchemaDdl, + applyMigrationEntries, + shutdown: async () => {}, + }); + return engine; +} + +let said: string[]; +beforeEach(() => { + said = []; + seams.bootSchemaStack.mockReset(); + seams.exitOneShotCommand.mockClear(); + const capture = (...parts: unknown[]) => { said.push(parts.map(String).join(' ')); }; + vi.spyOn(console, 'log').mockImplementation(capture); + vi.spyOn(console, 'warn').mockImplementation(capture); + vi.spyOn(console, 'error').mockImplementation(capture); + vi.spyOn(process.stdout, '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.stdout.write); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Run one command; answer every line it printed and the status it ends with. */ +async function drive( + command: { run(argv: string[], opts?: Config): Promise }, + argv: string[], +): Promise<{ lines: string[]; exit: number | undefined }> { + const savedExit = process.exitCode; + process.exitCode = undefined; + try { + const err = await command.run([...argv, '--database-url', 'postgres://db.example/app'], config).then(() => null, (e: unknown) => e); + if (err !== null && !isExitSignal(err)) throw err; + const exit = err !== null + ? (err as { oclif?: { exit?: number } }).oclif?.exit + : seams.exitOneShotCommand.mock.calls.at(-1)?.[0]; + return { lines: said.join('\n').split('\n'), exit }; + } finally { + process.exitCode = savedExit; + } +} + +/** The line printed under the entry acting on `e`: its finding and its remedy. */ +function lineUnder(lines: string[], e: Entry): string { + const at = lines.findIndex((l) => l.includes(` ${e.table}.${e.column} [`)); + expect(at, `no ${e.table}.${e.column} entry in:\n${lines.join('\n')}`).toBeGreaterThanOrEqual(0); + return lines[at + 1]!; +} + +/** Everything printed below the summary line. */ +function below(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); +} + +/** A held entry's line: the finding, then what it waits for — never the differ's remedy. */ +function expectHeldLine(line: string, e: Entry): void { + expect(line).toContain(e.message.slice(0, -ALLOW_DESTRUCTIVE_REMEDY.length)); + expect(line).not.toContain('--allow-destructive'); + expect(line).toMatch(/\(see below\)\.$/); +} + +const REMEDY_SENTENCE = /"os migrate apply --allow-destructive" to drop it/; +const RE_RUN_OFFER = /re-run with --allow-destructive to include them/; + +// --------------------------------------------------------------------------- +// 1. The roster is apply's +// --------------------------------------------------------------------------- + +describe('the roster is the drop gates apply has', () => { + it("every `refused_…` message in apply.ts is claimed by a row — a new gate's refusal fails here until it has one", () => { + const source = readFileSync(APPLY_SOURCE, 'utf8'); + const discovered = [...new Set([...source.matchAll(/message: '(refused_[a-z_]+)'/g)].map((m) => m[1]!))].sort(); + // Not vacuous: the scan reaches the refusals this was filed on. + expect(discovered.length).toBeGreaterThanOrEqual(5); + expect(discovered).toEqual([...GATES.flatMap((g) => g.refusals), ...NOT_A_DROP_GATE].sort()); + }); + + it('the rows are the gates the doors read, in the order apply asks them', () => { + expect(APPLY_DROP_GATES.map((g) => g.id)).toEqual(GATES.map((g) => g.id)); + }); + + it.each(GATES.map((g) => [g.id, g] as const))('%s: its entry is judged by its gate and by no other', (_id, row) => { + expect(APPLY_DROP_GATES.filter((g) => g.judges(row.entry)).map((g) => g.id)).toEqual([row.id]); + expect(APPLY_DROP_GATES.some((g) => g.judges(UNGATED) || g.judges(INDEX))).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// 2. Held: every door names the gate's command, never --allow-destructive +// --------------------------------------------------------------------------- + +const HELD = GATES.flatMap((row) => row.held.map((h) => [row.id, h.state, row, h.db, h] as const)); + +describe('a drop apply refuses here', () => { + it.each(HELD)('%s (%s): os migrate plan lists and counts it, names the remedy, and offers --allow-destructive for nothing', async (_id, _state, row, db) => { + bootOver([INDEX, row.entry, UNGATED], db); + const { lines, exit } = await drive(MigratePlan, []); + expect(exit, lines.join('\n')).toBe(0); + + expectHeldLine(lineUnder(lines, row.entry), row.entry); + // Still drift: listed and counted. + expect(lines.join('\n')).toContain('3 change(s): 1 safe, 0 needs-confirm, 2 destructive'); + // The gate it does not guard keeps the differ's message. + expect(lineUnder(lines, UNGATED)).toBe(` ${UNGATED.message}`); + + const footer = below(lines).join('\n'); + expect(footer).toContain('Apply with: os migrate apply'); + expect(footer).not.toMatch(/add --allow-destructive/); + for (const command of row.remedy) expect(footer).toContain(`"${command}"`); + }, 60_000); + + it.each(HELD)('%s (%s): os migrate apply --yes lists it with the remedy, and offers no re-run with --allow-destructive', async (_id, _state, row, db) => { + bootOver([INDEX, row.entry, UNGATED], db); + const { lines, exit } = await drive(MigrateApply, ['--yes']); + expect(exit, lines.join('\n')).toBe(0); + + expectHeldLine(lineUnder(lines, row.entry), row.entry); + const after = below(lines).join('\n'); + expect(after).not.toMatch(RE_RUN_OFFER); + for (const command of row.remedy) expect(after).toContain(`"${command}"`); + expect(after).toContain('2 destructive change(s) will be SKIPPED, and a re-run with --allow-destructive cannot include them yet:'); + expect(after).toContain('Skipped 2 destructive change(s); a re-run with --allow-destructive cannot include them yet (see above).'); + // The safe work still runs: held drops hold back only the destructive half. + expect(applyMigrationEntries).toHaveBeenCalledTimes(1); + expect(applyMigrationEntries.mock.calls[0]![1]).toEqual({ allowDestructive: false }); + }, 60_000); + + it.each(HELD)('%s (%s): os migrate apply --allow-destructive --yes lists it with the remedy, then its gate refuses, reading the gate once', async (_id, _state, row, db, held) => { + const engine = bootOver([INDEX, row.entry, UNGATED], db); + const { lines, exit } = await drive(MigrateApply, ['--allow-destructive', '--yes']); + expect(exit, lines.join('\n')).toBe(1); + expect(flushSchemaDdl).not.toHaveBeenCalled(); + expect(applyMigrationEntries).not.toHaveBeenCalled(); + + expectHeldLine(lineUnder(lines, row.entry), row.entry); + // The gate's own refusal, as it always was: the doors above name the + // commands it names. + const refusal = lines.find((l) => l.includes('✗ Refusing')); + expect(refusal, lines.join('\n')).toContain(`✗ ${held.refusal}`); + if (held.names !== false) for (const command of row.remedy) expect(refusal).toContain(command); + // The listing and the gate share one read of the gate's verdict. + expect(row.readsOf(engine)).toBe(1); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// 3. CONTROL — released: both doors print what they always printed +// --------------------------------------------------------------------------- + +describe('CONTROL: a drop its gate releases', () => { + it.each(GATES.map((g) => [g.id, g] as const))('%s: os migrate plan offers --allow-destructive for it, as it always did', async (_id, row) => { + const engine = bootOver([INDEX, row.entry, UNGATED], row.released); + const { lines, exit } = await drive(MigratePlan, []); + expect(exit, lines.join('\n')).toBe(0); + expect(lineUnder(lines, row.entry)).toBe(` ${row.entry.message}`); + expect(lineUnder(lines, row.entry)).toMatch(REMEDY_SENTENCE); + expect(below(lines).filter((l) => l.trim() !== '' && !/^\s+\d+ms$/.test(l))).toEqual([ + ' Apply with: os migrate apply (add --allow-destructive for drops / tightenings)', + ]); + // …and the verdict WAS read: this control is not passing for want of a read. + expect(row.readsOf(engine)).toBe(1); + }, 60_000); + + it.each(GATES.map((g) => [g.id, g] as const))('%s: os migrate apply --yes offers the re-run, as it always did', async (_id, row) => { + bootOver([INDEX, row.entry, UNGATED], row.released); + const { lines, exit } = await drive(MigrateApply, ['--yes']); + expect(exit, lines.join('\n')).toBe(0); + expect(lineUnder(lines, row.entry)).toBe(` ${row.entry.message}`); + const after = below(lines).join('\n'); + expect(after).toContain('2 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).'); + expect(after).toContain('Skipped 2 destructive change(s) (re-run with --allow-destructive to include them).'); + }, 60_000); + + it('no gated drop at all: neither door reads a gate', async () => { + for (const command of [MigratePlan, MigrateApply]) { + const engine = bootOver([INDEX, UNGATED], {}); + const { lines, exit } = await drive(command, command === MigrateApply ? ['--yes'] : []); + expect(exit, lines.join('\n')).toBe(0); + expect(engine.find).not.toHaveBeenCalled(); + expect(engine.findOne).not.toHaveBeenCalled(); + expect(lineUnder(lines, UNGATED)).toBe(` ${UNGATED.message}`); + } + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// 4. --json reads nothing for the doors, and its document is the report +// --------------------------------------------------------------------------- + +describe('--json', () => { + it.each(GATES.map((g) => [g.id, g] as const))('%s held: os migrate plan --json reads no gate and carries the differ\'s message', async (_id, row) => { + const engine = bootOver([INDEX, row.entry, UNGATED], row.held[0]!.db); + const { exit } = await drive(MigratePlan, ['--json']); + expect(exit).toBe(0); + expect(row.readsOf(engine)).toBe(0); + const out = (process.stdout.write as unknown as { mock: { calls: unknown[][] } }).mock.calls.map((c) => String(c[0])).join(''); + const doc = JSON.parse(out.trim()); + expect(doc.changes.map((c: Entry) => c.message)).toEqual([INDEX.message, row.entry.message, UNGATED.message]); + }, 60_000); +});