fix(api): drop orphan tool results to satisfy strict role sequence (#745)
* fix(api): drop orphan tool results to satisfy Mistral/OpenAI strict role sequence * test: add test for orphan tool results and restore gemini comments
This commit is contained in:
@@ -2856,3 +2856,91 @@ test('classifies chat-completions endpoint 404 failures with endpoint_not_found
|
|||||||
}),
|
}),
|
||||||
).rejects.toThrow('openai_category=endpoint_not_found')
|
).rejects.toThrow('openai_category=endpoint_not_found')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('preserves valid tool_result and drops orphan tool_result', async () => {
|
||||||
|
let requestBody: Record<string, unknown> | undefined
|
||||||
|
|
||||||
|
globalThis.fetch = (async (_input, init) => {
|
||||||
|
requestBody = JSON.parse(String(init?.body))
|
||||||
|
|
||||||
|
return new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
id: 'chatcmpl-1',
|
||||||
|
model: 'mistral-large-latest',
|
||||||
|
choices: [
|
||||||
|
{
|
||||||
|
message: {
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'done',
|
||||||
|
},
|
||||||
|
finish_reason: 'stop',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
usage: {
|
||||||
|
prompt_tokens: 12,
|
||||||
|
completion_tokens: 4,
|
||||||
|
total_tokens: 16,
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
{
|
||||||
|
headers: {
|
||||||
|
'Content-Type': 'application/json',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}) as FetchType
|
||||||
|
|
||||||
|
const client = createOpenAIShimClient({}) as OpenAIShimClient
|
||||||
|
|
||||||
|
await client.beta.messages.create({
|
||||||
|
model: 'mistral-large-latest',
|
||||||
|
system: 'test system',
|
||||||
|
messages: [
|
||||||
|
{ role: 'user', content: 'Search and then I will interrupt' },
|
||||||
|
{
|
||||||
|
role: 'assistant',
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'tool_use',
|
||||||
|
id: 'valid_call_1',
|
||||||
|
name: 'Search',
|
||||||
|
input: { query: 'openclaude' },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: 'user',
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'tool_result',
|
||||||
|
tool_use_id: 'valid_call_1',
|
||||||
|
content: 'Found it!',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: 'tool_result',
|
||||||
|
tool_use_id: 'orphan_call_2',
|
||||||
|
content: 'Interrupted result',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: 'user',
|
||||||
|
content: 'What happened?',
|
||||||
|
}
|
||||||
|
],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
max_tokens: 64,
|
||||||
|
stream: false,
|
||||||
|
})
|
||||||
|
|
||||||
|
const messages = requestBody?.messages as Array<Record<string, unknown>>
|
||||||
|
|
||||||
|
// Should have: system, user, assistant (tool_use), tool (valid_call_1), user
|
||||||
|
// Should NOT have: tool (orphan_call_2)
|
||||||
|
|
||||||
|
const toolMessages = messages.filter(m => m.role === 'tool')
|
||||||
|
expect(toolMessages.length).toBe(1)
|
||||||
|
expect(toolMessages[0].tool_call_id).toBe('valid_call_1')
|
||||||
|
|
||||||
|
const orphanMessage = toolMessages.find(m => m.tool_call_id === 'orphan_call_2')
|
||||||
|
expect(orphanMessage).toBeUndefined()
|
||||||
|
})
|
||||||
|
|||||||
@@ -349,6 +349,7 @@ function convertMessages(
|
|||||||
system: unknown,
|
system: unknown,
|
||||||
): OpenAIMessage[] {
|
): OpenAIMessage[] {
|
||||||
const result: OpenAIMessage[] = []
|
const result: OpenAIMessage[] = []
|
||||||
|
const knownToolCallIds = new Set<string>()
|
||||||
|
|
||||||
// System message first
|
// System message first
|
||||||
const sysText = convertSystemPrompt(system)
|
const sysText = convertSystemPrompt(system)
|
||||||
@@ -368,13 +369,21 @@ function convertMessages(
|
|||||||
const toolResults = content.filter((b: { type?: string }) => b.type === 'tool_result')
|
const toolResults = content.filter((b: { type?: string }) => b.type === 'tool_result')
|
||||||
const otherContent = content.filter((b: { type?: string }) => b.type !== 'tool_result')
|
const otherContent = content.filter((b: { type?: string }) => b.type !== 'tool_result')
|
||||||
|
|
||||||
// Emit tool results as tool messages
|
// Emit tool results as tool messages, but ONLY if we have a matching tool_use ID.
|
||||||
|
// Mistral/OpenAI strictly require tool messages to follow an assistant message with tool_calls.
|
||||||
|
// If the user interrupted (ESC) and a synthetic tool_result was generated without a recorded tool_use,
|
||||||
|
// emitting it here would cause a "role must alternate" or "unexpected role" error.
|
||||||
for (const tr of toolResults) {
|
for (const tr of toolResults) {
|
||||||
result.push({
|
const id = tr.tool_use_id ?? 'unknown'
|
||||||
role: 'tool',
|
if (knownToolCallIds.has(id)) {
|
||||||
tool_call_id: tr.tool_use_id ?? 'unknown',
|
result.push({
|
||||||
content: convertToolResultContent(tr.content, tr.is_error),
|
role: 'tool',
|
||||||
})
|
tool_call_id: id,
|
||||||
|
content: convertToolResultContent(tr.content, tr.is_error),
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
logForDebugging(`Dropping orphan tool_result for ID: ${id} to prevent API error`)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Emit remaining user content
|
// Emit remaining user content
|
||||||
@@ -415,9 +424,11 @@ function convertMessages(
|
|||||||
input?: unknown
|
input?: unknown
|
||||||
extra_content?: Record<string, unknown>
|
extra_content?: Record<string, unknown>
|
||||||
signature?: string
|
signature?: string
|
||||||
}, index) => {
|
}) => {
|
||||||
|
const id = tu.id ?? `call_${crypto.randomUUID().replace(/-/g, '')}`
|
||||||
|
knownToolCallIds.add(id)
|
||||||
const toolCall: NonNullable<OpenAIMessage['tool_calls']>[number] = {
|
const toolCall: NonNullable<OpenAIMessage['tool_calls']>[number] = {
|
||||||
id: tu.id ?? `call_${crypto.randomUUID().replace(/-/g, '')}`,
|
id,
|
||||||
type: 'function' as const,
|
type: 'function' as const,
|
||||||
function: {
|
function: {
|
||||||
name: tu.name ?? 'unknown',
|
name: tu.name ?? 'unknown',
|
||||||
@@ -442,7 +453,6 @@ function convertMessages(
|
|||||||
|
|
||||||
// Merge into existing google-specific metadata if present
|
// Merge into existing google-specific metadata if present
|
||||||
const existingGoogle = (toolCall.extra_content?.google as Record<string, unknown>) ?? {}
|
const existingGoogle = (toolCall.extra_content?.google as Record<string, unknown>) ?? {}
|
||||||
|
|
||||||
toolCall.extra_content = {
|
toolCall.extra_content = {
|
||||||
...toolCall.extra_content,
|
...toolCall.extra_content,
|
||||||
google: {
|
google: {
|
||||||
|
|||||||
Reference in New Issue
Block a user