fix(mcp): isolate pending capture tasks - #815
Conversation
posthog-python Compliance ReportDate: 2026-08-04 09:13:11 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
|
|
@PostHog/team-mcp-analytics im not sure if this was intended or not, but i found out while reviewing some other stuff |
|
Reviews (1): Last reviewed commit: "test(mcp): await background capture futu..." | Re-trigger Greptile |
gesh
left a comment
There was a problem hiding this comment.
Looks good to me!
cc @lucasheriques as he might be more familiar with this implementation and whether it was intentional
lucasheriques
left a comment
There was a problem hiding this comment.
approving. mechanism is sound.
verified: asyncio.Task isn't a concurrent.futures.Future subclass, so the two drain filters are disjoint. locking is deadlock free, _tasks_lock is released before both add_done_callback and concurrent.futures.wait. owner and loop identity match end to end through instrument(). background=True for PostHogMCP fixes a real gap, covered by test_posthog_mcp_sync_flush_drains_capture_from_async_host.
two non blocking comments below, neither changes behaviour.
the one that matters: flush() is what gets trailing captures over the line before the loop closes. if it silently stops draining, a host that does tool call, flush(), exit loses every event, and nothing in the suite would catch that.
💡 Motivation and Context
MCP capture tasks were tracked globally, so flushing one analytics instance could wait for another instance's captures. Async drains could also encounter tasks created on another event loop.
Track each capture task by its lifecycle owner and event loop. Async analytics now drain only their own tasks on the current loop, while
PostHogMCPconsistently schedules and drains its captures on the shared background loop.💚 How did you test it?
uv run --extra test pytest -q posthog/test/mcp(145 passed)uv run ruff check posthog/mcp/__init__.py posthog/mcp/_instrumentation.py posthog/mcp/posthog_mcp.py posthog/test/mcp/_helpers.py posthog/test/mcp/test_review_fixes.py posthog/test/mcp/test_pending_tasks.pyuv run ruff format --check posthog/mcp/__init__.py posthog/mcp/_instrumentation.py posthog/mcp/posthog_mcp.py posthog/test/mcp/_helpers.py posthog/test/mcp/test_review_fixes.py posthog/test/mcp/test_pending_tasks.pygit diff --check📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi implemented the human-directed fix from autoreview report 38. The selected approach scopes pending capture work by owner and event loop, and keeps the synchronous
PostHogMCPlifecycle on the shared background loop soflush()andshutdown()can drain it safely. The generated patch changeset describes the user-visible fix.This agent-assisted change requires human review before merge.