Skip to content

[refactor] pkg/ctxutil + pkg/envutil: 3 call sites bypass ctxutil.OrBackground, incl. wasm/non-wasm divergence #52402

Description

@github-actions

Overview

Semantic function clustering analysis of the pkg/ctxutil and pkg/envutil package slice.

The two analyzed files are themselves clean — no outlier functions, no internal duplicates, correct one-file-per-feature organization. All findings below are call sites elsewhere in the repo that hand-roll logic these packages already provide.

Key Issues

# Type Location Severity
1 Exact duplicate pkg/workflow/skills_ref_resolution.go:70-73 High
2 Divergent sibling pkg/workflow/github_cli.go:49-51 High
3 Functional duplicate pkg/cli/docker_images.go:70-75 Medium

1. Exact inline duplicate of ctxutil.OrBackground

pkg/workflow/skills_ref_resolution.go:70-73 reproduces the helper byte-for-byte:

ctx := data.Ctx
if ctx == nil {
    ctx = context.Background()
}

This is precisely the pattern ctxutil.OrBackground was created to eliminate — its doc comment quotes this exact snippet as the anti-pattern.

Fix: ctx := ctxutil.OrBackground(data.Ctx)

2. wasm / non-wasm build variants diverge (most notable finding)

Two sibling files in the same package solve the identical problem differently:

// pkg/workflow/github_cli_wasm.go:59-61  — uses the helper
func ghUnavailableCommand(ctx context.Context) *exec.Cmd {
    ctx = ctxutil.OrBackground(ctx)
    return exec.CommandContext(ctx, "echo", "gh CLI not available in Wasm")
}

// pkg/workflow/github_cli.go:49-51  — hand-rolled, and picks TODO() not Background()
if ctx == nil {
    ctx = context.TODO()
}

The wasm variant was migrated to ctxutil; the primary build path was not. Beyond the duplication, the two paths disagree on the fallback context type.

Fix: ctx = ctxutil.OrBackground(ctx) in setupGHCommand, and update the doc comment at line 42 which currently documents the context.TODO() behavior.

3. normalizeDockerContext is a redundant named helper

// pkg/cli/docker_images.go:70-75
func normalizeDockerContext(ctx context.Context) context.Context {
    if ctx == nil {
        return context.TODO()
    }
    return ctx
}

Same signature and same purpose as ctxutil.OrBackground, differing only in TODO() vs Background().

Fix: delete and call ctxutil.OrBackground directly at its call sites.

Function inventory and cluster analysis

pkg/ctxutil/ctxutil.go (package ctxutil)

  • OrBackground(ctx context.Context) context.Context

pkg/envutil/envutil.go (package envutil)

  • warn(debugLog *logger.Logger, msg string) — unexported shared helper
  • GetIntFromEnv(envVar string, defaultValue, minValue, maxValue int, debugLog *logger.Logger) int
  • GetBoolFromEnv(envVar string, defaultValue bool, debugLog *logger.Logger) bool
  • GetStringFromEnv(envVar, defaultValue string, debugLog *logger.Logger) string

Cluster: Get*FromEnv — three functions sharing the read → parse → validate → default shape. The common warning path is already factored into warn. Organization is correct; no action needed.

Generics assessment: GetIntFromEnv / GetBoolFromEnv / GetStringFromEnv look like generics candidates, but each has a genuinely different parse function, a different validation contract (bounds vs. none), and a different message format. A generic rewrite would need parser and validator callbacks and would be longer than the current code. Recommend leaving as-is.

Outlier check: none. Every function in both files matches its file's stated purpose.

envutil duplicate check: no inline os.Getenv + parse + default patterns found elsewhere. Other strconv.Atoi / ParseBool hits (pkg/parser/schedule_*.go, pkg/semverutil, pkg/typeutil, pkg/colorwriter) parse non-environment input and are correctly out of scope.

Adoption: both packages are actively used across 13 files, so these three sites are stragglers from an incomplete migration rather than an unadopted abstraction.

Fallback context convention: Background() vs TODO()

The three sites split on the nil fallback: skills_ref_resolution.go uses Background(), while github_cli.go and docker_images.go use TODO(). ctxutil.OrBackground standardizes on Background().

Semantically Background() is correct here — TODO() signals "a context should be plumbed through but has not been yet," which is a marker for incomplete refactoring, not a runtime fallback. Adopting the helper resolves the inconsistency in the right direction, but it is a small behavioral note worth a reviewer's eye since TODO() and Background() are otherwise identical at runtime.

Next Actions

  • Replace the inline block in pkg/workflow/skills_ref_resolution.go:70-73 with ctxutil.OrBackground
  • Migrate setupGHCommand in pkg/workflow/github_cli.go and fix its stale doc comment
  • Delete normalizeDockerContext in pkg/cli/docker_images.go and inline the helper at call sites
  • Consider a lint rule for the if ctx == nil fallback pattern to stop regressions

Estimated effort: ~1 hour. All three are mechanical, behavior-preserving except the deliberate TODO()Background() normalization.

Analysis Metadata

  • Files analyzed: 2 (pkg/ctxutil/ctxutil.go, pkg/envutil/envutil.go)
  • Functions cataloged: 5
  • Clusters identified: 2
  • Outliers in slice: 0
  • Duplicate call sites found: 3
  • Method: Serena semantic analysis + repo-wide pattern search for the slice's helper signatures

Generated by 🔧 Semantic Function Refactoring · sonnet46 · 193.1 AIC · ⌖ 20.4 AIC · ⊞ 9.6K ·

  • expires on Aug 14, 2026, 7:16 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions