[BUG] Agent.initialize() is not retry-safe: MCP tool re-registration masks the real failure

[strandly-the-agent]

Checks

• I have updated to the lastest minor and patch version of Strands
• I have checked the documentation and this is not expected behavior
• I have searched ./issues and there are no duplicates of my issue

SDK Language

TypeScript

Strands Version

main @ 397176b71ea68ec57551960475279824824a3da2 (monorepo strands-ts, package version 0.0.1-development)

Language Runtime Version

Node.js 22.14.0

Operating System

Linux 6.1.161-18.298.amzn2023.aarch64 (aarch64)

Installation Method

git clone

Steps to Reproduce

Agent.initialize() (strands-ts/src/agent/agent.ts:780-823) guards re-entry only with the _initialized boolean, which is set at the very end (agent.ts:822). So if anything in the body throws, _initialized stays false and the entire body re-runs on the next call — including MCP tool registration at agent.ts:786-795:

await Promise.all(
  this._mcpClients.map(async (client) => {
    const tools = await client.listTools()
    this._toolRegistry.add(tools)     // agent.ts:789 — no dedupe guard
    ...

ToolRegistry.add() throws on a name it already holds (strands-ts/src/registry/tool-registry.ts:31-33). So attempt 2 dies in the MCP block before reaching anything else, and the user sees ToolValidationError: ... already registered instead of the actual root cause.

The sandbox-vended-tools loop immediately below it (agent.ts:799-809) already guards exactly this way — if (this._toolRegistry.get(sandboxTool.name)) → skip. The MCP block doesn't.

Repro (tsx, run from the monorepo; SDK points at the strands-ts workspace):

/** Agent.initialize() is not retry-safe: MCP tool registration re-runs on every call. */
const SDK = process.env.SDK ?? '/path/to/harness-sdk/strands-ts'
const { Agent } = await import(`${SDK}/src/agent/agent.js`)
const { McpClient } = await import(`${SDK}/src/mcp/client.js`)
const { MockMessageModel } = await import(`${SDK}/src/__fixtures__/mock-message-model.js`)

/** Stands in for a real MCP server: the real client class, one stubbed tool. */
class StubMcpClient extends (McpClient as any) {
  listToolsCalls = 0
  constructor() {
    super({ url: 'http://127.0.0.1:1/mcp' })
  }
  async listTools(): Promise<any[]> {
    this.listToolsCalls++
    return [
      {
        name: 'mcp_search',
        description: 'search the docs',
        inputSchema: { type: 'object', properties: {}, required: [] },
        stream: async function* (): any {
          return { type: 'toolResultBlock', toolUseId: 'x', status: 'success', content: [] }
        },
      },
    ]
  }
}

/** Any plugin that validates in initAgent() and throws — e.g. a memory store that can't reach its backend. */
class FailingPlugin {
  name = 'failing-plugin'
  calls = 0
  initAgent(): void {
    this.calls++
    throw new Error('REAL ROOT CAUSE: KB permission denied')
  }
}

const mcp = new StubMcpClient()
const plugin = new FailingPlugin()
const model = new MockMessageModel()
model.addTurn({ type: 'textBlock', text: 'hi' }, { stopReason: 'endTurn' })
const agent = new Agent({ model, tools: [mcp as any], plugins: [plugin], printer: false })

const show = (r: PromiseSettledResult<unknown>): string =>
  r.status === 'rejected' ? `${(r.reason as Error).constructor.name}: ${(r.reason as Error).message}` : 'RESOLVED'

for (const attempt of [1, 2, 3]) {
  console.log(`initialize() #${attempt}:`, show((await Promise.allSettled([agent.initialize()]))[0]))
}
console.log('mcp.listTools() calls:', mcp.listToolsCalls, '| plugin.initAgent calls:', plugin.calls)
console.log('invoke():          ', show((await Promise.allSettled([agent.invoke('hi')]))[0]))

$ SDK=.../strands-ts npx tsx mcp-retry-mask.ts
initialize() #1: Error: REAL ROOT CAUSE: KB permission denied
initialize() #2: ToolValidationError: Tool with name 'mcp_search' already registered
initialize() #3: ToolValidationError: Tool with name 'mcp_search' already registered
mcp.listTools() calls: 3 | plugin.initAgent calls: 1
invoke():           ToolValidationError: Tool with name 'mcp_search' already registered

The same root cause fires with no failing plugin at all — just two concurrent initialize() calls, where any real listTools() latency is enough to interleave them:

const agent = new Agent({ model, tools: [new StubMcpClient() as any], printer: false })
const rs = await Promise.allSettled([agent.initialize(), agent.initialize()])

initialize()#0: RESOLVED
initialize()#1: ToolValidationError: Tool with name 'mcp_search' already registered

Expected Behavior

A retried initialize() surfaces the failure that actually stopped initialization. MCP tool registration should be idempotent across calls — either skipped when the tool is already registered (matching the sandbox-vended-tools loop at agent.ts:801) or not re-run at all once it has succeeded. Concurrent initialize() calls should not be able to double-register the same MCP tools.

Actual Behavior

The second and every later initialize() (and any invoke()/stream(), which lazily initializes) rejects with ToolValidationError: Tool with name '<name>' already registered, hiding the real cause. listTools() is also re-issued to the MCP server on every attempt (3 calls for 3 attempts above). With two concurrent initialize() calls, one of them fails this way even though nothing is misconfigured.

Additional Context

Found by strandly-the-agent while reviewing #3482, which fixes a different fail-open in PluginRegistry. Worth noting the interaction: #3482 makes a plugin-init failure permanent so that every later initialize() replays the original error, but for any agent that also has an MCP tool, this bug intercepts the retry first — so the improved error is never what the user sees. Verified against main @ 397176b71 and against #3482's head c79ad41d: byte-identical output on both, so this is independent of that PR and not caused by it.

Reach: any Agent configured with an MCP client tool plus any plugin whose initAgent() can throw — including plugins the SDK injects by default, e.g. AgentDelegation (strands-ts/src/agent/agent-delegation.ts:103-112) or a MemoryManager store whose initialize() rethrows on permissions/connectivity (strands-ts/src/memory/memory-manager.ts:220-224) — where the caller retries after the first failure. Also any code path that calls initialize() concurrently, with no failing plugin needed.

Not verified against a real MCP server: the repro subclasses the real McpClient and stubs listTools(), so it exercises Agent.initialize()'s registration path but not the transport.

Possible Solution

Make the MCP block idempotent, mirroring the guard the sandbox loop already uses:

const tools = await client.listTools()
this._toolRegistry.addOrReplace(tools)   // instead of .add(tools)

addOrReplace already exists (tool-registry.ts:41) and is what onToolsChanged uses two lines later (agent.ts:792), so re-running the block would become harmless. That alone does not fix the concurrent case, though — a durable fix is to give Agent.initialize() a real re-entrancy guard by memoizing its promise, the same shape #3482 applies to the plugin registry:

public async initialize(): Promise<void> {
  this._initializationPromise ??= this._initialize()
  return this._initializationPromise
}

That would also fix the double-firing of InitializedEvent and observeAgent() filed separately. Happy to open a PR if a maintainer confirms the preferred shape — in particular whether a failed Agent.initialize() should stay failed for the agent's lifetime (as #3482 chose for plugins) or be retriable.

Related Issues

#3482, #3477

[rohit-jsfreaky]

I traced this against #3482 and #3711. #3482 makes plugin-registry failures fail closed, but it does not serialize Agent.initialize(), so MCP registration can still mask the original failure and concurrent calls can still race.

I would like to take this on with a focused TypeScript fix and regression coverage. My proposed scope is:

• make Agent.initialize() share a single in-flight initialization attempt;
• keep MCP registration idempotent for a later retry if retries remain supported;
• add tests for retrying after a downstream initialization failure and for concurrent initialize() calls.

Before I start, could a maintainer confirm the intended failure semantics: should a failed Agent.initialize() remain failed for that Agent instance, matching #3482, or should callers be able to retry it after correcting the underlying problem?