feat(tools): add target_client_id to host_file_* and computer_use_* tools#29397
Conversation
…ools Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
credence-the-bot
Bot
merged commit 0eb6e76
into
Credence/targeted-host-proxy-phase2
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e7b58fd91
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| targetClientId == null && | ||
| transportInterface != null && | ||
| !supportsHostProxy(transportInterface) && | ||
| assistantEventHub.listClientsByCapability("host_file").length > 1 | ||
| ) { |
There was a problem hiding this comment.
The new target_client_id path only guards the targetClientId == null case, so if a caller specifies a target that disconnects before execution, this code skips the ambiguity check and later falls through to local filesystem execution when HostFileProxy.instance.isAvailable() is false. That can run host_file_read/write/edit against the daemon/container filesystem instead of the intended host client, which is a correctness and safety regression introduced by adding explicit targeting.
Useful? React with 👍 / 👎.
| ctx.hostCuProxy.stepCount, | ||
| reasoning, | ||
| signal, | ||
| targetClientId, |
There was a problem hiding this comment.
Passing target_client_id straight into hostCuProxy.request without checking that the client is still connected and has host_cu capability causes a bad/stale ID to hang until proxy timeout instead of failing fast. This is newly introduced by threading target_client_id through CU actions; a typo or stale ID now adds repeated 60s delays per step in multi-client sessions.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Missing target_client_id on computer_use_observe — incomplete transformation across proxied CU tools
The PR adds target_client_id to every proxied computer-use tool definition in definitions.ts (click, type_text, key, scroll, drag, wait, open_app, run_applescript) but omits it from computer_use_observe. The observe tool IS proxied to the client — it falls through to the non-terminal branch in surfaceProxyResolver at assistant/src/daemon/conversation-surfaces.ts:1787-1796, where targetClientId is extracted from input.target_client_id and forwarded to ctx.hostCuProxy.request(). Without the property in the observe tool's schema (properties: {} at line 498), the LLM has no way to provide a target_client_id for observe calls, so in multi-client setups the observation request cannot be targeted to a specific client. Terminal tools (computer_use_done, computer_use_respond) correctly omit it since they resolve locally without a client round-trip.
(Refers to lines 496-499)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🚩 Pre-existing: buildUserFacingLabel reads cancelLabel for deny action
At conversation-surfaces.ts:1724, the deny branch reads surfaceData?.cancelLabel instead of surfaceData?.denyLabel. This means the deny action's user-facing label falls back to the cancel button's label rather than a dedicated deny label. This is a pre-existing issue not introduced by this PR, but it's in adjacent unchanged code within the same function.
(Refers to lines 1724-1726)
Was this helpful? React with 👍 or 👎 to provide feedback.
…ools (#29397) Co-authored-by: credence-the-bot <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…29398) * 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 <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(routes): enforce targetClientId binding on host-file-result and host-cu-result (#29395) Co-authored-by: credence-the-bot <credence-the-bot@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(tools): add target_client_id to host_file_* and computer_use_* tools (#29397) Co-authored-by: credence-the-bot <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(macos): targetClientId acceptance guard and result header for host_file and host_cu (#29396) Co-authored-by: credence-the-bot <credence-the-bot@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * 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 <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * 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 * fix(host-proxy): correct Phase 2 proxy fixes for file/cu routes - 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). * fix(host-proxy): trim x-vellum-client-id, descriptive 403 message, fix test mocks 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) * fix(host-proxy): trim header + descriptive 403 msg + fix host-cu-routes test mock 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 * fix(host-cu-proxy): include targetClientId in request body and cancel messages 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 4e7fcbc. --------- Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: credence-the-bot <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: credence-the-bot <credence-the-bot@users.noreply.github.com> Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
Adds optional
target_client_idfield tohost_file_read,host_file_write,host_file_editschemas and all 8computer_use_*action tools schemas. Threads it from executor through to the proxy request call. Includes multi-client guard (requires explicit target when >1 capable client from non-host-proxy interface). Depends on PR #29394 (core proxy changes, already merged into feature branch).