fix(cli)!: os migrate apply --allow-destructive holds back every organization_id drop until the organization-ownership ceremony verifies (#23684)

.changeset/23684-drop-gate-host-tables.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/cli': minor
+---
+
+fix(cli)!: `os migrate apply --allow-destructive` holds back the `organization_id` drop on every table the organization-ownership ceremony plans, not only the platform inventory's, until the ceremony has verified (ADR-0131 D7, D10 item 5)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no authorable spelling, export or stored shape is removed, renamed or re-shaped, and no stored row is rewritten, converted or deleted, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is one CLI invocation: an `os migrate apply --allow-destructive` run that would drop `organization_id` from a table outside the platform inventory is refused until the organization-ownership ceremony's marker is verified; the remedy is running that ceremony, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
+
+**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
+
+**The contract this meets.** The drop gate's own header (`packages/cli/src/utils/organization-ownership-drop-gate.ts`) already states it: the drop of `organization_id` on a table the ceremony owns waits for the ceremony's verified marker, and "⛔ No flag stands it down". ADR-0131 D10 item 5 is the rule behind it: a v18 runtime refuses to boot against a database the ceremony has not completed, and offers no escape hatch that silently skips that check. A column dropped ahead of the ceremony is that escape hatch, because the boot refusal then finds no column to refuse over.
+
+**FROM.** The gate held back only the tables of the platform inventory. Every other `organization_id` drop the schema-drift report offered was applied by `os migrate apply --allow-destructive` with no ceremony marker on the database: on the objects a host declares platform-global (ADR-0131 D7, the `org-scoping` service's `platformGlobalObjects`), and on any host or application object that declares no organization column of its own. Measured on a host composition that declares two objects platform-global: `os migrate apply --database-url file:v17.db --allow-destructive --yes --json` exited 0 and applied `drop_column organization_id` on both.
+
+**TO.** The gate holds back every `organization_id` drop the drift offers. The ceremony's plan meets every physical table that carries the column, with an inventory row, a `--fate-supplement` row, the application-object fate, or a refusal by name; the drift offers only a column that is physically there. So this is the ceremony's own table set, read without a copy of its inventory and without the fate-supplement file `os migrate apply` is never handed. It includes an application object that declares `systemFields: { tenant: false }` over a table that still carries the column: the ceremony plans that table, and the v18 boot refusal refuses over it too. While the `sys_migration` row `adr-0131-organization-ownership` is not verified, the run is refused with the existing refusal. It exits 1 and prints "Refusing this --allow-destructive run: it would drop organization_id from TABLES", naming `os migrate organization-ownership --out PLAN`, then `os migrate organization-ownership --apply --plan-file PLAN`, as the remedy. Under `--json` it answers `message: 'refused_organization_ownership_unmigrated'` with the `tables`, the `detail` and the `marker`. As before, the refusal holds the whole `--allow-destructive` run: nothing in it is created, applied or dropped.
+
+**Unchanged.** Once the ceremony's marker is verified, the drop applies. A destructive change to any other column applies as before. A run without `--allow-destructive` is unchanged. There is no new flag, and no flag skips the refusal.
+
+**Remedy.** Run `os migrate organization-ownership` (with `--fate-supplement FILE` when the host provides objects of its own), review its plan, run `os migrate organization-ownership --apply --plan-file PLAN`, then re-run `os migrate apply --allow-destructive`.

packages/cli/src/utils/organization-ownership-drop-gate.ts

@@ -6,35 +6,50 @@
  * ceremony verified on this database?
  *
  * A v18 runtime declares no organization column on the ceremony's
- * column-drop tables (the D7 tables and the `sys_metadata` family), so the
- * drift report lists each physical one as an unmapped, destructive column and
+ * column-drop tables (the D7 tables, the platform's and the ones a deployment
+ * declares platform-global, and the `sys_metadata` family), so the drift
+ * report lists each physical one as an unmapped, destructive column and
  * names `os migrate apply --allow-destructive` as its remedy. Applied ahead of
  * the ceremony, that drop skips its promotion and report steps, turns every
  * legacy organization row into an environment row, and leaves no physical
  * column for the v18 boot refusal to find — so the server starts with the
  * marker never written. D10 item 5 forbids exactly that escape hatch. So the
- * drop of `organization_id` on any table in the ceremony's inventory waits for
- * the ceremony's verified marker, as `sys_account.issuer` (#17440) and
+ * drop of `organization_id` on any table the ceremony plans waits for the
+ * ceremony's verified marker, as `sys_account.issuer` (#17440) and
  * `sys_comment.reactions` (#23281) wait for theirs. ⛔ No flag stands it down.
+ *
+ * ## Which tables: every `organization_id` drop the drift offers (#23684)
+ *
+ * The ceremony's table set is the database's, not a list's: its plan meets
+ * every physical table that carries `organization_id`, with the platform
+ * inventory's row, a host fate supplement's row, the application-object fate,
+ * or a refusal by name (`organization-ownership-plan.ts`). The drift can offer
+ * to drop only a column that is physically there. So every `organization_id`
+ * drop it offers is on a table the ceremony plans, and this gate holds back
+ * every one: the platform inventory's tables, the objects a deployment
+ * declares platform-global (ADR-0131 D7), and the objects a host's fate
+ * supplement names, a file `os migrate apply` is never handed. The v18 boot
+ * refusal judges the same population, every object that declares no
+ * organization column, not the inventory.
+ *
+ * Reading the drift rather than a composed table list keeps one source: no
+ * second copy of the ceremony's inventory composition, and no part of the
+ * answer this command could fail to read.
  */
 
 import type { MigrationFlagEngine } from '@objectstack/platform-objects/system';
 import type { DataMigrationFlag } from '@objectstack/spec/system';
-import { ORGANIZATION_OWNERSHIP_INVENTORY } from './organization-ownership-inventory.js';
 
 /** The column the ceremony owns the drop of. */
 export const ORGANIZATION_COLUMN = 'organization_id';
 
-/** Every table the ceremony's inventory names. */
-const CEREMONY_TABLES: ReadonlySet<string> = new Set(ORGANIZATION_OWNERSHIP_INVENTORY.map((entry) => entry.object));
-
-/** The tables whose `organization_id` drop is among `intended`, sorted. */
+/** The tables whose `organization_id` drop is among `intended`, sorted: every one, since the ceremony plans each table that carries the column. */
 export function ceremonyOrganizationColumnDrops(
   intended: ReadonlyArray<{ table: string; op?: { type?: string; column?: string } }>,
 ): string[] {
   const tables = new Set<string>();
   for (const d of intended) {
-    if (d.op?.type === 'drop_column' && d.op.column === ORGANIZATION_COLUMN && CEREMONY_TABLES.has(d.table)) tables.add(d.table);
+    if (d.op?.type === 'drop_column' && d.op.column === ORGANIZATION_COLUMN) tables.add(d.table);
   }
   return [...tables].sort();
 }

packages/cli/src/utils/organization-ownership-platform-global-column-drop.integration.test.ts

@@ -40,6 +40,20 @@
  *
  * Every case runs the real command, plan then apply, over a fresh copy of the
  * database; each fate the case does not exercise is `report`. SQLite only.
+ *
+ * ## `os migrate apply --allow-destructive` on the same composition
+ *
+ * The schema-drift report of this composition offers the two declared
+ * objects' `organization_id` for the destructive apply. Its drop gate
+ * (`organization-ownership-drop-gate.ts`, ADR-0131 D10 item 5) holds that drop
+ * back until the ceremony's marker is verified — on these host-declared
+ * objects as on the platform's inventory, since the ceremony plans every
+ * table that carries the column. The real `os migrate apply` command, over a
+ * fresh copy of the same v17 database:
+ *
+ *  - no marker: refused, naming the ceremony; nothing dropped;
+ *  - CONTROL: a verified marker: the drop applies;
+ *  - CONTROL: a destructive drop of another column is unchanged.
  */
 
 import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
@@ -50,7 +64,9 @@ import { randomBytes } from 'node:crypto';
 import { tmpdir } from 'node:os';
 import { dirname, join, resolve } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
+import { ORGANIZATION_OWNERSHIP_MIGRATION_ID } from '@objectstack/spec/system';
 import { isExitSignal } from './format.js';
+import MigrateApply from '../commands/migrate/apply.js';
 import MigrateOrganizationOwnership from '../commands/migrate/organization-ownership.js';
 
 // Paid at module load, not inside a clocked case (`check:test-source-alias`).
@@ -248,6 +264,74 @@ async function ceremony(name: string, drop: string): Promise<Run> {
   }
 }
 
+/** A fresh copy of the v17 database for one `os migrate apply` case, with `prepare` run on it raw. */
+function applyCase(name: string, prepare?: (db: Database.Database) => void): string {
+  const caseDir = join(dir, 'apply-cases', name);
+  mkdirSync(caseDir, { recursive: true });
+  const dbFile = join(caseDir, 'v17.db');
+  copyFileSync(templateDb, dbFile);
+  if (prepare) {
+    const db = new Database(dbFile);
+    try {
+      prepare(db);
+    } finally {
+      db.close();
+    }
+  }
+  return dbFile;
+}
+
+/** The real `os migrate apply --allow-destructive --yes --json` over `dbFile`, from the host's project. */
+async function migrateApply(dbFile: string): Promise<{ payload: any; exitCode: number }> {
+  const savedExit = process.exitCode;
+  const swallow = ((_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;
+  const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(swallow);
+  vi.spyOn(process.stderr, 'write').mockImplementation(swallow);
+  vi.spyOn(console, 'log').mockImplementation(() => {});
+  vi.spyOn(console, 'warn').mockImplementation(() => {});
+  vi.spyOn(console, 'error').mockImplementation(() => {});
+  // `run()` ends the one-shot process itself once the document is written.
+  let processExit: number | undefined;
+  vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
+    processExit = code ?? 0;
+    throw new Error(`__PROCESS_EXIT__:${processExit}`);
+  }) as never);
+  process.chdir(projectDir);
+  let thrownExit: number | undefined;
+  try {
+    try {
+      await MigrateApply.run(['--database-url', `file:${dbFile}`, '--allow-destructive', '--yes', '--json'], { root: CLI_ROOT });
+    } catch (error) {
+      const trapped = error instanceof Error && error.message.startsWith('__PROCESS_EXIT__');
+      if (!trapped && !isExitSignal(error)) throw error;
+      if (!trapped) thrownExit = (error as { oclif?: { exit?: number } }).oclif?.exit;
+    }
+    const out = stdout.mock.calls.map((c) => String(c[0])).join('').trim();
+    let payload: any;
+    try {
+      payload = JSON.parse(out);
+    } catch {
+      payload = { unparsed: out.slice(-600) };
+    }
+    return { payload, exitCode: processExit ?? thrownExit ?? (process.exitCode as number | undefined) ?? 0 };
+  } finally {
+    process.chdir(savedCwd);
+    process.exitCode = savedExit;
+    vi.restoreAllMocks();
+  }
+}
+
+/** The `organization_id` drops among an apply payload's entries, by table, sorted. */
+const organizationDropsIn = (entries: any[] | undefined): string[] =>
+  (entries ?? [])
+    .filter((d) => d?.op?.type === 'drop_column' && d.op.column === 'organization_id')
+    .map((d) => String(d.table))
+    .sort();
+
 const columnDrops = (report: any): { status: string; summary: Record<string, number>; blockers: string[] } =>
   report.steps.find((s: { step: string }) => s.step === 'column-drops');
 const blockersFor = (report: any, table: string): string[] =>
@@ -337,3 +421,66 @@ describe('organization-ownership --apply: column-drop on an object the host decl
     expect(exitCode).toBe(1);
   }, CASE_TIMEOUT_MS);
 });
