Skip to content

Commit fa6f3ab

Browse files
committed
fix(session): stop the base store warning on every session load
The live-turns read went through the isogit base's load(), which also validates metadata.json against the vendor shape. This store writes token usage to the sidecar instead, so every cold load failed that validation, warned, and re-parsed turns.jsonl from disk. Parse the base segment directly; metadata already has its own lenient reader that still warns on a malformed document. Relaxing the vendored schema would add a ledgered patch for a file the base store no longer owns.
1 parent ad064f9 commit fa6f3ab

2 files changed

Lines changed: 95 additions & 32 deletions

File tree

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { defined } from "../../testkit/defined.js";
2+
import { withFileLogSink } from "../../testkit/file-log-sink.js";
23
import { withMockedModuleDuring } from "../../testkit/mock-module.js";
34
import { describe, test, expect } from "bun:test";
45
import fs from "node:fs";
@@ -14,6 +15,7 @@ import {
1415
} from "./optimized-context-store.js";
1516
import { segmentFileName, listSegmentFiles } from "./incremental-jsonl.js";
1617
import { createPruningCompactor } from "./compactor.js";
18+
import { LOG_NAMESPACE_ROOT } from "../branding.js";
1719

1820
const TURNS_FILE = "turns.jsonl";
1921

@@ -422,6 +424,81 @@ describe("createOptimizedContextStore load", () => {
422424
});
423425
});
424426

