fix(bonfire): end a live view when its context is cancelled and print later lines plainly (WR-09)

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: Ctrl+C or SIGTERM during a view restores the cursor and the terminal, then stops the command as before (or reaches its own signal handler). 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. Table cells, the item names and errors of `bonfire.Process` (in the process view and in its plain lines), the process view's operation label, and failure lines are stripped of control characters (`bonfire.Sanitize`).
+- 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: Ctrl+C or SIGTERM during a view restores the cursor and the terminal, then stops the command as before (or reaches its own signal handler). 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. When the context given to `bonfire.Process` is cancelled, the view ends with its last frame and later lines print plainly below it. Table cells, the item names and errors of `bonfire.Process` (in the process view and in its plain lines), the process view's operation label, and failure lines are stripped of control characters (`bonfire.Sanitize`).
 - 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:
 

modules/bonfire/live.go

@@ -325,6 +325,9 @@ func (m processModel) View() tea.View {
 type liveView struct {
 	owner *console
 	prog  *tea.Program
+	// done is closed when the program's Run has returned; printAbove never
+	// sends to a program whose Run has ended, whatever ended it.
+	done <-chan struct{}
 }
 
 // The running live view, for PrintAbove and the owning console's output.
@@ -348,6 +351,13 @@ var errLivePanic = errors.New("bonfire: live view work panicked")
 // 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.
+//
+// The program does not take ctx: when ctx is cancelled, a watcher ends
+// the view with its last frame and unregisters it in the same step under
+// liveMu, without waiting for work, so lines printed after the
+// cancellation print plainly below the view instead of reaching a stopped
+// program. A program whose Run ended on its own is unregistered as well
+// and is never sent lines.
 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()
@@ -355,6 +365,7 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo
 	// Claim the slot before building the program: a widget that loses it
 	// builds none, so it registers no child context on ctx and opens no
 	// TEA_TRACE file. NewProgram takes no bonfire lock and prints nothing.
+	finished := make(chan struct{})
 	liveMu.Lock()
 	if activeView != nil {
 		liveMu.Unlock()
@@ -364,18 +375,17 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo
 		tea.WithInput(nil),
 		tea.WithOutput(out),
 		tea.WithoutSignalHandler(),
-		tea.WithContext(ctx),
 		tea.WithColorProfile(c.profile()),
 	)
-	view := &liveView{owner: c, prog: prog}
+	view := &liveView{owner: c, prog: prog, done: finished}
 	activeView = view
 	liveMu.Unlock()
 
-	finished := make(chan struct{})
 	go func() {
 		defer close(finished)
 		_, _ = prog.Run()
 	}()
+	stopWatch := watchContext(ctx, view, finished)
 	stopSignals := watchSignals(view, finished)
 
 	completed := false
@@ -392,9 +402,12 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo
 		if activeView == view {
 			activeView = nil
 		}
+		// When the context watcher already finished the program, its Run
+		// has returned: Send returns at once and finished is closed.
 		prog.Send(finishMsg{err: result})
 		<-finished
 		liveMu.Unlock()
+		stopWatch()
 		stopSignals()
 	}()
 	err = work(prog.Send)
@@ -402,6 +415,38 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo
 	return true, err
 }
 
+// watchContext ends view when ctx is cancelled before the program
+// finishes: under liveMu it unregisters the view, sends the finish message
+// with ctx's error and waits for the program, the same single step as
+// runLive's cleanup, so a printer either reaches the running program or
+// waits and prints plainly after the last frame. When the program finishes
+// first (normal end, a signal, Run failing), the view is only unregistered.
+// The returned function waits for the watcher; call it after finished is
+// closed and without holding liveMu.
+func watchContext(ctx context.Context, view *liveView, finished <-chan struct{}) (wait func()) {
+	exited := make(chan struct{})
+	go func() {
+		defer close(exited)
+		select {
+		case <-ctx.Done():
+			liveMu.Lock()
+			if activeView == view {
+				activeView = nil
+			}
+			view.prog.Send(finishMsg{err: ctx.Err()})
+			<-finished
+			liveMu.Unlock()
+		case <-finished:
+			liveMu.Lock()
+			if activeView == view {
+				activeView = nil
+			}
+			liveMu.Unlock()
+		}
+	}()
+	return func() { <-exited }
+}
+
 // watchSignals relays SIGINT and SIGTERM (those not ignored) while view
 // runs. The views run without bubbletea's own signal handling, so without
 // this a Ctrl+C would end the process with the cursor still hidden. On a
