diff --git a/pkg/commands/builtin.go b/pkg/commands/builtin.go index 254612f00..5e6937a68 100644 --- a/pkg/commands/builtin.go +++ b/pkg/commands/builtin.go @@ -9,25 +9,30 @@ import ( "github.com/sipeed/picoclaw/pkg/config" ) -type runtimeContextKey struct{} - -// WithRuntime attaches command runtime capabilities to ctx for command handlers. -func WithRuntime(ctx context.Context, runtime Runtime) context.Context { - if ctx == nil { - ctx = context.Background() - } - return context.WithValue(ctx, runtimeContextKey{}, runtime) -} - -func runtimeFromContext(ctx context.Context) Runtime { - if ctx == nil { - return nil - } - runtime, _ := ctx.Value(runtimeContextKey{}).(Runtime) - return runtime -} - func BuiltinDefinitions(cfg *config.Config) []Definition { + return builtinDefinitions(cfg, nil) +} + +// BuiltinDefinitionsWithRuntime returns builtin command definitions with runtime-backed +// session command handlers enabled only when runtime is usable. +func BuiltinDefinitionsWithRuntime(cfg *config.Config, runtime Runtime) []Definition { + return builtinDefinitions(cfg, runtime) +} + +func builtinDefinitions(cfg *config.Config, runtime Runtime) []Definition { + sessionRuntime := runtimeIfUsable(runtime) + + var newHandler Handler + var sessionHandler Handler + if sessionRuntime != nil { + newHandler = func(_ context.Context, req Request) error { + return handleNewCommand(req, sessionRuntime, cfg) + } + sessionHandler = func(_ context.Context, req Request) error { + return handleSessionCommand(req, sessionRuntime) + } + } + return []Definition{ { Name: "start", @@ -55,18 +60,14 @@ func BuiltinDefinitions(cfg *config.Config) []Definition { Description: "Start a new chat session", Usage: "/new", Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, - Handler: func(ctx context.Context, req Request) error { - return handleNewCommand(ctx, req, cfg) - }, + Handler: newHandler, }, { Name: "session", Description: "Manage chat sessions", Usage: "/session [list|resume ]", Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, - Handler: func(ctx context.Context, req Request) error { - return handleSessionCommand(ctx, req) - }, + Handler: sessionHandler, }, { Name: "show", @@ -172,12 +173,7 @@ func replyText(text string) Handler { } } -func handleNewCommand(ctx context.Context, req Request, fallbackCfg *config.Config) error { - runtime := runtimeFromContext(ctx) - if runtime == nil || runtime.SessionOps() == nil || strings.TrimSpace(runtime.ScopeKey()) == "" { - return reply(req, "Command unavailable in current context.") - } - +func handleNewCommand(req Request, runtime Runtime, fallbackCfg *config.Config) error { scopeKey := runtime.ScopeKey() newSessionKey, err := runtime.SessionOps().StartNew(scopeKey) if err != nil { @@ -208,12 +204,7 @@ func handleNewCommand(ctx context.Context, req Request, fallbackCfg *config.Conf return reply(req, fmt.Sprintf("Started new session: %s (pruned %d old session(s))", newSessionKey, len(pruned))) } -func handleSessionCommand(ctx context.Context, req Request) error { - runtime := runtimeFromContext(ctx) - if runtime == nil || runtime.SessionOps() == nil || strings.TrimSpace(runtime.ScopeKey()) == "" { - return reply(req, "Command unavailable in current context.") - } - +func handleSessionCommand(req Request, runtime Runtime) error { args := strings.Fields(commandArgs(req.Text)) if len(args) < 1 { return reply(req, "Usage: /session [list|resume ]") @@ -278,6 +269,19 @@ func reply(req Request, text string) error { return req.Reply(text) } +func runtimeIfUsable(runtime Runtime) Runtime { + if runtime == nil { + return nil + } + if runtime.SessionOps() == nil { + return nil + } + if strings.TrimSpace(runtime.ScopeKey()) == "" { + return nil + } + return runtime +} + func enabledChannels(cfg *config.Config) []string { enabled := make([]string, 0, 8) if cfg.Channels.Telegram.Enabled { diff --git a/pkg/commands/builtin_test.go b/pkg/commands/builtin_test.go index 37b3b3867..0398cc627 100644 --- a/pkg/commands/builtin_test.go +++ b/pkg/commands/builtin_test.go @@ -1,6 +1,11 @@ package commands -import "testing" +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/session" +) func TestBuiltinDefinitions_ContainsTelegramDefaults(t *testing.T) { defs := BuiltinDefinitions(nil) @@ -32,7 +37,7 @@ func TestBuiltinDefinitions_WhatsAppOnlyHasBasicCommands(t *testing.T) { } } -func TestBuiltinDefinitions_SessionCommandsHaveHandlers(t *testing.T) { +func TestBuiltinDefinitions_DefaultSessionCommandsArePassthrough(t *testing.T) { defs := BuiltinDefinitions(nil) defByName := map[string]Definition{} @@ -44,8 +49,8 @@ func TestBuiltinDefinitions_SessionCommandsHaveHandlers(t *testing.T) { if !ok { t.Fatalf("missing /new definition") } - if newDef.Handler == nil { - t.Fatalf("/new should provide a runtime-backed handler") + if newDef.Handler != nil { + t.Fatalf("/new should be passthrough without runtime wiring") } if !contains(newDef.Aliases, "reset") { t.Fatalf("/new aliases=%v, want alias \"reset\"", newDef.Aliases) @@ -55,7 +60,44 @@ func TestBuiltinDefinitions_SessionCommandsHaveHandlers(t *testing.T) { if !ok { t.Fatalf("missing /session definition") } - if sessionDef.Handler == nil { - t.Fatalf("/session should provide a runtime-backed handler") + if sessionDef.Handler != nil { + t.Fatalf("/session should be passthrough without runtime wiring") + } +} + +type builtinTestSessionOps struct{} + +func (f *builtinTestSessionOps) ResolveActive(scopeKey string) (string, error) { return "", nil } +func (f *builtinTestSessionOps) StartNew(scopeKey string) (string, error) { return "", nil } +func (f *builtinTestSessionOps) List(scopeKey string) ([]session.SessionMeta, error) { + return nil, nil +} +func (f *builtinTestSessionOps) Resume(scopeKey string, index int) (string, error) { return "", nil } +func (f *builtinTestSessionOps) Prune(scopeKey string, limit int) ([]string, error) { return nil, nil } + +type builtinTestRuntime struct { + scope string + ops SessionOps +} + +func (f *builtinTestRuntime) Channel() string { return "whatsapp" } +func (f *builtinTestRuntime) ScopeKey() string { return f.scope } +func (f *builtinTestRuntime) SessionOps() SessionOps { return f.ops } +func (f *builtinTestRuntime) Config() *config.Config { return nil } + +func TestBuiltinDefinitionsWithRuntime_EnablesSessionHandlers(t *testing.T) { + runtime := &builtinTestRuntime{scope: "scope", ops: &builtinTestSessionOps{}} + defs := BuiltinDefinitionsWithRuntime(nil, runtime) + + defByName := map[string]Definition{} + for _, def := range defs { + defByName[def.Name] = def + } + + if defByName["new"].Handler == nil { + t.Fatalf("/new should provide runtime-backed handler when runtime is available") + } + if defByName["session"].Handler == nil { + t.Fatalf("/session should provide runtime-backed handler when runtime is available") } } diff --git a/pkg/commands/session_handlers_test.go b/pkg/commands/session_handlers_test.go index 031f7ee83..a51c8659d 100644 --- a/pkg/commands/session_handlers_test.go +++ b/pkg/commands/session_handlers_test.go @@ -2,6 +2,7 @@ package commands import ( "context" + "errors" "testing" "time" @@ -79,8 +80,6 @@ func (f *sessionHandlerFakeRuntime) Config() *config.Config { } func TestSessionHandlers_New_UsesRuntimeSessionOps(t *testing.T) { - t.Helper() - ops := &sessionHandlerFakeSessionOps{ startNewValue: "scope#2", pruneValue: []string{"scope#1"}, @@ -94,11 +93,9 @@ func TestSessionHandlers_New_UsesRuntimeSessionOps(t *testing.T) { }, } - ctx := WithRuntime(context.Background(), runtime) - var reply string - ex := NewExecutor(NewRegistry(BuiltinDefinitions(nil))) - res := ex.Execute(ctx, Request{ + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + res := ex.Execute(context.Background(), Request{ Channel: "whatsapp", Text: "/new", Reply: func(text string) error { @@ -125,8 +122,6 @@ func TestSessionHandlers_New_UsesRuntimeSessionOps(t *testing.T) { } func TestSessionHandlers_SessionResume_UsesRuntimeSessionOps(t *testing.T) { - t.Helper() - ops := &sessionHandlerFakeSessionOps{resumeValue: "scope#3"} runtime := &sessionHandlerFakeRuntime{ channel: "whatsapp", @@ -135,11 +130,9 @@ func TestSessionHandlers_SessionResume_UsesRuntimeSessionOps(t *testing.T) { cfg: &config.Config{}, } - ctx := WithRuntime(context.Background(), runtime) - var reply string - ex := NewExecutor(NewRegistry(BuiltinDefinitions(nil))) - res := ex.Execute(ctx, Request{ + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + res := ex.Execute(context.Background(), Request{ Channel: "whatsapp", Text: "/session resume 3", Reply: func(text string) error { @@ -163,8 +156,6 @@ func TestSessionHandlers_SessionResume_UsesRuntimeSessionOps(t *testing.T) { } func TestSessionHandlers_SessionList_UsesRuntimeSessionOps(t *testing.T) { - t.Helper() - ops := &sessionHandlerFakeSessionOps{ listValue: []session.SessionMeta{ { @@ -183,11 +174,9 @@ func TestSessionHandlers_SessionList_UsesRuntimeSessionOps(t *testing.T) { cfg: &config.Config{}, } - ctx := WithRuntime(context.Background(), runtime) - var reply string - ex := NewExecutor(NewRegistry(BuiltinDefinitions(nil))) - res := ex.Execute(ctx, Request{ + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + res := ex.Execute(context.Background(), Request{ Channel: "whatsapp", Text: "/session list", Reply: func(text string) error { @@ -206,3 +195,141 @@ func TestSessionHandlers_SessionList_UsesRuntimeSessionOps(t *testing.T) { t.Fatalf("reply=%q", reply) } } + +func TestSessionHandlers_MissingRuntime_Passthrough(t *testing.T) { + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, nil))) + + for _, input := range []string{"/new", "/session list"} { + res := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: input, + }) + if res.Outcome != OutcomePassthrough { + t.Fatalf("text=%q outcome=%v, want=%v", input, res.Outcome, OutcomePassthrough) + } + } +} + +func TestSessionHandlers_NilSessionOps_Passthrough(t *testing.T) { + runtime := &sessionHandlerFakeRuntime{ + channel: "whatsapp", + scope: "scope", + ops: nil, + cfg: &config.Config{}, + } + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + + for _, input := range []string{"/new", "/session list"} { + res := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: input, + }) + if res.Outcome != OutcomePassthrough { + t.Fatalf("text=%q outcome=%v, want=%v", input, res.Outcome, OutcomePassthrough) + } + } +} + +func TestSessionHandlers_EmptyScope_Passthrough(t *testing.T) { + runtime := &sessionHandlerFakeRuntime{ + channel: "whatsapp", + scope: " ", + ops: &sessionHandlerFakeSessionOps{}, + cfg: &config.Config{}, + } + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + + for _, input := range []string{"/new", "/session list"} { + res := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: input, + }) + if res.Outcome != OutcomePassthrough { + t.Fatalf("text=%q outcome=%v, want=%v", input, res.Outcome, OutcomePassthrough) + } + } +} + +func TestSessionHandlers_ErrorAndValidationReplies(t *testing.T) { + tests := []struct { + name string + text string + ops *sessionHandlerFakeSessionOps + wantReply string + }{ + { + name: "start new error", + text: "/new", + ops: &sessionHandlerFakeSessionOps{startNewErr: errors.New("boom")}, + wantReply: "Failed to start new session: boom", + }, + { + name: "prune error", + text: "/new", + ops: &sessionHandlerFakeSessionOps{ + startNewValue: "scope#2", + pruneErr: errors.New("prune failed"), + }, + wantReply: "Started new session (scope#2), but pruning old sessions failed: prune failed", + }, + { + name: "list error", + text: "/session list", + ops: &sessionHandlerFakeSessionOps{listErr: errors.New("list failed")}, + wantReply: "Failed to list sessions: list failed", + }, + { + name: "resume error", + text: "/session resume 2", + ops: &sessionHandlerFakeSessionOps{resumeErr: errors.New("resume failed")}, + wantReply: "Failed to resume session 2: resume failed", + }, + { + name: "resume missing index", + text: "/session resume", + ops: &sessionHandlerFakeSessionOps{}, + wantReply: "Usage: /session resume ", + }, + { + name: "resume non numeric index", + text: "/session resume abc", + ops: &sessionHandlerFakeSessionOps{}, + wantReply: "Usage: /session resume ", + }, + { + name: "resume zero index", + text: "/session resume 0", + ops: &sessionHandlerFakeSessionOps{}, + wantReply: "Usage: /session resume ", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + runtime := &sessionHandlerFakeRuntime{ + channel: "whatsapp", + scope: "scope", + ops: tc.ops, + cfg: &config.Config{}, + } + + var reply string + ex := NewExecutor(NewRegistry(BuiltinDefinitionsWithRuntime(nil, runtime))) + res := ex.Execute(context.Background(), Request{ + Channel: "whatsapp", + Text: tc.text, + Reply: func(text string) error { + reply = text + return nil + }, + }) + + if res.Outcome != OutcomeHandled { + t.Fatalf("outcome=%v, want=%v", res.Outcome, OutcomeHandled) + } + if reply != tc.wantReply { + t.Fatalf("reply=%q, want=%q", reply, tc.wantReply) + } + }) + } +}