- Notifications
You must be signed in to change notification settings - Fork3.1k
feat(issue_graph): Add focus parameter, cross-repo support, and status extraction#1511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
…tput- Refactor fetchNode to return raw issue data along with node, avoiding second API call in crawl loop- Add 30 second timeout to prevent runaway crawling- Include parent edges in output by reversing direction for display- Move cross-tool info from description to instructions.go (was already there)- Shorter tool description focusing on core functionality
76f4dbc to4dcc5b6CompareUse ListIssueTimeline API to find issues that have 'cross-referenced'events, which indicates they were mentioned in another issue. Thiscaptures relationships like 'mentioned in#815' that aren't visiblein the issue body or sub-issues.The timeline crawling:- Fetches up to 100 timeline events per issue- Filters for 'cross-referenced' event types- Extracts the source issue from cross-reference events- Adds edges with RelationTypeRelated- Queues referenced issues for further crawling
The text format already provides all necessary information for LLMsto understand the issue graph. The redundant JSON output was addingsignificant token overhead without providing additional value.The text format includes:- Graph summary with focus node info and hierarchy- All nodes with type, state, title, and body preview- Parent/child relationships- Related issue connections
- Add legend explaining node types (epic, batch, task, pr)- Rename 'EDGES' to 'SUB-ISSUES' with explicit parent→child label- Rename 'RELATED' to 'CROSS-REFERENCES' with mentioned/referenced label- Use ↔ symbol for bidirectional cross-references- Show '(none)' for empty sections instead of blank- Remove redundant JSON output to reduce token usage
…/PR relationshipsThis prompt provides a guided workflow for:1. Running issue_graph to see current state2. Searching for related issues/PRs that should be connected3. Identifying orphaned or missing relationships4. Proposing and adding connections with user approval5. Verifying connections via issue_graphParameters:- owner/repo/issue_number: The issue/PR to analyze- additional_repos: Cross-repo search (e.g., epic in planning repo)- known_links: User-provided issue URLs that should be connected
…s extraction- Add 'focus' parameter to shift focus to parent epic/batch- Support cross-repo sub-issues and parent lookup via GraphQL- Extract status updates from issue body, comments, and milestones- Add state reason (completed, not_planned, duplicate, merged)- Fetch project info for focus node- Parse full GitHub URLs for cross-repo references- Recognize GitHub Issue Types (Epic) for node classification- Enhanced tool description to emphasize using issue_graph first- Update issue_read description to defer to issue_graph for status- Update server instructions with comprehensive issue_graph guidance
| // Matches owner/repo#123 style references (cross-repo) | ||
| crossRepoRefRegex=regexp.MustCompile(`([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+)#(\d+)`) | ||
| // Matches full GitHub URLs like https://github.com/owner/repo/issues/123 or /pull/123 | ||
| githubURLRefRegex=regexp.MustCompile(`https?://(?:www\.)?github\.com/([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+)/(?:issues|pull)/(\d+)`) |
Check failure
Code scanning / CodeQL
Missing regular expression anchor High
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI about 21 hours ago
To fix the problem, we should add a^ anchor at the start of the regular expression and possibly a$ anchor at the end if we expect the entire string to be just a GitHub URL (often not the case for extraction in free text). In most extraction scenarios, we use functions likeFindAllStringSubmatch to extract all matches from a larger string, and each match is checked against the regex from the start; so using^ in each match ensures that random substrings are not matched. The minimal required fix is to update line 114 in filepkg/github/issue_graph.go by adding^ at the beginning of the regex, i.e., change
https?://(?:www\.)?github\.com/...to
^https?://(?:www\.)?github\.com/...No new imports or additional methods are needed; just a change in the regex definition. This change ensures that the extraction finds only URLs that start at the match, preventing invalid or embedded matches.
| @@ -111,7 +111,7 @@ | ||
| // Matches owner/repo#123 style references (cross-repo) | ||
| crossRepoRefRegex=regexp.MustCompile(`([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+)#(\d+)`) | ||
| // Matches full GitHub URLs like https://github.com/owner/repo/issues/123 or /pull/123 | ||
| githubURLRefRegex=regexp.MustCompile(`https?://(?:www\.)?github\.com/([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+)/(?:issues|pull)/(\d+)`) | ||
| githubURLRefRegex=regexp.MustCompile(`^https?://(?:www\.)?github\.com/([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+)/(?:issues|pull)/(\d+)`) | ||
| // Matches "closes #123", "fixes #123", "resolves #123" patterns (PR linking to issue) | ||
| closesRefRegex=regexp.MustCompile(`(?i)(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:(?:([a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?)/([a-zA-Z0-9._-]+))?#(\d+))`) | ||
| // URL pattern to remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull request overview
This PR adds a comprehensiveissue_graph tool to provide a unified view of issue/PR relationships and project status in a single API call, reducing the need for multiple separate queries.
Key Changes:
- New
issue_graphtool with focus parameter for automatic parent epic/batch discovery - Cross-repository relationship support via GraphQL for parent/sub-issue traversal
- Status extraction from milestones, issue bodies, and recent comments
- Enhanced tool descriptions and server instructions to guide LLMs toward using
issue_graphfor status queries
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/issue_graph.go | Core implementation of the graph crawler with BFS traversal, GraphQL queries, and cross-repo support (1647 lines) |
| pkg/github/issue_graph_test.go | Comprehensive unit tests covering reference extraction, node classification, graph formatting, and cross-repo scenarios |
| pkg/github/issue_graph_prompts.go | ConnectIssueGraph prompt for guided workflow to find and connect missing relationships |
| pkg/github/tools.go | Registers the new issue_graph tool and ConnectIssueGraph prompt in the issues toolset |
| pkg/github/issues.go | Updates issue_read description to defer to issue_graph for status/progress questions |
| pkg/github/instructions.go | Adds detailed guidance on when to use issue_graph as the primary tool for status queries |
| pkg/github/toolsnaps/issue_graph.snap | Tool schema snapshot for the new issue_graph tool |
| pkg/github/toolsnaps/issue_read.snap | Updated snapshot reflecting the modified issue_read description |
| README.md | Documents the new issue_graph tool with its parameters |
| work-plan.md | Unrelated OAuth scopes work documentation (should be removed from this PR) |
| e2e-repos-to-delete.txt | Unrelated e2e test cleanup file (should be removed from this PR) |
| docs/remote-server.md | Unrelated change from "Default" to "all" toolset name |
| | Name| Description| API URL| 1-Click Install (VS Code)| Read-only Link| 1-Click Read-only Install (VS Code)| | ||
| |----------------|--------------------------------------------------|-------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| |Default|["Default" toolset](../README.md#default-toolset)|https://api.githubcopilot.com/mcp/|[Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D)|[read-only](https://api.githubcopilot.com/mcp/readonly)|[Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D)| | ||
| |all|All available GitHub MCP tools|https://api.githubcopilot.com/mcp/|[Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D)|[read-only](https://api.githubcopilot.com/mcp/readonly)|[Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D)| |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This change from "Default" to "all" appears to be unrelated to theissue_graph feature. If this is an intentional change, it should be in a separate PR or explained in the PR description.
| |all| All available GitHub MCP tools|https://api.githubcopilot.com/mcp/|[Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D)|[read-only](https://api.githubcopilot.com/mcp/readonly)|[Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D)| | |
| |Default| All available GitHub MCP tools|https://api.githubcopilot.com/mcp/|[Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D)|[read-only](https://api.githubcopilot.com/mcp/readonly)|[Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D)| |
| vars:=map[string]interface{}{ | ||
| "owner":githubv4.String(owner), | ||
| "repo":githubv4.String(repo), | ||
| "number":githubv4.Int(int32(number)),//nolint:gosec // issue numbers are always small positive integers |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same int32 overflow issue as line 1416. Thegosec suppression should acknowledge the theoretical limitation or use a safer conversion pattern.
| } | ||
| // Add timeout to prevent runaway crawling | ||
| crawlCtx,cancel:=context.WithTimeout(ctx,7*time.Second) |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The timeout of 7 seconds is a magic number without explanation. Consider extracting this to a named constant at the package level (e.g.,MaxGraphCrawlTimeout) with a comment explaining why 7 seconds was chosen. This makes it easier to tune performance and understand the trade-offs.
| "number":githubv4.Int(int32(number)),//nolint:gosec // issue numbers are always small positive integers | ||
| } | ||
| queryCtx,cancel:=context.WithTimeout(ctx,2*time.Second) |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Another magic number timeout (2 seconds). Consider extracting to a named constant for consistency with the other timeout suggestions.
| **CRITICAL: For ANY question about issue/PR/epic status, updates, or progress - ALWAYS call 'issue_graph' FIRST before any other tool.** | ||
| 'issue_graph' is the FASTEST PATH to project status - one call returns the entire work hierarchy (epic → batches → tasks → PRs) instead of multiple separate API calls. | ||
| Trigger phrases that REQUIRE 'issue_graph' as the FIRST tool call: | ||
| - "update on..." / "status of..." / "progress on..." / "tell me about..." | ||
| - "what's happening with..." / "how is... going" / "state of..." | ||
| - "project status" / "how is the project going" / "overall progress" | ||
| - "epic" / "parent issue" / "sub-issues" / "blocking" / "depends on" | ||
| - ANY issue/PR number reference when asking for status or updates | ||
| Example: User asks "give me an update on issue #123" → Call issue_graph(owner, repo, 123) FIRST. | ||
| Example: User asks "what's the status of the project" → Call issue_graph with the epic/tracking issue number. | ||
| The 'issue_graph' tool returns in ONE call: | ||
| - Full hierarchy: epic → batch → task → PR relationships across the entire project | ||
| - All sub-issues and "closes/fixes" references | ||
| - Status updates extracted from issue bodies/comments | ||
| - Cross-references and related work | ||
| - Open/closed state of all related items | ||
| Use focus="epic" to automatically find and focus on the parent epic of any issue. | ||
| ONLY AFTER calling 'issue_graph', use other tools: | ||
| - 'issue_read' for full details of specific issues identified in the graph | ||
| - 'search_issues' to find related issues not in the graph | ||
| For creating/modifying issues: | ||
| - Check 'list_issue_types' first for organizations to use proper issue types | ||
| - Use 'search_issues' before creating new issues to avoid duplicates | ||
| - Always set 'state_reason' when closing issues` |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The instructions include many emphatic phrases like "CRITICAL", "ALWAYS", "MUST", "FIRST", etc. While these emphasize importance, they may be overly forceful. Consider whether a more balanced tone would be clearer and easier for LLMs to parse, while still conveying priority. The repetition of these strong directives across multiple sentences could potentially confuse rather than clarify.
| // Regular expressions for extracting issue references | ||
| var ( | ||
| // Matches #123 style references (same repo) | ||
| sameRepoRefRegex=regexp.MustCompile(`(?:^|[^\w])#(\d+)`) |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The regex pattern for same-repo references uses(?:^|[^\w])#(\d+) which will match#123 in the middle of words. This could potentially extract false positives from hex color codes (e.g.,#ffffff123), commit SHAs, or other contexts where# followed by digits doesn't represent an issue. Consider adding a word boundary check or being more specific about the preceding character to avoid these false matches.
| sameRepoRefRegex=regexp.MustCompile(`(?:^|[^\w])#(\d+)`) | |
| sameRepoRefRegex=regexp.MustCompile(`(?:^|[^\w])#(\d+)\b`) |
| mcp.WithDescription(t("TOOL_ISSUE_READ_DESCRIPTION",`Get detailed information about a single issue: full body, comments, sub-issues, or labels. | ||
| **USE issue_graph INSTEAD when user asks about:** | ||
| - Status, updates, or progress on issues/PRs/epics | ||
| - Project overview or how work is going | ||
| - Parent issues, sub-issues, or work hierarchy | ||
| - Related or blocking issues | ||
| issue_read is for: reading the full issue body, fetching comments, listing sub-issues of a known parent, or getting label details - AFTER you already know which specific issue you need.`)), |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The description update is very verbose and includes bold formatting (**USE issue_graph INSTEAD when user asks about:**). While this emphasizes the importance, consider if this level of detail is appropriate for a tool description field, or if it would be better served in documentation or server instructions. The description is quite long for an API tool definition.
| mcp.WithDescription(t("TOOL_ISSUE_READ_DESCRIPTION", `Get detailed information about a single issue: full body, comments, sub-issues, or labels. | |
| **USEissue_graphINSTEADwhenuserasksabout:** | |
| -Status,updates,orprogressonissues/PRs/epics | |
| -Projectovervieworhowworkisgoing | |
| -Parentissues,sub-issues,orworkhierarchy | |
| -Relatedorblockingissues | |
| issue_readisfor:readingthefullissuebody,fetchingcomments,listingsub-issuesofaknownparent,orgettinglabeldetails-AFTERyoualreadyknowwhichspecificissueyouneed.`)), | |
| mcp.WithDescription(t("TOOL_ISSUE_READ_DESCRIPTION",`Get detailed information about a single issue, including its body, comments, sub-issues, and labels.`)), |
| requestCount:=0 | ||
| mockedHTTPClient:=mock.NewMockedHTTPClient( | ||
| mock.WithRequestMatchHandler( | ||
| mock.GetReposIssuesByOwnerByRepoByIssueNumber, | ||
| http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) { | ||
| // Determine which issue is being requested based on URL | ||
| path:=r.URL.Path | ||
| varissue*github.Issue | ||
| switch { | ||
| casestrings.Contains(path,"/101"): | ||
| issue=subIssue1 | ||
| casestrings.Contains(path,"/102"): | ||
| issue=subIssue2 | ||
| default: | ||
| issue=parentIssue | ||
| } | ||
| w.WriteHeader(http.StatusOK) | ||
| _=json.NewEncoder(w).Encode(issue) | ||
| }), | ||
| ), | ||
| mock.WithRequestMatchHandler( | ||
| mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, | ||
| http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) { | ||
| requestCount++ |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The variablerequestCount is incremented but never checked in the test. If this is intended for debugging or future assertions, consider either using it in an assertion or removing it to keep the test code clean.
| timelineEvents,timelineResp,err:=gc.client.Issues.ListIssueTimeline(ctx,current.owner,current.repo,current.number,&github.ListOptions{ | ||
| PerPage:100, | ||
| }) |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The timeline events API call usesPerPage: 100 without pagination. For issues with many cross-references, this could miss events beyond the first 100. Consider either adding pagination or documenting this limitation as a known trade-off for performance.
| vars:=map[string]interface{}{ | ||
| "owner":githubv4.String(owner), | ||
| "repo":githubv4.String(repo), | ||
| "number":githubv4.Int(int32(number)),//nolint:gosec // issue numbers are always small positive integers |
CopilotAIDec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thegosec lint suppression comment references "issue numbers are always small positive integers", but the conversion toint32 could theoretically overflow for issue numbers above 2,147,483,647. While extremely unlikely in practice, GitHub's API usesint for issue numbers, notint32. Consider using a safer conversion or updating the comment to acknowledge this theoretical limitation.
| vars:=map[string]interface{}{ | |
| "owner":githubv4.String(owner), | |
| "repo":githubv4.String(repo), | |
| "number":githubv4.Int(int32(number)),//nolint:gosec // issue numbers are always small positive integers | |
| // Issue numbers must fit in int32 for githubv4.Int; GitHub API currently uses int, but issue numbers above 2,147,483,647 will overflow. | |
| ifnumber<1||number>int32(^uint32(0)>>1) { | |
| // Return nil or handle error gracefully if issue number is out of int32 range | |
| returnnil | |
| } | |
| vars:=map[string]interface{}{ | |
| "owner":githubv4.String(owner), | |
| "repo":githubv4.String(repo), | |
| "number":githubv4.Int(int32(number)),//nolint:gosec // issue numbers are always small positive integers; conversion will fail for numbers above int32 max |
Uh oh!
There was an error while loading.Please reload this page.
Closes:#1510
Enhances the
issue_graphtool to be theprimary tool for project status queries. This PR adds intelligent focus shifting, cross-repo relationship discovery, and status extraction to provide comprehensive project overviews in a single API call.Changes
New Features
focusparameter - Automatically shifts focus to parent epic or batchfocus="epic"- Finds and focuses on the nearest epic in the hierarchyfocus="batch"- Finds and focuses on the nearest batch/parent issueCross-repo support - Uses GraphQL to discover relationships across repositories
https://github.com/owner/repo/issues/123)Status extraction - Lightweight "if lucky" status detection
State reason - Shows why issues/PRs are in their current state
Project info - Fetches project name and status for the focus node
Issue Types recognition - Detects GitHub's native "Epic" issue type for classification
Tool Description Updates
issue_graphdescription to emphasize it as theprimary tool for status queriesissue_readdescription to defer toissue_graphfor status/progress questionsissue_graphExample Output
What an LLM Gets From This Data
From a single tool call, an LLM can understand:
Testing
findBestFocuswith cross-repo relationshipsChecklist
script/lintpassesscript/testpassesscript/generate-docs