Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/streamable-http.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ tools without changing their advertised schemas. Full typed body validation
and defaults still run after tool authorization guards. Older protocol versions
do not require or validate these standard parameter headers.

Tools whose `_meta.ui.visibility` excludes `"model"` (app-only tools such as
`ui_get`) are deliberately left unannotated. Their only callers are MCP App
views, which reach the server through the host's `tools/call` proxy; the MCP
Apps bridge carries no HTTP headers, so a required projection would reject
every view lookup with `-32020`. These calls route from the body arguments
instead.

### Basic HTTP Server

Start the server on the default port (8082):
Expand Down
15 changes: 15 additions & 0 deletions pkg/github/header_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import (
// projected to an Mcp-Param-* header. This guards the per-request header
// optimization used by the remote proxy: a future tool must not silently ship
// without its owner/repo header, so it can never fall back to body re-parsing.
//
// App-only tools are the documented exception: their only callers are MCP App
// views, which cannot set request headers, so requiring projection on them
// would reject every view lookup (see issue #3834). They must instead stay
// unannotated, and this test enforces that they are.
func TestAllToolsRoutingParamsGetHeaders(t *testing.T) {
inv, err := NewInventory(stubTranslator).WithToolsets([]string{"all"}).Build()
require.NoError(t, err)
Expand All @@ -22,8 +27,10 @@ func TestAllToolsRoutingParamsGetHeaders(t *testing.T) {
require.NotEmpty(t, tools)

checked := 0
appOnlyChecked := 0
for _, st := range tools {
tool := st.Tool
appOnly := inventory.IsAppOnlyTool(&tool)
inventory.AnnotateHeaderParams(&tool)
schema, ok := tool.InputSchema.(*jsonschema.Schema)
if !ok || schema == nil {
Expand All @@ -38,11 +45,19 @@ func TestAllToolsRoutingParamsGetHeaders(t *testing.T) {
if !ok || ps == nil {
continue
}
if appOnly {
require.NotContainsf(t, ps.Extra, "x-mcp-header",
"app-only tool %q param %q must not require an Mcp-Param-%s header",
tool.Name, prop, header)
appOnlyChecked++
continue
}
require.NotNilf(t, ps.Extra, "tool %q param %q missing x-mcp-header annotation", tool.Name, prop)
require.Equalf(t, header, ps.Extra["x-mcp-header"],
"tool %q param %q must project to Mcp-Param-%s", tool.Name, prop, header)
checked++
}
}
require.Positive(t, checked, "expected at least one owner/repo param across all toolsets")
require.Positive(t, appOnlyChecked, "expected at least one app-only owner/repo param across all toolsets")
}
148 changes: 148 additions & 0 deletions pkg/http/app_tool_headers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package http

import (
"context"
"encoding/json"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/github/github-mcp-server/v2/pkg/github"
"github.com/github/github-mcp-server/v2/pkg/http/headers"
"github.com/github/github-mcp-server/v2/pkg/inventory"
"github.com/github/github-mcp-server/v2/pkg/translations"
"github.com/github/github-mcp-server/v2/pkg/utils"
"github.com/go-chi/chi/v5"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestAppOnlyToolsDoNotRequireProjectedHeaders pins the Streamable HTTP header
// contract for app-only tools. MCP App views reach the server through the
// host's tools/call proxy, and the MCP Apps bridge (App.callServerTool) gives a
// view no way to set HTTP request headers, so requiring Mcp-Param-owner on
// ui_get makes every view lookup fail with -32020 in hosts that do not project
// headers for proxied calls (issue #3834). Model-visible tools keep the
// projection requirement.
func TestAppOnlyToolsDoNotRequireProjectedHeaders(t *testing.T) {
router := newAppToolHeaderRouter(t)

uiGetArgs := map[string]any{"method": "labels", "owner": "octo-org", "repo": "hello-world"}

t.Run("app-only tool without projected headers reaches the handler", func(t *testing.T) {
rec := postToolCall(t, router, "ui_get", uiGetArgs, nil)
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
assert.NotContains(t, rec.Body.String(), "header mismatch")
})

t.Run("app-only tool with projected headers still works", func(t *testing.T) {
rec := postToolCall(t, router, "ui_get", uiGetArgs, map[string]string{
headers.MCPParamOwnerHeader: "octo-org",
headers.MCPParamRepoHeader: "hello-world",
})
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
assert.NotContains(t, rec.Body.String(), "header mismatch")
})

t.Run("model-visible tool still requires projected headers", func(t *testing.T) {
rec := postToolCall(t, router, "issue_write", map[string]any{
"method": "create", "owner": "octo-org", "repo": "hello-world", "title": "x",
}, nil)
require.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String())
assert.Contains(t, rec.Body.String(), "-32020")
// Annotation iteration order is not fixed, so assert on the shared
// prefix instead of naming one projected parameter.
assert.Contains(t, rec.Body.String(), "header mismatch: missing Mcp-Param-")
})

t.Run("model-visible tool with projected headers reaches the handler", func(t *testing.T) {
rec := postToolCall(t, router, "issue_write", map[string]any{
"method": "create", "owner": "octo-org", "repo": "hello-world", "title": "x",
}, map[string]string{
headers.MCPParamOwnerHeader: "octo-org",
headers.MCPParamRepoHeader: "hello-world",
})
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
})
}

// newAppToolHeaderRouter builds the production HTTP handler around the real
// GitHub inventory, with tools/call results stubbed so the boundary behaviour
// can be asserted without credentials.
func newAppToolHeaderRouter(t *testing.T) http.Handler {
t.Helper()

apiHost, err := utils.NewAPIHost("https://api.github.com")
require.NoError(t, err)

handler := NewHTTPMcpHandler(
context.Background(),
&ServerConfig{Version: "test"},
github.BaseDeps{},
translations.NullTranslationHelper,
slog.Default(),
apiHost,
WithInventoryFactory(func(*http.Request) (*inventory.Inventory, error) {
return github.NewInventory(translations.NullTranslationHelper).
WithToolsets([]string{"all"}).
Build()
}),
WithGitHubMCPServerFactory(func(r *http.Request, deps github.ToolDependencies, inv *inventory.Inventory, _ *github.MCPServerConfig) (*mcp.Server, error) {
server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil)
server.AddReceivingMiddleware(stubToolsCall)
inv.RegisterTools(r.Context(), server, deps)
return server, nil
}),
WithScopeFetcher(allScopesFetcher{}),
)

router := chi.NewRouter()
handler.RegisterMiddleware(router)
handler.RegisterRoutes(router)
return router
}

// stubToolsCall answers tools/call without running a tool handler, so these
// tests exercise only the Streamable HTTP boundary.
func stubToolsCall(next mcp.MethodHandler) mcp.MethodHandler {
return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) {
if method == "tools/call" {
return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: "stubbed"}}}, nil
}
return next(ctx, method, req)
}
}

