From d08ff2c2c9e8721043141f3be4efdc424f7dad61 Mon Sep 17 00:00:00 2001
From: Sebastian <19554889+sebslight@users.noreply.github.com>
Date: Sat, 14 Feb 2026 22:34:39 -0500
Subject: [PATCH] refactor(agents): extract tool-error warning helpers

---
 src/agents/pi-embedded-runner/run/payloads.ts | 68 ++++++++++++-------
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/src/agents/pi-embedded-runner/run/payloads.ts b/src/agents/pi-embedded-runner/run/payloads.ts
index 28dc9613c9a..e7a4f74b89f 100644
--- a/src/agents/pi-embedded-runner/run/payloads.ts
+++ b/src/agents/pi-embedded-runner/run/payloads.ts
@@ -21,18 +21,50 @@ import {
 import { isLikelyMutatingToolName } from "../../tool-mutation.js";
 
 type ToolMetaEntry = { toolName: string; meta?: string };
+type LastToolError = {
+  toolName: string;
+  meta?: string;
+  error?: string;
+  mutatingAction?: boolean;
+  actionFingerprint?: string;
+};
+
+const RECOVERABLE_TOOL_ERROR_KEYWORDS = [
+  "required",
+  "missing",
+  "invalid",
+  "must be",
+  "must have",
+  "needs",
+  "requires",
+] as const;
+
+function isRecoverableToolError(error: string | undefined): boolean {
+  const errorLower = (error ?? "").toLowerCase();
+  return RECOVERABLE_TOOL_ERROR_KEYWORDS.some((keyword) => errorLower.includes(keyword));
+}
+
+function shouldShowToolErrorWarning(params: {
+  lastToolError: LastToolError;
+  hasUserFacingReply: boolean;
+  suppressToolErrors: boolean;
+}): boolean {
+  const isMutatingToolError =
+    params.lastToolError.mutatingAction ?? isLikelyMutatingToolName(params.lastToolError.toolName);
+  if (isMutatingToolError) {
+    return true;
+  }
+  if (params.suppressToolErrors) {
+    return false;
+  }
+  return !params.hasUserFacingReply && !isRecoverableToolError(params.lastToolError.error);
+}
 
 export function buildEmbeddedRunPayloads(params: {
   assistantTexts: string[];
   toolMetas: ToolMetaEntry[];
   lastAssistant: AssistantMessage | undefined;
-  lastToolError?: {
-    toolName: string;
-    meta?: string;
-    error?: string;
-    mutatingAction?: boolean;
-    actionFingerprint?: string;
-  };
+  lastToolError?: LastToolError;
   config?: OpenClawConfig;
   sessionKey: string;
   provider?: string;
@@ -219,23 +251,11 @@ export function buildEmbeddedRunPayloads(params: {
     const lastAssistantWasToolUse = params.lastAssistant?.stopReason === "toolUse";
     const hasUserFacingReply =
       replyItems.length > 0 && !lastAssistantHasToolCalls && !lastAssistantWasToolUse;
-    // Check if this is a recoverable/internal tool error that shouldn't be shown to users
-    // when there's already a user-facing reply (the model should have retried).
-    const errorLower = (params.lastToolError.error ?? "").toLowerCase();
-    const isRecoverableError =
-      errorLower.includes("required") ||
-      errorLower.includes("missing") ||
-      errorLower.includes("invalid") ||
-      errorLower.includes("must be") ||
-      errorLower.includes("must have") ||
-      errorLower.includes("needs") ||
-      errorLower.includes("requires");
-    const isMutatingToolError =
-      params.lastToolError.mutatingAction ??
-      isLikelyMutatingToolName(params.lastToolError.toolName);
-    const shouldShowToolError =
-      isMutatingToolError ||
-      (!hasUserFacingReply && !isRecoverableError && !params.config?.messages?.suppressToolErrors);
+    const shouldShowToolError = shouldShowToolErrorWarning({
+      lastToolError: params.lastToolError,
+      hasUserFacingReply,
+      suppressToolErrors: Boolean(params.config?.messages?.suppressToolErrors),
+    });
 
     // Always surface mutating tool failures so we do not silently confirm actions that did not happen.
     // Otherwise, keep the previous behavior and only surface non-recoverable failures when no reply exists.