← back to Cli Printing Press
fix(cli): map Cobra/pflag pre-RunE usage errors to exit code 2 (#1194)
dabf63f8753eeddc10a72b6b74f37a713f137e3c · 2026-05-12 12:18:14 -0400 · klatt42
* fix(cli): map Cobra/pflag pre-RunE usage errors to exit code 2
The emitted helpers.go documents `usageErr → code 2` per POSIX
convention, but Cobra and pflag generate usage errors (unknown flag,
unknown command, missing required flag, missing flag value, invalid
argument) inside rootCmd.Execute() before any user RunE runs. Those
errors never flow through usageErr, so ExitCode falls through to the
default and emits 1 — silently breaking the contract.
Adds isCobraUsageError(err) to detect the six pre-RunE shapes by
message prefix/substring (Cobra/pflag don't export typed sentinels)
and routes matching errors through usageErr in Execute(). The existing
unknown-flag hint-suggestion path was switched from `return ...` to
`err = ...` so the rewrapped error falls through to the usage wrap;
the test pins that switch and confirms hint-rewrapped errors still
classify as usage errors.
Tests:
- helpers_test.go.tmpl ships unit tests in every printed CLI:
table-driven coverage of all six error shapes, the hint-rewrap
pass-through, and the end-to-end Execute→ExitCode contract
(wrapped error yields code 2).
- usage_error_exit_test.go in upstream asserts the structural
contract: helper emits, Execute wires it, hint path was switched
to assignment.
End-to-end verified on a freshly generated CLI:
./loops-pp-cli --foobar → exit 2 (was 1)
./loops-pp-cli notarealsubcommand → exit 2 (was 1)
./loops-pp-cli --help → exit 0 (unchanged)
Discovered during the rok-brain-cli pilot where bad-flag exit codes
came back as 1 in the smoke test. PILOT_NOTES called this out as
"plan expected 2 per Cobra conventions"; the convention was already
in helpers.go, just bypassed by Cobra's own error path.
Note on file-count overlap with #1128: this PR bumps
expectedFiles +1 vs main (58→59 stytch, 63→64 clerk, 60→61 loops)
and adds internal/cli/root_test.go to mustInclude. PR #1128 makes
the same +1 bump for internal/cli/helpers_test.go. Whichever lands
first leaves the second to rebase to +2.
* fix(cli): anchor isCobraUsageError patterns to Cobra/pflag punctuation
Addresses Greptile P2 on #1194.
The three Contains/HasPrefix patterns for "required flag", "flag needs
an argument", and "invalid argument" were broader than necessary —
they would match an application's own RunE error that happened to
contain those substrings as prose. Cobra and pflag emit these errors
with specific punctuation that anchors the prefix unambiguously:
- required flag(s) "foo" not set → starts with `required flag(s) "`
- flag needs an argument: --foo → starts with `flag needs an argument:`
- invalid argument "x" for "--y" → starts with `invalid argument "`
The tighter patterns still match every real Cobra/pflag output (the
existing positive test cases continue to pass). Three new negative
test cases lock the tightness — application messages containing the
old substrings as prose now correctly classify as non-usage errors.
Files touched
M internal/generator/generator.goM internal/generator/generator_test.goM internal/generator/templates/root.go.tmplA internal/generator/templates/root_test.go.tmplA internal/generator/usage_error_exit_test.goM testdata/golden/cases/generate-golden-api-rich-auth/artifacts.txtM testdata/golden/cases/generate-golden-api/artifacts.txtA testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/root_test.goM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.goA testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root_test.go
Diff
commit dabf63f8753eeddc10a72b6b74f37a713f137e3c
Author: klatt42 <134792614+klatt42@users.noreply.github.com>
Date: Tue May 12 12:18:14 2026 -0400
fix(cli): map Cobra/pflag pre-RunE usage errors to exit code 2 (#1194)
* fix(cli): map Cobra/pflag pre-RunE usage errors to exit code 2
The emitted helpers.go documents `usageErr → code 2` per POSIX
convention, but Cobra and pflag generate usage errors (unknown flag,
unknown command, missing required flag, missing flag value, invalid
argument) inside rootCmd.Execute() before any user RunE runs. Those
errors never flow through usageErr, so ExitCode falls through to the
default and emits 1 — silently breaking the contract.
Adds isCobraUsageError(err) to detect the six pre-RunE shapes by
message prefix/substring (Cobra/pflag don't export typed sentinels)
and routes matching errors through usageErr in Execute(). The existing
unknown-flag hint-suggestion path was switched from `return ...` to
`err = ...` so the rewrapped error falls through to the usage wrap;
the test pins that switch and confirms hint-rewrapped errors still
classify as usage errors.
Tests:
- helpers_test.go.tmpl ships unit tests in every printed CLI:
table-driven coverage of all six error shapes, the hint-rewrap
pass-through, and the end-to-end Execute→ExitCode contract
(wrapped error yields code 2).
- usage_error_exit_test.go in upstream asserts the structural
contract: helper emits, Execute wires it, hint path was switched
to assignment.
End-to-end verified on a freshly generated CLI:
./loops-pp-cli --foobar → exit 2 (was 1)
./loops-pp-cli notarealsubcommand → exit 2 (was 1)
./loops-pp-cli --help → exit 0 (unchanged)
Discovered during the rok-brain-cli pilot where bad-flag exit codes
came back as 1 in the smoke test. PILOT_NOTES called this out as
"plan expected 2 per Cobra conventions"; the convention was already
in helpers.go, just bypassed by Cobra's own error path.
Note on file-count overlap with #1128: this PR bumps
expectedFiles +1 vs main (58→59 stytch, 63→64 clerk, 60→61 loops)
and adds internal/cli/root_test.go to mustInclude. PR #1128 makes
the same +1 bump for internal/cli/helpers_test.go. Whichever lands
first leaves the second to rebase to +2.
* fix(cli): anchor isCobraUsageError patterns to Cobra/pflag punctuation
Addresses Greptile P2 on #1194.
The three Contains/HasPrefix patterns for "required flag", "flag needs
an argument", and "invalid argument" were broader than necessary —
they would match an application's own RunE error that happened to
contain those substrings as prose. Cobra and pflag emit these errors
with specific punctuation that anchors the prefix unambiguously:
- required flag(s) "foo" not set → starts with `required flag(s) "`
- flag needs an argument: --foo → starts with `flag needs an argument:`
- invalid argument "x" for "--y" → starts with `invalid argument "`
The tighter patterns still match every real Cobra/pflag output (the
existing positive test cases continue to pass). Three new negative
test cases lock the tightness — application messages containing the
old substrings as prose now correctly classify as non-usage errors.
---
internal/generator/generator.go | 1 +
internal/generator/generator_test.go | 7 +-
internal/generator/templates/root.go.tmpl | 43 +++++++++++-
internal/generator/templates/root_test.go.tmpl | 77 ++++++++++++++++++++++
internal/generator/usage_error_exit_test.go | 55 ++++++++++++++++
.../generate-golden-api-rich-auth/artifacts.txt | 1 +
.../golden/cases/generate-golden-api/artifacts.txt | 1 +
.../internal/cli/root_test.go | 77 ++++++++++++++++++++++
.../printing-press-golden/internal/cli/root.go | 43 +++++++++++-
.../internal/cli/root_test.go | 77 ++++++++++++++++++++++
10 files changed, 377 insertions(+), 5 deletions(-)
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 9e9fadd2..9e406248 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1393,6 +1393,7 @@ func (g *Generator) renderSingleFiles() error {
singleFiles := map[string]string{
"main.go.tmpl": filepath.Join("cmd", naming.CLI(g.Spec.Name), "main.go"),
"helpers.go.tmpl": filepath.Join("internal", "cli", "helpers.go"),
+ "root_test.go.tmpl": filepath.Join("internal", "cli", "root_test.go"),
"doctor.go.tmpl": filepath.Join("internal", "cli", "doctor.go"),
"agent_context.go.tmpl": filepath.Join("internal", "cli", "agent_context.go"),
"profile.go.tmpl": filepath.Join("internal", "cli", "profile.go"),
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 381eeefa..e63960c1 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -51,6 +51,7 @@ func TestGenerateProjectsCompile(t *testing.T) {
"internal/cli/profile.go",
"internal/cli/feedback.go",
"internal/cli/agent_context.go",
+ "internal/cli/root_test.go",
"internal/cliutil/fanout.go",
"internal/cliutil/text.go",
"internal/cliutil/probe.go",
@@ -79,9 +80,9 @@ func TestGenerateProjectsCompile(t *testing.T) {
// Bump it AND add to mustInclude above when adding always-emitted
// templates. Per-spec dynamic files (per-resource command files,
// generated tests) account for the difference between fixtures.
- {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 58},
- {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 63},
- {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 60},
+ {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 59},
+ {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 64},
+ {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 61},
}
for _, tt := range tests {
diff --git a/internal/generator/templates/root.go.tmpl b/internal/generator/templates/root.go.tmpl
index 927850dc..c41075ce 100644
--- a/internal/generator/templates/root.go.tmpl
+++ b/internal/generator/templates/root.go.tmpl
@@ -71,7 +71,7 @@ func Execute() error {
if idx := strings.Index(msg, "unknown flag: "); idx >= 0 {
flagStr := strings.TrimSpace(msg[idx+len("unknown flag: "):])
if suggestion := suggestFlag(flagStr, rootCmd); suggestion != "" {
- return fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)
+ err = fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)
}
}
}
@@ -81,9 +81,50 @@ func Execute() error {
return derr
}
}
+ if err != nil && isCobraUsageError(err) {
+ // Cobra/pflag pre-RunE errors (unknown flag, unknown command,
+ // missing required, etc.) never flow through usageErr() because
+ // they originate inside rootCmd.Execute() before any user RunE
+ // runs. Without this wrap, ExitCode() falls through to the
+ // default and emits 1 — clobbering the conventional code-2 for
+ // usage errors that the helpers.go contract already promises.
+ return usageErr(err)
+ }
return err
}
+// isCobraUsageError reports whether err matches one of Cobra/pflag's
+// pre-RunE usage-error shapes. Detection is by message prefix to match
+// the same approach the unknown-flag hint path uses above; neither
+// Cobra nor pflag exports typed sentinels for these.
+//
+// Patterns are anchored to the literal punctuation Cobra and pflag
+// emit so an application's own RunE error that happens to contain the
+// substring "required flag" or "invalid argument" doesn't get
+// misclassified as a usage error.
+//
+// Patterns covered (Cobra v1.x + pflag v1.x as of 2026-05):
+// - "unknown flag: --foo" (pflag)
+// - "unknown shorthand flag: 'x' in -x" (pflag)
+// - "unknown command \"foo\" for ..." (Cobra)
+// - "required flag(s) \"foo\" not set" (Cobra MarkFlagRequired)
+// - "flag needs an argument: --foo" (pflag, missing value)
+// - "invalid argument \"x\" for \"--y\" flag: ..." (pflag, parse failure)
+//
+// Returns false for nil err.
+func isCobraUsageError(err error) bool {
+ if err == nil {
+ return false
+ }
+ msg := err.Error()
+ return strings.HasPrefix(msg, "unknown flag") ||
+ strings.HasPrefix(msg, "unknown shorthand flag") ||
+ strings.HasPrefix(msg, "unknown command") ||
+ strings.HasPrefix(msg, `required flag(s) "`) ||
+ strings.HasPrefix(msg, "flag needs an argument:") ||
+ strings.HasPrefix(msg, `invalid argument "`)
+}
+
func newRootCmd(flags *rootFlags) *cobra.Command {
rootCmd := &cobra.Command{
Use: "{{.Name}}-pp-cli",
diff --git a/internal/generator/templates/root_test.go.tmpl b/internal/generator/templates/root_test.go.tmpl
new file mode 100644
index 00000000..c40194da
--- /dev/null
+++ b/internal/generator/templates/root_test.go.tmpl
@@ -0,0 +1,77 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cli
+
+import (
+ "errors"
+ "fmt"
+ "testing"
+)
+
+// TestIsCobraUsageError covers the six pre-RunE error shapes Cobra and
+// pflag can produce before any user RunE runs. Each must be detected so
+// the caller can wrap in usageErr() and yield exit code 2.
+func TestIsCobraUsageError(t *testing.T) {
+ t.Parallel()
+ cases := []struct {
+ name string
+ errIn error
+ want bool
+ }{
+ {"nil", nil, false},
+ {"unknown flag", errors.New("unknown flag: --foob"), true},
+ {"unknown shorthand flag", errors.New("unknown shorthand flag: 'x' in -x"), true},
+ {"unknown command", errors.New("unknown command \"foo\" for \"cli\""), true},
+ {"required flag", errors.New("required flag(s) \"query\" not set"), true},
+ {"flag needs argument", errors.New("flag needs an argument: --query"), true},
+ {"invalid argument", errors.New("invalid argument \"abc\" for \"--limit\" flag: strconv.ParseInt: parsing \"abc\": invalid syntax"), true},
+ // Non-usage errors must NOT be flagged — they should retain their
+ // original (or wrapped *cliError) exit code.
+ {"API error pass-through", errors.New("API returned 500"), false},
+ {"network error pass-through", errors.New("dial tcp: connection refused"), false},
+ // Pattern-tightness regressions: application RunE errors that
+ // happen to mention "required flag" / "flag needs an argument" /
+ // "invalid argument" as prose must NOT classify as usage errors.
+ // The anchored prefixes guard against this; if the patterns are
+ // ever loosened back to Contains() these tests catch the drift.
+ {"app msg containing 'required flag' as prose", errors.New("the required flag config is missing from the manifest"), false},
+ {"app msg containing 'flag needs an argument' as prose", errors.New("this flag needs an argument upstream before retry"), false},
+ {"app msg containing 'invalid argument' as prose", errors.New("invalid argument provided to handler at /foo"), false},
+ }
+ for _, tc := range cases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ if got := isCobraUsageError(tc.errIn); got != tc.want {
+ t.Errorf("isCobraUsageError(%v) = %v, want %v", tc.errIn, got, tc.want)
+ }
+ })
+ }
+}
+
+// TestIsCobraUsageError_SurvivesHintRewrap covers the hint-suggestion
+// path in Execute(): after rewrapping an "unknown flag" error with
+// fmt.Errorf("%w\nhint: ...", err, ...), the rewrapped error must still
+// classify as a usage error so the exit code stays at 2 rather than
+// silently falling through to 1.
+func TestIsCobraUsageError_SurvivesHintRewrap(t *testing.T) {
+ t.Parallel()
+ original := errors.New("unknown flag: --foob")
+ wrapped := fmt.Errorf("%w\nhint: did you mean --foo?", original)
+ if !isCobraUsageError(wrapped) {
+ t.Errorf("hint-rewrapped unknown-flag error must still classify as usage error, got: %v", wrapped)
+ }
+}
+
+// TestExitCode_UsageError_WrappedAsCode2 covers the end-to-end contract:
+// after Execute() wraps a Cobra usage error in usageErr(), ExitCode()
+// must return 2, not 1. This is the user-visible promise — a bad flag
+// exits 2 (POSIX usage convention), not the generic-error-1 fallback.
+func TestExitCode_UsageError_WrappedAsCode2(t *testing.T) {
+ t.Parallel()
+ wrapped := usageErr(errors.New("unknown flag: --foob"))
+ if got := ExitCode(wrapped); got != 2 {
+ t.Errorf("ExitCode(usageErr(...)) = %d, want 2 (POSIX usage convention)", got)
+ }
+}
diff --git a/internal/generator/usage_error_exit_test.go b/internal/generator/usage_error_exit_test.go
new file mode 100644
index 00000000..ec3b75a2
--- /dev/null
+++ b/internal/generator/usage_error_exit_test.go
@@ -0,0 +1,55 @@
+package generator
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestUsageErrorExitCode_EmittedInRoot asserts the generator emits the
+// Cobra usage-error wrap in every printed CLI: the isCobraUsageError
+// helper is defined, Execute() wraps Cobra/pflag pre-RunE errors in
+// usageErr() before they reach ExitCode(), and the hint-suggestion
+// rewrap preserves the original error chain (so isCobraUsageError still
+// classifies it after the wrap).
+//
+// Behavior is exercised by the unit tests in root_test.go.tmpl, which
+// ship alongside root.go in every generated CLI. This canary just
+// proves the structural contract — the helper exists, the wrap is
+// wired, and the hint path was switched from `return ...` to `err = ...`.
+func TestUsageErrorExitCode_EmittedInRoot(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("usage-exit-canary")
+ outputDir := filepath.Join(t.TempDir(), "usage-exit-canary-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ rootSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "root.go"))
+ require.NoError(t, err, "root.go must be emitted")
+ src := string(rootSrc)
+
+ require.Contains(t, src, "func isCobraUsageError(",
+ "isCobraUsageError helper must be emitted into the printed CLI")
+ require.Contains(t, src, "if err != nil && isCobraUsageError(err) {",
+ "Execute() must wrap Cobra usage errors before returning")
+ require.Contains(t, src, "return usageErr(err)",
+ "the wrap must route through usageErr so ExitCode yields 2")
+
+ // The hint-suggestion rewrap path was switched from `return
+ // fmt.Errorf(...)` to `err = fmt.Errorf(...)` so the rewrapped
+ // error falls through to the usage wrap below. Pin that switch.
+ require.NotContains(t, src, `return fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)`,
+ "hint path must assign to err so the usage wrap can catch the rewrapped error")
+ require.Contains(t, src, `err = fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)`,
+ "hint path must assign to err (not early-return) so the usage wrap runs after")
+
+ // Test file must ship alongside so the consumer's `go test` exercises
+ // the behavior end-to-end.
+ testPath := filepath.Join(outputDir, "internal", "cli", "root_test.go")
+ testSrc, err := os.ReadFile(testPath)
+ require.NoError(t, err, "root_test.go must ship into the generated CLI")
+ require.Contains(t, string(testSrc), "TestExitCode_UsageError_WrappedAsCode2",
+ "emitted test file must cover the wrap → code-2 contract")
+}
diff --git a/testdata/golden/cases/generate-golden-api-rich-auth/artifacts.txt b/testdata/golden/cases/generate-golden-api-rich-auth/artifacts.txt
index 117fb898..31c512ec 100644
--- a/testdata/golden/cases/generate-golden-api-rich-auth/artifacts.txt
+++ b/testdata/golden/cases/generate-golden-api-rich-auth/artifacts.txt
@@ -4,6 +4,7 @@ printing-press-rich-auth/README.md
printing-press-rich-auth/AGENTS.md
printing-press-rich-auth/internal/cli/agent_context.go
printing-press-rich-auth/internal/cli/auth.go
+printing-press-rich-auth/internal/cli/root_test.go
printing-press-rich-auth/internal/cli/doctor.go
printing-press-rich-auth/internal/cli/helpers.go
printing-press-rich-auth/internal/config/config.go
diff --git a/testdata/golden/cases/generate-golden-api/artifacts.txt b/testdata/golden/cases/generate-golden-api/artifacts.txt
index 678df8d8..c5ebc37c 100644
--- a/testdata/golden/cases/generate-golden-api/artifacts.txt
+++ b/testdata/golden/cases/generate-golden-api/artifacts.txt
@@ -5,6 +5,7 @@ printing-press-golden/cmd/printing-press-golden-pp-cli/main.go
printing-press-golden/internal/client/client.go
printing-press-golden/internal/cli/helpers.go
printing-press-golden/internal/cli/root.go
+printing-press-golden/internal/cli/root_test.go
printing-press-golden/internal/cli/auth.go
printing-press-golden/internal/cli/projects_list.go
printing-press-golden/internal/cli/projects_create.go
diff --git a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/root_test.go b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/root_test.go
new file mode 100644
index 00000000..d04efeed
--- /dev/null
+++ b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/root_test.go
@@ -0,0 +1,77 @@
+// Copyright 2026 printing-press-golden. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cli
+
+import (
+ "errors"
+ "fmt"
+ "testing"
+)
+
+// TestIsCobraUsageError covers the six pre-RunE error shapes Cobra and
+// pflag can produce before any user RunE runs. Each must be detected so
+// the caller can wrap in usageErr() and yield exit code 2.
+func TestIsCobraUsageError(t *testing.T) {
+ t.Parallel()
+ cases := []struct {
+ name string
+ errIn error
+ want bool
+ }{
+ {"nil", nil, false},
+ {"unknown flag", errors.New("unknown flag: --foob"), true},
+ {"unknown shorthand flag", errors.New("unknown shorthand flag: 'x' in -x"), true},
+ {"unknown command", errors.New("unknown command \"foo\" for \"cli\""), true},
+ {"required flag", errors.New("required flag(s) \"query\" not set"), true},
+ {"flag needs argument", errors.New("flag needs an argument: --query"), true},
+ {"invalid argument", errors.New("invalid argument \"abc\" for \"--limit\" flag: strconv.ParseInt: parsing \"abc\": invalid syntax"), true},
+ // Non-usage errors must NOT be flagged — they should retain their
+ // original (or wrapped *cliError) exit code.
+ {"API error pass-through", errors.New("API returned 500"), false},
+ {"network error pass-through", errors.New("dial tcp: connection refused"), false},
+ // Pattern-tightness regressions: application RunE errors that
+ // happen to mention "required flag" / "flag needs an argument" /
+ // "invalid argument" as prose must NOT classify as usage errors.
+ // The anchored prefixes guard against this; if the patterns are
+ // ever loosened back to Contains() these tests catch the drift.
+ {"app msg containing 'required flag' as prose", errors.New("the required flag config is missing from the manifest"), false},
+ {"app msg containing 'flag needs an argument' as prose", errors.New("this flag needs an argument upstream before retry"), false},
+ {"app msg containing 'invalid argument' as prose", errors.New("invalid argument provided to handler at /foo"), false},
+ }
+ for _, tc := range cases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ if got := isCobraUsageError(tc.errIn); got != tc.want {
+ t.Errorf("isCobraUsageError(%v) = %v, want %v", tc.errIn, got, tc.want)
+ }
+ })
+ }
+}
+
+// TestIsCobraUsageError_SurvivesHintRewrap covers the hint-suggestion
+// path in Execute(): after rewrapping an "unknown flag" error with
+// fmt.Errorf("%w\nhint: ...", err, ...), the rewrapped error must still
+// classify as a usage error so the exit code stays at 2 rather than
+// silently falling through to 1.
+func TestIsCobraUsageError_SurvivesHintRewrap(t *testing.T) {
+ t.Parallel()
+ original := errors.New("unknown flag: --foob")
+ wrapped := fmt.Errorf("%w\nhint: did you mean --foo?", original)
+ if !isCobraUsageError(wrapped) {
+ t.Errorf("hint-rewrapped unknown-flag error must still classify as usage error, got: %v", wrapped)
+ }
+}
+
+// TestExitCode_UsageError_WrappedAsCode2 covers the end-to-end contract:
+// after Execute() wraps a Cobra usage error in usageErr(), ExitCode()
+// must return 2, not 1. This is the user-visible promise — a bad flag
+// exits 2 (POSIX usage convention), not the generic-error-1 fallback.
+func TestExitCode_UsageError_WrappedAsCode2(t *testing.T) {
+ t.Parallel()
+ wrapped := usageErr(errors.New("unknown flag: --foob"))
+ if got := ExitCode(wrapped); got != 2 {
+ t.Errorf("ExitCode(usageErr(...)) = %d, want 2 (POSIX usage convention)", got)
+ }
+}
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
index 218d9a66..efa2ef80 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root.go
@@ -65,7 +65,7 @@ func Execute() error {
if idx := strings.Index(msg, "unknown flag: "); idx >= 0 {
flagStr := strings.TrimSpace(msg[idx+len("unknown flag: "):])
if suggestion := suggestFlag(flagStr, rootCmd); suggestion != "" {
- return fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)
+ err = fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)
}
}
}
@@ -75,9 +75,50 @@ func Execute() error {
return derr
}
}
+ if err != nil && isCobraUsageError(err) {
+ // Cobra/pflag pre-RunE errors (unknown flag, unknown command,
+ // missing required, etc.) never flow through usageErr() because
+ // they originate inside rootCmd.Execute() before any user RunE
+ // runs. Without this wrap, ExitCode() falls through to the
+ // default and emits 1 — clobbering the conventional code-2 for
+ // usage errors that the helpers.go contract already promises.
+ return usageErr(err)
+ }
return err
}
+// isCobraUsageError reports whether err matches one of Cobra/pflag's
+// pre-RunE usage-error shapes. Detection is by message prefix to match
+// the same approach the unknown-flag hint path uses above; neither
+// Cobra nor pflag exports typed sentinels for these.
+//
+// Patterns are anchored to the literal punctuation Cobra and pflag
+// emit so an application's own RunE error that happens to contain the
+// substring "required flag" or "invalid argument" doesn't get
+// misclassified as a usage error.
+//
+// Patterns covered (Cobra v1.x + pflag v1.x as of 2026-05):
+// - "unknown flag: --foo" (pflag)
+// - "unknown shorthand flag: 'x' in -x" (pflag)
+// - "unknown command \"foo\" for ..." (Cobra)
+// - "required flag(s) \"foo\" not set" (Cobra MarkFlagRequired)
+// - "flag needs an argument: --foo" (pflag, missing value)
+// - "invalid argument \"x\" for \"--y\" flag: ..." (pflag, parse failure)
+//
+// Returns false for nil err.
+func isCobraUsageError(err error) bool {
+ if err == nil {
+ return false
+ }
+ msg := err.Error()
+ return strings.HasPrefix(msg, "unknown flag") ||
+ strings.HasPrefix(msg, "unknown shorthand flag") ||
+ strings.HasPrefix(msg, "unknown command") ||
+ strings.HasPrefix(msg, `required flag(s) "`) ||
+ strings.HasPrefix(msg, "flag needs an argument:") ||
+ strings.HasPrefix(msg, `invalid argument "`)
+}
+
func newRootCmd(flags *rootFlags) *cobra.Command {
rootCmd := &cobra.Command{
Use: "printing-press-golden-pp-cli",
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root_test.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root_test.go
new file mode 100644
index 00000000..d04efeed
--- /dev/null
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/root_test.go
@@ -0,0 +1,77 @@
+// Copyright 2026 printing-press-golden. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cli
+
+import (
+ "errors"
+ "fmt"
+ "testing"
+)
+
+// TestIsCobraUsageError covers the six pre-RunE error shapes Cobra and
+// pflag can produce before any user RunE runs. Each must be detected so
+// the caller can wrap in usageErr() and yield exit code 2.
+func TestIsCobraUsageError(t *testing.T) {
+ t.Parallel()
+ cases := []struct {
+ name string
+ errIn error
+ want bool
+ }{
+ {"nil", nil, false},
+ {"unknown flag", errors.New("unknown flag: --foob"), true},
+ {"unknown shorthand flag", errors.New("unknown shorthand flag: 'x' in -x"), true},
+ {"unknown command", errors.New("unknown command \"foo\" for \"cli\""), true},
+ {"required flag", errors.New("required flag(s) \"query\" not set"), true},
+ {"flag needs argument", errors.New("flag needs an argument: --query"), true},
+ {"invalid argument", errors.New("invalid argument \"abc\" for \"--limit\" flag: strconv.ParseInt: parsing \"abc\": invalid syntax"), true},
+ // Non-usage errors must NOT be flagged — they should retain their
+ // original (or wrapped *cliError) exit code.
+ {"API error pass-through", errors.New("API returned 500"), false},
+ {"network error pass-through", errors.New("dial tcp: connection refused"), false},
+ // Pattern-tightness regressions: application RunE errors that
+ // happen to mention "required flag" / "flag needs an argument" /
+ // "invalid argument" as prose must NOT classify as usage errors.
+ // The anchored prefixes guard against this; if the patterns are
+ // ever loosened back to Contains() these tests catch the drift.
+ {"app msg containing 'required flag' as prose", errors.New("the required flag config is missing from the manifest"), false},
+ {"app msg containing 'flag needs an argument' as prose", errors.New("this flag needs an argument upstream before retry"), false},
+ {"app msg containing 'invalid argument' as prose", errors.New("invalid argument provided to handler at /foo"), false},
+ }
+ for _, tc := range cases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ if got := isCobraUsageError(tc.errIn); got != tc.want {
+ t.Errorf("isCobraUsageError(%v) = %v, want %v", tc.errIn, got, tc.want)
+ }
+ })
+ }
+}
+
+// TestIsCobraUsageError_SurvivesHintRewrap covers the hint-suggestion
+// path in Execute(): after rewrapping an "unknown flag" error with
+// fmt.Errorf("%w\nhint: ...", err, ...), the rewrapped error must still
+// classify as a usage error so the exit code stays at 2 rather than
+// silently falling through to 1.
+func TestIsCobraUsageError_SurvivesHintRewrap(t *testing.T) {
+ t.Parallel()
+ original := errors.New("unknown flag: --foob")
+ wrapped := fmt.Errorf("%w\nhint: did you mean --foo?", original)
+ if !isCobraUsageError(wrapped) {
+ t.Errorf("hint-rewrapped unknown-flag error must still classify as usage error, got: %v", wrapped)
+ }
+}
+
+// TestExitCode_UsageError_WrappedAsCode2 covers the end-to-end contract:
+// after Execute() wraps a Cobra usage error in usageErr(), ExitCode()
+// must return 2, not 1. This is the user-visible promise — a bad flag
+// exits 2 (POSIX usage convention), not the generic-error-1 fallback.
+func TestExitCode_UsageError_WrappedAsCode2(t *testing.T) {
+ t.Parallel()
+ wrapped := usageErr(errors.New("unknown flag: --foob"))
+ if got := ExitCode(wrapped); got != 2 {
+ t.Errorf("ExitCode(usageErr(...)) = %d, want 2 (POSIX usage convention)", got)
+ }
+}
← 6cda2606 fix(cli): emit stderr truncation warning on single-page list
·
back to Cli Printing Press
·
chore(main): release 4.5.1 (#1170) a169ca49 →