Skip to content

Conversation

@SamMorrowDrums
Copy link
Collaborator

Summary

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.

Part of the ongoing effort to eliminate all NewServerToolLegacy usages in the codebase.

Changes

projects.go

  • Convert all 9 project functions to use NewTool with ToolHandlerFor[map[string]any, any] and 3-return-value handlers
  • Functions converted: ListProjects, GetProject, ListProjectFields, GetProjectField, ListProjectItems, GetProjectItem, AddProjectItem, UpdateProjectItem, DeleteProjectItem

projects_test.go

  • Update all 9 test functions to use new serverTool.Handler(deps) pattern
  • Remove unused mockClient declarations
  • Change handler invocation from 3-return to 2-return (handler returns ToolHandler which only has 2 returns)

labels.go

  • Convert GetLabel, ListLabels, and LabelWrite to NewTool pattern

labels_test.go

  • Update all 3 test functions to use new pattern

dynamic_tools.go

  • Refactor ListAvailableToolsets, GetToolsetsTools, and EnableToolset to return ServerTool directly
  • These functions use NewServerToolLegacy internally since they have special dependencies (*mcp.Server, *toolsets.ToolsetGroup) that cannot be handled by ToolDependencies

tools.go

  • Remove NewServerToolLegacy wrappers for dynamic tools registration since the functions now return ServerTool directly

Testing

  • All tests pass (script/test)
  • Lint passes (script/lint)

Stacked PR

This PR targets SamMorrowDrums/server-tool-refactor-security-advisories (#1600) and should be merged after that PR.

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

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>
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner December 13, 2025 18:46
Copilot AI review requested due to automatic review settings December 13, 2025 18:46
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 the projects, labels, and dynamic_tools modules to adopt the new NewTool pattern with ToolDependencies, eliminating the legacy NewServerToolLegacy wrapper pattern as part of an ongoing codebase modernization effort.

Key Changes:

  • Converted 9 project tools and 3 label tools to return toolsets.ServerTool directly using the NewTool pattern with proper ToolDependencies injection
  • Refactored dynamic toolset functions to return ServerTool directly while using NewServerToolLegacy internally for special dependencies
  • Updated all test files to use the new serverTool.Handler(deps) invocation pattern with 2-return handlers instead of 3-return handlers

Reviewed changes

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

Show a summary per file
File Description
pkg/github/tools.go Removed NewServerToolLegacy wrappers for projects, labels, and dynamic tools since these functions now return ServerTool directly
pkg/github/projects.go Converted all 9 project functions (ListProjects, GetProject, etc.) to use NewTool pattern with ToolHandlerFor[map[string]any, any] and dependency injection
pkg/github/projects_test.go Updated all 9 test functions to use serverTool.Handler(deps) pattern and removed unused mockClient declarations
pkg/github/labels.go Converted GetLabel, ListLabels, and LabelWrite functions to use NewTool pattern with ToolDependencies
pkg/github/labels_test.go Updated all 3 test functions to use new handler invocation pattern with 2-return handlers
pkg/github/dynamic_tools.go Refactored ListAvailableToolsets, GetToolsetsTools, and EnableToolset to return ServerTool directly using NewServerToolLegacy internally for special dependencies

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