fix(aws-strands): make the template MCP-client warning able to fire - #2615
Merged
Conversation
The constructor's MCP warning scanned `_templateFields.tools`, which is
`agent.tools.slice()` from the template Agent, for an entry that looks like
a bare `McpClient`. Measured against `@strands-agents/sdk` 1.1.0 that list
can never hold one: the SDK's `flattenTools` routes an `McpClient` into a
separate internal client list and only non-client entries reach the tool
registry that `Agent.tools` reads from. `new Agent({ tools: [client] })`
leaves `agent.tools` empty and the client in `_mcpClients`, so the loop
iterated a list the thing it was looking for cannot appear in.
The comment's stated reason was wrong too. The SDK does not resolve a
connected client's tools at construction time; `Agent.initialize()`, which
runs on the first invocation, is what calls `client.listTools()` and
registers the result, and `McpClient.listTools()` connects lazily. So
connected-versus-unconnected makes no difference. The real distinction is
that the adapter clones the resolved registry before `initialize()` has run,
which is what the README already says.
The warning is worth keeping: MCP tools silently missing from every
per-thread agent is exactly the kind of failure a construction-time line
should name. So it now reads the template's client list instead, through
`_readTemplateField`, which tries the public name before the underscore one
and yields `undefined` rather than throwing if neither is present. The
message drops the connected-versus-unconnected framing and tells the caller
to resolve the tools and drop the client from `tools`, which also keeps a
caller who passed both from being warned forever.
Tests cover both directions against a real `Agent`: an `McpClient` in
`tools` warns (and `agent.tools` is pinned empty in the same test, so the
assertion is about the warning and not about a client that resolved), and a
template carrying an ordinary resolved tool stays quiet. The first fails
before this change.
Contributor
Python Preview PackagesVersion
Install with uvAdd the TestPyPI index to your [[tool.uv.index]]
name = "testpypi"
url = "https://test.pypi.org/simple/"
explicit = trueThen install the packages you need: # Core SDK
uv add 'ag-ui-protocol==0.0.0.dev1788422932' --index testpypi
# Integrations (each already depends on the matching ag-ui-protocol preview)
uv add 'ag-ui-langgraph==0.0.0.dev1788422932' --index testpypi
uv add 'ag-ui-crewai==0.0.0.dev1788422932' --index testpypi
# NOTE: ag-ui-agent-spec depends on pyagentspec (git-only, not on PyPI).
# You will need to install pyagentspec separately from its git repo.
uv add 'ag-ui-agent-spec==0.0.0.dev1788422932' --index testpypi
uv add 'ag_ui_adk==0.0.0.dev1788422932' --index testpypi
uv add 'ag_ui_strands==0.0.0.dev1788422932' --index testpypiInstall with pippip install \
--index-url https://test.pypi.org/simple/ \
--extra-index-url https://pypi.org/simple/ \
ag-ui-protocol==0.0.0.dev1788422932
Commit: 6436619 |
@ag-ui/a2a-middleware
@ag-ui/a2ui-middleware
@ag-ui/event-throttle-middleware
@ag-ui/mcp-apps-middleware
@ag-ui/mcp-middleware
@ag-ui/a2a
@ag-ui/adk
@ag-ui/ag2
@ag-ui/agno
@ag-ui/aws-strands
@ag-ui/claude-agent-sdk
@ag-ui/claude-managed-agents
@ag-ui/crewai
@ag-ui/langchain
@ag-ui/langgraph
@ag-ui/llamaindex
@ag-ui/mastra
@ag-ui/pydantic-ai
@ag-ui/vercel-ai-sdk
@ag-ui/watsonx
@ag-ui/a2ui-toolkit
create-ag-ui-app
@ag-ui/client
@ag-ui/core
@ag-ui/encoder
@ag-ui/proto
commit: |
AlemTuzlak
approved these changes
Sep 3, 2026
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.
The problem
The TypeScript bridge's constructor carried a warning meant to catch an
McpClienthanded straight into the templateAgent'stoolsarray. It could never fire, and its comment gave the wrong reason for the behaviour it was warning about.The check was unreachable. It scanned
this._templateFields.tools, which isagent.tools.slice()off the template Agent, for an entry with aconnectmethod and noname. Measured against the installed@strands-agents/sdk1.1.0, that list cannot hold a client:flattenTools(dist/src/agent/agent.js) routes anMcpClientinto a separate_mcpClientsarray and passes only the rest into theToolRegistrythatAgent.toolsreads from. Probed directly,new Agent({ systemPrompt: "x", tools: [new McpClient({ transport })] })givesagent.tools.length === 0andagent._mcpClients.length === 1.The comment was wrong. It said Strands resolves a connected client's tools into
agent.toolsat construction time, so only an unconnected one would show up as a bare client. The SDK does no such thing at construction.Agent.initialize(), which is async and runs on the first invocation, is what callsclient.listTools()and registers the result;McpClient.listTools()connects lazily on its own. Connected versus unconnected makes no difference. The real distinction is that the adapter clones the resolved registry beforeinitialize()has ever run, which is what this package's README already explains.The change
The warning is kept, because the failure it names is real and silent: MCP tools missing from every per-thread agent, with nothing at construction time to say so. It now reads the template's MCP client list instead of its tool list, through the existing
_readTemplateFieldhelper, which tries the public name before the underscore one and returnsundefinedrather than throwing when neither is there.The message drops the connected-versus-unconnected framing, says how many client entries are in play, and tells the caller to resolve the tools and then drop the client from
tools. That last part matters for a caller who passed both the client and its resolved tools, who would otherwise be warned on every construction with nothing left to fix.The comment now states the actual mechanism.
Tests
Both directions, in
agent-config-forwarded-real-sdk.test.tsalongside the rest of the template-forwarding coverage, against a genuineAgent(the routing is aninstanceof McpClientcheck, so a fabricated template could not exercise it):McpClientintoolsproduces exactly one warning namingMcpClient. The same test pinstemplate.toolsas empty first, so the assertion is about the warning firing and not about a client that happened to resolve. This test fails onmainand passes with the change.The client is constructed with a stub transport cast to
never:McpClient's constructor only stores the transport, so the suite needs no live MCP server.Verification
pnpm exec vitest runinintegrations/aws-strands/typescript: 76 files, 1613 tests, all passing.src/agent.tsreverted tomain: the new warning test fails, the other 1612 pass.pnpm exec tsc --noEmitclean;prettier --checkclean on the three touched files.Docs
README.mdgained a short paragraph pointing out that the adapter warns at construction time, placed in the existing "Passing tools to the Agent" section right after the resolve-and-spread example. That section's resolved-versus-unresolved explanation landed in #2611 and needed no correction here.