From e01f491018c44b9c4c721724e72cc8e72e402a24 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Wed, 24 Jun 2026 14:20:00 -0400 Subject: [PATCH] Enrich issue_read get with hierarchy relationship signals The default issue_read `get` payload surfaced no hierarchy data, forcing agents to drop to raw REST (parent_issue_url) or scan sibling sub_issues to discover relationships. Enrich `get` with a layered, zero-extra-round-trip relationship signal derived from a single combined GraphQL query: - has_parent / has_children: cheap, always-emitted routing booleans (addresses Sam Morrow's #2726 review note). - parent: compact ref (number/title/state/url/repository) mirroring the existing get_parent payload keys; omitted when there is no parent. - sub_issues_summary: native subIssuesSummary counts (total/completed/ percent_completed); omitted when there are no sub-issues. The single-issue field-values GraphQL call in GetIssue is replaced by one combined query (fetchIssueReadEnrichment) returning field values + parent + subIssuesSummary, so `get` adds no round-trips. Enrichment is best-effort: a query failure still returns the base issue and never fails `get`. Parent titles are sanitized (parent may be cross-repo) and redacted under lockdown mode unless the parent content can be verified as safe; numeric/structural fields and counts stay intact. get_parent / sub_issue_write behavior is unchanged; tool descriptions clarify hierarchy is read here but written via sub_issue_write (no writable parent field). Refs github/planning-tracking#3306 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 3 +- pkg/github/__toolsnaps__/issue_read.snap | 2 +- pkg/github/__toolsnaps__/sub_issue_write.snap | 2 +- pkg/github/issues.go | 152 +++++++++- pkg/github/issues_test.go | 265 +++++++++++++++++- pkg/github/minimal_types.go | 25 ++ 6 files changed, 437 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 614404c0a1..a1173c6179 100644 --- a/README.md +++ b/README.md @@ -857,7 +857,7 @@ The following sets of tools are available: - `issue_number`: The number of the issue (number, required) - `method`: The read operation to perform on a single issue. Options are: - 1. get - Get details of a specific issue. + 1. get - Get details of a specific issue, including hierarchy relationship signals: has_parent and has_children (booleans, always present), plus a compact parent reference (parent) and sub-issue counts (sub_issues_summary) when those relationships exist. Hierarchy is READ here (and via get_parent / get_sub_issues); there is no writable parent field. To change an issue's parent, use sub_issue_write (add with replace_parent). An issue with no parent returns has_parent:false and omits parent. 2. get_comments - Get issue comments. 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. @@ -934,6 +934,7 @@ The following sets of tools are available: - 'add' - add a sub-issue to a parent issue in a GitHub repository. - 'remove' - remove a sub-issue from a parent issue in a GitHub repository. - 'reprioritize' - change the order of sub-issues within a parent issue in a GitHub repository. Use either 'after_id' or 'before_id' to specify the new position. + This tool WRITES hierarchy; the parent relationship is read back via issue_read (get returns has_parent / has_children / parent / sub_issues_summary) or issue_read get_parent. There is no writable parent field: to move an issue under a new parent, call 'add' with replace_parent=true. (string, required) - `owner`: Repository owner (string, required) - `replace_parent`: When true, replaces the sub-issue's current parent issue. Use with 'add' method only. (boolean, optional) diff --git a/pkg/github/__toolsnaps__/issue_read.snap b/pkg/github/__toolsnaps__/issue_read.snap index 9b882c79b6..e629b10f97 100644 --- a/pkg/github/__toolsnaps__/issue_read.snap +++ b/pkg/github/__toolsnaps__/issue_read.snap @@ -11,7 +11,7 @@ "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 (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", + "description": "The read operation to perform on a single issue.\nOptions are:\n1. get - Get details of a specific issue, including hierarchy relationship signals: has_parent and has_children (booleans, always present), plus a compact parent reference (parent) and sub-issue counts (sub_issues_summary) when those relationships exist. Hierarchy is READ here (and via get_parent / get_sub_issues); there is no writable parent field. To change an issue's parent, use sub_issue_write (add with replace_parent). An issue with no parent returns has_parent:false and omits parent.\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", diff --git a/pkg/github/__toolsnaps__/sub_issue_write.snap b/pkg/github/__toolsnaps__/sub_issue_write.snap index 1e4fcceabf..e95e9cf093 100644 --- a/pkg/github/__toolsnaps__/sub_issue_write.snap +++ b/pkg/github/__toolsnaps__/sub_issue_write.snap @@ -18,7 +18,7 @@ "type": "number" }, "method": { - "description": "The action to perform on a single sub-issue\nOptions are:\n- 'add' - add a sub-issue to a parent issue in a GitHub repository.\n- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.\n- 'reprioritize' - change the order of sub-issues within a parent issue in a GitHub repository. Use either 'after_id' or 'before_id' to specify the new position.\n\t\t\t\t", + "description": "The action to perform on a single sub-issue\nOptions are:\n- 'add' - add a sub-issue to a parent issue in a GitHub repository.\n- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.\n- 'reprioritize' - change the order of sub-issues within a parent issue in a GitHub repository. Use either 'after_id' or 'before_id' to specify the new position.\nThis tool WRITES hierarchy; the parent relationship is read back via issue_read (get returns has_parent / has_children / parent / sub_issues_summary) or issue_read get_parent. There is no writable parent field: to move an issue under a new parent, call 'add' with replace_parent=true.\n\t\t\t\t", "type": "string" }, "owner": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 5479f35795..2f3434504e 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -14,6 +14,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/lockdown" "github.com/github/github-mcp-server/pkg/sanitize" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -614,7 +615,7 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { Type: "string", Description: `The read operation to perform on a single issue. Options are: -1. get - Get details of a specific issue. +1. get - Get details of a specific issue, including hierarchy relationship signals: has_parent and has_children (booleans, always present), plus a compact parent reference (parent) and sub-issue counts (sub_issues_summary) when those relationships exist. Hierarchy is READ here (and via get_parent / get_sub_issues); there is no writable parent field. To change an issue's parent, use sub_issue_write (add with replace_parent). An issue with no parent returns has_parent:false and omits parent. 2. get_comments - Get issue comments. 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. @@ -762,13 +763,14 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies, minimalIssue := convertToMinimalIssue(issue) // Always drop the verbose REST IssueFieldValues; enrich with the GraphQL - // field_values view instead. + // field_values view and the hierarchy relationship signals instead. The + // enrichment is best-effort: a failure here must never fail `get`. minimalIssue.IssueFieldValues = nil if issue != nil && issue.NodeID != nil && *issue.NodeID != "" { gqlClient, err := deps.GetGQLClient(ctx) if err == nil { - if fieldValuesByID, err := fetchIssueFieldValuesByNodeID(ctx, gqlClient, []*github.Issue{issue}); err == nil { - minimalIssue.FieldValues = fieldValuesByID[*issue.NodeID] + if enrichment, err := fetchIssueReadEnrichment(ctx, gqlClient, *issue.NodeID); err == nil { + applyIssueReadEnrichment(ctx, &minimalIssue, enrichment, cache, flags.LockdownMode) } } } @@ -776,6 +778,55 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies, return MarshalledTextResult(minimalIssue), nil } +// lockdownRedactedTitle replaces a related issue's title when lockdown mode is enabled and the +// related content cannot be verified as safe. Numeric/structural fields stay intact. +const lockdownRedactedTitle = "[redacted - not from a trusted source]" + +// applyIssueReadEnrichment populates the hierarchy relationship signals (has_parent/has_children, +// parent, sub_issues_summary) and field_values onto the minimal issue. In lockdown mode the parent +// title is redacted unless the parent content can be verified as safe; counts and booleans are pure +// numbers and are always safe to surface. +func applyIssueReadEnrichment(ctx context.Context, minimalIssue *MinimalIssue, enrichment *issueReadEnrichment, cache *lockdown.RepoAccessCache, lockdownMode bool) { + if enrichment == nil { + return + } + + minimalIssue.FieldValues = enrichment.FieldValues + + if parent := enrichment.Parent; parent != nil { + ref := parent.Ref + if lockdownMode && !isSafeParentContent(ctx, cache, parent) { + ref.Title = lockdownRedactedTitle + } + minimalIssue.Parent = &ref + minimalIssue.HasParent = true + } + + if enrichment.SubIssuesSummary.Total > 0 { + summary := enrichment.SubIssuesSummary + minimalIssue.SubIssuesSummary = &summary + minimalIssue.HasChildren = true + } +} + +// isSafeParentContent reports whether the parent issue's title can be exposed under lockdown mode. +// It fails closed: any inability to positively verify safe content (missing cache, missing author, +// unparseable repository, or a lookup error) results in redaction. +func isSafeParentContent(ctx context.Context, cache *lockdown.RepoAccessCache, parent *issueReadParent) bool { + if cache == nil || parent.AuthorLogin == "" { + return false + } + owner, repo, ok := strings.Cut(parent.Ref.Repository, "/") + if !ok || owner == "" || repo == "" { + return false + } + safe, err := cache.IsSafeContent(ctx, parent.AuthorLogin, owner, repo) + if err != nil { + return false + } + return safe +} + func GetIssueComments(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { cache, err := deps.GetRepoAccessCache(ctx) if err != nil { @@ -1211,6 +1262,7 @@ Options are: - 'add' - add a sub-issue to a parent issue in a GitHub repository. - 'remove' - remove a sub-issue from a parent issue in a GitHub repository. - 'reprioritize' - change the order of sub-issues within a parent issue in a GitHub repository. Use either 'after_id' or 'before_id' to specify the new position. +This tool WRITES hierarchy; the parent relationship is read back via issue_read (get returns has_parent / has_children / parent / sub_issues_summary) or issue_read get_parent. There is no writable parent field: to move an issue under a new parent, call 'add' with replace_parent=true. `, }, "owner": { @@ -1649,6 +1701,98 @@ func fetchIssueFieldValuesByNodeID(ctx context.Context, gqlClient *githubv4.Clie return result, nil } +// issueReadEnrichmentQuery fetches, in a single GraphQL round-trip, the custom field values, +// parent reference, and sub-issue summary counts for the issues identified by their node IDs. +// It powers the issue_read `get` relationship signals without adding extra round-trips. +type issueReadEnrichmentQuery struct { + Nodes []struct { + Issue struct { + ID githubv4.ID + IssueFieldValues struct { + Nodes []IssueFieldValueFragment + } `graphql:"issueFieldValues(first: 25)"` + Parent *struct { + Number githubv4.Int + Title githubv4.String + State githubv4.String + URL githubv4.String + Author struct { + Login githubv4.String + } + Repository struct { + NameWithOwner githubv4.String + } + } + SubIssuesSummary struct { + Total githubv4.Int + Completed githubv4.Int + PercentCompleted githubv4.Int + } + } `graphql:"... on Issue"` + } `graphql:"nodes(ids: $ids)"` +} + +// issueReadParent is the parent reference plus the metadata needed to make a lockdown +// safe-content decision about whether the (possibly cross-repo) parent title may be exposed. +type issueReadParent struct { + Ref MinimalIssueRef + AuthorLogin string +} + +// issueReadEnrichment is the flattened result of the issue_read `get` enrichment query. +type issueReadEnrichment struct { + FieldValues []MinimalFieldValue + Parent *issueReadParent + SubIssuesSummary MinimalSubIssuesSummary +} + +// fetchIssueReadEnrichment runs one GraphQL nodes() query for the given issue node ID and returns +// its field values, parent reference, and sub-issue summary counts. The parent title is sanitized +// here because it may originate from a different repository. +func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, nodeID string) (*issueReadEnrichment, error) { + var q issueReadEnrichmentQuery + if err := gqlClient.Query(ctx, &q, map[string]any{"ids": []githubv4.ID{githubv4.ID(nodeID)}}); err != nil { + return nil, err + } + + enrichment := &issueReadEnrichment{} + for _, n := range q.Nodes { + idStr, ok := n.Issue.ID.(string) + if !ok || idStr != nodeID { + continue + } + + vals := make([]MinimalFieldValue, 0, len(n.Issue.IssueFieldValues.Nodes)) + for _, fv := range n.Issue.IssueFieldValues.Nodes { + if m, ok := fragmentToMinimalFieldValue(fv); ok { + vals = append(vals, m) + } + } + enrichment.FieldValues = vals + + if p := n.Issue.Parent; p != nil { + enrichment.Parent = &issueReadParent{ + Ref: MinimalIssueRef{ + Number: int(p.Number), + Title: sanitize.Sanitize(string(p.Title)), + State: string(p.State), + URL: string(p.URL), + Repository: string(p.Repository.NameWithOwner), + }, + AuthorLogin: string(p.Author.Login), + } + } + + enrichment.SubIssuesSummary = MinimalSubIssuesSummary{ + Total: int(n.Issue.SubIssuesSummary.Total), + Completed: int(n.Issue.SubIssuesSummary.Completed), + PercentCompleted: int(n.Issue.SubIssuesSummary.PercentCompleted), + } + break + } + return enrichment, nil +} + // searchIssuesHandler runs the REST issues search, enriches each hit with custom field values // fetched via a single follow-up GraphQL nodes() query, and applies any post-process options // (e.g. IFC labelling). diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 2dea639f8a..1776640f3d 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -48,6 +48,20 @@ func newRepoAccessHTTPClient() *http.Client { return &http.Client{Transport: &repoAccessMockTransport{responses: responses}} } +// newIssueReadEnrichmentMatcher builds a matcher for the issue_read `get` enrichment query for a +// single issue node ID. The query string is constructed from the typed githubv4.ID variable so the +// generated `$ids:[ID!]!` declaration matches the real client, while the comparison variables are +// reassigned to the []any shape the request body decodes into so equality holds. +func newIssueReadEnrichmentMatcher(nodeID string, response githubv4mock.GQLResponse) githubv4mock.Matcher { + matcher := githubv4mock.NewQueryMatcher( + issueReadEnrichmentQuery{}, + map[string]any{"ids": []githubv4.ID{githubv4.ID(nodeID)}}, + response, + ) + matcher.Variables = map[string]any{"ids": []any{nodeID}} + return matcher +} + func (rt *repoAccessMockTransport) RoundTrip(req *http.Request) (*http.Response, error) { if req.Body == nil { return nil, fmt.Errorf("missing request body") @@ -494,9 +508,6 @@ func Test_GetIssue_FieldValues_Enriched(t *testing.T) { GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssueWithFields), }) - gqlVars := map[string]any{ - "ids": []any{"I_node_99"}, - } gqlResponse := githubv4mock.DataResponse(map[string]any{ "nodes": []map[string]any{ { @@ -515,12 +526,13 @@ func Test_GetIssue_FieldValues_Enriched(t *testing.T) { }, }, }, + "parent": nil, + "subIssuesSummary": map[string]any{"total": 0, "completed": 0, "percentCompleted": 0}, }, }, }) - const nodesQueryString = "query($ids:[ID!]!){nodes(ids: $ids){... on Issue{id,issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}}}}" - matcher := githubv4mock.NewQueryMatcher(nodesQueryString, gqlVars, gqlResponse) + matcher := newIssueReadEnrichmentMatcher("I_node_99", gqlResponse) gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) cache := stubRepoAccessCache(nil, 15*time.Minute) @@ -557,6 +569,249 @@ func Test_GetIssue_FieldValues_Enriched(t *testing.T) { assert.Equal(t, "P1", returnedIssue.FieldValues[0].Value) assert.Equal(t, "estimate", returnedIssue.FieldValues[1].Field) assert.Equal(t, "2.5", returnedIssue.FieldValues[1].Value) + + // With no parent and no sub-issues, the routing booleans are explicit false and the + // optional relationship payloads are omitted. + assert.False(t, returnedIssue.HasParent, "has_parent should be false without a parent") + assert.False(t, returnedIssue.HasChildren, "has_children should be false without sub-issues") + assert.Nil(t, returnedIssue.Parent, "parent should be omitted when there is no parent") + assert.Nil(t, returnedIssue.SubIssuesSummary, "sub_issues_summary should be omitted with no sub-issues") +} + +func Test_GetIssue_HierarchyEnrichment(t *testing.T) { + mockIssue := &github.Issue{ + Number: github.Ptr(2990), + NodeID: github.Ptr("I_node_2990"), + Title: github.Ptr("Child issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/2990"), + User: &github.User{Login: github.Ptr("author")}, + } + + parentNode := map[string]any{ + "number": 2820, + "title": "Parent issue", + "state": "OPEN", + "url": "https://github.com/owner/repo/issues/2820", + "author": map[string]any{"login": "parentauthor"}, + "repository": map[string]any{ + "nameWithOwner": "owner/repo", + }, + } + + tests := []struct { + name string + parent any + summary map[string]any + lockdown bool + assertResponse func(t *testing.T, issue MinimalIssue) + }{ + { + name: "parent and children present", + parent: parentNode, + summary: map[string]any{"total": 4, "completed": 1, "percentCompleted": 25}, + assertResponse: func(t *testing.T, issue MinimalIssue) { + assert.True(t, issue.HasParent) + assert.True(t, issue.HasChildren) + require.NotNil(t, issue.Parent) + assert.Equal(t, 2820, issue.Parent.Number) + assert.Equal(t, "Parent issue", issue.Parent.Title) + assert.Equal(t, "OPEN", issue.Parent.State) + assert.Equal(t, "owner/repo", issue.Parent.Repository) + require.NotNil(t, issue.SubIssuesSummary) + assert.Equal(t, 4, issue.SubIssuesSummary.Total) + assert.Equal(t, 1, issue.SubIssuesSummary.Completed) + assert.Equal(t, 25, issue.SubIssuesSummary.PercentCompleted) + }, + }, + { + name: "no parent omits parent and sets has_parent false", + parent: nil, + summary: map[string]any{"total": 0, "completed": 0, "percentCompleted": 0}, + assertResponse: func(t *testing.T, issue MinimalIssue) { + assert.False(t, issue.HasParent) + assert.Nil(t, issue.Parent) + }, + }, + { + name: "has_children is false when total is zero even with completed nonzero", + parent: nil, + summary: map[string]any{"total": 0, "completed": 0, "percentCompleted": 0}, + assertResponse: func(t *testing.T, issue MinimalIssue) { + assert.False(t, issue.HasChildren) + assert.Nil(t, issue.SubIssuesSummary) + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue), + }) + + gqlResponse := githubv4mock.DataResponse(map[string]any{ + "nodes": []map[string]any{ + { + "id": "I_node_2990", + "issueFieldValues": map[string]any{"nodes": []map[string]any{}}, + "parent": tc.parent, + "subIssuesSummary": tc.summary, + }, + }, + }) + matcher := newIssueReadEnrichmentMatcher("I_node_2990", gqlResponse) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdown}), + } + serverTool := IssueRead(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(2990), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError, "expected result to not be an error") + + var returnedIssue MinimalIssue + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedIssue)) + tc.assertResponse(t, returnedIssue) + }) + } +} + +func Test_GetIssue_HierarchyEnrichment_Lockdown(t *testing.T) { + mockIssue := &github.Issue{ + Number: github.Ptr(2990), + NodeID: github.Ptr("I_node_2990"), + Title: github.Ptr("Child issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/2990"), + User: &github.User{Login: github.Ptr("author")}, + } + + parentNode := map[string]any{ + "number": 2820, + "title": "Sensitive parent title", + "state": "OPEN", + "url": "https://github.com/owner/repo/issues/2820", + "author": map[string]any{"login": "parentauthor"}, + "repository": map[string]any{ + "nameWithOwner": "owner/repo", + }, + } + + // In lockdown mode the issue's own author must be verified as safe (mirrors the existing + // REST lockdown gate). The repo-access cache performs push-access checks against its own + // REST client: the issue author ("author") has write access, while the parent author + // ("parentauthor") only has read access and so cannot be verified as safe. The parent title + // is therefore redacted while the numeric/structural fields remain intact. + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue), + }) + permClient := mockRESTPermissionServer(t, "read", map[string]string{"author": "write"}) + + gqlResponse := githubv4mock.DataResponse(map[string]any{ + "nodes": []map[string]any{ + { + "id": "I_node_2990", + "issueFieldValues": map[string]any{"nodes": []map[string]any{}}, + "parent": parentNode, + "subIssuesSummary": map[string]any{"total": 0, "completed": 0, "percentCompleted": 0}, + }, + }, + }) + matcher := newIssueReadEnrichmentMatcher("I_node_2990", gqlResponse) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(permClient, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": true}), + } + serverTool := IssueRead(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(2990), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError, "expected result to not be an error") + + var returnedIssue MinimalIssue + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedIssue)) + + require.NotNil(t, returnedIssue.Parent) + assert.True(t, returnedIssue.HasParent) + assert.Equal(t, lockdownRedactedTitle, returnedIssue.Parent.Title, "parent title should be redacted under lockdown") + // Numeric/structural fields remain intact even when redacted. + assert.Equal(t, 2820, returnedIssue.Parent.Number) + assert.Equal(t, "OPEN", returnedIssue.Parent.State) + assert.Equal(t, "owner/repo", returnedIssue.Parent.Repository) + assert.Equal(t, "https://github.com/owner/repo/issues/2820", returnedIssue.Parent.URL) +} + +func Test_GetIssue_HierarchyEnrichment_QueryFailureReturnsBaseIssue(t *testing.T) { + mockIssue := &github.Issue{ + Number: github.Ptr(2990), + NodeID: github.Ptr("I_node_2990"), + Title: github.Ptr("Child issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/2990"), + User: &github.User{Login: github.Ptr("author")}, + } + + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue), + }) + + matcher := newIssueReadEnrichmentMatcher("I_node_2990", githubv4mock.ErrorResponse("enrichment failed")) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}), + } + serverTool := IssueRead(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(2990), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.NotNil(t, result) + // Relationship enrichment must never fail `get`: the base issue is still returned. + require.False(t, result.IsError, "enrichment failure should not fail get") + + var returnedIssue MinimalIssue + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedIssue)) + assert.Equal(t, 2990, returnedIssue.Number) + assert.False(t, returnedIssue.HasParent) + assert.False(t, returnedIssue.HasChildren) + assert.Nil(t, returnedIssue.Parent) + assert.Nil(t, returnedIssue.SubIssuesSummary) } func Test_AddIssueComment(t *testing.T) { diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 256bdcb911..5cfaf5bc6b 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -342,6 +342,31 @@ type MinimalIssue struct { IssueType string `json:"issue_type,omitempty"` IssueFieldValues []MinimalIssueFieldValue `json:"issue_field_values,omitempty"` FieldValues []MinimalFieldValue `json:"field_values,omitempty"` + + // Hierarchy relationship signals. HasParent and HasChildren are always emitted + // (an explicit false is itself a useful routing signal); Parent and SubIssuesSummary + // are only populated when the relationship exists. + HasParent bool `json:"has_parent"` + HasChildren bool `json:"has_children"` + Parent *MinimalIssueRef `json:"parent,omitempty"` + SubIssuesSummary *MinimalSubIssuesSummary `json:"sub_issues_summary,omitempty"` +} + +// MinimalIssueRef is a compact reference to a related issue (e.g. a parent issue). +// Its keys mirror the get_parent (GetIssueParent) payload so both surfaces agree. +type MinimalIssueRef struct { + Number int `json:"number"` + Title string `json:"title"` + State string `json:"state"` + URL string `json:"url"` + Repository string `json:"repository,omitempty"` +} + +// MinimalSubIssuesSummary holds the native GraphQL subIssuesSummary counts for an issue. +type MinimalSubIssuesSummary struct { + Total int `json:"total"` + Completed int `json:"completed"` + PercentCompleted int `json:"percent_completed"` } // MinimalIssuesResponse is the trimmed output for a paginated list of issues.