From fbf0717a43d6a2bf31cee2ccc14510b87cf6b3f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 12:15:34 +0000 Subject: [PATCH 1/4] Initial plan From 90ca0b704769787fbb2056b4f3232077bdf60d91 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 12:36:28 +0000 Subject: [PATCH 2/4] Refactor DownloadWorkflowLogs to use options struct Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/context_cancellation_test.go | 12 ++++- pkg/cli/logs_ci_scenario_test.go | 37 ++++--------- pkg/cli/logs_command.go | 28 +++++++++- pkg/cli/logs_download_test.go | 13 ++++- pkg/cli/logs_json_stderr_order_test.go | 74 +++++++------------------- pkg/cli/logs_orchestrator.go | 56 ++++++++++++++++++- 6 files changed, 130 insertions(+), 90 deletions(-) diff --git a/pkg/cli/context_cancellation_test.go b/pkg/cli/context_cancellation_test.go index c986e059871..d361a59a5af 100644 --- a/pkg/cli/context_cancellation_test.go +++ b/pkg/cli/context_cancellation_test.go @@ -71,7 +71,10 @@ func TestDownloadWorkflowLogsWithCancellation(t *testing.T) { cancel() // Try to download logs with a cancelled context - err := DownloadWorkflowLogs(ctx, "", 10, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "", "", false, false, "", nil, "") + err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ + Count: 10, + OutputDir: "/tmp/test-logs", + }) // Should return context.Canceled error assert.ErrorIs(t, err, context.Canceled, "Should return context.Canceled error when context is cancelled") @@ -111,7 +114,12 @@ func TestDownloadWorkflowLogsTimeoutRespected(t *testing.T) { start := time.Now() // Use a workflow name that doesn't exist to avoid actual network calls - _ = DownloadWorkflowLogs(ctx, "nonexistent-workflow-12345", 100, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 1, "", "", false, false, "", nil, "") + _ = DownloadWorkflowLogs(ctx, LogsDownloadOptions{ + WorkflowName: "nonexistent-workflow-12345", + Count: 100, + OutputDir: "/tmp/test-logs", + Timeout: 1, + }) elapsed := time.Since(start) // Should complete within reasonable time (give 5 seconds buffer for test overhead) diff --git a/pkg/cli/logs_ci_scenario_test.go b/pkg/cli/logs_ci_scenario_test.go index 14490872725..41b44380f22 100644 --- a/pkg/cli/logs_ci_scenario_test.go +++ b/pkg/cli/logs_ci_scenario_test.go @@ -29,34 +29,15 @@ func TestLogsJSONOutputWithNoRuns(t *testing.T) { // Call DownloadWorkflowLogs with parameters that will result in no matching runs // We use a non-existent workflow name to ensure no results - err := DownloadWorkflowLogs( - ctx, - "nonexistent-workflow-12345", // Workflow that doesn't exist - 2, // count - "", // startDate - "", // endDate - tmpDir, // outputDir - "copilot", // engine - "", // ref - 0, // beforeRunID - 0, // afterRunID - "", // repoOverride - false, // verbose - false, // toolGraph - false, // noStaged - false, // firewallOnly - false, // noFirewall - false, // parse - true, // jsonOutput - THIS IS KEY - 10, // timeout - "summary.json", // summaryFile - "", // safeOutputType - false, // filteredIntegrity - false, // train - "", // format - nil, // artifactSets - "", // after - ) + err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ + WorkflowName: "nonexistent-workflow-12345", // Workflow that doesn't exist + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, // THIS IS KEY + Timeout: 10, + SummaryFile: "summary.json", + }) // Restore stdout and read output w.Close() diff --git a/pkg/cli/logs_command.go b/pkg/cli/logs_command.go index d2d95150afc..4e7ad14326a 100644 --- a/pkg/cli/logs_command.go +++ b/pkg/cli/logs_command.go @@ -243,7 +243,33 @@ Examples: logsCommandLog.Printf("Executing logs download: workflow=%s, count=%d, engine=%s, train=%v, after=%s", workflowName, count, engine, train, after) - return DownloadWorkflowLogs(cmd.Context(), workflowName, count, startDate, endDate, outputDir, engine, ref, beforeRunID, afterRunID, repoOverride, verbose, toolGraph, noStaged, firewallOnly, noFirewall, parse, jsonOutput, timeout, summaryFile, safeOutputType, filteredIntegrity, train, format, artifacts, after) + return DownloadWorkflowLogs(cmd.Context(), LogsDownloadOptions{ + WorkflowName: workflowName, + Count: count, + StartDate: startDate, + EndDate: endDate, + OutputDir: outputDir, + Engine: engine, + Ref: ref, + BeforeRunID: beforeRunID, + AfterRunID: afterRunID, + RepoOverride: repoOverride, + Verbose: verbose, + ToolGraph: toolGraph, + NoStaged: noStaged, + FirewallOnly: firewallOnly, + NoFirewall: noFirewall, + Parse: parse, + JSONOutput: jsonOutput, + Timeout: timeout, + SummaryFile: summaryFile, + SafeOutputType: safeOutputType, + FilteredIntegrity: filteredIntegrity, + Train: train, + Format: format, + ArtifactSets: artifacts, + After: after, + }) }, } diff --git a/pkg/cli/logs_download_test.go b/pkg/cli/logs_download_test.go index f7cb1fae828..d66aff4ae4d 100644 --- a/pkg/cli/logs_download_test.go +++ b/pkg/cli/logs_download_test.go @@ -21,7 +21,11 @@ func TestDownloadWorkflowLogs(t *testing.T) { // Test the DownloadWorkflowLogs function // This should either fail with auth error (if not authenticated) // or succeed with no results (if authenticated but no workflows match) - err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil, "") + err := DownloadWorkflowLogs(context.Background(), LogsDownloadOptions{ + Count: 1, + OutputDir: "./test-logs", + SummaryFile: "summary.json", + }) // If GitHub CLI is authenticated, the function may succeed but find no results // If not authenticated, it should return an auth error @@ -393,7 +397,12 @@ func TestDownloadWorkflowLogsWithEngineFilter(t *testing.T) { if !tt.expectError { // For valid engines, test that the function can be called without panic // It may still fail with auth errors, which is expected - err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", tt.engine, "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil, "") + err := DownloadWorkflowLogs(context.Background(), LogsDownloadOptions{ + Count: 1, + OutputDir: "./test-logs", + Engine: tt.engine, + SummaryFile: "summary.json", + }) // Clean up any created directories os.RemoveAll("./test-logs") diff --git a/pkg/cli/logs_json_stderr_order_test.go b/pkg/cli/logs_json_stderr_order_test.go index 371df14bbd6..3cdc11f335f 100644 --- a/pkg/cli/logs_json_stderr_order_test.go +++ b/pkg/cli/logs_json_stderr_order_test.go @@ -36,34 +36,15 @@ func TestLogsJSONOutputBeforeStderr(t *testing.T) { // Call DownloadWorkflowLogs with parameters that will result in no matching runs // This should trigger the warning message path - err := DownloadWorkflowLogs( - ctx, - "nonexistent-workflow-test-12345", // Workflow that doesn't exist - 2, // count - "", // startDate - "", // endDate - tmpDir, // outputDir - "copilot", // engine - "", // ref - 0, // beforeRunID - 0, // afterRunID - "", // repoOverride - false, // verbose - false, // toolGraph - false, // noStaged - false, // firewallOnly - false, // noFirewall - false, // parse - true, // jsonOutput - THIS IS KEY - 10, // timeout - "summary.json", // summaryFile - "", // safeOutputType - false, // filteredIntegrity - false, // train - "", // format - nil, // artifactSets - "", // after - ) + err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ + WorkflowName: "nonexistent-workflow-test-12345", // Workflow that doesn't exist + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, // THIS IS KEY + Timeout: 10, + SummaryFile: "summary.json", + }) // Close writers first stdoutW.Close() @@ -162,34 +143,15 @@ func TestLogsJSONAndStderrRedirected(t *testing.T) { // Call DownloadWorkflowLogs ctx := context.Background() - err := DownloadWorkflowLogs( - ctx, - "nonexistent-workflow-ci-test-67890", - 2, - "", - "", - tmpDir, - "copilot", - "", - 0, - 0, - "", - false, - false, - false, - false, - false, - false, - true, // jsonOutput - 10, - "summary.json", - "", // safeOutputType - false, // filteredIntegrity - false, // train - "", // format - nil, // artifactSets - "", // after - ) + err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ + WorkflowName: "nonexistent-workflow-ci-test-67890", + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, + Timeout: 10, + SummaryFile: "summary.json", + }) // Close the writer w.Close() diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 58ea9ec5a96..3a780cacf74 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -35,8 +35,62 @@ func getMaxConcurrentDownloads() int { return envutil.GetIntFromEnv("GH_AW_MAX_CONCURRENT_DOWNLOADS", MaxConcurrentDownloads, 1, 100, logsOrchestratorLog) } +type LogsDownloadOptions struct { + WorkflowName string + Count int + StartDate string + EndDate string + OutputDir string + Engine string + Ref string + BeforeRunID int64 + AfterRunID int64 + RepoOverride string + Verbose bool + ToolGraph bool + NoStaged bool + FirewallOnly bool + NoFirewall bool + Parse bool + JSONOutput bool + Timeout int + SummaryFile string + SafeOutputType string + FilteredIntegrity bool + Train bool + Format string + ArtifactSets []string + After string +} + // DownloadWorkflowLogs downloads and analyzes workflow logs with metrics -func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, startDate, endDate, outputDir, engine, ref string, beforeRunID, afterRunID int64, repoOverride string, verbose bool, toolGraph bool, noStaged bool, firewallOnly bool, noFirewall bool, parse bool, jsonOutput bool, timeout int, summaryFile string, safeOutputType string, filteredIntegrity bool, train bool, format string, artifactSets []string, after string) error { +func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error { + workflowName := opts.WorkflowName + count := opts.Count + startDate := opts.StartDate + endDate := opts.EndDate + outputDir := opts.OutputDir + engine := opts.Engine + ref := opts.Ref + beforeRunID := opts.BeforeRunID + afterRunID := opts.AfterRunID + repoOverride := opts.RepoOverride + verbose := opts.Verbose + toolGraph := opts.ToolGraph + noStaged := opts.NoStaged + firewallOnly := opts.FirewallOnly + noFirewall := opts.NoFirewall + parse := opts.Parse + jsonOutput := opts.JSONOutput + timeout := opts.Timeout + summaryFile := opts.SummaryFile + safeOutputType := opts.SafeOutputType + filteredIntegrity := opts.FilteredIntegrity + train := opts.Train + format := opts.Format + artifactSets := opts.ArtifactSets + after := opts.After + logsOrchestratorLog.Printf("Starting workflow log download: workflow=%s, count=%d, startDate=%s, endDate=%s, outputDir=%s, summaryFile=%s, safeOutputType=%s, filteredIntegrity=%v, train=%v, format=%s, artifactSets=%v, after=%s", workflowName, count, startDate, endDate, outputDir, summaryFile, safeOutputType, filteredIntegrity, train, format, artifactSets, after) // Validate and resolve artifact sets into a concrete filter (list of artifact base names). From be45a6aac7ea931d2046cac9aca6ed0517d50085 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 10 May 2026 16:05:43 +0000 Subject: [PATCH 3/4] Add draft ADR for DownloadWorkflowLogs options struct refactor --- ...ownload-workflow-logs-to-options-struct.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/adr/31336-refactor-download-workflow-logs-to-options-struct.md diff --git a/docs/adr/31336-refactor-download-workflow-logs-to-options-struct.md b/docs/adr/31336-refactor-download-workflow-logs-to-options-struct.md new file mode 100644 index 00000000000..90cae274955 --- /dev/null +++ b/docs/adr/31336-refactor-download-workflow-logs-to-options-struct.md @@ -0,0 +1,79 @@ +# ADR-31336: Refactor `DownloadWorkflowLogs` to a Typed Options Struct + +**Date**: 2026-05-10 +**Status**: Draft +**Deciders**: Unknown + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +`DownloadWorkflowLogs` in `pkg/cli/logs_orchestrator.go` had grown to a 26-parameter positional signature, including several adjacent `bool` parameters (e.g., `verbose, toolGraph, noStaged, firewallOnly, noFirewall, parse, jsonOutput, ...`). Eight callsites — one production callsite in `pkg/cli/logs_command.go` and seven test callsites — passed arguments by position, making them brittle and hard to read. Adjacent same-typed parameters carry a real risk of silent mis-ordering that the compiler cannot catch, and any addition or reordering of parameters required mechanical churn at every callsite. The function's orchestration logic is otherwise intact and not the subject of this change. + +### Decision + +We will replace the positional parameter list of `DownloadWorkflowLogs` with a typed options struct, `LogsDownloadOptions`, and update the signature to `func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error`. All existing callers will pass a named struct literal, allowing each call to specify only the fields it cares about while zero-valued fields preserve prior defaults. This is an interface refactor only — the orchestration logic, behavior, and field semantics are unchanged. + +### Alternatives Considered + +#### Alternative 1: Keep the positional signature + +Leaving the 26-parameter signature in place avoids any code churn but preserves the legibility and correctness risks (silent mis-ordering between adjacent bools, churn at every callsite when a parameter is added). Given that callsites already required line-by-line `// fieldName` comments to remain readable, this was rejected as continuing to accumulate technical debt. + +#### Alternative 2: Functional options pattern (`WithX(...)` constructors) + +Using a chain of `Option` functions (`opts ...Option`) is idiomatic in some Go libraries and offers extensibility for defaults. It was rejected because the parameter set is a flat configuration bag with no need for hidden state, lazy evaluation, or extensibility hooks; a plain struct literal is more discoverable in IDEs, simpler to read, and aligns with how callers (CLI flag plumbing) already aggregate values. + +#### Alternative 3: Split the function into smaller orchestrators + +Decomposing `DownloadWorkflowLogs` into multiple smaller functions, each with fewer parameters, would address the symptom by reducing surface area. It was rejected for this PR because it is a logic rewrite rather than an interface refactor and would expand scope significantly; the options-struct change is a low-risk first step that does not preclude future decomposition. + +### Consequences + +#### Positive +- Callsites become self-documenting via field names; mis-ordering between same-typed parameters is no longer possible. +- Adding a new optional parameter is a non-breaking change for callers that don't use it (zero value preserves prior behavior). +- Test code becomes substantially shorter — only fields relevant to the test need to be set. + +#### Negative +- The shim inside `DownloadWorkflowLogs` that re-binds every option field to a local variable preserves the existing function body verbatim but adds 25 trivial lines; future cleanup should reference `opts.X` directly and remove the shim. +- The public API of `pkg/cli` changes shape, so any out-of-tree caller (if any exist) must migrate. +- A 26-field struct is itself a smell; the struct documents the shape of the problem but does not solve it. Decomposition (Alternative 3) is still warranted as follow-up. + +#### Neutral +- Field defaults are now expressed implicitly via Go zero values rather than explicit positional arguments; reviewers must verify that "omitted" matches "previously passed as zero." +- All known callers (1 production + 7 tests) have been migrated in this PR; no deprecation period is needed. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Function Signature + +1. The exported function `DownloadWorkflowLogs` **MUST** have the signature `func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error`. +2. The exported type `LogsDownloadOptions` **MUST** be a struct defined in `pkg/cli/logs_orchestrator.go`. +3. New configuration parameters added to the workflow log download flow **MUST** be added as fields on `LogsDownloadOptions` rather than as additional positional parameters. +4. `DownloadWorkflowLogs` **MUST NOT** reintroduce any positional configuration parameters beyond `ctx` and `opts`. + +### Caller Conventions + +1. Callers of `DownloadWorkflowLogs` **MUST** pass a `LogsDownloadOptions` value constructed as a named struct literal (i.e., `LogsDownloadOptions{Field: value, ...}`). +2. Callers **SHOULD** specify only the fields whose non-zero values are meaningful for the call; reliance on zero values to express defaults is **RECOMMENDED**. +3. Test code **SHOULD NOT** restate the full set of fields when a subset suffices. + +### Field Semantics + +1. Zero values for fields on `LogsDownloadOptions` **MUST** preserve the same behavior as the previous positional signature passed `0`, `""`, `false`, or `nil` for the corresponding argument. +2. Changes to the meaning of any existing field on `LogsDownloadOptions` **MUST** be recorded in a new ADR that supersedes or amends this one. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25633125088) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 062bcac38d748250f332ec3dcde3f91eb941e653 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 16:18:07 +0000 Subject: [PATCH 4/4] Clarify logs timeout as TimeoutMinutes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/context_cancellation_test.go | 12 +++++------ pkg/cli/logs_ci_scenario_test.go | 14 ++++++------- pkg/cli/logs_command.go | 2 +- pkg/cli/logs_json_stderr_order_test.go | 28 +++++++++++++------------- pkg/cli/logs_orchestrator.go | 14 ++++++------- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/pkg/cli/context_cancellation_test.go b/pkg/cli/context_cancellation_test.go index d361a59a5af..3fbcbe98e94 100644 --- a/pkg/cli/context_cancellation_test.go +++ b/pkg/cli/context_cancellation_test.go @@ -107,18 +107,18 @@ func TestRunWorkflowsOnGitHubCancellationDuringExecution(t *testing.T) { assert.Error(t, err, "Should return an error") } -// TestDownloadWorkflowLogsTimeoutRespected tests that timeout is converted to context deadline +// TestDownloadWorkflowLogsTimeoutRespected tests that timeout-minutes is respected func TestDownloadWorkflowLogsTimeoutRespected(t *testing.T) { - // Test with a very short timeout (1 second) and verify the function returns quickly + // Use a short timeout in minutes and verify fast-fail behavior still returns quickly ctx := context.Background() start := time.Now() // Use a workflow name that doesn't exist to avoid actual network calls _ = DownloadWorkflowLogs(ctx, LogsDownloadOptions{ - WorkflowName: "nonexistent-workflow-12345", - Count: 100, - OutputDir: "/tmp/test-logs", - Timeout: 1, + WorkflowName: "nonexistent-workflow-12345", + Count: 100, + OutputDir: "/tmp/test-logs", + TimeoutMinutes: 1, }) elapsed := time.Since(start) diff --git a/pkg/cli/logs_ci_scenario_test.go b/pkg/cli/logs_ci_scenario_test.go index 41b44380f22..c3e262b6aba 100644 --- a/pkg/cli/logs_ci_scenario_test.go +++ b/pkg/cli/logs_ci_scenario_test.go @@ -30,13 +30,13 @@ func TestLogsJSONOutputWithNoRuns(t *testing.T) { // Call DownloadWorkflowLogs with parameters that will result in no matching runs // We use a non-existent workflow name to ensure no results err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ - WorkflowName: "nonexistent-workflow-12345", // Workflow that doesn't exist - Count: 2, - OutputDir: tmpDir, - Engine: "copilot", - JSONOutput: true, // THIS IS KEY - Timeout: 10, - SummaryFile: "summary.json", + WorkflowName: "nonexistent-workflow-12345", // Workflow that doesn't exist + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, // THIS IS KEY + TimeoutMinutes: 10, + SummaryFile: "summary.json", }) // Restore stdout and read output diff --git a/pkg/cli/logs_command.go b/pkg/cli/logs_command.go index 4e7ad14326a..9411c85d405 100644 --- a/pkg/cli/logs_command.go +++ b/pkg/cli/logs_command.go @@ -261,7 +261,7 @@ Examples: NoFirewall: noFirewall, Parse: parse, JSONOutput: jsonOutput, - Timeout: timeout, + TimeoutMinutes: timeout, SummaryFile: summaryFile, SafeOutputType: safeOutputType, FilteredIntegrity: filteredIntegrity, diff --git a/pkg/cli/logs_json_stderr_order_test.go b/pkg/cli/logs_json_stderr_order_test.go index 3cdc11f335f..e7c8a96ca34 100644 --- a/pkg/cli/logs_json_stderr_order_test.go +++ b/pkg/cli/logs_json_stderr_order_test.go @@ -37,13 +37,13 @@ func TestLogsJSONOutputBeforeStderr(t *testing.T) { // Call DownloadWorkflowLogs with parameters that will result in no matching runs // This should trigger the warning message path err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ - WorkflowName: "nonexistent-workflow-test-12345", // Workflow that doesn't exist - Count: 2, - OutputDir: tmpDir, - Engine: "copilot", - JSONOutput: true, // THIS IS KEY - Timeout: 10, - SummaryFile: "summary.json", + WorkflowName: "nonexistent-workflow-test-12345", // Workflow that doesn't exist + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, // THIS IS KEY + TimeoutMinutes: 10, + SummaryFile: "summary.json", }) // Close writers first @@ -144,13 +144,13 @@ func TestLogsJSONAndStderrRedirected(t *testing.T) { // Call DownloadWorkflowLogs ctx := context.Background() err := DownloadWorkflowLogs(ctx, LogsDownloadOptions{ - WorkflowName: "nonexistent-workflow-ci-test-67890", - Count: 2, - OutputDir: tmpDir, - Engine: "copilot", - JSONOutput: true, - Timeout: 10, - SummaryFile: "summary.json", + WorkflowName: "nonexistent-workflow-ci-test-67890", + Count: 2, + OutputDir: tmpDir, + Engine: "copilot", + JSONOutput: true, + TimeoutMinutes: 10, + SummaryFile: "summary.json", }) // Close the writer diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 3a780cacf74..c3f21c8e871 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -53,7 +53,7 @@ type LogsDownloadOptions struct { NoFirewall bool Parse bool JSONOutput bool - Timeout int + TimeoutMinutes int SummaryFile string SafeOutputType string FilteredIntegrity bool @@ -82,7 +82,7 @@ func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error { noFirewall := opts.NoFirewall parse := opts.Parse jsonOutput := opts.JSONOutput - timeout := opts.Timeout + timeoutMinutes := opts.TimeoutMinutes summaryFile := opts.SummaryFile safeOutputType := opts.SafeOutputType filteredIntegrity := opts.FilteredIntegrity @@ -153,10 +153,10 @@ func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error { // Start timeout timer if specified var startTime time.Time var timeoutReached bool - if timeout > 0 { + if timeoutMinutes > 0 { startTime = time.Now() if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Timeout set to %d minutes", timeout))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Timeout set to %d minutes", timeoutMinutes))) } } @@ -180,9 +180,9 @@ func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error { } // Check timeout if specified - if timeout > 0 { + if timeoutMinutes > 0 { elapsed := time.Since(startTime).Seconds() - if elapsed >= float64(timeout)*60 { + if elapsed >= float64(timeoutMinutes)*60 { timeoutReached = true if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Timeout reached after %.1f seconds, stopping download", elapsed))) @@ -593,7 +593,7 @@ func DownloadWorkflowLogs(ctx context.Context, opts LogsDownloadOptions) error { Branch: ref, AfterRunID: afterRunID, BeforeRunID: oldestRunID, // Continue from where we left off - Timeout: timeout, + Timeout: timeoutMinutes, } }