[refactor] Semantic clustering: token-resolver duplicates, dead WASM stubs, scattered preprocessors

[github-actions]

Overview

Semantic clustering analysis of 387 Go source files in pkg/workflow/ and 303 in pkg/cli/ (834 total non-test .go files across the repo) surfaced five high-signal refactoring opportunities. The codebase is generally well-organized — most "wrong file" concerns turn out to be intentional (e.g. WASM stubs paired with build-tagged production files, registry-style entity helpers, parser specs as self-contained modules). The findings below are the cases where consolidation, relocation, or deletion would meaningfully improve clarity.

Summary

#	Finding	Type	Files	Effort
1	Three near-identical addSafeOutput*GitHubTokenForConfig methods	Duplicate	safe_outputs_env.go	Low
2	Dead WASM stubs RunGit / GetCurrentGitTag with no callers	Dead code	git_helpers_wasm.go, git_helpers.go	Trivial
3	validateRunsOnValue lives in frontmatter_parsing.go, not runs_on_validation.go	Outlier	frontmatter_parsing.go, runs_on_validation.go	Low
4	Field-preprocessing helpers scattered across 3 files	Scattered helpers	templatables.go, parse_helpers.go, config_helpers.go	Low
5	Local-variable name shadows package-level validateRunsOnValue function	Name collision	side_repo_maintenance.go, maintenance_workflow_yaml.go	Trivial

---

Finding 1: Duplicate token-resolver methods (Priority: High)

pkg/workflow/safe_outputs_env.go defines three methods that share ~90% of their structure, differing only in which token-resolver helper they call and whether they handle GitHub App tokens:

• addSafeOutputGitHubTokenForConfig — safe_outputs_env.go:186-216 (calls getEffectiveSafeOutputGitHubToken, handles GitHub App)
• addSafeOutputCopilotGitHubTokenForConfig — safe_outputs_env.go:220-250 (calls getEffectiveCopilotRequestsToken, handles GitHub App)
• addSafeOutputAgentGitHubTokenForConfig — safe_outputs_env.go:259-277 (calls getEffectiveCopilotCodingAgentGitHubToken, no GitHub App handling per design)

The shared pattern is: read safeOutputsToken from data.SafeOutputs.GitHubToken, optionally use app token, pick the first non-empty configToken/safeOutputsToken, then emit github-token: <effective> via the resolver.

Recommended consolidation

type tokenResolver func(customToken string) string

func (c *Compiler) addSafeOutputGitHubTokenWithResolver(
    steps *[]string,
    data *WorkflowData,
    configToken string,
    resolver tokenResolver,
    useGitHubApp bool, // false for agent variant
) {
    var safeOutputsToken string
    if data.SafeOutputs != nil {
        safeOutputsToken = data.SafeOutputs.GitHubToken
    }

    effectiveCustomToken := configToken
    if effectiveCustomToken == "" {
        effectiveCustomToken = safeOutputsToken
    }

    if useGitHubApp && data.SafeOutputs != nil && data.SafeOutputs.GitHubApp != nil {
        if data.SafeOutputs.GitHubApp.shouldIgnoreMissingKey() {
            fallback := resolver(effectiveCustomToken)
            *steps = append(*steps, fmt.Sprintf("          github-token: %s\n",
                combineTokenExpressions("${{ steps.safe-outputs-app-token.outputs.token }}", fallback)))
            return
        }
        *steps = append(*steps, "          github-token: ${{ steps.safe-outputs-app-token.outputs.token }}\n")
        return
    }

    *steps = append(*steps, fmt.Sprintf("          github-token: %s\n", resolver(effectiveCustomToken)))
}

The three call sites become thin wrappers that pass the appropriate resolver. Eliminates ~60 lines of duplicated control flow.

Impact: ~90 lines → ~30 lines, one place to fix token-precedence bugs.

---

Finding 2: Dead WASM stubs with stale documentation (Priority: High)

pkg/workflow/git_helpers.go:20-27 documents two functions that no production code actually calls:

// Tag Detection:
//   - GetCurrentGitTag() - Detect current Git tag from environment or repository
// Command Execution with Spinner:
//   - RunGit() - Execute git command with spinner, returning stdout
//   - RunGitCombined() - Execute git command with spinner, returning combined stdout+stderr

Only RunGitCombined is implemented in the non-WASM build. RunGit and GetCurrentGitTag exist only in git_helpers_wasm.go:14,21 as WASM stubs that return empty/error values. A repo-wide search shows zero callers of either function (workflow.RunGit(, workflow.GetCurrentGitTag(, bare RunGit(, bare GetCurrentGitTag()). The actual callers in pkg/cli/trial_repository.go:195,551 use RunGitCombined instead.

Recommendation:

• Delete RunGit and GetCurrentGitTag from git_helpers_wasm.go
• Remove the corresponding lines from the docstring in git_helpers.go:23,26

Impact: Eliminates dead code and prevents misleading docs from sending future contributors to phantom functions.

---

Finding 3: validateRunsOnValue is in the wrong file (Priority: Medium)

pkg/workflow/frontmatter_parsing.go:85-125 defines validateRunsOnValue(value any) error — a structural validator for the runs-on field (string / array-of-strings / {group, labels} object).

A sibling file pkg/workflow/runs_on_validation.go already exists and contains validateRunsOn(frontmatter, markdownPath) error plus its docstring explicitly declares it as the home for runs-on validation. Splitting structural vs. semantic validation across two files makes it harder to spot all the rules a runs-on value must satisfy.

Recommendation: Move validateRunsOnValue to runs_on_validation.go. The single caller is frontmatter_parsing.go:31, which still compiles since both files share the workflow package.

Impact: Co-locates all runs-on validation logic; aligns with the file's own documented purpose.

---

Finding 4: Scattered field-preprocessing helpers (Priority: Medium)

Six preprocess*Field* functions normalize raw config maps before YAML unmarshaling, but they're split across three files with no clear allocation rule:

Function	File
preprocessBoolFieldAsString	templatables.go:147
preprocessIntFieldAsString	templatables.go:221
preprocessStringArrayFieldAsTemplatable	templatables.go:268
preprocessProtectedFilesField	parse_helpers.go:62
preprocessExpiresField	config_helpers.go:151
preprocessScheduleFields	schedule_preprocessing.go:103

All six share the same shape: take configData map[string]any + a logger, mutate the map, return early on type mismatches. They're invoked together from create-/close-/update-entity parsers.

Recommendation: Consolidate the generic field-shape helpers (preprocessBoolFieldAsString, preprocessIntFieldAsString, preprocessStringArrayFieldAsTemplatable, preprocessExpiresField, preprocessProtectedFilesField) into a single config_preprocessing.go. Leave preprocessScheduleFields where it is — it lives on *Compiler and is schedule-specific, not generic.

Impact: New contributors looking for "how do I normalize a templatable bool field" land on a single file; reduces accidental duplication of similar helpers when a new field type is added.

---

Finding 5: Local variable shadows function (Priority: Low)

pkg/workflow/side_repo_maintenance.go:575 and pkg/workflow/maintenance_workflow_yaml.go:682 both declare a local variable named validateRunsOnValue — which is also the name of a package-level function defined in frontmatter_parsing.go:85.

validateRunsOnValue := FormatRunsOn(nil, "ubuntu-latest")   // confusing variable name

The variable holds the formatted runs-on string for YAML emission; it has nothing to do with validation. Two readers would have to follow the FormatRunsOn definition to realize this is not invoking the validator.

Recommendation: Rename the local variables (e.g. runsOnYAML or formattedRunsOn) to remove the shadow.

Impact: Trivial fix, removes a real reading-comprehension hazard.

---

Analysis Metadata

• Files analyzed: 834 non-test .go files (find pkg -name '*.go' ! -name '*_test.go')
• Primary packages: pkg/workflow (387 files), pkg/cli (303 files), pkg/parser (41 files)
• Approach: function-signature inventory by file via grep -n '^func ', then cross-file pattern matching for naming clusters (addSafeOutput*Token, parseClose*Config, preprocess*Field, validate*), followed by call-site verification via repo-wide grep
• What was inspected and ruled out:
  • 7 *_wasm.go files — intentional build-tagged stubs, not duplicates (except for dead code in finding 2)
  • 51 codemod_*.go files — well-factored via codemod_factory.go registry pattern
  • 7 audit_report_render*.go files — split by render concern, no duplication found
  • model_identifier.go validate* helpers — self-contained spec implementation, intentional co-location
  • closeEntityRegistry / parseCloseEntityConfig — already DRY via registry pattern
  • parseCreateEntityConfig[T] / parseUpdateEntityConfigTyped[T] — already generic

References:

• §26317775078

Generated by 🔧 Semantic Function Refactoring · ● 27.3M · ◷

• expires on May 25, 2026, 12:18 AM UTC