feat(server): single-window tab-group routing + simplified ScreencastManager#1074
Merged
Conversation
When the host pins the request to a specific tab group via the new X-BrowserOS-Default-Tab-Group-Id header, every tab created by the agent is deterministically placed in that group. Pair with the existing X-BrowserOS-Default-Window-Id, hosts can model "one window, N agents, N tab groups" without the agent picking groups itself. Wiring: - Four page-creating tools (new_page, new_hidden_page, show_page, move_page) accept an optional tabGroupId and call groupTabs after their main op. Best-effort: a stale groupId logs a warn but does not fail the tool. - routes/mcp.ts reads the header and threads it through createMcpServer → registerTools. - registerTools auto-injects tabGroupId the same way it injects windowId today: only when the host set it, the tool's schema accepts it, and the caller didn't explicitly pick a value. - When defaultWindowId is set (single-window mode), four window-mutating tools (create_window, create_hidden_window, close_window, set_window_visibility) are filtered out of the registered surface so an agent can't break the host's invariant. Existing callers that don't set either header keep their behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Front wake-up
The previous manager maintained a sessions map keyed by targetId with
shared subscribers, a pendingStarts dedupe, a primeWithScreenshot
lane, lastFrame cache, and a `disposed` retry loop. In practice the
agent-company renderer subscribes one pane at a time per install, and
shared sessions surfaced subtle frame cross-talk while late
subscribers got blank canvases on static pages.
Replaces all of that with a singleton — `active: ScreencastSession |
null`. A new subscribe `tearDown`s any prior session (sending
`detached` to the displaced ws) and registers a fresh
`Page.screencastFrame` listener bound to the new ws. Hidden /
backgrounded tabs don't composite — `Page.startScreencast` returns
zero frames on a tab whose RenderWidget isn't visible. So just before
`Page.startScreencast`, the manager calls `Page.bringToFront` to
foreground the tab in its window. `setWebLifecycleState('active')` is
not sufficient — verified empirically.
Subscribe also catches `Unknown page` from the resolve step and
emits a clean `detached` status instead of letting the route close
with 1011 (which would spiral EventSource reconnects).
The `connected` status is emitted AFTER `bringToFront` resolves so
the agent-company SSE proxy can use the first WS message as the
focus-restore signal — bringToFront steals macOS focus to BrowserOS
and the proxy fires a counter-activate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
❌ Tests failed — 1/1064 failed
Failed tests
|
Contributor
Greptile SummaryThis PR introduces two capabilities: per-agent tab-group routing for page-creating MCP tools (
Confidence Score: 3/5The tab-group routing additions are safe; the ScreencastManager rewrite carries three unresolved concurrency hazards from the prior review round that can leave CDP screencast sessions running with no subscriber. The three ScreencastManager concurrency issues remain unaddressed — session leak on mid-flight WS close, concurrent subscribe race producing an orphaned frame listener, and windowId:0 in the displaced detach message. Each can leave a CDP stream running with no cleanup path until a subsequent subscribe displaces it. packages/browseros-agent/apps/server/src/api/services/screencast/screencast-manager.ts Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/browseros-agent/apps/server/src/tools/navigation.ts:208-211
The `tabGroupId` description for `new_hidden_page` says "Tab group the page joins **when later shown**", but `joinTabGroup` is called immediately after the hidden page is created — not deferred to `show_page`. An agent reading the description may infer the grouping is a no-op until `show_page` is called and add redundant logic; or, if the hidden tab's group assignment matters early (e.g. for a host-side UI reflecting pending tabs), the current behaviour is correct but the doc sets the wrong expectation.
```suggestion
tabGroupId: z
.string()
.optional()
.describe('Tab group to place the new hidden tab into'),
```
Reviews (2): Last reviewed commit: "test(server): update screencast-manager ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The old test created a hidden window and asserted a frame arrives — that worked because the previous manager primed via captureScreenshot, which bypasses Chromium's compositor-visibility check. The new manager uses Page.bringToFront + Page.startScreencast, which only reliably emits frames for visible windows. Switch the test to a visible window and add an assertion that a displaced subscriber receives `detached`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@greptile review |
Contributor
Author
|
Note on The failing test is This is pre-existing — PR #1060 (which originally added this test file) had the exact same I'll leave the test fix in. If the reviewer wants the CI environment issue addressed in this PR, happy to dig — but it looks like a runner/binary problem orthogonal to the manager rewrite. |
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
Two commits enabling the agent-company "one window, N agents, N tab groups" host model:
feat(server): per-agent tab group routing for page-creating tools—new_page,new_hidden_page,show_page,move_pagenow accept an optionaltabGroupIdand callgroupTabsafter page creation. Pairs withX-BrowserOS-Default-Tab-Group-Idso hosts can pin every page-creating tool call from a given agent into a specific tab group without the agent picking groups itself.refactor(server): simplify ScreencastManager to a singleton + bringToFront wake-up— drops the sessions map / pendingStarts / primeWithScreenshot / lastFrame / retry-loop complexity; oneactivesession at a time,tearDowndisplaces the prior subscriber, andPage.bringToFrontis called right beforePage.startScreencastso backgrounded tabs actually composite. Subscribe also catchesUnknown pageand emits a cleandetachedstatus instead of letting the route close with 1011 (which spirals EventSource reconnects).Why bringToFront
Page.startScreencaston a tab whose RenderWidget isn't visible returns zero frames — the compositor is paused. POC confirmedsetWebLifecycleState('active')alone is not sufficient;bringToFrontis the lightest wake-up that delivers frames. Side-effect: BrowserOS visibly switches its frontmost tab and steals macOS focus, so theconnectedstatus is emitted right afterbringToFrontto give the agent-company SSE proxy an early signal to counter-activate Electron.Test plan
bun run typecheckcleandetachedstatus, no spiralfeat/single-window-and-screencastPRRelated
Companion PR: browseros-ai/agent-company#104 — single app window + per-pane screencast + visibility toggle that consume this server-side foundation.
🤖 Generated with Claude Code