fix(design): loud integer-flag contract for --count/--retry/--timeout (#2032)

design variants --count abc silently generated ZERO variants and exited 0:
parseInt(NaN) flowed through Math.min into the generation loop bound. The
same NaN class was live on the two sibling flags in the same file:
--retry abc made generate() a silent no-op (attempt <= NaN never true, null
output, exit 0) and --timeout abc killed the serve board ~immediately
(setTimeout(NaN)).

New design/src/flag-utils.ts: parseIntFlag (pure, unit-testable) +
normalizeIntFlag (CLI wrapper). Contract matches the --viewports precedent
(error loudly on nonsense — these commands spend real image-API money, a
silent fixup hides typos from calling agents): undefined -> default; bare
flag/empty/non-integer ("3.7" rejected, not truncated)/below-min -> exit 1
with usage hint; above-max -> clamp with stderr warning. --count normalizes
at the variants() consumption site so programmatic callers are covered, with
the ceiling derived from STYLE_VARIATIONS.length instead of a magic 7; the
CLI passes the raw flag through (a pre-parseInt would truncate "3.7").

Tripwires live in test/design-flag-utils.test.ts — deliberately under test/,
not design/test/, which is invisible to the bun test glob, TEST_ROOTS, and
every workflow (wiring design/test/ into CI is a captured TODO).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

design/src/cli.ts

@@ -25,6 +25,7 @@ import { evolve } from "./evolve";
 import { generateDesignToCodePrompt } from "./design-to-code";
 import { serve } from "./serve";
 import { gallery } from "./gallery";
+import { normalizeIntFlag } from "./flag-utils";
 import {
   daemonStatus as daemonStatusClient,
   ensureDaemon,
@@ -137,7 +138,7 @@ async function main(): Promise<void> {
         briefFile: flags["brief-file"] as string,
         output: (flags.output as string) || "/tmp/gstack-mockup.png",
         check: !!flags.check,
-        retry: flags.retry ? parseInt(flags.retry as string) : 0,
+        retry: normalizeIntFlag(flags.retry, { name: "retry", def: 0, min: 0 }),
         size: flags.size as string,
         quality: flags.quality as string,
       });
@@ -163,7 +164,7 @@ async function main(): Promise<void> {
         if (flags["no-daemon"]) {
           await serve({
             html: outputPath,
-            timeout: flags.timeout ? parseInt(flags.timeout as string) : 600,
+            timeout: normalizeIntFlag(flags.timeout, { name: "timeout", def: 600, min: 1 }),
           });
         } else {
           await publishToDaemon({
@@ -197,7 +198,9 @@ async function main(): Promise<void> {
       await variants({
         brief: flags.brief as string,
         briefFile: flags["brief-file"] as string,
-        count: flags.count ? parseInt(flags.count as string) : 3,
+        // #2032: pass the RAW flag through — variants() normalizes at its
+        // consumption site (a pre-parseInt here would silently truncate "3.7").
+        count: flags.count,
         outputDir: (flags["output-dir"] as string) || "/tmp/gstack-variants/",
         size: flags.size as string,
         quality: flags.quality as string,

design/src/flag-utils.ts

@@ -0,0 +1,74 @@
+/**
+ * Integer flag normalization for the design CLI (#2032).
+ *
+ * The CLI's flag parser yields a string ("3"), boolean true (bare flag with
+ * no value), or undefined (flag absent). parseInt on those produced NaN that
+ * flowed silently into loop bounds and setTimeout:
+ *   --count abc   → for (i < NaN) never runs → ZERO variants, exit 0
+ *   --retry abc   → attempt <= NaN is false  → generate() silent no-op
+ *   --timeout abc → setTimeout(NaN) fires ~immediately → serve dies at boot
+ *
+ * Contract (matches the --viewports precedent, variants.ts: error LOUDLY on
+ * nonsense; these commands spend real image-API money, so a silent fixup
+ * hides typos from calling agents):
+ *   - undefined                  → default (flag absent)
+ *   - true / "" (bare flag)      → error: requires a value
+ *   - non-numeric / non-integer  → error ("3.7" is rejected, not truncated)
+ *   - below min                  → error
+ *   - above max (when given)     → clamp to max, stderr warning
+ *   - repeated flag              → parser is last-wins before we ever see it
+ */
+
+export interface IntFlagSpec {
+  name: string;
+  def: number;
+  min: number;
+  max?: number;
+}
+
+export type IntFlagResult =
+  | { ok: true; value: number; warning?: string }
+  | { ok: false; error: string };
+
+/** Pure decision function — unit-testable without process.exit. */
+export function parseIntFlag(raw: unknown, spec: IntFlagSpec): IntFlagResult {
+  const { name, def, min, max } = spec;
+  const bounds = `an integer >= ${min}${max !== undefined ? ` (max ${max})` : ""}`;
+
+  if (raw === undefined || raw === false) return { ok: true, value: def };
+  if (raw === true) {
+    return { ok: false, error: `--${name} requires a value. Expected ${bounds}.` };
+  }
+
+  const s = String(raw).trim();
+  if (s === "") {
+    return { ok: false, error: `--${name} requires a value. Expected ${bounds}.` };
+  }
+  if (!/^-?\d+$/.test(s)) {
+    return { ok: false, error: `Invalid --${name}: "${s}" is not an integer. Expected ${bounds}.` };
+  }
+
+  const n = parseInt(s, 10);
+  if (n < min) {
+    return { ok: false, error: `Invalid --${name}: ${n} is below the minimum of ${min}.` };
+  }
+  if (max !== undefined && n > max) {
+    return {
+      ok: true,
+      value: max,
+      warning: `--${name} ${n} exceeds the maximum of ${max}; using ${max}.`,
+    };
+  }
+  return { ok: true, value: n };
+}
+
+/** CLI wrapper: loud exit(1) on invalid input, stderr warning on clamp. */
+export function normalizeIntFlag(raw: unknown, spec: IntFlagSpec): number {
+  const r = parseIntFlag(raw, spec);
+  if (!r.ok) {
+    console.error(r.error);
+    process.exit(1);
+  }
+  if (r.warning) console.error(r.warning);
+  return r.value;
+}

design/src/variants.ts

@@ -8,11 +8,17 @@ import fs from "fs";
 import path from "path";
 import { requireApiKey } from "./auth";
 import { parseBrief } from "./brief";
+import { normalizeIntFlag } from "./flag-utils";
 
 export interface VariantsOptions {
   brief?: string;
   briefFile?: string;
-  count: number;
+  /**
+   * Raw CLI flag value or a number. Normalized inside variants() (#2032):
+   * nonsense errors loudly; above STYLE_VARIATIONS.length clamps with a
+   * warning — past that index variants degrade to duplicate base-brief runs.
+   */
+  count?: number | string | boolean;
   outputDir: string;
   size?: string;
   quality?: string;
@@ -153,7 +159,15 @@ export async function variants(options: VariantsOptions): Promise<void> {
     return;
   }
 
-  const count = Math.min(options.count, 7); // Cap at 7 style variations
+  // #2032: normalize at the consumption site so every caller (CLI or
+  // programmatic) gets the loud-on-nonsense contract; the ceiling derives
+  // from STYLE_VARIATIONS so it self-adjusts when styles are added.
+  const count = normalizeIntFlag(options.count, {
+    name: "count",
+    def: 3,
+    min: 1,
+    max: STYLE_VARIATIONS.length,
+  });
   const size = options.size || "1536x1024";
 
   console.error(`Generating ${count} variants...`);

test/design-flag-utils.test.ts

@@ -0,0 +1,113 @@
+/**
+ * design/src/flag-utils.ts — integer-flag contract (#2032, eng-review 7A/8A).
+ *
+ * Lives under test/ (NOT design/test/) deliberately: design/test/ is invisible
+ * to the bun test glob, scripts/test-free-shards TEST_ROOTS, and every CI
+ * workflow (eng-review 11A), so a tripwire there guards nothing. flag-utils is
+ * a pure module, so importing it from here is clean.
+ *
+ * The bug class: the design CLI parser yields string | true | undefined;
+ * parseInt produced NaN that flowed silently into loop bounds and setTimeout —
+ * `variants --count abc` generated ZERO variants and exited 0, `generate
+ * --retry abc` was a silent no-op, `serve --timeout abc` died at boot.
+ */
+
+import { describe, test, expect } from "bun:test";
+import { spawnSync } from "child_process";
+import * as path from "path";
+import { parseIntFlag } from "../design/src/flag-utils";
+
+const ROOT = path.resolve(import.meta.dir, "..");
+
+const COUNT_SPEC = { name: "count", def: 3, min: 1, max: 7 } as const;
+const RETRY_SPEC = { name: "retry", def: 0, min: 0 } as const;
+const TIMEOUT_SPEC = { name: "timeout", def: 600, min: 1 } as const;
+
+describe("parseIntFlag contract (#2032, codex 17a-c)", () => {
+  test("undefined → default (flag absent)", () => {
+    expect(parseIntFlag(undefined, COUNT_SPEC)).toEqual({ ok: true, value: 3 });
+    expect(parseIntFlag(undefined, RETRY_SPEC)).toEqual({ ok: true, value: 0 });
+  });
+
+  test("non-numeric string → error, never a silent default ('--count abc')", () => {
+    const r = parseIntFlag("abc", COUNT_SPEC);
+    expect(r.ok).toBe(false);
+    if (!r.ok) expect(r.error).toContain('"abc" is not an integer');
+  });
+
+  test("bare flag (parser yields true) → error 'requires a value'", () => {
+    const r = parseIntFlag(true, COUNT_SPEC);
+    expect(r.ok).toBe(false);
+    if (!r.ok) expect(r.error).toContain("requires a value");
+  });
+
+  test("empty string → error 'requires a value'", () => {
+    const r = parseIntFlag("", COUNT_SPEC);
+    expect(r.ok).toBe(false);
+    if (!r.ok) expect(r.error).toContain("requires a value");
+  });
+
+  test("non-integer '3.7' → error (rejected, not silently truncated to 3)", () => {
+    const r = parseIntFlag("3.7", COUNT_SPEC);
+    expect(r.ok).toBe(false);
+    if (!r.ok) expect(r.error).toContain("not an integer");
+  });
+
+  test("below min → error ('--count 0' billed the user for 1 they asked 0 of; now loud)", () => {
+    expect(parseIntFlag("0", COUNT_SPEC).ok).toBe(false);
+    expect(parseIntFlag("-2", COUNT_SPEC).ok).toBe(false);
+    // retry allows 0 (min: 0)
+    expect(parseIntFlag("0", RETRY_SPEC)).toEqual({ ok: true, value: 0 });
+  });
+
+  test("above max → clamp WITH warning (capability limit, not a user mistake)", () => {
+    const r = parseIntFlag("99", COUNT_SPEC);
+    expect(r.ok).toBe(true);
+    if (r.ok) {
+      expect(r.value).toBe(7);
+      expect(r.warning).toContain("exceeds the maximum");
+    }
+  });
+
+  test("in-range integers pass through untouched", () => {
+    expect(parseIntFlag("3", COUNT_SPEC)).toEqual({ ok: true, value: 3 });
+    expect(parseIntFlag(5, COUNT_SPEC)).toEqual({ ok: true, value: 5 });
+    expect(parseIntFlag("120", TIMEOUT_SPEC)).toEqual({ ok: true, value: 120 });
+  });
+
+  test("retry-NaN and timeout-NaN are errors, not silent no-ops (#2032 siblings)", () => {
+    // Pre-fix: --retry abc → generate() loop never ran (attempt <= NaN),
+    // printed null, exited 0. --timeout abc → setTimeout(NaN) ≈ immediate
+    // SERVE_TIMEOUT. Both members of the same NaN class, same file.
+    expect(parseIntFlag("abc", RETRY_SPEC).ok).toBe(false);
+    expect(parseIntFlag("abc", TIMEOUT_SPEC).ok).toBe(false);
+  });
+
+  test("NaN number input (legacy pre-parsed callers) → error", () => {
+    expect(parseIntFlag(Number.NaN, COUNT_SPEC).ok).toBe(false);
+  });
+});
+
+describe("normalizeIntFlag CLI wrapper (exit-1 semantics)", () => {
+  function runWrapper(rawExpr: string, specExpr: string): { status: number; stderr: string } {
+    const script = `
+      import { normalizeIntFlag } from "${ROOT}/design/src/flag-utils";
+      const v = normalizeIntFlag(${rawExpr}, ${specExpr});
+      console.log("VALUE:" + v);
+    `;
+    const res = spawnSync("bun", ["-e", script], { encoding: "utf-8", cwd: ROOT });
+    return { status: res.status ?? -1, stderr: res.stderr ?? "" };
+  }
+
+  test("invalid input exits 1 with the error on stderr", () => {
+    const r = runWrapper('"abc"', '{ name: "count", def: 3, min: 1, max: 7 }');
+    expect(r.status).toBe(1);
+    expect(r.stderr).toContain("not an integer");
+  });
+
+  test("clamp warns on stderr but exits 0", () => {
+    const r = runWrapper('"99"', '{ name: "count", def: 3, min: 1, max: 7 }');
+    expect(r.status).toBe(0);
+    expect(r.stderr).toContain("exceeds the maximum");
+  });
+});