Overview
The file pkg/workflow/safe_outputs_generation.go has grown to 1549 lines, making it difficult to maintain, test, and understand. This file handles multiple distinct responsibilities: configuration generation, tool filtering, runner formatting, and workspace management. Splitting it into focused modules will improve maintainability and enable comprehensive testing.
Current State
- File:
pkg/workflow/safe_outputs_generation.go
- Size: 1549 lines
- Functions: 25+ exported and internal functions
- Test Coverage: No test file found (
safe_outputs_generation_test.go does not exist)
- Complexity: High - handles 4+ distinct domains with significant cross-coupling
Full File Analysis
Functional Breakdown
1. Configuration Generation (Lines 61-792)
generateSafeOutputsConfig() - Main config builder (494 lines)- Helper functions for various safe-output types:
generateMaxConfig(), generateMaxWithAllowedLabelsConfig(), generateMaxWithTargetConfig(), etc.
- Responsibility: Transform workflow safe-outputs into normalized JSON config objects
- Issue: Large main function with repetitive code patterns for different tool types
2. Tool Filtering & JSON Generation (Lines 1007-1285)
generateFilteredToolsJSON() - Massive function (278 lines)- Enumerates all safe-output types and filters tool definitions
- Issue: Highly repetitive with switch-like boolean checks for 30+ tool types
3. State Inspection Functions (Lines 793-950)
hasAnySafeOutputEnabled() - Check if any safe-output is enabledgetEnabledSafeOutputToolNamesReflection() - Get enabled tools via reflectionHasSafeOutputsEnabled() - Public version with loggingapplyDefaultCreateIssue() - Apply defaults- Responsibility: Query safe-outputs state
- Issue: Duplicated logic and inconsistent naming (public vs internal)
4. Compiler Methods & Utilities (Lines 852-975, 1286-1455)
formatSafeOutputsRunsOn() - Format runner environmentformatDetectionRunsOn() - Format detection job runnerusesPatchesAndCheckouts() - Check feature usageaddRepoParameterIfNeeded() - Tool-specific parameter handlinggenerateDispatchWorkflowTool() - Tool construction- Responsibility: Workspace/runner configuration and tool construction
- Issue: Mixed concerns; some are compiler methods, others are utilities
5. Validation Helpers (Lines 982-1006)
checkAllEnabledToolsPresent() - Validate tool availabilityGetEnabledSafeOutputToolNames() - Get enabled tools (reflection wrapper)- Responsibility: Validation and state inspection
- Issue: Thin wrappers; could be consolidated
6. Dispatch Workflow Support (Lines 17-59)
populateDispatchWorkflowFiles() - Map workflow files- Responsibility: Track dispatch workflow file extensions
- Issue: Small standalone function; could be grouped with related workflow logic
Key Issues
- Single Responsibility Violation: File mixes config generation, filtering, state inspection, validation, and utility functions
- Repetitive Code:
generateFilteredToolsJSON() has 30+ near-identical conditional blocks
- Test Debt: No test file exists despite complex logic
- Naming Inconsistencies: Mix of
Has*(), Get*(), format*(), generate*() patterns
- Hidden Coupling: Helper functions tightly coupled to
SafeOutputsConfig structure
Refactoring Strategy
Proposed File Splits
1. safe_outputs_config_generation.go (~450 lines)
- Focus: Main configuration generation
- Functions to move:
generateSafeOutputsConfig() - Main builderresolveMaxForConfig()generateMaxConfig(), generateMaxWithAllowedLabelsConfig(), etc. (all generate* helpers)generateTargetConfigWithRepos()
- Responsibility: Transform SafeOutputsConfig into normalized JSON config objects
- Estimated LOC: 450
2. safe_outputs_tools_filtering.go (~350 lines)
- Focus: Tool enumeration and filtering
- Functions to move:
generateFilteredToolsJSON() - Main filtering logicaddRepoParameterIfNeeded() - Tool parameter customizationgenerateDispatchWorkflowTool() - Tool construction helper- Helper for extracting tool names (extract from
generateFilteredToolsJSON)
- Responsibility: Filter tool definitions based on enabled safe-outputs
- Estimated LOC: 350
- Refactor Opportunity: Replace repetitive 30+ conditionals with a lookup table or generator pattern
3. safe_outputs_state.go (~250 lines)
- Focus: State inspection and validation
- Functions to move:
hasAnySafeOutputEnabled() - Check if any enabledHasSafeOutputsEnabled() - Public wrapper with logginghasNonBuiltinSafeOutputsEnabled() - Check for non-builtin toolsGetEnabledSafeOutputToolNames() - Get tool list (reflection-based)getEnabledSafeOutputToolNamesReflection() - Reflection helpercheckAllEnabledToolsPresent() - ValidationGetSafeOutputsToolsJSON() - Embed tools data
- Responsibility: Query, inspect, and validate safe-outputs state
- Estimated LOC: 250
4. safe_outputs_runtime.go (~150 lines)
- Focus: Runtime/workspace configuration
- Functions to move:
formatSafeOutputsRunsOn() - Runner formattingformatDetectionRunsOn() - Detection job runnerusesPatchesAndCheckouts() - Feature detection
- Responsibility: Determine runtime environment for safe-outputs jobs
- Estimated LOC: 150
- Note: Consider making these standalone functions (not compiler methods) if appropriate
5. dispatch_workflow.go (~100 lines)
- Focus: Dispatch workflow handling
- Functions to move:
populateDispatchWorkflowFiles() - Map workflow files- Helper to find workflow files (if needed)
- Responsibility: Track dispatch workflow files and extensions
- Estimated LOC: 100
Shared Utilities (to extract)
Consider creating a utility file for common patterns:
safe_outputs_util.go:
- Type conversion helpers (e.g.,
resolveMaxForConfig)
- JSON construction patterns
- Logger instance (currently
safeOutputsConfigLog)
Interface Abstractions
To reduce coupling, consider introducing interfaces:
-
SafeOutputStateReader - Read-only view of safe-outputs state
type SafeOutputStateReader interface {
IsEnabled(toolName string) bool
EnabledTools() []string
HasNonBuiltin() bool
}
-
ToolDefiner - Generate tool definitions
type ToolDefiner interface {
DefineTool(name string) (map[string]any, error)
FilterTools(enabled []string) ([]map[string]any, error)
}
Test Coverage Plan
New Test Files
-
safe_outputs_config_generation_test.go
- Test
generateSafeOutputsConfig() with various safe-output combinations
- Test each
generate* helper function
- Test edge cases: nil values, empty arrays, invalid combinations
- Target coverage: >85%
- Key scenarios:
- Single safe-output enabled (create_issue, add_comment, etc.)
- Multiple safe-outputs enabled (interactions and combinations)
- Config with max values and allowed fields
- Missing or invalid fields
- Config with target repositories and permissions
-
safe_outputs_tools_filtering_test.go
- Test
generateFilteredToolsJSON() with different enabled tool sets
- Test tool filtering accuracy (only enabled tools included)
- Test parameter injection (
addRepoParameterIfNeeded())
- Test dispatch workflow tool generation
- Target coverage: >80%
- Key scenarios:
- All tools enabled
- Subset of tools enabled
- No tools enabled (should return
[])
- Tool-specific parameter variations
- Dispatch workflow tool construction
-
safe_outputs_state_test.go
- Test state inspection functions
- Test
hasAnySafeOutputEnabled() with various configs
- Test
GetEnabledSafeOutputToolNames() accuracy
- Test validation with missing/invalid tools
- Target coverage: >85%
- Key scenarios:
- Empty safe-outputs config
- Single tool enabled
- Multiple tools enabled
- Builtin vs non-builtin tools
- Tool validation against available tools
-
safe_outputs_runtime_test.go
- Test runner/workspace formatting
- Test
formatSafeOutputsRunsOn() with various runner configurations
- Test feature detection (
usesPatchesAndCheckouts())
- Target coverage: >80%
- Key scenarios:
- No safe-outputs (what runner to use?)
- Single safe-output with specific runner needs
- Multiple safe-outputs with conflicting runner needs
- Patch and checkout feature combinations
-
dispatch_workflow_test.go
- Test workflow file discovery
- Test
populateDispatchWorkflowFiles() with various file combinations
- Test workflow file resolution (.lock.yml > .yml > .md priority)
- Target coverage: >85%
- Key scenarios:
- Workflow with .lock.yml file
- Workflow with only .md file
- Workflow with .yml file
- Multiple dispatch workflows
- Missing workflow files
Integration Tests
Add integration tests in existing test files to verify:
- Config generation → tools filtering pipeline works correctly
- State inspection returns accurate tool names
- Generated runners match expected GitHub Actions format
- Full workflow compilation with safe-outputs works end-to-end
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically after refactoring
- Maintain Exports: Keep public API unchanged (exported functions with capital letters)
- Add Tests First: Write tests for each new file before moving code
- Incremental Changes: Move one module at a time, test after each split
- Run Tests Frequently: Execute
make test-unit after each file split
- Update Imports: Ensure all internal and external imports are correct
- Document Changes: Add comments explaining module boundaries and responsibilities
- Consider Reflection Usage: The
getEnabledSafeOutputToolNamesReflection() function is clever but expensive; document why it's needed and consider caching strategies
Acceptance Criteria
Additional Context
Related Files
safe_outputs_jobs.go (1031 lines) - Might be next candidate for refactoringsafe_outputs_config.go (1055 lines) - Already has dedicated focus- Test patterns: Review
pkg/workflow/compiler_*_test.go for testing conventions
Code Organization Philosophy
Per repository guidelines, prefer many small focused files grouped by functionality. This refactoring aligns with that philosophy by:
- Creating single-responsibility modules
- Improving testability
- Reducing cognitive load per file
- Making navigation and changes easier
Estimated Effort
- Difficulty: Medium (complex logic but clear boundaries)
- Time: ~4-6 hours for experienced contributor
- 1-2 hours: File splitting and import updates
- 1-2 hours: Test writing
- 1-2 hours: Validation and refinement
Priority: Medium
Complexity: Medium - clear module boundaries but requires careful testing
Expected Impact: Significantly improved maintainability, comprehensive test coverage, easier onboarding for new contributors
Generated by Daily File Diet · ◷
Overview
The file
pkg/workflow/safe_outputs_generation.gohas grown to 1549 lines, making it difficult to maintain, test, and understand. This file handles multiple distinct responsibilities: configuration generation, tool filtering, runner formatting, and workspace management. Splitting it into focused modules will improve maintainability and enable comprehensive testing.Current State
pkg/workflow/safe_outputs_generation.gosafe_outputs_generation_test.godoes not exist)Full File Analysis
Functional Breakdown
1. Configuration Generation (Lines 61-792)
generateSafeOutputsConfig()- Main config builder (494 lines)generateMaxConfig(),generateMaxWithAllowedLabelsConfig(),generateMaxWithTargetConfig(), etc.2. Tool Filtering & JSON Generation (Lines 1007-1285)
generateFilteredToolsJSON()- Massive function (278 lines)3. State Inspection Functions (Lines 793-950)
hasAnySafeOutputEnabled()- Check if any safe-output is enabledgetEnabledSafeOutputToolNamesReflection()- Get enabled tools via reflectionHasSafeOutputsEnabled()- Public version with loggingapplyDefaultCreateIssue()- Apply defaults4. Compiler Methods & Utilities (Lines 852-975, 1286-1455)
formatSafeOutputsRunsOn()- Format runner environmentformatDetectionRunsOn()- Format detection job runnerusesPatchesAndCheckouts()- Check feature usageaddRepoParameterIfNeeded()- Tool-specific parameter handlinggenerateDispatchWorkflowTool()- Tool construction5. Validation Helpers (Lines 982-1006)
checkAllEnabledToolsPresent()- Validate tool availabilityGetEnabledSafeOutputToolNames()- Get enabled tools (reflection wrapper)6. Dispatch Workflow Support (Lines 17-59)
populateDispatchWorkflowFiles()- Map workflow filesKey Issues
generateFilteredToolsJSON()has 30+ near-identical conditional blocksHas*(),Get*(),format*(),generate*()patternsSafeOutputsConfigstructureRefactoring Strategy
Proposed File Splits
1.
safe_outputs_config_generation.go(~450 lines)generateSafeOutputsConfig()- Main builderresolveMaxForConfig()generateMaxConfig(),generateMaxWithAllowedLabelsConfig(), etc. (allgenerate*helpers)generateTargetConfigWithRepos()2.
safe_outputs_tools_filtering.go(~350 lines)generateFilteredToolsJSON()- Main filtering logicaddRepoParameterIfNeeded()- Tool parameter customizationgenerateDispatchWorkflowTool()- Tool construction helpergenerateFilteredToolsJSON)3.
safe_outputs_state.go(~250 lines)hasAnySafeOutputEnabled()- Check if any enabledHasSafeOutputsEnabled()- Public wrapper with logginghasNonBuiltinSafeOutputsEnabled()- Check for non-builtin toolsGetEnabledSafeOutputToolNames()- Get tool list (reflection-based)getEnabledSafeOutputToolNamesReflection()- Reflection helpercheckAllEnabledToolsPresent()- ValidationGetSafeOutputsToolsJSON()- Embed tools data4.
safe_outputs_runtime.go(~150 lines)formatSafeOutputsRunsOn()- Runner formattingformatDetectionRunsOn()- Detection job runnerusesPatchesAndCheckouts()- Feature detection5.
dispatch_workflow.go(~100 lines)populateDispatchWorkflowFiles()- Map workflow filesShared Utilities (to extract)
Consider creating a utility file for common patterns:
safe_outputs_util.go:resolveMaxForConfig)safeOutputsConfigLog)Interface Abstractions
To reduce coupling, consider introducing interfaces:
SafeOutputStateReader- Read-only view of safe-outputs stateToolDefiner- Generate tool definitionsTest Coverage Plan
New Test Files
safe_outputs_config_generation_test.gogenerateSafeOutputsConfig()with various safe-output combinationsgenerate*helper functionsafe_outputs_tools_filtering_test.gogenerateFilteredToolsJSON()with different enabled tool setsaddRepoParameterIfNeeded())[])safe_outputs_state_test.gohasAnySafeOutputEnabled()with various configsGetEnabledSafeOutputToolNames()accuracysafe_outputs_runtime_test.goformatSafeOutputsRunsOn()with various runner configurationsusesPatchesAndCheckouts())dispatch_workflow_test.gopopulateDispatchWorkflowFiles()with various file combinationsIntegration Tests
Add integration tests in existing test files to verify:
Implementation Guidelines
make test-unitafter each file splitgetEnabledSafeOutputToolNamesReflection()function is clever but expensive; document why it's needed and consider caching strategiesAcceptance Criteria
make test-unit)make lint)make build)Additional Context
Related Files
safe_outputs_jobs.go(1031 lines) - Might be next candidate for refactoringsafe_outputs_config.go(1055 lines) - Already has dedicated focuspkg/workflow/compiler_*_test.gofor testing conventionsCode Organization Philosophy
Per repository guidelines, prefer many small focused files grouped by functionality. This refactoring aligns with that philosophy by:
Estimated Effort
Priority: Medium
Complexity: Medium - clear module boundaries but requires careful testing
Expected Impact: Significantly improved maintainability, comprehensive test coverage, easier onboarding for new contributors