diff --git a/actions/setup/js/types/safe-outputs-config.d.ts b/actions/setup/js/types/safe-outputs-config.d.ts index f22c4e6d252..623a7ad5aff 100644 --- a/actions/setup/js/types/safe-outputs-config.d.ts +++ b/actions/setup/js/types/safe-outputs-config.d.ts @@ -218,9 +218,16 @@ interface AssignMilestoneConfig extends SafeOutputConfig { * Configuration for assigning agents to issues */ interface AssignToAgentConfig extends SafeOutputConfig { - "default-agent"?: string; + name?: string; + model?: string; + "custom-agent"?: string; + "custom-instructions"?: string; + allowed?: string[]; target?: string; "target-repo"?: string; + "pull-request-repo"?: string; + "allowed-pull-request-repos"?: string[]; + "base-branch"?: string; "ignore-if-error"?: boolean; } diff --git a/pkg/cli/codemod_assign_to_agent.go b/pkg/cli/codemod_assign_to_agent.go new file mode 100644 index 00000000000..0d5b9c03772 --- /dev/null +++ b/pkg/cli/codemod_assign_to_agent.go @@ -0,0 +1,126 @@ +package cli + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var assignToAgentCodemodLog = logger.New("cli:codemod_assign_to_agent") + +// getAssignToAgentDefaultAgentCodemod creates a codemod for migrating the deprecated 'default-agent' key +// to the canonical 'name' key inside safe-outputs.assign-to-agent +func getAssignToAgentDefaultAgentCodemod() Codemod { + return Codemod{ + ID: "assign-to-agent-default-agent-to-name", + Name: "Migrate assign-to-agent default-agent to name", + Description: "Renames the deprecated 'default-agent' field to 'name' inside 'safe-outputs.assign-to-agent'", + IntroducedIn: "0.12.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + // Check if safe-outputs.assign-to-agent.default-agent exists + safeOutputsValue, hasSafeOutputs := frontmatter["safe-outputs"] + if !hasSafeOutputs { + return content, false, nil + } + + safeOutputsMap, ok := safeOutputsValue.(map[string]any) + if !ok { + return content, false, nil + } + + assignToAgentValue, hasAssignToAgent := safeOutputsMap["assign-to-agent"] + if !hasAssignToAgent { + return content, false, nil + } + + assignToAgentMap, ok := assignToAgentValue.(map[string]any) + if !ok { + return content, false, nil + } + + // Check if deprecated 'default-agent' key exists + _, hasDefaultAgent := assignToAgentMap["default-agent"] + if !hasDefaultAgent { + return content, false, nil + } + + // Don't migrate if 'name' already exists to avoid overwriting it + _, hasName := assignToAgentMap["name"] + if hasName { + assignToAgentCodemodLog.Print("Skipping migration: 'name' already exists in assign-to-agent config") + return content, false, nil + } + + // Parse frontmatter to get raw lines + frontmatterLines, markdown, err := parseFrontmatterLines(content) + if err != nil { + return content, false, err + } + + var modified bool + var inSafeOutputsBlock bool + var safeOutputsIndent string + var inAssignToAgentBlock bool + var assignToAgentIndent string + + result := make([]string, len(frontmatterLines)) + + for i, line := range frontmatterLines { + trimmedLine := strings.TrimSpace(line) + + // Track if we're in the safe-outputs block + if strings.HasPrefix(trimmedLine, "safe-outputs:") { + inSafeOutputsBlock = true + safeOutputsIndent = getIndentation(line) + result[i] = line + continue + } + + // Check if we've left the safe-outputs block + if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if hasExitedBlock(line, safeOutputsIndent) { + inSafeOutputsBlock = false + inAssignToAgentBlock = false + } + } + + // Track if we're in the assign-to-agent block within safe-outputs + if inSafeOutputsBlock && strings.HasPrefix(trimmedLine, "assign-to-agent:") { + inAssignToAgentBlock = true + assignToAgentIndent = getIndentation(line) + result[i] = line + continue + } + + // Check if we've left the assign-to-agent block + if inAssignToAgentBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if hasExitedBlock(line, assignToAgentIndent) { + inAssignToAgentBlock = false + } + } + + // Replace default-agent with name if in assign-to-agent block + if inAssignToAgentBlock && strings.HasPrefix(trimmedLine, "default-agent:") { + replacedLine, didReplace := findAndReplaceInLine(line, "default-agent", "name") + if didReplace { + result[i] = replacedLine + modified = true + assignToAgentCodemodLog.Printf("Replaced safe-outputs.assign-to-agent.default-agent with safe-outputs.assign-to-agent.name on line %d", i+1) + } else { + result[i] = line + } + } else { + result[i] = line + } + } + + if !modified { + return content, false, nil + } + + newContent := reconstructContent(result, markdown) + assignToAgentCodemodLog.Print("Applied assign-to-agent default-agent to name migration") + return newContent, true, nil + }, + } +} diff --git a/pkg/cli/codemod_assign_to_agent_test.go b/pkg/cli/codemod_assign_to_agent_test.go new file mode 100644 index 00000000000..40534f605fa --- /dev/null +++ b/pkg/cli/codemod_assign_to_agent_test.go @@ -0,0 +1,260 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetAssignToAgentDefaultAgentCodemod(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + assert.Equal(t, "assign-to-agent-default-agent-to-name", codemod.ID) + assert.Equal(t, "Migrate assign-to-agent default-agent to name", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.NotEmpty(t, codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestAssignToAgentCodemod_BasicMigration(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +engine: copilot +safe-outputs: + assign-to-agent: + default-agent: copilot +--- + +# Test Workflow` + + frontmatter := map[string]any{ + "on": "issues", + "engine": "copilot", + "safe-outputs": map[string]any{ + "assign-to-agent": map[string]any{ + "default-agent": "copilot", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "name: copilot") + assert.NotContains(t, result, "default-agent:") +} + +func TestAssignToAgentCodemod_PreservesIndentation(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + assign-to-agent: + default-agent: copilot + max: 2 +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "assign-to-agent": map[string]any{ + "default-agent": "copilot", + "max": 2, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, " name: copilot") + assert.Contains(t, result, " max: 2") + assert.NotContains(t, result, "default-agent:") +} + +func TestAssignToAgentCodemod_PreservesComment(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + assign-to-agent: + default-agent: copilot # the default agent +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "assign-to-agent": map[string]any{ + "default-agent": "copilot", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "name: copilot # the default agent") +} + +func TestAssignToAgentCodemod_NoSafeOutputs(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +engine: copilot +--- + +# Test` + + frontmatter := map[string]any{ + "on": "issues", + "engine": "copilot", + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestAssignToAgentCodemod_NoAssignToAgent(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + create-issue: + title: Bug +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "title": "Bug", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestAssignToAgentCodemod_NoDefaultAgent(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + assign-to-agent: + name: copilot +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "assign-to-agent": map[string]any{ + "name": "copilot", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestAssignToAgentCodemod_SkipsWhenNameAlreadyExists(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + assign-to-agent: + name: copilot + default-agent: other-agent +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "assign-to-agent": map[string]any{ + "name": "copilot", + "default-agent": "other-agent", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied, "Should not apply when 'name' already exists") + assert.Equal(t, content, result) +} + +func TestAssignToAgentCodemod_PreservesOtherSafeOutputs(t *testing.T) { + codemod := getAssignToAgentDefaultAgentCodemod() + + content := `--- +on: issues +safe-outputs: + create-issue: + title: Bug + assign-to-agent: + default-agent: copilot +--- + +# Test` + + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "title": "Bug", + }, + "assign-to-agent": map[string]any{ + "default-agent": "copilot", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "create-issue:") + assert.Contains(t, result, "name: copilot") + assert.NotContains(t, result, "default-agent:") +} + +func TestAssignToAgentCodemod_RegisteredInAllCodemods(t *testing.T) { + codemods := GetAllCodemods() + var found bool + for _, c := range codemods { + if c.ID == "assign-to-agent-default-agent-to-name" { + found = true + break + } + } + assert.True(t, found, "assign-to-agent-default-agent-to-name codemod should be registered in GetAllCodemods") +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b10612b05c5..f2c8bd3b0d3 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -34,10 +34,11 @@ func GetAllCodemods() []Codemod { getDiscussionFlagRemovalCodemod(), getMCPModeToTypeCodemod(), getInstallScriptURLCodemod(), - getBashAnonymousRemovalCodemod(), // Replace bash: with bash: false - getActivationOutputsCodemod(), // Transform needs.activation.outputs.* to steps.sanitized.outputs.* - getRolesToOnRolesCodemod(), // Move top-level roles to on.roles - getBotsToOnBotsCodemod(), // Move top-level bots to on.bots - getEngineStepsToTopLevelCodemod(), // Move engine.steps to top-level steps + getBashAnonymousRemovalCodemod(), // Replace bash: with bash: false + getActivationOutputsCodemod(), // Transform needs.activation.outputs.* to steps.sanitized.outputs.* + getRolesToOnRolesCodemod(), // Move top-level roles to on.roles + getBotsToOnBotsCodemod(), // Move top-level bots to on.bots + getEngineStepsToTopLevelCodemod(), // Move engine.steps to top-level steps + getAssignToAgentDefaultAgentCodemod(), // Rename deprecated default-agent to name in assign-to-agent } } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index aa9e2560621..391b6d55fd2 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) { codemods := GetAllCodemods() // Verify we have the expected number of codemods - expectedCount := 21 + expectedCount := 22 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -125,6 +125,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "roles-to-on-roles", "bots-to-on-bots", "engine-steps-to-top-level", + "assign-to-agent-default-agent-to-name", } require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods") diff --git a/pkg/workflow/assign_to_agent_test.go b/pkg/workflow/assign_to_agent_test.go new file mode 100644 index 00000000000..ddfcb59bc51 --- /dev/null +++ b/pkg/workflow/assign_to_agent_test.go @@ -0,0 +1,44 @@ +//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" +) + +// TestAssignToAgentCanonicalNameKey tests that 'name' is the canonical key for assigning an agent +func TestAssignToAgentCanonicalNameKey(t *testing.T) { + tmpDir := testutil.TempDir(t, "assign-to-agent-name-test") + + workflow := `--- +on: issues +engine: copilot +permissions: + contents: read +safe-outputs: + assign-to-agent: + name: copilot +--- + +# Test Workflow + +This workflow tests canonical 'name' key. +` + 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") + + compiler := NewCompilerWithVersion("1.0.0") + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.AssignToAgent, "AssignToAgent should not be nil") + assert.Equal(t, "copilot", workflowData.SafeOutputs.AssignToAgent.DefaultAgent, "Should parse 'name' key as DefaultAgent") +}