diff --git a/pkg/cli/codemod_expires_integer.go b/pkg/cli/codemod_expires_integer.go new file mode 100644 index 00000000000..8d4cd7751e0 --- /dev/null +++ b/pkg/cli/codemod_expires_integer.go @@ -0,0 +1,145 @@ +package cli + +import ( + "fmt" + "regexp" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var expiresIntegerCodemodLog = logger.New("cli:codemod_expires_integer") + +// expiresIntegerValuePattern matches an expires value that is a pure integer (possibly with a trailing comment) +var expiresIntegerValuePattern = regexp.MustCompile(`^(\s*)(\d+)(\s*)(#.*)?$`) + +// getExpiresIntegerToStringCodemod creates a codemod for converting integer expires values to day strings. +// Converts e.g. "expires: 7" to "expires: 7d" in all safe-outputs types. +func getExpiresIntegerToStringCodemod() Codemod { + return Codemod{ + ID: "expires-integer-to-string", + Name: "Convert expires integer to day string", + Description: "Converts integer 'expires' values (e.g., 'expires: 7') to day string format (e.g., 'expires: 7d') in safe-outputs types", + IntroducedIn: "0.13.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + // Check if safe-outputs exists + safeOutputsValue, hasSafeOutputs := frontmatter["safe-outputs"] + if !hasSafeOutputs { + return content, false, nil + } + + safeOutputsMap, ok := safeOutputsValue.(map[string]any) + if !ok { + return content, false, nil + } + + // Check if any safe-outputs type has an integer expires value + hasIntegerExpires := false + for _, outputTypeValue := range safeOutputsMap { + outputTypeMap, ok := outputTypeValue.(map[string]any) + if !ok { + continue + } + if expiresValue, hasExpires := outputTypeMap["expires"]; hasExpires { + switch expiresValue.(type) { + case int, int64, uint64: + hasIntegerExpires = true + } + } + } + + if !hasIntegerExpires { + return content, false, nil + } + + // Parse frontmatter to get raw lines + frontmatterLines, markdown, err := parseFrontmatterLines(content) + if err != nil { + return content, false, err + } + + // Convert integer expires values to day strings within safe-outputs blocks + result, modified := convertExpiresIntegersToDayStrings(frontmatterLines) + if !modified { + return content, false, nil + } + + // Reconstruct the content + newContent := reconstructContent(result, markdown) + expiresIntegerCodemodLog.Print("Applied expires integer-to-string migration") + return newContent, true, nil + }, + } +} + +// convertExpiresIntegersToDayStrings converts integer expires values to day strings within safe-outputs blocks. +// Only affects expires lines nested inside a safe-outputs block. +func convertExpiresIntegersToDayStrings(lines []string) ([]string, bool) { + var result []string + var modified bool + var inSafeOutputsBlock bool + var safeOutputsIndent string + + for i, line := range lines { + 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 = append(result, 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 + } + } + + // Convert integer expires to day string if inside safe-outputs block + if inSafeOutputsBlock && strings.HasPrefix(trimmedLine, "expires:") { + newLine, converted := convertExpiresLineToString(line) + if converted { + result = append(result, newLine) + modified = true + expiresIntegerCodemodLog.Printf("Converted integer expires to day string on line %d", i+1) + continue + } + } + + result = append(result, line) + } + + return result, modified +} + +// convertExpiresLineToString converts an expires line with an integer value to use a day string. +// For example: " expires: 7" -> " expires: 7d" +// Lines that already use a string format (e.g., "expires: 7d", "expires: 24h") are left unchanged. +// Returns the (possibly converted) line and whether a conversion was made. +func convertExpiresLineToString(line string) (string, bool) { + indent := getIndentation(line) + trimmedLine := strings.TrimSpace(line) + + // Extract the value part after "expires:" + valuePart := strings.TrimPrefix(trimmedLine, "expires:") + + // Match an integer value optionally followed by whitespace and a comment + matches := expiresIntegerValuePattern.FindStringSubmatch(valuePart) + if matches == nil { + // Not an integer value (already a string like "7d" or "false") + return line, false + } + + intValue := matches[2] // the digits + trailingWS := matches[3] // whitespace between value and comment + comment := matches[4] // optional trailing comment + + // Build the new line, preserving any trailing comment + if comment != "" { + return fmt.Sprintf("%sexpires: %sd%s%s", indent, intValue, trailingWS, comment), true + } + return fmt.Sprintf("%sexpires: %sd", indent, intValue), true +} diff --git a/pkg/cli/codemod_expires_integer_test.go b/pkg/cli/codemod_expires_integer_test.go new file mode 100644 index 00000000000..494784d0cf2 --- /dev/null +++ b/pkg/cli/codemod_expires_integer_test.go @@ -0,0 +1,342 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetExpiresIntegerToStringCodemod(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + assert.Equal(t, "expires-integer-to-string", codemod.ID) + assert.Equal(t, "Convert expires integer to day string", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.13.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestExpiresIntegerCodemod_ConvertsCreateIssue(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + expires: 7 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "expires": 7, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 7d") +} + +func TestExpiresIntegerCodemod_ConvertsCreateDiscussion(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-discussion: + expires: 30 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-discussion": map[string]any{ + "expires": 30, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 30d") +} + +func TestExpiresIntegerCodemod_ConvertsCreatePullRequest(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-pull-request: + expires: 14 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-pull-request": map[string]any{ + "expires": 14, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 14d") +} + +func TestExpiresIntegerCodemod_AlreadyStringFormat_NoChange(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + expires: 7d +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "expires": "7d", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestExpiresIntegerCodemod_HourStringFormat_NoChange(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + expires: 24h +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "expires": "24h", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestExpiresIntegerCodemod_NoSafeOutputs_NoChange(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestExpiresIntegerCodemod_PreservesComment(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + expires: 7 # expire after one week +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "expires": 7, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 7d # expire after one week") +} + +func TestExpiresIntegerCodemod_PreservesOtherFields(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + max: 5 + expires: 14 + labels: + - bug +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "max": 5, + "expires": 14, + "labels": []any{"bug"}, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 14d") + assert.Contains(t, result, "max: 5") + assert.Contains(t, result, "- bug") +} + +func TestExpiresIntegerCodemod_MultipleOutputTypes(t *testing.T) { + codemod := getExpiresIntegerToStringCodemod() + + content := `--- +on: workflow_dispatch +safe-outputs: + create-issue: + expires: 7 + create-discussion: + expires: 30 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "safe-outputs": map[string]any{ + "create-issue": map[string]any{ + "expires": 7, + }, + "create-discussion": map[string]any{ + "expires": 30, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "expires: 7d") + assert.Contains(t, result, "expires: 30d") +} + +func TestConvertExpiresLineToString_Integer(t *testing.T) { + tests := []struct { + name string + input string + expected string + changed bool + }{ + { + name: "simple integer", + input: " expires: 7", + expected: " expires: 7d", + changed: true, + }, + { + name: "integer with trailing comment", + input: " expires: 7 # expire after one week", + expected: " expires: 7d # expire after one week", + changed: true, + }, + { + name: "already day string", + input: " expires: 7d", + expected: " expires: 7d", + changed: false, + }, + { + name: "hour string", + input: " expires: 24h", + expected: " expires: 24h", + changed: false, + }, + { + name: "week string", + input: " expires: 2w", + expected: " expires: 2w", + changed: false, + }, + { + name: "false value", + input: " expires: false", + expected: " expires: false", + changed: false, + }, + { + name: "no indentation", + input: "expires: 1", + expected: "expires: 1d", + changed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, changed := convertExpiresLineToString(tt.input) + assert.Equal(t, tt.changed, changed, "changed flag should match") + assert.Equal(t, tt.expected, result, "converted line should match") + }) + } +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index be588a925ea..2975951e63c 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -41,5 +41,6 @@ func GetAllCodemods() []Codemod { getEngineStepsToTopLevelCodemod(), // Move engine.steps to top-level steps getAssignToAgentDefaultAgentCodemod(), // Rename deprecated default-agent to name in assign-to-agent getPlaywrightDomainsCodemod(), // Migrate tools.playwright.allowed_domains to network.allowed + getExpiresIntegerToStringCodemod(), // Convert expires integer (days) to string with 'd' suffix } } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 53311461993..213f0e02436 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 := 23 + expectedCount := 24 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -127,6 +127,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "engine-steps-to-top-level", "assign-to-agent-default-agent-to-name", "playwright-allowed-domains-migration", + "expires-integer-to-string", } require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods")