PR #81557: fix(agents): mark caught tool execution failures with isError:true (#81546)

• Repository: openclaw/openclaw
• Author: SymbolStar
• State: closed | merged: False
• Link: https://github.com/openclaw/openclaw/pull/81557

Description (problem / solution / changelog)

Summary

Fixes #81546.

When a tool throws inside the pi tool-definition adapter's catch path, the failure was wrapped as a status:"error" payload via jsonResult() but the resulting AgentToolResult had no isError flag. Downstream consumers that key off the structured flag instead of parsing the JSON status field then treated the result as a successful tool delivery containing text that happened to mention an error:

• anthropic-transport-stream.ts sends is_error: undefined (i.e. not an error) on the resulting tool_result block.
• session-transcript-repair.ts / runner result classifiers filter on isError === true.
• The model only sees a normal-looking tool result and decides not to retry.

In the reporter's session, an ambiguous-oldText edit failure surfaced as {status:"error", error:"Found 4 occurrences ..."} with isError=false; the agent read the file, then ended the session normally without ever retrying the write. Codex review confirmed the gap is exactly here in current main: caught tool exceptions become payload errors without an explicit error flag.

Change

src/agents/pi-tool-definition-adapter.ts – buildToolExecutionErrorResult now returns { ...jsonResult(...), isError: true } so caught tool exceptions are flagged as real errors on every adapter path (Anthropic is_error, OpenAI tool_result, transcript repair, mutating-failure tracking) while preserving the existing status:"error" payload shape for backwards compatibility.

The mutating-failure warning / retry-tracking logic in pi-embedded-subscribe.handlers.tools.ts is intentionally untouched – per the codex-review guidance we are not adding blind automatic retries of mutating edits, only restoring the model-visible failure marker so the model itself can react.

Tests

• src/agents/pi-tool-definition-adapter.test.ts – the existing "wraps tool errors into a tool result" and exec-alias cases now also assert the wrapped error result has isError: true (regression for #81546).

The pre-existing handler tests covering status:"error" recognition (pi-embedded-subscribe.handlers.tools.test.ts) continue to pass: that path already tolerated either flag, and now it gets the explicit one too.

Notes

• No prettier/format-only churn; only the targeted lines were edited.
• Verified no existing PR open for #81546 before pushing (gh pr list -R openclaw/openclaw --search "81546" --state all).

Changed files

• src/agents/pi-tool-definition-adapter.test.ts (modified, +5/-0)
• src/agents/pi-tool-definition-adapter.ts (modified, +6/-1)

PR #81564: fix(agents): preserve Pi tool result error flags (#81546)

• Repository: openclaw/openclaw
• Author: MonkeyLeeT
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/81564

Description (problem / solution / changelog)

Summary

Fixes #81546.

This marks Pi tool results with details.status of error or timeout as real model-visible failures in the embedded Pi tool_result bridge. That bridge is the seam pi-agent-core uses through afterToolCall to override the final toolResult.isError value written to transcript/model messages.

Root Cause

OpenClaw's tool-definition adapter catches tool exceptions and wraps them as a returned AgentToolResult with details.status: "error". In pi-agent-core, returned tool results are still finalized with isError: false; the core runtime only flips the final flag when the tool actually throws or when afterToolCall returns an isError override.

That means an ambiguous edit failure could be delivered to the model as a normal successful tool result whose JSON text happened to contain status: "error". The model then had no reliable failure signal to drive a retry.

This intentionally fixes the Pi finalization bridge instead of adding blind retry logic. It also preserves the failure flag when tool-result middleware mutates, rewrites, or redacts the result details.

Related: #81557 marks the adapter return object with isError: true, but pi-agent-core does not use returned result fields to set the finalized isError flag for successful returns. This PR fixes the path that controls the final transcript/model-visible flag.

Real Behavior Proof

Reported pre-fix symptom from #81546:

Line 194 | role=toolResult | isError=False
  text: {
    "status": "error",
    "tool": "edit",
    "error": "Found 4 occurrences ... Each oldText must be unique."
  }

Line 197 | role=assistant | stopReason=stop
  NO WRITE TOOL CALL — agent pivoted away from the task

After-fix focused harness proof at 3929f9e97121f4eb22aea7535246f69e82b39933:

• src/agents/pi-embedded-runner.extensions.test.ts now exercises the embedded Pi tool_result bridge with an edit result whose details.status is error and asserts the bridge returns isError: true.
• The same test mutates event.result in place through tool-result middleware and verifies the original pre-middleware error status still produces final isError: true.

Relevant local output:

$ pnpm_config_verify_deps_before_run=false pnpm test src/agents/pi-embedded-runner.extensions.test.ts src/agents/pi-embedded-subscribe.tools.test.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts src/agents/pi-tool-definition-adapter.test.ts
[test] starting test/vitest/vitest.unit-fast.config.ts
Test Files  1 passed (1)
Tests  18 passed (18)

[test] starting test/vitest/vitest.agents.config.ts
Test Files  3 passed (3)
Tests  53 passed (53)

[test] passed 2 Vitest shards in 7.78s

Verification

• pnpm_config_verify_deps_before_run=false pnpm test src/agents/pi-embedded-runner.extensions.test.ts src/agents/pi-embedded-subscribe.tools.test.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts src/agents/pi-tool-definition-adapter.test.ts
• pnpm_config_verify_deps_before_run=false pnpm check:changed
• git diff --check

pnpm check:changed completed successfully locally, including core typecheck, core test typecheck, core lint, import-cycle check, and changed-surface guard scripts. A plain pnpm test ... attempt got stuck in pnpm's dependency preflight after printing install completion, so the successful verification commands set pnpm_config_verify_deps_before_run=false to bypass that preflight.

Changed files

• CHANGELOG.md (modified, +1/-0)
• src/agents/pi-embedded-runner.extensions.test.ts (modified, +93/-0)
• src/agents/pi-embedded-runner/extensions.ts (modified, +11/-0)