diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index 8508563d9b..a95275128c 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -197,7 +197,7 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st // Download artifacts for the run auditLog.Printf("Downloading artifacts for run %d", runID) - err := downloadRunArtifacts(runID, runOutputDir, verbose) + err := downloadRunArtifacts(runID, runOutputDir, verbose, owner, repo, hostname) if err != nil { // Gracefully handle cases where the run legitimately has no artifacts if errors.Is(err, ErrNoArtifacts) { diff --git a/pkg/cli/audit_test.go b/pkg/cli/audit_test.go index 9e5e8e54de..1d62aa64c1 100644 --- a/pkg/cli/audit_test.go +++ b/pkg/cli/audit_test.go @@ -925,7 +925,7 @@ func TestAuditCachingBehavior(t *testing.T) { // Verify that downloadRunArtifacts skips download when valid summary exists // This is tested by checking that the function returns without error // and doesn't attempt to call `gh run download` - err := downloadRunArtifacts(run.DatabaseID, runOutputDir, false) + err := downloadRunArtifacts(run.DatabaseID, runOutputDir, false, "", "", "") if err != nil { t.Errorf("downloadRunArtifacts should skip download when valid summary exists, but got error: %v", err) } diff --git a/pkg/cli/logs_download.go b/pkg/cli/logs_download.go index 3319b11286..a53d7db6e6 100644 --- a/pkg/cli/logs_download.go +++ b/pkg/cli/logs_download.go @@ -288,8 +288,8 @@ func flattenAgentOutputsArtifact(outputDir string, verbose bool) error { } // downloadWorkflowRunLogs downloads and unzips workflow run logs using GitHub API -func downloadWorkflowRunLogs(runID int64, outputDir string, verbose bool) error { - logsDownloadLog.Printf("Downloading workflow run logs: run_id=%d, output_dir=%s", runID, outputDir) +func downloadWorkflowRunLogs(runID int64, outputDir string, verbose bool, owner, repo, hostname string) error { + logsDownloadLog.Printf("Downloading workflow run logs: run_id=%d, output_dir=%s, owner=%s, repo=%s", runID, outputDir, owner, repo) // Create a temporary file for the zip download tmpZip := filepath.Join(os.TempDir(), fmt.Sprintf("workflow-logs-%d.zip", runID)) @@ -301,7 +301,19 @@ func downloadWorkflowRunLogs(runID int64, outputDir string, verbose bool) error // Use gh api to download the logs zip file // The endpoint returns a 302 redirect to the actual zip file - output, err := workflow.RunGH("Downloading workflow logs...", "api", "repos/{owner}/{repo}/actions/runs/"+strconv.FormatInt(runID, 10)+"/logs") + var endpoint string + if owner != "" && repo != "" { + endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d/logs", owner, repo, runID) + } else { + endpoint = fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d/logs", runID) + } + + args := []string{"api", endpoint} + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) + } + + output, err := workflow.RunGH("Downloading workflow logs...", args...) if err != nil { // Check for authentication errors if strings.Contains(err.Error(), "exit status 4") { @@ -469,8 +481,8 @@ func listArtifacts(outputDir string) ([]string, error) { } // downloadRunArtifacts downloads artifacts for a specific workflow run -func downloadRunArtifacts(runID int64, outputDir string, verbose bool) error { - logsDownloadLog.Printf("Downloading run artifacts: run_id=%d, output_dir=%s", runID, outputDir) +func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, repo, hostname string) error { + logsDownloadLog.Printf("Downloading run artifacts: run_id=%d, output_dir=%s, owner=%s, repo=%s", runID, outputDir, owner, repo) // Check if artifacts already exist on disk (since they're immutable) if fileutil.DirExists(outputDir) && !fileutil.IsDirEmpty(outputDir) { @@ -499,8 +511,18 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool) error { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Created output directory "+outputDir)) } + // Build gh run download command with optional repo/hostname override for cross-repo and multi-host support + ghArgs := []string{"run", "download", strconv.FormatInt(runID, 10), "--dir", outputDir} + if owner != "" && repo != "" { + if hostname != "" && hostname != "github.com" { + ghArgs = append(ghArgs, "-R", hostname+"/"+owner+"/"+repo) + } else { + ghArgs = append(ghArgs, "-R", owner+"/"+repo) + } + } + if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Executing: gh run download %s --dir %s", strconv.FormatInt(runID, 10), outputDir))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(ghArgs, " "))) } // Start spinner for network operation @@ -509,7 +531,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool) error { spinner.Start() } - cmd := workflow.ExecGH("run", "download", strconv.FormatInt(runID, 10), "--dir", outputDir) + cmd := workflow.ExecGH(ghArgs...) output, err := cmd.CombinedOutput() if err != nil { @@ -560,7 +582,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool) error { } // Download and unzip workflow run logs - if err := downloadWorkflowRunLogs(runID, outputDir, verbose); err != nil { + if err := downloadWorkflowRunLogs(runID, outputDir, verbose, owner, repo, hostname); err != nil { // Log the error but don't fail the entire download process // Logs may not be available for all runs (e.g., expired or deleted) if verbose { diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 5628f084ff..026950eae4 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -188,7 +188,7 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s chunk := runsRemaining[:chunkSize] runsRemaining = runsRemaining[chunkSize:] - downloadResults := downloadRunArtifactsConcurrent(ctx, chunk, outputDir, verbose, remainingNeeded) + downloadResults := downloadRunArtifactsConcurrent(ctx, chunk, outputDir, verbose, remainingNeeded, repoOverride) for _, result := range downloadResults { if result.Skipped { @@ -505,7 +505,7 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s } // downloadRunArtifactsConcurrent downloads artifacts for multiple workflow runs concurrently -func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, outputDir string, verbose bool, maxRuns int) []DownloadResult { +func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, outputDir string, verbose bool, maxRuns int, repoOverride string) []DownloadResult { logsOrchestratorLog.Printf("Starting concurrent artifact download: runs=%d, outputDir=%s, maxRuns=%d", len(runs), outputDir, maxRuns) if len(runs) == 0 { return []DownloadResult{} @@ -541,6 +541,16 @@ func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, out // Get configured max concurrent downloads (default or from environment variable) maxConcurrent := getMaxConcurrentDownloads() + // Parse repoOverride into owner/repo once for cross-repo artifact download + var dlOwner, dlRepo string + if repoOverride != "" { + parts := strings.SplitN(repoOverride, "/", 2) + if len(parts) == 2 { + dlOwner = parts[0] + dlRepo = parts[1] + } + } + // Configure concurrent download pool with bounded parallelism and context cancellation. // The conc pool automatically handles panic recovery and prevents goroutine leaks. // WithContext enables graceful cancellation via Ctrl+C. @@ -598,7 +608,7 @@ func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, out } // No cached summary or version mismatch - download and process - err := downloadRunArtifacts(run.DatabaseID, runOutputDir, verbose) + err := downloadRunArtifacts(run.DatabaseID, runOutputDir, verbose, dlOwner, dlRepo, "") result := DownloadResult{ Run: run, diff --git a/pkg/cli/logs_orchestrator_test.go b/pkg/cli/logs_orchestrator_test.go index 46ca19c81b..0edede1eca 100644 --- a/pkg/cli/logs_orchestrator_test.go +++ b/pkg/cli/logs_orchestrator_test.go @@ -19,7 +19,7 @@ import ( // TestDownloadRunArtifactsConcurrent_EmptyRuns tests that empty runs slice returns empty results func TestDownloadRunArtifactsConcurrent_EmptyRuns(t *testing.T) { ctx := context.Background() - results := downloadRunArtifactsConcurrent(ctx, []WorkflowRun{}, "./test-logs", false, 5) + results := downloadRunArtifactsConcurrent(ctx, []WorkflowRun{}, "./test-logs", false, 5, "") assert.Empty(t, results, "Expected empty results for empty runs slice") } @@ -38,7 +38,7 @@ func TestDownloadRunArtifactsConcurrent_ResultOrdering(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5, "") // Verify we got all results require.Len(t, results, 5, "Expected 5 results") @@ -71,7 +71,7 @@ func TestDownloadRunArtifactsConcurrent_AllProcessed(t *testing.T) { tmpDir := testutil.TempDir(t, "test-orchestrator-*") // Pass maxRuns=3 as a hint, but all runs should still be processed - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3, "") // All runs should be processed to account for caching/filtering require.Len(t, results, 5, "All runs should be processed regardless of maxRuns parameter") @@ -101,7 +101,7 @@ func TestDownloadRunArtifactsConcurrent_ContextCancellation(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5, "") // Should still get results for all runs require.Len(t, results, 3, "Expected 3 results even with cancelled context") @@ -130,7 +130,7 @@ func TestDownloadRunArtifactsConcurrent_PartialCancellation(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 20) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 20, "") // Should get results for all runs (some may be skipped due to timeout) assert.Len(t, results, 20, "Should get results for all runs") @@ -165,7 +165,7 @@ func TestDownloadRunArtifactsConcurrent_NoResourceLeaks(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3, "") require.Len(t, results, 3, "Expected 3 results") @@ -248,7 +248,7 @@ func TestDownloadRunArtifactsConcurrent_ConcurrencyLimit(t *testing.T) { // We can't directly test the pool's behavior without mocking, // but we can verify the limit is configured correctly tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(context.Background(), runs, tmpDir, false, tt.runs) + results := downloadRunArtifactsConcurrent(context.Background(), runs, tmpDir, false, tt.runs, "") require.Len(t, results, tt.runs, "Expected %d results", tt.runs) @@ -271,7 +271,7 @@ func TestDownloadRunArtifactsConcurrent_LogsPath(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2, "") require.Len(t, results, 2, "Expected 2 results") @@ -295,7 +295,7 @@ func TestDownloadRunArtifactsConcurrent_ErrorHandling(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2, "") require.Len(t, results, 2, "Expected 2 results even with errors") @@ -324,7 +324,7 @@ func TestDownloadRunArtifactsConcurrent_MixedConclusions(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 5, "") require.Len(t, results, 5, "Expected 5 results") @@ -354,11 +354,11 @@ func TestDownloadRunArtifactsConcurrent_VerboseMode(t *testing.T) { tmpDir := testutil.TempDir(t, "test-orchestrator-*") // Test with verbose=false - resultsNonVerbose := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2) + resultsNonVerbose := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 2, "") require.Len(t, resultsNonVerbose, 2, "Non-verbose mode should return 2 results") // Test with verbose=true - resultsVerbose := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, true, 2) + resultsVerbose := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, true, 2, "") require.Len(t, resultsVerbose, 2, "Verbose mode should return 2 results") // Verify both modes return the same set of IDs (regardless of order) @@ -392,7 +392,7 @@ func TestDownloadRunArtifactsConcurrent_ResultStructure(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, []WorkflowRun{run}, tmpDir, false, 1) + results := downloadRunArtifactsConcurrent(ctx, []WorkflowRun{run}, tmpDir, false, 1, "") require.Len(t, results, 1, "Expected 1 result") @@ -437,7 +437,7 @@ func TestDownloadRunArtifactsConcurrent_PanicRecovery(t *testing.T) { } tmpDir := testutil.TempDir(t, "test-orchestrator-*") - results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3) + results := downloadRunArtifactsConcurrent(ctx, runs, tmpDir, false, 3, "") // Even if one download panicked, we should get results for all runs // (The actual panic recovery is tested by the conc pool library) diff --git a/pkg/cli/logs_parallel_test.go b/pkg/cli/logs_parallel_test.go index 898560aa4d..1ebd1981c3 100644 --- a/pkg/cli/logs_parallel_test.go +++ b/pkg/cli/logs_parallel_test.go @@ -14,7 +14,7 @@ import ( func TestDownloadRunArtifactsParallel(t *testing.T) { // Test with empty runs slice - results := downloadRunArtifactsConcurrent(context.Background(), []WorkflowRun{}, "./test-logs", false, 5) + results := downloadRunArtifactsConcurrent(context.Background(), []WorkflowRun{}, "./test-logs", false, 5, "") if len(results) != 0 { t.Errorf("Expected 0 results for empty runs, got %d", len(results)) } @@ -45,7 +45,7 @@ func TestDownloadRunArtifactsParallel(t *testing.T) { // This will fail since we don't have real GitHub CLI access, // but we can verify the structure and that no panics occur - results = downloadRunArtifactsConcurrent(context.Background(), runs, "./test-logs", false, 5) + results = downloadRunArtifactsConcurrent(context.Background(), runs, "./test-logs", false, 5, "") // We expect 2 results even if they fail if len(results) != 2 { @@ -85,7 +85,7 @@ func TestDownloadRunArtifactsParallelMaxRuns(t *testing.T) { } // Pass maxRuns=3 as a hint that we need 3 results, but all runs should be processed - results := downloadRunArtifactsConcurrent(context.Background(), runs, "./test-logs", false, 3) + results := downloadRunArtifactsConcurrent(context.Background(), runs, "./test-logs", false, 3, "") // All runs should be processed to account for potential caching/filtering if len(results) != 5 { @@ -284,7 +284,7 @@ func TestDownloadRunArtifactsParallelWithCancellation(t *testing.T) { } // Download with cancelled context - results := downloadRunArtifactsConcurrent(ctx, runs, "./test-logs", false, 5) + results := downloadRunArtifactsConcurrent(ctx, runs, "./test-logs", false, 5, "") // Should get results for all runs if len(results) != 2 {