+
+describe('os migrate apply --allow-destructive: the organization_id drop on a host-declared platform-global object waits for the ceremony', () => {
+  it('holds the drop back until the marker is verified, naming the ceremony, and drops nothing', async () => {
+    const dbFile = applyCase('no-marker');
+    const { payload, exitCode } = await migrateApply(dbFile);
+
+    const seen = { message: payload.message, error: payload.error, appliedOrganizationDrops: organizationDropsIn(payload.applied) };
+    expect(exitCode, JSON.stringify(seen)).toBe(1);
+    expect(payload).toMatchObject({
+      message: 'refused_organization_ownership_unmigrated',
+      tables: [...DECLARED].sort(),
+      applied: [],
+      created: [],
+      marker: null,
+    });
+    expect(payload.detail).toMatch(/os migrate organization-ownership --apply/);
+    // The drops the refusal held back are the ones the drift offered.
+    expect(organizationDropsIn(payload.skipped)).toEqual([...DECLARED].sort());
+    for (const table of HOST_OBJECTS) expect(hasOrganizationColumn(dbFile, table), table).toBe(true);
+  }, CASE_TIMEOUT_MS);
+
+  it('CONTROL: once the ceremony\'s marker is verified, the drop applies', async () => {
+    const dbFile = applyCase('verified-marker', (db) => {
+      const now = new Date().toISOString();
+      db.prepare('INSERT INTO sys_migration (id, last_run_at, verified_at, blocking) VALUES (?, ?, ?, 0)').run(
+        ORGANIZATION_OWNERSHIP_MIGRATION_ID,
+        now,
+        now,
+      );
+    });
+    const { payload, exitCode } = await migrateApply(dbFile);
+
+    expect(exitCode, JSON.stringify(payload).slice(0, 600)).toBe(0);
+    expect(payload.message).toBeUndefined();
+    expect(organizationDropsIn(payload.applied)).toEqual([...DECLARED].sort());
+    for (const table of DECLARED) expect(hasOrganizationColumn(dbFile, table), table).toBe(false);
+    // The undeclared object's column is the release's, and stays.
+    expect(hasOrganizationColumn(dbFile, AUDIT)).toBe(true);
+  }, CASE_TIMEOUT_MS);
+
+  it('CONTROL: a destructive drop of another column is unchanged', async () => {
+    // The declared objects' organization_id already gone, and a column no
+    // object declares on one of them: the run's one destructive change.
+    const dbFile = applyCase('other-column', (db) => {
+      for (const table of DECLARED) db.exec(`ALTER TABLE "${table}" DROP COLUMN organization_id`);
+      db.exec(`ALTER TABLE "${LEDGER}" ADD COLUMN legacy_23684_note TEXT`);
+    });
+    const { payload, exitCode } = await migrateApply(dbFile);
+
+    expect(exitCode, JSON.stringify(payload).slice(0, 600)).toBe(0);
+    expect(payload.message).toBeUndefined();
+    expect(
+      (payload.applied as any[]).filter((d) => d?.op?.type === 'drop_column').map((d) => `${d.table}.${d.op.column}`),
+    ).toEqual([`${LEDGER}.legacy_23684_note`]);
+    const db = new Database(dbFile, { readonly: true });
+    try {
+      const columns = (db.prepare(`PRAGMA table_info("${LEDGER}")`).all() as Array<{ name: string }>).map((c) => c.name);
+      expect(columns).not.toContain('legacy_23684_note');
+    } finally {
+      db.close();
+    }
+  }, CASE_TIMEOUT_MS);
+});