diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 21f80eba39..594aacf5f6 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -103,6 +103,9 @@ layering inside `overlay-root`. Activating a presented browser also focuses its There is no renderer prep/restore handshake or lifetime background-throttling override. Screenshot capture temporarily enables frame production inside the shared serialized queue, -restores the previous throttling policy on success or failure, invalidates before each attempt, -and retries known first-frame failures within the 5-second capture budget. Viewport screenshots use `capturePage({ stayHidden:false })`; -full-page screenshots use the existing CDP path with layout metrics and screenshot clip. +restores the previous throttling policy on success or failure, and retains a 5-second capture budget. +The browser tool takes one viewport frame through Electron's frame subscription and releases the +subscription on completion or cancellation. A resized resident guest can produce fresh pixels +while `capturePage()` leaves its surface-copy request pending; waiting for animation frames or +retrying the copy does not repair that state. Full-page screenshots retain the CDP path with layout +metrics and screenshot clip. diff --git a/docs/explorer-sidebar.md b/docs/explorer-sidebar.md index 618ceb4c87..7d0b0a59f4 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..de0eafe677 100644 --- a/packages/app/e2e/browser/explorer-pane-placement.spec.ts +++ b/packages/app/e2e/browser/explorer-pane-placement.spec.ts @@ -7,7 +7,7 @@ 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 { seedCorruptedWorkspaceLayout } from "../support/helpers/workspace-layout"; import { openChangesTreePanel, waitForWorkspaceTabsVisible, @@ -93,132 +93,221 @@ async function dragChipTo( await page.mouse.up(); } +function collectConsoleErrors(page: Page): string[] { + const errors: string[] = []; + page.on("console", (message) => { + if (message.type() === "error") errors.push(message.text()); + }); + page.on("pageerror", (error) => errors.push(`pageerror: ${String(error)}`)); + return errors; +} + test.describe("explorer pane tab placement", () => { test("agents open in the main pane and New-tab splits keep the app usable", async ({ page, + withWorkspace, + e2eWorkerClient, }, 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 consoleErrors = collectConsoleErrors(page); - const workspace = await seedWorkspace({ repoPrefix: "explorer-pane-placement-" }); + const workspace = await withWorkspace({ prefix: "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("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 }); + }); + + await test.step("select Changes in Explorer", async () => { + await selectExplorerChanges(page); + }); + + await test.step("an agent appearing now opens in the main pane, not the explorer pane", async () => { + const agent = await e2eWorkerClient.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 change the selected Explorer view. + await expect(visible(page, "changes-tree-panel").first()).toBeVisible(); + }); + + 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 + New main. Explorer keeps its own rail. + await expect(visible(page, "workspace-tabs-row")).toHaveCount(2, { timeout: 10_000 }); + await expect(explorerTabRow(page)).toHaveCount(1); + await expect(page.getByTestId("workspace-new-tab-panel")).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 New tab pane (focuses it) must not lock anything. Aim + // above the vertically centred launcher, which would open a tab instead. + const emptyPane = newTabPaneChild(page); + await emptyPane.click({ position: { x: 40, y: 120 }, timeout: 5_000 }); + + // A second drag must work: center-drop the agent tab back into the New 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", }); - await test.step("select Changes in Explorer", async () => { - await selectExplorerChanges(page); + // The agent chip must have replaced New in that pane and stay clickable. + await expect(page.getByTestId("workspace-new-tab-panel")).toHaveCount(0, { + timeout: 10_000, }); + await agentTabChip(page, agentId) + .first() + .click({ position: { x: 12, y: 13 } }); - 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 change the selected Explorer view. - await expect(visible(page, "changes-tree-panel").first()).toBeVisible(); + // 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 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 + New main. Explorer keeps its own rail. - await expect(visible(page, "workspace-tabs-row")).toHaveCount(2, { timeout: 10_000 }); - await expect(explorerTabRow(page)).toHaveCount(1); - await expect(page.getByTestId("workspace-new-tab-panel")).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 New tab pane (focuses it) must not lock anything. Aim - // above the vertically centred launcher, which would open a tab instead. - const emptyPane = newTabPaneChild(page); - await emptyPane.click({ position: { x: 40, y: 120 }, timeout: 5_000 }); - - // A second drag must work: center-drop the agent tab back into the New 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 replaced New in that pane and stay clickable. - await expect(page.getByTestId("workspace-new-tab-panel")).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(); - } + await testInfo.attach("console-errors", { + body: JSON.stringify(consoleErrors, null, 2), + contentType: "application/json", + }); }); }); + +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(); +} + +// 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, + withWorkspace, +}) => { + const workspace = await withWorkspace({ prefix: "last-pane-hidden-explorer-" }); + 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); +}); + +test("visible Explorer does not replace the last ordinary workspace pane", async ({ + page, + withWorkspace, +}) => { + const workspace = await withWorkspace({ prefix: "last-pane-visible-explorer-" }); + 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); +}); + +test("reloading a saved hidden generated Explorer recovers a usable workspace", async ({ + page, + withWorkspace, +}) => { + const otherWorkspace = await withWorkspace({ prefix: "uncorrupted-explorer-" }); + await gotoWorkspace(page, otherWorkspace.workspaceId); + await openAgentDraftFromLauncher(page); + + const workspace = await withWorkspace({ prefix: "saved-hidden-explorer-" }); + await gotoWorkspace(page, workspace.workspaceId); + await expectNewLauncher(page); + await seedCorruptedWorkspaceLayout(page, workspace.workspaceId); + await page.reload(); + await expectNewLauncher(page); + await gotoWorkspace(page, otherWorkspace.workspaceId); + await expect(draftTabChip(page)).toHaveCount(1); +}); 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/model-search-across-providers.spec.ts b/packages/app/e2e/browser/model-search-across-providers.spec.ts index ba2c37f895..c09de28199 100644 --- a/packages/app/e2e/browser/model-search-across-providers.spec.ts +++ b/packages/app/e2e/browser/model-search-across-providers.spec.ts @@ -247,6 +247,7 @@ catalogTest( await test.step("open a draft with a one-row runtime catalog", async () => { await setRuntimeCatalog(client, 1, workspace.repoPath); await gotoWorkspace(page, workspace.workspaceId); + await clickNewChat(page); await expectComposerVisible(page); }); await test.step("open New Agent after the provider publishes repeated model rows", async () => { 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/e2e/support/helpers/workspace-layout.ts b/packages/app/e2e/support/helpers/workspace-layout.ts new file mode 100644 index 0000000000..8fbc893f5f --- /dev/null +++ b/packages/app/e2e/support/helpers/workspace-layout.ts @@ -0,0 +1,63 @@ +import type { Page } from "@playwright/test"; +import { z } from "zod"; +import { WorkspaceLayoutPersistedStateSchema } from "../../../src/stores/workspace-layout-storage"; +import { + buildWorkspaceTabPersistenceKey, + type WorkspaceTab, +} from "../../../src/workspace-tabs/model"; +import { getServerId } from "./server-id"; + +/** Seed the reported hidden Explorer-only layout with 2,250 saved agent draft tabs. */ +export async function seedCorruptedWorkspaceLayout(page: Page, workspaceId: string): Promise { + const raw = await page.evaluate(() => localStorage.getItem("workspace-layout-state")); + if (raw === null) throw new Error("Explorer fixture: workspace layout was not persisted"); + const stored = z + .strictObject({ + state: WorkspaceLayoutPersistedStateSchema, + version: z.number().int().nonnegative().optional(), + }) + .parse(JSON.parse(raw), { + error: () => "Explorer fixture: invalid persisted workspace layout", + }); + const key = buildWorkspaceTabPersistenceKey({ serverId: getServerId(), workspaceId }); + if (!key || !Object.hasOwn(stored.state.layoutByWorkspace, key)) { + throw new Error(`Explorer fixture: no persisted layout for workspace ${workspaceId}`); + } + const tabs: WorkspaceTab[] = [ + { tabId: "files", target: { kind: "files" }, createdAt: 1 }, + { tabId: "changes_tree", target: { kind: "changes_tree" }, createdAt: 1 }, + ...Array.from( + { length: 2250 }, + (_, index): WorkspaceTab => ({ + tabId: `draft_saved_${index}`, + target: { kind: "draft", draftId: `draft_saved_${index}` }, + createdAt: index + 2, + }), + ), + ]; + const corruptedLayout = { + 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 = WorkspaceLayoutPersistedStateSchema.parse({ + ...stored.state, + layoutByWorkspace: { ...stored.state.layoutByWorkspace, [key]: corruptedLayout }, + explorerPaneIdByWorkspace: { + ...stored.state.explorerPaneIdByWorkspace, + [key]: "pane_generated_report_equivalent", + }, + }); + await page.evaluate( + (value) => localStorage.setItem("workspace-layout-state", value), + JSON.stringify(stored), + ); +} 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 9cbfb5d7f1..286030fd86 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-storage.ts b/packages/app/src/stores/workspace-layout-storage.ts new file mode 100644 index 0000000000..ae916188c5 --- /dev/null +++ b/packages/app/src/stores/workspace-layout-storage.ts @@ -0,0 +1,114 @@ +import { z } from "zod"; +import type { SplitNode, WorkspaceLayout } from "./workspace-layout-actions"; + +const WorkspaceDraftTabSetupStorageSchema = z.strictObject({ + provider: z.string(), + cwd: z.string(), + modeId: z.string().nullable(), + model: z.string().nullable(), + thinkingOptionId: z.string().nullable(), + featureValues: z.record(z.string(), z.union([z.boolean(), z.string(), z.null()])), +}); +const WorkspaceTabTargetStorageSchema = z.discriminatedUnion("kind", [ + z.strictObject({ kind: z.literal("new_tab") }), + z.strictObject({ + kind: z.literal("draft"), + draftId: z.string(), + setup: WorkspaceDraftTabSetupStorageSchema.optional(), + }), + z.strictObject({ kind: z.literal("agent"), agentId: z.string() }), + z.strictObject({ + kind: z.literal("provider_subagent"), + parentAgentId: z.string(), + subagentId: z.string(), + }), + z.strictObject({ kind: z.literal("terminal"), terminalId: z.string() }), + z.strictObject({ kind: z.literal("browser"), browserId: z.string() }), + z.strictObject({ kind: z.literal("changes_tree") }), + z.strictObject({ kind: z.literal("files") }), + z.strictObject({ kind: z.literal("pull_request") }), + z.strictObject({ + kind: z.literal("file"), + path: z.string(), + lineStart: z.number().int().positive().optional(), + lineEnd: z.number().int().positive().optional(), + }), + z.strictObject({ + kind: z.literal("working_diff"), + focusPath: z.string().optional(), + focusRequestId: z.number().optional(), + // COMPAT(workingDiffTarget): accepted from pre-canonical tab ids; normalization removes them. + mode: z.enum(["uncommitted", "base"]).optional(), + baseRef: z.string().nullable().optional(), + ignoreWhitespace: z.boolean().optional(), + }), + z.strictObject({ kind: z.literal("setup"), workspaceId: z.string() }), + z.strictObject({ kind: z.literal("commit_diff"), sha: z.string() }), + z.discriminatedUnion("context", [ + z.strictObject({ + kind: z.literal("plugin"), + pluginId: z.string(), + panelId: z.string(), + context: z.literal("workspace"), + }), + z.strictObject({ + kind: z.literal("plugin"), + pluginId: z.string(), + panelId: z.string(), + context: z.literal("agent"), + agentId: z.string(), + }), + ]), +]); +const WorkspaceTabStorageSchema = z.strictObject({ + tabId: z.string(), + target: WorkspaceTabTargetStorageSchema, + createdAt: z.number(), + state: z.json().optional(), +}); +const SplitNodeStorageSchema: z.ZodType = z.lazy(() => + z.discriminatedUnion("kind", [ + z.strictObject({ + kind: z.literal("pane"), + pane: z.strictObject({ + id: z.string(), + tabIds: z.array(z.string()), + focusedTabId: z.string().nullable(), + tabs: z.array(WorkspaceTabStorageSchema).optional(), + hidden: z.boolean().optional(), + }), + }), + z.strictObject({ + kind: z.literal("group"), + group: z.strictObject({ + id: z.string(), + direction: z.enum(["horizontal", "vertical"]), + children: z.array(SplitNodeStorageSchema), + sizes: z.array(z.number()), + }), + }), + ]), +); +const WorkspaceLayoutStorageSchema: z.ZodType = z.strictObject({ + root: SplitNodeStorageSchema, + focusedPaneId: z.string().nullable(), + parentTabIdByTabId: z.record(z.string(), z.string()).optional(), +}); +export const WorkspaceLayoutPersistedStateSchema = z.strictObject({ + pinnedAgentIdsByWorkspace: z.record(z.string(), z.array(z.string())).optional(), + layoutByWorkspace: z.record(z.string(), WorkspaceLayoutStorageSchema), + splitSizesByWorkspace: z.record(z.string(), z.record(z.string(), z.array(z.number()))).optional(), + explorerSidebarWidthByWorkspace: z.record(z.string(), z.number()).optional(), + // COMPAT(explorerSidebarWidth): added in v0.6, remove after 2027-08-25. + explorerSidebarRatioByWorkspace: z.record(z.string(), z.number()).optional(), + // COMPAT(explorerSidebarNaming): accepted from builds that called this dock the Side panel. + sidePanelRatioByWorkspace: z.record(z.string(), z.number()).optional(), + // The persisted keys keep their pre-rename spelling: the schema is strict, so a + // rename here would fail every existing blob and wipe the layout it describes. + explorerPaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), + explorerSidebarPaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), + sidePaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), + // COMPAT(pullRequestAutoAdd): PR detection stopped opening a tab in v0.5; accepted + // and ignored so upgrading does not discard the layout. Remove after 2027-08-20. + acknowledgedPullRequestByWorkspace: z.record(z.string(), z.string()).optional(), +}); 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..b7b9d57677 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -2,7 +2,8 @@ import AsyncStorage from "@react-native-async-storage/async-storage"; import { useEffect, useState } from "react"; import { create } from "zustand"; import { persist } from "zustand/middleware"; -import { z } from "zod"; +import type { z } from "zod"; +import { WorkspaceLayoutPersistedStateSchema } from "./workspace-layout-storage"; import type { JsonValue } from "@getpaseo/protocol/agent-types"; import type { WorkspaceTab, WorkspaceTabTarget } from "@/workspace-tabs/model"; import { @@ -36,7 +37,7 @@ import { openTabInLayoutBackground, replaceTabTargetInLayout, revealTargetInLayout, - restoreEmptyPanesInLayout, + restoreWorkspaceLayout, reconcileWorkspaceTabs, removePaneFromTree, removeTabFromTree, @@ -174,118 +175,6 @@ interface WorkspaceFocusRestorationState { // Preserve four user-created split levels beneath that bookkeeping node. const MAX_TREE_DEPTH = 5; -const WorkspaceDraftTabSetupStorageSchema = z.strictObject({ - provider: z.string(), - cwd: z.string(), - modeId: z.string().nullable(), - model: z.string().nullable(), - thinkingOptionId: z.string().nullable(), - featureValues: z.record(z.string(), z.union([z.boolean(), z.string(), z.null()])), -}); -const WorkspaceTabTargetStorageSchema = z.discriminatedUnion("kind", [ - z.strictObject({ kind: z.literal("new_tab") }), - z.strictObject({ - kind: z.literal("draft"), - draftId: z.string(), - setup: WorkspaceDraftTabSetupStorageSchema.optional(), - }), - z.strictObject({ kind: z.literal("agent"), agentId: z.string() }), - z.strictObject({ - kind: z.literal("provider_subagent"), - parentAgentId: z.string(), - subagentId: z.string(), - }), - z.strictObject({ kind: z.literal("terminal"), terminalId: z.string() }), - z.strictObject({ kind: z.literal("browser"), browserId: z.string() }), - z.strictObject({ kind: z.literal("changes_tree") }), - z.strictObject({ kind: z.literal("files") }), - z.strictObject({ kind: z.literal("pull_request") }), - z.strictObject({ - kind: z.literal("file"), - path: z.string(), - lineStart: z.number().int().positive().optional(), - lineEnd: z.number().int().positive().optional(), - }), - z.strictObject({ - kind: z.literal("working_diff"), - focusPath: z.string().optional(), - focusRequestId: z.number().optional(), - // COMPAT(workingDiffTarget): accepted from pre-canonical tab ids; normalization removes them. - mode: z.enum(["uncommitted", "base"]).optional(), - baseRef: z.string().nullable().optional(), - ignoreWhitespace: z.boolean().optional(), - }), - z.strictObject({ kind: z.literal("setup"), workspaceId: z.string() }), - z.strictObject({ kind: z.literal("commit_diff"), sha: z.string() }), - z.discriminatedUnion("context", [ - z.strictObject({ - kind: z.literal("plugin"), - pluginId: z.string(), - panelId: z.string(), - context: z.literal("workspace"), - }), - z.strictObject({ - kind: z.literal("plugin"), - pluginId: z.string(), - panelId: z.string(), - context: z.literal("agent"), - agentId: z.string(), - }), - ]), -]); -const WorkspaceTabStorageSchema = z.strictObject({ - tabId: z.string(), - target: WorkspaceTabTargetStorageSchema, - createdAt: z.number(), - state: z.json().optional(), -}); -const SplitNodeStorageSchema: z.ZodType = z.lazy(() => - z.discriminatedUnion("kind", [ - z.strictObject({ - kind: z.literal("pane"), - pane: z.strictObject({ - id: z.string(), - tabIds: z.array(z.string()), - focusedTabId: z.string().nullable(), - tabs: z.array(WorkspaceTabStorageSchema).optional(), - hidden: z.boolean().optional(), - }), - }), - z.strictObject({ - kind: z.literal("group"), - group: z.strictObject({ - id: z.string(), - direction: z.enum(["horizontal", "vertical"]), - children: z.array(SplitNodeStorageSchema), - sizes: z.array(z.number()), - }), - }), - ]), -); -const WorkspaceLayoutStorageSchema: z.ZodType = z.strictObject({ - root: SplitNodeStorageSchema, - focusedPaneId: z.string().nullable(), - parentTabIdByTabId: z.record(z.string(), z.string()).optional(), -}); -const WorkspaceLayoutPersistedStateSchema = z.strictObject({ - pinnedAgentIdsByWorkspace: z.record(z.string(), z.array(z.string())).optional(), - layoutByWorkspace: z.record(z.string(), WorkspaceLayoutStorageSchema), - splitSizesByWorkspace: z.record(z.string(), z.record(z.string(), z.array(z.number()))).optional(), - explorerSidebarWidthByWorkspace: z.record(z.string(), z.number()).optional(), - // COMPAT(explorerSidebarWidth): added in v0.6, remove after 2027-08-25. - explorerSidebarRatioByWorkspace: z.record(z.string(), z.number()).optional(), - // COMPAT(explorerSidebarNaming): accepted from builds that called this dock the Side panel. - sidePanelRatioByWorkspace: z.record(z.string(), z.number()).optional(), - // The persisted keys keep their pre-rename spelling: the schema is strict, so a - // rename here would fail every existing blob and wipe the layout it describes. - explorerPaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), - explorerSidebarPaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), - sidePaneIdByWorkspace: z.record(z.string(), z.string().nullable()).optional(), - // COMPAT(pullRequestAutoAdd): PR detection stopped opening a tab in v0.5; accepted - // and ignored so upgrading does not discard the layout. Remove after 2027-08-20. - acknowledgedPullRequestByWorkspace: z.record(z.string(), z.string()).optional(), -}); - const LEGACY_EXPLORER_SIDEBAR_REFERENCE_WIDTH = 1440; const WORKSPACE_LAYOUT_PERSIST_VERSION = 2; @@ -634,7 +523,7 @@ function ensurePersistedExplorerSidebarPane(input: { paneId: split.paneId, hidden: true, }); - const seededLayout = restoreEmptyPanesInLayout( + const seededLayout = restoreWorkspaceLayout( stripEphemeralTabsFromLayout(hiddenLayout ?? split.layout), split.paneId, ); @@ -991,14 +880,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 +1041,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 +1114,7 @@ export function createWorkspaceLayoutStore( } } if ( + state.layoutByWorkspace[normalizedWorkspaceKey] && nextLayout === rawLayout && pinnedAgentIdsByWorkspace === state.pinnedAgentIdsByWorkspace ) { @@ -1400,7 +1291,7 @@ export function createWorkspaceLayoutStore( } const restoredLayout = normalizedToPaneId === explorerSidebarPaneId - ? restoreEmptyPanesInLayout(nextLayout, explorerSidebarPaneId) + ? restoreWorkspaceLayout(nextLayout, explorerSidebarPaneId) : nextLayout; const normalizedNextLayout = keepWorkspaceFocusOutOfExplorerSidebar( restoredLayout, @@ -1813,21 +1704,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/browser-tabs.e2e.mjs b/packages/desktop/e2e/browser-tabs.e2e.mjs index 208661e033..40de9de6e2 100644 --- a/packages/desktop/e2e/browser-tabs.e2e.mjs +++ b/packages/desktop/e2e/browser-tabs.e2e.mjs @@ -240,20 +240,6 @@ async function callBrowserTool(client, name, args = {}) { return mcpPayload(await client.callTool({ name, args }), name); } -async function callBrowserToolUntilReady(client, name, args = {}) { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const result = await client.callTool({ name, args }); - const payload = result.structuredContent; - if (payload?.ok === true) return payload.result; - if (payload?.ok !== false || payload.error?.retryable !== true) { - return mcpPayload(result, name); - } - await delay(100); - } - throw new Error(`${name} remained unavailable for ${timeoutMs}ms`); -} - async function waitForGuestSelector(client, browserId) { const deadline = Date.now() + 5_000; while (Date.now() < deadline) { @@ -373,7 +359,8 @@ async function readPresentation(page, browserId) { async function readViewport(client, browserId) { const evaluated = await callBrowserTool(client, "browser_evaluate", { browserId, - function: "() => ({ width: window.innerWidth, height: window.innerHeight })", + function: + "() => ({ width: window.innerWidth, height: window.innerHeight, scale: window.devicePixelRatio })", }); return JSON.parse(evaluated.resultJson); } @@ -639,10 +626,11 @@ async function runRegression({ const deviceSizeMenuPainted = await selectDeviceSize(page, "iPhone SE · 375×667"); assert(deviceSizeMenuPainted, "Device size menu did not paint above the browser surface"); + const deviceViewport = await readViewport(client, browserId); recordViewportMismatch( failures, "device size menu paints and receives input above the browser surface", - await readViewport(client, browserId), + deviceViewport, { width: 375, height: 667 }, ); @@ -651,6 +639,12 @@ async function runRegression({ text: "Bridge target", timeoutMs: 5_000, }); + const resizedScreenshot = await callBrowserTool(client, "browser_screenshot", { browserId }); + assert( + resizedScreenshot.width === Math.round(375 * deviceViewport.scale) && + resizedScreenshot.height === Math.round(667 * deviceViewport.scale), + `Screenshot after resize returned ${resizedScreenshot.width}×${resizedScreenshot.height}`, + ); const requestedViewport = { width: 640, height: 480 }; await callBrowserTool(client, "browser_resize", { browserId, ...requestedViewport }); recordViewportMismatch( @@ -726,7 +720,7 @@ async function runRegression({ { timeout: timeoutMs }, ); try { - await callBrowserToolUntilReady(client, "browser_screenshot", { browserId }); + await callBrowserTool(client, "browser_screenshot", { browserId }); } catch (error) { failures.push(`inactive browser remains captureable: ${String(error)}`); } 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}`); } diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index a36b7dea0d..ee7143adc6 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -1,6 +1,5 @@ -import type { Rectangle } from "electron"; import { describe, expect, test, vi } from "vitest"; -import type { TabImage } from "./service.js"; +import { executeAutomationCommand, type BrowserRegistry, type TabImage } from "./service.js"; import { adaptWebContents, HostSnapshotEngineRegistry } from "./ipc.js"; import type { IsolatedKeyboardInputEvent } from "./trusted-input.js"; @@ -119,13 +118,11 @@ class FakeWebContents { public readonly debugger = new FakeDebugger(); public readonly inputEvents: IsolatedKeyboardInputEvent[] = []; public readonly loadedUrls: string[] = []; - public readonly captures: Array<{ - rect: Rectangle | undefined; - options: { stayHidden?: boolean } | undefined; - }> = []; + public frameListener: ((image: TabImage) => void) | null = null; + public endedFrameSubscriptions = 0; public readonly invalidations: string[] = []; private consoleMessageListener: ConsoleMessageListener | null = null; - private destroyedListener: (() => void) | null = null; + public readonly destroyedListeners = new Set<() => void>(); public destroyed = false; public constructor(private readonly webContentsId: number) {} @@ -175,12 +172,14 @@ class FakeWebContents { public reload(): void {} - public async capturePage( - rect?: Rectangle, - options?: { stayHidden?: boolean }, - ): Promise { - this.captures.push({ rect, options }); - return new FakeImage(); + public beginFrameSubscription(onlyDirty: boolean, callback: (image: TabImage) => void): void { + expect(onlyDirty).toBe(false); + this.frameListener = callback; + } + + public endFrameSubscription(): void { + this.frameListener = null; + this.endedFrameSubscriptions += 1; } public invalidate(): void { @@ -198,7 +197,12 @@ class FakeWebContents { public once(event: "destroyed", listener: () => void): void { expect(event).toBe("destroyed"); - this.destroyedListener = listener; + this.destroyedListeners.add(listener); + } + + public removeListener(event: "destroyed", listener: () => void): void { + expect(event).toBe("destroyed"); + this.destroyedListeners.delete(listener); } public emitConsoleMessage(input: { @@ -216,7 +220,11 @@ class FakeWebContents { public destroy(): void { this.destroyed = true; this.debugger.emitDetach(); - this.destroyedListener?.(); + this.frameListener = null; + for (const listener of this.destroyedListeners) { + this.destroyedListeners.delete(listener); + listener(); + } } } @@ -245,16 +253,86 @@ describe("browser automation IPC adapter", () => { ]); }); - test("delegates viewport capture to the guest without a renderer prep bridge", async () => { + test("captures one rendered frame and releases the subscription", async () => { const contents = new FakeWebContents(20); const tab = adaptWebContents(contents); + const controller = new AbortController(); + const capture = tab.captureFrame(controller.signal); + contents.frameListener?.(new FakeImage()); - const image = await tab.capturePage({ stayHidden: false }); - tab.invalidate(); + expect((await capture).getSize()).toEqual({ width: 640, height: 480 }); + expect(contents.frameListener).toBeNull(); + controller.abort(); + expect(contents.endedFrameSubscriptions).toBe(1); + }); - expect(image.getSize()).toEqual({ width: 640, height: 480 }); - expect(contents.captures).toEqual([{ rect: undefined, options: { stayHidden: false } }]); - expect(contents.invalidations).toEqual(["invalidate"]); + test("cancels a pending frame subscription when the capture budget expires", async () => { + const contents = new FakeWebContents(2001); + const tab = adaptWebContents(contents); + const controller = new AbortController(); + const capture = tab.captureFrame(controller.signal); + const failure = expect(capture).rejects.toThrow("capture deadline"); + controller.abort(new Error("capture deadline")); + + await failure; + expect(contents.frameListener).toBeNull(); + expect(contents.endedFrameSubscriptions).toBe(1); + }); + + test("closing a guest settles its screenshot and releases the next queued capture", async () => { + vi.useFakeTimers(); + try { + const closing = new FakeWebContents(2002); + const next = new FakeWebContents(2003); + const tabs = new Map([ + ["closing", adaptWebContents(closing)], + ["next", adaptWebContents(next)], + ]); + const registry: BrowserRegistry = { + listRegisteredBrowserIds: () => [...tabs.keys()], + listRegisteredBrowserIdsForWorkspace: () => [...tabs.keys()], + getTabContents: (id) => tabs.get(id) ?? null, + getBrowserWorkspaceId: () => null, + getWorkspaceActiveBrowserId: () => null, + }; + const capture = (browserId: string) => + executeAutomationCommand( + { + type: "browser.automation.execute.request", + requestId: browserId, + command: { command: "screenshot", args: { browserId, fullPage: false } }, + }, + registry, + ); + let closedResult: unknown; + const closedCapture = Promise.resolve(capture("closing")).then((result) => { + closedResult = result; + return result; + }); + const nextCapture = capture("next"); + await vi.advanceTimersByTimeAsync(0); + expect(closing.frameListener).not.toBeNull(); + closing.destroy(); + await vi.advanceTimersByTimeAsync(0); + expect(closedResult).toEqual({ + requestId: "closing", + ok: false, + error: { + code: "browser_tab_closed", + message: "Browser tab closing has been closed", + retryable: false, + }, + }); + expect(closing.destroyedListeners.size).toBe(0); + expect(next.frameListener).not.toBeNull(); + next.frameListener?.(new FakeImage()); + await expect(nextCapture).resolves.toMatchObject({ ok: true }); + await closedCapture; + expect(next.destroyedListeners.size).toBe(1); + } finally { + await vi.runAllTimersAsync(); + vi.useRealTimers(); + } }); test("collects console messages until the guest is destroyed", () => { diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index d21c51b84f..42c38d2e2c 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -1,4 +1,3 @@ -import type { Rectangle } from "electron"; import { ipcMain } from "electron"; import { BrowserAutomationExecuteRequestSchema } from "@getpaseo/protocol/browser-automation/rpc-schemas"; import type { @@ -16,7 +15,7 @@ import { promptShimInstallScript, promptShimRestoreScript, } from "./dialog-handling.js"; -import { executeAutomationCommand } from "./service.js"; +import { BrowserTabClosedError, executeAutomationCommand } from "./service.js"; import { BrowserSnapshotEngine } from "./snapshot-engine.js"; import { listRegisteredPaseoBrowserIds, @@ -92,6 +91,7 @@ interface ConsoleMessageEmitter { } interface BrowserAutomationWebContents extends ConsoleMessageEmitter { + removeListener(event: "destroyed", listener: () => void): void; readonly id: number; readonly debugger: WebContentsDebugger; getURL(): string; @@ -105,7 +105,8 @@ interface BrowserAutomationWebContents extends ConsoleMessageEmitter { goBack(): void; goForward(): void; reload(): void; - capturePage(rect?: Rectangle, options?: { stayHidden?: boolean }): Promise; + beginFrameSubscription(onlyDirty: boolean, callback: (image: TabImage) => void): void; + endFrameSubscription(): void; invalidate(): void; getBackgroundThrottling(): boolean; setBackgroundThrottling(allowed: boolean): void; @@ -130,7 +131,7 @@ export function adaptWebContents(contents: BrowserAutomationWebContents): TabCon goBack: () => contents.goBack(), goForward: () => contents.goForward(), reload: () => contents.reload(), - capturePage: (captureOptions) => contents.capturePage(undefined, captureOptions), + captureFrame: (signal) => captureViewportFrame(contents, signal), invalidate: () => contents.invalidate(), withFrameProduction: async (capture) => { const previous = contents.getBackgroundThrottling(); @@ -154,6 +155,41 @@ export function adaptWebContents(contents: BrowserAutomationWebContents): TabCon }; } +function captureViewportFrame( + contents: BrowserAutomationWebContents, + signal: AbortSignal, +): Promise { + signal.throwIfAborted(); + return new Promise((resolve, reject) => { + const stop = () => { + signal.removeEventListener("abort", abort); + contents.removeListener("destroyed", destroyed); + if (!contents.isDestroyed()) contents.endFrameSubscription(); + }; + const abort = () => { + stop(); + reject(signal.reason); + }; + const destroyed = () => { + stop(); + reject(new BrowserTabClosedError()); + }; + signal.addEventListener("abort", abort, { once: true }); + contents.once("destroyed", destroyed); + try { + // A resized resident guest can paint while capturePage's surface-copy + // request remains pending. Subscribe to its rendered frames instead. + contents.beginFrameSubscription(false, (image) => { + stop(); + resolve(image); + }); + } catch (error) { + stop(); + reject(error); + } + }); +} + function getCdpQueue(contentsId: number): CdpSessionQueue { const existing = cdpQueuesByContentsId.get(contentsId); if (existing) { diff --git a/packages/desktop/src/features/browser-automation/service.test.ts b/packages/desktop/src/features/browser-automation/service.test.ts index cbce7ae3bc..dee624a3f7 100644 --- a/packages/desktop/src/features/browser-automation/service.test.ts +++ b/packages/desktop/src/features/browser-automation/service.test.ts @@ -39,7 +39,7 @@ class FakeTab implements TabContents { public readonly loadedUrls: string[] = []; public readonly scripts: string[] = []; public readonly actions: string[] = []; - public readonly capturedViewports: Array<{ stayHidden?: boolean }> = []; + public readonly capturedViewports: AbortSignal[] = []; public readonly debugCommands: Array<{ command: string; params?: Record }> = []; public readonly inputEvents: IsolatedKeyboardInputEvent[] = []; private readonly captureStartWaiters: Array<() => void> = []; @@ -71,7 +71,6 @@ class FakeTab implements TabContents { public captureNeverPaints = false; public captureThrows = false; public captureErrorMessage = "capture failed"; - public viewportCaptureFailuresBeforeSuccess = 0; public deferCaptures = false; public fullPageScreenshotThrows = false; public fullPageScreenshotErrorMessage = "UnknownVizError"; @@ -168,14 +167,10 @@ class FakeTab implements TabContents { this.actions.push("reload"); } - public async capturePage(options?: { stayHidden?: boolean }): Promise { - this.capturedViewports.push(options ?? {}); + public async captureFrame(signal: AbortSignal): Promise { + this.capturedViewports.push(signal); this.actions.push("capture"); this.resolveCaptureStartWaiters(); - if (this.viewportCaptureFailuresBeforeSuccess > 0) { - this.viewportCaptureFailuresBeforeSuccess -= 1; - throw new Error(this.captureErrorMessage); - } if (this.captureThrows) { throw new Error(this.captureErrorMessage); } @@ -1564,7 +1559,8 @@ describe("executeAutomationCommand", () => { height: 480, }, }); - expect(browser.tab.capturedViewports).toEqual([{ stayHidden: false }]); + expect(browser.tab.capturedViewports).toHaveLength(1); + expect(browser.tab.capturedViewports[0].aborted).toBe(true); expect(browser.tab.actions).toEqual(["invalidate", "capture"]); }); @@ -1590,6 +1586,7 @@ describe("executeAutomationCommand", () => { }, }); expect(browser.tab.actions).toEqual(["invalidate", "capture"]); + expect(browser.tab.capturedViewports[0].aborted).toBe(true); } finally { vi.useRealTimers(); } @@ -1634,76 +1631,6 @@ describe("executeAutomationCommand", () => { expect(browser.tab.actions).toEqual(["invalidate", "capture"]); }); - test("screenshot retries UnknownVizError until the first viewport frame appears", async () => { - vi.useFakeTimers(); - try { - const browser = new BrowserAutomationHarness(); - browser.tab.captureErrorMessage = "UnknownVizError"; - browser.tab.viewportCaptureFailuresBeforeSuccess = 2; - - const resultPromise = browser.execute({ - command: "screenshot", - args: { browserId: BROWSER_A }, - }); - await vi.advanceTimersByTimeAsync(400); - - await expect(resultPromise).resolves.toEqual({ - requestId: "req-screenshot", - ok: true, - result: { - command: "screenshot", - browserId: BROWSER_A, - mimeType: "image/png", - dataBase64: "iVBORwECAw==", - width: 640, - height: 480, - }, - }); - expect(browser.tab.capturedViewports).toHaveLength(3); - expect(browser.tab.actions).toEqual([ - "invalidate", - "capture", - "invalidate", - "capture", - "invalidate", - "capture", - ]); - } finally { - vi.useRealTimers(); - } - }); - - test("screenshot returns no-frame after UnknownVizError spends the retry budget", async () => { - vi.useFakeTimers(); - try { - const browser = new BrowserAutomationHarness(); - browser.tab.captureThrows = true; - browser.tab.captureErrorMessage = "UnknownVizError"; - - const resultPromise = browser.execute({ - command: "screenshot", - args: { browserId: BROWSER_A }, - }); - await vi.advanceTimersByTimeAsync(5_000); - - await expect(resultPromise).resolves.toEqual({ - requestId: "req-screenshot", - ok: false, - error: { - code: "screenshot_no_frame", - message: "The tab has not painted yet. Retry the screenshot.", - retryable: true, - }, - }); - expect(browser.tab.capturedViewports.length).toBeGreaterThan(1); - expect(browser.tab.actions.filter((action) => action === "invalidate")).toHaveLength( - browser.tab.capturedViewports.length, - ); - } finally { - vi.useRealTimers(); - } - }); - test("overlapping screenshots serialize through the shared capture queue", async () => { const browser = new BrowserAutomationHarness(); browser.tab.deferCaptures = true; diff --git a/packages/desktop/src/features/browser-automation/service.ts b/packages/desktop/src/features/browser-automation/service.ts index a5e89378c9..ca6247e543 100644 --- a/packages/desktop/src/features/browser-automation/service.ts +++ b/packages/desktop/src/features/browser-automation/service.ts @@ -35,7 +35,7 @@ export interface TabContents { goBack(): void; goForward(): void; reload(): void; - capturePage(options?: TabCapturePageOptions): Promise; + captureFrame(signal: AbortSignal): Promise; invalidate(): void; withFrameProduction(capture: () => Promise): Promise; sendInputEvent(event: IsolatedKeyboardInputEvent): void; @@ -51,10 +51,6 @@ export interface TabImage { getSize(): { width: number; height: number }; } -export interface TabCapturePageOptions { - stayHidden?: boolean; -} - export interface BrowserRegistry { listRegisteredBrowserIds(): string[]; listRegisteredBrowserIdsForWorkspace(workspaceId: string): string[]; @@ -98,6 +94,13 @@ async function withDialogCapture( return dialogs.length > 0 ? { ...result, dialogs } : result; } +export class BrowserTabClosedError extends Error { + public constructor() { + super("Browser tab has been closed"); + this.name = "BrowserTabClosedError"; + } +} + class ScreenshotNoFrameError extends Error { public constructor(message = SCREENSHOT_NO_FRAME_MESSAGE) { super(message); @@ -215,7 +218,22 @@ async function runPaintedPixelCapture( } async function capturePaintedViewport(contents: TabContents): Promise { - return runPaintedPixelCapture(contents, () => contents.capturePage({ stayHidden: false })); + return runSerializedPixelCapture(() => + contents.withFrameProduction(async () => { + const deadline = Date.now() + PIXEL_CAPTURE_TIMEOUT_MS; + const controller = new AbortController(); + try { + await waitForPaint(contents, deadline); + contents.invalidate(); + return await withPixelCaptureTimeout( + contents.captureFrame(controller.signal), + deadline - Date.now(), + ); + } finally { + controller.abort(); + } + }), + ); } function tabInfoFromContents( @@ -1217,6 +1235,9 @@ async function executeScreenshot( try { image = await capturePaintedViewport(target.contents); } catch (error) { + if (error instanceof BrowserTabClosedError) { + return fail(requestId, "browser_tab_closed", `Browser tab ${browserId} has been closed`); + } if (isScreenshotNoFrameError(error)) { return screenshotNoFrameFailure(requestId, error); }