Skip to content

Commit 7f1f5ec

Browse files
openperfopenperf
committed
fix(sessions): stop doctor OOM on large session stores and reclaim stale store temps
`openclaw doctor` loaded the full sessions.json via loadSessionStore with the default cache-write plus return clone, materializing a multi-hundred-MB monolithic store several times and exhausting the heap (#56827). The read-only doctor checks (state integrity, heartbeat target, codex route scan) now load with { skipCache: true, clone: false } so the store is materialized once. Orphaned session-store atomic-write temps were also never reclaimed: the store write went through the generic atomic writer, staging a shared .fs-safe-replace.<pid>.<uuid>.tmp not identifiable as a store temp. Give the store write a store-specific tempPrefix so its temps stage as sessions.json.<pid>.<uuid>.tmp, classify them (isSessionStoreTempArtifactName), and reclaim stale ones via the disk-budget sweep and the unreferenced-artifact prune on a short staleness window so in-flight temps are preserved. Fixes #56827
1 parent a56f452 commit 7f1f5ec

10 files changed

Lines changed: 222 additions & 14 deletions

File tree

‎src/commands/doctor-heartbeat-session-target.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export function describeHeartbeatSessionTargetIssues(cfg: OpenClawConfig): strin
120120
}
121121
const storeAgentId = resolvedAgentId;
122122
const storePath = resolveStorePath(cfg.session?.store, { agentId: storeAgentId });
123-
const store = loadSessionStore(storePath);
123+
const store = loadSessionStore(storePath, { skipCache: true, clone: false });
124124
const entry = store[canonicalSession];
125125
if (entry) {
126126
continue;

‎src/commands/doctor-state-integrity.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,10 @@ export async function noteStateIntegrity(
852852
);
853853
}
854854

