Skip to content

Commit c07af11

Browse files
committed
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)
- 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USccGXR13sWyTeEwe8QFdH
1 parent 15ebc02 commit c07af11

3 files changed

Lines changed: 33 additions & 11 deletions

File tree

‎.changeset/15211-cli-apply-reviewed-plan.md‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,34 @@
11
---
2-
'@objectstack/cli': patch
2+
'@objectstack/cli': minor
33
---
44

5-
feat(cli): `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10)
5+
feat(cli)!: `os migrate organization-ownership --apply` executes only the plan the operator reviewed (ADR-0131 D10)
66

7-
Clause-②: no
7+
Clause-②: yes (widening)
88

9-
`--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:
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is the accept set of one CLI command's writing mode: `--apply` without the reviewed plan file is refused, and the remedy is in the invocation an operator types (add `--plan-file`), 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 nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
1010

11-
- **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.
12-
- **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`.
13-
- **Another database** (`plan-database-mismatch`).
11+
**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.
12+
13+
`--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:
14+
15+
- **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.
16+
- **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`.
17+
- **The database** (`plan-database-mismatch`).
1418

1519
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.
1620

17-
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.
21+
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.
1822

1923
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`.
2024

21-
`--apply` has not shipped in a release yet (its changeset is still pending), so no released invocation changes. The one-line form is:
25+
## FROM → TO
26+
27+
| invocation | FROM (18.0.0-next.1) | TO |
28+
|:--|:--|:--|
29+
| `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` |
30+
| `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` |
31+
| `--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 |
2232

2333
```bash
2434
os migrate organization-ownership --out plan.json # plan, then review plan.json

‎packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
type OrganizationOwnershipApplyReport,
4343
type StepReport,
4444
} from './organization-ownership-apply.js';
45+
import { inventoryDigest } from './organization-ownership-plan.js';
4546

4647
// [#10126] Pay the first load of what the boot reaches through dynamic
4748
// `import()`s at MODULE LOAD, outside the clocked hook.
@@ -126,6 +127,8 @@ async function ceremony(posture: string, db: Database.Database): Promise<Organiz
126127
},
127128
posture,
128129
database: 'v17',
130+
// The plan the operator reviewed: the one this run computes.
131+
reviewedPlan: { file: 'plan.json', generatedAt: '2026-10-14T00:00:00Z', database: 'v17', inventoryDigest: inventoryDigest(), posture },
129132
catalog,
130133
door: { save: async (type, name) => { throw new Error(`the door was asked to save ${type} ${name}`); } },
131134
templates: { find: async () => undefined },

‎packages/cli/src/utils/organization-ownership-not-declared.integration.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,10 @@ function projectFor(name: string, config: Record<string, unknown>): string {
9191
return projectDir;
9292
}
9393

94-
/** The real command, `--apply`, from `projectDir`, over a fresh copy of the fixture. */
94+
/**
95+
* The real command from `projectDir`, over a fresh copy of the fixture: the plan
96+
* run first (its file is the reviewed plan `--apply` executes), then `--apply`.
97+
*/
9598
async function applyIn(projectDir: string): Promise<{ dbFile: string; report: any; exitCode: number }> {
9699
const dbFile = join(projectDir, 'v17.db');
97100
copyFileSync(templateDb, dbFile);
@@ -109,9 +112,15 @@ async function applyIn(projectDir: string): Promise<{ dbFile: string; report: an
109112
process.chdir(projectDir);
110113
let exitCode: number | undefined;
111114
try {
115+
const planFile = join(projectDir, 'plan.json');
116+
await MigrateOrganizationOwnership.run(['--out', planFile, '--database-url', `file:${dbFile}`, '--json'], { root: CLI_ROOT });
117+
stdout.mockClear();
112118
try {
113119
await MigrateOrganizationOwnership.run(
114-
['--apply', '--backup-taken', '--out', join(projectDir, 'apply.json'), '--database-url', `file:${dbFile}`, '--json'],
120+
[
121+
'--apply', '--backup-taken', '--plan-file', planFile,
122+
'--out', join(projectDir, 'apply.json'), '--database-url', `file:${dbFile}`, '--json',
123+
],
115124
{ root: CLI_ROOT },
116125
);
117126
} catch (error) {

0 commit comments

Comments
 (0)