Repository navigation
feat(mcp): add a sendInitializeEvent option - #5401
Conversation
ff80431 to
9649f86
Compare
Coherence0 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 7 bullets (5 invariants, 2 requirements), 0 problems Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
|
@PostHog review |
|
@gesh PostHog Review is reviewing this pull request. |
captureInitialize: false stops $mcp_initialize. The option is on by default. Many clients reconnect before every few tool calls, so the handshake event can outnumber $mcp_tool_call. With the option off, the initialize wrapper skips captureEvent. Nothing else changes: it still mints the session token and records the negotiated protocol version, so tool calls keep their sessions and client identity. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9649f86 to
52f806e
Compare
There was a problem hiding this comment.
Not approved — escalated to a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The public SDK option and analytics write-path change lack independent assurance on the current head; the clean review covers an older commit. Request a current-head review from Manoel Aranda Neto, Lucas Faria, or another teammate or agent reviewer before re-requesting.
- Author wrote 14% of the modified lines and has 31 merged PRs in these paths (familiarity MODERATE).
- Risky territory without independent assurance: Public API contracts — packages/mcp/src/types.ts adds an optional captureInitialize setting to the exported MCPAnalyticsOptions interface, with its default supplied in packages/mcp/src/index.ts.; Data ingestion or write paths — packages/mcp/src/extensions/instrumentation.ts conditionally suppresses publication of initialization analytics while leaving session creation and negotiated-protocol persistence outside the condition.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 16L, 3F substantive, 93L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (93L, 6F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ 52f806e · reviewed head 52f806e |
lucasheriques
left a comment
There was a problem hiding this comment.
LGTM, approving. One naming question worth settling before merge, plus an optional docs note.
| * Send `$mcp_initialize` for each `initialize` handshake. On by default. | ||
| * Set to `false` to stop sending it. Nothing else changes. | ||
| */ | ||
| captureInitialize?: boolean |
There was a problem hiding this comment.
These option names collide with the manual capture methods: PostHogMCP.captureInitialize(data) / captureToolsList(data) already exist. Reading instrument(server, ph, { captureInitialize: false }) next to ph.captureInitialize(...) is confusing, and this is public API we'd have to deprecate in two SDKs to change later.
Could we pick names that don't overload the method? A few options:
sendInitializeEvent/sendToolsListEventinitializeEvent: false/toolsListEvent: false- one
disabledEvents: ['$mcp_initialize', '$mcp_tools_list']list, which also covers the next event we want to make optional
Not a strong preference on which one, just want it decided before we ship it. If you'd rather keep the names, fine, but let's do it on purpose. (Same applies to captureToolsList in #5404.)
| The reasoning behind these defaults is in [ADR-0013](./docs/adr/0013-analytics-capture-is-on-by-default.md). | ||
|
|
||
| Set `captureInitialize: false` to stop sending `$mcp_initialize`. Many clients reconnect before | ||
| every few tool calls, so this event can outnumber `$mcp_tool_call`. Nothing else changes: tool |
There was a problem hiding this comment.
optional: "Nothing else changes" is true for the SDK, but the dashboard template's users/sessions tiles and the onboarding waiting state both read $mcp_initialize. Maybe "Nothing else changes in what the SDK sends" or a short note? Fine to skip if we're moving those off initialize soon anyway.
There was a problem hiding this comment.
Added in 80bc985: "Nothing else changes in what the SDK sends", plus a note that onboarding and the dashboard's users and sessions tiles read $mcp_initialize.
PostHogMCP already has a captureInitialize() method that sends the event by hand. An option with the same name next to that method makes readers link the two. The new name says what the option controls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MCP onboarding and the users and sessions tiles of the default MCP dashboard read $mcp_initialize. The README now says that they show no data while sendInitializeEvent is off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
$mcp_initializecan outnumber$mcp_tool_call.Part of one change across three repos: PostHog/posthog-python#1115 (
posthog.mcp) and PostHog/sdk-specs#141 (the spec).Follow-up, stacked on this PR: #5404 adds the same option for
$mcp_tools_list.Changes
sendInitializeEvent.DEFAULT_OPTIONSsets it totrue, so nothing changes unless a host turns it off.captureInitialize, becausePostHogMCP.captureInitialize()is already a method that sends the event by hand.sendInitializeEvent: false, theinitializewrapper skipscaptureEvent.$mcp_initialize. The README says that they show no data while the option is off.minorchangeset.Testing
stateless-session.test.ts, withsendInitializeEvent: false:$mcp_initialize, and pod B resolves the minted token's session, client name, version and protocol.sdk-v1,nest-v1(17/17) andnest-v2(37/39, 2 pinned failures) pass.sdk-v2passes after the rebase ontomain, which fixed the twolonglivedrows.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
claude-opus-5-5).🤖 Generated with Claude Code