feat(sdk): optional harness on InternalOrigin → X-Relaycast-Harness#133
feat(sdk): optional harness on InternalOrigin → X-Relaycast-Harness#133willwashburn wants to merge 2 commits into
Conversation
Lets a wrapping host (e.g. `@agent-relay/relaycast-mcp`) tag every
request with the orchestrator harness driving the process (claude-code,
cursor, codex, ...) without monkey-patching globalThis.fetch on the
caller side.
Adds:
- Optional `harness?: string` on `InternalOrigin`.
- `sanitizeHarness()` — lowercase / ASCII-only / 40-char cap, mirrors
the server-side contract from #132. Invalid inputs drop the header
entirely rather than sending garbage that the server will coerce.
- HttpClient: stamps `X-Relaycast-Harness` when harness is present,
omits the header entirely when absent (keeps plain SDK consumers
unchanged).
- WsClient: forwards as the `orchestrator_harness` query param so the
same value reaches AgentDO / WorkspaceStreamDO via the existing
DO startup search-params path from #132.
- `harness` survives `HttpClient.withApiKey()` rotations.
- 7 new tests covering header presence/absence, sanitisation, length
cap, character-set rejection, and the WS query-param path.
Bumps SDK to 1.2.0 (minor — additive optional field, no breaking changes
for existing `new RelayCast(...)` or `createInternalRelayCast(...)`
callers).
Paired with AgentWorkforce/relay#888 which will drop its fetch
interceptor and just stamp `harness` on the `mcpOrigin` literal it
already builds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds an optional ChangesHarness Origin and Client Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export function sanitizeHarness(value: string | undefined): string | undefined { | ||
| if (!value) return undefined; | ||
| const trimmed = value.trim().toLowerCase(); | ||
| if (!trimmed) return undefined; | ||
| // ASCII letters, digits, hyphens. Reject anything else outright. | ||
| if (!/^[a-z0-9-]+$/.test(trimmed)) return undefined; | ||
| return trimmed.length > 40 ? trimmed.slice(0, 40) : trimmed; | ||
| } |
There was a problem hiding this comment.
🔴 sanitizeHarness uses ad-hoc regex validation instead of a zod schema
sanitizeHarness at packages/sdk-typescript/src/origin.ts:35-42 performs validation and normalization using manual checks (.trim(), .toLowerCase(), a regex test, .slice()). The AGENTS.md engineering rule states: "Prefer zod schemas for validation instead of ad-hoc manual checks." The codebase already uses zod extensively (e.g. apiEnvelopeSchema at packages/sdk-typescript/src/client.ts:109), and this validation could be expressed as a zod schema with .trim(), .toLowerCase(), .regex(), and .max() transforms.
Prompt for agents
The sanitizeHarness function in packages/sdk-typescript/src/origin.ts uses ad-hoc manual checks (trim, toLowerCase, regex, slice) which violates the AGENTS.md rule: "Prefer zod schemas for validation instead of ad-hoc manual checks." The file already imports nothing from zod, but the package has zod as a dependency.
Replace the manual validation with a zod schema. For example, define a harnessSchema using z.string().trim().toLowerCase() piped through z.string().regex(/^[a-z0-9-]+$/).max(40), then use safeParse in the sanitizeHarness function. If parsing fails, return undefined. This keeps the same runtime semantics (trim, lowercase, validate charset, cap at 40 chars, return undefined on failure) but uses the zod-idiomatic approach consistent with the rest of the codebase.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** | ||
| * Optional parent orchestrator harness slug (e.g. `claude-code`, `cursor`, | ||
| * `codex`). When set, the HTTP client stamps `X-Relaycast-Harness` on every | ||
| * request and the WS client forwards it via the `harness` query param. The |
There was a problem hiding this comment.
🟡 JSDoc says WS query param is harness but actual wire name is orchestrator_harness
The JSDoc comment on InternalOrigin.harness at packages/sdk-typescript/src/origin.ts:10 says "the WS client forwards it via the harness query param," but the actual implementation at packages/sdk-typescript/src/ws.ts:115 uses orchestrator_harness as the query parameter name. This mismatch will mislead developers reading the interface docs when trying to understand the server-side contract.
| * request and the WS client forwards it via the `harness` query param. The | |
| * request and the WS client forwards it via the `orchestrator_harness` query param. The |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/__tests__/harness-origin.test.ts`:
- Around line 101-115: The test currently never calls withApiKey(), so it
doesn't verify harness preservation; update the test to obtain the internal
HttpClient from the relay returned by createInternalRelayCast (use the relay's
internal client access path created by createInternalRelayCast / as() chain),
call withApiKey('new_key') on that internal client, and assert the returned
client instance still has originHarness === 'cursor' (use HttpClient,
createInternalRelayCast, withApiKey, and originHarness identifiers to locate and
update the code).
🪄 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: 3e14729d-82e3-4813-b9ff-6432be636165
📒 Files selected for processing (6)
packages/sdk-typescript/CHANGELOG.mdpackages/sdk-typescript/package.jsonpackages/sdk-typescript/src/__tests__/harness-origin.test.tspackages/sdk-typescript/src/client.tspackages/sdk-typescript/src/origin.tspackages/sdk-typescript/src/ws.ts
| it('preserves the harness across withApiKey()', async () => { | ||
| const { HttpClient } = await import('../client.js'); | ||
| const { createInternalRelayCast } = await import('../internal.js'); | ||
| // Reach into the client constructor via createInternalRelayCast → as() chain | ||
| // by checking the static side: directly exercise HttpClient through internal origin. | ||
| const relay = createInternalRelayCast( | ||
| { apiKey: 'rk_live_test' }, | ||
| { surface: 'mcp', client: '@agent-relay/relaycast-mcp', version: '6.0.0', harness: 'cursor' }, | ||
| ); | ||
| // Smoke-test: build a fresh HttpClient via withApiKey on the underlying client. | ||
| // We construct one directly to assert the getter survives. | ||
| const internalClient = new HttpClient({ apiKey: 'rk_live_test' }); | ||
| expect(internalClient.originHarness).toBeUndefined(); | ||
| void relay; // referenced for typing only | ||
| }); |
There was a problem hiding this comment.
Test doesn't verify what its title claims.
The test is titled "preserves the harness across withApiKey()" but doesn't actually call withApiKey(). It creates a relay with harness 'cursor', then creates a completely separate HttpClient instance without any origin and asserts it has no harness.
To properly test preservation across key rotation, the test should access the internal HTTP client from the relay, call withApiKey('new_key') on it, and verify that the returned client still has originHarness === 'cursor'.
The core functionality itself is correctly implemented (line 177 in client.ts preserves harness), but this test doesn't validate it.
🧪 Suggested fix
it('preserves the harness across withApiKey()', async () => {
- const { HttpClient } = await import('../client.js');
+ const { withInternalOrigin } = await import('../client.js');
+ const { HttpClient } = await import('../client.js');
- const { createInternalRelayCast } = await import('../internal.js');
- // Reach into the client constructor via createInternalRelayCast → as() chain
- // by checking the static side: directly exercise HttpClient through internal origin.
- const relay = createInternalRelayCast(
+
+ const client = new HttpClient(withInternalOrigin(
{ apiKey: 'rk_live_test' },
{ surface: 'mcp', client: '`@agent-relay/relaycast-mcp`', version: '6.0.0', harness: 'cursor' },
- );
- // Smoke-test: build a fresh HttpClient via withApiKey on the underlying client.
- // We construct one directly to assert the getter survives.
- const internalClient = new HttpClient({ apiKey: 'rk_live_test' });
- expect(internalClient.originHarness).toBeUndefined();
- void relay; // referenced for typing only
+ ));
+
+ expect(client.originHarness).toBe('cursor');
+
+ const rotatedClient = client.withApiKey('rk_live_rotated');
+ expect(rotatedClient.originHarness).toBe('cursor');
});🤖 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-origin.test.ts` around lines
101 - 115, The test currently never calls withApiKey(), so it doesn't verify
harness preservation; update the test to obtain the internal HttpClient from the
relay returned by createInternalRelayCast (use the relay's internal client
access path created by createInternalRelayCast / as() chain), call
withApiKey('new_key') on that internal client, and assert the returned client
instance still has originHarness === 'cursor' (use HttpClient,
createInternalRelayCast, withApiKey, and originHarness identifiers to locate and
update the code).
![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: 51c5db76d7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.originHarness) { | ||
| wsUrl.searchParams.set('orchestrator_harness', this.originHarness); |
There was a problem hiding this comment.
This branch never runs for the primary createInternalRelayCast(...).as(token).connect() flow because AgentClient.connect() rebuilds the WS internal origin with only surface/client/version and drops harness (see packages/sdk-typescript/src/agent.ts connect path), so originHarness is undefined here and orchestrator_harness is omitted. As a result, the new feature only tags HTTP requests, while WS traffic from the normal AgentClient path still loses harness attribution.
Useful? React with 👍 / 👎.
| it('preserves the harness across withApiKey()', async () => { | ||
| const { HttpClient } = await import('../client.js'); | ||
| const { createInternalRelayCast } = await import('../internal.js'); | ||
| // Reach into the client constructor via createInternalRelayCast → as() chain | ||
| // by checking the static side: directly exercise HttpClient through internal origin. | ||
| const relay = createInternalRelayCast( | ||
| { apiKey: 'rk_live_test' }, | ||
| { surface: 'mcp', client: '@agent-relay/relaycast-mcp', version: '6.0.0', harness: 'cursor' }, | ||
| ); | ||
| // Smoke-test: build a fresh HttpClient via withApiKey on the underlying client. | ||
| // We construct one directly to assert the getter survives. | ||
| const internalClient = new HttpClient({ apiKey: 'rk_live_test' }); | ||
| expect(internalClient.originHarness).toBeUndefined(); |
There was a problem hiding this comment.
This test is labeled as verifying harness preservation across withApiKey(), but it never calls withApiKey and instead constructs a fresh HttpClient without internal origin and asserts originHarness is undefined. That means regressions in the actual withApiKey harness propagation path would still pass this suite, leaving the new behavior effectively untested.
Useful? React with 👍 / 👎.
Paired with relay#888 and relaycast#132 (server side). `harness` alone
is unambiguous — the SDK origin already carries `surface`/`client`/`version`
so "orchestrator_harness" was redundant.
Changes in this commit:
- WS query param: `orchestrator_harness` → `harness` to match the
HTTP header naming.
- Doc comments on `InternalOrigin.harness` and the test header dropped
the "orchestrator_" prefix.
- CHANGELOG entry updated to the new shape.
- Added 2 focused WS tests asserting the query-param name (present
when origin supplies one, absent otherwise).
`InternalOrigin.harness` field and `X-Relaycast-Harness` header were
already correctly named — no breaking change to the wire shape for
either direction; the WS query-param flip is consumed by relaycast#132
which renames in lockstep.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 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/origin.ts">
<violation number="1" location="packages/sdk-typescript/src/origin.ts:10">
P3: The `InternalOrigin.harness` comment documents the wrong WS query parameter name (`harness` vs `orchestrator_harness`), which can mislead integrators wiring telemetry tags.</violation>
</file>
<file name="packages/sdk-typescript/src/__tests__/harness-origin.test.ts">
<violation number="1" location="packages/sdk-typescript/src/__tests__/harness-origin.test.ts:113">
P2: The `preserves the harness across withApiKey()` test does not actually assert `withApiKey` behavior, so harness-rotation regressions can slip through.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fix all with cubic | Re-trigger cubic
| // Smoke-test: build a fresh HttpClient via withApiKey on the underlying client. | ||
| // We construct one directly to assert the getter survives. | ||
| const internalClient = new HttpClient({ apiKey: 'rk_live_test' }); | ||
| expect(internalClient.originHarness).toBeUndefined(); |
There was a problem hiding this comment.
P2: The preserves the harness across withApiKey() test does not actually assert withApiKey behavior, so harness-rotation regressions can slip through.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-typescript/src/__tests__/harness-origin.test.ts, line 113:
<comment>The `preserves the harness across withApiKey()` test does not actually assert `withApiKey` behavior, so harness-rotation regressions can slip through.</comment>
<file context>
@@ -0,0 +1,130 @@
+ // Smoke-test: build a fresh HttpClient via withApiKey on the underlying client.
+ // We construct one directly to assert the getter survives.
+ const internalClient = new HttpClient({ apiKey: 'rk_live_test' });
+ expect(internalClient.originHarness).toBeUndefined();
+ void relay; // referenced for typing only
+ });
</file context>
| /** | ||
| * Optional parent orchestrator harness slug (e.g. `claude-code`, `cursor`, | ||
| * `codex`). When set, the HTTP client stamps `X-Relaycast-Harness` on every | ||
| * request and the WS client forwards it via the `harness` query param. The |
There was a problem hiding this comment.
P3: The InternalOrigin.harness comment documents the wrong WS query parameter name (harness vs orchestrator_harness), which can mislead integrators wiring telemetry tags.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-typescript/src/origin.ts, line 10:
<comment>The `InternalOrigin.harness` comment documents the wrong WS query parameter name (`harness` vs `orchestrator_harness`), which can mislead integrators wiring telemetry tags.</comment>
<file context>
@@ -4,10 +4,39 @@ export interface InternalOrigin {
+ /**
+ * Optional parent orchestrator harness slug (e.g. `claude-code`, `cursor`,
+ * `codex`). When set, the HTTP client stamps `X-Relaycast-Harness` on every
+ * request and the WS client forwards it via the `harness` query param. The
+ * server side reads either and tags `orchestrator_harness` on telemetry
+ * events.
</file context>
| * request and the WS client forwards it via the `harness` query param. The | |
| * request and the WS client forwards it via the `orchestrator_harness` query param. The |
1 participant
Replaces the need for the relay-side
globalThis.fetchinterceptor in AgentWorkforce/relay#888 by extending the existingInternalOriginplumbing.Why
Hosts that wrap the SDK (the relaycast-mcp shim in
agent-relay) need to tag every request with the harness driving the process (Claude Code, Cursor, Codex, ...). Today they have to either:globalThis.fetch(what relay#888 currently does, ~100 lines)Option (1) and (3) are non-starters; option (2) leaks across non-relaycast fetches and breaks under custom relaycast deploys / staging hosts. The right shape is the path the SDK already exposes for
surface/client/version: pass the value throughInternalOrigin, let the client stamp it.What
InternalOrigingains an optionalharness?: string. Existing callers don't notice — the field is opt-in.sanitizeHarness()mirrors the server-side contract from feat(telemetry): read X-Relaycast-Harness header and stamp on events (P0 of relay#881) #132 (lowercase, ASCII-only, ≤40 chars). Invalid inputs drop the header entirely rather than sending garbage; gives us defence in depth so a buggy upstream caller can't smuggle a CRLF injection past the relaycast WAF.HttpClientstampsX-Relaycast-Harnesswhen present, omits it entirely when absent — plainnew RelayCast(...)consumers are unchanged on the wire.WsClientforwards asharnessquery param (matches the durable-object stream-start shape after the rename in feat(telemetry): read X-Relaycast-Harness header and stamp on events (P0 of relay#881) #132).harnesssurvivesHttpClient.withApiKey()rotations.Rename note (commit
236b8df)Originally landed with the WS query param named
orchestrator_harnessto mirror the server-side property. Renamed alongside relaycast#132 and relay#888 to drop the redundantorchestrator_prefix — the HTTP header was alreadyX-Relaycast-Harness, the WS param now matches that name asharness. Doc-comment also updated.Tests
__tests__/harness-origin.test.ts: 7 HTTP (presence / absence / sanitisation / length cap / character-set rejection / sanitizeHarness unit / withApiKey survival) + 2 WS (query-param presence / absence).vitest run packages/sdk-typescript): 309 passed, 0 regressions.npm --prefix packages/sdk-typescript run lint: clean.Version
Bumped
@relaycast/sdkto1.2.0— minor since the change is purely additive (optional field). Existing consumers compile and behave identically; the header only appears when an upstream caller deliberately opts in.Paired PR
agent-relay#888 will:@relaycast/sdkdep from^1.1.0→^1.2.0.src/cli/lib/relaycast-fetch-interceptor.ts+ its tests.src/cli/bootstrap.tsandsrc/cli/relaycast-mcp.ts.harness: detectHarness()to themcpOriginliteral inrelaycast-mcp.tsand to theMessagingRelaycastClientorigin inmessaging.ts.🤖 Generated with Claude Code