fix(agent-manager): fix sidebar navigation and shortcuts with sections (#8647)

Navigation (cmd+option+up/down) and jump shortcuts (cmd+1-9) used sortedWorktrees() which ignores section layout. When sections exist, the visual order differs from the flat worktree order, causing jumps to non-adjacent items and mismatched shortcut badges. Introduce buildSidebarOrder() to compute the true visual order matching what the user sees (respecting section boundaries and collapsed state), and buildShortcutMap() for consistent shortcut numbering across all visible worktrees including those inside sections.
2026-08-29 03:44:06 +08:00 · 2026-04-09 15:31:33 +02:00
parent a00b880902
commit 7b3756f27d
3 changed files with 200 additions and 56 deletions
@@ -1,5 +1,12 @@
 import { describe, it, expect } from "bun:test"
-import { buildTopLevelItems, isGrouped, isGroupStart, isGroupEnd } from "../../webview-ui/agent-manager/section-helpers"
+import {
+  buildTopLevelItems,
+  buildSidebarOrder,
+  buildShortcutMap,
+  isGrouped,
+  isGroupStart,
+  isGroupEnd,
+} from "../../webview-ui/agent-manager/section-helpers"
 import type { WorktreeState, SectionState } from "../../webview-ui/src/types/messages"

 function wt(id: string, opts: Partial<WorktreeState> = {}): WorktreeState {
@@ -112,3 +119,102 @@ describe("isGroupEnd", () => {
    expect(isGroupEnd(list[3]!, 3, list)).toBe(false)
  })
 })
+
+describe("buildSidebarOrder", () => {
+  it("returns LOCAL + all sorted worktrees when no sections exist", () => {
+    const sorted = [wt("a"), wt("b"), wt("c")]
+    const items = buildTopLevelItems([], [], sorted, [])
+    const result = buildSidebarOrder(items, sorted, [], () => [], [])
+    expect(result).toEqual([
+      { type: "local", id: "local" },
+      { type: "wt", id: "a" },
+      { type: "wt", id: "b" },
+      { type: "wt", id: "c" },
+    ])
+  })
+
+  it("includes section worktrees in visual order", () => {
+    const s1 = sec("s1", 0)
+    const w1 = wt("w1", { sectionId: "s1" })
+    const w2 = wt("w2", { sectionId: "s1" })
+    const w3 = wt("w3")
+    const sorted = [w1, w2, w3]
+    const items = buildTopLevelItems([s1], [w3], sorted, ["s1", "w3"])
+    const members = (id: string) => (id === "s1" ? [w1, w2] : [])
+    const result = buildSidebarOrder(items, sorted, [s1], members, [])
+    expect(result).toEqual([
+      { type: "local", id: "local" },
+      { type: "wt", id: "w1" },
+      { type: "wt", id: "w2" },
+      { type: "wt", id: "w3" },
+    ])
+  })
+
+  it("skips worktrees in collapsed sections", () => {
+    const s1 = sec("s1", 0, { collapsed: true })
+    const w1 = wt("w1", { sectionId: "s1" })
+    const w2 = wt("w2")
+    const sorted = [w1, w2]
+    const items = buildTopLevelItems([s1], [w2], sorted, ["s1", "w2"])
+    const members = (id: string) => (id === "s1" ? [w1] : [])
+    const result = buildSidebarOrder(items, sorted, [s1], members, [])
+    expect(result).toEqual([
+      { type: "local", id: "local" },
+      { type: "wt", id: "w2" },
+    ])
+  })
+
+  it("respects section order between sections and ungrouped worktrees", () => {
+    const s1 = sec("s1", 0)
+    const s2 = sec("s2", 1)
+    const w1 = wt("w1", { sectionId: "s1" })
+    const w2 = wt("w2")
+    const w3 = wt("w3", { sectionId: "s2" })
+    const sorted = [w1, w2, w3]
+    const items = buildTopLevelItems([s1, s2], [w2], sorted, ["s1", "w2", "s2"])
+    const members = (id: string) => {
+      if (id === "s1") return [w1]
+      if (id === "s2") return [w3]
+      return []
+    }
+    const result = buildSidebarOrder(items, sorted, [s1, s2], members, [])
+    expect(result.map((r) => r.id)).toEqual(["local", "w1", "w2", "w3"])
+  })
+
+  it("appends unassigned sessions after worktrees", () => {
+    const sorted = [wt("a")]
+    const items = buildTopLevelItems([], [], sorted, [])
+    const sessions = [{ id: "sess1" }, { id: "sess2" }]
+    const result = buildSidebarOrder(items, sorted, [], () => [], sessions)
+    expect(result).toEqual([
+      { type: "local", id: "local" },
+      { type: "wt", id: "a" },
+      { type: "session", id: "sess1" },
+      { type: "session", id: "sess2" },
+    ])
+  })
+})
+
+describe("buildShortcutMap", () => {
+  it("assigns 1-based shortcuts up to 9", () => {
+    const order = [
+      { type: "local" as const, id: "local" },
+      { type: "wt" as const, id: "a" },
+      { type: "wt" as const, id: "b" },
+    ]
+    const map = buildShortcutMap(order)
+    expect(map.get("local")).toBe(1)
+    expect(map.get("a")).toBe(2)
+    expect(map.get("b")).toBe(3)
+  })
+
+  it("caps at 9 shortcuts", () => {
+    const order = Array.from({ length: 12 }, (_, i) => ({
+      type: "wt" as const,
+      id: `w${i}`,
+    }))
+    const map = buildShortcutMap(order)
+    expect(map.size).toBe(9)
+    expect(map.has("w9")).toBe(false)
+  })
+})
@@ -94,7 +94,15 @@ import { BranchSelect } from "./BranchSelect"
 import { WorktreeItem } from "./WorktreeItem"
 import SectionHeader from "./SectionHeader"
 import { randomColor } from "./section-colors"
