Skip to content

Commit 6048cd4

Browse files
clawsweeper[bot]abnershangTakhoffman
authored
fix(cron): keep recovered tool warnings diagnostic (#84308)
Summary: - The PR threads middleware tool-error metadata into reply payloads, teaches cron outcome and diagnostics code to keep marked recovered warnings non-fatal, and adds focused regression coverage plus a changelog entry. - Reproducibility: yes. Source inspection shows current main lacks a non-terminal recovered-warning path in cr ... fication, and the linked source PR includes a terminal runtime probe for the affected cron payload outcome. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(cron): keep recovered tool warnings diagnostic Validation: - ClawSweeper review passed for head 8b8a36e. - Required merge gates passed before the squash merge. Prepared head SHA: 8b8a36e Review: #84308 (comment) Co-authored-by: abnershang <abner.shang@gmail.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
1 parent d7896ed commit 6048cd4

12 files changed

Lines changed: 219 additions & 9 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
1717
- Discord: preserve disabled presentation buttons when adapting and rendering Discord message controls. (#84188) Thanks @100menotu001.
1818
- Twitch: add a test-only client-manager registry reset helper so non-isolated Twitch tests can clear cached managers between cases. Fixes #83887. (#84244) Thanks @hclsys.
1919
- Cron: use structured embedded-run denial metadata for isolated scheduled tasks so blocked exec requests fail the job without treating ordinary assistant prose as a denial. (#84067) Thanks @abnershang.
20+
- Cron: keep recovered tool warnings diagnostic for successful scheduled runs so final cron output is delivered instead of being replaced by a post-processing warning. (#84045) Thanks @abnershang.
2021
- Plugins/perf: thread explicit plugin discovery results through `loadBundledCapabilityRuntimeRegistry`, `resolveBundledPluginSources`, and `listChannelCatalogEntries` so callers that already hold a discovery result skip redundant filesystem walks. Thanks @SebTardif.
2122
- harden update restart script creation [AI]. (#84088) Thanks @pgondhi987.
2223
- Docker: keep the bundled Codex plugin in official release image keep lists so the default OpenAI agent harness remains available after Docker pruning. Fixes #83613. (#83626) Thanks @YuanHanzhong.

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { AssistantMessage } from "@earendil-works/pi-ai";
22
import { describe, expect, it } from "vitest";
3+
import { getReplyPayloadMetadata } from "../../../auto-reply/reply-payload.js";
34
import { formatBillingErrorMessage } from "../../pi-embedded-helpers.js";
45
import { makeAssistantMessageFixture } from "../../test-helpers/assistant-message-fixtures.js";
56
import {
@@ -457,6 +458,9 @@ describe("buildEmbeddedRunPayloads", () => {
457458
expect(payloads[1]?.isError).toBe(true);
458459
expect(payloads[1]?.text).toContain("Write");
459460
expect(payloads[1]?.text).not.toContain("missing");
461+
expect(getReplyPayloadMetadata(payloads[1] as object)?.nonTerminalToolErrorWarning).toBe(
462+
undefined,
463+
);
460464
});
461465

462466
it("shows exec tool errors when assistant output claims success", () => {
@@ -474,6 +478,9 @@ describe("buildEmbeddedRunPayloads", () => {
474478
expect(payloads[1]?.isError).toBe(true);
475479
expect(payloads[1]?.text).toContain("Exec");
476480
expect(payloads[1]?.text).not.toContain("python: command not found");
481+
expect(getReplyPayloadMetadata(payloads[1] as object)?.nonTerminalToolErrorWarning).toBe(
482+
undefined,
483+
);
477484
});
478485

479486
it("shows mutating tool errors when assistant output does not acknowledge the failure", () => {

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,28 @@ describe("buildEmbeddedRunPayloads tool-error warnings", () => {
351351
});
352352
});
353353

354+
it("marks middleware tool-error warnings after assistant output as non-terminal", () => {
355+
const payloads = buildPayloads({
356+
assistantTexts: ["Queued 3 topics."],
357+
lastToolError: {
358+
toolName: "exec",
359+
error: "Tool output unavailable due to post-processing error",
360+
middlewareError: true,
361+
},
362+
verboseLevel: "off",
363+
});
364+
365+
expect(payloads).toHaveLength(2);
366+
expect(payloads[0]?.text).toBe("Queued 3 topics.");
367+
expect(payloads[1]).toMatchObject({
368+
isError: true,
369+
});
370+
expect(payloads[1]?.text).toContain("Exec failed");
371+
expect(getReplyPayloadMetadata(payloads[1] as object)).toMatchObject({
372+
nonTerminalToolErrorWarning: true,
373+
});
374+
});
375+
354376
it("surfaces concise bash tool errors when verbose mode is off", () => {
355377
const payloads = buildPayloads({
356378
lastToolError: { toolName: "bash", error: "command failed" },

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ function shouldIncludeToolErrorDetails(params: {
136136
);
137137
}
138138

139+
function shouldMarkNonTerminalToolErrorWarning(lastToolError: ToolErrorSummary): boolean {
140+
return lastToolError.middlewareError === true;
141+
}
142+
139143
function resolveToolErrorWarningPolicy(params: {
140144
lastToolError: ToolErrorSummary;
141145
hasUserFacingReply: boolean;
@@ -221,6 +225,7 @@ export function buildEmbeddedRunPayloads(params: {
221225
presentation?: ReplyPayload["presentation"];
222226
interactive?: ReplyPayload["interactive"];
223227
channelData?: Record<string, unknown>;
228+
nonTerminalToolErrorWarning?: boolean;
224229
sourceReplyMirror?: {
225230
idempotencyKey?: string;
226231
};
@@ -509,6 +514,9 @@ export function buildEmbeddedRunPayloads(params: {
509514
replyItems.push({
510515
text: warningText,
511516
isError: true,
517+
nonTerminalToolErrorWarning:
518+
hasUserFacingAssistantReply &&
519+
shouldMarkNonTerminalToolErrorWarning(params.lastToolError),
512520
});
513521
}
514522
}
@@ -530,6 +538,11 @@ export function buildEmbeddedRunPayloads(params: {
530538
if (item.isError !== undefined) {
531539
payload.isError = item.isError;
532540
}
541+
if (item.nonTerminalToolErrorWarning) {
542+
setReplyPayloadMetadata(payload, {
543+
nonTerminalToolErrorWarning: true,
544+
});
545+
}
533546
if (item.replyToId) {
534547
payload.replyToId = item.replyToId;
535548
}

‎src/agents/pi-embedded-subscribe.handlers.tools.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,47 @@ describe("handleToolExecutionEnd cron.add commitment tracking", () => {
281281
});
282282

283283
describe("handleToolExecutionEnd mutating failure recovery", () => {
284+
it("marks middleware failures on the last tool error", async () => {
285+
const { ctx } = createTestContext();
286+
287+
await handleToolExecutionStart(
288+
ctx as never,
289+
{
290+
type: "tool_execution_start",
291+
toolName: "exec",
292+
toolCallId: "tool-exec-middleware-error",
293+
args: { cmd: "echo ok" },
294+
} as never,
295+
);
296+
297+
await handleToolExecutionEnd(
298+
ctx as never,
299+
{
300+
type: "tool_execution_end",
301+
toolName: "exec",
302+
toolCallId: "tool-exec-middleware-error",
303+
isError: false,
304+
result: {
305+
content: [
306+
{
307+
type: "text",
308+
text: "Tool output unavailable due to post-processing error.",
309+
},
310+
],
311+
details: {
312+
status: "error",
313+
middlewareError: true,
314+
},
315+
},
316+
} as never,
317+
);
318+
319+
expect(ctx.state.lastToolError).toMatchObject({
320+
toolName: "exec",
321+
middlewareError: true,
322+
});
323+
});
324+
284325
it("clears edit failure when the retry succeeds through common file path aliases", async () => {
285326
const { ctx } = createTestContext();
286327

‎src/agents/pi-embedded-subscribe.handlers.tools.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,19 @@ const beforeToolCallModuleLoader = createLazyImportLoader<BeforeToolCallModule>(
6868
const LIVE_EXEC_OUTPUT_MAX_CHARS = 8000;
6969
const LIVE_EXEC_UPDATE_MIN_INTERVAL_MS = 250;
7070

71+
function isMiddlewareToolResultError(result: unknown): boolean {
72+
if (!result || typeof result !== "object") {
73+
return false;
74+
}
75+
const details = (result as { details?: unknown }).details;
76+
return Boolean(
77+
details &&
78+
typeof details === "object" &&
79+
!Array.isArray(details) &&
80+
(details as { middlewareError?: unknown }).middlewareError === true,
81+
);
82+
}
83+
7184
function loadExecApprovalReply(): Promise<ExecApprovalReplyModule> {
7285
return execApprovalReplyModuleLoader.load();
7386
}
@@ -942,6 +955,7 @@ export async function handleToolExecutionEnd(
942955
...(errorCode ? { errorCode } : {}),
943956
error: errorMessage,
944957
timedOut: isToolResultTimedOut(sanitizedResult) || undefined,
958+
middlewareError: isMiddlewareToolResultError(sanitizedResult) || undefined,
945959
mutatingAction: callSummary?.mutatingAction,
946960
actionFingerprint: callSummary?.actionFingerprint,
947961
fileTarget: callSummary?.fileTarget,

‎src/agents/tool-error-summary.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export type ToolErrorSummary = {
77
errorCode?: string;
88
error?: string;
99
timedOut?: boolean;
10+
middlewareError?: boolean;
1011
mutatingAction?: boolean;
1112
actionFingerprint?: string;
1213
fileTarget?: FileTarget;

‎src/auto-reply/reply-payload.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ export type ReplyPayloadMetadata = {
163163
idempotencyKey?: string;
164164
};
165165
beforeAgentRunBlocked?: boolean;
166+
/** Warning synthesized from an observed tool error after the run produced assistant output. */
167+
nonTerminalToolErrorWarning?: boolean;
166168
};
167169

168170
const replyPayloadMetadata = new WeakMap<object, ReplyPayloadMetadata>();

‎src/cron/isolated-agent.helpers.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it } from "vitest";
2+
import { setReplyPayloadMetadata } from "../auto-reply/reply-payload.js";
23
import { resolveCronPayloadOutcome } from "./isolated-agent/helpers.js";
34

45
describe("resolveCronPayloadOutcome", () => {
@@ -39,6 +40,51 @@ describe("resolveCronPayloadOutcome", () => {
3940
expect(result.summary).toBe("Write completed successfully.");
4041
});
4142

43+
it("keeps non-terminal tool warnings diagnostic when final assistant output succeeded", () => {
44+
const toolWarning = setReplyPayloadMetadata(
45+
{
46+
text: "⚠️ Exec failed",
47+
isError: true,
48+
},
49+
{ nonTerminalToolErrorWarning: true },
50+
);
51+
52+
const result = resolveCronPayloadOutcome({
53+
payloads: [{ text: "Queued 3 topics." }, toolWarning],
54+
finalAssistantVisibleText: "Queued 3 topics.",
55+
preferFinalAssistantVisibleText: true,
56+
});
57+
58+
expect(result.hasFatalErrorPayload).toBe(false);
59+
expect(result.embeddedRunError).toBeUndefined();
60+
expect(result.summary).toBe("Queued 3 topics.");
61+
expect(result.outputText).toBe("Queued 3 topics.");
62+
expect(result.deliveryPayloads).toEqual([{ text: "Queued 3 topics." }]);
63+
});
64+
65+
it("keeps marked middleware warnings diagnostic after structured cron output", () => {
66+
const mediaPayload = { mediaUrl: "file:///tmp/cron-report.png" };
67+
const toolWarning = setReplyPayloadMetadata(
68+
{
69+
text: "⚠️ Exec failed",
70+
isError: true,
71+
},
72+
{ nonTerminalToolErrorWarning: true },
73+
);
74+
75+
const result = resolveCronPayloadOutcome({
76+
payloads: [mediaPayload, toolWarning],
77+
});
78+
79+
expect(result.hasFatalErrorPayload).toBe(false);
80+
expect(result.embeddedRunError).toBeUndefined();
81+
expect(result.summary).toBeUndefined();
82+
expect(result.outputText).toBeUndefined();
83+
expect(result.synthesizedText).toBeUndefined();
84+
expect(result.deliveryPayloads).toEqual([mediaPayload]);
85+
expect(result.deliveryPayloadHasStructuredContent).toBe(true);
86+
});
87+
4288
it("treats trailing message delivery warnings as non-fatal when final assistant text exists", () => {
4389
const result = resolveCronPayloadOutcome({
4490
payloads: [{ text: "Draft output" }, { text: "⚠️ ✉️ Message failed", isError: true }],

‎src/cron/isolated-agent/helpers.ts‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { hasOutboundReplyContent } from "openclaw/plugin-sdk/reply-payload";
22
import { DEFAULT_HEARTBEAT_ACK_MAX_CHARS } from "../../auto-reply/heartbeat.js";
3+
import { getReplyPayloadMetadata } from "../../auto-reply/reply-payload.js";
34
import type { ReplyPayload } from "../../auto-reply/reply-payload.js";
45
import { normalizeOptionalString } from "../../shared/string-coerce.js";
56
import { truncateUtf16Safe } from "../../utils.js";
@@ -97,6 +98,9 @@ export function pickSummaryFromPayloads(
9798
}
9899
}
99100
for (let i = payloads.length - 1; i >= 0; i--) {
101+
if (isNonTerminalToolErrorWarning(payloads[i])) {
102+
continue;
103+
}
100104
const summary = pickSummaryFromOutput(payloads[i]?.text);
101105
if (summary) {
102106
return summary;
@@ -118,6 +122,9 @@ export function pickLastNonEmptyTextFromPayloads(
118122
}
119123
}
120124
for (let i = payloads.length - 1; i >= 0; i--) {
125+
if (isNonTerminalToolErrorWarning(payloads[i])) {
126+
continue;
127+
}
121128
const clean = (payloads[i]?.text ?? "").trim();
122129
if (clean) {
123130
return clean;
@@ -195,14 +202,26 @@ function isCronMessagePresentationWarning(text: string | undefined): boolean {
195202
);
196203
}
197204

205+
function isNonTerminalToolErrorWarning(payload: object | undefined): boolean {
206+
return Boolean(payload && getReplyPayloadMetadata(payload)?.nonTerminalToolErrorWarning);
207+
}
208+
209+
function isSuccessfulCronPayload(payload: DeliveryPayload | undefined): boolean {
210+
return (
211+
payload?.isError !== true &&
212+
(isDeliverablePayload(payload) || payloadHasStructuredDeliveryContent(payload))
213+
);
214+
}
215+
198216
export function resolveCronPayloadOutcome(params: {
199217
payloads: DeliveryPayload[];
200218
runLevelError?: unknown;
201219
failureSignal?: CronFailureSignal | undefined;
202220
finalAssistantVisibleText?: string | undefined;
203221
preferFinalAssistantVisibleText?: boolean;
204222
}): CronPayloadOutcome {
205-
const firstText = params.payloads[0]?.text ?? "";
223+
const firstText =
224+
params.payloads.find((payload) => !isNonTerminalToolErrorWarning(payload))?.text ?? "";
206225
const fallbackSummary =
207226
pickSummaryFromPayloads(params.payloads) ?? pickSummaryFromOutput(firstText);
208227
const fallbackOutputText = pickLastNonEmptyTextFromPayloads(params.payloads);
@@ -223,23 +242,33 @@ export function resolveCronPayloadOutcome(params: {
223242
const hasSuccessfulPayloadAfterLastError =
224243
!params.runLevelError &&
225244
lastErrorPayloadIndex >= 0 &&
226-
params.payloads
227-
.slice(lastErrorPayloadIndex + 1)
228-
.some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
245+
params.payloads.slice(lastErrorPayloadIndex + 1).some(isSuccessfulCronPayload);
229246
const hasSuccessfulPayloadBeforeLastError =
230247
!params.runLevelError &&
231248
lastErrorPayloadIndex > 0 &&
232-
params.payloads
233-
.slice(0, lastErrorPayloadIndex)
234-
.some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
249+
params.payloads.slice(0, lastErrorPayloadIndex).some(isSuccessfulCronPayload);
250+
const lastErrorPayload =
251+
lastErrorPayloadIndex >= 0 ? params.payloads[lastErrorPayloadIndex] : undefined;
252+
const hasRecoveringTerminalOutput =
253+
normalizedFinalAssistantVisibleText !== undefined ||
254+
hasSuccessfulPayloadAfterLastError ||
255+
hasSuccessfulPayloadBeforeLastError;
256+
const hasNonTerminalToolErrorWarning =
257+
!params.runLevelError &&
258+
params.failureSignal?.fatalForCron !== true &&
259+
hasRecoveringTerminalOutput &&
260+
isNonTerminalToolErrorWarning(lastErrorPayload);
235261
const hasPendingPresentationWarning =
236262
!params.runLevelError &&
237263
params.failureSignal?.fatalForCron !== true &&
238264
lastErrorPayloadIndex >= 0 &&
239265
isCronMessagePresentationWarning(lastErrorPayloadText) &&
240266
(normalizedFinalAssistantVisibleText !== undefined || hasSuccessfulPayloadBeforeLastError);
241267
const hasFatalStructuredErrorPayload =
242-
hasErrorPayload && !hasSuccessfulPayloadAfterLastError && !hasPendingPresentationWarning;
268+
hasErrorPayload &&
269+
!hasSuccessfulPayloadAfterLastError &&
270+
!hasPendingPresentationWarning &&
271+
!hasNonTerminalToolErrorWarning;
243272
const hasStructuredDeliveryPayloads = selectedDeliveryPayloads.some((payload) =>
244273
payloadHasStructuredDeliveryContent(payload),
245274
);

0 commit comments

Comments
 (0)