💭 The diff scope covers HostedBrowserWebview.tsx, PreviewAutomationHosts.tsx, and previewGuestViewport files. Since PreviewView.tsx is no longer part of the diff and the previously flagged duplicate effect code has moved into HostedBrowserWebview, I want to check that file directly to see if the issue was resolved or just relocated.

1. browse_code — apps/web/src/browser/HostedBrowserWebview.tsx

2. browse_code — apps/web/src/components/preview/previewGuestViewport.ts

💭 The earlier flagged issues both look addressed now - the rAF/registration ordering with hasWebContents gating, and using effectiveViewport as the drag-live ref. But I'm noticing a new concern: since effectiveViewport updates on every pointermove during drag, the guestViewportKey would also change on every move, which could cause excessive re-registration. I'm also questioning whether mutating a ref directly during render (as opposed to inside an effect) breaks React's render-purity rules, though it's a common pattern so it's ambiguous whether that's a real finding. I'm also wondering if applying the CDP override on every drag frame causes unnecessary guest re-layout for performance reasons, and whether the override gets properly cleared on unmount or tab switch, plus checking how data-preview-css-width interacts with fittedSourceViewport when fit-source mode is active. I'm now noticing a potential mismatch: when fitSourceContent is true, the layout derives its dimensions from fittedSourceViewport rather than effectiveViewport, but the guest override still pins to effectiveViewport's width/height — meaning the CDP override could pin the guest to different dimensions than its actual rendered element box, which would reintroduce the exact bug this PR is meant to fix, but specifically in fit-source mode. I need to check resolveFittedBrowserViewport and browserViewportLayout.ts to confirm this discrepancy, and also verify behavior when deviceToolbarVisible is false.

3. browse_code — apps/web/src/browser/browserViewportLayout.ts

4. browse_code — apps/web/src/browser/useBrowserViewportResize.ts

💭 I'm looking at the fit-source-content case in HostedBrowserWebview, tracing how fittedSourceViewport only diverges from effectiveViewport when the viewport tag is "fill" — in that scenario the layout switches to a fixed freeform arrangement. Once viewport is "fill", the guest override gets cleared entirely, so the guest reports its own element-derived dimensions, and I verify these end up equal to the fitted width scaled by zoom, which seems consistent. I'm now checking the parallel case for non-fit mode with a fixed viewport, tracing how the element width relates to the effective viewport width times the normalized zoom factor. I realize this mobile-emulation threshold applies to freeform desktop windows too, which could cause a small window to unexpectedly switch to mobile layout—but that logic lives in Manager.ts, outside the desktop-app scope I'm restricted to, so I need to redirect my focus to files in apps/web instead. Looking there, one candidate issue is that a ref is being mutated directly during render, which is an impure pattern that could cause problems under React's strict or concurrent mode. I'm now tracing through the effect's dependency logic step by step: during a drag, each pointer move triggers a new viewport key causing the effect to rerun and reapply via requestAnimationFrame, which seems fine; after drag commit the key stays the same so nothing reruns, also fine; and if the drag is cancelled, the viewport reverts and the key changes back, correctly re-triggering the override. So this particular effect flow doesn't appear to be buggy. But I'm now spotting a real inconsistency: the code seems to mix up tabId (the server-side identifier) with runtimeTabId (used elsewhere for registering webviews and applying guest viewports), and I need to check how desktopByTabId is actually keyed in the preview state store to see if this mismatch is a genuine bug.

5. git_tools — desktopByTabId

💭 Good, tabId keying checks out. Now I'm noticing that useThreadPreviewState subscribes to the entire thread preview state in HostedBrowserWebview, which would trigger re-renders on every nav status, loading, or favicon change—unlike elsewhere where useShallow selectors are used with the browser surface store. I want to check whether useThreadPreviewState even accepts a selector argument.

6. git_tools — export function useThreadPreviewState|export const useThreadPreviewState

