Refactor: Move import/merge code to imports.go and fix linting

- Created pkg/workflow/imports.go with mergeTools and mergeMCPServers functions - Removed these functions from compiler.go for better code organization - Fixed linting issue: removed unnecessary nil check in mergeMCPServers (S1031) - All tests passing including import-related tests - Scout workflow still compiles correctly with Tavily MCP server Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan · Oct 7, 2025 · Oct 6, 2025 · Oct 6, 2025 · Oct 6, 2025 · Oct 6, 2025
commit 1946adede5f28947686913d7fd6af8dcfb3c0827
diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go
@@ -1176,81 +1176,6 @@ func (c *Compiler) extractCommandConfig(frontmatter map[string]any) (commandName
 	return "", nil
 }
 
-// mergeTools merges two tools maps, combining allowed arrays when keys coincide
-// Handles newline-separated JSON objects from multiple imports/includes
-func (c *Compiler) mergeTools(topTools map[string]any, includedToolsJSON string) (map[string]any, error) {
-	if includedToolsJSON == "" || includedToolsJSON == "{}" {
-		return topTools, nil
-	}
-
-	// Split by newlines to handle multiple JSON objects from different imports/includes
-	lines := strings.Split(includedToolsJSON, "\n")
-	result := topTools
-	if result == nil {
-		result = make(map[string]any)
-	}
-
-	for _, line := range lines {
-		line = strings.TrimSpace(line)
-		if line == "" || line == "{}" {
-			continue
-		}
-
-		var includedTools map[string]any
-		if err := json.Unmarshal([]byte(line), &includedTools); err != nil {
-			continue // Skip invalid lines
-		}
-
-		// Merge this set of tools
-		merged, err := parser.MergeTools(result, includedTools)
-		if err != nil {
-			return nil, fmt.Errorf("failed to merge tools: %w", err)
-		}
-		result = merged
-	}
-
-	return result, nil
-}
-
-// mergeMCPServers merges mcp-servers from imports with top-level mcp-servers
-// Takes object maps and merges them directly
-func (c *Compiler) mergeMCPServers(topMCPServers map[string]any, importedMCPServersJSON string) (map[string]any, error) {
-	if importedMCPServersJSON == "" || importedMCPServersJSON == "{}" {
-		return topMCPServers, nil
-	}
-
-	// Initialize result with top-level MCP servers
-	result := make(map[string]any)
-	if topMCPServers != nil {
-		for k, v := range topMCPServers {
-			result[k] = v
-		}
-	}
-
-	// Split by newlines to handle multiple JSON objects from different imports
-	lines := strings.Split(importedMCPServersJSON, "\n")
-
-	for _, line := range lines {
-		line = strings.TrimSpace(line)
-		if line == "" || line == "{}" {
-			continue
-		}
-
-		// Parse JSON line to map
-		var importedMCPServers map[string]any
-		if err := json.Unmarshal([]byte(line), &importedMCPServers); err != nil {
-			continue // Skip invalid lines
-		}
-
-		// Merge MCP servers - imported servers take precedence over top-level ones
-		for serverName, serverConfig := range importedMCPServers {
-			result[serverName] = serverConfig
-		}
-	}
-
-	return result, nil
-}
-
 // mergeSafeJobsFromIncludes merges safe-jobs from included files and detects conflicts
 func (c *Compiler) mergeSafeJobsFromIncludes(topSafeJobs map[string]*SafeJobConfig, includedContentJSON string) (map[string]*SafeJobConfig, error) {
 	if includedContentJSON == "" || includedContentJSON == "{}" {

diff --git a/pkg/workflow/imports.go b/pkg/workflow/imports.go
@@ -0,0 +1,82 @@
+package workflow
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/githubnext/gh-aw/pkg/parser"
+)
+
+// mergeTools merges two tools maps, combining allowed arrays when keys coincide
+// Handles newline-separated JSON objects from multiple imports/includes
+func (c *Compiler) mergeTools(topTools map[string]any, includedToolsJSON string) (map[string]any, error) {
+	if includedToolsJSON == "" || includedToolsJSON == "{}" {
+		return topTools, nil
+	}
+
+	// Split by newlines to handle multiple JSON objects from different imports/includes
+	lines := strings.Split(includedToolsJSON, "\n")
+	result := topTools
+	if result == nil {
+		result = make(map[string]any)
+	}
+
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if line == "" || line == "{}" {
+			continue
+		}
+
+		var includedTools map[string]any
+		if err := json.Unmarshal([]byte(line), &includedTools); err != nil {
+			continue // Skip invalid lines
+		}
+
+		// Merge this set of tools
+		merged, err := parser.MergeTools(result, includedTools)
+		if err != nil {
+			return nil, fmt.Errorf("failed to merge tools: %w", err)
+		}
+		result = merged
+	}
+
+	return result, nil
+}
+
+// mergeMCPServers merges mcp-servers from imports with top-level mcp-servers
+// Takes object maps and merges them directly
+func (c *Compiler) mergeMCPServers(topMCPServers map[string]any, importedMCPServersJSON string) (map[string]any, error) {
+	if importedMCPServersJSON == "" || importedMCPServersJSON == "{}" {
+		return topMCPServers, nil
+	}
+
+	// Initialize result with top-level MCP servers
+	result := make(map[string]any)
+	for k, v := range topMCPServers {
+		result[k] = v
+	}
+
+	// Split by newlines to handle multiple JSON objects from different imports
+	lines := strings.Split(importedMCPServersJSON, "\n")
+
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if line == "" || line == "{}" {
+			continue
+		}
+
+		// Parse JSON line to map
+		var importedMCPServers map[string]any
+		if err := json.Unmarshal([]byte(line), &importedMCPServers); err != nil {
+			continue // Skip invalid lines
+		}
+
+		// Merge MCP servers - imported servers take precedence over top-level ones
+		for serverName, serverConfig := range importedMCPServers {
+			result[serverName] = serverConfig
+		}
+	}
+
+	return result, nil
+}
diff --git a/pkg/workflow/imports_test.go b/pkg/workflow/imports_test.go
@@ -162,25 +162,25 @@ This is a test workflow with multiple imports.
 }
 
 func TestCompileWorkflowWithMCPServersImport(t *testing.T) {
-// Create a temporary directory for test files
-tempDir := t.TempDir()
+	// Create a temporary directory for test files
+	tempDir := t.TempDir()
 
-// Create a shared mcp-servers file (like tavily-mcp.md)
-sharedMCPPath := filepath.Join(tempDir, "shared-mcp.md")
-sharedMCPContent := `---
+	// Create a shared mcp-servers file (like tavily-mcp.md)
+	sharedMCPPath := filepath.Join(tempDir, "shared-mcp.md")
+	sharedMCPContent := `---
 mcp-servers:
   tavily:
     url: "https://mcp.tavily.com/mcp/?tavilyApiKey=test"
     allowed: ["*"]
 ---
 `
-if err := os.WriteFile(sharedMCPPath, []byte(sharedMCPContent), 0644); err != nil {
-t.Fatalf("Failed to write shared MCP file: %v", err)
-}
+	if err := os.WriteFile(sharedMCPPath, []byte(sharedMCPContent), 0644); err != nil {
+		t.Fatalf("Failed to write shared MCP file: %v", err)
+	}
 
-// Create a workflow file that imports the shared MCP server
-workflowPath := filepath.Join(tempDir, "test-workflow.md")
-workflowContent := `---
+	// Create a workflow file that imports the shared MCP server
+	workflowPath := filepath.Join(tempDir, "test-workflow.md")
+	workflowContent := `---
 on: issues
 permissions:
   contents: read
@@ -196,37 +196,37 @@ tools:
 
 This is a test workflow with imported MCP server.
 `
-if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil {
-t.Fatalf("Failed to write workflow file: %v", err)
-}
+	if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil {
+		t.Fatalf("Failed to write workflow file: %v", err)
+	}
 
-// Compile the workflow
-compiler := workflow.NewCompiler(false, "", "test")
-if err := compiler.CompileWorkflow(workflowPath); err != nil {
-t.Fatalf("CompileWorkflow failed: %v", err)
-}
+	// Compile the workflow
+	compiler := workflow.NewCompiler(false, "", "test")
+	if err := compiler.CompileWorkflow(workflowPath); err != nil {
+		t.Fatalf("CompileWorkflow failed: %v", err)
+	}
 
-// Read the generated lock file
-lockFilePath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml"
-lockFileContent, err := os.ReadFile(lockFilePath)
-if err != nil {
-t.Fatalf("Failed to read lock file: %v", err)
-}
+	// Read the generated lock file
+	lockFilePath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml"
+	lockFileContent, err := os.ReadFile(lockFilePath)
+	if err != nil {
+		t.Fatalf("Failed to read lock file: %v", err)
+	}
 
-workflowData := string(lockFileContent)
+	workflowData := string(lockFileContent)
 
-// Verify that the compiled workflow contains the imported MCP server
-if !strings.Contains(workflowData, "tavily") {
-t.Error("Expected compiled workflow to contain tavily MCP server from imported file")
-}
+	// Verify that the compiled workflow contains the imported MCP server
+	if !strings.Contains(workflowData, "tavily") {
+		t.Error("Expected compiled workflow to contain tavily MCP server from imported file")
+	}
 
-// Verify the MCP URL is present
-if !strings.Contains(workflowData, "https://mcp.tavily.com/mcp") {
-t.Error("Expected compiled workflow to contain Tavily MCP URL from imported file")
-}
+	// Verify the MCP URL is present
+	if !strings.Contains(workflowData, "https://mcp.tavily.com/mcp") {
+		t.Error("Expected compiled workflow to contain Tavily MCP URL from imported file")
+	}
 
-// Verify it's configured as an HTTP MCP server
-if !strings.Contains(workflowData, `"type": "http"`) {
-t.Error("Expected tavily to be configured as HTTP MCP server")
-}
+	// Verify it's configured as an HTTP MCP server
+	if !strings.Contains(workflowData, `"type": "http"`) {
+		t.Error("Expected tavily to be configured as HTTP MCP server")
+	}
 }