Skip to content

Enforce-readiness: goroutinemissingrecover and trimleftright are clean, type/AST-resolved, zero production violations #49266

Description

@github-actions

Summary

Two recently-added, non-enforced custom analyzers are clean against the current codebase and ready to add to the CI gate (.github/workflows/cgo.yml:1362/1365, LINTER_FLAGS).

Findings

1. goroutinemissingrecover (pkg/linters/goroutinemissingrecover/goroutinemissingrecover.go)

Flags go func() { ... }() bodies lacking a top-level defer+recover() guard. Manually audited all 10 non-test go func() launch sites in pkg/:

Audited sites (all compliant)
  • pkg/cli/mcp_tools_privileged.go:77,306
  • pkg/cli/update_check.go:267
  • pkg/cli/bootstrap_profile_github_app.go:217
  • pkg/cli/forecast_compute.go:232,251
  • pkg/cli/docker_images.go:178
  • pkg/cli/compile_update_check.go:67
  • pkg/cli/bootstrap_profile_helpers.go:360
  • pkg/console/spinner.go:192

Every site already installs defer func() { if r := recover(); r != nil { ... } }() (or wraps it via defer close(...)+recover pattern) as a direct top-level statement of the goroutine body. Zero violations.

The analyzer's own AST logic is also sound: it correctly refuses to descend into nested *ast.FuncLit when searching for a recover() call (so a recover buried in an inner closure doesn't falsely count as protecting the outer goroutine — see containsRecoverCall), and it resolves the builtin via pass.TypesInfo.Uses[ident].(*types.Builtin) rather than name-matching, so a user-shadowed recover identifier isn't mistaken for the real builtin (verified in testdata/src/b/b.go).

2. trimleftright (pkg/linters/trimleftright/trimleftright.go)

Flags strings.TrimLeft/strings.TrimRight calls with a multi-character literal cutset (the classic "treats each character independently, not as a substring" Go gotcha). Grepped every non-test strings.TrimLeft(/strings.TrimRight( call site in pkg/ (~80 call sites across pkg/cli, pkg/workflow, pkg/parser, pkg/stringutil, pkg/console): every cutset is either a single rune, whitespace (" \t", "\r\n"), a path separator ("/", "-", string(filepath.Separator)), or a pure-digit set ("0123456789") — all of which the analyzer's looksSuspiciousCutset correctly treats as intentional character-class trimming (excepted) rather than a TrimPrefix/TrimSuffix mixup. Zero violations.

Recommendation

Add both -goroutinemissingrecover and -trimleftright to the LINTER_FLAGS list in .github/workflows/cgo.yml (both the default-build gate at line 1362 and the GOOS=js GOARCH=wasm gate at line 1365), following the same pattern used for the last enforce-readiness batch (appendoneelement, timenowsub, stringsjoinone, closed in #47916).

Validation checklist

  • Confirm make golint-custom LINTER_FLAGS="-goroutinemissingrecover -trimleftright -test=false" reports zero findings on ./cmd/... ./pkg/...
  • Confirm the wasm-scoped package list (./pkg/console ./pkg/parser ./pkg/styles ./pkg/tty ./pkg/workflow) also reports zero findings under GOOS=js GOARCH=wasm
  • Add both flags to both cgo.yml invocations
  • Update any enforce-readiness tracking doc/comment listing currently-enforced analyzers

Effort

Small — CI config change plus a verification run; no source changes expected.

Generated by 🤖 Sergo - Serena Go Expert · agent · 250.3 AIC · ⌖ 6.22 AIC · ⊞ 6K ·

  • expires on Aug 6, 2026, 9:04 PM UTC-08:00

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions