Merge pull request #14127 from Kilo-Org/sugary-satellite

fix(agent-manager): keep only the target tab on Close Others
This commit is contained in:
Marius
2026-09-15 09:33:49 +02:00
committed by GitHub
7 changed files with 299 additions and 24 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---
Fix "Close Others" in the Agent Manager leaving closed session tabs open in the local tab bar.
@@ -0,0 +1,179 @@
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
let reviewActive = false
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
reviewActive = false
},
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")
reviewActive = false
remove(REVIEW)
},
selectReviewTab: () => {
calls.push("selectReview")
termActive = undefined
session = undefined
pending = undefined
reviewActive = true
},
selectSessionTab: (id, isPendingTab) => {
calls.push(`select:${id}:${isPendingTab}`)
reviewActive = false
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: () => (reviewActive && open.includes(REVIEW) ? REVIEW : (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("reveals a review target and never routes it through the session path", () => {
const s = scene(["ses:a", REVIEW, TERM_1], { active: "ses:a" })
closeOthers(REVIEW, s.deps)
expect(s.calls).toEqual(["deactivate", "selectReview", "sessionClose:ses:a", "closeTerminal:terminal:1"])
expect(s.open).toEqual([REVIEW])
expect(s.visible()).toBe(REVIEW)
})
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")
})
})
@@ -8,6 +8,7 @@ import {
openSessionTab,
insertSessionTabAfter,
pendingTabForCreated,
pruneClosed,
reconcileTabs,
reconcileTrackedTabs,
replacePendingTab,
@@ -245,6 +246,32 @@ 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"])
})
it("drops a closed id even when it is present in current and order", () => {
expect(
restoreTrackedTabs(inventory(["s1", "s2"]), ["s1", "s2"], ["s1", "s2"], trackedPending, reorder, new Set(["s2"])),
).toEqual(["s1"])
})
it("prunes suppressed ids the host no longer tracks", () => {
const closed = new Set(["s1", "s2"])
pruneClosed(closed, [{ id: "s1" }])
expect([...closed]).toEqual(["s1"])
})
})
describe("tracked tab reconcile", () => {
@@ -143,6 +143,7 @@ import {
addPendingTab as addLocalPendingTab,
nextTabAfterClose,
openSessionTab,
pruneClosed,
reconcileTrackedTabs,
replacePendingTab,
restoreTrackedTabs,
@@ -311,6 +312,19 @@ 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.
// Keyed per project so switching projects cannot prune another project's
// suppression entry. Kept until the host stops tracking the id so a stale push
// cannot resurrect the tab; see `restoreTrackedTabs` in the state handler.
const closedLocals = new Map<string, Set<string>>()
const closedSet = () => {
const key = currentProjectId() ?? "single"
const existing = closedLocals.get(key)
if (existing) return existing
const set = new Set<string>()
closedLocals.set(key, set)
return set
}
const [sidebarWidth, setSidebarWidth] = createSignal(persisted?.sidebarWidth ?? DEFAULT_SIDEBAR_WIDTH)
const sidebar = createSidebarCollapse(vscode, { initial: persisted?.sidebarCollapsed })
const sidebarCollapsed = sidebar.collapsed
@@ -723,6 +737,7 @@ const AgentManagerContent: Component = () => {
return id
}
const placeLocal = (id: string, pending: string | undefined, active: string | undefined) => {
closedSet().delete(id)
const existing = localSessionIDs().includes(id)
const next = pending
? replacePendingTab({ ids: localSessionIDs(), active }, pending, id)
@@ -1188,12 +1203,16 @@ 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())
const closed = closedSet()
pruneClosed(closed, state.sessions)
const restored = restoreTrackedTabs(
trackedSessionInventory(state.sessions, session.sessions()),
tracked,
localSessionIDs(),
state.tabOrder?.[LOCAL],
isPending,
applyTabOrder,
closed,
)
if (restored) setLocalSessionIDs(restored)
if (switched === "switched" && needsLocalDraft(localSessionIDs(), terms.forSelection(nsKey(LOCAL)))) addPendingTab()
@@ -1999,6 +2018,7 @@ const AgentManagerContent: Component = () => {
}
forgetSessionFocus(sessionId)
if (pending || localSet().has(sessionId)) {
if (!pending) closedSet().add(sessionId)
setLocalSessionIDs((prev) => prev.filter((id) => id !== sessionId))
}
if (pending) {
@@ -0,0 +1,45 @@
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
selectReviewTab: () => 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)
const review = target === deps.REVIEW_TAB_ID
if (terminal) deps.activateTerminal(target)
if (!terminal) deps.deactivateTerminal()
if (review) deps.selectReviewTab()
if (!terminal && !review) 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)
}
}
@@ -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.
@@ -165,11 +165,21 @@ export function restoreTrackedTabs(
order: string[] | undefined,
check: PendingTabCheck,
apply: ApplyLocalTabOrder,
closed: ReadonlySet<string> = 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,
// including through `current`, `base`, `merged`, or the `order` path.
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))
ids.filter(
(id) =>
!closed.has(id) &&
!inventory.external?.has(id) &&
!inventory.unresolved?.has(id) &&
!inventory.rejected?.has(id),
)
const real = current.filter((id) => !check(id) && !closed.has(id))
if (locals.length > 0 && real.length === 0) {
if (!order) return locals
@@ -194,6 +204,14 @@ export function restoreTrackedTabs(
return changed ? merged : undefined
}
/** Drop suppressed ids the host no longer tracks, so a later restore can re-add them. */
export function pruneClosed(closed: Set<string>, sessions: readonly { id: string }[]): void {
const live = new Set(sessions.map((entry) => entry.id))
for (const id of closed) {
if (!live.has(id)) closed.delete(id)
}
}
export function reconcileTrackedTabs(
current: string[],
loaded: readonly string[],