Repository navigation
feat(vscode): add integrated browser editor tab - #14836
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 760048d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 760048d)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (38 files)
Reviewed by deepseek-v4.1-flash · Input: 130.4K · Output: 35.8K · Cached: 2.6M Review guidance: REVIEW.md from base branch |
Extract shared webview browser adapter and host control mapping to remove duplication flagged by the duplication check. Track broker owners so the Agent Manager and browser tab close only their own entries, and guard the Agent Manager route on the exact displayed session.
| /** Close every entry owned by one surface, leaving other owners' entries intact. */ | ||
| closeOwned(owner: BrowserOwner): Promise<void> { | ||
| return Promise.all( | ||
| [...this.entries.values()] |
There was a problem hiding this comment.
WARNING: closeOwned only scans this.entries, so it misses in-flight open operations that have not registered an entry yet, which makes it weaker than the previous closeAll.
The old closeAll enumerated entries + pending (via sessions()) and routed through close(), whose serial(key, ...) waits for a pending create and then retires the entry. closeOwned iterates [...this.entries.values()] only, so a browser open still awaiting ensureBrowser()/create() when closeAll() runs is never stopped; the create completes afterwards and leaves a live proxy/context until broker dispose. This can be reached from AgentManagerProvider.onWorkspaceChanged() (pinned-workspace sync) and refreshBrowserAutomation(). Consider tracking the owner for pending keys, or document that closeOwned cannot cancel in-flight opens.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // While the Agent Manager panel is focused on this exact session, it | ||
| // owns the browser for that shared root session. Other sidebar and | ||
| // editor-tab sessions still resolve here. | ||
| if (agentManager?.isActive(route.sessionId)) return |
There was a problem hiding this comment.
WARNING: Ownership is keyed to the session the Agent Manager is currently displayed on, not to whether it actually owns a browser for that session, so a shared root session can end up with two owners.
isActive(route.sessionId) is only true while visiblePresence.displayed() equals the route's session. The Agent Manager keeps a browser entry alive across session switches (it only closes on explicit close, project change, or dispose), and displayed() is null when the panel shows a terminal/review/history view. So when the Agent Manager has a live browser for shared root session X but currently displays Y (or a non-session view), the guard does not fire and the editor-tab resolver also claims X; the broker then holds both ${project}\0X (Agent Manager) and \0X (tab) entries. Session-scoped lookups become ambiguous: get(X) returns undefined when two entries match (browser-broker.ts:463), and tab refresh/back/forward/inspect throw Browser session identity is ambiguous from require (browser-broker.ts:1062). Deciding based on whether the Agent Manager owns an entry for route.sessionId, rather than on the displayed session, would express the intended ownership.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| /** Builds a neutral control message from a surface message that shares these fields. */ | ||
| export function browserControlMessage(kind: BrowserControlKind, fields: BrowserControlFields): BrowserControlMessage { | ||
| return { kind, ...fields } |
There was a problem hiding this comment.
SUGGESTION: { kind, ...fields } places the spread after kind, so a kind property on the incoming surface message would silently override the caller-supplied control kind - a guarantee the previous explicit field list provided. No current caller sets a top-level kind, so this is latent; return { ...fields, kind } preserves the guarantee at no cost.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
What Problem This Solves
The Integrated Browser only worked inside Agent Manager. A session in the sidebar or a Kilo editor tab could call
browser_open, but had no way to see the page it opened.Why This Change Was Made
The Agent Manager browser already has a broker, a screencast stream, and a full panel UI. This change reuses that implementation for a new editor-tab browser instead of building a second one:
browser-control.ts, used by both Agent Manager and the editor tab.BrowserBrokerroute ownership to multiple owners, disambiguated by project id, so sidebar sessions and Agent Manager sessions can coexist.BrowserTabProvider(one webview panel per session) plus abrowserTab.*message contract and a webview adapter over the sharedBrowserPanel.Kilo Code: Open Integrated Browsercommand. Thebrowser_opentool also reveals the tab automatically.User Impact
kilo-code.new.experimental.browserAutomationsetting.Evidence
Automated checks:
bun run check-typesandcheck-types:webview: 0 errors.bun run lint,bun run knip,bun run bundle: clean.Manual verification in an isolated VS Code instance:
Kilo Code: Open Integrated Browseropens the tab and renders a local page.browser_opentool auto-reveals the tab without opening Agent Manager.Notes
packages/kilo-docs/source-links.mdis regenerated because the tab's download button references the Chrome URL.