-
Notifications
You must be signed in to change notification settings - Fork 35
Description
Overview
The file pkg/workflow/safe_outputs_config.go has grown to 1,169 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/workflow/safe_outputs_config.go - Size: 1,169 lines
- Test Coverage: ❌ No test file found (
safe_outputs_config_test.godoes not exist) - Complexity: High - Contains 10 main functions handling multiple responsibilities:
- Configuration extraction and parsing (~330 lines)
- Tool name enumeration and checking (~140 lines)
- Messages/mentions configuration (~190 lines)
- JSON generation for safe outputs config (~380 lines)
- Filtered tools JSON generation (~135 lines)
Context: Campaign Progress
Current Status: 19 files exceed the 800-line healthy threshold
The codebase has significant file size issues that need systematic refactoring to improve maintainability.
Refactoring Strategy
Proposed File Splits
Based on semantic analysis, split the file into the following modules:
1. safe_outputs_enabled.go
- Functions:
HasSafeOutputsEnabled()GetEnabledSafeOutputToolNames()formatSafeOutputsRunsOn()
- Responsibility: Tool availability checking and enumeration
- Estimated LOC: ~150 lines
- Rationale: These functions are purely about querying which tools are enabled
2. safe_outputs_extraction.go
- Functions:
extractSafeOutputsConfig()(main orchestrator, lines 174-479)
- Responsibility: Frontmatter parsing and config extraction
- Estimated LOC: ~330 lines
- Rationale: Single large function that deserves its own file for focused testing
3. safe_outputs_messages.go
- Functions:
parseMessagesConfig()parseMentionsConfig()serializeMessagesConfig()normalizeSafeOutputIdentifier()
- Responsibility: Message and mention configuration handling
- Estimated LOC: ~190 lines
- Rationale: Related to user-facing messages and mentions escaping
4. safe_outputs_generation.go
- Functions:
generateSafeOutputsConfig()(lines 656-1033)generateFilteredToolsJSON()(lines 1034-1169)
- Responsibility: JSON configuration generation for workflow compilation
- Estimated LOC: ~515 lines
- Rationale: Both functions generate JSON output for different purposes
Shared Utilities
No additional utility extraction needed - the normalization function fits naturally in the messages module.
Interface Abstractions
Consider introducing a ConfigParser interface if more config sources are added in the future:
// ConfigParser handles parsing of configuration from different sources
type ConfigParser interface {
ParseMessages(map[string]any) (*SafeOutputMessagesConfig, error)
ParseMentions(any) (*MentionsConfig, error)
}This would enable easier testing with mock parsers.
Test Coverage Plan
Add comprehensive tests for each new file:
1. safe_outputs_enabled_test.go
- Test cases:
HasSafeOutputsEnabled()with various configurations (nil, empty, single tool, multiple tools)GetEnabledSafeOutputToolNames()returns correct tool names and sorts themformatSafeOutputsRunsOn()handles nil config and custom runners
- Target coverage: >80%
2. safe_outputs_extraction_test.go
- Test cases:
- Valid frontmatter extraction for all safe-output types
- Invalid/malformed frontmatter handling
- Edge cases: empty maps, wrong types, missing fields
- Integration with parseMessagesConfig and parseMentionsConfig
- Target coverage: >80%
3. safe_outputs_messages_test.go
- Test cases:
parseMessagesConfig()with all message typesparseMentionsConfig()with boolean and object configurationsserializeMessagesConfig()round-trip (parse → serialize → parse)normalizeSafeOutputIdentifier()converts dashes to underscores- Edge cases: nil inputs, max value truncation, invalid types
- Target coverage: >80%
4. safe_outputs_generation_test.go
- Test cases:
generateSafeOutputsConfig()produces valid JSON for all tool types- Default max values are applied correctly
- Allowed labels/targets are preserved
generateFilteredToolsJSON()filters tools correctly- Edge cases: nil config, empty tools, large configurations
- Target coverage: >80%
Implementation Guidelines
-
Preserve Behavior: Ensure all existing functionality works identically
- The
extractSafeOutputsConfigfunction is central to compilation - any changes must be validated
- The
-
Maintain Exports: Keep public API unchanged (exported functions/types)
HasSafeOutputsEnabledandGetEnabledSafeOutputToolNamesare used by other packages
-
Add Tests First: Write tests for each new file before refactoring
- Create test files with placeholder tests for current behavior
- Refactor incrementally while keeping tests green
-
Incremental Changes: Split one module at a time
- Start with
safe_outputs_enabled.go(smallest, fewest dependencies) - Then
safe_outputs_messages.go - Then
safe_outputs_extraction.go - Finally
safe_outputs_generation.go
- Start with
-
Run Tests Frequently: Verify
make test-unitpasses after each split -
Update Imports: Ensure all import paths are correct
- Functions moved between files in the same package require no import changes
- Verify no circular dependencies are introduced
-
Document Changes: Add comments explaining module boundaries
- Add file-level comments describing each module's purpose
Acceptance Criteria
- Original file is split into 4 focused files
- Each new file is under 600 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) -
make recompilesucceeds (all workflow files recompile correctly)
Additional Context
- Repository Guidelines: Follow patterns in
.github/instructions/developer.instructions.md - Code Organization: Prefer many small files grouped by functionality (see developer instructions)
- Testing: Match existing test patterns in
pkg/workflow/*_test.go - Logger: Maintain the existing
safeOutputsConfigLoglogger in each file that needs it
Estimated Effort
Medium - The file has clear functional boundaries, but the extraction function is complex and requires careful testing to ensure no regression in workflow compilation behavior.
Expected Impact
- ✅ Improved maintainability - Each module has a single, clear responsibility
- ✅ Easier testing - Smaller files with focused functionality are easier to test thoroughly
- ✅ Reduced complexity - Developers can understand one module without reading 1,169 lines
- ✅ Better navigation - Related functionality is grouped in dedicated files
- ✅ Foundation for future features - New safe-output types can be added more easily
AI generated by Daily File Diet