fix: canonicalize workspace tab routes and prevent orphan runs

2026-03-04 17:39:19 +07:00
parent 7578846c27
commit 26ea214d64
16 changed files with 723 additions and 88 deletions
@@ -18,6 +18,8 @@ type UseWorkspaceTabLayoutInput = {
    tabHorizontalPadding: number;
    estimatedCharWidth: number;
    closeButtonWidth: number;
+    compactLabelCharCap?: number;
+    compactDenseLabelCharCap?: number;
  };
 };

@@ -86,6 +86,8 @@ export function WorkspaceDesktopTabsRow({
      tabHorizontalPadding: theme.spacing[3],
      estimatedCharWidth: 7,
      closeButtonWidth: 22,
+      compactLabelCharCap: 10,
+      compactDenseLabelCharCap: 8,
    }),
    [theme.spacing]
  );
@@ -190,6 +192,9 @@ export function WorkspaceDesktopTabsRow({
                  onHoverOut={() => {
                    setHoveredTabKey((current) => (current === tab.key ? null : current));
                  }}
+                  onPressIn={() => {
+                    onNavigateTab(tab.tabId);
+                  }}
                  onPress={() => {
                    onNavigateTab(tab.tabId);
                  }}
@@ -0,0 +1,89 @@
+import { describe, expect, it } from "vitest";
+import {
+  buildHostWorkspaceRoute,
+  buildHostWorkspaceTabRoute,
+} from "@/utils/host-routes";
+import { resolveWorkspaceTabCanonicalPath } from "@/screens/workspace/workspace-route-canonicalization";
+
+describe("resolveWorkspaceTabCanonicalPath", () => {
+  it("canonicalizes workspace root routes to /tab/<tabId> when a tab is active", () => {
+    const serverId = "local";
+    const workspaceId = "/tmp/repo";
+
+    expect(
+      resolveWorkspaceTabCanonicalPath({
+        activeTabId: "draft_abc123",
+        pathname: buildHostWorkspaceRoute(serverId, workspaceId),
+        serverId,
+        workspaceId,
+      })
+    ).toBe(buildHostWorkspaceTabRoute(serverId, workspaceId, "draft_abc123"));
+  });
+
+  it("does not skip canonical path detection due to query-string tabId", () => {
+    const serverId = "local";
+    const workspaceId = "/tmp/repo";
+
+    expect(
+      resolveWorkspaceTabCanonicalPath({
+        activeTabId: "draft_abc123",
+        pathname: `${buildHostWorkspaceRoute(serverId, workspaceId)}?tabId=draft_abc123`,
+        serverId,
+        workspaceId,
+      })
+    ).toBe(buildHostWorkspaceTabRoute(serverId, workspaceId, "draft_abc123"));
+  });
+
+  it("strips redundant query params from canonical /tab routes", () => {
+    const serverId = "local";
+    const workspaceId = "/tmp/repo";
+    const canonicalPath = buildHostWorkspaceTabRoute(
+      serverId,
+      workspaceId,
+      "draft_abc123"
+    );
+
+    expect(
+      resolveWorkspaceTabCanonicalPath({
+        activeTabId: "draft_abc123",
+        pathname: `${canonicalPath}?tabId=not_used`,
+        serverId,
+        workspaceId,
+      })
+    ).toBe(canonicalPath);
+  });
+
+  it("does nothing when the pathname is exactly canonical", () => {
+    const serverId = "local";
+    const workspaceId = "/tmp/repo";
+    const canonicalPath = buildHostWorkspaceTabRoute(
+      serverId,
+      workspaceId,
+      "draft_abc123"
+    );
+
+    expect(
+      resolveWorkspaceTabCanonicalPath({
+        activeTabId: "draft_abc123",
+        pathname: canonicalPath,
+        serverId,
+        workspaceId,
+      })
+    ).toBeNull();
+  });
+
+  it("does not canonicalize when the pathname belongs to another workspace", () => {
+    const serverId = "local";
+    const workspaceId = "/tmp/repo";
+    const otherWorkspacePath = buildHostWorkspaceRoute(serverId, "/tmp/other");
+
+    expect(
+      resolveWorkspaceTabCanonicalPath({
+        activeTabId: "draft_abc123",
+        pathname: otherWorkspacePath,
+        serverId,
+        workspaceId,
+      })
+    ).toBeNull();
+  });
+});
@@ -0,0 +1,55 @@
+import {
+  buildHostWorkspaceTabRoute,
+  parseHostWorkspaceRouteFromPathname,
+} from "@/utils/host-routes";
+
+function stripSearchAndHash(pathname: string): string {
+  const hashIndex = pathname.indexOf("#");
+  const queryIndex = pathname.indexOf("?");
+  const end = [hashIndex, queryIndex]
+    .filter((index) => index >= 0)
+    .reduce((min, index) => Math.min(min, index), pathname.length);
+  return pathname.slice(0, end);
+}
+
+type ResolveWorkspaceTabCanonicalPathInput = {
+  activeTabId: string | null;
+  pathname: string;
+  serverId: string;
+  workspaceId: string;
+};
+
+export function resolveWorkspaceTabCanonicalPath(
+  input: ResolveWorkspaceTabCanonicalPathInput
+): string | null {
+  const { activeTabId, pathname, serverId, workspaceId } = input;
+  if (!activeTabId || !serverId || !workspaceId) {
+    return null;
+  }
+
+  const canonicalPath = buildHostWorkspaceTabRoute(serverId, workspaceId, activeTabId);
+  if (canonicalPath === "/") {
+    return null;
+  }
+
+  const activeWorkspaceRoute = parseHostWorkspaceRouteFromPathname(pathname);
+  if (
+    !activeWorkspaceRoute ||
+    activeWorkspaceRoute.serverId !== serverId ||
+    activeWorkspaceRoute.workspaceId !== workspaceId
+  ) {
+    return null;
+  }
+
+  const pathOnly = stripSearchAndHash(pathname);
+  if (pathOnly !== canonicalPath) {
+    return canonicalPath;
+  }
+
+  // Keep workspace tab URLs clean: no query/hash source-of-truth for tab routing.
+  if (pathname !== canonicalPath) {
+    return canonicalPath;
+  }
+
+  return null;
+}
@@ -8,7 +8,7 @@ import {
  View,
 } from "react-native";
 import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
-import { useRouter } from "expo-router";
+import { usePathname, useRouter } from "expo-router";
 import * as Clipboard from "expo-clipboard";
 import {
  Bot,
@@ -50,9 +50,9 @@ import {
  useWorkspaceTabsStore,
 } from "@/stores/workspace-tabs-store";
 import {
-  buildHostWorkspaceRoute,
  buildHostWorkspaceTabRoute,
  decodeWorkspaceIdFromPathSegment,
+  encodeWorkspaceIdForPathSegment,
 } from "@/utils/host-routes";
 import { normalizeWorkspaceIdentity } from "@/utils/workspace-identity";
 import { useHostRuntimeSession } from "@/runtime/host-runtime";
@@ -82,6 +82,7 @@ import {
 import {
  deriveWorkspaceTabModel,
 } from "@/screens/workspace/workspace-tab-model";
+import { resolveWorkspaceTabCanonicalPath } from "@/screens/workspace/workspace-route-canonicalization";

 const TERMINALS_QUERY_STALE_TIME = 5_000;
 const NEW_TAB_AGENT_OPTION_ID = "__new_tab_agent__";
@@ -111,6 +112,26 @@ function decodeSegment(value: string): string {
  }
 }

+function buildWorkspaceTabHref(
+  serverId: string,
+  workspaceId: string,
+  tabId: string
+): { pathname: string; params: { serverId: string; workspaceId: string; tabId: string } } | null {
+  const encodedWorkspaceId = encodeWorkspaceIdForPathSegment(workspaceId);
+  const trimmedTabId = tabId.trim();
+  const trimmedServerId = serverId.trim();
+  if (!encodedWorkspaceId || !trimmedTabId || !trimmedServerId) {
+    return null;
+  }
+  return {
+    pathname: "/h/[serverId]/workspace/[workspaceId]/tab/[tabId]",
+    params: {
+      serverId: trimmedServerId,
+      workspaceId: encodedWorkspaceId,
+      tabId: trimmedTabId,
+    },
+  };
+}

 export function WorkspaceScreen({
  serverId,
@@ -136,6 +157,11 @@ function WorkspaceScreenContent({
  const { theme } = useUnistyles();
  const toast = useToast();
  const router = useRouter();
+  const routePathname = usePathname();
+  const pathname =
+    Platform.OS === "web" && typeof window !== "undefined"
+      ? `${window.location.pathname}${window.location.search}${window.location.hash}`
+      : routePathname;
  const isMobile =
    UnistylesRuntime.breakpoint === "xs" || UnistylesRuntime.breakpoint === "sm";

@@ -215,13 +241,15 @@ function WorkspaceScreenContent({
            target: { kind: "terminal", terminalId: createdTerminal.id },
          });
        if (tabId) {
-          router.replace(
-            buildHostWorkspaceTabRoute(
-              normalizedServerId,
-              normalizedWorkspaceId,
-              tabId
-            ) as any
+          const href = buildWorkspaceTabHref(
+            normalizedServerId,
+            normalizedWorkspaceId,
+            tabId
          );
+          if (!href) {
+            return;
+          }
+          router.replace(href as any);
        }
      }
    },
@@ -413,6 +441,7 @@ function WorkspaceScreenContent({
  const closeWorkspaceTab = useWorkspaceTabsStore((state) => state.closeTab);
  const promoteDraftToAgent = useWorkspaceTabsStore((state) => state.promoteDraftToAgent);
  const reorderWorkspaceTabs = useWorkspaceTabsStore((state) => state.reorderTabs);
+  const [pendingRouteTabId, setPendingRouteTabId] = useState<string | null>(null);

  useEffect(() => {
    const normalized = typeof routeTabId === "string" ? routeTabId.trim() : "";
@@ -447,6 +476,22 @@ function WorkspaceScreenContent({
    routeTabId,
  ]);

+  const normalizedRouteTabId = trimNonEmpty(routeTabId);
+  const effectiveRouteTabId = pendingRouteTabId ?? normalizedRouteTabId;
+
+  useEffect(() => {
+    if (!pendingRouteTabId || !normalizedRouteTabId) {
+      return;
+    }
+    if (normalizedRouteTabId === pendingRouteTabId) {
+      setPendingRouteTabId(null);
+    }
+  }, [normalizedRouteTabId, pendingRouteTabId]);
+
+  useEffect(() => {
+    setPendingRouteTabId(null);
+  }, [normalizedServerId, normalizedWorkspaceId]);
+
  const tabModel = useMemo(
    () =>
      deriveWorkspaceTabModel({
@@ -455,9 +500,9 @@ function WorkspaceScreenContent({
        uiTabs,
        tabOrder,
        focusedTabId,
-        routeTabId,
+        routeTabId: effectiveRouteTabId,
      }),
-    [focusedTabId, routeTabId, tabOrder, terminals, uiTabs, workspaceAgents]
+    [effectiveRouteTabId, focusedTabId, tabOrder, terminals, uiTabs, workspaceAgents]
  );
  const activeTabId = tabModel.activeTabId;

@@ -468,23 +513,76 @@ function WorkspaceScreenContent({
    focusTab({ serverId: normalizedServerId, workspaceId: normalizedWorkspaceId, tabId: activeTabId });
  }, [activeTabId, focusTab, normalizedServerId, normalizedWorkspaceId, persistenceKey]);

-  const lastCanonicalTabIdRef = useRef<string | null>(null);
  useEffect(() => {
-    if (!activeTabId || !normalizedServerId || !normalizedWorkspaceId) {
+    if (!pendingRouteTabId || !activeTabId) {
      return;
    }
-    if (routeTabId && routeTabId.trim() === activeTabId) {
-      lastCanonicalTabIdRef.current = activeTabId;
+    if (pendingRouteTabId === activeTabId) {
+      setPendingRouteTabId(null);
+    }
+  }, [activeTabId, pendingRouteTabId]);
+
+  const lastCanonicalPathRef = useRef<string | null>(null);
+  useEffect(() => {
+    const hasRouteIdentitySearchParams =
+      Platform.OS === "web" &&
+      typeof window !== "undefined" &&
+      (() => {
+        const params = new URLSearchParams(window.location.search);
+        return (
+          params.has("tabId") ||
+          params.has("workspaceId") ||
+          params.has("serverId")
+        );
+      })();
+    const derivedCanonicalPath = resolveWorkspaceTabCanonicalPath({
+      activeTabId,
+      pathname,
+      serverId: normalizedServerId,
+      workspaceId: normalizedWorkspaceId,
+    });
+    const canonicalPath =
+      derivedCanonicalPath ??
+      (hasRouteIdentitySearchParams && activeTabId
+        ? buildHostWorkspaceTabRoute(
+            normalizedServerId,
+            normalizedWorkspaceId,
+            activeTabId
+          )
+        : null);
+    if (!canonicalPath) {
+      lastCanonicalPathRef.current = null;
      return;
    }
-    if (lastCanonicalTabIdRef.current === activeTabId) {
+    if (lastCanonicalPathRef.current === canonicalPath) {
      return;
    }
-    lastCanonicalTabIdRef.current = activeTabId;
-    router.replace(
-      buildHostWorkspaceTabRoute(normalizedServerId, normalizedWorkspaceId, activeTabId) as any
-    );
-  }, [activeTabId, normalizedServerId, normalizedWorkspaceId, router, routeTabId]);
+    lastCanonicalPathRef.current = canonicalPath;
+    if (Platform.OS === "web" && typeof window !== "undefined") {
+      if (
+        window.location.pathname !== canonicalPath ||
+        window.location.search.length > 0 ||
+        window.location.hash.length > 0
+      ) {
+        window.history.replaceState(window.history.state, "", canonicalPath);
+      }
+      return;
+    }
+    const href = activeTabId
+      ? buildWorkspaceTabHref(normalizedServerId, normalizedWorkspaceId, activeTabId)
+      : null;
+    if (href) {
+      router.replace(href as any);
+      return;
+    }
+    router.replace(canonicalPath as any);
+  }, [
+    activeTabId,
+    normalizedServerId,
+    normalizedWorkspaceId,
+    pathname,
+    router,
+  ]);

  const activeTab = tabModel.activeTab;

@@ -509,11 +607,23 @@ function WorkspaceScreenContent({
      if (!tabId || !normalizedServerId || !normalizedWorkspaceId) {
        return;
      }
-      router.replace(
-        buildHostWorkspaceTabRoute(normalizedServerId, normalizedWorkspaceId, tabId) as any
+      setPendingRouteTabId(tabId);
+      focusTab({
+        serverId: normalizedServerId,
+        workspaceId: normalizedWorkspaceId,
+        tabId,
+      });
+      const href = buildWorkspaceTabHref(
+        normalizedServerId,
+        normalizedWorkspaceId,
+        tabId
      );
+      if (!href) {
+        return;
+      }
+      router.replace(href as any);
    },
-    [normalizedServerId, normalizedWorkspaceId, router]
+    [focusTab, normalizedServerId, normalizedWorkspaceId, router]
  );

  const emptyWorkspaceSeedRef = useRef<string | null>(null);
@@ -521,6 +631,9 @@ function WorkspaceScreenContent({
    if (!persistenceKey) {
      return;
    }
+    if (effectiveRouteTabId) {
+      return;
+    }
    if (tabs.length > 0) {
      emptyWorkspaceSeedRef.current = null;
      return;
@@ -545,6 +658,7 @@ function WorkspaceScreenContent({
    normalizedWorkspaceId,
    openDraftTab,
    persistenceKey,
+    effectiveRouteTabId,
    tabs.length,
  ]);

@@ -12,6 +12,8 @@ const metrics = {
  tabHorizontalPadding: 12,
  estimatedCharWidth: 7,
  closeButtonWidth: 22,
+  compactLabelCharCap: 10,
+  compactDenseLabelCharCap: 8,
 };

 describe("computeWorkspaceTabLayout", () => {
@@ -60,4 +62,28 @@ describe("computeWorkspaceTabLayout", () => {

    expect(result.mode).toBe("icon");
  });
+
+  it("keeps compact-with-label mode for realistic desktop width with nine tabs", () => {
+    const result = computeWorkspaceTabLayout({
+      viewportWidth: 1180,
+      tabLabelLengths: [18, 21, 14, 15, 19, 12, 17, 16, 20],
+      metrics,
+    });
+
+    expect(result.mode).toBe("compact");
+    expect(result.showLabels).toBe(true);
+    expect(result.showCloseButtons).toBe(false);
+  });
+
+  it("uses dense compact labels for larger tab counts before icon-only", () => {
+    const result = computeWorkspaceTabLayout({
+      viewportWidth: 1100,
+      tabLabelLengths: [18, 21, 14, 15, 19, 12, 17, 16, 20],
+      metrics,
+    });
+
+    expect(result.mode).toBe("compact");
+    expect(result.showLabels).toBe(true);
+    expect(result.showCloseButtons).toBe(false);
+  });
 });
@@ -14,6 +14,8 @@ export type WorkspaceTabLayoutInput = {
    tabHorizontalPadding: number;
    estimatedCharWidth: number;
    closeButtonWidth: number;
+    compactLabelCharCap?: number;
+    compactDenseLabelCharCap?: number;
  };
 };

@@ -70,6 +72,12 @@ export function computeWorkspaceTabLayout(
  );
  const baseTabWidth = input.metrics.tabIconWidth + input.metrics.tabHorizontalPadding * 2;
  const estimateLabelWidth = (labelLength: number) => labelLength * input.metrics.estimatedCharWidth;
+  const compactLabelCharCap = Math.max(1, input.metrics.compactLabelCharCap ?? 12);
+  const compactDenseLabelCharCap = Math.max(
+    1,
+    input.metrics.compactDenseLabelCharCap ?? Math.max(1, compactLabelCharCap - 2)
+  );
+  const effectiveCompactLabelCharCap = tabCount >= 8 ? compactDenseLabelCharCap : compactLabelCharCap;

  const fullTabWidths = input.tabLabelLengths.map((rawLength) => {
    const labelLength = Math.max(rawLength, 1);
@@ -87,7 +95,8 @@ export function computeWorkspaceTabLayout(

  const compactTabWidths = input.tabLabelLengths.map((rawLength) => {
    const labelLength = Math.max(rawLength, 1);
-    const estimatedWidth = baseTabWidth + estimateLabelWidth(labelLength);
+    const estimatedWidth =
+      baseTabWidth + estimateLabelWidth(Math.min(labelLength, effectiveCompactLabelCharCap));
    return clamp(estimatedWidth, input.metrics.minTabWidth, input.metrics.maxTabWidth);
  });
  const compactTotal = computeTotalRowWidth({
@@ -114,7 +114,7 @@ describe("deriveWorkspaceTabModel", () => {
    ]);
  });

-  it("prefers route tab, then focused tab, then first tab for active selection", () => {
+  it("prefers route tab when present, then focused tab, then first tab for active selection", () => {
    const base = {
      workspaceAgents: [makeAgent({ id: "agent-a" }), makeAgent({ id: "agent-b" })],
      terminals: [],
@@ -136,6 +136,54 @@ describe("deriveWorkspaceTabModel", () => {
        routeTabId: "agent_agent-a",
      }).activeTabId
    ).toBe("agent_agent-a");
+
+    expect(
+      deriveWorkspaceTabModel({
+        ...base,
+        focusedTabId: "agent_agent-b",
+        routeTabId: "agent_agent-missing",
+      }).activeTabId
+    ).toBe("agent_agent-b");
+  });
+
+  it("switches active tab target when route tab id changes", () => {
+    const modelA = deriveWorkspaceTabModel({
+      workspaceAgents: [makeAgent({ id: "agent-a" }), makeAgent({ id: "agent-b" })],
+      terminals: [{ id: "term-1", name: "shell" }],
+      uiTabs: [],
+      tabOrder: ["agent_agent-a", "terminal_term-1", "agent_agent-b"],
+      routeTabId: "terminal_term-1",
+    });
+
+    const modelB = deriveWorkspaceTabModel({
+      workspaceAgents: [makeAgent({ id: "agent-a" }), makeAgent({ id: "agent-b" })],
+      terminals: [{ id: "term-1", name: "shell" }],
+      uiTabs: [],
+      tabOrder: ["agent_agent-a", "terminal_term-1", "agent_agent-b"],
+      routeTabId: "agent_agent-b",
+    });
+
+    expect(modelA.activeTab?.descriptor.tabId).toBe("terminal_term-1");
+    expect(modelA.activeTab?.target.kind).toBe("terminal");
+    expect(modelB.activeTab?.descriptor.tabId).toBe("agent_agent-b");
+    expect(modelB.activeTab?.target.kind).toBe("agent");
+  });
+
+  it("re-resolves active content for a new workspace when prior focused tab is not available", () => {
+    const model = deriveWorkspaceTabModel({
+      workspaceAgents: [makeAgent({ id: "workspace-b-agent", title: "B" })],
+      terminals: [],
+      uiTabs: [],
+      tabOrder: ["agent_workspace-b-agent"],
+      focusedTabId: "agent_workspace-a-agent",
+      routeTabId: null,
+    });
+
+    expect(model.activeTabId).toBe("agent_workspace-b-agent");
+    expect(model.activeTab?.target).toEqual({
+      kind: "agent",
+      agentId: "workspace-b-agent",
+    });
  });

  it("covers regression: non-archived attention agent remains visible even if UI tabs omitted it", () => {
@@ -212,11 +212,12 @@ export function deriveWorkspaceTabModel(input: {
  const routeTabId = trimNonEmpty(input.routeTabId);
  const focusedTabId = trimNonEmpty(input.focusedTabId);

-  const activeTabId = routeTabId && openTabIds.has(routeTabId)
-    ? routeTabId
-    : focusedTabId && openTabIds.has(focusedTabId)
-      ? focusedTabId
-      : tabs[0]?.descriptor.tabId ?? null;
+  const activeTabId =
+    routeTabId && openTabIds.has(routeTabId)
+      ? routeTabId
+      : focusedTabId && openTabIds.has(focusedTabId)
+        ? focusedTabId
+        : tabs[0]?.descriptor.tabId ?? null;

  const activeTab = activeTabId
    ? tabs.find((tab) => tab.descriptor.tabId === activeTabId) ?? null