From 0d378535ecad6cb699042ec379e1c768308b77ed Mon Sep 17 00:00:00 2001 From: ex-devo <183700570+ex-devo@users.noreply.github.com> Date: Tue, 28 Apr 2026 08:39:09 +0200 Subject: [PATCH 1/2] feat(issues): add native issue dependency tools - Adds issue dependency read operations so agents can inspect blockers and downstream blocked work through the issue read tool - Adds issue dependency write support so agents can create and remove native blocked-by relationships without relying on issue comments - Registers the dependency tool and updates tests and tool snapshots so the GitHub workflow exposes blocker management consistently --- .../__toolsnaps__/issue_dependency_write.snap | 43 +++ pkg/github/__toolsnaps__/issue_read.snap | 6 +- pkg/github/helper_test.go | 22 +- pkg/github/issues.go | 224 +++++++++++++- pkg/github/issues_test.go | 281 +++++++++++++++++- pkg/github/tools.go | 1 + 6 files changed, 562 insertions(+), 15 deletions(-) create mode 100644 pkg/github/__toolsnaps__/issue_dependency_write.snap diff --git a/pkg/github/__toolsnaps__/issue_dependency_write.snap b/pkg/github/__toolsnaps__/issue_dependency_write.snap new file mode 100644 index 0000000000..1462b1148d --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_dependency_write.snap @@ -0,0 +1,43 @@ +{ + "annotations": { + "title": "Change issue dependency" + }, + "description": "Add or remove issue dependencies in a GitHub repository.", + "inputSchema": { + "properties": { + "issue_id": { + "description": "The ID of the related issue to add or remove as a blocking dependency", + "type": "number" + }, + "issue_number": { + "description": "The issue number to modify", + "type": "number" + }, + "method": { + "description": "The action to perform on an issue dependency.\nOptions are:\n- 'add_blocked_by' - add a blocking dependency to the issue.\n- 'remove_blocked_by' - remove a blocking dependency from the issue.\n", + "enum": [ + "add_blocked_by", + "remove_blocked_by" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "issue_number", + "issue_id" + ], + "type": "object" + }, + "name": "issue_dependency_write" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/issue_read.snap b/pkg/github/__toolsnaps__/issue_read.snap index 21aa361f5e..6e5cf859bf 100644 --- a/pkg/github/__toolsnaps__/issue_read.snap +++ b/pkg/github/__toolsnaps__/issue_read.snap @@ -11,12 +11,14 @@ "type": "number" }, "method": { - "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get details of a specific issue.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues of the issue.\n4. get_labels - Get labels assigned to the issue.\n", + "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get details of a specific issue.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues of the issue.\n4. get_labels - Get labels assigned to the issue.\n5. get_dependencies_blocked_by - Get dependencies that block the issue.\n6. get_dependencies_blocking - Get dependencies the issue is blocking.\n", "enum": [ "get", "get_comments", "get_sub_issues", - "get_labels" + "get_labels", + "get_dependencies_blocked_by", + "get_dependencies_blocking" ], "type": "string" }, diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index ff752f5f37..1c87dd5db3 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -54,15 +54,19 @@ const ( GetReposCommitsCheckRunsByOwnerByRepoByRef = "GET /repos/{owner}/{repo}/commits/{ref}/check-runs" // Issues endpoints - GetReposIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}" - GetReposIssuesCommentsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" - PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" - PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" - PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" - GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" - PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" - DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/sub_issue" - PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}/sub_issues/priority" + GetReposIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}" + GetReposIssuesCommentsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" + PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" + PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" + PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" + GetReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by" + PostReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by" + DeleteReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumberByIssueID = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by/{issue_id}" + GetReposIssuesDependenciesBlockingByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocking" + GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" + PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" + DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/sub_issue" + PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}/sub_issues/priority" // Pull request endpoints GetReposPullsByOwnerByRepo = "GET /repos/{owner}/{repo}/pulls" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 81161626bb..99959b4f31 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -214,8 +214,10 @@ Options are: 2. get_comments - Get issue comments. 3. get_sub_issues - Get sub-issues of the issue. 4. get_labels - Get labels assigned to the issue. +5. get_dependencies_blocked_by - Get dependencies that block the issue. +6. get_dependencies_blocking - Get dependencies the issue is blocking. `, - Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels"}, + Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels", "get_dependencies_blocked_by", "get_dependencies_blocking"}, }, "owner": { Type: "string", @@ -293,6 +295,12 @@ Options are: case "get_labels": result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber) return result, nil, err + case "get_dependencies_blocked_by": + result, err := GetIssueDependenciesBlockedBy(ctx, client, deps, owner, repo, issueNumber, pagination) + return result, nil, err + case "get_dependencies_blocking": + result, err := GetIssueDependenciesBlocking(ctx, client, deps, owner, repo, issueNumber, pagination) + return result, nil, err default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -477,6 +485,86 @@ func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependenc return utils.NewToolResultText(string(r)), nil } +func GetIssueDependenciesBlockedBy(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { + return getIssueDependencies(ctx, client, deps, owner, repo, issueNumber, "blocked_by", pagination) +} + +func GetIssueDependenciesBlocking(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { + return getIssueDependencies(ctx, client, deps, owner, repo, issueNumber, "blocking", pagination) +} + +func getIssueDependencies(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, relation string, pagination PaginationParams) (*mcp.CallToolResult, error) { + cache, err := deps.GetRepoAccessCache(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get repo access cache: %w", err) + } + flags := deps.GetFlags(ctx) + + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/%s", owner, repo, issueNumber, relation) + req, err := client.NewRequest(http.MethodGet, path, nil) + if err != nil { + return nil, fmt.Errorf("failed to create dependencies request: %w", err) + } + + q := req.URL.Query() + if pagination.Page != 0 { + q.Set("page", fmt.Sprintf("%d", pagination.Page)) + } + if pagination.PerPage != 0 { + q.Set("per_page", fmt.Sprintf("%d", pagination.PerPage)) + } + req.URL.RawQuery = q.Encode() + + var issues []*github.Issue + resp, err := client.Do(ctx, req, &issues) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list issue dependencies", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list issue dependencies", resp, body), nil + } + + if flags.LockdownMode { + if cache == nil { + return nil, fmt.Errorf("lockdown cache is not configured") + } + filteredIssues := make([]*github.Issue, 0, len(issues)) + for _, issue := range issues { + user := issue.GetUser() + if user == nil { + continue + } + login := user.GetLogin() + if login == "" { + continue + } + isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil + } + if isSafeContent { + filteredIssues = append(filteredIssues, issue) + } + } + issues = filteredIssues + } + + minimalIssues := make([]MinimalIssue, 0, len(issues)) + for _, issue := range issues { + if issue != nil { + minimalIssues = append(minimalIssues, convertToMinimalIssue(issue)) + } + } + + return MarshalledTextResult(minimalIssues), nil +} + func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { // Get current labels on the issue using GraphQL var query struct { @@ -675,6 +763,140 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }) } +// IssueDependencyWrite creates a tool to add or remove dependency edges for an issue. +func IssueDependencyWrite(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "issue_dependency_write", + Description: t("TOOL_ISSUE_DEPENDENCY_WRITE_DESCRIPTION", "Add or remove issue dependencies in a GitHub repository."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ISSUE_DEPENDENCY_WRITE_USER_TITLE", "Change issue dependency"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: `The action to perform on an issue dependency. +Options are: +- 'add_blocked_by' - add a blocking dependency to the issue. +- 'remove_blocked_by' - remove a blocking dependency from the issue. +`, + Enum: []any{"add_blocked_by", "remove_blocked_by"}, + }, + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The issue number to modify", + }, + "issue_id": { + Type: "number", + Description: "The ID of the related issue to add or remove as a blocking dependency", + }, + }, + Required: []string{"method", "owner", "repo", "issue_number", "issue_id"}, + }, + }, + []scopes.Scope{scopes.Repo}, + 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 + } + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueID, err := RequiredInt(args, "issue_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + switch strings.ToLower(method) { + case "add_blocked_by": + result, err := AddIssueDependencyBlockedBy(ctx, client, owner, repo, issueNumber, issueID) + return result, nil, err + case "remove_blocked_by": + result, err := RemoveIssueDependencyBlockedBy(ctx, client, owner, repo, issueNumber, issueID) + return result, nil, err + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil + } + }) +} + +func AddIssueDependencyBlockedBy(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, issueID int) (*mcp.CallToolResult, error) { + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/blocked_by", owner, repo, issueNumber) + req, err := client.NewRequest(http.MethodPost, path, map[string]int{"issue_id": issueID}) + if err != nil { + return nil, fmt.Errorf("failed to create add dependency request: %w", err) + } + + var issue github.Issue + resp, err := client.Do(ctx, req, &issue) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add issue dependency", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add issue dependency", resp, body), nil + } + + return MarshalledTextResult(convertToMinimalIssue(&issue)), nil +} + +func RemoveIssueDependencyBlockedBy(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, issueID int) (*mcp.CallToolResult, error) { + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/blocked_by/%d", owner, repo, issueNumber, issueID) + req, err := client.NewRequest(http.MethodDelete, path, nil) + if err != nil { + return nil, fmt.Errorf("failed to create remove dependency request: %w", err) + } + + var issue github.Issue + resp, err := client.Do(ctx, req, &issue) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove issue dependency", resp, err), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to remove issue dependency", resp, body), nil + } + + return MarshalledTextResult(convertToMinimalIssue(&issue)), nil +} + // SubIssueWrite creates a tool to add a sub-issue to a parent issue. func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 9c20824746..ee43f02936 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2197,7 +2197,6 @@ func Test_AddSubIssue(t *testing.T) { }, }, } - tests := []struct { name string mockedClient *http.Client @@ -2621,6 +2620,284 @@ func Test_GetSubIssues(t *testing.T) { } } +func Test_GetIssueDependenciesBlockedBy(t *testing.T) { + serverTool := IssueRead(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + mockIssues := []*github.Issue{ + { + Number: github.Ptr(101), + Title: github.Ptr("Blocking issue 1"), + Body: github.Ptr("First blocking dependency"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/101"), + User: &github.User{Login: github.Ptr("user1")}, + }, + { + Number: github.Ptr(102), + Title: github.Ptr("Blocking issue 2"), + Body: github.Ptr("Second blocking dependency"), + State: github.Ptr("closed"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/102"), + User: &github.User{Login: github.Ptr("user2")}, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectedIssues []*github.Issue + expectedErrMsg string + }{ + { + name: "successful blocked_by dependency listing", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssues), + }), + requestArgs: map[string]any{ + "method": "get_dependencies_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectedIssues: mockIssues, + }, + { + name: "successful blocked_by dependency listing with pagination", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber: expectQueryParams(t, map[string]string{ + "page": "3", + "per_page": "25", + }).andThen(mockResponse(t, http.StatusOK, mockIssues)), + }), + requestArgs: map[string]any{ + "method": "get_dependencies_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "page": float64(3), + "perPage": float64(25), + }, + expectedIssues: mockIssues, + }, + { + name: "blocked_by dependency listing not found", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + }), + requestArgs: map[string]any{ + "method": "get_dependencies_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + }, + expectedErrMsg: "failed to list issue dependencies", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + gqlClient := githubv4.NewClient(nil) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(gqlClient, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.requestArgs) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectedErrMsg != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + textContent := getTextResult(t, result) + var returnedIssues []MinimalIssue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssues) + require.NoError(t, err) + require.Len(t, returnedIssues, len(tc.expectedIssues)) + for i, expected := range tc.expectedIssues { + assert.Equal(t, expected.GetNumber(), returnedIssues[i].Number) + assert.Equal(t, expected.GetTitle(), returnedIssues[i].Title) + assert.Equal(t, expected.GetBody(), returnedIssues[i].Body) + assert.Equal(t, expected.GetState(), returnedIssues[i].State) + assert.Equal(t, expected.GetHTMLURL(), returnedIssues[i].HTMLURL) + assert.Equal(t, expected.GetUser().GetLogin(), returnedIssues[i].User.Login) + } + }) + } +} + +func Test_GetIssueDependenciesBlocking(t *testing.T) { + serverTool := IssueRead(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + mockIssues := []*github.Issue{ + { + Number: github.Ptr(201), + Title: github.Ptr("Blocked issue 1"), + Body: github.Ptr("First issue being blocked"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/201"), + User: &github.User{Login: github.Ptr("user3")}, + }, + } + + client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesDependenciesBlockingByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssues), + })) + gqlClient := githubv4.NewClient(nil) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(gqlClient, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_dependencies_blocking", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + var returnedIssues []MinimalIssue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssues) + require.NoError(t, err) + require.Len(t, returnedIssues, 1) + assert.Equal(t, mockIssues[0].GetNumber(), returnedIssues[0].Number) + assert.Equal(t, mockIssues[0].GetTitle(), returnedIssues[0].Title) + assert.Equal(t, mockIssues[0].GetUser().GetLogin(), returnedIssues[0].User.Login) +} + +func Test_IssueDependencyWrite(t *testing.T) { + serverTool := IssueDependencyWrite(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "issue_dependency_write", tool.Name) + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "method") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "owner") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "repo") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_number") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_id") + assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo", "issue_number", "issue_id"}) + + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent issue"), + Body: github.Ptr("Dependency edge updated"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{Login: github.Ptr("maintainer")}, + } + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful add blocked_by dependency", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{"issue_id": float64(101)}).andThen( + mockResponse(t, http.StatusCreated, mockIssue), + ), + }), + requestArgs: map[string]any{ + "method": "add_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "issue_id": float64(101), + }, + expectedIssue: mockIssue, + }, + { + name: "successful remove blocked_by dependency", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumberByIssueID: mockResponse(t, http.StatusOK, mockIssue), + }), + requestArgs: map[string]any{ + "method": "remove_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "issue_id": float64(101), + }, + expectedIssue: mockIssue, + }, + { + name: "add blocked_by dependency failure", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusUnprocessableEntity, `{"message":"Validation failed"}`), + }), + requestArgs: map[string]any{ + "method": "add_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "issue_id": float64(42), + }, + expectedErrMsg: "failed to add issue dependency", + }, + { + name: "missing issue_id parameter", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "remove_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectedErrMsg: "missing required parameter: issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.requestArgs) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectedErrMsg != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + textContent := getTextResult(t, result) + var returnedIssue MinimalIssue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.Number) + assert.Equal(t, tc.expectedIssue.GetTitle(), returnedIssue.Title) + assert.Equal(t, tc.expectedIssue.GetBody(), returnedIssue.Body) + assert.Equal(t, tc.expectedIssue.GetState(), returnedIssue.State) + assert.Equal(t, tc.expectedIssue.GetHTMLURL(), returnedIssue.HTMLURL) + assert.Equal(t, tc.expectedIssue.GetUser().GetLogin(), returnedIssue.User.Login) + }) + } +} + func Test_RemoveSubIssue(t *testing.T) { // Verify tool definition once serverTool := SubIssueWrite(translations.NullTranslationHelper) @@ -2654,7 +2931,6 @@ func Test_RemoveSubIssue(t *testing.T) { }, }, } - tests := []struct { name string mockedClient *http.Client @@ -2862,7 +3138,6 @@ func Test_ReprioritizeSubIssue(t *testing.T) { }, }, } - tests := []struct { name string mockedClient *http.Client diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 559088f6d6..23d36e0b85 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -210,6 +210,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListIssueTypes(t), IssueWrite(t), AddIssueComment(t), + IssueDependencyWrite(t), SubIssueWrite(t), // User tools From 0639436bd12bbe0aa4ac583f6ce0d477f4658c6b Mon Sep 17 00:00:00 2001 From: ex-devo <183700570+ex-devo@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:55:19 +0200 Subject: [PATCH 2/2] feat(github): streamline issue project workflow tools - Adds explicit issue and project item identifiers so workflow agents can connect issues, project items, sub-issues, and dependencies without parsing ambiguous response IDs - Adds safe issue label mutation so readiness labels can be added and removed without replacing unrelated labels - Adds project item updates by issue or pull request identity with batched field updates so Status and Priority changes no longer require brittle project item lookup steps - Adds project issue creation that creates the issue, adds it to the project, and applies initial Status and Priority for the common backlog workflow - Updates tool metadata and tests to document the safer GitHub workflow surface and preserve existing compatible paths --- .../__toolsnaps__/create_project_issue.snap | 90 +++ .../__toolsnaps__/issue_label_write.snap | 46 ++ pkg/github/__toolsnaps__/label_write.snap | 4 +- pkg/github/__toolsnaps__/projects_write.snap | 13 +- pkg/github/helper_test.go | 3 + pkg/github/issues.go | 136 +++- pkg/github/issues_granular.go | 20 +- pkg/github/issues_test.go | 102 ++- pkg/github/labels.go | 2 +- pkg/github/minimal_types.go | 47 +- pkg/github/projects.go | 643 ++++++++++++++++-- pkg/github/projects_test.go | 286 +++++++- pkg/github/tools.go | 2 + 13 files changed, 1321 insertions(+), 73 deletions(-) create mode 100644 pkg/github/__toolsnaps__/create_project_issue.snap create mode 100644 pkg/github/__toolsnaps__/issue_label_write.snap diff --git a/pkg/github/__toolsnaps__/create_project_issue.snap b/pkg/github/__toolsnaps__/create_project_issue.snap new file mode 100644 index 0000000000..074ad99d90 --- /dev/null +++ b/pkg/github/__toolsnaps__/create_project_issue.snap @@ -0,0 +1,90 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Create project issue" + }, + "description": "Create a GitHub issue, add it to a GitHub Project, and set initial project Status and Priority fields.", + "inputSchema": { + "properties": { + "assignees": { + "description": "GitHub usernames to assign to this issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "body": { + "description": "Issue body content", + "type": "string" + }, + "labels": { + "description": "Labels to apply to the issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "priority_field_id": { + "description": "Project Priority field ID.", + "type": "number" + }, + "priority_value": { + "description": "Initial Priority field value or option ID.", + "type": "string" + }, + "project_number": { + "description": "The project's number.", + "type": "number" + }, + "project_owner": { + "description": "Project owner login. Defaults to the repository owner when omitted.", + "type": "string" + }, + "project_owner_type": { + "description": "Project owner type.", + "enum": [ + "user", + "org" + ], + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "status_field_id": { + "description": "Project Status field ID.", + "type": "number" + }, + "status_value": { + "description": "Initial Status field value or option ID. For github-workflow this should be Backlog's option ID.", + "type": "string" + }, + "title": { + "description": "Issue title", + "type": "string" + }, + "type": { + "description": "Issue type name, when the repository supports issue types", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "title", + "project_number", + "status_field_id", + "status_value", + "priority_field_id", + "priority_value" + ], + "type": "object" + }, + "name": "create_project_issue" +} diff --git a/pkg/github/__toolsnaps__/issue_label_write.snap b/pkg/github/__toolsnaps__/issue_label_write.snap new file mode 100644 index 0000000000..04baee6202 --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_label_write.snap @@ -0,0 +1,46 @@ +{ + "annotations": { + "title": "Change issue labels" + }, + "description": "Add or remove labels on an issue without replacing unrelated labels.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "Issue number to update", + "type": "number" + }, + "labels": { + "description": "Labels to add or remove", + "items": { + "type": "string" + }, + "type": "array" + }, + "method": { + "description": "The label operation to perform.", + "enum": [ + "add", + "remove" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "issue_number", + "labels" + ], + "type": "object" + }, + "name": "issue_label_write" +} diff --git a/pkg/github/__toolsnaps__/label_write.snap b/pkg/github/__toolsnaps__/label_write.snap index f0aca8cc99..b6d193ed20 100644 --- a/pkg/github/__toolsnaps__/label_write.snap +++ b/pkg/github/__toolsnaps__/label_write.snap @@ -2,7 +2,7 @@ "annotations": { "title": "Write operations on repository labels." }, - "description": "Perform write operations on repository labels. To set labels on issues, use the 'update_issue' tool.", + "description": "Perform write operations on repository labels. To add or remove labels on issues, use the 'issue_label_write' tool.", "inputSchema": { "properties": { "color": { @@ -48,4 +48,4 @@ "type": "object" }, "name": "label_write" -} \ No newline at end of file +} diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index f6d3197b84..e8a853aebc 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -15,7 +15,7 @@ "type": "number" }, "item_id": { - "description": "The project item ID. Required for 'update_project_item' and 'delete_project_item' methods.", + "description": "The numeric project item ID. Required for 'delete_project_item'. For 'update_project_item', provide this or identify the content with item_owner, item_repo, item_type, and issue_number or pull_request_number.", "type": "number" }, "item_owner": { @@ -84,8 +84,15 @@ "type": "string" }, "updated_field": { - "description": "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {\"id\": 123456, \"value\": \"New Value\"}. Required for 'update_project_item' method.", + "description": "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {\"id\": 123456, \"value\": \"New Value\"}. For 'update_project_item', provide updated_field or updated_fields.", "type": "object" + }, + "updated_fields": { + "description": "List of project field updates, each with the field ID and new value. Example: [{\"id\": 123456, \"value\": \"In Progress\"}, {\"id\": 234567, \"value\": \"P1\"}].", + "items": { + "type": "object" + }, + "type": "array" } }, "required": [ @@ -96,4 +103,4 @@ "type": "object" }, "name": "projects_write" -} \ No newline at end of file +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 1c87dd5db3..4b30738173 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -59,6 +59,9 @@ const ( PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" + GetReposIssuesLabelsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/labels" + PostReposIssuesLabelsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/labels" + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}" GetReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by" PostReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by" DeleteReposIssuesDependenciesBlockedByByOwnerByRepoByIssueNumberByIssueID = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/dependencies/blocked_by/{issue_id}" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 99959b4f31..913bf49a37 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -104,6 +104,7 @@ func getCloseStateReason(stateReason string) IssueClosedStateReason { // IssueFragment represents a fragment of an issue node in the GraphQL API. type IssueFragment struct { + ID githubv4.ID Number githubv4.Int Title githubv4.String Body githubv4.String @@ -763,6 +764,127 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool }) } +// IssueLabelWrite creates a tool to add or remove issue labels without replacing unrelated labels. +func IssueLabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "issue_label_write", + Description: t("TOOL_ISSUE_LABEL_WRITE_DESCRIPTION", "Add or remove labels on an issue without replacing unrelated labels."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ISSUE_LABEL_WRITE_USER_TITLE", "Change issue labels"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: "The label operation to perform.", + Enum: []any{"add", "remove"}, + }, + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "Issue number to update", + }, + "labels": { + Type: "array", + Description: "Labels to add or remove", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + Required: []string{"method", "owner", "repo", "issue_number", "labels"}, + }, + }, + []scopes.Scope{scopes.Repo}, + 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 + } + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + labels, err := OptionalStringArrayParam(args, "labels") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(labels) == 0 { + return utils.NewToolResultError("labels must contain at least one label"), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + switch method { + case "add": + addedLabels, resp, err := client.Issues.AddLabelsToIssue(ctx, owner, repo, issueNumber, labels) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add issue labels", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + return marshalIssueLabelWriteResponse(issueNumber, addedLabels) + case "remove": + for _, label := range labels { + resp, err := client.Issues.RemoveLabelForIssue(ctx, owner, repo, issueNumber, label) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove issue label", resp, err), nil, nil + } + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + } + remainingLabels, resp, err := client.Issues.ListLabelsByIssue(ctx, owner, repo, issueNumber, nil) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list issue labels", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + return marshalIssueLabelWriteResponse(issueNumber, remainingLabels) + default: + return utils.NewToolResultError("method must be either 'add' or 'remove'"), nil, nil + } + }, + ) +} + +func marshalIssueLabelWriteResponse(issueNumber int, labels []*github.Label) (*mcp.CallToolResult, any, error) { + labelNames := make([]string, 0, len(labels)) + for _, label := range labels { + if label != nil { + labelNames = append(labelNames, label.GetName()) + } + } + + response := map[string]any{ + "issue_number": issueNumber, + "labels": labelNames, + } + r, err := json.Marshal(response) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(r)), nil, nil +} + // IssueDependencyWrite creates a tool to add or remove dependency edges for an issue. func IssueDependencyWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( @@ -1448,8 +1570,11 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo // Return minimal response with just essential information minimalResponse := MinimalResponse{ - ID: fmt.Sprintf("%d", issue.GetID()), - URL: issue.GetHTMLURL(), + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + IssueNumber: issue.GetNumber(), + IssueID: issue.GetID(), + IssueNodeID: issue.GetNodeID(), } r, err := json.Marshal(minimalResponse) @@ -1573,8 +1698,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 // Return minimal response with just essential information minimalResponse := MinimalResponse{ - ID: fmt.Sprintf("%d", updatedIssue.GetID()), - URL: updatedIssue.GetHTMLURL(), + ID: fmt.Sprintf("%d", updatedIssue.GetID()), + URL: updatedIssue.GetHTMLURL(), + IssueNumber: updatedIssue.GetNumber(), + IssueID: updatedIssue.GetID(), + IssueNodeID: updatedIssue.GetNodeID(), } r, err := json.Marshal(minimalResponse) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index fe3b4bcc9b..e5f42a5b93 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -94,8 +94,11 @@ func issueUpdateTool( defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", issue.GetID()), - URL: issue.GetHTMLURL(), + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + IssueNumber: issue.GetNumber(), + IssueID: issue.GetID(), + IssueNodeID: issue.GetNodeID(), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil @@ -178,8 +181,11 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT defer func() { _ = resp.Body.Close() }() r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", issue.GetID()), - URL: issue.GetHTMLURL(), + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + IssueNumber: issue.GetNumber(), + IssueID: issue.GetID(), + IssueNodeID: issue.GetNodeID(), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil @@ -819,8 +825,10 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv } r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%v", mutation.SetIssueFieldValue.Issue.ID), - URL: string(mutation.SetIssueFieldValue.Issue.URL), + ID: fmt.Sprintf("%v", mutation.SetIssueFieldValue.Issue.ID), + URL: string(mutation.SetIssueFieldValue.Issue.URL), + IssueNumber: int(mutation.SetIssueFieldValue.Issue.Number), + IssueNodeID: fmt.Sprintf("%v", mutation.SetIssueFieldValue.Issue.ID), }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index ee43f02936..09ef50e91e 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -120,6 +120,8 @@ func Test_GetIssue(t *testing.T) { // Setup mock issue for success case mockIssue := &github.Issue{ + ID: github.Ptr(int64(10042)), + NodeID: github.Ptr("I_10042"), Number: github.Ptr(42), Title: github.Ptr("Test Issue"), Body: github.Ptr("This is a test issue"), @@ -136,6 +138,8 @@ func Test_GetIssue(t *testing.T) { }, } mockIssue2 := &github.Issue{ + ID: github.Ptr(int64(10422)), + NodeID: github.Ptr("I_10422"), Number: github.Ptr(422), Title: github.Ptr("Test Issue 2"), Body: github.Ptr("This is a test issue 2"), @@ -266,6 +270,9 @@ func Test_GetIssue(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.Number) + assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.IssueNumber) + assert.Equal(t, tc.expectedIssue.GetID(), returnedIssue.IssueID) + assert.Equal(t, tc.expectedIssue.GetNodeID(), returnedIssue.IssueNodeID) assert.Equal(t, tc.expectedIssue.GetTitle(), returnedIssue.Title) assert.Equal(t, tc.expectedIssue.GetBody(), returnedIssue.Body) assert.Equal(t, tc.expectedIssue.GetState(), returnedIssue.State) @@ -386,6 +393,74 @@ func Test_AddIssueComment(t *testing.T) { } } +func Test_IssueLabelWrite(t *testing.T) { + serverTool := IssueLabelWrite(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "issue_label_write", tool.Name) + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "method") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "labels") + assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo", "issue_number", "labels"}) + + t.Run("add labels preserves existing labels", func(t *testing.T) { + client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesLabelsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, []*github.Label{ + {Name: github.Ptr("type:task")}, + {Name: github.Ptr("status:ready")}, + }), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "add", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "labels": []any{"status:ready"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + textContent := getTextResult(t, result) + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.Equal(t, float64(42), response["issue_number"]) + assert.ElementsMatch(t, []any{"type:task", "status:ready"}, response["labels"]) + }) + + t.Run("remove label returns remaining labels", func(t *testing.T) { + client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }), + GetReposIssuesLabelsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, []*github.Label{ + {Name: github.Ptr("type:task")}, + }), + })) + deps := BaseDeps{Client: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "remove", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "labels": []any{"status:ready"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + textContent := getTextResult(t, result) + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.ElementsMatch(t, []any{"type:task"}, response["labels"]) + }) +} + func Test_SearchIssues(t *testing.T) { // Verify tool definition once serverTool := SearchIssues(translations.NullTranslationHelper) @@ -714,6 +789,8 @@ func Test_CreateIssue(t *testing.T) { // Setup mock issue for success case mockIssue := &github.Issue{ + ID: github.Ptr(int64(10123)), + NodeID: github.Ptr("I_10123"), Number: github.Ptr(123), Title: github.Ptr("Test Issue"), Body: github.Ptr("This is a test issue"), @@ -765,6 +842,8 @@ func Test_CreateIssue(t *testing.T) { name: "successful issue creation with minimal fields", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PostReposIssuesByOwnerByRepo: mockResponse(t, http.StatusCreated, &github.Issue{ + ID: github.Ptr(int64(10124)), + NodeID: github.Ptr("I_10124"), Number: github.Ptr(124), Title: github.Ptr("Minimal Issue"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/124"), @@ -780,6 +859,8 @@ func Test_CreateIssue(t *testing.T) { }, expectError: false, expectedIssue: &github.Issue{ + ID: github.Ptr(int64(10124)), + NodeID: github.Ptr("I_10124"), Number: github.Ptr(124), Title: github.Ptr("Minimal Issue"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/124"), @@ -845,6 +926,9 @@ func Test_CreateIssue(t *testing.T) { require.NoError(t, err) assert.Equal(t, tc.expectedIssue.GetHTMLURL(), returnedIssue.URL) + assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.IssueNumber) + assert.Equal(t, tc.expectedIssue.GetID(), returnedIssue.IssueID) + assert.Equal(t, tc.expectedIssue.GetNodeID(), returnedIssue.IssueNodeID) }) } } @@ -1047,6 +1131,7 @@ func Test_ListIssues(t *testing.T) { // Mock issues data mockIssuesAll := []map[string]any{ { + "id": "I_1001", "number": 123, "title": "First Issue", "body": "This is the first test issue", @@ -1065,6 +1150,7 @@ func Test_ListIssues(t *testing.T) { }, }, { + "id": "I_1002", "number": 456, "title": "Second Issue", "body": "This is the second test issue", @@ -1087,6 +1173,7 @@ func Test_ListIssues(t *testing.T) { mockIssuesOpen := []map[string]any{mockIssuesAll[0], mockIssuesAll[1]} mockIssuesClosed := []map[string]any{ { + "id": "I_1003", "number": 789, "title": "Closed Issue", "body": "This is a closed issue", @@ -1272,8 +1359,8 @@ func Test_ListIssues(t *testing.T) { } // Define the actual query strings that match the implementation - qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" - qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{id,number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{id,number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -1374,6 +1461,8 @@ func Test_UpdateIssue(t *testing.T) { // Mock issues for reuse across test cases mockBaseIssue := &github.Issue{ + ID: github.Ptr(int64(20123)), + NodeID: github.Ptr("I_20123"), Number: github.Ptr(123), Title: github.Ptr("Title"), Body: github.Ptr("Description"), @@ -1386,6 +1475,8 @@ func Test_UpdateIssue(t *testing.T) { } mockUpdatedIssue := &github.Issue{ + ID: github.Ptr(int64(20123)), + NodeID: github.Ptr("I_20123"), Number: github.Ptr(123), Title: github.Ptr("Updated Title"), Body: github.Ptr("Updated Description"), @@ -1399,6 +1490,8 @@ func Test_UpdateIssue(t *testing.T) { } mockReopenedIssue := &github.Issue{ + ID: github.Ptr(int64(20123)), + NodeID: github.Ptr("I_20123"), Number: github.Ptr(123), Title: github.Ptr("Title"), State: github.Ptr("open"), @@ -1689,6 +1782,8 @@ func Test_UpdateIssue(t *testing.T) { "type": "Bug", }).andThen( mockResponse(t, http.StatusOK, &github.Issue{ + ID: github.Ptr(int64(20123)), + NodeID: github.Ptr("I_20123"), Number: github.Ptr(123), Title: github.Ptr("Updated Title"), Body: github.Ptr("Updated Description"), @@ -1821,6 +1916,9 @@ func Test_UpdateIssue(t *testing.T) { require.NoError(t, err) assert.Equal(t, tc.expectedIssue.GetHTMLURL(), updateResp.URL) + assert.Equal(t, tc.expectedIssue.GetNumber(), updateResp.IssueNumber) + assert.Equal(t, tc.expectedIssue.GetID(), updateResp.IssueID) + assert.Equal(t, tc.expectedIssue.GetNodeID(), updateResp.IssueNodeID) }) } } diff --git a/pkg/github/labels.go b/pkg/github/labels.go index 0dbb622d91..e7c8d2dd63 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -215,7 +215,7 @@ func LabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool { ToolsetLabels, mcp.Tool{ Name: "label_write", - Description: t("TOOL_LABEL_WRITE_DESCRIPTION", "Perform write operations on repository labels. To set labels on issues, use the 'update_issue' tool."), + Description: t("TOOL_LABEL_WRITE_DESCRIPTION", "Perform write operations on repository labels. To add or remove labels on issues, use the 'issue_label_write' tool."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_LABEL_WRITE_TITLE", "Write operations on repository labels."), ReadOnlyHint: false, diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index a8757c51c3..49a8e86a3b 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -1,6 +1,7 @@ package github import ( + "fmt" "time" "github.com/google/go-github/v82/github" @@ -134,8 +135,27 @@ type MinimalTag struct { // Success is implicit in the HTTP response status, and all other information // can be derived from the URL or fetched separately if needed. type MinimalResponse struct { - ID string `json:"id"` - URL string `json:"url"` + ID string `json:"id"` + URL string `json:"url"` + IssueNumber int `json:"issue_number,omitempty"` + IssueID int64 `json:"issue_id,omitempty"` + IssueNodeID string `json:"issue_node_id,omitempty"` +} + +// MinimalProjectItem is the trimmed output type for Project V2 items. +type MinimalProjectItem struct { + ID any `json:"id,omitempty"` + ProjectItemID any `json:"project_item_id,omitempty"` + ProjectItemNodeID string `json:"project_item_node_id,omitempty"` + ContentType string `json:"content_type,omitempty"` + ContentOwner string `json:"content_owner,omitempty"` + ContentRepo string `json:"content_repo,omitempty"` + ContentNumber int `json:"content_number,omitempty"` + ContentID any `json:"content_id,omitempty"` + ContentNodeID string `json:"content_node_id,omitempty"` + ContentLabels []string `json:"content_labels,omitempty"` + Raw map[string]any `json:"raw,omitempty"` + Fields []map[string]any `json:"fields,omitempty"` } type MinimalProject struct { @@ -171,6 +191,9 @@ type MinimalReactions struct { // MinimalIssue is the trimmed output type for issue objects to reduce verbosity. type MinimalIssue struct { + IssueNumber int `json:"issue_number,omitempty"` + IssueID int64 `json:"issue_id,omitempty"` + IssueNodeID string `json:"issue_node_id,omitempty"` Number int `json:"number"` Title string `json:"title"` Body string `json:"body,omitempty"` @@ -321,6 +344,9 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal func convertToMinimalIssue(issue *github.Issue) MinimalIssue { m := MinimalIssue{ + IssueNumber: issue.GetNumber(), + IssueID: issue.GetID(), + IssueNodeID: issue.GetNodeID(), Number: issue.GetNumber(), Title: issue.GetTitle(), Body: issue.GetBody(), @@ -387,13 +413,16 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue { func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { m := MinimalIssue{ - Number: int(fragment.Number), - Title: sanitize.Sanitize(string(fragment.Title)), - Body: sanitize.Sanitize(string(fragment.Body)), - State: string(fragment.State), - Comments: int(fragment.Comments.TotalCount), - CreatedAt: fragment.CreatedAt.Format(time.RFC3339), - UpdatedAt: fragment.UpdatedAt.Format(time.RFC3339), + IssueNumber: int(fragment.Number), + IssueID: fragment.DatabaseID, + IssueNodeID: fmt.Sprintf("%v", fragment.ID), + Number: int(fragment.Number), + Title: sanitize.Sanitize(string(fragment.Title)), + Body: sanitize.Sanitize(string(fragment.Body)), + State: string(fragment.State), + Comments: int(fragment.Comments.TotalCount), + CreatedAt: fragment.CreatedAt.Format(time.RFC3339), + UpdatedAt: fragment.UpdatedAt.Format(time.RFC3339), User: &MinimalUser{ Login: string(fragment.Author.Login), }, diff --git a/pkg/github/projects.go b/pkg/github/projects.go index dcb9193eca..2d838661b7 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -437,7 +438,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "item_id": { Type: "number", - Description: "The project item ID. Required for 'update_project_item' and 'delete_project_item' methods.", + Description: "The numeric project item ID. Required for 'delete_project_item'. For 'update_project_item', provide this or identify the content with item_owner, item_repo, item_type, and issue_number or pull_request_number.", }, "item_type": { Type: "string", @@ -462,7 +463,12 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "updated_field": { Type: "object", - Description: "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {\"id\": 123456, \"value\": \"New Value\"}. Required for 'update_project_item' method.", + Description: "Object consisting of the ID of the project field to update and the new value for the field. To clear the field, set value to null. Example: {\"id\": 123456, \"value\": \"New Value\"}. For 'update_project_item', provide updated_field or updated_fields.", + }, + "updated_fields": { + Type: "array", + Description: "List of project field updates, each with the field ID and new value. Example: [{\"id\": 123456, \"value\": \"In Progress\"}, {\"id\": 234567, \"value\": \"P1\"}].", + Items: &jsonschema.Schema{Type: "object"}, }, "body": { Type: "string", @@ -558,19 +564,15 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return addProjectItem(ctx, gqlClient, owner, ownerType, projectNumber, itemOwner, itemRepo, itemNumber, itemType) case projectsMethodUpdateProjectItem: - itemID, err := RequiredBigInt(args, "item_id") + itemID, err := resolveProjectItemIDForUpdate(ctx, client, owner, ownerType, projectNumber, args) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - rawUpdatedField, exists := args["updated_field"] - if !exists { - return utils.NewToolResultError("missing required parameter: updated_field"), nil, nil - } - fieldValue, ok := rawUpdatedField.(map[string]any) - if !ok || fieldValue == nil { - return utils.NewToolResultError("updated_field must be an object"), nil, nil + fieldValues, err := buildProjectFieldValues(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } - return updateProjectItem(ctx, client, owner, ownerType, projectNumber, itemID, fieldValue) + return updateProjectItem(ctx, client, owner, ownerType, projectNumber, itemID, fieldValues) case projectsMethodDeleteProjectItem: itemID, err := RequiredBigInt(args, "item_id") if err != nil { @@ -603,6 +605,222 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return tool } +// CreateProjectIssue creates an issue, adds it to a Project V2 board, and sets initial project fields. +func CreateProjectIssue(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataProjects, + mcp.Tool{ + Name: "create_project_issue", + Description: t("TOOL_CREATE_PROJECT_ISSUE_DESCRIPTION", "Create a GitHub issue, add it to a GitHub Project, and set initial project Status and Priority fields."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_PROJECT_ISSUE_TITLE", "Create project issue"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "title": { + Type: "string", + Description: "Issue title", + }, + "body": { + Type: "string", + Description: "Issue body content", + }, + "labels": { + Type: "array", + Description: "Labels to apply to the issue", + Items: &jsonschema.Schema{Type: "string"}, + }, + "assignees": { + Type: "array", + Description: "GitHub usernames to assign to this issue", + Items: &jsonschema.Schema{Type: "string"}, + }, + "type": { + Type: "string", + Description: "Issue type name, when the repository supports issue types", + }, + "project_owner": { + Type: "string", + Description: "Project owner login. Defaults to the repository owner when omitted.", + }, + "project_owner_type": { + Type: "string", + Description: "Project owner type.", + Enum: []any{"user", "org"}, + }, + "project_number": { + Type: "number", + Description: "The project's number.", + }, + "status_field_id": { + Type: "number", + Description: "Project Status field ID.", + }, + "status_value": { + Type: "string", + Description: "Initial Status field value or option ID. For github-workflow this should be Backlog's option ID.", + }, + "priority_field_id": { + Type: "number", + Description: "Project Priority field ID.", + }, + "priority_value": { + Type: "string", + Description: "Initial Priority field value or option ID.", + }, + }, + Required: []string{"owner", "repo", "title", "project_number", "status_field_id", "status_value", "priority_field_id", "priority_value"}, + }, + }, + []scopes.Scope{scopes.Repo, scopes.Project}, + 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 + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + title, err := RequiredParam[string](args, "title") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, err := OptionalParam[string](args, "body") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + labels, err := OptionalStringArrayParam(args, "labels") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + assignees, err := OptionalStringArrayParam(args, "assignees") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueType, err := OptionalParam[string](args, "type") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + projectOwner, err := OptionalParam[string](args, "project_owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if projectOwner == "" { + projectOwner = owner + } + projectOwnerType, err := OptionalParam[string](args, "project_owner_type") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + projectNumber, err := RequiredInt(args, "project_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + statusFieldID, err := RequiredBigInt(args, "status_field_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + statusValue, ok := args["status_value"] + if !ok { + return utils.NewToolResultError("missing required parameter: status_value"), nil, nil + } + priorityFieldID, err := RequiredBigInt(args, "priority_field_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + priorityValue, ok := args["priority_value"] + if !ok { + return utils.NewToolResultError("missing required parameter: priority_value"), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + if projectOwnerType == "" { + projectOwnerType, err = detectOwnerType(ctx, client, projectOwner, projectNumber) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + + issueRequest := &github.IssueRequest{ + Title: github.Ptr(title), + Body: github.Ptr(body), + Labels: &labels, + Assignees: &assignees, + } + if issueType != "" { + issueRequest.Type = github.Ptr(issueType) + } + + issue, resp, err := client.Issues.Create(ctx, owner, repo, issueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusCreated { + body, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body), nil, nil + } + + addedItem, err := addProjectItemData(ctx, gqlClient, projectOwner, projectOwnerType, projectNumber, owner, repo, issue.GetNumber(), "issue") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if addedItem.ProjectItemID == 0 { + return utils.NewToolResultError("created project item did not include a numeric project_item_id"), nil, nil + } + + updateResult, _, err := updateProjectItem(ctx, client, projectOwner, projectOwnerType, projectNumber, addedItem.ProjectItemID, []map[string]any{ + {"id": statusFieldID, "value": statusValue}, + {"id": priorityFieldID, "value": priorityValue}, + }) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if updateResult != nil && updateResult.IsError { + return updateResult, nil, nil + } + + response := map[string]any{ + "issue_number": issue.GetNumber(), + "issue_id": issue.GetID(), + "issue_node_id": issue.GetNodeID(), + "html_url": issue.GetHTMLURL(), + "project_item_id": addedItem.ProjectItemID, + "project_item_node_id": addedItem.ProjectItemNodeID, + } + r, err := json.Marshal(response) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + // Helper functions for consolidated projects tools func listProjects(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) { @@ -830,8 +1048,13 @@ func listProjectItems(ctx context.Context, client *github.Client, args map[strin } defer func() { _ = resp.Body.Close() }() + items, err := convertProjectItemsToResponse(projectItems) + if err != nil { + return nil, nil, err + } + response := map[string]any{ - "items": projectItems, + "items": items, "pageInfo": buildPageInfo(resp), } @@ -949,7 +1172,12 @@ func getProjectItem(ctx context.Context, client *github.Client, owner, ownerType return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project item", resp, body), nil, nil } - r, err := json.Marshal(projectItem) + item, err := convertProjectItemToResponse(projectItem) + if err != nil { + return nil, nil, err + } + + r, err := json.Marshal(item) if err != nil { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -957,8 +1185,8 @@ func getProjectItem(ctx context.Context, client *github.Client, owner, ownerType return utils.NewToolResultText(string(r)), nil, nil } -func updateProjectItem(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, itemID int64, fieldValue map[string]any) (*mcp.CallToolResult, any, error) { - updatePayload, err := buildUpdateProjectItem(fieldValue) +func updateProjectItem(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, itemID int64, fieldValues []map[string]any) (*mcp.CallToolResult, any, error) { + updatePayload, err := buildUpdateProjectItem(fieldValues) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -988,7 +1216,12 @@ func updateProjectItem(ctx context.Context, client *github.Client, owner, ownerT } return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, ProjectUpdateFailedError, resp, body), nil, nil } - r, err := json.Marshal(updatedItem) + item, err := convertProjectItemToResponse(updatedItem) + if err != nil { + return nil, nil, err + } + + r, err := json.Marshal(item) if err != nil { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -1063,9 +1296,21 @@ func resolveProjectNodeID(ctx context.Context, gqlClient *githubv4.Client, owner } // addProjectItem adds an item to a project by resolving the issue/PR number to a node ID -func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (*mcp.CallToolResult, any, error) { +type projectItemAddResult struct { + ID string `json:"id"` + ProjectItemID int64 `json:"project_item_id,omitempty"` + ProjectItemNodeID string `json:"project_item_node_id"` + ContentType string `json:"content_type"` + ContentOwner string `json:"content_owner"` + ContentRepo string `json:"content_repo"` + ContentNumber int `json:"content_number"` + ContentNodeID string `json:"content_node_id"` + Message string `json:"message"` +} + +func addProjectItemData(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (*projectItemAddResult, error) { if itemType != "issue" && itemType != "pull_request" { - return utils.NewToolResultError("item_type must be either 'issue' or 'pull_request'"), nil, nil + return nil, fmt.Errorf("item_type must be either 'issue' or 'pull_request'") } // Resolve the item number to a node ID @@ -1077,14 +1322,15 @@ func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, owne nodeID, err = resolvePullRequestNodeID(ctx, gqlClient, itemOwner, itemRepo, itemNumber) } if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to resolve %s: %v", itemType, err)), nil, nil + return nil, fmt.Errorf("failed to resolve %s: %w", itemType, err) } // Use GraphQL to add the item to the project var mutation struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` } @@ -1092,7 +1338,7 @@ func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, owne // Resolve the project number to a node ID projectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber) if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + return nil, err } // Add the item to the project @@ -1103,12 +1349,27 @@ func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, owne err = gqlClient.Mutate(ctx, &mutation, input, nil) if err != nil { - return utils.NewToolResultError(fmt.Sprintf(ProjectAddFailedError+": %v", err)), nil, nil - } + return nil, fmt.Errorf(ProjectAddFailedError+": %w", err) + } + + itemNodeID := fmt.Sprintf("%v", mutation.AddProjectV2ItemByID.Item.ID) + return &projectItemAddResult{ + ID: itemNodeID, + ProjectItemID: int64(mutation.AddProjectV2ItemByID.Item.DatabaseID), + ProjectItemNodeID: itemNodeID, + ContentType: itemType, + ContentOwner: itemOwner, + ContentRepo: itemRepo, + ContentNumber: itemNumber, + ContentNodeID: fmt.Sprintf("%v", nodeID), + Message: fmt.Sprintf("Successfully added %s %s/%s#%d to project %s/%d", itemType, itemOwner, itemRepo, itemNumber, owner, projectNumber), + }, nil +} - result := map[string]any{ - "id": mutation.AddProjectV2ItemByID.Item.ID, - "message": fmt.Sprintf("Successfully added %s %s/%s#%d to project %s/%d", itemType, itemOwner, itemRepo, itemNumber, owner, projectNumber), +func addProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (*mcp.CallToolResult, any, error) { + result, err := addProjectItemData(ctx, gqlClient, owner, ownerType, projectNumber, itemOwner, itemRepo, itemNumber, itemType) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } r, err := json.Marshal(result) @@ -1327,32 +1588,334 @@ func validateAndConvertToInt64(value any) (int64, error) { } } -// buildUpdateProjectItem constructs UpdateProjectItemOptions from the input map. -func buildUpdateProjectItem(input map[string]any) (*github.UpdateProjectItemOptions, error) { - if input == nil { - return nil, fmt.Errorf("updated_field must be an object") +func convertProjectItemsToResponse(projectItems []*github.ProjectV2Item) ([]map[string]any, error) { + items := make([]map[string]any, 0, len(projectItems)) + for _, projectItem := range projectItems { + item, err := convertProjectItemToResponse(projectItem) + if err != nil { + return nil, err + } + items = append(items, item) } + return items, nil +} - idField, ok := input["id"] - if !ok { - return nil, fmt.Errorf("updated_field.id is required") +func convertProjectItemToResponse(projectItem any) (map[string]any, error) { + raw, err := projectItemToMap(projectItem) + if err != nil { + return nil, err } + addProjectItemIdentityFields(raw) + return raw, nil +} - fieldID, err := validateAndConvertToInt64(idField) +func projectItemToMap(projectItem any) (map[string]any, error) { + r, err := json.Marshal(projectItem) if err != nil { - return nil, fmt.Errorf("updated_field.id: %w", err) + return nil, fmt.Errorf("failed to marshal project item: %w", err) + } + + var raw map[string]any + if err := json.Unmarshal(r, &raw); err != nil { + return nil, fmt.Errorf("failed to unmarshal project item: %w", err) + } + return raw, nil +} + +func addProjectItemIdentityFields(item map[string]any) { + if id, ok := item["id"]; ok { + item["project_item_id"] = id + } + if nodeID, ok := item["node_id"].(string); ok && nodeID != "" { + item["project_item_node_id"] = nodeID + } + + content, ok := item["content"].(map[string]any) + if !ok || content == nil { + return + } + + if contentType := contentTypeFromContent(content); contentType != "" { + item["content_type"] = contentType + } + if id, ok := content["id"]; ok { + item["content_id"] = id + } + if nodeID, ok := content["node_id"].(string); ok && nodeID != "" { + item["content_node_id"] = nodeID + } + if number, ok := numberFromAny(content["number"]); ok { + item["content_number"] = number + } + if owner, repo := ownerRepoFromContent(content); owner != "" && repo != "" { + item["content_owner"] = owner + item["content_repo"] = repo + } + if labels := labelsFromContent(content); len(labels) > 0 { + item["content_labels"] = labels } +} + +func contentTypeFromContent(content map[string]any) string { + for _, key := range []string{"type", "__typename"} { + if value, ok := content[key].(string); ok && value != "" { + normalized := strings.ToLower(value) + if normalized == "pullrequest" { + return "pull_request" + } + return normalized + } + } + if htmlURL, ok := content["html_url"].(string); ok { + if strings.Contains(htmlURL, "/pull/") { + return "pull_request" + } + if strings.Contains(htmlURL, "/issues/") { + return "issue" + } + } + return "" +} + +func ownerRepoFromContent(content map[string]any) (string, string) { + if repositoryURL, ok := content["repository_url"].(string); ok && repositoryURL != "" { + if owner, repo := ownerRepoFromURL(repositoryURL, "/repos/"); owner != "" && repo != "" { + return owner, repo + } + } + if htmlURL, ok := content["html_url"].(string); ok && htmlURL != "" { + return ownerRepoFromURL(htmlURL, "github.com/") + } + return "", "" +} + +func ownerRepoFromURL(rawURL, marker string) (string, string) { + index := strings.Index(rawURL, marker) + if index == -1 { + return "", "" + } + path := strings.Trim(rawURL[index+len(marker):], "/") + parts := strings.Split(path, "/") + if len(parts) < 2 { + return "", "" + } + return parts[0], parts[1] +} - valueField, ok := input["value"] +func labelsFromContent(content map[string]any) []string { + rawLabels, ok := content["labels"].([]any) if !ok { - return nil, fmt.Errorf("updated_field.value is required") + return nil + } + labels := make([]string, 0, len(rawLabels)) + for _, rawLabel := range rawLabels { + switch label := rawLabel.(type) { + case string: + labels = append(labels, label) + case map[string]any: + if name, ok := label["name"].(string); ok && name != "" { + labels = append(labels, name) + } + } } + return labels +} - payload := &github.UpdateProjectItemOptions{ - Fields: []*github.UpdateProjectV2Field{{ +func numberFromAny(value any) (int, bool) { + switch v := value.(type) { + case float64: + return int(v), float64(int(v)) == v + case int: + return v, true + case int64: + return int(v), int64(int(v)) == v + case json.Number: + i, err := v.Int64() + return int(i), err == nil + default: + return 0, false + } +} + +func resolveProjectItemIDForUpdate(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, args map[string]any) (int64, error) { + if _, exists := args["item_id"]; exists { + return RequiredBigInt(args, "item_id") + } + + itemType, itemNumber, itemOwner, itemRepo, err := projectContentIdentityFromArgs(args) + if err != nil { + return 0, err + } + + return findProjectItemIDByContent(ctx, client, owner, ownerType, projectNumber, itemOwner, itemRepo, itemNumber, itemType) +} + +func projectContentIdentityFromArgs(args map[string]any) (string, int, string, string, error) { + itemOwner, err := RequiredParam[string](args, "item_owner") + if err != nil { + return "", 0, "", "", err + } + itemRepo, err := RequiredParam[string](args, "item_repo") + if err != nil { + return "", 0, "", "", err + } + itemType, err := OptionalParam[string](args, "item_type") + if err != nil { + return "", 0, "", "", err + } + + if itemType == "" || itemType == "issue" { + if _, exists := args["issue_number"]; exists { + itemNumber, err := RequiredInt(args, "issue_number") + return "issue", itemNumber, itemOwner, itemRepo, err + } + } + if itemType == "" || itemType == "pull_request" { + if _, exists := args["pull_request_number"]; exists { + itemNumber, err := RequiredInt(args, "pull_request_number") + return "pull_request", itemNumber, itemOwner, itemRepo, err + } + } + + return "", 0, "", "", fmt.Errorf("provide item_id or identify content with item_owner, item_repo, and issue_number or pull_request_number") +} + +func findProjectItemIDByContent(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, itemOwner, itemRepo string, itemNumber int, itemType string) (int64, error) { + opts := &github.ListProjectItemsOptions{ + ListProjectsOptions: github.ListProjectsOptions{ + ListProjectsPaginationOptions: github.ListProjectsPaginationOptions{ + PerPage: github.Ptr(MaxProjectsPerPage), + }, + }, + } + + for { + var projectItems []*github.ProjectV2Item + var resp *github.Response + var err error + if ownerType == "org" { + projectItems, resp, err = client.Projects.ListOrganizationProjectItems(ctx, owner, projectNumber, opts) + } else { + projectItems, resp, err = client.Projects.ListUserProjectItems(ctx, owner, projectNumber, opts) + } + if err != nil { + return 0, err + } + + for _, projectItem := range projectItems { + item, err := convertProjectItemToResponse(projectItem) + if err != nil { + return 0, err + } + if projectItemMatchesContent(item, itemOwner, itemRepo, itemNumber, itemType) { + id, ok := validateAndConvertProjectItemID(item["project_item_id"]) + if !ok { + return 0, fmt.Errorf("matched project item has no numeric project_item_id") + } + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + return id, nil + } + } + + next := "" + if resp != nil { + next = resp.After + if resp.Body != nil { + _ = resp.Body.Close() + } + } + if next == "" { + break + } + opts.ListProjectsOptions.ListProjectsPaginationOptions.After = github.Ptr(next) + } + + return 0, fmt.Errorf("project item not found for %s %s/%s#%d", itemType, itemOwner, itemRepo, itemNumber) +} + +func projectItemMatchesContent(item map[string]any, owner, repo string, number int, itemType string) bool { + contentOwner, _ := item["content_owner"].(string) + contentRepo, _ := item["content_repo"].(string) + contentType, _ := item["content_type"].(string) + contentNumber, ok := numberFromAny(item["content_number"]) + + return ok && + strings.EqualFold(contentOwner, owner) && + strings.EqualFold(contentRepo, repo) && + contentNumber == number && + (contentType == "" || contentType == itemType) +} + +func validateAndConvertProjectItemID(value any) (int64, bool) { + id, err := validateAndConvertToInt64(value) + return id, err == nil +} + +func buildProjectFieldValues(args map[string]any) ([]map[string]any, error) { + if rawUpdatedFields, exists := args["updated_fields"]; exists { + fields, ok := rawUpdatedFields.([]any) + if !ok || len(fields) == 0 { + return nil, fmt.Errorf("updated_fields must be a non-empty array") + } + fieldValues := make([]map[string]any, 0, len(fields)) + for _, rawField := range fields { + fieldValue, ok := rawField.(map[string]any) + if !ok || fieldValue == nil { + return nil, fmt.Errorf("updated_fields entries must be objects") + } + fieldValues = append(fieldValues, fieldValue) + } + return fieldValues, nil + } + + rawUpdatedField, exists := args["updated_field"] + if !exists { + return nil, fmt.Errorf("missing required parameter: updated_field") + } + fieldValue, ok := rawUpdatedField.(map[string]any) + if !ok || fieldValue == nil { + return nil, fmt.Errorf("updated_field must be an object") + } + return []map[string]any{fieldValue}, nil +} + +// buildUpdateProjectItem constructs UpdateProjectItemOptions from the input maps. +func buildUpdateProjectItem(inputs []map[string]any) (*github.UpdateProjectItemOptions, error) { + if len(inputs) == 0 { + return nil, fmt.Errorf("updated_field must be an object") + } + + fields := make([]*github.UpdateProjectV2Field, 0, len(inputs)) + for index, input := range inputs { + if input == nil { + return nil, fmt.Errorf("updated_fields[%d] must be an object", index) + } + + idField, ok := input["id"] + if !ok { + return nil, fmt.Errorf("updated_field.id is required") + } + + fieldID, err := validateAndConvertToInt64(idField) + if err != nil { + return nil, fmt.Errorf("updated_field.id: %w", err) + } + + valueField, ok := input["value"] + if !ok { + return nil, fmt.Errorf("updated_field.value is required") + } + + fields = append(fields, &github.UpdateProjectV2Field{ ID: fieldID, Value: valueField, - }}, + }) + } + + payload := &github.UpdateProjectItemOptions{ + Fields: fields, } return payload, nil diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 9b0e07292f..df8c995d77 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -189,7 +189,20 @@ func Test_ProjectsList_ListProjectFields(t *testing.T) { func Test_ProjectsList_ListProjectItems(t *testing.T) { toolDef := ProjectsList(translations.NullTranslationHelper) - items := []map[string]any{{"id": 1001, "archived_at": nil, "content": map[string]any{"title": "Issue 1"}}} + items := []map[string]any{{ + "id": 1001, + "node_id": "PVTI_item1", + "content_type": "Issue", + "content": map[string]any{ + "id": 2001, + "node_id": "I_issue1", + "number": 42, + "title": "Issue 1", + "html_url": "https://github.com/octo-org/repo/issues/42", + "repository_url": "https://api.github.com/repos/octo-org/repo", + "labels": []map[string]any{{"name": "status:ready"}}, + }, + }} t.Run("success organization", func(t *testing.T) { mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -219,6 +232,17 @@ func Test_ProjectsList_ListProjectItems(t *testing.T) { itemsList, ok := response["items"].([]any) require.True(t, ok) assert.Equal(t, 1, len(itemsList)) + item, ok := itemsList[0].(map[string]any) + require.True(t, ok) + assert.Equal(t, float64(1001), item["project_item_id"]) + assert.Equal(t, "PVTI_item1", item["project_item_node_id"]) + assert.Equal(t, "issue", item["content_type"]) + assert.Equal(t, "octo-org", item["content_owner"]) + assert.Equal(t, "repo", item["content_repo"]) + assert.Equal(t, float64(42), item["content_number"]) + assert.Equal(t, float64(2001), item["content_id"]) + assert.Equal(t, "I_issue1", item["content_node_id"]) + assert.ElementsMatch(t, []any{"status:ready"}, item["content_labels"]) }) } @@ -353,7 +377,20 @@ func Test_ProjectsGet_GetProjectField(t *testing.T) { func Test_ProjectsGet_GetProjectItem(t *testing.T) { toolDef := ProjectsGet(translations.NullTranslationHelper) - item := map[string]any{"id": 1001, "archived_at": nil, "content": map[string]any{"title": "Issue 1"}} + item := map[string]any{ + "id": 1001, + "node_id": "PVTI_item1", + "content_type": "Issue", + "content": map[string]any{ + "id": 2001, + "node_id": "I_issue1", + "number": 42, + "title": "Issue 1", + "html_url": "https://github.com/octo-org/repo/issues/42", + "repository_url": "https://api.github.com/repos/octo-org/repo", + "labels": []map[string]any{{"name": "status:ready"}}, + }, + } t.Run("success organization", func(t *testing.T) { mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -382,6 +419,13 @@ func Test_ProjectsGet_GetProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1001), response["project_item_id"]) + assert.Equal(t, "PVTI_item1", response["project_item_node_id"]) + assert.Equal(t, "issue", response["content_type"]) + assert.Equal(t, "octo-org", response["content_owner"]) + assert.Equal(t, "repo", response["content_repo"]) + assert.Equal(t, float64(42), response["content_number"]) + assert.ElementsMatch(t, []any{"status:ready"}, response["content_labels"]) }) t.Run("missing item_id", func(t *testing.T) { @@ -425,6 +469,7 @@ func Test_ProjectsWrite(t *testing.T) { assert.Contains(t, inputSchema.Properties, "issue_number") assert.Contains(t, inputSchema.Properties, "pull_request_number") assert.Contains(t, inputSchema.Properties, "updated_field") + assert.Contains(t, inputSchema.Properties, "updated_fields") assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner", "project_number"}) // Verify DestructiveHint is set @@ -486,7 +531,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` }{}, @@ -498,7 +544,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "addProjectV2ItemById": map[string]any{ "item": map[string]any{ - "id": "PVTI_item1", + "id": "PVTI_item1", + "databaseId": 1001, }, }, }), @@ -530,6 +577,13 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1001), response["project_item_id"]) + assert.Equal(t, "PVTI_item1", response["project_item_node_id"]) + assert.Equal(t, "issue", response["content_type"]) + assert.Equal(t, "item-owner", response["content_owner"]) + assert.Equal(t, "item-repo", response["content_repo"]) + assert.Equal(t, float64(123), response["content_number"]) + assert.Equal(t, "I_issue123", response["content_node_id"]) assert.Contains(t, response["message"], "Successfully added") }) @@ -583,7 +637,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { struct { AddProjectV2ItemByID struct { Item struct { - ID githubv4.ID + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` } } `graphql:"addProjectV2ItemById(input: $input)"` }{}, @@ -595,7 +650,8 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "addProjectV2ItemById": map[string]any{ "item": map[string]any{ - "id": "PVTI_item2", + "id": "PVTI_item2", + "databaseId": 1002, }, }, }), @@ -627,6 +683,10 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1002), response["project_item_id"]) + assert.Equal(t, "PVTI_item2", response["project_item_node_id"]) + assert.Equal(t, "pull_request", response["content_type"]) + assert.Equal(t, float64(456), response["content_number"]) assert.Contains(t, response["message"], "Successfully added") }) @@ -737,6 +797,98 @@ func Test_ProjectsWrite_UpdateProjectItem(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) assert.NotNil(t, response["id"]) + assert.Equal(t, float64(1001), response["project_item_id"]) + }) + + t.Run("success with multiple updated fields", func(t *testing.T) { + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + + client := gh.NewClient(mockedClient) + deps := BaseDeps{ + Client: client, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_item", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "item_id": float64(1001), + "updated_fields": []any{ + map[string]any{ + "id": float64(101), + "value": "In Progress", + }, + map[string]any{ + "id": float64(102), + "value": "P1", + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, float64(1001), response["project_item_id"]) + }) + + t.Run("success by issue content identity", func(t *testing.T) { + projectItems := []map[string]any{{ + "id": 1001, + "node_id": "PVTI_item1", + "content_type": "Issue", + "content": map[string]any{ + "id": 2001, + "node_id": "I_issue123", + "number": 123, + "html_url": "https://github.com/item-owner/item-repo/issues/123", + "repository_url": "https://api.github.com/repos/item-owner/item-repo", + "labels": []map[string]any{ + {"name": "status:ready"}, + }, + }, + }} + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetOrgsProjectsV2ItemsByProject: mockResponse(t, http.StatusOK, projectItems), + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + + client := gh.NewClient(mockedClient) + deps := BaseDeps{ + Client: client, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_item", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "item_owner": "item-owner", + "item_repo": "item-repo", + "item_type": "issue", + "issue_number": float64(123), + "updated_field": map[string]any{ + "id": float64(101), + "value": "In Progress", + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, float64(1001), response["project_item_id"]) }) t.Run("missing updated_field", func(t *testing.T) { @@ -815,6 +967,128 @@ func Test_ProjectsWrite_DeleteProjectItem(t *testing.T) { }) } +func Test_CreateProjectIssue(t *testing.T) { + toolDef := CreateProjectIssue(translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(toolDef.Tool.Name, toolDef.Tool)) + + assert.Equal(t, "create_project_issue", toolDef.Tool.Name) + inputSchema := toolDef.Tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, inputSchema.Properties, "status_field_id") + assert.Contains(t, inputSchema.Properties, "priority_field_id") + assert.ElementsMatch(t, inputSchema.Required, []string{"owner", "repo", "title", "project_number", "status_field_id", "status_value", "priority_field_id", "priority_value"}) + + mockIssue := &gh.Issue{ + ID: gh.Ptr(int64(12345)), + NodeID: gh.Ptr("I_issue123"), + Number: gh.Ptr(123), + Title: gh.Ptr("P2 [APP] Task: Build thing"), + HTMLURL: gh.Ptr("https://github.com/item-owner/item-repo/issues/123"), + } + updatedItem := map[string]any{"id": 1001, "archived_at": nil} + + mockedRESTClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepo: mockResponse(t, http.StatusCreated, mockIssue), + PatchUsersProjectsV2ItemsByUsernameByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + mockedGQLClient := 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("item-owner"), + "repo": githubv4.String("item-repo"), + "issueNumber": githubv4.Int(123), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "id": "I_issue123", + }, + }, + }), + ), + githubv4mock.NewQueryMatcher( + struct { + User struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"user(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String("project-owner"), + "projectNumber": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project1", + }, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + AddProjectV2ItemByID struct { + Item struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + } + } `graphql:"addProjectV2ItemById(input: $input)"` + }{}, + githubv4.AddProjectV2ItemByIdInput{ + ProjectID: githubv4.ID("PVT_project1"), + ContentID: githubv4.ID("I_issue123"), + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "addProjectV2ItemById": map[string]any{ + "item": map[string]any{ + "id": "PVTI_item1", + "databaseId": 1001, + }, + }, + }), + ), + ) + + restClient := gh.NewClient(mockedRESTClient) + gqlClient := githubv4.NewClient(mockedGQLClient) + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "owner": "item-owner", + "repo": "item-repo", + "title": "P2 [APP] Task: Build thing", + "body": "Body", + "labels": []any{"type:task"}, + "project_owner": "project-owner", + "project_owner_type": "user", + "project_number": float64(1), + "status_field_id": float64(101), + "status_value": "Backlog", + "priority_field_id": float64(102), + "priority_value": "P2", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + textContent := getTextResult(t, result) + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + assert.Equal(t, float64(123), response["issue_number"]) + assert.Equal(t, float64(12345), response["issue_id"]) + assert.Equal(t, "I_issue123", response["issue_node_id"]) + assert.Equal(t, float64(1001), response["project_item_id"]) + assert.Equal(t, "PVTI_item1", response["project_item_node_id"]) +} + func Test_ProjectsList_ListProjectStatusUpdates(t *testing.T) { toolDef := ProjectsList(translations.NullTranslationHelper) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 23d36e0b85..da723da918 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -209,6 +209,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListIssues(t), ListIssueTypes(t), IssueWrite(t), + IssueLabelWrite(t), AddIssueComment(t), IssueDependencyWrite(t), SubIssueWrite(t), @@ -283,6 +284,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ProjectsList(t), ProjectsGet(t), ProjectsWrite(t), + CreateProjectIssue(t), // Label tools GetLabel(t),