diff --git a/pkg/workflow/copilot_engine_execution.go b/pkg/workflow/copilot_engine_execution.go index ca294d3add5..c7f3fa64c68 100644 --- a/pkg/workflow/copilot_engine_execution.go +++ b/pkg/workflow/copilot_engine_execution.go @@ -24,10 +24,12 @@ package workflow import ( "fmt" "maps" + "os" "strconv" "strings" "time" + "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" ) @@ -236,6 +238,19 @@ COPILOT_CLI_INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" copilotGitHubToken = "${{ secrets.COPILOT_GITHUB_TOKEN }}" } + // Warn when copilot-requests is enabled and timeout-minutes > 60. + // The github.token Copilot session is bound at creation time and expires after ~1 hour, + // causing silent mid-run authentication failures for long-running workflows. + if useCopilotRequests && workflowData.TimeoutMinutes != "" { + timeoutVal := parseTimeoutMinutesInt(workflowData.TimeoutMinutes) + if timeoutVal > 60 { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + "features: copilot-requests: true with timeout-minutes > 60 may cause mid-run "+ + "authentication failures: the github.token Copilot session expires after ~1 hour. "+ + "Set timeout-minutes <= 60 or configure COPILOT_GITHUB_TOKEN as a PAT instead.")) + } + } + env := map[string]string{ "XDG_CONFIG_HOME": "/home/runner", "COPILOT_AGENT_RUNNER_TYPE": "STANDALONE", @@ -452,3 +467,19 @@ func generateCopilotSessionFileCopyStep() GitHubActionStep { return GitHubActionStep(step) } + +// parseTimeoutMinutesInt parses the integer value from a timeout-minutes string. +// The input is expected in the form "timeout-minutes: N" (e.g. "timeout-minutes: 90"). +// GitHub Actions expression values (e.g. "${{ inputs.timeout }}") are skipped and return 0. +// Returns 0 if the value is an expression or cannot be parsed as an integer. +func parseTimeoutMinutesInt(timeoutMinutes string) int { + val := strings.TrimPrefix(timeoutMinutes, "timeout-minutes: ") + if strings.HasPrefix(val, "${{") { + return 0 // GitHub Actions expression – skip + } + n, err := strconv.Atoi(strings.TrimSpace(val)) + if err != nil { + return 0 + } + return n +} diff --git a/pkg/workflow/copilot_requests_timeout_warning_test.go b/pkg/workflow/copilot_requests_timeout_warning_test.go new file mode 100644 index 00000000000..6f0c347c7e7 --- /dev/null +++ b/pkg/workflow/copilot_requests_timeout_warning_test.go @@ -0,0 +1,162 @@ +//go:build !integration + +package workflow + +import ( + "bytes" + "io" + "os" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/constants" +) + +// TestParseTimeoutMinutesInt tests the parseTimeoutMinutesInt helper. +func TestParseTimeoutMinutesInt(t *testing.T) { + tests := []struct { + name string + input string + expected int + }{ + { + name: "parses integer value", + input: "timeout-minutes: 90", + expected: 90, + }, + { + name: "parses value at boundary", + input: "timeout-minutes: 60", + expected: 60, + }, + { + name: "parses small value", + input: "timeout-minutes: 20", + expected: 20, + }, + { + name: "expression returns zero", + input: "timeout-minutes: ${{ inputs.timeout }}", + expected: 0, + }, + { + name: "empty string returns zero", + input: "", + expected: 0, + }, + { + name: "bare integer without prefix returns zero (non-parseable)", + input: "not-a-number", + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseTimeoutMinutesInt(tt.input) + if got != tt.expected { + t.Errorf("parseTimeoutMinutesInt(%q) = %d, want %d", tt.input, got, tt.expected) + } + }) + } +} + +// TestCopilotRequestsTimeoutWarning tests that a compile-time warning is emitted when +// features: copilot-requests: true is combined with timeout-minutes > 60. +func TestCopilotRequestsTimeoutWarning(t *testing.T) { + engine := NewCopilotEngine() + + tests := []struct { + name string + timeoutMins string + features map[string]any + expectWarning bool + }{ + { + name: "warning emitted: copilot-requests enabled and timeout > 60", + timeoutMins: "timeout-minutes: 90", + features: map[string]any{ + string(constants.CopilotRequestsFeatureFlag): true, + }, + expectWarning: true, + }, + { + name: "no warning: copilot-requests enabled but timeout == 60", + timeoutMins: "timeout-minutes: 60", + features: map[string]any{ + string(constants.CopilotRequestsFeatureFlag): true, + }, + expectWarning: false, + }, + { + name: "no warning: copilot-requests enabled but timeout < 60", + timeoutMins: "timeout-minutes: 30", + features: map[string]any{ + string(constants.CopilotRequestsFeatureFlag): true, + }, + expectWarning: false, + }, + { + name: "no warning: copilot-requests disabled and timeout > 60", + timeoutMins: "timeout-minutes: 90", + features: map[string]any{}, + expectWarning: false, + }, + { + name: "no warning: copilot-requests absent and timeout > 60", + timeoutMins: "timeout-minutes: 90", + features: nil, + expectWarning: false, + }, + { + name: "no warning: expression timeout is gracefully skipped", + timeoutMins: "timeout-minutes: ${{ inputs.timeout }}", + features: map[string]any{ + string(constants.CopilotRequestsFeatureFlag): true, + }, + expectWarning: false, + }, + { + name: "no warning: empty timeout is skipped", + timeoutMins: "", + features: map[string]any{ + string(constants.CopilotRequestsFeatureFlag): true, + }, + expectWarning: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workflowData := &WorkflowData{ + TimeoutMinutes: tt.timeoutMins, + Features: tt.features, + Tools: map[string]any{}, + } + + // Capture stderr + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + engine.GetExecutionSteps(workflowData, "/tmp/test.log") + + w.Close() + os.Stderr = oldStderr + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + stderrOutput := buf.String() + + expectedPhrase := "copilot-requests: true with timeout-minutes > 60" + if tt.expectWarning { + if !strings.Contains(stderrOutput, expectedPhrase) { + t.Errorf("expected warning containing %q in stderr, got:\n%s", expectedPhrase, stderrOutput) + } + } else { + if strings.Contains(stderrOutput, expectedPhrase) { + t.Errorf("did not expect warning %q in stderr, got:\n%s", expectedPhrase, stderrOutput) + } + } + }) + } +}