From 07f38efd98578d3efdbbf1a3829ba4a54b3a2b34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 17:23:25 +0000 Subject: [PATCH 1/2] feat: add serena tools-to-import codemod Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4b18acd2-1622-4e97-8f1b-4960fd6ce6a9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_serena_import.go | 269 ++++++++++++++++++++++++++ pkg/cli/codemod_serena_import_test.go | 150 ++++++++++++++ pkg/cli/fix_codemods.go | 1 + pkg/cli/fix_codemods_test.go | 4 +- 4 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 pkg/cli/codemod_serena_import.go create mode 100644 pkg/cli/codemod_serena_import_test.go diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go new file mode 100644 index 00000000000..d100295694c --- /dev/null +++ b/pkg/cli/codemod_serena_import.go @@ -0,0 +1,269 @@ +package cli + +import ( + "fmt" + "slices" + "sort" + "strings" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/sliceutil" +) + +var serenaImportCodemodLog = logger.New("cli:codemod_serena_import") + +// getSerenaToSharedImportCodemod creates a codemod that migrates removed tools.serena +// configuration to an equivalent imports entry using shared/mcp/serena.md. +func getSerenaToSharedImportCodemod() Codemod { + return Codemod{ + ID: "serena-tools-to-shared-import", + Name: "Migrate tools.serena to shared Serena import", + Description: "Removes 'tools.serena' and adds an equivalent 'imports' entry using shared/mcp/serena.md with languages.", + IntroducedIn: "1.0.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + toolsAny, hasTools := frontmatter["tools"] + if !hasTools { + return content, false, nil + } + + toolsMap, ok := toolsAny.(map[string]any) + if !ok { + return content, false, nil + } + + serenaAny, hasSerena := toolsMap["serena"] + if !hasSerena { + return content, false, nil + } + + languages, ok := extractSerenaLanguages(serenaAny) + if !ok || len(languages) == 0 { + serenaImportCodemodLog.Print("Found tools.serena but could not extract languages - skipping migration") + return content, false, nil + } + + alreadyImported := hasSerenaSharedImport(frontmatter) + + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + result, modified := removeFieldFromBlock(lines, "serena", "tools") + if !modified { + return lines, false + } + + result = removeTopLevelBlockIfEmpty(result, "tools") + + if alreadyImported { + return result, true + } + + return addSerenaImport(result, languages), true + }) + if applied { + if alreadyImported { + serenaImportCodemodLog.Print("Removed tools.serena (shared/mcp/serena.md import already present)") + } else { + serenaImportCodemodLog.Printf("Migrated tools.serena to shared/mcp/serena.md import with %d language(s)", len(languages)) + } + } + return newContent, applied, err + }, + } +} + +func extractSerenaLanguages(serenaAny any) ([]string, bool) { + switch serena := serenaAny.(type) { + case []string: + return sliceutil.Deduplicate(serena), len(serena) > 0 + case []any: + var languages []string + for _, item := range serena { + lang, ok := item.(string) + if ok && strings.TrimSpace(lang) != "" { + languages = append(languages, lang) + } + } + return sliceutil.Deduplicate(languages), len(languages) > 0 + case string: + trimmed := strings.TrimSpace(serena) + if trimmed == "" { + return nil, false + } + return []string{trimmed}, true + case map[string]any: + languagesAny, hasLanguages := serena["languages"] + if !hasLanguages { + return nil, false + } + return extractSerenaLanguagesFromLanguagesField(languagesAny) + default: + return nil, false + } +} + +func extractSerenaLanguagesFromLanguagesField(languagesAny any) ([]string, bool) { + switch languages := languagesAny.(type) { + case []string: + return sliceutil.Deduplicate(languages), len(languages) > 0 + case []any: + var result []string + for _, item := range languages { + lang, ok := item.(string) + if ok && strings.TrimSpace(lang) != "" { + result = append(result, lang) + } + } + return sliceutil.Deduplicate(result), len(result) > 0 + case string: + trimmed := strings.TrimSpace(languages) + if trimmed == "" { + return nil, false + } + return []string{trimmed}, true + case map[string]any: + var result []string + for language := range languages { + if strings.TrimSpace(language) != "" { + result = append(result, language) + } + } + sort.Strings(result) + return sliceutil.Deduplicate(result), len(result) > 0 + default: + return nil, false + } +} + +func hasSerenaSharedImport(frontmatter map[string]any) bool { + importsAny, hasImports := frontmatter["imports"] + if !hasImports { + return false + } + + switch imports := importsAny.(type) { + case []string: + return slices.ContainsFunc(imports, isSerenaImportPath) + case []any: + for _, entry := range imports { + switch typed := entry.(type) { + case string: + if isSerenaImportPath(typed) { + return true + } + case map[string]any: + usesAny, hasUses := typed["uses"] + if !hasUses { + continue + } + uses, ok := usesAny.(string) + if ok && isSerenaImportPath(uses) { + return true + } + } + } + } + + return false +} + +func isSerenaImportPath(path string) bool { + trimmed := strings.TrimSpace(path) + return trimmed == "shared/mcp/serena.md" || trimmed == "shared/mcp/serena" +} + +func addSerenaImport(lines []string, languages []string) []string { + entry := []string{ + " - uses: shared/mcp/serena.md", + " with:", + " languages: " + formatStringArrayInline(languages), + } + + importsIdx := -1 + importsEnd := len(lines) + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, "imports:") { + importsIdx = i + for j := i + 1; j < len(lines); j++ { + if isTopLevelKey(lines[j]) { + importsEnd = j + break + } + } + break + } + } + + if importsIdx >= 0 { + result := make([]string, 0, len(lines)+len(entry)) + result = append(result, lines[:importsEnd]...) + result = append(result, entry...) + result = append(result, lines[importsEnd:]...) + return result + } + + insertAt := 0 + for i, line := range lines { + if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), "engine:") { + insertAt = i + 1 + break + } + } + + importBlock := make([]string, 0, 1+len(entry)) + importBlock = append(importBlock, "imports:") + importBlock = append(importBlock, entry...) + + result := make([]string, 0, len(lines)+len(importBlock)) + result = append(result, lines[:insertAt]...) + result = append(result, importBlock...) + result = append(result, lines[insertAt:]...) + return result +} + +func formatStringArrayInline(values []string) string { + quoted := make([]string, 0, len(values)) + for _, value := range values { + quoted = append(quoted, fmt.Sprintf("%q", value)) + } + return "[" + strings.Join(quoted, ", ") + "]" +} + +func removeTopLevelBlockIfEmpty(lines []string, blockName string) []string { + blockIdx := -1 + blockEnd := len(lines) + for i, line := range lines { + if isTopLevelKey(line) && strings.HasPrefix(strings.TrimSpace(line), blockName+":") { + blockIdx = i + for j := i + 1; j < len(lines); j++ { + if isTopLevelKey(lines[j]) { + blockEnd = j + break + } + } + break + } + } + + if blockIdx == -1 { + return lines + } + + hasMeaningfulNestedContent := false + for _, line := range lines[blockIdx+1 : blockEnd] { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + hasMeaningfulNestedContent = true + break + } + + if hasMeaningfulNestedContent { + return lines + } + + result := make([]string, 0, len(lines)-(blockEnd-blockIdx)) + result = append(result, lines[:blockIdx]...) + result = append(result, lines[blockEnd:]...) + return result +} diff --git a/pkg/cli/codemod_serena_import_test.go b/pkg/cli/codemod_serena_import_test.go new file mode 100644 index 00000000000..1bab907672d --- /dev/null +++ b/pkg/cli/codemod_serena_import_test.go @@ -0,0 +1,150 @@ +//go:build !integration + +package cli + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSerenaToSharedImportCodemod(t *testing.T) { + codemod := getSerenaToSharedImportCodemod() + + t.Run("migrates tools.serena short syntax to imports", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: ["go", "typescript"] +strict: false +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": []any{"go", "typescript"}, + }, + "strict": false, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not return an error") + assert.True(t, applied, "Codemod should be applied when tools.serena is present") + assert.NotContains(t, result, "tools:\n", "Codemod should remove empty tools block") + assert.NotContains(t, result, "serena:", "Codemod should remove tools.serena configuration") + assert.Contains(t, result, "imports:", "Codemod should add imports block") + assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Codemod should add Serena shared import") + assert.Contains(t, result, "languages: [\"go\", \"typescript\"]", "Codemod should preserve short syntax languages") + }) + + t.Run("migrates tools.serena long syntax languages object to imports", func(t *testing.T) { + content := `--- +engine: copilot +tools: + serena: + languages: + go: + version: "1.21" + typescript: +strict: false +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "tools": map[string]any{ + "serena": map[string]any{ + "languages": map[string]any{ + "go": map[string]any{ + "version": "1.21", + }, + "typescript": nil, + }, + }, + }, + "strict": false, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not return an error") + assert.True(t, applied, "Codemod should be applied for long syntax tools.serena") + assert.NotContains(t, result, "serena:", "Codemod should remove tools.serena block") + assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Codemod should add Serena shared import") + assert.Contains(t, result, "languages: [\"go\", \"typescript\"]", "Codemod should convert language object keys to array") + }) + + t.Run("removes tools.serena when shared import already exists without adding duplicate", func(t *testing.T) { + content := `--- +engine: copilot +imports: + - uses: shared/mcp/serena.md + with: + languages: ["go", "typescript"] +tools: + serena: ["go", "typescript"] + github: + toolsets: [default] +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "imports": []any{ + map[string]any{ + "uses": "shared/mcp/serena.md", + "with": map[string]any{ + "languages": []any{"go", "typescript"}, + }, + }, + }, + "tools": map[string]any{ + "serena": []any{"go", "typescript"}, + "github": map[string]any{ + "toolsets": []any{"default"}, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not return an error") + assert.True(t, applied, "Codemod should be applied when tools.serena is present") + assert.NotContains(t, result, "\n serena:", "Codemod should remove tools.serena field") + assert.Contains(t, result, "github:", "Codemod should preserve other tools.* entries") + assert.Equal(t, 1, strings.Count(result, "shared/mcp/serena.md"), "Codemod should not add a duplicate Serena import") + }) + + t.Run("does not modify workflows without tools.serena", func(t *testing.T) { + content := `--- +engine: copilot +imports: + - uses: shared/mcp/serena.md + with: + languages: ["go"] +--- + +# Test Workflow +` + frontmatter := map[string]any{ + "engine": "copilot", + "imports": []any{ + map[string]any{ + "uses": "shared/mcp/serena.md", + "with": map[string]any{ + "languages": []any{"go"}, + }, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not return an error") + assert.False(t, applied, "Codemod should not be applied when tools.serena is absent") + assert.Equal(t, content, result, "Content should remain unchanged when no migration is needed") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index 61bc8034f38..c7a64fb3472 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -52,6 +52,7 @@ func GetAllCodemods() []Codemod { getGitHubAppClientIDCodemod(), // Rename deprecated github-app.app-id to github-app.client-id getSafeInputsToMCPScriptsCodemod(), // Rename safe-inputs to mcp-scripts getPluginsToDependenciesCodemod(), // Migrate plugins to dependencies (plugins removed in favour of APM) + getSerenaToSharedImportCodemod(), // Migrate removed tools.serena to shared/mcp/serena.md import getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos getDIFCProxyToIntegrityProxyCodemod(), // Migrate deprecated features.difc-proxy to tools.github.integrity-proxy } diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 71923c4289d..6bf98eab310 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 := 32 + expectedCount := 33 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -83,6 +83,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "safe-inputs-to-mcp-scripts", "steps-run-secrets-to-env", "engine-env-secrets-to-engine-config", + "serena-tools-to-shared-import", } for _, expectedID := range expectedIDs { @@ -137,6 +138,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "github-app-app-id-to-client-id", "safe-inputs-to-mcp-scripts", "plugins-to-dependencies", + "serena-tools-to-shared-import", "github-repos-to-allowed-repos", "features-difc-proxy-to-tools-github", } From 4f6b8ab0e4f4bc94c920706c9a2bec713071d242 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 17:28:57 +0000 Subject: [PATCH 2/2] test: harden serena codemod assertions Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4b18acd2-1622-4e97-8f1b-4960fd6ce6a9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_serena_import.go | 2 +- pkg/cli/codemod_serena_import_test.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go index d100295694c..5e6409ee5f4 100644 --- a/pkg/cli/codemod_serena_import.go +++ b/pkg/cli/codemod_serena_import.go @@ -38,7 +38,7 @@ func getSerenaToSharedImportCodemod() Codemod { languages, ok := extractSerenaLanguages(serenaAny) if !ok || len(languages) == 0 { - serenaImportCodemodLog.Print("Found tools.serena but could not extract languages - skipping migration") + serenaImportCodemodLog.Print("Found tools.serena but languages configuration is invalid or empty - skipping migration; verify tools.serena languages are set") return content, false, nil } diff --git a/pkg/cli/codemod_serena_import_test.go b/pkg/cli/codemod_serena_import_test.go index 1bab907672d..927dae1f546 100644 --- a/pkg/cli/codemod_serena_import_test.go +++ b/pkg/cli/codemod_serena_import_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -34,11 +35,15 @@ strict: false result, applied, err := codemod.Apply(content, frontmatter) require.NoError(t, err, "Codemod should not return an error") assert.True(t, applied, "Codemod should be applied when tools.serena is present") - assert.NotContains(t, result, "tools:\n", "Codemod should remove empty tools block") assert.NotContains(t, result, "serena:", "Codemod should remove tools.serena configuration") assert.Contains(t, result, "imports:", "Codemod should add imports block") assert.Contains(t, result, "- uses: shared/mcp/serena.md", "Codemod should add Serena shared import") assert.Contains(t, result, "languages: [\"go\", \"typescript\"]", "Codemod should preserve short syntax languages") + + parsed, parseErr := parser.ExtractFrontmatterFromContent(result) + require.NoError(t, parseErr, "Result should contain valid frontmatter") + _, hasTools := parsed.Frontmatter["tools"] + assert.False(t, hasTools, "Codemod should remove empty tools key from frontmatter") }) t.Run("migrates tools.serena long syntax languages object to imports", func(t *testing.T) {