diff --git a/Makefile b/Makefile index 4368022f4c2..4b4d8656666 100644 --- a/Makefile +++ b/Makefile @@ -371,8 +371,13 @@ test-impacted-go: if (current_pkg != "") print current_pkg "\t^(" pattern ")$$"; \ } \ ' "$$SELECTED_GO_TESTS" | while IFS=" " read -r pkg pattern; do \ - echo "Running impacted Go unit tests in $$pkg with pattern $$pattern"; \ - go test -v -parallel=4 -timeout=10m -short -run "$$pattern" "$$pkg" || exit 1; \ + if [ "$${#pattern}" -gt 30000 ]; then \ + echo "Running impacted Go unit tests in $$pkg (pattern too long, running full package)"; \ + go test -v -parallel=4 -timeout=10m -short "$$pkg" || exit 1; \ + else \ + echo "Running impacted Go unit tests in $$pkg with pattern $$pattern"; \ + go test -v -parallel=4 -timeout=10m -short -run "$$pattern" "$$pkg" || exit 1; \ + fi; \ done || exit 1; \ exit 0; \ fi; \ diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 8bffbf7644b..64e29f2eb8f 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -7496,6 +7496,12 @@ safe-outputs: # (optional) concurrency-group: "example-value" + # Timeout for the safe_outputs job in minutes. Defaults to 45 minutes. Increase + # for workflows with many sequential safe output operations (e.g. + # push_to_pull_request_branch against large repositories). + # (optional) + timeout-minutes: 120 + # Explicit additional custom workflow jobs that the consolidated safe_outputs job # should depend on. # (optional) diff --git a/pkg/parser/schema_compiler.go b/pkg/parser/schema_compiler.go index 2705c457851..e70d87feb0d 100644 --- a/pkg/parser/schema_compiler.go +++ b/pkg/parser/schema_compiler.go @@ -157,6 +157,7 @@ var safeOutputMetaFields = map[string]bool{ "runs-on": true, "messages": true, "needs": true, + "timeout-minutes": true, } // GetSafeOutputTypeKeys returns the list of safe output type keys from the embedded main workflow schema. diff --git a/pkg/parser/schema_test.go b/pkg/parser/schema_test.go index 78f4ce24309..8b49019c19f 100644 --- a/pkg/parser/schema_test.go +++ b/pkg/parser/schema_test.go @@ -959,6 +959,7 @@ func TestGetSafeOutputTypeKeys(t *testing.T) { "runs-on", "messages", "needs", + "timeout-minutes", } for _, meta := range metaFields { diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index cd155d572cf..6e5024bd49c 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -10210,6 +10210,12 @@ "description": "Concurrency group for the safe-outputs job. When set, the safe-outputs job will use this concurrency group with cancel-in-progress: false. Supports GitHub Actions expressions.", "examples": ["my-workflow-safe-outputs", "safe-outputs-${{ github.repository }}"] }, + "timeout-minutes": { + "type": "integer", + "description": "Timeout for the safe_outputs job in minutes. Defaults to 45 minutes. Increase for workflows with many sequential safe output operations (e.g. push_to_pull_request_branch against large repositories).", + "minimum": 1, + "examples": [30, 60, 120] + }, "needs": { "type": "array", "description": "Explicit additional custom workflow jobs that the consolidated safe_outputs job should depend on.", diff --git a/pkg/workflow/compile_outputs_comment_test.go b/pkg/workflow/compile_outputs_comment_test.go index 08c51ad40af..a9df3a80373 100644 --- a/pkg/workflow/compile_outputs_comment_test.go +++ b/pkg/workflow/compile_outputs_comment_test.go @@ -268,8 +268,8 @@ This workflow tests the safe_outputs job generation. } // Verify job properties - if !strings.Contains(lockContent, "timeout-minutes: 15") { - t.Error("Expected 10-minute timeout in safe_outputs job") + if !strings.Contains(lockContent, "timeout-minutes: 45") { + t.Error("Expected 45-minute timeout in safe_outputs job") } if !strings.Contains(lockContent, "permissions:\n contents: read\n discussions: write\n issues: write\n pull-requests: write") { diff --git a/pkg/workflow/compile_outputs_issue_test.go b/pkg/workflow/compile_outputs_issue_test.go index b51c438151b..4db35a338f5 100644 --- a/pkg/workflow/compile_outputs_issue_test.go +++ b/pkg/workflow/compile_outputs_issue_test.go @@ -396,8 +396,8 @@ This workflow tests the create-issue job generation. } // Verify job properties - if !strings.Contains(lockContent, "timeout-minutes: 15") { - t.Error("Expected 15-minute timeout in consolidated safe_outputs job") + if !strings.Contains(lockContent, "timeout-minutes: 45") { + t.Error("Expected 45-minute timeout in consolidated safe_outputs job") } if !strings.Contains(lockContent, "issues: write") { diff --git a/pkg/workflow/compile_outputs_label_test.go b/pkg/workflow/compile_outputs_label_test.go index a7d90b3e2cd..f7c0a6aa20f 100644 --- a/pkg/workflow/compile_outputs_label_test.go +++ b/pkg/workflow/compile_outputs_label_test.go @@ -128,8 +128,8 @@ This workflow tests the safe_outputs job generation. } // Verify job properties - if !strings.Contains(lockContent, "timeout-minutes: 15") { - t.Error("Expected 10-minute timeout in safe_outputs job") + if !strings.Contains(lockContent, "timeout-minutes: 45") { + t.Error("Expected 45-minute timeout in safe_outputs job") } if !strings.Contains(lockContent, "permissions:\n contents: read\n issues: write\n pull-requests: write") { diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index d5a12d5262d..43be9992e67 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -1527,9 +1527,9 @@ Test content` t.Error("Expected add_labels in handler config") } - // Check that the consolidated job has correct timeout (15 minutes for consolidated job) - if !strings.Contains(yamlStr, "timeout-minutes: 15") { - t.Error("Expected timeout-minutes: 15 for consolidated safe_outputs job") + // Check that the consolidated job has correct timeout (45 minutes for consolidated job) + if !strings.Contains(yamlStr, "timeout-minutes: 45") { + t.Error("Expected timeout-minutes: 45 for consolidated safe_outputs job") } } diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 1a21f015583..3937406e749 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -615,13 +615,24 @@ func (c *Compiler) buildSafeOutputsJobFromParts( // secrets (e.g. for GitHub App token minting) are accessible in this job. safeOutputsEnvironment := resolveSafeOutputsEnvironment(data) + // Use the configured timeout or fall back to the default of 45 minutes. + // The previous default was 15 minutes, which is insufficient for workflows + // with many sequential safe output operations (e.g. push_to_pull_request_branch + // against large monorepos). 45 minutes is the new default; users can override + // via safe-outputs.timeout-minutes in frontmatter. + const defaultSafeOutputsTimeoutMinutes = 45 + timeoutMinutes := defaultSafeOutputsTimeoutMinutes + if data.SafeOutputs.TimeoutMinutes > 0 { + timeoutMinutes = data.SafeOutputs.TimeoutMinutes + } + job := &Job{ Name: "safe_outputs", If: RenderCondition(jobCondition), RunsOn: c.formatFrameworkJobRunsOn(data), Environment: c.indentYAMLLines(safeOutputsEnvironment, " "), Permissions: permissions.RenderToYAML(), - TimeoutMinutes: 15, // Slightly longer timeout for consolidated job with multiple steps + TimeoutMinutes: timeoutMinutes, Concurrency: concurrency, Env: jobEnv, Steps: steps, diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index 75bf0fdcd96..fbc1c6d72be 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -167,7 +167,7 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { } // Verify timeout is set - assert.Equal(t, 15, job.TimeoutMinutes) + assert.Equal(t, 45, job.TimeoutMinutes) // Verify job condition is set assert.NotEmpty(t, job.If) @@ -264,6 +264,53 @@ func TestBuildConsolidatedSafeOutputsJobNeedsIncludesConfiguredDependencies(t *t assert.Equal(t, 1, secretsFetcherCount, "duplicate configured dependencies should be deduplicated") } +// TestBuildConsolidatedSafeOutputsJobTimeoutMinutes tests that the timeout-minutes field +// is correctly applied to the safe_outputs job, with 45 minutes as the default. +func TestBuildConsolidatedSafeOutputsJobTimeoutMinutes(t *testing.T) { + tests := []struct { + name string + timeoutMinutes int + expectedTimeout int + }{ + { + name: "default timeout (0 = unset) falls back to 45 minutes", + timeoutMinutes: 0, + expectedTimeout: 45, + }, + { + name: "custom timeout of 30 minutes", + timeoutMinutes: 30, + expectedTimeout: 30, + }, + { + name: "custom timeout of 120 minutes", + timeoutMinutes: 120, + expectedTimeout: 120, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + compiler.jobManager = NewJobManager() + + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{TitlePrefix: "[Test] "}, + TimeoutMinutes: tt.timeoutMinutes, + }, + } + + job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(constants.AgentJobName), "test-workflow.md") + require.NoError(t, err, "Should build job without error") + require.NotNil(t, job, "Job should not be nil") + + assert.Equal(t, tt.expectedTimeout, job.TimeoutMinutes, "Job timeout should match expected value") + }) + } +} + func TestCompileSafeOutputsNeedsForCustomCredentialJob(t *testing.T) { tempDir := t.TempDir() workflowPath := filepath.Join(tempDir, "safe-needs.md") @@ -327,6 +374,80 @@ jobs: assert.Contains(t, string(lockContent), "needs.secrets_fetcher.outputs.app_id", "compiled workflow should retain custom needs output references") } +func TestCompileSafeOutputsTimeoutMinutesFromFrontmatter(t *testing.T) { + tests := []struct { + name string + frontmatter string + expectedTimeout string + }{ + { + name: "default timeout is 45 minutes", + frontmatter: `--- +on: + workflow_dispatch: {} +permissions: + contents: read +safe-outputs: + noop: + report-as-issue: false +--- +# Test +`, + expectedTimeout: "timeout-minutes: 45", + }, + { + name: "custom timeout of 30 minutes from frontmatter", + frontmatter: `--- +on: + workflow_dispatch: {} +permissions: + contents: read +safe-outputs: + timeout-minutes: 30 + noop: + report-as-issue: false +--- +# Test +`, + expectedTimeout: "timeout-minutes: 30", + }, + { + name: "custom timeout of 120 minutes from frontmatter", + frontmatter: `--- +on: + workflow_dispatch: {} +permissions: + contents: read +safe-outputs: + timeout-minutes: 120 + noop: + report-as-issue: false +--- +# Test +`, + expectedTimeout: "timeout-minutes: 120", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + workflowPath := filepath.Join(tempDir, "safe-timeout.md") + require.NoError(t, os.WriteFile(workflowPath, []byte(tt.frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + lockContent, err := os.ReadFile(lockPath) + require.NoError(t, err) + + assert.Contains(t, string(lockContent), tt.expectedTimeout, + "safe_outputs job should have the expected timeout-minutes value") + }) + } +} + func TestBuildJobLevelSafeOutputEnvVars(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index dd08249fae6..7597c03bf0d 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -714,6 +714,7 @@ type SafeOutputsConfig struct { Needs []string `yaml:"needs,omitempty"` // Additional custom workflow jobs that safe_outputs should depend on Environment string `yaml:"environment,omitempty"` // Override the GitHub deployment environment for the safe-outputs job (defaults to the top-level environment: field) Actions map[string]*SafeOutputActionConfig `yaml:"actions,omitempty"` // Custom GitHub Actions mounted as safe output tools (resolved at compile time) + TimeoutMinutes int `yaml:"timeout-minutes,omitempty"` // Timeout for the safe_outputs job in minutes. Defaults to 45. AutoInjectedCreateIssue bool `yaml:"-"` // Internal: true when create-issues was automatically injected by the compiler (not user-configured) } diff --git a/pkg/workflow/safe_output_refactor_test.go b/pkg/workflow/safe_output_refactor_test.go index a066e58b157..2fee4c00b4c 100644 --- a/pkg/workflow/safe_output_refactor_test.go +++ b/pkg/workflow/safe_output_refactor_test.go @@ -157,8 +157,8 @@ safe-outputs: t.Errorf("Expected output %q or outputs section not found", tt.expectedOutput) } - // Verify timeout is set (consolidated safe_outputs job uses 15 minute timeout) - if !strings.Contains(yamlStr, "timeout-minutes: 15") && !strings.Contains(yamlStr, "timeout-minutes:") { + // Verify timeout is set (consolidated safe_outputs job uses 45 minute default timeout) + if !strings.Contains(yamlStr, "timeout-minutes: 45") && !strings.Contains(yamlStr, "timeout-minutes:") { t.Error("Expected timeout-minutes not found in output") } diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index 3899b7c28e2..048509641fc 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -532,6 +532,48 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } + // Handle timeout-minutes configuration + if timeoutMinutes, exists := outputMap["timeout-minutes"]; exists { + switch v := timeoutMinutes.(type) { + case int: + if v >= 1 { + config.TimeoutMinutes = v + } + case int64: + if v >= 1 { + if v > int64(math.MaxInt) { + safeOutputsConfigLog.Printf("timeout-minutes: int64 value %d exceeds platform int range, clamping to %d", v, math.MaxInt) + config.TimeoutMinutes = math.MaxInt + } else { + config.TimeoutMinutes = int(v) + } + } + case uint64: + if v >= 1 { + if v > uint64(math.MaxInt) { + safeOutputsConfigLog.Printf("timeout-minutes: uint64 value %d exceeds platform int range, clamping to %d", v, math.MaxInt) + config.TimeoutMinutes = math.MaxInt + } else { + config.TimeoutMinutes = int(v) + } + } + case float64: + // Reject NaN/Inf and out-of-range floats before narrowing — int(NaN)/int(±Inf) + // are implementation-defined and can produce surprising values. + if v != v || v > float64(math.MaxInt) || v < float64(math.MinInt) { + safeOutputsConfigLog.Printf("timeout-minutes: float value %.2f is out of range, ignoring", v) + break + } + intVal := int(v) + if v != float64(intVal) { + safeOutputsConfigLog.Printf("timeout-minutes: float value %.2f truncated to integer %d", v, intVal) + } + if intVal >= 1 { + config.TimeoutMinutes = intVal + } + } + } + // Handle messages configuration if messages, exists := outputMap["messages"]; exists { if messagesMap, ok := messages.(map[string]any); ok {