return to prev non ui check

Author: mattdholloway

internal/ghmcp/server.go

@@ -347,6 +347,15 @@ func addUserAgentsMiddleware(cfg github.MCPServerConfig, restClient *gogithub.Cl
 			}
 
 			message := initializeRequest
+
+			// Debug log client capabilities for MCP Apps support diagnosis
+			if message.Params.Capabilities != nil {
+				slog.Info("client capabilities",
+					"experimental", message.Params.Capabilities.Experimental,
+					"client", message.Params.ClientInfo.Name,
+				)
+			}
+
 			userAgent := fmt.Sprintf(
 				"github-mcp-server/%s (%s/%s)",
 				cfg.Version,

pkg/github/issues.go

@@ -1086,7 +1086,7 @@ Options are:
 			},
 		},
 		[]scopes.Scope{scopes.Repo},
-		func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
+		func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
 			method, err := RequiredParam[string](args, "method")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
@@ -1101,22 +1101,21 @@ Options are:
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			// When insiders mode is enabled and the client supports MCP Apps UI,
-			// check if this is a UI form submission. The UI sends _ui_submitted=true
-			// to distinguish form submissions from LLM calls. Without this flag,
-			// show the UI so the user can review/edit before submitting.
-			// Clients without MCP Apps support skip this gate and execute directly.
+			// When insiders mode is enabled, check if this is a UI form submission.
+			// The UI sends _ui_submitted=true to distinguish form submissions from LLM calls.
+			// Without this flag, return a message so the client can show a form.
+			// If no form is displayed, the model should call again with _ui_submitted=true.
 			uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
 
-			if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(req) && !uiSubmitted {
+			if deps.GetFlags(ctx).InsidersMode && !uiSubmitted {
 				if method == "update" {
 					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. The interactive form will be displayed.", issueNumber, owner, repo)), nil, nil
+					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
 				}
-				return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. The interactive form will be displayed.", owner, repo)), nil, nil
+				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
 			}
 
 			title, err := OptionalParam[string](args, "title")

pkg/github/issues_test.go

@@ -933,11 +933,10 @@ func Test_CreateIssue(t *testing.T) {
 	}
 }
 
-// Test_IssueWrite_InsidersMode_NoUISupport verifies that when insiders mode is
-// enabled but the client does not support MCP Apps UI (no session or no UI
-// capability), the tool executes directly instead of returning a "Ready to..."
-// form message.
-func Test_IssueWrite_InsidersMode_NoUISupport(t *testing.T) {
+// Test_IssueWrite_InsidersMode_UIGate verifies the insiders mode UI gate
+// behavior: the first call returns a form-ready message, and a subsequent
+// call with _ui_submitted=true executes the action.
+func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) {
 	t.Parallel()
 
 	mockIssue := &github.Issue{
@@ -959,22 +958,37 @@ func Test_IssueWrite_InsidersMode_NoUISupport(t *testing.T) {
 	}
 	handler := serverTool.Handler(deps)
 
-	// Request has no session — simulates a client without MCP Apps support
-	request := createMCPRequest(map[string]interface{}{
-		"method": "create",
-		"owner":  "owner",
-		"repo":   "repo",
-		"title":  "Test",
+	t.Run("without _ui_submitted returns form message", func(t *testing.T) {
+		request := createMCPRequest(map[string]interface{}{
+			"method": "create",
+			"owner":  "owner",
+			"repo":   "repo",
+			"title":  "Test",
+		})
+		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")
+		assert.Contains(t, textContent.Text, "_ui_submitted",
+			"message should instruct model to retry with _ui_submitted")
+	})
+
+	t.Run("with _ui_submitted executes directly", func(t *testing.T) {
+		request := createMCPRequest(map[string]interface{}{
+			"method":        "create",
+			"owner":         "owner",
+			"repo":          "repo",
+			"title":         "Test",
+			"_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",
+			"tool should return the created issue URL")
 	})
-	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-	require.NoError(t, err)
-	require.NotNil(t, result)
-
-	textContent := getTextResult(t, result)
-	assert.NotContains(t, textContent.Text, "interactive form will be displayed",
-		"tool should execute directly when client has no MCP Apps support")
-	assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
-		"tool should return the created issue URL")
 }
 
 func Test_ListIssues(t *testing.T) {

pkg/github/pullrequests.go

@@ -545,7 +545,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
 			},
 		},
 		[]scopes.Scope{scopes.Repo},
-		func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
+		func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
 			owner, err := RequiredParam[string](args, "owner")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
@@ -555,14 +555,13 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			// When insiders mode is enabled and the client supports MCP Apps UI,
-			// check if this is a UI form submission. The UI sends _ui_submitted=true
-			// to distinguish form submissions from LLM calls.
-			// Clients without MCP Apps support skip this gate and execute directly.
+			// When insiders mode is enabled, check if this is a UI form submission.
+			// The UI sends _ui_submitted=true to distinguish form submissions from LLM calls.
+			// If no form is displayed, the model should call again with _ui_submitted=true.
 			uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
 
-			if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(req) && !uiSubmitted {
-				return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. The interactive form will be displayed.", owner, repo)), nil, nil
+			if deps.GetFlags(ctx).InsidersMode && !uiSubmitted {
+				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
 			}
 
 			// When creating PR, title/head/base are required

pkg/github/pullrequests_test.go

@@ -2172,10 +2172,10 @@ func Test_CreatePullRequest(t *testing.T) {
 	}
 }
 
-// Test_CreatePullRequest_InsidersMode_NoUISupport verifies that when insiders
-// mode is enabled but the client does not support MCP Apps UI, the tool
-// executes directly instead of returning a "Ready to..." form message.
-func Test_CreatePullRequest_InsidersMode_NoUISupport(t *testing.T) {
+// Test_CreatePullRequest_InsidersMode_UIGate verifies the insiders mode UI gate
+// behavior: the first call returns a form-ready message, and a subsequent
+// call with _ui_submitted=true executes the action.
+func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) {
 	t.Parallel()
 
 	mockPR := &github.PullRequest{
@@ -2200,23 +2200,39 @@ func Test_CreatePullRequest_InsidersMode_NoUISupport(t *testing.T) {
 	}
 	handler := serverTool.Handler(deps)
 
-	// Request has no session — simulates a client without MCP Apps support
-	request := createMCPRequest(map[string]interface{}{
-		"owner": "owner",
-		"repo":  "repo",
-		"title": "Test PR",
-		"head":  "feature",
-		"base":  "main",
+	t.Run("without _ui_submitted returns form message", func(t *testing.T) {
+		request := createMCPRequest(map[string]interface{}{
+			"owner": "owner",
+			"repo":  "repo",
+			"title": "Test PR",
+			"head":  "feature",
+			"base":  "main",
+		})
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+
+		textContent := getTextResult(t, result)
+		assert.Contains(t, textContent.Text, "Ready to create a pull request")
+		assert.Contains(t, textContent.Text, "_ui_submitted",
+			"message should instruct model to retry with _ui_submitted")
+	})
+
+	t.Run("with _ui_submitted executes directly", func(t *testing.T) {
+		request := createMCPRequest(map[string]interface{}{
+			"owner":         "owner",
+			"repo":          "repo",
+			"title":         "Test PR",
+			"head":          "feature",
+			"base":          "main",
+			"_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/pull/42",
+			"tool should return the created PR URL")
 	})
-	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-	require.NoError(t, err)
-	require.NotNil(t, result)
-
-	textContent := getTextResult(t, result)
-	assert.NotContains(t, textContent.Text, "interactive form will be displayed",
-		"tool should execute directly when client has no MCP Apps support")
-	assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
-		"tool should return the created PR URL")
 }
 
 func TestCreateAndSubmitPullRequestReview(t *testing.T) {