427+
describe("createOptimizedContextStore load warnings", () => {
428+
function contextStoreWarnings(logged: string): string[] {
429+
return logged
430+
.split("\n")
431+
.filter((line) => line.length > 0)
432+
.map((line) => JSON.parse(line) as Record<string, unknown>)
433+
.filter(
434+
(record) =>
435+
record.level === "warning" &&
436+
record.category === `${LOG_NAMESPACE_ROOT}.session.context-store`,
437+
)
438+
.map((record) => JSON.stringify(record));
439+
}
440+
441+
const pendingOp = {
442+
correlationId: "corr-1",
443+
kind: "approval" as const,
444+
registeredAt: 1_700_000_000_000,
445+
gateId: "gate-1",
446+
};
447+
const usage = {
448+
input: 7,
449+
output: 3,
450+
cacheRead: 0,
451+
cacheWrite: 0,
452+
thinking: 0,
453+
};
454+
455+
test("a cold load of a store written the current way logs no warning", async () => {
456+
const dir = tempDir();
457+
const writer = await createOptimizedContextStore(dir);
458+
await writer.writeTurns([turn("a"), turn("b")]);
459+
await writer.writeMetadata({
460+
pendingOperations: [pendingOp],
461+
tokenUsage: usage,
462+
});
463+
await writer.commit({ message: "checkpoint: tool-execution" });
464+
465+
let loaded: Awaited<ReturnType<typeof writer.load>> | undefined;
466+
const logged = await withFileLogSink(async () => {
467+
const resumed = await createOptimizedContextStore(dir);
468+
loaded = await resumed.load();
469+
});
470+
471+
expect(contextStoreWarnings(logged)).toEqual([]);
472+
expect(turnTexts(defined(loaded).turns)).toEqual(["a", "b"]);
473+
expect(defined(loaded).pendingOperations).toEqual([pendingOp]);
474+
expect(defined(loaded).tokenUsage).toEqual(usage);
475+
});
476+
477+
test("a malformed metadata.json still warns on load", async () => {
478+
const dir = tempDir();
479+
const writer = await createOptimizedContextStore(dir);
480+
await writer.writeTurns([turn("a")]);
481+
await writer.writeMetadata(EMPTY_CHECKPOINT_METADATA);
482+
await writer.commit({ message: "checkpoint: tool-execution" });
483+
fs.writeFileSync(
484+
path.join(dir, "metadata.json"),
485+
JSON.stringify({ pendingOperations: 42 }),
486+
);
487+
488+
let loaded: Awaited<ReturnType<typeof writer.load>> | undefined;
489+
const logged = await withFileLogSink(async () => {
490+
const resumed = await createOptimizedContextStore(dir);
491+
loaded = await resumed.load();
492+
});
493+
494+
const warnings = contextStoreWarnings(logged);
495+
expect(warnings).toHaveLength(1);
496+
expect(warnings[0]).toContain("metadata.json has unexpected structure");
497+
expect(turnTexts(defined(loaded).turns)).toEqual(["a"]);
498+
expect(defined(loaded).pendingOperations).toEqual([]);
499+
});
500+
});
501+
425502
describe("loadRecentTurns", () => {
426503
test("reads only the tail segments needed to satisfy the window, in order", async () => {
427504
const dir = tempDir();

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

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,8 @@ export async function loadRecentTurns(
510510
}
511511

512512
/**
513-
* Resilient parse of `turns.jsonl` alone (extras merged by the caller).
514-
* Mirrors `load()`'s recovery when the isogit base hard-fails, so a torn or
515-
* poisoned base cannot block the healing write.
513+
* Resilient parse of `turns.jsonl` alone (extras merged by the caller), so a
514+
* torn or poisoned base cannot block the healing write.
516515
*/
517516
async function readBaseTurnsFromDisk(dir: string): Promise<ConversationTurn[]> {
518517
const basePath = path.join(dir, TURNS_FILE);
@@ -1364,9 +1363,7 @@ export async function createSessionStores(
13641363
return count;
13651364
}
13661365

1367-
async function readLiveTurnsCached(
1368-
signal?: AbortSignal,
1369-
): Promise<ConversationTurn[]> {
1366+
async function readLiveTurnsCached(): Promise<ConversationTurn[]> {
13701367
const segments = await turnSegmentStats();
13711368
if (
13721369
turnsCache !== null &&
@@ -1381,23 +1378,14 @@ export async function createSessionStores(
13811378
: sharedSegmentPrefix(turnsCache.segments, segments);
13821379
// Base segment stat unchanged: its parse is still exactly what disk
13831380
// holds, so only the newer segments get re-read and re-parsed.
1384-
let baseTurns: ConversationTurn[];
1385-
if (reused >= 1 && turnsCache !== null) {
1386-
baseTurns = turnsCache.baseTurns;
1387-
} else {
1388-
try {
1389-
baseTurns = (await base.load(signal)).turns;
1390-
} catch (cause) {
1391-
// A torn or poisoned base must not block the healing write; recover
1392-
// the usable turns the same way load() always has. Corrupt
1393-
// metadata.json also slides — callers here only need the turns.
1394-
log.warn(
1395-
"base context store load failed; recovering base segment from disk",
1396-
{ cause: cause instanceof Error ? cause.message : String(cause) },
1397-
);
1398-
baseTurns = await readBaseTurnsFromDisk(dir);
1399-
}
1400-
}
1381+
// Otherwise parse turns.jsonl here rather than through the isogit base's
1382+
// load(): that also validates metadata.json against the vendor shape,
1383+
// which this store no longer writes (tokenUsage lives on the sidecar).
1384+
// Metadata is read separately by readSessionMetadata.
1385+
const baseTurns =
1386+
reused >= 1 && turnsCache !== null
1387+
? turnsCache.baseTurns
1388+
: await readBaseTurnsFromDisk(dir);
14011389
const parsedExtras: ConversationTurn[][] = [];
14021390
for (let index = 1; index < segments.length; index++) {
14031391
const cached =
@@ -1428,10 +1416,8 @@ export async function createSessionStores(
14281416
return merged;
14291417
}
14301418

1431-
async function readLiveTurns(
1432-
signal?: AbortSignal,
1433-
): Promise<ConversationTurn[]> {
1434-
return readLiveTurnsCached(signal);
1419+
async function readLiveTurns(): Promise<ConversationTurn[]> {
1420+
return readLiveTurnsCached();
14351421
}
14361422

14371423
async function resolveFirstWrite(
@@ -1644,15 +1630,15 @@ export async function createSessionStores(
16441630

16451631
const store: ContextStore & AuditStore = {
16461632
// Full-history read: the reactor's initialization contract. Turns come
1647-
// through the shared generation cache (strict base parse with resilient
1648-
// recovery, resilient extras, orphan-tail heal), so a repeated load within
1649-
// one store generation does not re-parse segments whose stat is unchanged.
1633+
// through the shared generation cache (resilient base and extras,
1634+
// orphan-tail heal), so a repeated load within one store generation does
1635+
// not re-parse segments whose stat is unchanged.
16501636
// Metadata stays a fresh read: corrupt metadata.json soft-defaults so
16511637
// resume does not die or wipe pending ops.
1652-
async load(signal) {
1638+
async load() {
16531639
let turns: ConversationTurn[];
16541640
try {
1655-
turns = await readLiveTurnsCached(signal);
1641+
turns = await readLiveTurnsCached();
16561642
} catch (cause) {
16571643
// Unrecoverable history: rethrow with the file name in the message.
16581644
throw new Error(

0 commit comments

Comments
 (0)