Repository navigation
fix(desktop): agent browser opens show in the right pane, even when away - #9047
saddlepaddle wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Register and create slot-specific pane stores apps/desktop/src/renderer/routes/.../useV2WorkspacePaneLayout/... |
The workspace hook now creates pane stores through a slot-aware factory. The factory configures pane placement for the rightPaneLayout slot. |
Open background browsers in the right pane apps/desktop/src/renderer/routes/.../openUrlInV2Workspace.ts, apps/desktop/src/renderer/routes/.../useBrowserOpenRequests.ts, apps/desktop/src/renderer/routes/.../openBackgroundBrowser/* |
The open flow supplies a pane ID and disables focus. It uses a mounted right-pane store when available; otherwise, it creates and persists the right-pane layout and selection. Browser state updates now inspect tabs in both pane layouts. Tests cover new-tab opening, current-tab replacement, distinct pane IDs, and the mounted-store path. |
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant useBrowserOpenRequests
participant openBackgroundBrowser
participant openUrlInV2Workspace
participant rightPaneStore
participant collections
participant rightSidebar
useBrowserOpenRequests->>openBackgroundBrowser: background browser request
openBackgroundBrowser->>openUrlInV2Workspace: URL, paneId, focus false
openUrlInV2Workspace->>rightPaneStore: addTab or openPane
alt right-pane store is mounted
openBackgroundBrowser->>rightSidebar: open sidebar
else right-pane store is unmounted
openBackgroundBrowser->>collections: persist rightPaneLayout and selection
openBackgroundBrowser->>rightSidebar: open sidebar
end
Suggested reviewers: kitenite
Merge Risk: 
84703
-
Background browser opens move to the right pane; no merge-blocking risk was established.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title names the main change: agent browser opens from the background now show the browser in the right pane. The title is in conventional commit format and matches the changeset, which routes back… Description check The description follows the template. It has a 'What & why' section that explains the bug, the cause, and the fix, and it references related PR #9204. It has a 'How I tested it' section that lists the…Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the main change: agent browser opens from the background now show the browser in the right pane. The title is in conventional commit format and matches the changeset, which routes background browser opens to rightPaneLayout and sets rightSidebarOpen. The title does not mention the live-store update or the helper refactor, which is expected for a short title.
Full details: Description check
-
Explanation
The description follows the template. It has a 'What & why' section that explains the bug, the cause, and the fix, and it references related PR
#9204. It has a 'How I tested it' section that lists the unit test coverage, the typecheck, lint, and test runs, and a clear note that the change was not verified in the running app. The checklist is present. Two checklist items are unchecked: the fork-maintainer edit item, which may not apply, and the CLA item, which the author has not confirmed. The template also asks for screenshots for user-visible bug fixes, and none are included. The description does not link an issue number, though the template asks for one when available.Resolution
Optional: add screenshots or a CDP capture of the before and after behavior, since the template asks for them on user-visible bug fixes. Optional: link a tracking issue if one exists. Confirm the CLA and fork-maintainer checklist items, or mark them as not applicable.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 7 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
 [Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryBackground browser opens now select a browser in the right pane and open the sidebar, including when the user is in another workspace.
No actionable issues were found. Review evidence came from code inspection; the running desktop app was not tested. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Background browser request] --> B{Right store mounted?}
B -->|Yes| C[Open browser in live right store]
C --> D[Hook saves right layout]
C --> E[Open right sidebar]
B -->|No| F[Build right store from saved layout]
F --> G[Save right layout and open sidebar together]
E --> H[Start browser runtime]
G --> H
H --> I[Save browser state in its layout]
Reviews (1) · Last reviewed commit: "fix(desktop): agent browser opens show i..." · Reviewed by Greptile |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
A browser that an agent opened in the background was written into the center layout. moveViewerPanesToRight then moved it to the right pane, and nothing opened the right pane, so it never appeared. Write it straight into rightPaneLayout as the active tab and set rightSidebarOpen in the same update. When the workspace is mounted in this window, apply it to the live right store instead, without keyboard focus, so the new tab becomes active there too. The background browser's state is saved into whichever layout holds the pane.
9d4a80a to
847031c
Compare
What & why
When an agent opened a browser while the right pane was closed, the browser never appeared (reported by Kiet). Background browser opens (
browser.onOpenRequestwithshow: false, handled inuseBrowserOpenRequests→openBackgroundBrowser) wrote the browser into the center layout.moveViewerPanesToRightthen moved it to the right pane, and nothing opened the right pane. That happened whether the workspace was on screen or not.Now the open writes the right state directly:
rightPaneLayoutas the active tab, andrightSidebarOpenis set totrue, in one row update. If you are in another workspace, the right pane is open on the new browser when you come back.open browserop: it always adds a new tab. The browser bridge's default target iscurrent-tab, which reuses the agent's unpinned browser, so with the op everysuperset browser openwould pile up a new tab. This uses the right store's own placement (placeInTabs) instead.paneLayout, so it lost track of the pane once it was moved right.createPaneLayoutStoreis shared byuseV2WorkspacePaneLayoutand the background open, so both build right-pane stores the same way (getPaneKey,placeInTabs).openUrlInV2Workspacetakes an optionalpaneIdandfocus. The background open sets the pane id upfront instead of reading back the active pane.This replaces an earlier idea of a hook that opens the sidebar whenever a pane appears in the right store: that only works while the workspace is on screen.
How I tested it
openBackgroundBrowser.test.ts, rewritten:rightSidebarOpenis set;current-tabreplaces the unpinned right-pane browser, never a pinned one;focusRequest.bun run typecheckinapps/desktop,bun run lint, and the full desktopbun test(8,871 pass, 0 fail). Rebased on main after feat(cli): let agents drive the right sidebar with superset panes #9204: typecheck, lint and the renderer route suites (1,458 pass) re-run.Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)Summary by CodeRabbit