fix(code-mode): avoid repair mode for invalid nested calls (#130478)

* fix(code-mode): restore nested input validation

* fix(code-mode): trust exact read-only dispatches

* test(code-mode): prove nested recovery boundaries

* test(code-mode): split headless validation coverage

* fix(code-mode): preserve guest network errors during validation

* fix(code-mode): preserve wrapped network errors

* fix(code-mode): keep side-effecting plugins restricted

---------

Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com>
Co-authored-by: roboclaw-bot <309084314+roboclaw-bot@users.noreply.github.com>
This commit is contained in:
Vito Cappello
2026-08-26 23:22:01 -04:00
committed by GitHub
parent b9840aeefa
commit fc7031baae
12 changed files with 202 additions and 40 deletions
+1 -1
View File
@@ -2072,7 +2072,7 @@ src/agents/tool-result-error.ts 3
src/agents/tool-schema-hints.ts 1
src/agents/tool-search-catalog.ts 6
src/agents/tool-search-directory.ts 1
src/agents/tool-search-runtime.ts 6
src/agents/tool-search-runtime.ts 4
src/agents/tool-search-transcript.ts 3
src/agents/tool-search.ts 1
src/agents/tools-effective-inventory-build.ts 2
@@ -72,6 +72,7 @@ import {
BEFORE_TOOL_CALL_SOURCE_TOOL,
BEFORE_TOOL_CALL_WRAPPED,
clearBeforeToolCallWrappedMarker,
getBeforeToolCallDiagnosticOptions,
getBeforeToolCallHookContext,
getBeforeToolCallSourceTool,
type BeforeToolCallDiagnosticOptions,
@@ -93,10 +94,6 @@ import {
} from "./tool-result-error.js";
import type { AnyAgentTool } from "./tools/common.js";
type BeforeToolCallWrapperOptions = {
approvalMode?: "request" | "report" | "deny";
emitDiagnostics: boolean;
};
type ForwardedToolExecution = (...args: unknown[]) => ReturnType<AnyAgentTool["execute"]>;
const MAX_TRACKED_ADJUSTED_PARAMS = 1024;
const INTERNAL_DISPOSED_RESULT = {
@@ -297,7 +294,7 @@ export function buildBlockedToolResult(params: {
export function wrapToolWithBeforeToolCallHook(
tool: AnyAgentTool,
ctx?: HookContext,
options: { approvalMode?: "request" | "report" | "deny"; emitDiagnostics?: boolean } = {},
options: Partial<BeforeToolCallDiagnosticOptions> = {},
): AnyAgentTool {
const execute = tool.execute;
if (!execute) {
@@ -305,8 +302,8 @@ export function wrapToolWithBeforeToolCallHook(
}
const toolName = tool.name || "tool";
const diagnosticIdentity = resolveToolDiagnosticIdentity(tool);
const hookOptions: BeforeToolCallWrapperOptions = {
...(options.approvalMode ? { approvalMode: options.approvalMode } : {}),
const hookOptions: BeforeToolCallDiagnosticOptions = {
...options,
emitDiagnostics: options.emitDiagnostics !== false,
};
const toolContentPolicy = resolveDiagnosticModelContentCapturePolicy(ctx?.config);
@@ -550,7 +547,8 @@ export function wrapToolWithBeforeToolCallHook(
? await invoke()
: await runWithGenericToolActionDecision(tool, toolCallId, invoke);
} catch (error) {
throw tool.resultContentSource === "network" &&
throw hookOptions.protectNetworkErrors !== false &&
tool.resultContentSource === "network" &&
getBeforeToolCallFailureDisposition(error) === undefined
? protectNetworkToolExecutionError(error, "Tool execution failed.", signal)
: error;
@@ -696,7 +694,7 @@ export function wrapToolWithBeforeToolCallHook(
enumerable: true,
});
Object.defineProperty(wrappedTool, BEFORE_TOOL_CALL_DIAGNOSTIC_OPTIONS, {
value: hookOptions satisfies BeforeToolCallDiagnosticOptions,
value: hookOptions,
enumerable: false,
});
Object.defineProperty(wrappedTool, BEFORE_TOOL_CALL_SOURCE_TOOL, {
@@ -714,12 +712,14 @@ export function wrapToolWithBeforeToolCallHook(
export function rewrapToolWithBeforeToolCallHook(
tool: AnyAgentTool,
ctx?: HookContext,
options: { approvalMode?: "request" | "report" | "deny"; emitDiagnostics?: boolean } = {},
options: Partial<BeforeToolCallDiagnosticOptions> = {},
): AnyAgentTool {
const preservedContext = getBeforeToolCallHookContext(tool);
const sourceTool = getBeforeToolCallSourceTool(tool) ?? tool;
const preservedOptions = getBeforeToolCallDiagnosticOptions(tool);
const wrapperOptions = { ...preservedOptions, ...options };
if (sourceTool === tool) {
return wrapToolWithBeforeToolCallHook(tool, ctx ?? preservedContext, options);
return wrapToolWithBeforeToolCallHook(tool, ctx ?? preservedContext, wrapperOptions);
}
// Preserve post-wrap schema/metadata while restoring the source execute function.
const rewrapSource: AnyAgentTool = {
@@ -729,7 +729,7 @@ export function rewrapToolWithBeforeToolCallHook(
clearBeforeToolCallWrappedMarker(rewrapSource);
copyBeforeToolCallWrapperMetadata(tool, rewrapSource);
copyAgentToolSourceExecutionGuard(tool, rewrapSource);
return wrapToolWithBeforeToolCallHook(rewrapSource, ctx ?? preservedContext, options);
return wrapToolWithBeforeToolCallHook(rewrapSource, ctx ?? preservedContext, wrapperOptions);
}
function recordPreExecutionBlockedToolCall(toolCallId?: string, runId?: string): void {
+8
View File
@@ -3,6 +3,8 @@ import type { AnyAgentTool } from "./tools/common.js";
export type BeforeToolCallDiagnosticOptions = {
emitDiagnostics: boolean;
protectNetworkErrors?: boolean;
approvalMode?: "request" | "report" | "deny";
};
export const BEFORE_TOOL_CALL_WRAPPED = Symbol("beforeToolCallWrapped");
@@ -46,6 +48,12 @@ export function setBeforeToolCallDiagnosticsEnabled(tool: AnyAgentTool, enabled:
}
}
export function getBeforeToolCallDiagnosticOptions(
tool: AnyAgentTool,
): BeforeToolCallDiagnosticOptions | undefined {
return withBeforeToolCallMetadata(tool)[BEFORE_TOOL_CALL_DIAGNOSTIC_OPTIONS];
}
/** Copy before_tool_call marker metadata when another wrapper replaces a tool. */
export function copyBeforeToolCallHookMarker(source: AnyAgentTool, target: AnyAgentTool): void {
if (!isToolWrappedWithBeforeToolCallHook(source)) {
+1
View File
@@ -84,6 +84,7 @@ export async function runCodeModeExec(params: {
}
const runtime = new ToolSearchRuntime(params.ctx, toToolSearchConfig(config), {
prepareInput: true,
validateInput: true,
});
params.onRuntime?.(runtime);
const bridgeDispatch = createCodeModeBridgeDispatchState();
+1
View File
@@ -220,6 +220,7 @@ export async function runCodeModeScriptHeadless(params: {
const codeModeRunId = `cm_headless_${randomUUID()}`;
const runtime = new ToolSearchRuntime(params.ctx, toToolSearchConfig(config), {
prepareInput: true,
validateInput: true,
});
const bridgeDispatch = createCodeModeBridgeDispatchState();
const namespaceCatalog = runtime.namespaceEntries();
@@ -0,0 +1,41 @@
import { expect, it, vi } from "vitest";
import { runCodeModeScriptHeadless } from "./code-mode.js";
import {
createToolSearchCatalogRef,
registerHeadlessToolSearchCatalog,
type ToolSearchToolContext,
} from "./tool-search.js";
import { jsonResult, type AnyAgentTool } from "./tools/common.js";
it("rejects schema-invalid nested input before headless tool execution", async () => {
const strict: AnyAgentTool = {
name: "headless_strict",
label: "headless_strict",
description: "Strict headless test tool",
parameters: {
type: "object",
properties: { value: { type: "string" } },
required: ["value"],
additionalProperties: false,
},
execute: vi.fn(async () => jsonResult({ unexpected: true })),
};
const config = { tools: { codeMode: { enabled: false, timeoutMs: 60_000 } } } as never;
const catalogRef = createToolSearchCatalogRef();
registerHeadlessToolSearchCatalog({ catalogRef, tools: [strict] });
const ctx: ToolSearchToolContext = { config, runtimeConfig: config, agentId: "main", catalogRef };
const result = await runCodeModeScriptHeadless({
ctx,
code: "return await headless_strict({ value: 42 });",
wallClockMs: 120_000,
});
expect(result.status).toBe("failed");
if (result.status !== "failed") {
throw new Error("expected headless Code Mode failure");
}
expect(result.error).toContain("value");
expect(result.toolCallCount).toBe(1);
expect(strict.execute).not.toHaveBeenCalled();
});
+7 -7
View File
@@ -325,10 +325,6 @@ function isPendingBridgeRequestReplaySafe(
return binding ? runtime.isReplaySafeExactId(binding.id) : false;
}
function isPendingBridgeRequestSideEffectFree(request: PendingBridgeRequest): boolean {
return request.method === "nodes" && (request.args[0] === "list" || request.args[0] === "get");
}
function enforceSnapshotStateLimits(params: {
snapshotBytes: Uint8Array;
config: CodeModeConfig;
@@ -367,10 +363,14 @@ export function createPendingBridgeStates(params: {
const target = params.catalogProjection.byCallableName.get(String(request.args[0]));
const yieldRunSignal = target?.name === "sessions_yield" ? params.ctx.abortSignal : undefined;
const tracksDispatch = request.method !== "sleep";
const sideEffectFree = isPendingBridgeRequestSideEffectFree(request);
// Exact catalog binding rejects shadowed or untrusted tools before replay-safety applies.
const recoverySafe =
(request.method === "nodes" && (request.args[0] === "list" || request.args[0] === "get")) ||
(request.method === "callValue" &&
isPendingBridgeRequestReplaySafe(request, params.runtime, params.catalogProjection));
if (tracksDispatch) {
params.bridgeDispatch.started = true;
if (!sideEffectFree) {
if (!recoverySafe) {
params.bridgeDispatch.potentiallyMutatingDispatches += 1;
}
}
@@ -398,7 +398,7 @@ export function createPendingBridgeStates(params: {
...request,
promise: completion.then((settled) => {
const trustedNoStart = tracksDispatch && consumeTrustedToolNoStartError(settled);
if (trustedNoStart && !sideEffectFree) {
if (trustedNoStart && !recoverySafe) {
params.bridgeDispatch.potentiallyMutatingDispatches = Math.max(
0,
params.bridgeDispatch.potentiallyMutatingDispatches - 1,
+92 -3
View File
@@ -4,10 +4,12 @@ import {
type Context,
type Model,
} from "openclaw/plugin-sdk/llm";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { setPluginToolMeta } from "../plugins/tools.js";
import { applyCodeModeCatalog } from "./code-mode.js";
import {
createCodeModeHarness,
fakeTool,
pluginToolWithExecute,
resetCodeModeTestState,
} from "./code-mode.test-support.js";
@@ -140,6 +142,93 @@ describe("Code Mode agent-loop error recovery", () => {
});
});
it("returns a schema-invalid nested call for ordinary recovery before execution", async () => {
const terminal = pluginToolWithExecute("terminal", "Open a terminal", async () =>
jsonResult({ unexpected: true }),
);
const recover = pluginToolWithExecute("recover_task", "Recover the task", async () =>
jsonResult({ recovered: true }),
);
const { agent, providerContexts, reconciliationCandidates } = await runCodeModeAgent({
hiddenTools: [terminal, recover],
programs: [
"return await terminal({ value: 42 });",
'return await recover_task({ value: "continue" });',
],
});
expect(providerContexts).toHaveLength(3);
expect(providerContexts[1]?.messages).toContainEqual(
expect.objectContaining({
role: "toolResult",
toolName: "exec",
isError: true,
details: expect.objectContaining({
status: "failed",
failurePhase: "bridge",
bridgeDispatchStarted: true,
error: expect.stringContaining("value"),
}),
}),
);
expect(terminal.execute).not.toHaveBeenCalled();
expect(recover.execute).toHaveBeenCalledOnce();
expect(reconciliationCandidates).toBe(0);
expect(agent.state.messages.at(-1)).toMatchObject({
role: "assistant",
content: [{ type: "text", text: "recovered" }],
});
});
it("returns an exact replay-safe post-dispatch failure for ordinary recovery", async () => {
const readOnly = fakeTool("sessions_history", "Read session history");
readOnly.execute = vi.fn(async () => {
throw new ToolInputError("read constraint rejected after dispatch");
}) as AnyAgentTool["execute"];
const recover = pluginToolWithExecute("recover_task", "Recover the task", async () =>
jsonResult({ recovered: true }),
);
const { providerContexts, reconciliationCandidates } = await runCodeModeAgent({
hiddenTools: [readOnly, recover],
programs: ["return await sessions_history({});", "return await recover_task({});"],
});
expect(providerContexts).toHaveLength(3);
expect(readOnly.execute).toHaveBeenCalledOnce();
expect(recover.execute).toHaveBeenCalledOnce();
expect(reconciliationCandidates).toBe(0);
});
it("keeps replay-safe side-effecting plugin failures in restricted reconciliation", async () => {
const appliedChanges: string[] = [];
const mutation = pluginToolWithExecute("plugin_mutation", "Mutate plugin state", async () => {
appliedChanges.push("plugin state changed");
throw new ToolInputError("plugin rejected input after mutation");
});
setPluginToolMeta(mutation, {
pluginId: "side-effecting-replay-safe-test",
optional: false,
replaySafe: true,
sideEffecting: true,
});
const recover = pluginToolWithExecute("recover_task", "Recover the task", async () =>
jsonResult({ recovered: true }),
);
const { providerContexts, reconciliationCandidates } = await runCodeModeAgent({
hiddenTools: [mutation, recover],
programs: ["return await plugin_mutation({});", "return await recover_task({});"],
});
expect(providerContexts).toHaveLength(1);
expect(mutation.execute).toHaveBeenCalledOnce();
expect(recover.execute).not.toHaveBeenCalled();
expect(reconciliationCandidates).toBe(1);
expect(appliedChanges).toEqual(["plugin state changed"]);
});
it("lets the model correct successive JavaScript syntax and runtime errors", async () => {
const complete = pluginToolWithExecute("complete_task", "Complete the task", async () =>
jsonResult({ completed: true }),
@@ -194,11 +283,11 @@ describe("Code Mode agent-loop error recovery", () => {
expect(reconciliationCandidates).toBe(1);
});
it("routes a partially applied mutation to restricted reconciliation without replay", async () => {
it("routes a partially applied mutation with an input error to restricted reconciliation", async () => {
const appliedChanges: string[] = [];
const applyPatch = pluginToolWithExecute("apply_patch", "Apply a patch", async () => {
appliedChanges.push("first hunk applied");
throw new Error("second hunk is ambiguous");
throw new ToolInputError("second hunk input is ambiguous after applying the first");
});
const write = pluginToolWithExecute("write", "Repeat a mutation", async () =>
jsonResult({ repeated: true }),
@@ -233,7 +233,7 @@ describe("Code Mode subscribed bridge lifecycle", () => {
),
);
expect(suspended.status).toBe("waiting");
expect(target.execute).toHaveBeenCalledOnce();
await vi.waitFor(() => expect(target.execute).toHaveBeenCalledOnce());
expect(countActiveToolExecutions(harness.runId)).toBe(1);
const parked = testing.activeRuns.get(suspended.runId as string);
+6
View File
@@ -492,6 +492,12 @@ describe("Code Mode guest execution", () => {
sessionKey: "agent:main:main",
runId: "run-code-mode",
catalogRef,
toolHookContext: {
agentId: "main",
sessionId: "session-code-mode",
sessionKey: "agent:main:main",
runId: "run-code-mode",
},
});
let result = await expectDefined(tools[0], "exec tool").execute("code-call-network-error", {
+24
View File
@@ -190,6 +190,30 @@ function wrapCatalogTool(tool: AnyAgentTool, hookContext?: HookContext): AnyAgen
return wrapToolWithBeforeToolCallHook(tool, hookContext);
}
export function prepareToolSearchCatalogExecutionTool(
entry: ToolSearchCatalogEntry,
options: { prepareInput?: boolean; validateInput?: boolean },
): CatalogTool {
const prepareInput =
options.prepareInput &&
entry.source === "openclaw" &&
"prepareBeforeToolCallParams" in entry.tool &&
typeof entry.tool.prepareBeforeToolCallParams === "function";
const validateInput = options.validateInput && entry.source === "openclaw";
if (!prepareInput && !validateInput) {
return entry.tool;
}
// SAFETY: both gates above restrict wrapper execution to OpenClaw-owned catalog tools.
const tool = entry.tool as AnyAgentTool;
const wrapperOptions = options.prepareInput ? { protectNetworkErrors: false } : undefined;
if (!isToolWrappedWithBeforeToolCallHook(tool)) {
return wrapToolWithBeforeToolCallHook(tool, undefined, wrapperOptions);
}
return wrapperOptions
? rewrapToolWithBeforeToolCallHook(tool, undefined, wrapperOptions)
: entry.tool;
}
function toCatalogEntry(
tool: CatalogTool,
sourceOverride?: CatalogSource,
+8 -16
View File
@@ -12,8 +12,6 @@ import { levenshteinDistance } from "../shared/levenshtein-distance.js";
import {
getBeforeToolCallFailureDisposition,
isPreExecutionBlockedToolResult,
isToolWrappedWithBeforeToolCallHook,
wrapToolWithBeforeToolCallHook,
} from "./agent-tools.before-tool-call.js";
import { runWithToolExecutionValidation } from "./agent-tools.execution-validation.js";
import { getChannelAgentToolMeta } from "./channel-tool-metadata.js";
@@ -25,6 +23,7 @@ import {
} from "./tool-result-error.js";
import {
compactToolSearchCatalogEntry,
prepareToolSearchCatalogExecutionTool,
resolveCatalog,
visibleCatalogEntries,
} from "./tool-search-catalog.js";
@@ -597,7 +596,9 @@ export class ToolSearchRuntime {
}
const pluginMeta = getPluginToolMeta(entry.tool as Parameters<typeof getPluginToolMeta>[0]);
if (pluginMeta) {
return pluginMeta.mcp ? false : pluginMeta.replaySafe === true;
return pluginMeta.mcp
? false
: pluginMeta.replaySafe === true && pluginMeta.sideEffecting !== true;
}
if (getChannelAgentToolMeta(entry.tool as never)) {
return false;
@@ -644,15 +645,7 @@ export class ToolSearchRuntime {
return snapshot;
};
const validateInput = this.options.validateInput && entry.source === "openclaw";
const prepareInput =
this.options.prepareInput &&
entry.source === "openclaw" &&
"prepareBeforeToolCallParams" in entry.tool &&
typeof entry.tool.prepareBeforeToolCallParams === "function";
const executionTool =
(prepareInput || validateInput) && !isToolWrappedWithBeforeToolCallHook(entry.tool as never)
? wrapToolWithBeforeToolCallHook(entry.tool as never)
: entry.tool;
const executionTool = prepareToolSearchCatalogExecutionTool(entry, this.options);
const runExecution = async () => {
const parentToolCallId = options?.parentToolCallId ?? toolCallId;
const signal = options?.signal ?? this.ctx.abortSignal;
@@ -707,11 +700,10 @@ export class ToolSearchRuntime {
)
: await runExecution();
const acceptedResult = await acceptResultBeforeProjection(result);
const parentToolCallId = options?.parentToolCallId;
if (parentToolCallId) {
if (options?.parentToolCallId) {
this.terminalTargetBatchByParent.set(
parentToolCallId,
this.terminalTargetBatchByParent.get(parentToolCallId) !== false &&
options.parentToolCallId,
this.terminalTargetBatchByParent.get(options.parentToolCallId) !== false &&
acceptedResult.terminate === true,
);
}