func postToolCall(t *testing.T, router http.Handler, name string, args map[string]any, paramHeaders map[string]string) *httptest.ResponseRecorder {
t.Helper()

params, err := json.Marshal(map[string]any{
"name": name,
"arguments": args,
"_meta": map[string]any{
mcp.MetaKeyProtocolVersion: inventory.ProtocolVersionMultiRoundTrip,
mcp.MetaKeyClientInfo: map[string]any{"name": "test", "version": "1.0.0"},
mcp.MetaKeyClientCapabilities: map[string]any{},
},
})
require.NoError(t, err)

req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(
`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":`+string(params)+`}`))
req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON)
req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", "))
req.Header.Set(headers.MCPProtocolVersionHeader, inventory.ProtocolVersionMultiRoundTrip)
req.Header.Set(headers.MCPMethodHeader, "tools/call")
req.Header.Set(headers.MCPNameHeader, name)
req.Header.Set(headers.AuthorizationHeader, "ghs_test-token")
for key, value := range paramHeaders {
req.Header.Set(key, value)
}

recorder := httptest.NewRecorder()
router.ServeHTTP(recorder, req)
return recorder
}
81 changes: 81 additions & 0 deletions pkg/inventory/app_only_typed_schema_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package inventory

import (
"context"
"encoding/json"
"testing"

"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestTypedAppOnlyToolOmitsInferredHeaderAnnotations pins the header contract
// for app-only tools whose input schema is INFERRED from the handler's input
// type, rather than declared explicitly as ui_get does. The two tools below
// share one input type: the inferred schema is cached per type, so the
// annotation has to be selected per tool visibility when it is read out of the
// cache. Annotating the app-only one makes every view call fail with -32020 in
// hosts that do not project headers for proxied calls (issue #3834).
func TestTypedAppOnlyToolOmitsInferredHeaderAnnotations(t *testing.T) {
handler := func(_ context.Context, _ *mcp.CallToolRequest, input typedTestInput) (*mcp.CallToolResult, typedTestOutput, error) {
return nil, typedTestOutput{Query: input.Query}, nil
}

appOnlyTool := NewServerToolWithContextHandler(
mcp.Tool{
Name: "typed_app_only_tool",
Meta: mcp.Meta{
"ui": map[string]any{
"visibility": []string{"app"},
},
},
},
testToolsetMetadata("test"),
handler,
)
require.Nil(t, appOnlyTool.Tool.InputSchema, "the schema under test must be inferred")
require.True(t, IsAppOnlyTool(&appOnlyTool.Tool))

modelTool := NewServerToolWithContextHandler(
mcp.Tool{Name: "typed_model_tool"},
testToolsetMetadata("test"),
handler,
)

server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil)
modelTool.RegisterFunc(server, nil)
appOnlyTool.RegisterFunc(server, nil)
session := connectTypedTestClient(t, server, "")

list, err := session.ListTools(context.Background(), nil)
require.NoError(t, err)
require.Len(t, list.Tools, 2)

schemas := make(map[string]map[string]any, len(list.Tools))
for _, tool := range list.Tools {
var schema map[string]any
require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, tool.InputSchema)), &schema))
schemas[tool.Name] = schema
}

