fix(mcp): anchor virtual tools to conversations - #963
Conversation
posthog-python Compliance ReportDate: 2026-09-17 10:48:37 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
|
|
The PR appears safe to merge, with no new actionable findings or outstanding previous findings. Reviews (2) · Last reviewed commit: "fix(mcp): anchor virtual tools to conver..." |
| def _conversation_prompt_back(lifecycle: Any) -> Tuple[Any, bool]: | ||
| """``(prompt_back_block_or_None, delivered)`` for a virtual tool's reply. | ||
|
|
||
| A minted handle rides a prompt-back block so the agent can echo it on its | ||
| next call and keep the whole exchange in one session; an echoed one needs no | ||
| delivery. ``delivered`` gates stamping the handle on the event, so a handle | ||
| the agent never received is never recorded.""" | ||
| if lifecycle.conversation_id and lifecycle.minted_conversation_id: | ||
| block = mcp_types.TextContent( | ||
| type="text", text=build_prompt_back(lifecycle.conversation_id)["text"] | ||
| ) | ||
| return block, True | ||
| return None, bool(lifecycle.conversation_id) |
There was a problem hiding this comment.
This helper repeats the same mint/delivery logic in _instrument_lowlevel.py and _instrument_v2.py, recreating the risk of adapters drifting apart. It violates the repository’s “says everything once and only once” requirement, which must be satisfied before merging. Move the shared policy into one helper and leave only adapter-specific result wrapping in each adapter.
Context Used: Be direct and concise: state the issue, its impact, and the fix, with no preamble or praise. Do not comment on alphabetical sorting, trailing commas, or formatting. Linters catch these. Judge code by four simplicity rules: it passes all the tests, ex... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/mcp/_instrument_fastmcp.py
Line: 337-349
Comment:
**Duplicated prompt-back policy**
This helper repeats the same mint/delivery logic in `_instrument_lowlevel.py` and `_instrument_v2.py`, recreating the risk of adapters drifting apart. It violates the repository’s “says everything once and only once” requirement, which must be satisfied before merging. Move the shared policy into one helper and leave only adapter-specific result wrapping in each adapter.
**Context Used:** Be direct and concise: state the issue, its impact, and the fix, with no preamble or praise. Do not comment on alphabetical sorting, trailing commas, or formatting. Linters catch these. Judge code by four simplicity rules: it passes all the tests, ex... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
3bea84f to
31d5a92
Compare
8daec48 to
3aac206
Compare
31d5a92 to
24b200b
Compare
af09f57 to
dc9873c
Compare
dc9873c to
4cd42e9
Compare
Generated-By: PostHog Desktop Task-Id: e71933e1-34f5-4e2f-b1dc-da92ab68ff26
4cd42e9 to
536b049
Compare
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: router (cheap-first pass) + delegated reviewers as warranted Verdict: ✅ APPROVE (round 2 @ d12371b)No actionable defect was found. The shared virtual-result helper preserves text order and prompt-back behavior across all MCP adapters. Key findingsNone. ConvergenceBoth review rounds found the conversation and session behavior consistent. Reviewer summaries
Previous rounds (1)round 1 @ 536b049 — APPROVE: a second reviewer validated all adapter paths. Automated by QA Swarm — not a human review |
Generated-By: PostHog Desktop Task-Id: c9e11d68-ecf2-4b5a-aca5-5f8b15c973d0
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
Problem
With
enable_conversation_id=True, the virtual tools did not use conversation IDs. Their events could use a different session from related tool calls.Change
conversation_idfields to enabled virtual tools.This applies to both virtual tools, renamed tools, and MCP v1 and v2.
Tests
Created with PostHog Desktop