Repository navigation
feat(mcp): send $mcp_unknown_tool and $mcp_input_required from posthogmcpsdk - #347
Merged
Merged
Conversation
This was referenced Oct 1, 2026
lucasheriques
changed the title
Contributor
posthog-go Compliance ReportDate: 2026-10-02 12:36:00 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
|
[Medium risk] Adds new event types to the analytics SDK. No new blocking issue was identified, though the stated release-version replacement remains a pre-merge step. Reviews (2) · Last reviewed commit: "chore(mcp): pin posthog-go to the head o..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
lucasheriques
force-pushed
the
lucas/mcp-go-adapter-optional-events
branch
from
October 1, 2026 23:22
74a3565 to
c7ac66b
Compare
lucasheriques
force-pushed
the
lucas/mcp-go-adapter-optional-events
branch
from
October 1, 2026 23:52
c7ac66b to
05a0395
Compare
lucasheriques
force-pushed
the
lucas/mcp-go-adapter-optional-events
branch
from
October 2, 2026 00:01
05a0395 to
be15306
Compare
lucasheriques
marked this pull request as ready for review
October 2, 2026 00:16
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
stamphog
Bot
dismissed
their stale review
October 2, 2026 00:39
The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.
lucasheriques
force-pushed
the
lucas/mcp-go-adapter-optional-events
branch
from
October 2, 2026 00:39
f12ed95 to
6ce1821
Compare
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The PR pins posthog-go to a pseudo-version of an unmerged stacked PR's head, and its own description says not to merge in that state. The go.mod change has no go.sum update in the diff, so the build may also break.
- Author wrote 100% of the modified lines and has 2 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
- posthogmcpsdk/go.mod pins posthog-go to a pseudo-version of #345's head, an unmerged commit. The PR description explicitly says not to merge with it.
- No go.sum change appears in the diff for the new pseudo-version dependency, so the module may not build or verify.
- This is the last PR in a stack (#345, #344, #346, then this one), and the release carrying #345 does not exist yet.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 248L, 4F substantive, 795L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (795L, 9F, two-areas, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 6ce1821 · reviewed head 6ce1821 |
…gmcpsdk The go-sdk adapter no longer counts an unregistered-tool call or an input_required round as $mcp_tool_call. Unknown tools are recognised by go-sdk's own unknown tool error, an echoed conversation handle still joins the $mcp_unknown_tool event, and a legacy call that exhausts go-sdk's shim is a failed call with $mcp_error_type input_required. Tested: posthogmcpsdk go vet and go test -race on go-sdk v1.6.1 and v1.8.0 (round tests skip on v1.6.1), root go vet and posthogmcp tests, bin/api-diff, probe cases X1 and C15 across eras. Reviewer notes: depends on the core PR (#345) being released; the adapter's posthog-go requirement is bumped to that release before this leaves draft.
… the real thing A registered tool that forwards another tool's unknown tool error was recorded as $mcp_unknown_tool, losing the failed call. The adapter now requires go-sdk's message to name the requested tool. A result that asks for input next to an error is a failed call, not a round the client receives. Tested: posthogmcpsdk go vet and go test -race -count=1 ./... on go-sdk v1.6.1 and v1.8.0 (the new cases failed first on v1.8.0); root posthogmcp tests; bin/api-diff.
The adapter needs the manual events API from #345, released in v1.30.0. Tested: CI's build, vet and go test -race against the release with the replace dropped; go vet and go test -race in posthogmcpsdk.
lucasheriques
force-pushed
the
lucas/mcp-go-adapter-optional-events
branch
from
October 2, 2026 12:31
6ce1821 to
2a64d90
Compare
There was a problem hiding this comment.
Approved.
No showstoppers: a contained change to the MCP analytics adapter, with thorough tests, from an author with strong familiarity and no unresolved concerns. The go.mod change only bumps the sibling posthog-go module to an already released version, with no scripts or hooks.
- Author wrote 100% of the modified lines and has 2 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 248L, 4F substantive, 795L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (795L, 9F, two-areas, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 2a64d90 · reviewed head 2a64d90 |
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
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
posthogmcpsdkcounted a call naming an unregistered tool, and aninput_requiredround, as$mcp_tool_call. The SDK spec says neither is a tool call (PostHog/sdk-specs#87, "Tool call event" and "Optional events"). The adapter now sends$mcp_unknown_tooland$mcp_input_requiredthrough the manual API added in #345.jsonrpc.CodeInvalidParams, messageunknown tool ...), not by the cached tool list, which is stale in both directions on a server that sends nolist_changed. An echoed conversation handle still joins the event; one is never minted.input_required: a round the client receives (go-sdk v1.8, 2026-07-28 clients) sends$mcp_input_requiredwith the request methods, never their content. The retry that completes the call sends the$mcp_tool_call.$mcp_tool_callwith$mcp_error_typeinput_required.Stack
Last of four: #345, #344 and #346 are merged and released in v1.30.0.
posthogmcpsdk/go.modrequires v1.30.0.Testing
In
posthogmcpsdk, on go-sdk v1.6.1 and v1.8.0 (the round tests skip on v1.6.1):go vetandgo test -race -count=1 ./...pass. Rootgo vet ./...andbin/api-diffpass. Probe cases X1 (unknown tool) and C15 (input_required) across the e1 to e4 eras give one$mcp_unknown_toolwith no$mcp_tool_call, and$mcp_input_requiredthen one$mcp_tool_callon v1.8.0. Two independent reviewers (Opus and Sonnet) found nine issues in the first version; all but one cosmetic duplication are fixed, with tests that failed first.