Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit10a0f98

Browse files
SamMorrowDrumsomgitsadsCopilot
authored
refactor(notifications): migrate notifications.go to NewTool pattern (#1592)
* refactor(notifications): migrate notifications.go to NewTool patternConvert all notification tools to use the new NewTool helper withToolDependencies injection.Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* Refactor repositories.go tools to use NewTool pattern with ToolDependenciesConvert all 18 tool functions in repositories.go to use the new NewTool helperpattern with typed ToolDependencies, isolating type assertions to a singlelocation and improving code maintainability.Functions converted:- GetCommit, ListCommits, ListBranches- CreateOrUpdateFile, CreateRepository, GetFileContents- ForkRepository, DeleteFile, CreateBranch, PushFiles- ListTags, GetTag, ListReleases, GetLatestRelease, GetReleaseByTag- ListStarredRepositories, StarRepository, UnstarRepositoryThis is part of a stacked PR series to systematically migrate all toolfiles to the new pattern.Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* refactor(issues): migrate issues.go to NewTool patternConvert all 8 tool functions in issues.go to use the new NewToolhelper pattern which standardizes dependency injection:- IssueRead: GetClient, GetGQLClient, RepoAccessCache, Flags- ListIssueTypes: GetClient- AddIssueComment: GetClient- SubIssueWrite: GetClient- SearchIssues: GetClient- IssueWrite: GetClient, GetGQLClient- ListIssues: GetGQLClient- AssignCopilotToIssue: GetGQLClientUpdated tools.go to use direct function calls instead ofNewServerToolLegacy wrappers. Updated all tests in issues_test.goto use the new ToolDependencies pattern and Handler() method.Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* refactor(pullrequests): convert PR tools to NewTool patternConvert all 10 pull request tool functions to use the NewToolpattern with ToolDependencies injection:- PullRequestRead- CreatePullRequest- UpdatePullRequest- ListPullRequests- MergePullRequest- SearchPullRequests- UpdatePullRequestBranch- PullRequestReviewWrite- AddCommentToPendingReview- RequestCopilotReviewUpdate tools.go to use direct function calls (removingNewServerToolLegacy wrappers) for PR functions.Update all tests in pullrequests_test.go to use the newhandler pattern with deps and 2-value return.Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* Refactor actions.go to use NewTool patternConvert all 14 tool functions in actions.go to use the NewTool pattern withToolDependencies for dependency injection. This is part of a broader effortto standardize the tool implementation pattern across the codebase.Changes:- ListWorkflows, ListWorkflowRuns, RunWorkflow, GetWorkflowRun- GetWorkflowRunLogs, ListWorkflowJobs, GetJobLogs- RerunWorkflowRun, RerunFailedJobs, CancelWorkflowRun- ListWorkflowRunArtifacts, DownloadWorkflowRunArtifact- DeleteWorkflowRunLogs, GetWorkflowRunUsageThe 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 interfaceCo-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* refactor(git): migrate GetRepositoryTree to NewTool pattern* refactor(security): migrate code_scanning, secret_scanning, dependabot to NewTool patternCo-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* refactor(discussions): migrate to NewTool patternCo-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* Refactor security_advisories tools to use NewTool patternConvert 4 functions from NewServerToolLegacy wrapper to NewTool:- ListGlobalSecurityAdvisories- GetGlobalSecurityAdvisory- ListRepositorySecurityAdvisories- ListOrgRepositorySecurityAdvisoriesUpdate tools.go toolset registration and tests.Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* refactor: convert projects, labels, and dynamic_tools to NewTool patternThis PR converts projects.go, labels.go, and dynamic_tools.go from thelegacy NewServerToolLegacy wrapper pattern to the new NewTool pattern withproper 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 registrationCo-authored-by: Adam Holt <omgitsads@users.noreply.github.com>* Add --features CLI flag for feature flag supportAdd 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 chainThis enables tools/resources/prompts that have FeatureFlagEnable set toa flag name that is passed via --features. The checker uses a simpleset 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 metadataThis commit adds comprehensive validation tests to ensure all MCP itemshave 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 toolsetAlso fixes a bug discovered by these tests: ToolsetMetadataGit was definedbut not added to AvailableToolsets(), causing get_repository_tree to havean invalid toolset ID.* Fix default toolsets behavior when not in dynamic modeWhen no toolsets are specified and dynamic mode is disabled, the servershould use the default toolsets. The bug was introduced when addingdynamic toolsets support:1. CleanToolsets(nil) was converting nil to empty slice2. Empty slice passed to WithToolsets means 'no toolsets'3. This resulted in zero tools being registeredFix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets)and only set empty slice when dynamic mode is enabled without explicittoolsets.* 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 NewToolsetGroupThis 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 patternsThis resolves the concern about mixed concerns in doc generation - thetoolset metadata and resource templates can now be created without anyruntime dependencies, while handlers are generated on-demand when depsare 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 translationsThis eliminates hardcoded toolset lists - everything is now derived fromthe 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 filesThis refactoring improves code organization and makes the registry'spurpose 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 flagsTwo 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 toolsetLabels are closely related to issues - you add labels to issues,search issues by label, etc. Keeping them in a separate toolsetrequired users to explicitly enable 'labels' to get this functionality.Moving to issues toolset makes labels available by default sinceissues is a default toolset.* Restore labels toolset with get_label in both issues and labelsThis 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 toolsetsin 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 toolsTests 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 modeIn dynamic mode, explicitly set HasTools/HasResources/HasPrompts=truesince toolsets with those capabilities can be enabled at runtime.This ensures clients know the server supports these features evenwhen 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 RegistryAddress review feedback to use maps for collections. Added lookup maps(toolsByName, resourcesByURI, promptsByName) while keeping slices forordered iteration. This provides O(1) lookup for:- FindToolByName- filterToolsByName (used by ForMCPRequest)- filterResourcesByURI- filterPromptsByNameMaps are built once during Build() and shared in ForMCPRequest copies.* perf(registry): O(1) HasToolset lookup via pre-computed setAdd toolsetIDSet (map[ToolsetID]bool) to Registry for O(1) HasToolset lookups.Previously HasToolset iterated through all tools, resourceTemplates, and promptsto 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 alreadycollected all valid toolset IDs. This adds zero new iteration - just returns theexisting validIDs map.processToolsets now returns 6 values:- enabledToolsets, unrecognized, toolsetIDs, toolsetIDSet, defaultToolsetIDs, descriptions* simplify: remove lazy toolsByName map - not needed for actual use casesFindToolByName() is only called once per request at most (to find toolset IDfor 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 becausethey'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 cleanCloses#1618Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>* 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 errorsAddresses review feedback on PR#1620* Refactor GenerateToolsetsHelp() to use strings.Builder patternCo-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>---------Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parentf735f45 commit10a0f98

File tree

68 files changed

+11908
-8609
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+11908
-8609
lines changed

‎.github/workflows/conformance.yml‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
name:Conformance Test
2+
3+
on:
4+
pull_request:
5+
6+
permissions:
7+
contents:read
8+
9+
jobs:
10+
conformance:
11+
runs-on:ubuntu-latest
12+
13+
steps:
14+
-name:Check out code
15+
uses:actions/checkout@v6
16+
with:
17+
# Fetch full history to access merge-base
18+
fetch-depth:0
19+
20+
-name:Set up Go
21+
uses:actions/setup-go@v6
22+
with:
23+
go-version-file:"go.mod"
24+
25+
-name:Download dependencies
26+
run:go mod download
27+
28+
-name:Run conformance test
29+
id:conformance
30+
run:|
31+
# Run conformance test, capture stdout for summary
32+
script/conformance-test > conformance-summary.txt 2>&1 ||true
33+
34+
# Output the summary
35+
cat conformance-summary.txt
36+
37+
# Check result
38+
if grep -q "RESULT: ALL TESTS PASSED" conformance-summary.txt; then
39+
echo "status=passed" >> $GITHUB_OUTPUT
40+
else
41+
echo "status=differences" >> $GITHUB_OUTPUT
42+
fi
43+
44+
-name:Generate Job Summary
45+
run:|
46+
# Add the full markdown report to the job summary
47+
echo "# MCP Server Conformance Report" >> $GITHUB_STEP_SUMMARY
48+
echo "" >> $GITHUB_STEP_SUMMARY
49+
echo "Comparing PR branch against merge-base with \`origin/main\`" >> $GITHUB_STEP_SUMMARY
50+
echo "" >> $GITHUB_STEP_SUMMARY
51+
52+
# Extract and append the report content (skip the header since we added our own)
53+
tail -n +5 conformance-report/CONFORMANCE_REPORT.md >> $GITHUB_STEP_SUMMARY
54+
55+
echo "" >> $GITHUB_STEP_SUMMARY
56+
echo "---" >> $GITHUB_STEP_SUMMARY
57+
echo "" >> $GITHUB_STEP_SUMMARY
58+
59+
# Add interpretation note
60+
if [ "${{ steps.conformance.outputs.status }}" = "passed" ]; then
61+
echo "✅ **All conformance tests passed** - No behavioral differences detected." >> $GITHUB_STEP_SUMMARY
62+
else
63+
echo "⚠️ **Differences detected** - Review the diffs above to ensure changes are intentional." >> $GITHUB_STEP_SUMMARY
64+
echo "" >> $GITHUB_STEP_SUMMARY
65+
echo "Common expected differences:" >> $GITHUB_STEP_SUMMARY
66+
echo "- New tools/toolsets added" >> $GITHUB_STEP_SUMMARY
67+
echo "- Tool descriptions updated" >> $GITHUB_STEP_SUMMARY
68+
echo "- Capability changes (intentional improvements)" >> $GITHUB_STEP_SUMMARY
69+
fi

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,5 @@ bin/
1919
# binary
2020
github-mcp-server
2121

22-
.history
22+
.history
23+
conformance-report/

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ You can also configure specific tools using the `--tools` flag. Tools can be use
384384
- Tools, toolsets, and dynamic toolsets can all be used together
385385
- Read-only mode takes priority: write tools are skipped if`--read-only` is set, even if explicitly requested via`--tools`
386386
- Tool names must match exactly (e.g.,`get_file_contents`, not`getFileContents`). Invalid tool names will cause the server to fail at startup with an error message
387+
- When tools are renamed, old names are preserved as aliases for backward compatibility. See[Deprecated Tool Aliases](docs/deprecated-tool-aliases.md) for details.
387388

388389
###Using Toolsets With Docker
389390

@@ -459,7 +460,6 @@ The following sets of tools are available:
459460
|`code_security`| Code security related tools, such as GitHub Code Scanning|
460461
|`dependabot`| Dependabot tools|
461462
|`discussions`| GitHub Discussions related tools|
462-
|`experiments`| Experimental features that are not considered stable yet|
463463
|`gists`| GitHub Gist related tools|
464464
|`git`| GitHub Git API related tools for low-level Git operations|
465465
|`issues`| GitHub Issues related tools|

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp