mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(ai): preserve literal DSML tool arguments
## What Problem This Solves DSML parameter recovery loses the literal `__proto__` argument, causing a call containing only that argument to remain unrecovered. The same provider pipeline also repeats endpoint parsing, model-family checks, and tool-result text cleanup. ## User Impact Recovered DeepSeek tool calls retain declared argument names, including `__proto__`, with the same ordinary object shape and duplicate-name last-value behavior as JSON arguments. The helper cleanup preserves existing endpoint restrictions, request shaping, usage and failover behavior. ## Why This Change Was Made - Define recovered DSML parameters as own data properties, preserving their normal enumerability, writability and configurability. - Reuse the existing Gemini-family predicate and non-empty tool-result text sanitizer. - Replace three parse-only URL exception paths with `URL.parse`, preserving their existing route predicates. Measured production reduction: 23 lines (33 added, 56 removed). The regression adds 31 test lines; no dependency or public API is added. ## Fixes found along the way An ordinary object assignment invokes the inherited `__proto__` setter. Because DSML values are strings, that argument disappears; if it is the only argument, recovery rejects the call. The JSON fallback already preserves the key. The regression runs through the completions stream reducer and checks reserved and ordinary names plus duplicate-name overwrite behavior. ## Evidence - Original code: the `__proto__` regression failed (`stop` instead of `toolUse`); the other 14 tests passed. Fixed code: all 15 tests passed, preserving own enumerable properties, the ordinary prototype, and duplicate-name last-write behavior. - Full AI suite: 2,904 tests in 149 files passed. Both cycle checks report zero. All seven local changed files match the remotely tested SHA-256 digests. - Test cost: `pnpm test packages/ai/src/transports/openai-completions-dsml.test.ts --maxWorkers=1 --reporter=dot` took 1.94 seconds wall on Linux Testbox; Vitest reported 945 ms total. Original failing run took 2.88 seconds wall. CI timing is pending the hosted run. - Independent Codex review found no actionable P0-P2 issues. Full `check-changed` passed, including all core test-type groups, lint and boundary guards. The canonical SDK API comparison reports no API changes. - Tested candidate tree: `0eec1534eed2fab3567828267914e0c045e656cc`; published commit will contain that exact tree. Proof ran on Blacksmith Testbox `tbx_01m3tmabk0s6555s9b8k83rxkt`.
This commit is contained in:
parent
508e2069f8
commit
0f47f89d7d
7 changed files with 64 additions and 56 deletions
|
|
@ -15,6 +15,7 @@ import {
|
|||
extractToolResultText,
|
||||
} from "./providers/tool-result-text.js";
|
||||
import type { ResolvedOpenAICompletionsCompat } from "./transports/openai-completions-compat.js";
|
||||
import { sanitizeNonEmptyTransportPayloadText } from "./transports/transport-stream-shared.js";
|
||||
import type { Context, Model, ThinkingContent, ToolCall } from "./types.js";
|
||||
import { sanitizeSurrogates } from "./utils/sanitize-unicode.js";
|
||||
import {
|
||||
|
|
@ -23,17 +24,11 @@ import {
|
|||
stripSystemPromptRelocatableBoundary,
|
||||
} from "./utils/system-prompt-cache-boundary.js";
|
||||
|
||||
const EMPTY_TOOL_RESULT_TEXT = "(no output)";
|
||||
type ChatCompletionContentPartVideo = {
|
||||
type: "video_url";
|
||||
video_url: { url: string };
|
||||
};
|
||||
|
||||
function sanitizeToolResultText(text: string, fallback: string): string {
|
||||
const sanitized = sanitizeSurrogates(text);
|
||||
return sanitized.trim().length > 0 ? sanitized : fallback;
|
||||
}
|
||||
|
||||
/** Whether replayed messages require a tools marker for proxy compatibility. */
|
||||
export function hasToolCallHistory(messages: Context["messages"]): boolean {
|
||||
return messages.some(
|
||||
|
|
@ -252,10 +247,7 @@ export function convertMessages(
|
|||
const textResult = extractToolResultText(toolMsg.content);
|
||||
const mediaPlaceholder = describeToolResultMediaPlaceholder(toolMsg.content);
|
||||
const images = toolMsg.content.filter(isImageWithMediaPayload);
|
||||
const content = sanitizeToolResultText(
|
||||
textResult,
|
||||
mediaPlaceholder ?? EMPTY_TOOL_RESULT_TEXT,
|
||||
);
|
||||
const content = sanitizeNonEmptyTransportPayloadText(textResult, mediaPlaceholder);
|
||||
const toolResultMsg: ChatCompletionToolMessageParam = {
|
||||
role: "tool",
|
||||
content,
|
||||
|
|
|
|||
|
|
@ -94,28 +94,21 @@ export function isOpenAICodexResponsesModel(model: {
|
|||
|
||||
function isNativeOpenAICodexResponsesBaseUrl(baseUrl?: string): boolean {
|
||||
const trimmed = typeof baseUrl === "string" ? baseUrl.trim() : "";
|
||||
if (!trimmed) {
|
||||
const url = URL.parse(trimmed);
|
||||
if (!url || (url.protocol !== "http:" && url.protocol !== "https:")) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const url = new URL(trimmed);
|
||||
if (url.protocol !== "http:" && url.protocol !== "https:") {
|
||||
return false;
|
||||
}
|
||||
if (url.hostname.toLowerCase() !== "chatgpt.com") {
|
||||
return false;
|
||||
}
|
||||
const pathname = url.pathname.replace(/\/+$/u, "").toLowerCase();
|
||||
return [
|
||||
"/backend-api",
|
||||
"/backend-api/v1",
|
||||
"/backend-api/codex",
|
||||
"/backend-api/codex/v1",
|
||||
"/backend-api/codex/responses",
|
||||
].includes(pathname);
|
||||
} catch {
|
||||
if (url.hostname.toLowerCase() !== "chatgpt.com") {
|
||||
return false;
|
||||
}
|
||||
const pathname = url.pathname.replace(/\/+$/u, "").toLowerCase();
|
||||
return [
|
||||
"/backend-api",
|
||||
"/backend-api/v1",
|
||||
"/backend-api/codex",
|
||||
"/backend-api/codex/v1",
|
||||
"/backend-api/codex/responses",
|
||||
].includes(pathname);
|
||||
}
|
||||
|
||||
export function usesNativeOpenAICodexResponsesBackend(model: {
|
||||
|
|
|
|||
|
|
@ -273,6 +273,37 @@ describe("openai completions DSML", () => {
|
|||
expect(JSON.stringify(events)).not.toContain("DSML");
|
||||
});
|
||||
|
||||
it.each(["__proto__", "constructor", "text"])(
|
||||
"preserves literal DSML parameter name %s as an own scalar argument",
|
||||
async (name) => {
|
||||
const model = createDeepSeekCompletionsModel();
|
||||
const output = createAssistantOutput(model);
|
||||
const content =
|
||||
'<|DSML|tool_calls><|DSML|invoke name="echo">' +
|
||||
`<|DSML|parameter name="${name}" string="true">first value</|DSML|parameter>` +
|
||||
`<|DSML|parameter name="${name}" string="true">scalar value</|DSML|parameter>` +
|
||||
"</|DSML|invoke></|DSML|tool_calls>";
|
||||
|
||||
await processCompletionsStream(
|
||||
streamChunks([makeCompletionsChunk({ content }, "stop")]),
|
||||
output,
|
||||
model,
|
||||
{ push() {} },
|
||||
);
|
||||
|
||||
expect(output.stopReason).toBe("toolUse");
|
||||
expect(output.content).toHaveLength(1);
|
||||
const call = output.content.find((block) => block.type === "toolCall");
|
||||
expect(call).toMatchObject({ type: "toolCall", name: "echo" });
|
||||
expect(call?.arguments).toStrictEqual({ [name]: "scalar value" });
|
||||
expect(Object.getOwnPropertyDescriptor(call?.arguments, name)).toMatchObject({
|
||||
value: "scalar value",
|
||||
enumerable: true,
|
||||
});
|
||||
expect(Object.getPrototypeOf(call?.arguments)).toBe(Object.prototype);
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects an oversized DeepSeek DSML block when the crossing chunk contains its close", async () => {
|
||||
const model = createDeepSeekCompletionsModel();
|
||||
const output = createAssistantOutput(model);
|
||||
|
|
|
|||
|
|
@ -222,7 +222,12 @@ function parseDeepSeekDsmlInvokeArguments(body: string): Record<string, unknown>
|
|||
if (rawValue.length === 0) {
|
||||
continue;
|
||||
}
|
||||
args[name] = decodeDeepSeekDsmlText(rawValue);
|
||||
Object.defineProperty(args, name, {
|
||||
value: decodeDeepSeekDsmlText(rawValue),
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
}
|
||||
if (Object.keys(args).length > 0) {
|
||||
return args;
|
||||
|
|
|
|||
|
|
@ -70,11 +70,8 @@ function isKnownOpenAICompletionsEndpoint(model: Pick<Model, "baseUrl">): boolea
|
|||
if (endpointClass === "openai-public" || endpointClass === "azure-openai") {
|
||||
return true;
|
||||
}
|
||||
try {
|
||||
return isAzureOpenAICompatibleHost(new URL(model.baseUrl).hostname.toLowerCase());
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const endpoint = URL.parse(model.baseUrl);
|
||||
return endpoint !== null && isAzureOpenAICompatibleHost(endpoint.hostname.toLowerCase());
|
||||
}
|
||||
|
||||
function resolveOpenAICompletionsMaxTokens(
|
||||
|
|
|
|||
|
|
@ -1,9 +1,6 @@
|
|||
import type { Context, Model } from "@openclaw/llm-core";
|
||||
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
|
||||
import {
|
||||
isGoogleGemini3FlashModel,
|
||||
isGoogleGemini3ProModel,
|
||||
} from "../internal/google-model-family.js";
|
||||
import { isGoogleGemini3ThinkingLevelModel } from "../internal/google-model-family.js";
|
||||
import { detectOpenAICompletionsCompat } from "./openai-completions-compat.js";
|
||||
import {
|
||||
GEMINI_THOUGHT_SIGNATURE_VALIDATOR_SKIP,
|
||||
|
|
@ -20,10 +17,6 @@ function isGoogleOpenAICompatModel(model: OpenAIModeModel): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
function requiresGoogleCompatToolCallThoughtSignature(model: OpenAIModeModel): boolean {
|
||||
return isGoogleGemini3ProModel(model.id) || isGoogleGemini3FlashModel(model.id);
|
||||
}
|
||||
|
||||
const GOOGLE_COMPAT_THOUGHT_SIGNATURE_ELLIPSIS_RE = /[\u2026]|\.\.\./;
|
||||
const GOOGLE_COMPAT_THOUGHT_SIGNATURE_BASE64_RE = /^[A-Za-z0-9+/=]+$/;
|
||||
|
||||
|
|
@ -43,7 +36,7 @@ function injectToolCallThoughtSignatures(
|
|||
return;
|
||||
}
|
||||
const sigById = new Map<string, string>();
|
||||
const fallbackSig = requiresGoogleCompatToolCallThoughtSignature(model)
|
||||
const fallbackSig = isGoogleGemini3ThinkingLevelModel(model.id)
|
||||
? GEMINI_THOUGHT_SIGNATURE_VALIDATOR_SKIP
|
||||
: undefined;
|
||||
for (const msg of context.messages ?? []) {
|
||||
|
|
|
|||
|
|
@ -80,19 +80,16 @@ function isOfficialOpenAIResponsesBaseUrl(baseUrl: string | undefined): boolean
|
|||
if (!baseUrl) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const url = new URL(baseUrl);
|
||||
return (
|
||||
url.origin === "https://api.openai.com" &&
|
||||
url.username === "" &&
|
||||
url.password === "" &&
|
||||
url.search === "" &&
|
||||
url.hash === "" &&
|
||||
url.pathname.replace(/\/+$/, "") === "/v1"
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const url = URL.parse(baseUrl);
|
||||
return (
|
||||
url !== null &&
|
||||
url.origin === "https://api.openai.com" &&
|
||||
url.username === "" &&
|
||||
url.password === "" &&
|
||||
url.search === "" &&
|
||||
url.hash === "" &&
|
||||
url.pathname.replace(/\/+$/, "") === "/v1"
|
||||
);
|
||||
}
|
||||
export function supportsNativeOpenAIResponsesEndpoint(params: {
|
||||
provider: string;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue