Commit Graph

44 Commits

Author SHA1 Message Date
Copilot 80b0306557 Replace go-github-mock with stretchr/testify for actions/issues/projects tests (#1737)
* Initial plan

* migrate tests from go-github-mock to internal testify-based mock

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* address feedback in testmock helper

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* tweak testmock path matching edge case

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* refine testmock options and path matching

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* simplify matchPath and document delete endpoint

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Replace go-github-mock usage in tests with shared HTTP mock helper

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Replace go-github-mock usage in tests with shared HTTP mock helper

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* fix tests and lint after mock cleanup

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Remove import completely

* Partial removal in repositories_test.go

* Final removal

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>
Co-authored-by: JoannaaKL <joannaakl@github.com>
2026-01-06 10:45:29 +01:00
Sam Morrow 3c453dd9dd refactor: inject deps via context instead of closures
This refactor addresses performance issues in per-request server scenarios
where creating ~90 handler closures per request was causing latency.

Changes:
- Add ContextWithDeps, DepsFromContext, MustDepsFromContext to dependencies.go
- Add NewServerToolWithContextHandler, NewServerToolWithRawContextHandler to inventory
- Convert all 89 tool handlers from closure pattern to direct context-based deps
- Update all tests to inject deps into context before calling handlers
- Mark old NewServerTool and NewServerToolFromHandler as deprecated

The new pattern:
- Before: func(deps) handler { return func(ctx, req, args) { use deps } }
- After: func(ctx, deps, req, args) { use deps }

Dependencies are now injected into context once (via ContextWithDeps) and
extracted by NewTool internally before passing to handlers. This eliminates
closure creation on the hot path for remote servers.
2025-12-18 11:57:32 +01:00
Sam Morrow ce2e4f9472 refactor: Introduce Inventory pattern with builder, filtering, and per-request optimization (#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 <4619+omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* Migrate context_tools to new ServerTool pattern (#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 <oholt@github.com>

* 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 <omgitsads@users.noreply.github.com>

* refactor(gists): migrate gists.go to NewTool pattern (#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 <oholt@github.com>

* 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 <omgitsads@users.noreply.github.com>

---------

Co-authored-by: Adam Holt <oholt@github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>

---------

Co-authored-by: Adam Holt <oholt@github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>

* refactor(notifications): migrate notifications.go to NewTool pattern (#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 <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* refactor(git): migrate GetRepositoryTree to NewTool pattern

* refactor(security): migrate code_scanning, secret_scanning, dependabot to NewTool pattern

Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>

* refactor(discussions): migrate to NewTool pattern

Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* 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 <omgitsads@users.noreply.github.com>

* 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 #1618

Co-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 errors

Addresses review feedback on PR #1620

* Refactor GenerateToolsetsHelp() to use strings.Builder pattern

Co-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>

* Port functional changes from main to registry pattern

Port three functional improvements from main branch:
- GraphQL review comments grouped as threads (#1554)
- get_file_contents description improvement (#1582)
- create_or_update_file SHA validation fix (#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 #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 #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 <4619+omgitsads@users.noreply.github.com>
Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com>
Co-authored-by: Adam Holt <oholt@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>
2025-12-17 09:56:24 +01:00
Tony Truong 5a4338c685 adding review comments grouped as threads (#1554)
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
License Check / license-check (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
* adding review comments grouped as threads

* minor fix

* minor edit

* update docs

* fix docs

* increase limit to 100

* fixtext
2025-12-15 12:59:53 +01:00
Adam Holt 60aef5d2e3 Convert to modelcontextprotocol/go-sdk (#1428)
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 <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: omgitsads <4619+omgitsads@users.noreply.github.com>
Co-authored-by: LuluBeatson <lulubeatson@github.com>
Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com>
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Co-authored-by: Sam Morrow <info@sam-morrow.com>
2025-12-01 14:30:59 +01:00
JoannaaKL 28b868dad4 Add in memory cache for lockdown mode (#1416)
* Apply lockdown mode to issues and pull requests

* Add cache

* Unlock in defer

* Add muesli/cache2go

* [WIP] Replace custom cache in lockdown.go with cache2go struct (#1425)

* Initial plan

* Replace custom cache with cache2go library

- Added github.com/muesli/cache2go dependency
- Replaced custom map-based cache with cache2go.CacheTable
- Removed manual timer management (scheduleExpiry, ensureEntry methods)
- Removed timer field from repoAccessCacheEntry struct
- Updated GetRepoAccessInfo to use cache2go's Value() and Add() methods
- Updated SetTTL to flush and re-add entries with new TTL
- Used unique cache names per instance to avoid test interference
- All existing tests pass with the new implementation

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Final verification complete

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Use muesli for cache

* Make RepoAccessCache a singleton (#1426)

* Initial plan

* Implement RepoAccessCache as a singleton pattern

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Complete singleton implementation and verification

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Remove cacheIDCounter as requested

Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>

* Update mutexes

* .

* Reuse cache

* .

* .

* Fix logic after vibe coding

* Update docs

* .

* Refactoring to make the code pretty

* Hide lockdown logic behind shouldFilter function

* .

* Tests

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
2025-11-21 10:34:51 +01:00
Ksenia Bobrova f3b9a63311 Report error when API silently fails to add review comment (#1441) 2025-11-19 10:11:17 +01:00
Jonathan c73f06fa29 bumps google/go-github to v79 2025-11-15 22:07:38 +01:00
JoannaaKL 2e5418093f Add lockdown mode to filter issue (#1371)
* Add lockdown mode to filter issue

* Update flag name

* Update pkg/lockdown/lockdown.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Merge two graphql queries into one

* Don't use Issue.Repository

* Add function signature

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-11-10 17:01:21 +01:00
Jonathan cf0e05e300 Update to google/go-github@v77 (#1357)
* update to google/go-github@v77

* licences update from required CI build

* fixes licences

* fixes possible nil reference

* refresh licences due to recent conflicts

---------

Co-authored-by: Roberto Nacu <kerobbi@github.com>
2025-11-07 08:06:27 -08:00
Jonathan 6f5040734a Updates github-mcp-server to go-github/v76 (#1289)
* updates github-mcp-server to go-github/v76

* updates license files

* refactors ListProjects to make use of
google/go-github ProjectsService

* refactors GetProject to make use of google/go-github ProjectsService

* declaring commit without a pointer

* cleanup additional commit pointer

---------

Co-authored-by: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com>
2025-10-27 13:53:53 +01:00
Ksenia Bobrova e5522fca31 Consolidate pull request review tools (#1192)
* Consolidate pull request review tools

* Prompt tweaks + deleting snap

* Server instructions change

* Add enums

* Remove excessive mentions of event parameter

* Doc update
2025-10-10 16:56:01 +02:00
Ksenia Bobrova d0d4b0e5cb Consolidating pull requests get tools (#1172)
* Consolidating tools draft

* Consolidate pullrequest tools

* Prompt tweaks

* Fixes
2025-10-07 11:05:22 +02:00
Tony Truong 9db2e17204 Updating tool get_pull_request_comments -> get_pull_request_review_comments (#1062)
* update get_pull_request_comments to get_pull_request_review_comments to signify difference

* fix remaining old references

* cleanup dangling tool snap
2025-09-09 13:26:22 +02:00
Matt Holloway 358a415081 Add minimal response to CRUD tools, repositories and search toolsets (#988)
* add comprehensive minimal response where appropriate

* remove unneeded comments

* remove incorrect diff param

* update docs

* rm comment

* Update pkg/github/repositories.go

Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com>

* update toolsnaps and docs

* change minimal_output to use new OptionalBoolParamWithDefault

* Update pkg/github/repositories.go

Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com>

* refactor minimal conversion funcs to minimal_types.go

* consolidate response structs and remove unneeded message field

* consolidate response further

* remove CloneURL field

* Update pkg/github/repositories.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update pkg/github/server.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix undefined

* change incorrect comment

* remove old err var declaration

* Update pkg/github/repositories.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix syntax issue

* update toolsnaps

---------

Co-authored-by: Lulu <59149422+LuluBeatson@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-09-01 18:13:36 +01:00
Gökhan Arkan d52c1d4ece Fix duplicate filter issues across all search tools (#828)
* Enhance search functionality with new query filters and utility functions

- Added multiple test cases for searching issues with various query filters in `issues_test.go`.
- Introduced utility functions in `search_utils.go` to check for specific filters and extract repository information from queries.
- Created comprehensive tests in `search_utils_test.go` to validate the new filtering logic and ensure accurate query parsing.

* Refactor repository filter extraction in search utilities

- Renamed `extractRepoFilter` to `hasRepoFilter` to simplify the function's purpose.
- Updated test cases in `search_utils_test.go` to reflect the new function name and logic.
- Adjusted tests to focus on the presence of the `repo:` filter rather than extracting owner and repo details.

* Enhance search functionality with additional query filters and utility functions

- Added new test cases for searching pull requests and users with various query filters in `pullrequests_test.go` and `search_test.go`.
- Implemented a utility function `hasTypeFilter` in `search_utils.go` to check for the presence of `type:` filters in queries.
- Updated the search handler to conditionally prepend the `type:` filter based on the presence of existing filters, improving query handling.

* Updated both regex patterns to handle this edge case
2025-08-13 10:15:22 +01:00
Tommaso Moro 521d5e9258 Bump go-github to v74 (#826)
* update all imports to use v74

* go mod tidy

* update third party licenses

* rename

* autogen licenses
2025-08-06 22:11:50 +01:00
MayorFaj 45e90aedd4 feat: add reviewers parameter to UpdatePullRequest (#285)
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Docker / build (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
License Check / license-check (push) Has been cancelled
* feat: add reviewers parameter to UpdatePullRequest and update tests

* Update pullrequests.go

* feat: enhance update pull request functionality with reviewers support

* update README to clarify optional reviewers parameter in API documentation- go run ./cmd/github-mcp-server generate-docs

* feat: enhance UpdatePullRequest to return early if no updates or reviewers are provided

* Add updating draft state to `update_pull_request` tool (#774)

* initial impl of pull request draft state update

* appease linter

* update README

* add nosec

* fixed err return type for json marshalling

* add gql test

* Add support for org-level discussions in list_discussions tool (#775)

* make repo optional, and default to .github when not provided. improve tool description

* autogen

* update tests

* small copy paste error fixes

* refactor: streamline UpdatePullRequest logic and enhance test cases for reviewer updates

* refactor: remove redundant draft update tests and streamline UpdatePullRequest logic

* test: add unit tests for updating pull request draft state

* refactor: simplify UpdatePullRequest tests by removing unused mock data

---------

Co-authored-by: Matt Holloway <mattdholloway@github.com>
Co-authored-by: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com>
2025-07-31 11:05:38 +01:00
Matt Holloway d5e1f48728 Add updating draft state to update_pull_request tool (#774)
* initial impl of pull request draft state update

* appease linter

* update README

* add nosec

* fixed err return type for json marshalling

* add gql test
2025-07-29 10:58:06 +01:00
Lulu b5e3348179 fix: shorten long tool name for adding pr review comments (#697)
License Check / license-check (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
* shorten tool name

* update function name to match tool name

* adjust wording of descriptions
2025-07-18 10:57:05 +01:00
Martin Høst Normark be91795fd3 Bump go-github to v73.0.0 (#597)
* Bump go-github to v73.0.0

* Clean up go.mod and update licenses

* Updated remaining imports to use github package v73 instead of v72

---------

Co-authored-by: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com>
Co-authored-by: tommaso-moro <tommaso-moro@github.com>
2025-07-15 15:21:24 +01:00
JoannaaKL 8bd715235b Add repo and owner 2025-06-26 13:53:13 +02:00
JoannaaKL 805358ba3d Split pr and issue search
Add description

Extract common code

Test fixes

Updated search description

Move search prs to prs toolset

Update tools snaps
2025-06-26 13:53:13 +02:00
JoannaaKL 5f924342f1 Add search pull requests tool 2025-06-26 13:53:13 +02:00
anant-rustagi 23b16cfeb7 feat: add pagination support to get_pull_request_files tool (#561)
License Check / license-check (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Unit Tests / build (macos-latest) (push) Has been cancelled
Unit Tests / build (ubuntu-latest) (push) Has been cancelled
Unit Tests / build (windows-latest) (push) Has been cancelled
Lint / lint (push) Has been cancelled
- Add WithPagination() to tool definition
- Use OptionalPaginationParams to handle page and perPage parameters
- Pass pagination parameters to GitHub API ListFiles call
- Update tests to include pagination scenarios
- Update tool schema snapshot

Fixes #527

Co-authored-by: Anant rustagi <anantrustagi@Anants-MacBook-Pro.local>
2025-06-25 16:26:53 +02:00
Sam Morrow f90ff16f2e move to new approach and update testing 2025-06-25 08:12:38 +02:00
William Martin 3e988d5ab5 Add toolsnaps for every tool 2025-06-18 10:38:32 +02:00
Pranav RK c7a872b9a1 Bump go-github to v72.0.0 2025-06-02 10:55:04 +02:00
William Martin eca853b28a Split PR review creation, commenting, submission and deletion 2025-05-19 17:50:36 +02:00
William Martin 2fb1be93f5 Support requesting copilot as a reviewer 2025-05-14 22:57:43 +02:00
Arya Soni a563bd6e1d Add request_copilot_review tool with placeholder implementation 2025-05-14 22:57:43 +02:00
Ashwin Bhat 6c05b4009a Add tools for one-off PR comments and replying to PR review comments (#143)
* Add add_pull_request_review_comment tool for PR review comments

Adds the ability to add review comments to pull requests with support for line, multi-line, and file-level comments, as well as replying to existing comments.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add reply_to_pull_request_review_comment tool

Adds a new tool to reply to existing pull request review comments using the GitHub API's comment reply endpoint. This allows for threaded discussions on pull request reviews.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Update README with new PR review comment tools

* rebase

* use new getClient function inadd and reply pr review tools

* Unify PR review comment tools into a single consolidated tool

The separate AddPullRequestReviewComment and ReplyToPullRequestReviewComment tools have been merged into a single tool that handles both creating new comments and replying to existing ones. This approach simplifies the API and provides a more consistent interface for users.

- Made commit_id and path optional when using in_reply_to for replies
- Updated the tests to verify both comment and reply functionality
- Removed the separate ReplyToPullRequestReviewComment tool
- Fixed test expectations to match how errors are returned

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Update README to reflect the unified PR review comment tool

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Javier Uruen Val <juruen@github.com>
2025-04-12 11:45:10 +02:00
Sam Morrow 3ec8699deb chore: groundwork for multi-user to server 2025-04-10 01:59:03 +02:00
monotykamary 56c1fce434 feat: Add update_pull_request tool (#122)
* feat: add update_pull_request tool

* refactor: address feedback on optionalParamOK helper

* docs: add update_pull_request tool documentation

* refactor: update optionalParamsOK as exported member

* fix: rename to exported function
2025-04-09 08:11:10 +02:00
Sam Morrow c85dd07e5e chore: export pr funcs 2025-04-08 09:33:37 +02:00
Alexander Yastrebov 755b816b7c pkg/github: fix use of per page parameter
Page size tool parameter names were changed to `perPage` within #90
while GitHub API uses `per_page` parameter name.

This change fixes overlooked inconsistencies.

Follow up on #90
Follow up on #129
Fixes #136

Signed-off-by: Alexander Yastrebov <yastrebov.alex@gmail.com>
2025-04-07 16:48:56 +02:00
Ashwin Bhat 31471a4b4f Add line parameter support to create_pull_request_review tool (#118)
* Add line parameter support to create_pull_request_review tool

- Updated schema to make path and body the only required fields
- Added line parameter as alternative to position for inline comments
- Updated handler to accept either position or line based on GitHub API spec
- Added new test case that verifies line parameter works properly
- Updated error messages for better validation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Expand PR review API with multi-line comment support

- Added new parameters: line, side, start_line, start_side
- Added proper validation for multi-line comment parameters
- Improved validation logic to handle parameter combinations
- Added test cases for regular and multi-line comments
- Updated schema documentation for better tool discoverability

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* gofmt

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Javier Uruen Val <juruen@github.com>
2025-04-07 16:20:17 +02:00
William Martin 6f7458ae3c Conform list tools
License Check / license-check (push) Has been cancelled
Docker / build (push) Has been cancelled
Unit Tests / build (macos-latest) (push) Has been cancelled
Unit Tests / build (ubuntu-latest) (push) Has been cancelled
Unit Tests / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
Lint / lint (push) Has been cancelled
2025-04-04 15:51:50 +02:00
Javier Uruen Val ca1a8f96d4 assert request params and body (#95) 2025-04-04 13:57:35 +02:00
Javier Uruen Val 5f3926d535 add support for create_pull_request (#63)
Co-authored-by: Toby Padilla <toby@toby.sh>
2025-04-01 15:48:39 -06:00
Javier Uruen Val 92bceb5fa5 linter 2025-03-26 11:03:52 +01:00
Javier Uruen Val 7c772d2612 add support for the create_pull_request_review_tool (#33) 2025-03-24 13:50:49 +01:00
Sam Morrow dd46f8a837 add a framework for translations 2025-03-20 18:33:30 +01:00
Javier Uruen Val 3dddc2abcd add initial tests 2025-03-17 13:26:14 +01:00