Remove mode field from custom MCP servers schema and add migration codemod

pkg/cli/codemod_mcp_mode_to_type.go

@@ -0,0 +1,107 @@
+package cli
+
+import (
+	"strings"
+
+	"github.com/githubnext/gh-aw/pkg/logger"
+)
+
+var mcpModeToTypeCodemodLog = logger.New("cli:codemod_mcp_mode_to_type")
+
+// getMCPModeToTypeCodemod creates a codemod for migrating 'mode' to 'type' in custom MCP server configurations
+func getMCPModeToTypeCodemod() Codemod {
+	return Codemod{
+		ID:           "mcp-mode-to-type-migration",
+		Name:         "Migrate MCP 'mode' to 'type'",
+		Description:  "Renames 'mode' field to 'type' in custom MCP server configurations (mcp-servers section). Does not affect GitHub or Serena tool configurations.",
+		IntroducedIn: "0.7.0",
+		Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
+			// Check if mcp-servers section exists
+			mcpServersValue, hasMCPServers := frontmatter["mcp-servers"]
+			if !hasMCPServers {
+				return content, false, nil
+			}
+
+			mcpServersMap, ok := mcpServersValue.(map[string]any)
+			if !ok {
+				return content, false, nil
+			}
+
+			// Check if any MCP server has a 'mode' field
+			hasMode := false
+			for _, serverValue := range mcpServersMap {
+				serverConfig, ok := serverValue.(map[string]any)
+				if !ok {
+					continue
+				}
+				if _, hasModeField := serverConfig["mode"]; hasModeField {
+					hasMode = true
+					break
+				}
+			}
+
+			if !hasMode {
+				return content, false, nil
+			}
+
+			// Parse frontmatter to get raw lines
+			frontmatterLines, markdown, err := parseFrontmatterLines(content)
+			if err != nil {
+				return content, false, err
+			}
+
+			// Rename 'mode' to 'type' in all MCP servers
+			result, modified := renameModeToTypeInMCPServers(frontmatterLines)
+			if !modified {
+				return content, false, nil
+			}
+
+			// Reconstruct the content
+			newContent := reconstructContent(result, markdown)
+			mcpModeToTypeCodemodLog.Print("Applied MCP mode-to-type migration")
+			return newContent, true, nil
+		},
+	}
+}
+
+// renameModeToTypeInMCPServers renames 'mode:' to 'type:' within mcp-servers blocks
+func renameModeToTypeInMCPServers(lines []string) ([]string, bool) {
+	var result []string
+	var modified bool
+	var inMCPServers bool
+	var mcpServersIndent string
+
+	for i, line := range lines {
+		trimmedLine := strings.TrimSpace(line)
+
+		// Track if we're in mcp-servers block
+		if strings.HasPrefix(trimmedLine, "mcp-servers:") {
+			inMCPServers = true
+			mcpServersIndent = getIndentation(line)
+			result = append(result, line)
+			continue
+		}
+
+		// Check if we've left mcp-servers block
+		if inMCPServers && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") {
+			if hasExitedBlock(line, mcpServersIndent) {
+				inMCPServers = false
+			}
+		}
+
+		// Rename 'mode:' to 'type:' if we're in mcp-servers block
+		if inMCPServers && strings.HasPrefix(trimmedLine, "mode:") {
+			newLine, replaced := findAndReplaceInLine(line, "mode", "type")
+			if replaced {
+				result = append(result, newLine)
+				modified = true
+				mcpModeToTypeCodemodLog.Printf("Renamed 'mode' to 'type' on line %d", i+1)
+				continue
+			}
+		}
+
+		result = append(result, line)
+	}
+
+	return result, modified
+}

pkg/cli/codemod_mcp_mode_to_type_test.go

@@ -0,0 +1,209 @@
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMCPModeToTypeCodemod(t *testing.T) {
+	codemod := getMCPModeToTypeCodemod()
+
+	t.Run("renames mode to type in custom MCP servers", func(t *testing.T) {
+		content := `---
+engine: copilot
+tools:
+  github: null
+mcp-servers:
+  my-custom-server:
+    mode: stdio
+    command: npx
+    args: ["-y", "@my/server"]
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"tools": map[string]any{
+				"github": nil,
+			},
+			"mcp-servers": map[string]any{
+				"my-custom-server": map[string]any{
+					"mode":    "stdio",
+					"command": "npx",
+					"args":    []any{"-y", "@my/server"},
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error when applying codemod")
+		assert.True(t, modified, "Should modify content")
+		assert.Contains(t, result, "type: stdio", "Should rename mode to type")
+		assert.NotContains(t, result, "mode: stdio", "Should not contain old mode field")
+	})
+
+	t.Run("does not modify workflows without mcp-servers", func(t *testing.T) {
+		content := `---
+engine: copilot
+tools:
+  github:
+    mode: remote
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"tools": map[string]any{
+				"github": map[string]any{
+					"mode": "remote",
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error")
+		assert.False(t, modified, "Should not modify content without mcp-servers")
+		assert.Equal(t, content, result, "Content should remain unchanged")
+	})
+
+	t.Run("does not modify GitHub tool mode field", func(t *testing.T) {
+		content := `---
+engine: copilot
+tools:
+  github:
+    mode: remote
+mcp-servers:
+  my-server:
+    mode: stdio
+    command: node
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"tools": map[string]any{
+				"github": map[string]any{
+					"mode": "remote",
+				},
+			},
+			"mcp-servers": map[string]any{
+				"my-server": map[string]any{
+					"mode":    "stdio",
+					"command": "node",
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error")
+		assert.True(t, modified, "Should modify mcp-servers")
+		assert.Contains(t, result, "mode: remote", "Should keep GitHub tool mode field")
+		assert.Contains(t, result, "type: stdio", "Should rename mode in mcp-servers to type")
+		assert.NotContains(t, result, "my-server:\n    mode: stdio", "Should not contain mode in mcp-servers")
+	})
+
+	t.Run("handles multiple MCP servers with mode", func(t *testing.T) {
+		content := `---
+engine: copilot
+mcp-servers:
+  server1:
+    mode: stdio
+    command: npm
+  server2:
+    mode: http
+    url: http://localhost:8080
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"mcp-servers": map[string]any{
+				"server1": map[string]any{
+					"mode":    "stdio",
+					"command": "npm",
+				},
+				"server2": map[string]any{
+					"mode": "http",
+					"url":  "http://localhost:8080",
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error")
+		assert.True(t, modified, "Should modify content")
+		assert.Contains(t, result, "type: stdio", "Should rename first server mode")
+		assert.Contains(t, result, "type: http", "Should rename second server mode")
+		assert.NotContains(t, result, "mode: stdio", "Should not contain mode: stdio")
+		assert.NotContains(t, result, "mode: http", "Should not contain mode: http")
+	})
+
+	t.Run("does not modify when no mode field exists", func(t *testing.T) {
+		content := `---
+engine: copilot
+mcp-servers:
+  my-server:
+    type: stdio
+    command: node
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"mcp-servers": map[string]any{
+				"my-server": map[string]any{
+					"type":    "stdio",
+					"command": "node",
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error")
+		assert.False(t, modified, "Should not modify content when no mode field")
+		assert.Equal(t, content, result, "Content should remain unchanged")
+	})
+
+	t.Run("preserves comments and formatting", func(t *testing.T) {
+		content := `---
+engine: copilot
+mcp-servers:
+  my-server:
+    # MCP connection mode
+    mode: stdio  # Use stdio transport
+    command: node
+---
+
+# Test Workflow
+`
+
+		frontmatter := map[string]any{
+			"engine": "copilot",
+			"mcp-servers": map[string]any{
+				"my-server": map[string]any{
+					"mode":    "stdio",
+					"command": "node",
+				},
+			},
+		}
+
+		result, modified, err := codemod.Apply(content, frontmatter)
+		require.NoError(t, err, "Should not error")
+		assert.True(t, modified, "Should modify content")
+		assert.Contains(t, result, "# MCP connection mode", "Should preserve comment")
+		assert.Contains(t, result, "type: stdio  # Use stdio transport", "Should preserve inline comment and formatting")
+		assert.NotContains(t, result, "mode: stdio", "Should not contain old field")
+	})
+}

pkg/cli/fix_codemods.go

@@ -32,5 +32,6 @@ func GetAllCodemods() []Codemod {
 		getGrepToolRemovalCodemod(),
 		getMCPNetworkMigrationCodemod(),
 		getDiscussionFlagRemovalCodemod(),
+		getMCPModeToTypeCodemod(),
 	}
 }

pkg/cli/fix_codemods_test.go

@@ -41,7 +41,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) {
 	codemods := GetAllCodemods()
 
 	// Verify we have the expected number of codemods
-	expectedCount := 14
+	expectedCount := 15
 	assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount)
 
 	// Verify all codemods have required fields
@@ -116,6 +116,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) {
 		"grep-tool-removal",
 		"mcp-network-to-top-level-migration",
 		"add-comment-discussion-removal",
+		"mcp-mode-to-type-migration",
 	}
 
 	require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods")

pkg/parser/schemas/main_workflow_schema.json

@@ -3425,15 +3425,10 @@
                 },
                 "description": "Environment variables"
               },
-              "mode": {
-                "type": "string",
-                "enum": ["stdio", "http", "remote", "local"],
-                "description": "MCP server mode"
-              },
               "type": {
                 "type": "string",
                 "enum": ["stdio", "http", "remote", "local"],
-                "description": "MCP server type"
+                "description": "MCP connection type. Use 'stdio' for command-based or container-based servers, 'http' for HTTP-based servers. 'local' is an alias for 'stdio' and is normalized during parsing."
               },
               "version": {
                 "type": ["string", "number"],