fix(app): show Setup tabs only for failures (#3634)

* fix(app): only surface failed workspace setup Running and successful setup snapshots were treated as reasons to seed a background Setup tab, adding noise to every workspace with setup commands. Seed only failed setup and classify that tab as main-workspace content so the failure stays visible without stealing focus. * fix(app): surface failed setup once Keep the failed-setup surfacing claim with the setup snapshot so closing its background tab remains respected after the workspace remounts. Explicitly prefer the main pane for a new failure tab without moving a user-placed existing tab. * fix(app): keep setup tabs out of side panel A preferred main-pane placement can fall back when users remove that pane. Keep that implicit fallback out of the focused Side panel when another workspace pane is available.
2026-08-21 11:17:04 +02:00
parent bc02cfd8cd
commit 04624cb419
10 changed files with 271 additions and 61 deletions
@@ -1,7 +1,10 @@
 import { test, expect } from "../support/fixtures";
 import { createTempGitRepo } from "../support/helpers/workspace";
 import {
+  closeSetupTab,
  waitForWorkspaceTabsVisible,
+  expectFailedSetupTabSeededInMainPane,
+  expectSetupTabNotSeeded,
  expectNoTerminalTabs,
  clickFirstTerminalTab,
  expectFirstTerminalTabContains,
@@ -19,6 +22,7 @@ import {
  expectSetupPanel,
  openHomeWithProject,
  navigateToWorkspaceViaSidebar,
+  returnHomeFromWorkspace,
  openWorkspaceScriptsMenu,
  startWorkspaceScriptFromMenu,
  closeWorkspaceScriptsMenu,
@@ -39,7 +43,7 @@ interface WorkspaceScriptStarter {
 }

 test.describe("Workspace setup streaming", () => {
-  test("opens the setup tab when a workspace is created from the sidebar", async ({ page }) => {
+  test("does not seed the setup tab while workspace setup is running", async ({ page }) => {
    const client = await connectWorkspaceSetupClient();
    const repo = await createTempGitRepo("setup-open-", {
      paseoConfig: {
@@ -60,7 +64,7 @@ test.describe("Workspace setup streaming", () => {
      await openHomeWithProject(page, repo.path);
      await navigateToWorkspaceViaSidebar(page, workspace.id);

-      await expectSetupPanel(page);
+      await expectSetupTabNotSeeded(page, workspace.id);
    } finally {
      await client.close();
      await repo.cleanup();
@@ -99,6 +103,8 @@ test.describe("Workspace setup streaming", () => {
      await openHomeWithProject(page, repo.path);
      await navigateToWorkspaceViaSidebar(page, workspace.id);

+      await expectSetupTabNotSeeded(page, workspace.id);
+      await expectSetupPanel(page);
      await waitForWorkspaceTabsVisible(page);
      await clickNewChat(page);
      await expectComposerVisible(page, { timeout: 30_000 });
@@ -156,7 +162,7 @@ test.describe("Workspace setup streaming", () => {
    }
  });

-  test("streams a failed setup snapshot when setup fails", async () => {
+  test("seeds a failed setup tab once in the main pane", async ({ page }) => {
    const client = await connectWorkspaceSetupClient();
    const repo = await createTempGitRepo("setup-failure-", {
      paseoConfig: {
@@ -173,7 +179,7 @@ test.describe("Workspace setup streaming", () => {
        (payload) => payload.status === "failed" && payload.detail.log.includes("setup failed"),
      );

-      await createWorkspaceThroughDaemon(client, {
+      const workspace = await createWorkspaceThroughDaemon(client, {
        cwd: repo.path,
        worktreeSlug: "workspace-setup-failure",
      });
@@ -182,6 +188,16 @@ test.describe("Workspace setup streaming", () => {
      expect(failedPayload.detail.log).toContain("starting setup");
      expect(failedPayload.detail.log).toContain("setup failed");
      expect(failedPayload.error).toMatch(/failed/i);
+
+      await openHomeWithProject(page, repo.path);
+      await navigateToWorkspaceViaSidebar(page, workspace.id);
+      await waitForWorkspaceTabsVisible(page);
+      await expectFailedSetupTabSeededInMainPane(page, workspace.id);
+
+      await closeSetupTab(page, workspace.id);
+      await returnHomeFromWorkspace(page);
+      await navigateToWorkspaceViaSidebar(page, workspace.id);
+      await expectSetupTabNotSeeded(page, workspace.id);
    } finally {
      await client.close();
      await repo.cleanup();
@@ -276,6 +276,11 @@ export async function navigateToWorkspaceViaSidebar(
  });
 }

+export async function returnHomeFromWorkspace(page: Page): Promise<void> {
+  await page.getByRole("button", { name: "Home" }).click();
+  await expect(page).not.toHaveURL(/\/workspace\//, { timeout: 30_000 });
+}
+
 export async function openWorkspaceScriptsMenu(page: Page): Promise<void> {
  await page.getByTestId("workspace-scripts-button").click();
  await expect(page.getByTestId("workspace-scripts-menu")).toBeVisible({ timeout: 10_000 });
@@ -13,6 +13,46 @@ export async function getWorkspaceTabTestIds(page: Page): Promise<string[]> {
  return ids;
 }

+function setupTabTestId(workspaceId: string): string {
+  return `workspace-tab-setup_${workspaceId}`;
+}
+
+async function waitForSetupToReachWorkspace(page: Page): Promise<void> {
+  const actionsButton = page.getByTestId("workspace-header-menu-trigger");
+  await expect(actionsButton).toBeVisible({ timeout: 30_000 });
+  await actionsButton.click();
+  await expect(page.getByTestId("workspace-header-show-setup")).toBeVisible({ timeout: 30_000 });
+  await page.keyboard.press("Escape");
+}
+
+export async function expectSetupTabNotSeeded(page: Page, workspaceId: string): Promise<void> {
+  await waitForSetupToReachWorkspace(page);
+  const tab = page.getByTestId(setupTabTestId(workspaceId));
+  for (let attempt = 0; attempt < 5; attempt += 1) {
+    await expect(tab).toHaveCount(0);
+    await page.waitForTimeout(100);
+  }
+}
+
+export async function expectFailedSetupTabSeededInMainPane(
+  page: Page,
+  workspaceId: string,
+): Promise<void> {
+  const tabId = setupTabTestId(workspaceId);
+  await expect(page.getByTestId(tabId).filter({ visible: true }).first()).toBeVisible({
+    timeout: 30_000,
+  });
+  const panel = await ensureSidePanel(page);
+  await expect(panel.getByTestId(tabId)).toHaveCount(0);
+}
+
+export async function closeSetupTab(page: Page, workspaceId: string): Promise<void> {
+  const tabId = setupTabTestId(workspaceId);
+  await page.getByTestId(tabId).filter({ visible: true }).first().click({ button: "right" });
+  await page.getByTestId(`workspace-tab-context-setup_${workspaceId}-close`).click();
+  await expect(page.getByTestId(tabId)).toHaveCount(0);
+}
+
 function visibleTestId(page: Page, testId: string) {
  return page.getByTestId(testId).filter({ visible: true });
 }
@@ -59,6 +59,7 @@ import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-stor
 import {
  canDismissPaneInLayout,
  collectAllTabs,
+  DEFAULT_PANE_ID,
  findPaneById,
  getFocusedBrowserId,
  FOCUSED_PANE_PLACEMENT,
@@ -91,7 +92,11 @@ import {
  useHosts,
 } from "@/runtime/host-runtime";
 import { prefetchProvidersSnapshot } from "@/hooks/use-providers-snapshot";
-import { shouldShowWorkspaceSetup, useWorkspaceSetupStore } from "@/stores/workspace-setup-store";
+import {
+  shouldSeedWorkspaceSetupTab,
+  shouldShowWorkspaceSetup,
+  useWorkspaceSetupStore,
+} from "@/stores/workspace-setup-store";
 import { useWorkspace } from "@/stores/session-store-hooks";
 import { useWorkspaceTerminalSessionRetention } from "@/terminal/hooks/use-workspace-terminal-session-retention";
 import type { CheckoutStatusPayload } from "@/git/use-status-query";
@@ -185,7 +190,6 @@ import { useWorkspaceCheckoutStatus } from "@/screens/workspace/use-workspace-ch
 import { useHasPullRequest } from "@/panels/pull-request";
 import { fileStateForFilesView } from "@/panels/file/state";

-const WORKSPACE_SETUP_AUTO_OPEN_WINDOW_MS = 30_000;
 const WORKSPACE_FLOATING_PANEL_PORTAL_HOST_PREFIX = "workspace-floating-panels";
 const EMPTY_UI_TABS: WorkspaceTab[] = [];
 const EMPTY_WORKSPACE_SCRIPTS: WorkspaceDescriptor["scripts"] = [];
@@ -1692,6 +1696,7 @@ function WorkspaceScreenContent({
    persistenceKey ? (state.snapshots[persistenceKey] ?? null) : null,
  );
  const ensureWorkspaceSetupStatus = useWorkspaceSetupStore((state) => state.ensureSetupStatus);
+  const claimFailedSetupSurface = useWorkspaceSetupStore((state) => state.claimFailedSetupSurface);
  const showWorkspaceSetup = shouldShowWorkspaceSetup(workspaceSetupSnapshot);
  const uiTabs = useMemo(
    () => (workspaceLayout ? collectAllTabs(workspaceLayout.root) : EMPTY_UI_TABS),
@@ -1966,7 +1971,6 @@ function WorkspaceScreenContent({
  );

  const emptyWorkspaceSeedRef = useRef<string | null>(null);
-  const autoOpenedSetupTabWorkspaceRef = useRef<string | null>(null);

  useEffect(() => {
    if (!isRouteFocused) {
@@ -1975,25 +1979,7 @@ function WorkspaceScreenContent({
    if (!persistenceKey) {
      return;
    }
-    if (!workspaceSetupSnapshot || !showWorkspaceSetup) {
-      if (autoOpenedSetupTabWorkspaceRef.current === persistenceKey) {
-        autoOpenedSetupTabWorkspaceRef.current = null;
-      }
-      return;
-    }
-
-    const snapshotAge = Date.now() - workspaceSetupSnapshot.updatedAt;
-    const shouldAutoOpen =
-      workspaceSetupSnapshot.status === "running" ||
-      snapshotAge <= WORKSPACE_SETUP_AUTO_OPEN_WINDOW_MS;
-    if (!shouldAutoOpen) {
-      return;
-    }
-    if (hasSetupTab) {
-      autoOpenedSetupTabWorkspaceRef.current = persistenceKey;
-      return;
-    }
-    if (autoOpenedSetupTabWorkspaceRef.current === persistenceKey) {
+    if (!shouldSeedWorkspaceSetupTab(workspaceSetupSnapshot)) {
      return;
    }

@@ -2004,27 +1990,30 @@ function WorkspaceScreenContent({
    if (!target) {
      return;
    }
-
-    const tabId = openSupportingTab({
-      isCompact: isMobile,
-      workspaceKey: persistenceKey,
-      target,
-      openInSidePanelByDefault,
-      background: true,
-    });
-    if (!tabId) {
+    if (
+      !claimFailedSetupSurface({
+        serverId: normalizedServerId,
+        workspaceId: normalizedWorkspaceId,
+      })
+    ) {
+      return;
+    }
+    if (hasSetupTab) {
      return;
    }

-    autoOpenedSetupTabWorkspaceRef.current = persistenceKey;
+    openWorkspaceTabInBackground(persistenceKey, target, {
+      mode: "prefer",
+      paneId: DEFAULT_PANE_ID,
+    });
  }, [
+    claimFailedSetupSurface,
    hasSetupTab,
-    isMobile,
    isRouteFocused,
    normalizedWorkspaceId,
-    openInSidePanelByDefault,
+    normalizedServerId,
+    openWorkspaceTabInBackground,
    persistenceKey,
-    showWorkspaceSetup,
    workspaceSetupSnapshot,
  ]);

@@ -255,7 +255,7 @@ export interface WorkspaceTabSnapshot {
  hasActivePendingDraftCreate?: boolean;
 }

-const DEFAULT_PANE_ID = "main";
+export const DEFAULT_PANE_ID = "main";
 /** The pane id is persisted, so it keeps its pre-rename spelling. */
 export const SIDE_PANEL_PANE_ID = "explorer";
 const DEFAULT_LAYOUT_GROUP_ID = "workspace-root";
@@ -1162,15 +1162,17 @@ export function removeTabFromTree(root: SplitNode, tabId: string): SplitNode {
  }).root;
 }

-// Tab kinds the user works *in* rather than consults. Reconciliation opens these
-// with nobody behind the click, and the side panel can hold focus from an earlier
-// reveal, so an ambient open must not drop an agent there just because focus stayed.
+// Tab kinds that belong in the main workspace rather than the side panel. Ambient
+// opens, and preferred opens whose requested pane no longer exists, have nobody
+// behind the fallback. The side panel can retain focus from an earlier reveal, so
+// placement must route these back to the main workspace.
 const SIDE_PANEL_EXCLUDED_TAB_KINDS: ReadonlySet<WorkspaceTabTarget["kind"]> = new Set([
  "agent",
  "provider_subagent",
  "terminal",
  "draft",
  "browser",
+  "setup",
 ]);

 function resolvePlacementPane(input: {
@@ -1196,7 +1198,7 @@ function resolvePlacementPane(input: {
    findPaneById(createDefaultLayout().root, DEFAULT_PANE_ID);
  invariant(focusedPane, "Workspace layout must always have a pane");
  if (
-    input.placement.mode !== "ambient" ||
+    (input.placement.mode !== "ambient" && input.placement.mode !== "prefer") ||
    focusedPane.id !== input.sidePanelPaneId ||
    !SIDE_PANEL_EXCLUDED_TAB_KINDS.has(input.target.kind)
  ) {
@@ -921,6 +921,68 @@ describe("workspace-layout-store actions", () => {
    expect(layout.focusedPaneId).toBe("explorer");
  });

+  it("places a background setup tab in the main pane without moving focus", () => {
+    const workspaceKey = createWorkspaceKey();
+    const store = workspaceLayoutStore.getState();
+    store.openTab({
+      workspaceKey,
+      target: { kind: "agent", agentId: "agent-1" },
+      intent: "reveal",
+    });
+    store.focusPane(workspaceKey, "explorer");
+
+    const setupTabId = store.openTab({
+      workspaceKey,
+      target: { kind: "setup", workspaceId: WORKSPACE_ID },
+      intent: "background",
+      placement: { mode: "prefer", paneId: "main" },
+    });
+
+    const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
+    expect(findPaneContainingTab(layout.root, setupTabId as string)?.id).toBe("main");
+    expect(layout.focusedPaneId).toBe("explorer");
+  });
+
+  it("keeps a failed setup tab out of the focused side panel when main was removed", () => {
+    const workspaceKey = createWorkspaceKey();
+    const store = workspaceLayoutStore.getState();
+    workspaceLayoutStore.setState((state) => ({
+      layoutByWorkspace: {
+        ...state.layoutByWorkspace,
+        [workspaceKey]: normalizeLayout({
+          root: {
+            kind: "group",
+            group: {
+              id: "root",
+              direction: "horizontal",
+              sizes: [0.5, 0.5],
+              children: [
+                createPane({ id: "secondary", tabIds: [] }),
+                createPane({ id: "explorer", tabIds: [] }),
+              ],
+            },
+          },
+          focusedPaneId: "explorer",
+        }),
+      },
+      sidePanelPaneIdByWorkspace: {
+        ...state.sidePanelPaneIdByWorkspace,
+        [workspaceKey]: "explorer",
+      },
+    }));
+
+    const setupTabId = store.openTab({
+      workspaceKey,
+      target: { kind: "setup", workspaceId: WORKSPACE_ID },
+      intent: "background",
+      placement: { mode: "prefer", paneId: "main" },
+    });
+
+    const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey];
+    expect(findPaneContainingTab(layout.root, setupTabId as string)?.id).toBe("secondary");
+    expect(layout.focusedPaneId).toBe("explorer");
+  });
+
  it("keeps ambient browser opens out of the focused explorer pane", () => {
    const workspaceKey = createWorkspaceKey();
    const store = workspaceLayoutStore.getState();
@@ -19,6 +19,7 @@ import {
  convertDraftToAgentInLayout,
  createTabInLayout,
  createDefaultLayout,
+  DEFAULT_PANE_ID,
  AMBIENT_PLACEMENT,
  createWorkspaceLayoutWithSidePanel,
  FOCUSED_PANE_PLACEMENT,
@@ -62,6 +63,7 @@ export {
  collectAllPanes,
  collectAllTabs,
  createDefaultLayout,
+  DEFAULT_PANE_ID,
  createWorkspaceLayoutWithSidePanel,
  FOCUSED_PANE_PLACEMENT,
  findPaneById,
@@ -1,11 +1,32 @@
 import { beforeEach, describe, expect, it } from "vitest";
 import {
+  shouldSeedWorkspaceSetupTab,
  shouldShowWorkspaceSetup,
  useWorkspaceSetupStore,
+  type WorkspaceSetupSnapshot,
  type WorkspaceSetupStatusClient,
  type WorkspaceSetupStatusResult,
 } from "./workspace-setup-store";

+function makeSnapshot(
+  status: WorkspaceSetupSnapshot["status"],
+  error: string | null = null,
+): WorkspaceSetupSnapshot {
+  return {
+    workspaceId: "workspace-1",
+    status,
+    detail: {
+      type: "worktree_setup",
+      worktreePath: "/Users/test/project",
+      branchName: "main",
+      log: "",
+      commands: [],
+    },
+    error,
+    updatedAt: Date.now(),
+  };
+}
+
 const DEFAULT_SNAPSHOT: WorkspaceSetupStatusResult["snapshot"] = {
  status: "running",
  detail: {
@@ -105,6 +126,7 @@ describe("workspace-setup-store", () => {
      pendingWorkspaceSetup: null,
      snapshots: {},
      requestedKeys: new Set(),
+      surfacedFailedSetupKeys: new Set(),
    });
  });

@@ -199,6 +221,40 @@ describe("workspace-setup-store", () => {
    ).toBe(true);
  });

+  it("seeds a setup tab only for failed setup", () => {
+    expect(shouldSeedWorkspaceSetupTab(null)).toBe(false);
+    expect(shouldSeedWorkspaceSetupTab(makeSnapshot("running"))).toBe(false);
+    expect(shouldSeedWorkspaceSetupTab(makeSnapshot("completed"))).toBe(false);
+    expect(shouldSeedWorkspaceSetupTab(makeSnapshot("failed", "Setup failed"))).toBe(true);
+  });
+
+  it("claims one failed setup surface until a later setup lifecycle begins", () => {
+    const store = useWorkspaceSetupStore.getState();
+    const identity = { serverId: "server-1", workspaceId: "42" };
+
+    store.upsertProgress({
+      serverId: identity.serverId,
+      payload: { ...makeSnapshot("failed", "Setup failed"), workspaceId: identity.workspaceId },
+    });
+
+    expect(store.claimFailedSetupSurface(identity)).toBe(true);
+    expect(useWorkspaceSetupStore.getState().claimFailedSetupSurface(identity)).toBe(false);
+
+    useWorkspaceSetupStore.getState().upsertProgress({
+      serverId: identity.serverId,
+      payload: { ...makeSnapshot("running"), workspaceId: identity.workspaceId },
+    });
+    useWorkspaceSetupStore.getState().upsertProgress({
+      serverId: identity.serverId,
+      payload: {
+        ...makeSnapshot("failed", "Setup failed again"),
+        workspaceId: identity.workspaceId,
+      },
+    });
+
+    expect(useWorkspaceSetupStore.getState().claimFailedSetupSurface(identity)).toBe(true);
+  });
+
  it("ensureSetupStatus fetches setup status once and stores the snapshot", async () => {
    const { client, calls } = makeClient(resolveDefault);

@@ -37,13 +37,19 @@ export function shouldShowWorkspaceSetup(snapshot: WorkspaceSetupSnapshot | null
  return snapshot.error !== null || snapshot.detail.commands.length > 0;
 }

+export function shouldSeedWorkspaceSetupTab(snapshot: WorkspaceSetupSnapshot | null): boolean {
+  return snapshot?.status === "failed";
+}
+
 interface WorkspaceSetupStoreState {
  pendingWorkspaceSetup: PendingWorkspaceSetup | null;
  snapshots: Record<string, WorkspaceSetupSnapshot>;
  requestedKeys: Set<string>;
+  surfacedFailedSetupKeys: Set<string>;
  beginWorkspaceSetup: (value: PendingWorkspaceSetup) => void;
  clearWorkspaceSetup: () => void;
  upsertProgress: (input: { serverId: string; payload: WorkspaceSetupProgressPayload }) => void;
+  claimFailedSetupSurface: (input: { serverId: string; workspaceId: string }) => boolean;
  ensureSetupStatus: (input: {
    serverId: string;
    workspaceId: string;
@@ -61,6 +67,7 @@ export const useWorkspaceSetupStore = create<WorkspaceSetupStoreState>()((set, g
  pendingWorkspaceSetup: null,
  snapshots: {},
  requestedKeys: new Set(),
+  surfacedFailedSetupKeys: new Set(),
  beginWorkspaceSetup: (value) => {
    set({ pendingWorkspaceSetup: value });
  },
@@ -73,15 +80,38 @@ export const useWorkspaceSetupStore = create<WorkspaceSetupStoreState>()((set, g
      return;
    }

-    set((state) => ({
-      snapshots: {
-        ...state.snapshots,
-        [key]: {
-          ...payload,
-          updatedAt: Date.now(),
+    set((state) => {
+      const surfacedFailedSetupKeys = new Set(state.surfacedFailedSetupKeys);
+      if (payload.status !== "failed") {
+        surfacedFailedSetupKeys.delete(key);
+      }
+      return {
+        snapshots: {
+          ...state.snapshots,
+          [key]: {
+            ...payload,
+            updatedAt: Date.now(),
+          },
        },
-      },
-    }));
+        surfacedFailedSetupKeys,
+      };
+    });
+  },
+  claimFailedSetupSurface: ({ serverId, workspaceId }) => {
+    const key = buildWorkspaceSetupKey({ serverId, workspaceId });
+    if (!key) {
+      return false;
+    }
+
+    let claimed = false;
+    set((state) => {
+      if (state.snapshots[key]?.status !== "failed" || state.surfacedFailedSetupKeys.has(key)) {
+        return state;
+      }
+      claimed = true;
+      return { surfacedFailedSetupKeys: new Set(state.surfacedFailedSetupKeys).add(key) };
+    });
+    return claimed;
  },
  ensureSetupStatus: async ({ serverId, workspaceId, client }) => {
    const key = buildWorkspaceSetupKey({ serverId, workspaceId });
@@ -124,12 +154,14 @@ export const useWorkspaceSetupStore = create<WorkspaceSetupStoreState>()((set, g
    }

    set((state) => {
-      if (!(key in state.snapshots)) {
+      if (!(key in state.snapshots) && !state.surfacedFailedSetupKeys.has(key)) {
        return state;
      }
      const next = { ...state.snapshots };
      delete next[key];
-      return { snapshots: next };
+      const surfacedFailedSetupKeys = new Set(state.surfacedFailedSetupKeys);
+      surfacedFailedSetupKeys.delete(key);
+      return { snapshots: next, surfacedFailedSetupKeys };
    });
  },
  clearServer: (serverId) => {
@@ -137,10 +169,16 @@ export const useWorkspaceSetupStore = create<WorkspaceSetupStoreState>()((set, g
      const nextEntries = Object.entries(state.snapshots).filter(
        ([key]) => !key.startsWith(`${serverId}:`),
      );
-      if (nextEntries.length === Object.keys(state.snapshots).length) {
+      const surfacedFailedSetupKeys = new Set(
+        [...state.surfacedFailedSetupKeys].filter((key) => !key.startsWith(`${serverId}:`)),
+      );
+      if (
+        nextEntries.length === Object.keys(state.snapshots).length &&
+        surfacedFailedSetupKeys.size === state.surfacedFailedSetupKeys.size
+      ) {
        return state;
      }
-      return { snapshots: Object.fromEntries(nextEntries) };
+      return { snapshots: Object.fromEntries(nextEntries), surfacedFailedSetupKeys };
    });
  },
 }));