Skip to content

Commit 559efd4

Browse files
committed
Preserve a worktree instead of silently dropping its stash entry
git status never reports a git stash the sub-agent ran mid-task, so a worktree holding one looked clean and got removed automatically — the stash entry survives in the repo's shared refs/stash either way, but went silently orphaned with no indication of which worktree it came from. createSubAgentWorktree now captures the stash list as a baseline; cleanup diffs the current list against it and preserves the worktree with a notice naming the new stash entries instead of removing it. Also preserves the caught error as "cause" on both WorktreeError throw sites in this file, and collapses two stray double blank lines.
1 parent 5b5d0f2 commit 559efd4

5 files changed

Lines changed: 137 additions & 15 deletions

File tree

src/subagent/task-tool-worktree.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,45 @@ describe("createTaskTool worktree isolation", () => {
169169
const { stdout } = await run("git", ["worktree", "list"], { cwd: repo });
170170
expect(stdout).toContain(worktreePath!);
171171
});
172+
173+
test("preserves a worktree the sub-agent left stashed, with a notice naming the stash", async () => {
174+
const repo = await makeRepo();
175+
tempDirs.push(repo);
176+
const workdirBase = await mkdtemp(join(tmpdir(), "corbits-workdir-"));
177+
tempDirs.push(workdirBase);
178+
179+
let worktreePath: string | undefined;
180+
const tool = createTaskTool({
181+
permissionGate: testPermissionGate,
182+
cwd: repo,
183+
getWorkdirBase: () => workdirBase,
184+
provider,
185+
useWorktree: true,
186+
run: async (params) => {
187+
worktreePath = params.cwd;
188+
// Simulate the sub-agent stashing mid-task: `git status` reports
189+
// clean afterward even though the work is not actually gone — it is
190+
// parked in the repo's shared refs/stash.
191+
await writeFile(join(params.cwd, "wip.txt"), "half-finished change");
192+
await run("git", ["add", "."], { cwd: params.cwd });
193+
await run("git", ["stash"], { cwd: params.cwd });
194+
return "done";
195+
},
196+
});
197+
198+
const result = await callTask(tool, { description: "Stashing job", prompt: "Do the work" });
199+
200+
expect(result).toContain("done");
201+
expect(result).toContain("stash");
202+
expect(worktreePath).toBeDefined();
203+
204+
// The worktree itself is preserved rather than silently removed —
205+
// `git status` alone would have called this clean.
206+
const { stdout } = await run("git", ["worktree", "list"], { cwd: repo });
207+
expect(stdout).toContain(worktreePath!);
208+
209+
// The stash entry the sub-agent created is still recoverable.
210+
const { stdout: stashList } = await run("git", ["stash", "list"], { cwd: repo });
211+
expect(stashList).toContain("stash@{0}");
212+
});
172213
});

src/subagent/task-tool.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,11 +490,13 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
490490
}
491491

