Conversation
posthog-python Compliance ReportDate: 2026-10-01T16:07:50.736753+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
|
[Medium risk] Adds safe input field name recording to MCP tool-call events. The PR should not merge until declared input names are captured for referenced standalone FastMCP schemas. Reviews (2) · Last reviewed commit: "fix(mcp): scope input schemas to tool ca..." |
2f602c9 to
57a3d9a
Compare
Generated-By: PostHog Desktop Task-Id: dda38523-5b8d-47b6-a11a-64cd59a812fe
Generated-By: PostHog Desktop Task-Id: dda38523-5b8d-47b6-a11a-64cd59a812fe
c8b2409 to
c520c54
Compare
lucasheriques
left a comment
There was a problem hiding this comment.
The no-values design and the analytics-key filtering hold up. Approving, with a few parity questions, since the same property would carry different data in Python and JS.
| assert properties["$mcp_input_keys"] == ["a", "b"] | ||
|
|
||
|
|
||
| async def test_lowlevel_does_not_reuse_a_listed_schema_for_input_names(): |
There was a problem hiding this comment.
question: Raw low-level servers always record ["[redacted]"], even after a listing, and this test locks that in. JS uses listed schemas per session here. I'm guessing 57a3d9a followed the same reasoning as _standalone_injected_parameters (listings can differ across requests). For names, though, a stale schema only risks recording a name the server declared, not a private one. Was there another concern?
| ] | ||
|
|
||
|
|
||
| def _aliases_used( |
There was a problem hiding this comment.
question: This differs from JS describeAliasesUsed, which takes the first alias present and never calls should_record_input_key. Here a too-long alias falls through to a later one, and the rule runs on the canonical name (absent from the input, with declared=True). Same input, different $mcp_input_aliases_used per SDK. Match JS, or land the stricter rule there too? Matching JS also removes recording_decisions.
| model_ours = data.tool_model_parameter_injected.get(name, model_injectable) | ||
| if _dispatch_can_differ(high_level): | ||
| model_ours = False | ||
| input_schema = None |
There was a problem hiding this comment.
question: With _dispatch_can_differ, any middleware (even FastMCP's LoggingMiddleware) redacts every name: ['a', 'b'] becomes ['[redacted]']. The v2 path handles middleware by resolving through server.list_tools() in the request. Could 1.x do the same for input names?
| return declared, injectable | ||
|
|
||
|
|
||
| def _input_schema_view( |
There was a problem hiding this comment.
question: Reading the code, $ref dereferencing only runs on the mcp 1.x path, while v2 passes raw tool.parameters. Would a root-$ref schema redact everything under mcp 2.x? If so, moving it into _declared_properties would cover every adapter.
| return properties if isinstance(properties, dict) else {} | ||
|
|
||
|
|
||
| def _tool_input_schema_v2(high_level: Any, name: str) -> Optional[Dict[str, Any]]: |
There was a problem hiding this comment.
nits:
- This and
_tool_input_schemain fastmcp are identical. Pass the schema tostart_tool_call_lifecyclein every adapter and drop thereplacecalls. - README: document
should_record_input_key, the 20/64 limits, and that$mcp_parametersstill carries raw names. - The helper tests could be one
parametrizetable, like JS'sit.each.
💡 Motivation and Context
MCP server owners need to measure which input names agents use. This must not expose argument values or unknown field names.
This PR ports the input-name and alias contract from
@posthog/mcp0.19.0 and 0.20.0. It adds$mcp_input_keys,$mcp_input_aliases_used, and a helper for custom dispatchers.💚 How did you test it?
uv run pytest -q posthog/test/mcp --timeout=30uv run ruff check posthog/mcp posthog/test/mcp/test_tool_input.py posthog/test/mcp/test_features_m4.pyuv run mypy --no-site-packages --config-file mypy.ini posthog/mcp | uv run mypy-baseline filtermake public_api_check📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Codex implemented the agreed Python port. The implementation keeps schemas isolated by session and redacts unknown names by default.
Created with PostHog Desktop