@@ -477,15 +522,25 @@ func printAboveLines(s string) bool {
 }
 
 // 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.
+// the console that started the view, and reports whether it did. A view
+// whose program has ended (its context was cancelled, or Run returned on
+// its own) is not running: printAbove unregisters it if it is still
+// registered and reports false, so the caller prints s plainly.
 func printAbove(owner *console, s string) bool {
 	liveMu.Lock()
 	defer liveMu.Unlock()
-	if activeView == nil || (owner != nil && activeView.owner != owner) {
+	v := activeView
+	if v == nil || (owner != nil && v.owner != owner) {
 		return false
 	}
+	select {
+	case <-v.done:
+		activeView = nil
+		return false
+	default:
+	}
 	// One message for the whole block, sent while holding liveMu: blocks
 	// from different goroutines keep the order of their calls.
-	activeView.prog.Send(printLine(strings.TrimSuffix(s, "\n")))
+	v.prog.Send(printLine(strings.TrimSuffix(s, "\n")))
 	return true
 }

modules/bonfire/live_test.go

@@ -655,3 +655,60 @@ func TestNestedProcessBuildsNoProgram(t *testing.T) {
 		t.Errorf("the item's success line is missing:\n%s", ansi.Strip(out.String()))
 	}
 }
+
+// TestPrintAboveAfterCancel (WR-09): cancelling the context given to
+// Process ends its view and unregisters it without waiting for the work
+// function; the lines printed afterwards through Println, Info and
+// bonfire.PrintAbove (which reports false, so the caller prints the line
+// itself, as diary does) are printed plainly once each, none lost.
+func TestPrintAboveAfterCancel(t *testing.T) {
+	out := &lockedBuffer{}
+	c := newConsole(strings.NewReader(""), out, io.Discard, noEnv, boolPtr(true), boolPtr(true))
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	var got []string
+	err := Process(ctx, c, "Pushing", []string{"first", "second", "third"}, func(_ context.Context, item string) error {
+		got = append(got, item)
+		if item != "second" {
+			return nil
+		}
+		cancel()
+		registered := true
+		for deadline := time.Now().Add(2 * time.Second); time.Now().Before(deadline); time.Sleep(5 * time.Millisecond) {
+			liveMu.Lock()
+			registered = activeView != nil
+			liveMu.Unlock()
+			if !registered {
+				break
+			}
+		}
+		if registered {
+			t.Errorf("view still registered 2s after the cancellation")
+		}
+		c.Println("after-cancel-println")
+		c.Info("after-cancel-info")
+		if PrintAbove("after-cancel-printabove") {
+			t.Errorf("PrintAbove reported true after the cancellation")
+		} else {
+			_, _ = io.WriteString(out, "after-cancel-printabove\n")
+		}
+		return nil
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Errorf("Process returned %v, want context.Canceled", err)
+	}
+	for _, item := range got {
+		if item == "third" {
+			t.Error("fn ran for the item after the cancellation")
+		}
+	}
+	plain := ansi.Strip(out.String())
+	for _, line := range []string{"after-cancel-println", "after-cancel-info", "after-cancel-printabove"} {
+		if n := strings.Count(plain, line); n != 1 {
+			t.Errorf("%q printed %d times, want 1:\n%s", line, n, plain)
+		}
+	}
+	if PrintAbove("after") {
+		t.Error("a view is still registered after Process returned")
+	}
+}