feat(pip): add computer use snapshot preview - #2050
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds Computer Use latest-snapshot PiP with shared NativeKit coordination, trusted MCP snapshot observation, bounded JPEG processing, typed desktop contracts, Canvas fallback rendering, dismissal handling, and lifecycle/session staleness protections. ChangesComputer Use Snapshot PiP
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant ToolManager
participant ComputerUsePreviewPresenter
participant AgentPreviewCoordinator
participant AgentComputerUsePiP
ToolManager->>ComputerUsePreviewPresenter: notify trusted snapshot started/completed
ComputerUsePreviewPresenter->>ComputerUsePreviewPresenter: validate and transform latest image
ComputerUsePreviewPresenter->>AgentPreviewCoordinator: prepare and present target
AgentPreviewCoordinator-->>ComputerUsePreviewPresenter: native-overlay or renderer-canvas
ComputerUsePreviewPresenter->>AgentComputerUsePiP: publish surface and Canvas frame events
AgentComputerUsePiP->>ComputerUsePreviewPresenter: set mode or dismiss preview
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zerob13
marked this pull request as ready for review
July 28, 2026 07:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/main/desktop/preview/AgentPreviewCoordinator.ts (2)
157-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
releaseClaimleavesdismissedRunsentries behind.
dismissedRunsis only pruned insideclaim()when a different runId claims the same session, so sessions that are dismissed and then torn down (e.g.destroySessionBrowser→releaseClaim) keep an entry untilshutdown(). Consider deleting the dismissal when the matching claim is released.♻️ Suggested cleanup
if ( claim && (target.source == null || claim.source === target.source) && (target.runId == null || claim.runId === target.runId) ) { this.claims.delete(target.sessionId) + if (this.dismissedRuns.get(target.sessionId) === claim.runId) { + this.dismissedRuns.delete(target.sessionId) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/desktop/preview/AgentPreviewCoordinator.ts` around lines 157 - 167, Update releaseClaim in AgentPreviewCoordinator to remove the session’s dismissedRuns entry when the released target matches the current claim, using the same source and runId matching conditions as claim deletion. Preserve removeTarget behavior and avoid clearing dismissals for non-matching claims.
257-268: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA single
pushImagefailure permanently disables the native overlay.
disableNative('frame_push_failed')setsunavailable = trueand stops the overlay, so one transient push failure downgrades every subsequent preview (Browser and Computer Use) to renderer-canvas for the remaining process lifetime, with no recovery path. Consider tolerating a bounded number of consecutive failures (or a failure window) before disabling native permanently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/desktop/preview/AgentPreviewCoordinator.ts` around lines 257 - 268, The native overlay path currently permanently disables itself after one failed pushImage call. Update the failure handling around pushImage and disableNative to track consecutive failures or failures within a bounded window, reset that tracking after a successful push, and call disableNative('frame_push_failed') only after the configured threshold is reached.src/renderer/src/components/computerUse/AgentComputerUsePiP.vue (1)
132-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSignificant duplication with
AgentBrowserPiP.vue(drag/positioning/preview-mode queue).
updateHostSize,clampPosition,onPointerDown/onPointerMove/onPointerUp/cancelDrag, and thependingPreviewRequest/drainPreviewRequestssingle-flight queue are near line-for-line copies of the equivalent logic inAgentBrowserPiP.vue. Given both files already show subtle divergence (e.g. the unmount-queue gap above), extracting a shared composable (e.g.usePipDragging,usePreviewModeQueue) would reduce future drift risk. Not blocking for this PR.Also applies to: 270-312
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/computerUse/AgentComputerUsePiP.vue` around lines 132 - 213, Extract the duplicated PiP dragging/positioning logic and preview-mode request queue from AgentComputerUsePiP.vue and AgentBrowserPiP.vue into shared composables, such as usePipDragging and usePreviewModeQueue. Update both components to consume the shared implementations while preserving their existing behavior, including queue draining and unmount cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/computer-use-snapshot-pip/plan.md`:
- Around line 308-319: Update the “Verification Commands” section to include the
required Electron build and complete main-suite validation commands, alongside
the existing checks. If those commands are maintained elsewhere, replace or
supplement the list with an explicit link to the authoritative commands in
tasks.md.
In `@src/main/desktop/computerUse/ComputerUsePreviewPresenter.ts`:
- Around line 180-234: Add cleanup for states whose host window is missing or
destroyed, including releasing any coordinator claim associated with the state.
Update the host validation/lookup flow around setPreviewMode and related state
access, or register a closed-window cleanup hook, so entries are removed even
when stopped is not sent. Preserve existing session and host-id checks while
ensuring stale state cannot block future claims.
In `@src/main/mcp/toolManager.ts`:
- Around line 903-951: Gate the implicit snapshot in
captureComputerUsePreviewSnapshot with the same checkToolPermission and
plugin-policy evaluation used for model-driven calls. Treat get_window_state as
read permission, and skip capture unless that permission is already granted for
the server/session; do not prompt or bypass policy. Keep
scheduleComputerUsePreviewAfterClick responsible only for eligibility and
scheduling.
In `@src/renderer/src/components/browser/AgentBrowserPiP.vue`:
- Around line 278-285: Update dismiss() in
src/renderer/src/components/browser/AgentBrowserPiP.vue#L278-L285 and
src/renderer/src/components/computerUse/AgentComputerUsePiP.vue#L172-L178 to
catch failures from the awaited dismissPreview client calls and log or report
the errors, preventing unhandled rejections while preserving the existing
early-return and UI state behavior.
In `@src/renderer/src/components/computerUse/AgentComputerUsePiP.vue`:
- Around line 393-402: Update the onBeforeUnmount teardown in
AgentComputerUsePiP.vue to submit the final 'stopped' preview-mode transition
through the existing pendingPreviewRequest/drainPreviewRequests single-flight
queue, using the same mechanism as syncPreviewMode and the session-switch
watcher. Preserve the current cleanup ordering and only enqueue the request when
currentSessionId.value exists.
---
Nitpick comments:
In `@src/main/desktop/preview/AgentPreviewCoordinator.ts`:
- Around line 157-167: Update releaseClaim in AgentPreviewCoordinator to remove
the session’s dismissedRuns entry when the released target matches the current
claim, using the same source and runId matching conditions as claim deletion.
Preserve removeTarget behavior and avoid clearing dismissals for non-matching
claims.
- Around line 257-268: The native overlay path currently permanently disables
itself after one failed pushImage call. Update the failure handling around
pushImage and disableNative to track consecutive failures or failures within a
bounded window, reset that tracking after a successful push, and call
disableNative('frame_push_failed') only after the configured threshold is
reached.
In `@src/renderer/src/components/computerUse/AgentComputerUsePiP.vue`:
- Around line 132-213: Extract the duplicated PiP dragging/positioning logic and
preview-mode request queue from AgentComputerUsePiP.vue and AgentBrowserPiP.vue
into shared composables, such as usePipDragging and usePreviewModeQueue. Update
both components to consume the shared implementations while preserving their
existing behavior, including queue draining and unmount cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6aa2de99-92ce-44a4-a5f2-fef6d1ff2efd
📒 Files selected for processing (40)
docs/architecture/nativekit-agent-browser-pip/spec.mddocs/features/agent-browser-pip/spec.mddocs/features/computer-use-snapshot-pip/plan.mddocs/features/computer-use-snapshot-pip/spec.mddocs/features/computer-use-snapshot-pip/tasks.mdsrc/main/app/composition.tssrc/main/desktop/browser/YoBrowserPresenter.tssrc/main/desktop/computerUse/ComputerUsePreviewPresenter.tssrc/main/desktop/preview/AgentPreviewCoordinator.tssrc/main/desktop/routes.tssrc/main/mcp/index.tssrc/main/mcp/toolManager.tssrc/main/tool/index.tssrc/renderer/api/BrowserClient.tssrc/renderer/api/ComputerUseClient.tssrc/renderer/src/apps/chat-main/ChatTabView.vuesrc/renderer/src/components/browser/AgentBrowserPiP.vuesrc/renderer/src/components/computerUse/AgentComputerUsePiP.vuesrc/shared/contracts/events.tssrc/shared/contracts/events/browser.events.tssrc/shared/contracts/events/computerUse.events.tssrc/shared/contracts/routes.tssrc/shared/contracts/routes/browser.routes.tssrc/shared/contracts/routes/computerUse.routes.tssrc/shared/types/computerUse.tssrc/shared/types/desktop.tssrc/shared/types/mcp.tstest/main/desktop/browser/AgentBrowserNativeOverlay.test.tstest/main/desktop/browser/YoBrowserPresenter.test.tstest/main/desktop/computerUse/ComputerUsePreviewPresenter.test.tstest/main/desktop/preview/AgentPreviewCoordinator.test.tstest/main/mcp/toolManager.test.tstest/main/routes/contracts.test.tstest/main/routes/dispatcher.test.tstest/main/tool/toolService.test.tstest/renderer/api/clients.test.tstest/renderer/components/AgentBrowserPiP.test.tstest/renderer/components/AgentComputerUsePiP.test.tstest/renderer/components/ChatTabView.test.tstest/setup.ts
💤 Files with no reviewable changes (1)
- test/main/desktop/browser/AgentBrowserNativeOverlay.test.ts
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
1 participant
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
get_window_stateimages through a bounded main-process pipeline with a renderer Canvas fallbackget_window_state({ pid, window_id })after an eligible successfulclick; the private result is never returned to the model, conversation, cache, or generic MCP result eventWhy
Browser PiP owns a continuously capturable
WebContentsView, while Computer Use only exposes discrete window snapshots. Users still need to supervise desktop automation without switching away from DeepChat. This implementation presents the latest honest snapshot and adds one event-driven post-click refresh instead of background polling.UX
Computer Use exposes Close only. It has no expand/open-panel action and forwards no input to the target application.
Safety and performance
Validation
pnpm run format:checkpnpm run i18npnpm run lintpnpm run typecheckpnpm exec electron-vite buildRemaining QA
Summary by CodeRabbit