diff --git a/design-shotgun/SKILL.md.tmpl b/design-shotgun/SKILL.md.tmpl index ac3d1ecdd..66bd1c83f 100644 --- a/design-shotgun/SKILL.md.tmpl +++ b/design-shotgun/SKILL.md.tmpl @@ -18,7 +18,6 @@ allowed-tools: - Read - Glob - Grep - - Agent - AskUserQuestion gbrain: schema: 1 @@ -227,8 +226,8 @@ them, not one team on different days. Use AskUserQuestion to confirm before spending API credits: -> "These are the {N} directions I'll generate. Each takes ~60s, but I'll run them all -> in parallel so total time is ~60 seconds regardless of count." +> "These are the {N} directions I'll generate. They run in parallel, so the whole set +> typically takes about 60 seconds." Options: - A) Generate all {N} — looks good @@ -258,57 +257,43 @@ console.log("GSTACK_STEP_OK"); Then `cp "/current.png" "$_DESIGN_DIR/current.png"` and Read it so the user sees what you're evolving from. -**Launch N Agent subagents in a single message** (parallel execution). Use the Agent -tool with `subagent_type: "general-purpose"` and `run_in_background: false` for each -variant (parallel foreground calls in one message still run concurrently, and the -comparison board needs every variant's result). Each agent is independent and handles -its own generation, quality check, verification, and retry. +**Generate every variant with one `$D variants --briefs-file` call.** Write one entry per +confirmed concept to `$_DESIGN_DIR/briefs.json`: a JSON array of `{"brief": ""}` objects, in concept order (A, B, C, ...), at most 7. When +evolving, add `"screenshot": "<_DESIGN_DIR>/current.png"` to every entry. Then run this +Bash call with `timeout: 600000` and wait for it to return: -**Important: $D path propagation.** The `$D` variable from DESIGN SETUP is a shell -variable that agents do NOT inherit. Substitute the resolved absolute path (from the -`DESIGN_READY: /path/to/design` output in Step 0) into each agent prompt. - -**Agent prompt template** (one per variant, substitute all `{...}` values): - -``` -Generate a design variant and save it. - -Design binary: {absolute path to $D binary} -Brief: {the full variant-specific brief for this direction} -Output: /tmp/variant-{letter}.png -Final location: {_DESIGN_DIR absolute path}/variant-{letter}.png - -Steps: -1. Run: {$D path} generate --brief "{brief}" --output /tmp/variant-{letter}.png -2. If the command fails with a rate limit error (429 or "rate limit"), wait 5 seconds - and retry. Up to 3 retries. -3. If the output file is missing or empty after the command succeeds, retry once. -4. Copy: cp /tmp/variant-{letter}.png {_DESIGN_DIR}/variant-{letter}.png -5. Quality check: {$D path} check --image {_DESIGN_DIR}/variant-{letter}.png --brief "{brief}" - If quality check fails, retry generation once. -6. Verify: ls -lh {_DESIGN_DIR}/variant-{letter}.png -7. Report exactly one of: - VARIANT_{letter}_DONE: {file size} - VARIANT_{letter}_FAILED: {error description} - VARIANT_{letter}_RATE_LIMITED: exhausted retries +```bash +_VARIANT_TMP=$(mktemp -d /tmp/gstack-variants-XXXXXX) +"$D" variants --briefs-file "$_DESIGN_DIR/briefs.json" --output-dir "$_VARIANT_TMP"; _VARIANTS_EXIT=$? +cp "$_VARIANT_TMP"/variant-*.png "$_DESIGN_DIR"/ 2>/dev/null +echo "VARIANTS_EXIT=$_VARIANTS_EXIT" ``` -For the evolve path, replace step 1 with: -``` -{$D path} evolve --screenshot {_DESIGN_DIR}/current.png --brief "{brief}" --output /tmp/variant-{letter}.png -``` +The command starts the variants 1.5s apart, retries rate limits with backoff, regenerates a +missing or empty image once, runs the vision check on each image and regenerates once +when it fails, and starts no new work after 9 minutes. It prints one +`VARIANT__DONE`, `_FAILED` or `_RATE_LIMITED` line per variant on stderr and a JSON +summary on stdout: per variant `path`, `operation`, `status`, `error`, `retryable` and +`check.status` (`pass`, `fail` or `skipped`). Exit 0 means every variant was generated, 3 +means some failed, 1 means none were generated or the briefs file was invalid (the error +names the entry and field; fix it and rerun). -**Generate to `/tmp/`, then `cp`:** in sandboxed sessions `$D generate --output` under -`~/.gstack/` can abort ("The operation was aborted"), while `/tmp/` works. +**Generate to `/tmp/`, then `cp`:** in sandboxed sessions `$D` output under `~/.gstack/` +can abort ("The operation was aborted"), while `/tmp/` works. ### Step 3d: Results -After all agents complete: +After the command returns: 1. Read each generated PNG inline (Read tool) so the user sees all variants at once. 2. Report status: "All {N} variants generated in ~{actual time}. {successes} succeeded, - {failures} failed." -3. For any failures: report explicitly with the error. Do NOT silently skip. + {failures} failed." A `skipped` check is missing automated coverage, not a pass: say so. +3. For any failures: report explicitly with the error. Do NOT silently skip. Rerun each + failed variant with `retryable: true` once, using its own operation and brief: + `"$D" generate --brief "" --output /tmp/variant-.png`, or for a screenshot + entry `"$D" evolve --screenshot "$_DESIGN_DIR/current.png" --brief "" --output /tmp/variant-.png`, + then `cp` it into `$_DESIGN_DIR`. 4. If zero variants succeeded: fall back to sequential generation (one at a time with `$D generate`, showing each as it lands). Tell the user: "Parallel generation failed (likely rate limiting). Falling back to sequential..." diff --git a/design/src/check.ts b/design/src/check.ts index f1880e148..fbc14c46e 100644 --- a/design/src/check.ts +++ b/design/src/check.ts @@ -8,20 +8,30 @@ import { requireApiKey } from "./auth"; import { receiptedFetch } from "./receipted-fetch"; import { modelRejectionHint, visionRequestBody } from "./models"; +/** `pass` stays true when the check could not run; `status: "skipped"` marks that + * no automated validation happened, so callers never count it as validated. */ export interface CheckResult { pass: boolean; + status: "pass" | "fail" | "skipped"; issues: string; } +export interface CheckOptions { + apiKey?: string; + fetchFn?: typeof globalThis.fetch; + signal?: AbortSignal; +} + /** * Check a generated mockup against the original brief. */ -export async function checkMockup(imagePath: string, brief: string): Promise { - const apiKey = requireApiKey(); +export async function checkMockup(imagePath: string, brief: string, opts: CheckOptions = {}): Promise { + const apiKey = opts.apiKey ?? requireApiKey(); const imageData = fs.readFileSync(imagePath).toString("base64"); const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 60_000); + const signal = opts.signal ? AbortSignal.any([controller.signal, opts.signal]) : controller.signal; try { const response = await receiptedFetch("check-screenshot-request", "https://api.openai.com/v1/chat/completions", { @@ -56,30 +66,30 @@ export async function checkMockup(imagePath: string, brief: string): Promise { break; case "variants": - await variants({ + process.exitCode = await variants({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, + briefsFile: flags["briefs-file"] as string, // #2032: pass the RAW flag through — variants() normalizes at its // consumption site (a pre-parseInt here would silently truncate "3.7"). count: flags.count, diff --git a/design/src/commands.ts b/design/src/commands.ts index 17d5811a7..e38bce9a1 100644 --- a/design/src/commands.ts +++ b/design/src/commands.ts @@ -20,9 +20,9 @@ export const COMMANDS = new Map { console.error(` Analyzed current design: ${analysis.slice(0, 100)}...`); // Step 2: Generate evolved version using analysis + brief - const evolvedPrompt = [ - "Generate a pixel-perfect UI mockup that is an improved version of an existing design.", - "", - "CURRENT DESIGN (what exists now):", - analysis, - "", - "REQUESTED CHANGES:", - options.brief, - "", - "Generate a new mockup that keeps the existing layout structure but applies the requested changes.", - "The result should look like a real production UI. All text must be readable.", - "1536x1024 pixels.", - ].join("\n"); + const evolvedPrompt = evolvePrompt(analysis, options.brief); const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 240_000); @@ -103,12 +91,35 @@ export async function evolve(options: EvolveOptions): Promise { } } +/** The image prompt for an evolved mockup: the analyzed current design plus the requested changes. */ +export function evolvePrompt(analysis: string, brief: string): string { + return [ + "Generate a pixel-perfect UI mockup that is an improved version of an existing design.", + "", + "CURRENT DESIGN (what exists now):", + analysis, + "", + "REQUESTED CHANGES:", + brief, + "", + "Generate a new mockup that keeps the existing layout structure but applies the requested changes.", + "The result should look like a real production UI. All text must be readable.", + "1536x1024 pixels.", + ].join("\n"); +} + /** * Analyze a screenshot to produce a detailed description for re-generation. */ -async function analyzeScreenshot(apiKey: string, imageBase64: string): Promise { +export async function analyzeScreenshot( + apiKey: string, + imageBase64: string, + fetchFn: typeof globalThis.fetch = globalThis.fetch, + batchSignal?: AbortSignal, +): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 30_000); + const signal = batchSignal ? AbortSignal.any([controller.signal, batchSignal]) : controller.signal; try { const response = await receiptedFetch("evolve-screenshot-analysis-request", "https://api.openai.com/v1/chat/completions", { @@ -130,8 +141,8 @@ async function analyzeScreenshot(apiKey: string, imageBase64: string): Promise { + return new Promise(resolve => { + if (signal?.aborted) return resolve(); + const done = () => { clearTimeout(timer); signal?.removeEventListener("abort", done); resolve(); }; + const timer = setTimeout(done, ms); + signal?.addEventListener("abort", done, { once: true }); + }); +} + /** * Generate a single variant with retry on 429. * * Exported for testability. Pass `fetchFn` to inject a stubbed fetch in tests; - * production code uses the global fetch by default. + * production code uses the global fetch by default. `signal` (the batch + * deadline) stops backoff waits, the request and its body read; once it has + * aborted no new attempt starts. */ export async function generateVariant( apiKey: string, @@ -50,7 +78,8 @@ export async function generateVariant( size: string, quality: string, fetchFn: typeof globalThis.fetch = globalThis.fetch, -): Promise<{ path: string; success: boolean; error?: string }> { + signal?: AbortSignal, +): Promise { const maxRetries = 3; const MAX_RETRY_AFTER_MS = 60_000; // cap honored Retry-After to bound stalls let lastError = ""; @@ -59,7 +88,7 @@ export async function generateVariant( try { body = imageRequestBody(prompt, { size, quality }); } catch (err: any) { - return { path: outputPath, success: false, error: err.message }; + return { path: outputPath, success: false, error: err.message, retryable: false }; } for (let attempt = 0; attempt <= maxRetries; attempt++) { @@ -67,9 +96,10 @@ export async function generateVariant( // Exponential backoff: 2s, 4s, 8s const delay = Math.pow(2, attempt) * 1000; console.error(` Rate limited, retrying in ${delay / 1000}s...`); - await new Promise(r => setTimeout(r, delay)); + await sleep(delay, signal); } skipLeadingDelay = false; + if (signal?.aborted) return { path: outputPath, success: false, error: DEADLINE_ERROR, retryable: true }; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 240_000); @@ -82,12 +112,11 @@ export async function generateVariant( "Content-Type": "application/json", }, body, - signal: controller.signal, + signal: signal ? AbortSignal.any([controller.signal, signal]) : controller.signal, }, fetchFn); - clearTimeout(timeout); - if (response.status === 429) { + clearTimeout(timeout); lastError = "Rate limited (429)"; const retryAfter = response.headers.get("retry-after"); if (retryAfter) { @@ -105,7 +134,7 @@ export async function generateVariant( } if (waitMs !== null) { if (waitMs > 0) { - await new Promise(resolve => setTimeout(resolve, waitMs)); + await sleep(waitMs, signal); } // Honored Retry-After (incl. 0 / past date "retry now") — skip the // next iteration's leading exponential sleep so we don't double-wait. @@ -117,17 +146,19 @@ export async function generateVariant( if (!response.ok) { const error = await response.text(); + clearTimeout(timeout); if (response.status === 403 && error.includes("organization must be verified")) { - return { path: outputPath, success: false, error: "OpenAI organization verification required. Go to https://platform.openai.com/settings/organization to verify." }; + return { path: outputPath, success: false, error: "OpenAI organization verification required. Go to https://platform.openai.com/settings/organization to verify.", retryable: false }; } - return { path: outputPath, success: false, error: `API error (${response.status}): ${error.slice(0, 200)}${modelRejectionHint(response.status, error, "image")}` }; + return { path: outputPath, success: false, error: `API error (${response.status}): ${error.slice(0, 200)}${modelRejectionHint(response.status, error, "image")}`, retryable: response.status >= 500 }; } const data = await response.json() as any; + clearTimeout(timeout); const imageItem = data.output?.find((item: any) => item.type === "image_generation_call"); if (!imageItem?.result) { - return { path: outputPath, success: false, error: "No image data in response" }; + return { path: outputPath, success: false, error: "No image data in response", retryable: true }; } fs.writeFileSync(outputPath, Buffer.from(imageItem.result, "base64")); @@ -135,19 +166,188 @@ export async function generateVariant( } catch (err: any) { clearTimeout(timeout); if (err.name === "AbortError") { - return { path: outputPath, success: false, error: "Timeout (240s)" }; + return { path: outputPath, success: false, error: signal?.aborted ? DEADLINE_ERROR : "Timeout (240s)", retryable: true }; } lastError = err.message; } } - return { path: outputPath, success: false, error: lastError }; + return { path: outputPath, success: false, error: lastError, rateLimited: lastError === "Rate limited (429)", retryable: true }; +} + +export interface BriefEntry { + brief: string; + screenshot?: string; +} + +/** One entry per variant; the ceiling matches the built-in style directions. */ +export const MAX_BRIEFS = STYLE_VARIATIONS.length; +/** Whole-batch deadline; the skill's Bash call allows 600s. */ +export const BATCH_DEADLINE_MS = 540_000; +const STAGGER_MS = 1500; + +/** + * Read and validate a --briefs-file before any billable call. Errors name the + * file, the entry index and the field. + */ +export function readBriefsFile(file: string): BriefEntry[] { + const fail = (problem: string): never => { throw new Error(`--briefs-file ${file}: ${problem}`); }; + let raw: unknown; + try { + raw = JSON.parse(fs.readFileSync(file, "utf-8")); + } catch (err: any) { + return fail(`cannot read a JSON array (${err.message})`); + } + if (!Array.isArray(raw) || raw.length < 1 || raw.length > MAX_BRIEFS) { + return fail(`expected a JSON array of 1 to ${MAX_BRIEFS} entries like [{"brief": "...", "screenshot": "optional.png"}]`); + } + return raw.map((entry: any, index) => { + if (!entry || typeof entry !== "object" || Array.isArray(entry)) fail(`entry ${index}: expected an object with a "brief" field`); + const unknown = Object.keys(entry).find(key => key !== "brief" && key !== "screenshot"); + if (unknown) fail(`entry ${index}: unknown field "${unknown}" (allowed: brief, screenshot)`); + if (typeof entry.brief !== "string" || !entry.brief.trim()) fail(`entry ${index}: field "brief" must be a non-empty string`); + if (entry.screenshot === undefined) return { brief: entry.brief }; + if (typeof entry.screenshot !== "string" || !entry.screenshot) fail(`entry ${index}: field "screenshot" must be a non-empty string`); + let readable = false; + try { + fs.accessSync(entry.screenshot, fs.constants.R_OK); + readable = fs.statSync(entry.screenshot).isFile(); + } catch {} + if (!readable) fail(`entry ${index}: field "screenshot" is not a readable file: ${entry.screenshot}`); + return { brief: entry.brief, screenshot: entry.screenshot }; + }); +} + +export interface BatchVariant { + variant: string; + path: string; + operation: "generate" | "evolve"; + status: "done" | "failed" | "rate_limited"; + error: string | null; + retryable: boolean; + check: { status: CheckResult["status"]; issues: string } | null; +} + +export interface BatchOptions { + apiKey: string; + outputDir: string; + size: string; + quality: string; + fetchFn?: typeof globalThis.fetch; + deadlineMs?: number; + staggerMs?: number; +} + +const hasImage = (file: string) => fs.existsSync(file) && fs.statSync(file).size > 0; + +/** + * Generate one variant per brief with staggered parallel launches under one + * batch deadline. Each variant repeats its own operation (generate, or evolve + * when it has a screenshot) for one missing/empty output and for one failed + * check, and always ends with a terminal status. + */ +export async function runBriefsBatch(entries: BriefEntry[], opts: BatchOptions): Promise { + const fetchFn = opts.fetchFn ?? globalThis.fetch; + const controller = new AbortController(); + const deadline = setTimeout(() => controller.abort(), opts.deadlineMs ?? BATCH_DEADLINE_MS); + const signal = controller.signal; + + const runOne = async (entry: BriefEntry, index: number): Promise => { + const variant = String.fromCharCode(65 + index); + const outputPath = path.join(opts.outputDir, `variant-${variant}.png`); + const operation = entry.screenshot ? "evolve" : "generate"; + const finish = (status: BatchVariant["status"], error: string | null, retryable: boolean, check: BatchVariant["check"]): BatchVariant => { + const detail = status === "done" + ? `${(fs.statSync(outputPath).size / 1024).toFixed(0)}KB, check ${check!.status}` + : error; + console.error(`VARIANT_${variant}_${status.toUpperCase()}: ${detail}`); + return { variant, path: outputPath, operation, status, error, retryable, check }; + }; + const attempt = async (): Promise => { + if (!entry.screenshot) return generateVariant(opts.apiKey, entry.brief, outputPath, opts.size, opts.quality, fetchFn, signal); + let analysis: string; + try { + analysis = await analyzeScreenshot(opts.apiKey, fs.readFileSync(entry.screenshot).toString("base64"), fetchFn, signal); + } catch (err: any) { + return { path: outputPath, success: false, error: signal.aborted ? DEADLINE_ERROR : `Screenshot analysis failed: ${err.message}`, retryable: true }; + } + return generateVariant(opts.apiKey, evolvePrompt(analysis, entry.brief), outputPath, opts.size, opts.quality, fetchFn, signal); + }; + const check = async (): Promise> => { + if (signal.aborted) return { status: "skipped", issues: `${DEADLINE_ERROR} before the check` }; + try { + const result = await checkMockup(outputPath, entry.brief, { apiKey: opts.apiKey, fetchFn, signal }); + return { status: result.status, issues: result.issues }; + } catch (err: any) { + return { status: "skipped", issues: signal.aborted ? `${DEADLINE_ERROR} during the check` : `Check failed to run: ${err.message}` }; + } + }; + + await sleep(index * (opts.staggerMs ?? STAGGER_MS), signal); + if (signal.aborted) return finish("failed", `${DEADLINE_ERROR} before start`, true, null); + console.error(` Starting variant ${variant} (${operation})...`); + + let result = await attempt(); + if (result.success && !hasImage(outputPath)) result = await attempt(); + if (result.success && !hasImage(outputPath)) result = { path: outputPath, success: false, error: "Output file missing or empty", retryable: true }; + if (!result.success) return finish(result.rateLimited ? "rate_limited" : "failed", result.error ?? "unknown error", result.retryable ?? true, null); + + let checked = await check(); + if (checked.status === "fail" && !signal.aborted) { + const regenerated = await attempt(); + if (regenerated.success && hasImage(outputPath)) checked = await check(); + } + return finish("done", null, false, checked); + }; + + try { + return await Promise.all(entries.map(runOne)); + } finally { + clearTimeout(deadline); + } +} + +/** Exit code for a finished batch: 0 all generated, 3 some failed, 1 none generated. */ +export function batchExitCode(results: BatchVariant[]): number { + const done = results.filter(r => r.status === "done").length; + if (done === results.length) return 0; + return done === 0 ? 1 : 3; +} + +async function variantsFromBriefsFile(options: VariantsOptions): Promise { + if (typeof options.briefsFile !== "string") throw new Error("--briefs-file needs the path of a JSON file"); + if (options.brief !== undefined || options.briefFile !== undefined || options.viewports !== undefined) { + throw new Error("--briefs-file cannot be combined with --brief, --brief-file or --viewports; put each variant's brief in the file."); + } + const entries = readBriefsFile(options.briefsFile); + if (options.count !== undefined) console.error(`warning: --count is ignored with --briefs-file; generating ${entries.length} variants, one per entry.`); + const apiKey = requireApiKey(); + fs.mkdirSync(options.outputDir, { recursive: true }); + + console.error(`Generating ${entries.length} variants from ${options.briefsFile}...`); + const startTime = Date.now(); + const results = await runBriefsBatch(entries, { + apiKey, outputDir: options.outputDir, size: options.size || "1536x1024", quality: options.quality || "high", + }); + const done = results.filter(r => r.status === "done"); + console.error(`\n${done.length}/${results.length} variants generated (${((Date.now() - startTime) / 1000).toFixed(1)}s)`); + console.log(JSON.stringify({ + outputDir: options.outputDir, + count: results.length, + succeeded: done.length, + failed: results.length - done.length, + validated: done.filter(r => r.check?.status === "pass").length, + variants: results, + }, null, 2)); + return batchExitCode(results); } /** - * Generate N variants with staggered parallel execution. + * Generate N variants with staggered parallel execution. Returns the process + * exit code (always 0 outside --briefs-file). */ -export async function variants(options: VariantsOptions): Promise { +export async function variants(options: VariantsOptions): Promise { + if (options.briefsFile !== undefined) return variantsFromBriefsFile(options); const apiKey = requireApiKey(); const baseBrief = options.briefFile ? parseBrief(options.briefFile, true) @@ -160,7 +360,7 @@ export async function variants(options: VariantsOptions): Promise { // If viewports specified, generate responsive variants instead of style variants if (options.viewports) { await generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality); - return; + return 0; } // #2032: normalize at the consumption site so every caller (CLI or @@ -229,6 +429,7 @@ export async function variants(options: VariantsOptions): Promise { paths: succeeded, errors: failed, }, null, 2)); + return 0; } const VIEWPORT_CONFIGS: Record = { diff --git a/design/test/fixtures/briefs-fetch-stub.ts b/design/test/fixtures/briefs-fetch-stub.ts new file mode 100644 index 000000000..6a6f31fe8 --- /dev/null +++ b/design/test/fixtures/briefs-fetch-stub.ts @@ -0,0 +1,22 @@ +/** + * Preloaded into the design CLI by variants-briefs.test.ts: replaces fetch so + * `variants --briefs-file` runs end to end without network or billing. + * BRIEFS_STUB_FAIL lists brief substrings whose image requests return 400. + * Every call is appended to BRIEFS_STUB_LOG as one JSON line. + */ +import fs from "fs"; + +const TINY_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII="; +const failing = (process.env.BRIEFS_STUB_FAIL ?? "").split(",").filter(Boolean); + +globalThis.fetch = (async (input: any, init?: any) => { + const url = String(input); + const body = String(init?.body ?? ""); + fs.appendFileSync(process.env.BRIEFS_STUB_LOG!, JSON.stringify({ url, body }) + "\n"); + if (url.endsWith("/v1/chat/completions")) { + return Response.json({ choices: [{ message: { content: "PASS" } }] }); + } + if (failing.some(token => body.includes(token))) return new Response("bad request", { status: 400 }); + return Response.json({ output: [{ type: "image_generation_call", result: TINY_PNG_BASE64 }] }); +}) as typeof globalThis.fetch; diff --git a/design/test/models.test.ts b/design/test/models.test.ts index a057e632f..5877a427d 100644 --- a/design/test/models.test.ts +++ b/design/test/models.test.ts @@ -200,7 +200,7 @@ describe("image-generation call sites (Responses API)", () => { describe("vision call sites (Chat Completions)", () => { test("check", async () => { stubFetch((u) => ok(u, "PASS")); - expect(await checkMockup(path.join(dir, "shot.png"), "brief")).toEqual({ pass: true, issues: "" }); + expect(await checkMockup(path.join(dir, "shot.png"), "brief")).toEqual({ pass: true, status: "pass", issues: "" }); expectVisionShape(calls[0]); stubFetch(() => rejected()); await checkMockup(path.join(dir, "shot.png"), "brief"); @@ -256,7 +256,7 @@ describe("vision budgets leave room for reasoning", () => { test("check, diff, memory, design-to-code and evolve analysis all answer", async () => { stubFetch(reasoningStub("PASS")); - expect(await checkMockup(path.join(dir, "shot.png"), "brief")).toEqual({ pass: true, issues: "" }); + expect(await checkMockup(path.join(dir, "shot.png"), "brief")).toEqual({ pass: true, status: "pass", issues: "" }); stubFetch(reasoningStub(JSON.stringify({ differences: [], summary: "same", matchScore: 100 }))); expect((await diffMockups(path.join(dir, "shot.png"), path.join(dir, "shot.png"))).matchScore).toBe(100); stubFetch(reasoningStub(JSON.stringify({ colors: [], typography: [], spacing: [], layout: [], mood: "calm" }))); diff --git a/design/test/variants-briefs.test.ts b/design/test/variants-briefs.test.ts new file mode 100644 index 000000000..de935d0fb --- /dev/null +++ b/design/test/variants-briefs.test.ts @@ -0,0 +1,236 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { batchExitCode, readBriefsFile, runBriefsBatch, type BriefEntry } from "../src/variants"; + +const TINY_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII="; +const CLI = path.resolve(import.meta.dir, "../src/cli.ts"); +const STUB = path.resolve(import.meta.dir, "fixtures/briefs-fetch-stub.ts"); + +type Call = { url: string; text: string }; +type Reply = (call: Call, nth: number) => Response; + +const image = (result = TINY_PNG_BASE64) => + Response.json({ output: [{ type: "image_generation_call", result }] }); +const vision = (content: string) => Response.json({ choices: [{ message: { content } }] }); + +/** Routes image, check and screenshot-analysis requests to separate scripted replies. */ +function stubFetch(routes: { image?: Reply; check?: Reply; analysis?: Reply }) { + const calls: Array = []; + const counts: Record = { image: 0, check: 0, analysis: 0 }; + const fetchFn = (async (input: any, init?: any) => { + const url = String(input); + const text = String(init?.body ?? ""); + const kind = url.endsWith("/v1/responses") ? "image" + : text.includes("UI quality checker") ? "check" : "analysis"; + const call = { url, text, kind }; + calls.push(call); + const reply = routes[kind as keyof typeof routes]; + const nth = counts[kind]++; + if (reply) return reply(call, nth); + return kind === "image" ? image() : kind === "check" ? vision("PASS") : vision("A dashboard with a header."); + }) as typeof globalThis.fetch; + return { fetchFn, calls }; +} + +let dir: string; +let savedHome: string | undefined; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "variants-briefs-")); + savedHome = process.env.GSTACK_HOME; + process.env.GSTACK_HOME = dir; +}); + +afterEach(() => { + if (savedHome === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = savedHome; + fs.rmSync(dir, { recursive: true, force: true }); +}); + +const batch = (entries: BriefEntry[], fetchFn: typeof globalThis.fetch, extra: { deadlineMs?: number; staggerMs?: number } = {}) => + runBriefsBatch(entries, { apiKey: "sk-test", outputDir: dir, size: "1024x1024", quality: "low", fetchFn, staggerMs: 0, ...extra }); + +function writeBriefs(value: unknown): string { + const file = path.join(dir, "briefs.json"); + fs.writeFileSync(file, JSON.stringify(value)); + return file; +} + +describe("readBriefsFile", () => { + test("accepts 1 to 7 entries with optional screenshots", () => { + const shot = path.join(dir, "current.png"); + fs.writeFileSync(shot, Buffer.from(TINY_PNG_BASE64, "base64")); + expect(readBriefsFile(writeBriefs([{ brief: "a" }, { brief: "b", screenshot: shot }]))) + .toEqual([{ brief: "a" }, { brief: "b", screenshot: shot }]); + }); + + test.each([ + ["not an array", { brief: "a" }, "JSON array of 1 to 7"], + ["empty", [], "JSON array of 1 to 7"], + ["too many", Array.from({ length: 8 }, () => ({ brief: "x" })), "JSON array of 1 to 7"], + ["missing brief", [{ brief: "ok" }, { screenshot: "x.png" }], 'entry 1: field "brief"'], + ["blank brief", [{ brief: " " }], 'entry 0: field "brief"'], + ["unknown field", [{ brief: "a", count: 3 }], 'entry 0: unknown field "count"'], + ["unreadable screenshot", [{ brief: "a", screenshot: "/nonexistent/shot.png" }], 'entry 0: field "screenshot" is not a readable file'], + ])("%s names the file, entry and field", (_label, value, message) => { + const file = writeBriefs(value); + expect(() => readBriefsFile(file)).toThrow(message); + expect(() => readBriefsFile(file)).toThrow(file); + }); + + test("malformed JSON names the file", () => { + const file = path.join(dir, "bad.json"); + fs.writeFileSync(file, "[{"); + expect(() => readBriefsFile(file)).toThrow(`--briefs-file ${file}`); + }); +}); + +describe("runBriefsBatch", () => { + test("each variant sends its own brief and ends DONE with a passing check", async () => { + const { fetchFn, calls } = stubFetch({}); + const results = await batch([{ brief: "CALM-DASHBOARD" }, { brief: "BOLD-DASHBOARD" }], fetchFn); + expect(results.map(r => [r.variant, r.status, r.check?.status, r.operation])).toEqual([ + ["A", "done", "pass", "generate"], ["B", "done", "pass", "generate"], + ]); + const images = calls.filter(c => c.kind === "image"); + expect(images).toHaveLength(2); + expect(images.filter(c => c.text.includes("CALM-DASHBOARD"))).toHaveLength(1); + expect(images.filter(c => c.text.includes("BOLD-DASHBOARD"))).toHaveLength(1); + expect(fs.statSync(path.join(dir, "variant-B.png")).size).toBeGreaterThan(0); + expect(batchExitCode(results)).toBe(0); + }); + + test("429 backs off and retries the same request", async () => { + const { fetchFn, calls } = stubFetch({ + image: (_call, nth) => nth === 0 ? new Response("slow down", { status: 429, headers: { "Retry-After": "0" } }) : image(), + }); + const [result] = await batch([{ brief: "one" }], fetchFn); + expect(result!.status).toBe("done"); + expect(calls.filter(c => c.kind === "image")).toHaveLength(2); + }); + + test("exhausted 429 retries end RATE_LIMITED and retryable", async () => { + const { fetchFn } = stubFetch({ image: () => new Response("slow down", { status: 429, headers: { "Retry-After": "0" } }) }); + const [result] = await batch([{ brief: "one" }], fetchFn); + expect(result).toMatchObject({ status: "rate_limited", retryable: true, check: null }); + expect(batchExitCode([result!])).toBe(1); + }); + + test("an empty output file is regenerated once", async () => { + const { fetchFn, calls } = stubFetch({ image: (_call, nth) => nth === 0 ? image("=") : image() }); + const [result] = await batch([{ brief: "one" }], fetchFn); + expect(result!.status).toBe("done"); + expect(calls.filter(c => c.kind === "image")).toHaveLength(2); + }); + + test("a failed check regenerates once with the same brief, then rechecks", async () => { + const { fetchFn, calls } = stubFetch({ check: (_call, nth) => vision(nth === 0 ? "FAIL: header text is garbled" : "PASS") }); + const [result] = await batch([{ brief: "CHECK-ME" }], fetchFn); + expect(result).toMatchObject({ status: "done", check: { status: "pass", issues: "" } }); + const images = calls.filter(c => c.kind === "image"); + expect(images).toHaveLength(2); + expect(images.every(c => c.text.includes("CHECK-ME"))).toBe(true); + expect(calls.filter(c => c.kind === "check")).toHaveLength(2); + }); + + test("an unavailable check is reported as skipped, never as validated", async () => { + const { fetchFn } = stubFetch({ check: () => new Response("down", { status: 503 }) }); + const [result] = await batch([{ brief: "one" }], fetchFn); + expect(result).toMatchObject({ status: "done", check: { status: "skipped" } }); + }); + + test("screenshot entries evolve, and a failed check repeats the evolve operation", async () => { + const shot = path.join(dir, "current.png"); + fs.writeFileSync(shot, Buffer.from(TINY_PNG_BASE64, "base64")); + const { fetchFn, calls } = stubFetch({ check: (_call, nth) => vision(nth === 0 ? "FAIL: cramped" : "PASS") }); + const [result] = await batch([{ brief: "EVOLVE-ME", screenshot: shot }], fetchFn); + expect(result).toMatchObject({ operation: "evolve", status: "done", check: { status: "pass" } }); + expect(calls.map(c => c.kind)).toEqual(["analysis", "image", "check", "analysis", "image", "check"]); + expect(calls.filter(c => c.kind === "image").every(c => c.text.includes("REQUESTED CHANGES") && c.text.includes("EVOLVE-ME"))).toBe(true); + }); + + test("a non-retryable API error fails only its own variant", async () => { + const { fetchFn } = stubFetch({ image: call => call.text.includes("BROKEN") ? new Response("bad", { status: 400 }) : image() }); + const results = await batch([{ brief: "fine" }, { brief: "BROKEN" }], fetchFn); + expect(results.map(r => [r.status, r.retryable])).toEqual([["done", false], ["failed", false]]); + expect(batchExitCode(results)).toBe(3); + }); + + test("the batch deadline stops new launches and in-flight requests; every variant is terminal", async () => { + const { fetchFn, calls } = stubFetch({}); + const slow = (async (input: any, init?: any) => { + if (String(input).endsWith("/v1/responses") && String(init?.body).includes("SLOW")) { + return new Promise((_resolve, reject) => { + init.signal.addEventListener("abort", () => reject(Object.assign(new Error("aborted"), { name: "AbortError" }))); + }); + } + return fetchFn(input, init); + }) as typeof globalThis.fetch; + const started = Date.now(); + const results = await batch([{ brief: "SLOW" }, { brief: "late" }], slow, { deadlineMs: 300, staggerMs: 5_000 }); + expect(Date.now() - started).toBeLessThan(3_000); + expect(results.map(r => [r.status, r.retryable, r.error])).toEqual([ + ["failed", true, "Batch deadline reached"], + ["failed", true, "Batch deadline reached before start"], + ]); + expect(calls).toHaveLength(0); + }); +}); + +describe("$D variants --briefs-file (CLI end to end with a stubbed fetch)", () => { + const run = (args: string[], fail = "") => { + const log = path.join(dir, "fetch.log"); + fs.writeFileSync(log, ""); + const result = spawnSync(process.execPath, ["--preload", STUB, CLI, "variants", ...args], { + cwd: dir, + env: { PATH: process.env.PATH!, HOME: dir, GSTACK_HOME: dir, OPENAI_API_KEY: "sk-test-stub", BRIEFS_STUB_LOG: log, BRIEFS_STUB_FAIL: fail }, + encoding: "utf-8", + timeout: 60_000, + }); + const calls = fs.readFileSync(log, "utf-8").split("\n").filter(Boolean); + return { ...result, calls }; + }; + const out = () => path.join(dir, "out"); + + test("all variants succeed: exit 0, JSON on stdout, status lines on stderr", () => { + const file = writeBriefs([{ brief: "FIRST" }, { brief: "SECOND" }]); + const r = run(["--briefs-file", file, "--output-dir", out(), "--count", "5"]); + expect(r.status, r.stderr).toBe(0); + const json = JSON.parse(r.stdout); + expect(json).toMatchObject({ count: 2, succeeded: 2, failed: 0, validated: 2 }); + expect(json.variants.map((v: any) => [v.variant, v.status, v.error, v.retryable])).toEqual([["A", "done", null, false], ["B", "done", null, false]]); + expect(r.stderr).toContain("--count is ignored"); + expect(r.stderr).toMatch(/VARIANT_A_DONE: /); + expect(r.stderr).toMatch(/VARIANT_B_DONE: /); + expect(fs.existsSync(path.join(out(), "variant-B.png"))).toBe(true); + }); + + test("some variants fail: exit 3", () => { + const file = writeBriefs([{ brief: "FIRST" }, { brief: "BROKEN" }]); + const r = run(["--briefs-file", file, "--output-dir", out()], "BROKEN"); + expect(r.status, r.stderr).toBe(3); + expect(JSON.parse(r.stdout).variants[1]).toMatchObject({ status: "failed", retryable: false }); + expect(r.stderr).toMatch(/VARIANT_B_FAILED: API error \(400\)/); + }); + + test("all variants fail: exit 1", () => { + const file = writeBriefs([{ brief: "BROKEN" }]); + expect(run(["--briefs-file", file, "--output-dir", out()], "BROKEN").status).toBe(1); + }); + + test("invalid input exits 1 before any billable call", () => { + const file = writeBriefs([{ brief: "fine" }, { brief: "" }]); + const invalid = run(["--briefs-file", file, "--output-dir", out()]); + expect(invalid.status).toBe(1); + expect(invalid.stderr).toContain('entry 1: field "brief"'); + expect(invalid.calls).toHaveLength(0); + const conflict = run(["--briefs-file", writeBriefs([{ brief: "a" }]), "--brief", "also", "--output-dir", out()]); + expect(conflict.status).toBe(1); + expect(conflict.stderr).toContain("cannot be combined with --brief"); + expect(conflict.calls).toHaveLength(0); + }); +}); diff --git a/test/design-consultation-command-contract.test.ts b/test/design-consultation-command-contract.test.ts index 0fb50e558..1bae2ceb7 100644 --- a/test/design-consultation-command-contract.test.ts +++ b/test/design-consultation-command-contract.test.ts @@ -80,10 +80,10 @@ test('actual CLI quality check distinguishes failure from skipped coverage despi try { const failed = f.run(['check', '--image', f.image, '--brief', 'Readable title'], { MODE: 'check' }); expect(failed.status, failed.stderr).toBe(0); - expect(JSON.parse(failed.stdout)).toEqual({ pass: false, issues: 'illegible title' }); + expect(JSON.parse(failed.stdout)).toEqual({ pass: false, status: 'fail', issues: 'illegible title' }); const unavailable = f.run(['check', '--image', f.image, '--brief', 'Readable title'], { MODE: 'unavailable' }); expect(unavailable.status, unavailable.stderr).toBe(0); - expect(JSON.parse(unavailable.stdout)).toEqual({ pass: true, issues: 'Vision check unavailable — skipped' }); + expect(JSON.parse(unavailable.stdout)).toEqual({ pass: true, status: 'skipped', issues: 'Vision check unavailable — skipped' }); } finally { f.cleanup(); } }); diff --git a/test/run-in-background-guidance.test.ts b/test/run-in-background-guidance.test.ts index e6868e06b..a9dfcad59 100644 --- a/test/run-in-background-guidance.test.ts +++ b/test/run-in-background-guidance.test.ts @@ -310,7 +310,6 @@ const GENERATED_WITH_GUIDANCE = [ // guidance and its bounded worker policy is specified in its own skeleton. 'design-consultation/sections/proposal-and-preview.md', 'design-review/SKILL.md', - 'design-shotgun/SKILL.md', 'document-release/sections/release-body.md', 'office-hours/SKILL.md', 'office-hours/sections/design-and-handoff.md',