Explain invocation presentation failures

This commit is contained in:
2026-10-26 14:28:58 +00:00
parent 215fc34d51
commit 13a4d4eacd
7 changed files with 96 additions and 13 deletions

View File

@@ -73,7 +73,7 @@ func TestHelpAndVersionDestinationFailuresUseReporterObservations(t *testing.T)
if !errors.Is(outcome.Presentation.Output.WriteError, writeErr) || outcome.Presentation.Output.State != DeliveryIncomplete { if !errors.Is(outcome.Presentation.Output.WriteError, writeErr) || outcome.Presentation.Output.State != DeliveryIncomplete {
t.Errorf("Execute(%q) presentation = %#v", args, outcome.Presentation) t.Errorf("Execute(%q) presentation = %#v", args, outcome.Presentation)
} }
if !strings.Contains(diagnostics.String(), "ERROR: Command failed.") { if !strings.Contains(diagnostics.String(), "deliver") || !strings.Contains(diagnostics.String(), "output") {
t.Errorf("Execute(%q) diagnostics = %q", args, diagnostics.String()) t.Errorf("Execute(%q) diagnostics = %q", args, diagnostics.String())
} }
} }

View File

@@ -1,6 +1,6 @@
# Invocation reporting audit remediation # Invocation reporting audit remediation
Status: remediation in progress; 2 of 7 findings complete. Status: remediation in progress; 3 of 7 findings complete.
## Review scope ## Review scope
@@ -185,7 +185,12 @@ detail-rendering path as needed to establish that the shared fix applies there.
## 3. Explain failures introduced by CommandKit presentation ## 3. Explain failures introduced by CommandKit presentation
Priority: P2. Status: open. Priority: P2. Status: complete.
- [x] Completed: presentation failures that turn success into failure now retain
a core-owned rendering/delivery explanation. Supplied explanations and causes,
generic handler fallback, quiet rendering, domain status, and failed-sink
behavior remain covered. Validation: `GOTOOLCHAIN=local go test ./...` at root.
### Finding and evidence ### Finding and evidence

View File

@@ -518,7 +518,7 @@ func TestHelpEntryFormsPresentReportAndDiagnosticFailures(t *testing.T) {
for _, args := range [][]string{nil, {"notes"}, {"--help"}, {"-h"}, {"notes", "--help"}, {"help"}, {"help", "notes"}} { for _, args := range [][]string{nil, {"notes"}, {"--help"}, {"-h"}, {"notes", "--help"}, {"help"}, {"help", "notes"}} {
var diagnostics bytes.Buffer var diagnostics bytes.Buffer
result := app.Execute(context.Background(), args, commandkit.Streams{Out: failingWriter{reportErr}, Err: &diagnostics}) result := app.Execute(context.Background(), args, commandkit.Streams{Out: failingWriter{reportErr}, Err: &diagnostics})
if result.Kind != commandkit.OutcomeOperational || result.Presentation.Output.State != commandkit.DeliveryIncomplete || !errors.Is(result.Presentation.Output.WriteError, reportErr) || result.Presentation.Diagnostics.State != commandkit.DeliveryComplete || !strings.Contains(diagnostics.String(), "ERROR: Command failed.") { if result.Kind != commandkit.OutcomeOperational || result.Presentation.Output.State != commandkit.DeliveryIncomplete || !errors.Is(result.Presentation.Output.WriteError, reportErr) || result.Presentation.Diagnostics.State != commandkit.DeliveryComplete || (!strings.Contains(diagnostics.String(), "deliver") || !strings.Contains(diagnostics.String(), "output")) {
t.Fatalf("%v: result=%#v diagnostics=%q", args, result, diagnostics.String()) t.Fatalf("%v: result=%#v diagnostics=%q", args, result, diagnostics.String())
} }
result = app.Execute(context.Background(), args, commandkit.Streams{Out: failingWriter{reportErr}, Err: failingWriter{diagnosticErr}}) result = app.Execute(context.Background(), args, commandkit.Streams{Out: failingWriter{reportErr}, Err: failingWriter{diagnosticErr}})

View File

@@ -117,7 +117,7 @@ func TestExecuteFinalizesEarlyAndIgnoredWriterFailures(t *testing.T) {
if outcome.Kind != OutcomeOperational || outcome.Status != 1 || !errors.Is(outcome.Presentation.Output.WriteError, writeErr) { if outcome.Kind != OutcomeOperational || outcome.Status != 1 || !errors.Is(outcome.Presentation.Output.WriteError, writeErr) {
t.Fatalf("ignored output failure outcome = %#v", outcome) t.Fatalf("ignored output failure outcome = %#v", outcome)
} }
if !strings.Contains(diagnostics.String(), "ERROR: Command failed.") { if !strings.Contains(diagnostics.String(), "deliver") || !strings.Contains(diagnostics.String(), "output") {
t.Fatalf("fallback diagnostic = %q", diagnostics.String()) t.Fatalf("fallback diagnostic = %q", diagnostics.String())
} }

View File

@@ -281,6 +281,9 @@ func (r *Reporter) finalize(outcome Outcome, completion func(*Reporter)) Outcome
state.mu.Lock() state.mu.Lock()
if presentationFailedLocked(state) { if presentationFailedLocked(state) {
if outcome.Status == 0 && !hasExplanation(outcome.Explanation) {
outcome.Explanation.Message = presentationFailureMessage(state)
}
outcome = applyPresentationFailure(outcome) outcome = applyPresentationFailure(outcome)
} }
if outcome.Status != 0 { if outcome.Status != 0 {
@@ -328,18 +331,38 @@ func applyPresentationFailure(outcome Outcome) Outcome {
} }
func ensureFallbackExplanation(outcome Outcome) Outcome { func ensureFallbackExplanation(outcome Outcome) Outcome {
if outcome.Explanation.Message != "" { if hasExplanation(outcome.Explanation) {
return outcome return outcome
} }
for _, detail := range outcome.Explanation.Details {
if detail != nil {
return outcome
}
}
outcome.Explanation = Explanation{Message: "Command failed."} outcome.Explanation = Explanation{Message: "Command failed."}
return outcome return outcome
} }
func hasExplanation(explanation Explanation) bool {
if explanation.Message != "" {
return true
}
for _, detail := range explanation.Details {
if detail != nil {
return true
}
}
return false
}
func presentationFailureMessage(state *reportingState) string {
switch {
case state.output.renderError != nil:
return "Unable to render command output."
case state.output.writeError != nil:
return "Unable to deliver command output."
case state.diagnostics.renderError != nil:
return "Unable to render invocation diagnostics."
default:
return "Unable to deliver invocation diagnostics."
}
}
func renderTerminalLocked(state *reportingState, explanation Explanation) error { func renderTerminalLocked(state *reportingState, explanation Explanation) error {
destination := state.destination(destinationDiagnostics) destination := state.destination(destinationDiagnostics)
destination.submitted = true destination.submitted = true

View File

@@ -1,12 +1,67 @@
package commandkit package commandkit
import ( import (
"bytes"
"errors" "errors"
"fmt" "fmt"
"io" "io"
"strings"
"testing" "testing"
) )
func TestReporterExplainsItsOwnFailuresWithoutExposingCauses(t *testing.T) {
for _, kind := range []string{"output", "diagnostics"} {
for _, quiet := range []bool{false, true} {
t.Run(fmt.Sprintf("%s/quiet=%t", kind, quiet), func(t *testing.T) {
var diagnostics bytes.Buffer
reporter := newReporter(Streams{Err: &diagnostics})
if quiet {
reporter.setVisibility(modeQuiet, false, false)
}
failure := errors.New("private rendering detail")
render := func(io.Writer) error { return failure }
if kind == "output" {
_ = reporter.RenderOutput(render)
} else {
_ = reporter.Warn("", render)
}
cause := errors.New("private application cause")
got := reporter.finalize(Domain("example", 0).WithCause(cause), nil)
if got.Kind != OutcomeDomain || got.Status != 1 || got.Cause != cause {
t.Fatalf("outcome=%#v", got)
}
if !strings.Contains(got.Explanation.Message, "render") || !strings.Contains(got.Explanation.Message, kind) || !strings.Contains(diagnostics.String(), got.Explanation.Message) {
t.Fatalf("explanation=%#v diagnostics=%q", got.Explanation, diagnostics.String())
}
if strings.Contains(diagnostics.String(), "private") {
t.Fatalf("private error text was exposed: %q", diagnostics.String())
}
})
}
}
}
func TestReporterPreservesExplicitSuccessExplanationOnPresentationFailure(t *testing.T) {
for _, message := range []string{"supplied context", ""} {
t.Run(message, func(t *testing.T) {
var diagnostics bytes.Buffer
reporter := newReporter(Streams{Err: &diagnostics})
_ = reporter.RenderOutput(func(io.Writer) error { return errors.New("render failed") })
calls := 0
got := reporter.finalize(Success().WithExplanation(message, func(io.Writer) error {
calls++
return nil
}), nil)
if got.Status != 1 || got.Explanation.Message != message || calls != 1 {
t.Fatalf("outcome=%#v calls=%d", got, calls)
}
if message == "" && diagnostics.Len() != 0 {
t.Fatalf("explicit zero-byte explanation replaced: %q", diagnostics.String())
}
})
}
}
func TestReporterPreservesJoinedRendererEvidence(t *testing.T) { func TestReporterPreservesJoinedRendererEvidence(t *testing.T) {
for _, kind := range []string{"output", "detail"} { for _, kind := range []string{"output", "detail"} {
for _, test := range []struct { for _, test := range []struct {

View File

@@ -592,7 +592,7 @@ func TestReporterPresentationFailureChangesOutcomeAndPreservesDomain(t *testing.
if outcome.Kind != OutcomeOperational || outcome.Status != 1 || !errors.Is(outcome.Cause, cause) { if outcome.Kind != OutcomeOperational || outcome.Status != 1 || !errors.Is(outcome.Cause, cause) {
t.Fatalf("success presentation failure outcome = %#v", outcome) t.Fatalf("success presentation failure outcome = %#v", outcome)
} }
if !errors.Is(outcome.Presentation.Output.WriteError, outputErr) || diagnostics.String() != "ERROR: Command failed.\n" { if !errors.Is(outcome.Presentation.Output.WriteError, outputErr) || (!strings.Contains(diagnostics.String(), "deliver") || !strings.Contains(diagnostics.String(), "output")) {
t.Fatalf("presentation=%#v diagnostics=%q", outcome.Presentation, diagnostics.String()) t.Fatalf("presentation=%#v diagnostics=%q", outcome.Presentation, diagnostics.String())
} }
@@ -644,7 +644,7 @@ func TestReporterUsesHealthyDiagnosticsAfterOutputRenderFailure(t *testing.T) {
if outcome.Kind != OutcomeOperational || outcome.Status != 1 { if outcome.Kind != OutcomeOperational || outcome.Status != 1 {
t.Fatalf("outcome = %#v", outcome) t.Fatalf("outcome = %#v", outcome)
} }
if !errors.Is(outcome.Presentation.Output.RenderError, renderErr) || diagnostics.String() != "ERROR: Command failed.\n" { if !errors.Is(outcome.Presentation.Output.RenderError, renderErr) || (!strings.Contains(diagnostics.String(), "render") || !strings.Contains(diagnostics.String(), "output")) {
t.Fatalf("presentation=%#v diagnostics=%q", outcome.Presentation, diagnostics.String()) t.Fatalf("presentation=%#v diagnostics=%q", outcome.Presentation, diagnostics.String())
} }
} }