diff --git a/docs/cli.md b/docs/cli.md index 9b95ab6..1074015 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -245,8 +245,8 @@ Both commands require one ID operand and accept `--config`, `--overlay`, and `--format text|yaml` (default `text`). Prompt inspection also accepts singleton `--version`, defaulting to the empty string. PromptKit owns version selection: an omitted version works for a single-version ID, while an ambiguous ID is an -operational error. These commands have no built-in `--scope` selection; their namespace is -`promptkit`. +operational error. These commands have no built-in `--scope` selection; their +namespace is `promptkit`. Prompt YAML has these fields. Input order is preserved and an empty input list is `[]`. The prompt hash is an opaque owning-library equality value. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 8e019c8..20271eb 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Initial implementation review and remediation -Status: remediation in progress. +Status: all ten findings addressed. Date: 2026-10-07. @@ -14,8 +14,8 @@ behavioral tests against the accepted ADRs, architecture policy, and selected These are static review findings. No tests were added or executed during this review; prior completion notes are historical evidence, not a new validation -result. The triggering cases below follow from the current code paths. Focused -regression checks are proposed completion evidence for subsequent fixes. +result. The triggering cases below describe those baseline code paths. The remediation +progress and validation evidence record the subsequent fixes. The findings below describe the review baseline. Completion is tracked here as implementation fixes land. @@ -25,29 +25,6 @@ implementation fixes land. - R-01 complete: required selectors are checked after scope selection; unrelated scopes remain usable, and help describes conditional requiredness. -## Architecture assessment - -The main architecture is implemented: - -- The root `commandkit` package owns public named types, opaque handles, - per-instance registration, typed bindings, outcomes, standard handlers, scope - assembly, and ConfigKit/PromptKit integration. -- `internal/cobra` owns Cobra/pflag operations behind a neutral internal bridge. - Public signatures do not expose the backend. Each invocation rebuilds backend - state; configuration and engine acquisition remain lazy. -- Standard families own their namespaces. Applications retain custom commands, - loaders, domain behavior, executable entry points, streams, and process exit. -- Scope capabilities and prompt/profile callbacks can be supplied independently. - Native YAML writers and public upstream inspection APIs are used. Profile - reports include `ExtraParams` without additional redaction. -- The module uses published dependencies rather than sibling internal packages - or local replacement directives. Simple and composed consumer examples exist. - -No package restructuring or additional implementation framework is recommended. -Diff, catalogs, JSON output, consumer migrations, publication tooling, and -concurrent-dispatch guarantees are deliberate deferrals, not missing initial -architecture. The issues below affect behavior within the implemented structure. - - R-02 complete: inline document starts are recognized and native content is retained, including block scalars. @@ -72,12 +49,53 @@ architecture. The issues below affect behavior within the implemented structure. - R-09 complete: PromptKit selector collisions are checked against installed standard flags; profile-only `version` and custom `scope` selectors are usable. +- R-10 complete: help inventories and usage discover all registered children, + including non-runnable metadata-only group chains. + +### Validation evidence + +Each fix includes a focused regression check and passed `go test ./...` before +its commit. The final combined suite also passed with caching disabled using +`go test -count=1 ./...`. Go formatting, diff whitespace, and affected local +Markdown links were checked. No live services or credentials were used. + +| Findings | Regression evidence | +| --- | --- | +| R-01, R-08 | [Configuration selection tests](../../config_test.go): scope-specific requiredness, help, unknown/default absent scopes, and callback avoidance. | +| R-02 | [Native framing tests](../../show_test.go) and existing aggregate show tests in [configkit_test.go](../../configkit_test.go). | +| R-03 | [Cancellation tests](../../cancellation_test.go): all four configuration operations, both definition operations, pre-acquisition cancellation, cancellation during successful acquisition, and completed aggregate validation prefix/diagnostics. | +| R-04, R-06, R-07, R-10 | [Public invocation tests](../../execute_external_test.go): every help entry form, independent writer errors, outcome defaults/domain preservation, no-output diagnostics, and metadata-only group discovery. | +| R-05 | [Validation tests](../../configkit_test.go): header/record failures, short writes, completed prefixes, independent acquisition errors, and combined stream failures. | +| R-09 | [PromptKit tests](../../promptkit_test.go): profile-only version selector, custom scope selector, typed delivery, and rejection of actual installed flag collisions. | + +## Architecture assessment + +The main architecture is implemented: + +- The root `commandkit` package owns public named types, opaque handles, + per-instance registration, typed bindings, outcomes, standard handlers, scope + assembly, and ConfigKit/PromptKit integration. +- `internal/cobra` owns Cobra/pflag operations behind a neutral internal bridge. + Public signatures do not expose the backend. Each invocation rebuilds backend + state; configuration and engine acquisition remain lazy. +- Standard families own their namespaces. Applications retain custom commands, + loaders, domain behavior, executable entry points, streams, and process exit. +- Scope capabilities and prompt/profile callbacks can be supplied independently. + Native YAML writers and public upstream inspection APIs are used. Profile + reports include `ExtraParams` without additional redaction. +- The module uses published dependencies rather than sibling internal packages + or local replacement directives. Simple and composed consumer examples exist. + +No package restructuring or additional implementation framework is recommended. +Diff, catalogs, JSON output, consumer migrations, publication tooling, and +concurrent-dispatch guarantees are deliberate deferrals, not missing initial +architecture. The issues below affect behavior within the implemented structure. + ## Findings and priority P1 means a supported invocation is blocked or its report content is corrupted. P2 means a selected behavior or public contract is incomplete or unnecessarily -restricted. All listed items need resolution before calling the initial target -fully aligned. +restricted. The historical findings below have been resolved as recorded above. | ID | Priority | Finding | Review category | | --- | --- | --- | --- | diff --git a/execute_external_test.go b/execute_external_test.go index 878bfd9..325ea20 100644 --- a/execute_external_test.go +++ b/execute_external_test.go @@ -505,3 +505,50 @@ func TestNilDiagnosticWritersHaveNoOutputAndDoNotHideCauses(t *testing.T) { } } } + +func TestParentHelpDiscoversMetadataOnlyGroups(t *testing.T) { + leaf, err := commandkit.Group("notes", "Read notes metadata.") + if err != nil { + t.Fatal(err) + } + parent, err := commandkit.Group("topics", "Browse topic metadata.", leaf) + if err != nil { + t.Fatal(err) + } + app, err := commandkit.New(commandkit.Identity{Name: "example"}, parent) + if err != nil { + t.Fatal(err) + } + loads := 0 + if err := commandkit.RegisterConfig(app, commandkit.ConfigSuite[struct{}]{Scopes: []commandkit.ConfigScope[struct{}]{{Name: "app", Operations: []commandkit.ConfigOperation{commandkit.ConfigValidate}, Acquire: func(context.Context, commandkit.ConfigRequest, struct{}) (commandkit.Acquired[commandkit.ConfigView], error) { + loads++ + return commandkit.Acquired[commandkit.ConfigView]{}, nil + }}}}); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + args []string + content string + }{ + {nil, "Browse topic metadata."}, + {[]string{"topics"}, "Read notes metadata."}, + {[]string{"topics", "--help"}, "Read notes metadata."}, + {[]string{"help", "topics"}, "Read notes metadata."}, + {[]string{"topics", "notes"}, "Read notes metadata."}, + {[]string{"help", "topics", "notes"}, "Read notes metadata."}, + } { + var out bytes.Buffer + result := app.Execute(context.Background(), tc.args, commandkit.Streams{Out: &out}) + if result.Kind != commandkit.OutcomeSuccess || !strings.Contains(out.String(), tc.content) || loads != 0 { + t.Fatalf("%v: result=%#v output=%q loads=%d", tc.args, result, out.String(), loads) + } + if len(tc.args) == 1 && (!strings.Contains(out.String(), "Available Commands:") || !strings.Contains(out.String(), "[command]")) { + t.Fatalf("missing child inventory/usage: %q", out.String()) + } + } + var diagnostic bytes.Buffer + result := app.Execute(context.Background(), []string{"topics", "unknown"}, commandkit.Streams{Err: &diagnostic}) + if result.Kind != commandkit.OutcomeUsage || !strings.Contains(diagnostic.String(), "[command]") { + t.Fatalf("result=%#v diagnostic=%q", result, diagnostic.String()) + } +} diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 9439b8f..0e140e6 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -96,7 +96,7 @@ func UsageLine(root *Command, path []string) (string, *Failure) { return "", &Failure{Message: "usage path is not registered"} } line := selected.UseLine() - if selected.HasAvailableSubCommands() { + if selected.HasSubCommands() { line += " [command]" } return line, nil @@ -235,11 +235,11 @@ func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Com const usageTemplate = `{{with (or .Long .Short)}}{{. | trimTrailingWhitespaces}} {{end}}Usage: - {{.UseLine}}{{if .HasAvailableSubCommands}} - {{.CommandPath}} [command]{{end}}{{if .HasAvailableSubCommands}} + {{.UseLine}}{{if .HasSubCommands}} + {{.CommandPath}} [command]{{end}}{{if .HasSubCommands}} -Available Commands:{{range .Commands}}{{if .IsAvailableCommand}} - {{rpad .Name .NamePadding}} {{.Short}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} +Available Commands:{{range .Commands}} + {{rpad .Name .NamePadding}} {{.Short}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} Flags: {{.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}