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
7 changes: 6 additions & 1 deletion pkg/workflow/assign_to_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToA
return &AssignToAgentConfig{}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling path returns an empty AssignToAgentConfig without setting Max to the default value of 1. While the config generation layer will apply the default later, it would be more consistent with other parsers like assign-to-user (line 31-33 in assign_to_user.go) to set the default in the error case as well. Consider: return &AssignToAgentConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 1}}

See below for a potential fix:

		// Handle null case: create config with default max
		return &AssignToAgentConfig{
			BaseSafeOutputConfig: BaseSafeOutputConfig{
				Max: 1,
			},
		}

Copilot uses AI. Check for mistakes.
}

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
}
1 change: 1 addition & 0 deletions pkg/workflow/safe_outputs_config_generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 9 additions & 4 deletions pkg/workflow/safe_outputs_config_generation_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
208 changes: 208 additions & 0 deletions pkg/workflow/safe_outputs_default_max_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
Loading