Skip to content

fix(deepseek-web): bind native DSML tool calls to the request nonce (policy change — please review) - #15448

Merged
diegosouzapw merged 11 commits into
diegosouzapw:release/v3.8.52from
arashashrafii:feat/deepseek-dsml-nonce-binding
Oct 10, 2026
Merged

diegosouzapw merged 11 commits into
diegosouzapw:release/v3.8.52from
arashashrafii:feat/deepseek-dsml-nonce-binding

Conversation

@arashashrafii

Copy link
Copy Markdown
Contributor

What this proposes

Native DeepSeek DSML replies (<||DSML|| invoke>, <|DSML|invoke>, the WMAdapter ▁-token form)
were trusted on shape alone: any well-formed <invoke> became a tool call, with no check that

  1. its _nonce matched the request binding, or
  2. its arguments satisfied the tool's JSON schema.

A copied or hallucinated envelope therefore executed. The canonical <tool>{json}</tool> path has had
both checks since #9343; the DSML path had neither.

What it changes

Piece Effect
finalizeParsedToolCalls holds every call this fork parses to the full nonce + schema contract
finalizeBoundCalls applies that contract on the shared tag/DSML path to any call carrying _nonce; upstream's nameless <tool><parameter> shape (no JSON body to bind) keeps working
hasMalformedDeepSeekToolMarkup/Intent detect DSML intent so the executor repairs once instead of leaking the envelope
exhausted repair now fails closed with 502 instead of echoing the half-formed envelope back as best-effort text

The policy question for review — please read before merging

Upstream explicitly documents the opposite:

// Native DSML dialect (no nonce concept — same policy as the XML/tag-suffix shapes, which likewise carry no JSON body to bind).

This PR deliberately departs from that for DSML, on the argument that DSML bodies do carry a JSON
payload (and, in practice, a _nonce parameter), so the guarantee the canonical path already makes
should extend to them.

Consequences to weigh:

  • It changes the error contract for an unrecoverable envelope from 200 + raw text to 502
    (tests/unit/deepseek-web-tool-call-retry.test.ts is updated here to assert the new contract).
  • The three variants/executor tests that previously asserted the permissive policy now assert the
    bound one; a test using single-pipe DSML was moved to that dialect so it exercises the fork path
    rather than upstream's parser.
  • The fork dialects (single-pipe ASCII |DSML|, ▁-token) are parsed by this fork; upstream's
    double-pipe parser runs first and keeps ownership of its own inputs.

If you prefer to keep the "no nonce concept" policy, close this and #14103 stands alone — the
dialect coverage there is independent of this proposal.

Verification

arashashrafii and others added 8 commits October 3, 2026 12:00
Removes src/shared/contracts/openai.ts and its test — the module was
never imported by any route/handler (verified via grep across
open-sse/, src/, tests/), so it added maintenance surface with zero
runtime effect. Fixes the two OPENAI_COMPATIBILITY.md doc-links that
pointed at the removed test file to point at existing coverage
instead (chat-completions-route-shape-gate.test.ts,
audio-transcription-handler.test.ts, audio-speech-handler.test.ts).

Also drops the src/app/api/v1/batches/route.ts sanitizeErrorMessage()
hunk from this PR: it duplicates the same Hard Rule diegosouzapw#12 fix already
proposed standalone in diegosouzapw#14115 and diegosouzapw#14145, so it is left for whichever
of those lands first rather than triple-implemented here.

The real fix in this PR — DSML tool-call normalization for
DeepSeek-shaped web providers in webTools.ts/deepseekWebTools.ts —
is unaffected and remains the mergeable core.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…ratchet

Fixes a real regression this PR's own DSML changes introduced against the
pre-existing tests/unit/deepseek-web-tools-salvage-leading-json.test.ts
suite: parseFullWidthDsmlCalls() treated the mere presence of a stray/
corrupted "||DSML||" delimiter fragment (e.g. trailing garbage after an
otherwise well-formed <tool>{json} block with no actual <invoke name="...">
tag) as "recognized", short-circuiting parseDeepSeekToolCalls() before the
canonical tag parser's JSON-salvage fallback ever ran. It now only reports
recognized:true when a structurally well-formed invoke tag was found
(matching the "malformed or unknown native markers remain content"
contract for genuinely well-formed-but-unresolvable invokes).

Also splits the new parseNativeDeepSeekCalls/parseFullWidthDsmlCalls
functions (deepseekWebTools.ts) and resolveImageBaseUrl (imageGeneration.ts)
into smaller helpers to clear the complexity/cognitive-complexity ratchet
this PR's diff newly crossed (verified via
scripts/check/check-complexity-ratchets.mjs --base-ref
origin/release/v3.8.51, now 0/0 new violations) — no behavior change beyond
the recognized-flag fix above.

Corrects the changelog fragment: it used the wrong format for
check:changelog-integrity (prose + "Closes #N" lines instead of a "- "
bullet) and, per review, overclaimed a "canonical contract" this PR does
not ship — rewritten to describe only what is actually wired.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@arashashrafii
arashashrafii marked this pull request as ready for review October 3, 2026 20:56
diegosouzapw and others added 3 commits October 8, 2026 17:55
diegosouzapw#14103 landed on the base as a squash; the 7 inherited commits now merge
to the base content, leaving only this PR's own commit as the delta.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…inding comments

hasMalformedDeepSeekToolMarkup required whitespace after the DSML marker,
so an unbound <|DSML|invoke ...> reply yielded no calls and no repair
signal and leaked as a 200 text reply. Accept \s*. Comments now state the
real policy: single-pipe/full-width DSML is nonce-bound and schema-checked,
the double-pipe dialect and the canonical <tool> envelope keep the shared
diegosouzapw#9343 behavior (no schema validation there). Adds the changelog fragment
for the 200 -> 502 contract change.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…dule

deepseekWebTools.ts crossed the 1200-line cap for unfrozen files and
deepseek-web.ts grew past its frozen size. Move nonce binding, schema
validation and the malformed-envelope detectors to
deepseekWebToolBinding.ts (re-exported), and share one 502 helper in the
executor's repair branch. No behavior change.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit f3a25e8 into diegosouzapw:release/v3.8.52 Oct 10, 2026
28 of 51 checks passed
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @arashashrafii — this is in the upcoming release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants