Skip to content

Commit 236a73e

Browse files
gesheli-r-ph
authored andcommitted
feat(mcp): resolve fresh low-level tool ownership (#1009)
* feat(mcp): resolve fresh low-level tool ownership Add resolve_original_tool for raw MCP SDK 1.x and 2.x servers. Prefer ownership learned from a served listing and keep unknown ownership fail-open. Generated-By: PostHog Desktop Task-Id: db1a8a08-fe03-43b6-9e49-705e8b1f375e * fix(mcp): preserve listed argument ownership Generated-By: PostHog Desktop Task-Id: db1a8a08-fe03-43b6-9e49-705e8b1f375e * fix(mcp): preserve arguments for unknown schemas Generated-By: PostHog Desktop Task-Id: db1a8a08-fe03-43b6-9e49-705e8b1f375e * fix(mcp): preserve low-level tool schemas Generated-By: PostHog Desktop Task-Id: db1a8a08-fe03-43b6-9e49-705e8b1f375e (cherry picked from commit 127009d)
1 parent d01cb33 commit 236a73e

17 files changed

Lines changed: 827 additions & 63 deletions
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
pypi/posthog: minor
3+
---
4+
5+
Add `resolve_original_tool` for low-level MCP servers. Fresh server instances can now remove PostHog-owned arguments before strict tool validation.
6+
7+
Raw low-level servers also remove PostHog-owned arguments after `tools/list`. A tool-owned `context` remains tool data.

‎posthog/mcp/_argument_ownership.py‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
"""Resolve ownership of arguments that PostHog adds to MCP tool schemas."""
2+
3+
from __future__ import annotations
4+
5+
import inspect
6+
from collections.abc import Mapping
7+
from typing import Any, Dict, FrozenSet, Optional, Tuple
8+
9+
from ._context_parameters import is_context_enabled, schema_has_param
10+
from ._model_parameters import is_capture_model_enabled
11+
from .logger import log
12+
13+
_COMPLEX_SCHEMA_KEYS = ("$ref", "oneOf", "allOf", "anyOf")
14+
15+
16+
def analytics_owned_parameters(
17+
options: Any,
18+
input_schema: Any,
19+
) -> FrozenSet[str]:
20+
"""Return the enabled arguments that PostHog can add to this schema."""
21+
enabled = set()
22+
if is_context_enabled(options.context):
23+
enabled.add("context")
24+
if options.enable_conversation_id:
25+
enabled.add("conversation_id")
26+
if is_capture_model_enabled(options.capture_model):
27+
enabled.add("llm_model")
28+
29+
if isinstance(input_schema, dict) and any(
30+
input_schema.get(key) for key in _COMPLEX_SCHEMA_KEYS
31+
):
32+
return frozenset()
33+
return frozenset(
34+
name for name in enabled if not schema_has_param(input_schema, name)
35+
)
36+
37+
38+
def cache_listed_tool_ownership(
39+
data: Any, tool: Any, *, schema_attribute: str
40+
) -> FrozenSet[str]:
41+
"""Cache ownership from the host schema before PostHog changes it."""
42+
name = getattr(tool, "name", None)
43+
if not isinstance(name, str):
44+
return frozenset()
45+
schema = getattr(tool, schema_attribute, None)
46+
ownership = analytics_owned_parameters(data.options, schema)
47+
data.tool_analytics_parameter_ownership[name] = ownership
48+
if isinstance(schema, dict):
49+
data.tool_input_schemas[name] = schema
50+
else:
51+
data.tool_input_schemas.pop(name, None)
52+
return ownership
53+
54+
55+
async def resolve_lowlevel_tool_ownership(
56+
data: Any, name: str
57+
) -> Tuple[Optional[FrozenSet[str]], Optional[Dict[str, Any]]]:
58+
"""Resolve one raw tool. A served listing on this instance has priority."""
59+
if name in data.tool_analytics_parameter_ownership:
60+
return (
61+
data.tool_analytics_parameter_ownership[name],
62+
data.tool_input_schemas.get(name),
63+
)
64+
65+
resolver = data.options.resolve_original_tool
66+
if resolver is None:
67+
return None, None
68+
69+
try:
70+
descriptor = resolver(name)
71+
if inspect.isawaitable(descriptor):
72+
descriptor = await descriptor
73+
if descriptor is None:
74+
return None, None
75+
schema = _descriptor_input_schema(descriptor)
76+
if not isinstance(schema, dict):
77+
log(
78+
f"Warning: resolve_original_tool failed for tool {name!r}: "
79+
"resolver returned no usable input schema"
80+
)
81+
return None, None
82+
return analytics_owned_parameters(data.options, schema), schema
83+
except Exception as error: # noqa: BLE001 - analytics must not break dispatch
84+
log(f"Warning: resolve_original_tool failed for tool {name!r}: {error}")
85+
return None, None
86+
87+
88+
def _descriptor_input_schema(descriptor: Any) -> Any:
89+
"""Read MCP 1.x, MCP 2.x, and dictionary tool descriptors."""
90+
if isinstance(descriptor, Mapping):
91+
if "inputSchema" in descriptor:
92+
return descriptor["inputSchema"]
93+
return descriptor.get("input_schema")
94+
95+
schema = getattr(descriptor, "input_schema", None)
96+
if schema is not None:
97+
return schema
98+
return getattr(descriptor, "inputSchema", None)

‎posthog/mcp/_context_parameters.py‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@ def add_context_parameter_to_schema(
4848
"""Return a new JSON Schema dict with a ``context`` string property added.
4949
5050
Returns the input unchanged (logging a warning) for schemas that already
51-
define ``context`` or use ``oneOf``/``allOf``/``anyOf``. ``required`` controls
52-
whether ``context`` is added to the schema's ``required`` list — pass ``False``
53-
where the advertised schema is also used to validate inbound calls (the
54-
low-level server), so a call omitting ``context`` is not rejected."""
51+
define ``context`` or use ``$ref``/``oneOf``/``allOf``/``anyOf``. ``required``
52+
controls whether ``context`` is added to the schema's ``required`` list —
53+
pass ``False`` where the advertised schema is also used to validate inbound
54+
calls (the low-level server), so a call omitting ``context`` is not rejected.
55+
"""
5556
schema = input_schema
5657

5758
if (
@@ -64,9 +65,10 @@ def add_context_parameter_to_schema(
6465
)
6566
return schema
6667

67-
if schema and (schema.get("oneOf") or schema.get("allOf") or schema.get("anyOf")):
68+
if schema and any(schema.get(key) for key in ("$ref", "oneOf", "allOf", "anyOf")):
6869
log(
69-
f'WARN: Tool "{tool_name}" has complex schema (oneOf/allOf/anyOf). Skipping context injection.'
70+
f'WARN: Tool "{tool_name}" has complex schema '
71+
"($ref/oneOf/allOf/anyOf). Skipping context injection."
7072
)
7173
return schema
7274

‎posthog/mcp/_conversation_id.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ def add_conversation_id_to_schema(
3535
input_schema: Optional[Dict[str, Any]], tool_name: str = "unknown"
3636
) -> Optional[Dict[str, Any]]:
3737
"""Return a new JSON Schema with an optional ``conversation_id`` string property.
38-
Skips schemas that already define it or use ``oneOf``/``allOf``/``anyOf``."""
38+
Skips schemas that already define it or use
39+
``$ref``/``oneOf``/``allOf``/``anyOf``.
40+
"""
3941
schema = input_schema
4042
if (
4143
schema
@@ -48,7 +50,7 @@ def add_conversation_id_to_schema(
4850
f"WARN: Tool \"{tool_name}\" already has '{CONVERSATION_ID_PARAM_NAME}'. Skipping injection."
4951
)
5052
return schema
51-
if schema and (schema.get("oneOf") or schema.get("allOf") or schema.get("anyOf")):
53+
if schema and any(schema.get(key) for key in ("$ref", "oneOf", "allOf", "anyOf")):
5254
log(
5355
f'WARN: Tool "{tool_name}" has complex schema. Skipping conversation_id injection.'
5456
)

‎posthog/mcp/_instrument_lowlevel.py‎

Lines changed: 57 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@
2222

2323
import mcp.types as mcp_types
2424

25+
from ._argument_ownership import (
26+
cache_listed_tool_ownership,
27+
resolve_lowlevel_tool_ownership,
28+
)
2529
from ._context_parameters import is_context_enabled, schema_has_param
2630
from ._conversation_id import build_prompt_back
2731
from ._event_types import MCPAnalyticsEventType
2832
from ._instrumentation import (
2933
advertised_tool_names,
3034
apply_virtual_tool_injection,
3135
collect_listed_tools,
36+
copy_tools_list_result,
3237
extract_tools,
3338
is_first_listing_page,
3439
mutate_tool_schema,
@@ -54,9 +59,11 @@
5459

5560

5661
def instrument_low_level(server: Any, data: MCPAnalyticsData) -> None:
57-
"""Instrument a raw ``mcp.server.Server``. ``context`` is injected as an
58-
optional schema property and NOT stripped — that schema is also the call's
59-
validation schema, and a typical ``(name, arguments)`` handler ignores extra keys."""
62+
"""Instrument a raw ``mcp.server.Server``.
63+
64+
The adapter removes arguments that a listing or resolver proves PostHog
65+
owns. Unknown arguments pass through unchanged.
66+
"""
6067
data.server_name = getattr(server, "name", None)
6168
data.server_version = getattr(server, "version", None)
6269
_wrap_call_tool(server, data, strip_injected=False)
@@ -186,11 +193,21 @@ def _wrap_call_tool(
186193
async def handler(req: Any) -> Any:
187194
name = req.params.name
188195
arguments = dict(req.params.arguments or {})
189-
strip, model_ours, input_schema = (
190-
await _standalone_ownership(data, high_level, name, req.params.meta)
191-
if strip_injected
192-
else (set(), data.tool_model_parameter_injected.get(name), None)
193-
)
196+
parameter_ownership = None
197+
if strip_injected:
198+
strip, model_ours, input_schema = await _standalone_ownership(
199+
data, high_level, name, req.params.meta
200+
)
201+
else:
202+
parameter_ownership, input_schema = await resolve_lowlevel_tool_ownership(
203+
data, name
204+
)
205+
strip = set(parameter_ownership or ())
206+
model_ours = (
207+
"llm_model" in parameter_ownership
208+
if parameter_ownership is not None
209+
else data.tool_model_parameter_injected.get(name)
210+
)
194211
client_name, client_version = _client_info(server)
195212
protocol_version = _protocol_version(server)
196213
mcp_session_id = _mcp_session_id(server)
@@ -213,6 +230,7 @@ async def handler(req: Any) -> Any:
213230
protocol_version=protocol_version,
214231
extra={"session_id": mcp_session_id, "ctx": _request_context(server)},
215232
input_schema=input_schema,
233+
analytics_owned_parameters=parameter_ownership,
216234
)
217235

218236
if lifecycle.is_missing_capability and (
@@ -327,6 +345,7 @@ async def handler(req: Any) -> Any:
327345

328346

329347
def _inject_tool_schemas(
348+
server: Any,
330349
data: MCPAnalyticsData,
331350
tools: list,
332351
*,
@@ -345,6 +364,7 @@ def _inject_tool_schemas(
345364
verdicts: Dict[str, bool] = {}
346365
for tool in tools:
347366
schema = getattr(tool, "inputSchema", None)
367+
cache_listed_tool_ownership(data, tool, schema_attribute="inputSchema")
348368
mutate_tool_schema(
349369
data,
350370
tool,
@@ -363,6 +383,11 @@ def _inject_tool_schemas(
363383
# Which one dispatches is unknown, so the strip fails closed.
364384
data.tool_model_parameter_injected[tool.name] = False
365385

386+
cache = getattr(server, "_tool_cache", None)
387+
if isinstance(cache, dict):
388+
for tool in tools:
389+
cache[tool.name] = tool
390+
366391

367392
def _wrap_list_tools(
368393
server: Any,
@@ -404,34 +429,36 @@ async def probe_raw_tool_names(_ctx: Any = None) -> Optional[Set[str]]:
404429
"registering your handlers."
405430
)
406431
return None
407-
result = await original(mcp_types.ListToolsRequest(method="tools/list"))
432+
result = copy_tools_list_result(
433+
await original(mcp_types.ListToolsRequest(method="tools/list"))
434+
)
408435
tools = extract_tools(result)
409-
# `original` is usually the SDK's own list_tools decorator, which rebuilds
410-
# `Server._tool_cache` from these un-injected schemas every time it runs.
411-
# That cache is what the SDK validates real tool arguments against, so
412-
# without re-injecting here the next real call is rejected for sending the
413-
# `context` we advertised. Same reason the `req is None` branch below
414-
# injects.
436+
# Resolve ownership from the host listing. Apply injection only to the
437+
# copy so a shared host descriptor remains original.
415438
_inject_tool_schemas(
416-
data, tools, context_required=context_required, high_level=high_level
439+
server,
440+
data,
441+
tools,
442+
context_required=context_required,
443+
high_level=high_level,
417444
)
418445
return advertised_tool_names(tools)
419446

420447
data.raw_tool_names_probe = probe_raw_tool_names
421448

422449
async def handler(req: Any) -> Any:
423450
# The server calls the handler with None to populate its tool cache.
424-
# Skip analytics there — but still inject, because that cache is the
425-
# schema the SDK validates calls against. This adapter advertises
426-
# `context`/`conversation_id` without stripping them, so a cache built
427-
# from un-injected schemas rejects the very arguments we told the agent
428-
# to send ("Additional properties are not allowed") on any tool with
429-
# `additionalProperties: false`.
451+
# Skip analytics there, but return the same injected schema as a client
452+
# listing.
430453
if req is None:
431-
result = await original(req)
454+
result = copy_tools_list_result(await original(req))
432455
tools = extract_tools(result)
433456
_inject_tool_schemas(
434-
data, tools, context_required=context_required, high_level=high_level
457+
server,
458+
data,
459+
tools,
460+
context_required=context_required,
461+
high_level=high_level,
435462
)
436463
return result
437464

@@ -464,7 +491,7 @@ async def handler(req: Any) -> Any:
464491

465492
start = time.monotonic()
466493
try:
467-
result = await original(req)
494+
result = copy_tools_list_result(await original(req))
468495
except Exception as error:
469496
await lifecycle.record_error(error, (time.monotonic() - start) * 1000)
470497
raise
@@ -481,7 +508,11 @@ async def handler(req: Any) -> Any:
481508
)
482509

483510
_inject_tool_schemas(
484-
data, tools, context_required=context_required, high_level=high_level
511+
server,
512+
data,
513+
tools,
514+
context_required=context_required,
515+
high_level=high_level,
485516
)
486517

487518
result = apply_virtual_tool_injection(

0 commit comments

Comments
 (0)