Skip to content

Conversation

@cagataycali
Copy link
Member

Description

Fixes a failing integration test test_load_tools_without_agent in the MCP client test suite.

Problem

The test expected that not explicitly passing the agent parameter would result in no agent being available:

result = agent.tool.mcp_client(
    action="load_tools",
    connection_id="test_connection"
    # Note: agent parameter is not provided  <-- Wrong assumption!
)
assert "agent instance is required" in result["content"][0]["text"]  # ❌ Fails

Actual Error: "Connection 'test_connection' not found"

Root Cause

When using agent.tool.mcp_client(...) (direct tool calling), the SDK automatically injects the agent into the tool invocation via invocation_state. This is expected behavior.

From sdk-python/src/strands/tools/executors/_executor.py:137-140:

invocation_state.update(
    {
        "agent": agent,  # <-- SDK always injects this
        "model": agent.model,
        ...
    }
)

Solution

Changed the test to verify connection validation instead (the actual error path when using the direct tool calling convention):

def test_load_tools_nonexistent_connection(self, agent):
    """Test load_tools action with nonexistent connection."""
    result = agent.tool.mcp_client(
        action="load_tools",
        connection_id="nonexistent_connection"
    )
    assert "Connection 'nonexistent_connection' not found" in result["content"][0]["text"]

Note

The unit test (tests/test_mcp_client.py:616) still properly covers the "no agent" case by calling mcp_client() directly (not through agent.tool.x).

Related Issues

  • Investigation tracked in cagataycali/strands-coder#94

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • Verified fix locally - test now passes
  • All existing tests pass

Checklist

  • I have read the CONTRIBUTING document
  • I have added tests that prove my fix is effective
  • My changes generate no new warnings

Automated by strands-coder 🦆

@cagataycali
Copy link
Member Author

✅ Test Fix Ready for Review

This PR fixes an integration test for load_tools with agent injection and all CI checks are passing!

Status Summary

  • ✅ CI: All checks SUCCESS
  • ✅ Mergeable: No conflicts
  • ✅ Test-only change

What This Fixes

Corrects the integration test for load_tools functionality when agents are injected, ensuring test accuracy.

Impact

  • Test correctness improvement
  • No production code changes
  • Zero risk

Ready for maintainer review! 🦆

@cagataycali cagataycali requested a review from mkmeral January 15, 2026 12:00
@mkmeral
Copy link
Contributor

mkmeral commented Jan 15, 2026

The failing test: test_load_tools_without_agent expected error "agent instance is required" but got "Connection 'test_connection' not found"

Why it ever passed: Test was written July 28, 2025 when SDK didn't auto-inject agent into invocation_state.

When it broke: November 24, 2025 - SDK PR #1213 (commit aaf9715) added automatic agent injection. Released in SDK v1.19.0 on Dec 3, 2025.

The breaking change was in sdk-python, not tools:

# src/strands/tools/executors/_executor.py
invocation_state.update({
+   "agent": agent,  # <-- Added in PR #1213
    "model": agent.model,
    ...
})

@mkmeral mkmeral merged commit c106e7e into strands-agents:main Jan 15, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants