feat(checkpoints): per-file/per-step rollback core: rollback.ts + changeJournal read-failure semantics (B3c v2-11, epic #1375) - #54
Open
easonLiangWorldedtech wants to merge 1 commit into
Conversation
…restore + changeJournal read-failure semantics (B3c v2-11, epic Zoo-Code-Org#1375)
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Required CI passed. Waiting for automated review of the latest commit. If automated review does not start, a maintainer must restart it. Review-state labels are managed by this workflow; do not edit them manually. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
The rollback core half of the per-file/per-step rollback feature (B3c, epic Zoo-Code-Org#1375), stacked on #53 (v2-10, the shadow restore service this PR consumes):
src/core/checkpoints/rollback.ts(new): the rollback API —rollbackFile(task, stepCheckpointId, filePath),rollbackStep(task, stepFiles, stepCheckpointId?), andrestoreLatestFile(task, filePath)(the forward direction). "Rollback" = UNDO the change-card step: every file the step touched is restored to its PRE-step state, resolved from the B2 change journal (changes.jsonl) entries — an earlier step's entry checkpoint when one exists; the task-start baseline (service.baseHash) for the file's first change (undoing a create removes the file, undoing a delete restores it); multi-write steps (two entries sharing one checkpoint id) resolve through the first entry. A file written again by a later step is rejected (File was modified in a later step; roll back the latest change card first) instead of silently overwriting the newer state;restoreLatestFileon a file the task never wrote is a successful no-op. All failures are shaped into per-file outcomes (RollbackFileOutcome/RollbackStepOutcome), never thrown.getCheckpointService(task)→restoreFile(target, filePath)from feat(checkpoints): shadow restoreFile service for per-file rollback (B3c v2-10, epic #1375) #53 — so only the named file's working-tree content is replaced; the shadow repo HEAD and the checkpoint list are untouched.src/core/checkpoints/changeJournal.ts:loadChangesnow treats only an ABSENT journal file as an empty history (ENOENT →[]). Any other read failure (permissions, I/O, EISDIR) is rethrown — a journal that cannot be read must not be indistinguishable from one that is legitimately empty, or a rollback could report a no-op success without ever reading the history.__tests__/rollback.spec.ts(new): 26 tests across the threerollbackFile/rollbackStep/restoreLatestFiledescribes (resolution rules, multi-write, latest-only rejection, per-file failure isolation, journal-unavailable vs journal-unreadable, non-Error rejection stringification, exact diagnostic-message pinning) using a real journal on disk (appendChange) with the checkpoint service double;__tests__/changeJournal.spec.tsgains the EISDIR read-failure propagation test and a nullish-rejection rethrow test.Line count / split rationale
794 changed lines (789+/5-) across 5 files — above the 400 soft design target, under the 1000 hard cap; rationale:
rollback.ts+ its full spec + thechangeJournalENOENT-only delta are one unit — the plan's Stryker invariant requires every killing test for this PR's diff lines to live in the same PR, and the plan itself marks the rollback spec split as the only non-file-boundary cut (it was evaluated and kept whole here: the core/restore describes are the killing tests forrollback.ts, which is this PR's only new source file). Thestryker.config.mjsdelta (10 lines) is the gate configuration needed for the module-scope constants documented under Provenance.Plan note: the approved v2 plan placed the rollback core at #v2-10 and the shadow service at #v2-11 (rows B12/B13).
rollback.tscallsservice.restoreFile(...)from #53 at compile time, so the service PR must land first; the two PRs' content is swapped relative to the plan rows (documented in #53's body), with the slugs renamed to match.Provenance
rollback.tsis byte-identical to the B3c head10fdd195bon the upstream rollback branch (blob508789c9…).rollback.spec.tsderives from the B3c spec with one v2-new mutation-testing addition: the service-error test now also pins the exactconsole.errordiagnostic ([checkpointRollback] failed to restore … from checkpoint …: …) — the Stryker run at the original head showed the L141 message literal uncovered by assertions.changeJournalENOENT-only delta is a v2-new tightening (15+/4-) required by the rollback consumers, with its regression tests (EISDIR propagation; nullish-rejection rethrow, which pins the optional-chaining in the ENOENT guard).stryker.config.mjsgainscoverageAnalysis: "perTest"+ignoreStatic: true(v2-new gate config, 10 lines): under Stryker 10's vitest runner the test environment is long-lived per run, so module-scope code (the sixconsterror-message declarations inrollback.ts) executes once at first load and its mutants are unobservable at test time — they reportstatic: true/coveredBy: []and would always "survive". Stryker'signoreStaticreports them as Ignored instead of Survived (six such mutants in this PR's diff); the values themselves stay covered by the exact pinning assertions inrollback.spec.ts.Not included here (later v2 PRs)
Local gates
tsc --noEmit(src): clean.core/checkpoints): 6 files, 89 tests passed.--prune-suppressions --max-warnings=0on the touched spec files + fullpnpm lintvia the pre-commit hook: clean;src/eslint-suppressions.jsoncounts unchanged (content-equal verified after the run).2a499d841, head3e75647dc): 123 changed-code mutants, 123 Killed, 0 Survived, 0 NoCoverage, 0 timeout (117 changed lines; the 6 module-scope string-literal mutants ofrollback.tsL50-55 reported Ignored-static viaignoreStatic, see Provenance).