From 1fc6409f95e9d0e6ba34d886fb4be5c8e46a0cbd Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 15:40:01 +0000 Subject: [PATCH] Close deferred reporting before final delivery --- docs/consumers/guide.md | 4 ++- docs/roadmap/audit/catalog.md | 8 ++++- docs/roadmap/implementation.md | 9 +++++- reporter.go | 49 ++++++++++------------------ reporter_deferred_test.go | 58 ++++++++++++++++++++++++++++++++-- reporter_error_test.go | 4 +-- reporter_test.go | 47 +++++++++++++++++---------- 7 files changed, 123 insertions(+), 56 deletions(-) diff --git a/docs/consumers/guide.md b/docs/consumers/guide.md index fdc787b..8961010 100644 --- a/docs/consumers/guide.md +++ b/docs/consumers/guide.md @@ -136,7 +136,9 @@ installation alongside aliases and integration commands. `Report()` for formatted substantive values and optional text templates; use `Output()` or `RenderOutput()` for deliberate raw output. Use `Warn`, `Info`, `Progress`, and `Debug` for categorized messages. Queue a summary with -`AtEnd().Info` when it belongs after the handler returns. The +`AtEnd().Info` when it belongs after the handler returns. Deferred submissions +are accepted until finalization begins; later submissions return +`ErrReporterClosed`. The [CLI reference](../cli.md#reporting) owns flag grammar, visibility, prefixes, and status behavior. diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index bd4781b..9ebf3bd 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -1,6 +1,6 @@ # Consolidated audit catalog -Status: **25 distinct open findings: three P2 and 22 P3**, consolidated from 30 +Status: **24 distinct open findings: two P2 and 22 P3**, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: `db732e3a1de55c844f77870e1c6db683c2685d5d`. @@ -76,6 +76,12 @@ Also noted in the [documentation cross-references](13-documentation.md#previousl ## F03 — P2: Finalization can discard an accepted deferred message +**Complete.** The acceptance cutoff and batch detachment now share one locked +transition. A deterministic regression reproduces the former lost submission and +checks late empty/hidden submissions, accepted delivery, completion, and terminal +ordering. Focused reporter tests passed normally and under the race detector; +existing tests cover visibility and presentation failures. + Core reporter finalization detaches the deferred batch before closing acceptance. A concurrent submission can return success while presentation is in progress, then be discarded when finalization clears the queue. The returned diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 5d0fd56..54b442b 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Audit remediation implementation plan -Status: in progress; Stages 1–2 complete. +Status: in progress; Stages 1–3 complete. Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint, intervening changes recorded decisions and retired the completed older @@ -163,6 +163,13 @@ Complete when both binding replay and per-kind seed boundaries are isolated. ## Stage 3 — Close deferred acceptance atomically +**Complete.** Finalization closes deferred acceptance while detaching its +batch; completion and terminal reporting remain available. The channel-controlled +regression failed against the original code and now passes alongside focused +reporter and hidden-deferred tests, both normally and with `-race -count=1`, in +the aligned Go 1.27.0 workspace with offline, read-only module resolution. The +old reporter test wrappers were replaced with real selection and finalization. + Findings: F03 and the reporter-lifecycle portion of F24. Source: `reporter.go`. 1. Add a private deferred-acceptance/finalization state distinct from the final diff --git a/reporter.go b/reporter.go index 5548863..77527ed 100644 --- a/reporter.go +++ b/reporter.go @@ -36,15 +36,16 @@ const ( ) type reportingState struct { - mu sync.Mutex - output destinationState - diagnostics destinationState - closed bool - mode reportingMode - format Format - progress bool - progressSet bool - deferred []deferredMessage + mu sync.Mutex + output destinationState + diagnostics destinationState + closed bool + deferredClosed bool + mode reportingMode + format Format + progress bool + progressSet bool + deferred []deferredMessage } type destinationState struct { @@ -78,8 +79,8 @@ type Reporter struct { } // DeferredReporter queues messages for delivery during invocation finalization. -// Copies share the same invocation reporting state. Submissions made after -// finalization return ErrReporterClosed. +// Copies share the same invocation reporting state. Submissions return +// ErrReporterClosed once finalization begins, before the accepted batch is delivered. type DeferredReporter struct { state *reportingState } @@ -222,7 +223,8 @@ func (r *Reporter) Progress(message string, details ...Detail) error { // AtEnd returns a view for messages delivered after the handler returns and // before terminal explanation delivery. The deferred view shares this -// invocation's state. +// invocation's state. Acceptance ends when finalization begins, before queued +// messages are delivered. func (r *Reporter) AtEnd() DeferredReporter { if r == nil { return DeferredReporter{} @@ -258,7 +260,7 @@ func (r DeferredReporter) enqueue(category messageCategory, message string, deta state := r.state state.mu.Lock() defer state.mu.Unlock() - if state.closed { + if state.closed || state.deferredClosed { return ErrReporterClosed } if !hasDetails(details) { @@ -290,6 +292,7 @@ func (r *Reporter) finalize(outcome Outcome, completion func(*Reporter)) Outcome state.mu.Unlock() return normalizeOutcome(outcome) } + state.deferredClosed = true deferred := state.deferred state.deferred = nil state.mu.Unlock() @@ -664,23 +667,3 @@ func (state *reportingState) deliveryResultLocked(destination reportingDestinati } return result } - -func (r *Reporter) setVisibility(mode reportingMode, progress bool, progressSet bool) { - if r == nil || r.state == nil { - return - } - r.state.mu.Lock() - r.state.mode = mode - r.state.progress = progress - r.state.progressSet = progressSet - r.state.mu.Unlock() -} - -func (r *Reporter) close() { - if r == nil || r.state == nil { - return - } - r.state.mu.Lock() - r.state.closed = true - r.state.mu.Unlock() -} diff --git a/reporter_deferred_test.go b/reporter_deferred_test.go index c797e0f..afb0315 100644 --- a/reporter_deferred_test.go +++ b/reporter_deferred_test.go @@ -13,7 +13,7 @@ func TestHiddenSimpleDeferredMessagesDoNotAccumulate(t *testing.T) { for _, mode := range []reportingMode{modeOrdinary, modeQuiet} { t.Run(fmt.Sprint(mode), func(t *testing.T) { reporter := newReporter(Streams{}) - reporter.setVisibility(mode, false, false) + setReporterVisibility(reporter, mode, false, false) for index := 0; index < 256; index++ { if err := reporter.AtEnd().Debug(fmt.Sprintf("hidden %d", index), nil); err != nil { t.Fatal(err) @@ -45,7 +45,7 @@ func TestHiddenSimpleDeferredMessagesDoNotAccumulate(t *testing.T) { func TestHiddenDeferredDetailsRemainDeferredAndCaptured(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) - reporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(reporter, modeQuiet, false, false) failure := errors.New("hidden rendering failed") var order []string details := []Detail{func(w io.Writer) error { @@ -67,3 +67,57 @@ func TestHiddenDeferredDetailsRemainDeferredAndCaptured(t *testing.T) { t.Fatalf("hidden content leaked or queue retained: %q", diagnostics.String()) } } + +type gatedDiagnosticWriter struct { + bytes.Buffer + entered chan struct{} + release chan struct{} +} + +func (w *gatedDiagnosticWriter) Write(p []byte) (int, error) { + if w.entered != nil { + close(w.entered) + w.entered = nil + <-w.release + } + return w.Buffer.Write(p) +} + +func TestReporterDeferredAcceptanceEndsBeforeDelivery(t *testing.T) { + writer := &gatedDiagnosticWriter{entered: make(chan struct{}), release: make(chan struct{})} + entered := writer.entered + reporter := newReporter(Streams{Err: writer}) + setReporterVisibility(reporter, modeVerbose, false, false) + deferred := reporter.AtEnd() + if err := deferred.Warn("accepted"); err != nil { + t.Fatal(err) + } + completionEntered, finishCompletion := make(chan struct{}), make(chan struct{}) + done := make(chan Outcome, 1) + go func() { + done <- reporter.finalize(Failure("terminal"), func(r *Reporter) { + close(completionEntered) + <-finishCompletion + _ = r.Info("completion") + }) + }() + <-entered + late := make(chan error, 1) + go func() { late <- deferred.Warn("late") }() + close(writer.release) + <-completionEntered + if err := <-late; !errors.Is(err, ErrReporterClosed) { + t.Errorf("late submission = %v", err) + } + if err := deferred.Debug(""); !errors.Is(err, ErrReporterClosed) { + t.Errorf("empty hidden submission = %v", err) + } + close(finishCompletion) + outcome := <-done + if got := writer.String(); got != "WARN: accepted\ncompletion\nERROR: terminal\n" { + t.Errorf("diagnostics = %q", got) + } + if outcome.Presentation.Diagnostics.State != DeliveryComplete { + t.Fatalf("outcome = %#v", outcome) + } +} diff --git a/reporter_error_test.go b/reporter_error_test.go index 570765d..b5569eb 100644 --- a/reporter_error_test.go +++ b/reporter_error_test.go @@ -16,7 +16,7 @@ func TestReporterExplainsItsOwnFailuresWithoutExposingCauses(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) if quiet { - reporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(reporter, modeQuiet, false, false) } failure := errors.New("private rendering detail") render := func(io.Writer) error { return failure } @@ -84,7 +84,7 @@ func TestReporterPreservesJoinedRendererEvidence(t *testing.T) { writeErr := errors.Join(errors.New("sink failure"), errors.New("sink detail")) destination := &reporterTestWriter{err: writeErr} reporter := newReporter(Streams{Out: destination, Err: destination}) - reporter.setVisibility(modeVerbose, false, false) + setReporterVisibility(reporter, modeVerbose, false, false) render := func(w io.Writer) error { _, err := io.WriteString(w, "report") return test.combine(renderErr, err) diff --git a/reporter_test.go b/reporter_test.go index f532029..183fa00 100644 --- a/reporter_test.go +++ b/reporter_test.go @@ -6,6 +6,8 @@ import ( "io" "strings" "testing" + + commandmeta "go.maximumdirect.net/commandkit/internal/commandmeta" ) type reporterTestWriter struct { @@ -77,7 +79,7 @@ func TestReporterLeavesCallerWriterOpen(t *testing.T) { func TestReporterQuietOutputDiscardsSuccessfully(t *testing.T) { var destination bytes.Buffer reporter := newReporter(Streams{Out: &destination}) - reporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(reporter, modeQuiet, false, false) writer := reporter.Output() if n, err := writer.Write([]byte("hidden")); err != nil || n != len("hidden") { @@ -174,8 +176,8 @@ func TestReporterClosedHandlesReturnClosedErrorIncludingQuiet(t *testing.T) { var destination bytes.Buffer reporter := newReporter(Streams{Out: &destination}) writer := reporter.Output() - reporter.setVisibility(modeQuiet, false, false) - reporter.close() + setReporterVisibility(reporter, modeQuiet, false, false) + _ = reporter.finalize(Success(), nil) if n, err := writer.Write([]byte("late")); n != 0 || !errors.Is(err, ErrReporterClosed) { t.Fatalf("late Write() = %d, %v", n, err) @@ -193,7 +195,7 @@ func TestNewReporterNormalizesNilDestinationsAndStartsOrdinary(t *testing.T) { if reporter.state.mode != modeOrdinary || reporter.state.progress || reporter.state.progressSet { t.Fatalf("initial visibility = mode %v, progress %v (set %v)", reporter.state.mode, reporter.state.progress, reporter.state.progressSet) } - reporter.setVisibility(modeDebug, true, true) + setReporterVisibility(reporter, modeDebug, true, true) if reporter.state.mode != modeDebug || !reporter.state.progress || !reporter.state.progressSet { t.Fatalf("visibility was not retained: mode %v, progress %v (set %v)", reporter.state.mode, reporter.state.progress, reporter.state.progressSet) } @@ -219,7 +221,7 @@ func TestReporterMessageVisibilityAndProgressPrecedence(t *testing.T) { 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) + setReporterVisibility(reporter, test.mode, test.progress, test.progressSet) _ = reporter.Info("not used") _ = reporter.Warn("not used") _ = reporter.Debug("DEBUG") @@ -234,7 +236,7 @@ func TestReporterMessageVisibilityAndProgressPrecedence(t *testing.T) { func TestReporterFormatsMessagesAndPreservesDetailOrder(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) - reporter.setVisibility(modeVerbose, false, false) + setReporterVisibility(reporter, 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 { @@ -255,7 +257,7 @@ func TestReporterFormatsMessagesAndPreservesDetailOrder(t *testing.T) { func TestReporterHiddenRenderersRunAgainstDiscardAndRetainErrors(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) - reporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(reporter, modeQuiet, false, false) renderErr := errors.New("native renderer failed") calls := 0 err := reporter.Warn("hidden warning", func(w io.Writer) error { @@ -281,7 +283,7 @@ 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) + setReporterVisibility(reporter, modeVerbose, false, false) _ = reporter.RenderOutput(func(io.Writer) error { return renderErr }) _ = reporter.Warn("warning", func(io.Writer) error { return diagnosticErr }) @@ -294,7 +296,7 @@ func TestReporterRecordsIgnoredRenderErrors(t *testing.T) { func TestReporterRenderOutputRunsInQuietAndRetainsFailure(t *testing.T) { destination := &reporterTestWriter{} reporter := newReporter(Streams{Out: destination}) - reporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(reporter, modeQuiet, false, false) renderErr := errors.New("quiet encoder failed") calls := 0 _ = reporter.RenderOutput(func(w io.Writer) error { @@ -318,7 +320,7 @@ func TestReporterCountsSuccessfulZeroByteRenderAsSubmitted(t *testing.T) { if err := reporter.RenderOutput(func(io.Writer) error { return nil }); err != nil { t.Fatal(err) } - reporter.setVisibility(modeVerbose, false, false) + setReporterVisibility(reporter, modeVerbose, false, false) if err := reporter.Warn("", func(io.Writer) error { return nil }); err != nil { t.Fatal(err) } @@ -406,7 +408,7 @@ 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) + setReporterVisibility(reporter, modeVerbose, false, false) start := make(chan struct{}) ready := make(chan struct{}, 2) done := make(chan struct{}, 2) @@ -440,7 +442,7 @@ func TestReporterConcurrentOperationsRemainWhole(t *testing.T) { 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) + setReporterVisibility(reporter, modeVerbose, false, false) start := make(chan struct{}) ready := make(chan struct{}, 2) done := make(chan struct{}, 2) @@ -478,7 +480,7 @@ func TestReporterConcurrentOperationsRemainWhole(t *testing.T) { func TestReporterEmptyHeadingDoesNotAddPrefixOrBlankLine(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) - reporter.setVisibility(modeVerbose, false, false) + setReporterVisibility(reporter, modeVerbose, false, false) if err := reporter.Warn("", func(w io.Writer) error { _, err := io.WriteString(w, "native multiline\nsecond line\n") return err @@ -496,7 +498,7 @@ func TestReporterEmptyHeadingDoesNotAddPrefixOrBlankLine(t *testing.T) { func TestReporterFinalizesDeferredMessagesBeforeCompletionAndExplanation(t *testing.T) { var output, diagnostics bytes.Buffer reporter := newReporter(Streams{Out: &output, Err: &diagnostics}) - reporter.setVisibility(modeDebug, false, false) + setReporterVisibility(reporter, modeDebug, false, false) if err := reporter.Info("started"); err != nil { t.Fatal(err) } @@ -522,7 +524,7 @@ func TestReporterFinalizesDeferredMessagesBeforeCompletionAndExplanation(t *test func TestReporterDeferredSubmissionCopiesDetailsAndRetainsDeferredFailures(t *testing.T) { var diagnostics bytes.Buffer reporter := newReporter(Streams{Err: &diagnostics}) - reporter.setVisibility(modeVerbose, false, false) + setReporterVisibility(reporter, modeVerbose, false, false) writeFirst := Detail(func(w io.Writer) error { _, err := io.WriteString(w, "first detail\n"); return err }) writeSecond := Detail(func(w io.Writer) error { _, err := io.WriteString(w, "second detail\n"); return err }) details := []Detail{writeFirst} @@ -542,7 +544,7 @@ func TestReporterDeferredSubmissionCopiesDetailsAndRetainsDeferredFailures(t *te renderErr := errors.New("deferred renderer failed") failedReporter := newReporter(Streams{Err: io.Discard}) - failedReporter.setVisibility(modeQuiet, false, false) + setReporterVisibility(failedReporter, modeQuiet, false, false) if err := failedReporter.AtEnd().Warn("hidden", func(io.Writer) error { return renderErr }); err != nil { t.Fatal(err) } @@ -695,3 +697,16 @@ func TestReporterFinalizationClosesOutputAndDeferredHandles(t *testing.T) { t.Fatalf("late output emitted: result=%q diagnostics=%q", output.String(), diagnostics.String()) } } + +func setReporterVisibility(reporter *Reporter, mode reportingMode, progress, progressSet bool) { + selection := commandmeta.ReportingSelection{Progress: progress, ProgressSet: progressSet} + switch mode { + case modeQuiet: + selection.Mode = commandmeta.ReportingModeQuiet + case modeVerbose: + selection.Mode = commandmeta.ReportingModeVerbose + case modeDebug: + selection.Mode = commandmeta.ReportingModeDebug + } + reporter.applySelection(selection) +}