From 8fc87bafef8cefea3e3d18866cbe8e98395b5157 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 04:50:25 +0000 Subject: [PATCH 1/2] refactor(parser): extract extractBuiltinMCPTools helper to remove duplicate logic The tools-section processing loop (serena rejection + github/playwright handling) was duplicated verbatim in two branches of ExtractMCPConfigurations: one for the no-mcp-servers path and one for the mcp-servers path. Extract it into a shared extractBuiltinMCPTools helper to eliminate the duplication. No functional changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/parser/mcp.go | 78 +++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 44 deletions(-) diff --git a/pkg/parser/mcp.go b/pkg/parser/mcp.go index 27ad90cd53c..5018f282057 100644 --- a/pkg/parser/mcp.go +++ b/pkg/parser/mcp.go @@ -173,32 +173,15 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( } } + // Process built-in MCP tools from tools section (github, playwright) + if err := extractBuiltinMCPTools(frontmatter, serverFilter, &configs); err != nil { + return nil, err + } + // Get mcp-servers section from frontmatter mcpServersSection, hasMCPServers := frontmatter["mcp-servers"] if !hasMCPServers { mcpLog.Print("No mcp-servers section found, checking for built-in tools") - // Also check tools section for built-in MCP tools (github, playwright) - toolsSection, hasTools := frontmatter["tools"] - if hasTools { - if tools, ok := toolsSection.(map[string]any); ok { - for toolName, toolValue := range tools { - if toolName == "serena" { - return nil, fmt.Errorf("tools.serena is removed") - } - // Only handle built-in MCP tools (github, playwright) - if toolName == "github" || toolName == "playwright" { - config, err := processBuiltinMCPTool(toolName, toolValue, serverFilter) - if err != nil { - return nil, err - } - if config != nil { - mcpLog.Printf("Added built-in MCP tool: %s", toolName) - configs = append(configs, *config) - } - } - } - } - } mcpLog.Printf("Extracted %d MCP configurations total", len(configs)) return configs, nil // No mcp-servers configured, but we might have safe-outputs and built-in tools } @@ -208,28 +191,6 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( return nil, fmt.Errorf("mcp-servers section must be a map, got %T. Example:\nmcp-servers:\n my-server:\n command: \"npx @my/tool\"\n args: [\"--port\", \"3000\"]", mcpServersSection) } - // Process built-in MCP tools from tools section - toolsSection, hasTools := frontmatter["tools"] - if hasTools { - if tools, ok := toolsSection.(map[string]any); ok { - for toolName, toolValue := range tools { - if toolName == "serena" { - return nil, fmt.Errorf("tools.serena is removed") - } - // Only handle built-in MCP tools (github, playwright) - if toolName == "github" || toolName == "playwright" { - config, err := processBuiltinMCPTool(toolName, toolValue, serverFilter) - if err != nil { - return nil, err - } - if config != nil { - configs = append(configs, *config) - } - } - } - } - } - // Process custom MCP servers from mcp-servers section mcpLog.Printf("Processing %d custom MCP servers", len(mcpServers)) for serverName, serverValue := range mcpServers { @@ -257,6 +218,35 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( return configs, nil } +// extractBuiltinMCPTools reads the tools section and appends github/playwright configs to configs. +// It returns an error if a removed tool (serena) is present. +func extractBuiltinMCPTools(frontmatter map[string]any, serverFilter string, configs *[]RegistryMCPServerConfig) error { + toolsSection, hasTools := frontmatter["tools"] + if !hasTools { + return nil + } + tools, ok := toolsSection.(map[string]any) + if !ok { + return nil + } + for toolName, toolValue := range tools { + if toolName == "serena" { + return fmt.Errorf("tools.serena is removed") + } + if toolName == "github" || toolName == "playwright" { + config, err := processBuiltinMCPTool(toolName, toolValue, serverFilter) + if err != nil { + return err + } + if config != nil { + mcpLog.Printf("Added built-in MCP tool: %s", toolName) + *configs = append(*configs, *config) + } + } + } + return nil +} + // processBuiltinMCPTool handles built-in MCP tools (github and playwright) func processBuiltinMCPTool(toolName string, toolValue any, serverFilter string) (*RegistryMCPServerConfig, error) { // Apply server filter if specified From 20d75b5b514963bcd8f9bebfc0045d591ee73988 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 13:01:23 +0000 Subject: [PATCH 2/2] fix(parser): restore mcp-servers error precedence for built-in tool extraction Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/mcp.go | 14 +++++++++----- pkg/parser/mcp_test.go | 11 +++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/pkg/parser/mcp.go b/pkg/parser/mcp.go index 5018f282057..5b894408360 100644 --- a/pkg/parser/mcp.go +++ b/pkg/parser/mcp.go @@ -173,15 +173,14 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( } } - // Process built-in MCP tools from tools section (github, playwright) - if err := extractBuiltinMCPTools(frontmatter, serverFilter, &configs); err != nil { - return nil, err - } - // Get mcp-servers section from frontmatter mcpServersSection, hasMCPServers := frontmatter["mcp-servers"] if !hasMCPServers { mcpLog.Print("No mcp-servers section found, checking for built-in tools") + // Process built-in MCP tools from tools section (github, playwright) + if err := extractBuiltinMCPTools(frontmatter, serverFilter, &configs); err != nil { + return nil, err + } mcpLog.Printf("Extracted %d MCP configurations total", len(configs)) return configs, nil // No mcp-servers configured, but we might have safe-outputs and built-in tools } @@ -191,6 +190,11 @@ func ExtractMCPConfigurations(frontmatter map[string]any, serverFilter string) ( return nil, fmt.Errorf("mcp-servers section must be a map, got %T. Example:\nmcp-servers:\n my-server:\n command: \"npx @my/tool\"\n args: [\"--port\", \"3000\"]", mcpServersSection) } + // Process built-in MCP tools from tools section (github, playwright) + if err := extractBuiltinMCPTools(frontmatter, serverFilter, &configs); err != nil { + return nil, err + } + // Process custom MCP servers from mcp-servers section mcpLog.Printf("Processing %d custom MCP servers", len(mcpServers)) for serverName, serverValue := range mcpServers { diff --git a/pkg/parser/mcp_test.go b/pkg/parser/mcp_test.go index d3def3d698f..dcd363a10e6 100644 --- a/pkg/parser/mcp_test.go +++ b/pkg/parser/mcp_test.go @@ -142,6 +142,17 @@ func TestExtractMCPConfigurations(t *testing.T) { expectError: true, expectedErrContains: "tools.serena is removed", }, + { + name: "mcp-servers type error takes precedence over removed serena", + frontmatter: map[string]any{ + "mcp-servers": "invalid", + "tools": map[string]any{ + "serena": true, + }, + }, + expectError: true, + expectedErrContains: "mcp-servers section must be a map", + }, { name: "GitHub tool default configuration", frontmatter: map[string]any{