fix(agent): skip failed server capability results#686
Conversation
📝 WalkthroughWalkthroughThe PR fixes a silent failure in ChangesException Filtering in Multi-Server Capability Aggregation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/agents/test_agent_tasks_concurrency.py (1)
163-163: ⚡ Quick winUse consistent test marker.
The test uses
@pytest.mark.asynciowhile other tests in this file use@pytest.mark.anyio(lines 68, 102). For consistency with the existing test suite, use@pytest.mark.anyio.📝 Proposed fix
-@pytest.mark.asyncio +@pytest.mark.anyio async def test_get_capabilities_filters_failed_servers(monkeypatch):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/agents/test_agent_tasks_concurrency.py` at line 163, Replace the inconsistent test marker by changing the decorator pytest.mark.asyncio to pytest.mark.anyio for the test that currently uses `@pytest.mark.asyncio` so it matches the rest of the file; update the decorator on the test function (replace pytest.mark.asyncio with pytest.mark.anyio) to ensure consistent use across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/agents/test_agent_tasks_concurrency.py`:
- Line 163: Replace the inconsistent test marker by changing the decorator
pytest.mark.asyncio to pytest.mark.anyio for the test that currently uses
`@pytest.mark.asyncio` so it matches the rest of the file; update the decorator on
the test function (replace pytest.mark.asyncio with pytest.mark.anyio) to ensure
consistent use across tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7542d079-5ab5-4d65-8fb1-623ec81fe0ed
📒 Files selected for processing (2)
src/mcp_agent/agents/agent.pytests/agents/test_agent_tasks_concurrency.py
Summary
Fixes
get_capabilities_taskwhen fetching capabilities for all servers and one server fails. The previous implementation usedasyncio.gather(..., return_exceptions=True)and then zipped the raw results into the returned dict, which let exception objects masquerade asServerCapabilitiesvalues.This now logs failed server capability lookups and omits those failed entries from the aggregate result, keeping the return value type-consistent for the servers that did respond.
Tests
uv run ruff check src/mcp_agent/agents/agent.py tests/agents/test_agent_tasks_concurrency.pyuv run pytest tests/agents/test_agent_tasks_concurrency.pyNote: pytest emits existing deprecation warnings from pytest-asyncio/Pydantic, but the targeted tests pass.
Fixes #671
Summary by CodeRabbit