feat(telemetry): harness detection + outgoing header + install/update + SDK events (P0 of #881)#888
Open
willwashburn wants to merge 6 commits into
Open
feat(telemetry): harness detection + outgoing header + install/update + SDK events (P0 of #881)#888willwashburn wants to merge 6 commits into
willwashburn wants to merge 6 commits into
Conversation
Detect the parent harness driving the CLI/broker (Claude Code, Cursor,
Codex, Gemini, Aider, Cline, Continue, Windsurf, Zed) so PostHog can
split usage by orchestrator. Adds `orchestrator_harness` to
`CommonProperties` on every event.
Implementation (Option A — TS detects, passes to broker via env):
- `packages/telemetry/src/harness.ts` walks the parent process tree on
Linux (`/proc/<pid>/comm` + `PPid:`) and macOS (`ps -o ppid=,command=`)
to find the first ancestor matching a known basename, capped at 10
hops. Cached after the first call.
- `src/telemetry.rs` mirrors the classifier and adds a `getppid()`-based
walk for the SDK case where user code spawns the broker directly.
Both sides honour `AGENT_RELAY_ORCHESTRATOR_HARNESS` first so the CLI
can detect once and avoid duplicate work.
- Fallback is `unknown` on every failure — keeping the long tail visible
in PostHog tells us which harnesses to add to the classifier next.
Schema lives in three places that must stay in sync: the TS union in
`harness.ts`, the Rust slug list in `telemetry.rs`, and the relaycast
server-side enum in AgentWorkforce/relaycast#132. Slugs are
lower-kebab-case, ASCII, ≤11 chars so they round-trip cleanly through
the relaycast sanitiser without coercion.
Also adds `surface: "broker"` on every broker-originated event so
dashboards can split CLI vs broker vs SDK with a single filter.
Two follow-on pieces of the P0 instrumentation, kept separate from the
detection commit so the diffs read cleanly.
SDK telemetry (`packages/sdk/src/telemetry.ts`)
- Tiny fire-and-forget PostHog client inlined into the SDK to avoid
pulling `posthog-node` as a runtime dep (the SDK is a published
library consumed by user code).
- No-ops silently when `AGENT_RELAY_POSTHOG_KEY` is unset — coexists
with the build-time key injection from #883 without regressing the
forks/local-dev path.
- Respects existing opt-outs (`AGENT_RELAY_TELEMETRY_DISABLED`,
`DO_NOT_TRACK`).
- Uses `AbortSignal.timeout(5_000)` so a slow PostHog never blocks user
workflows.
- `trackSdkEvent`/`flushSdkTelemetry` are wired into `WorkflowBuilder.run`
(emits `sdk_workflow_run` with workflow name, step count, duration,
success, error_class) and the primary `AgentRelayClient` methods
(spawnPty, spawnProvider, release, listAgents, sendMessage —
`sdk_method_call`).
- All SDK events carry `surface: "sdk"` so dashboards can split CLI vs
broker vs SDK on a single filter.
Outgoing X-Relaycast-Harness header (`src/cli/lib/relaycast-fetch-interceptor.ts`)
- The `@relaycast/sdk` HTTP client builds its own header set internally
and doesn't expose a hook for arbitrary headers. Rather than fork the
SDK or wrap every API surface, we patch `globalThis.fetch` to stamp
the harness header on requests to `relaycast.dev` hosts.
- Installed once from `bootstrap.ts` (covers the main CLI) and again
from the `relaycast-mcp.ts` stdio entrypoint (covers the case where
Claude Code/Cursor/Zed spawn the MCP shim directly via .mcp.json).
- Caller-provided overrides win; non-relaycast hosts pass through
unchanged.
- Paired with the relaycast-side reader in
AgentWorkforce/relaycast#132.
Contributor
📝 WalkthroughWalkthroughThis PR adds telemetry event types and an SDK PostHog client, instruments SDK methods and workflows, implements orchestrator harness detection (TS + Rust), propagates harness via a CLI fetch interceptor and child env, enhances CLI update telemetry, adds tests, and updates trajectory/release docs. ChangesTelemetry event schema and orchestrator harness foundation
SDK telemetry client and method instrumentation
CLI harness propagation and fetch interception
Broker (Rust) harness detection and telemetry integration
Release and trajectory documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/telemetry/src/harness.ts (1)
121-137: 💤 Low valueStatic analysis flag is a false positive —
pidis a numeric value fromprocess.ppid.OpenGrep flags this
execSynccall as a command injection risk, butpidoriginates fromprocess.ppid(a number) and subsequent parent PID parsing. It's never user-controlled input. The coercion via template string is safe.For defense-in-depth, you could use
execFileSyncwith an argument array, but this is optional given the controlled input source.♻️ Optional: use execFileSync for defense-in-depth
-import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process';function readDarwinProcInfo(pid: number): ProcInfo | null { try { // Single `ps` call returns ppid and full command path. - const out = execSync(`ps -o ppid=,command= -p ${pid}`, { + const out = execFileSync('ps', ['-o', 'ppid=,command=', '-p', String(pid)], { encoding: 'utf-8', timeout: 1000, stdio: ['ignore', 'pipe', 'ignore'], }).trim();🤖 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/telemetry/src/harness.ts` around lines 121 - 137, Static analyzer flags the execSync call in readDarwinProcInfo as a command-injection risk; validate and harden the call by ensuring pid is a finite integer and replacing execSync with execFileSync using an argv array to avoid shell interpolation. In function readDarwinProcInfo, coerce/validate pid (Number.isFinite and Math.floor) then call execFileSync('ps', ['-o', 'ppid=,command=', '-p', String(pid)], { encoding: 'utf-8', timeout: 1000, stdio: ['ignore','pipe','ignore'] }) and keep the same parsing of out, returning the same ProcInfo or null; this preserves behavior while removing the shell-invoked execSync usage.src/telemetry.rs (1)
758-812: 💤 Low valueEnvironment variable tests can be flaky when run in parallel.
Rust's
cargo testruns tests in parallel by default. Tests that modify environment variables likeAGENT_RELAY_ORCHESTRATOR_HARNESSandAGENT_RELAY_TELEMETRY_DISABLEDcan interfere with each other if they run concurrently. For example,detect_orchestrator_harness_respects_env_hint(lines 802-811),disabled_client_does_not_panic(lines 754-771), anddo_not_track_disables_telemetry(lines 814-826) all modify shared process state without synchronization. Even though cleanup occurs at test end, one test's cleanup may not complete before another test reads the variable.Consider using
#[serial]from theserial_testcrate, or run tests withcargo test -- --test-threads=1.🤖 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 `@src/telemetry.rs` around lines 758 - 812, Tests that mutate process-wide environment variables (disabled_client_does_not_panic, detect_orchestrator_harness_respects_env_hint, do_not_track_disables_telemetry) are flaky when run in parallel; mark these tests as serial to avoid race conditions by adding #[serial] (from the serial_test crate) to each test and adding serial_test to dev-dependencies in Cargo.toml, or alternatively run tests single-threaded (cargo test -- --test-threads=1); ensure you apply the attribute to the three test functions named above so env set/remove calls are serialized.packages/sdk/src/workflows/builder.ts (1)
506-517: ⚡ Quick winAvoid awaiting telemetry import in
finally.At Line [509], awaiting the dynamic import delays
run()completion/throw path. For fire-and-forget telemetry, schedule this without awaiting.Suggested fix
- try { - const { trackSdkEvent } = await import('../telemetry.js'); - trackSdkEvent('sdk_workflow_run', { - workflow_name: workflowName, - step_count: stepCount, - success, - duration_ms: Date.now() - startedAt, - ...(errorClass ? { error_class: errorClass } : {}), - }); - } catch { - // Telemetry must never alter user-visible behaviour. - } + void import('../telemetry.js') + .then(({ trackSdkEvent }) => { + trackSdkEvent('sdk_workflow_run', { + workflow_name: workflowName, + step_count: stepCount, + success, + duration_ms: Date.now() - startedAt, + ...(errorClass ? { error_class: errorClass } : {}), + }); + }) + .catch(() => { + // Telemetry must never alter user-visible behaviour. + });🤖 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/src/workflows/builder.ts` around lines 506 - 517, The telemetry dynamic import inside the finally block of the workflow run is being awaited (the try { const { trackSdkEvent } = await import('../telemetry.js'); ... } catch { }) which can delay run() completion; change it to fire-and-forget by removing the await and scheduling the import as a non-blocking promise (e.g., import('../telemetry.js').then(({ trackSdkEvent }) => trackSdkEvent('sdk_workflow_run', { workflow_name: workflowName, step_count: stepCount, success, duration_ms: Date.now() - startedAt, ...(errorClass ? { error_class: errorClass } : {}) })).catch(() => {})), so trackSdkEvent is invoked asynchronously without affecting run()’s completion or throw path.packages/sdk/src/client.ts (1)
19-19: ⚡ Quick winUse the shared telemetry wrapper to avoid drift.
Line [116]-Line [146] re-implements logic already exposed by
instrumentMethodinpackages/sdk/src/telemetry.ts. Reusing the shared helper keepssdk_method_callpayload semantics centralized.Also applies to: 116-146
🤖 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/src/client.ts` at line 19, The SDK client re-implements telemetry logic for emitting sdk_method_call instead of using the shared wrapper; replace the custom tracking block that manually calls trackSdkEvent (the logic between the custom start/stop/event code around the SDK method invocation) with a call to the shared instrumentMethod helper from telemetry.ts so payload semantics stay centralized. Locate the custom tracking code in the client method(s) that call trackSdkEvent and refactor to call instrumentMethod with the same method name, args, result/error handling, and any metadata so sdk_method_call payloads are produced by instrumentMethod rather than duplicated here.
🤖 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/src/telemetry.ts`:
- Around line 118-127: The code that creates and writes the machine id (using
fs.openSync -> writeSync -> closeSync with machineIdPath) can leak a file
descriptor if fs.writeSync throws; update the block that calls fs.openSync,
fs.writeSync and fs.closeSync (the machineId creation logic around
machineIdPath) to ensure the descriptor is always closed by wrapping
writeSync/closeSync in a try/finally (or equivalent), only calling fs.closeSync
if the fd was successfully opened; also handle errors from fs.openSync by
guarding the close and rethrowing the original error so callers see the failure.
In `@src/cli/bootstrap.ts`:
- Around line 124-126: The code unconditionally writes
process.env[HARNESS_ENV_VAR] with the result of detectOrchestratorHarness(),
which can be the literal 'unknown' and thus prevents child processes from
running their own detector; change the logic in the bootstrap flow so that after
calling detectOrchestratorHarness() you only assign process.env[HARNESS_ENV_VAR]
when the detected value is a real harness (e.g., not null/empty and not the
string 'unknown'), leaving the env var unset otherwise so children can run
detectOrchestratorHarness() themselves; update the block that references
HARNESS_ENV_VAR and detectOrchestratorHarness() accordingly.
In `@src/cli/relaycast-mcp.ts`:
- Around line 603-613: The dynamic import and call to
installRelaycastFetchInterceptor is currently fire-and-forget and may race with
startup; change the flow so we await the import and the
installRelaycastFetchInterceptor() call before invoking
startPatchedStdio(optionsFromEnv()). Specifically, ensure the promise returned
by import('./lib/relaycast-fetch-interceptor.js') is awaited, then call
installRelaycastFetchInterceptor() and await it if it returns a promise, handle
errors (e.g., log or swallow) and only after successful/handled installation
call startPatchedStdio with optionsFromEnv().
---
Nitpick comments:
In `@packages/sdk/src/client.ts`:
- Line 19: The SDK client re-implements telemetry logic for emitting
sdk_method_call instead of using the shared wrapper; replace the custom tracking
block that manually calls trackSdkEvent (the logic between the custom
start/stop/event code around the SDK method invocation) with a call to the
shared instrumentMethod helper from telemetry.ts so payload semantics stay
centralized. Locate the custom tracking code in the client method(s) that call
trackSdkEvent and refactor to call instrumentMethod with the same method name,
args, result/error handling, and any metadata so sdk_method_call payloads are
produced by instrumentMethod rather than duplicated here.
In `@packages/sdk/src/workflows/builder.ts`:
- Around line 506-517: The telemetry dynamic import inside the finally block of
the workflow run is being awaited (the try { const { trackSdkEvent } = await
import('../telemetry.js'); ... } catch { }) which can delay run() completion;
change it to fire-and-forget by removing the await and scheduling the import as
a non-blocking promise (e.g., import('../telemetry.js').then(({ trackSdkEvent })
=> trackSdkEvent('sdk_workflow_run', { workflow_name: workflowName, step_count:
stepCount, success, duration_ms: Date.now() - startedAt, ...(errorClass ? {
error_class: errorClass } : {}) })).catch(() => {})), so trackSdkEvent is
invoked asynchronously without affecting run()’s completion or throw path.
In `@packages/telemetry/src/harness.ts`:
- Around line 121-137: Static analyzer flags the execSync call in
readDarwinProcInfo as a command-injection risk; validate and harden the call by
ensuring pid is a finite integer and replacing execSync with execFileSync using
an argv array to avoid shell interpolation. In function readDarwinProcInfo,
coerce/validate pid (Number.isFinite and Math.floor) then call
execFileSync('ps', ['-o', 'ppid=,command=', '-p', String(pid)], { encoding:
'utf-8', timeout: 1000, stdio: ['ignore','pipe','ignore'] }) and keep the same
parsing of out, returning the same ProcInfo or null; this preserves behavior
while removing the shell-invoked execSync usage.
In `@src/telemetry.rs`:
- Around line 758-812: Tests that mutate process-wide environment variables
(disabled_client_does_not_panic, detect_orchestrator_harness_respects_env_hint,
do_not_track_disables_telemetry) are flaky when run in parallel; mark these
tests as serial to avoid race conditions by adding #[serial] (from the
serial_test crate) to each test and adding serial_test to dev-dependencies in
Cargo.toml, or alternatively run tests single-threaded (cargo test --
--test-threads=1); ensure you apply the attribute to the three test functions
named above so env set/remove calls are serialized.
🪄 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: bde8ab4f-ffba-4482-bcb2-54fb6e66f6a8
📒 Files selected for processing (15)
packages/sdk/src/client.tspackages/sdk/src/index.tspackages/sdk/src/telemetry.tspackages/sdk/src/workflows/builder.tspackages/telemetry/src/client.tspackages/telemetry/src/events.tspackages/telemetry/src/harness.test.tspackages/telemetry/src/harness.tspackages/telemetry/src/index.tssrc/cli/bootstrap.tssrc/cli/lib/core-maintenance.tssrc/cli/lib/relaycast-fetch-interceptor.test.tssrc/cli/lib/relaycast-fetch-interceptor.tssrc/cli/relaycast-mcp.tssrc/telemetry.rs
Comment on lines
+124
to
+126
| if (!process.env[HARNESS_ENV_VAR]) { | ||
| process.env[HARNESS_ENV_VAR] = detectOrchestratorHarness(); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid propagating unknown as the harness value.
At Line 124, setting HARNESS_ENV_VAR unconditionally (including 'unknown') prevents children from running their own detector, which your comment says should happen when the hint is absent.
Suggested fix
if (!process.env[HARNESS_ENV_VAR]) {
- process.env[HARNESS_ENV_VAR] = detectOrchestratorHarness();
+ const detectedHarness = detectOrchestratorHarness();
+ if (detectedHarness !== 'unknown') {
+ process.env[HARNESS_ENV_VAR] = detectedHarness;
+ }
}📝 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
| if (!process.env[HARNESS_ENV_VAR]) { | |
| process.env[HARNESS_ENV_VAR] = detectOrchestratorHarness(); | |
| } | |
| if (!process.env[HARNESS_ENV_VAR]) { | |
| const detectedHarness = detectOrchestratorHarness(); | |
| if (detectedHarness !== 'unknown') { | |
| process.env[HARNESS_ENV_VAR] = detectedHarness; | |
| } | |
| } |
🤖 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 `@src/cli/bootstrap.ts` around lines 124 - 126, The code unconditionally writes
process.env[HARNESS_ENV_VAR] with the result of detectOrchestratorHarness(),
which can be the literal 'unknown' and thus prevents child processes from
running their own detector; change the logic in the bootstrap flow so that after
calling detectOrchestratorHarness() you only assign process.env[HARNESS_ENV_VAR]
when the detected value is a real harness (e.g., not null/empty and not the
string 'unknown'), leaving the env var unset otherwise so children can run
detectOrchestratorHarness() themselves; update the block that references
HARNESS_ENV_VAR and detectOrchestratorHarness() accordingly.
Comment on lines
603
to
613
| // When relaycast-mcp runs as its own stdio subprocess (e.g. spawned by | ||
| // Claude Code/Cursor/Zed via .mcp.json), the parent CLI's bootstrap | ||
| // hook never ran — install the harness header interceptor ourselves so | ||
| // outgoing relaycast calls still carry `X-Relaycast-Harness`. | ||
| void import('./lib/relaycast-fetch-interceptor.js') | ||
| .then(({ installRelaycastFetchInterceptor }) => { | ||
| installRelaycastFetchInterceptor(); | ||
| }) | ||
| .catch(() => undefined); | ||
|
|
||
| startPatchedStdio(optionsFromEnv()).catch((error) => { |
Contributor
There was a problem hiding this comment.
Await interceptor installation before MCP startup.
At Line 607, this fire-and-forget import races with startup; early relaycast requests can be sent before the header patch is active.
Suggested fix
if (isEntrypoint()) {
- void import('./lib/relaycast-fetch-interceptor.js')
- .then(({ installRelaycastFetchInterceptor }) => {
- installRelaycastFetchInterceptor();
- })
- .catch(() => undefined);
-
- startPatchedStdio(optionsFromEnv()).catch((error) => {
+ const boot = async (): Promise<void> => {
+ try {
+ const { installRelaycastFetchInterceptor } = await import('./lib/relaycast-fetch-interceptor.js');
+ installRelaycastFetchInterceptor();
+ } catch {
+ // best-effort
+ }
+ await startPatchedStdio(optionsFromEnv());
+ };
+
+ boot().catch((error) => {
const message = error instanceof Error ? (error.stack ?? error.message) : String(error);
process.stderr.write(`${message}\n`);
process.exit(1);
});
}📝 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
| // When relaycast-mcp runs as its own stdio subprocess (e.g. spawned by | |
| // Claude Code/Cursor/Zed via .mcp.json), the parent CLI's bootstrap | |
| // hook never ran — install the harness header interceptor ourselves so | |
| // outgoing relaycast calls still carry `X-Relaycast-Harness`. | |
| void import('./lib/relaycast-fetch-interceptor.js') | |
| .then(({ installRelaycastFetchInterceptor }) => { | |
| installRelaycastFetchInterceptor(); | |
| }) | |
| .catch(() => undefined); | |
| startPatchedStdio(optionsFromEnv()).catch((error) => { | |
| // When relaycast-mcp runs as its own stdio subprocess (e.g. spawned by | |
| // Claude Code/Cursor/Zed via .mcp.json), the parent CLI's bootstrap | |
| // hook never ran — install the harness header interceptor ourselves so | |
| // outgoing relaycast calls still carry `X-Relaycast-Harness`. | |
| const boot = async (): Promise<void> => { | |
| try { | |
| const { installRelaycastFetchInterceptor } = await import('./lib/relaycast-fetch-interceptor.js'); | |
| installRelaycastFetchInterceptor(); | |
| } catch { | |
| // best-effort | |
| } | |
| await startPatchedStdio(optionsFromEnv()); | |
| }; | |
| boot().catch((error) => { | |
| const message = error instanceof Error ? (error.stack ?? error.message) : String(error); | |
| process.stderr.write(`${message}\n`); | |
| process.exit(1); | |
| }); |
🤖 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 `@src/cli/relaycast-mcp.ts` around lines 603 - 613, The dynamic import and call
to installRelaycastFetchInterceptor is currently fire-and-forget and may race
with startup; change the flow so we await the import and the
installRelaycastFetchInterceptor() call before invoking
startPatchedStdio(optionsFromEnv()). Specifically, ensure the promise returned
by import('./lib/relaycast-fetch-interceptor.js') is awaited, then call
installRelaycastFetchInterceptor() and await it if it returns a promise, handle
errors (e.g., log or swallow) and only after successful/handled installation
call startPatchedStdio with optionsFromEnv().
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 15 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/src/client.ts">
<violation number="1" location="packages/sdk/src/client.ts:121">
P2: Avoid duplicating `sdk_method_call` wrapper logic in `client.ts`; reuse the shared `instrumentMethod` helper to prevent drift in telemetry behavior.</violation>
</file>
<file name="packages/telemetry/src/harness.ts">
<violation number="1" location="packages/telemetry/src/harness.ts:97">
P2: `commandBasename` splits on whitespace too early, so quoted executable paths with spaces are misparsed and can be classified as `unknown`.</violation>
</file>
<file name="src/cli/relaycast-mcp.ts">
<violation number="1" location="src/cli/relaycast-mcp.ts:607">
P2: Interceptor installation is raced against startup, so early Relaycast requests may miss `X-Relaycast-Harness`.</violation>
</file>
<file name="packages/sdk/src/telemetry.ts">
<violation number="1" location="packages/sdk/src/telemetry.ts:217">
P2: Caller-supplied properties currently override required common telemetry fields; keep canonical SDK fields authoritative.</violation>
<violation number="2" location="packages/sdk/src/telemetry.ts:242">
P2: Truncating `inFlight` drops unresolved events, so `flushSdkTelemetry()` may not wait for all outstanding telemetry posts.</violation>
</file>
<file name="packages/sdk/src/workflows/builder.ts">
<violation number="1" location="packages/sdk/src/workflows/builder.ts:509">
P2: `run()` now blocks on telemetry module import in `finally`; make telemetry dispatch non-blocking so workflow completion returns immediately.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| * Fire-and-forget — never throws, and the user-observable behaviour is | ||
| * identical with or without telemetry configured. | ||
| */ | ||
| async function trackMethod<T>(methodName: string, fn: () => Promise<T>): Promise<T> { |
There was a problem hiding this comment.
P2: Avoid duplicating sdk_method_call wrapper logic in client.ts; reuse the shared instrumentMethod helper to prevent drift in telemetry behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/client.ts, line 121:
<comment>Avoid duplicating `sdk_method_call` wrapper logic in `client.ts`; reuse the shared `instrumentMethod` helper to prevent drift in telemetry behavior.</comment>
<file context>
@@ -112,6 +113,38 @@ function isProcessRunning(pid: number): boolean {
+ * Fire-and-forget — never throws, and the user-observable behaviour is
+ * identical with or without telemetry configured.
+ */
+async function trackMethod<T>(methodName: string, fn: () => Promise<T>): Promise<T> {
+ const startedAt = Date.now();
+ try {
</file context>
|
|
||
| /** Extract a basename from a process command string. */ | ||
| function commandBasename(command: string): string { | ||
| const trimmed = command.trim().split(/\s+/)[0] ?? ''; |
There was a problem hiding this comment.
P2: commandBasename splits on whitespace too early, so quoted executable paths with spaces are misparsed and can be classified as unknown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/telemetry/src/harness.ts, line 97:
<comment>`commandBasename` splits on whitespace too early, so quoted executable paths with spaces are misparsed and can be classified as `unknown`.</comment>
<file context>
@@ -0,0 +1,218 @@
+
+/** Extract a basename from a process command string. */
+function commandBasename(command: string): string {
+ const trimmed = command.trim().split(/\s+/)[0] ?? '';
+ const stripped = trimmed.replace(/^["']|["']$/g, '');
+ // Posix and Windows-style separators.
</file context>
Comment on lines
607
to
615
| void import('./lib/relaycast-fetch-interceptor.js') | ||
| .then(({ installRelaycastFetchInterceptor }) => { | ||
| installRelaycastFetchInterceptor(); | ||
| }) | ||
| .catch(() => undefined); | ||
|
|
||
| startPatchedStdio(optionsFromEnv()).catch((error) => { | ||
| const message = error instanceof Error ? (error.stack ?? error.message) : String(error); | ||
| process.stderr.write(`${message}\n`); |
There was a problem hiding this comment.
P2: Interceptor installation is raced against startup, so early Relaycast requests may miss X-Relaycast-Harness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/relaycast-mcp.ts, line 607:
<comment>Interceptor installation is raced against startup, so early Relaycast requests may miss `X-Relaycast-Harness`.</comment>
<file context>
@@ -600,6 +600,16 @@ export function optionsFromEnv(): PatchedMcpServerOptions {
+ // Claude Code/Cursor/Zed via .mcp.json), the parent CLI's bootstrap
+ // hook never ran — install the harness header interceptor ourselves so
+ // outgoing relaycast calls still carry `X-Relaycast-Harness`.
+ void import('./lib/relaycast-fetch-interceptor.js')
+ .then(({ installRelaycastFetchInterceptor }) => {
+ installRelaycastFetchInterceptor();
</file context>
| export function trackSdkEvent(event: string, properties: Record<string, unknown> = {}): void { | ||
| init(); | ||
| if (!enabled) return; | ||
| const p = postEvent({ event, properties }).catch(() => undefined); |
There was a problem hiding this comment.
P2: Truncating inFlight drops unresolved events, so flushSdkTelemetry() may not wait for all outstanding telemetry posts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/telemetry.ts, line 242:
<comment>Truncating `inFlight` drops unresolved events, so `flushSdkTelemetry()` may not wait for all outstanding telemetry posts.</comment>
<file context>
@@ -0,0 +1,312 @@
+export function trackSdkEvent(event: string, properties: Record<string, unknown> = {}): void {
+ init();
+ if (!enabled) return;
+ const p = postEvent({ event, properties }).catch(() => undefined);
+ inFlight.push(p);
+ // Cap the queue so a long-lived process doesn't accumulate references.
</file context>
Comment on lines
+217
to
+218
| ...commonProps, | ||
| ...body.properties, |
There was a problem hiding this comment.
P2: Caller-supplied properties currently override required common telemetry fields; keep canonical SDK fields authoritative.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/telemetry.ts, line 217:
<comment>Caller-supplied properties currently override required common telemetry fields; keep canonical SDK fields authoritative.</comment>
<file context>
@@ -0,0 +1,312 @@
+ event: body.event,
+ distinct_id: distinctId,
+ properties: {
+ ...commonProps,
+ ...body.properties,
+ },
</file context>
Suggested change
| ...commonProps, | |
| ...body.properties, | |
| ...body.properties, | |
| ...commonProps, |
Comment on lines
+509
to
+516
| const { trackSdkEvent } = await import('../telemetry.js'); | ||
| trackSdkEvent('sdk_workflow_run', { | ||
| workflow_name: workflowName, | ||
| step_count: stepCount, | ||
| success, | ||
| duration_ms: Date.now() - startedAt, | ||
| ...(errorClass ? { error_class: errorClass } : {}), | ||
| }); |
There was a problem hiding this comment.
P2: run() now blocks on telemetry module import in finally; make telemetry dispatch non-blocking so workflow completion returns immediately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/workflows/builder.ts, line 509:
<comment>`run()` now blocks on telemetry module import in `finally`; make telemetry dispatch non-blocking so workflow completion returns immediately.</comment>
<file context>
@@ -469,6 +478,49 @@ export class WorkflowBuilder {
+ // Lazy import keeps the no-telemetry-key path zero-cost for users who
+ // never enable telemetry.
+ try {
+ const { trackSdkEvent } = await import('../telemetry.js');
+ trackSdkEvent('sdk_workflow_run', {
+ workflow_name: workflowName,
</file context>
Suggested change
| const { trackSdkEvent } = await import('../telemetry.js'); | |
| trackSdkEvent('sdk_workflow_run', { | |
| workflow_name: workflowName, | |
| step_count: stepCount, | |
| success, | |
| duration_ms: Date.now() - startedAt, | |
| ...(errorClass ? { error_class: errorClass } : {}), | |
| }); | |
| void import('../telemetry.js') | |
| .then(({ trackSdkEvent }) => { | |
| trackSdkEvent('sdk_workflow_run', { | |
| workflow_name: workflowName, | |
| step_count: stepCount, | |
| success, | |
| duration_ms: Date.now() - startedAt, | |
| ...(errorClass ? { error_class: errorClass } : {}), | |
| }); | |
| }) | |
| .catch(() => undefined); |
![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: 2417c8e267
ℹ️ 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".
Comment on lines
607
to
613
| void import('./lib/relaycast-fetch-interceptor.js') | ||
| .then(({ installRelaycastFetchInterceptor }) => { | ||
| installRelaycastFetchInterceptor(); | ||
| }) | ||
| .catch(() => undefined); | ||
|
|
||
| startPatchedStdio(optionsFromEnv()).catch((error) => { |
There was a problem hiding this comment.
The interceptor is loaded via a fire-and-forget dynamic import() and startPatchedStdio() is invoked immediately afterward, so startup requests can race ahead before installRelaycastFetchInterceptor() runs. In the isEntrypoint() path, resolvePatchedStdioBootstrapOptions() may call registerOrRotate right away, which means those early relaycast calls can miss X-Relaycast-Harness and lose the harness attribution this change is intended to guarantee.
Useful? React with 👍 / 👎.
`orchestrator_harness` was redundant — the SDK origin already carries `surface`/`client`/`version`, and "harness" alone unambiguously names the parent process driving the agent. Renaming now (before events land in PostHog with the longer name) keeps the schema honest. Renames across relay: - PostHog property: `orchestrator_harness` → `harness` - TS type: `OrchestratorHarness` → `Harness` - TS function: `detectOrchestratorHarness()` → `detectHarness()` - Rust function: `detect_orchestrator_harness()` → `detect_harness()` - Rust struct field: `orchestrator_harness` → `harness` - Env var: `AGENT_RELAY_ORCHESTRATOR_HARNESS` → `AGENT_RELAY_HARNESS` - Doc comments updated to match Constant name `HARNESS_ENV_VAR` and HTTP header `X-Relaycast-Harness` already used "harness" — unchanged. Paired with renames in relaycast#132 (server side) and relaycast#133 (SDK). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/cli/bootstrap.ts (1)
124-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't propagate
unknownto child processes.Writing
AGENT_RELAY_HARNESS=unknownhere prevents children from attempting their own detection, which contradicts the comment above and can permanently drop harness attribution for broker-spawned flows. Only export real harness slugs.Suggested fix
if (!process.env[HARNESS_ENV_VAR]) { - process.env[HARNESS_ENV_VAR] = detectHarness(); + const harness = detectHarness(); + if (harness !== 'unknown') { + process.env[HARNESS_ENV_VAR] = harness; + } }🤖 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 `@src/cli/bootstrap.ts` around lines 124 - 126, The current logic sets process.env[HARNESS_ENV_VAR] to detectHarness() even when the detector returns "unknown", which propagates an invalid harness to child processes; change the assignment so you only export a real slug: call detectHarness(), check that process.env[HARNESS_ENV_VAR] is unset and the returned harness !== "unknown", and only then set process.env[HARNESS_ENV_VAR] to that value (referencing HARNESS_ENV_VAR and detectHarness in src/cli/bootstrap.ts).
🤖 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.
Duplicate comments:
In `@src/cli/bootstrap.ts`:
- Around line 124-126: The current logic sets process.env[HARNESS_ENV_VAR] to
detectHarness() even when the detector returns "unknown", which propagates an
invalid harness to child processes; change the assignment so you only export a
real slug: call detectHarness(), check that process.env[HARNESS_ENV_VAR] is
unset and the returned harness !== "unknown", and only then set
process.env[HARNESS_ENV_VAR] to that value (referencing HARNESS_ENV_VAR and
detectHarness in src/cli/bootstrap.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d3a5373e-344b-4db0-9ece-bdb8998b8040
📒 Files selected for processing (9)
packages/sdk/src/telemetry.tspackages/telemetry/src/client.tspackages/telemetry/src/events.tspackages/telemetry/src/harness.test.tspackages/telemetry/src/harness.tspackages/telemetry/src/index.tssrc/cli/bootstrap.tssrc/cli/lib/relaycast-fetch-interceptor.tssrc/telemetry.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/telemetry/src/client.ts
- packages/telemetry/src/index.ts
- packages/telemetry/src/harness.test.ts
- src/cli/lib/relaycast-fetch-interceptor.ts
- packages/telemetry/src/events.ts
- packages/sdk/src/telemetry.ts
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/telemetry/src/events.ts">
<violation number="1" location="packages/telemetry/src/events.ts:83">
P2: Renaming the common property to `harness` breaks the `orchestrator_harness` telemetry contract and can silently drop this dimension for downstream consumers.</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
| * as `unknown` until we add them to the classifier. Always present so | ||
| * dashboards can group on it without coalescing. | ||
| */ | ||
| harness: string; |
There was a problem hiding this comment.
P2: Renaming the common property to harness breaks the orchestrator_harness telemetry contract and can silently drop this dimension for downstream consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/telemetry/src/events.ts, line 83:
<comment>Renaming the common property to `harness` breaks the `orchestrator_harness` telemetry contract and can silently drop this dimension for downstream consumers.</comment>
<file context>
@@ -74,13 +74,13 @@ export interface CommonProperties {
+ * dashboards can group on it without coalescing.
*/
- orchestrator_harness: string;
+ harness: string;
/**
* Coarse-grained surface that emitted the event: `cli`, `broker`, or
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
packages/telemetry/src/events.ts (1)
76-83: ⚡ Quick winTighten
orchestrator_harnessto the shared harness union.Using
stringhere allows arbitrary values and weakens the cross-package contract. Reuse the sharedHarnesstype so invalid slugs are caught at compile time.♻️ Proposed fix
+import type { Harness } from './harness.js'; + export interface CommonProperties { @@ - orchestrator_harness: string; + orchestrator_harness: Harness;🤖 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/telemetry/src/events.ts` around lines 76 - 83, The field orchestrator_harness is typed as a plain string which weakens the contract; change its type to the shared Harness union (replace orchestrator_harness: string with orchestrator_harness: Harness), add the appropriate import for Harness from the shared types module used across packages, and ensure any usages/initializers that currently assign arbitrary strings are updated to use a valid Harness member (or the canonical 'unknown' value defined by the Harness union).packages/sdk/src/client.ts (1)
431-437: ⚡ Quick winTrack validation failures in
spawnProvidertoo.The headless-provider guard throws before
trackMethod, so this failure path is missing fromsdk_method_calltelemetry.♻️ Proposed fix
async spawnProvider(input: SpawnProviderInput): Promise<{ name: string; runtime: AgentRuntime }> { - const transport = resolveSpawnTransport(input); - if (transport === 'headless' && !isHeadlessProvider(input.provider)) { - throw new Error( - `provider '${input.provider}' does not support headless transport (supported: claude, opencode)` - ); - } - return trackMethod('AgentRelayClient.spawnProvider', () => - this.transport.request('/api/spawn', { + { + const transport = resolveSpawnTransport(input); + if (transport === 'headless' && !isHeadlessProvider(input.provider)) { + throw new Error( + `provider '${input.provider}' does not support headless transport (supported: claude, opencode)` + ); + } + return this.transport.request('/api/spawn', { method: 'POST', body: JSON.stringify({ name: input.name, @@ }), - }) + }); + } ); }Also applies to: 438-460
🤖 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/src/client.ts` around lines 431 - 437, The headless-provider guard in spawnProvider (using resolveSpawnTransport and isHeadlessProvider) throws before telemetry is recorded so sdk_method_call misses these validation failures; update spawnProvider to ensure trackMethod is invoked for this path by either moving the transport/provider validation to after trackMethod is called or by wrapping the guard failure with a trackMethod call that records the failure (including error info) before throwing; touch the code around resolveSpawnTransport, isHeadlessProvider, and the existing trackMethod invocation so all validation errors are reported to telemetry.
🤖 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 @.trajectories/completed/2026-05/traj_0z98tkaigaxg.md:
- Line 69: The completion line currently duplicates the same info ("All 17 steps
completed in 29min. (completed in 29 minutes)"); replace it with a single,
non-redundant statement such as "All 17 steps completed in 29 minutes." by
removing the parenthetical duplicate so only one clear completion sentence
remains (look for the exact string "All 17 steps completed in 29min. (completed
in 29 minutes)").
In @.trajectories/completed/2026-05/traj_5q8i0iz4klpo.md:
- Line 69: Remove the duplicated completion time phrase in the markdown line
that currently reads "All 17 steps completed in 29min. (completed in 29
minutes)"; edit the text in the trajectory file so it contains a single,
consistent completion statement (e.g., "All 17 steps completed in 29 minutes.")
by removing the redundant parenthetical or the shorter "29min." form.
In @.trajectories/completed/2026-05/traj_8ljgydz61do5.md:
- Line 69: Remove the duplicated completion phrase by keeping a single clear
sentence — replace "All 17 steps completed in 35min. (completed in 35 minutes)"
with one consistent line, e.g., "All 17 steps completed in 35 minutes."; remove
the redundant fragment (either "All 17 steps completed in 35min." or "(completed
in 35 minutes)") so only one completion statement remains.
In @.trajectories/completed/2026-05/traj_elx0fcwgs37x.md:
- Line 69: The file contains a duplicated completion sentence ("All 17 steps
completed in 40min. (completed in 40 minutes)"); remove the redundant phrase so
only one clear statement remains (either "All 17 steps completed in 40min." or
"All 17 steps completed in 40 minutes") — update the line that contains that
exact text to a single, non-duplicated completion message.
In @.trajectories/completed/2026-05/traj_ootb5rt3tozd.md:
- Line 73: Remove the duplicated completion time phrase from the trajectory
summary by editing the line that currently reads "All 17 steps completed in
38min. 1 step(s) needed retries: implement-slice. (completed in 38 minutes)";
keep a single, non-redundant sentence (for example "All 17 steps completed in
38min. 1 step(s) needed retries: implement-slice.") and delete the repeated
"(completed in 38 minutes)" portion so the summary is not duplicated.
In @.trajectories/completed/2026-05/traj_t5uknesn2fcw.md:
- Line 69: Remove the duplicated completion text by replacing the string "All 17
steps completed in 33min. (completed in 33 minutes)" with a single clear
statement (e.g., "All 17 steps completed in 33min."), ensuring only one
completion message remains in the file
.trajectories/completed/2026-05/traj_t5uknesn2fcw.md and that the redundant
parenthetical "(completed in 33 minutes)" is deleted.
In @.trajectories/completed/2026-05/traj_u33qn99ijbh4.md:
- Line 69: The file contains a duplicated completion sentence "All 17 steps
completed in 24min. (completed in 24 minutes)"; edit the markdown (look for that
exact string in traj_u33qn99ijbh4.md) and remove the redundant phrase so only
one concise statement remains (e.g., keep "All 17 steps completed in 24
minutes."), then save and commit the change.
In @.trajectories/completed/2026-05/traj_v9x3o92ag682.md:
- Line 69: Remove the duplicated completion phrase by deleting the parenthetical
duplicate "(completed in 28 minutes)" from the line that currently reads "All 17
steps completed in 28min. (completed in 28 minutes)"; leave a single, consistent
statement such as "All 17 steps completed in 28min." (or replace with the
expanded form "All 17 steps completed in 28 minutes." if you prefer full words)
so the message appears only once.
---
Nitpick comments:
In `@packages/sdk/src/client.ts`:
- Around line 431-437: The headless-provider guard in spawnProvider (using
resolveSpawnTransport and isHeadlessProvider) throws before telemetry is
recorded so sdk_method_call misses these validation failures; update
spawnProvider to ensure trackMethod is invoked for this path by either moving
the transport/provider validation to after trackMethod is called or by wrapping
the guard failure with a trackMethod call that records the failure (including
error info) before throwing; touch the code around resolveSpawnTransport,
isHeadlessProvider, and the existing trackMethod invocation so all validation
errors are reported to telemetry.
In `@packages/telemetry/src/events.ts`:
- Around line 76-83: The field orchestrator_harness is typed as a plain string
which weakens the contract; change its type to the shared Harness union (replace
orchestrator_harness: string with orchestrator_harness: Harness), add the
appropriate import for Harness from the shared types module used across
packages, and ensure any usages/initializers that currently assign arbitrary
strings are updated to use a valid Harness member (or the canonical 'unknown'
value defined by the Harness union).
🪄 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: c8446114-d9e4-4db5-871d-d1eab6a6d700
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
.trajectories/compacted/release-6.2.3.json.trajectories/compacted/release-6.2.3.md.trajectories/completed/2026-05/traj_0z98tkaigaxg.md.trajectories/completed/2026-05/traj_1778873209642_c70e32ab.json.trajectories/completed/2026-05/traj_1778873209642_c70e32ab.md.trajectories/completed/2026-05/traj_1778873211616_6db3b2cd.json.trajectories/completed/2026-05/traj_1778873211616_6db3b2cd.md.trajectories/completed/2026-05/traj_5q8i0iz4klpo.md.trajectories/completed/2026-05/traj_8ljgydz61do5.md.trajectories/completed/2026-05/traj_citl6rhje2sc.json.trajectories/completed/2026-05/traj_citl6rhje2sc.md.trajectories/completed/2026-05/traj_elx0fcwgs37x.md.trajectories/completed/2026-05/traj_ij5b3kcatvwn.md.trajectories/completed/2026-05/traj_ootb5rt3tozd.md.trajectories/completed/2026-05/traj_t5uknesn2fcw.md.trajectories/completed/2026-05/traj_u33qn99ijbh4.md.trajectories/completed/2026-05/traj_v87cyrs8dke9.json.trajectories/completed/2026-05/traj_v87cyrs8dke9.md.trajectories/completed/2026-05/traj_v87cyrs8dke9.trace.json.trajectories/completed/2026-05/traj_v9x3o92ag682.md.trajectories/completed/2026-05/traj_wbn62q4cq16h.md.trajectories/completed/traj_1778873052429_03a4dacb.json.trajectories/completed/traj_1778873197540_01102ade.json.trajectories/completed/traj_1778873199489_f2ce4060.json.trajectories/completed/traj_1778873201502_0dacf7c5.json.trajectories/completed/traj_1778873203502_4c225b7e.json.trajectories/completed/traj_1778873205470_a4e5f0cb.json.trajectories/completed/traj_1778873207471_b7def991.json.trajectories/completed/traj_1778873209642_c70e32ab.json.trajectories/completed/traj_1778873211616_6db3b2cd.json.trajectories/completed/traj_1778874205797_81e92307.json.trajectories/completed/traj_1778874216773_c6b12ab2.json.trajectories/completed/traj_1778874218579_a0225559.json.trajectories/completed/traj_1778874224855_9c722c4b.json.trajectories/completed/traj_1778874226983_3367d527.json.trajectories/completed/traj_1778874229373_9cce9465.json.trajectories/completed/traj_1778874240339_51b823cd.json.trajectories/completed/traj_1778874241076_caa675a9.json.trajectories/completed/traj_1778874248966_e29c4c54.json.trajectories/completed/traj_1778874249983_12a98df3.json.trajectories/completed/traj_1778874258229_0bdc53d8.json.trajectories/completed/traj_1778874261453_55f49624.json.trajectories/completed/traj_1778874261608_48fb9bf5.json.trajectories/completed/traj_1778874269139_d7d7485a.json.trajectories/completed/traj_1778874274412_70843e0e.json.trajectories/completed/traj_1778874274581_71efa470.json.trajectories/completed/traj_1778874282200_39ad11db.json.trajectories/completed/traj_1778874283570_ce3585b8.json.trajectories/completed/traj_1778874289674_e3f868c8.json.trajectories/completed/traj_1778874291950_0b1b5c1f.json.trajectories/completed/traj_1778874295927_4083d181.json.trajectories/completed/traj_1778874296362_bdf727ff.json.trajectories/index.jsonCHANGELOG.mdcrates/broker/Cargo.tomlcrates/broker/src/telemetry.rspackages/sdk/src/client.tspackages/sdk/src/index.tspackages/sdk/src/telemetry.tspackages/sdk/src/workflows/builder.tspackages/telemetry/src/client.tspackages/telemetry/src/events.tspackages/telemetry/src/harness.test.tspackages/telemetry/src/harness.tssrc/cli/bootstrap.tssrc/cli/commands/passthrough.tssrc/cli/lib/attach.ts
✅ Files skipped from review due to trivial changes (47)
- crates/broker/Cargo.toml
- .trajectories/completed/2026-05/traj_citl6rhje2sc.md
- .trajectories/completed/traj_1778874218579_a0225559.json
- .trajectories/completed/traj_1778874289674_e3f868c8.json
- .trajectories/completed/traj_1778874269139_d7d7485a.json
- .trajectories/completed/traj_1778873201502_0dacf7c5.json
- .trajectories/completed/traj_1778873207471_b7def991.json
- .trajectories/completed/2026-05/traj_citl6rhje2sc.json
- .trajectories/completed/traj_1778874283570_ce3585b8.json
- .trajectories/completed/traj_1778874274412_70843e0e.json
- .trajectories/completed/traj_1778874224855_9c722c4b.json
- .trajectories/completed/traj_1778874291950_0b1b5c1f.json
- .trajectories/completed/traj_1778873209642_c70e32ab.json
- .trajectories/completed/traj_1778874282200_39ad11db.json
- .trajectories/completed/traj_1778874240339_51b823cd.json
- .trajectories/completed/traj_1778874229373_9cce9465.json
- .trajectories/completed/traj_1778874248966_e29c4c54.json
- src/cli/lib/attach.ts
- .trajectories/completed/2026-05/traj_wbn62q4cq16h.md
- .trajectories/completed/traj_1778874226983_3367d527.json
- .trajectories/completed/traj_1778873205470_a4e5f0cb.json
- .trajectories/completed/traj_1778874205797_81e92307.json
- .trajectories/completed/traj_1778873199489_f2ce4060.json
- .trajectories/completed/traj_1778874241076_caa675a9.json
- src/cli/commands/passthrough.ts
- .trajectories/index.json
- .trajectories/completed/traj_1778873203502_4c225b7e.json
- .trajectories/completed/2026-05/traj_ij5b3kcatvwn.md
- .trajectories/completed/traj_1778874249983_12a98df3.json
- .trajectories/completed/2026-05/traj_1778873209642_c70e32ab.json
- .trajectories/completed/traj_1778874296362_bdf727ff.json
- .trajectories/completed/traj_1778874216773_c6b12ab2.json
- .trajectories/completed/traj_1778873197540_01102ade.json
- .trajectories/completed/traj_1778874258229_0bdc53d8.json
- .trajectories/completed/traj_1778874261453_55f49624.json
- .trajectories/completed/2026-05/traj_1778873211616_6db3b2cd.md
- .trajectories/completed/2026-05/traj_1778873209642_c70e32ab.md
- .trajectories/completed/2026-05/traj_v87cyrs8dke9.trace.json
- .trajectories/compacted/release-6.2.3.json
- .trajectories/completed/traj_1778874295927_4083d181.json
- .trajectories/completed/traj_1778874274581_71efa470.json
- .trajectories/completed/traj_1778873052429_03a4dacb.json
- .trajectories/completed/traj_1778873211616_6db3b2cd.json
- .trajectories/completed/2026-05/traj_1778873211616_6db3b2cd.json
- .trajectories/completed/traj_1778874261608_48fb9bf5.json
- CHANGELOG.md
- .trajectories/completed/2026-05/traj_v87cyrs8dke9.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/sdk/src/index.ts
- src/cli/bootstrap.ts
- packages/telemetry/src/harness.test.ts
- packages/sdk/src/workflows/builder.ts
- packages/telemetry/src/client.ts
- packages/sdk/src/telemetry.ts
- packages/telemetry/src/harness.ts
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 29min. (completed in 29 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 29min. (completed in 29 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 29min. (completed in 29 minutes)
+- All 17 steps completed in 29min.📝 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
| - All 17 steps completed in 29min. (completed in 29 minutes) | |
| - All 17 steps completed in 29min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 29min. (completed in 29 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_0z98tkaigaxg.md at line 69, The
completion line currently duplicates the same info ("All 17 steps completed in
29min. (completed in 29 minutes)"); replace it with a single, non-redundant
statement such as "All 17 steps completed in 29 minutes." by removing the
parenthetical duplicate so only one clear completion sentence remains (look for
the exact string "All 17 steps completed in 29min. (completed in 29 minutes)").
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 29min. (completed in 29 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 29min. (completed in 29 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 29min. (completed in 29 minutes)
+- All 17 steps completed in 29min.📝 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
| - All 17 steps completed in 29min. (completed in 29 minutes) | |
| - All 17 steps completed in 29min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 29min. (completed in 29 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_5q8i0iz4klpo.md at line 69, Remove the
duplicated completion time phrase in the markdown line that currently reads "All
17 steps completed in 29min. (completed in 29 minutes)"; edit the text in the
trajectory file so it contains a single, consistent completion statement (e.g.,
"All 17 steps completed in 29 minutes.") by removing the redundant parenthetical
or the shorter "29min." form.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 35min. (completed in 35 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 35min. (completed in 35 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 35min. (completed in 35 minutes)
+- All 17 steps completed in 35min.📝 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
| - All 17 steps completed in 35min. (completed in 35 minutes) | |
| - All 17 steps completed in 35min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 35min. (completed in 35 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_8ljgydz61do5.md at line 69, Remove the
duplicated completion phrase by keeping a single clear sentence — replace "All
17 steps completed in 35min. (completed in 35 minutes)" with one consistent
line, e.g., "All 17 steps completed in 35 minutes."; remove the redundant
fragment (either "All 17 steps completed in 35min." or "(completed in 35
minutes)") so only one completion statement remains.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 40min. (completed in 40 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 40min. (completed in 40 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 40min. (completed in 40 minutes)
+- All 17 steps completed in 40min.📝 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
| - All 17 steps completed in 40min. (completed in 40 minutes) | |
| - All 17 steps completed in 40min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 40min. (completed in 40 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_elx0fcwgs37x.md at line 69, The file
contains a duplicated completion sentence ("All 17 steps completed in 40min.
(completed in 40 minutes)"); remove the redundant phrase so only one clear
statement remains (either "All 17 steps completed in 40min." or "All 17 steps
completed in 40 minutes") — update the line that contains that exact text to a
single, non-duplicated completion message.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice. (completed in 38 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice. (completed in 38 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice. (completed in 38 minutes)
+- All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice.📝 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
| - All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice. (completed in 38 minutes) | |
| - All 17 steps completed in 38min. 1 step(s) needed retries: implement-slice. |
🧰 Tools
🪛 LanguageTool
[grammar] ~73-~73: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 38min. 1 step(s) needed retries: implement-sl...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_ootb5rt3tozd.md at line 73, Remove the
duplicated completion time phrase from the trajectory summary by editing the
line that currently reads "All 17 steps completed in 38min. 1 step(s) needed
retries: implement-slice. (completed in 38 minutes)"; keep a single,
non-redundant sentence (for example "All 17 steps completed in 38min. 1 step(s)
needed retries: implement-slice.") and delete the repeated "(completed in 38
minutes)" portion so the summary is not duplicated.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 33min. (completed in 33 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 33min. (completed in 33 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 33min. (completed in 33 minutes)
+- All 17 steps completed in 33min.📝 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
| - All 17 steps completed in 33min. (completed in 33 minutes) | |
| - All 17 steps completed in 33min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 33min. (completed in 33 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_t5uknesn2fcw.md at line 69, Remove the
duplicated completion text by replacing the string "All 17 steps completed in
33min. (completed in 33 minutes)" with a single clear statement (e.g., "All 17
steps completed in 33min."), ensuring only one completion message remains in the
file .trajectories/completed/2026-05/traj_t5uknesn2fcw.md and that the redundant
parenthetical "(completed in 33 minutes)" is deleted.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 24min. (completed in 24 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 24min. (completed in 24 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 24min. (completed in 24 minutes)
+- All 17 steps completed in 24min.📝 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
| - All 17 steps completed in 24min. (completed in 24 minutes) | |
| - All 17 steps completed in 24min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 24min. (completed in 24 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_u33qn99ijbh4.md at line 69, The file
contains a duplicated completion sentence "All 17 steps completed in 24min.
(completed in 24 minutes)"; edit the markdown (look for that exact string in
traj_u33qn99ijbh4.md) and remove the redundant phrase so only one concise
statement remains (e.g., keep "All 17 steps completed in 24 minutes."), then
save and commit the change.
|
|
||
| _Agent: orchestrator_ | ||
|
|
||
| - All 17 steps completed in 28min. (completed in 28 minutes) |
Contributor
There was a problem hiding this comment.
Remove redundant completion time statement.
The completion statement is duplicated with slightly different wording: "All 17 steps completed in 28min. (completed in 28 minutes)".
✂️ Proposed fix
-- All 17 steps completed in 28min. (completed in 28 minutes)
+- All 17 steps completed in 28min.📝 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
| - All 17 steps completed in 28min. (completed in 28 minutes) | |
| - All 17 steps completed in 28min. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: Ensure spelling is correct
Context: ...hestrator_ - All 17 steps completed in 28min. (completed in 28 minutes)
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 @.trajectories/completed/2026-05/traj_v9x3o92ag682.md at line 69, Remove the
duplicated completion phrase by deleting the parenthetical duplicate "(completed
in 28 minutes)" from the line that currently reads "All 17 steps completed in
28min. (completed in 28 minutes)"; leave a single, consistent statement such as
"All 17 steps completed in 28min." (or replace with the expanded form "All 17
steps completed in 28 minutes." if you prefer full words) so the message appears
only once.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.trajectories/index.json:
- Around line 1079-1091: In .trajectories/index.json the entries for
traj_citl6rhje2sc and traj_uoh2y4raguxp use machine-local .claude/worktrees/...
absolute paths; update each entry’s "path" value to the tracked repo-relative
location under .trajectories/completed/... (e.g.,
.trajectories/completed/2026-05/<same-filename>.json) so the index points to
repository files instead of local worktree files; ensure the filenames remain
unchanged and only the path prefix is replaced.
In `@crates/broker/src/telemetry.rs`:
- Around line 618-619: The telemetry emission is using the old key
"orchestrator_harness" which breaks the cross-surface schema; change the JSON
insertion to emit "harness" instead by replacing the obj.insert call that
currently uses "orchestrator_harness" with obj.insert("harness".to_string(),
json!(self.harness)); (the code around the obj.insert call that references
self.harness and the surrounding telemetry serialization should be updated so
the key is "harness").
In `@packages/telemetry/src/harness.ts`:
- Around line 57-60: The second Claude helper pattern in HARNESS_PATTERNS
(mapping to harness 'claude-code') is case-sensitive and should be made
case-insensitive to match the Rust broker classifier; update the RegExp for the
entry with re: /^Claude(?:\s+Helper)?$/ to include the /i flag so it becomes
case-insensitive while keeping the same pattern and harness name.
🪄 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: 7c48cc71-b4c1-47ed-b367-01afa3a30ca0
📒 Files selected for processing (7)
.trajectories/completed/2026-05/traj_uoh2y4raguxp.json.trajectories/completed/2026-05/traj_uoh2y4raguxp.md.trajectories/index.jsoncrates/broker/src/telemetry.rspackages/sdk/src/telemetry.tspackages/telemetry/src/harness.test.tspackages/telemetry/src/harness.ts
✅ Files skipped from review due to trivial changes (2)
- .trajectories/completed/2026-05/traj_uoh2y4raguxp.md
- .trajectories/completed/2026-05/traj_uoh2y4raguxp.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/src/telemetry.ts
Comment on lines
+1079
to
+1091
| "traj_citl6rhje2sc": { | ||
| "title": "Review and fix PR 888 comments and conflicts", | ||
| "status": "completed", | ||
| "startedAt": "2026-05-19T16:54:14.672Z", | ||
| "completedAt": "2026-05-19T16:54:23.471Z", | ||
| "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_citl6rhje2sc.json" | ||
| }, | ||
| "traj_uoh2y4raguxp": { | ||
| "title": "Relax harness slug validation", | ||
| "status": "completed", | ||
| "startedAt": "2026-05-19T17:08:28.774Z", | ||
| "completedAt": "2026-05-19T17:12:38.005Z", | ||
| "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_uoh2y4raguxp.json" |
Contributor
There was a problem hiding this comment.
Store repo-relative trajectory paths here.
These new entries point at machine-local .claude/worktrees/... paths, so the index will reference files that do not exist on other checkouts or after that worktree is removed. Please point them at the tracked repo paths under .trajectories/completed/... instead.
🛠️ Proposed fix
- "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_citl6rhje2sc.json"
+ "path": ".trajectories/completed/2026-05/traj_citl6rhje2sc.json"
...
- "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_uoh2y4raguxp.json"
+ "path": ".trajectories/completed/2026-05/traj_uoh2y4raguxp.json"📝 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
| "traj_citl6rhje2sc": { | |
| "title": "Review and fix PR 888 comments and conflicts", | |
| "status": "completed", | |
| "startedAt": "2026-05-19T16:54:14.672Z", | |
| "completedAt": "2026-05-19T16:54:23.471Z", | |
| "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_citl6rhje2sc.json" | |
| }, | |
| "traj_uoh2y4raguxp": { | |
| "title": "Relax harness slug validation", | |
| "status": "completed", | |
| "startedAt": "2026-05-19T17:08:28.774Z", | |
| "completedAt": "2026-05-19T17:12:38.005Z", | |
| "path": "/Users/will/Projects/AgentWorkforce/relay/.claude/worktrees/agent-a90562b7ed038f1a0/.trajectories/completed/2026-05/traj_uoh2y4raguxp.json" | |
| "traj_citl6rhje2sc": { | |
| "title": "Review and fix PR 888 comments and conflicts", | |
| "status": "completed", | |
| "startedAt": "2026-05-19T16:54:14.672Z", | |
| "completedAt": "2026-05-19T16:54:23.471Z", | |
| "path": ".trajectories/completed/2026-05/traj_citl6rhje2sc.json" | |
| }, | |
| "traj_uoh2y4raguxp": { | |
| "title": "Relax harness slug validation", | |
| "status": "completed", | |
| "startedAt": "2026-05-19T17:08:28.774Z", | |
| "completedAt": "2026-05-19T17:12:38.005Z", | |
| "path": ".trajectories/completed/2026-05/traj_uoh2y4raguxp.json" |
🤖 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 @.trajectories/index.json around lines 1079 - 1091, In
.trajectories/index.json the entries for traj_citl6rhje2sc and traj_uoh2y4raguxp
use machine-local .claude/worktrees/... absolute paths; update each entry’s
"path" value to the tracked repo-relative location under
.trajectories/completed/... (e.g.,
.trajectories/completed/2026-05/<same-filename>.json) so the index points to
repository files instead of local worktree files; ensure the filenames remain
unchanged and only the path prefix is replaced.
Comment on lines
+618
to
+619
| obj.insert("orchestrator_harness".to_string(), json!(self.harness)); | ||
| obj.insert("surface".to_string(), json!("broker")); |
Contributor
There was a problem hiding this comment.
Emit harness, not orchestrator_harness.
Line 618 is still serializing the old key, so broker events won't match the renamed cross-surface schema and will fragment dashboards/queries from the CLI and SDK.
🛠️ Proposed fix
- obj.insert("orchestrator_harness".to_string(), json!(self.harness));
+ obj.insert("harness".to_string(), json!(self.harness));
obj.insert("surface".to_string(), json!("broker"));📝 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
| obj.insert("orchestrator_harness".to_string(), json!(self.harness)); | |
| obj.insert("surface".to_string(), json!("broker")); | |
| obj.insert("harness".to_string(), json!(self.harness)); | |
| obj.insert("surface".to_string(), json!("broker")); |
🤖 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 `@crates/broker/src/telemetry.rs` around lines 618 - 619, The telemetry
emission is using the old key "orchestrator_harness" which breaks the
cross-surface schema; change the JSON insertion to emit "harness" instead by
replacing the obj.insert call that currently uses "orchestrator_harness" with
obj.insert("harness".to_string(), json!(self.harness)); (the code around the
obj.insert call that references self.harness and the surrounding telemetry
serialization should be updated so the key is "harness").
Comment on lines
+57
to
+60
| const HARNESS_PATTERNS: Array<{ harness: ClassifiedHarness; re: RegExp }> = [ | ||
| // Claude Code ships as `claude` on PATH plus a desktop "Claude" app on macOS. | ||
| { harness: 'claude-code', re: /^claude(?:-code)?(?:\.exe)?$/i }, | ||
| { harness: 'claude-code', re: /^Claude(?:\s+Helper)?$/ }, |
Contributor
There was a problem hiding this comment.
Make the Claude helper matcher case-insensitive.
claude helper currently falls through to null because this is the only Claude-helper pattern and it lacks /i. That makes the TS detector diverge from the Rust broker classifier for the same process name.
🛠️ Proposed fix
- { harness: 'claude-code', re: /^Claude(?:\s+Helper)?$/ },
+ { harness: 'claude-code', re: /^claude(?:\s+helper)?(?:\.exe)?$/i },🤖 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/telemetry/src/harness.ts` around lines 57 - 60, The second Claude
helper pattern in HARNESS_PATTERNS (mapping to harness 'claude-code') is
case-sensitive and should be made case-insensitive to match the Rust broker
classifier; update the RegExp for the entry with re: /^Claude(?:\s+Helper)?$/ to
include the /i flag so it becomes case-insensitive while keeping the same
pattern and harness name.
Member
Author
|
Addressed the latest round of feedback on branch Code changes:
Trajectory housekeeping:
Already addressed on
Verification:
CI status on Generated by Claude Code |
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
2 participants
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.
P0 of AgentWorkforce/relay#881. Pairs with AgentWorkforce/relaycast#132 which reads the
X-Relaycast-Harnessheader and stampsharnesson every server-side event.Sub-tasks
1. Harness detection in the broker
Detect the parent harness driving the CLI/broker — Claude Code vs Cursor vs Codex vs Gemini vs Aider vs Cline vs Continue vs Windsurf vs Zed — so PostHog can split usage by orchestrator.
Implementation: Option A (TS detects, passes via env to broker). The TS CLI walks its own parent process tree once on startup, sets
AGENT_RELAY_HARNESS=<slug>on the env before spawning the broker. The Rust broker reads the env hint; if unset (e.g. user code spawns the broker directly via the SDK), the broker walksgetppid()itself. Both sides cache the result.Files
packages/telemetry/src/harness.ts(new) — TS classifier +/proc+pswalkers, cached. ExportsdetectHarness()and theHarnessunion.packages/telemetry/src/harness.test.ts(new) — 26 unit tests covering classifier, basename extraction, env hint precedence, caching.src/telemetry.rs— addsharnessfield toTelemetryClient,detect_harness()(env hint →/procon Linux,pson macOS,unknownfallback), 3 new unit tests.packages/telemetry/src/events.ts— addsharness: stringtoCommonProperties(always present so dashboards can group on it without coalescing) plus optionalsurface: 'cli' | 'broker' | 'sdk'.packages/telemetry/src/client.ts— populatesharnessin common props.src/cli/bootstrap.ts—propagateVersionsToChildren()now also exportsAGENT_RELAY_HARNESS.2. Outgoing
X-Relaycast-HarnessheaderThe
@relaycast/sdkHTTP client builds its own header set internally and doesn't expose a hook for arbitrary headers. Rather than fork the SDK or duplicate every API surface, the CLI patchesglobalThis.fetchto stamp the header on requests torelaycast.devhosts.Files
src/cli/lib/relaycast-fetch-interceptor.ts(new) — host-scoped, additive, idempotent patch. Caller-provided headers win.src/cli/lib/relaycast-fetch-interceptor.test.ts(new) — 5 tests covering header presence, host filtering, override preservation, idempotency, andRequestobject handling.src/cli/bootstrap.ts— installs the interceptor fromrunCli().src/cli/relaycast-mcp.ts— also installs from the stdio entrypoint so the MCP shim picks it up when Claude Code/Cursor/Zed launch it directly via.mcp.json.3.
cli_installandcli_updateeventscli_installfires once per machine on the first run after install, detected via the absence of a stored prefs file. Wired intoinitTelemetry()so it carries the full set of common properties (includingharness).cli_updatefires fromrunUpdateCommandafternpm install -g agent-relay@latestreturns, withfrom_version/to_version/success/ optionalerror_class.Files
packages/telemetry/src/events.ts—CliInstallEvent,CliUpdateEventtype defs (Tier 4).packages/telemetry/src/client.ts—firstRunDetectedflag +track('cli_install', ...)at the end of init.src/cli/lib/core-maintenance.ts—track('cli_update', ...)on both success and failure paths.4. Minimal SDK telemetry
The SDK is a published library consumed by user code. The implementation requirements were strict: no heavy runtime deps (no
posthog-node), no-op when no key, respect opt-out env vars, never block user workflows. I inlined a tinyfetch-based client rather than depend on@agent-relay/telemetry(which would pull in posthog-node transitively).Files
packages/sdk/src/telemetry.ts(new) — tiny fire-and-forget client,AbortSignal.timeout(5_000)so a slow PostHog never blocks user code. ReadsAGENT_RELAY_POSTHOG_KEY(coexists with chore(telemetry): inject PostHog key at build time (P0.5 of #881) #883's build-time injection contract). Uses the same machine-id derived anonymous ID as the CLI.packages/sdk/src/workflows/builder.ts— wrapsWorkflowBuilder.run()withsdk_workflow_run(workflow_name, step_count, success, duration_ms, error_class).packages/sdk/src/client.ts— wrapsspawnPty,spawnProvider,release,listAgents,sendMessagewithsdk_method_call.packages/sdk/src/index.ts— exportstrackSdkEvent,flushSdkTelemetryfor user code that wants to flush before exit.All SDK events carry
surface: "sdk"so dashboards can split CLI/broker/SDK with one filter.Harness enum contract
Canonical slugs (kept in sync between
packages/telemetry/src/harness.ts,src/telemetry.rs, and the relaycast server-side parser in #132):Slugs are lower-kebab-case, ASCII-only, ≤11 chars — round-trip cleanly through the relaycast sanitiser (lowercase / ASCII / 40-char cap) without coercion.
unknownis the expected long-tail baseline; watching what shows up asunknownin PostHog tells us which harnesses to add to the classifier next.Rename note (commit
7fffdd9)Originally landed as
orchestrator_harness/OrchestratorHarness/AGENT_RELAY_ORCHESTRATOR_HARNESS. Renamed across all three PRs (relay#888 + relaycast#132 + relaycast#133) to drop the redundantorchestrator_prefix —harnessalone is unambiguous, the SDK origin already carriessurface/client/version, andX-Relaycast-Harnessheader was always the short form. Renaming now keeps events from landing in PostHog with the longer name.Coexistence with #883 (P0.5)
option_env!("AGENT_RELAY_POSTHOG_KEY")build-time injection from chore(telemetry): inject PostHog key at build time (P0.5 of #881) #883. No reverts.getPostHogConfig()— unchanged.AGENT_RELAY_POSTHOG_KEYenv var. Forks / local dev / CI without the key continue to no-op silently.Test plan
Verified on macOS 25.4.0 (worktree-local, ARM64). Linux path covered by
/procreader (compile-tested viacargo build --release).cargo build --release— cleancargo test --lib telemetry— 15 passed (3 new harness tests)cargo clippy --all-targets -- -D warnings— cleancargo fmt --check— cleannpm --prefix packages/telemetry run build— cleannpm --prefix packages/sdk run check— cleannpx tsc --noEmit(root) — cleannpx vitest run packages/telemetry src/cli/lib/relaycast-fetch-interceptor.test.ts— 31 passed (26 harness tests + 5 interceptor tests)npx vitest run src/cli/— 264 passed, 5 skipped, 0 regressionsAGENT_RELAY_POSTHOG_KEYunset → telemetry no-ops (Rust:posthog_api_key_treats_empty_as_unset; SDK:init()returns early)orchestrator_harness→harnessrename — same counts, no regressions.Manual contract check: longest slug is
claude-code(11 chars), all ASCII lowercase — well under the relaycast sanitiser's 40-char cap and shape requirements.Out of scope (deferred to P1+)
distinct_idacross client/server🤖 Generated with Claude Code