From fa29de7183cf5a7bd33ea90166078f4529a4a50b Mon Sep 17 00:00:00 2001 From: Badri Narayanan S Date: Wed, 14 Jan 2026 23:01:13 +0530 Subject: [PATCH] fix: handle unsigned thinking blocks in tool loops (#120) When Claude Code strips thinking signatures it doesn't recognize, the proxy would drop unsigned thinking blocks, causing the error "Expected thinking but found text". This fix detects unsigned thinking blocks and triggers recovery to close the tool loop. Co-Authored-By: Claude --- src/format/request-converter.js | 10 ++- src/format/signature-cache.js | 8 ++ src/format/thinking-utils.js | 16 ++++ src/server.js | 11 +++ tests/test-cross-model-thinking.cjs | 132 +++++++++++++++++++++++++++- 5 files changed, 171 insertions(+), 6 deletions(-) diff --git a/src/format/request-converter.js b/src/format/request-converter.js index 5225422..da2d9da 100644 --- a/src/format/request-converter.js +++ b/src/format/request-converter.js @@ -16,6 +16,7 @@ import { reorderAssistantContent, filterUnsignedThinkingBlocks, hasGeminiHistory, + hasUnsignedThinkingBlocks, needsThinkingRecovery, closeToolLoopForThinking } from './thinking-utils.js'; @@ -87,10 +88,11 @@ export function convertAnthropicToGoogle(anthropicRequest) { processedMessages = closeToolLoopForThinking(messages, 'gemini'); } - // For Claude: apply recovery only for cross-model (Gemini→Claude) switch - // Detected by checking if history has Gemini-style tool_use with thoughtSignature - if (isClaudeModel && isThinking && hasGeminiHistory(messages) && needsThinkingRecovery(messages)) { - logger.debug('[RequestConverter] Applying thinking recovery for Claude (cross-model from Gemini)'); + // For Claude: apply recovery for cross-model (Gemini→Claude) or unsigned thinking blocks + // Unsigned thinking blocks occur when Claude Code strips signatures it doesn't understand + const needsClaudeRecovery = hasGeminiHistory(messages) || hasUnsignedThinkingBlocks(messages); + if (isClaudeModel && isThinking && needsClaudeRecovery && needsThinkingRecovery(messages)) { + logger.debug('[RequestConverter] Applying thinking recovery for Claude'); processedMessages = closeToolLoopForThinking(messages, 'claude'); } diff --git a/src/format/signature-cache.js b/src/format/signature-cache.js index 944c821..5adc1b0 100644 --- a/src/format/signature-cache.js +++ b/src/format/signature-cache.js @@ -112,3 +112,11 @@ export function getCachedSignatureFamily(signature) { export function getThinkingCacheSize() { return thinkingSignatureCache.size; } + +/** + * Clear all entries from the thinking signature cache. + * Used for testing cold cache scenarios. + */ +export function clearThinkingSignatureCache() { + thinkingSignatureCache.clear(); +} diff --git a/src/format/thinking-utils.js b/src/format/thinking-utils.js index 14ce530..88b055a 100644 --- a/src/format/thinking-utils.js +++ b/src/format/thinking-utils.js @@ -42,6 +42,22 @@ export function hasGeminiHistory(messages) { ); } +/** + * Check if conversation has unsigned thinking blocks that will be dropped. + * These cause "Expected thinking but found text" errors. + * @param {Array} messages - Array of messages + * @returns {boolean} True if any assistant message has unsigned thinking blocks + */ +export function hasUnsignedThinkingBlocks(messages) { + return messages.some(msg => { + if (msg.role !== 'assistant' && msg.role !== 'model') return false; + if (!Array.isArray(msg.content)) return false; + return msg.content.some(block => + isThinkingPart(block) && !hasValidSignature(block) + ); + }); +} + /** * Sanitize a thinking part by keeping only allowed fields */ diff --git a/src/server.js b/src/server.js index 86c92a2..cd21571 100644 --- a/src/server.js +++ b/src/server.js @@ -18,6 +18,7 @@ const __dirname = path.dirname(__filename); import { forceRefresh } from './auth/token-extractor.js'; import { REQUEST_BODY_LIMIT } from './constants.js'; import { AccountManager } from './account-manager/index.js'; +import { clearThinkingSignatureCache } from './format/signature-cache.js'; import { formatDuration } from './utils/helpers.js'; import { logger } from './utils/logger.js'; import usageStats from './modules/usage-stats.js'; @@ -161,6 +162,16 @@ app.use((req, res, next) => { next(); }); +/** + * Test endpoint - Clear thinking signature cache + * Used for testing cold cache scenarios in cross-model tests + */ +app.post('/test/clear-signature-cache', (req, res) => { + clearThinkingSignatureCache(); + logger.debug('[Test] Cleared thinking signature cache'); + res.json({ success: true, message: 'Thinking signature cache cleared' }); +}); + /** * Health check endpoint - Detailed status * Returns status of all accounts including rate limits and model quotas diff --git a/tests/test-cross-model-thinking.cjs b/tests/test-cross-model-thinking.cjs index 28d3afe..1a34060 100644 --- a/tests/test-cross-model-thinking.cjs +++ b/tests/test-cross-model-thinking.cjs @@ -241,6 +241,130 @@ async function testGeminiToClaude(CLAUDE_MODEL, GEMINI_MODEL) { } } +async function testGeminiToClaudeColdCache(CLAUDE_MODEL, GEMINI_MODEL) { + console.log('\n' + '='.repeat(60)); + console.log('TEST: Gemini → Claude Cross-Model Switch (COLD CACHE)'); + console.log('Simulates: thinking block with NO signature (stripped by Claude Code)'); + console.log('Expected error without fix: "Expected thinking but found text"'); + console.log('='.repeat(60)); + console.log(''); + + const claudeConfig = getModelConfig('claude'); + const geminiConfig = getModelConfig('gemini'); + + // TURN 1: Get response from Gemini with tool use + console.log('TURN 1: Request to Gemini (get tool_use)'); + console.log('-'.repeat(40)); + + const turn1Messages = [ + { role: 'user', content: 'Run the command "whoami" to show current user.' } + ]; + + const turn1Result = await streamRequest({ + model: GEMINI_MODEL, + max_tokens: geminiConfig.max_tokens, + stream: true, + tools, + thinking: geminiConfig.thinking, + messages: turn1Messages + }); + + const turn1Content = analyzeContent(turn1Result.content); + console.log(` Thinking: ${turn1Content.hasThinking ? 'YES' : 'NO'}`); + console.log(` Signature: ${turn1Content.hasSignature ? 'YES' : 'NO'}`); + console.log(` Tool Use: ${turn1Content.hasToolUse ? 'YES' : 'NO'}`); + + if (!turn1Content.hasToolUse) { + console.log(' SKIP: No tool use in turn 1'); + return { passed: false, skipped: true }; + } + + // Build assistant content simulating what Claude Code sends back + // CRITICAL: No signature on thinking block - simulates Claude Code stripping it + const assistantContent = []; + + // Add thinking block WITHOUT signature - this is what causes the issue + // Claude Code strips signatures it doesn't understand + assistantContent.push({ + type: 'thinking', + thinking: turn1Content.hasThinking && turn1Content.thinking[0] + ? turn1Content.thinking[0].thinking + : 'I need to run the whoami command.' + // NO signature field - simulating Claude Code stripping it + }); + + // Add text block + assistantContent.push({ + type: 'text', + text: turn1Content.hasText && turn1Content.text[0] + ? turn1Content.text[0].text + : 'I will run the whoami command for you.' + }); + + // Add tool_use blocks (also without thoughtSignature) + for (const tool of turn1Content.toolUse) { + assistantContent.push({ + type: 'tool_use', + id: tool.id, + name: tool.name, + input: tool.input + // NO thoughtSignature - Claude Code strips this too + }); + } + + console.log(` Built assistant content with UNSIGNED thinking block`); + + // TURN 2: Switch to Claude with unsigned thinking in history + console.log('\nTURN 2: Request to Claude (with UNSIGNED thinking block)'); + console.log('-'.repeat(40)); + console.log(` Assistant content: ${JSON.stringify(assistantContent).substring(0, 300)}...`); + + const turn2Messages = [ + { role: 'user', content: 'Run the command "whoami" to show current user.' }, + { role: 'assistant', content: assistantContent }, + { + role: 'user', + content: [{ + type: 'tool_result', + tool_use_id: turn1Content.toolUse[0].id, + content: 'testuser' + }] + } + ]; + + try { + const turn2Result = await streamRequest({ + model: CLAUDE_MODEL, + max_tokens: claudeConfig.max_tokens, + stream: true, + tools, + thinking: claudeConfig.thinking, + messages: turn2Messages + }); + + const turn2Content = analyzeContent(turn2Result.content); + console.log(` Response received: YES`); + console.log(` Stop reason: ${turn2Result.stop_reason}`); + console.log(` Thinking: ${turn2Content.hasThinking ? 'YES' : 'NO'}`); + console.log(` Text: ${turn2Content.hasText ? 'YES' : 'NO'}`); + console.log(` Tool Use: ${turn2Content.hasToolUse ? 'YES' : 'NO'}`); + + // Success if we got any response without error + const passed = turn2Content.hasText || turn2Content.hasThinking || turn2Content.hasToolUse; + console.log(` Result: ${passed ? 'PASS' : 'FAIL'}`); + return { passed }; + } catch (error) { + // Check for the specific error from issue #120 + const isExpectedError = error.message.includes('Expected') && + error.message.includes('thinking') && + error.message.includes('found'); + console.log(` Error: ${error.message.substring(0, 200)}`); + console.log(` Is issue #120 error: ${isExpectedError ? 'YES' : 'NO'}`); + console.log(` Result: FAIL`); + return { passed: false, error: error.message, isIssue120Error: isExpectedError }; + } +} + async function testSameModelContinuation(CLAUDE_MODEL) { console.log('\n' + '='.repeat(60)); console.log('TEST: Same Model Continuation - Claude (Control Test)'); @@ -479,11 +603,15 @@ async function main() { const geminiToClaude = await testGeminiToClaude(CLAUDE_MODEL, GEMINI_MODEL); results.push({ name: 'Gemini → Claude', ...geminiToClaude }); - // Test 3: Same model Claude (control) + // Test 3: Gemini → Claude with COLD CACHE (simulates cache expiry) + const geminiToClaudeCold = await testGeminiToClaudeColdCache(CLAUDE_MODEL, GEMINI_MODEL); + results.push({ name: 'Gemini → Claude (Cold Cache)', ...geminiToClaudeCold }); + + // Test 4: Same model Claude (control) const sameModelClaude = await testSameModelContinuation(CLAUDE_MODEL); results.push({ name: 'Same Model (Claude → Claude)', ...sameModelClaude }); - // Test 4: Same model Gemini (control) + // Test 5: Same model Gemini (control) const sameModelGemini = await testSameModelContinuationGemini(GEMINI_MODEL); results.push({ name: 'Same Model (Gemini → Gemini)', ...sameModelGemini });