Fix Critical and High AI chat review findings

C1 - Two-phase streaming tool call pattern: buffer all function calls
during the stream, execute sequentially after stream completes, then
make ONE continuation with all outputs. Previously, N parallel tool
calls created N forked conversation branches.

C2 - Context length errors in streaming path: added RethrowContextLengthErrors
helper wrapper that puts try/catch only around MoveNextAsync, keeping
yield return outside. C# CS1626 prohibits yield inside try/catch, so
the wrapper is the only valid pattern for exception remapping in async
iterators.

H1 - Restore stream-write body in second catch in StreamMessage:
the SSE error event was lost in a prior edit.

H2 - Clone all relevant ResponseCreationOptions fields in
CloneOptionsWithPreviousResponseId: EndUserId, MaxOutputTokenCount,
TextOptions, TruncationMode, ParallelToolCallsEnabled,
StoredOutputEnabled, ToolChoice, Temperature, TopP, ServiceTier,
Metadata.

H3 - IsContextLengthError now parses structured JSON error code via
TryExtractErrorCode: handles status 413, code token_limit_exceeded,
and context_length_exceeded. Falls back to message substring match.

Bonus: wire EndUserId into CreateResponseOptionsAsync (SDK supports it).
Add LogMcpToolArgumentParseError logger message.

Author: BenjaminMichaelis

EssentialCSharp.Chat.Shared/Services/AIChatService.cs

@@ -172,7 +172,11 @@ private static string SanitizeForXmlContext(string? input) =>
         input?.Replace("<", "\u2039").Replace(">", "\u203A") ?? string.Empty;
 
     /// <summary>
-    /// Processes streaming updates from the OpenAI Responses API, handling both regular responses and function calls
+    /// Processes streaming updates from the OpenAI Responses API.
+    /// Buffers all function call items before executing them — this is critical for correctness:
+    /// if the model emits multiple parallel tool calls, firing a separate continuation per call
+    /// creates forked conversation branches. Collecting all calls and submitting all outputs
+    /// in a single continuation matches the non-streaming behavior.
     /// </summary>
     private async IAsyncEnumerable<(string text, string? responseId)> ProcessStreamingUpdatesAsync(
 #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.
@@ -187,10 +191,14 @@ private static string SanitizeForXmlContext(string? input) =>
         // Track this leg's response ID so tool-call continuations chain from it,
         // ensuring the model's context includes the user's message + reasoning.
         string? currentLegResponseId = null;
+#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.
+        List<FunctionCallResponseItem>? pendingFunctionCalls = null;
 
-        await foreach (var update in streamingUpdates.WithCancellation(cancellationToken))
+        // Wrap the raw stream to convert context-length API errors to our domain exception.
+        // C# does not allow yield inside a try/catch, so error remapping is done in a
+        // separate helper that puts try/catch only around MoveNextAsync.
+        await foreach (var update in RethrowContextLengthErrors(streamingUpdates, responseOptions.PreviousResponseId, cancellationToken))
         {
-#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.
             if (update is StreamingResponseCreatedUpdate created)
             {
                 // Emit the response ID early so the controller can record ownership
@@ -200,104 +208,124 @@ private static string SanitizeForXmlContext(string? input) =>
             }
             else if (update is StreamingResponseOutputItemDoneUpdate itemDone)
             {
-                // Check if this is a function call that needs to be executed
                 if (itemDone.Item is FunctionCallResponseItem functionCallItem && mcpClient != null)
                 {
                     if (toolCallDepth >= 10)
                         throw new InvalidOperationException("Maximum tool call depth exceeded.");
 
-                    // Chain the continuation from THIS leg's response ID so the model sees:
-                    //   context(outer) → userMessage + AI reasoning + funcCall (stored in currentLegResponseId)
-                    //   → [toolOutput only] → next response
-                    // Using the outer previousResponseId here would omit the user's message from context.
-                    var continuationOptions = CloneOptionsWithPreviousResponseId(responseOptions, currentLegResponseId);
-
-                    await foreach (var functionResult in ExecuteFunctionCallAsync(functionCallItem, continuationOptions, mcpClient, toolCallDepth + 1, endUserId, cancellationToken))
-                    {
-                        yield return functionResult;
-                    }
+                    // Buffer all function calls — do NOT fire continuations inline.
+                    // Sending one continuation per call would create N forked conversation
+                    // branches, each missing the other N-1 tool results.
+                    pendingFunctionCalls ??= [];
+                    pendingFunctionCalls.Add(functionCallItem);
                 }
             }
             else if (update is StreamingResponseOutputTextDeltaUpdate deltaUpdate)
             {
                 yield return (deltaUpdate.Delta.ToString(), null);
             }
-            else if (update is StreamingResponseCompletedUpdate)
+            // StreamingResponseCompletedUpdate: ResponseId already emitted above — no-op.
+        }
+
+        // After the stream completes, execute all buffered tool calls and send ALL outputs
+        // in a single continuation request. This mirrors the non-streaming loop in
+        // GetChatCompletionCore and avoids conversation branching.
+        if (pendingFunctionCalls is { Count: > 0 } && mcpClient != null)
+        {
+            var continuationOptions = CloneOptionsWithPreviousResponseId(responseOptions, currentLegResponseId);
+            var outputItems = new List<ResponseItem>(pendingFunctionCalls.Count);
+
+            foreach (var functionCallItem in pendingFunctionCalls)
             {
-                // ResponseId was already emitted from StreamingResponseCreatedUpdate at the start
-                // of this response leg — no need to emit again from the completion event.
+                outputItems.Add(await ExecuteSingleToolCallAsync(functionCallItem, toolCallDepth, endUserId, mcpClient, cancellationToken));
             }
+
+            var continuationStream = _ResponseClient.CreateResponseStreamingAsync(outputItems, continuationOptions, cancellationToken);
 #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.
+
+            await foreach (var result in ProcessStreamingUpdatesAsync(continuationStream, continuationOptions, mcpClient, toolCallDepth + 1, endUserId, cancellationToken))
+            {
+                yield return result;
+            }
         }
     }
 
     /// <summary>
-    /// Executes a function call and streams the response
+    /// Wraps a streaming response enumerable to remap <see cref="ClientResultException"/>
+    /// context-length errors to <see cref="ConversationContextLimitExceededException"/>.
+    /// <para>
+    /// C# prohibits <c>yield return</c> inside a <c>try</c> block with a <c>catch</c> clause
+    /// (CS1626). By putting the <c>try/catch</c> only around <c>MoveNextAsync</c> and the
+    /// <c>yield return</c> outside, we satisfy the compiler while still remapping exceptions.
+    /// </para>
     /// </summary>
-    private async IAsyncEnumerable<(string text, string? responseId)> ExecuteFunctionCallAsync(
 #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.
-        FunctionCallResponseItem functionCallItem,
-        ResponseCreationOptions responseOptions,
+    private static async IAsyncEnumerable<StreamingResponseUpdate> RethrowContextLengthErrors(
+        IAsyncEnumerable<StreamingResponseUpdate> source,
+        string? previousResponseId,
+        [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken = default)
+    {
+        await using var enumerator = source.GetAsyncEnumerator(cancellationToken);
+        while (true)
+        {
+            bool hasNext;
+            try { hasNext = await enumerator.MoveNextAsync(); }
+            catch (ClientResultException ex) when (IsContextLengthError(ex))
+            { throw new ConversationContextLimitExceededException(previousResponseId, ex); }
+
+            if (!hasNext) break;
+            yield return enumerator.Current; // yield return is outside try/catch — valid
+        }
+    }
 #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.
-        McpClient mcpClient,
+
+    /// <summary>
+    /// Executes a single MCP tool call and returns the output item to include in the
+    /// continuation request. Handles allowlist validation and argument-parsing errors
+    /// so a single bad tool never aborts the entire continuation.
+    /// </summary>
+#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.
+    private async Task<ResponseItem> ExecuteSingleToolCallAsync(
+        FunctionCallResponseItem functionCallItem,
         int toolCallDepth,
-        string? endUserId = null,
-        [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken = default)
+        string? endUserId,
+        McpClient mcpClient,
+        CancellationToken cancellationToken)
     {
         // Defense-in-depth: validate tool name against static allowlist before executing.
         if (!IsMcpToolAllowed(functionCallItem.FunctionName))
         {
             LogMcpToolCallRejected(_Logger, functionCallItem.FunctionName, endUserId);
-            // Feed a benign error back to the model so it can recover gracefully.
-            // The functionCallItem is in the stored response at PreviousResponseId — only send the output.
-#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.
-            var errorItems = new List<ResponseItem>
-            {
-                new FunctionCallOutputResponseItem(
-                    functionCallItem.CallId,
-                    $"Tool '{functionCallItem.FunctionName}' is not available.")
-            };
-#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.
-            var recoveryStream = _ResponseClient.CreateResponseStreamingAsync(
-                errorItems, responseOptions, cancellationToken);
-            await foreach (var result in ProcessStreamingUpdatesAsync(
-                recoveryStream, responseOptions, mcpClient, toolCallDepth, endUserId, cancellationToken))
-            {
-                yield return result;
-            }
-            yield break;
+            return new FunctionCallOutputResponseItem(
+                functionCallItem.CallId,
+                $"Tool '{functionCallItem.FunctionName}' is not available.");
         }
 
         LogMcpToolCallInvokedStream(_Logger, functionCallItem.FunctionName, toolCallDepth, endUserId);
-        // A dictionary of arguments to pass to the tool. Each key represents a parameter name, and its associated value represents the argument value.
-        var arguments = ParseToolArguments(functionCallItem.FunctionArguments);
 
-        // Execute the function call using the MCP client
+        Dictionary<string, object?> arguments;
+        try
+        {
+            arguments = ParseToolArguments(functionCallItem.FunctionArguments);
+        }
+        catch (Exception ex)
+        {
+            LogMcpToolArgumentParseError(_Logger, functionCallItem.FunctionName, ex, endUserId);
+            return new FunctionCallOutputResponseItem(
+                functionCallItem.CallId,
+                $"Error parsing arguments for '{functionCallItem.FunctionName}': invalid JSON.");
+        }
+
         var toolResult = await mcpClient.CallToolAsync(
             functionCallItem.FunctionName,
             arguments: arguments,
             cancellationToken: cancellationToken);
 
-        // The functionCallItem is already stored in the server's context (referenced by
-        // responseOptions.PreviousResponseId). Only the tool output is new content.
-#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.
-        var inputItems = new List<ResponseItem>
-        {
-            new FunctionCallOutputResponseItem(functionCallItem.CallId, McpToolResultFormatter.GetModelInput(toolResult))
-        };
-#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.
-
-        // Stream the function call response using the same processing logic
-        var functionResponseStream = _ResponseClient.CreateResponseStreamingAsync(
-            inputItems,
-            responseOptions,
-            cancellationToken);
-
-        await foreach (var result in ProcessStreamingUpdatesAsync(functionResponseStream, responseOptions, mcpClient, toolCallDepth, endUserId, cancellationToken))
-        {
-            yield return result;
-        }
+        return new FunctionCallOutputResponseItem(
+            functionCallItem.CallId,
+            McpToolResultFormatter.GetModelInput(toolResult));
     }
+#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.
 
     /// <summary>
     /// Creates response options with optional features
@@ -330,12 +358,11 @@ private async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
             options.PreviousResponseId = previousResponseId;
         }
 
-        // endUserId is reserved for forwarding to Azure OpenAI for end-user attribution
-        // (Microsoft Defender prompt-shield correlation). OpenAI .NET SDK v2.7.0 does not
-        // expose ResponseCreationOptions.User; this parameter is intentionally discarded
-        // until SDK support is available.
+        // Wire up end-user ID for Azure OpenAI abuse monitoring and Microsoft Defender
+        // prompt-shield correlation. The SDK now exposes EndUserId directly.
         // See: https://learn.microsoft.com/en-us/azure/defender-for-cloud/gain-end-user-context-ai
-        _ = endUserId;
+        if (!string.IsNullOrEmpty(endUserId))
+            options.EndUserId = endUserId;
 
         // Add tools if provided
         if (tools != null)
@@ -480,9 +507,10 @@ private bool IsMcpToolAllowed(string toolName)
     }
 
     /// <summary>
