From bbee5467bcb9fa3807ce854c5d20bcfb98fdabcf Mon Sep 17 00:00:00 2001 From: Marvinthebored Date: Thu, 20 Aug 2026 18:27:46 +0800 Subject: [PATCH] fix(agents): run code-mode exec when a model sends a blank code alias (#126599) Treat blank code and command aliases as absent while preserving mismatch rejection when both aliases contain different instructions. Keep trusted hook and policy rewrites normalized at the Code Mode owner boundary. Co-authored-by: Marvinthebored Co-authored-by: Ayaan Zaidi --- docs/tools/code-mode.md | 3 +- ...s.before-tool-call.integration.e2e.test.ts | 108 ++++++++++++++++++ .../agent-tools.before-tool-call.policy.ts | 20 ++-- .../agent-tools.before-tool-call.wrapper.ts | 2 +- src/agents/code-mode-control-tools.ts | 34 +++--- src/agents/code-mode-runtime.ts | 17 ++- src/agents/code-mode.guest.test.ts | 46 ++++++++ 7 files changed, 191 insertions(+), 39 deletions(-) diff --git a/docs/tools/code-mode.md b/docs/tools/code-mode.md index 57f0ec660351..5293231887d4 100644 --- a/docs/tools/code-mode.md +++ b/docs/tools/code-mode.md @@ -418,7 +418,8 @@ Rules: - `code` is the documented model-facing field. - `command` is accepted as an exec-compatible alias for hook policies and trusted rewrites (the normal OpenClaw shell exec tool also uses a `command` - field); when both are present, the values must match. + field). Blank aliases are treated as absent; when both aliases are non-empty, + their values must match. - `language` defaults to `"javascript"`; the schema exposes it as a flat string enum (`"javascript" | "typescript"`), not a `oneOf`/`anyOf` union, since some providers reject those shapes. diff --git a/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts b/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts index 2c1c76d29093..54070bd0e540 100644 --- a/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts +++ b/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts @@ -961,6 +961,40 @@ describe("before_tool_call hook deduplication (#15502)", () => { }, ); + beforeToolCallHook.mockClear(); + const blankCodeAliasResult = await def.execute( + "call-code-mode-exec-blank-code", + { code: "", command: "return 3;" }, + undefined, + undefined, + extensionContext, + ); + + expect(blankCodeAliasResult.details).toMatchObject({ + status: "blocked", + reason: "blocked before code-mode execution", + }); + expect(beforeToolCallHook).toHaveBeenCalledWith( + { + toolName: "exec", + params: { code: "return 3;", command: "return 3;" }, + toolKind: "code_mode_exec", + toolInputKind: "javascript", + runId: "run-main", + toolCallId: "call-code-mode-exec-blank-code", + }, + { + toolName: "exec", + toolKind: "code_mode_exec", + toolInputKind: "javascript", + agentId: "main", + sessionKey: "agent:main:main", + sessionId: "session-main", + runId: "run-main", + toolCallId: "call-code-mode-exec-blank-code", + }, + ); + beforeToolCallHook.mockClear(); const typescriptResult = await def.execute( "call-code-mode-exec-typescript", @@ -1276,6 +1310,49 @@ describe("before_tool_call hook deduplication (#15502)", () => { }); }); + it("fails closed when a hook blanks one code-mode exec alias", async () => { + // A blank alias from the caller is treated as absent, but a hook that + // deliberately blanks `code` is a policy decision: mirror it so neither + // alias survives, rather than silently running the original command. + beforeToolCallHook = installBeforeToolCallHook({ + runBeforeToolCallImpl: async () => ({ params: { code: "" } }), + }); + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + const tool = markCodeModeControlTool( + asAgentTool({ + name: CODE_MODE_EXEC_TOOL_NAME, + execute, + description: "exec", + parameters: {}, + }), + ); + const [def] = toToolDefinitions([tool], { + agentId: "main", + sessionKey: "agent:main:main", + sessionId: "session-main", + runId: "run-main", + }); + if (!def) { + throw new Error("missing custom tool definition"); + } + const extensionContext = {} as Parameters[4]; + + await def.execute( + "call-code-mode-exec-blank-rewrite", + { code: "", command: "return 1;" }, + undefined, + undefined, + extensionContext, + ); + + expect(execute).toHaveBeenCalledWith( + "call-code-mode-exec-blank-rewrite", + { code: "", command: "" }, + undefined, + undefined, + ); + }); + it("renormalizes trusted policy rewrites before code-mode exec hooks observe params", async () => { resetGlobalHookRunner(); const normalHook = vi.fn(async () => undefined); @@ -1308,6 +1385,9 @@ describe("before_tool_call hook deduplication (#15502)", () => { }, }; } + if (eventValue.toolCallId === "call-code-mode-trusted-blank") { + return { params: { code: "", command: "return 4;" } }; + } return undefined; }, }, @@ -1360,6 +1440,13 @@ describe("before_tool_call hook deduplication (#15502)", () => { undefined, extensionContext, ); + await def.execute( + "call-code-mode-trusted-blank", + { code: "return 4;", command: "return 4;" }, + undefined, + undefined, + extensionContext, + ); expect(normalHook).toHaveBeenNthCalledWith( 1, @@ -1471,6 +1558,23 @@ describe("before_tool_call hook deduplication (#15502)", () => { undefined, undefined, ); + expect(normalHook).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ params: { code: "", command: "" } }), + expect.anything(), + ); + expect(trustedObserver).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ params: { code: "", command: "" } }), + expect.anything(), + ); + expect(execute).toHaveBeenNthCalledWith( + 3, + "call-code-mode-trusted-blank", + { code: "", command: "" }, + undefined, + undefined, + ); expect( consumeAdjustedParamsForToolCall("call-code-mode-trusted-command", "run-main"), ).toEqual({ command: "return 2;", code: "return 2;" }); @@ -1481,6 +1585,10 @@ describe("before_tool_call hook deduplication (#15502)", () => { command: "const value: number = 3;", language: "typescript", }); + expect(consumeAdjustedParamsForToolCall("call-code-mode-trusted-blank", "run-main")).toEqual({ + code: "", + command: "", + }); } finally { setActivePluginRegistry(createEmptyPluginRegistry()); resetGlobalHookRunner(); diff --git a/src/agents/agent-tools.before-tool-call.policy.ts b/src/agents/agent-tools.before-tool-call.policy.ts index c8a63d068ecc..5e2a8c29dfce 100644 --- a/src/agents/agent-tools.before-tool-call.policy.ts +++ b/src/agents/agent-tools.before-tool-call.policy.ts @@ -48,7 +48,7 @@ import type { } from "./agent-tools.before-tool-call.types.js"; import { getCodeModeExecBeforeHookMetadataForToolKind, - normalizeCodeModeExecBeforeHookParamsForToolKind, + reconcileCodeModeExecBeforeHookParams, } from "./code-mode-control-tools.js"; import { admitSingleToolCallLoop } from "./tool-loop-admission.js"; import { normalizeToolPolicyName } from "./tool-policy.js"; @@ -206,6 +206,9 @@ export async function runBeforeToolCallHook(args: { ...(args.ctx?.requester ? { requester: args.ctx.requester } : {}), }); const toolContext = buildToolContext(toolIdentity); + // Policies form a mutation chain. Reconcile each decision against the prior + // alias pair so an explicit blank rewrite remains fail-closed. + let trustedPolicyParams = normalizedParams; const trustedPolicyResult = shouldRunTrustedPolicies ? await runTrustedToolPolicies( { @@ -224,13 +227,16 @@ export async function runBeforeToolCallHook(args: { ...(args.ctx?.config ? { config: args.ctx.config } : {}), deriveEvent: deriveToolEventParams, normalizeEvent(eventValue) { - const normalizedEventParams = normalizeCodeModeExecBeforeHookParamsForToolKind({ - toolKind: eventValue.toolKind, - params: eventValue.params, + const normalizedEventParams = reconcileCodeModeExecBeforeHookParams({ + owner: { toolKind: eventValue.toolKind }, + originalParams: trustedPolicyParams, + hookParams: trustedPolicyParams, + adjustedParams: eventValue.params, }); if (!isPlainObject(normalizedEventParams)) { return undefined; } + trustedPolicyParams = normalizedEventParams; const normalizedEventIdentity = getCodeModeExecBeforeHookMetadataForToolKind({ toolKind: eventValue.toolKind, params: normalizedEventParams, @@ -277,11 +283,7 @@ export async function runBeforeToolCallHook(args: { trustedApprovalResolution = approvalOutcome.approvalResolution; } } - const rawPolicyAdjustedParams = trustedApprovalParams ?? trustedPolicyResult?.params ?? params; - const policyAdjustedParams = normalizeCodeModeExecBeforeHookParamsForToolKind({ - toolKind: args.toolKind, - params: rawPolicyAdjustedParams, - }); + const policyAdjustedParams = trustedApprovalParams ?? trustedPolicyResult?.params ?? params; const policyAdjustedToolIdentity = getCodeModeExecBeforeHookMetadataForToolKind({ toolKind: args.toolKind, diff --git a/src/agents/agent-tools.before-tool-call.wrapper.ts b/src/agents/agent-tools.before-tool-call.wrapper.ts index bca32e26ca76..ef6fdfdb90b8 100644 --- a/src/agents/agent-tools.before-tool-call.wrapper.ts +++ b/src/agents/agent-tools.before-tool-call.wrapper.ts @@ -126,7 +126,7 @@ export function finalizeBeforeToolCallExecutionParams(params: { finalizerMode: "adapter" | "wrapped"; }): unknown { const reconciledParams = reconcileCodeModeExecBeforeHookParams({ - tool: params.tool, + owner: { tool: params.tool }, originalParams: params.preparedParams, hookParams: params.hookParams, adjustedParams: params.adjustedParams, diff --git a/src/agents/code-mode-control-tools.ts b/src/agents/code-mode-control-tools.ts index 57a88c968eb8..947e0bf1c0b4 100644 --- a/src/agents/code-mode-control-tools.ts +++ b/src/agents/code-mode-control-tools.ts @@ -2,6 +2,7 @@ * Tags Code Mode exec/wait control tools and normalizes hook params for the * exec-compatible before-tool-call surface. */ +import { readNonBlankString } from "@openclaw/normalization-core/string-coerce"; import { isPlainObject } from "../utils.js"; import { normalizeToolPolicyName } from "./tool-policy.js"; import type { AnyAgentTool } from "./tools/common.js"; @@ -67,15 +68,15 @@ function normalizeCodeModeExecParams(params: unknown): unknown { if (!isPlainObject(params)) { return params; } - const code = params.code; - const command = params.command; - if (typeof code === "string" && typeof command !== "string") { + const code = readNonBlankString(params.code); + const command = readNonBlankString(params.command); + if (code !== undefined && command === undefined) { // Code-mode accepts both `code` and generic exec `command`; keep them paired // so downstream hooks can read either shape. - return { ...params, command: params.code }; + return { ...params, command: code }; } - if (typeof command === "string" && typeof code !== "string") { - return { ...params, code: params.command }; + if (command !== undefined && code === undefined) { + return { ...params, code: command }; } return params; } @@ -121,26 +122,21 @@ export function normalizeCodeModeExecBeforeHookParams(params: { return normalizeCodeModeExecParams(params.params); } -/** Normalize before-hook params when only the code-mode tool kind is available. */ -export function normalizeCodeModeExecBeforeHookParamsForToolKind(params: { - toolKind: unknown; - params: unknown; -}): unknown { - if (params.toolKind !== CODE_MODE_EXEC_TOOL_KIND) { - return params.params; - } - return normalizeCodeModeExecParams(params.params); -} +type CodeModeExecReconcileOwner = { tool: AnyAgentTool } | { toolKind: unknown }; -/** Reconcile hook-adjusted `code` and `command` fields after code-mode normalization. */ +/** Reconcile policy- or hook-adjusted aliases after raw-input normalization. */ export function reconcileCodeModeExecBeforeHookParams(params: { - tool: AnyAgentTool; + owner: CodeModeExecReconcileOwner; originalParams: unknown; hookParams: unknown; adjustedParams: unknown; }): unknown { + const isCodeModeExecOwner = + "tool" in params.owner + ? isCodeModeExecTool(params.owner.tool) + : params.owner.toolKind === CODE_MODE_EXEC_TOOL_KIND; if ( - !isCodeModeExecTool(params.tool) || + !isCodeModeExecOwner || !isPlainObject(params.originalParams) || !isPlainObject(params.hookParams) || !isPlainObject(params.adjustedParams) diff --git a/src/agents/code-mode-runtime.ts b/src/agents/code-mode-runtime.ts index eb04a55cd11d..ce02cadf52f5 100644 --- a/src/agents/code-mode-runtime.ts +++ b/src/agents/code-mode-runtime.ts @@ -1,4 +1,5 @@ import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { readNonBlankString } from "@openclaw/normalization-core/string-coerce"; import { uniqueValues } from "@openclaw/normalization-core/string-normalization"; import { parse, tokenizer } from "acorn"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -282,17 +283,15 @@ export function readCode(args: unknown): { restartSafe: boolean; } { const params = asToolParamsRecord(args); - const codeParam = params.code; - const commandParam = params.command; - if ( - typeof codeParam === "string" && - typeof commandParam === "string" && - codeParam !== commandParam - ) { + // Full-schema tool calls can materialize an unused alias as blank. + // Only nonblank aliases participate in divergence checks. + const codeAlias = readNonBlankString(params.code); + const commandAlias = readNonBlankString(params.command); + if (codeAlias !== undefined && commandAlias !== undefined && codeAlias !== commandAlias) { throw new ToolInputError("code and command must match when both are provided."); } - const code = typeof commandParam === "string" ? commandParam : codeParam; - if (typeof code !== "string" || !code.trim()) { + const code = commandAlias ?? codeAlias; + if (code === undefined) { throw new ToolInputError("code or command must be a non-empty string."); } const language = params.language; diff --git a/src/agents/code-mode.guest.test.ts b/src/agents/code-mode.guest.test.ts index 3ed135c981f7..940c9e444b56 100644 --- a/src/agents/code-mode.guest.test.ts +++ b/src/agents/code-mode.guest.test.ts @@ -65,6 +65,52 @@ describe("Code Mode guest execution", () => { ).rejects.toThrow("code and command must match when both are provided"); }); + it.each([ + { alias: "blank code", args: { code: "", command: "return 7;" } }, + { alias: "whitespace code", args: { code: " ", command: "return 7;" } }, + { alias: "blank command", args: { code: "return 7;", command: "" } }, + { alias: "whitespace command", args: { code: "return 7;", command: " \n " } }, + ])("runs the populated alias when the other is $alias", async ({ args }) => { + const { config, catalogRef, tools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...tools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + const result = resultDetails( + await expectDefined(tools[0], "tools[0] test invariant").execute( + "code-call-blank-alias", + args, + ), + ); + + expect(result.status).toBe("completed"); + expect(result.value).toBe(7); + }); + + it("still rejects when both aliases are blank", async () => { + const { config, catalogRef, tools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...tools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + await expect( + expectDefined(tools[0], "tools[0] test invariant").execute("code-call-blank-both", { + code: "", + command: " ", + }), + ).rejects.toThrow("code or command must be a non-empty string"); + }); + it.each([ { code: "ls -la /workspace/" }, { code: "ls -1" },