mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-12 18:26:26 +00:00
fix(core): catch content-only thinking-tag leaks on all OpenAI-compatible providers (#8818)
* fix(core): catch content-only thinking-tag leaks on all OpenAI-compatible providers Production captures (issue #6666) show hybrid-thinking models occasionally bypassing the reasoning channel and emitting their thinking as literal <think>/<thinking> text inside content. The content-only leak fallback only covered the DashScope provider, and its 128-char candidate cap released confirmed opening tags mid-stream, so real-world leaks (always longer than the cap, typically unclosed) still reached users. - Enable contentOnlyThinkingTagLeaks on DefaultOpenAICompatibleProvider so every OpenAI-compatible endpoint gets the conservative fallback (gated to turns that start with a thinking tag and carry no structured reasoning or prior visible content); drop the now-redundant DashScope override. - Classify an opening tag followed by content with no balancing closing tag as an unclosed thinking block: held mid-stream, rejected as PROTOCOL_TAG_LEAK at stream end. Whitespace-only tails stay undecided. - Exempt confirmed opening tags with real content from the length-cap release so long unclosed blocks are rejected instead of leaked. Adds regression tests replaying the sanitized production shape (red on the previous code) plus a control proving the provider gate. * fix(core): document the fail-closed trade-off and cover the over-cap throw Review follow-up: state honestly that a legitimate balanced literal longer than the candidate cap whose closing tag has not arrived yet is rejected along with real leaks (indistinguishable at that point), and add a regression test for the mid-stream fail-closed throw on over-cap confirmed opening tags. * fix(core): parse closed thinking tags by default * test(core): update DashScope parsing-options assertion after override removal The vendor-specific getResponseParsingOptions override was removed in the parent commit so DashScope inherits the default provider's options, which now include taggedThinkingTags alongside contentOnlyThinkingTagLeaks. * fix(core): reject nested unclosed thinking blocks * fix(core): handle nested thinking tags * fix(core): preserve generic thinking-tag compatibility * fix(core): defer thinking leak rejection until finish * fix(core): note streaming-only scope of thinking-leak defense
This commit is contained in:
parent
5f29e7f788
commit
647500caf6
6 changed files with 170 additions and 24 deletions
|
|
@ -544,6 +544,77 @@ describe('OpenAIContentConverter', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('rejects the recorded production unclosed <thinking> content leak (issue #6666)', () => {
|
||||
// Production capture shape (sanitized): a hybrid-thinking model
|
||||
// skipped the reasoning channel entirely and streamed its thinking as
|
||||
// literal <thinking> text inside content — no reasoning_content on
|
||||
// any chunk, no tool calls, and the tag is never closed before stop.
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const opening = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('opening', { content: '<thi' }),
|
||||
stream,
|
||||
);
|
||||
const body = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('body', {
|
||||
content:
|
||||
'nking>\nThe user wants to query the compute resources for ' +
|
||||
'project space 10088. Let me check the available APIs.',
|
||||
}),
|
||||
stream,
|
||||
);
|
||||
|
||||
expect(opening.candidates?.[0]?.content?.parts).toEqual([]);
|
||||
expect(body.candidates?.[0]?.content?.parts).toEqual([]);
|
||||
expect(() => finishStream(stream, 'stop')).toThrowError(
|
||||
expect.objectContaining({ type: 'PROTOCOL_TAG_LEAK' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('holds a long confirmed opening tag until its closing tag arrives', () => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const text = `<thinking>${'x'.repeat(200)}</thinking>`;
|
||||
|
||||
const opening = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('long-balanced', {
|
||||
content: `<thinking>${'x'.repeat(200)}`,
|
||||
}),
|
||||
stream,
|
||||
);
|
||||
const closing = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('long-balanced', { content: '</thinking>' }, 'stop'),
|
||||
stream,
|
||||
);
|
||||
|
||||
expect(opening.candidates?.[0]?.content?.parts).toEqual([]);
|
||||
expect(closing.candidates?.[0]?.content?.parts).toEqual([{ text }]);
|
||||
});
|
||||
|
||||
it('leaks the production <thinking> shape without provider provenance', () => {
|
||||
// Control for the test above: without contentOnlyThinkingTagLeaks the
|
||||
// same stream passes through verbatim — the defense is provider-gated,
|
||||
// so endpoints whose provider does not opt in remain exposed.
|
||||
const stream = withStreamParser();
|
||||
const response = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk(
|
||||
'literal',
|
||||
{
|
||||
content:
|
||||
'<thinking>\nThe user wants to query the compute resources.',
|
||||
},
|
||||
'stop',
|
||||
),
|
||||
stream,
|
||||
);
|
||||
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([
|
||||
{
|
||||
text: '<thinking>\nThe user wants to query the compute resources.',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['split literal block', ['<thi', 'nk>literal</think>']],
|
||||
['empty block with a separate finish chunk', ['<think>\n\n</think>', '']],
|
||||
|
|
@ -551,6 +622,7 @@ describe('OpenAIContentConverter', () => {
|
|||
'two split valid blocks',
|
||||
['<think>\n\n', '</think><thi', 'nk>literal</think>'],
|
||||
],
|
||||
['long empty block', [`<thinking>${' '.repeat(128)}</thinking>`, '']],
|
||||
])('preserves content-only %s', (_name, chunks) => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
|
|
@ -570,24 +642,33 @@ describe('OpenAIContentConverter', () => {
|
|||
expect(parts.every((part) => part.thought !== true)).toBe(true);
|
||||
});
|
||||
|
||||
it('releases a long undecided prefix before the stream finishes', () => {
|
||||
it('releases a long unconfirmed prefix before the stream finishes', () => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const text = `<think>${' '.repeat(257)}`;
|
||||
const text = `<think${' '.repeat(257)}`;
|
||||
|
||||
const response = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('literal', { content: text }),
|
||||
stream,
|
||||
);
|
||||
const continuation = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('continuation', { content: 'literal' }, 'stop'),
|
||||
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([{ text }]);
|
||||
});
|
||||
|
||||
it('rejects an unclosed whitespace-only block at stream finish', () => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const response = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('unclosed', {
|
||||
content: `<thinking>${' '.repeat(128)}`,
|
||||
}),
|
||||
stream,
|
||||
);
|
||||
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([{ text }]);
|
||||
expect(continuation.candidates?.[0]?.content?.parts).toEqual([
|
||||
{ text: 'literal' },
|
||||
]);
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([]);
|
||||
expect(() => finishStream(stream, 'stop')).toThrowError(
|
||||
expect.objectContaining({ type: 'PROTOCOL_TAG_LEAK' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('preserves a leak-shaped literal without provider provenance', () => {
|
||||
|
|
@ -631,17 +712,38 @@ describe('OpenAIContentConverter', () => {
|
|||
expect(parts.map((part) => part.text).join('')).toBe(chunks.join(''));
|
||||
});
|
||||
|
||||
it('fails closed when a suspicious prefix exceeds the buffer limit', () => {
|
||||
it('rejects an unclosed outer block containing a balanced nested block', () => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
|
||||
expect(() =>
|
||||
converter.convertOpenAIChunkToGemini(
|
||||
streamChunk(
|
||||
'nested-unclosed',
|
||||
{
|
||||
content: '<thinking><thinking>inner</thinking>outer text',
|
||||
},
|
||||
'stop',
|
||||
),
|
||||
stream,
|
||||
),
|
||||
).toThrowError(expect.objectContaining({ type: 'PROTOCOL_TAG_LEAK' }));
|
||||
});
|
||||
|
||||
it('fails closed for a long suspicious prefix at stream finish', () => {
|
||||
const stream = withStreamParser();
|
||||
stream.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const content = '<think></think><think>9<think>' + 'x'.repeat(257);
|
||||
|
||||
expect(() =>
|
||||
converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('long-leak', { content }),
|
||||
stream,
|
||||
),
|
||||
).toThrowError(expect.objectContaining({ type: 'PROTOCOL_TAG_LEAK' }));
|
||||
const response = converter.convertOpenAIChunkToGemini(
|
||||
streamChunk('long-leak', { content }),
|
||||
stream,
|
||||
);
|
||||
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([]);
|
||||
expect(() => finishStream(stream, 'stop')).toThrowError(
|
||||
expect.objectContaining({ type: 'PROTOCOL_TAG_LEAK' }),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
|
|
@ -5079,6 +5181,7 @@ describe('OpenAIContentConverter', () => {
|
|||
|
||||
it('should handle a single chunk delta with both reasoning_content and content simultaneously', () => {
|
||||
const ctx = withStreamParser();
|
||||
ctx.responseParsingOptions = { contentOnlyThinkingTagLeaks: true };
|
||||
const part =
|
||||
converter.convertOpenAIChunkToGemini(
|
||||
{
|
||||
|
|
@ -5215,7 +5318,10 @@ describe('OpenAIContentConverter', () => {
|
|||
},
|
||||
],
|
||||
} as unknown as OpenAI.Chat.ChatCompletion,
|
||||
requestContext,
|
||||
{
|
||||
...requestContext,
|
||||
responseParsingOptions: { contentOnlyThinkingTagLeaks: true },
|
||||
},
|
||||
);
|
||||
|
||||
expect(response.candidates?.[0]?.content?.parts).toEqual([
|
||||
|
|
|
|||
|
|
@ -1160,8 +1160,25 @@ function classifyContentOnlyThinkingTagPrefix(
|
|||
for (const closing of [false, true, false]) {
|
||||
const tagLength = consumeTag(rest, closing);
|
||||
if (tagLength === null) return 'pending';
|
||||
if (tagLength === undefined) return 'clean';
|
||||
if (tagLength === undefined) {
|
||||
if (!closing) return 'clean';
|
||||
break;
|
||||
}
|
||||
rest = rest.slice(tagLength).trimStart();
|
||||
if (closing && !rest) return 'clean';
|
||||
// An opening tag followed by ordinary text is only a legitimate literal
|
||||
// if a closing tag still balances it later. Without one, the turn is an
|
||||
// unclosed thinking block — the exact shape of the recorded production
|
||||
// leaks (issue #6666). Hold it mid-stream (a closing tag may still
|
||||
// arrive) and reject it once the stream finishes. Whitespace-only tails
|
||||
// stay undecided: they may still resolve into a closing tag.
|
||||
if (
|
||||
closing === false &&
|
||||
/\S/.test(rest) &&
|
||||
!/<\/think(?:ing)?\s*>/i.test(rest)
|
||||
) {
|
||||
return streamFinished ? 'leaked' : 'pending';
|
||||
}
|
||||
}
|
||||
|
||||
let depth = 1;
|
||||
|
|
@ -1564,8 +1581,18 @@ export function convertOpenAIChunkToGemini(
|
|||
Boolean(choice.finish_reason) &&
|
||||
!closingTagName &&
|
||||
!/\S/.test(combinedCandidateText);
|
||||
// The length cap releases undecided prefixes (e.g. a literal "<t" that
|
||||
// never resolves) so ordinary content is not buffered forever. Once the
|
||||
// candidate has committed to a complete opening tag, though, releasing
|
||||
// it can leak the whole block — production thinking-tag leaks are longer
|
||||
// than the cap (issue #6666). Keep those held until a closing tag arrives
|
||||
// or the finished-stream check rejects an unclosed block.
|
||||
const confirmedOpeningTagCandidate =
|
||||
LEADING_THINKING_TAG_PATTERN.test(combinedCandidateText) &&
|
||||
!combinedCandidateText.trimStart().startsWith('</');
|
||||
const releaseContentOnlyCandidate =
|
||||
contentOnlyThinkingState === 'pending' &&
|
||||
!confirmedOpeningTagCandidate &&
|
||||
(Boolean(choice.finish_reason) ||
|
||||
combinedCandidateText.trimStart().length >
|
||||
MAX_THINKING_TAG_CANDIDATE_LENGTH);
|
||||
|
|
@ -1591,6 +1618,7 @@ export function convertOpenAIChunkToGemini(
|
|||
requestContext.pendingThinkingTagCandidate = undefined;
|
||||
} else if (isPossibleTag) {
|
||||
if (
|
||||
!confirmedOpeningTagCandidate &&
|
||||
!closingTagName &&
|
||||
combinedCandidateText.trimStart().length >
|
||||
MAX_THINKING_TAG_CANDIDATE_LENGTH
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@ import type {
|
|||
ChatCompletionContentPartWithCache,
|
||||
ChatCompletionToolWithCache,
|
||||
} from './types.js';
|
||||
import type { OpenAIResponseParsingOptions } from '../responseParsingOptions.js';
|
||||
import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js';
|
||||
import { createDebugLogger } from '../../../utils/debugLogger.js';
|
||||
import {
|
||||
|
|
@ -166,11 +165,6 @@ export class DashScopeOpenAICompatibleProvider extends DefaultOpenAICompatiblePr
|
|||
super(contentGeneratorConfig, cliConfig);
|
||||
}
|
||||
|
||||
getResponseParsingOptions(): OpenAIResponseParsingOptions {
|
||||
// ponytail: DashScope-only fallback; remove after provider output stabilizes.
|
||||
return { contentOnlyThinkingTagLeaks: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines whether to use the DashScope-compatible provider.
|
||||
* Covers the official regional hosts (DASHSCOPE_REGIONAL_HOSTS),
|
||||
|
|
|
|||
|
|
@ -93,6 +93,14 @@ describe('DefaultOpenAICompatibleProvider', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getResponseParsingOptions', () => {
|
||||
it('enables leak handling without treating balanced tags as protocol', () => {
|
||||
expect(provider.getResponseParsingOptions()).toEqual({
|
||||
contentOnlyThinkingTagLeaks: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildHeaders', () => {
|
||||
it('should build headers with User-Agent', () => {
|
||||
const headers = provider.buildHeaders();
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import type { Config } from '../../../config/config.js';
|
|||
import type { ContentGeneratorConfig } from '../../contentGenerator.js';
|
||||
import { DEFAULT_MAX_RETRIES, resolveRequestTimeout } from '../constants.js';
|
||||
import type { OpenAICompatibleProvider } from './types.js';
|
||||
import type { OpenAIResponseParsingOptions } from '../responseParsingOptions.js';
|
||||
import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js';
|
||||
import {
|
||||
tokenLimit,
|
||||
|
|
@ -123,6 +124,15 @@ export class DefaultOpenAICompatibleProvider
|
|||
return {};
|
||||
}
|
||||
|
||||
getResponseParsingOptions(): OpenAIResponseParsingOptions {
|
||||
// Hybrid-thinking models occasionally bypass the reasoning channel and
|
||||
// emit their thinking as literal <think>/<thinking> tags inside content
|
||||
// (observed in production on qwen3-class models, issue #6666).
|
||||
// Honored on the streaming path only; non-streaming responses are
|
||||
// not classified.
|
||||
return { contentOnlyThinkingTagLeaks: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply output token limit to a request's max_tokens parameter.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ export class MiniMaxOpenAICompatibleProvider extends DefaultOpenAICompatibleProv
|
|||
}
|
||||
}
|
||||
|
||||
getResponseParsingOptions(): OpenAIResponseParsingOptions {
|
||||
override getResponseParsingOptions(): OpenAIResponseParsingOptions {
|
||||
return { taggedThinkingTags: true };
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue