diff --git a/AGENTS.md b/AGENTS.md index 4d6cafd90..da0f62241 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -507,6 +507,26 @@ DEBUG_COLORS=0 DEBUG=* ./awmg --config config.toml - **mTLS**: Mutual TLS can be enabled with `--tls-cert`, `--tls-key`, and `--tls-ca` flags (or corresponding env vars) to require client certificates for all connections - **HMAC request signing**: Set `--hmac-secret` (or `MCP_GATEWAY_HMAC_SECRET`) to require HMAC-SHA256 signed requests; protects against replay attacks using `X-MCP-Timestamp`, `X-MCP-Nonce`, and `X-MCP-Signature` headers +## SDK Upgrade Process + +Before upgrading `github.com/modelcontextprotocol/go-sdk`, verify the two canary tests still pass: + +| Test | File | Guards | +|---|---|---| +| `TestMaxRetriesSentinelCanary` | `internal/mcp/http_transport_test.go` | `streamableMaxRetries = -1` disables SDK-level auto-reconnect; gateway owns reconnect logic | +| `TestArgumentValidationBypassCanary` | `internal/server/tool_registry_test.go` | `Server.AddTool` (method receiver) skips argument validation, allowing draft-07 schemas from backends | + +Run them explicitly after bumping the version: + +```bash +go test ./internal/mcp/ -run TestMaxRetriesSentinelCanary -v +go test ./internal/server/ -run TestArgumentValidationBypassCanary -v +``` + +If either canary fails after an upgrade, **stop and investigate** before merging: +- `TestMaxRetriesSentinelCanary` failure → SDK changed `MaxRetries: -1` semantics; update `streamableMaxRetries` and reconnect logic in `internal/mcp/http_transport.go`. +- `TestArgumentValidationBypassCanary` failure → SDK added argument validation to the `AddTool` method path; switch to a different bypass mechanism and update `registerToolWithoutValidation` in `internal/server/tool_registry.go`. + ## Resources - [README.md](./README.md) - Full documentation diff --git a/internal/server/routed.go b/internal/server/routed.go index 6e3ba8dc9..e06f1ea2d 100644 --- a/internal/server/routed.go +++ b/internal/server/routed.go @@ -123,6 +123,7 @@ func createFilteredServer(unifiedServer *UnifiedServer, backendID string) *sdk.S Name: toolInfo.Name, // Without prefix for the client Description: toolInfo.Description, InputSchema: toolInfo.InputSchema, // Include schema for clients + Annotations: toolInfo.Annotations, // Preserve readOnly/destructive hints }, func(ctx context.Context, req *sdk.CallToolRequest, _ interface{}) (*sdk.CallToolResult, interface{}, error) { logRouted.Printf("[ROUTED] Calling unified handler for: %s", toolNameCopy) return handler(ctx, req, nil) diff --git a/internal/server/routed_test.go b/internal/server/routed_test.go index c94b1734a..0156415d3 100644 --- a/internal/server/routed_test.go +++ b/internal/server/routed_test.go @@ -214,6 +214,68 @@ func TestCreateFilteredServer_ToolFiltering(t *testing.T) { _ = filteredServer // Use variable to avoid unused error } +// TestCreateFilteredServer_AnnotationsPropagated verifies that ToolAnnotations +// (e.g. readOnly, destructive hints) are forwarded to clients in routed mode. +// Regression test for the bug where createFilteredServer omitted Annotations. +func TestCreateFilteredServer_AnnotationsPropagated(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{}, + } + + ctx := context.Background() + us, err := NewUnified(ctx, cfg) + require.NoError(err, "NewUnified() failed") + defer us.Close() + + destructiveHint := true + mockHandler := func(ctx context.Context, req *sdk.CallToolRequest, state interface{}) (*sdk.CallToolResult, interface{}, error) { + return &sdk.CallToolResult{}, state, nil + } + + us.toolsMu.Lock() + us.tools["github___safe_read"] = &ToolInfo{ + Name: "github___safe_read", + Description: "Read-only tool with annotations", + BackendID: "github", + InputSchema: map[string]interface{}{"type": "object"}, + Annotations: &sdk.ToolAnnotations{ + ReadOnlyHint: true, + DestructiveHint: &destructiveHint, + }, + Handler: mockHandler, + } + us.toolsMu.Unlock() + + filteredServer := createFilteredServer(us, "github") + require.NotNil(filteredServer) + + // Connect a client via in-memory transport and call ListTools to verify annotations. + serverTransport, clientTransport := sdk.NewInMemoryTransports() + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + go func() { _ = filteredServer.Run(ctx, serverTransport) }() + + client := sdk.NewClient(&sdk.Implementation{Name: "test-client", Version: "1.0"}, &sdk.ClientOptions{}) + clientSession, err := client.Connect(ctx, clientTransport, nil) + require.NoError(err) + defer clientSession.Close() + + listResult, err := clientSession.ListTools(ctx, &sdk.ListToolsParams{}) + require.NoError(err) + require.Len(listResult.Tools, 1, "Expected exactly one tool in filtered server") + + tool := listResult.Tools[0] + assert.Equal("safe_read", tool.Name) + require.NotNil(tool.Annotations, "Annotations must be forwarded to routed-mode clients") + assert.True(tool.Annotations.ReadOnlyHint, "ReadOnlyHint must be preserved") + require.NotNil(tool.Annotations.DestructiveHint, "DestructiveHint pointer must be preserved") + assert.True(*tool.Annotations.DestructiveHint, "DestructiveHint value must be preserved") +} + func TestGetToolHandler(t *testing.T) { cfg := &config.Config{ Servers: map[string]*config.ServerConfig{},