fix: exec runs unbounded by default and is never retried

src/_baseClient.ts

@@ -1,4 +1,4 @@
-import { errorForStatus, SandboxError } from './_errors.js';
+import { errorForStatus, SandboxError, SandboxTimeoutError } from './_errors.js';
 import { resolveConfig, type Config, type ConfigInput } from './_config.js';
 import { DEFAULT_MAX_RETRIES, shouldRetry, sleepForAttempt } from './_retries.js';
 
@@ -13,6 +13,13 @@ export interface RequestOptions {
   path: string;
   query?: Record<string, unknown> | undefined;
   body?: unknown;
+  // Per-request timeout. `null` disables the timeout entirely, used for
+  // long-running calls like exec, where the API works for the full duration
+  // of the request. Omitted means the client-wide default.
+  timeoutMs?: number | null | undefined;
+  // Set false for calls that must not be re-sent (exec): a failed attempt may
+  // have executed server-side, so retrying could run the command again.
+  retry?: boolean | undefined;
 }
 
 // HTTP transport shared by the generated resource classes. Owns auth/header
@@ -36,16 +43,21 @@ export class BaseClient {
   async request<T>(options: RequestOptions): Promise<T> {
     const url = buildUrl(this.config.baseUrl, options.path, options.query);
     const init = buildInit(this.config, options);
+    const timeoutMs = options.timeoutMs === undefined ? this.config.timeoutMs : options.timeoutMs;
+    const maxRetries = options.retry === false ? 0 : this.maxRetries;
 
     let lastError: unknown = null;
 
-    for (let attempt = 0; attempt <= this.maxRetries; attempt++) {
+    for (let attempt = 0; attempt <= maxRetries; attempt++) {
       let response: Response;
       try {
-        response = await withTimeout(fetch(url, init), this.config.timeoutMs);
+        response = await withTimeout(fetch(url, init), timeoutMs);
       } catch (err) {
+        // A timed-out request may have executed server-side, so retrying
+        // could run it again. Surface the timeout instead.
+        if (err instanceof SandboxTimeoutError) throw err;
         lastError = err;
-        if (attempt < this.maxRetries) {
+        if (attempt < maxRetries) {
           await sleepForAttempt(attempt);
           continue;
         }
@@ -56,7 +68,7 @@ export class BaseClient {
         return (await decodeBody(response)) as T;
       }
 
-      if (shouldRetry(response.status) && attempt < this.maxRetries) {
+      if (shouldRetry(response.status) && attempt < maxRetries) {
         await sleepForAttempt(attempt);
         continue;
       }
@@ -100,13 +112,15 @@ const buildInit = (config: Config, options: RequestOptions): RequestInit => {
   return { method: options.method, headers, body };
 };
 
-const withTimeout = <T>(promise: Promise<T>, timeoutMs: number): Promise<T> => {
+const withTimeout = <T>(promise: Promise<T>, timeoutMs: number | null): Promise<T> => {
+  // `null` disables the timeout: the caller waits as long as the request runs.
+  if (timeoutMs === null) return promise;
   // AbortController would be cleaner, but fetch in older Node 18 had spotty
   // signal support. setTimeout+race is universally supported and keeps the
   // pending fetch from leaking once we've already given up.
   let timer: NodeJS.Timeout | undefined;
   const timeout = new Promise<never>((_, reject) => {
-    timer = setTimeout(() => reject(new Error(`request timed out after ${timeoutMs}ms`)), timeoutMs);
+    timer = setTimeout(() => reject(new SandboxTimeoutError(`request timed out after ${timeoutMs}ms`)), timeoutMs);
   });
   return Promise.race([promise, timeout]).finally(() => {
     if (timer) clearTimeout(timer);

src/_errors.ts

@@ -46,6 +46,16 @@ export class ServerError extends SandboxError {
   }
 }
 
+// Raised when a request exceeds its timeout. Never retried by the client: the
+// server may still be executing the request (e.g. a long-running exec), so a
+// retry could run it again.
+export class SandboxTimeoutError extends SandboxError {
+  constructor(message: string) {
+    super(message);
+    this.name = 'SandboxTimeoutError';
+  }
+}
+
 // Map an HTTP status code to the appropriate SandboxError subclass.
 export const errorForStatus = (
   statusCode: number,

src/resources/sandboxes.ts

@@ -106,11 +106,13 @@ export class Sandboxes {
    *
    * Run a command in a running sandbox and return its output and exit code
    */
-  async exec(id: string, body: ExecRequest): Promise<ExecResponse> {
+  async exec(id: string, body: ExecRequest, requestOptions?: { timeoutMs?: number }): Promise<ExecResponse> {
     return this.client.request<ExecResponse>({
       method: 'POST',
       path: `/v1/sandbox/${id}/exec`,
       body,
+      timeoutMs: requestOptions?.timeoutMs ?? null,
+      retry: false,
     });
   }
 }

tests/timeout.test.ts

@@ -0,0 +1,63 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { BaseClient } from '../src/_baseClient.js';
+import { SandboxTimeoutError } from '../src/_errors.js';
+
+const jsonResponse = (): Response =>
+  new Response('{}', { status: 200, headers: { 'content-type': 'application/json' } });
+
+const slowFetch = (delayMs: number): typeof fetch =>
+  vi.fn(
+    () =>
+      new Promise<Response>((resolve) => {
+        setTimeout(() => resolve(jsonResponse()), delayMs);
+      }),
+  );
+
+describe('request timeouts', () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  it('timeoutMs: null disables the client-wide timeout', async () => {
+    vi.stubGlobal('fetch', slowFetch(50));
+    const client = new BaseClient({ baseUrl: 'http://sandbox.test', timeoutMs: 10 });
+
+    await expect(
+      client.request({ method: 'POST', path: '/v1/sandbox/abc/exec', timeoutMs: null }),
+    ).resolves.toEqual({});
+  });
+
+  it('a per-request timeoutMs bounds the call and throws without retrying', async () => {
+    const fetchMock = slowFetch(50);
+    vi.stubGlobal('fetch', fetchMock);
+    const client = new BaseClient({ baseUrl: 'http://sandbox.test' });
+
+    await expect(
+      client.request({ method: 'POST', path: '/v1/sandbox/abc/exec', timeoutMs: 10 }),
+    ).rejects.toBeInstanceOf(SandboxTimeoutError);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('retry: false stops network errors from being re-sent', async () => {
+    const fetchMock = vi.fn().mockRejectedValue(new TypeError('fetch failed'));
+    vi.stubGlobal('fetch', fetchMock);
+    const client = new BaseClient({ baseUrl: 'http://sandbox.test' });
+
+    await expect(
+      client.request({ method: 'POST', path: '/v1/sandbox/abc/exec', retry: false }),
+    ).rejects.toThrow('Network error');
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('other requests still retry network errors', async () => {
+    const fetchMock = vi
+      .fn()
+      .mockRejectedValueOnce(new TypeError('fetch failed'))
+      .mockResolvedValueOnce(jsonResponse());
+    vi.stubGlobal('fetch', fetchMock);
+    const client = new BaseClient({ baseUrl: 'http://sandbox.test' });
+
+    await expect(client.request({ method: 'GET', path: '/v1/sandbox' })).resolves.toEqual({});
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+  });
+});