-
Notifications
You must be signed in to change notification settings - Fork 46
Description
Overview
The file pkg/cli/mcp_inspect.go has grown to 1011 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_inspect.go - Size: 1011 lines
- Test Coverage: 440 lines (test-to-source ratio: 0.44)
- Complexity: Multiple distinct functional domains (workflow inspection, safe-inputs server management, HTTP server orchestration, MCP inspector spawning)
Full File Analysis
Function Distribution
The file contains 12 functions spanning 4 major functional domains:
Workflow MCP Inspection (3 functions, ~300 lines)
InspectWorkflowMCP(lines 112-294) - Main entry point for workflow inspectionlistWorkflowsWithMCP(lines 297-330) - Lists workflows with MCP configurationsfilterOutSafeOutputs(lines 33-41) - Filters safe-outputs from MCP listapplyImportsToFrontmatter(lines 45-109) - Merges imported MCP servers and tools
Safe-Inputs Server Management (4 functions, ~400 lines)
writeSafeInputsFiles(lines 333-431) - Writes safe-inputs files to diskstartSafeInputsHTTPServer(lines 434-462) - Starts Node.js HTTP serverfindAvailablePort(lines 465-480) - Port discovery logicwaitForServerReady(lines 483-506) - HTTP server health checkingstartSafeInputsServer(lines 509-603) - Orchestrates safe-inputs setup
Safe-Inputs Inspector (1 function, ~120 lines)
spawnSafeInputsInspector(lines 607-720) - Generates files, starts server, launches inspector
MCP Inspector Spawning (2 functions, ~200 lines)
spawnMCPInspector(lines 724-928) - Launches@modelcontextprotocol/inspectorwith stdio server managementNewMCPInspectSubcommand(lines 932-1011) - Cobra command configuration
Complexity Hotspots
InspectWorkflowMCP(182 lines) - Handles frontmatter parsing, import processing, validation, safe-inputs detection, and server inspectionstartSafeInputsServer(94 lines) - Complex orchestration with temporary directory management, port discovery, server startup, and health checkingspawnMCPInspector(204 lines) - Manages stdio server lifecycle, process monitoring with goroutines, and inspector spawningwriteSafeInputsFiles(98 lines) - Writes 9+ JavaScript files and generates tool handlers dynamically
Coupling Issues
- Safe-inputs server logic is tightly coupled to workflow inspection
- HTTP server management mixed with file generation
- Process lifecycle management (goroutines, cleanup) in same function as inspector spawning
- Command configuration depends on multiple helper functions across different domains
Refactoring Strategy
Proposed File Splits
Based on semantic analysis, split the file into the following modules:
-
mcp_inspect.go(Main command entry point)- Functions:
NewMCPInspectSubcommand,InspectWorkflowMCP - Responsibility: CLI command configuration and workflow inspection orchestration
- Estimated LOC: ~250 lines
- Functions:
-
mcp_inspect_list.go(Workflow listing and filtering)- Functions:
listWorkflowsWithMCP,filterOutSafeOutputs - Responsibility: Listing workflows with MCP servers
- Estimated LOC: ~100 lines
- Functions:
-
mcp_inspect_imports.go(Import processing)- Functions:
applyImportsToFrontmatter - Responsibility: Merging imported MCP servers and tools into frontmatter
- Estimated LOC: ~100 lines
- Functions:
-
mcp_inspect_safe_inputs_server.go(Safe-inputs HTTP server management)- Functions:
startSafeInputsServer,startSafeInputsHTTPServer,findAvailablePort,waitForServerReady - Responsibility: Safe-inputs HTTP server lifecycle (port discovery, startup, health checking)
- Estimated LOC: ~200 lines
- Functions:
-
mcp_inspect_safe_inputs_files.go(Safe-inputs file generation)- Functions:
writeSafeInputsFiles - Responsibility: Generating and writing safe-inputs JavaScript/tool files
- Estimated LOC: ~150 lines
- Functions:
-
mcp_inspect_safe_inputs_inspector.go(Safe-inputs inspector)- Functions:
spawnSafeInputsInspector - Responsibility: Spawning inspector for safe-inputs MCP server
- Estimated LOC: ~150 lines
- Functions:
-
mcp_inspect_inspector.go(MCP inspector spawning)- Functions:
spawnMCPInspector - Responsibility: Launching
@modelcontextprotocol/inspectorwith stdio server management - Estimated LOC: ~250 lines
- Functions:
Interface Abstractions
Consider introducing interfaces to reduce coupling:
// ServerStarter abstracts safe-inputs HTTP server startup
type ServerStarter interface {
StartServer(config *workflow.SafeInputsConfig, verbose bool) (*parser.MCPServerConfig, *exec.Cmd, string, error)
}
// FileWriter abstracts safe-inputs file generation
type FileWriter interface {
WriteFiles(dir string, config *workflow.SafeInputsConfig, verbose bool) error
}
// PortFinder abstracts port discovery logic
type PortFinder interface {
FindAvailablePort(startPort int, verbose bool) int
}Test Coverage Plan
Add comprehensive tests for each new file:
-
mcp_inspect_list_test.go- Test cases: Filter safe-outputs, list workflows with/without MCP
- Target coverage: >80%
-
mcp_inspect_imports_test.go- Test cases: Merge imported MCP servers, merge tools, handle empty imports
- Target coverage: >80%
-
mcp_inspect_safe_inputs_server_test.go- Test cases: Port discovery, server startup, health checking, timeout handling
- Target coverage: >80%
-
mcp_inspect_safe_inputs_files_test.go- Test cases: Write JavaScript files, generate tool handlers, handle missing tools
- Target coverage: >80%
-
mcp_inspect_safe_inputs_inspector_test.go- Test cases: Spawn inspector, handle missing Node.js, cleanup on error
- Target coverage: >80%
-
mcp_inspect_inspector_test.go- Test cases: Spawn inspector, start stdio servers, process cleanup, timeout handling
- Target coverage: >80%
-
mcp_inspect_test.go- Test cases: End-to-end workflow inspection, integration tests
- Target coverage: >80%
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically
- Maintain Exports: Keep public API unchanged (
NewMCPInspectSubcommand,InspectWorkflowMCP) - Add Tests First: Write tests for each new file before refactoring
- Incremental Changes: Split one module at a time
- Run Tests Frequently: Verify
make test-unitpasses after each split - Update Imports: Ensure all import paths are correct
- Document Changes: Add comments explaining module boundaries
Acceptance Criteria
- Original file is split into 7 focused files
- Each new file is under 300 lines
- All tests pass (
make test-unit) - Test coverage is ≥80% for new files
- No breaking changes to public API
- Code passes linting (
make lint) - Build succeeds (
make build)
Additional Context
- Repository Guidelines: Follow patterns in
AGENTS.mdandspecs/cli-command-patterns.md - Code Organization: Prefer many small files grouped by functionality
- Testing: Match existing test patterns in
pkg/cli/*_test.go - Safe-Inputs: This command manages safe-inputs HTTP servers for inspection - careful refactoring needed to preserve server lifecycle management
Related Files:
pkg/cli/mcp_inspect_test.go- Existing test file (440 lines)pkg/workflow/safe_inputs_*.go- Safe-inputs configuration and generationpkg/parser/mcp.go- MCP server configuration parsing
Key Dependencies:
- Safe-inputs server management requires Node.js
- HTTP server uses ports 3000-3010 range
- Inspector spawning uses
npx@modelcontextprotocol/inspector``
Priority: Medium
Effort: Large (7 file splits, complex server lifecycle management, goroutine cleanup)
Expected Impact: Improved maintainability, easier testing, reduced complexity, clearer separation of concerns
AI generated by Daily File Diet