From 6c339e27f7fb1d69e0ae789953c89dc2e59a9a10 Mon Sep 17 00:00:00 2001 From: lpf Date: Tue, 17 Feb 2026 11:01:55 +0800 Subject: [PATCH] fix bug --- pkg/agent/loop.go | 218 +++++++++++++++++++++++++++- pkg/agent/loop_config_path_test.go | 54 +++++++ pkg/agent/loop_fallback_test.go | 92 ++++++++++++ pkg/agent/loop_language_test.go | 52 +++++++ pkg/agent/loop_model_switch_test.go | 29 ++++ 5 files changed, 442 insertions(+), 3 deletions(-) create mode 100644 pkg/agent/loop_config_path_test.go create mode 100644 pkg/agent/loop_fallback_test.go create mode 100644 pkg/agent/loop_language_test.go create mode 100644 pkg/agent/loop_model_switch_test.go diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ccd4efd..a6830dd 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -19,6 +19,7 @@ import ( "sync" "sync/atomic" "time" + "unicode" "clawgo/pkg/bus" "clawgo/pkg/config" @@ -364,7 +365,7 @@ func (al *AgentLoop) runSessionWorker(ctx context.Context, sessionKey string, wo if errors.Is(err, context.Canceled) { return } - response = al.naturalizeUserFacingText(taskCtx, fmt.Sprintf("处理消息时发生错误：%v", err)) + response = al.formatProcessingErrorMessage(msg, err) } if response != "" && shouldPublishSyntheticResponse(msg) { @@ -386,6 +387,49 @@ func (al *AgentLoop) clearWorkerCancel(worker *sessionWorker) { worker.cancelMu.Unlock() } +func (al *AgentLoop) formatProcessingErrorMessage(msg bus.InboundMessage, err error) string { + if al.preferChineseUserFacingText(msg.SessionKey, msg.Content) { + return err.Error() + } + return err.Error() +} + +func (al *AgentLoop) preferChineseUserFacingText(sessionKey, currentContent string) bool { + zhCount, enCount := countLanguageSignals(currentContent) + + if al != nil && al.sessions != nil && strings.TrimSpace(sessionKey) != "" { + history := al.sessions.GetHistory(sessionKey) + seenUserTurns := 0 + for i := len(history) - 1; i >= 0 && seenUserTurns < 6; i-- { + if history[i].Role != "user" { + continue + } + seenUserTurns++ + z, e := countLanguageSignals(history[i].Content) + zhCount += z + enCount += e + } + } + + if zhCount == 0 && enCount == 0 { + return false + } + return zhCount >= enCount +} + +func countLanguageSignals(text string) (zhCount int, enCount int) { + for _, r := range text { + if unicode.In(r, unicode.Han) { + zhCount++ + continue + } + if r <= unicode.MaxASCII && unicode.IsLetter(r) { + enCount++ + } + } + return zhCount, enCount +} + func (al *AgentLoop) removeWorker(sessionKey string, worker *sessionWorker) { al.workersMu.Lock() defer al.workersMu.Unlock() @@ -1547,12 +1591,12 @@ func (al *AgentLoop) callLLMWithModelFallback( } lastErr = err - if !isQuotaOrRateLimitError(err) { + if !shouldRetryWithFallbackModel(err) { return nil, err } if idx < len(candidates)-1 { - logger.WarnCF("agent", "Model quota/rate-limit reached, trying fallback model", map[string]interface{}{ + logger.WarnCF("agent", "Model request failed, trying fallback model", map[string]interface{}{ "failed_model": model, "next_model": candidates[idx+1], logger.FieldError: err.Error(), @@ -1610,6 +1654,35 @@ func isQuotaOrRateLimitError(err error) bool { return false } +func isModelProviderSelectionError(err error) bool { + if err == nil { + return false + } + + msg := strings.ToLower(err.Error()) + keywords := []string{ + "unknown provider", + "unknown model", + "unsupported model", + "model not found", + "no such model", + "invalid model", + "does not exist", + "not available for model", + } + + for _, keyword := range keywords { + if strings.Contains(msg, keyword) { + return true + } + } + return false +} + +func shouldRetryWithFallbackModel(err error) bool { + return isQuotaOrRateLimitError(err) || isModelProviderSelectionError(err) +} + func buildProviderToolDefs(toolDefs []map[string]interface{}) ([]providers.ToolDefinition, error) { providerToolDefs := make([]providers.ToolDefinition, 0, len(toolDefs)) for i, td := range toolDefs { @@ -2242,11 +2315,27 @@ func (al *AgentLoop) handleSlashCommand(ctx context.Context, msg bus.InboundMess return true, "", err } path := al.normalizeConfigPathForAgent(fields[2]) + oldValue, oldExists := al.getMapValueByPathForAgent(cfgMap, path) value := al.parseConfigValueForAgent(strings.Join(fields[3:], " ")) if err := al.setMapValueByPathForAgent(cfgMap, path, value); err != nil { return true, "", err } + modelSwitched := path == "agents.defaults.model" && (!oldExists || strings.TrimSpace(fmt.Sprintf("%v", oldValue)) != strings.TrimSpace(fmt.Sprintf("%v", value))) + compactedOnSwitch := false + if modelSwitched { + if err := al.compactContextBeforeModelSwitch(ctx, msg.SessionKey); err != nil { + logger.WarnCF("agent", "Pre-switch context compaction failed", map[string]interface{}{ + "session_key": msg.SessionKey, + logger.FieldError: err.Error(), + }) + } else { + compactedOnSwitch = true + } + } + + al.applyRuntimeModelConfig(path, value) + data, err := json.MarshalIndent(cfgMap, "", " ") if err != nil { return true, "", err @@ -2266,8 +2355,14 @@ func (al *AgentLoop) handleSlashCommand(ctx context.Context, msg bus.InboundMess } return true, "", fmt.Errorf("hot reload failed, config rolled back: %w", err) } + if modelSwitched && compactedOnSwitch { + return true, fmt.Sprintf("Updated %s = %v\nContext compacted before model switch\nHot reload not applied: %v", path, value, err), nil + } return true, fmt.Sprintf("Updated %s = %v\nHot reload not applied: %v", path, value, err), nil } + if modelSwitched && compactedOnSwitch { + return true, fmt.Sprintf("Updated %s = %v\nContext compacted before model switch\nGateway hot reload signal sent", path, value), nil + } return true, fmt.Sprintf("Updated %s = %v\nGateway hot reload signal sent", path, value), nil default: return true, "Usage: /config get | /config set ", nil @@ -2326,6 +2421,16 @@ func (al *AgentLoop) getConfigPathForCommands() string { if fromEnv := strings.TrimSpace(os.Getenv("CLAWGO_CONFIG")); fromEnv != "" { return fromEnv } + args := os.Args + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--config" && i+1 < len(args) { + return args[i+1] + } + if strings.HasPrefix(arg, "--config=") { + return strings.TrimPrefix(arg, "--config=") + } + } return filepath.Join(config.GetConfigDir(), "config.json") } @@ -2407,6 +2512,113 @@ func (al *AgentLoop) triggerGatewayReloadFromAgent() (bool, error) { return configops.TriggerGatewayReload(al.getConfigPathForCommands(), errGatewayNotRunningSlash) } +func (al *AgentLoop) compactContextBeforeModelSwitch(ctx context.Context, sessionKey string) error { + if strings.TrimSpace(sessionKey) == "" { + return nil + } + + history := al.sessions.GetHistory(sessionKey) + if len(history) <= 1 { + return nil + } + + keepRecent := al.compactionCfg.KeepRecentMessages + if keepRecent <= 0 { + keepRecent = 20 + } + if keepRecent >= len(history) { + keepRecent = len(history) / 2 + } + if keepRecent <= 0 || keepRecent >= len(history) { + return nil + } + + maxSummaryChars := al.compactionCfg.MaxSummaryChars + if maxSummaryChars <= 0 { + maxSummaryChars = 6000 + } + maxTranscriptChars := al.compactionCfg.MaxTranscriptChars + if maxTranscriptChars <= 0 { + maxTranscriptChars = 24000 + } + + compactUntil := len(history) - keepRecent + if compactUntil <= 0 { + return nil + } + + currentSummary := al.sessions.GetSummary(sessionKey) + compactCtx, cancel := context.WithTimeout(ctx, 25*time.Second) + defer cancel() + newSummary, err := al.buildCompactedSummary(compactCtx, currentSummary, history[:compactUntil], maxTranscriptChars) + if err != nil { + return err + } + newSummary = strings.TrimSpace(newSummary) + if newSummary == "" { + return nil + } + if len(newSummary) > maxSummaryChars { + newSummary = truncateString(newSummary, maxSummaryChars) + } + + before, after, err := al.sessions.CompactHistory(sessionKey, newSummary, keepRecent) + if err != nil { + return err + } + logger.InfoCF("agent", "Context compacted before model switch", map[string]interface{}{ + "session_key": sessionKey, + "before_messages": before, + "after_messages": after, + "kept_recent": keepRecent, + "summary_chars": len(newSummary), + "model_after": al.model, + }) + return nil +} + +func (al *AgentLoop) applyRuntimeModelConfig(path string, value interface{}) { + switch path { + case "agents.defaults.model": + newModel := strings.TrimSpace(fmt.Sprintf("%v", value)) + if newModel == "" { + return + } + al.model = newModel + case "agents.defaults.model_fallbacks": + al.modelFallbacks = parseModelFallbacks(value) + } +} + +func parseModelFallbacks(value interface{}) []string { + switch v := value.(type) { + case []string: + out := make([]string, 0, len(v)) + for _, item := range v { + s := strings.TrimSpace(item) + if s != "" { + out = append(out, s) + } + } + return out + case []interface{}: + out := make([]string, 0, len(v)) + for _, item := range v { + s := strings.TrimSpace(fmt.Sprintf("%v", item)) + if s != "" { + out = append(out, s) + } + } + return out + default: + s := strings.TrimSpace(fmt.Sprintf("%v", value)) + if s == "" { + return nil + } + return []string{s} + } +} + // truncateString truncates a string to max length func truncateString(s string, maxLen int) string { if len(s) <= maxLen { diff --git a/pkg/agent/loop_config_path_test.go b/pkg/agent/loop_config_path_test.go new file mode 100644 index 0000000..b55872a --- /dev/null +++ b/pkg/agent/loop_config_path_test.go @@ -0,0 +1,54 @@ +package agent + +import ( + "os" + "path/filepath" + "testing" + + "clawgo/pkg/config" +) + +func TestGetConfigPathForCommands_FromArgs(t *testing.T) { + oldArgs := os.Args + oldEnv, hadEnv := os.LookupEnv("CLAWGO_CONFIG") + t.Cleanup(func() { + os.Args = oldArgs + if hadEnv { + _ = os.Setenv("CLAWGO_CONFIG", oldEnv) + } else { + _ = os.Unsetenv("CLAWGO_CONFIG") + } + }) + + _ = os.Unsetenv("CLAWGO_CONFIG") + os.Args = []string{"clawgo", "gateway", "run", "--config", "/tmp/custom-config.json"} + + al := &AgentLoop{} + got := al.getConfigPathForCommands() + if got != "/tmp/custom-config.json" { + t.Fatalf("expected config path from args, got %q", got) + } +} + +func TestGetConfigPathForCommands_Default(t *testing.T) { + oldArgs := os.Args + oldEnv, hadEnv := os.LookupEnv("CLAWGO_CONFIG") + t.Cleanup(func() { + os.Args = oldArgs + if hadEnv { + _ = os.Setenv("CLAWGO_CONFIG", oldEnv) + } else { + _ = os.Unsetenv("CLAWGO_CONFIG") + } + }) + + _ = os.Unsetenv("CLAWGO_CONFIG") + os.Args = []string{"clawgo", "gateway", "run"} + + al := &AgentLoop{} + got := al.getConfigPathForCommands() + want := filepath.Join(config.GetConfigDir(), "config.json") + if got != want { + t.Fatalf("expected default config path %q, got %q", want, got) + } +} diff --git a/pkg/agent/loop_fallback_test.go b/pkg/agent/loop_fallback_test.go new file mode 100644 index 0000000..f67fd82 --- /dev/null +++ b/pkg/agent/loop_fallback_test.go @@ -0,0 +1,92 @@ +package agent + +import ( + "context" + "fmt" + "testing" + + "clawgo/pkg/providers" +) + +type fallbackTestProvider struct { + byModel map[string]fallbackResult + called []string +} + +type fallbackResult struct { + resp *providers.LLMResponse + err error +} + +func (p *fallbackTestProvider) Chat(ctx context.Context, messages []providers.Message, tools []providers.ToolDefinition, model string, options map[string]interface{}) (*providers.LLMResponse, error) { + p.called = append(p.called, model) + if r, ok := p.byModel[model]; ok { + return r.resp, r.err + } + return nil, fmt.Errorf("unexpected model: %s", model) +} + +func (p *fallbackTestProvider) GetDefaultModel() string { + return "" +} + +func TestCallLLMWithModelFallback_RetriesOnUnknownProvider(t *testing.T) { + p := &fallbackTestProvider{ + byModel: map[string]fallbackResult{ + "gemini-3-flash": {err: fmt.Errorf(`API error (status 502): {"error":{"message":"unknown provider for model gemini-3-flash"}}`)}, + "gpt-4o-mini": {resp: &providers.LLMResponse{Content: "ok"}}, + }, + } + + al := &AgentLoop{ + provider: p, + model: "gemini-3-flash", + modelFallbacks: []string{"gpt-4o-mini"}, + } + + resp, err := al.callLLMWithModelFallback(context.Background(), nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp == nil || resp.Content != "ok" { + t.Fatalf("unexpected response: %+v", resp) + } + if len(p.called) != 2 { + t.Fatalf("expected 2 model attempts, got %d (%v)", len(p.called), p.called) + } + if p.called[0] != "gemini-3-flash" || p.called[1] != "gpt-4o-mini" { + t.Fatalf("unexpected model order: %v", p.called) + } + if al.model != "gpt-4o-mini" { + t.Fatalf("expected model switch to fallback, got %q", al.model) + } +} + +func TestCallLLMWithModelFallback_NoRetryOnNonRetryableError(t *testing.T) { + p := &fallbackTestProvider{ + byModel: map[string]fallbackResult{ + "gemini-3-flash": {err: fmt.Errorf("API error (status 500): internal server error")}, + }, + } + + al := &AgentLoop{ + provider: p, + model: "gemini-3-flash", + modelFallbacks: []string{"gpt-4o-mini"}, + } + + _, err := al.callLLMWithModelFallback(context.Background(), nil, nil, nil) + if err == nil { + t.Fatalf("expected error") + } + if len(p.called) != 1 { + t.Fatalf("expected single model attempt, got %d (%v)", len(p.called), p.called) + } +} + +func TestShouldRetryWithFallbackModel_UnknownProviderError(t *testing.T) { + err := fmt.Errorf(`API error (status 502): {"error":{"message":"unknown provider for model gemini-3-flash","type":"servererror"}}`) + if !shouldRetryWithFallbackModel(err) { + t.Fatalf("expected unknown provider error to trigger fallback retry") + } +} diff --git a/pkg/agent/loop_language_test.go b/pkg/agent/loop_language_test.go new file mode 100644 index 0000000..5bc95b7 --- /dev/null +++ b/pkg/agent/loop_language_test.go @@ -0,0 +1,52 @@ +package agent + +import ( + "errors" + "strings" + "testing" + + "clawgo/pkg/bus" + "clawgo/pkg/session" +) + +func TestFormatProcessingErrorMessage_ChineseCurrentMessage(t *testing.T) { + al := &AgentLoop{} + msg := bus.InboundMessage{ + SessionKey: "s-zh-current", + Content: "请帮我看一下这个错误", + } + + out := al.formatProcessingErrorMessage(msg, errors.New("boom")) + if !strings.HasPrefix(out, "处理消息时发生错误：") { + t.Fatalf("expected Chinese error prefix, got %q", out) + } +} + +func TestFormatProcessingErrorMessage_EnglishCurrentMessage(t *testing.T) { + al := &AgentLoop{} + msg := bus.InboundMessage{ + SessionKey: "s-en-current", + Content: "Please help debug this issue", + } + + out := al.formatProcessingErrorMessage(msg, errors.New("boom")) + if !strings.HasPrefix(out, "Error processing message:") { + t.Fatalf("expected English error prefix, got %q", out) + } +} + +func TestFormatProcessingErrorMessage_UsesSessionHistoryLanguage(t *testing.T) { + sm := session.NewSessionManager(t.TempDir()) + sm.AddMessage("s-history", "user", "请继续，按这个方向修复") + + al := &AgentLoop{sessions: sm} + msg := bus.InboundMessage{ + SessionKey: "s-history", + Content: "ok", + } + + out := al.formatProcessingErrorMessage(msg, errors.New("boom")) + if !strings.HasPrefix(out, "处理消息时发生错误：") { + t.Fatalf("expected Chinese error prefix from session history, got %q", out) + } +} diff --git a/pkg/agent/loop_model_switch_test.go b/pkg/agent/loop_model_switch_test.go new file mode 100644 index 0000000..b03d78d --- /dev/null +++ b/pkg/agent/loop_model_switch_test.go @@ -0,0 +1,29 @@ +package agent + +import "testing" + +func TestApplyRuntimeModelConfig_Model(t *testing.T) { + al := &AgentLoop{model: "old-model"} + al.applyRuntimeModelConfig("agents.defaults.model", "new-model") + if al.model != "new-model" { + t.Fatalf("expected runtime model updated, got %q", al.model) + } +} + +func TestApplyRuntimeModelConfig_ModelFallbacks(t *testing.T) { + al := &AgentLoop{modelFallbacks: []string{"old-fallback"}} + al.applyRuntimeModelConfig("agents.defaults.model_fallbacks", []interface{}{"gpt-4o-mini", "", "claude-3-5-sonnet"}) + if len(al.modelFallbacks) != 2 { + t.Fatalf("expected 2 fallbacks, got %d: %v", len(al.modelFallbacks), al.modelFallbacks) + } + if al.modelFallbacks[0] != "gpt-4o-mini" || al.modelFallbacks[1] != "claude-3-5-sonnet" { + t.Fatalf("unexpected fallbacks: %v", al.modelFallbacks) + } +} + +func TestParseModelFallbacks_StringValue(t *testing.T) { + fallbacks := parseModelFallbacks("gpt-4o-mini") + if len(fallbacks) != 1 || fallbacks[0] != "gpt-4o-mini" { + t.Fatalf("unexpected parse result: %v", fallbacks) + } +}