Skip to content

fix(cli): capture orchestrator harness for the detached broker#1075

Closed
willwashburn wants to merge 2 commits into
mainfrom
fix/broker-harness-env
Closed

fix(cli): capture orchestrator harness for the detached broker#1075
willwashburn wants to merge 2 commits into
mainfrom
fix/broker-harness-env

Conversation

@willwashburn

Copy link
Copy Markdown
Member

Problem

relay#1069 made the broker forward its detected harness to the relaycast backend, but server telemetry (PostHog 296966) still records harness: "unknown" for ~100% of Rust-broker traffic — verified on v8.3.2/v8.3.3 brokers (@relaycast/sdk-rust 2.3.0).

Root cause: the broker is launched detached (its own session via setsidbroker-lifecycle.ts spawns it with detached: true). By the time the broker runs detect_orchestrator_harness, its parent is the init process, so the parent-PID walk finds no claude-code / codex / cursor ancestor. #1069's forwarding works correctly — it just forwards "unknown".

The asymmetry this explains:

Path Runs… Detection Result
JS / MCP (@relaycast/sdk) in-process under the harness parent walk sees the harness ✓ claude-code, codex
Rust broker daemon detached, own session parent walk sees launchd/init ✗ unknown

Fix

The agent-relay CLI is still in-process under the harness when up is invoked. Detect the harness there and stamp AGENT_RELAY_ORCHESTRATOR_HARNESS into the env handed to the broker — the established pattern for passing values to the broker (cf. AGENT_RELAY_STATE_DIR). The detached broker checks env before walking the process tree (detect_orchestrator_harness in crates/broker/src/telemetry.rs), so it now forwards the real harness, and the agents it spawns inherit it too.

  • New ensureOrchestratorHarnessEnv(env, detect?) helper, called at the top of runUpCommand so both the background (detached re-invoke) and foreground paths inherit it.
  • An explicit value already in env wins — including the re-invoked detached up inheriting ours, so detection runs once and isn't clobbered.
  • detect is injectable → unit-tested without depending on the real process tree.

Tests

  • broker-lifecycle.test.ts: +4 cases (stamps detected harness; ignores unknown; respects explicit value; threads env to the real detector via RELAYCAST_HARNESS). Suite green (17 tests).
  • CLI tsc --noEmit clean; eslint clean (the 6 pre-existing warnings are unrelated, in refreshDashboardAssetsIfStale).

Rollout

Ships in the next agent-relay release. After adoption, sdk-rust rows in 296966 should flip from unknown to the real harness. This is the second half of the harness fix (forwarding plumbing #1069 + this detection capture).

🤖 Generated with Claude Code

relay#1069 made the broker forward its detected harness to the relaycast
backend, but server telemetry still records harness="unknown" for 100% of
Rust-broker traffic. Root cause: the broker is launched detached (its own
session via setsid), so by the time it runs `detect_orchestrator_harness`
its parent is the init process and the process-tree walk finds no
claude-code / codex / cursor ancestor. The forwarding works — it just
forwards "unknown".

The `agent-relay` CLI, however, is still in-process under the harness when
`up` runs. Detect the harness there and stamp
`AGENT_RELAY_ORCHESTRATOR_HARNESS` into the env passed to the broker (the
established pattern for handing values to the broker — see
AGENT_RELAY_STATE_DIR). The detached broker reads env before walking the
process tree, so it now forwards the real harness as the
`X-Relaycast-Harness` header / `?harness=` query, and the agents it spawns
inherit it too. An explicit value already in the env wins — including the
re-invoked detached `up` inheriting ours, so detection runs once.

Extracted as `ensureOrchestratorHarnessEnv(env, detect?)` with an injectable
detector so the behavior is unit-tested without depending on the real
process tree (+4 tests). CLI tsc + eslint clean; broker-lifecycle suite
green (17 tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@willwashburn willwashburn requested a review from khaliqgant as a code owner June 10, 2026 03:54
@gemini-code-assist

Copy link
Copy Markdown

Warning

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

@codeant-ai

codeant-ai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7fd5d089-8913-497a-b53b-819fd5a34436

📥 Commits

Reviewing files that changed from the base of the PR and between 852ec02 and 7b91404.

📒 Files selected for processing (1)
  • packages/cli/src/cli/lib/broker-lifecycle.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/cli/src/cli/lib/broker-lifecycle.ts

📝 Walkthrough

Walkthrough

Adds ensureOrchestratorHarnessEnv to detect and stamp the orchestrator harness into process env; runUpCommand invokes it at startup so the detached broker inherits the harness. Tests cover stamping, unknown handling, preserving existing env, and detector access to env-derived keys.

Changes

Orchestrator Harness Propagation

Layer / File(s) Summary
ensureOrchestratorHarnessEnv implementation and tests
packages/cli/src/cli/lib/broker-lifecycle.ts, packages/cli/src/cli/lib/broker-lifecycle.test.ts
New exported ensureOrchestratorHarnessEnv(env, detect?) reads env, runs detectOrchestratorHarness, and sets env[ORCHESTRATOR_HARNESS_ENV] only when absent and detection != UNKNOWN_ORCHESTRATOR_HARNESS. Tests verify stamping, skipping unknown, preserving existing values, and detector env access.
Broker startup integration
packages/cli/src/cli/lib/broker-lifecycle.ts
runUpCommand now calls ensureOrchestratorHarnessEnv(deps.env) during startup so the child broker process inherits the orchestrator harness env value.

Sequence Diagram

sequenceDiagram
  participant runUpCommand
  participant ensureFunc as ensureOrchestratorHarnessEnv
  participant detectFunc as detectOrchestratorHarness
  participant env as ProcessEnv
  runUpCommand->>ensureFunc: ensureOrchestratorHarnessEnv(deps.env)
  ensureFunc->>detectFunc: detect(env)
  detectFunc-->>ensureFunc: orchestrator harness
  alt harness !== UNKNOWN and env key not set
    ensureFunc->>env: env[ORCHESTRATOR_HARNESS_ENV] = harness
  else harness === UNKNOWN or env key already set
    ensureFunc-->>env: no change
  end
  ensureFunc-->>runUpCommand: return
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • khaliqgant

Poem

🐰 A tiny hare hops through the start,
Stamps the harness in the env with art,
If unknown it stays discreet,
If set, it keeps that beat—
Broker wakes with harness in its heart.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding harness detection capture for the detached broker process.
Description check ✅ Passed The description includes comprehensive Problem, Fix, and Tests sections with detailed context, but is missing explicit summary and test plan checklist from the template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/broker-harness-env

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

❤️ Share

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

@codeant-ai

codeant-ai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@willwashburn

Copy link
Copy Markdown
Member Author

Closing as redundant. bootstrap.ts (runClipropagateTelemetryContextToChildren) already detects the orchestrator harness in the non-detached outer CLI and sets process.env.AGENT_RELAY_ORCHESTRATOR_HARNESS before any command runs. Since deps.env === process.env (core.ts:277) and the broker is spawned with env: deps.env, the broker already inherits it — the guard in ensureOrchestratorHarnessEnv short-circuits, so this is a no-op. The unknown traffic is genuinely harness-less sessions, not a detection gap. Pursuing per-worker harness attribution instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant