Repository navigation
fix(cli): os migrate plan and apply name each drop gate's command, never --allow-destructive, for the drops apply refuses - #25029
Conversation
… --allow-destructive, for a drop apply refuses The sys_account.issuer and sys_comment.reactions predicates move out of apply.ts into utils/apply-drop-gates.ts beside the organization_id gate's, with one read per gate per run shared by the listing and the gate. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6
The pin discovers apply's drop-gate refusals from its source and holds plan and apply to naming each gate's command for a held drop. The two gate refusal tests' fixtures spell the drift entry's finding as `message`, the key ManagedDriftEntry declares. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmjU2x1faLZ39DVTFyVca6
📓 Docs Drift CheckThis PR changes 1 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 8 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 30 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 83578d7a927ed03c733d72f74f67d752d26d45af && git checkout 83578d7a927ed03c733d72f74f67d752d26d45af
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4d991588ffcdedce1b79fc96ba2779c67c4f8d62 fc8973ea8d081684ee56547adf16765dfd100b8a && git checkout -B drift-repro 4d991588ffcdedce1b79fc96ba2779c67c4f8d62 && git merge --no-ff fc8973ea8d081684ee56547adf16765dfd100b8a
node scripts/docs-audit/affected-docs.mjs --json 4d991588ffcdedce1b79fc96ba2779c67c4f8d62
|
席位核对 · ACCEPT
Accepted deviations:
Out-of-scope dispositions (one line each):
Landing: through the merge queue by this seat once every check on this head reads Generated by Claude Code |
Fixes #24958
Clause-②: no
os migrate planandos migrate applyno longer offer--allow-destructiveas the way on for a drop thatos migrate applyrefuses whatever flag is passed. Three ofapply's drop gates refuse the whole--allow-destructiverun while such a drop is in it:sys_account.issuerwhile its retirement pre-flight does not pass,sys_comment.reactionswhile no verified run ofos migrate comment-reactions --applyis recorded, and anorganization_iddrop while the organization-ownership ceremony has not verified (ADR-0131 D10 item 5). Each such drop is still listed and counted as drift in both commands. Its line names what it waits for, the footer names that gate's commands, and no line offers a re-run with--allow-destructivewhile one is held. A database with no held drop prints what it printed before, byte for byte, in both commands. The--jsondocuments, the exit codes and every refusal (rule and text) are unchanged.What changed
packages/cli/src/utils/apply-drop-gates.ts(new):apply's drop gates as the doors read them.apply.tsmove here unchanged:isAccountIssuerDropandisCommentReactionsDrop.isCeremonyOrganizationDropasks the drop gate's ownceremonyOrganizationColumnDrops.applyasks the first two from here.DropGateReadsmakes each gate's read at most once per run:probeAccountIdentityCollisions(plugin-auth),readCommentReactionsConversionandreadOrganizationOwnershipMarker, the same functionsapply's gates call. The listing and the gate share that read, so they cannot answer differently, and a held run reads each gate once, as before. A read that throws is remembered as thrown, so the issuer gate's "could not read" refusal is unchanged.APPLY_DROP_GATES: one row per gate, inapply's order, with its predicate, how it judges its read, the listing line and the footer line. There is no second inventory, dialect or table list.readHeldDrops,describeDriftEntriesanddescribeHeldDropsgeneralise PR fix(cli): os migrate plan names the ownership ceremony, not --allow-destructive, for the organization_id drops apply refuses #24954'sceremonyHeldDrops,describeWithCeremonyHoldsanddescribeCeremonyHold. A gate that judges no listed entry is never read. Theorganization_idlines keep PR fix(cli): os migrate plan names the ownership ceremony, not --allow-destructive, for the organization_id drops apply refuses #24954's text byte for byte.packages/cli/src/commands/migrate/plan.ts: the plan asks the module instead of its three local helpers (removed), so the issuer and reactions gates join the ceremony's.packages/cli/src/commands/migrate/apply.ts:--json.N destructive change(s) will be SKIPPED, and a re-run with --allow-destructive cannot include them yet:followed by one line per gate, and the post-apply line isSkipped N destructive change(s); a re-run with --allow-destructive cannot include them yet (see above).--jsondocuments and exit codes are untouched.packages/cli/test/migrate-drop-gate-remedies.pin.test.ts(new, unit tier): the enumeration pin, described below.packages/cli/src/commands/migrate/apply.organization-ownership-refusal.test.ts,apply.account-issuer-json-refusal.test.ts: fixture triage. Each fixture spelled the drift entry's finding asdescription, a keyManagedDriftEntrydoes not declare, and carried nomessage, which it requires. The listing now readsmessagefor a held drop, and on these fixtures it threw before the gate. Both org human-face cases went red (the refusal never printed). The issuer human-face control stayed green, exiting 1 from that throw rather than from its refusal. Both fixtures now carry the differ'smessage, and the issuer control also asserts that its refusal printed..changeset/24958-migrate-drop-gate-remedies.md:@objectstack/clipatch.Measured at the public door
Setup:
create-objectstack@17.7.0app had booted once (43 tables carryingorganization_id,sys_comment.reactionspresent, no ceremony marker, no reactions conversion recorded, nosys_account.issuer). Variants were shaped by hand:held: one comment carrying a stored reaction.reactions: asheld, plus a verified ceremony marker row.orgonly: a verified reactions-conversion row.issuer: both rows verified, plus the legacyissuercolumn, the(provider_id, account_id)unique index dropped, and a second account row sharing the first one's key.issuerclean: asissuerwith no second row.verified: both rows verified.fresh18: a fresh 18 database plus one orphan column.62bc12930(main) and then at this branch, from a minimal project directory withOS_AUTH_SECRETset so the auth family composes, stdin from/dev/null. Every run used its own fresh copy.The card's three mechanism assumptions, on main:
held,os migrate apply --yesexits 0 and lists eachorganization_iddrop assys_job.organization_id: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it., then prints⚠ 22 destructive change(s) will be SKIPPED (re-run with --allow-destructive to include them).and, after applying,⚠ Skipped 22 destructive change(s) (re-run with --allow-destructive to include them).That re-run is refused, exit 1. Onheldthe reactions gate refuses it first. Onorgonlythe drop gate refuses it:✗ Refusing this --allow-destructive run: it would drop organization_id from sys_job, sys_job_queue, … Nothing in this run was applied.reactions,os migrate planlistssys_comment.reactions: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it.and closes withApply with: os migrate apply (add --allow-destructive for drops / tightenings).os migrate apply --allow-destructive --yesexits 1:✗ Refusing to drop sys_comment.reactions: … Run "os migrate comment-reactions" to preview, then "os migrate comment-reactions --apply"; once it reports the run verified, re-run this command.The gate's predicate moved out ofapply.tsunchanged: the same reads, the same answer, the same refusal text (the refusal lines and exit codes below are identical).issuer, the plan listssys_account.issuer: column exists in the database but not in metadata (orphaned) — "os migrate apply --allow-destructive" to drop it.with the same footer, andapply --allow-destructive --yesexits 1:✗ Refusing to drop sys_account.issuer: … Resolve them, then re-run "os migrate account-issuer" as the post-check before applying.The controlissuercleanpasses the pre-flight (✓ sys_account.issuer retirement pre-flight: clean over 1 row(s).) and applies, exit 0, so there the plan's--allow-destructiveis right and is kept.After, on this branch. The
heldplan's footer:os migrate apply --yesonheldprints the same two gate lines under⚠ 22 destructive change(s) will be SKIPPED, and a re-run with --allow-destructive cannot include them yet:, then⚠ Skipped 22 destructive change(s); a re-run with --allow-destructive cannot include them yet (see above).The held entries read:and the
issuerfooter:⚠ sys_account.issuer above is the retired account issuer, and its retirement pre-flight finds 1 (provider_id, account_id) key(s) held by more than one row on this database, so "os migrate apply --allow-destructive" refuses every run until it passes (the other 23 destructive change(s) above wait for it too). Run "os migrate account-issuer" for its report, resolve what it lists, then re-run it as the post-check.Byte for byte, main against this branch, 24 runs. Log timestamps, the duration line and the per-run copy's directory were normalised before comparing. "Refusals and exit" compares every
Refusingline plus the exit code.heldplan·apply --yes·apply --allow-destructive --yesreactionsissuerorgonlyverifiedissuercleanfresh18plan·apply --yesheld·verifiedplan --json·apply --yes --jsonEvery differing line is a held entry's line, a gate's footer line, or a skip line.
orgonly's plan is identical because PR #24954 already handled it, and the generalised code reproduces that text.The enumeration pin: red on main, then green; one ablation
packages/cli/test/migrate-drop-gate-remedies.pin.test.ts, 33 cases.message: 'refused_…'fromapply.tsand requires each to be claimed by a roster row (or byrefused_unloadable_host_config, which is not a drop gate). The rows must equalAPPLY_DROP_GATES, in order, and each row's entry must be judged by its own gate and no other. A new gate's refusal fails the first case, by name, until it has a row, and its row's cases fail until the module reads it.sys_accountunreadable; reactions and the ceremony: no row, and an unverified row):planlists and counts the entry, its line carries no--allow-destructive, and the footer names every command of the gate's remedy and offers--allow-destructivefor nothing.apply --yes: the same listing line and footer; no line offers a re-run with--allow-destructive; the safe work still applies.apply --allow-destructive --yes: the same listing line, then the gate's own refusal, exit 1, nothing written, naming the same commands. The gate's read was made once.--json. The plan document carries the differ's messages and reads no gate.objectqlservice holding the rows. The boot, the occupancy probe and the one-shot exit are replaced. The subjects resolve through relativesrc/imports, not a package'sexports, so nodist/preflight applies.apply.tsandplan.tsat62bc12930(main), tree only, the new module and pin keptnot to contain '--allow-destructive'). The ceremony's two plan cases pass: PR #24954 already handled them. The issuer and reactions plan controls fail only at their final read-count assertion, because main never reads those gates. Restored: blobs2d4a0663659aand5d66675a3243equal HEAD's,git diff HEADis empty andgit statusis clean.scripts/ablation-replace.mjs: the reactions gate's judgement.verified ? nullchanged to.verified !== undefined /* ABLATION */ ? null(the read made, its verdict ignored)d93231fab096tobdb42c39683d. Restored: blob equals HEAD'sd93231fab096, andgit diff HEADis empty.Local verification
All at the final head
fc8973ea8d, a clean merge oforigin/main(73432299c; no incoming file underpackages/clior the gates' packages), unless a line says otherwise.pnpm --filter @objectstack/cli typecheck: exit 0, test layer included (check:test-typecheck: OK).@objectstack/cliunit tier (vitest run --project unit --maxWorkers=2):Test Files 317 passed (317),Tests 4717 passed (4717), at8bb7ff69ff, the branch's last own commit before the merge. Atfc8973ea8dthe pin, its neighbours and the tier partition re-ran green:plan.apply-limit, both gate-refusal tests,migrate-confirmation-exit.pin,exit-signal.pinandvitest-tiers-partition,Tests 225 passed.apply's gates against real databases were run anyway because the gates' reads moved:comment-reactions.integration,apply-organization-ownership-gate.integrationandapply.account-issuer-retirement.integration,Tests 16 passed.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack: 68 derived commands, plus the 11 declared WIDE-population families, 79 in all, each exit captured before any pipe.check:dual-build-cjs-loadsandcheck:i18n-coverageexited 3 (PREREQUISITE NOT MET: nodist/for packages this diff does not touch). Afterpnpm buildboth exited 0 on the same head.--ran:✓ dispatch-gates --ran: 68 derived famil(ies) accounted for — 68 run, 0 NOT-MEASURED (a DERIVED zero — all 68 recorded an exit code and none of them is 3).node scripts/check-platform-checklist.mjs --changed-from origin/main: exit 0 (no anchored file in the change set).pnpm lint(eslint . --no-inline-config, the whole repository): exit 0, no findings.Acceptance notes
apply.organization-ownership-refusal.test.tsandapply.account-issuer-json-refusal.test.tsare outside the claimed surface. Their fixtures could not represent a real drift entry, and the change made that visible (see "What changed"). They needed a one-key fixture correction, plus one added assertion in the issuer control so it can no longer pass on an unrelated exit 1.apply(with or without--allow-destructive) andplannow make a gate's read whenever that gate's drop is listed. Before, a run without the flag made none. For the issuer gate that read is the pre-flight's walk ofsys_account, which it already made on every--allow-destructiverun. With the flag, the listing and the gate share one read, so the count is unchanged. That is pinned, and the existing issuer control'stoHaveBeenCalledTimes(1)still holds.--jsonmakes no new read.--allow-destructivesentence on their own lines; each gate's footer line counts them as waiting for it. The category headingDestructive (requires --allow-destructive)is unchanged.[schema-drift]lines (Boot drift lines on a 17.x database before the ownership ceremony prescribeos migrate apply --allow-destructivefor eachorganization_idcolumn, a run the drop gate refuses #24956, the engine lane) still print"os migrate apply --allow-destructive" to drop it.duringapply's flush, in the same run as the new lines.issuerdatabase,apply --allow-destructive's pre-flight report printsissuers [(none)]for two rows whose stored issuers arehttps://idp-a.exampleandhttps://idp-b.example.os migrate account-issueron the same database printsissuers [https://idp-a.example, https://idp-b.example]. The pre-flightapplyuses (probeAccountIdentityCollisions, plugin-auth) reads throughengine.find, which projects only the declared fields, and v18 no longer declaresissuer. Not changed here: the refusal path is held off.Generated by Claude Code