From 8dbb25aba952176d0b54e6af55aa55d62b8cdbe8 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 15:44:55 +0000 Subject: [PATCH] Release reporter destination references after finalization --- docs/roadmap/audit/catalog.md | 6 +++++- docs/roadmap/implementation.md | 7 ++++++- reporter.go | 2 ++ reporter_test.go | 37 ++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index 535a341..b1d0127 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -1,6 +1,6 @@ # Consolidated audit catalog -Status: **20 distinct open findings: 20 P3**, consolidated from 30 +Status: **19 distinct open findings: 19 P3**, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: `db732e3a1de55c844f77870e1c6db683c2685d5d`. @@ -159,6 +159,10 @@ Also noted in the [documentation cross-references](13-documentation.md#previousl ## F07 — P3: Closed reporting handles retain both borrowed destinations +**Complete.** Finalization drops direct destination references after presentation +is captured. Focused reporter tests cover successful, failed and canceled outcomes, +closed handles, retained causes, and caller ownership. + A retained closed `Reporter`, `Output`, or `DeferredReporter` keeps the shared reporting state and both destination writers reachable. Applications retaining a closed handle can therefore retain large buffers or resource-bearing writer diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f708247..f31dc88 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Audit remediation implementation plan -Status: in progress; Stages 1–6 complete. +Status: in progress; Stages 1–7 complete. Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint, intervening changes recorded decisions and retired the completed older @@ -280,6 +280,11 @@ script/shell validation. Stage 14 relies on this repaired fixture. ## Stage 7 — Release finalized reporter destination references +**Complete.** Finalization clears both borrowed writer references after +capturing presentation, without closing them. Tests cover success, operational +failure, cancellation, late handles and writer ownership; focused reporter tests +pass in the offline aligned Go 1.27.0 workspace with `-mod=readonly`. + Findings: F07. Source: `reporter.go`; builds on stage 3. 1. After deferred/completion/terminal presentation and capture of observations, diff --git a/reporter.go b/reporter.go index 77527ed..4c38777 100644 --- a/reporter.go +++ b/reporter.go @@ -327,6 +327,8 @@ func (r *Reporter) finalize(outcome Outcome, completion func(*Reporter)) Outcome state.closed = true state.deferred = nil outcome.Presentation = state.presentationLocked() + state.output.writer = nil + state.diagnostics.writer = nil state.mu.Unlock() return normalizeOutcome(outcome) } diff --git a/reporter_test.go b/reporter_test.go index 183fa00..a2555e9 100644 --- a/reporter_test.go +++ b/reporter_test.go @@ -2,6 +2,7 @@ package commandkit import ( "bytes" + "context" "errors" "io" "strings" @@ -710,3 +711,39 @@ func setReporterVisibility(reporter *Reporter, mode reportingMode, progress, pro } reporter.applySelection(selection) } + +func TestReporterFinalizationReleasesBorrowedDestinations(t *testing.T) { + cause := errors.New("operation failed") + for _, tc := range []struct { + name string + outcome Outcome + cause error + }{ + {name: "success", outcome: Success()}, + {name: "failure", outcome: Failure("failed").WithCause(cause), cause: cause}, + {name: "cancellation", outcome: Failure("canceled").WithCause(context.Canceled), cause: context.Canceled}, + } { + t.Run(tc.name, func(t *testing.T) { + out, diagnostics := &callerOwnedWriter{}, &callerOwnedWriter{} + reporter := newReporter(Streams{Out: out, Err: diagnostics}) + output, deferred := reporter.Output(), reporter.AtEnd() + result := reporter.finalize(tc.outcome, nil) + if reporter.state.output.writer != nil || reporter.state.diagnostics.writer != nil || out.closeCalls != 0 || diagnostics.closeCalls != 0 || tc.cause != nil && !errors.Is(result.Cause, tc.cause) { + t.Fatalf("outcome=%#v state=%#v", result, reporter.state) + } + outLen, diagnosticLen := out.Len(), diagnostics.Len() + if _, err := output.Write([]byte("late")); !errors.Is(err, ErrReporterClosed) { + t.Errorf("output error=%v", err) + } + if err := reporter.Warn("late"); !errors.Is(err, ErrReporterClosed) { + t.Errorf("reporter error=%v", err) + } + if err := deferred.Warn("late"); !errors.Is(err, ErrReporterClosed) { + t.Errorf("deferred error=%v", err) + } + if out.Len() != outLen || diagnostics.Len() != diagnosticLen { + t.Fatal("late operation touched destinations") + } + }) + } +}