Skip to content

Enforce panicinlibrarycode in CI and tune it for accepted repo patterns#34374

Merged
pelikhan merged 2 commits into
mainfrom
copilot/fix-panic-in-library-code-analyzer
May 24, 2026
Merged

Enforce panicinlibrarycode in CI and tune it for accepted repo patterns#34374
pelikhan merged 2 commits into
mainfrom
copilot/fix-panic-in-library-code-analyzer

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 24, 2026

panicinlibrarycode was registered and tested, but the cgo custom-linter gate only selected errstringmatch, so the rule never blocked new production panic() calls. This updates the analyzer and CI wiring so the rule is enforced without tripping on established, intentional panic patterns already used in pkg/.

  • Enable the analyzer in the blocking CI gate

    • Update .github/workflows/cgo.yml to run panicinlibrarycode alongside errstringmatch in make golint-custom.
  • Refine panicinlibrarycode to match repo reality

    • Skip panics inside sync.Once.Do(func() { ... }) lazy-init closures.
    • Skip panics whose message starts with BUG:.
    • Skip panics in init() functions.
    • Skip panics in functions whose doc comments explicitly declare a panic contract.
  • Add focused analyzer coverage

    • Extend linter test fixtures to cover the newly allowed patterns while preserving existing diagnostics for ordinary library panic() calls.
  • Document intentional panic contracts where needed

    • Add brief doc-comment panic contracts to the small number of intentional sites that are neither sync.Once, BUG:, nor init() based, so they are explicit in code and recognized by the analyzer.

Example of the newly allowed contract pattern:

// computeAllowedClaudeToolsString ...
// Panics if callers pass a Claude-specific tools section instead of neutral tools.
func (e *ClaudeEngine) computeAllowedClaudeToolsString(...) string {
	if _, hasClaudeSection := tools["claude"]; hasClaudeSection {
		panic("computeAllowedClaudeToolsString should only receive neutral tools, not claude section tools")
	}
	// ...
}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix panicinlibrarycode analyzer enforcement in CI Enforce panicinlibrarycode in CI and tune it for accepted repo patterns May 24, 2026
Copilot AI requested a review from gh-aw-bot May 24, 2026 05:46
@pelikhan pelikhan marked this pull request as ready for review May 24, 2026 06:51
Copilot AI review requested due to automatic review settings May 24, 2026 06:51
@pelikhan pelikhan merged commit 0defa4d into main May 24, 2026
@pelikhan pelikhan deleted the copilot/fix-panic-in-library-code-analyzer branch May 24, 2026 06:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR wires the panicinlibrarycode analyzer into the blocking CI custom-linter gate and updates the analyzer so it can enforce “no new panic() in pkg/” without flagging a set of intentional, established patterns (lazy init via sync.Once, BUG: panics, init() panics, and explicitly documented panic contracts).

Changes:

  • Enforce panicinlibrarycode in the cgo CI workflow by running it alongside errstringmatch in make golint-custom.
  • Refine panicinlibrarycode to skip specific accepted panic patterns (sync.Once closures, BUG:-prefixed messages, init() functions, and documented panic contracts).
  • Add/extend analyzer fixtures and add doc-comment panic contracts on intentional panic sites in pkg/workflow.
Show a summary per file
File Description
pkg/workflow/strings.go Adds a doc-comment panic contract for a crypto/rand failure panic path so the analyzer can allow it.
pkg/workflow/model_aliases.go Documents an intentional panic when embedded JSON is invalid to satisfy the analyzer’s contract-based exemption.
pkg/workflow/claude_tools.go Documents a precondition panic contract for the “neutral tools only” invariant.
pkg/workflow/agentic_engine.go Documents an intentional panic for invalid built-in engine registration.
pkg/linters/panic-in-library-code/testdata/src/panicinlibrarycode/panicinlibrarycode.go Extends fixtures to cover the newly allowed panic patterns.
pkg/linters/panic-in-library-code/panic-in-library-code.go Implements the new skip logic using inspect.WithStack plus helpers for the allowed patterns.
.github/workflows/cgo.yml Updates CI to include -panicinlibrarycode in the custom-linters gate.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 7/7 changed files
  • Comments generated: 2

Comment on lines +158 to +162
case *ast.CallExpr:
if len(e.Args) == 0 {
return "", false
}
return stringPrefix(pass, e.Args[0])

func isInInitFunction(stack []ast.Node) bool {
decl := enclosingFuncDecl(stack)
return decl != nil && decl.Name != nil && decl.Name.Name == "init"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

4 participants