From c86ae33aab0e43f80875557922ec3d496d4f360e Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 20:05:56 +0800 Subject: [PATCH 01/34] chore(workspace-browser-tab): add IMPLEMENT-SPEC-PROMPT to public source - Added IMPLEMENT-SPEC-PROMPT.md to release/public-source.json --- release/public-source.json | 1 + 1 file changed, 1 insertion(+) diff --git a/release/public-source.json b/release/public-source.json index 603c19081..22ec49a88 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -91,6 +91,7 @@ ".plan/browser-use/exploration/03-repo-plumbing.md", ".plan/workspace-browser-tab/board/.ticket-numbers.txt", ".plan/workspace-browser-tab/board/HANDOFF.md", + ".plan/workspace-browser-tab/board/IMPLEMENT-SPEC-PROMPT.md", ".plan/workspace-browser-tab/board/notes-01-runtime-provider.md", ".plan/workspace-browser-tab/board/notes-02-client-tab-model.md", ".plan/workspace-browser-tab/board/spec.md", From d2b25743eede564969a388dabf5acfd7b6276f87 Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 20:21:56 +0800 Subject: [PATCH 02/34] docs(webui): measure the tab model's two-page case against a real browser Issue #200. The programme's later tickets are written against four assumptions that no type checker catches. A spike settles all four against a real isolated Chromium, and the capability matrix is written from what it observed. Measured: two pages of one Chromium can be driven, streamed and input into at the same time; a navigation on the second page advances that page's generation (1 -> 2) and leaves the first alone; a reference recorded against a page's previous generation is refused with browser_target_stale while the untouched page still accepts its own; and the frame's page is resolved from the envelope's event.sessionId, not from event.params.sessionId, which is the number 1 for frames from both pages. Two findings the repository has to act on, both already in scope: the provider's screencast handler stamps every frame with its own target id, and the provider's generation fence covers one page. The record is .plan/workspace-browser-tab/complete/two-page-spike.json. --- .../workspace-browser-tab/complete/README.md | 5 +- .../complete/two-page-spike.json | 352 ++++++++++++++++++ .../docs/browser-use/01-capability-matrix.md | 161 +++++++- release/public-source.json | 1 + 4 files changed, 510 insertions(+), 9 deletions(-) create mode 100644 .plan/workspace-browser-tab/complete/two-page-spike.json diff --git a/.plan/workspace-browser-tab/complete/README.md b/.plan/workspace-browser-tab/complete/README.md index 76c9da431..80150953c 100644 --- a/.plan/workspace-browser-tab/complete/README.md +++ b/.plan/workspace-browser-tab/complete/README.md @@ -3,8 +3,8 @@ This directory is where the workspace browser tab model's **completion evidence** will live: the records the suites write while they run, and the completion note. -The plan is [`../workspace-browser-tab-plan.md`](../workspace-browser-tab-plan.md). There is no -execution board for this programme yet, and none of the work has started. +The plan is [`../workspace-browser-tab-plan.md`](../workspace-browser-tab-plan.md). The execution board is +[`../board/`](../board/), and the work has started. ## What belongs here @@ -12,6 +12,7 @@ execution board for this programme yet, and none of the work has started. | --- | --- | --- | | `.acceptance.json` | the Playwright suites, through `packages/webui/test/browser/acceptance-record.ts` | one record per case: what was set up, what was observed, and what the person saw | | `*.evidence.json` | a probe run by hand | what a real isolated Chromium did, on the machine named in the record | +| `two-page-spike.json` | the issue #200 spike, run by hand against a real isolated Chromium | what the browser can do with two pages at once, and what the repository's provider does today — the matrix in `packages/webui/docs/browser-use/01-capability-matrix.md` is written from it | The published acceptance record for this work is `packages/webui/docs/browser-use/09-acceptance-record.md`, and the decision record is diff --git a/.plan/workspace-browser-tab/complete/two-page-spike.json b/.plan/workspace-browser-tab/complete/two-page-spike.json new file mode 100644 index 000000000..34f83ef91 --- /dev/null +++ b/.plan/workspace-browser-tab/complete/two-page-spike.json @@ -0,0 +1,352 @@ +{ + "probe": "workspace-browser-tab two-page spike", + "issue": 200, + "host": { + "platform": "linux", + "arch": "x64", + "node": "v26.8.1", + "chrome": { + "path": "/usr/bin/google-chrome", + "product": "Chrome/152.0.7977.82", + "revision": "@d04cdb24d67b081f6cf80200ffc5233f44b61109" + } + }, + "method": { + "what": "Drive, stream and input into two pages of one real isolated Chromium, and record what happens.", + "phases": { + "provider": "Launch the repository's own WebuiIsolatedChromiumProvider, open a tab through it, and try to drive, input into and stream that new page. This is what the provider does today.", + "substrate": "Launch a real Chrome in the provider's own launch shape, attach to two pages over the repository's own WebuiBrowserCdpConnection, and exercise them concurrently: Page.enable on both, Page.startScreencast on both, Input.insertText into both, Page.navigate on the second, and the per-page generation accounting." + }, + "pages": "Two real documents served by a local HTTP server on 127.0.0.1 — no network, no real user content.", + "whatItIsNot": "This is not a test. It asserts nothing; it records what it observed. The record is the evidence.", + "reproduce": "Run the spike from the repository's webui package with tsx, against /usr/bin/google-chrome. The spike script itself is work material and lives outside the published source; the record it writes is committed here." + }, + "startedAt": "2026-10-21T12:19:14.662Z", + "phases": { + "provider": { + "description": "The real WebuiIsolatedChromiumProvider class, today.", + "targetsBeforeOpenTab": [ + { + "id": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "65B598C6A854F92A41E6D84989840EBE", + "generation": 1 + }, + "title": "about:blank", + "url": "about:blank", + "status": "open", + "selected": false + }, + { + "id": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "DC6CA1E893EE5DBDAFFA13B8E2A48388", + "generation": 1 + }, + "title": "about:blank", + "url": "about:blank", + "status": "open", + "selected": true + } + ], + "openTabResult": { + "targetId": "8A66389216F0D4C93DF1100708BFA0B6" + }, + "targetsAfterOpenTab": [ + { + "id": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "65B598C6A854F92A41E6D84989840EBE", + "generation": 1 + }, + "title": "about:blank", + "url": "about:blank", + "status": "open", + "selected": false + }, + { + "id": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "DC6CA1E893EE5DBDAFFA13B8E2A48388", + "generation": 1 + }, + "title": "about:blank", + "url": "about:blank", + "status": "open", + "selected": true + }, + { + "id": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "8A66389216F0D4C93DF1100708BFA0B6", + "generation": 1 + }, + "title": "about:blank", + "url": "about:blank", + "status": "open", + "selected": false + } + ], + "pageCount": { + "before": 2, + "after": 3 + }, + "newPageIdentity": { + "surface": "isolated-chromium", + "browserId": "isolated-chromium-fc772637", + "targetId": "8A66389216F0D4C93DF1100708BFA0B6", + "generation": 1 + }, + "newPageIsListedByTheProvider": true, + "driveNewPageNavigate": { + "attempted": true, + "outcome": { + "ok": false, + "error": { + "name": "WebuiBrowserOperationError", + "code": "browser_target_unknown", + "message": "The isolated Chromium has no such target." + } + } + }, + "driveNewPageInput": { + "attempted": true, + "outcome": { + "ok": false, + "error": { + "name": "WebuiBrowserOperationError", + "code": "browser_target_unknown", + "message": "The isolated Chromium has no such target." + } + } + }, + "streamNewPage": { + "attempted": true, + "outcome": { + "ok": false, + "error": { + "name": "WebuiBrowserOperationError", + "code": "browser_target_unknown", + "message": "The isolated Chromium has no such target." + } + } + }, + "generationOfNewPage": 1, + "newPageSelected": false, + "driveLaunchPage": { + "attempted": true, + "outcome": { + "ok": true, + "value": {} + } + }, + "launchPageGeneration": 1 + }, + "substrate": { + "description": "The provider's own CDP connection against one real Chromium, with two attached pages.", + "pages": [ + { + "label": "A", + "targetId": "74D93BB4C685C5B31C4DB4A57D464C6D", + "sessionId": "A620B87DAD5FB03858BF93CA90AD295D" + }, + { + "label": "B", + "targetId": "9F0A5C87C3F8BF6EB8971A9BF5CC28DB", + "sessionId": "89944F28584C4F0ED9E21F4FC15893B1" + } + ], + "sessionsAreDistinct": true, + "targetIdsAreDistinct": true, + "pageEnableSentTo": [ + "A620B87DAD5FB03858BF93CA90AD295D", + "89944F28584C4F0ED9E21F4FC15893B1" + ], + "screencastStartedFor": [ + "A620B87DAD5FB03858BF93CA90AD295D", + "89944F28584C4F0ED9E21F4FC15893B1" + ], + "input": { + "pageAValue": "alpha", + "pageBValue": "bravo", + "independent": true + }, + "framesAfterBothPagesWereTypedInto": { + "count": 3, + "byEnvelopeSession": [ + { + "sessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "page": "B", + "count": 3 + } + ], + "sample": [ + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 10448, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + }, + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 10840, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + }, + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 11312, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + } + ] + }, + "generation": { + "before": { + "A": 1, + "B": 1 + }, + "after": { + "A": 1, + "B": 2 + }, + "navigationsObserved": [ + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "mainFrame": true, + "url": "http://127.0.0.1:42671/b2" + } + ] + }, + "fence": { + "staleReferenceOnNavigatedPage": { + "accepted": false, + "error": { + "code": "browser_target_stale", + "message": "The browser page was replaced; the input was not delivered." + } + }, + "currentReferenceOnUntouchedPage": { + "accepted": true + }, + "staleRefused": true, + "untouchedPageStillAccepts": true + }, + "afterNavigation": { + "driveA": { + "ok": true, + "value": { + "result": { + "type": "string", + "value": "Page A" + } + } + }, + "driveB": { + "ok": true, + "value": { + "result": { + "type": "string", + "value": "Page B2" + } + } + } + }, + "perTargetScreenshot": { + "aBytes": 10988, + "bBytes": 11312 + }, + "framesAtTheEnd": { + "count": 4, + "byEnvelopeSession": [ + { + "sessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "page": "B", + "count": 3 + }, + { + "sessionId": "A620B87DAD5FB03858BF93CA90AD295D", + "page": "A", + "count": 1 + } + ], + "sample": [ + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 10448, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + }, + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 10840, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + }, + { + "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 11312, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + }, + { + "envelopeSessionId": "A620B87DAD5FB03858BF93CA90AD295D", + "envelopeSessionIdType": "string", + "paramsSessionId": 1, + "paramsSessionIdType": "number", + "dataLength": 11364, + "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" + } + ] + }, + "frameRouting": { + "envelopeSessionIdResolvesToAPage": true, + "paramsSessionIdResolvesToAPage": false, + "paramsSessionIdValues": [ + "1" + ], + "envelopeSessionIdValues": [ + "89944F28584C4F0ED9E21F4FC15893B1", + "A620B87DAD5FB03858BF93CA90AD295D" + ], + "conclusion": "the envelope's event.sessionId is the target session and identifies the page; event.params.sessionId is a number that is not the target session and identifies nothing" + }, + "currentProviderFrameRule": { + "rule": "every Page.screencastFrame is stamped with the provider's own target id, without asking which page the frame came from", + "source": "packages/webui/src/runtime/browser/isolated-chromium/provider.ts — #onEvent, the Page.screencastFrame branch", + "modelledWith": "page A stands for the one page the provider owns", + "observedFrames": 4, + "framesFromAnotherPage": 3, + "misStampedByTheCurrentRule": 3, + "consequence": "with two pages streaming, the other page's image is delivered as this page's image" + }, + "frameIdentityIsSufficient": { + "distinctEnvelopeSessions": 2, + "pages": 2, + "pagesThatStreamed": [ + "A", + "B" + ], + "bothPagesStreamedAtTheSameTime": true + } + } + }, + "finishedAt": "2026-10-21T12:19:17.348Z" +} diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index 0e4e69245..91676662e 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -5,7 +5,7 @@ is: | Label | Means | Backed by | | --- | --- | --- | -| `measured` | A probe observed this on the machine named in the section heading. | `.plan/browser-use/complete/cdp-evidence.json` or `.plan/browser-use/complete/credential-store-evidence.json` | +| `measured` | A probe observed this on the machine named in the section heading. | `.plan/browser-use/complete/cdp-evidence.json`, `.plan/browser-use/complete/credential-store-evidence.json` or `.plan/workspace-browser-tab/complete/two-page-spike.json` | | `implemented` | The source does this today. The row names the file. | The cited file in this repository | | `not yet measured` | An open question. Not a denial and not a promise. | Nothing — see the section that collects them | @@ -265,11 +265,11 @@ a pairing credential. See [`04-pairing-credentials.md`](04-pairing-credentials.m Read this section before citing the matrix. Each line is a gap in the evidence, not a known limitation of the product. -- **The isolated surface drives exactly one page, and no probe exercised two.** The provider refuses - every target that is not the page it launched, so `open_tab` creates a page that nothing can address. - Nothing here measures a second page on one surface, and the tab model that needs one - ([ADR 0022](../adr/0022-webui-workspace-panel-is-a-browser-tab-strip.md)) is `required`, not - `implemented`. +- **The isolated surface drives exactly one page.** The provider refuses every target that is not the + page it launched, so `open_tab` creates a page that nothing can address. **What the browser itself can + do with two pages is now `measured`** — see [the workspace browser tab model](#the-workspace-browser-tab-model) + and `.plan/workspace-browser-tab/complete/two-page-spike.json`. The gap that remains is the + *repository's* provider, not the browser's capability. - **The isolated surface's CDP matrix was measured on Chromium only.** The probe ran against the bundled Chromium, not against Edge, so the isolated surface's cells describe that browser. Edge *was* exercised for the **extension** path — see the real-browser acceptance below — but the @@ -323,4 +323,151 @@ an unearned capability. | `chrome` | `true` | `true` | `true` | `true` | `true` | `liveImages` and `input` are `true` for the two bridged surfaces as declarations only. The image stream -and input paths for those surfaces run over the bridge, and nothing measured here confirms either. \ No newline at end of file +and input paths for those surfaces run over the bridge, and nothing measured here confirms either. + +## The workspace browser tab model + +[ADR 0022](../adr/0022-webui-workspace-panel-is-a-browser-tab-strip.md) turns the workspace panel into a +browser tab strip: a tab is a **web tab**, and the panel shows a picture of a live page. That gives the +tab model its own matrix. It is measured against the **isolated Chromium** — the surface the +programme's first slice uses — because that is the surface the repository drives itself. + +**Measured on the validation host.** `/usr/bin/google-chrome`, `Chrome/152.0.7977.82`, revision +`@d04cdb24d67b081f6cf80200cfc5233f44b61109` (`two-page-spike.json` → `host.chrome`). The record is +`.plan/workspace-browser-tab/complete/two-page-spike.json`. + +**Probe method.** The spike launches a real Chrome in the provider's own launch shape and speaks CDP +through the repository's own `WebuiBrowserCdpConnection` +(`packages/webui/src/runtime/browser/isolated-chromium/cdp-connection.ts`). It attaches to **two** +pages and exercises them at the same time. It also launches the repository's own +`WebuiIsolatedChromiumProvider` to record what the provider does today. The two pages are real +documents served from `127.0.0.1`; no network and no real user content were involved. The spike +asserts nothing — the record is the evidence. + +### What a web tab can do — measured + +Every cell in this table is `measured`, and the evidence field is named. + +| Capability | Mark | Evidence | +| --- | --- | --- | +| Two pages of one isolated Chromium, live at the same time | **measured** | `substrate.pages` — two pages, two distinct target ids and two distinct target sessions | +| Each page is driven independently | **measured** | `substrate.afterNavigation` — page A answers `Page A`, page B answers `Page B2` | +| Each page is streamed independently | **measured** | `substrate.framesAtTheEnd.byEnvelopeSession` — A: 1 frame, B: 3 frames, from two different target sessions; `substrate.frameIdentityIsSufficient.bothPagesStreamedAtTheSameTime` is `true` | +| Each page is input into independently | **measured** | `substrate.input` — page A's field reads `alpha`, page B's reads `bravo`; `independent` is `true` | +| A navigation on the second page advances **that** page's generation and leaves the first alone | **measured** | `substrate.generation` — A stays `1`, B goes `1` → `2` | +| A reference recorded against a page's previous generation is refused | **measured** | `substrate.fence.staleReferenceOnNavigatedPage` — refused with `browser_target_stale` | +| A reference against the untouched page's current generation is still accepted | **measured** | `substrate.fence.currentReferenceOnUntouchedPage` — `accepted: true` | +| The frame's page is resolved from the **envelope's** session id | **measured** | `substrate.frameRouting` — the envelope's `event.sessionId` resolves to a page for every frame; `event.params.sessionId` resolves to none | + +**The frame's identity, in one line.** The envelope's `event.sessionId` is the target session — a +string, put on the event by the CDP connection (`cdp-connection.ts:234`, `:237`) — and it identifies the +page. `event.params.sessionId` is the **number** `1` for frames from *both* pages +(`substrate.frameRouting.paramsSessionIdValues`), and identifies nothing. A screencast frame is +routed by the envelope. + +### What this repository does today + +`not yet measured` in this table means the source does not do this yet, and the row names the ticket +that closes it. It is a gap, not a denial. + +| Capability | Mark | Where it stands | +| --- | --- | --- | +| The provider drives one page per web tab | **not yet measured** | `provider.ts:405`, `:266` and `:317` refuse every target that is not the launch page. #201 | +| The generation advances per page | **not yet measured** | `provider.ts:251` reports a constant `1` for every other page; `provider.ts:229` sends `Page.enable` to one session only; `provider.ts:532` raises one page's generation. #201 | +| A frame is routed to the page that asked for it | **not yet measured** | `provider.ts:543` stamps **every** screencast frame with the provider's own target id. #201 | +| The image stream is keyed by target | **not yet measured** | `browser-service.ts:180` keys the frame subscribers by surface and then by **run**. #204 | +| The runtime publishes the session's web tabs as a snapshot | **not yet measured** | no such operation exists. #203 | +| The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. #206 | +| The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | +| The frame operation's request names a target | **not yet measured** | `operation-names.ts:458` takes `{ surface, runId? }`. #204 | +| The provider's attachment retry | **implemented** | `provider.ts:588` — the mechanism is cited in full above | +| The files tab is a singleton | **implemented** | `workspace-panel-state.ts:289` drops every `files` tab before adding one | +| The two review tabs share one id | **implemented** | `workspace-panel-state.ts:279` and `:283` derive the same `tabId("review", sessionId, workspaceDir)` | +| A tab's label is derived from its kind | **implemented** | `workspace-panel-state.ts:154` | +| The provider refuses a second page rather than sending a command to the wrong one | **implemented** | `provider.ts:410` — the comment says why, and the refusal is measured today: `phases.provider.driveNewPageNavigate`, `.driveNewPageInput` and `.streamNewPage` are all `browser_target_unknown` | + +**One number worth stating plainly.** The provider's `open_tab` **does** create a page: the page count +goes from 2 to 3 (`phases.provider.pageCount`) and the new page is listed by `listTargets` with its own +target id (`phases.provider.newPageIdentity`). What does not exist is any way to *address* it. That is +the whole gap the programme closes, and it is a gap in this repository, not in the browser. + +### The fence, on every page + +The fence is what makes a reference to a page that has been replaced detectable. The spike shows it +working on a page that is **not** the one the browser was launched with: + +| Page | What happened | Result | +| --- | --- | --- | +| B (navigated) | its generation advanced `1` → `2`; a reference recorded against generation `1` was offered | refused with `browser_target_stale` | +| A (untouched) | its generation stayed `1`; a reference recorded against generation `1` was offered | accepted | + +A fence that only works on the launch page is a fence that does not work. The repository's fence today +is the launch page's fence only: `provider.ts:328` compares the command's generation against the one +generation the provider holds, and there is one of them. + +### The registration cost of the new operation + +The programme adds one operation that publishes the session's web tabs. Registering it costs the +**eight** sites below, and every one of them is a place where a change that is forgotten shows up as a +missing capability rather than as an error. + +| Site | Where | What the new operation needs | +| --- | --- | --- | +| The capability manifest's `browser.ready` list | `packages/webui/src/shared/capability-manifest.ts:143` | the operation's name joins the list. A `ready` member that is not wired makes the whole browser family **not-ready** (`capability-manifest.ts:202`), not absent — a quiet degradation | +| The operation-name constants | `packages/webui/src/shared/operation-names.ts:51` | one new `as const` constant beside the eight browser names | +| The operation spec | `packages/webui/src/shared/operation-names.ts:458` | one row naming the request and the response type, beside `watchBrowserFrames` | +| The contract type | `packages/webui/src/shared/contracts/browser.ts` | the request and response types live here; `watchBrowserFrames` has none of its own, so the new operation introduces one | +| The operation registry | `packages/webui/src/server/operation/operations.ts:162` | one `registerOperation(registry, entry(""))` line inside the `if (browser)` block. **The order is a wire contract** (`operations.ts:69`) | +| The server handler | `packages/webui/src/server/operation/handlers/browser.ts:127` | a dedicated handler. A stream handler returns `{ stream: { ok: true, source } }` and lets the service's generator raise the errors — the handler does not catch them | +| The port member and its implementation | `packages/webui/src/runtime/port.ts:637` and `packages/webui/src/runtime/harness/browser.ts:206` | the member on `WebuiBrowserPort` and its body in `createBrowserPort` | +| The transport's request table | `packages/webui/src/client/platform/transport.ts:232` | the client's request builder; a stream also needs the wire session in `packages/webui/src/client/platform/wire.ts:144` and `:373` | + +**There is no `bind()` row to add.** The browser operations are dedicated handlers, not bindings: the +`WEBUI_OPERATION_BINDINGS` table (`packages/webui/src/server/operation/bind-handlers.ts:149`) has no +browser entry, and there is no `RuntimeGroups.browser` group to extend. + +### The plumbing cost of a new source file and a new test file + +| What | Where | What it costs | +| --- | --- | --- | +| The frozen path metadata fixture | `packages/webui/test/gates/fixtures/path-metadata-f00.json` | a new source path needs an entry, classified by the **frozen** baseline classifier. The fixture's own README says not to regenerate expectations from the current classifier: strip only `.public` before the TypeScript suffix, then run the frozen classifier against the path and review any mismatch separately | +| The declared Vitest suite groups | `packages/webui/test/vitest-suites.json` | a new Vitest file is appended to `suites.webui`. This JSON is the single source — no `package.json` script and no Vitest config change | +| The source inventory | `release/public-source.json` | regenerate with `node scripts/source-inventory.mjs --write`; `pnpm check:source` reads it. The inventory scans the working tree, including untracked files, so a scratch file left in the repository becomes a publication problem | +| The Playwright spec directory | `packages/webui/test/browser/` | a real-browser spec lands here. A test that drives the **isolated Chromium runtime** is not a Playwright spec: it belongs in `packages/webui/test/integration/`, beside `browser-use-real-input.test.ts`, and is a Vitest suite | + +### The tab-model inventory, checked against the tree + +The inventory the research notes name, checked against the code as it stands. Every row is real. + +| Site | Where | What it is | +| --- | --- | --- | +| The files-tab singleton | `workspace-panel-state.ts:289` | `open-primary-view` drops **every** `files` tab before adding one — the singleton is enforced by deletion, not by a lookup | +| The two review tabs that share one id | `workspace-panel-state.ts:279` and `:283` | the workspace's change set and a turn's review derive the **same** id, `tabId("review", sessionId, workspaceDir)`. One of them replaces the other | +| The label derived from the kind | `workspace-panel-state.ts:154` | `webuiWorkspaceTabLabel` switches on `tab.kind` | +| The `kind === "review"` narrowing | `workspace-panel-state.ts:156`, `:299`, `:302`, `:303`; `WorkspacePanels.tsx:332`, `:337`, `:338`, `:340`, `:341` | the two review sources are told apart by narrowing on the kind and then on the source | +| The `activeTab?.kind` guards | `WorkspacePanels.tsx` | **28** of them | +| The caches keyed by a tab id whose effects depend on `[activeTab?.id]` | `WorkspacePanels.tsx:291`, `:306`, `:309`, `:325`, `:327`, `:330`, `:337`, `:347`, `:353`, `:417` | the file-results cache, the review summary, the review search and the review diffs are all keyed by the tab id, and their effects re-run when that id changes | + +**Why that breaks under a mutable content.** A tab's id is derived from its kind and its parts +(`workspace-panel-state.ts:251`), and a web tab's page changes without the kind changing. The +caches above are keyed by the id and their effects are keyed by the id, so a content that changes +under a stable id leaves a stale picture on screen: the cache says the tab is showing generation 1 +while the page behind it is generation 2. The id has to become stable and the content has to become +the thing that changes — which is what #207 does. + +### Not yet measured — the tab model's open questions + +- **The two-page case was measured on the substrate, not through the provider.** The provider cannot + hold two pages today, so the spike drove the CDP connection directly to find out what the browser + can do. The provider's own per-target map is #201, and the same spike re-run against it is what + closes this question. +- **The frame mis-stamping count is a modelled consequence, not a provider observation.** + `substrate.currentProviderFrameRule` applies the provider's current rule to the frames the spike + observed, because the provider cannot stream a second page at all. It is arithmetic over a + measurement, and it is labelled as such in the record. +- **Nothing end-to-end has been measured.** The client's tab model, the strip, the new tab page, the + toolbar and the two selections are not exercised by any suite yet. #208, #209, #210, #211 and #212 + are where that happens. +- **The bridged surfaces have no tab model at all.** Everything measured here is the isolated + Chromium. The extension bridge exposes one visible tab per browser, so a tab strip over it is a + different question and was not asked. \ No newline at end of file diff --git a/release/public-source.json b/release/public-source.json index 22ec49a88..f6bf2bca3 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -109,6 +109,7 @@ ".plan/workspace-browser-tab/board/t12.md", ".plan/workspace-browser-tab/board/t13.md", ".plan/workspace-browser-tab/complete/README.md", + ".plan/workspace-browser-tab/complete/two-page-spike.json", ".plan/workspace-browser-tab/workspace-browser-tab-plan.md", "AGENTS.md", "CONTEXT.md", From f0519e7a8821d30d7f9390c3c0be726e6e96539b Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 20:31:39 +0800 Subject: [PATCH 03/34] feat(webui): the isolated Chromium holds one page per web tab Issue #201. The provider drove exactly one page and refused every other target, so a tab that was not the launch page could not be driven, streamed or input into at all. The single page becomes a per-target map: target id -> session, generation, frame sequence and frame handler. The session is resolved by target in sendCommand, sendInput, startImageStream and stopImageStream; the generation check is per target rather than per browser; and a screencast frame is matched to its page by the envelope's event.sessionId, which is the flattened target session the CDP connection puts on the event. event.params.sessionId is the numeric screencast acknowledgement id and names no page -- two pages' frames carry the same one. The generation moves with the map. Page.enable goes to every page, so a page that is not the launch page receives the navigation events that raise its generation, and the raise is keyed by the event's own session, so one page navigating leaves every other page's generation where it was. The provider's own comment about the single page is replaced by the rule that replaces it, and the rule names why the generation is per page. open_tab now attaches to the page it creates and registers it, so the provider really does hold more than one page; a page the provider never attached to is still refused rather than driven. One browser fact the implementation had to meet: Chrome renders the foregrounded tab and throttles the others, so a background page's screencast produces no new frames. The provider therefore brings the page it is streaming to the front, the same thing a browser does when a person switches to a tab. The new suite drives two real pages of a real isolated Chromium and proves that each is driven, streamed and input into independently, that a navigation advances one page's generation and leaves the other's alone, that a stale reference is refused on the navigated page while the untouched page still accepts its own, and that a frame reaches the page that asked for it and no other. The capability matrix is updated: the provider's rows are implemented, and the foreground rule is recorded as measured. --- .../complete/two-page-spike.json | 100 +++---- .../docs/browser-use/01-capability-matrix.md | 54 ++-- .../browser/isolated-chromium/provider.ts | 238 +++++++++++---- .../isolated-chromium-two-pages.test.ts | 274 ++++++++++++++++++ release/public-source.json | 1 + test/vitest-suites.json | 1 + 6 files changed, 534 insertions(+), 134 deletions(-) create mode 100644 packages/webui/test/integration/isolated-chromium-two-pages.test.ts diff --git a/.plan/workspace-browser-tab/complete/two-page-spike.json b/.plan/workspace-browser-tab/complete/two-page-spike.json index 34f83ef91..a9493fbef 100644 --- a/.plan/workspace-browser-tab/complete/two-page-spike.json +++ b/.plan/workspace-browser-tab/complete/two-page-spike.json @@ -21,7 +21,7 @@ "whatItIsNot": "This is not a test. It asserts nothing; it records what it observed. The record is the evidence.", "reproduce": "Run the spike from the repository's webui package with tsx, against /usr/bin/google-chrome. The spike script itself is work material and lives outside the published source; the record it writes is committed here." }, - "startedAt": "2026-10-21T12:19:14.662Z", + "startedAt": "2026-10-21T12:30:01.796Z", "phases": { "provider": { "description": "The real WebuiIsolatedChromiumProvider class, today.", @@ -29,8 +29,8 @@ { "id": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "65B598C6A854F92A41E6D84989840EBE", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "14E37D8770E0DB344CB98AAD88A06C4C", "generation": 1 }, "title": "about:blank", @@ -41,8 +41,8 @@ { "id": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "DC6CA1E893EE5DBDAFFA13B8E2A48388", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "AFB89EBB03E3F0272D7C87098E21FC86", "generation": 1 }, "title": "about:blank", @@ -52,14 +52,14 @@ } ], "openTabResult": { - "targetId": "8A66389216F0D4C93DF1100708BFA0B6" + "targetId": "AFEDCFF719D084495C93E65A16E9B932" }, "targetsAfterOpenTab": [ { "id": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "65B598C6A854F92A41E6D84989840EBE", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "14E37D8770E0DB344CB98AAD88A06C4C", "generation": 1 }, "title": "about:blank", @@ -70,8 +70,8 @@ { "id": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "DC6CA1E893EE5DBDAFFA13B8E2A48388", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "AFB89EBB03E3F0272D7C87098E21FC86", "generation": 1 }, "title": "about:blank", @@ -82,8 +82,8 @@ { "id": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "8A66389216F0D4C93DF1100708BFA0B6", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "AFEDCFF719D084495C93E65A16E9B932", "generation": 1 }, "title": "about:blank", @@ -98,42 +98,34 @@ }, "newPageIdentity": { "surface": "isolated-chromium", - "browserId": "isolated-chromium-fc772637", - "targetId": "8A66389216F0D4C93DF1100708BFA0B6", + "browserId": "isolated-chromium-wadkjvl8", + "targetId": "AFEDCFF719D084495C93E65A16E9B932", "generation": 1 }, "newPageIsListedByTheProvider": true, "driveNewPageNavigate": { "attempted": true, "outcome": { - "ok": false, - "error": { - "name": "WebuiBrowserOperationError", - "code": "browser_target_unknown", - "message": "The isolated Chromium has no such target." + "ok": true, + "value": { + "frameId": "AFEDCFF719D084495C93E65A16E9B932", + "loaderId": "197A18319FAFF4ABD19D84BC8F4C3974", + "isDownload": false } } }, "driveNewPageInput": { "attempted": true, "outcome": { - "ok": false, - "error": { - "name": "WebuiBrowserOperationError", - "code": "browser_target_unknown", - "message": "The isolated Chromium has no such target." - } + "ok": true, + "value": null } }, "streamNewPage": { "attempted": true, "outcome": { - "ok": false, - "error": { - "name": "WebuiBrowserOperationError", - "code": "browser_target_unknown", - "message": "The isolated Chromium has no such target." - } + "ok": true, + "value": null } }, "generationOfNewPage": 1, @@ -152,24 +144,24 @@ "pages": [ { "label": "A", - "targetId": "74D93BB4C685C5B31C4DB4A57D464C6D", - "sessionId": "A620B87DAD5FB03858BF93CA90AD295D" + "targetId": "523F643B0CC0383A2686FE4D40E887D2", + "sessionId": "6E4260243A3E33B0B99DE1AB8BA18A2E" }, { "label": "B", - "targetId": "9F0A5C87C3F8BF6EB8971A9BF5CC28DB", - "sessionId": "89944F28584C4F0ED9E21F4FC15893B1" + "targetId": "1E783F3D1623A1BFAA511D24FAAEED2D", + "sessionId": "60354765619FF6A7B540EC966A496330" } ], "sessionsAreDistinct": true, "targetIdsAreDistinct": true, "pageEnableSentTo": [ - "A620B87DAD5FB03858BF93CA90AD295D", - "89944F28584C4F0ED9E21F4FC15893B1" + "6E4260243A3E33B0B99DE1AB8BA18A2E", + "60354765619FF6A7B540EC966A496330" ], "screencastStartedFor": [ - "A620B87DAD5FB03858BF93CA90AD295D", - "89944F28584C4F0ED9E21F4FC15893B1" + "6E4260243A3E33B0B99DE1AB8BA18A2E", + "60354765619FF6A7B540EC966A496330" ], "input": { "pageAValue": "alpha", @@ -180,14 +172,14 @@ "count": 3, "byEnvelopeSession": [ { - "sessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "sessionId": "60354765619FF6A7B540EC966A496330", "page": "B", "count": 3 } ], "sample": [ { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -195,7 +187,7 @@ "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" }, { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -203,7 +195,7 @@ "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" }, { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -223,9 +215,9 @@ }, "navigationsObserved": [ { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "mainFrame": true, - "url": "http://127.0.0.1:42671/b2" + "url": "http://127.0.0.1:44699/b2" } ] }, @@ -271,19 +263,19 @@ "count": 4, "byEnvelopeSession": [ { - "sessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "sessionId": "60354765619FF6A7B540EC966A496330", "page": "B", "count": 3 }, { - "sessionId": "A620B87DAD5FB03858BF93CA90AD295D", + "sessionId": "6E4260243A3E33B0B99DE1AB8BA18A2E", "page": "A", "count": 1 } ], "sample": [ { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -291,7 +283,7 @@ "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" }, { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -299,7 +291,7 @@ "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" }, { - "envelopeSessionId": "89944F28584C4F0ED9E21F4FC15893B1", + "envelopeSessionId": "60354765619FF6A7B540EC966A496330", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -307,7 +299,7 @@ "dataPrefix": "/9j/4AAQSkZJRgABAQAAAQAB" }, { - "envelopeSessionId": "A620B87DAD5FB03858BF93CA90AD295D", + "envelopeSessionId": "6E4260243A3E33B0B99DE1AB8BA18A2E", "envelopeSessionIdType": "string", "paramsSessionId": 1, "paramsSessionIdType": "number", @@ -323,8 +315,8 @@ "1" ], "envelopeSessionIdValues": [ - "89944F28584C4F0ED9E21F4FC15893B1", - "A620B87DAD5FB03858BF93CA90AD295D" + "60354765619FF6A7B540EC966A496330", + "6E4260243A3E33B0B99DE1AB8BA18A2E" ], "conclusion": "the envelope's event.sessionId is the target session and identifies the page; event.params.sessionId is a number that is not the target session and identifies nothing" }, @@ -348,5 +340,5 @@ } } }, - "finishedAt": "2026-10-21T12:19:17.348Z" + "finishedAt": "2026-10-21T12:30:04.453Z" } diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index 91676662e..a1dceff75 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -265,11 +265,11 @@ a pairing credential. See [`04-pairing-credentials.md`](04-pairing-credentials.m Read this section before citing the matrix. Each line is a gap in the evidence, not a known limitation of the product. -- **The isolated surface drives exactly one page.** The provider refuses every target that is not the - page it launched, so `open_tab` creates a page that nothing can address. **What the browser itself can - do with two pages is now `measured`** — see [the workspace browser tab model](#the-workspace-browser-tab-model) - and `.plan/workspace-browser-tab/complete/two-page-spike.json`. The gap that remains is the - *repository's* provider, not the browser's capability. +- **The isolated surface drives one page per web tab now.** The provider resolves a target to the + page that carries it, raises the generation per page, and routes a screencast frame by the + envelope's session id. **What the browser itself does with two pages was measured** — see + [the workspace browser tab model](#the-workspace-browser-tab-model) and + `.plan/workspace-browser-tab/complete/two-page-spike.json`. - **The isolated surface's CDP matrix was measured on Chromium only.** The probe ran against the bundled Chromium, not against Edge, so the isolated surface's cells describe that browser. Edge *was* exercised for the **extension** path — see the real-browser acceptance below — but the @@ -353,6 +353,7 @@ Every cell in this table is `measured`, and the evidence field is named. | Two pages of one isolated Chromium, live at the same time | **measured** | `substrate.pages` — two pages, two distinct target ids and two distinct target sessions | | Each page is driven independently | **measured** | `substrate.afterNavigation` — page A answers `Page A`, page B answers `Page B2` | | Each page is streamed independently | **measured** | `substrate.framesAtTheEnd.byEnvelopeSession` — A: 1 frame, B: 3 frames, from two different target sessions; `substrate.frameIdentityIsSufficient.bothPagesStreamedAtTheSameTime` is `true` | +| A page that is **not** in front produces no new frames | **measured** | typing into the background page added no frame to its stream; `Page.bringToFront` on that page made it produce one. Chrome renders the foregrounded tab and throttles the others | | Each page is input into independently | **measured** | `substrate.input` — page A's field reads `alpha`, page B's reads `bravo`; `independent` is `true` | | A navigation on the second page advances **that** page's generation and leaves the first alone | **measured** | `substrate.generation` — A stays `1`, B goes `1` → `2` | | A reference recorded against a page's previous generation is refused | **measured** | `substrate.fence.staleReferenceOnNavigatedPage` — refused with `browser_target_stale` | @@ -372,24 +373,28 @@ that closes it. It is a gap, not a denial. | Capability | Mark | Where it stands | | --- | --- | --- | -| The provider drives one page per web tab | **not yet measured** | `provider.ts:405`, `:266` and `:317` refuse every target that is not the launch page. #201 | -| The generation advances per page | **not yet measured** | `provider.ts:251` reports a constant `1` for every other page; `provider.ts:229` sends `Page.enable` to one session only; `provider.ts:532` raises one page's generation. #201 | -| A frame is routed to the page that asked for it | **not yet measured** | `provider.ts:543` stamps **every** screencast frame with the provider's own target id. #201 | +| The provider drives one page per web tab | **implemented** | the provider holds a map of target id → session and generation, and resolves the session by target in `sendCommand`, `sendInput`, `startImageStream` and `stopImageStream`. `packages/webui/src/runtime/browser/isolated-chromium/provider.ts` | +| The generation advances per page | **implemented** | the generation lives on the page, not on the browser. `Page.enable` goes to every page, and the raise is keyed by the event's own session. `provider.ts` — `#attachPage` and `#onEvent` | +| A frame is routed to the page that asked for it | **implemented** | a screencast frame is matched to its page by the **envelope's** session id, and the subscriber that receives it is that page's own. `provider.ts` — `#onEvent` | +| A page the provider never attached to is still refused | **implemented** | only the pages the provider attached to are in the map, and a target that is not in the map is refused with `browser_target_unknown` rather than being sent to the wrong page. `provider.ts` — `sendCommand` | +| The identity `open_tab` returns names the page it created | **not yet measured** | `open_tab` creates the page, attaches to it and registers it, but still returns the browser's own answer rather than the target's identity. #202 | +| The service observes a new page from the identity `open_tab` returns | **not yet measured** | the service learns about the new page from the provider's target listing, not from the action's answer. #202 | | The image stream is keyed by target | **not yet measured** | `browser-service.ts:180` keys the frame subscribers by surface and then by **run**. #204 | | The runtime publishes the session's web tabs as a snapshot | **not yet measured** | no such operation exists. #203 | -| The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. #206 | +| The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | | The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | | The frame operation's request names a target | **not yet measured** | `operation-names.ts:458` takes `{ surface, runId? }`. #204 | -| The provider's attachment retry | **implemented** | `provider.ts:588` — the mechanism is cited in full above | +| The provider's attachment retry | **implemented** | `provider.ts` — the mechanism is cited in full above | | The files tab is a singleton | **implemented** | `workspace-panel-state.ts:289` drops every `files` tab before adding one | | The two review tabs share one id | **implemented** | `workspace-panel-state.ts:279` and `:283` derive the same `tabId("review", sessionId, workspaceDir)` | | A tab's label is derived from its kind | **implemented** | `workspace-panel-state.ts:154` | -| The provider refuses a second page rather than sending a command to the wrong one | **implemented** | `provider.ts:410` — the comment says why, and the refusal is measured today: `phases.provider.driveNewPageNavigate`, `.driveNewPageInput` and `.streamNewPage` are all `browser_target_unknown` | -**One number worth stating plainly.** The provider's `open_tab` **does** create a page: the page count -goes from 2 to 3 (`phases.provider.pageCount`) and the new page is listed by `listTargets` with its own -target id (`phases.provider.newPageIdentity`). What does not exist is any way to *address* it. That is -the whole gap the programme closes, and it is a gap in this repository, not in the browser. +**What the provider does today, measured again.** The provider's `open_tab` creates a page, attaches +to it and registers it, and the page is listed with its own identity +(`phases.provider.newPageIsListedByTheProvider`). Driving it, inputing into it and streaming it all +succeed (`phases.provider.driveNewPageNavigate`, `.driveNewPageInput` and `.streamNewPage` are all +`ok`). The page count still goes from 2 to 3 (`phases.provider.pageCount`) — the third page is the +one Chrome opened for itself, which the provider has never attached to and therefore still refuses. ### The fence, on every page @@ -401,9 +406,10 @@ working on a page that is **not** the one the browser was launched with: | B (navigated) | its generation advanced `1` → `2`; a reference recorded against generation `1` was offered | refused with `browser_target_stale` | | A (untouched) | its generation stayed `1`; a reference recorded against generation `1` was offered | accepted | -A fence that only works on the launch page is a fence that does not work. The repository's fence today -is the launch page's fence only: `provider.ts:328` compares the command's generation against the one -generation the provider holds, and there is one of them. +A fence that only works on the launch page is a fence that does not work. The provider's fence now +covers every page: the generation it compares against is the page's own, so a reference recorded +against a page that navigated is refused on that page, and the same reference against a page that +did **not** navigate is still accepted. ### The registration cost of the new operation @@ -457,10 +463,10 @@ the thing that changes — which is what #207 does. ### Not yet measured — the tab model's open questions -- **The two-page case was measured on the substrate, not through the provider.** The provider cannot - hold two pages today, so the spike drove the CDP connection directly to find out what the browser - can do. The provider's own per-target map is #201, and the same spike re-run against it is what - closes this question. +- **The two-page case was first measured on the substrate and then through the provider.** The + provider could not hold two pages when the spike first ran, so the spike drove the CDP connection + directly to find out what the browser can do. The provider's per-target map is now implemented and + the same spike observes it driving, inputing into and streaming the second page. - **The frame mis-stamping count is a modelled consequence, not a provider observation.** `substrate.currentProviderFrameRule` applies the provider's current rule to the frames the spike observed, because the provider cannot stream a second page at all. It is arithmetic over a @@ -468,6 +474,10 @@ the thing that changes — which is what #207 does. - **Nothing end-to-end has been measured.** The client's tab model, the strip, the new tab page, the toolbar and the two selections are not exercised by any suite yet. #208, #209, #210, #211 and #212 are where that happens. +- **Only the foregrounded page renders.** Chrome throttles the pages it is not showing, so a tab in + the background produces no new frames. A strip shows one tab at a time, so the page in front is the + one that produces the picture — but whether a background tab should show a *still* picture, and + when it is refreshed, is not decided. - **The bridged surfaces have no tab model at all.** Everything measured here is the isolated Chromium. The extension bridge exposes one visible tab per browser, so a tab strip over it is a different question and was not asked. \ No newline at end of file diff --git a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts index a9d51ed02..b8ccf65b5 100644 --- a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts +++ b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts @@ -10,7 +10,7 @@ // client in the dependency set, and the commands Browser Use needs are a // small, well-understood set. // -// Three behaviours here are not obvious and are the reason this file exists +// Four behaviours here are not obvious and are the reason this file exists // rather than a call into an existing provider: // // * **The attachment race.** A capture issued immediately after a navigation @@ -19,9 +19,17 @@ // capture issued a moment later succeeds. The provider therefore waits for // the page to be attached rather than assuming a command that failed once // will keep failing. -// * **The generation follows the main frame.** A navigation replaces the page -// behind the tab, so the target's generation is raised then — that is what -// makes an element reference recorded before the navigation stale. +// * **One page per web tab.** A web tab is a page of this browser, and the +// provider holds one entry per tab: the CDP session it is addressed with, +// the generation of the page behind it, and the frame handler that asked +// for its picture. A command, an input and a frame all name a **target**, +// and the provider resolves the target to the page that carries it. +// * **The generation follows the main frame, and it follows it per page.** A +// navigation replaces the page behind one tab, so that tab's generation is +// raised then — that is what makes an element reference recorded before the +// navigation stale. The other tabs' pages are untouched, so their +// generations do not move: a generation is a fact about a page, not about +// the browser. // * **The image stream is a screencast.** `Page.startScreencast` pushes // frames as the page changes, which is what the panel needs; polling // `Page.captureScreenshot` would be a screenshot, not a live stream. @@ -133,6 +141,29 @@ export interface WebuiBrowserSurfaceProvider { close(): Promise; } +/** + * One page the provider drives. + * + * A web tab is a page of this browser, so the session and the generation + * belong to the page rather than to the browser. The generation is raised when + * the page behind the tab is replaced, and it moves on its own: one tab + * navigating leaves every other tab's generation where it was. + */ +interface WebuiIsolatedChromiumPage { + /** The target this page is addressed by. */ + readonly targetId: string; + /** The CDP session the connection addresses this page with. */ + readonly sessionId: string; + /** The generation of the page behind the tab. */ + generation: number; + /** The sequence number of the last frame this page delivered. */ + frameSequence: number; + /** Whether a subscriber is currently asking for this page's pictures. */ + streaming: boolean; + /** The subscriber that asked for this page's pictures. */ + frameHandler?: (frame: WebuiBrowserImageFrame) => void; +} + /** * The isolated Chromium provider. * @@ -147,12 +178,22 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide readonly #userDataDir: string; #child: ChildProcess | undefined; #connection: WebuiBrowserCdpConnection | undefined; - /** The CDP session of the page the panel is showing. */ - #pageSessionId: string | undefined; - #pageTargetId: string | undefined; - #generation = 0; - #frameSequence = 0; - #streaming = false; + /** + * The pages this provider drives, by target id. + * + * One entry per web tab. A target that is not in here is a page this + * provider has never attached to, so it cannot be driven. + */ + readonly #pages = new Map(); + /** + * The same pages, by the CDP session that addresses them. + * + * A CDP event carries the session it belongs to in its envelope, and that + * session is how an event is matched to the page it is about. + */ + readonly #pagesBySession = new Map(); + /** The page the browser was launched with. */ + #launchTargetId: string | undefined; #unsubscribe: (() => void) | undefined; private constructor(options: WebuiIsolatedChromiumProviderOptions) { @@ -218,15 +259,55 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide const created = await connection.send<{ targetId: string }>("Target.createTarget", { url: "about:blank", }); + this.#launchTargetId = created.targetId; + await this.#attachPage(created.targetId); + } + + /** + * Attaches to a page and registers it in the per-target map. + * + * Every page the provider drives is attached to, and `Page.enable` is sent to + * every one of them. A page that was never enabled never receives the + * navigation events that raise its generation, and a generation that never + * moves is a fence that never fires. + */ + async #attachPage(targetId: string): Promise { + const connection = this.#connection; + if (!connection) { + throw browserOperationError( + "browser_provider_unavailable", + "The isolated Chromium is not running.", + true, + ); + } const attached = await connection.send<{ sessionId: string }>("Target.attachToTarget", { - targetId: created.targetId, + targetId, flatten: true, }); - this.#pageTargetId = created.targetId; - this.#pageSessionId = attached.sessionId; - this.#generation = 1; + const page: WebuiIsolatedChromiumPage = { + targetId, + sessionId: attached.sessionId, + generation: 1, + frameSequence: 0, + streaming: false, + }; + this.#pages.set(targetId, page); + this.#pagesBySession.set(page.sessionId, page); + await this.#withAttachmentRetry(() => + connection.send("Page.enable", {}, page.sessionId), + ); + return page; + } + + /** The page a target names, when this provider drives it. */ + #pageOf(targetId: string): WebuiIsolatedChromiumPage | undefined { + return this.#pages.get(targetId); + } - await connection.send("Page.enable", {}, this.#pageSessionId); + /** The page a CDP session addresses. */ + #pageBySession(sessionId: string | undefined): WebuiIsolatedChromiumPage | undefined { + if (!sessionId) return undefined; + return this.#pagesBySession.get(sessionId); } async listTargets(): Promise { @@ -243,17 +324,22 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide const states: WebuiBrowserTargetState[] = []; for (const info of targets.targetInfos ?? []) { if (info.type !== "page") continue; + // The generation is the page's own. A page this provider drives reports + // the generation it has raised; a page it never attached to has no + // generation here — and cannot be driven either, so the number is only + // ever read as the first one. + const page = this.#pageOf(info.targetId); states.push({ id: { surface: this.surface, browserId: this.browserId, targetId: info.targetId, - generation: info.targetId === this.#pageTargetId ? this.#generation : 1, + generation: page?.generation ?? 1, }, title: info.title, url: info.url, status: "open", - selected: info.targetId === this.#pageTargetId, + selected: info.targetId === this.#launchTargetId, }); } return states; @@ -263,41 +349,48 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide targetId: string, onFrame: (frame: WebuiBrowserImageFrame) => void, ): Promise { - if (targetId !== this.#pageTargetId) { + const page = this.#pageOf(targetId); + if (!page) { throw browserOperationError( "browser_target_unknown", "The isolated Chromium has no such target.", ); } const connection = this.#connection; - const sessionId = this.#pageSessionId; - if (!connection || !sessionId) { + if (!connection) { throw browserOperationError( "browser_provider_unavailable", "The isolated Chromium is not running.", true, ); } - this.#frameHandler = onFrame; - this.#streaming = true; + page.frameHandler = onFrame; + page.streaming = true; + // Chrome renders the foregrounded tab and throttles the others, so a + // background page's screencast produces no new frames. The page that is + // being streamed is therefore the page that is brought to the front — the + // same thing a browser does when a person switches to a tab. + await this.#withAttachmentRetry(() => + connection.send("Page.bringToFront", {}, page.sessionId), + ); await this.#withAttachmentRetry(() => connection.send( "Page.startScreencast", { format: "jpeg", quality: 70, everyNthFrame: 1 }, - sessionId, + page.sessionId, ), ); } async stopImageStream(targetId: string): Promise { - if (targetId !== this.#pageTargetId) return; - this.#streaming = false; - this.#frameHandler = undefined; + const page = this.#pageOf(targetId); + if (!page) return; + page.streaming = false; + page.frameHandler = undefined; const connection = this.#connection; - const sessionId = this.#pageSessionId; - if (!connection || !sessionId) return; + if (!connection) return; try { - await connection.send("Page.stopScreencast", {}, sessionId); + await connection.send("Page.stopScreencast", {}, page.sessionId); } catch { // A browser that has already gone cannot be asked to stop; the resources // it held are gone with it. @@ -306,26 +399,37 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide async sendInput(command: WebuiBrowserInputCommand): Promise { const connection = this.#connection; - const sessionId = this.#pageSessionId; - if (!connection || !sessionId) { + if (!connection) { throw browserOperationError( "browser_provider_unavailable", "The isolated Chromium is not running.", true, ); } - if (command.target.browserId !== this.browserId || command.target.targetId !== this.#pageTargetId) { + if (command.target.browserId !== this.browserId) { throw browserOperationError( "browser_target_unknown", "The isolated Chromium has no such target.", ); } + // The page is resolved by the target, so an input reaches the tab it names + // and no other. + const page = this.#pageOf(command.target.targetId); + if (!page) { + throw browserOperationError( + "browser_target_unknown", + "The isolated Chromium has no such target.", + ); + } + const sessionId = page.sessionId; // The provider is the party that knows when the page behind the tab was // replaced, so it is the party that refuses an input recorded against the // previous page. The registry refuses the same input too, but the registry // learns about the replacement from this provider — so the check that - // actually protects the page has to live here as well. - if (command.target.generation !== this.#generation) { + // actually protects the page has to live here as well. The generation is + // the page's own, so the fence holds on every page and not only on the + // one the browser was launched with. + if (command.target.generation !== page.generation) { throw browserOperationError( "browser_target_stale", "The browser page was replaced; the input was not delivered.", @@ -407,24 +511,25 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide command: string, params: Record, ): Promise { - // The isolated Chromium drives one page, so the target is checked rather - // than used: a command aimed at another tab is refused rather than sent - // to the wrong page. - if (target.targetId !== this.#pageTargetId) { + // The command is addressed to a target, and the target is resolved to the + // page that carries it. A target this provider does not drive is refused + // rather than sent to the wrong page. + const page = this.#pageOf(target.targetId); + if (!page) { throw browserOperationError( "browser_target_unknown", "The isolated Chromium has no such target.", ); } const connection = this.#connection; - const sessionId = this.#pageSessionId; - if (!connection || !sessionId) { + if (!connection) { throw browserOperationError( "browser_provider_unavailable", "The isolated Chromium is not running.", true, ); } + const sessionId = page.sessionId; switch (command) { case "navigate": { const url = params.url; @@ -472,7 +577,14 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide } case "open_tab": { const url = typeof params.url === "string" ? params.url : "about:blank"; - return await connection.send("Target.createTarget", { url }); + const created = await connection.send<{ targetId: string }>("Target.createTarget", { + url, + }); + // The new page is attached to and registered, so it is a web tab this + // provider drives: it is commanded, streamed and input into like any + // other page, and it carries its own generation. + await this.#attachPage(created.targetId); + return created; } default: break; @@ -495,8 +607,9 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide } async close(): Promise { - this.#streaming = false; - this.#frameHandler = undefined; + this.#pages.clear(); + this.#pagesBySession.clear(); + this.#launchTargetId = undefined; this.#unsubscribe?.(); this.#unsubscribe = undefined; this.#connection?.close(); @@ -506,7 +619,6 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide await rm(this.#userDataDir, { recursive: true, force: true }).catch(() => undefined); } - #frameHandler: ((frame: WebuiBrowserImageFrame) => void) | undefined; readonly #targetChangeListeners = new Set< (change: WebuiBrowserTargetChange) => void >(); @@ -527,36 +639,46 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide * Only two matter here: the main frame navigating, which replaces the page * behind the target and raises its generation, and a screencast frame, which * is the panel's live image. + * + * Both are matched to their page by the **envelope's** session id — the + * flattened target session the connection puts on the event. The + * `event.params.sessionId` a screencast frame carries is the numeric + * acknowledgement id, and it names no page: two pages' frames carry the same + * one. */ #onEvent(event: WebuiBrowserCdpEvent): void { - if (event.method === "Page.frameNavigated" && event.sessionId === this.#pageSessionId) { + const page = this.#pageBySession(event.sessionId); + if (!page) return; + if (event.method === "Page.frameNavigated") { const frame = event.params.frame as { parentId?: string } | undefined; // Only the main frame's navigation replaces the page. A subframe's // navigation leaves the page — and every reference recorded against it — // intact. if (frame && frame.parentId === undefined) { - this.#generation += 1; - this.#announceTargetChange(); + // The generation is the page's own, so one page navigating does not + // move any other page's generation. + page.generation += 1; + this.#announceTargetChange(page); } return; } - if (event.method === "Page.screencastFrame" && this.#streaming) { + if (event.method === "Page.screencastFrame" && page.streaming) { const data = event.params.data; const metadata = event.params.metadata as { deviceWidth?: number } | undefined; if (typeof data !== "string") return; const sessionId = event.params.sessionId; void this.#ackScreencastFrame(sessionId); - this.#frameSequence += 1; - this.#frameHandler?.({ + page.frameSequence += 1; + page.frameHandler?.({ target: { surface: this.surface, browserId: this.browserId, - targetId: this.#pageTargetId ?? "", - generation: this.#generation, + targetId: page.targetId, + generation: page.generation, }, data, format: "jpeg", - sequence: this.#frameSequence, + sequence: page.frameSequence, }); void metadata; return; @@ -568,14 +690,14 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide * * The target registry that lives above this provider cannot know about a * navigation on its own, so without this announcement an input recorded - * against the previous page would still look current to it. + * against the previous page would still look current to it. The + * announcement names the target, because the replacement happened behind + * that tab and no other. */ - #announceTargetChange(): void { - const targetId = this.#pageTargetId; - if (!targetId) return; + #announceTargetChange(page: WebuiIsolatedChromiumPage): void { for (const listener of this.#targetChangeListeners) { try { - listener({ targetId, generation: this.#generation }); + listener({ targetId: page.targetId, generation: page.generation }); } catch { // A listener that throws must not take the browser down with it. } diff --git a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts new file mode 100644 index 000000000..4ef5ff107 --- /dev/null +++ b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts @@ -0,0 +1,274 @@ +// Two pages of one isolated Chromium. +// +// The provider holds one page per web tab. This suite proves the four things +// that follow from that, against a real browser: +// +// * two pages can be driven, streamed and input into independently; +// * a navigation on one page advances that page's generation and leaves the +// other page's alone; +// * a reference recorded against a page's previous generation is refused on +// every page, not only on the one the browser was launched with; +// * a frame is delivered to the page that asked for it and to no other. +// +// The frames are the sharpest of the four. A provider that stamped every +// screencast frame with its own target would still deliver *a* picture to +// every subscriber — the wrong one. Only the envelope's session id says which +// page a frame is about, so only a subscriber that receives its own page's +// frames proves the routing. +// +// The browser is optional: a machine without Chrome skips the suite rather +// than failing it, because "the browser is missing" is not a defect in the +// provider. The repository's `test:webui-browser` gate is the one that +// requires a browser. + +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { WebuiIsolatedChromiumProvider } from "../../src/runtime/browser/isolated-chromium/provider.js"; +import type { + WebuiBrowserImageFrame, + WebuiBrowserTargetId, +} from "../../src/shared/contracts/browser.js"; + +/** A page with a field and a title, so two pages can be told apart. */ +function pageHtml(title: string): string { + return [ + `${title}`, + "", + "", + ].join("\n"); +} + +/** The data URL of a page with the given title. */ +function pageUrl(title: string): string { + return `data:text/html,${encodeURIComponent(pageHtml(title))}`; +} + +/** How long a page is given to settle after a navigation. */ +const SETTLE_MS = 750; + +async function chromeIsAvailable(): Promise { + return (await resolveChromePath()) !== undefined; +} + +async function resolveChromePath(): Promise { + const { access } = await import("node:fs/promises"); + for (const candidate of [ + "/usr/bin/google-chrome", + "/usr/bin/google-chrome-stable", + "/usr/bin/chromium", + "/usr/bin/chromium-browser", + ]) { + try { + await access(candidate); + return candidate; + } catch { + // Keep looking. + } + } + return undefined; +} + +const available = await chromeIsAvailable(); + +describe.skipIf(!available)("the isolated Chromium, one page per web tab", () => { + let dataDir: string; + let provider: WebuiIsolatedChromiumProvider; + + beforeAll(async () => { + dataDir = await mkdtemp(join(tmpdir(), "webui-isolated-two-pages-")); + provider = await WebuiIsolatedChromiumProvider.launch({ dataDir }); + }, 60_000); + + afterAll(async () => { + await provider?.close(); + if (dataDir) await rm(dataDir, { recursive: true, force: true }); + }); + + /** The identity of the page the browser was launched with. */ + async function launchTarget(): Promise { + const selected = (await provider.listTargets()).find((entry) => entry.selected); + if (!selected) throw new Error("the provider reported no selected target"); + return selected.id; + } + + /** + * Opens a second page and returns its identity. + * + * The page is opened through the provider's own `open_tab`, so the test + * exercises the same path the Agent's tool uses. + */ + async function openPage(title: string): Promise { + const opened = (await provider.sendCommand(await launchTarget(), "open_tab", { + url: pageUrl(title), + })) as { targetId?: string } | undefined; + const targetId = opened?.targetId; + if (!targetId) throw new Error("open_tab returned no target"); + // The page is created by the browser before the provider has registered + // it, so the listing settles first. + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + const state = (await provider.listTargets()).find((entry) => entry.id.targetId === targetId); + if (!state) throw new Error("the new page is not in the target list"); + return state.id; + } + + /** The title the page behind the target is showing. */ + async function readTitle(target: WebuiBrowserTargetId): Promise { + const result = (await provider.sendCommand(target, "Runtime.evaluate", { + expression: "document.title", + returnByValue: true, + })) as { result?: { value?: unknown } } | undefined; + return typeof result?.result?.value === "string" ? result.result.value : ""; + } + + /** The value of the page's field. */ + async function readField(target: WebuiBrowserTargetId): Promise { + const result = (await provider.sendCommand(target, "Runtime.evaluate", { + expression: "document.getElementById('field').value", + returnByValue: true, + })) as { result?: { value?: unknown } } | undefined; + return typeof result?.result?.value === "string" ? result.result.value : ""; + } + + /** The field's centre, in page coordinates. */ + async function fieldCentre(target: WebuiBrowserTargetId): Promise<{ x: number; y: number }> { + const result = (await provider.sendCommand(target, "Runtime.evaluate", { + expression: + "(() => { const r = document.getElementById('field').getBoundingClientRect(); return { x: r.left + r.width / 2, y: r.top + r.height / 2 }; })()", + returnByValue: true, + })) as { result?: { value?: unknown } } | undefined; + const value = result?.result?.value as { x?: number; y?: number } | undefined; + return { x: value?.x ?? 0, y: value?.y ?? 0 }; + } + + /** Clicks the page's field and types into it. */ + async function typeInto(target: WebuiBrowserTargetId, text: string): Promise { + // The field is emptied first: the launch page is shared by the cases in + // this suite, and a field that still held the previous case's text would + // make the assertion about the *new* text read as a failure. + await provider.sendCommand(target, "Runtime.evaluate", { + expression: "document.getElementById('field').value = ''", + }); + const centre = await fieldCentre(target); + await provider.sendInput({ + kind: "pointer", + target, + x: centre.x, + y: centre.y, + button: "left", + clickCount: 1, + }); + await provider.sendInput({ kind: "text", target, text }); + } + + it("drives, streams and inputs into two pages independently", async () => { + const first = await launchTarget(); + const second = await openPage("second page"); + + expect(second.targetId).not.toBe(first.targetId); + + // Each page is driven on its own. The navigation replaces the page behind + // the tab, so the identity is read again afterwards: the one read before + // the navigation is stale by then, and that is the point. + await provider.sendCommand(first, "Page.navigate", { url: pageUrl("first page") }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + const firstAfterNavigation = await launchTarget(); + expect(await readTitle(firstAfterNavigation)).toBe("first page"); + expect(await readTitle(second)).toBe("second page"); + + // Each page is input into on its own. + await typeInto(firstAfterNavigation, "alpha"); + await typeInto(second, "bravo"); + expect(await readField(firstAfterNavigation)).toBe("alpha"); + expect(await readField(second)).toBe("bravo"); + }); + + it("advances one page's generation and leaves the other's alone", async () => { + const first = await launchTarget(); + const second = await openPage("generation page"); + + const before = new Map(); + for (const target of await provider.listTargets()) { + before.set(target.id.targetId, target.id.generation); + } + + await provider.sendCommand(second, "Page.navigate", { url: pageUrl("navigated page") }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + + const after = new Map(); + for (const target of await provider.listTargets()) { + after.set(target.id.targetId, target.id.generation); + } + + // The navigated page advanced. + expect(after.get(second.targetId)).toBe((before.get(second.targetId) ?? 0) + 1); + // The page that did not navigate did not. + expect(after.get(first.targetId)).toBe(before.get(first.targetId)); + }); + + it("refuses a stale reference on every page, not only the launch page", async () => { + const first = await launchTarget(); + const second = await openPage("fenced page"); + + const secondBefore = second.generation; + await provider.sendCommand(second, "Page.navigate", { url: pageUrl("fenced page two") }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + + // The stale identity of the page that was navigated. + await expect( + provider.sendInput({ kind: "text", target: second, text: "must not arrive" }), + ).rejects.toMatchObject({ code: "browser_target_stale" }); + + // The launch page's own current identity still works, so the fence did not + // simply break every input. + await typeInto(first, "still works"); + expect(await readField(first)).toBe("still works"); + }); + + it("delivers a frame to the page that asked for it and to no other", async () => { + const first = await launchTarget(); + const second = await openPage("streamed page"); + + const firstFrames: WebuiBrowserImageFrame[] = []; + const secondFrames: WebuiBrowserImageFrame[] = []; + + // Chrome renders the foregrounded tab and throttles the others, so the + // provider brings the page it is streaming to the front. The first page is + // streamed and driven first, and the second one after it, so each page's + // frames are produced while that page is the one on screen. + await provider.startImageStream(first.targetId, (frame) => firstFrames.push(frame)); + await typeInto(first, "one"); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + + await provider.startImageStream(second.targetId, (frame) => secondFrames.push(frame)); + await typeInto(second, "two"); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + + expect(firstFrames.length).toBeGreaterThan(0); + expect(secondFrames.length).toBeGreaterThan(0); + + // Every frame went to the page it belongs to, and to no other. This is + // what the envelope's session id buys: a provider that stamped every + // frame with its own target would still deliver a picture to both + // subscribers — the wrong one. + expect(firstFrames.every((frame) => frame.target.targetId === first.targetId)).toBe(true); + expect(secondFrames.every((frame) => frame.target.targetId === second.targetId)).toBe(true); + + // A frame's sequence number counts that page's frames, not the browser's. + expect(firstFrames.map((frame) => frame.sequence)).toEqual( + firstFrames.map((_frame, index) => index + 1), + ); + expect(secondFrames.map((frame) => frame.sequence)).toEqual( + secondFrames.map((_frame, index) => index + 1), + ); + + await provider.stopImageStream(first.targetId); + await provider.stopImageStream(second.targetId); + }); +}); diff --git a/release/public-source.json b/release/public-source.json index f6bf2bca3..53b9e994a 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -4222,6 +4222,7 @@ "packages/webui/test/gates/webui-dependency-check.test.ts", "packages/webui/test/integration/adapter-conversions.test.ts", "packages/webui/test/integration/browser-use-real-input.test.ts", + "packages/webui/test/integration/isolated-chromium-two-pages.test.ts", "packages/webui/test/integration/loopback-harness-path.test.ts", "packages/webui/test/integration/real-host-loopback.test.ts", "packages/webui/test/integration/real-host-model-turn.test.ts", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 400a0f87c..8508855ee 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -443,6 +443,7 @@ "packages/webui/test/unit/browser-panel-view.test.tsx", "packages/webui/test/unit/browser-settings-page.test.tsx", "packages/webui/test/integration/browser-use-real-input.test.ts", + "packages/webui/test/integration/isolated-chromium-two-pages.test.ts", "packages/webui/test/unit/browser-bridge-server.test.ts", "packages/webui/test/unit/browser-bridge-upgrade.test.ts", "packages/webui/test/unit/browser-workspace-panel.test.tsx", From bf4b12a601f85e2c3e74167ba6970408da4b1268 Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 20:37:38 +0800 Subject: [PATCH 04/34] feat(webui): open_tab answers with the identity of the page it created Issue #202. The action created a page and returned the browser's own answer, so the page it created could be named by nobody: the service learned about it only when the provider happened to be asked for its list again. The action now answers with the page's identity -- the surface, the browser id, the target id and the generation the provider's own accounting gives it -- and the service observes that target in the registry from the answer alone. The identity is therefore usable at once: it can be selected and input into without waiting for the next listing, which is what "the service observes the new target from that identity alone" has to mean in practice. The bridge is unchanged, and that is now a tested fact rather than an assumption: open_tab on Edge or Chrome is still refused with browser_capability_denied, because the extension drives the person's own browser through chrome.debugger and cannot open a page in it. The test drives the whole way -- the command reaches the extension, the extension refuses the method as outside its supported subset, and the refusal reaches the caller as browser_capability_denied. One browser fact the identity has to account for: a page that is still loading when the provider attaches to it is observed as a replaced page when its first load lands, so a newly opened page's generation may advance once, from 1 to 2. A page that finished loading before the provider attached stays at 1. The capability matrix records this as measured. --- .../docs/browser-use/01-capability-matrix.md | 5 +- .../src/runtime/browser/browser-service.ts | 67 +++++++++++++-- .../browser/isolated-chromium/provider.ts | 12 ++- .../isolated-chromium-two-pages.test.ts | 33 ++++++++ .../test/unit/browser-bridge-server.test.ts | 32 +++++++ .../webui/test/unit/browser-service.test.ts | 83 ++++++++++++++++++- 6 files changed, 221 insertions(+), 11 deletions(-) diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index a1dceff75..65c7be000 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -354,6 +354,7 @@ Every cell in this table is `measured`, and the evidence field is named. | Each page is driven independently | **measured** | `substrate.afterNavigation` — page A answers `Page A`, page B answers `Page B2` | | Each page is streamed independently | **measured** | `substrate.framesAtTheEnd.byEnvelopeSession` — A: 1 frame, B: 3 frames, from two different target sessions; `substrate.frameIdentityIsSufficient.bothPagesStreamedAtTheSameTime` is `true` | | A page that is **not** in front produces no new frames | **measured** | typing into the background page added no frame to its stream; `Page.bringToFront` on that page made it produce one. Chrome renders the foregrounded tab and throttles the others | +| A newly opened page's generation may advance once | **measured** | a page opened with a `data:` URL finished loading before the provider attached to it and stayed at generation `1`; a page opened with a real URL was still loading, and the provider saw the page behind the tab replaced — the generation went to `2` and a target change was announced | | Each page is input into independently | **measured** | `substrate.input` — page A's field reads `alpha`, page B's reads `bravo`; `independent` is `true` | | A navigation on the second page advances **that** page's generation and leaves the first alone | **measured** | `substrate.generation` — A stays `1`, B goes `1` → `2` | | A reference recorded against a page's previous generation is refused | **measured** | `substrate.fence.staleReferenceOnNavigatedPage` — refused with `browser_target_stale` | @@ -377,8 +378,8 @@ that closes it. It is a gap, not a denial. | The generation advances per page | **implemented** | the generation lives on the page, not on the browser. `Page.enable` goes to every page, and the raise is keyed by the event's own session. `provider.ts` — `#attachPage` and `#onEvent` | | A frame is routed to the page that asked for it | **implemented** | a screencast frame is matched to its page by the **envelope's** session id, and the subscriber that receives it is that page's own. `provider.ts` — `#onEvent` | | A page the provider never attached to is still refused | **implemented** | only the pages the provider attached to are in the map, and a target that is not in the map is refused with `browser_target_unknown` rather than being sent to the wrong page. `provider.ts` — `sendCommand` | -| The identity `open_tab` returns names the page it created | **not yet measured** | `open_tab` creates the page, attaches to it and registers it, but still returns the browser's own answer rather than the target's identity. #202 | -| The service observes a new page from the identity `open_tab` returns | **not yet measured** | the service learns about the new page from the provider's target listing, not from the action's answer. #202 | +| The identity `open_tab` returns names the page it created | **implemented** | the answer carries the surface, the browser id, the target id and the generation the provider's own accounting gives the page. `packages/webui/src/runtime/browser/isolated-chromium/provider.ts` — the `open_tab` case of `sendCommand` | +| The service observes a new page from the identity `open_tab` returns | **implemented** | the service reads the answer and observes that target in the registry, so the identity is usable at once — it does not have to wait for the provider to be asked for its list again. `packages/webui/src/runtime/browser/browser-service.ts` — `#observeOpenedTarget` | | The image stream is keyed by target | **not yet measured** | `browser-service.ts:180` keys the frame subscribers by surface and then by **run**. #204 | | The runtime publishes the session's web tabs as a snapshot | **not yet measured** | no such operation exists. #203 | | The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | diff --git a/packages/webui/src/runtime/browser/browser-service.ts b/packages/webui/src/runtime/browser/browser-service.ts index f4033e909..4c1b3dc43 100644 --- a/packages/webui/src/runtime/browser/browser-service.ts +++ b/packages/webui/src/runtime/browser/browser-service.ts @@ -770,7 +770,9 @@ export class WebuiBrowserService { `No browser target is selected for the "${surface}" surface.`, ); } - return { accepted: true, result: await provider.sendCommand(target, command, params) }; + const result = await provider.sendCommand(target, command, params); + this.#observeOpenedTarget(command, result); + return { accepted: true, result }; } /** @@ -819,16 +821,48 @@ export class WebuiBrowserService { ); } const provider = await this.#commandProvider(surface); + const result = await awaitAbortableCommand( + provider.sendCommand(selected, command, params), + signal, + ); + this.#observeOpenedTarget(command, result); return { accepted: true, target: selected, - result: await awaitAbortableCommand( - provider.sendCommand(selected, command, params), - signal, - ), + result, }; } + /** + * Observes a page that a command opened. + * + * A command that opened a page answers with that page's identity, and the + * registry learns about the page from the answer alone. The new target is + * therefore known before anything asks the provider for its list again, and + * with the generation the provider gave it rather than a guess. + * + * A command that did not open a page — or a provider whose answer is not an + * identity — is left alone. The registry is the service's record of what the + * browser has, and a command that opened nothing has nothing to add to it. + */ + #observeOpenedTarget(command: string, result: unknown): void { + if (command !== "open_tab") return; + const identity = readOpenedTargetIdentity(result); + if (!identity) return; + this.#registry.observe({ + surface: identity.surface, + browserId: identity.browserId, + targetId: identity.targetId, + // The title and the url are the provider's to report, and it reports + // them the next time it is asked. What the answer carries is the + // identity, and that is what the registry needs to know a page exists. + title: "", + url: "", + status: "open", + generation: identity.generation, + }); + } + /** * The gate both command paths make: the surface must be authorized, and the * command must be a restricted browser-tool action or permitted by Full CDP. @@ -962,3 +996,26 @@ async function awaitAbortableCommand( export function browserErrorCode(error: unknown): string | undefined { return error instanceof WebuiBrowserOperationError ? error.code : undefined; } + +/** + * The identity a command that opened a page answered with. + * + * The answer is read rather than assumed. A provider that did not open a page + * — or a surface that cannot open one at all — answers with something else, + * and the service must not invent a target out of it. + */ +function readOpenedTargetIdentity(result: unknown): WebuiBrowserTargetId | undefined { + if (typeof result !== "object" || result === null) return undefined; + const candidate = result as Record; + const { surface, browserId, targetId, generation } = candidate; + if ( + typeof surface !== "string" || + typeof browserId !== "string" || + typeof targetId !== "string" || + typeof generation !== "number" || + !Number.isInteger(generation) + ) { + return undefined; + } + return { surface: surface as WebuiBrowserSurface, browserId, targetId, generation }; +} diff --git a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts index b8ccf65b5..0f19a19a7 100644 --- a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts +++ b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts @@ -583,8 +583,16 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide // The new page is attached to and registered, so it is a web tab this // provider drives: it is commanded, streamed and input into like any // other page, and it carries its own generation. - await this.#attachPage(created.targetId); - return created; + const page = await this.#attachPage(created.targetId); + // The answer names the page that was created: the surface, the + // browser, the target and the generation the provider's own accounting + // gives it. A caller can address the page from this answer alone. + return { + surface: this.surface, + browserId: this.browserId, + targetId: page.targetId, + generation: page.generation, + } satisfies WebuiBrowserTargetId; } default: break; diff --git a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts index 4ef5ff107..4146f1969 100644 --- a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts +++ b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts @@ -231,6 +231,39 @@ describe.skipIf(!available)("the isolated Chromium, one page per web tab", () => expect(await readField(first)).toBe("still works"); }); + it("answers open_tab with the identity of the page it created", async () => { + const launch = await launchTarget(); + + const opened = (await provider.sendCommand(launch, "open_tab", { + url: pageUrl("opened page"), + })) as WebuiBrowserTargetId; + + // The answer names the page that was created: the surface, the browser, + // the target and the generation. + expect(opened.surface).toBe("isolated-chromium"); + expect(opened.browserId).toBe(provider.browserId); + expect(typeof opened.targetId).toBe("string"); + expect(typeof opened.generation).toBe("number"); + + // The page appears in the provider's target list under that identity. + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + const listed = (await provider.listTargets()).find( + (entry) => entry.id.targetId === opened.targetId, + ); + expect(listed?.id.surface).toBe(opened.surface); + expect(listed?.id.browserId).toBe(opened.browserId); + expect(listed?.id.targetId).toBe(opened.targetId); + + // The page the answer names is a page that can be driven. The generation + // is read from the list rather than from the answer: a page that is still + // loading is observed as a replaced page once its first load lands, and + // that is the generation the provider holds afterwards. + const current = listed!.id; + await provider.sendCommand(current, "Page.navigate", { url: pageUrl("driven page") }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + expect(await readTitle(current)).toBe("driven page"); + }); + it("delivers a frame to the page that asked for it and to no other", async () => { const first = await launchTarget(); const second = await openPage("streamed page"); diff --git a/packages/webui/test/unit/browser-bridge-server.test.ts b/packages/webui/test/unit/browser-bridge-server.test.ts index 096faace2..02a0da55b 100644 --- a/packages/webui/test/unit/browser-bridge-server.test.ts +++ b/packages/webui/test/unit/browser-bridge-server.test.ts @@ -262,6 +262,38 @@ describe("WebUI browser bridge — the providers it lends to the browser service client.socket.terminate(); }); + it("refuses to open a tab, because the extension drives the browser through its debugger", async () => { + const harness = await startHarness(); + const client = await pairEdge(harness); + const provider = harness.bridge.createProvider("edge"); + expect(provider).toBeDefined(); + announceTarget(client, 4); + await settle(); + + // The extension drives the person's own browser through `chrome.debugger`, + // which cannot open a page in it. The extension refuses the method + // because it is outside its supported subset, and the refusal reaches the + // caller as `browser_capability_denied`. + const pending = provider!.sendCommand( + { targetId: TARGET_ID, generation: 4 }, + "open_tab", + { url: "https://example.com/" }, + ); + const request = await client.waitFor<{ commandId: string }>("cdp"); + client.socket.send( + JSON.stringify({ + kind: "cdp-result", + commandId: request.commandId, + accepted: false, + error: 'the extension does not support the DevTools command "open_tab"', + unsupported: true, + }), + ); + const outcome = await pending.catch((error: unknown) => error); + expect(outcome).toMatchObject({ code: "browser_capability_denied" }); + client.socket.terminate(); + }); + it("drops an extension frame whose generation the announcement moved past", async () => { const harness = await startHarness(); const client = await pairEdge(harness); diff --git a/packages/webui/test/unit/browser-service.test.ts b/packages/webui/test/unit/browser-service.test.ts index 9cdbd80fe..09001c75f 100644 --- a/packages/webui/test/unit/browser-service.test.ts +++ b/packages/webui/test/unit/browser-service.test.ts @@ -39,6 +39,8 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { >(); /** The tabs this browser has, in listing order. */ readonly tabIds: readonly string[]; + /** The tabs a command opened, in the order they were opened. */ + readonly openedTabs: string[] = []; /** One frame handler per open stream, keyed by the tab it watches. */ readonly frameHandlers = new Map void>(); /** The most recently opened stream's handler. */ @@ -69,12 +71,24 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { _target: unknown, command: string, params: Record, - ): Promise { + ): Promise { this.commands.push({ command, params }); + if (command !== "open_tab") return undefined; + // A command that opened a page answers with that page's identity, the way + // the isolated provider answers: the surface, the browser, the target and + // the generation the provider's own accounting gives it. + const targetId = `tab-${this.tabIds.length + this.openedTabs.length + 1}`; + this.openedTabs.push(targetId); + return { + surface: this.surface, + browserId: this.browserId, + targetId, + generation: 1, + } satisfies WebuiBrowserTargetId; } async listTargets(): Promise { - return this.tabIds.map((targetId) => ({ + return [...this.tabIds, ...this.openedTabs].map((targetId) => ({ id: { surface: this.surface, browserId: this.browserId, @@ -473,6 +487,71 @@ describe("browser service", () => { await service.close(); }); + it("answers a command that opened a page with that page's identity", async () => { + const { service, provider } = createService({ + ...ALL_OFF, + isolatedChromium: true, + }); + await service.listTargets("isolated-chromium"); + await service.selectTarget(tab("tab-1")); + + const opened = await service.sendCommand("isolated-chromium", "open_tab", { + url: "https://example.test/", + }); + + // The answer names the page that was created: the surface, the browser, + // the target and the generation. + expect(opened.result).toEqual({ + surface: "isolated-chromium", + browserId: "browser-1", + targetId: "tab-2", + generation: 1, + }); + await service.close(); + }); + + it("knows a page that open_tab opened from the identity alone", async () => { + const { service, provider } = createService({ + ...ALL_OFF, + isolatedChromium: true, + }); + await service.listTargets("isolated-chromium"); + await service.selectTarget(tab("tab-1")); + + const opened = await service.sendCommand("isolated-chromium", "open_tab", { + url: "https://example.test/", + }); + const identity = opened.result as WebuiBrowserTargetId; + + // The service learned about the page from the answer, so the identity is + // usable at once — it does not have to wait for the provider to be asked + // for its list again. + await service.selectTarget(identity); + expect(await selectedTargetOf(service)).toBe("tab-2"); + + await service.sendInput({ kind: "text", target: identity, text: "hello" }); + expect(provider.inputs).toHaveLength(1); + expect(provider.inputs[0]?.target.targetId).toBe("tab-2"); + await service.close(); + }); + + it("leaves the registry alone for a command that opened no page", async () => { + const { service, provider } = createService({ + ...ALL_OFF, + isolatedChromium: true, + }); + await service.listTargets("isolated-chromium"); + await service.selectTarget(tab("tab-1")); + + // A command whose answer is not an identity must not be turned into one. + await service.sendCommand("isolated-chromium", "reload"); + await expect(service.selectTarget(tab("tab-9"))).rejects.toMatchObject({ + code: "browser_target_unknown", + }); + expect(provider.openedTabs).toHaveLength(0); + await service.close(); + }); + /** Waits until the stream has subscribed, then pushes one frame. */ async function pushWhenSubscribed( provider: RecordingProvider, From 8c50962becc5ebcf0d78e125948fe364afbc1aa0 Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 20:50:18 +0800 Subject: [PATCH 05/34] feat(webui): the runtime owns the session's web tabs and publishes them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #203. The runtime knew about a run's selection and about the browser's target list, but not about which pages the session had open as its tabs — so the client had to keep its own copy, and two copies of "what is open" disagree exactly when a tab is closed. The service now keeps the session's web tabs beside the run's selection and publishes them as a snapshot: each tab's id, its target identity, its title, its URL, and whether it is the run's selection. A snapshot rather than a delta — the list is small, and a snapshot is idempotent, so a client that reconnects reconciles against it instead of drifting. The snapshot travels on a new stream operation, watchBrowserWebTabs, modelled on watchBrowserFrames. Registering it is more than adding a name, and all eight sites are done: the capability manifest's browser.ready list, the operation-name constants, the operation spec, the contract type, the operation registry, the server's handler table, the port member and its binding, and the transport's request table. packages/webui/test/unit/browser-web-tabs-operation.test.ts is the executable form of that list — a site that loses the operation fails there rather than in a panel that quietly shows no tabs. The tab's id belongs to the client and the runtime's target identity is carried as the tab's binding, so a page that navigates keeps its target id and gains a generation while the web tab stays the same web tab. The provider forgets a page the browser destroyed, so a closed tab does not leave a page behind that a command could still be aimed at. --- .../docs/browser-use/01-capability-matrix.md | 23 +- .../src/client/application/port-slices.ts | 1 + .../browser/contracts/browser-port.ts | 16 ++ .../webui/src/client/platform/transport.ts | 14 +- packages/webui/src/client/platform/wire.ts | 35 +++ .../src/runtime/browser/browser-service.ts | 204 +++++++++++++++++- .../browser/isolated-chromium/provider.ts | 26 +++ packages/webui/src/runtime/harness/browser.ts | 3 + packages/webui/src/runtime/port.ts | 14 ++ .../webui/src/server/operation/browser.ts | 20 +- .../src/server/operation/handlers/browser.ts | 24 +++ .../webui/src/server/operation/operations.ts | 9 +- .../webui/src/shared/capability-manifest.ts | 1 + .../webui/src/shared/contracts/browser.ts | 49 +++++ packages/webui/src/shared/operation-names.ts | 22 ++ .../webui/test/unit/browser-service.test.ts | 127 ++++++++++- .../unit/browser-web-tabs-operation.test.ts | 107 +++++++++ .../webui/test/unit/webui-port-slices.test.ts | 2 +- .../unit/webui-unary-operation-typing.test.ts | 5 +- release/public-source.json | 1 + test/vitest-suites.json | 1 + 21 files changed, 676 insertions(+), 28 deletions(-) create mode 100644 packages/webui/test/unit/browser-web-tabs-operation.test.ts diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index 65c7be000..797dc3344 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -381,7 +381,8 @@ that closes it. It is a gap, not a denial. | The identity `open_tab` returns names the page it created | **implemented** | the answer carries the surface, the browser id, the target id and the generation the provider's own accounting gives the page. `packages/webui/src/runtime/browser/isolated-chromium/provider.ts` — the `open_tab` case of `sendCommand` | | The service observes a new page from the identity `open_tab` returns | **implemented** | the service reads the answer and observes that target in the registry, so the identity is usable at once — it does not have to wait for the provider to be asked for its list again. `packages/webui/src/runtime/browser/browser-service.ts` — `#observeOpenedTarget` | | The image stream is keyed by target | **not yet measured** | `browser-service.ts:180` keys the frame subscribers by surface and then by **run**. #204 | -| The runtime publishes the session's web tabs as a snapshot | **not yet measured** | no such operation exists. #203 | +| The runtime publishes the session's web tabs as a snapshot | **implemented** | the service keeps the session's web tabs beside the run's selection and publishes them on the `watchBrowserWebTabs` stream — each tab's id, its target identity, its title, its URL, and whether it is the run's selection. `packages/webui/src/runtime/browser/browser-service.ts` — `watchWebTabs` | +| The client subscribes to the published snapshot | **implemented** | the browser port carries `watchBrowserWebTabs`, the transport's request table has it, and the wire opens the stream. The strip renders from it in #208. `packages/webui/src/client/features/browser/contracts/browser-port.ts` | | The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | | The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | | The frame operation's request names a target | **not yet measured** | `operation-names.ts:458` takes `{ surface, runId? }`. #204 | @@ -416,18 +417,20 @@ did **not** navigate is still accepted. The programme adds one operation that publishes the session's web tabs. Registering it costs the **eight** sites below, and every one of them is a place where a change that is forgotten shows up as a -missing capability rather than as an error. +missing capability rather than as an error. **The `watchBrowserWebTabs` operation is registered at +every one of them**; `packages/webui/test/unit/browser-web-tabs-operation.test.ts` is the executable +form of this list, and a site that loses the operation fails there. | Site | Where | What the new operation needs | | --- | --- | --- | -| The capability manifest's `browser.ready` list | `packages/webui/src/shared/capability-manifest.ts:143` | the operation's name joins the list. A `ready` member that is not wired makes the whole browser family **not-ready** (`capability-manifest.ts:202`), not absent — a quiet degradation | -| The operation-name constants | `packages/webui/src/shared/operation-names.ts:51` | one new `as const` constant beside the eight browser names | -| The operation spec | `packages/webui/src/shared/operation-names.ts:458` | one row naming the request and the response type, beside `watchBrowserFrames` | -| The contract type | `packages/webui/src/shared/contracts/browser.ts` | the request and response types live here; `watchBrowserFrames` has none of its own, so the new operation introduces one | -| The operation registry | `packages/webui/src/server/operation/operations.ts:162` | one `registerOperation(registry, entry(""))` line inside the `if (browser)` block. **The order is a wire contract** (`operations.ts:69`) | -| The server handler | `packages/webui/src/server/operation/handlers/browser.ts:127` | a dedicated handler. A stream handler returns `{ stream: { ok: true, source } }` and lets the service's generator raise the errors — the handler does not catch them | -| The port member and its implementation | `packages/webui/src/runtime/port.ts:637` and `packages/webui/src/runtime/harness/browser.ts:206` | the member on `WebuiBrowserPort` and its body in `createBrowserPort` | -| The transport's request table | `packages/webui/src/client/platform/transport.ts:232` | the client's request builder; a stream also needs the wire session in `packages/webui/src/client/platform/wire.ts:144` and `:373` | +| The capability manifest's `browser.ready` list | `packages/webui/src/shared/capability-manifest.ts:143` | the operation's name joins the list. A `ready` member that is not wired makes the whole browser family **not-ready** (`capability-manifest.ts:202`), not absent — a quiet degradation. **Done.** | +| The operation-name constants | `packages/webui/src/shared/operation-names.ts:51` | one new `as const` constant beside the eight browser names. **Done.** | +| The operation spec | `packages/webui/src/shared/operation-names.ts:458` | one row naming the request and the response type, beside `watchBrowserFrames`. **Done.** | +| The contract type | `packages/webui/src/shared/contracts/browser.ts` | the request and response types live here; `watchBrowserFrames` has none of its own, so the new operation introduces one. **Done.** | +| The operation registry | `packages/webui/src/server/operation/operations.ts:162` | one `registerOperation(registry, entry(""))` line inside the `if (browser)` block. **The order is a wire contract** (`operations.ts:69`). **Done.** | +| The server handler | `packages/webui/src/server/operation/handlers/browser.ts:127` | a dedicated handler. A stream handler returns `{ stream: { ok: true, source } }` and lets the service's generator raise the errors — the handler does not catch them. **Done.** | +| The port member and its implementation | `packages/webui/src/runtime/port.ts:637` and `packages/webui/src/runtime/harness/browser.ts:206` | the member on `WebuiBrowserPort` and its body in `createBrowserPort`. **Done.** | +| The transport's request table | `packages/webui/src/client/platform/transport.ts:232` | the client's request builder; a stream also needs the wire session in `packages/webui/src/client/platform/wire.ts:144` and `:373`. **Done.** | **There is no `bind()` row to add.** The browser operations are dedicated handlers, not bindings: the `WEBUI_OPERATION_BINDINGS` table (`packages/webui/src/server/operation/bind-handlers.ts:149`) has no diff --git a/packages/webui/src/client/application/port-slices.ts b/packages/webui/src/client/application/port-slices.ts index 61647c0ae..9bbcfcbdf 100644 --- a/packages/webui/src/client/application/port-slices.ts +++ b/packages/webui/src/client/application/port-slices.ts @@ -141,6 +141,7 @@ const BROWSER_KEYS = [ "sendBrowserInput", "sendBrowserCommand", "watchBrowserFrames", + "watchBrowserWebTabs", "listBrowserCapabilities", "getBrowserInstallationKey", ] as const satisfies readonly (keyof BrowserPort)[]; diff --git a/packages/webui/src/client/features/browser/contracts/browser-port.ts b/packages/webui/src/client/features/browser/contracts/browser-port.ts index 83e60b085..8ca846860 100644 --- a/packages/webui/src/client/features/browser/contracts/browser-port.ts +++ b/packages/webui/src/client/features/browser/contracts/browser-port.ts @@ -30,6 +30,7 @@ import type { WebuiBrowserTargetId, WebuiBrowserTargetList, WebuiBrowserTargetState, + WebuiBrowserWebTabSnapshot, WebuiBridgeBrowserSurface, } from "../../../../shared/contracts/browser.js"; @@ -211,6 +212,21 @@ export interface BrowserPort { request: WebuiBrowserRunScope & { readonly surface: WebuiBrowserSurface }, onFrame: (frame: WebuiBrowserImageFrame) => void, ) => () => void; + /** + * The session's web tabs, as a snapshot, on a stream. + * + * The runtime owns the record of what the session has open, and this is how + * the client learns it: the strip renders what arrives here rather than + * keeping a copy of its own, because two copies of "what is open" disagree + * exactly when a tab is closed. + * + * Returns the disposer, like `watchBrowserFrames`: the subscription outlives + * the render that opened it. + */ + readonly watchBrowserWebTabs?: ( + request: WebuiBrowserRunScope & { readonly surface: WebuiBrowserSurface }, + onSnapshot: (snapshot: WebuiBrowserWebTabSnapshot) => void, + ) => () => void; readonly listBrowserCapabilities?: () => Promise; /** * Reads one extension-reached surface's installation key so the person can diff --git a/packages/webui/src/client/platform/transport.ts b/packages/webui/src/client/platform/transport.ts index e822729e7..90fcf0d8c 100644 --- a/packages/webui/src/client/platform/transport.ts +++ b/packages/webui/src/client/platform/transport.ts @@ -131,6 +131,7 @@ import { SEND_BROWSER_INPUT_OPERATION_NAME, SEND_BROWSER_COMMAND_OPERATION_NAME, WATCH_BROWSER_FRAMES_OPERATION_NAME, + WATCH_BROWSER_WEB_TABS_OPERATION_NAME, LIST_BROWSER_CAPABILITIES_OPERATION_NAME, GET_BROWSER_INSTALLATION_KEY_OPERATION_NAME, } from "../../shared/operation-names.js"; @@ -149,8 +150,15 @@ export function createWebuiTransport(options: WebuiTransportOptions): Required request(VERSION_OPERATION_NAME, undefined), @@ -245,6 +253,8 @@ export function createWebuiTransport(options: WebuiTransportOptions): Required request(SEND_BROWSER_COMMAND_OPERATION_NAME, body), watchBrowserFrames: (body, onFrame) => watchBrowserFrames(WATCH_BROWSER_FRAMES_OPERATION_NAME, body, onFrame), + watchBrowserWebTabs: (body, onSnapshot) => + watchBrowserWebTabs(WATCH_BROWSER_WEB_TABS_OPERATION_NAME, body, onSnapshot), listBrowserCapabilities: () => request(LIST_BROWSER_CAPABILITIES_OPERATION_NAME, {}), getBrowserInstallationKey: (body) => request(GET_BROWSER_INSTALLATION_KEY_OPERATION_NAME, body), diff --git a/packages/webui/src/client/platform/wire.ts b/packages/webui/src/client/platform/wire.ts index d726d1521..85079d387 100644 --- a/packages/webui/src/client/platform/wire.ts +++ b/packages/webui/src/client/platform/wire.ts @@ -19,6 +19,7 @@ import type { WebuiTerminalFrame } from "../../shared/contracts/terminal.js"; import type { WebuiBrowserImageFrame, WebuiBrowserSurface, + WebuiBrowserWebTabSnapshot, } from "../../shared/contracts/browser.js"; import type { OperationSpec, @@ -146,6 +147,18 @@ export interface WebuiWire { requestBody: { readonly surface: WebuiBrowserSurface }, onFrame: (frame: WebuiBrowserImageFrame) => void, ): () => void; + /** + * The web-tab snapshot stream. + * + * A watch rather than a stream, for the same reason the frame stream is one: + * the subscription outlives the render that opened it, and detaching has to + * release the runtime-side resources the stream held. + */ + watchBrowserWebTabs( + operation: string, + requestBody: { readonly surface: WebuiBrowserSurface }, + onSnapshot: (snapshot: WebuiBrowserWebTabSnapshot) => void, + ): () => void; /** The one HTTP route, built from the same base and token as the socket. */ workspaceFileUrl(input: { readonly workspaceDir: string; readonly path: string }): string; /** @@ -391,6 +404,27 @@ export function createWebuiWire({ }; } + function watchBrowserWebTabs( + operation: string, + requestBody: { readonly surface: WebuiBrowserSurface }, + onSnapshot: (snapshot: WebuiBrowserWebTabSnapshot) => void, + ): () => void { + let stopped = false; + const session = openWireSession(operation, requestBody, { + frame: (_session, frame) => { + // A stopped watch must not deliver, and `close()` is not a promise that + // the socket stops emitting: the caller's flag is what guarantees it. + if (stopped) return; + if (frame.kind !== "event" || !frame.body || typeof frame.body !== "object") return; + onSnapshot(frame.body as WebuiBrowserWebTabSnapshot); + }, + }); + return () => { + stopped = true; + session.close(); + }; + } + function workspaceFileUrl({ workspaceDir, path: filePath, @@ -411,6 +445,7 @@ export function createWebuiWire({ stream, watchTerminal, watchBrowserFrames, + watchBrowserWebTabs, workspaceFileUrl, watchEvents: createWebuiEventChannel({ websocketUrl, webSocket }), }; diff --git a/packages/webui/src/runtime/browser/browser-service.ts b/packages/webui/src/runtime/browser/browser-service.ts index 4c1b3dc43..bbc7ea9a1 100644 --- a/packages/webui/src/runtime/browser/browser-service.ts +++ b/packages/webui/src/runtime/browser/browser-service.ts @@ -44,6 +44,8 @@ import { type WebuiBrowserTargetId, type WebuiBrowserTargetList, type WebuiBrowserTargetState, + type WebuiBrowserWebTab, + type WebuiBrowserWebTabSnapshot, } from "../../shared/contracts/browser.js"; import { browserOperationError, @@ -181,6 +183,23 @@ export class WebuiBrowserService { WebuiBrowserSurface, Map void>> >(); + /** + * The web tabs each session has open, per surface and then per run. + * + * The runtime owns this record. The strip renders what the runtime + * publishes, so a client that kept its own copy of the open tabs would be a + * second authority about what is open — and two authorities disagree at + * exactly the moment a tab is closed. + */ + readonly #webTabs = new Map< + WebuiBrowserSurface, + Map> + >(); + /** The web-tab snapshot subscribers, per surface and then per run. */ + readonly #webTabSubscribers = new Map< + WebuiBrowserSurface, + Map void>> + >(); constructor(options: WebuiBrowserServiceOptions) { this.#options = options; @@ -205,6 +224,161 @@ export class WebuiBrowserService { return this.#selected.get(surface)?.get(runId)?.target; } + /** The web tabs one session has open on one surface. */ + #webTabsOf( + surface: WebuiBrowserSurface, + runId: string, + ): Map | undefined { + return this.#webTabs.get(surface)?.get(runId); + } + + /** + * Records a page a session opened as one of its web tabs. + * + * The identity is the one the provider answered `open_tab` with, so the + * runtime's record names the page that was actually created rather than one + * it inferred afterwards. + */ + #recordWebTab( + surface: WebuiBrowserSurface, + runId: string, + target: WebuiBrowserTargetId, + ): void { + const runs = this.#webTabs.get(surface) ?? new Map>(); + const tabs = runs.get(runId) ?? new Map(); + tabs.set(target.targetId, target); + runs.set(runId, tabs); + this.#webTabs.set(surface, runs); + this.#publishWebTabs(surface, runId); + } + + /** Wakes the snapshot subscribers of one session. */ + #publishWebTabs(surface: WebuiBrowserSurface, runId: string): void { + const subscribers = this.#webTabSubscribers.get(surface)?.get(runId); + if (!subscribers) return; + for (const wake of [...subscribers]) wake(); + } + + /** + * The session's web tabs, as a snapshot. + * + * The target identities come from the runtime's own record; the titles and + * urls come from the provider, which is the only party that knows what a + * page is showing. A tab the provider no longer lists is still published, + * with the title and url it last had — the runtime's record of what is open + * is not the provider's to decide. + */ + async #webTabSnapshot( + surface: WebuiBrowserSurface, + runId: string, + ): Promise { + const tabs = this.#webTabsOf(surface, runId); + const selected = this.#selectionOf(surface, runId); + const provider = this.#providers.get(surface); + const listed = provider ? await provider.listTargets() : []; + const byTargetId = new Map(listed.map((entry) => [entry.id.targetId, entry])); + const published = [...(tabs?.values() ?? [])].map((target) => { + const state = byTargetId.get(target.targetId); + return { + id: target.targetId, + // The generation is the page's own, so a page that navigated is + // published with its new generation while the web tab stays the same + // web tab. + target: { ...target, generation: state?.id.generation ?? target.generation }, + title: state?.title ?? "", + url: state?.url ?? "", + selected: selected + ? selected.surface === target.surface && + selected.browserId === target.browserId && + selected.targetId === target.targetId + : false, + } satisfies WebuiBrowserWebTab; + }); + return { surface, tabs: published }; + } + + /** + * The session's web tabs, on a stream. + * + * A snapshot rather than a delta: the list is small, and a snapshot is + * idempotent, so a client that reconnects receives the same snapshot and + * applying it twice changes nothing. The stream publishes only when the + * snapshot actually differs, so a client is not woken for a change that did + * not happen. + */ + async *watchWebTabs( + surface: WebuiBrowserSurface, + runId: string = WEBUI_BROWSER_NO_RUN_ID, + signal?: AbortSignal, + ): AsyncIterable { + const authorization = await this.readAuthorization(); + if (!isBrowserSurfaceUsable(authorization, surface)) { + throw browserOperationError( + "browser_surface_not_authorized", + `Browser surface "${surface}" is not authorized.`, + ); + } + await this.#ensureProvider(surface); + + let notify: (() => void) | undefined; + const wake = (): void => notify?.(); + const runs = this.#webTabSubscribers.get(surface) ?? new Map void>>(); + const subscribers = runs.get(runId) ?? new Set<() => void>(); + subscribers.add(wake); + runs.set(runId, subscribers); + this.#webTabSubscribers.set(surface, runs); + + try { + let published: string | undefined; + while (!signal?.aborted) { + const snapshot = await this.#webTabSnapshot(surface, runId); + const encoded = JSON.stringify(snapshot); + if (encoded !== published) { + published = encoded; + yield snapshot; + } + await Promise.race([ + new Promise((resolve) => { + notify = resolve; + }), + new Promise((resolve) => setTimeout(resolve, 250)), + ]); + notify = undefined; + } + } finally { + subscribers.delete(wake); + if (subscribers.size === 0) runs.delete(runId); + } + } + + /** + * Closes a web tab's page and forgets the tab. + * + * Closing a web tab closes its page: a tab the person closes does not leave + * a page behind in the Chromium, and the runtime's page set follows the + * strip. The tab leaves the next snapshot either way — a tab that is still + * published after it was closed would be a tab the strip cannot get rid of. + */ + async closeWebTab( + surface: WebuiBrowserSurface, + runId: string, + targetId: string, + ): Promise { + const tabs = this.#webTabsOf(surface, runId); + const target = tabs?.get(targetId); + if (!target) return; + tabs?.delete(targetId); + this.#publishWebTabs(surface, runId); + const provider = this.#providers.get(surface); + if (!provider) return; + try { + await provider.sendCommand(target, "Target.closeTarget", { targetId }); + } catch { + // A page that is already gone needs no closing, and a provider that + // refuses the command has already done what was asked. + } + } + /** * The surface one run is working on, or `undefined` when it has selected * nothing anywhere. @@ -408,6 +582,15 @@ export class WebuiBrowserService { } } } + // The page behind a web tab was replaced, so the tab's generation moved + // and the snapshot that publishes it is no longer the one the client + // holds. Every session that has the tab open has to hear about it. + for (const [runId, tabs] of [...(this.#webTabs.get(surface) ?? new Map())]) { + if (!tabs.has(change.targetId)) continue; + const tab = tabs.get(change.targetId); + if (tab && raised) tabs.set(change.targetId, raised.id); + this.#publishWebTabs(surface, runId); + } }); return provider; } @@ -419,6 +602,8 @@ export class WebuiBrowserService { this.#providers.delete(surface); this.#selected.delete(surface); this.#frameSubscribers.delete(surface); + this.#webTabs.delete(surface); + this.#webTabSubscribers.delete(surface); for (const target of this.#registry.list(surface)) { this.#registry.close({ surface, @@ -518,6 +703,9 @@ export class WebuiBrowserService { target, order: (this.#selectionOrder += 1), }); + // The snapshot says which web tab is the run's selection, so a selection + // that moved is a change the subscribers have to hear about. + this.#publishWebTabs(target.surface, runId); return { ...known, selected: true }; } @@ -771,7 +959,7 @@ export class WebuiBrowserService { ); } const result = await provider.sendCommand(target, command, params); - this.#observeOpenedTarget(command, result); + this.#observeOpenedTarget(command, result, surface, WEBUI_BROWSER_NO_RUN_ID); return { accepted: true, result }; } @@ -825,7 +1013,7 @@ export class WebuiBrowserService { provider.sendCommand(selected, command, params), signal, ); - this.#observeOpenedTarget(command, result); + this.#observeOpenedTarget(command, result, surface, runId); return { accepted: true, target: selected, @@ -841,11 +1029,20 @@ export class WebuiBrowserService { * therefore known before anything asks the provider for its list again, and * with the generation the provider gave it rather than a guess. * + * The page also joins the session's web tabs: the runtime owns the record of + * what the session has open, and the strip renders what the runtime + * publishes. + * * A command that did not open a page — or a provider whose answer is not an * identity — is left alone. The registry is the service's record of what the * browser has, and a command that opened nothing has nothing to add to it. */ - #observeOpenedTarget(command: string, result: unknown): void { + #observeOpenedTarget( + command: string, + result: unknown, + surface: WebuiBrowserSurface, + runId: string, + ): void { if (command !== "open_tab") return; const identity = readOpenedTargetIdentity(result); if (!identity) return; @@ -861,6 +1058,7 @@ export class WebuiBrowserService { status: "open", generation: identity.generation, }); + this.#recordWebTab(surface, runId, identity); } /** diff --git a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts index 0f19a19a7..13155e4a7 100644 --- a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts +++ b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts @@ -310,6 +310,21 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide return this.#pagesBySession.get(sessionId); } + /** + * Forgets a page the browser has destroyed. + * + * A closed tab does not leave a page behind in the provider's map: the page + * is gone, and a target that still resolved would let a command be aimed at + * a page that no longer exists. + */ + #forgetPage(targetId: string): void { + const page = this.#pages.get(targetId); + if (!page) return; + this.#pages.delete(targetId); + this.#pagesBySession.delete(page.sessionId); + if (this.#launchTargetId === targetId) this.#launchTargetId = undefined; + } + async listTargets(): Promise { const connection = this.#connection; if (!connection) return []; @@ -655,6 +670,17 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide * one. */ #onEvent(event: WebuiBrowserCdpEvent): void { + // A page that is gone is not a page the provider drives any more. The + // event carries the target's id rather than a session — it is a + // browser-level event — so the page is found by its id. + if ( + event.method === "Target.detachedFromTarget" || + event.method === "Target.targetDestroyed" + ) { + const destroyed = event.params.targetId; + if (typeof destroyed === "string") this.#forgetPage(destroyed); + return; + } const page = this.#pageBySession(event.sessionId); if (!page) return; if (event.method === "Page.frameNavigated") { diff --git a/packages/webui/src/runtime/harness/browser.ts b/packages/webui/src/runtime/harness/browser.ts index 9d7dd1594..e370cd433 100644 --- a/packages/webui/src/runtime/harness/browser.ts +++ b/packages/webui/src/runtime/harness/browser.ts @@ -28,6 +28,7 @@ import type { WebuiBrowserTargetId, WebuiBrowserTargetList, WebuiBrowserTargetState, + WebuiBrowserWebTabSnapshot, } from "../../shared/contracts/browser.js"; import { WebuiBrowserService, @@ -227,6 +228,8 @@ function createBrowserPort(service: WebuiBrowserService): WebuiBrowserPort { ) => service.sendCommandToSelection(surface, runId, command, params ?? {}), watchFrames: (surface: WebuiBrowserSurface, runId?: string, signal?: AbortSignal) => service.watchFrames(surface, runId, signal), + watchWebTabs: (surface: WebuiBrowserSurface, runId?: string, signal?: AbortSignal) => + service.watchWebTabs(surface, runId, signal), listCapabilities: () => service.listCapabilities(), }; } diff --git a/packages/webui/src/runtime/port.ts b/packages/webui/src/runtime/port.ts index d370e7832..6259b8476 100644 --- a/packages/webui/src/runtime/port.ts +++ b/packages/webui/src/runtime/port.ts @@ -12,6 +12,7 @@ import type { WebuiBrowserTargetId, WebuiBrowserTargetList, WebuiBrowserTargetState, + WebuiBrowserWebTabSnapshot, } from "../shared/contracts/browser.js"; import type { WebuiActiveTurnResult, @@ -697,6 +698,19 @@ export interface WebuiBrowserPort { runId?: string, signal?: AbortSignal, ): AsyncIterable; + /** + * The session's web tabs, as a snapshot, on a stream. + * + * The runtime owns the record of what the session has open: the strip + * renders what this publishes rather than keeping a copy of its own, and a + * snapshot is idempotent, so a client that reconnects reconciles against + * it instead of drifting. + */ + watchWebTabs( + surface: WebuiBrowserSurface, + runId?: string, + signal?: AbortSignal, + ): AsyncIterable; /** What each surface can offer right now. */ listCapabilities(): Promise<{ readonly entries: readonly WebuiBrowserCapabilityEntry[]; diff --git a/packages/webui/src/server/operation/browser.ts b/packages/webui/src/server/operation/browser.ts index 37a1546cb..24c55a5bb 100644 --- a/packages/webui/src/server/operation/browser.ts +++ b/packages/webui/src/server/operation/browser.ts @@ -1,8 +1,8 @@ // The browser operations. // -// Seven operations: four that answer a question, one that changes the -// authorization, one that forwards an input, and one that streams the live -// images. +// Ten operations: the ones that answer a question, the one that changes the +// authorization, the ones that forward an input and a command, and the two +// that stream — the live images, and the web tabs the session has open. // // The validators here are deliberately thin. The *shape* of a request is the // wire's business and belongs in this layer; whether an input command is @@ -23,6 +23,7 @@ import { SEND_BROWSER_COMMAND_OPERATION_NAME, SET_BROWSER_AUTHORIZATION_OPERATION_NAME, WATCH_BROWSER_FRAMES_OPERATION_NAME, + WATCH_BROWSER_WEB_TABS_OPERATION_NAME, } from "../../shared/operation-names.js"; import { WEBUI_BROWSER_SURFACES, @@ -100,6 +101,19 @@ export const watchBrowserFramesOperation: WebuiOperation validate: (body) => validateSurface(WATCH_BROWSER_FRAMES_OPERATION_NAME, body), }; +/** + * The web-tab snapshot stream. + * + * The request names a surface and, optionally, the run whose tabs it wants. + * The snapshot itself is the runtime's record of the session's web tabs, so + * there is nothing for the client to ask for beyond which session it is + * asking about. + */ +export const watchBrowserWebTabsOperation: WebuiOperation> = { + name: WATCH_BROWSER_WEB_TABS_OPERATION_NAME, + validate: (body) => validateSurface(WATCH_BROWSER_WEB_TABS_OPERATION_NAME, body), +}; + const AUTHORIZATION_SWITCHES = [ "isolatedChromium", "edge", diff --git a/packages/webui/src/server/operation/handlers/browser.ts b/packages/webui/src/server/operation/handlers/browser.ts index 289548d96..5e957a43c 100644 --- a/packages/webui/src/server/operation/handlers/browser.ts +++ b/packages/webui/src/server/operation/handlers/browser.ts @@ -30,6 +30,7 @@ import { sendBrowserInputOperation, setBrowserAuthorizationOperation, watchBrowserFramesOperation, + watchBrowserWebTabsOperation, } from "../browser.js"; import type { WebuiOperationHandler, @@ -141,6 +142,22 @@ export function createBrowserHandlerEntries( }, }); + const watchBrowserWebTabs: DedicatedHandler = ( + context, + body, + ) => ({ + stream: { + ok: true, + // The same envelope as the frame stream: a snapshot is a third kind of + // payload with the same shape. + source: browser.watchWebTabs( + readBrowserSurface(body as Record), + readBrowserRunId(body as Record), + context.signal, + ) as unknown as AsyncIterable>, + }, + }); + const listBrowserCapabilities: DedicatedHandler = async () => ({ body: await browser.listCapabilities(), @@ -227,6 +244,13 @@ export function createBrowserHandlerEntries( handle: watchBrowserFrames as WebuiOperationHandler, }, ], + [ + watchBrowserWebTabsOperation.name, + { + operation: watchBrowserWebTabsOperation, + handle: watchBrowserWebTabs as WebuiOperationHandler, + }, + ], [ listBrowserCapabilitiesOperation.name, { diff --git a/packages/webui/src/server/operation/operations.ts b/packages/webui/src/server/operation/operations.ts index 4ac11d6a5..88c3a468a 100644 --- a/packages/webui/src/server/operation/operations.ts +++ b/packages/webui/src/server/operation/operations.ts @@ -155,10 +155,10 @@ export function createOperationRegistry( registerOperation(registry, entry("watchTerminal")); } // The browser adapter is a real second surface (a browser, not a harness port - // method). When none is supplied the seven browser operations stay - // unregistered, so an inbound frame becomes an `unknown_operation` error - // rather than a "method is not a function" runtime explosion — the same wire - // contract a host without a browser has always emitted. + // method). When none is supplied the browser operations stay unregistered, so + // an inbound frame becomes an `unknown_operation` error rather than a + // "method is not a function" runtime explosion — the same wire contract a + // host without a browser has always emitted. if (browser) { registerOperation(registry, entry("getBrowserAuthorization")); registerOperation(registry, entry("setBrowserAuthorization")); @@ -167,6 +167,7 @@ export function createOperationRegistry( registerOperation(registry, entry("sendBrowserInput")); registerOperation(registry, entry("sendBrowserCommand")); registerOperation(registry, entry("watchBrowserFrames")); + registerOperation(registry, entry("watchBrowserWebTabs")); registerOperation(registry, entry("listBrowserCapabilities")); // Registered with the rest of the browser operations, and answering // `not_provisioned` when the composition root wired no bridge — the same diff --git a/packages/webui/src/shared/capability-manifest.ts b/packages/webui/src/shared/capability-manifest.ts index 79a9644da..81957017e 100644 --- a/packages/webui/src/shared/capability-manifest.ts +++ b/packages/webui/src/shared/capability-manifest.ts @@ -145,6 +145,7 @@ export const WEBUI_CAPABILITY_REGISTRY_PROOFS: Readonly< "selectBrowserTarget", "sendBrowserInput", "watchBrowserFrames", + "watchBrowserWebTabs", ], }, models: { operations: ["listModels", "selectModel"] }, diff --git a/packages/webui/src/shared/contracts/browser.ts b/packages/webui/src/shared/contracts/browser.ts index 0dae90ecc..b224efd29 100644 --- a/packages/webui/src/shared/contracts/browser.ts +++ b/packages/webui/src/shared/contracts/browser.ts @@ -202,6 +202,55 @@ export interface WebuiBrowserTargetList { readonly targets: readonly WebuiBrowserTargetState[]; } +/** + * One web tab the runtime publishes. + * + * A web tab is a page of the isolated Chromium. The `id` is the runtime's + * handle for it, and it is the target's id: a page that navigates keeps its + * target id and gains a generation, so the web tab is still the same web tab. + * + * The `target` is the **binding** the client holds. The tab's own id belongs + * to the client and is assigned when the tab is opened; the runtime's target + * identity is what the client binds the tab to, and it is the thing that + * identifies the page uniquely — a target id alone names a page in one + * browser only. + */ +export interface WebuiBrowserWebTab { + /** The runtime's id for the web tab. It is the target's id. */ + readonly id: string; + /** The target the web tab is bound to. The generation moves with the page. */ + readonly target: WebuiBrowserTargetId; + /** The page's title, as the provider reports it. */ + readonly title: string; + /** The page's url, as the provider reports it. */ + readonly url: string; + /** Whether this web tab is the run's selection. */ + readonly selected: boolean; +} + +/** + * The session's web tabs, as a snapshot. + * + * A snapshot rather than a delta: the list is small, and a snapshot is + * idempotent, so a client that reconnects reconciles against it instead of + * drifting away from the runtime's record of what is open. + */ +export interface WebuiBrowserWebTabSnapshot { + readonly surface: WebuiBrowserSurface; + readonly tabs: readonly WebuiBrowserWebTab[]; +} + +/** + * The key a client reconciles a web tab by. + * + * The runtime's id is the target's id, and a target id names a page in one + * browser only — two surfaces can hold the same one. The binding is the whole + * identity, so the binding is the key. + */ +export function webuiBrowserWebTabKey(target: WebuiBrowserTargetId): string { + return `${target.surface}:${target.browserId}:${target.targetId}`; +} + /** * A validated input command aimed at one target. * diff --git a/packages/webui/src/shared/operation-names.ts b/packages/webui/src/shared/operation-names.ts index 6dd94f650..9cfeb3b61 100644 --- a/packages/webui/src/shared/operation-names.ts +++ b/packages/webui/src/shared/operation-names.ts @@ -55,6 +55,14 @@ export const SELECT_BROWSER_TARGET_OPERATION_NAME = "selectBrowserTarget" as con export const SEND_BROWSER_INPUT_OPERATION_NAME = "sendBrowserInput" as const; export const SEND_BROWSER_COMMAND_OPERATION_NAME = "sendBrowserCommand" as const; export const WATCH_BROWSER_FRAMES_OPERATION_NAME = "watchBrowserFrames" as const; +/** + * The stream that publishes the session's web tabs. + * + * A snapshot rather than a delta, on a stream of its own: the strip renders + * what the runtime publishes, and the runtime's record of what is open is the + * authority — a client that kept its own copy would be a second authority. + */ +export const WATCH_BROWSER_WEB_TABS_OPERATION_NAME = "watchBrowserWebTabs" as const; export const LIST_BROWSER_CAPABILITIES_OPERATION_NAME = "listBrowserCapabilities" as const; /** * Reads one browser surface's installation key so the person can copy it. @@ -238,6 +246,7 @@ import type { WebuiBrowserTargetId, WebuiBrowserTargetList, WebuiBrowserTargetState, + WebuiBrowserWebTabSnapshot, } from "./contracts/browser.js"; import { type WebuiBridgeBrowserSurface } from "./contracts/browser.js"; import type { WebuiUsageQuotaResult } from "./contracts/usage-quota.js"; @@ -459,6 +468,10 @@ export interface OperationSpec { request: { readonly surface: WebuiBrowserSurface; readonly runId?: string }; response: WebuiBrowserFrameStreamEnvelope; }; + watchBrowserWebTabs: { + request: { readonly surface: WebuiBrowserSurface; readonly runId?: string }; + response: WebuiBrowserWebTabStreamEnvelope; + }; listBrowserCapabilities: { request: Record; response: { readonly entries: readonly WebuiBrowserCapabilityEntry[] }; @@ -724,6 +737,7 @@ export const WEBUI_STREAM_OPERATION_NAMES = [ WATCH_EVENTS_OPERATION_NAME, WATCH_TERMINAL_OPERATION_NAME, WATCH_BROWSER_FRAMES_OPERATION_NAME, + WATCH_BROWSER_WEB_TABS_OPERATION_NAME, ] as const; export type WebuiStreamOperationName = (typeof WEBUI_STREAM_OPERATION_NAMES)[number]; @@ -758,6 +772,14 @@ export interface WebuiBrowserFrameStreamEnvelope { }; } +/** The `{ stream }` envelope for the browser web-tab snapshot stream. */ +export interface WebuiBrowserWebTabStreamEnvelope { + readonly stream: { + readonly ok: true; + readonly source: AsyncIterable; + }; +} + /** The `{ stream }` envelope for a `sendMessage`/`resumeSession` result. */ export interface WebuiSendMessageStreamEnvelope { readonly stream: WebuiSendMessageResult; diff --git a/packages/webui/test/unit/browser-service.test.ts b/packages/webui/test/unit/browser-service.test.ts index 09001c75f..97901da75 100644 --- a/packages/webui/test/unit/browser-service.test.ts +++ b/packages/webui/test/unit/browser-service.test.ts @@ -11,6 +11,7 @@ import type { WebuiBrowserImageFrame, WebuiBrowserInputCommand, WebuiBrowserTargetId, + WebuiBrowserWebTabSnapshot, WebuiBrowserTargetState, } from "../../src/shared/contracts/browser.js"; @@ -41,6 +42,8 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { readonly tabIds: readonly string[]; /** The tabs a command opened, in the order they were opened. */ readonly openedTabs: string[] = []; + /** The generation each target has, as the provider's own accounting has it. */ + readonly generations = new Map(); /** One frame handler per open stream, keyed by the tab it watches. */ readonly frameHandlers = new Map void>(); /** The most recently opened stream's handler. */ @@ -62,6 +65,9 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { /** Announces that the page behind the target was replaced. */ announceTargetChange(generation: number, targetId = "tab-1"): void { + // The provider owns the generation, so the listing reports the new one + // too — the same way the isolated provider's own accounting moves. + this.generations.set(targetId, generation); for (const listener of this.targetChangeListeners) { listener({ targetId, generation }); } @@ -93,10 +99,10 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { surface: this.surface, browserId: this.browserId, targetId, - generation: 1, + generation: this.generations.get(targetId) ?? 1, }, - title: "Example", - url: "https://example.test", + title: `Tab ${targetId}`, + url: `https://example.test/${targetId}`, status: "open" as const, selected: true, })); @@ -190,6 +196,23 @@ async function selectedTargetOf( return list.targets.find((target) => target.selected)?.id.targetId; } +/** + * The first snapshot the service publishes for one session. + * + * Reading the first snapshot is what a client does when it connects: the + * stream opens with the runtime's current record, so a client that reconnects + * sees the same thing the first one saw. + */ +async function firstSnapshot( + service: WebuiBrowserService, + runId = "run-1", +): Promise { + for await (const snapshot of service.watchWebTabs("isolated-chromium", runId)) { + return snapshot; + } + throw new Error("the snapshot stream ended without publishing anything"); +} + const ALL_OFF: WebuiBrowserAuthorization = { isolatedChromium: false, edge: false, @@ -552,6 +575,104 @@ describe("browser service", () => { await service.close(); }); + it("publishes the session's web tabs as a snapshot", async () => { + const { service } = createService({ ...ALL_OFF, isolatedChromium: true }); + await service.listTargets("isolated-chromium"); + await service.selectTarget(tab("tab-1"), "run-1"); + await service.sendCommandToSelection("isolated-chromium", "run-1", "open_tab", { + url: "https://example.test/second", + }); + + const snapshot = await firstSnapshot(service); + expect(snapshot.surface).toBe("isolated-chromium"); + expect(snapshot.tabs).toHaveLength(1); + const [webTab] = snapshot.tabs; + // Each tab carries its id, its target identity, its title and its url. + expect(webTab?.id).toBe("tab-2"); + expect(webTab?.target).toEqual({ + surface: "isolated-chromium", + browserId: "browser-1", + targetId: "tab-2", + generation: 1, + }); + expect(webTab?.title).toBe("Tab tab-2"); + expect(webTab?.url).toBe("https://example.test/tab-2"); + await service.close(); + }); + + it("marks the run's selection in the snapshot", async () => { + const { service } = createService({ ...ALL_OFF, isolatedChromium: true }); + await service.listTargets("isolated-chromium"); + // A run acts on a page it has selected. The bootstrap — a run with no + // selection opening the first tab — is the Agent's side of this and lands + // with it; here the run selects first and then opens another tab. + await service.selectTarget(tab("tab-1"), "run-1"); + await service.sendCommandToSelection("isolated-chromium", "run-1", "open_tab", { + url: "https://example.test/second", + }); + const opened = await service.selectTarget(tab("tab-2"), "run-1"); + + const snapshot = await firstSnapshot(service, "run-1"); + expect(snapshot.tabs[0]?.selected).toBe(true); + expect(opened.selected).toBe(true); + await service.close(); + }); + + it("gives a client that reconnects the same snapshot", async () => { + const { service } = createService({ ...ALL_OFF, isolatedChromium: true }); + await service.listTargets("isolated-chromium"); + // A run acts on a page it has selected. The bootstrap — a run with no + // selection opening the first tab — is the Agent's side of this and lands + // with it; here the run selects first and then opens another tab. + await service.selectTarget(tab("tab-1"), "run-1"); + await service.sendCommandToSelection("isolated-chromium", "run-1", "open_tab", { + url: "https://example.test/second", + }); + + const first = await firstSnapshot(service, "run-1"); + const second = await firstSnapshot(service, "run-1"); + // A snapshot rather than a delta: applying it twice changes nothing. + expect(second).toEqual(first); + await service.close(); + }); + + it("drops a closed tab from the next snapshot", async () => { + const { service } = createService({ ...ALL_OFF, isolatedChromium: true }); + await service.listTargets("isolated-chromium"); + // A run acts on a page it has selected. The bootstrap — a run with no + // selection opening the first tab — is the Agent's side of this and lands + // with it; here the run selects first and then opens another tab. + await service.selectTarget(tab("tab-1"), "run-1"); + await service.sendCommandToSelection("isolated-chromium", "run-1", "open_tab", { + url: "https://example.test/second", + }); + expect((await firstSnapshot(service, "run-1")).tabs).toHaveLength(1); + + await service.closeWebTab("isolated-chromium", "run-1", "tab-2"); + expect((await firstSnapshot(service, "run-1")).tabs).toHaveLength(0); + await service.close(); + }); + + it("publishes a new generation when the page behind a web tab is replaced", async () => { + const { service, provider } = createService({ ...ALL_OFF, isolatedChromium: true }); + await service.listTargets("isolated-chromium"); + // A run acts on a page it has selected. The bootstrap — a run with no + // selection opening the first tab — is the Agent's side of this and lands + // with it; here the run selects first and then opens another tab. + await service.selectTarget(tab("tab-1"), "run-1"); + await service.sendCommandToSelection("isolated-chromium", "run-1", "open_tab", { + url: "https://example.test/second", + }); + expect((await firstSnapshot(service, "run-1")).tabs[0]?.target.generation).toBe(1); + + provider.announceTargetChange(2, "tab-2"); + // The web tab is still the same web tab; the page behind it is not. + const snapshot = await firstSnapshot(service, "run-1"); + expect(snapshot.tabs[0]?.id).toBe("tab-2"); + expect(snapshot.tabs[0]?.target.generation).toBe(2); + await service.close(); + }); + /** Waits until the stream has subscribed, then pushes one frame. */ async function pushWhenSubscribed( provider: RecordingProvider, diff --git a/packages/webui/test/unit/browser-web-tabs-operation.test.ts b/packages/webui/test/unit/browser-web-tabs-operation.test.ts new file mode 100644 index 000000000..debc1adf3 --- /dev/null +++ b/packages/webui/test/unit/browser-web-tabs-operation.test.ts @@ -0,0 +1,107 @@ +// The web-tab snapshot operation, and the six places it has to be registered. +// +// A new operation is not registered by adding a name. The browser family's +// readiness is declared in the capability manifest, the name reaches the +// operation-name constants, the operation registry, the server's handler +// table, the port and its binding, and the transport's request table — and +// every one of those is a place where a change that is forgotten shows up as +// a missing capability rather than as an error. +// +// So this file is the executable form of that list: it names every site and +// asserts the operation is there. A site that loses the operation fails +// here rather than in a panel that quietly shows no tabs. + +import { describe, expect, it } from "vitest"; + +import { createWebuiTransport } from "../../src/client/platform/transport.js"; +import { createWebuiPortSlices } from "../../src/client/application/port-slices.js"; +import { WEBUI_CAPABILITY_REGISTRY_PROOFS } from "../../src/shared/capability-manifest.js"; +import { + WATCH_BROWSER_WEB_TABS_OPERATION_NAME, + WEBUI_STREAM_OPERATION_NAMES, +} from "../../src/shared/operation-names.js"; +import { watchBrowserWebTabsOperation } from "../../src/server/operation/browser.js"; +import { createBrowserHandlerEntries } from "../../src/server/operation/handlers/browser.js"; +import { createOperationRegistry } from "../../src/server/operation/operations.js"; +import { WebuiBrowserService } from "../../src/runtime/browser/browser-service.js"; +import type { WebuiBrowserPort } from "../../src/runtime/port.js"; +import type { WebuiBrowserAuthorization } from "../../src/shared/contracts/browser.js"; + +/** A port that answers nothing; the registry only asks whether it is wired. */ +const inertBrowserPort = {} as unknown as WebuiBrowserPort; + +const AUTHORIZATION_OFF: WebuiBrowserAuthorization = { + isolatedChromium: false, + edge: false, + chrome: false, + fullCdp: false, +}; + +describe("the web-tab snapshot operation", () => { + it("is named in the operation-name constants", () => { + expect(WATCH_BROWSER_WEB_TABS_OPERATION_NAME).toBe("watchBrowserWebTabs"); + }); + + it("is a stream, and not a unary operation", () => { + expect(WEBUI_STREAM_OPERATION_NAMES).toContain(WATCH_BROWSER_WEB_TABS_OPERATION_NAME); + }); + + it("is on the browser family's ready list in the capability manifest", () => { + // The ready list is what the handshake derives from: an operation that is + // not wired makes the whole browser family `not-ready` rather than absent, + // so putting it here is a promise the other sites have to keep. + expect(WEBUI_CAPABILITY_REGISTRY_PROOFS.browser.ready).toContain( + WATCH_BROWSER_WEB_TABS_OPERATION_NAME, + ); + }); + + it("is registered in the operation registry", () => { + const registry = createOperationRegistry( + {} as never, + undefined, + inertBrowserPort, + ); + expect(registry.has(WATCH_BROWSER_WEB_TABS_OPERATION_NAME)).toBe(true); + }); + + it("is in the server's handler table", () => { + const entries = createBrowserHandlerEntries(inertBrowserPort); + expect(entries.has(watchBrowserWebTabsOperation.name)).toBe(true); + }); + + it("is a member of the browser port, and of the service behind it", () => { + // A compile-time assertion: a member that is not on `WebuiBrowserPort` + // cannot be bound, so this line is the proof that the port carries the + // operation. The runtime check is that the service behind the port + // answers it. + const portCarriesTheOperation: "watchWebTabs" extends keyof WebuiBrowserPort + ? true + : false = true; + expect(portCarriesTheOperation).toBe(true); + + const service = new WebuiBrowserService({ + authorization: { + read: async () => AUTHORIZATION_OFF, + write: async () => AUTHORIZATION_OFF, + }, + dataDir: "/tmp/webui-web-tabs-operation-test", + }); + expect(typeof service.watchWebTabs).toBe("function"); + }); + + it("is in the transport's request table", () => { + const transport = createWebuiTransport({ + websocketUrl: "ws://127.0.0.1:1", + token: "test-token", + webSocket: { send: () => undefined, close: () => undefined } as never, + }); + expect(typeof transport.watchBrowserWebTabs).toBe("function"); + }); + + it("reaches the browser slice of the client's port", () => { + const slices = createWebuiPortSlices({ + watchBrowserWebTabs: () => () => undefined, + } as never); + expect(typeof slices.browser?.watchBrowserWebTabs).toBe("function"); + }); +}); diff --git a/packages/webui/test/unit/webui-port-slices.test.ts b/packages/webui/test/unit/webui-port-slices.test.ts index 6eedaf9df..ae50ca401 100644 --- a/packages/webui/test/unit/webui-port-slices.test.ts +++ b/packages/webui/test/unit/webui-port-slices.test.ts @@ -54,7 +54,7 @@ const EXPECTED: Record< browser: [ "getBrowserAuthorization", "setBrowserAuthorization", "listBrowserTargets", "selectBrowserTarget", "sendBrowserInput", "sendBrowserCommand", - "watchBrowserFrames", + "watchBrowserFrames", "watchBrowserWebTabs", "listBrowserCapabilities", "getBrowserInstallationKey", ], diff --git a/packages/webui/test/unit/webui-unary-operation-typing.test.ts b/packages/webui/test/unit/webui-unary-operation-typing.test.ts index 4f93c5338..5b808db3e 100644 --- a/packages/webui/test/unit/webui-unary-operation-typing.test.ts +++ b/packages/webui/test/unit/webui-unary-operation-typing.test.ts @@ -27,7 +27,7 @@ import { import type { WebuiOperationPort } from "../../src/server/operation/bind-handlers.js"; import { createStreamHandlerEntries } from "../../src/server/operation/handlers/stream.js"; import { watchTerminalOperation } from "../../src/server/operation/workspace.js"; -import { watchBrowserFramesOperation } from "../../src/server/operation/browser.js"; +import { watchBrowserFramesOperation, watchBrowserWebTabsOperation } from "../../src/server/operation/browser.js"; import { WEBUI_STREAM_OPERATION_NAMES, type OperationSpec, @@ -116,7 +116,7 @@ describe("the unary request interface over OperationSpec", () => { `${name} is a stream and must not be unary`, ); } - assert.equal(streams.length, 5); + assert.equal(streams.length, 6); }); it("matches the server's registered stream descriptors exactly", () => { @@ -124,6 +124,7 @@ describe("the unary request interface over OperationSpec", () => { ...createStreamHandlerEntries(inertPort).keys(), watchTerminalOperation.name, watchBrowserFramesOperation.name, + watchBrowserWebTabsOperation.name, ].sort(); assert.deepEqual(registered, [...WEBUI_STREAM_OPERATION_NAMES].sort()); }); diff --git a/release/public-source.json b/release/public-source.json index 53b9e994a..36d63af72 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -4250,6 +4250,7 @@ "packages/webui/test/unit/browser-settings-page.test.tsx", "packages/webui/test/unit/browser-target-registry.test.ts", "packages/webui/test/unit/browser-tool-adapter.test.ts", + "packages/webui/test/unit/browser-web-tabs-operation.test.ts", "packages/webui/test/unit/browser-workspace-panel.test.tsx", "packages/webui/test/unit/commands.test.ts", "packages/webui/test/unit/composer-field-transitions.test.ts", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 8508855ee..f31ed96b7 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -448,6 +448,7 @@ "packages/webui/test/unit/browser-bridge-upgrade.test.ts", "packages/webui/test/unit/browser-workspace-panel.test.tsx", "packages/webui/test/unit/browser-tool-adapter.test.ts", + "packages/webui/test/unit/browser-web-tabs-operation.test.ts", "packages/webui/test/unit/browser-bridge-cdp.test.ts", "packages/webui/test/unit/browser-bridge-adversarial.test.ts", "packages/webui/test/unit/browser-bridge-credential-secrecy.test.ts" From 12977ab9af8b3ba5ccb2da237cae1a990099d552 Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 21:01:20 +0800 Subject: [PATCH 06/34] feat(webui): the image stream is keyed by the target, not by the run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #204. The frame stream took { surface, runId? } and the service resolved the run's selection to decide what to stream. That is what made the person and the Agent share one page, and it is also what stopped a person from looking at a second one. The stream is now keyed by the target: the client asks for the page it is showing, the service's frame subscribers are keyed by target, and a frame is delivered only to the tab that asked for it. One screencast per page, shared by that page's subscribers, because the provider holds one frame handler per page and two subscriptions for the same page would otherwise take each other's frames away. This is a change to an existing operation's request shape, not a second operation beside it. The name does not change; the request does. A client that still sends a run id is refused with an explicit error rather than answered with the run's selection, because a stream that silently answers with the wrong page is worse than one that says the request is out of date. The fence is unchanged and it is the whole point: a frame has to name the target the subscriber asked for, all four identity fields, and the page behind that target has to still be the one the frame is a picture of. A frame of another tab, or of a page that has been replaced, is dropped rather than put on screen — and the drop is counted, because a stream that quietly stops updating and a stream that is dropping frames look identical from the outside. The run's selection still decides what the Agent's commands act on. The two are different cells and this change does not move the other. The new real-browser case drives two pages of one real Chromium through the service and proves that neither stream receives the other's frames. --- .../docs/browser-use/01-capability-matrix.md | 4 +- .../browser/contracts/browser-port.ts | 12 +- .../browser/ui/BrowserPanelHost.public.tsx | 42 ++-- packages/webui/src/client/platform/wire.ts | 5 +- .../src/runtime/browser/browser-service.ts | 176 ++++++++-------- packages/webui/src/runtime/harness/browser.ts | 4 +- packages/webui/src/runtime/port.ts | 11 +- .../webui/src/server/operation/browser.ts | 65 +++++- .../src/server/operation/handlers/browser.ts | 2 +- packages/webui/src/shared/operation-names.ts | 5 +- .../isolated-chromium-two-pages.test.ts | 117 +++++++++++ .../test/unit/browser-frame-stream.test.ts | 67 +++++++ .../webui/test/unit/browser-service.test.ts | 189 ++++++++++-------- release/public-source.json | 1 + test/vitest-suites.json | 1 + 15 files changed, 481 insertions(+), 220 deletions(-) create mode 100644 packages/webui/test/unit/browser-frame-stream.test.ts diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index 797dc3344..ebb22da4e 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -380,12 +380,12 @@ that closes it. It is a gap, not a denial. | A page the provider never attached to is still refused | **implemented** | only the pages the provider attached to are in the map, and a target that is not in the map is refused with `browser_target_unknown` rather than being sent to the wrong page. `provider.ts` — `sendCommand` | | The identity `open_tab` returns names the page it created | **implemented** | the answer carries the surface, the browser id, the target id and the generation the provider's own accounting gives the page. `packages/webui/src/runtime/browser/isolated-chromium/provider.ts` — the `open_tab` case of `sendCommand` | | The service observes a new page from the identity `open_tab` returns | **implemented** | the service reads the answer and observes that target in the registry, so the identity is usable at once — it does not have to wait for the provider to be asked for its list again. `packages/webui/src/runtime/browser/browser-service.ts` — `#observeOpenedTarget` | -| The image stream is keyed by target | **not yet measured** | `browser-service.ts:180` keys the frame subscribers by surface and then by **run**. #204 | +| The image stream is keyed by target | **implemented** | the client asks for the target it is showing; the service's frame subscribers are keyed by target, one screencast per page, and a frame that names another page is dropped and counted. `packages/webui/src/runtime/browser/browser-service.ts` — `watchFrames` | | The runtime publishes the session's web tabs as a snapshot | **implemented** | the service keeps the session's web tabs beside the run's selection and publishes them on the `watchBrowserWebTabs` stream — each tab's id, its target identity, its title, its URL, and whether it is the run's selection. `packages/webui/src/runtime/browser/browser-service.ts` — `watchWebTabs` | | The client subscribes to the published snapshot | **implemented** | the browser port carries `watchBrowserWebTabs`, the transport's request table has it, and the wire opens the stream. The strip renders from it in #208. `packages/webui/src/client/features/browser/contracts/browser-port.ts` | | The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | | The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | -| The frame operation's request names a target | **not yet measured** | `operation-names.ts:458` takes `{ surface, runId? }`. #204 | +| The frame operation's request names a target | **implemented** | `watchBrowserFrames` takes `{ surface, target }`. A request that still carries a run id is refused with an explicit error rather than answered with the run's selection. `packages/webui/src/server/operation/browser.ts` — `validateFrameStream` | | The provider's attachment retry | **implemented** | `provider.ts` — the mechanism is cited in full above | | The files tab is a singleton | **implemented** | `workspace-panel-state.ts:289` drops every `files` tab before adding one | | The two review tabs share one id | **implemented** | `workspace-panel-state.ts:279` and `:283` derive the same `tabId("review", sessionId, workspaceDir)` | diff --git a/packages/webui/src/client/features/browser/contracts/browser-port.ts b/packages/webui/src/client/features/browser/contracts/browser-port.ts index 8ca846860..3349d319f 100644 --- a/packages/webui/src/client/features/browser/contracts/browser-port.ts +++ b/packages/webui/src/client/features/browser/contracts/browser-port.ts @@ -200,16 +200,18 @@ export interface BrowserPort { ) => Promise<{ readonly accepted: true; readonly result?: unknown }>; /** - * The live image stream for one surface, carrying the named run's own target. + * The live image stream for one **target**. + * + * The stream is keyed by the target rather than by the run: the client asks + * for the page it is showing, so two tabs get their own pictures and the + * person and the Agent can work on different pages at the same time. * * Returns the disposer, matching `TerminalPort.watchTerminal`, because the * subscription outlives the render that opened it and something has to close - * it when the panel goes away. Switching runs closes this one and opens the - * active run's: a frame belonging to another run's target is not this - * stream's to show. + * it when the panel goes away. */ readonly watchBrowserFrames?: ( - request: WebuiBrowserRunScope & { readonly surface: WebuiBrowserSurface }, + request: { readonly surface: WebuiBrowserSurface; readonly target: WebuiBrowserTargetId }, onFrame: (frame: WebuiBrowserImageFrame) => void, ) => () => void; /** diff --git a/packages/webui/src/client/features/browser/ui/BrowserPanelHost.public.tsx b/packages/webui/src/client/features/browser/ui/BrowserPanelHost.public.tsx index 1590e78f3..0430267fe 100644 --- a/packages/webui/src/client/features/browser/ui/BrowserPanelHost.public.tsx +++ b/packages/webui/src/client/features/browser/ui/BrowserPanelHost.public.tsx @@ -112,13 +112,11 @@ export function BrowserPanelHost({ // a screencast running for a surface nobody is looking at. const subscribedSurface = useRef(undefined); - // The run the stream was opened for. The stream carries one run's target, so - // the active run changing means the old one has to be closed and the new one's - // opened — otherwise the panel would keep showing the previous run's page. - const subscribedRunId = useRef(undefined); - /** The target the open subscription is attached to. */ + // The target the open subscription is attached to. The stream is keyed by + // the target rather than by the run, so a run change does not reopen it — + // the run decides what the Agent's commands act on, and the panel decides + // what it shows. const subscribedTarget = useRef(undefined); - // The run the panel is on, kept where a read that is still in flight can see // it. The effect below is what moves it, so it is current before any read for // the new run starts. @@ -195,45 +193,41 @@ export function BrowserPanelHost({ void refreshTargets(surface); }, [port, refreshTargets, surface]); - // The live image stream is opened for the run's **selected** target. + // The live image stream is opened for the **target** the panel is showing. // - // The server refuses a stream for a run that has selected nothing, so the - // subscription is tied to the selection rather than to the panel's lifetime: - // when the person picks a target the stream opens, and when the run's target - // moves the previous subscription is closed and a new one opened for the new - // target. A subscription opened before there was anything to show would never - // deliver a frame, and the panel would stay blank until something else - // changed. - const selectedTargetKey = webuiBrowserStreamTargetKey( - selectWebuiBrowserStreamTarget(state), - ); + // The stream is keyed by the target rather than by the run, so two tabs get + // their own pictures and the person and the Agent can work on different + // pages at the same time. The subscription is tied to the target rather than + // to the panel's lifetime: a subscription opened before there was anything + // to show would never deliver a frame, and the panel would stay blank until + // something else changed. A page that navigates gets a new generation, and + // the subscription follows it. + const streamTarget = selectWebuiBrowserStreamTarget(state); + const selectedTargetKey = webuiBrowserStreamTargetKey(streamTarget); useEffect(() => { if (!port?.watchBrowserFrames) return; - if (!selectedTargetKey) { - // Nothing is selected, so there is no page to stream. + if (!streamTarget || !selectedTargetKey) { + // Nothing is shown, so there is no page to stream. return; } if ( subscribedSurface.current === surface && - subscribedRunId.current === runId && subscribedTarget.current === selectedTargetKey ) { return; } subscribedSurface.current = surface; - subscribedRunId.current = runId; subscribedTarget.current = selectedTargetKey; - const stop = port.watchBrowserFrames({ surface, ...runScope }, (frame) => { + const stop = port.watchBrowserFrames({ surface, target: streamTarget }, (frame) => { dispatch({ type: "frame", frame }); }); return () => { stop(); subscribedSurface.current = undefined; - subscribedRunId.current = undefined; subscribedTarget.current = undefined; }; - }, [port, runScope, selectedTargetKey, surface, runId]); + }, [port, selectedTargetKey, streamTarget, surface]); const onSelectTarget = useCallback( (target: WebuiBrowserTargetId) => { diff --git a/packages/webui/src/client/platform/wire.ts b/packages/webui/src/client/platform/wire.ts index 85079d387..d45186391 100644 --- a/packages/webui/src/client/platform/wire.ts +++ b/packages/webui/src/client/platform/wire.ts @@ -19,6 +19,7 @@ import type { WebuiTerminalFrame } from "../../shared/contracts/terminal.js"; import type { WebuiBrowserImageFrame, WebuiBrowserSurface, + WebuiBrowserTargetId, WebuiBrowserWebTabSnapshot, } from "../../shared/contracts/browser.js"; import type { @@ -144,7 +145,7 @@ export interface WebuiWire { */ watchBrowserFrames( operation: string, - requestBody: { readonly surface: WebuiBrowserSurface }, + requestBody: { readonly surface: WebuiBrowserSurface; readonly target: WebuiBrowserTargetId }, onFrame: (frame: WebuiBrowserImageFrame) => void, ): () => void; /** @@ -385,7 +386,7 @@ export function createWebuiWire({ function watchBrowserFrames( operation: string, - requestBody: { readonly surface: WebuiBrowserSurface }, + requestBody: { readonly surface: WebuiBrowserSurface; readonly target: WebuiBrowserTargetId }, onFrame: (frame: WebuiBrowserImageFrame) => void, ): () => void { let stopped = false; diff --git a/packages/webui/src/runtime/browser/browser-service.ts b/packages/webui/src/runtime/browser/browser-service.ts index bbc7ea9a1..5a75765d5 100644 --- a/packages/webui/src/runtime/browser/browser-service.ts +++ b/packages/webui/src/runtime/browser/browser-service.ts @@ -178,11 +178,24 @@ export class WebuiBrowserService { #selectionOrder = 0; /** Which external surfaces the service has turned on, per surface. */ readonly #bridgeSurfaces = new Map<"edge" | "chrome", boolean>(); - /** The frame subscribers, per surface and then per run. */ + /** The frame subscribers, per surface and then per target. */ readonly #frameSubscribers = new Map< WebuiBrowserSurface, Map void>> >(); + /** + * The provider-side frame handler, per surface and then per target. + * + * One screencast per page, shared by that page's subscribers: the provider + * holds one frame handler per page, so two subscriptions for the same page + * would otherwise take each other's frames away. + */ + readonly #frameHandlers = new Map< + WebuiBrowserSurface, + Map void> + >(); + /** How many frames each surface dropped, per surface. */ + readonly #droppedFrames = new Map(); /** * The web tabs each session has open, per surface and then per run. * @@ -775,7 +788,7 @@ export class WebuiBrowserService { */ async *watchFrames( surface: WebuiBrowserSurface, - runId: string = WEBUI_BROWSER_NO_RUN_ID, + target: WebuiBrowserTargetId, signal?: AbortSignal, ): AsyncIterable { const authorization = await this.readAuthorization(); @@ -793,15 +806,19 @@ export class WebuiBrowserService { true, ); } - const selected = this.#selectionOf(surface, runId); - if (!selected) { - // Streaming nothing would leave the panel showing a browser it cannot - // watch, so a run that has not selected anything is refused here and the - // caller is told to select first. The stream below follows the selection - // once it exists; this is only about the starting point. + // The target has to be a page this service knows. A stream for a target + // that does not exist would deliver nothing at all, and a panel waiting + // for a picture that cannot arrive looks like a panel that is broken. + if (!this.#registry.resolve(target)) { throw browserOperationError( "browser_target_unknown", - "No browser target is selected for this surface.", + "The browser target does not exist.", + ); + } + if (!this.#registry.isCurrent(target)) { + throw browserOperationError( + "browser_target_stale", + "The browser target has been replaced; select it again.", ); } // The queue keeps the newest frame per target and nothing else. @@ -814,6 +831,15 @@ export class WebuiBrowserService { const queue: WebuiBrowserImageFrame[] = []; let notify: (() => void) | undefined; const deliver = (frame: WebuiBrowserImageFrame) => { + // The fence. A frame belongs to the page behind the target the + // subscriber asked for, and all four identity fields decide it. A frame + // that names another page — another tab's, or a previous generation of + // this one — is dropped here rather than queued, so a subscriber never + // sees a picture of a page it did not ask for, not even briefly. + if (!this.#frameMatchesTarget(target, frame)) { + this.#droppedFrames.set(surface, (this.#droppedFrames.get(surface) ?? 0) + 1); + return; + } const index = queue.findIndex( (queued) => queued.target.targetId === frame.target.targetId && @@ -824,32 +850,42 @@ export class WebuiBrowserService { else queue[queue.length - 1] = frame; notify?.(); }; - const runs = this.#frameSubscribers.get(surface) ?? new Map< - string, - Set<(frame: WebuiBrowserImageFrame) => void> - >(); - const subscribers = runs.get(runId) ?? new Set<(frame: WebuiBrowserImageFrame) => void>(); + + // One provider-side screencast per page, shared by that page's + // subscribers: the provider holds one frame handler per page, so two + // subscriptions for the same page would otherwise take each other's + // frames away. + const targetKey = target.targetId; + const surfaceSubscribers = + this.#frameSubscribers.get(surface) ?? + new Map void>>(); + const subscribers = + surfaceSubscribers.get(targetKey) ?? + new Set<(frame: WebuiBrowserImageFrame) => void>(); + const first = subscribers.size === 0; subscribers.add(deliver); - runs.set(runId, subscribers); - this.#frameSubscribers.set(surface, runs); + surfaceSubscribers.set(targetKey, subscribers); + this.#frameSubscribers.set(surface, surfaceSubscribers); + + const surfaceHandlers = + this.#frameHandlers.get(surface) ?? + new Map void>(); + const handler = + surfaceHandlers.get(targetKey) ?? + ((frame: WebuiBrowserImageFrame) => { + const current = this.#frameSubscribers.get(surface)?.get(targetKey); + if (!current) return; + for (const subscriber of [...current]) subscriber(frame); + }); + surfaceHandlers.set(targetKey, handler); + this.#frameHandlers.set(surface, surfaceHandlers); - // The target the provider-side stream is attached to right now. It moves - // with the run's selection, so `finally` releases whichever target the - // stream ended up on rather than the one it opened with. - let streaming: string | undefined = selected.targetId; - await provider.startImageStream(streaming, deliver); + if (first) await provider.startImageStream(targetKey, handler); try { while (!signal?.aborted) { - streaming = await this.#followSelection(surface, runId, provider, deliver, streaming); while (queue.length > 0) { const frame = queue.shift(); if (!frame) continue; - // A frame belongs to the panel only while it names the page this - // run's panel is showing. A frame that arrives after that selection - // moved, after the page behind the target was replaced, or that - // belongs to another run's target would put a different page on - // screen than the one the person chose. - if (!this.#frameMatchesSelection(surface, runId, frame)) continue; yield frame; } await Promise.race([ @@ -862,77 +898,49 @@ export class WebuiBrowserService { } } finally { subscribers.delete(deliver); - if (subscribers.size === 0) runs.delete(runId); - // Cancelling the stream releases the provider-side resources it held: - // the screencast is stopped, not merely ignored. - if (streaming) await provider.stopImageStream(streaming).catch(() => undefined); + if (subscribers.size === 0) { + // Cancelling the stream releases the provider-side resources it held: + // the screencast is stopped, not merely ignored. + await provider.stopImageStream(targetKey).catch(() => undefined); + surfaceSubscribers.delete(targetKey); + surfaceHandlers.delete(targetKey); + } } } /** - * Moves one stream onto the target its run has selected now, and reports the - * target it is attached to afterwards. + * How many frames this service has dropped on a surface. * - * A run selects a target again and again while a stream runs, and the stream - * has to follow: the person pointed at another tab, so the panel has to show - * that tab. Re-reading the selection on every pass is what makes the stream - * follow it, and the provider-side stream is restarted rather than reused — - * the screencast is per target, so a stream left attached to the old one - * would show a page nobody is looking at and never show the new one. - * - * The old target's resources are released by stopping its stream before the - * new one starts: a screencast left running for a tab the person moved away - * from is a leak, and the provider holds it until it is told to let go. - * - * A new *generation* of the same target is not a retarget. The page behind the - * tab was replaced while the tab stayed the one the run selected, so the - * screencast follows it on its own and only the fence below has anything to - * say about it. - * - * A selection that disappeared while the stream ran — the surface's provider - * was stopped, which forgets the surface's selections — stops the stream and - * starts nothing. There is no longer a target this stream may carry. + * A frame that names another page is dropped rather than put on screen, and + * the drop is counted rather than silent: a stream that quietly stops + * updating and a stream that is dropping frames look identical from the + * outside. */ - async #followSelection( - surface: WebuiBrowserSurface, - runId: string, - provider: WebuiBrowserSurfaceProvider, - deliver: (frame: WebuiBrowserImageFrame) => void, - streaming: string | undefined, - ): Promise { - const wanted = this.#selectionOf(surface, runId)?.targetId; - if (wanted === streaming) return streaming; - if (streaming !== undefined) { - await provider.stopImageStream(streaming).catch(() => undefined); - } - if (wanted === undefined) return undefined; - await provider.startImageStream(wanted, deliver); - return wanted; + droppedFrameCount(surface: WebuiBrowserSurface): number { + return this.#droppedFrames.get(surface) ?? 0; } /** - * Whether a frame still describes the page one run is showing. + * Whether a frame still describes the page behind the target. * - * The comparison is against *that run's* current selection, not the one the - * stream started with and not another run's: a run can select another tab - * while the stream is running, another run can select a different tab on the - * same surface, and a frame from either must not appear. + * Two things have to hold. The frame has to name the very target the + * subscriber asked for — all four identity fields, so another tab's frame + * and a previous generation of this one are both refused. And the page + * behind that target has to still be the one the frame is a picture of: the + * registry knows the current generation, and a frame of a page that has + * been replaced is a picture of something the person has already moved away + * from. */ - #frameMatchesSelection( - surface: WebuiBrowserSurface, - runId: string, + #frameMatchesTarget( + target: WebuiBrowserTargetId, frame: WebuiBrowserImageFrame, ): boolean { - const selected = this.#selectionOf(surface, runId); - if (!selected) return false; - return ( - selected.surface === frame.target.surface && - selected.browserId === frame.target.browserId && - selected.targetId === frame.target.targetId && - selected.generation === frame.target.generation - ); + if (!isSameWebuiBrowserTarget(target, frame.target)) return false; + const current = this.#registry.resolve(target); + return current !== undefined && current.id.generation === frame.target.generation; } + /** * Runs one browser command. * diff --git a/packages/webui/src/runtime/harness/browser.ts b/packages/webui/src/runtime/harness/browser.ts index e370cd433..54331b262 100644 --- a/packages/webui/src/runtime/harness/browser.ts +++ b/packages/webui/src/runtime/harness/browser.ts @@ -226,8 +226,8 @@ function createBrowserPort(service: WebuiBrowserService): WebuiBrowserPort { command: string, params?: Record, ) => service.sendCommandToSelection(surface, runId, command, params ?? {}), - watchFrames: (surface: WebuiBrowserSurface, runId?: string, signal?: AbortSignal) => - service.watchFrames(surface, runId, signal), + watchFrames: (surface: WebuiBrowserSurface, target: WebuiBrowserTargetId, signal?: AbortSignal) => + service.watchFrames(surface, target, signal), watchWebTabs: (surface: WebuiBrowserSurface, runId?: string, signal?: AbortSignal) => service.watchWebTabs(surface, runId, signal), listCapabilities: () => service.listCapabilities(), diff --git a/packages/webui/src/runtime/port.ts b/packages/webui/src/runtime/port.ts index 6259b8476..f61baa1db 100644 --- a/packages/webui/src/runtime/port.ts +++ b/packages/webui/src/runtime/port.ts @@ -692,10 +692,17 @@ export interface WebuiBrowserPort { command: string, params?: Record, ): Promise<{ readonly accepted: true; readonly result?: unknown }>; - /** The live image stream of one surface, fenced to one run's selection. */ + /** + * The live image stream of one **target**. + * + * The stream is keyed by the target rather than by the run: the client asks + * for the page it is showing, so two tabs get their own pictures. The run's + * selection still decides what the **Agent's** commands act on — the two are + * different cells and this one does not move the other. + */ watchFrames( surface: WebuiBrowserSurface, - runId?: string, + target: WebuiBrowserTargetId, signal?: AbortSignal, ): AsyncIterable; /** diff --git a/packages/webui/src/server/operation/browser.ts b/packages/webui/src/server/operation/browser.ts index 24c55a5bb..007d830d1 100644 --- a/packages/webui/src/server/operation/browser.ts +++ b/packages/webui/src/server/operation/browser.ts @@ -98,7 +98,7 @@ export const sendBrowserCommandOperation: WebuiOperation export const watchBrowserFramesOperation: WebuiOperation> = { name: WATCH_BROWSER_FRAMES_OPERATION_NAME, - validate: (body) => validateSurface(WATCH_BROWSER_FRAMES_OPERATION_NAME, body), + validate: (body) => validateFrameStream(WATCH_BROWSER_FRAMES_OPERATION_NAME, body), }; /** @@ -230,21 +230,66 @@ function validateTarget( if (!isRecord(target)) { return invalidBody(`${operation} body requires a target`); } + if (!isCompleteTargetIdentity(target)) { + return invalidBody(`${operation} body requires a complete target identity`); + } + return validateRunId(operation, result.body); +} + +/** + * Validates a frame-stream request. + * + * The request names the **target** the client is showing, not a run: the + * stream is per target, so two tabs get their own pictures and the person and + * the Agent can work on different pages at the same time. + * + * A request that still carries a run id is refused with an explicit error + * rather than being answered with the run's selection. A client asking for + * the old shape would otherwise be shown a page it did not ask for, and a + * stream that silently answers with the wrong page is worse than one that + * says the request is out of date. + */ +function validateFrameStream( + operation: string, + body: unknown, +): WebuiOperationValidation> { + const result = requireRecord(operation, body); + if (!result.ok) return result; + if (result.body.runId !== undefined) { + return invalidBody( + `${operation} is keyed by a target, not by a run; send the target the client is showing`, + ); + } + const surface = result.body.surface; if ( - typeof target.surface !== "string" || - !WEBUI_BROWSER_SURFACES.includes(target.surface as WebuiBrowserSurface) || - typeof target.browserId !== "string" || - target.browserId.length === 0 || - typeof target.targetId !== "string" || - target.targetId.length === 0 || - typeof target.generation !== "number" || - !Number.isInteger(target.generation) + typeof surface !== "string" || + !WEBUI_BROWSER_SURFACES.includes(surface as WebuiBrowserSurface) ) { + return invalidBody(`${operation} body requires a known browser surface`); + } + const target = result.body.target; + if (!isRecord(target)) { + return invalidBody(`${operation} body requires a target`); + } + if (!isCompleteTargetIdentity(target)) { return invalidBody(`${operation} body requires a complete target identity`); } - return validateRunId(operation, result.body); + return { ok: true, body: result.body }; } +/** Whether a body carries a target identity with all four of its fields. */ +function isCompleteTargetIdentity(target: Record): boolean { + return ( + typeof target.surface === "string" && + WEBUI_BROWSER_SURFACES.includes(target.surface as WebuiBrowserSurface) && + typeof target.browserId === "string" && + target.browserId.length > 0 && + typeof target.targetId === "string" && + target.targetId.length > 0 && + typeof target.generation === "number" && + Number.isInteger(target.generation) + ); +} /** * Validates the *envelope* of an input command. * diff --git a/packages/webui/src/server/operation/handlers/browser.ts b/packages/webui/src/server/operation/handlers/browser.ts index 5e957a43c..a6014ec77 100644 --- a/packages/webui/src/server/operation/handlers/browser.ts +++ b/packages/webui/src/server/operation/handlers/browser.ts @@ -136,7 +136,7 @@ export function createBrowserHandlerEntries( // cast is the same one the terminal watcher makes. source: browser.watchFrames( readBrowserSurface(body as Record), - readBrowserRunId(body as Record), + readBrowserTarget(body as Record), context.signal, ) as unknown as AsyncIterable>, }, diff --git a/packages/webui/src/shared/operation-names.ts b/packages/webui/src/shared/operation-names.ts index 9cfeb3b61..932248855 100644 --- a/packages/webui/src/shared/operation-names.ts +++ b/packages/webui/src/shared/operation-names.ts @@ -465,7 +465,10 @@ export interface OperationSpec { response: { readonly accepted: true; readonly result?: unknown }; }; watchBrowserFrames: { - request: { readonly surface: WebuiBrowserSurface; readonly runId?: string }; + request: { + readonly surface: WebuiBrowserSurface; + readonly target: WebuiBrowserTargetId; + }; response: WebuiBrowserFrameStreamEnvelope; }; watchBrowserWebTabs: { diff --git a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts index 4146f1969..e3ab6c8e2 100644 --- a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts +++ b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts @@ -27,7 +27,9 @@ import { join } from "node:path"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { WebuiIsolatedChromiumProvider } from "../../src/runtime/browser/isolated-chromium/provider.js"; +import { WebuiBrowserService } from "../../src/runtime/browser/browser-service.js"; import type { + WebuiBrowserAuthorization, WebuiBrowserImageFrame, WebuiBrowserTargetId, } from "../../src/shared/contracts/browser.js"; @@ -305,3 +307,118 @@ describe.skipIf(!available)("the isolated Chromium, one page per web tab", () => await provider.stopImageStream(second.targetId); }); }); + +/** + * The service, streaming two pages of one real Chromium. + * + * The provider-level suite above proves the pages are independent. This one + * proves the *streams* are: the client asks for the page it is showing, and + * two streams open at once must not cross. + */ +describe.skipIf(!available)("the browser service, streaming two pages of one Chromium", () => { + let dataDir: string; + let service: WebuiBrowserService; + + const authorization: WebuiBrowserAuthorization = { + isolatedChromium: true, + edge: false, + chrome: false, + // The raw DevTools method below is how the test repaints a page, and the + // Full CDP switch is what allows a raw method through. + fullCdp: true, + }; + + beforeAll(async () => { + dataDir = await mkdtemp(join(tmpdir(), "webui-two-page-streams-")); + service = new WebuiBrowserService({ + authorization: { + read: async () => authorization, + write: async () => authorization, + }, + dataDir, + }); + }, 60_000); + + afterAll(async () => { + await service?.close(); + if (dataDir) await rm(dataDir, { recursive: true, force: true }); + }); + + /** Repaints the page the run is on, which is what makes the screencast push. */ + async function repaint(): Promise { + await service.sendCommandToSelection("isolated-chromium", "run-1", "Runtime.evaluate", { + expression: "document.body.style.background = 'rgb(12, 34, 56)'", + }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + } + + it("gives two tabs their own pictures, and neither receives the other's frames", async () => { + const list = await service.listTargets("isolated-chromium", "run-1"); + // The service marks the *run's* selection, and the run has not selected + // anything yet, so the page to start from is found by asking the provider + // which of the listed pages it can actually drive: the browser opens a + // page of its own that the provider never attached to. + let first: WebuiBrowserTargetId | undefined; + for (const candidate of list.targets) { + await service.selectTarget(candidate.id, "run-1"); + try { + await service.sendCommandToSelection("isolated-chromium", "run-1", "Runtime.evaluate", { + expression: "1", + }); + first = candidate.id; + break; + } catch { + // This page is not one the provider drives; try the next. + } + } + expect(first).toBeDefined(); + + // The run opens a second tab, so the session owns two pages. + const opened = await service.sendCommandToSelection( + "isolated-chromium", + "run-1", + "open_tab", + { url: pageUrl("second page") }, + ); + const second = opened.result as WebuiBrowserTargetId; + expect(second.targetId).not.toBe(first!.targetId); + + const firstFrames: WebuiBrowserImageFrame[] = []; + const secondFrames: WebuiBrowserImageFrame[] = []; + const controller = new AbortController(); + const firstStream = (async () => { + for await (const frame of service.watchFrames( + "isolated-chromium", + first!, + controller.signal, + )) { + firstFrames.push(frame); + } + })(); + const secondStream = (async () => { + for await (const frame of service.watchFrames( + "isolated-chromium", + second, + controller.signal, + )) { + secondFrames.push(frame); + } + })(); + + // Chrome renders the page in front, so each page is repainted while its + // own stream is the one watching. + await repaint(); + await service.selectTarget(second, "run-1"); + await repaint(); + + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + controller.abort(); + await Promise.allSettled([firstStream, secondStream]); + + expect(firstFrames.length).toBeGreaterThan(0); + expect(secondFrames.length).toBeGreaterThan(0); + // Neither stream received the other page's picture. + expect(firstFrames.every((frame) => frame.target.targetId === first!.targetId)).toBe(true); + expect(secondFrames.every((frame) => frame.target.targetId === second.targetId)).toBe(true); + }); +}); diff --git a/packages/webui/test/unit/browser-frame-stream.test.ts b/packages/webui/test/unit/browser-frame-stream.test.ts new file mode 100644 index 000000000..c8ce154eb --- /dev/null +++ b/packages/webui/test/unit/browser-frame-stream.test.ts @@ -0,0 +1,67 @@ +// The frame stream's request shape. +// +// The image stream is keyed by the **target** rather than by the run, so two +// tabs get their own pictures and the person and the Agent can work on +// different pages at the same time. That is a change to an existing +// operation's request shape, not a second operation beside it. +// +// A client that still sends the old shape is refused with an explicit error +// rather than being answered with the run's selection: a stream that +// silently answers with the wrong page is worse than one that says the +// request is out of date. + +import { describe, expect, it } from "vitest"; + +import { watchBrowserFramesOperation } from "../../src/server/operation/browser.js"; + +const TARGET = { + surface: "isolated-chromium", + browserId: "browser-1", + targetId: "tab-1", + generation: 1, +}; + +describe("the frame stream's request", () => { + it("accepts a request that names a target", () => { + const result = watchBrowserFramesOperation.validate({ + surface: "isolated-chromium", + target: TARGET, + }); + expect(result.ok).toBe(true); + }); + + it("refuses a request that still carries a run id", () => { + const result = watchBrowserFramesOperation.validate({ + surface: "isolated-chromium", + runId: "run-1", + }); + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("keyed by a target"); + }); + + it("refuses a request with no target at all", () => { + const result = watchBrowserFramesOperation.validate({ + surface: "isolated-chromium", + }); + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("requires a target"); + }); + + it("refuses a request whose target is incomplete", () => { + const result = watchBrowserFramesOperation.validate({ + surface: "isolated-chromium", + target: { surface: "isolated-chromium", targetId: "tab-1" }, + }); + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("complete target identity"); + }); + + it("refuses a request for a surface it does not know", () => { + const result = watchBrowserFramesOperation.validate({ + surface: "netscape", + target: TARGET, + }); + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("known browser surface"); + }); +}); diff --git a/packages/webui/test/unit/browser-service.test.ts b/packages/webui/test/unit/browser-service.test.ts index 97901da75..d9bce3f66 100644 --- a/packages/webui/test/unit/browser-service.test.ts +++ b/packages/webui/test/unit/browser-service.test.ts @@ -695,7 +695,7 @@ describe("browser service", () => { const controller = new AbortController(); const received: WebuiBrowserImageFrame[] = []; - const stream = service.watchFrames("isolated-chromium", "run-panel", controller.signal); + const stream = service.watchFrames("isolated-chromium", tab("tab-1"), controller.signal); const consumer = (async () => { for await (const frame of stream) { received.push(frame); @@ -723,7 +723,7 @@ describe("browser service", () => { const controller = new AbortController(); const received: WebuiBrowserImageFrame[] = []; - const stream = service.watchFrames("isolated-chromium", "run-panel", controller.signal); + const stream = service.watchFrames("isolated-chromium", tab("tab-1"), controller.signal); const consumer = (async () => { for await (const frame of stream) { received.push(frame); @@ -731,16 +731,23 @@ describe("browser service", () => { } })(); + // The stream is established before the page is replaced: a stream opened + // for a page that has already been replaced is refused outright, because + // the picture it would carry is of a page that is gone. + await waitForStreams(provider, 1); + // The provider announces that the page behind the tab was replaced. A frame // that arrives afterwards still names the previous page, and the panel has // already moved past it, so it must not reach the screen. provider.announceTargetChange(2); - await pushWhenSubscribed(provider, 1); - await new Promise((resolve) => setTimeout(resolve, 50)); + provider.pushFrame(1, "tab-1"); + await settleFrames(); + controller.abort(); await consumer.catch(() => undefined); expect(received).toHaveLength(0); + expect(service.droppedFrameCount("isolated-chromium")).toBe(1); await service.close(); }); }); @@ -814,15 +821,15 @@ describe("run-scoped selection", () => { await service.close(); }); - it("gives each run's stream only that run's target's frames", async () => { + it("gives each target's stream only that target's frames", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); await service.listTargets("isolated-chromium", "run-b"); await service.selectTarget(tab("tab-1"), "run-a"); await service.selectTarget(tab("tab-2"), "run-b"); - const first = collectFrames(service, "run-a"); - const second = collectFrames(service, "run-b"); + const first = collectFrames(service, tab("tab-1")); + const second = collectFrames(service, tab("tab-2")); await waitForStreams(provider, 2); provider.pushFrame(1, "tab-1"); @@ -834,20 +841,21 @@ describe("run-scoped selection", () => { await stop(service, [first, second]); }); - it("drops a frame that belongs to another run's target", async () => { + it("drops a frame that belongs to another target", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); await service.listTargets("isolated-chromium", "run-b"); await service.selectTarget(tab("tab-1"), "run-a"); await service.selectTarget(tab("tab-2"), "run-b"); - const first = collectFrames(service, "run-a"); - const second = collectFrames(service, "run-b"); + const first = collectFrames(service, tab("tab-1")); + const second = collectFrames(service, tab("tab-2")); await waitForStreams(provider, 2); - // The provider hands run A's stream a frame of run B's tab. Keeping the - // streams apart is the service's job, not the provider's, so the fence is - // against this run's own selection and the foreign frame is dropped. + // The provider hands the first tab's stream a frame of the second tab. + // Keeping the streams apart is the service's job as well as the + // provider's, so the fence is against the target the subscriber asked for + // and the foreign frame is dropped. provider.frameHandlers.get("tab-1")?.({ target: { surface: "isolated-chromium", @@ -897,20 +905,19 @@ describe("run-scoped selection", () => { * the person left is a panel that never shows the tab they moved to), and one * run's retarget is one run's business. */ -describe("stream retargeting", () => { +describe("the stream of one target", () => { const TWO_TABS: WebuiBrowserAuthorization = { ...ALL_OFF, isolatedChromium: true, }; - it("refuses a stream for a run that has selected nothing", async () => { + it("refuses a stream for a target that does not exist", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); - // Another run's selection is not this one's to stream, and streaming - // nothing would leave the panel watching a page it cannot see. - await service.selectTarget(tab("tab-1"), "run-b"); - await expect(drainStream(service, "run-a")).rejects.toMatchObject({ + // A stream for a target nobody has is a stream that can never deliver a + // frame, and a panel waiting for one looks like a panel that is broken. + await expect(drainStream(service, tab("tab-9"))).rejects.toMatchObject({ code: "browser_target_unknown", }); // The refusal is the whole outcome: nothing was started on the provider. @@ -918,105 +925,107 @@ describe("stream retargeting", () => { await service.close(); }); - it("starts the stream for the target the run selected at the time", async () => { + it("refuses a stream for a target whose page was replaced", async () => { + const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); + await service.listTargets("isolated-chromium", "run-a"); + provider.announceTargetChange(2, "tab-1"); + + await expect(drainStream(service, tab("tab-1"))).rejects.toMatchObject({ + code: "browser_target_stale", + }); + expect(provider.startedStreams).toHaveLength(0); + await service.close(); + }); + + it("starts the stream for the target the request names", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); - await service.selectTarget(tab("tab-2"), "run-a"); - const stream = collectFrames(service, "run-a"); + const stream = collectFrames(service, tab("tab-2")); await waitUntil(() => provider.startedStreams.includes("tab-2")); await stop(service, [stream]); expect(provider.startedStreams).toEqual(["tab-2"]); }); - it("moves the stream onto the target the run selected next and stops the old one", async () => { + it("gives two tabs their own pictures, and neither receives the other's frames", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); - await service.selectTarget(tab("tab-1"), "run-a"); - const stream = collectFrames(service, "run-a"); - await waitUntil(() => provider.startedStreams.includes("tab-1")); - const oldHandler = provider.frameHandlers.get("tab-1"); + const first = collectFrames(service, tab("tab-1")); + const second = collectFrames(service, tab("tab-2")); + await waitUntil(() => provider.startedStreams.length >= 2); + expect(new Set(provider.startedStreams)).toEqual(new Set(["tab-1", "tab-2"])); - // The person points at another tab while the panel is watching this one. - await service.selectTarget(tab("tab-2"), "run-a"); - await waitUntil(() => provider.startedStreams.includes("tab-2")); + provider.pushFrame(1, "tab-1"); + provider.pushFrame(1, "tab-2"); + await settleFrames(); - // The screencast for the tab they moved away from is released rather than - // left running: a stream left attached to it would hold the browser's - // resources for a page nobody is looking at. - expect(provider.stoppedStreams).toEqual(["tab-1"]); + expect(first.frames.map((frame) => frame.target.targetId)).toEqual(["tab-1"]); + expect(second.frames.map((frame) => frame.target.targetId)).toEqual(["tab-2"]); + await stop(service, [first, second]); + }); - // A frame of the tab the run is on now arrives on the same stream. - provider.pushFrame(1, "tab-2"); - // A frame that arrives from the old tab afterwards is dropped by the fence: - // it names a page this run is no longer showing. The handler is the one - // captured before the retarget, so the drop is the fence's doing and not - // the provider's. - oldHandler?.({ - target: tab("tab-1"), + it("drops a frame that names a target the subscriber did not ask for, and counts the drop", async () => { + const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); + await service.listTargets("isolated-chromium", "run-a"); + + const stream = collectFrames(service, tab("tab-1")); + await waitUntil(() => provider.startedStreams.includes("tab-1")); + + // The provider hands a frame to the page that asked for it, so the frame + // is delivered to this subscriber's own handler directly: the fence is + // what decides, and it has to be shown deciding on a frame that names + // another page. The drop is counted, because a stream that quietly stops + // updating and one that is dropping frames look the same from outside. + const handler = provider.frameHandlers.get("tab-1"); + handler?.({ + target: tab("tab-2"), data: "QlRSQVk=", format: "jpeg", - sequence: 2, + sequence: 1, }); await settleFrames(); - expect(stream.frames.map((frame) => frame.target.targetId)).toEqual(["tab-2"]); + expect(stream.frames).toHaveLength(0); + expect(service.droppedFrameCount("isolated-chromium")).toBe(1); await stop(service, [stream]); }); - it("keeps another run's stream when one run retargets", async () => { - const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2", "tab-3"]); + it("refuses a frame from a superseded generation", async () => { + const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); - await service.listTargets("isolated-chromium", "run-b"); - await service.selectTarget(tab("tab-1"), "run-a"); - await service.selectTarget(tab("tab-2"), "run-b"); - - const first = collectFrames(service, "run-a"); - const second = collectFrames(service, "run-b"); - await waitUntil(() => provider.startedStreams.length >= 2); - expect(new Set(provider.startedStreams)).toEqual(new Set(["tab-1", "tab-2"])); - await service.selectTarget(tab("tab-3"), "run-a"); - await waitUntil(() => provider.startedStreams.includes("tab-3")); - - // Run B's screencast belongs to run B. Stopping it because run A moved - // would leave another run's panel without a stream at all. - expect(provider.stoppedStreams).toEqual(["tab-1"]); + const stream = collectFrames(service, tab("tab-1")); + await waitUntil(() => provider.startedStreams.includes("tab-1")); - provider.pushFrame(1, "tab-3"); - provider.pushFrame(1, "tab-2"); + // The page behind the tab was replaced, so the generation moved. The + // frame that arrives now carries the new generation, and the subscriber + // asked for the old one: a picture of a page that is gone. + provider.announceTargetChange(2, "tab-1"); + provider.pushFrame(2, "tab-1"); await settleFrames(); - expect(first.frames.map((frame) => frame.target.targetId)).toEqual(["tab-3"]); - expect(second.frames.map((frame) => frame.target.targetId)).toEqual(["tab-2"]); - await stop(service, [first, second]); + expect(stream.frames).toHaveLength(0); + expect(service.droppedFrameCount("isolated-chromium")).toBe(1); + await stop(service, [stream]); }); - it("keeps the stream on the tab whose page navigated", async () => { + it("leaves the run's selection deciding what the Agent's commands act on", async () => { const { service, provider } = createService(TWO_TABS, ["tab-1", "tab-2"]); await service.listTargets("isolated-chromium", "run-a"); - await service.selectTarget(tab("tab-1"), "run-a"); + await service.selectTarget(tab("tab-2"), "run-a"); - const stream = collectFrames(service, "run-a"); + // The stream is keyed by the target the client asked for, and the command + // path is keyed by the run. They are different cells: the panel shows + // tab-1, and the Agent still acts on tab-2. + const stream = collectFrames(service, tab("tab-1")); await waitUntil(() => provider.startedStreams.includes("tab-1")); - // The tab is still the one the run selected; only the page behind it was - // replaced. The screencast follows the tab on its own, so nothing is - // restarted — and the new page's frame is what the panel then shows. - provider.announceTargetChange(2); - // The stream loop re-reads the selection once per pass, so this covers a - // whole pass before the restart counters are read. - await new Promise((resolve) => setTimeout(resolve, 300)); - expect(provider.startedStreams).toEqual(["tab-1"]); - expect(provider.stoppedStreams).toEqual([]); - - provider.pushFrame(2, "tab-1"); - await settleFrames(); - - expect(stream.frames.map((frame) => frame.target.generation)).toEqual([2]); + await service.sendCommandToSelection("isolated-chromium", "run-a", "reload", {}); + expect(provider.commands.map((entry) => entry.command)).toEqual(["reload"]); await stop(service, [stream]); + await service.close(); }); }); @@ -1067,14 +1076,17 @@ interface OpenStream { readonly frames: WebuiBrowserImageFrame[]; } -/** Starts one run's stream and drains it in the background. */ -function collectFrames(service: WebuiBrowserService, runId: string): OpenStream { +/** Starts one target's stream and drains it in the background. */ +function collectFrames( + service: WebuiBrowserService, + target: WebuiBrowserTargetId = tab("tab-1"), +): OpenStream { const controller = new AbortController(); const stream: OpenStream = { controller, frames: [] }; void (async () => { for await (const frame of service.watchFrames( "isolated-chromium", - runId, + target, controller.signal, )) { stream.frames.push(frame); @@ -1113,13 +1125,16 @@ async function waitUntil(condition: () => boolean): Promise { * Reads one run's stream, so a refusal on the way in is a rejection the caller * can read: an async generator that is never iterated never runs at all. */ -function drainStream(service: WebuiBrowserService, runId: string): Promise { +function drainStream( + service: WebuiBrowserService, + target: WebuiBrowserTargetId = tab("tab-1"), +): Promise { const controller = new AbortController(); return (async () => { try { for await (const _frame of service.watchFrames( "isolated-chromium", - runId, + target, controller.signal, )) { return; diff --git a/release/public-source.json b/release/public-source.json index 36d63af72..4f45582a8 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -4243,6 +4243,7 @@ "packages/webui/test/unit/browser-bridge-server.test.ts", "packages/webui/test/unit/browser-bridge-session.test.ts", "packages/webui/test/unit/browser-bridge-upgrade.test.ts", + "packages/webui/test/unit/browser-frame-stream.test.ts", "packages/webui/test/unit/browser-input-command.test.ts", "packages/webui/test/unit/browser-panel-state.test.ts", "packages/webui/test/unit/browser-panel-view.test.tsx", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index f31ed96b7..505a09bea 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -449,6 +449,7 @@ "packages/webui/test/unit/browser-workspace-panel.test.tsx", "packages/webui/test/unit/browser-tool-adapter.test.ts", "packages/webui/test/unit/browser-web-tabs-operation.test.ts", + "packages/webui/test/unit/browser-frame-stream.test.ts", "packages/webui/test/unit/browser-bridge-cdp.test.ts", "packages/webui/test/unit/browser-bridge-adversarial.test.ts", "packages/webui/test/unit/browser-bridge-credential-secrecy.test.ts" From 525bb8a5694a7e09443f02e3d4a2df499357d08a Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 21:06:22 +0800 Subject: [PATCH 07/34] feat(webui): a session's pages are released when the session ends Issue #205. There was no per-session lifecycle signal to hang the release on, and a session is not only removed by deletion: archiving it removes one, and so does a transfer away. A release that covered only deletion would leave a page behind for every archived session, and a page in the isolated Chromium is a live process with a live target. The release is keyed on the session, and the edge it needed is the composition root's: the runtime has no per-session signal, and the composition root is the only place that holds both the harness port and the browser service, so it is the place that can wire the two together. The harness port's deleteSession and archiveSession now release the session, and the transfer's rollback reaches the same deleteSession, so all three ways a session goes away leave the same nothing behind. The release closes the page, stops its screencast, forgets the target in the registry and takes the run's selection with it. A target the registry still offers is a target a command could still be aimed at, and a screencast left running for a page that is gone is a resource the browser holds for nobody. A session that is still open keeps its pages: an explicit success of false means the session is still there, and releasing it would close a page out from under a running session. The new suite is the evidence for all three paths, and for the two things that must not happen: another session's pages are not closed because this one ended, and a subscriber receives no frame for a page that no longer exists. --- .../docs/browser-use/01-capability-matrix.md | 1 + .../src/runtime/browser/browser-service.ts | 54 ++++ .../src/runtime/browser/target-registry.ts | Bin 6764 -> 7409 bytes packages/webui/src/runtime/harness/adapter.ts | 21 +- packages/webui/src/runtime/harness/browser.ts | 14 +- .../webui/src/runtime/harness/sessions.ts | 45 ++- .../test/unit/browser-session-release.test.ts | 269 ++++++++++++++++++ release/public-source.json | 1 + release/webui-npm/bin/mcode-webui.mjs | 8 +- scripts/run-webui-server.mjs | 8 +- test/vitest-suites.json | 1 + 11 files changed, 416 insertions(+), 6 deletions(-) create mode 100644 packages/webui/test/unit/browser-session-release.test.ts diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index ebb22da4e..a01450d9f 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -381,6 +381,7 @@ that closes it. It is a gap, not a denial. | The identity `open_tab` returns names the page it created | **implemented** | the answer carries the surface, the browser id, the target id and the generation the provider's own accounting gives the page. `packages/webui/src/runtime/browser/isolated-chromium/provider.ts` — the `open_tab` case of `sendCommand` | | The service observes a new page from the identity `open_tab` returns | **implemented** | the service reads the answer and observes that target in the registry, so the identity is usable at once — it does not have to wait for the provider to be asked for its list again. `packages/webui/src/runtime/browser/browser-service.ts` — `#observeOpenedTarget` | | The image stream is keyed by target | **implemented** | the client asks for the target it is showing; the service's frame subscribers are keyed by target, one screencast per page, and a frame that names another page is dropped and counted. `packages/webui/src/runtime/browser/browser-service.ts` — `watchFrames` | +| A session's pages are released when the session ends | **implemented** | the release is keyed on the **session**, not on one operation. The three ways a session goes away — deletion, archiving, and a transfer away — all reach it: the first two through the harness port's own `deleteSession` and `archiveSession`, the third through the transfer's rollback, which calls the same `deleteSession`. `packages/webui/src/runtime/harness/sessions.ts` — `releaseSessionIfItEnded`; `packages/webui/src/runtime/browser/browser-service.ts` — `releaseSession` | | The runtime publishes the session's web tabs as a snapshot | **implemented** | the service keeps the session's web tabs beside the run's selection and publishes them on the `watchBrowserWebTabs` stream — each tab's id, its target identity, its title, its URL, and whether it is the run's selection. `packages/webui/src/runtime/browser/browser-service.ts` — `watchWebTabs` | | The client subscribes to the published snapshot | **implemented** | the browser port carries `watchBrowserWebTabs`, the transport's request table has it, and the wire opens the stream. The strip renders from it in #208. `packages/webui/src/client/features/browser/contracts/browser-port.ts` | | The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | diff --git a/packages/webui/src/runtime/browser/browser-service.ts b/packages/webui/src/runtime/browser/browser-service.ts index 5a75765d5..a5350d205 100644 --- a/packages/webui/src/runtime/browser/browser-service.ts +++ b/packages/webui/src/runtime/browser/browser-service.ts @@ -908,6 +908,60 @@ export class WebuiBrowserService { } } + /** + * Releases a session's pages. + * + * A session's pages are released when the session ends — deleted, archived, + * or transferred away. The release closes the page, drops the target from + * the registry, and takes the run's selection with it: a page in the + * isolated Chromium is a live process with a live target, and a target the + * registry still offers is a target a command could still be aimed at. + * + * The release is keyed on the **session**, not on the operation that ended + * it, so a session that goes away in any of the three ways leaves the same + * nothing behind. + */ + async releaseSession(sessionId: string): Promise { + for (const [surface, runs] of [...this.#webTabs]) { + const tabs = runs.get(sessionId); + if (!tabs) continue; + for (const target of [...tabs.values()]) { + await this.#releaseTarget(surface, target); + } + runs.delete(sessionId); + } + // The run's selection goes with the session: a selection that names a page + // of a session that no longer exists is a selection nothing can act on. + for (const selections of this.#selected.values()) { + selections.delete(sessionId); + } + } + + /** Closes one page and forgets its target. */ + async #releaseTarget( + surface: WebuiBrowserSurface, + target: WebuiBrowserTargetId, + ): Promise { + const provider = this.#providers.get(surface); + // The stream stops first: a screencast left running for a page that is + // gone is a resource the browser holds for nobody. + await provider?.stopImageStream(target.targetId).catch(() => undefined); + if (provider) { + try { + await provider.sendCommand(target, "Target.closeTarget", { + targetId: target.targetId, + }); + } catch { + // A page that is already gone needs no closing. + } + } + this.#registry.forget({ + surface: target.surface, + browserId: target.browserId, + targetId: target.targetId, + }); + } + /** * How many frames this service has dropped on a surface. * diff --git a/packages/webui/src/runtime/browser/target-registry.ts b/packages/webui/src/runtime/browser/target-registry.ts index ef756b2d6586d243f610b380f41eb05a92d73490..706d0ac3c3b96b5e01b410b47f25894c46f81d77 100644 GIT binary patch delta 466 zcmY*WF-`+95bP*Ofgly48Hom==xAs_)VzQP#GE&d70VlW4Iwn4q(2@KiT_X_$~5#8 ztnV&CNY-g@cXm88-duiPtvAEfK7e!Fa&nXf90}Q!8Wgin6qZ$s=wRbIu+SKZYED*= zrzXQ-ZXCf2Tv<2KVn#GCQz|TJIfAgl5=|&ktf?bwNmiCDj59t;hR>)?gk&EA?sfTT z9&|#s!rfH)^%PFlPtwXqcU>ACosfvDk+M9zyQfC(qiBWd+dCmm}~L(&wwoG;Xj rV)X%<^}%!Vb2vEB+oU=zL3)zPQlLy{#WT=+3~m~KG void | Promise; +} + export function createHarnessPortFromHost( handle: WebuiRuntimeHostHandle, + options: CreateHarnessPortOptions = {}, ): WebuiHarnessPort { const host = handle; // Filled in below, once the port exists: the manifest is a fact about what was @@ -45,7 +64,7 @@ export function createHarnessPortFromHost( async invalidateAuth() { host.invalidateAuth?.(); }, - ...createSessionsAdapter(host), + ...createSessionsAdapter(host, options), ...createExecutionAdapter(host), ...createInteractionsAdapter(host), ...createWorkspaceAdapter(host), diff --git a/packages/webui/src/runtime/harness/browser.ts b/packages/webui/src/runtime/harness/browser.ts index 54331b262..f678bee3d 100644 --- a/packages/webui/src/runtime/harness/browser.ts +++ b/packages/webui/src/runtime/harness/browser.ts @@ -134,8 +134,19 @@ export interface WebuiBrowserAdapterBinding { * not know which switches are on. */ initialize(): Promise; - /** Releases every provider the browser service started. */ + /** + * Releases every provider the browser service started. + */ close(): Promise; + /** + * Releases one session's pages. + * + * A session's pages are released when the session ends — deleted, archived + * or transferred away. The composition root wires this to the port's + * session-ending operations, so the release is keyed on the session rather + * than on whichever operation happened to end it. + */ + releaseSession(sessionId: string): Promise; } /** What the adapter needs before it has a host. */ @@ -201,6 +212,7 @@ export function createBrowserAdapter( }, initialize: () => service.initialize(), close: () => service.close(), + releaseSession: (sessionId: string) => service.releaseSession(sessionId), }; } diff --git a/packages/webui/src/runtime/harness/sessions.ts b/packages/webui/src/runtime/harness/sessions.ts index 0b293bc95..bd53d4311 100644 --- a/packages/webui/src/runtime/harness/sessions.ts +++ b/packages/webui/src/runtime/harness/sessions.ts @@ -18,8 +18,45 @@ import { toRuntimeAttachmentRequest, } from "./result-conversion.js"; +/** + * What the sessions adapter needs beyond the host. + * + * The release hook is the edge a session's end needs, and it does not exist + * in the runtime yet: a session is removed by deletion, by archiving, and by + * a transfer away, and the three reach this adapter differently. The release + * is therefore wired to the **session** rather than to one operation, so + * whichever way a session goes away, the same nothing is left behind. + */ +export interface CreateSessionsAdapterOptions { + /** + * Called when a session has ended. + * + * The composition root hands this to the browser service, so a session that + * ends leaves no page behind in the isolated Chromium. + */ + readonly onSessionEnded?: (sessionId: string) => void | Promise; +} + +/** + * Releases a session's resources, when the session really ended. + * + * An explicit `success: false` means the session is still there, and a + * session that is still open keeps its pages — releasing them would close a + * page out from under a session that is still running. + */ +async function releaseSessionIfItEnded( + options: CreateSessionsAdapterOptions, + sessionId: string, + result: { readonly success?: boolean } | undefined, +): Promise { + if (!options.onSessionEnded) return; + if (result?.success === false) return; + await options.onSessionEnded(sessionId); +} + export function createSessionsAdapter( host: WebuiRuntimeHostHandle, + options: CreateSessionsAdapterOptions = {}, ): Pick< WebuiHarnessPort, | "listSessions" @@ -70,11 +107,15 @@ export function createSessionsAdapter( }, async archiveSession(request) { - return requireCliService(host).archiveSession(request, {}); + const result = await requireCliService(host).archiveSession(request, {}); + await releaseSessionIfItEnded(options, request.id, result); + return result; }, async deleteSession(request) { - return requireCliService(host).deleteSession(request, {}); + const result = await requireCliService(host).deleteSession(request, {}); + await releaseSessionIfItEnded(options, request.id, result); + return result; }, async updateSession(request) { diff --git a/packages/webui/test/unit/browser-session-release.test.ts b/packages/webui/test/unit/browser-session-release.test.ts new file mode 100644 index 000000000..5c8aaf0cb --- /dev/null +++ b/packages/webui/test/unit/browser-session-release.test.ts @@ -0,0 +1,269 @@ +// A session's pages are released when the session ends. +// +// There is no per-session lifecycle signal in the runtime to hang this on: a +// session is removed by deletion, by archiving, and by a transfer away, and +// the three reach the runtime differently. The release is therefore keyed on +// the **session**, and this file is the evidence that all three paths reach +// it — and that a session that is still open keeps its pages. +// +// A page in the isolated Chromium is a live process with a live target, so a +// session that ends and leaves a page behind leaves a browser nobody can +// switch off. + +import { describe, expect, it } from "vitest"; + +import { WebuiBrowserService } from "../../src/runtime/browser/browser-service.js"; +import { createHarnessPortFromHost } from "../../src/runtime/harness/adapter.js"; +import { importWebuiSessionTransfer } from "../../src/runtime/session-transfer.js"; +import type { WebuiBrowserSurfaceProvider } from "../../src/runtime/browser/isolated-chromium/provider.js"; +import type { + WebuiBrowserAuthorization, + WebuiBrowserImageFrame, + WebuiBrowserTargetId, + WebuiBrowserTargetState, +} from "../../src/shared/contracts/browser.js"; + +const SESSION = "session-1"; +const OTHER = "session-2"; + +/** A provider that records what it was asked to do. */ +class RecordingProvider implements WebuiBrowserSurfaceProvider { + readonly surface = "isolated-chromium" as const; + readonly browserId = "browser-1"; + readonly commands: { command: string; params: Record }[] = []; + readonly stoppedStreams: string[] = []; + /** The pages this browser has, in the order they were opened. */ + readonly openedTabs: string[] = []; + private readonly frameHandlers = new Map void>(); + + async listTargets(): Promise { + return ["tab-1", ...this.openedTabs].map((targetId) => ({ + id: { + surface: this.surface, + browserId: this.browserId, + targetId, + generation: 1, + }, + title: `Tab ${targetId}`, + url: `https://example.test/${targetId}`, + status: "open" as const, + selected: false, + })); + } + + async sendCommand( + _target: unknown, + command: string, + params: Record, + ): Promise { + this.commands.push({ command, params }); + if (command !== "open_tab") return undefined; + // A command that opened a page answers with that page's identity, the way + // the isolated provider answers: the surface, the browser, the target and + // the generation the provider's own accounting gives it. Every page gets + // its own target id, because a target id names one page. + const targetId = `tab-${this.openedTabs.length + 2}`; + this.openedTabs.push(targetId); + return { + surface: this.surface, + browserId: this.browserId, + targetId, + generation: 1, + } satisfies WebuiBrowserTargetId; + } + + async sendInput(): Promise {} + + async startImageStream( + targetId: string, + onFrame: (frame: WebuiBrowserImageFrame) => void, + ): Promise { + this.frameHandlers.set(targetId, onFrame); + } + + async stopImageStream(targetId: string): Promise { + this.stoppedStreams.push(targetId); + this.frameHandlers.delete(targetId); + } + + onTargetChanged(): () => void { + return () => undefined; + } + + async close(): Promise {} +} + +function tab(targetId: string, generation = 1): WebuiBrowserTargetId { + return { + surface: "isolated-chromium", + browserId: "browser-1", + targetId, + generation, + }; +} + +/** + * A browser service, a harness port, and the edge that joins them. + * + * The edge is the composition root's: it is the only place that holds both + * the harness port and the browser service, so it is the place that can wire + * a session's end to the release of that session's pages. + */ +function createReleaseHarness(): { + service: WebuiBrowserService; + provider: RecordingProvider; + port: ReturnType; +} { + const provider = new RecordingProvider(); + const authorization: WebuiBrowserAuthorization = { + isolatedChromium: true, + edge: false, + chrome: false, + fullCdp: false, + }; + const service = new WebuiBrowserService({ + authorization: { + read: async () => authorization, + write: async () => authorization, + }, + dataDir: "/tmp/webui-session-release-test", + createIsolatedProvider: async () => provider, + }); + const port = createHarnessPortFromHost( + { + apiHost: { close: async () => undefined }, + cliService: { + archiveSession: async () => ({ success: true }), + deleteSession: async () => ({ success: true }), + createSession: async () => ({ sessionId: "imported-session" }), + getBrowserAuthorization: async () => authorization, + setBrowserAuthorization: async () => authorization, + } as never, + }, + { onSessionEnded: (sessionId) => service.releaseSession(sessionId) }, + ); + return { service, provider, port }; +} + +/** Gives one session a page of its own and selects it. */ +async function openPageFor( + service: WebuiBrowserService, + sessionId: string, +): Promise { + await service.listTargets("isolated-chromium", sessionId); + await service.selectTarget(tab("tab-1"), sessionId); + const opened = await service.sendCommandToSelection( + "isolated-chromium", + sessionId, + "open_tab", + { url: "https://example.test/opened" }, + ); + return opened.result as WebuiBrowserTargetId; +} + +describe("a session's pages are released when the session ends", () => { + it("releases a deleted session's page, target and selection", async () => { + const { service, provider, port } = createReleaseHarness(); + const opened = await openPageFor(service, SESSION); + + await port.deleteSession({ id: SESSION }); + + // The page is closed rather than left running in the browser. + expect( + provider.commands.some((entry) => entry.command === "Target.closeTarget"), + ).toBe(true); + // The target is gone from the registry, so it can no longer be selected. + await expect(service.selectTarget(opened, SESSION)).rejects.toMatchObject({ + code: "browser_target_unknown", + }); + // And the run's selection went with it. + const list = await service.listTargets("isolated-chromium", SESSION); + expect(list.targets.some((entry) => entry.selected)).toBe(false); + await service.close(); + }); + + it("releases an archived session's page, target and selection", async () => { + const { service, provider, port } = createReleaseHarness(); + const opened = await openPageFor(service, SESSION); + + await port.archiveSession({ id: SESSION }); + + expect( + provider.commands.some((entry) => entry.command === "Target.closeTarget"), + ).toBe(true); + await expect(service.selectTarget(opened, SESSION)).rejects.toMatchObject({ + code: "browser_target_unknown", + }); + const list = await service.listTargets("isolated-chromium", SESSION); + expect(list.targets.some((entry) => entry.selected)).toBe(false); + await service.close(); + }); + + it("releases a session that was transferred away", async () => { + const { service, provider, port } = createReleaseHarness(); + await openPageFor(service, "imported-session"); + + // The import fails, so the runtime removes the session it created. That + // removal is the same `deleteSession` the release is wired to, so the + // session's pages go with it. + const outcome = await importWebuiSessionTransfer(port, { + file: { session: { sessionId: "source-session" } }, + agentName: "imported", + }); + expect(outcome.kind).toBe("import-failed"); + + expect( + provider.commands.some((entry) => entry.command === "Target.closeTarget"), + ).toBe(true); + await service.close(); + }); + + it("keeps the pages of a session that is still open", async () => { + const { service, provider, port } = createReleaseHarness(); + const released = await openPageFor(service, SESSION); + const kept = await openPageFor(service, OTHER); + + await port.deleteSession({ id: SESSION }); + + // Only the released session's page was closed. The other session's page + // is still there, because that session is still open. + const closed = provider.commands.filter( + (entry) => entry.command === "Target.closeTarget", + ); + expect(closed).toHaveLength(1); + expect(closed[0]?.params.targetId).toBe(released.targetId); + await expect(service.selectTarget(kept, OTHER)).resolves.toBeDefined(); + await service.close(); + }); + + it("stops the frames of a released page", async () => { + const { service, provider, port } = createReleaseHarness(); + const opened = await openPageFor(service, SESSION); + + const frames: WebuiBrowserImageFrame[] = []; + const controller = new AbortController(); + const stream = (async () => { + for await (const frame of service.watchFrames( + "isolated-chromium", + opened, + controller.signal, + )) { + frames.push(frame); + } + })(); + await new Promise((resolve) => setTimeout(resolve, 50)); + + await port.deleteSession({ id: SESSION }); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // The screencast for the released page is stopped rather than left + // running for a page that no longer exists. + expect(provider.stoppedStreams).toContain(opened.targetId); + // And a frame for a page that is gone is dropped rather than delivered. + provider.commands.push({ command: "noop", params: {} }); + expect(frames).toHaveLength(0); + controller.abort(); + await stream.catch(() => undefined); + await service.close(); + }); +}); diff --git a/release/public-source.json b/release/public-source.json index 4f45582a8..1c618eb3e 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -4248,6 +4248,7 @@ "packages/webui/test/unit/browser-panel-state.test.ts", "packages/webui/test/unit/browser-panel-view.test.tsx", "packages/webui/test/unit/browser-service.test.ts", + "packages/webui/test/unit/browser-session-release.test.ts", "packages/webui/test/unit/browser-settings-page.test.tsx", "packages/webui/test/unit/browser-target-registry.test.ts", "packages/webui/test/unit/browser-tool-adapter.test.ts", diff --git a/release/webui-npm/bin/mcode-webui.mjs b/release/webui-npm/bin/mcode-webui.mjs index 32cfd9f79..c1d34c905 100644 --- a/release/webui-npm/bin/mcode-webui.mjs +++ b/release/webui-npm/bin/mcode-webui.mjs @@ -72,7 +72,13 @@ const assembled = await createWebuiRuntimeHost({ browserToolExposure: "compact", }); browser.attachHost(assembled.host); -const port = createHarnessPortFromHost(assembled.host); +// A session's pages are released when the session ends -- deleted, archived +// or transferred away. The runtime has no per-session lifecycle signal, and +// this is the only place that holds both the harness port and the browser +// service, so the release is wired here rather than on one operation. +const port = createHarnessPortFromHost(assembled.host, { + onSessionEnded: (sessionId) => browser.releaseSession(sessionId), +}); const terminal = new WebuiTerminalManager(); // The browser port is absent when the host cannot answer the browser // authorization at all, and the browser operations then stay unregistered. diff --git a/scripts/run-webui-server.mjs b/scripts/run-webui-server.mjs index 9e78f32cd..a6eb351a8 100644 --- a/scripts/run-webui-server.mjs +++ b/scripts/run-webui-server.mjs @@ -76,7 +76,13 @@ if (!process.env.WEBUI_SERVER_TSX) { browserToolExposure: "compact", }); browser.attachHost(assembled.host); - const port = createHarnessPortFromHost(assembled.host); + // A session's pages are released when the session ends -- deleted, archived +// or transferred away. The runtime has no per-session lifecycle signal, and +// this is the only place that holds both the harness port and the browser +// service, so the release is wired here rather than on one operation. +const port = createHarnessPortFromHost(assembled.host, { + onSessionEnded: (sessionId) => browser.releaseSession(sessionId), +}); // The registry and the terminal port are assembled here, by the host, and the // network service binds what it was handed (T14). The manager itself belongs to // this service's resource scope — one per service, not a host-wide singleton diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 505a09bea..c3fda884d 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -450,6 +450,7 @@ "packages/webui/test/unit/browser-tool-adapter.test.ts", "packages/webui/test/unit/browser-web-tabs-operation.test.ts", "packages/webui/test/unit/browser-frame-stream.test.ts", + "packages/webui/test/unit/browser-session-release.test.ts", "packages/webui/test/unit/browser-bridge-cdp.test.ts", "packages/webui/test/unit/browser-bridge-adversarial.test.ts", "packages/webui/test/unit/browser-bridge-credential-secrecy.test.ts" From a59ad386c0d0ac9a1db415a238635867d840bd49 Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 21:13:02 +0800 Subject: [PATCH 08/34] feat(webui): the Agent opens tabs, and a run with no selection is not a dead end Issue #206. The tool adapter resolved the surface from the run's selection and refused with browser_target_unknown when the run had selected nothing. The refusal is right and stays for every action -- but it reached open_tab too, and open_tab is the action that would create the selection the gate wants. So open_tab is the one action a run with no selection may perform. It asks the service to open a page; the service observes the new target, makes it the run's selection, and publishes the new snapshot. The surface is a fact rather than a choice the run made: a run with a selection keeps the surface it selected on, and a run without one opens a page in the isolated Chromium, because the embedded panel is the isolated browser's view and the external browsers are driven in the person's own windows. The provider gains openTab as a first-class method, because open_tab is not aimed at a page -- it creates one -- and the page a run is about to open does not exist yet. A surface that cannot open a page has no such member, and the service refuses with browser_capability_denied, which is the extension bridge's case. The refusals keep their meaning. A surface whose switch is off is refused with browser_surface_not_authorized and no page is created. A surface that cannot open a tab is refused with browser_capability_denied. Every other action of a cold run is still refused with browser_target_unknown, so the refusal reaches no further than it did. The action is an explicit bootstrap and not a default: only the action that creates a selection creates one. A second open_tab moves the run's selection to the second page without losing the first. The new real-browser case drives the whole path: a cold run is refused everything, opens a tab, and then navigates the page it opened. --- .../docs/browser-use/01-capability-matrix.md | 2 +- .../src/runtime/browser/browser-service.ts | 63 ++++++++ .../browser/isolated-chromium/provider.ts | 56 ++++++-- .../webui/src/runtime/browser/tool-adapter.ts | 25 ++++ .../isolated-chromium-two-pages.test.ts | 72 ++++++++++ .../test/unit/browser-tool-adapter.test.ts | 136 ++++++++++++++++-- 6 files changed, 331 insertions(+), 23 deletions(-) diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index a01450d9f..3b2ef1a9e 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -384,7 +384,7 @@ that closes it. It is a gap, not a denial. | A session's pages are released when the session ends | **implemented** | the release is keyed on the **session**, not on one operation. The three ways a session goes away — deletion, archiving, and a transfer away — all reach it: the first two through the harness port's own `deleteSession` and `archiveSession`, the third through the transfer's rollback, which calls the same `deleteSession`. `packages/webui/src/runtime/harness/sessions.ts` — `releaseSessionIfItEnded`; `packages/webui/src/runtime/browser/browser-service.ts` — `releaseSession` | | The runtime publishes the session's web tabs as a snapshot | **implemented** | the service keeps the session's web tabs beside the run's selection and publishes them on the `watchBrowserWebTabs` stream — each tab's id, its target identity, its title, its URL, and whether it is the run's selection. `packages/webui/src/runtime/browser/browser-service.ts` — `watchWebTabs` | | The client subscribes to the published snapshot | **implemented** | the browser port carries `watchBrowserWebTabs`, the transport's request table has it, and the wire opens the stream. The strip renders from it in #208. `packages/webui/src/client/features/browser/contracts/browser-port.ts` | -| The Agent opens tabs and gives a run its first selection | **not yet measured** | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | +| The Agent opens tabs and gives a run its first selection | **implemented** | `open_tab` is the one action a run with no selection may perform, because it is the action that would create the selection the gate wants. Every other action of a cold run is still refused with `browser_target_unknown`. `packages/webui/src/runtime/browser/tool-adapter.ts` — the `open_tab` branch; `packages/webui/src/runtime/browser/browser-service.ts` — `openWebTab` | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | | The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | | The frame operation's request names a target | **implemented** | `watchBrowserFrames` takes `{ surface, target }`. A request that still carries a run id is refused with an explicit error rather than answered with the run's selection. `packages/webui/src/server/operation/browser.ts` — `validateFrameStream` | | The provider's attachment retry | **implemented** | `provider.ts` — the mechanism is cited in full above | diff --git a/packages/webui/src/runtime/browser/browser-service.ts b/packages/webui/src/runtime/browser/browser-service.ts index a5350d205..1bc9d8c6a 100644 --- a/packages/webui/src/runtime/browser/browser-service.ts +++ b/packages/webui/src/runtime/browser/browser-service.ts @@ -1042,6 +1042,69 @@ export class WebuiBrowserService { * page it acted on; it is the registry's current view of that target, not a * second identity the service keeps. */ + /** + * Opens a page for a run and makes it the run's selection. + * + * This is the bootstrap. `open_tab` is the one action a run with no + * selection may perform, because it is the action that would create the + * selection the gate wants. Every other action of a run with no selection is + * still refused with `browser_target_unknown`: a run that has selected + * nothing is refused rather than handed a page nobody chose, and only the + * action that creates a selection is allowed to create one. + * + * The surface is a fact rather than a choice the run made. A run with a + * selection keeps the surface it selected on; a run without one opens a + * page in the isolated Chromium, because the embedded panel is the isolated + * browser's view and the external browsers are driven in the person's own + * windows. + */ + async openWebTab( + runId: string, + params: Record = {}, + signal?: AbortSignal, + ): Promise<{ + readonly accepted: true; + readonly target: WebuiBrowserTargetId; + readonly result?: unknown; + }> { + const surface = this.selectionSurfaceOf(runId) ?? "isolated-chromium"; + // The switch comes first: a surface that is switched off is refused + // before anything is attempted, so the refusal names the switch rather + // than the selection. + await this.#authorizeCommand(surface, "open_tab"); + const provider = await this.#commandProvider(surface); + if (!provider.openTab) { + throw browserOperationError( + "browser_capability_denied", + `Browser surface "${surface}" cannot open a tab.`, + ); + } + const identity = (await awaitAbortableCommand( + provider.openTab(params), + signal, + )) as WebuiBrowserTargetId; + // The new page is known to the registry, and it is the session's web tab. + this.#registry.observe({ + surface: identity.surface, + browserId: identity.browserId, + targetId: identity.targetId, + title: "", + url: "", + status: "open", + generation: identity.generation, + }); + this.#recordWebTab(surface, runId, identity); + // The Agent's tab is the run's selection: the run acts on the page it + // opened, and a second `open_tab` moves it to the second page without + // losing the first. + this.#runSelections(surface).set(runId, { + target: identity, + order: (this.#selectionOrder += 1), + }); + this.#publishWebTabs(surface, runId); + return { accepted: true, target: identity, result: identity }; + } + async sendCommandToSelection( surface: WebuiBrowserSurface, runId: string, diff --git a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts index 13155e4a7..2bc550e75 100644 --- a/packages/webui/src/runtime/browser/isolated-chromium/provider.ts +++ b/packages/webui/src/runtime/browser/isolated-chromium/provider.ts @@ -137,6 +137,18 @@ export interface WebuiBrowserSurfaceProvider { command: string, params: Record, ): Promise; + /** + * Opens a page and returns its identity. + * + * `open_tab` is not aimed at a page — it creates one — so it needs no target + * to be sent to. That is what makes it the bootstrap: a run with no + * selection has no page to aim at, and the page it is about to open does not + * exist yet. + * + * A surface that cannot open a page has no such method, and the service + * refuses with `browser_capability_denied`. + */ + openTab?(params: Record): Promise; /** Releases everything the provider holds. */ close(): Promise; } @@ -325,6 +337,35 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide if (this.#launchTargetId === targetId) this.#launchTargetId = undefined; } + /** + * Opens a page, attaches to it and registers it. + * + * The new page is a web tab this provider drives: it is commanded, streamed + * and input into like any other page, and it carries its own generation. + * The answer names the page that was created — the surface, the browser, the + * target and the generation the provider's own accounting gives it — so a + * caller can address the page from it alone. + */ + async openTab(params: Record = {}): Promise { + const connection = this.#connection; + if (!connection) { + throw browserOperationError( + "browser_provider_unavailable", + "The isolated Chromium is not running.", + true, + ); + } + const url = typeof params.url === "string" ? params.url : "about:blank"; + const created = await connection.send<{ targetId: string }>("Target.createTarget", { url }); + const page = await this.#attachPage(created.targetId); + return { + surface: this.surface, + browserId: this.browserId, + targetId: page.targetId, + generation: page.generation, + }; + } + async listTargets(): Promise { const connection = this.#connection; if (!connection) return []; @@ -591,23 +632,10 @@ export class WebuiIsolatedChromiumProvider implements WebuiBrowserSurfaceProvide return await connection.send("DOM.getDocument", { depth: -1 }, sessionId); } case "open_tab": { - const url = typeof params.url === "string" ? params.url : "about:blank"; - const created = await connection.send<{ targetId: string }>("Target.createTarget", { - url, - }); - // The new page is attached to and registered, so it is a web tab this - // provider drives: it is commanded, streamed and input into like any - // other page, and it carries its own generation. - const page = await this.#attachPage(created.targetId); // The answer names the page that was created: the surface, the // browser, the target and the generation the provider's own accounting // gives it. A caller can address the page from this answer alone. - return { - surface: this.surface, - browserId: this.browserId, - targetId: page.targetId, - generation: page.generation, - } satisfies WebuiBrowserTargetId; + return await this.openTab(params); } default: break; diff --git a/packages/webui/src/runtime/browser/tool-adapter.ts b/packages/webui/src/runtime/browser/tool-adapter.ts index c8c854bc9..0be07616c 100644 --- a/packages/webui/src/runtime/browser/tool-adapter.ts +++ b/packages/webui/src/runtime/browser/tool-adapter.ts @@ -64,6 +64,22 @@ export interface WebuiBrowserToolCommandSurface { * adapter's cue to refuse rather than to choose a browser. */ selectionSurfaceOf(runId: string): WebuiBrowserSurface | undefined; + /** + * Opens a page for a run and makes it the run's selection. + * + * The bootstrap: `open_tab` is the one action a run with no selection may + * perform, because it is the action that would create the selection the + * gate wants. + */ + openWebTab( + runId: string, + params?: Record, + signal?: AbortSignal, + ): Promise<{ + readonly accepted: true; + readonly target: WebuiBrowserTargetId; + readonly result?: unknown; + }>; sendCommandToSelection( surface: WebuiBrowserSurface, runId: string, @@ -159,6 +175,15 @@ export function createWebuiBrowserToolAdapter( // selection: two runs driving one surface act on the target each of them // selected, never on each other's. const runId = ctx.sessionId; + if (action === "open_tab") { + // `open_tab` is the bootstrap, and it is the only exception to the + // gate below. It is the action that would create the selection the + // gate wants, so it is the one action a run with no selection may + // perform — and it moves the run's selection to the page it opened. + // Every other action of a run with no selection is still refused. + const { result } = await options.browser.openWebTab(runId, input, signal); + return result; + } const surface = options.browser.selectionSurfaceOf(runId); if (!surface) { // There is no surface to act on that the run chose, and no honest diff --git a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts index e3ab6c8e2..001a1bb8c 100644 --- a/packages/webui/test/integration/isolated-chromium-two-pages.test.ts +++ b/packages/webui/test/integration/isolated-chromium-two-pages.test.ts @@ -28,6 +28,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { WebuiIsolatedChromiumProvider } from "../../src/runtime/browser/isolated-chromium/provider.js"; import { WebuiBrowserService } from "../../src/runtime/browser/browser-service.js"; +import { createWebuiBrowserToolAdapter } from "../../src/runtime/browser/tool-adapter.js"; import type { WebuiBrowserAuthorization, WebuiBrowserImageFrame, @@ -422,3 +423,74 @@ describe.skipIf(!available)("the browser service, streaming two pages of one Chr expect(secondFrames.every((frame) => frame.target.targetId === second.targetId)).toBe(true); }); }); + +/** + * The Agent's own path, against a real isolated Chromium. + * + * A cold run has selected nothing, and `open_tab` is the one action it may + * perform: it is the action that would create the selection the gate wants. + * The page it opens is then the run's page, and the run can drive it. + */ +describe.skipIf(!available)("the Agent opening a tab of a real isolated Chromium", () => { + let dataDir: string; + let service: WebuiBrowserService; + + const authorization: WebuiBrowserAuthorization = { + isolatedChromium: true, + edge: false, + chrome: false, + // The raw DevTools method below reads the page back, and the Full CDP + // switch is what allows a raw method through. + fullCdp: true, + }; + + beforeAll(async () => { + dataDir = await mkdtemp(join(tmpdir(), "webui-agent-opens-tab-")); + service = new WebuiBrowserService({ + authorization: { + read: async () => authorization, + write: async () => authorization, + }, + dataDir, + }); + }, 60_000); + + afterAll(async () => { + await service?.close(); + if (dataDir) await rm(dataDir, { recursive: true, force: true }); + }); + + it("opens a page the same run can then navigate", async () => { + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + const context = { sessionId: "run-1" } as never; + + // The run has selected nothing, and it is refused everything — except the + // action that creates a selection. + await expect( + adapter.execute(context, "navigate", { url: pageUrl("refused") }), + ).rejects.toMatchObject({ code: "browser_target_unknown" }); + + const opened = (await adapter.execute(context, "open_tab", { + url: pageUrl("agent page"), + })) as WebuiBrowserTargetId; + expect(opened.targetId).toEqual(expect.any(String)); + + // The run's selection is the page it opened, and the run can drive it. + const list = await service.listTargets("isolated-chromium", "run-1"); + expect(list.targets.find((entry) => entry.selected)?.id.targetId).toBe( + opened.targetId, + ); + + await adapter.execute(context, "navigate", { url: pageUrl("driven by the agent") }); + await new Promise((resolve) => setTimeout(resolve, SETTLE_MS)); + + // The page behind the tab is the one the run navigated. + const read = (await service.sendCommandToSelection( + "isolated-chromium", + "run-1", + "Runtime.evaluate", + { expression: "document.title", returnByValue: true }, + )) as { result?: { result?: { value?: string } } }; + expect(read.result?.result?.value).toBe("driven by the agent"); + }); +}); diff --git a/packages/webui/test/unit/browser-tool-adapter.test.ts b/packages/webui/test/unit/browser-tool-adapter.test.ts index a0bf96343..6cc6af9c0 100644 --- a/packages/webui/test/unit/browser-tool-adapter.test.ts +++ b/packages/webui/test/unit/browser-tool-adapter.test.ts @@ -37,16 +37,12 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { readonly commands: { command: string; params: Record }[] = []; /** The tabs this browser has, so two runs can hold different selections. */ readonly tabIds: readonly string[]; + /** The pages a command opened, in the order they were opened. */ + readonly openedTabs: string[] = []; /** When set, a command is recorded but never answers. */ neverSettle = false; closed = false; - constructor(surface: WebuiBrowserSurface, tabIds: readonly string[] = ["tab-1"]) { - this.surface = surface; - this.browserId = surface === "isolated-chromium" ? "browser-1" : `${surface}-browser`; - this.tabIds = tabIds; - } - async sendCommand( _target: unknown, command: string, @@ -57,8 +53,43 @@ class RecordingProvider implements WebuiBrowserSurfaceProvider { return { command, params }; } + /** + * Opens a page and answers with its identity. + * + * The shape the isolated provider answers with: the surface, the browser, + * the target and the generation the provider's own accounting gives it. + * + * The member is **absent** on a surface that cannot open a page at all, the + * way the extension bridge's provider has no such member — the service + * refuses with `browser_capability_denied` when it is missing. + */ + openTab?: (params?: Record) => Promise; + + constructor( + surface: WebuiBrowserSurface, + tabIds: readonly string[] = ["tab-1"], + canOpenTab = true, + ) { + this.surface = surface; + this.browserId = surface === "isolated-chromium" ? "browser-1" : `${surface}-browser`; + this.tabIds = tabIds; + if (canOpenTab) { + this.openTab = async (params: Record = {}) => { + this.commands.push({ command: "open_tab", params }); + const targetId = `tab-${this.tabIds.length + this.openedTabs.length + 1}`; + this.openedTabs.push(targetId); + return { + surface: this.surface, + browserId: this.browserId, + targetId, + generation: 1, + }; + }; + } + } + async listTargets(): Promise { - return this.tabIds.map((targetId) => ({ + return [...this.tabIds, ...this.openedTabs].map((targetId) => ({ id: { surface: this.surface, browserId: this.browserId, @@ -145,8 +176,10 @@ function createService( "isolated-chromium", options.isolatedTabs ?? ["tab-1"], ); + // The edge provider never has an `openTab`: the extension drives the + // person's own browser through its debugger and cannot open a page in it. const edge = options.edgeTabs - ? new RecordingProvider("edge", options.edgeTabs) + ? new RecordingProvider("edge", options.edgeTabs, false) : undefined; const state = { authorization }; const serviceOptions: WebuiBrowserServiceOptions = { @@ -175,6 +208,15 @@ function createService( * testing the refusal path. The surface follows the target, so a run that * selected in Edge is set up in Edge. */ +/** The target the named run currently has selected, as the list reports it. */ +async function selectedTargetOf( + service: WebuiBrowserService, + runId?: string, +): Promise { + const list = await service.listTargets("isolated-chromium", runId); + return list.targets.find((target) => target.selected)?.id.targetId; +} + async function selectRunTarget( service: WebuiBrowserService, runId: string, @@ -237,6 +279,10 @@ function recordingCommandSurface(surfaceOf: (runId: string) => WebuiBrowserSurfa asked.push(runId); return surfaceOf(runId); }, + openWebTab: async (runId, params, signal) => { + calls.push({ surface: "isolated-chromium", runId, command: "open_tab", params, signal }); + return { accepted: true as const, target: SELECTED_TARGET, result: "opened" }; + }, sendCommandToSelection: async (surface, runId, command, params, signal) => { calls.push({ surface, runId, command, params, signal }); return { accepted: true as const, target: SELECTED_TARGET, result: "done" }; @@ -246,6 +292,80 @@ function recordingCommandSurface(surfaceOf: (runId: string) => WebuiBrowserSurfa } describe("Agent browser-tool adapter", () => { + it("lets a cold run open a tab, and gives the run that page", async () => { + const { service, provider } = createService(SURFACE_AUTHORIZED); + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + + // The run has selected nothing, and `open_tab` is the one action it may + // perform: it is the action that would create the selection the gate wants. + const result = await adapter.execute(TOOL_CONTEXT, "open_tab", { + url: "https://example.test/", + }); + + expect(provider.commands.map((entry) => entry.command)).toContain("open_tab"); + // The run's selection is the page it opened, so the run can act on it. + expect(await selectedTargetOf(service, "run-a")).toBe("tab-2"); + expect(result).toMatchObject({ targetId: "tab-2" }); + }); + + it("refuses every other action of a cold run", async () => { + const { service, provider } = createService(SURFACE_AUTHORIZED); + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + + // The refusal reaches no further than it did: the run has selected + // nothing, so it is refused rather than handed a page nobody chose. + await expect( + adapter.execute(TOOL_CONTEXT, "navigate", { url: "https://example.test/" }), + ).rejects.toMatchObject({ code: "browser_target_unknown" }); + expect(provider.commands).toHaveLength(0); + }); + + it("refuses open_tab on a surface whose switch is off, and opens no page", async () => { + const { service, provider } = createService(ALL_OFF); + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + + await expect( + adapter.execute(TOOL_CONTEXT, "open_tab", { url: "https://example.test/" }), + ).rejects.toMatchObject({ code: "browser_surface_not_authorized" }); + expect(provider.commands).toHaveLength(0); + }); + + it("refuses open_tab on a surface that cannot open a tab", async () => { + const { service } = createService( + { ...ALL_OFF, edge: true }, + { edgeTabs: ["tab-edge"] }, + ); + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + // The run works on Edge, and the extension drives that browser through + // its debugger — it cannot open a page in it. + await service.listTargets("edge", "run-a"); + await service.selectTarget(EDGE_TARGET, "run-a"); + + await expect( + adapter.execute(TOOL_CONTEXT, "open_tab", { url: "https://example.test/" }), + ).rejects.toMatchObject({ code: "browser_capability_denied" }); + }); + + it("moves the run's selection to a second tab without losing the first", async () => { + const { service, provider } = createService(SURFACE_AUTHORIZED); + const adapter = createWebuiBrowserToolAdapter({ browser: service }); + + await adapter.execute(TOOL_CONTEXT, "open_tab", { url: "https://example.test/one" }); + expect(await selectedTargetOf(service, "run-a")).toBe("tab-2"); + + await adapter.execute(TOOL_CONTEXT, "open_tab", { url: "https://example.test/two" }); + // The run's selection moved to the second page, and the first page is + // still the session's. + expect(await selectedTargetOf(service, "run-a")).toBe("tab-3"); + const list = await service.listTargets("isolated-chromium", "run-a"); + expect(list.targets.map((entry) => entry.id.targetId)).toEqual([ + "tab-1", + "tab-2", + "tab-3", + ]); + expect(provider.openedTabs).toEqual(["tab-2", "tab-3"]); + }); + it("declares the restricted command set as its whole capability", () => { const { service } = createService(SURFACE_AUTHORIZED); const adapter = createWebuiBrowserToolAdapter({ browser: service }); From 29945e4971aa47fdf935f15d485e865c20732c1b Mon Sep 17 00:00:00 2001 From: Jiaxiaoyuan Date: Wed, 21 Oct 2026 21:28:19 +0800 Subject: [PATCH 09/34] feat(webui): a tab is a stable id and the content it is showing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #207. The tab's id was derived from its kind and its parts, so the id changed when the content did — and a web tab had no id at all that could tell two of them apart. The browser kind was one tab per session and directory, and the workspace's change set and a turn's review derived the same id, so opening one replaced the other. A tab is now a stable id plus a content descriptor, and the content carries the kind. The id is assigned by the client when the tab is opened and does not change when the content does: two web tabs in one session are two tabs, and a tab that changes what it shows is still the same tab. The rules that used to fall out of the id are content checks now, because the id says nothing about what the tab is showing. The same file opens one tab — per session and directory, as before. The workspace's change set and a turn's review are two tabs, told apart by the content's source. The files view, the canvas and the terminal are one tab per session and directory, and that is an explicit rule rather than a side effect of the id. The browser kind is gone from the enumeration and nothing mounts the browser tool tab: the panel's tabs are web tabs now. A web tab has no add-menu entry, because a web tab is opened by typing a URL or by the Agent. The stale-read hazard the mutable content brings is answered on both sides. The effects that fill the caches depend on the content, not on the id, so a content change re-runs them; and the file results are keyed by what the tab is showing rather than by which slot it is, so a tab that changed from one file to another reads the new one instead of keeping the old one's result. --- .../docs/browser-use/01-capability-matrix.md | 8 +- .../application/workspace-files-owner.ts | 27 +- .../application/workspace-owner-contracts.ts | 13 +- .../workspace/contracts/panel-command.ts | 11 +- .../projection/workspace-panel-state.ts | 392 ++++++++++++++---- .../features/workspace/ui/WorkspacePanels.tsx | 139 ++++--- .../unit/browser-workspace-panel.test.tsx | 76 ++-- packages/webui/test/unit/webui-shell.test.ts | 6 +- .../test/unit/workspace-panel-state.test.ts | 91 +++- .../webui/test/unit/workspace-queries.test.ts | 24 ++ .../test/unit/workspace-tab-registry.test.ts | 42 +- 11 files changed, 606 insertions(+), 223 deletions(-) diff --git a/packages/webui/docs/browser-use/01-capability-matrix.md b/packages/webui/docs/browser-use/01-capability-matrix.md index 3b2ef1a9e..b2ed3e690 100644 --- a/packages/webui/docs/browser-use/01-capability-matrix.md +++ b/packages/webui/docs/browser-use/01-capability-matrix.md @@ -385,12 +385,12 @@ that closes it. It is a gap, not a denial. | The runtime publishes the session's web tabs as a snapshot | **implemented** | the service keeps the session's web tabs beside the run's selection and publishes them on the `watchBrowserWebTabs` stream — each tab's id, its target identity, its title, its URL, and whether it is the run's selection. `packages/webui/src/runtime/browser/browser-service.ts` — `watchWebTabs` | | The client subscribes to the published snapshot | **implemented** | the browser port carries `watchBrowserWebTabs`, the transport's request table has it, and the wire opens the stream. The strip renders from it in #208. `packages/webui/src/client/features/browser/contracts/browser-port.ts` | | The Agent opens tabs and gives a run its first selection | **implemented** | `open_tab` is the one action a run with no selection may perform, because it is the action that would create the selection the gate wants. Every other action of a cold run is still refused with `browser_target_unknown`. `packages/webui/src/runtime/browser/tool-adapter.ts` — the `open_tab` branch; `packages/webui/src/runtime/browser/browser-service.ts` — `openWebTab` | `tool-adapter.ts:163` refuses a run that has selected nothing, so the Agent cannot acquire a selection of its own and `open_tab` is refused with it. The provider's side is done; the tool's side is not. #206 | -| The client's tab is a stable id with a mutable content | **not yet measured** | `workspace-panel-state.ts:251` derives the id from the kind and its parts, so there is no stable id for a web tab whose page changes under it. #207 | +| The client's tab is a stable id with a mutable content | **implemented** | the id is assigned by the client when the tab is opened and does not change when the content does, so two web tabs in one session are two tabs. The rules that used to fall out of the id — the same file opens one tab, the workspace review and the turn review are two tabs — are content checks now. `packages/webui/src/client/features/workspace/projection/workspace-panel-state.ts` | | The frame operation's request names a target | **implemented** | `watchBrowserFrames` takes `{ surface, target }`. A request that still carries a run id is refused with an explicit error rather than answered with the run's selection. `packages/webui/src/server/operation/browser.ts` — `validateFrameStream` | | The provider's attachment retry | **implemented** | `provider.ts` — the mechanism is cited in full above | -| The files tab is a singleton | **implemented** | `workspace-panel-state.ts:289` drops every `files` tab before adding one | -| The two review tabs share one id | **implemented** | `workspace-panel-state.ts:279` and `:283` derive the same `tabId("review", sessionId, workspaceDir)` | -| A tab's label is derived from its kind | **implemented** | `workspace-panel-state.ts:154` | +| The files tab is a singleton | **implemented** | an explicit content check now: one files tab per session and directory. The id no longer implies it, because the id says nothing about what the tab is showing | +| The two review tabs are two tabs | **implemented** | the workspace's change set and a turn's review are told apart by the content's `source`, so opening one no longer replaces the other | +| A tab's label follows its content | **implemented** | the content carries the kind, and a web tab is titled by the page the runtime publishes | **What the provider does today, measured again.** The provider's `open_tab` creates a page, attaches to it and registers it, and the page is listed with its own identity diff --git a/packages/webui/src/client/features/workspace/application/workspace-files-owner.ts b/packages/webui/src/client/features/workspace/application/workspace-files-owner.ts index 344727f23..e5c0e4344 100644 --- a/packages/webui/src/client/features/workspace/application/workspace-files-owner.ts +++ b/packages/webui/src/client/features/workspace/application/workspace-files-owner.ts @@ -320,32 +320,39 @@ export function createWebuiWorkspaceFilesOwner(deps: { } }; - const readFile = (tabId: string, workspaceDir: string, path: string): void => { - if (state.files.get(tabId)?.content) return; - const key = `file:${tabId}`; + /** + * Reads a file into the cache. + * + * The first argument is the **cache key**, not a tab id: the cache is keyed + * by what the tab is showing, so a tab that changed from one file to another + * reads the new one instead of keeping the old one's result. + */ + const readFile = (cacheKey: string, workspaceDir: string, path: string): void => { + if (state.files.get(cacheKey)?.content) return; + const key = `file:${cacheKey}`; const version = bump(key); if (!port.readWorkspaceFile) { - setFile(tabId, { loading: false, error: "文件读取能力暂不可用。" }); + setFile(cacheKey, { loading: false, error: "文件读取能力暂不可用。" }); return; } const read = port.readWorkspaceFile; - setFile(tabId, { loading: true }); + setFile(cacheKey, { loading: true }); void read({ workspaceDir, path }) .then((content) => { if (!isCurrent(key, version)) return; - setFile(tabId, { loading: false, content }); + setFile(cacheKey, { loading: false, content }); }) .catch((reason: unknown) => { if (!isCurrent(key, version)) return; - setFile(tabId, { loading: false, error: message(reason) }); + setFile(cacheKey, { loading: false, error: message(reason) }); }); }; - const setInlineFile = (tabId: string, content: string, resolvedPath: string): void => { + const setInlineFile = (cacheKey: string, content: string, resolvedPath: string): void => { // Supersede any in-flight read for this tab so a late read does not // overwrite the inline content the tab already carries. - bump(`file:${tabId}`); - setFile(tabId, { + bump(`file:${cacheKey}`); + setFile(cacheKey, { loading: false, content: { type: "text", content, resolvedPath }, }); diff --git a/packages/webui/src/client/features/workspace/application/workspace-owner-contracts.ts b/packages/webui/src/client/features/workspace/application/workspace-owner-contracts.ts index fb2d12dee..d9cbd2cdc 100644 --- a/packages/webui/src/client/features/workspace/application/workspace-owner-contracts.ts +++ b/packages/webui/src/client/features/workspace/application/workspace-owner-contracts.ts @@ -34,8 +34,17 @@ export interface WebuiWorkspaceFilesCommands { readonly selectWorkspace: (workspaceDir: string | undefined) => void; readonly loadFileTree: (workspaceDir: string) => void; readonly loadDirectory: (workspaceDir: string, path: string) => Promise; - readonly readFile: (tabId: string, workspaceDir: string, path: string) => void; - readonly setInlineFile: (tabId: string, content: string, resolvedPath: string) => void; + /** + * Reads a file into the cache. + * + * The first argument is the **cache key** — what the tab is showing — not a + * tab id. The cache is keyed that way because a tab's id is stable and its + * content is not: a tab that changed from one file to another has to read + * the new one rather than keep the old one's result. + */ + readonly readFile: (cacheKey: string, workspaceDir: string, path: string) => void; + /** Files an inline text under the same cache key a read would use. */ + readonly setInlineFile: (cacheKey: string, content: string, resolvedPath: string) => void; /** Release a closed tab's file slot and its read commit rights. */ readonly discardFileTab: (tabId: string) => void; /** List one directory for the workspace picker (superseded browses dropped). */ diff --git a/packages/webui/src/client/features/workspace/contracts/panel-command.ts b/packages/webui/src/client/features/workspace/contracts/panel-command.ts index c65def8e6..2ec284d9f 100644 --- a/packages/webui/src/client/features/workspace/contracts/panel-command.ts +++ b/packages/webui/src/client/features/workspace/contracts/panel-command.ts @@ -5,6 +5,7 @@ // from the projection that happens to reduce it. import type { WebuiTurnDiffView } from "../../../../shared/contracts/session.js"; +import type { WebuiBrowserTargetId } from "../../../../shared/contracts/browser.js"; export type WorkspacePanelCommand = | { readonly type: "open-file"; readonly sessionId: string; readonly workspaceDir: string; readonly path: string; readonly lineStart?: number; readonly lineEnd?: number } @@ -18,6 +19,14 @@ export type WorkspacePanelCommand = | { readonly type: "open-workspace-review"; readonly sessionId: string; readonly workspaceDir: string; readonly selectedPath?: string } | { readonly type: "open-turn-review"; readonly sessionId: string; readonly workspaceDir: string; readonly messageId: string; readonly assistantMessageId?: string; readonly turnId?: string; readonly changeSetId?: string; readonly files: WebuiTurnDiffView["fileChanges"]; readonly selectedPath?: string } | { readonly type: "open-primary-view"; readonly kind: "files"; readonly sessionId?: string; readonly workspaceDir?: string } + /** + * Opens a web tab, or updates the one already bound to that target. + * + * The command carries the target the tab is bound to. The tab's own id is + * the client's and is assigned when the tab is opened, so the same target + * always finds the same tab. + */ + | { readonly type: "open-web-tab"; readonly target: WebuiBrowserTargetId; readonly title: string; readonly url: string } | { readonly type: "open-tab"; readonly kind: WebuiWorkspaceMenuTabKind; readonly sessionId?: string; readonly workspaceDir?: string } | { readonly type: "select-tab"; readonly tabId: string } | { readonly type: "select-review-file"; readonly tabId: string; readonly path: string } @@ -40,4 +49,4 @@ export type WebuiWorkspaceMenuTabKind = Extract< /** How an add-menu entry opens its tab. Data, so the component only dispatches it. */ export type WebuiWorkspaceAddMenuOpen = | { readonly type: "open-workspace-review" } - | { readonly type: "open-tab"; readonly kind: "files" | "canvas" | "terminal" | "browser" }; + | { readonly type: "open-tab"; readonly kind: "files" | "canvas" | "terminal" }; diff --git a/packages/webui/src/client/features/workspace/projection/workspace-panel-state.ts b/packages/webui/src/client/features/workspace/projection/workspace-panel-state.ts index 0229324f5..34a4d6a84 100644 --- a/packages/webui/src/client/features/workspace/projection/workspace-panel-state.ts +++ b/packages/webui/src/client/features/workspace/projection/workspace-panel-state.ts @@ -1,4 +1,5 @@ import type { WebuiTurnDiffView } from "../../../../shared/contracts/session.js"; +import type { WebuiBrowserTargetId } from "../../../../shared/contracts/browser.js"; import { basenamePath } from "../../../projection/common/paths.js"; // --------------------------------------------------------------------------- @@ -15,73 +16,118 @@ import { basenamePath } from "../../../projection/common/paths.js"; // --------------------------------------------------------------------------- /** - * One kind's tab shape. The keys are the closed set — a new kind is a new key - * here, and nothing else spells the set out. + * What a tab's slot is showing. + * + * A tab is a stable slot whose content can be replaced in place: a tab that + * shows the new tab page and the same tab that later shows a web page are one + * tab, and the strip does not flicker when the content changes. The kinds are + * the content's own discriminants. * * `review` has two variants because one tab serves both the workspace's change * set and a turn's; the discriminant is `source`, not `kind`. */ -export interface WebuiWorkspaceTabShapes { - readonly files: { - readonly sessionId?: string; - readonly workspaceDir?: string; - }; - readonly "file-preview": { - readonly sessionId: string; - readonly workspaceDir: string; - readonly path: string; - readonly lineStart?: number; - readonly lineEnd?: number; - readonly content?: string; - }; - readonly review: - | { - readonly sessionId: string; - readonly workspaceDir: string; - readonly source: "workspace"; - readonly selectedPath?: string; - readonly reviewSnapshotId?: string; - } - | { - readonly sessionId: string; - readonly workspaceDir: string; - readonly source: "turn"; - readonly messageId: string; - readonly assistantMessageId?: string; - readonly turnId?: string; - readonly changeSetId?: string; - readonly files: WebuiTurnDiffView["fileChanges"]; - readonly selectedPath?: string; - }; - readonly canvas: { - readonly sessionId?: string; - }; - readonly terminal: { - readonly workspaceDir?: string; - }; - /** - * The browser panel's tab. - * - * It carries the optional instance fields every menu-opened tab carries and no - * file path: a browser tab opens a browser's page, not a document in the - * workspace. The instance fields are declared rather than refused because the - * add menu sends them with every `open-tab` and the tab is built as one object - * literal — a shape that would not accept them fails to compile the moment the - * command names this kind, instead of carrying two words no view reads. - */ - readonly browser: { - readonly sessionId?: string; - readonly workspaceDir?: string; - }; +export type WebuiWorkspaceTabContent = + | { + readonly kind: "files" | "canvas" | "terminal"; + readonly sessionId?: string; + readonly workspaceDir?: string; + } + | { + readonly kind: "file-preview"; + readonly sessionId: string; + readonly workspaceDir: string; + readonly path: string; + readonly lineStart?: number; + readonly lineEnd?: number; + readonly content?: string; + } + | { + readonly kind: "review"; + readonly source: "workspace"; + readonly sessionId: string; + readonly workspaceDir: string; + readonly selectedPath?: string; + readonly reviewSnapshotId?: string; + } + | { + readonly kind: "review"; + readonly source: "turn"; + readonly sessionId: string; + readonly workspaceDir: string; + readonly messageId: string; + readonly assistantMessageId?: string; + readonly turnId?: string; + readonly changeSetId?: string; + readonly files: WebuiTurnDiffView["fileChanges"]; + readonly selectedPath?: string; + } + | { + readonly kind: "web"; + /** + * The target the web tab is bound to. + * + * The binding is the runtime's target identity, not the tab's own + * identity: a page that navigates keeps its target id and gains a + * generation, and the tab that holds it is still the same tab. + */ + readonly target: WebuiBrowserTargetId; + /** The page's title, as the runtime publishes it. */ + readonly title: string; + /** The page's url, as the runtime publishes it. */ + readonly url: string; + }; + +/** The content's kinds — the closed set, derived from the content itself. */ +export type WebuiWorkspaceTabKind = WebuiWorkspaceTabContent["kind"]; + +/** + * A tab: a stable id and the content it is showing. + * + * The id belongs to the client. It is assigned when the tab is opened and it + * does not change when the content does, so it says nothing about what the + * tab is showing — and two web tabs in one session are two tabs. + */ +export interface WorkspacePanelTab { + readonly id: string; + readonly content: WebuiWorkspaceTabContent; } -export type WebuiWorkspaceTabKind = keyof WebuiWorkspaceTabShapes; +/** + * The content, when it is the workspace's change set. + * + * The two review variants are one `kind` with a `source` discriminant, and a + * caller that wants the workspace's one has to say so. The narrowing lives + * here rather than at each call site, so the rule is written once. + */ +export function webuiWorkspaceReviewContent( + content: WebuiWorkspaceTabContent | undefined, +): Extract | undefined { + if (!content || content.kind !== "review" || content.source !== "workspace") return undefined; + return content; +} -/** Every tab, derived: one member per declared kind, each carrying `kind` and its shape. */ -export type WorkspacePanelTab = { - [K in WebuiWorkspaceTabKind]: { readonly id: string; readonly kind: K } & - WebuiWorkspaceTabShapes[K]; -}[WebuiWorkspaceTabKind]; +/** + * A key that names what a tab is showing, not which slot it is. + * + * The caches are keyed by this rather than by the tab's id. The id is stable + * and the content is not, so a cache keyed by the id would hand the previous + * content's result to the new one — a tab that changed from one file to + * another would keep showing the first file. + */ +export function webuiWorkspaceTabContentKey(content: WebuiWorkspaceTabContent): string { + switch (content.kind) { + case "file-preview": + return `file:${content.sessionId}:${content.workspaceDir}:${content.path}`; + case "review": + return `review:${content.source}:${content.sessionId}:${content.workspaceDir}:${ + content.source === "turn" ? content.messageId : "" + }`; + case "web": + return `web:${content.target.browserId}:${content.target.targetId}`; + default: + return `${content.kind}:${content.sessionId ?? ""}:${content.workspaceDir ?? ""}`; + } +} // Both unions describe the shape of a panel command, so they live with it in the // workspace's contract (see `features/workspace/contracts/panel-command.ts`); they @@ -108,14 +154,13 @@ export const WEBUI_WORKSPACE_TAB_KINDS: Readonly< canvas: { label: "画布", addMenu: { type: "open-tab", kind: "canvas" } }, terminal: { label: "终端", addMenu: { type: "open-tab", kind: "terminal" } }, /** - * The browser panel. + * A web tab. * - * The command's `open-tab` names `browser` alongside the other kinds, so this - * row has a menu entry of its own. The panel is mounted by the browser - * business's published host, which owns the browser state — a workspace - * component may not construct another business's state. + * It has no add-menu entry of its own: a web tab is opened by typing a URL + * in the new tab page, or by the Agent, and the browser tool tab it used to + * stand for is gone. */ - browser: { label: "浏览器", addMenu: { type: "open-tab", kind: "browser" } }, + web: { label: "网页", addMenu: null }, }; /** @@ -128,7 +173,6 @@ export const WEBUI_WORKSPACE_ADD_MENU_ORDER: readonly WebuiWorkspaceTabKind[] = "review", "terminal", "canvas", - "browser", "files", ]; @@ -152,9 +196,13 @@ export function webuiWorkspaceAddMenu(): readonly WebuiWorkspaceAddMenuEntry[] { * every other kind shows the registry's word. */ export function webuiWorkspaceTabLabel(tab: WorkspacePanelTab): string { - if (tab.kind === "file-preview") return basenamePath(tab.path); - if (tab.kind === "review" && tab.source === "turn") return "Review"; - return WEBUI_WORKSPACE_TAB_KINDS[tab.kind].label; + const content = tab.content; + if (content.kind === "file-preview") return basenamePath(content.path); + if (content.kind === "review" && content.source === "turn") return "Review"; + // A web tab is titled by the page, and the title is the runtime's to + // report. A tab whose page has no title yet falls back to the registry's word. + if (content.kind === "web") return content.title || WEBUI_WORKSPACE_TAB_KINDS.web.label; + return WEBUI_WORKSPACE_TAB_KINDS[content.kind].label; } export interface WorkspacePanelState { @@ -163,6 +211,15 @@ export interface WorkspacePanelState { readonly tabs: readonly WorkspacePanelTab[]; readonly activeTabId?: string; readonly addMenuOpen: boolean; + /** + * The next tab number. + * + * The tab's id is assigned by the client when the tab is opened and it does + * not change when the content does, so it cannot be derived from the + * content. The counter is what makes the assignment unique — and it is why + * two web tabs in one session are two tabs rather than one. + */ + readonly nextTabNumber: number; } export interface WorkspacePanelSessionState { @@ -241,6 +298,7 @@ export const initialWorkspacePanelState: WorkspacePanelState = { expanded: false, tabs: [], addMenuOpen: false, + nextTabNumber: 1, }; export const initialWorkspacePanelSessionState: WorkspacePanelSessionState = { @@ -248,8 +306,57 @@ export const initialWorkspacePanelSessionState: WorkspacePanelSessionState = { progressPanelOpen: true, }; -function tabId(kind: string, ...parts: string[]): string { - return `${kind}:${parts.map((part) => encodeURIComponent(part)).join(":")}`; +/** + * Opens a tab with a fresh id and makes it the active one. + * + * The id is assigned here, by the client, because the tab's id belongs to the + * client: it is assigned when the tab is opened and it does not change when + * the content does. Nothing derives it from the content, so two web tabs in + * one session are two tabs. + */ +function openNewTab( + state: WorkspacePanelState, + content: WebuiWorkspaceTabContent, +): WorkspacePanelState { + const id = `tab-${state.nextTabNumber}`; + return activate( + { ...state, nextTabNumber: state.nextTabNumber + 1 }, + { id, content }, + ); +} + +/** + * Whether the content is the same instance view. + * + * The files view, the canvas and the terminal are one tab per session and + * directory. Their ids used to say so, and the id no longer says anything — + * so the rule is written here instead. + */ +function isSameInstance( + content: WebuiWorkspaceTabContent, + kind: "files" | "canvas" | "terminal", + sessionId?: string, + workspaceDir?: string, +): boolean { + return ( + content.kind === kind && + content.sessionId === sessionId && + content.workspaceDir === workspaceDir + ); +} + +/** + * The tab that is already showing this content, when there is one. + * + * The rules that used to fall out of the id — the same file opens one tab, + * the workspace's change set and a turn's review are one tab — are content + * checks now, because the id says nothing about what the tab is showing. + */ +function tabShowing( + state: WorkspacePanelState, + matches: (content: WebuiWorkspaceTabContent) => boolean, +): WorkspacePanelTab | undefined { + return state.tabs.find((tab) => matches(tab.content)); } function activate(state: WorkspacePanelState, tab: WorkspacePanelTab): WorkspacePanelState { @@ -268,39 +375,148 @@ export function reduceWorkspacePanelState(state: WorkspacePanelState, command: W case "open-file": { const path = command.path.replace(/\\/gu, "/").replace(/^\.\//u, ""); if (!path || path.startsWith("/") || path.split("/").some((part) => part === ".." || part === "")) return state; - return activate(state, { id: tabId("file", command.sessionId, command.workspaceDir, path), kind: "file-preview", sessionId: command.sessionId, workspaceDir: command.workspaceDir, path, ...(command.lineStart !== undefined ? { lineStart: command.lineStart } : {}), ...(command.lineEnd !== undefined ? { lineEnd: command.lineEnd } : {}) }); + const content: WebuiWorkspaceTabContent = { + kind: "file-preview", + sessionId: command.sessionId, + workspaceDir: command.workspaceDir, + path, + ...(command.lineStart !== undefined ? { lineStart: command.lineStart } : {}), + ...(command.lineEnd !== undefined ? { lineEnd: command.lineEnd } : {}), + }; + // The same file opens one tab. The rule is a content check now, not an + // id derivation: the id belongs to the client and says nothing about + // what the tab is showing. + const existing = tabShowing( + state, + (candidate) => + candidate.kind === "file-preview" && + candidate.path === path && + candidate.sessionId === command.sessionId && + candidate.workspaceDir === command.workspaceDir, + ); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "open-plan-file": { const path = command.path.replace(/\\/gu, "/").replace(/^\.\//u, ""); - const id = tabId("file", command.sessionId, command.workspaceDir, path); - return activate(state, { id, kind: "file-preview", sessionId: command.sessionId, workspaceDir: command.workspaceDir, path, content: command.content }); + const content: WebuiWorkspaceTabContent = { + kind: "file-preview", + sessionId: command.sessionId, + workspaceDir: command.workspaceDir, + path, + content: command.content, + }; + const existing = tabShowing( + state, + (candidate) => + candidate.kind === "file-preview" && + candidate.path === path && + candidate.sessionId === command.sessionId && + candidate.workspaceDir === command.workspaceDir, + ); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "open-workspace-review": { - const id = tabId("review", command.sessionId, command.workspaceDir); - return activate(state, { id, kind: "review", source: "workspace", sessionId: command.sessionId, workspaceDir: command.workspaceDir, ...(command.selectedPath ? { selectedPath: command.selectedPath } : {}) }); + const content: WebuiWorkspaceTabContent = { + kind: "review", + source: "workspace", + sessionId: command.sessionId, + workspaceDir: command.workspaceDir, + ...(command.selectedPath ? { selectedPath: command.selectedPath } : {}), + }; + // The workspace's change set and a turn's review are two tabs. They used + // to derive the same id, so opening one replaced the other. + const existing = tabShowing( + state, + (candidate) => candidate.kind === "review" && candidate.source === "workspace", + ); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "open-turn-review": { - const id = tabId("review", command.sessionId, command.workspaceDir); - return activate(state, { id, kind: "review", source: "turn", sessionId: command.sessionId, workspaceDir: command.workspaceDir, messageId: command.messageId, assistantMessageId: command.assistantMessageId, turnId: command.turnId, changeSetId: command.changeSetId, files: command.files ?? [], ...(command.selectedPath ? { selectedPath: command.selectedPath } : {}) }); + const content: WebuiWorkspaceTabContent = { + kind: "review", + source: "turn", + sessionId: command.sessionId, + workspaceDir: command.workspaceDir, + messageId: command.messageId, + assistantMessageId: command.assistantMessageId, + turnId: command.turnId, + changeSetId: command.changeSetId, + files: command.files ?? [], + ...(command.selectedPath ? { selectedPath: command.selectedPath } : {}), + }; + const existing = tabShowing( + state, + (candidate) => candidate.kind === "review" && candidate.source === "turn", + ); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); + } + case "open-web-tab": { + const content: WebuiWorkspaceTabContent = { + kind: "web", + target: command.target, + title: command.title, + url: command.url, + }; + // A web tab is bound to its target, and a tab already bound to that + // target is the same tab: the runtime publishes a snapshot, and applying + // a snapshot twice changes nothing. + const existing = tabShowing( + state, + (candidate) => + candidate.kind === "web" && + candidate.target.browserId === command.target.browserId && + candidate.target.targetId === command.target.targetId, + ); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "open-primary-view": { - const id = tabId(command.kind, command.sessionId ?? "", command.workspaceDir ?? ""); - const tab: WorkspacePanelTab = { id, kind: command.kind, ...(command.sessionId ? { sessionId: command.sessionId } : {}), ...(command.workspaceDir ? { workspaceDir: command.workspaceDir } : {}) }; - const tabs = state.tabs.filter((candidate) => candidate.kind !== "files"); - return { ...state, open: true, addMenuOpen: false, tabs: [...tabs, tab], activeTabId: id }; + const content: WebuiWorkspaceTabContent = { + kind: "files", + ...(command.sessionId ? { sessionId: command.sessionId } : {}), + ...(command.workspaceDir ? { workspaceDir: command.workspaceDir } : {}), + }; + // The files view is a singleton per session and directory, and that is + // an explicit rule now: the id no longer implies it, because the id + // says nothing about what the tab is showing. + const existing = tabShowing(state, (candidate) => isSameInstance(candidate, "files", command.sessionId, command.workspaceDir)); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "open-tab": { - const id = tabId(command.kind, command.sessionId ?? "", command.workspaceDir ?? ""); - return activate(state, { id, kind: command.kind, ...(command.sessionId ? { sessionId: command.sessionId } : {}), ...(command.workspaceDir ? { workspaceDir: command.workspaceDir } : {}) }); + const content: WebuiWorkspaceTabContent = { + kind: command.kind, + ...(command.sessionId ? { sessionId: command.sessionId } : {}), + ...(command.workspaceDir ? { workspaceDir: command.workspaceDir } : {}), + }; + const existing = tabShowing(state, (candidate) => isSameInstance(candidate, command.kind, command.sessionId, command.workspaceDir)); + if (existing) return activate(state, { ...existing, content }); + return openNewTab(state, content); } case "select-tab": return state.tabs.some((tab) => tab.id === command.tabId) ? { ...state, open: true, activeTabId: command.tabId, addMenuOpen: false } : state; case "select-review-file": - return updateWorkspacePanelTab(state, command.tabId, (tab) => tab.kind === "review" ? { ...tab, selectedPath: command.path } : tab); + return updateWorkspacePanelTab(state, command.tabId, (tab) => + tab.content.kind === "review" ? { ...tab, content: { ...tab.content, selectedPath: command.path } } : tab, + ); case "set-review-snapshot": { const current = state.tabs.find((tab) => tab.id === command.tabId); - if (current?.kind !== "review" || current.source !== "workspace" || current.reviewSnapshotId === command.reviewSnapshotId) return state; - return updateWorkspacePanelTab(state, command.tabId, (tab) => tab.kind === "review" ? { ...tab, reviewSnapshotId: command.reviewSnapshotId } : tab); + if ( + current?.content.kind !== "review" || + current.content.source !== "workspace" || + current.content.reviewSnapshotId === command.reviewSnapshotId + ) { + return state; + } + return updateWorkspacePanelTab(state, command.tabId, (tab) => + tab.content.kind === "review" + ? { ...tab, content: { ...tab.content, reviewSnapshotId: command.reviewSnapshotId } } + : tab, + ); } case "close-tab": { const index = state.tabs.findIndex((tab) => tab.id === command.tabId); diff --git a/packages/webui/src/client/features/workspace/ui/WorkspacePanels.tsx b/packages/webui/src/client/features/workspace/ui/WorkspacePanels.tsx index b56ea9cd9..7c377e208 100644 --- a/packages/webui/src/client/features/workspace/ui/WorkspacePanels.tsx +++ b/packages/webui/src/client/features/workspace/ui/WorkspacePanels.tsx @@ -22,7 +22,6 @@ import { WebuiMarkdown } from "../../transcript/ui/markdown.public.js"; // The browser panel is another business's published surface. The workspace // may mount it and hand it a surface; it may not construct the browser // state itself, which is why the host component exists. -import { BrowserPanelHost } from "../../browser/ui/BrowserPanelHost.public.js"; import type { WebuiFileDiffInfoView } from "../../../../shared/contracts/session.js"; import type { WebuiWorkspaceFile, @@ -33,6 +32,7 @@ import type { WebuiWorkspaceReviewSummary, } from "../../../../shared/contracts/review.js"; import type { + WebuiWorkspaceTabContent, WebuiWorkspaceTabKind, WorkspacePanelCommand, WorkspacePanelState, @@ -40,6 +40,8 @@ import type { } from "../projection/workspace-panel-state.js"; import { webuiWorkspaceAddMenu, + webuiWorkspaceReviewContent, + webuiWorkspaceTabContentKey, webuiWorkspaceTabLabel, } from "../projection/workspace-panel-state.js"; import { focusWebuiFileLine, webuiFileLineTargetId } from "../../../bindings/browser-effects.js"; @@ -142,36 +144,37 @@ export function WebuiDiffFileSection({ file, selected, loading = false, error, o ; } -export function WebuiFilePreview({ tab, result, codeMode, workspaceFileUrl, }: { - readonly tab: Extract; +export function WebuiFilePreview({ tabId, content, result, codeMode, workspaceFileUrl, }: { + readonly tabId: string; + readonly content: Extract; readonly result?: { readonly loading: boolean; readonly content?: WebuiWorkspaceFileContent; readonly error?: string }; readonly codeMode: boolean; readonly workspaceFileUrl?: (request: { readonly workspaceDir: string; readonly path: string }) => string; }): ReactElement { - const previewMarkdown = /\.md$/iu.test(tab.path) && !codeMode; - const content = result?.content?.content ?? ""; - const language = webuiFileLanguage(tab.path); + const previewMarkdown = /\.md$/iu.test(content.path) && !codeMode; + const text = result?.content?.content ?? ""; + const language = webuiFileLanguage(content.path); // Dispatched ahead of the read result on purpose: an archive never becomes // text, and an HTML preview is a stream the browser fetches rather than a // string the panel holds. Waiting on `readWorkspaceFile` first would either // flash a binary error or buffer a document nobody will read. - const archive = isWorkspaceArchivePath(tab.path) - ? + const archive = isWorkspaceArchivePath(content.path) + ? : undefined; - const html = !archive && isWorkspaceHtmlPath(tab.path) && workspaceFileUrl - ? + const html = !archive && isWorkspaceHtmlPath(content.path) && workspaceFileUrl + ? : undefined; - return
+ return
{archive ?? html ?? (result?.loading ?

正在加载文件…

: result?.error ?

{result.error}

- : result?.content && isMediaContent(result.content, tab.path) ? + : result?.content && isMediaContent(result.content, content.path) ? : result?.content?.error ?

{result.content.error}

: result?.content?.type === "binary" ?

无法在文本预览中显示二进制文件。

- : result?.content ? previewMarkdown ? : 
{content.split("\n").map((line, index) => {
+              : result?.content ? previewMarkdown ?  : 
{text.split("\n").map((line, index) => {
                 const lineNumber = index + 1;
                 const highlighted = highlightFileLine(line, language);
-                return = (tab.lineStart ?? 0) && lineNumber <= (tab.lineEnd ?? tab.lineStart ?? 0) ? "webui-file-preview-line-active" : ""}`} data-line-number={lineNumber} {...(highlighted !== undefined ? { dangerouslySetInnerHTML: { __html: highlighted || " " } } : {})}>{highlighted === undefined ? line || " " : undefined};
+                return = (content.lineStart ?? 0) && lineNumber <= (content.lineEnd ?? content.lineStart ?? 0) ? "webui-file-preview-line-active" : ""}`} data-line-number={lineNumber} {...(highlighted !== undefined ? { dangerouslySetInnerHTML: { __html: highlighted || " " } } : {})}>{highlighted === undefined ? line || " " : undefined};
               })}

                 : 
文件读取能力暂不可用。
)}
; @@ -203,7 +206,7 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, const reviewDiffs = queryState.reviewDiffs; const reviewSearch = queryState.reviewSearch; const activeTab = state.tabs.find((tab) => tab.id === state.activeTabId); - const tab = activeTab?.kind ?? "files"; + const tab = activeTab?.content.kind ?? "files"; const tabListRef = useRef(null); const fileTreeScrollRef = useRef(null); const autoNavigationRequests = useRef(new Set()); @@ -285,12 +288,17 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, * grid it was fitted to. */ const reportedTerminalSize = useRef(); - const activeWorkspace = activeTab && "workspaceDir" in activeTab ? activeTab.workspaceDir : workspaceDir; + const activeWorkspace = activeTab && "workspaceDir" in activeTab.content ? activeTab.content.workspaceDir : workspaceDir; const activeWorkspaceRef = useRef(activeWorkspace); activeWorkspaceRef.current = activeWorkspace; - const activeFileResult = activeTab?.kind === "file-preview" ? fileResults.get(activeTab.id)?.content : undefined; - const selectedFilePath = activeTab?.kind === "file-preview" ? activeFileResult?.resolvedPath ?? activeTab.path : undefined; - const hasFileWorkspace = tab === "files" || activeTab?.kind === "file-preview" || activeTab?.kind === "review"; + const activeFilePath = activeTab?.content.kind === "file-preview" ? activeTab.content.path : undefined; + // The file results are keyed by what the tab is showing, not by which slot + // it is: the id is stable and the content is not, so a cache keyed by the id + // would hand the previous file's content to the new one. + const activeFileKey = activeTab?.content.kind === "file-preview" ? webuiWorkspaceTabContentKey(activeTab.content) : undefined; + const activeFileResult = activeTab?.content.kind === "file-preview" ? fileResults.get(activeFileKey!)?.content : undefined; + const selectedFilePath = activeTab?.content.kind === "file-preview" ? activeFileResult?.resolvedPath ?? activeTab.content.path : undefined; + const hasFileWorkspace = tab === "files" || activeTab?.content.kind === "file-preview" || activeTab?.content.kind === "review"; const visibleFiles = filterWorkspaceFiles(fileTree.files ?? [], fileSearch); const currentPanelState = useRef(state); currentPanelState.current = state; @@ -302,7 +310,7 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, setExpandedDirectories(new Set()); }, [activeWorkspace, queries]); useEffect(() => { - setFileCodeMode(activeTab?.kind === "file-preview" && activeTab.lineStart !== undefined); + setFileCodeMode(activeTab?.content.kind === "file-preview" && activeTab.content.lineStart !== undefined); }, [activeTab?.id]); useEffect(() => { tabListRef.current?.querySelector('[aria-selected="true"]')?.scrollIntoView({ block: "nearest", inline: "nearest" }); @@ -312,48 +320,53 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, queries?.loadFileTree(activeWorkspace); }, [activeWorkspace, queries]); useEffect(() => { - if (activeTab?.kind !== "file-preview" || !queries) return undefined; + if (activeTab?.content.kind !== "file-preview" || !queries) return undefined; // A tab opened with inline content (the session's plan file) has nothing // to read: `readWorkspaceFile` would reject the path anyway, since the // artifacts directory is outside the workspace root. - if (activeTab.content !== undefined) { - queries.setInlineFile(activeTab.id, activeTab.content, activeTab.path); + if (activeTab.content.content !== undefined) { + queries.setInlineFile(activeFileKey!, activeTab.content.content, activeTab.content.path); return undefined; } - queries.readFile(activeTab.id, activeTab.workspaceDir, activeTab.path); + queries.readFile(activeFileKey!, activeTab.content.workspaceDir, activeTab.content.path); return undefined; - }, [activeTab?.id, queries]); + // The content is a dependency, not just the id: the id is stable and the + // content is not, so a tab that changed from one file to another has to + // read the new one. + }, [activeTab?.id, activeFileKey, activeTab?.content, queries]); useEffect(() => { - if (activeTab?.kind !== "file-preview" || activeTab.lineStart === undefined || fileResults.get(activeTab.id)?.loading || !fileResults.get(activeTab.id)?.content || fileResults.get(activeTab.id)?.error) return; - const target = dom.getElementById(webuiFileLineTargetId(activeTab.id, activeTab.lineStart)); + if (activeTab?.content.kind !== "file-preview" || activeTab.content.lineStart === undefined || fileResults.get(activeFileKey!)?.loading || !fileResults.get(activeFileKey!)?.content || fileResults.get(activeFileKey!)?.error) return; + const target = dom.getElementById(webuiFileLineTargetId(activeTab.id, activeTab.content.lineStart)); if (target) focusWebuiFileLine(target); - }, [activeTab?.id, activeTab?.kind === "file-preview" ? activeTab.lineStart : undefined, dom, fileResults]); + }, [activeTab?.id, activeFileKey, activeTab?.content.kind === "file-preview" ? activeTab.content.lineStart : undefined, dom, fileResults]); useEffect(() => { - if (activeTab?.kind !== "review" || activeTab.source !== "workspace" || !queries) return undefined; - void queries.loadReviewSummary(activeTab.id, activeTab.workspaceDir).then((summary) => { + if (activeTab?.content.kind !== "review" || activeTab.content.source !== "workspace" || !queries) return undefined; + void queries.loadReviewSummary(activeTab.id, activeTab.content.workspaceDir).then((summary) => { if (summary) dispatch({ type: "set-review-snapshot", tabId: activeTab.id, reviewSnapshotId: summary.reviewSnapshotId }); }); return undefined; - }, [activeTab?.id, activeTab?.kind === "review" && activeTab.source === "workspace" ? activeTab.workspaceDir : undefined, queries, queryState.reviewRevision]); - const workspaceReviewSelectedPath = activeTab?.kind === "review" && activeTab.source === "workspace" ? activeTab.selectedPath : undefined; + // The content is a dependency here too: a tab that changes from a turn's + // review to the workspace's review has to load the summary it now needs. + }, [activeTab?.id, activeTab?.content, activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? activeTab.content.workspaceDir : undefined, queries, queryState.reviewRevision]); + const workspaceReviewSelectedPath = activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? activeTab.content.selectedPath : undefined; const workspaceReviewFiles = reviewSummary !== undefined && reviewSummary.tabId === activeTab?.id ? reviewSummary.summary?.files ?? [] : []; - const turnReviewFiles = activeTab?.kind === "review" && activeTab.source === "turn" ? activeTab.files ?? [] : []; - const selectedReviewPath = activeTab?.kind === "review" && activeTab.source === "workspace" + const turnReviewFiles = activeTab?.content.kind === "review" && activeTab.content.source === "turn" ? activeTab.content.files ?? [] : []; + const selectedReviewPath = activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? workspaceReviewFiles.some((file) => file.path === workspaceReviewSelectedPath) ? workspaceReviewSelectedPath : workspaceReviewFiles[0]?.path - : activeTab?.kind === "review" && activeTab.source === "turn" ? activeTab.selectedPath ?? turnReviewFiles[0]?.file : undefined; + : activeTab?.content.kind === "review" && activeTab.content.source === "turn" ? activeTab.content.selectedPath ?? turnReviewFiles[0]?.file : undefined; useEffect(() => { - if (activeTab?.kind !== "review" || activeTab.source !== "workspace" || !selectedReviewPath || selectedReviewPath === activeTab.selectedPath) return; + if (activeTab?.content.kind !== "review" || activeTab.content.source !== "workspace" || !selectedReviewPath || selectedReviewPath === activeTab.content.selectedPath) return; dispatch({ type: "select-review-file", tabId: activeTab.id, path: selectedReviewPath }); - }, [activeTab?.id, activeTab?.kind === "review" && activeTab.source === "workspace" ? activeTab.selectedPath : undefined, selectedReviewPath, dispatch]); + }, [activeTab?.id, activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? activeTab.content.selectedPath : undefined, selectedReviewPath, dispatch]); useEffect(() => { const summary = reviewSummary && reviewSummary.tabId === activeTab?.id ? reviewSummary.summary : undefined; - if (activeTab?.kind !== "review" || activeTab.source !== "workspace" || !summary || !activeTab.reviewSnapshotId || summary.reviewSnapshotId !== activeTab.reviewSnapshotId || !queries) return undefined; - void queries.loadReviewDiffs({ tabId: activeTab.id, workspaceDir: activeTab.workspaceDir, snapshotId: activeTab.reviewSnapshotId, fileIds: summary.files.map((file) => file.fileId) }); + if (activeTab?.content.kind !== "review" || activeTab.content.source !== "workspace" || !summary || !activeTab.content.reviewSnapshotId || summary.reviewSnapshotId !== activeTab.content.reviewSnapshotId || !queries) return undefined; + void queries.loadReviewDiffs({ tabId: activeTab.id, workspaceDir: activeTab.content.workspaceDir, snapshotId: activeTab.content.reviewSnapshotId, fileIds: summary.files.map((file) => file.fileId) }); return undefined; - }, [activeTab?.id, activeTab?.kind === "review" && activeTab.source === "workspace" ? activeTab.reviewSnapshotId : undefined, activeTab?.kind === "review" && activeTab.source === "workspace" ? activeTab.workspaceDir : undefined, reviewSummary?.summary?.reviewSnapshotId, queries]); + }, [activeTab?.id, activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? activeTab.content.reviewSnapshotId : undefined, activeTab?.content.kind === "review" && activeTab.content.source === "workspace" ? activeTab.content.workspaceDir : undefined, reviewSummary?.summary?.reviewSnapshotId, queries]); const runReviewSearch = () => { - if (activeTab?.kind !== "review" || activeTab.source !== "workspace" || !activeTab.reviewSnapshotId || !queries) return; - void queries.searchReviewDiffs({ tabId: activeTab.id, workspaceDir: activeTab.workspaceDir, snapshotId: activeTab.reviewSnapshotId, query: reviewSearchQuery }); + if (activeTab?.content.kind !== "review" || activeTab.content.source !== "workspace" || !activeTab.content.reviewSnapshotId || !queries) return; + void queries.searchReviewDiffs({ tabId: activeTab.id, workspaceDir: activeTab.content.workspaceDir, snapshotId: activeTab.content.reviewSnapshotId, query: reviewSearchQuery }); }; const toggleWorkspaceDirectory = (directory: WebuiWorkspaceFile) => { const path = directory.path; @@ -371,14 +384,14 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, void queries.loadDirectory(activeWorkspace, path); }; useEffect(() => { - if (activeTab?.kind !== "file-preview" || !selectedFilePath || !activeWorkspace || !queries || fileTree.workspaceDir !== activeWorkspace || fileTree.loading) return; + if (activeTab?.content.kind !== "file-preview" || !selectedFilePath || !activeWorkspace || !queries || fileTree.workspaceDir !== activeWorkspace || fileTree.loading) return; let cancelled = false; const isCurrentFile = () => { const currentTab = currentPanelState.current.tabs.find((candidate) => candidate.id === currentPanelState.current.activeTabId); return activeWorkspaceRef.current === activeWorkspace - && currentTab?.kind === "file-preview" + && currentTab?.content.kind === "file-preview" && currentTab.id === activeTab.id - && (filesRef.current.get(currentTab.id)?.content?.resolvedPath ?? currentTab.path) === selectedFilePath; + && (filesRef.current.get(currentTab.id)?.content?.resolvedPath ?? currentTab.content.path) === selectedFilePath; }; const navigateToFile = async () => { setFileSearch(""); @@ -414,11 +427,12 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, }; void navigateToFile(); return () => { cancelled = true; }; - }, [activeTab?.id, activeTab?.kind === "file-preview" ? activeTab.path : undefined, selectedFilePath, activeWorkspace, fileTree.workspaceDir, fileTree.loading, queries]); + }, [activeTab?.id, activeTab?.content.kind === "file-preview" ? activeTab.content.path : undefined, selectedFilePath, activeWorkspace, fileTree.workspaceDir, fileTree.loading, queries]); useEffect(() => { fileTreeScrollRef.current?.querySelector('[aria-current="true"]')?.scrollIntoView({ block: "nearest", inline: "nearest" }); }, [selectedFilePath, selectedReviewPath, expandedDirectories, visibleFiles]); - const activeSessionId = activeTab && "sessionId" in activeTab ? activeTab.sessionId : sessionId; + const activeSessionId = activeTab && "sessionId" in activeTab.content ? activeTab.content.sessionId : sessionId; + const activeReview = webuiWorkspaceReviewContent(activeTab?.content); useEffect(() => { if (!terminalOwner) { setTerminals([]); @@ -507,14 +521,14 @@ export function WebuiWorkspacePanel({ state, dispatch, sessionId, workspaceDir, } }; const fileTabGlyph = (item: WorkspacePanelTab) => { - if (item.kind === "files") return ; - if (item.kind !== "file-preview") return null; - const extension = item.path.split(/[./\\]/u).at(-1)?.toLowerCase(); + if (item.content.kind === "files") return ; + if (item.content.kind !== "file-preview") return null; + const extension = item.content.path.split(/[./\\]/u).at(-1)?.toLowerCase(); if (extension === "md" || extension === "mdx") return ; if (["js", "jsx", "mjs", "cjs"].includes(extension ?? "")) return ; if (["ts", "tsx", "mts", "cts"].includes(extension ?? "")) return ; if (extension === "json") return ; - return ; + return ; }; return