refactor(commands): consolidate /switch into commands package, fix ! prefix
Move /switch model and /switch channel handling from inline loop.go logic into cmd_switch.go using the SubCommand + Deps pattern. This removes the OutcomePassthrough branch in handleCommand entirely. Also replace the hardcoded "/" prefix check with commands.HasCommandPrefix so that "!" prefixed commands are correctly routed to the Executor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
3f388a080f
commit
ac3b00e80e
6 changed files with 345 additions and 52 deletions
|
|
@ -66,8 +66,6 @@ type processOptions struct {
|
||||||
const (
|
const (
|
||||||
defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json."
|
defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json."
|
||||||
sessionKeyAgentPrefix = "agent:"
|
sessionKeyAgentPrefix = "agent:"
|
||||||
switchCommandToken = "/switch"
|
|
||||||
commandMentionSeparator = "@"
|
|
||||||
metadataKeyAccountID = "account_id"
|
metadataKeyAccountID = "account_id"
|
||||||
metadataKeyGuildID = "guild_id"
|
metadataKeyGuildID = "guild_id"
|
||||||
metadataKeyTeamID = "team_id"
|
metadataKeyTeamID = "team_id"
|
||||||
|
|
@ -121,6 +119,28 @@ func NewAgentLoop(
|
||||||
}
|
}
|
||||||
return al.channelManager.GetEnabledChannels()
|
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(
|
al.cmdExecutor = commands.NewExecutor(
|
||||||
commands.NewRegistry(commands.BuiltinDefinitions(deps)),
|
commands.NewRegistry(commands.BuiltinDefinitions(deps)),
|
||||||
|
|
@ -1484,10 +1504,7 @@ func (al *AgentLoop) handleCommand(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
msg bus.InboundMessage,
|
msg bus.InboundMessage,
|
||||||
) (string, bool) {
|
) (string, bool) {
|
||||||
// Generic command routing is delegated to the centralized commands executor.
|
if !commands.HasCommandPrefix(msg.Content) {
|
||||||
// Session lifecycle commands are intentionally out of scope for this PR layer.
|
|
||||||
content := strings.TrimSpace(msg.Content)
|
|
||||||
if !strings.HasPrefix(content, "/") {
|
|
||||||
return "", false
|
return "", false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1520,54 +1537,10 @@ func (al *AgentLoop) handleCommand(
|
||||||
return result.Reply, true
|
return result.Reply, true
|
||||||
}
|
}
|
||||||
return "", true
|
return "", true
|
||||||
case commands.OutcomePassthrough:
|
|
||||||
parts := strings.Fields(content)
|
|
||||||
if len(parts) == 0 {
|
|
||||||
return "", false
|
|
||||||
}
|
|
||||||
cmd := parts[0]
|
|
||||||
if at := strings.Index(cmd, commandMentionSeparator); at > 0 {
|
|
||||||
cmd = cmd[:at]
|
|
||||||
}
|
|
||||||
if cmd != switchCommandToken {
|
|
||||||
return "", false
|
|
||||||
}
|
|
||||||
|
|
||||||
args := parts[1:]
|
|
||||||
if len(args) < 3 || args[1] != "to" {
|
|
||||||
return "Usage: /switch [model|channel] to <name>", true
|
|
||||||
}
|
|
||||||
target := args[0]
|
|
||||||
value := args[2]
|
|
||||||
|
|
||||||
switch target {
|
|
||||||
case "model":
|
|
||||||
defaultAgent := al.registry.GetDefaultAgent()
|
|
||||||
if defaultAgent == nil {
|
|
||||||
return "No default agent configured", true
|
|
||||||
}
|
|
||||||
oldModel := defaultAgent.Model
|
|
||||||
defaultAgent.Model = value
|
|
||||||
if al.cfg != nil {
|
|
||||||
al.cfg.Agents.Defaults.ModelName = value
|
|
||||||
al.cfg.Agents.Defaults.Model = value
|
|
||||||
}
|
|
||||||
return fmt.Sprintf("Switched model from %s to %s", oldModel, value), true
|
|
||||||
case "channel":
|
|
||||||
if al.channelManager == nil {
|
|
||||||
return "Channel manager not initialized", true
|
|
||||||
}
|
|
||||||
if _, exists := al.channelManager.GetChannel(value); !exists && value != "cli" {
|
|
||||||
return fmt.Sprintf("Channel '%s' not found or not enabled", value), true
|
|
||||||
}
|
|
||||||
return fmt.Sprintf("Switched target channel to %s", value), true
|
|
||||||
default:
|
default:
|
||||||
return fmt.Sprintf("Unknown switch target: %s", target), true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return "", false
|
return "", false
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func mapCommandError(result commands.ExecuteResult) string {
|
func mapCommandError(result commands.ExecuteResult) string {
|
||||||
if result.Command == "" {
|
if result.Command == "" {
|
||||||
|
|
|
||||||
|
|
@ -8,5 +8,6 @@ func BuiltinDefinitions(deps *Deps) []Definition {
|
||||||
helpCommand(deps),
|
helpCommand(deps),
|
||||||
showCommand(deps),
|
showCommand(deps),
|
||||||
listCommand(deps),
|
listCommand(deps),
|
||||||
|
switchCommand(deps),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
59
pkg/commands/cmd_switch.go
Normal file
59
pkg/commands/cmd_switch.go
Normal file
|
|
@ -0,0 +1,59 @@
|
||||||
|
package commands
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
)
|
||||||
|
|
||||||
|
func switchCommand(deps *Deps) Definition {
|
||||||
|
return Definition{
|
||||||
|
Name: "switch",
|
||||||
|
Description: "Switch model or channel",
|
||||||
|
SubCommands: []SubCommand{
|
||||||
|
{
|
||||||
|
Name: "model",
|
||||||
|
Description: "Switch to a different model",
|
||||||
|
ArgsUsage: "to <name>",
|
||||||
|
Handler: func(_ context.Context, req Request) error {
|
||||||
|
if req.Reply == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if deps.SwitchModel == nil {
|
||||||
|
return req.Reply("Command unavailable in current context.")
|
||||||
|
}
|
||||||
|
// Parse: /switch model to <value>
|
||||||
|
value := nthToken(req.Text, 3) // tokens: [/switch, model, to, <value>]
|
||||||
|
if nthToken(req.Text, 2) != "to" || value == "" {
|
||||||
|
return req.Reply("Usage: /switch model to <name>")
|
||||||
|
}
|
||||||
|
oldModel, err := deps.SwitchModel(value)
|
||||||
|
if err != nil {
|
||||||
|
return req.Reply(err.Error())
|
||||||
|
}
|
||||||
|
return req.Reply(fmt.Sprintf("Switched model from %s to %s", oldModel, value))
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "channel",
|
||||||
|
Description: "Switch to a different channel",
|
||||||
|
ArgsUsage: "to <name>",
|
||||||
|
Handler: func(_ context.Context, req Request) error {
|
||||||
|
if req.Reply == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if deps.SwitchChannel == nil {
|
||||||
|
return req.Reply("Command unavailable in current context.")
|
||||||
|
}
|
||||||
|
value := nthToken(req.Text, 3)
|
||||||
|
if nthToken(req.Text, 2) != "to" || value == "" {
|
||||||
|
return req.Reply("Usage: /switch channel to <name>")
|
||||||
|
}
|
||||||
|
if err := deps.SwitchChannel(value); err != nil {
|
||||||
|
return req.Reply(err.Error())
|
||||||
|
}
|
||||||
|
return req.Reply(fmt.Sprintf("Switched target channel to %s", value))
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
238
pkg/commands/cmd_switch_test.go
Normal file
238
pkg/commands/cmd_switch_test.go
Normal file
|
|
@ -0,0 +1,238 @@
|
||||||
|
package commands
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSwitchModel_Success(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchModel: func(value string) (string, error) {
|
||||||
|
return "old-model", nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch model to gpt-4",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
want := "Switched model from old-model to gpt-4"
|
||||||
|
if reply != want {
|
||||||
|
t.Fatalf("reply=%q, want=%q", reply, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchModel_MissingToKeyword(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchModel: func(value string) (string, error) {
|
||||||
|
return "old", nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch model gpt-4",
|
||||||
|
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: /switch model to <name>" {
|
||||||
|
t.Fatalf("reply=%q, want usage message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchModel_MissingValue(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchModel: func(value string) (string, error) {
|
||||||
|
return "old", nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch model to",
|
||||||
|
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: /switch model to <name>" {
|
||||||
|
t.Fatalf("reply=%q, want usage message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchModel_Error(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchModel: func(value string) (string, error) {
|
||||||
|
return "", fmt.Errorf("model not found")
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch model to bad-model",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
if reply != "model not found" {
|
||||||
|
t.Fatalf("reply=%q, want error message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchModel_NilDep(t *testing.T) {
|
||||||
|
deps := &Deps{}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch model to gpt-4",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
if reply != "Command unavailable in current context." {
|
||||||
|
t.Fatalf("reply=%q, want unavailable message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchChannel_Success(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchChannel: func(value string) error {
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch channel to telegram",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
want := "Switched target channel to telegram"
|
||||||
|
if reply != want {
|
||||||
|
t.Fatalf("reply=%q, want=%q", reply, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchChannel_Error(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchChannel: func(value string) error {
|
||||||
|
return fmt.Errorf("channel '%s' not found", value)
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch channel to unknown",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
if reply != "channel 'unknown' not found" {
|
||||||
|
t.Fatalf("reply=%q, want error message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitchChannel_NilDep(t *testing.T) {
|
||||||
|
deps := &Deps{}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch channel to telegram",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
if reply != "Command unavailable in current context." {
|
||||||
|
t.Fatalf("reply=%q, want unavailable message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitch_BangPrefix(t *testing.T) {
|
||||||
|
deps := &Deps{
|
||||||
|
SwitchModel: func(value string) (string, error) {
|
||||||
|
return "old", nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "!switch model to gpt-4",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("! prefix: outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
if reply != "Switched model from old to gpt-4" {
|
||||||
|
t.Fatalf("! prefix: reply=%q, want success message", reply)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSwitch_NoSubCommand(t *testing.T) {
|
||||||
|
deps := &Deps{}
|
||||||
|
ex := NewExecutor(NewRegistry(BuiltinDefinitions(deps)))
|
||||||
|
|
||||||
|
var reply string
|
||||||
|
res := ex.Execute(context.Background(), Request{
|
||||||
|
Text: "/switch",
|
||||||
|
Reply: func(text string) error {
|
||||||
|
reply = text
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if res.Outcome != OutcomeHandled {
|
||||||
|
t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled)
|
||||||
|
}
|
||||||
|
// Should get usage message from executor's sub-command routing
|
||||||
|
if reply == "" {
|
||||||
|
t.Fatal("expected usage reply for bare /switch")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -11,4 +11,6 @@ type Deps struct {
|
||||||
GetModelInfo func() (name, provider string)
|
GetModelInfo func() (name, provider string)
|
||||||
ListAgentIDs func() []string
|
ListAgentIDs func() []string
|
||||||
GetEnabledChannels func() []string
|
GetEnabledChannels func() []string
|
||||||
|
SwitchModel func(value string) (oldModel string, err error)
|
||||||
|
SwitchChannel func(value string) error
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -65,6 +65,26 @@ func secondToken(input string) string {
|
||||||
return parts[1]
|
return parts[1]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// HasCommandPrefix returns true if the input starts with a recognized
|
||||||
|
// command prefix (e.g. "/" or "!").
|
||||||
|
func HasCommandPrefix(input string) bool {
|
||||||
|
token := firstToken(input)
|
||||||
|
if token == "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
_, ok := trimCommandPrefix(token)
|
||||||
|
return ok
|
||||||
|
}
|
||||||
|
|
||||||
|
// nthToken returns the 0-indexed token from whitespace-split input.
|
||||||
|
func nthToken(input string, n int) string {
|
||||||
|
parts := strings.Fields(strings.TrimSpace(input))
|
||||||
|
if n >= len(parts) {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
return parts[n]
|
||||||
|
}
|
||||||
|
|
||||||
func normalizeCommandName(name string) string {
|
func normalizeCommandName(name string) string {
|
||||||
return strings.ToLower(strings.TrimSpace(name))
|
return strings.ToLower(strings.TrimSpace(name))
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue