feat(macos): targetClientId acceptance guard and result header for host_file and host_cu#29396
Merged
credence-the-bot[bot] merged 1 commit intoMay 3, 2026
Conversation
…st_file and host_cu Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🚩 hostAppControlRequest and hostBrowserRequest lack targetClientId support — intentional scope boundary
This PR adds targetClientId to HostFileRequest and HostCuRequest but does not add it to HostAppControlRequest (MessageTypes.swift:1687-1719) or HostBrowserRequest. Their shouldIgnoreHostToolRequest cases (EventStreamClient.swift:628-635) still lack the targetClientId != nil bypass, and postAppControlResult (HostProxyClient.swift:96-99) still omits the X-Vellum-Client-Id header. This appears intentional (app-control and browser don't have cross-client proxy routing yet), but worth confirming these request types don't need cross-client targeting in this iteration.
(Refers to lines 628-635)
Was this helpful? React with 👍 or 👎 to provide feedback.
credence-the-bot
Bot
merged commit 290a78a
into
Credence/targeted-host-proxy-phase2
7 checks passed
![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: befd70cf1f
ℹ️ 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".
Comment on lines
+384
to
+388
| 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 { |
There was a problem hiding this comment.
isUntargetedLocal now depends on mainWindow?.conversationManager.conversations, which can be empty/nil before the main window is created or before conversations hydrate. In that state, untargeted host_file_request events that already passed EventStreamClient's locallyOwnedConversationIds check are incorrectly dropped, so legitimate local host-file operations never run. This is a regression from the previous behavior (execute after transport-level ownership filtering) and the same pattern is duplicated in the host_cu_request branch.
Useful? React with 👍 / 👎.
4 tasks
credence-the-bot Bot
added a commit
that referenced
this pull request
May 3, 2026
…st_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>
noanflaherty
pushed a commit
that referenced
this pull request
May 3, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MessageTypes.swift: addstargetClientId: String?toHostFileRequestandHostCuRequestEventStreamClient.swift: updatesshouldIgnoreHostToolRequestto pass through targetedhost_file_requestandhost_cu_request(same ashost_bash_requestin Phase 1)AppDelegate+ConnectionSetup.swift: addsisTargeted || isUntargetedLocalguard to.hostFileRequestand.hostCuRequesthandlersHostProxyClient.swift: addsX-Vellum-Client-Idheader topostFileResultandpostCuResult(required by Phase 2 route validation PR)Depends on PR #29394 (already merged into feature branch).