From 561a4a79515309b45a373cf06d01791f7dc11fd2 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 May 2026 16:24:05 +0100 Subject: [PATCH 01/21] Strip _meta.ui when client lacks UI capability Per the MCP Apps 2026-01-26 spec, servers SHOULD check client capabilities before advertising UI-enabled tools. Extend the inventory strip gate to remove _meta.ui not only when the feature flag is off, but also when the request context explicitly reports the client lacks UI support (HasUISupport returns supported=false, ok=true). When the capability is unknown (ok=false, e.g. stdio paths), fall through to the existing feature-flag gate so existing behaviour is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/http/handler_test.go | 41 +++++++++++++++++++++++++ pkg/inventory/registry.go | 45 +++++++++++++++++++++------- pkg/inventory/registry_test.go | 55 +++++++++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 11 deletions(-) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index a36469133c..4f697ee0cb 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -904,3 +904,44 @@ func TestInsidersRoutePreservesUIMeta(t *testing.T) { require.False(t, plainEnabled, "FF should be off for non-insiders ctx") require.Len(t, plainTools, 1) } + +// TestUIMetaStrippedWhenClientLacksCapability verifies that even on the +// /insiders path (where the feature flag is on), UI metadata is stripped from +// tools/list responses when the client did NOT advertise the +// io.modelcontextprotocol/ui extension capability. Per the 2026-01-26 MCP +// Apps spec, servers SHOULD check client capabilities before exposing +// UI-enabled tools. +func TestUIMetaStrippedWhenClientLacksCapability(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(nil, false) + build := func() *inventory.Inventory { + inv, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{uiTool}). + WithFeatureChecker(checker). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) + return inv + } + + insidersCtx := ghcontext.WithInsidersMode(context.Background(), true) + withoutUICap := ghcontext.WithUISupport(insidersCtx, false) + withUICap := ghcontext.WithUISupport(insidersCtx, true) + + stripped := build().ToolsForRegistration(withoutUICap) + require.Len(t, stripped, 1) + require.Nil(t, stripped[0].Tool.Meta["ui"], "_meta.ui should be stripped when client lacks UI capability") + + preserved := build().ToolsForRegistration(withUICap) + require.Len(t, preserved, 1) + require.NotNil(t, preserved[0].Tool.Meta["ui"], "_meta.ui should be preserved when client advertises UI capability") + require.Equal(t, uiURI, preserved[0].Tool.Meta["ui"].(map[string]any)["resourceUri"]) + + // Unknown capability falls through to the FF gate (insiders ctx → kept). + unknown := build().ToolsForRegistration(insidersCtx) + require.Len(t, unknown, 1) + require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on") +} diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index d147cbfc66..b8a70a3420 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -7,6 +7,7 @@ import ( "slices" "sort" + ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -169,26 +170,50 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { // ToolsForRegistration returns AvailableTools(ctx) post-processed exactly as // RegisterTools would expose them: with MCP Apps UI metadata stripped when -// the remote_mcp_ui_apps feature flag is not enabled in ctx. Useful for -// documentation generators and diagnostics that need the same view of the -// tool surface the server would register. +// the client cannot consume it. Useful for documentation generators and +// diagnostics that need the same view of the tool surface the server would +// register. +// +// The strip applies when EITHER of the following is true: +// +// - The remote_mcp_ui_apps feature flag is not enabled in ctx (server-side gate). +// - The client explicitly did not advertise the io.modelcontextprotocol/ui +// extension capability (per the 2026-01-26 MCP Apps spec, servers SHOULD +// check client capabilities before exposing UI-enabled tools). When the +// capability is unknown (e.g. stdio paths that do not populate the +// context flag) the feature-flag gate is the sole source of truth. func (r *Inventory) ToolsForRegistration(ctx context.Context) []ServerTool { tools := r.AvailableTools(ctx) - if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { + if shouldStripMCPAppsMetadata(ctx, r.checkFeatureFlag(ctx, mcpAppsFeatureFlag)) { tools = stripMCPAppsMetadata(tools) } return tools } +// shouldStripMCPAppsMetadata centralises the strip decision so the same logic +// is exercised by tests and by RegisterTools. +func shouldStripMCPAppsMetadata(ctx context.Context, featureFlagEnabled bool) bool { + if !featureFlagEnabled { + return true + } + // Feature flag is on. Respect the client capability if it is known. + if supported, ok := ghcontext.HasUISupport(ctx); ok && !supported { + return true + } + return false +} + // RegisterTools registers all available tools with the server using the provided dependencies. -// The context is used for feature flag evaluation. +// The context is used for feature flag evaluation and client capability checks. // // 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. +// when either the MCP Apps feature flag is not enabled for this request, or +// the client did not advertise the io.modelcontextprotocol/ui extension. 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.ToolsForRegistration(ctx) { tool.RegisterFunc(s, deps) diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index 372f756023..20b1fb718c 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -6,6 +6,7 @@ import ( "fmt" "testing" + ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/require" ) @@ -2211,7 +2212,7 @@ func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) [ toolCopy := tools[i].Tool out = append(out, &toolCopy) } - if !reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { + if shouldStripMCPAppsMetadata(ctx, reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag)) { for _, tt := range out { delete(tt.Meta, "ui") if len(tt.Meta) == 0 { @@ -2221,3 +2222,55 @@ func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) [ } return out } + +// TestShouldStripMCPAppsMetadata verifies the spec-conformant strip decision: +// strip when the feature flag is off, OR when the client explicitly does not +// advertise the io.modelcontextprotocol/ui extension. +func TestShouldStripMCPAppsMetadata(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setupCtx func() context.Context + ffOn bool + want bool + }{ + { + name: "FF off, capability unknown -> strip", + setupCtx: context.Background, + ffOn: false, + want: true, + }, + { + name: "FF off, capability present -> strip (FF wins)", + setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) }, + ffOn: false, + want: true, + }, + { + name: "FF on, capability unknown -> keep", + setupCtx: context.Background, + ffOn: true, + want: false, + }, + { + name: "FF on, capability present -> keep", + setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) }, + ffOn: true, + want: false, + }, + { + name: "FF on, capability explicitly absent -> strip", + setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), false) }, + ffOn: true, + want: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := shouldStripMCPAppsMetadata(tc.setupCtx(), tc.ffOn) + require.Equal(t, tc.want, got) + }) + } +} From 69f786b87c70e3d06d238e82dc8b149a82d9972d Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 May 2026 16:24:11 +0100 Subject: [PATCH 02/21] Align UI resources with MCP Apps 2026-01-26 polish recommendations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Explicitly set prefersBorder on every UI resource — false for the get_me profile card, true for the issue/PR write forms — since hosts' defaults vary. * Declare an empty csp on issue_write_ui and pr_write_ui to document that they need no external origins. * Point spec link comment at the stable 2026-01-26 location. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/ui_resources.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/pkg/github/ui_resources.go b/pkg/github/ui_resources.go index c41d2ac3f1..ab3ebfd163 100644 --- a/pkg/github/ui_resources.go +++ b/pkg/github/ui_resources.go @@ -10,6 +10,9 @@ import ( // These are static resources (not templates) that serve HTML content for // MCP App-enabled tools. The HTML is built from React/Primer components // in the ui/ directory using `script/build-ui`. +// +// Resource metadata follows the stable 2026-01-26 MCP Apps spec: +// https://github.com/modelcontextprotocol/ext-apps/blob/main/specification/2026-01-26/apps.mdx func RegisterUIResources(s *mcp.Server) { // Register the get_me UI resource s.AddResource( @@ -27,14 +30,14 @@ func RegisterUIResources(s *mcp.Server) { URI: GetMeUIResourceURI, MIMEType: MCPAppMIMEType, Text: html, - // MCP Apps UI metadata - CSP configuration to allow loading GitHub avatars - // See: https://github.com/modelcontextprotocol/ext-apps/blob/main/specification/draft/apps.mdx Meta: mcp.Meta{ "ui": map[string]any{ + // Allow loading images from GitHub's avatar CDN. "csp": map[string]any{ - // Allow loading images from GitHub's avatar CDN "resourceDomains": []string{"https://avatars.githubusercontent.com"}, }, + // Profile card renders inline within chat without a host border. + "prefersBorder": false, }, }, }, @@ -59,6 +62,14 @@ func RegisterUIResources(s *mcp.Server) { URI: IssueWriteUIResourceURI, MIMEType: MCPAppMIMEType, Text: html, + Meta: mcp.Meta{ + "ui": map[string]any{ + // No external origins required; documents the secure default. + "csp": map[string]any{}, + // Form surface benefits from a host-provided border. + "prefersBorder": true, + }, + }, }, }, }, nil @@ -81,6 +92,12 @@ func RegisterUIResources(s *mcp.Server) { URI: PullRequestWriteUIResourceURI, MIMEType: MCPAppMIMEType, Text: html, + Meta: mcp.Meta{ + "ui": map[string]any{ + "csp": map[string]any{}, + "prefersBorder": true, + }, + }, }, }, }, nil From c0dca1f1b8c25dc72d70dc7fb5433fb790324cb5 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 May 2026 16:29:15 +0100 Subject: [PATCH 03/21] Adopt MCP Apps 2026-01-26 view-side capabilities * Declare appCapabilities.availableDisplayModes (defaults to ["inline"]) during initialization, as required by the new spec. * Track McpUiHostContext (and its updates via onhostcontextchanged) and thread it into AppProvider, which now picks up host-supplied theme + CSS style variables and projects them onto the root element so Primer components inherit host theming. * Add setModelContext and openLink helpers to useMcpApp. issue-write and pr-write call setModelContext on a successful submission so the agent has the new entity in its next-turn context; get-me uses openLink for the profile's external blog link. The pinned @modelcontextprotocol/ext-apps ^1.7.2 was already resolved to 1.7.2 in the lockfile, so no dependency bump is required for the new HostContext / openLink / updateModelContext APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ui/src/apps/get-me/App.tsx | 77 +++++++++++++++++++------------ ui/src/apps/issue-write/App.tsx | 25 ++++++++-- ui/src/apps/pr-write/App.tsx | 23 ++++++--- ui/src/components/AppProvider.tsx | 42 +++++++++++++---- ui/src/hooks/useMcpApp.ts | 74 +++++++++++++++++++++++++++-- 5 files changed, 187 insertions(+), 54 deletions(-) diff --git a/ui/src/apps/get-me/App.tsx b/ui/src/apps/get-me/App.tsx index a20aae17c5..c181fcab90 100644 --- a/ui/src/apps/get-me/App.tsx +++ b/ui/src/apps/get-me/App.tsx @@ -1,4 +1,5 @@ import { StrictMode, useState } from "react"; +import type React from "react"; import { createRoot } from "react-dom/client"; import { Avatar, Box, Text, Link, Heading, Spinner } from "@primer/react"; import { @@ -62,8 +63,20 @@ function AvatarWithFallback({ src, login, size }: { src?: string; login: string; ); } -function UserCard({ user }: { user: UserData }) { +function UserCard({ + user, + onOpenLink, +}: { + user: UserData; + onOpenLink?: (url: string) => void; +}) { const d = user.details || {}; + const handleClick = + onOpenLink && + ((url: string) => (e: React.MouseEvent) => { + e.preventDefault(); + onOpenLink(url); + }); return ( - {d.blog} + + {d.blog} + )} {d.email && ( @@ -140,41 +159,39 @@ function UserCard({ user }: { user: UserData }) { } function GetMeApp() { - const { error, toolResult } = useMcpApp({ + const { error, toolResult, hostContext, openLink } = useMcpApp({ appName: "github-mcp-server-get-me", }); - if (error) { - return Error: {error.message}; - } - - if (!toolResult) { - return ( - - - Loading user data... - - ); - } - - // Parse user data from tool result - const textContent = toolResult.content?.find((c: { type: string }) => c.type === "text"); - if (!textContent || !("text" in textContent)) { - return No user data in response; - } + const content = (() => { + if (error) { + return Error: {error.message}; + } + if (!toolResult) { + return ( + + + Loading user data... + + ); + } + const textContent = toolResult.content?.find((c: { type: string }) => c.type === "text"); + if (!textContent || !("text" in textContent)) { + return No user data in response; + } + try { + const userData = JSON.parse(textContent.text as string) as UserData; + return void openLink(url)} />; + } catch { + return Failed to parse user data; + } + })(); - try { - const userData = JSON.parse(textContent.text as string) as UserData; - return ; - } catch { - return Failed to parse user data; - } + return {content}; } createRoot(document.getElementById("root")!).render( - - - + ); diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index de72b0a78a..863543fc14 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -121,7 +121,7 @@ function CreateIssueApp() { const [error, setError] = useState(null); const [successIssue, setSuccessIssue] = useState(null); - const { app, error: appError, toolInput, callTool } = useMcpApp({ + const { app, error: appError, toolInput, callTool, hostContext, setModelContext } = useMcpApp({ appName: "github-mcp-server-issue-write", }); @@ -181,6 +181,19 @@ function CreateIssueApp() { try { const issueData = JSON.parse(textContent.text as string); setSuccessIssue(issueData); + // Per the MCP Apps 2026-01-26 spec, push the created/updated issue + // into the model's context so subsequent agent turns have it. + void setModelContext({ + structuredContent: issueData, + content: [ + { + type: "text", + text: isUpdateMode + ? `Issue #${issueNumber} in ${owner}/${repo} was updated by the user via the issue-write view.` + : `A new issue was created in ${owner}/${repo} by the user via the issue-write view.`, + }, + ], + }); } catch { setSuccessIssue({ title, body }); } @@ -191,8 +204,9 @@ function CreateIssueApp() { } finally { setIsSubmitting(false); } - }, [title, body, owner, repo, isUpdateMode, issueNumber, callTool]); + }, [title, body, owner, repo, isUpdateMode, issueNumber, callTool, setModelContext]); + const body_node = (() => { if (appError) { return ( @@ -307,12 +321,13 @@ function CreateIssueApp() { ); + })(); + + return {body_node}; } createRoot(document.getElementById("root")!).render( - - - + ); diff --git a/ui/src/apps/pr-write/App.tsx b/ui/src/apps/pr-write/App.tsx index f5ddbdf29d..bfefdbede0 100644 --- a/ui/src/apps/pr-write/App.tsx +++ b/ui/src/apps/pr-write/App.tsx @@ -126,7 +126,7 @@ function CreatePRApp() { const [isDraft, setIsDraft] = useState(false); const [maintainerCanModify, setMaintainerCanModify] = useState(true); - const { app, error: appError, toolInput, callTool } = useMcpApp({ + const { app, error: appError, toolInput, callTool, hostContext, setModelContext } = useMcpApp({ appName: "github-mcp-server-create-pull-request", }); @@ -175,6 +175,17 @@ function CreatePRApp() { if (textContent && textContent.type === "text" && textContent.text) { const prData = JSON.parse(textContent.text); setSuccessPR(prData); + // Push the new PR into the model context so subsequent agent + // turns can reference it (MCP Apps 2026-01-26 ui/update-model-context). + void setModelContext({ + structuredContent: prData, + content: [ + { + type: "text", + text: `A new pull request was created in ${owner}/${repo} by the user via the create-pull-request view.`, + }, + ], + }); } } } catch (e) { @@ -182,11 +193,11 @@ function CreatePRApp() { } finally { setIsSubmitting(false); } - }, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, callTool]); + }, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, callTool, setModelContext]); if (successPR) { return ( - + ); @@ -194,7 +205,7 @@ function CreatePRApp() { if (!app && !appError) { return ( - + @@ -204,14 +215,14 @@ function CreatePRApp() { if (appError) { return ( - + {appError.message} ); } return ( - + { - // Set up theme data attributes for proper Primer theming - const prefersDark = window.matchMedia("(prefers-color-scheme: dark)").matches; - const colorMode = prefersDark ? "dark" : "light"; + // Prefer the host-supplied theme; fall back to the OS preference. + const colorMode = + hostTheme === "light" || hostTheme === "dark" + ? hostTheme + : window.matchMedia("(prefers-color-scheme: dark)").matches + ? "dark" + : "light"; document.body.setAttribute("data-color-mode", colorMode); document.body.setAttribute("data-light-theme", "light"); document.body.setAttribute("data-dark-theme", "dark"); - }, []); + }, [hostTheme]); + + // Project the host's standardized CSS variables onto the root so child + // components can consume them via `var(--color-...)`. We rely on Primer's + // own defaults when the host does not supply variables. + const styleVars = useMemo(() => { + if (!hostVariables) return undefined; + const out: Record = {}; + for (const [key, value] of Object.entries(hostVariables)) { + if (typeof value === "string") out[key] = value; + } + return out as CSSProperties; + }, [hostVariables]); + + const colorMode = + hostTheme === "light" || hostTheme === "dark" ? hostTheme : "auto"; return ( - + - + {children} diff --git a/ui/src/hooks/useMcpApp.ts b/ui/src/hooks/useMcpApp.ts index 54bfa791a7..b060ea6ee2 100644 --- a/ui/src/hooks/useMcpApp.ts +++ b/ui/src/hooks/useMcpApp.ts @@ -1,11 +1,23 @@ import { useApp as useExtApp } from "@modelcontextprotocol/ext-apps/react"; -import type { App } from "@modelcontextprotocol/ext-apps"; +import type { + App, + McpUiDisplayMode, + McpUiHostContext, + McpUiUpdateModelContextRequest, +} from "@modelcontextprotocol/ext-apps"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { useState, useCallback } from "react"; +import { useState, useCallback, useEffect } from "react"; interface UseMcpAppOptions { appName: string; appVersion?: string; + /** + * Display modes this view supports. Per the MCP Apps 2026-01-26 spec, a + * view MUST declare every display mode it supports during initialization. + * Defaults to ["inline"] which is the only mode the bundled github-mcp-server + * views currently render. + */ + availableDisplayModes?: McpUiDisplayMode[]; onToolResult?: (result: CallToolResult) => void; onToolInput?: (input: Record) => void; } @@ -15,21 +27,38 @@ interface UseMcpAppReturn { error: Error | null; toolResult: CallToolResult | null; toolInput: Record | null; + hostContext: McpUiHostContext | undefined; callTool: (name: string, args: Record) => Promise; + /** + * Sends `ui/update-model-context` so the agent's next turn sees the + * supplied structured content / blocks. No-op when the app isn't connected. + */ + setModelContext: ( + params: McpUiUpdateModelContextRequest["params"] + ) => Promise; + /** + * Sends `ui/open-link` so the host opens an external URL in the user's + * browser. Falls back to `window.open` when the app isn't connected. + */ + openLink: (url: string) => Promise; } export function useMcpApp({ appName, appVersion = "1.0.0", + availableDisplayModes = ["inline"], onToolResult, onToolInput, }: UseMcpAppOptions): UseMcpAppReturn { const [toolResult, setToolResult] = useState(null); const [toolInput, setToolInput] = useState | null>(null); + const [hostContext, setHostContext] = useState(undefined); + // The SDK's autoResize=true installs a ResizeObserver that emits + // `ui/notifications/size-changed` automatically; no manual wiring needed. const { app, error } = useExtApp({ appInfo: { name: appName, version: appVersion }, - capabilities: {}, + capabilities: { availableDisplayModes }, autoResize: true, strict: import.meta.env.DEV, onAppCreated: (app) => { @@ -42,10 +71,19 @@ export function useMcpApp({ setToolInput(args); onToolInput?.(args); }; + app.onhostcontextchanged = (params) => { + setHostContext((prev) => ({ ...(prev ?? {}), ...params })); + }; app.onerror = console.error; }, }); + useEffect(() => { + if (!app) return; + const initial = app.getHostContext(); + if (initial) setHostContext(initial); + }, [app]); + const callTool = useCallback( async (name: string, args: Record) => { if (!app) throw new Error("App not connected"); @@ -54,5 +92,33 @@ export function useMcpApp({ [app] ); - return { app, error, toolResult, toolInput, callTool }; + const setModelContext = useCallback( + async (params) => { + if (!app) return; + await app.updateModelContext(params); + }, + [app] + ); + + const openLink = useCallback( + async (url) => { + if (!app) { + window.open(url, "_blank", "noopener,noreferrer"); + return; + } + await app.openLink({ url }); + }, + [app] + ); + + return { + app, + error, + toolResult, + toolInput, + hostContext, + callTool, + setModelContext, + openLink, + }; } From 5d47ccc32fbf5358e45539fb4b29791fbc1ef535 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 17 Mar 2026 14:47:57 +0100 Subject: [PATCH 04/21] feat: add create_project and create_iteration_field methods to projects_write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new methods to the consolidated projects_write tool: - create_project: creates a new GitHub ProjectsV2 for a user or org - create_iteration_field: adds an iteration field to an existing project Changes addressing review feedback: - Validate owner_type is exactly 'user' or 'org' in create_project - Use resolveProjectNodeID (GraphQL) instead of getProjectNodeID (REST) to avoid HTTP response body leaks - Add omitempty to Iterations JSON tag - Rename iterations item field startDate to start_date for consistency - Validate iteration elements instead of silently skipping invalid ones - Use explicit response structs with snake_case JSON tags - Add test for auto-detected owner_type in create_iteration_field - Use stubExporters() in test deps for nil-safety Co-authored-by: João Doria de Souza Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 12 +- pkg/github/__toolsnaps__/projects_write.snap | 56 ++- pkg/github/projects.go | 323 ++++++++++++- pkg/github/projects_test.go | 2 +- pkg/github/projects_v2_test.go | 457 +++++++++++++++++++ pkg/github/toolset_instructions.go | 4 + 6 files changed, 832 insertions(+), 22 deletions(-) create mode 100644 pkg/github/projects_v2_test.go diff --git a/README.md b/README.md index 9082a3b642..0cb16df768 100644 --- a/README.md +++ b/README.md @@ -1028,22 +1028,26 @@ The following sets of tools are available: - `project_number`: The project's number. Required for 'list_project_fields', 'list_project_items', and 'list_project_status_updates' methods. (number, optional) - `query`: Filter/query string. For list_projects: filter by title text and state (e.g. "roadmap is:open"). For list_project_items: advanced filtering using GitHub's project filtering syntax. (string, optional) -- **projects_write** - Modify GitHub Project items +- **projects_write** - Manage GitHub Projects - **Required OAuth Scopes**: `project` - `body`: The body of the status update (markdown). Used for 'create_project_status_update' method. (string, optional) + - `field_name`: The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method. (string, optional) - `issue_number`: The issue number (use when item_type is 'issue' for 'add_project_item' method). Provide either issue_number or pull_request_number. (number, optional) - `item_id`: The project item ID. Required for 'update_project_item' and 'delete_project_item' methods. (number, optional) - `item_owner`: The owner (user or organization) of the repository containing the issue or pull request. Required for 'add_project_item' method. (string, optional) - `item_repo`: The name of the repository containing the issue or pull request. Required for 'add_project_item' method. (string, optional) - `item_type`: The item's type, either issue or pull_request. Required for 'add_project_item' method. (string, optional) + - `iteration_duration`: Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method. (number, optional) + - `iterations`: Custom iterations for 'create_iteration_field' method. Only set this when you need iterations with varying durations, breaks between them, or specific titles. Otherwise omit it: GitHub auto-creates three iterations of 'iteration_duration' days starting on 'start_date', which is the right choice for most cases. (object[], optional) - `method`: The method to execute (string, required) - `owner`: The project owner (user or organization login). The name is not case sensitive. (string, required) - - `owner_type`: Owner type (user or org). If not provided, will be automatically detected. (string, optional) - - `project_number`: The project's number. (number, required) + - `owner_type`: Owner type (user or org). Required for 'create_project' method. If not provided for other methods, will be automatically detected. (string, optional) + - `project_number`: The project's number. Required for all methods except 'create_project'. (number, optional) - `pull_request_number`: The pull request number (use when item_type is 'pull_request' for 'add_project_item' method). Provide either issue_number or pull_request_number. (number, optional) - - `start_date`: The start date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method. (string, optional) + - `start_date`: Start date in YYYY-MM-DD format. Used for 'create_project_status_update' and 'create_iteration_field' methods. (string, optional) - `status`: The status of the project. Used for 'create_project_status_update' method. (string, optional) - `target_date`: The target date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method. (string, optional) + - `title`: The project title. Required for 'create_project' method. (string, optional) - `updated_field`: Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {"id": 123456, "value": "New Value"}. Required for 'update_project_item' method. (object, optional) diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index f6d3197b84..6c9d349f63 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -1,15 +1,19 @@ { "annotations": { "destructiveHint": true, - "title": "Modify GitHub Project items" + "title": "Manage GitHub Projects" }, - "description": "Add, update, or delete project items, or create status updates in a GitHub Project.", + "description": "Create and manage GitHub Projects: create projects, add/update/delete items, create status updates, and add iteration fields.", "inputSchema": { "properties": { "body": { "description": "The body of the status update (markdown). Used for 'create_project_status_update' method.", "type": "string" }, + "field_name": { + "description": "The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method.", + "type": "string" + }, "issue_number": { "description": "The issue number (use when item_type is 'issue' for 'add_project_item' method). Provide either issue_number or pull_request_number.", "type": "number" @@ -34,13 +38,46 @@ ], "type": "string" }, + "iteration_duration": { + "description": "Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method.", + "type": "number" + }, + "iterations": { + "description": "Custom iterations for 'create_iteration_field' method. Only set this when you need iterations with varying durations, breaks between them, or specific titles. Otherwise omit it: GitHub auto-creates three iterations of 'iteration_duration' days starting on 'start_date', which is the right choice for most cases.", + "items": { + "additionalProperties": false, + "properties": { + "duration": { + "description": "Duration in days", + "type": "number" + }, + "start_date": { + "description": "Start date in YYYY-MM-DD format", + "type": "string" + }, + "title": { + "description": "Iteration title (e.g. 'Sprint 1')", + "type": "string" + } + }, + "required": [ + "title", + "start_date", + "duration" + ], + "type": "object" + }, + "type": "array" + }, "method": { "description": "The method to execute", "enum": [ "add_project_item", "update_project_item", "delete_project_item", - "create_project_status_update" + "create_project_status_update", + "create_project", + "create_iteration_field" ], "type": "string" }, @@ -49,7 +86,7 @@ "type": "string" }, "owner_type": { - "description": "Owner type (user or org). If not provided, will be automatically detected.", + "description": "Owner type (user or org). Required for 'create_project' method. If not provided for other methods, will be automatically detected.", "enum": [ "user", "org" @@ -57,7 +94,7 @@ "type": "string" }, "project_number": { - "description": "The project's number.", + "description": "The project's number. Required for all methods except 'create_project'.", "type": "number" }, "pull_request_number": { @@ -65,7 +102,7 @@ "type": "number" }, "start_date": { - "description": "The start date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method.", + "description": "Start date in YYYY-MM-DD format. Used for 'create_project_status_update' and 'create_iteration_field' methods.", "type": "string" }, "status": { @@ -83,6 +120,10 @@ "description": "The target date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method.", "type": "string" }, + "title": { + "description": "The project title. Required for 'create_project' method.", + "type": "string" + }, "updated_field": { "description": "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {\"id\": 123456, \"value\": \"New Value\"}. Required for 'update_project_item' method.", "type": "object" @@ -90,8 +131,7 @@ }, "required": [ "method", - "owner", - "project_number" + "owner" ], "type": "object" }, diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 7c383c1111..9c7310c0ff 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -45,6 +45,8 @@ const ( projectsMethodListProjectStatusUpdates = "list_project_status_updates" projectsMethodGetProjectStatusUpdate = "get_project_status_update" projectsMethodCreateProjectStatusUpdate = "create_project_status_update" + projectsMethodCreateProject = "create_project" + projectsMethodCreateIterationField = "create_iteration_field" ) // GraphQL types for ProjectV2 status updates @@ -403,9 +405,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { ToolsetMetadataProjects, mcp.Tool{ Name: "projects_write", - Description: t("TOOL_PROJECTS_WRITE_DESCRIPTION", "Add, update, or delete project items, or create status updates in a GitHub Project."), + Description: t("TOOL_PROJECTS_WRITE_DESCRIPTION", "Create and manage GitHub Projects: create projects, add/update/delete items, create status updates, and add iteration fields."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_PROJECTS_WRITE_USER_TITLE", "Modify GitHub Project items"), + Title: t("TOOL_PROJECTS_WRITE_USER_TITLE", "Manage GitHub Projects"), ReadOnlyHint: false, DestructiveHint: jsonschema.Ptr(true), }, @@ -420,11 +422,13 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { projectsMethodUpdateProjectItem, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, + projectsMethodCreateProject, + projectsMethodCreateIterationField, }, }, "owner_type": { Type: "string", - Description: "Owner type (user or org). If not provided, will be automatically detected.", + Description: "Owner type (user or org). Required for 'create_project' method. If not provided for other methods, will be automatically detected.", Enum: []any{"user", "org"}, }, "owner": { @@ -433,7 +437,11 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "project_number": { Type: "number", - Description: "The project's number.", + Description: "The project's number. Required for all methods except 'create_project'.", + }, + "title": { + Type: "string", + Description: "The project title. Required for 'create_project' method.", }, "item_id": { Type: "number", @@ -475,14 +483,45 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "start_date": { Type: "string", - Description: "The start date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method.", + Description: "Start date in YYYY-MM-DD format. Used for 'create_project_status_update' and 'create_iteration_field' methods.", }, "target_date": { Type: "string", Description: "The target date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method.", }, + "field_name": { + Type: "string", + Description: "The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method.", + }, + "iteration_duration": { + Type: "number", + Description: "Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method.", + }, + "iterations": { + Type: "array", + Description: "Custom iterations for 'create_iteration_field' method. Only set this when you need iterations with varying durations, breaks between them, or specific titles. Otherwise omit it: GitHub auto-creates three iterations of 'iteration_duration' days starting on 'start_date', which is the right choice for most cases.", + Items: &jsonschema.Schema{ + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, + Properties: map[string]*jsonschema.Schema{ + "title": { + Type: "string", + Description: "Iteration title (e.g. 'Sprint 1')", + }, + "start_date": { + Type: "string", + Description: "Start date in YYYY-MM-DD format", + }, + "duration": { + Type: "number", + Description: "Duration in days", + }, + }, + Required: []string{"title", "start_date", "duration"}, + }, + }, }, - Required: []string{"method", "owner", "project_number"}, + Required: []string{"method", "owner"}, }, }, []scopes.Scope{scopes.Project}, @@ -502,17 +541,22 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(err.Error()), nil, nil } - projectNumber, err := RequiredInt(args, "project_number") + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - client, err := deps.GetClient(ctx) + // create_project does not require project_number or a REST client + if method == projectsMethodCreateProject { + return createProject(ctx, gqlClient, owner, ownerType, args) + } + + projectNumber, err := RequiredInt(args, "project_number") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - gqlClient, err := deps.GetGQLClient(ctx) + client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -595,6 +639,8 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(err.Error()), nil, nil } return createProjectStatusUpdate(ctx, gqlClient, owner, ownerType, projectNumber, body, status, startDate, targetDate) + case projectsMethodCreateIterationField: + return createIterationField(ctx, gqlClient, owner, ownerType, projectNumber, args) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1438,6 +1484,265 @@ func resolvePullRequestNodeID(ctx context.Context, gqlClient *githubv4.Client, o return query.Repository.PullRequest.ID, nil } +// createProject handles the create_project method for ProjectsWrite. +func createProject(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, args map[string]any) (*mcp.CallToolResult, any, error) { + if ownerType == "" { + return utils.NewToolResultError("owner_type is required for create_project"), nil, nil + } + if ownerType != "user" && ownerType != "org" { + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil + } + + title, err := RequiredParam[string](args, "title") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + ownerID, err := getOwnerNodeID(ctx, gqlClient, owner, ownerType) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to get owner ID: %v", err)), nil, nil + } + + var mutation struct { + CreateProjectV2 struct { + ProjectV2 struct { + ID string + Number int + Title string + URL string + } + } `graphql:"createProjectV2(input: $input)"` + } + + input := githubv4.CreateProjectV2Input{ + OwnerID: githubv4.ID(ownerID), + Title: githubv4.String(title), + } + + err = gqlClient.Mutate(ctx, &mutation, input, nil) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to create project: %v", err)), nil, nil + } + + result := struct { + ID string `json:"id"` + Number int `json:"number"` + Title string `json:"title"` + URL string `json:"url"` + }{ + ID: mutation.CreateProjectV2.ProjectV2.ID, + Number: mutation.CreateProjectV2.ProjectV2.Number, + Title: mutation.CreateProjectV2.ProjectV2.Title, + URL: mutation.CreateProjectV2.ProjectV2.URL, + } + + return MarshalledTextResult(result), nil, nil +} + +// createIterationField handles the create_iteration_field method for ProjectsWrite. +// +// GitHub's GraphQL API requires two mutations to fully configure an iteration field: +// 1. createProjectV2Field creates the field with DataType=ITERATION (no schedule yet). +// 2. updateProjectV2Field sets the start date, duration, and optional named iterations. +// +// If step 2 fails, the field already exists with default settings and can be reconfigured +// by calling this method again (the create will fail with a duplicate-name error, which +// surfaces clearly) or by deleting the field via the GitHub UI. +func createIterationField(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, args map[string]any) (*mcp.CallToolResult, any, error) { + fieldName, err := RequiredParam[string](args, "field_name") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + duration, err := RequiredInt(args, "iteration_duration") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + startDateStr, err := RequiredParam[string](args, "start_date") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + projectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to get project ID: %v", err)), nil, nil + } + + // Step 1: Create the iteration field. + var createMutation struct { + CreateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"createProjectV2Field(input: $input)"` + } + + createInput := githubv4.CreateProjectV2FieldInput{ + ProjectID: githubv4.ID(projectID), + DataType: githubv4.ProjectV2CustomFieldType("ITERATION"), + Name: githubv4.String(fieldName), + } + + err = gqlClient.Mutate(ctx, &createMutation, createInput, nil) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to create iteration field: %v", err)), nil, nil + } + + fieldID := createMutation.CreateProjectV2Field.ProjectV2Field.ProjectV2IterationField.ID + + // Step 2: Configure the iteration field with start date and duration. + var updateMutation struct { + UpdateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + Configuration struct { + Iterations []struct { + ID string + Title string + StartDate string + Duration int + } + } + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"updateProjectV2Field(input: $input)"` + } + + parsedStartDate, err := time.Parse("2006-01-02", startDateStr) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to parse start_date %s: %v", startDateStr, err)), nil, nil + } + + // GitHub's ProjectV2IterationFieldConfigurationInput requires `iterations` as a + // non-null array, so we always send at least an empty slice. When omitted, GitHub + // generates a default set of iterations from start_date and duration. + iterationsInput := []ProjectV2IterationFieldIterationInput{} + + if rawIterations, ok := args["iterations"].([]any); ok && len(rawIterations) > 0 { + for i, item := range rawIterations { + iterMap, ok := item.(map[string]any) + if !ok { + return utils.NewToolResultError(fmt.Sprintf("iterations[%d] must be an object", i)), nil, nil + } + iterTitle, ok := iterMap["title"].(string) + if !ok || iterTitle == "" { + return utils.NewToolResultError(fmt.Sprintf("iterations[%d]: title is required and must be a non-empty string", i)), nil, nil + } + iterStartDate, ok := iterMap["start_date"].(string) + if !ok || iterStartDate == "" { + return utils.NewToolResultError(fmt.Sprintf("iterations[%d]: start_date is required and must be a non-empty string", i)), nil, nil + } + iterDuration, ok := iterMap["duration"].(float64) + if !ok || iterDuration <= 0 { + return utils.NewToolResultError(fmt.Sprintf("iterations[%d]: duration is required and must be a positive number", i)), nil, nil + } + + parsedIterStartDate, err := time.Parse("2006-01-02", iterStartDate) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("iterations[%d]: failed to parse start_date %q: %v", i, iterStartDate, err)), nil, nil + } + + iterationsInput = append(iterationsInput, ProjectV2IterationFieldIterationInput{ + Title: githubv4.String(iterTitle), + StartDate: githubv4.Date{Time: parsedIterStartDate}, + Duration: githubv4.Int(int32(iterDuration)), //nolint:gosec // Iteration durations are small day counts + }) + } + } + + configInput := ProjectV2IterationFieldConfigurationInput{ + Duration: githubv4.Int(int32(duration)), //nolint:gosec // Iteration durations are small day counts + StartDate: githubv4.Date{Time: parsedStartDate}, + Iterations: iterationsInput, + } + + updateInput := UpdateProjectV2FieldInput{ + FieldID: githubv4.ID(fieldID), + IterationConfiguration: &configInput, + } + + err = gqlClient.Mutate(ctx, &updateMutation, updateInput, nil) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to update iteration configuration: %v", err)), nil, nil + } + + field := updateMutation.UpdateProjectV2Field.ProjectV2Field.ProjectV2IterationField + iterResults := make([]map[string]any, 0, len(field.Configuration.Iterations)) + for _, iter := range field.Configuration.Iterations { + iterResults = append(iterResults, map[string]any{ + "id": iter.ID, + "title": iter.Title, + "start_date": iter.StartDate, + "duration": iter.Duration, + }) + } + + result := map[string]any{ + "id": field.ID, + "name": field.Name, + "configuration": map[string]any{ + "iterations": iterResults, + }, + } + + return MarshalledTextResult(result), nil, nil +} + +// getOwnerNodeID resolves a GitHub user or organization login to its GraphQL node ID. +func getOwnerNodeID(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string) (string, error) { + if ownerType == "org" { + var query struct { + Organization struct { + ID string + } `graphql:"organization(login: $login)"` + } + variables := map[string]any{ + "login": githubv4.String(owner), + } + err := gqlClient.Query(ctx, &query, variables) + return query.Organization.ID, err + } + + var query struct { + User struct { + ID string + } `graphql:"user(login: $login)"` + } + variables := map[string]any{ + "login": githubv4.String(owner), + } + err := gqlClient.Query(ctx, &query, variables) + return query.User.ID, err +} + +// UpdateProjectV2FieldInput is the GraphQL input for the updateProjectV2Field mutation. +// These types are defined locally because the pinned shurcooL/githubv4 release +// (v0.0.0-20240727222349) does not yet expose them. Upstream master now generates +// equivalent types, so this block can be removed when the dependency is next bumped. +type UpdateProjectV2FieldInput struct { + FieldID githubv4.ID `json:"fieldId"` + IterationConfiguration *ProjectV2IterationFieldConfigurationInput `json:"iterationConfiguration,omitempty"` +} + +// ProjectV2IterationFieldConfigurationInput is the GraphQL input for configuring an iteration field. +// GitHub's schema marks iterations as a required non-null list, so the field is not omitempty. +type ProjectV2IterationFieldConfigurationInput struct { + Duration githubv4.Int `json:"duration"` + StartDate githubv4.Date `json:"startDate"` + Iterations []ProjectV2IterationFieldIterationInput `json:"iterations"` +} + +// ProjectV2IterationFieldIterationInput is the GraphQL input for a single iteration definition. +type ProjectV2IterationFieldIterationInput struct { + StartDate githubv4.Date `json:"startDate"` + Duration githubv4.Int `json:"duration"` + Title githubv4.String `json:"title"` +} + // detectOwnerType attempts to detect the owner type by trying both user and org // Returns the detected type ("user" or "org") and any error encountered func detectOwnerType(ctx context.Context, client *github.Client, owner string, projectNumber int) (string, error) { diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 306c74b41e..a9787298af 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -569,7 +569,7 @@ func Test_ProjectsWrite(t *testing.T) { assert.Contains(t, inputSchema.Properties, "issue_number") assert.Contains(t, inputSchema.Properties, "pull_request_number") assert.Contains(t, inputSchema.Properties, "updated_field") - assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner", "project_number"}) + assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner"}) // Verify DestructiveHint is set assert.NotNil(t, toolDef.Tool.Annotations) diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go new file mode 100644 index 0000000000..69d4d6395f --- /dev/null +++ b/pkg/github/projects_v2_test.go @@ -0,0 +1,457 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ProjectsWrite_CreateProject(t *testing.T) { + t.Parallel() + + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("success user project", func(t *testing.T) { + t.Parallel() + + mockedClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + User struct { + ID string + } `graphql:"user(login: $login)"` + }{}, + map[string]any{ + "login": githubv4.String("octocat"), + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "id": "U_octocat", + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + CreateProjectV2 struct { + ProjectV2 struct { + ID string + Number int + Title string + URL string + } + } `graphql:"createProjectV2(input: $input)"` + }{}, + githubv4.CreateProjectV2Input{ + OwnerID: githubv4.ID("U_octocat"), + Title: githubv4.String("New Project"), + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project123", + "number": 1, + "title": "New Project", + "url": "https://github.com/users/octocat/projects/1", + }, + }, + }), + ), + ) + + deps := BaseDeps{ + GQLClient: githubv4.NewClient(mockedClient), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project", + "owner": "octocat", + "owner_type": "user", + "title": "New Project", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, "PVT_project123", response["id"]) + assert.Equal(t, float64(1), response["number"]) + assert.Equal(t, "New Project", response["title"]) + assert.Equal(t, "https://github.com/users/octocat/projects/1", response["url"]) + }) + + t.Run("missing owner_type returns error", func(t *testing.T) { + t.Parallel() + + deps := BaseDeps{ + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project", + "owner": "octocat", + "title": "New Project", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.True(t, result.IsError) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "owner_type is required") + }) + + t.Run("invalid owner_type returns error", func(t *testing.T) { + t.Parallel() + + deps := BaseDeps{ + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project", + "owner": "octocat", + "owner_type": "invalid", + "title": "New Project", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.True(t, result.IsError) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "invalid owner_type") + assert.Contains(t, textContent.Text, "must be") + }) +} + +// resolveProjectNodeIDOrgMatcher returns a GraphQL query matcher for resolving +// an org project node ID via resolveProjectNodeID. +func resolveProjectNodeIDOrgMatcher(owner string, projectNumber int, nodeID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Organization struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"organization(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String(owner), + "projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec // test constant + }, + githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "projectV2": map[string]any{ + "id": nodeID, + }, + }, + }), + ) +} + +func createFieldMatcher() githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + CreateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"createProjectV2Field(input: $input)"` + }{}, + githubv4.CreateProjectV2FieldInput{ + ProjectID: githubv4.ID("PVT_project1"), + DataType: githubv4.ProjectV2CustomFieldType("ITERATION"), + Name: githubv4.String("Sprint"), + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2Field": map[string]any{ + "projectV2Field": map[string]any{ + "id": "PVTIF_field1", + "name": "Sprint", + }, + }, + }), + ) +} + +func updateFieldIterationResponse() githubv4mock.GQLResponse { + return githubv4mock.DataResponse(map[string]any{ + "updateProjectV2Field": map[string]any{ + "projectV2Field": map[string]any{ + "id": "PVTIF_field1", + "name": "Sprint", + "configuration": map[string]any{ + "iterations": []any{ + map[string]any{ + "id": "PVTI_iter1", + "title": "Sprint 1", + "startDate": "2025-01-20", + "duration": 7, + }, + }, + }, + }, + }, + }) +} + +func Test_ProjectsWrite_CreateIterationField(t *testing.T) { + t.Parallel() + + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("success with iterations", func(t *testing.T) { + t.Parallel() + + mockGQLClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 1, "PVT_project1"), + createFieldMatcher(), + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + Configuration struct { + Iterations []struct { + ID string + Title string + StartDate string + Duration int + } + } + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"updateProjectV2Field(input: $input)"` + }{}, + UpdateProjectV2FieldInput{ + FieldID: githubv4.ID("PVTIF_field1"), + IterationConfiguration: &ProjectV2IterationFieldConfigurationInput{ + Duration: githubv4.Int(7), + StartDate: githubv4.Date{Time: time.Date(2025, 1, 20, 0, 0, 0, 0, time.UTC)}, + Iterations: []ProjectV2IterationFieldIterationInput{ + { + Title: githubv4.String("Sprint 1"), + StartDate: githubv4.Date{Time: time.Date(2025, 1, 20, 0, 0, 0, 0, time.UTC)}, + Duration: githubv4.Int(7), + }, + }, + }, + }, + nil, + updateFieldIterationResponse(), + ), + ) + + deps := BaseDeps{ + GQLClient: githubv4.NewClient(mockGQLClient), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_iteration_field", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "field_name": "Sprint", + "iteration_duration": float64(7), + "start_date": "2025-01-20", + "iterations": []any{ + map[string]any{ + "title": "Sprint 1", + "start_date": "2025-01-20", + "duration": float64(7), + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, "PVTIF_field1", response["id"]) + }) + + t.Run("success without iterations", func(t *testing.T) { + t.Parallel() + + mockGQLClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 1, "PVT_project1"), + createFieldMatcher(), + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + Configuration struct { + Iterations []struct { + ID string + Title string + StartDate string + Duration int + } + } + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"updateProjectV2Field(input: $input)"` + }{}, + UpdateProjectV2FieldInput{ + FieldID: githubv4.ID("PVTIF_field1"), + IterationConfiguration: &ProjectV2IterationFieldConfigurationInput{ + Duration: githubv4.Int(7), + StartDate: githubv4.Date{Time: time.Date(2025, 1, 20, 0, 0, 0, 0, time.UTC)}, + Iterations: []ProjectV2IterationFieldIterationInput{}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2Field": map[string]any{ + "projectV2Field": map[string]any{ + "id": "PVTIF_field1", + "name": "Sprint", + "configuration": map[string]any{ + "iterations": []any{}, + }, + }, + }, + }), + ), + ) + + deps := BaseDeps{ + GQLClient: githubv4.NewClient(mockGQLClient), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_iteration_field", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "field_name": "Sprint", + "iteration_duration": float64(7), + "start_date": "2025-01-20", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, "PVTIF_field1", response["id"]) + }) + + t.Run("success with auto-detected owner_type", func(t *testing.T) { + t.Parallel() + + // detectOwnerType uses REST to probe user first, then org + mockRESTClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUsersProjectsV2ByUsernameByProject: mockResponse(t, http.StatusNotFound, nil), + GetOrgsProjectsV2ByProject: mockResponse(t, http.StatusOK, map[string]any{ + "id": 1, + "node_id": "PVT_project1", + "title": "Org Project", + }), + }) + + mockGQLClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 1, "PVT_project1"), + createFieldMatcher(), + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2Field struct { + ProjectV2Field struct { + ProjectV2IterationField struct { + ID string + Name string + Configuration struct { + Iterations []struct { + ID string + Title string + StartDate string + Duration int + } + } + } `graphql:"... on ProjectV2IterationField"` + } + } `graphql:"updateProjectV2Field(input: $input)"` + }{}, + UpdateProjectV2FieldInput{ + FieldID: githubv4.ID("PVTIF_field1"), + IterationConfiguration: &ProjectV2IterationFieldConfigurationInput{ + Duration: githubv4.Int(14), + StartDate: githubv4.Date{Time: time.Date(2025, 2, 1, 0, 0, 0, 0, time.UTC)}, + Iterations: []ProjectV2IterationFieldIterationInput{}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2Field": map[string]any{ + "projectV2Field": map[string]any{ + "id": "PVTIF_field1", + "name": "Sprint", + "configuration": map[string]any{ + "iterations": []any{}, + }, + }, + }, + }), + ), + ) + + deps := BaseDeps{ + Client: mustNewGHClient(t, mockRESTClient), + GQLClient: githubv4.NewClient(mockGQLClient), + Obsv: stubExporters(), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_iteration_field", + "owner": "octo-org", + "project_number": float64(1), + "field_name": "Sprint", + "iteration_duration": float64(14), + "start_date": "2025-02-01", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, "PVTIF_field1", response["id"]) + }) +} diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index bc9da4e65c..ba6659612a 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -39,6 +39,10 @@ func generateProjectsToolsetInstructions(_ *inventory.Inventory) string { Workflow: 1) list_project_fields (get field IDs), 2) list_project_items (with pagination), 3) optional updates. +Project lifecycle: Use create_project to create a new ProjectsV2 for a user or organization (requires owner_type and title). Returns the new project's id, number, title, and url; pass the returned number as project_number to subsequent project tools. + +Iteration fields: Use create_iteration_field to add a new ITERATION field (e.g. "Sprint") to an existing project. Required: field_name, iteration_duration (days), start_date (YYYY-MM-DD). Only pass the iterations array when iterations need varying durations, breaks between them, or specific titles; otherwise omit it and GitHub creates three default iterations of iteration_duration days starting on start_date. + Status updates: Use list_project_status_updates to read recent project status updates (newest first). Use get_project_status_update with a node ID to get a single update. Use create_project_status_update to create a new status update for a project. Field usage: From 3a4c660033187435c838a46147d9346a35ac5952 Mon Sep 17 00:00:00 2001 From: Omid Mogasemi Date: Fri, 20 Mar 2026 19:02:46 +0000 Subject: [PATCH 05/21] fix: restore thread id in get_review_comments response --- pkg/github/minimal_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index ff4149a225..a93d29ead5 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -1547,7 +1547,7 @@ type MinimalReviewComment struct { // MinimalReviewThread is the trimmed output type for PR review thread objects. type MinimalReviewThread struct { - ID string + ID string `json:"id"` IsResolved bool `json:"is_resolved"` IsOutdated bool `json:"is_outdated"` IsCollapsed bool `json:"is_collapsed"` From 29a86780bf104bf91b5cd3ddfa2579564a68eab6 Mon Sep 17 00:00:00 2001 From: John CSA <103165870+jluocsa@users.noreply.github.com> Date: Sat, 23 May 2026 11:43:02 -0700 Subject: [PATCH 06/21] docs: add installation guides for Zed and OpenCode Adds two new installation guides under docs/installation-guides/ covering MCP host applications that are not yet documented: - install-zed.md: covers Zed's 'context_servers' settings key (command + args shape), the official GitHub MCP extension as an easier alternative, remote/local setup, the 'mcp::' permission key format introduced in Zed v0.224.0, and OAuth-vs-PAT trade-offs. - install-opencode.md: covers OpenCode's 'mcp' config block (type-discriminated local/remote, command-as-array, 'environment' instead of 'env'), the 'oauth: false' opt-out needed when using a PAT, the '{env:VAR}' interpolation pattern, and the per-agent tool-gating pattern recommended for token-heavy servers like GitHub. Also adds both hosts to: - docs/installation-guides/README.md installation-guides index and the support-by-host-application table. - README.md 'Install in other MCP hosts' and 'Install in Other MCP Hosts' lists. Closes #2531. --- README.md | 4 + docs/installation-guides/README.md | 4 + docs/installation-guides/install-opencode.md | 154 +++++++++++++++++++ docs/installation-guides/install-zed.md | 103 +++++++++++++ 4 files changed, 265 insertions(+) create mode 100644 docs/installation-guides/install-opencode.md create mode 100644 docs/installation-guides/install-zed.md diff --git a/README.md b/README.md index 0cb16df768..495eb98992 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,9 @@ Alternatively, to manually configure VS Code, choose the appropriate JSON block - **[Claude Applications](/docs/installation-guides/install-claude.md)** - Installation guide for Claude Desktop and Claude Code CLI - **[Codex](/docs/installation-guides/install-codex.md)** - Installation guide for OpenAI Codex - **[Cursor](/docs/installation-guides/install-cursor.md)** - Installation guide for Cursor IDE +- **[OpenCode](/docs/installation-guides/install-opencode.md)** - Installation guide for the OpenCode terminal agent - **[Windsurf](/docs/installation-guides/install-windsurf.md)** - Installation guide for Windsurf IDE +- **[Zed](/docs/installation-guides/install-zed.md)** - Installation guide for Zed editor - **[Rovo Dev CLI](/docs/installation-guides/install-rovo-dev-cli.md)** - Installation guide for Rovo Dev CLI > **Note:** Each MCP host application needs to configure a GitHub App or OAuth App to support remote access via OAuth. Any host application that supports remote MCP servers should support the remote GitHub server with PAT authentication. Configuration details and support levels vary by host. Make sure to refer to the host application's documentation for more info. @@ -356,7 +358,9 @@ For other MCP host applications, please refer to our installation guides: - **[Claude Code & Claude Desktop](docs/installation-guides/install-claude.md)** - Installation guide for Claude Code and Claude Desktop - **[Cursor](docs/installation-guides/install-cursor.md)** - Installation guide for Cursor IDE - **[Google Gemini CLI](docs/installation-guides/install-gemini-cli.md)** - Installation guide for Google Gemini CLI +- **[OpenCode](docs/installation-guides/install-opencode.md)** - Installation guide for the OpenCode terminal agent - **[Windsurf](docs/installation-guides/install-windsurf.md)** - Installation guide for Windsurf IDE +- **[Zed](docs/installation-guides/install-zed.md)** - Installation guide for Zed editor For a complete overview of all installation options, see our **[Installation Guides Index](docs/installation-guides)**. diff --git a/docs/installation-guides/README.md b/docs/installation-guides/README.md index 0c0f7840ef..61ea7eafb1 100644 --- a/docs/installation-guides/README.md +++ b/docs/installation-guides/README.md @@ -11,9 +11,11 @@ This directory contains detailed installation instructions for the GitHub MCP Se - **[Cursor](install-cursor.md)** - Installation guide for Cursor IDE - **[Google Gemini CLI](install-gemini-cli.md)** - Installation guide for Google Gemini CLI - **[OpenAI Codex](install-codex.md)** - Installation guide for OpenAI Codex +- **[OpenCode](install-opencode.md)** - Installation guide for the OpenCode terminal agent - **[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 +- **[Zed](install-zed.md)** - Installation guide for Zed editor ## Support by Host Application @@ -29,8 +31,10 @@ This directory contains detailed installation instructions for the GitHub MCP Se | Cline | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Cursor | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Google Gemini CLI | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | +| OpenCode | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Roo Code | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | | Windsurf | ✅ | ✅ PAT + ❌ No OAuth | Docker or Go build, GitHub PAT | Easy | +| Zed | ✅ | ✅ 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 via `GITHUB_PAT_TOKEN` env var (`bearer_token_env_var`) | Easy | diff --git a/docs/installation-guides/install-opencode.md b/docs/installation-guides/install-opencode.md new file mode 100644 index 0000000000..10e0e2db2a --- /dev/null +++ b/docs/installation-guides/install-opencode.md @@ -0,0 +1,154 @@ +# Install GitHub MCP Server in OpenCode + +[OpenCode](https://opencode.ai) is a terminal-based AI coding agent that exposes MCP servers under the `mcp` key in `opencode.json` (or `opencode.jsonc`). For general setup information (prerequisites, Docker installation, security best practices), see the [Installation Guides README](./README.md). + +## Prerequisites + +1. OpenCode installed (`brew install sst/tap/opencode` or see [OpenCode install docs](https://opencode.ai/docs/)) +2. [GitHub Personal Access Token](https://github.com/settings/personal-access-tokens/new) with appropriate scopes +3. For local installation: [Docker](https://www.docker.com/) installed and running + +> [!IMPORTANT] +> The OpenCode docs note that the GitHub MCP server can add a lot of tokens to your context. Consider limiting toolsets — for example, by setting `X-MCP-Toolsets` on the remote server or `--toolsets` on the local server — to keep prompts within your model's context window. See the [Server Configuration Guide](../server-configuration.md) and the [main README's toolsets section](../../README.md#available-toolsets). + +## Remote Server (Recommended) + +Uses GitHub's hosted server at `https://api.githubcopilot.com/mcp/`. Edit your [OpenCode config](https://opencode.ai/docs/config/) (typically `~/.config/opencode/opencode.json`, or `opencode.json` in your project root) and add the following under `mcp`: + +```json +{ + "$schema": "https://opencode.ai/config.json", + "mcp": { + "github": { + "type": "remote", + "url": "https://api.githubcopilot.com/mcp/", + "enabled": true, + "oauth": false, + "headers": { + "Authorization": "Bearer YOUR_GITHUB_PAT" + } + } + } +} +``` + +Replace `YOUR_GITHUB_PAT` with your [GitHub Personal Access Token](https://github.com/settings/tokens). The `oauth: false` setting disables OpenCode's automatic OAuth discovery and tells it to use the PAT in `Authorization` instead — without this, OpenCode may try the OAuth flow first. + +### Using an environment variable for the PAT + +OpenCode supports environment-variable interpolation in config values via `{env:VAR_NAME}`. To avoid putting your PAT directly in `opencode.json`: + +```json +{ + "$schema": "https://opencode.ai/config.json", + "mcp": { + "github": { + "type": "remote", + "url": "https://api.githubcopilot.com/mcp/", + "enabled": true, + "oauth": false, + "headers": { + "Authorization": "Bearer {env:GITHUB_PERSONAL_ACCESS_TOKEN}" + } + } + } +} +``` + +Set `GITHUB_PERSONAL_ACCESS_TOKEN` in your shell environment before starting OpenCode. + +## Local Server (Docker) + +The local GitHub MCP server runs via Docker and requires Docker Desktop (or another Docker runtime) to be installed and running. + +```json +{ + "$schema": "https://opencode.ai/config.json", + "mcp": { + "github": { + "type": "local", + "command": [ + "docker", "run", "-i", "--rm", + "-e", "GITHUB_PERSONAL_ACCESS_TOKEN", + "ghcr.io/github/github-mcp-server" + ], + "enabled": true, + "environment": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "YOUR_GITHUB_PAT" + } + } + } +} +``` + +> [!IMPORTANT] +> OpenCode expects `command` as a **single array** combining the executable and its arguments (e.g. `["docker", "run", "-i", ...]`), and the env-var key is `environment` (not `env`). This differs from hosts like Zed and Cursor. + +## Verify Installation + +1. Restart OpenCode (or start a new session). +2. Check that the server is discovered: + ```sh + opencode mcp list + ``` +3. Try a prompt that references the server by name to bias the model toward its tools: + ``` + Use the github tool to list my recently merged pull requests. + ``` + +## Managing the Server + +OpenCode exposes a few useful subcommands for MCP servers: + +| Command | Purpose | +| --- | --- | +| `opencode mcp list` | List configured MCP servers and their auth/connection status. | +| `opencode mcp debug github` | Show auth status, test HTTP connectivity, and walk through OAuth discovery for the `github` server. | +| `opencode mcp auth github` | Trigger an OAuth flow manually (only relevant if `oauth` is not set to `false`). | +| `opencode mcp logout github` | Clear stored OAuth tokens for the server. | + +## Disabling Tools Per-Agent + +Because the GitHub MCP server can register a large number of tools, you may want to **disable them globally** and **re-enable them only for specific agents**. OpenCode uses the `_*` glob pattern to match all tools from a server: + +```json +{ + "$schema": "https://opencode.ai/config.json", + "mcp": { + "github": { + "type": "remote", + "url": "https://api.githubcopilot.com/mcp/", + "enabled": true, + "oauth": false, + "headers": { "Authorization": "Bearer {env:GITHUB_PERSONAL_ACCESS_TOKEN}" } + } + }, + "tools": { + "github_*": false + }, + "agent": { + "github-helper": { + "tools": { "github_*": true } + } + } +} +``` + +This pattern is recommended by the [OpenCode MCP docs](https://opencode.ai/docs/mcp-servers/) for servers with many tools. + +## Troubleshooting + +- **`401 Unauthorized` from the remote server**: confirm your PAT is valid and not expired. If you set `oauth: false`, OpenCode will not attempt an OAuth fallback — the `Authorization` header must be correct. +- **Server marked failed in `opencode mcp list`**: run `opencode mcp debug github` to see the exact connectivity and auth diagnostics. +- **Tools missing from prompts**: check that `enabled: true` is set on the server and that you have not disabled `github_*` in your `tools` block without re-enabling it for the current agent. +- **Context window exceeded**: the GitHub MCP server can register many tools. Use server-side toolset filtering (`X-MCP-Toolsets` header) to register only the toolsets you need. +- **Docker errors on the local server**: ensure Docker Desktop is running and the `ghcr.io/github/github-mcp-server` image has been pulled (`docker pull ghcr.io/github/github-mcp-server`). + +## Important Notes + +- **Configuration key**: OpenCode uses `mcp` (not `mcpServers` or `context_servers`). +- **Type discriminator**: every entry must include `"type": "local"` or `"type": "remote"`. +- **Command shape**: `command` is a single array combining the executable and its arguments. +- **Environment variable key**: `environment` (not `env`). +- **OAuth**: enabled by default for remote servers. Set `"oauth": false` when using PAT-in-`Authorization`, otherwise OpenCode may try OAuth first. +- **Env interpolation**: use `{env:VAR_NAME}` in string values to read from the shell environment instead of hard-coding secrets. diff --git a/docs/installation-guides/install-zed.md b/docs/installation-guides/install-zed.md new file mode 100644 index 0000000000..d0e07b6d8e --- /dev/null +++ b/docs/installation-guides/install-zed.md @@ -0,0 +1,103 @@ +# Install GitHub MCP Server in Zed + +[Zed](https://zed.dev) is a high-performance multiplayer code editor with native MCP support. Zed exposes MCP servers under the `context_servers` settings key. For general setup information (prerequisites, Docker installation, security best practices), see the [Installation Guides README](./README.md). + +## Prerequisites + +1. Zed installed (latest version — Zed v0.224.0+ recommended for the modern `agent.tool_permissions` settings shape) +2. [GitHub Personal Access Token](https://github.com/settings/personal-access-tokens/new) with appropriate scopes +3. For local installation: [Docker](https://www.docker.com/) installed and running + +## Installation Methods + +There are two ways to install the GitHub MCP server in Zed: + +- **Option A — Zed Extension (easiest):** a community-maintained [GitHub MCP extension](https://zed.dev/extensions/mcp-server-github) is available in the Zed extension gallery. Install it from the Agent Panel's top-right menu → "View Server Extensions", or from the command palette via the `zed: extensions` action. After installation, Zed pops up a modal asking for your GitHub Personal Access Token. +- **Option B — Custom Server (recommended for the official remote endpoint):** add the configuration manually to `settings.json` to use either GitHub's hosted remote server or the official Docker image directly. The rest of this guide covers Option B. + +## Remote Server (Recommended) + +Uses GitHub's hosted server at `https://api.githubcopilot.com/mcp/`. Open your Zed [settings file](https://zed.dev/docs/configuring-zed.html#settings-files) (Command Palette → `zed: open settings`) and add the configuration below under `context_servers`. + +```json +{ + "context_servers": { + "github": { + "url": "https://api.githubcopilot.com/mcp/", + "headers": { + "Authorization": "Bearer YOUR_GITHUB_PAT" + } + } + } +} +``` + +Replace `YOUR_GITHUB_PAT` with your [GitHub Personal Access Token](https://github.com/settings/tokens). To customize toolsets, add server-side headers like `X-MCP-Toolsets` or `X-MCP-Readonly` to the `headers` object — see the [Server Configuration Guide](../server-configuration.md). + +> [!NOTE] +> If you omit the `Authorization` header, Zed will attempt the standard MCP OAuth flow on first use. The GitHub MCP server does not currently advertise OAuth for non-Copilot hosts, so a Personal Access Token in the `Authorization` header is the supported path. + +## Local Server (Docker) + +The local GitHub MCP server runs via Docker and requires Docker Desktop (or another Docker runtime) to be installed and running. + +```json +{ + "context_servers": { + "github": { + "command": "docker", + "args": [ + "run", "-i", "--rm", + "-e", "GITHUB_PERSONAL_ACCESS_TOKEN", + "ghcr.io/github/github-mcp-server" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "YOUR_GITHUB_PAT" + } + } + } +} +``` + +> [!IMPORTANT] +> Zed expects `command` as a **string** plus a separate `args` array, not a single array combining both. This differs from hosts like OpenCode and Claude Desktop. + +## Verify Installation + +1. Open the Agent Panel and click into its Settings view (or run `agent: open settings`). +2. Find `github` in the context servers list. A green indicator dot with the tooltip "Server is active" confirms a working configuration. Other colors and tooltip messages indicate startup or auth errors. +3. Try a prompt that should invoke a tool — for example, `List my recent GitHub pull requests`. Zed will prompt for tool approval before the first call unless your `agent.tool_permissions.default` is set to `"allow"`. + +## Tool Permissions (Optional) + +Zed v0.224.0+ controls tool approval via `agent.tool_permissions`. Approve a specific GitHub MCP tool without per-call prompts by using the `mcp::` key format: + +```json +{ + "agent": { + "tool_permissions": { + "default": "confirm", + "rules": [ + { "tool": "mcp:github:list_pull_requests", "permission": "allow" }, + { "tool": "mcp:github:list_issues", "permission": "allow" } + ] + } + } +} +``` + +See the [Zed tool permissions docs](https://zed.dev/docs/ai/tool-permissions.html) for the full schema. + +## Troubleshooting + +- **Server indicator stays red / "Server is not running"**: check the Agent Panel's settings view for the per-server error string. Most common cause is invalid JSON in `settings.json` — Zed surfaces JSON parse errors in the editor itself. +- **`401 Unauthorized`**: verify your PAT has not expired and includes the scopes for the tools you intend to call. The remote endpoint will reject requests with no `Authorization` header (no anonymous access). +- **Tools missing from prompts**: confirm the Agent profile in use has not disabled the server. If you're using a [custom profile](https://zed.dev/docs/ai/agent-panel.html#custom-profiles), make sure `enable_all_context_servers` is `true` or that `github` is explicitly listed. +- **Docker errors on the local server**: ensure Docker Desktop is running and the `ghcr.io/github/github-mcp-server` image has been pulled at least once. Try `docker pull ghcr.io/github/github-mcp-server` from a terminal. + +## Important Notes + +- **Configuration key**: Zed uses `context_servers` (not `mcpServers`). +- **Command shape**: `command` is a string + separate `args` array. +- **OAuth**: omitting `Authorization` triggers Zed's MCP OAuth flow, but the GitHub MCP server's PAT-based auth is the supported path today. +- **External agents**: MCP servers configured in `context_servers` are forwarded to [external agents](https://zed.dev/docs/ai/external-agents.html) via the Agent Client Protocol. From 03da19109e053b019b8d9555843f532ae132708b Mon Sep 17 00:00:00 2001 From: Emily Chen Date: Fri, 10 Apr 2026 10:05:36 +0000 Subject: [PATCH 07/21] docs: improve Claude installation guide with Windows PowerShell support - Fix README.md: Remove non-existent 'Claude Web' from description - Add Windows PowerShell environment variable example for loading PAT from .env file The previous documentation only showed bash syntax for loading environment variables from .env files, which doesn't work on Windows PowerShell. This adds a PowerShell equivalent to help Windows users set up the GitHub MCP Server correctly. --- docs/installation-guides/README.md | 2 +- docs/installation-guides/install-claude.md | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/installation-guides/README.md b/docs/installation-guides/README.md index 61ea7eafb1..46581aa77e 100644 --- a/docs/installation-guides/README.md +++ b/docs/installation-guides/README.md @@ -6,7 +6,7 @@ This directory contains detailed installation instructions for the GitHub MCP Se - **[Copilot CLI](install-copilot-cli.md)** - Installation guide for GitHub Copilot CLI - **[GitHub Copilot in other IDEs](install-other-copilot-ides.md)** - Installation for JetBrains, Visual Studio, Eclipse, and Xcode with GitHub Copilot - **[Antigravity](install-antigravity.md)** - Installation for Google Antigravity IDE -- **[Claude Applications](install-claude.md)** - Installation guide for Claude Web, Claude Desktop and Claude Code CLI +- **[Claude Applications](install-claude.md)** - Installation guide for Claude Desktop and Claude Code CLI - **[Cline](install-cline.md)** - Installation guide for Cline - **[Cursor](install-cursor.md)** - Installation guide for Cursor IDE - **[Google Gemini CLI](install-gemini-cli.md)** - Installation guide for Google Gemini CLI diff --git a/docs/installation-guides/install-claude.md b/docs/installation-guides/install-claude.md index 67003fb69a..c64484977a 100644 --- a/docs/installation-guides/install-claude.md +++ b/docs/installation-guides/install-claude.md @@ -37,11 +37,17 @@ echo -e ".env\n.mcp.json" >> .gitignore claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/mcp","headers":{"Authorization":"Bearer YOUR_GITHUB_PAT"}}' ``` -With an environment variable: +With an environment variable (Linux/macOS): ```bash claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/mcp","headers":{"Authorization":"Bearer '"$(grep GITHUB_PAT .env | cut -d '=' -f2)"'"}}' ``` +With an environment variable (Windows PowerShell): +```powershell +$env:GITHUB_PAT = (Get-Content .env | Select-String "^GITHUB_PAT=").ToString().Split("=")[1] +claude mcp add-json github "{`"type`":`"http`",`"url`":`"https://api.githubcopilot.com/mcp`",`"headers`":{`"Authorization`":`"Bearer $env:GITHUB_PAT`"}}" +``` + > **About the `--scope` flag** (optional): Use this to specify where the configuration is stored: > - `local` (default): Available only to you in the current project (was called `project` in older versions) > - `project`: Shared with everyone in the project via `.mcp.json` file From d6b9dc94114496b670797eb7aa8205dedc131162 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 10:59:19 +0200 Subject: [PATCH 08/21] docs: address review feedback on env-var examples - Linux/macOS: actually set GITHUB_PAT instead of inlining via subshell, matching the heading. - PowerShell: use Select-Object -First 1, split with max 2 parts, and trim quotes/whitespace so common .env formats work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/installation-guides/install-claude.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/installation-guides/install-claude.md b/docs/installation-guides/install-claude.md index c64484977a..d66b34776b 100644 --- a/docs/installation-guides/install-claude.md +++ b/docs/installation-guides/install-claude.md @@ -39,12 +39,14 @@ claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/ With an environment variable (Linux/macOS): ```bash -claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/mcp","headers":{"Authorization":"Bearer '"$(grep GITHUB_PAT .env | cut -d '=' -f2)"'"}}' +export GITHUB_PAT="$(grep '^GITHUB_PAT=' .env | cut -d '=' -f2-)" +claude mcp add-json github '{"type":"http","url":"https://api.githubcopilot.com/mcp","headers":{"Authorization":"Bearer '"$GITHUB_PAT"'"}}' ``` With an environment variable (Windows PowerShell): ```powershell -$env:GITHUB_PAT = (Get-Content .env | Select-String "^GITHUB_PAT=").ToString().Split("=")[1] +$githubPatLine = Get-Content .env | Select-String "^\s*GITHUB_PAT\s*=" | Select-Object -First 1 +$env:GITHUB_PAT = ($githubPatLine.Line -split "=", 2)[1].Trim().Trim('"').Trim("'") claude mcp add-json github "{`"type`":`"http`",`"url`":`"https://api.githubcopilot.com/mcp`",`"headers`":{`"Authorization`":`"Bearer $env:GITHUB_PAT`"}}" ``` From 830ad2c6132ca6fdf6daa46f19f7102fef7e71a8 Mon Sep 17 00:00:00 2001 From: Dmitry Korobitsin Date: Sun, 15 Mar 2026 17:17:27 +0100 Subject: [PATCH 09/21] docs: clarify that / uses default toolset in remote MCP server --- docs/remote-server.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/remote-server.md b/docs/remote-server.md index 5a82f1c2e1..80699edcb2 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -19,7 +19,7 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) | | ---- | ----------- | ------- | ------------------------- | -------------- | ----------------------------------- | -| apps
`all` | All available GitHub MCP tools | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D) | +| apps
`default` | Default toolset | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D) | | workflow
`actions` | GitHub Actions workflows and CI/CD operations | https://api.githubcopilot.com/mcp/x/actions | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/actions/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%2Freadonly%22%7D) | | codescan
`code_security` | Code security related tools, such as GitHub Code Scanning | https://api.githubcopilot.com/mcp/x/code_security | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/code_security/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%2Freadonly%22%7D) | | copilot
`copilot` | Copilot related tools | https://api.githubcopilot.com/mcp/x/copilot | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-copilot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcopilot%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/copilot/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-copilot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcopilot%2Freadonly%22%7D) | From 07f1d020307d31553918d283fab7ec7da5717375 Mon Sep 17 00:00:00 2001 From: Dmitry Korobitsin Date: Tue, 17 Mar 2026 13:32:03 +0100 Subject: [PATCH 10/21] docs: fix remote default toolset generation --- cmd/github-mcp-server/generate_docs.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index efa8f7c393..0218920571 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -365,9 +365,9 @@ func generateRemoteToolsetsDoc() string { buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n") buf.WriteString("| ---- | ----------- | ------- | ------------------------- | -------------- | ----------------------------------- |\n") - // Add "all" toolset first (special case) - allIcon := octiconImg("apps", "../") - fmt.Fprintf(&buf, "| %s
`all` | All available GitHub MCP tools | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2F%%22%%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Freadonly%%22%%7D) |\n", allIcon) + // Add "default" toolset first (special case) + defaultIcon := octiconImg("apps", "../") + fmt.Fprintf(&buf, "| %s
`default` | Default toolset | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2F%%22%%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Freadonly%%22%%7D) |\n", defaultIcon) // AvailableToolsets() returns toolsets that have tools, sorted by ID // Exclude context (handled separately) From b0d9854388d6fb51bde7b67aa4394a5081a96988 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 11:07:17 +0200 Subject: [PATCH 11/21] docs: also advertise /x/all meta toolset 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> --- cmd/github-mcp-server/generate_docs.go | 8 +++++--- docs/remote-server.md | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 0218920571..78ed8361a8 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -365,9 +365,11 @@ func generateRemoteToolsetsDoc() string { buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n") buf.WriteString("| ---- | ----------- | ------- | ------------------------- | -------------- | ----------------------------------- |\n") - // Add "default" toolset first (special case) - defaultIcon := octiconImg("apps", "../") - fmt.Fprintf(&buf, "| %s
`default` | Default toolset | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2F%%22%%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Freadonly%%22%%7D) |\n", defaultIcon) + // Add "default" and "all" meta toolsets first (special cases). The base + // URL serves the default toolset; /x/all enables every toolset at once. + metaIcon := octiconImg("apps", "../") + fmt.Fprintf(&buf, "| %s
`default` | Default toolset | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2F%%22%%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Freadonly%%22%%7D) |\n", metaIcon) + fmt.Fprintf(&buf, "| %s
`all` | All available GitHub MCP tools | https://api.githubcopilot.com/mcp/x/all | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-all&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Fx%%2Fall%%22%%7D) | [read-only](https://api.githubcopilot.com/mcp/x/all/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-all&config=%%7B%%22type%%22%%3A%%20%%22http%%22%%2C%%22url%%22%%3A%%20%%22https%%3A%%2F%%2Fapi.githubcopilot.com%%2Fmcp%%2Fx%%2Fall%%2Freadonly%%22%%7D) |\n", metaIcon) // AvailableToolsets() returns toolsets that have tools, sorted by ID // Exclude context (handled separately) diff --git a/docs/remote-server.md b/docs/remote-server.md index 80699edcb2..aa083d2f29 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -20,6 +20,7 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) | | ---- | ----------- | ------- | ------------------------- | -------------- | ----------------------------------- | | apps
`default` | Default toolset | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D) | +| apps
`all` | All available GitHub MCP tools | https://api.githubcopilot.com/mcp/x/all | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-all&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fall%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/all/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-all&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fall%2Freadonly%22%7D) | | workflow
`actions` | GitHub Actions workflows and CI/CD operations | https://api.githubcopilot.com/mcp/x/actions | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/actions/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%2Freadonly%22%7D) | | codescan
`code_security` | Code security related tools, such as GitHub Code Scanning | https://api.githubcopilot.com/mcp/x/code_security | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/code_security/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%2Freadonly%22%7D) | | copilot
`copilot` | Copilot related tools | https://api.githubcopilot.com/mcp/x/copilot | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-copilot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcopilot%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/copilot/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-copilot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcopilot%2Freadonly%22%7D) | From 92667523b3c56c0e66bf13141357d24eea01f5f5 Mon Sep 17 00:00:00 2001 From: Nelson Joppi Date: Sat, 7 Mar 2026 02:36:51 -0300 Subject: [PATCH 12/21] fix: return project item id usable for updates --- pkg/github/projects.go | 10 +++++++++- pkg/github/projects_test.go | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 9c7310c0ff..53ce510516 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strconv" "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -1125,7 +1126,8 @@ func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, owne var mutation struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + FullDatabaseID string `graphql:"fullDatabaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` } @@ -1151,6 +1153,12 @@ func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, owne "id": mutation.AddProjectV2ItemByID.Item.ID, "message": fmt.Sprintf("Successfully added %s %s/%s#%d to project %s/%d", itemType, itemOwner, itemRepo, itemNumber, owner, projectNumber), } + if fullDatabaseID := mutation.AddProjectV2ItemByID.Item.FullDatabaseID; fullDatabaseID != "" { + result["full_database_id"] = fullDatabaseID + if itemID, err := strconv.ParseInt(fullDatabaseID, 10, 64); err == nil { + result["item_id"] = itemID + } + } r, err := json.Marshal(result) if err != nil { diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index a9787298af..ad5ce6db86 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -630,7 +630,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + FullDatabaseID string `graphql:"fullDatabaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` }{}, @@ -642,7 +643,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "addProjectV2ItemById": map[string]any{ "item": map[string]any{ - "id": "PVTI_item1", + "id": "PVTI_item1", + "fullDatabaseId": "1001", }, }, }), @@ -674,6 +676,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1001), response["item_id"]) + assert.Equal(t, "1001", response["full_database_id"]) assert.Contains(t, response["message"], "Successfully added") }) @@ -727,7 +731,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + FullDatabaseID string `graphql:"fullDatabaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` }{}, @@ -739,7 +744,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "addProjectV2ItemById": map[string]any{ "item": map[string]any{ - "id": "PVTI_item2", + "id": "PVTI_item2", + "fullDatabaseId": "1002", }, }, }), @@ -771,6 +777,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1002), response["item_id"]) + assert.Equal(t, "1002", response["full_database_id"]) assert.Contains(t, response["message"], "Successfully added") }) From 28846988e997bd604fcb2c283842170f9758a92f Mon Sep 17 00:00:00 2001 From: Roger Garza Date: Fri, 13 Feb 2026 07:37:27 -0600 Subject: [PATCH 13/21] fix: add MCP initialize handshake to mcpcurl 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 ... --- cmd/mcpcurl/main.go | 122 +++++++++++++++++++++++++---- cmd/mcpcurl/main_test.go | 161 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 13 deletions(-) create mode 100644 cmd/mcpcurl/main_test.go diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go index f35e6926c3..db8fac65d4 100644 --- a/cmd/mcpcurl/main.go +++ b/cmd/mcpcurl/main.go @@ -1,7 +1,7 @@ package main import ( - "bytes" + "bufio" "crypto/rand" "encoding/json" "fmt" @@ -376,8 +376,8 @@ func buildJSONRPCRequest(method, toolName string, arguments map[string]any) (str return string(jsonData), nil } -// executeServerCommand runs the specified command, sends the JSON request to stdin, -// and returns the response from stdout +// executeServerCommand runs the specified command, performs the MCP initialization +// handshake, sends the JSON request to stdin, and returns the response from stdout. func executeServerCommand(cmdStr, jsonRequest string) (string, error) { // Split the command string into command and arguments cmdParts := strings.Fields(cmdStr) @@ -393,9 +393,14 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) { return "", fmt.Errorf("failed to create stdin pipe: %w", err) } - // Setup stdout and stderr pipes - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout + // Setup stdout pipe for line-by-line reading + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return "", fmt.Errorf("failed to create stdout pipe: %w", err) + } + + // Stderr still uses a buffer + var stderr strings.Builder cmd.Stderr = &stderr // Start the command @@ -403,18 +408,109 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) { return "", fmt.Errorf("failed to start command: %w", err) } - // Write the JSON request to stdin + // Ensure the child process is cleaned up on any error after Start() + cleanup := func() { + _ = stdin.Close() + _ = cmd.Wait() + } + + // Use a scanner with a large buffer for reading JSON-RPC responses + scanner := bufio.NewScanner(stdoutPipe) + scanner.Buffer(make([]byte, 0, 1024*1024), 1024*1024) // 1MB max line size + + // Step 1: Send MCP initialize request + initReq, err := buildInitializeRequest() + if err != nil { + cleanup() + return "", fmt.Errorf("failed to build initialize request: %w", err) + } + if _, err := io.WriteString(stdin, initReq+"\n"); err != nil { + cleanup() + return "", fmt.Errorf("failed to write initialize request: %w", err) + } + + // Step 2: Read initialize response (skip any server notifications) + if _, err := readJSONRPCResponse(scanner); err != nil { + cleanup() + return "", fmt.Errorf("failed to read initialize response: %w, stderr: %s", err, stderr.String()) + } + + // Step 3: Send initialized notification + if _, err := io.WriteString(stdin, buildInitializedNotification()+"\n"); err != nil { + cleanup() + return "", fmt.Errorf("failed to write initialized notification: %w", err) + } + + // Step 4: Send the actual request if _, err := io.WriteString(stdin, jsonRequest+"\n"); err != nil { - return "", fmt.Errorf("failed to write to stdin: %w", err) + cleanup() + return "", fmt.Errorf("failed to write request: %w", err) + } + + // Step 5: Read the actual response (skip any server notifications) + response, err := readJSONRPCResponse(scanner) + if err != nil { + cleanup() + return "", fmt.Errorf("failed to read response: %w, stderr: %s", err, stderr.String()) } - _ = stdin.Close() - // Wait for the command to complete - if err := cmd.Wait(); err != nil { - return "", fmt.Errorf("command failed: %w, stderr: %s", err, stderr.String()) + // Close stdin and wait for process to exit. The server will see EOF and + // exit with a non-zero status, which is expected — we already have the response. + cleanup() + + return response, nil +} + +// buildInitializeRequest creates the MCP initialize handshake request. +func buildInitializeRequest() (string, error) { + id, err := rand.Int(rand.Reader, big.NewInt(10000)) + if err != nil { + return "", fmt.Errorf("failed to generate random ID: %w", err) + } + msg := map[string]any{ + "jsonrpc": "2.0", + "id": int(id.Int64()), + "method": "initialize", + "params": map[string]any{ + "protocolVersion": "2024-11-05", + "capabilities": map[string]any{}, + "clientInfo": map[string]any{ + "name": "mcpcurl", + "version": "0.1.0", + }, + }, } + data, err := json.Marshal(msg) + if err != nil { + return "", fmt.Errorf("failed to marshal initialize request: %w", err) + } + return string(data), nil +} + +// buildInitializedNotification creates the MCP initialized notification. +func buildInitializedNotification() string { + return `{"jsonrpc":"2.0","method":"notifications/initialized"}` +} - return stdout.String(), nil +// readJSONRPCResponse reads lines from the scanner, skipping server-initiated +// notifications (messages without an "id" field), and returns the first response. +func readJSONRPCResponse(scanner *bufio.Scanner) (string, error) { + for scanner.Scan() { + line := scanner.Text() + // JSON-RPC responses have an "id" field; notifications do not. + var msg map[string]json.RawMessage + if err := json.Unmarshal([]byte(line), &msg); err != nil { + return "", fmt.Errorf("failed to parse JSON-RPC message: %w", err) + } + if _, hasID := msg["id"]; hasID { + return line, nil + } + // No "id" — this is a notification, skip it + } + if err := scanner.Err(); err != nil { + return "", err + } + return "", fmt.Errorf("unexpected end of output") } func printResponse(response string, prettyPrint bool) error { diff --git a/cmd/mcpcurl/main_test.go b/cmd/mcpcurl/main_test.go new file mode 100644 index 0000000000..c31f95b732 --- /dev/null +++ b/cmd/mcpcurl/main_test.go @@ -0,0 +1,161 @@ +package main + +import ( + "bufio" + "encoding/json" + "strings" + "testing" +) + +func TestReadJSONRPCResponse_DirectResponse(t *testing.T) { + t.Parallel() + input := `{"jsonrpc":"2.0","id":1,"result":{"tools":[]}}` + "\n" + scanner := bufio.NewScanner(strings.NewReader(input)) + + got, err := readJSONRPCResponse(scanner) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != `{"jsonrpc":"2.0","id":1,"result":{"tools":[]}}` { + t.Fatalf("unexpected response: %s", got) + } +} + +func TestReadJSONRPCResponse_SkipsNotifications(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"jsonrpc":"2.0","method":"notifications/resources/list_changed","params":{}}`, + `{"jsonrpc":"2.0","method":"notifications/tools/list_changed"}`, + `{"jsonrpc":"2.0","id":42,"result":{"content":[{"type":"text","text":"hello"}]}}`, + }, "\n") + "\n" + scanner := bufio.NewScanner(strings.NewReader(input)) + + got, err := readJSONRPCResponse(scanner) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var msg map[string]json.RawMessage + if err := json.Unmarshal([]byte(got), &msg); err != nil { + t.Fatalf("response is not valid JSON: %v", err) + } + // Verify we got the response with id:42, not a notification + var id int + if err := json.Unmarshal(msg["id"], &id); err != nil { + t.Fatalf("failed to parse id: %v", err) + } + if id != 42 { + t.Fatalf("expected id 42, got %d", id) + } +} + +func TestReadJSONRPCResponse_NoResponse(t *testing.T) { + t.Parallel() + // Only notifications, no response + input := `{"jsonrpc":"2.0","method":"notifications/resources/list_changed","params":{}}` + "\n" + scanner := bufio.NewScanner(strings.NewReader(input)) + + _, err := readJSONRPCResponse(scanner) + if err == nil { + t.Fatal("expected error for missing response, got nil") + } + if !strings.Contains(err.Error(), "unexpected end of output") { + t.Fatalf("expected 'unexpected end of output' error, got: %v", err) + } +} + +func TestReadJSONRPCResponse_EmptyInput(t *testing.T) { + t.Parallel() + scanner := bufio.NewScanner(strings.NewReader("")) + + _, err := readJSONRPCResponse(scanner) + if err == nil { + t.Fatal("expected error for empty input, got nil") + } +} + +func TestReadJSONRPCResponse_InvalidJSON(t *testing.T) { + t.Parallel() + input := "not valid json\n" + scanner := bufio.NewScanner(strings.NewReader(input)) + + _, err := readJSONRPCResponse(scanner) + if err == nil { + t.Fatal("expected error for invalid JSON, got nil") + } + if !strings.Contains(err.Error(), "failed to parse JSON-RPC message") { + t.Fatalf("expected parse error, got: %v", err) + } +} + +func TestBuildInitializeRequest(t *testing.T) { + t.Parallel() + got, err := buildInitializeRequest() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var msg map[string]json.RawMessage + if err := json.Unmarshal([]byte(got), &msg); err != nil { + t.Fatalf("result is not valid JSON: %v", err) + } + + // Verify required fields + for _, field := range []string{"jsonrpc", "id", "method", "params"} { + if _, ok := msg[field]; !ok { + t.Errorf("missing required field %q", field) + } + } + + // Verify method + var method string + if err := json.Unmarshal(msg["method"], &method); err != nil { + t.Fatalf("failed to parse method: %v", err) + } + if method != "initialize" { + t.Errorf("expected method 'initialize', got %q", method) + } + + // Verify params contain protocolVersion and clientInfo + var params map[string]json.RawMessage + if err := json.Unmarshal(msg["params"], ¶ms); err != nil { + t.Fatalf("failed to parse params: %v", err) + } + for _, field := range []string{"protocolVersion", "capabilities", "clientInfo"} { + if _, ok := params[field]; !ok { + t.Errorf("missing params field %q", field) + } + } + + var version string + if err := json.Unmarshal(params["protocolVersion"], &version); err != nil { + t.Fatalf("failed to parse protocolVersion: %v", err) + } + if version != "2024-11-05" { + t.Errorf("expected protocolVersion '2024-11-05', got %q", version) + } +} + +func TestBuildInitializedNotification(t *testing.T) { + t.Parallel() + got := buildInitializedNotification() + + var msg map[string]json.RawMessage + if err := json.Unmarshal([]byte(got), &msg); err != nil { + t.Fatalf("result is not valid JSON: %v", err) + } + + // Must have jsonrpc and method + var method string + if err := json.Unmarshal(msg["method"], &method); err != nil { + t.Fatalf("failed to parse method: %v", err) + } + if method != "notifications/initialized" { + t.Errorf("expected method 'notifications/initialized', got %q", method) + } + + // Must NOT have an id (it's a notification) + if _, hasID := msg["id"]; hasID { + t.Error("notification should not have an 'id' field") + } +} From c05e1bb1d8771d6fb2129b3cd685c73694626688 Mon Sep 17 00:00:00 2001 From: Roger Garza Date: Fri, 13 Feb 2026 19:32:18 -0600 Subject: [PATCH 14/21] fix: surface JSON-RPC error responses in mcpcurl readJSONRPCResponse now checks for an "error" field in responses and returns a descriptive error instead of silently passing it through. --- cmd/mcpcurl/main.go | 3 +++ cmd/mcpcurl/main_test.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go index db8fac65d4..0dad1ea1ac 100644 --- a/cmd/mcpcurl/main.go +++ b/cmd/mcpcurl/main.go @@ -503,6 +503,9 @@ func readJSONRPCResponse(scanner *bufio.Scanner) (string, error) { return "", fmt.Errorf("failed to parse JSON-RPC message: %w", err) } if _, hasID := msg["id"]; hasID { + if errField, hasErr := msg["error"]; hasErr { + return "", fmt.Errorf("server returned error: %s", string(errField)) + } return line, nil } // No "id" — this is a notification, skip it diff --git a/cmd/mcpcurl/main_test.go b/cmd/mcpcurl/main_test.go index c31f95b732..3d0b00d2a5 100644 --- a/cmd/mcpcurl/main_test.go +++ b/cmd/mcpcurl/main_test.go @@ -88,6 +88,23 @@ func TestReadJSONRPCResponse_InvalidJSON(t *testing.T) { } } +func TestReadJSONRPCResponse_ServerError(t *testing.T) { + t.Parallel() + input := `{"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"method not found"}}` + "\n" + scanner := bufio.NewScanner(strings.NewReader(input)) + + _, err := readJSONRPCResponse(scanner) + if err == nil { + t.Fatal("expected error for server error response, got nil") + } + if !strings.Contains(err.Error(), "server returned error") { + t.Fatalf("expected 'server returned error', got: %v", err) + } + if !strings.Contains(err.Error(), "method not found") { + t.Fatalf("expected error to contain server message, got: %v", err) + } +} + func TestBuildInitializeRequest(t *testing.T) { t.Parallel() got, err := buildInitializeRequest() From e5f19db688fd56fa4fbbf62061a944ebff61347e Mon Sep 17 00:00:00 2001 From: ra-n-dom <129428390+ra-n-dom@users.noreply.github.com> Date: Sun, 31 May 2026 11:18:56 +0200 Subject: [PATCH 15/21] review: switch cleanup to defer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- cmd/mcpcurl/main.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go index 0dad1ea1ac..f40e842530 100644 --- a/cmd/mcpcurl/main.go +++ b/cmd/mcpcurl/main.go @@ -408,11 +408,13 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) { return "", fmt.Errorf("failed to start command: %w", err) } - // Ensure the child process is cleaned up on any error after Start() - cleanup := func() { + // Ensure the child process is cleaned up on every return path. + // stdin must be closed before Wait so the server sees EOF and exits; + // its non-zero exit status on EOF is expected, so we ignore the error. + defer func() { _ = stdin.Close() _ = cmd.Wait() - } + }() // Use a scanner with a large buffer for reading JSON-RPC responses scanner := bufio.NewScanner(stdoutPipe) @@ -421,43 +423,33 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) { // Step 1: Send MCP initialize request initReq, err := buildInitializeRequest() if err != nil { - cleanup() return "", fmt.Errorf("failed to build initialize request: %w", err) } if _, err := io.WriteString(stdin, initReq+"\n"); err != nil { - cleanup() return "", fmt.Errorf("failed to write initialize request: %w", err) } // Step 2: Read initialize response (skip any server notifications) if _, err := readJSONRPCResponse(scanner); err != nil { - cleanup() return "", fmt.Errorf("failed to read initialize response: %w, stderr: %s", err, stderr.String()) } // Step 3: Send initialized notification if _, err := io.WriteString(stdin, buildInitializedNotification()+"\n"); err != nil { - cleanup() return "", fmt.Errorf("failed to write initialized notification: %w", err) } // Step 4: Send the actual request if _, err := io.WriteString(stdin, jsonRequest+"\n"); err != nil { - cleanup() return "", fmt.Errorf("failed to write request: %w", err) } // Step 5: Read the actual response (skip any server notifications) response, err := readJSONRPCResponse(scanner) if err != nil { - cleanup() return "", fmt.Errorf("failed to read response: %w, stderr: %s", err, stderr.String()) } - // Close stdin and wait for process to exit. The server will see EOF and - // exit with a non-zero status, which is expected — we already have the response. - cleanup() - return response, nil } From 2bd162acaf4c234f6aa6e73d948129b06f9b25d1 Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Thu, 21 May 2026 03:46:36 +0800 Subject: [PATCH 16/21] fix: support team pull request reviewers --- README.md | 2 +- .../request_pull_request_reviewers.snap | 4 ++-- .../__toolsnaps__/update_pull_request.snap | 4 ++-- pkg/github/granular_tools_test.go | 7 ++++-- pkg/github/pullrequests.go | 6 +++-- pkg/github/pullrequests_granular.go | 24 +++++++++++++++++-- pkg/github/pullrequests_test.go | 18 ++++++++++++++ 7 files changed, 54 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 495eb98992..dc08311a4b 100644 --- a/README.md +++ b/README.md @@ -1163,7 +1163,7 @@ The following sets of tools are available: - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number to update (number, required) - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames to request reviews from (string[], optional) + - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - `state`: New state (string, optional) - `title`: New title (string, optional) diff --git a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap index 67b7014474..7e6d33a274 100644 --- a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap +++ b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap @@ -21,7 +21,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -37,4 +37,4 @@ "type": "object" }, "name": "request_pull_request_reviewers" -} \ No newline at end of file +} diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap index ef330188ff..640df79702 100644 --- a/pkg/github/__toolsnaps__/update_pull_request.snap +++ b/pkg/github/__toolsnaps__/update_pull_request.snap @@ -34,7 +34,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -61,4 +61,4 @@ "type": "object" }, "name": "update_pull_request" -} \ No newline at end of file +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 90b42b22c5..ae34c1dd42 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -773,7 +773,10 @@ func TestGranularUpdatePullRequestState(t *testing.T) { func TestGranularRequestPullRequestReviewers(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)}), + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"user1"}, + "team_reviewers": []any{"team1"}, + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)})), })) deps := BaseDeps{Client: client} serverTool := GranularRequestPullRequestReviewers(translations.NullTranslationHelper) @@ -783,7 +786,7 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) { "owner": "owner", "repo": "repo", "pullNumber": float64(1), - "reviewers": []string{"user1", "user2"}, + "reviewers": []string{"user1", "owner/team1"}, }) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 3910a96b95..7f1751b970 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -752,7 +752,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo }, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{ Type: "string", }, @@ -944,8 +944,10 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) reviewersRequest := github.ReviewersRequest{ - Reviewers: reviewers, + Reviewers: userReviewers, + TeamReviewers: teamReviewers, } _, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest) diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 30d7f78d62..6bc2b99f36 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -297,7 +297,7 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{Type: "string"}, }, }, @@ -325,13 +325,17 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i if len(reviewers) == 0 { return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) + pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{ + Reviewers: userReviewers, + TeamReviewers: teamReviewers, + }) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to request reviewers", resp, err), nil, nil } @@ -351,6 +355,22 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i return st } +func splitPullRequestReviewers(reviewers []string) ([]string, []string) { + userReviewers := make([]string, 0, len(reviewers)) + teamReviewers := make([]string, 0) + + for _, reviewer := range reviewers { + org, team, ok := strings.Cut(reviewer, "/") + if ok && org != "" && team != "" && !strings.Contains(team, "/") { + teamReviewers = append(teamReviewers, team) + continue + } + userReviewers = append(userReviewers, reviewer) + } + + return userReviewers, teamReviewers +} + // GranularCreatePullRequestReview creates a tool to create a PR review. func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 097651b66e..0faee23e2b 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -257,6 +257,24 @@ func Test_UpdatePullRequest(t *testing.T) { expectError: false, expectedPR: mockPRWithReviewers, }, + { + name: "successful PR update with user and team reviewers", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"reviewer1"}, + "team_reviewers": []any{"platform"}, + }).andThen(mockResponse(t, http.StatusOK, mockPRWithReviewers)), + GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "reviewers": []any{"reviewer1", "owner/platform"}, + }, + expectError: false, + expectedPR: mockPRWithReviewers, + }, { name: "successful PR update (title only)", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ From 04c8dcbc8f7aba67e8671003783f647a784e25ef Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Mon, 1 Jun 2026 17:13:19 +0100 Subject: [PATCH 17/21] Skip MCP App form when issue/PR write carries non-form params (#2589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Skip MCP App form when issue/PR write carries non-form params When MCP Apps are enabled and the client supports UI, issue_write and create_pull_request route the call to an interactive form. The form only collects a subset of fields and rebuilds the submit payload from scratch, so any parameter it cannot represent was silently dropped — e.g. labels, assignees, milestone, type, state and issue_fields (priority) for issue_write. Skip the form and execute directly whenever the call carries a parameter outside the set the form collects and re-sends. This generalizes the previous state-only guard and is robust to future parameter additions (an unrecognized param now bypasses the form rather than being lost). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Forward original tool params on MCP App form submit The issue-write and pr-write forms rebuilt their submit payload from scratch, so any parameter the form does not render was dropped on submit. Spread the original toolInput first and override only the edited fields, so unsupported params (e.g. issue_fields, labels, state) are preserved when the user submits the form. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 82 +++++++++++++-------- pkg/github/issues_test.go | 123 ++++++++++++++++++++++++++++++++ pkg/github/pullrequests.go | 41 +++++++++-- pkg/github/pullrequests_test.go | 43 +++++++++++ ui/src/apps/issue-write/App.tsx | 3 +- ui/src/apps/pr-write/App.tsx | 3 +- 6 files changed, 259 insertions(+), 36 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 0469789812..6e9cdae53b 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1763,6 +1763,36 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st // IssueWriteUIResourceURI is the URI for the issue_write tool's MCP App UI resource. const IssueWriteUIResourceURI = "ui://github-mcp-server/issue-write" +// issueWriteFormParams are the parameters the issue_write MCP App form collects +// and re-sends on submit. The form only supports title/body editing (plus the +// routing/identity fields), so any other parameter present on a call cannot be +// represented by the form. +var issueWriteFormParams = map[string]struct{}{ + "method": {}, + "owner": {}, + "repo": {}, + "title": {}, + "body": {}, + "issue_number": {}, + "_ui_submitted": {}, +} + +// issueWriteHasNonFormParams reports whether the call carries any parameter the +// issue_write MCP App form cannot represent (anything outside issueWriteFormParams, +// e.g. labels, assignees, issue_fields or a state change). Such calls must bypass +// the UI form and execute directly so the supplied values aren't silently dropped. +func issueWriteHasNonFormParams(args map[string]any) bool { + for key, value := range args { + if value == nil { + continue + } + if _, ok := issueWriteFormParams[key]; !ok { + return true + } + } + return false +} + // IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write // (with the issue_fields parameter). LegacyIssueWrite is served when the flag // is off. Both register under the tool name "issue_write"; exactly one is @@ -1908,26 +1938,22 @@ Options are: return utils.NewToolResultError(err.Error()), nil, nil } - // When MCP Apps are enabled and the client supports UI, - // check if this is a UI form submission. The UI sends _ui_submitted=true - // to distinguish form submissions from LLM calls. + // When MCP Apps are enabled and the client supports UI, route the + // call to the interactive form unless it is itself a form submission + // (the UI sends _ui_submitted=true) or it carries parameters the form + // cannot represent (e.g. labels, assignees or issue_fields). Those + // must be applied directly so their values aren't silently dropped. uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted") - if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted { + if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) { if method == "update" { - // Skip the UI form when a state change is requested because - // the form only handles title/body editing and would lose the - // state transition (e.g. closing or reopening the issue). - if _, hasState := args["state"]; !hasState { - issueNumber, numErr := RequiredInt(args, "issue_number") - if numErr != nil { - return utils.NewToolResultError("issue_number is required for update method"), nil, nil - } - return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil + issueNumber, numErr := RequiredInt(args, "issue_number") + if numErr != nil { + return utils.NewToolResultError("issue_number is required for update method"), nil, nil } - } else { - return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil + return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil } + return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil } title, err := OptionalParam[string](args, "title") @@ -2144,26 +2170,22 @@ Options are: return utils.NewToolResultError(err.Error()), nil, nil } - // When MCP Apps are enabled and the client supports UI, - // check if this is a UI form submission. The UI sends _ui_submitted=true - // to distinguish form submissions from LLM calls. + // When MCP Apps are enabled and the client supports UI, route the + // call to the interactive form unless it is itself a form submission + // (the UI sends _ui_submitted=true) or it carries parameters the form + // cannot represent (e.g. labels, assignees or issue_fields). Those + // must be applied directly so their values aren't silently dropped. uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted") - if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted { + if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) { if method == "update" { - // Skip the UI form when a state change is requested because - // the form only handles title/body editing and would lose the - // state transition (e.g. closing or reopening the issue). - if _, hasState := args["state"]; !hasState { - issueNumber, numErr := RequiredInt(args, "issue_number") - if numErr != nil { - return utils.NewToolResultError("issue_number is required for update method"), nil, nil - } - return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil + issueNumber, numErr := RequiredInt(args, "issue_number") + if numErr != nil { + return utils.NewToolResultError("issue_number is required for update method"), nil, nil } - } else { - return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil + return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil } + return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil } title, err := OptionalParam[string](args, "title") diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index b04370976e..d794ad1679 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1700,6 +1700,129 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { assert.Contains(t, textContent.Text, "Ready to update issue #1", "update without state should show UI form") }) + + t.Run("UI client with issue_fields skips form and executes directly", func(t *testing.T) { + // The MCP App form does not collect or re-send issue_fields, so a call + // carrying them must bypass the form and apply the values directly. + fieldsClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{ + "title": "Issue with fields", + "body": "", + "labels": []any{}, + "assignees": []any{}, + "issue_field_values": []any{ + map[string]any{"field_id": float64(101), "value": "P1"}, + }, + }).andThen( + mockResponse(t, http.StatusCreated, &github.Issue{ + Number: github.Ptr(125), + Title: github.Ptr("Issue with fields"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"), + State: github.Ptr("open"), + }), + ), + })) + fieldsGQLClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + issueFieldWriteMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "fullDatabaseId": "101", + "name": "Priority", + "dataType": "single_select", + "options": []any{ + map[string]any{"fullDatabaseId": "9001", "name": "P1"}, + }, + }, + }, + }, + }, + }), + ), + )) + + fieldsDeps := BaseDeps{ + Client: fieldsClient, + GQLClient: fieldsGQLClient, + featureChecker: featureCheckerFor(MCPAppsFeatureFlag), + } + fieldsHandler := serverTool.Handler(fieldsDeps) + + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Issue with fields", + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "field_option_name": "P1"}, + }, + }) + result, err := fieldsHandler(ContextWithDeps(context.Background(), fieldsDeps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, "Ready to create an issue", + "issue_fields should skip UI form") + assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/125", + "issue_fields call should execute directly and return issue URL") + }) + + t.Run("UI client with labels skips form and executes directly", func(t *testing.T) { + // The form does not collect labels, so a call carrying them must bypass + // the form rather than silently drop them. + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Test", + "labels": []any{"bug"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, "Ready to create an issue", + "labels should skip UI form") + assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", + "labels call should execute directly and return issue URL") + }) +} + +func Test_issueWriteHasNonFormParams(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args map[string]any + want bool + }{ + {name: "no params", args: map[string]any{}, want: false}, + {name: "only form params", args: map[string]any{"method": "create", "owner": "o", "repo": "r", "title": "t", "body": "b", "issue_number": float64(1), "_ui_submitted": true}, want: false}, + {name: "labels present", args: map[string]any{"title": "t", "labels": []any{"bug"}}, want: true}, + {name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: true}, + {name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: true}, + {name: "type present", args: map[string]any{"title": "t", "type": "Bug"}, want: true}, + {name: "issue_fields present", args: map[string]any{"issue_fields": []any{map[string]any{"field_name": "Priority"}}}, want: true}, + {name: "state present", args: map[string]any{"state": "closed"}, want: true}, + {name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: true}, + {name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: true}, + {name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, issueWriteHasNonFormParams(tc.args)) + }) + } } func Test_ListIssues(t *testing.T) { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 7f1751b970..05028850d7 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -544,6 +544,37 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, deps Tool // PullRequestWriteUIResourceURI is the URI for the create_pull_request tool's MCP App UI resource. const PullRequestWriteUIResourceURI = "ui://github-mcp-server/pr-write" +// pullRequestWriteFormParams are the parameters the create_pull_request MCP App +// form collects and re-sends on submit. Any other parameter present on a call +// cannot be represented by the form. +var pullRequestWriteFormParams = map[string]struct{}{ + "owner": {}, + "repo": {}, + "title": {}, + "body": {}, + "head": {}, + "base": {}, + "draft": {}, + "maintainer_can_modify": {}, + "_ui_submitted": {}, +} + +// pullRequestWriteHasNonFormParams reports whether the call carries any parameter +// the create_pull_request MCP App form cannot represent (anything outside +// pullRequestWriteFormParams). Such calls must bypass the UI form and execute +// directly so the supplied values aren't silently dropped. +func pullRequestWriteHasNonFormParams(args map[string]any) bool { + for key, value := range args { + if value == nil { + continue + } + if _, ok := pullRequestWriteFormParams[key]; !ok { + return true + } + } + return false +} + // CreatePullRequest creates a tool to create a new pull request. func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( @@ -611,12 +642,14 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultError(err.Error()), nil, nil } - // When MCP Apps are enabled and the client supports UI, - // check if this is a UI form submission. The UI sends _ui_submitted=true - // to distinguish form submissions from LLM calls. + // When MCP Apps are enabled and the client supports UI, route the + // call to the interactive form unless it is itself a form submission + // (the UI sends _ui_submitted=true) or it carries parameters the form + // cannot represent. Those must be applied directly so their values + // aren't silently dropped. uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted") - if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted { + if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestWriteHasNonFormParams(args) { return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. IMPORTANT: The PR has NOT been created yet. Do NOT tell the user the PR was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 0faee23e2b..aff71e4c1a 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -2485,6 +2485,49 @@ func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) { assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42", "non-UI client should execute directly") }) + + t.Run("UI client with non-form param skips form and executes directly", func(t *testing.T) { + // A parameter the form does not collect must bypass the form rather than + // be silently dropped. + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Test PR", + "head": "feature", + "base": "main", + "reviewers": []any{"octocat"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, "Ready to create a pull request", + "non-form param should skip UI form") + assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42", + "non-form param call should execute directly and return PR URL") + }) +} + +func Test_pullRequestWriteHasNonFormParams(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args map[string]any + want bool + }{ + {name: "no params", args: map[string]any{}, want: false}, + {name: "only form params", args: map[string]any{"owner": "o", "repo": "r", "title": "t", "body": "b", "head": "h", "base": "b", "draft": true, "maintainer_can_modify": false, "_ui_submitted": true}, want: false}, + {name: "unknown param present", args: map[string]any{"title": "t", "reviewers": []any{"octocat"}}, want: true}, + {name: "nil value is ignored", args: map[string]any{"reviewers": nil}, want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, pullRequestWriteHasNonFormParams(tc.args)) + }) + } } func TestCreateAndSubmitPullRequestReview(t *testing.T) { diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index 863543fc14..fedb7f24f4 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -152,6 +152,7 @@ function CreateIssueApp() { try { const params: Record = { + ...(toolInput as Record | undefined), method: isUpdateMode ? "update" : "create", owner, repo, @@ -204,7 +205,7 @@ function CreateIssueApp() { } finally { setIsSubmitting(false); } - }, [title, body, owner, repo, isUpdateMode, issueNumber, callTool, setModelContext]); + }, [title, body, owner, repo, isUpdateMode, issueNumber, toolInput, callTool, setModelContext]); const body_node = (() => { if (appError) { diff --git a/ui/src/apps/pr-write/App.tsx b/ui/src/apps/pr-write/App.tsx index bfefdbede0..abbeacb124 100644 --- a/ui/src/apps/pr-write/App.tsx +++ b/ui/src/apps/pr-write/App.tsx @@ -156,6 +156,7 @@ function CreatePRApp() { try { const result = await callTool("create_pull_request", { + ...(toolInput as Record | undefined), owner, repo, title: title.trim(), body: body.trim(), @@ -193,7 +194,7 @@ function CreatePRApp() { } finally { setIsSubmitting(false); } - }, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, callTool, setModelContext]); + }, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, toolInput, callTool, setModelContext]); if (successPR) { return ( From 7e79ae9aa55b6caf75c1bc327aca7f65de0ef756 Mon Sep 17 00:00:00 2001 From: Timur Date: Mon, 1 Jun 2026 11:45:53 +0200 Subject: [PATCH 18/21] feat: replace include_diff with detail enum on get_commit Replace the get_commit tool's two boolean flags (include_diff, include_patch) with a single detail enum: none / stats / full_patch. Why: - The two-boolean shape had an awkward dependency ("include_patch only applies when include_diff is true") and an impossible state (include_patch=true, include_diff=false) that was silently ignored. - A single discriminator collapses three meaningful response shapes into one orthogonal choice, makes the most expensive option ("full_patch") self-describing, and eliminates the "diff vs patch" naming confusion. Behavior: - Default ("stats") matches the previous default (include_diff=true, include_patch=false): per-file metadata with no patch text. Existing callers using defaults are unaffected. - "none" omits Stats and Files entirely (was include_diff=false). - "full_patch" is the new opt-in level that adds the unified diff to each MinimalCommitFile. Breaking change: callers that previously passed include_diff or include_patch must switch to detail. Callers using the defaults are unaffected. Changes: - Added Patch field to MinimalCommitFile. - Added commitDetail type, parseCommitDetail, and migrated convertToMinimalCommit to take a commitDetail. - Updated get_commit schema, list_commits caller (commitDetailNone), unit tests, toolsnap, and README. Co-authored-by: Sam Morrow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- pkg/github/__toolsnaps__/get_commit.snap | 13 ++- pkg/github/minimal_types.go | 73 ++++++++++----- pkg/github/repositories.go | 19 ++-- pkg/github/repositories_test.go | 114 +++++++++++++++++++++++ 5 files changed, 188 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index dc08311a4b..dff62321b8 100644 --- a/README.md +++ b/README.md @@ -1221,7 +1221,7 @@ The following sets of tools are available: - **get_commit** - Get commit details - **Required OAuth Scopes**: `repo` - - `include_diff`: Whether to include file diffs and stats in the response. Default is true. (boolean, optional) + - `detail`: Level of detail to include for changed files. "none" omits stats and files entirely. "stats" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. "full_patch" additionally includes the unified diff content for each file and can be very large. (string, optional) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) diff --git a/pkg/github/__toolsnaps__/get_commit.snap b/pkg/github/__toolsnaps__/get_commit.snap index 9e2346b59d..122e6210b3 100644 --- a/pkg/github/__toolsnaps__/get_commit.snap +++ b/pkg/github/__toolsnaps__/get_commit.snap @@ -6,10 +6,15 @@ "description": "Get details for a commit from a GitHub repository", "inputSchema": { "properties": { - "include_diff": { - "default": true, - "description": "Whether to include file diffs and stats in the response. Default is true.", - "type": "boolean" + "detail": { + "default": "stats", + "description": "Level of detail to include for changed files. \"none\" omits stats and files entirely. \"stats\" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. \"full_patch\" additionally includes the unified diff content for each file and can be very large.", + "enum": [ + "none", + "stats", + "full_patch" + ], + "type": "string" }, "owner": { "description": "Repository owner", diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index a93d29ead5..5200be297f 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -108,6 +108,7 @@ type MinimalCommitFile struct { Additions int `json:"additions,omitempty"` Deletions int `json:"deletions,omitempty"` Changes int `json:"changes,omitempty"` + Patch string `json:"patch,omitempty"` } // MinimalPRFile represents a file changed in a pull request. @@ -1463,8 +1464,34 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author return minimalCommit } -// convertToMinimalCommit converts a GitHub API RepositoryCommit to MinimalCommit -func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) MinimalCommit { +// commitDetail controls how much per-file information convertToMinimalCommit +// includes in its output. +type commitDetail string + +const ( + // commitDetailNone omits Stats and Files entirely. + commitDetailNone commitDetail = "none" + // commitDetailStats includes Stats and Files with metadata only + // (filename, status, additions, deletions, changes) but no patch text. + commitDetailStats commitDetail = "stats" + // commitDetailFullPatch additionally includes the unified diff for each file. + commitDetailFullPatch commitDetail = "full_patch" +) + +// parseCommitDetail validates the user-supplied detail value and returns the +// default (stats) when the value is empty. +func parseCommitDetail(s string) (commitDetail, error) { + switch s { + case "": + return commitDetailStats, nil + case string(commitDetailNone), string(commitDetailStats), string(commitDetailFullPatch): + return commitDetail(s), nil + default: + return "", fmt.Errorf("invalid detail %q: must be one of \"none\", \"stats\", \"full_patch\"", s) + } +} + +func convertToMinimalCommit(commit *github.RepositoryCommit, detail commitDetail) MinimalCommit { minimalCommit := newMinimalCommitFromCore( commit.GetSHA(), commit.GetHTMLURL(), @@ -1473,28 +1500,32 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool) commit.Committer, ) - // Only include stats and files if includeDiffs is true - if includeDiffs { - if commit.Stats != nil { - minimalCommit.Stats = &MinimalCommitStats{ - Additions: commit.Stats.GetAdditions(), - Deletions: commit.Stats.GetDeletions(), - Total: commit.Stats.GetTotal(), - } + if detail == commitDetailNone { + return minimalCommit + } + + if commit.Stats != nil { + minimalCommit.Stats = &MinimalCommitStats{ + Additions: commit.Stats.GetAdditions(), + Deletions: commit.Stats.GetDeletions(), + Total: commit.Stats.GetTotal(), } + } - if len(commit.Files) > 0 { - minimalCommit.Files = make([]MinimalCommitFile, 0, len(commit.Files)) - for _, file := range commit.Files { - minimalFile := MinimalCommitFile{ - Filename: file.GetFilename(), - Status: file.GetStatus(), - Additions: file.GetAdditions(), - Deletions: file.GetDeletions(), - Changes: file.GetChanges(), - } - minimalCommit.Files = append(minimalCommit.Files, minimalFile) + if len(commit.Files) > 0 { + minimalCommit.Files = make([]MinimalCommitFile, 0, len(commit.Files)) + for _, file := range commit.Files { + minimalFile := MinimalCommitFile{ + Filename: file.GetFilename(), + Status: file.GetStatus(), + Additions: file.GetAdditions(), + Deletions: file.GetDeletions(), + Changes: file.GetChanges(), + } + if detail == commitDetailFullPatch { + minimalFile.Patch = file.GetPatch() } + minimalCommit.Files = append(minimalCommit.Files, minimalFile) } } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index d682b5c3d7..040a968cf9 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -46,10 +46,11 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { Type: "string", Description: "Commit SHA, branch name, or tag name", }, - "include_diff": { - Type: "boolean", - Description: "Whether to include file diffs and stats in the response. Default is true.", - Default: json.RawMessage(`true`), + "detail": { + Type: "string", + Enum: []any{"none", "stats", "full_patch"}, + Description: "Level of detail to include for changed files. \"none\" omits stats and files entirely. \"stats\" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. \"full_patch\" additionally includes the unified diff content for each file and can be very large.", + Default: json.RawMessage(`"stats"`), }, }, Required: []string{"owner", "repo", "sha"}, @@ -69,7 +70,11 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - includeDiff, err := OptionalBoolParamWithDefault(args, "include_diff", true) + detailRaw, err := OptionalParam[string](args, "detail") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + detail, err := parseCommitDetail(detailRaw) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -106,7 +111,7 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { } // Convert to minimal commit - minimalCommit := convertToMinimalCommit(commit, includeDiff) + minimalCommit := convertToMinimalCommit(commit, detail) r, err := json.Marshal(minimalCommit) if err != nil { @@ -252,7 +257,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool { // Convert to minimal commits minimalCommits := make([]MinimalCommit, len(commits)) for i, commit := range commits { - minimalCommits[i] = convertToMinimalCommit(commit, false) + minimalCommits[i] = convertToMinimalCommit(commit, commitDetailNone) } r, err := json.Marshal(minimalCommits) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 03535f1d26..e1b7f94f53 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1028,6 +1028,120 @@ func Test_GetCommit(t *testing.T) { } } +func Test_GetCommit_Detail(t *testing.T) { + mockCommit := &github.RepositoryCommit{ + SHA: github.Ptr("abc123def456"), + HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"), + Commit: &github.Commit{ + Message: github.Ptr("First commit"), + }, + Stats: &github.CommitStats{ + Additions: github.Ptr(10), + Deletions: github.Ptr(2), + Total: github.Ptr(12), + }, + Files: []*github.CommitFile{ + { + Filename: github.Ptr("file1.go"), + Status: github.Ptr("modified"), + Additions: github.Ptr(10), + Deletions: github.Ptr(2), + Changes: github.Ptr(12), + Patch: github.Ptr("@@ -1,2 +1,10 @@\n+new line"), + }, + }, + } + + cases := []struct { + name string + args map[string]any + expectFiles bool + expectStats bool + expectPatch bool + expectError string + }{ + { + name: "default returns stats", + args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456"}, + expectFiles: true, + expectStats: true, + expectPatch: false, + }, + { + name: "detail=none omits stats and files", + args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "none"}, + expectFiles: false, + expectStats: false, + expectPatch: false, + }, + { + name: "detail=stats returns metadata without patch", + args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "stats"}, + expectFiles: true, + expectStats: true, + expectPatch: false, + }, + { + name: "detail=full_patch includes patch text", + args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "full_patch"}, + expectFiles: true, + expectStats: true, + expectPatch: true, + }, + { + name: "invalid detail value is rejected", + args: map[string]any{"owner": "owner", "repo": "repo", "sha": "abc123def456", "detail": "everything"}, + expectError: `invalid detail "everything"`, + }, + } + + serverTool := GetCommit(translations.NullTranslationHelper) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCommitsByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockCommit), + }) + client := mustNewGHClient(t, mockedClient) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectError != "" { + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, tc.expectError) + return + } + require.False(t, result.IsError) + + var returned MinimalCommit + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returned)) + + if tc.expectStats { + require.NotNil(t, returned.Stats) + assert.Equal(t, 12, returned.Stats.Total) + } else { + assert.Nil(t, returned.Stats) + } + + if tc.expectFiles { + require.Len(t, returned.Files, 1) + assert.Equal(t, "file1.go", returned.Files[0].Filename) + if tc.expectPatch { + assert.Equal(t, "@@ -1,2 +1,10 @@\n+new line", returned.Files[0].Patch) + } else { + assert.Empty(t, returned.Files[0].Patch) + } + } else { + assert.Empty(t, returned.Files) + } + }) + } +} + func Test_ListCommits(t *testing.T) { // Verify tool definition once serverTool := ListCommits(translations.NullTranslationHelper) From 2a5d38a2825519e109bcbd3ccfd13a459582a455 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Tue, 2 Jun 2026 14:36:43 +0100 Subject: [PATCH 19/21] MCP Apps: Open created issue/PR link via host open-link capability (#2593) * Open created issue/PR link via host open-link capability The success views for issue-write and pr-write rendered a plain anchor to the created/updated issue or PR. MCP Apps run in a sandboxed iframe where target="_blank" navigation may be blocked, so clicking the link did nothing in some hosts. Route the click through the host's ui/open-link capability (already exposed by useMcpApp as openLink), which asks the host to open the URL in the user's browser. The hook now also falls back to window.open when the host denies the request, in addition to the existing no-app fallback. The href is retained so right-click/copy and native fallback still work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prevent default anchor navigation before URL check When the success-view link URL was unavailable ("#"), the click handler returned before calling e.preventDefault(), so the anchor's default target="_blank" navigation still ran and could open a stray blank tab. Call preventDefault() first, then no-op when the URL is unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ui/src/apps/issue-write/App.tsx | 13 ++++++++++++- ui/src/apps/pr-write/App.tsx | 14 ++++++++++++-- ui/src/hooks/useMcpApp.ts | 7 ++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index fedb7f24f4..6c46b8c081 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -33,12 +33,14 @@ function SuccessView({ repo, submittedTitle, isUpdate, + openLink, }: { issue: IssueResult; owner: string; repo: string; submittedTitle: string; isUpdate: boolean; + openLink: (url: string) => Promise; }) { const issueUrl = issue.html_url || issue.url || issue.URL || "#"; @@ -87,6 +89,14 @@ function SuccessView({ href={issueUrl} target="_blank" rel="noopener noreferrer" + onClick={(e) => { + // MCP Apps run in a sandboxed iframe where a plain anchor may be + // blocked, so route the click through the host's open-link + // capability (falls back to window.open). + e.preventDefault(); + if (issueUrl === "#") return; + void openLink(issueUrl); + }} style={{ fontWeight: 600, fontSize: "14px", @@ -121,7 +131,7 @@ function CreateIssueApp() { const [error, setError] = useState(null); const [successIssue, setSuccessIssue] = useState(null); - const { app, error: appError, toolInput, callTool, hostContext, setModelContext } = useMcpApp({ + const { app, error: appError, toolInput, callTool, hostContext, setModelContext, openLink } = useMcpApp({ appName: "github-mcp-server-issue-write", }); @@ -232,6 +242,7 @@ function CreateIssueApp() { repo={repo} submittedTitle={title} isUpdate={isUpdateMode} + openLink={openLink} /> ); } diff --git a/ui/src/apps/pr-write/App.tsx b/ui/src/apps/pr-write/App.tsx index abbeacb124..245753a1bc 100644 --- a/ui/src/apps/pr-write/App.tsx +++ b/ui/src/apps/pr-write/App.tsx @@ -36,11 +36,13 @@ function SuccessView({ owner, repo, submittedTitle, + openLink, }: { pr: PRResult; owner: string; repo: string; submittedTitle: string; + openLink: (url: string) => Promise; }) { const prUrl = pr.html_url || pr.url || pr.URL || "#"; @@ -89,6 +91,14 @@ function SuccessView({ href={prUrl} target="_blank" rel="noopener noreferrer" + onClick={(e) => { + // MCP Apps run in a sandboxed iframe where a plain anchor may be + // blocked, so route the click through the host's open-link + // capability (falls back to window.open). + e.preventDefault(); + if (prUrl === "#") return; + void openLink(prUrl); + }} style={{ fontWeight: 600, fontSize: "14px", @@ -126,7 +136,7 @@ function CreatePRApp() { const [isDraft, setIsDraft] = useState(false); const [maintainerCanModify, setMaintainerCanModify] = useState(true); - const { app, error: appError, toolInput, callTool, hostContext, setModelContext } = useMcpApp({ + const { app, error: appError, toolInput, callTool, hostContext, setModelContext, openLink } = useMcpApp({ appName: "github-mcp-server-create-pull-request", }); @@ -199,7 +209,7 @@ function CreatePRApp() { if (successPR) { return ( - + ); } diff --git a/ui/src/hooks/useMcpApp.ts b/ui/src/hooks/useMcpApp.ts index b060ea6ee2..cf386520f0 100644 --- a/ui/src/hooks/useMcpApp.ts +++ b/ui/src/hooks/useMcpApp.ts @@ -106,7 +106,12 @@ export function useMcpApp({ window.open(url, "_blank", "noopener,noreferrer"); return; } - await app.openLink({ url }); + const result = await app.openLink({ url }); + // The host may deny the request (e.g. blocked domain or user cancelled). + // Fall back to a direct window.open so the link still works. + if (result?.isError) { + window.open(url, "_blank", "noopener,noreferrer"); + } }, [app] ); From 33849e98eb4797107d033b6d29ffbd5f6d9e8a2b Mon Sep 17 00:00:00 2001 From: Ross Tarrant Date: Thu, 4 Jun 2026 10:17:23 +0100 Subject: [PATCH 20/21] fix: Empty assignees array should clear assignees (#2600) * fix: Empty assignees array should clear assignees --- pkg/github/issues.go | 41 ++++++++++++++++++++--- pkg/github/issues_test.go | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 6e9cdae53b..ef9bbc4305 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1972,12 +1972,16 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + assigneesValue, assigneesProvided := args["assignees"] + assigneesProvided = assigneesProvided && assigneesValue != nil // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + labelsValue, labelsProvided := args["labels"] + labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2049,7 +2053,10 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf) + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ + AssigneesProvided: assigneesProvided, + LabelsProvided: labelsProvided, + }) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2204,12 +2211,16 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + assigneesValue, assigneesProvided := args["assignees"] + assigneesProvided = assigneesProvided && assigneesValue != nil // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + labelsValue, labelsProvided := args["labels"] + labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2266,7 +2277,10 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf) + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, UpdateIssueOptions{ + AssigneesProvided: assigneesProvided, + LabelsProvided: labelsProvided, + }) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2330,7 +2344,24 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo return utils.NewToolResultText(string(r)), nil } -func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int) (*mcp.CallToolResult, error) { +// UpdateIssueOptions controls which optional fields are included in an issue update request. +type UpdateIssueOptions struct { + // AssigneesProvided sends the assignees field even when the slice is empty. + AssigneesProvided bool + // LabelsProvided sends the labels field even when the slice is empty. + LabelsProvided bool +} + +func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) { + updateOptions := UpdateIssueOptions{ + AssigneesProvided: len(assignees) > 0, + LabelsProvided: len(labels) > 0, + } + for _, opt := range opts { + updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided + updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided + } + // Create the issue request with only provided fields issueRequest := &github.IssueRequest{} @@ -2343,11 +2374,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.Body = github.Ptr(body) } - if len(labels) > 0 { + if updateOptions.LabelsProvided { issueRequest.Labels = &labels } - if len(assignees) > 0 { + if updateOptions.AssigneesProvided { issueRequest.Assignees = &assignees } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index d794ad1679..7e47cdb527 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2987,6 +2987,33 @@ func Test_UpdateIssue(t *testing.T) { expectError: false, expectedIssue: mockUpdatedIssue, }, + { + name: "partial update clears labels and assignees", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{}, + "assignees": []any{}, + }).andThen( + mockResponse(t, http.StatusOK, &github.Issue{ + Number: github.Ptr(123), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + }), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "labels": []any{}, + "assignees": []any{}, + }, + expectError: false, + expectedIssue: &github.Issue{ + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + }, + }, { name: "partial update with issue fields reconciled by names", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -3406,6 +3433,47 @@ func Test_UpdateIssue(t *testing.T) { } } +func Test_LegacyUpdateIssueClearsLabelsAndAssignees(t *testing.T) { + serverTool := LegacyIssueWrite(translations.NullTranslationHelper) + updatedIssue := &github.Issue{ + Number: github.Ptr(8), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/8"), + } + + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{}, + "assignees": []any{}, + }).andThen(mockResponse(t, http.StatusOK, updatedIssue)), + })) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(8), + "labels": []any{}, + "assignees": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + if result.IsError { + t.Fatalf("Unexpected error result: %s", getErrorResult(t, result).Text) + } + textContent := getTextResult(t, result) + + var updateResp MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &updateResp)) + assert.Equal(t, updatedIssue.GetHTMLURL(), updateResp.URL) +} + func Test_ParseISOTimestamp(t *testing.T) { tests := []struct { name string From 457f59932ac041c9276e03e634b0e0c30f19ba3e Mon Sep 17 00:00:00 2001 From: Alon Dahari Date: Fri, 5 Jun 2026 15:04:22 +0100 Subject: [PATCH 21/21] Add confidence parameter to issue mutation MCP tools (#2605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an optional confidence integer parameter (0–100) to update_issue_type, update_issue_labels, and set_issue_fields MCP tools. The confidence score is passed through to the REST/GraphQL API on mutation calls. - Rename structs to WithIntent (labelWithIntent, issueTypeWithIntent) - Add confidence schema property (integer, min 0, max 100) with prompt guidance describing what different confidence levels represent - Update tool descriptions to encourage including confidence scores - Pass confidence in the API request body alongside rationale/suggest Closes github/plan-track-agentic-toolkit#219 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 3 +- .../request_pull_request_reviewers.snap | 2 +- .../__toolsnaps__/set_issue_fields.snap | 11 +- .../__toolsnaps__/update_issue_labels.snap | 11 +- .../__toolsnaps__/update_issue_type.snap | 11 +- .../__toolsnaps__/update_pull_request.snap | 2 +- pkg/github/granular_tools_test.go | 321 ++++++++++++++++++ pkg/github/issues_granular.go | 96 ++++-- 8 files changed, 425 insertions(+), 32 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 0b75a61bac..63fb28dc44 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -198,6 +198,7 @@ runtime behavior (such as output formatting) won't appear here. - **update_issue_type** - Update Issue Type - **Required OAuth Scopes**: `repo` + - `confidence`: How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal. (string, optional) - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) - `issue_type`: The issue type to set (string, required) @@ -240,7 +241,7 @@ runtime behavior (such as output formatting) won't appear here. - `owner`: Repository owner (username or organization) (string, required) - `pullNumber`: The pull request number (number, required) - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames to request reviews from (string[], required) + - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], required) - **resolve_review_thread** - Resolve Review Thread - **Required OAuth Scopes**: `repo` diff --git a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap index 7e6d33a274..20f1ab62b6 100644 --- a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap +++ b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap @@ -37,4 +37,4 @@ "type": "object" }, "name": "request_pull_request_reviewers" -} +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/set_issue_fields.snap b/pkg/github/__toolsnaps__/set_issue_fields.snap index 88c88fdc65..e46febeeda 100644 --- a/pkg/github/__toolsnaps__/set_issue_fields.snap +++ b/pkg/github/__toolsnaps__/set_issue_fields.snap @@ -4,13 +4,22 @@ "openWorldHint": true, "title": "Set Issue Fields" }, - "description": "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue.", + "description": "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue. When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "fields": { "description": "Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value.", "items": { "properties": { + "confidence": { + "description": "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + "enum": [ + "low", + "medium", + "high" + ], + "type": "string" + }, "date_value": { "description": "The value to set for a date field (ISO 8601 date string)", "type": "string" diff --git a/pkg/github/__toolsnaps__/update_issue_labels.snap b/pkg/github/__toolsnaps__/update_issue_labels.snap index 3bdbdfc9ef..21f7fea6b6 100644 --- a/pkg/github/__toolsnaps__/update_issue_labels.snap +++ b/pkg/github/__toolsnaps__/update_issue_labels.snap @@ -4,7 +4,7 @@ "openWorldHint": true, "title": "Update Issue Labels" }, - "description": "Update the labels of an existing issue. This replaces the current labels with the provided list.", + "description": "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "issue_number": { @@ -22,6 +22,15 @@ }, { "properties": { + "confidence": { + "description": "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + "enum": [ + "low", + "medium", + "high" + ], + "type": "string" + }, "is_suggestion": { "description": "If true, this label is sent to the API as a suggestion (suggest:true) rather than an applied label. Whether the label is applied or recorded as a proposal is determined by the API.", "type": "boolean" diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap index da749cd466..2f39b2d3b8 100644 --- a/pkg/github/__toolsnaps__/update_issue_type.snap +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -4,9 +4,18 @@ "openWorldHint": true, "title": "Update Issue Type" }, - "description": "Update the type of an existing issue (e.g. 'bug', 'feature').", + "description": "Update the type of an existing issue (e.g. 'bug', 'feature'). When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice.", "inputSchema": { "properties": { + "confidence": { + "description": "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + "enum": [ + "low", + "medium", + "high" + ], + "type": "string" + }, "is_suggestion": { "description": "If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API.", "type": "boolean" diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap index 640df79702..3d87fe75fe 100644 --- a/pkg/github/__toolsnaps__/update_pull_request.snap +++ b/pkg/github/__toolsnaps__/update_pull_request.snap @@ -61,4 +61,4 @@ "type": "object" }, "name": "update_pull_request" -} +} \ No newline at end of file diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index ae34c1dd42..eb688a0b9f 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -461,6 +461,91 @@ func TestGranularUpdateIssueLabelsInvalidRationale(t *testing.T) { } } +func TestGranularUpdateIssueLabelsConfidence(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "label with confidence triggers object form", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": "bug", "confidence": "high"}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "confidence": "high"}, + }, + }, + }, + { + name: "label with confidence and rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash", "confidence": "medium"}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash", "confidence": "medium"}, + }, + }, + }, + { + name: "invalid confidence value", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": "bug", "confidence": "very_high"}, + }, + }, + expectedReq: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.expectedReq == nil { + // Error case + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "confidence must be one of: low, medium, high") + return + } + + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + } +} + func TestGranularUpdateIssueMilestone(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ @@ -642,6 +727,128 @@ func TestGranularUpdateIssueTypeInvalidRationale(t *testing.T) { } } +func TestGranularUpdateIssueTypeConfidence(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "type with confidence only", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + "confidence": "high", + }, + expectedReq: map[string]any{ + "type": map[string]any{ + "value": "bug", + "confidence": "high", + }, + }, + }, + { + name: "type with confidence and rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "feature", + "rationale": "Asks for dark mode support", + "confidence": "medium", + }, + expectedReq: map[string]any{ + "type": map[string]any{ + "value": "feature", + "rationale": "Asks for dark mode support", + "confidence": "medium", + }, + }, + }, + { + name: "type with low confidence triggers object form", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + "confidence": "low", + }, + expectedReq: map[string]any{ + "type": map[string]any{ + "value": "bug", + "confidence": "low", + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + } +} + +func TestGranularUpdateIssueTypeInvalidConfidence(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedErrText string + }{ + { + name: "invalid confidence value", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + "confidence": "very_high", + }, + expectedErrText: "confidence must be one of: low, medium, high", + }, + { + name: "confidence wrong type", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + "confidence": float64(85), + }, + expectedErrText: "parameter confidence is not of type string", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrText) + }) + } +} + func TestGranularUpdateIssueState(t *testing.T) { tests := []struct { name string @@ -1389,6 +1596,120 @@ func TestGranularSetIssueFields(t *testing.T) { assert.Contains(t, textContent.Text, "field rationale must be 280 characters or less") }) + t.Run("successful set with confidence", func(t *testing.T) { + confidence := "high" + matchers := []githubv4mock.Matcher{ + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(5), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{"id": "ISSUE_123"}, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + SetIssueFieldValue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + } + IssueFieldValues []struct { + TextValue struct { + Value string + } `graphql:"... on IssueFieldTextValue"` + SingleSelectValue struct { + Name string + } `graphql:"... on IssueFieldSingleSelectValue"` + DateValue struct { + Value string + } `graphql:"... on IssueFieldDateValue"` + NumberValue struct { + Value float64 + } `graphql:"... on IssueFieldNumberValue"` + } + } `graphql:"setIssueFieldValue(input: $input)"` + }{}, + SetIssueFieldValueInput{ + IssueID: githubv4.ID("ISSUE_123"), + IssueFields: []IssueFieldCreateOrUpdateInput{ + { + FieldID: githubv4.ID("FIELD_1"), + TextValue: githubv4.NewString(githubv4.String("hello")), + Confidence: &confidence, + }, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "setIssueFieldValue": map[string]any{ + "issue": map[string]any{ + "id": "ISSUE_123", + "number": 5, + "url": "https://github.com/owner/repo/issues/5", + }, + }, + }), + ), + } + + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matchers...)) + deps := BaseDeps{GQLClient: gqlClient} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{ + "field_id": "FIELD_1", + "text_value": "hello", + "confidence": "high", + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + + t.Run("invalid confidence value returns error", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{ + "field_id": "FIELD_1", + "text_value": "hello", + "confidence": "very_high", + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "confidence must be one of: low, medium, high") + }) + t.Run("successful set with suggest flag", func(t *testing.T) { suggestTrue := githubv4.Boolean(true) matchers := []githubv4mock.Matcher{ diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 73fa75413c..22d26cc47f 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -258,17 +258,18 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor ) } -// labelWithRationale represents the object form of a label entry, allowing a -// rationale and/or suggest flag to be sent alongside the label name. -type labelWithRationale struct { - Name string `json:"name"` - Rationale string `json:"rationale,omitempty"` - Suggest bool `json:"suggest,omitempty"` +// labelWithIntent represents the object form of a label entry, allowing a +// rationale, confidence level, and/or suggest flag to be sent alongside the label name. +type labelWithIntent struct { + Name string `json:"name"` + Rationale string `json:"rationale,omitempty"` + Confidence string `json:"confidence,omitempty"` + Suggest bool `json:"suggest,omitempty"` } // labelsUpdateRequest is a custom request body for updating an issue's labels // where individual labels may optionally include a rationale. Each element of -// Labels is either a string (label name) or a labelWithRationale object. +// Labels is either a string (label name) or a labelWithIntent object. type labelsUpdateRequest struct { Labels []any `json:"labels"` } @@ -279,7 +280,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_labels", - Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Update the labels of an existing issue. This replaces the current labels with the provided list."), + Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_LABELS_USER_TITLE", "Update Issue Labels"), ReadOnlyHint: false, @@ -321,6 +322,11 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S "State the concrete signal (e.g. 'Reports a crash when saving' → bug).", MaxLength: jsonschema.Ptr(280), }, + "confidence": { + Type: "string", + Description: "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + Enum: []any{"low", "medium", "high"}, + }, "is_suggestion": { Type: "boolean", Description: "If true, this label is sent to the API as a suggestion (suggest:true) rather than an applied label. " + @@ -387,18 +393,25 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S if len([]rune(rationale)) > 280 { return utils.NewToolResultError("label rationale must be 280 characters or less"), nil, nil } + confidence, err := OptionalParam[string](v, "confidence") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if confidence != "" && confidence != "low" && confidence != "medium" && confidence != "high" { + return utils.NewToolResultError("confidence must be one of: low, medium, high"), nil, nil + } isSuggestion, err := OptionalParam[bool](v, "is_suggestion") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if rationale == "" && !isSuggestion { + if rationale == "" && !isSuggestion && confidence == "" { payload = append(payload, name) } else { useObjectForm = true - payload = append(payload, labelWithRationale{Name: name, Rationale: rationale, Suggest: isSuggestion}) + payload = append(payload, labelWithIntent{Name: name, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion}) } default: - return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale' and/or 'is_suggestion'"), nil, nil + return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale', 'confidence', and/or 'is_suggestion'"), nil, nil } } @@ -470,18 +483,19 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor ) } -// issueTypeWithRationale represents the object form of the issue type field, -// allowing a rationale and/or suggest flag to be sent alongside the type name. -type issueTypeWithRationale struct { - Value string `json:"value"` - Rationale string `json:"rationale,omitempty"` - Suggest bool `json:"suggest,omitempty"` +// issueTypeWithIntent represents the object form of the issue type field, +// allowing a rationale, confidence level, and/or suggest flag to be sent alongside the type name. +type issueTypeWithIntent struct { + Value string `json:"value"` + Rationale string `json:"rationale,omitempty"` + Confidence string `json:"confidence,omitempty"` + Suggest bool `json:"suggest,omitempty"` } // issueTypeUpdateRequest is a custom request body for updating an issue type -// with an optional rationale, using the object form that the REST API accepts. +// with optional intent metadata, using the object form that the REST API accepts. type issueTypeUpdateRequest struct { - Type issueTypeWithRationale `json:"type"` + Type issueTypeWithIntent `json:"type"` } // GranularUpdateIssueType creates a tool to update an issue's type. @@ -490,7 +504,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_type", - Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Update the type of an existing issue (e.g. 'bug', 'feature')."), + Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Update the type of an existing issue (e.g. 'bug', 'feature'). When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_TYPE_USER_TITLE", "Update Issue Type"), ReadOnlyHint: false, @@ -523,6 +537,11 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser "State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature).", MaxLength: jsonschema.Ptr(280), }, + "confidence": { + Type: "string", + Description: "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + Enum: []any{"low", "medium", "high"}, + }, "is_suggestion": { Type: "boolean", Description: "If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. " + @@ -558,6 +577,13 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if len([]rune(rationale)) > 280 { return utils.NewToolResultError("parameter rationale must be 280 characters or less"), nil, nil } + confidence, err := OptionalParam[string](args, "confidence") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if confidence != "" && confidence != "low" && confidence != "medium" && confidence != "high" { + return utils.NewToolResultError("confidence must be one of: low, medium, high"), nil, nil + } isSuggestion, err := OptionalParam[bool](args, "is_suggestion") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -569,12 +595,13 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser } var body any - if rationale != "" || isSuggestion { + if rationale != "" || isSuggestion || confidence != "" { body = &issueTypeUpdateRequest{ - Type: issueTypeWithRationale{ - Value: issueType, - Rationale: rationale, - Suggest: isSuggestion, + Type: issueTypeWithIntent{ + Value: issueType, + Rationale: rationale, + Confidence: confidence, + Suggest: isSuggestion, }, } } else { @@ -887,6 +914,7 @@ type IssueFieldCreateOrUpdateInput struct { SingleSelectOptionID *githubv4.ID `json:"singleSelectOptionId,omitempty"` Delete *githubv4.Boolean `json:"delete,omitempty"` Rationale *githubv4.String `json:"rationale,omitempty"` + Confidence *string `json:"confidence,omitempty"` Suggest *githubv4.Boolean `json:"suggest,omitempty"` } @@ -896,7 +924,7 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv ToolsetMetadataIssues, mcp.Tool{ Name: "set_issue_fields", - Description: t("TOOL_SET_ISSUE_FIELDS_DESCRIPTION", "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue."), + Description: t("TOOL_SET_ISSUE_FIELDS_DESCRIPTION", "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue. When setting values, include a confidence level (low, medium, or high) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_SET_ISSUE_FIELDS_USER_TITLE", "Set Issue Fields"), ReadOnlyHint: false, @@ -956,6 +984,11 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv "State the concrete signal (e.g. 'Reports a crash when saving' → high priority).", MaxLength: jsonschema.Ptr(280), }, + "confidence": { + Type: "string", + Description: "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + Enum: []any{"low", "medium", "high"}, + }, "is_suggestion": { Type: "boolean", Description: "If true, this field value is sent to the API as a suggestion (suggest:true) rather than an applied value. " + @@ -1073,6 +1106,17 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv } } + confidence, err := OptionalParam[string](fieldMap, "confidence") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if confidence != "" && confidence != "low" && confidence != "medium" && confidence != "high" { + return utils.NewToolResultError("confidence must be one of: low, medium, high"), nil, nil + } + if confidence != "" { + input.Confidence = &confidence + } + isSuggestion, err := OptionalParam[bool](fieldMap, "is_suggestion") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil