Detect MCP requests that omit the optional transport headers - #130
Merged
Merged
Conversation
`is_mcp_request()` gated on `Mcp-Session-Id` or `MCP-Protocol-Version`. Both are optional: the session header only exists if the server issued one at `initialize`, and the protocol header only entered the spec in the 2025-06-18 revision (mcp-adapter's own validator treats it as absent-is-fine). ChatGPT's client (`openai-mcp/1.0.0`) sends neither — it carries its own `X-OpenAI-Session`. So `record()` returned early and every ability it executed produced no Mixpanel event, silently, while clients that do send a header worked. Confirmed from a captured request against an Easy MCP AI endpoint. Widen detection to a union of signals: - the two transport headers (authoritative when present) - `Accept: text/event-stream` — the streamable-HTTP transport requires clients to accept both `application/json` and `text/event-stream` on POST, so it is present exactly where the headers above are not - a request path resolving to an MCP endpoint, covering pretty REST permalinks, the `?rest_route=` fallback, and `X-Original-URL` for proxies that rewrite `REQUEST_URI` Still fails closed: this is only consulted from `wp_after_execute_ability`, where the only non-MCP callers are our admin-ajax UI and the plain REST abilities route — neither sends `text/event-stream` nor posts to an `/mcp` path.
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.
The bug
MarketplaceAbilitiesTracking::is_mcp_request()recognised an MCP call by exactly two headers:Both are optional.
Mcp-Session-Idexists only if the server issued one atinitialize;MCP-Protocol-Versiononly entered the spec in the 2025-06-18 revision, and mcp-adapter's own validator documents that "A missing header is accepted".ChatGPT's client sends neither. Captured from a real request to an Easy MCP AI endpoint:
It uses
X-OpenAI-Sessionin place ofMcp-Session-Id. Sorecord()returned early, the ability executed normally, and no event was ever emitted — silently. Gemini, which does send a header, worked fine, which is what made this look client-specific rather than like a detection bug.The fix
Detection becomes a union of signals:
Mcp-Session-Id/MCP-Protocol-VersionAccept: text/event-streamapplication/jsonandtext/event-streamon POST — so it is present exactly where the optional headers are not.?rest_route=fallback, andX-Original-URLfor proxies that rewriteREQUEST_URI(group.one's Varnish tier does).It still fails closed. This is only consulted from
wp_after_execute_ability, so the request is already known to be an ability execution; the only non-MCP callers there are our admin-ajax UI and the plain REST abilities route, and neither sendstext/event-streamnor posts to an/mcppath. Theonecom_abilities_is_mcp_requestfilter still overrides everything.Verified
Exercised against the captured headers plus the paths that must not be treated as MCP:
Companion MR
Onecom_Abilities_Trackingin the plugin repo (wp-in/one.com-wp-plugin-onecom-themes-plugins) has the byte-identical check and the same bug — in fact the ability in the captured log (onecom/create-post) is one of its abilities, so this PR alone does not fix the reported case. A matching MR is raised there.Notes
2.0.8-beta.3(in bothcomposer.jsonandMarketplace::VERSION) is needed to ship it.CHANGELOG.mdwill conflict trivially with Capitalise the MCP ability Mixpanel event name #129 depending on merge order — both add to## [Unreleased].master(missingfrontend/package-lock.json; pre-existing PHPCS violations), unrelated to this change.