diff --git a/.changeset/close-others-local-tabs.md b/.changeset/close-others-local-tabs.md new file mode 100644 index 00000000000..d49e35015fd --- /dev/null +++ b/.changeset/close-others-local-tabs.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Fix "Close Others" in the Agent Manager leaving closed session tabs open in the local tab bar. diff --git a/packages/kilo-vscode/tests/unit/close-others.test.ts b/packages/kilo-vscode/tests/unit/close-others.test.ts new file mode 100644 index 00000000000..0fe242889f4 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/close-others.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from "bun:test" +import { closeOthers, type CloseOthersDeps } from "../../webview-ui/agent-manager/close-others" + +const REVIEW = "review" +const TERM_1 = "terminal:1" +const TERM_2 = "terminal:2" +const PENDING = "sidebar-pending:1" +const isPending = (id: string) => id.startsWith("sidebar-pending:") + +/** + * Fake tab bar that mirrors AgentManagerApp: `selectSessionTab` does not clear + * the active terminal (the app relies on `deactivateTerminal` for that), and a + * script terminal survives `closeTerminal` until the host confirms closure. + */ +function scene(ids: string[], opts: { active?: string; term?: string; keep?: string[] } = {}) { + const open = [...ids] + const calls: string[] = [] + const keep = new Set(opts.keep ?? []) + let termActive = opts.term + let session = opts.active + let pending: string | undefined + const remove = (id: string) => { + const index = open.indexOf(id) + if (index >= 0) open.splice(index, 1) + } + const deps: CloseOthersDeps = { + REVIEW_TAB_ID: REVIEW, + tabIds: () => [...open], + isPending, + activateTerminal: (id) => { + calls.push(`activate:${id}`) + termActive = id + }, + deactivateTerminal: () => { + calls.push("deactivate") + termActive = undefined + }, + closeTerminal: (id) => { + calls.push(`closeTerminal:${id}`) + if (keep.has(id)) return + remove(id) + if (termActive === id) termActive = undefined + }, + closeReview: () => { + calls.push("closeReview") + remove(REVIEW) + }, + selectSessionTab: (id, isPendingTab) => { + calls.push(`select:${id}:${isPendingTab}`) + if (isPendingTab) { + pending = id + session = undefined + return + } + session = id + pending = undefined + }, + sessionClose: (id) => { + calls.push(`sessionClose:${id}`) + const wasActive = session === id + const index = open.indexOf(id) + remove(id) + if (!wasActive) return + // AgentManagerApp picks a neighbor when the active tab closes. + const next = open[Math.min(index, open.length - 1)] + if (!next) { + session = undefined + pending = undefined + return + } + calls.push(`select:${next}:${isPending(next)}`) + if (isPending(next)) { + pending = next + session = undefined + return + } + session = next + pending = undefined + }, + } + return { deps, calls, open, visible: () => termActive ?? session ?? pending } +} + +describe("agent manager close others", () => { + it("reveals an active session target before closing the other tabs", () => { + const s = scene(["ses:a", "ses:b", TERM_1], { active: "ses:a" }) + + closeOthers("ses:a", s.deps) + + expect(s.calls).toEqual(["deactivate", "select:ses:a:false", "sessionClose:ses:b", "closeTerminal:terminal:1"]) + expect(s.open).toEqual(["ses:a"]) + expect(s.visible()).toBe("ses:a") + }) + + it("reveals a non-active session target and keeps only it", () => { + const s = scene(["ses:a", "ses:b", TERM_1], { active: "ses:a" }) + + closeOthers("ses:b", s.deps) + + expect(s.calls[0]).toBe("deactivate") + expect(s.open).toEqual(["ses:b"]) + expect(s.visible()).toBe("ses:b") + }) + + it("does not steal selection while closing the previously active tab", () => { + const s = scene(["ses:a", "ses:b"], { active: "ses:a" }) + + closeOthers("ses:b", s.deps) + + expect(s.calls.filter((call) => call.startsWith("select:"))).toEqual(["select:ses:b:false"]) + expect(s.visible()).toBe("ses:b") + }) + + it("activates a terminal target before closing the other tabs", () => { + const s = scene(["ses:a", TERM_1, TERM_2], { active: "ses:a" }) + + closeOthers(TERM_2, s.deps) + + expect(s.calls[0]).toBe(`activate:${TERM_2}`) + expect(s.open).toEqual([TERM_2]) + expect(s.visible()).toBe(TERM_2) + }) + + it("closes an open review tab among the others", () => { + const s = scene(["ses:a", REVIEW, TERM_1], { active: "ses:a" }) + + closeOthers("ses:a", s.deps) + + expect(s.calls).toContain("closeReview") + expect(s.open).toEqual(["ses:a"]) + expect(s.visible()).toBe("ses:a") + }) + + it("closes a pending draft among the others", () => { + const s = scene(["ses:a", PENDING, TERM_1], { active: "ses:a" }) + + closeOthers("ses:a", s.deps) + + expect(s.calls).toContain(`sessionClose:${PENDING}`) + expect(s.open).toEqual(["ses:a"]) + expect(s.visible()).toBe("ses:a") + }) + + it("keeps a session target visible when a running script terminal stays open", () => { + const s = scene(["ses:a", TERM_1], { active: "ses:a", term: TERM_1, keep: [TERM_1] }) + + closeOthers("ses:a", s.deps) + + expect(s.calls[0]).toBe("deactivate") + expect(s.open).toEqual(["ses:a", TERM_1]) + expect(s.visible()).toBe("ses:a") + }) +}) diff --git a/packages/kilo-vscode/tests/unit/local-tabs.test.ts b/packages/kilo-vscode/tests/unit/local-tabs.test.ts index 058bd6b514f..39e2c729ae1 100644 --- a/packages/kilo-vscode/tests/unit/local-tabs.test.ts +++ b/packages/kilo-vscode/tests/unit/local-tabs.test.ts @@ -245,6 +245,20 @@ describe("tracked tab restore", () => { "s2", ]) }) + + it("never restores a local tab the user just closed while the host still lists it", () => { + // Close Others closed "s2"; an early host state push still lists it. The + // optimistic close must win, or the tab reappears and later pushes keep it. + expect( + restoreTrackedTabs(inventory(["s1", "s2"]), ["s1"], ["s1", "s2"], trackedPending, reorder, new Set(["s2"])), + ).toEqual(["s1"]) + }) + + it("still restores other missing locals while a closed tab is suppressed", () => { + expect( + restoreTrackedTabs(inventory(["s1", "s2", "s3"]), ["s1"], undefined, trackedPending, identity, new Set(["s2"])), + ).toEqual(["s1", "s3"]) + }) }) describe("tracked tab reconcile", () => { diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index 4ee355de189..f880a5ed25c 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -311,6 +311,10 @@ const AgentManagerContent: Component = () => { /** Remove a session ID from the local tab (no-op if absent). */ const evictLocal = (sid: string) => setLocalSessionIDs((prev) => (prev.includes(sid) ? prev.filter((id) => id !== sid) : prev)) + // Local sessions the user closed while a host state push can still list them. + // Kept until the host stops tracking the id so a stale push cannot resurrect + // the tab; see `restoreTrackedTabs` at the `agentManager.state` handler. + const closedLocals = new Set() const [sidebarWidth, setSidebarWidth] = createSignal(persisted?.sidebarWidth ?? DEFAULT_SIDEBAR_WIDTH) const sidebar = createSidebarCollapse(vscode, { initial: persisted?.sidebarCollapsed }) const sidebarCollapsed = sidebar.collapsed @@ -723,6 +727,7 @@ const AgentManagerContent: Component = () => { return id } const placeLocal = (id: string, pending: string | undefined, active: string | undefined) => { + closedLocals.delete(id) const existing = localSessionIDs().includes(id) const next = pending ? replacePendingTab({ ids: localSessionIDs(), active }, pending, id) @@ -1188,12 +1193,17 @@ const AgentManagerContent: Component = () => { if (ms?.worktreeId) setSelection(ms.worktreeId) } // Restore local session IDs from persisted state (sessions with no worktreeId) + const tracked = trackedSessionInventory(state.sessions, session.sessions()) + for (const id of closedLocals) { + if (!state.sessions.some((entry) => entry.id === id)) closedLocals.delete(id) + } const restored = restoreTrackedTabs( - trackedSessionInventory(state.sessions, session.sessions()), + tracked, localSessionIDs(), state.tabOrder?.[LOCAL], isPending, applyTabOrder, + closedLocals, ) if (restored) setLocalSessionIDs(restored) if (switched === "switched" && needsLocalDraft(localSessionIDs(), terms.forSelection(nsKey(LOCAL)))) addPendingTab() @@ -1999,6 +2009,7 @@ const AgentManagerContent: Component = () => { } forgetSessionFocus(sessionId) if (pending || localSet().has(sessionId)) { + if (!pending) closedLocals.add(sessionId) setLocalSessionIDs((prev) => prev.filter((id) => id !== sessionId)) } if (pending) { diff --git a/packages/kilo-vscode/webview-ui/agent-manager/close-others.ts b/packages/kilo-vscode/webview-ui/agent-manager/close-others.ts new file mode 100644 index 00000000000..c426f944474 --- /dev/null +++ b/packages/kilo-vscode/webview-ui/agent-manager/close-others.ts @@ -0,0 +1,44 @@ +import { isTerminalTabId } from "./terminal/state" + +/** The subset of tab-bar handlers that closing every other tab needs. */ +export interface CloseOthersDeps { + REVIEW_TAB_ID: string + tabIds: () => readonly string[] + isPending: (id: string) => boolean + activateTerminal: (id: string) => void + deactivateTerminal: () => void + closeTerminal: (id: string) => void + closeReview: () => void + selectSessionTab: (id: string, pending: boolean) => void + sessionClose: (id: string) => void +} + +/** + * Close every tab except `target`. + * + * Reveal the target first. Closing the previously active tab afterwards + * cannot pull selection onto a neighbor that is itself about to close, and a + * session target cannot stay hidden behind an active terminal. The ids are + * snapshotted before the closes mutate the tab list. + */ +export function closeOthers(target: string, deps: CloseOthersDeps) { + const ids = [...deps.tabIds()] + const terminal = isTerminalTabId(target) + if (terminal) deps.activateTerminal(target) + if (!terminal) { + deps.deactivateTerminal() + deps.selectSessionTab(target, deps.isPending(target)) + } + for (const id of ids) { + if (id === target) continue + if (isTerminalTabId(id)) { + deps.closeTerminal(id) + continue + } + if (id === deps.REVIEW_TAB_ID) { + deps.closeReview() + continue + } + deps.sessionClose(id) + } +} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/tab-rendering.tsx b/packages/kilo-vscode/webview-ui/agent-manager/tab-rendering.tsx index 6b432e449e3..adb1ba88bd3 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/tab-rendering.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/tab-rendering.tsx @@ -16,6 +16,7 @@ import { TooltipKeybind } from "@kilocode/kilo-ui/tooltip" import { SortableTab, SortableReviewTab } from "./sortable-tab" import type { TerminalStateControls } from "./terminal" import { isTerminalTabId, renderTerminalTab } from "./terminal" +import { closeOthers } from "./close-others" import type { SessionInfo } from "../src/types/messages" import type { Activity } from "../src/utils/session-activity" import { parseBindingTokens } from "./keybind-tokens" @@ -204,26 +205,6 @@ function renderSessionTab(s: SessionInfo, deps: TabRenderDeps): JSX.Element { ) } -function closeOthers(target: string, deps: TabRenderDeps) { - for (const id of deps.tabIds()) { - if (id === target) continue - if (isTerminalTabId(id)) { - deps.closeTerminal(id) - continue - } - if (id === deps.REVIEW_TAB_ID) { - deps.closeReview() - continue - } - deps.sessionClose(id) - } - if (isTerminalTabId(target)) { - deps.activateTerminal(target) - return - } - deps.selectSessionTab(target, deps.isPending(target)) -} - // Terminal-specific renderers (layer + add button) live in `./terminal/render.tsx` // and are re-exported for convenience so AgentManagerApp.tsx has a single // import point for tab rendering. diff --git a/packages/kilo-vscode/webview-ui/src/utils/local-tabs.ts b/packages/kilo-vscode/webview-ui/src/utils/local-tabs.ts index 0111d768e75..56d69b7dd0a 100644 --- a/packages/kilo-vscode/webview-ui/src/utils/local-tabs.ts +++ b/packages/kilo-vscode/webview-ui/src/utils/local-tabs.ts @@ -165,8 +165,11 @@ export function restoreTrackedTabs( order: string[] | undefined, check: PendingTabCheck, apply: ApplyLocalTabOrder, + closed: ReadonlySet = new Set(), ): string[] | undefined { - const locals = [...inventory.local] + // A close is optimistic in the webview: the host can still list the session + // in an intermediate state push. Never resurrect an id the user just closed. + const locals = inventory.local.filter((id) => !closed.has(id)) const evict = (ids: string[]) => ids.filter((id) => !inventory.external?.has(id) && !inventory.unresolved?.has(id) && !inventory.rejected?.has(id)) const real = current.filter((id) => !check(id))