From bdc7180bed904f184cd8346026c001298cb481e7 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Wed, 17 Jun 2026 16:31:18 -0400 Subject: [PATCH 1/3] Add issue dependency read/write MCP tools Add two feature-flagged tools for issue blocked-by / blocking relationships, gated behind the issue_dependencies flag so they stay off the default tool surface (auto-enabled for insiders only): - issue_dependency_read: get_blocked_by / get_blocking via the Issue.blockedBy / Issue.blocking GraphQL connections, cursor-paginated. - issue_dependency_write: add / remove x blocked_by / blocking via the addBlockedBy / removeBlockedBy mutations. Accepts issue numbers and resolves them to node IDs in a single aliased query; "blocking" is the inverse of "blocked_by" with the subject/related roles swapped. Closes the MCP gap behind the gh CLI dependency verbs (cli/cli#13057). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 34 +- docs/insiders-features.md | 34 +- ...dependency_read_ff_issue_dependencies.snap | 49 ++ ...ependency_write_ff_issue_dependencies.snap | 60 +++ pkg/github/feature_flags.go | 9 + pkg/github/issue_dependencies.go | 444 ++++++++++++++++++ pkg/github/issue_dependencies_test.go | 404 ++++++++++++++++ pkg/github/tools.go | 2 + 8 files changed, 1034 insertions(+), 2 deletions(-) create mode 100644 pkg/github/__toolsnaps__/issue_dependency_read_ff_issue_dependencies.snap create mode 100644 pkg/github/__toolsnaps__/issue_dependency_write_ff_issue_dependencies.snap create mode 100644 pkg/github/issue_dependencies.go create mode 100644 pkg/github/issue_dependencies_test.go diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 590cb6597..6a93e65db 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -75,7 +75,7 @@ runtime behavior (such as output formatting) won't appear here. - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) - **ui_get** - Get UI data - - **Required OAuth Scopes**: `repo`, `read:org` + - **Required OAuth Scopes (any of)**: `repo`, `read:org` - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) @@ -324,4 +324,36 @@ runtime behavior (such as output formatting) won't appear here. - `repo`: Repository name (string, required) - `start_line`: Optional 1-based starting line of the window of interest. Only ranges overlapping [start_line, end_line] are returned, clamped to the window. (number, optional) +### `issue_dependencies` + +- **issue_dependency_read** - Read issue dependencies + - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) + - `issue_number`: The number of the issue (number, required) + - `method`: The read operation to perform on a single issue's dependencies. + Options are: + 1. get_blocked_by - List the issues that block this issue (this issue is blocked by them). + 2. get_blocking - List the issues that this issue blocks. + (string, required) + - `owner`: The owner of the repository (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `repo`: The name of the repository (string, required) + +- **issue_dependency_write** - Change issue dependency + - **Required OAuth Scopes**: `repo` + - `issue_number`: The number of the subject issue (number, required) + - `method`: The action to perform. + Options are: + - 'add' - create the dependency relationship. + - 'remove' - delete the dependency relationship. (string, required) + - `owner`: The owner of the subject issue's repository (string, required) + - `related_issue_number`: The number of the related issue to link or unlink (number, required) + - `related_owner`: The owner of the related issue's repository. Defaults to 'owner' when omitted. (string, optional) + - `related_repo`: The name of the related issue's repository. Defaults to 'repo' when omitted. (string, optional) + - `repo`: The name of the subject issue's repository (string, required) + - `type`: The relationship direction relative to the subject issue. + Options are: + - 'blocked_by' - the subject issue is blocked by the related issue. + - 'blocking' - the subject issue blocks the related issue. (string, required) + diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 3306b5cd8..5ac600ed6 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -69,7 +69,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) - **ui_get** - Get UI data - - **Required OAuth Scopes**: `repo`, `read:org` + - **Required OAuth Scopes (any of)**: `repo`, `read:org` - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) @@ -144,6 +144,38 @@ The list below is generated from the Go source. It covers tool **inventory and s - `repo`: Repository name (string, required) - `start_line`: Optional 1-based starting line of the window of interest. Only ranges overlapping [start_line, end_line] are returned, clamped to the window. (number, optional) +### `issue_dependencies` + +- **issue_dependency_read** - Read issue dependencies + - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) + - `issue_number`: The number of the issue (number, required) + - `method`: The read operation to perform on a single issue's dependencies. + Options are: + 1. get_blocked_by - List the issues that block this issue (this issue is blocked by them). + 2. get_blocking - List the issues that this issue blocks. + (string, required) + - `owner`: The owner of the repository (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `repo`: The name of the repository (string, required) + +- **issue_dependency_write** - Change issue dependency + - **Required OAuth Scopes**: `repo` + - `issue_number`: The number of the subject issue (number, required) + - `method`: The action to perform. + Options are: + - 'add' - create the dependency relationship. + - 'remove' - delete the dependency relationship. (string, required) + - `owner`: The owner of the subject issue's repository (string, required) + - `related_issue_number`: The number of the related issue to link or unlink (number, required) + - `related_owner`: The owner of the related issue's repository. Defaults to 'owner' when omitted. (string, optional) + - `related_repo`: The name of the related issue's repository. Defaults to 'repo' when omitted. (string, optional) + - `repo`: The name of the subject issue's repository (string, required) + - `type`: The relationship direction relative to the subject issue. + Options are: + - 'blocked_by' - the subject issue is blocked by the related issue. + - 'blocking' - the subject issue blocks the related issue. (string, required) + --- diff --git a/pkg/github/__toolsnaps__/issue_dependency_read_ff_issue_dependencies.snap b/pkg/github/__toolsnaps__/issue_dependency_read_ff_issue_dependencies.snap new file mode 100644 index 000000000..4a09d7871 --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_dependency_read_ff_issue_dependencies.snap @@ -0,0 +1,49 @@ +{ + "annotations": { + "readOnlyHint": true, + "title": "Read issue dependencies" + }, + "description": "Read an issue's dependency relationships in a GitHub repository: the issues that block it (blocked_by) or the issues it blocks (blocking).", + "inputSchema": { + "properties": { + "after": { + "description": "Cursor for pagination. Use the cursor from the previous response.", + "type": "string" + }, + "issue_number": { + "description": "The number of the issue", + "type": "number" + }, + "method": { + "description": "The read operation to perform on a single issue's dependencies.\nOptions are:\n1. get_blocked_by - List the issues that block this issue (this issue is blocked by them).\n2. get_blocking - List the issues that this issue blocks.\n", + "enum": [ + "get_blocked_by", + "get_blocking" + ], + "type": "string" + }, + "owner": { + "description": "The owner of the repository", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "The name of the repository", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "issue_number" + ], + "type": "object" + }, + "name": "issue_dependency_read" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/issue_dependency_write_ff_issue_dependencies.snap b/pkg/github/__toolsnaps__/issue_dependency_write_ff_issue_dependencies.snap new file mode 100644 index 000000000..0d1723b2f --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_dependency_write_ff_issue_dependencies.snap @@ -0,0 +1,60 @@ +{ + "annotations": { + "title": "Change issue dependency" + }, + "description": "Add or remove an issue dependency relationship in a GitHub repository. Use type 'blocked_by' to record that the subject issue is blocked by a related issue, or type 'blocking' to record that the subject issue blocks a related issue. The related issue defaults to the same repository as the subject unless related_owner/related_repo are provided.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The number of the subject issue", + "type": "number" + }, + "method": { + "description": "The action to perform.\nOptions are:\n- 'add' - create the dependency relationship.\n- 'remove' - delete the dependency relationship.", + "enum": [ + "add", + "remove" + ], + "type": "string" + }, + "owner": { + "description": "The owner of the subject issue's repository", + "type": "string" + }, + "related_issue_number": { + "description": "The number of the related issue to link or unlink", + "type": "number" + }, + "related_owner": { + "description": "The owner of the related issue's repository. Defaults to 'owner' when omitted.", + "type": "string" + }, + "related_repo": { + "description": "The name of the related issue's repository. Defaults to 'repo' when omitted.", + "type": "string" + }, + "repo": { + "description": "The name of the subject issue's repository", + "type": "string" + }, + "type": { + "description": "The relationship direction relative to the subject issue.\nOptions are:\n- 'blocked_by' - the subject issue is blocked by the related issue.\n- 'blocking' - the subject issue blocks the related issue.", + "enum": [ + "blocked_by", + "blocking" + ], + "type": "string" + } + }, + "required": [ + "method", + "type", + "owner", + "repo", + "issue_number", + "related_issue_number" + ], + "type": "object" + }, + "name": "issue_dependency_write" +} \ No newline at end of file diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 835179532..1c1d08135 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -21,6 +21,13 @@ const FeatureFlagIssueFields = "remote_mcp_issue_fields" // is not advertised by default, keeping the tool surface small unless opted in. const FeatureFlagFileBlame = "file_blame" +// FeatureFlagIssueDependencies is the feature flag name for the issue dependency +// tools (issue_dependency_read / issue_dependency_write), which read and edit an +// issue's blocked-by / blocking relationships. It is gated so these tools are not +// advertised in the default surface, keeping the fixed tool-schema cost small +// unless explicitly opted in. +const FeatureFlagIssueDependencies = "issue_dependencies" + // AllowedFeatureFlags is the allowlist of feature flags that can be enabled // by users via --features CLI flag or X-MCP-Features HTTP header. // Only flags in this list are accepted; unknown flags are silently ignored. @@ -33,6 +40,7 @@ var AllowedFeatureFlags = []string{ FeatureFlagIssuesGranular, FeatureFlagPullRequestsGranular, FeatureFlagFileBlame, + FeatureFlagIssueDependencies, } // InsidersFeatureFlags is the list of feature flags that insiders mode enables. @@ -44,6 +52,7 @@ var InsidersFeatureFlags = []string{ FeatureFlagCSVOutput, FeatureFlagIssueFields, FeatureFlagFileBlame, + FeatureFlagIssueDependencies, } // FeatureFlags defines runtime feature toggles that adjust tool behavior. diff --git a/pkg/github/issue_dependencies.go b/pkg/github/issue_dependencies.go new file mode 100644 index 000000000..0837d65d2 --- /dev/null +++ b/pkg/github/issue_dependencies.go @@ -0,0 +1,444 @@ +package github + +import ( + "context" + "fmt" + "strings" + + ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +// AddBlockedByInput represents the input for the addBlockedBy GraphQL mutation. +// The pinned githubv4 library predates the dependency mutations, so the input +// type is declared here. The Go type name must match the GraphQL input type name. +type AddBlockedByInput struct { + IssueID githubv4.ID `json:"issueId"` + BlockingIssueID githubv4.ID `json:"blockingIssueId"` + ClientMutationID *githubv4.String `json:"clientMutationId,omitempty"` +} + +// RemoveBlockedByInput represents the input for the removeBlockedBy GraphQL mutation. +type RemoveBlockedByInput struct { + IssueID githubv4.ID `json:"issueId"` + BlockingIssueID githubv4.ID `json:"blockingIssueId"` + ClientMutationID *githubv4.String `json:"clientMutationId,omitempty"` +} + +// dependencyIssueNode is the minimal projection returned for each related issue +// in a blocked-by / blocking listing. +type dependencyIssueNode struct { + Number githubv4.Int + Title githubv4.String + State githubv4.String + URL githubv4.String + Repository struct { + NameWithOwner githubv4.String + } +} + +// dependencyConnection mirrors the shape of an IssueConnection returned by the +// blockedBy / blocking fields. +type dependencyConnection struct { + TotalCount githubv4.Int + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + Nodes []dependencyIssueNode +} + +// minimalDependencyIssue is the JSON-serialised form of a related issue. +type minimalDependencyIssue struct { + Number int `json:"number"` + Title string `json:"title"` + State string `json:"state"` + URL string `json:"url"` + Repository string `json:"repository"` +} + +// IssueDependencyRead creates a tool to read an issue's blocked-by and blocking +// relationships. It is a separate, feature-flagged tool (rather than a method on +// the default issue_read) so the whole dependency capability can be gated as a +// unit without enlarging the default issue tool surface. +func IssueDependencyRead(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: `The read operation to perform on a single issue's dependencies. +Options are: +1. get_blocked_by - List the issues that block this issue (this issue is blocked by them). +2. get_blocking - List the issues that this issue blocks. +`, + Enum: []any{"get_blocked_by", "get_blocking"}, + }, + "owner": { + Type: "string", + Description: "The owner of the repository", + }, + "repo": { + Type: "string", + Description: "The name of the repository", + }, + "issue_number": { + Type: "number", + Description: "The number of the issue", + }, + }, + Required: []string{"method", "owner", "repo", "issue_number"}, + } + WithCursorPagination(schema) + + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "issue_dependency_read", + Description: t("TOOL_ISSUE_DEPENDENCY_READ_DESCRIPTION", "Read an issue's dependency relationships in a GitHub repository: the issues that block it (blocked_by) or the issues it blocks (blocking)."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ISSUE_DEPENDENCY_READ_USER_TITLE", "Read issue dependencies"), + ReadOnlyHint: true, + }, + InputSchema: schema, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + method, err := RequiredParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + if _, pageProvided := args["page"]; pageProvided { + return utils.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil, nil + } + pagination, err := OptionalCursorPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + gqlPagination, err := pagination.ToGraphQLParams() + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + switch method { + case "get_blocked_by": + result, err := GetIssueBlockedBy(ctx, gqlClient, owner, repo, issueNumber, gqlPagination) + return result, nil, err + case "get_blocking": + result, err := GetIssueBlocking(ctx, gqlClient, owner, repo, issueNumber, gqlPagination) + return result, nil, err + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil + } + }) + st.FeatureFlagEnable = FeatureFlagIssueDependencies + return st +} + +func dependencyQueryVars(owner, repo string, issueNumber int, pagination *GraphQLPaginationParams) map[string]any { + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "issueNumber": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers + "first": githubv4.Int(*pagination.First), + } + if pagination.After != nil { + vars["after"] = githubv4.String(*pagination.After) + } else { + vars["after"] = (*githubv4.String)(nil) + } + return vars +} + +func dependencyResult(conn dependencyConnection) *mcp.CallToolResult { + issues := make([]minimalDependencyIssue, 0, len(conn.Nodes)) + for _, node := range conn.Nodes { + issues = append(issues, minimalDependencyIssue{ + Number: int(node.Number), + Title: string(node.Title), + State: string(node.State), + URL: string(node.URL), + Repository: string(node.Repository.NameWithOwner), + }) + } + return MarshalledTextResult(map[string]any{ + "issues": issues, + "totalCount": int(conn.TotalCount), + "pageInfo": map[string]any{ + "hasNextPage": bool(conn.PageInfo.HasNextPage), + "endCursor": string(conn.PageInfo.EndCursor), + }, + }) +} + +// GetIssueBlockedBy lists the issues that block the given issue. +func GetIssueBlockedBy(ctx context.Context, client *githubv4.Client, owner, repo string, issueNumber int, pagination *GraphQLPaginationParams) (*mcp.CallToolResult, error) { + var query struct { + Repository struct { + Issue struct { + BlockedBy dependencyConnection `graphql:"blockedBy(first: $first, after: $after)"` + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + if err := client.Query(ctx, &query, dependencyQueryVars(owner, repo, issueNumber, pagination)); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get blocked-by issues", err), nil + } + return dependencyResult(query.Repository.Issue.BlockedBy), nil +} + +// GetIssueBlocking lists the issues that the given issue blocks. +func GetIssueBlocking(ctx context.Context, client *githubv4.Client, owner, repo string, issueNumber int, pagination *GraphQLPaginationParams) (*mcp.CallToolResult, error) { + var query struct { + Repository struct { + Issue struct { + Blocking dependencyConnection `graphql:"blocking(first: $first, after: $after)"` + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + if err := client.Query(ctx, &query, dependencyQueryVars(owner, repo, issueNumber, pagination)); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get blocking issues", err), nil + } + return dependencyResult(query.Repository.Issue.Blocking), nil +} + +// IssueDependencyWrite creates a tool to add or remove an issue dependency +// (blocked-by / blocking) relationship. It accepts issue numbers and resolves +// them to GraphQL node IDs before calling the addBlockedBy / removeBlockedBy +// mutations. "blocking" is the inverse of "blocked_by", so both directions are +// served by the same mutation pair with the issue arguments swapped. +func IssueDependencyWrite(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "issue_dependency_write", + Description: t("TOOL_ISSUE_DEPENDENCY_WRITE_DESCRIPTION", + "Add or remove an issue dependency relationship in a GitHub repository. "+ + "Use type 'blocked_by' to record that the subject issue is blocked by a related issue, "+ + "or type 'blocking' to record that the subject issue blocks a related issue. "+ + "The related issue defaults to the same repository as the subject unless related_owner/related_repo are provided."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ISSUE_DEPENDENCY_WRITE_USER_TITLE", "Change issue dependency"), + ReadOnlyHint: false, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: `The action to perform. +Options are: +- 'add' - create the dependency relationship. +- 'remove' - delete the dependency relationship.`, + Enum: []any{"add", "remove"}, + }, + "type": { + Type: "string", + Description: `The relationship direction relative to the subject issue. +Options are: +- 'blocked_by' - the subject issue is blocked by the related issue. +- 'blocking' - the subject issue blocks the related issue.`, + Enum: []any{"blocked_by", "blocking"}, + }, + "owner": { + Type: "string", + Description: "The owner of the subject issue's repository", + }, + "repo": { + Type: "string", + Description: "The name of the subject issue's repository", + }, + "issue_number": { + Type: "number", + Description: "The number of the subject issue", + }, + "related_issue_number": { + Type: "number", + Description: "The number of the related issue to link or unlink", + }, + "related_owner": { + Type: "string", + Description: "The owner of the related issue's repository. Defaults to 'owner' when omitted.", + }, + "related_repo": { + Type: "string", + Description: "The name of the related issue's repository. Defaults to 'repo' when omitted.", + }, + }, + Required: []string{"method", "type", "owner", "repo", "issue_number", "related_issue_number"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + method, err := RequiredParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relationshipType, err := RequiredParam[string](args, "type") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relatedIssueNumber, err := RequiredInt(args, "related_issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relatedOwner, err := OptionalParam[string](args, "related_owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + relatedRepo, err := OptionalParam[string](args, "related_repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if relatedOwner == "" { + relatedOwner = owner + } + if relatedRepo == "" { + relatedRepo = repo + } + + method = strings.ToLower(method) + relationshipType = strings.ToLower(relationshipType) + if method != "add" && method != "remove" { + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil + } + if relationshipType != "blocked_by" && relationshipType != "blocking" { + return utils.NewToolResultError(fmt.Sprintf("unknown type: %s", relationshipType)), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + subjectID, relatedID, err := resolveIssueNodeIDs(ctx, gqlClient, owner, repo, issueNumber, relatedOwner, relatedRepo, relatedIssueNumber) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to resolve issues", err), nil, nil + } + + // "A blocks B" is recorded as addBlockedBy(issueId: B, blockingIssueId: A). + // For type 'blocked_by' the subject is blocked by the related issue. + // For type 'blocking' the subject blocks the related issue, so the roles swap. + blockedID, blockingID := subjectID, relatedID + if relationshipType == "blocking" { + blockedID, blockingID = relatedID, subjectID + } + + result, err := writeBlockedByRelationship(ctx, gqlClient, method, blockedID, blockingID) + return result, nil, err + }) + st.FeatureFlagEnable = FeatureFlagIssueDependencies + return st +} + +// resolveIssueNodeIDs resolves the subject and related issue numbers to their +// GraphQL node IDs in a single aliased query. +func resolveIssueNodeIDs(ctx context.Context, client *githubv4.Client, owner, repo string, issueNumber int, relatedOwner, relatedRepo string, relatedIssueNumber int) (githubv4.ID, githubv4.ID, error) { + var query struct { + Subject struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $subjectNumber)"` + } `graphql:"subject: repository(owner: $subjectOwner, name: $subjectRepo)"` + Related struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $relatedNumber)"` + } `graphql:"related: repository(owner: $relatedOwner, name: $relatedRepo)"` + } + vars := map[string]any{ + "subjectOwner": githubv4.String(owner), + "subjectRepo": githubv4.String(repo), + "subjectNumber": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers + "relatedOwner": githubv4.String(relatedOwner), + "relatedRepo": githubv4.String(relatedRepo), + "relatedNumber": githubv4.Int(relatedIssueNumber), // #nosec G115 - issue numbers are always small positive integers + } + if err := client.Query(ctx, &query, vars); err != nil { + return "", "", err + } + return query.Subject.Issue.ID, query.Related.Issue.ID, nil +} + +// writeBlockedByRelationship runs the addBlockedBy / removeBlockedBy mutation and +// returns a minimal description of the affected issues. +func writeBlockedByRelationship(ctx context.Context, client *githubv4.Client, method string, blockedID, blockingID githubv4.ID) (*mcp.CallToolResult, error) { + type mutationIssue struct { + Number githubv4.Int + URL githubv4.String + } + + switch method { + case "add": + var mutation struct { + AddBlockedBy struct { + Issue mutationIssue + BlockingIssue mutationIssue + } `graphql:"addBlockedBy(input: $input)"` + } + input := AddBlockedByInput{IssueID: blockedID, BlockingIssueID: blockingID} + if err := client.Mutate(ctx, &mutation, input, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to add issue dependency", err), nil + } + return MarshalledTextResult(map[string]any{ + "message": "dependency added", + "blocked_issue": map[string]any{"number": int(mutation.AddBlockedBy.Issue.Number), "url": string(mutation.AddBlockedBy.Issue.URL)}, + "blocking_issue": map[string]any{"number": int(mutation.AddBlockedBy.BlockingIssue.Number), "url": string(mutation.AddBlockedBy.BlockingIssue.URL)}, + }), nil + case "remove": + var mutation struct { + RemoveBlockedBy struct { + Issue mutationIssue + BlockingIssue mutationIssue + } `graphql:"removeBlockedBy(input: $input)"` + } + input := RemoveBlockedByInput{IssueID: blockedID, BlockingIssueID: blockingID} + if err := client.Mutate(ctx, &mutation, input, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to remove issue dependency", err), nil + } + return MarshalledTextResult(map[string]any{ + "message": "dependency removed", + "blocked_issue": map[string]any{"number": int(mutation.RemoveBlockedBy.Issue.Number), "url": string(mutation.RemoveBlockedBy.Issue.URL)}, + "blocking_issue": map[string]any{"number": int(mutation.RemoveBlockedBy.BlockingIssue.Number), "url": string(mutation.RemoveBlockedBy.BlockingIssue.URL)}, + }), nil + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil + } +} diff --git a/pkg/github/issue_dependencies_test.go b/pkg/github/issue_dependencies_test.go new file mode 100644 index 000000000..cb3830665 --- /dev/null +++ b/pkg/github/issue_dependencies_test.go @@ -0,0 +1,404 @@ +package github + +import ( + "context" + "encoding/json" + "testing" + + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/internal/toolsnaps" + "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 Test_IssueDependencyRead(t *testing.T) { + // Verify tool definition once (flag-gated variant snap) + serverTool := IssueDependencyRead(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name+"_ff_"+FeatureFlagIssueDependencies, tool)) + require.Equal(t, FeatureFlagIssueDependencies, serverTool.FeatureFlagEnable) + + assert.Equal(t, "issue_dependency_read", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.True(t, tool.Annotations.ReadOnlyHint) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "method") + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "issue_number") + assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "issue_number"}) + + blockedByQuery := githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Issue struct { + BlockedBy dependencyConnection `graphql:"blockedBy(first: $first, after: $after)"` + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(123), + "first": githubv4.Int(30), + "after": (*githubv4.String)(nil), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "blockedBy": map[string]any{ + "totalCount": 1, + "pageInfo": map[string]any{ + "hasNextPage": false, + "endCursor": "", + }, + "nodes": []map[string]any{ + { + "number": 7, + "title": "Blocker", + "state": "OPEN", + "url": "https://github.com/owner/repo/issues/7", + "repository": map[string]any{"nameWithOwner": "owner/repo"}, + }, + }, + }, + }, + }, + }), + ) + + blockingQuery := githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Issue struct { + Blocking dependencyConnection `graphql:"blocking(first: $first, after: $after)"` + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(123), + "first": githubv4.Int(30), + "after": (*githubv4.String)(nil), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "blocking": map[string]any{ + "totalCount": 2, + "pageInfo": map[string]any{ + "hasNextPage": true, + "endCursor": "Y3Vyc29y", + }, + "nodes": []map[string]any{ + { + "number": 8, + "title": "Blocked A", + "state": "OPEN", + "url": "https://github.com/owner/repo/issues/8", + "repository": map[string]any{"nameWithOwner": "owner/repo"}, + }, + { + "number": 9, + "title": "Blocked B", + "state": "CLOSED", + "url": "https://github.com/owner/repo/issues/9", + "repository": map[string]any{"nameWithOwner": "owner/repo"}, + }, + }, + }, + }, + }, + }), + ) + + tests := []struct { + name string + method string + matcher githubv4mock.Matcher + expectError bool + expectedCount int + expectedFirst int + expectedNext bool + }{ + { + name: "get_blocked_by returns blockers", + method: "get_blocked_by", + matcher: blockedByQuery, + expectedCount: 1, + expectedFirst: 7, + expectedNext: false, + }, + { + name: "get_blocking returns blocked issues", + method: "get_blocking", + matcher: blockingQuery, + expectedCount: 2, + expectedFirst: 8, + expectedNext: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(tc.matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": tc.method, + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, "expected result to not be an error") + + text := getTextResult(t, result) + var payload struct { + Issues []minimalDependencyIssue `json:"issues"` + Total int `json:"totalCount"` + Page struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + } + require.NoError(t, json.Unmarshal([]byte(text.Text), &payload)) + require.Len(t, payload.Issues, tc.expectedCount) + assert.Equal(t, tc.expectedFirst, payload.Issues[0].Number) + assert.Equal(t, tc.expectedNext, payload.Page.HasNextPage) + }) + } +} + +func Test_IssueDependencyRead_Errors(t *testing.T) { + serverTool := IssueDependencyRead(translations.NullTranslationHelper) + + t.Run("rejects page-based pagination", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "page": float64(2), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + errText := getErrorResult(t, result) + assert.Contains(t, errText.Text, "cursor-based pagination") + }) + + t.Run("missing required param", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_blocked_by", + "owner": "owner", + "repo": "repo", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + getErrorResult(t, result) + }) +} + +func Test_IssueDependencyWrite(t *testing.T) { + // Verify tool definition once (flag-gated variant snap) + serverTool := IssueDependencyWrite(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name+"_ff_"+FeatureFlagIssueDependencies, tool)) + require.Equal(t, FeatureFlagIssueDependencies, serverTool.FeatureFlagEnable) + + assert.Equal(t, "issue_dependency_write", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.False(t, tool.Annotations.ReadOnlyHint) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "method") + assert.Contains(t, schema.Properties, "type") + assert.Contains(t, schema.Properties, "issue_number") + assert.Contains(t, schema.Properties, "related_issue_number") + assert.ElementsMatch(t, schema.Required, []string{"method", "type", "owner", "repo", "issue_number", "related_issue_number"}) + + resolveMatcher := func(subjectID, relatedID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Subject struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $subjectNumber)"` + } `graphql:"subject: repository(owner: $subjectOwner, name: $subjectRepo)"` + Related struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $relatedNumber)"` + } `graphql:"related: repository(owner: $relatedOwner, name: $relatedRepo)"` + }{}, + map[string]any{ + "subjectOwner": githubv4.String("owner"), + "subjectRepo": githubv4.String("repo"), + "subjectNumber": githubv4.Int(1), + "relatedOwner": githubv4.String("owner"), + "relatedRepo": githubv4.String("repo"), + "relatedNumber": githubv4.Int(2), + }, + githubv4mock.DataResponse(map[string]any{ + "subject": map[string]any{"issue": map[string]any{"id": subjectID}}, + "related": map[string]any{"issue": map[string]any{"id": relatedID}}, + }), + ) + } + + type mutationIssue struct { + Number githubv4.Int + URL githubv4.String + } + + addMutation := func(issueID, blockingID string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + AddBlockedBy struct { + Issue mutationIssue + BlockingIssue mutationIssue + } `graphql:"addBlockedBy(input: $input)"` + }{}, + AddBlockedByInput{IssueID: githubv4.ID(issueID), BlockingIssueID: githubv4.ID(blockingID)}, + nil, + githubv4mock.DataResponse(map[string]any{ + "addBlockedBy": map[string]any{ + "issue": map[string]any{"number": 1, "url": "https://github.com/owner/repo/issues/1"}, + "blockingIssue": map[string]any{"number": 2, "url": "https://github.com/owner/repo/issues/2"}, + }, + }), + ) + } + + removeMutation := func(issueID, blockingID string) githubv4mock.Matcher { + return githubv4mock.NewMutationMatcher( + struct { + RemoveBlockedBy struct { + Issue mutationIssue + BlockingIssue mutationIssue + } `graphql:"removeBlockedBy(input: $input)"` + }{}, + RemoveBlockedByInput{IssueID: githubv4.ID(issueID), BlockingIssueID: githubv4.ID(blockingID)}, + nil, + githubv4mock.DataResponse(map[string]any{ + "removeBlockedBy": map[string]any{ + "issue": map[string]any{"number": 1, "url": "https://github.com/owner/repo/issues/1"}, + "blockingIssue": map[string]any{"number": 2, "url": "https://github.com/owner/repo/issues/2"}, + }, + }), + ) + } + + tests := []struct { + name string + method string + relationship string + matchers []githubv4mock.Matcher + expectedMessage string + }{ + { + name: "add blocked_by uses subject as blocked", + method: "add", + relationship: "blocked_by", + // subject(1) is blocked by related(2): issueId=subject, blockingIssueId=related + matchers: []githubv4mock.Matcher{resolveMatcher("I_subject", "I_related"), addMutation("I_subject", "I_related")}, + expectedMessage: "dependency added", + }, + { + name: "add blocking swaps roles", + method: "add", + relationship: "blocking", + // subject(1) blocks related(2): issueId=related, blockingIssueId=subject + matchers: []githubv4mock.Matcher{resolveMatcher("I_subject", "I_related"), addMutation("I_related", "I_subject")}, + expectedMessage: "dependency added", + }, + { + name: "remove blocked_by", + method: "remove", + relationship: "blocked_by", + matchers: []githubv4mock.Matcher{resolveMatcher("I_subject", "I_related"), removeMutation("I_subject", "I_related")}, + expectedMessage: "dependency removed", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(tc.matchers...)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": tc.method, + "type": tc.relationship, + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "related_issue_number": float64(2), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, "expected result to not be an error") + + text := getTextResult(t, result) + var payload struct { + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(text.Text), &payload)) + assert.Equal(t, tc.expectedMessage, payload.Message) + }) + } +} + +func Test_IssueDependencyWrite_Validation(t *testing.T) { + serverTool := IssueDependencyWrite(translations.NullTranslationHelper) + + cases := []struct { + name string + args map[string]any + }{ + { + name: "unknown type", + args: map[string]any{ + "method": "add", + "type": "related_to", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "related_issue_number": float64(2), + }, + }, + { + name: "missing related_issue_number", + args: map[string]any{ + "method": "add", + "type": "blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + getErrorResult(t, result) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 2c894e573..4fbc5846e 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -218,6 +218,8 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { LegacyIssueWrite(t), AddIssueComment(t), SubIssueWrite(t), + IssueDependencyRead(t), + IssueDependencyWrite(t), // User tools SearchUsers(t), From d9b1b703aa0f1946cd3c58fb078318452609d27c Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Wed, 17 Jun 2026 16:53:55 -0400 Subject: [PATCH 2/3] Add get_parent method to issue_read Add an upward parent read to issue_read, the counterpart to the existing downward get_sub_issues. Uses the GraphQL Issue.parent field and returns a null parent when the issue is not a sub-issue. Kept always-on (not feature gated) to mirror get_sub_issues; the dependency tools remain flag-gated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 5 +- pkg/github/__toolsnaps__/issue_read.snap | 3 +- pkg/github/issues.go | 56 ++++++++++- pkg/github/issues_test.go | 122 +++++++++++++++++++++++ 4 files changed, 180 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 404135aed..62be1f9b1 100644 --- a/README.md +++ b/README.md @@ -859,8 +859,9 @@ The following sets of tools are available: Options are: 1. get - Get details of a specific issue. 2. get_comments - Get issue comments. - 3. get_sub_issues - Get sub-issues of the issue. - 4. get_labels - Get labels assigned to the issue. + 3. get_sub_issues - Get sub-issues (children) of the issue. + 4. get_parent - Get the parent issue, if this issue is a sub-issue of another. + 5. get_labels - Get labels assigned to the issue. (string, required) - `owner`: The owner of the repository (string, required) - `page`: Page number for pagination (min 1) (number, optional) diff --git a/pkg/github/__toolsnaps__/issue_read.snap b/pkg/github/__toolsnaps__/issue_read.snap index 21aa361f5..9b882c79b 100644 --- a/pkg/github/__toolsnaps__/issue_read.snap +++ b/pkg/github/__toolsnaps__/issue_read.snap @@ -11,11 +11,12 @@ "type": "number" }, "method": { - "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get details of a specific issue.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues of the issue.\n4. get_labels - Get labels assigned to the issue.\n", + "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get details of a specific issue.\n2. get_comments - Get issue comments.\n3. get_sub_issues - Get sub-issues (children) of the issue.\n4. get_parent - Get the parent issue, if this issue is a sub-issue of another.\n5. get_labels - Get labels assigned to the issue.\n", "enum": [ "get", "get_comments", "get_sub_issues", + "get_parent", "get_labels" ], "type": "string" diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f98982f1e..395ca15db 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -733,10 +733,11 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { Options are: 1. get - Get details of a specific issue. 2. get_comments - Get issue comments. -3. get_sub_issues - Get sub-issues of the issue. -4. get_labels - Get labels assigned to the issue. +3. get_sub_issues - Get sub-issues (children) of the issue. +4. get_parent - Get the parent issue, if this issue is a sub-issue of another. +5. get_labels - Get labels assigned to the issue. `, - Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels"}, + Enum: []any{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"}, }, "owner": { Type: "string", @@ -816,6 +817,9 @@ Options are: case "get_sub_issues": result, err := GetSubIssues(ctx, client, deps, owner, repo, issueNumber, pagination) return attachIFC(result), nil, err + case "get_parent": + result, err := GetIssueParent(ctx, gqlClient, owner, repo, issueNumber) + return attachIFC(result), nil, err case "get_labels": result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber) return attachIFC(result), nil, err @@ -1015,6 +1019,52 @@ func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependenc return utils.NewToolResultText(string(r)), nil } +// GetIssueParent returns the parent issue of the given issue, or a null parent +// when the issue is not a sub-issue of any other issue. It reads the GraphQL +// Issue.parent field, the upward counterpart to the downward get_sub_issues read. +func GetIssueParent(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { + var query struct { + Repository struct { + Issue struct { + Parent *struct { + Number githubv4.Int + Title githubv4.String + State githubv4.String + URL githubv4.String + Repository struct { + NameWithOwner githubv4.String + } + } + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "issueNumber": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers + } + + if err := client.Query(ctx, &query, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get issue parent", err), nil + } + + parent := query.Repository.Issue.Parent + if parent == nil { + return MarshalledTextResult(map[string]any{"parent": nil}), nil + } + + return MarshalledTextResult(map[string]any{ + "parent": map[string]any{ + "number": int(parent.Number), + "title": string(parent.Title), + "state": string(parent.State), + "url": string(parent.URL), + "repository": string(parent.Repository.NameWithOwner), + }, + }), nil +} + func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { // Get current labels on the issue using GraphQL var query struct { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 5775daf37..eb7f426fd 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -3822,6 +3822,128 @@ func Test_GetIssueLabels(t *testing.T) { } } +func Test_GetIssueParent(t *testing.T) { + t.Parallel() + + serverTool := IssueRead(translations.NullTranslationHelper) + + parentMatcherStruct := struct { + Repository struct { + Issue struct { + Parent *struct { + Number githubv4.Int + Title githubv4.String + State githubv4.String + URL githubv4.String + Repository struct { + NameWithOwner githubv4.String + } + } + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{} + + vars := map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(123), + } + + tests := []struct { + name string + mockedClient *http.Client + expectToolError bool + expectedText string + }{ + { + name: "issue has a parent", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + parentMatcherStruct, + vars, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "parent": map[string]any{ + "number": githubv4.Int(42), + "title": githubv4.String("Parent issue"), + "state": githubv4.String("OPEN"), + "url": githubv4.String("https://github.com/owner/repo/issues/42"), + "repository": map[string]any{ + "nameWithOwner": githubv4.String("owner/repo"), + }, + }, + }, + }, + }), + ), + ), + expectedText: `"number":42`, + }, + { + name: "issue has no parent", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + parentMatcherStruct, + vars, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "parent": nil, + }, + }, + }), + ), + ), + expectedText: `"parent":null`, + }, + { + name: "graphql error", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + parentMatcherStruct, + vars, + githubv4mock.ErrorResponse("issue not found"), + ), + ), + expectToolError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(tc.mockedClient) + client := mustNewGHClient(t, nil) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}), + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "get_parent", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.NotNil(t, result) + + if tc.expectToolError { + assert.True(t, result.IsError) + return + } + assert.False(t, result.IsError) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedText) + }) + } +} + func Test_AddSubIssue(t *testing.T) { // Verify tool definition once serverTool := SubIssueWrite(translations.NullTranslationHelper) From 409844ebf26b5abfa42e6645f68e46684f2e1fdd Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Thu, 18 Jun 2026 10:47:33 -0400 Subject: [PATCH 3/3] Fail fast on self-dependency in issue_dependency_write Reject a write where the subject and related issue are identical before resolving node IDs or issuing a mutation, avoiding two API round-trips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/github/issue_dependencies.go | 4 ++++ pkg/github/issue_dependencies_test.go | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pkg/github/issue_dependencies.go b/pkg/github/issue_dependencies.go index 0837d65d2..6fbcc9eed 100644 --- a/pkg/github/issue_dependencies.go +++ b/pkg/github/issue_dependencies.go @@ -343,6 +343,10 @@ Options are: return utils.NewToolResultError(fmt.Sprintf("unknown type: %s", relationshipType)), nil, nil } + if owner == relatedOwner && repo == relatedRepo && issueNumber == relatedIssueNumber { + return utils.NewToolResultError("an issue cannot block or depend on itself"), nil, nil + } + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil diff --git a/pkg/github/issue_dependencies_test.go b/pkg/github/issue_dependencies_test.go index cb3830665..47859e937 100644 --- a/pkg/github/issue_dependencies_test.go +++ b/pkg/github/issue_dependencies_test.go @@ -358,6 +358,28 @@ func Test_IssueDependencyWrite(t *testing.T) { assert.Equal(t, tc.expectedMessage, payload.Message) }) } + + t.Run("self dependency fails before any API call", func(t *testing.T) { + // Register no matchers: the handler must return before resolving node IDs or mutating. + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "add", + "type": "blocked_by", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "related_issue_number": float64(1), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError, "expected result to be an error") + + text := getTextResult(t, result) + assert.Contains(t, text.Text, "itself") + }) } func Test_IssueDependencyWrite_Validation(t *testing.T) {