Skip to content

[refactor] Semantic function clustering: dispatch/call-workflow duplicates and permission factory consolidation #33901

@github-actions

Description

@github-actions

Overview

Semantic clustering of the 382 Go source files in pkg/workflow/ (plus 41 in pkg/parser/ and 302 in pkg/cli/) surfaces a small number of high-impact refactoring opportunities. Most of the package is well-organized — single-feature files, consistent naming clusters (compiler_*, safe_outputs_*, mcp_*, create_*/update_* entity actions) — but a few near-duplicate function families and an over-fragmented permissions factory stand out as worth consolidating.

The most impactful finding is a symmetric duplication between the dispatch-workflow and call-workflow safe-output families: four input-extraction helpers, two tool-generators, and two file-resolver loops are ~85–95% identical, differing only by trigger key (workflow_dispatch vs workflow_call).

Summary

  • Files analyzed: 825 Go source files (excluding _test.go)
  • Primary focus: pkg/workflow/ (382 files)
  • High-impact findings: 3
  • Medium-impact findings: 2
  • Outliers noted: 2

Critical Findings

Finding 1 — Workflow input extraction: 4 near-duplicate functions

Four functions extract inputs: blocks from a workflow definition. They share the same map-walking shape and differ only by the trigger key and source format (YAML vs. markdown frontmatter):

Function File Trigger Source
extractWorkflowDispatchInputs pkg/workflow/dispatch_workflow_validation.go:192 workflow_dispatch YAML
extractMDWorkflowDispatchInputs pkg/workflow/dispatch_workflow_file_resolver.go:120 workflow_dispatch .md frontmatter
extractWorkflowCallInputs pkg/workflow/call_workflow_validation.go:184 workflow_call YAML
extractMDWorkflowCallInputs pkg/workflow/call_workflow_validation.go:195 workflow_call .md frontmatter

The walk pattern (on<trigger>inputsmap[string]any) is repeated four times with identical type-assertion ladders.

Recommendation: Introduce a single helper that takes the parsed workflow map and the trigger key, plus a thin variant that reads markdown frontmatter:

func extractInputsFromWorkflow(workflow map[string]any, trigger string) map[string]any
func extractInputsFromMarkdown(mdPath, trigger string) (map[string]any, error)
func extractInputsFromYAML(workflowPath, trigger string) (map[string]any, error)

Keep the existing names as 3-line wrappers if call sites should stay descriptive, or migrate the 7 call sites in safe_outputs_dispatch.go, safe_outputs_call_workflow.go, and compiler_safe_output_jobs.go directly.

Impact: ~100 lines of duplicate map-walking code collapsed into one well-tested helper.

Finding 2 — MCP tool generators: 2 near-duplicate functions

  • generateDispatchWorkflowTool(workflowName, workflowInputs)pkg/workflow/safe_outputs_dispatch.go:108
  • generateCallWorkflowTool(workflowName, workflowInputs)pkg/workflow/safe_outputs_call_workflow.go:75

The two functions are ~95% identical. Differences:

  1. Description text ("Dispatch the '%s' workflow with workflow_dispatch trigger..." vs "Call the '%s' reusable workflow via workflow_call...").
  2. Internal metadata key (_workflow_name vs _call_workflow_name).
  3. Log labels.

Both call the shared buildInputSchema (already extracted into pkg/workflow/build_input_schema.go), so the only remaining duplication is the boilerplate around it.

Recommendation: Extract a generateWorkflowToolDefinition(opts workflowToolOpts) helper that takes the description format, metadata key, and log prefix as parameters. Both generators become 5-line callers.

Finding 3 — Workflow file populators: 2 near-duplicate functions

  • populateDispatchWorkflowFilespkg/workflow/safe_outputs_dispatch.go:29 (52 lines)
  • populateCallWorkflowFilespkg/workflow/safe_outputs_call_workflow.go:26 (43 lines)
Show duplicated extension-resolution block

Both functions iterate Workflows[], call findWorkflowFile, then run the same lockExists > ymlExists > mdExists ladder:

if fileResult.lockExists {
    extension = ".lock.yml"
} else if fileResult.ymlExists {
    extension = ".yml"
} else if fileResult.mdExists {
    // .md-only: same-batch compilation target
    extension = ".lock.yml"
} else {
    // warn and continue
}

The call-workflow variant additionally formats a relative path (./.github/workflows/<name><ext>); the dispatch-workflow variant additionally walks the file to detect aw_context. Otherwise the scaffolding is identical.

Recommendation: Extract resolveWorkflowExtension(fileResult) (string, bool) and have both populators consume it. The dispatch-specific aw_context probe and the call-specific relative-path formatting stay in their respective files as small post-processing steps.

Medium-Impact Findings

Finding 4 — permissions_factory.go has 16 narrow constructors

pkg/workflow/permissions_factory.go defines 16 NewPermissionsX() constructors enumerating every combination needed by callers:

Show enumerated constructors
  • NewPermissionsContentsRead
  • NewPermissionsContentsReadIssuesWrite
  • NewPermissionsContentsReadIssuesWritePRWrite
  • NewPermissionsContentsWrite
  • NewPermissionsContentsWritePRWrite
  • NewPermissionsContentsWriteIssuesWritePRWrite
  • NewPermissionsContentsReadDiscussionsWrite
  • NewPermissionsContentsReadIssuesWriteDiscussionsWrite
  • NewPermissionsContentsReadPRWrite
  • NewPermissionsContentsReadSecurityEventsWrite
  • NewPermissionsContentsReadSecurityEventsWriteActionsRead
  • NewPermissionsActionsWrite
  • plus NewPermissions, NewPermissionsReadAll, NewPermissionsWriteAll, NewPermissionsNone, NewPermissionsEmpty, NewPermissionsFromMap, NewPermissionsAllRead

Each combinatorial constructor is a 4-line wrapper around NewPermissionsFromMap(map[PermissionScope]PermissionLevel{...}).

Recommendation: Most callers can use NewPermissionsFromMap directly with an inline literal. The remaining recurring shapes (e.g. the contents-read+issues-write trio) can be expressed via a variadic builder:

func NewPermissionsWith(pairs ...PermissionPair) *Permissions
// usage: NewPermissionsWith(Read(Contents), Write(Issues), Write(PullRequests))

This collapses 12+ named constructors into one composable helper and removes the cognitive overhead of inventing a new NewPermissionsContentsRead<Foo>Write<Bar> name each time a new combination is needed.

Finding 5 — Validation files: 70+ files, several minimal

pkg/workflow/ contains 77 files with validate* functions, including several *_validation.go files under 1KB:

  • tools_validation.go (908 bytes, single function)
  • npm_validation_wasm.go (130 bytes, 1-line stub)
  • repository_features_validation_wasm.go (215 bytes, stub)
  • docker_validation_wasm.go (228 bytes, 3 tiny stubs)

The _wasm.go files are build-tag stubs and are correct as-is — they need to live in separate files for //go:build js || wasm exclusion.

However, the broader *_validation.go family (e.g. strict_mode_env_validation.go, strict_mode_network_validation.go, strict_mode_permissions_validation.go, strict_mode_sandbox_validation.go, strict_mode_steps_validation.go, strict_mode_update_check_validation.go, strict_mode_validation.go) might benefit from a sub-package pkg/workflow/strictmode/ — though this is a larger architectural change and should be weighed against the existing one-file-per-feature convention this repo follows.

Recommendation: Low priority. Only consolidate if the team is already planning a broader reorganization. The current naming convention makes files trivially discoverable via grep.

Outliers

Outlier 1 — Asymmetric placement of mdHas* helpers

  • mdHasWorkflowCall lives in pkg/workflow/call_workflow_validation.go:263
  • mdHasWorkflowDispatch lives in pkg/workflow/dispatch_workflow_file_resolver.go:100 (separate file from dispatch_workflow_validation.go)

Recommendation: When consolidating the duplicates in Findings 1–3, co-locate the mdHas* helpers symmetrically — either both inside *_validation.go files or both inside *_file_resolver.go files. Pick one.

Outlier 2 — Thin merge* wrappers in role_checks.go

role_checks.go:647-659 defines three near-identical wrappers around mergeStringSlicesDedup:

func (c *Compiler) mergeSkipRoles(...) []string { return mergeStringSlicesDedup(top, imported, "skip-roles") }
func (c *Compiler) mergeSkipBots(...)  []string { return mergeStringSlicesDedup(top, imported, "skip-bots")  }
func (c *Compiler) mergeBots(...)      []string { return mergeStringSlicesDedup(top, imported, "bots")       }

Each is a 1-line passthrough. They add no validation, no transformation, and no behavior on top of the shared helper.

Recommendation: Inline the three callers to use mergeStringSlicesDedup directly with the literal label. The wrappers add a hop without adding meaning.

Refactoring Priorities

Priority Item Estimated Effort Files Touched
P1 Findings 1–3 (dispatch/call duplication) 3–5 hours 5–7 files
P2 Finding 4 (permissions factory) 4–6 hours 1 file + many callers
P3 Outlier 1 (symmetric placement) 30 min 2 files
P3 Outlier 2 (thin merge wrappers) 30 min 1 file + 3 callers
P3 Finding 5 (validation fragmentation) Not recommended n/a

What's Working Well

Worth noting — the analysis surfaced several well-organized patterns that should be preserved:

  • Safe-output entity actions (create_issue.go, update_pull_request.go, etc.) follow one-file-per-feature and share helpers via create_entity_helpers.go, update_entity_helpers.go, close_entity_helpers.go.
  • Codemods (pkg/cli/codemod_*.go, 51 files) cleanly isolate each migration in its own file with a shared codemod_factory.go for common shapes.
  • Schedule parsing (pkg/parser/schedule_*.go) is split by responsibility (_parser, _cron_detection, _fuzzy_scatter, _time_utils) — clean separation.
  • Already-extracted helpers like buildInputSchema and RenderGitHubMCPRemoteConfig (the latter explicitly commented as extracted from Claude/Copilot duplication) show the team is actively consolidating duplication as it's noticed.

Analysis Metadata

  • Total Go files analyzed: 825 (pkg/**/*.go, excluding _test.go)
  • Primary focus: pkg/workflow/ (382 files)
  • Detection method: Serena LSP (Go gopls) + naming-pattern clustering + manual review of near-duplicate candidates
  • Analysis date: 2026-05-22
  • Workflow run: §26260313480

Generated by 🔧 Semantic Function Refactoring · ● 34.7M ·

  • expires on May 24, 2026, 12:13 AM UTC

Metadata

Metadata

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions