DSH | dsh-tool-call-guard | neutralize tool calls with invalid JSON arguments before they brick a session (400 on vLLM replay) #5132
Replies: 6 comments
|
Follow-up — important discovery about We first implemented the guard as an
Net effect: the Where the fix actually works: the adapter's message serializer (where harness messages become wire objects — the only place the data is already being copied). Our OpenAI-compatible adapter now sanitizes during serialization: invalid-arguments calls become honest text records, and their tool results are re-expressed as user messages (the #4668 orphan pattern). End-to-end verified against the live vLLM endpoint: the previously-bricked session replays clean ( This strengthens the case for putting the validation in the official serializers: it's the one layer every request already flows through, it's per-adapter-owned (no cross-cutting mutation needed), and the freeze/next() constraints make anything above it observation-only. The plugin keeps its waterfall listener purely as a detector for visibility. |
|
Upstream fix surface analysis — where the core fix lives Now that the session is saved by the plugin (and by our OpenAI-compatible adapter), I looked at where the DeepSeek core would fix this natively. The surface is small and the change is mechanical. In const toolCalls = message.content.filter((block) => block.type === "tool-call").map((block) => ({
id: block.id,
type: "function",
function: { name: block.name, arguments: block.arguments }
}));A model that emits malformed Proposed behavior (the same two rules the plugin and our OpenAI-compatible adapter implement):
Two properties worth preserving in whatever shape the core lands: the durable session log stays append-only (sanitize on the wire, never rewrite stored history — that is what dsh-session-repair is for, and it complements this), and valid calls pay only one Happy to draft the PR against |
|
Ready-to-lift patch — since external PRs are currently closed Per CONTRIBUTING ("we cannot accept external pull requests at the moment"), here is the complete proposed change as a reviewable diff instead — maintainers are welcome to lift it verbatim, adjust to house style, or use it as the blueprint for the in-house fix. Full file also attached as a gist: dsh-llm-deepseek-arguments-guard.patch (raw patch in that repo). Scope: +/** Validate one tool-call block's arguments. Returns the block when its
+ * `arguments` parse as JSON; otherwise undefined. */
+function validToolCall(block) {
+ try {
+ JSON.parse(block.arguments);
+ return block;
+ } catch {
+ return void 0;
+ }
+}
+
/** Serialize one assistant message (text + reasoning + tool calls). */
function serializeAssistant(message) {
const text = flattenText(message.content);
const reasoning = message.content.filter((block) => block.type === "reasoning").map((block) => block.text).join("");
- const toolCalls = message.content.filter((block) => block.type === "tool-call").map((block) => ({
- id: block.id,
- type: "function",
- function: {
- name: block.name,
- arguments: block.arguments
- }
- }));
+ let dropped = [];
+ const toolCalls = message.content.filter((block) => block.type === "tool-call").filter((block) => {
+ const valid = validToolCall(block) !== void 0;
+ if (!valid) dropped.push(block);
+ return valid;
+ }).map((block) => ({
+ id: block.id,
+ type: "function",
+ function: {
+ name: block.name,
+ arguments: block.arguments
+ }
+ }));
+ for (const block of dropped) {
+ const note = `[A tool call to '${block.name}' was removed from history because its arguments were malformed JSON. Original arguments as emitted: ${block.arguments}]`;
+ text = text.length > 0 ? text + "\n" + note : note;
+ }
return {
role: "assistant",
content: text,
Verified end-to-end in production: our OpenAI-compatible adapter implements exactly this logic; a session bricked by GLM-5.3-Flash's malformed If the team prefers a different neutralization shape (e.g. empty-object arguments instead of text records), the dropped-id collection structure above supports either. |
|
Small liftability issue in the inline diff: current alpha.2 declares |
|
@Jstn-1g — thanks for the careful review against
The earlier draft's "same treatment" claim for the image variant was aspirational rather than literal — the full diff now shows it explicitly. Everything else (design: drop + honest record + result re-expression, one |
|
Update: we put the patch through a real local test cycle since the last comment, and it turned up problems on our side — posting the corrections for the record.
The corrected patch + behavior tests are now in the same repo: https://github.com/alchemistwu/dsh-serializer-patch Verification run on our side:
Apologies to @Jstn-1g and anyone who spent review time on the earlier draft — the design (drop + honest record + result re-expression) is unchanged and now actually matches the code. |
Uh oh!
There was an error while loading. Please reload this page.
The failure I hit in production
zai-org/GLM-5.3-Flashon vLLM 0.27 (OpenAI-compatible,--enable-auto-tool-choice) emitted oneweb_searchcall whoseargumentsstring contained unescaped inner quotes:Two things then happened:
The session was permanently bricked — every later turn 400s — until the log entry was repaired by hand. The same asymmetry exists across ecosystems (openai-agents-python #2061, vLLM #41122), so this is not vLLM-specific; any strict OpenAI-compatible server does it.
What I shipped
dsh-tool-call-guard — intercepts the
llm/streamwaterfall and neutralizes any assistant tool-call block whoseargumentsfailJSON.parse:[Tool Result: <tool>] …) — this follows the orphaned-tool-result re-expression pattern from discussion Plugins can break the wire protocol #4668, and keeps the conversation protocol-balanced (no danglingtool_calls, no orphanrole:"tool").Properties: zero overhead on clean history (one
JSON.parseper block, object-identity passthrough), the append-only session log is never rewritten, provider-agnostic (it sits above every adapter), fail-open, zero config.Where I think the core should go
The guard works as a plugin, but the deeper fix belongs in the official adapters' serializers (llm-deepseek first — the orphan-balancing pattern from #4668 already lives there): validate
argumentswhen assemblingtool_callsfor the wire and neutralize in place, so every stock install is protected without a plugin. Until then, the plugin covers it.Related work: dsh-session-repair repairs already-poisoned logs at load time (the empty tool-call ID family); this plugin is the wire-side complement — prevention rather than cure. Together they cover the whole lifecycle.
Registry PR: awesome-dsh-plugin#3883. Feedback welcome — especially on the honest-record text format and whether result re-expression as
role:useris the right long-term shape.All reactions