fix: restore CI to green (pin mcp<2, unmask and fix a racy test) - #64
Draft
HarleyCoops wants to merge 1 commit into
Draft
fix: restore CI to green (pin mcp<2, unmask and fix a racy test)#64HarleyCoops wants to merge 1 commit into
HarleyCoops wants to merge 1 commit into
Conversation
CI has been red on main since 2026-07-28 (run #93, fcad067). Three independent problems, all reproduced locally in a clean venv. 1. mcp 2.0.0 removed mcp.server.fastmcp. pyproject declared "mcp>=1.2" with no upper bound, so CI resolved 2.x and mythos/mcp_server.py failed to import FastMCP; FastMCP is not exposed from mcp.server either. The except ImportError guard re-raises as RuntimeError at module import time, which killed pytest collection and took the entire suite down rather than just the MCP test. Pin mcp>=1.2,<2 in both the dev and mcp extras; migrating to the 2.x server API is separate work. 2. tests/test_backends.py built an HTTPError with a fake fp lacking close(). HTTPError wraps fp in a finalizer that calls close() on collection, so the AttributeError surfaced as an unraisable exception attributed to whatever unrelated test was running at GC time. Give the fixture a close(). 3. With that noise removed, test_codex_streams_jsonl_events_to_trace_and_sink failed on a real race. CodexCli.run streams stdout to the trace from a daemon thread that it only joins after process.wait() returns, so nothing orders the reader against the fake wait(). The test read the trace once and passed only when it won the race — it did in isolation, not under full-suite load. Poll for the streamed event instead, preserving the assertion's intent that the trace fills during the run rather than after it. Production code is unchanged. Full suite: 106 passed, three consecutive clean runs. compileall, CLI help, /health, and MCP tool registration all verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQpUQYoSDgvMZkVF2NbGiq
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
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.
CI has been red on
mainsince 2026-07-28 — run #93 atfcad0674fails the "Run unit tests" step on both Python versions in about two seconds. Three independent problems were stacked behind that, each hiding the next. All reproduced locally in a clean venv withpip install -e ".[dev]".1. mcp 2.0.0 removed
mcp.server.fastmcppyproject.tomldeclaredmcp>=1.2with no upper bound, so CI resolved mcp 2.0.0:FastMCPis not exposed frommcp.serverin 2.x either — the subpackage is gone. Theexcept ImportErrorguard atmythos/mcp_server.py:28-34re-raises asRuntimeErrorat module import time, so this killed pytest collection and took the whole suite down rather than just the MCP test.The error message sends you toward
pip install -e '.[mcp]', which is a red herring: thedevextra already includesmcp. The problem is the version, not a missing extra.Pinned
mcp>=1.2,<2in both thedevandmcpextras. Migratingmythos/mcp_server.pyto the 2.x server API is real work and deliberately not attempted here.2. A fake
fpwithoutclose()corrupted unrelated teststests/test_backends.pybuilds anHTTPErrorwithfp=ResponseBody().HTTPErrorwrapsfpin a finalizer that callsclose()during garbage collection, and the fixture had noclose(). The resultingAttributeError: 'ResponseBody' object has no attribute 'close'surfaced as aPytestUnraisableExceptionWarningattributed to whatever unrelated test happened to be running when the collector got to it — which is why it presented as a failure intest_sol_staged_pipeline.py.3. The failure that was hiding underneath is a real race
With the GC noise removed,
test_codex_streams_jsonl_events_to_trace_and_sinkfailed on its own merits:CodexCli.runstreams stdout into the trace from a daemon thread (sol/client.py:143-145) that it only joins afterprocess.wait()returns (sol/client.py:159-160). Nothing orders that reader thread against the fakewait(). The test read the trace exactly once and passed only when it won the race — which it did in isolation, but not under full-suite load.Replaced the single read with a bounded poll. That preserves what the assertion is actually for — the trace fills during the run, not after it — without racing the reader.
The production code is correct as written and is unchanged by this PR.
run()joins both reader threads before touching the trace, so callers always see a complete file. Only the test was wrong.Verification
python -m compileall -q mythosmath-to-manim --version,serve-mcp --help/healthreturns{'status': 'ok', 'version': '1.1.0'}Scope
Two test files and one dependency constraint. No production code, no behavior change.
Generated by Claude Code