From 8f80a09c27b8e61975f7682eb5c81cc5b63bb8fd Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 25 Sep 2026 09:48:52 +0200 Subject: [PATCH] fix(agent-manager): evict stale worktree pool slots and cold-create A pooled slot deleted on disk outside the pool made every create fail, because simple-git throws when constructed on a missing directory and the exception bypassed the pool discard path. Validate the slot before use, evict stale entries, and try remaining slots before falling back to a cold worktree add. Log worktree creation failures to the Output channel. --- .changeset/stale-worktree-pool-slot.md | 5 ++ .../src/agent-manager/worktree-create.ts | 1 + .../src/agent-manager/worktree-pool.ts | 28 +++++-- .../tests/unit/worktree-create.test.ts | 4 +- .../tests/unit/worktree-pool.test.ts | 81 ++++++++++++++++++- 5 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 .changeset/stale-worktree-pool-slot.md diff --git a/.changeset/stale-worktree-pool-slot.md b/.changeset/stale-worktree-pool-slot.md new file mode 100644 index 000000000000..3d1d66a69440 --- /dev/null +++ b/.changeset/stale-worktree-pool-slot.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Fix Agent Manager worktree creation failing when a pooled worktree slot is deleted on disk. The pool now evicts the stale slot and falls back to a normal worktree, and worktree creation failures are written to the Output channel. diff --git a/packages/kilo-vscode/src/agent-manager/worktree-create.ts b/packages/kilo-vscode/src/agent-manager/worktree-create.ts index bcb5cfe61b2d..fb13f57227e9 100644 --- a/packages/kilo-vscode/src/agent-manager/worktree-create.ts +++ b/packages/kilo-vscode/src/agent-manager/worktree-create.ts @@ -87,6 +87,7 @@ export async function createWorktreeOnDisk( probeFailed: manager.gitProbeFailed, err: error, }) + ctx.log(`Worktree creation failed: ${msg}`) report(opts, { message: msg, code: errorCode }) ctx.postToWebview({ type: "agentManager.worktreeSetup", diff --git a/packages/kilo-vscode/src/agent-manager/worktree-pool.ts b/packages/kilo-vscode/src/agent-manager/worktree-pool.ts index fa2cba974ee9..2ac76c82d686 100644 --- a/packages/kilo-vscode/src/agent-manager/worktree-pool.ts +++ b/packages/kilo-vscode/src/agent-manager/worktree-pool.ts @@ -101,11 +101,20 @@ export class WorktreePool { */ async claim(branch: string, oid: string, auto = false): Promise<{ path: string; branch: string } | undefined> { if (!this.has()) return undefined - const exact = this.slots.find((slot) => slot.baseOid === oid) - if (exact) return this.take(exact, branch, oid, true, auto) - const delta = await this.findDelta(oid) - if (!delta) return undefined - return this.take(delta, branch, oid, false, auto) + // take() discards a slot it cannot use (for example one deleted on disk), + // so keep trying the remaining slots, exact base first, then a small delta, + // before falling back to a cold worktree add. + for (const slot of this.slots.filter((known) => known.baseOid === oid)) { + const claimed = await this.take(slot, branch, oid, true, auto) + if (claimed) return claimed + } + for (let left = this.slots.length; left > 0; left--) { + const delta = await this.findDelta(oid) + if (!delta) return undefined + const claimed = await this.take(delta, branch, oid, false, auto) + if (claimed) return claimed + } + return undefined } /** True when at least one slot is available. Pure in-memory check. */ @@ -194,6 +203,15 @@ export class WorktreePool { exact: boolean, auto: boolean, ): Promise<{ path: string; branch: string } | undefined> { + // A slot can be deleted on disk outside the pool, for example by a + // worktree-hygiene script. simple-git throws when constructed on a missing + // directory, so validate the slot before touching it and evict the stale + // entry instead of failing the whole creation. + if (!fs.existsSync(path.join(slot.path, ".git"))) { + this.deps.log(`worktree pool: slot missing on disk, evicting ${slot.path}`) + await this.discard(slot) + return undefined + } const git = this.deps.client(slot.path) // For generated names, reuse the slot directory name as the branch so the // worktree folder and branch keep matching, as they do without the pool. diff --git a/packages/kilo-vscode/tests/unit/worktree-create.test.ts b/packages/kilo-vscode/tests/unit/worktree-create.test.ts index 4e7086fc1de3..92ec8321d20d 100644 --- a/packages/kilo-vscode/tests/unit/worktree-create.test.ts +++ b/packages/kilo-vscode/tests/unit/worktree-create.test.ts @@ -25,6 +25,7 @@ describe("createWorktreeOnDisk", () => { const messages: AgentManagerOutMessage[] = [] const failures: WorktreeCreationFailure[] = [] + const logs: string[] = [] const state = { getDefaultBaseBranch: () => undefined } const ctx = { getWorktreeManager: () => new WorktreeManager(root, () => {}), @@ -32,7 +33,7 @@ describe("createWorktreeOnDisk", () => { postToWebview: (message: AgentManagerOutMessage) => messages.push(message), capture: () => {}, pushState: () => {}, - log: () => {}, + log: (message: string) => logs.push(message), } as unknown as CreateWorktreeOnDiskContext const value = await createWorktreeOnDisk(ctx, { @@ -53,5 +54,6 @@ describe("createWorktreeOnDisk", () => { status: "error", errorCode: "no_commits", }) + expect(logs.some((line) => line.includes("This repository has no commits yet."))).toBe(true) }) }) diff --git a/packages/kilo-vscode/tests/unit/worktree-pool.test.ts b/packages/kilo-vscode/tests/unit/worktree-pool.test.ts index 4ce50a1b838d..2705f29f62f8 100644 --- a/packages/kilo-vscode/tests/unit/worktree-pool.test.ts +++ b/packages/kilo-vscode/tests/unit/worktree-pool.test.ts @@ -36,8 +36,14 @@ async function createTempRepo(): Promise { return dir } -function createManager(root: string, poolSize = 1, rewarmDelay = 0): WorktreeManager { - const manager = new WorktreeManager(root, () => undefined, undefined, undefined, poolSize) +function createManager(root: string, poolSize = 1, rewarmDelay = 0, logs?: string[]): WorktreeManager { + const manager = new WorktreeManager( + root, + logs ? (msg) => logs.push(msg) : () => undefined, + undefined, + undefined, + poolSize, + ) manager.rewarmDelay = rewarmDelay return manager } @@ -75,6 +81,19 @@ async function waitForPooledSlot(root: string, timeout = 10000): Promise throw new Error("Timed out waiting for a pooled slot") } +async function waitForPooledSlots(root: string, count: number, timeout = 10000): Promise { + const deadline = Date.now() + timeout + while (Date.now() < deadline) { + const slots: string[] = [] + for (const slot of await pooledSlots(root)) { + if ((await slotMeta(slot))?.pooled === true) slots.push(slot) + } + if (slots.length >= count) return slots + await new Promise((resolve) => setTimeout(resolve, 25)) + } + throw new Error(`Timed out waiting for ${count} pooled slots`) +} + describe("WorktreeManager pool warm-up", () => { it("creates a detached slot that discoverWorktrees skips", async () => { const root = await createTempRepo() @@ -194,6 +213,64 @@ describe("WorktreeManager pool claim", () => { }) }) +describe("WorktreeManager pool stale slot", () => { + it("evicts a slot whose directory was deleted and cold-creates instead", async () => { + const root = await createTempRepo() + const logs: string[] = [] + const manager = createManager(root, 1, 0, logs) + + manager.warmPool() + const slot = await waitForPooledSlot(root) + + await fs.rm(slot, { recursive: true, force: true }) + expect(existsSync(slot)).toBe(false) + + const result = await manager.createWorktree({}) + expect(existsSync(result.path)).toBe(true) + expect(result.path).not.toBe(slot) + expect((await simpleGit(result.path).raw(["status", "--porcelain"])).trim()).toBe("") + expect(logs.some((line) => line.includes("slot missing on disk, evicting"))).toBe(true) + + // The stale slot is evicted and the next create succeeds as well. + const second = await manager.createWorktree({}) + expect(existsSync(second.path)).toBe(true) + }) + + it("reuses a healthy slot when another pooled slot was deleted", async () => { + const root = await createTempRepo() + const logs: string[] = [] + const manager = createManager(root, 2, 0, logs) + + manager.warmPool() + const original = await waitForPooledSlots(root, 2) + + // The first claim consumes the pool's first slot. Identifying it pins the + // creation order, so the slot that stays in the pool is deterministically + // the one claim() tries first after the replacement warm. + const first = await manager.createWorktree({}) + const firstReal = await fs.realpath(first.path) + const remaining: string[] = [] + for (const slot of original) { + if ((await fs.realpath(slot)) !== firstReal) remaining.push(slot) + } + expect(remaining).toHaveLength(1) + const stale = remaining[0]! + + const refilled = await waitForPooledSlots(root, 2) + const healthy = refilled.find((slot) => slot !== stale) + expect(healthy).toBeDefined() + + await fs.rm(stale, { recursive: true, force: true }) + expect(existsSync(stale)).toBe(false) + + const result = await manager.createWorktree({}) + + expect(await fs.realpath(result.path)).toBe(await fs.realpath(healthy!)) + expect(existsSync(stale)).toBe(false) + expect(logs.some((line) => line.includes("slot missing on disk, evicting"))).toBe(true) + }) +}) + describe("WorktreeManager pool reconcile", () => { it("trusts the slot HEAD over stale metadata when adopting", async () => { const root = await createTempRepo()