mcptest: add SetSamplingHandler and SetElicitationHandler by basilalshukaili · Pull Request #902 · mark3labs/mcp-go

basilalshukaili · 2026-06-02T21:03:51Z

The mcptest package is very convenient for testing MCP servers — especially tools, prompts, resources, and hooks — without standing up a real transport. However, it currently has no way to test tools that call back into the client via sampling (server.RequestSampling) or elicitation (server.RequestElicitation). Anyone writing such a tool today has to wire up InProcessTransport + InProcessSession by hand, which is considerably more boilerplate.

What this PR does

Adds two new methods to mcptest.Server:

// SetSamplingHandler registers a handler for server→client sampling requests.
// Must be called before Start().
func (s *Server) SetSamplingHandler(h client.SamplingHandler)

// SetElicitationHandler registers a handler for server→client elicitation requests.
// Must be called before Start().
func (s *Server) SetElicitationHandler(h client.ElicitationHandler)

When a handler is registered:

It is passed as a client.ClientOption to client.NewClient, so the capability is advertised in the initialize handshake.
client.Start() is called instead of transport.Start() — this is the key fix: client.Start() wires the bidirectional SetRequestHandler on the underlying *transport.Stdio so that server→client JSON-RPC requests (sampling, elicitation) are dispatched to the handler rather than being dropped with No request handler configured.
When a sampling handler is set, mcpServer.EnableSampling() is called automatically inside the server goroutine, so callers do not need to add it via AddServerOptions.

Changes

File	What changed
`mcptest/mcptest.go`	New fields + methods, `client.Start()` instead of `transport.Start()`, auto `EnableSampling()`
`mcptest/mcptest_sampling_elicitation_test.go`	Two new end-to-end tests through the real stdio transport

Tests

$ go test ./mcptest/... -v
--- PASS: TestServerWithSamplingHandler (0.00s)
--- PASS: TestServerWithElicitationHandler (0.00s)
--- PASS: TestServerWithTool (0.00s)
... (all existing tests still pass)
PASS ok github.com/mark3labs/mcp-go/mcptest

All pre-existing mcptest tests continue to pass. The client and client/transport packages time out in this environment (pre-existing on main as well); server, mcp, e2e, mcptest, and tracing all pass.

Summary by CodeRabbit

Release Notes

New Features
- Test servers now support registering handlers for sampling (LLM requests) and elicitation (confirmation requests), enabling tool testing without external dependencies.
Tests
- Added end-to-end tests demonstrating sampling and elicitation request handling.

The mcptest.Server helper previously had no way to test tools that call server.RequestSampling or server.RequestElicitation, because the underlying client was created without sampling/elicitation options and the stdio transport request-handler was never wired up. This change: - Adds SetSamplingHandler(client.SamplingHandler) to mcptest.Server - Adds SetElicitationHandler(client.ElicitationHandler) to mcptest.Server - Passes the handlers as client.ClientOption to client.NewClient - Uses client.Start() instead of transport.Start() so the bidirectional request handler (needed for server-to-client sampling/elicitation calls over stdio) is registered before the Initialize handshake - Automatically calls mcpServer.EnableSampling() when a sampling handler is set, removing boilerplate from callers Two tests are added in mcptest_sampling_elicitation_test.go covering both paths end-to-end through the real stdio transport.

mark-iii-labs-huly · 2026-06-02T21:03:59Z

Connected to Huly®: MCP_G-466

coderabbitai · 2026-06-02T21:04:07Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d54ba9f2-3804-4cd6-a3a2-a13b339a7f91

📥 Commits

Reviewing files that changed from the base of the PR and between d5c0727 and 5b56d08.

📒 Files selected for processing (2)

mcptest/mcptest.go
mcptest/mcptest_sampling_elicitation_test.go

Walkthrough

The PR extends mcptest.Server with sampling and elicitation handler registration and integration. Tests can now call SetSamplingHandler and SetElicitationHandler to configure request handling, and Start() automatically wires those handlers into the client and enables sampling when present, before the Initialize handshake.

Changes

Sampling and Elicitation Handler Support

Layer / File(s)	Summary
Handler Registration API `mcptest/mcptest.go`	Server struct gains `samplingHandler` and `elicitationHandler` fields. New `SetSamplingHandler` and `SetElicitationHandler` exported methods allow test registration prior to `Start()`.
Handler Integration in Server Startup `mcptest/mcptest.go`	`Start()` captures registered handler state, conditionally enables sampling in the server goroutine, builds `clientOpts` from handlers, and calls `s.client.Start(ctx)` before `Initialize()` to register request handlers during handshake.
End-to-End Tests and Test Doubles `mcptest/mcptest_sampling_elicitation_test.go`	`fixedSamplingHandler` and `fixedElicitationHandler` test doubles track invocations and return canned responses. `TestServerWithSamplingHandler` and `TestServerWithElicitationHandler` validate tool execution paths via `ServerFromContext`, confirming handler invocation and tool output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

mark3labs/mcp-go#339: Both modify mcptest.Server.Start to control startup flow using explicit context.Context and internal initialization wiring.
mark3labs/mcp-go#149: Both extend mcptest.Server and its Start() initialization flow; this PR adds sampling/elicitation handler registration to the existing test harness.

Suggested reviewers

pottekkat
rwjblue-glean
ezynda3
dugenkui03

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description is comprehensive and well-structured, but does not follow the required template with Type of Change, Checklist, and other standard sections.	Fill out the missing template sections: select Type of Change (appears to be 'New feature'), complete the Checklist items, and verify code style compliance.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely and clearly describes the main change: adding two new handler methods to mcptest.Server.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ezynda3

PR Review: #902 — feat(mcptest): add sampling and elicitation handler support

Verdict: PASS ✅

Security (Step 2 + reviewer)

No findings. Static scan was clean across secrets, exec, SQL, gob, unsafe, TLS, math/rand, panic, and debug leftovers.

Logic errors

None identified.

Regressions vs base (Step 3)

go build ./... — clean on both base and head.
go vet ./... — clean on both.
go test ./mcptest/... -race -count=1 — clean; both new tests (TestServerWithSamplingHandler, TestServerWithElicitationHandler) pass under -race.
golangci-lint run ./mcptest/... — 0 issues.
gofmt -l and go mod tidy — clean.
One flake observed in server.TestMCPServer_HybridModeDetection on the first full -race run on HEAD; it does not reproduce in isolation on either base or head (3/3 passes), and the test does not touch any code modified by this PR. Treating as pre-existing flake, not a regression.

Go idiom & style

Exported SetSamplingHandler / SetElicitationHandler have godocs starting with the identifier name. ✅
Error wrapping uses %w (fmt.Errorf("client.Start(): %w", err)). ✅
Goroutine capture comment correctly notes why samplingHandler is snapshotted before the go func() (it is read inside the goroutine to gate EnableSampling()). The elicitation handler is only read on the main goroutine, so capture is not required for it; capturing both for symmetry would be marginally cleaner but is not required.
Test doubles' callCount fields are mutated without sync — fine because each test issues exactly one call, but worth noting if these helpers ever get reused concurrently.

Suggestions (non-blocking)

Consider making SetSamplingHandler / SetElicitationHandler panic (or return an error) if called after Start() to fail loudly rather than silently being ignored.
Add a negative-path test where the sampling handler returns an error or the elicitation handler declines/cancels, to cover the error branches in RequestSampling / RequestElicitation flows from tests' perspective.
Switching from transport.Start() to client.Start() is the right call — client.Start() is idempotent on the transport and is what wires SetRequestHandler for the BidirectionalInterface, which is exactly what sampling/elicitation need. The PR description accurately describes this.

Summary

Small, well-scoped, well-tested addition that fills a real gap in mcptest (no way to exercise tools that call back into the client). Implementation is correct, idiomatic, and all CI checks plus local race tests pass.

basilalshukaili · 2026-06-09T11:17:49Z

Just checking in on this one — CI is green (tests, lint, golangci-lint, verify-codegen) and CodeRabbit had no blocking feedback. Glad to adjust anything the maintainers would like. Thanks for the reviews!

ezynda3 reviewed Jun 4, 2026

View reviewed changes

ezynda3 merged commit 3b66aa7 into mark3labs:main Jun 16, 2026
6 checks passed

Navigation Menu

Repository navigation

mcptest: add SetSamplingHandler and SetElicitationHandler - #902

mcptest: add SetSamplingHandler and SetElicitationHandler#902
ezynda3 merged 1 commit into
mark3labs:mainfrom
basilalshukaili:mcptest-sampling-elicitation-handlers

basilalshukaili commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

mark-iii-labs-huly Bot commented Jun 2, 2026

Uh oh!

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

❌ Failed checks (1 inconclusive)

Uh oh!

ezynda3 left a comment

Uh oh!

basilalshukaili commented Jun 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

basilalshukaili commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Changes

Tests

Summary by CodeRabbit

Release Notes

Uh oh!

mark-iii-labs-huly Bot commented Jun 2, 2026

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 inconclusive)

Uh oh!

ezynda3 left a comment

Choose a reason for hiding this comment

PR Review: #902 — feat(mcptest): add sampling and elicitation handler support

Security (Step 2 + reviewer)

Logic errors

Regressions vs base (Step 3)

Go idiom & style

Suggestions (non-blocking)

Summary

Uh oh!

basilalshukaili commented Jun 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

basilalshukaili commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading