Skip to content

Commit 5968998

Browse files
Merge branch 'main' into fix/2714-sanitize-issue-comment-body
2 parents 82771e4 + b7bc3dc commit 5968998

6 files changed

Lines changed: 442 additions & 26 deletions

File tree

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ The following sets of tools are available:
891891
- `issue_number`: The number of the issue (number, required)
892892
- `method`: The read operation to perform on a single issue.
893893
Options are:
894-
1. get - Get details of a specific issue.
894+
1. get - Get issue details. Also returns best-effort hierarchy flags (`has_parent`, `has_children`); `parent` and `sub_issues_summary` are optional relationship summaries.
895895
2. get_comments - Get issue comments.
896896
3. get_sub_issues - Get sub-issues (children) of the issue.
897897
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:
968968
- 'add' - add a sub-issue to a parent issue in a GitHub repository.
969969
- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.
970970
- '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.
971-
(string, required)
971+
Writes issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.
972+
(string, required)
972973
- `owner`: Repository owner (string, required)
973974
- `replace_parent`: When true, replaces the sub-issue's current parent issue. Use with 'add' method only. (boolean, optional)
974975
- `repo`: Repository name (string, required)

pkg/github/__toolsnaps__/issue_read.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"type": "number"
1313
},
1414
"method": {
15-
"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",
15+
"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",
1616
"enum": [
1717
"get",
1818
"get_comments",

pkg/github/__toolsnaps__/sub_issue_write.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"type": "number"
2121
},
2222
"method": {
23-
"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",
23+
"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",
2424
"type": "string"
2525
},
2626
"owner": {

pkg/github/issues.go

Lines changed: 158 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1515
"github.com/github/github-mcp-server/pkg/ifc"
1616
"github.com/github/github-mcp-server/pkg/inventory"
17+
"github.com/github/github-mcp-server/pkg/lockdown"
1718
"github.com/github/github-mcp-server/pkg/sanitize"
1819
"github.com/github/github-mcp-server/pkg/scopes"
1920
"github.com/github/github-mcp-server/pkg/translations"
@@ -612,14 +613,13 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool {
612613
Properties: map[string]*jsonschema.Schema{
613614
"method": {
614615
Type: "string",
615-
Description: `The read operation to perform on a single issue.
616-
Options are:
617-
1. get - Get details of a specific issue.
618-
2. get_comments - Get issue comments.
619-
3. get_sub_issues - Get sub-issues (children) of the issue.
620-
4. get_parent - Get the parent issue, if this issue is a sub-issue of another.
621-
5. get_labels - Get labels assigned to the issue.
622-
`,
616+
Description: "The read operation to perform on a single issue.\n" +
617+
"Options are:\n" +
618+
"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" +
619+
"2. get_comments - Get issue comments.\n" +
620+
"3. get_sub_issues - Get sub-issues (children) of the issue.\n" +
621+
"4. get_parent - Get the parent issue, if this issue is a sub-issue of another.\n" +
622+
"5. get_labels - Get labels assigned to the issue.\n",
623623
Enum: []any{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"},
624624
},
625625
"owner": {
@@ -762,20 +762,69 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
762762
minimalIssue := convertToMinimalIssue(issue)
763763

764764
// Always drop the verbose REST IssueFieldValues; enrich with the GraphQL
765-
// field_values view instead.
765+
// field_values view and the hierarchy relationship signals instead. The
766+
// enrichment is best-effort: a failure here must never fail `get`.
766767
minimalIssue.IssueFieldValues = nil
767768
if issue != nil && issue.NodeID != nil && *issue.NodeID != "" {
768769
gqlClient, err := deps.GetGQLClient(ctx)
769770
if err == nil {
770-
if fieldValuesByID, err := fetchIssueFieldValuesByNodeID(ctx, gqlClient, []*github.Issue{issue}); err == nil {
771-
minimalIssue.FieldValues = fieldValuesByID[*issue.NodeID]
771+
if enrichment, err := fetchIssueReadEnrichment(ctx, gqlClient, *issue.NodeID); err == nil {
772+
applyIssueReadEnrichment(ctx, &minimalIssue, enrichment, cache, flags.LockdownMode)
772773
}
773774
}
774775
}
775776

776777
return MarshalledTextResult(minimalIssue), nil
777778
}
778779

780+
// applyIssueReadEnrichment populates the hierarchy relationship signals (has_parent/has_children,
781+
// parent, sub_issues_summary) and field_values onto the minimal issue. In lockdown mode the parent
782+
// reference is omitted unless the parent content can be verified as safe; has_parent and the numeric
783+
// counts are structural routing signals and are always safe to surface.
784+
func applyIssueReadEnrichment(ctx context.Context, minimalIssue *MinimalIssue, enrichment *issueReadEnrichment, cache *lockdown.RepoAccessCache, lockdownMode bool) {
785+
if enrichment == nil {
786+
return
787+
}
788+
789+
minimalIssue.FieldValues = enrichment.FieldValues
790+
minimalIssue.HasParent = ToBoolPtr(enrichment.Parent != nil)
791+
minimalIssue.HasChildren = ToBoolPtr(enrichment.SubIssuesSummary.Total > 0)
792+
793+
if parent := enrichment.Parent; parent != nil {
794+
// Surface the parent reference only when it is safe to expose. Under lockdown an
795+
// unverified (possibly cross-repo) parent is omitted entirely, mirroring how unsafe
796+
// comments and sub-issues are filtered out. has_parent still routes an agent to
797+
// get_parent if it needs to follow up.
798+
if !lockdownMode || isSafeParentContent(ctx, cache, parent) {
799+
ref := parent.Ref
800+
minimalIssue.Parent = &ref
801+
}
802+
}
803+
804+
if enrichment.SubIssuesSummary.Total > 0 {
805+
summary := enrichment.SubIssuesSummary
806+
minimalIssue.SubIssuesSummary = &summary
807+
}
808+
}
809+
810+
// isSafeParentContent reports whether the parent issue reference can be exposed under lockdown mode.
811+
// It fails closed: any inability to positively verify safe content (missing cache, missing author,
812+
// unparseable repository, or a lookup error) results in the parent reference being omitted.
813+
func isSafeParentContent(ctx context.Context, cache *lockdown.RepoAccessCache, parent *issueReadParent) bool {
814+
if cache == nil || parent.AuthorLogin == "" {
815+
return false
816+
}
817+
owner, repo, ok := strings.Cut(parent.Ref.Repository, "/")
818+
if !ok || owner == "" || repo == "" {
819+
return false
820+
}
821+
safe, err := cache.IsSafeContent(ctx, parent.AuthorLogin, owner, repo)
822+
if err != nil {
823+
return false
824+
}
825+
return safe
826+
}
827+
779828
func GetIssueComments(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) {
780829
cache, err := deps.GetRepoAccessCache(ctx)
781830
if err != nil {
@@ -1347,12 +1396,12 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
13471396
Properties: map[string]*jsonschema.Schema{
13481397
"method": {
13491398
Type: "string",
1350-
Description: `The action to perform on a single sub-issue
1351-
Options are:
1352-
- 'add' - add a sub-issue to a parent issue in a GitHub repository.
1353-
- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.
1354-
- '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.
1355-
`,
1399+
Description: "The action to perform on a single sub-issue\n" +
1400+
"Options are:\n" +
1401+
"- 'add' - add a sub-issue to a parent issue in a GitHub repository.\n" +
1402+
"- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.\n" +
1403+
"- '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" +
1404+
"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",
13561405
},
13571406
"owner": {
13581407
Type: "string",
@@ -1790,6 +1839,98 @@ func fetchIssueFieldValuesByNodeID(ctx context.Context, gqlClient *githubv4.Clie
17901839
return result, nil
17911840
}
17921841

1842+
// issueReadEnrichmentQuery fetches, in a single GraphQL round-trip, the custom field values,
1843+
// parent reference, and sub-issue summary counts for the issues identified by their node IDs.
1844+
// It powers the issue_read `get` relationship signals without adding extra round-trips.
1845+
type issueReadEnrichmentQuery struct {
1846+
Nodes []struct {
1847+
Issue struct {
1848+
ID githubv4.ID
1849+
IssueFieldValues struct {
1850+
Nodes []IssueFieldValueFragment
1851+
} `graphql:"issueFieldValues(first: 25)"`
1852+
Parent *struct {
1853+
Number githubv4.Int
1854+
Title githubv4.String
1855+
State githubv4.String
1856+
URL githubv4.String
1857+
Author struct {
1858+
Login githubv4.String
1859+
}
1860+
Repository struct {
1861+
NameWithOwner githubv4.String
1862+
}
1863+
}
1864+
SubIssuesSummary struct {
1865+
Total githubv4.Int
1866+
Completed githubv4.Int
1867+
PercentCompleted githubv4.Int
1868+
}
1869+
} `graphql:"... on Issue"`
1870+
} `graphql:"nodes(ids: $ids)"`
1871+
}
1872+
1873+
// issueReadParent is the parent reference plus the metadata needed to make a lockdown
1874+
// safe-content decision about whether the (possibly cross-repo) parent title may be exposed.
1875+
type issueReadParent struct {
1876+
Ref MinimalIssueRef
1877+
AuthorLogin string
1878+
}
1879+
1880+
// issueReadEnrichment is the flattened result of the issue_read `get` enrichment query.
1881+
type issueReadEnrichment struct {
1882+
FieldValues []MinimalFieldValue
1883+
Parent *issueReadParent
1884+
SubIssuesSummary MinimalSubIssuesSummary
1885+
}
1886+
1887+
// fetchIssueReadEnrichment runs one GraphQL nodes() query for the given issue node ID and returns
1888+
// its field values, parent reference, and sub-issue summary counts. The parent title is sanitized
1889+
// here because it may originate from a different repository.
1890+
func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, nodeID string) (*issueReadEnrichment, error) {
1891+
var q issueReadEnrichmentQuery
1892+
if err := gqlClient.Query(ctx, &q, map[string]any{"ids": []githubv4.ID{githubv4.ID(nodeID)}}); err != nil {
1893+
return nil, err
1894+
}
1895+
1896+
enrichment := &issueReadEnrichment{}
1897+
for _, n := range q.Nodes {
1898+
idStr, ok := n.Issue.ID.(string)
1899+
if !ok || idStr != nodeID {
1900+
continue
1901+
}
1902+
1903+
vals := make([]MinimalFieldValue, 0, len(n.Issue.IssueFieldValues.Nodes))
1904+
for _, fv := range n.Issue.IssueFieldValues.Nodes {
1905+
if m, ok := fragmentToMinimalFieldValue(fv); ok {
1906+
vals = append(vals, m)
1907+
}
1908+
}
1909+
enrichment.FieldValues = vals
1910+
1911+
if p := n.Issue.Parent; p != nil {
1912+
enrichment.Parent = &issueReadParent{
1913+
Ref: MinimalIssueRef{
1914+
Number: int(p.Number),
1915+
Title: sanitize.Sanitize(string(p.Title)),
1916+
State: string(p.State),
1917+
URL: string(p.URL),
1918+
Repository: string(p.Repository.NameWithOwner),
1919+
},
1920+
AuthorLogin: string(p.Author.Login),
1921+
}
1922+
}
1923+
1924+
enrichment.SubIssuesSummary = MinimalSubIssuesSummary{
1925+
Total: int(n.Issue.SubIssuesSummary.Total),
1926+
Completed: int(n.Issue.SubIssuesSummary.Completed),
1927+
PercentCompleted: int(n.Issue.SubIssuesSummary.PercentCompleted),
1928+
}
1929+
break
1930+
}
1931+
return enrichment, nil
1932+
}
1933+
17931934
// searchIssuesHandler runs the REST issues search, enriches each hit with custom field values
17941935
// fetched via a single follow-up GraphQL nodes() query, and applies any post-process options
17951936
// (e.g. IFC labelling).

0 commit comments

Comments
 (0)