Skip to content

Refactor workflow cache/action/validation paths by extracting focused helpers#36248

Merged
pelikhan merged 7 commits into
mainfrom
copilot/lint-monster-refactor-long-functions-part-2
Jun 1, 2026
Merged

Refactor workflow cache/action/validation paths by extracting focused helpers#36248
pelikhan merged 7 commits into
mainfrom
copilot/lint-monster-refactor-long-functions-part-2

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

This PR addresses the lint-monster Part 2 workflow refactor scope by decomposing long functions in pkg/workflow (cache support, action resolution, validation) into smaller units while preserving behavior. The changes target the specified hotspots and reduce control-flow complexity through helper extraction and reuse.

  • Cache parsing and config extraction (pkg/workflow/cache.go)

    • Split parseCacheMemoryEntry into focused parsers for identity/key, retention, scope, restore-only, and allowed extensions.
    • Split extractCacheMemoryConfig into default-entry and array/object parsing helpers.
    • Split generateCacheSteps into parse/render helpers (parseCacheStepConfigs, writeCacheStep, field writers) to isolate YAML parsing from step emission.
    • Added safer numeric normalization for YAML-derived values (int/float64/uint64) in parseOptionalInt.
  • Action resolution and lock parsing (pkg/workflow/action_reference.go, action_cache.go, action_sha_checker.go)

    • Decomposed resolveSetupActionRef into mode-specific helpers (resolveSetupActionModeRef, resolveSetupReleaseModeRef) with shared tag/SHA resolution helpers.
    • Decomposed resolveActionReference into explicit helpers for frontmatter tag detection, setup-action resolution, and release pin resolution.
    • Refactored deduplicateEntries by separating grouping, candidate ranking, removal collection, and deletion.
    • Refactored ExtractActionsFromLockFile into YAML validation + match parsing + version resolution helpers.
  • Workflow validation decomposition (pkg/workflow/agent_validation.go, call_workflow_validation.go)

    • Split validateWorkflowRunBranches into trigger parsing, required-workflows validation, and strict/non-strict branch-warning handling.
    • Split validateCallWorkflow into self-reference checks, existence checks, YAML/MD trigger checks, and reusable trigger-validation helpers.
// Before: one long function handled parsing + rendering + option formatting.
// After: rendering is composed from small helpers.
func writeCacheStep(builder *strings.Builder, cache map[string]any, idx int, total int) {
	stepName := resolveCacheStepName(cache, idx, total)
	fmt.Fprintf(builder, "      - name: %s\n", stepName)
	fmt.Fprintf(builder, "        uses: %s\n", getActionPin("actions/cache"))
	builder.WriteString("        with:\n")
	writeCacheStepValue(builder, "key", cache["key"])
	writeCachePath(builder, cache["path"])
	writeCacheRestoreKeys(builder, cache["restore-keys"])
	writeCacheStepValue(builder, "upload-chunk-size", cache["upload-chunk-size"])
	writeCacheStepValue(builder, "fail-on-cache-miss", cache["fail-on-cache-miss"])
	writeCacheStepValue(builder, "lookup-only", cache["lookup-only"])
}

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.3M ·

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor long functions in workflow and CLI packages Refactor workflow cache/action/validation paths by extracting focused helpers Jun 1, 2026
Copilot AI requested a review from gh-aw-bot June 1, 2026 15:20
@pelikhan pelikhan marked this pull request as ready for review June 1, 2026 15:42
Copilot AI review requested due to automatic review settings June 1, 2026 15:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors several pkg/workflow hotspots by extracting smaller helper functions for cache parsing/rendering, action reference resolution, lock parsing, and workflow validation, aiming to reduce control-flow complexity while preserving behavior.

Changes:

  • Decomposed cache-memory parsing/config extraction and cache step generation into focused helpers (including new numeric normalization for YAML-derived ints).
  • Split action setup/reference resolution and action lock parsing into reusable helpers; refactored action cache deduplication into discrete phases.
  • Broke down workflow validation logic (call-workflow + workflow_run) into smaller validation helpers.
Show a summary per file
File Description
pkg/workflow/call_workflow_validation.go Extracts call-workflow validation into dedicated helpers (self-reference, existence, trigger support).
pkg/workflow/cache.go Refactors cache-memory parsing and cache step generation; introduces parseOptionalInt for safer YAML numeric normalization.
pkg/workflow/agent_validation.go Splits workflow_run validation into trigger parsing + focused validation helpers.
pkg/workflow/action_sha_checker.go Extracts lock YAML validation and action usage parsing/version resolution helpers.
pkg/workflow/action_reference.go Refactors setup action reference resolution and release pinning into mode-specific helpers.
pkg/workflow/action_cache.go Refactors deduplication into grouping, ranking, removal collection, and deletion helpers.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 2

Comment thread pkg/workflow/cache.go
Comment on lines +210 to +214
if floatValue, ok := value.(float64); ok {
if math.IsNaN(floatValue) || math.IsInf(floatValue, 0) {
return nil
}
// Validate retention-days bounds
if entry.RetentionDays != nil {
if err := validateIntRange(*entry.RetentionDays, 1, 90, "retention-days"); err != nil {
return entry, err
}
if floatValue != math.Trunc(floatValue) {
Comment thread pkg/workflow/cache.go
Comment on lines +196 to +205
// parseOptionalInt safely converts YAML numeric values (int, float64, uint64) to *int.
//
// It returns nil when the input cannot be represented as an integer for the current
// architecture, including:
// - NaN/Inf float64 values
// - fractional float64 values
// - float64 values outside the exact-integer range [-2^53, 2^53]
// - uint64 values larger than math.MaxInt
// - unsupported types
func parseOptionalInt(value any) *int {
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 1, 2026

@copilot /copilot-review

@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 1, 2026

@copilot apply copilot-review skill

Copilot AI and others added 3 commits June 1, 2026 17:04
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 1, 2026

@copilot apply copilot-review skill

Applied the copilot-review skill and addressed the follow-up review findings in commits 546e94a and a11240b (including parseOptionalInt architecture-safe bounds handling and targeted regression tests).

@pelikhan pelikhan merged commit aec5d90 into main Jun 1, 2026
26 checks passed
@pelikhan pelikhan deleted the copilot/lint-monster-refactor-long-functions-part-2 branch June 1, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[lint-monster] Refactor long functions in workflow and CLI packages (Part 2 of 2)

4 participants