[sergo] Sergo Report: String Literal Centralization + Interface Pollution - 2026-05-14 #32058
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #32291. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Executive Summary
Run 9 of the Sergo Go static-analysis agent. Strategy mix: 50% cached (extend Run 8's magic-literal-duplication scan from numeric/octal literals to string literals) + 50% new (interface-pollution audit using Serena LSP
find_referencing_symbolsto count interface implementers/users). Both halves converged on the same theme — DRY violations and shape duplication that the compiler can't catch.Two strong findings drove issue creation:
GetWorkflowDir()doc/code contract drift + 27 literal-bypass sites — the README documents an env-var override the implementation never reads, and 27 production sites hardcode the literal so the helper is bypassed anyway.SHAResolver/ActionSHAResolverinterface duplication — identical single-method interfaces in two packages, satisfied by the same single concrete type, used inconsistently across 17+ sites.Serena LSP fully operational. No tool changes (23 tools stable since Run 4).
find_referencing_symbolsperformed well on focused single-symbol queries (~2.3 s round-trip).Tools Snapshot
activate_project,find_referencing_symbols(LSP audit of two interface symbols)Strategy Selection
Cached Reuse Component (50%) — String literal duplication
FilePermSensitive/Public/DirPermSensitive/Publicconstants (now present inpkg/constants/constants.go:73-91— confirming Run 8 issue #aw_sg8a1 drove that work)..github/workflowsliteral,GH_AW_*env-var literals,.lock.ymlextension literal.New Exploration Component (50%) — Interface pollution audit
type X interface { ... }decls in non-test pkg/ (21 found). For each, use Serenafind_referencing_symbolsto count usage and locate implementers.Combined Rationale
Both halves attack "declared-twice, used-once" — string literals on the data side, interface types on the abstraction side. Constants/helpers and interfaces both come in two failure modes: under-adopted (helper exists, callers bypass it) and over-declared (two parallel forms compete). Run 9 found exactly one of each.
Codebase Context
pkg/constants,pkg/cli,pkg/workflow,pkg/actionpins,pkg/parserFindings Summary
Detailed Findings
High — GetWorkflowDir() doc/code contract drift + 27 literal-bypass sites
pkg/constants/constants.go:334-336—GetWorkflowDir()returnsfilepath.Join(".github", "workflows")with no env-var read.pkg/constants/README.md:349, 488— both claimGH_AW_WORKFLOWS_DIRis read at call time.".github/workflows"literal (21 files):pkg/cli/workflows.go:36-38(local shim duplicates the literal),pkg/cli/enable.go:280,pkg/cli/run_workflow_execution.go:203,pkg/cli/compile_watch.go:32,pkg/cli/add_command.go:331,pkg/cli/resolver.go:23,pkg/cli/compile_orchestrator.go:53,pkg/cli/lint_command.go:77,pkg/cli/remove_command.go:215, 331, 360,pkg/cli/fix_command.go:118,pkg/cli/mcp_list.go:24,pkg/cli/list_workflows_command.go:74,pkg/cli/upgrade_command.go:275,pkg/cli/mcp_safe_update_cache.go:21,pkg/cli/logs_utils.go:33,pkg/cli/update_container_pins.go:171,pkg/cli/compile_validation.go:186,pkg/cli/trial_helpers.go:69,pkg/parser/import_bfs.go:402,pkg/parser/import_remote.go:61, etc.constants.GetWorkflowDir().os.Getenv("GH_AW_WORKFLOWS_DIR")read in helper; sweep literals to call site. → Issue #aw_sg9a1.Medium —
SHAResolver/ActionSHAResolverinterface duplicationTwo packages declare structurally identical single-method interfaces:
pkg/actionpins/actionpins.go:59-61→SHAResolverpkg/workflow/action_resolver.go:16-18→ActionSHAResolverVia Serena
find_referencing_symbols:actionpins.SHAResolver: 1 use (consumer fieldPinContext.Resolveratpkg/actionpins/actionpins.go:88).workflow.ActionSHAResolver: 15+ uses inpkg/cli/copilot_setup.go,pkg/workflow/action_pins.go,pkg/workflow/action_reference.go,pkg/workflow/maintenance_workflow.go,pkg/workflow/maintenance_workflow_yaml.go,pkg/workflow/side_repo_maintenance.go.Single production implementer:
*workflow.ActionResolver(which structurally satisfies both). The duplicate exists only by historical accident; pkg/workflow already imports pkg/actionpins.workflow.ActionSHAResolver, switch ~17 call sites toactionpins.SHAResolver. → Issue #aw_sg9a2.Medium —
ValidatableToolinterface with single implementerpkg/workflow/permissions_toolset_data.go:73-78definesValidatableTool(2-method interface).*GitHubToolConfig(same file, line 81+).ValidatePermissions(in same package).Low —
".lock.yml"string literal in 21 production files (41 occurrences)No central helper exists. Most call sites concatenate workflow basename + literal. Candidate for a
constants.LockFileExtensionorconstants.LockFileName(workflowID)helper, but the literal is shorter than the constant reference would be (".lock.yml"vsconstants.LockFileExtension), so net DX is unclear. Note for future runs.Low —
GH_AW_*env-var literals (216 occurrences in 42 files) partial adoption ofconstants.EnvVar*namespkg/constants/engine_constants.goalready definesEnvVarPrompt,EnvVarMCPConfig,EnvVarSafeOutputs,EnvVarMaxTurns,EnvVarStartupTimeout,EnvVarToolTimeout,EnvVarGitHubToken, etc. (lines 196-320). Significant coverage but inconsistent uptake — manypkg/workflow/*_engine.gofiles still inline the literal. Pattern is bigger than one issue, and many literals are actually appearing in YAML template generation (not Go code) where the constant won't help. Deferred.Improvement Tasks Generated
Task 1: Fix
GetWorkflowDir()env-var contract + sweep 27 literal sitesgo test ./....Task 2: Consolidate
SHAResolver/ActionSHAResolverworkflow.ActionSHAResolver; switch 17 sites toactionpins.SHAResolver;go build ./....Success Metrics
.github/workflowsliteral sites.Historical Context
Strategy Performance
Cumulative Statistics
Confirmed prior issue traction
FilePermSensitive/Public/DirPermSensitive/Publicnow present atpkg/constants/constants.go:73-91. ✅Recommendations
Immediate
GetWorkflowDircontract + sweep) — high impact, fixes a doc/code contract bug.Longer-term
ValidatableTool) when refactoring permission logic; consider inlining if no second impl materializes within a release cycle.GH_AW_*env-var literal sprawl is partially addressed byconstants.EnvVar*; consider a one-off lint rule to fail-CI on new"GH_AW_*"literals outsidepkg/constants/.Next Run Preview
Suggested focus areas
errors.New— gather allerrors.Newcalls in pkg/; identify candidates for package-levelErr*sentinels.logger.New("<pkg>:<file>"); check for typos / drift in 200+ call sites.Strategy evolution
Default*/Get*helper inpkg/constants/andpkg/*util/next.Generated by Sergo - The Serena Go Expert
Run ID: 25842525997
Strategy: string-literal-centralization-plus-interface-pollution-audit
Beta Was this translation helpful? Give feedback.
All reactions