[file-diet] File Diet: Split pkg/workflow/safe_outputs_handler_registry.go (1091 lines)

[github-actions]

Overview

The file pkg/workflow/safe_outputs_handler_registry.go has grown to 1091 lines, making it difficult to maintain and test. It centers on a single giant handlerRegistry map literal (spanning lines 57-1091) containing 41 handler builder closures, plus a small set of GitHub-token resolution helper functions at the top of the file. This task involves refactoring it into smaller, focused files grouped by functional domain, with dedicated test coverage.

Current State

• File: pkg/workflow/safe_outputs_handler_registry.go
• Size: 1091 lines
• Test Coverage: No test file found (safe_outputs_handler_registry_test.go does not exist)
• Complexity: One monolithic var handlerRegistry = map[string]handlerBuilder{...} literal contains 41 handler-builder closures (one per safe-output type), each independently reading its own cfg.<X> field and building a config map via the handlerConfigBuilder fluent API. There is no shared state between most closures beyond cfg and a handful of shared helpers (resolveHandlerGitHubToken, getEffectiveFooterForTemplatable, etc.), making the map trivially splittable by domain without touching call sites.

Full File Analysis

Detailed Breakdown

Helper functions (lines 1-56) - token resolution, kept as-is or moved to a small shared file:

• resolveHandlerGitHubToken (L16)
• resolveApproveWorkflowRunGitHubToken (L27)
• resolveHandlerGitHubTokenWithStepID (L37)
• handlerSupportsPerHandlerGitHubAppToken (L45)

handlerRegistry map entries (L57-1091), grouped by domain:

• Issues: create_issue (L58), close_issue (L135), update_issue (L380), link_sub_issue (L435), set_issue_type (L1048), set_issue_field (L1071)
• Discussions: create_discussion (L110), close_discussion (L155), update_discussion (L408)
• Labels/metadata: add_labels (L172), remove_labels (L199), replace_label (L217), assign_milestone (L264)
• Comments: add_comment (L88), hide_comment (L718), create_pull_request_review_comment (L464), reply_to_pull_request_review_comment (L500)
• Pull requests: create_pull_request (L533, largest single entry ~86 lines), push_to_pull_request_branch (L619), update_pull_request (L667), merge_pull_request (L687), close_pull_request (L702), mark_pull_request_as_ready_for_review (L280), add_reviewer (L248), dismiss_pull_request_review (L315), submit_pull_request_review (L482), resolve_pull_request_review_thread (L517)
• Workflow/CI: approve_workflow_run (L296), create_code_scanning_alert (L331), create_check_run (L345), dispatch_workflow (L733), dispatch_repository (L760), call_workflow (L780), autofix_code_scanning_alert (L948), upload_code_coverage (L937)
• Assets/artifacts: upload_asset (L890), upload_artifact (L904)
• Projects: create_project (L961), update_project (L980), create_project_status_update (L1036)
• Agents/assignment: create_agent_session (L366), assign_to_agent (L866), assign_to_user (L1000), unassign_from_user (L1018)
• Diagnostics/misc: missing_tool (L797), missing_data (L808), noop (L819), report_incomplete (L830), create_report_incomplete_issue (L841), update_release (L452)

No duplicate logic was found beyond the common newHandlerConfigBuilder()...Build() pattern, which is already correctly factored into the shared handlerConfigBuilder. The main complexity driver is sheer entry count in one map literal rather than algorithmic complexity.

Refactoring Strategy

Proposed File Splits

Split the single handlerRegistry map into several map[string]handlerBuilder partial maps defined in domain-specific files, merged into the final handlerRegistry via a small init/merge step (or by keeping one var handlerRegistry = mergeHandlerMaps(...) in the original file).

1. safe_outputs_handler_registry_issues.go

   • Functions: create_issue, close_issue, update_issue, link_sub_issue, set_issue_type, set_issue_field, add_labels, remove_labels, replace_label, assign_milestone
   • Responsibility: Issue lifecycle and metadata handler builders
   • Estimated LOC: ~330

2. safe_outputs_handler_registry_discussions.go

   • Functions: create_discussion, close_discussion, update_discussion
   • Responsibility: Discussion lifecycle handler builders
   • Estimated LOC: ~60

3. safe_outputs_handler_registry_pull_requests.go

   • Functions: create_pull_request, push_to_pull_request_branch, update_pull_request, merge_pull_request, close_pull_request, mark_pull_request_as_ready_for_review, add_reviewer, dismiss_pull_request_review, submit_pull_request_review, resolve_pull_request_review_thread, create_pull_request_review_comment, reply_to_pull_request_review_comment
   • Responsibility: Pull request lifecycle and review handler builders
   • Estimated LOC: ~350

