diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 59e5deaee..0845b5950 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -2,14 +2,13 @@ package commands import ( "context" + "fmt" ) type Outcome int const ( - // OutcomePassthrough means this input should continue through normal agent flow. OutcomePassthrough Outcome = iota - // OutcomeHandled means a command handler executed (with or without handler error). OutcomeHandled ) @@ -41,36 +40,47 @@ func (e *Executor) Execute(ctx context.Context, req Request) ExecuteResult { return ExecuteResult{Outcome: OutcomePassthrough, Command: cmdName} } - passthroughCommand := "" + def, found := e.reg.Lookup(cmdName) + if !found { + return ExecuteResult{Outcome: OutcomePassthrough, Command: cmdName} + } - for _, def := range e.reg.Definitions() { - if !matchesCommand(def, cmdName) { - continue - } - if passthroughCommand == "" { - passthroughCommand = def.Name - } + return e.executeDefinition(ctx, req, def) +} + +func (e *Executor) executeDefinition(ctx context.Context, req Request, def Definition) ExecuteResult { + // Simple command — no sub-commands + if len(def.SubCommands) == 0 { if def.Handler == nil { - continue + return ExecuteResult{Outcome: OutcomePassthrough, Command: def.Name} } err := def.Handler(ctx, req) return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name, Err: err} } - if passthroughCommand != "" { - return ExecuteResult{Outcome: OutcomePassthrough, Command: passthroughCommand} + + // Sub-command routing + subName := secondToken(req.Text) + if subName == "" { + if req.Reply != nil { + _ = req.Reply("Usage: " + def.EffectiveUsage()) + } + return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name} } - return ExecuteResult{Outcome: OutcomePassthrough, Command: cmdName} -} - -func matchesCommand(def Definition, cmdName string) bool { - if normalizeCommandName(def.Name) == cmdName { - return true - } - for _, alias := range def.Aliases { - if normalizeCommandName(alias) == cmdName { - return true + normalized := normalizeCommandName(subName) + for _, sc := range def.SubCommands { + if normalizeCommandName(sc.Name) == normalized { + if sc.Handler == nil { + return ExecuteResult{Outcome: OutcomePassthrough, Command: def.Name} + } + err := sc.Handler(ctx, req) + return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name, Err: err} } } - return false + + // Unknown sub-command + if req.Reply != nil { + _ = req.Reply(fmt.Sprintf("Unknown parameter: %s. Usage: %s", subName, def.EffectiveUsage())) + } + return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name} } diff --git a/pkg/commands/executor_test.go b/pkg/commands/executor_test.go index ca35f35d7..49f203bae 100644 --- a/pkg/commands/executor_test.go +++ b/pkg/commands/executor_test.go @@ -3,6 +3,7 @@ package commands import ( "context" "errors" + "strings" "testing" ) @@ -108,29 +109,20 @@ func TestExecutor_SupportedCommandWithNilHandler_ReturnsPassthrough(t *testing.T } func TestExecutor_NilHandlerDoesNotMaskLaterHandler(t *testing.T) { - called := false + // With Lookup-based dispatch, the first registered definition for a name wins. + // A definition with nil Handler and no SubCommands returns Passthrough. defs := []Definition{ {Name: "placeholder"}, - { - Name: "placeholder", - Handler: func(context.Context, Request) error { - called = true - return nil - }, - }, } ex := NewExecutor(NewRegistry(defs)) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/placeholder"}) - if res.Outcome != OutcomeHandled { - t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + if res.Outcome != OutcomePassthrough { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomePassthrough) } if res.Command != "placeholder" { t.Fatalf("command=%q, want=%q", res.Command, "placeholder") } - if !called { - t.Fatalf("expected later handler to be called") - } } func TestExecutor_HandlerErrorIsPropagated(t *testing.T) { @@ -175,3 +167,94 @@ func TestExecutor_SupportsBangPrefixAndCaseInsensitiveCommand(t *testing.T) { t.Fatalf("expected handler to be called") } } + +func TestExecutor_SubCommand_RoutesToCorrectHandler(t *testing.T) { + modelCalled := false + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model", Handler: func(_ context.Context, _ Request) error { + modelCalled = true + return nil + }}, + {Name: "channel"}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs)) + + res := ex.Execute(context.Background(), Request{Text: "/show model"}) + if res.Outcome != OutcomeHandled { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + } + if !modelCalled { + t.Fatal("model sub-command handler was not called") + } +} + +func TestExecutor_SubCommand_NoArg_RepliesUsage(t *testing.T) { + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model"}, + {Name: "channel"}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs)) + + var reply string + res := ex.Execute(context.Background(), Request{ + Text: "/show", + Reply: func(text string) error { reply = text; return nil }, + }) + if res.Outcome != OutcomeHandled { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + } + if reply != "Usage: /show [model|channel]" { + t.Fatalf("reply=%q, want usage message", reply) + } +} + +func TestExecutor_SubCommand_UnknownArg_RepliesError(t *testing.T) { + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model"}, + }, + }, + } + ex := NewExecutor(NewRegistry(defs)) + + var reply string + res := ex.Execute(context.Background(), Request{ + Text: "/show foobar", + 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, "foobar") { + t.Fatalf("reply=%q, should mention unknown sub-command", reply) + } +} + +func TestExecutor_SubCommand_NilHandler_ReturnsPassthrough(t *testing.T) { + defs := []Definition{ + { + Name: "show", + SubCommands: []SubCommand{ + {Name: "model"}, // nil Handler + }, + }, + } + ex := NewExecutor(NewRegistry(defs)) + + res := ex.Execute(context.Background(), Request{Text: "/show model"}) + if res.Outcome != OutcomePassthrough { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomePassthrough) + } +}