The cobra root command is declared as "server", but the binary is built and
distributed as "github-mcp-server". Two user-visible consequences:
* "github-mcp-server completion <shell>" emits a completion dispatcher
keyed on "server" (_server, __start_server, ...). Installed as
completions for github-mcp-server they never fire, because the function
names and the final "complete ... server" registration refer to a
command that is not on the user's PATH.
* "--help" prints "Usage: server [command]", naming a command that does
not exist.
Name the root command after the binary. cmd/mcpcurl already does this
correctly (Use: "mcpcurl"). Nothing else in the tree refers to the root
command by name, and no test asserts it.
When deploying the MCP server behind an OAuth proxy (e.g. for GHES,
which does not natively support RFC 8414, RFC 7591, or PKCE), the
/.well-known/oauth-protected-resource endpoint currently always derives
the authorization_servers URL from GITHUB_HOST. There is no way to
point clients at a different authorization server without intercepting
that endpoint at the ingress/proxy layer.
The oauth.Config struct already has an AuthorizationServer field with
the conditional logic in place (pkg/http/oauth/oauth.go), but it was
never wired to any configuration surface.
This commit exposes it as:
- --authorization-server CLI flag on the http subcommand
- GITHUB_AUTHORIZATION_SERVER environment variable (via viper's
existing GITHUB_ prefix + automatic env mapping)
When set, the value is passed through ServerConfig into oauth.Config,
and the protected resource metadata advertises it directly instead of
calling apiHost.AuthorizationServerURL().
* feat(repos): add confirmed repository deletion
Add a destructive delete_repository tool that requires an exact owner/repo confirmation through multi-round-trip elicitation. Gate the tool to MCP protocol 2026-07-28 and newer across local and remote transports.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* refactor(inventory): generalize tool availability guards
Gate protocol-restricted tools on required elicitation capabilities and enforce direct calls inside the registered handler so SDK result finalization remains intact.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* feat(http): protect MRTR request state
Seal repository deletion targets for self-hosted HTTP with a stable AES-256-GCM key. Hide only delete_repository when no key is configured and expose an optional sealer interface for remote integrators.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(repos): expire deletion confirmations
Bind sealed repository deletion state to the immutable repository ID and a ten-minute expiry. Re-check identity before deletion so replay cannot affect a recreated repository.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(http): preserve tool and scope restrictions
Apply static allowlists before removing unavailable tools and fail closed on invalid configured tool names. Model independent OAuth requirements as conjunctive groups so repository deletion requires both delete_repo and repo.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(repos): require protected confirmation state
Give stdio a process-local request-state sealer and make deletion fail closed without one. Preserve legacy any-of OAuth behavior globally while documenting and enforcing delete_repository's conjunctive delete_repo and repo requirements.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(oauth): request repository deletion scope
Include delete_repo in the supported OAuth scope set used by stdio login, HTTP protected-resource metadata, and tool filtering.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(oauth): require deletion scope opt-in
Keep delete_repo in protected-resource discovery for step-up authorization while excluding it from the default stdio OAuth grant.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* refactor(oauth): derive scope sets from catalog
Generate protected-resource supported scopes and the lower-risk default OAuth grant from one canonical scope definition list.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* refactor(scopes): own OAuth scope catalog
Move supported and default OAuth scope policy into pkg/scopes so protected-resource metadata and stdio grants derive from the scope domain package.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
* fix(scopes): require workflow scope opt-in
Keep workflow and codespace in protected-resource discovery while excluding both from the default OAuth grant alongside delete_repo.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
---------
Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
Keep PEM loading and installation-token provider construction at the CLI leaf, then pass a generic refreshing token provider through the existing HTTP transports. Rebase the feature onto current main and keep the HTTP command unchanged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 646357dd-c89f-4973-9a5c-e6c5fc18818c
Add non-interactive GitHub App installation authentication to the stdio
server, so headless deployments (CI, Kubernetes, background agents) can
authenticate without a browser, device code, or elicitation. This is the
outstanding follow-up tracked in #1333: OAuth login shipped the interactive
user-to-server flows, but PEM-based server-to-server auth was still needed to
remove the interactive requirement.
The new internal/githubapp package signs a short-lived RS256 JWT with the
app's private key, exchanges it for an installation access token, and refreshes
it transparently before expiry. It exposes a Provider whose AccessToken method
mirrors oauth.Manager so it plugs into the existing BearerAuthTransport token
provider. Only the standard library and golang.org/x/oauth2 are used.
The private key is injected safely: a file path (GITHUB_APP_PRIVATE_KEY_PATH,
preferred — mountable as a secret and kept off argv and out of the environment)
or an inline GITHUB_APP_PRIVATE_KEY env var. There is intentionally no flag for
the key contents, which would otherwise leak via the process command line.
App auth is mutually exclusive with a PAT and with OAuth login. A loud startup
warning and a dedicated docs page (docs/github-app-auth.md, with Docker and
Kubernetes examples) cover the security considerations: this injects a
high-privilege credential alongside the agent and is not recommended without an
independent security review.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(ui): render success view when an MCP App tool executed up-front
The create_pull_request / issue_write / update_pull_request Views decided
form-vs-success from in-app submit state only, ignoring the tool-result the
host delivers on render. Per the MCP Apps 2026-01-26 spec the host renders a
View whenever the tool carries _meta.ui.resourceUri — independent of whether
the server deferred or executed. So when the server executed up-front (e.g.
show_ui=false, or parameters the form can't represent) the View still showed
its "Create pull request" input form over an already-created PR, which reads
as a bug (it even shows a PR number).
Drive the Views off the result instead: a new shared completedToolResult()
helper returns parsed data only for a genuine completed success, and returns
null for the awaiting_user_submission deferral sentinel, errors, or no result.
Each write View now shows its success card when that completed result is
present, so the form is only ever shown while the action is genuinely deferred.
Reconciles the show/defer state machine at the View (decision layer that the
host result feeds). See github/copilot-mcp-core#1864.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(ui): scope tool-result to the current invocation
Address review feedback: the write Views derive their success card from
`toolResult`, but it wasn't cleared when a new invocation arrived (only the
in-app `successPR`/`successIssue` was reset on `toolInput` change). A completed
result from a previous invocation could briefly render a stale success card
over the next, still-deferred form.
Clear `toolResult` whenever a new `tool-input` notification arrives. The spec
guarantees `tool-input` precedes that invocation's `tool-result`, so this scopes
the result to the current invocation centrally in the hook — fixing all three
Views without per-app invocation keys.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(mcp-apps): remove show_ui — it can't suppress app rendering
show_ui promised "skip the form and execute directly", but it can't deliver:
the host renders an MCP App for any tool that carries _meta.ui.resourceUri, and
the 2026-01-26 MCP Apps spec has no per-call/per-result way to opt out of
rendering. show_ui only flipped the server's defer decision, so show_ui=false
created the PR/issue up-front yet the host still rendered the app — exactly the
contradiction this work set out to fix. And show_ui is only ever exposed to
clients that support UI, i.e. precisely the clients that always render the app.
Remove it entirely:
- Drop the show_ui schema property, the form-param allowlist entry, and the
showUI term from the defer predicate in create_pull_request and issue_write.
The gate is now FF && clientSupportsUI && !_ui_submitted && !hasNonFormParams.
- Delete the now-unused UI-only schema-property strip machinery in
pkg/inventory (uiOnlySchemaProperties, stripUIOnlySchemaProperties,
stripSchemaProperties) and the exported ConditionalSchemaPropertyDescriptions,
which existed solely to surface show_ui to UI-capable clients. _meta.ui
stripping is untouched.
- Drop the conditional-property annotation from the docs generator.
- Update toolsnaps, generated docs, and tests.
With the up-front-execution Views now rendering the result (success card), the
remaining contract is simple: when MCP Apps are enabled the form is the path,
and the form is only shown while the action is genuinely deferred.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(mcp-apps): centralize the show/defer decision (single source of truth)
The defer-to-form predicate was triplicated across create_pull_request,
update_pull_request, and issue_write, each with its own near-identical
*HasNonFormParams function. As more MCP App tools are added this duplication
would grow and the copies could silently drift.
Extract one shared gate in ui_capability.go:
- shouldDeferToForm(ctx, deps, req, args, formParams) — the single show/defer
decision (MCP Apps enabled, client supports UI, not a form submission, and no
non-form params).
- hasNonFormParams(args, formParams) — one generic helper replacing the three
per-tool functions.
- uiSubmitted(args) — small shared predicate.
Each handler is now a one-line `if shouldDeferToForm(...) { return awaiting }`.
The per-tool form-parameter allowlists and the user-facing messages stay
per-tool (that is the genuine per-tool config). Pure refactor — behavior
unchanged; existing tests now exercise the generic helper.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(oauth): add stdio OAuth 2.1 stdio login
Introduce internal/oauth, a self-contained library that performs the
user-facing GitHub OAuth login the stdio server uses to obtain a token
without a pre-provisioned PAT. It is independent of MCP: client concerns
(elicitation) sit behind the Prompter interface so the flows are testable
without a live session.
What it provides:
- Authorization-code + PKCE flow with a local loopback callback server,
state/CSRF validation, and XSS-safe result pages.
- Device-authorization flow as a fallback (headless, containers).
- A Manager that selects the most secure available channel
(browser auto-open -> URL elicitation -> last-resort user action),
runs a single flow at a time, and exposes a refreshing token source.
Both GitHub OAuth Apps and GitHub Apps are supported without special
casing: the token is modeled as an x/oauth2 refreshing TokenSource, so
expiring GitHub App user tokens are renewed transparently (the gap that
made a stored-token approach silently die after ~8h).
When a client lacks secure URL elicitation and the flow falls back to a
tool-response message, the message advises the user that their agent/CLI/
IDE does not appear to support URL elicitation and suggests requesting it
for improved security.
Tests exercise real protocol behavior against an httptest GitHub stand-in:
PKCE challenge/verifier, GitHub App refresh-on-expiry, device polling,
URL elicitation, declined prompts, the last-resort action with advisory,
and single-flight concurrency.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(oauth): reap browser launcher and keep native callback on loopback
Address code review:
- openBrowser: reap the launcher process asynchronously so it does not
linger as a zombie for the lifetime of the server.
- listenCallback: take an explicit bindAll flag and bind to all interfaces
only inside a container (where the published port arrives via eth0).
A native run, even with a fixed callback port, now stays on 127.0.0.1
instead of 0.0.0.0.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(oauth): fail fast when a fixed callback port is unavailable
A fixed --oauth-callback-port is registered with the OAuth app and chosen
deliberately, so a bind failure means another process holds the port and
could intercept the authorization redirect. Treat that as fatal instead of
silently downgrading to the device flow, which would mask the conflict.
Also warn, when binding the callback inside a container, that the listener
is on all interfaces and should be published to loopback only so the
authorization code is not exposed on the container network.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(oauth): surface refresh failures, bound refresh, prefer device flow when headless
Addresses pre-merge review of the OAuth stdio core:
- Log a one-time warning when token refresh fails instead of silently
returning an empty access token, so a forced re-login isn't a surprise.
- Bound each background token refresh with a 30s HTTP client timeout so a
stalled GitHub token endpoint can't block tool calls indefinitely.
- On a headless host (no display server) with a random callback port, fall
back to the device-code flow — the only channel reachable from a browser
on another machine — instead of dead-ending on an unreachable localhost
redirect. A generic browser-open failure still offers the manual URL.
- Mark the callback bind failure with a sentinel so the fixed-port-busy
fatal path can't misreport an unrelated error as a port conflict.
- Export NormalizeHost so callers can recognize the default github.com host
(consumed by the build-time baked-in credential guard).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(oauth): wire stdio OAuth 2.1 login into the server (2/4) (#2710)
* feat(oauth): wire stdio OAuth 2.1 login into the server
Connect the internal/oauth core library to the stdio MCP server so users
can authenticate with an OAuth App or GitHub App client ID instead of a
static personal access token.
- BearerAuthTransport gains a TokenProvider that is consulted per request,
letting the lazily-acquired, auto-refreshing OAuth token take effect
without rebuilding the client.
- createGitHubClients uses BearerAuthTransport (and skips go-github's
WithAuthToken, which would pin a static token) when a TokenProvider is set.
- RunStdioServer starts without a token and installs receiving middleware
that runs the authorization flow on the first tool call, surfacing the
auth URL or device code via elicitation (or a tool result as a fallback).
- Tool filtering uses the requested OAuth scopes; the default supported set
hides nothing, while a narrower --oauth-scopes both narrows the grant and
filters tools accordingly.
- A sessionPrompter adapts the MCP server session to oauth.Prompter, keeping
the authorization URL off the model's context.
- New stdio flags: --oauth-client-id/-client-secret/-scopes/-callback-port.
This is stdio-only and deliberately does not touch MCP-HTTP auth.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(oauth): address review — omit empty bearer header, guard token/oauth
- BearerAuthTransport omits the Authorization header entirely when the token
is empty (pre-authorization) rather than sending an empty "Bearer " value.
- RunStdioServer rejects the ambiguous combination of a static Token and an
OAuthManager up front, enforcing the documented mutual exclusivity.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs(oauth): clarify SupportedScopes is the stdio default and tool filter
Document that stdio OAuth login requests these scopes by default and then
filters the exposed tools to the scopes actually granted, so a tool whose
required scope is absent from this list is hidden under default OAuth even
though a PAT carrying that scope would expose it. Keep the list in sync with
tool scope requirements when scopes change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Distinguish undeliverable auth prompts from user declines
An elicitation prompt that the client cannot deliver (a transport or
protocol failure) was treated the same as a user actively declining: any
display error cancelled the flow. That conflated a system failure with a
deliberate "no", so a client that advertised URL elicitation but failed
to deliver it would hard-fail the login instead of degrading.
Add an ErrPromptUnavailable sentinel alongside ErrPromptDeclined and have
the MCP adapter return it when Elicit fails at the transport level. The
manager now falls back to the manual user-action channel on an
undeliverable prompt (keeping the background flow alive so the user can
still authorize out of band), while a genuine decline still aborts. A
context-cancelled prompt is checked first so an ending flow is never
misread as a transport failure.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* build(oauth): bake in default OAuth credentials for official releases (3/4) (#2711)
* build(oauth): bake in default OAuth credentials via build-time ldflags
Inject the public OAuth client credentials (stored as the OAUTH_CLIENT_ID
and OAUTH_CLIENT_SECRET repo secrets) at build time via -ldflags so
official binaries and images ship a working default app for zero-config
login. Security relies on PKCE, not on the secret. Local/dev builds leave
the values empty and continue to require an explicit token or
--oauth-client-id.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(oauth): recognize github.com host aliases for the baked-in client
Match the default host via oauth.NormalizeHost instead of only an empty
host string, so an explicit GITHUB_HOST=github.com (or api.github.com)
still counts as the default and keeps zero-config baked-in login working.
GHES and ghe.com users continue to bring their own --oauth-client-id.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs(oauth): document stdio OAuth login; make PAT optional in install config (#2717)
Add a dedicated Local Server OAuth Login guide (docs/oauth-login.md) covering
the PKCE/device flows, display channels and the URL-elicitation security
advisory, scope-based tool filtering, the fixed-port Docker recipe and its
loopback/port-safety behavior, bringing your own OAuth or GitHub App, and the
GitHub Enterprise Server / ghe.com requirement to register an app on that host
(custom --gh-host directs login at that instance's authorization server).
Reflect that the local server now logs in with OAuth by default on github.com:
- README: make the stdio Docker install badges OAuth-first (fixed callback port
8085 published to loopback), drop the PAT prompt, and reframe the PAT as an
optional alternative with a pointer to the new guide.
- server.json: make GITHUB_PERSONAL_ACCESS_TOKEN optional and publish the OAuth
callback port so the registry default works without a token.
Co-authored-by: Copilot <223556219+Copilot@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: Copilot <223556219+Copilot@users.noreply.github.com>
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
MCP Server Diff / mcp-diff-http (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
* Add repo-scoped support to list_issue_types tool
* Render multi-scope tools as "any of" in generated docs
* Clarify issue type field description for repo-scoped list_issue_types
* Add explicit show_ui parameter to UI-enabled write tools
Today the server decides whether to route issue_write and create_pull_request
through the MCP App form using two implicit signals: _ui_submitted (set by
the form on submit) and a heuristic that bypasses the form when the call
carries any parameter the form cannot represent (labels, assignees,
issue_fields, state, reviewers, etc.). The model had no first-class,
documented way to say "execute directly, do not show a form".
Add a show_ui boolean parameter to the input schema of IssueWrite,
LegacyIssueWrite, and CreatePullRequest. It defaults to true and is
visible only to clients that advertise MCP App UI support: the strip
happens per-request in inventory.ToolsForRegistration via a new
stripUIOnlySchemaProperties helper, gated by the same predicate that
already strips _meta.ui (shouldStripMCPAppsMetadata). The two strips share
one decision so the schema and metadata stay in lock-step.
Form-routing predicate becomes:
MCPApps FF on && client supports UI &&
!_ui_submitted && show_ui && !hasNonFormParams
show_ui=false is a new explicit way for the model to opt out. The existing
non-form-param auto-bypass stays as a safety net, and the React forms keep
sending _ui_submitted=true on submit unchanged. get_me is out of scope
because its UI is pure client-side card rendering with no server-side
gating to replace.
The current strip gate ("strip when FF is off OR capability explicitly
absent") mirrors today's _meta.ui behavior exactly, including the
"capability unknown" case. For stdio that means UI-capable schemas are
exposed to any FF-enabled client. The handler-side clientSupportsUI check
still gates form execution at call time, so it is functionally a no-op for
non-UI stdio clients. A separate follow-up will tighten the gate to
"strip on unknown too" and wire an InitializedHandler in stdio to
re-register the un-stripped surface only after a UI-capable client has
advertised; the two changes must ship together to avoid breaking stdio.
docs/feature-flags.md and docs/insiders-features.md include an unrelated
"reviewers" description update picked up by script/generate-docs from
commit 2bd162ac ("fix: support team pull request reviewers"), which
updated the source schema but did not regenerate docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Clarify where show_ui appears in generated docs
The code comments next to the show_ui schema entries (and the
uiOnlySchemaProperties allowlist) said the property is documented in
"toolsnaps / README". README is generated from the stripped (non-UI)
schema, so show_ui is not actually in it — it only appears in toolsnaps
and the feature-flag / insiders docs. Reword the comments to match
reality.
Comment-only change; no behavior or test impact.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Guard issue_write/create_pull_request schemas against UI-gating desync
The form-routing logic depends on a hand-maintained classification of
each schema property into form-resendable vs known-non-form. A new
property added without updating the classification would silently shift
UI gating behavior (e.g. a form-incompatible param wouldn't trigger the
safety-net bypass).
Add Test_issueWriteSchemaClassification and Test_createPullRequestSchemaClassification
that enumerate each tool's InputSchema.Properties and require every
property to be classified as exactly one of:
- form-resendable (member of issueWriteFormParams / pullRequestWriteFormParams)
- known-non-form (test-local allowlist)
A future schema addition without classification fails the test with a
message pointing at the exact set the contributor needs to update.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Mark conditional schema parameters in generated docs
Previously `show_ui` was listed in docs/feature-flags.md and
docs/insiders-features.md alongside ordinary parameters with no
indication that it is hidden from clients without MCP App UI support.
A reader scanning the parameter list would assume it is always available.
Add a programmatic conditional-property mechanism:
- `inventory.ConditionalSchemaPropertyDescriptions()` exposes a
map[propertyName]conditionDescription derived from the same
uiOnlySchemaProperties allowlist that drives the per-request strip
in ToolsForRegistration. Single source of truth.
- The doc generator (writeToolDoc) consults this map and appends
"conditional — <description>" to the parameter's parenthesised
type/required suffix.
Example rendered output:
- `show_ui`: Whether to render the MCP App form... (boolean, optional,
conditional — only visible to clients that advertise MCP App UI support)
A small test (TestConditionalSchemaPropertyDescriptions) ensures every
entry in uiOnlySchemaProperties has a description, so a future stripped
property addition can't silently lose its doc marker.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
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 <sammorrowdrums@github.com>
Per @SamMorrowDrums review — replace the manual cleanup() calls before
each error return with a single defer right after cmd.Start(). Same
behaviour, less code.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mcpcurl was sending tools/list and tools/call requests without first
performing the MCP initialize handshake, causing the server to silently
reject all requests and discover zero tools.
Before:
$ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help
(no tools listed)
After:
$ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help
Available Commands:
add_comment_to_pending_review ...
add_issue_comment ...
create_branch ...
The default toolset row covers /mcp/ but /x/all is still a real, useful
meta toolset that enables every toolset at once. Render both as special
rows above the per-toolset list.
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>
* 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>
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>
The HTTP server now respects the same static CLI flags as the stdio
server: --toolsets, --tools, --exclude-tools, --read-only,
--dynamic-toolsets, and --insiders.
A static inventory is built once at startup from these flags, producing
a pre-filtered tool/resource/prompt universe. Per-request headers
(X-MCP-Toolsets, X-MCP-Tools, etc.) can only narrow within these
bounds, never expand beyond them. When no static flags are set, the
existing behavior is preserved — headers have full access to all
toolsets.
Fixes#2156
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of new CLI flags (--server-name, --server-title), reuse the
existing string override mechanism that already supports tool title/
description overrides throughout the codebase.
Users can now configure the server name and title via:
- GITHUB_MCP_SERVER_NAME / GITHUB_MCP_SERVER_TITLE env vars
- "SERVER_NAME" / "SERVER_TITLE" keys in github-mcp-server-config.json
This is consistent with how all other user-visible strings are
overridden (e.g. GITHUB_MCP_TOOL_GET_ME_USER_TITLE). No new struct
fields or CLI flags are needed.
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Allows users running multiple GitHub MCP Server instances (e.g., for
github.com and GitHub Enterprise Server) to override the server name and
title in the MCP initialization response.
- Add --server-name / GITHUB_SERVER_NAME flag+env to override name
- Add --server-title / GITHUB_SERVER_TITLE flag+env to override title
- Defaults remain "github-mcp-server" and "GitHub MCP Server"
- Applies to both stdio and HTTP server modes
- Add tests for default and custom name/title
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
* wip injecting ff function into tool as dep
* remove debug
* fix linter
* add better test
* adding compile time check
* move experimental to seperate config/ff value
* adding test var
* fixing test
* adding flag and possibility to call feature checker
* fixing name
* get it working
* clean up approach by moving cleantools inside builder, this simplifies remote server too
* add tests for trimming and deduplication
* error out in the builder if there are unrecognized tools
---------
Co-authored-by: Sam Morrow <info@sam-morrow.com>
The generate-docs command now outputs toolset IDs (e.g., `actions`, `code_security`)
wrapped in backticks instead of display names (e.g., "Actions", "Code Security").
This ensures the manual changes from PR #1756 persist when the docs are regenerated,
fixing the issue where users need to configure the actual toolset ID, not the display name.
Changes:
- Modified generateRemoteToolsetsDoc() to use `idStr` instead of `formattedName()`
- Modified generateRemoteOnlyToolsetsDoc() to use `idStr` instead of `formattedName()`
- Both functions now wrap the toolset ID in backticks for clarity
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Scope filtering is now a built-in feature rather than a configurable option.
The server automatically fetches token scopes at startup and filters tools
accordingly. If scope detection fails, it logs a warning and continues with
all tools available.
Add the ability to filter tools based on token scopes for PAT users.
This uses an HTTP HEAD request to GitHub's API to discover token scopes.
New components:
- pkg/scopes/filter.go: HasRequiredScopes checks if scopes satisfy tool requirements
- pkg/scopes/fetcher.go: FetchTokenScopes gets scopes via HTTP HEAD to GitHub API
- pkg/github/scope_filter.go: CreateScopeFilter creates inventory.ToolFilter
Integration:
- Add --filter-by-scope flag to stdio command (disabled by default)
- When enabled, fetches token scopes on startup
- Tools requiring unavailable scopes are hidden from tool list
- Gracefully continues without filtering if scope fetch fails (logs warning)
This allows the OSS server to have similar scope-based tool visibility
as the remote server, and the filter logic can be reused by remote server.
- Replace NewToolWithScopes with updated NewTool that includes scope parameters
- All tools now use the same constructor with explicit scope requirements
- Documentation only shows accepted scopes when they differ from required
- Removed NewToolWithScopes and NewToolFromHandlerWithScopes helper functions
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
- Updated all remaining tools with OAuth scope information
- Added scope documentation generation to generate-docs command
- Documentation now shows Required and Accepted OAuth scopes for each tool
- All 100+ tools now have scope information defined
- Tests pass, linter passes, documentation generated successfully
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>