diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 8181f37492..1c4dab127d 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1337,12 +1337,12 @@ }, "timeout-minutes": { "type": "integer", - "description": "Workflow timeout in minutes (GitHub Actions standard field). Defaults to 15 minutes for agentic workflows. Has sensible defaults and can typically be omitted.", + "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 15 minutes for agentic workflows.", + "description": "Deprecated: Use 'timeout-minutes' instead. Workflow timeout in minutes. Defaults to 20 minutes for agentic workflows.", "examples": [5, 10, 30], "deprecated": true }, diff --git a/pkg/workflow/compiler_timeout_default_test.go b/pkg/workflow/compiler_timeout_default_test.go new file mode 100644 index 0000000000..0e01f5b401 --- /dev/null +++ b/pkg/workflow/compiler_timeout_default_test.go @@ -0,0 +1,172 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/githubnext/gh-aw/pkg/constants" +) + +func TestDefaultTimeoutMinutesApplied(t *testing.T) { + tests := []struct { + name string + frontmatter string + expectedTimeout int + description string + }{ + { + name: "no timeout specified - should use default", + frontmatter: `--- +on: workflow_dispatch +permissions: + contents: read +engine: copilot +---`, + expectedTimeout: constants.DefaultAgenticWorkflowTimeoutMinutes, + description: "When timeout-minutes is not specified, default should be applied", + }, + { + name: "explicit timeout specified - should use explicit value", + frontmatter: `--- +on: workflow_dispatch +permissions: + contents: read +timeout-minutes: 30 +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 +---`, + expectedTimeout: 45, + description: "When deprecated timeout_minutes is specified, that value should be used", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temporary directory for test + tmpDir, err := os.MkdirTemp("", "timeout-default-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create test workflow file + testContent := tt.frontmatter + "\n\n# Test Workflow\n\nTest workflow for timeout-minutes default behavior.\n" + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read the generated lock file + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Check that the expected timeout is present in the lock file + expectedTimeoutStr := "timeout-minutes: " + string(rune(tt.expectedTimeout+'0')) + if tt.expectedTimeout >= 10 { + expectedTimeoutStr = "timeout-minutes: " + intToString(tt.expectedTimeout) + } + + if !strings.Contains(string(lockContent), expectedTimeoutStr) { + t.Errorf("%s\nExpected timeout-minutes: %d in compiled workflow, but not found\nLock file content:\n%s", + tt.description, tt.expectedTimeout, string(lockContent)) + } + + // Verify the timeout appears in the execution step (not just anywhere in the file) + // The timeout should be in a step, not in comments + lines := strings.Split(string(lockContent), "\n") + foundTimeoutInStep := false + for _, line := range lines { + trimmed := strings.TrimSpace(line) + // Skip comments + if strings.HasPrefix(trimmed, "#") { + continue + } + if strings.Contains(trimmed, expectedTimeoutStr) { + foundTimeoutInStep = true + break + } + } + + if !foundTimeoutInStep { + t.Errorf("%s\nExpected timeout-minutes: %d in a workflow step (not in comments)\nLock file content:\n%s", + tt.description, tt.expectedTimeout, string(lockContent)) + } + }) + } +} + +// Helper function to convert int to string +func intToString(n int) string { + if n == 0 { + return "0" + } + if n < 0 { + return "-" + intToString(-n) + } + result := "" + for n > 0 { + result = string(rune('0'+n%10)) + result + n /= 10 + } + return result +} + +func TestDefaultTimeoutMinutesConstantValue(t *testing.T) { + // This test ensures the constant is set to the expected value + // If this test fails, it means the constant was changed and documentation + // should be updated accordingly + expectedDefault := 20 + if constants.DefaultAgenticWorkflowTimeoutMinutes != expectedDefault { + t.Errorf("DefaultAgenticWorkflowTimeoutMinutes constant is %d, but test expects %d. "+ + "If you changed the constant, please update the schema documentation in pkg/parser/schemas/main_workflow_schema.json", + constants.DefaultAgenticWorkflowTimeoutMinutes, expectedDefault) + } +} + +func TestSchemaDocumentationMatchesConstant(t *testing.T) { + // Read the schema file + schemaPath := filepath.Join("..", "parser", "schemas", "main_workflow_schema.json") + schemaContent, err := os.ReadFile(schemaPath) + if err != nil { + t.Fatalf("Failed to read schema file: %v", err) + } + + // Check that the schema mentions the correct default value + expectedText := "Defaults to 20 minutes for agentic workflows" + if !strings.Contains(string(schemaContent), expectedText) { + t.Errorf("Schema documentation does not mention the correct default timeout.\n"+ + "Expected to find: %q\n"+ + "Please update the timeout-minutes description in %s to match DefaultAgenticWorkflowTimeoutMinutes constant (%d minutes)", + expectedText, schemaPath, constants.DefaultAgenticWorkflowTimeoutMinutes) + } + + // Count occurrences - should appear at least twice (timeout-minutes and timeout_minutes deprecated) + 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", + expectedText, occurrences) + } +}