From af11ede7727bafa60e57c91c1e18a6d3af9f2332 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 25 Aug 2026 14:57:17 -0400 Subject: [PATCH] feat: log which MCP client connected, fix missing logging capability Two fixes in the same startup path: 1. The McpServer never declared the logging capability, so every sendLoggingMessage call (including the existing startup/shutdown/fatal error messages) was a silent no-op regardless of when it was sent. Declared logging: {} in capabilities. 2. server.server.getClientVersion() is only populated once the client's initialize request has been processed -- reading it synchronously right after server.connect() races that request, since connect() only waits for the transport to start, not for the handshake to finish. Moved the read into a server.server.oninitialized callback, which fires only once the handshake is fully done. The client's identity (name/version from its initialize request) is now logged on connect and recorded as mcp.client.name/mcp.client.version on every subsequent tool-execution trace span, matching the equivalent changes in @mapbox/mcp-server and @mapbox/mcp-devkit-server. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 2 + src/index.ts | 26 ++++++++- src/utils/tracing.ts | 25 ++++++++- test/utils/tracing.test.ts | 112 +++++++++++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 test/utils/tracing.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ce66630..768afb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Unreleased +- fix: startup/shutdown/error logging messages now actually reach clients. The `McpServer` never declared the `logging` capability, so every `sendLoggingMessage` call (including the existing "server started"/"shutting down"/"Fatal error" messages) was a silent no-op regardless of when it was sent. Declared `logging: {}` in the server's capabilities. +- feat: the server now identifies which MCP client connected to it. `server.server.getClientVersion()` (populated from the `clientInfo` sent in the client's `initialize` request) is now logged on connect, e.g. `Client identified as: claude-ai v1.0.0`. Reading it required moving the read to a `server.server.oninitialized` callback rather than right after `server.connect()`, since `getClientVersion()` is only populated once the client's `initialize` request has actually been processed, which is not guaranteed by the time `connect()`'s promise resolves. Also recorded as `mcp.client.name`/`mcp.client.version` on every subsequent tool-execution trace span (`withToolSpan`), so OTel-backed traces can be filtered or grouped by client. Matches the equivalent changes landing in `@mapbox/mcp-server` and `@mapbox/mcp-devkit-server`. - chore: bump `@modelcontextprotocol/sdk` to `1.30.0`. Verified this version's own `SUPPORTED_PROTOCOL_VERSIONS` constant does not include the `2026-07-28` spec revision (https://blog.modelcontextprotocol.io/posts/2026-07-28/) — this repo uses neither elicitation nor sampling, so that migration is lower-priority here regardless; tracked in #40. - chore: add `scripts/check-llms-links.cjs` (`npm run check-llms-links`) to report broken links across all `llms.txt` files exposed from docs.mapbox.com, and flag drift between the curated list in `docsSearchIndex.ts` and the live root index; `npm run check-llms-links:deep` (`--deep`) additionally crawls every docs.mapbox.com sub-link referenced in those files (currently ~1,400 URLs) to proactively catch broken doc pages before customers hit them (#39) - docs: note in CONTRIBUTING.md that unsolicited third-party directory/discovery listing PRs are out of scope and will be closed without review diff --git a/src/index.ts b/src/index.ts index 52402b8..8df9ec2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,11 @@ import { parseToolConfigFromArgs, filterTools } from './config/toolConfig.js'; import { getCoreTools } from './tools/toolRegistry.js'; import { getAllResources } from './resources/resourceRegistry.js'; import { getVersionInfo } from './utils/versionUtils.js'; -import { initializeTracing, shutdownTracing } from './utils/tracing.js'; +import { + initializeTracing, + shutdownTracing, + setClientInfo +} from './utils/tracing.js'; // Parse configuration from command-line arguments const config = parseToolConfigFromArgs(); @@ -27,7 +31,8 @@ const server = new McpServer( { capabilities: { tools: {}, - resources: {} + resources: {}, + logging: {} } } ); @@ -46,6 +51,23 @@ resources.forEach((resource) => { async function main() { await initializeTracing(); + // Registered before connect() so it's already in place the moment the + // client's initialize handshake completes. getClientVersion() is only + // populated once the server has processed the client's `initialize` + // request -- reading it synchronously right after `server.connect()` + // races that request, since connect() only waits for the transport to + // start, not for the handshake to finish. + server.server.oninitialized = () => { + const clientInfo = server.server.getClientVersion(); + // Recorded so every subsequent tool-execution span carries it too -- + // see setClientInfo's doc comment in tracing.ts. + setClientInfo(clientInfo); + server.server.sendLoggingMessage({ + level: 'info', + data: `Client identified as: ${clientInfo?.name ?? 'unknown'} v${clientInfo?.version ?? 'unknown'}` + }); + }; + const transport = new StdioServerTransport(); await server.connect(transport); diff --git a/src/utils/tracing.ts b/src/utils/tracing.ts index e329f3d..2bb09f6 100644 --- a/src/utils/tracing.ts +++ b/src/utils/tracing.ts @@ -137,6 +137,23 @@ export function getTracer() { return trace.getTracer('mapbox-mcp-docs-server'); } +/** + * The connected client's `clientInfo` (name/version), as reported in its + * `initialize` request. There is exactly one client per stdio server + * process, so this is safe as module-level state; set once via + * `setClientInfo` after the initialize handshake completes (see + * `server.server.oninitialized` in index.ts) and read by every tool span + * created afterward, so traces can be filtered/grouped by which MCP client + * (Claude Desktop, Cursor, VS Code, etc.) made the call. + */ +let currentClientInfo: { name?: string; version?: string } | undefined; + +export function setClientInfo( + info: { name?: string; version?: string } | undefined +): void { + currentClientInfo = info; +} + /** * Create a span for tool execution and run the callback within its context. * Automatically marks the span success or error and ends it. @@ -150,7 +167,13 @@ export async function withToolSpan( kind: SpanKind.INTERNAL, attributes: { 'tool.name': toolName, - 'operation.type': 'tool_execution' + 'operation.type': 'tool_execution', + ...(currentClientInfo?.name && { + 'mcp.client.name': currentClientInfo.name + }), + ...(currentClientInfo?.version && { + 'mcp.client.version': currentClientInfo.version + }) } }); diff --git a/test/utils/tracing.test.ts b/test/utils/tracing.test.ts new file mode 100644 index 0000000..49d0dd0 --- /dev/null +++ b/test/utils/tracing.test.ts @@ -0,0 +1,112 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + withToolSpan, + getTracer, + setClientInfo +} from '../../src/utils/tracing.js'; + +// Mock the OpenTelemetry modules to avoid actual tracing in tests +vi.mock('@opentelemetry/sdk-node', () => ({ + NodeSDK: vi.fn().mockImplementation(() => ({ + start: vi.fn(), + shutdown: vi.fn().mockResolvedValue(undefined) + })) +})); + +vi.mock('@opentelemetry/api', () => ({ + trace: { + getTracer: vi.fn().mockReturnValue({ + startSpan: vi.fn().mockReturnValue({ + setAttributes: vi.fn(), + setStatus: vi.fn(), + recordException: vi.fn(), + end: vi.fn() + }) + }), + setSpan: vi.fn((ctx) => ctx) + }, + context: { + active: vi.fn().mockReturnValue({}), + with: vi.fn((_ctx, fn) => fn()) + }, + SpanStatusCode: { + OK: 1, + ERROR: 2 + }, + SpanKind: { + INTERNAL: 0, + CLIENT: 3 + }, + diag: { + setLogger: vi.fn() + }, + DiagLogLevel: { + NONE: 0, + ERROR: 30, + WARN: 50, + INFO: 60, + DEBUG: 70, + VERBOSE: 80 + } +})); + +describe('tracing utilities', () => { + beforeEach(() => { + vi.clearAllMocks(); + setClientInfo(undefined); + }); + + describe('withToolSpan', () => { + it('creates a tool span with basic attributes and no client info by default', async () => { + const tracer = getTracer(); + const mockSpan = { + setAttributes: vi.fn(), + setStatus: vi.fn(), + recordException: vi.fn(), + end: vi.fn() + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(tracer.startSpan).mockReturnValue(mockSpan as any); + + await withToolSpan('test_tool', async () => 'result'); + + expect(tracer.startSpan).toHaveBeenCalledWith('tool.test_tool', { + kind: expect.any(Number), + attributes: { + 'tool.name': 'test_tool', + 'operation.type': 'tool_execution' + } + }); + }); + + it('includes the connected client name/version once set via setClientInfo', async () => { + const tracer = getTracer(); + const mockSpan = { + setAttributes: vi.fn(), + setStatus: vi.fn(), + recordException: vi.fn(), + end: vi.fn() + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(tracer.startSpan).mockReturnValue(mockSpan as any); + + setClientInfo({ name: 'claude-ai', version: '1.0.0' }); + await withToolSpan('test_tool', async () => 'result'); + + expect(tracer.startSpan).toHaveBeenCalledWith('tool.test_tool', { + kind: expect.any(Number), + attributes: { + 'tool.name': 'test_tool', + 'operation.type': 'tool_execution', + 'mcp.client.name': 'claude-ai', + 'mcp.client.version': '1.0.0' + } + }); + }); + }); +});