diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 9709a852c..05af64cab 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1080,13 +1080,19 @@ Options are: if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(ctx, req) && !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 + // 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 } - 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 + } 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 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 e78a03fcb..d06721be7 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1000,6 +1000,112 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) { assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", "non-UI client should execute directly") }) + + t.Run("UI client with state change skips form and executes directly", func(t *testing.T) { + mockBaseIssue := &github.Issue{ + Number: github.Ptr(1), + Title: github.Ptr("Test"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/1"), + } + issueIDQueryResponse := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "id": "I_kwDOA0xdyM50BPaO", + }, + }, + }) + closeSuccessResponse := githubv4mock.DataResponse(map[string]any{ + "closeIssue": map[string]any{ + "issue": map[string]any{ + "id": "I_kwDOA0xdyM50BPaO", + "number": 1, + "url": "https://github.com/owner/repo/issues/1", + "state": "CLOSED", + }, + }, + }) + completedReason := IssueClosedStateReasonCompleted + + closeClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockBaseIssue), + })) + closeGQLClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + 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(1), + }, + issueIDQueryResponse, + ), + githubv4mock.NewMutationMatcher( + struct { + CloseIssue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + State githubv4.String + } + } `graphql:"closeIssue(input: $input)"` + }{}, + CloseIssueInput{ + IssueID: "I_kwDOA0xdyM50BPaO", + StateReason: &completedReason, + }, + nil, + closeSuccessResponse, + ), + )) + + closeDeps := BaseDeps{ + Client: closeClient, + GQLClient: closeGQLClient, + Flags: FeatureFlags{InsidersMode: true}, + } + closeHandler := serverTool.Handler(closeDeps) + + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "completed", + }) + result, err := closeHandler(ContextWithDeps(context.Background(), closeDeps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, "Ready to update issue", + "state change should skip UI form") + assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", + "state change should execute directly and return issue URL") + }) + + t.Run("UI client update without state change returns form message", func(t *testing.T) { + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "title": "New Title", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "Ready to update issue #1", + "update without state should show UI form") + }) } func Test_ListIssues(t *testing.T) {