diff --git a/.changeset/fix-github-tool-config-codemods.md b/.changeset/fix-github-tool-config-codemods.md new file mode 100644 index 00000000000..779c12673a3 --- /dev/null +++ b/.changeset/fix-github-tool-config-codemods.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add `gh aw fix` codemods for the `tools.github.toolset` typo and legacy `tools.github.allowed-repos: current` alias. diff --git a/docs/adr/50841-add-codemods-for-toolset-typo-and-allowed-repos-current.md b/docs/adr/50841-add-codemods-for-toolset-typo-and-allowed-repos-current.md new file mode 100644 index 00000000000..f2cb252627d --- /dev/null +++ b/docs/adr/50841-add-codemods-for-toolset-typo-and-allowed-repos-current.md @@ -0,0 +1,48 @@ +# ADR-50841: Add Fix Codemods for `toolset` Typo and `allowed-repos: current` Legacy Alias + +**Date**: 2026-08-06 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +The daily cross-repo compatibility audit revealed two distinct, unambiguous migration gaps in `gh aw fix --write`: the `tools.github.toolset:` (singular) typo — for which the compiler already emits "Did you mean 'toolsets'?" — and the `tools.github.allowed-repos: current` legacy alias, which strict-mode rejects because only `all`, `public`, or `${{ github.repository }}` are accepted values. Both issues caused compilation failures in external repos (`githubnext/gh-aw-trial-oxpecker-test`, `pelikhan/github-agentic-workflows`) that `fix --write` could not automatically resolve. The existing codemod framework (`GetAllCodemods()`, `applyFrontmatterLineTransform`) provides a well-established pattern for exactly these line-level YAML rewrites, and both transformations are deterministic with no ambiguity about the desired output. + +### Decision + +We will implement two new codemods — `toolset-singular-to-toolsets` and `allowed-repos-current-to-github-repository` — and register them in `GetAllCodemods()` immediately after the existing `github-repos-to-allowed-repos` codemod. Both codemods use the existing `applyFrontmatterLineTransform` hook with indentation-aware, context-scoped line parsers that skip comments, preserve trailing comments, and are guarded by a frontmatter pre-check to remain no-ops on already-migrated files. The `allowed-repos` codemod adds `findTrailingCommentIndex` to correctly split YAML-comment boundaries from values (a `#` only starts a comment when preceded by whitespace). + +### Alternatives Considered + +#### Alternative 1: Accept Legacy Forms in the Compiler + +Relax strict-mode validation to silently accept `toolset:` (singular) and `allowed-repos: current` as valid input, interpreting them as their canonical equivalents at parse time. This removes the need for migration codemods entirely. + +Not chosen because it undermines the strict-mode design goal: strict-mode exists specifically to reject ambiguous or deprecated configurations. Silently accepting `current` would mean workflows that rely on the deprecated alias never get upgraded to the more explicit `${{ github.repository }}` form, and the compiler's "Did you mean 'toolsets'?" guidance loses its purpose. + +#### Alternative 2: Require Manual Fixes, Improve Error Messages Only + +Keep `fix --write` unchanged; instead, improve the compiler error messages to be copy-paste-ready (e.g., display the exact replacement YAML inline). Users manually apply the one-line change. + +Not chosen because the codemod framework already exists for exactly this purpose, and "Did you mean 'toolsets'?" is already in place. Adding automation is consistent with the project's existing pattern and directly reduces friction for the 20+ repos affected across future audit runs. + +### Consequences + +#### Positive +- `gh aw fix --write` now closes two additional identified failure clusters from the daily cross-repo audit, reducing manual intervention needed by external repo maintainers. +- Both codemods are idempotent and scoped strictly to `tools.github` block lines, preventing false-positive rewrites in other YAML blocks. +- `findTrailingCommentIndex` correctly handles `#` characters embedded in quoted YAML values (e.g., `"#current"` alias), improving robustness of the comment-preservation logic. + +#### Negative +- Two more entries in `GetAllCodemods()` increase the ordered list that tests must enumerate; any future reordering requires updating `fix_codemods_test.go` in two places. +- The line-based YAML parsing approach (rather than a full AST parser) is a simplification that can fail on unusual but valid YAML: multi-document files, block scalars, or flow-style mappings that span multiple lines. These edge cases are untested. + +#### Neutral +- Both codemods are registered with `IntroducedIn: "0.85.5"`, matching the patch release expected to ship the new migrations. +- The `allowed-repos` codemod normalizes output to double-quoted `"${{ github.repository }}"` regardless of the input quoting style (`current`, `"current"`, `'current'`), which is a minor stylistic imposition but aligns with the canonical form used throughout the codebase. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/codemod_allowed_repos_current.go b/pkg/cli/codemod_allowed_repos_current.go new file mode 100644 index 00000000000..d90f00121f3 --- /dev/null +++ b/pkg/cli/codemod_allowed_repos_current.go @@ -0,0 +1,180 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var allowedReposCurrentCodemodLog = logger.New("cli:codemod_allowed_repos_current") + +// getAllowedReposCurrentToGitHubRepositoryCodemod creates a codemod that migrates the +// unsupported legacy 'current' alias for tools.github.allowed-repos to the equivalent +// '${{ github.repository }}' expression, which is the only accepted way to scope +// access to the current repository. +func getAllowedReposCurrentToGitHubRepositoryCodemod() Codemod { + return Codemod{ + ID: "allowed-repos-current-to-github-repository", + Name: "Migrate 'tools.github.allowed-repos: current' to '${{ github.repository }}'", + Description: "Rewrites the legacy 'current' alias for tools.github.allowed-repos to the accepted '${{ github.repository }}' expression.", + IntroducedIn: "0.85.5", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if !hasAllowedReposCurrentValue(frontmatter) { + return content, false, nil + } + newContent, applied, err := applyFrontmatterLineTransform(content, rewriteAllowedReposCurrentValue) + if applied { + allowedReposCurrentCodemodLog.Print("Migrated 'tools.github.allowed-repos: current' to '${{ github.repository }}'") + } + return newContent, applied, err + }, + } +} + +// hasAllowedReposCurrentValue returns true if tools.github.allowed-repos is set to the +// legacy string value 'current'. +func hasAllowedReposCurrentValue(frontmatter map[string]any) bool { + toolsAny, hasTools := frontmatter["tools"] + if !hasTools { + return false + } + toolsMap, ok := toolsAny.(map[string]any) + if !ok { + return false + } + githubAny, hasGitHub := toolsMap["github"] + if !hasGitHub { + return false + } + githubMap, ok := githubAny.(map[string]any) + if !ok { + return false + } + allowedReposAny, hasAllowedRepos := githubMap["allowed-repos"] + if !hasAllowedRepos { + return false + } + allowedReposStr, ok := allowedReposAny.(string) + if !ok { + return false + } + return allowedReposStr == "current" +} + +// rewriteAllowedReposCurrentValue rewrites the value of 'allowed-repos: current' (or any +// quoted variant) to '${{ github.repository }}' within the tools.github configuration block. +func rewriteAllowedReposCurrentValue(lines []string) ([]string, bool) { + var result []string + modified := false + + var inTools, inToolsGithub bool + var toolsIndent, toolsChildIndent, toolsGithubIndent string + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + + // Skip empty lines without resetting state + if trimmed == "" { + result = append(result, line) + continue + } + + // Exit blocks when indentation signals we've left them + if !strings.HasPrefix(trimmed, "#") { + if inToolsGithub && hasExitedBlock(line, toolsGithubIndent) { + inToolsGithub = false + } + if inTools && hasExitedBlock(line, toolsIndent) { + inTools = false + inToolsGithub = false + } + } + + // Detect top-level 'tools:' block + if strings.HasPrefix(trimmed, "tools:") && getIndentation(line) == "" { + inTools = true + inToolsGithub = false + toolsIndent = getIndentation(line) + toolsChildIndent = "" + result = append(result, line) + continue + } + + lineIndent := getIndentation(line) + if inTools && toolsChildIndent == "" && isDescendant(lineIndent, toolsIndent) { + toolsChildIndent = lineIndent + } + + // Detect direct 'github:' block inside 'tools:' + if inTools && strings.HasPrefix(trimmed, "github:") && lineIndent == toolsChildIndent { + inToolsGithub = true + toolsGithubIndent = lineIndent + result = append(result, line) + continue + } + + // Rewrite the value of 'allowed-repos: current' when inside tools.github + if inToolsGithub && strings.HasPrefix(trimmed, "allowed-repos:") { + if isDescendant(lineIndent, toolsGithubIndent) { + if newLine, replaced := replaceAllowedReposCurrentLineValue(line); replaced { + result = append(result, newLine) + modified = true + allowedReposCurrentCodemodLog.Printf("Migrated 'allowed-repos: current' on line %d", i+1) + continue + } + } + } + + result = append(result, line) + } + + return result, modified +} + +// replaceAllowedReposCurrentLineValue replaces the value of an 'allowed-repos:' line with +// '${{ github.repository }}' if the current value (unquoted, single-quoted, or double-quoted) +// is 'current'. Preserves indentation and trailing comments. +func replaceAllowedReposCurrentLineValue(line string) (string, bool) { + parts := strings.SplitN(line, ":", 2) + if len(parts) < 2 { + return line, false + } + + leadingSpace := getIndentation(line) + valuePart := strings.TrimSpace(parts[1]) + + // Split off any trailing comment. Per YAML rules, a '#' only starts a comment + // when preceded by whitespace (or at the start of the value), so a '#' embedded + // inside a quoted value (e.g. "cur#rent") is not mistaken for a comment. + value := valuePart + comment := "" + if idx := findTrailingCommentIndex(valuePart); idx >= 0 { + value = strings.TrimSpace(valuePart[:idx]) + comment = " " + valuePart[idx:] + } + + unquoted := strings.Trim(value, `"'`) + if unquoted != "current" { + return line, false + } + + allowedReposCurrentCodemodLog.Print("Replacing 'current' value with '${{ github.repository }}'") + return fmt.Sprintf(`%sallowed-repos: "${{ github.repository }}"%s`, leadingSpace, comment), true +} + +// findTrailingCommentIndex returns the index of the '#' that starts a trailing YAML +// comment in value, or -1 if there is none. A '#' only starts a comment when it is +// at the beginning of the (trimmed) value or immediately preceded by whitespace, +// which prevents a '#' embedded inside a quoted string from being misinterpreted. +func findTrailingCommentIndex(value string) int { + for i, r := range value { + if r != '#' { + continue + } + if i == 0 || value[i-1] == ' ' || value[i-1] == '\t' { + return i + } + } + return -1 +} diff --git a/pkg/cli/codemod_allowed_repos_current_test.go b/pkg/cli/codemod_allowed_repos_current_test.go new file mode 100644 index 00000000000..89d44958c3b --- /dev/null +++ b/pkg/cli/codemod_allowed_repos_current_test.go @@ -0,0 +1,297 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAllowedReposCurrentToGitHubRepositoryCodemod(t *testing.T) { + codemod := getAllowedReposCurrentToGitHubRepositoryCodemod() + + t.Run("metadata is populated", func(t *testing.T) { + assert.Equal(t, "allowed-repos-current-to-github-repository", codemod.ID) + assert.NotEmpty(t, codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.85.5", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) + }) + + t.Run("rewrites unquoted current value", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + toolsets: [default] + allowed-repos: current +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "toolsets": []any{"default"}, + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, `allowed-repos: "${{ github.repository }}"`, "Should rewrite current to the github.repository expression") + assert.NotContains(t, result, "allowed-repos: current", "Should not contain the old current value") + }) + + t.Run("rewrites quoted current value", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: "current" +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, `allowed-repos: "${{ github.repository }}"`, "Should rewrite current to the github.repository expression") + }) + + t.Run("rewrites single-quoted current value", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: 'current' +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, `allowed-repos: "${{ github.repository }}"`, "Should rewrite single-quoted current to the github.repository expression") + }) + + t.Run("no-op when allowed-repos is already an expression", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: "${{ github.repository }}" +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "${{ github.repository }}", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not apply when already migrated") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("no-op when allowed-repos is an array", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: + - "myorg/*" +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": []any{"myorg/*"}, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not apply when allowed-repos is an array") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("no-op when allowed-repos is set to all", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: "all" +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "all", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not apply when allowed-repos is 'all'") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("preserves trailing comments", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: current # legacy alias +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, `allowed-repos: "${{ github.repository }}" # legacy alias`, "Should preserve trailing comment") + }) + + t.Run("only treats whitespace-preceded hash as a comment marker", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + allowed-repos: current # see docs on "#current" alias +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, `allowed-repos: "${{ github.repository }}" # see docs on "#current" alias`, "Should preserve the full comment including embedded hash") + }) + + t.Run("does not rewrite nested non-top-level tools github allowed-repos", func(t *testing.T) { + content := `--- +engine: copilot +wrapper: + tools: + github: + allowed-repos: current +tools: + github: + allowed-repos: current +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "wrapper": map[string]any{ + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + }, + "tools": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should rewrite direct top-level tools.github allowed-repos") + assert.Contains(t, result, " allowed-repos: current", "Should preserve nested non-top-level tools.github value") + assert.Contains(t, result, ` allowed-repos: "${{ github.repository }}"`, "Should rewrite top-level tools.github value") + }) + + t.Run("does not rewrite nested custom github allowed-repos", func(t *testing.T) { + content := `--- +engine: copilot +tools: + custom: + github: + allowed-repos: current + github: + allowed-repos: current +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "custom": map[string]any{ + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + "github": map[string]any{ + "allowed-repos": "current", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should rewrite direct tools.github allowed-repos") + assert.Contains(t, result, " allowed-repos: current", "Should preserve nested custom github allowed-repos") + assert.Contains(t, result, ` allowed-repos: "${{ github.repository }}"`, "Should rewrite direct tools.github value") + }) +} diff --git a/pkg/cli/codemod_toolset_singular.go b/pkg/cli/codemod_toolset_singular.go new file mode 100644 index 00000000000..7b71b48a236 --- /dev/null +++ b/pkg/cli/codemod_toolset_singular.go @@ -0,0 +1,129 @@ +package cli + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var toolsetSingularCodemodLog = logger.New("cli:codemod_toolset_singular") + +// getToolsetSingularToToolsetsCodemod creates a codemod that renames the mistyped +// singular 'toolset:' field to the correct plural 'toolsets:' field within the +// tools.github configuration block. +func getToolsetSingularToToolsetsCodemod() Codemod { + return Codemod{ + ID: "toolset-singular-to-toolsets", + Name: "Rename 'tools.github.toolset' to 'tools.github.toolsets'", + Description: "Renames the mistyped singular 'toolset:' field to the correct plural 'toolsets:' inside the tools.github configuration block.", + IntroducedIn: "0.85.5", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if !hasSingularToolsetField(frontmatter) { + return content, false, nil + } + newContent, applied, err := applyFrontmatterLineTransform(content, renameToolsetSingularToToolsets) + if applied { + toolsetSingularCodemodLog.Print("Renamed 'tools.github.toolset' to 'tools.github.toolsets'") + } + return newContent, applied, err + }, + } +} + +// hasSingularToolsetField returns true if tools.github has a mistyped singular +// 'toolset' field and does not already have a 'toolsets' field. +func hasSingularToolsetField(frontmatter map[string]any) bool { + toolsAny, hasTools := frontmatter["tools"] + if !hasTools { + return false + } + toolsMap, ok := toolsAny.(map[string]any) + if !ok { + return false + } + githubAny, hasGitHub := toolsMap["github"] + if !hasGitHub { + return false + } + githubMap, ok := githubAny.(map[string]any) + if !ok { + return false + } + _, hasToolset := githubMap["toolset"] + _, hasToolsets := githubMap["toolsets"] // only check existence, not value + if hasToolset && !hasToolsets { + toolsetSingularCodemodLog.Print("Mistyped singular 'toolset' field found in tools.github") + } + return hasToolset && !hasToolsets +} + +// renameToolsetSingularToToolsets renames 'toolset:' to 'toolsets:' within the +// tools.github configuration block. +func renameToolsetSingularToToolsets(lines []string) ([]string, bool) { + var result []string + modified := false + + var inTools, inToolsGithub bool + var toolsIndent, toolsChildIndent, toolsGithubIndent string + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + + // Skip empty lines without resetting state + if trimmed == "" { + result = append(result, line) + continue + } + + // Exit blocks when indentation signals we've left them + if !strings.HasPrefix(trimmed, "#") { + if inToolsGithub && hasExitedBlock(line, toolsGithubIndent) { + inToolsGithub = false + } + if inTools && hasExitedBlock(line, toolsIndent) { + inTools = false + inToolsGithub = false + } + } + + // Detect top-level 'tools:' block + if strings.HasPrefix(trimmed, "tools:") && getIndentation(line) == "" { + inTools = true + inToolsGithub = false + toolsIndent = getIndentation(line) + toolsChildIndent = "" + result = append(result, line) + continue + } + + lineIndent := getIndentation(line) + if inTools && toolsChildIndent == "" && isDescendant(lineIndent, toolsIndent) { + toolsChildIndent = lineIndent + } + + // Detect direct 'github:' block inside 'tools:' + if inTools && strings.HasPrefix(trimmed, "github:") && lineIndent == toolsChildIndent { + inToolsGithub = true + toolsGithubIndent = lineIndent + result = append(result, line) + continue + } + + // Rename 'toolset:' to 'toolsets:' when inside tools.github + if inToolsGithub && strings.HasPrefix(trimmed, "toolset:") { + if isDescendant(lineIndent, toolsGithubIndent) { + newLine, replaced := findAndReplaceInLine(line, "toolset", "toolsets") + if replaced { + result = append(result, newLine) + modified = true + toolsetSingularCodemodLog.Printf("Renamed 'toolset' to 'toolsets' on line %d", i+1) + continue + } + } + } + + result = append(result, line) + } + + return result, modified +} diff --git a/pkg/cli/codemod_toolset_singular_test.go b/pkg/cli/codemod_toolset_singular_test.go new file mode 100644 index 00000000000..5308079c232 --- /dev/null +++ b/pkg/cli/codemod_toolset_singular_test.go @@ -0,0 +1,177 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestToolsetSingularToToolsetsCodemod(t *testing.T) { + codemod := getToolsetSingularToToolsetsCodemod() + + t.Run("metadata is populated", func(t *testing.T) { + assert.Equal(t, "toolset-singular-to-toolsets", codemod.ID) + assert.NotEmpty(t, codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.85.5", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) + }) + + t.Run("renames toolset to toolsets under tools.github", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + mode: remote + toolset: [default] + allowed-repos: "all" +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "mode": "remote", + "toolset": []any{"default"}, + "allowed-repos": "all", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, "toolsets: [default]", "Should rename toolset to toolsets") + assert.NotContains(t, result, "\n toolset: ", "Should not contain old toolset: field") + }) + + t.Run("no-op when toolsets already present", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + toolsets: [default] +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "toolsets": []any{"default"}, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not apply when already migrated") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("no-op when tools.github is absent", func(t *testing.T) { + content := `--- +engine: copilot +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not apply when tools.github is absent") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("does not rename toolset in comments", func(t *testing.T) { + content := `--- +engine: copilot +tools: + github: + # toolset: legacy comment + toolset: default +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "github": map[string]any{ + "toolset": "default", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should have applied the codemod") + assert.Contains(t, result, "toolsets: default", "Should rename toolset key") + assert.Contains(t, result, "# toolset: legacy comment", "Should not rename toolset in comments") + }) + + t.Run("no-op when toolset appears outside tools.github", func(t *testing.T) { + content := `--- +engine: copilot +other: + toolset: some-value +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "other": map[string]any{ + "toolset": "some-value", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.False(t, applied, "Should not rename toolset outside tools.github") + assert.Equal(t, content, result, "Content should remain unchanged") + }) + + t.Run("does not rename nested custom github toolset", func(t *testing.T) { + content := `--- +engine: copilot +tools: + custom: + github: + toolset: custom-value + github: + toolset: default +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "custom": map[string]any{ + "github": map[string]any{ + "toolset": "custom-value", + }, + }, + "github": map[string]any{ + "toolset": "default", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Should not error") + assert.True(t, applied, "Should rename tools.github.toolset") + assert.Contains(t, result, " toolset: custom-value", "Should preserve nested custom github toolset") + assert.Contains(t, result, " toolsets: default", "Should rename direct tools.github toolset") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index 9661e73a951..4d2bd8ac5f9 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -94,6 +94,8 @@ func GetAllCodemods() []Codemod { getPullRequestTargetCheckoutFalseCodemod(), // Add checkout: false for pull_request_target workflows when safe getDependabotPermissionsCodemod(), // Add vulnerability-alerts: read when dependabot toolset is used getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos + getToolsetSingularToToolsetsCodemod(), // Rename mistyped tools.github.toolset to tools.github.toolsets + getAllowedReposCurrentToGitHubRepositoryCodemod(), // Migrate legacy tools.github.allowed-repos: current to ${{ github.repository }} getCopilotRequestsFeatureToPermissionsCodemod(), // Migrate features.copilot-requests to permissions.copilot-requests getByokCopilotFeatureRemovalCodemod(), // Remove deprecated features.byok-copilot (Copilot BYOK is default) getInlineAgentsFeatureRemovalCodemod(), // Remove deprecated features.inline-agents (inline sub-agents now default) diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 43b05ae61a4..3c338f45b21 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -115,6 +115,8 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "pull-request-target-checkout-false", "dependabot-toolset-permissions", "github-repos-to-allowed-repos", + "toolset-singular-to-toolsets", + "allowed-repos-current-to-github-repository", "features-copilot-requests-to-permissions", "features-byok-copilot-removal", "features-inline-agents-removal", @@ -233,6 +235,8 @@ func expectedCodemodOrder() []string { "pull-request-target-checkout-false", "dependabot-toolset-permissions", "github-repos-to-allowed-repos", + "toolset-singular-to-toolsets", + "allowed-repos-current-to-github-repository", "features-copilot-requests-to-permissions", "features-byok-copilot-removal", "features-inline-agents-removal",