From 926d04913da02bdbc3fa2c8c906a2b966600acef Mon Sep 17 00:00:00 2001 From: Aakash Shah Date: Thu, 30 Apr 2026 12:28:57 -0700 Subject: [PATCH 1/7] improve dependabot error message (#2375) --- pkg/github/dependabot.go | 17 +++++++++++++++-- pkg/github/dependabot_test.go | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/pkg/github/dependabot.go b/pkg/github/dependabot.go index 6f0da1b208..541cc5c1e7 100644 --- a/pkg/github/dependabot.go +++ b/pkg/github/dependabot.go @@ -69,7 +69,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo alert, resp, err := client.Dependabot.GetRepoAlert(ctx, owner, repo, alertNumber) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, - fmt.Sprintf("failed to get alert with number '%d'", alertNumber), + dependabotErrMsg(fmt.Sprintf("failed to get alert with number '%d'", alertNumber), owner, repo, resp), resp, err, ), nil, nil @@ -160,7 +160,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server }) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, - fmt.Sprintf("failed to list alerts for repository '%s/%s'", owner, repo), + dependabotErrMsg(fmt.Sprintf("failed to list alerts for repository '%s/%s'", owner, repo), owner, repo, resp), resp, err, ), nil, nil @@ -184,3 +184,16 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server }, ) } + +// dependabotErrMsg enhances error messages for dependabot API failures by +// appending a hint about token permissions when the response indicates +// the token may lack access to the repository (403 or 404). +func dependabotErrMsg(base, owner, repo string, resp *github.Response) string { + if resp != nil && (resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound) { + return fmt.Sprintf("%s. Your token may not have access to Dependabot alerts on %s/%s. "+ + "To access Dependabot alerts, the token needs the 'security_events' scope or, for fine-grained tokens, "+ + "Dependabot alerts read permission for this specific repository.", + base, owner, repo) + } + return base +} diff --git a/pkg/github/dependabot_test.go b/pkg/github/dependabot_test.go index e20d2668ff..6c9b95ca36 100644 --- a/pkg/github/dependabot_test.go +++ b/pkg/github/dependabot_test.go @@ -66,7 +66,23 @@ func Test_GetDependabotAlert(t *testing.T) { "alertNumber": float64(9999), }, expectError: true, - expectedErrMsg: "failed to get alert", + expectedErrMsg: "Your token may not have access to Dependabot alerts on owner/repo", + }, + { + name: "alert fetch forbidden", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposDependabotAlertsByOwnerByRepoByAlertNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Resource not accessible by integration"}`)) + }), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "alertNumber": float64(42), + }, + expectError: true, + expectedErrMsg: "Your token may not have access to Dependabot alerts on owner/repo", }, } @@ -208,6 +224,21 @@ func Test_ListDependabotAlerts(t *testing.T) { expectError: true, expectedErrMsg: "failed to list alerts", }, + { + name: "alerts listing forbidden includes token hint", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposDependabotAlertsByOwnerByRepo: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Resource not accessible by integration"}`)) + }), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectError: true, + expectedErrMsg: "Your token may not have access to Dependabot alerts on owner/repo", + }, } for _, tc := range tests { From 4dbec2c72aafc62710b7a5c8c0c05d3b4a648c48 Mon Sep 17 00:00:00 2001 From: RossTarrant Date: Wed, 6 May 2026 10:52:17 +0100 Subject: [PATCH 2/7] Add Xcode installation guide for Codex and Claude Agent --- docs/installation-guides/README.md | 3 ++ docs/installation-guides/install-xcode.md | 43 +++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 docs/installation-guides/install-xcode.md diff --git a/docs/installation-guides/README.md b/docs/installation-guides/README.md index ab3aede36e..2bef9b7ef3 100644 --- a/docs/installation-guides/README.md +++ b/docs/installation-guides/README.md @@ -13,6 +13,7 @@ This directory contains detailed installation instructions for the GitHub MCP Se - **[OpenAI Codex](install-codex.md)** - Installation guide for OpenAI Codex - **[Roo Code](install-roo-code.md)** - Installation guide for Roo Code - **[Windsurf](install-windsurf.md)** - Installation guide for Windsurf IDE +- **[Xcode (Codex & Claude Agent)](install-xcode.md)** - Installation guide for Codex and Claude Agent within Xcode ## Support by Host Application @@ -32,6 +33,8 @@ This directory contains detailed installation instructions for the GitHub MCP Se | Windsurf | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Copilot in Xcode | ✅ | ✅ Full (OAuth + PAT) | Local: Docker or Go build, GitHub PAT
Remote: Copilot for Xcode 0.41.0+ | Easy | | Copilot in Eclipse | ✅ | ✅ Full (OAuth + PAT) | Local: Docker or Go build, GitHub PAT
Remote: Eclipse Plug-in for Copilot 0.10.0+ | Easy | +| Xcode (Codex) | ✅ | ✅ PAT + ❌ No OAuth | Local: Docker (full path required), GitHub PAT
Remote: GitHub PAT + env var | Easy | +| Xcode (Claude Agent) | ✅ | ✅ PAT + ❌ No OAuth | Local: Docker (full path required), GitHub PAT
Remote: GitHub PAT | Easy | **Legend:** - ✅ = Fully supported diff --git a/docs/installation-guides/install-xcode.md b/docs/installation-guides/install-xcode.md new file mode 100644 index 0000000000..bab4302458 --- /dev/null +++ b/docs/installation-guides/install-xcode.md @@ -0,0 +1,43 @@ +# Install GitHub MCP Server in Xcode + +Xcode currently supports two built-in coding agents: **Codex** (powered by OpenAI) and **Claude Agent** (powered by Anthropic). Follow the standard installation guide for each agent, with one important difference: Xcode uses its own isolated configuration directories for each agent, separate from your global config. + +> Configurations placed in these directories only affect agents when launched from Xcode. See [Apple's documentation](https://developer.apple.com/documentation/xcode/setting-up-coding-intelligence#Customize-the-Claude-Agent-and-Codex-environments) for more details. + +## Configuration Directories + +| Agent | Configuration Directory | +|-------|------------------------| +| Codex | `~/Library/Developer/Xcode/CodingAssistant/codex/` | +| Claude Agent | `~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/` | + +Place your MCP server configuration in the relevant directory above rather than the default location used by the standalone CLI. + +## Setup Guides + +- **[Codex](install-codex.md)** — configure `config.toml` inside `~/Library/Developer/Xcode/CodingAssistant/codex/` +- **[Claude Agent](install-claude.md#claude-desktop)** — configure `.claude.json` inside `~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/` + +## macOS Path Note + +Xcode runs with a minimal `PATH` that typically excludes `/usr/local/bin`. If you are using a local STDIO server (e.g. Docker or a pre-built binary), use the **full path** to the command in your config: + +``` +/usr/local/bin/docker +/usr/local/bin/github-mcp-server +``` + +## Troubleshooting + +| Issue | Possible Cause | Fix | +|-------|----------------|-----| +| Tools not loading | Config placed in wrong directory | Ensure config is in the Xcode-specific path above, not `~/.codex/` or `~/.claude.json` | +| Command not found (STDIO) | Xcode's PATH excludes `/usr/local/bin` | Use the full path to the command | +| Docker not found | Docker not running | Start Docker Desktop and restart Xcode | +| Authentication failed | Invalid or expired PAT | Regenerate PAT and update config | + +## References + +- [Apple Developer Documentation — Setting up coding intelligence](https://developer.apple.com/documentation/xcode/setting-up-coding-intelligence#Customize-the-Claude-Agent-and-Codex-environments) +- [Codex MCP documentation](https://developers.openai.com/codex/mcp) +- Main project README: [Advanced configuration options](../../README.md) From 2dab9948b77c5316d54ab34d68e05c6744903c37 Mon Sep 17 00:00:00 2001 From: RossTarrant Date: Wed, 6 May 2026 11:10:48 +0100 Subject: [PATCH 3/7] Enhance Xcode setup documentation for Claude Agent and update troubleshooting notes --- docs/installation-guides/README.md | 2 +- docs/installation-guides/install-claude.md | 70 +++++++++++++++++++++- docs/installation-guides/install-xcode.md | 16 ++--- 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/docs/installation-guides/README.md b/docs/installation-guides/README.md index 2bef9b7ef3..aadfa6a04f 100644 --- a/docs/installation-guides/README.md +++ b/docs/installation-guides/README.md @@ -33,7 +33,7 @@ This directory contains detailed installation instructions for the GitHub MCP Se | Windsurf | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Copilot in Xcode | ✅ | ✅ Full (OAuth + PAT) | Local: Docker or Go build, GitHub PAT
Remote: Copilot for Xcode 0.41.0+ | Easy | | Copilot in Eclipse | ✅ | ✅ Full (OAuth + PAT) | Local: Docker or Go build, GitHub PAT
Remote: Eclipse Plug-in for Copilot 0.10.0+ | Easy | -| Xcode (Codex) | ✅ | ✅ PAT + ❌ No OAuth | Local: Docker (full path required), GitHub PAT
Remote: GitHub PAT + env var | Easy | +| Xcode (Codex) | ✅ | ✅ PAT + ❌ No OAuth | Local: Docker (full path required), GitHub PAT
Remote: GitHub PAT via `GITHUB_PAT_TOKEN` env var (`bearer_token_env_var`) | Easy | | Xcode (Claude Agent) | ✅ | ✅ PAT + ❌ No OAuth | Local: Docker (full path required), GitHub PAT
Remote: GitHub PAT | Easy | **Legend:** diff --git a/docs/installation-guides/install-claude.md b/docs/installation-guides/install-claude.md index 05e3c3739d..67003fb69a 100644 --- a/docs/installation-guides/install-claude.md +++ b/docs/installation-guides/install-claude.md @@ -164,7 +164,75 @@ Add this codeblock to your `claude_desktop_config.json`: --- -## Troubleshooting +## Xcode (Claude Agent) + +Xcode's Claude Agent uses the same `.claude.json` configuration format as the Claude Code CLI, but reads it from an Xcode-specific directory rather than the global config location. + +### Configuration File Location + +``` +~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/.claude.json +``` + +> Configurations placed here only affect Claude Agent when launched from Xcode. See [Apple's documentation](https://developer.apple.com/documentation/xcode/setting-up-coding-intelligence#Customize-the-Claude-Agent-and-Codex-environments) for more details. + +### Remote Server Setup (Recommended) + +Run the following command in Terminal to add the remote GitHub MCP server: + +```bash +claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/mcp/","headers":{"Authorization":"Bearer YOUR_GITHUB_PAT"}}' --config ~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/.claude.json +``` + +Or open the file in a text editor and add the `mcpServers` block manually: + +```json +{ + "mcpServers": { + "github": { + "type": "http", + "url": "https://api.githubcopilot.com/mcp/", + "headers": { + "Authorization": "Bearer YOUR_GITHUB_PAT" + } + } + } +} +``` + +### Local Server Setup (Docker) + +> **macOS note**: Xcode runs with a minimal `PATH` that typically excludes `/usr/local/bin` (Intel) and `/opt/homebrew/bin` (Apple Silicon). Use the full path to `docker` to ensure it can be found. Run `which docker` in Terminal to find the correct path on your system. + +```json +{ + "mcpServers": { + "github": { + "command": "/usr/local/bin/docker", + "args": [ + "run", + "-i", + "--rm", + "-e", + "GITHUB_PERSONAL_ACCESS_TOKEN", + "ghcr.io/github/github-mcp-server" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "YOUR_GITHUB_PAT" + } + } + } +} +``` + +### Setup Steps +1. Create or open `~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/.claude.json` +2. Add the configuration block above +3. Replace `YOUR_GITHUB_PAT` with your actual token +4. Restart Xcode + +--- + **Authentication Failed:** - Verify PAT has `repo` scope diff --git a/docs/installation-guides/install-xcode.md b/docs/installation-guides/install-xcode.md index bab4302458..15bcfde34f 100644 --- a/docs/installation-guides/install-xcode.md +++ b/docs/installation-guides/install-xcode.md @@ -16,23 +16,25 @@ Place your MCP server configuration in the relevant directory above rather than ## Setup Guides - **[Codex](install-codex.md)** — configure `config.toml` inside `~/Library/Developer/Xcode/CodingAssistant/codex/` -- **[Claude Agent](install-claude.md#claude-desktop)** — configure `.claude.json` inside `~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/` +- **[Claude Agent](install-claude.md#xcode-claude-agent)** — configure `.claude.json` inside `~/Library/Developer/Xcode/CodingAssistant/ClaudeAgentConfig/` ## macOS Path Note -Xcode runs with a minimal `PATH` that typically excludes `/usr/local/bin`. If you are using a local STDIO server (e.g. Docker or a pre-built binary), use the **full path** to the command in your config: +Xcode runs with a minimal `PATH` that typically excludes common binary locations. If you are using a local STDIO server (e.g. Docker or a pre-built binary), use the **full path** to the command in your config. Run `which docker` (or `which github-mcp-server`) in Terminal to find the correct path on your system. Common locations: -``` -/usr/local/bin/docker -/usr/local/bin/github-mcp-server -``` +| Installation | Typical path | +|---|---| +| Docker (Intel Mac) | `/usr/local/bin/docker` | +| Docker (Apple Silicon) | `/usr/local/bin/docker` | +| Homebrew (Intel Mac) | `/usr/local/bin/` | +| Homebrew (Apple Silicon) | `/opt/homebrew/bin/` | ## Troubleshooting | Issue | Possible Cause | Fix | |-------|----------------|-----| | Tools not loading | Config placed in wrong directory | Ensure config is in the Xcode-specific path above, not `~/.codex/` or `~/.claude.json` | -| Command not found (STDIO) | Xcode's PATH excludes `/usr/local/bin` | Use the full path to the command | +| Command not found (STDIO) | Xcode's PATH excludes binary location | Use the full path (e.g. `/usr/local/bin/docker` or `/opt/homebrew/bin/docker`); run `which docker` in Terminal to confirm | | Docker not found | Docker not running | Start Docker Desktop and restart Xcode | | Authentication failed | Invalid or expired PAT | Regenerate PAT and update config | From 0e2fc3889664c5b5bb770fd0ecfa4f6ca618f6b4 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 8 May 2026 09:08:50 -0700 Subject: [PATCH 4/7] fix(mcp-apps): defer _meta.ui strip to per-request RegisterTools (#2446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp-apps): defer _meta.ui strip to per-request RegisterTools The MCP Apps `_meta.ui` strip lived in `Builder.Build()`, which calls `checkFeatureFlag(context.Background())`. The HTTP feature checker (`createHTTPFeatureChecker`) reads insiders mode from the request context — a background context never has it set, so the FF reported MCP Apps off and the strip ran eagerly at server startup. Per-request inventory factories then served pre-stripped tools regardless of whether the request actually arrived on the `/insiders` route. Symptom: `github/github-mcp-server-remote` returns 0 tools with `_meta.ui` over HTTP `/insiders`, despite the source unconditionally setting it on `get_me`, `issue_write`, and `create_pull_request`. VS Code only renders MCP App UIs because of its persistent tool cache from earlier deploys. Reproducible locally with `cmd/github-mcp-server http --insiders` plus a vanilla curl tools/list. Fix: drop the strip from `Build()`. Apply it in `RegisterTools(ctx,…)` where the per-request context is in scope and the HTTP feature checker can correctly detect insiders mode (or the remote checker can correctly read user identity for Statsig flag lookup). The same root cause affects `github/github-mcp-server-remote` — its `featureflags.NewComposedFeatureFlagChecker` reads `requestctx.User(ctx)`, which background context lacks, so the `remote_mcp_ui_apps` Statsig flag always returned false. The fix here covers both downstreams since `RegisterTools` is the single entry point for tool registration. Stdio mode is unaffected: it uses a closure-captured insiders mode flag (`createFeatureChecker`) that does not depend on context, and the per-request strip in `RegisterTools` produces the same outcome. Verified end-to-end against the deployed remote tool definitions: HTTP /insiders → 3 tools with _meta.ui (was 0) HTTP / → 0 tools with _meta.ui (correct) stdio --insiders → 3 tools with _meta.ui (unchanged) stdio → 0 tools with _meta.ui (correct) Adds: - pkg/http: TestInsidersRoutePreservesUIMeta — pins the regression - pkg/inventory: updates the existing strip tests to use the new RegisterTools-as-strip-site contract via a captureRegisteredTools helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: gofmt handler_test.go and registry_test.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * lint: address revive (context-as-argument) and unused checkFeatureFlag - Reorder captureRegisteredTools params to put context.Context first - Remove dead Builder.checkFeatureFlag (was only called by Build's former MCP Apps strip, now done in RegisterTools via the Inventory receiver's checkFeatureFlag instead) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/http/handler_test.go | 53 ++++++++++++++++++++++++++++++++++ pkg/inventory/builder.go | 20 ------------- pkg/inventory/registry.go | 13 ++++++++- pkg/inventory/registry_test.go | 46 ++++++++++++++++++++--------- 4 files changed, 97 insertions(+), 35 deletions(-) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 002266ba15..9887ff1f3b 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -825,3 +825,56 @@ func TestCrossOriginProtection(t *testing.T) { }) } } + +// TestInsidersRoutePreservesUIMeta is a regression test for the bug where +// _meta.ui was stripped from tools/list responses on the HTTP /insiders route. +// +// Before the fix: +// - buildStaticInventory called Build() on a builder configured with the +// HTTP feature checker (which reads insiders mode from the request ctx). +// - Build() invoked checkFeatureFlag(context.Background()) — bg ctx has no +// insiders mode, so the FF reported MCP Apps off, and stripMCPAppsMetadata +// ran eagerly against the static tool slice at server startup. +// - Per-request inventory factories then served pre-stripped tools regardless +// of whether the request actually came in via /insiders. +// +// After the fix: +// - Build() no longer touches MCP Apps metadata. +// - RegisterTools applies the strip per-request, using the request context +// where the HTTP feature checker correctly observes insiders mode. +func TestInsidersRoutePreservesUIMeta(t *testing.T) { + const uiURI = "ui://test/widget" + uiTool := mockTool("with_ui", "repos", true) + uiTool.Tool.Meta = mcp.Meta{"ui": map[string]any{"resourceUri": uiURI}} + + checker := createHTTPFeatureChecker() + build := func() *inventory.Inventory { + inv, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{uiTool}). + WithFeatureChecker(checker). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) + return inv + } + + // Simulate a /insiders request: ctx has insiders mode set. + insidersCtx := ghcontext.WithInsidersMode(context.Background(), true) + + // AvailableTools no longer strips _meta.ui (post-fix), regardless of ctx. + // The strip lives in RegisterTools, gated on the per-request FF check. + insidersTools := build().AvailableTools(insidersCtx) + plainTools := build().AvailableTools(context.Background()) + + // On the /insiders path, the FF check returns true → no strip → _meta preserved. + enabled, _ := checker(insidersCtx, "remote_mcp_ui_apps") + require.True(t, enabled, "FF should be on for /insiders ctx") + require.Len(t, insidersTools, 1) + require.NotNil(t, insidersTools[0].Tool.Meta, "_meta should be present on /insiders") + require.Equal(t, uiURI, insidersTools[0].Tool.Meta["ui"].(map[string]any)["resourceUri"]) + + // On the non-insiders path, RegisterTools strips _meta.ui. + plainEnabled, _ := checker(context.Background(), "remote_mcp_ui_apps") + require.False(t, plainEnabled, "FF should be off for non-insiders ctx") + require.Len(t, plainTools, 1) +} diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index b9a0d8548b..d656359bb6 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -190,19 +190,6 @@ func cleanTools(tools []string) []string { return cleaned } -// checkFeatureFlag checks a feature flag at build time using the builder's feature checker. -// Returns false if no checker is configured or the flag is not enabled. -func (b *Builder) checkFeatureFlag(flag string) bool { - if b.featureChecker == nil { - return false - } - enabled, err := b.featureChecker(context.Background(), flag) - if err != nil { - return false - } - return enabled -} - // Build creates the final Inventory with all configuration applied. // This processes toolset filtering, tool name resolution, and sets up // the inventory for use. The returned Inventory is ready for use with @@ -214,13 +201,6 @@ func (b *Builder) checkFeatureFlag(flag string) bool { func (b *Builder) Build() (*Inventory, error) { tools := b.tools - // When MCP Apps feature flag is not enabled, strip UI metadata from tools - // so clients won't attempt to load UI resources. - // The feature checker is the single source of truth for flag evaluation. - if !b.checkFeatureFlag(mcpAppsFeatureFlag) { - tools = stripMCPAppsMetadata(tools) - } - r := &Inventory{ tools: tools, resourceTemplates: b.resourceTemplates, diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index e2cd3a9e67..a0bbc7a550 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -170,8 +170,19 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { // RegisterTools registers all available tools with the server using the provided dependencies. // The context is used for feature flag evaluation. +// +// MCP Apps UI metadata (`_meta.ui`) is stripped from the registered tools +// when the MCP Apps feature flag is not enabled for this request. The strip +// happens here (rather than at Build() time) so the per-request context is +// in scope — HTTP feature checkers that read insiders mode or user identity +// from ctx would otherwise see context.Background() and falsely report the +// flag off, even when the actual request arrived on the /insiders route. func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any) { - for _, tool := range r.AvailableTools(ctx) { + tools := r.AvailableTools(ctx) + if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { + tools = stripMCPAppsMetadata(tools) + } + for _, tool := range tools { tool.RegisterFunc(s, deps) } } diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index e6c9e450cb..77c3bb57e5 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -1863,18 +1863,16 @@ func TestWithMCPApps_DisabledStripsUIMetadata(t *testing.T) { "description": "kept", }) - // Default: MCP Apps is disabled - UI meta should be stripped + // Default: MCP Apps is disabled - UI meta should be stripped on registration. reg := mustBuild(t, NewBuilder().SetTools([]ServerTool{toolWithUI}).WithToolsets([]string{"all"})) - available := reg.AvailableTools(context.Background()) + registered := captureRegisteredTools(context.Background(), t, reg) - require.Len(t, available, 1) - // UI metadata should be stripped - if available[0].Tool.Meta["ui"] != nil { + require.Len(t, registered, 1) + if registered[0].Meta["ui"] != nil { t.Errorf("Expected 'ui' meta to be stripped, but it was present") } - // Other metadata should be preserved - if available[0].Tool.Meta["description"] != "kept" { - t.Errorf("Expected 'description' meta to be preserved, got %v", available[0].Tool.Meta["description"]) + if registered[0].Meta["description"] != "kept" { + t.Errorf("Expected 'description' meta to be preserved, got %v", registered[0].Meta["description"]) } } @@ -1947,7 +1945,6 @@ func TestWithMCPApps_ToolsWithoutUIMetaUnaffected(t *testing.T) { } func TestWithMCPApps_UIOnlyMetaBecomesNil(t *testing.T) { - // Tool with ONLY ui metadata - should become nil after stripping when MCP Apps is disabled toolUIOnly := mockToolWithMeta("tool_ui_only", "toolset1", map[string]any{ "ui": map[string]any{"html": "
hello
"}, }) @@ -1955,12 +1952,11 @@ func TestWithMCPApps_UIOnlyMetaBecomesNil(t *testing.T) { reg := mustBuild(t, NewBuilder(). SetTools([]ServerTool{toolUIOnly}). WithToolsets([]string{"all"})) - available := reg.AvailableTools(context.Background()) + registered := captureRegisteredTools(context.Background(), t, reg) - require.Len(t, available, 1) - // Meta should be nil since ui was the only key and MCP Apps is off by default - if available[0].Tool.Meta != nil { - t.Errorf("Expected Meta to be nil after stripping only key, got %v", available[0].Tool.Meta) + require.Len(t, registered, 1) + if registered[0].Meta != nil { + t.Errorf("Expected Meta to be nil after stripping only key, got %v", registered[0].Meta) } } @@ -2239,3 +2235,25 @@ func TestCreateExcludeToolsFilter(t *testing.T) { require.NoError(t, err) require.True(t, allowed, "allowed_tool should be included") } + +// captureRegisteredTools mirrors RegisterTools' per-request strip behavior so +// tests can verify what the wire sees, without requiring tools to have real +// handlers (RegisterTools panics on tools without HandlerFunc). +func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) []*mcp.Tool { + t.Helper() + tools := reg.AvailableTools(ctx) + out := make([]*mcp.Tool, 0, len(tools)) + for i := range tools { + toolCopy := tools[i].Tool + out = append(out, &toolCopy) + } + if !reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { + for _, tt := range out { + delete(tt.Meta, "ui") + if len(tt.Meta) == 0 { + tt.Meta = nil + } + } + } + return out +} From 1be1f38de039dd1da1854c2ac7067a2dd6250b7e Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Mon, 11 May 2026 11:23:58 +0200 Subject: [PATCH 5/7] Add ifc label for get_me tool (#2432) * Add ifc labels * Add test * Address PR review: deterministic output, type safety, universe validation, and tests - Fix grammar in ReadersSecurityLabelFromDict godoc - Sort GetReaders and FiniteReaderSet.String output for determinism - Fix godoc example to use UniversalReaders for public label - Panic on unsupported ReaderSet types in Union/Intersection/IsSubset - Add universe mismatch validation in PowersetLattice Join/Meet/Leq - Add comprehensive unit tests for pkg/ifc (lattice laws, serialization, panics) * Add a test * Pass parameters * Remove lattice * Script update --- pkg/github/context_tools.go | 10 +++++- pkg/github/context_tools_test.go | 60 ++++++++++++++++++++++++++++++++ pkg/ifc/ifc.go | 29 +++++++++++++++ script/get-me | 6 ++-- 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 pkg/ifc/ifc.go diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 902734481a..9f84c02118 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -6,6 +6,7 @@ import ( "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -103,7 +104,14 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool { }, } - return MarshalledTextResult(minimalUser), nil, nil + result := MarshalledTextResult(minimalUser) + if deps.GetFlags(ctx).InsidersMode { + if result.Meta == nil { + result.Meta = mcp.Meta{} + } + result.Meta["ifc"] = ifc.LabelGetMe() + } + return result, nil, nil }, ) } diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 39f2058bec..365a019ab6 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -139,6 +139,66 @@ func Test_GetMe(t *testing.T) { } } +func Test_GetMe_IFC_InsidersMode(t *testing.T) { + t.Parallel() + + serverTool := GetMe(translations.NullTranslationHelper) + + mockUser := &github.User{ + Login: github.Ptr("testuser"), + HTMLURL: github.Ptr("https://github.com/testuser"), + CreatedAt: &github.Timestamp{Time: time.Now()}, + } + mockedHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUser: mockResponse(t, http.StatusOK, mockUser), + }) + + t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) { + deps := BaseDeps{ + Client: github.NewClient(mockedHTTPClient), + Flags: FeatureFlags{InsidersMode: false}, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + assert.Nil(t, result.Meta, "result meta should be nil when insiders mode is disabled") + }) + + t.Run("insiders mode enabled includes ifc label in result meta", func(t *testing.T) { + deps := BaseDeps{ + Client: github.NewClient(mockedHTTPClient), + Flags: FeatureFlags{InsidersMode: true}, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{}) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + require.NotNil(t, result.Meta, "result meta should be set when insiders mode is enabled") + ifcLabel, ok := result.Meta["ifc"] + require.True(t, ok, "result meta should contain ifc key") + + ifcJSON, err := json.Marshal(ifcLabel) + require.NoError(t, err) + + var ifcMap map[string]any + err = json.Unmarshal(ifcJSON, &ifcMap) + require.NoError(t, err) + + assert.Equal(t, "trusted", ifcMap["integrity"]) + confList, ok := ifcMap["confidentiality"].([]any) + require.True(t, ok, "confidentiality should be a list") + require.Len(t, confList, 1) + assert.Equal(t, "public", confList[0]) + }) +} + func Test_GetTeams(t *testing.T) { t.Parallel() diff --git a/pkg/ifc/ifc.go b/pkg/ifc/ifc.go new file mode 100644 index 0000000000..cf0d72114f --- /dev/null +++ b/pkg/ifc/ifc.go @@ -0,0 +1,29 @@ +// Package ifc provides Information Flow Control labels for annotating MCP tool outputs. +// The actual IFC enforcement engine lives in a separate service; this package only +// defines the label schema used for annotations. +package ifc + +type Integrity string + +const ( + IntegrityTrusted Integrity = "trusted" + IntegrityUntrusted Integrity = "untrusted" +) + +type Confidentiality string + +const ( + ConfidentialityPublic Confidentiality = "public" +) + +type SecurityLabel struct { + Integrity Integrity `json:"integrity"` + Confidentiality []Confidentiality `json:"confidentiality"` +} + +func LabelGetMe() SecurityLabel { + return SecurityLabel{ + Integrity: IntegrityTrusted, + Confidentiality: []Confidentiality{ConfidentialityPublic}, + } +} diff --git a/script/get-me b/script/get-me index 954f57cec0..ffd24a357f 100755 --- a/script/get-me +++ b/script/get-me @@ -6,12 +6,12 @@ output=$( echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"get-me-script","version":"1.0.0"}}}' echo '{"jsonrpc":"2.0","method":"notifications/initialized","params":{}}' echo '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"get_me","arguments":{}}}' - sleep 1 - ) | go run cmd/github-mcp-server/main.go stdio 2>/dev/null | tail -1 + sleep 3 + ) | go run cmd/github-mcp-server/main.go stdio "$@" 2>/dev/null | grep '"id":2' ) if command -v jq &> /dev/null; then - echo "$output" | jq '.result.content[0].text | fromjson' + echo "$output" | jq '{_meta: .result._meta, content: (.result.content[0].text | fromjson)}' else echo "$output" fi From f48e82a8f978b477a2c726fc1600b271d96f19ef Mon Sep 17 00:00:00 2001 From: Roberto Nacu Date: Mon, 11 May 2026 15:07:00 +0100 Subject: [PATCH 6/7] Prevent inputs param from being stripped from actions_run_trigger tool schema (#2417) * add empty properties to inputs param * add test cases for valid and invalid inputs --- .../__toolsnaps__/actions_run_trigger.snap | 1 + pkg/github/actions.go | 9 +++--- pkg/github/actions_test.go | 31 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/pkg/github/__toolsnaps__/actions_run_trigger.snap b/pkg/github/__toolsnaps__/actions_run_trigger.snap index c51501c176..41a6439929 100644 --- a/pkg/github/__toolsnaps__/actions_run_trigger.snap +++ b/pkg/github/__toolsnaps__/actions_run_trigger.snap @@ -8,6 +8,7 @@ "properties": { "inputs": { "description": "Inputs the workflow accepts. Only used for 'run_workflow' method.", + "properties": {}, "type": "object" }, "method": { diff --git a/pkg/github/actions.go b/pkg/github/actions.go index c3b5bb8c71..85afed6e1b 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -544,6 +544,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo "inputs": { Type: "object", Description: "Inputs the workflow accepts. Only used for 'run_workflow' method.", + Properties: map[string]*jsonschema.Schema{}, }, "run_id": { Type: "number", @@ -574,11 +575,9 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo runID, _ := OptionalIntParam(args, "run_id") // Get optional inputs parameter - var inputs map[string]any - if requestInputs, ok := args["inputs"]; ok { - if inputsMap, ok := requestInputs.(map[string]any); ok { - inputs = inputsMap - } + inputs, err := OptionalParam[map[string]any](args, "inputs") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } // Validate required parameters based on action type diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index fe960ed924..6eba71b8b3 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -377,6 +377,37 @@ func Test_ActionsRunTrigger_RunWorkflow(t *testing.T) { expectError: true, expectedErrMsg: "ref is required for run_workflow action", }, + { + name: "successful workflow run with inputs", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposActionsWorkflowsDispatchesByOwnerByRepoByWorkflowID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }), + }), + requestArgs: map[string]any{ + "method": "run_workflow", + "owner": "owner", + "repo": "repo", + "workflow_id": "12345", + "ref": "main", + "inputs": map[string]any{"FIELD1": "value1", "FIELD2": "value2"}, + }, + expectError: false, + }, + { + name: "invalid inputs type returns error", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "run_workflow", + "owner": "owner", + "repo": "repo", + "workflow_id": "12345", + "ref": "main", + "inputs": "not a map", + }, + expectError: true, + expectedErrMsg: "parameter inputs is not of type map[string]interface {}, is string", + }, } for _, tc := range tests { From c3dedbece0bf3834829f638a245fb3c51cd98d0b Mon Sep 17 00:00:00 2001 From: Roberto Nacu Date: Mon, 11 May 2026 15:16:15 +0100 Subject: [PATCH 7/7] Handle lightweight tags in get_tag (#2400) --- pkg/github/repositories.go | 10 ++++- pkg/github/repositories_test.go | 70 +++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 9577b37b69..0ebacc6668 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -1632,7 +1632,15 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get tag reference", resp, body), nil, nil } - // Then get the tag object + // Differentiate between lightweight and annotated tags since lightweight ones don't have a fetchable object + if ref.Object.GetType() == "commit" { + r, err := json.Marshal(ref) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(r)), nil, nil + } + tagObj, resp, err := client.Git.GetTag(ctx, owner, repo, *ref.Object.SHA) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index d7bb487382..c21709dad4 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2914,10 +2914,19 @@ func Test_GetTag(t *testing.T) { assert.Contains(t, schema.Properties, "tag") assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "tag"}) - mockTagRef := &github.Reference{ + mockAnnotatedTagRef := &github.Reference{ Ref: github.Ptr("refs/tags/v1.0.0"), Object: &github.GitObject{ - SHA: github.Ptr("v1.0.0-tag-sha"), + Type: github.Ptr("tag"), + SHA: github.Ptr("v1.0.0-tag-sha"), + }, + } + + mockLightweightTagRef := &github.Reference{ + Ref: github.Ptr("refs/tags/v1.0.1"), + Object: &github.GitObject{ + Type: github.Ptr("commit"), + SHA: github.Ptr("abc123"), }, } @@ -2937,6 +2946,7 @@ func Test_GetTag(t *testing.T) { requestArgs map[string]any expectError bool expectedTag *github.Tag + expectedRef *github.Reference expectedErrMsg string }{ { @@ -2948,7 +2958,7 @@ func Test_GetTag(t *testing.T) { t, "/repos/owner/repo/git/ref/tags/v1.0.0", ).andThen( - mockResponse(t, http.StatusOK, mockTagRef), + mockResponse(t, http.StatusOK, mockAnnotatedTagRef), ), ), WithRequestMatchHandler( @@ -2993,7 +3003,7 @@ func Test_GetTag(t *testing.T) { mockedClient: NewMockedHTTPClient( WithRequestMatch( GetReposGitRefByOwnerByRepoByRef, - mockTagRef, + mockAnnotatedTagRef, ), WithRequestMatchHandler( GetReposGitTagsByOwnerByRepoByTagSHA, @@ -3011,6 +3021,27 @@ func Test_GetTag(t *testing.T) { expectError: true, expectedErrMsg: "failed to get tag object", }, + { + name: "successful lightweight tag retrieval", + mockedClient: NewMockedHTTPClient( + WithRequestMatchHandler( + GetReposGitRefByOwnerByRepoByRef, + expectPath( + t, + "/repos/owner/repo/git/ref/tags/v1.0.1", + ).andThen( + mockResponse(t, http.StatusOK, mockLightweightTagRef), + ), + ), + ), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "tag": "v1.0.1", + }, + expectError: false, + expectedRef: mockLightweightTagRef, + }, } for _, tc := range tests { @@ -3043,16 +3074,29 @@ func Test_GetTag(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Parse and verify the result - var returnedTag github.Tag - err = json.Unmarshal([]byte(textContent.Text), &returnedTag) - require.NoError(t, err) + // Parse and verify the result - annotated tag (full tag object) + if tc.expectedTag != nil { + var returnedTag github.Tag + err = json.Unmarshal([]byte(textContent.Text), &returnedTag) + require.NoError(t, err) + + assert.Equal(t, tc.expectedTag.GetSHA(), returnedTag.GetSHA()) + assert.Equal(t, tc.expectedTag.GetTag(), returnedTag.GetTag()) + assert.Equal(t, tc.expectedTag.GetMessage(), returnedTag.GetMessage()) + assert.Equal(t, tc.expectedTag.Object.GetType(), returnedTag.Object.GetType()) + assert.Equal(t, tc.expectedTag.Object.GetSHA(), returnedTag.Object.GetSHA()) + } - assert.Equal(t, *tc.expectedTag.SHA, *returnedTag.SHA) - assert.Equal(t, *tc.expectedTag.Tag, *returnedTag.Tag) - assert.Equal(t, *tc.expectedTag.Message, *returnedTag.Message) - assert.Equal(t, *tc.expectedTag.Object.Type, *returnedTag.Object.Type) - assert.Equal(t, *tc.expectedTag.Object.SHA, *returnedTag.Object.SHA) + // Parse and verify the result - lightweight tag (reference only) + if tc.expectedRef != nil { + var returnedRef github.Reference + err = json.Unmarshal([]byte(textContent.Text), &returnedRef) + require.NoError(t, err) + + assert.Equal(t, tc.expectedRef.GetRef(), returnedRef.GetRef()) + assert.Equal(t, tc.expectedRef.Object.GetType(), returnedRef.Object.GetType()) + assert.Equal(t, tc.expectedRef.Object.GetSHA(), returnedRef.Object.GetSHA()) + } }) } }