From f7dc069ae5fc5716bb3c259ffc382b4b4b482e8f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 14 Sep 2026 23:31:25 +0200 Subject: [PATCH] fix(app): prevent and recover workspace layout crash loops --- docs/explorer-sidebar.md | 7 +- .../changes-shortcut-placement.spec.ts | 3 +- .../browser/composer-control-density.spec.ts | 1 + ...t-terminal-shortcut-pane-placement.spec.ts | 7 +- .../e2e/browser/empty-pane-keyboard.spec.ts | 3 +- .../browser/explorer-pane-placement.spec.ts | 120 ++++++- packages/app/e2e/browser/launcher-tab.spec.ts | 1 + .../workspace-navigation-regression.spec.ts | 11 +- .../workspace/workspace-desktop-tabs-row.tsx | 3 + .../src/stores/workspace-layout-actions.ts | 142 ++++---- .../src/stores/workspace-layout-store.test.ts | 326 ++++++++++++------ .../app/src/stores/workspace-layout-store.ts | 42 ++- packages/desktop/e2e/packaged-app-smoke.js | 1 + 13 files changed, 477 insertions(+), 190 deletions(-) diff --git a/docs/explorer-sidebar.md b/docs/explorer-sidebar.md index 379db5a31f..679f212312 100644 --- a/docs/explorer-sidebar.md +++ b/docs/explorer-sidebar.md @@ -57,8 +57,11 @@ lifecycle. layout store remembers one ordinary pane per workspace. The first side open creates a full-height right split around the workspace root; later side opens reuse it. -Closing the pane or moving away its final tab removes it normally and clears the remembered id. A -later side open creates a new pane. There is no hidden side-pane lifecycle. +Removing a pane clears its remembered id; a later side open creates a new pane. The last visible +ordinary pane stays when its final tab closes and shows the New launcher. An empty workspace does +not automatically create an agent draft tab; choosing Agent opens one. Explorer cannot replace the +workspace canvas, even when visible. Restoring a saved layout enforces the same rule while preserving Explorer and saved +tab content. There is no hidden side-pane lifecycle. Placement intent still controls existing tabs: diff --git a/packages/app/e2e/browser/changes-shortcut-placement.spec.ts b/packages/app/e2e/browser/changes-shortcut-placement.spec.ts index 214a420e8f..a27880a111 100644 --- a/packages/app/e2e/browser/changes-shortcut-placement.spec.ts +++ b/packages/app/e2e/browser/changes-shortcut-placement.spec.ts @@ -1,5 +1,5 @@ import { expect, test } from "../support/fixtures"; -import { gotoWorkspace } from "../support/helpers/launcher"; +import { clickNewChat, gotoWorkspace } from "../support/helpers/launcher"; import { seedWorkspace } from "../support/helpers/seed-client"; import { waitForWorkspaceTabsVisible } from "../support/helpers/workspace-tabs"; @@ -12,6 +12,7 @@ test("Changes shortcut reveals the Changes tree in Explorer", async ({ page }) = await page.setViewportSize({ width: 1400, height: 900 }); await gotoWorkspace(page, workspace.workspaceId); await waitForWorkspaceTabsVisible(page); + await clickNewChat(page); await page.keyboard.press(CHANGES_SHORTCUT); diff --git a/packages/app/e2e/browser/composer-control-density.spec.ts b/packages/app/e2e/browser/composer-control-density.spec.ts index 95eb658cdf..9c81037526 100644 --- a/packages/app/e2e/browser/composer-control-density.spec.ts +++ b/packages/app/e2e/browser/composer-control-density.spec.ts @@ -70,6 +70,7 @@ test.describe("Composer control density across tab switches", () => { }); await gotoWorkspace(page, workspace.workspaceId); await clickNewChat(page); + await clickNewChat(page); const draftTabs = page .locator('[data-testid^="workspace-tab-draft"]') diff --git a/packages/app/e2e/browser/direct-terminal-shortcut-pane-placement.spec.ts b/packages/app/e2e/browser/direct-terminal-shortcut-pane-placement.spec.ts index 194be40ef9..0c3787f46d 100644 --- a/packages/app/e2e/browser/direct-terminal-shortcut-pane-placement.spec.ts +++ b/packages/app/e2e/browser/direct-terminal-shortcut-pane-placement.spec.ts @@ -1,6 +1,10 @@ import { expect, test } from "../support/fixtures"; import { runWorkspaceActionFromCommandCenter } from "../support/helpers/command-center-workspace-actions"; -import { gotoWorkspace, pressDirectNewTabShortcut } from "../support/helpers/launcher"; +import { + clickNewChat, + gotoWorkspace, + pressDirectNewTabShortcut, +} from "../support/helpers/launcher"; import { seedWorkspace } from "../support/helpers/seed-client"; import { waitForWorkspaceTabsVisible } from "../support/helpers/workspace-tabs"; @@ -11,6 +15,7 @@ test.describe("Direct terminal shortcut pane placement", () => { try { await gotoWorkspace(page, workspace.workspaceId); await waitForWorkspaceTabsVisible(page); + await clickNewChat(page); await runWorkspaceActionFromCommandCenter(page, "Split pane right"); const focusedPaneChild = page diff --git a/packages/app/e2e/browser/empty-pane-keyboard.spec.ts b/packages/app/e2e/browser/empty-pane-keyboard.spec.ts index 0a90da72d8..96aac4d0d7 100644 --- a/packages/app/e2e/browser/empty-pane-keyboard.spec.ts +++ b/packages/app/e2e/browser/empty-pane-keyboard.spec.ts @@ -1,7 +1,7 @@ import { expect, type Locator, type Page } from "@playwright/test"; import { test } from "../support/fixtures"; import { runWorkspaceActionFromCommandCenter } from "../support/helpers/command-center-workspace-actions"; -import { gotoWorkspace } from "../support/helpers/launcher"; +import { clickNewChat, gotoWorkspace } from "../support/helpers/launcher"; import { seedWorkspace } from "../support/helpers/seed-client"; import { waitForWorkspaceTabsVisible } from "../support/helpers/workspace-tabs"; @@ -43,6 +43,7 @@ test.describe("New tab keyboard launcher", () => { try { await gotoWorkspace(page, workspace.workspaceId); await waitForWorkspaceTabsVisible(page); + await clickNewChat(page); await runWorkspaceActionFromCommandCenter(page, "Split pane right"); const launcher = visibleNewTabPanel(page); diff --git a/packages/app/e2e/browser/explorer-pane-placement.spec.ts b/packages/app/e2e/browser/explorer-pane-placement.spec.ts index 750a2f3dc5..b407d7673c 100644 --- a/packages/app/e2e/browser/explorer-pane-placement.spec.ts +++ b/packages/app/e2e/browser/explorer-pane-placement.spec.ts @@ -107,9 +107,10 @@ test.describe("explorer pane tab placement", () => { let agentId = ""; try { - await test.step("empty workspace seeds a draft in the main pane", async () => { + await test.step("choose Agent from the empty workspace launcher", async () => { await gotoWorkspace(page, workspace.workspaceId); await waitForWorkspaceTabsVisible(page); + await openAgentDraftFromLauncher(page); await expect(draftTabChip(page).first()).toBeVisible({ timeout: 30_000 }); }); @@ -222,3 +223,120 @@ test.describe("explorer pane tab placement", () => { } }); }); + +async function closeOnlyDraft(page: Page): Promise { + await draftTabChip(page).hover(); + await page.getByRole("button", { name: "Close", exact: true }).click(); +} + +async function moveOnlyDraftIntoRightSplit(page: Page): Promise { + await page.getByRole("button", { name: "More actions", exact: true }).click(); + await page.getByRole("menuitem", { name: "Split pane right", exact: true }).click(); + const target = await emptyPaneBox(page); + await dragChipTo(page, draftTabChip(page), { + x: target.x + target.width / 2, + y: target.y + target.height / 2, + }); + await expect(visible(page, "workspace-tabs-row")).toHaveCount(1); +} + +async function expectNewLauncher(page: Page): Promise { + await expect( + page.getByTestId("workspace-new-tab-panel").getByRole("button", { name: "Agent", exact: true }), + ).toBeVisible(); + await expect(draftTabChip(page)).toHaveCount(0); + await expect(page.getByRole("textbox", { name: "Message agent..." })).toHaveCount(0); + await expect(page.getByText("Paseo ran into a problem.", { exact: true })).toHaveCount(0); +} + +async function openAgentDraftFromLauncher(page: Page): Promise { + await expectNewLauncher(page); + await page + .getByTestId("workspace-new-tab-panel") + .getByRole("button", { name: "Agent", exact: true }) + .click(); + await expect(page.getByRole("textbox", { name: "Message agent..." })).toBeVisible(); +} + +async function persistHiddenGeneratedExplorer(page: Page): Promise { + await page.evaluate(() => { + const stored = JSON.parse(localStorage.getItem("workspace-layout-state")!); + const key = Object.keys(stored.state.layoutByWorkspace)[0]; + const tabs = [ + { tabId: "files", target: { kind: "files" }, createdAt: 1 }, + { tabId: "changes_tree", target: { kind: "changes_tree" }, createdAt: 1 }, + ...Array.from({ length: 2250 }, (_, index) => ({ + tabId: `draft_saved_${index}`, + target: { kind: "draft", draftId: `draft_saved_${index}` }, + createdAt: index + 2, + })), + ]; + stored.state.layoutByWorkspace[key] = { + root: { + kind: "pane", + pane: { + id: "pane_generated_report_equivalent", + hidden: true, + tabIds: tabs.map((tab) => tab.tabId), + tabs, + focusedTabId: tabs[tabs.length - 1].tabId, + }, + }, + focusedPaneId: null, + }; + stored.state.explorerPaneIdByWorkspace[key] = "pane_generated_report_equivalent"; + localStorage.setItem("workspace-layout-state", JSON.stringify(stored)); + }); +} + +// Explorer cannot replace the ordinary workspace canvas, including on restore. +test("closing the last split-born tab with hidden Explorer keeps a usable workspace", async ({ + page, +}) => { + const workspace = await seedWorkspace({ repoPrefix: "last-pane-hidden-explorer-" }); + try { + await gotoWorkspace(page, workspace.workspaceId); + await openAgentDraftFromLauncher(page); + await closeOnlyDraft(page); + await expectNewLauncher(page); + await openAgentDraftFromLauncher(page); + await moveOnlyDraftIntoRightSplit(page); + await closeOnlyDraft(page); + await expectNewLauncher(page); + await page.reload(); + await expectNewLauncher(page); + } finally { + await workspace.cleanup(); + } +}); + +test("visible Explorer does not replace the last ordinary workspace pane", async ({ page }) => { + const workspace = await seedWorkspace({ repoPrefix: "last-pane-visible-explorer-" }); + try { + await gotoWorkspace(page, workspace.workspaceId); + await page.getByRole("button", { name: "Open Explorer sidebar", exact: true }).click(); + await openAgentDraftFromLauncher(page); + await moveOnlyDraftIntoRightSplit(page); + await closeOnlyDraft(page); + await expectNewLauncher(page); + await page.reload(); + await expectNewLauncher(page); + } finally { + await workspace.cleanup(); + } +}); + +test("reloading a saved hidden generated Explorer recovers a usable workspace", async ({ + page, +}) => { + const workspace = await seedWorkspace({ repoPrefix: "saved-hidden-explorer-" }); + try { + await gotoWorkspace(page, workspace.workspaceId); + await expectNewLauncher(page); + await persistHiddenGeneratedExplorer(page); + await page.reload(); + await expectNewLauncher(page); + } finally { + await workspace.cleanup(); + } +}); diff --git a/packages/app/e2e/browser/launcher-tab.spec.ts b/packages/app/e2e/browser/launcher-tab.spec.ts index 9e2dc9de57..a9c1e9c43a 100644 --- a/packages/app/e2e/browser/launcher-tab.spec.ts +++ b/packages/app/e2e/browser/launcher-tab.spec.ts @@ -156,6 +156,7 @@ test.describe("Tab creation", () => { const newTabs = page .locator('[data-testid^="workspace-tab-tab_"]') .filter({ hasText: "New tab" }); + await expect(newTabs.first()).toBeVisible(); const countBefore = await newTabs.count(); await pressNewTabShortcut(page); diff --git a/packages/app/e2e/browser/workspace-navigation-regression.spec.ts b/packages/app/e2e/browser/workspace-navigation-regression.spec.ts index 08a4604aec..84cc68dd14 100644 --- a/packages/app/e2e/browser/workspace-navigation-regression.spec.ts +++ b/packages/app/e2e/browser/workspace-navigation-regression.spec.ts @@ -11,6 +11,7 @@ import { expectWorkspaceTabVisible, openWorkspaceWithAgents, } from "../support/helpers/archive-tab"; +import { clickNewChat } from "../support/helpers/launcher"; import { expectComposerVisible } from "../support/helpers/composer"; import { daemonWsRoutePattern } from "../support/helpers/daemon-port"; import { seedWorkspace } from "../support/helpers/seed-client"; @@ -132,13 +133,14 @@ test.describe("Workspace navigation regression", () => { await expect(page.getByText("Add a project", { exact: true })).toHaveCount(0); }); - test("keeps one replacement draft after returning from settings and closing the last tab", async ({ + test("shows the New launcher after returning from settings and closing the last agent draft tab", async ({ page, withWorkspace, }) => { const workspace = await withWorkspace({ prefix: "workspace-settings-back-tab-" }); await workspace.navigateTo(); + await clickNewChat(page); await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(1); await openSettings(page); @@ -148,7 +150,12 @@ test.describe("Workspace navigation regression", () => { await closeFirstVisibleDraftTab(page); - await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(1); + await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(0); + await expect( + page + .getByTestId("workspace-new-tab-panel") + .getByRole("button", { name: "Agent", exact: true }), + ).toBeVisible(); }); test("keeps the workspace rendered while reconnecting to the host", async ({ page }) => { 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 5c183dc94c..d1a8f8bf11 100644 --- a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx +++ b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx @@ -740,6 +740,7 @@ function TabChip({ dragHandleProps: DraggableListDragHandleProps | undefined; }) { const { closeButtonTestId, contextMenuTestId, menuEntries } = resolvedTab; + const { t } = useTranslation(); const middleClickRef = useMiddleClickClose( useCallback(() => void onCloseTab(tab.tabId), [onCloseTab, tab.tabId]), ); @@ -889,6 +890,8 @@ function TabChip({ { createTabId: () => string; state?: JsonValue; } @@ -161,6 +164,7 @@ interface RetargetTabInLayoutResult { } interface ConvertDraftToAgentInLayoutInput { + explorerSidebarPaneId: string | null; layout: WorkspaceLayout; tabId: string; agentId: string; @@ -179,7 +183,7 @@ interface ReorderFocusedPaneTabsInLayoutInput { interface CloseTabInLayoutInput { layout: WorkspaceLayout; tabId: string; - preserveEmptyPaneId?: string | null; + explorerSidebarPaneId: string | null; } interface ClosePaneInLayoutInput { @@ -1157,14 +1161,54 @@ function restoreEmptyPanesInNode( }); } -export function restoreEmptyPanesInLayout( +function findOrdinaryPane( + root: SplitNodeInternal, + explorerSidebarPaneId: string | null, +): SplitPaneInternal | null { + if (root.kind === "pane") return root.pane.id === explorerSidebarPaneId ? null : root.pane; + for (const child of root.group.children) { + const pane = findOrdinaryPane(child, explorerSidebarPaneId); + if (pane) return pane; + } + return null; +} + +/** Restores the workspace canvas and empty tabs without changing Explorer or saved content. */ +export function restoreWorkspaceLayout( layout: WorkspaceLayout, - explorerSidebarPaneId: string | null = null, + explorerSidebarPaneId: string | null, + createNodeId = defaultWorkspaceLayoutIds.createNodeId, ): WorkspaceLayout { const normalized = normalizeLayout(layout); + let root = asInternalNode(normalized.root); + let focusedPaneId = normalized.focusedPaneId; + if (!collectAllPanes(root).some((pane) => pane.id !== explorerSidebarPaneId)) { + const ordinaryPane = findOrdinaryPane(root, explorerSidebarPaneId); + if (ordinaryPane) { + root = updatePaneInTree(root, { + paneId: ordinaryPane.id, + updater: (pane) => ({ ...pane, hidden: false }), + }); + focusedPaneId = ordinaryPane.id; + } else { + // Explorer may have a generated persisted id; its role, not its spelling, + // determines whether it can serve as the workspace canvas. + focusedPaneId = DEFAULT_PANE_ID; + if (explorerSidebarPaneId === DEFAULT_PANE_ID) { + focusedPaneId = createNodeId("pane"); + } + root = createGroupNode({ + id: createNodeId("group"), + direction: "horizontal", + children: [createPaneNode({ id: focusedPaneId, tabs: [] }), root], + sizes: [0.78, 0.22], + }); + } + } return { ...normalized, - root: restoreEmptyPanesInNode(asInternalNode(normalized.root), explorerSidebarPaneId), + focusedPaneId, + root: restoreEmptyPanesInNode(root, explorerSidebarPaneId), }; } @@ -1470,8 +1514,11 @@ export function closeTabInLayout(input: CloseTabInLayoutInput): WorkspaceLayout return null; } const preserveEmptyPaneId = - input.preserveEmptyPaneId ?? - (pane.id === DEFAULT_PANE_ID || pane.id === EXPLORER_SIDEBAR_PANE_ID ? pane.id : null); + pane.id === DEFAULT_PANE_ID || + pane.id === input.explorerSidebarPaneId || + isLastVisibleOrdinaryPane(input.layout, pane.id, input.explorerSidebarPaneId) + ? pane.id + : null; const closeSuccessorTabId = getCloseSuccessorTabId({ pane, @@ -1514,6 +1561,17 @@ export function closeTabInLayout(input: CloseTabInLayoutInput): WorkspaceLayout return nextLayoutWithParentMap; } +function isLastVisibleOrdinaryPane( + layout: WorkspaceLayout, + paneId: string, + explorerSidebarPaneId: string | null | undefined, +): boolean { + const ordinaryPanes = collectAllPanes(layout.root).filter( + (pane) => pane.id !== explorerSidebarPaneId, + ); + return ordinaryPanes.length === 1 && ordinaryPanes[0].id === paneId; +} + /** * Whether dismissing this pane would do anything — removing it, or hiding it if it * is the Explorer sidebar. A workspace always has somewhere to look, so the last pane the @@ -1533,10 +1591,9 @@ export function canDismissPaneInLayout( if (!pane || pane.hidden === true) { return false; } - const ordinaryPanes = collectAllPanes(layout.root).filter( - (candidate) => candidate.id !== explorerSidebarPaneId, - ); - return paneId === explorerSidebarPaneId ? ordinaryPanes.length > 0 : ordinaryPanes.length > 1; + return paneId === explorerSidebarPaneId + ? collectAllPanes(layout.root).some((candidate) => candidate.id !== explorerSidebarPaneId) + : !isLastVisibleOrdinaryPane(layout, paneId, explorerSidebarPaneId); } /** @@ -1553,14 +1610,7 @@ export function closePaneInLayout(input: ClosePaneInLayoutInput): WorkspaceLayou if (!panePath) { return null; } - const visibleOrdinaryPaneIds = listPaneIds(layout.root).filter( - (paneId) => paneId !== input.explorerSidebarPaneId, - ); - if ( - input.paneId !== input.explorerSidebarPaneId && - visibleOrdinaryPaneIds.length <= 1 && - visibleOrdinaryPaneIds.includes(input.paneId) - ) { + if (isLastVisibleOrdinaryPane(input.layout, input.paneId, input.explorerSidebarPaneId)) { return null; } @@ -1643,6 +1693,7 @@ export function retargetTabInLayout( closeTabInLayout({ layout: input.layout, tabId: input.tabId, + explorerSidebarPaneId: input.explorerSidebarPaneId, }) ?? input.layout; return { layout: @@ -1806,6 +1857,7 @@ export function convertDraftToAgentInLayout( closeTabInLayout({ layout: input.layout, tabId: input.tabId, + explorerSidebarPaneId: input.explorerSidebarPaneId, }) ?? input.layout; return { layout: @@ -2275,6 +2327,7 @@ function collapseStaleEntityTabs(input: { snapshot: WorkspaceTabSnapshot; visibleAgentIds: Set; knownTerminalIds: Set; + explorerSidebarPaneId: string | null; }): WorkspaceLayout { const { snapshot, visibleAgentIds, knownTerminalIds } = input; let nextLayout = input.layout; @@ -2284,6 +2337,7 @@ function collapseStaleEntityTabs(input: { closeTabInLayout({ layout: nextLayout, tabId: tab.tabId, + explorerSidebarPaneId: input.explorerSidebarPaneId, }) ?? nextLayout; } if ( @@ -2295,6 +2349,7 @@ function collapseStaleEntityTabs(input: { closeTabInLayout({ layout: nextLayout, tabId: tab.tabId, + explorerSidebarPaneId: input.explorerSidebarPaneId, }) ?? nextLayout; } } @@ -2360,43 +2415,6 @@ function addMissingEntityTabs(input: { return nextLayout; } -function seedDraftForEmptyWorkspace(input: { - layout: WorkspaceLayout; - snapshot: WorkspaceTabSnapshot; - activeAgentIds: Set; - knownTerminalIds: Set; - explorerSidebarPaneId: string | null; -}): WorkspaceLayout { - const ready = input.snapshot.agentsHydrated && input.snapshot.terminalsHydrated; - const creatingContent = - input.snapshot.hasActivePendingDraftCreate === true || - input.snapshot.hasActivePendingTerminalCreate === true; - const hasWorkspaceEntities = input.activeAgentIds.size > 0 || input.knownTerminalIds.size > 0; - const explorerTabIds = new Set( - input.explorerSidebarPaneId - ? (findPaneById(input.layout.root, input.explorerSidebarPaneId)?.tabIds ?? []) - : [], - ); - const hasContentTab = collectAllTabs(input.layout.root).some( - (tab) => tab.target.kind !== "new_tab" && !explorerTabIds.has(tab.tabId), - ); - if (!ready || creatingContent || hasWorkspaceEntities || hasContentTab) { - return input.layout; - } - - const draftId = generateDraftId(); - return ( - createTabInLayout({ - layout: input.layout, - target: { kind: "draft", draftId }, - now: Date.now(), - placement: FOCUSED_PANE_PLACEMENT, - explorerSidebarPaneId: input.explorerSidebarPaneId, - createTabId: () => draftId, - })?.layout ?? input.layout - ); -} - export function reconcileWorkspaceTabs( state: WorkspaceTabReconcileState, snapshot: WorkspaceTabSnapshot, @@ -2465,6 +2483,7 @@ export function reconcileWorkspaceTabs( closeTabInLayout({ layout: nextLayout, tabId: tab.tabId, + explorerSidebarPaneId: state.explorerSidebarPaneId, }) ?? nextLayout; } } @@ -2474,6 +2493,7 @@ export function reconcileWorkspaceTabs( snapshot, visibleAgentIds, knownTerminalIds, + explorerSidebarPaneId: state.explorerSidebarPaneId, }); nextLayout = addMissingEntityTabs({ @@ -2486,14 +2506,6 @@ export function reconcileWorkspaceTabs( explorerSidebarPaneId: state.explorerSidebarPaneId, }); - nextLayout = seedDraftForEmptyWorkspace({ - layout: nextLayout, - snapshot, - activeAgentIds, - knownTerminalIds, - explorerSidebarPaneId: state.explorerSidebarPaneId, - }); - if (reconciledFocusedTabId) { nextLayout = focusTabInLayout({ diff --git a/packages/app/src/stores/workspace-layout-store.test.ts b/packages/app/src/stores/workspace-layout-store.test.ts index 4f861a04ee..26e0736945 100644 --- a/packages/app/src/stores/workspace-layout-store.test.ts +++ b/packages/app/src/stores/workspace-layout-store.test.ts @@ -1085,6 +1085,118 @@ describe("workspace-layout-store actions", () => { expect(workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]).toBe(before); }); + it.each(["hideExplorerSidebar", "showExplorerSidebar"] as const)( + "retains the last ordinary split after %s", + (setExplorerVisibility) => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + const tabId = store.openTab({ + workspaceKey, + target: { kind: "draft", draftId: "draft-origin" }, + intent: "new", + }) as string; + store[setExplorerVisibility](workspaceKey); + const splitPaneId = store.splitPaneEmpty(workspaceKey, { + targetPaneId: "main", + position: "right", + }) as string; + store.moveTabToPane(workspaceKey, tabId, splitPaneId); + expect( + findPaneById(workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey].root, "main"), + ).toBeNull(); + + store.closeTab(workspaceKey, tabId); + + const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + expect( + collectAllPanes(layout.root) + .filter((pane) => pane.id !== "explorer") + .map((pane) => pane.id), + ).toEqual([splitPaneId]); + expect(findPaneById(layout.root, splitPaneId)?.tabIds).toHaveLength(1); + expect(layout.focusedPaneId).toBe(splitPaneId); + }, + ); + + it.each(["explorer", "pane_generated_explorer"])( + "restores an ordinary pane beside saved hidden %s without losing tabs", + async (explorerId) => { + const workspaceKey = createWorkspaceKey(); + const saved = createPane({ + id: explorerId, + hidden: true, + tabIds: ["files", "draft_saved"], + targetsByTabId: { files: { kind: "files" } }, + stateByTabId: { draft_saved: { savedContent: "preserve me" } }, + }); + await AsyncStorage.setItem( + "workspace-layout-state", + JSON.stringify({ + version: 2, + state: { + layoutByWorkspace: { [workspaceKey]: { root: saved, focusedPaneId: null } }, + explorerPaneIdByWorkspace: { [workspaceKey]: explorerId }, + }, + }), + ); + const restored = createWorkspaceLayoutStore(createDeterministicWorkspaceLayoutIds()); + await restored.persist.rehydrate(); + const state = restored.getState(); + const layout = state.layoutByWorkspace[workspaceKey]; + expect(collectAllPanes(layout.root).map((pane) => pane.id)).toEqual(["main"]); + expect(layout.focusedPaneId).toBe("main"); + expect(findPaneById(layout.root, explorerId)).toEqual( + saved.kind === "pane" ? saved.pane : null, + ); + expect(state.explorerSidebarPaneIdByWorkspace[workspaceKey]).toBe(explorerId); + const snapshot = { + agentsHydrated: true, + terminalsHydrated: true, + activeAgentIds: [], + autoOpenAgentIds: [], + standaloneTerminalIds: [], + }; + state.reconcileTabs(workspaceKey, snapshot); + const afterFirstReconcile = restored.getState().layoutByWorkspace[workspaceKey]; + state.reconcileTabs(workspaceKey, snapshot); + expect(restored.getState().layoutByWorkspace[workspaceKey]).toEqual(afterFirstReconcile); + expect( + collectAllTabs(afterFirstReconcile.root).filter((tab) => tab.target.kind === "draft"), + ).toHaveLength(1); + expect( + collectAllTabs(afterFirstReconcile.root).find((tab) => tab.tabId === "draft_saved")?.state, + ).toEqual({ savedContent: "preserve me" }); + }, + ); + + it("reveals a saved hidden ordinary pane without replacing its content", async () => { + const workspaceKey = createWorkspaceKey(); + const saved = createPane({ + id: "pane_saved_ordinary", + hidden: true, + tabIds: ["draft_saved"], + stateByTabId: { draft_saved: { savedContent: "preserve me" } }, + }); + await AsyncStorage.setItem( + "workspace-layout-state", + JSON.stringify({ + version: 2, + state: { + layoutByWorkspace: { [workspaceKey]: { root: saved, focusedPaneId: null } }, + }, + }), + ); + const restored = createWorkspaceLayoutStore(createDeterministicWorkspaceLayoutIds()); + await restored.persist.rehydrate(); + const layout = restored.getState().layoutByWorkspace[workspaceKey]; + expect(collectAllPanes(layout.root).map((pane) => pane.id)).toEqual(["pane_saved_ordinary"]); + expect(layout.focusedPaneId).toBe("pane_saved_ordinary"); + expect(findPaneById(layout.root, "pane_saved_ordinary")?.tabIds).toEqual(["draft_saved"]); + expect(collectAllTabs(layout.root).find((tab) => tab.tabId === "draft_saved")?.state).toEqual({ + savedContent: "preserve me", + }); + }); + it("migrates legacy layouts to include the hidden registered explorer pane", async () => { const legacyLayout = createDefaultLayout(); await AsyncStorage.setItem( @@ -3081,6 +3193,13 @@ describe("workspace-layout-store actions", () => { intent: "reveal", }); store.closeTab(workspaceKey, tabId!); + store.reconcileTabs(workspaceKey, { + agentsHydrated: true, + terminalsHydrated: true, + activeAgentIds: [], + autoOpenAgentIds: [], + standaloneTerminalIds: [], + }); const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; const mainTab = collectAllTabs(layout.root).find( @@ -3370,95 +3489,10 @@ describe("workspace-layout-store actions", () => { expect(findPaneById(layout.root, "main")?.focusedTabId).toBe("agent_agent-1"); }); - it("reconcileTabs lands on a draft when the hydrated workspace is empty", () => { - const workspaceKey = createWorkspaceKey(); - - workspaceLayoutStore.getState().reconcileTabs(workspaceKey, { - agentsHydrated: true, - terminalsHydrated: true, - activeAgentIds: [], - autoOpenAgentIds: [], - knownTerminalIds: [], - standaloneTerminalIds: [], - hasActivePendingDraftCreate: false, - }); - - const layout = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; - const mainTab = collectAllTabs(layout.root).find( - (tab) => findPaneContainingTab(layout.root, tab.tabId)?.id === "main", - ); - expect(mainTab?.target.kind).toBe("draft"); - expect(findPaneById(layout.root, "main")?.focusedTabId).toBe(mainTab?.tabId); - }); - - it("reconcileTabs stays stable when startup removes a duplicate agent kept in Explorer", () => { + it("reconcileTabs initializes the New launcher once when the hydrated workspace is empty", () => { const workspaceKey = createWorkspaceKey(); const store = workspaceLayoutStore.getState(); - workspaceLayoutStore.setState((state) => ({ - layoutByWorkspace: { - ...state.layoutByWorkspace, - [workspaceKey]: { - root: { - kind: "group", - group: { - id: "root", - direction: "horizontal", - sizes: [0.78, 0.22], - children: [ - createPane({ - id: "main", - tabIds: ["new", "draft-origin-agent"], - focusedTabId: "draft-origin-agent", - targetsByTabId: { - new: { kind: "new_tab" }, - "draft-origin-agent": { kind: "agent", agentId: "agent-1" }, - }, - }), - createPane({ - id: "explorer", - tabIds: ["files", "changes_tree", "agent_agent-1"], - focusedTabId: "agent_agent-1", - targetsByTabId: { - files: { kind: "files" }, - changes_tree: { kind: "changes_tree" }, - "agent_agent-1": { kind: "agent", agentId: "agent-1" }, - }, - }), - ], - }, - }, - focusedPaneId: "main", - }, - }, - explorerSidebarPaneIdByWorkspace: { - ...state.explorerSidebarPaneIdByWorkspace, - [workspaceKey]: "explorer", - }, - })); - - store.reconcileTabs(workspaceKey, { - agentsHydrated: false, - terminalsHydrated: false, - activeAgentIds: [], - autoOpenAgentIds: [], - knownTerminalIds: [], - standaloneTerminalIds: [], - }); - expect(workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey].focusedPaneId).toBe( - "main", - ); - - workspaceLayoutStore.setState((state) => ({ - layoutByWorkspace: { - ...state.layoutByWorkspace, - [workspaceKey]: { - ...state.layoutByWorkspace[workspaceKey], - focusedPaneId: "explorer", - }, - }, - })); - - const emptySnapshot = { + const snapshot = { agentsHydrated: true, terminalsHydrated: true, activeAgentIds: [], @@ -3467,21 +3501,117 @@ describe("workspace-layout-store actions", () => { standaloneTerminalIds: [], hasActivePendingDraftCreate: false, }; - store.reconcileTabs(workspaceKey, emptySnapshot); - store.reconcileTabs(workspaceKey, emptySnapshot); - const afterSecondReconcile = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; - store.reconcileTabs(workspaceKey, emptySnapshot); - const afterThirdReconcile = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; - expect(afterThirdReconcile).toBe(afterSecondReconcile); - const mainTabs = collectAllTabs(afterThirdReconcile.root).filter( - (tab) => findPaneContainingTab(afterThirdReconcile.root, tab.tabId)?.id === "main", - ); - expect(mainTabs).toEqual([ - expect.objectContaining({ target: expect.objectContaining({ kind: "draft" }) }), + store.reconcileTabs(workspaceKey, snapshot); + + const initialized = workspaceLayoutStore.getState(); + const layout = initialized.layoutByWorkspace[workspaceKey]; + expect(collectAllTabs(layout.root).map((tab) => tab.target)).toEqual([ + { kind: "new_tab" }, + { kind: "files" }, + { kind: "changes_tree" }, ]); + expect(layout.focusedPaneId).toBe("main"); + + store.reconcileTabs(workspaceKey, snapshot); + + expect(workspaceLayoutStore.getState()).toBe(initialized); }); + it.each([ + ["main", "explorer"], + ["pane_ordinary", "pane_generated_explorer"], + ])( + "reconcileTabs stays stable when startup removes a duplicate from %s beside %s", + (ordinaryPaneId, explorerPaneId) => { + const workspaceKey = createWorkspaceKey(); + const store = workspaceLayoutStore.getState(); + workspaceLayoutStore.setState((state) => ({ + layoutByWorkspace: { + ...state.layoutByWorkspace, + [workspaceKey]: { + root: { + kind: "group", + group: { + id: "root", + direction: "horizontal", + sizes: [0.78, 0.22], + children: [ + createPane({ + id: ordinaryPaneId, + tabIds: ["draft-origin-agent"], + focusedTabId: "draft-origin-agent", + targetsByTabId: { + "draft-origin-agent": { kind: "agent", agentId: "agent-1" }, + }, + }), + createPane({ + id: explorerPaneId, + tabIds: ["files", "changes_tree", "agent_agent-1"], + focusedTabId: "agent_agent-1", + targetsByTabId: { + files: { kind: "files" }, + changes_tree: { kind: "changes_tree" }, + "agent_agent-1": { kind: "agent", agentId: "agent-1" }, + }, + }), + ], + }, + }, + focusedPaneId: ordinaryPaneId, + }, + }, + explorerSidebarPaneIdByWorkspace: { + ...state.explorerSidebarPaneIdByWorkspace, + [workspaceKey]: explorerPaneId, + }, + })); + + store.reconcileTabs(workspaceKey, { + agentsHydrated: false, + terminalsHydrated: false, + activeAgentIds: [], + autoOpenAgentIds: [], + knownTerminalIds: [], + standaloneTerminalIds: [], + }); + expect(workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey].focusedPaneId).toBe( + ordinaryPaneId, + ); + + workspaceLayoutStore.setState((state) => ({ + layoutByWorkspace: { + ...state.layoutByWorkspace, + [workspaceKey]: { + ...state.layoutByWorkspace[workspaceKey], + focusedPaneId: explorerPaneId, + }, + }, + })); + + const emptySnapshot = { + agentsHydrated: true, + terminalsHydrated: true, + activeAgentIds: [], + autoOpenAgentIds: [], + knownTerminalIds: [], + standaloneTerminalIds: [], + hasActivePendingDraftCreate: false, + }; + store.reconcileTabs(workspaceKey, emptySnapshot); + store.reconcileTabs(workspaceKey, emptySnapshot); + const afterSecondReconcile = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + store.reconcileTabs(workspaceKey, emptySnapshot); + const afterThirdReconcile = workspaceLayoutStore.getState().layoutByWorkspace[workspaceKey]; + + expect(afterThirdReconcile).toBe(afterSecondReconcile); + const mainTabs = collectAllTabs(afterThirdReconcile.root).filter( + (tab) => findPaneContainingTab(afterThirdReconcile.root, tab.tabId)?.id === ordinaryPaneId, + ); + expect(mainTabs).toEqual([expect.objectContaining({ target: { kind: "new_tab" } })]); + }, + ); + it("reconcileTabs does not auto-open subagents omitted from autoOpenAgentIds", () => { const workspaceKey = createWorkspaceKey(); diff --git a/packages/app/src/stores/workspace-layout-store.ts b/packages/app/src/stores/workspace-layout-store.ts index afb1d2694d..a8c6220063 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -36,7 +36,7 @@ import { openTabInLayoutBackground, replaceTabTargetInLayout, revealTargetInLayout, - restoreEmptyPanesInLayout, + restoreWorkspaceLayout, reconcileWorkspaceTabs, removePaneFromTree, removeTabFromTree, @@ -634,7 +634,7 @@ function ensurePersistedExplorerSidebarPane(input: { paneId: split.paneId, hidden: true, }); - const seededLayout = restoreEmptyPanesInLayout( + const seededLayout = restoreWorkspaceLayout( stripEphemeralTabsFromLayout(hiddenLayout ?? split.layout), split.paneId, ); @@ -991,14 +991,10 @@ export function createWorkspaceLayoutStore( }, }; } - const preserveEmptyPaneId = - closingPane?.id === "main" || closingPane?.id === explorerSidebarPaneId - ? closingPane.id - : null; const closedLayout = closeTabInLayout({ layout, tabId: normalizedTabId, - preserveEmptyPaneId, + explorerSidebarPaneId, }); const nextLayoutBeforeFocusNormalization = closedLayout && @@ -1156,8 +1152,13 @@ export function createWorkspaceLayoutStore( return null; } + const layout = getWorkspaceLayout(get().layoutByWorkspace, normalizedWorkspaceKey); const result = convertDraftToAgentInLayout({ - layout: getWorkspaceLayout(get().layoutByWorkspace, normalizedWorkspaceKey), + layout, + explorerSidebarPaneId: resolveExplorerSidebarPaneId( + layout, + get().explorerSidebarPaneIdByWorkspace[normalizedWorkspaceKey], + ), tabId: normalizedTabId, agentId: normalizedAgentId, }); @@ -1224,6 +1225,7 @@ export function createWorkspaceLayoutStore( } } if ( + state.layoutByWorkspace[normalizedWorkspaceKey] && nextLayout === rawLayout && pinnedAgentIdsByWorkspace === state.pinnedAgentIdsByWorkspace ) { @@ -1400,7 +1402,7 @@ export function createWorkspaceLayoutStore( } const restoredLayout = normalizedToPaneId === explorerSidebarPaneId - ? restoreEmptyPanesInLayout(nextLayout, explorerSidebarPaneId) + ? restoreWorkspaceLayout(nextLayout, explorerSidebarPaneId) : nextLayout; const normalizedNextLayout = keepWorkspaceFocusOutOfExplorerSidebar( restoredLayout, @@ -1813,21 +1815,23 @@ export function createWorkspaceLayoutStore( for (const [workspaceKey, persistedLayout] of Object.entries( result.data.layoutByWorkspace, )) { - const strippedLayout = stripEphemeralTabsFromLayout(persistedLayout); + const restoredLayout = restoreWorkspaceLayout( + stripEphemeralTabsFromLayout(persistedLayout), + resolveExplorerSidebarPaneId( + persistedLayout, + explorerSidebarPaneIdByWorkspace[workspaceKey], + ), + ids.createNodeId, + ); const explorerSidebar = ensurePersistedExplorerSidebarPane({ - layout: strippedLayout, + layout: restoredLayout, registeredPaneId: explorerSidebarPaneIdByWorkspace[workspaceKey], ids, }); - if (!explorerSidebar) { - layoutByWorkspace[workspaceKey] = restoreEmptyPanesInLayout(strippedLayout); - continue; + layoutByWorkspace[workspaceKey] = explorerSidebar?.layout ?? restoredLayout; + if (explorerSidebar) { + explorerSidebarPaneIdByWorkspace[workspaceKey] = explorerSidebar.paneId; } - layoutByWorkspace[workspaceKey] = restoreEmptyPanesInLayout( - explorerSidebar.layout, - explorerSidebar.paneId, - ); - explorerSidebarPaneIdByWorkspace[workspaceKey] = explorerSidebar.paneId; } return { ...currentState, diff --git a/packages/desktop/e2e/packaged-app-smoke.js b/packages/desktop/e2e/packaged-app-smoke.js index f3c5a30cda..385c99fa09 100644 --- a/packages/desktop/e2e/packaged-app-smoke.js +++ b/packages/desktop/e2e/packaged-app-smoke.js @@ -781,6 +781,7 @@ async function openSmokeWorkspace({ appPath, env, page, daemonHome }) { }); await page.getByRole("button", { name: "Sandbox smoke workspace", exact: true }).click(); await page.waitForURL((url) => url.pathname.endsWith(`/workspace/${workspace.workspaceId}`)); + await page.getByRole("button", { name: "Agent", exact: true }).click(); await page.getByRole("textbox", { name: "Message agent..." }).click(); console.log(`Packaged desktop smoke: renderer opened workspace ${workspace.workspaceId}`); }