fix(save): preserve future-build saves on fresh boot (#196)#198
Conversation
A save written by a newer build (snapshot.simVersion > this build's LATEST_SIM_VERSION) was silently overwritten by autosave / "Save Now" on the older build — data loss. Root cause: hasIncompatibleSave() returned true on ANY deserialize throw, including FutureSimVersionError, so decideBootMode routed a future-sim save to a FRESH boot rather than the Continue/New Game SavePrompt. autosaveSuspended was only armed in bootFromSave's FutureSimVersionError catch (reached only via the SavePrompt's Continue), so for a future-sim save it never fired and bootFresh -> resetSessionState left it false. The preserve-future-save intent was therefore inert at boot. Fix: classify the incompatibility in save.ts (classifySaveCompatibility: incompatible-future via FutureSimVersionError vs incompatible-corrupt for any other throw) and, in decideBootMode's fresh-boot branch, arm autosaveSuspended=true after bootFresh only for the future case so autosave + Save Now no longer clobber the recoverable bytes. Corrupt saves still overwrite freely (unchanged). hasIncompatibleSave() now delegates to the classifier (behavior unchanged for the Save/Load dialog). Un-quarantine the #196 regression test and re-derive it against the real fixed flow: seed a real captured save, bump simVersion past LATEST, reload -> boot shows Choose Difficulty (not a Continue prompt) -> pick difficulty -> assert Save Now is a no-op (preserved future-sim bytes survive unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b8d230d7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…deleted (Codex P2) Codex review of PR #198 flagged that the new fresh-boot path arms autosaveSuspended for a recoverable future-build save but never clears it when the player explicitly deletes that save via the Save/Load dialog. The dialog calls deleteSave() directly and never notified GameScene, so after a Delete the running fresh session could no longer save (onSaveNow returned false, the autosave guard skipped every write) until a page reload. Wire an onDelete callback through SaveLoadDialogCallbacks: the dialog invokes it right after deleteSave(), and GameScene clears autosaveSuspended (the bytes that suspension was protecting are gone). Add an e2e regression test that deletes the preserved future save and asserts Save Now then succeeds. Also add direct unit tests for classifySaveCompatibility's four verdicts (none / compatible / incompatible-future / incompatible-corrupt), locking in the load-bearing distinction that a future-sim save suspends autosave while a sub-MIN / tampered save does not (ship-review advisory). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the P2 in
One low-severity item I'm deferring for your judgment (not fixed): when autosave is suspended to protect a future-build save, |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Fixes #196 — a save written by a newer build (
snapshot.simVersion > LATEST_SIM_VERSION) was silently overwritten by autosave / "Save Now" on the older build, destroying recoverable bytes.Root cause
hasIncompatibleSave()(src/platform/save.ts) ran a fulldeserializeWorldStateand returnedtrueon any throw, includingFutureSimVersionError. SodecideBootMode(() => hasSave() && !hasIncompatibleSave())routed a future-sim save to a fresh boot (showDifficultySelectThenBoot) instead of the Continue/New GameSavePrompt.autosaveSuspendedwas only armed insidebootFromSave'sFutureSimVersionErrorcatch — reached only via the SavePrompt's Continue — so for a future-sim save it never fired, andbootFresh → resetSessionStateleft itfalse. The preserve-future-save intent was inert at boot.Fix
save.ts— newclassifySaveCompatibility(): 'none' | 'compatible' | 'incompatible-future' | 'incompatible-corrupt'distinguishes a recoverable future-build save (FutureSimVersionError) from definitive corruption.hasIncompatibleSave()now delegates to it (behavior unchanged for the Save/Load dialog).game-scene.ts— boot classifies the save once; the fresh-boot branch armsautosaveSuspended = trueafterbootFreshonly forincompatible-future, mirroringbootFromSave's existingFutureSimVersionErrorcatch. Corrupt saves still overwrite freely.tests/menu-and-dialog.spec.ts— un-quarantined the Future-build save is not protected from overwrite (autosaveSuspended never armed on incompatible-save boot) #196 regression test and re-derived it against the real fixed flow: seed a real captured save → bumpsimVersionpast LATEST → reload → assertbootScreen === 'difficulty-select'(not a Continue prompt) → pick a difficulty → assert Save Now is a no-op (preserved future-sim bytes survive unchanged).No
simVersionbump — this is a boot/save-flow fix, not a sim algorithm/field change.Verification
npx playwright test tests/menu-and-dialog.spec.ts→ exit 0 (11 passed, the two e2e: pheromone ON/OFF screenshot-diff test is flaky (unfit for hard gate) #193 flaky tests remain quarantined). Non-vacuous: reverting only the source files makes the new test fail at the Save-Now assertion.npm run verify→ exit 0 (prettier, eslint, tsc, lint:types, sim/asset guards, 2303 Vitest tests).ship-review(basemain):passed: true, 0 actionable / 0 advisory.UAT for the owner
Seed a "future-build" save (bump
snapshot.simVersionpast LATEST), confirm it survives a fresh boot and that Save Now / autosave no longer overwrite it.🤖 Generated with Claude Code