Skip to content

Commit 1f6d285

Browse files
committed
Merge branch 'main' into reneexeener/promote-issue-write-with-issue-fields
2 parents 7fb61e6 + d27540f commit 1f6d285

16 files changed

Lines changed: 708 additions & 35 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ The following sets of tools are available:
12121212
- `description`: Repository description (string, optional)
12131213
- `name`: Repository name (string, required)
12141214
- `organization`: Organization to create the repository in (omit to create in your personal account) (string, optional)
1215-
- `private`: Whether repo should be private (boolean, optional)
1215+
- `private`: Whether the repository should be private. Defaults to true (private) when omitted. (boolean, optional)
12161216

12171217
- **delete_file** - Delete file
12181218
- **Required OAuth Scopes**: `repo`

cmd/github-mcp-server/generate_docs.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
257257
}
258258
sort.Strings(paramNames)
259259

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

285-
fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
287+
if cond, isConditional := conditional[propName]; isConditional {
288+
fmt.Fprintf(buf, " - `%s`: %s (%s, %s, conditional — %s)", propName, description, typeStr, requiredStr, cond)
289+
} else {
290+
fmt.Fprintf(buf, " - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
291+
}
286292
if i < len(paramNames)-1 {
287293
buf.WriteString("\n")
288294
}

docs/feature-flags.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ runtime behavior (such as output formatting) won't appear here.
4444
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
4545
- `owner`: Repository owner (string, required)
4646
- `repo`: Repository name (string, required)
47+
- `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)
4748
- `title`: PR title (string, required)
4849

4950
- **get_me** - Get my user profile

docs/insiders-features.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
3838
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
3939
- `owner`: Repository owner (string, required)
4040
- `repo`: Repository name (string, required)
41+
- `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)
4142
- `title`: PR title (string, required)
4243

4344
- **get_me** - Get my user profile

pkg/github/__toolsnaps__/create_pull_request.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
"description": "Repository name",
4343
"type": "string"
4444
},
45+
"show_ui": {
46+
"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.",
47+
"type": "boolean"
48+
},
4549
"title": {
4650
"description": "PR title",
4751
"type": "string"

pkg/github/__toolsnaps__/create_repository.snap

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
"type": "string"
2323
},
2424
"private": {
25-
"description": "Whether repo should be private",
25+
"default": true,
26+
"description": "Whether the repository should be private. Defaults to true (private) when omitted.",
2627
"type": "boolean"
2728
}
2829
},

pkg/github/__toolsnaps__/issue_write.snap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@
9696
"description": "Repository name",
9797
"type": "string"
9898
},
99+
"show_ui": {
100+
"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.",
101+
"type": "boolean"
102+
},
99103
"state": {
100104
"description": "New state",
101105
"enum": [

pkg/github/issues.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,7 @@ var issueWriteFormParams = map[string]struct{}{
16441644
"title": {},
16451645
"body": {},
16461646
"issue_number": {},
1647+
"show_ui": {},
16471648
"_ui_submitted": {},
16481649
}
16491650

@@ -1784,6 +1785,17 @@ Options are:
17841785
Required: []string{"field_name"},
17851786
},
17861787
},
1788+
// show_ui is hidden from clients that do not advertise MCP App
1789+
// UI support. The strip happens per-request in
1790+
// inventory.ToolsForRegistration; it is present in the static
1791+
// schema (and therefore in toolsnaps and the feature-flag /
1792+
// insiders docs) so the UI-capable surface is fully
1793+
// documented. It is intentionally not in the main README,
1794+
// which renders the stripped (non-UI) schema.
1795+
"show_ui": {
1796+
Type: "boolean",
1797+
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.",
1798+
},
17871799
},
17881800
Required: []string{"method", "owner", "repo"},
17891801
},
@@ -1805,13 +1817,19 @@ Options are:
18051817
}
18061818

18071819
// When MCP Apps are enabled and the client supports UI, route the
1808-
// call to the interactive form unless it is itself a form submission
1809-
// (the UI sends _ui_submitted=true) or it carries parameters the form
1810-
// cannot represent (e.g. labels, assignees or issue_fields). Those
1811-
// must be applied directly so their values aren't silently dropped.
1820+
// call to the interactive form unless:
1821+
// - it is itself a form submission (the UI sends _ui_submitted=true),
1822+
// - the caller explicitly asked to skip the UI (show_ui=false), or
1823+
// - it carries parameters the form cannot represent (e.g. labels,
1824+
// assignees or issue_fields). Those must be applied directly so
1825+
// their values aren't silently dropped.
18121826
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
1827+
showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
1828+
if err != nil {
1829+
return utils.NewToolResultError(err.Error()), nil, nil
1830+
}
18131831