492492
let worktreeCwd: string | undefined;
493+
let worktreeStashBaseline: readonly string[] = [];
493494
if (deps.useWorktree === true) {
494495
const worktreePath = join(deps.getWorkdirBase(), "worktrees", generateSessionId());
495496
try {
496497
const worktree = await createSubAgentWorktree(deps.cwd, worktreePath);
497498
worktreeCwd = worktree.path;
499+
worktreeStashBaseline = worktree.stashBaseline;
498500
} catch (err) {
499501
// Admit already happened and the strip session may be "running" —
500502
// release the ledger slot and fail the session so a worktree setup
@@ -514,7 +516,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
514516
// (or preserved with a notice) rather than leaked.
515517
const finishWithWorktree = async (result: ToolResult): Promise<ToolResult> => {
516518
if (worktreeCwd === undefined) return result;
517-
const cleanup = await cleanupSubAgentWorktree(deps.cwd, worktreeCwd);
519+
const cleanup = await cleanupSubAgentWorktree(deps.cwd, worktreeCwd, worktreeStashBaseline);
518520
if (cleanup.status === "preserved") {
519521
return { ...result, content: `${result.content}\n\n${cleanup.notice}` };
520522
}
@@ -583,7 +585,6 @@ const reported = appendSubAgentParentHints(result, hintOptions);
583585
taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`),
584586
);
585587

586-
587588
} catch (err) {
588589
if (
589590
isSubAgentCancelError(err, childCtl.signal) ||

src/subagent/worktree.test.ts

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,28 @@ describe("createSubAgentWorktree", () => {
2626
const { exec, calls } = recordingExec({
2727
"rev-parse": { stdout: "/repo\n" },
2828
worktree: { stdout: "" },
29+
stash: { stdout: "" },
2930
});
3031
const result = await createSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
3132
expect(result.path).toBe("/repo/.worktrees/abc");
33+
expect(result.stashBaseline).toEqual([]);
3234
expect(calls).toEqual([
3335
["rev-parse", "--show-toplevel"],
3436
["worktree", "add", "--detach", "/repo/.worktrees/abc", "HEAD"],
37+
["stash", "list"],
3538
]);
3639
});
3740

41+
test("captures the current stash list as a baseline", async () => {
42+
const { exec } = recordingExec({
43+
"rev-parse": { stdout: "/repo\n" },
44+
worktree: { stdout: "" },
45+
stash: { stdout: "stash@{0}: WIP on main: abc1234 pre-existing stash\n" },
46+
});
47+
const result = await createSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
48+
expect(result.stashBaseline).toEqual(["stash@{0}: WIP on main: abc1234 pre-existing stash"]);
49+
});
50+
3851
test("fails closed when repoCwd is not a git repository", async () => {
3952
const { exec } = recordingExec({
4053
"rev-parse": { error: new Error("not a git repository") },
@@ -56,15 +69,17 @@ describe("createSubAgentWorktree", () => {
5669
});
5770

5871
describe("cleanupSubAgentWorktree", () => {
59-
test("removes a clean worktree", async () => {
72+
test("removes a clean worktree with no new stash entries", async () => {
6073
const { exec, calls } = recordingExec({
6174
status: { stdout: "" },
75+
stash: { stdout: "" },
6276
worktree: { stdout: "" },
6377
});
64-
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
78+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
6579
expect(result).toEqual({ status: "removed", path: "/repo/.worktrees/abc" });
6680
expect(calls).toEqual([
6781
["status", "--porcelain", "--ignored"],
82+
["stash", "list"],
6883
["worktree", "remove", "/repo/.worktrees/abc"],
6984
]);
7085
});
@@ -73,7 +88,7 @@ describe("cleanupSubAgentWorktree", () => {
7388
const { exec, calls } = recordingExec({
7489
status: { stdout: "!! dist/output.txt\n" },
7590
});
76-
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
91+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
7792
expect(result.status).toBe("preserved");
7893
if (result.status === "preserved") {
7994
expect(result.notice).toContain("uncommitted changes");
@@ -85,7 +100,7 @@ describe("cleanupSubAgentWorktree", () => {
85100
const { exec, calls } = recordingExec({
86101
status: { stdout: " M src/index.ts\n" },
87102
});
88-
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
103+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
89104
expect(result.status).toBe("preserved");
90105
expect(result).toMatchObject({ path: "/repo/.worktrees/abc" });
91106
if (result.status === "preserved") {
@@ -99,19 +114,45 @@ describe("cleanupSubAgentWorktree", () => {
99114
const { exec } = recordingExec({
100115
status: { error: new Error("no such directory") },
101116
});
102-
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
117+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
103118
expect(result.status).toBe("preserved");
104119
});
105120

106121
test("preserves the worktree when removal fails", async () => {
107-
const { exec } = recordingExec({
122+
const { exec, calls } = recordingExec({
108123
status: { stdout: "" },
124+
stash: { stdout: "" },
109125
worktree: { error: new Error("worktree is locked") },
110126
});
111-
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", exec);
127+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
112128
expect(result.status).toBe("preserved");
113129
if (result.status === "preserved") {
114130
expect(result.notice).toContain("could not be removed automatically");
115131
}
116132
});
133+
134+
test("preserves a clean worktree that created a new stash entry", async () => {
135+
const { exec, calls } = recordingExec({
136+
status: { stdout: "" },
137+
stash: { stdout: "stash@{0}: WIP on (no branch): abc1234 sub-agent work\n" },
138+
});
139+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [], exec);
140+
expect(result.status).toBe("preserved");
141+
if (result.status === "preserved") {
142+
expect(result.notice).toContain("stash entry");
143+
expect(result.notice).toContain("stash@{0}");
144+
}
145+
expect(calls.some((call) => call[0] === "worktree")).toBe(false);
146+
});
147+
148+
test("does not flag a stash entry that predates this worktree", async () => {
149+
const preexisting = "stash@{0}: WIP on main: abc1234 unrelated older stash";
150+
const { exec } = recordingExec({
151+
status: { stdout: "" },
152+
stash: { stdout: `${preexisting}\n` },
153+
worktree: { stdout: "" },
154+
});
155+
const result = await cleanupSubAgentWorktree("/repo", "/repo/.worktrees/abc", [preexisting], exec);
156+
expect(result).toEqual({ status: "removed", path: "/repo/.worktrees/abc" });
157+
});
117158
});

src/subagent/worktree.ts

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,26 @@ export class WorktreeError extends Error {}
2020

2121
export type SubAgentWorktree = {
2222
path: string;
23+
// The repo's `git stash list` output at the moment this worktree was
24+
// created (see stashList). Stash refs live on the shared repo, not the
25+
// worktree, so cleanup diffs against this baseline to notice stash entries
26+
// the sub-agent created while it ran — see cleanupSubAgentWorktree.
27+
stashBaseline: string[];
2328
};
2429

30+
// The repo's stash list as an array of "stash@{N}: <message>" lines (empty
31+
// when there are none, or when the lookup itself fails — a failed lookup
32+
// must never make cleanup MORE willing to remove a worktree, so it degrades
33+
// to "no visible change" rather than to "worktree is clean").
34+
async function stashList(repoCwd: string, exec: WorktreeExec): Promise<string[]> {
35+
try {
36+
const { stdout } = await exec(["stash", "list"], { cwd: repoCwd });
37+
return stdout.split("\n").filter((line) => line.trim().length > 0);
38+
} catch {
39+
return [];
40+
}
41+
}
42+
2543
// Creates a fresh git worktree at `path`, detached at the current HEAD of
2644
// `repoCwd`. Fails closed: `repoCwd` must be inside a git working tree and
2745
// `git worktree add` must succeed, or this throws WorktreeError with a
@@ -33,31 +51,41 @@ export async function createSubAgentWorktree(
3351
): Promise<SubAgentWorktree> {
3452
try {
3553
await exec(["rev-parse", "--show-toplevel"], { cwd: repoCwd });
36-
} catch {
54+
} catch (err) {
3755
throw new WorktreeError(
3856
`Cannot create an isolated sub-agent worktree: "${repoCwd}" is not inside a git repository.`,
57+
{ cause: err },
3958
);
4059
}
4160
try {
4261
await exec(["worktree", "add", "--detach", path, "HEAD"], { cwd: repoCwd });
4362
} catch (err) {
4463
throw new WorktreeError(
4564
`Failed to create sub-agent worktree at "${path}": ${err instanceof Error ? err.message : String(err)}`,
65+
{ cause: err },
4666
);
4767
}
48-
return { path };
68+
const stashBaseline = await stashList(repoCwd, exec);
69+
return { path, stashBaseline };
4970
}
5071

5172
export type WorktreeCleanupResult =
5273
| { status: "removed"; path: string }
5374
| { status: "preserved"; path: string; notice: string };
5475

55-
// Removes the worktree if it has no uncommitted changes; otherwise leaves it
56-
// in place (the sub-agent's work is not ours to discard) and returns a
57-
// notice the caller should surface to the operator.
76+
// Removes the worktree if it has no uncommitted changes and no new stash
77+
// entries; otherwise leaves it in place (the sub-agent's work is not ours to
78+
// discard) and returns a notice the caller should surface to the operator.
79+
// `git status` never reports a `git stash` the sub-agent ran mid-task — the
80+
// stash itself survives in the repo's shared refs/stash either way, but
81+
// without this check it goes silently orphaned with no indication of which
82+
// worktree it came from. `stashBaseline` (from createSubAgentWorktree) is
83+
// diffed against the current stash list so only entries created since this
84+
// worktree was checked out are attributed to it.
5885
export async function cleanupSubAgentWorktree(
5986
repoCwd: string,
6087
path: string,
88+
stashBaseline: readonly string[] = [],
6189
exec: WorktreeExec = defaultExec,
6290
): Promise<WorktreeCleanupResult> {
6391
let dirty: boolean;
@@ -79,6 +107,18 @@ export async function cleanupSubAgentWorktree(
79107
notice: `Sub-agent worktree at ${path} has uncommitted changes and was left in place.`,
80108
};
81109
}
110+
const currentStashes = await stashList(repoCwd, exec);
111+
const baselineSet = new Set(stashBaseline);
112+
const newStashes = currentStashes.filter((entry) => !baselineSet.has(entry));
113+
if (newStashes.length > 0) {
114+
return {
115+
status: "preserved",
116+
path,
117+
notice: `Sub-agent worktree at ${path} was left in place: it created ${
118+
newStashes.length === 1 ? "a stash entry" : `${newStashes.length} stash entries`
119+
} that would otherwise go unrecovered (${newStashes.join("; ")}).`,
120+
};
121+
}
82122
try {
83123
await exec(["worktree", "remove", path], { cwd: repoCwd });
84124
} catch (err) {

src/tui/components/operator-modal.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ function renderMarkdownLines(lines: readonly StyledSegment[][]): ReactNode {
4444
);
4545
}
4646

47-
4847
// Two-column layout when all options are short enough to fit side by side.
4948
// Each column gets half the inner width minus a small gap for the number prefix.
5049
function renderOptionsGrid(options: string[], selected: number, innerWidth: number): ReactNode {

0 commit comments

Comments
 (0)