Add atomic reporting and message categories

2026-10-26 02:42:22 +00:00
parent f5a5bbac58
commit 5df88a6d94
3 changed files with 490 additions and 1 deletions
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -137,7 +137,7 @@ introduced by reporter synchronization.

 ## Stage 3: Implement atomic rendering and categorized messages

-Status: pending.
+Status: complete.

 1. Implement `RenderOutput`, `Info`, `Warn`, `Debug`, and `Progress` with the
   exact roadmap signatures. A render operation or complete categorized message
--- a/reporter.go
+++ b/reporter.go
@@ -1,7 +1,9 @@
 package commandkit

 import (
+	"errors"
 	"io"
+	"strings"
 	"sync"
 )

@@ -21,6 +23,15 @@ const (
 	destinationDiagnostics
 )

+type messageCategory uint8
+
+const (
+	categoryInfo messageCategory = iota
+	categoryWarning
+	categoryDebug
+	categoryProgress
+)
+
 type reportingState struct {
 	mu          sync.Mutex
 	output      destinationState
@@ -53,6 +64,20 @@ type managedOutputWriter struct {
 	destination reportingDestination
 }

+type coordinatedWriter struct {
+	state       *reportingState
+	destination reportingDestination
+	writeError  error
+}
+
+func (w *coordinatedWriter) Write(p []byte) (int, error) {
+	n, err := w.state.writeLocked(w.destination, p)
+	if err != nil && w.writeError == nil {
+		w.writeError = err
+	}
+	return n, err
+}
+
 func newReporter(streams Streams) *Reporter {
 	streams = normalizeStreams(streams)
 	return &Reporter{state: &reportingState{
@@ -77,6 +102,175 @@ func (w managedOutputWriter) Write(p []byte) (int, error) {
 	return w.state.write(w.destination, p)
 }

+// RenderOutput coordinates a complete substantive report rendering operation.
+func (r *Reporter) RenderOutput(write func(io.Writer) error) error {
+	if r == nil || r.state == nil {
+		return ErrReporterClosed
+	}
+	state := r.state
+	state.mu.Lock()
+	defer state.mu.Unlock()
+	if state.closed {
+		return ErrReporterClosed
+	}
+	if write == nil {
+		return nil
+	}
+
+	destination := state.destination(destinationOutput)
+	destination.submitted = true
+	writer := &coordinatedWriter{state: state, destination: destinationOutput}
+	renderErr := write(writer)
+	if state.mode == modeQuiet {
+		destination.suppressed = true
+	} else if renderErr == nil && writer.writeError == nil {
+		destination.delivered = true
+	}
+	return finishRenderLocked(state, destinationOutput, renderErr, writer.writeError)
+}
+
+// Info submits an informational message.
+func (r *Reporter) Info(message string, details ...Detail) error {
+	return r.submitMessage(categoryInfo, message, details)
+}
+
+// Warn submits a warning message.
+func (r *Reporter) Warn(message string, details ...Detail) error {
+	return r.submitMessage(categoryWarning, message, details)
+}
+
+// Debug submits a debug message.
+func (r *Reporter) Debug(message string, details ...Detail) error {
+	return r.submitMessage(categoryDebug, message, details)
+}
+
+// Progress submits a progress message.
+func (r *Reporter) Progress(message string, details ...Detail) error {
+	return r.submitMessage(categoryProgress, message, details)
+}
+
+func (r *Reporter) submitMessage(category messageCategory, message string, details []Detail) error {
+	if r == nil || r.state == nil {
+		return ErrReporterClosed
+	}
+	state := r.state
+	state.mu.Lock()
+	defer state.mu.Unlock()
+	if state.closed {
+		return ErrReporterClosed
+	}
+
+	details = append([]Detail(nil), details...)
+	hasDetails := false
+	for _, detail := range details {
+		if detail != nil {
+			hasDetails = true
+			break
+		}
+	}
+	if message == "" && !hasDetails {
+		return nil
+	}
+
+	visible := state.visible(category)
+	destination := state.destination(destinationDiagnostics)
+	destination.submitted = true
+	if !visible {
+		destination.suppressed = true
+		if !hasDetails {
+			return nil
+		}
+	}
+
+	var renderErr, writeErr error
+	if visible && message != "" {
+		writer := &coordinatedWriter{state: state, destination: destinationDiagnostics}
+		_, writeErr = writer.Write([]byte(messageLine(category, message)))
+		writeErr = writer.writeError
+	}
+	if visible && writeErr != nil {
+		return writeErr
+	}
+	for _, detail := range details {
+		if detail == nil {
+			continue
+		}
+		var detailErr, detailWriteErr error
+		if visible {
+			writer := &coordinatedWriter{state: state, destination: destinationDiagnostics}
+			detailErr = detail(writer)
+			detailWriteErr = writer.writeError
+		} else {
+			detailErr = detail(io.Discard)
+		}
+		detailRenderErr := recordRendererErrorLocked(state, destinationDiagnostics, detailErr, detailWriteErr)
+		if renderErr == nil {
+			renderErr = detailRenderErr
+		}
+		if writeErr == nil {
+			writeErr = detailWriteErr
+		}
+	}
+	if visible && renderErr == nil && writeErr == nil {
+		destination.delivered = true
+	}
+	return combineOperationErrors(renderErr, writeErr)
+}
+
+func (state *reportingState) visible(category messageCategory) bool {
+	switch category {
+	case categoryInfo, categoryWarning:
+		return state.mode == modeVerbose || state.mode == modeDebug
+	case categoryDebug:
+		return state.mode == modeDebug
+	case categoryProgress:
+		if state.mode == modeQuiet {
+			return false
+		}
+		if state.progressSet {
+			return state.progress
+		}
+		return state.mode == modeDebug
+	default:
+		return false
+	}
+}
+
+func messageLine(category messageCategory, message string) string {
+	prefix := ""
+	switch category {
+	case categoryWarning:
+		prefix = "WARN: "
+	case categoryDebug:
+		prefix = "DEBUG: "
+	}
+	return prefix + strings.TrimSuffix(message, "\n") + "\n"
+}
+
+func finishRenderLocked(state *reportingState, destination reportingDestination, renderErr, writeErr error) error {
+	renderErr = recordRendererErrorLocked(state, destination, renderErr, writeErr)
+	return combineOperationErrors(renderErr, writeErr)
+}
+
+func recordRendererErrorLocked(state *reportingState, destination reportingDestination, renderErr, writeErr error) error {
+	if renderErr == nil || (writeErr != nil && errors.Is(renderErr, writeErr)) {
+		return nil
+	}
+	state.recordRenderErrorLocked(destination, renderErr)
+	return renderErr
+}
+
+func combineOperationErrors(renderErr, writeErr error) error {
+	switch {
+	case renderErr == nil:
+		return writeErr
+	case writeErr == nil || errors.Is(renderErr, writeErr):
+		return renderErr
+	default:
+		return errors.Join(renderErr, writeErr)
+	}
+}
+
 func (state *reportingState) write(destination reportingDestination, p []byte) (int, error) {
 	state.mu.Lock()
 	defer state.mu.Unlock()
--- a/reporter_test.go
+++ b/reporter_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"io"
+	"strings"
 	"testing"
 )

@@ -172,3 +173,297 @@ func TestNewReporterNormalizesNilDestinationsAndStartsOrdinary(t *testing.T) {
 		t.Fatalf("visibility was not retained: mode %v, progress %v (set %v)", reporter.state.mode, reporter.state.progress, reporter.state.progressSet)
 	}
 }
+
+func TestReporterMessageVisibilityAndProgressPrecedence(t *testing.T) {
+	tests := []struct {
+		name        string
+		mode        reportingMode
+		progress    bool
+		progressSet bool
+		want        string
+	}{
+		{name: "ordinary", mode: modeOrdinary},
+		{name: "verbose", mode: modeVerbose, want: "not used\nWARN: not used\n"},
+		{name: "debug", mode: modeDebug, want: "not used\nWARN: not used\nDEBUG: DEBUG\nPROGRESS\n"},
+		{name: "quiet overrides progress", mode: modeQuiet, progress: true, progressSet: true},
+		{name: "explicit ordinary progress", mode: modeOrdinary, progress: true, progressSet: true, want: "PROGRESS\n"},
+		{name: "explicit debug disable", mode: modeDebug, progressSet: true, want: "not used\nWARN: not used\nDEBUG: DEBUG\n"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var output bytes.Buffer
+			reporter := newReporter(Streams{Err: &output})
+			reporter.setVisibility(test.mode, test.progress, test.progressSet)
+			_ = reporter.Info("not used")
+			_ = reporter.Warn("not used")
+			_ = reporter.Debug("DEBUG")
+			_ = reporter.Progress("PROGRESS")
+			if got := output.String(); got != test.want {
+				t.Fatalf("output = %q, want %q", got, test.want)
+			}
+		})
+	}
+}
+
+func TestReporterFormatsMessagesAndPreservesDetailOrder(t *testing.T) {
+	var diagnostics bytes.Buffer
+	reporter := newReporter(Streams{Err: &diagnostics})
+	reporter.setVisibility(modeVerbose, false, false)
+	if err := reporter.Warn("Native warning\nwith second line\n", nil,
+		func(w io.Writer) error { _, err := io.WriteString(w, "detail one\n"); return err },
+		func(w io.Writer) error { _, err := io.WriteString(w, "detail two\n"); return err }); err != nil {
+		t.Fatal(err)
+	}
+	if err := reporter.Info("", func(w io.Writer) error {
+		_, err := io.WriteString(w, "native labelled detail\n")
+		return err
+	}); err != nil {
+		t.Fatal(err)
+	}
+	want := "WARN: Native warning\nwith second line\ndetail one\ndetail two\nnative labelled detail\n"
+	if got := diagnostics.String(); got != want {
+		t.Fatalf("diagnostics = %q, want %q", got, want)
+	}
+}
+
+func TestReporterHiddenRenderersRunAgainstDiscardAndRetainErrors(t *testing.T) {
+	var diagnostics bytes.Buffer
+	reporter := newReporter(Streams{Err: &diagnostics})
+	reporter.setVisibility(modeQuiet, false, false)
+	renderErr := errors.New("native renderer failed")
+	calls := 0
+	err := reporter.Warn("hidden warning", func(w io.Writer) error {
+		calls++
+		if _, writeErr := io.WriteString(w, "hidden detail"); writeErr != nil {
+			return writeErr
+		}
+		return renderErr
+	})
+	if !errors.Is(err, renderErr) {
+		t.Fatalf("Warn() error = %v, want renderer error", err)
+	}
+	if calls != 1 || diagnostics.Len() != 0 {
+		t.Fatalf("hidden renderer calls=%d, destination=%q", calls, diagnostics.String())
+	}
+	result := reporter.state.presentation().Diagnostics
+	if result.State != DeliveryIncomplete || !errors.Is(result.RenderError, renderErr) || result.WriteError != nil {
+		t.Fatalf("diagnostic result = %#v", result)
+	}
+}
+
+func TestReporterRecordsIgnoredRenderErrors(t *testing.T) {
+	renderErr := errors.New("output renderer failed")
+	diagnosticErr := errors.New("diagnostic renderer failed")
+	reporter := newReporter(Streams{Out: io.Discard, Err: io.Discard})
+	reporter.setVisibility(modeVerbose, false, false)
+	_ = reporter.RenderOutput(func(io.Writer) error { return renderErr })
+	_ = reporter.Warn("warning", func(io.Writer) error { return diagnosticErr })
+
+	result := reporter.state.presentation()
+	if !errors.Is(result.Output.RenderError, renderErr) || !errors.Is(result.Diagnostics.RenderError, diagnosticErr) {
+		t.Fatalf("ignored render errors were not retained: %#v", result)
+	}
+}
+
+func TestReporterRenderOutputRunsInQuietAndRetainsFailure(t *testing.T) {
+	destination := &reporterTestWriter{}
+	reporter := newReporter(Streams{Out: destination})
+	reporter.setVisibility(modeQuiet, false, false)
+	renderErr := errors.New("quiet encoder failed")
+	calls := 0
+	_ = reporter.RenderOutput(func(w io.Writer) error {
+		calls++
+		if _, err := io.WriteString(w, "discarded report"); err != nil {
+			return err
+		}
+		return renderErr
+	})
+	result := reporter.state.presentation().Output
+	if calls != 1 || destination.writes != 0 {
+		t.Fatalf("quiet rendering calls=%d destination writes=%d", calls, destination.writes)
+	}
+	if result.State != DeliveryIncomplete || !errors.Is(result.RenderError, renderErr) || result.WriteError != nil {
+		t.Fatalf("quiet render result = %#v", result)
+	}
+}
+
+func TestReporterCountsSuccessfulZeroByteRenderAsSubmitted(t *testing.T) {
+	reporter := newReporter(Streams{Out: io.Discard, Err: io.Discard})
+	if err := reporter.RenderOutput(func(io.Writer) error { return nil }); err != nil {
+		t.Fatal(err)
+	}
+	reporter.setVisibility(modeVerbose, false, false)
+	if err := reporter.Warn("", func(io.Writer) error { return nil }); err != nil {
+		t.Fatal(err)
+	}
+	result := reporter.state.presentation()
+	if result.Output.State != DeliveryComplete || result.Diagnostics.State != DeliveryComplete {
+		t.Fatalf("zero-byte successful render states = %#v", result)
+	}
+}
+
+func TestReporterCountsHiddenZeroByteRenderAsSuppressed(t *testing.T) {
+	reporter := newReporter(Streams{Err: io.Discard})
+	if err := reporter.Warn("", func(io.Writer) error { return nil }); err != nil {
+		t.Fatal(err)
+	}
+	if got := reporter.state.presentation().Diagnostics.State; got != DeliverySuppressed {
+		t.Fatalf("hidden zero-byte render state = %v, want suppressed", got)
+	}
+}
+
+func TestReporterRenderOutputSeparatesRenderAndWriteFailures(t *testing.T) {
+	t.Run("renderer failure leaves destination usable", func(t *testing.T) {
+		var output bytes.Buffer
+		reporter := newReporter(Streams{Out: &output})
+		renderErr := errors.New("render failed")
+		if err := reporter.RenderOutput(func(io.Writer) error { return renderErr }); !errors.Is(err, renderErr) {
+			t.Fatalf("RenderOutput() error = %v", err)
+		}
+		if _, err := reporter.Output().Write([]byte("still usable")); err != nil {
+			t.Fatalf("destination rejected later write: %v", err)
+		}
+		result := reporter.state.presentation().Output
+		if !errors.Is(result.RenderError, renderErr) || result.WriteError != nil || output.String() != "still usable" {
+			t.Fatalf("result=%#v output=%q", result, output.String())
+		}
+	})
+
+	t.Run("propagated write error is not a renderer error", func(t *testing.T) {
+		writeErr := errors.New("writer failed")
+		destination := &reporterTestWriter{err: writeErr}
+		reporter := newReporter(Streams{Out: destination})
+		err := reporter.RenderOutput(func(w io.Writer) error {
+			_, writeErr := io.WriteString(w, "report")
+			return writeErr
+		})
+		if !errors.Is(err, writeErr) {
+			t.Fatalf("RenderOutput() error = %v", err)
+		}
+		result := reporter.state.presentation().Output
+		if result.RenderError != nil || !errors.Is(result.WriteError, writeErr) {
+			t.Fatalf("result = %#v", result)
+		}
+	})
+
+	t.Run("ignored write error is retained and returned", func(t *testing.T) {
+		writeErr := errors.New("writer failed")
+		reporter := newReporter(Streams{Out: &reporterTestWriter{err: writeErr}})
+		err := reporter.RenderOutput(func(w io.Writer) error {
+			_, _ = w.Write([]byte("report"))
+			return nil
+		})
+		if !errors.Is(err, writeErr) {
+			t.Fatalf("RenderOutput() error = %v", err)
+		}
+	})
+
+	t.Run("independent rendering and writing failures remain separate", func(t *testing.T) {
+		writeErr := errors.New("writer failed")
+		renderErr := errors.New("renderer failed independently")
+		reporter := newReporter(Streams{Out: &reporterTestWriter{err: writeErr}})
+		err := reporter.RenderOutput(func(w io.Writer) error {
+			_, _ = w.Write([]byte("report"))
+			return renderErr
+		})
+		if !errors.Is(err, writeErr) || !errors.Is(err, renderErr) {
+			t.Fatalf("RenderOutput() error = %v", err)
+		}
+		result := reporter.state.presentation().Output
+		if !errors.Is(result.WriteError, writeErr) || !errors.Is(result.RenderError, renderErr) {
+			t.Fatalf("result = %#v", result)
+		}
+	})
+}
+
+func TestReporterConcurrentOperationsRemainWhole(t *testing.T) {
+	t.Run("render callback and message", func(t *testing.T) {
+		var output bytes.Buffer
+		reporter := newReporter(Streams{Out: &output, Err: &output})
+		reporter.setVisibility(modeVerbose, false, false)
+		start := make(chan struct{})
+		ready := make(chan struct{}, 2)
+		done := make(chan struct{}, 2)
+		go func() {
+			ready <- struct{}{}
+			<-start
+			_ = reporter.RenderOutput(func(w io.Writer) error {
+				_, _ = io.WriteString(w, "report-")
+				_, err := io.WriteString(w, "complete\n")
+				return err
+			})
+			done <- struct{}{}
+		}()
+		go func() {
+			ready <- struct{}{}
+			<-start
+			_ = reporter.Info("message")
+			done <- struct{}{}
+		}()
+		<-ready
+		<-ready
+		close(start)
+		<-done
+		<-done
+		got := output.String()
+		if got != "report-complete\nmessage\n" && got != "message\nreport-complete\n" {
+			t.Fatalf("concurrent output interleaved complete operations: %q", got)
+		}
+	})
+
+	t.Run("message details and render callback", func(t *testing.T) {
+		var output bytes.Buffer
+		reporter := newReporter(Streams{Out: &output, Err: &output})
+		reporter.setVisibility(modeVerbose, false, false)
+		start := make(chan struct{})
+		ready := make(chan struct{}, 2)
+		done := make(chan struct{}, 2)
+		go func() {
+			ready <- struct{}{}
+			<-start
+			_ = reporter.Info("first", func(w io.Writer) error {
+				_, _ = io.WriteString(w, "detail-")
+				_, err := io.WriteString(w, "complete\n")
+				return err
+			})
+			done <- struct{}{}
+		}()
+		go func() {
+			ready <- struct{}{}
+			<-start
+			_ = reporter.RenderOutput(func(w io.Writer) error {
+				_, err := io.WriteString(w, "report\n")
+				return err
+			})
+			done <- struct{}{}
+		}()
+		<-ready
+		<-ready
+		close(start)
+		<-done
+		<-done
+		got := output.String()
+		if got != "first\ndetail-complete\nreport\n" && got != "report\nfirst\ndetail-complete\n" {
+			t.Fatalf("concurrent output interleaved complete operations: %q", got)
+		}
+	})
+}
+
+func TestReporterEmptyHeadingDoesNotAddPrefixOrBlankLine(t *testing.T) {
+	var diagnostics bytes.Buffer
+	reporter := newReporter(Streams{Err: &diagnostics})
+	reporter.setVisibility(modeVerbose, false, false)
+	if err := reporter.Warn("", func(w io.Writer) error {
+		_, err := io.WriteString(w, "native multiline\nsecond line\n")
+		return err
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if got := diagnostics.String(); got != "native multiline\nsecond line\n" {
+		t.Fatalf("diagnostics = %q", got)
+	}
+	if strings.HasPrefix(diagnostics.String(), "WARN:") {
+		t.Fatal("empty heading added a category prefix")
+	}
+}