[navidnadali]

Problem

McpHttpServer.authenticateRequest is an Effect.fn span around the whole handled request, so traces attribute tool-call time to authentication. Production traces showed up to 14 s of tool-call time under this span; on the benchmark its p95 was 950 ms at 80 sessions, almost none of it authentication.

Change

• The auth span covers only admitting the request: the credential check, the mode limit and the Linked environments gate.
• The request runs after it, under the McpServer.…/<method> span, and each tool call gets its own mcp.tool.<name> span that times it. Its attributes name the tool (rpc.system, rpc.method, mcp.tool.name), never its arguments.
• docs/operations/observability.md documents the span names.

Rejected and missing credentials return the same HTTP statuses, bodies and headers as before. Server only; trace shape only.

Scope and approval

This is submitted under the contributing guide's exception for a very small, focused fix to an obvious bug: the defect is shown by tests that fail on main (below), the product change is +49/−8 lines in two files (the rest is tests), and it changes no product behaviour, default or setting.

A small, focused fix for an obvious defect in trace attribution; no product behaviour, default, setting or contract change.

Background and measurements: #31002 (context, not an approval request).

Verification

Failing first: two existing cases in apps/server/src/mcp/McpHttpServer.test.ts gain span assertions:

• vp test run apps/server/src/mcp/McpHttpServer.test.ts -t "admits provider and OAuth client credentials and points only clients at OAuth": records span start and end order (not time) and asserts the auth span ends before the tool span starts, and that a rejected credential's warning stays on the auth span.

• -t "preserves actionable screenshot failure text": the tool span is named for the tool and carries exactly three attributes.

• On main @ 8b44f42b1 the new assertions fail (5 failed, 24 passed: the credential case and the four screenshot-failure rows); with this change: 29/29.

• vp run check:changed: passes (format, lint, knip, typecheck of 3 packages, and 30 tests in the 2 affected test files), on macOS with Node 26.12.1. The server integration suites (vp test run apps/server/integration) also pass: 300 tests in 26 files.

Live server, one request per build: on upstream the auth span took 29.55 ms and wrapped a 22.28 ms tools/call; with this change authentication takes about 1 ms and is followed by McpServer tools/call → mcp.tool.<name>. A whole-build benchmark comparison (many other changes) read the auth span's p95 as 950 → 51 ms at 80 sessions: the span now measures something different, so this is not a speed-up.

Not checked

• No request gets faster; no performance claim.
• Unchanged and out of scope: image tools still write raw arguments (up to 500 characters) onto the McpServer tools/call span, and regular tools' inner spans parent to a startup span.

Made with Claude Opus 5.5 via Claude Code. Reviewed by GPT-6.1 Sol via Codex and, independently, by Claude Opus 5.5 via Claude Code.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at 6cc9779

Macroscope's review found this PR approvable — This change separates MCP authentication timing from tool execution and adds tool-specific spans without argument attributes. Existing admission checks and tool behavior remain unchanged, so the remaining risk is limited to telemetry attribution.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The HTTP MCP server now separates authentication from invocation-scope provisioning and wraps toolkit and image-tool handlers in named spans with RPC and tool-name attributes. Tests check span attributes, trace relationships, timing, and rejected-token behavior. Documentation describes the span sequence.

Changes: MCP HTTP tracing

Layer / File(s)	Summary
Authentication and invocation scope apps/server/src/mcp/McpHttpServer.ts, apps/server/src/mcp/McpHttpServer.test.ts	Authentication now returns either an HTTP response or an invocation scope containing the linked-environments setting. Tests check authentication span timing and rejected-token behavior.
Tool spans and validation apps/server/src/mcp/McpHttpServer.ts, apps/server/src/mcp/McpHttpServer.test.ts, docs/operations/observability.md	Toolkit and image-tool handlers now run inside named spans with RPC and tool-name attributes. Tests check span attributes, and the documentation describes the span sequence.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant McpHttpServer
  participant McpServer
  participant withToolSpans
  Client->>McpHttpServer: Send HTTP MCP request
  McpHttpServer->>McpHttpServer: authenticateRequest and resolve invocation scope
  McpHttpServer->>McpServer: Run request under method span
  McpServer->>withToolSpans: Invoke registered tool
  withToolSpans-->>McpServer: Return tool result

Loading

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: Minimal · up to 6cc97

No blocking risk was identified in the authentication and tool tracing changes.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "fix(server): MCP auth spans no longer include the tool call" names the main change: the authentication span no longer wraps tool-call time. It is a conventional commit title in plain langua…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the defect, expected trace behavior, scope rationale, tests and observed results, lim…

Full details: Title check

Explanation

The title "fix(server): MCP auth spans no longer include the tool call" names the main change: the authentication span no longer wraps tool-call time. It is a conventional commit title in plain language, matching the change to McpHttpServer tracing in apps/server. It does not cover the new mcp.tool.<name> spans, which is acceptable for a title.

Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the defect, expected trace behavior, scope rationale, tests and observed results, limitations, and the agent and harness used.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 8b44f42
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: ceb019b1-0972-4ac3-97b8-b8b95aacba78

Commits

Reviewing files that changed from the base of the PR and between 8b44f42 and 6cc9779.

Files selected for processing: 3

• apps/server/src/mcp/McpHttpServer.test.ts
• apps/server/src/mcp/McpHttpServer.ts
• docs/operations/observability.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

Verified head 6cc9779eb06925dee5008d2cbcdabd819bc35441 as-is on a Linux Boat VM. Reproduced the bug on github/main at 9e85538535 first, using a separate worktree. Installed each checkout with vp i and ran one background dev server at a time.

Used repository Playwright and /usr/bin/google-chrome to pair the actual web client, add a project pointing to a real git repository, select Codex gpt-6.1-sol, and submit real provider turns. No mocks, fixtures, or demo data. Both builds completed the turns and returned the expected tool results.

The persisted server traces prove the changed boundary:

                              main                 PR
orchestrator_capabilities     auth wraps call      auth ends before call
preview_status                auth wraps call      auth ends before call
device_screenshot             auth wraps call      auth ends before call
mcp.tool.<name> spans          absent               present for all three

Each PR tool span has exactly rpc.system, rpc.method, and mcp.tool.name, with no argument attributes. The screenshot tool's device-access-disabled error remains identical. Missing and invalid credentials still return the same 401 bodies and headers, normalizing only date and port. Rejection warnings stay on authentication spans and rejected requests produce no handler/tool spans. This verifies trace attribution, with no speed-up claim.

blacksmith-check ran vp test run apps/server/src/mcp/McpHttpServer.test.ts: 29/29 passed. The PR head also reports 30 successful CI checks, none failing or pending.

Sanitized command and trace transcript

Not verified

Live OAuth client credentials, mode limits, and linked-environment admission were not exercised; the focused suite covers those admission paths. Native desktop/iOS/Android clients, other providers, and successful hardware screenshots were not exercised on this Linux VM. The screenshot check exercised the real error path with device access disabled. No source changes were needed.

[navidnadali]

Note

Written by Claude Opus 5.5 (Claude Code) on behalf of Navid

Thanks for running this end to end against real provider turns. The main-vs-PR trace comparison and the unchanged 401 paths cover what this change should and shouldn't touch. Nothing further planned from my side unless you'd like the tool spans shaped differently.