From 58f7b5bf2f27cf6c16760181b52d45bc734907d8 Mon Sep 17 00:00:00 2001 From: mingmxren Date: Wed, 4 Mar 2026 00:37:38 +0800 Subject: [PATCH] refactor(commands): replace Deps with per-request Runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate stateless Registry (cached on AgentLoop) from per-request Runtime (passed to handlers at execution time). This enables future session management features to inject per-request context without modifying the command registry. - Rename Deps → Runtime, move to runtime.go - Change Handler signature: func(ctx, req) error → func(ctx, req, rt *Runtime) error - NewExecutor now takes (registry, runtime) — executor is created per-request - BuiltinDefinitions() no longer takes parameters (stateless) - AgentLoop caches cmdRegistry, builds Runtime via buildRuntime() - Update all cmd_*.go handlers and tests Co-Authored-By: Claude Opus 4.6 --- pkg/agent/loop.go | 93 +++++++++++++------------ pkg/channels/telegram/telegram.go | 2 +- pkg/commands/builtin.go | 12 ++-- pkg/commands/builtin_test.go | 30 ++++---- pkg/commands/cmd_agents.go | 8 +-- pkg/commands/cmd_help.go | 6 +- pkg/commands/cmd_list.go | 16 ++--- pkg/commands/cmd_show.go | 12 ++-- pkg/commands/cmd_start.go | 2 +- pkg/commands/cmd_switch.go | 14 ++-- pkg/commands/cmd_switch_test.go | 37 +++++----- pkg/commands/dispatcher.go | 2 +- pkg/commands/executor.go | 9 +-- pkg/commands/executor_test.go | 36 +++++----- pkg/commands/{deps.go => runtime.go} | 9 ++- pkg/commands/show_list_handlers_test.go | 7 +- 16 files changed, 146 insertions(+), 149 deletions(-) rename pkg/commands/{deps.go => runtime.go} (54%) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 4f986870e..0b4be28e0 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -47,7 +47,7 @@ type AgentLoop struct { channelManager *channels.Manager mediaStore media.MediaStore transcriber voice.Transcriber - cmdExecutor *commands.Executor + cmdRegistry *commands.Registry } // processOptions configures how a message is processed @@ -103,48 +103,7 @@ func NewAgentLoop( fallback: fallbackChain, } - deps := &commands.Deps{ - Config: cfg, - GetModelInfo: func() (string, string) { - agent := al.registry.GetDefaultAgent() - if agent == nil { - return cfg.Agents.Defaults.GetModelName(), cfg.Agents.Defaults.Provider - } - return agent.Model, cfg.Agents.Defaults.Provider - }, - ListAgentIDs: al.registry.ListAgentIDs, - GetEnabledChannels: func() []string { - if al.channelManager == nil { - return nil - } - return al.channelManager.GetEnabledChannels() - }, - SwitchModel: func(value string) (string, error) { - defaultAgent := al.registry.GetDefaultAgent() - if defaultAgent == nil { - return "", fmt.Errorf("no default agent configured") - } - oldModel := defaultAgent.Model - defaultAgent.Model = value - if al.cfg != nil { - al.cfg.Agents.Defaults.ModelName = value - al.cfg.Agents.Defaults.Model = value - } - return oldModel, nil - }, - SwitchChannel: func(value string) error { - if al.channelManager == nil { - return fmt.Errorf("channel manager not initialized") - } - if _, exists := al.channelManager.GetChannel(value); !exists && value != "cli" { - return fmt.Errorf("channel '%s' not found or not enabled", value) - } - return nil - }, - } - al.cmdExecutor = commands.NewExecutor( - commands.NewRegistry(commands.BuiltinDefinitions(deps)), - ) + al.cmdRegistry = commands.NewRegistry(commands.BuiltinDefinitions()) return al } @@ -1508,11 +1467,13 @@ func (al *AgentLoop) handleCommand( return "", false } - executor := al.cmdExecutor - if executor == nil { + if al.cmdRegistry == nil { return "", false } + rt := al.buildRuntime() + executor := commands.NewExecutor(al.cmdRegistry, rt) + var commandReply string result := executor.Execute(ctx, commands.Request{ Channel: msg.Channel, @@ -1539,6 +1500,48 @@ func (al *AgentLoop) handleCommand( } } +func (al *AgentLoop) buildRuntime() *commands.Runtime { + return &commands.Runtime{ + Config: al.cfg, + GetModelInfo: func() (string, string) { + agent := al.registry.GetDefaultAgent() + if agent == nil { + return al.cfg.Agents.Defaults.GetModelName(), al.cfg.Agents.Defaults.Provider + } + return agent.Model, al.cfg.Agents.Defaults.Provider + }, + ListAgentIDs: al.registry.ListAgentIDs, + GetEnabledChannels: func() []string { + if al.channelManager == nil { + return nil + } + return al.channelManager.GetEnabledChannels() + }, + SwitchModel: func(value string) (string, error) { + defaultAgent := al.registry.GetDefaultAgent() + if defaultAgent == nil { + return "", fmt.Errorf("no default agent configured") + } + oldModel := defaultAgent.Model + defaultAgent.Model = value + if al.cfg != nil { + al.cfg.Agents.Defaults.ModelName = value + al.cfg.Agents.Defaults.Model = value + } + return oldModel, nil + }, + SwitchChannel: func(value string) error { + if al.channelManager == nil { + return fmt.Errorf("channel manager not initialized") + } + if _, exists := al.channelManager.GetChannel(value); !exists && value != "cli" { + return fmt.Errorf("channel '%s' not found or not enabled", value) + } + return nil + }, + } +} + func mapCommandError(result commands.ExecuteResult) string { if result.Command == "" { return fmt.Sprintf("Failed to execute command: %v", result.Err) diff --git a/pkg/channels/telegram/telegram.go b/pkg/channels/telegram/telegram.go index afd0a8510..a2035853c 100644 --- a/pkg/channels/telegram/telegram.go +++ b/pkg/channels/telegram/telegram.go @@ -130,7 +130,7 @@ func (c *TelegramChannel) Start(ctx context.Context) error { "username": c.bot.Username(), }) - c.startCommandRegistration(c.ctx, commands.BuiltinDefinitions(&commands.Deps{Config: c.config})) + c.startCommandRegistration(c.ctx, commands.BuiltinDefinitions()) go func() { if err = bh.Start(); err != nil { diff --git a/pkg/commands/builtin.go b/pkg/commands/builtin.go index f35874d23..48d3d3ce1 100644 --- a/pkg/commands/builtin.go +++ b/pkg/commands/builtin.go @@ -2,12 +2,14 @@ package commands // BuiltinDefinitions returns all built-in command definitions. // Each command group is defined in its own cmd_*.go file. -func BuiltinDefinitions(deps *Deps) []Definition { +// Definitions are stateless — runtime dependencies are provided +// via the Runtime parameter passed to handlers at execution time. +func BuiltinDefinitions() []Definition { return []Definition{ startCommand(), - helpCommand(deps), - showCommand(deps), - listCommand(deps), - switchCommand(deps), + helpCommand(), + showCommand(), + listCommand(), + switchCommand(), } } diff --git a/pkg/commands/builtin_test.go b/pkg/commands/builtin_test.go index dbff65e5e..66a84825e 100644 --- a/pkg/commands/builtin_test.go +++ b/pkg/commands/builtin_test.go @@ -18,8 +18,7 @@ func findDefinitionByName(t *testing.T, defs []Definition, name string) Definiti } func TestBuiltinHelpHandler_ReturnsFormattedMessage(t *testing.T) { - deps := &Deps{} - defs := BuiltinDefinitions(deps) + defs := BuiltinDefinitions() helpDef := findDefinitionByName(t, defs, "help") if helpDef.Handler == nil { t.Fatalf("/help handler should not be nil") @@ -32,7 +31,7 @@ func TestBuiltinHelpHandler_ReturnsFormattedMessage(t *testing.T) { reply = text return nil }, - }) + }, nil) if err != nil { t.Fatalf("/help handler error: %v", err) } @@ -46,11 +45,8 @@ func TestBuiltinHelpHandler_ReturnsFormattedMessage(t *testing.T) { } func TestBuiltinShowChannel_PreservesUserVisibleBehavior(t *testing.T) { - deps := &Deps{} - defs := BuiltinDefinitions(deps) - - // show now uses sub-commands, so we need the executor to route - ex := NewExecutor(NewRegistry(defs)) + defs := BuiltinDefinitions() + ex := NewExecutor(NewRegistry(defs), nil) cases := []string{"telegram", "whatsapp"} for _, channel := range cases { @@ -74,13 +70,13 @@ func TestBuiltinShowChannel_PreservesUserVisibleBehavior(t *testing.T) { } func TestBuiltinListChannels_UsesGetEnabledChannels(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ GetEnabledChannels: func() []string { return []string{"telegram", "slack"} }, } - defs := BuiltinDefinitions(deps) - ex := NewExecutor(NewRegistry(defs)) + defs := BuiltinDefinitions() + ex := NewExecutor(NewRegistry(defs), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -99,13 +95,13 @@ func TestBuiltinListChannels_UsesGetEnabledChannels(t *testing.T) { } func TestBuiltinShowAgents_RestoresOldBehavior(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ ListAgentIDs: func() []string { return []string{"default", "coder"} }, } - defs := BuiltinDefinitions(deps) - ex := NewExecutor(NewRegistry(defs)) + defs := BuiltinDefinitions() + ex := NewExecutor(NewRegistry(defs), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -124,13 +120,13 @@ func TestBuiltinShowAgents_RestoresOldBehavior(t *testing.T) { } func TestBuiltinListAgents_RestoresOldBehavior(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ ListAgentIDs: func() []string { return []string{"default", "coder"} }, } - defs := BuiltinDefinitions(deps) - ex := NewExecutor(NewRegistry(defs)) + defs := BuiltinDefinitions() + ex := NewExecutor(NewRegistry(defs), rt) var reply string res := ex.Execute(context.Background(), Request{ diff --git a/pkg/commands/cmd_agents.go b/pkg/commands/cmd_agents.go index 55ea2190d..c459516eb 100644 --- a/pkg/commands/cmd_agents.go +++ b/pkg/commands/cmd_agents.go @@ -7,12 +7,12 @@ import ( ) // agentsHandler returns a shared handler for both /show agents and /list agents. -func agentsHandler(deps *Deps) Handler { - return func(_ context.Context, req Request) error { - if deps.ListAgentIDs == nil { +func agentsHandler() Handler { + return func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.ListAgentIDs == nil { return req.Reply(unavailableMsg) } - ids := deps.ListAgentIDs() + ids := rt.ListAgentIDs() if len(ids) == 0 { return req.Reply("No agents registered") } diff --git a/pkg/commands/cmd_help.go b/pkg/commands/cmd_help.go index 0bbcfc0af..deca55bab 100644 --- a/pkg/commands/cmd_help.go +++ b/pkg/commands/cmd_help.go @@ -6,13 +6,13 @@ import ( "strings" ) -func helpCommand(deps *Deps) Definition { +func helpCommand() Definition { return Definition{ Name: "help", Description: "Show this help message", Usage: "/help", - Handler: func(_ context.Context, req Request) error { - defs := BuiltinDefinitions(deps) + Handler: func(_ context.Context, req Request, _ *Runtime) error { + defs := BuiltinDefinitions() return req.Reply(formatHelpMessage(defs)) }, } diff --git a/pkg/commands/cmd_list.go b/pkg/commands/cmd_list.go index cd85e57b8..bf47b6e9c 100644 --- a/pkg/commands/cmd_list.go +++ b/pkg/commands/cmd_list.go @@ -6,7 +6,7 @@ import ( "strings" ) -func listCommand(deps *Deps) Definition { +func listCommand() Definition { return Definition{ Name: "list", Description: "List available options", @@ -14,11 +14,11 @@ func listCommand(deps *Deps) Definition { { Name: "models", Description: "Configured models", - Handler: func(_ context.Context, req Request) error { - if deps.GetModelInfo == nil { + Handler: func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.GetModelInfo == nil { return req.Reply(unavailableMsg) } - name, provider := deps.GetModelInfo() + name, provider := rt.GetModelInfo() if provider == "" { provider = "configured default" } @@ -31,11 +31,11 @@ func listCommand(deps *Deps) Definition { { Name: "channels", Description: "Enabled channels", - Handler: func(_ context.Context, req Request) error { - if deps.GetEnabledChannels == nil { + Handler: func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.GetEnabledChannels == nil { return req.Reply(unavailableMsg) } - enabled := deps.GetEnabledChannels() + enabled := rt.GetEnabledChannels() if len(enabled) == 0 { return req.Reply("No channels enabled") } @@ -45,7 +45,7 @@ func listCommand(deps *Deps) Definition { { Name: "agents", Description: "Registered agents", - Handler: agentsHandler(deps), + Handler: agentsHandler(), }, }, } diff --git a/pkg/commands/cmd_show.go b/pkg/commands/cmd_show.go index 1ec3ca5a9..c655e6880 100644 --- a/pkg/commands/cmd_show.go +++ b/pkg/commands/cmd_show.go @@ -5,7 +5,7 @@ import ( "fmt" ) -func showCommand(deps *Deps) Definition { +func showCommand() Definition { return Definition{ Name: "show", Description: "Show current configuration", @@ -13,25 +13,25 @@ func showCommand(deps *Deps) Definition { { Name: "model", Description: "Current model and provider", - Handler: func(_ context.Context, req Request) error { - if deps.GetModelInfo == nil { + Handler: func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.GetModelInfo == nil { return req.Reply(unavailableMsg) } - name, provider := deps.GetModelInfo() + name, provider := rt.GetModelInfo() return req.Reply(fmt.Sprintf("Current Model: %s (Provider: %s)", name, provider)) }, }, { Name: "channel", Description: "Current channel", - Handler: func(_ context.Context, req Request) error { + Handler: func(_ context.Context, req Request, _ *Runtime) error { return req.Reply(fmt.Sprintf("Current Channel: %s", req.Channel)) }, }, { Name: "agents", Description: "Registered agents", - Handler: agentsHandler(deps), + Handler: agentsHandler(), }, }, } diff --git a/pkg/commands/cmd_start.go b/pkg/commands/cmd_start.go index e1efd6613..8b500aa10 100644 --- a/pkg/commands/cmd_start.go +++ b/pkg/commands/cmd_start.go @@ -7,7 +7,7 @@ func startCommand() Definition { Name: "start", Description: "Start the bot", Usage: "/start", - Handler: func(_ context.Context, req Request) error { + Handler: func(_ context.Context, req Request, _ *Runtime) error { return req.Reply("Hello! I am PicoClaw 🦞") }, } diff --git a/pkg/commands/cmd_switch.go b/pkg/commands/cmd_switch.go index 19a6b94d8..c0a3d7970 100644 --- a/pkg/commands/cmd_switch.go +++ b/pkg/commands/cmd_switch.go @@ -5,7 +5,7 @@ import ( "fmt" ) -func switchCommand(deps *Deps) Definition { +func switchCommand() Definition { return Definition{ Name: "switch", Description: "Switch model or channel", @@ -14,8 +14,8 @@ func switchCommand(deps *Deps) Definition { Name: "model", Description: "Switch to a different model", ArgsUsage: "to ", - Handler: func(_ context.Context, req Request) error { - if deps.SwitchModel == nil { + Handler: func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.SwitchModel == nil { return req.Reply(unavailableMsg) } // Parse: /switch model to @@ -23,7 +23,7 @@ func switchCommand(deps *Deps) Definition { if nthToken(req.Text, 2) != "to" || value == "" { return req.Reply("Usage: /switch model to ") } - oldModel, err := deps.SwitchModel(value) + oldModel, err := rt.SwitchModel(value) if err != nil { return req.Reply(err.Error()) } @@ -34,15 +34,15 @@ func switchCommand(deps *Deps) Definition { Name: "channel", Description: "Switch to a different channel", ArgsUsage: "to ", - Handler: func(_ context.Context, req Request) error { - if deps.SwitchChannel == nil { + Handler: func(_ context.Context, req Request, rt *Runtime) error { + if rt == nil || rt.SwitchChannel == nil { return req.Reply(unavailableMsg) } value := nthToken(req.Text, 3) if nthToken(req.Text, 2) != "to" || value == "" { return req.Reply("Usage: /switch channel to ") } - if err := deps.SwitchChannel(value); err != nil { + if err := rt.SwitchChannel(value); err != nil { return req.Reply(err.Error()) } return req.Reply(fmt.Sprintf("Switched target channel to %s", value)) diff --git a/pkg/commands/cmd_switch_test.go b/pkg/commands/cmd_switch_test.go index 7b3534dc3..088beaed4 100644 --- a/pkg/commands/cmd_switch_test.go +++ b/pkg/commands/cmd_switch_test.go @@ -7,12 +7,12 @@ import ( ) func TestSwitchModel_Success(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchModel: func(value string) (string, error) { return "old-model", nil }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -32,12 +32,12 @@ func TestSwitchModel_Success(t *testing.T) { } func TestSwitchModel_MissingToKeyword(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchModel: func(value string) (string, error) { return "old", nil }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -56,12 +56,12 @@ func TestSwitchModel_MissingToKeyword(t *testing.T) { } func TestSwitchModel_MissingValue(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchModel: func(value string) (string, error) { return "old", nil }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -80,12 +80,12 @@ func TestSwitchModel_MissingValue(t *testing.T) { } func TestSwitchModel_Error(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchModel: func(value string) (string, error) { return "", fmt.Errorf("model not found") }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -104,8 +104,7 @@ func TestSwitchModel_Error(t *testing.T) { } func TestSwitchModel_NilDep(t *testing.T) { - deps := &Deps{} - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), &Runtime{}) var reply string res := ex.Execute(context.Background(), Request{ @@ -124,12 +123,12 @@ func TestSwitchModel_NilDep(t *testing.T) { } func TestSwitchChannel_Success(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchChannel: func(value string) error { return nil }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -149,12 +148,12 @@ func TestSwitchChannel_Success(t *testing.T) { } func TestSwitchChannel_Error(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchChannel: func(value string) error { return fmt.Errorf("channel '%s' not found", value) }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -173,8 +172,7 @@ func TestSwitchChannel_Error(t *testing.T) { } func TestSwitchChannel_NilDep(t *testing.T) { - deps := &Deps{} - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), &Runtime{}) var reply string res := ex.Execute(context.Background(), Request{ @@ -193,12 +191,12 @@ func TestSwitchChannel_NilDep(t *testing.T) { } func TestSwitch_BangPrefix(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ SwitchModel: func(value string) (string, error) { return "old", nil }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{ @@ -217,8 +215,7 @@ func TestSwitch_BangPrefix(t *testing.T) { } func TestSwitch_NoSubCommand(t *testing.T) { - deps := &Deps{} - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), &Runtime{}) var reply string res := ex.Execute(context.Background(), Request{ diff --git a/pkg/commands/dispatcher.go b/pkg/commands/dispatcher.go index 2b1bbc806..62ee600f2 100644 --- a/pkg/commands/dispatcher.go +++ b/pkg/commands/dispatcher.go @@ -5,7 +5,7 @@ import ( "strings" ) -type Handler func(ctx context.Context, req Request) error +type Handler func(ctx context.Context, req Request, rt *Runtime) error type Request struct { Channel string diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 72b9c9ccb..f2910fcab 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -22,10 +22,11 @@ type ExecuteResult struct { type Executor struct { reg *Registry + rt *Runtime } -func NewExecutor(reg *Registry) *Executor { - return &Executor{reg: reg} +func NewExecutor(reg *Registry, rt *Runtime) *Executor { + return &Executor{reg: reg, rt: rt} } // Execute implements a two-state command decision: @@ -60,7 +61,7 @@ func (e *Executor) executeDefinition(ctx context.Context, req Request, def Defin if def.Handler == nil { return ExecuteResult{Outcome: OutcomePassthrough, Command: def.Name} } - err := def.Handler(ctx, req) + err := def.Handler(ctx, req, e.rt) return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name, Err: err} } @@ -77,7 +78,7 @@ func (e *Executor) executeDefinition(ctx context.Context, req Request, def Defin if sc.Handler == nil { return ExecuteResult{Outcome: OutcomePassthrough, Command: def.Name} } - err := sc.Handler(ctx, req) + err := sc.Handler(ctx, req, e.rt) return ExecuteResult{Outcome: OutcomeHandled, Command: def.Name, Err: err} } } diff --git a/pkg/commands/executor_test.go b/pkg/commands/executor_test.go index 49f203bae..09350f1b6 100644 --- a/pkg/commands/executor_test.go +++ b/pkg/commands/executor_test.go @@ -9,7 +9,7 @@ import ( func TestExecutor_RegisteredWithoutHandler_ReturnsPassthrough(t *testing.T) { defs := []Definition{{Name: "show"}} - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "whatsapp", Text: "/show"}) if res.Outcome != OutcomePassthrough { @@ -19,7 +19,7 @@ func TestExecutor_RegisteredWithoutHandler_ReturnsPassthrough(t *testing.T) { func TestExecutor_UnknownSlashCommand_ReturnsPassthrough(t *testing.T) { defs := []Definition{{Name: "show"}} - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/unknown"}) if res.Outcome != OutcomePassthrough { @@ -32,13 +32,13 @@ func TestExecutor_SupportedCommandWithHandler_ReturnsHandled(t *testing.T) { defs := []Definition{ { Name: "help", - Handler: func(context.Context, Request) error { + Handler: func(context.Context, Request, *Runtime) error { called = true return nil }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/help@my_bot"}) if res.Outcome != OutcomeHandled { @@ -56,7 +56,7 @@ func TestExecutor_AliasWithoutHandler_ReturnsPassthrough(t *testing.T) { Aliases: []string{"display"}, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "whatsapp", Text: "/display"}) if res.Outcome != OutcomePassthrough { @@ -73,13 +73,13 @@ func TestExecutor_AliasWithHandler_ReturnsHandled(t *testing.T) { { Name: "clear", Aliases: []string{"reset"}, - Handler: func(context.Context, Request) error { + Handler: func(context.Context, Request, *Runtime) error { called = true return nil }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/reset"}) if res.Outcome != OutcomeHandled { @@ -97,7 +97,7 @@ func TestExecutor_SupportedCommandWithNilHandler_ReturnsPassthrough(t *testing.T defs := []Definition{ {Name: "placeholder"}, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/placeholder list"}) if res.Outcome != OutcomePassthrough { @@ -114,7 +114,7 @@ func TestExecutor_NilHandlerDoesNotMaskLaterHandler(t *testing.T) { defs := []Definition{ {Name: "placeholder"}, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/placeholder"}) if res.Outcome != OutcomePassthrough { @@ -130,12 +130,12 @@ func TestExecutor_HandlerErrorIsPropagated(t *testing.T) { defs := []Definition{ { Name: "help", - Handler: func(context.Context, Request) error { + Handler: func(context.Context, Request, *Runtime) error { return wantErr }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "/help"}) if res.Outcome != OutcomeHandled { @@ -151,13 +151,13 @@ func TestExecutor_SupportsBangPrefixAndCaseInsensitiveCommand(t *testing.T) { defs := []Definition{ { Name: "help", - Handler: func(context.Context, Request) error { + Handler: func(context.Context, Request, *Runtime) error { called = true return nil }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Channel: "telegram", Text: "!HELP"}) if res.Outcome != OutcomeHandled { @@ -174,7 +174,7 @@ func TestExecutor_SubCommand_RoutesToCorrectHandler(t *testing.T) { { Name: "show", SubCommands: []SubCommand{ - {Name: "model", Handler: func(_ context.Context, _ Request) error { + {Name: "model", Handler: func(_ context.Context, _ Request, _ *Runtime) error { modelCalled = true return nil }}, @@ -182,7 +182,7 @@ func TestExecutor_SubCommand_RoutesToCorrectHandler(t *testing.T) { }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Text: "/show model"}) if res.Outcome != OutcomeHandled { @@ -203,7 +203,7 @@ func TestExecutor_SubCommand_NoArg_RepliesUsage(t *testing.T) { }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) var reply string res := ex.Execute(context.Background(), Request{ @@ -227,7 +227,7 @@ func TestExecutor_SubCommand_UnknownArg_RepliesError(t *testing.T) { }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) var reply string res := ex.Execute(context.Background(), Request{ @@ -251,7 +251,7 @@ func TestExecutor_SubCommand_NilHandler_ReturnsPassthrough(t *testing.T) { }, }, } - ex := NewExecutor(NewRegistry(defs)) + ex := NewExecutor(NewRegistry(defs), nil) res := ex.Execute(context.Background(), Request{Text: "/show model"}) if res.Outcome != OutcomePassthrough { diff --git a/pkg/commands/deps.go b/pkg/commands/runtime.go similarity index 54% rename from pkg/commands/deps.go rename to pkg/commands/runtime.go index af1a63fe3..a1da81e06 100644 --- a/pkg/commands/deps.go +++ b/pkg/commands/runtime.go @@ -2,11 +2,10 @@ package commands import "github.com/sipeed/picoclaw/pkg/config" -// Deps provides runtime data to command handlers without importing -// agent or channel packages. Function fields are called at handler -// invocation time, not at construction time, so late-bound values -// (e.g. channelManager set after NewAgentLoop) are visible. -type Deps struct { +// Runtime provides runtime dependencies to command handlers. It is constructed +// per-request by the agent loop so that per-request state (like session scope) +// can coexist with long-lived callbacks (like GetModelInfo). +type Runtime struct { Config *config.Config GetModelInfo func() (name, provider string) ListAgentIDs func() []string diff --git a/pkg/commands/show_list_handlers_test.go b/pkg/commands/show_list_handlers_test.go index 42ce4fa10..047708f0f 100644 --- a/pkg/commands/show_list_handlers_test.go +++ b/pkg/commands/show_list_handlers_test.go @@ -7,8 +7,7 @@ import ( ) func TestShowListHandlers_ChannelPolicy(t *testing.T) { - deps := &Deps{} - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), nil) var telegramReply string handled := ex.Execute(context.Background(), Request{ @@ -58,12 +57,12 @@ func TestShowListHandlers_ChannelPolicy(t *testing.T) { } func TestShowListHandlers_ListHandledOnAllChannels(t *testing.T) { - deps := &Deps{ + rt := &Runtime{ GetEnabledChannels: func() []string { return []string{"telegram"} }, } - ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps))) + ex := NewExecutor(NewRegistry(BuiltinDefinitions()), rt) var reply string res := ex.Execute(context.Background(), Request{