Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cmd/github-mcp-server/generate_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
}
sort.Strings(paramNames)

conditional := inventory.ConditionalSchemaPropertyDescriptions()

for i, propName := range paramNames {
prop := schema.Properties[propName]
required := slices.Contains(schema.Required, propName)
Expand All @@ -282,7 +284,11 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
// Indent any continuation lines in the description to maintain markdown formatting
description := indentMultilineDescription(prop.Description, " ")

fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
if cond, isConditional := conditional[propName]; isConditional {
fmt.Fprintf(buf, " - `%s`: %s (%s, %s, conditional — %s)", propName, description, typeStr, requiredStr, cond)
} else {
fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
}
if i < len(paramNames)-1 {
buf.WriteString("\n")
}
Expand Down
4 changes: 3 additions & 1 deletion docs/feature-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ runtime behavior (such as output formatting) won't appear here.
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
- `title`: PR title (string, required)

- **get_me** - Get my user profile
Expand All @@ -66,6 +67,7 @@ runtime behavior (such as output formatting) won't appear here.
- `milestone`: Milestone number (number, optional)
- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
- `state`: New state (string, optional)
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
- `title`: Issue title (string, optional)
Expand Down Expand Up @@ -240,7 +242,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`
Expand Down
2 changes: 2 additions & 0 deletions docs/insiders-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
- `title`: PR title (string, required)

- **get_me** - Get my user profile
Expand All @@ -60,6 +61,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
- `milestone`: Milestone number (number, optional)
- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — only visible to clients that advertise MCP App UI support)
- `state`: New state (string, optional)
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
- `title`: Issue title (string, optional)
Expand Down
4 changes: 4 additions & 0 deletions pkg/github/__toolsnaps__/create_pull_request.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
"description": "Repository name",
"type": "string"
},
"show_ui": {
"description": "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action.",
"type": "boolean"
},
"title": {
"description": "PR title",
"type": "string"
Expand Down
4 changes: 4 additions & 0 deletions pkg/github/__toolsnaps__/issue_write.snap
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
"description": "Repository name",
"type": "string"
},
"show_ui": {
"description": "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action.",
"type": "boolean"
},
"state": {
"description": "New state",
"enum": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@
"description": "Repository name",
"type": "string"
},
"show_ui": {
"description": "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action.",
"type": "boolean"
},
"state": {
"description": "New state",
"enum": [
Expand Down
55 changes: 45 additions & 10 deletions pkg/github/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -1774,6 +1774,7 @@ var issueWriteFormParams = map[string]struct{}{
"title": {},
"body": {},
"issue_number": {},
"show_ui": {},
Comment thread
mattdholloway marked this conversation as resolved.
"_ui_submitted": {},
}

Expand Down Expand Up @@ -1918,6 +1919,17 @@ Options are:
Required: []string{"field_name"},
},
},
// show_ui is hidden from clients that do not advertise MCP App
// UI support. The strip happens per-request in
// inventory.ToolsForRegistration; it is present in the static
// schema (and therefore in toolsnaps and the feature-flag /
// insiders docs) so the UI-capable surface is fully
// documented. It is intentionally not in the main README,
// which renders the stripped (non-UI) schema.
"show_ui": {
Type: "boolean",
Description: "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action.",
},
},
Required: []string{"method", "owner", "repo"},
},
Expand All @@ -1939,13 +1951,19 @@ Options are:
}

// 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.
// call to the interactive form unless:
// - it is itself a form submission (the UI sends _ui_submitted=true),
// - the caller explicitly asked to skip the UI (show_ui=false), 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")
showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}

if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !issueWriteHasNonFormParams(args) {
if method == "update" {
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
Expand Down Expand Up @@ -2150,6 +2168,17 @@ Options are:
Type: "number",
Description: "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.",
},
// show_ui is hidden from clients that do not advertise MCP App
// UI support. The strip happens per-request in
// inventory.ToolsForRegistration; it is present in the static
// schema (and therefore in toolsnaps and the feature-flag /
// insiders docs) so the UI-capable surface is fully
// documented. It is intentionally not in the main README,
// which renders the stripped (non-UI) schema.
"show_ui": {
Type: "boolean",
Description: "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, or state changes) and the user has already confirmed the action.",
},
},
Required: []string{"method", "owner", "repo"},
},
Expand All @@ -2171,13 +2200,19 @@ Options are:
}

// 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.
// call to the interactive form unless:
// - it is itself a form submission (the UI sends _ui_submitted=true),
// - the caller explicitly asked to skip the UI (show_ui=false), 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")
showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}

if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !issueWriteHasNonFormParams(args) {
if method == "update" {
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
Expand Down
133 changes: 133 additions & 0 deletions pkg/github/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/http/headers"
transportpkg "github.com/github/github-mcp-server/pkg/http/transport"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/go-github/v87/github"
"github.com/google/jsonschema-go/jsonschema"
Expand Down Expand Up @@ -1794,6 +1795,86 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
"labels call should execute directly and return issue URL")
})

t.Run("UI client with show_ui=false skips form and executes directly", func(t *testing.T) {
// show_ui=false is the explicit, model-facing way to opt out of the
// form. It must bypass the form even when every other condition would
// route the call there (UI capability, MCP Apps flag on, no
// _ui_submitted, only form params present).
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Test",
"show_ui": false,
})
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",
"show_ui=false should skip UI form")
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
"show_ui=false call should execute directly and return issue URL")
})

t.Run("UI client with show_ui=true returns form message", func(t *testing.T) {
// show_ui=true is the explicit, redundant-with-the-default way to ask
// for the form. It must still route through the form and must not be
// treated as a non-form parameter that would trigger the safety-net
// bypass.
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Test",
"show_ui": true,
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "Ready to create an issue",
"show_ui=true should still route through the form")
})

t.Run("UI client with show_ui=false and _ui_submitted=true executes directly", func(t *testing.T) {
// _ui_submitted and show_ui=false are two ways to say "execute
// directly". When both are set there must be no conflict — the call
// still executes directly.
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Test",
"show_ui": false,
"_ui_submitted": true,
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
"show_ui=false + _ui_submitted should execute directly")
})

t.Run("non-UI client with show_ui=false executes directly (no regression)", func(t *testing.T) {
// show_ui is irrelevant when the client does not support UI; the call
// must execute directly exactly as it does today.
request := createMCPRequest(map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Test",
"show_ui": false,
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
"non-UI client should execute directly regardless of show_ui")
})
}

func Test_issueWriteHasNonFormParams(t *testing.T) {
Expand All @@ -1806,6 +1887,8 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
}{
{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: "show_ui true is a form param", args: map[string]any{"title": "t", "show_ui": true}, want: false},
{name: "show_ui false is a form param", args: map[string]any{"title": "t", "show_ui": false}, 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},
Expand All @@ -1825,6 +1908,56 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
}
}

// Test_issueWriteSchemaClassification fails when a schema property is added
// without classifying it as either form-resendable (issueWriteFormParams) or
// known-non-form (knownNonForm below). Without this guard, an unclassified
// property would silently flip UI gating: form-incompatible fields would
// stop tripping the safety-net bypass and the form would drop their values.
func Test_issueWriteSchemaClassification(t *testing.T) {
t.Parallel()

// Schema properties the MCP App form cannot represent — their presence
// must trigger the safety-net bypass via issueWriteHasNonFormParams.
knownNonForm := map[string]struct{}{
"assignees": {},
"labels": {},
"milestone": {},
"type": {},
"state": {},
"state_reason": {},
"duplicate_of": {},
"issue_fields": {}, // only on the FF-enabled IssueWrite variant
}

cases := []struct {
name string
tool inventory.ServerTool
}{
{name: "IssueWrite", tool: IssueWrite(translations.NullTranslationHelper)},
{name: "LegacyIssueWrite", tool: LegacyIssueWrite(translations.NullTranslationHelper)},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
schema, ok := tc.tool.Tool.InputSchema.(*jsonschema.Schema)
require.True(t, ok, "InputSchema should be *jsonschema.Schema")

for prop := range schema.Properties {
_, isForm := issueWriteFormParams[prop]
_, isNonForm := knownNonForm[prop]

assert.Falsef(t, isForm && isNonForm,
"property %q is classified as both form-resendable and non-form — pick one", prop)
assert.Truef(t, isForm || isNonForm,
"property %q in %s schema is unclassified — add it to issueWriteFormParams (pkg/github/issues.go) "+
"if the MCP App form can carry it on submit, otherwise add it to the knownNonForm allowlist in this test",
prop, tc.name)
}
})
}
}

func Test_ListIssues(t *testing.T) {
// Verify tool definition
serverTool := ListIssues(translations.NullTranslationHelper)
Expand Down
Loading
Loading