Skip to content

Refactor: extract shared singleflight cache for current-repository lookups #32952

@github-actions

Description

@github-actions

Summary

Two packages independently define identical mutex-guarded singleflight cache structs for the "current repository" lookup. Both have the same fields, same access pattern, same defensive comment about avoiding sync.Once reset races, and call a network/exec-based uncached function under lock. This is structural duplication that will drift.

Locations

Site Apkg/cli/repo.go:18-26 defines:

type repoSlugCacheState struct {
	mu     sync.Mutex
	result string
	err    error
	done   bool
}

var currentRepoSlugCache repoSlugCacheState

Used by GetCurrentRepoSlug() at pkg/cli/repo.go:93-109. Calls getCurrentRepoSlugUncached() (gh CLI + git remote fallback).

Site Bpkg/workflow/repository_features_validation.go:66-78 defines:

type currentRepositoryCacheState struct {
	mu     sync.Mutex
	result string
	err    error
	done   bool
}

var currentRepositoryCache currentRepositoryCacheState

Used by getCurrentRepository() at pkg/workflow/repository_features_validation.go:158-175. Calls getCurrentRepositoryUncached() (repository.Current() via gh CLI).

Both structs carry the same defensive comment:

"Using a mutex-guarded struct instead of sync.Once avoids the data race that arises when resetting sync.Once via struct assignment (= sync.Once{}) after first use."

The duplication is exact: same field names, same field order, same types, same purpose. The two functions even reach different repositories of truth (pkg/cli calls workflow.RunGH, pkg/workflow calls repository.Current()), and that divergence is what makes the duplication worth consolidating before they drift further apart.

Impact

  • Severity: Medium
  • Affected files: 2 (both production code paths shared by CLI commands and the workflow compiler)
  • Risk: Bug fixes or behavioral changes (e.g., adding TTL, cache invalidation, observability hooks) made in one struct will silently miss the other. Already we can see slight divergence: cli/repo.go returns owner/repo and falls back to parsing git remote get-url origin, while workflow/repository_features_validation.go uses repository.Current() directly. If a test resets one cache, the other still holds stale state.

Recommendation

Extract a small generic singleflight cache helper that captures the pattern. The simplest shape — a one-shot caching wrapper that doesn't need the full sync.Once dance:

// pkg/syncutil/onceloader.go (new)
package syncutil

import "sync"

// OnceLoader caches the result of a fallible, expensive one-shot fetch.
// Safe for concurrent use; loader is invoked at most once.
type OnceLoader[T any] struct {
	mu     sync.Mutex
	result T
	err    error
	done   bool
}

func (o *OnceLoader[T]) Get(loader func() (T, error)) (T, error) {
	o.mu.Lock()
	defer o.mu.Unlock()
	if !o.done {
		o.result, o.err = loader()
		o.done = true
	}
	return o.result, o.err
}

Then each call site becomes:

// pkg/cli/repo.go
var currentRepoSlugCache syncutil.OnceLoader[string]

func GetCurrentRepoSlug() (string, error) {
	return currentRepoSlugCache.Get(getCurrentRepoSlugUncached)
}

// pkg/workflow/repository_features_validation.go
var currentRepositoryCache syncutil.OnceLoader[string]

func getCurrentRepository() (string, error) {
	return currentRepositoryCache.Get(getCurrentRepositoryUncached)
}

As a bonus this also adds defer o.mu.Unlock() instead of the manual Unlock() calls used in both sites today — see related finding below.

An alternative is to keep both structs but consolidate them into a single helper in pkg/cliutil if you'd rather not introduce a new package.

Validation

  • All existing tests for pkg/cli/repo.go and pkg/workflow/repository_features_validation.go pass without modification.
  • New unit tests for the shared OnceLoader cover: single-call success, single-call error, repeated calls returning cached result (and not invoking loader again), concurrent races (loader invoked exactly once under -race).
  • grep confirms no remaining *CacheState structs with identical shape.

Estimated effort

Small (2-3 hours including tests).

Context

Discovered in Sergo Run 12 (2026-05-18) during the sync-primitives audit (new exploration component). This is the first cross-package structural duplication found by Sergo's mutex audit; previous duplication findings were function/dispatch-table level.

Generated by 🤖 Sergo - Serena Go Expert · ● 19.2M ·

  • expires on May 25, 2026, 5:15 AM UTC

Metadata

Metadata

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