diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index f5a0d9e40..9d74e1d3e 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -25,7 +25,7 @@ jobs: - uses: actions/checkout@v4 - id: meta - run: echo "tag=${{ env.IMAGE }}:${{ hashFiles('.github/docker/Dockerfile.ci', 'package.json', 'bun.lock') }}" >> "$GITHUB_OUTPUT" + run: echo "tag=${{ env.IMAGE }}:${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'latest' || hashFiles('.github/docker/Dockerfile.ci', 'package.json', 'bun.lock') }}" >> "$GITHUB_OUTPUT" - uses: docker/login-action@v3 with: diff --git a/design/src/cli.ts b/design/src/cli.ts index a55d92555..6b0207622 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -32,6 +32,8 @@ import { shutdownDaemon, } from "./daemon-client"; import { spawn as nodeSpawn } from "child_process"; +import fs from "fs"; +import path from "path"; function parseArgs(argv: string[]): { command: string; @@ -121,8 +123,8 @@ async function runSetup(): Promise { } } -async function main(): Promise { - const { command, flags, positionals } = parseArgs(process.argv); +async function main(argv = process.argv): Promise { + const { command, flags, positionals } = parseArgs(argv); if (!COMMANDS.has(command)) { console.error(`Unknown command: ${command}`); @@ -132,7 +134,7 @@ async function main(): Promise { switch (command) { case "generate": - await generate({ + await generateWithRoundArtifacts({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, output: (flags.output as string) || "/tmp/gstack-mockup.png", @@ -206,7 +208,7 @@ async function main(): Promise { break; case "iterate": - await iterate({ + await iterateWithRoundArtifacts({ session: flags.session as string, feedback: flags.feedback as string, output: (flags.output as string) || "/tmp/gstack-iterate.png", @@ -318,6 +320,180 @@ async function main(): Promise { } } +const ROUND_MANIFEST = ".gstack-design-rounds.json"; +const ROUND_MANIFEST_LOCK = ".gstack-design-rounds.lock"; +const ROUND_RESERVATION_LIMIT = 1000; + +interface RoundAttempt { + label: string; + path: string; + success: boolean; + error?: string; + reserved?: boolean; +} + +type RoundManifest = Record; + +interface RoundArtifactPlan { + aliasOutput: string; + primaryOutput: string; + roundKey: string; + label: string; +} + +function roundBaseName(outputPath: string): string | null { + const parsed = path.parse(outputPath); + if (parsed.ext !== ".png") return null; + if (parsed.name === "variant-recommended") return parsed.name; + if (/^variant-iteration-\d+$/.test(parsed.name)) return parsed.name; + return null; +} + +function labelForIndex(index: number): string { + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + if (index < alphabet.length) return alphabet[index]; + return `${index + 1}`; +} + +function roundKey(outputPath: string, baseName: string): string { + return path.join(path.dirname(outputPath), baseName); +} + +function readRoundManifest(dir: string): RoundManifest { + const manifestPath = path.join(dir, ROUND_MANIFEST); + if (!fs.existsSync(manifestPath)) return {}; + try { + return JSON.parse(fs.readFileSync(manifestPath, "utf-8")) as RoundManifest; + } catch { + return {}; + } +} + +function writeRoundManifest(dir: string, manifest: RoundManifest): void { + fs.mkdirSync(dir, { recursive: true }); + const manifestPath = path.join(dir, ROUND_MANIFEST); + const tempPath = path.join(dir, `${ROUND_MANIFEST}.${process.pid}.${Date.now()}.tmp`); + fs.writeFileSync(tempPath, JSON.stringify(manifest, null, 2)); + fs.renameSync(tempPath, manifestPath); +} + +function withRoundManifestLock(dir: string, action: () => T): T { + fs.mkdirSync(dir, { recursive: true }); + const lockPath = path.join(dir, ROUND_MANIFEST_LOCK); + let lockFd: number | undefined; + + for (let attempt = 0; attempt < ROUND_RESERVATION_LIMIT; attempt++) { + try { + lockFd = fs.openSync(lockPath, "wx"); + break; + } catch (err: any) { + if (err.code !== "EEXIST") throw err; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 1); + } + } + + if (lockFd === undefined) { + throw new Error(`Timed out reserving design round artifacts in ${dir}`); + } + + try { + return action(); + } finally { + fs.closeSync(lockFd); + fs.unlinkSync(lockPath); + } +} + +function planRoundArtifacts(outputPath: string): RoundArtifactPlan | null { + const baseName = roundBaseName(outputPath); + if (!baseName) return null; + + const dir = path.dirname(outputPath); + const key = roundKey(outputPath, baseName); + return withRoundManifestLock(dir, () => { + const manifest = readRoundManifest(dir); + const attempts = manifest[key] || []; + + for (let index = attempts.length; index < attempts.length + ROUND_RESERVATION_LIMIT; index++) { + const label = labelForIndex(index); + const primaryOutput = path.join(dir, `${baseName}-${label}.png`); + try { + fs.closeSync(fs.openSync(primaryOutput, "wx")); + } catch (err: any) { + if (err.code === "EEXIST") continue; + throw err; + } + + attempts.push({ label, path: primaryOutput, success: false, reserved: true }); + manifest[key] = attempts; + writeRoundManifest(dir, manifest); + return { aliasOutput: outputPath, primaryOutput, roundKey: key, label }; + } + + throw new Error(`Unable to reserve a design round artifact after ${ROUND_RESERVATION_LIMIT} attempts`); + }); +} + +function recordRoundAttempt(plan: RoundArtifactPlan, success: boolean, error?: string): void { + const dir = path.dirname(plan.aliasOutput); + withRoundManifestLock(dir, () => { + const manifest = readRoundManifest(dir); + const attempts = manifest[plan.roundKey] || []; + const existing = attempts.find(attempt => attempt.label === plan.label); + const attempt = { label: plan.label, path: plan.primaryOutput, success, error }; + if (existing) { + Object.assign(existing, attempt); + } else { + attempts.push(attempt); + } + manifest[plan.roundKey] = attempts; + writeRoundManifest(dir, manifest); + + if (!success && fs.existsSync(plan.primaryOutput) && fs.statSync(plan.primaryOutput).size === 0) { + fs.unlinkSync(plan.primaryOutput); + } + }); +} + +function copyRoundAlias(plan: RoundArtifactPlan): void { + if (plan.primaryOutput === plan.aliasOutput) return; + fs.copyFileSync(plan.primaryOutput, plan.aliasOutput); +} + +async function generateWithRoundArtifacts(options: Parameters[0]): Promise { + const plan = planRoundArtifacts(options.output); + if (!plan) { + await generate(options); + return; + } + + try { + await generate({ ...options, output: plan.primaryOutput }); + recordRoundAttempt(plan, true); + copyRoundAlias(plan); + } catch (err: any) { + recordRoundAttempt(plan, false, err.message || String(err)); + throw err; + } +} + +async function iterateWithRoundArtifacts(options: Parameters[0]): Promise { + const plan = planRoundArtifacts(options.output); + if (!plan) { + await iterate(options); + return; + } + + try { + await iterate({ ...options, output: plan.primaryOutput }); + recordRoundAttempt(plan, true); + copyRoundAlias(plan); + } catch (err: any) { + recordRoundAttempt(plan, false, err.message || String(err)); + throw err; + } +} + /** * Default `$D compare --serve` path: ensure the persistent daemon is up, * publish the board, open the browser to its URL, then exit. The daemon @@ -400,7 +576,88 @@ async function resolveImagePaths(input: string): Promise { } // Comma-separated or single path - return input.split(",").map(p => p.trim()); + const resolved: string[] = []; + const missing: string[] = []; + for (const imagePath of input.split(",").map(p => p.trim()).filter(Boolean)) { + const roundImages = resolveRoundImageAlias(imagePath); + if (roundImages) { + resolved.push(...roundImages.paths); + missing.push(...roundImages.missing); + } else { + resolved.push(imagePath); + if (!fs.existsSync(imagePath)) missing.push(path.basename(imagePath)); + } + } + + if (missing.length > 0) { + throw new Error(`Missing generated design variants: ${missing.join(", ")}`); + } + + return resolved; +} + +function resolveRoundImageAlias(imagePath: string): { paths: string[]; missing: string[] } | null { + const baseName = roundBaseName(imagePath); + if (!baseName) return null; + + const dir = path.dirname(imagePath); + const manifest = readRoundManifest(dir); + const attempts = manifest[roundKey(imagePath, baseName)] || []; + + if (attempts.length > 0) { + return { + paths: attempts.filter(attempt => attempt.success && fs.existsSync(attempt.path)).map(attempt => attempt.path), + missing: attempts + .filter(attempt => !attempt.success || !fs.existsSync(attempt.path)) + .map(attempt => `${baseName}-${attempt.label}.png`), + }; + } + + const discovered = discoverRoundImages(dir, baseName); + if (discovered.paths.length > 0 || discovered.missing.length > 0) { + return discovered; + } + + if (fs.existsSync(imagePath)) { + return { paths: [imagePath], missing: [] }; + } + + return { paths: [], missing: [path.basename(imagePath)] }; +} + +function discoverRoundImages(dir: string, baseName: string): { paths: string[]; missing: string[] } { + if (!fs.existsSync(dir)) return { paths: [], missing: [] }; + + const matches = fs.readdirSync(dir) + .map(name => { + const match = name.match(new RegExp(`^${escapeRegExp(baseName)}-([A-Z])\\.png$`)); + return match ? { label: match[1], name } : null; + }) + .filter((match): match is { label: string; name: string } => match !== null) + .sort((a, b) => a.label.localeCompare(b.label)); + + if (matches.length === 0) return { paths: [], missing: [] }; + + const highestIndex = matches[matches.length - 1].label.charCodeAt(0) - 65; + const byLabel = new Map(matches.map(match => [match.label, match.name])); + const paths: string[] = []; + const missing: string[] = []; + + for (let i = 0; i <= highestIndex; i++) { + const label = labelForIndex(i); + const name = byLabel.get(label); + if (name) { + paths.push(path.join(dir, name)); + } else { + missing.push(`${baseName}-${label}.png`); + } + } + + return { paths, missing }; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } // Self-execution shortcut: when invoked with --daemon-mode, this same @@ -408,14 +665,22 @@ async function resolveImagePaths(input: string): Promise { // the production install to a single executable; daemon-client.ts spawns // ` --daemon-mode` (or `bun run cli.ts --daemon-mode` in dev) // rather than relying on a separate daemon.ts file at a known path. -if (process.argv.includes("--daemon-mode")) { +if (import.meta.main && process.argv.includes("--daemon-mode")) { const { start } = await import("./daemon"); start(); // start() binds Bun.serve and registers signal handlers; this branch // never falls through to main(). Process stays alive on the bound port. -} else { +} else if (import.meta.main) { main().catch((err) => { console.error(err.message || err); process.exit(1); }); } + +export { + main, + resolveImagePaths, + planRoundArtifacts, + recordRoundAttempt, + resolveRoundImageAlias, +}; diff --git a/design/test/variant-preservation.test.ts b/design/test/variant-preservation.test.ts new file mode 100644 index 000000000..fc53d9993 --- /dev/null +++ b/design/test/variant-preservation.test.ts @@ -0,0 +1,160 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { + planRoundArtifacts, + recordRoundAttempt, + resolveImagePaths, +} from "../src/cli"; + +const PNG_BYTES = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=", + "base64", +); + +function writePng(filePath: string): void { + fs.writeFileSync(filePath, PNG_BYTES); +} + +function writeManifest(tmpDir: string, entries: Record): void { + fs.writeFileSync( + path.join(tmpDir, ".gstack-design-rounds.json"), + JSON.stringify(entries, null, 2), + ); +} + +describe("plan-design-review round variant preservation", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "variant-preservation-")); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("recommended round allocates sequence-stable suffixed paths", () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const first = planRoundArtifacts(alias); + + expect(first?.primaryOutput).toBe(path.join(tmpDir, "variant-recommended-A.png")); + expect(first?.aliasOutput).toBe(alias); + + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: first!.primaryOutput, success: true }, + ], + }); + + const second = planRoundArtifacts(alias); + expect(second?.primaryOutput).toBe(path.join(tmpDir, "variant-recommended-B.png")); + }); + + test("concurrent round reservations claim distinct paths and preserve every variant", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const plans = await Promise.all( + Array.from({ length: 3 }, async () => { + const plan = planRoundArtifacts(alias)!; + await Promise.resolve(); + writePng(plan.primaryOutput); + recordRoundAttempt(plan, true); + return plan; + }), + ); + + expect(plans.map(plan => plan.label).sort()).toEqual(["A", "B", "C"]); + expect(new Set(plans.map(plan => plan.primaryOutput)).size).toBe(3); + expect(plans.every(plan => fs.existsSync(plan.primaryOutput))).toBe(true); + await expect(resolveImagePaths(alias)).resolves.toEqual( + plans.map(plan => plan.primaryOutput), + ); + }); + + test("failed reservations are recorded without blocking a later variant", () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const failed = planRoundArtifacts(alias)!; + recordRoundAttempt(failed, false, "API error"); + + const retry = planRoundArtifacts(alias)!; + writePng(retry.primaryOutput); + recordRoundAttempt(retry, true); + + expect(retry.label).toBe("B"); + expect(fs.existsSync(failed.primaryOutput)).toBe(false); + expect(fs.existsSync(retry.primaryOutput)).toBe(true); + }); + + test("recommended round alias expands to every successful generated candidate", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + const variantB = path.join(tmpDir, "variant-recommended-B.png"); + writePng(variantA); + writePng(variantB); + writePng(alias); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + { label: "B", path: variantB, success: true }, + ], + }); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA, variantB]); + }); + + test("iteration round discovers A/B/C candidates without collapsing onto the alias", async () => { + const alias = path.join(tmpDir, "variant-iteration-01.png"); + const variantA = path.join(tmpDir, "variant-iteration-01-A.png"); + const variantB = path.join(tmpDir, "variant-iteration-01-B.png"); + const variantC = path.join(tmpDir, "variant-iteration-01-C.png"); + writePng(variantA); + writePng(variantB); + writePng(variantC); + writePng(alias); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA, variantB, variantC]); + }); + + test("single recommended generation preserves the suffixed candidate and ignores alias copy", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + writePng(variantA); + writePng(alias); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + ], + }); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA]); + }); + + test("failed sibling leaves successful candidate in place and reports missing index", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + const variantB = path.join(tmpDir, "variant-recommended-B.png"); + writePng(variantA); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + { label: "B", path: variantB, success: false, error: "API error" }, + ], + }); + + await expect(resolveImagePaths(alias)).rejects.toThrow("variant-recommended-B.png"); + expect(fs.existsSync(variantA)).toBe(true); + }); + + test("initial 3-option board paths are unchanged", async () => { + const variantA = path.join(tmpDir, "variant-A.png"); + const variantB = path.join(tmpDir, "variant-B.png"); + const variantC = path.join(tmpDir, "variant-C.png"); + writePng(variantA); + writePng(variantB); + writePng(variantC); + + const input = `${variantA},${variantB},${variantC}`; + await expect(resolveImagePaths(input)).resolves.toEqual([variantA, variantB, variantC]); + }); +});