Skip to content

[refactor] Semantic function clustering: GetGitHubToken env-lookup drift + scattered cron helpersΒ #45143

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw β€” 2026-07-13

Executive Summary

Analyzed non-test Go sources across pkg/ (β‰ˆ1,058 .go files; dominant packages pkg/workflow at 447, pkg/cli at 345, pkg/parser at 51). The codebase is largely well-organized β€” most packages follow a clear one-feature-per-file convention and cross-package helpers are reused rather than copied. Function-length and generic duplicate-code are already tracked by other workflows ([lint-monster] #45033, [duplicate-code] #44873), so this report focuses narrowly on semantic clustering / organization drift: a small number of genuine near-duplicates and scattered inline helpers.

Findings: 3 actionable (1 drift-risk near-duplicate, 2 low-priority intra-cluster consolidations). No functions found in clearly wrong files.

Well-organized clusters (no action needed) βœ“

  • Codemod cluster β€” β‰ˆ80 pkg/cli/codemod_*.go files, each holding exactly one codemod behind a shared codemod_factory.go. Textbook one-feature-per-file. βœ“
  • Schedule-frequency classification β€” pkg/cli/add_interactive_schedule.go:classifyScheduleFrequency correctly delegates to parser.IsHourlyCron/IsDailyCron/IsWeeklyCron rather than re-implementing cron detection. βœ“
  • wasm/non-wasm pairs β€” github.go/github_wasm.go, virtual_fs.go/virtual_fs_wasm.go, remote_fetch* correctly split via //go:build tags. βœ“

Identified Issues

1. Near-duplicate with active drift: GetGitHubToken env-lookup β€” Priority 2

The GITHUB_TOKEN β†’ GH_TOKEN environment-variable lookup is implemented twice across the build-tag pair, and the two copies have already drifted:

  • pkg/parser/github.go:62 (//go:build !js && !wasm) β€” passes githubLog, then falls back to gh auth token.
  • pkg/parser/github_wasm.go:11 (//go:build js || wasm) β€” passes nil logger, different error text, no CLI fallback.
// github.go (non-wasm)
if token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", githubLog); token != "" { return token, nil }
if token := envutil.GetStringFromEnv("GH_TOKEN", "", githubLog); token != "" { return token, nil }

// github_wasm.go (wasm)
if token := envutil.GetStringFromEnv("GITHUB_TOKEN", "", nil); token != "" { return token, nil }
if token := envutil.GetStringFromEnv("GH_TOKEN", "", nil); token != "" { return token, nil }

Risk: the shared env-var precedence (GITHUB_TOKEN before GH_TOKEN) is exactly the kind of contract that silently diverges when only one copy is edited.

Recommendation: extract the env-lookup prefix into a build-tag-agnostic helper in a non-tagged file:

func githubTokenFromEnv(log *logger.Logger) string {
    if t := envutil.GetStringFromEnv("GITHUB_TOKEN", "", log); t != "" { return t }
    return envutil.GetStringFromEnv("GH_TOKEN", "", log)
}

Both GetGitHubToken variants call it; only the gh auth token fallback stays build-tagged. Effort: ~30 min. Benefit: single source of truth for token precedence.

2. Repeated inline numeric-field check in schedule_cron_detection.go β€” Priority 3

IsDailyCron, IsHourlyCron, and IsWeeklyCron (pkg/parser/schedule_cron_detection.go:21/56/90) each open-code the same digit-only scan (4 occurrences total) plus repeated len(fields) != 5 and wildcard guards:

for _, ch := range minute {
    if ch < '0' || ch > '9' { return false }
}

Ironically the file already declares a pre-compiled cronFieldPattern regex (line 17) that these functions don't use for this purpose.

Recommendation: extract isNumericCronField(s string) bool (and optionally cronFields(cron string) ([]string, bool) returning the guarded 5 fields). Effort: ~20 min. Benefit: removes 4 copies of the scan, one definition of "numeric field".

3. Template duplication in schedule cron-warning constructors β€” Priority 3

addDailyCronWarning / addHourlyCronWarning / addWeeklyCronWarning (pkg/workflow/schedule_preprocessing.go:461/485/511) share identical boilerplate: strings.Fields(cronExpr) β†’ build a "...Consider using fuzzy schedule ... to distribute workflow execution times and reduce load spikes." message β†’ c.IncrementWarningCount() β†’ c.addScheduleWarning(msg).

Recommendation: factor the log+increment+store tail into one helper emitScheduleWarning(cronExpr, msg string); each function keeps only its distinct field extraction and message text. Effort: ~20 min. Benefit: the shared wording and warning-count bookkeeping live in one place.


Refactoring Recommendations (prioritized)

  1. Priority 2 β€” Deduplicate GetGitHubToken env lookup (drift risk). ~30 min.
  2. Priority 3 β€” Extract isNumericCronField helper in schedule_cron_detection.go. ~20 min.
  3. Priority 3 β€” Extract emitScheduleWarning helper in schedule_preprocessing.go. ~20 min.

Implementation Checklist

  • Extract build-tag-agnostic githubTokenFromEnv helper; wire both GetGitHubToken variants
  • Extract isNumericCronField (and field-guard) helper in schedule_cron_detection.go
  • Extract emitScheduleWarning boilerplate helper in schedule_preprocessing.go
  • Run existing schedule/cron and parser tests to confirm no behavior change

Analysis Metadata

  • Packages analyzed: all of pkg/ (focus: pkg/workflow, pkg/cli, pkg/parser, pkg/console)
  • Non-test Go files: β‰ˆ1,058
  • Actionable findings: 3 (1 drift-risk near-duplicate, 2 intra-cluster consolidations)
  • Outliers (functions in wrong files): 0
  • Detection method: naming-pattern clustering + targeted signature/implementation comparison
  • Scope note: function-length and generic duplicate-code are covered by other workflows and intentionally excluded here

Generated by πŸ”§ Semantic Function Refactoring Β· 227.9 AIC Β· βŒ– 8.72 AIC Β· ⊞ 9.1K Β· β—·

  • expires on Jul 14, 2026, 4:11 PM UTC-08:00

Metadata

Metadata

Type

No type

Fields

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