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>
* 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>
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>
* 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
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.
* refactor: separate ServerTool into own file with HandlerFunc pattern
- Extract ServerTool struct into pkg/toolsets/server_tool.go
- Add ToolDependencies struct for passing common dependencies to handlers
- HandlerFunc allows lazy handler generation from Tool definitions
- NewServerTool for new dependency-based tools
- NewServerToolLegacy for backward compatibility with existing handlers
- Update toolsets.go to store and pass dependencies
- Update all call sites to use NewServerToolLegacy
Co-authored-by: Adam Holt <4619+omgitsads@users.noreply.github.com>
* Wire ToolDependencies through toolsets
- Move ToolDependencies to pkg/github/dependencies.go with proper types
- Use 'any' in toolsets package to avoid circular dependencies
- Add NewTool/NewToolFromHandler helpers that isolate type assertion
- Tool implementations will be fully typed with no assertions scattered
- Infrastructure ready for incremental tool migration
* refactor(search): migrate search tools to new ServerTool pattern
Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers,
SearchOrgs) to use the new NewTool helper and ToolDependencies pattern.
- Functions now take only TranslationHelperFunc and return ServerTool
- Handler generation uses ToolDependencies for typed access to clients
- Update tools.go call sites to remove getClient parameter
- Update tests to use new Handler(deps) pattern
This demonstrates the migration pattern for additional tool files.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* Migrate context_tools to new ServerTool pattern (#1590)
* refactor(search): migrate search tools to new ServerTool pattern
Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers,
SearchOrgs) to use the new NewTool helper and ToolDependencies pattern.
- Functions now take only TranslationHelperFunc and return ServerTool
- Handler generation uses ToolDependencies for typed access to clients
- Update tools.go call sites to remove getClient parameter
- Update tests to use new Handler(deps) pattern
This demonstrates the migration pattern for additional tool files.
Co-authored-by: Adam Holt <oholt@github.com>
* Migrate context_tools to new ServerTool pattern
Convert GetMe, GetTeams, and GetTeamMembers to use the new typed
dependency injection pattern:
- Functions now take only translations helper, return toolsets.ServerTool
- Handler is generated lazily via deps.GetClient/deps.GetGQLClient
- Tests updated to use serverTool.Handler(deps) pattern
- Fixed error return pattern to return nil for Go error (via result.IsError)
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(gists): migrate gists.go to NewTool pattern (#1591)
* Migrate context_tools to new ServerTool pattern
Convert GetMe, GetTeams, and GetTeamMembers to use the new typed
dependency injection pattern:
- Functions now take only translations helper, return toolsets.ServerTool
- Handler is generated lazily via deps.GetClient/deps.GetGQLClient
- Tests updated to use serverTool.Handler(deps) pattern
- Fixed error return pattern to return nil for Go error (via result.IsError)
Co-authored-by: Adam Holt <oholt@github.com>
* refactor(gists): migrate gists.go to NewTool pattern
Convert all gist tools (ListGists, GetGist, CreateGist, UpdateGist)
to use the new NewTool helper with ToolDependencies injection.
- Remove getClient parameter from function signatures
- Use deps.GetClient(ctx) inside handlers
- Standardize error handling with utils.NewToolResultErrorFromErr()
- Update all tests to use serverTool.Handler(deps) pattern
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
---------
Co-authored-by: Adam Holt <oholt@github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
---------
Co-authored-by: Adam Holt <oholt@github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(notifications): migrate notifications.go to NewTool pattern (#1592)
* refactor(notifications): migrate notifications.go to NewTool pattern
Convert all notification tools to use the new NewTool helper with
ToolDependencies injection.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* Refactor repositories.go tools to use NewTool pattern with ToolDependencies
Convert all 18 tool functions in repositories.go to use the new NewTool helper
pattern with typed ToolDependencies, isolating type assertions to a single
location and improving code maintainability.
Functions converted:
- GetCommit, ListCommits, ListBranches
- CreateOrUpdateFile, CreateRepository, GetFileContents
- ForkRepository, DeleteFile, CreateBranch, PushFiles
- ListTags, GetTag, ListReleases, GetLatestRelease, GetReleaseByTag
- ListStarredRepositories, StarRepository, UnstarRepository
This is part of a stacked PR series to systematically migrate all tool
files to the new pattern.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(issues): migrate issues.go to NewTool pattern
Convert all 8 tool functions in issues.go to use the new NewTool
helper pattern which standardizes dependency injection:
- IssueRead: GetClient, GetGQLClient, RepoAccessCache, Flags
- ListIssueTypes: GetClient
- AddIssueComment: GetClient
- SubIssueWrite: GetClient
- SearchIssues: GetClient
- IssueWrite: GetClient, GetGQLClient
- ListIssues: GetGQLClient
- AssignCopilotToIssue: GetGQLClient
Updated tools.go to use direct function calls instead of
NewServerToolLegacy wrappers. Updated all tests in issues_test.go
to use the new ToolDependencies pattern and Handler() method.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(pullrequests): convert PR tools to NewTool pattern
Convert all 10 pull request tool functions to use the NewTool
pattern with ToolDependencies injection:
- PullRequestRead
- CreatePullRequest
- UpdatePullRequest
- ListPullRequests
- MergePullRequest
- SearchPullRequests
- UpdatePullRequestBranch
- PullRequestReviewWrite
- AddCommentToPendingReview
- RequestCopilotReview
Update tools.go to use direct function calls (removing
NewServerToolLegacy wrappers) for PR functions.
Update all tests in pullrequests_test.go to use the new
handler pattern with deps and 2-value return.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* Refactor actions.go to use NewTool pattern
Convert all 14 tool functions in actions.go to use the NewTool pattern with
ToolDependencies for dependency injection. This is part of a broader effort
to standardize the tool implementation pattern across the codebase.
Changes:
- ListWorkflows, ListWorkflowRuns, RunWorkflow, GetWorkflowRun
- GetWorkflowRunLogs, ListWorkflowJobs, GetJobLogs
- RerunWorkflowRun, RerunFailedJobs, CancelWorkflowRun
- ListWorkflowRunArtifacts, DownloadWorkflowRunArtifact
- DeleteWorkflowRunLogs, GetWorkflowRunUsage
The new pattern:
- Takes only translations.TranslationHelperFunc as parameter
- Returns toolsets.ServerTool with Tool and Handler
- Handler receives ToolDependencies for client access
- Enables better testability and consistent interface
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(git): migrate GetRepositoryTree to NewTool pattern
* refactor(security): migrate code_scanning, secret_scanning, dependabot to NewTool pattern
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor(discussions): migrate to NewTool pattern
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* Refactor security_advisories tools to use NewTool pattern
Convert 4 functions from NewServerToolLegacy wrapper to NewTool:
- ListGlobalSecurityAdvisories
- GetGlobalSecurityAdvisory
- ListRepositorySecurityAdvisories
- ListOrgRepositorySecurityAdvisories
Update tools.go toolset registration and tests.
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* refactor: convert projects, labels, and dynamic_tools to NewTool pattern
This PR converts projects.go, labels.go, and dynamic_tools.go from the
legacy NewServerToolLegacy wrapper pattern to the new NewTool pattern with
proper ToolDependencies.
Changes:
- projects.go: Convert all 9 project functions to use NewTool with
ToolHandlerFor[map[string]any, any] and 3-return-value handlers
- projects_test.go: Update tests to use new serverTool.Handler(deps) pattern
- labels.go: Convert GetLabel, ListLabels, and LabelWrite to NewTool pattern
- labels_test.go: Update tests to use new pattern
- dynamic_tools.go: Refactor functions to return ServerTool directly
(using NewServerToolLegacy internally since they have special dependencies)
- tools.go: Remove NewServerToolLegacy wrappers for dynamic tools registration
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
* Add --features CLI flag for feature flag support
Add CLI flag and config support for feature flags in the local server:
- Add --features flag to main.go (StringSlice, comma-separated)
- Add EnabledFeatures field to StdioServerConfig and MCPServerConfig
- Create createFeatureChecker() that builds a set from enabled features
- Wire WithFeatureChecker() into the toolset group filter chain
This enables tools/resources/prompts that have FeatureFlagEnable set to
a flag name that is passed via --features. The checker uses a simple
set membership test for O(1) lookup.
Usage:
github-mcp-server stdio --features=my_feature,another_feature
GITHUB_FEATURES=my_feature github-mcp-server stdio
* Add validation tests for tools, resources, and prompts metadata
This commit adds comprehensive validation tests to ensure all MCP items
have required metadata:
- TestAllToolsHaveRequiredMetadata: Validates Toolset.ID and Annotations
- TestAllToolsHaveValidToolsetID: Ensures toolsets are in AvailableToolsets()
- TestAllResourcesHaveRequiredMetadata: Validates resource metadata
- TestAllPromptsHaveRequiredMetadata: Validates prompt metadata
- TestToolReadOnlyHintConsistency: Validates IsReadOnly() matches annotation
- TestNoDuplicate*Names: Ensures unique names across tools/resources/prompts
- TestAllToolsHaveHandlerFunc: Ensures all tools have handlers
- TestDefaultToolsetsAreValid: Validates default toolset IDs
- TestToolsetMetadataConsistency: Ensures consistent descriptions per toolset
Also fixes a bug discovered by these tests: ToolsetMetadataGit was defined
but not added to AvailableToolsets(), causing get_repository_tree to have
an invalid toolset ID.
* Fix default toolsets behavior when not in dynamic mode
When no toolsets are specified and dynamic mode is disabled, the server
should use the default toolsets. The bug was introduced when adding
dynamic toolsets support:
1. CleanToolsets(nil) was converting nil to empty slice
2. Empty slice passed to WithToolsets means 'no toolsets'
3. This resulted in zero tools being registered
Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets)
and only set empty slice when dynamic mode is enabled without explicit
toolsets.
* refactor: address PR review feedback for toolsets
- Rename AddDeprecatedToolAliases to WithDeprecatedToolAliases for
immutable filter chain consistency (returns new ToolsetGroup)
- Remove unused mockGetRawClient from generate_docs.go (use nil instead)
- Remove legacy ServerTool functions (NewServerToolLegacy and
NewServerToolFromHandlerLegacy) - no usages
- Add panic in Handler()/RegisterFunc() when HandlerFunc is nil
- Add HasHandler() method for checking if tool has a handler
- Add tests for HasHandler and nil handler panic behavior
- Update all tests to use new WithDeprecatedToolAliases pattern
* refactor: Apply HandlerFunc pattern to resources for stateless NewToolsetGroup
This change applies the same HandlerFunc pattern used by tools to resources,
allowing NewToolsetGroup to be fully stateless (only requiring translations).
Key changes:
- Add ResourceHandlerFunc type to toolsets package
- Update ServerResourceTemplate to use HandlerFunc instead of direct Handler
- Add HasHandler() and Handler(deps) methods to ServerResourceTemplate
- Update RegisterResourceTemplates to take deps parameter
- Refactor repository resource definitions to use HandlerFunc pattern
- Make AllResources(t) stateless (only takes translations)
- Make NewToolsetGroup(t) stateless (only takes translations)
- Update generate_docs.go - no longer needs mock clients
- Update tests to use new patterns
This resolves the concern about mixed concerns in doc generation - the
toolset metadata and resource templates can now be created without any
runtime dependencies, while handlers are generated on-demand when deps
are provided during registration.
* refactor: simplify ForMCPRequest switch cases
* refactor(generate_docs): use strings.Builder and AllTools() iteration
- Replace slice joining with strings.Builder for all doc generation
- Iterate AllTools() directly instead of ToolsetIDs()/ToolsForToolset()
- Removes need for special 'dynamic' toolset handling (no tools = no output)
- Context toolset still explicitly handled for custom description
- Consistent pattern across generateToolsetsDoc, generateToolsDoc,
generateRemoteToolsetsDoc, and generateDeprecatedAliasesTable
* feat(toolsets): add AvailableToolsets() with exclude filter
- Add AvailableToolsets() method that returns toolsets with actual tools
- Support variadic exclude parameter for filtering out specific toolsets
- Simplifies doc generation by removing manual skip logic
- Naturally excludes empty toolsets (like 'dynamic') without special cases
* refactor(generate_docs): hoist success logging to generateAllDocs
* refactor: consolidate toolset validation into ToolsetGroup
- Add Default field to ToolsetMetadata and derive defaults from metadata
- Move toolset validation into WithToolsets (trims whitespace, dedupes, tracks unrecognized)
- Add UnrecognizedToolsets() method for warning about typos
- Add DefaultToolsetIDs() method to derive defaults from metadata
- Remove redundant functions: CleanToolsets, GetValidToolsetIDs, AvailableToolsets, GetDefaultToolsetIDs
- Update DynamicTools to take ToolsetGroup for schema enum generation
- Add stubTranslator for cases needing ToolsetGroup without translations
This eliminates hardcoded toolset lists - everything is now derived from
the actual registered tools and their metadata.
* refactor: rename toolsets package to registry with builder pattern
- Rename pkg/toolsets to pkg/registry (better reflects its purpose)
- Split monolithic toolsets.go into focused files:
- registry.go: Core Registry struct and MCP methods
- builder.go: Builder pattern for creating Registry instances
- filters.go: All filtering logic (toolsets, read-only, feature flags)
- resources.go: ServerResourceTemplate type
- prompts.go: ServerPrompt type
- errors.go: Error types
- server_tool.go: ServerTool and ToolsetMetadata (existing)
- Fix lint: Rename RegistryBuilder to Builder (avoid stuttering)
- Update all imports across ~45 files
This refactoring improves code organization and makes the registry's
purpose clearer. The builder pattern provides a clean API:
reg := registry.NewBuilder().
SetTools(tools).
WithReadOnly(true).
WithToolsets([]string{"repos"}).
Build()
* fix: remove unnecessary type arguments in helper_test.go
* fix: restore correct behavior for --tools and --toolsets flags
Two behavioral regressions were fixed in resolveEnabledToolsets():
1. When --tools=X is used without --toolsets, the server should only
register the specified tools, not the default toolsets. Now returns
an empty slice instead of nil when EnabledTools is set.
2. When --toolsets=all --dynamic-toolsets is used, the 'all' and 'default'
pseudo-toolsets should be removed so only the dynamic management tools
are registered. This matches the original pre-refactor behavior.
* Move labels tools to issues toolset
Labels are closely related to issues - you add labels to issues,
search issues by label, etc. Keeping them in a separate toolset
required users to explicitly enable 'labels' to get this functionality.
Moving to issues toolset makes labels available by default since
issues is a default toolset.
* Restore labels toolset with get_label in both issues and labels
This restores conformance with the original behavior where:
- get_label is in issues toolset (read-only label access for issue workflows)
- get_label, list_label, label_write are in labels toolset (full management)
The duplicate get_label registration is intentional - it was in both toolsets
in the original implementation. Added test exception to allow this case.
* Fix instruction generation and capability advertisement
- Expand nil toolsets to default IDs before GenerateInstructions
(nil means 'use defaults' in registry but instructions need actual names)
- Remove unconditional HasTools/HasResources/HasPrompts=true in NewServer
(let SDK determine capabilities based on registered items, matching main)
* Add tests for dynamic toolset management tools
Tests cover:
- list_available_toolsets: verifies toolsets are listed with enabled status
- get_toolset_tools: verifies tools can be retrieved for a toolset
- enable_toolset: verifies toolset can be enabled and marked as enabled
- enable_toolset invalid: verifies proper error for non-existent toolset
- toolsets enum: verifies tools have proper enum values in schema
* Advertise all capabilities in dynamic toolsets mode
In dynamic mode, explicitly set HasTools/HasResources/HasPrompts=true
since toolsets with those capabilities can be enabled at runtime.
This ensures clients know the server supports these features even
when no tools/resources/prompts are initially registered.
* Improve conformance test with dynamic tool calls and JSON normalization
- Add dynamic tool call testing (list_available_toolsets, get_toolset_tools, enable_toolset)
- Parse and sort embedded JSON in text fields for proper comparison
- Separate progress output (stderr) from summary (stdout) for CI
- Add test type field to distinguish standard vs dynamic tests
* Add conformance-report to .gitignore
* Add conformance test CI workflow
- Runs on pull requests to main
- Compares PR branch against merge-base with origin/main
- Outputs full conformance report to GitHub Actions Job Summary
- Uploads detailed report as artifact for deeper investigation
- Does not fail the build on differences (may be intentional)
* Add map indexes for O(1) lookups in Registry
Address review feedback to use maps for collections. Added lookup maps
(toolsByName, resourcesByURI, promptsByName) while keeping slices for
ordered iteration. This provides O(1) lookup for:
- FindToolByName
- filterToolsByName (used by ForMCPRequest)
- filterResourcesByURI
- filterPromptsByName
Maps are built once during Build() and shared in ForMCPRequest copies.
* perf(registry): O(1) HasToolset lookup via pre-computed set
Add toolsetIDSet (map[ToolsetID]bool) to Registry for O(1) HasToolset lookups.
Previously HasToolset iterated through all tools, resourceTemplates, and prompts
to check if any belonged to the given toolset. Now it's a simple map lookup.
The set is populated during the single-pass processToolsets() call, which already
collected all valid toolset IDs. This adds zero new iteration - just returns the
existing validIDs map.
processToolsets now returns 6 values:
- enabledToolsets, unrecognized, toolsetIDs, toolsetIDSet, defaultToolsetIDs, descriptions
* simplify: remove lazy toolsByName map - not needed for actual use cases
FindToolByName() is only called once per request at most (to find toolset ID
for dynamic enablement). The SDK handles tool dispatch after registration.
A simple linear scan over ~90 tools is trivially fast and avoids:
- sync.Once complexity
- Map allocation
- Premature optimization for non-existent 'repeated lookups'
The pre-computed maps we keep (toolsetIDSet, etc.) are justified because
they're used for filtering logic that runs on every request.
* Add generic tool filtering mechanisms to registry package
- Add Enabled field to ServerTool for self-filtering based on context
- Add ToolFilter type and WithFilter method to Builder for cross-cutting filters
- Update isToolEnabled to check Enabled function and builder filters in order:
1. Tool's Enabled function
2. Feature flags (FeatureFlagEnable/FeatureFlagDisable)
3. Read-only filter
4. Builder filters
5. Toolset/additional tools check
- Add FilteredTools method to Registry as alias for AvailableTools
- Add comprehensive tests for all new functionality
- All tests pass and linter is clean
Closes#1618
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
* docs: improve filter evaluation order and FilteredTools documentation
- Add numbered filter evaluation order to isToolEnabled function doc
- Number inline comments for each filter step (1-5)
- Clarify FilteredTools error return is for future extensibility
- Document that library consumers may need to surface recoverable errors
Addresses review feedback on PR #1620
* Refactor GenerateToolsetsHelp() to use strings.Builder pattern
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
---------
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
* Port functional changes from main to registry pattern
Port three functional improvements from main branch:
- GraphQL review comments grouped as threads (#1554)
- get_file_contents description improvement (#1582)
- create_or_update_file SHA validation fix (#1621)
Adapted implementations to use the new registry pattern with:
- BaseDeps for providing clients via ToolDependencies interface
- deps.GetClient(ctx) and deps.GetGQLClient(ctx) patterns
- Updated tests to use GraphQL mocks for review comments
- Added SHA validation test cases for create_or_update_file
* fix(e2e): Fix e2e test compilation and add rate limit handling
- Fix DefaultToolsetIDs() type mismatch by using github.GetDefaultToolsetIDs()
- Add waitForRateLimit() to check and wait for rate limits before each test
- Add skip conditions for Copilot tests when Copilot isn't available
- Use multi-line file content in TestPullRequestReviewCommentSubmit for
multi-line review comments to work correctly
- Improve error messages to include response details
* fix(gists): Use proper GitHub API error handling for observability
The gists.go file was using NewToolResultErrorFromErr for GitHub API
errors, which breaks the error middleware tracking that the remote
server uses for observability and incident detection.
Changed API errors (client.Gists.List, Get, Create, Edit) to use
ghErrors.NewGitHubAPIErrorResponse which properly:
- Records errors in the context for middleware access
- Preserves the response object for rate limit and status tracking
- Maintains consistency with other tools that use this pattern
This ensures production observability is maintained for Gist operations.
* chore: Update server.json schema to 2025-12-11
- Update schema URL to latest version (2025-12-11)
- Remove 'status' field (now managed by registry per 2025-09-29 changelog)
* fix(get_file_contents): Restore correct implementation from #1582
The refactor incorrectly restructured the GetFileContents logic:
- Move 'if rawOpts.SHA != "" { ref = rawOpts.SHA }' before GetContents call
- Always call GetContents first (not conditionally based on path suffix)
- Restore matchFiles helper function for proper fallback handling
- Use matchFiles when Contents API fails or raw API fails
This aligns with the improvements from PR #1582 that was merged into main.
* Rename registry to inventory in comments
Update remaining references to 'registry' in code comments to use
'inventory' consistently after the package rename.
---------
Co-authored-by: Adam Holt <4619+omgitsads@users.noreply.github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
Co-authored-by: Adam Holt <oholt@github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
GoReleaser Release / release (push) Has been cancelled
License Check / license-check (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (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
* add enabledTools to StdioServerConfig
* add EnabledTools to MCPServerConfig, and logic to bypass toolset config if present
* add logic to register specific tools
* update readme
* Update to be consistent with: https://docs.google.com/document/d/1tOOBJ4y9xY61QVrO18ymuVt4SO9nV-z2B4ckaL2f9IU/edit?tab=t.0#heading=h.ffto4e5dwzlf
specifically
- allow for --tools and dynamic toolset mode together
- allow for --tools and --toolsets together
* go mod tidy
* update
* clean up comment
* fix
* fix
* updte
* update
* clean up
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (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
GoReleaser Release / release (push) Has been cancelled
License Check / license-check (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
Fix environment variable mapping for read-only mode configuration
Add SetEnvKeyReplacer to viper configuration to properly map environment
variables with underscores to flag names with dashes. This enables the
documented GITHUB_READ_ONLY=1 environment variable to work correctly.
Without this fix, viper was looking for GITHUB_READ-ONLY (with dash) but
the documentation and standard convention use GITHUB_READ_ONLY (with
underscore).
Fixes issue where read-only mode was not being activated when using
GITHUB_READ_ONLY=1 in Docker containers.
* add sliding window for actions logs
* refactor: fix sliding
* remove trim content
* only use up to 1mb of memory for logs
* update to tail lines in second pass
* add better memory usage calculation
* increase window size to 5MB
* update test
* update vers
* undo vers change
* add incremental memory tracking
* use ring buffer
* remove unused ctx param
* remove manual GC clear
* fix cca feedback
* extract ring buffer logic to new package
* handle log content processing errors and use correct param for maxjobloglines
* fix tailing
* account for if tailLines exceeds window size
* add profiling thats reusable
* remove profiler testing
* refactor profiler: introduce safeMemoryDelta for accurate memory delta calculations
* linter fixes
* Update pkg/buffer/buffer.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* use flag for maxJobLogLines
* add param passing for context window size
* refactor: rename contextWindowSize to contentWindowSize for consistency
* fix: use tailLines if bigger but only if <= 5000
* fix: limit tailLines to a maximum of 500 for log content download
* Update cmd/github-mcp-server/main.go
Co-authored-by: Adam Holt <omgitsads@github.com>
* Update cmd/github-mcp-server/main.go
Co-authored-by: Adam Holt <omgitsads@github.com>
* move profiler to internal/
* update actions test with new profiler location
* fix: adjust buffer size limits
* make line buffer 1028kb
* fix mod path
* change test to use same buffer size as normal use
* improve test for non-sliding window implementation to not count empty lines
* make test memory measurement more accurate
* remove impossible conditional
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Adam Holt <omgitsads@github.com>
This commit cleanly separates config parsing, stdio server execution and
mcp server construction. Aside from significant clarity improvements, it
allows for direct construction of the mcp server in e2e tests to allow
for breakpoint debugging.
Sam Morrow