💭 Since there's no selector support, this component re-renders on every preview state change even though it uses useShallow. I'm wondering if the recomputation from HostedBrowserWebview is costly since it re-renders the webview and calls presentContent each time -- if layout is a new object identity on every render, that would recreate syncContentPresentation and trigger the rAF effect unnecessarily on each pass. I'm also questioning whether presentContent could trigger a feedback loop through the store subscription, though it likely no-ops if content is equal, so this seems like a pre-existing pattern rather than a new issue. Given the check's actual scope is UI consistency -- shared components, Tailwind ownership, themes, layout preservation -- I should focus on whether responsive geometry is preserved across panel, inline-preview, and desktop Electron layout modes rather than chasing this performance tangent. I'm now tracing whether the guest viewport override applies correctly for mini-player/inline preview, confirming HostedBrowserWebview mounts fine with snapshot viewport, and checking that the hasWebContents gating correctly defers the effect until webContents exists, including after crash recovery regenerates the webview. The more concerning finding is that the "fill" viewport case triggers a debugger attachment via ensureControlSession on every tab at mount time even when no actual override is needed, which has real side effects since Electron won't allow DevTools to open while a debugger is attached. I want to check whether ensureControlSession in Manager.ts guards against this unnecessary attach for the no-op fill case.

7. git_tools — const ensureControlSession

💭 I'm realizing that attaching the CDP debugger at mount time on every preview webview, even when unneeded, could cause issues: it fails silently if DevTools is already open, but once attached, it might also block the user from opening DevTools afterward on that guest view. I want to verify whether that's actually the case. I should search Manager for existing handling around openDevTools and isDevToolsOpened to see if there's already a guard for this interaction.

8. git_tools — openDevTools|devtools-opened

💭 DevTools does detach the debugger session and restore it afterward, but that detach likely clears any CDP-based viewport override since Emulation overrides are session-scoped. Since nothing re-applies the override once DevTools closes and the debugger reattaches, that's a real regression for the fixed-viewport preview feature.

9. git_tools — const restoreControlSession

💭 This confirms my suspicion: the code itself acknowledges the viewport override is lost on session detach and must be reapplied on reattach, but restoreControlSession only reapplies the color scheme, not the viewport. So toggling DevTools on a preview with a fixed viewport silently drops the device-metrics override, leaving the toolbar showing the wrong size while the guest actually renders at the host-derived dimensions. The proper fix lives in Manager.ts, which stores tab state and isn't something I can touch given my include patterns, so I'm weighing whether the web-side effect around HostedBrowserWebview.tsx (lines 210-219) can reasonably own this — it's the effect currently responsible for reapplying overrides, so a comment there flagging the gap and suggesting the desktop tab state as the durable owner seems appropriate. I'm also checking whether webview swaps or other detach paths (crash recovery, automation completion, screenshot capture) trigger similar loss, and want to trace where detachControlSession gets called to be thorough.

