feat(host-browser): accept target_client_id LLM param to override interface preference#30066
Merged
Merged
Conversation
…erface preference Adds parity with host_bash / host_file_* / host_cu: the LLM can now pass --target-client-id <id> on `assistant browser` to route CDP commands to a specific connected client (chrome-extension or macOS) instead of always falling through the hardcoded HOST_BROWSER_INTERFACE_PREFERENCE order. Changes: - host-browser-proxy.ts: resolveTargetClient() accepts optional targetClientId; when supplied, looks up client directly in host_browser roster (capability check via listClientsByCapability) and returns it for same-actor enforcement. request() signature gains targetClientId? param and threads it through. - extension-cdp-client.ts: ExtensionCdpClient and createExtensionCdpClient accept targetClientId? and forward it to proxy.request(). - factory.ts: GetCdpClientOptions gains targetClientId?; getCdpClient(), buildCandidateList(), and buildPinnedCandidateList() thread it to createExtensionCdpClient(). - browser-execution.ts: acquireCdpClientWithMode() extracts target_client_id from input (same pattern as host_file) and passes to getCdpClient(). - cli/commands/browser.ts: adds --target-client-id parent flag (mirrors --browser-mode) and injects as input.target_client_id. - skills/vellum-browser-use/SKILL.md: documents --target-client-id with example and pointer to `assistant clients list --capability host_browser`. - Tests: 5 new proxy tests (explicit routing, missing client, missing capability, actor mismatch, regression guard) + 1 factory threading test. Updated 2 existing factory tests for the expanded call signature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@chatgpt-codex-connector @devin review please Adds |
![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: 3615803caf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| try { | ||
| const raw = getCdpClient(context, { mode: effectiveMode }); | ||
| const raw = getCdpClient(context, { mode: effectiveMode, targetClientId }); |
There was a problem hiding this comment.
When target_client_id is provided, this call still uses effectiveMode, which can be a previously remembered sticky mode (for example local) from earlier turns in the same conversation. In that case the request never goes through the host-browser proxy and the explicit client target is silently ignored, so commands can run against the wrong browser instance even though the caller asked for a specific client.
Useful? React with 👍 / 👎.
Comment on lines
162
to
+164
| mode === "auto" | ||
| ? buildCandidateList(context) | ||
| : buildPinnedCandidateList(context, mode); | ||
| ? buildCandidateList(context, targetClientId) | ||
| : buildPinnedCandidateList(context, mode, targetClientId); |
There was a problem hiding this comment.
In auto mode, passing targetClientId still builds the full fallback chain (extension → cdp-inspect/local). If the targeted host client is stale, missing, or lacks host_browser, the extension attempt fails and the factory can silently fail over to another backend, executing on an unintended browser instead of surfacing a targeting error. Explicit client targeting should fail closed here.
Useful? React with 👍 / 👎.
…nreachable target without fallback Addresses two Codex P1 findings from PR #30066: P1 #1 (browser-execution.ts:acquireCdpClientWithMode): when target_client_id is present, the effectiveMode was still derived from the conversation's sticky backend (e.g. "local" from a prior turn). The targeted client was silently ignored and commands ran against the wrong browser. Fix: bypass the sticky preference entirely when targetClientId is set — force effectiveMode to "extension" so the request always reaches the host-browser proxy. Also guard the sticky-fallback retry path (auto + stale memo) with targetClientId == null so a targeting failure surfaces as an error rather than rerouting to local. P1 #2 (factory.ts:buildCandidateList): in auto mode with targetClientId, the full fallback chain (extension → cdp-inspect → local) was still built. If the targeted host client was stale or the proxy unavailable, the factory silently failed over to cdp-inspect or local and executed on an unintended browser. Fix: when targetClientId is provided, return only a single extension candidate. If no Chrome Extension is connected, throw a CdpError immediately with a clear message that names the unreachable target_client_id — no fallback. Tests added: - factory.test.ts: buildCandidateList with targetClientId → single extension candidate; no extension → throws transport_error naming the target; create() threads targetClientId through to createExtensionCdpClient. - factory.test.ts: getCdpClient auto + targetClientId + no extension → throws, local not tried; auto + targetClientId + extension → only extension used. - browser-execution-acquire.test.ts: acquireCdpClientWithMode sticky-local + target_client_id → getCdpClient receives mode:extension; sticky honored when no target_client_id. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@chatgpt-codex-connector @devin re-review please Fixed both P1 findings (commit 7645e05):
|
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Contributor
Author
Cycle 1 wrapStatus: ready for merge.
|
This was referenced May 9, 2026
noanflaherty
pushed a commit
that referenced
this pull request
May 10, 2026
Adds target_client_id to all 8 app_control_* tool input schemas (start, observe, key, key_combo, sequence, type, click, drag) and threads it through the surfaceProxyResolver dispatch branch in conversation-surfaces.ts. Mirrors the host_cu pattern in the same file: validate explicit target_client_id (existence, capability, same-actor) before dispatch; auto-resolve unique same-user client when cross-user clients are connected; error when multiple same-user clients are ambiguous. Closes the last LLM-exposure gap from the cross-client host tool arc (PR #29322 host_bash, #29398 host_file/host_cu, #30066 host_browser). Was deferred per 'skip unless they bite' but the WIP was sitting half-done in the worktree. Tests: - conversation-surfaces-app-control.test.ts: new 317-line file covering unavailability, end-to-end dispatch, local stop short- circuit, target_client_id existence/capability/same-actor checks, multi-client error path, and cross-user rejection. - app-control-flow.test.ts: extended with target_client_id checks. Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
credence-the-bot Bot
added a commit
that referenced
this pull request
May 10, 2026
Drops the chrome-extension-first interface preference from HostBrowserProxy and removes the now-unused `getPreferredClientByCapability` method on the event hub. Auto-resolution now picks the most-recently-active host_browser client (lastActiveAt-desc), matching the host_bash / host_file_* / host_cu / host_app_control family. When the LLM needs a specific transport (e.g. Chrome Extension's `chrome.debugger` over the macOS CDP bridge), it can pass `target_client_id` explicitly via the LLM-facing param shipped in #30066. Behavior change: when a user has both Chrome Extension and macOS clients connected, host_browser previously routed to the Chrome Extension by hardcoded preference. It now routes to whichever client was active most recently. The LLM-facing `target_client_id` override (#30066) is the supported way to pin a transport. Closes the last item from the cross-client host tool tech-debt list. Was deferred per 'skip unless they bite' alongside the app_control `target_client_id` LLM-param work; lands together with PR #30200.
noanflaherty
pushed a commit
that referenced
this pull request
May 10, 2026
) * refactor(host-browser): retire HOST_BROWSER_INTERFACE_PREFERENCE Drops the chrome-extension-first interface preference from HostBrowserProxy and removes the now-unused `getPreferredClientByCapability` method on the event hub. Auto-resolution now picks the most-recently-active host_browser client (lastActiveAt-desc), matching the host_bash / host_file_* / host_cu / host_app_control family. When the LLM needs a specific transport (e.g. Chrome Extension's `chrome.debugger` over the macOS CDP bridge), it can pass `target_client_id` explicitly via the LLM-facing param shipped in #30066. Behavior change: when a user has both Chrome Extension and macOS clients connected, host_browser previously routed to the Chrome Extension by hardcoded preference. It now routes to whichever client was active most recently. The LLM-facing `target_client_id` override (#30066) is the supported way to pin a transport. Closes the last item from the cross-client host tool tech-debt list. Was deferred per 'skip unless they bite' alongside the app_control `target_client_id` LLM-param work; lands together with PR #30200. * refactor(host-browser): purge stale interface-preference JSDoc Devin caught three JSDoc blocks still referencing 'interface- preference order/resolution' after the constant was deleted: - HostBrowserProxy.request() — described the no-actor fallback as 'interface-preference resolution'; now matches the resolver ('most-recently-active host_browser client'). - extension-cdp-client.ts — targetClientId JSDoc. - factory.ts — targetClientId JSDoc. Also dropped a history-style comment in the test ('chrome- extension preference was retired now that LLMs can pick…') — code-comments rule is current-state only. Tests still 27/27. Lint clean. --------- Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
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
Closes the parity gap between
host_browserand the other host-proxy capabilities (host_bash,host_file_*,host_cu): the LLM can now pass--target-client-id <id>onassistant browserto route CDP commands to a specific connected client instead of always applying the hardcodedHOST_BROWSER_INTERFACE_PREFERENCE = ["chrome-extension", "macos"]order.Prior art:
Why this matters:
macosChanges
host-browser-proxy.tsresolveTargetClient()acceptstargetClientId?; when supplied, looks up client directly inhost_browserroster (capability check vialistClientsByCapability).request()gainstargetClientId?param.extension-cdp-client.tsExtensionCdpClientandcreateExtensionCdpClientaccept and forwardtargetClientId?.factory.tsGetCdpClientOptions.targetClientId?; threaded throughgetCdpClient,buildCandidateList,buildPinnedCandidateListtocreateExtensionCdpClient.browser-execution.tsacquireCdpClientWithModeextractsinput.target_client_idand passes togetCdpClient(same pattern ashost_file).cli/commands/browser.ts--target-client-id <id>parent flag (mirrors--browser-mode), injected asinput.target_client_id.skills/vellum-browser-use/SKILL.mdassistant clients list.Test plan
targetClientIdflows tocreateExtensionCdpClientbun test src/__tests__/host-browser-proxy.test.ts— 27 passbun test src/__tests__/host-browser-routes.test.ts— all passbun test src/tools/browser/cdp-client/__tests__/factory.test.ts— 88 passbun test src/daemon/__tests__/conversation-tool-setup.test.ts— 44 passbunx tsc --noEmit— no errorsbun run lint— clean🤖 Generated with Claude Code