fix(sse): drop empty-choices chunks without usage instead of injecting retry text (#3502)

open-sse/utils/stream.ts

@@ -1674,8 +1674,14 @@ export function createSSEStream(options: StreamOptions = {}) {
                   // OpenAI-compatible streaming with `stream_options.include_usage=true`
                   // ends with a usage-only chunk where `choices` is deliberately `[]`.
                   // Forward that standards-compliant chunk instead of turning it into an
-                  // empty-response error. Keep the existing hardening for malformed empty
-                  // choices chunks without valid usage.
+                  // empty-response error.
+                  //
+                  // For a malformed empty `choices: []` chunk WITHOUT valid usage we DROP
+                  // it (log server-side only). We must NOT inject an assistant-content
+                  // chunk like "[OmniRoute] Upstream returned an empty response. Please
+                  // retry." with finish_reason: "stop" — clients (Goose/opencode) feed that
+                  // text back as a turn and spin in a retry loop. This restores the #3400
+                  // behavior that #3422 inadvertently reverted (regression #3388/#3502).
                   if (Array.isArray(parsed.choices) && parsed.choices.length === 0) {
                     const emptyChoicesUsage = extractUsage(parsed) ?? parsed.usage;
                     if (hasValidUsage(emptyChoicesUsage)) {
@@ -1690,29 +1696,8 @@ export function createSSEStream(options: StreamOptions = {}) {
                     }
 
                     console.warn(
-                      `[STREAM] Upstream returned empty choices array (${provider || "provider"}:${model || "unknown"}) — emitting error chunk`
+                      `[STREAM] Upstream returned empty choices array (${provider || "provider"}:${model || "unknown"}) — dropping chunk`
                     );
-                    const errorChunk = {
-                      id: parsed.id || `omniroute-empty-choices-${Date.now()}`,
-                      object: "chat.completion.chunk",
-                      created: parsed.created || Math.floor(Date.now() / 1000),
-                      model: parsed.model || model || "unknown",
-                      choices: [
-                        {
-                          index: 0,
-                          delta: {
-                            role: "assistant",
-                            content: "[OmniRoute] Upstream returned an empty response. Please retry.",
-                          },
-                          finish_reason: "stop",
-                        },
-                      ],
-                    };
-                    output = `data: ${JSON.stringify(errorChunk)}\n`;
-                    injectedUsage = true;
-                    clientPayload = errorChunk;
-                    reqLogger?.appendConvertedChunk?.(output);
-                    controller.enqueue(encoder.encode(output));
                     continue;
                   }
 

tests/unit/empty-choices-no-inject.test.ts

@@ -0,0 +1,100 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-empty-inject-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+const core = await import("../../src/lib/db/core.ts");
+
+const { createSSEStream } = await import("../../open-sse/utils/stream.ts");
+const { FORMATS } = await import("../../open-sse/translator/formats.ts");
+
+const textEncoder = new TextEncoder();
+
+async function readTransformed(chunks: string[], options: Record<string, unknown>) {
+  const source = new ReadableStream({
+    start(controller) {
+      for (const chunk of chunks) {
+        controller.enqueue(textEncoder.encode(chunk));
+      }
+      controller.close();
+    },
+  });
+  return new Response(source.pipeThrough(createSSEStream(options as never))).text();
+}
+
+test.after(() => {
+  core.resetDbInstance();
+  if (fs.existsSync(TEST_DATA_DIR)) {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  }
+});
+
+// Regression guard for #3502 / #3388: PR #3422 ("allow OpenAI usage-only empty
+// choices chunks") re-introduced the assistant-content injection
+// "[OmniRoute] Upstream returned an empty response. Please retry." for empty
+// `choices: []` chunks that carry NO valid usage. That injected text is fed back
+// by clients (Goose/opencode) as a turn, producing the retry loop #3400 fixed.
+// An empty-no-usage chunk must be DROPPED, never injected as content.
+test("empty choices WITHOUT usage are dropped, not injected as retry text (#3502)", async () => {
+  const text = await readTransformed(
+    [
+      `data: ${JSON.stringify({
+        id: "chatcmpl_x",
+        object: "chat.completion.chunk",
+        created: 1,
+        model: "mimo-v2.5-free",
+        choices: [],
+      })}\n\n`,
+      `data: [DONE]\n\n`,
+    ],
+    {
+      mode: "passthrough",
+      sourceFormat: FORMATS.OPENAI,
+      provider: "opencode-zen",
+      model: "mimo-v2.5-free",
+      body: { messages: [{ role: "user", content: "hi" }] },
+    }
+  );
+
+  assert.doesNotMatch(text, /Upstream returned an empty response/);
+  assert.doesNotMatch(text, /Please retry/);
+});
+
+// #3422 behavior that MUST be preserved: a standards-compliant
+// `stream_options.include_usage` final chunk has `choices: []` WITH usage and
+// must be forwarded (not dropped, not turned into an error).
+test("empty choices WITH valid usage are forwarded (preserve #3422)", async () => {
+  const text = await readTransformed(
+    [
+      `data: ${JSON.stringify({
+        id: "chatcmpl_y",
+        object: "chat.completion.chunk",
+        created: 1,
+        model: "gpt-4.1-mini",
+        choices: [{ index: 0, delta: { role: "assistant", content: "hello" } }],
+      })}\n\n`,
+      `data: ${JSON.stringify({
+        id: "chatcmpl_y",
+        object: "chat.completion.chunk",
+        created: 1,
+        model: "gpt-4.1-mini",
+        choices: [],
+        usage: { prompt_tokens: 5, completion_tokens: 3, total_tokens: 8 },
+      })}\n\n`,
+      `data: [DONE]\n\n`,
+    ],
+    {
+      mode: "passthrough",
+      sourceFormat: FORMATS.OPENAI,
+      provider: "openai",
+      model: "gpt-4.1-mini",
+      body: { messages: [{ role: "user", content: "hi" }] },
+    }
+  );
+
+  assert.doesNotMatch(text, /Upstream returned an empty response/);
+  assert.match(text, /"total_tokens":8/);
+});

tests/unit/stream-utils.test.ts

@@ -1656,10 +1656,12 @@ test("createSSEStream passthrough drops empty choices array chunks", async () =>
     }
   );
 
-  // Empty choices without usage still get replaced with a synthetic error chunk.
-  assert.match(text, /\[OmniRoute\] Upstream returned an empty response/);
-  assert.match(text, /"finish_reason":"stop"/);
-  // Subsequent valid chunks should still be present
+  // Empty choices WITHOUT usage are DROPPED, never replaced with a synthetic
+  // "[OmniRoute] Upstream returned an empty response. Please retry." chunk. That
+  // injection (reintroduced by #3422) was fed back by clients as a turn and caused
+  // the retry loop #3388/#3502, which #3400 had fixed by dropping the chunk.
+  assert.doesNotMatch(text, /\[OmniRoute\] Upstream returned an empty response/);
+  // Subsequent valid chunks must still pass through untouched.
   assert.match(text, /"content":"Hello"/);
   assert.match(text, /"finish_reason":"stop"/);
   assert.equal(onCompletePayload.status, 200);