Merge pull request #793 from Wibias/restore/773-openai-chat-eof

fix(openai-chat): complete streams that end after output without terminal

Author: Wibias

src/adapters/openai-chat.ts

@@ -697,6 +697,8 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
       const pendingToolCalls: PendingToolCall[] = [];
       let toolCallSeq = 0;
       const flushToolCalls = function* (): Generator<AdapterEvent> {
+        // Do not treat flushed tool calls as user-facing output for the finish-less EOF
+        // fallback — incomplete tool args must stay on the truncation path.
         for (const call of pendingToolCalls) {
           if (!call.id) call.id = `call_${++toolCallSeq}`;
           yield { type: "tool_call_start", id: call.id, name: call.name };
@@ -711,6 +713,9 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
       // explicit `[DONE]` sentinel OR a chunk carrying a non-null `finish_reason` (some
       // OpenAI-compatible providers omit `[DONE]` but do send finish_reason).
       let finishReason: string | undefined;
+      // Only answer text enables the finish-less EOF fallback. Reasoning-only streams can be
+      // suppressed by hideThinkingSummary and must not complete as empty successful turns.
+      let sawUserFacingOutput = false;
 
       // Single per-line handler shared by the streaming loop and the EOF residual-frame flush, so
       // a final frame is parsed identically wherever it lands (no duplicated, drift-prone parsing).
@@ -780,6 +785,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
             yield { type: "reasoning_raw_delta", text: delta.reasoning_content };
           }
           if (typeof delta.content === "string" && delta.content.length > 0) {
+            sawUserFacingOutput = true;
             yield { type: "text_delta", text: delta.content };
           }
 
@@ -837,10 +843,8 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
         if (buffer.length > 0) {
           if ((yield* handleDataLine(buffer)) === "terminate") return;
         }
-        // Reader EOF. A graceful close shows at least one terminal signal: `[DONE]` (returns above),
-        // a non-null finish_reason (sawFinish), or a trailing usage chunk (providers emit usage only
-        // at end-of-generation). If NONE of those were seen, the stream was cut mid-flight — fail
-        // closed so the bridge emits a classified response.failed rather than a silent truncation.
+        // Reader EOF. Prefer failing closed before flushing pending tool calls so the bridge
+        // never sees a fabricated tool_call_end on a truncated mid-assembly stream.
         //
         // Checked BEFORE flushToolCalls(), because that helper emits tool_call_end and there is no
         // taking it back: a half-assembled argument string would reach the client as a completed
@@ -856,16 +860,19 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
           yield { type: "error", message: "upstream stream ended mid tool call without a terminal signal — possible truncation" };
           return;
         }
-        yield* flushToolCalls();
-        if (!sawFinish && pendingUsage === undefined) {
+        // Finish-less EOF is only safe when answer text was emitted. Reasoning-only / usage-only
+        // truncations must stay on the error path (hideThinkingSummary can suppress reasoning).
+        // Trailing usage alone is not a terminal signal for this adapter (#735 / restore #773).
+        if (!sawFinish && !sawUserFacingOutput) {
           debugProviderDiagnostic("openai-chat", "stream-truncated", {
             finishReason: finishReason ?? null,
-            hadUsage: false,
+            hadUsage: pendingUsage !== undefined,
           });
           yield { type: "error", message: "upstream stream ended without a terminal signal ([DONE] or finish_reason) — possible truncation" };
           return;
         }
-        // Graceful close that omitted [DONE] but delivered finish_reason and/or final usage.
+        yield* flushToolCalls();
+        // Graceful close that omitted [DONE] but delivered finish_reason and/or answer text.
         const stopReason = finishReason === "length"
           ? "max_tokens"
           : finishReason === "content_filter"

tests/openai-chat-eof.test.ts

@@ -12,11 +12,18 @@ async function collect(gen: AsyncGenerator<AdapterEvent>): Promise<AdapterEvent[
 }
 
 describe("openai-chat stream EOF fail-closed", () => {
-  test("truncated stream (no [DONE], no finish_reason) yields a terminal error, not a clean done", async () => {
+  test("truncated stream (no [DONE], no finish_reason) yields done when content was emitted", async () => {
     const response = new Response('data: {"choices":[{"delta":{"content":"par"}}]}\n\n');
     const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
     const last = events[events.length - 1];
-    expect(last.type).toBe("error");
+    expect(last.type).toBe("done");
+    expect(events.some(e => e.type === "error")).toBe(false);
+  });
+
+  test("empty EOF without content still errors", async () => {
+    const response = new Response("");
+    const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
+    expect(events.at(-1)?.type).toBe("error");
     expect(events.some(e => e.type === "done")).toBe(false);
   });
 
@@ -128,10 +135,49 @@ describe("openai-chat stream EOF fail-closed", () => {
     expect(events.some(e => e.type === "error")).toBe(false);
   });
 
-  test("genuinely truncated stream WITHOUT a trailing newline still fails closed", async () => {
-    // Mid-content frame, no terminator, no newline — must remain a terminal error.
+  test("genuinely truncated stream WITHOUT a trailing newline completes when content was emitted", async () => {
+    // Mid-content frame, no terminator, no newline — content was yielded, so accept done.
     const response = new Response('data: {"choices":[{"delta":{"content":"par"}}]}');
     const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
+    expect(events.at(-1)?.type).toBe("done");
+    expect(events.some(e => e.type === "error")).toBe(false);
+  });
+
+  test("EOF with pending tool calls and no finish_reason fails closed", async () => {
+    const response = new Response(
+      'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"get_weather","arguments":"{\\"a\\":"}}]}}]}\n\n',
+    );
+    const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
+    expect(events.at(-1)?.type).toBe("error");
+    expect(events.some(e => e.type === "done")).toBe(false);
+    expect(events.some(e => e.type === "tool_call_end")).toBe(false);
+  });
+
+  test("reasoning-only EOF without finish_reason fails closed", async () => {
+    const response = new Response(
+      'data: {"choices":[{"delta":{"reasoning_content":"thinking..."}}]}\n\n',
+    );
+    const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
+    expect(events.at(-1)?.type).toBe("error");
+    expect(events.some(e => e.type === "done")).toBe(false);
+  });
+
+  test("usage-only EOF with pending tool calls fails closed", async () => {
+    const response = new Response(
+      'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"get_weather","arguments":"{}"}}]}}]}\n\n' +
+        'data: {"choices":[],"usage":{"prompt_tokens":1,"completion_tokens":2,"total_tokens":3}}\n\n',
+    );
+    const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
+    expect(events.at(-1)?.type).toBe("error");
+    expect(events.some(e => e.type === "done")).toBe(false);
+    expect(events.some(e => e.type === "tool_call_end")).toBe(false);
+  });
+
+  test("usage-only EOF without answer text fails closed", async () => {
+    const response = new Response(
+      'data: {"choices":[],"usage":{"prompt_tokens":1,"completion_tokens":2,"total_tokens":3}}\n\n',
+    );
+    const events = await collect(createOpenAIChatAdapter(provider).parseStream(response));
     expect(events.at(-1)?.type).toBe("error");
     expect(events.some(e => e.type === "done")).toBe(false);
   });

tests/sidebar-routes.test.ts

@@ -51,6 +51,11 @@ describe("GET /api/update/badge", () => {
 });
 
 describe("GET /api/github/star", () => {
+  // These hit a real `gh` spawn (auth then api). AUTH_TIMEOUT_MS is 5s and API_TIMEOUT_MS
+  // is 10s in star-state.ts — Bun's default 5s test budget races the auth kill on a slow
+  // Windows credential helper and flakes as a timeout even when spawnGh is correct.
+  const STAR_ROUTE_TIMEOUT_MS = 20_000;
+
   test("is routed and reports one of the three known states", async () => {
     invalidateStarStatusCache();
     const { status, body } = await call("GET", "/api/github/star");
@@ -59,7 +64,7 @@ describe("GET /api/github/star", () => {
     expect(["starred", "not-starred", "unauthenticated"]).toContain(star.state);
     expect(star.repo).toBe("lidge-jun/opencodex");
     expect(star.url).toBe("https://github.com/lidge-jun/opencodex");
-  });
+  }, { timeout: STAR_ROUTE_TIMEOUT_MS });
 
   test("never serializes gh output, tokens, or account identifiers", async () => {
     invalidateStarStatusCache();
@@ -71,7 +76,7 @@ describe("GET /api/github/star", () => {
     expect(raw.toLowerCase()).not.toContain("scope");
     expect(raw).not.toContain("gho_");
     expect(raw).not.toContain("ghp_");
-  });
+  }, { timeout: STAR_ROUTE_TIMEOUT_MS });
 });
 
 describe("route surface", () => {