fix(github-mcp): vendor bash-mcp-sdk v2.0.0 to enforce union types - #5
Conversation
Every numeric identifier parameter in tools-read.json and tools-write.json declares `"type": ["integer", "string"]` so clients can send an issue or PR number either way. The pinned SDK v1.0.0 read a list-valued `type` as no constraint at all, so those 37 parameters accepted any value — a boolean, a float — and passed it straight to gh. v2.0.0 checks the value against each alternative and rejects one that matches none, naming both: `number expected integer or string, got boolean`. The tool schemas already ship the shape v2.0.0 wants, so nothing in mcp-server-gh changes. The schema suite's "default satisfies its own schema" check normalizes a list-valued `type` the same way, so a future union-typed default cannot slip past it, and the External References entry now points at the SDK this server actually vendors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The union-type fix here is a sharp catch — a list-valued Verified that one independently rather than take the comment's word for it: Both hold exactly as documented, so |
Bumps the vendored protocol handler from
shopwareLabs/bash-mcp-sdkv1.0.0 to v2.0.0 and corrects the two places in the docs that named the old pin.Why
Every numeric identifier parameter across
tools-read.jsonandtools-write.jsondeclares"type": ["integer", "string"], because clients send an issue or PR number both ways. v1.0.0 of the SDK read a list-valuedtypeas no constraint at all — the validator's type check selected on($t | type) == "string"and skipped everything else — so those 37 parameters were unvalidated. A boolean or a float reachedghuntouched.v2.0.0 normalizes a declared
typeto a list and accepts a value that matches any member, on a property and onitems.typealike. A list that is empty or holds a non-string member is treated as malformed and left unenforced rather than rejecting everything.What changes for callers
issue_view {"number": 339}issue_view {"number": "339"}issue_view {"number": true}ghisError:number expected integer or string, got booleanissue_view {"number": 3.5}ghisError:number expected integer or string, got number (non-integer)The SDK calls this a major release for that reason. In this repository the rejected shapes are calls no client makes deliberately, and the stricter-validation entry it extends is still under
[Unreleased], so no version bump accompanies it.Contents
.mcp-sdk.lock→v2.0.0, andplugins/github-mcp/shared/mcpserver_core.shre-vendored by.github/scripts/vendor-mcp-sdk.sh. The file is byte-identical tolib/mcpserver_core.shat that tag; the validator is the only thing that moved between the two releases.plugins/github-mcp/CHANGELOG.md: the[Unreleased]entry namedv1.0.0and stated that the pinned SDK does not enforce array-valued types. Both are now false, so the entry says what v2.0.0 does instead.plugin-tests/github-mcp/tool_schemas.bats: the "default satisfies the constraints declared beside it" check only looked at string-valuedtype, so a default on a union-typed property skipped it entirely. It now normalizes the declaration the same way the validator does. No shipped schema has such a default today — this keeps a future one from slipping past.plugins/github-mcp/AGENTS.md: External References pointed atmuthuishere/mcp-server-bash-sdk, which the vendoring superseded. It now names the SDK the file actually comes from and the lock that pins it.No test was added for the union enforcement itself. That is the SDK's own behavior and it is covered by
tests/mcp_argument_validation.batsupstream; this repository tests the seam, not its dependency.