diff --git a/packages/app/e2e/browser/explorer-pane-placement.spec.ts b/packages/app/e2e/browser/explorer-pane-placement.spec.ts new file mode 100644 index 000000000..35a820daa --- /dev/null +++ b/packages/app/e2e/browser/explorer-pane-placement.spec.ts @@ -0,0 +1,231 @@ +// Locks two behaviours that used to break together on web/desktop: +// 1) with the explorer pane focused, a newly appearing agent must auto-open into the +// MAIN pane in the background — the explorer pane is a background surface and must +// never swallow entity tabs or lose the user's focus, and +// 2) splitting a tab out of a pane leaves that pane empty, and the app must stay +// interactive in the [agent | empty main | explorer] state (the reporter hit an +// app-wide lockup here with no way to close the empty pane). +import { expect, type Locator, type Page } from "@playwright/test"; +import { test } from "../support/fixtures"; +import { gotoWorkspace } from "../support/helpers/launcher"; +import { seedWorkspace } from "../support/helpers/seed-client"; +import { ensureExplorerPane, waitForWorkspaceTabsVisible } from "../support/helpers/workspace-tabs"; + +function visible(page: Page, testId: string): Locator { + return page.getByTestId(testId).filter({ visible: true }); +} + +function agentTabChip(page: Page, agentId: string): Locator { + return visible(page, `workspace-tab-agent_${agentId}`); +} + +function draftTabChip(page: Page): Locator { + return page.locator('[data-testid^="workspace-tab-draft_"]').filter({ visible: true }); +} + +/** The explorer pane is the only tab row carrying the Changes tab. */ +function explorerTabRow(page: Page): Locator { + return visible(page, "workspace-tabs-row").filter({ + has: page.getByTestId("workspace-tab-working_diff"), + }); +} + +function mainTabRow(page: Page): Locator { + return visible(page, "workspace-tabs-row").filter({ + hasNot: page.getByTestId("workspace-tab-working_diff"), + }); +} + +async function focusExplorerPane(page: Page): Promise { + await ensureExplorerPane(page); + const changesTab = visible(page, "workspace-tab-working_diff").first(); + await changesTab.click({ position: { x: 12, y: 13 } }); + await expect(changesTab).toHaveAttribute("aria-selected", "true"); +} + +async function closeSeededDraftInMainPane(page: Page): Promise { + const chip = draftTabChip(page).first(); + await chip.hover(); + await page + .locator('[data-testid^="workspace-draft-close-"]') + .filter({ visible: true }) + .first() + .click(); + await expect(draftTabChip(page)).toHaveCount(0, { timeout: 15_000 }); +} + +async function emptyPaneBox(page: Page) { + const paneChild = page + .getByTestId("split-group-child") + .filter({ hasText: "No tabs in this pane." }) + .first(); + await expect(paneChild).toBeVisible({ timeout: 15_000 }); + const box = await paneChild.boundingBox(); + expect(box).not.toBeNull(); + if (!box) throw new Error("Empty pane has no bounding box"); + return box; +} + +async function paneBoxContainingChip(page: Page, chip: Locator) { + const paneChild = page.getByTestId("split-group-child").filter({ has: chip }).first(); + await expect(paneChild).toBeVisible({ timeout: 15_000 }); + const box = await paneChild.boundingBox(); + expect(box).not.toBeNull(); + if (!box) throw new Error("Pane has no bounding box"); + return box; +} + +/** dnd-kit pointer drag from a tab chip to an absolute point. */ +async function dragChipTo( + page: Page, + chip: Locator, + target: { x: number; y: number }, +): Promise { + const chipBox = await chip.boundingBox(); + expect(chipBox).not.toBeNull(); + if (!chipBox) throw new Error("Dragged chip has no bounding box"); + + await page.mouse.move(chipBox.x + chipBox.width / 2, chipBox.y + chipBox.height / 2); + await page.mouse.down(); + // Exceed the 8px PointerSensor activation distance before heading to the target. + await page.mouse.move(chipBox.x + chipBox.width / 2 + 12, chipBox.y + chipBox.height / 2 + 4); + await page.mouse.move(target.x, target.y, { steps: 20 }); + await page.mouse.up(); +} + +test.describe("explorer pane tab placement", () => { + test("agents open in the main pane and empty-pane splits keep the app usable", async ({ + page, + }, testInfo) => { + const consoleErrors: string[] = []; + page.on("console", (message) => { + if (message.type() === "error") consoleErrors.push(message.text()); + }); + page.on("pageerror", (error) => consoleErrors.push(`pageerror: ${String(error)}`)); + + const workspace = await seedWorkspace({ repoPrefix: "explorer-pane-placement-" }); + let agentId = ""; + + try { + await test.step("empty workspace seeds a draft in the main pane", async () => { + await gotoWorkspace(page, workspace.workspaceId); + await waitForWorkspaceTabsVisible(page); + await expect(draftTabChip(page).first()).toBeVisible({ timeout: 30_000 }); + }); + + await test.step("focus the explorer pane", async () => { + await focusExplorerPane(page); + }); + + await test.step("an agent appearing now opens in the main pane, not the explorer pane", async () => { + const agent = await workspace.client.createAgent({ + provider: "mock", + cwd: workspace.repoPath, + workspaceId: workspace.workspaceId, + title: "Explorer Placement Agent", + modeId: "load-test", + model: "ten-second-stream", + // Keep the agent streaming through the drag so pane contents are live, + // matching the reporter's "spinners were still going" environment. + initialPrompt: "stream please", + }); + agentId = agent.id; + await expect(agentTabChip(page, agentId).first()).toBeVisible({ timeout: 30_000 }); + + await expect( + mainTabRow(page).first().getByTestId(`workspace-tab-agent_${agentId}`), + ).toBeVisible(); + await expect( + explorerTabRow(page).first().getByTestId(`workspace-tab-agent_${agentId}`), + ).toHaveCount(0); + // The background open must not steal the explorer pane's focus. + await expect(visible(page, "workspace-tab-working_diff").first()).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + await test.step("close the draft; the agent is the main pane's only tab", async () => { + await closeSeededDraftInMainPane(page); + await expect( + mainTabRow(page).first().locator('[data-testid^="workspace-tab-"]'), + ).toHaveCount(1); + await testInfo.attach("before-drag", { + body: await page.screenshot(), + contentType: "image/png", + }); + }); + + await test.step("split the agent out of the main pane, leaving it empty", async () => { + const chip = agentTabChip(page, agentId).first(); + const mainBox = await paneBoxContainingChip(page, chip); + // Land inside the left 15% edge band of the main pane content. + await dragChipTo(page, chip, { + x: mainBox.x + mainBox.width * 0.06, + y: mainBox.y + mainBox.height * 0.6, + }); + await testInfo.attach("after-split", { + body: await page.screenshot(), + contentType: "image/png", + }); + // The split must have taken: agent pane + empty main + explorer = 3 tab rows. + await expect(visible(page, "workspace-tabs-row")).toHaveCount(3, { timeout: 10_000 }); + await expect(page.getByText("No tabs in this pane.")).toBeVisible(); + }); + + await test.step("app must stay interactive after the split", async () => { + // JS main thread alive? + const pong = await page.evaluate("1 + 1"); + expect(pong).toBe(2); + + // Pointer interaction alive? Selecting the agent chip must still work. + const chip = agentTabChip(page, agentId).first(); + await chip.click({ position: { x: 12, y: 13 }, timeout: 5_000 }); + await expect(chip).toHaveAttribute("aria-selected", "true", { timeout: 5_000 }); + + // Clicking inside the empty pane (focuses it) must not lock anything. + const emptyPane = page + .getByTestId("split-group-child") + .filter({ hasText: "No tabs in this pane." }) + .first(); + await emptyPane.click({ timeout: 5_000 }); + + // The empty pane's + menu must open and dismiss cleanly. + const emptyPaneTrigger = emptyPane.getByTestId("workspace-new-tab-menu-trigger").first(); + await emptyPaneTrigger.click({ timeout: 5_000 }); + await page.keyboard.press("Escape"); + + // A second drag must work: center-drop the agent tab back into the empty pane. + const target = await emptyPaneBox(page); + await dragChipTo(page, chip, { + x: target.x + target.width / 2, + y: target.y + target.height / 2, + }); + await testInfo.attach("after-center-drop", { + body: await page.screenshot(), + contentType: "image/png", + }); + + // The agent chip must have moved into the previously empty pane and stay clickable. + await expect(page.getByText("No tabs in this pane.")).toHaveCount(0, { timeout: 10_000 }); + await agentTabChip(page, agentId) + .first() + .click({ position: { x: 12, y: 13 } }); + + // The explorer toggle must still respond. + await visible(page, "workspace-explorer-toggle").first().click({ timeout: 5_000 }); + await testInfo.attach("after-interactions", { + body: await page.screenshot(), + contentType: "image/png", + }); + }); + + await testInfo.attach("console-errors", { + body: JSON.stringify(consoleErrors, null, 2), + contentType: "application/json", + }); + } finally { + await workspace.cleanup(); + } + }); +}); diff --git a/packages/app/src/desktop/browser/new-tab-requests/index.test.ts b/packages/app/src/desktop/browser/new-tab-requests/index.test.ts index 92fd80f7c..d8ae58717 100644 --- a/packages/app/src/desktop/browser/new-tab-requests/index.test.ts +++ b/packages/app/src/desktop/browser/new-tab-requests/index.test.ts @@ -9,6 +9,7 @@ function createLayoutWithBrowser(browserId: string): WorkspaceLayout { layout: createDefaultLayout(), target: { kind: "browser", browserId }, now: 1, + explorerPaneId: null, }).layout; } diff --git a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx index 540f6b4a9..c5468b731 100644 --- a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx +++ b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx @@ -1183,7 +1183,6 @@ function ResolvedWorkspaceDesktopTabsRow({ serverId: normalizedServerId, workspaceId: normalizedWorkspaceId, }); - const focusPane = useWorkspaceLayoutStore((state) => state.focusPane); const openTabFocused = useWorkspaceLayoutStore((state) => state.openTabFocused); const handleTabsContainerLayout = useCallback((event: LayoutChangeEvent) => { @@ -1426,12 +1425,9 @@ function ResolvedWorkspaceDesktopTabsRow({ if (!workspaceKey) { return; } - if (paneId) { - focusPane(workspaceKey, paneId); - } - openTabFocused(workspaceKey, target); + openTabFocused(workspaceKey, target, { paneId }); }, - [focusPane, openTabFocused, paneId, workspaceKey], + [openTabFocused, paneId, workspaceKey], ); const handleOpenChanges = useCallback(() => openPanelTarget(CHANGES_TARGET), [openPanelTarget]); const handleOpenFiles = useCallback(() => openPanelTarget(FILES_TARGET), [openPanelTarget]); diff --git a/packages/app/src/screens/workspace/workspace-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index 231580776..a41e9aa16 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -58,6 +58,7 @@ import { getFocusedBrowserId, resolveExplorerPaneId, type WorkspaceLayout, + type WorkspaceTabOpenOptions, useWorkspaceLayoutStore, useWorkspaceLayoutStoreHydrated, } from "@/stores/workspace-layout-store"; @@ -1460,8 +1461,11 @@ function resolveExplorerOpen( interface WorkspaceTerminalTabActionsInput { persistenceKey: string | null; - focusWorkspacePane: (workspaceKey: string, paneId: string) => void; - openWorkspaceTabFocused: (workspaceKey: string, target: WorkspaceTabTarget) => string | null; + openWorkspaceTabFocused: ( + workspaceKey: string, + target: WorkspaceTabTarget, + options?: WorkspaceTabOpenOptions, + ) => string | null; labels: { workspacePathUnavailable: string; terminalQueued: string; @@ -1482,7 +1486,6 @@ interface WorkspaceTerminalTabActions { function useWorkspaceTerminalTabActions({ persistenceKey, - focusWorkspacePane, openWorkspaceTabFocused, labels, toast, @@ -1492,12 +1495,9 @@ function useWorkspaceTerminalTabActions({ if (!persistenceKey) { return; } - if (paneId) { - focusWorkspacePane(persistenceKey, paneId); - } - openWorkspaceTabFocused(persistenceKey, { kind: "terminal", terminalId }); + openWorkspaceTabFocused(persistenceKey, { kind: "terminal", terminalId }, { paneId }); }, - [focusWorkspacePane, openWorkspaceTabFocused, persistenceKey], + [openWorkspaceTabFocused, persistenceKey], ); const handleScriptTerminalSelected = useCallback( (terminalId: string) => { @@ -1656,7 +1656,6 @@ function WorkspaceScreenContent({ handleTerminalCreateFailed, } = useWorkspaceTerminalTabActions({ persistenceKey, - focusWorkspacePane, openWorkspaceTabFocused, labels: { workspacePathUnavailable: t("workspace.header.toasts.workspacePathUnavailable"), @@ -1951,7 +1950,11 @@ function WorkspaceScreenContent({ }, [isRouteFocused, normalizedServerId, setFocusedAgentId, setFocusedTerminalId]); const openWorkspaceDraftTab = useCallback( - function openWorkspaceDraftTab(input?: { draftId?: string; focus?: boolean }) { + function openWorkspaceDraftTab(input?: { + draftId?: string; + focus?: boolean; + paneId?: string | null; + }) { if (!persistenceKey) { return null; } @@ -1961,10 +1964,11 @@ function WorkspaceScreenContent({ draftId: trimNonEmpty(input?.draftId) ?? generateDraftId(), }); invariant(target?.kind === "draft", "Draft tab target must be valid"); + const placement = { paneId: input?.paneId }; if (input?.focus === false) { - return openWorkspaceTabInBackground(persistenceKey, target); + return openWorkspaceTabInBackground(persistenceKey, target, placement); } - return openWorkspaceTabFocused(persistenceKey, target); + return openWorkspaceTabFocused(persistenceKey, target, placement); }, [openWorkspaceTabFocused, openWorkspaceTabInBackground, persistenceKey], ); @@ -2355,12 +2359,9 @@ function WorkspaceScreenContent({ const handleCreateDraftTab = useCallback( (input?: { paneId?: string }) => { - if (input?.paneId && persistenceKey) { - focusWorkspacePane(persistenceKey, input.paneId); - } - openWorkspaceDraftTab(); + openWorkspaceDraftTab({ paneId: input?.paneId }); }, - [focusWorkspacePane, openWorkspaceDraftTab, persistenceKey], + [openWorkspaceDraftTab], ); const handleCreateTerminal = useStableEvent(createTerminal); @@ -2377,13 +2378,14 @@ function WorkspaceScreenContent({ if (!persistenceKey || !getIsElectron()) { return; } - if (input?.paneId) { - focusWorkspacePane(persistenceKey, input.paneId); - } const { browserId } = createWorkspaceBrowser(); - openWorkspaceTabFocused(persistenceKey, { kind: "browser", browserId }); + openWorkspaceTabFocused( + persistenceKey, + { kind: "browser", browserId }, + { paneId: input?.paneId }, + ); }, - [focusWorkspacePane, openWorkspaceTabFocused, persistenceKey], + [openWorkspaceTabFocused, persistenceKey], ); const handleOpenUrlInBrowserTab = useCallback( @@ -3149,10 +3151,9 @@ function WorkspaceScreenContent({ if (!persistenceKey) { return; } - if (input.focusPaneBeforeOpen && input.paneId) { - focusWorkspacePane(persistenceKey, input.paneId); - } - const tabId = openWorkspaceChildTabFocused(persistenceKey, target, input.tab.tabId); + const tabId = openWorkspaceChildTabFocused(persistenceKey, target, input.tab.tabId, { + paneId: input.focusPaneBeforeOpen ? input.paneId : null, + }); if (tabId) { navigateToTabId(tabId); } @@ -3178,7 +3179,6 @@ function WorkspaceScreenContent({ }), [ handleCloseTabById, - focusWorkspacePane, fileNavigationRevisionByTabId, handleOpenWorkspaceFileFromPane, navigateToTabId, diff --git a/packages/app/src/stores/workspace-layout-actions.ts b/packages/app/src/stores/workspace-layout-actions.ts index 045c9c33f..23d264d1c 100644 --- a/packages/app/src/stores/workspace-layout-actions.ts +++ b/packages/app/src/stores/workspace-layout-actions.ts @@ -103,6 +103,10 @@ interface OpenTabInLayoutInput { layout: WorkspaceLayout; target: WorkspaceTabTarget; now: number; + /** Explicit placement from a pane-local affordance. Wins over every other rule. */ + paneId?: string | null; + /** Required so a new caller cannot silently opt out of the explorer placement rule. */ + explorerPaneId: string | null; } interface OpenTabInLayoutResult { @@ -198,6 +202,7 @@ export interface WorkspaceTabReconcileState { pinnedAgentIds?: ReadonlySet | null; pendingAgentIds?: ReadonlySet | null; hiddenAgentIds?: ReadonlySet | null; + explorerPaneId: string | null; } export interface WorkspaceTabSnapshot { @@ -1119,18 +1124,61 @@ export function removeTabFromTree(root: SplitNode, tabId: string): SplitNode { }).root; } -function insertNewTabIntoFocusedPane(input: { - layout: WorkspaceLayout; +// The explorer pane is a background surface for the PR, files, and assistant output. +// It can hold focus without the user ever having looked at it, so an ambient open of +// something the user works *in* must not land there just because focus leaked. +const EXPLORER_EXCLUDED_TAB_KINDS: ReadonlySet = new Set([ + "agent", + "provider_subagent", + "terminal", + "draft", + "browser", +]); + +function resolvePlacementPane(input: { + layout: { root: SplitNode; focusedPaneId: string | null }; target: WorkspaceTabTarget; - now: number; - focus: boolean; -}): OpenTabInLayoutResult { - const layout = asInternalLayout(input.layout); + paneId: string | null | undefined; + explorerPaneId: string | null; +}): SplitPane { + const requestedPane = findPaneById(input.layout.root, input.paneId); + if (requestedPane) { + return requestedPane; + } + const focusedPane = - findPaneById(layout.root, layout.focusedPaneId) ?? - collectAllPanes(layout.root)[0] ?? + findPaneById(input.layout.root, input.layout.focusedPaneId) ?? + collectAllPanes(input.layout.root)[0] ?? findPaneById(createDefaultLayout().root, DEFAULT_PANE_ID); invariant(focusedPane, "Workspace layout must always have a pane"); + if ( + focusedPane.id !== input.explorerPaneId || + !EXPLORER_EXCLUDED_TAB_KINDS.has(input.target.kind) + ) { + return focusedPane; + } + + // `collectAllPanes` skips hidden panes, so the chain falls back to the explorer + // pane itself only when it is the sole visible pane. The explorer is the only pane + // anything ever hides, so today that means "the explorer is the only pane". + const panes = collectAllPanes(input.layout.root); + return ( + panes.find((pane) => pane.id === DEFAULT_PANE_ID && pane.id !== focusedPane.id) ?? + panes.find((pane) => pane.id !== focusedPane.id) ?? + focusedPane + ); +} + +function insertNewTabIntoPane( + input: OpenTabInLayoutInput & { focus: boolean }, +): OpenTabInLayoutResult { + const layout = asInternalLayout(input.layout); + const targetPane = resolvePlacementPane({ + layout, + target: input.target, + paneId: input.paneId, + explorerPaneId: input.explorerPaneId, + }); const tabId = buildDeterministicWorkspaceTabId(input.target); const nextTab: WorkspaceTab = { @@ -1139,17 +1187,17 @@ function insertNewTabIntoFocusedPane(input: { createdAt: input.now, }; - const preservedFocusTabId = focusedPane.focusedTabId ?? tabId; + const preservedFocusTabId = targetPane.focusedTabId ?? tabId; return { tabId, layout: withNormalizedParentTabMap({ root: insertTabIntoPane(layout.root, { - paneId: focusedPane.id, + paneId: targetPane.id, tab: nextTab, focusTabId: input.focus ? tabId : preservedFocusTabId, }), - focusedPaneId: input.focus ? focusedPane.id : layout.focusedPaneId, + focusedPaneId: input.focus ? targetPane.id : layout.focusedPaneId, parentTabIdByTabId: input.layout.parentTabIdByTabId, }), }; @@ -1197,7 +1245,7 @@ export function openTabInLayoutFocused(input: OpenTabInLayoutInput): OpenTabInLa }; } - return insertNewTabIntoFocusedPane({ ...input, focus: true }); + return insertNewTabIntoPane({ ...input, focus: true }); } export function openTabInLayoutBackground(input: OpenTabInLayoutInput): OpenTabInLayoutResult { @@ -1210,7 +1258,7 @@ export function openTabInLayoutBackground(input: OpenTabInLayoutInput): OpenTabI }; } - return insertNewTabIntoFocusedPane({ ...input, focus: false }); + return insertNewTabIntoPane({ ...input, focus: false }); } export function closeTabInLayout(input: CloseTabInLayoutInput): WorkspaceLayout | null { @@ -1719,31 +1767,18 @@ function isTerminalTab( return tab.target.kind === "terminal"; } -function openEntityTabWithoutFocusing( - layout: WorkspaceLayout, - target: WorkspaceTabTarget, -): WorkspaceLayout { - const internalLayout = asInternalLayout(layout); - const focusedPane = - findPaneById(internalLayout.root, internalLayout.focusedPaneId) ?? - collectAllPanes(internalLayout.root)[0] ?? - findPaneById(createDefaultLayout().root, DEFAULT_PANE_ID); - invariant(focusedPane, "Workspace layout must always have a pane"); - - const tabId = buildDeterministicWorkspaceTabId(target); - return withNormalizedParentTabMap({ - root: insertTabIntoPane(internalLayout.root, { - paneId: focusedPane.id, - tab: { - tabId, - target, - createdAt: Date.now(), - }, - focusTabId: focusedPane.focusedTabId ?? tabId, - }), - focusedPaneId: internalLayout.focusedPaneId, - parentTabIdByTabId: layout.parentTabIdByTabId, - }); +function openEntityTabWithoutFocusing(input: { + layout: WorkspaceLayout; + target: WorkspaceTabTarget; + explorerPaneId: string | null; +}): WorkspaceLayout { + return insertNewTabIntoPane({ + layout: input.layout, + target: input.target, + now: Date.now(), + explorerPaneId: input.explorerPaneId, + focus: false, + }).layout; } interface EntityTabGroup { @@ -1832,12 +1867,14 @@ function addMissingEntityTabs(input: { representedAgentIds: Set; standaloneTerminalIds: Set; hasActivePendingDraftCreate: boolean; + explorerPaneId: string | null; }): WorkspaceLayout { const { autoOpenAgentIds, representedAgentIds, standaloneTerminalIds, hasActivePendingDraftCreate, + explorerPaneId, } = input; let nextLayout = input.layout; const currentEntityTabs = collectAllTabs(nextLayout.root); @@ -1856,9 +1893,10 @@ function addMissingEntityTabs(input: { if (hasActivePendingDraftCreate && !representedAgentIds.has(agentId)) { continue; } - nextLayout = openEntityTabWithoutFocusing(nextLayout, { - kind: "agent", - agentId, + nextLayout = openEntityTabWithoutFocusing({ + layout: nextLayout, + target: { kind: "agent", agentId }, + explorerPaneId, }); currentAgentIds.add(agentId); } @@ -1868,9 +1906,10 @@ function addMissingEntityTabs(input: { if (currentTerminalIds.has(terminalId)) { continue; } - nextLayout = openEntityTabWithoutFocusing(nextLayout, { - kind: "terminal", - terminalId, + nextLayout = openEntityTabWithoutFocusing({ + layout: nextLayout, + target: { kind: "terminal", terminalId }, + explorerPaneId, }); currentTerminalIds.add(terminalId); } @@ -1961,6 +2000,7 @@ export function reconcileWorkspaceTabs( representedAgentIds, standaloneTerminalIds, hasActivePendingDraftCreate: snapshot.hasActivePendingDraftCreate ?? false, + explorerPaneId: state.explorerPaneId, }); if (reconciledFocusedTabId) { diff --git a/packages/app/src/stores/workspace-layout-store.test.ts b/packages/app/src/stores/workspace-layout-store.test.ts index e43672f3d..78c9816e4 100644 --- a/packages/app/src/stores/workspace-layout-store.test.ts +++ b/packages/app/src/stores/workspace-layout-store.test.ts @@ -493,7 +493,7 @@ describe("workspace-layout-store actions", () => { ]); }); - it("reveals the registered explorer pane for a first detected pull request", () => { + it("adds a first detected pull request to the explorer pane without revealing or focusing it", () => { const workspaceKey = createWorkspaceKey(); const store = workspaceLayoutStore.getState(); @@ -503,8 +503,172 @@ describe("workspace-layout-store actions", () => { const layout = state.layoutByWorkspace[workspaceKey]; const explorerPaneId = state.explorerPaneIdByWorkspace[workspaceKey]; expect(explorerPaneId).toBe("explorer"); - expect(findPaneById(layout.root, explorerPaneId)?.hidden).toBeUndefined(); expect(findPaneContainingTab(layout.root, "pull_request")?.id).toBe(explorerPaneId); + expect(findPaneById(layout.root, explorerPaneId)?.hidden).toBe(true); + expect(layout.focusedPaneId).toBe("main"); + expect(state.acknowledgedPullRequestByWorkspace[workspaceKey]).toBe( + "url:https://example.test/pulls/1", + ); + }); + + it("leaves a pull request tab the user opened elsewhere where it lives", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + const pullRequestTabId = store.openTabFocused( + workspaceKey, + { kind: "pull_request" }, + { paneId: "main" }, + ); + const before = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + + store.observePullRequest(workspaceKey, "url:https://example.test/pulls/1"); + + const state = workspaceLayoutStore.getState(); + const layout = state.layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, pullRequestTabId as string)?.id).toBe("main"); + expect(collectAllTabs(layout.root).map((tab) => tab.tabId)).toEqual([pullRequestTabId]); + expect(findPaneById(layout.root, "explorer")?.hidden).toBe(true); + expect(layout.focusedPaneId).toBe(before.focusedPaneId); + expect(state.acknowledgedPullRequestByWorkspace[workspaceKey]).toBe( + "url:https://example.test/pulls/1", + ); + }); + + it("leaves an explorer-pane background tab in the only other pane instead of emptying it", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + const setupTabId = store.openTabFocused( + workspaceKey, + { kind: "setup", workspaceId: WORKSPACE_ID }, + { paneId: "main" }, + ); + + store.openTabInExplorerPaneBackground(workspaceKey, { + kind: "setup", + workspaceId: WORKSPACE_ID, + }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, setupTabId as string)?.id).toBe("main"); + expect(findPaneById(layout.root, "main")).toBeTruthy(); + expect(findPaneById(layout.root, "explorer")?.hidden).toBe(true); + expect(layout.focusedPaneId).toBe("main"); + }); + + it("keeps ambient entity tabs out of the explorer pane after a pull request is detected", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + + store.observePullRequest(workspaceKey, "url:https://example.test/pulls/1"); + store.focusPane(workspaceKey, "explorer"); + const agentTabId = store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(agentTabId).toBeTruthy(); + expect(findPaneContainingTab(layout.root, agentTabId as string)?.id).toBe("main"); + expect(layout.focusedPaneId).toBe("main"); + }); + + it("keeps a background entity tab out of the focused explorer pane without moving focus", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + store.focusPane(workspaceKey, "explorer"); + + const terminalTabId = store.openTabInBackground(workspaceKey, { + kind: "terminal", + terminalId: "terminal-1", + }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, terminalTabId as string)?.id).toBe("main"); + expect(layout.focusedPaneId).toBe("explorer"); + }); + + it("honours an explicit pane id over the explorer reroute", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + + const draftTabId = store.openTabFocused( + workspaceKey, + { kind: "draft", draftId: "draft-1" }, + { paneId: "explorer" }, + ); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, draftTabId as string)?.id).toBe("explorer"); + expect(layout.focusedPaneId).toBe("explorer"); + }); + + it("keeps non-entity tabs in the focused explorer pane", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + store.focusPane(workspaceKey, "explorer"); + + const filesTabId = store.openTabFocused(workspaceKey, { kind: "files" }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, filesTabId as string)?.id).toBe("explorer"); + }); + + it("focuses an entity tab the user dragged into the explorer pane where it lives", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + const agentTabId = store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + store.showPane(workspaceKey, "explorer"); + store.moveTabToPane(workspaceKey, agentTabId as string, "explorer"); + store.focusPane(workspaceKey, "main"); + + store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, agentTabId as string)?.id).toBe("explorer"); + expect(layout.focusedPaneId).toBe("explorer"); + }); + + it("keeps reconcile auto-opened agents out of the focused explorer pane", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + store.focusPane(workspaceKey, "explorer"); + + store.reconcileTabs(workspaceKey, { + agentsHydrated: true, + terminalsHydrated: true, + activeAgentIds: ["agent-1", "agent-2"], + autoOpenAgentIds: ["agent-1", "agent-2"], + knownAgentIds: ["agent-1", "agent-2"], + standaloneTerminalIds: [], + }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, "agent_agent-2")?.id).toBe("main"); + expect(layout.focusedPaneId).toBe("explorer"); + }); + + it("opens an entity tab in the explorer pane when it is the only pane", () => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + workspaceLayoutStore.setState((state) => ({ + layoutByWorkspace: { + ...state.layoutByWorkspace, + [workspaceKey]: normalizeLayout({ + root: createPane({ id: "explorer", tabIds: [] }), + focusedPaneId: "explorer", + }), + }, + explorerPaneIdByWorkspace: { + ...state.explorerPaneIdByWorkspace, + [workspaceKey]: "explorer", + }, + })); + + const agentTabId = store.openTabFocused(workspaceKey, { kind: "agent", agentId: "agent-1" }); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect(findPaneContainingTab(layout.root, agentTabId as string)?.id).toBe("explorer"); }); it("creates setup in the hidden explorer pane without changing focus", () => { diff --git a/packages/app/src/stores/workspace-layout-store.ts b/packages/app/src/stores/workspace-layout-store.ts index 282670d62..7515cb86a 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -81,13 +81,22 @@ interface WorkspaceLayoutStore { focusRestorationByWorkspace: Record; explorerPaneIdByWorkspace: Record; acknowledgedPullRequestByWorkspace: Record; - openTabFocused: (workspaceKey: string, target: WorkspaceTabTarget) => string | null; + openTabFocused: ( + workspaceKey: string, + target: WorkspaceTabTarget, + options?: WorkspaceTabOpenOptions, + ) => string | null; openChildTabFocused: ( workspaceKey: string, target: WorkspaceTabTarget, parentTabId: string, + options?: WorkspaceTabOpenOptions, + ) => string | null; + openTabInBackground: ( + workspaceKey: string, + target: WorkspaceTabTarget, + options?: WorkspaceTabOpenOptions, ) => string | null; - openTabInBackground: (workspaceKey: string, target: WorkspaceTabTarget) => string | null; openTabInExplorerPaneBackground: ( workspaceKey: string, target: WorkspaceTabTarget, @@ -137,6 +146,15 @@ interface WorkspaceLayoutStore { purgeWorkspace: (workspaceKey: string) => void; } +/** + * `paneId` states which pane the user acted in — the `+` menu of a pane's tab rail, + * a subagent link inside a pane. Without it the open is ambient (sidebar click, + * reconcile auto-open) and the store decides the pane. + */ +export interface WorkspaceTabOpenOptions { + paneId?: string | null; +} + interface EnsureExplorerPaneResult { paneId: string; created: boolean; @@ -350,6 +368,19 @@ function ensurePersistedExplorerPane(input: { return { layout: hiddenLayout ?? split.layout, paneId: split.paneId }; } +function getOpenTabPlacement( + state: WorkspaceLayoutStore, + workspaceKey: string, + options: WorkspaceTabOpenOptions | undefined, +): { layout: WorkspaceLayout; paneId: string | null; explorerPaneId: string | null } { + const layout = getWorkspaceLayout(state.layoutByWorkspace, workspaceKey); + return { + layout, + paneId: trimNonEmpty(options?.paneId), + explorerPaneId: resolveExplorerPaneId(layout, state.explorerPaneIdByWorkspace[workspaceKey]), + }; +} + function withoutFocusRestoration( state: WorkspaceLayoutStore, workspaceKey: string, @@ -401,7 +432,7 @@ export function createWorkspaceLayoutStore( focusRestorationByWorkspace: {}, explorerPaneIdByWorkspace: {}, acknowledgedPullRequestByWorkspace: {}, - openTabFocused: (workspaceKey, target) => { + openTabFocused: (workspaceKey, target, options) => { const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); const normalizedTarget = normalizeWorkspaceTabTarget(target); if (!normalizedWorkspaceKey || !normalizedTarget) { @@ -409,7 +440,7 @@ export function createWorkspaceLayoutStore( } const result = openTabInLayoutFocused({ - layout: getWorkspaceLayout(get().layoutByWorkspace, normalizedWorkspaceKey), + ...getOpenTabPlacement(get(), normalizedWorkspaceKey, options), target: normalizedTarget, now: Date.now(), }); @@ -432,7 +463,7 @@ export function createWorkspaceLayoutStore( return result.tabId; }, - openChildTabFocused: (workspaceKey, target, parentTabId) => { + openChildTabFocused: (workspaceKey, target, parentTabId, options) => { const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); const normalizedParentTabId = trimNonEmpty(parentTabId); const normalizedTarget = normalizeWorkspaceTabTarget(target); @@ -441,7 +472,7 @@ export function createWorkspaceLayoutStore( } const result = openTabInLayoutFocused({ - layout: getWorkspaceLayout(get().layoutByWorkspace, normalizedWorkspaceKey), + ...getOpenTabPlacement(get(), normalizedWorkspaceKey, options), target: normalizedTarget, now: Date.now(), }); @@ -471,7 +502,7 @@ export function createWorkspaceLayoutStore( return result.tabId; }, - openTabInBackground: (workspaceKey, target) => { + openTabInBackground: (workspaceKey, target, options) => { const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); const normalizedTarget = normalizeWorkspaceTabTarget(target); if (!normalizedWorkspaceKey || !normalizedTarget) { @@ -479,7 +510,7 @@ export function createWorkspaceLayoutStore( } const result = openTabInLayoutBackground({ - layout: getWorkspaceLayout(get().layoutByWorkspace, normalizedWorkspaceKey), + ...getOpenTabPlacement(get(), normalizedWorkspaceKey, options), target: normalizedTarget, now: Date.now(), }); @@ -520,47 +551,26 @@ export function createWorkspaceLayoutStore( return state; } - const originalFocusedPaneId = layout.focusedPaneId; - const revealedLayout = focusPaneInLayout({ layout, paneId: explorerPane.id }) ?? layout; + // A new tab is placed straight into the explorer pane, hidden or not. + // A tab already open elsewhere stays where the user put it — this open + // must not move, reveal, or focus anything. const result = openTabInLayoutBackground({ - layout: revealedLayout, + layout, target: normalizedTarget, now: Date.now(), + paneId: explorerPane.id, + explorerPaneId: explorerPane.id, }); - const currentPane = findPaneContainingTab(result.layout.root, result.tabId); - const placedLayout = - currentPane?.id === explorerPane.id - ? result.layout - : (moveTabToPaneInLayout({ - layout: result.layout, - tabId: result.tabId, - toPaneId: explorerPane.id, - }) ?? result.layout); - const restoredVisibility = explorerPane.hidden - ? (setPaneHiddenInLayout({ - layout: placedLayout, - paneId: explorerPane.id, - hidden: true, - }) ?? placedLayout) - : placedLayout; - const restoredLayout = - originalFocusedPaneId && originalFocusedPaneId !== explorerPane.id - ? (focusPaneInLayout({ - layout: restoredVisibility, - paneId: originalFocusedPaneId, - }) ?? restoredVisibility) - : restoredVisibility; tabId = result.tabId; return { - ...withoutFocusRestoration(state, normalizedWorkspaceKey), explorerPaneIdByWorkspace: { ...state.explorerPaneIdByWorkspace, [normalizedWorkspaceKey]: explorerPane.id, }, layoutByWorkspace: { ...state.layoutByWorkspace, - [normalizedWorkspaceKey]: restoredLayout, + [normalizedWorkspaceKey]: result.layout, }, }; }); @@ -616,9 +626,15 @@ export function createWorkspaceLayoutStore( return null; } const parentTabId = trimNonEmpty(input.parentTabId); + const placement = { paneId: explorerPane.paneId }; const tabId = parentTabId - ? get().openChildTabFocused(normalizedWorkspaceKey, normalizedTarget, parentTabId) - : get().openTabFocused(normalizedWorkspaceKey, normalizedTarget); + ? get().openChildTabFocused( + normalizedWorkspaceKey, + normalizedTarget, + parentTabId, + placement, + ) + : get().openTabFocused(normalizedWorkspaceKey, normalizedTarget, placement); if (!tabId) { return null; } @@ -637,47 +653,26 @@ export function createWorkspaceLayoutStore( return; } - set((state) => { - if ( - state.acknowledgedPullRequestByWorkspace[normalizedWorkspaceKey] === - normalizedIdentity - ) { - return state; - } - - const layout = getWorkspaceLayout(state.layoutByWorkspace, normalizedWorkspaceKey); - const explorerPaneId = resolveExplorerPaneId( - layout, - state.explorerPaneIdByWorkspace[normalizedWorkspaceKey], - ); - const explorerPane = findPaneById(layout.root, explorerPaneId); - if (!explorerPane) { - return state; - } - const placementLayout = - focusPaneInLayout({ layout, paneId: explorerPane.id }) ?? layout; - const result = openTabInLayoutFocused({ - layout: placementLayout, - target: { kind: "pull_request" }, - now: Date.now(), - }); - - return { - ...withoutFocusRestoration(state, normalizedWorkspaceKey), - acknowledgedPullRequestByWorkspace: { - ...state.acknowledgedPullRequestByWorkspace, - [normalizedWorkspaceKey]: normalizedIdentity, - }, - explorerPaneIdByWorkspace: { - ...state.explorerPaneIdByWorkspace, - [normalizedWorkspaceKey]: explorerPane.id, - }, - layoutByWorkspace: { - ...state.layoutByWorkspace, - [normalizedWorkspaceKey]: result.layout, - }, - }; + if ( + get().acknowledgedPullRequestByWorkspace[normalizedWorkspaceKey] === normalizedIdentity + ) { + return; + } + // A detected pull request is a background add: the tab appears in the + // explorer pane, but nothing is revealed and focus never moves. + const tabId = get().openTabInExplorerPaneBackground(normalizedWorkspaceKey, { + kind: "pull_request", }); + if (!tabId) { + return; + } + + set((state) => ({ + acknowledgedPullRequestByWorkspace: { + ...state.acknowledgedPullRequestByWorkspace, + [normalizedWorkspaceKey]: normalizedIdentity, + }, + })); }, closeTab: (workspaceKey, tabId) => { const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); @@ -828,6 +823,10 @@ export function createWorkspaceLayoutStore( pinnedAgentIds: state.pinnedAgentIdsByWorkspace[normalizedWorkspaceKey] ?? null, pendingAgentIds: state.pendingAgentIdsByWorkspace[normalizedWorkspaceKey] ?? null, hiddenAgentIds: state.hiddenAgentIdsByWorkspace[normalizedWorkspaceKey] ?? null, + explorerPaneId: resolveExplorerPaneId( + currentLayout, + state.explorerPaneIdByWorkspace[normalizedWorkspaceKey], + ), }, snapshot, );