From 49a10690c6be7436123e2056f04ee05d7582052d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 02:59:28 +0000 Subject: [PATCH] Connect invocation execution to one reporter --- docs/roadmap/implementation.md | 12 +- execute.go | 209 ++++++++++++--------------------- execute_reporting_test.go | 142 ++++++++++++++++++++++ flag_values.go | 4 +- identity.go | 12 +- reporter.go | 26 ++++ 6 files changed, 261 insertions(+), 144 deletions(-) create mode 100644 execute_reporting_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index efe0910..4b45d9a 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Invocation reporting implementation plan -Status: Stages 1–6 complete. Stage 7 is next. +Status: Stages 1–7 complete. Stage 8 is next. ## Objective and authority @@ -275,7 +275,7 @@ buildability. ## Stage 7: Connect invocation execution to one reporter -Status: pending. +Status: complete. Primary files: [identity.go](../../identity.go), [execute.go](../../execute.go), [route.go](../../route.go), @@ -307,6 +307,14 @@ cleanup writes, ignored writer failures, repeated invocations, alias identity, and usage guidance in quiet. Preserve factory-free help/preflight behavior and existing input conversion/cause tests under the new API. +The invocation now owns one reporter, applies ordered reporting selection before +preparation and handler execution, and finalizes normal outcomes through the +reporter. Focused internal suites pass. Root-package execution remains blocked +by the planned help/version migration and remaining legacy test callers; the +root test command was run and its compile errors are recorded for those later +steps. Focused invocation coverage has been added for reporter freshness, +selection, quiet usage guidance, early errors, and ignored output failures. + ## Stage 8: Migrate help and version presentation Status: pending. diff --git a/execute.go b/execute.go index 15efbcd..d05ad01 100644 --- a/execute.go +++ b/execute.go @@ -18,45 +18,59 @@ import ( // nil context returns an operational failure, and nil streams are replaced with // io.Discard. func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outcome { - if ctx == nil { - return a.present(Failure(errors.New("invocation context must not be nil")), nil, streams) - } - streams = normalizeStreams(streams) current := newExecution(a, ctx, streams) + if a == nil { + err := errors.New("cannot execute commands on a nil app") + return current.finalize(Failure(err.Error()).WithCause(err), nil) + } + if ctx == nil { + err := errors.New("invocation context must not be nil") + return current.finalize(Failure(err.Error()).WithCause(err), nil) + } routedSnapshot := current.snapshot current.route = resolveEnteredRoute(args, routedSnapshot) parsed := current.parse(current.route.args) + current.reporter.applySelection(parsed.Reporting) + var outcome Outcome + path := parsed.Path if parsed.Failure != nil { - return current.present(Usage(errors.New(parsed.Failure.Message)), parsed.Path) - } - if current.route.kind == routeDiscovery { + outcome = Usage(parsed.Failure.Message) + } else if current.route.kind == routeDiscovery { if parsed.HelpRequested { - return current.present(current.renderHelp(parsed.Path), clonePath(current.route.enteredPath)) + outcome = current.renderHelp(parsed.Path) + path = clonePath(current.route.enteredPath) + } else { + entered := current.route.enteredPath.String() + outcome = Usage(fmt.Sprintf("incomplete command path %q; use %q --help", entered, entered)) + path = clonePath(current.route.enteredPath) } - entered := current.route.enteredPath.String() - return current.present(Usage(fmt.Errorf("incomplete command path %q; use %q --help", entered, entered)), clonePath(current.route.enteredPath)) - } - payload, ok := routedSnapshot.runners[parsed.Key] - if !ok || payload == nil { - return current.present(current.renderHelp(parsed.Path), parsed.Path) - } - if parsed.HelpRequested || !payload.hasRun() { - if _, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, false); err != nil { - return current.present(Usage(err), parsed.Path) + } else { + payload, ok := routedSnapshot.runners[parsed.Key] + if !ok || payload == nil { + outcome = current.renderHelp(parsed.Path) + } else if parsed.HelpRequested || !payload.hasRun() { + if _, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, false); err != nil { + outcome = classifyInputError(err) + } else { + outcome = current.renderHelp(parsed.Path) + } + } else { + run, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, true) + if err != nil { + outcome = classifyInputError(err) + } else { + outcome = run(current) + } } - return current.present(current.renderHelp(parsed.Path), parsed.Path) } - run, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, true) - if err != nil { - return current.present(Usage(err), parsed.Path) - } - return current.present(run(current), parsed.Path) + return current.finalize(outcome, path) } type execution struct { app *App ctx context.Context streams Streams + reporter *Reporter options commandmeta.ParseOptions snapshot *routingSnapshot session *internalcobra.Session @@ -78,7 +92,7 @@ func (current *execution) commandContext() commandExecution { func (current *execution) invocation() Invocation { return Invocation{ Context: current.ctx, - Streams: current.streams, + reporter: current.reporter, enteredPath: current.route.enteredPath.String(), canonicalPath: current.route.canonicalPath.String(), } @@ -89,11 +103,13 @@ func (current commandExecution) renderHelpTargets(targets []string) Outcome { } func newExecution(app *App, ctx context.Context, streams Streams) *execution { + streams = normalizeStreams(streams) current := &execution{ - app: app, - ctx: ctx, - streams: streams, - options: commandmeta.ParseOptions{Context: ctx, Out: streams.Out, Err: streams.Err}, + app: app, + ctx: ctx, + streams: streams, + reporter: newReporter(streams), + options: commandmeta.ParseOptions{Context: ctx, Out: streams.Out, Err: streams.Err}, } if app != nil { current.snapshot = app.routeSnapshot() @@ -108,7 +124,7 @@ func (current *execution) parse(args []string) commandmeta.Result { if current.session == nil { current.prepareSession() } - return current.session.Parse(args) + return reduceReportingOccurrences(current.session.Parse(args)) } func (current *execution) prepareSession() { @@ -145,7 +161,8 @@ func (current *execution) presentationSession() *internalcobra.Session { func (current *execution) renderHelp(path []string) Outcome { if current.app == nil { - return Failure(errors.New("cannot render help on a nil app")) + err := errors.New("cannot render help on a nil app") + return Failure(err.Error()).WithCause(err) } session := current.presentationSession() view := current.app.helpView(current.route, current.snapshot) @@ -154,11 +171,13 @@ func (current *execution) renderHelp(path []string) Outcome { func (current *execution) renderHelpTargets(targets []string) Outcome { if current.app == nil { - return Failure(errors.New("cannot render help on a nil app")) + err := errors.New("cannot render help on a nil app") + return Failure(err.Error()).WithCause(err) } snapshot := current.app.routeSnapshot() if snapshot == nil { - return Failure(errors.New("cannot render help without command metadata")) + err := errors.New("cannot render help without command metadata") + return Failure(err.Error()).WithCause(err) } route := resolveEnteredRoute(targets, snapshot) // Explicit help targets are paths, not command invocations with operands. @@ -192,21 +211,6 @@ func (current *execution) usageLine(path []string) string { return current.app.usageFallback() } -func (current *execution) present(outcome Outcome, path []string) Outcome { - usageLine := func(path []string) string { return current.usageLine(path) } - switch { - case current.route.kind == routeDiscovery: - path = clonePath(current.route.enteredPath) - usageLine = func([]string) string { - return fmt.Sprintf("Usage: %s %s [command]", current.app.identity.Name, current.route.enteredPath.String()) - } - case current.route.kind == routeCanonical && !current.route.enteredPath.Equal(current.route.canonicalPath): - path = clonePath(current.route.enteredPath) - usageLine = func([]string) string { return current.routeUsageLine() } - } - return current.app.presentWithUsageLine(outcome, path, current.streams, usageLine) -} - func (current *execution) routeUsageLine() string { line := current.usageLine(clonePath(current.route.canonicalPath)) canonical := strings.TrimSpace(current.app.identity.Name + " " + current.route.canonicalPath.String()) @@ -215,57 +219,31 @@ func (current *execution) routeUsageLine() string { return line + "\nAlias for: " + canonical } -func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome { - return a.presentWithUsageLine(outcome, path, streams, a.usageLine) +func (current *execution) finalize(outcome Outcome, path []string) Outcome { + if outcome.Kind == OutcomeUsage { + details := append([]Detail(nil), outcome.Explanation.Details...) + details = append(details, func(w io.Writer) error { + line := current.usageText(path) + if !strings.HasSuffix(line, "\n") { + line += "\n" + } + _, err := io.WriteString(w, line) + return err + }) + outcome.Explanation.Details = details + } + return current.reporter.finalize(outcome, nil) } -func (a *App) presentWithUsageLine(outcome Outcome, path []string, streams Streams, usageLine func([]string) string) Outcome { - if outcome.Kind == "" && outcome.Status == 0 && outcome.Domain == "" && - outcome.Cause == nil && outcome.DeliveryError == nil && - !outcome.ReportIncomplete && len(outcome.Diagnostics) == 0 { - outcome.Kind = OutcomeSuccess +func (current *execution) usageText(path []string) string { + switch { + case current.route.kind == routeDiscovery && current.app != nil: + return fmt.Sprintf("Usage: %s %s [command]", current.app.identity.Name, current.route.enteredPath.String()) + case current.route.kind == routeCanonical && !current.route.enteredPath.Equal(current.route.canonicalPath): + return current.routeUsageLine() + default: + return current.usageLine(path) } - if outcome.DiagnosticDelivery == "" { - outcome.DiagnosticDelivery = DeliveryNone - } - streams = normalizeStreams(streams) - if outcome.DiagnosticDelivery == DeliveryComplete { - return outcome - } - diagnostics := append([]Diagnostic(nil), outcome.Diagnostics...) - if outcome.Cause != nil && !hasErrorDiagnostic(diagnostics) { - diagnostics = append(diagnostics, MessageDiagnostic(SeverityError, outcome.Cause.Error())) - } - if outcome.Kind == OutcomeUsage { - diagnostics = append(diagnostics, MessageDiagnostic(SeverityError, usageLine(path))) - } - outcome.Diagnostics = diagnostics - if len(diagnostics) == 0 { - return outcome - } - delivered := false - for _, diagnostic := range diagnostics { - if diagnostic.Write == nil { - continue - } - delivered = true - writer := &checkedWriter{writer: streams.Err} - writeErr := diagnostic.Write(writer) - if writer.failure != nil { - if writeErr == nil { - writeErr = writer.failure - } else if !errors.Is(writeErr, writer.failure) { - writeErr = errors.Join(writeErr, writer.failure) - } - } - if writeErr != nil { - return deliveryFailure(outcome, writeErr) - } - } - if delivered { - outcome.DiagnosticDelivery = DeliveryComplete - } - return outcome } func (a *App) usageLine(path []string) string { @@ -284,42 +262,3 @@ func (a *App) usageFallback() string { } return fmt.Sprintf("Usage: %s [command]", name) } - -func hasErrorDiagnostic(diagnostics []Diagnostic) bool { - for _, diagnostic := range diagnostics { - if diagnostic.Write != nil && diagnostic.Severity == SeverityError { - return true - } - } - return false -} - -func deliveryFailure(outcome Outcome, err error) Outcome { - if outcome.DeliveryError == nil { - outcome.DeliveryError = err - } else { - outcome.DeliveryError = errors.Join(outcome.DeliveryError, err) - } - outcome.DiagnosticDelivery = DeliveryIncomplete - if outcome.Kind != OutcomeDomain { - outcome.Kind = OutcomeOperational - outcome.Status = 1 - } - return outcome -} - -type checkedWriter struct { - writer io.Writer - failure error -} - -func (w *checkedWriter) Write(data []byte) (int, error) { - n, err := w.writer.Write(data) - if err == nil && n < len(data) { - err = io.ErrShortWrite - } - if err != nil && w.failure == nil { - w.failure = err - } - return n, err -} diff --git a/execute_reporting_test.go b/execute_reporting_test.go new file mode 100644 index 0000000..25b030c --- /dev/null +++ b/execute_reporting_test.go @@ -0,0 +1,142 @@ +package commandkit + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "strings" + "testing" +) + +func TestExecuteGivesEachInvocationItsReporterAndAppliesSelection(t *testing.T) { + var reporters []*Reporter + command := NewCommand(CommandSpec[struct{}]{ + Name: "run", + Run: func(inv Invocation, _ struct{}) Outcome { + reporter := inv.Reporter() + reporters = append(reporters, reporter) + if _, err := fmt.Fprint(reporter.Output(), "result\n"); err != nil { + t.Errorf("write result: %v", err) + } + if err := reporter.Warn("attention"); err != nil { + t.Errorf("submit warning: %v", err) + } + return Outcome{ + Kind: OutcomeDomain, Domain: "accepted", Status: 0, + Presentation: PresentationResult{ + Output: DeliveryResult{State: DeliveryIncomplete, WriteError: io.ErrClosedPipe}, + }, + } + }, + }) + app, err := New(Identity{Name: "Example"}, command) + if err != nil { + t.Fatal(err) + } + + var quietOut, quietErr bytes.Buffer + quiet := app.Execute(context.Background(), []string{"run", "--quiet"}, Streams{Out: &quietOut, Err: &quietErr}) + if quiet.Kind != OutcomeDomain || quiet.Status != 0 || quiet.Domain != "accepted" { + t.Fatalf("quiet outcome = %#v", quiet) + } + if quietOut.Len() != 0 || quietErr.Len() != 0 { + t.Fatalf("quiet output = %q, diagnostics = %q", quietOut.String(), quietErr.String()) + } + if quiet.Presentation.Output.State != DeliverySuppressed || quiet.Presentation.Diagnostics.State != DeliverySuppressed { + t.Fatalf("quiet presentation = %#v", quiet.Presentation) + } + + var verboseOut, verboseErr bytes.Buffer + verbose := app.Execute(context.Background(), []string{"run", "--verbose"}, Streams{Out: &verboseOut, Err: &verboseErr}) + if verbose.Kind != OutcomeDomain || verbose.Status != 0 { + t.Fatalf("verbose outcome = %#v", verbose) + } + if verboseOut.String() != "result\n" || verboseErr.String() != "WARN: attention\n" { + t.Fatalf("verbose output = %q, diagnostics = %q", verboseOut.String(), verboseErr.String()) + } + if verbose.Presentation.Output.State != DeliveryComplete || verbose.Presentation.Diagnostics.State != DeliveryComplete { + t.Fatalf("verbose presentation = %#v", verbose.Presentation) + } + if len(reporters) != 2 || reporters[0] == nil || reporters[1] == nil || reporters[0] == reporters[1] { + t.Fatalf("invocation reporters = %#v", reporters) + } + if _, err := reporters[0].Output().Write([]byte("late")); !errors.Is(err, ErrReporterClosed) { + t.Fatalf("write through finalized reporter error = %v", err) + } +} + +func TestExecuteQuietUsageKeepsAliasGuidanceVisible(t *testing.T) { + command := NewCommand(CommandSpec[struct{}]{ + Name: "run", + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) + app, err := New(Identity{Name: "Example"}, command, Alias("r", "run")) + if err != nil { + t.Fatal(err) + } + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"r", "--quiet", "--unknown"}, Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage || outcome.Status != 2 { + t.Fatalf("outcome = %#v", outcome) + } + if output.Len() != 0 { + t.Fatalf("usage output = %q", output.String()) + } + if got := diagnostics.String(); !strings.Contains(got, "ERROR: unknown flag") || !strings.Contains(got, "Usage: Example r") || !strings.Contains(got, "Alias for: Example run") { + t.Fatalf("usage diagnostics = %q", got) + } + if outcome.Presentation.Diagnostics.State != DeliveryComplete { + t.Fatalf("diagnostic presentation = %#v", outcome.Presentation.Diagnostics) + } +} + +type executeReportingFailureWriter struct{ err error } + +func (w executeReportingFailureWriter) Write([]byte) (int, error) { return 0, w.err } + +func TestExecuteFinalizesEarlyAndIgnoredWriterFailures(t *testing.T) { + command := NewCommand(CommandSpec[struct{}]{ + Name: "run", + Run: func(inv Invocation, _ struct{}) Outcome { + _, _ = inv.Reporter().Output().Write([]byte("report")) + return Success() + }, + }) + app, err := New(Identity{Name: "Example"}, command) + if err != nil { + t.Fatal(err) + } + + writeErr := errors.New("result destination failed") + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"run"}, Streams{ + Out: executeReportingFailureWriter{err: writeErr}, Err: &diagnostics, + }) + if outcome.Kind != OutcomeOperational || outcome.Status != 1 || !errors.Is(outcome.Presentation.Output.WriteError, writeErr) { + t.Fatalf("ignored output failure outcome = %#v", outcome) + } + if !strings.Contains(diagnostics.String(), "ERROR: Command failed.") { + t.Fatalf("fallback diagnostic = %q", diagnostics.String()) + } + + diagnostics.Reset() + outcome = app.Execute(nil, []string{"run"}, Streams{Err: &diagnostics}) + if outcome.Kind != OutcomeOperational || outcome.Status != 1 || outcome.Cause == nil { + t.Fatalf("nil context outcome = %#v", outcome) + } + if !strings.Contains(diagnostics.String(), "ERROR: invocation context must not be nil\n") { + t.Fatalf("nil context diagnostic = %q", diagnostics.String()) + } + + diagnostics.Reset() + var nilApp *App + outcome = nilApp.Execute(context.Background(), []string{"run"}, Streams{Err: &diagnostics}) + if outcome.Kind != OutcomeOperational || outcome.Status != 1 || outcome.Cause == nil { + t.Fatalf("nil app outcome = %#v", outcome) + } + if !strings.Contains(diagnostics.String(), "ERROR: cannot execute commands on a nil app\n") { + t.Fatalf("nil app diagnostic = %q", diagnostics.String()) + } +} diff --git a/flag_values.go b/flag_values.go index 0350bd6..c9efdb3 100644 --- a/flag_values.go +++ b/flag_values.go @@ -90,7 +90,7 @@ func (values FlagValues[I]) ApplyTo(target *I, selected ...Flag[I]) error { func classifyInputError(err error) Outcome { if errors.Is(err, ErrFlagWiring) { - return Failure(err) + return Failure(err.Error()).WithCause(err) } - return Usage(err) + return Usage(err.Error()).WithCause(err) } diff --git a/identity.go b/identity.go index e155243..bf300c8 100644 --- a/identity.go +++ b/identity.go @@ -37,20 +37,22 @@ func normalizeStreams(streams Streams) Streams { return streams } -// Invocation provides execution context and output streams to a command -// handler. For command aliases, EnteredPath and CanonicalPath expose the route -// the caller used and the selected canonical command path. Both omit the +// Invocation provides execution context and reporting to a command handler. +// For command aliases, EnteredPath and CanonicalPath expose the route the +// caller used and the selected canonical command path. Both omit the // executable name, flags, and operands. type Invocation struct { // Context carries caller cancellation and request-scoped values. Context context.Context - // Streams are the caller-selected output destinations. - Streams Streams + reporter *Reporter enteredPath string canonicalPath string } +// Reporter returns the reporting facility for this invocation. +func (inv Invocation) Reporter() *Reporter { return inv.reporter } + // EnteredPath returns the command path used to invoke the selected command, // excluding the executable name, flags, and operands. It is empty for an // Invocation that was not created by command execution. diff --git a/reporter.go b/reporter.go index 1b82a03..395415a 100644 --- a/reporter.go +++ b/reporter.go @@ -5,6 +5,8 @@ import ( "io" "strings" "sync" + + commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" ) type reportingMode uint8 @@ -100,6 +102,30 @@ func newReporter(streams Streams) *Reporter { }} } +func (r *Reporter) applySelection(selection commandmeta.ReportingSelection) { + if r == nil || r.state == nil { + return + } + state := r.state + state.mu.Lock() + defer state.mu.Unlock() + if state.closed { + return + } + switch selection.Mode { + case commandmeta.ReportingModeQuiet: + state.mode = modeQuiet + case commandmeta.ReportingModeVerbose: + state.mode = modeVerbose + case commandmeta.ReportingModeDebug: + state.mode = modeDebug + default: + state.mode = modeOrdinary + } + state.progress = selection.Progress + state.progressSet = selection.ProgressSet +} + // Output returns a managed writer for substantive command results. func (r *Reporter) Output() io.Writer { if r == nil || r.state == nil {