diff --git a/.github/workflows/mcp-diff.yml b/.github/workflows/mcp-diff.yml index 305428923..f901e31f8 100644 --- a/.github/workflows/mcp-diff.yml +++ b/.github/workflows/mcp-diff.yml @@ -27,6 +27,16 @@ jobs: - name: Build UI uses: ./.github/actions/build-ui + - name: Stash UI artifacts for baseline checkout + # mcp-server-diff checks the baseline ref out into a separate working + # directory and runs install_command there. Without these prebuilt + # artifacts, pkg/github/ui_dist/ would be empty on the baseline side + # and UIAssetsAvailable() would return false, producing a false-positive + # diff that "adds" _meta.ui to MCP Apps tools on every PR. + run: | + mkdir -p "${RUNNER_TEMP}/ui_dist" + cp pkg/github/ui_dist/*.html "${RUNNER_TEMP}/ui_dist/" + - name: Generate diff configurations id: configs # The generator imports pkg/github so any new entry in @@ -43,7 +53,10 @@ jobs: uses: SamMorrowDrums/mcp-server-diff@v2.3.5 with: setup_go: "false" - install_command: go mod download + install_command: | + go mod download + mkdir -p pkg/github/ui_dist + cp "${RUNNER_TEMP}"/ui_dist/*.html pkg/github/ui_dist/ start_command: go run ./cmd/github-mcp-server stdio env_vars: | GITHUB_PERSONAL_ACCESS_TOKEN=test-token @@ -79,6 +92,13 @@ jobs: - name: Build UI uses: ./.github/actions/build-ui + - name: Stash UI artifacts for baseline checkout + # See the stdio job above for rationale: the action's baseline checkout + # has no UI artifacts unless we hand them over via RUNNER_TEMP. + run: | + mkdir -p "${RUNNER_TEMP}/ui_dist" + cp pkg/github/ui_dist/*.html "${RUNNER_TEMP}/ui_dist/" + - name: Generate diff configurations id: configs # See script/print-mcp-diff-configs/main.go. The http-headers variant @@ -97,7 +117,10 @@ jobs: uses: SamMorrowDrums/mcp-server-diff@v2.3.5 with: setup_go: "false" - install_command: go mod download + install_command: | + go mod download + mkdir -p pkg/github/ui_dist + cp "${RUNNER_TEMP}"/ui_dist/*.html pkg/github/ui_dist/ http_start_command: go run ./cmd/github-mcp-server http --port 8082 http_startup_wait_ms: "5000" configurations: ${{ steps.configs.outputs.configurations }} diff --git a/.gitignore b/.gitignore index 8d5d8b7ea..dc0a5f3a3 100644 --- a/.gitignore +++ b/.gitignore @@ -17,9 +17,9 @@ bin/ .DS_Store # binary -github-mcp-server -mcpcurl -e2e.test +/github-mcp-server +/mcpcurl +/e2e.test .history conformance-report/ diff --git a/README.md b/README.md index 6d2964965..55df81033 100644 --- a/README.md +++ b/README.md @@ -829,21 +829,6 @@ The following sets of tools are available: - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) -- **add_sub_issue** - Add Sub-Issue - - **Required OAuth Scopes**: `repo` - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `replace_parent`: If true, reparent the sub-issue if it already has a parent (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) - -- **create_issue** - Create Issue - - **Required OAuth Scopes**: `repo` - - `body`: Issue body content (optional) (string, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `title`: Issue title (string, required) - - **get_label** - Get a specific label from a repository - **Required OAuth Scopes**: `repo` - `name`: Label name. (string, required) @@ -870,6 +855,7 @@ The following sets of tools are available: - `assignees`: Usernames to assign to this issue (string[], optional) - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) + - `issue_fields`: Issue field values to set. Each item requires field_name and either value or field_option_name. field_option_name is for single-select fields and is resolved to the corresponding option ID automatically. (object[], optional) - `issue_number`: Issue number to update (number, optional) - `labels`: Labels to apply to this issue (string[], optional) - `method`: Write operation to perform on a single issue. @@ -885,12 +871,6 @@ The following sets of tools are available: - `title`: Issue title (string, optional) - `type`: Type of this issue. Only use if the repository has issue types configured. Use list_issue_types tool to get valid type values for the organization. If the repository doesn't support issue types, omit this parameter. (string, optional) -- **list_issue_fields** - List issue fields - - **Required OAuth Scopes**: `repo`, `read:org` - - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` - - `owner`: The account owner of the repository or organization. The name is not case sensitive. (string, required) - - `repo`: The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly. (string, optional) - - **list_issue_types** - List available issue types - **Required OAuth Scopes**: `read:org` - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` @@ -900,7 +880,6 @@ The following sets of tools are available: - **Required OAuth Scopes**: `repo` - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) - - `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional) - `labels`: Filter by labels (string[], optional) - `orderBy`: Order issues by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) @@ -909,22 +888,6 @@ The following sets of tools are available: - `since`: Filter by date (ISO 8601 timestamp) (string, optional) - `state`: Filter by state, by default both open and closed issues are returned when not provided (string, optional) -- **remove_sub_issue** - Remove Sub-Issue - - **Required OAuth Scopes**: `repo` - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (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 - - **Required OAuth Scopes**: `repo` - - `after_id`: The ID of the sub-issue to place this after (either after_id OR before_id should be specified) (number, optional) - - `before_id`: The ID of the sub-issue to place this before (either after_id OR before_id should be specified) (number, optional) - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `sub_issue_id`: The ID of the sub-issue to reorder. ID is not the same as issue number (number, required) - - **search_issues** - Search issues - **Required OAuth Scopes**: `repo` - `order`: Sort order (string, optional) @@ -935,13 +898,6 @@ The following sets of tools are available: - `repo`: Optional repository name. If provided with owner, only issues for this repository are listed. (string, optional) - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) -- **set_issue_fields** - Set Issue Fields - - **Required OAuth Scopes**: `repo` - - `fields`: Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value. (object[], required) - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - **sub_issue_write** - Change sub-issue - **Required OAuth Scopes**: `repo` - `after_id`: The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified) (number, optional) @@ -958,57 +914,6 @@ The following sets of tools are available: - `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) -- **update_issue_assignees** - Update Issue Assignees - - **Required OAuth Scopes**: `repo` - - `assignees`: GitHub usernames to assign to this issue (string[], required) - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_body** - Update Issue Body - - **Required OAuth Scopes**: `repo` - - `body`: The new body content for the issue (string, required) - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_labels** - Update Issue Labels - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `labels`: Labels to apply to this issue. ([], required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_milestone** - Update Issue Milestone - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `milestone`: The milestone number to set on the issue (integer, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_state** - Update Issue State - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `state`: The new state for the issue (string, required) - - `state_reason`: The reason for the state change (only for closed state) (string, optional) - -- **update_issue_title** - Update Issue Title - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `title`: The new title for the issue (string, required) - -- **update_issue_type** - Update Issue Type - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `issue_type`: The issue type to set (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this type. State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature). (string, optional) - - `repo`: Repository name (string, required) -
@@ -1161,19 +1066,6 @@ The following sets of tools are available: - `startSide`: For multi-line comments, the starting side of the diff that the comment applies to. LEFT indicates the previous state, RIGHT indicates the new state (string, optional) - `subjectType`: The level at which the comment is targeted (string, required) -- **add_pull_request_review_comment** - Add Pull Request Review Comment - - **Required OAuth Scopes**: `repo` - - `body`: The comment body (string, required) - - `line`: The line number in the diff to comment on (optional) (number, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `path`: The relative path of the file to comment on (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `side`: The side of the diff to comment on (optional) (string, optional) - - `startLine`: The start line of a multi-line comment (optional) (number, optional) - - `startSide`: The start side of a multi-line comment (optional) (string, optional) - - `subjectType`: The subject type of the comment (string, required) - - **add_reply_to_pull_request_comment** - Add reply to pull request comment - **Required OAuth Scopes**: `repo` - `body`: The text of the reply (string, required) @@ -1193,21 +1085,6 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - `title`: PR title (string, required) -- **create_pull_request_review** - Create Pull Request Review - - **Required OAuth Scopes**: `repo` - - `body`: The review body text (optional) (string, optional) - - `commitID`: The SHA of the commit to review (optional, defaults to latest) (string, optional) - - `event`: The review action to perform. If omitted, creates a pending review. (string, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **delete_pending_pull_request_review** - Delete Pending Pull Request Review - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - **list_pull_requests** - List pull requests - **Required OAuth Scopes**: `repo` - `base`: Filter by base branch (string, optional) @@ -1260,17 +1137,6 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional) -- **request_pull_request_reviewers** - Request Pull Request Reviewers - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames to request reviews from (string[], required) - -- **resolve_review_thread** - Resolve Review Thread - - **Required OAuth Scopes**: `repo` - - `threadID`: The node ID of the review thread to resolve (e.g., PRRT_kwDOxxx) (string, required) - - **search_pull_requests** - Search pull requests - **Required OAuth Scopes**: `repo` - `order`: Sort order (string, optional) @@ -1281,18 +1147,6 @@ The following sets of tools are available: - `repo`: Optional repository name. If provided with owner, only pull requests for this repository are listed. (string, optional) - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) -- **submit_pending_pull_request_review** - Submit Pending Pull Request Review - - **Required OAuth Scopes**: `repo` - - `body`: The review body text (optional) (string, optional) - - `event`: The review action to perform (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **unresolve_review_thread** - Unresolve Review Thread - - **Required OAuth Scopes**: `repo` - - `threadID`: The node ID of the review thread to unresolve (e.g., PRRT_kwDOxxx) (string, required) - - **update_pull_request** - Edit pull request - **Required OAuth Scopes**: `repo` - `base`: New base branch name (string, optional) @@ -1306,13 +1160,6 @@ The following sets of tools are available: - `state`: New state (string, optional) - `title`: New title (string, optional) -- **update_pull_request_body** - Update Pull Request Body - - **Required OAuth Scopes**: `repo` - - `body`: The new body content for the pull request (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - **update_pull_request_branch** - Update pull request branch - **Required OAuth Scopes**: `repo` - `expectedHeadSha`: The expected SHA of the pull request's HEAD ref (string, optional) @@ -1320,27 +1167,6 @@ The following sets of tools are available: - `pullNumber`: Pull request number (number, required) - `repo`: Repository name (string, required) -- **update_pull_request_draft_state** - Update Pull Request Draft State - - **Required OAuth Scopes**: `repo` - - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **update_pull_request_state** - Update Pull Request State - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `state`: The new state for the pull request (string, required) - -- **update_pull_request_title** - Update Pull Request Title - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `title`: The new title for the pull request (string, required) -
diff --git a/cmd/github-mcp-server/feature_flag_docs.go b/cmd/github-mcp-server/feature_flag_docs.go new file mode 100644 index 000000000..e52237b13 --- /dev/null +++ b/cmd/github-mcp-server/feature_flag_docs.go @@ -0,0 +1,139 @@ +package main + +import ( + "context" + "fmt" + "os" + "reflect" + "sort" + "strings" + + "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" +) + +// generateInsidersFeaturesDocs refreshes the auto-generated section of +// docs/insiders-features.md with the tools and schemas affected by each +// Insiders feature flag. +func generateInsidersFeaturesDocs(docsPath string) error { + body := generateFlaggedToolsDoc(github.InsidersFeatureFlags, "_No Insiders-only tool changes._") + return rewriteAutomatedSection(docsPath, "START AUTOMATED INSIDERS TOOLS", "END AUTOMATED INSIDERS TOOLS", body) +} + +// generateFeatureFlagsDocs refreshes the auto-generated section of +// docs/feature-flags.md with the tools and schemas affected by each +// user-controllable feature flag. +func generateFeatureFlagsDocs(docsPath string) error { + body := generateFlaggedToolsDoc(github.AllowedFeatureFlags, "_No user-controllable feature flags affect tool registration._") + return rewriteAutomatedSection(docsPath, "START AUTOMATED FEATURE FLAG TOOLS", "END AUTOMATED FEATURE FLAG TOOLS", body) +} + +// generateFlaggedToolsDoc renders, for each flag in the input set, the tools +// whose registration or definition differs from the default user experience. +// Each affected tool is printed with its full schema using the same writer +// used by the README so the output style stays consistent. +func generateFlaggedToolsDoc(flags []string, emptyMessage string) string { + t, _ := translations.TranslationHelper() + defaultTools := indexToolsByName(buildInventoryWithFlags(t, nil).ToolsForRegistration(context.Background())) + + var buf strings.Builder + hasAny := false + + for _, flag := range flags { + affected := flaggedToolDiff(t, flag, defaultTools) + if len(affected) == 0 { + continue + } + + if hasAny { + buf.WriteString("\n\n") + } + hasAny = true + + fmt.Fprintf(&buf, "### `%s`\n\n", flag) + for i, tool := range affected { + writeToolDoc(&buf, tool) + if i < len(affected)-1 { + buf.WriteString("\n\n") + } + } + } + + if !hasAny { + return emptyMessage + } + // Leading/trailing newlines around the body produce blank lines between + // our content and the surrounding marker comments, so the trailing comment + // doesn't get absorbed into the final list item by markdown renderers. + return "\n" + strings.TrimSuffix(buf.String(), "\n") + "\n" +} + +// flaggedToolDiff returns the tools whose definition (input schema or meta) +// differs from the default-flagged inventory when only the given flag is on, +// plus tools that exist only in the flag-on inventory. Results are sorted by +// tool name. +func flaggedToolDiff(t translations.TranslationHelperFunc, flag string, defaultTools map[string]inventory.ServerTool) []inventory.ServerTool { + flagTools := buildInventoryWithFlags(t, map[string]bool{flag: true}).ToolsForRegistration(context.Background()) + + out := make([]inventory.ServerTool, 0) + seen := make(map[string]struct{}, len(flagTools)) + + for _, tool := range flagTools { + if _, ok := seen[tool.Tool.Name]; ok { + continue + } + seen[tool.Tool.Name] = struct{}{} + + baseline, hadBaseline := defaultTools[tool.Tool.Name] + if hadBaseline && reflect.DeepEqual(tool.Tool.InputSchema, baseline.Tool.InputSchema) && reflect.DeepEqual(tool.Tool.Meta, baseline.Tool.Meta) { + continue + } + out = append(out, tool) + } + + sort.Slice(out, func(i, j int) bool { return out[i].Tool.Name < out[j].Tool.Name }) + return out +} + +// buildInventoryWithFlags constructs an inventory whose feature checker treats +// the given flags as enabled and every other flag as disabled. Passing nil +// produces the default-flagged inventory. +func buildInventoryWithFlags(t translations.TranslationHelperFunc, enabled map[string]bool) *inventory.Inventory { + checker := func(_ context.Context, flag string) (bool, error) { + return enabled[flag], nil + } + inv, _ := github.NewInventory(t). + WithToolsets([]string{"all"}). + WithFeatureChecker(checker). + Build() + return inv +} + +// indexToolsByName returns a map keyed by tool name. When duplicates exist +// (e.g. flag-gated dual registrations), the first occurrence wins, mirroring +// AvailableTools' deterministic sort order. +func indexToolsByName(tools []inventory.ServerTool) map[string]inventory.ServerTool { + out := make(map[string]inventory.ServerTool, len(tools)) + for _, tool := range tools { + if _, ok := out[tool.Tool.Name]; ok { + continue + } + out[tool.Tool.Name] = tool + } + return out +} + +// rewriteAutomatedSection reads a markdown file, replaces the content between +// the named markers with body, and writes it back. +func rewriteAutomatedSection(path, startMarker, endMarker, body string) error { + content, err := os.ReadFile(path) //#nosec G304 + if err != nil { + return fmt.Errorf("failed to read docs file: %w", err) + } + updated, err := replaceSection(string(content), startMarker, endMarker, body) + if err != nil { + return err + } + return os.WriteFile(path, []byte(updated), 0600) //#nosec G306 +} diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 7a97e4f66..efa8f7c39 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -29,6 +29,12 @@ func init() { rootCmd.AddCommand(generateDocsCmd) } +// noFeatureFlagsChecker reports every feature flag as disabled. It models the +// default user experience used by the generated documentation. +func noFeatureFlagsChecker(_ context.Context, _ string) (bool, error) { + return false, nil +} + func generateAllDocs() error { for _, doc := range []struct { path string @@ -37,6 +43,8 @@ func generateAllDocs() error { // File to edit, function to generate its docs {"README.md", generateReadmeDocs}, {"docs/remote-server.md", generateRemoteServerDocs}, + {"docs/insiders-features.md", generateInsidersFeaturesDocs}, + {"docs/feature-flags.md", generateFeatureFlagsDocs}, {"docs/tool-renaming.md", generateDeprecatedAliasesDocs}, } { if err := doc.fn(doc.path); err != nil { @@ -51,9 +59,16 @@ func generateReadmeDocs(readmePath string) error { // Create translation helper t, _ := translations.TranslationHelper() - // (not available to regular users) while including tools with FeatureFlagDisable. + // The README documents the default user experience: tools that are + // enabled with no special flags set. Installing a checker that reports + // every flag as disabled excludes tools gated by FeatureFlagEnable and + // keeps the legacy variants of tools gated by FeatureFlagDisable, so + // flag-gated duplicates don't appear twice. // Build() can only fail if WithTools specifies invalid tools - not used here - r, _ := github.NewInventory(t).WithToolsets([]string{"all"}).Build() + r, _ := github.NewInventory(t). + WithToolsets([]string{"all"}). + WithFeatureChecker(noFeatureFlagsChecker). + Build() // Generate toolsets documentation toolsetsDoc := generateToolsetsDoc(r) @@ -155,7 +170,7 @@ func generateToolsetsDoc(i *inventory.Inventory) string { } func generateToolsDoc(r *inventory.Inventory) string { - tools := r.AvailableTools(context.Background()) + tools := r.ToolsForRegistration(context.Background()) if len(tools) == 0 { return "" } @@ -214,6 +229,15 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) { } } + // MCP App UI metadata (only rendered when the remote_mcp_ui_apps flag + // applied to the inventory; for the no-flags README this section is + // stripped by inventory.ToolsForRegistration before rendering). + if ui, ok := tool.Tool.Meta["ui"].(map[string]any); ok { + if uri, ok := ui["resourceUri"].(string); ok && uri != "" { + fmt.Fprintf(buf, " - **MCP App UI**: `%s`\n", uri) + } + } + // Parameters if tool.Tool.InputSchema == nil { buf.WriteString(" - No parameters required") diff --git a/docs/feature-flags.md b/docs/feature-flags.md new file mode 100644 index 000000000..a552e71a0 --- /dev/null +++ b/docs/feature-flags.md @@ -0,0 +1,267 @@ +# Feature Flags + +Feature flags let you opt into experimental tool behavior on top of the default +GitHub MCP Server surface. Insiders Mode turns on a curated subset of these +flags automatically — see [Insiders Features](./insiders-features.md) for that +specific set. + +For background on how flags resolve at request time, see the [resolution +section in the Insiders docs](./insiders-features.md#how-feature-flags-are-resolved). + +## Enabling a flag + +| Method | Remote Server | Local Server | +|--------|---------------|--------------| +| Header | `X-MCP-Features: ,` | N/A | +| CLI flag | N/A | `--features=,` | +| Environment variable | N/A | `GITHUB_FEATURES=,` | + +Only flags listed in +[`AllowedFeatureFlags`](../pkg/github/feature_flags.go) can be enabled by +end users. Insiders-only flags are not user-toggleable. + +--- + +## Tools affected by each flag + +The list below is regenerated from the Go source. For each user-controllable +feature flag, it lists every tool whose **inventory or input schema** differs +from the default — either because the flag introduces a new tool, or because +it selects a flag-aware variant of an existing tool. Flags that only affect +runtime behavior (such as output formatting) won't appear here. + + + +### `remote_mcp_ui_apps` + +- **create_pull_request** - Open new pull request + - **Required OAuth Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/pr-write` + - `base`: Branch to merge into (string, required) + - `body`: PR description (string, optional) + - `draft`: Create as draft PR (boolean, optional) + - `head`: Branch containing changes (string, required) + - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `title`: PR title (string, required) + +- **get_me** - Get my user profile + - **MCP App UI**: `ui://github-mcp-server/get-me` + - No parameters required + +- **issue_write** - Create or update issue + - **Required OAuth Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/issue-write` + - `assignees`: Usernames to assign to this issue (string[], optional) + - `body`: Issue body content (string, optional) + - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) + - `issue_number`: Issue number to update (number, optional) + - `labels`: Labels to apply to this issue (string[], optional) + - `method`: Write operation to perform on a single issue. + Options are: + - 'create' - creates a new issue. + - 'update' - updates an existing issue. + (string, required) + - `milestone`: Milestone number (number, optional) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `state`: New state (string, optional) + - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) + - `title`: Issue title (string, optional) + - `type`: Type of this issue. Only use if the repository has issue types configured. Use list_issue_types tool to get valid type values for the organization. If the repository doesn't support issue types, omit this parameter. (string, optional) + +### `remote_mcp_issue_fields` + +- **list_issue_fields** - List issue fields + - **Required OAuth Scopes**: `repo`, `read:org` + - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - `owner`: The account owner of the repository or organization. The name is not case sensitive. (string, required) + - `repo`: The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly. (string, optional) + +- **list_issues** - List issues + - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) + - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) + - `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional) + - `labels`: Filter by labels (string[], optional) + - `orderBy`: Order issues 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) + - `since`: Filter by date (ISO 8601 timestamp) (string, optional) + - `state`: Filter by state, by default both open and closed issues are returned when not provided (string, optional) + +### `issues_granular` + +- **add_sub_issue** - Add Sub-Issue + - **Required OAuth Scopes**: `repo` + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `replace_parent`: If true, reparent the sub-issue if it already has a parent (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) + +- **create_issue** - Create Issue + - **Required OAuth Scopes**: `repo` + - `body`: Issue body content (optional) (string, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `title`: Issue title (string, required) + +- **remove_sub_issue** - Remove Sub-Issue + - **Required OAuth Scopes**: `repo` + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (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 + - **Required OAuth Scopes**: `repo` + - `after_id`: The ID of the sub-issue to place this after (either after_id OR before_id should be specified) (number, optional) + - `before_id`: The ID of the sub-issue to place this before (either after_id OR before_id should be specified) (number, optional) + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The ID of the sub-issue to reorder. ID is not the same as issue number (number, required) + +- **set_issue_fields** - Set Issue Fields + - **Required OAuth Scopes**: `repo` + - `fields`: Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value. (object[], required) + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_assignees** - Update Issue Assignees + - **Required OAuth Scopes**: `repo` + - `assignees`: GitHub usernames to assign to this issue (string[], required) + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_body** - Update Issue Body + - **Required OAuth Scopes**: `repo` + - `body`: The new body content for the issue (string, required) + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_labels** - Update Issue Labels + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `labels`: Labels to apply to this issue. ([], required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_milestone** - Update Issue Milestone + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `milestone`: The milestone number to set on the issue (integer, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_state** - Update Issue State + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `state`: The new state for the issue (string, required) + - `state_reason`: The reason for the state change (only for closed state) (string, optional) + +- **update_issue_title** - Update Issue Title + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `title`: The new title for the issue (string, required) + +- **update_issue_type** - Update Issue Type + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `issue_type`: The issue type to set (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this type. State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature). (string, optional) + - `repo`: Repository name (string, required) + +### `pull_requests_granular` + +- **add_pull_request_review_comment** - Add Pull Request Review Comment + - **Required OAuth Scopes**: `repo` + - `body`: The comment body (string, required) + - `line`: The line number in the diff to comment on (optional) (number, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `path`: The relative path of the file to comment on (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `side`: The side of the diff to comment on (optional) (string, optional) + - `startLine`: The start line of a multi-line comment (optional) (number, optional) + - `startSide`: The start side of a multi-line comment (optional) (string, optional) + - `subjectType`: The subject type of the comment (string, required) + +- **create_pull_request_review** - Create Pull Request Review + - **Required OAuth Scopes**: `repo` + - `body`: The review body text (optional) (string, optional) + - `commitID`: The SHA of the commit to review (optional, defaults to latest) (string, optional) + - `event`: The review action to perform. If omitted, creates a pending review. (string, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **delete_pending_pull_request_review** - Delete Pending Pull Request Review + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **request_pull_request_reviewers** - Request Pull Request Reviewers + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `reviewers`: GitHub usernames to request reviews from (string[], required) + +- **resolve_review_thread** - Resolve Review Thread + - **Required OAuth Scopes**: `repo` + - `threadID`: The node ID of the review thread to resolve (e.g., PRRT_kwDOxxx) (string, required) + +- **submit_pending_pull_request_review** - Submit Pending Pull Request Review + - **Required OAuth Scopes**: `repo` + - `body`: The review body text (optional) (string, optional) + - `event`: The review action to perform (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **unresolve_review_thread** - Unresolve Review Thread + - **Required OAuth Scopes**: `repo` + - `threadID`: The node ID of the review thread to unresolve (e.g., PRRT_kwDOxxx) (string, required) + +- **update_pull_request_body** - Update Pull Request Body + - **Required OAuth Scopes**: `repo` + - `body`: The new body content for the pull request (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_draft_state** - Update Pull Request Draft State + - **Required OAuth Scopes**: `repo` + - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_state** - Update Pull Request State + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `state`: The new state for the pull request (string, required) + +- **update_pull_request_title** - Update Pull Request Title + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `title`: The new title for the pull request (string, required) + + diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 90afe7219..c221b8758 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -20,6 +20,76 @@ For configuration examples, see the [Server Configuration Guide](./server-config --- +## Tools added or changed by Insiders Mode + +The list below is generated from the Go source. It covers tool **inventory and schema deltas** introduced by each Insiders feature flag — newly registered tools, or existing tools whose input schema or MCP metadata changes when the flag is on. Flags that only affect runtime behavior (e.g. output formatting or extra field lookups behind an existing schema) won't appear here; those are documented in the prose sections of this file. + + + +### `remote_mcp_ui_apps` + +- **create_pull_request** - Open new pull request + - **Required OAuth Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/pr-write` + - `base`: Branch to merge into (string, required) + - `body`: PR description (string, optional) + - `draft`: Create as draft PR (boolean, optional) + - `head`: Branch containing changes (string, required) + - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `title`: PR title (string, required) + +- **get_me** - Get my user profile + - **MCP App UI**: `ui://github-mcp-server/get-me` + - No parameters required + +- **issue_write** - Create or update issue + - **Required OAuth Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/issue-write` + - `assignees`: Usernames to assign to this issue (string[], optional) + - `body`: Issue body content (string, optional) + - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) + - `issue_number`: Issue number to update (number, optional) + - `labels`: Labels to apply to this issue (string[], optional) + - `method`: Write operation to perform on a single issue. + Options are: + - 'create' - creates a new issue. + - 'update' - updates an existing issue. + (string, required) + - `milestone`: Milestone number (number, optional) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `state`: New state (string, optional) + - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) + - `title`: Issue title (string, optional) + - `type`: Type of this issue. Only use if the repository has issue types configured. Use list_issue_types tool to get valid type values for the organization. If the repository doesn't support issue types, omit this parameter. (string, optional) + +### `remote_mcp_issue_fields` + +- **list_issue_fields** - List issue fields + - **Required OAuth Scopes**: `repo`, `read:org` + - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` + - `owner`: The account owner of the repository or organization. The name is not case sensitive. (string, required) + - `repo`: The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly. (string, optional) + +- **list_issues** - List issues + - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) + - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) + - `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional) + - `labels`: Filter by labels (string[], optional) + - `orderBy`: Order issues 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) + - `since`: Filter by date (ISO 8601 timestamp) (string, optional) + - `state`: Filter by state, by default both open and closed issues are returned when not provided (string, optional) + + + +--- + ## MCP Apps [MCP Apps](https://modelcontextprotocol.io/docs/extensions/apps) is an extension to the Model Context Protocol that enables servers to deliver interactive user interfaces to end users. Instead of returning plain text that the LLM must interpret and relay, tools can render forms, profiles, and dashboards right in the chat using MCP Apps. diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index a125864f0..3441afaf5 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -29,6 +29,29 @@ "description": "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.", "type": "number" }, + "issue_fields": { + "description": "Issue field values to set. Each item requires field_name and either value or field_option_name. field_option_name is for single-select fields and is resolved to the corresponding option ID automatically.", + "items": { + "properties": { + "field_name": { + "description": "Issue field name", + "type": "string" + }, + "field_option_name": { + "description": "Single-select option name to resolve and set for the field", + "type": "string" + }, + "value": { + "description": "Value for text/number/date/single-select fields. For single-select, you can use field_option_name instead." + } + }, + "required": [ + "field_name" + ], + "type": "object" + }, + "type": "array" + }, "issue_number": { "description": "Issue number to update", "type": "number" diff --git a/pkg/github/__toolsnaps__/list_issues.snap b/pkg/github/__toolsnaps__/list_issues.snap index b1d1c7a21..a4be59bb0 100644 --- a/pkg/github/__toolsnaps__/list_issues.snap +++ b/pkg/github/__toolsnaps__/list_issues.snap @@ -18,27 +18,6 @@ ], "type": "string" }, - "field_filters": { - "description": "Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date).", - "items": { - "properties": { - "field_name": { - "description": "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", - "type": "string" - }, - "value": { - "description": "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", - "type": "string" - } - }, - "required": [ - "field_name", - "value" - ], - "type": "object" - }, - "type": "array" - }, "labels": { "description": "Filter by labels", "items": { diff --git a/pkg/github/__toolsnaps__/list_issues_ff_remote_mcp_issue_fields.snap b/pkg/github/__toolsnaps__/list_issues_ff_remote_mcp_issue_fields.snap new file mode 100644 index 000000000..b1d1c7a21 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_issues_ff_remote_mcp_issue_fields.snap @@ -0,0 +1,92 @@ +{ + "annotations": { + "readOnlyHint": true, + "title": "List issues" + }, + "description": "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.", + "inputSchema": { + "properties": { + "after": { + "description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.", + "type": "string" + }, + "direction": { + "description": "Order direction. If provided, the 'orderBy' also needs to be provided.", + "enum": [ + "ASC", + "DESC" + ], + "type": "string" + }, + "field_filters": { + "description": "Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date).", + "items": { + "properties": { + "field_name": { + "description": "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", + "type": "string" + }, + "value": { + "description": "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", + "type": "string" + } + }, + "required": [ + "field_name", + "value" + ], + "type": "object" + }, + "type": "array" + }, + "labels": { + "description": "Filter by labels", + "items": { + "type": "string" + }, + "type": "array" + }, + "orderBy": { + "description": "Order issues by field. If provided, the 'direction' also needs to be provided.", + "enum": [ + "CREATED_AT", + "UPDATED_AT", + "COMMENTS" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "since": { + "description": "Filter by date (ISO 8601 timestamp)", + "type": "string" + }, + "state": { + "description": "Filter by state, by default both open and closed issues are returned when not provided", + "enum": [ + "OPEN", + "CLOSED" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "name": "list_issues" +} \ No newline at end of file diff --git a/pkg/github/csv_output.go b/pkg/github/csv_output.go index cb70e32d7..6acb8b2fd 100644 --- a/pkg/github/csv_output.go +++ b/pkg/github/csv_output.go @@ -56,14 +56,16 @@ func withCSVOutput(tools []inventory.ServerTool) []inventory.ServerTool { return tools } +// isCSVOutputTool reports whether the given tool should have its handler +// wrapped to honor the csv_output feature flag. Wrapping happens at slice +// construction time, before the per-request feature-flag filter chooses which +// variant of a flag-gated tool to register, so flag-gated list_* tools are +// included on equal footing — only the live variant ever runs at request time. func isCSVOutputTool(tool inventory.ServerTool) bool { if !tool.Toolset.Default { return false } - if !strings.HasPrefix(tool.Tool.Name, "list_") { - return false - } - return tool.FeatureFlagEnable == "" && tool.FeatureFlagDisable == "" + return strings.HasPrefix(tool.Tool.Name, "list_") } func wrapHandlerWithCSVOutput(next inventory.HandlerFunc) inventory.HandlerFunc { diff --git a/pkg/github/csv_output_test.go b/pkg/github/csv_output_test.go index d0bef3893..246902d49 100644 --- a/pkg/github/csv_output_test.go +++ b/pkg/github/csv_output_test.go @@ -38,6 +38,26 @@ func TestCSVOutputAppliedToDefaultListTools(t *testing.T) { } } +func TestCSVOutputAppliesToFlagGatedListTools(t *testing.T) { + enabledOnly := testCSVOutputTool("list_things", `[{"number":1}]`) + enabledOnly.FeatureFlagEnable = FeatureFlagIssueFields + disabledOnly := testCSVOutputTool("list_legacy_things", `[{"number":2}]`) + disabledOnly.FeatureFlagDisable = FeatureFlagIssueFields + + tools := withCSVOutput([]inventory.ServerTool{enabledOnly, disabledOnly}) + require.Len(t, tools, 2) + + // Both flag-gated variants get the CSV wrapper; the per-request flag filter + // decides which one actually registers, and the runtime csv_output check + // decides whether the wrapper converts the response. + deps := newCSVOutputTestDeps(true) + for _, tool := range tools { + result, err := tool.Handler(deps)(ContextWithDeps(context.Background(), deps), testCSVOutputRequest()) + require.NoError(t, err) + assert.Contains(t, textResult(t, result), "number\n") + } +} + func TestCSVOutputOnlyAppliesToDefaultToolsets(t *testing.T) { nonDefaultListTool := testCSVOutputToolWithToolset("list_discussions", `[{"number":1}]`, ToolsetMetadataDiscussions) diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 19399e7ac..6f04be7f1 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -11,6 +11,11 @@ const FeatureFlagCSVOutput = "csv_output" // FeatureFlagIFCLabels is the feature flag name for IFC security labels in tool results. const FeatureFlagIFCLabels = "ifc_labels" +// FeatureFlagIssueFields is the feature flag name for Issues 2.0 custom field +// support: the list_issue_fields tool, the field_filters input on list_issues, +// and field_values enrichment in list_issues / search_issues output. +const FeatureFlagIssueFields = "remote_mcp_issue_fields" + // AllowedFeatureFlags is the allowlist of feature flags that can be enabled // by users via --features CLI flag or X-MCP-Features HTTP header. // Only flags in this list are accepted; unknown flags are silently ignored. @@ -18,6 +23,7 @@ const FeatureFlagIFCLabels = "ifc_labels" var AllowedFeatureFlags = []string{ MCPAppsFeatureFlag, FeatureFlagCSVOutput, + FeatureFlagIssueFields, FeatureFlagIssuesGranular, FeatureFlagPullRequestsGranular, } @@ -30,6 +36,7 @@ var InsidersFeatureFlags = []string{ MCPAppsFeatureFlag, FeatureFlagCSVOutput, FeatureFlagIFCLabels, + FeatureFlagIssueFields, } // FeatureFlags defines runtime feature toggles that adjust tool behavior. diff --git a/pkg/github/issue_fields.go b/pkg/github/issue_fields.go index 70f1a7c51..1eabbc02f 100644 --- a/pkg/github/issue_fields.go +++ b/pkg/github/issue_fields.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strconv" ghcontext "github.com/github/github-mcp-server/pkg/context" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -19,6 +20,7 @@ import ( // IssueField represents a repository issue field definition. type IssueField struct { ID string `json:"id"` + DatabaseID int64 `json:"full_database_id,omitempty"` Name string `json:"name"` Description string `json:"description,omitempty"` DataType string `json:"data_type"` @@ -37,36 +39,42 @@ type IssueSingleSelectFieldOption struct { // issueFieldNode is the GraphQL fragment for a single issue field in the IssueFields union. // Only the fragment matching __typename is populated; read from the matching fragment. +// fullDatabaseId (BigInt scalar, returned as string) is fetched on each concrete type because +// shurcooL/githubv4 does not support interface fragments at the top level of a union. type issueFieldNode struct { TypeName githubv4.String `graphql:"__typename"` IssueFieldText struct { - ID githubv4.ID - Name githubv4.String - Description githubv4.String - DataType githubv4.String - Visibility githubv4.String + ID githubv4.ID + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String } `graphql:"... on IssueFieldText"` IssueFieldNumber struct { - ID githubv4.ID - Name githubv4.String - Description githubv4.String - DataType githubv4.String - Visibility githubv4.String + ID githubv4.ID + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String } `graphql:"... on IssueFieldNumber"` IssueFieldDate struct { - ID githubv4.ID - Name githubv4.String - Description githubv4.String - DataType githubv4.String - Visibility githubv4.String + ID githubv4.ID + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String } `graphql:"... on IssueFieldDate"` IssueFieldSingleSelect struct { - ID githubv4.ID - Name githubv4.String - Description githubv4.String - DataType githubv4.String - Visibility githubv4.String - Options []struct { + ID githubv4.ID + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String + Options []struct { ID githubv4.ID Name githubv4.String Description githubv4.String @@ -95,8 +103,9 @@ type issueFieldsOrgQuery struct { } // ListIssueFields creates a tool to list issue field definitions for a repository or organization. +// Gated by FeatureFlagIssueFields: the tool is only registered when the flag is on. func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "list_issue_fields", @@ -148,6 +157,8 @@ func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultText(string(r)), nil, nil }) + st.FeatureFlagEnable = FeatureFlagIssueFields + return st } // fetchIssueFields returns the issue field definitions for the given owner. @@ -197,6 +208,7 @@ func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { } f = IssueField{ ID: fmt.Sprintf("%v", node.IssueFieldSingleSelect.ID), + DatabaseID: parseFullDatabaseID(string(node.IssueFieldSingleSelect.FullDatabaseID)), Name: string(node.IssueFieldSingleSelect.Name), Description: string(node.IssueFieldSingleSelect.Description), DataType: string(node.IssueFieldSingleSelect.DataType), @@ -206,6 +218,7 @@ func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { case "IssueFieldText": f = IssueField{ ID: fmt.Sprintf("%v", node.IssueFieldText.ID), + DatabaseID: parseFullDatabaseID(string(node.IssueFieldText.FullDatabaseID)), Name: string(node.IssueFieldText.Name), Description: string(node.IssueFieldText.Description), DataType: string(node.IssueFieldText.DataType), @@ -214,6 +227,7 @@ func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { case "IssueFieldNumber": f = IssueField{ ID: fmt.Sprintf("%v", node.IssueFieldNumber.ID), + DatabaseID: parseFullDatabaseID(string(node.IssueFieldNumber.FullDatabaseID)), Name: string(node.IssueFieldNumber.Name), Description: string(node.IssueFieldNumber.Description), DataType: string(node.IssueFieldNumber.DataType), @@ -222,6 +236,7 @@ func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { case "IssueFieldDate": f = IssueField{ ID: fmt.Sprintf("%v", node.IssueFieldDate.ID), + DatabaseID: parseFullDatabaseID(string(node.IssueFieldDate.FullDatabaseID)), Name: string(node.IssueFieldDate.Name), Description: string(node.IssueFieldDate.Description), DataType: string(node.IssueFieldDate.DataType), @@ -234,3 +249,16 @@ func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { } return fields } + +// parseFullDatabaseID converts a BigInt scalar string (e.g. "12345") to int64. +// Returns 0 if the string is empty or cannot be parsed. +func parseFullDatabaseID(s string) int64 { + if s == "" { + return 0 + } + n, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return 0 + } + return n +} diff --git a/pkg/github/issue_fields_test.go b/pkg/github/issue_fields_test.go index 238c0455b..2c2b26ee2 100644 --- a/pkg/github/issue_fields_test.go +++ b/pkg/github/issue_fields_test.go @@ -75,12 +75,13 @@ func Test_ListIssueFields(t *testing.T) { "issueFields": map[string]any{ "nodes": []any{ map[string]any{ - "__typename": "IssueFieldText", - "id": "IFT_1", - "name": "DRI", - "description": "Directly responsible individual", - "dataType": "TEXT", - "visibility": "ORG_ONLY", + "__typename": "IssueFieldText", + "id": "IFT_1", + "fullDatabaseId": "42", + "name": "DRI", + "description": "Directly responsible individual", + "dataType": "TEXT", + "visibility": "ORG_ONLY", }, }, }, @@ -89,6 +90,7 @@ func Test_ListIssueFields(t *testing.T) { expectedFields: []IssueField{ { ID: "IFT_1", + DatabaseID: 42, Name: "DRI", Description: "Directly responsible individual", DataType: "TEXT", @@ -107,12 +109,13 @@ func Test_ListIssueFields(t *testing.T) { "issueFields": map[string]any{ "nodes": []any{ map[string]any{ - "__typename": "IssueFieldSingleSelect", - "id": "IFSS_1", - "name": "Priority", - "description": "Level of importance", - "dataType": "SINGLE_SELECT", - "visibility": "ALL", + "__typename": "IssueFieldSingleSelect", + "id": "IFSS_1", + "fullDatabaseId": "99", + "name": "Priority", + "description": "Level of importance", + "dataType": "SINGLE_SELECT", + "visibility": "ALL", "options": []any{ map[string]any{ "id": "OPT_1", @@ -133,6 +136,7 @@ func Test_ListIssueFields(t *testing.T) { expectedFields: []IssueField{ { ID: "IFSS_1", + DatabaseID: 99, Name: "Priority", Description: "Level of importance", DataType: "SINGLE_SELECT", @@ -165,18 +169,19 @@ func Test_ListIssueFields(t *testing.T) { "issueFields": map[string]any{ "nodes": []any{ map[string]any{ - "__typename": "IssueFieldText", - "id": "IFT_1", - "name": "DRI", - "dataType": "TEXT", - "visibility": "ORG_ONLY", + "__typename": "IssueFieldText", + "id": "IFT_1", + "fullDatabaseId": "77", + "name": "DRI", + "dataType": "TEXT", + "visibility": "ORG_ONLY", }, }, }, }, }), expectedFields: []IssueField{ - {ID: "IFT_1", Name: "DRI", DataType: "TEXT", Visibility: "ORG_ONLY"}, + {ID: "IFT_1", DatabaseID: 77, Name: "DRI", DataType: "TEXT", Visibility: "ORG_ONLY"}, }, }, { @@ -190,18 +195,19 @@ func Test_ListIssueFields(t *testing.T) { "issueFields": map[string]any{ "nodes": []any{ map[string]any{ - "__typename": "IssueFieldNumber", - "id": "IFN_1", - "name": "Engineering Staffing", - "dataType": "NUMBER", - "visibility": "ORG_ONLY", + "__typename": "IssueFieldNumber", + "id": "IFN_1", + "fullDatabaseId": "101", + "name": "Engineering Staffing", + "dataType": "NUMBER", + "visibility": "ORG_ONLY", }, }, }, }, }), expectedFields: []IssueField{ - {ID: "IFN_1", Name: "Engineering Staffing", DataType: "NUMBER", Visibility: "ORG_ONLY"}, + {ID: "IFN_1", DatabaseID: 101, Name: "Engineering Staffing", DataType: "NUMBER", Visibility: "ORG_ONLY"}, }, }, { @@ -215,18 +221,19 @@ func Test_ListIssueFields(t *testing.T) { "issueFields": map[string]any{ "nodes": []any{ map[string]any{ - "__typename": "IssueFieldDate", - "id": "IFD_1", - "name": "Target Date", - "dataType": "DATE", - "visibility": "ORG_ONLY", + "__typename": "IssueFieldDate", + "id": "IFD_1", + "fullDatabaseId": "202", + "name": "Target Date", + "dataType": "DATE", + "visibility": "ORG_ONLY", }, }, }, }, }), expectedFields: []IssueField{ - {ID: "IFD_1", Name: "Target Date", DataType: "DATE", Visibility: "ORG_ONLY"}, + {ID: "IFD_1", DatabaseID: 202, Name: "Target Date", DataType: "DATE", Visibility: "ORG_ONLY"}, }, }, { @@ -284,6 +291,7 @@ func Test_ListIssueFields(t *testing.T) { require.Equal(t, len(tc.expectedFields), len(returnedFields)) for i, expected := range tc.expectedFields { assert.Equal(t, expected.ID, returnedFields[i].ID) + assert.Equal(t, expected.DatabaseID, returnedFields[i].DatabaseID) assert.Equal(t, expected.Name, returnedFields[i].Name) assert.Equal(t, expected.DataType, returnedFields[i].DataType) assert.Equal(t, expected.Visibility, returnedFields[i].Visibility) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index e56e793a4..0e4ad9c2f 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -37,6 +37,14 @@ type CloseIssueInput struct { // Used to extend the functionality of the githubv4 library to support closing issues as duplicates. type IssueClosedStateReason string +// IssueWriteFieldInput is a user-friendly issue field input for issue_write. +// Field IDs and option IDs are resolved internally before calling the REST API. +type IssueWriteFieldInput struct { + FieldName string + Value any + FieldOptionName string +} + const ( IssueClosedStateReasonCompleted IssueClosedStateReason = "COMPLETED" IssueClosedStateReasonDuplicate IssueClosedStateReason = "DUPLICATE" @@ -105,6 +113,46 @@ func getCloseStateReason(stateReason string) IssueClosedStateReason { } } +// issueFieldWriteMetadataNode queries only the fields needed to resolve a write: the field's +// fullDatabaseId (BigInt scalar, returned as string) plus its name and data type for validation. +// shurcooL/githubv4 cannot use interface-level fragments at union top-level, so we repeat +// fullDatabaseId on each concrete type; all four implement IssueFieldCommon. +type issueFieldWriteMetadataNode struct { + TypeName githubv4.String `graphql:"__typename"` + IssueFieldText struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on IssueFieldText"` + IssueFieldNumber struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on IssueFieldNumber"` + IssueFieldDate struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on IssueFieldDate"` + IssueFieldSingleSelect struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + DataType githubv4.String + Options []struct { + FullDatabaseID githubv4.String `graphql:"fullDatabaseId"` + Name githubv4.String + } + } `graphql:"... on IssueFieldSingleSelect"` +} + +type issueFieldWriteMetadataQuery struct { + Repository struct { + IssueFields struct { + Nodes []issueFieldWriteMetadataNode + } `graphql:"issueFields(first: 100)"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + // IssueFieldRef resolves the name of an issue field across its concrete types. // IssueFields is a union of IssueFieldDate, IssueFieldNumber, IssueFieldSingleSelect, IssueFieldText, // so we have to ask for `name` on each member. @@ -153,6 +201,158 @@ type IssueFieldValueFragment struct { } `graphql:"... on IssueFieldTextValue"` } +func optionalIssueWriteFields(args map[string]any) ([]IssueWriteFieldInput, error) { + issueFieldsRaw, exists := args["issue_fields"] + if !exists { + return nil, nil + } + + var inputMaps []map[string]any + switch v := issueFieldsRaw.(type) { + case []any: + for _, item := range v { + itemMap, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf("each issue_fields item must be an object") + } + inputMaps = append(inputMaps, itemMap) + } + case []map[string]any: + inputMaps = v + default: + return nil, fmt.Errorf("issue_fields must be an array") + } + + issueFields := make([]IssueWriteFieldInput, 0, len(inputMaps)) + for _, itemMap := range inputMaps { + fieldName, err := RequiredParam[string](itemMap, "field_name") + if err != nil || strings.TrimSpace(fieldName) == "" { + return nil, fmt.Errorf("field_name is required for each issue_fields item") + } + + fieldOptionName, err := OptionalParam[string](itemMap, "field_option_name") + if err != nil { + return nil, err + } + + value, hasValue := itemMap["value"] + if hasValue && value == nil { + return nil, fmt.Errorf("value cannot be null for field %q", fieldName) + } + + if hasValue && fieldOptionName != "" { + return nil, fmt.Errorf("issue field %q cannot specify both value and field_option_name", fieldName) + } + + if !hasValue && fieldOptionName == "" { + return nil, fmt.Errorf("issue field %q must specify either value or field_option_name", fieldName) + } + + issueFields = append(issueFields, IssueWriteFieldInput{ + FieldName: fieldName, + Value: value, + FieldOptionName: fieldOptionName, + }) + } + + return issueFields, nil +} + +func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []IssueWriteFieldInput) ([]*github.IssueRequestFieldValue, error) { + if len(issueFields) == 0 { + return nil, nil + } + + ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") + var query issueFieldWriteMetadataQuery + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { + return nil, fmt.Errorf("failed to query issue fields metadata: %w", err) + } + + // Build name → node map, dispatching on concrete type to extract name. + fieldByName := make(map[string]issueFieldWriteMetadataNode, len(query.Repository.IssueFields.Nodes)) + for _, node := range query.Repository.IssueFields.Nodes { + var name string + switch string(node.TypeName) { + case "IssueFieldText": + name = string(node.IssueFieldText.Name) + case "IssueFieldNumber": + name = string(node.IssueFieldNumber.Name) + case "IssueFieldDate": + name = string(node.IssueFieldDate.Name) + case "IssueFieldSingleSelect": + name = string(node.IssueFieldSingleSelect.Name) + default: + continue + } + fieldByName[strings.ToLower(strings.TrimSpace(name))] = node + } + + resolved := make([]*github.IssueRequestFieldValue, 0, len(issueFields)) + for _, fieldInput := range issueFields { + node, ok := fieldByName[strings.ToLower(strings.TrimSpace(fieldInput.FieldName))] + if !ok { + return nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo) + } + + var fullDatabaseIDStr, dataType string + switch string(node.TypeName) { + case "IssueFieldText": + fullDatabaseIDStr = string(node.IssueFieldText.FullDatabaseID) + dataType = string(node.IssueFieldText.DataType) + case "IssueFieldNumber": + fullDatabaseIDStr = string(node.IssueFieldNumber.FullDatabaseID) + dataType = string(node.IssueFieldNumber.DataType) + case "IssueFieldDate": + fullDatabaseIDStr = string(node.IssueFieldDate.FullDatabaseID) + dataType = string(node.IssueFieldDate.DataType) + case "IssueFieldSingleSelect": + fullDatabaseIDStr = string(node.IssueFieldSingleSelect.FullDatabaseID) + dataType = string(node.IssueFieldSingleSelect.DataType) + } + + fieldID := parseFullDatabaseID(fullDatabaseIDStr) + if fieldID == 0 { + return nil, fmt.Errorf("issue field %q is missing fullDatabaseId", fieldInput.FieldName) + } + + resolvedValue := fieldInput.Value + if fieldInput.FieldOptionName != "" { + if !strings.EqualFold(dataType, "single_select") { + return nil, fmt.Errorf("issue field %q is %q, so field_option_name cannot be used", fieldInput.FieldName, dataType) + } + + optionFound := false + for _, option := range node.IssueFieldSingleSelect.Options { + if strings.EqualFold(strings.TrimSpace(string(option.Name)), strings.TrimSpace(fieldInput.FieldOptionName)) { + optionID := parseFullDatabaseID(string(option.FullDatabaseID)) + if optionID == 0 { + return nil, fmt.Errorf("issue field option %q on field %q is missing fullDatabaseId", fieldInput.FieldOptionName, fieldInput.FieldName) + } + resolvedValue = optionID + optionFound = true + break + } + } + + if !optionFound { + return nil, fmt.Errorf("issue field option %q was not found for field %q", fieldInput.FieldOptionName, fieldInput.FieldName) + } + } + + resolved = append(resolved, &github.IssueRequestFieldValue{ + FieldID: fieldID, + Value: resolvedValue, + }) + } + + return resolved, nil +} + // IssueFragment represents a fragment of an issue node in the GraphQL API. type IssueFragment struct { Number githubv4.Int @@ -280,6 +480,123 @@ func getIssueQueryType(hasLabels bool, hasSince bool) any { } } +// --- Legacy list_issues GraphQL types --- +// +// These mirror the pre-Issues-2.0 shape of the list_issues query and exist solely +// to back the FeatureFlagIssueFields-disabled variant of the tool. They omit the +// IssueFieldValues selection and the filterBy: {issueFieldValues: ...} clause so +// the request does not depend on server-side issue_fields GraphQL features and +// does not pay the wire/server cost of fetching custom field values when the flag +// is off. Delete this whole block (and its callers) when FeatureFlagIssueFields +// is removed. + +type LegacyIssueFragment struct { + Number githubv4.Int + Title githubv4.String + Body githubv4.String + State githubv4.String + DatabaseID int64 + + Author struct { + Login githubv4.String + } + CreatedAt githubv4.DateTime + UpdatedAt githubv4.DateTime + Labels struct { + Nodes []struct { + Name githubv4.String + ID githubv4.String + Description githubv4.String + } + } `graphql:"labels(first: 100)"` + Comments struct { + TotalCount githubv4.Int + } `graphql:"comments"` +} + +type LegacyIssueQueryFragment struct { + Nodes []LegacyIssueFragment `graphql:"nodes"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + TotalCount int +} + +type LegacyIssueQueryResult interface { + GetLegacyIssueFragment() LegacyIssueQueryFragment + GetIsPrivate() bool +} + +type LegacyListIssuesQuery struct { + Repository struct { + Issues LegacyIssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type LegacyListIssuesQueryTypeWithLabels struct { + Repository struct { + Issues LegacyIssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type LegacyListIssuesQueryWithSince struct { + Repository struct { + Issues LegacyIssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type LegacyListIssuesQueryTypeWithLabelsWithSince struct { + Repository struct { + Issues LegacyIssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +func (q *LegacyListIssuesQuery) GetLegacyIssueFragment() LegacyIssueQueryFragment { + return q.Repository.Issues +} +func (q *LegacyListIssuesQuery) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) } + +func (q *LegacyListIssuesQueryTypeWithLabels) GetLegacyIssueFragment() LegacyIssueQueryFragment { + return q.Repository.Issues +} +func (q *LegacyListIssuesQueryTypeWithLabels) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func (q *LegacyListIssuesQueryWithSince) GetLegacyIssueFragment() LegacyIssueQueryFragment { + return q.Repository.Issues +} +func (q *LegacyListIssuesQueryWithSince) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func (q *LegacyListIssuesQueryTypeWithLabelsWithSince) GetLegacyIssueFragment() LegacyIssueQueryFragment { + return q.Repository.Issues +} +func (q *LegacyListIssuesQueryTypeWithLabelsWithSince) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func getLegacyIssueQueryType(hasLabels bool, hasSince bool) any { + switch { + case hasLabels && hasSince: + return &LegacyListIssuesQueryTypeWithLabelsWithSince{} + case hasLabels: + return &LegacyListIssuesQueryTypeWithLabels{} + case hasSince: + return &LegacyListIssuesQueryWithSince{} + default: + return &LegacyListIssuesQuery{} + } +} + // IssueRead creates a tool to get details of a specific issue in a GitHub repository. func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ @@ -1149,7 +1466,7 @@ func parseRepositoryURL(repoURL string) (string, string, bool) { // SearchIssueResult wraps a REST search hit with its custom issue field values, fetched in a follow-up GraphQL nodes() query. type SearchIssueResult struct { *github.Issue - FieldValues []MinimalIssueFieldValue `json:"field_values,omitempty"` + FieldValues []MinimalFieldValue `json:"field_values,omitempty"` } // MarshalJSON serializes SearchIssueResult, suppressing the raw issue_field_values from the @@ -1198,7 +1515,7 @@ type searchIssuesNodesQuery struct { // fetchIssueFieldValuesByNodeID runs one GraphQL nodes() query for the given REST issues and // returns a map of node_id -> flattened field values. Issues without a node_id are skipped, and // an empty result set short-circuits the round-trip. -func fetchIssueFieldValuesByNodeID(ctx context.Context, gqlClient *githubv4.Client, issues []*github.Issue) (map[string][]MinimalIssueFieldValue, error) { +func fetchIssueFieldValuesByNodeID(ctx context.Context, gqlClient *githubv4.Client, issues []*github.Issue) (map[string][]MinimalFieldValue, error) { ids := make([]githubv4.ID, 0, len(issues)) for _, iss := range issues { if iss == nil || iss.NodeID == nil || *iss.NodeID == "" { @@ -1215,15 +1532,15 @@ func fetchIssueFieldValuesByNodeID(ctx context.Context, gqlClient *githubv4.Clie return nil, err } - result := make(map[string][]MinimalIssueFieldValue, len(q.Nodes)) + result := make(map[string][]MinimalFieldValue, len(q.Nodes)) for _, n := range q.Nodes { idStr, ok := n.Issue.ID.(string) if !ok || idStr == "" { continue } - vals := make([]MinimalIssueFieldValue, 0, len(n.Issue.IssueFieldValues.Nodes)) + vals := make([]MinimalFieldValue, 0, len(n.Issue.IssueFieldValues.Nodes)) for _, fv := range n.Issue.IssueFieldValues.Nodes { - if m, ok := fragmentToMinimalIssueFieldValue(fv); ok { + if m, ok := fragmentToMinimalFieldValue(fv); ok { vals = append(vals, m) } } @@ -1261,7 +1578,7 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil } - var fieldValuesByID map[string][]MinimalIssueFieldValue + var fieldValuesByID map[string][]MinimalFieldValue if len(result.Issues) > 0 { gqlClient, err := deps.GetGQLClient(ctx) if err != nil { @@ -1392,6 +1709,27 @@ Options are: Type: "number", Description: "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.", }, + "issue_fields": { + Type: "array", + Description: "Issue field values to set. Each item requires field_name and either value or field_option_name. field_option_name is for single-select fields and is resolved to the corresponding option ID automatically.", + Items: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "field_name": { + Type: "string", + Description: "Issue field name", + }, + "value": { + Description: "Value for text/number/date/single-select fields. For single-select, you can use field_option_name instead.", + }, + "field_option_name": { + Type: "string", + Description: "Single-select option name to resolve and set for the field", + }, + }, + Required: []string{"field_name"}, + }, + }, }, Required: []string{"method", "owner", "repo"}, }, @@ -1493,6 +1831,11 @@ Options are: return utils.NewToolResultError("duplicate_of can only be used when state_reason is 'duplicate'"), nil, nil } + issueFields, err := optionalIssueWriteFields(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil @@ -1503,16 +1846,21 @@ Options are: return utils.NewToolResultErrorFromErr("failed to get GraphQL client", err), nil, nil } + issueFieldValues, err := resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil + } + switch method { case "create": - result, err := CreateIssue(ctx, client, owner, repo, title, body, assignees, labels, milestoneNum, issueType) + result, err := CreateIssue(ctx, client, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues) return result, nil, err case "update": issueNumber, err := RequiredInt(args, "issue_number") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, state, stateReason, duplicateOf) + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, state, stateReason, duplicateOf) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -1522,17 +1870,18 @@ Options are: return st } -func CreateIssue(ctx context.Context, client *github.Client, owner string, repo string, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string) (*mcp.CallToolResult, error) { +func CreateIssue(ctx context.Context, client *github.Client, owner string, repo string, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue) (*mcp.CallToolResult, error) { if title == "" { return utils.NewToolResultError("missing required parameter: title"), nil } // Create the issue request issueRequest := &github.IssueRequest{ - Title: github.Ptr(title), - Body: github.Ptr(body), - Assignees: &assignees, - Labels: &labels, + Title: github.Ptr(title), + Body: github.Ptr(body), + Assignees: &assignees, + Labels: &labels, + IssueFieldValues: issueFieldValues, } if milestoneNum != 0 { @@ -1575,7 +1924,7 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo return utils.NewToolResultText(string(r)), nil } -func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, state string, stateReason string, duplicateOf int) (*mcp.CallToolResult, error) { +func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, state string, stateReason string, duplicateOf int) (*mcp.CallToolResult, error) { // Create the issue request with only provided fields issueRequest := &github.IssueRequest{} @@ -1604,6 +1953,10 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.Type = github.Ptr(issueType) } + if len(issueFieldValues) > 0 { + issueRequest.IssueFieldValues = issueFieldValues + } + updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, @@ -1700,7 +2053,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 return utils.NewToolResultText(string(r)), nil } -// ListIssues creates a tool to list and filter repository issues +// ListIssues creates a tool to list and filter repository issues. This variant is +// gated by FeatureFlagIssueFields and exposes the Issues 2.0 field_filters input +// plus field_values output enrichment. When the flag is off, LegacyListIssues is +// served instead. Both registrations share the tool name "list_issues" and rely on +// the inventory's feature-flag filter to make exactly one active at a time. func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ Type: "object", @@ -1762,7 +2119,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } WithCursorPagination(schema) - return NewTool( + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "list_issues", @@ -1962,6 +2319,211 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } return result, nil, nil }) + st.FeatureFlagEnable = FeatureFlagIssueFields + return st +} + +// LegacyListIssues is the FeatureFlagIssueFields-disabled variant of list_issues. +// It exposes the pre-Issues-2.0 schema (no field_filters) and uses a GraphQL query +// path that does not select issueFieldValues or pass the issue_fields filter, so +// the request does not depend on server-side issue_fields features and does not pay +// for custom field values when the flag is off. Both this and ListIssues register +// under the tool name "list_issues"; exactly one is active for any given request +// thanks to mutually exclusive FeatureFlagEnable / FeatureFlagDisable annotations. +// Delete this function (and the rest of the Legacy* block) when the flag is removed. +func LegacyListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "state": { + Type: "string", + Description: "Filter by state, by default both open and closed issues are returned when not provided", + Enum: []any{"OPEN", "CLOSED"}, + }, + "labels": { + Type: "array", + Description: "Filter by labels", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, + "orderBy": { + Type: "string", + Description: "Order issues by field. If provided, the 'direction' also needs to be provided.", + Enum: []any{"CREATED_AT", "UPDATED_AT", "COMMENTS"}, + }, + "direction": { + Type: "string", + Description: "Order direction. If provided, the 'orderBy' also needs to be provided.", + Enum: []any{"ASC", "DESC"}, + }, + "since": { + Type: "string", + Description: "Filter by date (ISO 8601 timestamp)", + }, + }, + Required: []string{"owner", "repo"}, + } + WithCursorPagination(schema) + + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "list_issues", + Description: t("TOOL_LIST_ISSUES_DESCRIPTION", "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_LIST_ISSUES_USER_TITLE", "List issues"), + ReadOnlyHint: true, + }, + InputSchema: schema, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + state, err := OptionalParam[string](args, "state") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + state = strings.ToUpper(state) + var states []githubv4.IssueState + switch state { + case "OPEN", "CLOSED": + states = []githubv4.IssueState{githubv4.IssueState(state)} + default: + states = []githubv4.IssueState{githubv4.IssueStateOpen, githubv4.IssueStateClosed} + } + + labels, err := OptionalStringArrayParam(args, "labels") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + orderBy, err := OptionalParam[string](args, "orderBy") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + direction, err := OptionalParam[string](args, "direction") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + orderBy = strings.ToUpper(orderBy) + switch orderBy { + case "CREATED_AT", "UPDATED_AT", "COMMENTS": + default: + orderBy = "CREATED_AT" + } + direction = strings.ToUpper(direction) + switch direction { + case "ASC", "DESC": + default: + direction = "DESC" + } + + since, err := OptionalParam[string](args, "since") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + var sinceTime time.Time + var hasSince bool + if since != "" { + sinceTime, err = parseISOTimestamp(since) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), nil, nil + } + hasSince = true + } + hasLabels := len(labels) > 0 + + pagination, err := OptionalCursorPaginationParams(args) + if err != nil { + return nil, nil, err + } + if _, pageProvided := args["page"]; pageProvided { + return utils.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil, nil + } + _, perPageProvided := args["perPage"] + paginationExplicit := perPageProvided + paginationParams, err := pagination.ToGraphQLParams() + if err != nil { + return nil, nil, err + } + if !paginationExplicit { + defaultFirst := int32(DefaultGraphQLPageSize) + paginationParams.First = &defaultFirst + } + + client, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil, nil + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "states": states, + "orderBy": githubv4.IssueOrderField(orderBy), + "direction": githubv4.OrderDirection(direction), + "first": githubv4.Int(*paginationParams.First), + } + if paginationParams.After != nil { + vars["after"] = githubv4.String(*paginationParams.After) + } else { + vars["after"] = (*githubv4.String)(nil) + } + if hasLabels { + labelStrings := make([]githubv4.String, len(labels)) + for i, label := range labels { + labelStrings[i] = githubv4.String(label) + } + vars["labels"] = labelStrings + } + if hasSince { + vars["since"] = githubv4.DateTime{Time: sinceTime} + } + + issueQuery := getLegacyIssueQueryType(hasLabels, hasSince) + if err := client.Query(ctx, issueQuery, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse( + ctx, + "failed to list issues", + err, + ), nil, nil + } + + var resp MinimalIssuesResponse + var isPrivate bool + if queryResult, ok := issueQuery.(LegacyIssueQueryResult); ok { + resp = convertLegacyToMinimalIssuesResponse(queryResult.GetLegacyIssueFragment()) + isPrivate = queryResult.GetIsPrivate() + } + + result := MarshalledTextResult(resp) + if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { + if result.Meta == nil { + result.Meta = mcp.Meta{} + } + result.Meta["ifc"] = ifc.LabelListIssues(isPrivate) + } + return result, nil, nil + }) + st.FeatureFlagDisable = FeatureFlagIssueFields + return st } // rawFieldFilter is the user-supplied {field_name, value} pair before type resolution. diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 4f08b7214..1b51bd88f 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -393,6 +393,90 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) { }) } +func Test_GetIssue_FieldValues(t *testing.T) { + // Verify that issue_field_values from the REST API are present in the returned object. + serverTool := IssueRead(translations.NullTranslationHelper) + + mockIssueWithFields := &github.Issue{ + Number: github.Ptr(99), + Title: github.Ptr("Issue with field values"), + Body: github.Ptr("body"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/99"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + IssueFieldValues: []*github.IssueFieldValue{ + { + IssueFieldID: 1001, + NodeID: "FV_node_1", + DataType: "single_select", + Value: "High", + SingleSelectOption: &github.IssueFieldValueSingleSelectOption{ + ID: 42, + Name: "High", + Color: "red", + }, + }, + { + IssueFieldID: 1002, + NodeID: "FV_node_2", + DataType: "text", + Value: "some text value", + }, + }, + } + + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssueWithFields), + }) + + cache := stubRepoAccessCache(nil, 15*time.Minute) + flags := stubFeatureFlags(map[string]bool{"lockdown-mode": false}) + deps := BaseDeps{ + Client: mustNewGHClient(t, mockedClient), + GQLClient: defaultGQLClient, + RepoAccessCache: cache, + Flags: flags, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(99), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.NotNil(t, result) + + textContent := getTextResult(t, result) + + var returnedIssue MinimalIssue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + + require.Len(t, returnedIssue.IssueFieldValues, 2, "expected two issue field values") + + first := returnedIssue.IssueFieldValues[0] + assert.Equal(t, int64(1001), first.IssueFieldID) + assert.Equal(t, "FV_node_1", first.NodeID) + assert.Equal(t, "single_select", first.DataType) + assert.Equal(t, "High", first.Value) + require.NotNil(t, first.SingleSelectOption) + assert.Equal(t, int64(42), first.SingleSelectOption.ID) + assert.Equal(t, "High", first.SingleSelectOption.Name) + assert.Equal(t, "red", first.SingleSelectOption.Color) + + second := returnedIssue.IssueFieldValues[1] + assert.Equal(t, int64(1002), second.IssueFieldID) + assert.Equal(t, "FV_node_2", second.NodeID) + assert.Equal(t, "text", second.DataType) + assert.Equal(t, "some text value", second.Value) + assert.Nil(t, second.SingleSelectOption) +} + func Test_AddIssueComment(t *testing.T) { // Verify tool definition once serverTool := AddIssueComment(translations.NullTranslationHelper) @@ -1082,8 +1166,9 @@ func Test_SearchIssues_FieldValuesEnrichment(t *testing.T) { gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: gqlClient, + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + featureChecker: featureCheckerFor(FeatureFlagIssueFields), } handler := serverTool.Handler(deps) @@ -1102,7 +1187,7 @@ func Test_SearchIssues_FieldValuesEnrichment(t *testing.T) { require.Equal(t, 2, *response.Total) require.Len(t, response.Items, 2) assert.Equal(t, 42, *response.Items[0].Number) - assert.Equal(t, []MinimalIssueFieldValue{ + assert.Equal(t, []MinimalFieldValue{ {Field: "priority", Value: "P1"}, {Field: "estimate", Value: "2.5"}, }, response.Items[0].FieldValues) @@ -1127,6 +1212,7 @@ func Test_CreateIssue(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "labels") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "milestone") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "type") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_fields") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo"}) // Setup mock issue for success case @@ -1143,12 +1229,13 @@ func Test_CreateIssue(t *testing.T) { } tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]any - expectError bool - expectedIssue *github.Issue - expectedErrMsg string + name string + mockedClient *http.Client + mockedGQLClient *http.Client + requestArgs map[string]any + expectError bool + expectedIssue *github.Issue + expectedErrMsg string }{ { name: "successful issue creation with all fields", @@ -1203,6 +1290,77 @@ func Test_CreateIssue(t *testing.T) { State: github.Ptr("open"), }, }, + { + name: "successful issue creation with issue fields reconciled by names", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{ + "title": "Issue with fields", + "body": "", + "labels": []any{}, + "assignees": []any{}, + "issue_field_values": []any{ + map[string]any{"field_id": float64(101), "value": float64(9001)}, + map[string]any{"field_id": float64(102), "value": "Acme"}, + }, + }).andThen( + mockResponse(t, http.StatusCreated, &github.Issue{ + Number: github.Ptr(125), + Title: github.Ptr("Issue with fields"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"), + State: github.Ptr("open"), + }), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + issueFieldWriteMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "fullDatabaseId": "101", + "name": "Priority", + "dataType": "single_select", + "options": []any{ + map[string]any{"fullDatabaseId": "9001", "name": "P1"}, + }, + }, + map[string]any{ + "__typename": "IssueFieldText", + "fullDatabaseId": "102", + "name": "Customer", + "dataType": "text", + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Issue with fields", + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "field_option_name": "P1"}, + map[string]any{"field_name": "Customer", "value": "Acme"}, + }, + }, + expectError: false, + expectedIssue: &github.Issue{ + Number: github.Ptr(125), + Title: github.Ptr("Issue with fields"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"), + State: github.Ptr("open"), + }, + }, { name: "issue creation fails", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -1220,13 +1378,32 @@ func Test_CreateIssue(t *testing.T) { expectError: false, expectedErrMsg: "missing required parameter: title", }, + { + name: "issue_fields rejects both value and field_option_name", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Invalid fields", + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "value": "P1", "field_option_name": "P1"}, + }, + }, + expectError: false, + expectedErrMsg: "cannot specify both value and field_option_name", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := mustNewGHClient(t, tc.mockedClient) - gqlClient := githubv4.NewClient(nil) + gqlHTTPClient := tc.mockedGQLClient + if gqlHTTPClient == nil { + gqlHTTPClient = githubv4mock.NewMockedHTTPClient() + } + gqlClient := githubv4.NewClient(gqlHTTPClient) deps := BaseDeps{ Client: client, GQLClient: gqlClient, @@ -1446,7 +1623,8 @@ func Test_ListIssues(t *testing.T) { // Verify tool definition serverTool := ListIssues(translations.NullTranslationHelper) tool := serverTool.Tool - require.NoError(t, toolsnaps.Test(tool.Name, tool)) + require.NoError(t, toolsnaps.Test(tool.Name+"_ff_"+FeatureFlagIssueFields, tool)) + require.Equal(t, FeatureFlagIssueFields, serverTool.FeatureFlagEnable) assert.Equal(t, "list_issues", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1809,9 +1987,9 @@ func Test_ListIssues(t *testing.T) { // (including float formatting); #789 has no field values. switch issue.Number { case 123: - assert.Equal(t, []MinimalIssueFieldValue{{Field: "priority", Value: "P1"}}, issue.FieldValues) + assert.Equal(t, []MinimalFieldValue{{Field: "priority", Value: "P1"}}, issue.FieldValues) case 456: - assert.Equal(t, []MinimalIssueFieldValue{ + assert.Equal(t, []MinimalFieldValue{ {Field: "due", Value: "2026-06-01"}, {Field: "estimate", Value: "2.5"}, {Field: "notes", Value: "needs triage"}, @@ -2363,6 +2541,95 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { }) } +func Test_LegacyListIssues_Definition(t *testing.T) { + serverTool := LegacyListIssues(translations.NullTranslationHelper) + tool := serverTool.Tool + + // LegacyListIssues claims the base tool name "list_issues" and produces the + // FeatureFlagIssueFields-disabled schema (no field_filters). It owns the + // canonical list_issues.snap; the FeatureFlagIssueFields-enabled variant + // owns list_issues_ff_.snap. + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + require.Equal(t, "list_issues", tool.Name) + require.Equal(t, FeatureFlagIssueFields, serverTool.FeatureFlagDisable) + require.Empty(t, serverTool.FeatureFlagEnable) + + props := tool.InputSchema.(*jsonschema.Schema).Properties + assert.Contains(t, props, "owner") + assert.Contains(t, props, "repo") + assert.Contains(t, props, "state") + assert.Contains(t, props, "labels") + assert.Contains(t, props, "since") + assert.NotContains(t, props, "field_filters", "legacy list_issues must not advertise field_filters") +} + +func Test_LegacyListIssues_OmitsFieldValuesAndFilters(t *testing.T) { + t.Parallel() + + serverTool := LegacyListIssues(translations.NullTranslationHelper) + + mockIssues := []map[string]any{ + { + "number": 7, + "title": "Legacy issue", + "body": "body", + "state": "OPEN", + "databaseId": 7, + "createdAt": "2026-01-01T00:00:00Z", + "updatedAt": "2026-01-01T00:00:00Z", + "author": map[string]any{"login": "octocat"}, + "labels": map[string]any{"nodes": []map[string]any{}}, + "comments": map[string]any{"totalCount": 0}, + }, + } + pageInfo := map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "c1", + "endCursor": "c1", + } + + // The legacy query must NOT reference issueFieldValues (neither in the selection + // set nor in filterBy). The matcher's query string therefore omits both. + const legacyQuery = "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + vars := map[string]any{ + "owner": "owner", + "repo": "repo", + "states": []any{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": nil, + } + response := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "isPrivate": false, + "issues": map[string]any{ + "nodes": mockIssues, + "pageInfo": pageInfo, + "totalCount": 1, + }, + }, + }) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher(legacyQuery, vars, response))) + + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, "expected non-error result; got: %v", getTextResult(t, result).Text) + + var resp MinimalIssuesResponse + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &resp)) + require.Len(t, resp.Issues, 1) + assert.Equal(t, 7, resp.Issues[0].Number) + assert.Nil(t, resp.Issues[0].FieldValues, "legacy list_issues must not return field_values") +} + func Test_UpdateIssue(t *testing.T) { // Verify tool definition serverTool := IssueWrite(translations.NullTranslationHelper) @@ -2384,6 +2651,7 @@ func Test_UpdateIssue(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "state") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "state_reason") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "duplicate_of") + assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_fields") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo"}) // Mock issues for reuse across test cases @@ -2495,6 +2763,63 @@ func Test_UpdateIssue(t *testing.T) { expectError: false, expectedIssue: mockUpdatedIssue, }, + { + name: "partial update with issue fields reconciled by names", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "issue_field_values": []any{ + map[string]any{"field_id": float64(101), "value": float64(9001)}, + map[string]any{"field_id": float64(102), "value": "Acme"}, + }, + "title": "Updated Title", + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedIssue), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + issueFieldWriteMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "fullDatabaseId": "101", + "name": "Priority", + "dataType": "single_select", + "options": []any{map[string]any{"fullDatabaseId": "9001", "name": "P1"}}, + }, + map[string]any{ + "__typename": "IssueFieldText", + "fullDatabaseId": "102", + "name": "Customer", + "dataType": "text", + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "title": "Updated Title", + "issue_fields": []any{ + map[string]any{"field_name": "Priority", "field_option_name": "P1"}, + map[string]any{"field_name": "Customer", "value": "Acme"}, + }, + }, + expectError: false, + expectedIssue: mockUpdatedIssue, + }, { name: "issue not found when updating non-state fields only", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index bad5196a9..5ad7656f0 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -220,6 +220,31 @@ type MinimalReactions struct { Eyes int `json:"eyes"` } +// MinimalIssueFieldValueSingleSelectOption is the trimmed output type for a single-select option of an issue field value. +type MinimalIssueFieldValueSingleSelectOption struct { + ID int64 `json:"id"` + Name string `json:"name"` + Color string `json:"color"` +} + +// MinimalIssueFieldValue is the trimmed output type for a custom field value attached to an issue, +// populated from REST API responses (e.g. get_issue). For GraphQL-sourced field values see MinimalFieldValue. +type MinimalIssueFieldValue struct { + IssueFieldID int64 `json:"issue_field_id,omitempty"` + NodeID string `json:"node_id,omitempty"` + DataType string `json:"data_type,omitempty"` + Value any `json:"value,omitempty"` + SingleSelectOption *MinimalIssueFieldValueSingleSelectOption `json:"single_select_option,omitempty"` +} + +// MinimalFieldValue is the trimmed output type for a custom field value resolved via GraphQL +// (e.g. list_issues, search_issues). Single-value variants populate Value; Values is reserved for multi-select. +type MinimalFieldValue struct { + Field string `json:"field"` + Value string `json:"value,omitempty"` + Values []string `json:"values,omitempty"` +} + // MinimalIssue is the trimmed output type for issue objects to reduce verbosity. type MinimalIssue struct { Number int `json:"number"` @@ -242,15 +267,8 @@ type MinimalIssue struct { ClosedAt string `json:"closed_at,omitempty"` ClosedBy string `json:"closed_by,omitempty"` IssueType string `json:"issue_type,omitempty"` - FieldValues []MinimalIssueFieldValue `json:"field_values,omitempty"` -} - -// MinimalIssueFieldValue is the trimmed output type for a custom issue field value. -// Single-value variants (date, number, single-select, text) populate Value. Values is reserved for multi-select. -type MinimalIssueFieldValue struct { - Field string `json:"field"` - Value string `json:"value,omitempty"` - Values []string `json:"values,omitempty"` + IssueFieldValues []MinimalIssueFieldValue `json:"issue_field_values,omitempty"` + FieldValues []MinimalFieldValue `json:"field_values,omitempty"` } // MinimalIssuesResponse is the trimmed output for a paginated list of issues. @@ -435,6 +453,26 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue { m.IssueType = issueType.GetName() } + for _, fv := range issue.IssueFieldValues { + if fv == nil { + continue + } + mfv := MinimalIssueFieldValue{ + IssueFieldID: fv.IssueFieldID, + NodeID: fv.NodeID, + DataType: fv.DataType, + Value: fv.Value, + } + if opt := fv.SingleSelectOption; opt != nil { + mfv.SingleSelectOption = &MinimalIssueFieldValueSingleSelectOption{ + ID: opt.ID, + Name: opt.Name, + Color: opt.Color, + } + } + m.IssueFieldValues = append(m.IssueFieldValues, mfv) + } + if r := issue.Reactions; r != nil { m.Reactions = &MinimalReactions{ TotalCount: r.GetTotalCount(), @@ -471,7 +509,7 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { } for _, fv := range fragment.IssueFieldValues.Nodes { - if mfv, ok := fragmentToMinimalIssueFieldValue(fv); ok { + if mfv, ok := fragmentToMinimalFieldValue(fv); ok { m.FieldValues = append(m.FieldValues, mfv) } } @@ -479,32 +517,32 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { return m } -// fragmentToMinimalIssueFieldValue flattens the union value fragment into a single +// fragmentToMinimalFieldValue flattens the union value fragment into a single // {field, value} pair. Returns ok=false if the typename is unrecognised. -func fragmentToMinimalIssueFieldValue(fv IssueFieldValueFragment) (MinimalIssueFieldValue, bool) { +func fragmentToMinimalFieldValue(fv IssueFieldValueFragment) (MinimalFieldValue, bool) { switch fv.TypeName { case "IssueFieldDateValue": - return MinimalIssueFieldValue{ + return MinimalFieldValue{ Field: fv.DateValue.Field.Name(), Value: string(fv.DateValue.Value), }, true case "IssueFieldNumberValue": - return MinimalIssueFieldValue{ + return MinimalFieldValue{ Field: fv.NumberValue.Field.Name(), Value: strconv.FormatFloat(float64(fv.NumberValue.Value), 'f', -1, 64), }, true case "IssueFieldSingleSelectValue": - return MinimalIssueFieldValue{ + return MinimalFieldValue{ Field: fv.SingleSelectValue.Field.Name(), Value: string(fv.SingleSelectValue.Value), }, true case "IssueFieldTextValue": - return MinimalIssueFieldValue{ + return MinimalFieldValue{ Field: fv.TextValue.Field.Name(), Value: string(fv.TextValue.Value), }, true } - return MinimalIssueFieldValue{}, false + return MinimalFieldValue{}, false } func convertToMinimalIssuesResponse(fragment IssueQueryFragment) MinimalIssuesResponse { @@ -525,6 +563,51 @@ func convertToMinimalIssuesResponse(fragment IssueQueryFragment) MinimalIssuesRe } } +// legacyFragmentToMinimalIssue converts the FeatureFlagIssueFields-disabled +// LegacyIssueFragment into a MinimalIssue. MinimalIssue.FieldValues is left +// nil so omitempty drops it from JSON output. Delete with the rest of the +// Legacy* block when the flag is removed. +func legacyFragmentToMinimalIssue(fragment LegacyIssueFragment) MinimalIssue { + m := MinimalIssue{ + Number: int(fragment.Number), + Title: sanitize.Sanitize(string(fragment.Title)), + Body: sanitize.Sanitize(string(fragment.Body)), + State: string(fragment.State), + Comments: int(fragment.Comments.TotalCount), + CreatedAt: fragment.CreatedAt.Format(time.RFC3339), + UpdatedAt: fragment.UpdatedAt.Format(time.RFC3339), + User: &MinimalUser{ + Login: string(fragment.Author.Login), + }, + } + + for _, label := range fragment.Labels.Nodes { + m.Labels = append(m.Labels, string(label.Name)) + } + + return m +} + +// convertLegacyToMinimalIssuesResponse mirrors convertToMinimalIssuesResponse for +// the FeatureFlagIssueFields-disabled list_issues variant. +func convertLegacyToMinimalIssuesResponse(fragment LegacyIssueQueryFragment) MinimalIssuesResponse { + minimalIssues := make([]MinimalIssue, 0, len(fragment.Nodes)) + for _, issue := range fragment.Nodes { + minimalIssues = append(minimalIssues, legacyFragmentToMinimalIssue(issue)) + } + + return MinimalIssuesResponse{ + Issues: minimalIssues, + TotalCount: fragment.TotalCount, + PageInfo: MinimalPageInfo{ + HasNextPage: bool(fragment.PageInfo.HasNextPage), + HasPreviousPage: bool(fragment.PageInfo.HasPreviousPage), + StartCursor: string(fragment.PageInfo.StartCursor), + EndCursor: string(fragment.PageInfo.EndCursor), + }, + } +} + func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment { m := MinimalIssueComment{ ID: comment.GetID(), diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 70dfab8d9..49edb00ff 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -204,6 +204,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { IssueRead(t), SearchIssues(t), ListIssues(t), + LegacyListIssues(t), ListIssueTypes(t), ListIssueFields(t), IssueWrite(t), diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index d54b3f12d..d147cbfc6 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -167,6 +167,19 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { return r.toolsetDescriptions } +// ToolsForRegistration returns AvailableTools(ctx) post-processed exactly as +// RegisterTools would expose them: with MCP Apps UI metadata stripped when +// the remote_mcp_ui_apps feature flag is not enabled in ctx. Useful for +// documentation generators and diagnostics that need the same view of the +// tool surface the server would register. +func (r *Inventory) ToolsForRegistration(ctx context.Context) []ServerTool { + tools := r.AvailableTools(ctx) + if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { + tools = stripMCPAppsMetadata(tools) + } + return tools +} + // RegisterTools registers all available tools with the server using the provided dependencies. // The context is used for feature flag evaluation. // @@ -177,11 +190,7 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { // from ctx would otherwise see context.Background() and falsely report the // flag off, even when the actual request arrived on the /insiders route. func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any) { - tools := r.AvailableTools(ctx) - if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) { - tools = stripMCPAppsMetadata(tools) - } - for _, tool := range tools { + for _, tool := range r.ToolsForRegistration(ctx) { tool.RegisterFunc(s, deps) } }