From ad8a150e9d2259aa689e39a386aad06a43beef31 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 06:40:08 +0000 Subject: [PATCH 1/7] feat(cli): --apply executes only the reviewed plan file (ADR-0131 D10, C7b S4) --apply requires --plan-file and refuses, before any write, when the inventory digest it computes or the database differs from the reviewed plan's; a checkpoint under another digest is superseded and reported. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/15211-cli-apply-reviewed-plan.md | 25 ++++ .../22937-cli-organization-ownership-apply.md | 2 +- ...organization-ownership.apply-flags.test.ts | 55 +++++++- .../migrate/organization-ownership.ts | 62 ++++++++- .../organization-ownership-apply.test.ts | 96 ++++++++++++- .../src/utils/organization-ownership-apply.ts | 127 +++++++++++++++++- 6 files changed, 357 insertions(+), 10 deletions(-) create mode 100644 .changeset/15211-cli-apply-reviewed-plan.md diff --git a/.changeset/15211-cli-apply-reviewed-plan.md b/.changeset/15211-cli-apply-reviewed-plan.md new file mode 100644 index 00000000000..f9237102265 --- /dev/null +++ b/.changeset/15211-cli-apply-reviewed-plan.md @@ -0,0 +1,25 @@ +--- +'@objectstack/cli': patch +--- + +feat(cli): `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10) + +Clause-②: no + +`--apply` now requires `--plan-file FILE`, the plan file a plan run wrote. Before it writes anything it compares that file with the plan it computes, and refuses when either differs: + +- **Another inventory digest** (`plan-digest-mismatch`): the fates it would run are not the ones the reviewed plan showed. That happens with a different `--fate-supplement`, or with another release's inventory. Without this check, planning with supplement A and applying with supplement B ran B's fates unreviewed, including a mirror deletion A never listed. +- **Another database** (`plan-database-mismatch`). + +The refusal names both digests (or both databases) and the remedy: run the plan again with the same flags, review it, and pass the new file. No flag skips the comparison. + +A file that is not an organization-ownership plan, or cannot be read, is refused as `plan-file-unusable`. `--apply` without the flag is refused as `plan-file-required`. Both refusals come before the backup question and before the database is opened. + +A checkpoint recorded under another digest is no longer discarded silently. The reviewed plan supersedes it: every step runs again (each is idempotent), and the apply report names the discarded checkpoint in `restartedFrom`, beside `reviewedPlan`. + +`--apply` has not shipped in a release yet (its changeset is still pending), so no released invocation changes. The one-line form is: + +```bash +os migrate organization-ownership --out plan.json # plan, then review plan.json +os migrate organization-ownership --apply --plan-file plan.json --backup-taken # perform exactly that plan +``` diff --git a/.changeset/22937-cli-organization-ownership-apply.md b/.changeset/22937-cli-organization-ownership-apply.md index 4b2c410f716..2dfb22ac222 100644 --- a/.changeset/22937-cli-organization-ownership-apply.md +++ b/.changeset/22937-cli-organization-ownership-apply.md @@ -21,5 +21,5 @@ The command that planned the ceremony now performs it. Without `--apply` nothing ```bash os migrate organization-ownership --out plan.json # read the plan first -os migrate organization-ownership --apply --backup-taken # then perform it +os migrate organization-ownership --apply --plan-file plan.json --backup-taken # then perform exactly that plan ``` diff --git a/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts b/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts index beeef571143..6593ae64c29 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts @@ -4,10 +4,16 @@ * `os migrate organization-ownership --apply` asks about the backup before it * opens the database (ADR-0131 D10 ceremony item 2): without `--backup-taken` * a non-interactive run is refused, and the boot never happens. The flag means - * nothing without `--apply`, and is refused there too. + * nothing without `--apply`, and is refused there too. Before the backup + * question, `--apply` requires the plan file the operator reviewed + * (`--plan-file`), readable as a plan; the digest comparison itself is pinned + * in `organization-ownership-apply.test.ts`. */ import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; const boot = vi.hoisted(() => vi.fn(async () => { @@ -28,6 +34,19 @@ const CASE_TIMEOUT_MS = 60_000; const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); +const PLAN_DIR = mkdtempSync(join(tmpdir(), 'os-c7b-plan-file-')); +/** A plan file as a plan run writes it (the fields the apply reads). */ +const PLAN_FILE = join(PLAN_DIR, 'plan.json'); +writeFileSync(PLAN_FILE, JSON.stringify({ + kind: 'organization-ownership-plan', + ceremony: 'ADR-0131 D10', + generatedAt: '2026-10-14T00:00:00Z', + database: 'fixture.db', + inventoryDigest: 'f'.repeat(64), +})); +const NOT_A_PLAN = join(PLAN_DIR, 'report.json'); +writeFileSync(NOT_A_PLAN, JSON.stringify({ kind: 'organization-ownership-apply' })); + async function runJson(argv: string[]): Promise<{ payload: Record; exitCode: unknown }> { const chunks: string[] = []; const savedExit = process.exitCode; @@ -53,9 +72,39 @@ afterEach(() => { boot.mockClear(); }); +describe('os migrate organization-ownership --apply — the reviewed plan file', () => { + it('refuses --apply without --plan-file, before the backup question and the boot', async () => { + const { payload, exitCode } = await runJson(['--apply', '--backup-taken', '--json']); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'plan-file-required' }); + expect(exitCode).toBe(1); + expect(boot).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); + + it('refuses a --plan-file that is not a plan, naming it, before the boot', async () => { + const { payload, exitCode } = await runJson(['--apply', '--plan-file', NOT_A_PLAN, '--backup-taken', '--json']); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'plan-file-unusable', file: NOT_A_PLAN }); + expect(exitCode).toBe(1); + expect(boot).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); + + it('refuses a --plan-file that does not exist, before the boot', async () => { + const missing = join(PLAN_DIR, 'missing.json'); + const { payload } = await runJson(['--apply', '--plan-file', missing, '--backup-taken', '--json']); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'plan-file-unusable', file: missing }); + expect(boot).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); + + it('refuses --plan-file without --apply, before the boot', async () => { + const { payload, exitCode } = await runJson(['--plan-file', PLAN_FILE, '--json']); + expect(payload).toMatchObject({ error: 'plan_refused', reason: 'flag-without-apply' }); + expect(exitCode).toBe(1); + expect(boot).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); +}); + describe('os migrate organization-ownership --apply — the backup precondition', () => { it('refuses a non-interactive --apply without --backup-taken, before the boot', async () => { - const { payload, exitCode } = await runJson(['--apply', '--json']); + const { payload, exitCode } = await runJson(['--apply', '--plan-file', PLAN_FILE, '--json']); expect(payload).toMatchObject({ error: 'apply_refused', reason: 'backup-unacknowledged' }); expect(exitCode).toBe(1); expect(boot).not.toHaveBeenCalled(); @@ -69,7 +118,7 @@ describe('os migrate organization-ownership --apply — the backup precondition' }, CASE_TIMEOUT_MS); it('with --backup-taken, goes on to the boot', async () => { - const { payload } = await runJson(['--apply', '--backup-taken', '--json']); + const { payload } = await runJson(['--apply', '--plan-file', PLAN_FILE, '--backup-taken', '--json']); expect(payload).toMatchObject({ error: 'boot_failed', reason: 'boot-failed' }); expect(boot).toHaveBeenCalledTimes(1); expect(boot.mock.calls[0]).toEqual([expect.objectContaining({ deferSchemaDdl: true, composeHostStack: true })]); diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index 3db1862da26..1dbe8d73a2f 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -1,6 +1,6 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -import { writeFile } from 'node:fs/promises'; +import { readFile, writeFile } from 'node:fs/promises'; import { resolve } from 'node:path'; import { createInterface } from 'node:readline'; import { Command, Flags } from '@oclif/core'; @@ -34,11 +34,14 @@ import { applyOrganizationOwnership, checkpointFromDetails, ORGANIZATION_OWNERSHIP_MIGRATION_ID, + ReviewedPlanRefusal, + reviewedPlanFromFile, TemplateChoiceRefusal, type ApplyExecutor, type CeremonyLedgerPort, type ColumnDropOutcome, type OrganizationOwnershipApplyReport, + type ReviewedPlan, } from '../../utils/organization-ownership-apply.js'; /** Who the ceremony's metadata saves name as their actor. */ @@ -100,6 +103,17 @@ async function confirm(question: string): Promise { * `verified_at` when the post-check also reads zero discrepancies. A report * file is written beside the plan's, never over an existing one. * + * ## `--plan-file FILE` (the plan the operator reviewed) + * + * `--apply` requires the plan file a plan run wrote, and executes only that + * plan: it refuses, before anything is written, when the inventory digest it + * computes differs from the file's (a different `--fate-supplement`, another + * release) or the file names another database. Both digests and the remedy — + * plan again, review, hand the new file in — are named. A checkpoint recorded + * under another digest is superseded by the reviewed plan: the run starts over + * and its report says so (`restartedFrom`). There is no flag that skips the + * comparison. + * * ## `--fate-supplement FILE` (the host's own objects, ADR-0131 D10) * * The inventory holds the platform's fates. An object a HOST provides — a @@ -136,7 +150,7 @@ export default class MigrateOrganizationOwnership extends Command { '$ os migrate organization-ownership', '$ os migrate organization-ownership --out plans/v18-ownership.json', '$ os migrate organization-ownership --database-url postgres://… --json', - '$ os migrate organization-ownership --apply --backup-taken', + '$ os migrate organization-ownership --apply --plan-file plans/v18-ownership.json --backup-taken', '$ os migrate organization-ownership --fate-supplement host-fates.json', ]; @@ -158,6 +172,12 @@ export default class MigrateOrganizationOwnership extends Command { 'post-check. Idempotent; an interrupted run resumes from its recorded checkpoint. Requires a backup', default: false, }), + 'plan-file': Flags.string({ + description: + 'With --apply (required): the plan file a plan run wrote and you reviewed. The run refuses, before any write, ' + + 'when its inventory digest differs from that file\'s (another --fate-supplement, another release) or the file ' + + 'names another database', + }), 'backup-taken': Flags.boolean({ description: 'With --apply: state that a backup of this database has been taken. The platform takes none and assumes ' + @@ -235,6 +255,10 @@ export default class MigrateOrganizationOwnership extends Command { await refuse({ reason: 'flag-without-apply', detail: '--template-choice is only meaningful with --apply' }, '--template-choice is only meaningful with --apply.'); return; } + if (flags['plan-file']) { + await refuse({ reason: 'flag-without-apply', detail: '--plan-file is only meaningful with --apply' }, '--plan-file is only meaningful with --apply.'); + return; + } if (flags['backup-taken']) { await refuse({ reason: 'flag-without-apply', detail: '--backup-taken is only meaningful with --apply' }, '--backup-taken is only meaningful with --apply.'); return; @@ -349,13 +373,40 @@ export default class MigrateOrganizationOwnership extends Command { /** `--apply`: the backup precondition, the boot, the ceremony, the report file. */ private async runApply( - flags: { 'database-url'?: string; out?: string; json?: boolean; 'backup-taken'?: boolean; 'template-choice'?: string[] }, + flags: { + 'database-url'?: string; + out?: string; + json?: boolean; + 'backup-taken'?: boolean; + 'template-choice'?: string[]; + 'plan-file'?: string; + }, posture: string, refuse: (payload: Record, message: string) => Promise, inventory: readonly InventoryEntry[] | undefined, ): Promise { const json = Boolean(flags.json); const timer = createTimer(); + // The reviewed plan, read before the backup question and the boot. + if (!flags['plan-file']) { + await refuse( + { reason: 'plan-file-required', detail: 'pass --plan-file with the plan file you reviewed' }, + 'Refused: --apply executes only a plan you reviewed. Run the plan first (os migrate organization-ownership, with ' + + 'the same --fate-supplement), review the file it writes, then re-run --apply with --plan-file FILE.', + ); + return; + } + let reviewedPlan: ReviewedPlan; + const planFile = resolve(flags['plan-file']); + try { + reviewedPlan = reviewedPlanFromFile(planFile, await readFile(planFile, 'utf8')); + } catch (error) { + const refusal = error instanceof ReviewedPlanRefusal + ? error + : new ReviewedPlanRefusal('plan-file-unusable', `The plan file ${planFile} cannot be read: ${(error as Error).message}.`, { file: planFile }); + await refuse({ reason: refusal.reason, ...refusal.detail, detail: refusal.message }, `Refused: ${refusal.message}`); + return; + } if (!flags['backup-taken']) { const target = flags['database-url'] ?? process.env.OS_DATABASE_URL ?? 'the project database'; const yes = !json && (await confirm( @@ -506,11 +557,16 @@ export default class MigrateOrganizationOwnership extends Command { }, }, ledger, + reviewedPlan, ...(inventory ? { inventory } : {}), ...(json ? {} : { onProgress: (message: string) => printStep(message) }), }); } catch (error: unknown) { if (isExitSignal(error)) throw error; + if (error instanceof ReviewedPlanRefusal) { + await refuse({ reason: error.reason, ...error.detail, detail: error.message }, `Refused: ${error.message}`); + return; + } if (error instanceof TemplateChoiceRefusal) { await refuse({ reason: 'template-choice-refused', detail: error.message }, `Refused: ${error.message}`); return; diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index 388fad9ee3d..909f3f12157 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -22,8 +22,11 @@ import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { permissionSetDefinitionFromRow } from '@objectstack/core'; +import { inventoryDigest } from './organization-ownership-plan.js'; import { applyOrganizationOwnership, + ReviewedPlanRefusal, + reviewedPlanFromFile, TemplateChoiceRefusal, type ApplyExecutor, type CatalogPort, @@ -32,6 +35,7 @@ import { type ColumnDropPort, type MetadataDoorPort, type OrganizationOwnershipApplyReport, + type ReviewedPlan, type TemplatePort, } from './organization-ownership-apply.js'; @@ -285,10 +289,18 @@ async function accessAfter(c: ReturnType): Promise { const apply = ( c: ReturnType, l: ReturnType, - opts: { posture?: string; executor?: ApplyExecutor; templateChoices?: string[] } = {}, + opts: { posture?: string; executor?: ApplyExecutor; templateChoices?: string[]; reviewed?: Partial } = {}, ): Promise => { c.boot(); return applyOrganizationOwnership({ + // The plan the operator reviewed: by default, the one this run computes. + reviewedPlan: { + file: 'plan.json', + generatedAt: '2026-10-11T00:00:00Z', + database: file, + inventoryDigest: inventoryDigest(), + ...opts.reviewed, + }, executor: opts.executor ?? executor(), ...(opts.templateChoices ? { templateChoices: opts.templateChoices } : {}), posture: opts.posture ?? 'single', @@ -444,6 +456,88 @@ describe('ADR-0131 D10 --apply — the acceptance fixture', () => { }); }); +describe('ADR-0131 D10 --apply — only the reviewed plan is executed', () => { + it('refuses, before any write, a plan file whose inventory digest is not the one this run computes', async () => { + const c = catalog(); + const l = ledger(); + const hash = createHash('sha256').update(readFileSync(file)).digest('hex'); + + const refused = apply(c, l, { reviewed: { inventoryDigest: 'a'.repeat(64) } }); + + await expect(refused).rejects.toBeInstanceOf(ReviewedPlanRefusal); + await expect(refused).rejects.toMatchObject({ + reason: 'plan-digest-mismatch', + detail: { reviewed: 'a'.repeat(64), current: inventoryDigest() }, + }); + expect(writes).toEqual([]); + expect(l.checkpoint).toBeNull(); + expect(l.verdicts).toEqual([]); + expect(createHash('sha256').update(readFileSync(file)).digest('hex')).toBe(hash); + }); + + it('refuses, before any write, a plan file computed for another database', async () => { + const c = catalog(); + const l = ledger(); + + await expect(apply(c, l, { reviewed: { database: '/elsewhere/other.db' } })).rejects.toMatchObject({ + reason: 'plan-database-mismatch', + detail: { reviewed: '/elsewhere/other.db', current: file }, + }); + expect(writes).toEqual([]); + expect(l.checkpoint).toBeNull(); + }); + + it('a checkpoint recorded under another digest is superseded by the reviewed plan: every step runs again, and the report names it', async () => { + const c = catalog(); + const l = ledger(); + l.checkpoint = { + ceremony: 'ADR-0131 D10', + inventoryDigest: 'b'.repeat(64), + completed: { attribution: { at: '2026-10-10T00:00:00Z', summary: {} }, rewrite: { at: '2026-10-10T00:00:00Z', summary: {} } }, + }; + + const report = await apply(c, l); + + expect(report.restartedFrom).toEqual({ inventoryDigest: 'b'.repeat(64), steps: ['attribution', 'rewrite'] }); + expect(report.resumedFrom).toEqual([]); + expect(report.steps.find((s) => s.step === 'attribution')!.status).toBe('done'); + expect(report.reviewedPlan).toEqual({ file: 'plan.json', generatedAt: '2026-10-11T00:00:00Z' }); + expect(l.checkpoint!.inventoryDigest).toBe(inventoryDigest()); + }); + + it('a resumed run under the same digest reports no restart', async () => { + const c = catalog(); + const l = ledger(); + await apply(c, l); + const again = await apply(c, l); + expect(again.restartedFrom).toBeUndefined(); + }); +}); + +describe('reviewedPlanFromFile', () => { + const plan = { kind: 'organization-ownership-plan', ceremony: 'ADR-0131 D10', generatedAt: 'g', database: 'd', inventoryDigest: 'x' }; + + it('reads the three compared fields of a plan file', () => { + expect(reviewedPlanFromFile('p.json', JSON.stringify({ ...plan, tables: [] }))).toEqual({ + file: 'p.json', generatedAt: 'g', database: 'd', inventoryDigest: 'x', + }); + }); + + it.each([ + ['not JSON', '{'], + ['an apply report, not a plan', JSON.stringify({ ...plan, kind: 'organization-ownership-apply' })], + ['no digest', JSON.stringify({ ...plan, inventoryDigest: '' })], + ['no database', JSON.stringify({ ...plan, database: undefined })], + ])('refuses a file that is %s', (_label, text) => { + expect(() => reviewedPlanFromFile('p.json', text)).toThrow(ReviewedPlanRefusal); + try { + reviewedPlanFromFile('p.json', text); + } catch (error) { + expect(error).toMatchObject({ reason: 'plan-file-unusable', detail: { file: 'p.json' } }); + } + }); +}); + describe('ADR-0131 D10 --apply — idempotent, resumable, gated', () => { it('a second run resumes from the checkpoint and writes nothing', async () => { const c = catalog(); diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 5805c2d00e2..2ae887f9298 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -88,6 +88,19 @@ import { * blocker stops the ceremony there — fate order is the contract — and the * report names the blocker and its remedy. * + * ## Only the plan the operator reviewed + * + * The apply executes the fates of the plan it computes now, and that plan's + * inputs can move between the review and the run: a host fate supplement + * swapped for another, a release whose inventory moved. So the apply takes + * the kept plan file ({@link ReviewedPlan}) and refuses, before anything is + * written, when the inventory digest it computes is not the one that file + * recorded, or the file names another database ({@link ReviewedPlanRefusal}). + * The remedy is always the same: plan again, review it, hand the new file in. + * A checkpoint recorded under another digest is therefore one the reviewed + * plan supersedes: the run starts over (each step is idempotent), and the + * report names the discarded checkpoint ({@link OrganizationOwnershipApplyReport.restartedFrom}). + * * ## What it never does * * ⛔ Delete a row outside fate 2. ⛔ Give an unattributable row an owner: @@ -154,6 +167,72 @@ export interface CeremonyLedgerPort { write(checkpoint: CeremonyCheckpoint, verdict: { complete: boolean; blocking: number; advisory: number }): Promise; } +/** + * What the apply reads from the kept `--plan` file: the plan the operator + * reviewed. Only these three fields are compared or reported. + */ +export interface ReviewedPlan { + /** The plan file's path, for the refusal and the report. */ + file: string; + generatedAt: string; + database: string; + inventoryDigest: string; +} + +/** + * The apply would execute fates other than the reviewed plan's, or against + * another database. Thrown before anything is written; the remedy is a new + * plan, reviewed and handed in. + */ +export class ReviewedPlanRefusal extends Error { + constructor( + readonly reason: 'plan-file-unusable' | 'plan-digest-mismatch' | 'plan-database-mismatch', + message: string, + readonly detail: Readonly> = {}, + ) { + super(message); + this.name = 'ReviewedPlanRefusal'; + } +} + +/** + * Parse the kept plan file into a {@link ReviewedPlan}, or refuse naming the + * file: not JSON, not an organization-ownership plan, or missing the digest or + * the database it was computed for. + */ +export function reviewedPlanFromFile(file: string, text: string): ReviewedPlan { + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch (error) { + throw new ReviewedPlanRefusal( + 'plan-file-unusable', + `The plan file ${file} is not JSON (${error instanceof Error ? error.message : String(error)}). Hand in the file ` + + 'the plan run wrote.', + { file }, + ); + } + const p = parsed as Partial> | null; + if ( + !p || + typeof p !== 'object' || + p.kind !== 'organization-ownership-plan' || + p.ceremony !== 'ADR-0131 D10' || + typeof p.inventoryDigest !== 'string' || + !p.inventoryDigest || + typeof p.database !== 'string' || + typeof p.generatedAt !== 'string' + ) { + throw new ReviewedPlanRefusal( + 'plan-file-unusable', + `The file ${file} is not an organization-ownership plan (it needs kind, ceremony, inventoryDigest, database and ` + + 'generatedAt). Hand in the file the plan run wrote.', + { file }, + ); + } + return { file, generatedAt: p.generatedAt, database: p.database, inventoryDigest: p.inventoryDigest }; +} + export interface StepReport { step: CeremonyStep; /** `checkpointed`: finished by an earlier run; `not-reached`: an earlier step stopped the run. */ @@ -200,6 +279,13 @@ export interface OrganizationOwnershipApplyReport { database: string; posture: string; inventoryDigest: string; + /** The plan file this run executed: its digest equals {@link inventoryDigest}. */ + reviewedPlan: { file: string; generatedAt: string }; + /** + * A checkpoint recorded under another inventory digest, which the reviewed + * plan superseded: its steps were run again from the start. + */ + restartedFrom?: { inventoryDigest: string; steps: CeremonyStep[] }; /** Steps an earlier run finished, read from the checkpoint. */ resumedFrom: CeremonyStep[]; steps: StepReport[]; @@ -221,6 +307,8 @@ export interface ApplyOptions { templates: TemplatePort; columnDrops: ColumnDropPort; ledger: CeremonyLedgerPort; + /** The kept plan file the operator reviewed; the run refuses unless it matches (see the module doc). */ + reviewedPlan: ReviewedPlan; inventory?: readonly InventoryEntry[]; /** * The operator's choice for each template conflict: one candidate row id per @@ -1165,6 +1253,32 @@ async function remainingFor(run: Run, table: TablePlan, complete: boolean): Prom } } +/** + * Refuse, before anything is written, an apply whose plan is not the reviewed + * one: another inventory digest (another supplement, another release's + * inventory), or another database. + */ +function refuseUnreviewedPlan(plan: OrganizationOwnershipPlan, reviewed: ReviewedPlan): void { + if (reviewed.inventoryDigest !== plan.inventoryDigest) { + throw new ReviewedPlanRefusal( + 'plan-digest-mismatch', + `The plan file ${reviewed.file} was computed from inventory ${reviewed.inventoryDigest}, and this run computes ` + + `inventory ${plan.inventoryDigest}: the fates it would execute are not the ones that plan showed (a different ` + + '--fate-supplement, or another release). Nothing was written. Run the plan again with the same flags, review ' + + 'it, and pass the new file with --plan-file.', + { file: reviewed.file, reviewed: reviewed.inventoryDigest, current: plan.inventoryDigest }, + ); + } + if (reviewed.database !== plan.database) { + throw new ReviewedPlanRefusal( + 'plan-database-mismatch', + `The plan file ${reviewed.file} was computed for the database ${reviewed.database}, and this run opened ` + + `${plan.database}. Nothing was written. Plan this database, review it, and pass that file with --plan-file.`, + { file: reviewed.file, reviewed: reviewed.database, current: plan.database }, + ); + } +} + /** * Run the ceremony's apply: the fates in order, resuming from the checkpoint, * then the post-check. Throws only for a failure mid-write (the checkpoint @@ -1188,6 +1302,7 @@ export async function applyOrganizationOwnership(opts: ApplyOptions): Promise checkpoint.completed[s]); const steps: StepReport[] = []; - if (stored && resumedFrom.length === 0 && Object.keys(stored.completed ?? {}).length > 0) { - opts.onProgress?.('the inventory changed since the recorded checkpoint, so every step runs again (each is idempotent)'); + const superseded = stored && stored.inventoryDigest !== before.inventoryDigest + ? CEREMONY_STEPS.filter((s) => stored.completed?.[s]) + : []; + if (stored && superseded.length > 0) { + opts.onProgress?.( + `the recorded checkpoint (inventory ${stored.inventoryDigest}) is superseded by the reviewed plan ` + + `(inventory ${before.inventoryDigest}), so every step runs again (each is idempotent)`, + ); } let stopped = false; @@ -1285,6 +1406,8 @@ export async function applyOrganizationOwnership(opts: ApplyOptions): Promise 0 ? { restartedFrom: { inventoryDigest: stored.inventoryDigest, steps: superseded } } : {}), resumedFrom, steps, tables, From bc3525a9cf5887de573f74e38ff44d467384d3f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 07:33:04 +0000 Subject: [PATCH 2/7] chore(changeset): leave #22937's pending note as it is; its correction is the seat's call (C7b S4) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/22937-cli-organization-ownership-apply.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22937-cli-organization-ownership-apply.md b/.changeset/22937-cli-organization-ownership-apply.md index 2dfb22ac222..4b2c410f716 100644 --- a/.changeset/22937-cli-organization-ownership-apply.md +++ b/.changeset/22937-cli-organization-ownership-apply.md @@ -21,5 +21,5 @@ The command that planned the ceremony now performs it. Without `--apply` nothing ```bash os migrate organization-ownership --out plan.json # read the plan first -os migrate organization-ownership --apply --plan-file plan.json --backup-taken # then perform exactly that plan +os migrate organization-ownership --apply --backup-taken # then perform it ``` From 66b826f9d9b159bfaf180fc8abde3cde44ec0f16 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:17:04 +0000 Subject: [PATCH 3/7] chore(changeset): the pending apply note's example carries --plan-file (seat answer Q-S4 A, C7b S4) `--apply` now refuses without the reviewed plan file, so the example the compiled CHANGELOG prints names it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/22937-cli-organization-ownership-apply.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22937-cli-organization-ownership-apply.md b/.changeset/22937-cli-organization-ownership-apply.md index 4b2c410f716..19a10268f9f 100644 --- a/.changeset/22937-cli-organization-ownership-apply.md +++ b/.changeset/22937-cli-organization-ownership-apply.md @@ -21,5 +21,5 @@ The command that planned the ceremony now performs it. Without `--apply` nothing ```bash os migrate organization-ownership --out plan.json # read the plan first -os migrate organization-ownership --apply --backup-taken # then perform it +os migrate organization-ownership --apply --backup-taken --plan-file plan.json # then perform the plan you reviewed ``` From 0802a12756f042750737f571072111c396702f24 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:51:37 +0000 Subject: [PATCH 4/7] fix(cli): the apply refuses a plan reviewed under another tenancy posture; main's --apply callers hand in a plan file (C7b S4 review) - F2: the plan file's recorded posture is read and compared; plan-posture-mismatch names both postures and the remedy. - F1: the fate-supplement integration cases pass a plan file. - F3: the one-shot family's --apply write mode runs the plan first and hands its file in, so the seed-write and key-material pins reach the apply's boot again; a mode declaring preceding runs must reach its own write. - F4: #23280's pending note's example carries --plan-file (seat answer Q-S4 A, extended); #22937's note and the module doc say the plan file comes before the backup question. - The command's mapping of a post-boot plan refusal is pinned. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/15211-cli-apply-reviewed-plan.md | 5 ++- .../22937-cli-organization-ownership-apply.md | 2 +- .changeset/23280-host-fate-supplement.md | 2 +- ...organization-ownership.apply-flags.test.ts | 1 + ...ership.fate-supplement.integration.test.ts | 37 ++++++++++++++++- .../migrate/organization-ownership.ts | 10 +++-- .../organization-ownership-apply.test.ts | 29 +++++++++++-- .../src/utils/organization-ownership-apply.ts | 33 +++++++++++---- ...igrate.one-shot-family.integration.test.ts | 41 +++++++++++++++---- 9 files changed, 130 insertions(+), 30 deletions(-) diff --git a/.changeset/15211-cli-apply-reviewed-plan.md b/.changeset/15211-cli-apply-reviewed-plan.md index f9237102265..19ded5cdcbc 100644 --- a/.changeset/15211-cli-apply-reviewed-plan.md +++ b/.changeset/15211-cli-apply-reviewed-plan.md @@ -9,11 +9,12 @@ Clause-②: no `--apply` now requires `--plan-file FILE`, the plan file a plan run wrote. Before it writes anything it compares that file with the plan it computes, and refuses when either differs: - **Another inventory digest** (`plan-digest-mismatch`): the fates it would run are not the ones the reviewed plan showed. That happens with a different `--fate-supplement`, or with another release's inventory. Without this check, planning with supplement A and applying with supplement B ran B's fates unreviewed, including a mirror deletion A never listed. +- **Another tenancy posture** (`plan-posture-mismatch`): the plan file records the posture it was computed under (`OS_TENANCY_POSTURE` / `OS_MULTI_ORG_ENABLED`), and the posture decides which rows attribution gives an owner and which it reports. A plan reviewed under a wall, which lists rows as reported, would otherwise give them owners when applied under `single`. - **Another database** (`plan-database-mismatch`). -The refusal names both digests (or both databases) and the remedy: run the plan again with the same flags, review it, and pass the new file. No flag skips the comparison. +The refusal names both values and the remedy: run the plan again with the same flags (or run the apply under the plan's posture), review it, and pass the new file. No flag skips the comparison. -A file that is not an organization-ownership plan, or cannot be read, is refused as `plan-file-unusable`. `--apply` without the flag is refused as `plan-file-required`. Both refusals come before the backup question and before the database is opened. +A file that is not an organization-ownership plan (missing its digest, posture or database), or cannot be read, is refused as `plan-file-unusable`. `--apply` without the flag is refused as `plan-file-required`. Both refusals come before the backup question and before the database is opened. A checkpoint recorded under another digest is no longer discarded silently. The reviewed plan supersedes it: every step runs again (each is idempotent), and the apply report names the discarded checkpoint in `restartedFrom`, beside `reviewedPlan`. diff --git a/.changeset/22937-cli-organization-ownership-apply.md b/.changeset/22937-cli-organization-ownership-apply.md index 19a10268f9f..76c57152232 100644 --- a/.changeset/22937-cli-organization-ownership-apply.md +++ b/.changeset/22937-cli-organization-ownership-apply.md @@ -8,7 +8,7 @@ Clause-②: yes (widening) The command that planned the ceremony now performs it. Without `--apply` nothing changes: the read-only plan, written to a file. With `--apply`: -- **The backup comes first.** It is the operator's act: pass `--backup-taken`, or answer yes at the prompt. A non-interactive run without the flag is refused before the database is opened. +- **The reviewed plan, then the backup.** `--apply` takes the plan file the plan run wrote (`--plan-file FILE`) and executes only that plan. The backup is the operator's act: pass `--backup-taken`, or answer yes at the prompt. A run without the plan file, or a non-interactive run without `--backup-taken`, is refused before the database is opened. - **The fates run in order**, each step stopping the run where it meets a blocker: 1. **Attribution** — every NULL `organization_id` row the plan derives an owner for takes it (a parent, subject or holder anchor; under `single`, the Default Organization). A row with no derivable owner is never assigned one: it stays NULL and is listed by id. 2. **Authored catalog rows** — permission sets and positions an organization authored in Setup become environment metadata through the metadata door (under `single`; under a wall they are reported), with each position's binding rows carried into its `permissionSets`. diff --git a/.changeset/23280-host-fate-supplement.md b/.changeset/23280-host-fate-supplement.md index 66488a6a3b9..293877a69fd 100644 --- a/.changeset/23280-host-fate-supplement.md +++ b/.changeset/23280-host-fate-supplement.md @@ -17,5 +17,5 @@ The v18 organization-ownership ceremony refuses any platform-prefixed table that ```bash os migrate organization-ownership --fate-supplement host-fates.json --out plan.json -os migrate organization-ownership --fate-supplement host-fates.json --apply --backup-taken +os migrate organization-ownership --fate-supplement host-fates.json --apply --backup-taken --plan-file plan.json ``` diff --git a/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts b/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts index 6593ae64c29..e73d5a09d6b 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.apply-flags.test.ts @@ -43,6 +43,7 @@ writeFileSync(PLAN_FILE, JSON.stringify({ generatedAt: '2026-10-14T00:00:00Z', database: 'fixture.db', inventoryDigest: 'f'.repeat(64), + posture: 'single', })); const NOT_A_PLAN = join(PLAN_DIR, 'report.json'); writeFileSync(NOT_A_PLAN, JSON.stringify({ kind: 'organization-ownership-apply' })); diff --git a/packages/cli/src/commands/migrate/organization-ownership.fate-supplement.integration.test.ts b/packages/cli/src/commands/migrate/organization-ownership.fate-supplement.integration.test.ts index d042c190688..b2d30d28318 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.fate-supplement.integration.test.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.fate-supplement.integration.test.ts @@ -88,6 +88,23 @@ afterEach(() => { rmSync(dir, { recursive: true, force: true }); }); +/** + * A plan file as a plan run writes it: `--apply` executes only a reviewed plan. + * The apply step is mocked here, so only the file's parse runs. + */ +function writePlan(): string { + const file = join(dir, 'reviewed-plan.json'); + writeFileSync(file, JSON.stringify({ + kind: 'organization-ownership-plan', + ceremony: 'ADR-0131 D10', + generatedAt: '2026-10-13T00:00:00.000Z', + database: 'fixture.db', + inventoryDigest: 'f'.repeat(64), + posture: 'single', + })); + return file; +} + function writeSupplement(rows: unknown): string { const file = join(dir, 'host-fates.json'); writeFileSync(file, JSON.stringify(rows)); @@ -144,7 +161,7 @@ describe('the plan reads the host fate supplement through --fate-supplement', () describe('--apply hands the merged inventory to the apply step', () => { it('with --fate-supplement, the apply step receives the host row', async () => { const file = writeSupplement([{ object: 'sys_domain', fate: 'report', citation: CITE }]); - const { payload } = await runJson(['--apply', '--backup-taken', '--fate-supplement', file, ...out('apply.json')]); + const { payload } = await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--fate-supplement', file, ...out('apply.json')]); expect(payload).toMatchObject({ report: { kind: 'organization-ownership-apply' } }); expect(apply).toHaveBeenCalledTimes(1); const inventory = (apply.mock.calls[0]![0] as { inventory?: Array> }).inventory; @@ -152,12 +169,28 @@ describe('--apply hands the merged inventory to the apply step', () => { }, CASE_TIMEOUT_MS); it('control: without it, the apply step is handed no inventory and reads the platform\'s', async () => { - await runJson(['--apply', '--backup-taken', ...out('apply.json')]); + await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), ...out('apply.json')]); expect(apply).toHaveBeenCalledTimes(1); expect(apply.mock.calls[0]![0]).not.toHaveProperty('inventory'); }, CASE_TIMEOUT_MS); }); +describe('a refusal of the reviewed plan raised after the boot', () => { + it('reaches the operator as apply_refused with its reason and both values, never as a resumable failure', async () => { + const { ReviewedPlanRefusal } = await import('../../utils/organization-ownership-apply.js'); + apply.mockImplementation(async () => { + throw new ReviewedPlanRefusal('plan-posture-mismatch', 'computed under isolated, this run resolves single', { + file: 'reviewed-plan.json', + reviewed: 'isolated', + current: 'single', + }); + }); + const { payload, exitCode } = await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), ...out('apply.json')]); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'plan-posture-mismatch', reviewed: 'isolated', current: 'single' }); + expect(exitCode).toBe(1); + }, CASE_TIMEOUT_MS); +}); + describe('a supplement it cannot use is refused before the database is opened, inside the --json envelope', () => { it('a missing file (plan)', async () => { const file = join(dir, 'absent.json'); diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index 1dbe8d73a2f..514209c936b 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -90,9 +90,11 @@ async function confirm(question: string): Promise { * * ## `--apply` (ADR-0131 D10 ceremony items 2 to 4) * - * The writing mode. It asks about the backup first — the operator's act and - * the precondition (D10 item 2): `--backup-taken`, or a yes at the prompt; a - * non-interactive run without the flag is refused before the boot. It then + * The writing mode. It reads the reviewed plan file first (`--plan-file`, the + * file a plan run wrote; without it the run is refused before the boot), then + * asks about the backup — the operator's act and the precondition (D10 item + * 2): `--backup-taken`, or a yes at the prompt; a non-interactive run without + * the flag is refused before the boot. It then * performs the fates in order — attribution, the authored catalog conversion, * the template promotion, the settings global-rung move, the verified id→name * rewrite, mirror deletion, column drops — through `../../utils/organization-ownership-apply.ts`, @@ -142,7 +144,7 @@ export default class MigrateOrganizationOwnership extends Command { 'The v18 organization-ownership ceremony (ADR-0131 D10). Without --apply, the read-only plan: per table, the fate, ' + 'the row counts, the rows whose owner cannot be derived (by id), and which tables will receive NOT NULL, written ' + 'to a file — never a row or a column changed; it refuses, naming it, any table it cannot enumerate. With --apply ' + - '(and a backup): attribution, the authored catalog conversion, the template promotion, the settings move, the ' + + '(the reviewed plan file in --plan-file, and a backup), only that plan: attribution, the authored catalog conversion, the template promotion, the settings move, the ' + 'verified id-to-name rewrite, mirror deletion and column drops, in that order, ' + 'resumable from a recorded checkpoint, then the post-check.'; diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index 909f3f12157..c74ce528294 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -299,6 +299,7 @@ const apply = ( generatedAt: '2026-10-11T00:00:00Z', database: file, inventoryDigest: inventoryDigest(), + posture: opts.posture ?? 'single', ...opts.reviewed, }, executor: opts.executor ?? executor(), @@ -475,6 +476,25 @@ describe('ADR-0131 D10 --apply — only the reviewed plan is executed', () => { expect(createHash('sha256').update(readFileSync(file)).digest('hex')).toBe(hash); }); + it('refuses, before any write, a plan computed under another tenancy posture, naming both and the remedy', async () => { + const c = catalog(); + const l = ledger(); + const hash = createHash('sha256').update(readFileSync(file)).digest('hex'); + + // Reviewed under a wall, where anchorless rows are reported; run under single, where they would take an owner. + const refused = apply(c, l, { posture: 'single', reviewed: { posture: 'isolated' } }); + + await expect(refused).rejects.toBeInstanceOf(ReviewedPlanRefusal); + await expect(refused).rejects.toMatchObject({ + reason: 'plan-posture-mismatch', + detail: { reviewed: 'isolated', current: 'single' }, + message: expect.stringMatching(/computed under the isolated tenancy posture, and this run resolves single .*Nothing was written\..*--plan-file/s), + }); + expect(writes).toEqual([]); + expect(l.checkpoint).toBeNull(); + expect(createHash('sha256').update(readFileSync(file)).digest('hex')).toBe(hash); + }); + it('refuses, before any write, a plan file computed for another database', async () => { const c = catalog(); const l = ledger(); @@ -515,11 +535,13 @@ describe('ADR-0131 D10 --apply — only the reviewed plan is executed', () => { }); describe('reviewedPlanFromFile', () => { - const plan = { kind: 'organization-ownership-plan', ceremony: 'ADR-0131 D10', generatedAt: 'g', database: 'd', inventoryDigest: 'x' }; + const plan = { + kind: 'organization-ownership-plan', ceremony: 'ADR-0131 D10', generatedAt: 'g', database: 'd', inventoryDigest: 'x', posture: 'single', + }; - it('reads the three compared fields of a plan file', () => { + it('reads the compared fields of a plan file', () => { expect(reviewedPlanFromFile('p.json', JSON.stringify({ ...plan, tables: [] }))).toEqual({ - file: 'p.json', generatedAt: 'g', database: 'd', inventoryDigest: 'x', + file: 'p.json', generatedAt: 'g', database: 'd', inventoryDigest: 'x', posture: 'single', }); }); @@ -528,6 +550,7 @@ describe('reviewedPlanFromFile', () => { ['an apply report, not a plan', JSON.stringify({ ...plan, kind: 'organization-ownership-apply' })], ['no digest', JSON.stringify({ ...plan, inventoryDigest: '' })], ['no database', JSON.stringify({ ...plan, database: undefined })], + ['no posture', JSON.stringify({ ...plan, posture: undefined })], ])('refuses a file that is %s', (_label, text) => { expect(() => reviewedPlanFromFile('p.json', text)).toThrow(ReviewedPlanRefusal); try { diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 2ae887f9298..7ef15511e91 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -95,7 +95,9 @@ import { * swapped for another, a release whose inventory moved. So the apply takes * the kept plan file ({@link ReviewedPlan}) and refuses, before anything is * written, when the inventory digest it computes is not the one that file - * recorded, or the file names another database ({@link ReviewedPlanRefusal}). + * recorded, the posture it runs under is not the one the plan was computed + * under (the posture decides what attribution and the authored conversion + * write), or the file names another database ({@link ReviewedPlanRefusal}). * The remedy is always the same: plan again, review it, hand the new file in. * A checkpoint recorded under another digest is therefore one the reviewed * plan supersedes: the run starts over (each step is idempotent), and the @@ -169,7 +171,7 @@ export interface CeremonyLedgerPort { /** * What the apply reads from the kept `--plan` file: the plan the operator - * reviewed. Only these three fields are compared or reported. + * reviewed. Only these fields are compared or reported. */ export interface ReviewedPlan { /** The plan file's path, for the refusal and the report. */ @@ -177,6 +179,8 @@ export interface ReviewedPlan { generatedAt: string; database: string; inventoryDigest: string; + /** The tenancy posture the plan was computed under. */ + posture: string; } /** @@ -186,7 +190,7 @@ export interface ReviewedPlan { */ export class ReviewedPlanRefusal extends Error { constructor( - readonly reason: 'plan-file-unusable' | 'plan-digest-mismatch' | 'plan-database-mismatch', + readonly reason: 'plan-file-unusable' | 'plan-digest-mismatch' | 'plan-posture-mismatch' | 'plan-database-mismatch', message: string, readonly detail: Readonly> = {}, ) { @@ -221,16 +225,18 @@ export function reviewedPlanFromFile(file: string, text: string): ReviewedPlan { typeof p.inventoryDigest !== 'string' || !p.inventoryDigest || typeof p.database !== 'string' || - typeof p.generatedAt !== 'string' + typeof p.generatedAt !== 'string' || + typeof p.posture !== 'string' || + !p.posture ) { throw new ReviewedPlanRefusal( 'plan-file-unusable', - `The file ${file} is not an organization-ownership plan (it needs kind, ceremony, inventoryDigest, database and ` + - 'generatedAt). Hand in the file the plan run wrote.', + `The file ${file} is not an organization-ownership plan (it needs kind, ceremony, inventoryDigest, posture, ` + + 'database and generatedAt). Hand in the file the plan run wrote.', { file }, ); } - return { file, generatedAt: p.generatedAt, database: p.database, inventoryDigest: p.inventoryDigest }; + return { file, generatedAt: p.generatedAt, database: p.database, inventoryDigest: p.inventoryDigest, posture: p.posture }; } export interface StepReport { @@ -1256,7 +1262,7 @@ async function remainingFor(run: Run, table: TablePlan, complete: boolean): Prom /** * Refuse, before anything is written, an apply whose plan is not the reviewed * one: another inventory digest (another supplement, another release's - * inventory), or another database. + * inventory), another tenancy posture, or another database. */ function refuseUnreviewedPlan(plan: OrganizationOwnershipPlan, reviewed: ReviewedPlan): void { if (reviewed.inventoryDigest !== plan.inventoryDigest) { @@ -1269,6 +1275,17 @@ function refuseUnreviewedPlan(plan: OrganizationOwnershipPlan, reviewed: Reviewe { file: reviewed.file, reviewed: reviewed.inventoryDigest, current: plan.inventoryDigest }, ); } + if (reviewed.posture !== plan.posture) { + throw new ReviewedPlanRefusal( + 'plan-posture-mismatch', + `The plan file ${reviewed.file} was computed under the ${reviewed.posture} tenancy posture, and this run resolves ` + + `${plan.posture} (OS_TENANCY_POSTURE / OS_MULTI_ORG_ENABLED): the posture decides which rows attribution gives ` + + 'an owner and which it reports, and whether authored sets and positions are converted, so the fates it would ' + + 'execute are not the ones that plan showed. Nothing was written. Run the apply under the posture the plan was ' + + 'computed under, or plan again under this one, review it, and pass the new file with --plan-file.', + { file: reviewed.file, reviewed: reviewed.posture, current: plan.posture }, + ); + } if (reviewed.database !== plan.database) { throw new ReviewedPlanRefusal( 'plan-database-mismatch', diff --git a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts index 7c41063e255..375e54af38a 100644 --- a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts @@ -126,6 +126,12 @@ interface Mode { label: string; /** The argv after the command id. `@DB@` is the case's database URL, `@DIR@` the project directory. */ argv: string[]; + /** + * Runs of the same command that must precede this mode on the same case, as + * an operator's would (each a mode pinned as no-write itself), before any + * state is read. + */ + after?: string[][]; } interface Caller { @@ -199,9 +205,15 @@ const CALLERS: Record = { argv: ['--out', '@DIR@/organization-ownership-plan.json', '--database-url', '@DB@', '--json'], }], // `--apply` writes the ceremony's fates and its report file; never a seed. + // It executes only a plan the operator reviewed, so the plan run (the + // no-write mode above) writes that file first. write: [{ label: 'migrate organization-ownership --apply', - argv: ['--apply', '--backup-taken', '--out', '@DIR@/organization-ownership-apply.json', '--database-url', '@DB@', '--json'], + after: [['--out', '@DIR@/organization-ownership-reviewed-plan.json', '--database-url', '@DB@', '--json']], + argv: [ + '--apply', '--backup-taken', '--plan-file', '@DIR@/organization-ownership-reviewed-plan.json', + '--out', '@DIR@/organization-ownership-apply.json', '--database-url', '@DB@', '--json', + ], }], }, 'commands/migrate/plan.ts': { @@ -427,16 +439,22 @@ async function readSeededRows(dbFile: string): Promise { let caseSeq = 0; /** A fresh copy of the served database, and the argv with its placeholders filled. */ -function prepareCase(argv: string[], opts: { absent?: boolean } = {}): { dbFile: string; argv: string[] } { +function prepareCase(argv: string[], opts: { absent?: boolean } = {}): { dbFile: string; argv: string[]; fill: (argv: string[]) => string[] } { caseSeq += 1; const caseDir = join(dir, 'cases', String(caseSeq)); mkdirSync(caseDir, { recursive: true }); const dbFile = join(caseDir, 'app.db'); if (!opts.absent) copyFileSync(templateDb, dbFile); - return { - dbFile, - argv: argv.map((a) => a.replace('@DB@', `file:${dbFile}`).replace('@DIR@', caseDir)), - }; + const fill = (raw: string[]): string[] => raw.map((a) => a.replace('@DB@', `file:${dbFile}`).replace('@DIR@', caseDir)); + return { dbFile, argv: fill(argv), fill }; +} + +/** The runs a mode declares must precede it ({@link Mode.after}), on the same case. */ +async function runBefore(run: Invoke, c: { fill: (argv: string[]) => string[] }, after: string[][] | undefined): Promise { + for (const argv of after ?? []) { + const result = await runJson(run, c.fill(argv)); + expect(result.payload?.error, `a preceding run failed: ${JSON.stringify(result.payload).slice(0, 400)}`).toBeUndefined(); + } } beforeAll(async () => { @@ -535,12 +553,16 @@ describe('[#21391] a no-write mode does not bring a missing SQLite file into exi }); describe('[#21391] a write mode runs no seed write', () => { - it.each(WRITE)('$label', async ({ run, argv }) => { + it.each(WRITE)('$label', async ({ run, argv, after }) => { const c = prepareCase(argv); + await runBefore(run, c, after); const before = await readSeededRows(c.dbFile); expect(before).toEqual([expect.objectContaining({ name: 'Acme', status: 'won' })]); - await runJson(run, c.argv); + const result = await runJson(run, c.argv); + // A mode that declares its preceding runs reached its own write, never a + // refusal for want of them (the pin below would hold vacuously otherwise). + if (after) expect(result.payload?.error, JSON.stringify(result.payload).slice(0, 400)).toBeUndefined(); // A seed replay puts `won` back to `open` and bumps `updated_at`. expect(await readSeededRows(c.dbFile)).toEqual(before); @@ -624,8 +646,9 @@ describe('[#21471] no mode creates key material in an empty key home, in a devel expect(readdirSync(home)).toEqual(['dev-crypto-key']); }, CASE_TIMEOUT_MS); - it.each([...NO_WRITE, ...WRITE])('$label', async ({ run, argv }) => { + it.each([...NO_WRITE, ...WRITE])('$label', async ({ run, argv, ...mode }) => { const c = prepareCase(argv); + await runBefore(run, c, 'after' in mode ? mode.after : undefined); const result = await runJson(run, c.argv); // The run reached its report or its refusal, never a failed boot: a boot From 91db38b7c00ba76fbabf3244147ecdb835e7f4cc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:07:14 +0000 Subject: [PATCH 5/7] revert(changeset): leave the consumed #22937 and #23280 notes as they are (seat ruling, C7b S4 delta review) Both notes shipped in 18.0.0-next.1; this PR's own note states the refused invocation and its replacement instead. Q-S4 A is withdrawn. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/22937-cli-organization-ownership-apply.md | 4 ++-- .changeset/23280-host-fate-supplement.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/22937-cli-organization-ownership-apply.md b/.changeset/22937-cli-organization-ownership-apply.md index 76c57152232..4b2c410f716 100644 --- a/.changeset/22937-cli-organization-ownership-apply.md +++ b/.changeset/22937-cli-organization-ownership-apply.md @@ -8,7 +8,7 @@ Clause-②: yes (widening) The command that planned the ceremony now performs it. Without `--apply` nothing changes: the read-only plan, written to a file. With `--apply`: -- **The reviewed plan, then the backup.** `--apply` takes the plan file the plan run wrote (`--plan-file FILE`) and executes only that plan. The backup is the operator's act: pass `--backup-taken`, or answer yes at the prompt. A run without the plan file, or a non-interactive run without `--backup-taken`, is refused before the database is opened. +- **The backup comes first.** It is the operator's act: pass `--backup-taken`, or answer yes at the prompt. A non-interactive run without the flag is refused before the database is opened. - **The fates run in order**, each step stopping the run where it meets a blocker: 1. **Attribution** — every NULL `organization_id` row the plan derives an owner for takes it (a parent, subject or holder anchor; under `single`, the Default Organization). A row with no derivable owner is never assigned one: it stays NULL and is listed by id. 2. **Authored catalog rows** — permission sets and positions an organization authored in Setup become environment metadata through the metadata door (under `single`; under a wall they are reported), with each position's binding rows carried into its `permissionSets`. @@ -21,5 +21,5 @@ The command that planned the ceremony now performs it. Without `--apply` nothing ```bash os migrate organization-ownership --out plan.json # read the plan first -os migrate organization-ownership --apply --backup-taken --plan-file plan.json # then perform the plan you reviewed +os migrate organization-ownership --apply --backup-taken # then perform it ``` diff --git a/.changeset/23280-host-fate-supplement.md b/.changeset/23280-host-fate-supplement.md index 293877a69fd..66488a6a3b9 100644 --- a/.changeset/23280-host-fate-supplement.md +++ b/.changeset/23280-host-fate-supplement.md @@ -17,5 +17,5 @@ The v18 organization-ownership ceremony refuses any platform-prefixed table that ```bash os migrate organization-ownership --fate-supplement host-fates.json --out plan.json -os migrate organization-ownership --fate-supplement host-fates.json --apply --backup-taken --plan-file plan.json +os migrate organization-ownership --fate-supplement host-fates.json --apply --backup-taken ``` From c07af11aa8ec63c94d856922bc53f8f9f48ec792 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:09:11 +0000 Subject: [PATCH 6/7] fix(cli)!: main's two new --apply callers hand in a reviewed plan; the note is minor with its BREAKING banner (C7b S4 delta review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F5: organization-ownership-not-declared runs the plan first and passes its file; organization-ownership-apply.served-catalog passes the reviewedPlan its run computes. - F6: --apply shipped in 18.0.0-next.1, so refusing its bare invocation is a narrowing and --plan-file a widening: minor, Clause-②: yes (widening), a BREAKING banner with the refused invocations and their replacement, and the ADR-0087 disposition. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/15211-cli-apply-reviewed-plan.md | 28 +++++++++++++------ ...p-apply.served-catalog.integration.test.ts | 3 ++ ...ownership-not-declared.integration.test.ts | 13 +++++++-- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/.changeset/15211-cli-apply-reviewed-plan.md b/.changeset/15211-cli-apply-reviewed-plan.md index 19ded5cdcbc..6951528c2bf 100644 --- a/.changeset/15211-cli-apply-reviewed-plan.md +++ b/.changeset/15211-cli-apply-reviewed-plan.md @@ -1,24 +1,34 @@ --- -'@objectstack/cli': patch +'@objectstack/cli': minor --- -feat(cli): `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10) +feat(cli)!: `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10) -Clause-②: no +Clause-②: yes (widening) -`--apply` now requires `--plan-file FILE`, the plan file a plan run wrote. Before it writes anything it compares that file with the plan it computes, and refuses when either differs: + -- **Another inventory digest** (`plan-digest-mismatch`): the fates it would run are not the ones the reviewed plan showed. That happens with a different `--fate-supplement`, or with another release's inventory. Without this check, planning with supplement A and applying with supplement B ran B's fates unreviewed, including a mirror deletion A never listed. -- **Another tenancy posture** (`plan-posture-mismatch`): the plan file records the posture it was computed under (`OS_TENANCY_POSTURE` / `OS_MULTI_ORG_ENABLED`), and the posture decides which rows attribution gives an owner and which it reports. A plan reviewed under a wall, which lists rows as reported, would otherwise give them owners when applied under `single`. -- **Another database** (`plan-database-mismatch`). +**BREAKING** (a narrowing of `--apply`'s accept set, released in 18.0.0-next.1), shipped as `minor` under the launch-window convention for breaking changes. It also widens the command: a new flag, `--plan-file`, new refusal reasons, and two new keys on the apply report. + +`--apply` now requires `--plan-file FILE`, the plan file a plan run wrote. Before it writes anything, it compares that file with the plan it computes, and refuses when any of these differs: + +- **The inventory digest** (`plan-digest-mismatch`). The fates it would run are not the ones the reviewed plan showed: a different `--fate-supplement`, or another release's inventory. Without this check, planning with supplement A and applying with supplement B ran B's fates unreviewed, including a mirror deletion A never listed. +- **The tenancy posture** (`plan-posture-mismatch`). The plan file records the posture it was computed under (`OS_TENANCY_POSTURE` / `OS_MULTI_ORG_ENABLED`), and the posture decides which rows attribution gives an owner and which it reports. A plan reviewed under a wall, which lists rows as reported, would otherwise give them owners when applied under `single`. +- **The database** (`plan-database-mismatch`). The refusal names both values and the remedy: run the plan again with the same flags (or run the apply under the plan's posture), review it, and pass the new file. No flag skips the comparison. -A file that is not an organization-ownership plan (missing its digest, posture or database), or cannot be read, is refused as `plan-file-unusable`. `--apply` without the flag is refused as `plan-file-required`. Both refusals come before the backup question and before the database is opened. +A file that is not an organization-ownership plan, or that lacks its digest, posture or database, or cannot be read, is refused as `plan-file-unusable`. `--apply` without the flag is refused as `plan-file-required`. Both refusals come before the backup question and before the database is opened. A checkpoint recorded under another digest is no longer discarded silently. The reviewed plan supersedes it: every step runs again (each is idempotent), and the apply report names the discarded checkpoint in `restartedFrom`, beside `reviewedPlan`. -`--apply` has not shipped in a release yet (its changeset is still pending), so no released invocation changes. The one-line form is: +## FROM → TO + +| invocation | FROM (18.0.0-next.1) | TO | +|:--|:--|:--| +| `os migrate organization-ownership --apply --backup-taken` | performed the plan it computed at apply time | refused, `plan-file-required`: run `os migrate organization-ownership --out plan.json`, review it, then `--apply --backup-taken --plan-file plan.json` | +| `os migrate organization-ownership --fate-supplement host-fates.json --apply --backup-taken` | performed the plan it computed with that supplement | refused, `plan-file-required`: plan with the same `--fate-supplement`, then add `--plan-file plan.json` | +| `--apply … --plan-file plan.json`, where the file was planned with another supplement, release, posture or database | (no such flag) | refused with `plan-digest-mismatch`, `plan-posture-mismatch` or `plan-database-mismatch`, naming both values | ```bash os migrate organization-ownership --out plan.json # plan, then review plan.json diff --git a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts index 008f535b31d..93dd7f0189c 100644 --- a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts @@ -42,6 +42,7 @@ import { type OrganizationOwnershipApplyReport, type StepReport, } from './organization-ownership-apply.js'; +import { inventoryDigest } from './organization-ownership-plan.js'; // [#10126] Pay the first load of what the boot reaches through dynamic // `import()`s at MODULE LOAD, outside the clocked hook. @@ -126,6 +127,8 @@ async function ceremony(posture: string, db: Database.Database): Promise { throw new Error(`the door was asked to save ${type} ${name}`); } }, templates: { find: async () => undefined }, diff --git a/packages/cli/src/utils/organization-ownership-not-declared.integration.test.ts b/packages/cli/src/utils/organization-ownership-not-declared.integration.test.ts index 50b16693bc7..75398dcd2fe 100644 --- a/packages/cli/src/utils/organization-ownership-not-declared.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-not-declared.integration.test.ts @@ -91,7 +91,10 @@ function projectFor(name: string, config: Record): string { return projectDir; } -/** The real command, `--apply`, from `projectDir`, over a fresh copy of the fixture. */ +/** + * The real command from `projectDir`, over a fresh copy of the fixture: the plan + * run first (its file is the reviewed plan `--apply` executes), then `--apply`. + */ async function applyIn(projectDir: string): Promise<{ dbFile: string; report: any; exitCode: number }> { const dbFile = join(projectDir, 'v17.db'); copyFileSync(templateDb, dbFile); @@ -109,9 +112,15 @@ async function applyIn(projectDir: string): Promise<{ dbFile: string; report: an process.chdir(projectDir); let exitCode: number | undefined; try { + const planFile = join(projectDir, 'plan.json'); + await MigrateOrganizationOwnership.run(['--out', planFile, '--database-url', `file:${dbFile}`, '--json'], { root: CLI_ROOT }); + stdout.mockClear(); try { await MigrateOrganizationOwnership.run( - ['--apply', '--backup-taken', '--out', join(projectDir, 'apply.json'), '--database-url', `file:${dbFile}`, '--json'], + [ + '--apply', '--backup-taken', '--plan-file', planFile, + '--out', join(projectDir, 'apply.json'), '--database-url', `file:${dbFile}`, '--json', + ], { root: CLI_ROOT }, ); } catch (error) { From d7ef4bd6d72fa79e16feb86a0417b56a5a8bda1c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:10:05 +0000 Subject: [PATCH 7/7] chore(spec): register the bare --apply's retirement in the ADR-0087 ledger (C7b S4 delta review) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH --- .changeset/15211-cli-apply-reviewed-plan.md | 3 +- ...tion-ownership-apply-plan-file-required.ts | 30 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.cli-organization-ownership-apply-plan-file-required.ts diff --git a/.changeset/15211-cli-apply-reviewed-plan.md b/.changeset/15211-cli-apply-reviewed-plan.md index 6951528c2bf..18d7bacff12 100644 --- a/.changeset/15211-cli-apply-reviewed-plan.md +++ b/.changeset/15211-cli-apply-reviewed-plan.md @@ -1,12 +1,13 @@ --- '@objectstack/cli': minor +'@objectstack/spec': patch --- feat(cli)!: `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10) Clause-②: yes (widening) - + **BREAKING** (a narrowing of `--apply`'s accept set, released in 18.0.0-next.1), shipped as `minor` under the launch-window convention for breaking changes. It also widens the command: a new flag, `--plan-file`, new refusal reasons, and two new keys on the apply report. diff --git a/packages/spec/src/migrations/entries/semantic/18.cli-organization-ownership-apply-plan-file-required.ts b/packages/spec/src/migrations/entries/semantic/18.cli-organization-ownership-apply-plan-file-required.ts new file mode 100644 index 00000000000..e8731570810 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cli-organization-ownership-apply-plan-file-required.ts @@ -0,0 +1,30 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'cli-organization-ownership-apply-plan-file-required', + // No backticks in `surface`: build-upgrade-guide.ts renders it inside a code + // span AND a table cell. + surface: + 'os migrate organization-ownership --apply, invoked without --plan-file (the bare --apply --backup-taken that ' + + '@objectstack/cli 18.0.0-next.1 shipped, with or without --fate-supplement)', + replacement: + 'Run the plan first, `os migrate organization-ownership --out plan.json` (with the same `--fate-supplement`, ' + + 'under the same tenancy posture, against the same database), review the file it writes, then ' + + '`os migrate organization-ownership --apply --backup-taken --plan-file plan.json`. A script or runbook that ' + + 'applies directly gains the plan step and the flag; nothing in a package source or a stored row changes.', + reason: + 'ADR-0131 D10 makes the ceremony an operator\'s act on a plan the operator reviewed. The apply computes its ' + + 'own plan at apply time, and that plan\'s inputs can move between the review and the run: a different host ' + + 'fate supplement, another release\'s inventory, another tenancy posture or another database. Planning with ' + + 'one supplement and applying with another ran the second one\'s fates unreviewed, including a mirror deletion ' + + 'the reviewed plan never listed. So the apply now reads the plan file the operator reviewed and refuses, before ' + + 'anything is written, when its inventory digest, posture or database differs from the run\'s. Without the ' + + 'file there is nothing to compare, so the bare invocation is refused.', + acceptanceCriteria: + 'Every invocation of `os migrate organization-ownership --apply` passes `--plan-file` with the file a plan run ' + + 'wrote, and the run reports `reviewedPlan`. A run that answers `plan-file-required`, `plan-digest-mismatch`, ' + + '`plan-posture-mismatch` or `plan-database-mismatch` has written nothing: plan again, review, and pass the new ' + + 'file.', +};