855-
const store = loadSessionStore(storePath);
855+
// Read-only diagnostic load: skip the cache and the defensive return clone so a
856+
// very large monolithic sessions.json is materialized once, not several times.
857+
// Re-cloning a multi-hundred-MB store here is what made `doctor` OOM (#56827).
858+
const store = loadSessionStore(storePath, { skipCache: true, clone: false });
856859
const sessionPathOpts = resolveSessionFilePathOptions({ agentId, storePath });
857860
const entries = Object.entries(store).filter(([, entry]) => entry && typeof entry === "object");
858861
if (entries.length > 0) {

‎src/commands/doctor/shared/codex-route-warnings.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2744,7 +2744,9 @@ export async function maybeRepairCodexSessionRoutes(params: {
27442744
}
27452745
if (!params.shouldRepair) {
27462746
const stale = targets.flatMap((target) => {
2747-
const sessionKeys = scanCodexSessionStoreRoutes(loadSessionStore(target.storePath));
2747+
const sessionKeys = scanCodexSessionStoreRoutes(
2748+
loadSessionStore(target.storePath, { skipCache: true, clone: false }),
2749+
);
27482750
return sessionKeys.map((sessionKey) => `${target.agentId}:${sessionKey}`);
27492751
});
27502752
return {
@@ -2767,7 +2769,9 @@ export async function maybeRepairCodexSessionRoutes(params: {
27672769
let repairedStores = 0;
27682770
let repairedSessions = 0;
27692771
for (const target of targets) {
2770-
const staleSessionKeys = scanCodexSessionStoreRoutes(loadSessionStore(target.storePath));
2772+
const staleSessionKeys = scanCodexSessionStoreRoutes(
2773+
loadSessionStore(target.storePath, { skipCache: true, clone: false }),
2774+
);
27712775
if (staleSessionKeys.length === 0) {
27722776
continue;
27732777
}

‎src/config/sessions/artifacts.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
isCompactionCheckpointTranscriptFileName,
55
isPrimarySessionTranscriptFileName,
66
isSessionArchiveArtifactName,
7+
isSessionStoreTempArtifactName,
78
isTrajectoryPointerArtifactName,
89
isTrajectoryRuntimeArtifactName,
910
isTrajectorySessionArtifactName,
@@ -23,6 +24,30 @@ describe("session artifact helpers", () => {
2324
expect(isSessionArchiveArtifactName("abc.jsonl")).toBe(false);
2425
});
2526

27+
it("classifies orphaned session store atomic-write temp files", () => {
28+
const uuid = "0f9c1a2b-3c4d-4e5f-8a9b-0c1d2e3f4a5b";
29+
const store = "sessions.json";
30+
// sessions.json.<pid>.<uuid>.tmp (current) and legacy sessions.json.<uuid>.tmp
31+
expect(isSessionStoreTempArtifactName(`sessions.json.12345.${uuid}.tmp`, store)).toBe(true);
32+
expect(isSessionStoreTempArtifactName(`sessions.json.${uuid}.tmp`, store)).toBe(true);
33+
// Never the live store, archives, transcripts, or unrelated temp files.
34+
expect(isSessionStoreTempArtifactName("sessions.json", store)).toBe(false);
35+
expect(isSessionStoreTempArtifactName("sessions.json.bak.1737420882", store)).toBe(false);
36+
expect(isSessionStoreTempArtifactName(`${uuid}.jsonl`, store)).toBe(false);
37+
expect(isSessionStoreTempArtifactName("sessions.json.tmp", store)).toBe(false);
38+
expect(isSessionStoreTempArtifactName("other.json.12345.tmp", store)).toBe(false);
39+
// Tracks a custom session.store filename (and only that store's temps); the
40+
// basename is regex-escaped so its dots are literal, not wildcards.
41+
expect(isSessionStoreTempArtifactName(`my-store.json.99.${uuid}.tmp`, "my-store.json")).toBe(
42+
true,
43+
);
44+
expect(isSessionStoreTempArtifactName(`my-store.json.99.${uuid}.tmp`, store)).toBe(false);
45+
expect(isSessionStoreTempArtifactName(`sessions.json.99.${uuid}.tmp`, "my-store.json")).toBe(
46+
false,
47+
);
48+
expect(isSessionStoreTempArtifactName(`sessionsXjson.99.${uuid}.tmp`, store)).toBe(false);
49+
});
50+
2651
it("classifies primary transcript files", () => {
2752
expect(isPrimarySessionTranscriptFileName("abc.jsonl")).toBe(true);
2853
expect(isPrimarySessionTranscriptFileName("keep.deleted.keep.jsonl")).toBe(true);

‎src/config/sessions/artifacts.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { escapeRegExp } from "../../shared/regexp.js";
2+
13
export type SessionArchiveReason = "bak" | "reset" | "deleted";
24

35
const ARCHIVE_TIMESTAMP_RE = /^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}(?:\.\d{3})?Z$/;
@@ -26,6 +28,36 @@ export function isSessionArchiveArtifactName(fileName: string): boolean {
2628
);
2729
}
2830

31+
// Compiled-pattern cache keyed by store basename. A disk sweep calls the matcher
32+
// once per file, so compiling the per-store pattern once (basenames are few — one
33+
// per agent store) keeps the hot path allocation-free.
34+
const SESSION_STORE_TEMP_RE_CACHE = new Map<string, RegExp>();
35+
36+
function sessionStoreTempPattern(storeBasename: string): RegExp {
37+
let pattern = SESSION_STORE_TEMP_RE_CACHE.get(storeBasename);
38+
if (!pattern) {
39+
pattern = new RegExp(
40+
`^${escapeRegExp(storeBasename)}\\.(?:\\d+\\.)?[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\\.tmp$`,
41+
"i",
42+
);
43+
SESSION_STORE_TEMP_RE_CACHE.set(storeBasename, pattern);
44+
}
45+
return pattern;
46+
}
47+
48+
// Atomic writes of the session store stage into `<store>.<pid>.<uuid>.tmp`
49+
// (legacy: `<store>.<uuid>.tmp`) and rename into place. A crash between write and
50+
// rename orphans the temp; these accumulate and waste disk (#56827). They are
51+
// never the live store, so a stale one is safe to reclaim. `storeBasename` is the
52+
// store filename (the atomic write's temp prefix, e.g. `sessions.json`), so a
53+
// custom-named `session.store` is matched too.
54+
export function isSessionStoreTempArtifactName(fileName: string, storeBasename: string): boolean {
55+
if (!storeBasename) {
56+
return false;
57+
}
58+
return sessionStoreTempPattern(storeBasename).test(fileName);
59+
}
60+
2961
export function parseCompactionCheckpointTranscriptFileName(fileName: string): {
3062
sessionId: string;
3163
checkpointId: string;

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

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
resolveTrajectoryPointerFilePath,
88
} from "../../trajectory/paths.js";
99
import { formatSessionArchiveTimestamp } from "./artifacts.js";
10-
import { enforceSessionDiskBudget } from "./disk-budget.js";
10+
import { enforceSessionDiskBudget, pruneUnreferencedSessionArtifacts } from "./disk-budget.js";
1111
import type { SessionEntry } from "./types.js";
1212

1313
async function expectPathExists(targetPath: string): Promise<void> {
@@ -102,6 +102,50 @@ describe("enforceSessionDiskBudget", () => {
102102
});
103103
});
104104

105+
it("reclaims stale store temps under pressure but never a fresh in-flight one (#56827)", async () => {
106+
await withTempDir({ prefix: "openclaw-disk-budget-" }, async (dir) => {
107+
const storePath = path.join(dir, "sessions.json");
108+
const sessionId = "keep";
109+
const transcriptPath = path.join(dir, `${sessionId}.jsonl`);
110+
const staleTemp = path.join(
111+
dir,
112+
"sessions.json.111.0f9c1a2b-3c4d-4e5f-8a9b-0c1d2e3f4a5b.tmp",
113+
);
114+
const freshTemp = path.join(
115+
dir,
116+
"sessions.json.222.1a2b3c4d-5e6f-4a8b-9c0d-1e2f3a4b5c6d.tmp",
117+
);
118+
const store: Record<string, SessionEntry> = {
119+
"agent:main:main": { sessionId, updatedAt: Date.now() },
120+
};
121+
await fs.writeFile(storePath, JSON.stringify(store, null, 2), "utf-8");
122+
await fs.writeFile(transcriptPath, "k".repeat(80), "utf-8");
123+
await fs.writeFile(staleTemp, "s".repeat(300), "utf-8");
124+
await fs.writeFile(freshTemp, "f".repeat(300), "utf-8");
125+
// Age the stale temp past the staleness window; the fresh one is in-flight.
126+
const old = new Date(Date.now() - 30 * 60 * 1000);
127+
await fs.utimes(staleTemp, old, old);
128+
129+
const result = await enforceSessionDiskBudget({
130+
store,
131+
storePath,
132+
maintenance: {
133+
maxDiskBytes: 750,
134+
highWaterBytes: 600,
135+
},
136+
warnOnly: false,
137+
});
138+
139+
// Stale orphan reclaimed; fresh in-flight temp (a live atomic-write source)
140+
// and referenced transcript preserved even though still over the high-water mark.
141+
await expectPathMissing(staleTemp);
142+
await expectPathExists(freshTemp);
143+
await expectPathExists(transcriptPath);
144+
expectBudgetResult(result);
145+
expect(result.removedFiles).toBe(1);
146+
});
147+
});
148+
105149
it("preserves runtime-provided session keys when removing entries for disk budget", async () => {
106150
await withTempDir({ prefix: "openclaw-disk-budget-" }, async (dir) => {
107151
const storePath = path.join(dir, "sessions.json");
@@ -287,3 +331,41 @@ describe("enforceSessionDiskBudget", () => {
287331
});
288332
});
289333
});
334+
335+
describe("pruneUnreferencedSessionArtifacts", () => {
336+
it("reclaims stale store temp sidecars but preserves in-flight ones (#56827)", async () => {
337+
await withTempDir({ prefix: "openclaw-prune-temp-" }, async (dir) => {
338+
const storePath = path.join(dir, "sessions.json");
339+
const staleTemp = path.join(
340+
dir,
341+
"sessions.json.111.0f9c1a2b-3c4d-4e5f-8a9b-0c1d2e3f4a5b.tmp",
342+
);
343+
const freshTemp = path.join(
344+
dir,
345+
"sessions.json.222.1a2b3c4d-5e6f-4a8b-9c0d-1e2f3a4b5c6d.tmp",
346+
);
347+
const store: Record<string, SessionEntry> = {
348+
"agent:main:main": { sessionId: "keep", updatedAt: Date.now() },
349+
};
350+
await fs.writeFile(storePath, JSON.stringify(store, null, 2), "utf-8");
351+
await fs.writeFile(staleTemp, "s".repeat(64), "utf-8");
352+
await fs.writeFile(freshTemp, "f".repeat(64), "utf-8");
353+
// Age the stale temp well past the temp staleness window; keep the other in-flight.
354+
const old = new Date(Date.now() - 30 * 60 * 1000);
355+
await fs.utimes(staleTemp, old, old);
356+
357+
const result = await pruneUnreferencedSessionArtifacts({
358+
store,
359+
storePath,
360+
// 30d general cutoff: a stale temp must be reclaimed by its own short window,
361+
// not by the unreferenced-artifact age threshold.
362+
olderThanMs: 30 * 24 * 60 * 60 * 1000,
363+
});
364+
365+
await expectPathMissing(staleTemp);
366+
await expectPathExists(freshTemp);
367+
await expectPathExists(storePath);
368+
expect(result.removedFiles).toBeGreaterThanOrEqual(1);
369+
});
370+
});
371+
});

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

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
isCompactionCheckpointTranscriptFileName,
1313
isPrimarySessionTranscriptFileName,
1414
isSessionArchiveArtifactName,
15+
isSessionStoreTempArtifactName,
1516
isTrajectorySessionArtifactName,
1617
} from "./artifacts.js";
1718
import { resolveSessionFilePath } from "./paths.js";
@@ -228,10 +229,23 @@ function isUnreferencedSessionArtifactFile(
228229
);
229230
}
230231

232+
// An orphaned `sessions.json.<pid>.<uuid>.tmp` older than this is never a live
233+
// atomic write (those rename within milliseconds), so it is safe to reclaim
234+
// regardless of the general unreferenced-artifact age threshold (#56827).
235+
const SESSION_STORE_TEMP_STALE_MS = 5 * 60 * 1000;
236+
231237
function isDiskBudgetRemovableSessionFile(
232-
file: Pick<SessionsDirFileStat, "canonicalPath" | "name">,
238+
file: Pick<SessionsDirFileStat, "canonicalPath" | "name" | "mtimeMs">,
233239
referencedPaths: ReadonlySet<string>,
240+
tempStaleCutoffMs: number,
241+
storeBasename: string,
234242
): boolean {
243+
// Store temps are only removable once clearly stale, even under disk pressure:
244+
// `replaceFileAtomic` uses this exact path as the live source before its rename,
245+
// so deleting a fresh in-flight temp would make another process's save fail.
246+
if (isSessionStoreTempArtifactName(file.name, storeBasename)) {
247+
return file.mtimeMs <= tempStaleCutoffMs;
248+
}
235249
return (
236250
isSessionArchiveArtifactName(file.name) ||
237251
isUnreferencedSessionArtifactFile(file, referencedPaths)
@@ -294,13 +308,20 @@ export async function pruneUnreferencedSessionArtifacts(params: {
294308
store: params.store,
295309
});
296310
const cutoffMs = Date.now() - olderThanMs;
311+
const tempCutoffMs = Date.now() - SESSION_STORE_TEMP_STALE_MS;
312+
const storeBasename = path.basename(params.storePath);
297313
const removableFiles = files
298-
.filter(
299-
(file) =>
300-
!params.excludeCanonicalPaths?.has(file.canonicalPath) &&
301-
file.mtimeMs <= cutoffMs &&
302-
isUnreferencedSessionArtifactFile(file, referencedPaths),
303-
)
314+
.filter((file) => {
315+
if (params.excludeCanonicalPaths?.has(file.canonicalPath)) {
316+
return false;
317+
}
318+
// Orphaned store atomic-write temps are reclaimed on their own short
319+
// staleness window, independent of the unreferenced-artifact age (#56827).
320+
if (isSessionStoreTempArtifactName(file.name, storeBasename)) {
321+
return file.mtimeMs <= tempCutoffMs;
322+
}
323+
return file.mtimeMs <= cutoffMs && isUnreferencedSessionArtifactFile(file, referencedPaths);
324+
})
304325
.toSorted((a, b) => a.mtimeMs - b.mtimeMs);
305326

306327
let removedFiles = 0;
@@ -396,8 +417,12 @@ export async function enforceSessionDiskBudget(params: {
396417
sessionsDir,
397418
store: params.store,
398419
});
420+
const tempStaleCutoffMs = Date.now() - SESSION_STORE_TEMP_STALE_MS;
421+
const storeBasename = path.basename(params.storePath);
399422
const removableFileQueue = files
400-
.filter((file) => isDiskBudgetRemovableSessionFile(file, referencedPaths))
423+
.filter((file) =>
424+
isDiskBudgetRemovableSessionFile(file, referencedPaths, tempStaleCutoffMs, storeBasename),
425+
)
401426
.toSorted((a, b) => a.mtimeMs - b.mtimeMs);
402427
for (const file of removableFileQueue) {
403428
if (total <= highWaterBytes) {

‎src/config/sessions/store.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,14 @@ async function writeSessionStoreAtomic(params: {
593593
store: Record<string, SessionEntry>;
594594
serialized: string;
595595
}): Promise<void> {
596-
await writeTextAtomic(params.storePath, params.serialized, { durable: false, mode: 0o600 });
596+
// Stage the temp as `sessions.json.<pid>.<uuid>.tmp` (not the generic
597+
// `.fs-safe-replace.*`) so a temp orphaned by a crash between write and rename
598+
// is identifiable as a session-store temp and reclaimable by cleanup (#56827).
599+
await writeTextAtomic(params.storePath, params.serialized, {
600+
durable: false,
601+
mode: 0o600,
602+
tempPrefix: path.basename(params.storePath),
603+
});
597604
updateSessionStoreWriteCaches({
598605
storePath: params.storePath,
599606
store: params.store,

‎src/infra/json-files.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,29 @@ describe("json file helpers", () => {
130130
});
131131
});
132132

133+
it("stages the atomic temp with a caller-provided prefix (#56827)", async () => {
134+
await withTempDir({ prefix: "openclaw-json-files-" }, async (base) => {
135+
const filePath = path.join(base, "sessions.json");
136+
// Spy without mocking: rename is still performed, but we capture the staged
137+
// temp path (its source) to confirm the prefix is applied.
138+
const renameSpy = vi.spyOn(fsPromises, "rename");
139+
140+
await writeTextAtomic(filePath, "new", { tempPrefix: path.basename(filePath) });
141+
142+
await expect(fsPromises.readFile(filePath, "utf8")).resolves.toBe("new");
143+
const stagedTemps = renameSpy.mock.calls.map((call) => path.basename(String(call[0])));
144+
// The orphan a crash would leave is now identifiable as a session-store temp.
145+
expect(
146+
stagedTemps.some((name) =>
147+
/^sessions\.json\.\d+\.[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.tmp$/i.test(
148+
name,
149+
),
150+
),
151+
).toBe(true);
152+
expect(stagedTemps.some((name) => name.startsWith(".fs-safe-replace"))).toBe(false);
153+
});
154+
});
155+
133156
it("refuses Windows copy fallback through symlink destinations", async () => {
134157
await withTempDir({ prefix: "openclaw-json-files-" }, async (base) => {
135158
const filePath = path.join(base, "state.json");

‎src/infra/json-files.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,12 @@ export type WriteTextAtomicOptions = {
117117
dirMode?: number;
118118
trailingNewline?: boolean;
119119
durable?: boolean;
120+
/**
121+
* Prefix for the staged `<prefix>.<pid>.<uuid>.tmp` file. Defaults to the
122+
* generic `.fs-safe-replace`; pass a target-specific prefix so an orphaned
123+
* temp (from a crash between write and rename) is identifiable and reclaimable.
124+
*/
125+
tempPrefix?: string;
120126
};
121127

122128
export async function writeTextAtomic(
@@ -133,5 +139,6 @@ export async function writeTextAtomic(
133139
copyFallbackOnPermissionError: true,
134140
syncTempFile: options?.durable !== false,
135141
syncParentDir: options?.durable !== false,
142+
...(options?.tempPrefix ? { tempPrefix: options.tempPrefix } : {}),
136143
});
137144
}

0 commit comments

Comments
 (0)