diff --git a/design/src/cli.ts b/design/src/cli.ts index 481eb29d46..f9b90a28a5 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -115,6 +115,13 @@ async function main(): Promise { process.exit(1); } + // Per-request timeout for OpenAI image-generation calls. Distinct from the + // existing `--timeout` flag, which controls the `compare --serve` / `serve` + // HTTP listener. See issue #1519. + const apiTimeoutMs = flags["api-timeout"] + ? parseInt(flags["api-timeout"] as string) + : undefined; + switch (command) { case "generate": await generate({ @@ -125,6 +132,7 @@ async function main(): Promise { retry: flags.retry ? parseInt(flags.retry as string) : 0, size: flags.size as string, quality: flags.quality as string, + apiTimeoutMs, }); break; @@ -175,6 +183,7 @@ async function main(): Promise { size: flags.size as string, quality: flags.quality as string, viewports: flags.viewports as string, + apiTimeoutMs, }); break; @@ -183,6 +192,7 @@ async function main(): Promise { session: flags.session as string, feedback: flags.feedback as string, output: (flags.output as string) || "/tmp/gstack-iterate.png", + apiTimeoutMs, }); break; @@ -235,6 +245,7 @@ async function main(): Promise { screenshot: flags.screenshot as string, brief: flags.brief as string, output: (flags.output as string) || "/tmp/gstack-evolved.png", + apiTimeoutMs, }); break; diff --git a/design/src/commands.ts b/design/src/commands.ts index c8331e970a..2db55a4646 100644 --- a/design/src/commands.ts +++ b/design/src/commands.ts @@ -17,17 +17,17 @@ export const COMMANDS = new Map`. + */ +export const DEFAULT_IMAGE_GEN_TIMEOUT_MS = 300_000; diff --git a/design/src/evolve.ts b/design/src/evolve.ts index c88ae6c660..c3e5d4e860 100644 --- a/design/src/evolve.ts +++ b/design/src/evolve.ts @@ -8,11 +8,13 @@ import fs from "fs"; import path from "path"; import { requireApiKey } from "./auth"; +import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants"; export interface EvolveOptions { screenshot: string; // Path to current site screenshot brief: string; // What to change ("make it calmer", "fix the hierarchy") output: string; // Output path for evolved mockup + apiTimeoutMs?: number; } /** @@ -52,7 +54,7 @@ export async function evolve(options: EvolveOptions): Promise { ].join("\n"); const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 120_000); + const timeout = setTimeout(() => controller.abort(), options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS); try { const response = await fetch("https://api.openai.com/v1/responses", { diff --git a/design/src/generate.ts b/design/src/generate.ts index 383c51aeeb..9ed73c5f50 100644 --- a/design/src/generate.ts +++ b/design/src/generate.ts @@ -8,6 +8,7 @@ import { requireApiKey } from "./auth"; import { parseBrief } from "./brief"; import { createSession, sessionPath } from "./session"; import { checkMockup } from "./check"; +import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants"; export interface GenerateOptions { brief?: string; @@ -17,6 +18,7 @@ export interface GenerateOptions { retry?: number; size?: string; quality?: string; + apiTimeoutMs?: number; } export interface GenerateResult { @@ -35,9 +37,10 @@ async function callImageGeneration( prompt: string, size: string, quality: string, + timeoutMs: number, ): Promise<{ responseId: string; imageData: string }> { const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 120_000); + const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch("https://api.openai.com/v1/responses", { @@ -105,6 +108,7 @@ export async function generate(options: GenerateOptions): Promise { console.error(` Feedback: "${options.feedback}"`); const startTime = Date.now(); + const apiTimeoutMs = options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS; + // Single deadline shared across threading + fallback so cumulative wait is + // bounded by apiTimeoutMs rather than 2×apiTimeoutMs. + const deadline = startTime + apiTimeoutMs; // Try multi-turn with previous_response_id first let success = false; let responseId = ""; try { - const result = await callWithThreading(apiKey, session.lastResponseId, options.feedback); + const result = await callWithThreading(apiKey, session.lastResponseId, options.feedback, apiTimeoutMs); responseId = result.responseId; fs.mkdirSync(path.dirname(options.output), { recursive: true }); @@ -45,13 +51,19 @@ export async function iterate(options: IterateOptions): Promise { console.error(` Threading failed: ${err.message}`); console.error(" Falling back to re-generation with accumulated feedback..."); + const remaining = deadline - Date.now(); + if (remaining <= 0) { + const secs = (apiTimeoutMs / 1000).toFixed(apiTimeoutMs % 1000 === 0 ? 0 : 1); + throw new Error(`Timeout (${secs}s)`); + } + // Fallback: re-generate with original brief + all feedback const accumulatedPrompt = buildAccumulatedPrompt( session.originalBrief, [...session.feedbackHistory, options.feedback] ); - const result = await callFresh(apiKey, accumulatedPrompt); + const result = await callFresh(apiKey, accumulatedPrompt, remaining); responseId = result.responseId; fs.mkdirSync(path.dirname(options.output), { recursive: true }); @@ -80,9 +92,10 @@ async function callWithThreading( apiKey: string, previousResponseId: string, feedback: string, + timeoutMs: number, ): Promise<{ responseId: string; imageData: string }> { const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 120_000); + const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch("https://api.openai.com/v1/responses", { @@ -128,9 +141,10 @@ async function callWithThreading( async function callFresh( apiKey: string, prompt: string, + timeoutMs: number, ): Promise<{ responseId: string; imageData: string }> { const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 120_000); + const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetch("https://api.openai.com/v1/responses", { diff --git a/design/src/variants.ts b/design/src/variants.ts index d52eb22829..1e24aa7d6a 100644 --- a/design/src/variants.ts +++ b/design/src/variants.ts @@ -8,6 +8,7 @@ import fs from "fs"; import path from "path"; import { requireApiKey } from "./auth"; import { parseBrief } from "./brief"; +import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants"; export interface VariantsOptions { brief?: string; @@ -17,6 +18,7 @@ export interface VariantsOptions { size?: string; quality?: string; viewports?: string; // "desktop,tablet,mobile" — generates at multiple sizes + apiTimeoutMs?: number; } const STYLE_VARIATIONS = [ @@ -42,6 +44,7 @@ export async function generateVariant( size: string, quality: string, fetchFn: typeof globalThis.fetch = globalThis.fetch, + timeoutMs: number = DEFAULT_IMAGE_GEN_TIMEOUT_MS, ): Promise<{ path: string; success: boolean; error?: string }> { const maxRetries = 3; const MAX_RETRY_AFTER_MS = 60_000; // cap honored Retry-After to bound stalls @@ -58,7 +61,7 @@ export async function generateVariant( skipLeadingDelay = false; const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 120_000); + const timeout = setTimeout(() => controller.abort(), timeoutMs); try { const response = await fetchFn("https://api.openai.com/v1/responses", { @@ -125,7 +128,7 @@ export async function generateVariant( } catch (err: any) { clearTimeout(timeout); if (err.name === "AbortError") { - return { path: outputPath, success: false, error: "Timeout (120s)" }; + return { path: outputPath, success: false, error: `Timeout (${timeoutMs}ms)` }; } lastError = err.message; } @@ -144,12 +147,13 @@ export async function variants(options: VariantsOptions): Promise { : parseBrief(options.brief!, false); const quality = options.quality || "high"; + const apiTimeoutMs = options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS; fs.mkdirSync(options.outputDir, { recursive: true }); // If viewports specified, generate responsive variants instead of style variants if (options.viewports) { - await generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality); + await generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality, apiTimeoutMs); return; } @@ -176,7 +180,7 @@ export async function variants(options: VariantsOptions): Promise { new Promise(resolve => setTimeout(resolve, delay)) .then(() => { console.error(` Starting variant ${String.fromCharCode(65 + i)}...`); - return generateVariant(apiKey, prompt, outputPath, size, quality); + return generateVariant(apiKey, prompt, outputPath, size, quality, undefined, apiTimeoutMs); }) ); } @@ -225,6 +229,7 @@ async function generateResponsiveVariants( outputDir: string, viewports: string, quality: string, + timeoutMs: number, ): Promise { const viewportList = viewports.split(",").map(v => v.trim().toLowerCase()); const configs = viewportList.map(v => VIEWPORT_CONFIGS[v]).filter(Boolean); @@ -250,7 +255,7 @@ async function generateResponsiveVariants( setTimeout(resolve, delay) ).then(() => { console.error(` Starting ${config.desc}...`); - return generateVariant(apiKey, prompt, outputPath, config.size, quality); + return generateVariant(apiKey, prompt, outputPath, config.size, quality, undefined, timeoutMs); }); }); diff --git a/design/test/api-timeout.test.ts b/design/test/api-timeout.test.ts new file mode 100644 index 0000000000..b77bf264ed --- /dev/null +++ b/design/test/api-timeout.test.ts @@ -0,0 +1,82 @@ +/** + * Regression coverage for issue #1519 — image-generation calls were timing + * out at a hardcoded 120s with no CLI override. The fix raised the default + * and exposed a per-invocation override via `--api-timeout`. + * + * These tests pin the contract that the constant exports at the new value + * and that the `timeoutMs` param is honored by the AbortController path. + */ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { generateVariant } from "../src/variants"; +import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "../src/constants"; + +describe("DEFAULT_IMAGE_GEN_TIMEOUT_MS", () => { + test("default is 300_000ms (5min) — see issue #1519", () => { + expect(DEFAULT_IMAGE_GEN_TIMEOUT_MS).toBe(300_000); + }); +}); + +describe("generateVariant timeoutMs override", () => { + let tmpDir: string; + let outputPath: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "api-timeout-")); + outputPath = path.join(tmpDir, "variant.png"); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("aborts after explicit timeoutMs when fetch never resolves", async () => { + // Stub fetch that waits for the signal to abort, then throws AbortError. + const fetchFn = (async (_input: any, init?: any) => { + const signal = init?.signal as AbortSignal | undefined; + await new Promise((_resolve, reject) => { + if (!signal) return; + signal.addEventListener("abort", () => { + const err: any = new Error("aborted"); + err.name = "AbortError"; + reject(err); + }); + }); + return new Response("never reached"); + }) as typeof globalThis.fetch; + + const t0 = Date.now(); + const result = await generateVariant( + "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn, 200, + ); + const elapsed = Date.now() - t0; + + expect(result.success).toBe(false); + expect(result.error).toBe("Timeout (200ms)"); + // Was aborted by the 200ms timer, not by exponential-backoff retry chain + expect(elapsed).toBeLessThan(2_000); + }); + + test("default timeoutMs is the shared constant when omitted", async () => { + // 1x1 transparent PNG, base64 + const TINY_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII="; + const fetchFn = (async () => + new Response( + JSON.stringify({ + output: [{ type: "image_generation_call", result: TINY_PNG_BASE64 }], + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + )) as typeof globalThis.fetch; + + // Should succeed using the default timeout — fetch resolves instantly here, + // so the timeoutMs value only matters for the AbortController setup not firing. + const result = await generateVariant( + "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn, + ); + + expect(result.success).toBe(true); + }); +});