fix(bonfire): reroute only the owning console's lines to a live view

- the registered view keeps the console that started it; Println, Printf and status lines go above it only from that console, other consoles write to their own writers (bonfire.PrintAbove still reaches any view) - runLive claims the single live slot by check and set under liveMu and reports when it could not, so the widget prints plain lines - unregister, finish and wait run deferred, so a panic in work leaves no registration behind and still propagates (WR-02)
2026-10-22 02:10:48 +02:00
parent b077dc3598
commit 86e4e270d3
6 changed files with 200 additions and 56 deletions
--- a/modules/bonfire/README.md
+++ b/modules/bonfire/README.md
@@ -27,7 +27,7 @@ Use cases:
 - Usage strings and argument-count checks derived from the declared arguments (`<name>` for required, `[name]` for optional).
 - Scalar flags, bare flags (`bonfire.Flag.Bare`, so `--force` alone stores `true`) and ordered repeatable flags (`bonfire.Flag.Repeatable`, read back through `bonfire.Input.Flags`).
 - Styled status lines: `bonfire.Output.Info`, `bonfire.Output.Success`, `bonfire.Output.Warning` and `bonfire.Output.Error` (the last one writes to the error stream).
- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command. Log lines and output printed while a view runs appear above it (`bonfire.PrintAbove`).
+- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command. Log lines, and output the command prints through the console that started the view, appear above a running view (`bonfire.PrintAbove`); other consoles (a `bonfire.Call` buffer, a console on a file) keep writing to their own writers.
 - Prompts: `bonfire.Output.Ask`, `bonfire.Output.Confirm` and `bonfire.Output.Secret` use huh fields on an interactive terminal, and `bonfire.Output.Choice` an arrow-key selector with a confirmation screen. Off a terminal they read lines from stdin as before (a piped password still works), return their defaults when input ends, and `bonfire.Output.Confirm` returns its default without asking. A cancelled prompt returns `bonfire.ErrAborted`. `bonfire.ConfirmProceed` asks before a destructive step only on an interactive terminal.
 - Degradation: every widget follows the terminal and the global options, and the `bonfire.Output` interface is unchanged for plugins:

--- a/modules/bonfire/helpers.go
+++ b/modules/bonfire/helpers.go
@@ -149,7 +149,7 @@ func Process(ctx context.Context, out Output, operation string, items []string,
 	}
 	if c, ok := out.(*console); ok && c.live() && len(items) > 0 {
 		var errs []error
-		_ = runLive(ctx, c.out, c, newProcessModel(c.palette, operation, items), func(send func(tea.Msg)) error {
+		started, _ := runLive(ctx, c.out, c, newProcessModel(c.palette, operation, items), func(send func(tea.Msg)) error {
 			for i, item := range items {
 				if err := ctx.Err(); err != nil {
 					errs = append(errs, err)
@@ -165,7 +165,9 @@ func Process(ctx context.Context, out Output, operation string, items []string,
 			}
 			return nil
 		})
-		return errors.Join(errs...)
+		if started {
+			return errors.Join(errs...)
+		}
 	}
 	var errs []error
 	for _, item := range items {
--- a/modules/bonfire/live.go
+++ b/modules/bonfire/live.go
@@ -8,6 +8,7 @@ package bonfire

 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -314,26 +315,33 @@ func (m processModel) View() tea.View {
 	return tea.NewView(b.String())
 }

-// The running live program, for PrintAbove. Only one live view runs at a
-// time; widgets started while one runs print plain lines.
-var (
-	liveMu     sync.Mutex
-	activeProg *tea.Program
-)
-
-// liveActive reports whether a live view is running.
-func liveActive() bool {
-	liveMu.Lock()
-	defer liveMu.Unlock()
-	return activeProg != nil
+// liveView is the running live program and the console that started it.
+type liveView struct {
+	owner *console
+	prog  *tea.Program
 }

+// The running live view, for PrintAbove and the owning console's output.
+// Only one live view runs at a time; a widget that cannot claim the slot
+// (a nested widget, or a second console at the same moment) prints plain
+// lines.
+var (
+	liveMu     sync.Mutex
+	activeView *liveView
+)
+
+// errLivePanic is the result the final frame shows when work panicked.
+var errLivePanic = errors.New("bonfire: live view work panicked")
+
 // runLive runs model as a bubbletea program on out while work runs on the
-// calling goroutine. work gets a send function for progress messages. When
-// work returns, the program is unregistered (so PrintAbove stops sending to
-// it), told to finish and waited for, all under liveMu; it never receives
-// lines after that.
-func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, work func(send func(tea.Msg)) error) error {
+// calling goroutine, with c as the owner of the view. It first claims the
+// single live slot under liveMu; when another view holds it, runLive
+// returns started false without running work, and the caller prints plain
+// lines instead. work gets a send function for progress messages. When
+// work returns or panics, the view is unregistered (so PrintAbove stops
+// sending to it), told to finish and waited for, all under liveMu; it
+// never receives lines after that, and a panic goes on after the cleanup.
+func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, work func(send func(tea.Msg)) error) (started bool, err error) {
 	if ctx == nil {
 		ctx = context.Background()
 	}
@@ -344,38 +352,61 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo
 		tea.WithContext(ctx),
 		tea.WithColorProfile(c.profile()),
 	)
+	view := &liveView{owner: c, prog: prog}
+	liveMu.Lock()
+	if activeView != nil {
+		liveMu.Unlock()
+		return false, nil
+	}
+	activeView = view
+	liveMu.Unlock()
+
 	finished := make(chan struct{})
 	go func() {
 		defer close(finished)
 		_, _ = prog.Run()
 	}()
-	liveMu.Lock()
-	activeProg = prog
-	liveMu.Unlock()

-	err := work(prog.Send)
-
-	// Unregister, finish and wait as one step: a printer that arrives
-	// meanwhile waits for the lock and then writes after the final frame,
-	// so no line reaches a stopped program or races the last frame.
-	liveMu.Lock()
-	activeProg = nil
-	prog.Send(finishMsg{err: err})
-	<-finished
-	liveMu.Unlock()
-	return err
+	completed := false
+	defer func() {
+		result := err
+		if !completed {
+			result = errLivePanic
+		}
+		// Unregister, finish and wait as one step: a printer that arrives
+		// meanwhile waits for the lock and then writes after the final
+		// frame, so no line reaches a stopped program or races the last
+		// frame.
+		liveMu.Lock()
+		if activeView == view {
+			activeView = nil
+		}
+		prog.Send(finishMsg{err: result})
+		<-finished
+		liveMu.Unlock()
+	}()
+	err = work(prog.Send)
+	completed = true
+	return true, err
 }

-// printAboveLines prints s, one or more lines, above the running live view.
-// It reports false when no view runs.
+// printAboveLines prints s, one or more lines, above the running live view,
+// whichever console started it (bonfire.PrintAbove). It reports false when
+// no view runs.
 func printAboveLines(s string) bool {
+	return printAbove(nil, s)
+}
+
+// printAbove prints s above the running live view when owner is nil or is
+// the console that started the view, and reports whether it did.
+func printAbove(owner *console, s string) bool {
 	liveMu.Lock()
 	defer liveMu.Unlock()
-	if activeProg == nil {
+	if activeView == nil || (owner != nil && activeView.owner != owner) {
 		return false
 	}
 	// One message for the whole block, sent while holding liveMu: blocks
 	// from different goroutines keep the order of their calls.
-	activeProg.Send(printLine(strings.TrimSuffix(s, "\n")))
+	activeView.prog.Send(printLine(strings.TrimSuffix(s, "\n")))
 	return true
 }
--- a/modules/bonfire/live_test.go
+++ b/modules/bonfire/live_test.go
@@ -427,3 +427,106 @@ func TestPrintAboveOrderedNoLoss(t *testing.T) {
 		}
 	})
 }
+
+// TestPrintlnStaysWithItsConsole (WR-02): while a view runs on console A,
+// another console's Println, Printf and Info go to its own writer; A's own
+// Println still prints above its view.
+func TestPrintlnStaysWithItsConsole(t *testing.T) {
+	bufA, bufB := &lockedBuffer{}, &lockedBuffer{}
+	a := newConsole(strings.NewReader(""), bufA, io.Discard, noEnv, boolPtr(true), boolPtr(true))
+	b := newConsole(strings.NewReader(""), bufB, bufB, noEnv, boolPtr(false), boolPtr(false))
+	err := a.Spinner("working", func() error {
+		b.Println("b-println")
+		b.Printf("b-printf %d\n", 1)
+		b.Info("b-info")
+		a.Println("a-println")
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	gotA, gotB := ansi.Strip(bufA.String()), ansi.Strip(bufB.String())
+	for _, want := range []string{"b-println", "b-printf 1", "b-info"} {
+		if !strings.Contains(gotB, want) {
+			t.Errorf("console B's writer lacks %q:\n%s", want, gotB)
+		}
+		if strings.Contains(gotA, want) {
+			t.Errorf("console B's %q printed above console A's view:\n%s", want, gotA)
+		}
+	}
+	if !strings.Contains(gotA, "a-println") {
+		t.Errorf("console A's own line missing above its view:\n%s", gotA)
+	}
+}
+
+// TestLiveViewSingleOwner (WR-02): two consoles starting a spinner at the
+// same moment never both run a live view; the one that loses prints its
+// plain line to its own writer.
+func TestLiveViewSingleOwner(t *testing.T) {
+	const rounds = 200
+	for round := range rounds {
+		bufs := [2]*lockedBuffer{{}, {}}
+		var start, entered sync.WaitGroup
+		start.Add(1)
+		entered.Add(2)
+		var wg sync.WaitGroup
+		for i := range 2 {
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				c := newConsole(strings.NewReader(""), bufs[i], io.Discard, noEnv, boolPtr(true), boolPtr(true))
+				start.Wait()
+				_ = c.Spinner(fmt.Sprintf("label-%d", i), func() error {
+					entered.Done()
+					entered.Wait() // both work functions run at once
+					return nil
+				})
+			}()
+		}
+		start.Done()
+		wg.Wait()
+		plain := 0
+		for i, b := range bufs {
+			if strings.Contains(ansi.Strip(b.String()), fmt.Sprintf("[...] label-%d", i)) {
+				plain++
+			}
+		}
+		if plain != 1 {
+			t.Fatalf("round %d: %d consoles printed their plain line to their own writer, want 1:\nA: %q\nB: %q",
+				round, plain, ansi.Strip(bufs[0].String()), ansi.Strip(bufs[1].String()))
+		}
+		if PrintAbove("after the round") {
+			t.Fatalf("round %d: a view is still registered", round)
+		}
+	}
+}
+
+// TestLiveViewPanicUnregisters (WR-02): a panic in the work function still
+// ends and unregisters the view; the next widget runs as a live view.
+func TestLiveViewPanicUnregisters(t *testing.T) {
+	out := &lockedBuffer{}
+	c := newConsole(strings.NewReader(""), out, io.Discard, noEnv, boolPtr(true), boolPtr(true))
+	func() {
+		defer func() {
+			if r := recover(); r != "boom" {
+				t.Fatalf("recovered %v, want the work function's panic", r)
+			}
+		}()
+		_ = c.Spinner("panicking", func() error { panic("boom") })
+	}()
+	if PrintAbove("after the panic") {
+		t.Fatal("the panicked view is still registered")
+	}
+	next := &lockedBuffer{}
+	c2 := newConsole(strings.NewReader(""), next, io.Discard, noEnv, boolPtr(true), boolPtr(true))
+	var during bool
+	if err := c2.Spinner("next", func() error {
+		during = PrintAbove("above the next view")
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if !during || strings.Contains(ansi.Strip(next.String()), "[...] next") {
+		t.Errorf("the next spinner did not run as a live view:\n%s", ansi.Strip(next.String()))
+	}
+}
--- a/modules/bonfire/output.go
+++ b/modules/bonfire/output.go
@@ -128,10 +128,11 @@ func (c *console) setGlobals(g Globals) {

 // live reports whether widgets animate: stdout is a terminal and none of
 // -q, --no-ansi and -n is set (D-03). A live view that is already running
-// makes nested widgets print plain lines.
+// makes nested widgets print plain lines: runLive then reports that it did
+// not start.
 func (c *console) live() bool {
 	g := c.globals
-	return c.tty && !g.Quiet && g.Color != ColorNever && !g.NoInteraction && !liveActive()
+	return c.tty && !g.Quiet && g.Color != ColorNever && !g.NoInteraction
 }

 // canPrompt reports whether prompts may wait for input from a person:
@@ -170,13 +171,14 @@ func (c *console) styled(w io.Writer) io.Writer {
 	return &colorprofile.Writer{Forward: w, Profile: c.profile()}
 }

-// emit writes s, a styled line or block, to w. While a live view runs on
-// stdout, lines for stdout print above it instead of tearing it.
+// emit writes s, a styled line or block, to w. While a live view this
+// console started runs on stdout, lines for stdout print above it instead
+// of tearing it.
 func (c *console) emit(w io.Writer, s string) {
 	if w == nil {
 		return
 	}
-	if w == c.out && printAboveLines(s) {
+	if w == c.out && printAbove(c, s) {
 		return
 	}
 	_, _ = io.WriteString(c.styled(w), s)
@@ -194,7 +196,7 @@ func (c *console) Println(a ...any) {
 		return
 	}
 	s := fmt.Sprintln(a...)
-	if printAboveLines(s) {
+	if printAbove(c, s) {
 		return
 	}
 	_, _ = io.WriteString(c.out, s)
@@ -205,7 +207,7 @@ func (c *console) Printf(format string, a ...any) {
 		return
 	}
 	s := fmt.Sprintf(format, a...)
-	if strings.HasSuffix(s, "\n") && printAboveLines(s) {
+	if strings.HasSuffix(s, "\n") && printAbove(c, s) {
 		return
 	}
 	_, _ = io.WriteString(c.out, s)
--- a/modules/bonfire/widgets.go
+++ b/modules/bonfire/widgets.go
@@ -26,13 +26,16 @@ func (c *console) Spinner(message string, fn func() error) error {
 	if c.globals.Quiet {
 		return fn()
 	}
-	if !c.live() {
-		c.Printf("[...] %s\n", message)
-		return fn()
+	if c.live() {
+		started, err := runLive(context.Background(), c.out, c, newSpinnerModel(c.palette, message), func(func(tea.Msg)) error {
+			return fn()
+		})
+		if started {
+			return err
+		}
 	}
-	return runLive(context.Background(), c.out, c, newSpinnerModel(c.palette, message), func(func(tea.Msg)) error {
-		return fn()
-	})
+	c.Printf("[...] %s\n", message)
+	return fn()
 }

 func (c *console) Progress(total int, fn func(Progress) error) error {
@@ -45,12 +48,15 @@ func (c *console) Progress(total int, fn func(Progress) error) error {
 	if c.globals.Quiet {
 		return fn(&quietProgress{c: c})
 	}
-	if !c.live() {
-		return fn(&progressBar{c: c, total: total, lastPct: -1})
+	if c.live() {
+		started, err := runLive(context.Background(), c.out, c, newProgressModel(c.palette, total), func(send func(tea.Msg)) error {
+			return fn(&liveProgress{send: send, total: total})
+		})
+		if started {
+			return err
+		}
 	}
-	return runLive(context.Background(), c.out, c, newProgressModel(c.palette, total), func(send func(tea.Msg)) error {
-		return fn(&liveProgress{send: send, total: total})
-	})
+	return fn(&progressBar{c: c, total: total, lastPct: -1})
 }

 // progressBar prints a line at every tenth of the work when widgets do not