Skip to content

Fix ToolAnnotations drop in routed mode; document SDK upgrade gate - #8567

Merged
lpcox merged 2 commits into
mainfrom
copilot/go-fan-reviews-go-sdk
Jul 3, 2026
Merged

Fix ToolAnnotations drop in routed mode; document SDK upgrade gate#8567
lpcox merged 2 commits into
mainfrom
copilot/go-fan-reviews-go-sdk

Conversation

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

createFilteredServer silently dropped ToolAnnotations when building per-backend servers for /mcp/{serverID}, so readOnly/destructive hints visible on the unified endpoint were invisible to routed-mode clients.

Changes

  • internal/server/routed.go — add Annotations: toolInfo.Annotations to the &sdk.Tool{} in createFilteredServer:

    registerToolWithoutValidation(server, &sdk.Tool{
        Name:        toolInfo.Name,
        Description: toolInfo.Description,
        InputSchema: toolInfo.InputSchema,
        Annotations: toolInfo.Annotations, // was missing
    }, ...)
  • internal/server/routed_test.go — add TestCreateFilteredServer_AnnotationsPropagated: registers a tool with ReadOnlyHint: true and a non-nil DestructiveHint pointer, connects a real client via sdk.NewInMemoryTransports(), calls ListTools, and asserts both annotation fields survive the round-trip.

  • AGENTS.md — add ## SDK Upgrade Process section documenting the two existing canary tests (TestMaxRetriesSentinelCanary, TestArgumentValidationBypassCanary), what each guards, the exact commands to run before merging an SDK bump, and what to investigate if either fails.

Copilot AI linked an issue Jul 3, 2026 that may be closed by this pull request
5 tasks
- Add Annotations: toolInfo.Annotations to &sdk.Tool{} in createFilteredServer
  so readOnly/destructive hints are visible to clients on /mcp/{serverID}
- Add TestCreateFilteredServer_AnnotationsPropagated regression test using
  sdk.NewInMemoryTransports() to verify end-to-end annotation propagation
- Add ## SDK Upgrade Process section to AGENTS.md documenting the two canary
  tests (TestMaxRetriesSentinelCanary, TestArgumentValidationBypassCanary)
  with run commands and guidance for investigating failures

Closes #8562
Copilot AI changed the title [WIP] Review Go module github.com/modelcontextprotocol/go-sdk Fix ToolAnnotations drop in routed mode; document SDK upgrade gate Jul 3, 2026
Copilot AI requested a review from lpcox July 3, 2026 12:44
Copilot finished work on behalf of lpcox July 3, 2026 12:44
@lpcox
lpcox marked this pull request as ready for review July 3, 2026 12:45
@lpcox
lpcox requested review from Copilot July 3, 2026 12:45

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

This PR fixes an inconsistency between unified (/mcp) and routed (/mcp/{serverID}) mode by ensuring tool annotation hints (e.g., readOnly, destructive) are preserved when creating per-backend routed servers, and it documents the SDK upgrade gate via existing canary tests.

Changes:

  • Propagate ToolAnnotations when registering tools in createFilteredServer (routed mode).
  • Add a regression test that exercises routed mode end-to-end via in-memory client/server transports and asserts annotations survive ListTools.
  • Document the SDK upgrade process and the two canary tests that must pass after bumping github.com/modelcontextprotocol/go-sdk.
Show a summary per file
File Description
internal/server/routed.go Ensures routed-mode tool registration includes Annotations so clients see readOnly/destructive hints.
internal/server/routed_test.go Adds an integration-style regression test validating annotations are preserved through the routed server’s ListTools response.
AGENTS.md Documents the SDK upgrade checklist and what to investigate if either canary test fails.

Review details

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Low

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.

[go-fan] Go Module Review: modelcontextprotocol/go-sdk

3 participants