Skip to content

Commit 036916a

Browse files
wAngBygwAngByg
committed
fix(sessions): recover store from stale temp artifacts safely
1 parent d4fcc38 commit 036916a

4 files changed

Lines changed: 436 additions & 8 deletions

File tree

‎src/config/sessions/artifacts.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ function sessionStoreTempPattern(storeBasename: string): RegExp {
5656
// never the live store, so a stale one is safe to reclaim. `storeBasename` is the
5757
// store filename (the atomic write's temp prefix, e.g. `sessions.json`), so a
5858
// custom-named `session.store` is matched too.
59+
export const SESSION_STORE_TEMP_STALE_MS = 5 * 60 * 1000;
60+
5961
export function isSessionStoreTempArtifactName(fileName: string, storeBasename: string): boolean {
6062
if (!storeBasename) {
6163
return false;

‎src/config/sessions/disk-budget.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
isCompactionCheckpointTranscriptFileName,
1414
isPrimarySessionTranscriptFileName,
1515
isSessionArchiveArtifactName,
16+
SESSION_STORE_TEMP_STALE_MS,
1617
isSessionStoreTempArtifactName,
1718
isTrajectorySessionArtifactName,
1819
} from "./artifacts.js";
@@ -301,10 +302,6 @@ function isUnreferencedSessionArtifactFile(
301302
);
302303
}
303304

304-
// An orphaned `sessions.json.<pid>.<uuid>.tmp` older than this is never a live
305-
// atomic write (those rename within milliseconds), so it is safe to reclaim
306-
// regardless of the general unreferenced-artifact age threshold (#56827).
307-
const SESSION_STORE_TEMP_STALE_MS = 5 * 60 * 1000;
308305
// Prompt blobs are written or mtime-refreshed before sessions.json points at
309306
// them. Treat fresh unreferenced blobs as in-flight so cleanup cannot strand a
310307
// durable promptRef that is about to be committed by another writer.

‎src/config/sessions/store-load.recovery.test.ts‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
import crypto from "node:crypto";
2+
import fs from "node:fs";
3+
import os from "node:os";
4+
import path from "node:path";
5+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
6+
import { SESSION_STORE_TEMP_STALE_MS } from "./artifacts.js";
7+
import { loadSessionStore } from "./store-load.js";
8+
9+
function makeValidStore(...sessionIds: string[]): Record<string, unknown> {
10+
const store: Record<string, unknown> = {};
11+
for (const id of sessionIds) {
12+
store[id] = { sessionId: id, updatedAt: Date.now() };
13+
}
14+
return store;
15+
}
16+
17+
function writeJson(filePath: string, data: unknown): void {
18+
fs.writeFileSync(filePath, JSON.stringify(data, null, 2), "utf-8");
19+
}
20+
21+
function makeTmpDir(): string {
22+
return fs.mkdtempSync(path.join(os.tmpdir(), "session-recovery-"));
23+
}
24+
25+
function setOlderThanMs(filePath: string, millisecondsAgo: number): void {
26+
const target = Date.now() - millisecondsAgo;
27+
fs.utimesSync(filePath, new Date(target), new Date(target));
28+
}
29+
30+
function setNowMtime(filePath: string): void {
31+
const now = new Date();
32+
fs.utimesSync(filePath, now, now);
33+
}
34+
35+
function makeSessionStoreTmpPath(dir: string, storeBase = "sessions.json"): string {
36+
return path.join(dir, `${storeBase}.12345.${crypto.randomUUID()}.tmp`);
37+
}
38+
39+
function makeLegacySessionStoreTmpPath(dir: string, storeBase = "sessions.json"): string {
40+
return path.join(dir, `${storeBase}.${crypto.randomUUID()}.tmp`);
41+
}
42+
43+
describe("session store load recovery", () => {
44+
let tmpDir: string;
45+
let storePath: string;
46+
47+
beforeEach(() => {
48+
tmpDir = makeTmpDir();
49+
storePath = path.join(tmpDir, "sessions.json");
50+
});
51+
52+
afterEach(() => {
53+
vi.restoreAllMocks();
54+
fs.rmSync(tmpDir, { recursive: true, force: true });
55+
});
56+
57+
it("returns empty when main file is missing even if .bak exists", () => {
58+
const bakPath = `${storePath}.bak`;
59+
writeJson(bakPath, makeValidStore("bak-session"));
60+
const store = loadSessionStore(storePath, { skipCache: true });
61+
expect(Object.keys(store)).toHaveLength(0);
62+
});
63+
64+
it("does not recover from .bak when main file is zero bytes", () => {
65+
fs.writeFileSync(storePath, "", "utf-8");
66+
writeJson(`${storePath}.bak`, makeValidStore("s1", "s2", "s3"));
67+
const store = loadSessionStore(storePath, { skipCache: true });
68+
expect(Object.keys(store)).toHaveLength(0);
69+
expect(fs.readFileSync(storePath, "utf-8")).toBe("");
70+
});
71+
72+
it("does not recover from .bak when main file has malformed JSON", () => {
73+
fs.writeFileSync(storePath, "{bad json", "utf-8");
74+
writeJson(`${storePath}.bak`, makeValidStore("bak-session"));
75+
const store = loadSessionStore(storePath, { skipCache: true });
76+
expect(Object.keys(store)).toHaveLength(0);
77+
expect(fs.readFileSync(storePath, "utf-8")).toBe("{bad json");
78+
});
79+
80+
it("recovers from stale legacy tmp when main is malformed", () => {
81+
fs.writeFileSync(storePath, "{bad json", "utf-8");
82+
const tmpFile = makeLegacySessionStoreTmpPath(tmpDir);
83+
writeJson(tmpFile, makeValidStore("tmp1", "tmp2"));
84+
setOlderThanMs(tmpFile, SESSION_STORE_TEMP_STALE_MS + 1_000);
85+
const store = loadSessionStore(storePath, { skipCache: true });
86+
expect(Object.keys(store)).toHaveLength(2);
87+
});
88+
89+
it("recovers from stale session-store tmp when main is malformed", () => {
90+
fs.writeFileSync(storePath, "{bad json", "utf-8");
91+
const sessionTmp = makeSessionStoreTmpPath(tmpDir);
92+
writeJson(sessionTmp, makeValidStore("session-tmp-1", "session-tmp-2"));
93+
setOlderThanMs(sessionTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
94+
const store = loadSessionStore(storePath, { skipCache: true });
95+
expect(Object.keys(store)).toHaveLength(2);
96+
});
97+
98+
it("does not rewrite the malformed primary when recovering from stale tmp", () => {
99+
fs.writeFileSync(storePath, "{bad json", "utf-8");
100+
const sessionTmp = makeSessionStoreTmpPath(tmpDir);
101+
writeJson(sessionTmp, makeValidStore("tmp-recovered"));
102+
setOlderThanMs(sessionTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
103+
const renameSpy = vi.spyOn(fs, "renameSync");
104+
105+
const store = loadSessionStore(storePath, { skipCache: true });
106+
107+
expect(store["tmp-recovered"]).toBeDefined();
108+
expect(fs.readFileSync(storePath, "utf-8")).toBe("{bad json");
109+
expect(renameSpy).not.toHaveBeenCalled();
110+
});
111+
112+
it("does not overwrite a newer primary installed during tmp recovery", () => {
113+
fs.writeFileSync(storePath, "{bad json", "utf-8");
114+
const sessionTmp = makeSessionStoreTmpPath(tmpDir);
115+
writeJson(sessionTmp, makeValidStore("from-tmp"));
116+
setOlderThanMs(sessionTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
117+
const newerPrimary = makeValidStore("newer-primary");
118+
const originalOpenSync = fs.openSync.bind(fs);
119+
const originalWriteFileSync = fs.writeFileSync.bind(fs);
120+
const openSpy = vi.spyOn(fs, "openSync").mockImplementation((target, flags, mode) => {
121+
if (target === sessionTmp) {
122+
originalWriteFileSync(storePath, JSON.stringify(newerPrimary, null, 2), "utf-8");
123+
}
124+
return originalOpenSync(target, flags, mode);
125+
});
126+
127+
const store = loadSessionStore(storePath, { skipCache: true });
128+
129+
expect(store["from-tmp"]).toBeDefined();
130+
expect(JSON.parse(fs.readFileSync(storePath, "utf-8"))["newer-primary"]).toBeDefined();
131+
expect(openSpy).toHaveBeenCalled();
132+
});
133+
134+
it("reads tmp recovery candidates through the opened file descriptor", () => {
135+
fs.writeFileSync(storePath, "{bad json", "utf-8");
136+
const sessionTmp = makeSessionStoreTmpPath(tmpDir);
137+
writeJson(sessionTmp, makeValidStore("fd-bound"));
138+
setOlderThanMs(sessionTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
139+
const readSpy = vi.spyOn(fs, "readFileSync");
140+
141+
const store = loadSessionStore(storePath, { skipCache: true });
142+
143+
expect(store["fd-bound"]).toBeDefined();
144+
expect(readSpy.mock.calls.some(([target]) => typeof target === "number")).toBe(true);
145+
expect(readSpy.mock.calls.some(([target]) => target === sessionTmp)).toBe(false);
146+
});
147+
148+
it("does not recover from .bak when main file is valid empty object", () => {
149+
writeJson(storePath, {});
150+
writeJson(`${storePath}.bak`, makeValidStore("bak-session"));
151+
const store = loadSessionStore(storePath, { skipCache: true });
152+
expect(Object.keys(store)).toHaveLength(0);
153+
});
154+
155+
it("does not recover from .bak or tmp when main file contains valid []", () => {
156+
writeJson(storePath, []);
157+
writeJson(`${storePath}.bak`, makeValidStore("bak-session"));
158+
const staleTmp = makeSessionStoreTmpPath(tmpDir);
159+
writeJson(staleTmp, makeValidStore("stale-session"));
160+
setOlderThanMs(staleTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
161+
const store = loadSessionStore(storePath, { skipCache: true });
162+
expect(Object.keys(store)).toHaveLength(0);
163+
expect(store["bak-session"]).toBeUndefined();
164+
expect(store["stale-session"]).toBeUndefined();
165+
});
166+
167+
it("recovers from stale current tmp preferred over fresh legacy tmp", () => {
168+
fs.writeFileSync(storePath, "{bad json", "utf-8");
169+
const sessionTmp = makeSessionStoreTmpPath(tmpDir);
170+
writeJson(sessionTmp, makeValidStore("session-tmp"));
171+
setOlderThanMs(sessionTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
172+
const freshLegacyTmp = makeLegacySessionStoreTmpPath(tmpDir);
173+
writeJson(freshLegacyTmp, makeValidStore("fresh"));
174+
setNowMtime(freshLegacyTmp);
175+
const store = loadSessionStore(storePath, { skipCache: true });
176+
expect(store["session-tmp"]).toBeDefined();
177+
expect(store["fresh"]).toBeUndefined();
178+
});
179+
180+
it("prefers newest stale tmp when multiple valid candidates exist", () => {
181+
fs.writeFileSync(storePath, "{bad json", "utf-8");
182+
const olderTmp = makeSessionStoreTmpPath(tmpDir);
183+
writeJson(olderTmp, makeValidStore("older-session"));
184+
setOlderThanMs(olderTmp, SESSION_STORE_TEMP_STALE_MS + 60_000);
185+
const newerTmp = makeSessionStoreTmpPath(tmpDir);
186+
writeJson(newerTmp, makeValidStore("newer-session"));
187+
setOlderThanMs(newerTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
188+
const store = loadSessionStore(storePath, { skipCache: true });
189+
expect(store["newer-session"]).toBeDefined();
190+
expect(store["older-session"]).toBeUndefined();
191+
expect(Object.keys(store)).toHaveLength(1);
192+
});
193+
194+
it("ignores temp artifacts younger than the cleanup stale window", () => {
195+
fs.writeFileSync(storePath, "{bad json", "utf-8");
196+
const youngTmp = makeSessionStoreTmpPath(tmpDir);
197+
writeJson(youngTmp, makeValidStore("young-only"));
198+
setOlderThanMs(youngTmp, 60_000);
199+
const store = loadSessionStore(storePath, { skipCache: true });
200+
expect(Object.keys(store)).toHaveLength(0);
201+
});
202+
203+
it("skips empty {} tmp candidate and continues to next valid candidate", () => {
204+
fs.writeFileSync(storePath, "{bad json", "utf-8");
205+
const emptyTmp = makeSessionStoreTmpPath(tmpDir);
206+
writeJson(emptyTmp, {});
207+
setOlderThanMs(emptyTmp, SESSION_STORE_TEMP_STALE_MS + 30_000);
208+
const validTmp = makeSessionStoreTmpPath(tmpDir);
209+
writeJson(validTmp, makeValidStore("recovered"));
210+
setOlderThanMs(validTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
211+
const store = loadSessionStore(storePath, { skipCache: true });
212+
expect(store["recovered"]).toBeDefined();
213+
expect(Object.keys(store)).toHaveLength(1);
214+
});
215+
216+
it("skips unrelated json tmp (no sessionId entries) and continues scanning", () => {
217+
fs.writeFileSync(storePath, "{bad json", "utf-8");
218+
const unrelatedTmp = makeSessionStoreTmpPath(tmpDir);
219+
writeJson(unrelatedTmp, { version: 1, config: { foo: "bar" } });
220+
setOlderThanMs(unrelatedTmp, SESSION_STORE_TEMP_STALE_MS + 30_000);
221+
const validTmp = makeSessionStoreTmpPath(tmpDir);
222+
writeJson(validTmp, makeValidStore("recovered"));
223+
setOlderThanMs(validTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
224+
const store = loadSessionStore(storePath, { skipCache: true });
225+
expect(store["recovered"]).toBeDefined();
226+
expect(Object.keys(store)).toHaveLength(1);
227+
});
228+
229+
it("ignores .bak and falls through to stale tmp", () => {
230+
fs.writeFileSync(storePath, "{bad json", "utf-8");
231+
writeJson(`${storePath}.bak`, { version: 1, config: { foo: "bar" } });
232+
const validTmp = makeSessionStoreTmpPath(tmpDir);
233+
writeJson(validTmp, makeValidStore("from-tmp"));
234+
setOlderThanMs(validTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
235+
const store = loadSessionStore(storePath, { skipCache: true });
236+
expect(store["from-tmp"]).toBeDefined();
237+
expect(Object.keys(store)).toHaveLength(1);
238+
});
239+
240+
it("does not recover from .bak or tmp when the primary store cannot be read", () => {
241+
fs.mkdirSync(storePath);
242+
writeJson(`${storePath}.bak`, makeValidStore("bak-session"));
243+
const validTmp = makeSessionStoreTmpPath(tmpDir);
244+
writeJson(validTmp, makeValidStore("tmp-session"));
245+
setOlderThanMs(validTmp, SESSION_STORE_TEMP_STALE_MS + 1_000);
246+
247+
const store = loadSessionStore(storePath, { skipCache: true });
248+
249+
expect(Object.keys(store)).toHaveLength(0);
250+
expect(store["bak-session"]).toBeUndefined();
251+
expect(store["tmp-session"]).toBeUndefined();
252+
expect(fs.statSync(storePath).isDirectory()).toBe(true);
253+
});
254+
});

0 commit comments

Comments
 (0)