diff --git a/README.md b/README.md index c7243033b..3bd3f0e1c 100644 --- a/README.md +++ b/README.md @@ -731,6 +731,7 @@ 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_relationships - Get relationships (blocks/blocked_by) for the issue. (string, required) - `owner`: The owner of the repository (string, required) - `page`: Page number for pagination (min 1) (number, optional) @@ -747,12 +748,16 @@ Options are: Options are: - 'create' - creates a new issue. - 'update' - updates an existing issue. +- 'add_relationship' - add a relationship between two issues. +- 'remove_relationship' - remove a relationship between two issues. (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) + - `relationship`: Relationship between source and target issues (string, optional) - `repo`: Repository name (string, required) - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) + - `target_issue_number`: Target issue number for relationship operations (number, optional) - `title`: Issue title (string, optional) - `type`: Type of this issue. Only use if the repository has issue types configured. Use list_issue_types tool to get valid type values for the organization. If the repository doesn't support issue types, omit this parameter. (string, optional) diff --git a/pkg/github/__toolsnaps__/issue_read.snap b/pkg/github/__toolsnaps__/issue_read.snap index c6a9e7306..cf9c3daf1 100644 --- a/pkg/github/__toolsnaps__/issue_read.snap +++ b/pkg/github/__toolsnaps__/issue_read.snap @@ -19,12 +19,13 @@ }, "method": { "type": "string", - "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_relationships - Get relationships (blocks/blocked_by) for the issue.\n", "enum": [ "get", "get_comments", "get_sub_issues", - "get_labels" + "get_labels", + "get_relationships" ] }, "owner": { diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 8c6634a02..a56517040 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -2,7 +2,7 @@ "annotations": { "title": "Create or update issue." }, - "description": "Create a new or update an existing issue in a GitHub repository.", + "description": "Create, update, or manage relationships for issues in a GitHub repository.", "inputSchema": { "type": "object", "required": [ @@ -39,10 +39,12 @@ }, "method": { "type": "string", - "description": "Write operation to perform on a single issue.\nOptions are:\n- 'create' - creates a new issue.\n- 'update' - updates an existing issue.\n", + "description": "Write operation to perform on a single issue.\nOptions are:\n- 'create' - creates a new issue.\n- 'update' - updates an existing issue.\n- 'add_relationship' - add a relationship between two issues.\n- 'remove_relationship' - remove a relationship between two issues.\n", "enum": [ "create", - "update" + "update", + "add_relationship", + "remove_relationship" ] }, "milestone": { @@ -53,6 +55,15 @@ "type": "string", "description": "Repository owner" }, + "relationship": { + "type": "string", + "description": "Relationship between source and target issues", + "enum": [ + "blocks", + "blocked_by", + "parent" + ] + }, "repo": { "type": "string", "description": "Repository name" @@ -74,6 +85,10 @@ "duplicate" ] }, + "target_issue_number": { + "type": "number", + "description": "Target issue number for relationship operations" + }, "title": { "type": "string", "description": "Issue title" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 46111a4d6..7f87119db 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strconv" "strings" "time" @@ -21,6 +23,13 @@ import ( "github.com/shurcooL/githubv4" ) +type IssueRelationshipEntry struct { + IssueNumber int `json:"issue_number"` + Relationship string `json:"relationship"` + Direction string `json:"direction"` + URL string `json:"url,omitempty"` +} + // CloseIssueInput represents the input for closing an issue via the GraphQL API. // Used to extend the functionality of the githubv4 library to support closing issues as duplicates. type CloseIssueInput struct { @@ -241,8 +250,9 @@ 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_relationships - Get relationships (blocks/blocked_by) for the issue. `, - Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels"}, + Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels", "get_relationships"}, }, "owner": { Type: "string", @@ -317,6 +327,9 @@ Options are: case "get_labels": result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber) return result, nil, err + case "get_relationships": + result, err := GetIssueRelationships(ctx, client, cache, owner, repo, issueNumber, pagination, flags) + return result, nil, err default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -426,6 +439,130 @@ func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdow return utils.NewToolResultText(string(r)), nil } +func GetIssueRelationships(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, pagination PaginationParams, flags FeatureFlags) (*mcp.CallToolResult, error) { + blockedBy, toolErr, err := listIssueDependencies(ctx, client, owner, repo, issueNumber, "blocked_by", pagination) + if toolErr != nil || err != nil { + return toolErr, err + } + blocking, toolErr, err := listIssueDependencies(ctx, client, owner, repo, issueNumber, "blocking", pagination) + if toolErr != nil || err != nil { + return toolErr, err + } + + normalized := make([]IssueRelationshipEntry, 0, len(blockedBy)+len(blocking)) + + appendEntry := func(issue *github.Issue, relationship string, direction string) error { + if issue == nil { + return nil + } + entry := IssueRelationshipEntry{ + IssueNumber: issue.GetNumber(), + Relationship: relationship, + Direction: direction, + URL: issue.GetHTMLURL(), + } + + if flags.LockdownMode { + if cache == nil { + return fmt.Errorf("lockdown cache is not configured") + } + login := "" + if issue.User != nil { + login = issue.User.GetLogin() + } + if login != "" { + isSafeContent, checkErr := cache.IsSafeContent(ctx, login, owner, repo) + if checkErr != nil { + return checkErr + } + if !isSafeContent { + return nil + } + } + } + + normalized = append(normalized, entry) + return nil + } + + for _, issue := range blockedBy { + if err := appendEntry(issue, "parent", "incoming"); err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil + } + } + for _, issue := range blocking { + if err := appendEntry(issue, "blocks", "outgoing"); err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil + } + } + + r, err := json.Marshal(normalized) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return utils.NewToolResultText(string(r)), nil +} + +func relationshipDirection(relationship string) string { + switch relationship { + case "blocks": + return "outgoing" + case "blocked_by": + return "incoming" + case "parent": + return "incoming" + default: + return "unknown" + } +} + +func listIssueDependencies(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, dependencyType string, pagination PaginationParams) ([]*github.Issue, *mcp.CallToolResult, error) { + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/%s", owner, repo, issueNumber, dependencyType) + + query := url.Values{} + if pagination.Page != 0 { + query.Set("page", strconv.Itoa(pagination.Page)) + } + if pagination.PerPage != 0 { + query.Set("per_page", strconv.Itoa(pagination.PerPage)) + } + if encoded := query.Encode(); encoded != "" { + path = fmt.Sprintf("%s?%s", path, encoded) + } + + req, err := client.NewRequest("GET", path, nil) + if err != nil { + return nil, nil, fmt.Errorf("failed to create request to list issue dependencies: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + + var issues []*github.Issue + resp, err := client.Do(ctx, req, &issues) + if err != nil { + return nil, ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to list issue dependencies", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusForbidden { + return nil, utils.NewToolResultError("issue dependencies API is not available for this repository or account"), nil + } + + if resp.StatusCode != http.StatusOK { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, nil, fmt.Errorf("failed to read response body: %w", readErr) + } + return nil, utils.NewToolResultError(fmt.Sprintf("failed to list issue dependencies: %s", string(body))), nil + } + + return issues, nil, nil +} + func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, pagination PaginationParams, featureFlags FeatureFlags) (*mcp.CallToolResult, error) { opts := &github.IssueListOptions{ ListOptions: github.ListOptions{ @@ -957,11 +1094,11 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) ( } } -// IssueWrite creates a tool to create a new or update an existing issue in a GitHub repository. -func IssueWrite(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) { +// IssueWrite creates a tool to create, update, or manage relationships for an issue in a GitHub repository. +func IssueWrite(getClient GetClientFn, getGQLClient GetGQLClientFn, cache *lockdown.RepoAccessCache, t translations.TranslationHelperFunc, flags FeatureFlags) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) { return mcp.Tool{ Name: "issue_write", - Description: t("TOOL_ISSUE_WRITE_DESCRIPTION", "Create a new or update an existing issue in a GitHub repository."), + Description: t("TOOL_ISSUE_WRITE_DESCRIPTION", "Create, update, or manage relationships for issues in a GitHub repository."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_ISSUE_WRITE_USER_TITLE", "Create or update issue."), ReadOnlyHint: false, @@ -975,8 +1112,10 @@ func IssueWrite(getClient GetClientFn, getGQLClient GetGQLClientFn, t translatio Options are: - 'create' - creates a new issue. - 'update' - updates an existing issue. +- 'add_relationship' - add a relationship between two issues. +- 'remove_relationship' - remove a relationship between two issues. `, - Enum: []any{"create", "update"}, + Enum: []any{"create", "update", "add_relationship", "remove_relationship"}, }, "owner": { Type: "string", @@ -990,6 +1129,10 @@ Options are: Type: "number", Description: "Issue number to update", }, + "target_issue_number": { + Type: "number", + Description: "Target issue number for relationship operations", + }, "title": { Type: "string", Description: "Issue title", @@ -1034,6 +1177,11 @@ Options are: Type: "number", Description: "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.", }, + "relationship": { + Type: "string", + Description: "Relationship between source and target issues", + Enum: []any{"blocks", "blocked_by", "parent"}, + }, }, Required: []string{"method", "owner", "repo"}, }, @@ -1132,8 +1280,38 @@ Options are: } result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, state, stateReason, duplicateOf) return result, nil, err + case "add_relationship": + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + targetIssueNumber, err := RequiredInt(args, "target_issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relationship, err := RequiredParam[string](args, "relationship") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := AddIssueRelationship(ctx, client, cache, owner, repo, issueNumber, targetIssueNumber, relationship, flags) + return result, nil, err + case "remove_relationship": + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + targetIssueNumber, err := RequiredInt(args, "target_issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relationship, err := RequiredParam[string](args, "relationship") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, err := RemoveIssueRelationship(ctx, client, cache, owner, repo, issueNumber, targetIssueNumber, relationship, flags) + return result, nil, err default: - return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil + return utils.NewToolResultError("invalid method, must be one of 'create', 'update', 'add_relationship', or 'remove_relationship'"), nil, nil } } } @@ -1312,6 +1490,189 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 return utils.NewToolResultText(string(r)), nil } +func normalizeRelationshipInput(relationship string) (string, string, error) { + switch strings.ToLower(relationship) { + case "blocks": + return "blocking", "blocks", nil + case "blocked_by": + return "blocked_by", "blocked_by", nil + case "parent": + return "blocked_by", "parent", nil + default: + return "", "", fmt.Errorf("relationship must be one of blocks, blocked_by, parent") + } +} + +func ensureIssueAccessibleForRelationship(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, flags FeatureFlags) (*github.Issue, *mcp.CallToolResult, error) { + issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber) + if err != nil { + return nil, ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get issue for lockdown check", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, nil, fmt.Errorf("failed to read response body: %w", readErr) + } + return nil, utils.NewToolResultError(fmt.Sprintf("failed to get issue for lockdown check: %s", string(body))), nil + } + + if !flags.LockdownMode { + return issue, nil, nil + } + + if cache == nil { + return nil, utils.NewToolResultError("lockdown cache is not configured"), nil + } + + login := issue.GetUser().GetLogin() + if login == "" { + return issue, nil, nil + } + + isSafeContent, checkErr := cache.IsSafeContent(ctx, login, owner, repo) + if checkErr != nil { + return nil, utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", checkErr)), nil + } + if !isSafeContent { + return nil, utils.NewToolResultError("access to issue is restricted by lockdown mode"), nil + } + + return issue, nil, nil +} + +func AddIssueRelationship(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, targetIssueNumber int, relationship string, flags FeatureFlags) (*mcp.CallToolResult, error) { + apiRelationship, outputRelationship, err := normalizeRelationshipInput(relationship) + if err != nil { + return utils.NewToolResultError(err.Error()), nil + } + + targetIssue, toolResult, err := ensureIssueAccessibleForRelationship(ctx, client, cache, owner, repo, targetIssueNumber, flags) + if toolResult != nil || err != nil { + return toolResult, err + } + + issueID := targetIssue.GetID() + if issueID == 0 { + return utils.NewToolResultError("target issue id is missing"), nil + } + + requestBody := map[string]any{ + "issue_id": issueID, + } + + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/%s", owner, repo, issueNumber, apiRelationship) + req, err := client.NewRequest("POST", path, requestBody) + if err != nil { + return nil, fmt.Errorf("failed to create request to add relationship: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + + var createdIssue github.Issue + resp, err := client.Do(ctx, req, &createdIssue) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to add issue relationship", + resp, + err, + ), nil + } + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + + if resp != nil && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, fmt.Errorf("failed to read response body: %w", readErr) + } + return utils.NewToolResultError(fmt.Sprintf("failed to add issue relationship: %s", string(body))), nil + } + + entry := IssueRelationshipEntry{ + IssueNumber: targetIssue.GetNumber(), + Relationship: outputRelationship, + Direction: relationshipDirection(outputRelationship), + URL: targetIssue.GetHTMLURL(), + } + if createdIssue.HTMLURL != nil && createdIssue.GetHTMLURL() != "" { + entry.URL = createdIssue.GetHTMLURL() + } + + response, err := json.Marshal(entry) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return utils.NewToolResultText(string(response)), nil +} + +func RemoveIssueRelationship(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, targetIssueNumber int, relationship string, flags FeatureFlags) (*mcp.CallToolResult, error) { + apiRelationship, outputRelationship, err := normalizeRelationshipInput(relationship) + if err != nil { + return utils.NewToolResultError(err.Error()), nil + } + + targetIssue, toolResult, err := ensureIssueAccessibleForRelationship(ctx, client, cache, owner, repo, targetIssueNumber, flags) + if toolResult != nil || err != nil { + return toolResult, err + } + + issueID := targetIssue.GetID() + if issueID == 0 { + return utils.NewToolResultError("target issue id is missing"), nil + } + + path := fmt.Sprintf("repos/%s/%s/issues/%d/dependencies/%s/%d", owner, repo, issueNumber, apiRelationship, issueID) + + req, err := client.NewRequest("DELETE", path, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request to remove relationship: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := client.Do(ctx, req, nil) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to remove issue relationship", + resp, + err, + ), nil + } + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + + if resp != nil && resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusOK { + body, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return nil, fmt.Errorf("failed to read response body: %w", readErr) + } + return utils.NewToolResultError(fmt.Sprintf("failed to remove issue relationship: %s", string(body))), nil + } + + entry := IssueRelationshipEntry{ + IssueNumber: targetIssue.GetNumber(), + Relationship: outputRelationship, + Direction: relationshipDirection(outputRelationship), + } + if targetIssue != nil { + entry.URL = targetIssue.GetHTMLURL() + } + + response, err := json.Marshal(entry) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return utils.NewToolResultText(string(response)), nil +} + // ListIssues creates a tool to list and filter repository issues func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) { schema := &jsonschema.Schema{ diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 48901ccdc..8eabe1e1c 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -819,7 +819,8 @@ func Test_CreateIssue(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) mockGQLClient := githubv4.NewClient(nil) - tool, _ := IssueWrite(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQLClient), translations.NullTranslationHelper) + cache := stubRepoAccessCache(mockGQLClient, 15*time.Minute) + tool, _ := IssueWrite(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQLClient), cache, translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_write", tool.Name) @@ -942,7 +943,8 @@ func Test_CreateIssue(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) gqlClient := githubv4.NewClient(nil) - _, handler := IssueWrite(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + cache := stubRepoAccessCache(gqlClient, 15*time.Minute) + _, handler := IssueWrite(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), cache, translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1289,7 +1291,8 @@ func Test_UpdateIssue(t *testing.T) { // Verify tool definition mockClient := github.NewClient(nil) mockGQLClient := githubv4.NewClient(nil) - tool, _ := IssueWrite(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQLClient), translations.NullTranslationHelper) + cache := stubRepoAccessCache(mockGQLClient, 15*time.Minute) + tool, _ := IssueWrite(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQLClient), cache, translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_write", tool.Name) @@ -1740,7 +1743,8 @@ func Test_UpdateIssue(t *testing.T) { // Setup clients with mocks restClient := github.NewClient(tc.mockedRESTClient) gqlClient := githubv4.NewClient(tc.mockedGQLClient) - _, handler := IssueWrite(stubGetClientFn(restClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + cache := stubRepoAccessCache(gqlClient, 15*time.Minute) + _, handler := IssueWrite(stubGetClientFn(restClient), stubGetGQLClientFn(gqlClient), cache, translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2018,6 +2022,179 @@ func Test_GetIssueComments(t *testing.T) { } } +func Test_GetIssueRelationships(t *testing.T) { + blockedBy := []*github.Issue{ + { + ID: github.Ptr[int64](2002), + Number: github.Ptr(2), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/2"), + User: &github.User{Login: github.Ptr("safeuser")}, + }, + } + blocking := []*github.Issue{ + { + ID: github.Ptr[int64](2003), + Number: github.Ptr(3), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/3"), + User: &github.User{Login: github.Ptr("safeuser")}, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectedEntries []IssueRelationshipEntry + lockdownEnabled bool + expectToolError bool + expectedErrMsg string + }{ + { + name: "successful relationships retrieval with parent and blocks mapping", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocked_by", + Method: "GET", + }, + expectQueryParams(t, map[string]string{ + "page": "2", + "per_page": "5", + }).andThen( + mockResponse(t, http.StatusOK, blockedBy), + ), + ), + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocking", + Method: "GET", + }, + expectQueryParams(t, map[string]string{ + "page": "2", + "per_page": "5", + }).andThen( + mockResponse(t, http.StatusOK, blocking), + ), + ), + ), + requestArgs: map[string]interface{}{ + "method": "get_relationships", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "page": float64(2), + "perPage": float64(5), + }, + expectedEntries: []IssueRelationshipEntry{ + { + IssueNumber: 2, + Relationship: "parent", + Direction: "incoming", + URL: "https://github.com/owner/repo/issues/2", + }, + { + IssueNumber: 3, + Relationship: "blocks", + Direction: "outgoing", + URL: "https://github.com/owner/repo/issues/3", + }, + }, + }, + { + name: "lockdown filters unsafe relationships", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocked_by", + Method: "GET", + }, + mockResponse(t, http.StatusOK, []*github.Issue{ + { + ID: github.Ptr[int64](2222), + Number: github.Ptr(2), + User: &github.User{Login: github.Ptr("testuser")}, + }, + }), + ), + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocking", + Method: "GET", + }, + mockResponse(t, http.StatusOK, []*github.Issue{}), + ), + ), + requestArgs: map[string]interface{}{ + "method": "get_relationships", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + }, + lockdownEnabled: true, + expectedEntries: []IssueRelationshipEntry{}, + }, + { + name: "api failure surfaces error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocked_by", + Method: "GET", + }, + mockResponse(t, http.StatusNotFound, `{"message":"Not Found"}`), + ), + mock.WithRequestMatch( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocking", + Method: "GET", + }, + []*github.Issue{}, + ), + ), + requestArgs: map[string]interface{}{ + "method": "get_relationships", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + }, + expectToolError: true, + expectedErrMsg: "issue dependencies", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + var gqlClient *githubv4.Client + if tc.lockdownEnabled { + gqlClient = githubv4.NewClient(newRepoAccessHTTPClient()) + } else { + gqlClient = githubv4.NewClient(nil) + } + cache := stubRepoAccessCache(gqlClient, 15*time.Minute) + flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled}) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), cache, translations.NullTranslationHelper, flags) + + request := createMCPRequest(tc.requestArgs) + result, _, err := handler(context.Background(), &request, tc.requestArgs) + + if tc.expectToolError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + var entries []IssueRelationshipEntry + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &entries)) + assert.Equal(t, tc.expectedEntries, entries) + }) + } +} + func Test_GetIssueLabels(t *testing.T) { t.Parallel() @@ -3548,6 +3725,183 @@ func Test_ReprioritizeSubIssue(t *testing.T) { } } +func Test_IssueRelationships_Write(t *testing.T) { + targetIssue := &github.Issue{ + ID: github.Ptr[int64](2002), + Number: github.Ptr(2), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/2"), + User: &github.User{Login: github.Ptr("safeuser")}, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + expectedResult IssueRelationshipEntry + flags FeatureFlags + }{ + { + name: "add parent relationship maps to blocked_by", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/2", + Method: "GET", + }, + mockResponse(t, http.StatusOK, targetIssue), + ), + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocked_by", + Method: "POST", + }, + expectRequestBody(t, map[string]any{ + "issue_id": float64(2002), + }).andThen( + mockResponse(t, http.StatusCreated, targetIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "method": "add_relationship", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "target_issue_number": float64(2), + "relationship": "parent", + }, + expectedResult: IssueRelationshipEntry{ + IssueNumber: 2, + Relationship: "parent", + Direction: "incoming", + URL: "https://github.com/owner/repo/issues/2", + }, + }, + { + name: "remove parent relationship succeeds", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/2", + Method: "GET", + }, + mockResponse(t, http.StatusOK, targetIssue), + ), + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/1/dependencies/blocked_by/2002", + Method: "DELETE", + }, + mockResponse(t, http.StatusNoContent, ""), + ), + ), + requestArgs: map[string]interface{}{ + "method": "remove_relationship", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "target_issue_number": float64(2), + "relationship": "parent", + }, + expectedResult: IssueRelationshipEntry{ + IssueNumber: 2, + Relationship: "parent", + Direction: "incoming", + URL: "https://github.com/owner/repo/issues/2", + }, + }, + { + name: "validation missing target issue", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "method": "add_relationship", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "relationship": "blocks", + }, + expectError: true, + expectedErrMsg: "missing required parameter: target_issue_number", + }, + { + name: "invalid relationship", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "method": "add_relationship", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "target_issue_number": float64(2), + "relationship": "depends", + }, + expectError: true, + expectedErrMsg: "relationship must be one of", + }, + { + name: "lockdown blocks unsafe target issue", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesByOwnerByRepoByIssueNumber, + &github.Issue{ + Number: github.Ptr(2), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + }, + ), + ), + requestArgs: map[string]interface{}{ + "method": "add_relationship", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "target_issue_number": float64(2), + "relationship": "blocks", + }, + expectError: true, + expectedErrMsg: "access to issue is restricted by lockdown mode", + flags: stubFeatureFlags(map[string]bool{"lockdown-mode": true}), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + var gqlClient *githubv4.Client + if tc.flags.LockdownMode { + gqlClient = githubv4.NewClient(newRepoAccessHTTPClient()) + } else { + gqlClient = githubv4.NewClient(nil) + } + cache := stubRepoAccessCache(gqlClient, 15*time.Minute) + flags := tc.flags + if flags == (FeatureFlags{}) { + flags = stubFeatureFlags(map[string]bool{"lockdown-mode": false}) + } + _, handler := IssueWrite(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), cache, translations.NullTranslationHelper, flags) + + request := createMCPRequest(tc.requestArgs) + result, _, err := handler(context.Background(), &request, tc.requestArgs) + + if tc.expectError { + require.NotNil(t, result) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + var entry IssueRelationshipEntry + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &entry)) + assert.Equal(t, tc.expectedResult, entry) + }) + } +} + func Test_ListIssueTypes(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index d37af98b8..dce3c418d 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -207,7 +207,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(GetLabel(getGQLClient, t)), ). AddWriteTools( - toolsets.NewServerTool(IssueWrite(getClient, getGQLClient, t)), + toolsets.NewServerTool(IssueWrite(getClient, getGQLClient, cache, t, flags)), toolsets.NewServerTool(AddIssueComment(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), toolsets.NewServerTool(SubIssueWrite(getClient, t)),