Skip to content

Conversation

@SamMorrowDrums
Copy link
Collaborator

Summary

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.

Changes

Functions converted:

  • GetCommit, ListCommits, ListBranches
  • CreateOrUpdateFile, CreateRepository, GetFileContents
  • ForkRepository, DeleteFile, CreateBranch, PushFiles
  • ListTags, GetTag, ListReleases, GetLatestRelease, GetReleaseByTag
  • ListStarredRepositories, StarRepository, UnstarRepository

Pattern Applied

Before:

func SomeFunction(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) {
    tool := mcp.Tool{...}
    handler := mcp.ToolHandlerFor[map[string]any, any](func(ctx context.Context, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
        client, err := getClient(ctx)
        // ...
    })
    return tool, handler
}

After:

func SomeFunction(t translations.TranslationHelperFunc) toolsets.ServerTool {
    return NewTool(
        mcp.Tool{...},
        func(deps ToolDependencies) mcp.ToolHandlerFor[map[string]any, any] {
            return func(ctx context.Context, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
                client, err := deps.GetClient(ctx)
                // ...
            }
        },
    )
}

Stacked PR Series

This is part of a stacked PR series to systematically migrate all tool files to the new pattern:

  1. refactor: Separate ServerTool with HandlerFunc pattern and ToolDependencies #1589 - Foundation + search.go
  2. Migrate context_tools to new ServerTool pattern #1590 - context_tools.go
  3. refactor(gists): migrate gists.go to NewTool pattern #1591 - gists.go
  4. refactor(notifications): migrate notifications.go to NewTool pattern #1592 - notifications.go
  5. This PR - repositories.go

Testing

  • All existing tests pass
  • No toolsnap changes (schema unchanged)
  • script/lint passes
  • script/test passes

Co-authored-by: Adam Holt oholt@github.com

Convert all notification tools to use the new NewTool helper with
ToolDependencies injection.

Co-authored-by: Adam Holt <oholt@github.com>
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner December 13, 2025 13:26
Copilot AI review requested due to automatic review settings December 13, 2025 13:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors all 18 tool functions in repositories.go to use the new NewTool helper pattern with typed ToolDependencies, eliminating scattered type assertions and improving code maintainability. This is the 5th and final PR in a stacked series systematically migrating tool files to this pattern.

Key Changes:

  • Function signatures simplified: removed getClient and getRawClient parameters, now return toolsets.ServerTool instead of tuples
  • Handlers access clients via deps.GetClient(ctx) and deps.GetRawClient(ctx)
  • Tests updated to create ToolDependencies structs and call serverTool.Handler(deps)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/github/repositories.go Converted all 18 tool functions (GetCommit, ListCommits, CreateBranch, PushFiles, etc.) to use NewTool pattern with ToolDependencies
pkg/github/tools.go Removed NewServerToolLegacy wrappers and updated function calls to new signatures
pkg/github/repositories_test.go Updated all test cases to use new pattern: create ToolDependencies, call Handler(deps), and verify results

…encies

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>
@SamMorrowDrums SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-repositories branch from 12ea669 to 37237a3 Compare December 13, 2025 14:17
@SamMorrowDrums SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-notifications branch from 34cf61e to 5e904e8 Compare December 13, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants