diff --git a/pkg/cli/codemod_tools_unknown_to_mcp_servers.go b/pkg/cli/codemod_tools_unknown_to_mcp_servers.go new file mode 100644 index 00000000000..3ffc9b217bb --- /dev/null +++ b/pkg/cli/codemod_tools_unknown_to_mcp_servers.go @@ -0,0 +1,302 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var unknownToolsToMCPServersCodemodLog = logger.New("cli:codemod_tools_unknown_to_mcp_servers") + +var knownBuiltInToolsForCodemod = map[string]bool{ + "github": true, + "playwright": true, + "agentic-workflows": true, + "cache-memory": true, + "repo-memory": true, + "bash": true, + "edit": true, + "web-fetch": true, + "web-search": true, + "safety-prompt": true, + "timeout": true, + "startup-timeout": true, + "mount-as-clis": true, +} + +type toolsBlockEntry struct { + key string + start int + end int + indent string + lines []string +} + +// getToolsUnknownToMCPServersCodemod migrates unknown tools entries to mcp-servers. +func getToolsUnknownToMCPServersCodemod() Codemod { + return Codemod{ + ID: "tools-unknown-to-mcp-servers", + Name: "Migrate unknown tools entries to mcp-servers", + Description: "Moves unknown entries from 'tools' to 'mcp-servers'. Adds a command placeholder and wraps list values under 'tools'.", + IntroducedIn: "0.9.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + toolsValue, hasTools := frontmatter["tools"] + if !hasTools { + return content, false, nil + } + + toolsMap, ok := toolsValue.(map[string]any) + if !ok || len(toolsMap) == 0 { + return content, false, nil + } + + return applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + toolsIndex, toolsEndIndex, entries, found := findTopLevelToolsEntries(lines) + if !found || len(entries) == 0 { + return lines, false + } + + unknownEntries := make([]toolsBlockEntry, 0) + unknownByStart := make(map[int]toolsBlockEntry) + for _, entry := range entries { + if knownBuiltInToolsForCodemod[entry.key] { + continue + } + unknownEntries = append(unknownEntries, entry) + unknownByStart[entry.start] = entry + } + + if len(unknownEntries) == 0 { + return lines, false + } + + shouldRemoveToolsBlock := len(unknownEntries) == len(entries) + removeLine := make([]bool, len(lines)) + if shouldRemoveToolsBlock { + for i := toolsIndex; i < toolsEndIndex; i++ { + removeLine[i] = true + } + } else { + for _, entry := range unknownEntries { + for i := entry.start; i < entry.end; i++ { + removeLine[i] = true + } + } + } + + filtered := make([]string, 0, len(lines)) + for i, line := range lines { + if !removeLine[i] { + filtered = append(filtered, line) + } + } + + mcpServerLines := make([]string, 0) + for _, entry := range unknownEntries { + toolValue := toolsMap[entry.key] + mcpServerLines = append(mcpServerLines, buildMCPServerEntryLines(entry, toolValue)...) + } + + updated := insertMCPServerEntries(filtered, mcpServerLines) + unknownToolsToMCPServersCodemodLog.Printf("Migrated %d unknown tools entries to mcp-servers", len(unknownEntries)) + return updated, true + }) + }, + } +} + +func findTopLevelToolsEntries(lines []string) (int, int, []toolsBlockEntry, bool) { + toolsIndex := -1 + toolsIndent := "" + toolsEnd := len(lines) + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, "tools:") { + toolsIndex = i + toolsIndent = getIndentation(line) + break + } + } + + if toolsIndex == -1 { + return -1, -1, nil, false + } + + for i := toolsIndex + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if hasExitedBlock(lines[i], toolsIndent) { + toolsEnd = i + break + } + } + + entryIndentLen := len(toolsIndent) + 2 + entries := make([]toolsBlockEntry, 0) + + for i := toolsIndex + 1; i < toolsEnd; { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + i++ + continue + } + + indent := getIndentation(lines[i]) + if !isValidToolsEntryLine(indent, trimmed, entryIndentLen) { + i++ + continue + } + + key := strings.TrimSpace(strings.SplitN(trimmed, ":", 2)[0]) + start := i + i++ + + for i < toolsEnd { + nextTrimmed := strings.TrimSpace(lines[i]) + if nextTrimmed == "" || strings.HasPrefix(nextTrimmed, "#") { + i++ + continue + } + + nextIndent := getIndentation(lines[i]) + if isValidToolsEntryLine(nextIndent, nextTrimmed, entryIndentLen) { + break + } + + if len(nextIndent) < entryIndentLen { + break + } + + i++ + } + + entries = append(entries, toolsBlockEntry{ + key: key, + start: start, + end: i, + indent: indent, + lines: append([]string(nil), lines[start:i]...), + }) + } + + return toolsIndex, toolsEnd, entries, true +} + +func isValidToolsEntryLine(indent, trimmed string, entryIndentLen int) bool { + return len(indent) == entryIndentLen && strings.Contains(trimmed, ":") && !strings.HasPrefix(trimmed, "-") +} + +func buildMCPServerEntryLines(entry toolsBlockEntry, toolValue any) []string { + switch typed := toolValue.(type) { + case []any: + return buildMCPServerFromToolList(entry.indent, entry.key, typed) + case []string: + values := make([]any, 0, len(typed)) + for _, v := range typed { + values = append(values, v) + } + return buildMCPServerFromToolList(entry.indent, entry.key, values) + case map[string]any: + block := append([]string(nil), entry.lines...) + if !hasTopLevelCommandField(block) { + insertion := entry.indent + " command: \"...\" # TODO: fill in command" + block = append(block[:1], append([]string{insertion}, block[1:]...)...) + } + return block + default: + return []string{ + fmt.Sprintf("%s%s:", entry.indent, entry.key), + entry.indent + " command: \"...\" # TODO: fill in command", + } + } +} + +func buildMCPServerFromToolList(indent, name string, values []any) []string { + lines := []string{ + fmt.Sprintf("%s%s:", indent, name), + indent + " command: \"...\" # TODO: fill in command", + indent + " tools:", + } + for _, value := range values { + lines = append(lines, fmt.Sprintf("%s - %s", indent, formatYAMLListItem(value))) + } + return lines +} + +func formatYAMLListItem(value any) string { + switch typed := value.(type) { + case string: + if typed == "" { + return "\"\"" + } + return typed + case nil: + return "null" + default: + return fmt.Sprintf("%v", typed) + } +} + +func hasTopLevelCommandField(lines []string) bool { + if len(lines) == 0 { + return false + } + + serverIndentLen := len(getIndentation(lines[0])) + expectedIndentLen := serverIndentLen + 2 + + for i := 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if len(getIndentation(lines[i])) == expectedIndentLen && strings.HasPrefix(trimmed, "command:") { + return true + } + } + + return false +} + +func insertMCPServerEntries(lines []string, entries []string) []string { + mcpIndex := -1 + mcpIndent := "" + mcpEnd := len(lines) + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if isTopLevelKey(line) && strings.HasPrefix(trimmed, "mcp-servers:") { + mcpIndex = i + mcpIndent = getIndentation(line) + break + } + } + + if mcpIndex == -1 { + result := append([]string{}, lines...) + result = append(result, "mcp-servers:") + result = append(result, entries...) + return result + } + + for i := mcpIndex + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if hasExitedBlock(lines[i], mcpIndent) { + mcpEnd = i + break + } + } + + result := make([]string, 0, len(lines)+len(entries)) + result = append(result, lines[:mcpEnd]...) + result = append(result, entries...) + result = append(result, lines[mcpEnd:]...) + return result +} diff --git a/pkg/cli/codemod_tools_unknown_to_mcp_servers_test.go b/pkg/cli/codemod_tools_unknown_to_mcp_servers_test.go new file mode 100644 index 00000000000..ab4ddb3b938 --- /dev/null +++ b/pkg/cli/codemod_tools_unknown_to_mcp_servers_test.go @@ -0,0 +1,118 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestToolsUnknownToMCPServersCodemod(t *testing.T) { + codemod := getToolsUnknownToMCPServersCodemod() + + t.Run("migrates serena list from tools to mcp-servers", func(t *testing.T) { + content := `--- +tools: + serena: ['typescript'] +--- + +# Workflow` + + frontmatter := map[string]any{ + "tools": map[string]any{ + "serena": []any{"typescript"}, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should apply without errors") + assert.True(t, modified, "Codemod should detect and migrate unknown tool entry") + assert.NotContains(t, result, " serena: ['typescript']", "Original tools.serena entry should be removed") + assert.Contains(t, result, "mcp-servers:", "mcp-servers block should be created") + assert.Contains(t, result, " serena:", "Serena should be moved under mcp-servers") + assert.Contains(t, result, " command: \"...\" # TODO: fill in command", "Placeholder command should be inserted") + assert.Contains(t, result, " - typescript", "List value should be wrapped under tools list") + }) + + t.Run("migrates tavily object and keeps built-in tools intact", func(t *testing.T) { + content := `--- +tools: + github: null + tavily: + tools: [search, search_news] +--- + +# Workflow` + + frontmatter := map[string]any{ + "tools": map[string]any{ + "github": nil, + "tavily": map[string]any{ + "tools": []any{"search", "search_news"}, + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should apply without errors") + assert.True(t, modified, "Codemod should migrate unknown tavily entry") + assert.Contains(t, result, "tools:\n github: null", "Built-in tools should remain in tools section") + assert.Contains(t, result, "mcp-servers:", "mcp-servers block should be present") + assert.Contains(t, result, " tavily:", "Tavily should be moved under mcp-servers") + assert.Contains(t, result, " command: \"...\" # TODO: fill in command", "Command placeholder should be added when missing") + assert.Contains(t, result, " tools: [search, search_news]", "Existing tavily config should be preserved") + }) + + t.Run("does not modify recognized built-in tool names", func(t *testing.T) { + content := `--- +tools: + github: null + bash: true +--- + +# Workflow` + + frontmatter := map[string]any{ + "tools": map[string]any{ + "github": nil, + "bash": true, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should not error on built-in tools") + assert.False(t, modified, "Codemod should not modify built-in tool entries") + assert.Equal(t, content, result, "Content should remain unchanged for built-in tool entries") + }) + + t.Run("appends migrated entries to existing mcp-servers", func(t *testing.T) { + content := `--- +tools: + serena: ['typescript'] +mcp-servers: + existing: + command: "node server.js" +--- + +# Workflow` + + frontmatter := map[string]any{ + "tools": map[string]any{ + "serena": []any{"typescript"}, + }, + "mcp-servers": map[string]any{ + "existing": map[string]any{ + "command": "node server.js", + }, + }, + } + + result, modified, err := codemod.Apply(content, frontmatter) + require.NoError(t, err, "Codemod should apply without errors") + assert.True(t, modified, "Codemod should migrate unknown tool into existing mcp-servers block") + assert.Contains(t, result, " existing:", "Existing server config should be preserved") + assert.Contains(t, result, " serena:", "Migrated server should be appended to existing mcp-servers block") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index e0f627d7eaf..9d2a67c3948 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -34,6 +34,7 @@ func GetAllCodemods() []Codemod { getScheduleAtToAroundCodemod(), getDeleteSchemaFileCodemod(), getGrepToolRemovalCodemod(), + getToolsUnknownToMCPServersCodemod(), getMCPNetworkMigrationCodemod(), getDiscussionFlagRemovalCodemod(), getMCPModeToTypeCodemod(), diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 04ec1009338..382ec3a4d15 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 := 30 + expectedCount := 31 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -80,6 +80,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "delete-schema-file", "grep-tool-removal", "mcp-network-to-top-level-migration", + "tools-unknown-to-mcp-servers", "safe-inputs-to-mcp-scripts", } @@ -117,6 +118,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "schedule-at-to-around-migration", "delete-schema-file", "grep-tool-removal", + "tools-unknown-to-mcp-servers", "mcp-network-to-top-level-migration", "add-comment-discussion-removal", "mcp-mode-to-type-migration",