mirror of https://github.com/openclaw/openclaw.git
fix: gate Telegram exec tool warnings behind verbose mode (#20560)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 7ce94931f0
Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com>
Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com>
Reviewed-by: @obviyus
This commit is contained in:
parent
b228c06bbd
commit
6b05916c14
|
|
@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai
|
|||
|
||||
### Fixes
|
||||
|
||||
- Telegram/Agents: gate exec/bash tool-failure warnings behind verbose mode so default Telegram replies stay clean while verbose sessions still surface diagnostics. (#20560) Thanks @obviyus.
|
||||
- Gateway/Daemon: forward `TMPDIR` into installed service environments so macOS LaunchAgent gateway runs can open SQLite temp/journal files reliably instead of failing with `SQLITE_CANTOPEN`. (#20512) Thanks @Clawborn.
|
||||
- Agents/Billing: include the active model that produced a billing error in user-facing billing messages (for example, `OpenAI (gpt-5.3)`) across payload, failover, and lifecycle error paths, so users can identify exactly which key needs credits. (#20510) Thanks @echoVic.
|
||||
- iOS/Screen: move `WKWebView` lifecycle ownership into `ScreenWebView` coordinator and explicit attach/detach flow to reduce gesture/lifecycle crash risk (`__NSArrayM insertObject:atIndex:` paths) during screen tab updates. (#20366) Thanks @ngutman.
|
||||
|
|
|
|||
|
|
@ -163,7 +163,7 @@ describe("buildEmbeddedRunPayloads", () => {
|
|||
expect(payloads[0]?.text).toBe("All good");
|
||||
});
|
||||
|
||||
it("adds tool error fallback when the assistant only invoked tools", () => {
|
||||
it("adds tool error fallback when the assistant only invoked tools and verbose mode is on", () => {
|
||||
const payloads = buildPayloads({
|
||||
lastAssistant: makeAssistant({
|
||||
stopReason: "toolUse",
|
||||
|
|
@ -178,6 +178,7 @@ describe("buildEmbeddedRunPayloads", () => {
|
|||
],
|
||||
}),
|
||||
lastToolError: { toolName: "exec", error: "Command exited with code 1" },
|
||||
verboseLevel: "on",
|
||||
});
|
||||
|
||||
expect(payloads).toHaveLength(1);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,52 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { buildEmbeddedRunPayloads } from "./payloads.js";
|
||||
|
||||
type BuildPayloadParams = Parameters<typeof buildEmbeddedRunPayloads>[0];
|
||||
|
||||
function buildPayloads(overrides: Partial<BuildPayloadParams> = {}) {
|
||||
return buildEmbeddedRunPayloads({
|
||||
assistantTexts: [],
|
||||
toolMetas: [],
|
||||
lastAssistant: undefined,
|
||||
sessionKey: "session:telegram",
|
||||
inlineToolResultsAllowed: false,
|
||||
verboseLevel: "off",
|
||||
reasoningLevel: "off",
|
||||
toolResultFormat: "plain",
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
describe("buildEmbeddedRunPayloads tool-error warnings", () => {
|
||||
it("suppresses exec tool errors when verbose mode is off", () => {
|
||||
const payloads = buildPayloads({
|
||||
lastToolError: { toolName: "exec", error: "command failed" },
|
||||
verboseLevel: "off",
|
||||
});
|
||||
|
||||
expect(payloads).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("shows exec tool errors when verbose mode is on", () => {
|
||||
const payloads = buildPayloads({
|
||||
lastToolError: { toolName: "exec", error: "command failed" },
|
||||
verboseLevel: "on",
|
||||
});
|
||||
|
||||
expect(payloads).toHaveLength(1);
|
||||
expect(payloads[0]?.isError).toBe(true);
|
||||
expect(payloads[0]?.text).toContain("Exec");
|
||||
expect(payloads[0]?.text).toContain("command failed");
|
||||
});
|
||||
|
||||
it("keeps non-exec mutating tool failures visible", () => {
|
||||
const payloads = buildPayloads({
|
||||
lastToolError: { toolName: "write", error: "permission denied" },
|
||||
verboseLevel: "off",
|
||||
});
|
||||
|
||||
expect(payloads).toHaveLength(1);
|
||||
expect(payloads[0]?.isError).toBe(true);
|
||||
expect(payloads[0]?.text).toContain("Write");
|
||||
});
|
||||
});
|
||||
|
|
@ -49,10 +49,16 @@ function shouldShowToolErrorWarning(params: {
|
|||
hasUserFacingReply: boolean;
|
||||
suppressToolErrors: boolean;
|
||||
suppressToolErrorWarnings?: boolean;
|
||||
verboseLevel?: VerboseLevel;
|
||||
}): boolean {
|
||||
if (params.suppressToolErrorWarnings) {
|
||||
return false;
|
||||
}
|
||||
const normalizedToolName = params.lastToolError.toolName.trim().toLowerCase();
|
||||
const verboseEnabled = params.verboseLevel === "on" || params.verboseLevel === "full";
|
||||
if ((normalizedToolName === "exec" || normalizedToolName === "bash") && !verboseEnabled) {
|
||||
return false;
|
||||
}
|
||||
const isMutatingToolError =
|
||||
params.lastToolError.mutatingAction ?? isLikelyMutatingToolName(params.lastToolError.toolName);
|
||||
if (isMutatingToolError) {
|
||||
|
|
@ -255,6 +261,7 @@ export function buildEmbeddedRunPayloads(params: {
|
|||
hasUserFacingReply: hasUserFacingAssistantReply,
|
||||
suppressToolErrors: Boolean(params.config?.messages?.suppressToolErrors),
|
||||
suppressToolErrorWarnings: params.suppressToolErrorWarnings,
|
||||
verboseLevel: params.verboseLevel,
|
||||
});
|
||||
|
||||
// Always surface mutating tool failures so we do not silently confirm actions that did not happen.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { extractToolErrorMessage } from "./pi-embedded-subscribe.tools.js";
|
||||
|
||||
describe("extractToolErrorMessage", () => {
|
||||
it("ignores non-error status values", () => {
|
||||
expect(extractToolErrorMessage({ details: { status: "0" } })).toBeUndefined();
|
||||
expect(extractToolErrorMessage({ details: { status: "completed" } })).toBeUndefined();
|
||||
expect(extractToolErrorMessage({ details: { status: "ok" } })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps error-like status values", () => {
|
||||
expect(extractToolErrorMessage({ details: { status: "failed" } })).toBe("failed");
|
||||
expect(extractToolErrorMessage({ details: { status: "timeout" } })).toBe("timeout");
|
||||
});
|
||||
});
|
||||
|
|
@ -29,6 +29,23 @@ function normalizeToolErrorText(text: string): string | undefined {
|
|||
: firstLine;
|
||||
}
|
||||
|
||||
function isErrorLikeStatus(status: string): boolean {
|
||||
const normalized = status.trim().toLowerCase();
|
||||
if (!normalized) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
normalized === "0" ||
|
||||
normalized === "ok" ||
|
||||
normalized === "success" ||
|
||||
normalized === "completed" ||
|
||||
normalized === "running"
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return /error|fail|timeout|timed[_\s-]?out|denied|cancel|invalid|forbidden/.test(normalized);
|
||||
}
|
||||
|
||||
function readErrorCandidate(value: unknown): string | undefined {
|
||||
if (typeof value === "string") {
|
||||
return normalizeToolErrorText(value);
|
||||
|
|
@ -59,7 +76,10 @@ function extractErrorField(value: unknown): string | undefined {
|
|||
return direct;
|
||||
}
|
||||
const status = typeof record.status === "string" ? record.status.trim() : "";
|
||||
return status ? normalizeToolErrorText(status) : undefined;
|
||||
if (!status || !isErrorLikeStatus(status)) {
|
||||
return undefined;
|
||||
}
|
||||
return normalizeToolErrorText(status);
|
||||
}
|
||||
|
||||
export function sanitizeToolResult(result: unknown): unknown {
|
||||
|
|
|
|||
Loading…
Reference in New Issue