fix: preserve only originally-required properties in strict tool schemas

src/services/api/openaiShim.test.ts

@@ -1739,12 +1739,70 @@ test('sanitizes malformed MCP tool schemas before sending them to OpenAI', async
     | undefined
 
   expect(parameters?.additionalProperties).toBe(false)
-  expect(parameters?.required).toEqual(['priority'])
+  // No required[] in the original schema → none added (optional properties must not be forced required)
+  expect(parameters?.required).toEqual([])
   expect(properties?.priority?.type).toBe('integer')
   expect(properties?.priority?.enum).toEqual([0, 1, 2, 3])
   expect(properties?.priority).not.toHaveProperty('default')
 })
 
+test('optional tool properties are not added to required[] — fixes Groq/Azure 400 tool_use_failed', async () => {
+  // Regression test for: all optional properties being sent as required in strict mode,
+  // causing providers like Groq to reject valid tool calls where the model omits optional args.
+  let requestBody: Record<string, unknown> | undefined
+
+  globalThis.fetch = (async (_input, init) => {
+    requestBody = JSON.parse(String(init?.body))
+
+    return new Response(
+      JSON.stringify({
+        id: 'chatcmpl-4',
+        model: 'gpt-4o',
+        choices: [{ message: { role: 'assistant', content: 'ok' }, finish_reason: 'stop' }],
+        usage: { prompt_tokens: 5, completion_tokens: 2, total_tokens: 7 },
+      }),
+      { headers: { 'Content-Type': 'application/json' } },
+    )
+  }) as FetchType
+
+  const client = createOpenAIShimClient({}) as OpenAIShimClient
+
+  await client.beta.messages.create({
+    model: 'gpt-4o',
+    messages: [{ role: 'user', content: 'read a file' }],
+    tools: [
+      {
+        name: 'Read',
+        description: 'Read a file',
+        input_schema: {
+          type: 'object',
+          properties: {
+            file_path: { type: 'string', description: 'Absolute path to file' },
+            offset: { type: 'number', description: 'Line to start from' },
+            limit: { type: 'number', description: 'Max lines to read' },
+            pages: { type: 'string', description: 'Page range for PDFs' },
+          },
+          required: ['file_path'],
+        },
+      },
+    ],
+    max_tokens: 16,
+    stream: false,
+  })
+
+  const parameters = (
+    requestBody?.tools as Array<{ function?: { parameters?: Record<string, unknown> } }>
+  )?.[0]?.function?.parameters
+
+  expect(parameters?.required).toEqual(['file_path'])
+
+  const required = parameters?.required as string[] | undefined
+  expect(required).not.toContain('offset')
+  expect(required).not.toContain('limit')
+  expect(required).not.toContain('pages')
+  expect(parameters?.additionalProperties).toBe(false)
+})
+
 // ---------------------------------------------------------------------------
 // Issue #202 — consecutive role coalescing (Devstral, Mistral strict templates)
 // ---------------------------------------------------------------------------
@@ -1782,7 +1840,7 @@ test('coalesces consecutive user messages to avoid alternation errors (issue #20
     stream: false,
   })
 
-  expect(sentMessages?.length).toBe(2) // system + 1 merged user
+  expect(sentMessages?.length).toBe(2)
   expect(sentMessages?.[0]?.role).toBe('system')
   expect(sentMessages?.[1]?.role).toBe('user')
   const userContent = sentMessages?.[1]?.content as string
@@ -1816,9 +1874,8 @@ test('coalesces consecutive assistant messages preserving tool_calls (issue #202
     stream: false,
   })
 
-  // system + user + merged assistant + tool
   const assistantMsgs = sentMessages?.filter(m => m.role === 'assistant')
-  expect(assistantMsgs?.length).toBe(1) // two assistant turns merged into one
+  expect(assistantMsgs?.length).toBe(1)
   expect(assistantMsgs?.[0]?.tool_calls?.length).toBeGreaterThan(0)
 })
 
@@ -1908,8 +1965,6 @@ test('non-streaming: empty string content does not fall through to reasoning_con
     stream: false,
   })) as { content: Array<Record<string, unknown>> }
 
-  // reasoning_content should be a thinking block, and also used as text
-  // since content is empty string (treated as absent)
   expect(result.content).toEqual([
     { type: 'thinking', thinking: 'Chain of thought here.' },
     { type: 'text', text: 'Chain of thought here.' },
@@ -2037,7 +2092,6 @@ test('streaming: thinking block closed before tool call', async () => {
 
   const types = events.map(e => e.type)
 
-  // Verify thinking block is started, then closed, then tool call starts
   const thinkingStartIdx = types.indexOf('content_block_start')
   const firstStopIdx = types.indexOf('content_block_stop')
   const toolStartIdx = types.indexOf(
@@ -2049,7 +2103,6 @@ test('streaming: thinking block closed before tool call', async () => {
   expect(firstStopIdx).toBeGreaterThan(thinkingStartIdx)
   expect(toolStartIdx).toBeGreaterThan(firstStopIdx)
 
-  // Verify thinking block start content
   const thinkingStart = events[thinkingStartIdx] as {
     content_block?: Record<string, unknown>
   }

src/services/api/openaiShim.ts

@@ -410,11 +410,13 @@ function normalizeSchemaForOpenAI(
     record.properties = normalizedProps
 
     if (strict) {
-      // OpenAI strict mode requires every property to be listed in required[]
-      const allKeys = Object.keys(normalizedProps)
-      record.required = Array.from(new Set([...existingRequired, ...allKeys]))
-      // OpenAI strict mode requires additionalProperties: false on all object
-      // schemas — override unconditionally to ensure nested objects comply.
+      // Keep only the properties that were originally marked required in the schema.
+      // Adding every property to required[] (the previous behaviour) caused strict
+      // OpenAI-compatible providers (Groq, Azure, etc.) to reject tool calls because
+      // the model correctly omits optional arguments — but the provider treats them
+      // as missing required fields and returns a 400 / tool_use_failed error.
+      record.required = existingRequired.filter(k => k in normalizedProps)
+      // additionalProperties: false is still required by strict-mode providers.
       record.additionalProperties = false
     } else {
       // For Gemini: keep only existing required keys that are present in properties