diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 3be65b3d67f0..6d27d0ded7e8 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -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 diff --git a/src/agents/agent-tools.before-tool-call.wrapper.ts b/src/agents/agent-tools.before-tool-call.wrapper.ts index 3dc252326cd9..d8c21867c758 100644 --- a/src/agents/agent-tools.before-tool-call.wrapper.ts +++ b/src/agents/agent-tools.before-tool-call.wrapper.ts @@ -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; 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 = {}, ): 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 = {}, ): 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 { diff --git a/src/agents/before-tool-call-metadata.ts b/src/agents/before-tool-call-metadata.ts index 08978c7db286..4b9706a9c8f0 100644 --- a/src/agents/before-tool-call-metadata.ts +++ b/src/agents/before-tool-call-metadata.ts @@ -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)) { diff --git a/src/agents/code-mode-execution.ts b/src/agents/code-mode-execution.ts index 54b7842604c9..28e9bfc2506f 100644 --- a/src/agents/code-mode-execution.ts +++ b/src/agents/code-mode-execution.ts @@ -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(); diff --git a/src/agents/code-mode-headless.ts b/src/agents/code-mode-headless.ts index 0cf814740a7f..c391d6575012 100644 --- a/src/agents/code-mode-headless.ts +++ b/src/agents/code-mode-headless.ts @@ -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(); diff --git a/src/agents/code-mode-headless.validation.test.ts b/src/agents/code-mode-headless.validation.test.ts new file mode 100644 index 000000000000..dbc1a30fb4bc --- /dev/null +++ b/src/agents/code-mode-headless.validation.test.ts @@ -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(); +}); diff --git a/src/agents/code-mode-state.ts b/src/agents/code-mode-state.ts index 8d687701fdc0..d0aedab8fc3f 100644 --- a/src/agents/code-mode-state.ts +++ b/src/agents/code-mode-state.ts @@ -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, diff --git a/src/agents/code-mode.agent-loop.test.ts b/src/agents/code-mode.agent-loop.test.ts index 1c7666d4e644..7694bf9b178b 100644 --- a/src/agents/code-mode.agent-loop.test.ts +++ b/src/agents/code-mode.agent-loop.test.ts @@ -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 }), diff --git a/src/agents/code-mode.bridge.lifecycle.test.ts b/src/agents/code-mode.bridge.lifecycle.test.ts index 22facc74e8fb..1bb947d6a2f6 100644 --- a/src/agents/code-mode.bridge.lifecycle.test.ts +++ b/src/agents/code-mode.bridge.lifecycle.test.ts @@ -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); diff --git a/src/agents/code-mode.guest.test.ts b/src/agents/code-mode.guest.test.ts index c2520877794a..50184de6c3a5 100644 --- a/src/agents/code-mode.guest.test.ts +++ b/src/agents/code-mode.guest.test.ts @@ -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", { diff --git a/src/agents/tool-search-catalog.ts b/src/agents/tool-search-catalog.ts index 21cf57482ea7..289f40040952 100644 --- a/src/agents/tool-search-catalog.ts +++ b/src/agents/tool-search-catalog.ts @@ -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, diff --git a/src/agents/tool-search-runtime.ts b/src/agents/tool-search-runtime.ts index c0df54b31453..7a5a974cb3de 100644 --- a/src/agents/tool-search-runtime.ts +++ b/src/agents/tool-search-runtime.ts @@ -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[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, ); }