From f99be3257cc848d84b6dec468cf0a39873894fea Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 7 Oct 2026 13:16:52 +0000 Subject: [PATCH] Add instance command registry and composition --- app.go | 41 +++++ command.go | 265 +++++++++++++++++++++++++++++++++ docs/development.md | 6 +- docs/roadmap/implementation.md | 7 +- registry.go | 27 ++++ registry_test.go | 223 +++++++++++++++++++++++++++ 6 files changed, 564 insertions(+), 5 deletions(-) create mode 100644 app.go create mode 100644 command.go create mode 100644 registry.go create mode 100644 registry_test.go diff --git a/app.go b/app.go new file mode 100644 index 0000000..cd3937e --- /dev/null +++ b/app.go @@ -0,0 +1,41 @@ +package commandkit + +import "fmt" + +// App owns one command registry and its application identity. +type App struct { + identity Identity + commands map[string]*commandNode +} + +type commandNode struct { + command Command + reserved bool +} + +// New creates an application with the supplied identity and initial commands. +// The identity name must not be empty. The root help and version names are +// reserved for generated commands. +func New(identity Identity, commands ...Command) (*App, error) { + if identity.Name == "" { + return nil, fmt.Errorf("application identity name must not be empty") + } + + app := &App{ + identity: identity, + commands: map[string]*commandNode{ + "help": { + command: Command{name: "help", summary: "Show help for commands."}, + reserved: true, + }, + "version": { + command: Command{name: "version", summary: "Show application version."}, + reserved: true, + }, + }, + } + if err := app.Register(commands...); err != nil { + return nil, err + } + return app, nil +} diff --git a/command.go b/command.go new file mode 100644 index 0000000..ffe1e0c --- /dev/null +++ b/command.go @@ -0,0 +1,265 @@ +package commandkit + +import ( + "fmt" + "strings" +) + +// Command is an opaque command declaration. Use NewCommand or Group to create +// declarations and WithChildren to compose them. +type Command struct { + name string + summary string + description string + children []Command + payload commandPayload +} + +// Supplied carries a value and whether it was explicitly provided. +type Supplied[T any] struct { + // Value is the associated value. + Value T + // Set reports whether a caller explicitly supplied the value. + Set bool +} + +// FlagMeta describes a command flag. +type FlagMeta struct { + // Name is the flag name without leading dashes. + Name string + // Short is an optional one-byte spelling. + Short string + // Usage briefly describes the flag. + Usage string + // Required reports whether execution requires the flag to be supplied. + Required bool +} + +// ArgumentMeta describes a positional command argument. +type ArgumentMeta struct { + // Name is the argument name shown in command help. + Name string + // Usage briefly describes the argument. + Usage string +} + +// Flag represents a typed binding for a command flag. +type Flag[I any] struct { + binding flagBinding[I] +} + +type flagBinding[I any] struct { + meta FlagMeta + prepare func([]string) (func(*I), error) + valid bool +} + +// Argument represents a typed binding for a positional command argument. +type Argument[I any] struct { + binding argumentBinding[I] +} + +type argumentBinding[I any] struct { + meta ArgumentMeta + min int + max int + prepare func([]string) (func(*I), error) + valid bool +} + +// CommandSpec declares one command using typed input and handler callbacks. +type CommandSpec[I any] struct { + // Name is one command path component. + Name string + // Summary is a short description of the command. + Summary string + // Description provides additional command detail. + Description string + // NewInput creates an input value for execution. A nil factory uses a fresh + // zero value. + NewInput func() I + // Flags declares the command's local flags. + Flags []Flag[I] + // Arguments declares the command's positional inputs. + Arguments []Argument[I] + // CheckInput validates declared combinations of parsed inputs. + CheckInput func(I) error + // Run handles an invocation. It may be nil for a command used as a group. + Run func(Invocation, I) Outcome +} + +type commandPayload interface { + validate() error + hasRun() bool + newInput() any + run(Invocation, any) Outcome +} + +type typedCommand[I any] struct { + spec CommandSpec[I] +} + +func (c typedCommand[I]) validate() error { + return validateBindings(c.spec.Flags, c.spec.Arguments) +} + +func (c typedCommand[I]) hasRun() bool { + return c.spec.Run != nil +} + +func (c typedCommand[I]) newInput() any { + var input I + if c.spec.NewInput != nil { + input = c.spec.NewInput() + } + return &input +} + +func (c typedCommand[I]) run(invocation Invocation, input any) Outcome { + return c.spec.Run(invocation, *input.(*I)) +} + +// NewCommand validates and creates a typed command declaration. It retains +// callbacks without invoking them and copies the declarative flag and argument +// slices. +func NewCommand[I any](spec CommandSpec[I]) (Command, error) { + if err := validateCommandName(spec.Name); err != nil { + return Command{}, err + } + copySpec := spec + copySpec.Flags = append([]Flag[I](nil), spec.Flags...) + copySpec.Arguments = append([]Argument[I](nil), spec.Arguments...) + payload := typedCommand[I]{spec: copySpec} + if err := payload.validate(); err != nil { + return Command{}, fmt.Errorf("command %q: %w", spec.Name, err) + } + return Command{ + name: spec.Name, + summary: spec.Summary, + description: spec.Description, + payload: payload, + }, nil +} + +// Group creates a non-runnable command group with the supplied children. +func Group(name, summary string, children ...Command) (Command, error) { + if err := validateCommandName(name); err != nil { + return Command{}, err + } + group := Command{name: name, summary: summary, children: cloneCommands(children)} + if err := validateCommandDeclaration(group); err != nil { + return Command{}, err + } + return group, nil +} + +// WithChildren returns a copy of parent with the supplied relative child +// declarations. The parent's optional handler and input bindings are retained. +func WithChildren(parent Command, children ...Command) (Command, error) { + if err := validateCommandDeclaration(parent); err != nil { + return Command{}, fmt.Errorf("parent command: %w", err) + } + combined := make([]Command, 0, len(parent.children)+len(children)) + combined = append(combined, parent.children...) + combined = append(combined, children...) + result := parent + result.children = cloneCommands(combined) + if err := validateCommandDeclaration(result); err != nil { + return Command{}, err + } + return result, nil +} + +func cloneCommands(commands []Command) []Command { + if len(commands) == 0 { + return nil + } + cloned := make([]Command, len(commands)) + for i, command := range commands { + cloned[i] = cloneCommand(command) + } + return cloned +} + +func cloneCommand(command Command) Command { + command.children = cloneCommands(command.children) + return command +} + +func validateCommandDeclaration(command Command) error { + if err := validateCommandName(command.name); err != nil { + return err + } + if command.payload != nil { + if err := command.payload.validate(); err != nil { + return fmt.Errorf("command %q: %w", command.name, err) + } + } + + children := make(map[string]struct{}, len(command.children)) + for _, child := range command.children { + if err := validateCommandDeclaration(child); err != nil { + return err + } + if _, exists := children[child.name]; exists { + return fmt.Errorf("duplicate child command %q under %q", child.name, command.name) + } + children[child.name] = struct{}{} + } + return nil +} + +func validateCommandName(name string) error { + fields := strings.Fields(name) + if len(fields) != 1 || fields[0] != name || strings.HasPrefix(name, "-") || name == "--" { + return fmt.Errorf("command name %q must be one non-option token", name) + } + return nil +} + +func validateBindings[I any](flags []Flag[I], arguments []Argument[I]) error { + flagNames := make(map[string]struct{}, len(flags)) + shortNames := make(map[string]struct{}, len(flags)) + for _, flag := range flags { + binding := flag.binding + if binding.meta.Name == "" || strings.ContainsAny(binding.meta.Name, " \t\r\n") || strings.HasPrefix(binding.meta.Name, "-") { + return fmt.Errorf("flag name %q must be a nonempty token without leading dashes", binding.meta.Name) + } + if binding.meta.Name == "help" { + return fmt.Errorf("flag name %q is reserved for help", binding.meta.Name) + } + if _, exists := flagNames[binding.meta.Name]; exists { + return fmt.Errorf("duplicate flag name %q", binding.meta.Name) + } + flagNames[binding.meta.Name] = struct{}{} + if binding.meta.Short != "" { + if len(binding.meta.Short) != 1 || binding.meta.Short == "-" { + return fmt.Errorf("short spelling %q must be one byte", binding.meta.Short) + } + if binding.meta.Short == "h" { + return fmt.Errorf("short spelling %q is reserved for help", binding.meta.Short) + } + if _, exists := shortNames[binding.meta.Short]; exists { + return fmt.Errorf("duplicate short spelling %q", binding.meta.Short) + } + shortNames[binding.meta.Short] = struct{}{} + } + if !binding.valid || binding.prepare == nil { + return fmt.Errorf("flag %q is missing parser or setter wiring", binding.meta.Name) + } + } + + for _, argument := range arguments { + binding := argument.binding + if binding.meta.Name == "" { + return fmt.Errorf("argument name must not be empty") + } + if binding.min < 0 || binding.max < -1 || (binding.max >= 0 && binding.min > binding.max) { + return fmt.Errorf("argument %q has invalid bounds %d..%d", binding.meta.Name, binding.min, binding.max) + } + if !binding.valid || binding.prepare == nil { + return fmt.Errorf("argument %q is missing parser or setter wiring", binding.meta.Name) + } + } + return nil +} diff --git a/docs/development.md b/docs/development.md index 1a93fc0..cddf29f 100644 --- a/docs/development.md +++ b/docs/development.md @@ -1,9 +1,9 @@ # Developing CommandKit CommandKit currently contains adopted repository policies, accepted architectural -decisions, implementation plans, and initial Go identity and outcome primitives. -Command dispatch is not implemented yet. The [README](../README.md) provides -project orientation; the +decisions, implementation plans, identity and outcome primitives, typed command +declarations, and an instance-owned registry. Command execution is not implemented +yet. The [README](../README.md) provides project orientation; the [documentation policy](policy/documentation.md#canonical-ownership) assigns canonical documentation owners. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 6c758a7..83f2cde 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Initial CommandKit implementation plan -Status: implementation underway; Stage 1 complete and Stages 2-14 pending. +Status: implementation underway; Stages 1-2 complete and Stages 3-14 pending. Date: 2026-10-07. @@ -169,7 +169,7 @@ Verified with `go test ./...`, `go mod verify`, and ## Stage 2: implement the instance registry and command declarations -**Status:** pending. **Prerequisite:** Stage 1. +**Status:** complete. **Prerequisite:** Stage 1. Implement `App`, opaque `Command`, metadata, group composition, and registration in root files such as `app.go`, `command.go`, and `registry.go`. @@ -234,6 +234,9 @@ package or exported bridge methods. registries; duplicate nested paths; runnable-parent composition; registration errors leave the previous inventory intact. Execution remains Stage 5 work. +**Completion:** added typed command declarations, group composition, root help and +version reservations, and atomic per-App registration. Verified with `go test ./...`. + ## Stage 3: implement typed bindings and input assembly **Status:** pending. **Prerequisite:** Stage 2. diff --git a/registry.go b/registry.go new file mode 100644 index 0000000..1bed211 --- /dev/null +++ b/registry.go @@ -0,0 +1,27 @@ +package commandkit + +import "fmt" + +// Register adds a batch of root commands to the application. The batch is +// validated before the registry is changed. +func (a *App) Register(commands ...Command) error { + if a == nil { + return fmt.Errorf("cannot register commands on a nil app") + } + + updated := make(map[string]*commandNode, len(a.commands)+len(commands)) + for name, node := range a.commands { + updated[name] = node + } + for _, command := range commands { + if err := validateCommandDeclaration(command); err != nil { + return err + } + if _, exists := updated[command.name]; exists { + return fmt.Errorf("command path %q is already registered", command.name) + } + updated[command.name] = &commandNode{command: cloneCommand(command)} + } + a.commands = updated + return nil +} diff --git a/registry_test.go b/registry_test.go new file mode 100644 index 0000000..3be94e6 --- /dev/null +++ b/registry_test.go @@ -0,0 +1,223 @@ +package commandkit + +import ( + "testing" +) + +type registryInput struct { + value string +} + +func TestNewAndRegisterRetainCallbacksWithoutCallingThem(t *testing.T) { + var factoryCalls, prepareCalls, setterCalls, checkCalls, runCalls int + flag := Flag[registryInput]{binding: flagBinding[registryInput]{ + meta: FlagMeta{Name: "mode"}, + valid: true, + prepare: func([]string) (func(*registryInput), error) { + prepareCalls++ + return func(*registryInput) { setterCalls++ }, nil + }, + }} + command, err := NewCommand(CommandSpec[registryInput]{ + Name: "Inspect", + NewInput: func() registryInput { + factoryCalls++ + return registryInput{} + }, + Flags: []Flag[registryInput]{flag}, + CheckInput: func(registryInput) error { + checkCalls++ + return nil + }, + Run: func(Invocation, registryInput) Outcome { + runCalls++ + return Success() + }, + }) + if err != nil { + t.Fatal(err) + } + group, err := Group("Tools", "tools", command) + if err != nil { + t.Fatal(err) + } + app, err := New(Identity{Name: "Example"}, group) + if err != nil { + t.Fatal(err) + } + if err := app.Register(); err != nil { + t.Fatal(err) + } + + if factoryCalls+prepareCalls+setterCalls+checkCalls+runCalls != 0 { + t.Fatalf("construction invoked a callback: factory=%d prepare=%d setter=%d check=%d run=%d", factoryCalls, prepareCalls, setterCalls, checkCalls, runCalls) + } + if !app.commands["Tools"].command.children[0].payload.hasRun() { + t.Fatal("registered command lost its handler") + } +} + +func TestAppsHaveIndependentRegistries(t *testing.T) { + first, err := New(Identity{Name: "First"}) + if err != nil { + t.Fatal(err) + } + second, err := New(Identity{Name: "Second"}) + if err != nil { + t.Fatal(err) + } + command, err := NewCommand(CommandSpec[struct{}]{Name: "custom"}) + if err != nil { + t.Fatal(err) + } + if err := first.Register(command); err != nil { + t.Fatal(err) + } + if _, exists := second.commands["custom"]; exists { + t.Fatal("registration leaked to another app") + } + if !first.commands["help"].reserved || !first.commands["version"].reserved { + t.Fatal("New did not reserve generated root commands") + } +} + +func TestCommandAndGroupNames(t *testing.T) { + for _, name := range []string{"", "--", "-option", "two words", " leading", "trailing "} { + if _, err := NewCommand(CommandSpec[struct{}]{Name: name}); err == nil { + t.Errorf("NewCommand accepted invalid name %q", name) + } + } + if _, err := NewCommand(CommandSpec[struct{}]{Name: "CamelCase"}); err != nil { + t.Fatalf("NewCommand rejected a custom mixed-case name: %v", err) + } + + first, err := NewCommand(CommandSpec[struct{}]{Name: "repeat"}) + if err != nil { + t.Fatal(err) + } + second, err := NewCommand(CommandSpec[struct{}]{Name: "repeat"}) + if err != nil { + t.Fatal(err) + } + if _, err := Group("parent", "", first, second); err == nil { + t.Fatal("Group accepted duplicate sibling paths") + } + if _, err := Group("parent", "", Command{}); err == nil { + t.Fatal("Group accepted an invalid child declaration") + } +} + +func TestWithChildrenRetainsRunnableParent(t *testing.T) { + parent, err := NewCommand(CommandSpec[struct{}]{Name: "parent", Run: func(Invocation, struct{}) Outcome { + return Success() + }}) + if err != nil { + t.Fatal(err) + } + child, err := NewCommand(CommandSpec[struct{}]{Name: "child"}) + if err != nil { + t.Fatal(err) + } + composed, err := WithChildren(parent, child) + if err != nil { + t.Fatal(err) + } + if composed.payload == nil || !composed.payload.hasRun() { + t.Fatal("WithChildren did not retain the runnable parent") + } + if len(composed.children) != 1 || composed.children[0].name != "child" { + t.Fatalf("children = %#v, want relative child declaration", composed.children) + } + if len(parent.children) != 0 { + t.Fatal("WithChildren changed the original parent declaration") + } +} + +func TestRegisterBatchIsAtomicAndRejectsReservedPaths(t *testing.T) { + app, err := New(Identity{Name: "Example"}) + if err != nil { + t.Fatal(err) + } + existing, err := NewCommand(CommandSpec[struct{}]{Name: "existing"}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(existing); err != nil { + t.Fatal(err) + } + before := app.commands["existing"] + added, err := NewCommand(CommandSpec[struct{}]{Name: "added"}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(added, existing); err == nil { + t.Fatal("Register accepted a duplicate path") + } + if app.commands["existing"] != before { + t.Fatal("failed batch changed an existing registration") + } + if _, exists := app.commands["added"]; exists { + t.Fatal("failed batch attached a partial registration") + } + for _, name := range []string{"help", "version"} { + reserved, err := NewCommand(CommandSpec[struct{}]{Name: name}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(reserved); err == nil { + t.Errorf("Register replaced reserved command %q", name) + } + } +} + +func TestNewRejectsEmptyIdentityAndInvalidBindingMetadata(t *testing.T) { + if _, err := New(Identity{}); err == nil { + t.Fatal("New accepted an empty identity name") + } + + validFlag := func(name, short string) Flag[registryInput] { + return Flag[registryInput]{binding: flagBinding[registryInput]{ + meta: FlagMeta{Name: name, Short: short}, + valid: true, + prepare: func([]string) (func(*registryInput), error) { return func(*registryInput) {}, nil }, + }} + } + for _, flags := range [][]Flag[registryInput]{ + {validFlag("one", "x"), validFlag("one", "y")}, + {validFlag("one", "x"), validFlag("two", "x")}, + {validFlag("help", "")}, + {{binding: flagBinding[registryInput]{meta: FlagMeta{Name: "missing"}}}}, + } { + if _, err := NewCommand(CommandSpec[registryInput]{Name: "flags", Flags: flags}); err == nil { + t.Errorf("NewCommand accepted invalid flag metadata: %#v", flags) + } + } + + badArgument := Argument[registryInput]{binding: argumentBinding[registryInput]{ + meta: ArgumentMeta{Name: "item"}, + min: 2, + max: 1, + valid: true, + prepare: func([]string) (func(*registryInput), error) { return func(*registryInput) {}, nil }, + }} + if _, err := NewCommand(CommandSpec[registryInput]{Name: "args", Arguments: []Argument[registryInput]{badArgument}}); err == nil { + t.Fatal("NewCommand accepted invalid argument bounds") + } +} + +func TestNewCommandCopiesBindingMetadata(t *testing.T) { + flags := []Flag[registryInput]{{binding: flagBinding[registryInput]{ + meta: FlagMeta{Name: "mode"}, + valid: true, + prepare: func([]string) (func(*registryInput), error) { return func(*registryInput) {}, nil }, + }}} + command, err := NewCommand(CommandSpec[registryInput]{Name: "run", Flags: flags}) + if err != nil { + t.Fatal(err) + } + flags[0].binding.meta.Name = "changed" + payload := command.payload.(typedCommand[registryInput]) + if got := payload.spec.Flags[0].binding.meta.Name; got != "mode" { + t.Fatalf("stored flag name = %q, want %q", got, "mode") + } +}