From 3615803caf50dc9e37aa433adbf9d2c174f73c3f Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 03:26:59 +0000 Subject: [PATCH 1/3] feat(host-browser): accept target_client_id LLM param to override interface preference Adds parity with host_bash / host_file_* / host_cu: the LLM can now pass --target-client-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 --- .../src/__tests__/host-browser-proxy.test.ts | 163 +++++++++++++++++- assistant/src/cli/commands/browser.ts | 25 ++- assistant/src/daemon/host-browser-proxy.ts | 45 +++-- .../src/tools/browser/browser-execution.ts | 9 +- .../cdp-client/__tests__/factory.test.ts | 30 +++- .../cdp-client/extension-cdp-client.ts | 8 + .../src/tools/browser/cdp-client/factory.ts | 16 +- skills/vellum-browser-use/SKILL.md | 16 ++ 8 files changed, 281 insertions(+), 31 deletions(-) diff --git a/assistant/src/__tests__/host-browser-proxy.test.ts b/assistant/src/__tests__/host-browser-proxy.test.ts index ef223e63f3f..d9e5b779153 100644 --- a/assistant/src/__tests__/host-browser-proxy.test.ts +++ b/assistant/src/__tests__/host-browser-proxy.test.ts @@ -394,9 +394,9 @@ describe("HostBrowserProxy", () => { // --------------------------------------------------------------------------- // Same-actor binding (cross-user enforcement) // - // The host-browser proxy is auto-resolved (the LLM-facing tool does not - // expose `target_client_id`), so same-user enforcement runs on the - // resolved candidate before we register the pending interaction: + // When the caller does not supply `targetClientId`, the proxy auto-resolves + // using `resolveTargetClient(sourceActorPrincipalId)` which filters clients + // to the same actor before applying the interface-preference order: // // 1. `resolveTargetClient(sourceActorPrincipalId)` filters candidate // clients to those owned by the caller's actor before applying the @@ -586,4 +586,161 @@ describe("HostBrowserProxy", () => { expect(getPublishedMessages()).toHaveLength(0); }); }); + + // --------------------------------------------------------------------------- + // Explicit targetClientId routing + // + // When `targetClientId` is supplied, the proxy skips the interface-preference + // sort and routes directly to the named client (subject to the same-actor + // enforcement that runs on all host-proxy requests). + // --------------------------------------------------------------------------- + + describe("explicit targetClientId routing", () => { + test("routes to the named client and persists targetClientId in pending state", async () => { + mockClients = [ + { + clientId: "macos-client", + interfaceId: "macos", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + // Explicitly target the macOS client even though chrome-extension + // would win under normal interface-preference ordering. + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + "macos-client", + ); + + expect(getPublishedMessages()).toHaveLength(1); + const sent = getPublishedMessages()[0] as Record; + const requestId = sent.requestId as string; + + const pending = pendingInteractions.get(requestId); + expect(pending?.targetClientId).toBe("macos-client"); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + + test("rejects when targetClientId does not match any connected client", async () => { + mockClients = [ + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + "nonexistent-client", + ); + + await expect(resultPromise).rejects.toThrow( + "no active extension connection", + ); + expect(getPublishedMessages()).toHaveLength(0); + }); + + test("rejects when targetClientId points to a client without host_browser capability", async () => { + // The client exists but is not in the host_browser roster, so + // listClientsByCapability("host_browser") does not return it. + mockClients = [ + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_bash"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + "ext-client", + ); + + await expect(resultPromise).rejects.toThrow( + "no active extension connection", + ); + expect(getPublishedMessages()).toHaveLength(0); + }); + + test("same-actor check rejects targetClientId that belongs to a different actor", async () => { + mockClients = [ + { + clientId: "other-user-ext", + interfaceId: "chrome-extension", + actorPrincipalId: "user-2", + capabilities: ["host_browser"], + }, + ]; + + // actor user-1 explicitly targets user-2's client — same-actor gate + // should fire and return an isError result (not reject the promise). + const result = await proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + "other-user-ext", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("Submitting actor does not match"); + expect(getPublishedMessages()).toHaveLength(0); + }); + + test("no targetClientId falls back to interface-preference order (regression guard)", async () => { + mockClients = [ + { + clientId: "macos-client", + interfaceId: "macos", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + // No targetClientId — should auto-resolve to chrome-extension (higher priority). + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + // targetClientId omitted + ); + + const requestId = (getPublishedMessages()[0] as Record) + .requestId as string; + const pending = pendingInteractions.get(requestId); + expect(pending?.targetClientId).toBe("ext-client"); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + await resultPromise; + }); + }); }); diff --git a/assistant/src/cli/commands/browser.ts b/assistant/src/cli/commands/browser.ts index 3e27fb0e7c2..37ef96bc34a 100644 --- a/assistant/src/cli/commands/browser.ts +++ b/assistant/src/cli/commands/browser.ts @@ -164,22 +164,31 @@ function buildSubcommand(parent: Command, meta: BrowserOperationMeta): void { session?: string; json?: boolean; browserMode?: string; + targetClientId?: string; }; const sessionId = parentOpts.session ?? "default"; const jsonMode = parentOpts.json ?? false; const conversationId = resolveContextConversationId(); // Map Commander camelCase options back to snake_case input keys, - // filtering out parent-level options (session, json, browserMode) - // and screenshot ergonomics (output). + // filtering out parent-level options (session, json, browserMode, + // targetClientId) and screenshot ergonomics (output). const input: Record = {}; - const excludeKeys = new Set(["session", "json", "output", "browserMode"]); - - // Inject parent-level --browser-mode into the operation input so - // the backend receives the mode override for backend pinning. + const excludeKeys = new Set([ + "session", + "json", + "output", + "browserMode", + "targetClientId", + ]); + + // Inject parent-level flags into the operation input. if (parentOpts.browserMode) { input.browser_mode = parentOpts.browserMode; } + if (parentOpts.targetClientId) { + input.target_client_id = parentOpts.targetClientId; + } for (const [key, value] of Object.entries(opts)) { if (excludeKeys.has(key)) continue; @@ -362,6 +371,10 @@ export function registerBrowserCommand(program: Command): void { "--browser-mode ", "Browser backend to use. Overrides automatic selection.", ).choices([...BROWSER_MODES]), + ) + .option( + "--target-client-id ", + "Route browser operations to a specific client. Obtain IDs from `assistant clients list --capability host_browser`.", ); browser.addHelpText( diff --git a/assistant/src/daemon/host-browser-proxy.ts b/assistant/src/daemon/host-browser-proxy.ts index 4a6c398c374..e35480a961e 100644 --- a/assistant/src/daemon/host-browser-proxy.ts +++ b/assistant/src/daemon/host-browser-proxy.ts @@ -34,17 +34,30 @@ const HOST_BROWSER_INTERFACE_PREFERENCE: InterfaceId[] = [ /** * Pick the host_browser-capable client to dispatch to. * - * When a `sourceActorPrincipalId` is supplied, candidate clients are - * filtered down to those owned by the same actor before applying the - * interface-preference order. Returns `undefined` when no same-actor - * client is connected; the caller surfaces this as the existing - * "no active extension connection" rejection. + * When `targetClientId` is supplied, the client with that id is looked + * up directly in the `host_browser`-capable roster. The same-actor check + * in `request()` still runs on the returned client when + * `sourceActorPrincipalId` is present. * - * When `sourceActorPrincipalId` is undefined (legacy callers without a - * resolved actor identity), falls through to the prior behavior so the - * registry singleton continues to work as before. + * When `sourceActorPrincipalId` is supplied (and no explicit target), + * candidate clients are filtered down to those owned by the same actor + * before applying the interface-preference order. Returns `undefined` + * when no same-actor client is connected; the caller surfaces this as + * the existing "no active extension connection" rejection. + * + * When neither is supplied (legacy callers without a resolved actor + * identity), falls through to the prior behavior so the registry + * singleton continues to work as before. */ -function resolveTargetClient(sourceActorPrincipalId: string | undefined) { +function resolveTargetClient( + sourceActorPrincipalId: string | undefined, + targetClientId?: string, +) { + if (targetClientId != null) { + const clients = assistantEventHub.listClientsByCapability("host_browser"); + return clients.find((c) => c.clientId === targetClientId); + } + if (sourceActorPrincipalId == null) { return assistantEventHub.getPreferredClientByCapability( "host_browser", @@ -126,10 +139,15 @@ export class HostBrowserProxy { /** * Send a host_browser request to the connected extension/macOS bridge. * + * When `targetClientId` is supplied, the proxy dispatches to that specific + * client (subject to the `host_browser` capability check and the same-actor + * gate below). This mirrors the `target_client_id` pattern on `host_bash`, + * `host_file_*`, and `host_cu`. + * * When `sourceActorPrincipalId` is supplied, the proxy refuses to dispatch * to a client owned by a different actor — same-user enforcement is the * authoritative gate against routing one actor's CDP request onto another - * actor's connected extension. The auto-resolved target's `clientId` and + * actor's connected extension. The resolved target's `clientId` and * `actorPrincipalId` are then persisted alongside the pending interaction * so that the result-route's same-actor check can verify the submitting * client at result time. @@ -143,6 +161,7 @@ export class HostBrowserProxy { conversationId: string, signal?: AbortSignal, sourceActorPrincipalId?: string, + targetClientId?: string, ): Promise { if (signal?.aborted) { return Promise.resolve({ content: "Aborted", isError: true }); @@ -152,7 +171,7 @@ export class HostBrowserProxy { // alongside the pending interaction registration. Same shape as // host-cu-proxy: the result-route same-actor check compares the // submitting client's actor against this captured value. - const preferredClient = resolveTargetClient(sourceActorPrincipalId); + const preferredClient = resolveTargetClient(sourceActorPrincipalId, targetClientId); // Same-user enforcement: when the caller's actor is known, refuse to // dispatch to a client owned by a different actor. This covers the @@ -172,7 +191,7 @@ export class HostBrowserProxy { if (rejection) return Promise.resolve(rejection); } - const targetClientId = preferredClient?.clientId; + const resolvedClientId = preferredClient?.clientId; const targetActorPrincipalId = preferredClient?.actorPrincipalId; const requestId = uuid(); @@ -216,7 +235,7 @@ export class HostBrowserProxy { pendingInteractions.register(requestId, { conversationId, kind: "host_browser", - targetClientId, + targetClientId: resolvedClientId, targetActorPrincipalId, rpcResolve: resolve as (v: unknown) => void, rpcReject: reject, diff --git a/assistant/src/tools/browser/browser-execution.ts b/assistant/src/tools/browser/browser-execution.ts index 6706d648bfc..9a60b6a3397 100644 --- a/assistant/src/tools/browser/browser-execution.ts +++ b/assistant/src/tools/browser/browser-execution.ts @@ -378,6 +378,11 @@ function acquireCdpClientWithMode( } const browserMode = modeResult.mode; + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + const rememberedKind = browserManager.getPreferredBackendKind( context.conversationId, ); @@ -387,7 +392,7 @@ function acquireCdpClientWithMode( : browserMode; try { - const raw = getCdpClient(context, { mode: effectiveMode }); + const raw = getCdpClient(context, { mode: effectiveMode, targetClientId }); const cdp = wrapWithKindMemo(raw, context.conversationId); return { cdp, browserMode }; } catch (err) { @@ -398,7 +403,7 @@ function acquireCdpClientWithMode( if (browserMode === "auto" && effectiveMode !== "auto") { browserManager.clearPreferredBackendKind(context.conversationId); try { - const raw = getCdpClient(context, { mode: "auto" }); + const raw = getCdpClient(context, { mode: "auto", targetClientId }); const cdp = wrapWithKindMemo(raw, context.conversationId); return { cdp, browserMode }; } catch (retryErr) { diff --git a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts index 38f2566199c..7e1c439a4ad 100644 --- a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts +++ b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts @@ -248,15 +248,15 @@ describe("getCdpClient", () => { ); expect(result).toEqual({ ok: true, via: "extension" }); expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); - // Call signature includes optional cdpSessionId and sourceActorPrincipalId - // (both undefined here — no pinned session id, no actor binding in this - // legacy ctx). The `sourceActorPrincipalId` param exists so the proxy - // can refuse cross-user dispatch under cross-client exposure. + // Call signature includes optional cdpSessionId, sourceActorPrincipalId, + // and targetClientId (all undefined here — no pinned session id, no actor + // binding, no explicit client target in this legacy ctx). expect(createExtensionCdpClientMock).toHaveBeenCalledWith( fakeProxy, "test-convo", undefined, undefined, + undefined, ); expect(createLocalCdpClientMock).not.toHaveBeenCalled(); expect(createCdpInspectClientMock).not.toHaveBeenCalled(); @@ -700,6 +700,28 @@ describe("getCdpClient", () => { "actor-bound", undefined, "user-actor-1", + undefined, + ); + }); + + test("threads targetClientId from options into createExtensionCdpClient", async () => { + const fakeProxy = makeAvailableProxy(); + mockSingletonProxy = fakeProxy; + const ctx = makeContext({ + conversationId: "targeted-client", + sourceActorPrincipalId: "user-actor-1", + }); + + const client = getCdpClient(ctx, { targetClientId: "specific-ext-client" }); + await client.send("Page.navigate"); + + expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); + expect(createExtensionCdpClientMock).toHaveBeenCalledWith( + fakeProxy, + "targeted-client", + undefined, + "user-actor-1", + "specific-ext-client", ); }); }); diff --git a/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts b/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts index f4af7873788..5ba697532b5 100644 --- a/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts +++ b/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts @@ -51,6 +51,11 @@ export class ExtensionCdpClient implements ScopedCdpClient { * boundary. */ private readonly sourceActorPrincipalId?: string, + /** + * Explicit target client id. When provided, the proxy routes directly to + * that client instead of applying the interface-preference order. + */ + private readonly targetClientId?: string, ) {} async send( @@ -82,6 +87,7 @@ export class ExtensionCdpClient implements ScopedCdpClient { this.conversationId, signal, this.sourceActorPrincipalId, + this.targetClientId, ); } catch (err) { throw new CdpError( @@ -203,11 +209,13 @@ export function createExtensionCdpClient( conversationId: string, cdpSessionId?: string, sourceActorPrincipalId?: string, + targetClientId?: string, ): ExtensionCdpClient { return new ExtensionCdpClient( proxy, conversationId, cdpSessionId, sourceActorPrincipalId, + targetClientId, ); } diff --git a/assistant/src/tools/browser/cdp-client/factory.ts b/assistant/src/tools/browser/cdp-client/factory.ts index 32b661cf8c0..80f098a1811 100644 --- a/assistant/src/tools/browser/cdp-client/factory.ts +++ b/assistant/src/tools/browser/cdp-client/factory.ts @@ -97,6 +97,12 @@ export interface GetCdpClientOptions { * and disables failover. */ mode?: BrowserMode; + /** + * Explicit target client id. When provided, the extension backend routes + * to this specific client instead of applying the interface-preference + * order. Mirrors the `target_client_id` pattern on host_bash/host_file/host_cu. + */ + targetClientId?: string; } /** @@ -151,10 +157,11 @@ export function getCdpClient( options?: GetCdpClientOptions, ): ScopedCdpClient { const mode: BrowserMode = options?.mode ?? "auto"; + const targetClientId = options?.targetClientId; const candidates = mode === "auto" - ? buildCandidateList(context) - : buildPinnedCandidateList(context, mode); + ? buildCandidateList(context, targetClientId) + : buildPinnedCandidateList(context, mode, targetClientId); log.debug( { @@ -182,6 +189,7 @@ export function getCdpClient( export function buildPinnedCandidateList( context: ToolContext, mode: Exclude, + targetClientId?: string, ): BackendCandidate[] { const { conversationId, sourceActorPrincipalId } = context; @@ -215,6 +223,7 @@ export function buildPinnedCandidateList( conversationId, undefined, sourceActorPrincipalId, + targetClientId, ); const backend = createExtensionBackend({ isAvailable: () => true, @@ -288,7 +297,7 @@ export function buildPinnedCandidateList( * * Exported for testing. */ -export function buildCandidateList(context: ToolContext): BackendCandidate[] { +export function buildCandidateList(context: ToolContext, targetClientId?: string): BackendCandidate[] { const { conversationId, sourceActorPrincipalId } = context; const candidates: BackendCandidate[] = []; const hostBrowserProxy = HostBrowserProxy.instance; @@ -304,6 +313,7 @@ export function buildCandidateList(context: ToolContext): BackendCandidate[] { conversationId, undefined, sourceActorPrincipalId, + targetClientId, ); const backend = createExtensionBackend({ isAvailable: () => true, diff --git a/skills/vellum-browser-use/SKILL.md b/skills/vellum-browser-use/SKILL.md index ea63c618a9c..f790acdba9a 100644 --- a/skills/vellum-browser-use/SKILL.md +++ b/skills/vellum-browser-use/SKILL.md @@ -84,6 +84,22 @@ If the user declines to install the extension: Only fall back to these if the user explicitly indicates they do not want to install the extension. Prefer `cdp-inspect` over `local`. +## Targeting a Specific Client + +When multiple clients support `host_browser` (e.g. two Chrome profiles, a macOS client and a Chrome extension), use `--target-client-id ` on the `assistant browser` parent command to pin all operations in the invocation to one specific client: + +```bash +assistant browser --target-client-id navigate --url https://example.com +``` + +Obtain client IDs from: + +```bash +assistant clients list --capability host_browser +``` + +Omit `--target-client-id` when only one client is connected — the default interface-preference order (`chrome-extension` first, then `macos`) picks the best available client automatically. + ## Session Management Use `--session ` on the `assistant browser` parent command to group sequential operations so they share browser state (same page, cookies, etc.). Different session IDs create independent browser contexts. From 7645e0508eb19ba41d15a108bdc1c3de488952af Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 03:53:27 +0000 Subject: [PATCH 2/3] fix(host-browser): honor target_client_id over sticky mode + reject unreachable target without fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../browser-execution-acquire.test.ts | 206 ++++++++++++++++++ .../src/tools/browser/browser-execution.ts | 10 +- .../cdp-client/__tests__/factory.test.ts | 108 +++++++++ .../src/tools/browser/cdp-client/factory.ts | 46 ++++ 4 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 assistant/src/tools/browser/__tests__/browser-execution-acquire.test.ts diff --git a/assistant/src/tools/browser/__tests__/browser-execution-acquire.test.ts b/assistant/src/tools/browser/__tests__/browser-execution-acquire.test.ts new file mode 100644 index 00000000000..fd123b1444f --- /dev/null +++ b/assistant/src/tools/browser/__tests__/browser-execution-acquire.test.ts @@ -0,0 +1,206 @@ +/** + * Tests for the target_client_id sticky-mode override in + * acquireCdpClientWithMode (browser-execution.ts). + * + * Fix 1: when target_client_id is provided, the sticky backend kind + * remembered from prior turns in the conversation must NOT be applied. + * The factory must receive mode:"extension" so the request reaches the + * host-browser proxy regardless of any prior local/cdp-inspect preference. + */ +import { beforeEach, describe, expect, mock, test } from "bun:test"; + +import type { ToolContext } from "../../types.js"; +import type { BrowserMode, CdpClientKind } from "../cdp-client/types.js"; + +// --------------------------------------------------------------------------- +// Captured call state +// --------------------------------------------------------------------------- + +interface CdpClientCallOpts { + mode?: BrowserMode; + targetClientId?: string; +} + +const getCdpClientCalls: CdpClientCallOpts[] = []; + +function makeFakeScopedClient(kind: CdpClientKind, conversationId: string) { + return { + kind, + conversationId, + send: mock(async () => ({})), + dispose: mock(() => {}), + }; +} + +const getCdpClientMock = mock( + (ctx: ToolContext, opts?: CdpClientCallOpts) => { + getCdpClientCalls.push({ + mode: opts?.mode, + targetClientId: opts?.targetClientId, + }); + return makeFakeScopedClient("extension", ctx.conversationId); + }, +); + +// --------------------------------------------------------------------------- +// Mutable sticky-kind control +// --------------------------------------------------------------------------- + +let stickyKind: CdpClientKind | null = null; + +const setPreferredBackendKindMock = mock( + (_conversationId: string, _kind: CdpClientKind) => {}, +); +const clearPreferredBackendKindMock = mock((_conversationId: string) => {}); + +// --------------------------------------------------------------------------- +// Module mocks (must be declared before dynamic import) +// --------------------------------------------------------------------------- + +mock.module("../cdp-client/factory.js", () => ({ + getCdpClient: getCdpClientMock, + buildCandidateList: mock(() => []), + isDesktopAutoCooldownActive: () => false, +})); + +mock.module("../browser-manager.js", () => ({ + browserManager: { + getPreferredBackendKind: (_conversationId: string) => stickyKind, + setPreferredBackendKind: setPreferredBackendKindMock, + clearPreferredBackendKind: clearPreferredBackendKindMock, + storeSnapshotBackendNodeMap: () => {}, + clearSnapshotBackendNodeMap: () => {}, + resolveSnapshotBackendNodeId: () => undefined, + isInteractive: () => false, + supportsRouteInterception: false, + }, +})); + +mock.module("../../../config/loader.js", () => ({ + getConfig: () => ({ + hostBrowser: { + cdpInspect: { + enabled: false, + host: "localhost", + port: 9222, + probeTimeoutMs: 500, + desktopAuto: { enabled: false, cooldownMs: 30_000 }, + }, + }, + }), +})); + +mock.module("../../../daemon/host-browser-proxy.js", () => ({ + HostBrowserProxy: { + get instance() { + return { + isAvailable: () => false, + hasExtensionClient: () => false, + request: () => Promise.reject(new Error("no extension")), + }; + }, + }, +})); + +mock.module("../runtime-check.js", () => ({ + checkBrowserRuntime: async () => ({ + playwrightAvailable: true, + chromiumInstalled: true, + chromiumPath: "/tmp/chromium", + error: null, + }), +})); + +mock.module("../../../util/logger.js", () => ({ + getLogger: () => ({ + debug: () => {}, + warn: () => {}, + info: () => {}, + error: () => {}, + }), +})); + +// Import under test after all mock.module calls. +const { executeBrowserAttach } = await import("../browser-execution.js"); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeContext(conversationId: string): ToolContext { + return { + conversationId, + workingDir: "/tmp", + trustClass: "guardian", + signal: new AbortController().signal, + } as unknown as ToolContext; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("acquireCdpClientWithMode: target_client_id overrides sticky backend mode", () => { + beforeEach(() => { + getCdpClientCalls.length = 0; + getCdpClientMock.mockClear(); + setPreferredBackendKindMock.mockClear(); + clearPreferredBackendKindMock.mockClear(); + stickyKind = null; + }); + + test("sticky local + target_client_id → getCdpClient receives mode:extension, not local", async () => { + // Simulate a prior turn that pinned the conversation to the local backend. + stickyKind = "local"; + + await executeBrowserAttach( + { target_client_id: "host-client-abc" }, + makeContext("sticky-local-override"), + ); + + expect(getCdpClientMock).toHaveBeenCalledTimes(1); + // Fix 1: sticky "local" must be bypassed when target_client_id is present. + expect(getCdpClientCalls[0].mode).toBe("extension"); + expect(getCdpClientCalls[0].targetClientId).toBe("host-client-abc"); + }); + + test("sticky cdp-inspect + target_client_id → getCdpClient receives mode:extension, not cdp-inspect", async () => { + stickyKind = "cdp-inspect"; + + await executeBrowserAttach( + { target_client_id: "host-client-xyz" }, + makeContext("sticky-inspect-override"), + ); + + expect(getCdpClientMock).toHaveBeenCalledTimes(1); + expect(getCdpClientCalls[0].mode).toBe("extension"); + expect(getCdpClientCalls[0].targetClientId).toBe("host-client-xyz"); + }); + + test("sticky local + no target_client_id → getCdpClient receives mode:local (sticky honored)", async () => { + // Without target_client_id, the sticky preference must still apply. + stickyKind = "local"; + + await executeBrowserAttach( + {}, // no target_client_id + makeContext("sticky-honored"), + ); + + expect(getCdpClientMock).toHaveBeenCalledTimes(1); + expect(getCdpClientCalls[0].mode).toBe("local"); + expect(getCdpClientCalls[0].targetClientId).toBeUndefined(); + }); + + test("no sticky + target_client_id → getCdpClient receives mode:extension", async () => { + stickyKind = null; // No prior sticky preference + + await executeBrowserAttach( + { target_client_id: "host-client-fresh" }, + makeContext("no-sticky-targeted"), + ); + + expect(getCdpClientMock).toHaveBeenCalledTimes(1); + expect(getCdpClientCalls[0].mode).toBe("extension"); + expect(getCdpClientCalls[0].targetClientId).toBe("host-client-fresh"); + }); +}); diff --git a/assistant/src/tools/browser/browser-execution.ts b/assistant/src/tools/browser/browser-execution.ts index 9a60b6a3397..24be14ca133 100644 --- a/assistant/src/tools/browser/browser-execution.ts +++ b/assistant/src/tools/browser/browser-execution.ts @@ -386,8 +386,12 @@ function acquireCdpClientWithMode( const rememberedKind = browserManager.getPreferredBackendKind( context.conversationId, ); + // target_client_id requires the extension proxy path — bypass any sticky + // backend remembered from prior turns so the explicit target always wins. const effectiveMode: BrowserMode = - browserMode === "auto" && rememberedKind !== null + targetClientId != null + ? "extension" + : browserMode === "auto" && rememberedKind !== null ? rememberedKind : browserMode; @@ -400,7 +404,9 @@ function acquireCdpClientWithMode( // a remembered backend kind that has since become unavailable. Drop // the stale memo and retry with fresh auto selection so a dead // sticky preference doesn't surface as a hard failure. - if (browserMode === "auto" && effectiveMode !== "auto") { + // Do not apply this fallback when target_client_id is set — a targeting + // failure must surface as an error, not silently route elsewhere. + if (browserMode === "auto" && effectiveMode !== "auto" && targetClientId == null) { browserManager.clearPreferredBackendKind(context.conversationId); try { const raw = getCdpClient(context, { mode: "auto", targetClientId }); diff --git a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts index 7e1c439a4ad..3ea1c56647b 100644 --- a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts +++ b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts @@ -724,6 +724,58 @@ describe("getCdpClient", () => { "specific-ext-client", ); }); + + // ── auto mode + targetClientId: no fallback (Fix 2) ───────────────── + + test("auto mode + targetClientId + no extension → throws targeting error, does not fall through to local", () => { + // No extension connected — without targetClientId, local would be the + // fallback. With targetClientId, the factory must fail immediately. + const ctx = makeContext({ conversationId: "auto-targeted-no-ext" }); + + expect(() => + getCdpClient(ctx, { mode: "auto", targetClientId: "specific-host-client" }), + ).toThrow( + expect.objectContaining({ + code: "transport_error", + message: expect.stringContaining("specific-host-client"), + }), + ); + expect(createLocalCdpClientMock).not.toHaveBeenCalled(); + expect(createCdpInspectClientMock).not.toHaveBeenCalled(); + }); + + test("auto mode + targetClientId + extension available → routes only to extension, no other backends tried", async () => { + cdpInspectEnabled = true; // cdp-inspect would normally be in the candidate list + const fakeProxy = makeAvailableProxy(); + mockSingletonProxy = fakeProxy; + const ctx = makeContext({ + conversationId: "auto-targeted-ext-available", + sourceActorPrincipalId: "actor-42", + }); + + const client = getCdpClient(ctx, { + mode: "auto", + targetClientId: "specific-host-client", + }); + expect(client.kind).toBe("extension"); + + const result = await client.send<{ ok: boolean; via: string }>( + "Page.navigate", + ); + expect(result).toEqual({ ok: true, via: "extension" }); + + expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); + expect(createExtensionCdpClientMock).toHaveBeenCalledWith( + fakeProxy, + "auto-targeted-ext-available", + undefined, + "actor-42", + "specific-host-client", + ); + // cdp-inspect and local must NOT have been tried — no fallback. + expect(createCdpInspectClientMock).not.toHaveBeenCalled(); + expect(createLocalCdpClientMock).not.toHaveBeenCalled(); + }); }); // ── buildCandidateList tests ───────────────────────────────────────────── @@ -814,6 +866,62 @@ describe("buildCandidateList", () => { expect(candidates.length).toBe(1); expect(candidates[0].kind).toBe("local"); }); + + // ── targetClientId: no-fallback single-extension list (Fix 2) ──────── + + test("targetClientId: returns a single extension candidate when extension is available", () => { + const fakeProxy = makeAvailableProxy(); + mockSingletonProxy = fakeProxy; + const ctx = makeContext({ conversationId: "target-with-ext" }); + + const candidates = buildCandidateList(ctx, "host-client-42"); + + // Must be exactly one candidate — no cdp-inspect or local fallbacks. + expect(candidates.length).toBe(1); + expect(candidates[0].kind).toBe("extension"); + expect(candidates[0].reason).toContain("host-client-42"); + }); + + test("targetClientId: throws transport_error immediately when no Chrome Extension is connected", () => { + // No extension — local would normally be the fallback, but targeting + // requires the proxy path and must fail loudly instead of routing elsewhere. + const ctx = makeContext({ conversationId: "target-no-ext" }); + + expect(() => buildCandidateList(ctx, "host-client-42")).toThrow( + expect.objectContaining({ + code: "transport_error", + message: expect.stringContaining("host-client-42"), + }), + ); + // Verify we did NOT silently fall through to a local candidate. + expect(createLocalCdpClientMock).not.toHaveBeenCalled(); + }); + + test("targetClientId: threads targetClientId into the extension candidate's create() call", async () => { + const fakeProxy = makeAvailableProxy(); + mockSingletonProxy = fakeProxy; + const ctx = makeContext({ + conversationId: "target-threads-id", + sourceActorPrincipalId: "actor-1", + }); + + const candidates = buildCandidateList(ctx, "host-client-99"); + expect(candidates.length).toBe(1); + + // Clear accumulated calls from prior tests in this describe block before + // materialising the candidate so the call count is unambiguous. + createExtensionCdpClientMock.mockClear(); + candidates[0].create(); + + expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); + expect(createExtensionCdpClientMock).toHaveBeenCalledWith( + fakeProxy, + "target-threads-id", + undefined, + "actor-1", + "host-client-99", + ); + }); }); // ── buildChainedClient failover tests ──────────────────────────────────── diff --git a/assistant/src/tools/browser/cdp-client/factory.ts b/assistant/src/tools/browser/cdp-client/factory.ts index 80f098a1811..dab88e591dd 100644 --- a/assistant/src/tools/browser/cdp-client/factory.ts +++ b/assistant/src/tools/browser/cdp-client/factory.ts @@ -302,6 +302,52 @@ export function buildCandidateList(context: ToolContext, targetClientId?: string const candidates: BackendCandidate[] = []; const hostBrowserProxy = HostBrowserProxy.instance; + // When a specific host client is targeted, only the extension proxy can + // route to it. Skip the fallback chain entirely: if the extension is + // unavailable, fail loudly rather than silently routing to a different + // browser. + if (targetClientId != null) { + if (!hostBrowserProxy.hasExtensionClient()) { + throw new CdpError( + "transport_error", + `Cannot reach target_client_id "${targetClientId}": no Chrome Extension connected`, + { + attemptDiagnostics: [ + { + candidateKind: "extension", + inclusionReason: "target_client_id requires extension proxy", + stage: "candidate_selection", + errorCode: "transport_error", + errorMessage: "no Chrome Extension connected", + }, + ], + }, + ); + } + return [ + { + kind: "extension", + reason: `target_client_id override: ${targetClientId}`, + create() { + const client = createExtensionCdpClient( + hostBrowserProxy, + conversationId, + undefined, + sourceActorPrincipalId, + targetClientId, + ); + const backend = createExtensionBackend({ + isAvailable: () => true, + sendCdp: (command, signal) => + dispatchThroughClient(client, command, signal), + dispose: () => client.dispose(), + }); + return { client, backend }; + }, + }, + ]; + } + // 1. Extension -- preferred when a Chrome Extension client is connected. if (hostBrowserProxy.hasExtensionClient()) { candidates.push({ From 792d8b82e43ee4a90022e25fc3566b26fa06e9e1 Mon Sep 17 00:00:00 2001 From: credence-the-bot Date: Sat, 9 May 2026 11:34:49 +0000 Subject: [PATCH 3/3] chore(skills): regenerate catalog.json --- skills/catalog.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/catalog.json b/skills/catalog.json index 1f6dcfe0347..0ebd68a77a1 100644 --- a/skills/catalog.json +++ b/skills/catalog.json @@ -97,7 +97,7 @@ } }, "compatibility": "Designed for Vellum personal assistants", - "updatedAt": "2026-05-08T11:54:33Z" + "updatedAt": "2026-05-08T10:16:16-04:00" }, { "id": "document-writer", @@ -741,7 +741,7 @@ } }, "compatibility": "Designed for Vellum personal assistants", - "updatedAt": "2026-05-01T16:30:23-04:00" + "updatedAt": "2026-05-09T03:26:59Z" }, { "id": "vellum-conversation-management",