feat(telemetry): track stateless MCP protocol connects#3013
feat(telemetry): track stateless MCP protocol connects#3013ignaciojimenezr wants to merge 1 commit into
Conversation
dosubot
Bot
added
the
size:M
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![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 1 file
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="mcpjam-inspector/client/src/hooks/use-server-state.ts">
<violation number="1" location="mcpjam-inspector/client/src/hooks/use-server-state.ts:1240">
P2: Telemetry for `stateless_protocol_connect` is currently sent before `reconnectServer(...)` finishes, so failed reconnects will still be counted as connects. That can skew protocol adoption metrics and make success-rate analysis unreliable. A safer pattern is to emit this event only after a successful reconnect (and optionally add a separate failure event in the catch path).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
WalkthroughThis change adds PostHog telemetry to stateless MCP reconnects in Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
mcpjam-inspector/client/src/hooks/__tests__/use-server-state.protocol-revalidation.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/hooks/use-server-state.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
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 |
![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
🤖 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 `@mcpjam-inspector/client/src/hooks/use-server-state.ts`:
- Around line 1226-1249: The stateless_protocol_connect telemetry in the
reconnect path is being emitted more than once for a single logical reconnect
when reconnectServerInternal retries through the OAuth fallback branch. Move or
guard the capture in use-server-state.ts so it only fires once per
user-initiated reconnect (for example, before the retry path starts, or behind a
per-attempt flag), and make sure guardedReconnectServer/reconnectServerInternal
do not re-evaluate and re-capture connectionDefaults on the second authorization
attempt.
🪄 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
Run ID: 7f4b3c09-2748-446d-b26c-7c8f193a9c61
📒 Files selected for processing (1)
mcpjam-inspector/client/src/hooks/use-server-state.ts
ignaciojimenezr
force-pushed
the
stateless-protocol-telemetry
branch
from
e01713d to
7079b5c
Compare
dosubot
Bot
added
size:L
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/hooks/use-server-state.ts (1)
1246-1264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin-resolution precedence is duplicated — a quiet drift risk.
The server-override/host-pin resolution here mirrors the logic in
buildResolverConnectionDefaults(Lines 1205-1223). Should either side's precedence ever change,protocol_pin_sourcewould silently misreport without any test catching it. Consider extracting a single shared resolver so telemetry and the wire payload can never disagree.🤖 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 `@mcpjam-inspector/client/src/hooks/use-server-state.ts` around lines 1246 - 1264, The pin-resolution logic in useServerState duplicates the precedence used by buildResolverConnectionDefaults, which can let protocol_pin_source drift from the actual wire payload. Extract the shared server-override/host-pin resolution into a single helper and use it from both buildResolverConnectionDefaults and the protocolPinSource calculation so they always derive from the same source of truth.
🤖 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.
Nitpick comments:
In `@mcpjam-inspector/client/src/hooks/use-server-state.ts`:
- Around line 1246-1264: The pin-resolution logic in useServerState duplicates
the precedence used by buildResolverConnectionDefaults, which can let
protocol_pin_source drift from the actual wire payload. Extract the shared
server-override/host-pin resolution into a single helper and use it from both
buildResolverConnectionDefaults and the protocolPinSource calculation so they
always derive from the same source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c37980d7-19be-4399-9e47-73d79fa2bfa1
📒 Files selected for processing (2)
mcpjam-inspector/client/src/hooks/__tests__/use-server-state.protocol-revalidation.test.tsxmcpjam-inspector/client/src/hooks/use-server-state.ts
2 participants
usePostHog for telemetry on stateless mcp
Summary by cubic
Track and enrich stateless MCP protocol connect telemetry via PostHog. Emits one
stateless_protocol_connectevent per successful connect when the effective protocol pin is a stateless version.standardEventProps("use_server_state")and rich metadata:mcp_protocol_version,protocol_pin_source,connection_transport,auth_mode, attempt type (direct|forced_oauth|synced_oauth_credentials|authorized_reconnect|protocol_revalidation), and lightweight counts (headers/capabilities/versions).isStatelessProtocolVersion(effectiveProtocolVersion); safe no-op whenusePostHogis unavailable.serverNameandurlare not included.Written for commit 7079b5c. Summary will update on new commits.