Main #1778
Closed
Main #1778
+29,010
−13,023
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add enabledTools to StdioServerConfig * add EnabledTools to MCPServerConfig, and logic to bypass toolset config if present * add logic to register specific tools * update readme * Update to be consistent with: https://docs.google.com/document/d/1tOOBJ4y9xY61QVrO18ymuVt4SO9nV-z2B4ckaL2f9IU/edit?tab=t.0#heading=h.ffto4e5dwzlf specifically - allow for --tools and dynamic toolset mode together - allow for --tools and --toolsets together * go mod tidy * update * clean up comment * fix * fix * updte * update * clean up
…beler (github#1498) * Initial plan * Add AI issue assessment workflow with github/ai-assessment-comment-labeler Co-authored-by: JoannaaKL <[email protected]> * Remove AI issue assessment workflow and prompt file per review feedback Co-authored-by: JoannaaKL <[email protected]> * Restore workflow and prompt, run on bug label or unlabeled issues Co-authored-by: JoannaaKL <[email protected]> * Update .github/workflows/ai-issue-assessment.yml Co-authored-by: Copilot <[email protected]> * Add default-issue-review.prompt.yml for unlabeled issues Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> Co-authored-by: JoannaaKL <[email protected]> Co-authored-by: Copilot <[email protected]>
* small doc update to add x-mcp-tools header * update * add server configuration guide * update * Update docs/server-configuration.md Co-authored-by: Copilot <[email protected]> * Update docs/remote-server.md Co-authored-by: Copilot <[email protected]> * Update docs/server-configuration.md Co-authored-by: Copilot <[email protected]> * Update README.md Co-authored-by: Copilot <[email protected]> * Update docs/server-configuration.md Co-authored-by: Copilot <[email protected]> * Update docs/server-configuration.md Co-authored-by: Copilot <[email protected]> * Update docs/server-configuration.md Co-authored-by: Tony Truong <[email protected]> * use relative paths * use relative path --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Tony Truong <[email protected]>
* Initial plan * Add ai_review_label to ai-issue-assessment.yml Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]>
Move from `mark3labs/mcp-go` to `modelcontextprotocol/go-sdk`. This is mostly focused on updating tool schema and tool handler signatures, along with any associated internal changes related to those changes. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: omgitsads <[email protected]> Co-authored-by: LuluBeatson <[email protected]> Co-authored-by: Lulu <[email protected]> Co-authored-by: SamMorrowDrums <[email protected]> Co-authored-by: Sam Morrow <[email protected]>
Port resource completion from the remote GitHub MCP Server --- Co-authored-by: Ksenia Bobrova <[email protected]>
* adding docs to github enterprise with oauth * adding necessary config for oauth proxima * Update README.md Co-authored-by: Copilot <[email protected]> * use better link --------- Co-authored-by: Copilot <[email protected]>
* Add tool handler shim to use server.AddTool instead of mcp.AddTool --- Co-authored-by: Ksenia Bobrova <[email protected]> Co-authored-by: Sam Morrow <[email protected]>
OpenAI strict mode requires the `properties` field to be present in
object schemas, even when empty. The jsonschema library uses omitempty
which causes empty maps to be omitted during JSON serialization.
This change uses json.RawMessage to bypass the library's serialization
and explicitly include `"properties": {}` in the output.
Fixes github#1548
* Correct lower-case issue state * Uppercase orderBy and direction as well * Auto-correct invalid enum parameters
* add suppport for tool aliases for backwards compatibility when tool names change * cleanup * log alias usage as warning * remove comments * remove mock data * remove unused code, move deprecated tool aliases to its own file * remove unused code and add tests * resolve tool aliases in its own explicit step * improve logic by returning aliases used in resolvetoolaliases * remove unused function * remove comments * remove comment * Update pkg/github/deprecated_tool_aliases.go Co-authored-by: Copilot <[email protected]> * restore comment --------- Co-authored-by: Copilot <[email protected]>
When tool parameter descriptions span multiple lines, the continuation lines now receive proper indentation to maintain markdown list formatting. This fixes the rendering issue where multi-line descriptions would break out of the parameter list structure. Fixes github#1494
The go-SDK migration changed MCP protocol handling to require proper initialization before tool calls. This updates the script to: - Add initialize request with protocol version and client info - Add notifications/initialized notification - Add arguments field to tools/call params - Keep stdin open with sleep for response - Gracefully handle missing jq dependency
The licenses script now: - Generates separate license reports per GOOS/GOARCH combination - Groups identical reports together (comma-separated arch names) - Adds a Table of Contents at the top of each platform file - Handles cases where different architectures have different dependencies (e.g., x/sys/unix vs x/sys/windows, mousetrap on Windows only) This addresses the issue discovered in cli/cli where some deps changed which changed the mod graph for different GOARCH and affected the exported licenses because go-licenses tries to find common ancestors.
Address review feedback: - Remove bash 4.0+ associative array requirement for macOS compatibility - Add cross-platform hash function (md5sum on Linux, md5 on macOS) - Ensure deterministic iteration order using sorted groups file - Add better error handling for failed go-licenses commands - Fix grammar: 'architecture(s)' -> 'architectures' - Add documentation for third-party/ being a union of all architectures - Use file-based state instead of associative arrays for portability
- Check now regenerates using ./script/licenses and compares - Add GOROOT/PATH setup in CI to fix go-licenses module info errors - Check both license files AND third-party directory for changes - See: google/go-licenses#244
The sort command uses locale-specific ordering which can differ between systems. Use LC_ALL=C to ensure consistent ordering in CI and locally.
* Improvements & refactoring of get_file_contents * Fix logical path when file or directory not found * Fix comment * Docs update * Do file matching when raw API returns error
* adding review comments grouped as threads * minor fix * minor edit * update docs * fix docs * increase limit to 100 * fixtext
* Add repos toolset instructions * 1. Make sha optional (if not supplied - GetContents is used to retrieve original sha) 2. Instruct LLM to supply actual sha using git command and move instructions to tool description. * Update toolsnaps and docs * Better error handling * Add tests * Clearing resources in time * Addressing review comments && checking etag * Test fixes
* docs: add Antigravity installation guide * docs: improve antigravity guide and address feedback * Update docs/installation-guides/install-antigravity.md Co-authored-by: Copilot <[email protected]> * Update docs/installation-guides/install-antigravity.md Co-authored-by: Iryna Kulakova <[email protected]> --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Iryna Kulakova <[email protected]>
* update docs * Update docs/tool-renaming.md Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
* Update pr template * Add prompts tested section to PR template (github#1617) * Initial plan * Add prompts tested section to PR template for tool changes Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
…ion (github#1606) * Initial plan * Add testify/mock-based HTTP mocking infrastructure and migrate git_test.go - Add MockHTTPClientWithHandlers helper function for HTTP-level mocking - Add path pattern matching support for GitHub API endpoints - Migrate pkg/github/git_test.go from go-github-mock to new infrastructure - Keep go-github-mock dependency for now (other files still use it) Co-authored-by: JoannaaKL <[email protected]> * Complete pilot migration and add migration documentation - Migrate pkg/github/code_scanning_test.go to new infrastructure - Add comprehensive migration documentation in docs/testing-migration.md - Fix linter warning in helper_test.go - All tests and lint checks pass - 2 of 16 test files migrated, 14 remaining for incremental migration Co-authored-by: JoannaaKL <[email protected]> * Address code review feedback: consolidate transport implementations - Remove unused OnAny method - Extract executeHandler helper to eliminate code duplication - Consolidate MockHTTPClientWithHandler to use MockHTTPClientWithHandlers - Simplify transport implementation (single multiHandlerTransport) - All tests and lint checks pass Co-authored-by: JoannaaKL <[email protected]> * Remove docs/testing-migration.md per review feedback Co-authored-by: JoannaaKL <[email protected]> * Reuse mock.EndpointPattern constants instead of hardcoded paths - Add mock import to code_scanning_test.go and git_test.go - Replace hardcoded paths with mock.GetRepos*.Pattern references - Ensures consistency with existing test patterns and easier maintenance Co-authored-by: JoannaaKL <[email protected]> * Define endpoint path constants in this repo instead of reusing go-github-mock - Add endpoint pattern constants to helper_test.go - Update code_scanning_test.go to use new constants - Update git_test.go to use new constants - Removes dependency on go-github-mock endpoint patterns - Prepares for eventual removal of go-github-mock dependency Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> Co-authored-by: JoannaaKL <[email protected]>
Bumps [actions/cache](https://github.com/actions/cache) from 4 to 5. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v4...v5) --- updated-dependencies: - dependency-name: actions/cache dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…r-request optimization (github#1589) * refactor: separate ServerTool into own file with HandlerFunc pattern - Extract ServerTool struct into pkg/toolsets/server_tool.go - Add ToolDependencies struct for passing common dependencies to handlers - HandlerFunc allows lazy handler generation from Tool definitions - NewServerTool for new dependency-based tools - NewServerToolLegacy for backward compatibility with existing handlers - Update toolsets.go to store and pass dependencies - Update all call sites to use NewServerToolLegacy Co-authored-by: Adam Holt <[email protected]> * Wire ToolDependencies through toolsets - Move ToolDependencies to pkg/github/dependencies.go with proper types - Use 'any' in toolsets package to avoid circular dependencies - Add NewTool/NewToolFromHandler helpers that isolate type assertion - Tool implementations will be fully typed with no assertions scattered - Infrastructure ready for incremental tool migration * refactor(search): migrate search tools to new ServerTool pattern Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers, SearchOrgs) to use the new NewTool helper and ToolDependencies pattern. - Functions now take only TranslationHelperFunc and return ServerTool - Handler generation uses ToolDependencies for typed access to clients - Update tools.go call sites to remove getClient parameter - Update tests to use new Handler(deps) pattern This demonstrates the migration pattern for additional tool files. Co-authored-by: Adam Holt <[email protected]> * Migrate context_tools to new ServerTool pattern (github#1590) * refactor(search): migrate search tools to new ServerTool pattern Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers, SearchOrgs) to use the new NewTool helper and ToolDependencies pattern. - Functions now take only TranslationHelperFunc and return ServerTool - Handler generation uses ToolDependencies for typed access to clients - Update tools.go call sites to remove getClient parameter - Update tests to use new Handler(deps) pattern This demonstrates the migration pattern for additional tool files. Co-authored-by: Adam Holt <[email protected]> * Migrate context_tools to new ServerTool pattern Convert GetMe, GetTeams, and GetTeamMembers to use the new typed dependency injection pattern: - Functions now take only translations helper, return toolsets.ServerTool - Handler is generated lazily via deps.GetClient/deps.GetGQLClient - Tests updated to use serverTool.Handler(deps) pattern - Fixed error return pattern to return nil for Go error (via result.IsError) Co-authored-by: Adam Holt <[email protected]> * refactor(gists): migrate gists.go to NewTool pattern (github#1591) * Migrate context_tools to new ServerTool pattern Convert GetMe, GetTeams, and GetTeamMembers to use the new typed dependency injection pattern: - Functions now take only translations helper, return toolsets.ServerTool - Handler is generated lazily via deps.GetClient/deps.GetGQLClient - Tests updated to use serverTool.Handler(deps) pattern - Fixed error return pattern to return nil for Go error (via result.IsError) Co-authored-by: Adam Holt <[email protected]> * refactor(gists): migrate gists.go to NewTool pattern Convert all gist tools (ListGists, GetGist, CreateGist, UpdateGist) to use the new NewTool helper with ToolDependencies injection. - Remove getClient parameter from function signatures - Use deps.GetClient(ctx) inside handlers - Standardize error handling with utils.NewToolResultErrorFromErr() - Update all tests to use serverTool.Handler(deps) pattern Co-authored-by: Adam Holt <[email protected]> --------- Co-authored-by: Adam Holt <[email protected]> Co-authored-by: Adam Holt <[email protected]> --------- Co-authored-by: Adam Holt <[email protected]> Co-authored-by: Adam Holt <[email protected]> * refactor(notifications): migrate notifications.go to NewTool pattern (github#1592) * refactor(notifications): migrate notifications.go to NewTool pattern Convert all notification tools to use the new NewTool helper with ToolDependencies injection. Co-authored-by: Adam Holt <[email protected]> * Refactor repositories.go tools to use NewTool pattern with ToolDependencies Convert all 18 tool functions in repositories.go to use the new NewTool helper pattern with typed ToolDependencies, isolating type assertions to a single location and improving code maintainability. Functions converted: - GetCommit, ListCommits, ListBranches - CreateOrUpdateFile, CreateRepository, GetFileContents - ForkRepository, DeleteFile, CreateBranch, PushFiles - ListTags, GetTag, ListReleases, GetLatestRelease, GetReleaseByTag - ListStarredRepositories, StarRepository, UnstarRepository This is part of a stacked PR series to systematically migrate all tool files to the new pattern. Co-authored-by: Adam Holt <[email protected]> * refactor(issues): migrate issues.go to NewTool pattern Convert all 8 tool functions in issues.go to use the new NewTool helper pattern which standardizes dependency injection: - IssueRead: GetClient, GetGQLClient, RepoAccessCache, Flags - ListIssueTypes: GetClient - AddIssueComment: GetClient - SubIssueWrite: GetClient - SearchIssues: GetClient - IssueWrite: GetClient, GetGQLClient - ListIssues: GetGQLClient - AssignCopilotToIssue: GetGQLClient Updated tools.go to use direct function calls instead of NewServerToolLegacy wrappers. Updated all tests in issues_test.go to use the new ToolDependencies pattern and Handler() method. Co-authored-by: Adam Holt <[email protected]> * refactor(pullrequests): convert PR tools to NewTool pattern Convert all 10 pull request tool functions to use the NewTool pattern with ToolDependencies injection: - PullRequestRead - CreatePullRequest - UpdatePullRequest - ListPullRequests - MergePullRequest - SearchPullRequests - UpdatePullRequestBranch - PullRequestReviewWrite - AddCommentToPendingReview - RequestCopilotReview Update tools.go to use direct function calls (removing NewServerToolLegacy wrappers) for PR functions. Update all tests in pullrequests_test.go to use the new handler pattern with deps and 2-value return. Co-authored-by: Adam Holt <[email protected]> * Refactor actions.go to use NewTool pattern Convert all 14 tool functions in actions.go to use the NewTool pattern with ToolDependencies for dependency injection. This is part of a broader effort to standardize the tool implementation pattern across the codebase. Changes: - ListWorkflows, ListWorkflowRuns, RunWorkflow, GetWorkflowRun - GetWorkflowRunLogs, ListWorkflowJobs, GetJobLogs - RerunWorkflowRun, RerunFailedJobs, CancelWorkflowRun - ListWorkflowRunArtifacts, DownloadWorkflowRunArtifact - DeleteWorkflowRunLogs, GetWorkflowRunUsage The new pattern: - Takes only translations.TranslationHelperFunc as parameter - Returns toolsets.ServerTool with Tool and Handler - Handler receives ToolDependencies for client access - Enables better testability and consistent interface Co-authored-by: Adam Holt <[email protected]> * refactor(git): migrate GetRepositoryTree to NewTool pattern * refactor(security): migrate code_scanning, secret_scanning, dependabot to NewTool pattern Co-authored-by: Adam Holt <[email protected]> * refactor(discussions): migrate to NewTool pattern Co-authored-by: Adam Holt <[email protected]> * Refactor security_advisories tools to use NewTool pattern Convert 4 functions from NewServerToolLegacy wrapper to NewTool: - ListGlobalSecurityAdvisories - GetGlobalSecurityAdvisory - ListRepositorySecurityAdvisories - ListOrgRepositorySecurityAdvisories Update tools.go toolset registration and tests. Co-authored-by: Adam Holt <[email protected]> * refactor: convert projects, labels, and dynamic_tools to NewTool pattern This PR converts projects.go, labels.go, and dynamic_tools.go from the legacy NewServerToolLegacy wrapper pattern to the new NewTool pattern with proper ToolDependencies. Changes: - projects.go: Convert all 9 project functions to use NewTool with ToolHandlerFor[map[string]any, any] and 3-return-value handlers - projects_test.go: Update tests to use new serverTool.Handler(deps) pattern - labels.go: Convert GetLabel, ListLabels, and LabelWrite to NewTool pattern - labels_test.go: Update tests to use new pattern - dynamic_tools.go: Refactor functions to return ServerTool directly (using NewServerToolLegacy internally since they have special dependencies) - tools.go: Remove NewServerToolLegacy wrappers for dynamic tools registration Co-authored-by: Adam Holt <[email protected]> * Add --features CLI flag for feature flag support Add CLI flag and config support for feature flags in the local server: - Add --features flag to main.go (StringSlice, comma-separated) - Add EnabledFeatures field to StdioServerConfig and MCPServerConfig - Create createFeatureChecker() that builds a set from enabled features - Wire WithFeatureChecker() into the toolset group filter chain This enables tools/resources/prompts that have FeatureFlagEnable set to a flag name that is passed via --features. The checker uses a simple set membership test for O(1) lookup. Usage: github-mcp-server stdio --features=my_feature,another_feature GITHUB_FEATURES=my_feature github-mcp-server stdio * Add validation tests for tools, resources, and prompts metadata This commit adds comprehensive validation tests to ensure all MCP items have required metadata: - TestAllToolsHaveRequiredMetadata: Validates Toolset.ID and Annotations - TestAllToolsHaveValidToolsetID: Ensures toolsets are in AvailableToolsets() - TestAllResourcesHaveRequiredMetadata: Validates resource metadata - TestAllPromptsHaveRequiredMetadata: Validates prompt metadata - TestToolReadOnlyHintConsistency: Validates IsReadOnly() matches annotation - TestNoDuplicate*Names: Ensures unique names across tools/resources/prompts - TestAllToolsHaveHandlerFunc: Ensures all tools have handlers - TestDefaultToolsetsAreValid: Validates default toolset IDs - TestToolsetMetadataConsistency: Ensures consistent descriptions per toolset Also fixes a bug discovered by these tests: ToolsetMetadataGit was defined but not added to AvailableToolsets(), causing get_repository_tree to have an invalid toolset ID. * Fix default toolsets behavior when not in dynamic mode When no toolsets are specified and dynamic mode is disabled, the server should use the default toolsets. The bug was introduced when adding dynamic toolsets support: 1. CleanToolsets(nil) was converting nil to empty slice 2. Empty slice passed to WithToolsets means 'no toolsets' 3. This resulted in zero tools being registered Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets) and only set empty slice when dynamic mode is enabled without explicit toolsets. * refactor: address PR review feedback for toolsets - Rename AddDeprecatedToolAliases to WithDeprecatedToolAliases for immutable filter chain consistency (returns new ToolsetGroup) - Remove unused mockGetRawClient from generate_docs.go (use nil instead) - Remove legacy ServerTool functions (NewServerToolLegacy and NewServerToolFromHandlerLegacy) - no usages - Add panic in Handler()/RegisterFunc() when HandlerFunc is nil - Add HasHandler() method for checking if tool has a handler - Add tests for HasHandler and nil handler panic behavior - Update all tests to use new WithDeprecatedToolAliases pattern * refactor: Apply HandlerFunc pattern to resources for stateless NewToolsetGroup This change applies the same HandlerFunc pattern used by tools to resources, allowing NewToolsetGroup to be fully stateless (only requiring translations). Key changes: - Add ResourceHandlerFunc type to toolsets package - Update ServerResourceTemplate to use HandlerFunc instead of direct Handler - Add HasHandler() and Handler(deps) methods to ServerResourceTemplate - Update RegisterResourceTemplates to take deps parameter - Refactor repository resource definitions to use HandlerFunc pattern - Make AllResources(t) stateless (only takes translations) - Make NewToolsetGroup(t) stateless (only takes translations) - Update generate_docs.go - no longer needs mock clients - Update tests to use new patterns This resolves the concern about mixed concerns in doc generation - the toolset metadata and resource templates can now be created without any runtime dependencies, while handlers are generated on-demand when deps are provided during registration. * refactor: simplify ForMCPRequest switch cases * refactor(generate_docs): use strings.Builder and AllTools() iteration - Replace slice joining with strings.Builder for all doc generation - Iterate AllTools() directly instead of ToolsetIDs()/ToolsForToolset() - Removes need for special 'dynamic' toolset handling (no tools = no output) - Context toolset still explicitly handled for custom description - Consistent pattern across generateToolsetsDoc, generateToolsDoc, generateRemoteToolsetsDoc, and generateDeprecatedAliasesTable * feat(toolsets): add AvailableToolsets() with exclude filter - Add AvailableToolsets() method that returns toolsets with actual tools - Support variadic exclude parameter for filtering out specific toolsets - Simplifies doc generation by removing manual skip logic - Naturally excludes empty toolsets (like 'dynamic') without special cases * refactor(generate_docs): hoist success logging to generateAllDocs * refactor: consolidate toolset validation into ToolsetGroup - Add Default field to ToolsetMetadata and derive defaults from metadata - Move toolset validation into WithToolsets (trims whitespace, dedupes, tracks unrecognized) - Add UnrecognizedToolsets() method for warning about typos - Add DefaultToolsetIDs() method to derive defaults from metadata - Remove redundant functions: CleanToolsets, GetValidToolsetIDs, AvailableToolsets, GetDefaultToolsetIDs - Update DynamicTools to take ToolsetGroup for schema enum generation - Add stubTranslator for cases needing ToolsetGroup without translations This eliminates hardcoded toolset lists - everything is now derived from the actual registered tools and their metadata. * refactor: rename toolsets package to registry with builder pattern - Rename pkg/toolsets to pkg/registry (better reflects its purpose) - Split monolithic toolsets.go into focused files: - registry.go: Core Registry struct and MCP methods - builder.go: Builder pattern for creating Registry instances - filters.go: All filtering logic (toolsets, read-only, feature flags) - resources.go: ServerResourceTemplate type - prompts.go: ServerPrompt type - errors.go: Error types - server_tool.go: ServerTool and ToolsetMetadata (existing) - Fix lint: Rename RegistryBuilder to Builder (avoid stuttering) - Update all imports across ~45 files This refactoring improves code organization and makes the registry's purpose clearer. The builder pattern provides a clean API: reg := registry.NewBuilder(). SetTools(tools). WithReadOnly(true). WithToolsets([]string{"repos"}). Build() * fix: remove unnecessary type arguments in helper_test.go * fix: restore correct behavior for --tools and --toolsets flags Two behavioral regressions were fixed in resolveEnabledToolsets(): 1. When --tools=X is used without --toolsets, the server should only register the specified tools, not the default toolsets. Now returns an empty slice instead of nil when EnabledTools is set. 2. When --toolsets=all --dynamic-toolsets is used, the 'all' and 'default' pseudo-toolsets should be removed so only the dynamic management tools are registered. This matches the original pre-refactor behavior. * Move labels tools to issues toolset Labels are closely related to issues - you add labels to issues, search issues by label, etc. Keeping them in a separate toolset required users to explicitly enable 'labels' to get this functionality. Moving to issues toolset makes labels available by default since issues is a default toolset. * Restore labels toolset with get_label in both issues and labels This restores conformance with the original behavior where: - get_label is in issues toolset (read-only label access for issue workflows) - get_label, list_label, label_write are in labels toolset (full management) The duplicate get_label registration is intentional - it was in both toolsets in the original implementation. Added test exception to allow this case. * Fix instruction generation and capability advertisement - Expand nil toolsets to default IDs before GenerateInstructions (nil means 'use defaults' in registry but instructions need actual names) - Remove unconditional HasTools/HasResources/HasPrompts=true in NewServer (let SDK determine capabilities based on registered items, matching main) * Add tests for dynamic toolset management tools Tests cover: - list_available_toolsets: verifies toolsets are listed with enabled status - get_toolset_tools: verifies tools can be retrieved for a toolset - enable_toolset: verifies toolset can be enabled and marked as enabled - enable_toolset invalid: verifies proper error for non-existent toolset - toolsets enum: verifies tools have proper enum values in schema * Advertise all capabilities in dynamic toolsets mode In dynamic mode, explicitly set HasTools/HasResources/HasPrompts=true since toolsets with those capabilities can be enabled at runtime. This ensures clients know the server supports these features even when no tools/resources/prompts are initially registered. * Improve conformance test with dynamic tool calls and JSON normalization - Add dynamic tool call testing (list_available_toolsets, get_toolset_tools, enable_toolset) - Parse and sort embedded JSON in text fields for proper comparison - Separate progress output (stderr) from summary (stdout) for CI - Add test type field to distinguish standard vs dynamic tests * Add conformance-report to .gitignore * Add conformance test CI workflow - Runs on pull requests to main - Compares PR branch against merge-base with origin/main - Outputs full conformance report to GitHub Actions Job Summary - Uploads detailed report as artifact for deeper investigation - Does not fail the build on differences (may be intentional) * Add map indexes for O(1) lookups in Registry Address review feedback to use maps for collections. Added lookup maps (toolsByName, resourcesByURI, promptsByName) while keeping slices for ordered iteration. This provides O(1) lookup for: - FindToolByName - filterToolsByName (used by ForMCPRequest) - filterResourcesByURI - filterPromptsByName Maps are built once during Build() and shared in ForMCPRequest copies. * perf(registry): O(1) HasToolset lookup via pre-computed set Add toolsetIDSet (map[ToolsetID]bool) to Registry for O(1) HasToolset lookups. Previously HasToolset iterated through all tools, resourceTemplates, and prompts to check if any belonged to the given toolset. Now it's a simple map lookup. The set is populated during the single-pass processToolsets() call, which already collected all valid toolset IDs. This adds zero new iteration - just returns the existing validIDs map. processToolsets now returns 6 values: - enabledToolsets, unrecognized, toolsetIDs, toolsetIDSet, defaultToolsetIDs, descriptions * simplify: remove lazy toolsByName map - not needed for actual use cases FindToolByName() is only called once per request at most (to find toolset ID for dynamic enablement). The SDK handles tool dispatch after registration. A simple linear scan over ~90 tools is trivially fast and avoids: - sync.Once complexity - Map allocation - Premature optimization for non-existent 'repeated lookups' The pre-computed maps we keep (toolsetIDSet, etc.) are justified because they're used for filtering logic that runs on every request. * Add generic tool filtering mechanisms to registry package - Add Enabled field to ServerTool for self-filtering based on context - Add ToolFilter type and WithFilter method to Builder for cross-cutting filters - Update isToolEnabled to check Enabled function and builder filters in order: 1. Tool's Enabled function 2. Feature flags (FeatureFlagEnable/FeatureFlagDisable) 3. Read-only filter 4. Builder filters 5. Toolset/additional tools check - Add FilteredTools method to Registry as alias for AvailableTools - Add comprehensive tests for all new functionality - All tests pass and linter is clean Closes github#1618 Co-authored-by: SamMorrowDrums <[email protected]> * docs: improve filter evaluation order and FilteredTools documentation - Add numbered filter evaluation order to isToolEnabled function doc - Number inline comments for each filter step (1-5) - Clarify FilteredTools error return is for future extensibility - Document that library consumers may need to surface recoverable errors Addresses review feedback on PR github#1620 * Refactor GenerateToolsetsHelp() to use strings.Builder pattern Co-authored-by: SamMorrowDrums <[email protected]> --------- Co-authored-by: Adam Holt <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: SamMorrowDrums <[email protected]> * Port functional changes from main to registry pattern Port three functional improvements from main branch: - GraphQL review comments grouped as threads (github#1554) - get_file_contents description improvement (github#1582) - create_or_update_file SHA validation fix (github#1621) Adapted implementations to use the new registry pattern with: - BaseDeps for providing clients via ToolDependencies interface - deps.GetClient(ctx) and deps.GetGQLClient(ctx) patterns - Updated tests to use GraphQL mocks for review comments - Added SHA validation test cases for create_or_update_file * fix(e2e): Fix e2e test compilation and add rate limit handling - Fix DefaultToolsetIDs() type mismatch by using github.GetDefaultToolsetIDs() - Add waitForRateLimit() to check and wait for rate limits before each test - Add skip conditions for Copilot tests when Copilot isn't available - Use multi-line file content in TestPullRequestReviewCommentSubmit for multi-line review comments to work correctly - Improve error messages to include response details * fix(gists): Use proper GitHub API error handling for observability The gists.go file was using NewToolResultErrorFromErr for GitHub API errors, which breaks the error middleware tracking that the remote server uses for observability and incident detection. Changed API errors (client.Gists.List, Get, Create, Edit) to use ghErrors.NewGitHubAPIErrorResponse which properly: - Records errors in the context for middleware access - Preserves the response object for rate limit and status tracking - Maintains consistency with other tools that use this pattern This ensures production observability is maintained for Gist operations. * chore: Update server.json schema to 2025-12-11 - Update schema URL to latest version (2025-12-11) - Remove 'status' field (now managed by registry per 2025-09-29 changelog) * fix(get_file_contents): Restore correct implementation from github#1582 The refactor incorrectly restructured the GetFileContents logic: - Move 'if rawOpts.SHA != "" { ref = rawOpts.SHA }' before GetContents call - Always call GetContents first (not conditionally based on path suffix) - Restore matchFiles helper function for proper fallback handling - Use matchFiles when Contents API fails or raw API fails This aligns with the improvements from PR github#1582 that was merged into main. * Rename registry to inventory in comments Update remaining references to 'registry' in code comments to use 'inventory' consistently after the package rename. --------- Co-authored-by: Adam Holt <[email protected]> Co-authored-by: Adam Holt <[email protected]> Co-authored-by: Adam Holt <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: SamMorrowDrums <[email protected]>
* Fix: path param should be optional * Add test
…cts tests (github#1737) * Initial plan * migrate tests from go-github-mock to internal testify-based mock Co-authored-by: JoannaaKL <[email protected]> * address feedback in testmock helper Co-authored-by: JoannaaKL <[email protected]> * tweak testmock path matching edge case Co-authored-by: JoannaaKL <[email protected]> * refine testmock options and path matching Co-authored-by: JoannaaKL <[email protected]> * simplify matchPath and document delete endpoint Co-authored-by: JoannaaKL <[email protected]> * Replace go-github-mock usage in tests with shared HTTP mock helper Co-authored-by: JoannaaKL <[email protected]> * Replace go-github-mock usage in tests with shared HTTP mock helper Co-authored-by: JoannaaKL <[email protected]> * fix tests and lint after mock cleanup Co-authored-by: JoannaaKL <[email protected]> * Remove import completely * Partial removal in repositories_test.go * Final removal --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> Co-authored-by: JoannaaKL <[email protected]>
Clarified instructions to run commands in the terminal instead of Claude Code CLI.
The CompleteParams.Context field is optional (marked omitempty) and can be nil when clients don't send it. The code was accessing Context.Arguments directly without checking if Context was nil first, causing a panic. This fix adds a nil check for Context before accessing Arguments.
Co-authored-by: SamMorrowDrums <[email protected]>
Co-authored-by: SamMorrowDrums <[email protected]>
Co-authored-by: SamMorrowDrums <[email protected]>
Co-authored-by: SamMorrowDrums <[email protected]>
Co-authored-by: SamMorrowDrums <[email protected]>
….1.1+ Co-authored-by: SamMorrowDrums <[email protected]>
Co-authored-by: SamMorrowDrums <[email protected]>
* Bringing back local mcp server registry config * Making auth header optional
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bringing back local mcp server registry config
Making auth header optional
Summary
Why
Fixes #
What changed
MCP impact
Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs