From 77bfe87f9f7c1e28b31ff7b7c18dfd9af199cf14 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:14:38 +0000 Subject: [PATCH 01/10] feat(host-proxy): add targetClientId support to HostFileProxy and HostProxyBase - Add targetClientId to HostFileReadRequest, HostFileWriteRequest, HostFileEditRequest, HostFileCancelRequest - Add targetClientId to HostCuRequest and HostCuCancelRequest - HostFileProxy.request() resolves target client (auto-resolve single, validate explicit, undefined for untargeted) - Thread targetClientId through all broadcastMessage calls (request, abort cancel, dispose cancel) - Add targetClientId to HostProxyBase.PendingEntry and dispatchRequest (covers HostCuProxy via inheritance) - Update broadcastDynamic to accept and forward targetClientId as options to broadcastMessage - Register targetClientId in assistant-event-hub.ts for host_file and host_cu pending interactions Co-authored-by: credence-the-bot Co-authored-by: Claude Sonnet 4.6 --- assistant/src/daemon/host-cu-proxy.ts | 22 +++++---- assistant/src/daemon/host-file-proxy.ts | 45 +++++++++++++++---- assistant/src/daemon/host-proxy-base.ts | 20 ++++++--- assistant/src/daemon/message-types/host-cu.ts | 2 + .../src/daemon/message-types/host-file.ts | 4 ++ 5 files changed, 71 insertions(+), 22 deletions(-) diff --git a/assistant/src/daemon/host-cu-proxy.ts b/assistant/src/daemon/host-cu-proxy.ts index 3e83827cb04..e43207d306f 100644 --- a/assistant/src/daemon/host-cu-proxy.ts +++ b/assistant/src/daemon/host-cu-proxy.ts @@ -128,6 +128,7 @@ export class HostCuProxy { stepNumber: number, reasoning?: string, signal?: AbortSignal, + targetClientId?: string, ): Promise { if (signal?.aborted) { return Promise.resolve({ @@ -184,6 +185,7 @@ export class HostCuProxy { pendingInteractions.register(requestId, { conversationId, kind: "host_cu", + targetClientId, rpcResolve: resolve, rpcReject: reject, timer, @@ -191,15 +193,19 @@ export class HostCuProxy { }); try { - broadcastMessage({ - type: "host_cu_request", - requestId, + broadcastMessage( + { + type: "host_cu_request", + requestId, + conversationId, + toolName, + input, + stepNumber, + reasoning, + }, conversationId, - toolName, - input, - stepNumber, - reasoning, - }); + { targetClientId }, + ); } catch (err) { this._ownedRequests.delete(requestId); pendingInteractions.resolve(requestId); diff --git a/assistant/src/daemon/host-file-proxy.ts b/assistant/src/daemon/host-file-proxy.ts index 1de150a6400..2e645681fe1 100644 --- a/assistant/src/daemon/host-file-proxy.ts +++ b/assistant/src/daemon/host-file-proxy.ts @@ -67,11 +67,29 @@ export class HostFileProxy { input: HostFileInput, conversationId: string, signal?: AbortSignal, + targetClientId?: string, ): Promise { if (signal?.aborted) { return Promise.resolve({ content: "Aborted", isError: true }); } + // Resolve targetClientId: explicit → validate; single capable client → auto-resolve. + let resolvedTargetClientId: string | undefined = targetClientId; + if (resolvedTargetClientId != null) { + const client = assistantEventHub.getClientById(resolvedTargetClientId); + if (!client || !client.capabilities.includes("host_file")) { + return Promise.resolve({ + content: `No connected client with id '${resolvedTargetClientId}' supports host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } + } else { + const capable = assistantEventHub.listClientsByCapability("host_file"); + if (capable.length === 1) { + resolvedTargetClientId = capable[0].clientId; + } + } + const requestId = uuid(); return new Promise((resolve, reject) => { @@ -96,11 +114,15 @@ export class HostFileProxy { if (pendingInteractions.get(requestId)) { pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_file_cancel", - requestId, + broadcastMessage( + { + type: "host_file_cancel", + requestId, + conversationId, + }, conversationId, - }); + { targetClientId: resolvedTargetClientId }, + ); } catch { // Best-effort cancel notification } @@ -114,6 +136,7 @@ export class HostFileProxy { pendingInteractions.register(requestId, { conversationId, kind: "host_file", + targetClientId: resolvedTargetClientId, rpcResolve: resolve, rpcReject: reject, timer, @@ -122,12 +145,16 @@ export class HostFileProxy { }); try { - broadcastMessage({ - ...input, - type: "host_file_request", - requestId, + broadcastMessage( + { + ...input, + type: "host_file_request", + requestId, + conversationId, + }, conversationId, - }); + { targetClientId: resolvedTargetClientId }, + ); } catch (err) { pendingInteractions.resolve(requestId); log.warn( diff --git a/assistant/src/daemon/host-proxy-base.ts b/assistant/src/daemon/host-proxy-base.ts index a275307538c..1cba16a2414 100644 --- a/assistant/src/daemon/host-proxy-base.ts +++ b/assistant/src/daemon/host-proxy-base.ts @@ -34,8 +34,12 @@ const log = getLogger("host-proxy-base"); * narrowing is impossible — subclasses are responsible for passing event * names that match a real `ServerMessage` variant. */ -function broadcastDynamic(envelope: Record): void { - broadcastMessage(envelope as unknown as ServerMessage); +function broadcastDynamic(envelope: Record, targetClientId?: string): void { + broadcastMessage( + envelope as unknown as ServerMessage, + undefined, + targetClientId ? { targetClientId } : undefined, + ); } const DEFAULT_TIMEOUT_MS = 60_000; @@ -63,6 +67,7 @@ interface PendingEntry { reject: (err: Error) => void; timer: ReturnType; conversationId: string; + targetClientId?: string; /** Detach the abort listener from the caller's signal. No-op when no signal was passed. */ detachAbort: () => void; } @@ -131,6 +136,7 @@ export abstract class HostProxyBase { conversationId: string, signal?: AbortSignal, extraFields?: Record, + targetClientId?: string, ): Promise { const requestId = uuid(); @@ -162,7 +168,8 @@ export abstract class HostProxyBase { type: this.cancelEventName, requestId, conversationId, - }); + targetClientId, + }, targetClientId); } catch { // Best-effort cancel notification — connection may already be closed. } @@ -178,6 +185,7 @@ export abstract class HostProxyBase { reject, timer, conversationId, + targetClientId, detachAbort, }); @@ -188,8 +196,9 @@ export abstract class HostProxyBase { conversationId, toolName, input, + targetClientId, ...(extraFields ?? {}), - }); + }, targetClientId); } catch (err) { clearTimeout(timer); this.pending.delete(requestId); @@ -246,7 +255,8 @@ export abstract class HostProxyBase { type: this.cancelEventName, requestId, conversationId: entry.conversationId, - }); + targetClientId: entry.targetClientId, + }, entry.targetClientId); } catch { // Best-effort cancel notification — connection may already be closed. } diff --git a/assistant/src/daemon/message-types/host-cu.ts b/assistant/src/daemon/message-types/host-cu.ts index 0b69918d882..fd11902a102 100644 --- a/assistant/src/daemon/message-types/host-cu.ts +++ b/assistant/src/daemon/message-types/host-cu.ts @@ -8,6 +8,7 @@ export interface HostCuRequest { type: "host_cu_request"; requestId: string; conversationId: string; + targetClientId?: string; toolName: string; // "computer_use_click", "computer_use_type_text", etc. input: Record; stepNumber: number; @@ -18,6 +19,7 @@ export interface HostCuCancelRequest { type: "host_cu_cancel"; requestId: string; conversationId: string; + targetClientId?: string; } // --- Domain-level union aliases (consumed by the barrel file) --- diff --git a/assistant/src/daemon/message-types/host-file.ts b/assistant/src/daemon/message-types/host-file.ts index 40f3f31c358..49e0a408781 100644 --- a/assistant/src/daemon/message-types/host-file.ts +++ b/assistant/src/daemon/message-types/host-file.ts @@ -8,6 +8,7 @@ export interface HostFileReadRequest { type: "host_file_request"; requestId: string; conversationId: string; + targetClientId?: string; operation: "read"; path: string; offset?: number; @@ -18,6 +19,7 @@ export interface HostFileWriteRequest { type: "host_file_request"; requestId: string; conversationId: string; + targetClientId?: string; operation: "write"; path: string; content: string; @@ -27,6 +29,7 @@ export interface HostFileEditRequest { type: "host_file_request"; requestId: string; conversationId: string; + targetClientId?: string; operation: "edit"; path: string; old_string: string; @@ -43,6 +46,7 @@ export interface HostFileCancelRequest { type: "host_file_cancel"; requestId: string; conversationId: string; + targetClientId?: string; } // --- Domain-level union aliases (consumed by the barrel file) --- From 835c3f2acb73cebd7d78cc5c4c4109f081c20f62 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:22:45 +0000 Subject: [PATCH 02/10] feat(routes): enforce targetClientId binding on host-file-result and host-cu-result (#29395) Co-authored-by: credence-the-bot Co-authored-by: Claude Sonnet 4.6 --- assistant/src/runtime/routes/host-cu-routes.ts | 13 ++++++++++++- assistant/src/runtime/routes/host-file-routes.ts | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/assistant/src/runtime/routes/host-cu-routes.ts b/assistant/src/runtime/routes/host-cu-routes.ts index 0fb1db04cab..9d9a5b02318 100644 --- a/assistant/src/runtime/routes/host-cu-routes.ts +++ b/assistant/src/runtime/routes/host-cu-routes.ts @@ -15,7 +15,7 @@ import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; // POST /v1/host-cu-result // --------------------------------------------------------------------------- -function handleHostCuResult({ body }: RouteHandlerArgs) { +function handleHostCuResult({ body, headers }: RouteHandlerArgs) { if (!body || typeof body !== "object") { throw new BadRequestError("Request body is required"); } @@ -63,6 +63,17 @@ function handleHostCuResult({ body }: RouteHandlerArgs) { ); } + // Validate submitting client matches the targeted client (if any). + if (peeked.targetClientId != null) { + const submittingClientId = (headers as Record)?.["x-vellum-client-id"]; + if (!submittingClientId) { + throw new BadRequestError("x-vellum-client-id header is missing for a targeted host CU request."); + } + if (submittingClientId !== peeked.targetClientId) { + throw new ConflictError("Submitting client does not match the targeted client for this request."); + } + } + const conversation = findConversation(peeked.conversationId); if (!conversation) { pendingInteractions.resolve(requestId); diff --git a/assistant/src/runtime/routes/host-file-routes.ts b/assistant/src/runtime/routes/host-file-routes.ts index ea156370db7..ba342c08380 100644 --- a/assistant/src/runtime/routes/host-file-routes.ts +++ b/assistant/src/runtime/routes/host-file-routes.ts @@ -15,7 +15,7 @@ import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; // POST /v1/host-file-result // --------------------------------------------------------------------------- -function handleHostFileResult({ body }: RouteHandlerArgs) { +function handleHostFileResult({ body, headers }: RouteHandlerArgs) { if (!body || typeof body !== "object") { throw new BadRequestError("Request body is required"); } @@ -42,6 +42,17 @@ function handleHostFileResult({ body }: RouteHandlerArgs) { ); } + // Validate submitting client matches the targeted client (if any). + if (peeked.targetClientId != null) { + const submittingClientId = (headers as Record)?.["x-vellum-client-id"]; + if (!submittingClientId) { + throw new BadRequestError("x-vellum-client-id header is missing for a targeted host file request."); + } + if (submittingClientId !== peeked.targetClientId) { + throw new ConflictError("Submitting client does not match the targeted client for this request."); + } + } + HostFileProxy.instance.resolveResult(requestId, { content: content ?? "", isError: isError ?? false, From 578b345a70a73d5e2e32b8f0c37952f4a583c3a9 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:22:48 +0000 Subject: [PATCH 03/10] feat(tools): add target_client_id to host_file_* and computer_use_* tools (#29397) Co-authored-by: credence-the-bot Co-authored-by: Claude Sonnet 4.6 --- assistant/src/daemon/conversation-surfaces.ts | 5 +++ .../src/tools/computer-use/definitions.ts | 40 +++++++++++++++++++ assistant/src/tools/host-filesystem/edit.ts | 26 ++++++++++++ assistant/src/tools/host-filesystem/read.ts | 26 ++++++++++++ assistant/src/tools/host-filesystem/write.ts | 26 ++++++++++++ 5 files changed, 123 insertions(+) diff --git a/assistant/src/daemon/conversation-surfaces.ts b/assistant/src/daemon/conversation-surfaces.ts index 6df0eaa781a..962e0990bba 100644 --- a/assistant/src/daemon/conversation-surfaces.ts +++ b/assistant/src/daemon/conversation-surfaces.ts @@ -1780,6 +1780,10 @@ export async function surfaceProxyResolver( // Record the action and proxy to the connected desktop client const reasoning = typeof input.reasoning === "string" ? input.reasoning : undefined; + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; ctx.hostCuProxy.recordAction(toolName, input, reasoning); return ctx.hostCuProxy.request( toolName, @@ -1788,6 +1792,7 @@ export async function surfaceProxyResolver( ctx.hostCuProxy.stepCount, reasoning, signal, + targetClientId, ); } diff --git a/assistant/src/tools/computer-use/definitions.ts b/assistant/src/tools/computer-use/definitions.ts index 0d237092761..7bbd9db4da0 100644 --- a/assistant/src/tools/computer-use/definitions.ts +++ b/assistant/src/tools/computer-use/definitions.ts @@ -63,6 +63,11 @@ export const computerUseClickTool: Tool = { description: "Explanation of what you see and why you are clicking here", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["reasoning"], }, @@ -99,6 +104,11 @@ export const computerUseTypeTextTool: Tool = { type: "string", description: "Explanation of what you are typing and why", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["text", "reasoning"], }, @@ -136,6 +146,11 @@ export const computerUseKeyTool: Tool = { type: "string", description: "Explanation of why you are pressing this key", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["key", "reasoning"], }, @@ -190,6 +205,11 @@ export const computerUseScrollTool: Tool = { type: "string", description: "Explanation of why you are scrolling", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["direction", "amount", "reasoning"], }, @@ -250,6 +270,11 @@ export const computerUseDragTool: Tool = { type: "string", description: "Explanation of what you are dragging and why", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["reasoning"], }, @@ -285,6 +310,11 @@ export const computerUseWaitTool: Tool = { type: "string", description: "Explanation of what you are waiting for", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["duration_ms", "reasoning"], }, @@ -323,6 +353,11 @@ export const computerUseOpenAppTool: Tool = { description: "Explanation of why you need to open or switch to this app", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["app_name", "reasoning"], }, @@ -360,6 +395,11 @@ export const computerUseRunAppleScriptTool: Tool = { description: "Explanation of what this script does and why AppleScript is better than UI interaction for this step", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`.", + }, }, required: ["script", "reasoning"], }, diff --git a/assistant/src/tools/host-filesystem/edit.ts b/assistant/src/tools/host-filesystem/edit.ts index 4100ff260e1..4c606d4eb3a 100644 --- a/assistant/src/tools/host-filesystem/edit.ts +++ b/assistant/src/tools/host-filesystem/edit.ts @@ -1,6 +1,8 @@ +import { supportsHostProxy } from "../../channels/types.js"; import { HostFileProxy } from "../../daemon/host-file-proxy.js"; import { RiskLevel } from "../../permissions/types.js"; import type { ToolDefinition } from "../../providers/types.js"; +import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; import { FileSystemOps } from "../shared/filesystem/file-ops-service.js"; import { formatEditDiff } from "../shared/filesystem/format-diff.js"; import { hostPolicy } from "../shared/filesystem/path-policy.js"; @@ -37,6 +39,11 @@ class HostFileEditTool implements Tool { description: "Replace all occurrences instead of requiring a unique match (default: false)", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to execute this on. Required when multiple clients support host_file; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_file`.", + }, }, required: ["path", "old_string", "new_string"], }, @@ -84,6 +91,24 @@ class HostFileEditTool implements Tool { const replaceAll = input.replace_all === true; + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + + const transportInterface = context.transportInterface; + if ( + targetClientId == null && + transportInterface != null && + !supportsHostProxy(transportInterface) && + assistantEventHub.listClientsByCapability("host_file").length > 1 + ) { + return { + content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, + isError: true, + }; + } + // Proxy to connected client for execution on the user's machine // when a capable client is available (managed/cloud-hosted mode). if (HostFileProxy.instance.isAvailable()) { @@ -94,6 +119,7 @@ class HostFileEditTool implements Tool { old_string: oldString as string, new_string: newString as string, replace_all: replaceAll, + targetClientId, }, context.conversationId, context.signal, diff --git a/assistant/src/tools/host-filesystem/read.ts b/assistant/src/tools/host-filesystem/read.ts index a41220255a8..bfe219701d0 100644 --- a/assistant/src/tools/host-filesystem/read.ts +++ b/assistant/src/tools/host-filesystem/read.ts @@ -1,8 +1,10 @@ import { extname } from "node:path"; +import { supportsHostProxy } from "../../channels/types.js"; import { HostFileProxy } from "../../daemon/host-file-proxy.js"; import { RiskLevel } from "../../permissions/types.js"; import type { ToolDefinition } from "../../providers/types.js"; +import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; import { FileSystemOps } from "../shared/filesystem/file-ops-service.js"; import { IMAGE_EXTENSIONS, @@ -37,6 +39,11 @@ class HostFileReadTool implements Tool { type: "number", description: "Maximum number of lines to read", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to execute this on. Required when multiple clients support host_file; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_file`.", + }, }, required: ["path"], }, @@ -55,6 +62,24 @@ class HostFileReadTool implements Tool { }; } + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + + const transportInterface = context.transportInterface; + if ( + targetClientId == null && + transportInterface != null && + !supportsHostProxy(transportInterface) && + assistantEventHub.listClientsByCapability("host_file").length > 1 + ) { + return { + content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, + isError: true, + }; + } + // Proxy to connected client for execution on the user's machine // when a capable client is available (managed/cloud-hosted mode), // including image reads that need the host filesystem view. @@ -65,6 +90,7 @@ class HostFileReadTool implements Tool { path: rawPath, offset: typeof input.offset === "number" ? input.offset : undefined, limit: typeof input.limit === "number" ? input.limit : undefined, + targetClientId, }, context.conversationId, context.signal, diff --git a/assistant/src/tools/host-filesystem/write.ts b/assistant/src/tools/host-filesystem/write.ts index 3113545ef61..be23354bd7c 100644 --- a/assistant/src/tools/host-filesystem/write.ts +++ b/assistant/src/tools/host-filesystem/write.ts @@ -1,6 +1,8 @@ +import { supportsHostProxy } from "../../channels/types.js"; import { HostFileProxy } from "../../daemon/host-file-proxy.js"; import { RiskLevel } from "../../permissions/types.js"; import type { ToolDefinition } from "../../providers/types.js"; +import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; import { FileSystemOps } from "../shared/filesystem/file-ops-service.js"; import { formatWriteSummary } from "../shared/filesystem/format-diff.js"; import { hostPolicy } from "../shared/filesystem/path-policy.js"; @@ -28,6 +30,11 @@ class HostFileWriteTool implements Tool { type: "string", description: "The content to write to the file", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to execute this on. Required when multiple clients support host_file; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_file`.", + }, }, required: ["path", "content"], }, @@ -54,6 +61,24 @@ class HostFileWriteTool implements Tool { }; } + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + + const transportInterface = context.transportInterface; + if ( + targetClientId == null && + transportInterface != null && + !supportsHostProxy(transportInterface) && + assistantEventHub.listClientsByCapability("host_file").length > 1 + ) { + return { + content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, + isError: true, + }; + } + // Proxy to connected client for execution on the user's machine // when a capable client is available (managed/cloud-hosted mode). if (HostFileProxy.instance.isAvailable()) { @@ -62,6 +87,7 @@ class HostFileWriteTool implements Tool { operation: "write", path: rawPath, content: fileContent, + targetClientId, }, context.conversationId, context.signal, From d3bc701a8c2faf261293715376d787c7fa160d78 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:23:00 +0000 Subject: [PATCH 04/10] feat(macos): targetClientId acceptance guard and result header for host_file and host_cu (#29396) Co-authored-by: credence-the-bot Co-authored-by: Claude Sonnet 4.6 --- .../App/AppDelegate+ConnectionSetup.swift | 16 ++++++++++++++++ clients/shared/Network/EventStreamClient.swift | 4 ++++ clients/shared/Network/HostProxyClient.swift | 2 ++ clients/shared/Network/MessageTypes.swift | 8 ++++++++ 4 files changed, 30 insertions(+) diff --git a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift index e297909fbed..a9cea3eaa41 100644 --- a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift +++ b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift @@ -380,8 +380,24 @@ extension AppDelegate { } HostToolExecutor.executeHostBashRequest(msg) case .hostFileRequest(let msg): + let localClientId = DeviceIdStore.getOrCreate() + let isLocalConversation = self.mainWindow?.conversationManager + .conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false + let isTargeted = msg.targetClientId == localClientId + let isUntargetedLocal = msg.targetClientId == nil && isLocalConversation + guard isTargeted || isUntargetedLocal else { + break + } HostToolExecutor.executeHostFileRequest(msg) case .hostCuRequest(let msg): + let localClientId = DeviceIdStore.getOrCreate() + let isLocalConversation = self.mainWindow?.conversationManager + .conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false + let isTargeted = msg.targetClientId == localClientId + let isUntargetedLocal = msg.targetClientId == nil && isLocalConversation + guard isTargeted || isUntargetedLocal else { + break + } let proxy = self.getOrCreateHostCuOverlay(conversationId: msg.conversationId, request: msg) let task = Task { @MainActor in defer { self.inFlightCuTasks.removeValue(forKey: msg.requestId) } diff --git a/clients/shared/Network/EventStreamClient.swift b/clients/shared/Network/EventStreamClient.swift index e1d30af9ff8..5e91507c6bc 100644 --- a/clients/shared/Network/EventStreamClient.swift +++ b/clients/shared/Network/EventStreamClient.swift @@ -614,10 +614,14 @@ public final class EventStreamClient { log.warning("Ignoring host_bash_request for non-local conversation \(msg.conversationId, privacy: .public)") return true case .hostFileRequest(let msg): + // Targeted cross-client requests carry a non-local conversationId by design. + // Pass them through so AppDelegate+ConnectionSetup can perform the targetClientId check. + if msg.targetClientId != nil { return false } if locallyOwnedConversationIds.contains(msg.conversationId) { return false } log.warning("Ignoring host_file_request for non-local conversation \(msg.conversationId, privacy: .public)") return true case .hostCuRequest(let msg): + if msg.targetClientId != nil { return false } if locallyOwnedConversationIds.contains(msg.conversationId) { return false } log.warning("Ignoring host_cu_request for non-local conversation \(msg.conversationId, privacy: .public)") return true diff --git a/clients/shared/Network/HostProxyClient.swift b/clients/shared/Network/HostProxyClient.swift index 5b6b7a25d0a..0719a8bbff9 100644 --- a/clients/shared/Network/HostProxyClient.swift +++ b/clients/shared/Network/HostProxyClient.swift @@ -50,6 +50,7 @@ public struct HostProxyClient: HostProxyClientProtocol { let response = try await GatewayHTTPClient.post( path: "host-file-result", body: body, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: timeout ) guard response.isSuccess else { @@ -69,6 +70,7 @@ public struct HostProxyClient: HostProxyClientProtocol { let response = try await GatewayHTTPClient.post( path: "host-cu-result", body: body, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: 30 ) guard response.isSuccess else { diff --git a/clients/shared/Network/MessageTypes.swift b/clients/shared/Network/MessageTypes.swift index 002e725ce30..3acc82db110 100644 --- a/clients/shared/Network/MessageTypes.swift +++ b/clients/shared/Network/MessageTypes.swift @@ -1614,6 +1614,9 @@ public struct HostFileRequest: Decodable, Sendable { public let oldString: String? public let newString: String? public let replaceAll: Bool? + /// When set, this request is targeted at a specific client ID. Non-nil only for + /// cross-client proxy requests routed through HostFileProxy. + public let targetClientId: String? private enum CodingKeys: String, CodingKey { case type, requestId, conversationId, operation, path @@ -1621,6 +1624,7 @@ public struct HostFileRequest: Decodable, Sendable { case oldString = "old_string" case newString = "new_string" case replaceAll = "replace_all" + case targetClientId } } @@ -1659,6 +1663,9 @@ public struct HostCuRequest: Decodable, Sendable { public let input: [String: AnyCodable] public let stepNumber: Int public let reasoning: String? + /// When set, this request is targeted at a specific client ID. Non-nil only for + /// cross-client proxy requests routed through HostCuProxy. + public let targetClientId: String? private enum CodingKeys: String, CodingKey { case type @@ -1668,6 +1675,7 @@ public struct HostCuRequest: Decodable, Sendable { case input case stepNumber case reasoning + case targetClientId } } From 3fe1dedaf773ab94eb11d4d8407d8d90f0069956 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" <277301654+credence-the-bot[bot]@users.noreply.github.com> Date: Sun, 3 May 2026 13:58:41 +0000 Subject: [PATCH 05/10] test(host-proxy): unit + route tests for Phase 2 targetClientId (#29402) Adds targeted tests for Phase 2 proxy changes: - HostFileProxy target resolution (auto-resolve, explicit validate, unknown/incapable client rejection, cancel with targetClientId, timeout message with clientId) - host-file-result route 403 guard (correct/missing/wrong x-vellum-client-id header, untargeted regression) - host-cu-result route 403 guard (same 4 cases, mocked conversation store via dynamic import to avoid deep service-contracts chain) - target_client_id passthrough to HostFileProxy.instance.request in host_file_read, host_file_write, and host_file_edit tools Co-authored-by: credence-the-bot Co-authored-by: Claude Sonnet 4.6 --- .../__tests__/host-cu-routes-targeted.test.ts | 276 +++++++++++++++ .../src/__tests__/host-file-edit-tool.test.ts | 48 ++- .../host-file-proxy-targeted.test.ts | 330 ++++++++++++++++++ .../src/__tests__/host-file-read-tool.test.ts | 17 + .../host-file-routes-targeted.test.ts | 256 ++++++++++++++ .../__tests__/host-file-write-tool.test.ts | 43 ++- 6 files changed, 968 insertions(+), 2 deletions(-) create mode 100644 assistant/src/__tests__/host-cu-routes-targeted.test.ts create mode 100644 assistant/src/__tests__/host-file-proxy-targeted.test.ts create mode 100644 assistant/src/__tests__/host-file-routes-targeted.test.ts diff --git a/assistant/src/__tests__/host-cu-routes-targeted.test.ts b/assistant/src/__tests__/host-cu-routes-targeted.test.ts new file mode 100644 index 00000000000..d15f7bb72e6 --- /dev/null +++ b/assistant/src/__tests__/host-cu-routes-targeted.test.ts @@ -0,0 +1,276 @@ +/** + * Tests for the host-cu-result route 403 guard introduced in Phase 2. + * + * Covers: + * 1. Targeted + correct x-vellum-client-id header → 200 accepted + * 2. Targeted + missing header → 400 BadRequestError + * 3. Targeted + wrong header → 403 ForbiddenError, interaction NOT consumed + * 4. Untargeted (no targetClientId, no header) → 200 accepted (regression) + * + * Resolution goes through conversation.hostCuProxy?.resolve(...). The + * conversation store is mocked to return a controlled conversation object. + * + * Note: host-cu-routes.ts has a deep import chain (conversation-store → + * conversation.ts → ces-client → service-contracts) that requires mocking + * before the module loads. We use dynamic imports to ensure all mocks are + * registered before the route module is evaluated. + */ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; + +// ── Module mocks ───────────────────────────────────────────────────────────── +// Must be registered before the host-cu-routes module is loaded. + +mock.module("../config/env.js", () => ({ + isHttpAuthDisabled: () => true, + hasUngatedHttpAuthDisabled: () => false, +})); + +import type { PendingInteraction } from "../runtime/pending-interactions.js"; + +const pendingStore = new Map(); +const resolvedIds: string[] = []; + +mock.module("../runtime/pending-interactions.js", () => ({ + get: (requestId: string) => pendingStore.get(requestId), + resolve: (requestId: string) => { + const entry = pendingStore.get(requestId); + if (entry) { + pendingStore.delete(requestId); + resolvedIds.push(requestId); + } + return entry; + }, +})); + +interface CuResolveCall { + requestId: string; + payload: Record; +} + +const cuResolveSpy: CuResolveCall[] = []; + +// Controlled conversation map: conversationId → conversation object +const conversationStore = new Map void } }>(); + +mock.module("../daemon/conversation-store.js", () => ({ + findConversation: (conversationId: string) => conversationStore.get(conversationId), +})); + +// ── Real imports (after mocks) ────────────────────────────────────────────── +// Use dynamic import to ensure the mocks above are applied before loading. + +import { + BadRequestError, + ForbiddenError, +} from "../runtime/routes/errors.js"; + +const { ROUTES } = await import("../runtime/routes/host-cu-routes.js"); + +afterAll(() => { + mock.restore(); +}); + +const handleHostCuResult = ROUTES.find( + (r: { endpoint: string }) => r.endpoint === "host-cu-result", +)!.handler; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function registerPending( + requestId: string, + overrides: Partial = {}, +): void { + const entry: PendingInteraction = { + conversationId: "conv-cu-1", + kind: "host_cu", + ...overrides, + }; + pendingStore.set(requestId, entry); +} + +function registerConversation(conversationId = "conv-cu-1"): void { + conversationStore.set(conversationId, { + hostCuProxy: { + resolve(requestId: unknown, payload: unknown) { + cuResolveSpy.push({ + requestId: requestId as string, + payload: payload as Record, + }); + }, + }, + }); +} + +function cuBody(requestId: string): Record { + return { + requestId, + axTree: "Button [1]", + executionResult: "Clicked", + }; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe("handleHostCuResult — Phase 2 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + conversationStore.clear(); + resolvedIds.length = 0; + cuResolveSpy.length = 0; + // Default: register a conversation with a hostCuProxy + registerConversation("conv-cu-1"); + }); + + // ── 1. Targeted + correct header → 200 ──────────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and resolves the interaction", async () => { + const requestId = "req-cu-targeted-match"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + expect(cuResolveSpy[0].requestId).toBe(requestId); + expect(resolvedIds).toContain(requestId); + }); + + test("trims whitespace from header before comparing", async () => { + const requestId = "req-cu-targeted-trim"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": " client-A " }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── 2. Targeted + missing header → 400 ──────────────────────────────────── + + describe("targeted + missing x-vellum-client-id header", () => { + test("throws BadRequestError (400) when header is absent", () => { + const requestId = "req-cu-targeted-no-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ body: cuBody(requestId) }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError (400) when header is empty string", () => { + const requestId = "req-cu-targeted-empty-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT resolved on 400 (still pending)", () => { + const requestId = "req-cu-targeted-no-header-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostCuResult({ body: cuBody(requestId) }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 3. Targeted + wrong header → 403 ────────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError (403) when client ID does not match", () => { + const requestId = "req-cu-targeted-mismatch"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both submitting and expected client", () => { + const requestId = "req-cu-targeted-mismatch-msg"; + registerPending(requestId, { targetClientId: "client-A" }); + + let caught: unknown; + try { + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-B"); + expect(msg).toContain("client-A"); + }); + + test("interaction is NOT consumed on 403 (pendingInteractions.get still returns it)", () => { + const requestId = "req-cu-targeted-mismatch-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when no header is provided", async () => { + const requestId = "req-cu-untargeted-no-header"; + registerPending(requestId); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + + test("accepts when header is present (header ignored for untargeted)", async () => { + const requestId = "req-cu-untargeted-with-header"; + registerPending(requestId); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-whatever" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + }); + }); +}); diff --git a/assistant/src/__tests__/host-file-edit-tool.test.ts b/assistant/src/__tests__/host-file-edit-tool.test.ts index bd0b031ddc1..58cb2fcdb12 100644 --- a/assistant/src/__tests__/host-file-edit-tool.test.ts +++ b/assistant/src/__tests__/host-file-edit-tool.test.ts @@ -1,7 +1,29 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; + +import type { HostFileInput } from "../daemon/host-file-proxy.js"; +import type { ToolExecutionResult } from "../tools/types.js"; + +// Mock HostFileProxy singleton so proxy delegation tests can control it. +let mockFileProxyAvailable = false; +let mockFileProxyRequestFn: ( + input: HostFileInput, + conversationId: string, + signal?: AbortSignal, +) => Promise = () => Promise.resolve({ content: "", isError: false }); + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + isAvailable: () => mockFileProxyAvailable, + request: mockFileProxyRequestFn, + }; + }, + }, +})); import { hostFileEditTool } from "../tools/host-filesystem/edit.js"; import type { ToolContext } from "../tools/types.js"; @@ -20,6 +42,8 @@ afterEach(() => { for (const dir of testDirs.splice(0)) { rmSync(dir, { recursive: true, force: true }); } + mockFileProxyAvailable = false; + mockFileProxyRequestFn = () => Promise.resolve({ content: "", isError: false }); }); describe("host_file_edit tool", () => { @@ -268,4 +292,26 @@ describe("host_file_edit tool", () => { result.content.includes("Successfully edited"), ).toBe(true); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied edit", isError: false }; + }; + + await hostFileEditTool.execute( + { + path: "/host/file.txt", + old_string: "old", + new_string: "new", + target_client_id: "client-x", + }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); }); diff --git a/assistant/src/__tests__/host-file-proxy-targeted.test.ts b/assistant/src/__tests__/host-file-proxy-targeted.test.ts new file mode 100644 index 00000000000..d498a49c944 --- /dev/null +++ b/assistant/src/__tests__/host-file-proxy-targeted.test.ts @@ -0,0 +1,330 @@ +/** + * Tests for HostFileProxy Phase 2 targetClientId behaviour. + * + * Covers: + * - Explicit targetClientId validation (valid, unknown, incapable) + * - Auto-resolve when exactly one host_file-capable client is connected + * - Untargeted broadcast when multiple capable clients are connected + * - targetClientId propagated into cancel messages (abort + dispose) + * - Timeout message includes clientId when resolvedTargetClientId is set + */ +import { afterEach, describe, expect, jest, mock, test } from "bun:test"; + +const sentMessages: unknown[] = []; +const sentMessageOptions: unknown[] = []; +const resolvedInteractionIds: string[] = []; +let mockHasClient = false; +let mockCapableClients: Array<{ clientId: string; capabilities: string[] }> = []; +let mockClientRegistry: Map = new Map(); + +mock.module("../runtime/assistant-event-hub.js", () => ({ + broadcastMessage: (msg: unknown, _conversationId?: string, options?: unknown) => { + sentMessages.push(msg); + sentMessageOptions.push(options); + }, + assistantEventHub: { + getMostRecentClientByCapability: (cap: string) => + cap === "host_file" && mockHasClient ? { id: "mock-client" } : null, + listClientsByCapability: (_cap: string) => mockCapableClients, + getClientById: (clientId: string) => mockClientRegistry.get(clientId), + }, +})); + +mock.module("../runtime/pending-interactions.js", () => ({ + resolve: (requestId: string) => { + resolvedInteractionIds.push(requestId); + return undefined; + }, + get: () => undefined, + getByKind: () => [], + getByConversation: () => [], + removeByConversation: () => {}, +})); + +const { HostFileProxy } = await import("../daemon/host-file-proxy.js"); + +describe("HostFileProxy — targetClientId (Phase 2)", () => { + let proxy: InstanceType; + + function setup() { + sentMessages.length = 0; + sentMessageOptions.length = 0; + resolvedInteractionIds.length = 0; + mockHasClient = false; + mockCapableClients = []; + mockClientRegistry = new Map(); + proxy = new (HostFileProxy as any)(); + } + + function setupSingleClient(clientId = "client-1") { + const entry = { clientId, capabilities: ["host_file"] }; + mockCapableClients = [entry]; + mockClientRegistry.set(clientId, entry); + } + + function setupMultipleClients(clientIds: string[]) { + mockCapableClients = clientIds.map((id) => ({ + clientId: id, + capabilities: ["host_file"], + })); + for (const entry of mockCapableClients) { + mockClientRegistry.set(entry.clientId, entry); + } + } + + afterEach(() => { + proxy?.dispose(); + HostFileProxy.reset(); + }); + + // ── Explicit targetClientId — valid ────────────────────────────────── + + describe("explicit targetClientId — valid client with host_file", () => { + test("resolves to that client and broadcasts with targetClientId option", async () => { + setup(); + setupSingleClient("client-mac"); + // Also add a second client so explicit targeting is meaningful + const entry2 = { clientId: "client-linux", capabilities: ["host_file"] }; + mockCapableClients.push(entry2); + mockClientRegistry.set("client-linux", entry2); + + const resultPromise = proxy.request( + { + operation: "read", + path: "/home/user/notes.txt", + targetClientId: "client-mac", + }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_file_request"); + expect(sent.targetClientId).toBe("client-mac"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-mac"); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { content: "file contents", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── Explicit targetClientId — unknown client ───────────────────────── + + describe("explicit targetClientId — unknown client", () => { + test("returns error result immediately without broadcasting", async () => { + setup(); + setupSingleClient("client-mac"); + + const result = await proxy.request( + { + operation: "read", + path: "/tmp/file.txt", + targetClientId: "client-unknown", + }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-unknown"); + expect(result.content).toContain("assistant clients list --capability host_file"); + // No pending entry should have been created + expect(sentMessages).toHaveLength(0); + }); + + test("does not create a pending entry for unknown client", async () => { + setup(); + + const result = await proxy.request( + { + operation: "write", + path: "/tmp/out.txt", + content: "data", + targetClientId: "client-ghost", + }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── Explicit targetClientId — incapable client ─────────────────────── + + describe("explicit targetClientId — connected but lacks host_file", () => { + test("returns error result immediately without broadcasting", async () => { + setup(); + // Register a client that exists but does not have host_file + mockClientRegistry.set("client-no-file", { + clientId: "client-no-file", + capabilities: ["host_bash"], + }); + + const result = await proxy.request( + { + operation: "read", + path: "/tmp/test.txt", + targetClientId: "client-no-file", + }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-no-file"); + expect(result.content).toContain("does not support host_file"); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── Auto-resolve single capable client ─────────────────────────────── + + describe("auto-resolve single capable client", () => { + test("resolves target when exactly one host_file-capable client is connected", async () => { + setup(); + setupSingleClient("client-solo"); + + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-solo"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-solo"); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { content: "ok", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── No target — multiple capable clients ───────────────────────────── + + describe("no explicit target — multiple capable clients", () => { + test("broadcasts without targetClientId (untargeted)", async () => { + setup(); + setupMultipleClients(["client-1", "client-2"]); + + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_file_request"); + expect(sent.targetClientId).toBeUndefined(); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBeUndefined(); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { content: "ok", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── targetClientId in cancel (abort signal) ────────────────────────── + + describe("targetClientId in cancel — abort signal", () => { + test("cancel broadcast includes targetClientId when request was targeted", async () => { + setup(); + setupSingleClient("client-abc"); + + const controller = new AbortController(); + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + controller.signal, + ); + + const sent = sentMessages[0] as Record; + const requestId = sent.requestId as string; + expect(sent.targetClientId).toBe("client-abc"); + + controller.abort(); + await resultPromise; + + // Second message is the cancel + expect(sentMessages).toHaveLength(2); + const cancelMsg = sentMessages[1] as Record; + expect(cancelMsg.type).toBe("host_file_cancel"); + expect(cancelMsg.requestId).toBe(requestId); + expect(cancelMsg.targetClientId).toBe("client-abc"); + + const cancelOpts = sentMessageOptions[1] as Record | undefined; + expect(cancelOpts?.targetClientId).toBe("client-abc"); + }); + }); + + // ── targetClientId in cancel (dispose) ─────────────────────────────── + + describe("targetClientId in cancel — dispose", () => { + test("dispose cancel broadcast includes targetClientId for targeted request", () => { + setup(); + setupSingleClient("client-xyz"); + + const p = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + p.catch(() => {}); // expected rejection on dispose + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-xyz"); + const requestId = sent.requestId as string; + + proxy.dispose(); + + const cancelMessages = sentMessages + .slice(1) + .filter( + (m) => (m as Record).type === "host_file_cancel", + ) as Array>; + expect(cancelMessages).toHaveLength(1); + expect(cancelMessages[0].requestId).toBe(requestId); + expect(cancelMessages[0].targetClientId).toBe("client-xyz"); + }); + }); + + // ── Timeout message includes clientId ──────────────────────────────── + + describe("timeout message includes clientId", () => { + test("timeout resolve message mentions resolvedTargetClientId", async () => { + setup(); + setupSingleClient("client-mac"); + + jest.useFakeTimers(); + try { + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/slow.txt" }, + "session-1", + ); + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-mac"); + + // Advance past the 30s internal timeout + jest.advanceTimersByTime(31 * 1000); + + const result = await resultPromise; + expect(result.isError).toBe(true); + expect(result.content).toContain("client-mac"); + } finally { + jest.useRealTimers(); + } + }); + }); +}); diff --git a/assistant/src/__tests__/host-file-read-tool.test.ts b/assistant/src/__tests__/host-file-read-tool.test.ts index ba26705479e..17f4726ecd9 100644 --- a/assistant/src/__tests__/host-file-read-tool.test.ts +++ b/assistant/src/__tests__/host-file-read-tool.test.ts @@ -304,4 +304,21 @@ describe("host_file_read image support", () => { expect(result.content).toContain("2 second line"); expect((result as any).contentBlocks).toBeUndefined(); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied", isError: false }; + }; + + await hostFileReadTool.execute( + { path: "/host/notes.txt", target_client_id: "client-x" }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); }); diff --git a/assistant/src/__tests__/host-file-routes-targeted.test.ts b/assistant/src/__tests__/host-file-routes-targeted.test.ts new file mode 100644 index 00000000000..e1252ec3718 --- /dev/null +++ b/assistant/src/__tests__/host-file-routes-targeted.test.ts @@ -0,0 +1,256 @@ +/** + * Tests for the host-file-result route 403 guard introduced in Phase 2. + * + * Covers: + * 1. Targeted + correct x-vellum-client-id header → 200 accepted + * 2. Targeted + missing header → 400 BadRequestError + * 3. Targeted + wrong header → 403 ForbiddenError, interaction NOT consumed + * 4. Untargeted (no targetClientId, no header) → 200 accepted (regression) + */ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; + +// ── Module mocks ──────────────────────────────────────────────────────────── + +mock.module("../config/env.js", () => ({ + isHttpAuthDisabled: () => true, + hasUngatedHttpAuthDisabled: () => false, +})); + +import type { PendingInteraction } from "../runtime/pending-interactions.js"; + +const pendingStore = new Map(); +const resolvedIds: string[] = []; + +mock.module("../runtime/pending-interactions.js", () => ({ + get: (requestId: string) => pendingStore.get(requestId), + resolve: (requestId: string) => { + const entry = pendingStore.get(requestId); + if (entry) { + pendingStore.delete(requestId); + resolvedIds.push(requestId); + } + return entry; + }, +})); + +interface FileResolveCall { + requestId: string; + result: { content: string; isError: boolean; imageData?: string }; +} + +const resolveSpy: FileResolveCall[] = []; + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + resolve( + requestId: string, + result: { content: string; isError: boolean; imageData?: string }, + ) { + resolveSpy.push({ requestId, result }); + }, + }; + }, + }, +})); + +// ── Real imports (after mocks) ────────────────────────────────────────────── + +import { + BadRequestError, + ForbiddenError, +} from "../runtime/routes/errors.js"; +import { ROUTES } from "../runtime/routes/host-file-routes.js"; + +afterAll(() => { + mock.restore(); +}); + +const handleHostFileResult = ROUTES.find( + (r) => r.endpoint === "host-file-result", +)!.handler; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +function registerPending( + requestId: string, + overrides: Partial = {}, +): void { + pendingStore.set(requestId, { + conversationId: "conv-1", + kind: "host_file", + ...overrides, + }); +} + +function fileBody(requestId: string): Record { + return { + requestId, + content: "file content", + isError: false, + }; +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("handleHostFileResult — Phase 2 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + resolvedIds.length = 0; + resolveSpy.length = 0; + }); + + // ── 1. Targeted + correct header → 200 ──────────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and resolves the interaction", async () => { + const requestId = "req-file-targeted-match"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolveSpy[0].requestId).toBe(requestId); + expect(resolvedIds).toContain(requestId); + }); + + test("trims whitespace from header before comparing", async () => { + const requestId = "req-file-targeted-trim"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": " client-A " }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── 2. Targeted + missing header → 400 ──────────────────────────────────── + + describe("targeted + missing x-vellum-client-id header", () => { + test("throws BadRequestError (400) when header is absent", () => { + const requestId = "req-file-targeted-no-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ body: fileBody(requestId) }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError (400) when header is empty string", () => { + const requestId = "req-file-targeted-empty-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT resolved on 400 (still pending)", () => { + const requestId = "req-file-targeted-no-header-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostFileResult({ body: fileBody(requestId) }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 3. Targeted + wrong header → 403 ────────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError (403) when client ID does not match", () => { + const requestId = "req-file-targeted-mismatch"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both submitting and expected client", () => { + const requestId = "req-file-targeted-mismatch-msg"; + registerPending(requestId, { targetClientId: "client-A" }); + + let caught: unknown; + try { + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-B"); + expect(msg).toContain("client-A"); + }); + + test("interaction is NOT consumed on 403 (pendingInteractions.get still returns it)", () => { + const requestId = "req-file-targeted-mismatch-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when no header is provided", async () => { + const requestId = "req-file-untargeted-no-header"; + registerPending(requestId); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + + test("accepts when header is present (header ignored for untargeted)", async () => { + const requestId = "req-file-untargeted-with-header"; + registerPending(requestId); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-whatever" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + }); + }); +}); diff --git a/assistant/src/__tests__/host-file-write-tool.test.ts b/assistant/src/__tests__/host-file-write-tool.test.ts index 5de7233e2c8..25879f0ffef 100644 --- a/assistant/src/__tests__/host-file-write-tool.test.ts +++ b/assistant/src/__tests__/host-file-write-tool.test.ts @@ -1,7 +1,29 @@ import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; + +import type { HostFileInput } from "../daemon/host-file-proxy.js"; +import type { ToolExecutionResult } from "../tools/types.js"; + +// Mock HostFileProxy singleton so proxy delegation tests can control it. +let mockFileProxyAvailable = false; +let mockFileProxyRequestFn: ( + input: HostFileInput, + conversationId: string, + signal?: AbortSignal, +) => Promise = () => Promise.resolve({ content: "", isError: false }); + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + isAvailable: () => mockFileProxyAvailable, + request: mockFileProxyRequestFn, + }; + }, + }, +})); import { hostFileWriteTool } from "../tools/host-filesystem/write.js"; import type { ToolContext } from "../tools/types.js"; @@ -20,6 +42,8 @@ afterEach(() => { for (const dir of testDirs.splice(0)) { rmSync(dir, { recursive: true, force: true }); } + mockFileProxyAvailable = false; + mockFileProxyRequestFn = () => Promise.resolve({ content: "", isError: false }); }); describe("host_file_write tool", () => { @@ -168,4 +192,21 @@ describe("host_file_write tool", () => { expect(existsSync(filePath)).toBe(true); expect(readFileSync(filePath, "utf-8")).toBe("deep"); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied write", isError: false }; + }; + + await hostFileWriteTool.execute( + { path: "/host/output.txt", content: "hello", target_client_id: "client-x" }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); }); From 1420930a66523d8bc0d6b68b3bc97c5633ded996 Mon Sep 17 00:00:00 2001 From: "credence-the-bot[bot]" Date: Sun, 3 May 2026 14:24:49 +0000 Subject: [PATCH 06/10] fix(host-proxy): update TOOLS.json, test mock, and openapi spec for Phase 2 - Add target_client_id to 8 CU action tools in bundled TOOLS.json manifest - Add listClientsByCapability/getClientById to host-file-proxy test mock - Regenerate openapi.yaml: adds 400/403 responses to host-cu-result and host-file-result routes --- assistant/openapi.yaml | 8 ++ .../src/__tests__/host-file-proxy.test.ts | 5 + .../bundled-skills/computer-use/TOOLS.json | 93 ++++++++++++++++--- 3 files changed, 94 insertions(+), 12 deletions(-) diff --git a/assistant/openapi.yaml b/assistant/openapi.yaml index 1d9377271dd..b82d8452feb 100644 --- a/assistant/openapi.yaml +++ b/assistant/openapi.yaml @@ -6205,6 +6205,10 @@ paths: required: - accepted additionalProperties: false + "400": + description: x-vellum-client-id header is missing for a targeted host CU request. + "403": + description: Submitting client does not match the targeted client for this request. requestBody: required: true content: @@ -6263,6 +6267,10 @@ paths: required: - accepted additionalProperties: false + "400": + description: x-vellum-client-id header is missing for a targeted host file request. + "403": + description: Submitting client does not match the targeted client for this request. requestBody: required: true content: diff --git a/assistant/src/__tests__/host-file-proxy.test.ts b/assistant/src/__tests__/host-file-proxy.test.ts index 1eb19b3176c..fdd9b0cda7f 100644 --- a/assistant/src/__tests__/host-file-proxy.test.ts +++ b/assistant/src/__tests__/host-file-proxy.test.ts @@ -8,6 +8,11 @@ mock.module("../runtime/assistant-event-hub.js", () => ({ assistantEventHub: { getMostRecentClientByCapability: (cap: string) => cap === "host_file" && mockHasClient ? { id: "mock-client" } : null, + listClientsByCapability: (cap: string) => + cap === "host_file" && mockHasClient + ? [{ clientId: "mock-client", capabilities: ["host_file"] }] + : [], + getClientById: (_id: string) => undefined, }, })); diff --git a/assistant/src/config/bundled-skills/computer-use/TOOLS.json b/assistant/src/config/bundled-skills/computer-use/TOOLS.json index c7580b2f324..532acb997e2 100644 --- a/assistant/src/config/bundled-skills/computer-use/TOOLS.json +++ b/assistant/src/config/bundled-skills/computer-use/TOOLS.json @@ -24,7 +24,11 @@ "properties": { "click_type": { "type": "string", - "enum": ["single", "double", "right"], + "enum": [ + "single", + "double", + "right" + ], "description": "Type of click to perform (default: \"single\")" }, "element_id": { @@ -42,9 +46,15 @@ "reasoning": { "type": "string", "description": "Explanation of what you see and why you are clicking here" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["reasoning"] + "required": [ + "reasoning" + ] }, "executor": "tools/computer-use-click.ts", "execution_target": "host" @@ -64,9 +74,16 @@ "reasoning": { "type": "string", "description": "Explanation of what you are typing and why" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["text", "reasoning"] + "required": [ + "text", + "reasoning" + ] }, "executor": "tools/computer-use-type-text.ts", "execution_target": "host" @@ -86,9 +103,16 @@ "reasoning": { "type": "string", "description": "Explanation of why you are pressing this key" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["key", "reasoning"] + "required": [ + "key", + "reasoning" + ] }, "executor": "tools/computer-use-key.ts", "execution_target": "host" @@ -115,7 +139,12 @@ }, "direction": { "type": "string", - "enum": ["up", "down", "left", "right"], + "enum": [ + "up", + "down", + "left", + "right" + ], "description": "Scroll direction" }, "amount": { @@ -125,9 +154,17 @@ "reasoning": { "type": "string", "description": "Explanation of why you are scrolling" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["direction", "amount", "reasoning"] + "required": [ + "direction", + "amount", + "reasoning" + ] }, "executor": "tools/computer-use-scroll.ts", "execution_target": "host" @@ -167,9 +204,15 @@ "reasoning": { "type": "string", "description": "Explanation of what you are dragging and why" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["reasoning"] + "required": [ + "reasoning" + ] }, "executor": "tools/computer-use-drag.ts", "execution_target": "host" @@ -189,9 +232,16 @@ "reasoning": { "type": "string", "description": "Explanation of what you are waiting for" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["duration_ms", "reasoning"] + "required": [ + "duration_ms", + "reasoning" + ] }, "executor": "tools/computer-use-wait.ts", "execution_target": "host" @@ -211,9 +261,16 @@ "reasoning": { "type": "string", "description": "Explanation of why you need to open or switch to this app" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["app_name", "reasoning"] + "required": [ + "app_name", + "reasoning" + ] }, "executor": "tools/computer-use-open-app.ts", "execution_target": "host" @@ -233,9 +290,16 @@ "reasoning": { "type": "string", "description": "Explanation of what this script does and why AppleScript is better than UI interaction for this step" + }, + "target_client_id": { + "type": "string", + "description": "ID of the specific client to target. Required when multiple clients support host_cu; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_cu`." } }, - "required": ["script", "reasoning"] + "required": [ + "script", + "reasoning" + ] }, "executor": "tools/computer-use-run-applescript.ts", "execution_target": "host" @@ -253,7 +317,9 @@ "description": "Human-readable summary of what was accomplished" } }, - "required": ["summary"] + "required": [ + "summary" + ] }, "executor": "tools/computer-use-done.ts", "execution_target": "host" @@ -275,7 +341,10 @@ "description": "Explanation of how you determined the answer" } }, - "required": ["answer", "reasoning"] + "required": [ + "answer", + "reasoning" + ] }, "executor": "tools/computer-use-respond.ts", "execution_target": "host" From 2370969a1254fa5426c948b757b0fbe9cc7fc66f Mon Sep 17 00:00:00 2001 From: Credence Date: Sun, 3 May 2026 14:59:40 +0000 Subject: [PATCH 07/10] fix(host-proxy): correct Phase 2 proxy fixes for file/cu routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `additionalResponses` to host-file-routes and host-cu-routes so the OpenAPI generator emits the 400/403 responses into the spec - Use ConflictError→ForbiddenError (HTTP 403) for wrong-client submissions - Read targetClientId from input object (tool callers embed it there) in addition to the legacy 4th parameter path - Differentiate error messages: unknown client vs incapable client - Include resolvedTargetClientId in request/cancel/dispose message bodies so receiving clients can verify the intended target - Update timeout message to name the targeted clientId - Fix getByKind mock to return {requestId, ...interaction} matching the real pending-interactions implementation - Rename resolveResult→resolve in HostFileProxy (aligns with host-bash pattern) All 9 host-file-proxy-targeted tests now pass (was 1/9). --- .../host-file-proxy-targeted.test.ts | 15 +++++-- assistant/src/daemon/host-file-proxy.ts | 43 +++++++++++++++---- .../src/runtime/routes/host-cu-routes.ts | 14 +++++- .../src/runtime/routes/host-file-routes.ts | 16 +++++-- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/assistant/src/__tests__/host-file-proxy-targeted.test.ts b/assistant/src/__tests__/host-file-proxy-targeted.test.ts index d498a49c944..d78b52de924 100644 --- a/assistant/src/__tests__/host-file-proxy-targeted.test.ts +++ b/assistant/src/__tests__/host-file-proxy-targeted.test.ts @@ -30,13 +30,21 @@ mock.module("../runtime/assistant-event-hub.js", () => ({ }, })); +const pendingInteractionMap = new Map>(); mock.module("../runtime/pending-interactions.js", () => ({ + register: (requestId: string, interaction: Record) => { + pendingInteractionMap.set(requestId, interaction); + }, resolve: (requestId: string) => { + const interaction = pendingInteractionMap.get(requestId); + pendingInteractionMap.delete(requestId); resolvedInteractionIds.push(requestId); - return undefined; + return interaction; }, - get: () => undefined, - getByKind: () => [], + get: (requestId: string) => pendingInteractionMap.get(requestId), + getByKind: (_kind: string) => Array.from(pendingInteractionMap.entries()) + .filter(([, v]) => v.kind === _kind) + .map(([requestId, v]) => ({ requestId, ...v })), getByConversation: () => [], removeByConversation: () => {}, })); @@ -50,6 +58,7 @@ describe("HostFileProxy — targetClientId (Phase 2)", () => { sentMessages.length = 0; sentMessageOptions.length = 0; resolvedInteractionIds.length = 0; + pendingInteractionMap.clear(); mockHasClient = false; mockCapableClients = []; mockClientRegistry = new Map(); diff --git a/assistant/src/daemon/host-file-proxy.ts b/assistant/src/daemon/host-file-proxy.ts index 2e645681fe1..14d73db28f0 100644 --- a/assistant/src/daemon/host-file-proxy.ts +++ b/assistant/src/daemon/host-file-proxy.ts @@ -74,15 +74,23 @@ export class HostFileProxy { } // Resolve targetClientId: explicit → validate; single capable client → auto-resolve. - let resolvedTargetClientId: string | undefined = targetClientId; + // Callers may embed targetClientId in the input object (tool handlers) or pass it as + // the 4th parameter (legacy). Prefer the explicit param; fall back to input field. + let resolvedTargetClientId: string | undefined = targetClientId ?? input.targetClientId; if (resolvedTargetClientId != null) { const client = assistantEventHub.getClientById(resolvedTargetClientId); - if (!client || !client.capabilities.includes("host_file")) { + if (!client) { return Promise.resolve({ content: `No connected client with id '${resolvedTargetClientId}' supports host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, isError: true, }); } + if (!client.capabilities.includes("host_file")) { + return Promise.resolve({ + content: `Client '${resolvedTargetClientId}' does not support host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } } else { const capable = assistantEventHub.listClientsByCapability("host_file"); if (capable.length === 1) { @@ -104,7 +112,9 @@ export class HostFileProxy { "Host file proxy request timed out", ); resolve({ - content: "Host file proxy timed out waiting for client response", + content: resolvedTargetClientId + ? `Host file proxy timed out waiting for response from client '${resolvedTargetClientId}'` + : "Host file proxy timed out waiting for client response", isError: true, }); }, timeoutSec * 1000); @@ -119,6 +129,9 @@ export class HostFileProxy { type: "host_file_cancel", requestId, conversationId, + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), }, conversationId, { targetClientId: resolvedTargetClientId }, @@ -151,6 +164,11 @@ export class HostFileProxy { type: "host_file_request", requestId, conversationId, + // Always include in message body so the receiving client can verify + // which endpoint was targeted (even when auto-resolved). + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), }, conversationId, { targetClientId: resolvedTargetClientId }, @@ -169,7 +187,7 @@ export class HostFileProxy { /** * Process a client result and resolve the RPC. Called by route handlers. */ - resolveResult( + resolve( requestId: string, response: { content: string; isError: boolean; imageData?: string }, ): void { @@ -200,11 +218,18 @@ export class HostFileProxy { for (const entry of pendingInteractions.getByKind("host_file")) { pendingInteractions.resolve(entry.requestId); try { - broadcastMessage({ - type: "host_file_cancel", - requestId: entry.requestId, - conversationId: entry.conversationId, - }); + broadcastMessage( + { + type: "host_file_cancel", + requestId: entry.requestId, + conversationId: entry.conversationId, + ...(entry.targetClientId != null + ? { targetClientId: entry.targetClientId } + : {}), + }, + entry.conversationId, + { targetClientId: entry.targetClientId as string | undefined }, + ); } catch { // Best-effort cancel notification } diff --git a/assistant/src/runtime/routes/host-cu-routes.ts b/assistant/src/runtime/routes/host-cu-routes.ts index 9d9a5b02318..18660c3917d 100644 --- a/assistant/src/runtime/routes/host-cu-routes.ts +++ b/assistant/src/runtime/routes/host-cu-routes.ts @@ -8,7 +8,7 @@ import { z } from "zod"; import { findConversation } from "../../daemon/conversation-store.js"; import * as pendingInteractions from "../pending-interactions.js"; -import { BadRequestError, ConflictError, NotFoundError } from "./errors.js"; +import { BadRequestError, ConflictError, ForbiddenError, NotFoundError } from "./errors.js"; import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; // --------------------------------------------------------------------------- @@ -70,7 +70,7 @@ function handleHostCuResult({ body, headers }: RouteHandlerArgs) { throw new BadRequestError("x-vellum-client-id header is missing for a targeted host CU request."); } if (submittingClientId !== peeked.targetClientId) { - throw new ConflictError("Submitting client does not match the targeted client for this request."); + throw new ForbiddenError("Submitting client does not match the targeted client for this request."); } } @@ -132,6 +132,16 @@ export const ROUTES: RouteDefinition[] = [ responseBody: z.object({ accepted: z.boolean(), }), + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted host CU request.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this request.", + }, + }, handler: handleHostCuResult, }, ]; diff --git a/assistant/src/runtime/routes/host-file-routes.ts b/assistant/src/runtime/routes/host-file-routes.ts index ba342c08380..baa45d58204 100644 --- a/assistant/src/runtime/routes/host-file-routes.ts +++ b/assistant/src/runtime/routes/host-file-routes.ts @@ -8,7 +8,7 @@ import { z } from "zod"; import { HostFileProxy } from "../../daemon/host-file-proxy.js"; import * as pendingInteractions from "../pending-interactions.js"; -import { BadRequestError, ConflictError, NotFoundError } from "./errors.js"; +import { BadRequestError, ConflictError, ForbiddenError, NotFoundError } from "./errors.js"; import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; // --------------------------------------------------------------------------- @@ -49,11 +49,11 @@ function handleHostFileResult({ body, headers }: RouteHandlerArgs) { throw new BadRequestError("x-vellum-client-id header is missing for a targeted host file request."); } if (submittingClientId !== peeked.targetClientId) { - throw new ConflictError("Submitting client does not match the targeted client for this request."); + throw new ForbiddenError("Submitting client does not match the targeted client for this request."); } } - HostFileProxy.instance.resolveResult(requestId, { + HostFileProxy.instance.resolve(requestId, { content: content ?? "", isError: isError ?? false, imageData, @@ -93,6 +93,16 @@ export const ROUTES: RouteDefinition[] = [ responseBody: z.object({ accepted: z.boolean(), }), + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted host file request.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this request.", + }, + }, handler: handleHostFileResult, }, ]; From a7617a63664477e25bee8ae18d0bed4f21acf467 Mon Sep 17 00:00:00 2001 From: Credence Date: Sun, 3 May 2026 15:05:47 +0000 Subject: [PATCH 08/10] fix(host-proxy): trim x-vellum-client-id, descriptive 403 message, fix test mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routes: - Trim x-vellum-client-id header; treat whitespace-only as missing (BadRequestError) Fixes test: 'throws BadRequestError (400) when header is empty string' - ForbiddenError message now names both submitting and expected client IDs e.g. 'Client "B" is not the target for this request (expected "A")' Fixes test: 'ForbiddenError message names both submitting and expected client' Tests: - host-file-routes-targeted: HostFileProxy.resolve mock now simulates pending-interaction consumption (delete from store + push to resolvedIds) Fixes tests: 'returns { accepted: true } and resolves the interaction' and 'untargeted request accepts when no header is provided' - host-cu-routes-targeted: hostCuProxy.resolve → processObservation; processObservation mock simulates pending-interaction cleanup; conversationStore type updated accordingly Fixes all 6 remaining host-cu route test failures - Both route test files: expand env mock to prevent Bun module-bleed when test files are run together locally - host-file-proxy.test.ts: rename resolveResult → resolve (method was renamed in the previous commit) --- .../src/__tests__/host-file-proxy.test.ts | 20 +++++++++---------- .../host-file-routes-targeted.test.ts | 6 ++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/assistant/src/__tests__/host-file-proxy.test.ts b/assistant/src/__tests__/host-file-proxy.test.ts index fdd9b0cda7f..cc0e3db132e 100644 --- a/assistant/src/__tests__/host-file-proxy.test.ts +++ b/assistant/src/__tests__/host-file-proxy.test.ts @@ -67,7 +67,7 @@ describe("HostFileProxy", () => { expect(pendingInteractions.get(requestId)).toBeDefined(); // Simulate client response - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "file contents here", isError: false, }); @@ -92,7 +92,7 @@ describe("HostFileProxy", () => { const sent = sentMessages[0] as Record; const requestId = sent.requestId as string; - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "ENOENT: no such file or directory", isError: true, }); @@ -116,7 +116,7 @@ describe("HostFileProxy", () => { const sent = sentMessages[0] as Record; const requestId = sent.requestId as string; - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "Image loaded on host", isError: false, imageData: PNG_HEADER.toString("base64"), @@ -152,7 +152,7 @@ describe("HostFileProxy", () => { expect(sent.content).toBe("new content"); const requestId = sent.requestId as string; - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "File written successfully", isError: false, }); @@ -180,7 +180,7 @@ describe("HostFileProxy", () => { expect(sent.new_string).toBe("bar"); const requestId = sent.requestId as string; - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "Edit applied successfully", isError: false, }); @@ -207,7 +207,7 @@ describe("HostFileProxy", () => { expect(pendingInteractions.get(requestId)).toBeDefined(); // Resolve to avoid test hanging (actual 30s timeout too long for test) - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "", isError: false, }); @@ -380,7 +380,7 @@ describe("HostFileProxy", () => { expect(result.content).toBe("Aborted"); // Late resolve should be silently ignored (no throw, no double-resolve) - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "late response", isError: false, }); @@ -393,7 +393,7 @@ describe("HostFileProxy", () => { test("silently ignores unknown requestId", () => { setup(); // Should not throw - proxy.resolveResult("unknown-id", { + proxy.resolve("unknown-id", { content: "", isError: false, }); @@ -453,7 +453,7 @@ describe("HostFileProxy", () => { const requestId = (sentMessages[0] as Record) .requestId as string; - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "file contents", isError: false, }); @@ -575,7 +575,7 @@ describe("HostFileProxy", () => { const requestId = sent.requestId as string; expect(pendingInteractions.get(requestId)).toBeDefined(); - proxy.resolveResult(requestId, { + proxy.resolve(requestId, { content: "file contents", isError: false, }); diff --git a/assistant/src/__tests__/host-file-routes-targeted.test.ts b/assistant/src/__tests__/host-file-routes-targeted.test.ts index e1252ec3718..c65940535af 100644 --- a/assistant/src/__tests__/host-file-routes-targeted.test.ts +++ b/assistant/src/__tests__/host-file-routes-targeted.test.ts @@ -48,6 +48,12 @@ mock.module("../daemon/host-file-proxy.js", () => ({ requestId: string, result: { content: string; isError: boolean; imageData?: string }, ) { + // Simulate the real resolve: consume the pending interaction + const entry = pendingStore.get(requestId); + if (entry) { + pendingStore.delete(requestId); + resolvedIds.push(requestId); + } resolveSpy.push({ requestId, result }); }, }; From 4e7fcbc7e3202811fefe1ddab4ad385048040156 Mon Sep 17 00:00:00 2001 From: Credence Date: Sun, 3 May 2026 15:12:57 +0000 Subject: [PATCH 09/10] fix(host-proxy): trim header + descriptive 403 msg + fix host-cu-routes test mock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routes (host-cu-routes.ts, host-file-routes.ts): - Trim x-vellum-client-id before comparison; whitespace-only → BadRequestError - ForbiddenError message names both client IDs: 'Client "B" is not the target for this request (expected "A")' Tests (host-cu-routes-targeted.test.ts): - conversationStore type: resolve → processObservation - registerConversation: hostCuProxy.resolve → processObservation with pending-interaction cleanup (matches real processObservation behaviour) - Expand env mock to prevent Bun module-bleed when files run together --- .../__tests__/host-cu-routes-targeted.test.ts | 28 +++++++++++++++++-- .../src/runtime/routes/host-cu-routes.ts | 7 +++-- .../src/runtime/routes/host-file-routes.ts | 7 +++-- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/assistant/src/__tests__/host-cu-routes-targeted.test.ts b/assistant/src/__tests__/host-cu-routes-targeted.test.ts index d15f7bb72e6..6e86e5d366e 100644 --- a/assistant/src/__tests__/host-cu-routes-targeted.test.ts +++ b/assistant/src/__tests__/host-cu-routes-targeted.test.ts @@ -23,6 +23,27 @@ import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; mock.module("../config/env.js", () => ({ isHttpAuthDisabled: () => true, hasUngatedHttpAuthDisabled: () => false, + getPlatformBaseUrl: () => "https://platform.example.com", + getGatewayInternalBaseUrl: () => "http://localhost:8080", + getIngressPublicBaseUrl: () => undefined, + setIngressPublicBaseUrl: () => {}, + getRuntimeHttpPort: () => 3000, + getRuntimeHttpHost: () => "0.0.0.0", + getSentryDsn: () => "", + getQdrantUrlEnv: () => undefined, + getQdrantHttpPortEnv: () => undefined, + getQdrantReadyzTimeoutMs: () => undefined, + getOllamaBaseUrlEnv: () => undefined, + setPlatformBaseUrl: () => {}, + getAssistantDomain: () => "example.com", + setPlatformAssistantId: () => {}, + getPlatformAssistantId: () => "test-assistant-id", + setPlatformOrganizationId: () => {}, + getPlatformOrganizationId: () => "test-org-id", + setPlatformUserId: () => {}, + getPlatformUserId: () => "test-user-id", + getPlatformInternalApiKey: () => "test-api-key", + validateEnv: () => {}, })); import type { PendingInteraction } from "../runtime/pending-interactions.js"; @@ -50,7 +71,7 @@ interface CuResolveCall { const cuResolveSpy: CuResolveCall[] = []; // Controlled conversation map: conversationId → conversation object -const conversationStore = new Map void } }>(); +const conversationStore = new Map void } }>(); mock.module("../daemon/conversation-store.js", () => ({ findConversation: (conversationId: string) => conversationStore.get(conversationId), @@ -91,7 +112,10 @@ function registerPending( function registerConversation(conversationId = "conv-cu-1"): void { conversationStore.set(conversationId, { hostCuProxy: { - resolve(requestId: unknown, payload: unknown) { + processObservation(requestId: unknown, payload: unknown) { + // Simulate what the real processObservation does: consume the pending interaction + pendingStore.delete(requestId as string); + resolvedIds.push(requestId as string); cuResolveSpy.push({ requestId: requestId as string, payload: payload as Record, diff --git a/assistant/src/runtime/routes/host-cu-routes.ts b/assistant/src/runtime/routes/host-cu-routes.ts index 18660c3917d..b7db2b45dda 100644 --- a/assistant/src/runtime/routes/host-cu-routes.ts +++ b/assistant/src/runtime/routes/host-cu-routes.ts @@ -65,12 +65,15 @@ function handleHostCuResult({ body, headers }: RouteHandlerArgs) { // Validate submitting client matches the targeted client (if any). if (peeked.targetClientId != null) { - const submittingClientId = (headers as Record)?.["x-vellum-client-id"]; + const rawClientId = (headers as Record)?.["x-vellum-client-id"]; + const submittingClientId = rawClientId?.trim() || undefined; if (!submittingClientId) { throw new BadRequestError("x-vellum-client-id header is missing for a targeted host CU request."); } if (submittingClientId !== peeked.targetClientId) { - throw new ForbiddenError("Submitting client does not match the targeted client for this request."); + throw new ForbiddenError( + `Client "${submittingClientId}" is not the target for this request (expected "${peeked.targetClientId}"). The targeted client must submit the result.`, + ); } } diff --git a/assistant/src/runtime/routes/host-file-routes.ts b/assistant/src/runtime/routes/host-file-routes.ts index baa45d58204..cc8238375b6 100644 --- a/assistant/src/runtime/routes/host-file-routes.ts +++ b/assistant/src/runtime/routes/host-file-routes.ts @@ -44,12 +44,15 @@ function handleHostFileResult({ body, headers }: RouteHandlerArgs) { // Validate submitting client matches the targeted client (if any). if (peeked.targetClientId != null) { - const submittingClientId = (headers as Record)?.["x-vellum-client-id"]; + const rawClientId = (headers as Record)?.["x-vellum-client-id"]; + const submittingClientId = rawClientId?.trim() || undefined; if (!submittingClientId) { throw new BadRequestError("x-vellum-client-id header is missing for a targeted host file request."); } if (submittingClientId !== peeked.targetClientId) { - throw new ForbiddenError("Submitting client does not match the targeted client for this request."); + throw new ForbiddenError( + `Client "${submittingClientId}" is not the target for this request (expected "${peeked.targetClientId}"). The targeted client must submit the result.`, + ); } } From 7483f2f4d186584b679c529d81c5a6d349ce26ae Mon Sep 17 00:00:00 2001 From: credence-the-bot Date: Sun, 3 May 2026 15:21:51 +0000 Subject: [PATCH 10/10] fix(host-cu-proxy): include targetClientId in request body and cancel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes mirroring the pattern established in host-file-proxy.ts: 1. host_cu_request body: add `...(targetClientId != null ? { targetClientId } : {})` so the Swift HostCuRequest.targetClientId field is non-nil and EventStreamClient.shouldIgnoreHostToolRequest bypasses the conversation-ownership filter for cross-client targeted requests. 2. abort-cancel (onAbort): pass targetClientId in host_cu_cancel body and as broadcastMessage routing options so the cancel reaches the correct targeted client. 3. dispose-cancel: same fix using entry.targetClientId from the pending interaction record. Resolves Devin 🔴 BUG_0002 and BUG_0003 on commit 4e7fcbc7. --- assistant/src/daemon/host-cu-proxy.ts | 34 ++++++++++++++++++++------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/assistant/src/daemon/host-cu-proxy.ts b/assistant/src/daemon/host-cu-proxy.ts index e43207d306f..18725c6e276 100644 --- a/assistant/src/daemon/host-cu-proxy.ts +++ b/assistant/src/daemon/host-cu-proxy.ts @@ -165,11 +165,18 @@ export class HostCuProxy { this._ownedRequests.delete(requestId); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_cu_cancel", - requestId, + broadcastMessage( + { + type: "host_cu_cancel", + requestId, + conversationId, + ...(targetClientId != null + ? { targetClientId } + : {}), + }, conversationId, - }); + { targetClientId }, + ); } catch { // Best-effort cancel notification } @@ -202,6 +209,8 @@ export class HostCuProxy { input, stepNumber, reasoning, + // Include in body so receiving client can verify targeted endpoint. + ...(targetClientId != null ? { targetClientId } : {}), }, conversationId, { targetClientId }, @@ -395,11 +404,18 @@ export class HostCuProxy { const entry = pendingInteractions.resolve(requestId); if (!entry) continue; try { - broadcastMessage({ - type: "host_cu_cancel", - requestId, - conversationId: entry.conversationId, - }); + broadcastMessage( + { + type: "host_cu_cancel", + requestId, + conversationId: entry.conversationId, + ...(entry.targetClientId != null + ? { targetClientId: entry.targetClientId } + : {}), + }, + entry.conversationId, + { targetClientId: entry.targetClientId as string | undefined }, + ); } catch { // Best-effort cancel notification }