fix(telemetry): record real mcp_client label instead of 'unknown' (extension WS dispatcher leak)#57
Merged
Conversation
…tension WS dispatcher leak) The extension's MCP dispatcher was passing the MCPBridgeClient INSTANCE OBJECT (`this`) into recordDispatch() where a STRING label was expected. The recorder's typeof gate failed on the object and stamped every telemetry_events row with mcp_client='unknown'. Live production DB: 61/61 events with mcp_client='unknown' across 2 install_uuids. Same root cause cascaded to model='unknown' because pricing receives the same broken value. The MCP server has been sending the canonical normalised label correctly the whole time on `payload.visualSession.client` (set by mcp/src/tools/manual.ts buildVisualSessionSidecar after the 13-label allowlist gate in mcp/src/tools/visual-session.ts). The extension reads it for visual-session lifecycle but the telemetry path never consumed it. Fix: add `extractMcpClientLabel(payload)` helper in extension/ws/mcp-tool-dispatcher.js that pulls the string from payload.visualSession.client (with trim + non-empty + type guards, fallback 'unknown'). Use it at both recordDispatch call sites (dispatchMcpToolRoute, dispatchMcpMessageRoute). Bridge-client object semantics for route handlers, restricted-response builders, and helper invocations are unchanged. Tests: new tests/mcp-dispatcher-client-label.test.js (24 assertions across 6 sections: allowlist passthrough, trim, missing payload, non-string client incl. the original object-leak shape, empty string, + source-string regression guard that BOTH recordDispatch sites use the helper). Wired into the root npm test chain right after mcp-metrics-recorder.test.js. Existing recorder + dispatcher + bridge + visual-session tests all stay green. Live impact starting next deploy: telemetry_events.mcp_client records the actual client name (Claude/Codex/OpenCode/...). Pricing-derived model populates correctly. popular_mcp_clients in /api/public-stats/global will populate as clients cross the k>=5 anonymity floor. Quick task 260515-i1j. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4 tasks
LakshmanTurlapati
added a commit
that referenced
this pull request
May 16, 2026
…59) * chore(release): bump version 0.9.66 -> 0.9.67 Bumps the public version string across manifest, package files, READMEs, store listing, and the showcase APP_VERSION constant (the latter was stale at 0.9.65 from the prior bump landing miss). Also captures the regenerated crawler artifacts (llms-full.txt + sitemap.xml) with today's lastmod stamp. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(telemetry): populate mcp_client on non-action message routes PR #57 fixed the bridge-object-as-client leak for action tools, but the dispatcher's `dispatchMcpMessageRoute` still recorded `unknown` for every non-action route (agent:register, mcp:get-tabs, mcp:get-dom, mcp:get-diagnostics, mcp:read-page) because those payloads never carry a `visualSession.client` sidecar -- it is built only by manual-tool dispatch in mcp/src/tools/manual.ts. Live prod DB after PR #57 deploy: 84 of 109 telemetry_events rows still landed as mcp_client='unknown', with `unknown` last_seen at the same wall-clock minute as the real-labelled rows (OpenClaw / Codex / ChatGPT). Confirms the leak is ongoing, not residual pre-fix data. Fix: add a per-bridge-connect cache in mcp-tool-dispatcher.js. The new `resolveMcpClientLabel(payload)` helper: - reads payload.visualSession.client first via the existing extractMcpClientLabel helper - on a real label, caches it in a module-level slot - on a payload-less / sidecar-less dispatch, falls back to the cached label before defaulting to 'unknown' Lifecycle: mcp-bridge-client.js calls clearLastKnownMcpClientLabel() from every fresh _ws.onopen so a different MCP client attaching on the same port (only one process can hold localhost:7225 at a time) cannot inherit the prior client's label. The reset is feature-gated on `typeof globalThis.clearLastKnownMcpClientLabel === 'function'` to defend against a load-order race. Both recordDispatch call sites in dispatchMcpToolRoute and dispatchMcpMessageRoute now go through resolveMcpClientLabel. Tests: tests/mcp-dispatcher-client-label.test.js extended from 24 to 38 assertions across 9 sections, covering: cache seeding on first real label, fallback fill-in for subsequent payload-less dispatches, reset on bridge reconnect (no inherit), update-not-stick on legitimate client switch within a connection, and a source-grep regression guard that both recordDispatch sites use resolveMcpClientLabel (not the narrower extractMcpClientLabel that lacks the fallback). Validation: full `npm test` and `node scripts/validate-extension.mjs` green. The first non-action message-route dispatch in a fresh connection still lands as 'unknown' (no telemetry signal exists pre- first-action-call), but every subsequent non-action call in the same connection now records the real client label. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(telemetry): scope client-label fallback by agentId (Codex PR #59 P2) Codex review on PR #59 flagged that the module-wide _lastKnownMcpClientLabel slot misattributes labels in multi-relay-client bridge topologies. mcp/src/bridge.ts runs in hub mode and maintains a relayClients Map serving multiple concurrent MCP-client processes (Claude, Codex, Cursor, ...) over the same extension WebSocket. With a single global slot, an action from agent A would seed the cache and then a payload-less mcp:get-tabs from agent B would be recorded as agent A's label until agent B issued its own first action. Fix: replace the string slot with a Map keyed by `payload.agentId` (injected on every post-register message by buildAgentPayload in mcp/src/agent-bridge.ts). resolveMcpClientLabel now: - reads agentId off the payload (null if absent) - on a real visualSession.client label + agentId, sets map[agentId] - on payload 'unknown' WITH agentId, falls back to map[agentId] - on payload 'unknown' WITHOUT agentId, returns 'unknown' (no cross- agent leak) The first agent:register message in a fresh agent scope still records 'unknown' because the MCP server learns its agent_id from the response -- one such record per relay-client session, unavoidable. clearLastKnownMcpClientLabel() clears the whole Map. The mcp-bridge- client.js onopen reset path is unchanged in shape; renaming would be churn for no behaviour change. Tests: extended from 38 to 45 assertions. New Test 6b asserts a payload-without-agentId returns 'unknown' even when another agent has a cached label. New Test 6c is the direct Codex P2 regression: agent A seeds 'Claude', agent B's payload-less get-tabs must return 'unknown' (not 'Claude'), and after agent B's own action seeds 'Codex' the two slots remain independent. Validation: full `npm test` and `node scripts/validate-extension.mjs` green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
LakshmanTurlapati
added a commit
that referenced
this pull request
May 18, 2026
* docs: start milestone v0.9.70 Showcase Dashboard Reliability (Streaming + Sync + Viewport) Three target features: (1) diagnose-first dashboard DOM-streaming fix continuing STREAM-07 attempt 2 of 5 from v0.9.69 Phase 276; (2) Sync-tab remote-control restoration on full-selfbrowsing.com; (3) 16:10 desktop viewport lock on the dashboard preview pane (mobile out of scope). PROJECT.md "Current Milestone" rewritten to v0.9.70 with goals + target features + key constraints + deferred candidates. Footer "Last updated" bumped to 2026-05-16. "Last shipped" updated to reflect v0.9.69 SHIP + four post-milestone fixes (PR #57/59/61/62/63/64) and the published artifacts (extension v0.9.67 zip, mcp-v0.9.2 npm, Fly auto-deploy). STATE.md frontmatter reset (milestone v0.9.70, status in_progress, progress 0%). Current Position set to "Not started (defining requirements)". Active Milestone Risk Register replaced with Active Milestone Carry-Forward (STREAM-07 attempt 2 + CWS click-through). Quick Tasks Completed table extended with PRs #59 / #61 / #62 / #63 / #64. Pending User-Gated Actions: v0.9.0 npm publish marked superseded by v0.9.2; added v0.9.67 -> extension-v0.9.67 retag decision. Phases dir cleared (8 v0.9.69 phase directories removed by gsd-tools phases clear). Carries over: 999.1-mcp-tool-gaps, INTEGRATION-CHECK.md, v0.9.63-INTEGRATION-CHECK.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(showcase): lock dashboard preview pane to 16:10 on desktop Phase 280 VIEWPORT requirement: the live DOM preview frame in the showcase dashboard had no fixed aspect ratio and stretched freely to container height, producing inconsistent letterboxing and the bug visible in milestone 0.9.70. Pin it to 16/10 on desktop, unset on mobile (<=768px and <=480px) where vertical layout takes over, and keep Maximized (100vh) and PiP layouts coherent (auto for maximized, explicit 16/10 for PiP). Adds tests/dashboard-preview-aspect-ratio.test.js — parses the SCSS and asserts the rule lives in the right selectors (desktop, both mobile media queries, Maximized, PiP). Wired into the root `npm test` chain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Telemetry was recording
mcp_client = 'unknown'on 100% of production events (61/61 rows / 2 install_uuids verified via live DB query). Same root cause was propagating tomodel = 'unknown'. This PR fixes the leak.Bug
extension/ws/mcp-bridge-client.js:523and:851invokedispatchMcpToolRoute({ ..., client: this })— passing the MCPBridgeClient instance object asclient. The dispatcher (extension/ws/mcp-tool-dispatcher.js:340, :424) forwarded that object straight intorecordDispatch(). The recorder (extension/utils/mcp-metrics-recorder.js:273) gates ontypeof input.client === 'string', which the object fails →'unknown'.The MCP server has been correctly attaching the canonical normalised label to
payload.visualSession.clientall along (mcp/src/tools/manual.ts:180, normalised against the 13-label allowlist atmcp/src/tools/visual-session.ts:19). The extension reads it for visual-session lifecycle (mcp-bridge-client.js:605-611) but the telemetry path never consumed it.Live evidence (pre-fix)
/api/public-stats/globalcorrectly returnedpopular_mcp_clients: []because every row aggregated into the same'unknown'bucket which fails the k>=5 anonymity floor.Fix
Added helper
extractMcpClientLabel(payload)inmcp-tool-dispatcher.jsthat returnspayload.visualSession.clientwhen it's a non-empty trimmed string, else'unknown'. Swapped bothrecordDispatchcall sites to use it. Bridge-client object semantics for route handlers, restricted-response builders, and helper invocations are unchanged.Test coverage
New
tests/mcp-dispatcher-client-label.test.js— 24 assertions across 6 sections:'unknown''unknown'— the object case directly reproduces the original bridge-instance leak shape'unknown'recordDispatch({blocks containextractMcpClientLabel(payload)AND neither contains the bareclient,arg. Catches a future partial revert.Wired into the root
npm testchain right aftermcp-metrics-recorder.test.js.Existing tests all green:
mcp-metrics-recorder(88/88),mcp-tool-routing-contract(151/151),mcp-bridge-client-lifecycle(55/55),mcp-metrics-no-pii-leakPASS,extension-record-dispatch-serializes(7/7),visual-session-agent-scoped(16/16),mcp-visual-tick-lifecycle(79/79).Test plan
node tests/mcp-dispatcher-client-label.test.js→ 24/24 PASS locallytelemetry_events.mcp_clientshows real labels (Claude/Codex/OpenCode/...) rather than 'unknown' in live DB/api/public-stats/globalpopular_mcp_clientspopulates once k>=5 floor is crossedNon-goals
agents/agent_runsempty tables — separate scheduled-agent feature, unrelatedmodel = 'unknown'— downstream of the same root cause; will self-correct once client label is realFixes telemetry data quality starting next deploy.
Quick task ID:
260515-i1j— see.planning/quick/260515-i1j-fix-telemetry-mcp-client-recording-as-un/for PLAN.md + SUMMARY.md.