Skip to content

Commit 6219a58

Browse files
committed
use hardcoded client name check for ui support
1 parent dfbd526 commit 6219a58

File tree

8 files changed

+168
-52
lines changed

8 files changed

+168
-52
lines changed

internal/ghmcp/server.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -347,15 +347,6 @@ func addUserAgentsMiddleware(cfg github.MCPServerConfig, restClient *gogithub.Cl
347347
}
348348

349349
message := initializeRequest
350-
351-
// Debug log client capabilities for MCP Apps support diagnosis
352-
if message.Params.Capabilities != nil {
353-
slog.Info("client capabilities",
354-
"experimental", message.Params.Capabilities.Experimental,
355-
"client", message.Params.ClientInfo.Name,
356-
)
357-
}
358-
359350
userAgent := fmt.Sprintf(
360351
"github-mcp-server/%s (%s/%s)",
361352
cfg.Version,

pkg/github/helper_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"bytes"
5+
"context"
56
"encoding/json"
67
"io"
78
"net/http"
@@ -290,6 +291,44 @@ func createMCPRequest(args any) mcp.CallToolRequest {
290291
}
291292
}
292293

294+
// createMCPRequestWithSession creates a CallToolRequest with a ServerSession
295+
// that has the given client name in its InitializeParams. This is used to test
296+
// UI capability detection based on ClientInfo.Name.
297+
func createMCPRequestWithSession(t *testing.T, clientName string, args any) mcp.CallToolRequest {
298+
t.Helper()
299+
300+
argsMap, ok := args.(map[string]interface{})
301+
if !ok {
302+
argsMap = make(map[string]interface{})
303+
}
304+
argsJSON, err := json.Marshal(argsMap)
305+
require.NoError(t, err)
306+
307+
srv := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
308+
309+
st, _ := mcp.NewInMemoryTransports()
310+
session, err := srv.Connect(context.Background(), st, &mcp.ServerSessionOptions{
311+
State: &mcp.ServerSessionState{
312+
InitializeParams: &mcp.InitializeParams{
313+
ClientInfo: &mcp.Implementation{Name: clientName},
314+
},
315+
},
316+
})
317+
require.NoError(t, err)
318+
319+
// Close the unused client-side transport and session
320+
t.Cleanup(func() {
321+
_ = session.Close()
322+
})
323+
324+
return mcp.CallToolRequest{
325+
Session: session,
326+
Params: &mcp.CallToolParamsRaw{
327+
Arguments: json.RawMessage(argsJSON),
328+
},
329+
}
330+
}
331+
293332
// getTextResult is a helper function that returns a text result from a tool call.
294333
func getTextResult(t *testing.T, result *mcp.CallToolResult) *mcp.TextContent {
295334
t.Helper()

pkg/github/issues.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ Options are:
10861086
},
10871087
},
10881088
[]scopes.Scope{scopes.Repo},
1089-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
1089+
func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
10901090
method, err := RequiredParam[string](args, "method")
10911091
if err != nil {
10921092
return utils.NewToolResultError(err.Error()), nil, nil
@@ -1101,21 +1101,20 @@ Options are:
11011101
return utils.NewToolResultError(err.Error()), nil, nil
11021102
}
11031103

1104-
// When insiders mode is enabled, check if this is a UI form submission.
1105-
// The UI sends _ui_submitted=true to distinguish form submissions from LLM calls.
1106-
// Without this flag, return a message so the client can show a form.
1107-
// If no form is displayed, the model should call again with _ui_submitted=true.
1104+
// When insiders mode is enabled and the client supports MCP Apps UI,
1105+
// check if this is a UI form submission. The UI sends _ui_submitted=true
1106+
// to distinguish form submissions from LLM calls.
11081107
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
11091108

1110-
if deps.GetFlags(ctx).InsidersMode && !uiSubmitted {
1109+
if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(req) && !uiSubmitted {
11111110
if method == "update" {
11121111
issueNumber, numErr := RequiredInt(args, "issue_number")
11131112
if numErr != nil {
11141113
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
11151114
}
1116-
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. The interactive form will be displayed. If no form appears, call this tool again with _ui_submitted set to true to proceed.", issueNumber, owner, repo)), nil, nil
1115+
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. The user will review and confirm via the interactive form.", issueNumber, owner, repo)), nil, nil
11171116
}
1118-
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. The interactive form will be displayed. If no form appears, call this tool again with _ui_submitted set to true to proceed.", owner, repo)), nil, nil
1117+
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. The user will review and confirm via the interactive form.", owner, repo)), nil, nil
11191118
}
11201119

11211120
title, err := OptionalParam[string](args, "title")

pkg/github/issues_test.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -934,8 +934,7 @@ func Test_CreateIssue(t *testing.T) {
934934
}
935935

936936
// Test_IssueWrite_InsidersMode_UIGate verifies the insiders mode UI gate
937-
// behavior: the first call returns a form-ready message, and a subsequent
938-
// call with _ui_submitted=true executes the action.
937+
// behavior: UI clients get a form message, non-UI clients execute directly.
939938
func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) {
940939
t.Parallel()
941940

@@ -958,8 +957,8 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) {
958957
}
959958
handler := serverTool.Handler(deps)
960959

