fix(sidebar): centralize tree action handling

apps/desktop/src/lib/__tests__/app/pinnedItems.spec.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { updatePinnedTreeNodeInPlace } from "@/lib/app/pinnedItems";
+import { migrateLegacyPinnedTreeNodeIds, syncPinnedTreeNodeStateInPlace, treeNodePinKey, updatePinnedTreeNodeInPlace } from "@/lib/app/pinnedItems";
 import { buildTreeNodesFromLayout } from "@/lib/sidebar/sidebarLayout";
 import type { ConnectionConfig, SidebarLayout, TreeNode } from "@/types/database";
 
@@ -17,7 +17,7 @@ describe("sidebar pinned tree nodes", () => {
       },
     ];
 
-    expect(updatePinnedTreeNodeInPlace(tree, "conn:db:b", true)).toBe("siblings");
+    expect(updatePinnedTreeNodeInPlace(tree, tree[0].children![1], true)).toBe("siblings");
 
     expect(tree[0].children?.map((node) => node.id)).toEqual(["conn:db:b", "conn:db:a"]);
     expect(tree[0].children?.[0].pinned).toBe(true);
@@ -29,12 +29,57 @@ describe("sidebar pinned tree nodes", () => {
       { id: "group-b", label: "B", type: "connection-group" },
     ];
 
-    expect(updatePinnedTreeNodeInPlace(tree, "group-b", true)).toBe("root");
+    expect(updatePinnedTreeNodeInPlace(tree, tree[1], true)).toBe("root");
 
     expect(tree.map((node) => node.id)).toEqual(["group-b", "group-a"]);
     expect(tree[0].pinned).toBe(true);
   });
 
