diff --git a/bun.lock b/bun.lock index 82d4740ea..71b65b73c 100644 --- a/bun.lock +++ b/bun.lock @@ -735,7 +735,7 @@ "isexe": ["isexe@3.1.5", "", {}, "sha512-6B3tLtFqtQS4ekarvLVMZ+X+VlvQekbe4taUkf/rhVO3d/h0M2rfARm/pXLcPEsjjMsFgrFgSrhQIxcSVrBz8w=="], - "isomorphic-git": ["isomorphic-git@1.41.9", "", { "dependencies": { "async-lock": "^1.4.1", "clean-git-ref": "^2.0.1", "crc-32": "^1.2.0", "diff3": "0.0.3", "ignore": "^5.1.4", "minimisted": "^2.0.0", "pako": "^1.0.10", "pify": "^4.0.1", "readable-stream": "^4.0.0", "sha.js": "^2.4.12", "simple-get": "^4.0.1" }, "bin": { "isogit": "cli.cjs" } }, "sha512-WOh4ujm5mznHphzQvwj9bVj+pQpV0oZ8H4lAnh4dSaie+lN/lJ2rb6rNOOcP+2m4z8IOQp186TkEULD28NMBJg=="], + "isomorphic-git": ["isomorphic-git@1.44.2", "", { "dependencies": { "async-lock": "^1.4.1", "clean-git-ref": "^2.0.1", "crc-32": "^1.2.0", "diff3": "0.0.3", "ignore": "^5.1.4", "minimisted": "^2.0.0", "pako": "^1.0.10", "pify": "^4.0.1", "readable-stream": "^4.0.0", "sha.js": "^2.4.12", "simple-get": "^4.0.1" }, "bin": { "isogit": "cli.cjs" } }, "sha512-RORfhiNxaHRkGXvTiIW1SU6ngH/jW7UnKndvIToGxrGja7s317CUcZ5chENyinnyCPz7Z4vAefA75Ty1Zsyc8w=="], "isomorphic-textencoder": ["isomorphic-textencoder@1.0.1", "", { "dependencies": { "fast-text-encoding": "^1.0.0" } }, "sha512-676hESgHullDdHDsj469hr+7t3i/neBKU9J7q1T4RHaWwLAsaQnywC0D1dIUId0YZ+JtVrShzuBk1soo0+GVcQ=="], diff --git a/src/session/optimized-context-store.test.ts b/src/session/optimized-context-store.test.ts index c6bfd9835..c66ac13c0 100644 --- a/src/session/optimized-context-store.test.ts +++ b/src/session/optimized-context-store.test.ts @@ -1,4 +1,5 @@ import { defined } from "../../testkit/defined.js"; +import { withMockedModuleDuring } from "../../testkit/mock-module.js"; import { describe, test, expect } from "bun:test"; import fs from "node:fs"; import os from "node:os"; @@ -1726,4 +1727,50 @@ describe("createOptimizedContextStore prompt dedupe (CL-9026)", () => { expect(tree).not.toContain(extraPrompt); expect(turnTexts((await store.load()).turns)).toEqual(turnTexts(live)); }); + + test("one remove failure still unstages the other paths in one index write", async () => { + const dir = tempDir(); + const store = await createOptimizedContextStore(dir); + const huge = "x".repeat(300_000); + const live = [turn(`live-${huge}`), turn("live-tail")]; + await store.writeTurns([...live]); + await store.writePrompt([turn(`prompt-${huge}`), turn("prompt-tail")]); + await store.writeMetadata(EMPTY_CHECKPOINT_METADATA); + await store.commit({ message: "two prompt segments" }); + const extraPrompt = segmentFileName(PROMPT_FILE, 1); + expect(await gitLsTree(dir)).toEqual( + expect.arrayContaining([PROMPT_FILE, extraPrompt]), + ); + + await store.writePrompt(cloneTurns(live)); + await store.writeMetadata(EMPTY_CHECKPOINT_METADATA); + + const removeCalls: (string | string[])[] = []; + await withMockedModuleDuring( + import.meta.resolve("isomorphic-git"), + (real: typeof import("isomorphic-git")) => { + const remove: typeof real.default.remove = async (args) => { + removeCalls.push(args.filepath); + const paths = Array.isArray(args.filepath) + ? args.filepath + : [args.filepath]; + if (paths.includes(extraPrompt)) { + throw new Error("index write failed"); + } + return real.default.remove(args); + }; + return { ...real, default: { ...real.default, remove } }; + }, + () => store.commit({ message: "drop prompt segments" }), + ); + + const tree = await gitLsTree(dir); + expect(tree).not.toContain(PROMPT_FILE); + expect(tree).toContain(extraPrompt); + const written = removeCalls.filter((filepath) => { + const paths = Array.isArray(filepath) ? filepath : [filepath]; + return !paths.includes(extraPrompt); + }); + expect(written).toEqual([[PROMPT_FILE]]); + }); }); diff --git a/src/session/optimized-context-store.ts b/src/session/optimized-context-store.ts index 5558ca5ea..157b5ae69 100644 --- a/src/session/optimized-context-store.ts +++ b/src/session/optimized-context-store.ts @@ -1332,14 +1332,34 @@ export async function createSessionStores( const remove = [...new Set(toRemove)].filter((p) => !add.includes(p)); extraPaths = [...new Set([...add, ...remove])]; - for (const filepath of add) { - await git.add({ fs, dir, filepath }); + // Array `git.add` / `git.remove` each write the index once. + // Array `remove` is isomorphic-git 1.43.0; array `add` is older. + // A mid-batch add failure leaves the index unwritten. Only the + // outer catch calls `resetIndexPaths`. + if (add.length > 0) { + await git.add({ fs, dir, filepath: add }); } - for (const filepath of remove) { + if (remove.length > 0) { try { - await git.remove({ fs, dir, filepath }); - } catch { - // Already absent from the index. + await git.remove({ fs, dir, filepath: remove }); + } catch (cause) { + // `GitIndex.delete` does not throw when a path is absent, so + // this is an index lock or write. It aborts before the index + // write. Skip the failing path and write the rest once. + let unstaged = false; + if (remove.length > 1) { + for (let index = 0; index < remove.length; index++) { + const rest = remove.filter((_, other) => other !== index); + try { + await git.remove({ fs, dir, filepath: rest }); + unstaged = true; + break; + } catch { + // This omission still aborted before the index write. + } + } + } + if (!unstaged) throw cause; } } const committed = await base.commit(options, signal);