-import { buildTopLevelItems, isGrouped, isGroupStart, isGroupEnd, type TopLevelItem } from "./section-helpers"
+import {
+  buildTopLevelItems,
+  buildSidebarOrder,
+  buildShortcutMap,
+  isGrouped,
+  isGroupStart,
+  isGroupEnd,
+  type TopLevelItem,
+} from "./section-helpers"
 import { sectionAwareDetector } from "./section-dnd"
 import { ConstrainDragXAxis } from "./constrain-drag-x"
 import { mergeWorktreeDiffs } from "./diff-state"
@@ -873,6 +881,14 @@ const AgentManagerContent: Component = () => {
  const topLevelItems = createMemo((): TopLevelItem[] =>
    buildTopLevelItems(sections(), ungrouped(), sortedWorktrees(), sidebarWorktreeOrder()),
  )
+
+  /** Flat visual order of all visible sidebar items — used for navigation and shortcut assignment. */
+  const sidebarOrder = createMemo(() =>
+    buildSidebarOrder(topLevelItems(), sortedWorktrees(), sections(), worktreesInSection, unassignedSessions()),
+  )
+  /** Map from sidebar item id → 1-based shortcut number (⌘1 for LOCAL, ⌘2 for first worktree, etc.) */
+  const shortcutMap = createMemo(() => buildShortcutMap(sidebarOrder()))
+
  const moveToSection = (ids: string[], sec: string | null) =>
    vscode.postMessage({ type: "agentManager.moveToSection", worktreeIds: ids, sectionId: sec })
  const moveSection = (sectionId: string, dir: -1 | 1) =>
@@ -887,54 +903,36 @@ const AgentManagerContent: Component = () => {
    })
  }

-  const scrollIntoView = (el: HTMLElement) => {
-    el.scrollIntoView({ block: "nearest", behavior: "smooth" })
-  }
+  const scrollIntoView = (el: HTMLElement) => el.scrollIntoView({ block: "nearest", behavior: "smooth" })

