fix(summer): send a delegate's stderr to the root's error writer through bonfire.ErrWriter (IN-13, quick-261028-rqd)

cmd/summer/runtime.go

@@ -150,13 +150,14 @@ func runAppBinary(ctx context.Context, argv []string, out bonfire.Output) error
 // streams, so it sees the terminal: it asks the production question, prints
 // styled summaries and animates its widgets. Otherwise its stdout goes
 // through out (tests and bonfire.Call roots capture it) and its stderr to
-// summer's stderr, so log lines and error blocks never mix into data such
-// as --json output.
+// the root's error stream (bonfire.ErrWriter), which is os.Stderr for the
+// real summer, so log lines and error blocks never mix into data such as
+// --json output, and a root built on buffers captures them.
 func appStreams(out bonfire.Output, terminal bool) (stdout, stderr io.Writer) {
 	if terminal {
 		return os.Stdout, os.Stderr
 	}
-	return out, os.Stderr
+	return out, bonfire.ErrWriter(out)
 }
 
 // runAppCmd runs cmd, the app binary started with argv, forwarding SIGINT

cmd/summer/runtime_test.go

@@ -346,6 +346,11 @@ func TestDelegateExitCodePassthrough(t *testing.T) {
 		if code != 2 {
 			t.Errorf("exit %d, want the binary's 2", code)
 		}
+		// IN-13: the binary's own error block reaches the root's error
+		// writer.
+		if n := strings.Count(errOut, "ERROR  the app binary failed"); n != 1 {
+			t.Errorf("the binary's error block reached the root's error writer %d times, want 1:\nstdout %q\nstderr %q", n, out, errOut)
+		}
 		for _, s := range []string{out, errOut} {
 			if strings.Contains(s, " ERROR ") || strings.Contains(s, "summer: migrate") {
 				t.Errorf("summer printed an error block of its own:\nstdout %q\nstderr %q", out, errOut)
@@ -419,9 +424,9 @@ func TestRunAppCmdMasksSecrets(t *testing.T) {
 	})
 }
 
-// TestDelegateStreams (CR-03): off a terminal the app binary's stdout goes
-// through summer's Output and its stderr to summer's stderr, never into
-// summer's stdout.
+// TestDelegateStreams (CR-03, IN-13): off a terminal the app binary's
+// stdout goes through summer's Output and its stderr to the root's error
+// writer (bonfire.ErrWriter), never into summer's stdout.
 func TestDelegateStreams(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("the fake app binary is a shell script")
@@ -438,23 +443,42 @@ func TestDelegateStreams(t *testing.T) {
 		t.Fatal(err)
 	}
 	t.Chdir(app)
-	out, err := runSummer(t, "route:list", "--json")
+	var stdout, stderr syncBuffer
+	root, err := bonfire.NewRootIO("summer", toolCommands(), strings.NewReader(""), &stdout, &stderr)
 	if err != nil {
-		t.Fatalf("route:list: %v\n%s", err, out)
+		t.Fatal(err)
 	}
-	if !strings.Contains(out, "data-line") {
-		t.Errorf("the binary's stdout did not reach summer's Output: %q", out)
+	if code := bonfire.Execute(context.Background(), root, []string{"route:list", "--json"}); code != bonfire.ExitOK {
+		t.Fatalf("route:list: exit %d\nstdout %q\nstderr %q", code, stdout.String(), stderr.String())
 	}
-	if strings.Contains(out, "log-line") {
-		t.Errorf("the binary's stderr landed in summer's stdout: %q", out)
+	if !strings.Contains(stdout.String(), "data-line") {
+		t.Errorf("the binary's stdout did not reach summer's Output: %q", stdout.String())
+	}
+	if strings.Contains(stdout.String(), "log-line") {
+		t.Errorf("the binary's stderr landed in summer's stdout: %q", stdout.String())
+	}
+	if !strings.Contains(stderr.String(), "log-line") {
+		t.Errorf("the binary's stderr did not reach the root's error writer: %q", stderr.String())
 	}
 }
 
-// TestAppStreams (CR-03): on a terminal the app binary gets summer's own
-// stdout and stderr; off a terminal its stdout goes through the Output and
-// its stderr to summer's stderr.
+// TestAppStreams (CR-03, IN-13): on a terminal the app binary gets
+// summer's own stdout and stderr; off a terminal its stdout goes through the
+// Output and its stderr to the Output's error writer (bonfire.ErrWriter),
+// which is os.Stderr for the real summer.
 func TestAppStreams(t *testing.T) {
-	var buf bytes.Buffer
+	var buf, errBuf bytes.Buffer
+	sep := bonfire.NewOutput(strings.NewReader(""), &buf, &errBuf)
+	if _, stderr := appStreams(sep, false); stderr != io.Writer(&errBuf) {
+		t.Errorf("not a terminal: stderr %T, want the Output's error writer", stderr)
+	}
+	real, err := bonfire.NewRoot("summer", nil, os.Stdout)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if real.ErrOrStderr() != os.Stderr {
+		t.Errorf("the real summer root's error stream is %v, want os.Stderr", real.ErrOrStderr())
+	}
 	out := bonfire.NewOutput(strings.NewReader(""), &buf, &buf)
 	if bonfire.IsTerminal(out) {
 		t.Fatal("a buffer console reports a terminal")
@@ -464,7 +488,7 @@ func TestAppStreams(t *testing.T) {
 		t.Errorf("terminal: streams %v, %v; want os.Stdout, os.Stderr", stdout, stderr)
 	}
 	stdout, stderr = appStreams(out, false)
-	if stdout != io.Writer(out) || stderr != os.Stderr {
-		t.Errorf("not a terminal: streams %v, %v; want the Output, os.Stderr", stdout, stderr)
+	if stdout != io.Writer(out) || stderr != io.Writer(&buf) {
+		t.Errorf("not a terminal: streams %T, %T; want the Output and its error writer", stdout, stderr)
 	}
 }

modules/bonfire/README.md

@@ -38,7 +38,7 @@ Use cases:
   | `-q` | none | info, success and warning lines hidden; errors kept | printed (`Println`, `Printf`, tables) | defaults, as with `-n` |
   | `--no-ansi` | none | symbols, no colors | printed | plain line prompts |
   | `-n` | none | as on the terminal | printed | defaults; `Secret` reads piped stdin or fails with a hint |
-- Command state for helpers: `bonfire.GlobalsOf`, `bonfire.VerbosityOf`, `bonfire.IsQuiet` and `bonfire.Interactive` read the running command's global options from its `bonfire.Output`.
+- Command state for helpers: `bonfire.GlobalsOf`, `bonfire.VerbosityOf`, `bonfire.IsQuiet` and `bonfire.Interactive` read the running command's global options from its `bonfire.Output`, and `bonfire.ErrWriter` returns the stream its root's error block goes to (stderr for the real binary, the injected error writer for a root built on buffers).
 - Passthrough commands (`bonfire.Command.Passthrough`) skip flag parsing, so every argument, including `--help`, reaches `bonfire.Input.Args` unchanged; this suits commands that forward to another program. `bonfire.Command.Long` adds extended help text that the help command shows instead of the description.
 - Artisan-style command list: a bare binary (or `--help`, or `list`) prints a version header (`bonfire.Version.Header`, read from the build information by `bonfire.BuildVersion`), the usage line, the global options and the commands, top-level names first and then one block per namespace, with aligned descriptions.
 - Global options on every command: `-v`/`-vv`/`-vvv` (`--verbose`), `-q` (`--quiet`, implies `-n`), `--ansi`/`--no-ansi` (last wins), `-n` (`--no-interaction`) and `--env`. `bonfire.PeekGlobals` reads them from the raw arguments before the root exists, so the generated main can pick the configuration environment and the log verbosity before plugins boot; it reads the `=value` forms the way pflag does (`--verbose=2`, `-v=3`, `--quiet=false`, `--ansi=false`), and a value-taking command flag followed by a global-looking token (`--name -v`) is read as the global before boot, while the command and its error block use cobra's reading; `bonfire.OnGlobals` hooks receive the parsed `bonfire.Globals` before every command runs (roots built by `bonfire.Call` run no hook).
@@ -160,6 +160,7 @@ if catalog.Has("blog:import") {
 | `bonfire.VerbosityOf` | The `-v` level of the command an Output belongs to. |
 | `bonfire.IsQuiet` | Whether the command runs with `-q`. |
 | `bonfire.IsTerminal` | Whether the Output writes to a terminal; long-running commands print a summary block for a person there and log records otherwise. |
+| `bonfire.ErrWriter` | The stream the root's error block goes to for the command an Output belongs to: `os.Stderr` for `bonfire.NewRoot` on `os.Stdout`, the error writer of `bonfire.NewRootIO` and `bonfire.NewOutput`, `os.Stderr` for other Output implementations. summer sends a delegated binary's stderr there. |
 | `bonfire.Details`, `bonfire.Detail` | Prints a title and aligned label/value lines in the palette (a `Warn` value in the warning color); nothing under `-q`. Used for startup summaries and shutdown reports. |
 | `bonfire.Interactive` | Whether the Output can ask a person: stdin and stdout are terminals and neither `-n` nor `-q` is set. |
 | `bonfire.ConfirmProceed` | Asks before a destructive step on an interactive terminal; returns true without asking otherwise. |

modules/bonfire/helpers.go

@@ -3,6 +3,8 @@ package bonfire
 import (
 	"context"
 	"errors"
+	"io"
+	"os"
 	"strings"
 
 	tea "charm.land/bubbletea/v2"
@@ -15,6 +17,7 @@ type consoleState interface {
 	consoleGlobals() Globals
 	consoleInteractive() bool
 	consoleTerminal() bool
+	consoleErrWriter() io.Writer
 }
 
 func (c *console) consoleGlobals() Globals { return c.globals }
@@ -23,6 +26,13 @@ func (c *console) consoleInteractive() bool { return c.canPrompt() }
 
 func (c *console) consoleTerminal() bool { return c.tty }
 
+func (c *console) consoleErrWriter() io.Writer {
+	if c.errStream != nil {
+		return c.errStream
+	}
+	return c.err
+}
+
 // GlobalsOf returns the global options of the command out belongs to, or
 // the zero Globals when out is not a bonfire console.
 func GlobalsOf(out Output) Globals {
@@ -63,6 +73,21 @@ func IsTerminal(out Output) bool {
 	return false
 }
 
+// ErrWriter returns the stream the root's error block goes to for the
+// command out belongs to: os.Stderr for a root built by NewRoot on
+// os.Stdout, the error writer given to NewRootIO or NewOutput (stdout when
+// NewOutput gets none), and os.Stderr for an Output that is not a bonfire
+// console. summer sends an app binary's stderr there, so a root built on
+// buffers captures it while the real summer keeps it on stderr.
+func ErrWriter(out Output) io.Writer {
+	if s, ok := out.(consoleState); ok {
+		if w := s.consoleErrWriter(); w != nil {
+			return w
+		}
+	}
+	return os.Stderr
+}
+
 // Detail is one line of a Details block: a label and its value. Warn
 // prints the value in the warning color.
 type Detail struct {

modules/bonfire/output.go

@@ -44,9 +44,12 @@ type Progress interface {
 }
 
 type console struct {
-	in     io.Reader
-	out    io.Writer
-	err    io.Writer
+	in  io.Reader
+	out io.Writer
+	err io.Writer
+	// errStream is the stream the root's error block goes to, when the
+	// console belongs to a root (ErrWriter); nil means err.
+	errStream io.Writer
 	lookup func(string) string
 	// tty is true when stdout is a terminal; it picks animated widgets
 	// and boxed tables.

modules/bonfire/output_test.go

@@ -4,9 +4,13 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"io"
+	"os"
 	"slices"
 	"strings"
 	"testing"
+
+	"github.com/spf13/cobra"
 )
 
 func TestNonTTYSpinner(t *testing.T) {
@@ -387,3 +391,51 @@ func TestHelpUsesSharedAdapter(t *testing.T) {
 		t.Fatalf("help = %q", got)
 	}
 }
+
+// stubOutput is an Output that is not a bonfire console.
+type stubOutput struct{ Output }
+
+// TestErrWriter (IN-13): ErrWriter returns the stream the root's error
+// block goes to: os.Stderr for NewRoot on os.Stdout, the error writer of
+// NewRootIO and NewOutput (stdout when NewOutput gets none), and os.Stderr
+// for an Output that is not a bonfire console.
+func TestErrWriter(t *testing.T) {
+	outputOf := func(t *testing.T, build func([]Command) (*cobra.Command, error)) Output {
+		t.Helper()
+		var got Output
+		cmd := Command{Name: "acme:grab", Description: "Grab the output", Run: func(_ context.Context, _ Input, out Output) error {
+			got = out
+			return nil
+		}}
+		root, err := build([]Command{cmd})
+		if err != nil {
+			t.Fatal(err)
+		}
+		root.SetArgs([]string{"acme:grab"})
+		if err := root.Execute(); err != nil {
+			t.Fatal(err)
+		}
+		return got
+	}
+
+	out := outputOf(t, func(c []Command) (*cobra.Command, error) { return NewRoot("acme", c, os.Stdout) })
+	if w := ErrWriter(out); w != os.Stderr {
+		t.Errorf("NewRoot on os.Stdout: ErrWriter = %v, want os.Stderr", w)
+	}
+	var stdout, stderr bytes.Buffer
+	out = outputOf(t, func(c []Command) (*cobra.Command, error) {
+		return NewRootIO("acme", c, strings.NewReader(""), &stdout, &stderr)
+	})
+	if w := ErrWriter(out); w != io.Writer(&stderr) {
+		t.Errorf("NewRootIO: ErrWriter = %v, want the error writer", w)
+	}
+	if w := ErrWriter(NewOutput(strings.NewReader(""), &stdout, &stderr)); w != io.Writer(&stderr) {
+		t.Errorf("NewOutput: ErrWriter = %v, want the error writer", w)
+	}
+	if w := ErrWriter(NewOutput(strings.NewReader(""), &stdout, nil)); w != io.Writer(&stdout) {
+		t.Errorf("NewOutput without an error writer: ErrWriter = %v, want stdout", w)
+	}
+	if w := ErrWriter(stubOutput{}); w != os.Stderr {
+		t.Errorf("a non-bonfire Output: ErrWriter = %v, want os.Stderr", w)
+	}
+}

modules/bonfire/root.go

@@ -53,18 +53,26 @@ func OnGlobals(fn func(Globals)) RootOption {
 // It accepts command values rather than a party registry to avoid an import cycle.
 // When out is the process stdout, the error block of Execute goes to stderr.
 func NewRoot(name string, commands []Command, out io.Writer, opts ...RootOption) (*cobra.Command, error) {
-	root, err := NewRootIO(name, commands, os.Stdin, out, out, opts...)
+	root, console, err := newRootIO(name, commands, os.Stdin, out, out, opts...)
 	if err != nil {
 		return nil, err
 	}
 	if out == nil || out == os.Stdout {
 		root.SetErr(os.Stderr)
+		console.errStream = os.Stderr
 	}
 	return root, nil
 }
 
 // NewRootIO is NewRoot with injected stdin/stdout/stderr for tests.
 func NewRootIO(name string, commands []Command, in io.Reader, out, errW io.Writer, opts ...RootOption) (*cobra.Command, error) {
+	root, _, err := newRootIO(name, commands, in, out, errW, opts...)
+	return root, err
+}
+
+// newRootIO builds the root of NewRootIO and returns its console too, so
+// NewRoot can record where it moved the error stream.
+func newRootIO(name string, commands []Command, in io.Reader, out, errW io.Writer, opts ...RootOption) (*cobra.Command, *console, error) {
 	if out == nil {
 		out = os.Stdout
 	}
@@ -84,6 +92,7 @@ func NewRootIO(name string, commands []Command, in io.Reader, out, errW io.Write
 		cfg.version = BuildVersion()
 	}
 	output := newConsole(in, out, errW, os.Getenv, nil, nil)
+	output.errStream = errW
 	root := &cobra.Command{
 		Use:           name,
 		SilenceUsage:  true,
@@ -116,24 +125,24 @@ func NewRootIO(name string, commands []Command, in io.Reader, out, errW io.Write
 	}
 	for _, command := range commands {
 		if seen[command.Name] {
-			return nil, fmt.Errorf("%w: %q", ErrDuplicateCommand, command.Name)
+			return nil, nil, fmt.Errorf("%w: %q", ErrDuplicateCommand, command.Name)
 		}
 		seen[command.Name] = true
 		cmd, err := st.wrap(command, true)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		root.AddCommand(cmd)
 	}
 
 	listCmd, err := st.wrap(st.listCommand(), false)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	root.AddCommand(listCmd)
 	helpCmd, err := st.wrap(st.helpCommand(), false)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	root.SetHelpCommand(helpCmd)
 	root.SetHelpFunc(func(*cobra.Command, []string) {
@@ -160,7 +169,7 @@ func NewRootIO(name string, commands []Command, in io.Reader, out, errW io.Write
 		}
 		return nil
 	}
-	return root, nil
+	return root, output, nil
 }
 
 // rootState is what the list, help and error rendering of one root share.

scripts/check-phase19.8.sh

@@ -136,8 +136,9 @@ BONFIRE_TESTS=(
 	# quick 261028-rqd (info findings): IN-03, IN-06, IN-02, IN-04.
 	TestPluginErrorWordedUnknownCommand TestExecuteKeepsRootHelp
 	TestErrorBlockUsesParsedGlobals TestFlagBool
-	# quick 261028-rqd: IN-11, IN-10, IN-12.
+	# quick 261028-rqd: IN-11, IN-10, IN-12, IN-13.
 	TestPrintAboveBlankLine TestErrorPrintsAboveView TestSignalRelayWaitsForStart
+	TestErrWriter
 )
 DOCSITE_TESTS=(
 	# Plan 04: the docs checker reads past the global options.