diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 85afed6e1..7f5c85faf 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -45,8 +45,7 @@ const ( actionsMethodDeleteWorkflowRunLogs = "delete_workflow_run_logs" ) -// handleFailedJobLogs gets logs for all failed jobs in a workflow run -func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, any, error) { +func handleRunJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, failedOnly bool, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, any, error) { // First, get all jobs for the workflow run jobs, resp, err := client.Actions.ListWorkflowJobs(ctx, owner, repo, runID, &github.ListWorkflowJobsOptions{ Filter: "latest", @@ -56,28 +55,34 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo } defer func() { _ = resp.Body.Close() }() - // Filter for failed jobs - var failedJobs []*github.WorkflowJob + // Filter jobs when requested. Otherwise return logs for every job in the run. + var selectedJobs []*github.WorkflowJob for _, job := range jobs.Jobs { - if job.GetConclusion() == "failure" { - failedJobs = append(failedJobs, job) + if !failedOnly || job.GetConclusion() == "failure" { + selectedJobs = append(selectedJobs, job) } } - if len(failedJobs) == 0 { + if len(selectedJobs) == 0 { + message := "No jobs found in this workflow run" + if failedOnly { + message = "No failed jobs found in this workflow run" + } result := map[string]any{ - "message": "No failed jobs found in this workflow run", - "run_id": runID, - "total_jobs": len(jobs.Jobs), - "failed_jobs": 0, + "message": message, + "run_id": runID, + "total_jobs": len(jobs.Jobs), + } + if failedOnly { + result["failed_jobs"] = 0 } r, _ := json.Marshal(result) return utils.NewToolResultText(string(r)), nil, nil } - // Collect logs for all failed jobs + // Collect logs for all selected jobs var logResults []map[string]any - for _, job := range failedJobs { + for _, job := range selectedJobs { jobResult, resp, err := getJobLogData(ctx, client, owner, repo, job.GetID(), job.GetName(), returnContent, tailLines, contentWindowSize) if err != nil { // Continue with other jobs even if one fails @@ -93,14 +98,20 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo logResults = append(logResults, jobResult) } + message := fmt.Sprintf("Retrieved logs for %d jobs", len(selectedJobs)) + if failedOnly { + message = fmt.Sprintf("Retrieved logs for %d failed jobs", len(selectedJobs)) + } result := map[string]any{ - "message": fmt.Sprintf("Retrieved logs for %d failed jobs", len(failedJobs)), + "message": message, "run_id": runID, "total_jobs": len(jobs.Jobs), - "failed_jobs": len(failedJobs), "logs": logResults, "return_format": map[string]bool{"content": returnContent, "urls": !returnContent}, } + if failedOnly { + result["failed_jobs"] = len(selectedJobs) + } r, err := json.Marshal(result) if err != nil { @@ -111,7 +122,19 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo } // handleSingleJobLogs gets logs for a single job -func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, any, error) { +func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, failedOnly bool, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, any, error) { + if failedOnly { + workflowJob, resp, err := client.Actions.GetWorkflowJobByID(ctx, owner, repo, jobID) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get workflow job", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if workflowJob.GetConclusion() != "failure" { + return utils.NewToolResultError(fmt.Sprintf("job_id %d has conclusion %q, not failure", jobID, workflowJob.GetConclusion())), nil, nil + } + } + jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines, contentWindowSize) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get job logs", resp, err), nil, nil @@ -623,8 +646,8 @@ func ActionsGetJobLogs(t translations.TranslationHelperFunc) inventory.ServerToo mcp.Tool{ Name: "get_job_logs", Description: t("TOOL_GET_JOB_LOGS_CONSOLIDATED_DESCRIPTION", `Get logs for GitHub Actions workflow jobs. -Use this tool to retrieve logs for a specific job or all failed jobs in a workflow run. -For single job logs, provide job_id. For all failed jobs in a run, provide run_id with failed_only=true. +Use this tool to retrieve logs for a specific job or all jobs in a workflow run. +Provide job_id for one job, or run_id for all jobs in a run. Set failed_only=true to restrict either mode to failed jobs only. `), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_GET_JOB_LOGS_CONSOLIDATED_USER_TITLE", "Get GitHub Actions workflow job logs"), @@ -643,15 +666,15 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i }, "job_id": { Type: "number", - Description: "The unique identifier of the workflow job. Required when getting logs for a single job.", + Description: "The unique identifier of the workflow job. Provide either job_id or run_id, not both.", }, "run_id": { Type: "number", - Description: "The unique identifier of the workflow run. Required when failed_only is true to get logs for all failed jobs in the run.", + Description: "The unique identifier of the workflow run. Provide either run_id or job_id, not both.", }, "failed_only": { Type: "boolean", - Description: "When true, gets logs for all failed jobs in the workflow run specified by run_id. Requires run_id to be provided.", + Description: "When true, only returns logs for failed jobs. With job_id, the job must have failed. With run_id, only failed jobs in the run are included.", }, "return_content": { Type: "boolean", @@ -711,23 +734,18 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Validate parameters - if failedOnly && runID == 0 { - return utils.NewToolResultError("run_id is required when failed_only is true"), nil, nil + if jobID > 0 && runID > 0 { + return utils.NewToolResultError("provide either job_id or run_id, not both"), nil, nil } - if !failedOnly && jobID == 0 { - return utils.NewToolResultError("job_id is required when failed_only is false"), nil, nil + if jobID == 0 && runID == 0 { + return utils.NewToolResultError("one of job_id or run_id must be provided"), nil, nil } - if failedOnly && runID > 0 { - // Handle failed-only mode: get logs for all failed jobs in the workflow run - return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize()) - } else if jobID > 0 { - // Handle single job mode - return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize()) + if runID > 0 { + return handleRunJobLogs(ctx, client, owner, repo, int64(runID), failedOnly, returnContent, tailLines, deps.GetContentWindowSize()) } - return utils.NewToolResultError("Either job_id must be provided for single job logs, or run_id with failed_only=true for failed job logs"), nil, nil + return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), failedOnly, returnContent, tailLines, deps.GetContentWindowSize()) }, ) return tool diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index 6eba71b8b..510828f04 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -581,11 +581,141 @@ func Test_ActionsGetJobLogs_SingleJob(t *testing.T) { assert.Contains(t, response, "logs_url") assert.Equal(t, "Job logs are available for download", response["message"]) }) + + t.Run("successful failed-only single job logs", func(t *testing.T) { + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposActionsJobsByOwnerByRepoByJobID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + job := &github.WorkflowJob{ + ID: github.Ptr(int64(123)), + Name: github.Ptr("test-job"), + Conclusion: github.Ptr("failure"), + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(job) + }), + GetReposActionsJobsLogsByOwnerByRepoByJobID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", "https://github.com/logs/job/123") + w.WriteHeader(http.StatusFound) + }), + }) + + client := github.NewClient(mockedClient) + deps := BaseDeps{ + Client: client, + ContentWindowSize: 5000, + } + handler := toolDef.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "job_id": float64(123), + "failed_only": true, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, float64(123), response["job_id"]) + assert.Contains(t, response, "logs_url") + }) + + t.Run("failed-only single job returns error for successful job", func(t *testing.T) { + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposActionsJobsByOwnerByRepoByJobID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + job := &github.WorkflowJob{ + ID: github.Ptr(int64(123)), + Name: github.Ptr("test-job"), + Conclusion: github.Ptr("success"), + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(job) + }), + }) + + client := github.NewClient(mockedClient) + deps := BaseDeps{ + Client: client, + ContentWindowSize: 5000, + } + handler := toolDef.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "job_id": float64(123), + "failed_only": true, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "success") + }) } func Test_ActionsGetJobLogs_FailedJobs(t *testing.T) { toolDef := ActionsGetJobLogs(translations.NullTranslationHelper) + t.Run("successful all jobs logs", func(t *testing.T) { + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposActionsRunsJobsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + jobs := &github.Jobs{ + TotalCount: github.Ptr(2), + Jobs: []*github.WorkflowJob{ + { + ID: github.Ptr(int64(1)), + Name: github.Ptr("test-job-1"), + Conclusion: github.Ptr("success"), + }, + { + ID: github.Ptr(int64(2)), + Name: github.Ptr("test-job-2"), + Conclusion: github.Ptr("failure"), + }, + }, + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(jobs) + }), + GetReposActionsJobsLogsByOwnerByRepoByJobID: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", "https://github.com/logs/job/"+r.URL.Path[len(r.URL.Path)-1:]) + w.WriteHeader(http.StatusFound) + }), + }) + + client := github.NewClient(mockedClient) + deps := BaseDeps{ + Client: client, + ContentWindowSize: 5000, + } + handler := toolDef.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "run_id": float64(456), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + assert.Equal(t, float64(456), response["run_id"]) + assert.Equal(t, float64(2), response["total_jobs"]) + assert.Len(t, response["logs"], 2) + assert.Contains(t, response["message"], "Retrieved logs for 2 jobs") + }) + t.Run("successful failed jobs logs", func(t *testing.T) { mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposActionsRunsJobsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -693,3 +823,39 @@ func Test_ActionsGetJobLogs_FailedJobs(t *testing.T) { assert.Equal(t, "No failed jobs found in this workflow run", response["message"]) }) } + +func Test_ActionsGetJobLogs_Validation(t *testing.T) { + toolDef := ActionsGetJobLogs(translations.NullTranslationHelper) + client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{ + Client: client, + ContentWindowSize: 5000, + } + handler := toolDef.Handler(deps) + + t.Run("requires one id", func(t *testing.T) { + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "one of job_id or run_id") + }) + + t.Run("rejects both ids", func(t *testing.T) { + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "job_id": float64(123), + "run_id": float64(456), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "not both") + }) +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 892b3045c..600149092 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -131,6 +131,7 @@ const ( PostReposActionsRunsRerunByOwnerByRepoByRunID = "POST /repos/{owner}/{repo}/actions/runs/{run_id}/rerun" PostReposActionsRunsRerunFailedJobsByOwnerByRepoByRunID = "POST /repos/{owner}/{repo}/actions/runs/{run_id}/rerun-failed-jobs" PostReposActionsRunsCancelByOwnerByRepoByRunID = "POST /repos/{owner}/{repo}/actions/runs/{run_id}/cancel" + GetReposActionsJobsByOwnerByRepoByJobID = "GET /repos/{owner}/{repo}/actions/jobs/{job_id}" GetReposActionsJobsLogsByOwnerByRepoByJobID = "GET /repos/{owner}/{repo}/actions/jobs/{job_id}/logs" DeleteReposActionsRunsLogsByOwnerByRepoByRunID = "DELETE /repos/{owner}/{repo}/actions/runs/{run_id}/logs"