10. git_tools — detachControlSession(

💭 This confirms a real issue: the override doesn't survive a control-session detach since it's not stored on tab state like colorScheme, so restoreControlSession can't bring it back. The proper fix would mean persisting the override on PreviewTabState and reapplying it in restoreControlSession, but that's a desktop-side change outside my file scope, so I can only flag it rather than propose a diff. Since this is a concrete regression risk introduced by the change itself, it's worth reporting. Now I want to check the PreviewAutomationHosts changes for correctness too.

11. browse_code — apps/web/src/components/preview/PreviewAutomationHosts.tsx

💭 I'm tracing a potential race condition: when the agent resizes the viewport, the server snapshot updates and triggers the HostedBrowserWebview effect, which then calls the human-path setViewport redundantly alongside the automation path's own call—applying the same override twice but through different mechanisms. I also notice a type-safety gap: PreviewAutomationHosts treats bridge.automation.setViewport as required in the contract, but older desktop preloads may not actually implement it, meaning the typed non-optional field could still be undefined at runtime. I'm also wondering about a race condition: if the viewport key changes rapidly during a drag, each rAF cancels the previous one, but the underlying IPC calls to apply the CDP viewport override aren't guaranteed to resolve in order, since they're async and could interleave in the main process before their sendCommand executes. That means an older, stale viewport update could theoretically land after a newer one, causing a flicker to the wrong size. Looking more closely, I notice applyViewportOverride calls ensureControlSession but sends its command directly without going through the semaphore that withControlSession uses elsewhere, so it isn't serialized against other automation control commands. This means it could genuinely race with something like a concurrent screenshot request holding that same control session, which feels like a real, if narrow, correctness gap worth flagging rather than pure speculation. Actually, ordering risk is low since both calls enter the same queue in sequence, and the rAF throttle limits calls to roughly one per frame anyway, so I won't flag this as speculative. I'll focus on concrete findings instead: the strongest one is that the device metrics override gets lost when DevTools detaches and nothing re-applies it, since the web effect won't re-fire — I should point this out in HostedBrowserWebview.tsx. Now I want to check whether the web side receives any signal when DevTools opens or closes that would affect hasWebContents or the key, so I'm looking into whether DesktopPreviewOverlay tracks a devToolsOpen field.

12. git_tools — interface DesktopPreviewOverlay|DesktopPreviewOverlay = {|export interface DesktopPreviewOverlay

13. browse_code — apps/web/src/previewStateStore.ts

💭 With no devtools signal confirming the web side can't re-apply after devtools close, the durable fix must live in the desktop tab state alongside colorScheme. I'll report this as the main finding, commenting on HostedBrowserWebview.tsx since that's the web owner of the override, and flag the fix belongs in restoreControlSession — this file is within scope for comments. Now I'm considering whether there's a worthwhile second finding to raise. Reconsidering the full-state subscription pattern in HostedBrowserWebview: since it hosts a webview and computes layout on every render while triggering presentContent through a rAF effect, subscribing to the entire thread preview state means it re-renders on every nav/loading/favicon/audible/zoom event across any tab — for every mounted webview host. That raises a possible perf concern where a new layout object each render could shift the effect's identity and retrigger presentContent, potentially causing a render loop, so I want to check whether presentContent has an equality guard. Though this behavior already existed before the current change, since the component was already re-rendering on any surface store update. That rAF-driven scroll read forces a layout on each preview event, which is a real jank source but feels borderline against the review scope. Checking the repo, other components like PreviewView also consume the whole preview state without narrowing, so this pattern is consistent with existing practice and I'll let it go. I'm also confirming device metrics overrides persist across navigations, and checking whether the hasWebContents gate effect properly handles webview regeneration after crash recovery. Following through, after a crash the webContents ID changes and the control session gets detached and reattached cleanly during setViewport, so that path is fine. I'm also noting that a debugger gets attached to every preview webview on mount via the fill case's clear flag, which has no visible banner in Electron unlike Chrome, and that seems benign unless something external tries to attach the debugger too. Now I'm looking at how the controller state handles this attachment sequence. Since withControlSession sets the controller to "agent" while applyViewportOverride bypasses that by calling ensureControlSession directly, there's no agent badge shown, which lines up with what the PR intends. I want to check whether anything elsewhere in the manager treats wc.debugger.isAttached() as a signal that an agent is actively controlling the session, so I should search for other places that check isAttached.

14. git_tools — isAttached()

💭 I'll flag the override-not-surviving-detach issue as an inline comment on lines 210–219 of HostedBrowserWebview.tsx, since this regression is genuinely introduced by this PR and hasn't been caught by prior review passes. I'm confirming the diff hunk numbering lines up so the comment lands on the right side, then checking whether a DevTools open/close is actually a reachable trigger for this path. Confirming that opening DevTools does reset device metrics — the debugger session detaching causes Chromium to revert the override, and there's already a comment in the codebase acknowledging this exact loss for color scheme, which validates my finding. So the bug: after DevTools closes on a fixed-viewport preview, the guest silently reverts to host dimensions while the toolbar still shows the old fixed size — the original issue resurfaces. The fix is to persist the requested override on PreviewTabState like colorScheme does and re-apply it on every reattach. I want to double-check there's no other place that re-applies the override automatically, like usePreviewBridge re-registering on state changes, before concluding this is a real regression.

15. browse_code — apps/web/src/components/preview/usePreviewBridge.ts

💭 I'm confirming there's no devtools signal so nothing re-applies, and drafting a review comment explaining that the device metrics override lives on the CDP control session and gets dropped whenever that session detaches, since only the color scheme gets re-applied on reattach, not the viewport. I'm double-checking the diff line positions to make sure the comment can be anchored correctly, then considering whether to raise a second issue — that the viewport function runs on registration even when clearing, forcing an unnecessary CDP debugger attach on every preview tab. That extra attach could subtly affect guest behavior or conflict with any external debugger attach, but it's minor and I decide not to raise it as a separate comment since the main finding is stronger.

16. modify_pr — One finding on the guest viewport override lifecycle. See the inline comment on apps/web/src/browser/HostedBrowserWebview.tsx.