Backport consolidated actions toolset #1584
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple GitHub Actions tools into a smaller set of method-based tools to simplify the API surface. The consolidation groups related operations together:
- Read operations: Multiple "list" and "get" tools consolidated into
actions_listandactions_get - Write operations: Multiple "run", "rerun", "cancel", and "delete" tools consolidated into
actions_run_trigger - Log retrieval:
get_job_logsremains separate (renamed fromGetJobLogstoActionsGetJobLogs)
Key changes:
- Tools now use a
methodparameter to specify the specific action to perform - Deprecated aliases maintain backward compatibility by mapping old tool names to new ones
- Tests updated to reflect the new consolidated structure
- Documentation regenerated to show new tool signatures
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/tools.go | Registers consolidated tools (ActionsList, ActionsGet, ActionsRunTrigger, ActionsGetJobLogs) instead of individual tools |
| pkg/github/deprecated_tool_aliases.go | Maps all old tool names to new consolidated tool names for backward compatibility |
| pkg/github/actions.go | Major refactoring - removes individual tool functions, adds consolidated tools with method-based routing and helper functions |
| pkg/github/actions_test.go | Refactors tests to test consolidated tools with method parameters, removes old individual tool tests |
| pkg/github/toolsnaps/*.snap | New/updated snapshots for consolidated tool schemas |
| README.md | Auto-generated documentation reflecting new tool structure |
| actionsMethodGetWorkflowRun = "get_workflow_run" | ||
| actionsMethodGetWorkflowJob = "get_workflow_job" | ||
| actionsMethodGetWorkflowRunUsage = "get_workflow_run_usage" | ||
| actionsMethodGetWorkflowRunLogsURL = "get_workflow_run_logs_url" |
There was a problem hiding this comment.
The method constant uses "get_workflow_run_logs_url" but the deprecated tool alias maps "get_workflow_run_logs" to "actions_get". For consistency with the old tool name and to avoid confusion, the method should be named "get_workflow_run_logs" not "get_workflow_run_logs_url". The "_url" suffix is unnecessary since the method returns URL information anyway, and it creates a naming mismatch with the deprecated tool.
| actionsMethodGetWorkflowRunLogsURL = "get_workflow_run_logs_url" | |
| actionsMethodGetWorkflowRunLogs = "get_workflow_run_logs" |
| return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to cancel workflow run", resp, err), nil, nil | ||
| } | ||
| } | ||
| defer func() { _ = resp.Body.Close() }() |
There was a problem hiding this comment.
Potential nil pointer dereference. If the API call at line 887 returns an error that is NOT an AcceptedError, and resp is nil, then line 894 (defer resp.Body.Close()) will panic. The defer statement should be moved inside the error check or wrapped in a nil check. The other helper functions (rerunWorkflowRun, rerunFailedJobs, deleteWorkflowRunLogs) have the same pattern but don't have this special AcceptedError handling, so they're safe.
| defer func() { _ = resp.Body.Close() }() | |
| if resp != nil { | |
| defer func() { _ = resp.Body.Close() }() | |
| } |
This PR concludes the Actions toolset consolidation rollout by bringing these changes back to the OSS repository
Closes: https://github.com/github/copilot-agent-services/issues/1053