Skip to content

fix: ensure Mcp-Param- headers are passed when required - #14

Merged
thedadams merged 1 commit into
obot-platform:mainfrom
thedadams:push-loprkssnksut
Sep 12, 2026
Merged

thedadams merged 1 commit into
obot-platform:mainfrom
thedadams:push-loprkssnksut

Conversation

@thedadams

Copy link
Copy Markdown
Member

A tool can specify that certain headers are required when using the streamable http transport. In the stateless protocol, the tools/call won't have information about which headers are required. This change caches the tool (not just the name) so that the proper headers can be included on the tools/call request.

Issue: obot-platform/obot#7840

Copilot AI balanced review requested due to automatic review settings September 12, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A moderate correctness issue and a test coverage gap remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates tool routing to retain definitions and generate Mcp-Param-* headers for downstream HTTP calls.

Changes:

  • Cache routed tool definitions and propagate call context.
  • Generate encoded parameter headers from tool schemas.
  • Add stateless/stateful integration coverage and update route tests.

Review findings:

  • component/http/client.go:389 — Moderate: large integer values can be lost and headers silently omitted.
  • http_param_headers_test.go:24 — Nit: the test does not assert generated downstream headers.
File summaries
File Description
http_param_headers_test.go Adds parameter-header integration coverage.
handler.go Propagates tool definitions during calls.
component/http/client.go Generates downstream parameter headers.
component/component.go Stores tool-call context.
catalog/routes.go Retains tools in routes.
catalog/compiler.go Populates tool route definitions.
catalog/compiler_test.go Updates route assertions.
catalog/catalog_test.go Updates route assertions.
Review details

Suppressed comments (1)

component/http/client.go:391

  • json.Unmarshal into any produces a float64, and this guard drops every integer outside the ±(2^53-1) range. JSON Schema integer arguments can validly be larger (for example, a 64-bit ID); when such a property has x-mcp-header, silently omitting the header breaks the downstream call. Decode numbers with UseNumber or preserve the raw decimal token instead of rejecting them.
		if value != math.Trunc(value) || value < -(1<<53-1) || value > 1<<53-1 {
			return "", false
		}
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread http_param_headers_test.go
A tool can specify that certain headers are required when using the
streamable http transport. In the stateless protocol, the tools/call
won't have information about which headers are required. This change
caches the tool (not just the name) so that the proper headers can be
included on the tools/call request.

Signed-off-by: Donnie Adams <donnie@obot.ai>
@thedadams
thedadams merged commit 4d3b495 into obot-platform:main Sep 12, 2026
4 checks passed
@thedadams
thedadams deleted the push-loprkssnksut branch September 12, 2026 17:10
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.

3 participants