Skip to content

Refactor: RunUpdateWorkflows/UpdateWorkflows/updateWorkflow trio each take ~14 positional params (options-struct candidate [Content truncated due to length] #31423

@github-actions

Description

@github-actions

Sergo finding (Run 6, 2026-05-11)

Parameter-list scan extending Run 5's DownloadWorkflowLogs finding (26 params, issue #31299/equivalent — referenced in tracker #31300). Run 6 enumerated all non-test ^func declarations in pkg/ whose single-line signature contains ≥10 identifiers and found a 3-function chain that all carry the same ~14 parameters in the same order. The args are forwarded layer-to-layer essentially verbatim. (#aw_sg6p1)

Locations

Function File:line Params Body
RunUpdateWorkflows pkg/cli/update_command.go:126 14 calls UpdateWorkflows
UpdateWorkflows pkg/cli/update_workflows.go:21 14 calls updateWorkflow in a loop
updateWorkflow pkg/cli/update_workflows.go:383 14 the per-workflow worker

Evidence

Production call chain (update_command.go:86update_command.go:131update_workflows.go:54):

// 1. cobra Run wires the cobra flags into the entry point
RunUpdateWorkflows(cmd.Context(), args, majorFlag, forceFlag, verbose, engineOverride,
    workflowDir, noStopAfter, stopAfter, noMergeFlag, disableReleaseBump, noCompile, noRedirect, coolDown)

// 2. RunUpdateWorkflows forwards 13 of those 14 to UpdateWorkflows
UpdateWorkflows(ctx, workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir,
    noStopAfter, stopAfter, noMerge, noCompile, noRedirect, coolDown)

// 3. UpdateWorkflows forwards them again to updateWorkflow inside the for-loop
updateWorkflow(ctx, wf, allowMajor, force, verbose, engineOverride,
    noStopAfter, stopAfter, noMerge, noCompile, noRedirect, coolDown)

Verified via Serena find_referencing_symbols:

  • RunUpdateWorkflows has 1 production caller + 2 test callers (TestRunUpdateWorkflows_NoSourceWorkflows, TestRunUpdateWorkflows_SpecificWorkflowNotFound)
  • Test callers spell out the full 14 positional args, several of which are empty strings or false placeholders — a known readability anti-signal:
    RunUpdateWorkflows(context.Background(), nil, false, false, false, "", "", false, "", false, false, false, false, 0)

Why the 3-way duplication matters

  • Adding a new option requires touching three function signatures, every test caller, and the cobra wiring — all in lockstep, and the order is significant since the args are positional.
  • The RunUpdateWorkflows-vs-UpdateWorkflows split exists only to layer in UpdateActions/UpdateContainerPins/UpdateActionsInWorkflowFiles side-effects. The forwarded params are otherwise identical, so refactoring all three becomes essentially mechanical.
  • The codebase already has the right pattern: compileSpecificFiles/compileAllFilesInDirectory (pkg/cli/compile_pipeline.go:41,206) accept a CompileConfig struct, not 14 positional bools/strings. The update-workflows trio just hasn't adopted it.

Severity

Medium — no correctness bug today, but the layering means a single new flag costs ~6 call-site edits + 14-position-shift risk in every test. This is also a precondition for the Run 5 DownloadWorkflowLogs refactor (#31300 tracker) being adoptable elsewhere.

Recommendation

Introduce an UpdateWorkflowsOptions (or UpdateConfig) struct that carries the 13 non-context fields:

type UpdateWorkflowsOptions struct {
    WorkflowNames       []string
    AllowMajor          bool
    Force               bool
    Verbose             bool
    EngineOverride      string
    WorkflowsDir        string
    NoStopAfter         bool
    StopAfter           string
    NoMerge             bool
    DisableReleaseBump  bool   // only RunUpdateWorkflows uses this
    NoCompile           bool
    NoRedirect          bool
    CoolDown            time.Duration
}

func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error
func UpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error
func updateWorkflow(ctx context.Context, wf *workflowWithSource, opts UpdateWorkflowsOptions) error

Follow the same shape CompileConfig already uses in pkg/cli/compile_pipeline.go so the codebase stays consistent.

Validation

  • All three signatures take (ctx context.Context, opts UpdateWorkflowsOptions, ...) (plus wf for updateWorkflow)
  • go test ./pkg/cli/... passes — both update_command_test.go cases and update_workflows_test.go
  • Cobra wiring in NewUpdateCommand builds the opts struct field-by-field — clearly named fields make this less error-prone than positional args
  • No remaining RunUpdateWorkflows(... 14 positional args ...) call sites

Estimated effort

Small/Medium — 3 signatures, ~6 call sites, mechanical. The struct is the easy part; the win is in tests where every call gets 14× less brittle.


Generated by Sergo — Strategy: parameter-list-systematic-scan-plus-deep-nesting (Run 6)
Run: §25651209481

Generated by Sergo - Serena Go Expert · ● 22M ·

  • expires on May 18, 2026, 5:10 AM UTC

Metadata

Metadata

Assignees

No one assigned

    Labels

    cookieIssue Monster Loves Cookies!sergo

    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