From 0341dd884e46fa3b850512d5cb6acfc653c0f143 Mon Sep 17 00:00:00 2001
From: Stefan Ernst <24554551+stefan-ernst@users.noreply.github.com>
Date: Mon, 2 Nov 2026 14:57:07 +0100
Subject: [PATCH] fix(llm): do not cache transient HTTP read failures (WI-1802)

The agent read cache treated any nil-error result without a top-level error
field as reusable. The HTTP tool returns 429/503 as ordinary JSON with a
status_code and body, so a transient failure was cached: the model's retry was
suppressed as a duplicate and the loop could stop as no_progress without the
provider ever being retried.

Cache a read only when its observable result is reusable; a transient HTTP
status (408/429/5xx) is not.
---
 internal/llm/agent.go | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/internal/llm/agent.go b/internal/llm/agent.go
index 37ef68354..24e658f3b 100644
--- a/internal/llm/agent.go
+++ b/internal/llm/agent.go
@@ -252,7 +252,7 @@ func RunAgent(ctx context.Context, client Client, cfg AgentConfig, userMessage s
 				terminalCompletedThisTurn = true
 				// A mutation may have changed what reads return.
 				readResults = map[string]string{}
-			} else if !isTerminalTool && !duplicateSuppressed && !afterTerminalSuppressed && succeeded {
+			} else if !isTerminalTool && !duplicateSuppressed && !afterTerminalSuppressed && succeeded && toolResultCacheable(result) {
 				readResults[key] = result
 			}
 			slog.Info("agent tool call",
@@ -498,3 +498,27 @@ func toolReturnedError(result string) bool {
 	}
 	return false
 }
+
+// toolResultCacheable reports whether a read tool's result may be reused for a
+// later identical call. Only the HTTP tool reports a status; a transient HTTP
+// failure must not be cached, because the retry may succeed and suppressing it
+// would stall the loop as no_progress without the provider ever being retried.
+func toolResultCacheable(result string) bool {
+	if !strings.Contains(result, `"status_code"`) {
+		return true
+	}
+	var probe struct {
+		StatusCode *int `json:"status_code"`
+	}
+	if err := json.NewDecoder(strings.NewReader(result)).Decode(&probe); err != nil || probe.StatusCode == nil {
+		return true
+	}
+	return !transientHTTPStatus(*probe.StatusCode)
+}
+
+// transientHTTPStatus reports whether an HTTP status is worth retrying rather
+// than reusing a cached read: 408 and 429 are timeout/throttling, and 5xx is a
+// server-side failure. All may clear on a retry.
+func transientHTTPStatus(code int) bool {
+	return code == 408 || code == 429 || code >= 500
+}