fix(sse): restore abort mapping, cache telemetry and fence safety on the non-streaming leg

changelog.d/fixes/chatcore-nonstreaming-regressions.md

@@ -0,0 +1 @@
+- Restore four non-streaming behaviours the server-owned tool loop refactor dropped: client aborts map to 499 with the fixed `Request aborted` message, an aborted request no longer logs a synthetic `clientResponse`, Claude prompt-cache telemetry is recorded again, and a body that cannot be canonicalized no longer throws when the tool loop is off.

open-sse/handlers/chatCore.ts

@@ -4927,7 +4927,14 @@ export async function handleChatCore({
           error: err.error || "Provider request failed",
           providerRequest: finalBody || translatedBody,
           providerResponse: isNetworkThrow ? undefined : err.response,
-          clientResponse: buildErrorBody(err.status, err.error || "Provider request failed"),
+          // On a client abort the client already disconnected before we got here, so this
+          // body is what we WOULD have sent, not what was delivered. The dashboard reads
+          // `clientResponse` as "what the client received", so logging it misleads —
+          // `error` above already records the reason. The pre-#12867 path omitted it here;
+          // the leg-based path must keep doing so.
+          clientResponse: isLocalStreamLifecycleError(err.originalError)
+            ? undefined
+            : buildErrorBody(err.status, err.error || "Provider request failed"),
           cacheSource: "upstream",
         });
         persistFailureUsage(err.status, err.errorCode || `upstream_${err.status}`);
@@ -4939,8 +4946,34 @@ export async function handleChatCore({
       const expectedConn = managedLease
         ? String(getCurrentConnectionId() || connectionId || "") || undefined
         : undefined;
+      // The identity is the tool loop's execution fence key, and deriveToolRequestIdentity
+      // canonicalizes the body — which by design rejects Dates, Maps and class instances.
+      // It was computed eagerly, so a body carrying any of those threw on EVERY
+      // non-streaming request even with SERVER_OWNED_TOOL_LOOP_ENABLED off (the default).
+      // Derive it only when the loop can run, and fail closed rather than crash: no
+      // identity means no fence, and without a fence the loop must not run.
+      let toolLoopEnabled = isServerOwnedToolLoopEnabled();
+      let postInjectionRequestIdentity = "";
+      if (toolLoopEnabled) {
+        try {
+          postInjectionRequestIdentity = derivePostInjectionRequestIdentity({
+            apiKeyId: memoryOwnerId || "local",
+            headers: clientRawRequest?.headers ?? null,
+            skillRequestId,
+            postInjectionBody: (body || {}) as Record<string, unknown>,
+          });
+        } catch (identityError) {
+          log?.warn?.(
+            "SERVER_OWNED_TOOL_LOOP",
+            `request body is not canonicalizable, skipping the loop: ${
+              identityError instanceof Error ? identityError.message : "unknown"
+            }`
+          );
+          toolLoopEnabled = false;
+        }
+      }
       const loopApply = await applyServerOwnedToolLoopIfNeeded({
-        enabled: isServerOwnedToolLoopEnabled(),
+        enabled: toolLoopEnabled,
         stream,
         isResponsesEndpoint,
         sourceFormat,
@@ -4951,12 +4984,7 @@ export async function handleChatCore({
           apiKeyId: memoryOwnerId || "local",
           sessionId: pipelineSessionId,
           requestId: skillRequestId,
-          requestIdentity: derivePostInjectionRequestIdentity({
-            apiKeyId: memoryOwnerId || "local",
-            headers: clientRawRequest?.headers ?? null,
-            skillRequestId,
-            postInjectionBody: (body || {}) as Record<string, unknown>,
-          }),
+          requestIdentity: postInjectionRequestIdentity,
           builtinToolNames: injectionResult.builtinToolNames,
           injectedCustomSkillNames: injectionResult.injectedCustomSkillNames,
           customSkillExecutionEnabled:
@@ -5066,6 +5094,16 @@ export async function handleChatCore({
         providerHeaders = normalizeHeaders(okLeg.headers);
       }
       finalBody = providerRequestCapture.body(okLeg.providerRequest || translatedBody);
+      // Built inside executeProviderRequest on the pre-#12867 path. The leg now owns the
+      // first non-streaming send, so that assignment never runs here and the meta stayed
+      // null — `_omniroute.claudePromptCache` silently vanished from every call log on
+      // this path. Same inputs, same helper, at the point where they are available.
+      claudePromptCacheLogMeta = buildClaudePromptCacheLogMeta(
+        targetFormat,
+        finalBody,
+        providerHeaders,
+        clientRawRequest?.headers
+      );
       const capturedOk = providerRequestCapture.latest?.();
       reqLogger.logTargetRequest(
         okLeg.requestUrl || capturedOk?.url || "",

open-sse/handlers/chatCore/nonStreamingProviderLeg.ts

@@ -23,6 +23,7 @@ import { restoreNonStreamingToolNames } from "./passthroughToolNames.ts";
 import { extractUsageFromResponse } from "../usageExtractor.ts";
 import { sanitizeUsagePayloadForRequest } from "../../utils/usageTracking.ts";
 import { createErrorResult, formatProviderError } from "../../utils/error.ts";
+import { isLocalStreamLifecycleError } from "@/shared/utils/circuitBreaker";
 import { unwrapClinepassEnvelope } from "../../utils/clinepassEnvelope.ts";
 import { unwrapClineNonStreamingEnvelope } from "./clineResponseEnvelope.ts";
 import {
@@ -232,7 +233,6 @@ function parseRetryAfterMs(response: Response): number | null {
   return null;
 }
 
-
 function finishOk(
   input: ProviderLegInput,
   params: {
@@ -462,14 +462,22 @@ export async function runNonStreamingProviderLeg(
     ) {
       throw error;
     }
-    const failureStatus =
-      error instanceof Error && error.name === "AbortError"
-        ? 499
-        : error instanceof Error && error.name === "TimeoutError"
-          ? 504
-          : 502;
-    const failureMessage =
-      error instanceof Error
+    // `abort(reason)` can reject with a raw string that has no `name`/`status`, so
+    // `error.name === "AbortError"` is too narrow — that shape fell through to the 502
+    // provider-failure default (#7907). chatCore classified this through
+    // isLocalStreamLifecycleError before this leg took over the first send; mirror it.
+    const isRequestAborted = isLocalStreamLifecycleError(error);
+    const failureStatus = isRequestAborted
+      ? 499
+      : error instanceof Error && error.name === "TimeoutError"
+        ? 504
+        : 502;
+    // A client abort is not a provider failure: formatProviderError would stamp the raw
+    // upstream text as `[499]: <reason>`, leaking it to the client. chatCore has always
+    // normalized this to the fixed "Request aborted".
+    const failureMessage = isRequestAborted
+      ? "Request aborted"
+      : error instanceof Error
         ? formatProviderError(error, provider, currentModel, failureStatus)
         : "Provider request failed";
     const receipt = buildReceipt(input, {