diff --git a/pkg/workflow/assign_to_agent.go b/pkg/workflow/assign_to_agent.go index ee04c85f946..f23e682ab85 100644 --- a/pkg/workflow/assign_to_agent.go +++ b/pkg/workflow/assign_to_agent.go @@ -32,7 +32,12 @@ func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToA return &AssignToAgentConfig{} } - assignToAgentLog.Printf("Parsed assign-to-agent config: default_agent=%s, allowed_count=%d, target=%s", config.DefaultAgent, len(config.Allowed), config.Target) + // Set default max if not specified + if config.Max == 0 { + config.Max = 1 + } + + assignToAgentLog.Printf("Parsed assign-to-agent config: default_agent=%s, allowed_count=%d, target=%s, max=%d", config.DefaultAgent, len(config.Allowed), config.Target, config.Max) return &config } diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 2d4a306c5d6..061b75d10ab 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -182,6 +182,7 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if data.SafeOutputs.AssignToAgent != nil { safeOutputsConfig["assign_to_agent"] = generateAssignToAgentConfig( data.SafeOutputs.AssignToAgent.Max, + 1, // default max data.SafeOutputs.AssignToAgent.DefaultAgent, data.SafeOutputs.AssignToAgent.Target, data.SafeOutputs.AssignToAgent.Allowed, diff --git a/pkg/workflow/safe_outputs_config_generation_helpers.go b/pkg/workflow/safe_outputs_config_generation_helpers.go index 1e1da7dbb97..9cc9b53943d 100644 --- a/pkg/workflow/safe_outputs_config_generation_helpers.go +++ b/pkg/workflow/safe_outputs_config_generation_helpers.go @@ -97,15 +97,20 @@ func generateMaxWithReviewersConfig(max int, defaultMax int, reviewers []string) } // generateAssignToAgentConfig creates a config with optional max, default_agent, target, and allowed -func generateAssignToAgentConfig(max int, defaultAgent string, target string, allowed []string) map[string]any { +func generateAssignToAgentConfig(max int, defaultMax int, defaultAgent string, target string, allowed []string) map[string]any { if safeOutputsConfigGenLog.Enabled() { - safeOutputsConfigGenLog.Printf("Generating assign-to-agent config: max=%d, defaultAgent=%s, target=%s, allowed_count=%d", - max, defaultAgent, target, len(allowed)) + safeOutputsConfigGenLog.Printf("Generating assign-to-agent config: max=%d, defaultMax=%d, defaultAgent=%s, target=%s, allowed_count=%d", + max, defaultMax, defaultAgent, target, len(allowed)) } config := make(map[string]any) + + // Apply default max if max is not specified + maxValue := defaultMax if max > 0 { - config["max"] = max + maxValue = max } + config["max"] = maxValue + if defaultAgent != "" { config["default_agent"] = defaultAgent } diff --git a/pkg/workflow/safe_outputs_default_max_test.go b/pkg/workflow/safe_outputs_default_max_test.go new file mode 100644 index 00000000000..307fe669433 --- /dev/null +++ b/pkg/workflow/safe_outputs_default_max_test.go @@ -0,0 +1,208 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestAssignToAgentDefaultMax tests that assign-to-agent has a default max of 1 +func TestAssignToAgentDefaultMax(t *testing.T) { + tmpDir := testutil.TempDir(t, "assign-to-agent-default-max-test") + + // Create a workflow with assign-to-agent but no explicit max + workflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + assign-to-agent: + name: copilot +--- + +# Test Workflow + +This workflow tests the default max for assign-to-agent. +` + testFile := filepath.Join(tmpDir, "test-assign-to-agent.md") + err := os.WriteFile(testFile, []byte(workflow), 0644) + require.NoError(t, err, "Failed to write test workflow") + + // Parse the workflow + compiler := NewCompilerWithVersion("1.0.0") + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + + // Verify assign-to-agent config exists and has default max of 1 + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.AssignToAgent, "AssignToAgent should not be nil") + assert.Equal(t, 1, workflowData.SafeOutputs.AssignToAgent.Max, "Default max should be 1") +} + +// TestDispatchWorkflowDefaultMax tests that dispatch-workflow has a default max of 1 +func TestDispatchWorkflowDefaultMax(t *testing.T) { + tmpDir := testutil.TempDir(t, "dispatch-workflow-default-max-test") + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + // Create a target workflow with workflow_dispatch + targetWorkflow := `name: Target +on: + workflow_dispatch: +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "Target workflow" +` + targetFile := filepath.Join(workflowsDir, "target.lock.yml") + err = os.WriteFile(targetFile, []byte(targetWorkflow), 0644) + require.NoError(t, err, "Failed to write target workflow") + + // Create a dispatcher workflow with dispatch-workflow but no explicit max + workflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + dispatch-workflow: + - target +--- + +# Test Workflow + +This workflow tests the default max for dispatch-workflow. +` + testFile := filepath.Join(tmpDir, "test-dispatch.md") + err = os.WriteFile(testFile, []byte(workflow), 0644) + require.NoError(t, err, "Failed to write test workflow") + + // Parse the workflow + compiler := NewCompilerWithVersion("1.0.0") + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + + // Verify dispatch-workflow config exists and has default max of 1 + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.DispatchWorkflow, "DispatchWorkflow should not be nil") + assert.Equal(t, 1, workflowData.SafeOutputs.DispatchWorkflow.Max, "Default max should be 1") +} + +// TestAssignToAgentExplicitMax tests that explicit max overrides the default +func TestAssignToAgentExplicitMax(t *testing.T) { + tmpDir := testutil.TempDir(t, "assign-to-agent-explicit-max-test") + + // Create a workflow with assign-to-agent with explicit max + workflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + assign-to-agent: + name: copilot + max: 5 +--- + +# Test Workflow + +This workflow tests explicit max for assign-to-agent. +` + testFile := filepath.Join(tmpDir, "test-assign-to-agent.md") + err := os.WriteFile(testFile, []byte(workflow), 0644) + require.NoError(t, err, "Failed to write test workflow") + + // Parse the workflow + compiler := NewCompilerWithVersion("1.0.0") + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + + // Verify assign-to-agent config has explicit max of 5 + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.AssignToAgent, "AssignToAgent should not be nil") + assert.Equal(t, 5, workflowData.SafeOutputs.AssignToAgent.Max, "Explicit max should be 5") +} + +// TestDispatchWorkflowExplicitMax tests that explicit max overrides the default +func TestDispatchWorkflowExplicitMax(t *testing.T) { + tmpDir := testutil.TempDir(t, "dispatch-workflow-explicit-max-test") + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Failed to create workflows directory") + + // Create a target workflow with workflow_dispatch + targetWorkflow := `name: Target +on: + workflow_dispatch: +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "Target workflow" +` + targetFile := filepath.Join(workflowsDir, "target.lock.yml") + err = os.WriteFile(targetFile, []byte(targetWorkflow), 0644) + require.NoError(t, err, "Failed to write target workflow") + + // Create a dispatcher workflow with explicit max + workflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + dispatch-workflow: + workflows: + - target + max: 3 +--- + +# Test Workflow + +This workflow tests explicit max for dispatch-workflow. +` + testFile := filepath.Join(tmpDir, "test-dispatch.md") + err = os.WriteFile(testFile, []byte(workflow), 0644) + require.NoError(t, err, "Failed to write test workflow") + + // Parse the workflow + compiler := NewCompilerWithVersion("1.0.0") + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + + // Verify dispatch-workflow config has explicit max of 3 + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.DispatchWorkflow, "DispatchWorkflow should not be nil") + assert.Equal(t, 3, workflowData.SafeOutputs.DispatchWorkflow.Max, "Explicit max should be 3") +} + +// TestGenerateAssignToAgentConfigDefaultMax tests the config generation with default max +func TestGenerateAssignToAgentConfigDefaultMax(t *testing.T) { + // Test with max=0 (should use default of 1) + config := generateAssignToAgentConfig(0, 1, "copilot", "", nil) + assert.Equal(t, 1, config["max"], "Should use default max of 1 when max is 0") + assert.Equal(t, "copilot", config["default_agent"], "Should have default agent") + + // Test with explicit max (should override default) + config = generateAssignToAgentConfig(5, 1, "copilot", "", nil) + assert.Equal(t, 5, config["max"], "Should use explicit max of 5") + assert.Equal(t, "copilot", config["default_agent"], "Should have default agent") + + // Test with target and allowed + config = generateAssignToAgentConfig(0, 1, "copilot", "issues", []string{"copilot", "custom"}) + assert.Equal(t, 1, config["max"], "Should use default max of 1") + assert.Equal(t, "copilot", config["default_agent"], "Should have default agent") + assert.Equal(t, "issues", config["target"], "Should have target") + assert.Equal(t, []string{"copilot", "custom"}, config["allowed"], "Should have allowed list") +}