fix(mcp): isolate orphan-reap sleep mock - #27676
Closed
bbernstein616 wants to merge 1 commit into
Closed
Conversation
… threads
Patching ``tools.mcp_tool.time.sleep`` (the previous "fix") is identical to
patching the global ``time.sleep`` — ``tools.mcp_tool.time`` *is* the time
module, and ``sleep`` is a module attribute. On CI under pytest-xdist the
test_kill_orphaned_uses_sigkill_when_available mock captured 207k calls
because worker/heartbeat threads kept calling ``time.sleep(1)`` while the
mock was active, blowing up ``assert_called_once_with(2)``.
Introduce a private ``_orphan_reap_sleep`` indirection in tools.mcp_tool and
have the test patch that symbol instead. The wrapper is unique to mcp_tool
so unrelated background sleepers can't pollute the mock's call list.
Verified with a stress harness (background thread spamming time.sleep(1))
that previously caused thousands of phantom call(1)s — mock now sees only
the intended call(2).
Tests: tests/tools/test_mcp_stability.py — 16/16 pass.
tests/tools/ -k mcp — 399/399 pass.
3 tasks
alt-glitch
added
type/test
Author
|
Hermes watchdog review: diff is scoped to tools/mcp_tool.py and tests/tools/test_mcp_stability.py; old polluted PR #27656 is closed. Independent verification on Bryan's machine using scripts/run_tests.sh: tests/tools/test_mcp_stability.py -> 16 passed; tests/tools/ -k mcp -> 399 passed (4 existing pytest.mark.ssh warnings). I attempted to merge after verification, but GitHub denied mergePullRequest for bbernstein616, so this is ready for someone with merge rights. |
Author
|
Watchdog verification update:
Ready to merge by a maintainer. No force-push, no gateway restart. |
Author
|
Closing from my fork while moving this work private/protected. |
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
2 participants
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.
Summary
Why
PR #27656 currently includes an unrelated nous_portal security commit because it was based on the wrong branch. This is the same MCP fix rebased cleanly onto current origin/main, with only tools/mcp_tool.py and tests/tools/test_mcp_stability.py changed.
Verification
Supersedes #27656.