4. safe_outputs_handler_registry_workflow.go

   • Functions: approve_workflow_run, create_code_scanning_alert, create_check_run, dispatch_workflow, dispatch_repository, call_workflow, autofix_code_scanning_alert, upload_code_coverage, upload_asset, upload_artifact
   • Responsibility: CI/workflow-triggering and artifact/coverage handler builders
   • Estimated LOC: ~180

5. safe_outputs_handler_registry_projects.go

   • Functions: create_project, update_project, create_project_status_update, assign_to_agent, assign_to_user, unassign_from_user, create_agent_session
   • Responsibility: Project board and agent-assignment handler builders
   • Estimated LOC: ~130

6. safe_outputs_handler_registry_misc.go

   • Functions: add_comment, hide_comment, update_release, missing_tool, missing_data, noop, report_incomplete, create_report_incomplete_issue
   • Responsibility: Comment and diagnostic/no-op handler builders
   • Estimated LOC: ~90

The base safe_outputs_handler_registry.go retains the token-resolution helpers (resolveHandlerGitHubToken, resolveApproveWorkflowRunGitHubToken, resolveHandlerGitHubTokenWithStepID, handlerSupportsPerHandlerGitHubAppToken) and a small combined var handlerRegistry = map[string]handlerBuilder{...} assembled from the domain maps (e.g. via maps.Copy from each partial map, or literal composition in an init()), keeping getSafeOutputHandlerByKey and other lookup call sites unchanged.

Shared Utilities

No new shared utility file is required; the existing handlerConfigBuilder already centralizes common map-building logic and should continue to be reused as-is by all split files.

Interface Abstractions

None needed - handlerBuilder is already a simple function type (func(cfg *SafeOutputsConfig) map[string]any); the split is purely file-organizational and does not require new interfaces.

Test Coverage Plan

Add dedicated test files, since none currently exist for this registry:

1. safe_outputs_handler_registry_issues_test.go

   • Test cases: each issue-domain handler builder returns nil when its config field is nil; returns expected map when populated (spot-check max, labels, github-token resolution via resolveHandlerGitHubToken with and without GitHubApp)
   • Target coverage: >80%

2. safe_outputs_handler_registry_pull_requests_test.go

   • Test cases: create_pull_request full option matrix (largest entry), push_to_pull_request_branch nil/non-nil, review-comment handlers with github-token resolution
   • Target coverage: >80%

3. safe_outputs_handler_registry_workflow_test.go

   • Test cases: approve_workflow_run token fallback behavior (must never silently use the default Actions token, per existing code comment), dispatch_workflow/dispatch_repository/call_workflow nil/non-nil paths
   • Target coverage: >80%

4. safe_outputs_handler_registry_projects_test.go and safe_outputs_handler_registry_misc_test.go

   • Test cases: nil/non-nil config paths for each remaining handler, footer/normalize_closing_keywords propagation where applicable
   • Target coverage: >80%

Also add/keep a small safe_outputs_handler_registry_test.go covering the retained helpers (resolveHandlerGitHubToken, resolveApproveWorkflowRunGitHubToken, handlerSupportsPerHandlerGitHubAppToken) and a sanity test that handlerRegistry contains exactly the expected 41 keys after the split (guards against accidental entry loss during the refactor).

Implementation Guidelines

1. Preserve Behavior: Ensure all existing functionality works identically
2. Maintain Exports: Keep public API unchanged (exported functions/types)
3. Add Tests First: Write tests for each new file before refactoring
4. Incremental Changes: Split one module at a time
5. Run Tests Frequently: Verify make test-unit passes after each split
6. Update Imports: Ensure all import paths are correct
7. Document Changes: Add comments explaining module boundaries

Acceptance Criteria

• Original file is split into 6 focused files (5 domain files + base helpers file)
• Each new file is under 500 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)

Additional Context

• Repository Guidelines: Follow patterns in .github/agents/developer.instructions.agent.md
• Code Organization: Prefer many small files grouped by functionality
• Testing: Match existing test patterns in pkg/workflow/*_test.go

---

Priority: Medium
Effort: Medium (mechanical map-literal split across 41 entries, no behavior change; new test files are the bulk of the work)
Expected Impact: Improved maintainability, easier testing, reduced complexity in pkg/workflow

Generated by 🧹 Daily File Diet · copilot · auto · 132.7 AIC · ⌖ 4.96 AIC · ⊞ 10.2K · ◷

• expires on Aug 26, 2026, 5:05 AM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · codex · mai10 · 2.66 AIC · ⌖ 1.5 AIC · ⊞ 17.8K · ◷