Skip to content

fix(inventory): do not require Mcp-Param headers on app-only tools - #3854

Open
Shanmukh-l33t wants to merge 2 commits into
github:mainfrom
Shanmukh-l33t:fix/app-only-tools-skip-param-headers
Open

Shanmukh-l33t wants to merge 2 commits into
github:mainfrom
Shanmukh-l33t:fix/app-only-tools-skip-param-headers

Conversation

@Shanmukh-l33t

@Shanmukh-l33t Shanmukh-l33t commented Nov 15, 2026 •

Copy link
Copy Markdown

Problem

While the issue_write MCP App form loads in VS Code, the form shows:

-32020: header mismatch: missing Mcp-Param-owner header

It happens "on almost every form we open" and clears on retry. Fixes #3834.

Root cause

Two facts combine:

  1. The form's metadata lookups are bridge calls, not model calls. ui/src/hooks/useMcpApp.ts:97 calls app.callServerTool({ name, arguments: args }). App.callServerTool posts a JSON-RPC tools/call over the MCP Apps PostMessageTransport and the host replays it against the server. A view has no API for HTTP request headers, so no code in ui/ can project Mcp-Param-* values.
  2. pkg/inventory/server_tool.go:290 AnnotateHeaderParams annotated owner and repo with x-mcp-header on every tool that has those properties, including ui_get. ui_get declares _meta.ui.visibility: ["app"] (pkg/github/ui_tools.go:47), so views are its only callers.

The go-sdk Streamable HTTP boundary then requires the header whenever the argument is present (go-sdk v1.8.0 mcp/streamable_headers.go:359 validateMcpHeaders, :402 validateParamHeaders) for Mcp-Protocol-Version >= 2026-07-28. Any host that does not project headers for proxied app calls therefore gets a hard -32020 on every view lookup: labels (ui/src/apps/issue-write/App.tsx:550), assignees (:577), milestones (:603), issue types (:631), issue fields (:659).

So the header requirement on ui_get was unsatisfiable for the only caller class that can reach it. The issue_types detail in the report is a red herring: issue_types legitimately sends only owner, and repo stays unannotated for that call because the annotation only applies to a non-null argument.

What changed

pkg/inventory/server_tool.go AnnotateHeaderParams now returns early for tools whose ui.visibility excludes "model", using the existing visibility logic exposed as inventory.IsAppOnlyTool (pkg/inventory/builder.go:401). Three consequences, all intended:

  • ui_get is no longer annotated in tools/list, so the SDK does not reject its calls when the header is absent.
  • owner/repo remain in the call arguments, so the handler and any proxy can still read them from the body.
  • Model-visible tools keep exactly the same projection and enforcement; hosts that do project headers for a view call keep working, because an unannotated parameter simply ignores the extra header.

docs/streamable-http.md records the exception. No tool names, schemas, UI, or dependencies change, so the generated README catalog is unaffected (verified below).

Tests

pkg/http/app_tool_headers_test.go drives the production NewHTTPMcpHandler around the real GitHub inventory at Mcp-Protocol-Version: 2026-07-28 and asserts four cases: app-only call without headers returns 200, app-only call with headers returns 200, issue_write without headers returns 400 -32020, issue_write with headers returns 200.

Red at the parent commit 0663b83 (fix removed, test files kept):

  • go test ./pkg/http/ -run 'AppOnly' -count=1 -v
    Subtest app-only_tool_without_projected_headers_reaches_the_handler fails: expected: 200, actual: 400, body {"jsonrpc":"2.0","id":1,"error":{"code":-32020,"message":"header mismatch: missing Mcp-Param-owner header for parameter \"owner\""}}, the exact string from the issue. The other three subtests pass, so the failure is specific to the app-only exemption.
  • go test ./pkg/inventory/ -run 'TestAnnotateHeaderParamsSkipsAppOnlyTools' -count=1 -v
    Fails with Expected nil, but got: map[string]interface {}{"x-mcp-header":"owner"}.
  • TestAllToolsRoutingParamsGetHeaders in pkg/github cannot be red at the parent commit: it calls the newly exported inventory.IsAppOnlyTool, so on unmodified main it fails to build with undefined: inventory.IsAppOnlyTool. Its new assertion (app-only tools must stay unannotated) is only meaningful together with the export, and it passes with this change.

Green with the fix:

  • go build ./...: clean.
  • go test ./pkg/inventory/... ./pkg/github/ -run 'Header' -count=1: ok pkg/inventory 0.110s, ok pkg/github 0.042s.
  • go test ./pkg/http/ -run 'AppOnly' -count=1 -v: PASS, all 4 subtests, ok pkg/http 0.125s.
  • go test ./pkg/inventory/... ./pkg/http/... ./pkg/github/ -count=1 -p 2: ok for pkg/inventory (0.471s), pkg/http (0.896s), pkg/http/headers (0.003s), pkg/http/middleware (0.005s), pkg/http/oauth (0.022s), pkg/http/servercard (0.003s), pkg/http/transport (0.011s) and pkg/github (19.847s); pkg/http/mark has no test files. 0 failures.
  • gofmt -s -l over the five changed Go files: no output. go vet ./pkg/inventory/... ./pkg/http/... ./pkg/github/: clean. golangci-lint run ./pkg/inventory/... ./pkg/http/ ./pkg/github/ (v2.14.0): 0 issues.
  • go run ./cmd/github-mcp-server generate-docs: git status afterwards is empty, so the generated docs and README catalog already match this branch and no regenerated diff is missing.

MCP impact

  • No tool or API changes

No tool is added, removed, renamed, or reshaped. ui_get stops advertising x-mcp-header on owner/repo; model-visible tools and their header contract are byte for byte unchanged.

Security / limits

  • No security or limits impact

The exemption applies only to tools whose declared _meta.ui.visibility excludes "model". Across the full tool inventory (136 tools with all toolsets enabled) that is ui_get alone, measured on this branch. It removes a routing hint, not an authorization check: scope challenges, availability guards, repository routing and body validation are untouched, and the arguments still carry owner/repo for the proxy.

Tool renaming

  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with golangci-lint run on the changed packages (0 issues)
  • Tested locally with ./script/test (the full -race sweep was not run to keep load off shared hardware; the affected packages were run in full, as listed above)

Docs

  • Updated (docs / examples)

docs/streamable-http.md documents the app-only exception. script/generate-docs produced no further diff.

Skills

  • Updated relevant skills or confirmed no bundled skill applies

No tool or toolset instruction changed, so no bundled skill text or instruction snapshot moves; generate-docs confirmed the catalog is identical.

Scope note

This removes the error for the form's own lookups. A view-initiated call to a model-visible tool (issue_write submit from the form) is still subject to header projection, so hosts that skip projection for proxied calls can still surface -32020 there once and recover on retry; that path is the host's projection gap, not something the tool contract can satisfy on the view's behalf.

ui_get declares `_meta.ui.visibility: ["app"]`, so its only callers are MCP
App views. Views reach the server through the host's tools/call proxy and
App.callServerTool posts JSON-RPC over the MCP Apps bridge, which gives a view
no way to set HTTP request headers. Because the generic owner/repo projection
in AnnotateHeaderParams annotated ui_get too, the go-sdk Streamable HTTP
boundary rejected every view lookup on protocol 2026-07-28 with
`-32020 header mismatch: missing Mcp-Param-owner header`, which is what the
issue_write form surfaced while loading labels, assignees, milestones, issue
types and custom fields.

Skip the annotation for tool definitions whose ui.visibility excludes "model".
Owner and repo stay in the call arguments for those tools, and model-visible
tools keep the projection requirement unchanged.
@Shanmukh-l33t
Shanmukh-l33t requested a review from a team as a code owner November 15, 2026 06:11
Copilot AI dynamic review requested due to automatic review settings November 15, 2026 06:11

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.

🟡 Changes recommended

App-only tools registered with inferred input schemas and concrete output types still receive x-mcp-header bindings, causing headerless app calls to be rejected.

1 open issue
What changed in this PR

This PR addresses #3834 by exempting app-only tools such as ui_get from required Mcp-Param-* headers, allowing form metadata lookups through hosts that do not project those headers.

