From b2b9273897b069d60211380767db4f25eeca3451 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Fri, 21 Aug 2026 13:35:12 -0400 Subject: [PATCH 1/5] Add atomic parent issue creation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 1 + docs/feature-flags.md | 2 + docs/insiders-features.md | 1 + pkg/github/__toolsnaps__/create_issue.snap | 5 + pkg/github/__toolsnaps__/issue_write.snap | 5 + pkg/github/issues.go | 119 ++++--- pkg/github/issues_create.go | 362 +++++++++++++++++++++ pkg/github/issues_create_test.go | 211 ++++++++++++ pkg/github/issues_granular.go | 23 ++ pkg/github/issues_test.go | 8 +- 10 files changed, 697 insertions(+), 40 deletions(-) create mode 100644 pkg/github/issues_create.go create mode 100644 pkg/github/issues_create_test.go diff --git a/README.md b/README.md index 9527478a0f..42a0c85f1c 100644 --- a/README.md +++ b/README.md @@ -937,6 +937,7 @@ The following sets of tools are available: (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, 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) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 0de5bdd722..31aa4419ff 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -67,6 +67,7 @@ runtime behavior (such as output formatting) won't appear here. (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, 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) @@ -122,6 +123,7 @@ runtime behavior (such as output formatting) won't appear here. - **Required OAuth Scopes**: `repo` - `body`: Issue body content (optional) (string, optional) - `owner`: Repository owner (username or organization) (string, required) + - `parent_issue_number`: Issue number of the parent issue. The new issue is created and attached to this parent in the same operation. (number, optional) - `repo`: Repository name (string, required) - `title`: Issue title (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 350522bf5e..b9e6623832 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -61,6 +61,7 @@ The list below is generated from the Go source. It covers tool **inventory and s (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, 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) diff --git a/pkg/github/__toolsnaps__/create_issue.snap b/pkg/github/__toolsnaps__/create_issue.snap index e0963741ac..72464a4233 100644 --- a/pkg/github/__toolsnaps__/create_issue.snap +++ b/pkg/github/__toolsnaps__/create_issue.snap @@ -17,6 +17,11 @@ "description": "Repository owner (username or organization)", "type": "string" }, + "parent_issue_number": { + "description": "Issue number of the parent issue. The new issue is created and attached to this parent in the same operation.", + "minimum": 1, + "type": "number" + }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index d4968c4f2f..4de94e36a0 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -91,6 +91,11 @@ "description": "Repository owner", "type": "string" }, + "parent_issue_number": { + "description": "Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation.", + "minimum": 1, + "type": "number" + }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index d6cc55e1ed..12e231a406 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -116,31 +116,36 @@ func getCloseStateReason(stateReason string) IssueClosedStateReason { } // issueFieldWriteMetadataNode queries only the fields needed to resolve a write: the field's -// fullDatabaseId (BigInt scalar, returned as string) plus its name and data type for validation. +// node ID, fullDatabaseId (BigInt scalar, returned as string), name, and data type for validation. // shurcooL/githubv4 cannot use interface-level fragments at union top-level, so we repeat // fullDatabaseId on each concrete type; all four implement IssueFieldCommon. type issueFieldWriteMetadataNode struct { TypeName githubv4.String `graphql:"__typename"` IssueFieldText struct { + ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldText"` IssueFieldNumber struct { + ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldNumber"` IssueFieldDate struct { + ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldDate"` IssueFieldSingleSelect struct { + ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String Options []struct { + ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String } @@ -308,33 +313,9 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli return nil, nil, nil } - ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") - var query issueFieldWriteMetadataQuery - vars := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - } - if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { - return nil, nil, fmt.Errorf("failed to query issue fields metadata: %w", err) - } - - // Build name → node map, dispatching on concrete type to extract name. - fieldByName := make(map[string]issueFieldWriteMetadataNode, len(query.Repository.IssueFields.Nodes)) - for _, node := range query.Repository.IssueFields.Nodes { - var name string - switch string(node.TypeName) { - case "IssueFieldText": - name = string(node.IssueFieldText.Name) - case "IssueFieldNumber": - name = string(node.IssueFieldNumber.Name) - case "IssueFieldDate": - name = string(node.IssueFieldDate.Name) - case "IssueFieldSingleSelect": - name = string(node.IssueFieldSingleSelect.Name) - default: - continue - } - fieldByName[strings.ToLower(strings.TrimSpace(name))] = node + fieldByName, err := fetchIssueFieldWriteMetadata(ctx, gqlClient, owner, repo) + if err != nil { + return nil, nil, err } resolved := make([]*github.IssueRequestFieldValue, 0, len(issueFields)) @@ -401,6 +382,38 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli return resolved, fieldIDsToDelete, nil } +func fetchIssueFieldWriteMetadata(ctx context.Context, gqlClient *githubv4.Client, owner, repo string) (map[string]issueFieldWriteMetadataNode, error) { + ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") + var query issueFieldWriteMetadataQuery + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { + return nil, fmt.Errorf("failed to query issue fields metadata: %w", err) + } + + fieldByName := make(map[string]issueFieldWriteMetadataNode, len(query.Repository.IssueFields.Nodes)) + for _, node := range query.Repository.IssueFields.Nodes { + var name string + switch string(node.TypeName) { + case "IssueFieldText": + name = string(node.IssueFieldText.Name) + case "IssueFieldNumber": + name = string(node.IssueFieldNumber.Name) + case "IssueFieldDate": + name = string(node.IssueFieldDate.Name) + case "IssueFieldSingleSelect": + name = string(node.IssueFieldSingleSelect.Name) + default: + continue + } + fieldByName[strings.ToLower(strings.TrimSpace(name))] = node + } + + return fieldByName, nil +} + // fetchExistingIssueFieldValues retrieves the current field values for an issue // as IssueRequestFieldValue entries, ready to be merged before an update. func fetchExistingIssueFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueNumber int) ([]*github.IssueRequestFieldValue, error) { @@ -2349,6 +2362,9 @@ var issueWriteFormParams = map[string]struct{}{ "_ui_submitted": {}, } +// parent_issue_number is intentionally omitted because the current form cannot +// represent it. Calls that supply a parent bypass the form instead of dropping it. + // issueWriteAwaitingFormResult builds the "awaiting form submission" stub // returned when issue_write hands off to the MCP App form. The body is shared // by IssueWrite and LegacyIssueWrite. The result is marked IsError=true so @@ -2425,6 +2441,11 @@ Options are: Type: "number", Description: "Issue number to update", }, + "parent_issue_number": { + Type: "number", + Description: "Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation.", + Minimum: jsonschema.Ptr(1.0), + }, "title": { Type: "string", Description: "Issue title", @@ -2611,6 +2632,19 @@ Options are: return utils.NewToolResultError(err.Error()), nil, nil } + parentIssueNumber, err := OptionalIntParam(args, "parent_issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + parentValue, parentProvided := args["parent_issue_number"] + parentProvided = parentProvided && parentValue != nil + if parentProvided && parentIssueNumber < 1 { + return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil, nil + } + if parentProvided && method != "create" { + return utils.NewToolResultError("parent_issue_number can only be used with the create method"), nil, nil + } + var issueFields []issueWriteFieldInput issueFields, err = optionalIssueWriteFields(args) if err != nil { @@ -2627,17 +2661,20 @@ Options are: return utils.NewToolResultErrorFromErr("failed to get GraphQL client", err), nil, nil } - var issueFieldValues []*github.IssueRequestFieldValue - var fieldIDsToDelete []int64 - if len(issueFields) > 0 { - issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil - } - } - switch method { case "create": + if parentProvided { + result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFields, parentIssueNumber) + return result, nil, err + } + + var issueFieldValues []*github.IssueRequestFieldValue + if len(issueFields) > 0 { + issueFieldValues, _, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil + } + } result, err := CreateIssue(ctx, client, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues) return result, nil, err case "update": @@ -2645,6 +2682,14 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + var issueFieldValues []*github.IssueRequestFieldValue + var fieldIDsToDelete []int64 + if len(issueFields) > 0 { + issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil + } + } result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ AssigneesProvided: assigneesProvided, LabelsProvided: labelsProvided, diff --git a/pkg/github/issues_create.go b/pkg/github/issues_create.go new file mode 100644 index 0000000000..ab7e4c8e09 --- /dev/null +++ b/pkg/github/issues_create.go @@ -0,0 +1,362 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "strconv" + "strings" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/go-github/v89/github" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +type CreateIssueInput struct { + RepositoryID githubv4.ID `json:"repositoryId"` + Title githubv4.String `json:"title"` + + Body *githubv4.String `json:"body,omitempty"` + AssigneeIDs *[]githubv4.ID `json:"assigneeIds,omitempty"` + MilestoneID *githubv4.ID `json:"milestoneId,omitempty"` + LabelIDs *[]githubv4.ID `json:"labelIds,omitempty"` + IssueTypeID *githubv4.ID `json:"issueTypeId,omitempty"` + ParentIssueID *githubv4.ID `json:"parentIssueId,omitempty"` + IssueFields *[]IssueFieldCreateOrUpdateInput `json:"issueFields,omitempty"` +} + +type createIssueMutation struct { + CreateIssue struct { + Issue struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + URL githubv4.URI + } + } `graphql:"createIssue(input: $input)"` +} + +type createIssueParentMetadataQuery struct { + Repository struct { + ID githubv4.ID + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $parentIssueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +func createIssueWithParent( + ctx context.Context, + client *github.Client, + gqlClient *githubv4.Client, + owner string, + repo string, + title string, + body string, + assignees []string, + labels []string, + milestoneNumber int, + issueType string, + issueFields []issueWriteFieldInput, + parentIssueNumber int, +) (*mcp.CallToolResult, error) { + if title == "" { + return utils.NewToolResultError("missing required parameter: title"), nil + } + if parentIssueNumber < 1 { + return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil + } + + repositoryID, parentIssueID, err := resolveCreateIssueParent(ctx, gqlClient, owner, repo, parentIssueNumber) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve parent issue", err), nil + } + + input := CreateIssueInput{ + RepositoryID: repositoryID, + Title: githubv4.String(title), + ParentIssueID: &parentIssueID, + } + if body != "" { + input.Body = githubv4.NewString(githubv4.String(body)) + } + + if len(labels) > 0 { + labelIDs := make([]githubv4.ID, 0, len(labels)) + for _, label := range labels { + labelID, err := getLabelID(ctx, gqlClient, owner, repo, label) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve label %q", label), err), nil + } + labelIDs = append(labelIDs, labelID) + } + input.LabelIDs = &labelIDs + } + + if len(assignees) > 0 { + assigneeIDs := make([]githubv4.ID, 0, len(assignees)) + for _, assignee := range assignees { + assigneeID, err := resolveUserID(ctx, gqlClient, assignee) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve assignee %q", assignee), err), nil + } + assigneeIDs = append(assigneeIDs, assigneeID) + } + input.AssigneeIDs = &assigneeIDs + } + + if milestoneNumber != 0 { + milestoneID, err := resolveMilestoneID(ctx, gqlClient, owner, repo, milestoneNumber) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve milestone", err), nil + } + input.MilestoneID = &milestoneID + } + + if issueType != "" { + issueTypeID, resp, err := resolveIssueTypeID(ctx, client, owner, repo, issueType) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to resolve issue type %q", issueType), resp, err), nil + } + input.IssueTypeID = &issueTypeID + } + + if len(issueFields) > 0 { + resolvedIssueFields, err := resolveIssueFieldCreateInputs(ctx, gqlClient, owner, repo, issueFields) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve issue_fields", err), nil + } + if len(resolvedIssueFields) > 0 { + input.IssueFields = &resolvedIssueFields + } + } + + var mutation createIssueMutation + mutationContext := ctx + if input.IssueFields != nil { + mutationContext = ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") + } + if err := gqlClient.Mutate(mutationContext, &mutation, input, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to create issue", err), nil + } + + response := MinimalResponse{ + ID: string(mutation.CreateIssue.Issue.FullDatabaseID), + URL: mutation.CreateIssue.Issue.URL.String(), + } + encoded, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil + } + return utils.NewToolResultText(string(encoded)), nil +} + +func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { + var query createIssueParentMetadataQuery + variables := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - issue numbers are small positive integers + } + if err := gqlClient.Query(ctx, &query, variables); err != nil { + return "", "", err + } + if query.Repository.ID == "" { + return "", "", fmt.Errorf("repository %s/%s was not found", owner, repo) + } + if query.Repository.Issue.ID == "" { + return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, owner, repo) + } + return query.Repository.ID, query.Repository.Issue.ID, nil +} + +func resolveUserID(ctx context.Context, gqlClient *githubv4.Client, login string) (githubv4.ID, error) { + var query struct { + User struct { + ID githubv4.ID + Login githubv4.String + } `graphql:"user(login: $login)"` + } + if err := gqlClient.Query(ctx, &query, map[string]any{"login": githubv4.String(login)}); err != nil { + return "", err + } + if query.User.ID == "" { + return "", fmt.Errorf("user %q was not found", login) + } + return query.User.ID, nil +} + +func resolveMilestoneID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, milestoneNumber int) (githubv4.ID, error) { + var query struct { + Repository struct { + Milestone struct { + ID githubv4.ID + } `graphql:"milestone(number: $milestoneNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + variables := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "milestoneNumber": githubv4.Int(milestoneNumber), // #nosec G115 - milestone numbers are small positive integers + } + if err := gqlClient.Query(ctx, &query, variables); err != nil { + return "", err + } + if query.Repository.Milestone.ID == "" { + return "", fmt.Errorf("milestone #%d was not found in %s/%s", milestoneNumber, owner, repo) + } + return query.Repository.Milestone.ID, nil +} + +func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo, issueTypeName string) (githubv4.ID, *github.Response, error) { + req, err := client.NewRequest(ctx, "GET", fmt.Sprintf("repos/%s/%s/issue-types", owner, repo), nil) + if err != nil { + return "", nil, err + } + + var issueTypes []*github.IssueType + resp, err := client.Do(req, &issueTypes) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return "", resp, err + } + for _, issueType := range issueTypes { + if issueType != nil && strings.EqualFold(strings.TrimSpace(issueType.GetName()), strings.TrimSpace(issueTypeName)) { + if issueType.GetNodeID() == "" { + return "", resp, fmt.Errorf("issue type %q is missing a node ID", issueTypeName) + } + return githubv4.ID(issueType.GetNodeID()), resp, nil + } + } + return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo) +} + +func resolveIssueFieldCreateInputs(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]IssueFieldCreateOrUpdateInput, error) { + fieldByName, err := fetchIssueFieldWriteMetadata(ctx, gqlClient, owner, repo) + if err != nil { + return nil, err + } + + resolved := make([]IssueFieldCreateOrUpdateInput, 0, len(issueFields)) + for _, fieldInput := range issueFields { + if fieldInput.Delete { + continue + } + + node, ok := fieldByName[strings.ToLower(strings.TrimSpace(fieldInput.FieldName))] + if !ok { + return nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo) + } + + input, err := issueFieldCreateInput(node, fieldInput) + if err != nil { + return nil, err + } + resolved = append(resolved, input) + } + return resolved, nil +} + +func issueFieldCreateInput(node issueFieldWriteMetadataNode, fieldInput issueWriteFieldInput) (IssueFieldCreateOrUpdateInput, error) { + input := IssueFieldCreateOrUpdateInput{} + var dataType string + + switch string(node.TypeName) { + case "IssueFieldText": + input.FieldID = node.IssueFieldText.ID + dataType = string(node.IssueFieldText.DataType) + case "IssueFieldNumber": + input.FieldID = node.IssueFieldNumber.ID + dataType = string(node.IssueFieldNumber.DataType) + case "IssueFieldDate": + input.FieldID = node.IssueFieldDate.ID + dataType = string(node.IssueFieldDate.DataType) + case "IssueFieldSingleSelect": + input.FieldID = node.IssueFieldSingleSelect.ID + dataType = string(node.IssueFieldSingleSelect.DataType) + default: + return input, fmt.Errorf("issue field %q has unsupported type %q", fieldInput.FieldName, node.TypeName) + } + if input.FieldID == "" { + return input, fmt.Errorf("issue field %q is missing a node ID", fieldInput.FieldName) + } + + switch strings.ToLower(dataType) { + case "text": + value := fmt.Sprint(fieldInput.Value) + input.TextValue = githubv4.NewString(githubv4.String(value)) + case "number": + value, err := issueFieldNumberValue(fieldInput.Value) + if err != nil { + return input, fmt.Errorf("issue field %q: %w", fieldInput.FieldName, err) + } + input.NumberValue = &value + case "date": + value, ok := fieldInput.Value.(string) + if !ok { + return input, fmt.Errorf("issue field %q requires a date string", fieldInput.FieldName) + } + input.DateValue = githubv4.NewString(githubv4.String(value)) + case "single_select": + optionName := fieldInput.FieldOptionName + if optionName == "" { + var ok bool + optionName, ok = fieldInput.Value.(string) + if !ok { + return input, fmt.Errorf("issue field %q requires a single-select option name", fieldInput.FieldName) + } + } + for _, option := range node.IssueFieldSingleSelect.Options { + if strings.EqualFold(strings.TrimSpace(string(option.Name)), strings.TrimSpace(optionName)) { + if option.ID == "" { + return input, fmt.Errorf("issue field option %q for field %q is missing a node ID", optionName, fieldInput.FieldName) + } + optionID := option.ID + input.SingleSelectOptionID = &optionID + return input, nil + } + } + return input, fmt.Errorf("issue field option %q was not found for field %q", optionName, fieldInput.FieldName) + default: + return input, fmt.Errorf("issue field %q has unsupported data type %q", fieldInput.FieldName, dataType) + } + + return input, nil +} + +func issueFieldNumberValue(value any) (githubv4.Float, error) { + switch value := value.(type) { + case float64: + return githubv4.Float(value), nil + case float32: + return githubv4.Float(value), nil + case int: + return githubv4.Float(value), nil + case int8: + return githubv4.Float(value), nil + case int16: + return githubv4.Float(value), nil + case int32: + return githubv4.Float(value), nil + case int64: + return githubv4.Float(value), nil + case uint: + return githubv4.Float(value), nil + case uint8: + return githubv4.Float(value), nil + case uint16: + return githubv4.Float(value), nil + case uint32: + return githubv4.Float(value), nil + case uint64: + return githubv4.Float(value), nil + case json.Number: + number, err := strconv.ParseFloat(string(value), 64) + return githubv4.Float(number), err + default: + return 0, fmt.Errorf("requires a numeric value") + } +} diff --git a/pkg/github/issues_create_test.go b/pkg/github/issues_create_test.go new file mode 100644 index 0000000000..51a29d6f25 --- /dev/null +++ b/pkg/github/issues_create_test.go @@ -0,0 +1,211 @@ +package github + +import ( + "context" + "encoding/json" + "testing" + + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/jsonschema-go/jsonschema" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + schema := serverTool.Tool.InputSchema + issueWriteSchema := schema.(*jsonschema.Schema) + assert.Contains(t, issueWriteSchema.Properties, "parent_issue_number") + assert.NotContains(t, issueWriteSchema.Required, "parent_issue_number") + + labelIDs := []githubv4.ID{"LABEL_backlog"} + parentID := githubv4.ID("ISSUE_parent") + expectedInput := CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Atomic child"), + Body: githubv4.NewString(githubv4.String("Created under its parent")), + LabelIDs: &labelIDs, + ParentIssueID: &parentID, + } + createMatcher := githubv4mock.NewMutationMatcher( + createIssueMutation{}, + expectedInput, + nil, + githubv4mock.DataResponse(map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{ + "fullDatabaseId": "12345", + "url": "https://github.com/owner/repo/issues/2", + }, + }, + }), + ) + assert.Contains(t, createMatcher.Request, "$input:CreateIssueInput!") + + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(1, "REPO_1", "ISSUE_parent"), + createIssueLabelMatcher("status:backlog", "LABEL_backlog"), + createMatcher, + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "body": "Created under its parent", + "labels": []any{"status:backlog"}, + "parent_issue_number": float64(1), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Equal(t, 3, gqlCalls(), "metadata lookups and exactly one create mutation are expected") + assert.Zero(t, restCounter.count.Load(), "parent creation must not use REST create or attachment requests") + + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, "12345", response.ID) + assert.Equal(t, "https://github.com/owner/repo/issues/2", response.URL) +} + +func TestIssueWriteCreateWithParentDoesNotFallbackAfterMutationFailure(t *testing.T) { + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(7, "REPO_1", "ISSUE_parent"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Atomic child"), + ParentIssueID: githubv4mock.Ptr[githubv4.ID]("ISSUE_parent"), + }, + nil, + githubv4mock.ErrorResponse("parent cannot accept sub-issues"), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + serverTool := IssueWrite(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "parent_issue_number": float64(7), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "failed to create issue") + assert.Equal(t, 2, gqlCalls(), "a failed create mutation must not trigger an attachment mutation") + assert.Zero(t, restCounter.count.Load(), "a failed create mutation must not fall back to REST create or attachment requests") +} + +func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + schema := serverTool.Tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "parent_issue_number") + + parentID := githubv4.ID("ISSUE_parent") + gqlHTTPClient := githubv4mock.NewMockedHTTPClient( + createIssueParentMatcher(3, "REPO_1", "ISSUE_parent"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Granular child"), + ParentIssueID: &parentID, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{ + "fullDatabaseId": "23456", + "url": "https://github.com/owner/repo/issues/4", + }, + }, + }), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Granular child", + "parent_issue_number": float64(3), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func createIssueParentMatcher(parentIssueNumber int, repositoryID, parentIssueID githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + createIssueParentMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "id": repositoryID, + "issue": map[string]any{ + "id": parentIssueID, + }, + }, + }), + ) +} + +func createIssueLabelMatcher(name string, id githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Label struct { + ID githubv4.ID + Name githubv4.String + } `graphql:"label(name: $name)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "name": githubv4.String(name), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "label": map[string]any{ + "id": id, + "name": name, + }, + }, + }), + ) +} diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index fb5ff32242..ef6fdb2c25 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -144,6 +144,11 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT Type: "string", Description: "Issue body content (optional)", }, + "parent_issue_number": { + Type: "number", + Description: "Issue number of the parent issue. The new issue is created and attached to this parent in the same operation.", + Minimum: jsonschema.Ptr(1.0), + }, }, Required: []string{"owner", "repo", "title"}, }, @@ -163,6 +168,15 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT return utils.NewToolResultError(err.Error()), nil, nil } body, _ := OptionalParam[string](args, "body") + parentIssueNumber, err := OptionalIntParam(args, "parent_issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + parentValue, parentProvided := args["parent_issue_number"] + parentProvided = parentProvided && parentValue != nil + if parentProvided && parentIssueNumber < 1 { + return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil, nil + } issueReq := github.CreateIssueRequest{ Title: title, @@ -176,6 +190,15 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + if parentProvided { + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, nil, nil, 0, "", nil, parentIssueNumber) + return result, nil, err + } + issue, resp, err := client.Issues.Create(ctx, owner, repo, issueReq) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create issue", resp, err), nil, nil diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7e9fb2ee8e..95fcef9780 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2258,6 +2258,7 @@ func Test_issueWriteHasNonFormParams(t *testing.T) { {name: "state present", args: map[string]any{"state": "closed"}, want: false}, {name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: false}, {name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: false}, + {name: "parent issue present", args: map[string]any{"parent_issue_number": float64(7)}, want: true}, {name: "unknown non-schema param present", args: map[string]any{"title": "t", "not_a_real_param": "x"}, want: true}, {name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false}, } @@ -2358,10 +2359,11 @@ func Test_issueWriteSchemaClassification(t *testing.T) { // Schema properties the MCP App form cannot represent — their presence // must trigger the safety-net bypass via hasNonFormParams. The - // form currently collects every schema property, so this allowlist is - // empty; add a property here only if it is added to the schema without + // Add a property here only if it is added to the schema without // corresponding form support. - knownNonForm := map[string]struct{}{} + knownNonForm := map[string]struct{}{ + "parent_issue_number": {}, + } cases := []struct { name string From 22e29eceec7c84b7c3c8f184b0e7e00cdbede7e1 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Fri, 21 Aug 2026 13:53:17 -0400 Subject: [PATCH 2/5] Support cross-repository parent issues Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f71d9868-eef8-4fb0-84c6-df7c9a6a0ade --- README.md | 4 +- docs/feature-flags.md | 6 +- docs/insiders-features.md | 4 +- pkg/github/__toolsnaps__/create_issue.snap | 8 + pkg/github/__toolsnaps__/issue_write.snap | 10 +- pkg/github/issues.go | 123 ++++++------- pkg/github/issues_create.go | 194 ++++----------------- pkg/github/issues_create_test.go | 90 +++++++++- pkg/github/issues_granular.go | 21 ++- pkg/github/issues_test.go | 8 +- 10 files changed, 235 insertions(+), 233 deletions(-) diff --git a/README.md b/README.md index 42a0c85f1c..5cf55d7520 100644 --- a/README.md +++ b/README.md @@ -937,7 +937,9 @@ The following sets of tools are available: (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (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) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 31aa4419ff..d2f5aa3b65 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -67,7 +67,9 @@ runtime behavior (such as output formatting) won't appear here. (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (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) @@ -124,6 +126,8 @@ runtime behavior (such as output formatting) won't appear here. - `body`: Issue body content (optional) (string, optional) - `owner`: Repository owner (username or organization) (string, required) - `parent_issue_number`: Issue number of the parent issue. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided. (string, optional) - `repo`: Repository name (string, required) - `title`: Issue title (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index b9e6623832..3cf32183e3 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -61,7 +61,9 @@ The list below is generated from the Go source. It covers tool **inventory and s (string, required) - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) + - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (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) diff --git a/pkg/github/__toolsnaps__/create_issue.snap b/pkg/github/__toolsnaps__/create_issue.snap index 72464a4233..b28668c191 100644 --- a/pkg/github/__toolsnaps__/create_issue.snap +++ b/pkg/github/__toolsnaps__/create_issue.snap @@ -22,6 +22,14 @@ "minimum": 1, "type": "number" }, + "parent_owner": { + "description": "Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided.", + "type": "string" + }, + "parent_repo": { + "description": "Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided.", + "type": "string" + }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 4de94e36a0..ce8a4c727b 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -92,10 +92,18 @@ "type": "string" }, "parent_issue_number": { - "description": "Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation.", + "description": "Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation.", "minimum": 1, "type": "number" }, + "parent_owner": { + "description": "Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided.", + "type": "string" + }, + "parent_repo": { + "description": "Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided.", + "type": "string" + }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 12e231a406..df1b02e724 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -116,36 +116,31 @@ func getCloseStateReason(stateReason string) IssueClosedStateReason { } // issueFieldWriteMetadataNode queries only the fields needed to resolve a write: the field's -// node ID, fullDatabaseId (BigInt scalar, returned as string), name, and data type for validation. +// fullDatabaseId (BigInt scalar, returned as string) plus its name and data type for validation. // shurcooL/githubv4 cannot use interface-level fragments at union top-level, so we repeat // fullDatabaseId on each concrete type; all four implement IssueFieldCommon. type issueFieldWriteMetadataNode struct { TypeName githubv4.String `graphql:"__typename"` IssueFieldText struct { - ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldText"` IssueFieldNumber struct { - ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldNumber"` IssueFieldDate struct { - ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String } `graphql:"... on IssueFieldDate"` IssueFieldSingleSelect struct { - ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String DataType githubv4.String Options []struct { - ID githubv4.ID FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` Name githubv4.String } @@ -313,9 +308,33 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli return nil, nil, nil } - fieldByName, err := fetchIssueFieldWriteMetadata(ctx, gqlClient, owner, repo) - if err != nil { - return nil, nil, err + ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") + var query issueFieldWriteMetadataQuery + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { + return nil, nil, fmt.Errorf("failed to query issue fields metadata: %w", err) + } + + // Build name → node map, dispatching on concrete type to extract name. + fieldByName := make(map[string]issueFieldWriteMetadataNode, len(query.Repository.IssueFields.Nodes)) + for _, node := range query.Repository.IssueFields.Nodes { + var name string + switch string(node.TypeName) { + case "IssueFieldText": + name = string(node.IssueFieldText.Name) + case "IssueFieldNumber": + name = string(node.IssueFieldNumber.Name) + case "IssueFieldDate": + name = string(node.IssueFieldDate.Name) + case "IssueFieldSingleSelect": + name = string(node.IssueFieldSingleSelect.Name) + default: + continue + } + fieldByName[strings.ToLower(strings.TrimSpace(name))] = node } resolved := make([]*github.IssueRequestFieldValue, 0, len(issueFields)) @@ -382,38 +401,6 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli return resolved, fieldIDsToDelete, nil } -func fetchIssueFieldWriteMetadata(ctx context.Context, gqlClient *githubv4.Client, owner, repo string) (map[string]issueFieldWriteMetadataNode, error) { - ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") - var query issueFieldWriteMetadataQuery - vars := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - } - if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { - return nil, fmt.Errorf("failed to query issue fields metadata: %w", err) - } - - fieldByName := make(map[string]issueFieldWriteMetadataNode, len(query.Repository.IssueFields.Nodes)) - for _, node := range query.Repository.IssueFields.Nodes { - var name string - switch string(node.TypeName) { - case "IssueFieldText": - name = string(node.IssueFieldText.Name) - case "IssueFieldNumber": - name = string(node.IssueFieldNumber.Name) - case "IssueFieldDate": - name = string(node.IssueFieldDate.Name) - case "IssueFieldSingleSelect": - name = string(node.IssueFieldSingleSelect.Name) - default: - continue - } - fieldByName[strings.ToLower(strings.TrimSpace(name))] = node - } - - return fieldByName, nil -} - // fetchExistingIssueFieldValues retrieves the current field values for an issue // as IssueRequestFieldValue entries, ready to be merged before an update. func fetchExistingIssueFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueNumber int) ([]*github.IssueRequestFieldValue, error) { @@ -2362,8 +2349,8 @@ var issueWriteFormParams = map[string]struct{}{ "_ui_submitted": {}, } -// parent_issue_number is intentionally omitted because the current form cannot -// represent it. Calls that supply a parent bypass the form instead of dropping it. +// Parent issue parameters are intentionally omitted because the current form cannot +// represent them. Calls that supply a parent bypass the form instead of dropping them. // issueWriteAwaitingFormResult builds the "awaiting form submission" stub // returned when issue_write hands off to the MCP App form. The body is shared @@ -2443,9 +2430,17 @@ Options are: }, "parent_issue_number": { Type: "number", - Description: "Issue number of the parent issue. Only used when method is 'create'. The new issue is created and attached to this parent in the same operation.", + Description: "Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation.", Minimum: jsonschema.Ptr(1.0), }, + "parent_owner": { + Type: "string", + Description: "Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided.", + }, + "parent_repo": { + Type: "string", + Description: "Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided.", + }, "title": { Type: "string", Description: "Issue title", @@ -2644,12 +2639,26 @@ Options are: if parentProvided && method != "create" { return utils.NewToolResultError("parent_issue_number can only be used with the create method"), nil, nil } + parentOwner, err := OptionalParam[string](args, "parent_owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + parentRepo, err := OptionalParam[string](args, "parent_repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if !parentProvided && (parentOwner != "" || parentRepo != "") { + return utils.NewToolResultError("parent_owner and parent_repo can only be used when parent_issue_number is provided"), nil, nil + } var issueFields []issueWriteFieldInput issueFields, err = optionalIssueWriteFields(args) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if parentProvided && len(issueFields) > 0 { + return utils.NewToolResultError("issue_fields cannot be used with parent_issue_number"), nil, nil + } client, err := deps.GetClient(ctx) if err != nil { @@ -2661,20 +2670,22 @@ Options are: return utils.NewToolResultErrorFromErr("failed to get GraphQL client", err), nil, nil } + var issueFieldValues []*github.IssueRequestFieldValue + var fieldIDsToDelete []int64 + if len(issueFields) > 0 { + issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil + } + } + switch method { case "create": if parentProvided { - result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFields, parentIssueNumber) + result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, assignees, labels, milestoneNum, issueType, parentIssueNumber, parentOwner, parentRepo) return result, nil, err } - var issueFieldValues []*github.IssueRequestFieldValue - if len(issueFields) > 0 { - issueFieldValues, _, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil - } - } result, err := CreateIssue(ctx, client, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues) return result, nil, err case "update": @@ -2682,14 +2693,6 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - var issueFieldValues []*github.IssueRequestFieldValue - var fieldIDsToDelete []int64 - if len(issueFields) > 0 { - issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil - } - } result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ AssigneesProvided: assigneesProvided, LabelsProvided: labelsProvided, diff --git a/pkg/github/issues_create.go b/pkg/github/issues_create.go index ab7e4c8e09..371e7fe889 100644 --- a/pkg/github/issues_create.go +++ b/pkg/github/issues_create.go @@ -4,10 +4,8 @@ import ( "context" "encoding/json" "fmt" - "strconv" "strings" - ghcontext "github.com/github/github-mcp-server/pkg/context" ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v89/github" @@ -19,13 +17,12 @@ type CreateIssueInput struct { RepositoryID githubv4.ID `json:"repositoryId"` Title githubv4.String `json:"title"` - Body *githubv4.String `json:"body,omitempty"` - AssigneeIDs *[]githubv4.ID `json:"assigneeIds,omitempty"` - MilestoneID *githubv4.ID `json:"milestoneId,omitempty"` - LabelIDs *[]githubv4.ID `json:"labelIds,omitempty"` - IssueTypeID *githubv4.ID `json:"issueTypeId,omitempty"` - ParentIssueID *githubv4.ID `json:"parentIssueId,omitempty"` - IssueFields *[]IssueFieldCreateOrUpdateInput `json:"issueFields,omitempty"` + Body *githubv4.String `json:"body,omitempty"` + AssigneeIDs *[]githubv4.ID `json:"assigneeIds,omitempty"` + MilestoneID *githubv4.ID `json:"milestoneId,omitempty"` + LabelIDs *[]githubv4.ID `json:"labelIds,omitempty"` + IssueTypeID *githubv4.ID `json:"issueTypeId,omitempty"` + ParentIssueID *githubv4.ID `json:"parentIssueId,omitempty"` } type createIssueMutation struct { @@ -38,12 +35,14 @@ type createIssueMutation struct { } type createIssueParentMetadataQuery struct { - Repository struct { - ID githubv4.ID + ChildRepository struct { + ID githubv4.ID + } `graphql:"childRepository: repository(owner: $owner, name: $repo)"` + ParentRepository struct { Issue struct { ID githubv4.ID } `graphql:"issue(number: $parentIssueNumber)"` - } `graphql:"repository(owner: $owner, name: $repo)"` + } `graphql:"parentRepository: repository(owner: $parentOwner, name: $parentRepo)"` } func createIssueWithParent( @@ -58,8 +57,9 @@ func createIssueWithParent( labels []string, milestoneNumber int, issueType string, - issueFields []issueWriteFieldInput, parentIssueNumber int, + parentOwner string, + parentRepo string, ) (*mcp.CallToolResult, error) { if title == "" { return utils.NewToolResultError("missing required parameter: title"), nil @@ -68,7 +68,8 @@ func createIssueWithParent( return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil } - repositoryID, parentIssueID, err := resolveCreateIssueParent(ctx, gqlClient, owner, repo, parentIssueNumber) + parentOwner, parentRepo = parentRepository(owner, repo, parentOwner, parentRepo) + repositoryID, parentIssueID, err := resolveCreateIssueParent(ctx, gqlClient, owner, repo, parentOwner, parentRepo, parentIssueNumber) if err != nil { return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve parent issue", err), nil } @@ -122,22 +123,8 @@ func createIssueWithParent( input.IssueTypeID = &issueTypeID } - if len(issueFields) > 0 { - resolvedIssueFields, err := resolveIssueFieldCreateInputs(ctx, gqlClient, owner, repo, issueFields) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve issue_fields", err), nil - } - if len(resolvedIssueFields) > 0 { - input.IssueFields = &resolvedIssueFields - } - } - var mutation createIssueMutation - mutationContext := ctx - if input.IssueFields != nil { - mutationContext = ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") - } - if err := gqlClient.Mutate(mutationContext, &mutation, input, nil); err != nil { + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to create issue", err), nil } @@ -152,23 +139,35 @@ func createIssueWithParent( return utils.NewToolResultText(string(encoded)), nil } -func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { +func parentRepository(owner, repo, parentOwner, parentRepo string) (string, string) { + if parentOwner == "" { + parentOwner = owner + } + if parentRepo == "" { + parentRepo = repo + } + return parentOwner, parentRepo +} + +func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo, parentOwner, parentRepo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { var query createIssueParentMetadataQuery variables := map[string]any{ "owner": githubv4.String(owner), "repo": githubv4.String(repo), + "parentOwner": githubv4.String(parentOwner), + "parentRepo": githubv4.String(parentRepo), "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - issue numbers are small positive integers } if err := gqlClient.Query(ctx, &query, variables); err != nil { return "", "", err } - if query.Repository.ID == "" { + if query.ChildRepository.ID == "" { return "", "", fmt.Errorf("repository %s/%s was not found", owner, repo) } - if query.Repository.Issue.ID == "" { - return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, owner, repo) + if query.ParentRepository.Issue.ID == "" { + return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, parentOwner, parentRepo) } - return query.Repository.ID, query.Repository.Issue.ID, nil + return query.ChildRepository.ID, query.ParentRepository.Issue.ID, nil } func resolveUserID(ctx context.Context, gqlClient *githubv4.Client, login string) (githubv4.ID, error) { @@ -233,130 +232,3 @@ func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo, } return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo) } - -func resolveIssueFieldCreateInputs(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]IssueFieldCreateOrUpdateInput, error) { - fieldByName, err := fetchIssueFieldWriteMetadata(ctx, gqlClient, owner, repo) - if err != nil { - return nil, err - } - - resolved := make([]IssueFieldCreateOrUpdateInput, 0, len(issueFields)) - for _, fieldInput := range issueFields { - if fieldInput.Delete { - continue - } - - node, ok := fieldByName[strings.ToLower(strings.TrimSpace(fieldInput.FieldName))] - if !ok { - return nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo) - } - - input, err := issueFieldCreateInput(node, fieldInput) - if err != nil { - return nil, err - } - resolved = append(resolved, input) - } - return resolved, nil -} - -func issueFieldCreateInput(node issueFieldWriteMetadataNode, fieldInput issueWriteFieldInput) (IssueFieldCreateOrUpdateInput, error) { - input := IssueFieldCreateOrUpdateInput{} - var dataType string - - switch string(node.TypeName) { - case "IssueFieldText": - input.FieldID = node.IssueFieldText.ID - dataType = string(node.IssueFieldText.DataType) - case "IssueFieldNumber": - input.FieldID = node.IssueFieldNumber.ID - dataType = string(node.IssueFieldNumber.DataType) - case "IssueFieldDate": - input.FieldID = node.IssueFieldDate.ID - dataType = string(node.IssueFieldDate.DataType) - case "IssueFieldSingleSelect": - input.FieldID = node.IssueFieldSingleSelect.ID - dataType = string(node.IssueFieldSingleSelect.DataType) - default: - return input, fmt.Errorf("issue field %q has unsupported type %q", fieldInput.FieldName, node.TypeName) - } - if input.FieldID == "" { - return input, fmt.Errorf("issue field %q is missing a node ID", fieldInput.FieldName) - } - - switch strings.ToLower(dataType) { - case "text": - value := fmt.Sprint(fieldInput.Value) - input.TextValue = githubv4.NewString(githubv4.String(value)) - case "number": - value, err := issueFieldNumberValue(fieldInput.Value) - if err != nil { - return input, fmt.Errorf("issue field %q: %w", fieldInput.FieldName, err) - } - input.NumberValue = &value - case "date": - value, ok := fieldInput.Value.(string) - if !ok { - return input, fmt.Errorf("issue field %q requires a date string", fieldInput.FieldName) - } - input.DateValue = githubv4.NewString(githubv4.String(value)) - case "single_select": - optionName := fieldInput.FieldOptionName - if optionName == "" { - var ok bool - optionName, ok = fieldInput.Value.(string) - if !ok { - return input, fmt.Errorf("issue field %q requires a single-select option name", fieldInput.FieldName) - } - } - for _, option := range node.IssueFieldSingleSelect.Options { - if strings.EqualFold(strings.TrimSpace(string(option.Name)), strings.TrimSpace(optionName)) { - if option.ID == "" { - return input, fmt.Errorf("issue field option %q for field %q is missing a node ID", optionName, fieldInput.FieldName) - } - optionID := option.ID - input.SingleSelectOptionID = &optionID - return input, nil - } - } - return input, fmt.Errorf("issue field option %q was not found for field %q", optionName, fieldInput.FieldName) - default: - return input, fmt.Errorf("issue field %q has unsupported data type %q", fieldInput.FieldName, dataType) - } - - return input, nil -} - -func issueFieldNumberValue(value any) (githubv4.Float, error) { - switch value := value.(type) { - case float64: - return githubv4.Float(value), nil - case float32: - return githubv4.Float(value), nil - case int: - return githubv4.Float(value), nil - case int8: - return githubv4.Float(value), nil - case int16: - return githubv4.Float(value), nil - case int32: - return githubv4.Float(value), nil - case int64: - return githubv4.Float(value), nil - case uint: - return githubv4.Float(value), nil - case uint8: - return githubv4.Float(value), nil - case uint16: - return githubv4.Float(value), nil - case uint32: - return githubv4.Float(value), nil - case uint64: - return githubv4.Float(value), nil - case json.Number: - number, err := strconv.ParseFloat(string(value), 64) - return githubv4.Float(number), err - default: - return 0, fmt.Errorf("requires a numeric value") - } -} diff --git a/pkg/github/issues_create_test.go b/pkg/github/issues_create_test.go index 51a29d6f25..b9b183683e 100644 --- a/pkg/github/issues_create_test.go +++ b/pkg/github/issues_create_test.go @@ -8,6 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,7 +19,11 @@ func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { schema := serverTool.Tool.InputSchema issueWriteSchema := schema.(*jsonschema.Schema) assert.Contains(t, issueWriteSchema.Properties, "parent_issue_number") + assert.Contains(t, issueWriteSchema.Properties, "parent_owner") + assert.Contains(t, issueWriteSchema.Properties, "parent_repo") assert.NotContains(t, issueWriteSchema.Required, "parent_issue_number") + assert.NotContains(t, issueWriteSchema.Required, "parent_owner") + assert.NotContains(t, issueWriteSchema.Required, "parent_repo") labelIDs := []githubv4.ID{"LABEL_backlog"} parentID := githubv4.ID("ISSUE_parent") @@ -45,7 +50,7 @@ func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { assert.Contains(t, createMatcher.Request, "$input:CreateIssueInput!") gqlHTTPClient, gqlCalls := countingGraphQLClient( - createIssueParentMatcher(1, "REPO_1", "ISSUE_parent"), + createIssueParentMatcher(1, "parent-owner", "parent-repo", "REPO_1", "ISSUE_parent"), createIssueLabelMatcher("status:backlog", "LABEL_backlog"), createMatcher, ) @@ -66,6 +71,8 @@ func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { "body": "Created under its parent", "labels": []any{"status:backlog"}, "parent_issue_number": float64(1), + "parent_owner": "parent-owner", + "parent_repo": "parent-repo", }) result, err := handler(ContextWithDeps(context.Background(), deps), &request) @@ -82,7 +89,7 @@ func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { func TestIssueWriteCreateWithParentDoesNotFallbackAfterMutationFailure(t *testing.T) { gqlHTTPClient, gqlCalls := countingGraphQLClient( - createIssueParentMatcher(7, "REPO_1", "ISSUE_parent"), + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), githubv4mock.NewMutationMatcher( createIssueMutation{}, CreateIssueInput{ @@ -124,10 +131,12 @@ func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { serverTool := GranularCreateIssue(translations.NullTranslationHelper) schema := serverTool.Tool.InputSchema.(*jsonschema.Schema) assert.Contains(t, schema.Properties, "parent_issue_number") + assert.Contains(t, schema.Properties, "parent_owner") + assert.Contains(t, schema.Properties, "parent_repo") parentID := githubv4.ID("ISSUE_parent") gqlHTTPClient := githubv4mock.NewMockedHTTPClient( - createIssueParentMatcher(3, "REPO_1", "ISSUE_parent"), + createIssueParentMatcher(3, "owner", "repo", "REPO_1", "ISSUE_parent"), githubv4mock.NewMutationMatcher( createIssueMutation{}, CreateIssueInput{ @@ -165,17 +174,88 @@ func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { assert.False(t, result.IsError) } -func createIssueParentMatcher(parentIssueNumber int, repositoryID, parentIssueID githubv4.ID) githubv4mock.Matcher { +func TestCreateIssueParentRepositoryRequiresParentIssueNumber(t *testing.T) { + tests := []struct { + name string + handler func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) + args map[string]any + want string + }{ + { + name: "issue_write", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_owner": "parent-owner", + }, + want: "can only be used when parent_issue_number is provided", + }, + { + name: "create_issue", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_repo": "parent-repo", + }, + want: "can only be used when parent_issue_number is provided", + }, + { + name: "issue fields", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "field_option_name": "High"}, + }, + }, + want: "issue_fields cannot be used with parent_issue_number", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + request := createMCPRequest(test.args) + result, err := test.handler(ContextWithDeps(context.Background(), BaseDeps{}), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, test.want) + }) + } +} + +func createIssueParentMatcher(parentIssueNumber int, parentOwner, parentRepo string, repositoryID, parentIssueID githubv4.ID) githubv4mock.Matcher { return githubv4mock.NewQueryMatcher( createIssueParentMetadataQuery{}, map[string]any{ "owner": githubv4.String("owner"), "repo": githubv4.String("repo"), + "parentOwner": githubv4.String(parentOwner), + "parentRepo": githubv4.String(parentRepo), "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small }, githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ + "childRepository": map[string]any{ "id": repositoryID, + }, + "parentRepository": map[string]any{ "issue": map[string]any{ "id": parentIssueID, }, diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index ef6fdb2c25..f217cc21d7 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -149,6 +149,14 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT Description: "Issue number of the parent issue. The new issue is created and attached to this parent in the same operation.", Minimum: jsonschema.Ptr(1.0), }, + "parent_owner": { + Type: "string", + Description: "Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided.", + }, + "parent_repo": { + Type: "string", + Description: "Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided.", + }, }, Required: []string{"owner", "repo", "title"}, }, @@ -177,6 +185,17 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT if parentProvided && parentIssueNumber < 1 { return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil, nil } + parentOwner, err := OptionalParam[string](args, "parent_owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + parentRepo, err := OptionalParam[string](args, "parent_repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if !parentProvided && (parentOwner != "" || parentRepo != "") { + return utils.NewToolResultError("parent_owner and parent_repo can only be used when parent_issue_number is provided"), nil, nil + } issueReq := github.CreateIssueRequest{ Title: title, @@ -195,7 +214,7 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, nil, nil, 0, "", nil, parentIssueNumber) + result, err := createIssueWithParent(ctx, client, gqlClient, owner, repo, title, body, nil, nil, 0, "", parentIssueNumber, parentOwner, parentRepo) return result, nil, err } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 95fcef9780..af1fdec89c 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2259,6 +2259,8 @@ func Test_issueWriteHasNonFormParams(t *testing.T) { {name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: false}, {name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: false}, {name: "parent issue present", args: map[string]any{"parent_issue_number": float64(7)}, want: true}, + {name: "parent owner present", args: map[string]any{"parent_owner": "octo-org"}, want: true}, + {name: "parent repo present", args: map[string]any{"parent_repo": "parent-repo"}, want: true}, {name: "unknown non-schema param present", args: map[string]any{"title": "t", "not_a_real_param": "x"}, want: true}, {name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false}, } @@ -2358,11 +2360,13 @@ func Test_issueWriteSchemaClassification(t *testing.T) { t.Parallel() // Schema properties the MCP App form cannot represent — their presence - // must trigger the safety-net bypass via hasNonFormParams. The - // Add a property here only if it is added to the schema without + // must trigger the safety-net bypass via hasNonFormParams. Add a + // property here only if it is added to the schema without // corresponding form support. knownNonForm := map[string]struct{}{ "parent_issue_number": {}, + "parent_owner": {}, + "parent_repo": {}, } cases := []struct { From 8481cdcfd1b420c7e8d61b0fd7210ac5d8670c07 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Fri, 21 Aug 2026 14:46:10 -0400 Subject: [PATCH 3/5] Require complete parent repository coordinates Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f71d9868-eef8-4fb0-84c6-df7c9a6a0ade --- README.md | 4 +-- docs/feature-flags.md | 8 +++--- docs/insiders-features.md | 4 +-- pkg/github/__toolsnaps__/create_issue.snap | 4 +-- pkg/github/__toolsnaps__/issue_write.snap | 4 +-- pkg/github/issues.go | 19 +++++-------- pkg/github/issues_create.go | 21 ++++++++++---- pkg/github/issues_create_test.go | 33 +++++++++++++++++++++- pkg/github/issues_granular.go | 8 +++--- 9 files changed, 71 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 5cf55d7520..2e6d94b84b 100644 --- a/README.md +++ b/README.md @@ -938,8 +938,8 @@ The following sets of tools are available: - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (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) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index d2f5aa3b65..0535631481 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -68,8 +68,8 @@ runtime behavior (such as output formatting) won't appear here. - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (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) @@ -126,8 +126,8 @@ runtime behavior (such as output formatting) won't appear here. - `body`: Issue body content (optional) (string, optional) - `owner`: Repository owner (username or organization) (string, required) - `parent_issue_number`: Issue number of the parent issue. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided. (string, optional) + - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when parent_issue_number is provided. (string, optional) - `repo`: Repository name (string, required) - `title`: Issue title (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 3cf32183e3..c6b1844045 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -62,8 +62,8 @@ The list below is generated from the Go source. It covers tool **inventory and s - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) + - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (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) diff --git a/pkg/github/__toolsnaps__/create_issue.snap b/pkg/github/__toolsnaps__/create_issue.snap index b28668c191..d7de241265 100644 --- a/pkg/github/__toolsnaps__/create_issue.snap +++ b/pkg/github/__toolsnaps__/create_issue.snap @@ -23,11 +23,11 @@ "type": "number" }, "parent_owner": { - "description": "Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided.", + "description": "Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when parent_issue_number is provided.", "type": "string" }, "parent_repo": { - "description": "Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided.", + "description": "Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when parent_issue_number is provided.", "type": "string" }, "repo": { diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index ce8a4c727b..20cc730f51 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -97,11 +97,11 @@ "type": "number" }, "parent_owner": { - "description": "Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided.", + "description": "Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided.", "type": "string" }, "parent_repo": { - "description": "Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided.", + "description": "Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided.", "type": "string" }, "repo": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index df1b02e724..ce088e1ebc 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2326,11 +2326,9 @@ const IssueWriteUIResourceURI = "ui://github-mcp-server/issue-write" // issueWriteFormParams are the parameters the issue_write MCP App form collects // and re-sends on submit. Any other parameter present on a call cannot be -// represented by the form. The form collects (and prefills) every parameter in -// the tool's current input schema, so hasNonFormParams against this set is a -// forward-compatibility safety net: a parameter added to the schema in the -// future but not yet wired into the form trips the check and bypasses the form -// so the supplied value isn't silently dropped. +// represented by the form, so hasNonFormParams bypasses the form rather than +// silently dropping it. Parent issue parameters are intentionally omitted +// because the current form cannot represent them. var issueWriteFormParams = map[string]struct{}{ "method": {}, "owner": {}, @@ -2349,9 +2347,6 @@ var issueWriteFormParams = map[string]struct{}{ "_ui_submitted": {}, } -// Parent issue parameters are intentionally omitted because the current form cannot -// represent them. Calls that supply a parent bypass the form instead of dropping them. - // issueWriteAwaitingFormResult builds the "awaiting form submission" stub // returned when issue_write hands off to the MCP App form. The body is shared // by IssueWrite and LegacyIssueWrite. The result is marked IsError=true so @@ -2435,11 +2430,11 @@ Options are: }, "parent_owner": { Type: "string", - Description: "Repository owner of the parent issue. Defaults to the value of owner. Only used when method is 'create' and parent_issue_number is provided.", + Description: "Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided.", }, "parent_repo": { Type: "string", - Description: "Repository name of the parent issue. Defaults to the value of repo. Only used when method is 'create' and parent_issue_number is provided.", + Description: "Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided.", }, "title": { Type: "string", @@ -2647,8 +2642,8 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if !parentProvided && (parentOwner != "" || parentRepo != "") { - return utils.NewToolResultError("parent_owner and parent_repo can only be used when parent_issue_number is provided"), nil, nil + if err := validateParentRepository(parentProvided, parentOwner, parentRepo); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } var issueFields []issueWriteFieldInput diff --git a/pkg/github/issues_create.go b/pkg/github/issues_create.go index 371e7fe889..0139486558 100644 --- a/pkg/github/issues_create.go +++ b/pkg/github/issues_create.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "strings" @@ -140,15 +141,25 @@ func createIssueWithParent( } func parentRepository(owner, repo, parentOwner, parentRepo string) (string, string) { - if parentOwner == "" { - parentOwner = owner - } - if parentRepo == "" { - parentRepo = repo + if parentOwner == "" && parentRepo == "" { + return owner, repo } return parentOwner, parentRepo } +func validateParentRepository(parentProvided bool, parentOwner, parentRepo string) error { + if !parentProvided { + if parentOwner != "" || parentRepo != "" { + return errors.New("parent_owner and parent_repo can only be used when parent_issue_number is provided") + } + return nil + } + if (parentOwner == "") != (parentRepo == "") { + return errors.New("parent_owner and parent_repo must be provided together") + } + return nil +} + func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo, parentOwner, parentRepo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { var query createIssueParentMetadataQuery variables := map[string]any{ diff --git a/pkg/github/issues_create_test.go b/pkg/github/issues_create_test.go index b9b183683e..cee88c65ad 100644 --- a/pkg/github/issues_create_test.go +++ b/pkg/github/issues_create_test.go @@ -174,7 +174,7 @@ func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { assert.False(t, result.IsError) } -func TestCreateIssueParentRepositoryRequiresParentIssueNumber(t *testing.T) { +func TestCreateIssueParentRepositoryValidation(t *testing.T) { tests := []struct { name string handler func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) @@ -210,6 +210,37 @@ func TestCreateIssueParentRepositoryRequiresParentIssueNumber(t *testing.T) { }, want: "can only be used when parent_issue_number is provided", }, + { + name: "issue_write requires parent repo with parent owner", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "parent_owner": "parent-owner", + }, + want: "parent_owner and parent_repo must be provided together", + }, + { + name: "create_issue requires parent owner with parent repo", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "parent_repo": "parent-repo", + }, + want: "parent_owner and parent_repo must be provided together", + }, { name: "issue fields", handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index f217cc21d7..863ad89ab1 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -151,11 +151,11 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT }, "parent_owner": { Type: "string", - Description: "Repository owner of the parent issue. Defaults to the value of owner. Only used when parent_issue_number is provided.", + Description: "Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when parent_issue_number is provided.", }, "parent_repo": { Type: "string", - Description: "Repository name of the parent issue. Defaults to the value of repo. Only used when parent_issue_number is provided.", + Description: "Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when parent_issue_number is provided.", }, }, Required: []string{"owner", "repo", "title"}, @@ -193,8 +193,8 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if !parentProvided && (parentOwner != "" || parentRepo != "") { - return utils.NewToolResultError("parent_owner and parent_repo can only be used when parent_issue_number is provided"), nil, nil + if err := validateParentRepository(parentProvided, parentOwner, parentRepo); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } issueReq := github.CreateIssueRequest{ From 60c43f9b8b00370d456bc4b144233e131b15cf12 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Fri, 21 Aug 2026 15:02:25 -0400 Subject: [PATCH 4/5] Fold atomic creation into issue tools Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f71d9868-eef8-4fb0-84c6-df7c9a6a0ade --- pkg/github/issues.go | 231 ++++++++++++++++++++++++++++++++++ pkg/github/issues_create.go | 245 ------------------------------------ 2 files changed, 231 insertions(+), 245 deletions(-) delete mode 100644 pkg/github/issues_create.go diff --git a/pkg/github/issues.go b/pkg/github/issues.go index ce088e1ebc..dcb01fd2b8 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -2702,6 +2703,236 @@ Options are: return st } +type CreateIssueInput struct { + RepositoryID githubv4.ID `json:"repositoryId"` + Title githubv4.String `json:"title"` + + Body *githubv4.String `json:"body,omitempty"` + AssigneeIDs *[]githubv4.ID `json:"assigneeIds,omitempty"` + MilestoneID *githubv4.ID `json:"milestoneId,omitempty"` + LabelIDs *[]githubv4.ID `json:"labelIds,omitempty"` + IssueTypeID *githubv4.ID `json:"issueTypeId,omitempty"` + ParentIssueID *githubv4.ID `json:"parentIssueId,omitempty"` +} + +type createIssueMutation struct { + CreateIssue struct { + Issue struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + URL githubv4.URI + } + } `graphql:"createIssue(input: $input)"` +} + +type createIssueParentMetadataQuery struct { + ChildRepository struct { + ID githubv4.ID + } `graphql:"childRepository: repository(owner: $owner, name: $repo)"` + ParentRepository struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $parentIssueNumber)"` + } `graphql:"parentRepository: repository(owner: $parentOwner, name: $parentRepo)"` +} + +func createIssueWithParent( + ctx context.Context, + client *github.Client, + gqlClient *githubv4.Client, + owner string, + repo string, + title string, + body string, + assignees []string, + labels []string, + milestoneNumber int, + issueType string, + parentIssueNumber int, + parentOwner string, + parentRepo string, +) (*mcp.CallToolResult, error) { + if title == "" { + return utils.NewToolResultError("missing required parameter: title"), nil + } + if parentIssueNumber < 1 { + return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil + } + + parentOwner, parentRepo = parentRepository(owner, repo, parentOwner, parentRepo) + repositoryID, parentIssueID, err := resolveCreateIssueParent(ctx, gqlClient, owner, repo, parentOwner, parentRepo, parentIssueNumber) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve parent issue", err), nil + } + + input := CreateIssueInput{ + RepositoryID: repositoryID, + Title: githubv4.String(title), + ParentIssueID: &parentIssueID, + } + if body != "" { + input.Body = githubv4.NewString(githubv4.String(body)) + } + + if len(labels) > 0 { + labelIDs := make([]githubv4.ID, 0, len(labels)) + for _, label := range labels { + labelID, err := getLabelID(ctx, gqlClient, owner, repo, label) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve label %q", label), err), nil + } + labelIDs = append(labelIDs, labelID) + } + input.LabelIDs = &labelIDs + } + + if len(assignees) > 0 { + assigneeIDs := make([]githubv4.ID, 0, len(assignees)) + for _, assignee := range assignees { + assigneeID, err := resolveUserID(ctx, gqlClient, assignee) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve assignee %q", assignee), err), nil + } + assigneeIDs = append(assigneeIDs, assigneeID) + } + input.AssigneeIDs = &assigneeIDs + } + + if milestoneNumber != 0 { + milestoneID, err := resolveMilestoneID(ctx, gqlClient, owner, repo, milestoneNumber) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve milestone", err), nil + } + input.MilestoneID = &milestoneID + } + + if issueType != "" { + issueTypeID, resp, err := resolveIssueTypeID(ctx, client, owner, repo, issueType) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to resolve issue type %q", issueType), resp, err), nil + } + input.IssueTypeID = &issueTypeID + } + + var mutation createIssueMutation + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to create issue", err), nil + } + + response := MinimalResponse{ + ID: string(mutation.CreateIssue.Issue.FullDatabaseID), + URL: mutation.CreateIssue.Issue.URL.String(), + } + encoded, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil + } + return utils.NewToolResultText(string(encoded)), nil +} + +func parentRepository(owner, repo, parentOwner, parentRepo string) (string, string) { + if parentOwner == "" && parentRepo == "" { + return owner, repo + } + return parentOwner, parentRepo +} + +func validateParentRepository(parentProvided bool, parentOwner, parentRepo string) error { + if !parentProvided { + if parentOwner != "" || parentRepo != "" { + return errors.New("parent_owner and parent_repo can only be used when parent_issue_number is provided") + } + return nil + } + if (parentOwner == "") != (parentRepo == "") { + return errors.New("parent_owner and parent_repo must be provided together") + } + return nil +} + +func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo, parentOwner, parentRepo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { + var query createIssueParentMetadataQuery + variables := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "parentOwner": githubv4.String(parentOwner), + "parentRepo": githubv4.String(parentRepo), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - issue numbers are small positive integers + } + if err := gqlClient.Query(ctx, &query, variables); err != nil { + return "", "", err + } + if query.ChildRepository.ID == "" { + return "", "", fmt.Errorf("repository %s/%s was not found", owner, repo) + } + if query.ParentRepository.Issue.ID == "" { + return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, parentOwner, parentRepo) + } + return query.ChildRepository.ID, query.ParentRepository.Issue.ID, nil +} + +func resolveUserID(ctx context.Context, gqlClient *githubv4.Client, login string) (githubv4.ID, error) { + var query struct { + User struct { + ID githubv4.ID + Login githubv4.String + } `graphql:"user(login: $login)"` + } + if err := gqlClient.Query(ctx, &query, map[string]any{"login": githubv4.String(login)}); err != nil { + return "", err + } + if query.User.ID == "" { + return "", fmt.Errorf("user %q was not found", login) + } + return query.User.ID, nil +} + +func resolveMilestoneID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, milestoneNumber int) (githubv4.ID, error) { + var query struct { + Repository struct { + Milestone struct { + ID githubv4.ID + } `graphql:"milestone(number: $milestoneNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + variables := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "milestoneNumber": githubv4.Int(milestoneNumber), // #nosec G115 - milestone numbers are small positive integers + } + if err := gqlClient.Query(ctx, &query, variables); err != nil { + return "", err + } + if query.Repository.Milestone.ID == "" { + return "", fmt.Errorf("milestone #%d was not found in %s/%s", milestoneNumber, owner, repo) + } + return query.Repository.Milestone.ID, nil +} + +func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo, issueTypeName string) (githubv4.ID, *github.Response, error) { + req, err := client.NewRequest(ctx, "GET", fmt.Sprintf("repos/%s/%s/issue-types", owner, repo), nil) + if err != nil { + return "", nil, err + } + + var issueTypes []*github.IssueType + resp, err := client.Do(req, &issueTypes) + if resp != nil && resp.Body != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return "", resp, err + } + for _, issueType := range issueTypes { + if issueType != nil && strings.EqualFold(strings.TrimSpace(issueType.GetName()), strings.TrimSpace(issueTypeName)) { + if issueType.GetNodeID() == "" { + return "", resp, fmt.Errorf("issue type %q is missing a node ID", issueTypeName) + } + return githubv4.ID(issueType.GetNodeID()), resp, nil + } + } + return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo) +} + func CreateIssue(ctx context.Context, client *github.Client, owner string, repo string, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue) (*mcp.CallToolResult, error) { if title == "" { return utils.NewToolResultError("missing required parameter: title"), nil diff --git a/pkg/github/issues_create.go b/pkg/github/issues_create.go deleted file mode 100644 index 0139486558..0000000000 --- a/pkg/github/issues_create.go +++ /dev/null @@ -1,245 +0,0 @@ -package github - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "strings" - - ghErrors "github.com/github/github-mcp-server/pkg/errors" - "github.com/github/github-mcp-server/pkg/utils" - "github.com/google/go-github/v89/github" - "github.com/modelcontextprotocol/go-sdk/mcp" - "github.com/shurcooL/githubv4" -) - -type CreateIssueInput struct { - RepositoryID githubv4.ID `json:"repositoryId"` - Title githubv4.String `json:"title"` - - Body *githubv4.String `json:"body,omitempty"` - AssigneeIDs *[]githubv4.ID `json:"assigneeIds,omitempty"` - MilestoneID *githubv4.ID `json:"milestoneId,omitempty"` - LabelIDs *[]githubv4.ID `json:"labelIds,omitempty"` - IssueTypeID *githubv4.ID `json:"issueTypeId,omitempty"` - ParentIssueID *githubv4.ID `json:"parentIssueId,omitempty"` -} - -type createIssueMutation struct { - CreateIssue struct { - Issue struct { - FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` - URL githubv4.URI - } - } `graphql:"createIssue(input: $input)"` -} - -type createIssueParentMetadataQuery struct { - ChildRepository struct { - ID githubv4.ID - } `graphql:"childRepository: repository(owner: $owner, name: $repo)"` - ParentRepository struct { - Issue struct { - ID githubv4.ID - } `graphql:"issue(number: $parentIssueNumber)"` - } `graphql:"parentRepository: repository(owner: $parentOwner, name: $parentRepo)"` -} - -func createIssueWithParent( - ctx context.Context, - client *github.Client, - gqlClient *githubv4.Client, - owner string, - repo string, - title string, - body string, - assignees []string, - labels []string, - milestoneNumber int, - issueType string, - parentIssueNumber int, - parentOwner string, - parentRepo string, -) (*mcp.CallToolResult, error) { - if title == "" { - return utils.NewToolResultError("missing required parameter: title"), nil - } - if parentIssueNumber < 1 { - return utils.NewToolResultError("parent_issue_number must be greater than 0"), nil - } - - parentOwner, parentRepo = parentRepository(owner, repo, parentOwner, parentRepo) - repositoryID, parentIssueID, err := resolveCreateIssueParent(ctx, gqlClient, owner, repo, parentOwner, parentRepo, parentIssueNumber) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve parent issue", err), nil - } - - input := CreateIssueInput{ - RepositoryID: repositoryID, - Title: githubv4.String(title), - ParentIssueID: &parentIssueID, - } - if body != "" { - input.Body = githubv4.NewString(githubv4.String(body)) - } - - if len(labels) > 0 { - labelIDs := make([]githubv4.ID, 0, len(labels)) - for _, label := range labels { - labelID, err := getLabelID(ctx, gqlClient, owner, repo, label) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve label %q", label), err), nil - } - labelIDs = append(labelIDs, labelID) - } - input.LabelIDs = &labelIDs - } - - if len(assignees) > 0 { - assigneeIDs := make([]githubv4.ID, 0, len(assignees)) - for _, assignee := range assignees { - assigneeID, err := resolveUserID(ctx, gqlClient, assignee) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, fmt.Sprintf("failed to resolve assignee %q", assignee), err), nil - } - assigneeIDs = append(assigneeIDs, assigneeID) - } - input.AssigneeIDs = &assigneeIDs - } - - if milestoneNumber != 0 { - milestoneID, err := resolveMilestoneID(ctx, gqlClient, owner, repo, milestoneNumber) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve milestone", err), nil - } - input.MilestoneID = &milestoneID - } - - if issueType != "" { - issueTypeID, resp, err := resolveIssueTypeID(ctx, client, owner, repo, issueType) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to resolve issue type %q", issueType), resp, err), nil - } - input.IssueTypeID = &issueTypeID - } - - var mutation createIssueMutation - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to create issue", err), nil - } - - response := MinimalResponse{ - ID: string(mutation.CreateIssue.Issue.FullDatabaseID), - URL: mutation.CreateIssue.Issue.URL.String(), - } - encoded, err := json.Marshal(response) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil - } - return utils.NewToolResultText(string(encoded)), nil -} - -func parentRepository(owner, repo, parentOwner, parentRepo string) (string, string) { - if parentOwner == "" && parentRepo == "" { - return owner, repo - } - return parentOwner, parentRepo -} - -func validateParentRepository(parentProvided bool, parentOwner, parentRepo string) error { - if !parentProvided { - if parentOwner != "" || parentRepo != "" { - return errors.New("parent_owner and parent_repo can only be used when parent_issue_number is provided") - } - return nil - } - if (parentOwner == "") != (parentRepo == "") { - return errors.New("parent_owner and parent_repo must be provided together") - } - return nil -} - -func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, owner, repo, parentOwner, parentRepo string, parentIssueNumber int) (githubv4.ID, githubv4.ID, error) { - var query createIssueParentMetadataQuery - variables := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "parentOwner": githubv4.String(parentOwner), - "parentRepo": githubv4.String(parentRepo), - "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - issue numbers are small positive integers - } - if err := gqlClient.Query(ctx, &query, variables); err != nil { - return "", "", err - } - if query.ChildRepository.ID == "" { - return "", "", fmt.Errorf("repository %s/%s was not found", owner, repo) - } - if query.ParentRepository.Issue.ID == "" { - return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, parentOwner, parentRepo) - } - return query.ChildRepository.ID, query.ParentRepository.Issue.ID, nil -} - -func resolveUserID(ctx context.Context, gqlClient *githubv4.Client, login string) (githubv4.ID, error) { - var query struct { - User struct { - ID githubv4.ID - Login githubv4.String - } `graphql:"user(login: $login)"` - } - if err := gqlClient.Query(ctx, &query, map[string]any{"login": githubv4.String(login)}); err != nil { - return "", err - } - if query.User.ID == "" { - return "", fmt.Errorf("user %q was not found", login) - } - return query.User.ID, nil -} - -func resolveMilestoneID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, milestoneNumber int) (githubv4.ID, error) { - var query struct { - Repository struct { - Milestone struct { - ID githubv4.ID - } `graphql:"milestone(number: $milestoneNumber)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - variables := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "milestoneNumber": githubv4.Int(milestoneNumber), // #nosec G115 - milestone numbers are small positive integers - } - if err := gqlClient.Query(ctx, &query, variables); err != nil { - return "", err - } - if query.Repository.Milestone.ID == "" { - return "", fmt.Errorf("milestone #%d was not found in %s/%s", milestoneNumber, owner, repo) - } - return query.Repository.Milestone.ID, nil -} - -func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo, issueTypeName string) (githubv4.ID, *github.Response, error) { - req, err := client.NewRequest(ctx, "GET", fmt.Sprintf("repos/%s/%s/issue-types", owner, repo), nil) - if err != nil { - return "", nil, err - } - - var issueTypes []*github.IssueType - resp, err := client.Do(req, &issueTypes) - if resp != nil && resp.Body != nil { - defer func() { _ = resp.Body.Close() }() - } - if err != nil { - return "", resp, err - } - for _, issueType := range issueTypes { - if issueType != nil && strings.EqualFold(strings.TrimSpace(issueType.GetName()), strings.TrimSpace(issueTypeName)) { - if issueType.GetNodeID() == "" { - return "", resp, fmt.Errorf("issue type %q is missing a node ID", issueTypeName) - } - return githubv4.ID(issueType.GetNodeID()), resp, nil - } - } - return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo) -} From 51ff1a967ebdd2ea5013a456f42860dfc3207293 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Fri, 21 Aug 2026 15:33:49 -0400 Subject: [PATCH 5/5] Harden atomic issue creation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f71d9868-eef8-4fb0-84c6-df7c9a6a0ade --- pkg/github/issues.go | 20 +- pkg/github/issues_create_test.go | 322 -------------- pkg/github/issues_test.go | 708 +++++++++++++++++++++++++++++++ 3 files changed, 721 insertions(+), 329 deletions(-) delete mode 100644 pkg/github/issues_create_test.go diff --git a/pkg/github/issues.go b/pkg/github/issues.go index dcb01fd2b8..5767d2ec96 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2726,11 +2726,13 @@ type createIssueMutation struct { type createIssueParentMetadataQuery struct { ChildRepository struct { - ID githubv4.ID + ID githubv4.ID + NameWithOwner githubv4.String } `graphql:"childRepository: repository(owner: $owner, name: $repo)"` ParentRepository struct { Issue struct { - ID githubv4.ID + ID githubv4.ID + Number githubv4.Int } `graphql:"issue(number: $parentIssueNumber)"` } `graphql:"parentRepository: repository(owner: $parentOwner, name: $parentRepo)"` } @@ -2817,6 +2819,9 @@ func createIssueWithParent( if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to create issue", err), nil } + if mutation.CreateIssue.Issue.FullDatabaseID == "" || mutation.CreateIssue.Issue.URL.URL == nil { + return utils.NewToolResultError("failed to create issue: response did not include the created issue"), nil + } response := MinimalResponse{ ID: string(mutation.CreateIssue.Issue.FullDatabaseID), @@ -2861,10 +2866,10 @@ func resolveCreateIssueParent(ctx context.Context, gqlClient *githubv4.Client, o if err := gqlClient.Query(ctx, &query, variables); err != nil { return "", "", err } - if query.ChildRepository.ID == "" { + if query.ChildRepository.NameWithOwner == "" { return "", "", fmt.Errorf("repository %s/%s was not found", owner, repo) } - if query.ParentRepository.Issue.ID == "" { + if query.ParentRepository.Issue.Number == 0 { return "", "", fmt.Errorf("parent issue #%d was not found in %s/%s", parentIssueNumber, parentOwner, parentRepo) } return query.ChildRepository.ID, query.ParentRepository.Issue.ID, nil @@ -2880,7 +2885,7 @@ func resolveUserID(ctx context.Context, gqlClient *githubv4.Client, login string if err := gqlClient.Query(ctx, &query, map[string]any{"login": githubv4.String(login)}); err != nil { return "", err } - if query.User.ID == "" { + if query.User.Login == "" { return "", fmt.Errorf("user %q was not found", login) } return query.User.ID, nil @@ -2890,7 +2895,8 @@ func resolveMilestoneID(ctx context.Context, gqlClient *githubv4.Client, owner, var query struct { Repository struct { Milestone struct { - ID githubv4.ID + ID githubv4.ID + Number githubv4.Int } `graphql:"milestone(number: $milestoneNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -2902,7 +2908,7 @@ func resolveMilestoneID(ctx context.Context, gqlClient *githubv4.Client, owner, if err := gqlClient.Query(ctx, &query, variables); err != nil { return "", err } - if query.Repository.Milestone.ID == "" { + if query.Repository.Milestone.Number == 0 { return "", fmt.Errorf("milestone #%d was not found in %s/%s", milestoneNumber, owner, repo) } return query.Repository.Milestone.ID, nil diff --git a/pkg/github/issues_create_test.go b/pkg/github/issues_create_test.go deleted file mode 100644 index cee88c65ad..0000000000 --- a/pkg/github/issues_create_test.go +++ /dev/null @@ -1,322 +0,0 @@ -package github - -import ( - "context" - "encoding/json" - "testing" - - "github.com/github/github-mcp-server/internal/githubv4mock" - "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/jsonschema-go/jsonschema" - "github.com/modelcontextprotocol/go-sdk/mcp" - "github.com/shurcooL/githubv4" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { - serverTool := IssueWrite(translations.NullTranslationHelper) - schema := serverTool.Tool.InputSchema - issueWriteSchema := schema.(*jsonschema.Schema) - assert.Contains(t, issueWriteSchema.Properties, "parent_issue_number") - assert.Contains(t, issueWriteSchema.Properties, "parent_owner") - assert.Contains(t, issueWriteSchema.Properties, "parent_repo") - assert.NotContains(t, issueWriteSchema.Required, "parent_issue_number") - assert.NotContains(t, issueWriteSchema.Required, "parent_owner") - assert.NotContains(t, issueWriteSchema.Required, "parent_repo") - - labelIDs := []githubv4.ID{"LABEL_backlog"} - parentID := githubv4.ID("ISSUE_parent") - expectedInput := CreateIssueInput{ - RepositoryID: githubv4.ID("REPO_1"), - Title: githubv4.String("Atomic child"), - Body: githubv4.NewString(githubv4.String("Created under its parent")), - LabelIDs: &labelIDs, - ParentIssueID: &parentID, - } - createMatcher := githubv4mock.NewMutationMatcher( - createIssueMutation{}, - expectedInput, - nil, - githubv4mock.DataResponse(map[string]any{ - "createIssue": map[string]any{ - "issue": map[string]any{ - "fullDatabaseId": "12345", - "url": "https://github.com/owner/repo/issues/2", - }, - }, - }), - ) - assert.Contains(t, createMatcher.Request, "$input:CreateIssueInput!") - - gqlHTTPClient, gqlCalls := countingGraphQLClient( - createIssueParentMatcher(1, "parent-owner", "parent-repo", "REPO_1", "ISSUE_parent"), - createIssueLabelMatcher("status:backlog", "LABEL_backlog"), - createMatcher, - ) - restHTTPClient := MockHTTPClientWithHandlers(nil) - restCounter := &countingRoundTripper{next: restHTTPClient.Transport} - restHTTPClient.Transport = restCounter - - deps := BaseDeps{ - Client: mustNewGHClient(t, restHTTPClient), - GQLClient: githubv4.NewClient(gqlHTTPClient), - } - handler := serverTool.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "create", - "owner": "owner", - "repo": "repo", - "title": "Atomic child", - "body": "Created under its parent", - "labels": []any{"status:backlog"}, - "parent_issue_number": float64(1), - "parent_owner": "parent-owner", - "parent_repo": "parent-repo", - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, getTextResult(t, result).Text) - assert.Equal(t, 3, gqlCalls(), "metadata lookups and exactly one create mutation are expected") - assert.Zero(t, restCounter.count.Load(), "parent creation must not use REST create or attachment requests") - - var response MinimalResponse - require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) - assert.Equal(t, "12345", response.ID) - assert.Equal(t, "https://github.com/owner/repo/issues/2", response.URL) -} - -func TestIssueWriteCreateWithParentDoesNotFallbackAfterMutationFailure(t *testing.T) { - gqlHTTPClient, gqlCalls := countingGraphQLClient( - createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), - githubv4mock.NewMutationMatcher( - createIssueMutation{}, - CreateIssueInput{ - RepositoryID: githubv4.ID("REPO_1"), - Title: githubv4.String("Atomic child"), - ParentIssueID: githubv4mock.Ptr[githubv4.ID]("ISSUE_parent"), - }, - nil, - githubv4mock.ErrorResponse("parent cannot accept sub-issues"), - ), - ) - restHTTPClient := MockHTTPClientWithHandlers(nil) - restCounter := &countingRoundTripper{next: restHTTPClient.Transport} - restHTTPClient.Transport = restCounter - - deps := BaseDeps{ - Client: mustNewGHClient(t, restHTTPClient), - GQLClient: githubv4.NewClient(gqlHTTPClient), - } - serverTool := IssueWrite(translations.NullTranslationHelper) - handler := serverTool.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "create", - "owner": "owner", - "repo": "repo", - "title": "Atomic child", - "parent_issue_number": float64(7), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.True(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, "failed to create issue") - assert.Equal(t, 2, gqlCalls(), "a failed create mutation must not trigger an attachment mutation") - assert.Zero(t, restCounter.count.Load(), "a failed create mutation must not fall back to REST create or attachment requests") -} - -func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { - serverTool := GranularCreateIssue(translations.NullTranslationHelper) - schema := serverTool.Tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "parent_issue_number") - assert.Contains(t, schema.Properties, "parent_owner") - assert.Contains(t, schema.Properties, "parent_repo") - - parentID := githubv4.ID("ISSUE_parent") - gqlHTTPClient := githubv4mock.NewMockedHTTPClient( - createIssueParentMatcher(3, "owner", "repo", "REPO_1", "ISSUE_parent"), - githubv4mock.NewMutationMatcher( - createIssueMutation{}, - CreateIssueInput{ - RepositoryID: githubv4.ID("REPO_1"), - Title: githubv4.String("Granular child"), - ParentIssueID: &parentID, - }, - nil, - githubv4mock.DataResponse(map[string]any{ - "createIssue": map[string]any{ - "issue": map[string]any{ - "fullDatabaseId": "23456", - "url": "https://github.com/owner/repo/issues/4", - }, - }, - }), - ), - ) - restHTTPClient := MockHTTPClientWithHandlers(nil) - - deps := BaseDeps{ - Client: mustNewGHClient(t, restHTTPClient), - GQLClient: githubv4.NewClient(gqlHTTPClient), - } - handler := serverTool.Handler(deps) - request := createMCPRequest(map[string]any{ - "owner": "owner", - "repo": "repo", - "title": "Granular child", - "parent_issue_number": float64(3), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - assert.False(t, result.IsError) -} - -func TestCreateIssueParentRepositoryValidation(t *testing.T) { - tests := []struct { - name string - handler func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) - args map[string]any - want string - }{ - { - name: "issue_write", - handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - serverTool := IssueWrite(translations.NullTranslationHelper) - return serverTool.Handler(BaseDeps{})(ctx, request) - }, - args: map[string]any{ - "method": "create", - "owner": "owner", - "repo": "repo", - "title": "Child", - "parent_owner": "parent-owner", - }, - want: "can only be used when parent_issue_number is provided", - }, - { - name: "create_issue", - handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - serverTool := GranularCreateIssue(translations.NullTranslationHelper) - return serverTool.Handler(BaseDeps{})(ctx, request) - }, - args: map[string]any{ - "owner": "owner", - "repo": "repo", - "title": "Child", - "parent_repo": "parent-repo", - }, - want: "can only be used when parent_issue_number is provided", - }, - { - name: "issue_write requires parent repo with parent owner", - handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - serverTool := IssueWrite(translations.NullTranslationHelper) - return serverTool.Handler(BaseDeps{})(ctx, request) - }, - args: map[string]any{ - "method": "create", - "owner": "owner", - "repo": "repo", - "title": "Child", - "parent_issue_number": float64(1), - "parent_owner": "parent-owner", - }, - want: "parent_owner and parent_repo must be provided together", - }, - { - name: "create_issue requires parent owner with parent repo", - handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - serverTool := GranularCreateIssue(translations.NullTranslationHelper) - return serverTool.Handler(BaseDeps{})(ctx, request) - }, - args: map[string]any{ - "owner": "owner", - "repo": "repo", - "title": "Child", - "parent_issue_number": float64(1), - "parent_repo": "parent-repo", - }, - want: "parent_owner and parent_repo must be provided together", - }, - { - name: "issue fields", - handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - serverTool := IssueWrite(translations.NullTranslationHelper) - return serverTool.Handler(BaseDeps{})(ctx, request) - }, - args: map[string]any{ - "method": "create", - "owner": "owner", - "repo": "repo", - "title": "Child", - "parent_issue_number": float64(1), - "issue_fields": []any{ - map[string]any{"field_name": "Priority", "field_option_name": "High"}, - }, - }, - want: "issue_fields cannot be used with parent_issue_number", - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - request := createMCPRequest(test.args) - result, err := test.handler(ContextWithDeps(context.Background(), BaseDeps{}), &request) - require.NoError(t, err) - require.True(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, test.want) - }) - } -} - -func createIssueParentMatcher(parentIssueNumber int, parentOwner, parentRepo string, repositoryID, parentIssueID githubv4.ID) githubv4mock.Matcher { - return githubv4mock.NewQueryMatcher( - createIssueParentMetadataQuery{}, - map[string]any{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "parentOwner": githubv4.String(parentOwner), - "parentRepo": githubv4.String(parentRepo), - "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small - }, - githubv4mock.DataResponse(map[string]any{ - "childRepository": map[string]any{ - "id": repositoryID, - }, - "parentRepository": map[string]any{ - "issue": map[string]any{ - "id": parentIssueID, - }, - }, - }), - ) -} - -func createIssueLabelMatcher(name string, id githubv4.ID) githubv4mock.Matcher { - return githubv4mock.NewQueryMatcher( - struct { - Repository struct { - Label struct { - ID githubv4.ID - Name githubv4.String - } `graphql:"label(name: $name)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - }{}, - map[string]any{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "name": githubv4.String(name), - }, - githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "label": map[string]any{ - "id": id, - "name": name, - }, - }, - }), - ) -} diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index af1fdec89c..bf024b545a 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -22,6 +22,7 @@ import ( "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -2240,6 +2241,713 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { }) } +func TestIssueWriteCreateWithParentAndLabelsUsesSingleMutation(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + schema := serverTool.Tool.InputSchema + issueWriteSchema := schema.(*jsonschema.Schema) + assert.Contains(t, issueWriteSchema.Properties, "parent_issue_number") + assert.Contains(t, issueWriteSchema.Properties, "parent_owner") + assert.Contains(t, issueWriteSchema.Properties, "parent_repo") + assert.NotContains(t, issueWriteSchema.Required, "parent_issue_number") + assert.NotContains(t, issueWriteSchema.Required, "parent_owner") + assert.NotContains(t, issueWriteSchema.Required, "parent_repo") + + labelIDs := []githubv4.ID{"LABEL_backlog"} + parentID := githubv4.ID("ISSUE_parent") + expectedInput := CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Atomic child"), + Body: githubv4.NewString(githubv4.String("Created under its parent")), + LabelIDs: &labelIDs, + ParentIssueID: &parentID, + } + createMatcher := githubv4mock.NewMutationMatcher( + createIssueMutation{}, + expectedInput, + nil, + githubv4mock.DataResponse(map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{ + "fullDatabaseId": "12345", + "url": "https://github.com/owner/repo/issues/2", + }, + }, + }), + ) + assert.Contains(t, createMatcher.Request, "$input:CreateIssueInput!") + + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(1, "parent-owner", "parent-repo", "REPO_1", "ISSUE_parent"), + createIssueLabelMatcher("status:backlog", "LABEL_backlog"), + createMatcher, + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "body": "Created under its parent", + "labels": []any{"status:backlog"}, + "parent_issue_number": float64(1), + "parent_owner": "parent-owner", + "parent_repo": "parent-repo", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Equal(t, 3, gqlCalls(), "metadata lookups and exactly one create mutation are expected") + assert.Zero(t, restCounter.count.Load(), "parent creation must not use REST create or attachment requests") + + var response MinimalResponse + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, "12345", response.ID) + assert.Equal(t, "https://github.com/owner/repo/issues/2", response.URL) +} + +func TestIssueWriteCreateWithParentDoesNotFallbackAfterMutationFailure(t *testing.T) { + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Atomic child"), + ParentIssueID: githubv4mock.Ptr[githubv4.ID]("ISSUE_parent"), + }, + nil, + githubv4mock.ErrorResponse("parent cannot accept sub-issues"), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + serverTool := IssueWrite(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "parent_issue_number": float64(7), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "failed to create issue") + assert.Equal(t, 2, gqlCalls(), "a failed create mutation must not trigger an attachment mutation") + assert.Zero(t, restCounter.count.Load(), "a failed create mutation must not fall back to REST create or attachment requests") +} + +func TestIssueWriteCreateWithParentRejectsIncompleteMutationResponse(t *testing.T) { + tests := []struct { + name string + data map[string]any + }{ + { + name: "missing issue", + data: map[string]any{"createIssue": map[string]any{"issue": nil}}, + }, + { + name: "missing database ID", + data: map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{"url": "https://github.com/owner/repo/issues/8"}, + }, + }, + }, + { + name: "missing URL", + data: map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{"fullDatabaseId": "34567"}, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Atomic child"), + ParentIssueID: githubv4mock.Ptr[githubv4.ID]("ISSUE_parent"), + }, + nil, + githubv4mock.DataResponse(test.data), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "parent_issue_number": float64(7), + }) + + serverTool := IssueWrite(translations.NullTranslationHelper) + result, err := serverTool.Handler(deps)( + ContextWithDeps(context.Background(), deps), + &request, + ) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "response did not include the created issue") + assert.Equal(t, 2, gqlCalls()) + assert.Zero(t, restCounter.count.Load()) + }) + } +} + +func TestIssueWriteCreateWithParentPreservesSupportedFields(t *testing.T) { + labelIDs := []githubv4.ID{"LABEL_bug"} + assigneeIDs := []githubv4.ID{"USER_octocat"} + milestoneID := githubv4.ID("MILESTONE_1") + issueTypeID := githubv4.ID("ISSUE_TYPE_bug") + parentID := githubv4.ID("ISSUE_parent") + expectedInput := CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Fully specified child"), + Body: githubv4.NewString(githubv4.String("Body")), + AssigneeIDs: &assigneeIDs, + MilestoneID: &milestoneID, + LabelIDs: &labelIDs, + IssueTypeID: &issueTypeID, + ParentIssueID: &parentID, + } + gqlHTTPClient, gqlCalls := countingGraphQLClient( + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), + createIssueLabelMatcher("bug", "LABEL_bug"), + createIssueUserMatcher("octocat", "USER_octocat"), + createIssueMilestoneMatcher(1, "MILESTONE_1"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + expectedInput, + nil, + githubv4mock.DataResponse(map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{ + "fullDatabaseId": "34567", + "url": "https://github.com/owner/repo/issues/8", + }, + }, + }), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/issue-types": mockResponse(t, http.StatusOK, []*github.IssueType{ + {Name: github.Ptr("Bug"), NodeID: github.Ptr("ISSUE_TYPE_bug")}, + }), + }) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + request := createMCPRequest(map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Fully specified child", + "body": "Body", + "assignees": []any{"octocat"}, + "labels": []any{"bug"}, + "milestone": float64(1), + "type": "Bug", + "parent_issue_number": float64(7), + }) + + serverTool := IssueWrite(translations.NullTranslationHelper) + result, err := serverTool.Handler(deps)( + ContextWithDeps(context.Background(), deps), + &request, + ) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Equal(t, 5, gqlCalls(), "four metadata lookups and exactly one create mutation are expected") + assert.Equal(t, int64(1), restCounter.count.Load(), "issue type resolution is the only expected REST call") +} + +func TestIssueWriteCreateWithParentRejectsMissingMetadata(t *testing.T) { + tests := []struct { + name string + args map[string]any + gqlMatchers []githubv4mock.Matcher + restHandlers map[string]http.HandlerFunc + want string + wantGQLCalls int + wantRESTCalls int64 + }{ + { + name: "child repository", + gqlMatchers: []githubv4mock.Matcher{ + createIssueMissingChildRepositoryMatcher(7), + }, + want: "failed to resolve parent issue", + wantGQLCalls: 1, + }, + { + name: "parent issue", + gqlMatchers: []githubv4mock.Matcher{ + createIssueMissingParentMatcher(7), + }, + want: "failed to resolve parent issue", + wantGQLCalls: 1, + }, + { + name: "milestone", + args: map[string]any{"milestone": float64(99)}, + gqlMatchers: []githubv4mock.Matcher{ + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), + createIssueMissingMilestoneMatcher(99), + }, + want: "failed to resolve milestone", + wantGQLCalls: 2, + }, + { + name: "assignee", + args: map[string]any{"assignees": []any{"missing-user"}}, + gqlMatchers: []githubv4mock.Matcher{ + createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent"), + createIssueMissingUserMatcher("missing-user"), + }, + want: `failed to resolve assignee "missing-user"`, + wantGQLCalls: 2, + }, + { + name: "issue type", + args: map[string]any{"type": "Missing"}, + gqlMatchers: []githubv4mock.Matcher{createIssueParentMatcher(7, "owner", "repo", "REPO_1", "ISSUE_parent")}, + restHandlers: map[string]http.HandlerFunc{ + "GET /repos/{owner}/{repo}/issue-types": mockResponse(t, http.StatusOK, []*github.IssueType{}), + }, + want: `failed to resolve issue type "Missing"`, + wantGQLCalls: 1, + wantRESTCalls: 1, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gqlHTTPClient, gqlCalls := countingGraphQLClient(test.gqlMatchers...) + restHTTPClient := MockHTTPClientWithHandlers(test.restHandlers) + restCounter := &countingRoundTripper{next: restHTTPClient.Transport} + restHTTPClient.Transport = restCounter + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + args := map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Atomic child", + "parent_issue_number": float64(7), + } + maps.Copy(args, test.args) + request := createMCPRequest(args) + + serverTool := IssueWrite(translations.NullTranslationHelper) + result, err := serverTool.Handler(deps)( + ContextWithDeps(context.Background(), deps), + &request, + ) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, test.want) + assert.Equal(t, test.wantGQLCalls, gqlCalls()) + assert.Equal(t, test.wantRESTCalls, restCounter.count.Load()) + }) + } +} + +func TestGranularCreateIssueWithParentUsesAtomicMutation(t *testing.T) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + schema := serverTool.Tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "parent_issue_number") + assert.Contains(t, schema.Properties, "parent_owner") + assert.Contains(t, schema.Properties, "parent_repo") + + parentID := githubv4.ID("ISSUE_parent") + gqlHTTPClient := githubv4mock.NewMockedHTTPClient( + createIssueParentMatcher(3, "owner", "repo", "REPO_1", "ISSUE_parent"), + githubv4mock.NewMutationMatcher( + createIssueMutation{}, + CreateIssueInput{ + RepositoryID: githubv4.ID("REPO_1"), + Title: githubv4.String("Granular child"), + ParentIssueID: &parentID, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createIssue": map[string]any{ + "issue": map[string]any{ + "fullDatabaseId": "23456", + "url": "https://github.com/owner/repo/issues/4", + }, + }, + }), + ), + ) + restHTTPClient := MockHTTPClientWithHandlers(nil) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Granular child", + "parent_issue_number": float64(3), + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestCreateIssueParentRepositoryValidation(t *testing.T) { + tests := []struct { + name string + handler func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) + args map[string]any + want string + }{ + { + name: "issue_write", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_owner": "parent-owner", + }, + want: "can only be used when parent_issue_number is provided", + }, + { + name: "create_issue", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_repo": "parent-repo", + }, + want: "can only be used when parent_issue_number is provided", + }, + { + name: "issue_write requires parent repo with parent owner", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "parent_owner": "parent-owner", + }, + want: "parent_owner and parent_repo must be provided together", + }, + { + name: "create_issue requires parent owner with parent repo", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "parent_repo": "parent-repo", + }, + want: "parent_owner and parent_repo must be provided together", + }, + { + name: "issue fields", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(1), + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "field_option_name": "High"}, + }, + }, + want: "issue_fields cannot be used with parent_issue_number", + }, + { + name: "issue_write rejects parent during update", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(2), + "parent_issue_number": float64(1), + }, + want: "parent_issue_number can only be used with the create method", + }, + { + name: "issue_write rejects zero parent number", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := IssueWrite(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(0), + }, + want: "parent_issue_number must be greater than 0", + }, + { + name: "create_issue rejects zero parent number", + handler: func(ctx context.Context, request *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + return serverTool.Handler(BaseDeps{})(ctx, request) + }, + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Child", + "parent_issue_number": float64(0), + }, + want: "parent_issue_number must be greater than 0", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + request := createMCPRequest(test.args) + result, err := test.handler(ContextWithDeps(context.Background(), BaseDeps{}), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, test.want) + }) + } +} + +func createIssueParentMatcher(parentIssueNumber int, parentOwner, parentRepo string, repositoryID, parentIssueID githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + createIssueParentMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "parentOwner": githubv4.String(parentOwner), + "parentRepo": githubv4.String(parentRepo), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "childRepository": map[string]any{ + "id": repositoryID, + "nameWithOwner": "owner/repo", + }, + "parentRepository": map[string]any{ + "issue": map[string]any{ + "id": parentIssueID, + "number": parentIssueNumber, + }, + }, + }), + ) +} + +func createIssueLabelMatcher(name string, id githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Label struct { + ID githubv4.ID + Name githubv4.String + } `graphql:"label(name: $name)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "name": githubv4.String(name), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "label": map[string]any{ + "id": id, + "name": name, + }, + }, + }), + ) +} + +func createIssueMissingChildRepositoryMatcher(parentIssueNumber int) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + createIssueParentMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "parentOwner": githubv4.String("owner"), + "parentRepo": githubv4.String("repo"), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "childRepository": nil, + "parentRepository": map[string]any{ + "issue": map[string]any{ + "id": "ISSUE_parent", + "number": parentIssueNumber, + }, + }, + }), + ) +} + +func createIssueMissingParentMatcher(parentIssueNumber int) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + createIssueParentMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "parentOwner": githubv4.String("owner"), + "parentRepo": githubv4.String("repo"), + "parentIssueNumber": githubv4.Int(parentIssueNumber), // #nosec G115 - test issue numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "childRepository": map[string]any{ + "id": "REPO_1", + "nameWithOwner": "owner/repo", + }, + "parentRepository": map[string]any{"issue": nil}, + }), + ) +} + +func createIssueUserMatcher(login string, id githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + User struct { + ID githubv4.ID + Login githubv4.String + } `graphql:"user(login: $login)"` + }{}, + map[string]any{"login": githubv4.String(login)}, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "id": id, + "login": login, + }, + }), + ) +} + +func createIssueMissingUserMatcher(login string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + User struct { + ID githubv4.ID + Login githubv4.String + } `graphql:"user(login: $login)"` + }{}, + map[string]any{"login": githubv4.String(login)}, + githubv4mock.DataResponse(map[string]any{"user": nil}), + ) +} + +func createIssueMilestoneMatcher(number int, id githubv4.ID) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Milestone struct { + ID githubv4.ID + Number githubv4.Int + } `graphql:"milestone(number: $milestoneNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "milestoneNumber": githubv4.Int(number), // #nosec G115 - test milestone numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "milestone": map[string]any{ + "id": id, + "number": number, + }, + }, + }), + ) +} + +func createIssueMissingMilestoneMatcher(number int) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Milestone struct { + ID githubv4.ID + Number githubv4.Int + } `graphql:"milestone(number: $milestoneNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "milestoneNumber": githubv4.Int(number), // #nosec G115 - test milestone numbers are small + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{"milestone": nil}, + }), + ) +} + func Test_issueWriteHasNonFormParams(t *testing.T) { t.Parallel()