From 42502c2d636f2e0e2290569df94b5e07add03cbf Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 15 Aug 2025 16:13:53 +0100 Subject: [PATCH] max-turns feature --- docs/frontmatter.md | 17 +- pkg/cli/templates/instructions.md | 4 +- pkg/parser/schemas/main_workflow_schema.json | 4 + pkg/workflow/agentic_engine.go | 8 + pkg/workflow/claude_engine.go | 2 + pkg/workflow/claude_engine_test.go | 6 +- pkg/workflow/codex_engine.go | 1 + pkg/workflow/compiler.go | 31 +++ pkg/workflow/max_turns_test.go | 229 +++++++++++++++++++ pkg/workflow/max_turns_validation_test.go | 164 +++++++++++++ 10 files changed, 461 insertions(+), 5 deletions(-) create mode 100644 pkg/workflow/max_turns_test.go create mode 100644 pkg/workflow/max_turns_validation_test.go diff --git a/docs/frontmatter.md b/docs/frontmatter.md index a633c657770..d66744fca27 100644 --- a/docs/frontmatter.md +++ b/docs/frontmatter.md @@ -21,6 +21,7 @@ The YAML frontmatter supports standard GitHub Actions properties plus additional - `engine`: AI engine configuration (claude/codex) - `tools`: Available tools and MCP servers for the AI engine - `stop-time`: Deadline when workflow should stop running (absolute or relative time) +- `max-turns`: Maximum number of chat iterations per run - `alias`: Alias name for the workflow - `ai-reaction`: Emoji reaction to add/remove on triggering GitHub item - `cache`: Cache configuration for workflow dependencies @@ -186,14 +187,24 @@ engine: ## Cost Control Options -### Stop Time (`stop-time:`) +### Maximum Turns (`max-turns:`) -Automatically disable workflow after a deadline. Supports both absolute timestamps and relative time deltas: +Limit the number of chat iterations within a single agentic run: -**Absolute time (multiple formats supported):** ```yaml +max-turns: 5 ``` +**Behavior:** +1. Passes the limit to the AI engine (e.g., Claude Code action) +2. Engine stops iterating when the turn limit is reached +3. Helps prevent runaway chat loops and control costs +4. Only applies to engines that support turn limiting (currently Claude) + +### Stop Time (`stop-time:`) + +Automatically disable workflow after a deadline: + **Relative time delta (calculated from compilation time):** ```yaml stop-time: "+25h" # 25 hours from now diff --git a/pkg/cli/templates/instructions.md b/pkg/cli/templates/instructions.md index 593c5183bfb..71c14786c44 100644 --- a/pkg/cli/templates/instructions.md +++ b/pkg/cli/templates/instructions.md @@ -74,6 +74,7 @@ The YAML frontmatter supports these fields: - `claude:` - Claude-specific tools - Custom tool names for MCP servers +- **`max-turns:`** - Maximum chat iterations per run (integer) - **`stop-time:`** - Deadline for workflow. Can be absolute timestamp ("YYYY-MM-DD HH:MM:SS") or relative delta (+25h, +3d, +1d12h30m) - **`alias:`** - Alternative workflow name (string) - **`cache:`** - Cache configuration for workflow dependencies (object or array) @@ -444,7 +445,8 @@ Agentic workflows compile to GitHub Actions YAML: 5. **Test with `gh aw compile`** before committing 6. **Review generated `.lock.yml`** files before deploying 7. **Set `stop-time`** for cost-sensitive workflows -8. **Use specific tool permissions** rather than broad access +8. **Set `max-turns`** to limit chat iterations and prevent runaway loops +9. **Use specific tool permissions** rather than broad access ## Validation diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5c74697bd92..809fa8d61e0 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -371,6 +371,10 @@ ] } }, + "max-turns": { + "type": "integer", + "description": "Maximum number of chat iterations per run" + }, "stop-time": { "type": "string", "description": "Time when workflow should stop running. Supports multiple formats: absolute dates (YYYY-MM-DD HH:MM:SS, June 1 2025, 1st June 2025, 06/01/2025, etc.) or relative time deltas (+25h, +3d, +1d12h30m)" diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 47463576436..dda1e4f8614 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -29,6 +29,9 @@ type AgenticEngine interface { // SupportsHTTPTransport returns true if this engine supports HTTP transport for MCP servers SupportsHTTPTransport() bool + // SupportsMaxTurns returns true if this engine supports the max-turns feature + SupportsMaxTurns() bool + // GetInstallationSteps returns the GitHub Actions steps needed to install this engine GetInstallationSteps(engineConfig *EngineConfig) []GitHubActionStep @@ -68,6 +71,7 @@ type BaseEngine struct { experimental bool supportsToolsWhitelist bool supportsHTTPTransport bool + supportsMaxTurns bool } func (e *BaseEngine) GetID() string { @@ -94,6 +98,10 @@ func (e *BaseEngine) SupportsHTTPTransport() bool { return e.supportsHTTPTransport } +func (e *BaseEngine) SupportsMaxTurns() bool { + return e.supportsMaxTurns +} + // EngineRegistry manages available agentic engines type EngineRegistry struct { engines map[string]AgenticEngine diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 8997ea3bfc5..77092a0db3c 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -25,6 +25,7 @@ func NewClaudeEngine() *ClaudeEngine { experimental: false, supportsToolsWhitelist: true, supportsHTTPTransport: true, // Claude supports both stdio and HTTP transport + supportsMaxTurns: true, // Claude supports max-turns feature }, } } @@ -51,6 +52,7 @@ func (e *ClaudeEngine) GetExecutionConfig(workflowName string, logFile string, e "claude_env": "|\n GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}", "allowed_tools": "", // Will be filled in during generation "timeout_minutes": "", // Will be filled in during generation + "max_turns": "", // Will be filled in during generation }, Environment: map[string]string{ "GH_TOKEN": "${{ secrets.GITHUB_TOKEN }}", diff --git a/pkg/workflow/claude_engine_test.go b/pkg/workflow/claude_engine_test.go index 261af2458f8..ad4ced8b138 100644 --- a/pkg/workflow/claude_engine_test.go +++ b/pkg/workflow/claude_engine_test.go @@ -76,6 +76,10 @@ func TestClaudeEngine(t *testing.T) { t.Error("Expected timeout_minutes input to be present") } + if _, hasMaxTurns := config.Inputs["max_turns"]; !hasMaxTurns { + t.Error("Expected max_turns input to be present") + } + // Check environment variables if config.Environment["GH_TOKEN"] != "${{ secrets.GITHUB_TOKEN }}" { t.Errorf("Expected GH_TOKEN environment variable, got '%s'", config.Environment["GH_TOKEN"]) @@ -109,7 +113,7 @@ func TestClaudeEngineConfiguration(t *testing.T) { } // Verify all required inputs are present - requiredInputs := []string{"prompt_file", "anthropic_api_key", "mcp_config", "claude_env", "allowed_tools", "timeout_minutes"} + requiredInputs := []string{"prompt_file", "anthropic_api_key", "mcp_config", "claude_env", "allowed_tools", "timeout_minutes", "max_turns"} for _, input := range requiredInputs { if _, exists := config.Inputs[input]; !exists { t.Errorf("Expected input '%s' to be present", input) diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index bf2095bcde0..0d79d26979b 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -20,6 +20,7 @@ func NewCodexEngine() *CodexEngine { experimental: true, supportsToolsWhitelist: true, supportsHTTPTransport: false, // Codex only supports stdio transport + supportsMaxTurns: false, // Codex does not support max-turns feature }, } } diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index aa1f60695d5..9a6e484ada2 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -196,6 +196,7 @@ type WorkflowData struct { AllowedTools string AI string // "claude" or "codex" (for backwards compatibility) EngineConfig *EngineConfig // Extended engine configuration + MaxTurns string StopTime string Alias string // for @alias trigger support AliasOtherEvents map[string]any // for merging alias with other events @@ -593,6 +594,11 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) } } + // Validate max-turns support for the current engine + if err := c.validateMaxTurnsSupport(result.Frontmatter, agenticEngine); err != nil { + return nil, fmt.Errorf("max-turns not supported: %w", err) + } + // Process @include directives in markdown content markdownContent, err := parser.ExpandIncludes(result.Markdown, markdownDir, false) if err != nil { @@ -639,6 +645,7 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps") workflowData.RunsOn = c.extractTopLevelYAMLSection(result.Frontmatter, "runs-on") workflowData.Cache = c.extractTopLevelYAMLSection(result.Frontmatter, "cache") + workflowData.MaxTurns = c.extractYAMLValue(result.Frontmatter, "max-turns") workflowData.StopTime = c.extractYAMLValue(result.Frontmatter, "stop-time") // Resolve relative stop-time to absolute time if needed @@ -2094,6 +2101,10 @@ func (c *Compiler) generateEngineExecutionSteps(yaml *strings.Builder, data *Wor if data.TimeoutMinutes != "" { yaml.WriteString(" " + data.TimeoutMinutes + "\n") } + } else if key == "max_turns" { + if data.MaxTurns != "" { + yaml.WriteString(fmt.Sprintf(" max_turns: %s\n", data.MaxTurns)) + } } else if value != "" { yaml.WriteString(fmt.Sprintf(" %s: %s\n", key, value)) } @@ -2193,3 +2204,23 @@ func (c *Compiler) validateHTTPTransportSupport(tools map[string]any, engine Age return nil } + +// validateMaxTurnsSupport validates that max-turns is only used with engines that support this feature +func (c *Compiler) validateMaxTurnsSupport(frontmatter map[string]any, engine AgenticEngine) error { + // Check if max-turns is specified in the frontmatter + _, hasMaxTurns := frontmatter["max-turns"] + if !hasMaxTurns { + // No max-turns specified, no validation needed + return nil + } + + // max-turns is specified, check if the engine supports it + if !engine.SupportsMaxTurns() { + return fmt.Errorf("max-turns not supported: engine '%s' does not support the max-turns feature", engine.GetID()) + } + + // Engine supports max-turns - additional validation could be added here if needed + // For now, we rely on JSON schema validation for format checking + + return nil +} diff --git a/pkg/workflow/max_turns_test.go b/pkg/workflow/max_turns_test.go new file mode 100644 index 00000000000..a85f7dedeb7 --- /dev/null +++ b/pkg/workflow/max_turns_test.go @@ -0,0 +1,229 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestMaxTurnsCompilation(t *testing.T) { + tests := []struct { + name string + content string + expectedMaxTurns string + shouldInclude bool + }{ + { + name: "workflow with max-turns", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: 3 +tools: + github: + allowed: [get_issue] +--- + +# Test Max Turns + +This workflow tests the max-turns feature.`, + expectedMaxTurns: "max_turns: 3", + shouldInclude: true, + }, + { + name: "workflow without max-turns", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +tools: + github: + allowed: [get_issue] +--- + +# Test Without Max Turns + +This workflow should not include max-turns.`, + expectedMaxTurns: "", + shouldInclude: false, + }, + { + name: "workflow with max-turns and timeout", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: 10 +timeout_minutes: 15 +tools: + github: + allowed: [get_issue] +--- + +# Test Max Turns and Timeout + +This workflow tests max-turns with timeout.`, + expectedMaxTurns: "max_turns: 10", + shouldInclude: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory for the test + tmpDir, err := os.MkdirTemp("", "max-turns-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create the test workflow file + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.content), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "") + if err := compiler.CompileWorkflow(testFile); 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) + } + + lockContentStr := string(lockContent) + + if tt.shouldInclude { + // Verify max_turns is included in the generated workflow + if !strings.Contains(lockContentStr, tt.expectedMaxTurns) { + t.Errorf("Expected max_turns to be included in generated workflow. Expected: %s\nActual content:\n%s", tt.expectedMaxTurns, lockContentStr) + } + + // Verify it's in the correct context (under the Claude action inputs) + if !strings.Contains(lockContentStr, "anthropics/claude-code-base-action") { + t.Error("Expected to find Claude action in generated workflow") + } + + // Look for max_turns in the inputs section + lines := strings.Split(lockContentStr, "\n") + foundAction := false + foundMaxTurns := false + for i, line := range lines { + if strings.Contains(line, "anthropics/claude-code-base-action") { + foundAction = true + } + if foundAction && strings.Contains(line, "with:") { + // Look for max_turns in the subsequent lines + for j := i + 1; j < len(lines) && strings.HasPrefix(lines[j], " "); j++ { + if strings.Contains(lines[j], "max_turns:") { + foundMaxTurns = true + break + } + } + break + } + } + + if !foundMaxTurns { + t.Error("Expected to find max_turns in the action inputs section") + } + } else { + // Verify max_turns is NOT included when not specified + if strings.Contains(lockContentStr, "max_turns:") { + t.Error("Expected max_turns NOT to be included when not specified in frontmatter") + } + } + }) + } +} + +func TestMaxTurnsValidation(t *testing.T) { + tests := []struct { + name string + content string + expectError bool + }{ + { + name: "valid integer max-turns", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: 5 +--- + +# Valid Max Turns`, + expectError: false, + }, + { + name: "invalid string max-turns", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: "invalid" +--- + +# Invalid Max Turns`, + expectError: true, + }, + { + name: "zero max-turns", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: 0 +--- + +# Zero Max Turns`, + expectError: false, // Zero should be valid (might mean unlimited) + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory for the test + tmpDir, err := os.MkdirTemp("", "max-turns-validation-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create the test workflow file + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.content), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "") + err = compiler.CompileWorkflow(testFile) + + if tt.expectError && err == nil { + t.Error("Expected compilation to fail but it succeeded") + } else if !tt.expectError && err != nil { + t.Errorf("Expected compilation to succeed but it failed: %v", err) + } + }) + } +} diff --git a/pkg/workflow/max_turns_validation_test.go b/pkg/workflow/max_turns_validation_test.go new file mode 100644 index 00000000000..16fcdf75d95 --- /dev/null +++ b/pkg/workflow/max_turns_validation_test.go @@ -0,0 +1,164 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestMaxTurnsValidationWithUnsupportedEngine(t *testing.T) { + tests := []struct { + name string + content string + engine string + expectError bool + errorMsg string + }{ + { + name: "max-turns with codex engine should fail", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: codex +max-turns: 5 +--- + +# Test Workflow + +This should fail because codex doesn't support max-turns.`, + engine: "codex", + expectError: true, + errorMsg: "max-turns not supported: engine 'codex' does not support the max-turns feature", + }, + { + name: "max-turns with claude engine should succeed", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +max-turns: 5 +--- + +# Test Workflow + +This should succeed because claude supports max-turns.`, + engine: "claude", + expectError: false, + }, + { + name: "codex engine without max-turns should succeed", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: codex +--- + +# Test Workflow + +This should succeed because no max-turns is specified.`, + engine: "codex", + expectError: false, + }, + { + name: "claude engine without max-turns should succeed", + content: `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: claude +--- + +# Test Workflow + +This should succeed because no max-turns is specified.`, + engine: "claude", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory for the test + tmpDir, err := os.MkdirTemp("", "max-turns-validation-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create a test workflow file + testFile := filepath.Join(tmpDir, "test.md") + err = os.WriteFile(testFile, []byte(tt.content), 0644) + if err != nil { + t.Fatal(err) + } + + // Create a compiler instance + compiler := NewCompiler(false, "", "test") + compiler.SetSkipValidation(false) + + // Try to compile the workflow + err = compiler.CompileWorkflow(testFile) + + if tt.expectError { + // We expect an error + if err == nil { + t.Errorf("Expected error but compilation succeeded") + return + } + + // Check if the error message contains the expected text + if !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("Expected error message to contain '%s', but got: %s", tt.errorMsg, err.Error()) + } + } else { + // We don't expect an error + if err != nil { + t.Errorf("Expected compilation to succeed but got error: %v", err) + } + } + }) + } +} + +func TestEngineSupportsMaxTurns(t *testing.T) { + tests := []struct { + name string + engineID string + expectedSupport bool + }{ + { + name: "claude engine supports max-turns", + engineID: "claude", + expectedSupport: true, + }, + { + name: "codex engine does not support max-turns", + engineID: "codex", + expectedSupport: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + registry := GetGlobalEngineRegistry() + engine, err := registry.GetEngine(tt.engineID) + if err != nil { + t.Fatalf("Failed to get engine '%s': %v", tt.engineID, err) + } + + actualSupport := engine.SupportsMaxTurns() + if actualSupport != tt.expectedSupport { + t.Errorf("Expected engine '%s' to have SupportsMaxTurns() = %v, but got %v", + tt.engineID, tt.expectedSupport, actualSupport) + } + }) + } +}