From 3706f6145c089c444ec31deeabdb87f925b02f36 Mon Sep 17 00:00:00 2001 From: James Cowan <112015792+jameslcowan@users.noreply.github.com> Date: Thu, 12 Mar 2026 22:29:26 -0300 Subject: [PATCH] fix(commands): resolve subcommand names typed as top-level commands Users naturally type /model instead of /show model, but subcommand names were not recognized as top-level input. When a top-level lookup fails, the executor now searches all parent definitions for a matching subcommand. Unique matches route directly; ambiguous names reply with the valid alternatives. Closes #1298 --- pkg/commands/executor.go | 50 +++++++++++++++++++++++- pkg/commands/executor_test.go | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 78a50e6c2..4870d88bd 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -3,6 +3,7 @@ package commands import ( "context" "fmt" + "strings" ) type Outcome int @@ -44,7 +45,7 @@ func (e *Executor) Execute(ctx context.Context, req Request) ExecuteResult { def, found := e.reg.Lookup(cmdName) if !found { - return ExecuteResult{Outcome: OutcomePassthrough, Command: cmdName} + return e.trySubCommandShortcut(ctx, req, cmdName) } return e.executeDefinition(ctx, req, def) @@ -87,3 +88,50 @@ func (e *Executor) executeDefinition(ctx context.Context, req Request, def Defin err := req.Reply(fmt.Sprintf("Unknown option: %s. Usage: %s", subName, def.EffectiveUsage())) return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name, Err: err} } + +// trySubCommandShortcut handles the case where a user types a subcommand +// name directly (e.g. "/model" instead of "/show model"). If exactly one +// parent command owns a subcommand with that name, we route to it. When +// multiple parents match, we reply with the valid alternatives so the user +// knows what to type. +func (e *Executor) trySubCommandShortcut(ctx context.Context, req Request, name string) ExecuteResult { + normalized := normalizeCommandName(name) + + type match struct { + parent Definition + sub SubCommand + } + + var matches []match + for _, def := range e.reg.Definitions() { + for _, sc := range def.SubCommands { + if normalizeCommandName(sc.Name) == normalized { + matches = append(matches, match{parent: def, sub: sc}) + } + } + } + + switch len(matches) { + case 0: + return ExecuteResult{Outcome: OutcomePassthrough, Command: name} + case 1: + m := matches[0] + if m.sub.Handler == nil { + return ExecuteResult{Outcome: OutcomePassthrough, Command: m.parent.Name} + } + err := m.sub.Handler(ctx, req, e.rt) + return ExecuteResult{Outcome: OutcomeHandled, Command: m.parent.Name, Err: err} + default: + // Multiple parents own a subcommand with this name. + if req.Reply == nil { + req.Reply = func(string) error { return nil } + } + options := make([]string, 0, len(matches)) + for _, m := range matches { + options = append(options, fmt.Sprintf("/%s %s", m.parent.Name, m.sub.Name)) + } + msg := fmt.Sprintf("Did you mean one of these?\n%s", strings.Join(options, "\n")) + err := req.Reply(msg) + return ExecuteResult{Outcome: OutcomeHandled, Command: name, Err: err} + } +} diff --git a/pkg/commands/executor_test.go b/pkg/commands/executor_test.go index 09350f1b6..4c5d4378e 100644 --- a/pkg/commands/executor_test.go +++ b/pkg/commands/executor_test.go @@ -242,6 +242,77 @@ func TestExecutor_SubCommand_UnknownArg_RepliesError(t *testing.T) { } } +func TestExecutor_SubCommandShortcut_UniqueMatch_RoutesToHandler(t *testing.T) { + called := false + defs := []Definition{ + { + Name: "check", + SubCommands: []SubCommand{ + {Name: "health", Handler: func(_ context.Context, _ Request, _ *Runtime) error { + called = true + return nil + }}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs), nil) + + res := ex.Execute(context.Background(), Request{Text: "/health"}) + if res.Outcome != OutcomeHandled { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + } + if !called { + t.Fatal("expected sub-command handler to be called via shortcut") + } +} + +func TestExecutor_SubCommandShortcut_AmbiguousMatch_RepliesAlternatives(t *testing.T) { + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model", Handler: func(_ context.Context, _ Request, _ *Runtime) error { return nil }}, + }, + }, + { + Name: "switch", + SubCommands: []SubCommand{ + {Name: "model", Handler: func(_ context.Context, _ Request, _ *Runtime) error { return nil }}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs), nil) + + var reply string + res := ex.Execute(context.Background(), Request{ + Text: "/model", + Reply: func(text string) error { reply = text; return nil }, + }) + if res.Outcome != OutcomeHandled { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + } + if !strings.Contains(reply, "/show model") || !strings.Contains(reply, "/switch model") { + t.Fatalf("reply=%q, expected both /show model and /switch model", reply) + } +} + +func TestExecutor_SubCommandShortcut_NoMatch_ReturnsPassthrough(t *testing.T) { + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model"}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs), nil) + + res := ex.Execute(context.Background(), Request{Text: "/nonexistent"}) + if res.Outcome != OutcomePassthrough { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomePassthrough) + } +} + func TestExecutor_SubCommand_NilHandler_ReturnsPassthrough(t *testing.T) { defs := []Definition{ {