Refactor actions.go to use NewTool pattern #1596
Open
+845
−777
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR successfully refactors all 14 tool functions in actions.go from the legacy pattern to the NewTool pattern with ToolDependencies for dependency injection. This is part of a broader effort to standardize tool implementation across the codebase.
Key Changes:
- Tool functions now accept only
translations.TranslationHelperFuncas a parameter instead ofGetClientFnand other dependencies - Return type changed from
(mcp.Tool, mcp.ToolHandlerFor[...])totoolsets.ServerTool - Dependencies (client, contentWindowSize) are now accessed via
ToolDependenciesstruct passed to handlers - Test files updated to use the new pattern with
toolDef.Handler(deps)instead of direct handler calls
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/github/actions.go | Refactored all 14 tool functions (ListWorkflows, ListWorkflowRuns, RunWorkflow, GetWorkflowRun, GetWorkflowRunLogs, ListWorkflowJobs, GetJobLogs, RerunWorkflowRun, RerunFailedJobs, CancelWorkflowRun, ListWorkflowRunArtifacts, DownloadWorkflowRunArtifact, DeleteWorkflowRunLogs, GetWorkflowRunUsage) to use NewTool pattern with ToolDependencies |
| pkg/github/tools.go | Updated tool registration in actions toolset to call tool functions directly without NewServerToolLegacy wrapper |
| pkg/github/actions_test.go | Updated all test functions to use new pattern: toolDef structure, ToolDependencies struct, and simplified handler invocation |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
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
Tools Converted
ListWorkflowsListWorkflowRunsRunWorkflowGetWorkflowRunGetWorkflowRunLogsListWorkflowJobsGetJobLogsRerunWorkflowRunRerunFailedJobsCancelWorkflowRunListWorkflowRunArtifactsDownloadWorkflowRunArtifactDeleteWorkflowRunLogsGetWorkflowRunUsagePattern Details
The new pattern:
translations.TranslationHelperFuncas parametertoolsets.ServerToolwithToolandHandlerToolDependenciesfor client accessFiles Modified
pkg/github/actions.go- Tool function implementationspkg/github/actions_test.go- Updated test signaturespkg/github/tools.go- Updated tool registrationTesting
Stack
Part of the server tool refactoring stack: