Add fetch tool details and improve sidebar shortcuts handling

2026-03-19 13:04:44 +07:00
parent 675d825f6b
commit 4ab307b10f
26 changed files with 960 additions and 197 deletions
@@ -1,5 +1,8 @@
 import { describe, expect, it } from 'vitest'
-import { buildSidebarProjectRowModel } from './sidebar-project-row-model'
+import {
+  buildSidebarProjectRowModel,
+  isSidebarProjectFlattened,
+} from './sidebar-project-row-model'
 import type {
  SidebarProjectEntry,
  SidebarWorkspaceEntry,
@@ -51,11 +54,11 @@ describe('buildSidebarProjectRowModel', () => {
    })

    expect(result).toEqual({
-      interaction: 'navigate',
-      chevron: 'disclosure',
-      trailingAction: 'none',
-      flattenedWorkspace,
+      kind: 'workspace_link',
+      workspace: flattenedWorkspace,
      selected: false,
+      chevron: null,
+      trailingAction: 'none',
    })
  })

@@ -78,10 +81,36 @@ describe('buildSidebarProjectRowModel', () => {
      },
    })

-    expect(result.selected).toBe(true)
+    expect(result).toMatchObject({
+      kind: 'workspace_link',
+      selected: true,
+    })
  })

-  it('keeps git projects as expandable sections with a new worktree action', () => {
+  it('flattens git projects with a single workspace and keeps the new worktree action', () => {
+    const flattenedWorkspace = workspace({
+      workspaceId: '/repo/main',
+      workspaceKind: 'local_checkout',
+    })
+
+    const result = buildSidebarProjectRowModel({
+      project: project({
+        projectKind: 'git',
+        workspaces: [flattenedWorkspace],
+      }),
+      collapsed: true,
+    })
+
+    expect(result).toEqual({
+      kind: 'workspace_link',
+      workspace: flattenedWorkspace,
+      selected: false,
+      chevron: null,
+      trailingAction: 'new_worktree',
+    })
+  })
+
+  it('keeps multi-workspace git projects as expandable sections with a new worktree action', () => {
    const result = buildSidebarProjectRowModel({
      project: project({
        projectKind: 'git',
@@ -94,11 +123,28 @@ describe('buildSidebarProjectRowModel', () => {
    })

    expect(result).toEqual({
-      interaction: 'toggle',
+      kind: 'project_section',
      chevron: 'expand',
      trailingAction: 'new_worktree',
-      flattenedWorkspace: null,
-      selected: false,
    })
  })
 })
+
+describe('isSidebarProjectFlattened', () => {
+  it('returns true for single-workspace projects regardless of kind', () => {
+    expect(isSidebarProjectFlattened(project({ projectKind: 'git', workspaces: [workspace()] }))).toBe(true)
+    expect(
+      isSidebarProjectFlattened(project({ projectKind: 'non_git', workspaces: [workspace()] }))
+    ).toBe(true)
+  })
+
+  it('returns false for multi-workspace projects', () => {
+    expect(
+      isSidebarProjectFlattened(
+        project({
+          workspaces: [workspace({ workspaceId: '/repo/main' }), workspace({ workspaceId: '/repo/feat' })],
+        })
+      )
+    ).toBe(false)
+  })
+})
@@ -3,12 +3,26 @@ import type {
  SidebarWorkspaceEntry,
 } from '@/hooks/use-sidebar-workspaces-list'

-export interface SidebarProjectRowModel {
-  interaction: 'toggle' | 'navigate'
-  chevron: 'expand' | 'collapse' | 'disclosure'
-  trailingAction: 'new_worktree' | 'none'
-  flattenedWorkspace: SidebarWorkspaceEntry | null
+export interface SidebarProjectWorkspaceLinkRowModel {
+  kind: 'workspace_link'
+  workspace: SidebarWorkspaceEntry
  selected: boolean
+  chevron: null
+  trailingAction: 'new_worktree' | 'none'
+}
+
+export interface SidebarProjectSectionRowModel {
+  kind: 'project_section'
+  chevron: 'expand' | 'collapse'
+  trailingAction: 'new_worktree' | 'none'
+}
+
+export type SidebarProjectRowModel =
+  | SidebarProjectWorkspaceLinkRowModel
+  | SidebarProjectSectionRowModel
+
+export function isSidebarProjectFlattened(project: SidebarProjectEntry): boolean {
+  return project.workspaces.length === 1
 }

 export function buildSidebarProjectRowModel(input: {
@@ -17,10 +31,9 @@ export function buildSidebarProjectRowModel(input: {
  serverId?: string | null
  activeWorkspaceSelection?: { serverId: string; workspaceId: string } | null
 }): SidebarProjectRowModel {
-  const flattenedWorkspace =
-    input.project.projectKind === 'non_git' && input.project.workspaces.length === 1
-      ? input.project.workspaces[0] ?? null
-      : null
+  const flattenedWorkspace = isSidebarProjectFlattened(input.project)
+    ? (input.project.workspaces[0] ?? null)
+    : null
  const selected =
    flattenedWorkspace !== null &&
    Boolean(input.serverId) &&
@@ -29,19 +42,17 @@ export function buildSidebarProjectRowModel(input: {

  if (flattenedWorkspace) {
    return {
-      interaction: 'navigate',
-      chevron: 'disclosure',
-      trailingAction: 'none',
-      flattenedWorkspace,
+      kind: 'workspace_link',
+      workspace: flattenedWorkspace,
      selected,
+      chevron: null,
+      trailingAction: input.project.projectKind === 'git' ? 'new_worktree' : 'none',
    }
  }

  return {
-    interaction: 'toggle',
+    kind: 'project_section',
    chevron: input.collapsed ? 'expand' : 'collapse',
    trailingAction: input.project.projectKind === 'git' ? 'new_worktree' : 'none',
-    flattenedWorkspace: null,
-    selected: false,
  }
 }
@@ -34,7 +34,7 @@ describe("buildSidebarShortcutModel", () => {
  it("builds shortcut targets in visual order and excludes collapsed projects", () => {
    const projects = [
      project("p1", [workspace("s1", "/repo/main"), workspace("s1", "/repo/feat-a")]),
-      project("p2", [workspace("s1", "/repo2/main")]),
+      project("p2", [workspace("s1", "/repo2/main"), workspace("s1", "/repo2/feat-a")]),
    ];

    const model = buildSidebarShortcutModel({
@@ -70,4 +70,16 @@ describe("buildSidebarShortcutModel", () => {
    expect(model.shortcutTargets[0]).toEqual({ serverId: "s", workspaceId: "/repo/w1" });
    expect(model.shortcutTargets[8]).toEqual({ serverId: "s", workspaceId: "/repo/w9" });
  });
+
+  it("ignores collapsed state for flattened single-workspace projects", () => {
+    const projects = [project("p1", [workspace("s1", "/repo/main")])];
+
+    const model = buildSidebarShortcutModel({
+      projects,
+      collapsedProjectKeys: new Set<string>(["p1"]),
+    });
+
+    expect(model.visibleTargets).toEqual([{ serverId: "s1", workspaceId: "/repo/main" }]);
+    expect(model.shortcutTargets).toEqual([{ serverId: "s1", workspaceId: "/repo/main" }]);
+  });
 });
@@ -2,6 +2,7 @@ import type {
  SidebarProjectEntry,
  SidebarWorkspaceEntry,
 } from '@/hooks/use-sidebar-workspaces-list'
+import { isSidebarProjectFlattened } from './sidebar-project-row-model'

 export interface SidebarShortcutWorkspaceTarget {
  serverId: string
@@ -34,7 +35,7 @@ export function buildSidebarShortcutModel(input: {
  const shortcutIndexByWorkspaceKey = new Map<string, number>()

  for (const project of input.projects) {
-    if (input.collapsedProjectKeys.has(project.projectKey)) {
+    if (!isSidebarProjectFlattened(project) && input.collapsedProjectKeys.has(project.projectKey)) {
      continue
    }

@@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";

 const tauriMock = vi.hoisted(() => {
  let listenerCleanup: (() => void) | null = null;
+  let disconnectCount = 0;

  const connect = vi.fn(async () => {
    let listenerActive = false;
@@ -21,6 +22,7 @@ const tauriMock = vi.hoisted(() => {
      }),
      send: vi.fn(async () => undefined),
      disconnect: vi.fn(async () => {
+        disconnectCount += 1;
        listenerCleanup?.();
      }),
    };
@@ -29,6 +31,11 @@ const tauriMock = vi.hoisted(() => {
  return {
    connect,
    getListenerCleanup: () => listenerCleanup,
+    getDisconnectCount: () => disconnectCount,
+    resetState: () => {
+      listenerCleanup = null;
+      disconnectCount = 0;
+    },
  };
 });

@@ -43,6 +50,7 @@ vi.mock("./tauri", () => ({
 describe("tauri-daemon-transport", () => {
  beforeEach(() => {
    tauriMock.connect.mockClear();
+    tauriMock.resetState();
  });

  it("does not unregister the websocket listener twice during close", async () => {
@@ -54,6 +62,82 @@ describe("tauri-daemon-transport", () => {
    await Promise.resolve();

    expect(() => transport.close()).not.toThrow();
+    await Promise.resolve();
    expect(tauriMock.getListenerCleanup()).toHaveBeenCalledTimes(1);
  });
+
+  it("disconnects the websocket only once after the connection is already open", async () => {
+    const mod = await import("./tauri-daemon-transport");
+    const transportFactory = mod.createTauriWebSocketTransportFactory();
+    expect(transportFactory).not.toBeNull();
+
+    const transport = transportFactory!({ url: "ws://localhost:6767/ws" });
+    await Promise.resolve();
+    await Promise.resolve();
+
+    expect(() => transport.close()).not.toThrow();
+    await Promise.resolve();
+
+    expect(tauriMock.getDisconnectCount()).toBe(1);
+    expect(tauriMock.getListenerCleanup()).toHaveBeenCalledTimes(1);
+  });
+
+  it("disconnects only once when close happens during async connect cleanup", async () => {
+    let resolveConnect!: (socket: any) => void;
+    let resolveSend!: () => void;
+
+    tauriMock.connect.mockImplementationOnce(
+      () =>
+        new Promise((resolve) => {
+          resolveConnect = resolve;
+        })
+    );
+
+    const mod = await import("./tauri-daemon-transport");
+    const transportFactory = mod.createTauriWebSocketTransportFactory();
+    expect(transportFactory).not.toBeNull();
+
+    const disconnect = vi.fn(async () => undefined);
+    const addListener = vi.fn(() => vi.fn());
+    const send = vi.fn(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveSend = resolve;
+        })
+    );
+
+    const transport = transportFactory!({ url: "ws://localhost:6767/ws" });
+    transport.send("queued-before-connect");
+
+    resolveConnect({ addListener, send, disconnect });
+    await Promise.resolve();
+
+    transport.close();
+    resolveSend();
+    await Promise.resolve();
+    await Promise.resolve();
+
+    expect(disconnect).toHaveBeenCalledTimes(1);
+  });
+
+  it("swallows the tauri duplicate-listener cleanup error when disconnect throws synchronously", async () => {
+    tauriMock.connect.mockImplementationOnce(async () => ({
+      addListener: vi.fn(() => vi.fn()),
+      send: vi.fn(async () => undefined),
+      disconnect: vi.fn(() => {
+        throw new TypeError(
+          "undefined is not an object (evaluating 'listeners[eventId].handlerId')"
+        );
+      }),
+    }));
+
+    const mod = await import("./tauri-daemon-transport");
+    const transportFactory = mod.createTauriWebSocketTransportFactory();
+    expect(transportFactory).not.toBeNull();
+
+    const transport = transportFactory!({ url: "ws://localhost:6767/ws" });
+    await Promise.resolve();
+
+    expect(() => transport.close()).not.toThrow();
+  });
 });
@@ -18,6 +18,13 @@ type TauriWebSocketModule = {
  connect(url: string, config?: unknown): Promise<TauriWebSocketConnection>;
 };

+function isTauriDuplicateListenerCleanupError(error: unknown): boolean {
+  return (
+    error instanceof TypeError &&
+    error.message.includes("listeners[eventId].handlerId")
+  );
+}
+
 function toTauriOutgoingMessage(
  data: string | Uint8Array | ArrayBuffer
 ): string | number[] {
@@ -49,6 +56,7 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |
    let disposed = false;
    let opened = false;
    let closeEmitted = false;
+    let disconnectPromise: Promise<void> | null = null;

    const openHandlers = new Set<() => void>();
    const closeHandlers = new Set<(event?: unknown) => void>();
@@ -104,6 +112,21 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |
      }
    };

+    const disconnectSocket = (socket: TauriWebSocketConnection) => {
+      if (disconnectPromise) {
+        return disconnectPromise;
+      }
+      disconnectPromise = Promise.resolve()
+        .then(() => socket.disconnect())
+        .catch((error) => {
+          if (!isTauriDuplicateListenerCleanupError(error)) {
+            emitError(error);
+          }
+        })
+        .then(() => undefined);
+      return disconnectPromise;
+    };
+
    const connect = async () => {
      try {
        let module: TauriWebSocketModule | null = getTauriWebSocketModule();
@@ -122,11 +145,7 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |

        const socket = await module.connect(url);
        if (disposed) {
-          try {
-            await socket.disconnect();
-          } catch {
-            // no-op
-          }
+          await disconnectSocket(socket);
          return;
        }

@@ -160,13 +179,8 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |
        }

        if (closeRequested) {
-          try {
-            await ws.disconnect();
-          } catch (error) {
-            emitError(error);
-          } finally {
-            emitClose(closeRequested);
-          }
+          await disconnectSocket(ws);
+          emitClose(closeRequested);
        }
      } catch (error) {
        emitError(error);
@@ -189,10 +203,7 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |
        if (disposed) return;
        closeRequested = { code, reason };
        if (!ws) return;
-        void ws
-          .disconnect()
-          .catch((error) => emitError(error))
-          .finally(() => emitClose(closeRequested));
+        void disconnectSocket(ws).finally(() => emitClose(closeRequested));
      },
      onMessage: (handler) => {
        messageHandlers.add(handler);
@@ -230,7 +241,13 @@ export function createTauriWebSocketTransportFactory(): DaemonTransportFactory |
      ...transport,
      close: (code?: number, reason?: string) => {
        wsListenerCleanup = null;
-        transport.close(code, reason);
+        try {
+          transport.close(code, reason);
+        } catch (error) {
+          if (!isTauriDuplicateListenerCleanupError(error)) {
+            throw error;
+          }
+        }
        disposed = true;
        ws = null;
      },
@@ -57,4 +57,26 @@ describe("tool-call detail state", () => {
      false
    );
  });
+
+  it("treats enriched search detail as meaningful", () => {
+    const detail: ToolCallDetail = {
+      type: "search",
+      query: "needle",
+      toolName: "grep",
+      content: "12:needle",
+      filePaths: ["src/index.ts"],
+    };
+
+    assert.strictEqual(hasMeaningfulToolCallDetail(detail), true);
+  });
+
+  it("treats fetch detail as meaningful", () => {
+    const detail: ToolCallDetail = {
+      type: "fetch",
+      url: "https://example.com",
+      result: "Fetched summary",
+    };
+
+    assert.strictEqual(hasMeaningfulToolCallDetail(detail), true);
+  });
 });
@@ -38,7 +38,15 @@ export function hasMeaningfulToolCallDetail(
    case "write":
      return Boolean(detail.filePath || detail.content);
    case "search":
-      return detail.query.trim().length > 0;
+      return Boolean(
+        detail.query.trim().length > 0 ||
+          detail.content ||
+          (detail.filePaths && detail.filePaths.length > 0) ||
+          (detail.webResults && detail.webResults.length > 0) ||
+          (detail.annotations && detail.annotations.length > 0)
+      );
+    case "fetch":
+      return Boolean(detail.url || detail.result || detail.codeText);
    case "worktree_setup":
      return Boolean(detail.branchName || detail.worktreePath || detail.log);
    case "sub_agent":
@@ -20,6 +20,7 @@ const TOOL_DETAIL_ICONS: Record<ToolCallDetail["type"], ToolCallIconComponent> =
  edit: Pencil,
  write: Pencil,
  search: Search,
+  fetch: Search,
  worktree_setup: SquareTerminal,
  sub_agent: Bot,
  plain_text: Wrench,