-    /// Returns a shallow clone of <paramref name="source"/> with
+    /// Returns a clone of <paramref name="source"/> with
     /// <see cref="ResponseCreationOptions.PreviousResponseId"/> replaced.
-    /// Used when chaining tool-call continuations from a specific response leg.
+    /// All behavior-affecting properties are copied so that tool-call continuation legs
+    /// produce identical generation behavior to the initial leg.
     /// </summary>
 #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.
     private static ResponseCreationOptions CloneOptionsWithPreviousResponseId(
@@ -493,10 +521,23 @@ private static ResponseCreationOptions CloneOptionsWithPreviousResponseId(
         {
             Instructions = source.Instructions,
             PreviousResponseId = previousResponseId,
+            EndUserId = source.EndUserId,
             ReasoningOptions = source.ReasoningOptions,
+            MaxOutputTokenCount = source.MaxOutputTokenCount,
+            TextOptions = source.TextOptions,
+            TruncationMode = source.TruncationMode,
+            ParallelToolCallsEnabled = source.ParallelToolCallsEnabled,
+            StoredOutputEnabled = source.StoredOutputEnabled,
+            ToolChoice = source.ToolChoice,
+            Temperature = source.Temperature,
+            TopP = source.TopP,
+            ServiceTier = source.ServiceTier,
         };
         foreach (var tool in source.Tools)
             clone.Tools.Add(tool);
+        if (source.Metadata is { Count: > 0 })
+            foreach (var kvp in source.Metadata)
+                clone.Metadata[kvp.Key] = kvp.Value;
         return clone;
     }
 #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.
@@ -538,6 +579,9 @@ private static ResponseCreationOptions CloneOptionsWithPreviousResponseId(
     [LoggerMessage(Level = LogLevel.Warning, Message = "AI tool call rejected — not on allowlist: tool={ToolName} user={EndUserId}")]
     private static partial void LogMcpToolCallRejected(ILogger logger, string toolName, string? endUserId);
 
+    [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to parse tool arguments for '{ToolName}': user={EndUserId}")]
+    private static partial void LogMcpToolArgumentParseError(ILogger logger, string toolName, Exception exception, string? endUserId);
+
     [LoggerMessage(Level = LogLevel.Warning, Message = "MCP tool skipped during option setup — not on allowlist: tool={ToolName}")]
     private static partial void LogMcpToolSkippedNotAllowed(ILogger logger, string toolName);
 
@@ -546,10 +590,41 @@ private static ResponseCreationOptions CloneOptionsWithPreviousResponseId(
 
     /// <summary>
     /// Returns <c>true</c> when the API error indicates the conversation context window was exceeded.
+    /// Prefers structured JSON error code from the response body; falls back to message text matching.
+    /// Also handles HTTP 413 (payload too large via API gateway) and <c>token_limit_exceeded</c>.
+    /// </summary>
+    private static bool IsContextLengthError(ClientResultException ex)
+    {
+        if (ex.Status is not (400 or 413)) return false;
+
+        // Prefer structured error code from the response body
+        var errorCode = TryExtractErrorCode(ex);
+        if (errorCode is not null)
+            return errorCode is "context_length_exceeded" or "token_limit_exceeded";
+
+        // Fallback: substring match on exception message (Azure OpenAI format may vary)
+        return ex.Message.Contains("context_length_exceeded", StringComparison.OrdinalIgnoreCase) ||
+               ex.Message.Contains("maximum context length", StringComparison.OrdinalIgnoreCase) ||
+               ex.Message.Contains("reduce the length of the messages", StringComparison.OrdinalIgnoreCase) ||
+               ex.Message.Contains("token_limit_exceeded", StringComparison.OrdinalIgnoreCase);
+    }
+
+    /// <summary>
+    /// Attempts to extract the <c>error.code</c> field from the raw JSON response body.
+    /// Returns <c>null</c> on any parse failure — this is best-effort.
     /// </summary>
-    private static bool IsContextLengthError(ClientResultException ex) =>
-        ex.Status == 400 &&
-        (ex.Message.Contains("context_length_exceeded", StringComparison.OrdinalIgnoreCase) ||
-         ex.Message.Contains("maximum context length", StringComparison.OrdinalIgnoreCase) ||
-         ex.Message.Contains("reduce the length of the messages", StringComparison.OrdinalIgnoreCase));
+    private static string? TryExtractErrorCode(ClientResultException ex)
+    {
+        try
+        {
+            var content = ex.GetRawResponse()?.Content;
+            if (content is null) return null;
+            using var doc = System.Text.Json.JsonDocument.Parse(content.ToMemory());
+            if (doc.RootElement.TryGetProperty("error", out var error) &&
+                error.TryGetProperty("code", out var code))
+                return code.GetString();
+        }
+        catch { /* best-effort — don't let error parsing crash the error handler */ }
+        return null;
+    }
 }