1814-
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
1832+
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !issueWriteHasNonFormParams(args) {
18151833
if method == "update" {
18161834
issueNumber, numErr := RequiredInt(args, "issue_number")
18171835
if numErr != nil {

pkg/github/issues_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/github/github-mcp-server/internal/toolsnaps"
1616
"github.com/github/github-mcp-server/pkg/http/headers"
1717
transportpkg "github.com/github/github-mcp-server/pkg/http/transport"
18+
"github.com/github/github-mcp-server/pkg/inventory"
1819
"github.com/github/github-mcp-server/pkg/translations"
1920
"github.com/google/go-github/v87/github"
2021
"github.com/google/jsonschema-go/jsonschema"
@@ -1792,6 +1793,86 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
17921793
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
17931794
"labels call should execute directly and return issue URL")
17941795
})
1796+
1797+
t.Run("UI client with show_ui=false skips form and executes directly", func(t *testing.T) {
1798+
// show_ui=false is the explicit, model-facing way to opt out of the
1799+
// form. It must bypass the form even when every other condition would
1800+
// route the call there (UI capability, MCP Apps flag on, no
1801+
// _ui_submitted, only form params present).
1802+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
1803+
"method": "create",
1804+
"owner": "owner",
1805+
"repo": "repo",
1806+
"title": "Test",
1807+
"show_ui": false,
1808+
})
1809+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1810+
require.NoError(t, err)
1811+
1812+
textContent := getTextResult(t, result)
1813+
assert.NotContains(t, textContent.Text, "Ready to create an issue",
1814+
"show_ui=false should skip UI form")
1815+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
1816+
"show_ui=false call should execute directly and return issue URL")
1817+
})
1818+
1819+
t.Run("UI client with show_ui=true returns form message", func(t *testing.T) {
1820+
// show_ui=true is the explicit, redundant-with-the-default way to ask
1821+
// for the form. It must still route through the form and must not be
1822+
// treated as a non-form parameter that would trigger the safety-net
1823+
// bypass.
1824+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
1825+
"method": "create",
1826+
"owner": "owner",
1827+
"repo": "repo",
1828+
"title": "Test",
1829+
"show_ui": true,
1830+
})
1831+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1832+
require.NoError(t, err)
1833+
1834+
textContent := getTextResult(t, result)
1835+
assert.Contains(t, textContent.Text, "Ready to create an issue",
1836+
"show_ui=true should still route through the form")
1837+
})
1838+
1839+
t.Run("UI client with show_ui=false and _ui_submitted=true executes directly", func(t *testing.T) {
1840+
// _ui_submitted and show_ui=false are two ways to say "execute
1841+
// directly". When both are set there must be no conflict — the call
1842+
// still executes directly.
1843+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
1844+
"method": "create",
1845+
"owner": "owner",
1846+
"repo": "repo",
1847+
"title": "Test",
1848+
"show_ui": false,
1849+
"_ui_submitted": true,
1850+
})
1851+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1852+
require.NoError(t, err)
1853+
1854+
textContent := getTextResult(t, result)
1855+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
1856+
"show_ui=false + _ui_submitted should execute directly")
1857+
})
1858+
1859+
t.Run("non-UI client with show_ui=false executes directly (no regression)", func(t *testing.T) {
1860+
// show_ui is irrelevant when the client does not support UI; the call
1861+
// must execute directly exactly as it does today.
1862+
request := createMCPRequest(map[string]any{
1863+
"method": "create",
1864+
"owner": "owner",
1865+
"repo": "repo",
1866+
"title": "Test",
1867+
"show_ui": false,
1868+
})
1869+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1870+
require.NoError(t, err)
1871+
1872+
textContent := getTextResult(t, result)
1873+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
1874+
"non-UI client should execute directly regardless of show_ui")
1875+
})
17951876
}
17961877

17971878
func Test_issueWriteHasNonFormParams(t *testing.T) {
@@ -1804,6 +1885,8 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
18041885
}{
18051886
{name: "no params", args: map[string]any{}, want: false},
18061887
{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},
1888+
{name: "show_ui true is a form param", args: map[string]any{"title": "t", "show_ui": true}, want: false},
1889+
{name: "show_ui false is a form param", args: map[string]any{"title": "t", "show_ui": false}, want: false},
18071890
{name: "labels present", args: map[string]any{"title": "t", "labels": []any{"bug"}}, want: true},
18081891
{name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: true},
18091892
{name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: true},
@@ -1823,6 +1906,56 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
18231906
}
18241907
}
18251908

