From 7a1067d3872e6816706db69823d650ed8aa67793 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 03:02:08 +0000 Subject: [PATCH] Route help and version through the reporter --- builtins.go | 97 +++++++---------------------- builtins_reporting_test.go | 110 +++++++++++++++++++++++++++++++++ docs/roadmap/implementation.md | 11 +++- execute.go | 7 ++- route_test.go | 4 +- 5 files changed, 146 insertions(+), 83 deletions(-) create mode 100644 builtins_reporting_test.go diff --git a/builtins.go b/builtins.go index 2f4fbe6..d9ae607 100644 --- a/builtins.go +++ b/builtins.go @@ -1,8 +1,6 @@ package commandkit import ( - "bytes" - "errors" "fmt" "io" "runtime/debug" @@ -58,7 +56,7 @@ func (a *App) builtinCommands() []Declaration { if identity.Version == "" { identity.Version = "dev" } - return renderVersionOutcome(identity, input.Format, invocation.Streams) + return renderVersionOutcome(identity, input.Format, invocation.Reporter()) }, }) return []Declaration{help, version} @@ -78,78 +76,41 @@ func parseVersionFormat(value string) (string, error) { return value, nil } -func (a *App) renderHelpOutcome(path []string, streams Streams) Outcome { - session := internalcobra.Prepare(a.routeSnapshot().metadata, commandmeta.ParseOptions{}) - return a.renderHelpOutcomeWithSession(path, streams, session) -} - -func (a *App) renderHelpTargets(targets []string, streams Streams) Outcome { - session := internalcobra.Prepare(a.routeSnapshot().metadata, commandmeta.ParseOptions{}) - return a.renderHelpTargetsWithSession(targets, streams, session) -} - -func (a *App) renderHelpTargetsWithSession(targets []string, streams Streams, session *internalcobra.Session) Outcome { - snapshot := a.routeSnapshot() - route := resolveEnteredRoute(targets, snapshot) - if route.kind == routeUnknown { - return a.renderHelpOutcomeWithSession(targets, streams, session) - } - options := commandmeta.ParseOptions{} - switch route.kind { - case routeCanonical: - options.FlagAliases = snapshot.flagAliases[route.canonicalPath.Key()].neutralRecords() - case routeDiscovery: - options.FlagAliases = snapshot.discoveryFlags[route.discoveryPath.Key()].neutralRecords() - } - session = internalcobra.Prepare(snapshot.metadata, options) - return a.renderHelpViewOutcomeWithSession(a.helpView(route, snapshot), streams, session) -} - -func (a *App) renderHelpOutcomeWithSession(path []string, streams Streams, session *internalcobra.Session) Outcome { - return a.renderHelpViewOutcomeWithSession(commandmeta.HelpView{EnteredPath: path, CanonicalPath: path}, streams, session) -} - -func (a *App) renderHelpViewOutcomeWithSession(view commandmeta.HelpView, streams Streams, session *internalcobra.Session) Outcome { +func (a *App) renderHelpViewOutcomeWithSession(view commandmeta.HelpView, reporter *Reporter, session *internalcobra.Session) Outcome { content, failure := session.RenderHelpView(view) if failure != nil { if failure.Usage { - return Usage(errors.New(failure.Message)) + return Usage(failure.Message) } - return Failure(errors.New(failure.Message)) - } - if err := writeReport(streams.Out, []byte(content)); err != nil { - return incompleteReport(err) + return Failure(failure.Message) } + _ = reporter.RenderOutput(func(w io.Writer) error { + _, err := io.WriteString(w, content) + return err + }) return Success() } -func renderVersionOutcome(identity Identity, format string, streams Streams) Outcome { - var content []byte +func renderVersionOutcome(identity Identity, format string, reporter *Reporter) Outcome { + if identity.Version == "" { + identity.Version = "dev" + } switch format { case "text": - name := identity.Name - if identity.Version == "" { - identity.Version = "dev" - } - line := name + " " + identity.Version + line := identity.Name + " " + identity.Version if identity.Revision != "" { line += " (" + identity.Revision + ")" } - content = []byte(line + "\n") + _ = reporter.RenderOutput(func(w io.Writer) error { + _, err := io.WriteString(w, line+"\n") + return err + }) case "yaml": - if identity.Version == "" { - identity.Version = "dev" - } - var buffer bytes.Buffer - if err := encodeYAML(&buffer, versionDocument{Name: identity.Name, Version: identity.Version, Revision: identity.Revision}); err != nil { - return Failure(err) - } - content = buffer.Bytes() + _ = reporter.RenderOutput(func(w io.Writer) error { + return encodeYAML(w, versionDocument{Name: identity.Name, Version: identity.Version, Revision: identity.Revision}) + }) default: - return Usage(fmt.Errorf("unsupported version format %q", format)) - } - if err := writeReport(streams.Out, content); err != nil { - return incompleteReport(err) + return Usage(fmt.Sprintf("unsupported version format %q", format)) } return Success() } @@ -160,22 +121,6 @@ func encodeYAML(writer io.Writer, document any) error { return encoder.Encode(document) } -func writeReport(writer io.Writer, content []byte) error { - checked := &checkedWriter{writer: writer} - _, err := checked.Write(content) - if err == nil { - err = checked.failure - } - return err -} - -func incompleteReport(err error) Outcome { - outcome := Failure(err) - outcome.ReportIncomplete = true - outcome.DeliveryError = err - return outcome -} - func readBuildInfo() *debug.BuildInfo { info, ok := debug.ReadBuildInfo() if !ok { diff --git a/builtins_reporting_test.go b/builtins_reporting_test.go new file mode 100644 index 0000000..e06bc8d --- /dev/null +++ b/builtins_reporting_test.go @@ -0,0 +1,110 @@ +package commandkit + +import ( + "bytes" + "context" + "errors" + "io" + "strings" + "testing" +) + +func builtinReportingTestApp(t *testing.T) *App { + t.Helper() + command := NewCommand(CommandSpec[struct{}]{ + Name: "run", + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) + app, err := New(Identity{Name: "Example", Version: "1.2.3"}, command, + Group("promptkit", "PromptKit", Group("list", "List", simpleRunnable("collections"))), + Alias("r", "run"), Alias("list collections", "promptkit list collections"), + ) + if err != nil { + t.Fatal(err) + } + return app +} + +func TestHelpAndVersionUseManagedOutput(t *testing.T) { + app := builtinReportingTestApp(t) + for _, args := range [][]string{{"help", "--quiet"}, {"version", "--quiet"}, {"list", "--help", "--quiet"}} { + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), args, Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != OutcomeSuccess || outcome.Status != 0 { + t.Errorf("Execute(%q) = %#v", args, outcome) + } + if output.Len() != 0 || diagnostics.Len() != 0 { + t.Errorf("Execute(%q) wrote output %q or diagnostics %q", args, output.String(), diagnostics.String()) + } + if outcome.Presentation.Output.State != DeliverySuppressed { + t.Errorf("Execute(%q) output presentation = %#v", args, outcome.Presentation.Output) + } + } + + for _, test := range []struct { + args []string + want string + }{ + {args: []string{"help", "run"}, want: "run"}, + {args: []string{"help", "r"}, want: "Alias for: Example run"}, + {args: []string{"list", "--help"}, want: "Reporting Flags"}, + {args: []string{"version"}, want: "Example 1.2.3"}, + {args: []string{"version", "--format=yaml"}, want: "version: 1.2.3"}, + } { + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), test.args, Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != OutcomeSuccess || !strings.Contains(output.String(), test.want) { + t.Errorf("Execute(%q) = %#v, output %q", test.args, outcome, output.String()) + } + } +} + +func TestHelpAndVersionDestinationFailuresUseReporterObservations(t *testing.T) { + app := builtinReportingTestApp(t) + writeErr := errors.New("help/version destination failed") + for _, args := range [][]string{{"help", "run"}, {"version"}} { + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), args, Streams{ + Out: executeReportingFailureWriter{err: writeErr}, Err: &diagnostics, + }) + if outcome.Kind != OutcomeOperational || outcome.Status != 1 { + t.Errorf("Execute(%q) = %#v", args, outcome) + } + if !errors.Is(outcome.Presentation.Output.WriteError, writeErr) || outcome.Presentation.Output.State != DeliveryIncomplete { + t.Errorf("Execute(%q) presentation = %#v", args, outcome.Presentation) + } + if !strings.Contains(diagnostics.String(), "ERROR: Command failed.") { + t.Errorf("Execute(%q) diagnostics = %q", args, diagnostics.String()) + } + } +} + +func TestInvalidHelpAndVersionRequestsRemainUsageOutcomes(t *testing.T) { + app := builtinReportingTestApp(t) + for _, args := range [][]string{{"help", "unknown"}, {"version", "--scope=unknown"}, {"version", "--format=unknown"}} { + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), args, Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage || outcome.Status != 2 { + t.Errorf("Execute(%q) = %#v", args, outcome) + } + if !strings.Contains(diagnostics.String(), "Usage: Example") { + t.Errorf("Execute(%q) diagnostics = %q", args, diagnostics.String()) + } + } +} + +func TestQuietBuiltinRenderingSuppressesDestinationWrites(t *testing.T) { + app := builtinReportingTestApp(t) + + // A quiet successful run must not touch its supplied result destination. + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"version", "--quiet"}, Streams{ + Out: executeReportingFailureWriter{err: io.ErrClosedPipe}, Err: &diagnostics, + }) + if outcome.Kind != OutcomeSuccess || outcome.Presentation.Output.State != DeliverySuppressed { + t.Fatalf("quiet version outcome = %#v", outcome) + } + if diagnostics.Len() != 0 { + t.Fatalf("quiet version diagnostics = %q", diagnostics.String()) + } +} diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 4b45d9a..415fdb8 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Invocation reporting implementation plan -Status: Stages 1–7 complete. Stage 8 is next. +Status: Stages 1–8 complete. Stage 9 is next. ## Objective and authority @@ -317,7 +317,7 @@ selection, quiet usage guidance, early errors, and ignored output failures. ## Stage 8: Migrate help and version presentation -Status: pending. +Status: complete. Primary files: [builtins.go](../../builtins.go), help metadata/rendering, and built-in consumer tests. @@ -340,6 +340,13 @@ alias-only discovery, shared reporting flags, invalid help/version requests, factory-free behavior, and help/version destination failures. Invalid requests remain usage outcomes unless presentation subsequently fails. +Help and version reports now render through the invocation reporter, including +quiet-mode rendering and destination-failure accounting. Tests cover quiet +success, canonical and alias help, alias-only discovery, invalid requests, and +destination failures. The internal suites pass; root-package execution remains +deferred because the existing external test package still references APIs +scheduled for the later suite reconciliation. + ## Stage 9: Add core debug events Status: pending. diff --git a/execute.go b/execute.go index d05ad01..b35baf9 100644 --- a/execute.go +++ b/execute.go @@ -166,7 +166,7 @@ func (current *execution) renderHelp(path []string) Outcome { } session := current.presentationSession() view := current.app.helpView(current.route, current.snapshot) - return current.app.renderHelpViewOutcomeWithSession(view, current.streams, session) + return current.app.renderHelpViewOutcomeWithSession(view, current.reporter, session) } func (current *execution) renderHelpTargets(targets []string) Outcome { @@ -184,7 +184,8 @@ func (current *execution) renderHelpTargets(targets []string) Outcome { // Discovery routes have no canonical path and retain only trailing options. if route.kind == routeUnknown || len(route.args) != len(route.canonicalPath) { session := internalcobra.Prepare(snapshot.metadata, current.options) - return current.app.renderHelpOutcomeWithSession(targets, current.streams, session) + view := commandmeta.HelpView{EnteredPath: targets, CanonicalPath: targets} + return current.app.renderHelpViewOutcomeWithSession(view, current.reporter, session) } options := current.options switch route.kind { @@ -195,7 +196,7 @@ func (current *execution) renderHelpTargets(targets []string) Outcome { } session := internalcobra.Prepare(snapshot.metadata, options) view := current.app.helpView(route, snapshot) - return current.app.renderHelpViewOutcomeWithSession(view, current.streams, session) + return current.app.renderHelpViewOutcomeWithSession(view, current.reporter, session) } func (current *execution) usageLine(path []string) string { diff --git a/route_test.go b/route_test.go index e235025..85b987d 100644 --- a/route_test.go +++ b/route_test.go @@ -36,7 +36,7 @@ func TestDiscoveryUsageRetainsEnteredPrefix(t *testing.T) { t.Run(prefix+"/"+test.name, func(t *testing.T) { var output, diagnostics bytes.Buffer outcome := app.Execute(context.Background(), append(strings.Fields(prefix), test.tail...), Streams{Out: &output, Err: &diagnostics}) - if outcome.Kind != OutcomeUsage || outcome.Status != 2 || outcome.DiagnosticDelivery != DeliveryComplete || output.Len() != 0 { + if outcome.Kind != OutcomeUsage || outcome.Status != 2 || outcome.Presentation.Diagnostics.State != DeliveryComplete || output.Len() != 0 { t.Fatalf("outcome = %#v, output %q", outcome, output.String()) } for _, fragment := range []string{test.want, "Usage: Demo " + prefix} { @@ -828,7 +828,7 @@ func TestAliasUsageSurvivesRegistrationDuringExecution(t *testing.T) { Name: "inspect", Run: func(Invocation, struct{}) Outcome { registrationErr = test.register(app) - return Usage(errors.New("operation rejected")) + return Usage("operation rejected").WithCause(errors.New("operation rejected")) }, }) var err error