[codex] Add integrated browser preview, annotations, and agent automation#3053
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
Contributor
|
🚀 Expo continuous deployment is ready!
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
changed the title
Comment on lines
+4243
to
+4249
| {shouldUsePlanSidebarSheet && previewPanelOpen && activeThreadRef ? ( | ||
| <RightPanelSheet open onClose={closePreviewPanel}> | ||
| <Suspense fallback={null}> | ||
| <PreviewPanel mode="sheet" threadRef={activeThreadRef} visible /> | ||
| </Suspense> | ||
| </RightPanelSheet> | ||
| ) : null} |
Contributor
There was a problem hiding this comment.
🟢 Low components/ChatView.tsx:4243
The mobile preview sheet at line 4243 conditionally renders based on previewPanelOpen, so it unmounts instantly when closed. The plan sidebar sheet at line 4250 stays mounted with open={planSidebarOpen}, allowing the @base-ui/react Sheet closing animation to play. This causes the preview panel to disappear jarringly on mobile instead of animating smoothly like the plan sidebar.
- {shouldUsePlanSidebarSheet && previewPanelOpen && activeThreadRef ? (
+ {shouldUsePlanSidebarSheet && activeThreadRef ? (
<RightPanelSheet open onClose={closePreviewPanel}>
<Suspense fallback={null}>
- <PreviewPanel mode="sheet" threadRef={activeThreadRef} visible />
+ <PreviewPanel mode="sheet" threadRef={activeThreadRef} visible={previewPanelOpen} />
</Suspense>
</RightPanelSheet>
) : null}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatView.tsx around lines 4243-4249:
The mobile preview sheet at line 4243 conditionally renders based on `previewPanelOpen`, so it unmounts instantly when closed. The plan sidebar sheet at line 4250 stays mounted with `open={planSidebarOpen}`, allowing the `@base-ui/react` `Sheet` closing animation to play. This causes the preview panel to disappear jarringly on mobile instead of animating smoothly like the plan sidebar.
Evidence trail:
apps/web/src/components/ChatView.tsx lines 4243-4264 (REVIEWED_COMMIT) — preview panel conditional mount vs. plan sidebar staying mounted.
apps/web/src/components/RightPanelSheet.tsx lines 6-29 (REVIEWED_COMMIT) — `keepMounted` on SheetPopup, `open` prop passed through to `Sheet`.
apps/web/src/components/ui/sheet.tsx line 3 — imports `@base-ui/react/dialog` as the Sheet primitive.
| workspaceRoot={activeWorkspaceRoot} | ||
| timestampFormat={timestampFormat} | ||
|
|
||
| {!shouldUsePlanSidebarSheet && |
Contributor
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:4243
On viewports wider than 980px, the inline RightPanelTabs (lines 4243–4275) only renders children for "preview" and "diff" surface kinds. When activeRightPanelSurface.kind is "terminal" or "plan", the children expression falls through to null, leaving the panel tabs visible with an empty content area. The sheet (mobile) version at 4277–4334 handles these cases correctly with PersistentThreadTerminalDrawer and PlanSidebar. Consider adding the missing surface kind branches to the inline rendering block so terminal and plan panels render on desktop.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatView.tsx around line 4243:
On viewports wider than 980px, the inline `RightPanelTabs` (lines 4243–4275) only renders children for `"preview"` and `"diff"` surface kinds. When `activeRightPanelSurface.kind` is `"terminal"` or `"plan"`, the children expression falls through to `null`, leaving the panel tabs visible with an empty content area. The sheet (mobile) version at 4277–4334 handles these cases correctly with `PersistentThreadTerminalDrawer` and `PlanSidebar`. Consider adding the missing surface kind branches to the inline rendering block so terminal and plan panels render on desktop.
Evidence trail:
apps/web/src/components/ChatView.tsx lines 4243-4275 (inline block handles only 'preview' and 'diff', falls through to null); lines 4277-4334 (sheet block handles 'preview', 'terminal', 'diff', and 'plan'); apps/web/src/rightPanelStore.ts lines 19-24 (RightPanelSurface type includes 'terminal' and 'plan' kinds); apps/web/src/components/ChatView.tsx line 1095 (terminal surface opened unconditionally regardless of viewport); apps/web/src/rightPanelLayout.ts line 1 (media query is max-width: 980px)
Contributor
There was a problem hiding this comment.
🟡 Medium
In recoverSessionForThread, when the "adopt-existing" branch is taken (lines 371-386), the function returns early at line 386 without calling prepareMcpSession. Since McpProviderSession.sessionsByThread is an in-memory map that is empty after server restart, recovering a pre-existing session via this path leaves the MCP session configuration unset, causing subsequent MCP tool calls to fail. Consider calling prepareMcpSession before returning in the adopt-existing branch, or document if this omission is intentional.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/Layers/ProviderService.ts around line 376:
In `recoverSessionForThread`, when the "adopt-existing" branch is taken (lines 371-386), the function returns early at line 386 without calling `prepareMcpSession`. Since `McpProviderSession.sessionsByThread` is an in-memory map that is empty after server restart, recovering a pre-existing session via this path leaves the MCP session configuration unset, causing subsequent MCP tool calls to fail. Consider calling `prepareMcpSession` before returning in the adopt-existing branch, or document if this omission is intentional.
Evidence trail:
apps/server/src/provider/Layers/ProviderService.ts lines 355-438 (recoverSessionForThread function), specifically lines 371-386 (adopt-existing branch returns without calling prepareMcpSession) vs line 400 (resume-thread branch calls prepareMcpSession). apps/server/src/provider/Layers/ProviderService.ts lines 217-224 (prepareMcpSession definition). apps/server/src/mcp/McpProviderSession.ts lines 12-19 (in-memory Map and setMcpProviderSession/readMcpProviderSession). apps/server/src/provider/Layers/ClaudeAdapter.ts lines 3449, 3475-3487 (readMcpProviderSession consumed conditionally for mcpServers config).
Comment on lines
+48
to
+59
| export const PreviewOpenTool = browserTool( | ||
| Tool.make("preview_open", { | ||
| description: | ||
| "Show and initialize the browser preview for the scoped thread, optionally reusing its current tab and navigating to a URL.", | ||
| parameters: PreviewAutomationOpenInput, | ||
| success: PreviewAutomationStatus, | ||
| failure: PreviewAutomationError, | ||
| dependencies, | ||
| }) | ||
| .annotate(Tool.Title, "Open browser preview") | ||
| .annotate(Tool.Destructive, false), | ||
| ); |
Contributor
There was a problem hiding this comment.
🟢 Low preview/tools.ts:48
The .annotate(Tool.Destructive, false) on line 58 is overwritten by browserTool(), which calls .annotate(Tool.Destructive, true) last. The final tool has Destructive: true instead of the intended false. Consider using safeBrowserTool() instead, which preserves Destructive: false as the final annotation.
-export const PreviewOpenTool = browserTool(
- Tool.make("preview_open", {
- description:
- "Show and initialize the browser preview for the scoped thread, optionally reusing its current tab and navigating to a URL.",
- parameters: PreviewAutomationOpenInput,
- success: PreviewAutomationStatus,
- failure: PreviewAutomationError,
- dependencies,
- })
- .annotate(Tool.Title, "Open browser preview")
- .annotate(Tool.Destructive, false),
-);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/mcp/toolkits/preview/tools.ts around lines 48-59:
The `.annotate(Tool.Destructive, false)` on line 58 is overwritten by `browserTool()`, which calls `.annotate(Tool.Destructive, true)` last. The final tool has `Destructive: true` instead of the intended `false`. Consider using `safeBrowserTool()` instead, which preserves `Destructive: false` as the final annotation.
Evidence trail:
apps/server/src/mcp/toolkits/preview/tools.ts lines 27-31 (browserTool and safeBrowserTool definitions), lines 48-59 (PreviewOpenTool using browserTool with .annotate(Tool.Destructive, false) on line 58 that gets overwritten), lines 61-70 (PreviewNavigateTool correctly using safeBrowserTool for comparison).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review 5 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
Contributor
There was a problem hiding this comment.
🟠 High
t3code/apps/web/src/composerDraftStore.ts
Line 1709 in 756b59c
normalizePersistedDraftsByThreadId reads previewAnnotations from persisted data but never includes them in the returned nextDraftsByThreadKey object. After a page reload, previewAnnotations are silently dropped because the normalization step omits the field, so toHydratedThreadDraft receives an undefined value even though the data exists in storage.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/composerDraftStore.ts around line 1709:
`normalizePersistedDraftsByThreadId` reads `previewAnnotations` from persisted data but never includes them in the returned `nextDraftsByThreadKey` object. After a page reload, `previewAnnotations` are silently dropped because the normalization step omits the field, so `toHydratedThreadDraft` receives an undefined value even though the data exists in storage.
Evidence trail:
apps/web/src/composerDraftStore.ts lines 125-145 (PersistedComposerThreadDraftState schema with previewAnnotations at line 130), lines 1576-1726 (normalizePersistedDraftsByThreadId function — previewAnnotations never extracted or included), lines 1709-1722 (output object missing previewAnnotations), lines 1687-1694 (emptiness check missing previewAnnotations), lines 1837-1843 (serialization correctly includes previewAnnotations), lines 2084-2085 (toHydratedThreadDraft reads previewAnnotations with ?? [] fallback)
Adds a desktop-only browser preview that lives in the right panel slot alongside plan/diff. Lets the user point an Electron <webview> at any URL — typed into a chrome-style URL bar, clicked from the empty-state list of detected localhost dev servers, or auto-opened by a project script with `previewUrl` set. Single-tab per thread. Server (Effect/Layers): - PreviewManager: per-(thread, tab) session metadata via SynchronizedRef + PubSub<PreviewEvent>; survives WS reconnect via `list`/replay. - PreviewPortScanner: lsof on macOS/Linux, TCP probe fallback on Windows; reference-counted polling so we only scan when subscribed. - WS RPC + streams (`preview.open|navigate|refresh|close|list|reportStatus`, `subscribePreviewEvents`, `subscribeDiscoveredLocalServers`). Desktop: - PreviewViewManager owns Chromium WebContents per tab, mediates navigation/zoom/devtools/clear-storage. registerWebview gates by webContents.getType() === "webview" and host-window match. - IPC channels for create/close/register/navigate/back/forward/refresh/ zoom/hardReload/openDevTools/clearCookies/clearCache/getBrowserPartition. - Forwards app-level shortcuts (mod+shift+J, mod+K, mod+,, mod+W) from the webview back to the main window. - Persisted browser session partition (cookies, cache). Web: - PreviewPanel/PreviewView/PreviewWebview render the surface; chrome row with back/forward/refresh + URL input + Open-in-browser + 3-dot menu (Hard reload, DevTools, Zoom −/+/reset, Clear cookies/cache). - usePreviewSession subscribes to server events; usePreviewBridge mirrors desktop state into the store and forwards Loading→Success/ LoadFailed back to the server. - previewStateStore: per-thread snapshot + desktopOverlay + recently- seen URLs (Zustand). - rightPanelStore arbitrates plan vs. preview vs. diff; ChatView's toggles strip the `?diff=1` URL hint when switching to preview and vice versa so the panels are mutually exclusive. - Top-nav Globe toggle in ChatHeader (desktop builds only) and a `mod+shift+J` keybinding routed via a typed previewActionBus. - PreviewEmptyState lists detected localhost servers (scanner + configured project URLs + recently-seen) with live "listening" pulse. - PreviewUnreachable: theme-aware port of Chromium's "site can't be reached" page. - Resizable inline panel (RightPanelResizeHandle + useResizableWidth); width persists to localStorage on drag-end. - Terminal link "Open in preview" context-menu integration for loopback URLs. Contracts: - preview.ts schemas (PreviewSessionSnapshot, PreviewNavStatus, PreviewEvent, RPC inputs/results, DiscoveredLocalServer). - ProjectScript schema gains optional `previewUrl` + `autoOpenPreview`. - New keybinding commands: preview.toggle/refresh/focusUrl/zoomIn/Out/ resetZoom; new `when:` contexts `previewFocus` / `previewOpen`. Shared: - @t3tools/shared/preview: normalizePreviewUrl, isPreviewableUrl, isLoopbackHost, newPreviewTabId, LSOF_LOCAL_HOST_TOKENS. Tests: - contracts: schema decode tests for all preview events/snapshots/inputs. - shared: URL normalization coverage. - server: PreviewManager (open/navigate/reportStatus/refresh/close, multi-subscriber isolation, idempotency); PortScanner (lsof parsing including IPv6, TCP probe, reference-counted polling). - web: previewStateStore (per-tab event application, dedupe, reconnect recovery); rightPanelStore arbitration.
Adds an in-page element picker to the preview browser. Clicking the crosshair button in the chrome row activates a blue-highlight picker inside the guest webview; clicking an element captures its component name (via react-grab), source location, html/css preview, and selector, then attaches it to the chat composer as a chip that serializes into an `<element_context>` block in the outgoing message. Architecture: - Per-`<webview>` preload bundle (`preview-pick-preload.cjs`) renders the overlay, hosts the picker event loop, and bubbles the picked payload back to main via the per-WebContents `wc.ipc` channel (not `sendToHost`, which only fires on the host renderer's <webview> element and never reaches main). - Main coordinates via `PreviewViewManager.pickElement(tabId)`, which cancels any in-flight session, force-focuses the guest (so the first click on a remote page actually reaches the preload), then awaits the payload. User-initiated cancels (Escape, beforeunload) echo `null` back to main; main-initiated cancels and supersession tear down silently to avoid the new-pick-resolves-with-stale-null race. - Renderer fetches partition + webPreferences + preload URL in a single `getPreviewConfig()` IPC call, snapshots the previously-focused host element before triggering a pick, and restores focus when the pick resolves so the user's textarea cursor isn't lost. Security posture for the guest webview: - `webpreferences="contextIsolation=false,sandbox=true,nodeIntegration=false"` centralized in `preview-webview-preferences.ts`. contextIsolation off is required so react-grab's `getElementContext` can reach the page's React DevTools hook on `globalThis`. sandbox stays on so the page cannot reach Node APIs even with shared globals (without it, the preload's `require` would land on the page's `globalThis` and any third-party site could send arbitrary IPC to main). - Defense in depth: a `will-attach-webview` handler in main, gated on the preview partition, force-pins `sandbox: true`, all `nodeIntegration*: false`, and the absolute preload PATH (not URL — that field rejects file:// URLs with "preload script must have absolute path" and silently disables the picker). Composer + transcript integration: - New `elementContexts` slice in `composerDraftStore` (mirrors the terminal-context slice: dedup by selector+tag+component+url, persist via partializer, restore on send-failure retry). - `ComposerPendingElementContexts` chip row above the editor. - `deriveDisplayedUserMessageState` now strips both `<element_context>` AND `<terminal_context>` blocks (element first, since it's appended last) and exposes element entries to `MessagesTimeline`, which renders them as compact chips beneath the message body. - Pick button is disabled with explanatory tooltip when the page failed to load (the React `<PreviewUnreachable>` overlay covers the webview, so picks would silently dangle otherwise). Tests added: - `preview-webview-preferences.test.ts` locks down the security flags (contextIsolation=false, sandbox=true, nodeIntegration=false, no whitespace, only true/false literal values). - `preview-pick-label-position.test.ts` covers the floating-label clamp/flip math (no off-screen overflow, flip-below when no room above, etc.). - `picked-element-payload.test.ts` validator coverage. - `elementContext.test.ts` for the serialization round-trip, normalization, dedup, and label formatting. - `composerDraftStore.test.ts` element-contexts slice (add, dedup, remove, set, clear, persistence round-trip). - `ChatView.logic.test.ts` sendable-content-with-element-only. Build: new `tsdown` entry inlines react-grab + bippy into the picker preload bundle (~59KB / 19KB gzipped).
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Add structured annotation payload validation and tests - Update preview preload to capture selected elements, regions, and strokes - Wire new preview annotation UI into the web app Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Add IPC and runtime plumbing for preview annotation theming - Generate and ship annotation CSS for the desktop overlay - Add pointer and artifact handling for browser preview interactions
- Move MCP session registry and preview broker out of `Layers/` and `Services/` - Update imports, tests, and server wiring to use the new module layout
- Move preview session and IPC wiring into the new preview module - Tighten IPC validation with schema-based handlers - Update preview asset paths and tests for the browser preview port
- derive preview partitions through `BrowserSession` - serialize session state and async preview control flow - update tests for screenshot, automation, and partition behavior
- Tie preview and debugger listeners to Effect scopes - Factor shared automation helpers for snapshot and input handling - Improve cleanup for browser preview sessions and port scanning
- Fetch preview sessions through atom-backed SWR state - Recover browser preview sessions after reconnects - Ignore older streamed snapshots when SWR revalidates
- Track preview store revisions per thread - Ignore stale SWR results while revalidating - Avoid restoring closed sessions from outdated data
- Replace attachment and favicon routes with signed asset URLs - Harden workspace and attachment asset resolution - Update browser preview components and shared contracts
- Add phase 0 and 0.5 ADRs, findings, and spike notes - Update browser preview docs and supporting UI/test files - Record the chosen renderer, automation, recording, tunnel, and input decisions
…browser automation via CDP. Consolidate and refine the architecture and process models for the new preview automation framework, including updated contracts, server-side broker, and desktop integration. Introduce new WS methods for client communication and enhance security measures for token management. Ensure comprehensive testing strategies are in place for all components.
juliusmarminge
force-pushed
the
codex/browser-preview-port
branch
from
379529e to
4b60227
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4b60227. Configure here.
| // mod+, → settings (macOS convention) | ||
| { key: ",", meta: true, shift: false, control: false }, | ||
| // mod+W → close tab/panel | ||
| { key: "w", meta: true, shift: false, control: false }, |
Contributor
There was a problem hiding this comment.
Preview shortcuts ignore Ctrl on Windows
Medium Severity
Preview webview shortcut forwarding treats “mod” as the Meta key only, while the rest of the app uses Ctrl on Windows and Linux. With focus in the integrated preview, chords like Ctrl+K or Ctrl+W are not forwarded to the main window, so palette, settings, and close-tab shortcuts fail outside macOS.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4b60227. Configure here.
| }), | ||
| ); | ||
| yield* emit(tabId, closed); | ||
| }); |
Contributor
There was a problem hiding this comment.
Recording lock after tab close
Medium Severity
Closing a preview tab while screencast recording is active does not clear the internal “active recording tab” marker. startRecording then rejects any other tab with “Only one browser recording can be active per window,” and stopRecording on a new tab cannot clear a marker tied to the closed tab id.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4b60227. Configure here.
Comment on lines
+24
to
+39
| useEffect(() => { | ||
| if (Math.abs(lastFactorRef.current - zoomFactor) < ZOOM_EPSILON) return; | ||
| lastFactorRef.current = zoomFactor; | ||
| setVisible(true); | ||
| if (timerRef.current !== null) window.clearTimeout(timerRef.current); | ||
| timerRef.current = window.setTimeout(() => { | ||
| setVisible(false); | ||
| timerRef.current = null; | ||
| }, HIDE_AFTER_MS); | ||
| return () => { | ||
| if (timerRef.current !== null) { | ||
| window.clearTimeout(timerRef.current); | ||
| timerRef.current = null; | ||
| } | ||
| }; | ||
| }, [zoomFactor]); |
Contributor
There was a problem hiding this comment.
🟢 Low preview/ZoomIndicator.tsx:24
If zoom changes by less than ZOOM_EPSILON while a timer is running, the cleanup clears the old timer but the effect returns early at line 25 without starting a new one. visible remains true indefinitely until another large zoom change occurs, causing the indicator to stick on the screen.
useEffect(() => {
+ // Compare against incoming prop, not ref which was already updated
+ const diff = Math.abs(lastFactorRef.current - zoomFactor);
if (Math.abs(lastFactorRef.current - zoomFactor) < ZOOM_EPSILON) return;
lastFactorRef.current = zoomFactor;
setVisible(true);
if (timerRef.current !== null) window.clearTimeout(timerRef.current);
timerRef.current = window.setTimeout(() => {
setVisible(false);
timerRef.current = null;
}, HIDE_AFTER_MS);
return () => {
if (timerRef.current !== null) {
window.clearTimeout(timerRef.current);
timerRef.current = null;
}
};
}, [zoomFactor]);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/preview/ZoomIndicator.tsx around lines 24-39:
If zoom changes by less than `ZOOM_EPSILON` while a timer is running, the cleanup clears the old timer but the effect returns early at line 25 without starting a new one. `visible` remains `true` indefinitely until another large zoom change occurs, causing the indicator to stick on the screen.
Evidence trail:
apps/web/src/components/preview/ZoomIndicator.tsx lines 24-39 at REVIEWED_COMMIT. Line 25: early return when delta < ZOOM_EPSILON. Lines 33-38: cleanup clears timer. React's useEffect semantics: cleanup from previous invocation runs before new effect body, so previous timer is cleared before the early return skips starting a new one. `visible` (set to true on line 27 of previous invocation) is never reset to false.
| } | ||
| } | ||
|
|
||
| export async function stopBrowserRecording( |
Contributor
There was a problem hiding this comment.
🟡 Medium browser/browserRecording.ts:91
If bridge.recording.stopScreencast(tabId) or bridge.recording.save(...) throws, the cleanup at lines 109-113 never executes, leaving active non-null, unsubscribeFrames still subscribed, and the store still showing activeTabId as the recording tab. Since startBrowserRecording returns early when active is non-null (line 61), the user cannot start a new recording without refreshing. Wrap the cleanup in a finally block or add a try/catch that ensures cleanup runs before rethrowing.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/browser/browserRecording.ts around line 91:
If `bridge.recording.stopScreencast(tabId)` or `bridge.recording.save(...)` throws, the cleanup at lines 109-113 never executes, leaving `active` non-null, `unsubscribeFrames` still subscribed, and the store still showing `activeTabId` as the recording tab. Since `startBrowserRecording` returns early when `active` is non-null (line 61), the user cannot start a new recording without refreshing. Wrap the cleanup in a `finally` block or add a try/catch that ensures cleanup runs before rethrowing.
Evidence trail:
apps/web/src/browser/browserRecording.ts lines 91-115 (stopBrowserRecording function): line 97 `await bridge.recording.stopScreencast(tabId)` and line 104 `await bridge.recording.save(...)` can throw; cleanup at lines 109-113 (`active = null`, `unsubscribeFrames?.()`, store updates) is unreachable on error. Line 61 in startBrowserRecording: `if (!bridge || active) return;` prevents new recordings when `active` is stale. Lines 81-88 show the analogous try/catch pattern used in startBrowserRecording but absent from stopBrowserRecording. Commit: REVIEWED_COMMIT.
Comment on lines
+129
to
+148
| function unionRects( | ||
| rects: ReadonlyArray<PreviewAnnotationRect>, | ||
| padding = 20, | ||
| ): PreviewAnnotationRect | null { | ||
| if (rects.length === 0) return null; | ||
| const left = Math.min(...rects.map((rect) => rect.x)); | ||
| const top = Math.min(...rects.map((rect) => rect.y)); | ||
| const right = Math.max(...rects.map((rect) => rect.x + rect.width)); | ||
| const bottom = Math.max(...rects.map((rect) => rect.y + rect.height)); | ||
| const x = Math.max(0, left - padding); | ||
| const y = Math.max(0, top - padding); | ||
| const maxWidth = Math.max(1, window.innerWidth - x); | ||
| const maxHeight = Math.max(1, window.innerHeight - y); | ||
| return { | ||
| x, | ||
| y, | ||
| width: Math.min(maxWidth, right - left + padding * 2), | ||
| height: Math.min(maxHeight, bottom - top + padding * 2), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Low preview/PickPreload.ts:129
When x or y is clamped to 0 due to negative padding (e.g., left=10, padding=20 gives x=0), width and height are still computed with the unclamped formula right - left + padding * 2, producing an incorrect bounding box. In the example, width becomes 90 instead of the correct 80. The dimensions should use right + padding - x and bottom + padding - y to account for the clamped origin.
- const x = Math.max(0, left - padding);
- const y = Math.max(0, top - padding);
- const maxWidth = Math.max(1, window.innerWidth - x);
- const maxHeight = Math.max(1, window.innerHeight - y);
- return {
- x,
- y,
- width: Math.min(maxWidth, right - left + padding * 2),
- height: Math.min(maxHeight, bottom - top + padding * 2),
+ const x = Math.max(0, left - padding);
+ const y = Math.max(0, top - padding);
+ const maxWidth = Math.max(1, window.innerWidth - x);
+ const maxHeight = Math.max(1, window.innerHeight - y);
+ return {
+ x,
+ y,
+ width: Math.min(maxWidth, right + padding - x),
+ height: Math.min(maxHeight, bottom + padding - y),🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/desktop/src/preview/PickPreload.ts around lines 129-148:
When `x` or `y` is clamped to 0 due to negative padding (e.g., `left=10, padding=20` gives `x=0`), `width` and `height` are still computed with the unclamped formula `right - left + padding * 2`, producing an incorrect bounding box. In the example, `width` becomes `90` instead of the correct `80`. The dimensions should use `right + padding - x` and `bottom + padding - y` to account for the clamped origin.
Evidence trail:
apps/desktop/src/preview/PickPreload.ts lines 129-148 at REVIEWED_COMMIT. Lines 138-139 clamp x,y to 0 with Math.max(0, left-padding). Lines 145-146 compute width/height as `right - left + padding * 2` and `bottom - top + padding * 2`, which don't account for the clamped origin. When clamping occurs (left < padding), the computed width exceeds the correct value by `padding - left`.
Comment on lines
+54
to
+65
| if (choice === "open-in-preview") { | ||
| try { | ||
| await input.api.preview.open({ | ||
| threadId: input.threadRef.threadId, | ||
| url: input.url, | ||
| }); | ||
| useRightPanelStore.getState().open(input.threadRef, "preview"); | ||
| } catch { | ||
| input.fallbackToBrowser(); | ||
| } | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Medium preview/openTerminalLinkInPreview.ts:54
After api.preview.open() returns a snapshot, the code discards it and only calls useRightPanelStore.getState().open(threadRef, "preview"). This opens the right panel but fails to update the store with the actual tabId from the server response, so the UI renders a "new" placeholder or stale tab instead of the newly created preview session. Consider using openBrowser(threadRef, snapshot.tabId) (or equivalent store update) to synchronize the UI with the server state.
- if (choice === "open-in-preview") {
+ if (choice === "open-in-preview") {
try {
- await input.api.preview.open({
+ const snapshot = await input.api.preview.open({
threadId: input.threadRef.threadId,
url: input.url,
});
- useRightPanelStore.getState().open(input.threadRef, "preview");
+ useRightPanelStore.getState().openBrowser(input.threadRef, snapshot.tabId);
} catch {
input.fallbackToBrowser();
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/preview/openTerminalLinkInPreview.ts around lines 54-65:
After `api.preview.open()` returns a `snapshot`, the code discards it and only calls `useRightPanelStore.getState().open(threadRef, "preview")`. This opens the right panel but fails to update the store with the actual `tabId` from the server response, so the UI renders a "new" placeholder or stale tab instead of the newly created preview session. Consider using `openBrowser(threadRef, snapshot.tabId)` (or equivalent store update) to synchronize the UI with the server state.
Evidence trail:
- apps/web/src/components/preview/openTerminalLinkInPreview.ts lines 54-64: snapshot from `api.preview.open()` not captured; `open(threadRef, "preview")` used instead of `openBrowser`
- apps/web/src/browser/openFileInPreview.ts lines 17-20: established pattern captures snapshot, calls `applyServerSnapshot`, `rememberUrl`, and `openBrowser(threadRef, snapshot.tabId)`
- apps/web/src/rightPanelStore.ts lines 83-86: `browserSurface(null)` creates `{ id: "browser:new", kind: "preview", resourceId: null }` placeholder
- apps/web/src/rightPanelStore.ts lines 196-205: `open(ref, "preview")` uses existing surface or creates placeholder with null tabId
- apps/web/src/rightPanelStore.ts lines 206-215: `openBrowser(ref, tabId)` creates proper surface with real tabId and removes placeholder
- apps/web/src/components/ChatView.tsx lines 1329-1334: async reconciliation via `reconcileBrowserSurfaces` effect
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a complete integrated browser workflow to T3 Code, spanning the web UI, Electron guest webview, environment server, provider sessions, and shared contracts.
preview_*automation toolsAgent automation
The environment server now hosts one reusable Streamable HTTP MCP endpoint at
/mcp. Provider sessions receive short-lived, capability-scoped bearer credentials when they start or resume; only token hashes are retained, and credentials are revoked with the provider session.The preview toolkit supports:
Automation is routed through a preview broker to the focused desktop owner and then executed against the existing visible Electron webview via CDP. It does not launch a separate headless browser or per-thread MCP process, so the agent and user share the same page, cookies, navigation history, and visual state.
Provider integration covers Codex, Claude, Cursor, Grok, and OpenCode session startup/resume paths.
Preview and annotation architecture
apps/serverowns local-server discovery, preview session state, WebSocket RPCs, MCP authentication, scoped provider credentials, and automation request routing.apps/webowns the right-side preview experience, per-thread state, focused automation ownership, composer attachments, and preview lifecycle UX.apps/desktopowns the sandboxed Electron webview, navigation/zoom state, screenshot and recording capture, element picking, annotation overlays, and CDP execution.packages/contractsandpackages/client-runtimedefine the shared preview, IPC, RPC, annotation, and automation protocols.The picker preload intentionally uses
contextIsolation=falseso React component metadata is visible, while retainingsandbox=trueandnodeIntegration=false; the main process also enforces the security-critical guest preferences before attachment.Reliability
202 Acceptedfor Codex Streamable HTTP compatibilityUser impact
Users can discover and open a local app inside T3 Code, inspect and annotate the actual rendered page, capture screenshots or recordings, attach precise visual context to a prompt, and ask the coding agent to operate that same visible browser directly.
Validation
vp checkvp run typecheckvp test run apps/desktop/src/preview-view-manager.test.ts apps/desktop/src/playwright-injected-runtime.test.tsvp test run apps/server/src/mcp/Layers/McpHttpServer.test.ts apps/server/src/mcp/Layers/PreviewAutomationBroker.test.ts apps/server/src/mcp/toolkits/preview/tools.test.ts apps/server/src/provider/Layers/CodexAdapter.test.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.tsvp test(3,438passed,7skipped)ready;preview_open,preview_status, andpreview_snapshotexecuted against the integratedt3.chatwebviewNote
High Risk
Large new main-process surface (CDP, arbitrary JS evaluate, file artifacts) and Linux
--no-sandboxfallback when the Electron sandbox is misconfigured; incorrect path checks or control handoff could affect security or stability.Overview
Adds the desktop Electron side of the integrated browser preview: a
PreviewManagerservice that owns per-tab webview lifecycle, navigation/zoom, screenshots and recordings underbrowserArtifactsDir, element picking with themed annotation overlays, and CDP-backed agent automation (snapshots, clicks, typing, scroll, evaluate, wait) with human-vs-agent control interruption.Wires this through many new IPC channels and a
previewsection on the preloadDesktopBridge, plusBrowserSessionfor scoped persistent partitions and session-wide cookie/cache clears. Bootstrap now callsinstallDesktopIpcHandlers()(previously the Effect was not invoked) and registers preview event forwarding.Supporting changes: Tailwind v4 build script that tree-shakes classes from
PickPreload.tsinto generated annotation CSS; playwright-core injected runtime for locator resolution; react-grab in the picker preload; Linux dev launches gain--no-sandboxwhenchrome-sandboxis not setuid-root.Reviewed by Cursor Bugbot for commit 4b60227. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add integrated browser preview, element annotations, and MCP-based agent automation to the desktop app
<webview>tags with tab lifecycle management, navigation controls, zoom, DevTools, and screen recording.t3-code) via bearer-token credentials issued by McpSessionRegistry.ts, enabling preview automation tools (preview_status,preview_snapshot,preview_click,preview_navigate, etc.)./api/assets/.COMPOSER_DRAFT_STORAGE_VERSIONis bumped to 7, invalidating persisted composer drafts from prior versions; desktop IPC handler installation was previously broken (Effect not called) and is now fixed.Macroscope summarized 4b60227.