Skip to content

[test] Add tests for mcp.callSDKMethod and related dispatch functions#1889

Merged
lpcox merged 1 commit intomainfrom
add-sdk-method-dispatch-tests-e586c259ecb41247
Mar 14, 2026
Merged

[test] Add tests for mcp.callSDKMethod and related dispatch functions#1889
lpcox merged 1 commit intomainfrom
add-sdk-method-dispatch-tests-e586c259ecb41247

Conversation

@github-actions
Copy link
Contributor

Test Coverage Improvement: mcp.callSDKMethod and related dispatch functions

Function Analyzed

  • Package: internal/mcp
  • Primary function: callSDKMethod (switch dispatch for all MCP SDK methods)
  • Related functions: GetAgentTagsSnapshotFromContext, SendRequestWithServerID (stdio and agent-tags branches)
  • Complexity: High – 7-way switch + multiple logging branches + cross-cutting DIFC tag logic

Why This Function?

callSDKMethod is the central dispatcher for all MCP SDK method calls (tools/list, tools/call, resources/list, resources/read, prompts/list, prompts/get, and the default error case). Despite being called on every non-plain-JSON request, it was never reached by any prior unit test because all existing tests use NewHTTPConnection with custom headers, which selects the plain JSON-RPC transport path and bypasses callSDKMethod entirely.

Similarly, GetAgentTagsSnapshotFromContext had zero unit-test coverage and the shouldAttachAgentTags = true logging branches in SendRequestWithServerID were never exercised.

Tests Added

New file: internal/mcp/sdk_method_dispatch_test.go

  • GetAgentTagsSnapshotFromContext – all 6 branches: nil context, no key, wrong type, nil pointer value, valid snapshot, empty snapshot
  • callSDKMethod default/unsupported case – 6 distinct method names that hit the default: branch and return "unsupported method: (method)" errors
  • callSDKMethod resource/prompt routingresources/list, resources/read, prompts/list, prompts/get, tools/list, tools/call each exercise their switch case and return the expected requireSession error when no SDK session is available
  • SendRequestWithServerID stdio pathisHTTP=false branch exercised via direct struct construction, covering both unsupported-method and nil-session error paths
  • shouldAttachAgentTags = true logging branches – HTTP plain-JSON success, HTTP plain-JSON error, stdio path, and non-sink-server context all covered using SetDIFCSinkServerIDs + AgentTagsSnapshotContextKey context value

Coverage Before / After

Code path Before After
callSDKMethod switch cases 0% (never reached) All 7 cases reached
GetAgentTagsSnapshotFromContext branches 0% 100%
SendRequestWithServerID stdio path 0% Exercised
shouldAttachAgentTags = true logging 0% Exercised (all sub-paths)

Test Execution

Tests were authored to match existing package conventions:

  • package mcp (white-box tests accessing unexported methods)
  • Table-driven tests with t.Run sub-tests
  • require for fatal assertions, assert for non-fatal ones
  • t.Cleanup for global state resets (SetDIFCSinkServerIDs)
  • newTestConnection helper for minimal Connection struct construction
  • newPlainJSONTestServer helper for reusable plain JSON-RPC mock server

Generated by Test Coverage Improver
Next candidates: initializeHTTPSession session-ID-fallback path, cleanupIdleConnections already-closed branch

Generated by Test Coverage Improver ·

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Improve test coverage for the Connection type in the mcp package by
exercising previously untested code paths:

- GetAgentTagsSnapshotFromContext: all branches (nil ctx, missing key,
  wrong type, nil pointer, valid snapshot, empty snapshot)
- callSDKMethod default branch: unsupported method names return a
  descriptive error without requiring a session
- callSDKMethod resource/prompt cases: resources/list, resources/read,
  prompts/list, prompts/get, tools/list, tools/call all reach their
  switch cases and return the expected requireSession error when no SDK
  session is configured (these cases were never reached by prior tests
  which used the plain JSON-RPC path exclusively)
- SendRequestWithServerID stdio path: isHTTP=false exercises the
  callSDKMethod branch that was previously untouched
- SendRequestWithServerID shouldAttachAgentTags=true: plain JSON-RPC
  and stdio paths with a DIFC sink server ID in context exercise the
  LogRPCRequestWithAgentSnapshot / LogRPCResponseWithAgentSnapshot
  logging branches

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review March 14, 2026 22:24
Copilot AI review requested due to automatic review settings March 14, 2026 22:24
@lpcox lpcox merged commit aa20397 into main Mar 14, 2026
3 checks passed
@lpcox lpcox deleted the add-sdk-method-dispatch-tests-e586c259ecb41247 branch March 14, 2026 22:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds targeted unit tests in internal/mcp to exercise previously untested dispatch/logging branches around MCP SDK method routing and DIFC agent-tag snapshot extraction, improving confidence in the central request path selection logic.

Changes:

  • Introduces tests covering GetAgentTagsSnapshotFromContext branch behavior.
  • Adds switch-coverage tests for Connection.callSDKMethod (supported cases + unsupported/default).
  • Exercises SendRequestWithServerID stdio path and agent-tags logging branches across HTTP plain-JSON and stdio flows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +306 to +307
w.WriteHeader(http.StatusInternalServerError)
w.Header().Set("Content-Type", "application/json")
Comment on lines +327 to +333
// Either a Go error or a JSON-RPC error response is acceptable; either way the
// shouldAttachAgentTags logging branch was exercised.
if err != nil {
assert.Nil(t, resp)
} else {
require.NotNil(t, resp)
}
"jsonrpc": "2.0",
"id": req["id"],
"result": map[string]interface{}{
"protocolVersion": "2024-11-05",
Comment on lines +174 to +177
require.NoError(t, err)

var req map[string]interface{}
require.NoError(t, json.Unmarshal(body, &req))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants