Skip to content

[refactor] Consolidate 13 hand-rolled sync.Once+error caches onto syncutil.OnceLoader #50747

Description

@github-actions

Semantic Function Clustering Analysis — pkg/styles, pkg/syncutil

Scope for this run (precomputed slice): pkg/styles, pkg/syncutil — 4 non-test Go files.

Executive Summary

Both packages in scope are internally well-organized: each file has a single clear purpose, function names cluster cleanly with their file, and no outliers or intra-package duplicates were found.

The significant finding is outward-facing: pkg/syncutil.OnceLoader[T] — the package's sole abstraction — is under-adopted. 13 call sites across pkg/parser and pkg/workflow hand-roll the exact contract OnceLoader already implements (a sync.Once + cached value + cached error triple, returned as (T, error)). This is the same functional duplicate replicated 13 times.

Metric Value
Files analyzed (in scope) 4
Functions/methods cataloged 9
Outliers found (wrong file) 0
Intra-package duplicates 0
Functional duplicates of a scoped abstraction 13

Finding 1 (Priority 1): OnceLoader[T] duplicated 13x as hand-rolled sync.Once triples

pkg/syncutil/onceloader.go defines exactly this contract:

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)
func (o *OnceLoader[T]) Override(result T, err error)
func (o *OnceLoader[T]) Reset()

It has 6 adopters (pkg/modelsdev/catalog.go, pkg/cli/repo.go, pkg/cli/org_issue_pr_helpers.go, pkg/workflow/docker_validation.go, pkg/workflow/repository_features_validation.go, plus test resets). Meanwhile 13 other sites open-code it.

Canonical duplicate shape — every site below is a near-exact instance:

// pkg/workflow/schema_validation.go:56
var (
	compiledSchemaOnce sync.Once
	compiledSchema     *jsonschema.Schema
	schemaCompileError error
)

func getCompiledSchema() (*jsonschema.Schema, error) {
	compiledSchemaOnce.Do(func() {
		compiledSchema, schemaCompileError = compileSchema(...)
	})
	return compiledSchema, schemaCompileError
}
// pkg/workflow/awf_config.go:92 — identical structure, different payload
var (
	compiledAWFConfigSchemaOnce sync.Once
	compiledAWFConfigSchema     *jsonschema.Schema
	awfConfigSchemaCompileError error
)

func getCompiledAWFConfigSchema() (*jsonschema.Schema, error) {
	compiledAWFConfigSchemaOnce.Do(func() {
		compiledAWFConfigSchema, awfConfigSchemaCompileError = compileSchema(...)
	})
	return compiledAWFConfigSchema, awfConfigSchemaCompileError
}

Both collapse to:

var compiledSchemaLoader syncutil.OnceLoader[*jsonschema.Schema]

func getCompiledSchema() (*jsonschema.Schema, error) {
	return compiledSchemaLoader.Get(func() (*jsonschema.Schema, error) {
		return compileSchema(...)
	})
}
All 13 duplicate sites (verified)

pkg/parser/schema_compiler.go — 6 instances in one var block:

Line Once var Value var Error var
38 mainWorkflowSchemaOnce compiledMainWorkflowSchema mainWorkflowSchemaError
39 mcpConfigSchemaOnce compiledMcpConfigSchema mcpConfigSchemaError
40 repoConfigSchemaOnce compiledRepoConfigSchema repoConfigSchemaError
41 awManifestSchemaOnce compiledAwManifestSchema awManifestSchemaError
56 parsedMainWorkflowSchemaDocOnce parsedMainWorkflowSchemaDocVal parsedMainWorkflowSchemaDocErr
60 parsedMcpConfigSchemaDocOnce parsedMcpConfigSchemaDocVal parsedMcpConfigSchemaDocErr

This single block declares 18 package-level variables that OnceLoader would reduce to 6.

pkg/parser/schema_deprecation.go:

  • L31 deprecatedFieldsOnce / deprecatedFieldsCache / deprecatedFieldsErr
  • L154 deprecatedFieldsDeepOnce / deprecatedFieldsDeepCache / deprecatedFieldsDeepErr

pkg/workflow:

  • schema_validation.go:56compiledSchemaOnce / compiledSchema / schemaCompileError
  • awf_config.go:92compiledAWFConfigSchemaOnce / compiledAWFConfigSchema / awfConfigSchemaCompileError
  • samples_validation.go:62compiledToolSchemasOnce / compiledToolSchemas / compiledToolSchemasErr
  • imports.go:165safeOutputTypeKeysOnce / safeOutputTypeKeys / safeOutputTypeKeysErr
  • model_aliases.go:50builtinModelAliasesOnce / builtinModelAliasesData / builtinModelAliasesErr

Why this matters beyond line count:

  1. Test isolation. OnceLoader exposes Reset() and Override(); a sync.Once cannot be reset. pkg/workflow/docker_validation_graceful_test.go and pkg/cli/repo_test_helpers_test.go reset their loaders cleanly — the 13 hand-rolled sites have no equivalent escape hatch, so any test needing a different cached value must either run first or not at all.
  2. Single source of truth for the concurrency contract. 13 independent implementations of "cache the error too" is 13 chances to get the mutex/visibility semantics subtly wrong.
  3. Declaration density. 39 package-level vars collapse to 13.

Recommended sequencing (each independently mergeable):

  • pkg/parser/schema_compiler.go — highest density (6 sites, 18 vars to 6)
  • pkg/parser/schema_deprecation.go — 2 sites
  • pkg/workflow/schema_validation.go + awf_config.go — 2 sites, near-identical bodies
  • pkg/workflow/samples_validation.go, imports.go, model_aliases.go — 3 sites

Estimated effort: 3-5 hours total. No behavior change; OnceLoader.Get preserves the cache-the-error semantics every site relies on.

Explicitly NOT candidates (checked and excluded)

These use sync.Once but have no error to cache, so OnceLoader[T] is the wrong shape — leave them alone:

  • pkg/actionpins/actionpins.go:125 — populates 3 unrelated caches, infallible
  • pkg/cli/model_costs.go:40 — infallible, swallows unmarshal error by design
  • pkg/testutil/tempdir.go:18, pkg/workflow/agentic_engine.go:527, pkg/workflow/runtime_definitions.go:220, pkg/workflow/model_alias_validation.go:43, pkg/workflow/model_aliases.go:73, pkg/workflow/samples_validation.go:66 — infallible one-shot init

Also correctly excluded: sync.OnceValue/sync.OnceValues sites (pkg/console/console.go:25-26, pkg/workflow/domains.go:22, pkg/workflow/permissions_toolset_data.go:39) already use the stdlib equivalent.

Finding 2 (Priority 3): pkg/styles build-tag parity is maintained by hand and untested

theme.go (//go:build !js && !wasm) and theme_wasm.go (//go:build js || wasm) must export an identical symbol surface — currently 11 colors, 3 borders, and 31 styles each. They are in parity today (verified symbol-by-symbol), but:

  • Both test files (theme_test.go, spec_test.go) are tagged //go:build !integration && !js && !wasm, so zero tests ever compile against theme_wasm.go.
  • make build-wasm is a real CI target, so drift surfaces as a wasm build break — but only for symbols an untagged consumer actually references. pkg/logger/logger.go is untagged and binds styles.ColorInfo etc. to WasmColor under wasm; a style added to theme.go and used only from !wasm code would drift silently.

Recommendation: add a wasm-tagged parity test, or a small go:generate/lint check asserting the two files export the same identifier set. Low urgency — this is a guardrail, not a present defect.

Not Found

  • Outliers: none. theme.go = palette + styles, huh_theme.go = huh form mapping, theme_wasm.go = wasm no-ops, onceloader.go = the OnceLoader type. Each file matches its name.
  • Scattered helpers: none. applyBlurredAndGroupStyles (huh_theme.go:71) is correctly co-located with its only caller.
  • Generics opportunities: none new — OnceLoader[T] already is the generic abstraction; Finding 1 is about using it.

Analysis Metadata

  • Files analyzed: 4 (pkg/styles/theme.go, pkg/styles/theme_wasm.go, pkg/styles/huh_theme.go, pkg/syncutil/onceloader.go)
  • Detection method: symbol inventory + naming-cluster analysis, cross-referenced against repo-wide sync.Once usage; every reported site read and verified against OnceLoader's contract
  • Analysis date: 2026-08-06

Generated by 🔧 Semantic Function Refactoring · sonnet46 · 302 AIC · ⌖ 17.8 AIC · ⊞ 9.7K ·

  • expires on Aug 7, 2026, 7:35 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