Adopt isPermissionError helper for gh CLI auth-error detection (sergo)

[github-actions]

Summary

Four production sites in pkg/cli reimplement strings.Contains(err.Error(), "exit status 4") to detect gh-CLI authentication errors, while a centralized helper isPermissionError already exists at pkg/cli/audit.go:250. Duplicated, divergent substring checks make the auth-error contract fragile: each site may include different supporting substrings ("authentication required", "GH_TOKEN", "not logged into any GitHub hosts", etc.), so behavior can drift across commands.

Existing centralized helper

// pkg/cli/audit.go:250
func isPermissionError(err error) bool {
	if err == nil {
		return false
	}
	errStr := err.Error()
	return strings.Contains(errStr, "authentication required") ||
		strings.Contains(errStr, "exit status 4") ||
		strings.Contains(errStr, "GitHub CLI authentication") ||
		strings.Contains(errStr, "permission") ||
		strings.Contains(errStr, "GH_TOKEN")
}

Call sites that should adopt the helper

File:Line	Current code	Notes
pkg/cli/logs_download.go:312	if strings.Contains(err.Error(), "exit status 4")	Only checks one of the five substrings — narrower than the helper
pkg/cli/logs_download.go:802	Same pattern, second site in same file
pkg/cli/logs_github_api.go:286-291	Five substring checks inlined	Has its own multi-substring policy that diverges from isPermissionError (e.g. checks "To use GitHub CLI in a GitHub Actions workflow" which the helper doesn't); reconcile and route through the helper

Related narrower checks worth consolidating:

File:Line	Current code	Suggested
pkg/cli/secrets.go:113	if !strings.Contains(err.Error(), "403")	Consider isPermissionError(err) or a sibling is403Error helper
pkg/cli/mcp_secrets.go:49	if strings.Contains(err.Error(), "403")	Same

Why

• Helper-underadoption failure mode documented in prior Sergo runs (8/9/10/11) — pattern persistent across the codebase.
• Duplicate substring lists drift independently: logs_github_api.go:286-291 already includes substrings the helper does not (auth-required messages specific to GitHub Actions workflow context).
• A single helper lets enrichGHError evolve (e.g. switch to *exec.ExitError + ExitCode() extraction) once and benefit every consumer.

Suggested fix

1. Move/widen isPermissionError so its substring set is the union of what callers need (or split into composable helpers: isAuthError, is403Error).
2. Replace the three direct sites above to call the helper.
3. Optionally: in enrichGHError (already at pkg/workflow/github_cli.go:81), extract the gh CLI exit code via errors.As(err, &exitErr) and store it on a structured error, so consumers can branch on errors.As instead of substring at all. Lower priority.

Validation

• All current callers of the three sites above continue to return the same user-facing message.
• go test ./pkg/cli/... passes.
• No new occurrences of strings.Contains(err.Error(), "exit status 4") outside the helper.

Severity

Medium — no user-visible bug today, but each new gh-CLI call site risks drifting from the contract, and the inline substring lists are already inconsistent across files.

Context

Found by Sergo Run 12 — helper-adoption follow-up + error-comparison audit. Run §25981825933.

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

• expires on May 24, 2026, 5:10 AM UTC