[sergo] panicinlibrarycode analyzer registered but not enforced in CI (#aw_sg17a1)

[github-actions]

Summary

The custom Go analyzer panicinlibrarycode (pkg/linters/panic-in-library-code/) is registered in cmd/linters/main.go:43 and would fire on any non-test panic() call under pkg/. However, the CI step in .github/workflows/cgo.yml:1040 runs it with LINTER_FLAGS="-errstringmatch -test=false", which positively selects only the errstringmatch analyzer. The inline comment in cgo.yml acknowledges the situation: "Enforce the production errstringmatch analyzer without blocking on unrelated legacy custom analyzer findings in tests or other analyzer families."

Result: panicinlibrarycode is shipped, tested (analyzer self-test passes), but silently un-enforced. There are 16 production panic() sites in pkg/ today; none carry //nolint:panicinlibrarycode. Any new control-flow panic introduced into prod will not be flagged by CI.

This is #aw_sg17a1, identified in Sergo Run 17.

Evidence

• Linter source: pkg/linters/panic-in-library-code/panic-in-library-code.go:18 registers Analyzer with doc "reports panic() calls in library code under pkg/ that should return errors instead". Skips test files and cmd/ packages.
• Registration: cmd/linters/main.go:43 — panicinlibrarycode.Analyzer.
• CI invocation: .github/workflows/cgo.yml:1040 — make golint-custom LINTER_FLAGS="-errstringmatch -test=false" (only errstringmatch enabled as a blocking gate).
• Serena LSP cross-check: find_referencing_symbols on Analyzer returns exactly 2 references — cmd/linters/main.go:42 (registration) and panic-in-library-code_test.go:14 (analyzer self-test). No other CI or build integration.
• Production violations (16 sites — grep -rn "panic(" pkg/ --include="*.go" | grep -v _test.go | grep -v testdata):
  • pkg/actionpins/actionpins.go:117 — sync.Once load of embedded action pins JSON
  • pkg/parser/virtual_fs.go:29, :38 — registration preconditions
  • pkg/testutil/tempdir.go:33, :42 — test-runs directory creation (testutil package, not in _test.go)
  • pkg/workflow/agentic_engine.go:463 — init() engine registration
  • pkg/workflow/cache.go:48 — defence-in-depth precondition on cacheID format (well documented)
  • pkg/workflow/claude_tools.go:179 — precondition in computeAllowedClaudeToolsString
  • pkg/workflow/domains.go:324 — embedded JSON load; :785 — BUG marker (validation should have caught)
  • pkg/workflow/engine_definition_loader.go:137 — embed walk
  • pkg/workflow/gh_cli_permissions.go:82, :141 — JSON load + invalid pattern
  • pkg/workflow/github_tool_to_toolset.go:23 — JSON load
  • pkg/workflow/model_aliases.go:84 — embedded JSON parse
  • pkg/workflow/permissions_toolset_data.go:44 — JSON load
  • pkg/workflow/pi_engine.go:139 — BUG marker (unreachable json.Marshal)
  • pkg/workflow/strings.go:176 — crypto/rand failure (fatal)

Most sites are idiomatic Go patterns (lazy sync.Once init-load of embedded data, BUG markers on logically-unreachable error paths, defence-in-depth preconditions). The linter does not differentiate these from genuine control-flow panics.

Severity

High (process/quality risk, not a runtime bug). A linter that exists but doesn't run is a maintenance liability — it suggests intent to enforce a rule, while the rule rots.

Recommendation

Choose one resolution; both unblock the analyzer for inclusion in the CI blocking gate:

Option A — Refine the linter (recommended): Teach panicinlibrarycode to skip the idiomatic patterns already established in this repo:

1. Panics inside *ast.FuncLit invoked from sync.Once.Do — these are lazy init.
2. Panics where the message starts with BUG: (or bug:) — explicit unreachable markers.
3. Panics in init() functions — registration failures are fatal at startup.
4. Panics in functions whose doc comment contains // Panics on or similar precondition contract.

This is consistent with how errstringmatch and largefunc were tuned for in-codebase reality.

Option B — Annotate: Add //nolint:panicinlibrarycode // <justification> above each of the 16 sites, then enable the analyzer in CI by changing the flag set in cgo.yml:1040 to include -panicinlibrarycode (or remove the positive selector and add -test=false exclusions for specific test-adjacent packages).

Option C — Retire: If the project no longer wants this rule, remove the linter package and its registration. Keeping unenforced rules invites confusion.

Validation

• Apply chosen resolution.
• Run make golint-custom locally with the new flag set; confirm zero findings on ./pkg/....
• Update .github/workflows/cgo.yml:1040 to include panicinlibrarycode in the blocking gate.
• Add a test under pkg/linters/panic-in-library-code/testdata/ covering the new skip patterns (Option A) or confirm existing tests still pass (Option B/C).

Related

• Established pattern (R7-R14): linter authors → CI integration → (nolint/redacted) annotations for defensible sites. Examples: errstringmatch (3 refactors + 4 (nolint/redacted) in R14), manualmutexunlock (R13 audit), excessivefuncparams (R6 #aw_sg6p2).
• This finding is structurally identical to the errstringmatch resolution from Run 14: the linter exists, but findings need to be classified as defensible vs not before CI can enforce it.

References:

• §26352428711

Generated by 🤖 Sergo - Serena Go Expert · ● opu47 18.5M · ◷

• expires on May 31, 2026, 5:13 AM UTC

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · ● hai45 28.8K · ◷