Enrich issue_read get with hierarchy relationship signals (#2764)
* 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> * Tighten hierarchy tool wording Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Omit unverified parent under lockdown instead of redacting title Align issue_read get parent enrichment with the codebase's existing lockdown patterns: rather than introducing a third, redaction-with-sentinel behavior, omit the whole parent reference when its (possibly cross-repo) content cannot be verified safe. This mirrors how unsafe comments, sub-issues, and PR reviews are filtered out. has_parent stays true so an agent can still route to get_parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Restore explicit issue read query matcher Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Omit hierarchy flags when enrichment fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address hierarchy review nits Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roberto Nacu <kerobbi@github.com>
This commit is contained in:
@@ -891,7 +891,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 issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries.
|
||||
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.
|
||||
@@ -968,7 +968,8 @@ 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.
|
||||
(string, required)
|
||||
Writes issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.
|
||||
(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)
|
||||
- `repo`: Repository name (string, required)
|
||||
|
||||
@@ -12,7 +12,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 issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries.\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",
|
||||
|
||||
@@ -20,7 +20,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.\nWrites issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.\n",
|
||||
"type": "string"
|
||||
},
|
||||
"owner": {
|
||||
|
||||
+158
-17
@@ -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"
|
||||
@@ -612,14 +613,13 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool {
|
||||
Properties: map[string]*jsonschema.Schema{
|
||||
"method": {
|
||||
Type: "string",
|
||||
Description: `The read operation to perform on a single issue.
|
||||
Options are:
|
||||
1. get - Get details of a specific issue.
|
||||
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.
|
||||
5. get_labels - Get labels assigned to the issue.
|
||||
`,
|
||||
Description: "The read operation to perform on a single issue.\n" +
|
||||
"Options are:\n" +
|
||||
"1. get - Get issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries.\n" +
|
||||
"2. get_comments - Get issue comments.\n" +
|
||||
"3. get_sub_issues - Get sub-issues (children) of the issue.\n" +
|
||||
"4. get_parent - Get the parent issue, if this issue is a sub-issue of another.\n" +
|
||||
"5. get_labels - Get labels assigned to the issue.\n",
|
||||
Enum: []any{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"},
|
||||
},
|
||||
"owner": {
|
||||
@@ -762,13 +762,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 +777,54 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
|
||||
return MarshalledTextResult(minimalIssue), nil
|
||||
}
|
||||
|
||||
// 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
|
||||
// reference is omitted unless the parent content can be verified as safe; has_parent and the numeric
|
||||
// counts are structural routing signals 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
|
||||
minimalIssue.HasParent = ToBoolPtr(enrichment.Parent != nil)
|
||||
minimalIssue.HasChildren = ToBoolPtr(enrichment.SubIssuesSummary.Total > 0)
|
||||
|
||||
if parent := enrichment.Parent; parent != nil {
|
||||
// Surface the parent reference only when it is safe to expose. Under lockdown an
|
||||
// unverified (possibly cross-repo) parent is omitted entirely, mirroring how unsafe
|
||||
// comments and sub-issues are filtered out. has_parent still routes an agent to
|
||||
// get_parent if it needs to follow up.
|
||||
if !lockdownMode || isSafeParentContent(ctx, cache, parent) {
|
||||
ref := parent.Ref
|
||||
minimalIssue.Parent = &ref
|
||||
}
|
||||
}
|
||||
|
||||
if enrichment.SubIssuesSummary.Total > 0 {
|
||||
summary := enrichment.SubIssuesSummary
|
||||
minimalIssue.SubIssuesSummary = &summary
|
||||
}
|
||||
}
|
||||
|
||||
// isSafeParentContent reports whether the parent issue reference 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 the parent reference being omitted.
|
||||
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 {
|
||||
@@ -1343,12 +1392,12 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
|
||||
Properties: map[string]*jsonschema.Schema{
|
||||
"method": {
|
||||
Type: "string",
|
||||
Description: `The action to perform on a single sub-issue
|
||||
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.
|
||||
`,
|
||||
Description: "The action to perform on a single sub-issue\n" +
|
||||
"Options 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" +
|
||||
"Writes issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.\n",
|
||||
},
|
||||
"owner": {
|
||||
Type: "string",
|
||||
@@ -1786,6 +1835,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).
|
||||
|
||||
+253
-5
@@ -49,6 +49,18 @@ func newRepoAccessHTTPClient() *http.Client {
|
||||
return &http.Client{Transport: &repoAccessMockTransport{responses: responses}}
|
||||
}
|
||||
|
||||
const issueReadEnrichmentQueryString = "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}}},parent{number,title,state,url,author{login},repository{nameWithOwner}},subIssuesSummary{total,completed,percentCompleted}}}}"
|
||||
|
||||
// newIssueReadEnrichmentMatcher builds a matcher for the issue_read `get` enrichment query for a
|
||||
// single issue node ID.
|
||||
func newIssueReadEnrichmentMatcher(nodeID string, response githubv4mock.GQLResponse) githubv4mock.Matcher {
|
||||
return githubv4mock.NewQueryMatcher(
|
||||
issueReadEnrichmentQueryString,
|
||||
map[string]any{"ids": []any{nodeID}},
|
||||
response,
|
||||
)
|
||||
}
|
||||
|
||||
func (rt *repoAccessMockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
if req.Body == nil {
|
||||
return nil, fmt.Errorf("missing request body")
|
||||
@@ -495,9 +507,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{
|
||||
{
|
||||
@@ -516,12 +525,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)
|
||||
@@ -558,6 +568,244 @@ 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.Equal(t, github.Ptr(false), returnedIssue.HasParent, "has_parent should be false without a parent")
|
||||
assert.Equal(t, github.Ptr(false), 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.Equal(t, github.Ptr(true), issue.HasParent)
|
||||
assert.Equal(t, github.Ptr(true), 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.Equal(t, github.Ptr(false), 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": 1, "percentCompleted": 0},
|
||||
assertResponse: func(t *testing.T, issue MinimalIssue) {
|
||||
assert.Equal(t, github.Ptr(false), 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
|
||||
// reference is therefore omitted entirely, while has_parent stays true so an agent can
|
||||
// still route to get_parent.
|
||||
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.Nil(t, returnedIssue.Parent, "parent reference should be omitted under lockdown when it cannot be verified safe")
|
||||
assert.Equal(t, github.Ptr(true), returnedIssue.HasParent, "has_parent should still be true so agents can route to get_parent")
|
||||
}
|
||||
|
||||
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.Nil(t, returnedIssue.HasParent)
|
||||
assert.Nil(t, returnedIssue.HasChildren)
|
||||
assert.Nil(t, returnedIssue.Parent)
|
||||
assert.Nil(t, returnedIssue.SubIssuesSummary)
|
||||
}
|
||||
|
||||
func Test_SearchIssues(t *testing.T) {
|
||||
|
||||
@@ -342,6 +342,32 @@ 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 populated when
|
||||
// hierarchy enrichment succeeds; SubIssuesSummary is populated when children exist,
|
||||
// and Parent when a parent exists and may be surfaced (under lockdown an unverified
|
||||
// parent reference is omitted while HasParent stays true).
|
||||
HasParent *bool `json:"has_parent,omitempty"`
|
||||
HasChildren *bool `json:"has_children,omitempty"`
|
||||
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) response shape.
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user