Skip to content

feat(telemetry): end-to-end harness attribution (UA-style)#160

Merged
willwashburn merged 3 commits into
mainfrom
feat/harness-ua-telemetry
Jun 3, 2026
Merged

feat(telemetry): end-to-end harness attribution (UA-style)#160
willwashburn merged 3 commits into
mainfrom
feat/harness-ua-telemetry

Conversation

@willwashburn

@willwashburn willwashburn commented Jun 3, 2026

Copy link
Copy Markdown
Member

User description

Supersedes the stale #133 / #132 — solves the same spirit (telemetry should record which harness is driving each request) with the design decisions made fresh: a User-Agent-style value, settable by any requestor (public option) and wrapping hosts (internal origin).

Why

The engine already reads X-Relaycast-Harness and stamps harness on every server telemetry event and request log. But nothing ever sent the header, so every event records harness: "unknown". The gap is a client-side sender — plus widening the contract so the value can carry model/settings UA-style.

What

SDK (@relaycast/sdk)

  • Public harness option on RelayCastOptions / ClientOptions and WsClientOptions, plus the internal InternalOrigin plumbing. Any requestor can self-identify ('human', 'codex', 'claude-code/2.3 (model=opus-4.8)'); wrapping hosts (relaycast-mcp) set it via the internal origin, which takes precedence over the public option.
  • Stamped as the X-Relaycast-Harness header on every HTTP request, and forwarded as the harness WS query param (browser WebSocket can't set custom headers).
  • sanitizeHarness() drops empty / malformed values (defence-in-depth against CRLF smuggling) and survives withApiKey() rotations. Header omitted entirely when unset — existing consumers unchanged on the wire.

Engine (@relaycast/engine)

  • extractHarness now accepts a UA-style token (cap 40 → 120; charset widened to a UA-safe set that still rejects CR/LF and control chars) and reads the header OR the harness query param — fixing the WebSocket path, which previously always resolved 'unknown' because the logger middleware only read the header.

Wire contract

value
HTTP header X-Relaycast-Harness: claude-code/2.3 (model=opus-4.8; fast)
WS query param ?harness=claude-code/2.3
Normalisation trimmed, lowercased, [a-z0-9 ._-/():=;,+], ≤120 chars; invalid → dropped (HTTP) / unknown (server)

Tests

  • SDK — 13 tests: sanitizeHarness unit, HTTP header presence/absence/sanitisation/truncation/CRLF-rejection, withApiKey() survival, origin-over-option precedence, WS query-param presence/absence.
  • Engine — 9 tests: header read, UA token, query-param fallback, header-over-query precedence, CRLF/disallowed-char rejection, truncation.
  • Full monorepo turbo build test lint green (25 tasks).

Scope notes

  • No openapi.yaml / README.md change: the X-Relaycast-Origin-* telemetry headers are intentionally undocumented internal plumbing; X-Relaycast-Harness follows that precedent.
  • Static bootstrap helpers (RelayCast.createWorkspace, RelaycastSetup) still use SDK_ORIGIN (pre-harness context) — left for a follow-up.
  • No @relaycast/types schema change: harness rides as a telemetry property, not part of TelemetryOrigin.

🤖 Generated with Claude Code


CodeAnt-AI Description

Attribute requests to the harness that sent them

What Changed

  • Requests now carry a harness identifier from the SDK to the server, so telemetry can show which tool or host drove each request.
  • WebSocket connections now send the same harness value in the connection URL, fixing cases where browser-based connections were recorded as unknown.
  • Harness values are accepted in a User-Agent-style format, kept lowercase, capped at 120 characters, and dropped if they contain empty or unsafe characters.
  • Wrapping hosts can set a harness once and keep it through API key changes; their value takes precedence over a public harness setting.

Impact

✅ Clearer telemetry attribution
✅ Fewer unknown harness events
✅ Safer harness reporting

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…e server contract)

The engine already reads X-Relaycast-Harness and stamps `harness` on every
server telemetry event and request log — but nothing sent the header, so every
event recorded `harness: "unknown"`. This wires up the missing sender and widens
the contract to carry a User-Agent-style identifier.

SDK (@relaycast/sdk):
- Public `harness` option on RelayCastOptions/ClientOptions and WsClientOptions,
  plus the internal InternalOrigin plumbing. Any requestor can self-identify
  ('human', 'codex', 'claude-code/2.3 (model=opus-4.8)'); wrapping hosts set it
  via the internal origin, which takes precedence over the public option.
