fix(github-mcp): vendor bash-mcp-sdk v2.0.0 to enforce union types - #5
Merged
Merged
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.