Skip to content

Commit a85e01a

Browse files
Fix review findings: autosave leak and permalink bypassing resume
- composition.ts: put() now removes the superseded save blob once the new record and index are safely written, instead of leaving every prior autosave orphaned in localStorage under its old saveId. - PlayApp.tsx: the ?campaign= auto-start effect now checks findLocalSave() first and resumes an existing run, matching the manual briefing path instead of always starting a fresh session. - PlayApp.test.tsx: covers the permalink-resume path.
1 parent a29210f commit a85e01a

3 files changed

Lines changed: 49 additions & 1 deletion

File tree

site/src/play/PlayApp.test.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,43 @@ describe("PlayApp cabinet presentation", () => {
111111
}
112112
});
113113

114+
it("resumes an existing local save when opened via its permanent ?campaign= link, rather than restarting", async () => {
115+
const user = userEvent.setup();
116+
const { unmount } = render(<PlayApp />);
117+
await screen.findByRole("heading", { name: "Adventure disk library" });
118+
119+
await user.click(screen.getByRole("button", { name: /The Bureaucracy/i }));
120+
await user.click(
121+
screen.getByRole("button", { name: "Load selected adventure" }),
122+
);
123+
await user.click(
124+
await screen.findByRole("button", {
125+
name: /Wait for the municipal registry/i,
126+
}),
127+
);
128+
const advancedScene =
129+
document.querySelector(".scene-body")?.textContent ?? "";
130+
expect(advancedScene).not.toMatch(/handwritten/i);
131+
unmount();
132+
133+
const originalLocation = window.location.href;
134+
window.history.pushState({}, "", "/?campaign=bulgaria-bureaucracy");
135+
try {
136+
render(<PlayApp />);
137+
expect(
138+
await screen.findByRole("heading", { name: "The Bureaucracy" }),
139+
).toBeVisible();
140+
expect(document.querySelector(".scene-body")?.textContent).toBe(
141+
advancedScene,
142+
);
143+
expect(
144+
screen.queryByRole("heading", { name: "Adventure disk library" }),
145+
).not.toBeInTheDocument();
146+
} finally {
147+
window.history.pushState({}, "", originalLocation);
148+
}
149+
});
150+
114151
it("ignores a submission that resolves after the player quits to the library", async () => {
115152
const user = userEvent.setup();
116153
render(<PlayApp />);

site/src/play/PlayApp.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,9 @@ function PlayAppReady({ demo }: { demo: BrowserDemo }) {
169169
if (!requested || !demo.findCampaign(requested)) return;
170170
autoStarted.current = true;
171171
setSelectedId(requested);
172-
void start(requested);
172+
const saveId = demo.findLocalSave(requested);
173+
if (saveId) void resume(requested, saveId);
174+
else void start(requested);
173175
}, [demo]);
174176

175177
function reducedMotion(): boolean {

site/src/play/composition.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,20 @@ function localPersistence(): SessionPersistence {
5555
return raw ? (JSON.parse(raw) as StoredSaveRecord) : undefined;
5656
},
5757
async put(record) {
58+
const supersededId = localStorage.getItem(
59+
campaignSaveIndexKey(record.campaignId),
60+
);
5861
localStorage.setItem(saveKey(record.saveId), JSON.stringify(record));
5962
localStorage.setItem(
6063
campaignSaveIndexKey(record.campaignId),
6164
record.saveId,
6265
);
66+
// Every autosave mints a fresh saveId (types.ts), so the previous full
67+
// record would otherwise sit in localStorage unreachable from the index.
68+
// Removed only after the new record and index are safely written, so a
69+
// failure here can never erase the only usable checkpoint.
70+
if (supersededId && supersededId !== record.saveId)
71+
localStorage.removeItem(saveKey(supersededId));
6372
},
6473
async delete(id) {
6574
const raw = await this.get(id);

0 commit comments

Comments
 (0)