fix: move MCP cleanup from Thread.__exit__ to orchestrator.run()

[zzstoatzz]

Summary

Fixes the runtime error from #1260 where calling run_sync() from Thread.__exit__ while already in an async context caused issues.

Problem

Thread.__exit__ is sync but MCP cleanup is async. When the orchestrator (which is async) exits the Thread context, calling run_sync(cleanup_thread_mcp_servers()) would:

1. Detect an event loop is already running
2. Spawn a new thread via run_sync_in_thread()
3. If interrupted, cause ContextVar corruption and ValueError: Token was created in a different Context

# This pattern caused the error:
async def run(self):
    with self.thread:  # Thread.__exit__ would call run_sync() from async context
        ...

Solution

Move MCP cleanup from Thread.__exit__ (sync) to orchestrator.run()'s finally block (async), where it can be properly awaited:

async def run(self):
    try:
        with self.thread:
            ...
    finally:
        _current_orchestrator.reset(token)
        # Clean up after Thread exits, only if outermost context
        if get_current_thread() is None:
            await cleanup_thread_mcp_servers()

Test plan

• Updated test test_mcp_cleanup_happens_in_orchestrator_not_thread to reflect new behavior
• Added test_cleanup_from_async_context_works regression test
• All 14 MCP integration tests pass
• All 67 engine/task tests pass

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a runtime error where MCP cleanup was attempted from a synchronous context (Thread.__exit__) while already in an async context, causing ContextVar corruption. The solution moves MCP server cleanup from Thread.__exit__ to the orchestrator's async finally block.

Key changes:

• Removed sync _cleanup_mcp_servers() method from Thread.__exit__ that was causing async/sync context conflicts
• Added async cleanup logic to orchestrator's finally block with proper nested context handling
• Updated and added tests to verify cleanup happens in orchestrator, not Thread

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/marvin/thread.py	Removed _cleanup_mcp_servers() method and cleanup logic from Thread.__exit__, simplifying it to only handle context reset
src/marvin/engine/orchestrator.py	Added MCP cleanup in orchestrator's finally block with check for outermost Thread context; imported cleanup_thread_mcp_servers and get_current_thread
tests/agents/test_mcp_integration.py	Updated test to verify cleanup happens in orchestrator not Thread; added regression test for async context cleanup

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.