Add bounded Browser page runtime and agent control - #1869
Conversation
…me-final-thr_p9qmfkggwm
…me-final-thr_p9qmfkggwm
…me-final-thr_p9qmfkggwm # Conflicts: # apps/app/src/components/secondary-panel/BrowserTabContent.tsx # apps/app/src/lib/plugin-slots.ts # apps/desktop/src/desktop-browser-ipc.ts # apps/desktop/src/desktop-browser-view.ts # apps/desktop/src/preload.ts # apps/desktop/test/desktop-browser-main-ipc.test.ts # apps/desktop/test/preload-browser-api.test.ts # apps/server/src/ws/client-protocol.ts # apps/server/src/ws/hub.ts # packages/domain/src/change-kinds.ts # packages/templates/src/generated/plugin-sdk-dts.generated.ts
|
🚨 SLOP COP 🚨 · I am SlopCop. I started parallel security, code quality, architecture, and performance reviews. I will also test the Browser workflow where possible. I will post one final review after I check the results. |
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain English summary: This PR lets approved tools control one exact Browser tab. They can inspect, click, type, scroll, navigate, run bounded code, and capture images.
The server, web app, and desktop shell match each request to one page revision. Plugins also gain a compact Browser toolbar action.
I found six issues:
- High: One script timeout can terminate unrelated scripts and page code in the same tab.
- High: Normal navigation leaves a short window where a script can run against the page that the user is leaving.
- Medium: A large DOM snapshot can stop the page renderer and delay cancellation.
- Medium: Screenshot cancellation does not stop the native capture task.
- Medium: Backspace, Delete, and arrow-key actions report success without native browser behavior.
- Medium: Deep recursive response data can escape the invalid-message guard and leave the WebSocket open.
I found no earlier Browser runtime that this change should reuse. A tab operation manager can centralize navigation state, cancellation, and hard limits.
Validation passed for all six affected package typechecks. The full desktop suite passed 231 tests, and the two separate test runs passed.
The native end-to-end test loaded Example Domain. The page runtime returned its heading, and image capture returned the same navigation epoch.
I left line comments with the trigger and a testable fix for each issue.
Summary
experimental_browserActionentry point without retaining the discarded Browser-specific inspection schemaSecurity and lifecycle
AbortSignalremains renderer-local; cancellation crosses Electron IPC only as a bounded{ tabId, requestId }messageVisual evidence
The runtime comparison uses the same 1280×900 desktop dev-app window,
Browser runtime visual QAthread, Browser Core Test Lab fixture, Team members route, localhost URL, Browser panel width, and device scale. Before is the exact merge basefa9a15ba; after is the exact PR headdd66e0893.Browser action: before
State: the localhost Team members page is open as a complete Browser page. The full
http://127.0.0.1:43173/URL is visible, and the merge base has no plugin Browser action.Browser action: after
Action: select Inspect this Browser page in the Browser toolbar.
Result: the same complete localhost page remains visible while a compact, non-modal result reports the live heading, control counts, input count, and navigation epoch for that exact tab revision.
Overflow interaction sequence
These two captures are both from exact head
dd66e0893in the same 1280×900 window. The Browser panel is deliberately constrained so only actions 2–4 fit inline.Step 1 — trigger in context: the overflow trigger appears immediately before the host external-link action, which remains rightmost. No hover-only state is required.
Step 2 — open menu: focus the overflow trigger and press Enter. The menu opens with ordered actions 5–8 while the trigger and external-link action remain visible in context.
Validation
Team members, 3 buttons, 1 input, and navigation epoch 1{ filter: "Dara", activity: "Invite member clicked through the Browser runtime.", scrollY: 420 }with a 778×1336 capturegit diff --checkpassedSupersedes the Browser runtime portion of #1642 and #1643. Does not merge or ship the Browser Context consumer.
BB-Thread-ID: thr_p9qmfkggwm