Repository navigation
Commit 5f11cec
fix(cli): os migrate plan names the ownership ceremony, not --allow-destructive, for the organization_id drops apply refuses (#24954)
Fixes #24925
Clause-②: no
`os migrate plan` no longer offers `--allow-destructive` for the
`organization_id` drops that `os migrate apply` refuses ahead of the
organization-ownership ceremony (ADR-0131 D10 item 5). On a database the
ceremony has not verified, each such drop is still listed and counted as
drift. Its line names the ceremony as the remedy, and the footer names
the ceremony's two commands. A verified database, or one with no
`organization_id` drop, gets the plan it got before, byte for byte. The
`--json` document, the exit code and `os migrate apply` are unchanged.
## What changed
- `packages/cli/src/commands/migrate/plan.ts`
- `ceremonyHeldDrops` answers which entries are held. It uses the two
functions `apply`'s drop gate asks: `ceremonyOrganizationColumnDrops`,
asked one entry at a time, and `readOrganizationOwnershipMarker`, read
through the stack's `objectql` service as `apply` reads it. There is no
second inventory and no table list. The marker is read only when such a
drop is listed, and never under `--json`.
- A held drop's line is the differ's finding without its
`--allow-destructive` sentence (`driftFindingWithoutRemedy`, the seam
`[#23925]` added), then: `a drop the organization-ownership ceremony
owns, and it has not verified on this database: the ceremony drops it
itself (see below).`
- The footer while a drop is held:
- `Apply with: os migrate apply` prints only when that command has
non-destructive or additive work. It never adds `--allow-destructive`.
- Then one warning counts the held drops and the other destructive
changes that wait for the ceremony. It names `os migrate
organization-ownership --out PLAN`, then `os migrate
organization-ownership --apply --plan-file PLAN --backup-taken`, spelled
as `apply`'s refusal spells them.
- A driver's `[#23925]` apply limit keeps its own warning, and its count
excludes held drops.
- `packages/cli/src/commands/migrate/plan.apply-limit.test.ts`
- Eight pins, listed below. They are folded into the neighbour so the
suite pays one module load, not two.
- The harness's stderr spy now runs the write callback. Each case no
longer waits out the 2 s drain timeout in `utils/one-shot-exit.ts`: the
file's 12 cases took 24 to 75 ms each after the first, on a shared box.
- `.changeset/24925-plan-names-ownership-ceremony.md`:
`@objectstack/cli` patch.
## One deviation from the suggested route, measured
The suggested route kept offering `--allow-destructive` for the other
destructive entries. But `apply`'s gate refuses the whole
`--allow-destructive` run while any `organization_id` drop is in it.
Case 1c of `apply-organization-ownership-gate.integration.test.ts` pins
that, and step 1 below shows it again: an unrelated
`sys_sso_provider.created_by` drop was not applied either. So `apply`
would perform none of the destructive changes. The footer offers the
flag for none of them, and names the rest as waiting for the ceremony.
## Measured
Setup:
- **Database.** An app from `create-objectstack@17.7.0` (npm), booted
once on 17.7.0 with `objectstack dev --seed-admin`. That leaves 43
tables carrying `organization_id`, no ownership marker,
`sys_comment.reactions` present and 0 comments.
- **How it was run.** This repository's CLI built at `feb09db14` (=
main), and then this branch. It ran from a minimal project directory
with `OS_AUTH_SECRET` set, so the auth family composes, against copies
of that SQLite file.
1. **The data path is held.**
- A plain `os migrate apply --allow-destructive --yes` exits 1 at the
reactions gate first, as in the card's walk.
- After `os migrate comment-reactions --apply --yes` (`Converted 0
reaction(s) … recorded as verified`), `os migrate apply
--allow-destructive --yes` exits **1**: `✗ Refusing this
--allow-destructive run: it would drop organization_id from sys_job,
sys_job_queue, sys_metadata, sys_metadata_commit, sys_metadata_history,
sys_migration, sys_migration_journal, sys_secret, a drop the
organization-ownership ceremony owns, and the ceremony has not verified
on this database. … Nothing in this run was applied.`
- Column reads before and after match: 45 tables carry `organization_id`
in both; `sys_job`, `sys_metadata` and `sys_secret` keep it;
`sys_sso_provider.created_by` is still there.
2. **Main's plan.** It prints `28 change(s): 6 safe, 0 needs-confirm, 22
destructive`. Each `organization_id` drop reads `… (orphaned) — "os
migrate apply --allow-destructive" to drop it.`, the footer reads `Apply
with: os migrate apply (add --allow-destructive for drops /
tightenings)`, and the ceremony is never named.
3. **No `sys_migration` table** (a copy with `DROP TABLE
sys_migration`). The branch's plan exits 0 and lists the 7 remaining
drops as held. It prints no WARN or ERROR line beyond the 3 boot WARN
lines the held copy prints too.
4. **This branch, on the same database.** The 8 drops carry the ceremony
line, and the footer is:
```text
ℹ 28 change(s): 6 safe, 0 needs-confirm, 22 destructive
Apply with: os migrate apply
⚠ 8 organization_id drop(s) above are the organization-ownership ceremony's, and it has not verified on this database, so "os migrate apply --allow-destructive" refuses every run until it has (the other 14 destructive change(s) above wait for it too). Run "os migrate organization-ownership --out PLAN" to write and review its plan, take a backup, then "os migrate organization-ownership --apply --plan-file PLAN --backup-taken"; it drops these columns itself.
```
**Byte for byte.** Main's `plan.ts` and this branch's, each built on the
same tree, ran over the same four databases. Log timestamps and the
duration line were normalised before comparing.
| database | main vs branch |
| :--- | :--- |
| 17.x, no marker | differs on the 8 held entry lines and the footer
only (19 diff lines) |
| 17.x plus a verified marker row | identical |
| 17.x with no `sys_migration` table | differs on the 7 held entry lines
and the footer only (17 diff lines) |
| a fresh 18 database (birth marker removed) plus one orphan column, no
`organization_id` drift | identical |
## Pins: red on main, then green; one ablation
Pins in `plan.apply-limit.test.ts`. `bootSchemaStack` is stubbed, and
the marker read is the real `readOrganizationOwnershipMarker` over a
fake engine.
- **Held, five cases.** No marker row; an unverified row; no
`sys_migration` table, where the read throws and the plan answers "not
verified"; nothing `apply` can do before the ceremony, so there is no
apply line; and beside a driver's declared apply limit.
- **`--json`.** The document is unchanged, and no marker is read.
- **Two controls.** A verified marker gives today's plan, and the marker
WAS read. No `organization_id` drop gives today's plan, and the marker
is never read.
| leg | result |
| :--- | :--- |
| `plan.ts` at `feb09db14` (main), tree only, restored after | 6 failed,
6 passed. The 5 held cases fail on their entry lines. The verified
control fails only at its final read-count assertion: main never reads
the marker, and its plan-output assertions above that line passed. |
| branch | 12 passed |
| ablation: `return marker.verified ? …` changed to `return true ? …`
(every database treated as verified), via `scripts/ablation-replace.mjs`
| 5 failed (the five held cases), 7 passed. Restored: blob
`03cb80830b10` equals HEAD's, and `git diff HEAD` is empty. |
The subject resolves through relative source imports (`./plan.js`), not
through a package's `exports`, so no `dist/` preflight applies.
## Local verification at `003e41ab51`
- `pnpm --filter @objectstack/cli typecheck` exits 0, test layer
included: `check:test-typecheck: OK`.
- `pnpm --filter @objectstack/cli exec vitest run --project unit
--maxWorkers=2` exits 0: `Test Files 315 passed (315)`, `Tests 4650
passed (4650)`.
- The `integration` tier is left to CI: this diff touches no integration
file, spawn entry or driver boot path.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 67 derived commands, plus the 11 declared
WIDE-population families. All 78 exit 0.
- Three first exited 3 or 1 and were re-run on the same head.
`check:dual-build-cjs-loads` and `check:i18n-coverage` printed
PREREQUISITE NOT MET and COULD NOT MEASURE until the unbuilt packages
they read were built. `check:route-envelope` hit an ENOENT on a
temporary tsup file my concurrent build was writing.
- `--ran`: `✓ dispatch-gates --ran: 67 derived famil(ies) accounted for
— 67 run, 0 NOT-MEASURED (a DERIVED zero — all 67 recorded an exit code
and none of them is 3).`
- `pnpm lint` (`eslint . --no-inline-config`) on this head exits 0 with
no findings printed.
## Acceptance notes
- **Ordering, measured.**
- On the pristine 17.x copy, the ceremony run straight (`--out`, then
`--apply … --backup-taken`) stops at its `rewrite` step:
`sys_user_permission_set has no permission_set column, so no grant can
name its set. Boot this release once against the database (its schema
sync adds the column), then re-run.`
- After `os migrate apply` without the flag (exit 0, 16 tables created
or extended, 6 changes applied), the same ceremony passes `rewrite`.
- A refused `os serve` boot adds that column too.
- So the plan's `Apply with: os migrate apply` line above the ceremony
line is a sanctioned first step, not a detour. In this minimal
composition the ceremony then stopped at `column-drops` on
`sys_flow_credential`, an object my measurement project does not
declare. That is an artefact of the measurement set-up, not of this
change.
- **Not changed here (held off by the claim), for the PM to route.**
- The boot's `[schema-drift]` lines in `driver-sql` still say `"os
migrate apply --allow-destructive" to drop it` for these columns.
Measured: `os serve` on a 17.x copy exits 1 after 22 `[schema-drift]`
lines; the boot guard's refusal that follows names the ceremony.
- `os migrate apply` without the flag still prints `22 destructive
change(s) will be SKIPPED (re-run with --allow-destructive to include
them).` and `Skipped 22 destructive change(s) (re-run with
--allow-destructive to include them).`, which is a re-run its own drop
gate refuses (`apply.ts`).
- The plan still offers `--allow-destructive` for
`sys_comment.reactions` before `os migrate comment-reactions` has run,
though `apply` refuses that drop too. Its predicate is inline in
`apply.ts`, with no exported function to share, so it is not folded in
here.
- The ceremony's two commands are spelled in `plan.ts` and, separately,
in `apply.ts`'s refusal (held off). No shared constant was introduced.
- Read, not measured: the boot guard also refuses a verified marker of
an older shape generation (`outdated`). `apply`'s drop gate, and so this
plan, ask only `isDataMigrationFlagVerified`. The plan follows `apply`
here by construction.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 5d27d32 commit 5f11cec
3 files changed
Lines changed: 342 additions & 11 deletions
File tree
- .changeset
- packages/cli/src/commands/migrate
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
32 | 54 | | |
33 | 55 | | |
34 | 56 | | |
35 | 57 | | |
| 58 | + | |
36 | 59 | | |
37 | 60 | | |
38 | 61 | | |
| |||
135 | 158 | | |
136 | 159 | | |
137 | 160 | | |
138 | | - | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
139 | 168 | | |
140 | 169 | | |
141 | 170 | | |
| |||
233 | 262 | | |
234 | 263 | | |
235 | 264 | | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
0 commit comments