Skip to content

test(host-proxy): unit + route tests for Phase 2 targetClientId#29402

Merged
credence-the-bot[bot] merged 1 commit into
Credence/targeted-host-proxy-phase2from
Credence/targeted-host-proxy-phase2-pr5
May 3, 2026
Merged

test(host-proxy): unit + route tests for Phase 2 targetClientId#29402
credence-the-bot[bot] merged 1 commit into
Credence/targeted-host-proxy-phase2from
Credence/targeted-host-proxy-phase2-pr5

Conversation

@credence-the-bot
Copy link
Copy Markdown
Contributor

@credence-the-bot credence-the-bot Bot commented May 3, 2026

Summary

Adds targeted tests for Phase 2 proxy changes — HostFileProxy target resolution (auto-resolve, explicit validate, unknown/incapable client rejection), host-file-result and host-cu-result route 403 guard (correct/missing/wrong x-vellum-client-id header), and regression tests confirming untargeted flows are unaffected. Also adds target_client_id passthrough assertions to existing host_file tool tests.

Test plan

  • host-file-proxy-targeted.test.ts: HostFileProxy target resolution (9 cases)
    • explicit valid targetClientId → resolves to that client, broadcastMessage receives { targetClientId } option
    • explicit unknown client → returns error, no broadcast
    • explicit incapable client → returns error, no broadcast
    • auto-resolve single capable client → broadcasts with targetClientId
    • multiple clients, no explicit target → broadcasts without targetClientId (untargeted)
    • abort signal cancel includes targetClientId
    • dispose cancel includes targetClientId
    • timeout message includes clientId when resolved
  • host-file-routes-targeted.test.ts: host-file-result 403 guard (10 cases)
    • targeted + correct header → 200 accepted
    • targeted + whitespace-trimmed header → 200 accepted
    • targeted + missing header → 400
    • targeted + empty header → 400
    • targeted + missing header → interaction NOT consumed (400)
    • targeted + wrong header → 403
    • targeted + wrong header → error message names both clients
    • targeted + wrong header → interaction NOT consumed (403)
    • untargeted + no header → 200 accepted (regression)
    • untargeted + header present → 200 accepted (header ignored)
  • host-cu-routes-targeted.test.ts: host-cu-result 403 guard (10 cases, same structure)
  • Tool passthrough tests in host-file-read/write/edit tool files (1 each)

🤖 Generated with Claude Code


Open in Devin Review

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: Claude Sonnet 4.6 <noreply@anthropic.com>
@credence-the-bot credence-the-bot Bot merged commit 56e76b8 into Credence/targeted-host-proxy-phase2 May 3, 2026
10 of 12 checks passed
@credence-the-bot credence-the-bot Bot deleted the Credence/targeted-host-proxy-phase2-pr5 branch May 3, 2026 13:58
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

credence-the-bot Bot added a commit that referenced this pull request May 3, 2026
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>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants