fix(core): os migrate resume completes a recorded-by run that committed a chunk or used a non-default --chunk-size (#21528) - #21554
Conversation
…a committed chunk, or started at another chunk size Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
… it started over Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
…ing load, the journal's chunk size, the changed-plan control Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
…mmitted chunk at a non-default chunk size Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
…e() over no rows Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
…till-refused bullet it makes false Claude-Session: https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3 Co-authored-by: Claude <noreply@anthropic.com>
…sume-plan-identity
📓 Docs Drift Check16 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 27 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 25fe097a8545c75f097741156a34d2954e88f932 && git checkout 25fe097a8545c75f097741156a34d2954e88f932
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fd5a1cd5973983bf8b1ad69a10148a85c74c9137 14e5287fa80db1806b55bcf4923f14c8bd18de56 && git checkout -B drift-repro fd5a1cd5973983bf8b1ad69a10148a85c74c9137 && git merge --no-ff 14e5287fa80db1806b55bcf4923f14c8bd18de56
node scripts/docs-audit/affected-docs.mjs --json fd5a1cd5973983bf8b1ad69a10148a85c74c9137 |
Contract reviewServed-tier: PR #21554 for card #21528, branch ① Derived judgmentsPublic surface of Accept-set of
The ruling's three pins are present and run the real runner: ② Semver level
The DELIBERATE CORRECTION red —
③ Boundary flags
Implemented-by: The VERDICT: PASS Generated by Claude Code |
Fixes #21528
Clause-②: no
What changed
runMigrationJournal(packages/core/src/utils/migration-journal.ts) recomputed a resumed run's chunk plan from the rowsload()returns at resume time, at the resumed plan's chunk size, and refusedPLAN_CHANGEDwhen that plan's hash differed from the onerun_startedrecorded. A resume now reads the chunk plan back fromrun_started, which has carried it since the runner's first commit (ADR-0119 D2 item 2: "carrying the plan hash and chunk plan"):hashMigrationPlanis unchanged. On a resume it hashes the RECORDED chunk boundaries with the plan's declared id and step names, so it compares the plan against what the run started over. The run's chunk size comes back from the journal. A plan whose id or steps changed still refusesPLAN_CHANGED.load()that returns N rows binds positionally, as before. One that returns exactly N − K rows (it selects only the remaining work, asrecorded-by's does) binds those rows, in order, to the chunks not yet committed. Any other count refusesPLAN_CHANGEDand names the step. Every resume that passed before binds exactly as before (N rows, the same boundaries).load()no longer returns, cannot be compensated. The unwind compensates this process's chunks newest-first, then halts withrun_failedat that chunk with areason. It does not handcompensate()other rows and journal a clean unwind.run_startedwith no recorded chunk plan (only a hand-written journal) keeps today's check: reproduce the recorded hash from the current rows.No published member is added.
MigrationPlan,MigrationPlanStep,MigrationJournalEventand therun_startedpayload keep their shapes.MigrationPlanStep.loadandRunMigrationJournalOptions.chunkSizegain TSDoc for what a resume does with them. The plan (recorded-by-sentinel.ts),os migrate resume's source and the list mode'sresumableare untouched. No new error code: both new refusals arePLAN_CHANGED, the code the same inputs drew before.The public door, before and after
packages/cli/src/commands/migrate/resume.recorded-by.integration.test.tsnow makes three more interrupted runs the way a crash does (a child process runs the recorded-by plan under the real runner and is SIGKILLed inside a chunk's transaction) and drives the realos migrate resume --json:resumable: true,committedChunks: [0],unknownChunks: [1]. Before (core built from1ac7308d7a):--run RUN_ID --yesexited 1 withRefused (PLAN_CHANGED): ... plan hash 037ebfcc70ee54096b8eb2a6aed8cd49 does not match the journal's f36863e5fee4bb4e40093c66a0b3096f. After: exit 0,completed, 2 of 2 chunks, no row left holding the sentinel,chunk_startedindices[0, 1, 1](chunk 0 is not run again).resumable: true. Before: exit 1,PLAN_CHANGED. After: exit 0,completed,chunksTotal: 2(the journal's size; the registered plan's default 200 would make one chunk).Refused (PLAN_CHANGED), before and after. The sentinel rows and the journal are untouched.Before the fix that file read 2 failed / 7 passed (the two acts above); after, 9 passed.
Tests (at
14e5287fa8, this branch's head)packages/core/src/utils/migration-journal.test.ts: 29 passed (23 before + 6 new: a shrinking load resumes after a committed chunk; a non-shrinking load resumes positionally from a runner-written journal; the journal's chunk size wins over the plan's; the changed-plan control, three ways (step renamed, plan id changed, step added), each refused withcode: 'PLAN_CHANGED'and zero journal writes; a row count that is neither N nor N − K is refused, naming the step; the unwind halt). The crash helper runs the REAL runner and stops a forward inside its chunk, so every resume reads a journal the runner wrote.packages/metadata-protocol/src/migrations/recorded-by-sentinel.test.ts: 8 passed (1 new: the real plan, started at size 2 and killed after chunk 0 committed, resumes with the plan the owner registers at its default size, 2 of 2 chunks).--project integration, run locally because this diff edits that file).@objectstack/core(withcheck:test-typecheck: 4 files / 4 errors held, unchanged),@objectstack/metadata-protocol,@objectstack/cli(test layer: 3 files / 28 errors held, unchanged), all exit 0.ae27f00812(this diff before the merge ofmain, which touched none of these files):@objectstack/core79 files / 2220 tests passed;@objectstack/metadata-protocol205 files passed, 3 skipped / 3152 tests passed, 19 skipped.@objectstack/cli's unit tier: only the tier-partition pin (test/vitest-tiers-partition.test.ts, 22 passed); this diff changes no CLI source.node scripts/pm/dispatch-gates.mjs --commandson14e5287fa8derived 67 commands; all 67 ran, reconciled with--ran(each line carrying its exit code): 0 NOT MEASURED, 66 exit 0, and one exit 1,node scripts/check-empty-changeset.mjs --base origin/main, explained in the next section. Four roster gates the derivation flags as sharing a directory with these paths also ran green:check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parity..tsfiles, none ignored byeslint.config.mjs.eslint --no-inline-config --format jsonread 4 files, 0 errors, 0 warnings. That config never enables type-aware linting (noparserOptions.project), so this diff cannot move the verdict of any file it does not touch. The repo-widepnpm lintis CI's.Reverse verification and ablation
migration-journal.tsrestored to1ac7308d7ain the working tree only, blobdf8d8d5009checked equal to the base's, then core rebuilt andnode scripts/ablation-dist-preflight.mjs @objectstack/core planResumedRun --absentpassed. Red as predicted: core unit 4 failed / 25 passed (the four resume pins; the control and the positional-resume pin stay green, as they should on both trees), the plan pin 1 failed / 7 passed, the door 2 failed / 7 passed (a and b; the control green). Restored withgit checkout HEAD --under an EXIT/INT/TERM trap, proven by the blob (361d75e7ee== HEAD) and an emptygit diff HEAD, then rebuilt, with the preflight in default mode showing the marker back in 4 built files and a clean tree.node scripts/ablation-replace.mjsplanted the naive positional fallback (rowsByChunk.get(c.index) ?? rowsByStep[...].slice(offset, offset + length)). The landing was shown by the anchor count 1 → 0 and the blob change. The unwind pin went red:expected 'compensated' to be 'failed', which is the run handing chunk 0 other rows and journalling a clean unwind. Restored by the tool: blob == HEAD,git diff HEADempty. The core unit suite imports the runner by relative path, so no build leg applies.The pending #21498 changeset: a correction to confirm
This PR changes
.changeset/21498-cli-compose-migration-recovery.md, which it did not add. That note's "Still refused" bullet said the runner refuses these two kinds of run withPLAN_CHANGED. This change makes that false, and both notes are still pending, so they would ship in one release. The bullet now says these runs reach the runner too and points to the@objectstack/coreentry for #21528.check-empty-changesetstays red on this by design: it is the gate's DELIBERATE CORRECTION class, and its remedy is to say so here and get the correction confirmed. Restoring the old bullet would publish a sentence this PR makes false. Please confirm the correction.The new changeset (
.changeset/21528-core-resume-started-over-plan.md) is an@objectstack/corepatch withClause-②: no. No member is added to a published contract. Resume now accepts the runs its list mode already advertises as resumable, as ADR-0119 D2 item 5 declares.Acceptance notes
resumableis plan presence only (resume.ts:Boolean(plans?.get(r.planId))). So a run whose plan genuinely changed, or whose rows moved, is still listedresumable: trueand then refused. The triage ruling keeps the list's wording out of this card. I read this from source; I did not measure the list for the control run.chunk_done, and a chunk that was committed and then compensated has one. So resuming forward a run whose in-run unwind failed partway would skip the chunks that unwind had undone. Forrecorded-by(a shrinkingload()), that run's row count matches neither binding, so it is refusedPLAN_CHANGED, as it was before. Only a plan whoseload()does not shrink would take the skip, and no such plan is registered on this tree. I read this from source and did not measure it. Carrier: none.Generated by Claude Code