Add Relaycast attribution and Agent Relay MCP tool events#1041
Conversation
|
CodeAnt AI is reviewing your PR. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds telemetry event types and CLI telemetry client behavior, derives Relaycast telemetry options from explicit values or environment, threads telemetry into SDK and CLI RelayCast/WsClient construction, instruments MCP tool calls with telemetry, and updates agent-relay naming/config across broker, runtime, plugins, tests, and docs. ChangesTelemetry Instrumentation for CLI and SDK
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant Wrapper as InboxPiggybackWrapper
participant Relay as RelayCast/WsClient
participant Telemetry as Telemetry.track
Client->>Wrapper: invoke tool (tool_name, args)
Wrapper->>Wrapper: record startedAt, derive tool_type/category
Wrapper->>Relay: execute tool call
alt Success
Relay-->>Wrapper: result
Wrapper->>Telemetry: agent_relay_tool_call(success: true, duration_ms, tool_name, tool_type, tool_category, transport)
else Failure
Relay-->>Wrapper: throws / error result
Wrapper->>Telemetry: agent_relay_tool_call(success: false, error_class, duration_ms, tool_name, tool_type, tool_category, transport)
end
Wrapper-->>Client: return result or recovery payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
codeant-ai
Bot
added
the
size:XL
![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: 45f94321c5
ℹ️ 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".
|
|
||
| export function relaycastTelemetryOptions( | ||
| explicit: RelaycastTelemetryOptions = {}, | ||
| env: NodeJS.ProcessEnv = process.env |
There was a problem hiding this comment.
In browser or edge runtimes that do not polyfill Node's global process, constructing an AgentRelay with a workspace key now calls relaycastTelemetryOptions() through createRelaycastClient, which evaluates this default parameter and throws before the RelayCast client is created. The SDK is documented for browser-worker delivery and other SDK code avoids static Node globals for this reason, so this should use a safe globalThis.process?.env fallback or only read env when process exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid. Fixed in 06743bf by removing the SDK helper default parameter that evaluated process.env. The SDK now reads env through a safe globalThis.process?.env fallback, and the regression test covers constructing AgentRelay when process is absent.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| track('mcp_action_call', { | ||
| tool_name: input.toolName, | ||
| action_type: input.actionType, | ||
| transport: input.transport ?? 'unknown', | ||
| success: input.success, | ||
| duration_ms: Date.now() - input.startedAt, | ||
| ...(input.errorClass ? { error_class: input.errorClass } : {}), | ||
| }); |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
mcp_action_call is instrumented in the MCP stdio server, but this process never initializes the telemetry client in normal usage (agent-relay mcp or the agent-relay-mcp.js entrypoint), so all track() calls from this file are no-ops and the events are dropped at runtime.
Suggestion: Initialize telemetry in the MCP entrypoint/startup path (or a shared MCP bootstrap) before creating the server, using initTelemetry with an appropriate app/surface and showNotice: false, and ensure shutdown() is called on process exit so mcp_action_call events can flush.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** packages/cli/src/cli/agent-relay-mcp.ts
**Line:** 1008:1015
**Comment:**
*HIGH: `mcp_action_call` is instrumented in the MCP stdio server, but this process never initializes the telemetry client in normal usage (`agent-relay mcp` or the `agent-relay-mcp.js` entrypoint), so all `track()` calls from this file are no-ops and the events are dropped at runtime.
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.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
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 fixThere was a problem hiding this comment.
Valid duplicate of the MCP init issue. Fixed in 06743bf by initializing telemetry in the MCP stdio startup path and avoiding immediate shutdown from the parent CLI mcp command flow.
| track('mcp_action_call', { | ||
| tool_name: input.toolName, | ||
| action_type: input.actionType, | ||
| transport: input.transport ?? 'unknown', | ||
| success: input.success, | ||
| duration_ms: Date.now() - input.startedAt, | ||
| ...(input.errorClass ? { error_class: input.errorClass } : {}), | ||
| }); |
There was a problem hiding this comment.
Suggestion: This new MCP telemetry path assumes the telemetry client is initialized, but the mcp command flow explicitly skips initTelemetry in CLI bootstrap, so this track call is a no-op at runtime and mcp_action_call events are never emitted. Initialize telemetry for MCP server runs (for example with notice disabled) before tool handling starts, or explicitly wire a telemetry transport that does not depend on the global CLI init path. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ MCP stdio server emits no mcp_action_call telemetry events.
- ⚠️ Dashboards cannot track MCP tool usage or error rates.
- ⚠️ MCP action grouping by action_type is never populated.
- ⚠️ Production MCP issues lack per-call telemetry traces.Steps of Reproduction ✅
1. Invoke the CLI MCP server via the normal entrypoint, e.g. run `agent-relay mcp` which
executes `runCli()` from `packages/cli/src/cli/index.ts:7-22` and dispatches to the `mcp`
command defined in `packages/cli/src/cli/bootstrap.ts:48-54`.
2. In `runCli()` (`packages/cli/src/cli/bootstrap.ts:89-102`), observe that
`shouldSkipTelemetryInit(argv)` (`bootstrap.ts:65-69`) returns `true` for the `mcp`
command because `STDIO_SERVER_COMMANDS` is defined as `new Set(['mcp'])` at
`bootstrap.ts:147-148`, so the guarded `initTelemetry({ ... })` call at
`bootstrap.ts:93-101` is never executed for MCP runs.
3. The `mcp` command action in `createProgram()` (`bootstrap.ts:48-54`) imports
`./agent-relay-mcp.js` and calls `startAgentRelayMcpStdio(mod.optionsFromEnv())`, which in
turn creates the MCP server (`createAgentRelayMcpServer`) and wraps tool handlers with
`enableInboxPiggyback` in `packages/cli/src/cli/agent-relay-mcp.ts:83-66 (wrapper starting
at 95-105)`.
4. When an MCP client invokes any registered tool (for example `invoke_action` or
`add_agent`), the wrapped handler in `enableInboxPiggyback` executes and calls
`trackMcpActionCall` at `agent-relay-mcp.ts:113-120, 18-25, 32-40`, which internally calls
`track('mcp_action_call', { ... })` at `agent-relay-mcp.ts:65-80`; however, since
`initTelemetry()` was never called, the telemetry client `client/commonProps/distinctId`
remain `null` (see `packages/cli/src/cli/telemetry/client.ts:22-27` and `initTelemetry` at
`client.ts:148-187`), so `track()` immediately returns without emitting anything
(`client.ts:189-203`), meaning `mcp_action_call` events are never actually sent for any
MCP tool execution.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/cli/src/cli/agent-relay-mcp.ts
**Line:** 1008:1015
**Comment:**
*Api Mismatch: This new MCP telemetry path assumes the telemetry client is initialized, but the `mcp` command flow explicitly skips `initTelemetry` in CLI bootstrap, so this `track` call is a no-op at runtime and `mcp_action_call` events are never emitted. Initialize telemetry for MCP server runs (for example with notice disabled) before tool handling starts, or explicitly wire a telemetry transport that does not depend on the global CLI init path.
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 fixThere was a problem hiding this comment.
Valid. Fixed in 06743bf: startAgentRelayMcpStdio now initializes telemetry with showNotice: false and surface: mcp, installs a beforeExit flush hook for the stdio entrypoint, and the CLI success path no longer immediately shuts telemetry down for stdio server commands.
|
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: 1
🧹 Nitpick comments (1)
packages/cli/src/cli/agent-relay-mcp.startup.test.ts (1)
401-451: ⚡ Quick winAdd one assertion for the stdio bootstrap RelayCast path.
These cases cover websocket, direct client, and
create_workspaceattribution, but the newresolveStdioBootstrapOptions()constructor wrapping inpackages/cli/src/cli/agent-relay-mcp.ts:1997-2002is still untested. One small assertion there would keep harness/distinct-id propagation from regressing on the bootstrap path too.🤖 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/cli/src/cli/agent-relay-mcp.startup.test.ts` around lines 401 - 451, Add one assertion that verifies the stdio bootstrap RelayCast path receives the Relaycast telemetry context: after calling createAgentRelayMcpServer(...) with telemetryTransport: 'stdio', assert that the options produced by resolveStdioBootstrapOptions (the stdio bootstrap constructor path) include harness and agentRelayDistinctId propagation (e.g., inspect the stdio/bootstrap client config in the test mocks similar to how websocket/direct clients are checked). Reference resolveStdioBootstrapOptions and createAgentRelayMcpServer and assert the stdio/bootstrap mock's config contains harness: 'claude/opus-48' and agentRelayDistinctId: 'distinct_test'.
🤖 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/cli/src/cli/agent-relay-mcp.ts`:
- Around line 1086-1094: The tracking call using trackMcpActionCall is emitted
too early (before the opportunistic inbox() piggyback completes), so move the
success-path tracking to just before the final return result to ensure
duration_ms covers the full wrapped handler; specifically, after you compute
result and await any inbox() fetch, call trackMcpActionCall({ toolName: name,
actionType, transport: telemetryTransport, startedAt, success:
!isErrorToolResult(result), ...(isErrorToolResult(result) ? { errorClass:
'ToolResultError' } : {}) }) so duration_ms is measured correctly; make the same
adjustment for the other identical tracking invocation near the other tool path
that currently mirrors this block.
---
Nitpick comments:
In `@packages/cli/src/cli/agent-relay-mcp.startup.test.ts`:
- Around line 401-451: Add one assertion that verifies the stdio bootstrap
RelayCast path receives the Relaycast telemetry context: after calling
createAgentRelayMcpServer(...) with telemetryTransport: 'stdio', assert that the
options produced by resolveStdioBootstrapOptions (the stdio bootstrap
constructor path) include harness and agentRelayDistinctId propagation (e.g.,
inspect the stdio/bootstrap client config in the test mocks similar to how
websocket/direct clients are checked). Reference resolveStdioBootstrapOptions
and createAgentRelayMcpServer and assert the stdio/bootstrap mock's config
contains harness: 'claude/opus-48' and agentRelayDistinctId: 'distinct_test'.
🪄 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: 87e95e2d-af92-4a35-a587-b920d05d5988
📒 Files selected for processing (14)
.agentworkforce/trajectories/active/traj_b1jrutolckfb/trajectory.jsonpackages/cli/src/cli/agent-relay-mcp.startup.test.tspackages/cli/src/cli/agent-relay-mcp.tspackages/cli/src/cli/commands/core.test.tspackages/cli/src/cli/lib/core-maintenance.tspackages/cli/src/cli/lib/relaycast-telemetry.tspackages/cli/src/cli/telemetry/client.test.tspackages/cli/src/cli/telemetry/client.tspackages/cli/src/cli/telemetry/events.tspackages/cli/src/cli/telemetry/index.tspackages/sdk/src/__tests__/agent-relay.test.tspackages/sdk/src/agent-relay.tspackages/sdk/src/messaging/relaycast.tspackages/sdk/src/relaycast-telemetry.ts
|
Handled the remaining review/user feedback across
Validated with focused CLI/SDK tests, broker MCP/snippets/PTY filters, and package builds. |
|
CodeAnt AI is running Incremental review |
|
Follow-up on Agent Relay tool kind naming in
Validated with focused CLI telemetry tests, broker MCP/snippets/PTY filters, the Python adapter test, and |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI Incremental review completed. |
|
CodeAnt AI is running Incremental review |
willwashburn
changed the title
codeant-ai
Bot
added
size:XXL
|
CodeAnt AI Incremental review completed. |
|
Preview deployed!
This preview will be cleaned up when the PR is merged or closed. |
![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 51 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="plugins/codex-relay-skill/scripts/setup.sh">
<violation number="1" location="plugins/codex-relay-skill/scripts/setup.sh:175">
P2: Installer dropped legacy `relaycast` key migration; existing configs get a second MCP block instead of reusing prior values. This breaks compatibility for upgraded users with customized relaycast settings.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| { | ||
| if (!in_block && $0 ~ /^[[:space:]]*mcp_servers[.]relaycast[.]command[[:space:]]*=/) { | ||
| if (!in_block && $0 ~ /^[[:space:]]*mcp_servers[.]agent-relay[.]command[[:space:]]*=/) { |
There was a problem hiding this comment.
P2: Installer dropped legacy relaycast key migration; existing configs get a second MCP block instead of reusing prior values. This breaks compatibility for upgraded users with customized relaycast settings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/codex-relay-skill/scripts/setup.sh, line 175:
<comment>Installer dropped legacy `relaycast` key migration; existing configs get a second MCP block instead of reusing prior values. This breaks compatibility for upgraded users with customized relaycast settings.</comment>
<file context>
@@ -172,30 +172,30 @@ ensure_relaycast_mcp_block() {
}
{
- if (!in_block && $0 ~ /^[[:space:]]*mcp_servers[.]relaycast[.]command[[:space:]]*=/) {
+ if (!in_block && $0 ~ /^[[:space:]]*mcp_servers[.]agent-relay[.]command[[:space:]]*=/) {
block_seen = 1
dotted_seen = 1
</file context>
1 participant
Summary
claude/opus-48.cli_install,cli_update, andagent_relay_tool_calltelemetry; tool-call payloads usetool_type/tool_categoryfor spawn, release, action, workspace, message, inbox, and related flows.agent-relay,mcp__agent-relay__..., andmcp_agent_relay_...; legacyrelaycastserver keys are only consumed for config migration/compatibility.Verification
npx vitest run packages/cli/src/cli/agent-relay-mcp.startup.test.ts packages/cli/src/cli/telemetry/client.test.ts packages/cli/src/cli/commands/core.test.tsnpm --prefix packages/cli run buildcargo test -p agent-relay-broker snippetscargo test -p agent-relay-broker injection_formatcargo test -p agent-relay-broker cli_mcp_argscargo test -p agent-relay-broker pty_workeruv run --with pytest --with pytest-asyncio --with aiohttp --with pyyaml pytest packages/sdk-py/tests/communicate/adapters/test_claude_sdk.py