Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand Down
6 changes: 6 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/parser/schema_compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ func TestGetSafeOutputTypeKeys(t *testing.T) {
"runs-on",
"messages",
"needs",
"timeout-minutes",
}

for _, meta := range metaFields {
Expand Down
6 changes: 6 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Comment thread
dsyme marked this conversation as resolved.
},
"needs": {
"type": "array",
"description": "Explicit additional custom workflow jobs that the consolidated safe_outputs job should depend on.",
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/compile_outputs_comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/compile_outputs_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/compile_outputs_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/compiler_jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
13 changes: 12 additions & 1 deletion pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
123 changes: 122 additions & 1 deletion pkg/workflow/compiler_safe_outputs_job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/compiler_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/safe_output_refactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
42 changes: 42 additions & 0 deletions pkg/workflow/safe_outputs_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Comment thread
Copilot marked this conversation as resolved.
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
}
Comment thread
Copilot marked this conversation as resolved.
}
}

// Handle messages configuration
if messages, exists := outputMap["messages"]; exists {
if messagesMap, ok := messages.(map[string]any); ok {
Expand Down
Loading