+  it("scopes duplicate node ids by database when pinning", () => {
+    const databaseA: TreeNode = {
+      id: "conn:a",
+      label: "A",
+      type: "database",
+      children: [{ id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "a" }],
+    };
+    const databaseB: TreeNode = {
+      id: "conn:b",
+      label: "B",
+      type: "database",
+      children: [{ id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "b" }],
+    };
+    const tree: TreeNode[] = [{ id: "conn", label: "Connection", type: "connection", children: [databaseA, databaseB] }];
+
+    expect(updatePinnedTreeNodeInPlace(tree, databaseA.children![0], true)).toBe("siblings");
+
+    expect(databaseA.children![0].pinned).toBe(true);
+    expect(databaseB.children![0].pinned).not.toBe(true);
+  });
+
+  it("clears stale legacy duplicate pins after switching to scoped keys", () => {
+    const tableA: TreeNode = { id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "a", pinned: true };
+    const tableB: TreeNode = { id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "b", pinned: true };
+    const tree: TreeNode[] = [
+      { id: "conn:a", label: "A", type: "database", children: [tableA] },
+      { id: "conn:b", label: "B", type: "database", children: [tableB] },
+    ];
+
+    syncPinnedTreeNodeStateInPlace(tree, new Set([treeNodePinKey(tableA)]));
+
+    expect(tableA.pinned).toBe(true);
+    expect(tableB.pinned).toBe(false);
+  });
+
+  it("migrates a legacy id once instead of pinning every colliding node", () => {
+    const tableA: TreeNode = { id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "a" };
+    const tableB: TreeNode = { id: "duplicate-table-id", label: "users", type: "table", connectionId: "conn", database: "b" };
+
+    const migrated = migrateLegacyPinnedTreeNodeIds([tableA, tableB], new Set(["duplicate-table-id"]));
+
+    expect(migrated.changed).toBe(true);
+    expect(migrated.ids).toEqual(new Set([treeNodePinKey(tableA)]));
+  });
+
   it("applies pinned state to connection groups when rebuilding from layout", () => {
     const layout: SidebarLayout = {
       groups: [

apps/desktop/src/lib/__tests__/sidebar/sidebarActionTarget.spec.ts

@@ -0,0 +1,62 @@
+import { describe, expect, it } from "vitest";
+import type { TreeNode } from "@/types/database";
+import { createSidebarActionRequest, createSidebarActionTarget, findSidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget";
+
+function tableNode(): TreeNode {
+  return {
+    id: "table-1",
+    label: "users",
+    type: "table",
+    connectionId: "connection-1",
+    database: "app",
+    schema: "public",
+    children: [{ id: "column-1", label: "id", type: "column" }],
+  };
+}
+
+describe("sidebar action targets", () => {
+  it("captures a frozen shallow snapshot without retaining children", () => {
+    const node = tableNode();
+    const target = createSidebarActionTarget(node);
+
+    node.label = "recycled-row";
+    node.connectionId = "connection-2";
+
+    expect(target).toMatchObject({ label: "users", connectionId: "connection-1", children: undefined, hiddenChildren: undefined });
+    expect(Object.isFrozen(target)).toBe(true);
+  });
+
+  it("captures selection independently from later selection changes", () => {
+    const selectedNodeIds = ["table-1", "table-2"];
+    const request = createSidebarActionRequest(tableNode(), selectedNodeIds, { initialEditing: true });
+
+    selectedNodeIds.splice(0, selectedNodeIds.length, "table-3");
+
+    expect(request.selectedNodeIds).toEqual(["table-1", "table-2"]);
+    expect(request.payload).toEqual({ initialEditing: true });
+    expect(Object.isFrozen(request.selectedNodeIds)).toBe(true);
+    expect(Object.isFrozen(request)).toBe(true);
+  });
+
+  it("does not resolve a recycled row with the same local id", () => {
+    const target = createSidebarActionTarget(tableNode());
+    const recycled = tableNode();
+    recycled.connectionId = "connection-2";
+
+    expect(findSidebarActionTarget([recycled], target)).toBeNull();
+  });
+
+  it("returns null when the action target was removed", () => {
+    const target = createSidebarActionTarget(tableNode());
+
+    expect(findSidebarActionTarget([], target)).toBeNull();
+  });
+
+  it("resolves targets stored only in hidden children", () => {
+    const node = tableNode();
+    const target = createSidebarActionTarget(node);
+    const root: TreeNode = { id: "root", label: "root", type: "database", hiddenChildren: [node] };
+
+    expect(findSidebarActionTarget([root], target)).toBe(node);
+  });
+});

apps/desktop/src/lib/__tests__/sidebar/sidebarDataOpenCoordinator.spec.ts

@@ -0,0 +1,95 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { cancelPendingSidebarDataOpen, runSidebarDataOpenImmediately } from "@/lib/sidebar/sidebarDataOpenCoordinator";
+
+describe("sidebarDataOpenCoordinator", () => {
+  afterEach(() => {
+    cancelPendingSidebarDataOpen();
+  });
+
+  it("cancels the active request when a newer open starts", async () => {
+    const cancel = vi.fn();
+    let firstIsCurrent = true;
+
+    runSidebarDataOpenImmediately(async (request) => {
+      request.registerCancel(cancel);
+      await Promise.resolve();
+      firstIsCurrent = request.isCurrent();
+    });
+    runSidebarDataOpenImmediately(() => undefined);
+    await Promise.resolve();
+
+    expect(cancel).toHaveBeenCalledOnce();
+    expect(firstIsCurrent).toBe(false);
+  });
+
+  it("runs registered cancellation immediately for an already stale request", async () => {
+    const cancel = vi.fn();
+    let registerLateCancel: (() => void) | undefined;
+
+    runSidebarDataOpenImmediately((request) => {
+      registerLateCancel = () => request.registerCancel(cancel);
+    });
+    runSidebarDataOpenImmediately(() => undefined);
+    registerLateCancel?.();
+    await Promise.resolve();
+
+    expect(cancel).toHaveBeenCalledOnce();
+  });
+
+  it("recovers after a runner throws synchronously", async () => {
+    const nextRunner = vi.fn();
+
+    runSidebarDataOpenImmediately(() => {
+      throw new Error("boom");
+    });
+    await Promise.resolve();
+    runSidebarDataOpenImmediately(nextRunner);
+    await Promise.resolve();
+
+    expect(nextRunner).toHaveBeenCalledOnce();
+  });
+
+  it("keeps only the newest rapid navigation request current", async () => {
+    const states: boolean[] = [];
+    const captures: Array<() => void> = [];
+
+    for (let index = 0; index < 3; index += 1) {
+      runSidebarDataOpenImmediately((request) => {
+        captures.push(() => states.push(request.isCurrent()));
+      });
+    }
+    captures.forEach((capture) => capture());
+
+    expect(states).toEqual([false, false, true]);
+  });
+
+  it("does not cancel work that was not registered as sidebar-owned", async () => {
+    const sidebarCancel = vi.fn();
+    const unrelatedEditorCancel = vi.fn();
+
+    runSidebarDataOpenImmediately((request) => request.registerCancel(sidebarCancel));
+    runSidebarDataOpenImmediately(() => undefined);
+    await Promise.resolve();
+
+    expect(sidebarCancel).toHaveBeenCalledOnce();
+    expect(unrelatedEditorCancel).not.toHaveBeenCalled();
+  });
+
+  it("prevents a superseded connection completion from becoming current", async () => {
+    let finishConnection: (() => void) | undefined;
+    let completionWasCurrent = true;
+
+    runSidebarDataOpenImmediately(async (request) => {
+      await new Promise<void>((resolve) => {
+        finishConnection = resolve;
+      });
+      completionWasCurrent = request.isCurrent();
+    });
+    runSidebarDataOpenImmediately(() => undefined);
+    finishConnection?.();
+    await Promise.resolve();
+    await Promise.resolve();
+
+    expect(completionWasCurrent).toBe(false);
+  });
+});

apps/desktop/src/lib/app/pinnedItems.ts

@@ -2,6 +2,33 @@ import type { TreeNode } from "@/types/database";
 
 export type PinnedTreeNodeUpdateScope = "missing" | "root" | "siblings";
 
+export function treeNodePinKey(node: TreeNode): string {
+  if (!node.connectionId) return node.id;
+  const identity = [node.database || "", node.schema || "", node.catalog || "", node.type, node.objectName || node.tableName || node.label, node.signature || "", node.id];
+  return `${node.connectionId}:pin:v2:${encodeURIComponent(JSON.stringify(identity))}`;
+}
+
+export function migrateLegacyPinnedTreeNodeIds(nodes: readonly TreeNode[], pinnedIds: Set<string>): { ids: Set<string>; changed: boolean } {
+  const next = new Set(pinnedIds);
+  let changed = false;
+  const visit = (items: readonly TreeNode[]) => {
+    for (const node of items) {
+      const pinKey = treeNodePinKey(node);
+      if (pinKey !== node.id && next.has(node.id) && !next.has(pinKey)) {
+        // A legacy id cannot distinguish colliding nodes. Claim it once for the
+        // first matching loaded node, then persist the fully scoped identity.
+        next.delete(node.id);
+        next.add(pinKey);
+        changed = true;
+      }
+      if (node.children) visit(node.children);
+      if (node.hiddenChildren) visit(node.hiddenChildren);
+    }
+  };
+  visit(nodes);
+  return { ids: next, changed };
+}
+
 export function orderPinnedFirst<T>(items: T[], isPinned: (item: T) => boolean): T[] {
   const pinned: T[] = [];
   const unpinned: T[] = [];
@@ -14,19 +41,20 @@ export function orderPinnedFirst<T>(items: T[], isPinned: (item: T) => boolean):
   return [...pinned, ...unpinned];
 }
 
-function findTreeNodeLocation(nodes: TreeNode[], id: string, parent: TreeNode | null = null): { node: TreeNode; parent: TreeNode | null } | null {
+function findTreeNodeLocation(nodes: TreeNode[], target: TreeNode, parent: TreeNode | null = null): { node: TreeNode; parent: TreeNode | null } | null {
+  const targetKey = treeNodePinKey(target);
   for (const node of nodes) {
-    if (node.id === id) return { node, parent };
+    if (node === target || treeNodePinKey(node) === targetKey) return { node, parent };
     if (node.children) {
-      const found = findTreeNodeLocation(node.children, id, node);
+      const found = findTreeNodeLocation(node.children, target, node);
       if (found) return found;
     }
   }
   return null;
 }
 
-export function updatePinnedTreeNodeInPlace(nodes: TreeNode[], id: string, pinned: boolean): PinnedTreeNodeUpdateScope {
-  const location = findTreeNodeLocation(nodes, id);
+export function updatePinnedTreeNodeInPlace(nodes: TreeNode[], target: TreeNode, pinned: boolean): PinnedTreeNodeUpdateScope {
+  const location = findTreeNodeLocation(nodes, target);
   if (!location) return "missing";
 
   location.node.pinned = pinned;
@@ -42,13 +70,47 @@ export function updatePinnedTreeNodeInPlace(nodes: TreeNode[], id: string, pinne
   return "root";
 }
 
-export function applyPinnedTreeNodeState(nodes: TreeNode[], pinnedIds: Set<string>): TreeNode[] {
+function clonePinnedTreeNode(node: TreeNode, pinnedIds: Set<string>, clones: WeakMap<TreeNode, TreeNode>): TreeNode {
+  const existing = clones.get(node);
+  if (existing) return existing;
+  const clone: TreeNode = {
+    ...node,
+    pinned: pinnedIds.has(treeNodePinKey(node)) || pinnedIds.has(node.id),
+  };
+  clones.set(node, clone);
+  if (node.children) clone.children = applyPinnedTreeNodeStateInternal(node.children, pinnedIds, clones);
+  if (node.hiddenChildren) clone.hiddenChildren = applyPinnedTreeNodeStateInternal(node.hiddenChildren, pinnedIds, clones);
+  return clone;
+}
+
+function applyPinnedTreeNodeStateInternal(nodes: TreeNode[], pinnedIds: Set<string>, clones: WeakMap<TreeNode, TreeNode>): TreeNode[] {
   return orderPinnedFirst(
-    nodes.map((node) => ({
-      ...node,
-      pinned: pinnedIds.has(node.id),
-      children: node.children ? applyPinnedTreeNodeState(node.children, pinnedIds) : node.children,
-    })),
+    nodes.map((node) => clonePinnedTreeNode(node, pinnedIds, clones)),
     (node) => !!node.pinned,
   );
 }
+
+export function applyPinnedTreeNodeState(nodes: TreeNode[], pinnedIds: Set<string>): TreeNode[] {
+  return applyPinnedTreeNodeStateInternal(nodes, pinnedIds, new WeakMap());
+}
+
+function syncPinnedTreeNodeStateInPlaceInternal(nodes: TreeNode[], pinnedIds: Set<string>, visited: WeakSet<TreeNode>): void {
+  for (const node of nodes) {
+    if (visited.has(node)) continue;
+    visited.add(node);
+    node.pinned = pinnedIds.has(treeNodePinKey(node)) || pinnedIds.has(node.id);
+    if (node.children) {
+      syncPinnedTreeNodeStateInPlaceInternal(node.children, pinnedIds, visited);
+      node.children = orderPinnedFirst(node.children, (child) => !!child.pinned);
+    }
+    if (node.hiddenChildren) {
+      syncPinnedTreeNodeStateInPlaceInternal(node.hiddenChildren, pinnedIds, visited);
+      node.hiddenChildren = orderPinnedFirst(node.hiddenChildren, (child) => !!child.pinned);
+    }
+  }
+  nodes.splice(0, nodes.length, ...orderPinnedFirst(nodes, (node) => !!node.pinned));
+}
+
+export function syncPinnedTreeNodeStateInPlace(nodes: TreeNode[], pinnedIds: Set<string>): void {
+  syncPinnedTreeNodeStateInPlaceInternal(nodes, pinnedIds, new WeakSet());
+}

apps/desktop/src/lib/sidebar/sidebarActionTarget.ts

@@ -0,0 +1,45 @@
+import type { TreeNode } from "@/types/database";
+
+export type SidebarActionTarget = Readonly<TreeNode>;
+
+export interface SidebarActionRequest<TPayload = undefined> {
+  target: SidebarActionTarget;
+  selectedNodeIds: readonly string[];
+  payload: TPayload;
+}
+
+function sameActionTarget(left: TreeNode, right: SidebarActionTarget): boolean {
+  return left.id === right.id && left.type === right.type && left.connectionId === right.connectionId && left.database === right.database && left.schema === right.schema && left.catalog === right.catalog && left.label === right.label && left.signature === right.signature;
+}
+
+export function findSidebarActionTarget(nodes: readonly TreeNode[], target: SidebarActionTarget): TreeNode | null {
+  const visited = new WeakSet<TreeNode>();
+  const find = (items: readonly TreeNode[]): TreeNode | null => {
+    for (const node of items) {
+      if (visited.has(node)) continue;
+      visited.add(node);
+      if (sameActionTarget(node, target)) return node;
+      const child = node.children ? find(node.children) : null;
+      if (child) return child;
+      const hiddenChild = node.hiddenChildren ? find(node.hiddenChildren) : null;
+      if (hiddenChild) return hiddenChild;
+    }
+    return null;
+  };
+  return find(nodes);
+}
+
+export function createSidebarActionTarget(node: TreeNode): SidebarActionTarget {
+  // Virtual rows can be recycled while an async dialog is opening, so actions
+  // must never retain the mutable row node or its potentially large child tree.
+  const meta = node.meta && typeof node.meta === "object" ? Object.freeze({ ...node.meta }) : node.meta;
+  return Object.freeze({ ...node, children: undefined, hiddenChildren: undefined, meta });
+}
+
+export function createSidebarActionRequest<TPayload>(node: TreeNode, selectedNodeIds: readonly string[], payload: TPayload): SidebarActionRequest<TPayload> {
+  return Object.freeze({
+    target: createSidebarActionTarget(node),
+    selectedNodeIds: Object.freeze([...selectedNodeIds]),
+    payload,
+  });
+}

apps/desktop/src/lib/sidebar/sidebarDangerDialog.ts

@@ -0,0 +1,22 @@
+import type { SidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget";
+
+export interface SidebarDangerDialogOption {
+  checked: boolean;
+  label: string;
+  hint: string;
+  onChange?: (checked: boolean) => void | Promise<void>;
+}
+
+export interface SidebarDangerDialogRequest {
+  target: SidebarActionTarget;
+  title: string;
+  message: string;
+  confirmLabel: string;
+  sql?: string;
+  details?: string;
+  detailsText?: string;
+  loading?: boolean;
+  closeOnConfirm?: boolean;
+  option?: SidebarDangerDialogOption;
+  confirm: () => void | Promise<void>;
+}

apps/desktop/src/lib/sidebar/sidebarDataOpenCoordinator.ts

@@ -0,0 +1,60 @@
+export interface SidebarDataOpenRequest {
+  isCurrent: () => boolean;
+  registerCancel: (cancel: () => void | Promise<void>) => void;
+}
+
+type OpenDataRunner = (request: SidebarDataOpenRequest) => void | Promise<void>;
+
+let generation = 0;
+let activeGeneration = 0;
+let activeCancel: (() => void | Promise<void>) | null = null;
+
+function runCancellation(cancel: (() => void | Promise<void>) | null) {
+  if (!cancel) return;
+  void Promise.resolve(cancel()).catch(() => undefined);
+}
+
+function supersedeCurrentRequest(): number {
+  generation += 1;
+  runCancellation(activeCancel);
+  activeCancel = null;
+  activeGeneration = 0;
+  return generation;
+}
+
+function executeRequest(requestGeneration: number, runner: OpenDataRunner) {
+  if (requestGeneration !== generation) return;
+  const request: SidebarDataOpenRequest = {
+    isCurrent: () => requestGeneration === generation,
+    registerCancel: (cancel) => {
+      if (requestGeneration !== generation) {
+        runCancellation(cancel);
+        return;
+      }
+      activeGeneration = requestGeneration;
+      activeCancel = cancel;
+    },
+  };
+  let result: void | Promise<void>;
+  try {
+    result = runner(request);
+  } catch {
+    result = undefined;
+  }
+  void Promise.resolve(result)
+    .catch(() => undefined)
+    .finally(() => {
+      if (activeGeneration !== requestGeneration) return;
+      activeGeneration = 0;
+      activeCancel = null;
+    });
+}
+
+export function runSidebarDataOpenImmediately(runner: OpenDataRunner) {
+  const requestGeneration = supersedeCurrentRequest();
+  executeRequest(requestGeneration, runner);
+}
+
+export function cancelPendingSidebarDataOpen() {
+  supersedeCurrentRequest();
+}

apps/desktop/src/lib/sidebar/sidebarDatabaseOpenState.ts

@@ -4,6 +4,6 @@ export function isSidebarDatabaseOpened(node: TreeNode, isTreeNodeChildrenLoaded
   return (node.type === "database" || node.type === "mongo-db" || node.type === "vector-database") && !!node.connectionId && node.database != null && isTreeNodeChildrenLoaded(node.id);
 }
 
-export function canCloseSidebarDatabaseConnection(node: TreeNode, isTreeNodeChildrenLoaded: (nodeId: string) => boolean): boolean {
-  return node.type === "database" && !!node.connectionId && node.database != null && isTreeNodeChildrenLoaded(node.id);
+export function canCloseSidebarDatabaseConnection(node: TreeNode, isTreeNodeChildrenLoaded: (nodeId: string) => boolean, isDatabaseUsedByOpenTab: (connectionId: string, database: string) => boolean = () => false): boolean {
+  return node.type === "database" && !!node.connectionId && node.database != null && (isTreeNodeChildrenLoaded(node.id) || isDatabaseUsedByOpenTab(node.connectionId, node.database));
 }

apps/desktop/src/lib/tabs/dataTabActivation.ts

@@ -4,8 +4,8 @@ function isErrorResult(result: QueryResult | undefined): boolean {
   return result?.columns.length === 1 && result.columns[0] === "Error";
 }
 
-export function canActivateExistingDataTableTab(tab: QueryTab): boolean {
-  if (tab.isExecuting) return true;
+export function canActivateExistingDataTableTab(tab: QueryTab, options: { activateExecuting?: boolean } = {}): boolean {
+  if (tab.isExecuting) return options.activateExecuting !== false;
   if (isErrorResult(tab.result)) return false;
   return !!tab.result || !!tab.results?.length;
 }