mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-19 01:51:21 +08:00
fix(agent-manager): harden Close Others tab suppression
This commit is contained in:
@@ -19,6 +19,7 @@ function scene(ids: string[], opts: { active?: string; term?: string; keep?: str
|
||||
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)
|
||||
@@ -30,6 +31,7 @@ function scene(ids: string[], opts: { active?: string; term?: string; keep?: str
|
||||
activateTerminal: (id) => {
|
||||
calls.push(`activate:${id}`)
|
||||
termActive = id
|
||||
reviewActive = false
|
||||
},
|
||||
deactivateTerminal: () => {
|
||||
calls.push("deactivate")
|
||||
@@ -43,10 +45,19 @@ function scene(ids: string[], opts: { active?: string; term?: string; keep?: str
|
||||
},
|
||||
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
|
||||
@@ -78,7 +89,12 @@ function scene(ids: string[], opts: { active?: string; term?: string; keep?: str
|
||||
pending = undefined
|
||||
},
|
||||
}
|
||||
return { deps, calls, open, visible: () => termActive ?? session ?? pending }
|
||||
return {
|
||||
deps,
|
||||
calls,
|
||||
open,
|
||||
visible: () => (reviewActive && open.includes(REVIEW) ? REVIEW : (termActive ?? session ?? pending)),
|
||||
}
|
||||
}
|
||||
|
||||
describe("agent manager close others", () => {
|
||||
@@ -131,6 +147,16 @@ describe("agent manager close others", () => {
|
||||
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" })
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
openSessionTab,
|
||||
insertSessionTabAfter,
|
||||
pendingTabForCreated,
|
||||
pruneClosed,
|
||||
reconcileTabs,
|
||||
reconcileTrackedTabs,
|
||||
replacePendingTab,
|
||||
@@ -259,6 +260,25 @@ describe("tracked tab restore", () => {
|
||||
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,
|
||||
@@ -312,9 +313,18 @@ const AgentManagerContent: Component = () => {
|
||||
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<string>()
|
||||
// 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
|
||||
@@ -727,7 +737,7 @@ const AgentManagerContent: Component = () => {
|
||||
return id
|
||||
}
|
||||
const placeLocal = (id: string, pending: string | undefined, active: string | undefined) => {
|
||||
closedLocals.delete(id)
|
||||
closedSet().delete(id)
|
||||
const existing = localSessionIDs().includes(id)
|
||||
const next = pending
|
||||
? replacePendingTab({ ids: localSessionIDs(), active }, pending, id)
|
||||
@@ -1194,16 +1204,15 @@ const AgentManagerContent: Component = () => {
|
||||
}
|
||||
// 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 closed = closedSet()
|
||||
pruneClosed(closed, state.sessions)
|
||||
const restored = restoreTrackedTabs(
|
||||
tracked,
|
||||
localSessionIDs(),
|
||||
state.tabOrder?.[LOCAL],
|
||||
isPending,
|
||||
applyTabOrder,
|
||||
closedLocals,
|
||||
closed,
|
||||
)
|
||||
if (restored) setLocalSessionIDs(restored)
|
||||
if (switched === "switched" && needsLocalDraft(localSessionIDs(), terms.forSelection(nsKey(LOCAL)))) addPendingTab()
|
||||
@@ -2009,7 +2018,7 @@ const AgentManagerContent: Component = () => {
|
||||
}
|
||||
forgetSessionFocus(sessionId)
|
||||
if (pending || localSet().has(sessionId)) {
|
||||
if (!pending) closedLocals.add(sessionId)
|
||||
if (!pending) closedSet().add(sessionId)
|
||||
setLocalSessionIDs((prev) => prev.filter((id) => id !== sessionId))
|
||||
}
|
||||
if (pending) {
|
||||
|
||||
@@ -9,6 +9,7 @@ export interface CloseOthersDeps {
|
||||
deactivateTerminal: () => void
|
||||
closeTerminal: (id: string) => void
|
||||
closeReview: () => void
|
||||
selectReviewTab: () => void
|
||||
selectSessionTab: (id: string, pending: boolean) => void
|
||||
sessionClose: (id: string) => void
|
||||
}
|
||||
@@ -24,11 +25,11 @@ export interface CloseOthersDeps {
|
||||
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()
|
||||
deps.selectSessionTab(target, deps.isPending(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)) {
|
||||
|
||||
@@ -168,11 +168,18 @@ export function restoreTrackedTabs(
|
||||
closed: ReadonlySet<string> = new Set(),
|
||||
): string[] | undefined {
|
||||
// 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.
|
||||
// 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
|
||||
@@ -197,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[],
|
||||
|
||||
Reference in New Issue
Block a user