appOnlyProperties := schemas["typed_app_only_tool"]["properties"].(map[string]any)
assert.NotContains(t, mustMarshalJSON(t, appOnlyProperties["owner"]), "x-mcp-header",
"an app-only tool must not project owner onto Mcp-Param-owner")
assert.NotContains(t, mustMarshalJSON(t, appOnlyProperties["repo"]), "x-mcp-header",
"an app-only tool must not project repo onto Mcp-Param-repo")

modelProperties := schemas["typed_model_tool"]["properties"].(map[string]any)
assert.Equal(t, "owner", modelProperties["owner"].(map[string]any)["x-mcp-header"],
"the model-visible tool that shares the input type keeps its routing metadata")
assert.Equal(t, "repo", modelProperties["repo"].(map[string]any)["x-mcp-header"])

result, err := session.CallTool(context.Background(), &mcp.CallToolParams{
Name: "typed_app_only_tool",
Arguments: map[string]any{"owner": "octo", "repo": "hello", "query": "is:open"},
})
require.NoError(t, err)
require.False(t, result.IsError)
assert.JSONEq(t, `{"query":"is:open"}`, mustMarshalJSON(t, result.StructuredContent),
"owner/repo stay in the call arguments even without the header projection")
}
16 changes: 12 additions & 4 deletions pkg/inventory/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"maps"
"slices"
"strings"

"github.com/modelcontextprotocol/go-sdk/mcp"
)

var (
Expand Down Expand Up @@ -393,11 +395,11 @@ func stripMCPAppsMetadata(tools []ServerTool) []ServerTool {
return result
}

// isAppOnlyTool reports whether a tool declares MCP Apps visibility that
// excludes the model (e.g. `_meta.ui.visibility: ["app"]`). Such tools are
// IsAppOnlyTool reports whether a tool definition declares MCP Apps visibility
// that excludes the model (e.g. `_meta.ui.visibility: ["app"]`). Such tools are
// only meant to be called by MCP App views, never by the model directly.
func isAppOnlyTool(tool *ServerTool) bool {
ui, ok := tool.Tool.Meta["ui"].(map[string]any)
func IsAppOnlyTool(tool *mcp.Tool) bool {
ui, ok := tool.Meta["ui"].(map[string]any)
if !ok {
return false
}
Expand All @@ -417,6 +419,12 @@ func isAppOnlyTool(tool *ServerTool) bool {
return len(visibility) > 0 && !slices.Contains(visibility, "model")
}

// isAppOnlyTool reports whether a tool declares MCP Apps visibility that
// excludes the model.
func isAppOnlyTool(tool *ServerTool) bool {
return IsAppOnlyTool(&tool.Tool)
}

// stripMetaKeys removes the specified Meta keys from a single tool.
// Returns a modified copy if changes were made, nil otherwise.
func stripMetaKeys(tool ServerTool, keys []string) *ServerTool {
Expand Down
37 changes: 37 additions & 0 deletions pkg/inventory/header_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,40 @@ func TestTypedHeaderParamsHTTP(t *testing.T) {
}
}
}

// TestAnnotateHeaderParamsSkipsAppOnlyTools pins the exemption for tools that
// only MCP App views can call. Views reach the server through the host's
// tools/call proxy and App.callServerTool carries no HTTP headers, so an
// x-mcp-header binding on an app-only tool makes the SDK reject every view
// lookup with -32020 at the HTTP boundary.
func TestAnnotateHeaderParamsSkipsAppOnlyTools(t *testing.T) {
appOnly := &mcp.Tool{
Name: "ui_get",
Meta: mcp.Meta{"ui": map[string]any{"visibility": []string{"app"}}},
InputSchema: &jsonschema.Schema{
Type: "object",
Properties: map[string]*jsonschema.Schema{"method": {Type: "string"}, "owner": {Type: "string"}, "repo": {Type: "string"}},
Required: []string{"method", "owner"},
},
}
AnnotateHeaderParams(appOnly)
schema := appOnly.InputSchema.(*jsonschema.Schema)
require.Nil(t, schema.Properties["owner"].Extra)
require.Nil(t, schema.Properties["repo"].Extra)

// Visibility that includes the model keeps the projection requirement.
for _, visibility := range [][]string{{"model", "app"}, {"model"}} {
modelVisible := &mcp.Tool{
Name: "issue_read",
Meta: mcp.Meta{"ui": map[string]any{"visibility": visibility}},
InputSchema: &jsonschema.Schema{
Type: "object",
Properties: map[string]*jsonschema.Schema{"owner": {Type: "string"}, "repo": {Type: "string"}},
},
}
AnnotateHeaderParams(modelVisible)
schema := modelVisible.InputSchema.(*jsonschema.Schema)
require.Equal(t, "owner", schema.Properties["owner"].Extra["x-mcp-header"])
require.Equal(t, "repo", schema.Properties["repo"].Extra["x-mcp-header"])
}
}
Loading