Changes:

  • Reuses exported visibility detection to skip header annotations for app-only tools.
  • Adds unit and HTTP regression coverage while preserving model-visible header requirements.
  • Documents the app-only exception.
File Description
pkg/​inventory/​server_tool.go Skips header annotations for app-only tools.
pkg/​inventory/​header_params_test.go Tests app-only exemption and model-visible annotations.
pkg/​inventory/​builder.go Exports the existing app-only visibility helper.
pkg/​http/​app_tool_headers_test.go Tests HTTP calls with and without projected headers.
pkg/​github/​header_params_test.go Checks the exemption across the GitHub tool inventory.
docs/​streamable-http.md Documents body-based routing for app-only calls.

Review effort: Balanced (dynamic)

Comment on lines +297 to +299
if IsAppOnlyTool(tool) {
return
}
An app-only tool that declares its input schema is already skipped by
AnnotateHeaderParams. One that lets the SDK infer the schema from its input
type was not: prepareTyped asks CachedInputSchemaFor, which annotates the
schema while building the shared per-type cache entry, where no tool metadata
is available to consult.

Cache the unannotated form alongside the annotated one and pick by
visibility when the schema is handed to a tool, so an app-only tool and a
model-visible tool that share an input type each get the right projection.
The regression test registers both tools over one input type and asserts the
app-only schema carries no x-mcp-header while the model-visible twin keeps
owner and repo.

Refs github#3834
@Shanmukh-l33t

Copy link
Copy Markdown
Author

Good catch, and it is a real hole rather than a stylistic one. Confirmed at source on a915221: prepareTyped calls CachedInputSchemaFor[In], and that annotates inside cachedSchemaFor while building the per-type entry, on a synthetic mcp.Tool{InputSchema: schema} that carries no Meta, so IsAppOnlyTool cannot see the tool's visibility. ui_get declares its schema explicitly and was already covered by the early return, but any app-only typed tool with an inferred input schema would keep the binding and stay rejected for its only caller class.

Fixed in 62e2188 by making the schema selection visibility aware:

  • cachedSchemaFor now keeps the unannotated schema next to the annotated one in the same entry (they are built in the same sync.Once, so there is still one inference per type and no new cache key).
  • CachedInputSchemaForTool[In](appOnly, ...) picks the variant for a specific tool and prepareTyped calls it with IsAppOnlyTool(tool). CachedInputSchemaFor keeps its current annotated behaviour for the direct callers that expect routing metadata.
  • Nothing model-visible changes: registerTyped still derives the boundary header schema from the same modernTool.InputSchema, and that schema keeps x-mcp-header, so enforcement for model-visible tools is byte for byte the same.

Regression test added, pkg/inventory/app_only_typed_schema_test.go, through typed registration as you asked: one handler, one input type, two tools registered on one server. The app-only tool must advertise no x-mcp-header on owner and repo, and the model-visible tool that shares the input type must keep both. The shared input type is the point, since it is the cache that leaked the annotation.

Red and green, run locally:

  • go test ./pkg/inventory/ -run TestTypedAppOnlyToolOmitsInferredHeaderAnnotations -count=1 -v with only the two source hunks stashed: FAIL, "{\"type\":\"string\",\"x-mcp-header\":\"owner\"}" should not contain "x-mcp-header" and the same for repo. With the fix: PASS.
  • go test ./pkg/inventory/... ./pkg/http/... ./pkg/github/ -count=1 -p 2: ok for inventory, http, http/headers, http/middleware, http/oauth, http/servercard, http/transport and github (14.8s), 0 failures. The pkg/http app-only tests from the first commit still pass unchanged.
  • go vet clean, gofmt -s -l clean, golangci-lint run ./pkg/inventory/... ./pkg/http/ ./pkg/github/ reports 0 issues.

The test also asserts the arguments keep owner/repo when the projection is absent, so the handler and any proxy can still read them from the body.

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.

Insiders Feedback: issue_write form in VS Code shows -32020 header mismatch: missing Mcp-Param-owner header while loading repository metadata

2 participants