From 97409b0857a2011006ff34356a266c01996a3fb8 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 27 Aug 2026 10:29:02 +0200 Subject: [PATCH] fix(agent-manager): recover checkpoint cleanup and blocked deletion states --- .../src/agent-manager/AgentManagerProvider.ts | 2 +- .../src/agent-manager/provider-lifecycle.ts | 8 +++ .../tests/unit/agent-manager-arch.test.ts | 1 + .../agent-manager-provider-lifecycle.test.ts | 25 +++++++ .../tests/unit/project-session-busy.test.ts | 21 ++++++ .../agent-manager/AgentManagerApp.tsx | 4 +- .../webview-ui/agent-manager/ProjectList.tsx | 4 +- .../agent-manager/ProjectSidebarBody.tsx | 4 +- .../webview-ui/agent-manager/SidebarBody.tsx | 3 +- .../webview-ui/agent-manager/WorktreeItem.tsx | 10 ++- .../agent-manager/project/session-busy.ts | 17 +++-- .../opencode/src/kilocode/snapshot/cleanup.ts | 22 +++++- .../snapshot-repository-cleanup.test.ts | 67 ++++++++++++++++++- 13 files changed, 170 insertions(+), 18 deletions(-) diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index 195695ad56..abfc4467fc 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -325,7 +325,7 @@ export class AgentManagerProvider implements Disposable { } const info = ev.properties?.info if (ev.type === "session.created" && info) this.removedSessions.delete(info.id) - const dir = info?.directory + const dir = info && !this.removedSessions.has(info.id) ? info.directory : undefined // Session events from sync or older backends can lack time/directory; a throw // would escape into the SSE dispatch loop and starve the other listeners. if (!info?.time || !dir || (info.parentID !== undefined && info.parentID !== null)) return diff --git a/packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts b/packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts index 4b02a14e55..c5a1378522 100644 --- a/packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts +++ b/packages/kilo-vscode/src/agent-manager/provider-lifecycle.ts @@ -208,6 +208,14 @@ export async function deleteLifecycleWorktree( await client.kilocode.removeSnapshot({ directory: ctx.root, worktree: worktree.path }, { throwOnError: true }) } catch (error) { host.log(`Failed to remove worktree snapshots: ${error}`) + host.post({ + type: "error", + code: "agentManager.snapshotCleanupFailed", + projectId: ctx.id, + worktreeId, + message: + "The worktree was deleted, but its checkpoint data could not be removed. Conversation history is preserved.", + }) } const orphaned = state.removeWorktree(worktreeId) host.removePR(worktreeId) diff --git a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts index a04419e5ec..b3911f7ea0 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts @@ -643,6 +643,7 @@ describe("Agent Manager Provider — onMessage routing", () => { expect(lifecycle).toContain("this.removedSessions.add(id)") expect(lifecycle).toContain("this.busySessions.delete(id)") + expect(lifecycle).toContain("info && !this.removedSessions.has(info.id) ? info.directory : undefined") expect(status).toContain("this.removedSessions.has(sid)") }) diff --git a/packages/kilo-vscode/tests/unit/agent-manager-provider-lifecycle.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-provider-lifecycle.test.ts index c31e248d1a..58a21e0b7d 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-provider-lifecycle.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-provider-lifecycle.test.ts @@ -132,6 +132,31 @@ describe("Agent Manager worktree deletion lifecycle", () => { expect(state.getWorktrees()).toHaveLength(1) }) + it("reports checkpoint cleanup failures while preserving and retargeting sessions", async () => { + const session = state.addSession("retained", state.getWorktrees()[0]!.id) + const post = mock(host.post) + host.post = post + client.kilocode.removeSnapshot.mockRejectedValue(new Error("checkpoint cleanup failed")) + + await deleteWorktree() + + expect(post).toHaveBeenCalledWith( + expect.objectContaining({ + type: "error", + code: "agentManager.snapshotCleanupFailed", + projectId: ctx.id, + }), + ) + expect(state.getWorktrees()).toHaveLength(0) + expect(routes).toContainEqual({ + sessionID: session.id, + projectID: ctx.id, + directory: ctx.root, + generation: ctx.generation, + }) + expect(client.session.delete).not.toHaveBeenCalled() + }) + it("retargets orphaned sessions to the exact project root without deleting them", async () => { const first = state.addSession("first", state.getWorktrees()[0]!.id) const second = state.addSession("second", state.getWorktrees()[0]!.id) diff --git a/packages/kilo-vscode/tests/unit/project-session-busy.test.ts b/packages/kilo-vscode/tests/unit/project-session-busy.test.ts index 27d480fcf5..d5b3de4869 100644 --- a/packages/kilo-vscode/tests/unit/project-session-busy.test.ts +++ b/packages/kilo-vscode/tests/unit/project-session-busy.test.ts @@ -28,4 +28,25 @@ describe("createSessionBusy", () => { it("marks sessions with an active status as busy", () => { expect(busy({ working: { type: "busy" } }).agent("wt-working")).toBe(true) }) + + it.each(["permission", "question"] as const)( + "blocks deletion for a pending %s without showing a running spinner", + (kind) => { + const state = createSessionBusy({ + statuses: () => ({ session: { type: "busy" } }), + permissions: () => (kind === "permission" ? [{ sessionID: "session" }] : []), + questions: () => (kind === "question" ? [{ sessionID: "session" }] : []), + managed: () => [{ id: "session", worktreeId: "worktree" }], + local: () => [], + projects: () => ({ other: [{ id: "session", worktreeId: "worktree" }] }), + active: () => "active", + }) + + expect(state.agent("worktree")).toBe(false) + expect(state.agent("worktree", true)).toBe(true) + expect(state.project("active", "worktree", true)).toBe(true) + expect(state.project("other", "worktree")).toBe(false) + expect(state.project("other", "worktree", true)).toBe(true) + }, + ) }) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index 4092092283..b5cf7e7d05 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -1825,7 +1825,7 @@ const AgentManagerContent: Component = () => { const confirmDeleteWorktree = (worktreeId: string) => { const wt = worktrees().find((w) => w.id === worktreeId) const run = runStatuses()[worktreeId]?.state - if (!wt || busyWorktrees().has(worktreeId) || isAgentBusy(worktreeId) || (run && run !== "idle")) return + if (!wt || busyWorktrees().has(worktreeId) || isAgentBusy(worktreeId, true) || (run && run !== "idle")) return // Second press/click: execute the delete if (pendingDelete() === worktreeId) { cancelPendingDelete() @@ -2323,7 +2323,7 @@ const AgentManagerContent: Component = () => { states={projectStates()} store={(id) => registry.ensure(id)} busy={(projectId, id) => registry.ensure(projectId).busy().has(id)} - working={(projectId, id) => projectBusy(projectId, id)} + working={(projectId, id, waiting) => projectBusy(projectId, id, waiting)} localBusy={(projectId) => projectBusy(projectId, null)} stats={projectLive.stats()} local={projectLive.local()} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/ProjectList.tsx b/packages/kilo-vscode/webview-ui/agent-manager/ProjectList.tsx index ee2c86e9be..2de694b5ed 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/ProjectList.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/ProjectList.tsx @@ -41,7 +41,7 @@ interface Props { defaultBase?: (projectId: string) => string | undefined onCreate?: (projectId: string) => void busy?: (projectId: string, id: string) => boolean - working?: (projectId: string, id: string) => boolean + working?: (projectId: string, id: string, waiting?: boolean) => boolean localBusy?: (projectId: string) => boolean bindings: Record t: LanguageContextValue["t"] @@ -217,7 +217,7 @@ export const ProjectList: Component = (props) => { state={props.states[project.id]} store={props.store?.(project.id)} busy={(id) => props.busy?.(project.id, id) ?? false} - working={(id) => props.working?.(project.id, id) ?? false} + working={(id, waiting) => props.working?.(project.id, id, waiting) ?? false} localBusy={() => props.localBusy?.(project.id) ?? false} stats={props.stats[project.id]} local={props.local[project.id]} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/ProjectSidebarBody.tsx b/packages/kilo-vscode/webview-ui/agent-manager/ProjectSidebarBody.tsx index 52741e1440..1c7ad15ac8 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/ProjectSidebarBody.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/ProjectSidebarBody.tsx @@ -41,7 +41,7 @@ interface Props { state?: AgentManagerStateMessage store?: ProjectStore busy?: (id: string) => boolean - working?: (id: string) => boolean + working?: (id: string, waiting?: boolean) => boolean localBusy?: () => boolean stats?: Record local?: LocalGitStats @@ -81,6 +81,7 @@ export const ProjectSidebarBody: Component = (props) => { onCleanup(() => clearTimeout(pendingTimer)) /** Arm on the first click, execute on the second, matching the legacy sidebar. */ const confirmDelete = (worktreeId: string) => { + if (props.busy?.(worktreeId) || props.working?.(worktreeId, true)) return if (pending() === worktreeId) { clearTimeout(pendingTimer) setPending(undefined) @@ -238,6 +239,7 @@ export const ProjectSidebarBody: Component = (props) => { pendingDelete={pending() === worktree.id} busy={props.busy?.(worktree.id) ?? false} working={props.working?.(worktree.id) || runs()[worktree.id]?.state === "running"} + blocked={props.working?.(worktree.id, true)} stale={state()?.staleWorktreeIds?.includes(worktree.id) === true} stats={props.stats?.[worktree.id]} shortcut={values().shortcut} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/SidebarBody.tsx b/packages/kilo-vscode/webview-ui/agent-manager/SidebarBody.tsx index 5e5e8ded6d..bc3a3bcd04 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/SidebarBody.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/SidebarBody.tsx @@ -80,7 +80,7 @@ export interface SidebarBodyProps { worktreeSubtitle: (wt: WorktreeState) => string | undefined pendingDelete: () => string | null busy: (id: string) => boolean - isAgentBusy: (id: string) => boolean + isAgentBusy: (id: string, waiting?: boolean) => boolean isStaleWorktree: (id: string) => boolean shortcutMap: () => Map worktreeStats: () => Record @@ -315,6 +315,7 @@ export const SidebarBody: Component = (props) => { pendingDelete={props.pendingDelete() === wt.id} busy={props.busy(wt.id)} working={props.isAgentBusy(wt.id)} + blocked={props.isAgentBusy(wt.id, true)} stale={props.isStaleWorktree(wt.id)} shortcut={props.shortcutMap().get(wt.id)} stats={props.worktreeStats()[wt.id]} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/WorktreeItem.tsx b/packages/kilo-vscode/webview-ui/agent-manager/WorktreeItem.tsx index 163a62bd3b..05fdfdc4df 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/WorktreeItem.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/WorktreeItem.tsx @@ -33,6 +33,7 @@ interface WorktreeItemProps { busy: boolean /** Whether an agent session on this worktree is actively working (shows spinner instead of branch icon). */ working: boolean + blocked?: boolean stale: boolean /** 1-indexed shortcut number shown as ⌘2, ⌘3, etc. Pass 0, >9, or undefined to hide. */ shortcut?: number @@ -200,7 +201,10 @@ export const WorktreeItem: Component = (props) => { onClick={() => props.onClick()} >
-
@@ -303,7 +307,7 @@ export const WorktreeItem: Component = (props) => { {props.shortcut} - +
setOverClose(true)} @@ -520,7 +524,7 @@ export const WorktreeItem: Component = (props) => { {t("agentManager.worktree.rename")} - + props.onDelete(new MouseEvent("click"))}> {t("agentManager.worktree.delete")} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/project/session-busy.ts b/packages/kilo-vscode/webview-ui/agent-manager/project/session-busy.ts index d417c0209a..b310d23dde 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/project/session-busy.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/project/session-busy.ts @@ -20,26 +20,31 @@ export function createSessionBusy(opts: { projects: () => Record active: () => string | undefined }) { - const any = (ids: string[]) => { + const any = (ids: string[], waiting = false) => { if (ids.length === 0) return false const statuses = opts.statuses() const blocked = new Set([...opts.permissions(), ...opts.questions()].map((item) => item.sessionID)) return ids.some((id) => { const status = statuses[id] - return !!status && status.type !== "idle" && !blocked.has(id) + if (waiting && blocked.has(id)) return true + return !!status && status.type !== "idle" && (waiting || !blocked.has(id)) }) } - const agent = (id: string) => + const agent = (id: string, waiting = false) => any( opts .managed() .filter((item) => item.worktreeId === id) .map((item) => item.id), + waiting, ) const local = () => any(opts.local()) - const project = (id: string, worktreeId: string | null) => { - if (id === opts.active()) return worktreeId === null ? local() : agent(worktreeId) - return any((opts.projects()[id] ?? []).filter((item) => item.worktreeId === worktreeId).map((item) => item.id)) + const project = (id: string, worktreeId: string | null, waiting = false) => { + if (id === opts.active()) return worktreeId === null ? any(opts.local(), waiting) : agent(worktreeId, waiting) + return any( + (opts.projects()[id] ?? []).filter((item) => item.worktreeId === worktreeId).map((item) => item.id), + waiting, + ) } return { any, agent, local, project, session: (id: string) => any([id]) } } diff --git a/packages/opencode/src/kilocode/snapshot/cleanup.ts b/packages/opencode/src/kilocode/snapshot/cleanup.ts index 13750cc60f..9571e53604 100644 --- a/packages/opencode/src/kilocode/snapshot/cleanup.ts +++ b/packages/opencode/src/kilocode/snapshot/cleanup.ts @@ -166,7 +166,7 @@ export namespace KiloSnapshotCleanup { if (!path.isAbsolute(input.worktree) || worktree === managed || !FSUtil.contains(managed, worktree)) return yield* Effect.fail(new Error("worktree must be an absolute path inside the managed worktrees directory")) const child = path.relative(managed, worktree).split(path.sep).filter(Boolean) - if (child.length !== 1 || !component(child[0]!)) + if (child.length !== 1 || !component(child[0])) return yield* Effect.fail(new Error("worktree must be a single safe path component")) const gitdir = path.join(root, input.project, Hash.fast(worktree)) if (!inside(root, gitdir) || normalized(gitdir) === normalized(root)) @@ -181,6 +181,26 @@ export namespace KiloSnapshotCleanup { const checked = yield* validate(input, paths) if (!(yield* absent(input, worktree))) return yield* Effect.fail(new Error("worktree must be absent before its snapshot repository is removed")) + if (checked.project.exists) { + const prefix = `.${path.basename(gitdir)}.cleanup-` + const entries = yield* input.fs.readDirectoryEntries(checked.project.canonical) + for (const entry of entries) { + if (!entry.name.startsWith(prefix)) continue + if ( + !/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(entry.name.slice(prefix.length)) + ) + continue + const target = path.join(checked.project.canonical, entry.name) + const retained = yield* inspect(input.fs, target) + yield* dir(retained, "snapshot cleanup quarantine") + if (!retained.exists) continue + if (normalized(retained.canonical) !== normalized(target) || (yield* pending(input.fs, target))) + return yield* Effect.fail(new Error("snapshot cleanup quarantine is unsafe or still pending")) + if (!(yield* absent(input, worktree))) + return yield* Effect.fail(new Error("worktree must be absent before its snapshot repository is removed")) + yield* Effect.uninterruptible(input.fs.remove(target, { recursive: true, force: true })) + } + } if (!checked.gitdir.exists) return true const final = yield* validate(input, paths) diff --git a/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts b/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts index ef520539f0..03b2845733 100644 --- a/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts +++ b/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts @@ -222,6 +222,71 @@ it.live("isolates snapshot repositories by project", () => }), ) +it.live("finishes an interrupted quarantine without deleting unrelated directories", () => + Effect.gen(function* () { + const base = yield* tmpdirScoped() + const input = item(base, "project", "retry") + const current = yield* repo(input) + yield* drop(input.worktree) + const quarantine = path.join( + path.dirname(current.dir), + `.${path.basename(current.dir)}.cleanup-${crypto.randomUUID()}`, + ) + const unrelated = path.join(path.dirname(current.dir), ".other.cleanup-00000000-0000-0000-0000-000000000000") + const fs = yield* FSUtil.Service + yield* fs.rename(current.dir, quarantine) + yield* write(path.join(unrelated, "keep"), "keep") + + expect(yield* remove(input)).toBe(true) + expect(yield* exist(quarantine)).toBe(false) + expect(yield* exist(unrelated)).toBe(true) + expect(yield* remove(input)).toBe(true) + }), +) + +it.live("preserves a pending quarantine and completes cleanup after materialization", () => + Effect.gen(function* () { + const base = yield* tmpdirScoped() + const input = item(base, "project", "retry-pending") + const current = yield* repo(input) + yield* drop(input.worktree) + const quarantine = path.join( + path.dirname(current.dir), + `.${path.basename(current.dir)}.cleanup-${crypto.randomUUID()}`, + ) + const fs = yield* FSUtil.Service + yield* fs.rename(current.dir, quarantine) + const marker = path.join(quarantine, "seed.index") + yield* write(marker, "pending") + + expect(Exit.isFailure(yield* remove(input).pipe(Effect.exit))).toBe(true) + expect(yield* exist(quarantine)).toBe(true) + yield* drop(marker) + expect(yield* remove(input)).toBe(true) + expect(yield* exist(quarantine)).toBe(false) + }), +) + +it.live("rejects a symlinked cleanup quarantine during a retry", () => + Effect.gen(function* () { + const base = yield* tmpdirScoped() + const input = item(base, "project", "retry-symlink") + const current = yield* repo(input) + yield* drop(input.worktree) + yield* drop(current.dir) + const outside = path.join(base, "outside-quarantine") + yield* write(path.join(outside, "keep"), "keep") + const quarantine = path.join( + path.dirname(current.dir), + `.${path.basename(current.dir)}.cleanup-${crypto.randomUUID()}`, + ) + yield* link(outside, quarantine) + + expect(Exit.isFailure(yield* remove(input).pipe(Effect.exit))).toBe(true) + expect(yield* exist(path.join(outside, "keep"))).toBe(true) + }), +) + it.live("removes an absent snapshot repository idempotently", () => Effect.gen(function* () { const base = yield* tmpdirScoped() @@ -353,7 +418,7 @@ it.live("rejects a dangling managed worktree symlink", () => Effect.gen(function* () { const base = yield* tmpdirScoped() const input = item(base, "dangling-worktree", "dangling-worktree") - const current = yield* repo(input) + yield* repo(input) yield* drop(input.worktree) yield* write(path.join(base, "outside"), "keep") yield* link(path.join(base, "missing"), input.worktree)