-
Notifications
You must be signed in to change notification settings - Fork 35
Description
The file pkg/workflow/copilot_engine.go has grown to 1,172 lines, making it difficult to maintain despite having excellent test coverage. This task involves refactoring it into smaller, focused files while preserving the strong test suite.
Current State
- File:
pkg/workflow/copilot_engine.go - Size: 1,172 lines
- Test Coverage: 1,148 lines (98% test-to-source ratio - excellent!)
- Complexity: Contains 18 functions handling engine initialization, installation, execution, MCP configuration, log parsing, and cleanup operations
- Codebase Context: 17 files in the repository currently exceed the 800-line threshold
Refactoring Strategy
Proposed File Splits
Based on semantic analysis of function groupings and responsibilities:
1. copilot_engine_core.go
- Functions:
NewCopilotEngine()GetDefaultDetectionModel()GetDeclaredOutputFiles()GetCleanupStep()
- Responsibility: Core engine initialization and lifecycle management
- Estimated LOC: ~100 lines
2. copilot_engine_installation.go
- Functions:
GetInstallationSteps()generateAWFInstallationStep()
- Responsibility: Installation step generation for Copilot CLI, Node.js, sandbox runtimes (SRT/AWF)
- Estimated LOC: ~250 lines
3. copilot_engine_execution.go
- Functions:
GetExecutionSteps()extractAddDirPaths()computeCopilotToolArguments()generateCopilotToolArgumentsComment()
- Responsibility: Execution step generation, CLI argument construction, tool permission management
- Estimated LOC: ~450 lines
4. copilot_engine_mcp.go
- Functions:
RenderMCPConfig()renderCopilotMCPConfig()
- Responsibility: MCP (Model Context Protocol) configuration rendering for various tools
- Estimated LOC: ~200 lines
5. copilot_engine_logs.go
- Functions:
ParseLogMetrics()parseCopilotToolCallsWithSequence()GetLogParserScriptId()GetLogFileForParsing()GetFirewallLogsCollectionStep()GetSquidLogsSteps()
- Responsibility: Log parsing, metrics extraction, firewall log handling
- Estimated LOC: ~300 lines
6. copilot_engine_errors.go
- Functions:
GetErrorPatterns()
- Responsibility: Error pattern definitions for log analysis
- Estimated LOC: ~100 lines
Shared Utilities
No new utility files needed - existing engine_helpers.go (424 lines) already provides shared functionality across engines.
Interface Abstractions
The current Engine interface (defined in engine.go) is well-designed and doesn't require changes. All refactored files will maintain the same public API.
Test Coverage Plan
The existing test file copilot_engine_test.go (1,148 lines) is comprehensive and should be similarly refactored:
1. copilot_engine_core_test.go
- Test cases: Engine initialization, default model, lifecycle
- Target coverage: >80%
2. copilot_engine_installation_test.go
- Test cases: Installation steps for various configurations (global/local, SRT/AWF/standard)
- Target coverage: >80%
3. copilot_engine_execution_test.go
- Test cases: Execution step generation, argument construction, tool permissions, custom agents
- Target coverage: >80%
4. copilot_engine_mcp_test.go
- Test cases: MCP config rendering for all supported tools (GitHub, Playwright, Serena, etc.)
- Target coverage: >80%
5. copilot_engine_logs_test.go
- Test cases: Log parsing, metrics extraction, firewall logs, tool call sequence parsing
- Target coverage: >80%
6. copilot_engine_errors_test.go
- Test cases: Error pattern matching and validation
- Target coverage: >80%
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically
- Maintain Exports: Keep public API unchanged (exported functions/types)
- Move Tests in Parallel: Refactor test files alongside implementation files
- Incremental Changes: Split one module at a time (suggested order: core → errors → logs → mcp → installation → execution)
- Run Tests Frequently: Verify
make test-unitpasses after each split - Update Imports: Ensure all import paths are correct and no circular dependencies
- Preserve Logger: Keep the shared
copilotLoglogger accessible across all files - Document Changes: Add brief comments explaining module boundaries
Acceptance Criteria
- Original file is split into 6 focused files
- Each new file is under 500 lines (target: 200-300 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) - Test file is similarly refactored into 6 corresponding test files
- Total lines of code may increase slightly due to package declarations and separation, but each file is independently maintainable
Additional Context
- Repository Guidelines: Follow patterns in
.github/instructions/developer.instructions.md - Code Organization: The codebase prefers many small files grouped by functionality (see existing patterns like
create_*.go) - Engine Pattern: Other engines follow similar patterns -
claude_engine.go(340 lines),codex_engine.go(639 lines),custom_engine.go(300 lines) - Testing: Match existing test patterns in
pkg/workflow/*_test.go - Shared Utilities: Use
engine_helpers.gofor cross-engine functionality - Logger Convention: Use
logger.New("workflow:filename")pattern for debug logging
Benefits
- Improved Maintainability: Smaller files are easier to understand and modify
- Easier Testing: Focused test files correspond to focused implementation files
- Reduced Complexity: Each file has a single, clear responsibility
- Better Navigation: Developers can quickly find relevant code
- Parallel Development: Multiple developers can work on different aspects simultaneously
- Consistency: Aligns with codebase patterns (e.g., engine separation, create functions)
Priority: Medium
Effort: Large (due to comprehensive test suite requiring parallel refactoring)
Expected Impact: Improved maintainability, easier testing, reduced cognitive load
Risk Level: Low (excellent test coverage provides safety net)
AI generated by Daily File Diet