🤖 feat: add browser preview tab switching#3399
Conversation
|
/coder-agents-review |
|
@codex review |
|
Chat: Spend limit reached | View chat Review history
deep-review v0.5.0 | Round 6 | Last posted: Round 6, 40 findings (1 P0, 2 P2, 20 P3, 1 P4, 8 Nit, 8 Note), APPROVE. Review Finding inventoryFindings
Contested and acknowledgedCRF-24 (Note, BrowserTab.test.tsx:209) - Optimistic update indistinguishable from polling in test
CRF-25 (Note, BrowserTab.tsx:549) - When no tab has active: true, first tab silently promoted to active
CRF-34 (Note, router.ts:484) - selectTab failure clears preview URL, but browser page didn't change
CRF-36 (Note, BrowserControlService.ts:132) - Zod validation error details discarded on schema mismatch
Round logRound 1Panel (18 reviewers). 2 P2, 13 P3, 1 P4, 7 Nit, 3 Note. Reviewed against 5d79d9b..c31f725. Round 2Panel (8 reviewers). 1 P0, 5 P3, 1 Nit, 3 Note new. Reviewed against 5d79d9b..1726a3f. Round 3BLOCKED. CRF-27 addressed. CRF-28 through CRF-36 silent. No review. Round 4Panel (5 reviewers). CRF-34 closed by panel (4/4 accept). 2 P3, 2 Note new. Reviewed against 5d79d9b..b60dd80. Round 5BLOCKED. CRF-37 through CRF-40 silent. No review. Round 6Churn guard: PROCEED. CRF-37/38/39 author-fixed. CRF-40 dropped (body-folded Note, thread resolved). Reviewed against 5d79d9b..8dd084d. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c31f725fb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c31f725fb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/coder-agents-review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b410e6f837
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/coder-agents-review |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid feature addition. The polling architecture correctly mirrors the existing session discovery pattern, the runBrowserCommandWithLoadingState extraction cleans up real duplication, and the concurrency handling (generation counter, ref-based staleness guards, cancelled flags) is well-designed. The keyboard tablist implementation covers the WAI-ARIA basics. Pariston tried to build a case against the approach and couldn't.
"I tried to build a case against this and couldn't. The problem is correctly understood, the solution is proportional, and the fix is at the right level." (Pariston)
Severity count: 2 P2, 13 P3, 1 P4, 7 Nit, 3 Note.
The two P2s are the most important: (1) the optimistic tab-active update matches by both tabId and label, which can mark two tabs active simultaneously when a label collides with a tabId (9 reviewers converged on this independently), and (2) parseBrowserTabsJson is the trust boundary between CLI output and the tab model, and 7 of its 9 branches have no test coverage.
The P3s fall into three clusters: test coverage gaps (error paths, ArrowLeft wrap, label formatters, old-tab deselection assertion), correctness/robustness issues (fail-fast on single malformed entry, misleading error messages, CLI arg injection class, schema-coupled comparator, double-click race), and UX/accessibility (aria-label on active tab, scroll-into-view, missing invariant comment).
🤖 This review was automatically generated with Coder Agents.
|
Addressed coder-agents-review Round 1 findings:
Validation after fixes:
|
|
/coder-agents-review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Pushed a small test-only stabilization for the BrowserTab error-state test after the unit CI caught a race. |
|
/coder-agents-review Please re-check after the BrowserTab test stabilization push. |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
All 26 R1 findings addressed. Thorough fix commit. Pariston verified the fixes and again found the approach proportional and correct.
R2 uncovered 1 P0 and 5 new P3s from the fix commit.
The P0 is a deterministic test failure in the new error-path test ("shows tab listing and switching errors"). It blocks CI. The remaining P3s are: a CRF-9 re-raise (keyboard wrap test still doesn't prove wrapping), a poll-during-switch race, a residual CRF-4 gap (error message misleading for success:false without details), array-level Zod validation rejecting all tabs for one bad entry, and an unconditional tabpanel landmark that orphans when the tablist is hidden.
Severity count: 1 P0, 5 P3, 1 Nit, 3 Note.
"I tried to build a case that the author's understanding of the problem is incomplete, that a simpler solution exists, or that the fix is at the wrong level of the causal chain. I couldn't." (Pariston)
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
CRF-27 (P0, deterministic test failure) was fixed in bc9b776 and CI is now green.
The remaining 9 R2 findings have no author response and no code changes. Further review is blocked until they are addressed, acknowledged, or contested.
Open P3s needing response:
- CRF-28: Keyboard wrap-around test still doesn't exercise wrapping (re-raise of CRF-9)
- CRF-29: Poll during slow tab switch overwrites optimistic state (one-line guard fix)
- CRF-30: Fallback error says "returned invalid JSON" when CLI reported
success: false - CRF-31: Zod safeParse validates all page tabs as one array; one bad entry rejects all
- CRF-32:
role="tabpanel"renders unconditionally; orphaned landmark when tablist hidden
Open Nit:
- CRF-33:
forloop test hides which case failed
Open Notes (acknowledge or contest):
- CRF-34: selectTab failure clears preview URL incorrectly
- CRF-35: Formatting test proves strings exist but not rendering slot
- CRF-36: Zod validation error details discarded
🤖 This review was automatically generated with Coder Agents.
|
Addressed the latest coder-agents-review round:
Validation after fixes:
|
|
@codex review Latest coder-agents-review findings were addressed and resolved. |
|
/coder-agents-review Latest findings were addressed and resolved; please re-check. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
All prior findings resolved. CRF-34 closed: 4/4 panel reviewers independently verified the author's defense by tracing BrowserSessionStateHub.markLoaded; the URL is preserved on failed commands, not cleared.
Pariston, for the third time, tried to build a case against the approach and couldn't: "The problem is correctly understood as a missing UI surface. The solution is proportional: it reuses the existing polling pattern, the existing CLI interface, and the existing loading-state infrastructure."
"The code survived the fight." (Hisoka)
Two new P3s and two Notes from this round. Both P3s are test coverage boundary cases, not correctness bugs.
Severity count: 2 P3, 2 Note.
src/browser/features/RightSidebar/BrowserTab/BrowserTab.test.tsx:495
Note [CRF-40] The tab switching error test asserts the error message appears and aria-busy clears, but does not verify that the original tab's aria-selected stays "true" or that the failed target's stays "false". Adding expect(view.getByTestId("browser-page-tab-t1").getAttribute("aria-selected")).toBe("true") after the error check would close the gap. (Chopper)
🤖
🤖 This review was automatically generated with Coder Agents.
|
@codex review Pushed deterministic integration-test fixes for the CI failures (removed live GitHub/submodule network dependencies) after reproducing them locally. |
|
/coder-agents-review Pushed deterministic integration-test fixes for the CI failures after reproducing them locally. |
|
Addressed the follow-up coder-agents-review round:
Validation after fixes:
|
|
@codex review Follow-up coder-agents-review findings were addressed and resolved. |
|
/coder-agents-review Follow-up findings were addressed and resolved; please re-check. |
There was a problem hiding this comment.
The R4 findings have no author response and no code changes. The post-R4 commit (79881f0) touches only tests/ipc/ files (CI infrastructure). Further review is blocked until the findings are addressed, acknowledged, or contested.
Open P3s needing response:
- CRF-37: No test verifies single-tab hides tab strip (CRF-16 boundary untested)
- CRF-38: All-entries-rejected parse returns empty tabs with no error signal
Open Notes (acknowledge or contest):
- CRF-39: Formatting test covers uncommon paths; most common plain-title path unasserted
- CRF-40: Tab switching error test does not verify selection state preserved after failure
🤖 This review was automatically generated with Coder Agents.
|
Follow-up response to the coder-agents-review body from 2026-05-27T12:20:05Z:
Validation:
|
|
@codex review Addressed the latest coder-agents-review body, including CRF-40. |
|
/coder-agents-review Addressed the latest review body, including CRF-40; please re-check current head. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cd8567d5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed Codex P2: the local submodule fixture repo now configures Validation:
|
|
@codex review Addressed the submodule fixture author identity issue. |
|
/coder-agents-review Codex's submodule fixture author-identity finding is fixed; please re-check current head. |
There was a problem hiding this comment.
All 40 findings resolved across 6 rounds. No new findings from Netero or the panel. CI green.
The PR adds browser page-tab awareness to the Browser Preview panel: ORPC schemas and endpoints for browser.listTabs and browser.selectTab, BrowserControlService CLI integration with defensive parsing, a WAI-ARIA tablist with keyboard navigation and optimistic state management, and a clean runBrowserCommandWithLoadingState refactoring that eliminated loading-state duplication.
"I tried to build a case against this and couldn't. The problem is correctly understood: users need tab awareness, and this PR adds it. The solution is proportional: it follows the existing poll-based discovery pattern, reuses the CLI control architecture, and the UI follows WAI-ARIA tablist conventions." (Pariston)
Ship it.
🤖 This review was automatically generated with Coder Agents.
|
@codex review All prior Codex comments are resolved and CI is green; please confirm the current head. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e935ff310
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed Codex P1: removed the unsupported Validation:
|
|
@codex review Addressed the tab command separator issue and added a flag-like tab ref guard. |
|
/coder-agents-review Codex's tab command separator finding is fixed; please re-check current head. |
|
@claude review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
/coder-agents-review All prior coder-agents-review findings are resolved, Codex approved, and CI is green. Please confirm current head. |
1 participant
Summary
Adds browser page-tab awareness to the Browser Preview panel so users can see and switch between
agent-browsertabs from Mux.Background
agent-browseralready supports multi-tab sessions, but Mux previously only showed the currently streamed page. If an agent opened another tab, users had no way to discover or select it from the Browser Preview UI.Implementation
browser.listTabsandbrowser.selectTab.BrowserControlServiceto validate session ownership, callagent-browser tab list/tab <ref>, and parse page-tab metadata.Validation
bun test src/browser/features/RightSidebar/BrowserTab/BrowserTab.test.tsx src/node/services/browser/BrowserControlService.test.tsmake typecheckmake lintmake static-checkRisks
Medium UI/backend integration risk: this touches Browser Preview polling, ORPC schemas, and
agent-browserCLI control paths. The new behavior is scoped to the experimental Agent Browser tab.Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh• Cost:$115.09