From dacc7435510b7d33f602de2cbef9e3fcbf32a60d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 03:03:10 +0000 Subject: [PATCH] Keep nested help commands with application handlers --- docs/roadmap/remediation.md | 40 ++++++++++++++++++++++----- execute.go | 2 +- execute_external_test.go | 55 +++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 8ac605e..9627437 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Performance implementation review findings -Status: R-01 requires remediation. +Status: R-01 complete; all recorded remediation work is addressed. Date: 2026-10-09. @@ -36,8 +36,8 @@ The selected structure is present: configuration framing scans only the YAML preamble while retaining the selected full native and framed buffers. -These changes implement PF-01–PF-06. The dispatch defect below is an ownership -regression in the session integration, rather than a missing subsystem. The +These changes implement PF-01–PF-06. The review identified an ownership +regression in the session integration, corrected under R-01 below. The [internal overview](../internal/overview.md) owns the maintained package inventory. ## R-01: restrict built-in help interception to the root command @@ -78,14 +78,14 @@ accepts it. No such additional restriction was selected. Required work: -- [ ] Identify the root built-in help command by its complete selected path +- [x] Identify the root built-in help command by its complete selected path or an equivalent explicit reserved-command identity. A root-path check such as `len(parsed.Path) == 1 && parsed.Path[0] == "help"` is sufficient for the current registry. Other runnable declarations must continue to their handler. -- [ ] Add a public behavior regression for an application-owned nested `help` +- [x] Add a public behavior regression for an application-owned nested `help` command. Verify it receives its typed input and that its handler outcome is preserved. Keep local `--help` and `help session help` as metadata operations. -- [ ] Retain existing root help, invalid-target, usage, fresh-invocation, +- [x] Retain existing root help, invalid-target, usage, fresh-invocation, registration-refresh, and diagnostic behavior. Use existing coverage where sufficient; verify the relevant regression and default suite after the fix. @@ -93,7 +93,33 @@ Do not remediate this by rejecting nested `help` names or imposing new command naming rules. Keep the session optimization and established ownership boundary. Mark this item complete only after the correction and its evidence are recorded. -## Evidence and limits +### Completion evidence + +**Completed:** `App.Execute` now recognizes the built-in help command only when +its selected path is the root `help` command. Nested application commands keep +their existing handler path and naming freedom; session reuse is retained. + +The public regression `TestExecuteRunsApplicationOwnedNestedHelp` verifies a +required typed operand reaches the nested handler and preserves its domain, +status, and cause. Local `session help --help` and explicit `help session help` +render the command's metadata without invoking that handler or requiring its +execution operand. + +The regression first failed against the original dispatcher with the reported +unexpected-input-type failure and passed after the correction. Focused validation +and the full default suite passed with readonly, offline module access: + +```sh +GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -count=1 . -run '^TestExecuteRunsApplicationOwnedNestedHelp$' +GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -count=1 ./... +``` + +Existing root help, invalid-target, usage, fresh-invocation, registration-refresh, +and diagnostic tests passed. Changed Go files were formatted; document links +and whitespace checks passed. No new public contract or command-name restriction +was introduced. + +## Original review evidence and limits - Reviewed the production diffs, changed behavioral tests, benchmark fixtures, architecture documentation, and completion/measurement records. Traced cache diff --git a/execute.go b/execute.go index b0f1f8e..7f047d8 100644 --- a/execute.go +++ b/execute.go @@ -42,7 +42,7 @@ func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outco if err := command.payload.checkInput(input); err != nil { return current.present(Usage(err), parsed.Path) } - if command.name == "help" { + if len(parsed.Path) == 1 && parsed.Path[0] == "help" { help, ok := input.(*helpInput) if !ok { return current.present(Failure(errors.New("built-in help input has an unexpected type")), parsed.Path) diff --git a/execute_external_test.go b/execute_external_test.go index a818a22..a569015 100644 --- a/execute_external_test.go +++ b/execute_external_test.go @@ -340,6 +340,61 @@ func TestExecuteHelpSkipsRequiredInputsAndRejectsNilContext(t *testing.T) { } } +func TestExecuteRunsApplicationOwnedNestedHelp(t *testing.T) { + type input struct{ topic commandkit.Supplied[string] } + var received []input + domainCause := errors.New("application help result") + help, err := commandkit.NewCommand(commandkit.CommandSpec[input]{ + Name: "help", + Description: "Explain a session topic.", + Arguments: []commandkit.Argument[input]{commandkit.ValueArgument( + commandkit.ArgumentMeta{Name: "TOPIC"}, true, + func(value string) (string, error) { return value, nil }, + func(target *input, value commandkit.Supplied[string]) { target.topic = value }, + )}, + Run: func(_ commandkit.Invocation, value input) commandkit.Outcome { + received = append(received, value) + return commandkit.Domain("session-help", 7, domainCause) + }, + }) + if err != nil { + t.Fatal(err) + } + session, err := commandkit.Group("session", "Session commands.", help) + if err != nil { + t.Fatal(err) + } + app, err := commandkit.New(commandkit.Identity{Name: "example"}, session) + if err != nil { + t.Fatal(err) + } + + var diagnostic bytes.Buffer + outcome := app.Execute(context.Background(), []string{"session", "help", "restore"}, commandkit.Streams{Err: &diagnostic}) + if outcome.Kind != commandkit.OutcomeDomain || outcome.Domain != "session-help" || outcome.Status != 7 || !errors.Is(outcome.Cause, domainCause) { + t.Fatalf("nested help handler outcome = %#v, diagnostics %q", outcome, diagnostic.String()) + } + if len(received) != 1 || received[0].topic != (commandkit.Supplied[string]{Value: "restore", Set: true}) { + t.Fatalf("nested help handler input = %#v", received) + } + + for _, args := range [][]string{{"session", "help", "--help"}, {"help", "session", "help"}} { + t.Run(strings.Join(args, " "), func(t *testing.T) { + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), args, commandkit.Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != commandkit.OutcomeSuccess || outcome.Status != 0 || diagnostics.Len() != 0 { + t.Fatalf("metadata help outcome = %#v, diagnostics %q", outcome, diagnostics.String()) + } + if !strings.Contains(output.String(), "Explain a session topic.") || !strings.Contains(output.String(), "TOPIC") { + t.Fatalf("metadata help omitted nested command details: %q", output.String()) + } + if len(received) != 1 { + t.Fatalf("metadata help invoked the application handler: %#v", received) + } + }) + } +} + func TestHelpRendersRegisteredMetadataAndResolvesExactPaths(t *testing.T) { type input struct { count commandkit.Supplied[int]