961-
t.Run("without _ui_submitted returns form message", func(t *testing.T) {
962-
request := createMCPRequest(map[string]interface{}{
960+
t.Run("UI client without _ui_submitted returns form message", func(t *testing.T) {
961+
request := createMCPRequestWithSession(t, "Visual Studio Code - Insiders", map[string]interface{}{
963962
"method": "create",
964963
"owner": "owner",
965964
"repo": "repo",
@@ -970,12 +969,10 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) {
970969

971970
textContent := getTextResult(t, result)
972971
assert.Contains(t, textContent.Text, "Ready to create an issue")
973-
assert.Contains(t, textContent.Text, "_ui_submitted",
974-
"message should instruct model to retry with _ui_submitted")
975972
})
976973

977-
t.Run("with _ui_submitted executes directly", func(t *testing.T) {
978-
request := createMCPRequest(map[string]interface{}{
974+
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
975+
request := createMCPRequestWithSession(t, "Visual Studio Code - Insiders", map[string]interface{}{
979976
"method": "create",
980977
"owner": "owner",
981978
"repo": "repo",
@@ -989,6 +986,21 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) {
989986
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
990987
"tool should return the created issue URL")
991988
})
989+
990+
t.Run("non-UI client executes directly without _ui_submitted", func(t *testing.T) {
991+
request := createMCPRequest(map[string]interface{}{
992+
"method": "create",
993+
"owner": "owner",
994+
"repo": "repo",
995+
"title": "Test",
996+
})
997+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
998+
require.NoError(t, err)
999+
1000+
textContent := getTextResult(t, result)
1001+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
1002+
"non-UI client should execute directly")
1003+
})
9921004
}
9931005

9941006
func Test_ListIssues(t *testing.T) {

pkg/github/pullrequests.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
545545
},
546546
},
547547
[]scopes.Scope{scopes.Repo},
548-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
548+
func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
549549
owner, err := RequiredParam[string](args, "owner")
550550
if err != nil {
551551
return utils.NewToolResultError(err.Error()), nil, nil
@@ -555,13 +555,13 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
555555
return utils.NewToolResultError(err.Error()), nil, nil
556556
}
557557

558-
// When insiders mode is enabled, check if this is a UI form submission.
559-
// The UI sends _ui_submitted=true to distinguish form submissions from LLM calls.
560-
// If no form is displayed, the model should call again with _ui_submitted=true.
558+
// When insiders mode is enabled and the client supports MCP Apps UI,
559+
// check if this is a UI form submission. The UI sends _ui_submitted=true
560+
// to distinguish form submissions from LLM calls.
561561
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
562562

563-
if deps.GetFlags(ctx).InsidersMode && !uiSubmitted {
564-
return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. The interactive form will be displayed. If no form appears, call this tool again with _ui_submitted set to true to proceed.", owner, repo)), nil, nil
563+
if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(req) && !uiSubmitted {
564+
return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. The user will review and confirm via the interactive form.", owner, repo)), nil, nil
565565
}
566566

567567
// When creating PR, title/head/base are required

pkg/github/pullrequests_test.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2173,8 +2173,7 @@ func Test_CreatePullRequest(t *testing.T) {
21732173
}
21742174

21752175
// Test_CreatePullRequest_InsidersMode_UIGate verifies the insiders mode UI gate
2176-
// behavior: the first call returns a form-ready message, and a subsequent
2177-
// call with _ui_submitted=true executes the action.
2176+
// behavior: UI clients get a form message, non-UI clients execute directly.
21782177
func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) {
21792178
t.Parallel()
21802179

@@ -2200,8 +2199,8 @@ func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) {
22002199
}
22012200
handler := serverTool.Handler(deps)
22022201

2203-
t.Run("without _ui_submitted returns form message", func(t *testing.T) {
2204-
request := createMCPRequest(map[string]interface{}{
2202+
t.Run("UI client without _ui_submitted returns form message", func(t *testing.T) {
2203+
request := createMCPRequestWithSession(t, "Visual Studio Code", map[string]interface{}{
22052204
"owner": "owner",
22062205
"repo": "repo",
22072206
"title": "Test PR",
@@ -2213,12 +2212,10 @@ func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) {
22132212

22142213
textContent := getTextResult(t, result)
22152214
assert.Contains(t, textContent.Text, "Ready to create a pull request")
2216-
assert.Contains(t, textContent.Text, "_ui_submitted",
2217-
"message should instruct model to retry with _ui_submitted")
22182215
})
22192216

2220-
t.Run("with _ui_submitted executes directly", func(t *testing.T) {
2221-
request := createMCPRequest(map[string]interface{}{
2217+
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
2218+
request := createMCPRequestWithSession(t, "Visual Studio Code", map[string]interface{}{
22222219
"owner": "owner",
22232220
"repo": "repo",
22242221
"title": "Test PR",
@@ -2233,6 +2230,22 @@ func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) {
22332230
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
22342231
"tool should return the created PR URL")
22352232
})
2233+
2234+
t.Run("non-UI client executes directly without _ui_submitted", func(t *testing.T) {
2235+
request := createMCPRequest(map[string]interface{}{
2236+
"owner": "owner",
2237+
"repo": "repo",
2238+
"title": "Test PR",
2239+
"head": "feature",
2240+
"base": "main",
2241+
})
2242+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2243+
require.NoError(t, err)
2244+
2245+
textContent := getTextResult(t, result)
2246+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
2247+
"non-UI client should execute directly")
2248+
})
22362249
}
22372250

22382251
func TestCreateAndSubmitPullRequestReview(t *testing.T) {

pkg/github/ui_capability.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,26 @@ package github
22

33
import "github.com/modelcontextprotocol/go-sdk/mcp"
44

5-
// UIExtensionID is the MCP Apps extension identifier used for capability negotiation.
6-
// Clients advertise MCP Apps support by including this key in their capabilities.
7-
// See: https://github.com/modelcontextprotocol/ext-apps
8-
const UIExtensionID = "io.modelcontextprotocol/ui"
9-
10-
// clientSupportsUI checks whether the client that sent this request supports
11-
// MCP Apps UI rendering. It inspects the client's experimental capabilities
12-
// for the MCP Apps extension identifier.
5+
// uiSupportedClients lists client names (from ClientInfo.Name) known to
6+
// support MCP Apps UI rendering.
137
//
14-
// When the client does not support MCP Apps, tools should skip any UI-gated
15-
// flow (e.g., interactive forms) and execute the action directly.
8+
// This is a temporary workaround until the Go SDK adds an Extensions field
9+
// to ClientCapabilities (see https://github.com/modelcontextprotocol/go-sdk/issues/777).
10+
// Once that lands, detection should use capabilities.extensions instead.
11+
var uiSupportedClients = map[string]bool{
12+
"Visual Studio Code - Insiders": true,
13+
"Visual Studio Code": true,
14+
}
15+
16+
// clientSupportsUI reports whether the MCP client that sent this request
17+
// supports MCP Apps UI rendering, based on its ClientInfo.Name.
1618
func clientSupportsUI(req *mcp.CallToolRequest) bool {
1719
if req == nil || req.Session == nil {
1820
return false
1921
}
2022
params := req.Session.InitializeParams()
21-
if params == nil || params.Capabilities == nil {
23+
if params == nil || params.ClientInfo == nil {
2224
return false
2325
}
24-
_, ok := params.Capabilities.Experimental[UIExtensionID]
25-
return ok
26+
return uiSupportedClients[params.ClientInfo.Name]
2627
}

pkg/github/ui_capability_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/modelcontextprotocol/go-sdk/mcp"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func Test_clientSupportsUI(t *testing.T) {
12+
t.Parallel()
13+
14+
tests := []struct {
15+
name string
16+
clientName string
17+
want bool
18+
}{
19+
{name: "VS Code Insiders", clientName: "Visual Studio Code - Insiders", want: true},
20+
{name: "VS Code Stable", clientName: "Visual Studio Code", want: true},
21+
{name: "unknown client", clientName: "some-other-client", want: false},
22+
{name: "empty client name", clientName: "", want: false},
23+
}
24+
25+
for _, tt := range tests {
26+
t.Run(tt.name, func(t *testing.T) {
27+
req := createMCPRequestWithSession(t, tt.clientName, nil)
28+
assert.Equal(t, tt.want, clientSupportsUI(&req))
29+
})
30+
}
31+
32+
t.Run("nil request", func(t *testing.T) {
33+
assert.False(t, clientSupportsUI(nil))
34+
})
35+
36+
t.Run("nil session", func(t *testing.T) {
37+
req := createMCPRequest(nil)
38+
assert.False(t, clientSupportsUI(&req))
39+
})
40+
}
41+
42+
func Test_clientSupportsUI_nilClientInfo(t *testing.T) {
43+
t.Parallel()
44+
45+
srv := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
46+
st, _ := mcp.NewInMemoryTransports()
47+
session, err := srv.Connect(context.Background(), st, &mcp.ServerSessionOptions{
48+
State: &mcp.ServerSessionState{
49+
InitializeParams: &mcp.InitializeParams{
50+
ClientInfo: nil,
51+
},
52+
},
53+
})
54+
if err != nil {
55+
t.Fatal(err)
56+
}
57+
t.Cleanup(func() { _ = session.Close() })
58+
59+
req := mcp.CallToolRequest{Session: session}
60+
assert.False(t, clientSupportsUI(&req))
61+
}

0 commit comments

Comments
 (0)