diff --git a/README.md b/README.md index e0ebe0f72..be9288e40 100644 --- a/README.md +++ b/README.md @@ -234,10 +234,11 @@ Optionally, you can add a similar example (i.e. without the mcp key) to a file c -### Install in Other Host Applications +### Install in Other MCP Hosts For other MCP host applications, please refer to our installation guides: +- **[GitHub Copilot in other IDEs](/docs/installation-guides/install-other-copilot-ides.md)** - Installation for JetBrains, Visual Studio, Eclipse, and Xcode with GitHub Copilot - **[Claude Code & Claude Desktop](docs/installation-guides/install-claude.md)** - Installation guide for Claude Code and Claude Desktop - **[Cursor](docs/installation-guides/install-cursor.md)** - Installation guide for Cursor IDE - **[Windsurf](docs/installation-guides/install-windsurf.md)** - Installation guide for Windsurf IDE @@ -448,21 +449,23 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - **get_discussion_comments** - Get discussion comments + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `discussionNumber`: Discussion Number (number, required) - `owner`: Repository owner (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `repo`: Repository name (string, required) - **list_discussion_categories** - List discussion categories - - `after`: Cursor for pagination, use the 'after' field from the previous response (string, optional) - - `before`: Cursor for pagination, use the 'before' field from the previous response (string, optional) - - `first`: Number of categories to return per page (min 1, max 100) (number, optional) - - `last`: Number of categories to return from the end (min 1, max 100) (number, optional) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - **list_discussions** - List discussions + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `category`: Optional filter by discussion category ID. If provided, only discussions with this category are listed. (string, optional) + - `direction`: Order direction. (string, optional) + - `orderBy`: Order discussions by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `repo`: Repository name (string, required) @@ -477,6 +480,13 @@ The following sets of tools are available (all are on by default): - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) +- **add_sub_issue** - Add sub-issue + - `issue_number`: The number of the parent issue (number, required) + - `owner`: Repository owner (string, required) + - `replace_parent`: When true, replaces the sub-issue's current parent issue (boolean, optional) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The ID of the sub-issue to add. ID is not the same as issue number (number, required) + - **assign_copilot_to_issue** - Assign Copilot to issue - `issueNumber`: Issue number (number, required) - `owner`: Repository owner (string, required) @@ -514,6 +524,27 @@ The following sets of tools are available (all are on by default): - `sort`: Sort order (string, optional) - `state`: Filter by state (string, optional) +- **list_sub_issues** - List sub-issues + - `issue_number`: Issue number (number, required) + - `owner`: Repository owner (string, required) + - `page`: Page number for pagination (default: 1) (number, optional) + - `per_page`: Number of results per page (max 100, default: 30) (number, optional) + - `repo`: Repository name (string, required) + +- **remove_sub_issue** - Remove sub-issue + - `issue_number`: The number of the parent issue (number, required) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The ID of the sub-issue to remove. ID is not the same as issue number (number, required) + +- **reprioritize_sub_issue** - Reprioritize sub-issue + - `after_id`: The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified) (number, optional) + - `before_id`: The ID of the sub-issue to be prioritized before (either after_id OR before_id should be specified) (number, optional) + - `issue_number`: The number of the parent issue (number, required) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The ID of the sub-issue to reprioritize. ID is not the same as issue number (number, required) + - **search_issues** - Search issues - `order`: Sort order (string, optional) - `owner`: Optional repository owner. If provided with repo, only notifications for this repository are listed. (string, optional) @@ -807,7 +838,7 @@ The following sets of tools are available (all are on by default): - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `q`: Search query using GitHub code search syntax (string, required) + - `query`: Search query using GitHub code search syntax (string, required) - `sort`: Sort field ('indexed' only) (string, optional) - **search_repositories** - Search repositories diff --git a/cmd/mcpcurl/README.md b/cmd/mcpcurl/README.md index 317c2b8e5..717ea207f 100644 --- a/cmd/mcpcurl/README.md +++ b/cmd/mcpcurl/README.md @@ -15,6 +15,26 @@ be executed against the configured MCP server. ## Installation +### Prerequisites +- Go 1.21 or later +- Access to the GitHub MCP Server from either Docker or local build + +### Build from Source +```bash +cd cmd/mcpcurl +go build -o mcpcurl +``` + +### Using Go Install +```bash +go install github.com/github/github-mcp-server/cmd/mcpcurl@latest +``` + +### Verify Installation +```bash +./mcpcurl --help +``` + ## Usage ```console diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go index bc192587a..17b4bc77c 100644 --- a/cmd/mcpcurl/main.go +++ b/cmd/mcpcurl/main.go @@ -280,6 +280,8 @@ func addCommandFromTool(toolsCmd *cobra.Command, tool *Tool, prettyPrint bool) { } case "number": cmd.Flags().Float64(name, 0, description) + case "integer": + cmd.Flags().Int64(name, 0, description) case "boolean": cmd.Flags().Bool(name, false, description) case "array": @@ -319,6 +321,10 @@ func buildArgumentsMap(cmd *cobra.Command, tool *Tool) (map[string]interface{}, if value, _ := cmd.Flags().GetFloat64(name); value != 0 { arguments[name] = value } + case "integer": + if value, _ := cmd.Flags().GetInt64(name); value != 0 { + arguments[name] = value + } case "boolean": // For boolean, we need to check if it was explicitly set if cmd.Flags().Changed(name) { diff --git a/docs/installation-guides/install-cursor.md b/docs/installation-guides/install-cursor.md index 82b36c3e6..b069addd3 100644 --- a/docs/installation-guides/install-cursor.md +++ b/docs/installation-guides/install-cursor.md @@ -7,10 +7,18 @@ ## Remote Server Setup (Recommended) -The remote GitHub MCP server is hosted by GitHub at `https://api.githubcopilot.com/mcp/` and supports Streamable HTTP protocol. Cursor currently supports remote servers with PAT authentication. +[![Install MCP Server](https://cursor.com/deeplink/mcp-install-dark.svg)](https://cursor.com/install-mcp?name=github&config=eyJ1cmwiOiJodHRwczovL2FwaS5naXRodWJjb3BpbG90LmNvbS9tY3AvIiwiaGVhZGVycyI6eyJBdXRob3JpemF0aW9uIjoiQmVhcmVyIFlPVVJfR0lUSFVCX1BBVCJ9LCJ0eXBlIjoiaHR0cCJ9) + +Uses GitHub's hosted server at https://api.githubcopilot.com/mcp/. Requires Cursor v0.48.0+ for Streamable HTTP support. While Cursor supports OAuth for some MCP servers, the GitHub server currently requires a Personal Access Token. + +### Install steps +1. Click the install button above and follow the flow, or go directly to your global MCP configuration file at `~/.cursor/mcp.json` and enter the code block below +2. In Tools & Integrations > MCP tools, click the pencil icon next to "github" +3. Replace `YOUR_GITHUB_PAT` with your actual [GitHub Personal Access Token](https://github.com/settings/tokens) +4. Save the file +5. Restart Cursor ### Streamable HTTP Configuration -As of Cursor v0.48.0, Cursor supports Streamable HTTP servers directly: ```json { @@ -25,12 +33,20 @@ As of Cursor v0.48.0, Cursor supports Streamable HTTP servers directly: } ``` -**Note**: You may need to update to the latest version, if the current version doesn't support direct Streamable HTTP - ## Local Server Setup -### Docker Installation (Required) -> **Important**: The npm package `@modelcontextprotocol/server-github` is no longer supported as of April 2025. Use the official Docker image `ghcr.io/github/github-mcp-server` instead. +[![Install MCP Server](https://cursor.com/deeplink/mcp-install-dark.svg)](https://cursor.com/install-mcp?name=github&config=eyJjb21tYW5kIjoiZG9ja2VyIiwiYXJncyI6WyJydW4iLCItaSIsIi0tcm0iLCItZSIsIkdJVEhVQl9QRVJTT05BTF9BQ0NFU1NfVE9LRU4iLCJnaGNyLmlvL2dpdGh1Yi9naXRodWItbWNwLXNlcnZlciJdLCJlbnYiOnsiR0lUSFVCX1BFUlNPTkFMX0FDQ0VTU19UT0tFTiI6IllPVVJfR0lUSFVCX1BHVCJ9fQ==) + +The local GitHub MCP server runs via Docker and requires Docker Desktop to be installed and running. + +### Install steps +1. Click the install button above and follow the flow, or go directly to your global MCP configuration file at `~/.cursor/mcp.json` and enter the code block below +2. In Tools & Integrations > MCP tools, click the pencil icon next to "github" +3. Replace `YOUR_GITHUB_PAT` with your actual [GitHub Personal Access Token](https://github.com/settings/tokens) +4. Save the file +5. Restart Cursor + +### Docker Configuration ```json { @@ -53,50 +69,18 @@ As of Cursor v0.48.0, Cursor supports Streamable HTTP servers directly: } ``` -## Installation Steps - -### Via Cursor Settings UI -1. Open Cursor -2. Navigate to **Settings** → **Tools & Integrations** → **MCP** -3. Click **"+ Add new global MCP server"** -4. This opens `~/.cursor/mcp.json` in the editor -5. Add your chosen configuration from above -6. Save the file -7. Restart Cursor - -### Manual Configuration -1. Create or edit the configuration file: - - **Global (all projects)**: `~/.cursor/mcp.json` - - **Project-specific**: `.cursor/mcp.json` in project root -2. Add your chosen configuration -3. Save the file -4. Restart Cursor completely - -### Token Security -- Create PATs with minimum required scopes: - - `repo` - For repository operations - - `read:packages` - For Docker image pull (local setup) - - Additional scopes based on tools you need -- Use separate PATs for different projects -- Regularly rotate tokens -- Never commit configuration files to version control - -## Configuration Details - -- **File paths**: - - Global: `~/.cursor/mcp.json` - - Project: `.cursor/mcp.json` -- **Scope**: Both global and project-specific configurations supported -- **Format**: Must be valid JSON (use a linter to verify) - -## Verification - -After installation: +> **Important**: The npm package `@modelcontextprotocol/server-github` is no longer supported as of April 2025. Use the official Docker image `ghcr.io/github/github-mcp-server` instead. + +## Configuration Files + +- **Global (all projects)**: `~/.cursor/mcp.json` +- **Project-specific**: `.cursor/mcp.json` in project root + +## Verify Installation 1. Restart Cursor completely -2. Open Settings → Tools & Integrations → MCP -3. Look for green dot next to your server name -4. In chat/composer, check "Available Tools" -5. Test with: "List my GitHub repositories" +2. Check for green dot in Settings → Tools & Integrations → MCP Tools +3. In chat/composer, check "Available Tools" +4. Test with: "List my GitHub repositories" ## Troubleshooting diff --git a/docs/installation-guides/install-other-copilot-ides.md b/docs/installation-guides/install-other-copilot-ides.md index 18ffdd84a..38b48bbbd 100644 --- a/docs/installation-guides/install-other-copilot-ides.md +++ b/docs/installation-guides/install-other-copilot-ides.md @@ -1,4 +1,4 @@ -# Install GitHub MCP Server in Copilot IDEs & GitHub.com +# Install GitHub MCP Server in Copilot IDEs Quick setup guide for the GitHub MCP server in GitHub Copilot across different IDEs. For VS Code instructions, refer to the [VS Code install guide in the README](/README.md#installation-in-vs-code) diff --git a/docs/testing.md b/docs/testing.md index dbdc3e080..226660e9d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -22,7 +22,7 @@ This project uses a combination of unit tests and end-to-end (e2e) tests to ensu ## toolsnaps: Tool Schema Snapshots - The `toolsnaps` utility ensures that the JSON schema for each tool does not change unexpectedly. -- Snapshots are stored in `__toolsnaps__/*.snap` files , where `*` represents the name of the tool +- Snapshots are stored in `__toolsnaps__/*.snap` files, where `*` represents the name of the tool - When running tests, the current tool schema is compared to the snapshot. If there is a difference, the test will fail and show a diff. - If you intentionally change a tool's schema, update the snapshots by running tests with the environment variable: `UPDATE_TOOLSNAPS=true go test ./...` - In CI (when `GITHUB_ACTIONS=true`), missing snapshots will cause a test failure to ensure snapshots are always diff --git a/pkg/github/__toolsnaps__/add_sub_issue.snap b/pkg/github/__toolsnaps__/add_sub_issue.snap new file mode 100644 index 000000000..2d462bcaf --- /dev/null +++ b/pkg/github/__toolsnaps__/add_sub_issue.snap @@ -0,0 +1,39 @@ +{ + "annotations": { + "title": "Add sub-issue", + "readOnlyHint": false + }, + "description": "Add a sub-issue to a parent issue in a GitHub repository.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The number of the parent issue", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "replace_parent": { + "description": "When true, replaces the sub-issue's current parent issue", + "type": "boolean" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to add. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "add_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_sub_issues.snap b/pkg/github/__toolsnaps__/list_sub_issues.snap new file mode 100644 index 000000000..70640e270 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_sub_issues.snap @@ -0,0 +1,38 @@ +{ + "annotations": { + "title": "List sub-issues", + "readOnlyHint": true + }, + "description": "List sub-issues for a specific issue in a GitHub repository.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "Issue number", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (default: 1)", + "type": "number" + }, + "per_page": { + "description": "Number of results per page (max 100, default: 30)", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number" + ], + "type": "object" + }, + "name": "list_sub_issues" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_sub_issue.snap b/pkg/github/__toolsnaps__/remove_sub_issue.snap new file mode 100644 index 000000000..a29020099 --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_sub_issue.snap @@ -0,0 +1,35 @@ +{ + "annotations": { + "title": "Remove sub-issue", + "readOnlyHint": false + }, + "description": "Remove a sub-issue from a parent issue in a GitHub repository.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The number of the parent issue", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to remove. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "remove_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap b/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap new file mode 100644 index 000000000..43c258b33 --- /dev/null +++ b/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap @@ -0,0 +1,43 @@ +{ + "annotations": { + "title": "Reprioritize sub-issue", + "readOnlyHint": false + }, + "description": "Reprioritize a sub-issue to a different position in the parent issue's sub-issue list.", + "inputSchema": { + "properties": { + "after_id": { + "description": "The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified)", + "type": "number" + }, + "before_id": { + "description": "The ID of the sub-issue to be prioritized before (either after_id OR before_id should be specified)", + "type": "number" + }, + "issue_number": { + "description": "The number of the parent issue", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to reprioritize. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "reprioritize_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/search_code.snap b/pkg/github/__toolsnaps__/search_code.snap index c85d6674d..e341f3e38 100644 --- a/pkg/github/__toolsnaps__/search_code.snap +++ b/pkg/github/__toolsnaps__/search_code.snap @@ -25,7 +25,7 @@ "minimum": 1, "type": "number" }, - "q": { + "query": { "description": "Search query using GitHub code search syntax", "type": "string" }, @@ -35,7 +35,7 @@ } }, "required": [ - "q" + "query" ], "type": "object" }, diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 3c441d5aa..19b56389c 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -62,8 +62,8 @@ func ListWorkflows(getClient GetClientFn, t translations.TranslationHelperFunc) // Set up list options opts := &github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, } workflows, resp, err := client.Actions.ListWorkflows(ctx, owner, repo, opts) @@ -200,8 +200,8 @@ func ListWorkflowRuns(getClient GetClientFn, t translations.TranslationHelperFun Event: event, Status: status, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, }, } @@ -503,8 +503,8 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun opts := &github.ListWorkflowJobsOptions{ Filter: filter, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, }, } @@ -1025,8 +1025,8 @@ func ListWorkflowRunArtifacts(getClient GetClientFn, t translations.TranslationH // Set up list options opts := &github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, } artifacts, resp, err := client.Actions.ListWorkflowRunArtifacts(ctx, owner, repo, runID, opts) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 23e2724d4..fce07ecdb 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,6 +13,110 @@ import ( "github.com/shurcooL/githubv4" ) +const DefaultGraphQLPageSize = 30 + +// Common interface for all discussion query types +type DiscussionQueryResult interface { + GetDiscussionFragment() DiscussionFragment +} + +// Implement the interface for all query types +func (q *BasicNoOrder) GetDiscussionFragment() DiscussionFragment { + return q.Repository.Discussions +} + +func (q *BasicWithOrder) GetDiscussionFragment() DiscussionFragment { + return q.Repository.Discussions +} + +func (q *WithCategoryAndOrder) GetDiscussionFragment() DiscussionFragment { + return q.Repository.Discussions +} + +func (q *WithCategoryNoOrder) GetDiscussionFragment() DiscussionFragment { + return q.Repository.Discussions +} + +type DiscussionFragment struct { + Nodes []NodeFragment + PageInfo PageInfoFragment + TotalCount githubv4.Int +} + +type NodeFragment struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + UpdatedAt githubv4.DateTime + Author struct { + Login githubv4.String + } + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` +} + +type PageInfoFragment struct { + HasNextPage bool + HasPreviousPage bool + StartCursor githubv4.String + EndCursor githubv4.String +} + +type BasicNoOrder struct { + Repository struct { + Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after)"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type BasicWithOrder struct { + Repository struct { + Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection })"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type WithCategoryAndOrder struct { + Repository struct { + Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection })"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type WithCategoryNoOrder struct { + Repository struct { + Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId)"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +func fragmentToDiscussion(fragment NodeFragment) *github.Discussion { + return &github.Discussion{ + Number: github.Ptr(int(fragment.Number)), + Title: github.Ptr(string(fragment.Title)), + HTMLURL: github.Ptr(string(fragment.URL)), + CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time}, + UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time}, + User: &github.User{ + Login: github.Ptr(string(fragment.Author.Login)), + }, + DiscussionCategory: &github.DiscussionCategory{ + Name: github.Ptr(string(fragment.Category.Name)), + }, + } +} + +func getQueryType(useOrdering bool, categoryID *githubv4.ID) any { + if categoryID != nil && useOrdering { + return &WithCategoryAndOrder{} + } + if categoryID != nil && !useOrdering { + return &WithCategoryNoOrder{} + } + if categoryID == nil && useOrdering { + return &BasicWithOrder{} + } + return &BasicNoOrder{} +} + func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussions", mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")), @@ -31,9 +135,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp mcp.WithString("category", mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."), ), + mcp.WithString("orderBy", + mcp.Description("Order discussions by field. If provided, the 'direction' also needs to be provided."), + mcp.Enum("CREATED_AT", "UPDATED_AT"), + ), + mcp.WithString("direction", + mcp.Description("Order direction."), + mcp.Enum("ASC", "DESC"), + ), + WithCursorPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Required params owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -43,107 +155,96 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(err.Error()), nil } - // Optional params category, err := OptionalParam[string](request, "category") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + orderBy, err := OptionalParam[string](request, "orderBy") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + direction, err := OptionalParam[string](request, "direction") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Get pagination parameters and convert to GraphQL format + pagination, err := OptionalCursorPaginationParams(request) + if err != nil { + return nil, err + } + paginationParams, err := pagination.ToGraphQLParams() + if err != nil { + return nil, err + } + client, err := getGQLClient(ctx) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil } - // If category filter is specified, use it as the category ID for server-side filtering var categoryID *githubv4.ID if category != "" { id := githubv4.ID(category) categoryID = &id } - // Now execute the discussions query - var discussions []*github.Discussion + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "first": githubv4.Int(*paginationParams.First), + } + if paginationParams.After != nil { + vars["after"] = githubv4.String(*paginationParams.After) + } else { + vars["after"] = (*githubv4.String)(nil) + } + + // this is an extra check in case the tool description is misinterpreted, because + // we shouldn't use ordering unless both a 'field' and 'direction' are provided + useOrdering := orderBy != "" && direction != "" + if useOrdering { + vars["orderByField"] = githubv4.DiscussionOrderField(orderBy) + vars["orderByDirection"] = githubv4.OrderDirection(direction) + } + if categoryID != nil { - // Query with category filter (server-side filtering) - var query struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100, categoryId: $categoryId)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "categoryId": *categoryID, - } - if err := client.Query(ctx, &query, vars); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } + vars["categoryId"] = *categoryID + } - // Map nodes to GitHub Discussion objects - for _, n := range query.Repository.Discussions.Nodes { - di := &github.Discussion{ - Number: github.Ptr(int(n.Number)), - Title: github.Ptr(string(n.Title)), - HTMLURL: github.Ptr(string(n.URL)), - CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, - DiscussionCategory: &github.DiscussionCategory{ - Name: github.Ptr(string(n.Category.Name)), - }, - } - discussions = append(discussions, di) - } - } else { - // Query without category filter - var query struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - } - if err := client.Query(ctx, &query, vars); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } + discussionQuery := getQueryType(useOrdering, categoryID) + if err := client.Query(ctx, discussionQuery, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } - // Map nodes to GitHub Discussion objects - for _, n := range query.Repository.Discussions.Nodes { - di := &github.Discussion{ - Number: github.Ptr(int(n.Number)), - Title: github.Ptr(string(n.Title)), - HTMLURL: github.Ptr(string(n.URL)), - CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, - DiscussionCategory: &github.DiscussionCategory{ - Name: github.Ptr(string(n.Category.Name)), - }, - } - discussions = append(discussions, di) + // Extract and convert all discussion nodes using the common interface + var discussions []*github.Discussion + var pageInfo PageInfoFragment + var totalCount githubv4.Int + if queryResult, ok := discussionQuery.(DiscussionQueryResult); ok { + fragment := queryResult.GetDiscussionFragment() + for _, node := range fragment.Nodes { + discussions = append(discussions, fragmentToDiscussion(node)) } + pageInfo = fragment.PageInfo + totalCount = fragment.TotalCount } - // Marshal and return - out, err := json.Marshal(discussions) + // Create response with pagination info + response := map[string]interface{}{ + "discussions": discussions, + "pageInfo": map[string]interface{}{ + "hasNextPage": pageInfo.HasNextPage, + "hasPreviousPage": pageInfo.HasPreviousPage, + "startCursor": string(pageInfo.StartCursor), + "endCursor": string(pageInfo.EndCursor), + }, + "totalCount": totalCount, + } + + out, err := json.Marshal(response) if err != nil { return nil, fmt.Errorf("failed to marshal discussions: %w", err) } @@ -236,6 +337,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")), mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")), mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")), + WithCursorPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Decode params @@ -248,6 +350,27 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati return mcp.NewToolResultError(err.Error()), nil } + // Get pagination parameters and convert to GraphQL format + pagination, err := OptionalCursorPaginationParams(request) + if err != nil { + return nil, err + } + + // Check if pagination parameters were explicitly provided + _, perPageProvided := request.GetArguments()["perPage"] + paginationExplicit := perPageProvided + + paginationParams, err := pagination.ToGraphQLParams() + if err != nil { + return nil, err + } + + // Use default of 30 if pagination was not explicitly provided + if !paginationExplicit { + defaultFirst := int32(DefaultGraphQLPageSize) + paginationParams.First = &defaultFirst + } + client, err := getGQLClient(ctx) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil @@ -260,7 +383,14 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati Nodes []struct { Body githubv4.String } - } `graphql:"comments(first:100)"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + TotalCount int + } `graphql:"comments(first: $first, after: $after)"` } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -268,16 +398,35 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati "owner": githubv4.String(params.Owner), "repo": githubv4.String(params.Repo), "discussionNumber": githubv4.Int(params.DiscussionNumber), + "first": githubv4.Int(*paginationParams.First), + } + if paginationParams.After != nil { + vars["after"] = githubv4.String(*paginationParams.After) + } else { + vars["after"] = (*githubv4.String)(nil) } if err := client.Query(ctx, &q, vars); err != nil { return mcp.NewToolResultError(err.Error()), nil } + var comments []*github.IssueComment for _, c := range q.Repository.Discussion.Comments.Nodes { comments = append(comments, &github.IssueComment{Body: github.Ptr(string(c.Body))}) } - out, err := json.Marshal(comments) + // Create response with pagination info + response := map[string]interface{}{ + "comments": comments, + "pageInfo": map[string]interface{}{ + "hasNextPage": q.Repository.Discussion.Comments.PageInfo.HasNextPage, + "hasPreviousPage": q.Repository.Discussion.Comments.PageInfo.HasPreviousPage, + "startCursor": string(q.Repository.Discussion.Comments.PageInfo.StartCursor), + "endCursor": string(q.Repository.Discussion.Comments.PageInfo.EndCursor), + }, + "totalCount": q.Repository.Discussion.Comments.TotalCount, + } + + out, err := json.Marshal(response) if err != nil { return nil, fmt.Errorf("failed to marshal comments: %w", err) } @@ -301,55 +450,22 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl mcp.Required(), mcp.Description("Repository name"), ), - mcp.WithNumber("first", - mcp.Description("Number of categories to return per page (min 1, max 100)"), - mcp.Min(1), - mcp.Max(100), - ), - mcp.WithNumber("last", - mcp.Description("Number of categories to return from the end (min 1, max 100)"), - mcp.Min(1), - mcp.Max(100), - ), - mcp.WithString("after", - mcp.Description("Cursor for pagination, use the 'after' field from the previous response"), - ), - mcp.WithString("before", - mcp.Description("Cursor for pagination, use the 'before' field from the previous response"), - ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Decode params var params struct { - Owner string - Repo string - First int32 - Last int32 - After string - Before string + Owner string + Repo string } if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } - // Validate pagination parameters - if params.First != 0 && params.Last != 0 { - return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil - } - if params.After != "" && params.Before != "" { - return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil - } - if params.After != "" && params.Last != 0 { - return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil - } - if params.Before != "" && params.First != 0 { - return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil - } - client, err := getGQLClient(ctx) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil } + var q struct { Repository struct { DiscussionCategories struct { @@ -357,16 +473,25 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl ID githubv4.ID Name githubv4.String } - } `graphql:"discussionCategories(first: 100)"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + TotalCount int + } `graphql:"discussionCategories(first: $first)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ "owner": githubv4.String(params.Owner), "repo": githubv4.String(params.Repo), + "first": githubv4.Int(25), } if err := client.Query(ctx, &q, vars); err != nil { return mcp.NewToolResultError(err.Error()), nil } + var categories []map[string]string for _, c := range q.Repository.DiscussionCategories.Nodes { categories = append(categories, map[string]string{ @@ -374,7 +499,20 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl "name": string(c.Name), }) } - out, err := json.Marshal(categories) + + // Create response with pagination info + response := map[string]interface{}{ + "categories": categories, + "pageInfo": map[string]interface{}{ + "hasNextPage": q.Repository.DiscussionCategories.PageInfo.HasNextPage, + "hasPreviousPage": q.Repository.DiscussionCategories.PageInfo.HasPreviousPage, + "startCursor": string(q.Repository.DiscussionCategories.PageInfo.StartCursor), + "endCursor": string(q.Repository.DiscussionCategories.PageInfo.EndCursor), + }, + "totalCount": q.Repository.DiscussionCategories.TotalCount, + } + + out, err := json.Marshal(response) if err != nil { return nil, fmt.Errorf("failed to marshal discussion categories: %w", err) } diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index c6688a519..aefaf2f8c 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -17,22 +17,126 @@ import ( var ( discussionsGeneral = []map[string]any{ - {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}}, - {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}}, + {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "updatedAt": "2023-01-01T00:00:00Z", "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}}, + {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "updatedAt": "2023-02-01T00:00:00Z", "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}}, } discussionsAll = []map[string]any{ - {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}}, - {"number": 2, "title": "Discussion 2 title", "createdAt": "2023-02-01T00:00:00Z", "url": "https://github.com/owner/repo/discussions/2", "category": map[string]any{"name": "Questions"}}, - {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}}, + { + "number": 1, + "title": "Discussion 1 title", + "createdAt": "2023-01-01T00:00:00Z", + "updatedAt": "2023-01-01T00:00:00Z", + "author": map[string]any{"login": "user1"}, + "url": "https://github.com/owner/repo/discussions/1", + "category": map[string]any{"name": "General"}, + }, + { + "number": 2, + "title": "Discussion 2 title", + "createdAt": "2023-02-01T00:00:00Z", + "updatedAt": "2023-02-01T00:00:00Z", + "author": map[string]any{"login": "user2"}, + "url": "https://github.com/owner/repo/discussions/2", + "category": map[string]any{"name": "Questions"}, + }, + { + "number": 3, + "title": "Discussion 3 title", + "createdAt": "2023-03-01T00:00:00Z", + "updatedAt": "2023-03-01T00:00:00Z", + "author": map[string]any{"login": "user3"}, + "url": "https://github.com/owner/repo/discussions/3", + "category": map[string]any{"name": "General"}, + }, } + + // Ordered mock responses + discussionsOrderedCreatedAsc = []map[string]any{ + discussionsAll[0], // Discussion 1 (created 2023-01-01) + discussionsAll[1], // Discussion 2 (created 2023-02-01) + discussionsAll[2], // Discussion 3 (created 2023-03-01) + } + + discussionsOrderedUpdatedDesc = []map[string]any{ + discussionsAll[2], // Discussion 3 (updated 2023-03-01) + discussionsAll[1], // Discussion 2 (updated 2023-02-01) + discussionsAll[0], // Discussion 1 (updated 2023-01-01) + } + + // only 'General' category discussions ordered by created date descending + discussionsGeneralOrderedDesc = []map[string]any{ + discussionsGeneral[1], // Discussion 3 (created 2023-03-01) + discussionsGeneral[0], // Discussion 1 (created 2023-01-01) + } + mockResponseListAll = githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ - "discussions": map[string]any{"nodes": discussionsAll}, + "discussions": map[string]any{ + "nodes": discussionsAll, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 3, + }, }, }) mockResponseListGeneral = githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ - "discussions": map[string]any{"nodes": discussionsGeneral}, + "discussions": map[string]any{ + "nodes": discussionsGeneral, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, + }, + }, + }) + mockResponseOrderedCreatedAsc = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{ + "nodes": discussionsOrderedCreatedAsc, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 3, + }, + }, + }) + mockResponseOrderedUpdatedDesc = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{ + "nodes": discussionsOrderedUpdatedDesc, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 3, + }, + }, + }) + mockResponseGeneralOrderedDesc = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{ + "nodes": discussionsGeneralOrderedDesc, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, + }, }, }) mockErrorRepoNotFound = githubv4mock.ErrorResponse("repository not found") @@ -40,62 +144,64 @@ var ( func Test_ListDiscussions(t *testing.T) { mockClient := githubv4.NewClient(nil) - // Verify tool definition and schema toolDef, _ := ListDiscussions(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "list_discussions", toolDef.Name) assert.NotEmpty(t, toolDef.Description) assert.Contains(t, toolDef.InputSchema.Properties, "owner") assert.Contains(t, toolDef.InputSchema.Properties, "repo") + assert.Contains(t, toolDef.InputSchema.Properties, "orderBy") + assert.Contains(t, toolDef.InputSchema.Properties, "direction") assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"}) - // mock for the call to ListDiscussions without category filter - var qDiscussions struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100)"` - } `graphql:"repository(owner: $owner, name: $repo)"` + // Variables matching what GraphQL receives after JSON marshaling/unmarshaling + varsListAll := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "first": float64(30), + "after": (*string)(nil), } - // mock for the call to get discussions with category filter - var qDiscussionsFiltered struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100, categoryId: $categoryId)"` - } `graphql:"repository(owner: $owner, name: $repo)"` + varsRepoNotFound := map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent-repo", + "first": float64(30), + "after": (*string)(nil), } - varsListAll := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), + varsDiscussionsFiltered := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "categoryId": "DIC_kwDOABC123", + "first": float64(30), + "after": (*string)(nil), } - varsRepoNotFound := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("nonexistent-repo"), + varsOrderByCreatedAsc := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "orderByField": "CREATED_AT", + "orderByDirection": "ASC", + "first": float64(30), + "after": (*string)(nil), } - varsDiscussionsFiltered := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "categoryId": githubv4.ID("DIC_kwDOABC123"), + varsOrderByUpdatedDesc := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "orderByField": "UPDATED_AT", + "orderByDirection": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + + varsCategoryWithOrder := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "categoryId": "DIC_kwDOABC123", + "orderByField": "CREATED_AT", + "orderByDirection": "DESC", + "first": float64(30), + "after": (*string)(nil), } tests := []struct { @@ -104,6 +210,7 @@ func Test_ListDiscussions(t *testing.T) { expectError bool errContains string expectedCount int + verifyOrder func(t *testing.T, discussions []*github.Discussion) }{ { name: "list all discussions without category filter", @@ -124,6 +231,80 @@ func Test_ListDiscussions(t *testing.T) { expectError: false, expectedCount: 2, // Only General discussions (matching the category ID) }, + { + name: "order by created at ascending", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "orderBy": "CREATED_AT", + "direction": "ASC", + }, + expectError: false, + expectedCount: 3, + verifyOrder: func(t *testing.T, discussions []*github.Discussion) { + // Verify discussions are ordered by created date ascending + require.Len(t, discussions, 3) + assert.Equal(t, 1, *discussions[0].Number, "First should be discussion 1 (created 2023-01-01)") + assert.Equal(t, 2, *discussions[1].Number, "Second should be discussion 2 (created 2023-02-01)") + assert.Equal(t, 3, *discussions[2].Number, "Third should be discussion 3 (created 2023-03-01)") + }, + }, + { + name: "order by updated at descending", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "orderBy": "UPDATED_AT", + "direction": "DESC", + }, + expectError: false, + expectedCount: 3, + verifyOrder: func(t *testing.T, discussions []*github.Discussion) { + // Verify discussions are ordered by updated date descending + require.Len(t, discussions, 3) + assert.Equal(t, 3, *discussions[0].Number, "First should be discussion 3 (updated 2023-03-01)") + assert.Equal(t, 2, *discussions[1].Number, "Second should be discussion 2 (updated 2023-02-01)") + assert.Equal(t, 1, *discussions[2].Number, "Third should be discussion 1 (updated 2023-01-01)") + }, + }, + { + name: "filter by category with order", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "category": "DIC_kwDOABC123", + "orderBy": "CREATED_AT", + "direction": "DESC", + }, + expectError: false, + expectedCount: 2, + verifyOrder: func(t *testing.T, discussions []*github.Discussion) { + // Verify only General discussions, ordered by created date descending + require.Len(t, discussions, 2) + assert.Equal(t, 3, *discussions[0].Number, "First should be discussion 3 (created 2023-03-01)") + assert.Equal(t, 1, *discussions[1].Number, "Second should be discussion 1 (created 2023-01-01)") + }, + }, + { + name: "order by without direction (should not use ordering)", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "orderBy": "CREATED_AT", + }, + expectError: false, + expectedCount: 3, + }, + { + name: "direction without order by (should not use ordering)", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "direction": "DESC", + }, + expectError: false, + expectedCount: 3, + }, { name: "repository not found error", reqParams: map[string]interface{}{ @@ -135,21 +316,40 @@ func Test_ListDiscussions(t *testing.T) { }, } + // Define the actual query strings that match the implementation + qBasicNoOrder := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithCategoryNoOrder := "query($after:String$categoryId:ID!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qBasicWithOrder := "query($after:String$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithCategoryAndOrder := "query($after:String$categoryId:ID!$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { var httpClient *http.Client switch tc.name { case "list all discussions without category filter": - // Simple case - no category filter - matcher := githubv4mock.NewQueryMatcher(qDiscussions, varsListAll, mockResponseListAll) + matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsListAll, mockResponseListAll) httpClient = githubv4mock.NewMockedHTTPClient(matcher) case "filter by category ID": - // Simple case - category filter using category ID directly - matcher := githubv4mock.NewQueryMatcher(qDiscussionsFiltered, varsDiscussionsFiltered, mockResponseListGeneral) + matcher := githubv4mock.NewQueryMatcher(qWithCategoryNoOrder, varsDiscussionsFiltered, mockResponseListGeneral) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "order by created at ascending": + matcher := githubv4mock.NewQueryMatcher(qBasicWithOrder, varsOrderByCreatedAsc, mockResponseOrderedCreatedAsc) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "order by updated at descending": + matcher := githubv4mock.NewQueryMatcher(qBasicWithOrder, varsOrderByUpdatedDesc, mockResponseOrderedUpdatedDesc) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "filter by category with order": + matcher := githubv4mock.NewQueryMatcher(qWithCategoryAndOrder, varsCategoryWithOrder, mockResponseGeneralOrderedDesc) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "order by without direction (should not use ordering)": + matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsListAll, mockResponseListAll) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "direction without order by (should not use ordering)": + matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsListAll, mockResponseListAll) httpClient = githubv4mock.NewMockedHTTPClient(matcher) case "repository not found error": - matcher := githubv4mock.NewQueryMatcher(qDiscussions, varsRepoNotFound, mockErrorRepoNotFound) + matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsRepoNotFound, mockErrorRepoNotFound) httpClient = githubv4mock.NewMockedHTTPClient(matcher) } @@ -167,15 +367,30 @@ func Test_ListDiscussions(t *testing.T) { } require.NoError(t, err) - var returnedDiscussions []*github.Discussion - err = json.Unmarshal([]byte(text), &returnedDiscussions) + // Parse the structured response with pagination info + var response struct { + Discussions []*github.Discussion `json:"discussions"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + StartCursor string `json:"startCursor"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + TotalCount int `json:"totalCount"` + } + err = json.Unmarshal([]byte(text), &response) require.NoError(t, err) - assert.Len(t, returnedDiscussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(returnedDiscussions)) + assert.Len(t, response.Discussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(response.Discussions)) + + // Verify order if verifyOrder function is provided + if tc.verifyOrder != nil { + tc.verifyOrder(t, response.Discussions) + } // Verify that all returned discussions have a category if filtered if _, hasCategory := tc.reqParams["category"]; hasCategory { - for _, discussion := range returnedDiscussions { + for _, discussion := range response.Discussions { require.NotNil(t, discussion.DiscussionCategory, "Discussion should have category") assert.NotEmpty(t, *discussion.DiscussionCategory.Name, "Discussion should have category name") } @@ -194,23 +409,13 @@ func Test_GetDiscussion(t *testing.T) { assert.Contains(t, toolDef.InputSchema.Properties, "discussionNumber") assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) - var q struct { - Repository struct { - Discussion struct { - Number githubv4.Int - Body githubv4.String - CreatedAt githubv4.DateTime - URL githubv4.String `graphql:"url"` - Category struct { - Name githubv4.String - } `graphql:"category"` - } `graphql:"discussion(number: $discussionNumber)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } + // Use exact string query that matches implementation output + qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,body,createdAt,url,category{name}}}}" + vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "discussionNumber": githubv4.Int(1), + "owner": "owner", + "repo": "repo", + "discussionNumber": float64(1), } tests := []struct { name string @@ -250,7 +455,7 @@ func Test_GetDiscussion(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - matcher := githubv4mock.NewQueryMatcher(q, vars, tc.response) + matcher := githubv4mock.NewQueryMatcher(qGetDiscussion, vars, tc.response) httpClient := githubv4mock.NewMockedHTTPClient(matcher) gqlClient := githubv4.NewClient(httpClient) _, handler := GetDiscussion(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) @@ -287,22 +492,18 @@ func Test_GetDiscussionComments(t *testing.T) { assert.Contains(t, toolDef.InputSchema.Properties, "discussionNumber") assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) - var q struct { - Repository struct { - Discussion struct { - Comments struct { - Nodes []struct { - Body githubv4.String - } - } `graphql:"comments(first:100)"` - } `graphql:"discussion(number: $discussionNumber)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } + // Use exact string query that matches implementation output + qGetComments := "query($after:String$discussionNumber:Int!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){comments(first: $first, after: $after){nodes{body},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}}" + + // Variables matching what GraphQL receives after JSON marshaling/unmarshaling vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "discussionNumber": githubv4.Int(1), + "owner": "owner", + "repo": "repo", + "discussionNumber": float64(1), + "first": float64(30), + "after": (*string)(nil), } + mockResponse := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ "discussion": map[string]any{ @@ -311,11 +512,18 @@ func Test_GetDiscussionComments(t *testing.T) { {"body": "This is the first comment"}, {"body": "This is the second comment"}, }, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, }, }, }, }) - matcher := githubv4mock.NewQueryMatcher(q, vars, mockResponse) + matcher := githubv4mock.NewQueryMatcher(qGetComments, vars, mockResponse) httpClient := githubv4mock.NewMockedHTTPClient(matcher) gqlClient := githubv4.NewClient(httpClient) _, handler := GetDiscussionComments(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) @@ -331,31 +539,38 @@ func Test_GetDiscussionComments(t *testing.T) { textContent := getTextResult(t, result) - var returnedComments []*github.IssueComment - err = json.Unmarshal([]byte(textContent.Text), &returnedComments) + // (Lines removed) + + var response struct { + Comments []*github.IssueComment `json:"comments"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + StartCursor string `json:"startCursor"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + TotalCount int `json:"totalCount"` + } + err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) - assert.Len(t, returnedComments, 2) + assert.Len(t, response.Comments, 2) expectedBodies := []string{"This is the first comment", "This is the second comment"} - for i, comment := range returnedComments { + for i, comment := range response.Comments { assert.Equal(t, expectedBodies[i], *comment.Body) } } func Test_ListDiscussionCategories(t *testing.T) { - var q struct { - Repository struct { - DiscussionCategories struct { - Nodes []struct { - ID githubv4.ID - Name githubv4.String - } - } `graphql:"discussionCategories(first: 100)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } + // Use exact string query that matches implementation output + qListCategories := "query($first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first){nodes{id,name},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + + // Variables matching what GraphQL receives after JSON marshaling/unmarshaling vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), + "owner": "owner", + "repo": "repo", + "first": float64(25), } + mockResp := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ "discussionCategories": map[string]any{ @@ -363,10 +578,17 @@ func Test_ListDiscussionCategories(t *testing.T) { {"id": "123", "name": "CategoryOne"}, {"id": "456", "name": "CategoryTwo"}, }, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, }, }, }) - matcher := githubv4mock.NewQueryMatcher(q, vars, mockResp) + matcher := githubv4mock.NewQueryMatcher(qListCategories, vars, mockResp) httpClient := githubv4mock.NewMockedHTTPClient(matcher) gqlClient := githubv4.NewClient(httpClient) @@ -382,11 +604,21 @@ func Test_ListDiscussionCategories(t *testing.T) { require.NoError(t, err) text := getTextResult(t, result).Text - var categories []map[string]string - require.NoError(t, json.Unmarshal([]byte(text), &categories)) - assert.Len(t, categories, 2) - assert.Equal(t, "123", categories[0]["id"]) - assert.Equal(t, "CategoryOne", categories[0]["name"]) - assert.Equal(t, "456", categories[1]["id"]) - assert.Equal(t, "CategoryTwo", categories[1]["name"]) + + var response struct { + Categories []map[string]string `json:"categories"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + StartCursor string `json:"startCursor"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + TotalCount int `json:"totalCount"` + } + require.NoError(t, json.Unmarshal([]byte(text), &response)) + assert.Len(t, response.Categories, 2) + assert.Equal(t, "123", response.Categories[0]["id"]) + assert.Equal(t, "CategoryOne", response.Categories[0]["name"]) + assert.Equal(t, "456", response.Categories[1]["id"]) + assert.Equal(t, "CategoryTwo", response.Categories[1]["name"]) } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 29d32bd18..f718c37cb 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -9,6 +9,7 @@ import ( "strings" "time" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" "github.com/google/go-github/v73/github" @@ -153,6 +154,408 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc } } +// AddSubIssue creates a tool to add a sub-issue to a parent issue. +func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("add_sub_issue", + mcp.WithDescription(t("TOOL_ADD_SUB_ISSUE_DESCRIPTION", "Add a sub-issue to a parent issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_ADD_SUB_ISSUE_USER_TITLE", "Add sub-issue"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("issue_number", + mcp.Required(), + mcp.Description("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to add. ID is not the same as issue number"), + ), + mcp.WithBoolean("replace_parent", + mcp.Description("When true, replaces the sub-issue's current parent issue"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := RequiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + replaceParent, err := OptionalParam[bool](request, "replace_parent") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + subIssueRequest := github.SubIssueRequest{ + SubIssueID: int64(subIssueID), + ReplaceParent: ToBoolPtr(replaceParent), + } + + subIssue, resp, err := client.SubIssue.Add(ctx, owner, repo, int64(issueNumber), subIssueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to add sub-issue", + resp, + err, + ), nil + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to add sub-issue: %s", string(body))), nil + } + + r, err := json.Marshal(subIssue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// ListSubIssues creates a tool to list sub-issues for a GitHub issue. +func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_sub_issues", + mcp.WithDescription(t("TOOL_LIST_SUB_ISSUES_DESCRIPTION", "List sub-issues for a specific issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_SUB_ISSUES_USER_TITLE", "List sub-issues"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("issue_number", + mcp.Required(), + mcp.Description("Issue number"), + ), + mcp.WithNumber("page", + mcp.Description("Page number for pagination (default: 1)"), + ), + mcp.WithNumber("per_page", + mcp.Description("Number of results per page (max 100, default: 30)"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := RequiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + page, err := OptionalIntParamWithDefault(request, "page", 1) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + perPage, err := OptionalIntParamWithDefault(request, "per_page", 30) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + opts := &github.IssueListOptions{ + ListOptions: github.ListOptions{ + Page: page, + PerPage: perPage, + }, + } + + subIssues, resp, err := client.SubIssue.ListByIssue(ctx, owner, repo, int64(issueNumber), opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to list sub-issues", + resp, + err, + ), nil + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil + } + + r, err := json.Marshal(subIssues) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } + +} + +// RemoveSubIssue creates a tool to remove a sub-issue from a parent issue. +// Unlike other sub-issue tools, this currently uses a direct HTTP DELETE request +// because of a bug in the go-github library. +// Once the fix is released, this can be updated to use the library method. +// See: https://github.com/google/go-github/pull/3613 +func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("remove_sub_issue", + mcp.WithDescription(t("TOOL_REMOVE_SUB_ISSUE_DESCRIPTION", "Remove a sub-issue from a parent issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_REMOVE_SUB_ISSUE_USER_TITLE", "Remove sub-issue"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("issue_number", + mcp.Required(), + mcp.Description("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to remove. ID is not the same as issue number"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := RequiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Create the request body + requestBody := map[string]interface{}{ + "sub_issue_id": subIssueID, + } + reqBodyBytes, err := json.Marshal(requestBody) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + // Create the HTTP request + url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issue", + client.BaseURL.String(), owner, repo, issueNumber) + req, err := http.NewRequestWithContext(ctx, "DELETE", url, strings.NewReader(string(reqBodyBytes))) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + httpClient := client.Client() // Use authenticated GitHub client + resp, err := httpClient.Do(req) + if err != nil { + var ghResp *github.Response + if resp != nil { + ghResp = &github.Response{Response: resp} + } + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to remove sub-issue", + ghResp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return mcp.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil + } + + // Parse and re-marshal to ensure consistent formatting + var result interface{} + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// ReprioritizeSubIssue creates a tool to reprioritize a sub-issue to a different position in the parent list. +func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("reprioritize_sub_issue", + mcp.WithDescription(t("TOOL_REPRIORITIZE_SUB_ISSUE_DESCRIPTION", "Reprioritize a sub-issue to a different position in the parent issue's sub-issue list.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_REPRIORITIZE_SUB_ISSUE_USER_TITLE", "Reprioritize sub-issue"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("issue_number", + mcp.Required(), + mcp.Description("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to reprioritize. ID is not the same as issue number"), + ), + mcp.WithNumber("after_id", + mcp.Description("The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified)"), + ), + mcp.WithNumber("before_id", + mcp.Description("The ID of the sub-issue to be prioritized before (either after_id OR before_id should be specified)"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := RequiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Handle optional positioning parameters + afterID, err := OptionalIntParam(request, "after_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + beforeID, err := OptionalIntParam(request, "before_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Validate that either after_id or before_id is specified, but not both + if afterID == 0 && beforeID == 0 { + return mcp.NewToolResultError("either after_id or before_id must be specified"), nil + } + if afterID != 0 && beforeID != 0 { + return mcp.NewToolResultError("only one of after_id or before_id should be specified, not both"), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + subIssueRequest := github.SubIssueRequest{ + SubIssueID: int64(subIssueID), + } + + if afterID != 0 { + afterIDInt64 := int64(afterID) + subIssueRequest.AfterID = &afterIDInt64 + } + if beforeID != 0 { + beforeIDInt64 := int64(beforeID) + subIssueRequest.BeforeID = &beforeIDInt64 + } + + subIssue, resp, err := client.SubIssue.Reprioritize(ctx, owner, repo, int64(issueNumber), subIssueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to reprioritize sub-issue", + resp, + err, + ), nil + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to reprioritize sub-issue: %s", string(body))), nil + } + + r, err := json.Marshal(subIssue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", @@ -630,8 +1033,8 @@ func GetIssueComments(getClient GetClientFn, t translations.TranslationHelperFun opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, }, } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 146259477..2bdb89b06 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -38,6 +38,9 @@ func Test_GetIssue(t *testing.T) { Body: github.Ptr("This is a test issue"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, } tests := []struct { @@ -111,6 +114,9 @@ func Test_GetIssue(t *testing.T) { assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) }) } } @@ -1629,3 +1635,969 @@ func TestAssignCopilotToIssue(t *testing.T) { }) } } + +func Test_AddSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := AddSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "add_sub_issue", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "issue_number") + assert.Contains(t, tool.InputSchema.Properties, "sub_issue_id") + assert.Contains(t, tool.InputSchema.Properties, "replace_parent") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue with a sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issue addition with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "replace_parent": true, + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful sub-issue addition with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(456), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful sub-issue addition with replace_parent false", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(789), + "replace_parent": false, + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Parent issue not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Sub-issue not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "validation failed - sub-issue cannot be parent of itself", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusUnprocessableEntity, `{"message": "Validation failed", "errors": [{"message": "Sub-issue cannot be a parent of itself"}]}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusForbidden, `{"message": "Must have write access to repository"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := AddSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssue github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) + assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) + assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} + +func Test_ListSubIssues(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListSubIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "list_sub_issues", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "issue_number") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "per_page") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number"}) + + // Setup mock sub-issues for success case + mockSubIssues := []*github.Issue{ + { + Number: github.Ptr(123), + Title: github.Ptr("Sub-issue 1"), + Body: github.Ptr("This is the first sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + User: &github.User{ + Login: github.Ptr("user1"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("bug"), + Color: github.Ptr("d73a4a"), + Description: github.Ptr("Something isn't working"), + }, + }, + }, + { + Number: github.Ptr(124), + Title: github.Ptr("Sub-issue 2"), + Body: github.Ptr("This is the second sub-issue"), + State: github.Ptr("closed"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/124"), + User: &github.User{ + Login: github.Ptr("user2"), + }, + Assignees: []*github.User{ + {Login: github.Ptr("assignee1")}, + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedSubIssues []*github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issues listing with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockSubIssues, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedSubIssues: mockSubIssues, + }, + { + name: "successful sub-issues listing with pagination", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + expectQueryParams(t, map[string]string{ + "page": "2", + "per_page": "10", + }).andThen( + mockResponse(t, http.StatusOK, mockSubIssues), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "page": float64(2), + "per_page": float64(10), + }, + expectError: false, + expectedSubIssues: mockSubIssues, + }, + { + name: "successful sub-issues listing with empty result", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + []*github.Issue{}, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedSubIssues: []*github.Issue{}, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "nonexistent", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "sub-issues feature gone/deprecated", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusGone, `{"message": "This feature has been deprecated"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter issue_number", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedErrMsg: "missing required parameter: issue_number", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ListSubIssues(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedSubIssues []*github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedSubIssues) + require.NoError(t, err) + + assert.Len(t, returnedSubIssues, len(tc.expectedSubIssues)) + for i, subIssue := range returnedSubIssues { + if i < len(tc.expectedSubIssues) { + assert.Equal(t, *tc.expectedSubIssues[i].Number, *subIssue.Number) + assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title) + assert.Equal(t, *tc.expectedSubIssues[i].State, *subIssue.State) + assert.Equal(t, *tc.expectedSubIssues[i].HTMLURL, *subIssue.HTMLURL) + assert.Equal(t, *tc.expectedSubIssues[i].User.Login, *subIssue.User.Login) + + if tc.expectedSubIssues[i].Body != nil { + assert.Equal(t, *tc.expectedSubIssues[i].Body, *subIssue.Body) + } + } + } + }) + } +} + +func Test_RemoveSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := RemoveSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "remove_sub_issue", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "issue_number") + assert.Contains(t, tool.InputSchema.Properties, "sub_issue_id") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue after sub-issue removal"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issue removal", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Sub-issue not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "bad request - invalid sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusBadRequest, `{"message": "Invalid sub_issue_id"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(-1), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "nonexistent", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusForbidden, `{"message": "Must have write access to repository"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := RemoveSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssue github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) + assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) + assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} + +func Test_ReprioritizeSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ReprioritizeSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "reprioritize_sub_issue", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "issue_number") + assert.Contains(t, tool.InputSchema.Properties, "sub_issue_id") + assert.Contains(t, tool.InputSchema.Properties, "after_id") + assert.Contains(t, tool.InputSchema.Properties, "before_id") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue with reprioritized sub-issues"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful reprioritization with after_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful reprioritization with before_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "before_id": float64(789), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "validation error - neither after_id nor before_id specified", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "either after_id or before_id must be specified", + }, + { + name: "validation error - both after_id and before_id specified", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + "before_id": float64(789), + }, + expectError: false, + expectedErrMsg: "only one of after_id or before_id should be specified, not both", + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Sub-issue not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "validation failed - positioning sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusUnprocessableEntity, `{"message": "Validation failed", "errors": [{"message": "Positioning sub-issue not found"}]}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusForbidden, `{"message": "Must have write access to repository"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "service unavailable", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesSubIssuesPriorityByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusServiceUnavailable, `{"message": "Service Unavailable"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "before_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ReprioritizeSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssue github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) + assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) + assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index a41edaf42..fdd418098 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -88,8 +88,8 @@ func ListNotifications(getClient GetClientFn, t translations.TranslationHelperFu All: filter == FilterIncludeRead, Participating: filter == FilterOnlyParticipating, ListOptions: github.ListOptions{ - Page: paginationParams.page, - PerPage: paginationParams.perPage, + Page: paginationParams.Page, + PerPage: paginationParams.PerPage, }, } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index d98dc334d..47b7c6bd2 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -403,8 +403,8 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun Sort: sort, Direction: direction, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, }, } @@ -622,8 +622,8 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper return nil, fmt.Errorf("failed to get GitHub client: %w", err) } opts := &github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, } files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) if err != nil { diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 2e56c8644..ecd36d7e0 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -58,8 +58,8 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too } opts := &github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, } client, err := getClient(ctx) @@ -139,7 +139,7 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(err.Error()), nil } // Set default perPage to 30 if not provided - perPage := pagination.perPage + perPage := pagination.PerPage if perPage == 0 { perPage = 30 } @@ -147,7 +147,7 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t SHA: sha, Author: author, ListOptions: github.ListOptions{ - Page: pagination.page, + Page: pagination.Page, PerPage: perPage, }, } @@ -217,8 +217,8 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( opts := &github.BranchListOptions{ ListOptions: github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, }, } @@ -1198,8 +1198,8 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool } opts := &github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, } client, err := getClient(ctx) diff --git a/pkg/github/search.go b/pkg/github/search.go index 04a1facc0..476ac0151 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -39,8 +39,8 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF opts := &github.SearchOptions{ ListOptions: github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, }, } @@ -83,7 +83,7 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to Title: t("TOOL_SEARCH_CODE_USER_TITLE", "Search code"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("q", + mcp.WithString("query", mcp.Required(), mcp.Description("Search query using GitHub code search syntax"), ), @@ -97,7 +97,7 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - query, err := RequiredParam[string](request, "q") + query, err := RequiredParam[string](request, "query") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -118,8 +118,8 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to Sort: sort, Order: order, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, }, } @@ -193,8 +193,8 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand Sort: sort, Order: order, ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, + PerPage: pagination.PerPage, + Page: pagination.Page, }, } diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 21f7a0ca2..9ea8e71ec 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -173,12 +173,12 @@ func Test_SearchCode(t *testing.T) { assert.Equal(t, "search_code", tool.Name) assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "q") + assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.Contains(t, tool.InputSchema.Properties, "page") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"q"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results mockSearchResult := &github.CodeSearchResult{ @@ -227,7 +227,7 @@ func Test_SearchCode(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "fmt.Println language:go", + "query": "fmt.Println language:go", "sort": "indexed", "order": "desc", "page": float64(1), @@ -251,7 +251,7 @@ func Test_SearchCode(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "fmt.Println language:go", + "query": "fmt.Println language:go", }, expectError: false, expectedResult: mockSearchResult, @@ -268,7 +268,7 @@ func Test_SearchCode(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "invalid:query", + "query": "invalid:query", }, expectError: true, expectedErrMsg: "failed to search code", diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 5dd48040e..a6ff1f782 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -56,8 +56,8 @@ func searchHandler( Sort: sort, Order: order, ListOptions: github.ListOptions{ - Page: pagination.page, - PerPage: pagination.perPage, + Page: pagination.Page, + PerPage: pagination.PerPage, }, } diff --git a/pkg/github/server.go b/pkg/github/server.go index ea476e3ac..193336b75 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -174,9 +174,7 @@ func OptionalStringArrayParam(r mcp.CallToolRequest, p string) ([]string, error) } } -// WithPagination returns a ToolOption that adds "page" and "perPage" parameters to the tool. -// The "page" parameter is optional, min 1. -// The "perPage" parameter is optional, min 1, max 100. If unset, defaults to 30. +// WithPagination adds REST API pagination parameters to a tool. // https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api func WithPagination() mcp.ToolOption { return func(tool *mcp.Tool) { @@ -193,12 +191,49 @@ func WithPagination() mcp.ToolOption { } } +// WithUnifiedPagination adds REST API pagination parameters to a tool. +// GraphQL tools will use this and convert page/perPage to GraphQL cursor parameters internally. +func WithUnifiedPagination() mcp.ToolOption { + return func(tool *mcp.Tool) { + mcp.WithNumber("page", + mcp.Description("Page number for pagination (min 1)"), + mcp.Min(1), + )(tool) + + mcp.WithNumber("perPage", + mcp.Description("Results per page for pagination (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + )(tool) + + mcp.WithString("after", + mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."), + )(tool) + } +} + +// WithCursorPagination adds only cursor-based pagination parameters to a tool (no page parameter). +func WithCursorPagination() mcp.ToolOption { + return func(tool *mcp.Tool) { + mcp.WithNumber("perPage", + mcp.Description("Results per page for pagination (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + )(tool) + + mcp.WithString("after", + mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."), + )(tool) + } +} + type PaginationParams struct { - page int - perPage int + Page int + PerPage int + After string } -// OptionalPaginationParams returns the "page" and "perPage" parameters from the request, +// OptionalPaginationParams returns the "page", "perPage", and "after" parameters from the request, // or their default values if not present, "page" default is 1, "perPage" default is 30. // In future, we may want to make the default values configurable, or even have this // function returned from `withPagination`, where the defaults are provided alongside @@ -212,12 +247,77 @@ func OptionalPaginationParams(r mcp.CallToolRequest) (PaginationParams, error) { if err != nil { return PaginationParams{}, err } + after, err := OptionalParam[string](r, "after") + if err != nil { + return PaginationParams{}, err + } return PaginationParams{ - page: page, - perPage: perPage, + Page: page, + PerPage: perPage, + After: after, + }, nil +} + +// OptionalCursorPaginationParams returns the "perPage" and "after" parameters from the request, +// without the "page" parameter, suitable for cursor-based pagination only. +func OptionalCursorPaginationParams(r mcp.CallToolRequest) (CursorPaginationParams, error) { + perPage, err := OptionalIntParamWithDefault(r, "perPage", 30) + if err != nil { + return CursorPaginationParams{}, err + } + after, err := OptionalParam[string](r, "after") + if err != nil { + return CursorPaginationParams{}, err + } + return CursorPaginationParams{ + PerPage: perPage, + After: after, + }, nil +} + +type CursorPaginationParams struct { + PerPage int + After string +} + +// ToGraphQLParams converts cursor pagination parameters to GraphQL-specific parameters. +func (p CursorPaginationParams) ToGraphQLParams() (*GraphQLPaginationParams, error) { + if p.PerPage > 100 { + return nil, fmt.Errorf("perPage value %d exceeds maximum of 100", p.PerPage) + } + if p.PerPage < 0 { + return nil, fmt.Errorf("perPage value %d cannot be negative", p.PerPage) + } + first := int32(p.PerPage) + + var after *string + if p.After != "" { + after = &p.After + } + + return &GraphQLPaginationParams{ + First: &first, + After: after, }, nil } +type GraphQLPaginationParams struct { + First *int32 + After *string +} + +// ToGraphQLParams converts REST API pagination parameters to GraphQL-specific parameters. +// This converts page/perPage to first parameter for GraphQL queries. +// If After is provided, it takes precedence over page-based pagination. +func (p PaginationParams) ToGraphQLParams() (*GraphQLPaginationParams, error) { + // Convert to CursorPaginationParams and delegate to avoid duplication + cursor := CursorPaginationParams{ + PerPage: p.PerPage, + After: p.After, + } + return cursor.ToGraphQLParams() +} + func MarshalledTextResult(v any) *mcp.CallToolResult { data, err := json.Marshal(v) if err != nil { diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 6353f254d..7f8f29c0d 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -489,8 +489,8 @@ func TestOptionalPaginationParams(t *testing.T) { name: "no pagination parameters, default values", params: map[string]any{}, expected: PaginationParams{ - page: 1, - perPage: 30, + Page: 1, + PerPage: 30, }, expectError: false, }, @@ -500,8 +500,8 @@ func TestOptionalPaginationParams(t *testing.T) { "page": float64(2), }, expected: PaginationParams{ - page: 2, - perPage: 30, + Page: 2, + PerPage: 30, }, expectError: false, }, @@ -511,8 +511,8 @@ func TestOptionalPaginationParams(t *testing.T) { "perPage": float64(50), }, expected: PaginationParams{ - page: 1, - perPage: 50, + Page: 1, + PerPage: 50, }, expectError: false, }, @@ -523,8 +523,8 @@ func TestOptionalPaginationParams(t *testing.T) { "perPage": float64(50), }, expected: PaginationParams{ - page: 2, - perPage: 50, + Page: 2, + PerPage: 50, }, expectError: false, }, diff --git a/pkg/github/tools.go b/pkg/github/tools.go index bd349171d..e01b7cc40 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -53,12 +53,16 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(SearchIssues(getClient, t)), toolsets.NewServerTool(ListIssues(getClient, t)), toolsets.NewServerTool(GetIssueComments(getClient, t)), + toolsets.NewServerTool(ListSubIssues(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(CreateIssue(getClient, t)), toolsets.NewServerTool(AddIssueComment(getClient, t)), toolsets.NewServerTool(UpdateIssue(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), + toolsets.NewServerTool(AddSubIssue(getClient, t)), + toolsets.NewServerTool(RemoveSubIssue(getClient, t)), + toolsets.NewServerTool(ReprioritizeSubIssue(getClient, t)), ).AddPrompts(toolsets.NewServerPrompt(AssignCodingAgentPrompt(t))) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools(