Overview
The file pkg/workflow/safe_outputs_config.go has grown to 1,254 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,254 lines
- Direct test file: None (coverage comes from
compiler_safe_outputs_config_test.go at 3,523 lines — excellent indirect coverage)
- Test-to-source ratio: ~2.8× (indirect)
- Functions: 8 functions;
extractSafeOutputsConfig spans ~760 lines alone
Full File Analysis
Section Breakdown
| Lines |
Content |
| 1–168 |
Type Definitions — 6 struct types: BaseSafeOutputConfig, SafeOutputsConfig, SafeOutputMessagesConfig, MentionsConfig, SecretMaskingConfig, SafeOutputStepConfig |
| 169–210 |
Architecture comment block for schema generation |
| 211–970 |
extractSafeOutputsConfig() — monolithic method (~760 lines) that parses all 40+ output-handler types plus 20+ global config fields inline |
| 972–1070 |
parseBaseSafeOutputConfig() and parseSamplesValue() — common base field parsing |
| 1090–1254 |
Runtime config builders — addHandlerManagerConfigEnvVar, buildMentionsHandlerConfig, dispatch copy helpers, getEngineAgentFileInfo |
Complexity Hotspots
extractSafeOutputsConfig (line 211): ~760 lines, handles 40+ handler types and 20+ global fields (allowed-domains, staged, env, github-token, max-patch-size, max-patch-files, threat-detection, runs-on, timeout-minutes, messages, mentions, footer, group-reports, report-failure-as-issue, failure-issue-repo, max-bot-mentions, steps, id-token, concurrency-group, needs, environment, jobs, scripts, actions, github-app).
- Integer parsing for
max-patch-size, max-patch-files, and timeout-minutes duplicates the same type-switch pattern three times in the same function.
addHandlerManagerConfigEnvVar (line 1090): ~90 lines, builds runtime env config — logically unrelated to YAML extraction.
Duplicate Patterns
The file contains three near-identical type-switch blocks (int/int64/uint64/float64) for parsing integer YAML fields — all within extractSafeOutputsConfig.
Refactoring Strategy
Proposed File Splits
-
safe_outputs_config_types.go
- Types:
BaseSafeOutputConfig, SafeOutputsConfig, SafeOutputMessagesConfig, MentionsConfig, SecretMaskingConfig, SafeOutputStepConfig
- Logger variable:
safeOutputsConfigLog
- Architecture doc comment
- Estimated LOC: ~170
-
safe_outputs_config_extraction.go
- Functions:
(*Compiler).extractSafeOutputsConfig, (*Compiler).parseBaseSafeOutputConfig, parseSamplesValue
- Responsibility: YAML frontmatter → structured Go config
- Estimated LOC: ~580 (after extracting global-field parsing into helpers)
-
safe_outputs_config_global.go
- New helper:
extractGlobalConfigFields() (or individual extractPatchConfig, extractTimeoutConfig, extractEnvConfig helpers)
- Deduplicates the three repeated integer type-switch blocks in
extractSafeOutputsConfig
- Responsibility: Parsing global safe-outputs config knobs (non-handler fields)
- Estimated LOC: ~200
-
safe_outputs_config_runtime.go
- Functions:
(*Compiler).addHandlerManagerConfigEnvVar, buildMentionsHandlerConfig, safeOutputsWithDispatchTargetRepo, safeOutputsWithDispatchTargetRef, (*Compiler).getEngineAgentFileInfo
- Responsibility: Runtime handler-manager config building and JSON serialization
- Estimated LOC: ~175
Shared Utilities
Extract the repeated integer type-switch (int/int64/uint64/float64 → bounded int) into:
safe_outputs_config_global.go: parseBoundedIntField(configMap, key, log) — returns (int, bool)
Interface Abstractions
No new interfaces required; the existing AgentFileProvider interface is already used by getEngineAgentFileInfo.
Test Coverage Plan
The existing test coverage is already high. After splitting:
-
safe_outputs_config_types_test.go (optional)
- Verify YAML tag round-trips for
SafeOutputsConfig and BaseSafeOutputConfig
- Estimated: ~50 lines
-
safe_outputs_config_global_test.go
parseBoundedIntField: integer, int64, uint64, float64, NaN/Inf, truncation, out-of-range
extractGlobalConfigFields: staged, env, github-token, allowed-domains, footer, group-reports
- Target coverage: >80%
-
Existing tests stay valid — compiler_safe_outputs_config_test.go (3,523 lines) provides comprehensive end-to-end coverage and should pass unchanged after a pure structural split.
Implementation Guidelines
- Preserve Behavior: Ensure all existing functionality works identically
- Maintain Exports: Keep public API unchanged (exported types and functions)
- Deduplicate First: Extract the repeated integer type-switch into
parseBoundedIntField before splitting files
- Incremental Changes: Split one module at a time — types → global helpers → runtime
- Run Tests Frequently: Verify
make test-unit passes after each split
- Update Imports: Ensure all import paths are correct (all files stay in
package workflow)
- Document Changes: Add comments explaining module boundaries
Acceptance Criteria
Additional Context
- Repository Guidelines: Follow patterns in
.github/agents/developer.instructions.agent.md
- Code Organization: Prefer many small files grouped by functionality (see
pkg/workflow/ — hundreds of focused files)
- Testing: Match existing test patterns in
pkg/workflow/*_test.go
- Companion files already split:
safe_outputs_config_helpers.go (61 lines), safe_outputs_config_generation.go (302 lines), safe_outputs_parser.go (90 lines) — this file is the remaining oversized piece
Priority: Medium
Effort: Large (40+ handler types, 8 functions, requires careful deduplication)
Expected Impact: Improved maintainability, easier navigation, deduplicated integer parsing logic
Generated by 🧹 Daily File Diet · 148.6 AIC · ⌖ 17.4 AIC · ⊞ 6.8K · ◷
Overview
The file
pkg/workflow/safe_outputs_config.gohas grown to 1,254 lines, making it difficult to maintain and test. This task involves refactoring it into smaller, focused files with improved test coverage.Current State
pkg/workflow/safe_outputs_config.gocompiler_safe_outputs_config_test.goat 3,523 lines — excellent indirect coverage)extractSafeOutputsConfigspans ~760 lines aloneFull File Analysis
Section Breakdown
BaseSafeOutputConfig,SafeOutputsConfig,SafeOutputMessagesConfig,MentionsConfig,SecretMaskingConfig,SafeOutputStepConfigextractSafeOutputsConfig()— monolithic method (~760 lines) that parses all 40+ output-handler types plus 20+ global config fields inlineparseBaseSafeOutputConfig()andparseSamplesValue()— common base field parsingaddHandlerManagerConfigEnvVar,buildMentionsHandlerConfig, dispatch copy helpers,getEngineAgentFileInfoComplexity Hotspots
extractSafeOutputsConfig(line 211): ~760 lines, handles 40+ handler types and 20+ global fields (allowed-domains, staged, env, github-token, max-patch-size, max-patch-files, threat-detection, runs-on, timeout-minutes, messages, mentions, footer, group-reports, report-failure-as-issue, failure-issue-repo, max-bot-mentions, steps, id-token, concurrency-group, needs, environment, jobs, scripts, actions, github-app).max-patch-size,max-patch-files, andtimeout-minutesduplicates the same type-switch pattern three times in the same function.addHandlerManagerConfigEnvVar(line 1090): ~90 lines, builds runtime env config — logically unrelated to YAML extraction.Duplicate Patterns
The file contains three near-identical type-switch blocks (int/int64/uint64/float64) for parsing integer YAML fields — all within
extractSafeOutputsConfig.Refactoring Strategy
Proposed File Splits
safe_outputs_config_types.goBaseSafeOutputConfig,SafeOutputsConfig,SafeOutputMessagesConfig,MentionsConfig,SecretMaskingConfig,SafeOutputStepConfigsafeOutputsConfigLogsafe_outputs_config_extraction.go(*Compiler).extractSafeOutputsConfig,(*Compiler).parseBaseSafeOutputConfig,parseSamplesValuesafe_outputs_config_global.goextractGlobalConfigFields()(or individualextractPatchConfig,extractTimeoutConfig,extractEnvConfighelpers)extractSafeOutputsConfigsafe_outputs_config_runtime.go(*Compiler).addHandlerManagerConfigEnvVar,buildMentionsHandlerConfig,safeOutputsWithDispatchTargetRepo,safeOutputsWithDispatchTargetRef,(*Compiler).getEngineAgentFileInfoShared Utilities
Extract the repeated integer type-switch (int/int64/uint64/float64 → bounded int) into:
safe_outputs_config_global.go:parseBoundedIntField(configMap, key, log)— returns(int, bool)Interface Abstractions
No new interfaces required; the existing
AgentFileProviderinterface is already used bygetEngineAgentFileInfo.Test Coverage Plan
The existing test coverage is already high. After splitting:
safe_outputs_config_types_test.go(optional)SafeOutputsConfigandBaseSafeOutputConfigsafe_outputs_config_global_test.goparseBoundedIntField: integer, int64, uint64, float64, NaN/Inf, truncation, out-of-rangeextractGlobalConfigFields: staged, env, github-token, allowed-domains, footer, group-reportsExisting tests stay valid —
compiler_safe_outputs_config_test.go(3,523 lines) provides comprehensive end-to-end coverage and should pass unchanged after a pure structural split.Implementation Guidelines
parseBoundedIntFieldbefore splitting filesmake test-unitpasses after each splitpackage workflow)Acceptance Criteria
safe_outputs_config.gois removed/split into focused filesmake test-unitpassesmake lintpassesmake buildsucceedsAdditional Context
.github/agents/developer.instructions.agent.mdpkg/workflow/— hundreds of focused files)pkg/workflow/*_test.gosafe_outputs_config_helpers.go(61 lines),safe_outputs_config_generation.go(302 lines),safe_outputs_parser.go(90 lines) — this file is the remaining oversized piecePriority: Medium
Effort: Large (40+ handler types, 8 functions, requires careful deduplication)
Expected Impact: Improved maintainability, easier navigation, deduplicated integer parsing logic