diff --git a/app.go b/app.go index adb36e1..65f541c 100644 --- a/app.go +++ b/app.go @@ -8,15 +8,13 @@ import ( // App owns one command registry and its application identity. type App struct { - identity Identity - commands map[string]*commandNode - commandTree *internalcobra.Command - configRegistered bool + identity Identity + commands map[string]*commandNode + commandTree *internalcobra.Command } type commandNode struct { - command Command - reserved bool + command Command } // New creates an application with the supplied identity and initial commands. @@ -27,23 +25,9 @@ func New(identity Identity, commands ...Command) (*App, error) { return nil, fmt.Errorf("application identity name must not be empty") } - app := &App{ - identity: identity, - commands: map[string]*commandNode{ - "help": { - command: Command{record: &commandRecord{name: "help", summary: "Show help for commands."}}, - reserved: true, - }, - "version": { - command: Command{record: &commandRecord{name: "version", summary: "Show application version."}}, - reserved: true, - }, - }, - } - if err := app.installBuiltins(); err != nil { - return nil, err - } - if err := app.Register(commands...); err != nil { + app := &App{identity: identity, commands: make(map[string]*commandNode)} + all := append(app.builtinCommands(), commands...) + if err := app.installBatch(all...); err != nil { return nil, err } return app, nil diff --git a/builtins.go b/builtins.go index 402d3a1..65ba063 100644 --- a/builtins.go +++ b/builtins.go @@ -29,7 +29,7 @@ type versionDocument struct { Revision string `yaml:"revision"` } -func (a *App) installBuiltins() error { +func (a *App) builtinCommands() []Command { help := captureCommand(CommandSpec[helpInput]{ Name: "help", Summary: "Show help for commands.", @@ -61,9 +61,7 @@ func (a *App) installBuiltins() error { return renderVersionOutcome(identity, input.Format, invocation.Streams) }, }) - a.commands["help"] = &commandNode{command: help, reserved: true} - a.commands["version"] = &commandNode{command: version, reserved: true} - return nil + return []Command{help, version} } func parseVersionScope(value string) (string, error) { diff --git a/command.go b/command.go index 04bef7a..c1fc3c4 100644 --- a/command.go +++ b/command.go @@ -263,12 +263,16 @@ func captureComposition(record *commandRecord) { } func validateCommandDeclaration(command Command) error { + return validateCommandPlacement(command) +} + +func validateCommandPlacement(command Command) error { var issues []error - collectDeclarationIssues(command, nil, &issues) + collectDeclarationIssues(command, nil, true, &issues) return errors.Join(issues...) } -func collectDeclarationIssues(command Command, parents []string, issues *[]error) { +func collectDeclarationIssues(command Command, parents []string, root bool, issues *[]error) { if command.record == nil { *issues = append(*issues, errors.New("invalid zero command declaration")) return @@ -283,6 +287,9 @@ func collectDeclarationIssues(command Command, parents []string, issues *[]error *issues = append(*issues, issue) } } + if record.rootOnly && !root { + *issues = append(*issues, fmt.Errorf("root-only command %q is nested beneath another command", label)) + } if record.invalid || record.absent { return } @@ -290,7 +297,7 @@ func collectDeclarationIssues(command Command, parents []string, issues *[]error if child.record == nil { continue } - collectDeclarationIssues(child, path, issues) + collectDeclarationIssues(child, path, false, issues) } } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 671d72f..635f6a1 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Implement the optional integration modules and captured registration API -Status: implementation in progress. Stages 1 through 5 are complete; subsequent work follows the ordered roadmap. +Status: implementation in progress. Stages 1 through 6 are complete; subsequent work follows the ordered roadmap. Execute the stages in order. Each stage is sized as a separate Luna coding-agent assignment. This document authorizes no publication or changes to sibling repositories. @@ -321,10 +321,14 @@ for routing snapshot/session consolidation. `configRegistered`, integration collision prechecks, and root namespace allowlists. Integration factories install through the public declaration path. -**Completion evidence:** conflicting roots in either order, invalid final batch -member, nested root-only commands, no-op errors, explicit absence, empty real -groups, and declaration reuse across two apps. Inspect that registry writes have -one boundary, without making its private name a public contract. +**Completion evidence — complete:** `New` now submits real help/version +commands and caller declarations to the same private batch installer used by +`Register`. Retained errors are checked before absence; nested root-only placement, +root/batch collisions, and invalid final members fail before registry mutation. +No-op-only registration leaves the cached tree intact; real empty groups and +reused declarations install correctly. Direct built-in writes, placeholders, +`configRegistered`, and the registry reservation flag were removed. Registry +mutation now has one boundary. Root tests pass. ## Stage 7: Connect neutral routing, Cobra sessions, and execution diff --git a/execute.go b/execute.go index 7e630db..8e606e3 100644 --- a/execute.go +++ b/execute.go @@ -147,7 +147,7 @@ func (a *App) commandAt(path []string) (Command, bool) { return Command{}, true } command, ok := a.commands[path[0]] - if !ok || (command.reserved && command.command.record.payload == nil) { + if !ok { return Command{}, false } current := command.command diff --git a/parser.go b/parser.go index 4e55a17..7a246fd 100644 --- a/parser.go +++ b/parser.go @@ -26,13 +26,8 @@ func (a *App) bridgeTree() *internalcobra.Command { func (a *App) buildBridgeTree() *internalcobra.Command { root := &internalcobra.Command{Name: a.identity.Name} names := make([]string, 0, len(a.commands)) - for name, node := range a.commands { - if node.command.record != nil && node.command.record.absent { - continue - } - if !node.reserved || node.command.record.payload != nil { - names = append(names, name) - } + for name := range a.commands { + names = append(names, name) } sort.Strings(names) for _, name := range names { diff --git a/registry.go b/registry.go index 70e9c52..b8fbb77 100644 --- a/registry.go +++ b/registry.go @@ -5,13 +5,17 @@ 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 { + return a.installBatch(commands...) +} + +func (a *App) installBatch(commands ...Command) error { if a == nil { return fmt.Errorf("cannot register commands on a nil app") } pending := make(map[string]*commandNode, len(commands)) for _, command := range commands { - if err := validateCommandDeclaration(command); err != nil { + if err := validateCommandPlacement(command); err != nil { return err } if command.record.absent { diff --git a/registry_test.go b/registry_test.go index acf9608..0875de3 100644 --- a/registry_test.go +++ b/registry_test.go @@ -56,8 +56,8 @@ func TestAppsHaveIndependentRegistries(t *testing.T) { 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") + if first.commands["help"] == nil || first.commands["help"].command.record.payload == nil || first.commands["version"] == nil || first.commands["version"].command.record.payload == nil { + t.Fatal("New did not install real help and version declarations") } } @@ -138,6 +138,55 @@ func TestRootGroupRetainsPlacementMetadata(t *testing.T) { } } +func TestBatchInstallationRejectsNestedRootOnlyAndKeepsExistingState(t *testing.T) { + root := RootGroup("config", "Configuration.") + app, err := New(Identity{Name: "Example"}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(Group("outer", "", root)); err == nil { + t.Fatal("installation accepted nested root-only declaration") + } + if _, exists := app.commands["outer"]; exists { + t.Fatal("failed batch partially installed its parent") + } + if err := app.Register(root); err != nil { + t.Fatalf("root-level declaration rejected: %v", err) + } +} + +func TestNoOpOnlyRegistrationPreservesRoutingSnapshot(t *testing.T) { + app, err := New(Identity{Name: "Example"}) + if err != nil { + t.Fatal(err) + } + before := app.bridgeTree() + if err := app.Register(NoCommand()); err != nil { + t.Fatal(err) + } + if after := app.bridgeTree(); after != before { + t.Fatal("no-op-only registration invalidated derived metadata") + } +} + +func TestCapturedDeclarationCanBeInstalledIntoMultipleApps(t *testing.T) { + declaration := Group("tools", "", NewCommand(CommandSpec[struct{}]{Name: "run"})) + first, err := New(Identity{Name: "First"}, declaration) + if err != nil { + t.Fatal(err) + } + second, err := New(Identity{Name: "Second"}, declaration) + if err != nil { + t.Fatal(err) + } + if _, ok := first.commandAt([]string{"tools", "run"}); !ok { + t.Fatal("first app lost reused declaration") + } + if _, ok := second.commandAt([]string{"tools", "run"}); !ok { + t.Fatal("second app lost reused declaration") + } +} + func TestWithErrorCopiesDeclarationAndRetainsWrappedCauses(t *testing.T) { first := errors.New("first construction issue") second := errors.New("second construction issue")