Skip to content

Conversation

@SamMorrowDrums
Copy link
Collaborator

Summary

Migrates all gist tools (ListGists, GetGist, CreateGist, UpdateGist) to the new NewTool pattern with ToolDependencies injection.

Part of stacked PRs for ServerTool refactoring:

  1. refactor: Separate ServerTool with HandlerFunc pattern and ToolDependencies #1589 - Foundation + search.go ⬅️ base
  2. Migrate context_tools to new ServerTool pattern #1590 - context_tools.go ⬅️ parent
  3. This PR - gists.go

Changes

  • Remove getClient parameter from all gist function signatures
  • Functions now only take t translations.TranslationHelperFunc and return toolsets.ServerTool
  • Use deps.GetClient(ctx) inside handlers to get the GitHub client
  • Standardize error handling with utils.NewToolResultErrorFromErr() instead of fmt.Errorf
  • Update all tests to use serverTool.Handler(deps) pattern with result.IsError checks

Testing

  • All existing gist tests updated to new pattern
  • script/lint passes
  • script/test passes

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

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>
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner December 13, 2025 12:07
Copilot AI review requested due to automatic review settings December 13, 2025 12:07
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 migrates gist-related tools to the new NewTool pattern with dependency injection, removing the getClient parameter from function signatures and using ToolDependencies instead. This is part of a stacked PR series refactoring the ServerTool architecture.

  • Removes getClient parameter from all gist tool functions
  • Adopts NewTool pattern with ToolDependencies for dependency injection
  • Standardizes error handling using utils.NewToolResultErrorFromErr
  • Updates all tests to use the new serverTool.Handler(deps) pattern with result.IsError checks

Reviewed changes

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

File Description
pkg/github/gists.go Refactored all four gist tools (ListGists, GetGist, CreateGist, UpdateGist) to use NewTool pattern with ToolDependencies, standardized error handling, and removed getClient parameter
pkg/github/tools.go Updated gist toolset registration to remove NewServerToolLegacy wrapper and call tool functions without getClient parameter
pkg/github/gists_test.go Updated all test cases to use new pattern with serverTool.Handler(deps), replaced error checking with result.IsError assertions, and added getErrorResult helper usage

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>
@SamMorrowDrums SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-gists branch from af97440 to 820ce1e Compare December 13, 2025 14:18
@SamMorrowDrums SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-context-tools branch from 07249a0 to b863da9 Compare December 13, 2025 14:18
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