-
Notifications
You must be signed in to change notification settings - Fork 217
Description
Overview
The file pkg/workflow/safe_outputs_config_generation.go has grown to 1023 lines, making it the largest source file in the codebase and 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_generation.go - Size: 1023 lines
- Test Coverage: No test file found (requires comprehensive test coverage)
- Function Count: 6 primary functions
- Complexity: High - 84 conditional checks on
data.SafeOutputs.*fields
Full File Analysis
Detailed Breakdown
The file contains three distinct functional domains:
1. Main Configuration Generation (lines 58-445)
generateSafeOutputsConfig()- 387 lines- Massive function with 84 conditional branches checking SafeOutputs fields
- Repetitive pattern: check if field exists → call helper → add to config map
- Clear candidates for extraction: each safe-output type could be a separate handler
2. Tool Definition Generation (lines 449-535)
generateCustomJobToolDefinition()- 87 lines- Converts SafeJobConfig into MCP tool definitions
- Self-contained logic for custom job schema generation
- Input type mapping (choice → string with enum, boolean, number, string)
3. Filtered Tools and Repo Parameter Handling (lines 539-926)
generateFilteredToolsJSON()- 242 lines- Loads all tools, filters based on enabled SafeOutputs
- Adds custom job tools (sorted for deterministic output)
- Generates dispatch_workflow tools dynamically
- Heavy function with multiple responsibilities
addRepoParameterIfNeeded()- 147 lines- Massive switch statement handling repo parameter injection
- Repetitive pattern for each tool type
- Could benefit from data-driven approach
4. Dispatch Workflow Tool Generation (lines 930-1023)
generateDispatchWorkflowTool()- 94 lines- Converts GitHub Actions workflow_dispatch inputs to MCP tool schema
- Input type mapping similar to custom job tools
Helper Functions Already Extracted:
safe_outputs_config_generation_helpers.go- ContainsgenerateMaxConfig,generatePullRequestConfig, etc.tool_description_enhancer.go- ContainsenhanceToolDescription
Complexity Hotspots
-
generateSafeOutputsConfig()(lines 58-445):- 84 conditional branches checking different SafeOutputs fields
- Highly repetitive pattern that could be data-driven
- Lines 69-340: Create/update/close operations
- Lines 342-379: Custom jobs handling
- Lines 381-441: Mentions and dispatch-workflow configuration
-
generateFilteredToolsJSON()(lines 539-777):- Multiple responsibilities: filtering, enhancing, custom jobs, dispatch workflows
- Lines 560-656: Building enabledTools map (97 lines of repetitive checks)
- Lines 685-707: Custom job tools generation
- Lines 713-765: Dispatch workflow tools generation
-
addRepoParameterIfNeeded()(lines 781-926):- Massive switch statement with repetitive patterns
- Each case checks config for hasAllowedRepos and targetRepoSlug
- Lines 845-895: Nested switch for SafeOutputTargetConfig tools
Duplicate/Similar Patterns
-
Config generation pattern (repeated 30+ times):
if data.SafeOutputs.FieldName != nil { safeOutputsConfig["tool_name"] = generateHelper(...) }
-
Tool enablement pattern (repeated 30+ times):
if data.SafeOutputs.FieldName != nil { enabledTools["tool_name"] = true }
-
Repo parameter check pattern (repeated 15+ times):
if config := safeOutputs.FieldName; config != nil { hasAllowedRepos = len(config.AllowedRepos) > 0 targetRepoSlug = config.TargetRepoSlug }
Refactoring Strategy
Proposed File Splits
Based on functional domains and responsibilities, split the file into the following modules:
1. safe_outputs_config_builder.go (~200 lines)
- Purpose: Main configuration generation orchestrator
- Functions:
generateSafeOutputsConfig()- Refactored to use registry patternbuildConfigForTool()- Helper to build config for a single tool
- Responsibility: Coordinate config generation using tool registry
- Estimated LOC: ~200 lines (simplified from current 387-line function)
2. safe_outputs_tool_registry.go (~150 lines)
- Purpose: Data-driven tool configuration registry
- Types:
SafeOutputToolHandler- Interface for tool config generationtoolRegistry- Map of tool names to handlers
- Functions:
registerTools()- Initialize registry with all tool handlers- Per-tool handler functions (small, focused)
- Responsibility: Eliminate repetitive conditional logic with registry pattern
- Estimated LOC: ~150 lines
3. safe_outputs_tool_definitions.go (~200 lines)
- Purpose: MCP tool definition generation
- Functions:
generateCustomJobToolDefinition()- Move from main filegenerateDispatchWorkflowTool()- Move from main filebuildInputSchema()- Extract common schema building logic
- Responsibility: Create MCP tool definitions from job/workflow configs
- Estimated LOC: ~200 lines
4. safe_outputs_tool_filter.go (~250 lines)
- Purpose: Tool filtering and enhancement
- Functions:
generateFilteredToolsJSON()- Simplified versionbuildEnabledToolsSet()- Extract tool enablement logicaddCustomJobTools()- Extract custom job handlingaddDispatchWorkflowTools()- Extract dispatch workflow handling
- Responsibility: Filter and enhance tools based on configuration
- Estimated LOC: ~250 lines (split from current 242-line function)
5. safe_outputs_repo_parameters.go (~150 lines)
- Purpose: Repo parameter injection for cross-repo operations
- Functions:
addRepoParameterIfNeeded()- Refactored with data-driven approachgetRepoConfigForTool()- Extract common repo config logic
- Responsibility: Add repo parameters to tools supporting cross-repo operations
- Estimated LOC: ~150 lines (simplified from current 147 lines)
6. safe_outputs_dispatch_workflows.go (~100 lines)
- Purpose: Dispatch workflow file handling
- Functions:
populateDispatchWorkflowFiles()- Move from main file- Helper functions for workflow file discovery
- Responsibility: Manage workflow file mapping for dispatch operations
- Estimated LOC: ~100 lines
Shared Utilities
The following files already exist and provide shared utilities:
safe_outputs_config_generation_helpers.go: Config helper functions (generateMaxConfig, etc.)tool_description_enhancer.go: Tool description enhancement logic
Test Coverage Plan
Add comprehensive tests for each new file:
1. safe_outputs_config_builder_test.go
- Test cases: Basic config generation, empty SafeOutputs, custom jobs, dispatch workflows
- Target coverage: >80%
- Focus: Integration tests verifying orchestration logic
2. safe_outputs_tool_registry_test.go
- Test cases: Registry initialization, tool handler lookup, per-tool config generation
- Target coverage: >85%
- Focus: Unit tests for each tool handler
3. safe_outputs_tool_definitions_test.go
- Test cases: Custom job tool generation, dispatch workflow tool generation, input schema building
- Target coverage: >80%
- Focus: Schema validation, input type mapping
4. safe_outputs_tool_filter_test.go
- Test cases: Tool filtering, enablement logic, custom job addition, dispatch workflow addition
- Target coverage: >80%
- Focus: Tool filtering correctness, deterministic ordering
5. safe_outputs_repo_parameters_test.go
- Test cases: Repo parameter injection, allowed-repos handling, target-repo defaults
- Target coverage: >80%
- Focus: Parameter injection for each tool type
6. safe_outputs_dispatch_workflows_test.go
- Test cases: Workflow file discovery, file extension priority (.lock.yml > .yml)
- Target coverage: >75%
- Focus: File mapping correctness
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically
- Maintain Exports: Keep public API unchanged (
generateSafeOutputsConfig,generateFilteredToolsJSONremain exported) - Add Tests First: Write tests for extracted modules before refactoring
- Incremental Changes: Split one module at a time, verify tests pass
- Run Tests Frequently: Verify
make test-unitpasses after each split - Update Imports: Ensure all import paths reference the new modules correctly
- Document Changes: Add comments explaining module boundaries and responsibilities
- Use Registry Pattern: Replace repetitive conditionals with data-driven registry
Acceptance Criteria
- Original file is split into 6 focused files
- Each new file is under 300 lines (target: 100-200 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) - Code passes formatting (
make fmt) - Build succeeds (
make build) - Registry pattern eliminates 80+ conditional branches
-
generateSafeOutputsConfigandgenerateFilteredToolsJSONremain exported with identical signatures
Additional Context
Architecture Benefits
After refactoring:
- Maintainability: Each file has a single, clear responsibility
- Testability: Smaller functions are easier to test in isolation
- Extensibility: Adding new safe-output tools requires minimal changes (add to registry)
- Readability: Code organization matches mental model of the system
Migration Path
- Create new files with extracted functions (keep original intact)
- Update original file to delegate to new modules
- Add comprehensive tests for new modules
- Verify all tests pass
- Remove deprecated code from original file (if any)
- Run full test suite and linting
Repository Guidelines
- Follow patterns in
AGENTS.mdandscratchpad/go-type-patterns.md - Use console formatting for debug logging (pkg/logger)
- Prefer many small files grouped by functionality
- Code organization: Validation (80-300 LOC per file)
Priority: Medium
Effort: Large (1023-line file with complex logic, requires careful extraction)
Expected Impact: Significantly improved maintainability, easier testing, reduced complexity, better code organization
AI generated by Daily File Diet
- expires on Feb 5, 2026, 1:33 PM UTC