- Stamped as the X-Relaycast-Harness header on every HTTP request and forwarded
  as the `harness` WS query param (browsers can't set custom WS headers).
- sanitizeHarness() drops empty/malformed values so a buggy caller can't smuggle
  a CRLF injection; the header is omitted entirely when unset — existing
  consumers are unchanged on the wire.

Engine (@relaycast/engine):
- extractHarness now accepts a UA-style token (cap 40 -> 120; widened to a
  UA-safe charset) and reads the header OR the `harness` query param, so the
  WebSocket path actually resolves a harness instead of always 'unknown'.

Tests: 13 SDK + 9 engine covering header/query presence, absence, sanitisation,
truncation, CRLF rejection, withApiKey() survival, and origin-over-option
precedence. Full monorepo build/test/lint green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@codeant-ai

codeant-ai Bot commented Jun 3, 2026

Copy link
Copy Markdown

CodeAnt AI is reviewing your PR.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@agent-relay-code[bot], we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 27 minutes and 18 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 167d945c-4bbb-4125-ac25-47b8f423e973

📥 Commits

Reviewing files that changed from the base of the PR and between 8be3f1a and 340d8d9.

📒 Files selected for processing (4)
  • .agentworkforce/trajectories/completed/2026-06/traj_bcy2wuz2urpu/summary.md
  • .agentworkforce/trajectories/completed/2026-06/traj_bcy2wuz2urpu/trajectory.json
  • README.md
  • openapi.yaml
📝 Walkthrough

Walkthrough

This PR introduces a harness identifier feature enabling request identification across HTTP and WebSocket transports. The engine extracts harness values from headers or query parameters, validates them, and integrates with logging/telemetry. The SDK normalizes harness input, exposes validation utilities, and propagates harness through HTTP and WebSocket clients via headers and query parameters respectively.

Changes

Harness Identifier Feature

Layer / File(s) Summary
Engine harness extraction & validation
packages/engine/src/lib/origin.ts, packages/engine/src/lib/__tests__/origin.test.ts
extractHarness now reads from X-Relaycast-Harness header with fallback to harness query parameter, validates against a permissive allowlist regex, and returns normalized (lowercased, ≤120 chars) identifiers or UNKNOWN_HARNESS when invalid. Tests cover header precedence, query fallback, case normalization, length truncation, and CRLF/control-char injection rejection.
Engine harness integration in middleware & telemetry
packages/engine/src/middleware/logger.ts, packages/engine/src/lib/serverTelemetry.ts
Logger middleware and server telemetry now extract harness from c.req.raw instead of cached headers, enabling dynamic harness detection for all request types including those bypassing the middleware.
SDK harness infrastructure & public exports
packages/sdk-typescript/src/origin.ts, packages/sdk-typescript/src/index.ts
InternalOrigin gains optional harness field; HARNESS_HEADER constant defines the HTTP wire contract (X-Relaycast-Harness); sanitizeHarness function normalizes input (trim, validate chars, lowercase, cap at 120 chars, return undefined if empty/invalid); both utilities exported at SDK root.
HTTP client harness support
packages/sdk-typescript/src/client.ts, packages/sdk-typescript/src/relay.ts
ClientOptions accepts optional harness string; HttpClient sanitizes and stores as _originHarness (preferring internal origin over public option), exposes via public getter, propagates as X-Relaycast-Harness header on every request when defined, and preserves harness through withApiKey() rotations.
WebSocket client harness support
packages/sdk-typescript/src/ws.ts, packages/sdk-typescript/src/agent.ts
WsClientOptions accepts optional harness string; WsClient sanitizes and forwards as harness query parameter in WebSocket connection URL when present; AgentClient.connect() passes HTTP client's originHarness to WS client initialization.
Comprehensive harness test coverage
packages/sdk-typescript/src/__tests__/harness.test.ts
Unit tests verify sanitizeHarness normalization (whitespace trimming, invalid-character rejection, lowercasing, 120-char cap); HTTP tests assert X-Relaycast-Harness header sent when valid, omitted when absent/invalid, sourced from public option or internal origin (with internal precedence), and preserved across rotations; WebSocket tests confirm harness query parameter presence/absence in connection URLs.

Documentation & Workflow Records

Layer / File(s) Summary
Changelog documentation
packages/sdk-typescript/CHANGELOG.md
Documents new optional harness field on client/relay options, validation/precedence/propagation behavior, and new root exports sanitizeHarness and HARNESS_HEADER.
Workflow metadata & execution records
.agentworkforce/trajectories/completed/2026-06/traj_1774522548467_9eeddc03/*
Trajectory and summary files record task completion, key decision to expose sanitizeHarness from SDK root, execution timeline, and retrospective noting harness telemetry/SDK export implementation with passing tests.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client Code
  participant HTTP as HttpClient
  participant Sanitize as sanitizeHarness
  participant Network as Network
  participant WS as WsClient

  Client->>HTTP: new(options: {harness?})
  HTTP->>Sanitize: sanitize(origin.harness || options.harness)
  Sanitize-->>HTTP: normalized string | undefined
  HTTP->>HTTP: store as _originHarness
  
  Client->>HTTP: request()
  HTTP->>Network: add X-Relaycast-Harness header
  Network-->>HTTP: response
  
  Client->>WS: new(options: {harness?})
  WS->>Sanitize: sanitize(origin.harness || options.harness)
  Sanitize-->>WS: normalized string | undefined
  WS->>WS: store as originHarness
  
  Client->>WS: connect()
  WS->>Network: WebSocket URL with harness query param
  Network-->>WS: connection
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • AgentWorkforce/relaycast#138: Earlier harness feature work on engine-layer extraction and telemetry integration that shares overlapping extractHarness and emitServerEvent changes in the same files.

Suggested labels

size:XXL, feature:harness, sdk:typescript, engine:core


🐰 A harness to hold the threads,
From header to query it spreads,
HTTP, WebSocket both in stride,
Sanitized and verified with pride!
~CodeRabbit 🎯✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(telemetry): end-to-end harness attribution (UA-style)' accurately and concisely summarizes the main change—implementing harness attribution for telemetry with User-Agent-style identifiers.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the motivation, implementation details across SDK and engine, wire contract, test coverage, and scope boundaries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/harness-ua-telemetry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Jun 3, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/sdk-typescript/src/client.ts">

<violation number="1" location="packages/sdk-typescript/src/client.ts:212">
P2: Per-request headers can override the sanitized harness header, breaking harness precedence and allowing unsanitized harness values to be emitted.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment on lines +212 to 213
...(this._originHarness ? { [HARNESS_HEADER]: this._originHarness } : {}),
...(options?.headers || {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Per-request headers can override the sanitized harness header, breaking harness precedence and allowing unsanitized harness values to be emitted.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-typescript/src/client.ts, line 212:

<comment>Per-request headers can override the sanitized harness header, breaking harness precedence and allowing unsanitized harness values to be emitted.</comment>

<file context>
@@ -192,6 +209,7 @@ export class HttpClient {
       'X-Relaycast-Origin-Surface': this._originSurface,
       'X-Relaycast-Origin-Client': this._originClient,
       'X-Relaycast-Origin-Version': this._originVersion,
+      ...(this._originHarness ? { [HARNESS_HEADER]: this._originHarness } : {}),
       ...(options?.headers || {}),
     };
</file context>
Suggested change
...(this._originHarness ? { [HARNESS_HEADER]: this._originHarness } : {}),
...(options?.headers || {}),
...(options?.headers || {}),
...(this._originHarness ? { [HARNESS_HEADER]: this._originHarness } : {}),

this.originSurface = origin.surface;
this.originClient = origin.client;
this.originVersion = origin.version;
this.originHarness = sanitizeHarness(origin.harness ?? options.harness);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Internal-origin precedence is applied before sanitization, so an invalid internal harness value suppresses a valid public harness option and results in no harness being sent at all. Sanitize the internal and public values separately, then apply precedence on the sanitized results so malformed internal metadata does not erase usable caller-provided attribution. [incorrect condition logic]

Severity Level: Major ⚠️
- ⚠️ WS sessions from internal hosts lose harness attribution.
- ⚠️ Telemetry/logging in `loggerMiddleware` see harness as unknown.
- ⚠️ WS harness behavior diverges from caller expectations with valid option.
- ⚠️ Same precedence pattern in HttpClient risks similar attribution loss.
Steps of Reproduction ✅
1. In an internal host, call `createInternalWsClient()` from
`packages/sdk-typescript/src/internal.ts:13-17` with:

   - `options` of type `WsClientOptions` including a valid public harness, e.g. `{ token:
   'at_live_test', harness: 'claude-code/2.3' }`.

   - `origin` of type `InternalOrigin` including an *invalid* harness, e.g. `{ surface:
   'mcp', client: '@relaycast/mcp', version: '0.1.2', harness: 'evil\r\nX-Inject: bad' }`.

   This uses the same internal entrypoint that `createRelayMcpServer` in
   `packages/mcp/src/server.ts:262-266` already uses, but with `origin.harness` populated.

2. `createInternalWsClient()` wraps the options via `withInternalWsOrigin(options,
origin)` in `packages/sdk-typescript/src/internal.ts:13-17`, which stores `origin` on the
non-enumerable `INTERNAL_WS_ORIGIN` symbol as implemented in `withInternalWsOrigin` at
`packages/sdk-typescript/src/ws.ts:42-53`.

3. Constructing `new WsClient(...)` calls the `WsClient` constructor in
`packages/sdk-typescript/src/ws.ts:80-103`. It reads the internal origin via
`readInternalWsOrigin` and then computes:

   - `this.originHarness = sanitizeHarness(origin.harness ?? options.harness);` at
   `packages/sdk-typescript/src/ws.ts:102`.

   Because `origin.harness` is a *defined* string, the nullish-coalescing expression
   `origin.harness ?? options.harness` selects the invalid internal value and never
   consults `options.harness`. `sanitizeHarness` in
   `packages/sdk-typescript/src/origin.ts:47-52` rejects this value (`HARNESS_ALLOWED`
   fails) and returns `undefined`, so `this.originHarness` becomes `undefined` even though
   the public `options.harness` is valid.

4. When `WsClient.connect()` runs (see `packages/sdk-typescript/src/ws.ts:105-117`), it
only forwards the harness when `this.originHarness` is truthy:

   - It sets various query params then conditionally does
   `wsUrl.searchParams.set('harness', this.originHarness);` inside `if
   (this.originHarness) { ... }` at `packages/sdk-typescript/src/ws.ts:115-117`.

   - Since `this.originHarness` is `undefined`, the `harness` query param is omitted
   entirely.

   On the server, `extractHarness` in `packages/engine/src/lib/origin.ts:38-47` reads
   `X-Relaycast-Harness` or the `harness` query param and returns `UNKNOWN_HARNESS` when
   neither is present. This result is wired into the logger in
   `packages/engine/src/middleware/logger.ts:106-120` and into the WebSocket upgrade path
   in `packages/engine/src/engine.ts:87-109`, so this session is logged and attributed as
   `'unknown'` even though the caller supplied a valid public `WsClientOptions.harness`
   value.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** packages/sdk-typescript/src/ws.ts
**Line:** 102:102
**Comment:**
	*Incorrect Condition Logic: Internal-origin precedence is applied before sanitization, so an invalid internal harness value suppresses a valid public `harness` option and results in no harness being sent at all. Sanitize the internal and public values separately, then apply precedence on the sanitized results so malformed internal metadata does not erase usable caller-provided attribution.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment thread packages/sdk-typescript/CHANGELOG.md Outdated

### Added
- Optional `harness` field on `RelayCastOptions`/`ClientOptions` and `WsClientOptions` (plus the internal `InternalOrigin` plumbing). A User-Agent-style identifier for the harness driving requests (e.g. `'claude-code/2.3 (model=opus-4.8)'`, `'codex'`, `'human'`); stamped as the `X-Relaycast-Harness` HTTP header and forwarded as the `harness` WS query param so server-side telemetry can attribute traffic. When a wrapping host supplies one via the internal origin it takes precedence over the public option. Invalid values (empty, control characters) are dropped rather than sent; the header is omitted entirely when no harness is set, so existing consumers are unchanged on the wire.
- `sanitizeHarness` exported from `./origin.js` — lowercases, restricts to a UA-safe character set, caps at 120 chars.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Architect Review — HIGH

The changelog states that sanitizeHarness is exported for consumers, but the package exports only expose ., ./communicate, and ./internal, and the public index does not re-export sanitizeHarness, so external callers cannot import it as documented.

Suggestion: Align the packaging with the documented API by exporting sanitizeHarness through the public surface (e.g. via src/index.ts and the exports map), or update the changelog/docs to clearly mark it as internal-only.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** packages/sdk-typescript/CHANGELOG.md
**Line:** 12:12
**Comment:**
	*HIGH: The changelog states that `sanitizeHarness` is exported for consumers, but the package exports only expose `.`, `./communicate`, and `./internal`, and the public index does not re-export `sanitizeHarness`, so external callers cannot import it as documented.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

Comment on lines +39 to +40
const raw = request.headers.get(HARNESS_HEADER)
?? new URL(request.url).searchParams.get('harness');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Falling back to harness from the URL query string for every request lets normal HTTP endpoints be re-attributed by simply appending ?harness=..., which breaks the stated wire contract (HTTP should use header, query should be WS-only) and enables telemetry/log poisoning. Restrict query-param fallback to WebSocket upgrade requests (or /v1/ws) and keep HTTP attribution header-based. [security]

Severity Level: Major ⚠️
- ❌ Any HTTP endpoint's harness spoofable via `?harness` query.
- ⚠️ HTTP request logs can be poisoned with fake harness.
- ⚠️ Telemetry misattributes traffic between harnesses and hosts.
Steps of Reproduction ✅
1. Start the Relaycast engine using `createEngine()` in
`packages/engine/src/engine.ts:56-78`, which wires `loggerMiddleware` on all routes via
`app.use('*', loggerMiddleware);`.

2. Observe in `packages/engine/src/middleware/logger.ts:106-113` that `loggerMiddleware`
calls `extractHarness(c.req.raw)` and stores the result in `c.set('harness', harness)` for
every request.

3. In `packages/engine/src/lib/origin.ts:38-40`, `extractHarness(request)` reads `const
raw = request.headers.get(HARNESS_HEADER) ?? new
URL(request.url).searchParams.get('harness');`, meaning that if the `X-Relaycast-Harness`
header is absent, it will unconditionally fall back to the `harness` query parameter for
any request URL.

4. Send an ordinary HTTP request (not a WebSocket upgrade) such as `GET
/v1/workspaces?harness=spoofed-harness` without an `X-Relaycast-Harness` header;
`extractHarness` will resolve `raw` from the query param, so `loggerMiddleware` logs
`harness: 'spoofed-harness'` (see `logger.ts:115-120`) and `emitServerEvent()` in
`packages/engine/src/lib/serverTelemetry.ts:36-48` will also emit telemetry with `harness:
'spoofed-harness'`, allowing HTTP telemetry attribution and logs to be re-labeled via
`?harness=` even though the public contract intends query-based harness only for the
`/v1/ws` WebSocket path (where the browser SDK legitimately sets `harness` in
`packages/sdk-typescript/src/ws.ts:110-117`).

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** packages/engine/src/lib/origin.ts
**Line:** 39:40
**Comment:**
	*Security: Falling back to `harness` from the URL query string for every request lets normal HTTP endpoints be re-attributed by simply appending `?harness=...`, which breaks the stated wire contract (HTTP should use header, query should be WS-only) and enables telemetry/log poisoning. Restrict query-param fallback to WebSocket upgrade requests (or `/v1/ws`) and keep HTTP attribution header-based.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

this._originVersion = origin.version;
// A wrapping host's internal origin is authoritative about the harness;
// fall back to the public `harness` option for plain consumers.
this._originHarness = sanitizeHarness(origin.harness ?? options.harness);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: This merge logic still uses the public harness option when internal-origin metadata exists but does not include a harness value, so wrapping hosts are not truly authoritative and caller-provided harness can leak through unexpectedly. Detect whether internal origin is present separately and ignore public harness whenever internal origin is attached. [logic error]

Severity Level: Major ⚠️
- ⚠️ Wrapping hosts cannot fully control harness attribution.
- ⚠️ Internal-origin harness precedence contract violated in HttpClient.
- ⚠️ Telemetry may mix host and caller harness identities.
Steps of Reproduction ✅
1. A wrapping host uses the internal entrypoint `createInternalRelayCast(options, origin)`
from `packages/sdk-typescript/src/internal.ts:6-10`, passing a `RelayCastOptions` object
that includes `apiKey` and a public `harness` (e.g. `{ apiKey: 'rk_live_test', harness:
'human' }`) and an `InternalOrigin` value with surface/client/version but no `harness`
property (similar to `mcpOrigin` in `packages/mcp/src/transports.ts:10-14` and
`packages/mcp/src/server.ts:149-153`).

2. `createInternalRelayCast` calls `withInternalOrigin(options, origin)`
(`client.ts:53-63`), which attaches the `InternalOrigin` as a non-enumerable
`[INTERNAL_ORIGIN]` symbol on the same `options` object, so `options` now has both
internal-origin metadata and the public `harness` field.

3. `RelayCast`'s constructor (`packages/sdk-typescript/src/relay.ts:135-148`) instantiates
`new HttpClient(options)`, and inside the `HttpClient` constructor (`client.ts:138-147`),
`const origin = readInternalOrigin(options) ?? SDK_ORIGIN;` returns the internal origin
while `options.harness` is still the caller-supplied value.

4. Because `this._originHarness` is set via `sanitizeHarness(origin.harness ??
options.harness)` (`client.ts:147`), and `origin.harness` is `undefined` in this scenario,
the public `options.harness` is used even though internal-origin metadata is present, so
subsequent requests from this wrapped host send `X-Relaycast-Harness` from the caller's
option (`request()` uses `_originHarness` in headers at `client.ts:206-213`), violating
the stated intent in the comment at `client.ts:145-146` that "A wrapping host's internal
origin is authoritative about the harness; fall back to the public `harness` option for
plain consumers."

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** packages/sdk-typescript/src/client.ts
**Line:** 147:147
**Comment:**
	*Logic Error: This merge logic still uses the public `harness` option when internal-origin metadata exists but does not include a harness value, so wrapping hosts are not truly authoritative and caller-provided harness can leak through unexpectedly. Detect whether internal origin is present separately and ignore public harness whenever internal origin is attached.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@codeant-ai

codeant-ai Bot commented Jun 3, 2026

Copy link
Copy Markdown

CodeAnt AI finished reviewing your PR.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 8be3f1a to this PR. The notes below describe what changed.

Reviewed PR #160 and fixed one packaging issue: sanitizeHarness/HARNESS_HEADER were documented as exported but not reachable from the SDK package root. Added the root export, a regression test, and aligned the changelog wording.

Verified locally:

  • npm run build --workspace=@relaycast/sdk
  • npm run build --workspace=@relaycast/engine
  • npm test --workspace=@relaycast/sdk
  • npm test --workspace=@relaycast/engine
  • npm run lint --workspace=@relaycast/sdk
  • npm run lint --workspace=@relaycast/engine

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/sdk-typescript/src/origin.ts (1)

29-53: 🏗️ Heavy lift

Centralize harness validation in a shared zod parser.

The SDK now hand-rolls the same trim/regex/length/lowercase contract that packages/engine/src/lib/origin.ts also hand-rolls. That makes the wire contract easy to drift again. A shared zod-backed parser would keep both sides in lockstep and align with the repo’s validation guidance.

As per coding guidelines "Prefer zod schemas for validation instead of ad-hoc manual checks".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk-typescript/src/origin.ts` around lines 29 - 53, Replace the
ad-hoc validation inside sanitizeHarness (and remove
HARNESS_ALLOWED/HARNESS_MAX_LENGTH usage here) with the shared zod-backed parser
used elsewhere: import the canonical harness zod schema/parser (e.g.,
harnessSchema or parseHarness) and call it to validate/transform the input so it
performs trim, regex, length cap and lowercasing; return undefined when the
parser indicates invalid/empty input and return the parser's transformed string
when valid. Ensure sanitizeHarness delegates entirely to that shared parser so
the wire contract is centralized and both sides stay in sync.
packages/sdk-typescript/src/__tests__/harness.test.ts (1)

147-198: ⚡ Quick win

Add an end-to-end WS attribution test through AgentClient.connect().

The WS coverage here only exercises new WsClient({ harness }), but the new production hop in packages/sdk-typescript/src/agent.ts is HttpClient.originHarness -> withInternalWsOrigin(...) -> WsClient. A regression there would still pass this suite and silently drop harness telemetry for the main SDK path.

Suggested test shape
 describe('harness — WS', () => {
+  it('forwards the HTTP client harness through AgentClient.connect()', async () => {
+    const constructed: string[] = [];
+    class MockWs {
+      static readonly OPEN = 1;
+      onopen: (() => void) | null = null;
+      onclose: (() => void) | null = null;
+      onmessage: ((event: { data: string }) => void) | null = null;
+      onerror: (() => void) | null = null;
+      send = vi.fn();
+      close = vi.fn();
+      constructor(url: string) {
+        constructed.push(url);
+      }
+    }
+    vi.stubGlobal('WebSocket', MockWs);
+
+    const { HttpClient } = await import('../client.js');
+    const { AgentClient } = await import('../agent.js');
+    const agent = new AgentClient(new HttpClient({
+      apiKey: 'rk_live_test',
+      harness: 'cursor',
+    }));
+
+    agent.connect();
+    await agent.disconnect();
+
+    const url = new URL(constructed[0]!);
+    expect(url.searchParams.get('harness')).toBe('cursor');
+  });
+
   it('forwards the harness as a `harness` query param on connect', async () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk-typescript/src/__tests__/harness.test.ts` around lines 147 -
198, Add an end-to-end WS attribution test that exercises the production path
AgentClient.connect -> HttpClient.originHarness -> withInternalWsOrigin ->
WsClient: create a test that stubs global WebSocket (like the existing WsClient
tests do), then instantiate AgentClient (or the exported AgentClient factory)
with a token and harness value and call AgentClient.connect(), assert the
constructed WebSocket URL contains the harness query param; also add a
complementary test where no harness is supplied and assert the harness param is
omitted. Target symbols: AgentClient.connect, HttpClient.originHarness,
withInternalWsOrigin, and WsClient to ensure the real SDK hop is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/sdk-typescript/src/client.ts`:
- Around line 145-147: The current assignment to this._originHarness uses
origin.harness ?? options.harness which falls back to the public harness when an
internal origin object exists but intentionally omits harness; change the
precedence so that the mere presence of an internal origin takes priority even
if its harness is undefined. Update the logic around this._originHarness and
sanitizeHarness to check for the internal origin's explicit harness property
(e.g., using Object.prototype.hasOwnProperty.call(origin, 'harness') or
equivalent) and only use options.harness when no internal origin was provided at
all; reference symbols: this._originHarness, sanitizeHarness, origin.harness,
options.harness.

---

Nitpick comments:
In `@packages/sdk-typescript/src/__tests__/harness.test.ts`:
- Around line 147-198: Add an end-to-end WS attribution test that exercises the
production path AgentClient.connect -> HttpClient.originHarness ->
withInternalWsOrigin -> WsClient: create a test that stubs global WebSocket
(like the existing WsClient tests do), then instantiate AgentClient (or the
exported AgentClient factory) with a token and harness value and call
AgentClient.connect(), assert the constructed WebSocket URL contains the harness
query param; also add a complementary test where no harness is supplied and
assert the harness param is omitted. Target symbols: AgentClient.connect,
HttpClient.originHarness, withInternalWsOrigin, and WsClient to ensure the real
SDK hop is covered.

In `@packages/sdk-typescript/src/origin.ts`:
- Around line 29-53: Replace the ad-hoc validation inside sanitizeHarness (and
remove HARNESS_ALLOWED/HARNESS_MAX_LENGTH usage here) with the shared zod-backed
parser used elsewhere: import the canonical harness zod schema/parser (e.g.,
harnessSchema or parseHarness) and call it to validate/transform the input so it
performs trim, regex, length cap and lowercasing; return undefined when the
parser indicates invalid/empty input and return the parser's transformed string
when valid. Ensure sanitizeHarness delegates entirely to that shared parser so
the wire contract is centralized and both sides stay in sync.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7394cc88-a7cb-423d-9b8c-66f963fac0a3

📥 Commits

Reviewing files that changed from the base of the PR and between 44e0717 and 8be3f1a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (14)
  • .agentworkforce/trajectories/completed/2026-06/traj_1774522548467_9eeddc03/summary.md
  • .agentworkforce/trajectories/completed/2026-06/traj_1774522548467_9eeddc03/trajectory.json
  • packages/engine/src/lib/__tests__/origin.test.ts
  • packages/engine/src/lib/origin.ts
  • packages/engine/src/lib/serverTelemetry.ts
  • packages/engine/src/middleware/logger.ts
  • packages/sdk-typescript/CHANGELOG.md
  • packages/sdk-typescript/src/__tests__/harness.test.ts
  • packages/sdk-typescript/src/agent.ts
  • packages/sdk-typescript/src/client.ts
  • packages/sdk-typescript/src/index.ts
  • packages/sdk-typescript/src/origin.ts
  • packages/sdk-typescript/src/relay.ts
  • packages/sdk-typescript/src/ws.ts

Comment on lines +145 to +147
// A wrapping host's internal origin is authoritative about the harness;
// fall back to the public `harness` option for plain consumers.
this._originHarness = sanitizeHarness(origin.harness ?? options.harness);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Internal origin does not fully override the public harness option.

Line 147 falls back to options.harness whenever origin.harness is undefined, even if an internal origin is present. That breaks the precedence promised in this PR: a wrapper that sets internal origin metadata but intentionally leaves harness unset will still leak the caller-supplied public harness and misattribute requests.

Suggested fix
   constructor(options: ClientOptions) {
-    const origin = readInternalOrigin(options) ?? SDK_ORIGIN;
+    const internalOrigin = readInternalOrigin(options);
+    const origin = internalOrigin ?? SDK_ORIGIN;
     this._apiKey = options.apiKey;
     this._baseUrl = options.baseUrl ?? 'https://gateway.relaycast.dev';
     this._originSurface = origin.surface;
     this._originClient = origin.client;
     this._originVersion = origin.version;
-    this._originHarness = sanitizeHarness(origin.harness ?? options.harness);
+    this._originHarness = sanitizeHarness(
+      internalOrigin ? internalOrigin.harness : options.harness,
+    );
     this._retryPolicy = normalizeRetryPolicy(options.retryPolicy);
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk-typescript/src/client.ts` around lines 145 - 147, The current
assignment to this._originHarness uses origin.harness ?? options.harness which
falls back to the public harness when an internal origin object exists but
intentionally omits harness; change the precedence so that the mere presence of
an internal origin takes priority even if its harness is undefined. Update the
logic around this._originHarness and sanitizeHarness to check for the internal
origin's explicit harness property (e.g., using
Object.prototype.hasOwnProperty.call(origin, 'harness') or equivalent) and only
use options.harness when no internal origin was provided at all; reference
symbols: this._originHarness, sanitizeHarness, origin.harness, options.harness.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/sdk-typescript/CHANGELOG.md">

<violation number="1" location="packages/sdk-typescript/CHANGELOG.md:12">
P3: Changelog sentence implies `HARNESS_HEADER` performs sanitization (lowercasing, charset restriction, length capping) when it is just a constant header string. Only `sanitizeHarness` applies those operations.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


### Added
- Optional `harness` field on `RelayCastOptions`/`ClientOptions` and `WsClientOptions` (plus the internal `InternalOrigin` plumbing). A User-Agent-style identifier for the harness driving requests (e.g. `'claude-code/2.3 (model=opus-4.8)'`, `'codex'`, `'human'`); stamped as the `X-Relaycast-Harness` HTTP header and forwarded as the `harness` WS query param so server-side telemetry can attribute traffic. When a wrapping host supplies one via the internal origin it takes precedence over the public option. Invalid values (empty, control characters) are dropped rather than sent; the header is omitted entirely when no harness is set, so existing consumers are unchanged on the wire.
- `sanitizeHarness` and `HARNESS_HEADER` exported from the SDK root — lowercases, restricts to a UA-safe character set, caps at 120 chars.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: Changelog sentence implies HARNESS_HEADER performs sanitization (lowercasing, charset restriction, length capping) when it is just a constant header string. Only sanitizeHarness applies those operations.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-typescript/CHANGELOG.md, line 12:

<comment>Changelog sentence implies `HARNESS_HEADER` performs sanitization (lowercasing, charset restriction, length capping) when it is just a constant header string. Only `sanitizeHarness` applies those operations.</comment>

<file context>
@@ -9,7 +9,7 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht
 ### Added
 - Optional `harness` field on `RelayCastOptions`/`ClientOptions` and `WsClientOptions` (plus the internal `InternalOrigin` plumbing). A User-Agent-style identifier for the harness driving requests (e.g. `'claude-code/2.3 (model=opus-4.8)'`, `'codex'`, `'human'`); stamped as the `X-Relaycast-Harness` HTTP header and forwarded as the `harness` WS query param so server-side telemetry can attribute traffic. When a wrapping host supplies one via the internal origin it takes precedence over the public option. Invalid values (empty, control characters) are dropped rather than sent; the header is omitted entirely when no harness is set, so existing consumers are unchanged on the wire.
-- `sanitizeHarness` exported from `./origin.js` — lowercases, restricts to a UA-safe character set, caps at 120 chars.
+- `sanitizeHarness` and `HARNESS_HEADER` exported from the SDK root — lowercases, restricts to a UA-safe character set, caps at 120 chars.
 
 ### Breaking
</file context>
Suggested change
- `sanitizeHarness` and `HARNESS_HEADER` exported from the SDK root lowercases, restricts to a UA-safe character set, caps at 120 chars.
- `sanitizeHarness` (and the `HARNESS_HEADER` constant) exported from the SDK root. `sanitizeHarness` lowercases, restricts to a UA-safe character set, caps at 120 chars.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer applied fixes — committed and pushed 340d8d9 to this PR. The notes below describe what changed.

Reviewed PR #160 and made fixes.

I added missing docs for the new harness attribution contract in README.md and openapi.yaml, and restored unrelated package-lock.json peer metadata churn from the PR.

Verified locally:

  • npx turbo build
  • npx turbo test --concurrency=1
  • npx turbo lint --concurrency=1
  • Focused SDK/engine harness tests and builds

@willwashburn willwashburn merged commit 4d948e5 into main Jun 3, 2026
5 checks passed
@willwashburn willwashburn deleted the feat/harness-ua-telemetry branch June 3, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant