Skip to content

fix(mcp): McpToolset.fromConfig ignoring stdioServerParams#1360

Merged
copybara-service[bot] merged 1 commit into
google:mainfrom
svetanis:fix/mcptoolset-fromconfig-stdioserverparams
Jul 22, 2026
Merged

fix(mcp): McpToolset.fromConfig ignoring stdioServerParams#1360
copybara-service[bot] merged 1 commit into
google:mainfrom
svetanis:fix/mcptoolset-fromconfig-stdioserverparams

Conversation

@svetanis

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:
When an agent config defines an MCP Toolset using stdioServerParams (config-driven), McpToolset.fromConfig correctly validates the config but then fails to use the provided parameters. It falls back to stdioConnectionParams(), which is null. This leads to a NullPointerException at session creation time inside DefaultMcpTransportBuilder.

Solution:
Extracted the connection-parameter resolution from fromConfig into a new @VisibleForTesting helper, McpToolset.resolveConnectionParameters(), and added the missing stdioServerParams branch, mapping via StdioServerParameters::toServerParameters (the type the underlying MCP SDK transport builder accepts). The chain now ends in orElseThrow so a broken invariant fails explicitly at fromConfig time instead of deferring to a NullPointerException at session creation.

  @VisibleForTesting
  static Object resolveConnectionParameters(McpToolsetConfig mcpToolsetConfig) {
    return Optional.<Object>ofNullable(mcpToolsetConfig.stdioConnectionParams())
        .or(() -> Optional.ofNullable(mcpToolsetConfig.sseServerParams()))
        .or(
            () ->
                Optional.ofNullable(mcpToolsetConfig.stdioServerParams())
                    .map(StdioServerParameters::toServerParameters))
        .orElseThrow(() -> new IllegalStateException("Validated MCP connection params missing."));
  }

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Please include a summary of passed java test results.
All 22 tests in McpToolsetTest.java pass locally (18 existing + 4 new). The following 4 tests were newly added to directly cover the extracted resolveConnectionParameters method — the core of the fix:

  • resolveConnectionParameters_stdioServerParams_convertsToSdkServerParameters — verifies the formerly-dead stdioServerParams branch now resolves to ServerParameters
  • resolveConnectionParameters_sseServerParams_passesThrough
  • resolveConnectionParameters_stdioConnectionParams_passesThrough
  • resolveConnectionParameters_nothingSet_throwsIllegalStateException

Manual End-to-End (E2E) Tests:

Verified the fix using a standalone end-to-end demo application that exercises the config-driven agent flow:

  1. Ran the demo which leverages a config-driven LlmAgent (configured via root_agent.yaml) to load a local file system MCP server via stdioServerParams. The MCP server is a Node-free Java implementation (JavaFileMcpServer) to remove any Node.js dependency.
  2. Verified that the agent correctly establishes the MCP stdio connection, loads the listFiles and readFile tools from the MCP server, and successfully executes a multi-step tool-calling exchange (one user turn, multiple function-call round trips) over the live connection.
  3. Confirmed that the tools successfully read the local filesystem and the model generates a final response without throwing the NullPointerException.

Before fix (NPE — server never launched):

18:33:56.311 [main] INFO com.google.adk.tools.mcp.McpToolset -- MCP session is null, initializing.
java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "connectionParams" is null
	at com.google.adk.tools.mcp.DefaultMcpTransportBuilder.build(DefaultMcpTransportBuilder.java:72)
	at com.google.adk.tools.mcp.McpSessionManager.initializeSession(McpSessionManager.java:78)
	at com.google.adk.tools.mcp.McpSessionManager.createSession(McpSessionManager.java:56)

Checklist

  • I have read the CONTRIBUTING.md document.
  • My pull request contains a single commit.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

The fix was verified end-to-end using a Node-free Java MCP server (JavaFileMcpServer) that replaces the usual npx @modelcontextprotocol/server-filesystem, removing any Node.js dependency for reproduction.

@hemasekhar-p hemasekhar-p self-assigned this Jul 20, 2026
@hemasekhar-p

Copy link
Copy Markdown
Contributor

Hi @svetanis, thank you for your contribution and We appreciate you taking the time to submit this pull request. Currently this PR is under review by our team, we will keep you updated if any additional information is required. thank you.

@kvmilos
kvmilos self-requested a review July 21, 2026 08:34
@kvmilos kvmilos self-assigned this Jul 21, 2026
@svetanis
svetanis force-pushed the fix/mcptoolset-fromconfig-stdioserverparams branch from 2ee0d9b to cf71d7b Compare July 21, 2026 23:28
@svetanis

Copy link
Copy Markdown
Contributor Author

@kvmilos ,

Thanks so much for the review and approval!
I've just rebased the branch against the latest main.

@copybara-service
copybara-service Bot merged commit 737c293 into google:main Jul 22, 2026
8 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.

McpToolset.fromConfig ignores stdioServerParams resulting in NullPointerException

3 participants