Skip to content

Commit bd95d0e

Browse files
committed
fix(git): skip one failed remove without staging the batch
Array git.remove throws before the index write, so swallowing that error committed every removed path still staged. Omit the failing path and write the rest once. If nothing can be unstaged, fail the commit instead.
1 parent 1004268 commit bd95d0e

2 files changed

Lines changed: 69 additions & 7 deletions

File tree

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { defined } from "../../testkit/defined.js";
2+
import { withMockedModuleDuring } from "../../testkit/mock-module.js";
23
import { describe, test, expect } from "bun:test";
34
import fs from "node:fs";
45
import os from "node:os";
@@ -1726,4 +1727,50 @@ describe("createOptimizedContextStore prompt dedupe (CL-9026)", () => {
17261727
expect(tree).not.toContain(extraPrompt);
17271728
expect(turnTexts((await store.load()).turns)).toEqual(turnTexts(live));
17281729
});
1730+
1731+
test("one remove failure still unstages the other paths in one index write", async () => {
1732+
const dir = tempDir();
1733+
const store = await createOptimizedContextStore(dir);
1734+
const huge = "x".repeat(300_000);
1735+
const live = [turn(`live-${huge}`), turn("live-tail")];
1736+
await store.writeTurns([...live]);
1737+
await store.writePrompt([turn(`prompt-${huge}`), turn("prompt-tail")]);
1738+
await store.writeMetadata(EMPTY_CHECKPOINT_METADATA);
1739+
await store.commit({ message: "two prompt segments" });
1740+
const extraPrompt = segmentFileName(PROMPT_FILE, 1);
1741+
expect(await gitLsTree(dir)).toEqual(
1742+
expect.arrayContaining([PROMPT_FILE, extraPrompt]),
1743+
);
1744+
1745+
await store.writePrompt(cloneTurns(live));
1746+
await store.writeMetadata(EMPTY_CHECKPOINT_METADATA);
1747+
1748+
const removeCalls: (string | string[])[] = [];
1749+
await withMockedModuleDuring(
1750+
import.meta.resolve("isomorphic-git"),
1751+
(real: typeof import("isomorphic-git")) => {
1752+
const remove: typeof real.default.remove = async (args) => {
1753+
removeCalls.push(args.filepath);
1754+
const paths = Array.isArray(args.filepath)
1755+
? args.filepath
1756+
: [args.filepath];
1757+
if (paths.includes(extraPrompt)) {
1758+
throw new Error("index write failed");
1759+
}
1760+
return real.default.remove(args);
1761+
};
1762+
return { ...real, default: { ...real.default, remove } };
1763+
},
1764+
() => store.commit({ message: "drop prompt segments" }),
1765+
);
1766+
1767+
const tree = await gitLsTree(dir);
1768+
expect(tree).not.toContain(PROMPT_FILE);
1769+
expect(tree).toContain(extraPrompt);
1770+
const written = removeCalls.filter((filepath) => {
1771+
const paths = Array.isArray(filepath) ? filepath : [filepath];
1772+
return !paths.includes(extraPrompt);
1773+
});
1774+
expect(written).toEqual([[PROMPT_FILE]]);
1775+
});
17291776
});

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,19 +1332,34 @@ export async function createSessionStores(
13321332
const remove = [...new Set(toRemove)].filter((p) => !add.includes(p));
13331333
extraPaths = [...new Set([...add, ...remove])];
13341334

1335-
// Array-form add/remove stages and unstages in one index write
1336-
// per call (isomorphic-git >= 1.42.7). On a mid-batch add failure
1337-
// the index is left unwritten and the catch below still resets
1338-
// every extraPath, so the all-or-nothing outcome matches the old
1339-
// per-file loop.
1335+
// Array `git.add` / `git.remove` each write the index once.
1336+
// Array `remove` is isomorphic-git 1.43.0; array `add` is older.
1337+
// A mid-batch add failure leaves the index unwritten. Only the
1338+
// outer catch calls `resetIndexPaths`.
13401339
if (add.length > 0) {
13411340
await git.add({ fs, dir, filepath: add });
13421341
}
13431342
if (remove.length > 0) {
13441343
try {
13451344
await git.remove({ fs, dir, filepath: remove });
1346-
} catch {
1347-
// Already absent from the index.
1345+
} catch (cause) {
1346+
// `GitIndex.delete` does not throw when a path is absent, so
1347+
// this is an index lock or write. It aborts before the index
1348+
// write. Skip the failing path and write the rest once.
1349+
let unstaged = false;
1350+
if (remove.length > 1) {
1351+
for (let index = 0; index < remove.length; index++) {
1352+
const rest = remove.filter((_, other) => other !== index);
1353+
try {
1354+
await git.remove({ fs, dir, filepath: rest });
1355+
unstaged = true;
1356+
break;
1357+
} catch {
1358+
// This omission still aborted before the index write.
1359+
}
1360+
}
1361+
}
1362+
if (!unstaged) throw cause;
13481363
}
13491364
}
13501365
const committed = await base.commit(options, signal);

0 commit comments

Comments
 (0)