mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-28 19:11:03 +08:00
fix(agent-manager): recover checkpoint cleanup and blocked deletion states
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)")
|
||||
})
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
@@ -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()}
|
||||
|
||||
@@ -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<string, string>
|
||||
t: LanguageContextValue["t"]
|
||||
@@ -217,7 +217,7 @@ export const ProjectList: Component<Props> = (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]}
|
||||
|
||||
@@ -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<string, WorktreeGitStats>
|
||||
local?: LocalGitStats
|
||||
@@ -81,6 +81,7 @@ export const ProjectSidebarBody: Component<Props> = (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> = (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}
|
||||
|
||||
@@ -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<string, number>
|
||||
worktreeStats: () => Record<string, WorktreeGitStats>
|
||||
@@ -315,6 +315,7 @@ export const SidebarBody: Component<SidebarBodyProps> = (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]}
|
||||
|
||||
@@ -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<WorktreeItemProps> = (props) => {
|
||||
onClick={() => props.onClick()}
|
||||
>
|
||||
<div class="am-wt-icon">
|
||||
<Show when={!props.busy && !props.working} fallback={<Spinner class="am-worktree-spinner" />}>
|
||||
<Show
|
||||
when={!props.busy && !props.working && !props.blocked}
|
||||
fallback={<Spinner class="am-worktree-spinner" />}
|
||||
>
|
||||
<Icon name="branch" size="small" />
|
||||
</Show>
|
||||
</div>
|
||||
@@ -303,7 +307,7 @@ export const WorktreeItem: Component<WorktreeItemProps> = (props) => {
|
||||
{props.shortcut}
|
||||
</span>
|
||||
</Show>
|
||||
<Show when={!props.busy && !props.working && !props.pendingDelete}>
|
||||
<Show when={!props.busy && !props.working && !props.blocked && !props.pendingDelete}>
|
||||
<div
|
||||
class="am-worktree-close"
|
||||
onMouseEnter={() => setOverClose(true)}
|
||||
@@ -520,7 +524,7 @@ export const WorktreeItem: Component<WorktreeItemProps> = (props) => {
|
||||
<Icon name="edit" size="small" />
|
||||
<ContextMenu.ItemLabel>{t("agentManager.worktree.rename")}</ContextMenu.ItemLabel>
|
||||
</ContextMenu.Item>
|
||||
<Show when={!props.busy && !props.working}>
|
||||
<Show when={!props.busy && !props.working && !props.blocked}>
|
||||
<ContextMenu.Item onSelect={() => props.onDelete(new MouseEvent("click"))}>
|
||||
<Icon name="trash" size="small" />
|
||||
<ContextMenu.ItemLabel>{t("agentManager.worktree.delete")}</ContextMenu.ItemLabel>
|
||||
|
||||
@@ -20,26 +20,31 @@ export function createSessionBusy(opts: {
|
||||
projects: () => Record<string, Item[]>
|
||||
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]) }
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user