fix: extract pure timeout logic, lazy import undici in SDK

Address review feedback:
- Extract resolveTimeoutMs() as pure function so the mapping logic is
  testable under Bun without hitting the Node-only Agent guard
- Test now verifies all mapping branches via resolveTimeoutMs + Agent
  instance check (toBeInstanceOf) instead of poking private fields
- Restore browser-safety in SDK: lazy import('undici') inside guard
  instead of top-level static import that would pull node:* into bundles
  even when the runtime guard prevents execution

Author: Leonid Skorobogatyy

packages/core/src/util/undici-dispatcher.ts

@@ -4,21 +4,27 @@ import { Agent } from "undici"
 // This kills long-running provider requests — including SSE streams with gaps
 // >300s between chunks — at the ~303s mark regardless of provider.timeout config.
 //
-// createUndiciDispatcher maps the provider `timeout` option to undici's
-// headersTimeout AND bodyTimeout so that:
-//   timeout: false      → both set to 0 (disabled)
-//   timeout: <number>   → both set to that value (ms)
-//   timeout: undefined  → no dispatcher (undici defaults apply, i.e. 300s)
+// resolveTimeoutMs maps the provider `timeout` option to a millisecond value:
+//   false      → 0 (disabled)
+//   <number> >0 → that value (ms)
+//   anything else → undefined (no dispatcher, undici defaults apply)
 //
-// The dispatcher is only created under Node. Bun's fetch has no such defaults,
-// so the guard via process.versions.bun keeps Bun behavior unchanged.
+// createUndiciDispatcher wraps resolveTimeoutMs with a Node-only guard and
+// constructs the Agent. Bun's fetch has no such defaults, so the guard via
+// process.versions.bun keeps Bun behavior unchanged.
 //
 // This does not conflict with the existing chunkTimeout / headerTimeout abort
 // signals: those use AbortSignal/AbortController which are independent of the
 // undici dispatcher's socket-level timeouts.
+export function resolveTimeoutMs(timeout: unknown): number | undefined {
+  if (timeout === false) return 0
+  if (typeof timeout === "number" && timeout > 0) return timeout
+  return undefined
+}
+
 export function createUndiciDispatcher(timeout: unknown) {
   if (typeof process !== "object" || (process.versions as Record<string, string | undefined>).bun) return undefined
-  const ms = timeout === false ? 0 : typeof timeout === "number" && timeout > 0 ? timeout : undefined
+  const ms = resolveTimeoutMs(timeout)
   if (ms === undefined) return undefined
   return new Agent({ headersTimeout: ms, bodyTimeout: ms })
 }

packages/core/test/util/undici-dispatcher.test.ts

@@ -1,46 +1,72 @@
 import { describe, expect, test } from "bun:test"
-import { createUndiciDispatcher } from "@opencode-ai/core/util/undici-dispatcher"
+import { Agent } from "undici"
+import { resolveTimeoutMs, createUndiciDispatcher } from "@opencode-ai/core/util/undici-dispatcher"
 
-describe("createUndiciDispatcher", () => {
-  test("positive number returns an Agent with matching headersTimeout and bodyTimeout", () => {
-    const agent = createUndiciDispatcher(600_000) as InstanceType<typeof import("undici").Agent> | undefined
-    if (!agent) return // Bun — dispatcher is always undefined
-    expect((agent as unknown as { headersTimeout: number }).headersTimeout).toBe(600_000)
-    expect((agent as unknown as { bodyTimeout: number }).bodyTimeout).toBe(600_000)
+describe("resolveTimeoutMs", () => {
+  test("false → 0 (disabled)", () => {
+    expect(resolveTimeoutMs(false)).toBe(0)
   })
 
-  test("false returns an Agent with headersTimeout and bodyTimeout set to 0", () => {
-    const agent = createUndiciDispatcher(false) as InstanceType<typeof import("undici").Agent> | undefined
-    if (!agent) return // Bun
-    expect((agent as unknown as { headersTimeout: number }).headersTimeout).toBe(0)
-    expect((agent as unknown as { bodyTimeout: number }).bodyTimeout).toBe(0)
+  test("positive number → that value in ms", () => {
+    expect(resolveTimeoutMs(600_000)).toBe(600_000)
   })
 
-  test("undefined returns undefined (undici defaults apply)", () => {
-    expect(createUndiciDispatcher(undefined)).toBeUndefined()
+  test("undefined → undefined (undici defaults apply)", () => {
+    expect(resolveTimeoutMs(undefined)).toBeUndefined()
   })
 
-  test("0 returns undefined (not a positive number)", () => {
-    expect(createUndiciDispatcher(0)).toBeUndefined()
+  test("0 → undefined (not a positive number)", () => {
+    expect(resolveTimeoutMs(0)).toBeUndefined()
   })
 
-  test("negative number returns undefined", () => {
-    expect(createUndiciDispatcher(-1)).toBeUndefined()
+  test("negative number → undefined", () => {
+    expect(resolveTimeoutMs(-1)).toBeUndefined()
   })
 
-  test("null returns undefined", () => {
-    expect(createUndiciDispatcher(null)).toBeUndefined()
+  test("null → undefined", () => {
+    expect(resolveTimeoutMs(null)).toBeUndefined()
   })
 
-  test("string returns undefined", () => {
-    expect(createUndiciDispatcher("300000")).toBeUndefined()
+  test("string → undefined", () => {
+    expect(resolveTimeoutMs("300000")).toBeUndefined()
   })
+})
 
+describe("createUndiciDispatcher", () => {
   test("Bun guard: returns undefined when process.versions.bun is set", () => {
     const original = (process.versions as Record<string, string | undefined>).bun
     ;(process.versions as Record<string, string | undefined>).bun = "1.0.0"
     try {
       expect(createUndiciDispatcher(600_000)).toBeUndefined()
+      expect(createUndiciDispatcher(false)).toBeUndefined()
+    } finally {
+      if (original === undefined) delete (process.versions as Record<string, string | undefined>).bun
+      else (process.versions as Record<string, string | undefined>).bun = original
+    }
+  })
+
+  test("returns undefined when no bun version and timeout resolves to undefined", () => {
+    const original = (process.versions as Record<string, string | undefined>).bun
+    ;(process.versions as Record<string, string | undefined>).bun = undefined
+    try {
+      expect(createUndiciDispatcher(undefined)).toBeUndefined()
+      expect(createUndiciDispatcher(0)).toBeUndefined()
+    } finally {
+      if (original === undefined) delete (process.versions as Record<string, string | undefined>).bun
+      else (process.versions as Record<string, string | undefined>).bun = original
+    }
+  })
+
+  // Under Bun, createUndiciDispatcher always returns undefined because of the
+  // guard. The Agent construction path only runs under Node. We verify it
+  // returns an Agent instance (not private fields) when the bun guard is
+  // removed and a valid timeout is provided.
+  test("returns an Agent instance under Node (bun guard bypassed)", () => {
+    const original = (process.versions as Record<string, string | undefined>).bun
+    ;(process.versions as Record<string, string | undefined>).bun = undefined
+    try {
+      const agent = createUndiciDispatcher(600_000)
+      expect(agent).toBeInstanceOf(Agent)
     } finally {
       if (original === undefined) delete (process.versions as Record<string, string | undefined>).bun
       else (process.versions as Record<string, string | undefined>).bun = original

packages/sdk/js/src/node-fetch.ts

@@ -1,15 +1,28 @@
-import { Agent } from "undici"
-
 // undici's fetch defaults headersTimeout and bodyTimeout to 300s, which kills
 // long-running sessions at the ~303s mark. The SDK client→server connection
 // (blocking POST /session/:id/message) needs both disabled so the request can
 // survive past 5 minutes. Bun is unaffected — its fetch has no such defaults.
-const nodeDispatcher =
-  typeof process === "undefined" || !process.versions || process.versions.bun
-    ? undefined
-    : new Agent({ headersTimeout: 0, bodyTimeout: 0 })
+//
+// undici is a hard dependency, but we still import it lazily inside the guard
+// so browser bundlers don't pull undici (and its node:* deps) into the bundle
+// when the SDK is loaded in a non-Node environment.
+type UndiciAgent = InstanceType<typeof import("undici").Agent>
+
+let nodeDispatcher: UndiciAgent | undefined | null
+
+async function ensureDispatcher() {
+  if (nodeDispatcher !== undefined) return nodeDispatcher
+  if (typeof process === "undefined" || !process.versions || process.versions.bun) {
+    nodeDispatcher = null
+    return null
+  }
+  const { Agent } = await import("undici")
+  nodeDispatcher = new Agent({ headersTimeout: 0, bodyTimeout: 0 })
+  return nodeDispatcher
+}
 
 export async function nodeFetchWithDispatcher(req: Request) {
-  if (!nodeDispatcher) return fetch(req)
-  return fetch(req, { dispatcher: nodeDispatcher } as RequestInit)
+  const dispatcher = await ensureDispatcher()
+  if (!dispatcher) return fetch(req)
+  return fetch(req, { dispatcher } as RequestInit)
 }