From 7f7e8923eb95c0f77426a286a47c7e6ddc1dd74f Mon Sep 17 00:00:00 2001 From: Christoph Leiter Date: Wed, 5 Aug 2026 23:23:00 +0200 Subject: [PATCH 1/2] fix(server): stop Claude replay from accumulating running subagents (#2876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(server): finish subagents the parent transcript can never end A skill-spawned subagent — a /code-review run, for example — has no Task tool_use for the parent to record, so its sidecar carries no toolUseId and its id appears nowhere in the parent transcript. Replay resolved no link, emitted no status observation, and the store defaulted the descriptor to running. The subagents track then reported a running child on every reopen, days after it had finished, and the archive dialog offered to stop it. Paseo only records an outcome for a child whose Task call it parsed out of the parent, so a child with no Task call is one no reader can finish. Resolve identity and status together and mark those completed. A child that does have a Task call but no result yet is the opposite case — the parent was interrupted mid-Task — and keeps running. Keying on "no Task call for this id" rather than "no toolUseId" also covers legacy grandchildren. Across 554 recorded subagent transcripts, 5 stuck rows had no toolUseId and 8 more named a Task call made inside a sibling's session; none of the 13 has a recoverable tool_result in the parent. Co-Authored-By: Claude Opus 5 * fix(server): admit only declared Claude subagents on replay * test(server): cover Claude workflow subagent accumulation --------- Co-authored-by: Claude Opus 5 Co-authored-by: Mohamed Boudra --- .../claude/agent.sub-agent-replay.test.ts | 115 ++++++++++++--- .../claude/subagents/replay-source.test.ts | 133 ++++++++++++++++-- .../claude/subagents/replay-source.ts | 92 ++++++++---- 3 files changed, 281 insertions(+), 59 deletions(-) diff --git a/packages/server/src/server/agent/providers/claude/agent.sub-agent-replay.test.ts b/packages/server/src/server/agent/providers/claude/agent.sub-agent-replay.test.ts index 0b7e96f73..9c8b26771 100644 --- a/packages/server/src/server/agent/providers/claude/agent.sub-agent-replay.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.sub-agent-replay.test.ts @@ -57,14 +57,18 @@ function taskToolResult(options: { isError?: boolean; mentionAgentId?: boolean } }); } -function sidechainEntry(): string { +function sidechainEntry(options: { agentId?: string; stopReason?: string | null } = {}): string { return JSON.stringify({ type: "assistant", isSidechain: true, - agentId: AGENT_ID, + agentId: options.agentId ?? AGENT_ID, sessionId: "replay-session", timestamp: "2026-07-26T06:27:50.000Z", - message: { role: "assistant", content: [{ type: "text", text: "summary of the docs" }] }, + message: { + role: "assistant", + content: [{ type: "text", text: "summary of the docs" }], + stop_reason: options.stopReason ?? null, + }, }); } @@ -77,24 +81,40 @@ describe("ClaudeAgentSession persisted subagent replay", () => { let cwd: string; let configDir: string; + function writeParentSession(parentLines: string[]): string { + const historyDir = claudeProjectDirSync(cwd, { configDir }); + mkdirSync(historyDir, { recursive: true }); + writeFileSync(path.join(historyDir, "replay-session.jsonl"), parentLines.join("\n")); + return path.join(historyDir, "replay-session", "subagents"); + } + + function writeSubagent(options: { + subagentDir: string; + agentId?: string; + meta?: string | null; + sidechainLines?: string[]; + }): void { + const agentId = options.agentId ?? AGENT_ID; + mkdirSync(options.subagentDir, { recursive: true }); + writeFileSync( + path.join(options.subagentDir, `agent-${agentId}.jsonl`), + (options.sidechainLines ?? [sidechainEntry({ agentId })]).join("\n"), + ); + if (options.meta !== null && options.meta !== undefined) { + writeFileSync(path.join(options.subagentDir, `agent-${agentId}.meta.json`), options.meta); + } + } + function writeSession(options: { parentLines: string[]; meta?: string | null; sidechainLines?: string[]; }): void { - const historyDir = claudeProjectDirSync(cwd, { configDir }); - mkdirSync(historyDir, { recursive: true }); - writeFileSync(path.join(historyDir, "replay-session.jsonl"), options.parentLines.join("\n")); - - const subagentDir = path.join(historyDir, "replay-session", "subagents"); - mkdirSync(subagentDir, { recursive: true }); - writeFileSync( - path.join(subagentDir, `agent-${AGENT_ID}.jsonl`), - (options.sidechainLines ?? [sidechainEntry()]).join("\n"), - ); - if (options.meta !== null && options.meta !== undefined) { - writeFileSync(path.join(subagentDir, `agent-${AGENT_ID}.meta.json`), options.meta); - } + writeSubagent({ + subagentDir: writeParentSession(options.parentLines), + meta: options.meta, + sidechainLines: options.sidechainLines, + }); } async function replayDescriptors(): Promise< @@ -198,6 +218,69 @@ describe("ClaudeAgentSession persisted subagent replay", () => { expect(statuses).toContain("running"); }); + test("finishes a Task subagent from its own end_turn when the parent outcome is missing", async () => { + writeSession({ + parentLines: [taskToolUse()], + meta: JSON.stringify({ toolUseId: TOOL_USE_ID }), + sidechainLines: [sidechainEntry({ stopReason: "end_turn" })], + }); + + expect(upserts(await replayDescriptors()).at(-1)).toMatchObject({ + id: TOOL_USE_ID, + status: "completed", + }); + }); + + test("does not replay a skill-spawned subagent the parent never named", async () => { + // The reported bug. A /code-review subagent has no Task tool_use, so its sidecar carries only + // agentType and its id appears nowhere in the parent — it used to replay as running forever, + // and every reopen resurrected it. + writeSession({ + parentLines: [parentEntry([{ type: "text", text: "running /code-review" }])], + meta: JSON.stringify({ agentType: "general-purpose" }), + sidechainLines: [sidechainEntry({ stopReason: "end_turn" })], + }); + + expect(upserts(await replayDescriptors())).toEqual([]); + }); + + test("does not accumulate internal workflow agents as replay-only running rows", async () => { + const subagentRoot = writeParentSession([ + parentEntry([ + { + type: "tool_use", + id: "toolu_workflow", + name: "Workflow", + input: { script: "await Promise.all([agent('one'), agent('two'), agent('three')])" }, + }, + ]), + ]); + const workflowDir = path.join(subagentRoot, "workflows", "wf_c240e728-6ea"); + + for (const agentId of ["a17a341a0902f5799", "ae2d01ba2dfa46032", "abc9f20546c326c53"]) { + writeSubagent({ + subagentDir: workflowDir, + agentId, + meta: JSON.stringify({ agentType: "workflow-subagent", spawnDepth: 1 }), + sidechainLines: [sidechainEntry({ agentId, stopReason: "end_turn" })], + }); + } + + expect(upserts(await replayDescriptors())).toEqual([]); + }); + + test("does not replay a subagent whose toolUseId names no Task call in this transcript", async () => { + // A grandchild recorded before spawnDepth existed: the Task call it names was made inside a + // sibling's session, so no tool_result for it can ever reach this parent. + writeSession({ + parentLines: [parentEntry([{ type: "text", text: "no task calls here" }])], + meta: JSON.stringify({ toolUseId: TOOL_USE_ID, agentType: "Explore" }), + sidechainLines: [sidechainEntry({ stopReason: "end_turn" })], + }); + + expect(upserts(await replayDescriptors())).toEqual([]); + }); + test.each([ ["truncated json", '{"toolUseId":"toolu_01DgLoPMW9"'], ["not json", "toolUseId: toolu"], diff --git a/packages/server/src/server/agent/providers/claude/subagents/replay-source.test.ts b/packages/server/src/server/agent/providers/claude/subagents/replay-source.test.ts index e47fe817f..6dceb8d0c 100644 --- a/packages/server/src/server/agent/providers/claude/subagents/replay-source.test.ts +++ b/packages/server/src/server/agent/providers/claude/subagents/replay-source.test.ts @@ -99,7 +99,7 @@ describe("observeReplaySubagents", () => { // Claude Code writes every descendant into the same subagents/ directory. A grandchild's // toolUseId names a Task call made inside its parent's session, so nothing here can resolve // it: it would replay as an extra row the live stream never showed, with no Task card and no - // outcome, running forever. One recorded session showed 10 subagents live and 22 on reopen. + // outcome. One recorded session showed 10 subagents live and 22 on reopen. const observations = observeReplaySubagents({ subagents: [ { agentId: AGENT_ID, meta: { toolUseId: TOOL_USE_ID, spawnDepth: 1 }, entries: [] }, @@ -126,8 +126,7 @@ describe("observeReplaySubagents", () => { expect(observations[0]).toMatchObject({ kind: "declared", id: TOOL_USE_ID }); }); - it("recovers identity from meta when the parent's Task call is missing", () => { - // The case the legacy scrape cannot handle: no tool_result, so no link and no identity. + it("drops a meta-linked subagent when the parent never declared its Task", () => { const observations = observeReplaySubagents({ subagents: [ { @@ -140,12 +139,7 @@ describe("observeReplaySubagents", () => { convertEntry: () => [], }); - expect(observations[0]).toMatchObject({ - id: TOOL_USE_ID, - toolCallId: TOOL_USE_ID, - title: "Explore", - description: "Find the code", - }); + expect(observations).toEqual([]); }); it("falls back to the scraped link when there is no meta file", () => { @@ -165,14 +159,127 @@ describe("observeReplaySubagents", () => { expect(rest.at(-1)).toMatchObject({ kind: "status", status: "completed" }); }); - it("keeps an unlinkable subagent visible under its own id", () => { - const [declared] = observeReplaySubagents({ + it("drops an unlinkable subagent", () => { + const observations = observeReplaySubagents({ subagents: [{ agentId: AGENT_ID, meta: null, entries: [] }], parent: emptyParent(), convertEntry: () => [], }); - expect(declared).toMatchObject({ id: AGENT_ID }); - expect(declared).not.toHaveProperty("toolCallId"); + expect(observations).toEqual([]); + }); + + it("drops a terminal subagent the parent has no Task call for", () => { + // A legacy grandchild: its toolUseId names a Task call made inside a sibling's transcript, so + // no tool_result for it can ever reach the parent. Left running, it never stops. + const descriptor = applyToStore( + observeReplaySubagents({ + subagents: [ + { + agentId: AGENT_ID, + meta: { toolUseId: TOOL_USE_ID }, + entries: [ + { + type: "assistant", + timestamp: "2026-07-31T03:41:00.000Z", + message: { stop_reason: "end_turn" }, + }, + ], + }, + ], + parent: emptyParent(), + convertEntry: () => [], + }), + ); + + expect(descriptor).toBeNull(); + }); + + it("drops a terminal skill-spawned subagent that has no link at all", () => { + // A /code-review run: no Task tool_use exists to name it, so its sidecar carries only + // agentType and its id appears nowhere in the parent transcript. + const descriptor = applyToStore( + observeReplaySubagents({ + subagents: [ + { + agentId: AGENT_ID, + meta: { agentType: "general-purpose" }, + entries: [ + { + type: "assistant", + timestamp: "2026-07-31T03:41:00.000Z", + message: { stop_reason: "end_turn" }, + }, + ], + }, + ], + parent: emptyParent(), + convertEntry: () => [], + }), + ); + + expect(descriptor).toBeNull(); + }); + + it("reports a scraped link's failure as failed", () => { + const descriptor = applyToStore( + observeReplaySubagents({ + subagents: [{ agentId: AGENT_ID, meta: null, entries: [] }], + parent: { + toolCalls: new Map([[TOOL_USE_ID, { title: "Explore" }]]), + linksByAgentId: new Map([[AGENT_ID, { toolCallId: TOOL_USE_ID, failed: true }]]), + outcomesByToolCallId: new Map(), + }, + convertEntry: () => [], + }), + ); + + expect(descriptor).toMatchObject({ id: TOOL_USE_ID, status: "failed" }); + }); + + it("drops a non-terminal linkless subagent", () => { + const observations = observeReplaySubagents({ + subagents: [ + { + agentId: AGENT_ID, + meta: { agentType: "general-purpose" }, + entries: [ + { + type: "assistant", + timestamp: "2026-07-31T03:41:00.000Z", + message: { stop_reason: null }, + }, + ], + }, + ], + parent: emptyParent(), + convertEntry: () => [], + }); + + expect(observations).toEqual([]); + }); + + it("finishes a terminal Task child when the parent has not recorded its outcome", () => { + const descriptor = applyToStore( + observeReplaySubagents({ + subagents: [ + { + agentId: AGENT_ID, + meta: { toolUseId: TOOL_USE_ID }, + entries: [ + { + type: "assistant", + timestamp: "2026-07-31T03:41:00.000Z", + message: { stop_reason: "end_turn" }, + }, + ], + }, + ], + parent: { ...parentWithTaskCall(), outcomesByToolCallId: new Map() }, + convertEntry: () => [], + }), + ); + + expect(descriptor).toMatchObject({ status: "completed" }); }); it("leaves a subagent with no recorded outcome running", () => { diff --git a/packages/server/src/server/agent/providers/claude/subagents/replay-source.ts b/packages/server/src/server/agent/providers/claude/subagents/replay-source.ts index 61fc2c54f..99eb8e6c6 100644 --- a/packages/server/src/server/agent/providers/claude/subagents/replay-source.ts +++ b/packages/server/src/server/agent/providers/claude/subagents/replay-source.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import type { AgentTimelineItem } from "../../../agent-sdk-types.js"; import { normalizeProviderReplayTimestamp } from "../../../provider-history-timestamps.js"; +import type { ProviderSubagentStatus } from "../../../provider-subagents/store.js"; import { resolveObservedClaudeModelId } from "../models.js"; import type { SubagentObservation } from "./observation.js"; import { buildClaudeSubagentSubtitle, type ClaudeSubagentUsage } from "./presentation.js"; @@ -152,51 +153,81 @@ export interface ClaudeReplayParentFacts { outcomesByToolCallId: ReadonlyMap; } -interface ResolvedLink { +interface ParentLink { id: string; - toolCallId: string | null; - failed: boolean | null; + toolCallId: string; + /** Terminal status recorded by the parent, or null when its Task has no outcome yet. */ + status: ProviderSubagentStatus | null; } /** - * Resolve the canonical subagent id. + * Resolve a child only when the parent transcript declares the corresponding Task. * - * `meta.toolUseId` is authoritative: it is the same id the live stream keys on. The legacy path - * scrapes `agentId:` out of stringified tool-result content, which both misses the link when the - * parent's tool_result is absent and matches unrelated text — on a real five-subagent session it - * produced eight ids, inventing `z`, `string`, and `update`. It stays only as a fallback for - * sessions recorded before the meta file existed. + * `meta.toolUseId` is authoritative for identity — it is the same id the live stream keys on. The + * legacy path scrapes `agentId:` out of stringified tool-result content, which both misses the link + * when the parent's tool_result is absent and matches unrelated text — on a real five-subagent + * session it produced eight ids, inventing `z`, `string`, and `update`. It stays only as a fallback + * for sessions recorded before the meta file existed. + * + * ``` + * meta.toolUseId matches a parent Task? + * ├─ yes → use it and the parent's outcome, when present + * └─ no → a legacy scraped agentId link matches a parent Task? + * ├─ yes → use it and its recorded outcome + * └─ no → drop it; live never declared this child either + * ``` + * + * Claude writes grandchildren and skill-owned sidechains beside direct children. Admitting those + * without a parent Task creates replay-only rows with no identity or lifecycle source. Filtering + * here keeps live and replay on the same declaration boundary. */ -function resolveLink( +function resolveParentLink( subagent: ClaudeReplaySubagentInput, parent: ClaudeReplayParentFacts, -): ResolvedLink { +): ParentLink | null { const metaToolUseId = subagent.meta?.toolUseId?.trim(); - if (metaToolUseId) { + if (metaToolUseId && parent.toolCalls.has(metaToolUseId)) { const outcome = parent.outcomesByToolCallId.get(metaToolUseId); + let status: ProviderSubagentStatus | null = null; + if (outcome) status = outcome.failed ? "failed" : "completed"; return { id: metaToolUseId, toolCallId: metaToolUseId, - failed: outcome ? outcome.failed : null, + status, }; } const scraped = parent.linksByAgentId.get(subagent.agentId); - if (scraped) { - return { id: scraped.toolCallId, toolCallId: scraped.toolCallId, failed: scraped.failed }; + if (scraped && parent.toolCalls.has(scraped.toolCallId)) { + return { + id: scraped.toolCallId, + toolCallId: scraped.toolCallId, + status: scraped.failed ? "failed" : "completed", + }; } - // No link at all: keep the subagent visible under its own agent id rather than dropping it. - return { id: subagent.agentId, toolCallId: null, failed: null }; + return null; +} + +/** A final `end_turn` is the child's own completion signal when the parent result is absent. */ +function readChildTerminalStatus( + entries: readonly ClaudeReplayEntry[], +): ProviderSubagentStatus | null { + for (let index = entries.length - 1; index >= 0; index--) { + const entry = entries[index]; + if (entry?.type !== "assistant") continue; + return entry.message?.stop_reason === "end_turn" ? "completed" : null; + } + return null; } /** - * The parent's tool input is the same source the live path reads, so it wins; meta.json fills the - * gap when the parent's Task call is missing from the transcript. + * The parent's tool input is the same identity source the live path reads; meta.json contributes + * provider details only after that declaration has admitted the child. */ function declareSubagent( subagent: ClaudeReplaySubagentInput, - link: ResolvedLink, + link: ParentLink, toolCall: { title?: string; description?: string } | undefined, ): SubagentObservation { const title = toolCall?.title ?? subagent.meta?.agentType; @@ -214,7 +245,7 @@ function declareSubagent( function observeSubtitle( subagent: ClaudeReplaySubagentInput, - link: ResolvedLink, + link: ParentLink, title: string | undefined, ): SubagentObservation | null { const runtime = readRuntime(subagent.entries); @@ -244,8 +275,9 @@ function observeSubagent( parent: ClaudeReplayParentFacts, convertEntry: (entry: ClaudeReplayEntry) => AgentTimelineItem[], ): SubagentObservation[] { - const link = resolveLink(subagent, parent); - const toolCall = link.toolCallId ? parent.toolCalls.get(link.toolCallId) : undefined; + const link = resolveParentLink(subagent, parent); + if (!link) return []; + const toolCall = parent.toolCalls.get(link.toolCallId); const observations: SubagentObservation[] = [declareSubagent(subagent, link, toolCall)]; const subtitle = observeSubtitle(subagent, link, toolCall?.title ?? subagent.meta?.agentType); @@ -263,14 +295,15 @@ function observeSubagent( } } - // A transcript with no recorded outcome is a subagent that never finished — leave it running - // rather than inventing a completion. - if (link.failed !== null) { + // Prefer the parent's result because it carries failure. If that result is missing, the child's + // final end_turn still proves completion. Any other shape remains running. + const terminalStatus = link.status ?? readChildTerminalStatus(subagent.entries); + if (terminalStatus) { const lastTimestamp = normalizeProviderReplayTimestamp(subagent.entries.at(-1)?.timestamp); observations.push({ kind: "status", id: link.id, - status: link.failed ? "failed" : "completed", + status: terminalStatus, ...(lastTimestamp ? { timestamp: lastTimestamp } : {}), }); } @@ -286,9 +319,8 @@ function observeSubagent( * another subagent. A grandchild's `toolUseId` names a Task call made inside its parent's session, * so nothing in this transcript can resolve it — surveyed across recorded sessions, every depth-1 * id matched a Task tool_use block in the parent transcript and no deeper id did. Replaying them - * anyway adds rows the live stream never showed, each with no Task card and no recoverable - * outcome, which leaves them running forever. One session recorded 10 subagents live and would - * replay 22. + * anyway adds rows the live stream never showed, each with no Task card and no recoverable outcome. + * One session recorded 10 subagents live and would replay 22. * * Absent depth means a session recorded before the field existed: keep the subagent rather than * lose it, matching how every other missing field here is treated. From eb95a6eea6266fda5bd41f38e629566672aeaa13 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 6 Aug 2026 00:05:21 +0200 Subject: [PATCH 2/2] Keep desktop browser tabs stable across focus and automation (#2907) * fix(desktop): preserve browser tabs across focus changes Electron can replace a guest WebContents when a webview is reparented, which reloads the page and can race automation against a destroyed target. Keep each browser guest on one permanent paintable surface and make user and agent resizing share one canonical viewport. * fix(desktop): recover browser surfaces after lifecycle changes * fix(desktop): preserve resident browser lifecycle state * fix(desktop): seed inactive browser viewport --- docs/browser-capture-harness.md | 11 +- .../browser/automation/handler.test.ts | 5 + .../src/desktop/browser/automation/handler.ts | 10 +- .../desktop/browser/pane/index.electron.tsx | 166 ++++++++++--- .../browser/resident-webviews.browser.test.ts | 196 +++++++++++++-- .../src/desktop/browser/resident-webviews.ts | 183 ++++++++++++-- .../app/src/desktop/browser/store/index.ts | 17 +- .../src/desktop/browser/store/state.test.ts | 43 ++++ .../app/src/desktop/browser/store/state.ts | 71 +++++- packages/desktop/e2e/browser-tabs.e2e.mjs | 223 +++++++++++++++++- .../features/browser-automation/ipc.test.ts | 78 +++++- .../src/features/browser-automation/ipc.ts | 10 + 12 files changed, 929 insertions(+), 84 deletions(-) diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 983801c7e..0038774dc 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -5,7 +5,7 @@ It validates the compositor behavior that unit tests cannot see: - the resident automation `` starts in the production parking state; - the parked guest remains paintable and has a copyable viewport frame; -- the resident webview guest is sized to 1280x800 logical pixels; +- a never-presented resident webview guest defaults to 1280x800 logical pixels; - multiple resident webviews are parked as an overlapping stack without per-capture stacking changes; - a newly attached resident webview whose first useful frame is delayed can be captured @@ -82,10 +82,11 @@ is usually saved as 2560x1600. Electron captures copy from the guest web contents' compositor surface. A resident webview parked with `display:none`, offscreen coordinates, or `opacity:0` can lose its -copyable surface. The production parking state keeps the host fixed at `left:0`, `top:0`, -`width:1px`, `height:1px`, `overflow:hidden`, `opacity:1`, and `pointer-events:none`. -The webviews inside stay full-size at 1280x800, `display:inline-flex`, and absolutely -overlap at `left:0`, `top:0`. +copyable surface. Each production webview keeps one permanent body-level surface. Presenting +or parking changes that surface's geometry without reparenting the webview. The parking state +uses `left:0`, `top:0`, `width:1px`, `height:1px`, `overflow:hidden`, `opacity:1`, and +`pointer-events:none`. The webview stays at its resolved logical viewport, defaulting to +1280x800 before first presentation, with `display:inline-flex` at `left:0`, `top:0`. There is no renderer prep/restore handshake. Main disables guest background throttling once when the webview attaches, then screenshot capture uses the shared serialized queue, diff --git a/packages/app/src/desktop/browser/automation/handler.test.ts b/packages/app/src/desktop/browser/automation/handler.test.ts index 56c3c28df..81e1f0bb6 100644 --- a/packages/app/src/desktop/browser/automation/handler.test.ts +++ b/packages/app/src/desktop/browser/automation/handler.test.ts @@ -411,6 +411,11 @@ describe("mountBrowserAutomationHandler", () => { height: 768, }, }); + expect(useBrowserStore.getState().browsersById[result.browserId]?.viewport).toEqual({ + mode: "fixed", + width: 1024, + height: 768, + }); expect(browser.browser.executedRequests).toHaveLength(1); }); diff --git a/packages/app/src/desktop/browser/automation/handler.ts b/packages/app/src/desktop/browser/automation/handler.ts index a75e666fa..b646ee9ed 100644 --- a/packages/app/src/desktop/browser/automation/handler.ts +++ b/packages/app/src/desktop/browser/automation/handler.ts @@ -5,7 +5,12 @@ import { removeResidentBrowserWebview, resizeResidentBrowserWebview, } from "@/desktop/browser/resident-webviews"; -import { createWorkspaceBrowser, getBrowserRecord, useBrowserStore } from "@/desktop/browser/store"; +import { + createFixedBrowserViewport, + createWorkspaceBrowser, + getBrowserRecord, + useBrowserStore, +} from "@/desktop/browser/store"; import { collectAllTabs, useWorkspaceLayoutStore } from "@/stores/workspace-layout-store"; import { buildWorkspaceTabPersistenceKey } from "@/workspace-tabs/model"; @@ -208,6 +213,9 @@ function resizeBrowserTabForRequest(params: { message: `No browser tab found for ID: ${browserId}`, }); } + useBrowserStore + .getState() + .setBrowserViewport(browserId, createFixedBrowserViewport(dimensions.width, dimensions.height)); return { requestId: request.requestId, diff --git a/packages/app/src/desktop/browser/pane/index.electron.tsx b/packages/app/src/desktop/browser/pane/index.electron.tsx index 5130d9ce0..5daa3fbd5 100644 --- a/packages/app/src/desktop/browser/pane/index.electron.tsx +++ b/packages/app/src/desktop/browser/pane/index.electron.tsx @@ -28,6 +28,7 @@ import { StyleSheet, useUnistyles, withUnistyles } from "react-native-unistyles" import { useTranslation } from "react-i18next"; import * as Clipboard from "expo-clipboard"; import { Button } from "@/components/ui/button"; +import { useRetainedPanelActive } from "@/components/retained-panel"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { useToast } from "@/contexts/toast-context"; import { @@ -49,10 +50,22 @@ import { isElectronRuntime, type DesktopBrowserShortcutEvent, } from "@/desktop/host"; -import { useBrowserStore, normalizeWorkspaceBrowserUrl } from "@/desktop/browser/store"; import { + type BrowserViewport, + createFixedBrowserViewport, + normalizeWorkspaceBrowserUrl, + RESPONSIVE_BROWSER_VIEWPORT, + useBrowserStore, +} from "@/desktop/browser/store"; +import { + applyBrowserWebviewViewport, + applyInactiveBrowserWebviewViewport, + isResidentBrowserWebviewReady, prepareBrowserWebview, + presentBrowserWebview, + rememberBrowserWebviewSize, releaseResidentBrowserWebview, + removeResidentBrowserWebview, takeResidentBrowserWebview, } from "../resident-webviews"; @@ -551,7 +564,7 @@ function DeviceSizeMenu({ onSelect, triggerStyle, }: { - selectedId: DeviceSizeId; + selectedId: DeviceSizeId | null; onSelect: (id: DeviceSizeId) => void; triggerStyle: (state: { hovered?: boolean; pressed?: boolean }) => StyleProp; }) { @@ -590,6 +603,22 @@ function DeviceSizeMenu({ ); } +function deviceSizeIdForViewport(viewport: BrowserViewport): DeviceSizeId | null { + if (viewport.mode === "responsive") { + return "responsive"; + } + return ( + DEVICE_SIZE_PRESETS.find( + (preset) => preset.width === viewport.width && preset.height === viewport.height, + )?.id ?? null + ); +} + +function rememberResolvedBrowserWebviewSize(browserId: string, webview: HTMLElement): void { + const bounds = webview.getBoundingClientRect(); + rememberBrowserWebviewSize({ browserId, width: bounds.width, height: bounds.height }); +} + // eslint-disable-next-line complexity export function BrowserPane({ browserId, @@ -610,8 +639,16 @@ export function BrowserPane({ const { t } = useTranslation(); const browser = useBrowserStore((state) => state.browsersById[browserId] ?? null); const updateBrowser = useBrowserStore((state) => state.updateBrowser); + const setBrowserViewport = useBrowserStore((state) => state.setBrowserViewport); + const browserViewport = browser?.viewport ?? RESPONSIVE_BROWSER_VIEWPORT; + const browserViewportRef = useRef(browserViewport); + browserViewportRef.current = browserViewport; + const isPresented = useRetainedPanelActive(); + const isPresentedRef = useRef(isPresented); + isPresentedRef.current = isPresented; const webviewRef = useRef(null); const webviewHostRef = useRef(null); + const webviewClipRef = useRef(null); const urlInputRef = useRef(null); const initialUrlRef = useRef(browser?.url ?? "https://example.com"); const browserIdRef = useRef(browserId); @@ -629,7 +666,6 @@ export function BrowserPane({ const toast = useToast(); const toastRef = useRef(toast); toastRef.current = toast; - const [deviceSizeId, setDeviceSizeId] = useState("responsive"); const [pendingSelection, setPendingSelection] = useState(null); // Screenshot is captured at selection time (overlay already torn down, no // scroll drift) and reused when the annotation card is submitted. @@ -727,7 +763,8 @@ export function BrowserPane({ } const host = webviewHostRef.current; - if (!host) { + const clip = webviewClipRef.current; + if (!host || !clip) { return; } @@ -740,6 +777,7 @@ export function BrowserPane({ const residentWebview = takeResidentBrowserWebview(browserId) as ElectronWebview | null; const webview = residentWebview ?? (document.createElement("webview") as ElectronWebview); webviewRef.current = webview; + domReadyRef.current = isResidentBrowserWebviewReady(webview); if (!residentWebview) { prepareBrowserWebview(webview, { browserId, @@ -747,14 +785,26 @@ export function BrowserPane({ initialUrl: initialUnsafeNavigationMessage ? "about:blank" : initialUrlRef.current, }); } - webview.style.display = "flex"; - webview.style.flex = "1"; - webview.style.width = "100%"; - webview.style.height = "100%"; - webview.style.border = "0"; - webview.style.background = "transparent"; + releaseResidentBrowserWebview(browserId, webview); + if (isPresentedRef.current) { + applyBrowserWebviewViewport(webview, browserViewportRef.current); + presentBrowserWebview(browserId, webview, host, clip); + } else { + applyInactiveBrowserWebviewViewport(browserId, webview, browserViewportRef.current); + } + const sizeObserver = + typeof ResizeObserver === "undefined" + ? null + : new ResizeObserver(() => { + if (!isPresentedRef.current) { + return; + } + presentBrowserWebview(browserIdRef.current, webview, host, clip); + rememberResolvedBrowserWebviewSize(browserIdRef.current, webview); + }); const handleStartLoading = () => { + domReadyRef.current = false; updateBrowser(browserId, { isLoading: true, lastError: null }); syncNavigationState({ syncUrl: false }); }; @@ -846,7 +896,11 @@ export function BrowserPane({ webview.addEventListener("focus", handleWebviewFocus); webview.addEventListener("mousedown", handleWebviewFocus); - host.appendChild(webview); + if (isPresentedRef.current) { + rememberResolvedBrowserWebviewSize(browserId, webview); + } + sizeObserver?.observe(host); + sizeObserver?.observe(clip); if (initialUnsafeNavigationMessage) { updateBrowserRef.current(browserIdRef.current, { isLoading: false, @@ -855,6 +909,7 @@ export function BrowserPane({ } return () => { + sizeObserver?.disconnect(); webview.removeEventListener("did-start-loading", handleStartLoading); webview.removeEventListener("did-stop-loading", handleStopLoading); webview.removeEventListener("will-navigate", handleWillNavigate); @@ -866,15 +921,13 @@ export function BrowserPane({ webview.removeEventListener("dom-ready", handleDomReady); webview.removeEventListener("focus", handleWebviewFocus); webview.removeEventListener("mousedown", handleWebviewFocus); - if (host.contains(webview)) { - const browserStillExists = Boolean( - useBrowserStore.getState().browsersById[browserIdRef.current], - ); - if (browserStillExists) { - releaseResidentBrowserWebview(browserIdRef.current, webview); - } else { - host.removeChild(webview); - } + const browserStillExists = Boolean( + useBrowserStore.getState().browsersById[browserIdRef.current], + ); + if (browserStillExists) { + releaseResidentBrowserWebview(browserIdRef.current, webview); + } else { + removeResidentBrowserWebview(browserIdRef.current); } if (webviewRef.current === webview) { webviewRef.current = null; @@ -884,6 +937,32 @@ export function BrowserPane({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [browserId, onFocusPane]); + useEffect(() => { + const webview = webviewRef.current; + if (!webview) { + return; + } + if (!isPresented) { + releaseResidentBrowserWebview(browserId, webview); + return; + } + applyBrowserWebviewViewport(webview, browserViewport); + const host = webviewHostRef.current; + const clip = webviewClipRef.current; + if (host && clip) { + presentBrowserWebview(browserId, webview, host, clip); + } + if (browserViewport.mode === "fixed") { + rememberBrowserWebviewSize({ + browserId, + width: browserViewport.width, + height: browserViewport.height, + }); + } else { + rememberResolvedBrowserWebviewSize(browserId, webview); + } + }, [browserId, browserViewport, isPresented]); + const navigate = useCallback( (nextUrl: string) => { const normalizedUrl = normalizeWorkspaceBrowserUrl(nextUrl); @@ -1525,12 +1604,26 @@ export function BrowserPane({ [selectorMode], ); - const devicePreset = useMemo( - () => - DEVICE_SIZE_PRESETS.find((preset) => preset.id === deviceSizeId) ?? DEVICE_SIZE_PRESETS[0], - [deviceSizeId], + const selectedDeviceSizeId = useMemo( + () => deviceSizeIdForViewport(browserViewport), + [browserViewport], + ); + const isResponsiveDevice = browserViewport.mode === "responsive"; + + const handleSelectDeviceSize = useCallback( + (deviceSizeId: DeviceSizeId) => { + const preset = + DEVICE_SIZE_PRESETS.find((candidate) => candidate.id === deviceSizeId) ?? + DEVICE_SIZE_PRESETS[0]; + setBrowserViewport( + browserId, + preset.width === null || preset.height === null + ? RESPONSIVE_BROWSER_VIEWPORT + : createFixedBrowserViewport(preset.width, preset.height), + ); + }, + [browserId, setBrowserViewport], ); - const isResponsiveDevice = devicePreset.width === null; const webviewHostStyle = useMemo( () => @@ -1546,15 +1639,13 @@ export function BrowserPane({ : { // Fixed-size device frame, centered within webviewWrap (see styles). display: "flex", - width: devicePreset.width ?? undefined, - maxWidth: "100%", - height: devicePreset.height ?? undefined, - maxHeight: "100%", + width: browserViewport.width, + height: browserViewport.height, minHeight: 0, background: theme.colors.surface0, boxShadow: "0 2px 16px rgba(0,0,0,0.25)", }, - [devicePreset.height, devicePreset.width, isResponsiveDevice, theme.colors.surface0], + [browserViewport, isResponsiveDevice, theme.colors.surface0], ); const webviewWrapStyle = useMemo( @@ -1566,6 +1657,10 @@ export function BrowserPane({ webviewHostRef.current = node; }, []); + const setWebviewClipNode = useCallback((node: unknown) => { + webviewClipRef.current = node instanceof HTMLElement ? node : null; + }, []); + if (!isElectronRuntime()) { return ( @@ -1624,8 +1719,8 @@ export function BrowserPane({ ) : null} - + {createElement("div", { ref: setWebviewHostNode, style: webviewHostStyle, @@ -1884,6 +1983,7 @@ const styles = StyleSheet.create((theme) => ({ }, annotationOverlay: { position: "absolute", + zIndex: 1, left: 0, right: 0, bottom: 0, diff --git a/packages/app/src/desktop/browser/resident-webviews.browser.test.ts b/packages/app/src/desktop/browser/resident-webviews.browser.test.ts index b3208205b..8d19983cd 100644 --- a/packages/app/src/desktop/browser/resident-webviews.browser.test.ts +++ b/packages/app/src/desktop/browser/resident-webviews.browser.test.ts @@ -1,12 +1,18 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { + applyBrowserWebviewViewport, + applyInactiveBrowserWebviewViewport, type BrowserWebviewProfileHost, clearResidentBrowserWebviewsForTests, ensureResidentBrowserWebview, getResidentBrowserWebview, + isResidentBrowserWebviewReady, prepareBrowserWebview, + presentBrowserWebview, + rememberBrowserWebviewSize, releaseResidentBrowserWebview, removeResidentBrowserWebview, + resizeResidentBrowserWebview, takeResidentBrowserWebview, } from "./resident-webviews"; import { @@ -54,14 +60,28 @@ function expectPermanentHostParking(host: HTMLElement): void { expect(host.style.position).toBe("fixed"); expect(host.style.left).toBe("0px"); expect(host.style.top).toBe("0px"); - expect(host.style.width).toBe("1px"); - expect(host.style.height).toBe("1px"); - expect(host.style.overflow).toBe("hidden"); + expect(host.style.width).toBe("100vw"); + expect(host.style.height).toBe("100vh"); + expect(host.style.overflow).toBe("visible"); expect(host.style.opacity).toBe("1"); expect(host.style.pointerEvents).toBe("none"); expect(host.style.display).toBe("block"); - expect(host.style.visibility).toBe("visible"); - expect(host.style.transform).toBe(""); + expect(host.style.zIndex).toBe("0"); +} + +function expectParkedSurface(surface: HTMLElement): void { + expect(surface.getAttribute("aria-hidden")).toBe("true"); + expect(surface.style.position).toBe("fixed"); + expect(surface.style.left).toBe("0px"); + expect(surface.style.top).toBe("0px"); + expect(surface.style.width).toBe("1px"); + expect(surface.style.height).toBe("1px"); + expect(surface.style.overflow).toBe("hidden"); + expect(surface.style.opacity).toBe("1"); + expect(surface.style.pointerEvents).toBe("none"); + expect(surface.style.display).toBe("block"); + expect(surface.style.visibility).toBe("visible"); + expect(surface.style.transform).toBe(""); } function expectResidentWebviewParking(webview: HTMLElement): void { @@ -94,12 +114,110 @@ describe("resident browser webviews", () => { const host = residentHost(); expect(visibleHost.children).toHaveLength(0); - expect(Array.from(host.children)).toEqual([webview]); + const surface = host.firstElementChild as HTMLElement; + expect(Array.from(surface.children)).toEqual([webview]); expect(webview.isConnected).toBe(true); expectPermanentHostParking(host); + expectParkedSurface(surface); expectResidentWebviewParking(webview); }); + it("presents and parks a browser without changing its webview parent", () => { + const webview = ensureTestBrowser({ + browserId: "browser-stable-parent", + workspaceId: "workspace-stable-parent", + url: "https://example.com", + }); + const anchor = document.createElement("div"); + const clip = document.createElement("div"); + Object.defineProperty(anchor, "getBoundingClientRect", { + value: () => ({ left: 40, top: 60, width: 640, height: 480 }), + }); + Object.defineProperty(clip, "getBoundingClientRect", { + value: () => ({ left: 40, top: 60, width: 640, height: 480 }), + }); + if (!webview?.parentElement) { + throw new Error("Expected resident browser surface"); + } + const permanentParent = webview.parentElement; + + presentBrowserWebview("browser-stable-parent", webview, anchor, clip); + expect(webview.parentElement).toBe(permanentParent); + expect(permanentParent.style.left).toBe("40px"); + expect(permanentParent.style.top).toBe("60px"); + expect(permanentParent.style.width).toBe("640px"); + expect(permanentParent.style.height).toBe("480px"); + expect(permanentParent.style.pointerEvents).toBe("auto"); + expect(permanentParent.getAttribute("aria-hidden")).toBe("false"); + + releaseResidentBrowserWebview("browser-stable-parent", webview); + expect(webview.parentElement).toBe(permanentParent); + expectParkedSurface(permanentParent); + + presentBrowserWebview("browser-stable-parent", webview, anchor, clip); + expect(webview.parentElement).toBe(permanentParent); + }); + + it("clips an oversized fixed viewport to its pane without resizing the webview", () => { + const webview = ensureTestBrowser({ + browserId: "browser-oversized", + workspaceId: "workspace-oversized", + url: "https://example.com", + }); + if (!webview?.parentElement) { + throw new Error("Expected resident browser surface"); + } + applyBrowserWebviewViewport(webview, { mode: "fixed", width: 2560, height: 1440 }); + const anchor = document.createElement("div"); + const clip = document.createElement("div"); + Object.defineProperty(anchor, "getBoundingClientRect", { + value: () => ({ left: -800, top: -300, width: 2560, height: 1440 }), + }); + Object.defineProperty(clip, "getBoundingClientRect", { + value: () => ({ left: 100, top: 150, width: 800, height: 600 }), + }); + + presentBrowserWebview("browser-oversized", webview, anchor, clip); + + expect(webview.parentElement.style.left).toBe("100px"); + expect(webview.parentElement.style.top).toBe("150px"); + expect(webview.parentElement.style.width).toBe("800px"); + expect(webview.parentElement.style.height).toBe("600px"); + expect(webview.style.left).toBe("-900px"); + expect(webview.style.top).toBe("-450px"); + expect(webview.style.width).toBe("2560px"); + expect(webview.style.height).toBe("1440px"); + + resizeResidentBrowserWebview({ browserId: "browser-oversized", width: 2560, height: 1440 }); + + expect(webview.style.left).toBe("-900px"); + expect(webview.style.top).toBe("-450px"); + expect(webview.parentElement.style.width).toBe("800px"); + expect(webview.parentElement.style.height).toBe("600px"); + }); + + it("retains document readiness with the resident webview", () => { + const webview = ensureTestBrowser({ + browserId: "browser-ready", + workspaceId: "workspace-ready", + url: "https://example.com", + }); + if (!webview) { + throw new Error("Expected resident browser webview"); + } + + expect(isResidentBrowserWebviewReady(webview)).toBe(false); + webview.dispatchEvent(new Event("dom-ready")); + releaseResidentBrowserWebview("browser-ready", webview); + + const reusedWebview = takeResidentBrowserWebview("browser-ready"); + expect(reusedWebview).toBe(webview); + expect(isResidentBrowserWebviewReady(reusedWebview as HTMLElement)).toBe(true); + + webview.dispatchEvent(new Event("did-start-loading")); + expect(isResidentBrowserWebviewReady(webview)).toBe(false); + }); + it("creates a resident webview for an agent-created unfocused tab", () => { const webview = ensureTestBrowser({ browserId: "browser-agent", @@ -197,8 +315,10 @@ describe("resident browser webviews", () => { }); expect(webview).toBe(staleWebview); - expect(Array.from(staleHost.children)).toEqual([staleWebview]); + const surface = staleHost.firstElementChild as HTMLElement; + expect(Array.from(surface.children)).toEqual([staleWebview]); expectPermanentHostParking(staleHost); + expectParkedSurface(surface); expectResidentWebviewParking(staleWebview); }); @@ -215,27 +335,73 @@ describe("resident browser webviews", () => { }); const host = residentHost(); - expect(firstWebview?.parentElement).toBe(host); - expect(secondWebview?.parentElement).toBe(host); + expect(firstWebview?.parentElement?.parentElement).toBe(host); + expect(secondWebview?.parentElement?.parentElement).toBe(host); + expect(firstWebview?.parentElement).not.toBe(secondWebview?.parentElement); expectResidentWebviewParking(firstWebview as HTMLElement); expectResidentWebviewParking(secondWebview as HTMLElement); }); - it("moves a resident webview into a visible pane without recreating the node", () => { + it("returns a resident webview without detaching it from its permanent surface", () => { const webview = ensureTestBrowser({ browserId: "browser-visible", workspaceId: "workspace-visible", url: "https://example.com", }); + const permanentParent = webview?.parentElement; const visibleWebview = takeResidentBrowserWebview("browser-visible"); expect(visibleWebview).toBe(webview); - expect(webview?.style.position).toBe(""); - expect(webview?.style.left).toBe(""); - expect(webview?.style.top).toBe(""); - expect(webview?.style.zIndex).toBe(""); - expect(takeResidentBrowserWebview("browser-visible")).toBeNull(); + expect(webview?.parentElement).toBe(permanentParent); + expect(takeResidentBrowserWebview("browser-visible")).toBe(webview); + }); + + it("applies an exact fixed viewport without a flex width override", () => { + const webview = document.createElement("webview"); + + applyBrowserWebviewViewport(webview, { mode: "fixed", width: 640, height: 480 }); + + expect(webview.style.flex).toBe("0 0 auto"); + expect(webview.style.width).toBe("640px"); + expect(webview.style.height).toBe("480px"); + }); + + it("parks a browser at its last resolved viewport dimensions", () => { + const visibleHost = document.createElement("div"); + const webview = document.createElement("webview"); + visibleHost.appendChild(webview); + document.body.appendChild(visibleHost); + rememberBrowserWebviewSize({ browserId: "browser-sized", width: 640, height: 480 }); + + releaseResidentBrowserWebview("browser-sized", webview); + + expect(webview.style.width).toBe("640px"); + expect(webview.style.height).toBe("480px"); + }); + + it("keeps an inactive in-place browser at its last resolved viewport dimensions", () => { + const webview = document.createElement("webview"); + rememberBrowserWebviewSize({ browserId: "browser-inactive", width: 640, height: 480 }); + + applyInactiveBrowserWebviewViewport("browser-inactive", webview, { mode: "responsive" }); + + expect(webview.style.flex).toBe("0 0 auto"); + expect(webview.style.width).toBe("640px"); + expect(webview.style.height).toBe("480px"); + }); + + it("parks a cold inactive browser at its canonical fixed viewport", () => { + const webview = document.createElement("webview"); + + applyInactiveBrowserWebviewViewport("browser-restored-fixed", webview, { + mode: "fixed", + width: 800, + height: 600, + }); + + expect(webview.style.width).toBe("800px"); + expect(webview.style.height).toBe("600px"); }); it("returns an existing visible pane webview instead of creating a resident duplicate", () => { diff --git a/packages/app/src/desktop/browser/resident-webviews.ts b/packages/app/src/desktop/browser/resident-webviews.ts index 92577f7da..65d4b3e02 100644 --- a/packages/app/src/desktop/browser/resident-webviews.ts +++ b/packages/app/src/desktop/browser/resident-webviews.ts @@ -3,14 +3,18 @@ import { type DesktopAttachedBrowserRegistration, type DesktopBrowserBridge, } from "@/desktop/host"; +import type { BrowserViewport } from "@/desktop/browser/store"; const RESIDENT_BROWSER_HOST_ID = "paseo-browser-resident-webviews"; const BROWSER_ID_ATTRIBUTE = "data-paseo-browser-id"; +const BROWSER_SURFACE_ATTRIBUTE = "data-paseo-browser-surface"; const RESIDENT_VIEWPORT_WIDTH = 1280; const RESIDENT_VIEWPORT_HEIGHT = 800; const residentWebviewsByBrowserId = new Map(); +const residentSurfacesByBrowserId = new Map(); const residentWebviewSizesByBrowserId = new Map(); +const readyResidentWebviews = new WeakSet(); interface BrowserWebviewElement extends HTMLElement { src: string; @@ -70,6 +74,15 @@ function registerBrowserWhenAttached( }); } +function registerBrowserReadiness(webview: HTMLElement): void { + webview.addEventListener("did-start-loading", () => { + readyResidentWebviews.delete(webview); + }); + webview.addEventListener("dom-ready", () => { + readyResidentWebviews.add(webview); + }); +} + function trimNonEmpty(value: string | null | undefined): string | null { if (typeof value !== "string") { return null; @@ -83,24 +96,52 @@ function readDocument(): Document | null { } function applyResidentHostParkingStyle(host: HTMLElement): void { - // Parked browser webviews must remain paintable at all times; screenshot - // correctness depends on the proven states in docs/browser-capture-harness.md. - host.setAttribute("aria-hidden", "true"); + // The host is permanent. Individual browser surfaces switch between their + // pane bounds and the proven paintable 1x1 parking geometry. + host.removeAttribute("aria-hidden"); host.style.position = "fixed"; host.style.left = "0"; host.style.top = "0"; - host.style.width = "1px"; - host.style.height = "1px"; - host.style.overflow = "hidden"; + host.style.width = "100vw"; + host.style.height = "100vh"; + host.style.overflow = "visible"; host.style.opacity = "1"; host.style.pointerEvents = "none"; host.style.display = "block"; - host.style.zIndex = ""; + host.style.zIndex = "0"; host.style.clipPath = ""; host.style.visibility = "visible"; host.style.transform = ""; } +function applyParkedBrowserSurfaceStyle(surface: HTMLElement): void { + surface.setAttribute("aria-hidden", "true"); + surface.style.position = "fixed"; + surface.style.left = "0"; + surface.style.top = "0"; + surface.style.width = "1px"; + surface.style.height = "1px"; + surface.style.overflow = "hidden"; + surface.style.opacity = "1"; + surface.style.pointerEvents = "none"; + surface.style.display = "block"; + surface.style.visibility = "visible"; + surface.style.transform = ""; +} + +function getBrowserSurface(browserId: string, ownerDocument: Document): HTMLElement { + const existing = residentSurfacesByBrowserId.get(browserId); + if (existing?.isConnected) { + return existing; + } + const surface = ownerDocument.createElement("div"); + surface.setAttribute(BROWSER_SURFACE_ATTRIBUTE, browserId); + applyParkedBrowserSurfaceStyle(surface); + getResidentBrowserHost(ownerDocument).appendChild(surface); + residentSurfacesByBrowserId.set(browserId, surface); + return surface; +} + function getResidentBrowserHost(ownerDocument: Document): HTMLElement { const existing = ownerDocument.getElementById(RESIDENT_BROWSER_HOST_ID); if (existing) { @@ -162,6 +203,105 @@ function clearResidentWebviewParkingStyle(webview: HTMLElement): void { webview.style.zIndex = ""; } +export function rememberBrowserWebviewSize(input: { + browserId: string; + width: number; + height: number; +}): { width: number; height: number } | null { + const browserId = trimNonEmpty(input.browserId); + if (!browserId || input.width <= 0 || input.height <= 0) { + return null; + } + const dimensions = { + width: Math.max(1, Math.round(input.width)), + height: Math.max(1, Math.round(input.height)), + }; + residentWebviewSizesByBrowserId.set(browserId, dimensions); + return dimensions; +} + +function applyBrowserWebviewDimensions(webview: HTMLElement, viewport: BrowserViewport): void { + webview.style.display = "flex"; + webview.style.border = "0"; + webview.style.background = "transparent"; + if (viewport.mode === "responsive") { + webview.style.flex = "1"; + webview.style.width = "100%"; + webview.style.height = "100%"; + return; + } + webview.style.flex = "0 0 auto"; + webview.style.width = `${viewport.width}px`; + webview.style.height = `${viewport.height}px`; +} + +export function applyBrowserWebviewViewport(webview: HTMLElement, viewport: BrowserViewport): void { + clearResidentWebviewParkingStyle(webview); + applyBrowserWebviewDimensions(webview, viewport); +} + +export function applyInactiveBrowserWebviewViewport( + browserId: string, + webview: HTMLElement, + viewport: BrowserViewport, +): void { + if (viewport.mode === "fixed") { + rememberBrowserWebviewSize({ browserId, width: viewport.width, height: viewport.height }); + } + applyResidentWebviewStyle(webview, trimNonEmpty(browserId)); +} + +export function presentBrowserWebview( + browserId: string, + webview: HTMLElement, + anchor: HTMLElement, + clip: HTMLElement, +): void { + const normalizedBrowserId = trimNonEmpty(browserId); + if (!normalizedBrowserId) { + return; + } + const ownerDocument = readDocument(); + if (!ownerDocument) { + return; + } + const surface = getBrowserSurface(normalizedBrowserId, ownerDocument); + if (webview.parentElement !== surface) { + surface.appendChild(webview); + } + const anchorBounds = anchor.getBoundingClientRect(); + const clipBounds = clip.getBoundingClientRect(); + const left = Math.max(anchorBounds.left, clipBounds.left); + const top = Math.max(anchorBounds.top, clipBounds.top); + const right = Math.min( + anchorBounds.left + anchorBounds.width, + clipBounds.left + clipBounds.width, + ); + const bottom = Math.min( + anchorBounds.top + anchorBounds.height, + clipBounds.top + clipBounds.height, + ); + const surfaceLeft = Math.ceil(left); + const surfaceTop = Math.ceil(top); + const surfaceRight = Math.floor(right); + const surfaceBottom = Math.floor(bottom); + const hasVisibleArea = surfaceRight > surfaceLeft && surfaceBottom > surfaceTop; + surface.setAttribute("aria-hidden", "false"); + surface.style.position = "fixed"; + surface.style.left = `${surfaceLeft}px`; + surface.style.top = `${surfaceTop}px`; + surface.style.width = `${Math.max(0, surfaceRight - surfaceLeft)}px`; + surface.style.height = `${Math.max(0, surfaceBottom - surfaceTop)}px`; + surface.style.overflow = "hidden"; + surface.style.opacity = "1"; + surface.style.pointerEvents = hasVisibleArea ? "auto" : "none"; + surface.style.display = "flex"; + surface.style.visibility = "visible"; + webview.style.position = "absolute"; + webview.style.left = `${Math.round(anchorBounds.left - surfaceLeft)}px`; + webview.style.top = `${Math.round(anchorBounds.top - surfaceTop)}px`; +} + export function prepareBrowserWebview( webview: HTMLElement, input: { @@ -181,6 +321,11 @@ export function prepareBrowserWebview( (webview as BrowserWebviewElement).src = input.initialUrl; } registerBrowserWhenAttached(webview as BrowserWebviewElement, input, browser); + registerBrowserReadiness(webview); +} + +export function isResidentBrowserWebviewReady(webview: HTMLElement): boolean { + return readyResidentWebviews.has(webview); } export function ensureResidentBrowserWebview(input: { @@ -247,8 +392,6 @@ export function takeResidentBrowserWebview(browserId: string): HTMLElement | nul return null; } - residentWebviewsByBrowserId.delete(normalizedBrowserId); - clearResidentWebviewParkingStyle(webview); return webview; } @@ -265,7 +408,11 @@ export function releaseResidentBrowserWebview(browserId: string, webview: HTMLEl residentWebviewsByBrowserId.set(normalizedBrowserId, webview); applyResidentWebviewStyle(webview, normalizedBrowserId); - getResidentBrowserHost(ownerDocument).appendChild(webview); + const surface = getBrowserSurface(normalizedBrowserId, ownerDocument); + applyParkedBrowserSurfaceStyle(surface); + if (webview.parentElement !== surface) { + surface.appendChild(webview); + } } export function resizeResidentBrowserWebview(input: { @@ -277,18 +424,18 @@ export function resizeResidentBrowserWebview(input: { if (!normalizedBrowserId) { return null; } - const width = Math.max(1, Math.round(input.width)); - const height = Math.max(1, Math.round(input.height)); - residentWebviewSizesByBrowserId.set(normalizedBrowserId, { width, height }); + const dimensions = rememberBrowserWebviewSize(input); + if (!dimensions) { + return null; + } const ownerDocument = readDocument(); const webview = ownerDocument ? findBrowserWebview(normalizedBrowserId, ownerDocument) : null; if (webview) { - webview.style.width = `${width}px`; - webview.style.height = `${height}px`; + applyBrowserWebviewDimensions(webview, { mode: "fixed", ...dimensions }); } - return { width, height }; + return dimensions; } export function removeResidentBrowserWebview(browserId: string): void { @@ -298,9 +445,12 @@ export function removeResidentBrowserWebview(browserId: string): void { } const resident = residentWebviewsByBrowserId.get(normalizedBrowserId) ?? null; + const surface = residentSurfacesByBrowserId.get(normalizedBrowserId) ?? null; residentWebviewsByBrowserId.delete(normalizedBrowserId); + residentSurfacesByBrowserId.delete(normalizedBrowserId); residentWebviewSizesByBrowserId.delete(normalizedBrowserId); resident?.remove(); + surface?.remove(); } export function clearResidentBrowserWebviewsForTests(): void { @@ -308,6 +458,7 @@ export function clearResidentBrowserWebviewsForTests(): void { webview.remove(); } residentWebviewsByBrowserId.clear(); + residentSurfacesByBrowserId.clear(); residentWebviewSizesByBrowserId.clear(); readDocument()?.getElementById(RESIDENT_BROWSER_HOST_ID)?.remove(); } diff --git a/packages/app/src/desktop/browser/store/index.ts b/packages/app/src/desktop/browser/store/index.ts index d7ff86b06..7ff21c733 100644 --- a/packages/app/src/desktop/browser/store/index.ts +++ b/packages/app/src/desktop/browser/store/index.ts @@ -7,18 +7,26 @@ import { type BrowserIndexState, type BrowserRecord, type BrowserRecordPatch, + type BrowserViewport, createBrowserRecord, + normalizeBrowserIndexState, normalizeBrowserUrl, removeBrowserFromIndex, sanitizeBrowsersForPersist, trimNonEmpty, } from "./state"; -export type { BrowserRecord } from "./state"; +export { + createFixedBrowserViewport, + RESPONSIVE_BROWSER_VIEWPORT, + type BrowserRecord, + type BrowserViewport, +} from "./state"; interface BrowserStoreState extends BrowserIndexState { createBrowser: (input?: { initialUrl?: string }) => string; updateBrowser: (browserId: string, patch: BrowserRecordPatch) => void; + setBrowserViewport: (browserId: string, viewport: BrowserViewport) => void; removeBrowser: (browserId: string) => void; } @@ -57,6 +65,9 @@ export const useBrowserStore = create()( updateBrowser: (browserId, patch) => { set((state) => applyBrowserPatch(state, browserId, patch)); }, + setBrowserViewport: (browserId, viewport) => { + set((state) => applyBrowserPatch(state, browserId, { viewport })); + }, removeBrowser: (browserId) => { set((state) => removeBrowserFromIndex(state, browserId)); }, @@ -65,6 +76,10 @@ export const useBrowserStore = create()( name: "workspace-browser-store", storage: createJSONStorage(() => AsyncStorage), partialize: (state) => sanitizeBrowsersForPersist(state), + merge: (persistedState, currentState) => ({ + ...currentState, + ...normalizeBrowserIndexState(persistedState), + }), }, ), ); diff --git a/packages/app/src/desktop/browser/store/state.test.ts b/packages/app/src/desktop/browser/store/state.test.ts index 02a258796..9d0d89bf5 100644 --- a/packages/app/src/desktop/browser/store/state.test.ts +++ b/packages/app/src/desktop/browser/store/state.test.ts @@ -2,7 +2,9 @@ import { describe, expect, it } from "vitest"; import { applyBrowserPatch, type BrowserIndexState, + createFixedBrowserViewport, createBrowserRecord, + normalizeBrowserIndexState, normalizeBrowserUrl, removeBrowserFromIndex, sanitizeBrowsersForPersist, @@ -57,12 +59,25 @@ describe("createBrowserRecord", () => { canGoForward: false, faviconUrl: null, lastError: null, + viewport: { mode: "responsive" }, createdAt: 1000, }); }); }); describe("applyBrowserPatch", () => { + it("updates the browser's canonical viewport", () => { + const initial = withRecords([ + createBrowserRecord({ browserId: "b1", initialUrl: "https://a.test", now: 0 }), + ]); + + const next = applyBrowserPatch(initial, "b1", { + viewport: createFixedBrowserViewport(639.6, 479.6), + }); + + expect(next.browsersById.b1?.viewport).toEqual({ mode: "fixed", width: 640, height: 480 }); + }); + it("normalizes URL updates", () => { const initial = withRecords([ createBrowserRecord({ browserId: "b1", initialUrl: "https://a.test", now: 0 }), @@ -145,4 +160,32 @@ describe("sanitizeBrowsersForPersist", () => { expect(persisted.browsersById.b1?.isLoading).toBe(false); expect(persisted.browsersById.b1?.lastError).toBe(null); }); + + it("preserves viewport state", () => { + const base = createBrowserRecord({ browserId: "b1", initialUrl: "https://a.test", now: 0 }); + const state = applyBrowserPatch(withRecords([base]), "b1", { + viewport: createFixedBrowserViewport(640, 480), + }); + + expect(sanitizeBrowsersForPersist(state).browsersById.b1?.viewport).toEqual({ + mode: "fixed", + width: 640, + height: 480, + }); + }); +}); + +describe("normalizeBrowserIndexState", () => { + it("defaults legacy persisted records to Responsive", () => { + const legacy = createBrowserRecord({ + browserId: "b1", + initialUrl: "https://a.test", + now: 0, + }) as Partial>; + delete legacy.viewport; + + expect( + normalizeBrowserIndexState({ browsersById: { b1: legacy } }).browsersById.b1?.viewport, + ).toEqual({ mode: "responsive" }); + }); }); diff --git a/packages/app/src/desktop/browser/store/state.ts b/packages/app/src/desktop/browser/store/state.ts index 1c4125e17..3909a7aa3 100644 --- a/packages/app/src/desktop/browser/store/state.ts +++ b/packages/app/src/desktop/browser/store/state.ts @@ -1,3 +1,9 @@ +export type BrowserViewport = + | { mode: "responsive" } + | { mode: "fixed"; width: number; height: number }; + +export const RESPONSIVE_BROWSER_VIEWPORT: BrowserViewport = { mode: "responsive" }; + export interface BrowserRecord { browserId: string; url: string; @@ -7,6 +13,7 @@ export interface BrowserRecord { canGoForward: boolean; faviconUrl: string | null; lastError: string | null; + viewport: BrowserViewport; createdAt: number; } @@ -16,6 +23,62 @@ export interface BrowserIndexState { browsersById: Record; } +export function createFixedBrowserViewport(width: number, height: number): BrowserViewport { + return { + mode: "fixed", + width: Math.max(1, Math.round(width)), + height: Math.max(1, Math.round(height)), + }; +} + +export function normalizeBrowserViewport(value: unknown): BrowserViewport { + if ( + value && + typeof value === "object" && + (value as { mode?: unknown }).mode === "fixed" && + typeof (value as { width?: unknown }).width === "number" && + typeof (value as { height?: unknown }).height === "number" + ) { + return createFixedBrowserViewport( + (value as { width: number }).width, + (value as { height: number }).height, + ); + } + return RESPONSIVE_BROWSER_VIEWPORT; +} + +function browserViewportsEqual(left: BrowserViewport, right: BrowserViewport): boolean { + return ( + left.mode === right.mode && + (left.mode === "responsive" || + (right.mode === "fixed" && left.width === right.width && left.height === right.height)) + ); +} + +export function normalizeBrowserIndexState(value: unknown): BrowserIndexState { + if (!value || typeof value !== "object") { + return { browsersById: {} }; + } + const browsersById = (value as { browsersById?: unknown }).browsersById; + if (!browsersById || typeof browsersById !== "object") { + return { browsersById: {} }; + } + return { + browsersById: Object.fromEntries( + Object.entries(browsersById).map(([browserId, record]) => { + const browser = record as BrowserRecord & { viewport?: unknown }; + return [ + browserId, + { + ...browser, + viewport: normalizeBrowserViewport(browser.viewport), + } satisfies BrowserRecord, + ]; + }), + ), + }; +} + export function trimNonEmpty(value: string | null | undefined): string | null { if (typeof value !== "string") { return null; @@ -55,6 +118,7 @@ export function createBrowserRecord(input: { canGoForward: false, faviconUrl: null, lastError: null, + viewport: RESPONSIVE_BROWSER_VIEWPORT, createdAt: input.now, }; } @@ -73,9 +137,13 @@ export function applyBrowserPatch( return state; } + const nextViewport = patch.viewport + ? normalizeBrowserViewport(patch.viewport) + : existing.viewport; const nextRecord: BrowserRecord = { ...existing, ...patch, + viewport: nextViewport, url: normalizeBrowserUrl(patch.url ?? existing.url), }; @@ -86,7 +154,8 @@ export function applyBrowserPatch( nextRecord.canGoBack === existing.canGoBack && nextRecord.canGoForward === existing.canGoForward && nextRecord.faviconUrl === existing.faviconUrl && - nextRecord.lastError === existing.lastError + nextRecord.lastError === existing.lastError && + browserViewportsEqual(nextRecord.viewport, existing.viewport) ) { return state; } diff --git a/packages/desktop/e2e/browser-tabs.e2e.mjs b/packages/desktop/e2e/browser-tabs.e2e.mjs index 3b2a833a9..6283f1fb0 100644 --- a/packages/desktop/e2e/browser-tabs.e2e.mjs +++ b/packages/desktop/e2e/browser-tabs.e2e.mjs @@ -272,11 +272,68 @@ async function readGuest(page, browserId) { return { webContentsId: webview.getWebContentsId(), parentId: webview.parentElement?.id ?? null, + width: Math.round(webview.getBoundingClientRect().width), + height: Math.round(webview.getBoundingClientRect().height), }; }, browserId); } -async function runRegression({ page, client, serverId, targetUrl }) { +async function readPresentation(page, browserId) { + return await page.evaluate((id) => { + const surface = document.querySelector(`[data-paseo-browser-surface="${id}"]`); + const clip = document.querySelector(`[data-testid="browser-webview-clip-${id}"]`); + if (!(surface instanceof HTMLElement) || !(clip instanceof HTMLElement)) return null; + const surfaceRect = surface.getBoundingClientRect(); + const clipRect = clip.getBoundingClientRect(); + const webview = surface.querySelector(`[data-paseo-browser-id="${id}"]`); + if (!(webview instanceof HTMLElement)) return null; + const webviewRect = webview.getBoundingClientRect(); + const outsidePoint = { + x: Math.max(0, Math.round(clipRect.left - 1)), + y: Math.round(clipRect.top + clipRect.height / 2), + }; + const outsideTarget = document.elementFromPoint(outsidePoint.x, outsidePoint.y); + return { + surface: { + left: Math.round(surfaceRect.left), + top: Math.round(surfaceRect.top), + right: Math.round(surfaceRect.right), + bottom: Math.round(surfaceRect.bottom), + }, + clip: { + left: Math.round(clipRect.left), + top: Math.round(clipRect.top), + right: Math.round(clipRect.right), + bottom: Math.round(clipRect.bottom), + }, + webview: { + left: Math.round(webviewRect.left), + top: Math.round(webviewRect.top), + }, + capturesOutsideInput: surface.contains(outsideTarget), + }; + }, browserId); +} + +async function readViewport(client, browserId) { + const evaluated = await callBrowserTool(client, "browser_evaluate", { + browserId, + function: "() => ({ width: window.innerWidth, height: window.innerHeight })", + }); + return JSON.parse(evaluated.resultJson); +} + +function recordViewportMismatch(failures, label, actual, expected) { + if (actual.width === expected.width && actual.height === expected.height) { + return; + } + failures.push( + `${label}: expected ${expected.width}x${expected.height}, received ${actual.width}x${actual.height}`, + ); +} + +async function runRegression({ page, client, serverId, targetUrl, callerAgentId }) { + const failures = []; const originalWorkspaceId = workspaceIds[0]; const originalWorkspaceRow = page.getByTestId( `sidebar-workspace-row-${serverId}:${originalWorkspaceId}`, @@ -300,6 +357,117 @@ async function runRegression({ page, client, serverId, targetUrl }) { ); const firstGuest = await readGuest(page, browserId); assert(firstGuest, "Original browser guest was not attached to its workspace pane"); + recordViewportMismatch( + failures, + "Responsive viewport follows the visible browser pane", + await readViewport(client, browserId), + { width: firstGuest.width, height: firstGuest.height }, + ); + + await callBrowserTool(client, "browser_wait", { + browserId, + text: "Bridge target", + timeoutMs: 5_000, + }); + const requestedViewport = { width: 640, height: 480 }; + await callBrowserTool(client, "browser_resize", { browserId, ...requestedViewport }); + recordViewportMismatch( + failures, + "browser_resize updates the visible shared viewport", + await readViewport(client, browserId), + requestedViewport, + ); + + const oversizedViewport = { width: 2560, height: 1440 }; + await callBrowserTool(client, "browser_resize", { browserId, ...oversizedViewport }); + recordViewportMismatch( + failures, + "oversized preset preserves the requested guest viewport", + await readViewport(client, browserId), + oversizedViewport, + ); + await page.waitForFunction( + ({ id, width, height }) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + return ( + webview instanceof HTMLElement && + Math.round(webview.getBoundingClientRect().width) === width && + Math.round(webview.getBoundingClientRect().height) === height + ); + }, + { id: browserId, ...oversizedViewport }, + { timeout: timeoutMs }, + ); + const presentation = await readPresentation(page, browserId); + assert(presentation, "Oversized browser presentation geometry was unavailable"); + if ( + presentation.surface.left < presentation.clip.left || + presentation.surface.top < presentation.clip.top || + presentation.surface.right > presentation.clip.right || + presentation.surface.bottom > presentation.clip.bottom + ) { + failures.push( + `oversized browser surface stays inside its pane: ${JSON.stringify(presentation)}`, + ); + } + if (presentation.capturesOutsideInput) { + failures.push("oversized browser surface captures input outside its pane"); + } + await callBrowserTool(client, "browser_resize", { browserId, ...oversizedViewport }); + const repeatedResizePresentation = await readPresentation(page, browserId); + assert(repeatedResizePresentation, "Repeated resize presentation geometry was unavailable"); + if ( + repeatedResizePresentation.webview.left !== presentation.webview.left || + repeatedResizePresentation.webview.top !== presentation.webview.top + ) { + failures.push( + `repeating an oversized resize preserves the guest offset: before ${JSON.stringify(presentation.webview)}, after ${JSON.stringify(repeatedResizePresentation.webview)}`, + ); + } + await callBrowserTool(client, "browser_resize", { browserId, ...requestedViewport }); + + await originalDeck.getByTestId(`workspace-tab-agent_${callerAgentId}`).click(); + await page.waitForTimeout(500); + try { + await callBrowserTool(client, "browser_screenshot", { browserId }); + } catch (error) { + failures.push(`inactive browser remains captureable: ${String(error)}`); + } + recordViewportMismatch( + failures, + "inactive browser preserves the shared viewport", + await readViewport(client, browserId), + requestedViewport, + ); + const focusContinuitySentinel = "preserve-browser-document-across-focus"; + await callBrowserTool(client, "browser_evaluate", { + browserId, + function: `() => { globalThis.__paseoFocusContinuity = ${JSON.stringify(focusContinuitySentinel)}; return globalThis.__paseoFocusContinuity; }`, + }); + + await originalDeck.getByTestId(`workspace-tab-browser_${browserId}`).click(); + await page.waitForFunction( + ({ id, webContentsId }) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + return ( + webview?.parentElement?.getAttribute("data-paseo-browser-surface") === id && + webview.parentElement.style.pointerEvents === "auto" && + webview.getWebContentsId() === webContentsId + ); + }, + { id: browserId, webContentsId: firstGuest.webContentsId }, + { timeout: timeoutMs }, + ); + const continuityResult = await callBrowserTool(client, "browser_evaluate", { + browserId, + function: "() => globalThis.__paseoFocusContinuity ?? null", + }); + const continuityValue = JSON.parse(continuityResult.resultJson); + if (continuityValue !== focusContinuitySentinel) { + failures.push( + `focusing the browser tab preserves the current document: expected ${JSON.stringify(focusContinuitySentinel)}, received ${JSON.stringify(continuityValue)}`, + ); + } for (const workspaceId of workspaceIds.slice(1)) { await page.getByTestId(`sidebar-workspace-row-${serverId}:${workspaceId}`).click(); @@ -312,21 +480,22 @@ async function runRegression({ page, client, serverId, targetUrl }) { ({ id, previousWebContentsId }) => { const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); return ( - webview?.parentElement?.id === "paseo-browser-resident-webviews" && + webview?.parentElement?.getAttribute("data-paseo-browser-surface") === id && + webview.parentElement.style.width === "1px" && typeof webview.getWebContentsId === "function" && - webview.getWebContentsId() !== previousWebContentsId + webview.getWebContentsId() === previousWebContentsId ); }, { id: browserId, previousWebContentsId: firstGuest.webContentsId }, { timeout: timeoutMs }, ); - const replacementGuest = await readGuest(page, browserId); - assert(replacementGuest, "Replacement browser guest was not parked after workspace eviction"); + const parkedGuest = await readGuest(page, browserId); + assert(parkedGuest, "Browser guest was not parked after workspace eviction"); const listed = await callBrowserTool(client, "browser_list_tabs"); assert( listed.tabs.some((tab) => tab.browserId === browserId), - "browser_list_tabs lost the original tab after guest replacement", + "browser_list_tabs lost the original tab after workspace eviction", ); const snapshot = await callBrowserTool(client, "browser_snapshot", { browserId }); @@ -344,10 +513,47 @@ async function runRegression({ page, client, serverId, targetUrl }) { timeoutMs: 5_000, }); + await originalWorkspaceRow.click(); + await originalDeck.getByTestId(`workspace-tab-browser_${browserId}`).click(); + await page.waitForFunction( + ({ id, webContentsId }) => { + const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`); + return ( + webview?.parentElement?.getAttribute("data-paseo-browser-surface") === id && + webview.parentElement.style.pointerEvents === "auto" && + webview.getWebContentsId() === webContentsId + ); + }, + { id: browserId, webContentsId: firstGuest.webContentsId }, + { timeout: timeoutMs }, + ); + recordViewportMismatch( + failures, + "browser viewport survives workspace eviction and reattachment", + await readViewport(client, browserId), + requestedViewport, + ); + + await originalDeck.getByRole("button", { name: "Annotate element" }).click(); + await page.waitForTimeout(250); + const selectorResult = await callBrowserTool(client, "browser_evaluate", { + browserId, + function: "() => Boolean(globalThis.__paseoSelector)", + }); + if (JSON.parse(selectorResult.resultJson) !== true) { + failures.push("reused loaded browser remains ready for element annotation after remount"); + } + + if (failures.length > 0) { + throw new Error(`Browser viewport regressions:\n- ${failures.join("\n- ")}`); + } + return { browserId, originalWebContentsId: firstGuest.webContentsId, - replacementWebContentsId: replacementGuest.webContentsId, + finalWebContentsId: parkedGuest.webContentsId, + viewport: "passed", + inactiveCapture: "passed", list: "passed", snapshot: "passed", click: "passed", @@ -446,10 +652,11 @@ async function main() { client, serverId: status.serverId, targetUrl: target.url, + callerAgentId, }); writeJson(path.join(artifactDir, "result.json"), report); console.log( - `Browser desktop browser E2E passed: WebContents ${report.originalWebContentsId} -> ${report.replacementWebContentsId}; list, snapshot, click passed.`, + `Browser desktop browser E2E passed: WebContents ${report.originalWebContentsId} remained ${report.finalWebContentsId}; viewport, inactive capture, focus continuity, list, snapshot, click passed.`, ); } catch (error) { console.error(`Browser desktop browser E2E failed. Artifacts: ${artifactDir}`); diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index 2cb168f4c..dba758196 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -20,11 +20,14 @@ class FakeDebugger { public blockCommands = false; public readonly blockedCommandNames = new Set(); public readonly failedCommandNames = new Set(); + public readonly failedCommandErrors = new Map(); public readonly promptDialogs: unknown[] = []; public failPromptDrain = false; + public beforeCommand: ((command: string) => void) | null = null; private messageListener: | ((event: unknown, method: string, params?: Record) => void) | null = null; + private detachListener: (() => void) | null = null; private readonly blockedCommands: Array<() => void> = []; public isAttached(): boolean { @@ -37,6 +40,11 @@ class FakeDebugger { public async sendCommand(command: string, params?: Record): Promise { this.commands.push({ command, params: params ?? {} }); + this.beforeCommand?.(command); + const commandError = this.failedCommandErrors.get(command); + if (commandError) { + throw commandError; + } if (this.failedCommandNames.has(command)) { throw new Error(`${command} failed`); } @@ -58,10 +66,15 @@ class FakeDebugger { } public on( - event: "message", - listener: (event: unknown, method: string, params?: Record) => void, + event: "message" | "detach", + listener: + | ((event: unknown, method: string, params?: Record) => void) + | (() => void), ): void { - expect(event).toBe("message"); + if (event === "detach") { + this.detachListener = listener; + return; + } this.messageListener = listener; } @@ -72,6 +85,11 @@ class FakeDebugger { this.messageListener({}, method, params); } + public emitDetach(): void { + this.attachedProtocolVersions.length = 0; + this.detachListener?.(); + } + public finishNextCommand(): void { const resolve = this.blockedCommands.shift(); if (!resolve) { @@ -92,6 +110,7 @@ type ConsoleMessageListener = ( 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; @@ -138,7 +157,9 @@ class FakeWebContents { return null; } - public async loadURL(): Promise {} + public async loadURL(url: string): Promise { + this.loadedUrls.push(url); + } public goBack(): void {} @@ -186,6 +207,7 @@ class FakeWebContents { public destroy(): void { this.destroyed = true; + this.debugger.emitDetach(); this.destroyedListener?.(); } } @@ -571,6 +593,54 @@ describe("browser automation IPC adapter", () => { ); warn.mockRestore(); }); + + test("does not run the command after the debugger target closes during setup", async () => { + const contents = new FakeWebContents(31); + contents.debugger.beforeCommand = (command) => { + if (command === "Page.enable") { + contents.destroy(); + } + }; + contents.debugger.failedCommandErrors.set( + "Page.enable", + new Error("target closed while handling command"), + ); + const tab = adaptWebContents(contents); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + await expect( + tab.captureDialogs?.(() => tab.loadURL("https://replacement.example.com")), + ).rejects.toThrow("target closed while handling command"); + expect(contents.loadedUrls).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + + warn.mockRestore(); + }); + + test("runs without dialog capture after a live debugger session detaches", async () => { + const contents = new FakeWebContents(32); + const tab = adaptWebContents(contents); + + await expect(tab.captureDialogs?.(async () => "captured")).resolves.toEqual({ + result: "captured", + dialogs: [], + }); + contents.debugger.emitDetach(); + contents.debugger.failedCommandNames.add("Page.enable"); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + + await expect( + tab.captureDialogs?.(() => tab.loadURL("https://replacement.example.com")), + ).resolves.toEqual({ result: undefined, dialogs: [] }); + expect(contents.loadedUrls).toEqual(["https://replacement.example.com"]); + expect(contents.debugger.attachedProtocolVersions).toEqual(["1.3"]); + expect(warn).toHaveBeenCalledWith( + "[browser-automation] Dialog capture unavailable; running command without it", + { contentsId: 32, error: expect.any(Error) }, + ); + + warn.mockRestore(); + }); }); class FakeHostWebContents { diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index 12f35b798..ea1941c31 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -74,6 +74,7 @@ interface WebContentsDebugger { event: "message", listener: (event: unknown, method: string, params?: Record) => void, ): void; + on?(event: "detach", listener: () => void): void; } interface ConsoleMessageEmitter { @@ -188,6 +189,7 @@ function getDialogMonitor( class DialogMonitor { private enabled = false; private listenerRegistered = false; + private detachGeneration = 0; private readonly activeCollectors: DialogCollector[] = []; public constructor( @@ -200,10 +202,14 @@ class DialogMonitor { task: () => Promise, ): Promise<{ result: T; dialogs: BrowserAutomationDialogEvent[] }> { const collector: DialogCollector = { dialogs: [] }; + const setupDetachGeneration = this.detachGeneration; try { await this.enable(); await this.installPromptShim(); } catch (error) { + if (this.contents.isDestroyed() || this.detachGeneration !== setupDetachGeneration) { + throw error; + } console.warn("[browser-automation] Dialog capture unavailable; running command without it", { contentsId: this.contentsId, error, @@ -244,6 +250,10 @@ class DialogMonitor { } void this.handleOpening(params ?? {}); }); + this.contents.debugger.on("detach", () => { + this.enabled = false; + this.detachGeneration += 1; + }); } await this.sendDebugCommand("Page.enable"); this.enabled = true;