fix: bound every await in the OpenAI/Codex reviewer path

.changeset/bounded-openai-timeouts.md

@@ -0,0 +1,11 @@
+---
+"@planningo/duul": patch
+---
+
+Fix silent multi-minute hangs in the OpenAI/Codex reviewer path
+
+- Configure the OpenAI client with `timeout: 120s, maxRetries: 0` so the SDK's defaults (600s timeout + 2 silent internal retries) can no longer stretch one hung request into 30 minutes of unlogged silence.
+- Classify abort/connection failures (no HTTP status) as retryable — the SDK's `APIUserAbortError`/`APIConnectionError` keep `name: 'Error'`, so the old `name === 'AbortError'` check never retried them.
+- Race the stateless (ChatGPT backend) stream against the 120s abort and abort the SDK stream controller in `finally`, as a backstop for mid-SSE stalls.
+- Bound the Codex OAuth token refresh fetch with a 30s timeout — it runs outside the review AbortController and could hang the whole review silently.
+- Log before each API call and tool execution, and log the previously-silent tool-loop continue paths (cache hit / repeat limit / budget block), so a stall now names its await.

src/__tests__/openai-stream-timeout.test.ts

@@ -0,0 +1,40 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { OpenAIProvider } from '../services/providers/openai.js';
+
+// Regression test: a stateless (ChatGPT backend) stream whose SSE connection
+// hangs must be cut off by the review timeout instead of awaiting forever.
+// The fake stream ignores the request signal — like a hung connection — so
+// only the explicit Promise.race in apiCallWithRetry can unblock the call.
+function hangingStream() {
+  let rejectHang: (err: Error) => void = () => {};
+  const hang = new Promise<never>((_, reject) => {
+    rejectHang = reject;
+  });
+  return {
+    aborted: false,
+    abort() {
+      this.aborted = true;
+      rejectHang(Object.assign(new Error('Request was aborted.'), { name: 'AbortError' }));
+    },
+    async *[Symbol.asyncIterator]() {
+      await hang;
+    },
+  };
+}
+
+test('stateless stream that never ends is aborted at the review deadline', async () => {
+  const provider = new OpenAIProvider({ chatgpt: { accessToken: 'tok', accountId: 'acct' } });
+  const stream = hangingStream();
+  (provider as unknown as { client: unknown }).client = { responses: { stream: () => stream } };
+
+  const start = Date.now();
+  await assert.rejects(
+    (provider as unknown as {
+      apiCallWithRetry(params: Record<string, unknown>, deadline: number): Promise<unknown>;
+    }).apiCallWithRetry({}, Date.now() + 300),
+    /aborted|deadline/i,
+  );
+  assert.ok(Date.now() - start < 5_000, 'call must fail near the deadline, not hang');
+  assert.equal(stream.aborted, true, 'underlying stream must be aborted');
+});

src/services/providers/codex-auth.ts

@@ -114,6 +114,9 @@ export async function refreshCodexToken(auth: CodexAuth): Promise<CodexAuth> {
 
   const res = await fetch(OAUTH_TOKEN_URL, {
     method: 'POST',
+    // This runs outside the review loop's AbortController; without its own
+    // timeout a stalled OAuth endpoint hangs the whole review silently.
+    signal: AbortSignal.timeout(30_000),
     headers: { 'Content-Type': 'application/json' },
     body: JSON.stringify({
       client_id: OAUTH_CLIENT_ID,

src/services/providers/openai.ts

@@ -257,6 +257,10 @@ export class OpenAIProvider implements ReviewerProvider {
   private buildClient(apiKey: string): OpenAI {
     return new OpenAI({
       apiKey,
+      // duul owns retries and timeouts. SDK defaults (600s timeout + 2 silent
+      // internal retries) can stretch one hung request into 30min of silence.
+      timeout: 120_000,
+      maxRetries: 0,
       ...(this.baseURL ? { baseURL: this.baseURL } : {}),
       ...(this.defaultHeaders ? { defaultHeaders: this.defaultHeaders } : {}),
     });
@@ -409,21 +413,25 @@ export class OpenAIProvider implements ReviewerProvider {
           callCounts.set(cacheKey, count);
 
           if (count > MAX_REPEAT_CALLS) {
+            console.error(`[duul]   ${call.name}(${argSummary}) -> repeat limit (${count} calls)`);
             toolResults.push({ type: 'function_call_output' as const, call_id: call.call_id, output: 'You have already read this content multiple times. Use the context you already have to complete your review.' });
             continue;
           }
 
           if (toolCache.has(cacheKey)) {
+            console.error(`[duul]   ${call.name}(${argSummary}) -> cache hit`);
             toolResults.push({ type: 'function_call_output' as const, call_id: call.call_id, output: toolCache.get(cacheKey)! });
             continue;
           }
 
           const currentLevel = getStrategyLevel();
           if (!isToolAllowed(call.name, currentLevel)) {
+            console.error(`[duul]   ${call.name}(${argSummary}) -> blocked (budget level ${currentLevel})`);
             toolResults.push({ type: 'function_call_output' as const, call_id: call.call_id, output: budgetMessage(call.name, currentLevel) });
             continue;
           }
 
+          console.error(`[duul]   → ${call.name}(${argSummary})`);
           const result = await executeFilesystemTool(effectiveRoot, call.name, args, workspaceScope, byteBudget);
           toolCache.set(cacheKey, result);
           allUsedTools.push(`${call.name}(${argSummary})`);
@@ -500,7 +508,9 @@ export class OpenAIProvider implements ReviewerProvider {
       const remaining = remainingReviewMs(deadline);
       if (remaining === 0) throw new ReviewTimeoutError();
       const controller = new AbortController();
-      const timeout = setTimeout(() => controller.abort(), Math.min(120_000, remaining));
+      const timeoutMs = Math.min(120_000, remaining);
+      const timeout = setTimeout(() => controller.abort(), timeoutMs);
+      console.error(`[duul] → openai ${this.stateless ? 'stream' : 'create'} (attempt ${attempt + 1}/${MAX_RETRIES}, timeout ${timeoutMs}ms)`);
       try {
         let response: OpenAI.Responses.Response;
         if (this.stateless) {
@@ -510,7 +520,22 @@ export class OpenAIProvider implements ReviewerProvider {
             params as Parameters<typeof this.client.responses.stream>[0],
             { signal: controller.signal },
           );
-          response = await this.aggregateStream(stream);
+          // The SDK does not reliably propagate the abort signal into a hung
+          // SSE iteration, so the 120s timeout must race the stream explicitly.
+          const aggregated = this.aggregateStream(stream);
+          aggregated.catch(() => {}); // late rejection after a lost race is expected
+          try {
+            response = await Promise.race([
+              aggregated,
+              new Promise<never>((_, reject) =>
+                controller.signal.addEventListener('abort', () =>
+                  reject(Object.assign(new Error(`OpenAI stream aborted after ${timeoutMs}ms`), { name: 'AbortError' })),
+                { once: true }),
+              ),
+            ]);
+          } finally {
+            if (!stream.aborted) stream.abort();
+          }
         } else {
           response = (await this.client.responses.create(
             { ...params, stream: false } as Parameters<typeof this.client.responses.create>[0],
@@ -538,7 +563,10 @@ export class OpenAIProvider implements ReviewerProvider {
           }
         }
 
-        const isRetryable = error instanceof Error && (status !== undefined ? (status === 429 || status >= 500) : error.name === 'AbortError');
+        // No HTTP status = abort/connection failure. The SDK's APIUserAbortError
+        // and APIConnectionError keep name 'Error', so match on the missing
+        // status rather than error.name.
+        const isRetryable = error instanceof Error && (status === undefined || status === 429 || status >= 500);
         if (isRetryable && attempt < MAX_RETRIES - 1) {
           const delay = 1000 * Math.pow(2, attempt);
           console.error(`[duul] Retry ${attempt + 1}/${MAX_RETRIES} after ${delay}ms`);