Skip to content

[Security] Keep copied tool closers inside Qwen parameter values - #58407

Open
jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/qwen3-parameter-value-boundary
Open

jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/qwen3-parameter-value-boundary

Conversation

@jperezdealgaba

Copy link
Copy Markdown
Contributor

Summary

Qwen-family XML tool parsers treat a </parameter> that appears inside a parameter value as the end of that value. When the copied text continues with another tool-call block, and the model then emits its own closing tags, the parser returns that block as a second tool call. The fix detects the extra closer (more closing tags than opening tags) and keeps the copied text in the first parameter. Later calls are held until the generation finishes, then dropped when the output is unbalanced. Balanced parallel calls are unchanged.

Changes

  • vllm/parser/qwen3.py: when closers outnumber openers, rebuild one call whose first parameter runs through the final closer. Streaming holds calls after the first until finish_streaming.
  • tests/parser/engine/test_qwen3.py: copied closers stay inside the parameter for the parser and the serving adapter, streaming does not emit the copied call, and balanced parallel calls still both return.
  • tests/parser/engine/test_seed_oss.py: same copied-closer case with seed wrapper tokens.

Codepath coverage

  • Chat completions non-streaming goes through Qwen3ParserToolAdapter.extract_tool_calls (qwen3_coder, qwen3_xml, and mimo all use that adapter). Covered.
  • Chat completions streaming goes through extract_tool_calls_streaming and finish_streaming, which serving calls when the stream finishes. Covered.
  • Responses uses the same tool parser. Covered.
  • seed_oss subclasses Qwen3Parser and uses its own wrapper tokens. Covered.
  • Nemotron V3 subclasses the same parser; existing nemotron parser tests still pass.

Duplicate-work check

No open or merged diff stops an extra closer inside a parameter value from becoming another call.

Tests

  • test_copied_closer_stays_inside_the_parameter
  • test_serving_adapter_keeps_copied_closer_in_the_parameter
  • test_streaming_does_not_emit_the_copied_call
  • test_balanced_parallel_calls_still_both_return
  • test_streaming_balanced_parallel_calls_flush_at_finish
  • test_copied_closer_stays_inside_the_parameter (seed_oss)
.venv/bin/python -m pytest tests/parser/engine/test_qwen3.py tests/parser/engine/test_seed_oss.py tests/parser/engine/test_nemotron_v3.py tests/parser/engine/test_qwen3_reasoning.py -q -p no:randomly
# 153 passed
.venv/bin/pre-commit run ruff-check --files vllm/parser/qwen3.py tests/parser/engine/test_qwen3.py tests/parser/engine/test_seed_oss.py
.venv/bin/pre-commit run ruff-format --files vllm/parser/qwen3.py tests/parser/engine/test_qwen3.py tests/parser/engine/test_seed_oss.py
# both passed

AI assistance

This PR was developed with AI assistance.

Made with Cursor

A parameter value that repeats the format's closing tags was parsed as a second tool call. When closers outnumber openers, keep that text in the first parameter and do not emit the later call.

Co-authored-by: Cursor Agent
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added qwen Related to Qwen models tool-calling labels Sep 23, 2026
@mergify

mergify Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jperezdealgaba.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

This branch has not been deployed

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

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants