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
2 changes: 1 addition & 1 deletion pkg/parser/schema_additional_properties_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func TestValidProperties_NotRejected(t *testing.T) {
"tools": map[string]any{
"github": nil,
},
"timeout_minutes": 10,
"timeout-minutes": 10,
"runs-on": "ubuntu-latest",
"safe-outputs": map[string]any{
"create-issue": nil,
Expand Down
15 changes: 4 additions & 11 deletions pkg/parser/schema_deprecated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,18 @@ func TestGetMainWorkflowDeprecatedFields(t *testing.T) {
t.Fatalf("GetMainWorkflowDeprecatedFields() error = %v", err)
}

// Check that timeout_minutes is in the list
// Check that timeout_minutes is NOT in the list (it was removed from schema completely)
// Users should use the timeout-minutes-migration codemod to migrate their workflows
found := false
for _, field := range deprecatedFields {
if field.Name == "timeout_minutes" {
found = true
// Check that it has a replacement suggestion
if field.Replacement == "" {
t.Errorf("Expected timeout_minutes to have a replacement suggestion, got empty string")
}
// Check that replacement is timeout-minutes
if field.Replacement != "timeout-minutes" {
t.Errorf("Expected replacement 'timeout-minutes', got '%s'", field.Replacement)
}
break
}
}

if !found {
t.Error("Expected timeout_minutes to be in the deprecated fields list")
if found {
t.Error("timeout_minutes should NOT be in the deprecated fields list (removed from schema)")
}
}

Expand Down
12 changes: 2 additions & 10 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) {
"permissions": "read-all",
"run-name": "Test Run",
"runs-on": "ubuntu-latest",
"timeout_minutes": 30,
"timeout-minutes": 30,
"concurrency": "test",
"env": map[string]string{"TEST": "value"},
"if": "true",
Expand Down Expand Up @@ -175,14 +175,6 @@ func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) {
wantErr: true,
errContains: "additional properties",
},
{
name: "invalid type for timeout_minutes",
frontmatter: map[string]any{
"timeout_minutes": "not-a-number",
},
wantErr: true,
errContains: "got string, want integer",
},
{
name: "valid frontmatter with complex on object",
frontmatter: map[string]any{
Expand Down Expand Up @@ -856,7 +848,7 @@ func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) {
name: "missing required on field with other valid fields",
frontmatter: map[string]any{
"engine": "copilot",
"timeout_minutes": 30,
"timeout-minutes": 30,
"permissions": map[string]any{
"issues": "write",
},
Expand Down
6 changes: 0 additions & 6 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1791,12 +1791,6 @@
"description": "Workflow timeout in minutes (GitHub Actions standard field). Defaults to 20 minutes for agentic workflows. Has sensible defaults and can typically be omitted.",
"examples": [5, 10, 30]
},
"timeout_minutes": {
"type": "integer",
"description": "Deprecated: Use 'timeout-minutes' instead. Workflow timeout in minutes. Defaults to 20 minutes for agentic workflows.",
"examples": [5, 10, 30],
"deprecated": true
},
"concurrency": {
"description": "Concurrency control to limit concurrent workflow runs (GitHub Actions standard field). Supports two forms: simple string for basic group isolation, or object with cancel-in-progress option for advanced control. Agentic workflows enhance this with automatic per-engine concurrency policies (defaults to single job per engine across all workflows) and token-based rate limiting. Default behavior: workflows in the same group queue sequentially unless cancel-in-progress is true. See https://docs.github.com/en/actions/using-jobs/using-concurrency",
"oneOf": [
Expand Down
5 changes: 2 additions & 3 deletions pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,8 @@ mkdir -p "$HOME/.cache"

// Add timeout at step level (GitHub Actions standard)
if workflowData.TimeoutMinutes != "" {
// Strip both possible prefixes (timeout_minutes or timeout-minutes)
timeoutValue := strings.TrimPrefix(workflowData.TimeoutMinutes, "timeout_minutes: ")
timeoutValue = strings.TrimPrefix(timeoutValue, "timeout-minutes: ")
// Strip timeout-minutes prefix
timeoutValue := strings.TrimPrefix(workflowData.TimeoutMinutes, "timeout-minutes: ")
stepLines = append(stepLines, fmt.Sprintf(" timeout-minutes: %s", timeoutValue))
} else {
stepLines = append(stepLines, fmt.Sprintf(" timeout-minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))) // Default timeout for agentic workflows
Expand Down
11 changes: 1 addition & 10 deletions pkg/workflow/compiler_orchestrator_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package workflow

import (
"fmt"
"os"
"strings"

"github.com/githubnext/gh-aw/pkg/console"
"github.com/githubnext/gh-aw/pkg/logger"
"github.com/githubnext/gh-aw/pkg/parser"
"github.com/goccy/go-yaml"
Expand Down Expand Up @@ -145,15 +143,8 @@ func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData
workflowData.Features = c.extractFeatures(frontmatter)
workflowData.If = c.extractIfCondition(frontmatter)

// Prefer timeout-minutes (new) over timeout_minutes (deprecated)
// Extract timeout-minutes (canonical form)
workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(frontmatter, "timeout-minutes")
if workflowData.TimeoutMinutes == "" {
workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(frontmatter, "timeout_minutes")
if workflowData.TimeoutMinutes != "" {
// Emit deprecation warning
fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Field 'timeout_minutes' is deprecated. Please use 'timeout-minutes' instead to follow GitHub Actions naming convention."))
}
}

workflowData.RunsOn = c.extractTopLevelYAMLSection(frontmatter, "runs-on")
workflowData.Environment = c.extractTopLevelYAMLSection(frontmatter, "environment")
Expand Down
19 changes: 3 additions & 16 deletions pkg/workflow/compiler_timeout_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,6 @@ engine: copilot
expectedTimeout: 30,
description: "When timeout-minutes is explicitly specified, that value should be used",
},
{
name: "deprecated timeout_minutes specified - should use that value",
frontmatter: `---
on: workflow_dispatch
permissions:
contents: read
timeout_minutes: 45
engine: copilot
strict: false
---`,
expectedTimeout: 45,
description: "When deprecated timeout_minutes is specified, that value should be used",
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -167,10 +154,10 @@ func TestSchemaDocumentationMatchesConstant(t *testing.T) {
expectedText, schemaPath, int(constants.DefaultAgenticWorkflowTimeout/time.Minute))
}

// Count occurrences - should appear at least twice (timeout-minutes and timeout_minutes deprecated)
// Count occurrences - should appear exactly once (only timeout-minutes, timeout_minutes removed)
occurrences := strings.Count(string(schemaContent), expectedText)
if occurrences < 2 {
t.Errorf("Expected to find at least 2 occurrences of %q in schema (for timeout-minutes and timeout_minutes fields), but found %d",
if occurrences != 1 {
t.Errorf("Expected to find exactly 1 occurrence of %q in schema (for timeout-minutes field only), but found %d",
expectedText, occurrences)
}
}
5 changes: 2 additions & 3 deletions pkg/workflow/copilot_engine_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,9 +547,8 @@ COPILOT_CLI_INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"

// Add timeout at step level (GitHub Actions standard)
if workflowData.TimeoutMinutes != "" {
// Strip both possible prefixes (timeout_minutes or timeout-minutes)
timeoutValue := strings.TrimPrefix(workflowData.TimeoutMinutes, "timeout_minutes: ")
timeoutValue = strings.TrimPrefix(timeoutValue, "timeout-minutes: ")
// Strip timeout-minutes prefix
timeoutValue := strings.TrimPrefix(workflowData.TimeoutMinutes, "timeout-minutes: ")
stepLines = append(stepLines, fmt.Sprintf(" timeout-minutes: %s", timeoutValue))
} else {
stepLines = append(stepLines, fmt.Sprintf(" timeout-minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))) // Default timeout for agentic workflows
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func (jm *JobManager) renderJob(job *Job) string {
fmt.Fprintf(&yaml, " %s\n", job.Concurrency)
}

// Add timeout_minutes if specified
// Add timeout-minutes if specified
if job.TimeoutMinutes > 0 {
fmt.Fprintf(&yaml, " timeout-minutes: %d\n", job.TimeoutMinutes)
}
Expand Down
71 changes: 0 additions & 71 deletions pkg/workflow/timeout_minutes_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/workflow/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) error
}

if data.TimeoutMinutes == "" {
data.TimeoutMinutes = fmt.Sprintf("timeout_minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))
data.TimeoutMinutes = fmt.Sprintf("timeout-minutes: %d", int(constants.DefaultAgenticWorkflowTimeout/time.Minute))
}

if data.RunsOn == "" {
Expand Down
10 changes: 5 additions & 5 deletions scripts/generate-schema-docs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,16 @@ for (const ref of allRefs) {
}
}

// Test 9: Verify that deprecated fields are excluded from output
// Test 9: Verify that deprecated fields are excluded from output and schema
allPassed &= assertNotContains(output, "timeout_minutes:", "Deprecated field timeout_minutes should NOT be in output");

allPassed &= assertContains(output, "timeout-minutes:", "Non-deprecated field timeout-minutes should be in output");

// Verify the schema actually has the deprecated field (sanity check)
if (schema.properties && schema.properties.timeout_minutes && schema.properties.timeout_minutes.deprecated === true) {
console.log("✓ PASS: Schema has timeout_minutes marked as deprecated");
// Verify the schema does NOT have the deprecated field (it was removed completely)
if (schema.properties && !schema.properties.timeout_minutes) {
console.log("✓ PASS: Schema does not have timeout_minutes field (removed completely)");
} else {
console.error("❌ FAIL: Schema should have timeout_minutes marked as deprecated");
console.error("❌ FAIL: Schema should NOT have timeout_minutes field");
allPassed = false;
}

Expand Down
Loading