-
Notifications
You must be signed in to change notification settings - Fork 235
Description
Overview
The file pkg/cli/mcp_server.go has grown to 1,036 lines, making it difficult to maintain and test. This task involves refactoring it into smaller, focused files with improved test coverage.
Current State
- File:
pkg/cli/mcp_server.go - Size: 1,036 lines
- Test Coverage: 3,147 test lines across multiple test files (excellent ratio ~3:1)
- Complexity: Single 841-line function (
createMCPServer) containing 8 inline tool registrations
Full File Analysis
Function Distribution
The file contains 7 functions:
mcpErrorData(lines 27-38): Helper for error marshalingNewMCPServerCommand(lines 41-81): Cobra command setuprunMCPServer(lines 84-112): Server initializationcreateMCPServer(lines 113-954): 841-line monster function with 8 tool registrationssanitizeForLog(lines 959-964): Input sanitizationloggingHandler(lines 972-1002): HTTP logging middlewarerunHTTPServer(lines 1005-1036): HTTP server setup
Tool Registration Breakdown
The createMCPServer function contains 8 inline tool registrations:
- status (line 143): ~69 lines - Workflow status queries
- compile (line 236): ~140 lines - Markdown to YAML compilation
- logs (line 412): ~152 lines - Log download and analysis
- audit (line 572): ~80 lines - Workflow run investigation
- mcp-inspect (line 661): ~71 lines - MCP server inspection
- add (line 741): ~59 lines - Add workflows from remote repos
- update (line 808): ~64 lines - Update workflows from sources
- fix (line 881): ~71 lines - Apply codemod fixes
Each tool includes:
- Argument struct definition
- Tool metadata (name, description, icons)
- Handler function implementation
- Context cancellation checks
- Command argument building
- Subprocess execution
- Error handling
- Output formatting
Complexity Hotspots
-
createMCPServerfunction (841 lines)- Contains 8 separate tool implementations inline
- High coupling between tool definitions and registration logic
- Difficult to test individual tools in isolation
- Hard to navigate and understand
-
Duplicate patterns
- Context cancellation checks repeated 8 times
- Command execution patterns repeated across tools
- Error handling code duplicated
- Output filtering (jq) logic repeated
-
Schema generation complexity
- Custom schema generation with elicitation defaults
- Schema modifications for specific tools
- Error handling for schema generation failures
Refactoring Strategy
Proposed File Splits
Split the file into focused modules following a consistent pattern:
-
mcp_server_command.go(~120 lines)- Functions:
NewMCPServerCommand,runMCPServer,mcpErrorData - Responsibility: Cobra command setup and server lifecycle
- Estimated LOC: 120
- Functions:
-
mcp_server_factory.go(~150 lines)- Functions:
createMCPServer,registerAllTools - Responsibility: Server creation and tool registration orchestration
- Estimated LOC: 150
- Functions:
-
mcp_server_tool_status.go(~80 lines)- Functions:
registerStatusTool, status handler - Responsibility: Status tool implementation
- Estimated LOC: 80
- Functions:
-
mcp_server_tool_compile.go(~180 lines)- Functions:
registerCompileTool, compile handler with schema generation - Responsibility: Compile tool implementation
- Estimated LOC: 180
- Functions:
-
mcp_server_tool_logs.go(~200 lines)- Functions:
registerLogsTool, logs handler with guardrail - Responsibility: Logs tool implementation
- Estimated LOC: 200
- Functions:
-
mcp_server_tool_audit.go(~100 lines)- Functions:
registerAuditTool, audit handler - Responsibility: Audit tool implementation
- Estimated LOC: 100
- Functions:
-
mcp_server_tool_inspect.go(~90 lines)- Functions:
registerMCPInspectTool, inspect handler - Responsibility: MCP inspect tool implementation
- Estimated LOC: 90
- Functions:
-
mcp_server_tool_add.go(~80 lines)- Functions:
registerAddTool, add handler - Responsibility: Add tool implementation
- Estimated LOC: 80
- Functions:
-
mcp_server_tool_update.go(~85 lines)- Functions:
registerUpdateTool, update handler - Responsibility: Update tool implementation
- Estimated LOC: 85
- Functions:
-
mcp_server_tool_fix.go(~90 lines)- Functions:
registerFixTool, fix handler - Responsibility: Fix tool implementation
- Estimated LOC: 90
- Functions:
-
mcp_server_http.go(~80 lines)- Functions:
runHTTPServer,loggingHandler,sanitizeForLog,responseWriter - Responsibility: HTTP/SSE transport layer
- Estimated LOC: 80
- Functions:
Shared Utilities
Extract common functionality:
mcp_server_helpers.go(~100 lines)- Functions for context cancellation checking
- Command execution wrapper (
execCmdpattern) - Output filtering (jq filter application)
- Error data marshaling patterns
- Estimated LOC: 100
Pattern for Tool Files
Each mcp_server_tool_*.go file follows this structure:
package cli
import (
"context"
"github.com/modelcontextprotocol/go-sdk/mcp"
)
// [ToolName]Args defines the input parameters for the [tool-name] tool
type [ToolName]Args struct {
// Field definitions with jsonschema tags
}
// register[ToolName]Tool adds the [tool-name] tool to the MCP server
func register[ToolName]Tool(server *mcp.Server, execCmd execCmdFunc) error {
// Tool registration logic
// Returns error if schema generation or registration fails
}
// handle[ToolName] implements the [tool-name] tool handler
func handle[ToolName](ctx context.Context, args [ToolName]Args, execCmd execCmdFunc) (*mcp.CallToolResult, error) {
// Tool implementation
}Test Coverage Plan
Each new file needs corresponding test files with comprehensive coverage:
-
mcp_server_command_test.go- Test cases: Command creation, flag parsing, server initialization
- Target coverage: >80%
-
mcp_server_factory_test.go- Test cases: Server creation with all tools, tool registration order
- Target coverage: >80%
-
mcp_server_tool_status_test.go- Test cases: Pattern filtering, jq filtering, error handling
- Target coverage: >80%
-
mcp_server_tool_compile_test.go- Test cases: Workflow compilation, strict mode, static analysis flags, schema defaults
- Target coverage: >80%
-
mcp_server_tool_logs_test.go- Test cases: Log downloading, timeout handling, firewall filtering, guardrail activation
- Target coverage: >80%
-
mcp_server_tool_audit_test.go- Test cases: Run ID parsing, URL parsing, job URL handling, JSON output
- Target coverage: >80%
-
mcp_server_tool_inspect_test.go- Test cases: Workflow listing, server inspection, tool details, secret checking
- Target coverage: >80%
-
mcp_server_tool_add_test.go- Test cases: Workflow addition, numbered copies, custom names
- Target coverage: >80%
-
mcp_server_tool_update_test.go- Test cases: Version updates, major updates, force flag
- Target coverage: >80%
-
mcp_server_tool_fix_test.go- Test cases: Codemod application, write flag, list codemods
- Target coverage: >80%
-
mcp_server_http_test.go- Test cases: HTTP server startup, logging middleware, request sanitization
- Target coverage: >80%
-
mcp_server_helpers_test.go- Test cases: Context cancellation, command execution, output filtering, error marshaling
- Target coverage: >80%
Important: Existing test coverage is excellent (3,147 lines). Ensure all existing tests still pass after refactoring and reorganize them to match the new file structure.
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically
- Maintain Exports: Keep public API unchanged (
NewMCPServerCommand) - Add Tests First: Write tests for each new file before refactoring
- Incremental Changes: Split one tool at a time, starting with simplest (e.g.,
add,update,fix) - Run Tests Frequently: Verify
make test-unitpasses after each split - Update Imports: Ensure all import paths are correct
- Document Changes: Add comments explaining tool registration pattern
Acceptance Criteria
- Original file is split into 12 focused files
- Each new file is under 200 lines
- All tests pass (
make test-unit) - Test coverage remains ≥80% for all new files
- No breaking changes to public API
- Code passes linting (
make lint) - Build succeeds (
make build) - MCP server functionality works identically (manual test)
- All 8 tools still function correctly
Additional Context
Repository Patterns
- File organization: This follows the pattern established in
pkg/workflow/where large files are split by functional domain - Test organization: Test files match source file names (e.g.,
mcp_server_tool_status_test.go) - Console formatting: Use
console.FormatErrorMessage()for error output to stderr - Logger usage: Create logger with
logger.New("mcp:server")pattern
Design Considerations
Why split by tool?
- Each tool is an independent feature with distinct arguments, validation, and execution
- Tool files can be developed, tested, and reviewed independently
- New tools can be added without modifying existing tool implementations
- Clearer separation of concerns
Why keep factory separate?
- Provides single point for tool registration orchestration
- Makes it easy to enable/disable tools
- Simplifies testing of server initialization
Why extract helpers?
- Eliminates code duplication across 8 tool implementations
- Makes patterns explicit and testable
- Provides clear API for tool development
Related Files
- Existing tests:
pkg/cli/mcp_server_*_test.go(multiple files, 3,147 total lines) - MCP SDK:
github.com/modelcontextprotocol/go-sdk/mcp - Console formatting:
pkg/console/ - Logger:
pkg/logger/
Testing Strategy
Use existing test patterns from mcp_server_*_test.go:
- Table-driven tests with
t.Run() - Test server creation in isolation
- Test tool handlers with mock command execution
- Test schema generation and validation
- Test error handling paths
Priority: Medium
Effort: Large (estimated 8-12 hours)
Expected Impact: Significantly improved maintainability, easier testing, reduced complexity, clearer tool implementation patterns
AI generated by Daily File Diet
- expires on Feb 6, 2026, 1:34 PM UTC