don't react with 💔 on telegram (#1492)

internal/agent/loop_pipeline_callbacks.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log/slog"
 	"slices"
+	"strconv"
 	"strings"
 	"time"
 
@@ -369,6 +370,22 @@ func (l *Loop) makeCallLLM(req *RunRequest, emitRun func(AgentEvent)) func(ctx c
 		provider := state.Provider
 		model := state.Model
 
+		// Issue 3: surface transient provider retries to the user ("Provider busy,
+		// retrying...") instead of a silent failure ending in a 💔 reaction. The
+		// providers' internal RetryDo / codex loops fire this hook before each retry
+		// attempt; the channel layer turns run.retrying into a placeholder update.
+		ctx = providers.WithRetryHook(ctx, func(attempt, maxAttempts int, _ error) {
+			emitRun(AgentEvent{
+				Type:    protocol.AgentEventRunRetrying,
+				AgentID: l.id,
+				RunID:   req.RunID,
+				Payload: map[string]string{
+					"attempt":     strconv.Itoa(attempt),
+					"maxAttempts": strconv.Itoa(maxAttempts),
+				},
+			})
+		})
+
 		// Enrich ChatRequest options to match v2 (providers need these for caching, routing, audit).
 		if chatReq.Options == nil {
 			chatReq.Options = make(map[string]any)

internal/agent/loop_pipeline_callbacks_test.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 
@@ -55,6 +56,61 @@ func (s *recordingSessionStore) AddMessage(_ context.Context, _ string, msg prov
 	s.added = append(s.added, msg)
 }
 
+// retryOnceProvider fails the first Chat call with a transient 429 via RetryDo
+// (which fires the injected retry hook), then succeeds — mimicking a flaky
+// provider whose internal RetryDo consumes the run's retry hook.
+type retryOnceProvider struct {
+	calls int
+	cfg   providers.RetryConfig
+}
+
+func (p *retryOnceProvider) Chat(ctx context.Context, _ providers.ChatRequest) (*providers.ChatResponse, error) {
+	return providers.RetryDo(ctx, p.cfg, func() (*providers.ChatResponse, error) {
+		p.calls++
+		if p.calls == 1 {
+			return nil, &providers.HTTPError{Status: 429, Body: "rate limited"}
+		}
+		return &providers.ChatResponse{Content: "ok"}, nil
+	})
+}
+
+func (p *retryOnceProvider) ChatStream(ctx context.Context, _ providers.ChatRequest, _ func(providers.StreamChunk)) (*providers.ChatResponse, error) {
+	return p.Chat(ctx, providers.ChatRequest{})
+}
+
+func (p *retryOnceProvider) DefaultModel() string { return "test-model" }
+func (p *retryOnceProvider) Name() string         { return "test-provider" }
+
+func TestMakeCallLLMEmitsRetryingOnTransientProviderError(t *testing.T) {
+	col := &eventCollector{}
+	loop := &Loop{id: "test-agent", onEvent: col.onEvent}
+	req := &RunRequest{RunID: "run-1", SessionKey: "sess-1", Channel: "telegram"}
+	state := &pipeline.RunState{
+		Provider: &retryOnceProvider{
+			cfg: providers.RetryConfig{Attempts: 2, MinDelay: time.Millisecond, MaxDelay: time.Millisecond},
+		},
+		Model:     "test-model",
+		Iteration: 0,
+	}
+
+	resp, err := loop.makeCallLLM(req, col.onEvent)(context.Background(), state, providers.ChatRequest{})
+	if err != nil {
+		t.Fatalf("makeCallLLM returned error: %v", err)
+	}
+	if resp == nil || resp.Content != "ok" {
+		t.Fatalf("unexpected response: %+v", resp)
+	}
+
+	retrying := col.filter(protocol.AgentEventRunRetrying)
+	if len(retrying) != 1 {
+		t.Fatalf("retrying events = %+v, want exactly one", retrying)
+	}
+	payload, ok := retrying[0].Payload.(map[string]string)
+	if !ok || payload["attempt"] != "1" || payload["maxAttempts"] != "2" {
+		t.Fatalf("retrying payload = %+v, want attempt=1 maxAttempts=2", retrying[0].Payload)
+	}
+}
+
 func TestMakeCallLLM_StreamsFinalThinkingWhenNoThinkingChunkArrives(t *testing.T) {
 	col := &eventCollector{}
 	loop := &Loop{id: "test-agent", onEvent: col.onEvent}

internal/channels/errors.go

@@ -1,11 +1,39 @@
 package channels
 
 import (
+	"errors"
 	"strings"
 
 	"github.com/nextlevelbuilder/goclaw/internal/providers"
 )
 
+// isTransientFailureMessage reports whether an agent run failure string is a
+// transient provider error (rate limit, timeout, overload, connection drop)
+// that the user can simply retry — as opposed to a permanent error (auth,
+// context overflow). Used to decide whether a failure should surface a
+// terminal 💔 reaction (see events.go reaction forwarding).
+func isTransientFailureMessage(errStr string) bool {
+	if errStr == "" {
+		return false
+	}
+	lower := strings.ToLower(errStr)
+	// Reuse the provider retryability classifier (429/5xx typed errors,
+	// connection reset, timeouts, rate limits, EOF) on the raw message.
+	if providers.IsRetryableError(errors.New(errStr)) {
+		return true
+	}
+	// Mirror FormatAgentError's transient matchers (rate limit, timeout,
+	// overload) plus HTTP 5xx status codes, which IsRetryableError only matches
+	// on typed HTTPError values — the string seen here has been re-wrapped.
+	return strings.Contains(lower, "rate limit") ||
+		strings.Contains(lower, "too many requests") ||
+		strings.Contains(lower, "429") ||
+		strings.Contains(lower, "timeout") ||
+		strings.Contains(lower, "deadline exceeded") ||
+		strings.Contains(lower, "overload") ||
+		strings.Contains(lower, "http 5")
+}
+
 // FormatAgentError converts internal error to user-friendly message.
 // Issue 958: Send user-friendly error on RunFailed instead of silent "...".
 func FormatAgentError(errStr string) string {

internal/channels/errors_test.go

@@ -63,3 +63,34 @@ func TestFormatAgentError_Empty(t *testing.T) {
 		t.Errorf("expected empty string for empty error, got %q", result)
 	}
 }
+
+func TestIsTransientFailureMessage(t *testing.T) {
+	t.Parallel()
+	transient := []string{
+		"llm call: HTTP 429: rate limited",
+		"request timeout",
+		"deadline exceeded",
+		"service overloaded",
+		"connection reset by peer",
+		"llm call: HTTP 503: service unavailable",
+		"too many requests",
+	}
+	for _, tc := range transient {
+		if !isTransientFailureMessage(tc) {
+			t.Errorf("expected %q to be transient", tc)
+		}
+	}
+
+	permanent := []string{
+		"unauthorized access",
+		"invalid api key",
+		"context length exceeded",
+		"request context budget exceeded",
+		"",
+	}
+	for _, tc := range permanent {
+		if isTransientFailureMessage(tc) {
+			t.Errorf("expected %q to be permanent", tc)
+		}
+	}
+}

internal/channels/events.go

@@ -417,7 +417,18 @@ func (m *Manager) HandleAgentEvent(eventType, runID string, payload any) {
 		case protocol.AgentEventRunCompleted:
 			status = "done"
 		case protocol.AgentEventRunFailed:
-			status = "error"
+			// Issue 3: transient provider failures (rate limit, timeout, overload)
+			// already surface a friendly retry message via FormatAgentError — a
+			// terminal 💔 reaction would be misleading. Clear any interim reaction
+			// instead; permanent errors keep the 💔.
+			if isTransientFailureMessage(extractPayloadString(payload, "error")) {
+				if err := reactionCh.ClearReaction(ctx, rc.ChatID, rc.MessageID); err != nil {
+					slog.Debug("reaction clear failed", "channel", rc.ChannelName, "error", err)
+				}
+				status = ""
+			} else {
+				status = "error"
+			}
 		case protocol.AgentEventRunCancelled:
 			status = "done"
 		}