Merge pull request #12963 from Kilo-Org/fix/agent-manager-provider-max-lines

refactor(vscode): keep large files below line caps
2026-09-24 16:02:55 +08:00 · 2026-08-07 08:30:22 +02:00
parent 4009bb8b5d 0a2e5c278b
commit d5e1f9dee1
7 changed files with 287 additions and 183 deletions
@@ -1698,7 +1698,6 @@ export class AgentManagerProvider implements Disposable {
    panel.reveal(false)
    focusPanelPrompt(panel, this.waitForPanelReady(panel), this.waitForPanelActive(panel))
  }
-
  public isActive(): boolean {
    return this.panel?.active === true
  }
@@ -50,6 +50,7 @@ const TSX_FILES = [
  path.join(ROOT, "webview-ui/agent-manager/SidebarBody.tsx"),
  path.join(ROOT, "webview-ui/agent-manager/TabBar.tsx"),
  path.join(ROOT, "webview-ui/agent-manager/ProjectBranchDialog.tsx"),
+  path.join(ROOT, "webview-ui/agent-manager/DefaultBaseBranchDialog.tsx"),
  path.join(ROOT, "webview-ui/agent-manager/tab-rendering.tsx"),
  path.join(ROOT, "webview-ui/agent-manager/terminal/TerminalTab.tsx"),
  path.join(ROOT, "webview-ui/agent-manager/terminal/SideTerminalPanel.tsx"),
@@ -866,7 +867,7 @@ describe("KiloProvider — pending session refresh on reconnect", () => {
 // ---------------------------------------------------------------------------

 describe("Agent Manager — dialog listener cleanup", () => {
-  const tsx = fs.readFileSync(TSX_FILE, "utf-8")
+  const tsx = fs.readFileSync(path.join(ROOT, "webview-ui/agent-manager/DefaultBaseBranchDialog.tsx"), "utf-8")

  /**
   * Regression: handleChangeDefaultBaseBranch subscribes to vscode.onMessage
@@ -874,34 +875,20 @@ describe("Agent Manager — dialog listener cleanup", () => {
   * and the Escape keydown handler. If the dialog closed via backdrop click or
   * external dialog.close(), the listener leaked and stacked on every reopen.
   *
-   * The fix ties unsub() to Solid's onCleanup inside the dialog.show() render
-   * function so it always disposes regardless of how the dialog closes.
+   * The fix ties unsub() to the dialog component's Solid cleanup so it always
+   * disposes regardless of how the dialog closes.
   */
-  it("handleChangeDefaultBaseBranch uses onCleanup(unsub) inside dialog.show", () => {
-    const fnStart = tsx.indexOf("const handleChangeDefaultBaseBranch")
-    expect(fnStart, "handleChangeDefaultBaseBranch must exist").toBeGreaterThan(-1)
-
-    // Grab the function body (enough to cover the dialog.show callback)
-    const snippet = tsx.slice(fnStart, fnStart + 2000)
-
-    // The dialog.show callback must register onCleanup(unsub)
-    const showIdx = snippet.indexOf("dialog.show(")
-    expect(showIdx, "dialog.show() call must exist").toBeGreaterThan(-1)
-    const afterShow = snippet.slice(showIdx)
-    expect(afterShow, "onCleanup(unsub) must be inside dialog.show callback").toContain("onCleanup(unsub)")
+  it("DefaultBaseBranchDialog disposes its message listener on cleanup", () => {
+    expect(tsx).toContain("const unsub = vscode.onMessage")
+    expect(tsx).toContain("onCleanup(unsub)")
  })

-  it("selectBranch does not manually call unsub (handled by onCleanup)", () => {
-    const fnStart = tsx.indexOf("const handleChangeDefaultBaseBranch")
-    const snippet = tsx.slice(fnStart, fnStart + 2000)
-
-    // Find the selectBranch function body
-    const selStart = snippet.indexOf("const selectBranch")
-    expect(selStart, "selectBranch must exist").toBeGreaterThan(-1)
-    const selEnd = snippet.indexOf("}", selStart + 50)
-    const selBody = snippet.slice(selStart, selEnd + 1)
-
-    expect(selBody, "selectBranch should not call unsub() directly").not.toContain("unsub()")
+  it("select does not manually call unsub (handled by onCleanup)", () => {
+    const selStart = tsx.indexOf("const select =")
+    expect(selStart, "select must exist").toBeGreaterThan(-1)
+    const selEnd = tsx.indexOf("}", selStart + 40)
+    const selBody = tsx.slice(selStart, selEnd + 1)
+    expect(selBody, "select should not call unsub() directly").not.toContain("unsub()")
  })
 })

@@ -0,0 +1,66 @@
+import { describe, expect, it } from "bun:test"
+import { createSessionVariants } from "../../webview-ui/src/context/session-variants"
+import type { ExtensionMessage, ModelSelection } from "../../webview-ui/src/types/messages"
+
+const model: ModelSelection = { providerID: "anthropic", modelID: "claude-sonnet-4" }
+
+function setup(session?: string) {
+  const selections: Record<string, string> = {}
+  const messages: Array<{ type: string; key?: string; value?: string }> = []
+  const order: string[] = []
+  let handler: ((message: ExtensionMessage) => void) | undefined
+  const variants = createSessionVariants({
+    selections: () => selections,
+    set: (key, value) => {
+      selections[key] = value
+    },
+    selected: () => model,
+    session: () => session,
+    agent: () => "code",
+    find: () => ({ variants: { low: {}, high: {} } }),
+    post: (message) => {
+      order.push("post")
+      messages.push(message)
+    },
+    listen: (next) => {
+      order.push("listen")
+      handler = next
+      return () => order.push("unsub")
+    },
+  })
+  return { variants, selections, messages, order, dispatch: (message: ExtensionMessage) => handler?.(message) }
+}
+
+describe("session variants", () => {
+  it("subscribes before requesting persisted variants and returns cleanup", () => {
+    const state = setup()
+    const unsub = state.variants.load()
+    expect(state.order).toEqual(["listen", "post"])
+    expect(state.messages).toEqual([{ type: "requestVariants" }])
+    unsub()
+    expect(state.order).toEqual(["listen", "post", "unsub"])
+  })
+
+  it("loads global variants without restoring stale session variants", () => {
+    const state = setup()
+    state.variants.load()
+    state.dispatch({
+      type: "variantsLoaded",
+      variants: { "agent/code/anthropic/claude-sonnet-4": "high", "session/old/model": "low" },
+    })
+    expect(state.selections).toEqual({ "agent/code/anthropic/claude-sonnet-4": "high" })
+  })
+
+  it("persists global selections but keeps session selections local", () => {
+    const global = setup()
+    global.variants.select("high")
+    expect(global.messages).toEqual([
+      { type: "persistVariant", key: "agent/code/anthropic/claude-sonnet-4", value: "high" },
+    ])
+
+    const scoped = setup("session-a")
+    scoped.variants.select("low")
+    expect(scoped.selections).toEqual({ "session/session-a/anthropic/claude-sonnet-4": "low" })
+    expect(scoped.messages).toEqual([])
+  })
+})
@@ -21,7 +21,6 @@ import type {
  AgentManagerKeybindingsMessage,
  AgentManagerMultiVersionProgressMessage,
  AgentManagerSendInitialMessage,
-  AgentManagerBranchesMessage,
  AgentManagerWorktreeDiffMessage,
  AgentManagerWorktreeDiffFileMessage,
  AgentManagerWorktreeDiffLoadingMessage,
@@ -43,7 +42,6 @@ import type {
  SectionState,
  SessionInfo,
  SessionCreatedMessage,
-  BranchInfo,
  TerminalDestination,
  TerminalFont,
 } from "../src/types/messages"
@@ -78,6 +76,7 @@ import { ProviderShell } from "../src/context/provider-shell"
 import { ChatView } from "../src/components/chat"
 import HistoryView from "../src/components/history/HistoryView"
 import { NewWorktreeDialog } from "./NewWorktreeDialog"
+import { DefaultBaseBranchDialog } from "./DefaultBaseBranchDialog"
 import { createModeRouter } from "./mode-router"
 import { ProjectList } from "./ProjectList"
 import { SidebarBody } from "./SidebarBody"
@@ -152,7 +151,6 @@ import type { ReviewComment } from "../diff-viewer/review-comments"
 import { clearReviewComposer, createReviewComposer } from "../diff-viewer/review-annotations"
 import type { SidebarSearchMenuRef } from "./SidebarSearchMenu"
 import { createSidebarSearch, type SidebarSearchItem } from "./sidebar-search"
-import { BranchSelect } from "../src/components/shared/BranchSelect"
 import { randomColor } from "./section-colors"
 import { createNewTaskDrafts } from "./new-task-drafts"
 import {
@@ -251,7 +249,6 @@ const AgentManagerContent: Component = () => {
    projectList().length === 0 || pid === undefined || pid === activeProjectId()

  const repoDefaultBranch = () => defaultBaseBranch() ?? repoDetectedBranch() ?? "main"
-  const hasConfiguredBranch = () => !!defaultBaseBranch()

  const DEFAULT_SIDEBAR_WIDTH = 260
  const MIN_SIDEBAR_WIDTH = 200
@@ -1744,99 +1741,15 @@ const AgentManagerContent: Component = () => {
  const setupScript = metrics.click("configure_setup_script", "worktree_settings", handleConfigureSetupScript)

  const handleChangeDefaultBaseBranch = () => {
-    const [search, setSearch] = createSignal("")
-    const [branches, setBranches] = createSignal<BranchInfo[]>([])
-    const [loading, setLoading] = createSignal(true)
-    const [highlighted, setHighlighted] = createSignal(-1)
-
-    const unsub = vscode.onMessage((msg) => {
-      if (msg.type === "agentManager.branches") {
-        const ev = msg as AgentManagerBranchesMessage
-        setBranches(ev.branches)
-        if (ev.defaultBranch) setRepoDetectedBranch(ev.defaultBranch)
-        setLoading(false)
-      }
-    })
-
-    vscode.postMessage({ type: "agentManager.requestBranches" })
-
-    const filtered = createMemo(() => {
-      const s = search().toLowerCase()
-      if (!s) return branches()
-      return branches().filter((b) => b.name.toLowerCase().includes(s))
-    })
-
-    const selectBranch = (name: string | undefined) => {
-      vscode.postMessage({ type: "agentManager.setDefaultBaseBranch", branch: name })
-      setDefaultBaseBranch(name)
-      dialog.close()
-    }
-
-    const handleKeyDown = (e: KeyboardEvent) => {
-      const items = filtered()
-      // offset by 1 for auto-detect option (-1 = auto-detect)
-      const total = items.length + 1
-      if (e.key === "ArrowDown") {
-        e.preventDefault()
-        e.stopPropagation()
-        setHighlighted((prev) => Math.min(prev + 1, total - 2))
-      } else if (e.key === "ArrowUp") {
-        e.preventDefault()
-        e.stopPropagation()
-        setHighlighted((prev) => Math.max(prev - 1, -1))
-      } else if (e.key === "Enter") {
-        e.preventDefault()
-        e.stopPropagation()
-        const idx = highlighted()
-        if (idx === -1) {
-          selectBranch(undefined)
-        } else {
-          const branch = items[idx]
-          if (branch) selectBranch(branch.name)
-        }
-      } else if (e.key === "Escape") {
-        e.preventDefault()
-        e.stopPropagation()
-        dialog.close()
-      }
-    }
-
-    dialog.show(() => {
-      onCleanup(unsub)
-      return (
-        <Dialog title={t("agentManager.worktree.defaultBaseBranch")} fit>
-          <div class="am-default-base-branch">
-            <BranchSelect
-              branches={filtered()}
-              loading={loading()}
-              search={search()}
-              onSearch={(v) => {
-                setSearch(v)
-                setHighlighted(-1)
-              }}
-              onSelect={(b) => selectBranch(b.name)}
-              onSearchKeyDown={handleKeyDown}
-              selected={defaultBaseBranch()}
-              highlighted={highlighted()}
-              onHighlight={setHighlighted}
-              searchPlaceholder={t("agentManager.dialog.searchBranches")}
-              emptyLabel={t("agentManager.import.noMatchingBranches")}
-              loadingLabel={t("agentManager.import.loadingBranches")}
-              defaultLabel={t("agentManager.dialog.branchBadge.default")}
-              remoteLabel={t("agentManager.dialog.branchBadge.remote")}
-              defaultName={defaultBaseBranch()}
-              autoOption={{
-                label: t("agentManager.worktree.defaultBaseBranchAuto"),
-                hint: repoDetectedBranch(),
-                active: !hasConfiguredBranch(),
-                highlighted: highlighted() === -1,
-                onSelect: () => selectBranch(undefined),
-              }}
-            />
-          </div>
-        </Dialog>
-      )
-    })
+    dialog.show(() => (
+      <DefaultBaseBranchDialog
+        selected={defaultBaseBranch()}
+        detected={repoDetectedBranch()}
+        onSelect={setDefaultBaseBranch}
+        onDetected={setRepoDetectedBranch}
+        onClose={() => dialog.close()}
+      />
+    ))
  }

  const handleShowKeyboardShortcuts = () => {
@@ -0,0 +1,110 @@
+/** @jsxImportSource solid-js */
+
+import { createMemo, createSignal, onCleanup, type Component } from "solid-js"
+import { Dialog } from "@kilocode/kilo-ui/dialog"
+import { BranchSelect } from "../src/components/shared/BranchSelect"
+import { useLanguage } from "../src/context/language"
+import { useVSCode } from "../src/context/vscode"
+import type { AgentManagerBranchesMessage, BranchInfo } from "../src/types/messages"
+
+interface Props {
+  selected?: string
+  detected?: string
+  onSelect: (branch?: string) => void
+  onDetected: (branch: string) => void
+  onClose: () => void
+}
+
+export const DefaultBaseBranchDialog: Component<Props> = (props) => {
+  const { t } = useLanguage()
+  const vscode = useVSCode()
+  const [search, setSearch] = createSignal("")
+  const [branches, setBranches] = createSignal<BranchInfo[]>([])
+  const [loading, setLoading] = createSignal(true)
+  const [highlighted, setHighlighted] = createSignal(-1)
+  const filtered = createMemo(() => {
+    const value = search().toLowerCase()
+    return value ? branches().filter((branch) => branch.name.toLowerCase().includes(value)) : branches()
+  })
+  const select = (branch?: string) => {
+    vscode.postMessage({ type: "agentManager.setDefaultBaseBranch", branch })
+    props.onSelect(branch)
+    props.onClose()
+  }
+  const unsub = vscode.onMessage((message) => {
+    if (message.type !== "agentManager.branches") return
+    const event = message as AgentManagerBranchesMessage
+    setBranches(event.branches)
+    if (event.defaultBranch) props.onDetected(event.defaultBranch)
+    setLoading(false)
+  })
+  onCleanup(unsub)
+  vscode.postMessage({ type: "agentManager.requestBranches" })
+
+  const keydown = (event: KeyboardEvent) => {
+    const items = filtered()
+    const total = items.length + 1
+    if (event.key === "ArrowDown") {
+      event.preventDefault()
+      event.stopPropagation()
+      setHighlighted((value) => Math.min(value + 1, total - 2))
+      return
+    }
+    if (event.key === "ArrowUp") {
+      event.preventDefault()
+      event.stopPropagation()
+      setHighlighted((value) => Math.max(value - 1, -1))
+      return
+    }
+    if (event.key === "Enter") {
+      event.preventDefault()
+      event.stopPropagation()
+      const index = highlighted()
+      if (index === -1) {
+        select()
+        return
+      }
+      const branch = items[index]
+      if (branch) select(branch.name)
+      return
+    }
+    if (event.key !== "Escape") return
+    event.preventDefault()
+    event.stopPropagation()
+    props.onClose()
+  }
+
+  return (
+    <Dialog title={t("agentManager.worktree.defaultBaseBranch")} fit>
+      <div class="am-default-base-branch">
+        <BranchSelect
+          branches={filtered()}
+          loading={loading()}
+          search={search()}
+          onSearch={(value) => {
+            setSearch(value)
+            setHighlighted(-1)
+          }}
+          onSelect={(branch) => select(branch.name)}
+          onSearchKeyDown={keydown}
+          selected={props.selected}
+          highlighted={highlighted()}
+          onHighlight={setHighlighted}
+          searchPlaceholder={t("agentManager.dialog.searchBranches")}
+          emptyLabel={t("agentManager.import.noMatchingBranches")}
+          loadingLabel={t("agentManager.import.loadingBranches")}
+          defaultLabel={t("agentManager.dialog.branchBadge.default")}
+          remoteLabel={t("agentManager.dialog.branchBadge.remote")}
+          defaultName={props.selected}
+          autoOption={{
+            label: t("agentManager.worktree.defaultBaseBranchAuto"),
+            hint: props.detected,
+            active: !props.selected,
+            highlighted: highlighted() === -1,
+            onSelect: () => select(),
+          }}
+        />
+      </div>
+    </Dialog>
+  )
+}
@@ -0,0 +1,73 @@
+import type { Accessor } from "solid-js"
+import type { ExtensionMessage, ModelSelection } from "../types/messages"
+import { getAgentVariant, getVariant, preserveVariant, variantKey } from "./session-variant-store"
+
+interface Model {
+  variants?: Record<string, unknown>
+}
+
+type Message = { type: "requestVariants" } | { type: "persistVariant"; key: string; value: string }
+
+interface Options {
+  selections: Accessor<Record<string, string>>
+  set: (key: string, value: string) => void
+  selected: (sessionID?: string) => ModelSelection | null
+  session: Accessor<string | undefined>
+  agent: (sessionID?: string) => string
+  find: (selection: ModelSelection) => Model | undefined
+  post: (message: Message) => void
+  listen: (handler: (message: ExtensionMessage) => void) => () => void
+}
+
+export function createSessionVariants(options: Options) {
+  const list = (sessionID?: string) => {
+    const selection = options.selected(sessionID)
+    if (!selection) return []
+    return Object.keys(options.find(selection)?.variants ?? {})
+  }
+
+  const agent = (name: string, selection: ModelSelection | null) => {
+    if (!selection) return undefined
+    return getAgentVariant(options.selections(), selection, options.find(selection), name)
+  }
+
+  const current = (sessionID?: string) => {
+    const sid = sessionID ?? options.session()
+    const selection = options.selected(sid)
+    if (!selection) return undefined
+    const variants = list(sid)
+    if (variants.length === 0) return undefined
+    return getVariant(options.selections(), selection, variants, options.agent(sid), sid)
+  }
+
+  const select = (value: string, sessionID?: string) => {
+    const sid = sessionID ?? options.session()
+    const selection = options.selected(sid)
+    if (!selection) return
+    const key = variantKey(selection, options.agent(sid), sid)
+    options.set(key, value)
+    if (!sid) options.post({ type: "persistVariant", key, value })
+  }
+
+  const carry = (selection: ModelSelection, value: string | undefined, name: string, sessionID?: string) => {
+    const next = preserveVariant(value, Object.keys(options.find(selection)?.variants ?? {}))
+    if (!next) return
+    const key = variantKey(selection, name, sessionID)
+    options.set(key, next)
+    if (!sessionID) options.post({ type: "persistVariant", key, value: next })
+  }
+
+  const load = () => {
+    const unsub = options.listen((message) => {
+      if (message.type !== "variantsLoaded") return
+      for (const [key, value] of Object.entries(message.variants)) {
+        if (key.startsWith("session/")) continue
+        options.set(key, value)
+      }
+    })
+    options.post({ type: "requestVariants" })
+    return unsub
+  }
+
+  return { carry, list, agent, current, select, load }
+}
@@ -74,14 +74,8 @@ import { errorIDs } from "./session-errors"
 import { PartStash } from "./part-stash"
 import { mergeParts, sameParts } from "./session-parts"
 import { state as todoState } from "./todo-revert"
-import {
-  getAgentVariant,
-  getVariant,
-  preserveVariant,
-  sessionVariantKeys,
-  transferVariants,
-  variantKey,
-} from "./session-variant-store"
+import { sessionVariantKeys, transferVariants, variantKey } from "./session-variant-store"
+import { createSessionVariants } from "./session-variants"
 import { KILO_AUTO, KILO_PROVIDER_ID, parseModelString } from "../../../src/shared/provider-model"
 import { reviewMetadata, type ReviewMessageData } from "../../../src/shared/review-comments"
 import { visibleMessages as filterVisibleMessages } from "./session-queue"
@@ -670,13 +664,18 @@ export const SessionProvider: ParentComponent = (props) => {
    })
  }

-  function carryVariant(selection: ModelSelection, current: string | undefined, agent: string, sessionID?: string) {
-    const value = preserveVariant(current, Object.keys(provider.findModel(selection)?.variants ?? {}))
-    if (!value) return
-    const key = variantKey(selection, agent, sessionID)
-    setStore("variantSelections", key, value)
-    if (!sessionID) vscode.postMessage({ type: "persistVariant", key, value })
-  }
+  const variants = createSessionVariants({
+    selections: () => store.variantSelections,
+    set: (key, value) => setStore("variantSelections", key, value),
+    selected,
+    session: currentSessionID,
+    agent: agentForScope,
+    find: provider.findModel,
+    post: vscode.postMessage,
+    listen: vscode.onMessage,
+  })
+  const { carry: carryVariant, list: variantList, agent: variantForAgent, current: currentVariant } = variants
+  const selectVariant = variants.select

  function selectModel(providerID: string, modelID: string, sessionID?: string) {
    const sid = sessionID ?? currentSessionID()
@@ -930,50 +929,7 @@ export const SessionProvider: ParentComponent = (props) => {
    clearTimeout(fallback)
  })

-  const variantList = (sessionID?: string) => {
-    const sel = selected(sessionID)
-    if (!sel) return []
-    const model = provider.findModel(sel)
-    if (!model?.variants) return []
-    return Object.keys(model.variants)
-  }
-
-  function variantForAgent(agentName: string, sel: ModelSelection | null) {
-    if (!sel) return undefined
-    const model = provider.findModel(sel)
-    return getAgentVariant(store.variantSelections, sel, model, agentName)
-  }
-
-  const currentVariant = (sessionID?: string) => {
-    const sid = sessionID ?? currentSessionID()
-    const sel = selected(sid)
-    if (!sel) return undefined
-    const list = variantList(sid)
-    if (list.length === 0) return undefined
-    return getVariant(store.variantSelections, sel, list, agentForScope(sid), sid)
-  }
-
-  const selectVariant = (value: string, sessionID?: string) => {
-    const sid = sessionID ?? currentSessionID()
-    const sel = selected(sid)
-    if (!sel) return
-    const key = variantKey(sel, agentForScope(sid), sid)
-    setStore("variantSelections", key, value)
-    if (!sid) vscode.postMessage({ type: "persistVariant", key, value })
-  }
-
-  // Load persisted variants from extension globalState
-  const unsubVariants = vscode.onMessage((message: ExtensionMessage) => {
-    if (message.type !== "variantsLoaded") return
-    for (const [k, v] of Object.entries(message.variants)) {
-      if (k.startsWith("session/")) continue
-      setStore("variantSelections", k, v)
-    }
-  })
-
-  vscode.postMessage({ type: "requestVariants" })
-
-  onCleanup(unsubVariants)
+  onCleanup(variants.load())

  // Load persisted per-mode model selections from model.json via extension host.
  // Uses replace semantics so a reset (empty payload) clears old entries.