Skip to content

Record how MCP queries arrive: transport, protocol, client - #173

Merged
jpr5 merged 7 commits into
mainfrom
feat/mcp-request-context-analytics
Oct 2, 2026
Merged

jpr5 merged 7 commits into
mainfrom
feat/mcp-request-context-analytics

Conversation

@jpr5

@jpr5 jpr5 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase P2 of the stateless-MCP migration plan. Every query_log row now records how the query arrived: transport, protocol era, the protocol version the client requested, client name, and OAuth client id. The analytics summary adds unique clients and a protocol/transport mix, including unclassified counts. The dashboard gets a "Unique Clients" tile. The weekly report gets "Unique clients" and "Protocol mix" lines with coverage ("N of M calls classified", plus partial/inconsistent markers). This is the baseline we need before the new stateless protocol ships (P4), so we can see adoption.

Changes

  • Add nullable transport, protocol era/version, client name and auth client id columns to query_log
  • Add the requestContext adapter, a shared client-string cleaner and rate-limited warning, and normalise the request-context fields in logQuery
  • Capture the MCP initialize handshake on /mcp and /sse after the transport accepts it, and write the session analytics context from the search and knowledge tools
  • Count unique clients, the legacy/modern and streamable_http/sse mix, and unclassified rows in the analytics summary
  • Add a Unique Clients tile to the analytics dashboard, hidden when the summary omits the count
  • Show unique clients and the protocol mix, with unclassified coverage, in the weekly search report
  • Seed analytics rows with fixed per-session transport, protocol, client and IP context, and read unique clients from the real summary

Worth calling out:

  • The migration is additive and nullable. Proof: a drop-and-remigrate run kept existing rows.
  • One shared cleaner for client-supplied strings. It strips NUL and other control characters and lone surrogates, trims, caps by code point, and drops over-long auth ids instead of truncating them. A NUL in a client name used to make Postgres reject the row.
  • The handshake is recorded only after the transport accepts initialize. On SSE the first accepted initialize wins.
  • protocol_era is "legacy" for every current writer; "modern" is reserved for the stateless protocol.
  • Unique clients are keyed by OAuth client id, falling back to IP and user agent. An IP of 'unknown' or '' counts as no IP.
  • Analytics fallbacks log a rate-limited warning, with the error class and no client bytes.

Local red-green proof

RED ran on 9d6f5e4. The final GREEN ran on d2a32be, whose tree is identical to this head after the commit regroup.

RED vs GREEN on final commit d2a32be (source: p2-red.txt vs p2-green-final.txt)

Item RED (9d6f5e4) GREEN (d2a32be)
\d query_log 5 columns absent transport, protocol_era, protocol_version, client_name, auth_client_id present, nullable
Column SELECT error: columns absent 4 rows: 3x streamable_http + 1x sse, all legacy / 2025-11-25 / p2-proof-client; auth NULL, oauth client_id, NULL (not ''), NULL
Summary curl HTTP 200, 5 contract fields absent HTTP 200, unique_client=2, legacy=4, modern=0, streamable_http=3, sse=1
Hand SQL unique-client count error: column absent 2, equals summary
Weekly report header no "Unique clients" / "Protocol mix" lines "Unique clients: 2", "Protocol mix: legacy 100% / modern 0%; transport: streamable_http 75% / sse 25%". exit=1 is the Notion publish with unset NOTION_TOKEN, same as RED
Gap analysis exit 0 exit 0, diff vs RED is only the node PID in a deprecation warning
Server log initialize lines no era/client capture initialize protocol=... client=... logged for all 4 sessions plus SSE
Extra: NUL+emoji client name n/a row written; client_name stored nulbad😀client (NUL stripped, emoji kept, 13 chars, has_nul=f)
Extra: unknown Mcp-Session-Id n/a 404, code -32001 "Session not found" (P1 intact)
Extra: summary vs hand SQL (after 5th row) n/a summary 3/5/0/4/1 = SQL 3/5/0/4/1 (unique/legacy/modern/http/sse)

Review

Three rounds of 25 reviewers each, then a narrowed final pass under surgical scope. The one bug fixed in the final pass: the accessor-failure path (errorClassName / analyticsContextFields) could throw and fail the tool call.

Tests

4202 passing. tsc (both projects), build, prettier with CI's globs, test-shape and version-sync are all clean.

Follow-ups (not in this PR)

Test-strength gaps and comment-accuracy items deferred under surgical scope:

  • Test gap: the 4xx-initialize "not recorded" half is vacuous.
  • Test gap: the Infinity test does not assert the warning; the "logQuery throws" test is unasserted.
  • Test gap: no SSE call-then-init-then-call test, no /mcp or SSE capture-at-init tests, no errorClassName fallback tests.
  • Test hygiene: warn spies restored outside finally, rate-limiter reset only in afterEach, duplicated fixtures and JWT secret.
  • Docs: unique_client and unique_ip treat '' / 'unknown' IP differently while the doc says they use the same predicate (doc fix, do not change unique_ip).
  • Comments: stale or inaccurate claims ("cannot throw", "raw value logged", "captured once at session init" on SSE, "negotiated" protocol_version, safeLogToken/cleaner claims, seed "never drift").
  • Design nits: CHECK constraint on the new columns; shared SQL constants for era/transport lists; dashboard lacks protocol counts; SSE auth_client_id fixed at GET; seed realism (no client_ip/ua); pair rounding can sum to 100.1%.

Pre-existing bugs found during review:

  • getMachineRelayRules bare catch silently disables relay exclusion (src/db/analytics.ts:187).
  • A rejected /mcp initialize (406/415/400) keeps its session slot and per-IP limiter until the reaper runs (src/server.ts).
  • /messages session lookup uses a plain-object index, so a prototype-key sid resolves to an inherited property (src/sse-handlers.ts:410).
  • /messages 404 log lines log ip and sid unsanitized.
  • The weekly report exits 1 when the Notion publish fails after the report file is written; its archive catch is empty, relay rows are unvalidated, and a build error skips Slack.
  • weekly-search-report.yml: the Sunday 09:07 UTC run with a 7-day window drops most of Sunday; no concurrency group.
  • docs/analytics.html: blocked/relay panels ignore the date range; local date getters are off by one at UTC+12..+14; pill counts skip safeNumber.
  • src/mcp/tools/knowledge.ts:358 returns raw error text to the client.
  • src/server.ts:4131 swallows a getConfig throw with no log.
  • seed-analytics never sets score_kind and is not in CI typecheck/format.
  • static-quality.yml runs prettier via npx without a pinned devDependency and tests only Node 24 while engines allows >=20; package.json engines >=20 but deps need >=20.19.0.

Context

Plan: https://www.notion.so/3ed3aa38185281d89892e2c4a20ed154
P1: #172

🤖 Generated with Claude Code

https://claude.ai/code/session_01EDxYQLKhDxwoYe8GV2noDY

jpr5 added 7 commits October 2, 2026 08:07
…te-limited warning, and normalise the request-context fields in logQuery
…port accepts it, and write the session analytics context from the search and knowledge tools
…and unclassified rows in the analytics summary
…t and IP context, and read unique clients from the real summary
@jpr5
jpr5 merged commit 443892e into main Oct 2, 2026
7 checks passed
@jpr5
jpr5 deleted the feat/mcp-request-context-analytics branch October 2, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant