Merge branch 'dev' into fix/764-winsw-env-and-liveness-probe

Author: Wibias

devlog/_plan/260731_pr_issue_triage_round/050_windows_ci_flake_rca.md

@@ -0,0 +1,82 @@
+# 050 — windows-latest 플레이크 RCA
+
+CI가 세 번 연속 빨간데 **매번 다른 테스트**가 죽었다. 이런 모양이면 보통 원인이
+테스트가 아니라 환경이다. 기록해두는 이유는, 다음에 이 색깔을 보는 사람이 같은
+진단을 처음부터 다시 하지 않도록 하기 위해서다.
+
+## 관측
+
+| 커밋 | 죽은 테스트 | 시간 |
+|---|---|---|
+| `bf9bc1ac8` | Windows tray packaging — detached tray host가 listen 소켓을 안 물고 뜨는지 | — |
+| `f2b61ee7e` | `GET /api/github/star` ×2 | 5015ms, 5001ms |
+| `6e1cdb7de` | server local API auth — pool retry 인가 안 함 ×2 | 5353ms, 412ms |
+
+`bf9bc1ac8`은 **내 푸시 이전** 커밋이다. 즉 이 라운드가 만든 문제가 아니다.
+셋 다 로컬에서 통과하고, 셋 다 해당 파일이 그 푸시의 diff에 없다.
+
+공통점은 5초다. Bun의 테스트 예산이 5초이고, 세 건 모두 **러너가 통제하는 무언가를
+기다리다** 그 예산을 넘겼다. 다만 기다리는 대상은 서로 다르다 — 하나의 원인으로
+묶으려다 틀리는 것보다 셋을 따로 보는 게 맞았다.
+
+## 원인 1 — sidebar: 진짜 `gh`를 띄운다
+
+`star-state.ts:55`의 `spawnGh()`가 사용자의 실제 `gh` 프로세스를 띄운다.
+`AUTH_TIMEOUT_MS = 5_000`.
+
+여기서 중요한 건, **이미 한 번 고쳐진 적이 있다는 것**이다. `0af17fbfd`가
+Windows `.cmd` shim 해석을 `commandInvocation`으로 돌렸고, 코드 주석이 그 사실을
+명시한다:
+
+> On Windows `gh` is a `.cmd` shim ... which is how these sidebar tests turned into
+> 5s timeouts on windows-latest while passing everywhere else.
+
+그런데 `f2b61ee7e`는 `0af17fbfd` **이후**인데도 같은 자리에서 죽었다. 바이너리를
+정확히 찾아주는 것과, 부하 걸린 러너에서 그 프로세스가 빨리 끝나는 것은 다른
+문제다. 첫 수정은 필요했지만 충분하지 않았다.
+
+진짜 문제는 **라우트 테스트가 외부 바이너리를 띄운다는 사실 자체**다. `gh`의 설치
+여부, 인증 헬퍼, Windows shim은 전부 라우트 계약 밖이다.
+
+수정: `star-state.ts`에 이미 있던 `StarDeps`의 주입 가능한 `runGh`를
+`setStarDepsForTests()`로 선택한다. 테스트는 자기가 실제로 주장하는 것 —
+라우트 도달 가능성, 응답 형태, 그리고 `gh` 출력·토큰·계정 식별자가 절대
+직렬화되지 않는다는 것 — 을 결정적인 fake로 검증한다. 5초가 0.25ms가 됐다.
+프로덕션은 진짜 러너를 그대로 쓴다.
+
+## 원인 2 — server auth: 한 테스트가 하니스를 네 번 띄운다
+
+malformed-detail 케이스 네 개가 각각 프록시/업스트림 하니스를 새로 세웠다.
+Windows에서는 그 기동 비용만으로 예산이 찼고, 마지막 요청이 아직 날아가는 중에
+다음 테스트가 시작돼 전역 `fetch`를 두고 경합했다. 412ms짜리 실패가 그 흔적이다.
+
+수정: 하니스 하나를 네 케이스가 공유한다. 각 케이스는 여전히 자기 원본 400과
+`acct-pool-a` 단일 dispatch를 증명한다.
+
+## 원인 3 — tray: 안 고쳤다
+
+`windows-tray.test.ts:247`이 PowerShell을 띄우고, 거기서 Bun 자식을 띄우고,
+같은 포트에 다시 bind한다. **자식이 listen 소켓을 물려받지 않는다는 것을 증명하는
+게 이 테스트의 존재 이유다.**
+
+결정적으로 만들려면 `src/tray/windows.ts:464`에 프로세스 기동 seam이 필요하다.
+테스트에서만 흉내내면 증명이 사라진다 — 소켓 상속은 진짜 프로세스를 띄워야만
+관측되는 성질이다. 그래서 손대지 않았다.
+
+**이건 미해결이다.** 초록으로 만들 수 있었지만 그렇게 하면 테스트가 지키던 것을
+버리는 것이다.
+
+## 하지 않은 것
+
+타임아웃을 올리지 않았다. 테스트를 skip하지 않았다. assertion을 지우지 않았다.
+셋 다 빨간색을 없애지만 신뢰성 신호를 침묵으로 바꾼다. `020`에서 무력한 테스트
+세 건을 지적해놓고 여기서 같은 짓을 하면 앞뒤가 안 맞는다.
+
+## 인접 작업
+
+PR #801(luvs01)이 Windows PowerShell 프로세스 열거 플레이크를 다룬다 — CIM
+열거가 첫 시도에 빈 결과를 주면 한 번 재시도. 우리가 고친 것과 다른 지점이고
+충돌하지 않는다.
+
+PR #805(Wibias)가 #764의 `--native` 잔여를 닫는다. 지난 라운드에서 내가 열어둔
+바로 그 절반이다.

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"

src/github/star-state.ts

@@ -79,19 +79,8 @@ async function spawnGh(args: string[], timeoutMs: number): Promise<{ status: num
   }
 }
 
-const defaultDeps: StarDeps = { runGh: spawnGh, nowMs: () => Date.now() };
-/** Test-only override so route tests never spawn a real `gh` (Windows CI hangs at AUTH_TIMEOUT). */
-let depsOverride: StarDeps | null = null;
-
-function activeDeps(deps?: StarDeps): StarDeps {
-  return deps ?? depsOverride ?? defaultDeps;
-}
-
-/** Swap the default `gh` runner for tests; pass `null` to restore production deps. */
-export function setStarDepsForTests(deps: StarDeps | null): void {
-  depsOverride = deps;
-  invalidateStarStatusCache();
-}
+const productionDeps: StarDeps = { runGh: spawnGh, nowMs: () => Date.now() };
+let defaultDeps = productionDeps;
 
 let cached: { timestamp: number; state: StarState } | null = null;
 /** Coalesces concurrent probes so parallel sidebar polls share one `gh` run. */
@@ -109,11 +98,10 @@ let generation = 0;
  * starred and 404 when not, so a non-zero exit is only meaningful once we know
  * the CLI is authenticated — hence the auth check first.
  */
-export async function probeStarState(deps?: StarDeps): Promise<StarState> {
-  const d = activeDeps(deps);
-  const auth = await d.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
+export async function probeStarState(deps: StarDeps = defaultDeps): Promise<StarState> {
+  const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
   if (!auth || auth.status !== 0) return "unauthenticated";
-  const starred = await d.runGh(
+  const starred = await deps.runGh(
     ["api", "--hostname", GH_HOSTNAME, `/user/starred/${STAR_REPO}`],
     API_TIMEOUT_MS,
   );
@@ -122,9 +110,8 @@ export async function probeStarState(deps?: StarDeps): Promise<StarState> {
 }
 
 /** Cached star state; `gh` is only spawned when the cache is cold or expired. */
-export async function getStarStatus(deps?: StarDeps): Promise<StarStatus> {
-  const d = activeDeps(deps);
-  const now = d.nowMs();
+export async function getStarStatus(deps: StarDeps = defaultDeps): Promise<StarStatus> {
+  const now = deps.nowMs();
   if (cached && now - cached.timestamp < CACHE_TTL_MS) {
     return { state: cached.state, repo: STAR_REPO, url: STAR_REPO_URL };
   }
@@ -137,7 +124,7 @@ export async function getStarStatus(deps?: StarDeps): Promise<StarStatus> {
     // The slot is cleared inside the same continuation that commits the cache. Using
     // `.finally()` for that defers it by a microtask, which leaves a window where the
     // next caller awaits an already-settled probe instead of starting a fresh one.
-    const probe = probeStarState(d).then(
+    const probe = probeStarState(deps).then(
       state => {
         if (inflight === probe) inflight = null;
         // A write landed while this read was in flight — its result is authoritative.
@@ -162,6 +149,17 @@ export function invalidateStarStatusCache(): void {
   inflight = null;
 }
 
+/**
+ * Route tests must not launch the user's `gh` executable: its installation,
+ * authentication helper, and Windows shim are all outside the route contract.
+ * Production keeps the real runner; tests install an explicit deterministic
+ * dependency and must reset it afterwards.
+ */
+export function setStarDepsForTests(deps: StarDeps | null): void {
+  defaultDeps = deps ?? productionDeps;
+  invalidateStarStatusCache();
+}
+
 /**
  * Star the repository through the user's `gh` login. Returns the resulting
  * state so the caller does not need a second round trip; an unauthenticated
@@ -173,20 +171,19 @@ export function invalidateStarStatusCache(): void {
  * management API.
  */
 export async function starRepository(
-  deps?: StarDeps,
+  deps: StarDeps = defaultDeps,
 ): Promise<{ ok: boolean; status: StarStatus; code?: StarErrorCode }> {
-  const d = activeDeps(deps);
-  const auth = await d.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
+  const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
   if (!auth || auth.status !== 0) {
     generation += 1;
-    cached = { timestamp: d.nowMs(), state: "unauthenticated" };
+    cached = { timestamp: deps.nowMs(), state: "unauthenticated" };
     return {
       ok: false,
       status: { state: "unauthenticated", repo: STAR_REPO, url: STAR_REPO_URL },
       code: "gh_unavailable",
     };
   }
-  const result = await d.runGh(
+  const result = await deps.runGh(
     ["api", "--hostname", GH_HOSTNAME, "-X", "PUT", `/user/starred/${STAR_REPO}`],
     API_TIMEOUT_MS,
   );
@@ -201,6 +198,6 @@ export async function starRepository(
   // Authoritative: this call just starred the repo. Bumping the generation makes any
   // read that is still in flight discard its now-obsolete observation.
   generation += 1;
-  cached = { timestamp: d.nowMs(), state: "starred" };
+  cached = { timestamp: deps.nowMs(), state: "starred" };
   return { ok: true, status: { state: "starred", repo: STAR_REPO, url: STAR_REPO_URL } };
 }

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/server-auth.test.ts

@@ -1955,14 +1955,18 @@ describe("server local API auth", () => {
   });
 
   test("missing or non-string detail never authorizes a pool retry", async () => {
-    for (const body of ["{}", '{"detail":null}', '{"detail":400}', '{"detail":{"message":"unsupported"}}']) {
-      const harness = await startPoolRetryHarness(() => rejectionResponse(body));
-      try {
+    const bodies = ["{}", '{"detail":null}', '{"detail":400}', '{"detail":{"message":"unsupported"}}'];
+    let nextBody = 0;
+    const harness = await startPoolRetryHarness(() => rejectionResponse(bodies[nextBody++]!));
+    try {
+      for (const body of bodies) {
+        const priorDispatches = harness.dispatches.length;
         await expectOriginal400(await harness.request(), body);
-        expect(harness.dispatches).toEqual(["acct-pool-a"]);
-      } finally {
-        await stopPoolRetryHarness(harness);
+        expect(harness.dispatches.slice(priorDispatches)).toEqual(["acct-pool-a"]);
       }
+      expect(nextBody).toBe(bodies.length);
+    } finally {
+      await stopPoolRetryHarness(harness);
     }
   });