feat(telemetry): read X-Relaycast-Harness header and stamp on events (P0 of relay#881)#132
Open
willwashburn wants to merge 2 commits into
Open
feat(telemetry): read X-Relaycast-Harness header and stamp on events (P0 of relay#881)#132willwashburn wants to merge 2 commits into
willwashburn wants to merge 2 commits into
Conversation
Read the X-Relaycast-Harness HTTP header (set by relay's MCP client per relay#881) and propagate it through to every server-side PostHog event as the orchestrator_harness property. Lets us segment usage by the host orchestrator (claude-code, cursor, codex, ...) without depending on the free-form User-Agent. The value is sanitized (lowercased, ASCII-only, length-capped) but not strict-enum-validated, so a relay client can ship a new harness ident without a server release. Missing / oversized / non-ASCII falls back to "unknown". Plumbing: - New extractOrchestratorHarness helper in lib/origin.ts. - loggerMiddleware reads the header and stashes it on c.var so every request log line and downstream telemetry call sees the same value. - emitServerEvent injects orchestrator_harness into the per-event properties. - For WS sessions the value is forwarded via search-params to AgentDO / WorkspaceStreamDO so the eventual ws_session_ended event in the DO also carries it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughExtracts a harness identifier from the ChangesOrchestrator Harness Tracking
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 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/server/src/lib/serverTelemetry.ts">
<violation number="1" location="packages/server/src/lib/serverTelemetry.ts:49">
P2: `orchestrator_harness` can be overridden by `properties` because `...normalizedProperties` is spread after it. Move the stamped field after the spread so request-derived value always wins.</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
Comment on lines
49
to
50
| orchestrator_harness: orchestratorHarness, | ||
| ...normalizedProperties, |
There was a problem hiding this comment.
P2: orchestrator_harness can be overridden by properties because ...normalizedProperties is spread after it. Move the stamped field after the spread so request-derived value always wins.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/src/lib/serverTelemetry.ts, line 49:
<comment>`orchestrator_harness` can be overridden by `properties` because `...normalizedProperties` is spread after it. Move the stamped field after the spread so request-derived value always wins.</comment>
<file context>
@@ -39,6 +46,7 @@ export function emitServerEvent(
origin: requiredOriginInfo(c.req.raw),
properties: {
workspace_id: workspaceId,
+ orchestrator_harness: orchestratorHarness,
...normalizedProperties,
},
</file context>
Suggested change
| orchestrator_harness: orchestratorHarness, | |
| ...normalizedProperties, | |
| ...normalizedProperties, | |
| orchestrator_harness: orchestratorHarness, |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Preview deployed!
This preview shares the staging database and will be cleaned up when the PR is merged or closed. Run E2E testsnpm run e2e -- https://pr132-api.relaycast.dev --ciOpen observer dashboard |
`harness` alone is unambiguous in this context and pairs with the
`X-Relaycast-Harness` HTTP header name (already correct). Renaming
before events land in PostHog with the longer name keeps the property
schema honest.
Server-side renames:
- PostHog event property: `orchestrator_harness` → `harness` (logger
middleware request log line + `emitServerEvent` body + DO
`ws_session_ended` event).
- Helper function: `extractOrchestratorHarness()` → `extractHarness()`.
- Constants: `ORCHESTRATOR_HARNESS_HEADER` → `HARNESS_HEADER`,
`UNKNOWN_ORCHESTRATOR_HARNESS` → `UNKNOWN_HARNESS`.
- Hono context var: `c.get('orchestratorHarness')` → `c.get('harness')`
(typed via `AppVariables.harness: string` in `env.ts`).
- DO connection meta field: `orchestrator_harness` → `harness` on
both `WorkspaceConnectionMeta` and the agent DO equivalent.
- WS query param forwarded from worker → DOs: `orchestrator_harness`
→ `harness` (matches the SDK-side rename in relaycast#133).
- Tests updated to assert the new shape.
Paired with relay#888 (CLI/broker stamps `harness` on every event) and
relaycast#133 (SDK forwards `harness` on origin / WS query param).
HTTP header name `X-Relaycast-Harness` was already correct — unchanged
in the wire shape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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/server/src/lib/serverTelemetry.ts`:
- Around line 34-39: The final "?? UNKNOWN_HARNESS" is dead code because
extractHarness(...) already always returns a string; update the harness
derivation to remove the unreachable fallback so it reads: prefer
c.get('harness') and otherwise use extractHarness(c.req.raw.headers). Locate the
harness assignment (variable harness) and the call to extractHarness in
serverTelemetry.ts and simplify the expression to remove the third fallback
(UNKNOWN_HARNESS).
🪄 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: 64530d64-f767-4b20-90f4-45e73b92c7ed
📒 Files selected for processing (9)
packages/server/src/durable-objects/agent.tspackages/server/src/durable-objects/workspaceStream.tspackages/server/src/env.tspackages/server/src/lib/__tests__/origin.test.tspackages/server/src/lib/__tests__/serverTelemetry.test.tspackages/server/src/lib/origin.tspackages/server/src/lib/serverTelemetry.tspackages/server/src/middleware/logger.tspackages/server/src/worker.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/durable-objects/agent.ts
Comment on lines
+34
to
+39
| // Prefer the value stashed by the logger middleware. Fall back to reading | ||
| // the header directly so emitters that bypass middleware (e.g. test harnesses | ||
| // or routes mounted before loggerMiddleware) still get a sane value. | ||
| const harness = c.get('harness') | ||
| ?? extractHarness(c.req.raw.headers) | ||
| ?? UNKNOWN_HARNESS; |
There was a problem hiding this comment.
Unreachable fallback in harness derivation chain.
The third fallback ?? UNKNOWN_HARNESS is unreachable because extractHarness always returns a string (never null or undefined). When c.get('harness') is undefined, extractHarness(c.req.raw.headers) is called and will return either a valid harness string or UNKNOWN_HARNESS directly, so the final ?? never triggers.
🧹 Proposed fix to remove dead code
- const harness = c.get('harness')
- ?? extractHarness(c.req.raw.headers)
- ?? UNKNOWN_HARNESS;
+ const harness = c.get('harness') ?? extractHarness(c.req.raw.headers);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Prefer the value stashed by the logger middleware. Fall back to reading | |
| // the header directly so emitters that bypass middleware (e.g. test harnesses | |
| // or routes mounted before loggerMiddleware) still get a sane value. | |
| const harness = c.get('harness') | |
| ?? extractHarness(c.req.raw.headers) | |
| ?? UNKNOWN_HARNESS; | |
| // Prefer the value stashed by the logger middleware. Fall back to reading | |
| // the header directly so emitters that bypass middleware (e.g. test harnesses | |
| // or routes mounted before loggerMiddleware) still get a sane value. | |
| const harness = c.get('harness') ?? extractHarness(c.req.raw.headers); |
🤖 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/server/src/lib/serverTelemetry.ts` around lines 34 - 39, The final
"?? UNKNOWN_HARNESS" is dead code because extractHarness(...) already always
returns a string; update the harness derivation to remove the unreachable
fallback so it reads: prefer c.get('harness') and otherwise use
extractHarness(c.req.raw.headers). Locate the harness assignment (variable
harness) and the call to extractHarness in serverTelemetry.ts and simplify the
expression to remove the third fallback (UNKNOWN_HARNESS).
There was a problem hiding this comment.
1 issue found across 9 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/server/src/lib/serverTelemetry.ts">
<violation number="1" location="packages/server/src/lib/serverTelemetry.ts:49">
P1: This rename changes the telemetry property key from `orchestrator_harness` to `harness`, which breaks the expected event schema for server telemetry.</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
| origin: requiredOriginInfo(c.req.raw), | ||
| properties: { | ||
| workspace_id: workspaceId, | ||
| harness, |
There was a problem hiding this comment.
P1: This rename changes the telemetry property key from orchestrator_harness to harness, which breaks the expected event schema for server telemetry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/src/lib/serverTelemetry.ts, line 49:
<comment>This rename changes the telemetry property key from `orchestrator_harness` to `harness`, which breaks the expected event schema for server telemetry.</comment>
<file context>
@@ -46,7 +46,7 @@ export function emitServerEvent(
properties: {
workspace_id: workspaceId,
- orchestrator_harness: orchestratorHarness,
+ harness,
...normalizedProperties,
},
</file context>
Suggested change
| harness, | |
| orchestrator_harness: harness, |
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
1 participant
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.
Server side of the harness instrumentation in AgentWorkforce/relay#881. Reads the
X-Relaycast-HarnessHTTP header and theharnessWS query param, sanitises both, and stamps the value on every server-side PostHog event.What
extractHarness(headers)inpackages/server/src/lib/origin.ts— lowercase, ASCII-only ([a-zA-Z0-9._-]), 40-char cap. Anything off-shape falls back to'unknown'.loggerMiddlewarereads the header once at request entry and stashes it onc.var.harness(typed viaAppVariables.harness: stringinenv.ts) so every downstream emitter and log line sees the same value without re-parsing the header.emitServerEventprefers the context variable, falls back to direct header read, then toUNKNOWN_HARNESS. The value is injected into every server-side PostHog event'sproperties.harness.ws_session_endedevent in the DO also carries it.Rename note (commit
455b091)Originally named everything
orchestrator_harnessto be unambiguous. Renamed across all three PRs (relay#888 + relaycast#132 + relaycast#133) to drop the redundantorchestrator_prefix — the HTTP header was alwaysX-Relaycast-Harness, the SDK origin field isharness, so the server-side property name might as well match. Renaming now (before events land in PostHog with the longer name) keeps the property schema honest.Server-side renames in this PR:
orchestrator_harness→harnessextractOrchestratorHarness()→extractHarness()ORCHESTRATOR_HARNESS_HEADER→HARNESS_HEADER,UNKNOWN_ORCHESTRATOR_HARNESS→UNKNOWN_HARNESSc.get('orchestratorHarness')→c.get('harness')orchestrator_harness→harnessTests
packages/server/src/lib/__tests__/origin.test.ts— 7 tests onextractHarness: missing / empty / lowercases / accepts unknown / oversized / disallowed chars.packages/server/src/lib/__tests__/serverTelemetry.test.ts— 5 tests onemitServerEventcovering both context-var and header fallback paths, plus the unknown default.Paired PRs
relay#888— CLI/broker detects harness, stampsX-Relaycast-Harnesson outgoing requests (currently via a fetch interceptor; will switch to the SDK origin path once feat(sdk): optional harness on InternalOrigin → X-Relaycast-Harness #133 publishes).relaycast#133—@relaycast/sdk1.2.0 addsharnesstoInternalOriginso the relay-side caller can replace the fetch interceptor with a clean origin-field set.🤖 Generated with Claude Code