fix(runtime): wire Relay MCP into cloud personas#205
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
CodeAnt AI is reviewing your PR. |
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. Warning Review limit reached
More reviews will be available in 25 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extends the cloud harness runner to read Relay MCP config from environment and refactors spec construction so non-Codex harnesses get relayMcp injected directly, while Codex harnesses spawn an agent-relay-broker to obtain MCP args that are injected into the harness spawn command; includes helpers and an integration test. ChangesRelay MCP Configuration for Cloud Harness Runner
Sequence DiagramsequenceDiagram
participant HarnessRunner as Harness Runner
participant RelayResolver as Relay Resolver
participant BrokerResolution as Broker Resolution
participant AgentRelayBroker as Agent Relay Broker
participant SpawnHandler as Spawn Handler
HarnessRunner->>RelayResolver: resolveRelayMcpFromEnv(env)
RelayResolver-->>HarnessRunner: RelayMcpConfig
alt Codex Harness
HarnessRunner->>BrokerResolution: resolveCodexAgentRelayMcpArgs(specInput, env)
BrokerResolution->>AgentRelayBroker: spawn broker binary with broker args
AgentRelayBroker-->>BrokerResolution: JSON output { args, sideEffectFiles, agentToken }
BrokerResolution->>HarnessRunner: injectCodexSubcommandArgs(spawnArgs, returnedArgs)
HarnessRunner->>SpawnHandler: spawn harness process with injected spawnArgs
else Non-Codex Harness
HarnessRunner->>SpawnHandler: inject relayMcp into spec.args
HarnessRunner->>SpawnHandler: spawn harness process with spec.args
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| }); | ||
| const task = run.prompt; | ||
| const spec = buildNonInteractiveSpec({ | ||
| const relayMcp = resolveRelayMcpFromEnv(args.env); |
There was a problem hiding this comment.
Suggestion: Relay MCP detection is using only the static launcher environment, so per-run env overrides on ctx.harness.run() are ignored. If a caller supplies RELAY_API_KEY / RELAY_AGENT_NAME (or overrides them) in run.env, relay wiring will not be activated for that run. Build Relay config from the effective run environment (including run-level overrides) before deciding whether to call the broker. [api mismatch]
Severity Level: Major ⚠️
- ❌ Relay MCP never enabled from run.env overrides.
- ⚠️ Handlers cannot pass per-run Relay config safely.
- ⚠️ Personas relying on dynamic Relay fail silently.Steps of Reproduction ✅
1. Build the cloud runtime defaults by calling `createCloudRuntimeDefaults` from
`packages/runtime/src/cloud-defaults.ts:67-96`, which computes `env = options.env ??
process.env` and passes it into `createProcessHarnessRunner` as `args.env`.
2. Wire this runtime into a handler context using `buildCtx` so that `ctx.harness.run` is
backed by `options.harnessRunner`, as shown in `packages/runtime/src/ctx.ts:20-22` where
`harness: { run: options.harnessRunner }` is set.
3. From a handler, invoke `ctx.harness.run` with per-run environment overrides containing
Relay credentials, e.g. `ctx.harness.run({ prompt: '...', env: { RELAY_API_KEY: 'k',
RELAY_AGENT_NAME: 'agent' } })`, which is supported by `HarnessRunArgs.env` ("Environment
overrides merged on top of the persona's env") in `packages/runtime/src/types.ts:62-71`.
4. Follow the execution into `createProcessHarnessRunner` in
`packages/runtime/src/cloud-defaults.ts:14-146`: `relayMcp` is computed from the static
launcher env via `const relayMcp = resolveRelayMcpFromEnv(args.env);` at line 64, while
the child process environment merges `...(run.env ?? {})` at lines 108-114. If
`options.env` (and thus `args.env`) does not contain `RELAY_API_KEY` / `RELAY_AGENT_NAME`
but `run.env` does, `resolveRelayMcpFromEnv` (lines 154-165) returns `undefined`, no Relay
wiring/broker call occurs, yet the spawned harness still receives the RELAY_* variables in
`childEnv`. Relay MCP integration silently remains disabled for runs configured only via
`run.env`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/runtime/src/cloud-defaults.ts
**Line:** 463:463
**Comment:**
*Api Mismatch: Relay MCP detection is using only the static launcher environment, so per-run `env` overrides on `ctx.harness.run()` are ignored. If a caller supplies `RELAY_API_KEY` / `RELAY_AGENT_NAME` (or overrides them) in `run.env`, relay wiring will not be activated for that run. Build Relay config from the effective run environment (including run-level overrides) before deciding whether to call the broker.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| args.relayMcp.apiKey, | ||
| '--base-url', |
There was a problem hiding this comment.
Suggestion: The Relay API key is passed as a command-line argument to the broker, which exposes it via process argv (and potentially shell/process inspection tooling). This leaks credentials outside intended secret channels; pass the key through environment variables or another non-argv secret transport. [security]
Severity Level: Critical 🚨
- ❌ Relay API key exposed via broker argv.
- ❌ Host process introspection can capture secret arguments.
- ⚠️ Violates secret-handling guidance for subprocess credentials.Steps of Reproduction ✅
1. Configure a Codex-based persona with Relay enabled and build cloud defaults by calling
`createCloudRuntimeDefaults` in `packages/runtime/src/cloud-defaults.ts:67-96`, with `env`
containing `RELAY_API_KEY` and `RELAY_AGENT_NAME` so `resolveRelayMcpFromEnv` can succeed.
2. Invoke the handler's `ctx.harness.run({ prompt: '...' })`; `buildCtx` wires
`ctx.harness.run` to `options.harnessRunner` in `packages/runtime/src/ctx.ts:20-22`, which
is the function returned by `createProcessHarnessRunner` in
`packages/runtime/src/cloud-defaults.ts:14-146`.
3. Inside `createProcessHarnessRunner`, when `harness === 'codex'` and Relay config is
present, execution enters the Codex Relay branch at lines 82-99 and calls
`resolveCodexAgentRelayMcpArgs` with `{ env: args.env, relayMcp, cwd, existingArgs:
codexExistingArgs(spawnArgs), log }` (lines 83-88).
4. In `resolveCodexAgentRelayMcpArgs` (`packages/runtime/src/cloud-defaults.ts:168-235`),
the broker argument list `brokerArgs` is constructed at lines 177-192, explicitly adding
`'--api-key', args.relayMcp.apiKey` at lines 183-184 before spawning the broker via
`spawnAndCapture` (lines 199-205); this causes the Relay API key to appear in the child
process command line (e.g. `agent-relay-broker ... --api-key sk-relay-test`), which can be
read from process listings (`ps`, `/proc/<pid>/cmdline`, or equivalent tooling), exposing
a secret via argv rather than a safer channel like environment variables.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/runtime/src/cloud-defaults.ts
**Line:** 583:584
**Comment:**
*Security: The Relay API key is passed as a command-line argument to the broker, which exposes it via process argv (and potentially shell/process inspection tooling). This leaks credentials outside intended secret channels; pass the key through environment variables or another non-argv secret transport.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (process.platform === 'linux') return `linux-${arch}`; | ||
| if (process.platform === 'darwin') return `darwin-${arch}`; | ||
| if (process.platform === 'win32') return 'win32-x64.exe'; | ||
| return undefined; |
There was a problem hiding this comment.
Suggestion: The Windows broker suffix is hardcoded to x64 even when running on arm64, so bundled broker resolution will pick the wrong binary name on Windows arm64 hosts and fall back unexpectedly. Include architecture in the Windows suffix logic the same way Linux/macOS do. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Windows arm64 cannot use bundled broker binary.
- ⚠️ Relay Codex sessions rely on external broker installation.
- ⚠️ Fallback path may break Relay-enabled personas.Steps of Reproduction ✅
1. On a Windows arm64 host, install the Relay SDK broker into the Daytona path expected by
`resolveSandboxAgentRelayBrokerBinary`, e.g.
`/opt/relay-smoke/node_modules/@agent-relay/sdk/bin/agent-relay-broker-win32-arm64.exe`,
matching the platform-specific naming scheme used for Linux/macOS in
`agentRelayBrokerPlatformSuffix` (`packages/runtime/src/cloud-defaults.ts:254-259`).
2. Configure a Codex persona with Relay enabled and build cloud defaults with
`createCloudRuntimeDefaults` (`packages/runtime/src/cloud-defaults.ts:67-96`), then run a
handler that calls `ctx.harness.run`, flowing into `createProcessHarnessRunner`
(`packages/runtime/src/cloud-defaults.ts:14-146`) and its call to
`resolveCodexAgentRelayMcpArgs` (lines 82-88) for Codex Relay wiring.
3. Inside `resolveCodexAgentRelayMcpArgs`
(`packages/runtime/src/cloud-defaults.ts:168-235`), the broker path is resolved by
`resolveAgentRelayBrokerBinary` at line 175, which delegates to
`resolveSandboxAgentRelayBrokerBinary` (lines 237-245); that function uses
`agentRelayBrokerPlatformSuffix` (lines 254-260) to compute a suffix based on
`process.platform` and `process.arch`.
4. On Windows arm64, `agentRelayBrokerPlatformSuffix` sets `arch` to `'arm64'` but returns
`'win32-x64.exe'` for `process.platform === 'win32'` at line 259, yielding a candidate
path ending in `agent-relay-broker-win32-x64.exe` that does not exist;
`canExecuteFileSync` (lines 263-270) fails, `resolveSandboxAgentRelayBrokerBinary` returns
`undefined`, and `resolveAgentRelayBrokerBinary` falls back to `'agent-relay-broker'`
(lines 237-241). If no matching `agent-relay-broker` is on PATH, the subsequent spawn in
`resolveCodexAgentRelayMcpArgs` fails and logs `harness.relay_mcp.broker_args_failed`,
breaking Codex Relay integration specifically on Windows arm64.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/runtime/src/cloud-defaults.ts
**Line:** 659:659
**Comment:**
*Incorrect Condition Logic: The Windows broker suffix is hardcoded to x64 even when running on arm64, so bundled broker resolution will pick the wrong binary name on Windows arm64 hosts and fall back unexpectedly. Include architecture in the Windows suffix logic the same way Linux/macOS do.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/runtime/src/cloud-defaults.ts`:
- Around line 575-590: The current brokerArgs builder always appends a
--base-url flag using baseUrl which defaults to 'https://api.relaycast.dev',
causing Codex launches to force a Relay URL even when none was configured;
update the logic that builds brokerArgs (the const baseUrl and brokerArgs array
around the 'mcp-args' construction) so that you only push/append the
'--base-url' and its value when args.relayMcp.baseUrl is defined (truthy),
otherwise omit the flag entirely to match the non-Codex path behavior.
🪄 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: 16766e76-851b-42b3-bee1-2f88edbf7c11
📒 Files selected for processing (2)
packages/runtime/src/cloud-defaults.tspackages/runtime/src/runner.test.ts
|
Implemented fixes for PR #205:
Verification run locally:
All passed. |
|
Implemented fixes for PR #205:
Verification run locally:
All passed. |
khaliqgant
changed the title
|
Reviewed the PR changes and removed the stray Validation run: No reviewer/bot comments were present in the provided |
|
Reviewed PR #205 against the checked-out diff and traced the runtime/persona-kit surfaces. I removed the accidental generated file Local verification passed:
|
|
Reviewed PR #205 against the checked-out diff and traced the runtime/persona-kit surfaces. I removed the accidental generated file Local verification passed:
|
|
Implemented the validated PR fixes:
Local validation passed:
|
|
Reviewed PR #205 against Validation run:
|
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #205 against Validation run:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Reviewed PR #205 against the checked-out diff and traced the Relay MCP harness changes through runtime spec construction and related tests. I did not find a current breakage that required code edits. Verification completed:
Note: |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #205 against the checked-out diff and traced the Relay MCP harness changes through runtime spec construction and related tests. I did not find a current breakage that required code edits. Verification completed:
Note: |
|
Fixed validated issues in PR #205:
Validation run locally:
|
|
✅ pr-reviewer applied fixes — committed and pushed Fixed validated issues in PR #205:
Validation run locally:
|
|
Reviewed PR #205 against Local verification run:
Note: the root |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #205 against Local verification run:
Note: the root |
|
Reviewed PR #205 against Local verification completed:
Note: |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #205 against Local verification completed:
Note: |
|
pr-reviewer could not complete review for #205 in AgentWorkforce/workforce. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #205 in AgentWorkforce/workforce. |
1 participant
Summary
ctx.harness.run()Claude and Codex launches throughagent-relay-broker mcp-args --registerwhen Relay env is presentexec, broker-returned MCP args, then original Codex args while passing the base args through--existing-argsmcpServers.agent-relayinto the existing strict--mcp-configpayload, preserving persona MCP servers and removing auto-injected legacyrelaycastagent-relay/relaycastoverrides as an intentional opt-out from broker injection@relaycast/mcpfallback when the broker helper is unavailable or returns invalid outputReview
codex-1andclaude-1in Relaycast.agent-relay-broker mcp-args --register, consume its JSON rather than hand-constructing MCP env, pass--cwd, and treat the Daytona/opt/relay-smokebinary path as cloud-image compatibility only.Notes
resolveRelayMcpFromEnv(args.env)assumes the broker/Daytona launcher putsRELAY_API_KEYandRELAY_AGENT_NAMEin the runtime process env, not only in persona/run env. This matches the existingrelayMcpcontract inpersona-kit.--existing-argsbecause Claude always has a base--mcp-config; the returned broker payload is then merged into that strict config.agent-relayor legacyrelaycastpreserve that override and skip the pre-registered broker fast path unless their config already carries equivalent token settings.Tests
pnpm --filter @agentworkforce/runtime testpnpm --filter @agentworkforce/persona-kit test -- interactive-spec.test.jsgit diff --check