mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(code-mode): nested tool errors lose their message when details only flag the error (#159359)
* fix(code-mode): keep nested tool error text when details only flag the error
Code Mode projects a nested native tool result to its `details` value. A
plugin tool that reports failure as `isError: true` with details such as
`{ error: true }` carries its reason only in text content, so the guest and
the model saw `{"error":true}` and could not explain the failure (observed
with visitor-access `visitor_list` when the plugin runtime was not started).
ToolSearchRuntime now owns the guest-value projection for both `callValue`
and a new `callExactValue`; the bridge's inline duplicate is removed. For an
explicit `isError: true` result whose details carry no string `message` or
`error`, the projection adds the text content as `message` (or returns
`{ message }` when details are absent). Other results, including core tool
failures reported through details status, are unchanged.
* fix(code-mode): validate output contracts against raw tool details
Output-schema validation must check what the tool returned, so the plain details unwrap stays for assertCatalogOutputMatchesSchema while only the guest projection adds error text.
* fix(code-mode): replace blank error messages with tool error text
Once details carry no non-empty string message or error, an explicit isError result takes its message from the text content, including when details.message is blank or not a string.
This commit is contained in:
parent
40d84ae55f
commit
f7df40bf06
4 changed files with 81 additions and 17 deletions
|
|
@ -215,9 +215,12 @@ const hits = await search({ query: "OpenClaw code mode" });
|
|||
```
|
||||
|
||||
Calling a native global or native catalog handle returns the normal tool's JSON `details`
|
||||
value directly. MCP handles retain the native MCP result (`content`, optional
|
||||
`structuredContent`, and optional `isError`). Exact catalog ids and raw `{ tool, result }` envelopes are not
|
||||
guest-visible.
|
||||
value directly. When a tool marks its result `isError: true` and its details carry no
|
||||
non-empty `message` or `error` string, the value includes the tool's text content
|
||||
as `message`, so guest code and the model can see why the call failed. MCP handles
|
||||
retain the native MCP result (`content`, optional `structuredContent`, and
|
||||
optional `isError`). Exact catalog ids and raw `{ tool, result }` envelopes are
|
||||
not guest-visible.
|
||||
|
||||
The `ls`, `find`, and `grep` tools include their bounded listing or search text
|
||||
in `content`, including empty-result messages and truncation notices. Directory
|
||||
|
|
|
|||
|
|
@ -339,16 +339,12 @@ export async function runBridgeRequest(params: {
|
|||
yieldMs: Math.max(1, Math.min(1_000, Math.floor(params.remainingMs / 4))),
|
||||
};
|
||||
}
|
||||
const called = await params.runtime.callExactId(binding.id, input, {
|
||||
value = await params.runtime.callExactValue(binding.id, input, {
|
||||
recoverySurface: "catalog",
|
||||
parentToolCallId: params.parentToolCallId,
|
||||
signal: params.signal,
|
||||
onUpdate: params.onUpdate,
|
||||
});
|
||||
value =
|
||||
isRecord(called.result) && "details" in called.result
|
||||
? called.result.details
|
||||
: called.result;
|
||||
break;
|
||||
}
|
||||
case "nodes": {
|
||||
|
|
|
|||
|
|
@ -124,6 +124,36 @@ it("allows omitted native empty inputs but preserves required fields", async ()
|
|||
expect(required.execute).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("keeps an explicit tool error's text when its details carry no message", async () => {
|
||||
const h = createCodeModeHarness();
|
||||
const reason = "Start the Gateway with visitor-access enabled before managing visitors.";
|
||||
const errorTool = (name: string, text: string, details: unknown) =>
|
||||
pluginToolWithExecute(name, "Fail", async () => ({
|
||||
content: [{ type: "text" as const, text }],
|
||||
details,
|
||||
isError: true,
|
||||
}));
|
||||
const tools = [
|
||||
errorTool("flag_only", reason, { error: true }),
|
||||
errorTool("no_details", "Gateway unavailable.", undefined),
|
||||
errorTool("blank_message", "Visitor store is locked.", { error: true, message: " " }),
|
||||
errorTool("structured", "Rendered failure.", { status: "failed", error: "structured" }),
|
||||
];
|
||||
applyCodeModeCatalog({ ...h.ctx, tools: [...h.tools, ...tools] });
|
||||
const result = resultDetails(
|
||||
await expectDefined(h.tools[0], "exec").execute("tool-error-text", {
|
||||
code: "return [await flag_only(), await no_details(), await blank_message(), await structured()];",
|
||||
}),
|
||||
);
|
||||
expect(result.status, JSON.stringify(result)).toBe("completed");
|
||||
expect(result.value).toEqual([
|
||||
{ error: true, message: reason },
|
||||
{ message: "Gateway unavailable." },
|
||||
{ error: true, message: "Visitor store is locked." },
|
||||
{ status: "failed", error: "structured" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("merges actual root and multiple server files without skipping declaration errors", () => {
|
||||
const files = createMcpApiVirtualFiles(
|
||||
["alpha", "beta", "index"].map((identifier) => ({
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
} from "./agent-tools.before-tool-call.js";
|
||||
import { runWithToolExecutionValidation } from "./agent-tools.execution-validation.js";
|
||||
import { getChannelAgentToolMeta } from "./channel-tool-metadata.js";
|
||||
import { collectTextContentBlocks } from "./content-blocks.js";
|
||||
import { setMcpCodeModeGuestResultFromAgentResult } from "./mcp-content.js";
|
||||
import { captureAgentPluginRuntimeRefresh } from "./plugin-runtime-refresh.js";
|
||||
import type { AgentToolResult } from "./runtime/index.js";
|
||||
|
|
@ -65,6 +66,11 @@ import type {
|
|||
} from "./tool-search-types.js";
|
||||
import { textResult, ToolInputError } from "./tools/common.js";
|
||||
|
||||
type ToolSearchExactCallOptions = Pick<
|
||||
ToolSearchCallOptions,
|
||||
"parentToolCallId" | "signal" | "onUpdate" | "recoverySurface" | "mcpNamespaceGuest"
|
||||
>;
|
||||
|
||||
function describeEntry(entry: ToolSearchCatalogEntry) {
|
||||
return {
|
||||
...compactToolSearchCatalogEntry(entry),
|
||||
|
|
@ -412,14 +418,7 @@ export class ToolSearchRuntime {
|
|||
);
|
||||
};
|
||||
|
||||
callExactId = async (
|
||||
id: string,
|
||||
input?: unknown,
|
||||
options?: Pick<
|
||||
ToolSearchCallOptions,
|
||||
"parentToolCallId" | "signal" | "onUpdate" | "recoverySurface" | "mcpNamespaceGuest"
|
||||
>,
|
||||
) => {
|
||||
callExactId = async (id: string, input?: unknown, options?: ToolSearchExactCallOptions) => {
|
||||
const catalog = resolveCatalog(this.ctx);
|
||||
return await this.callEntry(
|
||||
findEntryByExactId(catalog, id, { ...options, codeModeSkills: this.ctx.codeModeSkills }),
|
||||
|
|
@ -429,7 +428,10 @@ export class ToolSearchRuntime {
|
|||
};
|
||||
|
||||
callValue = async (id: string, input?: unknown, options?: ToolSearchCallOptions) =>
|
||||
unwrapToolResultValue((await this.call(id, input, options)).result);
|
||||
projectToolResultValue((await this.call(id, input, options)).result);
|
||||
|
||||
callExactValue = async (id: string, input?: unknown, options?: ToolSearchExactCallOptions) =>
|
||||
projectToolResultValue((await this.callExactId(id, input, options)).result);
|
||||
|
||||
observeNetworkContent(parentToolCallId: string): void {
|
||||
const state = this.networkInvocations.get(parentToolCallId) ?? { active: 0, observed: false };
|
||||
|
|
@ -699,3 +701,36 @@ export function formatToolSearchControlError(
|
|||
function unwrapToolResultValue(result: AgentToolResult<unknown>): unknown {
|
||||
return isRecord(result) && "details" in result ? result.details : result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Project a target result into the value Code Mode guests receive. Output
|
||||
* contracts validate the raw details; only the guest value gains error text.
|
||||
*/
|
||||
function projectToolResultValue(result: AgentToolResult<unknown>): unknown {
|
||||
if (!isRecord(result) || !("details" in result)) {
|
||||
return result;
|
||||
}
|
||||
const { details } = result;
|
||||
// An explicit error can explain itself only in model-facing text; keep that reason.
|
||||
if (result.isError !== true || hasErrorMessage(details)) {
|
||||
return details;
|
||||
}
|
||||
const message = collectTextContentBlocks(result.content)
|
||||
.map((text) => text.trim())
|
||||
.filter(Boolean)
|
||||
.join("\n");
|
||||
if (!message) {
|
||||
return details;
|
||||
}
|
||||
if (details === undefined || details === null) {
|
||||
return { message };
|
||||
}
|
||||
return isRecord(details) ? { ...details, message } : details;
|
||||
}
|
||||
|
||||
function hasErrorMessage(details: unknown): boolean {
|
||||
return (
|
||||
isRecord(details) &&
|
||||
[details.message, details.error].some((value) => typeof value === "string" && value.trim())
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue