Skip to content

Commit e06021a

Browse files
committed
fix(session): stage a drifted segment without walking clean blobs
Steady-state commits stage a tracked segment when its size, mtime, or inode differs from the last publish, so an external append lands without re-reading unchanged segments. A no-op skips the tool-output status walk unless those directories are still unobserved or their fingerprint moved.
1 parent c367d00 commit e06021a

2 files changed

Lines changed: 210 additions & 15 deletions

File tree

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,39 @@ describe("createOptimizedContextStore checkpoint", () => {
709709
expect(reads).toBe(0);
710710
});
711711

712+
test("an external append to a tracked segment is in the next commit", async () => {
713+
const dir = tempDir();
714+
const store = await createOptimizedContextStore(dir);
715+
const chunk = "s".repeat(100_000);
716+
const turns: ConversationTurn[] = [];
717+
const closed = segmentFileName(TURNS_FILE, 1);
718+
const tail = segmentFileName(TURNS_FILE, 2);
719+
do {
720+
turns.push(turn(`${turns.length}-${chunk}`));
721+
await store.writeTurns(turns);
722+
} while (!fs.existsSync(path.join(dir, tail)) && turns.length < 20);
723+
expect(fs.existsSync(path.join(dir, closed))).toBe(true);
724+
await store.writeMetadata(EMPTY_CHECKPOINT_METADATA);
725+
const first = await store.commit({ message: "three segments" });
726+
727+
const marker = "external-drift-bytes";
728+
fs.appendFileSync(path.join(dir, closed), `${marker}\n`);
729+
let secondHash = "";
730+
const unchangedReads = await countPathOps("readFile", tail, async () => {
731+
const second = await store.commit({ message: "publish drifted segment" });
732+
secondHash = second.hash;
733+
});
734+
expect(unchangedReads).toBe(0);
735+
expect(secondHash).not.toBe(first.hash);
736+
const { blob } = await git.readBlob({
737+
fs,
738+
dir,
739+
oid: secondHash,
740+
filepath: closed,
741+
});
742+
expect(new TextDecoder().decode(blob)).toContain(marker);
743+
});
744+
712745
test("a turn commit does not lstat every seeded tool-output blob", async () => {
713746
const dir = tempDir();
714747
const store = await createOptimizedContextStore(dir);
@@ -740,6 +773,29 @@ describe("createOptimizedContextStore checkpoint", () => {
740773
expect(lstats).toBeLessThan(blobCount + blobCount / 2);
741774
});
742775

776+
test("a no-op commit does not lstat every tool-output blob", async () => {
777+
const dir = tempDir();
778+
const store = await createOptimizedContextStore(dir);
779+
const blobCount = 24;
780+
for (let index = 0; index < blobCount; index++) {
781+
await store.writeBlob(
782+
`blob-${index}`,
783+
new TextEncoder().encode(`body-${index}`),
784+
"text/plain",
785+
);
786+
}
787+
await store.writeTurns([turn("seed")]);
788+
await store.writeMetadata(EMPTY_CHECKPOINT_METADATA);
789+
await store.commit({ message: "seed blobs" });
790+
791+
const lstats = await countPathOps(
792+
"lstat",
793+
`${path.sep}tool-output${path.sep}`,
794+
() => store.commit({ message: "no writes" }),
795+
);
796+
expect(lstats).toBe(0);
797+
});
798+
743799
test("seeded tool-output does not commit a no-op and a fresh store still heals blob bytes", async () => {
744800
const dir = tempDir();
745801
const store = await createOptimizedContextStore(dir);

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

Lines changed: 154 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -882,21 +882,75 @@ function highestSegmentIndexFromListing(
882882
return highest;
883883
}
884884

885+
/**
886+
* Size, mtime, and inode of one segment file. Compared to the fingerprint
887+
* saved at the last publish so a byte change stages the file and an unchanged
888+
* file is not re-read.
889+
*/
890+
interface SegmentStat {
891+
bytes: number;
892+
mtimeMs: number;
893+
ino: number;
894+
}
895+
896+
function segmentStatDrifted(
897+
observed: SegmentStat | undefined,
898+
published: SegmentStat | undefined,
899+
): boolean {
900+
if (observed === undefined || published === undefined) return true;
901+
return (
902+
observed.bytes !== published.bytes ||
903+
observed.mtimeMs !== published.mtimeMs ||
904+
observed.ino !== published.ino
905+
);
906+
}
907+
885908
/**
886909
* One readdir of the session dir answers every worktree-existence check in a
887-
* single commit, including the lazy listing of the tool-output dir. Surveyed
888-
* after the staged writes land, used before the lock releases.
910+
* single commit, including the lazy listing of the tool-output dir. Contiguous
911+
* turn and prompt segments are statted once here; that fingerprint is the
912+
* drift signal, not a content read. Surveyed after the staged writes land,
913+
* used before the lock releases.
889914
*/
890915
interface WorktreeListing {
891916
top: ReadonlySet<string>;
917+
segmentStats: ReadonlyMap<string, SegmentStat>;
892918
toolOutputFiles: () => Promise<ReadonlySet<string>>;
893919
}
894920

895-
async function surveyWorktree(dir: string): Promise<WorktreeListing> {
921+
async function segmentStatsForListing(
922+
dir: string,
923+
top: ReadonlySet<string>,
924+
turnsBaseName: string,
925+
): Promise<Map<string, SegmentStat>> {
926+
const names = [
927+
...contiguousSegmentNamesFromListing(top, turnsBaseName),
928+
...contiguousSegmentNamesFromListing(top, PROMPT_FILE),
929+
];
930+
const stats = new Map<string, SegmentStat>();
931+
await Promise.all(
932+
names.map(async (name) => {
933+
const fileStats = await fs.promises.stat(path.join(dir, name));
934+
stats.set(name, {
935+
bytes: fileStats.size,
936+
mtimeMs: fileStats.mtimeMs,
937+
ino: fileStats.ino,
938+
});
939+
}),
940+
);
941+
return stats;
942+
}
943+
944+
async function surveyWorktree(
945+
dir: string,
946+
turnsBaseName: string,
947+
): Promise<WorktreeListing> {
896948
const top = new Set(await fs.promises.readdir(dir));
949+
const segmentStats = await segmentStatsForListing(dir, top, turnsBaseName);
897950
let blobListing: Promise<ReadonlySet<string>> | null = null;
898951
return {
899952
top,
953+
segmentStats,
900954
toolOutputFiles() {
901955
blobListing ??= fs.promises
902956
.readdir(path.join(dir, TOOL_OUTPUT_DIR))
@@ -1100,10 +1154,12 @@ async function restoreTurnSegments(
11001154
*
11011155
* `stageUnchanged` re-adds every contiguous name (disk-as-truth for a rewrite,
11021156
* or the first commit of a store whose pending set may have been lost). After
1103-
* this store has published once, steady-state commits pass false and only
1104-
* stage untracked names here; the writer's modified paths are already pending.
1105-
* Removals stay disk-as-truth either way, so a heal unlink still unstages the
1106-
* same paths snapshot/restore would put back.
1157+
* this store has published once, steady-state commits stage a tracked segment
1158+
* only when its listing fingerprint (size, mtime, inode) differs from the
1159+
* fingerprint saved at the last publish. An unchanged fingerprint is not
1160+
* passed to `git.add`, so the file is not re-read. Untracked names are still
1161+
* added. Removals stay disk-as-truth either way, so a heal unlink still
1162+
* unstages the same paths snapshot/restore would put back.
11071163
*/
11081164
async function reconcileSegmentStaging(
11091165
dir: string,
@@ -1113,13 +1169,25 @@ async function reconcileSegmentStaging(
11131169
listing: WorktreeListing,
11141170
tracked: ReadonlySet<string>,
11151171
stageUnchanged: boolean,
1172+
publishedSegmentStats: ReadonlyMap<string, SegmentStat>,
11161173
): Promise<void> {
11171174
const contiguous = contiguousSegmentNamesFromListing(listing.top, baseName);
11181175
if (stageUnchanged) {
11191176
for (const name of contiguous) toAdd.push(name);
11201177
} else {
11211178
for (const name of contiguous) {
1122-
if (!tracked.has(name)) toAdd.push(name);
1179+
if (!tracked.has(name)) {
1180+
toAdd.push(name);
1181+
continue;
1182+
}
1183+
if (
1184+
segmentStatDrifted(
1185+
listing.segmentStats.get(name),
1186+
publishedSegmentStats.get(name),
1187+
)
1188+
) {
1189+
toAdd.push(name);
1190+
}
11231191
}
11241192
}
11251193

@@ -1157,13 +1225,16 @@ function extraCommitPaths(paths: readonly string[]): string[] {
11571225
* isomorphic-git applies `filepaths` inside the walk map, after it has already
11581226
* readdir'd a directory and lstat'd each child, so including `tool-output`
11591227
* stats every blob. Root files are checked first; a dirty one skips the
1160-
* directory walk entirely. The directory walk remains the empty-commit
1161-
* backstop, so a blob-only change still commits and a clean tree does not.
1228+
* directory walk entirely. `walkBlobDirectories` is the separate signal for a
1229+
* blob-only change: a store that has not yet observed the blob directories,
1230+
* or whose directory fingerprint (mtime, inode) moved, still walks. A no-op
1231+
* whose fingerprint is unchanged passes false and does not lstat every blob.
11621232
*/
11631233
async function managedPathsDiffer(
11641234
dir: string,
11651235
filepaths: readonly string[],
11661236
cache: GitCache,
1237+
walkBlobDirectories: boolean,
11671238
): Promise<boolean> {
11681239
if (filepaths.length === 0) return false;
11691240
const rootFiles: string[] = [];
@@ -1191,6 +1262,7 @@ async function managedPathsDiffer(
11911262
for (const filepath of nestedFiles) {
11921263
if (await nestedWorktreeDiffers(dir, filepath, cache)) return true;
11931264
}
1265+
if (!walkBlobDirectories) return false;
11941266
for (const filepath of directories) {
11951267
const matrix = await git.statusMatrix({
11961268
...cachedGit(dir, cache),
@@ -1201,6 +1273,35 @@ async function managedPathsDiffer(
12011273
return false;
12021274
}
12031275

1276+
interface DirectoryFingerprint {
1277+
mtimeMs: number;
1278+
ino: number;
1279+
}
1280+
1281+
function sameDirectoryFingerprint(
1282+
left: DirectoryFingerprint | null,
1283+
right: DirectoryFingerprint | null,
1284+
): boolean {
1285+
if (left === null || right === null) return left === right;
1286+
return left.mtimeMs === right.mtimeMs && left.ino === right.ino;
1287+
}
1288+
1289+
async function directoryFingerprint(
1290+
dir: string,
1291+
name: string,
1292+
): Promise<DirectoryFingerprint | null> {
1293+
try {
1294+
const stats = await fs.promises.lstat(path.join(dir, name));
1295+
if (!stats.isDirectory()) return null;
1296+
return { mtimeMs: stats.mtimeMs, ino: stats.ino };
1297+
} catch (cause) {
1298+
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") {
1299+
return null;
1300+
}
1301+
throw cause;
1302+
}
1303+
}
1304+
12041305
/**
12051306
* HEAD-vs-worktree for one file nested under a managed directory, without
12061307
* walking sibling blobs. Used only after root files already compared clean.
@@ -1344,8 +1445,34 @@ export async function createSessionStores(
13441445
const gitCache: GitCache = {};
13451446
// Full contiguous re-stage until this store has observed a clean or published
13461447
// generation. A fresh process heals disk the same way a rewrite does; later
1347-
// commits stage only the writer's modified paths plus untracked strays.
1448+
// commits stage a tracked segment only when its fingerprint drifted.
13481449
let stageEveryContiguousSegment = true;
1450+
// Fingerprints from the last publish. Segment stats avoid re-reading an
1451+
// unchanged segment; blob directory stats are the signal that skips the
1452+
// tool-output walk on a no-op commit.
1453+
let publishedSegmentStats = new Map<string, SegmentStat>();
1454+
let blobDirsObserved = false;
1455+
let toolOutputFingerprint: DirectoryFingerprint | null = null;
1456+
let evidenceFingerprint: DirectoryFingerprint | null = null;
1457+
1458+
async function blobDirectoriesNeedWalk(): Promise<boolean> {
1459+
if (!blobDirsObserved) return true;
1460+
const tool = await directoryFingerprint(dir, TOOL_OUTPUT_DIR);
1461+
const evidence = await directoryFingerprint(dir, EVIDENCE_ARCHIVE_DIR);
1462+
return (
1463+
!sameDirectoryFingerprint(tool, toolOutputFingerprint) ||
1464+
!sameDirectoryFingerprint(evidence, evidenceFingerprint)
1465+
);
1466+
}
1467+
1468+
async function rememberPublishedTree(
1469+
listing: WorktreeListing,
1470+
): Promise<void> {
1471+
publishedSegmentStats = new Map(listing.segmentStats);
1472+
toolOutputFingerprint = await directoryFingerprint(dir, TOOL_OUTPUT_DIR);
1473+
evidenceFingerprint = await directoryFingerprint(dir, EVIDENCE_ARCHIVE_DIR);
1474+
blobDirsObserved = true;
1475+
}
13491476
const ownerToken = opts?.ownerId ?? globalThis.crypto.randomUUID();
13501477
const explicitOwnerId = opts?.ownerId;
13511478
let turnsBaseName = TURNS_FILE;
@@ -2084,7 +2211,7 @@ export async function createSessionStores(
20842211
await writeTurnLog(stagedRewrite);
20852212
invalidateTurnsCache();
20862213
}
2087-
const listing = await surveyWorktree(dir);
2214+
const listing = await surveyWorktree(dir, turnsBaseName);
20882215
const toAdd: string[] = [];
20892216
const toRemove: string[] = [];
20902217

@@ -2101,8 +2228,9 @@ export async function createSessionStores(
21012228
// rewrite or heal, even when pendingSegmentPaths was lost. A rewrite,
21022229
// and the first commit of a store that has not yet published, re-add
21032230
// every contiguous segment so that staging matches snapshot/restore.
2104-
// Later commits leave unchanged segments unstaged: the writer already
2105-
// recorded modified paths, and untracked names are still added.
2231+
// Later commits stage a tracked segment only when its size, mtime, or
2232+
// inode drifted, so an external append is published and an unchanged
2233+
// segment is not re-read. Untracked names are still added.
21062234
const tracked = await listIndexPaths(dir, gitCache);
21072235
const stageUnchanged =
21082236
stagedRewrite !== null || stageEveryContiguousSegment;
@@ -2114,6 +2242,7 @@ export async function createSessionStores(
21142242
listing,
21152243
tracked,
21162244
stageUnchanged,
2245+
publishedSegmentStats,
21172246
);
21182247
await reconcileSegmentStaging(
21192248
dir,
@@ -2123,6 +2252,7 @@ export async function createSessionStores(
21232252
listing,
21242253
tracked,
21252254
stageUnchanged,
2255+
publishedSegmentStats,
21262256
);
21272257

21282258
if (listing.top.has(EVIDENCE_ARCHIVE_DIR)) {
@@ -2167,12 +2297,20 @@ export async function createSessionStores(
21672297
...toAdd,
21682298
...toRemove,
21692299
];
2170-
if (!(await managedPathsDiffer(dir, managedFilepaths, gitCache))) {
2300+
if (
2301+
!(await managedPathsDiffer(
2302+
dir,
2303+
managedFilepaths,
2304+
gitCache,
2305+
await blobDirectoriesNeedWalk(),
2306+
))
2307+
) {
21712308
const [head] = await base.log(1);
21722309
if (head !== undefined) {
21732310
pendingBlobFilepaths.clear();
21742311
pendingSegmentPaths.clear();
21752312
stageEveryContiguousSegment = false;
2313+
await rememberPublishedTree(listing);
21762314
return head;
21772315
}
21782316
}
@@ -2225,6 +2363,7 @@ export async function createSessionStores(
22252363
pendingBlobFilepaths.clear();
22262364
pendingSegmentPaths.clear();
22272365
stageEveryContiguousSegment = false;
2366+
await rememberPublishedTree(listing);
22282367
if (stagedRewrite !== null) {
22292368
liveTurnRefs = stagedRewrite;
22302369
unpublishedRewrite = null;

0 commit comments

Comments
 (0)