From c16711ee7e38a5be6f4a61a730d8bd3d42917445 Mon Sep 17 00:00:00 2001
From: muava12 <moeaph@gmail.com>
Date: Wed, 25 Feb 2026 00:12:58 +0800
Subject: [PATCH] fix(providers): make generic 400 errors retriable and fix
 double error message
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Remove blanket 400 → FailoverFormat mapping from classifyByStatus.
Status 400 is too broad - model-invalid, format, and unknown errors all
return 400. Now 400 errors are classified by message patterns only:
- modelInvalidPatterns → FailoverModelInvalid (retriable + warning)
- formatPatterns → FailoverFormat (non-retriable)
- no match → unknown (retriable, allows fallback)

Also fix double error message: Run() was sending a duplicate error to
the user channel when runAgentLoop had already sent the ⚠️ error.
---
 pkg/agent/loop.go                      | 7 ++++++-
 pkg/providers/error_classifier.go      | 5 +++--
 pkg/providers/error_classifier_test.go | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go
index 095e1012d..b2a8559e1 100644
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -169,7 +169,12 @@ func (al *AgentLoop) Run(ctx context.Context) error {
 
 			response, err := al.processMessage(ctx, msg)
 			if err != nil {
-				response = fmt.Sprintf("Error processing message: %v", err)
+				// Error already sent to user channel by runAgentLoop, just log here
+				logger.ErrorCF("agent", "Message processing failed", map[string]any{
+					"error":   err.Error(),
+					"channel": msg.Channel,
+					"chat_id": msg.ChatID,
+				})
 			}
 
 			if response != "" {
diff --git a/pkg/providers/error_classifier.go b/pkg/providers/error_classifier.go
index 935bd7e54..33478f086 100644
--- a/pkg/providers/error_classifier.go
+++ b/pkg/providers/error_classifier.go
@@ -182,6 +182,9 @@ func ClassifyError(err error, provider, model string) *FailoverError {
 }
 
 // classifyByStatus maps HTTP status codes to FailoverReason.
+// NOTE: 400 is intentionally NOT mapped here. Status 400 is too broad
+// (model-invalid, format errors, unknown API errors all return 400).
+// Instead, 400 errors are classified by message patterns in classifyByMessage.
 func classifyByStatus(status int) FailoverReason {
 	switch {
 	case status == 401 || status == 403:
@@ -192,8 +195,6 @@ func classifyByStatus(status int) FailoverReason {
 		return FailoverTimeout
 	case status == 429:
 		return FailoverRateLimit
-	case status == 400:
-		return FailoverFormat
 	case transientStatusCodes[status]:
 		return FailoverTimeout
 	}
diff --git a/pkg/providers/error_classifier_test.go b/pkg/providers/error_classifier_test.go
index 4f472abb2..8637cf865 100644
--- a/pkg/providers/error_classifier_test.go
+++ b/pkg/providers/error_classifier_test.go
@@ -41,7 +41,7 @@ func TestClassifyError_StatusCodes(t *testing.T) {
 		{402, FailoverBilling},
 		{408, FailoverTimeout},
 		{429, FailoverRateLimit},
-		{400, FailoverFormat},
+		// 400 is intentionally NOT here - classified by message patterns, not status code
 		{500, FailoverTimeout},
 		{502, FailoverTimeout},
 		{503, FailoverTimeout},