Skip to content

Commit 2cca355

Browse files
committed
fix(session): keep sidecar usage when metadata.json is torn
A crash during writeFile left "{not-json" as metadata.json. load() parsed that document before the token-usage sidecar, so the catch assigned emptyMetadata() and dropped a valid sidecar line. Sidecar usage is read even when the document throws. metadata.json is published by renaming a temp file in the same directory, so the published name is never partial.
1 parent f70f38d commit 2cca355

2 files changed

Lines changed: 65 additions & 11 deletions

File tree

‎src/session/optimized-context-store.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,6 +2214,21 @@ describe("createOptimizedContextStore metadata ownership (CL-10667)", () => {
22142214
expect((await store.load()).tokenUsage).toEqual(USAGE_TWO);
22152215
});
22162216

2217+
test("torn metadata.json does not zero a valid sidecar", async () => {
2218+
const dir = tempDir();
2219+
fs.writeFileSync(path.join(dir, "metadata.json"), "{not-json");
2220+
fs.writeFileSync(
2221+
path.join(dir, "token-usage.jsonl"),
2222+
`${JSON.stringify({ turnIndex: 1, usage: USAGE_ONE })}\n`,
2223+
);
2224+
const store = await createOptimizedContextStore(dir);
2225+
const loaded = await store.load();
2226+
expect(loaded.tokenUsage).toEqual(USAGE_ONE);
2227+
expect(loaded.tokenUsage.input).toBe(10);
2228+
expect(loaded.pendingOperations).toEqual([]);
2229+
expect(loaded.connectorState).toBeNull();
2230+
});
2231+
22172232
test("a torn last sidecar line drops and the prior record survives", async () => {
22182233
const dir = tempDir();
22192234
fs.writeFileSync(

‎src/session/optimized-context-store.ts‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -337,19 +337,32 @@ function readMetadataDocument(raw: string): Pick<
337337
}
338338

339339
/**
340-
* metadata.json + sidecar merge behind the same soft-default contract the
341-
* base store had: a throw from the caller's catch falls back to empty
342-
* defaults so a poisoned file cannot block resume.
340+
* metadata.json + sidecar merge. Sidecar wins over a legacy tokenUsage field,
341+
* which wins over empty. Parse failures stay inside this function: a torn
342+
* "{not-json" document soft-defaults pending ops and connector state, but the
343+
* sidecar is read anyway. Letting the throw escape made load()'s catch assign
344+
* emptyMetadata() and report all-zero usage.
343345
*/
344346
async function readSessionMetadata(dir: string): Promise<SessionMetadata> {
345-
const raw = await readTextOrNull(path.join(dir, METADATA_FILE));
346-
const document =
347-
raw !== null && raw.length > 0
348-
? readMetadataDocument(raw)
349-
: { pendingOperations: [], connectorState: null, legacyTokenUsage: null };
350347
const sidecarUsage = await readLatestSidecarUsage(dir);
351-
const tokenUsage = sidecarUsage ??
352-
document.legacyTokenUsage ?? { ...EMPTY_TOKEN_USAGE };
348+
let document: ReturnType<typeof readMetadataDocument> = {
349+
pendingOperations: [],
350+
connectorState: null,
351+
legacyTokenUsage: null,
352+
};
353+
try {
354+
const raw = await readTextOrNull(path.join(dir, METADATA_FILE));
355+
if (raw !== null && raw.length > 0) {
356+
document = readMetadataDocument(raw);
357+
}
358+
} catch (cause) {
359+
log.warn(
360+
"session metadata document unreadable; keeping sidecar usage and empty document defaults",
361+
{ cause: cause instanceof Error ? cause.message : String(cause) },
362+
);
363+
}
364+
const tokenUsage =
365+
sidecarUsage ?? document.legacyTokenUsage ?? { ...EMPTY_TOKEN_USAGE };
353366
return {
354367
pendingOperations: document.pendingOperations,
355368
tokenUsage,
@@ -1121,6 +1134,32 @@ export interface SessionStoreOptions {
11211134
ownerId?: string;
11221135
}
11231136

1137+
let metadataPublishSeq = 0;
1138+
1139+
/**
1140+
* Publish metadata.json only once the bytes are complete. writeFile on the
1141+
* published name can crash mid-write and leave "{not-json" as metadata.json;
1142+
* a temp sibling in the same directory plus rename is the published name
1143+
* appearing with a finished document, same as partial.jsonl.
1144+
*/
1145+
async function publishMetadataDocument(
1146+
dir: string,
1147+
desired: string,
1148+
): Promise<void> {
1149+
const target = path.join(dir, METADATA_FILE);
1150+
const tmp = path.join(
1151+
dir,
1152+
`.${METADATA_FILE}.${process.pid}.${(metadataPublishSeq += 1)}.tmp`,
1153+
);
1154+
try {
1155+
await fs.promises.writeFile(tmp, desired);
1156+
await fs.promises.rename(tmp, target);
1157+
} catch (cause) {
1158+
await fs.promises.rm(tmp, { force: true }).catch(() => undefined);
1159+
throw cause;
1160+
}
1161+
}
1162+
11241163
/**
11251164
* Interchange git-store wrapper that avoids O(session length) work per
11261165
* checkpoint: turns and prompts write as rolling segments so only the active
@@ -1200,7 +1239,7 @@ export async function createSessionStores(
12001239
metadataJsonSeeded = true;
12011240
}
12021241
if (writtenMetadataJson !== desired) {
1203-
await fs.promises.writeFile(path.join(dir, METADATA_FILE), desired);
1242+
await publishMetadataDocument(dir, desired);
12041243
writtenMetadataJson = desired;
12051244
}
12061245
}

0 commit comments

Comments
 (0)