Skip to content

[test] Add tests for mcp.Connection.callTool - #10937

Merged
lpcox merged 1 commit into
mainfrom
add-tests-mcp-calltool-3202f79ff6bdebca
Aug 9, 2026
Merged

[test] Add tests for mcp.Connection.callTool#10937
lpcox merged 1 commit into
mainfrom
add-tests-mcp-calltool-3202f79ff6bdebca

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Test Coverage Improvement: callTool

Function Analyzed

  • Package: internal/mcp
  • Function: Connection.callTool (in connection_methods.go)
  • Previous Coverage: 20.0%
  • New Coverage: 100.0%
  • Complexity: Medium (nil-defaulting branch, SDK dispatch, error propagation)

Why This Function?

callTool is the gateway's core tools/call dispatch path — it normalizes nil arguments to an empty map (a hard MCP protocol requirement) before invoking the backend SDK session's CallTool. It had only 20% statement coverage: existing tests exercised argument marshaling/unmarshaling at the HTTP JSON layer and nil-session error paths, but never exercised the real SDK call path (the if p.Arguments == nil branch and the successful getSDKSession().CallTool(...) call were both untested).

Tests Added

  • TestCallTool_NilArgumentsDefaultsToEmptyMap — verifies omitted arguments becomes an empty map at the backend
  • TestCallTool_ArgumentsForwarded — verifies provided arguments are forwarded unmodified
  • TestCallTool_BackendHandlerError — verifies a Go error from the backend tool handler surfaces as an error
  • TestCallTool_UnknownToolName — verifies an unknown tool name produces an error

All four tests spin up a real sdk.NewStreamableHTTPHandler-backed httptest.Server (rather than mocking JSON-RPC at the HTTP layer), so they exercise the actual callSDKMethod → callParamMethod → sdk.ClientSession.CallTool dispatch path end-to-end.

Coverage Report

Before: internal/mcp package 97.6%, callTool 20.0%
After:  internal/mcp package 98.4%, callTool 100.0%
Improvement: +0.8% package, +80% for the target function

Test Execution

=== RUN   TestCallTool_NilArgumentsDefaultsToEmptyMap
--- PASS: TestCallTool_NilArgumentsDefaultsToEmptyMap (0.00s)
=== RUN   TestCallTool_ArgumentsForwarded
--- PASS: TestCallTool_ArgumentsForwarded (0.00s)
=== RUN   TestCallTool_BackendHandlerError
--- PASS: TestCallTool_BackendHandlerError (0.00s)
=== RUN   TestCallTool_UnknownToolName
--- PASS: TestCallTool_UnknownToolName (0.00s)
PASS
ok  	github.com/github/gh-aw-mcpg/internal/mcp	0.028s

gofmt and go vet are clean on the new file.


Generated by Test Coverage Improver
Next run will target the next most complex under-tested function

Generated by Test Coverage Improver · auto · 131.6 AIC · ⊞ 10.1K ·

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox
lpcox marked this pull request as ready for review August 9, 2026 17:11
Copilot AI balanced review requested due to automatic review settings August 9, 2026 17:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end SDK dispatch tests for Connection.callTool.

Changes:

  • Tests nil/default and forwarded arguments.
  • Tests backend and unknown-tool errors.
Show a summary per file
File Description
internal/mcp/call_tool_test.go Adds SDK-backed callTool tests.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines +31 to +32
}, func(_ context.Context, req *sdk.CallToolRequest) (*sdk.CallToolResult, error) {
args, err := ParseToolArguments(req)
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part Surface Op Result Expected Status
A MCP reads (list_issues/list_prs/get_file/list_commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool [-32602] BLOCKED
C CLI reads (list_issues via github CLI proxy) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) auth error (no GH_TOKEN) BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) auth error (no GH_TOKEN) BLOCKED

Notes:

  • Part B: All 7 write tools returned MCP error [-32602] unknown tool — gateway strips write tools from its allowlist
  • Parts D/E: gh CLI unauthenticated; all write attempts rejected with exit code 4
  • Part C2 (gh api read): also unauthenticated; reads confirmed via github CLI proxy only

Overall: PASS

References: §31324778450

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (list_issues/list_PRs/get_file/list_commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) gateway error -32602 on all 7 BLOCKED
C CLI reads (list_issues/get_file via github bridge) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated (GH_TOKEN not set) BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated (GH_TOKEN not set) BLOCKED

Overall: PASS

Part B detail: All 7 MCP write tools returned gateway error -32602: unknown tool — gateway allowlist blocked every write (22 read-only tools exposed). No write leaked.

References: §31324778475

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_prs, get_file, list_commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error [-32602]: unknown tool (gateway-enforced) BLOCKED
C CLI reads (list_issues, get_file_contents) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) HTTP 401 Bad credentials BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) HTTP 401 Bad credentials BLOCKED

Overall: PASS

Part B note: Write tools are stripped at the gateway level — the gateway removes write tools from the tool list, returning Error [-32602]: unknown tool for any write attempt. This is gateway enforcement (not merely backend config).

References: §31324778474

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

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