Close deferred reporting before final delivery
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
49
reporter.go
49
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()
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user