fix: handle commands before checking route error

PR #1266 accidentally removed the logic that commands should be handled
before checking route errors. This caused global commands (/help, /show,
/switch) to be sent to LLM instead of being processed when routing fails.

This commit restores the original design from PR #959:
- Move handleCommand before routeErr check
- Add comment explaining the design intent
- Add regression test TestProcessMessage_CommandBeforeRouteError

Fixes regression introduced in #1266
This commit is contained in:
treepudding 2026-03-11 18:39:21 +08:00 committed by xuwenzhe
parent 8a398988d7
commit c4cd845374
2 changed files with 81 additions and 6 deletions

View file

@ -686,6 +686,15 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
} }
route, agent, routeErr := al.resolveMessageRoute(msg) route, agent, routeErr := al.resolveMessageRoute(msg)
// Commands are checked before requiring a successful route.
// Global commands (/help, /show, /switch) work even when routing fails;
// context-dependent commands check their own Runtime fields and report
// "unavailable" when the required capability is nil.
if response, handled := al.handleCommand(ctx, msg, agent, nil); handled {
return response, nil
}
if routeErr != nil { if routeErr != nil {
return "", routeErr return "", routeErr
} }
@ -722,12 +731,6 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
SendResponse: false, SendResponse: false,
} }
// context-dependent commands check their own Runtime fields and report
// "unavailable" when the required capability is nil.
if response, handled := al.handleCommand(ctx, msg, agent, &opts); handled {
return response, nil
}
return al.runAgentLoop(ctx, agent, opts) return al.runAgentLoop(ctx, agent, opts)
} }

View file

@ -1116,3 +1116,75 @@ func TestResolveMediaRefs_UsesMetaContentType(t *testing.T) {
t.Fatalf("expected jpeg prefix, got %q", result[0].Media[0][:30]) t.Fatalf("expected jpeg prefix, got %q", result[0].Media[0][:30])
} }
} }
// TestProcessMessage_CommandBeforeRouteError tests that global commands
// are handled even when routing fails. This is a regression test for
// https://github.com/sipeed/picoclaw/issues/1298
func TestProcessMessage_CommandBeforeRouteError(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "agent-test-*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
cfg := &config.Config{
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
Workspace: tmpDir,
Model: "test-model",
MaxTokens: 4096,
MaxToolIterations: 10,
},
},
Session: config.SessionConfig{
DMScope: "per-channel-peer",
},
}
msgBus := bus.NewMessageBus()
provider := &countingMockProvider{response: "LLM reply"}
al := NewAgentLoop(cfg, msgBus, provider)
helper := testHelper{al: al}
// Test /help command - this should work even without a valid route
// because /help is a global command that doesn't require agent context
helpResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{
Channel: "telegram",
SenderID: "user1",
ChatID: "chat1",
Content: "/help",
Peer: bus.Peer{
Kind: "direct",
ID: "user1",
},
})
// /help should return a help message, not an error about routing
if helpResp == "" {
t.Fatal("/help returned empty response")
}
if strings.Contains(helpResp, "no agent available") {
t.Fatalf("/help should work without valid route, got: %q", helpResp)
}
// LLM should not be called for /help command
if provider.calls != 0 {
t.Fatalf("LLM should not be called for /help, calls=%d", provider.calls)
}
// Test /show channel command - also a global command
showResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{
Channel: "telegram",
SenderID: "user1",
ChatID: "chat1",
Content: "/show channel",
Peer: bus.Peer{
Kind: "direct",
ID: "user1",
},
})
if showResp != "Current Channel: telegram" {
t.Fatalf("unexpected /show channel reply: %q", showResp)
}
}