-  // Navigate sidebar items with arrow keys
-  const navigate = (direction: "up" | "down") => {
-    const flat: { type: typeof LOCAL | "wt" | "session"; id: string }[] = [
-      { type: LOCAL, id: LOCAL },
-      ...sortedWorktrees().map((wt) => ({ type: "wt" as const, id: wt.id })),
-      ...unassignedSessions().map((s) => ({ type: "session" as const, id: s.id })),
-    ]
-    if (flat.length === 0) return
-
-    const current = selection() ?? session.currentSessionID()
-    const idx = current ? flat.findIndex((f) => f.id === current) : -1
-    const next = direction === "up" ? idx - 1 : idx + 1
-    if (next < 0 || next >= flat.length) return
-
-    const item = flat[next]!
-    if (item.type === LOCAL) {
-      selectLocal()
-    } else if (item.type === "wt") {
-      selectWorktree(item.id)
-    } else {
+  const focusSidebarItem = (item: { type: string; id: string }) => {
+    if (item.type === "local") selectLocal()
+    else if (item.type === "wt") selectWorktree(item.id)
+    else {
      saveTabMemory()
      setSelection(null)
      setReviewActive(false)
      session.selectSession(item.id)
    }
-
    const el = document.querySelector(`[data-sidebar-id="${item.id}"]`)
    if (el instanceof HTMLElement) scrollIntoView(el)
  }

-  // Jump to sidebar item by 1-based index (⌘1 = LOCAL, ⌘2 = first worktree, etc.)
+  // Navigate sidebar items with arrow keys (uses visual order from sidebarOrder)
+  const navigate = (direction: "up" | "down") => {
+    const flat = sidebarOrder()
+    if (flat.length === 0) return
+    const current = selection() ?? session.currentSessionID()
+    const idx = current ? flat.findIndex((f) => f.id === current) : -1
+    const next = direction === "up" ? idx - 1 : idx + 1
+    if (next < 0 || next >= flat.length) return
+    focusSidebarItem(flat[next]!)
+  }
+
+  // Jump to sidebar item by 0-based index into sidebarOrder (⌘1 = index 0 = LOCAL, ⌘2 = index 1, etc.)
  const jumpToItem = (index: number) => {
-    if (index === 0) {
-      selectLocal()
-      const el = document.querySelector(`[data-sidebar-id="local"]`)
-      if (el instanceof HTMLElement) scrollIntoView(el)
-      return
-    }
-    const wts = sortedWorktrees()
-    const wt = wts[index - 1]
-    if (!wt) return
-    selectWorktree(wt.id)
-    const el = document.querySelector(`[data-sidebar-id="${wt.id}"]`)
-    if (el instanceof HTMLElement) scrollIntoView(el)
+    const item = sidebarOrder()[index]
+    if (item) focusSidebarItem(item)
  }

  // Navigate tabs with Cmd+Alt+Left/Right
@@ -1759,7 +1757,9 @@ const AgentManagerContent: Component = () => {
      if (selection() === wt.id) {
        const next = nextSelectionAfterDelete(
          wt.id,
-          worktrees().map((w) => w.id),
+          sidebarOrder()
+            .filter((f) => f.type === "wt")
+            .map((f) => f.id),
        )
        if (next === LOCAL) selectLocal()
        else selectWorktree(next)
@@ -1782,7 +1782,9 @@ const AgentManagerContent: Component = () => {
      if (selection() === wt.id) {
        const next = nextSelectionAfterDelete(
          wt.id,
-          worktrees().map((w) => w.id),
+          sidebarOrder()
+            .filter((f) => f.type === "wt")
+            .map((f) => f.id),
        )
        if (next === LOCAL) selectLocal()
        if (next !== LOCAL) selectWorktree(next)
@@ -2304,12 +2306,7 @@ const AgentManagerContent: Component = () => {
                      <ConstrainDragXAxis />
                      <SortableProvider ids={wtIds()}>
                        {(() => {
-                          const renderWt = (
-                            wt: WorktreeState,
-                            idx: () => number,
-                            inSection?: boolean,
-                            list?: WorktreeState[],
-                          ) => {
+                          const renderWt = (wt: WorktreeState, idx: () => number, list?: WorktreeState[]) => {
                            const wtSessions = createMemo(() =>
                              managedSessions().filter((ms) => ms.worktreeId === wt.id),
                            )
@@ -2317,11 +2314,7 @@ const AgentManagerContent: Component = () => {
                              adjacentHint(
                                wt.id,
                                selection() ?? session.currentSessionID() ?? "",
-                                [
-                                  LOCAL as string,
-                                  ...sortedWorktrees().map((w) => w.id),
-                                  ...unassignedSessions().map((s) => s.id),
-                                ],
+                                sidebarOrder().map((f) => f.id),
                                kb().previousSession ?? "",
                                kb().nextSession ?? "",
                              )
@@ -2343,7 +2336,7 @@ const AgentManagerContent: Component = () => {
                                  busy={busyWorktrees().has(wt.id)}
                                  working={isAgentBusy(wt.id)}
                                  stale={isStaleWorktree(wt.id)}
-                                  shortcut={inSection ? undefined : idx() + 2}
+                                  shortcut={shortcutMap().get(wt.id)}
                                  stats={worktreeStats()[wt.id]}
                                  navHint={navHint()}
                                  sessions={wtSessions().length}
@@ -2417,9 +2410,7 @@ const AgentManagerContent: Component = () => {
                                      >
                                        <Show when={!sec.collapsed}>
                                          <div class="am-section-group-body">
-                                            <For each={members()}>
-                                              {(wt, wtIdx) => renderWt(wt, wtIdx, true, members())}
-                                            </For>
+                                            <For each={members()}>{(wt, wtIdx) => renderWt(wt, wtIdx, members())}</For>
                                          </div>
                                        </Show>
                                      </SectionHeader>
@@ -6,6 +6,8 @@ import type { WorktreeState, SectionState } from "../src/types/messages"

 export type TopLevelItem = { kind: "section"; section: SectionState } | { kind: "worktree"; wt: WorktreeState }

+export type SidebarItem = { type: "local" | "wt" | "session"; id: string }
+
 /** Check if this worktree is part of a multi-version group. */
 export const isGrouped = (wt: WorktreeState) => !!wt.groupId

@@ -60,3 +62,48 @@ export function buildTopLevelItems(
  }
  return result
 }
+
+/**
+ * Build the flat visual order of all sidebar items matching what the user sees.
+ * LOCAL is always first, then worktrees in visual order (respecting section layout and
+ * skipping collapsed sections), then unassigned sessions.
+ */
+export function buildSidebarOrder(
+  items: TopLevelItem[],
+  sorted: WorktreeState[],
+  sections: SectionState[],
+  members: (id: string) => WorktreeState[],
+  sessions: { id: string }[],
+): SidebarItem[] {
+  const result: SidebarItem[] = [{ type: "local", id: "local" }]
+  if (sections.length > 0) {
+    for (const item of items) {
+      if (item.kind === "section") {
+        if (!item.section.collapsed) {
+          for (const wt of members(item.section.id)) {
+            result.push({ type: "wt", id: wt.id })
+          }
+        }
+      } else {
+        result.push({ type: "wt", id: item.wt.id })
+      }
+    }
+  } else {
+    for (const wt of sorted) {
+      result.push({ type: "wt", id: wt.id })
+    }
+  }
+  for (const s of sessions) {
+    result.push({ type: "session", id: s.id })
+  }
+  return result
+}
+
+/** Build a map from sidebar item id → 1-based shortcut number (1 for LOCAL, 2+ for worktrees). */
+export function buildShortcutMap(order: SidebarItem[]): Map<string, number> {
+  const map = new Map<string, number>()
+  for (let i = 0; i < order.length && i < 9; i++) {
+    map.set(order[i]!.id, i + 1)
+  }
+  return map
+}