fix: preserve bootstrap paths and expose failed mutations (#16131)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 385dcbd8a9
Co-authored-by: Swader <1430603+Swader@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras

src/agents/pi-embedded-runner/run/payloads.e2e.test.ts

@@ -229,7 +229,56 @@ describe("buildEmbeddedRunPayloads", () => {
     expect(payloads[0]?.text).toContain("code 1");
   });
 
-  it("suppresses recoverable tool errors containing 'required'", () => {
+  it("suppresses recoverable tool errors containing 'required' for non-mutating tools", () => {
+    const payloads = buildEmbeddedRunPayloads({
+      assistantTexts: [],
+      toolMetas: [],
+      lastAssistant: undefined,
+      lastToolError: { toolName: "browser", error: "url required" },
+      sessionKey: "session:telegram",
+      inlineToolResultsAllowed: false,
+      verboseLevel: "off",
+      reasoningLevel: "off",
+      toolResultFormat: "plain",
+    });
+
+    // Recoverable errors should not be sent to the user
+    expect(payloads).toHaveLength(0);
+  });
+
+  it("suppresses recoverable tool errors containing 'missing' for non-mutating tools", () => {
+    const payloads = buildEmbeddedRunPayloads({
+      assistantTexts: [],
+      toolMetas: [],
+      lastAssistant: undefined,
+      lastToolError: { toolName: "browser", error: "url missing" },
+      sessionKey: "session:telegram",
+      inlineToolResultsAllowed: false,
+      verboseLevel: "off",
+      reasoningLevel: "off",
+      toolResultFormat: "plain",
+    });
+
+    expect(payloads).toHaveLength(0);
+  });
+
+  it("suppresses recoverable tool errors containing 'invalid' for non-mutating tools", () => {
+    const payloads = buildEmbeddedRunPayloads({
+      assistantTexts: [],
+      toolMetas: [],
+      lastAssistant: undefined,
+      lastToolError: { toolName: "browser", error: "invalid parameter: url" },
+      sessionKey: "session:telegram",
+      inlineToolResultsAllowed: false,
+      verboseLevel: "off",
+      reasoningLevel: "off",
+      toolResultFormat: "plain",
+    });
+
+    expect(payloads).toHaveLength(0);
+  });
+
+  it("shows recoverable tool errors for mutating tools", () => {
     const payloads = buildEmbeddedRunPayloads({
       assistantTexts: [],
       toolMetas: [],
@@ -242,16 +291,17 @@ describe("buildEmbeddedRunPayloads", () => {
       toolResultFormat: "plain",
     });
 
-    // Recoverable errors should not be sent to the user
-    expect(payloads).toHaveLength(0);
+    expect(payloads).toHaveLength(1);
+    expect(payloads[0]?.isError).toBe(true);
+    expect(payloads[0]?.text).toContain("required");
   });
 
-  it("suppresses recoverable tool errors containing 'missing'", () => {
+  it("shows mutating tool errors even when assistant output exists", () => {
     const payloads = buildEmbeddedRunPayloads({
-      assistantTexts: [],
+      assistantTexts: ["Done."],
       toolMetas: [],
-      lastAssistant: undefined,
-      lastToolError: { toolName: "message", error: "messageId missing" },
+      lastAssistant: { stopReason: "end_turn" } as AssistantMessage,
+      lastToolError: { toolName: "write", error: "file missing" },
       sessionKey: "session:telegram",
       inlineToolResultsAllowed: false,
       verboseLevel: "off",
@@ -259,15 +309,22 @@ describe("buildEmbeddedRunPayloads", () => {
       toolResultFormat: "plain",
     });
 
-    expect(payloads).toHaveLength(0);
+    expect(payloads).toHaveLength(2);
+    expect(payloads[0]?.text).toBe("Done.");
+    expect(payloads[1]?.isError).toBe(true);
+    expect(payloads[1]?.text).toContain("missing");
   });
 
-  it("suppresses recoverable tool errors containing 'invalid'", () => {
+  it("does not treat session_status read failures as mutating when explicitly flagged", () => {
     const payloads = buildEmbeddedRunPayloads({
-      assistantTexts: [],
+      assistantTexts: ["Status loaded."],
       toolMetas: [],
-      lastAssistant: undefined,
-      lastToolError: { toolName: "message", error: "invalid parameter: to" },
+      lastAssistant: { stopReason: "end_turn" } as AssistantMessage,
+      lastToolError: {
+        toolName: "session_status",
+        error: "model required",
+        mutatingAction: false,
+      },
       sessionKey: "session:telegram",
       inlineToolResultsAllowed: false,
       verboseLevel: "off",
@@ -275,7 +332,47 @@ describe("buildEmbeddedRunPayloads", () => {
       toolResultFormat: "plain",
     });
 
-    expect(payloads).toHaveLength(0);
+    expect(payloads).toHaveLength(1);
+    expect(payloads[0]?.text).toBe("Status loaded.");
+  });
+
+  it("dedupes identical tool warning text already present in assistant output", () => {
+    const seed = buildEmbeddedRunPayloads({
+      assistantTexts: [],
+      toolMetas: [],
+      lastAssistant: undefined,
+      lastToolError: {
+        toolName: "write",
+        error: "file missing",
+        mutatingAction: true,
+      },
+      sessionKey: "session:telegram",
+      inlineToolResultsAllowed: false,
+      verboseLevel: "off",
+      reasoningLevel: "off",
+      toolResultFormat: "plain",
+    });
+    const warningText = seed[0]?.text;
+    expect(warningText).toBeTruthy();
+
+    const payloads = buildEmbeddedRunPayloads({
+      assistantTexts: [warningText ?? ""],
+      toolMetas: [],
+      lastAssistant: { stopReason: "end_turn" } as AssistantMessage,
+      lastToolError: {
+        toolName: "write",
+        error: "file missing",
+        mutatingAction: true,
+      },
+      sessionKey: "session:telegram",
+      inlineToolResultsAllowed: false,
+      verboseLevel: "off",
+      reasoningLevel: "off",
+      toolResultFormat: "plain",
+    });
+
+    expect(payloads).toHaveLength(1);
+    expect(payloads[0]?.text).toBe(warningText);
   });
 
   it("shows non-recoverable tool errors to the user", () => {

src/agents/pi-embedded-runner/run/payloads.ts

@@ -18,6 +18,7 @@ import {
   extractAssistantThinking,
   formatReasoningMessage,
 } from "../../pi-embedded-utils.js";
+import { isLikelyMutatingToolName } from "../../tool-mutation.js";
 
 type ToolMetaEntry = { toolName: string; meta?: string };
 
@@ -25,7 +26,13 @@ export function buildEmbeddedRunPayloads(params: {
   assistantTexts: string[];
   toolMetas: ToolMetaEntry[];
   lastAssistant: AssistantMessage | undefined;
-  lastToolError?: { toolName: string; meta?: string; error?: string };
+  lastToolError?: {
+    toolName: string;
+    meta?: string;
+    error?: string;
+    mutatingAction?: boolean;
+    actionFingerprint?: string;
+  };
   config?: OpenClawConfig;
   sessionKey: string;
   provider?: string;
@@ -223,22 +230,37 @@ export function buildEmbeddedRunPayloads(params: {
       errorLower.includes("must have") ||
       errorLower.includes("needs") ||
       errorLower.includes("requires");
+    const isMutatingToolError =
+      params.lastToolError.mutatingAction ??
+      isLikelyMutatingToolName(params.lastToolError.toolName);
+    const shouldShowToolError = isMutatingToolError || (!hasUserFacingReply && !isRecoverableError);
 
-    // Show tool errors only when:
-    // 1. There's no user-facing reply AND the error is not recoverable
-    // Recoverable errors (validation, missing params) are already in the model's context
-    // and shouldn't be surfaced to users since the model should retry.
-    if (!hasUserFacingReply && !isRecoverableError) {
+    // 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.
+    if (shouldShowToolError) {
       const toolSummary = formatToolAggregate(
         params.lastToolError.toolName,
         params.lastToolError.meta ? [params.lastToolError.meta] : undefined,
         { markdown: useMarkdown },
       );
       const errorSuffix = params.lastToolError.error ? `: ${params.lastToolError.error}` : "";
-      replyItems.push({
-        text: `⚠️ ${toolSummary} failed${errorSuffix}`,
-        isError: true,
-      });
+      const warningText = `⚠️ ${toolSummary} failed${errorSuffix}`;
+      const normalizedWarning = normalizeTextForComparison(warningText);
+      const duplicateWarning = normalizedWarning
+        ? replyItems.some((item) => {
+            if (!item.text) {
+              return false;
+            }
+            const normalizedExisting = normalizeTextForComparison(item.text);
+            return normalizedExisting.length > 0 && normalizedExisting === normalizedWarning;
+          })
+        : false;
+      if (!duplicateWarning) {
+        replyItems.push({
+          text: warningText,
+          isError: true,
+        });
+      }
     }
   }
 

src/agents/pi-embedded-runner/run/types.ts

@@ -33,7 +33,13 @@ export type EmbeddedRunAttemptResult = {
   assistantTexts: string[];
   toolMetas: Array<{ toolName: string; meta?: string }>;
   lastAssistant: AssistantMessage | undefined;
-  lastToolError?: { toolName: string; meta?: string; error?: string };
+  lastToolError?: {
+    toolName: string;
+    meta?: string;
+    error?: string;
+    mutatingAction?: boolean;
+    actionFingerprint?: string;
+  };
   didSendViaMessagingTool: boolean;
   messagingToolSentTexts: string[];
   messagingToolSentTargets: MessagingToolSend[];