1909+
// Test_issueWriteSchemaClassification fails when a schema property is added
1910+
// without classifying it as either form-resendable (issueWriteFormParams) or
1911+
// known-non-form (knownNonForm below). Without this guard, an unclassified
1912+
// property would silently flip UI gating: form-incompatible fields would
1913+
// stop tripping the safety-net bypass and the form would drop their values.
1914+
func Test_issueWriteSchemaClassification(t *testing.T) {
1915+
t.Parallel()
1916+
1917+
// Schema properties the MCP App form cannot represent — their presence
1918+
// must trigger the safety-net bypass via issueWriteHasNonFormParams.
1919+
knownNonForm := map[string]struct{}{
1920+
"assignees": {},
1921+
"labels": {},
1922+
"milestone": {},
1923+
"type": {},
1924+
"state": {},
1925+
"state_reason": {},
1926+
"duplicate_of": {},
1927+
"issue_fields": {}, // only on the FF-enabled IssueWrite variant
1928+
}
1929+
1930+
cases := []struct {
1931+
name string
1932+
tool inventory.ServerTool
1933+
}{
1934+
{name: "IssueWrite", tool: IssueWrite(translations.NullTranslationHelper)},
1935+
{name: "LegacyIssueWrite", tool: LegacyIssueWrite(translations.NullTranslationHelper)},
1936+
}
1937+
1938+
for _, tc := range cases {
1939+
t.Run(tc.name, func(t *testing.T) {
1940+
t.Parallel()
1941+
schema, ok := tc.tool.Tool.InputSchema.(*jsonschema.Schema)
1942+
require.True(t, ok, "InputSchema should be *jsonschema.Schema")
1943+
1944+
for prop := range schema.Properties {
1945+
_, isForm := issueWriteFormParams[prop]
1946+
_, isNonForm := knownNonForm[prop]
1947+
1948+
assert.Falsef(t, isForm && isNonForm,
1949+
"property %q is classified as both form-resendable and non-form — pick one", prop)
1950+
assert.Truef(t, isForm || isNonForm,
1951+
"property %q in %s schema is unclassified — add it to issueWriteFormParams (pkg/github/issues.go) "+
1952+
"if the MCP App form can carry it on submit, otherwise add it to the knownNonForm allowlist in this test",
1953+
prop, tc.name)
1954+
}
1955+
})
1956+
}
1957+
}
1958+
18261959
func Test_ListIssues(t *testing.T) {
18271960
// Verify tool definition
18281961
serverTool := ListIssues(translations.NullTranslationHelper)

pkg/github/pullrequests.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,7 @@ var pullRequestWriteFormParams = map[string]struct{}{
599599
"base": {},
600600
"draft": {},
601601
"maintainer_can_modify": {},
602+
"show_ui": {},
602603
"_ui_submitted": {},
603604
}
604605

@@ -670,6 +671,17 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
670671
Type: "boolean",
671672
Description: "Allow maintainer edits",
672673
},
674+
// show_ui is hidden from clients that do not advertise MCP App
675+
// UI support. The strip happens per-request in
676+
// inventory.ToolsForRegistration; it is present in the static
677+
// schema (and therefore in toolsnaps and the feature-flag /
678+
// insiders docs) so the UI-capable surface is fully
679+
// documented. It is intentionally not in the main README,
680+
// which renders the stripped (non-UI) schema.
681+
"show_ui": {
682+
Type: "boolean",
683+
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.",
684+
},
673685
},
674686
Required: []string{"owner", "repo", "title", "head", "base"},
675687
},
@@ -686,13 +698,18 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
686698
}
687699

688700
// When MCP Apps are enabled and the client supports UI, route the
689-
// call to the interactive form unless it is itself a form submission
690-
// (the UI sends _ui_submitted=true) or it carries parameters the form
691-
// cannot represent. Those must be applied directly so their values
692-
// aren't silently dropped.
701+
// call to the interactive form unless:
702+
// - it is itself a form submission (the UI sends _ui_submitted=true),
703+
// - the caller explicitly asked to skip the UI (show_ui=false), or
704+
// - it carries parameters the form cannot represent. Those must be
705+
// applied directly so their values aren't silently dropped.
693706
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
707+
showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
708+
if err != nil {
709+
return utils.NewToolResultError(err.Error()), nil, nil
710+
}
694711

695-
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestWriteHasNonFormParams(args) {
712+
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !pullRequestWriteHasNonFormParams(args) {
696713
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
697714
}
698715

0 commit comments

Comments
 (0)