Bumps github.com/modelcontextprotocol/go-sdk from v1.6.0 to v1.6.1 and
removes the CrossOriginProtection bypass we previously installed on the
StreamableHTTP handler.
As of go-sdk v1.6.0 the cross-origin check is opt-in: a nil
CrossOriginProtection on StreamableHTTPOptions means no check is run.
v1.6.1 also marks the field itself as deprecated (the SDK recommends
wrapping the handler with middleware instead, and the field will be
removed in v1.8.0).
This server authenticates via bearer tokens, not cookies, so the
Sec-Fetch-Site CSRF check is unnecessary and would block browser-based
MCP clients. Leaving CrossOriginProtection unset preserves that
behavior without depending on a deprecated API.
Supersedes #2541.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The generic map decoding path for project field values treated 'name'
(ProjectV2FieldOption) and 'title' (ProjectV2FieldIteration) as plain
strings, but the GitHub API returns them as ProjectV2TextContent objects
with raw/html fields. As a result, single-select option names and
iteration titles could be returned empty when values reached the
minimal converter as map[string]any instead of typed structs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Return compact project item content and field values from project item tools to avoid verbose issue and pull request payloads.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mergeIssueFieldValues built the merged slice by iterating a Go map,
which produces non-deterministic ordering and caused a flake in
Test_UpdateIssue/partial_update_with_issue_fields_reconciled_by_names
(introduced in #2551). Switch to an order-preserving merge: emit
incoming entries first in their original order, then any existing
entries (in their original order) whose field IDs weren't seen in
incoming. Semantics (incoming wins, existing preserved) unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Companion to Test_GetIssue_FieldValues: when remote_mcp_issue_fields is
enabled, the GraphQL nodes() round-trip populates the enriched
field_values while the raw REST issue_field_values stays cleared.
Addresses the Copilot review suggestion on #2558.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the shared buildIssueWrite(includeIssueFields) helper with two
fully duplicated tool definitions. When the FeatureFlagIssueFields flag
is retired, LegacyIssueWrite can be deleted as a single function with no
merge thinking required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ports the gating from PR #2553 onto main (the original merge landed on a
stack base that did not make it to main).
Changes:
- pkg/inventory: FeatureFlagDisable becomes []string (any-listed-on → hide).
FeatureFlagEnable stays as a single string. This avoids the AND-of-enable
semantics from the earlier proposal, which encoded dependencies rather
than rollout knobs and had no real call site. Disable-OR is the case
that does need the slice (LegacyIssueWrite below).
- pkg/github/issues.go: split IssueWrite into IssueWrite (flag-enabled,
exposes issue_fields) and LegacyIssueWrite (flag-disabled, omits it).
Both register as 'issue_write'; mutually exclusive flag annotations
pick exactly one at runtime. Refactored into a shared buildIssueWrite
helper instead of duplicating the ~250-line tool definition.
- pkg/github/issues.go: GetIssue field_values enrichment now requires
the flag at runtime. The verbose REST IssueFieldValues is always
cleared from the response.
- Existing single-flag Disable call sites converted to slices.
- New toolsnap variant issue_write_ff_remote_mcp_issue_fields.snap; the
canonical issue_write.snap is owned by LegacyIssueWrite.
- README + flag docs regenerated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Expose fullDatabaseId (BigInt) in list_issue_fields
- Add issue_fields parameter to issue_write for setting field values
- Support single-select fields via field_option_name resolution
- Add REST API field value extraction in get_issue responses
- Update minimal types with IssueFieldValue for REST responses
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
X-Forwarded-Host and X-Forwarded-Proto were unconditionally honored when
constructing OAuth resource metadata URLs. In HTTP-mode deployments that
do not set --base-url and are not fronted by a proxy that strips these
headers, this lets an on-path client influence the URL advertised in
WWW-Authenticate and the /.well-known/oauth-protected-resource body.
This is a hardening change rather than a true vulnerability — exploiting
it requires HTTP without --base-url plus an attacker already positioned
to inject the header — but the unsafe default is worth closing.
Default behavior now derives host/scheme from r.Host and the TLS state.
Setups that rely on a trusted internal forwarder (e.g. an in-cluster
gateway that needs to preserve the originating hostname per request) can
opt back in with --trust-proxy-headers / GITHUB_TRUST_PROXY_HEADERS=1.
--base-url continues to take precedence in all cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds two auto-generated documentation sections that describe how
feature flags shape the tool surface:
- docs/insiders-features.md gets a per-flag block under its existing
hand-written prose. Each Insiders flag whose tools differ from the
default surface is listed with the full tool schema rendered through
the same writer used for README, so contributors can see exactly what
Insiders Mode adds or changes.
- docs/feature-flags.md is new and gives the same treatment to every
flag in AllowedFeatureFlags (user-controllable flags). It links back
to the Insiders doc for the auto-enabled subset.
Both sections are produced by a single generator that diffs the
flag-on inventory against the default-flagged inventory and reports any
tool that is new or has a different InputSchema/Meta. No reason
classification - just tools and their schemas, kept intentionally
simple so contributors don't have to update the generator when adding
a new flag.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The mcp-server-diff action checks the baseline ref out into a separate
working directory and runs install_command there. Without prebuilt UI
artifacts, pkg/github/ui_dist/ is empty on the baseline side and
UIAssetsAvailable() returns false, producing a false-positive diff that
"adds" _meta.ui to MCP Apps tools on every PR.
Stash the artifacts to RUNNER_TEMP after the workflow's build-ui step,
then restore them from install_command so both the baseline and PR
checkouts register identical MCP Apps UI metadata.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The bare `github-mcp-server`, `mcpcurl`, and `e2e.test` rules matched
those names anywhere in the tree, which silently ignored new files
created under `cmd/github-mcp-server/` (the rule treats the directory
component as a match). The intent was to ignore the binaries produced
by `go build` at repo root, so anchor each rule with a leading slash.
The existing `cmd/github-mcp-server/github-mcp-server` rule on line 2
continues to ignore the binary when built inside the cmd directory.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(issues): gate issue-fields features behind remote_mcp_issue_fields flag
Gates the recently merged issue-fields work (list_issue_fields tool,
field_values enrichment on list_issues/search_issues, and field_filters
input on list_issues) behind a new feature flag, also enabled in
insiders mode.
- list_issues splits into two same-named registrations: the field-aware
variant requires the flag, while LegacyListIssues (FeatureFlagDisable)
preserves the prior schema and GraphQL selection set so disabled
callers don't pay the extra wire/server cost.
- search_issues skips the field-values lookup when the flag is off.
- list_issue_fields requires the flag to be registered at all.
- Adopts <tool>_ff_<flag>.snap naming for flagged toolsnap variants so
same-named duplicates each get a distinct snapshot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: address PR review on issue-fields gating
- docs generator: install a no-flags feature checker so README reflects
the default user experience (tools enabled with no special flags),
fixing duplicate `list_issues` and removing granular/flagged-only
tools that were never meant to appear in the default docs.
- csv_output: drop the FeatureFlagEnable/Disable exclusion in
isCSVOutputTool. Wrapping happens before the per-request flag filter
picks the live variant, so flag-gated list_* tools wrap safely; this
restores CSV conversion for `list_issues` and enables it for
`list_issue_fields` when both flags are on.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add CSV output for list tools under insiders mode
* fix: resolve rebase feature flag conflicts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify feature-flag handling: collapse CSV dual-variant + skip filtering when no checker (#2516)
* refactor: generic toolset+name sort, clarify feature flag intent
Address review feedback on #2450:
- Collapse the three near-identical sort helpers in pkg/inventory/filters.go
into a generic sortByToolsetThenName so adding new inventory item types
doesn't require copying the comparator.
- Expand the doc comments on the three *WithoutFeatureFiltering helpers to
spell out why they exist: HTTP mode builds a static (process-wide)
inventory as an upper bound, but per-request feature flags from headers
(X-MCP-Features, X-MCP-Insiders) are evaluated later, so feature-flagged
variants must be preserved here.
- Strengthen the doc comment on ResolveFeatureFlags to make the contract
explicit: user-supplied flags are validated against AllowedFeatureFlags,
but insiders expansion deliberately is not — InsidersFeatureFlags may
include server-controlled flags that are not user-toggleable.
CORS comments are intentionally left for the PR author.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs(feature-flags): clarify allowed and insiders sets are independent
Also add tests covering:
- a user-toggleable flag (FeatureFlagIssuesGranular) that insiders does
not turn on automatically
- insiders mode not turning on user-only allowed flags
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(inventory): collapse three *WithoutFeatureFiltering helpers into StaticUpperBound
The three parallel methods (AvailableToolsWithoutFeatureFiltering,
AvailableResourceTemplatesWithoutFeatureFiltering,
AvailablePromptsWithoutFeatureFiltering) were always called as a triple
in exactly two places: HTTP buildStaticInventory and its test mirror.
They exist because the dual-variant pattern (sibling tools with mirrored
FeatureFlagEnable / FeatureFlagDisable on the same name, e.g. CSV output)
makes feature filtering at static-build time impossible — both variants
must be kept and resolved per-request.
Replace the three with one method, Inventory.StaticUpperBound(ctx), that
returns (tools, resources, prompts) and carries the rationale in its
doc comment. Reduces API surface, eliminates the triplication, and makes
the single "skip feature filtering" concept obvious to readers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: simplify feature-flag handling
Two related simplifications, both about treating insiders as a meta flag
that expands once at startup and then stops mattering:
- Collapse CSV's dual-variant pattern into a single tool whose handler
performs a runtime feature-flag check via deps.IsFeatureEnabled. CSV
is a pure response-format toggle, not a schema change, so it does not
need the dual-name pattern that genuine schema variants (granular
issues/PRs) still use.
- When no feature checker is installed, skip feature-flag filtering and
return the full upper bound. The static HTTP inventory now uses plain
AvailableTools/Resources/Prompts; the per-request inventory always
installs a checker, so MCP registration (which serves a tool name once)
always sees a deduplicated set. The bespoke StaticUpperBound helper and
the isToolEnabledWithFeatureFlags split go away.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* ci(mcp-diff): add insiders + per-feature configs
The mcp-diff matrix now includes:
- --insiders (and --insiders --read-only)
- one config per github.AllowedFeatureFlags entry, generated by
script/print-mcp-diff-configs so new user-controllable flags get
diffed automatically without editing the workflow
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs(insiders): explain feature-flag resolution for contributors
Adds a 'How feature flags are resolved' section covering:
- Insiders is a meta flag, like 'all'/'default' for toolsets
- User input -> allowlist filter -> insiders expansion ->
server-side fallback (remote only)
- AllowedFeatureFlags vs InsidersFeatureFlags are independent
- How to add a new feature flag, including the
TestGitHubPackageDoesNotReadInsidersMode guard
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(inventory): make feature-flag gating a regular ToolFilter
Move tool feature-flag evaluation out of isToolEnabled and into a
ToolFilter installed at the head of the pipeline by Build() when
WithFeatureChecker received a non-nil checker. The 'no checker = no
filtering' contract is now expressed structurally (the filter isn't
installed) instead of by a runtime nil check inside the helper.
Resources and prompts have no filter pipeline, so they call the now-pure
featureFlagAllowed helper behind an explicit r.featureChecker != nil
guard at the iteration site.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* perf(inventory): cache extracted toolset IDs in sort comparator
Avoid evaluating the extractor closures up to three times per comparison.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: correct MCP features header in cors
* docs: regenerate README for CSV output toolset
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: remove duplicate MCPFeaturesHeader from CORS headers
* ci(mcp-diff): add streamable-http job with header-based configs
Adds a sibling mcp-diff-http job that exercises the streamable-http
transport against a shared HTTP server, with per-config settings supplied
via X-MCP-* request headers — mirroring how the remote server is invoked
in production (server-side defaults + per-user header overrides).
The config generator gains a -transport flag:
- stdio (default, unchanged behaviour)
- http-headers (emits headers-only configs targeting a shared server)
Two new combined entries layer multiple headers together as a smoke test
for header-merging regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: regenerate after merging main
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Sam Morrow <info@sam-morrow.com>
Co-authored-by: sammorrowdrums <sammorrowdrums@github.com>
* Add custom field filtering to list_issues
* Flatten schema
* add repo fields flag
* test fix
---------
Co-authored-by: Sam Morrow <sammorrowdrums@github.com>
* Add list_org_issue_fields tool
* Clean up code
* complete struct fields & rename option type
* Drop created_at/updated_at from IssueField and IssueSingleSelectFieldOption
* Address feedback
* Address Copilot review: close resp.Body, set expectError=true for missing org test
* Adjust to list_issue_fields
* Add feature flag
* Allow tool to support read:org or repo
* Docs
* address comments
* Add repo_issue_fields flag
---------
Co-authored-by: Michael Jacholke <46944669+michaeljacholke@users.noreply.github.com>
* add `SearchCommits` tool
* run test
* run script/generate-docs
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* refactor(search_commits): share commit conversion, surface repo, tighten query docs
- Extract newMinimalCommitFromCore to share field mapping between
convertToMinimalCommit (RepositoryCommit) and the new
convertCommitResultToMinimalCommit (CommitResult), removing ~50
lines of duplicated logic from the search_commits handler.
- Add MinimalRepoRef and a search-only MinimalCommitSearchItem type
(embedding MinimalCommit) so cross-repo commit search results
identify the repo each commit came from. Keeping the field off
MinimalCommit avoids paying for a never-populated field on the
get_commit/list_commits output types.
- Rewrite the query description to teach the model the actual
commit-search qualifier surface (repo:/org:/user: scoping, author/
committer/date qualifiers, hash/tree/parent, merge:, is:public)
and reword the sort description to drop redundancy with the enum.
- Extend tests to assert the repository field is surfaced and to
cover commits with no resolved GitHub user (nil Author/Committer).
- Refresh README and toolsnap.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Sam Morrow <info@sam-morrow.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The current `search_code` query description is hand-wavy and gives the
model little usable guidance on GitHub code search syntax, which (per
analysis in #2390 across thousands of agent sessions) leads to repeated
422 ERROR_TYPE_QUERY_PARSING_FATAL responses from agents that guess at
plausible-but-invalid syntax.
Re-applies the spirit of #2442 by @jluocsa, originally suggested by
@danmoseley in #2390, but corrected against the actual endpoint this
tool calls.
Critically, this tool uses go-github's `client.Search.Code`, which hits
the legacy REST `/search/code` endpoint — NOT the new code search
("Blackbird"). Verified against the live API:
symbol:WithContext repo:github/github-mcp-server -> 0
/Get|Set/ repo:github/github-mcp-server -> 0
path:**/*.go func repo:github/github-mcp-server -> 0
filename:*.md repo:github/github-mcp-server -> 0
(Foo OR Bar) -path:vendor language:go -> 422
So `symbol:`, `/regex/`, path globs, filename globs, and parenthesized
boolean groups — features the proposal in #2442 listed — silently
return zero or fail. Documenting them would teach the model syntax
that doesn't work on this endpoint.
The new description focuses on what's actually supported by legacy
code search and the real bugs observed in #2390:
- `path:dir` is a prefix, NOT a glob (displaces `path:**/*.ts` guesses).
- `filename:exact.ext` is exact, NOT a glob (displaces `filename:*.md`).
- `/regex/` and `\|` inside quotes don't work — call this out so the
model stops generating them.
- `symbol:` doesn't work on this endpoint — call this out.
- Parenthesized boolean groups 422 — call this out so the model
stops wrapping `OR` chains in parens.
- Adds `extension:`, `in:file`, `in:path`, `size:`, `filename:`, `user:`
qualifiers that the previous text omitted.
- Implicit AND, `OR`, `NOT`, and `"quoted phrase"` for exact match are
documented positively.
- 256-char query limit.
All four examples in the new description are verified against the live
GitHub API and return non-zero results.
Co-authored-by: jluocsa <103165870+jluocsa@users.noreply.github.com>
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Include custom issue field values in list_issues response
Adds Issues 2.0 custom field values to each issue returned by the
list_issues GraphQL query, exposed on MinimalIssue as field_values:
[{field, value}]. Filtering by field is a separate concern (needs the
GraphQL IssueFilters input updated upstream) and is not included here.
shurcooL/graphql's response decoder walks every inline fragment of a
union regardless of __typename, so IssueFieldNumberValue.value is
aliased to valueNumber to avoid a Float-vs-String type clash when the
runtime variant is, e.g., a SingleSelectValue.
* Extend list_issues tests to cover Date/Number/Text field value variants
---------
Co-authored-by: Sam Morrow <info@sam-morrow.com>
Dynamic toolset discovery (the meta-tools enable_toolset, list_available_toolsets,
get_toolset_tools and the --dynamic-toolsets / GITHUB_DYNAMIC_TOOLSETS switch)
was a local-only feature never offered by the remote server. Removing it
deletes a meaningful chunk of branching, configuration surface and tests
for a path no longer in active use.
The deprecated closure-based NewServerToolWithDeps generic constructor was
only kept around for the dynamic tool registration path and is removed
together with it. Going forward there are exactly two constructors:
- NewServerTool — raw mcp.ToolHandler, no closure, no unmarshalling
- NewServerToolWithContextHandler[In, Out] — typed handler, deps via context
Inventory methods that only existed for the dynamic path
(ToolsForToolset, IsToolsetEnabled, EnableToolset, EnabledToolsetIDs)
are removed. ResolvedEnabledToolsets loses its dynamic flag.
Also strips dynamic references from the README, server configuration
docs, copilot-instructions, mcp-diff workflow, and conformance-test
script.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: simplify NewServerTool naming
- Rename NewServerToolWithRawContextHandler -> NewServerTool. This is the
preferred constructor for raw mcp.ToolHandler tools because it avoids
creating closures at registration time, which matters for per-request
servers that re-register all tools on every request.
- Rename deprecated generic NewServerTool[In, Out] -> NewServerToolWithDeps
to free up the simpler name and make its closure-based nature explicit.
The dynamic tools package is the only legitimate user of this constructor
because DynamicToolDependencies differs from the standard ToolDependencies.
- Remove deprecated NewServerToolFromHandler. Its only callers can use the
new NewServerTool directly via context-injected deps.
- Update all call sites in dependencies.go, dynamic_tools.go, and
registry_test.go.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: return isError for argument validation failures
When tool argument unmarshalling fails (wrong types, malformed JSON),
return a CallToolResult with IsError: true instead of a Go error.
Returning a Go error is converted by the SDK into a JSON-RPC protocol
error (-32603), which is invisible to agents and prevents self-correction.
Returning IsError: true with the validation message lets agents see the
problem and retry with corrected arguments.
Affects:
- NewServerToolWithDeps (was NewServerTool prior to the rename in #2510)
- NewServerToolWithContextHandler
Fixes#1952.
Re-applies #2488 by @blackwell-systems on top of the NewServerTool rename.
Co-authored-by: blackwell-systems <236632453+blackwell-systems@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: blackwell-systems <236632453+blackwell-systems@users.noreply.github.com>
- Rename NewServerToolWithRawContextHandler -> NewServerTool. This is the
preferred constructor for raw mcp.ToolHandler tools because it avoids
creating closures at registration time, which matters for per-request
servers that re-register all tools on every request.
- Rename deprecated generic NewServerTool[In, Out] -> NewServerToolWithDeps
to free up the simpler name and make its closure-based nature explicit.
The dynamic tools package is the only legitimate user of this constructor
because DynamicToolDependencies differs from the standard ToolDependencies.
- Remove deprecated NewServerToolFromHandler. Its only callers can use the
new NewServerTool directly via context-injected deps.
- Update all call sites in dependencies.go, dynamic_tools.go, and
registry_test.go.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `pull_request_read` tool description tells clients that
`get_review_comments` uses cursor-based pagination (`perPage`, `after`),
and the handler does plumb `after` through to the GraphQL query, but the
input schema only declared `page` and `perPage` (via `WithPagination`).
Because `after` was not advertised in `inputSchema`, MCP clients had
no way to request it, leaving cursor pagination effectively broken:
`perPage: 1` returned only the first thread with no way to advance, and
`page` was silently ignored by the GraphQL path.
This change adds `after` to the schema (string, optional) with a
description making clear it only applies to `get_review_comments`. All
other methods continue to ignore it. No handler behavior is changed.
- Add `after` schema property after `WithPagination` in `PullRequestRead`
- Regenerate `__toolsnaps__/pull_request_read.snap` and update README
- Add a regression test asserting `after` is in the schema and a new
table-driven case verifying the cursor is forwarded to the GraphQL query
Fixes#2122 (for the `get_review_comments` pagination part). The
remaining concerns in #2122 about unbounded response sizes for `get`,
`get_diff`, and `get_reviews` are deferred to follow-up design.
Co-authored-by: Sam Morrow <info@sam-morrow.com>
Sam Morrow