errorfwrapv/fmterrorfnoverbs BasicLit-only format-string blind spot still hides cacheRecoveryError's %v-not-%w bug (expired issu

[github-actions]

Overview

This exact bug was filed as #56533 by a prior Sergo run and auto-expired unfixed on 2026-09-03 (state_reason: not_planned) - the self-expiry checkbox lapsed with zero code changes. Re-verified today (2026-09-05): both the production bug and the linter blind spot that hides it are still present, unchanged.

Confirmed still-broken production code

pkg/cli/audit_run_pipeline.go:317-325:

func cacheRecoveryError(message string, runID int64, runOutputDir string, err error) error {
	return fmt.Errorf(message+"\n\n"+
		"To download artifacts, use the GitHub MCP server:\n\n"+
		"1. Use the github-mcp-server tool 'download_workflow_run_artifacts' with:\n"+
		"   - run_id: %d\n"+
		"   - output_directory: %s\n\n"+
		"2. After downloading, run this audit command again to analyze the cached artifacts.\n\n"+
		"Original error: %v", runID, runOutputDir, err)
}

err is still formatted with %v (line 324), not %w, so the returned error breaks the chain. This matters concretely here: the same file already relies on errors.Is(err, ErrNoArtifacts) at lines 281 and 283 to drive control flow for the other branch of this same recovery path, so callers of cacheRecoveryError's two call sites (lines 267-269, 296) reasonably expect the same chain-inspectable behavior and won't get it.

Why the linters still can't catch it

Both pkg/linters/errorfwrapv/errorfwrapv.go:70-73 and pkg/linters/fmterrorfnoverbs/fmterrorfnoverbs.go:45-48 extract the format string identically:

lit, ok := call.Args[0].(*ast.BasicLit)
if !ok || lit.Kind != token.STRING {
    return
}

cacheRecoveryError's format string is message+"...", a *ast.BinaryExpr (string concatenation), not a *ast.BasicLit, so both analyzers bail out before inspecting verbs or arguments. A repo-wide grep for concatenated fmt.Errorf(ident+"..." / fmt.Errorf("..."+ calls under pkg/ (non-test) turns up 6 production call sites; cacheRecoveryError is confirmed as the only one that both (a) uses a concatenated, non-literal format string and (b) trails a %v-formatted error argument - the other 5 already correctly use %w inside a concatenated literal, so today they're just invisible to the linter rather than actively wrong (a second-order enforcement-readiness gap, not a live bug).

Impact

1. Real bug, unresolved for the second review cycle: cacheRecoveryError's wrapped error loses chain identity; any future caller of the permission/cache-recovery path that tries errors.Is/errors.As on its result will silently fail to match, exactly mirroring the pattern the surrounding code already depends on.
2. Linter class gap spans two analyzers, not one: both errorfwrapv and fmterrorfnoverbs share the identical Args[0].(*ast.BasicLit) extraction, so a shared fix belongs in one place rather than being patched per-file, or the next concatenated-format-string call added anywhere in pkg/ will hit the same blind spot again in both directions (missed %w violations AND missed "no verbs, use errors.New" opportunities).

Recommendation

1. Fix cacheRecoveryError to use %w for the trailing err argument (single-line change, message rendering unaffected).
2. Harden format-string extraction to also resolve constant-foldable string concatenation (walk *ast.BinaryExpr chains of token.ADD over string literals/identifiers) - ideally as one shared helper in pkg/linters/internal/astutil that both errorfwrapv and fmterrorfnoverbs call, since they need the identical resolution logic.
3. Re-run the enforcement audit for both linters after the fix lands, before adding either to cgo.yml LINTER_FLAGS.

Validation checklist

• cacheRecoveryError wraps err with %w
• New shared helper (or per-linter fix) resolves concatenated format strings in both errorfwrapv and fmterrorfnoverbs
• New testdata case per linter covering a message + "literal"-style format string with a trailing error argument / no-verb string
• pkg/cli/audit_run_pipeline_test.go gains a case asserting errors.Is/errors.As works through cacheRecoveryError's result
• go test ./pkg/linters/... ./pkg/cli/... passes

Effort

Small-to-medium: a one-line %w fix in pkg/cli, plus a shared format-string-concatenation resolver used by two analyzer files, with matching testdata for both.

---

Filed by Sergo (automated Go static-analysis review) - re-filed after #56533 auto-expired unresolved.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.anthropic.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.anthropic.com"

See Network Configuration for more information.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 271 AIC · ⌖ 8.31 AIC · ⊞ 6.8K · ◷

• expires on Sep 11, 2026, 7:57 PM UTC-08:00