feat(mcp): add sampling request handler (1/3 for #10704) - #27130
feat(mcp): add sampling request handler (1/3 for #10704)#27130cbcoutinho wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the core handler logic for MCP client sampling support, fulfilling the first phase of the implementation plan. It focuses on the functional transformation between MCP sampling requests and Gemini generation calls, maintaining a clean separation of concerns by excluding UI components and transport wiring, which are reserved for subsequent PRs. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Model Context Protocol (MCP) sampling/createMessage request. It includes a new core handler, handleSamplingRequest, which converts MCP requests into Gemini API calls and maps the responses back to the MCP format. The implementation supports text and image content blocks while explicitly rejecting audio and unsupported types. Comprehensive unit tests have been added to verify role mapping, content conversion, parameter passing, and error handling. I have no feedback to provide as the implementation is solid and well-tested.
Implements the pure core of MCP sampling support: a single `handleSamplingRequest()` function in `packages/core/src/tools/mcp-sampling.ts` that converts an MCP `CreateMessageRequest` to a Gemini `ContentGenerator` call and maps the response back to a `CreateMessageResult`. This is the first of three planned PRs for google-gemini#10704, per maintainer guidance to extract sampling logic into its own named function rather than inlining it in `mcp-client.ts`. PR 2 will add the consent dialog/UI, PR 3 will register the handler in `connectToMcpServer`, advertise the `sampling: {}` capability, and wire in policy/approval. Scope of this PR: - text and image content blocks (audio + other types rejected with clear error messages — forward-compatible with future SDK revisions) - pass-through of `systemPrompt`, `temperature`, `maxTokens`, `stopSequences` - uses `config.getContentGenerator()` directly to bypass the agent's core system prompt (so the server's `systemPrompt` is honored cleanly) - uses `config.getModel()`; `modelPreferences` hints are ignored per RFC v1 - maps Gemini `FinishReason.MAX_TOKENS` -> MCP `maxTokens`, everything else -> `endTurn` - 14 unit tests, no changes to `mcp-client.ts` or any other file
98bcb01 to
566b5b2
Compare
Summary
First of three PRs implementing MCP client sampling support per issue #10704. This PR adds only the pure core handler — no UI, no client wiring, no policy integration, no capability advertised to servers yet.
Background
The prior consolidated PR #12801 was closed in favor of a 3-PR breakdown agreed with @jackwotherspoon (comment), starting with the explicit ask to "extract the sampling logic into its own named function so we don't bloat
connectToMcpServer."Planned follow-ups (separate PRs):
McpSamplingDialog+ consent UI +CoreEvent.McpSamplingRequestevent, with an isolated example harness (modeled on the ask-user dialog from feat: add AskUserDialog for UI component of AskUser tool #17344)connectToMcpServer, advertisesampling: {}capability, wrap with policy/approval, add the integration testThe transport memory-leak fix that originally rode along in #12801 has already shipped separately in #18054.
What's in this PR
packages/core/src/tools/mcp-sampling.ts— a single exported async function:What it does:
SamplingMessage[]→ GeminiContent[]text→{ text }partsimage→{ inlineData: { mimeType, data } }partsaudio→ rejected with a clear error (deferred)tool_usefrom newer SDK revisions) → rejectedrole: assistant→modelsystemPrompt→systemInstructionmaxTokens→maxOutputTokenstemperature→temperaturestopSequences→stopSequencesconfig.getContentGenerator()directly (intentionally bypassesGeminiClientso the agent's core system prompt does NOT leak into server-driven sampling)config.getModel();modelPreferenceshints from the request are ignored (per the RFC v1 — client retains model authority)FinishReason.MAX_TOKENS→ MCPmaxTokens; everything else →endTurnmcp-sampling-<uuid>prompt id per callpackages/core/src/tools/mcp-sampling.test.ts— 14 unit tests covering text + image, audio/unknown-type rejection, role mapping, conversation order, param pass-through, param omission,modelPreferencesignored, finish-reason mapping, empty/no-text candidate errors, and prompt-id uniqueness.No changes to
mcp-client.tsor anywhere else. The handler isn't reachable yet — wiring lives in PR 3.Test plan
npm run typecheck -w @google/gemini-cli-corenpx eslint packages/core/src/tools/mcp-sampling.ts packages/core/src/tools/mcp-sampling.test.tsnpx vitest run packages/core/src/tools/mcp-sampling.test.ts— 14/14 passnpx vitest run packages/core/src/tools/— 533 pass, 2 pre-existing skipped, no regressionsThis PR was generated with the help of AI, and reviewed by a Human