mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:20:43 +00:00
fix: tighten trusted tool media passthrough (#67303)
* fix: tighten trusted tool media passthrough * changelog: tighten trusted tool media passthrough (#67303) * address review: thread rawToolName into emitToolResultOutput and keep plugin-tool media passthrough - Pass rawToolName through emitToolResultOutput params so the emit and collect calls no longer reference an out-of-scope identifier (ReferenceError on any verbose tool-output path). - Widen builtinToolNames to all effective tool raw names for this run (core + bundled/trusted plugin tools), so plugin tools on the trusted media list still receive local MEDIA: passthrough. Admission-time client-tool conflict check keeps using the core-only set so unrelated plugin names do not spuriously reject client definitions; MEDIA passthrough is still gated by the raw-name set, so a client tool that normalize-collides with a plugin name cannot inherit its media trust. - Add unit coverage for bundled-plugin raw-name passthrough and for case-variant plugin-name collisions. * drop redundant String() casts flagged by oxlint no-useless-cast The names from effectiveTools, client tool function names, and the existingToolNames iterable are already typed as string, so wrapping them in String(...) adds nothing and trips oxlint's no-useless-cast rule.
This commit is contained in:
@@ -8,6 +8,8 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Gateway/tools: anchor trusted local `MEDIA:` tool-result passthrough on the exact raw name of this run's registered built-in tools, and reject client tool definitions whose names normalize-collide with a built-in or with another client tool in the same request (`400 invalid_request_error` on both JSON and SSE paths), so a client-supplied tool named like a built-in can no longer inherit its local-media trust. (#67303)
|
||||
|
||||
## 2026.4.15-beta.1
|
||||
|
||||
### Changes
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
resolveProviderTextTransforms,
|
||||
transformProviderSystemPrompt,
|
||||
} from "../../../plugins/provider-runtime.js";
|
||||
import { getPluginToolMeta } from "../../../plugins/tools.js";
|
||||
import { isSubagentSessionKey } from "../../../routing/session-key.js";
|
||||
import { normalizeOptionalLowercaseString } from "../../../shared/string-coerce.js";
|
||||
import { normalizeOptionalString } from "../../../shared/string-coerce.js";
|
||||
@@ -86,7 +87,11 @@ import {
|
||||
import { subscribeEmbeddedPiSession } from "../../pi-embedded-subscribe.js";
|
||||
import { createPreparedEmbeddedPiSettingsManager } from "../../pi-project-settings.js";
|
||||
import { applyPiAutoCompactionGuard } from "../../pi-settings.js";
|
||||
import { toClientToolDefinitions } from "../../pi-tool-definition-adapter.js";
|
||||
import {
|
||||
createClientToolNameConflictError,
|
||||
findClientToolNameConflicts,
|
||||
toClientToolDefinitions,
|
||||
} from "../../pi-tool-definition-adapter.js";
|
||||
import { createOpenClawCodingTools, resolveToolLoopDetectionConfig } from "../../pi-tools.js";
|
||||
import { wrapStreamFnTextTransforms } from "../../plugin-text-transforms.js";
|
||||
import { describeProviderRequestRoutingSummary } from "../../provider-attribution.js";
|
||||
@@ -962,6 +967,37 @@ export async function runEmbeddedAttempt(
|
||||
cfg: params.config,
|
||||
agentId: sessionAgentId,
|
||||
});
|
||||
// Exact raw names of every tool registered for this run, including
|
||||
// bundled/plugin tools. Used as the raw-name set for the trusted local
|
||||
// MEDIA: passthrough gate: a normalized alias is not sufficient — the
|
||||
// emitted tool name must match an exact registration of this run.
|
||||
const builtinToolNames = new Set(
|
||||
effectiveTools.flatMap((tool) => {
|
||||
const name = (tool.name ?? "").trim();
|
||||
return name ? [name] : [];
|
||||
}),
|
||||
);
|
||||
// Admission-time conflict check only against non-plugin core tools, to
|
||||
// preserve prior behavior where client tools may coexist with unrelated
|
||||
// plugin tool names. MEDIA passthrough is still gated by the raw-name
|
||||
// set above, so a client tool that normalize-collides with a plugin
|
||||
// tool cannot inherit the plugin's local-media trust.
|
||||
const coreBuiltinToolNames = new Set(
|
||||
effectiveTools.flatMap((tool) => {
|
||||
const name = (tool.name ?? "").trim();
|
||||
if (!name || getPluginToolMeta(tool)) {
|
||||
return [];
|
||||
}
|
||||
return [name];
|
||||
}),
|
||||
);
|
||||
const clientToolNameConflicts = findClientToolNameConflicts({
|
||||
tools: clientTools ?? [],
|
||||
existingToolNames: coreBuiltinToolNames,
|
||||
});
|
||||
if (clientToolNameConflicts.length > 0) {
|
||||
throw createClientToolNameConflictError(clientToolNameConflicts);
|
||||
}
|
||||
const clientToolDefs = clientTools
|
||||
? toClientToolDefinitions(
|
||||
clientTools,
|
||||
@@ -1529,6 +1565,7 @@ export async function runEmbeddedAttempt(
|
||||
sessionKey: sandboxSessionKey,
|
||||
sessionId: params.sessionId,
|
||||
agentId: sessionAgentId,
|
||||
builtinToolNames,
|
||||
internalEvents: params.internalEvents,
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -10,6 +10,7 @@ function createMockContext(overrides?: {
|
||||
shouldEmitToolOutput?: boolean;
|
||||
onToolResult?: ReturnType<typeof vi.fn>;
|
||||
toolResultFormat?: "markdown" | "plain";
|
||||
builtinToolNames?: ReadonlySet<string>;
|
||||
}): EmbeddedPiSubscribeContext {
|
||||
const onToolResult = overrides?.onToolResult ?? vi.fn();
|
||||
return {
|
||||
@@ -39,6 +40,7 @@ function createMockContext(overrides?: {
|
||||
deterministicApprovalPromptSent: false,
|
||||
},
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
builtinToolNames: overrides?.builtinToolNames,
|
||||
shouldEmitToolResult: vi.fn(() => false),
|
||||
shouldEmitToolOutput: vi.fn(() => overrides?.shouldEmitToolOutput ?? false),
|
||||
emitToolSummary: vi.fn(),
|
||||
@@ -173,6 +175,46 @@ describe("handleToolExecutionEnd media emission", () => {
|
||||
expect(ctx.state.pendingToolMediaUrls).toEqual([]);
|
||||
});
|
||||
|
||||
it("does NOT emit local media for case-variant collisions with trusted built-ins", async () => {
|
||||
const ctx = createMockContext({
|
||||
shouldEmitToolOutput: false,
|
||||
onToolResult: vi.fn(),
|
||||
builtinToolNames: new Set(["web_search"]),
|
||||
});
|
||||
|
||||
await handleToolExecutionEnd(ctx, {
|
||||
type: "tool_execution_end",
|
||||
toolName: "Web_Search",
|
||||
toolCallId: "tc-1",
|
||||
isError: false,
|
||||
result: {
|
||||
content: [{ type: "text", text: "MEDIA:/tmp/secret.png" }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(ctx.state.pendingToolMediaUrls).toEqual([]);
|
||||
});
|
||||
|
||||
it("still emits remote media for case-variant collisions with trusted built-ins", async () => {
|
||||
const ctx = createMockContext({
|
||||
shouldEmitToolOutput: false,
|
||||
onToolResult: vi.fn(),
|
||||
builtinToolNames: new Set(["web_search"]),
|
||||
});
|
||||
|
||||
await handleToolExecutionEnd(ctx, {
|
||||
type: "tool_execution_end",
|
||||
toolName: "Web_Search",
|
||||
toolCallId: "tc-1",
|
||||
isError: false,
|
||||
result: {
|
||||
content: [{ type: "text", text: "MEDIA:https://example.com/file.png" }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(ctx.state.pendingToolMediaUrls).toEqual(["https://example.com/file.png"]);
|
||||
});
|
||||
|
||||
it("emits remote media for MCP-provenance results", async () => {
|
||||
const onToolResult = vi.fn();
|
||||
const ctx = createMockContext({ shouldEmitToolOutput: false, onToolResult });
|
||||
|
||||
@@ -433,12 +433,13 @@ function readExecApprovalUnavailableDetails(result: unknown): {
|
||||
async function emitToolResultOutput(params: {
|
||||
ctx: ToolHandlerContext;
|
||||
toolName: string;
|
||||
rawToolName: string;
|
||||
meta?: string;
|
||||
isToolError: boolean;
|
||||
result: unknown;
|
||||
sanitizedResult: unknown;
|
||||
}) {
|
||||
const { ctx, toolName, meta, isToolError, result, sanitizedResult } = params;
|
||||
const { ctx, toolName, rawToolName, meta, isToolError, result, sanitizedResult } = params;
|
||||
const hasStructuredMedia =
|
||||
result &&
|
||||
typeof result === "object" &&
|
||||
@@ -511,10 +512,10 @@ async function emitToolResultOutput(params: {
|
||||
ctx.shouldEmitToolOutput() || shouldEmitCompactToolOutput({ toolName, result, outputText });
|
||||
if (shouldEmitOutput) {
|
||||
if (outputText) {
|
||||
ctx.emitToolOutput(toolName, meta, outputText, result);
|
||||
ctx.emitToolOutput(rawToolName, meta, outputText, result);
|
||||
if (ctx.params.toolResultFormat === "plain") {
|
||||
emittedToolOutputMediaUrls = await collectEmittedToolOutputMediaUrls(
|
||||
toolName,
|
||||
rawToolName,
|
||||
outputText,
|
||||
result,
|
||||
);
|
||||
@@ -533,7 +534,12 @@ async function emitToolResultOutput(params: {
|
||||
if (!mediaReply) {
|
||||
return;
|
||||
}
|
||||
const mediaUrls = filterToolResultMediaUrls(toolName, mediaReply.mediaUrls, result);
|
||||
const mediaUrls = filterToolResultMediaUrls(
|
||||
rawToolName,
|
||||
mediaReply.mediaUrls,
|
||||
result,
|
||||
ctx.builtinToolNames,
|
||||
);
|
||||
const pendingMediaUrls =
|
||||
mediaReply.audioAsVoice || emittedToolOutputMediaUrls.length === 0
|
||||
? mediaUrls
|
||||
@@ -779,7 +785,8 @@ export async function handleToolExecutionEnd(
|
||||
result?: unknown;
|
||||
},
|
||||
) {
|
||||
const toolName = normalizeToolName(evt.toolName);
|
||||
const rawToolName = evt.toolName;
|
||||
const toolName = normalizeToolName(rawToolName);
|
||||
const toolCallId = evt.toolCallId;
|
||||
const runId = ctx.params.runId;
|
||||
const isError = evt.isError;
|
||||
@@ -1099,7 +1106,15 @@ export async function handleToolExecutionEnd(
|
||||
`embedded run tool end: runId=${ctx.params.runId} tool=${toolName} toolCallId=${toolCallId}`,
|
||||
);
|
||||
|
||||
await emitToolResultOutput({ ctx, toolName, meta, isToolError, result, sanitizedResult });
|
||||
await emitToolResultOutput({
|
||||
ctx,
|
||||
toolName,
|
||||
rawToolName,
|
||||
meta,
|
||||
isToolError,
|
||||
result,
|
||||
sanitizedResult,
|
||||
});
|
||||
|
||||
// Run after_tool_call plugin hook (fire-and-forget)
|
||||
const hookRunnerAfter = ctx.hookRunner ?? (await loadHookRunnerGlobal()).getGlobalHookRunner();
|
||||
|
||||
@@ -93,6 +93,7 @@ export type EmbeddedPiSubscribeContext = {
|
||||
blockChunking?: BlockReplyChunking;
|
||||
blockChunker: EmbeddedBlockChunker | null;
|
||||
hookRunner?: HookRunner;
|
||||
builtinToolNames?: ReadonlySet<string>;
|
||||
noteLastAssistant: (msg: AgentMessage) => void;
|
||||
|
||||
shouldEmitToolResult: () => boolean;
|
||||
@@ -179,6 +180,7 @@ export type ToolHandlerContext = {
|
||||
state: ToolHandlerState;
|
||||
log: EmbeddedSubscribeLogger;
|
||||
hookRunner?: HookRunner;
|
||||
builtinToolNames?: ReadonlySet<string>;
|
||||
flushBlockReplyBuffer: () => void | Promise<void>;
|
||||
shouldEmitToolResult: () => boolean;
|
||||
shouldEmitToolOutput: () => boolean;
|
||||
|
||||
@@ -209,6 +209,35 @@ describe("subscribeEmbeddedPiSession", () => {
|
||||
expect(onPartialReply).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks local MEDIA urls from case-variant tool names in verbose output", async () => {
|
||||
const onToolResult = vi.fn();
|
||||
const { emit } = createSubscribedHarness({
|
||||
runId: "run",
|
||||
onToolResult,
|
||||
verboseLevel: "full",
|
||||
builtinToolNames: new Set(["web_search"]),
|
||||
});
|
||||
|
||||
emitToolRun({
|
||||
emit,
|
||||
toolName: "Web_Search",
|
||||
toolCallId: "tool-1",
|
||||
isError: false,
|
||||
result: {
|
||||
content: [{ type: "text", text: "Fetched page\nMEDIA:/tmp/secret.png" }],
|
||||
},
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onToolResult).toHaveBeenCalled();
|
||||
});
|
||||
const payload = onToolResult.mock.calls.at(-1)?.[0] as
|
||||
| { text?: string; mediaUrls?: string[] }
|
||||
| undefined;
|
||||
expect(payload?.text ?? "").toContain("Fetched page");
|
||||
expect(payload?.mediaUrls).toBeUndefined();
|
||||
});
|
||||
|
||||
it("attaches media from internal completion events even when assistant omits MEDIA lines", async () => {
|
||||
const onBlockReply = vi.fn();
|
||||
const { emit } = createSubscribedHarness({
|
||||
|
||||
@@ -277,6 +277,67 @@ describe("extractToolResultMediaPaths", () => {
|
||||
expect(isToolResultMediaTrusted("music_generate")).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks trusted-media aliases that are not exact registered built-ins", () => {
|
||||
expect(filterToolResultMediaUrls("bash", ["/etc/passwd"], undefined, new Set(["exec"]))).toEqual(
|
||||
[],
|
||||
);
|
||||
expect(
|
||||
filterToolResultMediaUrls(
|
||||
"Web_Search",
|
||||
["/etc/passwd"],
|
||||
undefined,
|
||||
new Set(["web_search"]),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps local media for exact registered built-in tool names", () => {
|
||||
expect(
|
||||
filterToolResultMediaUrls(
|
||||
"web_search",
|
||||
["/tmp/screenshot.png"],
|
||||
undefined,
|
||||
new Set(["web_search"]),
|
||||
),
|
||||
).toEqual(["/tmp/screenshot.png"]);
|
||||
});
|
||||
|
||||
it("keeps local media for bundled plugin tool names registered in this run", () => {
|
||||
// music_generate is a bundled-plugin trusted tool; when the runner
|
||||
// registers it for this run, its raw name must be allowed through the
|
||||
// exact-name gate just like a core built-in.
|
||||
expect(
|
||||
filterToolResultMediaUrls(
|
||||
"music_generate",
|
||||
["/tmp/song.mp3"],
|
||||
undefined,
|
||||
new Set(["music_generate"]),
|
||||
),
|
||||
).toEqual(["/tmp/song.mp3"]);
|
||||
});
|
||||
|
||||
it("strips local media for plugin-name collisions when the plugin is not registered", () => {
|
||||
expect(
|
||||
filterToolResultMediaUrls(
|
||||
"Music_Generate",
|
||||
["/etc/passwd"],
|
||||
undefined,
|
||||
new Set(["music_generate"]),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("still allows remote media for colliding aliases", () => {
|
||||
expect(
|
||||
filterToolResultMediaUrls(
|
||||
"bash",
|
||||
["/etc/passwd", "https://example.com/file.png"],
|
||||
undefined,
|
||||
new Set(["exec"]),
|
||||
),
|
||||
).toEqual(["https://example.com/file.png"]);
|
||||
});
|
||||
|
||||
it("does not trust local MEDIA paths for MCP-provenance results", () => {
|
||||
expect(
|
||||
filterToolResultMediaUrls("browser", ["/tmp/screenshot.png"], {
|
||||
|
||||
@@ -211,11 +211,24 @@ export function filterToolResultMediaUrls(
|
||||
toolName: string | undefined,
|
||||
mediaUrls: string[],
|
||||
result?: unknown,
|
||||
builtinToolNames?: ReadonlySet<string>,
|
||||
): string[] {
|
||||
if (mediaUrls.length === 0) {
|
||||
return mediaUrls;
|
||||
}
|
||||
if (isToolResultMediaTrusted(toolName, result)) {
|
||||
// When the current run provides its exact registered tool names (core
|
||||
// built-ins plus bundled/trusted plugin tools), require the raw emitted
|
||||
// tool name to match one of them before allowing local MEDIA: paths.
|
||||
// This blocks normalized aliases and case-variant collisions such as
|
||||
// "Bash" -> "bash" or "Web_Search" -> "web_search" from inheriting a
|
||||
// registered tool's media trust.
|
||||
if (builtinToolNames !== undefined) {
|
||||
const registeredName = toolName?.trim();
|
||||
if (!registeredName || !builtinToolNames.has(registeredName)) {
|
||||
return mediaUrls.filter((url) => HTTP_URL_RE.test(url.trim()));
|
||||
}
|
||||
}
|
||||
return mediaUrls;
|
||||
}
|
||||
return mediaUrls.filter((url) => HTTP_URL_RE.test(url.trim()));
|
||||
|
||||
@@ -415,7 +415,12 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
||||
return;
|
||||
}
|
||||
const { text: cleanedText, mediaUrls } = parseReplyDirectives(message);
|
||||
const filteredMediaUrls = filterToolResultMediaUrls(toolName, mediaUrls ?? [], result);
|
||||
const filteredMediaUrls = filterToolResultMediaUrls(
|
||||
toolName,
|
||||
mediaUrls ?? [],
|
||||
result,
|
||||
params.builtinToolNames,
|
||||
);
|
||||
if (!cleanedText && filteredMediaUrls.length === 0) {
|
||||
return;
|
||||
}
|
||||
@@ -724,6 +729,7 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
||||
blockChunking,
|
||||
blockChunker,
|
||||
hookRunner: params.hookRunner,
|
||||
builtinToolNames: params.builtinToolNames,
|
||||
noteLastAssistant,
|
||||
shouldEmitToolResult,
|
||||
shouldEmitToolOutput,
|
||||
|
||||
@@ -39,5 +39,11 @@ export type SubscribeEmbeddedPiSessionParams = {
|
||||
sessionId?: string;
|
||||
/** Agent identity for hook context — resolved from session config in attempt.ts. */
|
||||
agentId?: string;
|
||||
/**
|
||||
* Exact raw names of non-plugin OpenClaw tools registered for this run.
|
||||
* When provided, MEDIA: passthrough requires an exact match instead of only
|
||||
* a normalized-name collision with a trusted built-in.
|
||||
*/
|
||||
builtinToolNames?: ReadonlySet<string>;
|
||||
internalEvents?: AgentInternalEvent[];
|
||||
};
|
||||
|
||||
@@ -2,7 +2,14 @@ import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { ClientToolDefinition } from "./pi-embedded-runner/run/params.js";
|
||||
import { toClientToolDefinitions, toToolDefinitions } from "./pi-tool-definition-adapter.js";
|
||||
import {
|
||||
CLIENT_TOOL_NAME_CONFLICT_PREFIX,
|
||||
createClientToolNameConflictError,
|
||||
findClientToolNameConflicts,
|
||||
isClientToolNameConflictError,
|
||||
toClientToolDefinitions,
|
||||
toToolDefinitions,
|
||||
} from "./pi-tool-definition-adapter.js";
|
||||
|
||||
type ToolExecute = ReturnType<typeof toToolDefinitions>[number]["execute"];
|
||||
const extensionContext = {} as Parameters<ToolExecute>[4];
|
||||
@@ -177,3 +184,29 @@ describe("toClientToolDefinitions – param coercion", () => {
|
||||
expect(calledWith).toEqual({ action: "search", params: { q: "test", page: 1 } });
|
||||
});
|
||||
});
|
||||
|
||||
describe("client tool name conflict checks", () => {
|
||||
it("detects collisions with existing built-in names after normalization", () => {
|
||||
expect(
|
||||
findClientToolNameConflicts({
|
||||
tools: [makeClientTool("Web_Search"), makeClientTool("exec")],
|
||||
existingToolNames: ["web_search", "read"],
|
||||
}),
|
||||
).toEqual(["Web_Search"]);
|
||||
});
|
||||
|
||||
it("detects duplicate client tool names after normalization", () => {
|
||||
expect(
|
||||
findClientToolNameConflicts({
|
||||
tools: [makeClientTool("Weather"), makeClientTool("weather")],
|
||||
}),
|
||||
).toEqual(["Weather", "weather"]);
|
||||
});
|
||||
|
||||
it("wraps conflict errors with a stable prefix", () => {
|
||||
const err = createClientToolNameConflictError(["exec", "Web_Search"]);
|
||||
expect(err.message).toBe(`${CLIENT_TOOL_NAME_CONFLICT_PREFIX} exec, Web_Search`);
|
||||
expect(isClientToolNameConflictError(err)).toBe(true);
|
||||
expect(isClientToolNameConflictError(new Error("other failure"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -169,6 +169,50 @@ function splitToolExecuteArgs(args: ToolExecuteArgsAny): {
|
||||
};
|
||||
}
|
||||
|
||||
export const CLIENT_TOOL_NAME_CONFLICT_PREFIX = "client tool name conflict:";
|
||||
|
||||
export function findClientToolNameConflicts(params: {
|
||||
tools: ClientToolDefinition[];
|
||||
existingToolNames?: Iterable<string>;
|
||||
}): string[] {
|
||||
const existingNormalized = new Set<string>();
|
||||
for (const name of params.existingToolNames ?? []) {
|
||||
const trimmed = name.trim();
|
||||
if (trimmed) {
|
||||
existingNormalized.add(normalizeToolName(trimmed));
|
||||
}
|
||||
}
|
||||
|
||||
const conflicts = new Set<string>();
|
||||
const seenClientNames = new Map<string, string>();
|
||||
for (const tool of params.tools) {
|
||||
const rawName = (tool.function?.name ?? "").trim();
|
||||
if (!rawName) {
|
||||
continue;
|
||||
}
|
||||
const normalizedName = normalizeToolName(rawName);
|
||||
if (existingNormalized.has(normalizedName)) {
|
||||
conflicts.add(rawName);
|
||||
}
|
||||
const priorClientName = seenClientNames.get(normalizedName);
|
||||
if (priorClientName) {
|
||||
conflicts.add(priorClientName);
|
||||
conflicts.add(rawName);
|
||||
continue;
|
||||
}
|
||||
seenClientNames.set(normalizedName, rawName);
|
||||
}
|
||||
return Array.from(conflicts);
|
||||
}
|
||||
|
||||
export function createClientToolNameConflictError(conflicts: string[]): Error {
|
||||
return new Error(`${CLIENT_TOOL_NAME_CONFLICT_PREFIX} ${conflicts.join(", ")}`);
|
||||
}
|
||||
|
||||
export function isClientToolNameConflictError(err: unknown): err is Error {
|
||||
return err instanceof Error && err.message.startsWith(CLIENT_TOOL_NAME_CONFLICT_PREFIX);
|
||||
}
|
||||
|
||||
export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
|
||||
return tools.map((tool) => {
|
||||
const name = tool.name || "tool";
|
||||
|
||||
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
|
||||
import http from "node:http";
|
||||
import path from "node:path";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createClientToolNameConflictError } from "../agents/pi-tool-definition-adapter.js";
|
||||
import { HISTORY_CONTEXT_MARKER } from "../auto-reply/reply/history.js";
|
||||
import { CURRENT_MESSAGE_MARKER } from "../auto-reply/reply/mentions.js";
|
||||
import { emitAgentEvent } from "../infra/agent-events.js";
|
||||
@@ -323,6 +324,21 @@ describe("OpenResponses HTTP API (e2e)", () => {
|
||||
expect(agentCommand).toHaveBeenCalledTimes(0);
|
||||
await ensureResponseConsumed(resInvalidOverride);
|
||||
|
||||
agentCommand.mockClear();
|
||||
agentCommand.mockRejectedValueOnce(createClientToolNameConflictError(["exec"]));
|
||||
const resToolConflict = await postResponses(port, {
|
||||
model: "openclaw",
|
||||
input: "hi",
|
||||
tools: WEATHER_TOOL,
|
||||
});
|
||||
expect(resToolConflict.status).toBe(400);
|
||||
const toolConflictJson = (await resToolConflict.json()) as {
|
||||
error?: { code?: string; message?: string };
|
||||
};
|
||||
expect(toolConflictJson.error?.code).toBe("invalid_request_error");
|
||||
expect(toolConflictJson.error?.message).toBe("invalid tool configuration");
|
||||
await ensureResponseConsumed(resToolConflict);
|
||||
|
||||
mockAgentOnce([{ text: "hello" }]);
|
||||
const resUser = await postResponses(port, {
|
||||
user: "alice",
|
||||
|
||||
@@ -10,6 +10,7 @@ import { createHash, randomUUID } from "node:crypto";
|
||||
import type { IncomingMessage, ServerResponse } from "node:http";
|
||||
import type { ImageContent } from "../agents/command/types.js";
|
||||
import type { ClientToolDefinition } from "../agents/pi-embedded-runner/run/params.js";
|
||||
import { isClientToolNameConflictError } from "../agents/pi-tool-definition-adapter.js";
|
||||
import { createDefaultDeps } from "../cli/deps.js";
|
||||
import type { CliDeps } from "../cli/deps.types.js";
|
||||
import { agentCommandFromIngress } from "../commands/agent.js";
|
||||
@@ -779,6 +780,17 @@ export async function handleOpenResponsesHttpRequest(
|
||||
return true;
|
||||
}
|
||||
logWarn(`openresponses: non-stream response failed: ${String(err)}`);
|
||||
if (isClientToolNameConflictError(err)) {
|
||||
const response = createResponseResource({
|
||||
id: responseId,
|
||||
model,
|
||||
status: "failed",
|
||||
output: [],
|
||||
error: { code: "invalid_request_error", message: "invalid tool configuration" },
|
||||
});
|
||||
sendJson(res, 400, response);
|
||||
return true;
|
||||
}
|
||||
const response = createResponseResource({
|
||||
id: responseId,
|
||||
model,
|
||||
@@ -1101,6 +1113,24 @@ export async function handleOpenResponsesHttpRequest(
|
||||
logWarn(`openresponses: streaming response failed: ${String(err)}`);
|
||||
|
||||
finalUsage = finalUsage ?? createEmptyUsage();
|
||||
if (isClientToolNameConflictError(err)) {
|
||||
const errorResponse = createResponseResource({
|
||||
id: responseId,
|
||||
model,
|
||||
status: "failed",
|
||||
output: [],
|
||||
error: { code: "invalid_request_error", message: "invalid tool configuration" },
|
||||
usage: finalUsage,
|
||||
});
|
||||
|
||||
writeSseEvent(res, { type: "response.failed", response: errorResponse });
|
||||
emitAgentEvent({
|
||||
runId: responseId,
|
||||
stream: "lifecycle",
|
||||
data: { phase: "error" },
|
||||
});
|
||||
return;
|
||||
}
|
||||
const errorResponse = createResponseResource({
|
||||
id: responseId,
|
||||
model,
|
||||
|
||||
Reference in New Issue
Block a user