Conversation
Port posthog-js#5074 to PostHogMCP. Custom dispatchers now correlate calls through an agent-carried conversation_id and a derived session id, like instrument() already does. - enable_conversation_id constructor option, on by default - prepare_tool_list() injects conversation_id and _mcp_instructions - prepare_tool_call() accepts a carried session_id and returns the resolved session_id and conversation_id - new prepare_tool_result() delivers a minted handle without mutating the original result - capture methods accept conversation_id Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
posthog-python Compliance ReportDate: 2026-09-25T14:47:48.858943+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
|
gesh
marked this pull request as draft
September 25, 2026 14:46
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| prepared: Any = result | ||
| delivered = False | ||
| if state.output_instructions: | ||
| prepared, delivered = mirror_instructions_into_structured_content( |
There was a problem hiding this comment.
Low: Shared results can expose another caller's conversation handle
For result objects without model_copy, this helper falls back to modifying structuredContent in place. If a tool returns a cached or shared custom result object, the first caller's handle remains there and is served to later callers, who can replay it to place their events in that caller's conversation and session. Copy the result before adding the metadata, or decline structured-content delivery when a safe copy cannot be made.
PR overviewThis pull request adds conversation support to custom MCP dispatchers by attaching conversation metadata to tool results. One security concern remains: when a custom tool reuses a cached or shared result object, conversation metadata may leak between callers. A later caller could reuse the exposed handle to place events into another caller’s conversation and session, though this depends on tools returning shared objects. Open issues (1)
Fixed/addressed: 0 · PR risk: 3/10 |
Contributor
|
[Medium risk] Adds conversation tracking to the MCP dispatcher API. The PR should not merge until the advertised list API works and tuple results can deliver newly minted handles. Reviews (1) · Last reviewed commit: "feat(mcp): support conversations in cust..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| requires the constructor's ``collect_feedback`` option — the enable switch | ||
| that gates detection in :meth:`prepare_tool_call`). Returns a new list; | ||
| that gates detection in :meth:`prepare_tool_call`). By default it also | ||
| injects the optional ``conversation_id`` argument and declares |
Contributor
There was a problem hiding this comment.
conversation_id to prepare_tool_list(), but the method does not accept it. A dispatcher following that API gets a TypeError before it can advertise its tools. Add the parameter and its intended behavior, or remove the API promise.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/mcp/posthog_mcp.py
Line: 423
Comment:
**Advertised argument is missing** The changeset says custom dispatchers can pass `conversation_id` to `prepare_tool_list()`, but the method does not accept it. A dispatcher following that API gets a `TypeError` before it can advertise its tools. Add the parameter and its intended behavior, or remove the API promise.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+924
to
+930
| if isinstance(result, dict): | ||
| return inject_prompt_back(result, conversation_id) | ||
| target = getattr(result, "root", result) | ||
| content = getattr(target, "content", None) | ||
| copy_model = getattr(target, "model_copy", None) | ||
| if not isinstance(content, list) or not callable(copy_model): | ||
| return result |
Contributor
There was a problem hiding this comment.
(content_list, structured) tuple without a usable structured channel, _inject_prompt_back() does not append the newly minted handle to the content list. prepare_tool_result() then drops the conversation ID, so later calls cannot echo the handle and remain grouped. Handle tuple content as the existing FastMCP instrumentation does.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/mcp/posthog_mcp.py
Line: 924-930
Comment:
**Tuple results lose handles** If a custom dispatcher returns a `(content_list, structured)` tuple without a usable structured channel, `_inject_prompt_back()` does not append the newly minted handle to the content list. `prepare_tool_result()` then drops the conversation ID, so later calls cannot echo the handle and remain grouped. Handle tuple content as the existing FastMCP instrumentation does.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.This branch has not been deployed
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
1 participant
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.
💡 Motivation and Context
instrument()servers already group those calls throughconversation_id.@posthog/mcpfixed this for JS custom dispatchers in feat(mcp): support conversations in custom dispatchers posthog-js#5074. This PR ports that change so both SDKs behave the same.PostHogMCP.capture_tool_call()had noconversation_idargument, so a dispatcher could not work around the gap by hand.Changes
PostHogMCP(enable_conversation_id=False)restores the previous behavior.prepare_tool_list()adds an optionalconversation_idargument and declares_mcp_instructionson compatible output schemas. It copies tools and does not change the originals.prepare_tool_call()takes a newsession_idfor a session the transport already carries. It returns the resolvedsession_idandconversation_id.prepare_tool_result()returnsPreparedToolResult(result, session_id, conversation_id). It appends a minted handle tocontentand mirrors the handle intostructuredContentwhen the output schema declares it.prepare_tool_result()returnsconversation_id=Noneand keeps the derived session.capture_*methods acceptconversation_id.Note
Differences from JS:
prepare_tool_result()also handlesCallToolResultmodels, including one wrapped in an MCP SDK 1.xServerResult.PreparedToolCallcarries a private_conversation_statefield. It survives copy and pickle across workers.Both SDKs share two limits. A replica that never served the listing and gets no
original_toolreadsconversation_idbut does not strip it. Preparing an already-prepared list turns ownership off.llm_modelalready behaves the same way, and the README tells dispatchers to passoriginal_tool.💚 How did you test it?
test_posthog_mcp.py. They cover schema injection, opt-out, application-owned fields, echo validation, carried sessions, delivery with and without mutation, pickle round trips, error results, and capture.CallToolResultandServerResultresults.mcp-major: v2job.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
claude-opus-5-5[1m]) at the assignee's direction. Skill used:writing-pr-descriptions._conversation_id,_output_instructionsandsessionhelpers were reused.ServerResultgot only the structured channel. This PR fixes that.🤖 Generated with Claude Code