Skip to content

Commit c69a2c3

Browse files
Fix remaining Medium issues from second subagent review pass
- strictModeEnabled: false for MCP tool registration — external MCP schemas are not guaranteed to satisfy OpenAI strict-mode constraints (all properties required, additionalProperties: false everywhere). A single non-conforming schema with strictModeEnabled: true would cause a 400 at registration time for ALL tools in the request. - Guard ParseToolArguments in non-streaming path — JsonException on malformed model-generated arguments would previously bubble up as a 500. Now caught identically to the streaming path: logs warning and sends an error FunctionCallOutputResponseItem so the model can recover without aborting the loop. - Guard currentLegResponseId null before tool-call continuation — if the API never emits StreamingResponseCreatedUpdate, proceeding with PreviousResponseId = null causes a confusing 400 from the API. Now throws InvalidOperationException with a diagnostic message. - Log original ConversationContextLimitExceededException in mid-stream handler — previously wrapped in a new InvalidOperationException, losing the stack trace, PreviousResponseId, and inner exception.
1 parent 97db852 commit c69a2c3

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

EssentialCSharp.Chat.Shared/Services/AIChatService.cs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,13 @@ private static string SanitizeForXmlContext(string? input) =>
232232
// GetChatCompletionCore and avoids conversation branching.
233233
if (pendingFunctionCalls is { Count: > 0 } && mcpClient != null)
234234
{
235+
// Guard: if the API never sent StreamingResponseCreatedUpdate, currentLegResponseId
236+
// is null. Continuing would send FunctionCallOutputResponseItems referencing CallIds
237+
// the server has no record of (PreviousResponseId = null), causing a 400.
238+
if (currentLegResponseId is null)
239+
throw new InvalidOperationException(
240+
"Cannot continue tool-call chain: the streaming leg completed with tool calls but emitted no response ID.");
241+
235242
var continuationOptions = CloneOptionsWithPreviousResponseId(responseOptions, currentLegResponseId);
236243
var outputItems = new List<ResponseItem>(pendingFunctionCalls.Count);
237244

@@ -387,7 +394,11 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
387394
}
388395

389396
#pragma warning disable OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
390-
options.Tools.Add(ResponseTool.CreateFunctionTool(tool.Name, functionDescription: tool.Description, strictModeEnabled: true, functionParameters: BinaryData.FromString(tool.JsonSchema.GetRawText())));
397+
// strictModeEnabled: false — MCP tool schemas come from external servers and are not
398+
// guaranteed to satisfy OpenAI strict-mode constraints (all properties required,
399+
// additionalProperties: false everywhere). A single non-conforming schema with
400+
// strict mode enabled would cause a 400 at registration time for ALL tools.
401+
options.Tools.Add(ResponseTool.CreateFunctionTool(tool.Name, functionDescription: tool.Description, strictModeEnabled: false, functionParameters: BinaryData.FromString(tool.JsonSchema.GetRawText())));
391402
#pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
392403
}
393404
}
@@ -466,7 +477,20 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
466477
}
467478

468479
LogMcpToolCallInvoked(_Logger, functionCallItem.FunctionName, iteration, endUserId);
469-
var arguments = ParseToolArguments(functionCallItem.FunctionArguments);
480+
481+
Dictionary<string, object?> arguments;
482+
try
483+
{
484+
arguments = ParseToolArguments(functionCallItem.FunctionArguments);
485+
}
486+
catch (Exception ex)
487+
{
488+
LogMcpToolArgumentParseError(_Logger, functionCallItem.FunctionName, ex, endUserId);
489+
responseItems.Add(new FunctionCallOutputResponseItem(
490+
functionCallItem.CallId,
491+
$"Error parsing arguments for '{functionCallItem.FunctionName}': invalid JSON."));
492+
continue;
493+
}
470494

471495
var toolResult = await mcpClient.CallToolAsync(
472496
functionCallItem.FunctionName,

EssentialCSharp.Web/Controllers/ChatController.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ public async Task StreamMessage([FromBody] ChatMessageRequest request, Cancellat
143143
Response.ContentType = "application/json";
144144
await Response.WriteAsJsonAsync(new { error = "This conversation has grown too long. Please start a new one.", errorCode = "context_limit_exceeded" }, CancellationToken.None);
145145
}
146-
catch (ConversationContextLimitExceededException)
146+
catch (ConversationContextLimitExceededException ex)
147147
{
148-
LogChatStreamErrorMidStream(_Logger, new InvalidOperationException("Context limit exceeded mid-stream"), User.Identity?.Name);
148+
LogChatStreamErrorMidStream(_Logger, ex, User.Identity?.Name);
149149
try
150150
{
151151
await Response.WriteAsync("data: {\"type\":\"error\",\"message\":\"This conversation has grown too long. Please start a new one.\",\"errorCode\":\"context_limit_exceeded\"}\n\n", CancellationToken.None);

0 commit comments

Comments
 (0)