fix(bonfire): recognise cobra's unknown command error once in Execute (IN-03, quick-261028-rqd)

modules/bonfire/README.md

@@ -46,7 +46,7 @@ Use cases:
 - Hidden commands (`bonfire.Command.Hidden`) run but stay out of the list unless `list --all`.
 - JSON catalog: `list --format=json` prints every command, hidden ones flagged, with arguments, options, defaults and examples as a `bonfire.Listing` (`bonfire.Describe` builds it from a command slice).
 - Documentation coverage: `bonfire.CheckDocs` reports commands lacking a description, long help, examples, or argument and option descriptions.
-- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line, also a value that starts with `-`.
+- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), for an unknown command (recognised by `bonfire.Execute` from cobra's own error, never from the wording of a command's error) `Command "x" is not defined.` with cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line, also a value that starts with `-`.
 - Name checks at boot: two commands with the same name, or a command named `list` or `help`, make `bonfire.NewRoot` fail with `bonfire.ErrDuplicateCommand`; a command flag that reuses a global option's name or shorthand fails with `bonfire.ErrReservedFlag` instead of panicking.
 - Injectable streams (`bonfire.NewRootIO`, `bonfire.NewOutput`) so commands can be tested against buffers.
 - In-process calls: `bonfire.Call` runs a named command with arguments against any writer (Laravel `Artisan::call`), and `bonfire.Catalog` holds an application binary's final command list so code outside the Cobra root, such as the conga scheduler, can call any registered command.

modules/bonfire/errors.go

@@ -12,6 +12,7 @@ import (
 	"git.golem15.com/golem15/summercms/internal/argmask"
 	"git.golem15.com/golem15/summercms/modules/sunscreen"
 	"github.com/charmbracelet/colorprofile"
+	"github.com/spf13/cobra"
 )
 
 // UsageError marks a mistake in how a command was called: an unknown
@@ -40,13 +41,39 @@ func Usage(err error) error {
 	return &UsageError{Err: err}
 }
 
-// isUsage reports a UsageError in the chain or cobra's unknown command error.
+// isUsage reports a UsageError in the chain. cobra's unknown command error
+// counts only after Execute has converted it (unknownCommand).
 func isUsage(err error) bool {
 	var ue *UsageError
-	if errors.As(err, &ue) {
-		return true
+	return errors.As(err, &ue)
+}
+
+// unknownCommandError is cobra's unknown command error once Execute has
+// confirmed it came from cobra: Error keeps cobra's text, name and
+// suggestions are read from it once.
+type unknownCommandError struct {
+	msg         string
+	name        string
+	suggestions []string
+}
+
+func (e *unknownCommandError) Error() string { return e.msg }
+
+// unknownCommand converts err into a UsageError around an
+// unknownCommandError when it is cobra's unknown command error for args:
+// root.Find on the same args fails at the root with the same message, so
+// cobra rejected the command word and no command ran. Every other error,
+// including a command's own error worded the same way, is returned as is.
+func unknownCommand(root *cobra.Command, args []string, err error) error {
+	if err == nil || root == nil {
+		return err
 	}
-	return strings.HasPrefix(err.Error(), "unknown command")
+	cmd, _, ferr := root.Find(args)
+	if ferr == nil || cmd != root || ferr.Error() != err.Error() {
+		return err
+	}
+	name, suggestions := parseUnknownCommand(err.Error())
+	return &UsageError{Err: &unknownCommandError{msg: err.Error(), name: name, suggestions: suggestions}}
 }
 
 // HintError adds an explanation and a command to try to an error. The error
@@ -169,9 +196,10 @@ func WithExitCode(err error, code int) error {
 // and the message, the explanation and the Try lines of a HintError ({bin}
 // replaced by summer or bin/<name>, for the root named name), then "Run with
 // -v for details." at verbosity 0 or the wrapped error chain under "Caused
-// by:" at -v and above. cobra's unknown command error becomes `Command "x"
-// is not defined.` with its suggestions as Try lines, and other usage errors
-// suggest the command's help. Every message passes through sunscreen.Scrub,
+// by:" at -v and above. An unknown command, as recognised by Execute,
+// becomes `Command "x" is not defined.` with cobra's suggestions as Try
+// lines, and other usage errors suggest the command's help. A raw cobra
+// error passed in directly keeps cobra's text. Every message passes through sunscreen.Scrub,
 // and every line through Sanitize.
 // Errors wrapped by Reported print nothing.
 func RenderError(w io.Writer, name string, err error, g Globals) {
@@ -189,12 +217,12 @@ func RenderError(w io.Writer, name string, err error, g Globals) {
 	if errors.As(err, &hint) {
 		explanation, try = hint.Explanation, hint.Try
 	}
-	if strings.HasPrefix(message, "unknown command") {
-		cmdName, suggestions := parseUnknownCommand(message)
-		message = `Command "` + cmdName + `" is not defined.`
+	var unknown *unknownCommandError
+	if errors.As(err, &unknown) {
+		message = `Command "` + unknown.name + `" is not defined.`
 		if try == "" {
 			var lines []string
-			for _, s := range suggestions {
+			for _, s := range unknown.suggestions {
 				lines = append(lines, "{bin} "+s)
 			}
 			if len(lines) == 0 {

modules/bonfire/errors_test.go

@@ -342,3 +342,34 @@ func TestSanitize(t *testing.T) {
 		t.Errorf("Sanitize(%q) = %q, want it unchanged", clean, got)
 	}
 }
+
+// TestPluginErrorWordedUnknownCommand (IN-03): only cobra's own unknown
+// command error is a usage error. A command whose Run returns an error
+// worded the same way exits 1 and is printed verbatim, while a mistyped
+// command still exits 2 with `Command "x" is not defined.` and cobra's
+// suggestions as Try lines.
+func TestPluginErrorWordedUnknownCommand(t *testing.T) {
+	plainEnv(t)
+	commands := []Command{
+		{Name: "migrate", Description: "Migrate", Run: func(context.Context, Input, Output) error {
+			return errors.New(`unknown command "x" for "acme"`)
+		}},
+	}
+	_, errOut, code := runGolden(t, commands, "migrate")
+	if code != ExitFailure {
+		t.Errorf("plugin error: exit %d, want %d\n%s", code, ExitFailure, errOut)
+	}
+	if !strings.Contains(errOut, `unknown command "x" for "acme"`) || strings.Contains(errOut, "is not defined") {
+		t.Errorf("plugin error not printed verbatim:\n%s", errOut)
+	}
+
+	_, errOut, code = runGolden(t, commands, "migrat")
+	if code != ExitUsage {
+		t.Errorf("migrat: exit %d, want %d\n%s", code, ExitUsage, errOut)
+	}
+	for _, want := range []string{`Command "migrat" is not defined.`, "Try:", "bin/acme migrate"} {
+		if !strings.Contains(errOut, want) {
+			t.Errorf("migrat: block lacks %q:\n%s", want, errOut)
+		}
+	}
+}

modules/bonfire/execute.go

@@ -54,10 +54,11 @@ func Execute(ctx context.Context, root *cobra.Command, args []string) int {
 			// fang hands over a colorprofile writer built from the
 			// environment only; RenderError unwraps it and applies
 			// --ansi and --no-ansi itself.
+			err = unknownCommand(root, args, err)
 			RenderError(w, name, wrongDirectory(name, err, args, ""), g)
 		}),
 	)
-	return exitCode(err)
+	return exitCode(unknownCommand(root, args, err))
 }
 
 // Fail renders err like Execute does and returns ExitFailure (ExitOK for a