From fe6d1ae62a42e67bfb12b7f8e6143c705f375f38 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 3 Oct 2026 16:13:45 -0700 Subject: [PATCH] v1.91.17.0 fix: mvanhorn wave (checkpoint provenance, never-overwrite design images) (#3022) * fix: mark Remaining Work provenance so restore verifies guessed steps first (#3004) /context-save now starts every Remaining Work item with `Open.` and ends it with how this session knows it: `(path run)` with the observed outcome, `(path read)` or `(path assumed)` for concrete paths, and `(target state checked)`, `(code read)` or `(path assumed)` for writing steps. /context-restore keeps saved order and original text but splits the list into Next steps and Verify first (assumed, code-read, failed runs and unmarked concrete or writing items), prints a banner for pre-marker checkpoints, and option A walks saved order so an unverified first item is verified before any later step runs. Adds a prompt-byte contract test for the marker wire format and a periodic behavior E2E (context-restore-provenance-order) that seeds an assumed item ahead of a run item plus a code-read write. Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * fix(design): never overwrite a paid design image (#1529) Every image the API returns now goes through one persistence path (design/src/persist.ts): mkdir, exclusive `wx` claim that bumps x.png -> x-2.png ... x-999.png, write, fsync, close. A failure after the bytes arrived removes the claimed file and keeps a 0600 recovery copy in the temp dir; empty image data is a failure, never a 0-byte PNG. The API call is the only thing inside each retry loop, so a local save failure never buys a second image (variants, iterate fallback, generate --retry). generate, iterate, evolve and variants (both branches) print the same run fields (requested, saved, selected, failures, recovered) on every exit, with exit 0 / 2 (nothing saved) / 3 (stopped after saving). Each generate --retry attempt is its own saved image listed in `attempts`. compare accepts --images-file and JSON-array --images. The gallery lists bumped names, orders history by mtime then board-images.json, and marks approvals by approved_path. New bin/gstack-design-claim publishes staged images without clobbering; bin/gstack-design-approved resolves an approval record (legacy letter-only records still resolve; a missing approved_path is an error, never a substitute). Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * fix(design skills): read the paths $D printed, never fixed variant names (#1529) Every design flow now captures each generating $D command's JSON and exit code without set -e, does round accounting before the first board, check or inline view (stopping when nothing was saved), and carries the printed paths forward. Boards are built from board-images.json via compare --images-file after archiving any earlier feedback.json, and approvals map the confirmed letter through that file and record approved_path. design-shotgun subagents generate into a per-run mktemp staging directory and publish with bin/gstack-design-claim instead of a shared /tmp/variant-X.png plus cp. design-html and taste memory resolve approvals with gstack-design-approved; design-review verifies against the printed target path. test/design-printed-paths.test.ts bans fixed variant names, variant globs, variant-{letter} reads, cp/mv of $D output and literal check paths in the generated skills and resolvers (one content-anchored allowlist entry: the design-html discovery probe), and pins the accounting/board marker order. test/design-approval-flow.test.ts runs the generated board and approval blocks through a stubbed two-round flow. * chore: release v1.91.17.0 (mvanhorn fix wave: checkpoint provenance, never-overwrite design images) * fix(design): keep iterate sessions in the OS temp dir and normalize tripwire paths on Windows * fix(setup-deploy): report failing status commands, specify merge-method, health and workflow checks --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- CHANGELOG.md | 40 ++ TODOS.md | 25 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-design-approved | 24 ++ bin/gstack-design-claim | 55 +++ context-restore/SKILL.md | 29 +- context-restore/SKILL.md.tmpl | 29 +- context-save/SKILL.md | 24 +- context-save/SKILL.md.tmpl | 24 +- design-consultation/SKILL.md | 10 +- .../sections/proposal-and-preview.md | 28 +- .../sections/proposal-and-preview.md.tmpl | 14 +- design-html/SKILL.md | 21 +- design-html/SKILL.md.tmpl | 13 +- design-review/SKILL.md | 15 +- design-review/SKILL.md.tmpl | 7 +- design-shotgun/SKILL.md | 111 +++--- design-shotgun/SKILL.md.tmpl | 80 ++-- design/src/approval.ts | 56 +++ design/src/auth.ts | 19 +- design/src/cli.ts | 65 ++-- design/src/commands.ts | 41 +- design/src/evolve.ts | 99 +++-- design/src/gallery.ts | 43 ++- design/src/generate.ts | 134 ++++--- design/src/image-args.ts | 53 +++ design/src/iterate.ts | 103 ++--- design/src/persist.ts | 196 ++++++++++ design/src/session.ts | 5 +- design/src/variants.ts | 357 ++++++++++-------- design/test/approval.test.ts | 61 +++ design/test/cli-persistence.test.ts | 223 +++++++++++ design/test/gallery.test.ts | 36 +- design/test/image-args.test.ts | 52 +++ design/test/models.test.ts | 18 +- design/test/persist.test.ts | 165 ++++++++ office-hours/SKILL.md | 30 +- package.json | 2 +- plan-design-review/SKILL.md | 46 ++- plan-design-review/SKILL.md.tmpl | 17 +- .../sections/review-sections.md | 4 +- .../sections/review-sections.md.tmpl | 4 +- scripts/resolvers/design.ts | 87 +++-- setup-deploy/SKILL.md | 13 +- setup-deploy/SKILL.md.tmpl | 13 +- .../context-remaining-work-provenance.test.ts | 63 ++++ test/design-approval-flow.test.ts | 97 +++++ test/design-printed-paths.test.ts | 165 ++++++++ test/gstack-design-claim.test.ts | 118 ++++++ test/helpers/touchfiles-data.ts | 5 + test/skill-e2e-context-skills.test.ts | 69 ++++ 52 files changed, 2423 insertions(+), 589 deletions(-) create mode 100755 bin/gstack-design-approved create mode 100755 bin/gstack-design-claim create mode 100644 design/src/approval.ts create mode 100644 design/src/image-args.ts create mode 100644 design/src/persist.ts create mode 100644 design/test/approval.test.ts create mode 100644 design/test/cli-persistence.test.ts create mode 100644 design/test/image-args.test.ts create mode 100644 design/test/persist.test.ts create mode 100644 test/context-remaining-work-provenance.test.ts create mode 100644 test/design-approval-flow.test.ts create mode 100644 test/design-printed-paths.test.ts create mode 100644 test/gstack-design-claim.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e70b8e951..759c9b9d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,45 @@ # Changelog +## [1.91.17.0] - 2026-10-03 + +**Checkpoints say whether each next step was run, read or guessed, and `/context-restore` verifies the guesses first.** +**Paid design images are never overwritten, and design skills always show the round you just paid for.** + +This wave rebuilds three community PRs from @mvanhorn on current main. A checkpoint's Remaining Work used to read the same whether a step had been run or only inferred, so a resumed session could act on a guess and hit the wrong system (#3004, reported by @tomg65). Running `$D variants` twice into one folder replaced `variant-A.png`, `$D generate --retry` kept only the last image, and skills built boards from fixed `variant-A/B/C` names, so you could pay for images you never saw (#1529, reported by @SakenW). + +### What changes for you + +- **Remaining Work carries provenance.** Every item starts with `Open.` and ends with how the saving session knows it: `(path run)` with the outcome, `(path read)` or `(path assumed)` for commands, flags, config values and files; `(target state checked)`, `(code read)` or `(path assumed)` for writes. For example: + + 1. Open. Run the migration check with SWITCH_B=1 against staging. (path assumed) + 2. Open. Run bun test test/billing.test.ts. (path run) exit 0 + + `/context-restore` keeps the saved order but shows item 2 under **Next steps** and item 1 under **Verify first**, and "continue" starts by verifying item 1 read-only instead of jumping to item 2. Checkpoints saved before this release have no markers, so their command, path and write items land under Verify first, with a one-line banner saying so. +- **Nothing paid is lost.** A taken image name gets `-2`, `-3` and so on (`variant-A-2.png`); stderr says `note: exists; saved to (existing file kept)` and the JSON reports the real path. Every `--retry` attempt is its own saved image. If saving fails after the API returned the image, the bytes go to a private recovery copy in your temp directory and the message says where; a local write failure never buys a second image. +- **Every image command prints JSON, even on failure**: `requested`, `saved`, `selected`, `failures`, `recovered`. Exit 0 means the result is ready, 2 means nothing was saved, 3 means the run stopped after saving some images. +- **Boards show this round only.** Skills tell you how many of the requested images were saved, write the round's printed paths to `board-images.json`, build the board with `$D compare --images-file`, archive an old `feedback.json` first, and record `approved_path` in `approved.json`, so a later session opens exactly the image you approved. Old letter-only approvals still work. /design-shotgun subagents stage in a fresh temp directory and publish with `gstack-design-claim`, which never overwrites. +- **If you script `$D`,** read paths from the JSON instead of assuming the file name: + + out=$($D generate --brief "pricing page" --output designs/pricing.png); rc=$? + img=$(printf '%s' "$out" | jq -r .outputPath) + [ "$rc" -eq 0 ] && $D check --image "$img" --brief "pricing page" + + `variants` prints its list in `.paths`; `compare` takes it losslessly with `--images-file paths.json`. +- **Windows console flashes (#1784)** were confirmed fixed on main (v1.91.15.0) by @kaiwulff on a real Windows 11 desktop; this release ships no code for them. + +### Itemized changes + +#### Fixed +- `/context-save` Remaining Work did not record whether a step was run, read or assumed, and `/context-restore` offered the first item as the next action (#3004). Marker format from #3019 by @mvanhorn, field-tested by @tomg65. +- Design images were silently overwritten by later rounds and `--retry` attempts (#1529). Exclusive-create claim idea from #1737 by @mvanhorn. +- Comparison boards and quality checks read fixed `variant-A/B/C` names or a `variant-*.png` glob and mixed rounds; a stale `feedback.json` could approve a new round. +- `$D` wrote iterate session files to `/tmp`, which does not exist on Windows, so `generate` could not record its session and `iterate` could not find it there; sessions now live in the OS temp directory. +- /setup-deploy reported a failing status command as success (the pipe hid its exit code), matched `cd` anywhere in a workflow as a deploy workflow, and left the merge-method check, Fly `/health` detection and GitHub-Actions-only status command unspecified. + +#### Added +- `$D compare --images-file` and JSON-array `--images`, `bin/gstack-design-claim`, `bin/gstack-design-approved`, and `approved_path` in approval records. +- Periodic behavior eval `context-restore-provenance-order` (3-trial panel) and free contract tests for the provenance markers, `$D` persistence and the design-skill path rules. + ## [1.91.16.0] - 2026-10-03 **Installing or upgrading gstack never quietly changes another copy, and `./setup --status` shows every install.** diff --git a/TODOS.md b/TODOS.md index 7fbbaf3dd..83b18db0a 100644 --- a/TODOS.md +++ b/TODOS.md @@ -33,6 +33,31 @@ trials per case, apply the CASE_QUARANTINE entry rule instead of chasing one run at a time. Effort S. +### P2/P3: mvanhorn fix-wave deferrals (filed 2026-10-03, from the autoplan review of the wave) + +- **E6 (P2): behavior E2E for design round accounting** — stub `$D` so a + generating step reports `saved` shorter than `requested` (and exit 2 for zero + saved), run /plan-design-review or /design-shotgun against it, and assert the + skill tells the user how many paid images were saved, names the failures and + builds no board on zero saved. `test/design-printed-paths.test.ts` only pins + the marker order; whether the model reports the count is behavior. Context: + #1529. Effort M. +- **E7 (P3): prune or archive unapproved design rounds** — never-overwrite plus + `--retry` attempts and iterate outputs grow + `$GSTACK_STATE_ROOT/projects/$SLUG/designs/` without bound. Add a prune or + archive command for unapproved rounds and attempts (keep anything an + `approved.json` names) and show the design directory size in `$D gallery`. + Context: #1529, eng review. Effort S. +- **E5 (P3): restore verifies assumed items read-only** — `/context-restore` + could check `(path assumed)` and `(code read)` items itself with read-only + commands (file exists, flag is defined, table has a unique key) and report + what it found before offering option A. Context: #3004. Effort M. +- **E4 (P3): real-desktop Windows console smoke for spawn changes** — #1784 was + confirmed fixed on a real Windows 11 desktop by its reporter (v1.91.15.0); CI + can only check that spawns pass `windowsHide`. A smoke that starts browse from + a console-less parent and watches for new visible windows would catch a + regression the static sweep can't. Effort M. + ### P2/P3: parallel fix-wave follow-ups (filed 2026-10-02, from the approved fix-wave plan) - **Agent Skills distribution (#113)** — publish gstack in the Agent Skills diff --git a/VERSION b/VERSION index 448b3dfa7..456d26b01 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.16.0 +1.91.17.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index b25d702b2..7afe1f5d7 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.16.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.17.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-design-approved b/bin/gstack-design-approved new file mode 100755 index 000000000..7c3f360a2 --- /dev/null +++ b/bin/gstack-design-approved @@ -0,0 +1,24 @@ +#!/usr/bin/env bun +// gstack-design-approved — print the absolute path of the image an +// approved.json names. +// +// Usage: gstack-design-approved +// +// Prefers approved_path (relative to approved.json's directory). Legacy +// records with only approved_variant resolve to variant-.png. When the +// named image is missing, exits 1 with "approved image is missing; +// reselect from the board" and never substitutes another image. + +import { resolveApprovedImage } from '../design/src/approval'; + +const file = process.argv[2]; +if (!file) { + console.error('Usage: gstack-design-approved '); + process.exit(1); +} +const resolved = resolveApprovedImage(file); +if (!resolved.ok) { + console.error(resolved.error); + process.exit(1); +} +console.log(resolved.path); diff --git a/bin/gstack-design-claim b/bin/gstack-design-claim new file mode 100755 index 000000000..88a8c9f1f --- /dev/null +++ b/bin/gstack-design-claim @@ -0,0 +1,55 @@ +#!/bin/sh +# gstack-design-claim — publish a staged design image without overwriting. +# +# Usage: gstack-design-claim +# +# Creates with an exclusive, non-overwriting create (set -C +# noclobber, which opens with O_EXCL). When the name is taken it tries +# -2, -3 ... -999, the same rule as +# $D's claimOutputPath (x-2.png bumps to x-2-2.png). Prints the final path. +# On any failure it exits non-zero with problem/cause/fix and leaves the staged +# image in place, so the paid bytes survive. On success the staged copy is +# removed. + +set -u + +fail() { + printf 'gstack-design-claim: %s\ncause: %s\nfix: %s\n' "$1" "$2" "$3" >&2 + exit 1 +} + +[ "$#" -eq 2 ] || fail "expected 2 arguments, got $#" "usage: gstack-design-claim " "pass the staged image path, then the destination path" +src=$1 +dest=$2 + +[ -f "$src" ] && [ -s "$src" ] || fail "staged image $src is missing or empty" "the generating command did not save an image there" "use the outputPath the \$D command printed" + +dir=$(dirname "$dest") +base=$(basename "$dest") +case "$base" in + ?*.*) stem=${base%.*}; ext=".${base##*.}" ;; + *) stem=$base; ext="" ;; +esac + +err=$(mkdir -p "$dir" 2>&1) || fail "cannot create $dir; staged image kept at $src" "$err" "make $dir writable or choose another destination" + +n=1 +candidate=$dest +while :; do + if err=$( (set -C; : > "$candidate") 2>&1 ); then + if err=$(cat "$src" 2>&1 > "$candidate"); then + rm -f "$src" + printf '%s\n' "$candidate" + exit 0 + fi + rm -f "$candidate" + fail "cannot write $candidate; staged image kept at $src" "$err" "free disk space or make $dir writable, then rerun" + fi + if [ -e "$candidate" ] || [ -L "$candidate" ]; then + n=$((n + 1)) + [ "$n" -le 999 ] || fail "$dest and every name through $stem-999$ext already exist; staged image kept at $src" "999 images already use this name" "choose a new destination name or move older images out of $dir" + candidate="$dir/$stem-$n$ext" + continue + fi + fail "cannot create $candidate; staged image kept at $src" "$err" "make $dir writable or choose another destination" +done diff --git a/context-restore/SKILL.md b/context-restore/SKILL.md index f552fe9e1..d0345843b 100644 --- a/context-restore/SKILL.md +++ b/context-restore/SKILL.md @@ -506,6 +506,22 @@ checkpoint(s) from another project; only this project's are listed." If it print `LEGACY_BUCKET`, relay that line. If the chosen file is `ROOT_DIFFERS`, add "Saved from another checkout of this repository at `{project_root}`." as info. +**Sort Remaining Work by provenance.** Keep every item's original text and saved +order, and drop nothing. Put an item under **Verify first** when it: +- ends in `(path assumed)` or `(code read)`; +- ends in `(path run)` but its text reports a failure; +- has no marker and is a writing step (migration, sync, insert, import, a dialog that + writes) or names a concrete path (a runnable command, CLI flag or switch, config key + or value, or file or directory path). + +Every other item goes under **Next steps**: `(path run)` with a successful outcome, +`(path read)`, `(target state checked)`, and unmarked items that neither write nor +name a concrete path. Verifying means read-only inspection: read the file or the +target, or run a command that changes nothing. Checkpoints saved before provenance +markers existed have none, so their concrete-path items land under Verify first. When +the file has no provenance markers at all, print this line above the groups: +`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.` + Read the chosen file and present a summary: ``` @@ -522,7 +538,13 @@ Status: {status} {summary from saved file} ### Remaining Work -{remaining work items} +{legacy banner line, if it applies} + +Next steps +{Next steps items, in saved order, original text} + +Verify first (inspect read-only before executing anything) +{Verify first items, in saved order, original text} ### Notes {notes} @@ -540,7 +562,10 @@ After presenting, ask via AskUserQuestion: - B) Show the full saved file - C) Just needed the context, thanks -If A, summarize the first remaining work item and suggest starting there. +If A, take the first Remaining Work item in saved order. If it is under Next steps, +suggest starting there. If it is under Verify first, suggest verifying it (read-only) +before doing it or any later item, so a later runnable step never jumps ahead of an +unverified earlier one. --- diff --git a/context-restore/SKILL.md.tmpl b/context-restore/SKILL.md.tmpl index 4ba1571c9..55db95b21 100644 --- a/context-restore/SKILL.md.tmpl +++ b/context-restore/SKILL.md.tmpl @@ -180,6 +180,22 @@ checkpoint(s) from another project; only this project's are listed." If it print `LEGACY_BUCKET`, relay that line. If the chosen file is `ROOT_DIFFERS`, add "Saved from another checkout of this repository at `{project_root}`." as info. +**Sort Remaining Work by provenance.** Keep every item's original text and saved +order, and drop nothing. Put an item under **Verify first** when it: +- ends in `(path assumed)` or `(code read)`; +- ends in `(path run)` but its text reports a failure; +- has no marker and is a writing step (migration, sync, insert, import, a dialog that + writes) or names a concrete path (a runnable command, CLI flag or switch, config key + or value, or file or directory path). + +Every other item goes under **Next steps**: `(path run)` with a successful outcome, +`(path read)`, `(target state checked)`, and unmarked items that neither write nor +name a concrete path. Verifying means read-only inspection: read the file or the +target, or run a command that changes nothing. Checkpoints saved before provenance +markers existed have none, so their concrete-path items land under Verify first. When +the file has no provenance markers at all, print this line above the groups: +`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.` + Read the chosen file and present a summary: ``` @@ -196,7 +212,13 @@ Status: {status} {summary from saved file} ### Remaining Work -{remaining work items} +{legacy banner line, if it applies} + +Next steps +{Next steps items, in saved order, original text} + +Verify first (inspect read-only before executing anything) +{Verify first items, in saved order, original text} ### Notes {notes} @@ -214,7 +236,10 @@ After presenting, ask via AskUserQuestion: - B) Show the full saved file - C) Just needed the context, thanks -If A, summarize the first remaining work item and suggest starting there. +If A, take the first Remaining Work item in saved order. If it is under Next steps, +suggest starting there. If it is under Verify first, suggest verifying it (read-only) +before doing it or any later item, so a later runnable step never jumps ahead of an +unverified earlier one. --- diff --git a/context-save/SKILL.md b/context-save/SKILL.md index 803e7db3a..bdc3997b2 100644 --- a/context-save/SKILL.md +++ b/context-save/SKILL.md @@ -404,10 +404,30 @@ Using the gathered state plus your conversation history, produce a summary cover 1. **What's being worked on** — the high-level goal or feature 2. **Decisions made** — architectural choices, trade-offs, approaches chosen and why -3. **Remaining work** — concrete next steps, in priority order +3. **Remaining work** — concrete next steps, in priority order, each marked with + how this session knows it (see the provenance table below) 4. **Notes** — anything a future session needs to know (gotchas, blocked items, open questions, things that were tried and didn't work) +**Remaining Work provenance.** Every item starts with the status `Open.` and never +with a completion token such as `[done]` or `[executed]`. Then pick its trailing +marker by the first rule that applies: + +| # | The item is... | Marker | Use when this session... | +|---|----------------|--------|--------------------------| +| 1 | a writing step (migration, sync, insert, import, a dialog that writes), with or without a concrete path | `(target state checked)` | inspected the target's schema or state (for example the unique key) | +| | | `(code read)` | read the writer but not the target | +| | | `(path assumed)` | inferred the write without reading either | +| 2 | otherwise, names a concrete path: a runnable command, CLI flag or switch, config key or value, or file or directory path | `(path run)` | executed it and observed the outcome; state the outcome in the item (`exit 0`, `failed: `) | +| | | `(path read)` | read the code or file but did not execute it | +| | | `(path assumed)` | inferred it without running or reading it, however confident | +| 3 | anything else | none | (`Open.` alone) | + +Writing steps have their own markers because whether a write is safe to repeat +depends on the target's state, not on the code that writes. Examples: +`Open. Run the suite with CONFIG=staging. (path run) exit 0` and +`Open. Switch B reads config Y. (path assumed)`. + If the user provided a title, use it. Otherwise, infer a concise title (3-6 words) from the work being done. @@ -496,7 +516,7 @@ files_modified: ### Remaining Work -{Numbered list of concrete next steps, in priority order} +{Numbered list of concrete next steps, in priority order. Each item starts with `Open.` and ends with its provenance marker from Step 2, or no marker under rule 3.} ### Notes diff --git a/context-save/SKILL.md.tmpl b/context-save/SKILL.md.tmpl index d9bfef977..26d38cb6a 100644 --- a/context-save/SKILL.md.tmpl +++ b/context-save/SKILL.md.tmpl @@ -79,10 +79,30 @@ Using the gathered state plus your conversation history, produce a summary cover 1. **What's being worked on** — the high-level goal or feature 2. **Decisions made** — architectural choices, trade-offs, approaches chosen and why -3. **Remaining work** — concrete next steps, in priority order +3. **Remaining work** — concrete next steps, in priority order, each marked with + how this session knows it (see the provenance table below) 4. **Notes** — anything a future session needs to know (gotchas, blocked items, open questions, things that were tried and didn't work) +**Remaining Work provenance.** Every item starts with the status `Open.` and never +with a completion token such as `[done]` or `[executed]`. Then pick its trailing +marker by the first rule that applies: + +| # | The item is... | Marker | Use when this session... | +|---|----------------|--------|--------------------------| +| 1 | a writing step (migration, sync, insert, import, a dialog that writes), with or without a concrete path | `(target state checked)` | inspected the target's schema or state (for example the unique key) | +| | | `(code read)` | read the writer but not the target | +| | | `(path assumed)` | inferred the write without reading either | +| 2 | otherwise, names a concrete path: a runnable command, CLI flag or switch, config key or value, or file or directory path | `(path run)` | executed it and observed the outcome; state the outcome in the item (`exit 0`, `failed: `) | +| | | `(path read)` | read the code or file but did not execute it | +| | | `(path assumed)` | inferred it without running or reading it, however confident | +| 3 | anything else | none | (`Open.` alone) | + +Writing steps have their own markers because whether a write is safe to repeat +depends on the target's state, not on the code that writes. Examples: +`Open. Run the suite with CONFIG=staging. (path run) exit 0` and +`Open. Switch B reads config Y. (path assumed)`. + If the user provided a title, use it. Otherwise, infer a concise title (3-6 words) from the work being done. @@ -170,7 +190,7 @@ files_modified: ### Remaining Work -{Numbered list of concrete next steps, in priority order} +{Numbered list of concrete next steps, in priority order. Each item starts with `Open.` and ends with its provenance marker from Step 2, or no marker under rule 3.} ### Notes diff --git a/design-consultation/SKILL.md b/design-consultation/SKILL.md index 6922787e6..ea3d6829f 100644 --- a/design-consultation/SKILL.md +++ b/design-consultation/SKILL.md @@ -550,12 +550,14 @@ For interactive feedback, use `compare --serve` and its printed HTTP URL; openin If `DESIGN_READY`: the design binary is available for visual mockup generation. Commands: -- `$D generate --brief "..." --output /path.png` — generate a single mockup -- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants -- `$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve` — comparison board + HTTP server +- `$D generate --brief "..." --output /path.png` — generate a single mockup (prints `outputPath`) +- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants (prints `paths`) +- `$D compare --images-file /path/board-images.json --output /path/board.html --serve` — comparison board + HTTP server - `$D serve --html /path/board.html` — serve comparison board and collect feedback via HTTP - `$D check --image /path.png --brief "..."` — vision quality gate - `$D iterate --session /path/session.json --feedback "..." --output /path.png` — iterate + +Image commands never overwrite (a taken name gets `-2`) and always print JSON (`requested`, `saved`, `failures`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without `set -e`: `_OUT=$($D ...); _RC=$?`. - `$D extract --image /absolute/path.png` — print tokens and automatically update DESIGN.md in the current Git repository; no read-only flag `generate` returns `sessionFile`; `iterate` requires that existing session. `variants` returns `paths` but creates no session: regenerate with an updated brief instead. @@ -662,7 +664,7 @@ fonts [top-3], colors [top-3], layouts [top-3], aesthetics [top-3]. Bias generation toward these unless the user explicitly requests a different direction. Also avoid their strong rejections: [top-3 rejected per dimension]." -**Legacy fallback:** Glob `$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json` (resolve the root with gstack-paths); Read the five newest. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. +**Legacy fallback:** Glob `$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json` (resolve the root with gstack-paths); Read the five newest. To view an approved image, resolve it with `~/.claude/skills/gstack/bin/gstack-design-approved `. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. **Conflict handling:** If the current user request contradicts a strong persistent signal (e.g., "make it playful" when taste profile strongly prefers minimal), flag diff --git a/design-consultation/sections/proposal-and-preview.md b/design-consultation/sections/proposal-and-preview.md index 090737c77..b4cb37eaa 100644 --- a/design-consultation/sections/proposal-and-preview.md +++ b/design-consultation/sections/proposal-and-preview.md @@ -304,13 +304,17 @@ echo "DESIGN_DIR: $_DESIGN_DIR" Brief: Phase 3 aesthetic/colors/type/spacing/layout plus Phase 1 product context: ```bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -Run quality check on each successful path returned by `variants`; never include failed variants: + +**Round accounting:** names are never overwritten (a taken one is bumped), so use only the printed `saved` paths. Tell the user how many of `requested` paid images were saved and name each `failures` entry. Exit 2 means nothing was saved: report `failures` and stop here (offer Path B or skip); run no `$D check` or board. + +Run quality check on each saved path, starting with the first; never include failed variants: ```bash -$D check --image "$_DESIGN_DIR/variant-A.png" --brief "" +$D check --image "" --brief "" ``` Read JSON, not exit code: `pass: false` means regenerate addressing `issues`, then recheck. `pass: true` with an unavailable/skipped warning is missing automated coverage; disclose it and inspect visually. @@ -321,10 +325,12 @@ Read each accepted PNG inline, then open the board with those paths before invit ### Comparison Board + Feedback Loop -Use the successful, quality-checked paths in this example: + +Write this round's board images (printed paths that passed checks, in order) as a JSON array to `$_DESIGN_DIR/board-images.json` with the Write tool; board letters A, B, C follow that order. Then archive any earlier Submit so it cannot approve these images, and build the board: ```bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve +[ -f "$_DESIGN_DIR/feedback.json" ] && mv "$_DESIGN_DIR/feedback.json" "$_DESIGN_DIR/feedback-$(date -u +%Y%m%dT%H%M%SZ).json" +$D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html" --serve ``` This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with `BOARD_URL` means the daemon is serving. Save its full `http://127.0.0.1:N/boards//` URL. Only legacy `--no-daemon` needs a host background task; `SERVE_STARTED: port=N` gives root URL `http://127.0.0.1:N/`. @@ -346,18 +352,18 @@ After the response, read current feedback next to the board HTML: **Board or chat:** revisions regenerate; a final choice needs summary confirmation; skip goes to Phase 6 without a mockup. Ask if no choice/detail; never infer approval from a missing file. Submit with revision notes is a revision. **Regenerate:** -1. Revise the brief, preserving unrelated constraints. Archive this round's feedback files so old Submit cannot approve new images. -2. Run `$D variants` with the new brief (no session). Re-run the quality check and visual self-gate on every new image. -3. Rebuild: `$D compare --images "" --output "$_DESIGN_DIR/design-board.html"`, without `--serve`. +1. Revise the brief, preserving unrelated constraints. Archive this round's feedback files so old Submit cannot approve new images (the board block does this on rebuild). +2. Run `$D variants` with the new brief (no session), with the same capture and round accounting. Re-run the quality check and visual self-gate on every new image (its printed path). +3. Rebuild with the board block above (it rewrites board-images.json), without `--serve`. 4. Reload at the saved URL (keep its per-board path; legacy uses root): `jq -nc --arg html "$_DESIGN_DIR/design-board.html" '{html: $html}' | curl -sS -X POST "${BOARD_URL}api/reload" -H 'Content-Type: application/json' --data-binary @-` 5. Check reload succeeded, then AskUserQuestion at the same URL until a final choice, skip or stop. Failed generation/reload uses the fallback, not another wait. **SERVER FALLBACK:** Nonzero exit or no readiness marker: show each variant inline with Read, then AskUserQuestion: "The comparison board server failed to start. Which variant? Any changes?" Route chat feedback as above. -**After receiving feedback (any path):** summarize PREFERRED, RATINGS, YOUR NOTES, DIRECTION; AskUserQuestion "Is this right?" A confirmed final choice permits Write of `$_DESIGN_DIR/approved.json` with `approved_variant`, `feedback`, `date` (UTC), `screen` (the product page depicted by the chosen mockup), and `branch` (the current `git branch --show-current` result, empty if detached). Use valid JSON, never shell interpolation. This approves the image only; Q-final gates project writes. +**After receiving feedback (any path):** summarize PREFERRED, RATINGS, YOUR NOTES, DIRECTION; AskUserQuestion "Is this right?" A confirmed final choice permits Write of `$_DESIGN_DIR/approved.json` with `approved_variant`, `approved_path` (file name of that letter's entry in this board's `board-images.json`, never the directory listing), `feedback`, `date` (UTC), `screen` (the product page depicted by the chosen mockup), and `branch` (the current `git branch --show-current` result, empty if detached). Use valid JSON, never shell interpolation. This approves the image only; Q-final gates project writes. -After final image confirmation, `$D extract` would write DESIGN.md in a Git repo: run it only in a fresh non-repository scratch directory. Bind `$D` and `APPROVED_IMAGE` to absolute paths: +After final image confirmation, `$D extract` would write DESIGN.md in a Git repo: run it only in a fresh non-repository scratch directory. Bind `$D` and `APPROVED_IMAGE` (the confirmed letter's `board-images.json` entry) to absolute paths: ```bash _EXTRACT_DIR=$(mktemp -d /tmp/gstack-design-extract-XXXXXXXX) || exit 1 @@ -373,7 +379,7 @@ _EXTRACT_DIR=$(mktemp -d /tmp/gstack-design-extract-XXXXXXXX) || exit 1 Compare extracted tokens with the approved image and verified fonts; show discrepancies at Q-final. Empty arrays, an "Unable to extract" mood or command failure → disclose fallback to Phase 3 values, never invent measured tokens. -Late visual changes return to the feedback loop: regenerate, recheck, reconfirm, then extract again. Only `generate` supplies `sessionFile` for `$D iterate --session "" --feedback "" --output "$_DESIGN_DIR/refined.png"`; variants must regenerate. +Late visual changes return to the feedback loop: regenerate, recheck, reconfirm, then extract again. Only `generate` supplies `sessionFile` for `$D iterate --session "" --feedback "" --output "$_DESIGN_DIR/refined.png"`; use its printed `outputPath`. Variants must regenerate. **Plan mode:** Carry the approved mockup paths/tokens into Phase 6's "## Proposed DESIGN.md" plan section. Its Q-final approval governs saving that content; defer the actual DESIGN.md to implementation. diff --git a/design-consultation/sections/proposal-and-preview.md.tmpl b/design-consultation/sections/proposal-and-preview.md.tmpl index f76f9fcb0..42ffefefd 100644 --- a/design-consultation/sections/proposal-and-preview.md.tmpl +++ b/design-consultation/sections/proposal-and-preview.md.tmpl @@ -111,13 +111,17 @@ echo "DESIGN_DIR: $_DESIGN_DIR" Brief: Phase 3 aesthetic/colors/type/spacing/layout plus Phase 1 product context: ```bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -Run quality check on each successful path returned by `variants`; never include failed variants: + +**Round accounting:** names are never overwritten (a taken one is bumped), so use only the printed `saved` paths. Tell the user how many of `requested` paid images were saved and name each `failures` entry. Exit 2 means nothing was saved: report `failures` and stop here (offer Path B or skip); run no `$D check` or board. + +Run quality check on each saved path, starting with the first; never include failed variants: ```bash -$D check --image "$_DESIGN_DIR/variant-A.png" --brief "" +$D check --image "" --brief "" ``` Read JSON, not exit code: `pass: false` means regenerate addressing `issues`, then recheck. `pass: true` with an unavailable/skipped warning is missing automated coverage; disclose it and inspect visually. @@ -128,7 +132,7 @@ Read each accepted PNG inline, then open the board with those paths before invit {{DESIGN_SHOTGUN_LOOP}} -After final image confirmation, `$D extract` would write DESIGN.md in a Git repo: run it only in a fresh non-repository scratch directory. Bind `$D` and `APPROVED_IMAGE` to absolute paths: +After final image confirmation, `$D extract` would write DESIGN.md in a Git repo: run it only in a fresh non-repository scratch directory. Bind `$D` and `APPROVED_IMAGE` (the confirmed letter's `board-images.json` entry) to absolute paths: ```bash _EXTRACT_DIR=$(mktemp -d /tmp/gstack-design-extract-XXXXXXXX) || exit 1 @@ -144,7 +148,7 @@ _EXTRACT_DIR=$(mktemp -d /tmp/gstack-design-extract-XXXXXXXX) || exit 1 Compare extracted tokens with the approved image and verified fonts; show discrepancies at Q-final. Empty arrays, an "Unable to extract" mood or command failure → disclose fallback to Phase 3 values, never invent measured tokens. -Late visual changes return to the feedback loop: regenerate, recheck, reconfirm, then extract again. Only `generate` supplies `sessionFile` for `$D iterate --session "" --feedback "" --output "$_DESIGN_DIR/refined.png"`; variants must regenerate. +Late visual changes return to the feedback loop: regenerate, recheck, reconfirm, then extract again. Only `generate` supplies `sessionFile` for `$D iterate --session "" --feedback "" --output "$_DESIGN_DIR/refined.png"`; use its printed `outputPath`. Variants must regenerate. **Plan mode:** Carry the approved mockup paths/tokens into Phase 6's "## Proposed DESIGN.md" plan section. Its Q-final approval governs saving that content; defer the actual DESIGN.md to implementation. diff --git a/design-html/SKILL.md b/design-html/SKILL.md index 0409f2cfa..0b9a13915 100644 --- a/design-html/SKILL.md +++ b/design-html/SKILL.md @@ -399,13 +399,15 @@ Comparison boards are local HTML files: open them with `open file://...` on macO If `DESIGN_READY`: the design binary is available for visual mockup generation. Commands: -- `$D generate --brief "..." --output /path.png` — generate a single mockup -- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants -- `$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve` — comparison board + HTTP server +- `$D generate --brief "..." --output /path.png` — generate a single mockup (prints `outputPath`) +- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants (prints `paths`) +- `$D compare --images-file /path/board-images.json --output /path/board.html --serve` — comparison board + HTTP server - `$D serve --html /path/board.html` — serve comparison board and collect feedback via HTTP - `$D check --image /path.png --brief "..."` — vision quality gate - `$D iterate --session /path/session.json --feedback "..." --output /path.png` — iterate +Image commands never overwrite (a taken name gets `-2`) and always print JSON (`requested`, `saved`, `failures`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without `set -e`: `_OUT=$($D ...); _RC=$?`. + **Path rule:** Design artifacts belong in `$GSTACK_STATE_ROOT/projects/$SLUG/designs/`. Use `bin/gstack-paths` (docs/state-root.md). Keep it even if temporary; never substitute .context/, docs/designs/ or another directory. @@ -455,8 +457,17 @@ Now route based on what was found. Check these cases in order: ### Case A: approved.json exists (design-shotgun ran) -If `APPROVED` was found, read it. Extract: approved variant PNG path, user feedback, -screen name. Also read the CEO plan if one exists (it adds strategic context). +If `APPROVED` was found, read it for user feedback and screen name, and resolve the +approved PNG (never guess it from a letter or the newest variant): + +```bash +~/.claude/skills/gstack/bin/gstack-design-approved "" +``` + +It prints the approved image's absolute path. If it prints "approved image ... is +missing; reselect from the board" instead, tell the user exactly that, offer +/design-shotgun to reselect, and do not substitute another image. Also read the CEO +plan if one exists (it adds strategic context). Read `DESIGN.md` if it exists in the repo root. These tokens take priority for system-level values (fonts, brand colors, spacing scale). diff --git a/design-html/SKILL.md.tmpl b/design-html/SKILL.md.tmpl index 6d97772c6..9fea4b7d0 100644 --- a/design-html/SKILL.md.tmpl +++ b/design-html/SKILL.md.tmpl @@ -82,8 +82,17 @@ Now route based on what was found. Check these cases in order: ### Case A: approved.json exists (design-shotgun ran) -If `APPROVED` was found, read it. Extract: approved variant PNG path, user feedback, -screen name. Also read the CEO plan if one exists (it adds strategic context). +If `APPROVED` was found, read it for user feedback and screen name, and resolve the +approved PNG (never guess it from a letter or the newest variant): + +```bash +{{BIN_DIR}}/gstack-design-approved "" +``` + +It prints the approved image's absolute path. If it prints "approved image ... is +missing; reselect from the board" instead, tell the user exactly that, offer +/design-shotgun to reselect, and do not substitute another image. Also read the CEO +plan if one exists (it adds strategic context). Read `DESIGN.md` if it exists in the repo root. These tokens take priority for system-level values (fonts, brand colors, spacing scale). diff --git a/design-review/SKILL.md b/design-review/SKILL.md index 06072d2b8..c2aa3b7ef 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -736,13 +736,15 @@ Comparison boards are local HTML files: open them with `open file://...` on macO If `DESIGN_READY`: the design binary is available for visual mockup generation. Commands: -- `$D generate --brief "..." --output /path.png` — generate a single mockup -- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants -- `$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve` — comparison board + HTTP server +- `$D generate --brief "..." --output /path.png` — generate a single mockup (prints `outputPath`) +- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants (prints `paths`) +- `$D compare --images-file /path/board-images.json --output /path/board.html --serve` — comparison board + HTTP server - `$D serve --html /path/board.html` — serve comparison board and collect feedback via HTTP - `$D check --image /path.png --brief "..."` — vision quality gate - `$D iterate --session /path/session.json --feedback "..." --output /path.png` — iterate +Image commands never overwrite (a taken name gets `-2`) and always print JSON (`requested`, `saved`, `failures`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without `set -e`: `_OUT=$($D ...); _RC=$?`. + **Path rule:** Design artifacts belong in `$GSTACK_STATE_ROOT/projects/$SLUG/designs/`. Use `bin/gstack-paths` (docs/state-root.md). Keep it even if temporary; never substitute .context/, docs/designs/ or another directory. @@ -1736,9 +1738,12 @@ For each fixable finding, in impact order: If the gstack designer is available and the finding involves visual layout, hierarchy, or spacing (not just a CSS value fix like wrong color or font-size), generate a target mockup showing what the corrected version should look like: ```bash -$D generate --brief "" --output "$REPORT_DIR/screenshots/finding-NNN-target.png" +_OUT=$($D generate --brief "" --output "$REPORT_DIR/screenshots/finding-NNN-target.png"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` +Record the printed `outputPath` for this finding (images are never overwritten, so it may be bumped); later steps use that path. Exit 2: no mockup was saved; skip it for this finding. + Show the user: "Here's the current state (screenshot) and here's what it should look like (mockup). Now I'll fix the source to match." This step is optional — skip for trivial CSS fixes (wrong hex color, missing padding value). Use it for findings where the intended design isn't obvious from the description alone. @@ -1828,7 +1833,7 @@ DESIGN-FIX RISK: After all fixes are applied: 1. Re-run the design audit on all affected pages -2. If target mockups were generated during the fix loop AND `DESIGN_READY`: run `$D verify --mockup "$REPORT_DIR/screenshots/finding-NNN-target.png" --screenshot "$REPORT_DIR/screenshots/finding-NNN-after.png"` to compare the fix result against the target. Include pass/fail in the report. +2. If target mockups were generated during the fix loop AND `DESIGN_READY`: run `$D verify --mockup "" --screenshot "$REPORT_DIR/screenshots/finding-NNN-after.png"` to compare the fix result against the target. Include pass/fail in the report. 3. Compute final design score and AI slop score 4. Recompute the detector count the same way Phase 0 scanned: DOM mode re-dumps the affected pages after reload into `$REPORT_DIR/dom/$RUN_ID/` and rescans; source mode rescans the files you touched. Then delete `$REPORT_DIR/dom/$RUN_ID/` unless the user passed `--keep-dom`. 5. **If final scores are WORSE than baseline:** WARN prominently — something regressed diff --git a/design-review/SKILL.md.tmpl b/design-review/SKILL.md.tmpl index 6d7d70f0e..5b01fd0f0 100644 --- a/design-review/SKILL.md.tmpl +++ b/design-review/SKILL.md.tmpl @@ -179,9 +179,12 @@ For each fixable finding, in impact order: If the gstack designer is available and the finding involves visual layout, hierarchy, or spacing (not just a CSS value fix like wrong color or font-size), generate a target mockup showing what the corrected version should look like: ```bash -$D generate --brief "" --output "$REPORT_DIR/screenshots/finding-NNN-target.png" +_OUT=$($D generate --brief "" --output "$REPORT_DIR/screenshots/finding-NNN-target.png"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` +Record the printed `outputPath` for this finding (images are never overwritten, so it may be bumped); later steps use that path. Exit 2: no mockup was saved; skip it for this finding. + Show the user: "Here's the current state (screenshot) and here's what it should look like (mockup). Now I'll fix the source to match." This step is optional — skip for trivial CSS fixes (wrong hex color, missing padding value). Use it for findings where the intended design isn't obvious from the description alone. @@ -271,7 +274,7 @@ DESIGN-FIX RISK: After all fixes are applied: 1. Re-run the design audit on all affected pages -2. If target mockups were generated during the fix loop AND `DESIGN_READY`: run `$D verify --mockup "$REPORT_DIR/screenshots/finding-NNN-target.png" --screenshot "$REPORT_DIR/screenshots/finding-NNN-after.png"` to compare the fix result against the target. Include pass/fail in the report. +2. If target mockups were generated during the fix loop AND `DESIGN_READY`: run `$D verify --mockup "" --screenshot "$REPORT_DIR/screenshots/finding-NNN-after.png"` to compare the fix result against the target. Include pass/fail in the report. 3. Compute final design score and AI slop score 4. Recompute the detector count the same way Phase 0 scanned: DOM mode re-dumps the affected pages after reload into `$REPORT_DIR/dom/$RUN_ID/` and rescans; source mode rescans the files you touched. Then delete `$REPORT_DIR/dom/$RUN_ID/` unless the user passed `--keep-dom`. 5. **If final scores are WORSE than baseline:** WARN prominently — something regressed diff --git a/design-shotgun/SKILL.md b/design-shotgun/SKILL.md index d343c20ca..e3c05fd39 100644 --- a/design-shotgun/SKILL.md +++ b/design-shotgun/SKILL.md @@ -410,13 +410,15 @@ Comparison boards are local HTML files: open them with `open file://...` on macO If `DESIGN_READY`: the design binary is available for visual mockup generation. Commands: -- `$D generate --brief "..." --output /path.png` — generate a single mockup -- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants -- `$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve` — comparison board + HTTP server +- `$D generate --brief "..." --output /path.png` — generate a single mockup (prints `outputPath`) +- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants (prints `paths`) +- `$D compare --images-file /path/board-images.json --output /path/board.html --serve` — comparison board + HTTP server - `$D serve --html /path/board.html` — serve comparison board and collect feedback via HTTP - `$D check --image /path.png --brief "..."` — vision quality gate - `$D iterate --session /path/session.json --feedback "..." --output /path.png` — iterate +Image commands never overwrite (a taken name gets `-2`) and always print JSON (`requested`, `saved`, `failures`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without `set -e`: `_OUT=$($D ...); _RC=$?`. + **Path rule:** Design artifacts belong in `$GSTACK_STATE_ROOT/projects/$SLUG/designs/`. Use `bin/gstack-paths` (docs/state-root.md). Keep it even if temporary; never substitute .context/, docs/designs/ or another directory. @@ -552,7 +554,7 @@ fonts [top-3], colors [top-3], layouts [top-3], aesthetics [top-3]. Bias generation toward these unless the user explicitly requests a different direction. Also avoid their strong rejections: [top-3 rejected per dimension]." -**Legacy fallback:** Glob `$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json` (resolve the root with gstack-paths); Read the five newest. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. +**Legacy fallback:** Glob `$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json` (resolve the root with gstack-paths); Read the five newest. To view an approved image, resolve it with `~/.claude/skills/gstack/bin/gstack-design-approved `. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. **Conflict handling:** If the current user request contradicts a strong persistent signal (e.g., "make it playful" when taste profile strongly prefers minimal), flag @@ -576,14 +578,16 @@ _TASTE=$(find "$GSTACK_STATE_ROOT/projects/$SLUG/designs/" -name "approved.json" ``` If prior sessions exist, read each `approved.json` and extract patterns from the -approved variants. Merge these into the taste-profile.json-derived signal — if the +approved variants; resolve each approved image with `~/.claude/skills/gstack/bin/gstack-design-approved ` +(an error means that image is gone: skip it, never substitute another). Merge these into the taste-profile.json-derived signal — if the profile already says "user prefers Geist font" (from aggregated history), the approved.json files add the specific recent approval context. Limit to last 10 sessions. Try/catch JSON parse on each (skip corrupted files). **Updating taste profile after a design-shotgun session:** When the user picks a -variant, call `~/.claude/skills/gstack/bin/gstack-taste-update approved `. When they +variant, call `~/.claude/skills/gstack/bin/gstack-taste-update approved ` (the +`APPROVED_IMAGE` printed when approved.json is saved). When they explicitly reject a variant, call `~/.claude/skills/gstack/bin/gstack-taste-update rejected `. The CLI handles schema migration from approved.json, decay, and conflict flagging. @@ -674,58 +678,55 @@ variable that agents do NOT inherit. Substitute the resolved absolute path (from **Agent prompt template** (one per variant, substitute all `{...}` values): ``` -Generate a design variant and save it. +Generate a design variant and publish it without overwriting any image. Design binary: {absolute path to $D binary} +Claim helper: ~/.claude/skills/gstack/bin/gstack-design-claim Brief: {the full variant-specific brief for this direction} -Output: /tmp/variant-{letter}.png -Final location: {_DESIGN_DIR absolute path}/variant-{letter}.png +Design dir: {_DESIGN_DIR absolute path} -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} +Steps (no set -e; never cp or mv an image): +1. Stage in a fresh per-run directory: STAGE=$(mktemp -d /tmp/gstack-variant-XXXXXXXX) +2. Generate: OUT=$({$D path} generate --brief "{brief}" --output "$STAGE/variant.png"); RC=$?; echo "$OUT"; echo "EXIT: $RC" + Read outputPath, saved and failures from the JSON. Exit 2 = nothing saved: on a rate + limit (429 or "rate limit") wait 5 seconds and retry, up to 3 retries. +3. Publish: FINAL=$(~/.claude/skills/gstack/bin/gstack-design-claim "" "{Design dir}/variant-{letter}.png") + It never overwrites and prints the final (possibly bumped) path. Use FINAL from here on. + If it fails, report the error; the staged image stays at . +4. Quality check: {$D path} check --image "" --brief "{brief}" + If it fails, regenerate once (steps 2-3; the first image is kept). +5. Verify: ls -lh "" +6. Report exactly one of: + VARIANT_{letter}_DONE: {every published path, comma-separated, last one is the pick} {file size} + VARIANT_{letter}_FAILED: {error description, plus any published paths} VARIANT_{letter}_RATE_LIMITED: exhausted retries ``` -For the evolve path, replace step 1 with: +For the evolve path, replace the step 2 command with: ``` -{$D path} evolve --screenshot {_DESIGN_DIR}/current.png --brief "{brief}" --output /tmp/variant-{letter}.png +OUT=$({$D path} evolve --screenshot {_DESIGN_DIR}/current.png --brief "{brief}" --output "$STAGE/variant.png"); RC=$? ``` -**Generate to `/tmp/`, then `cp`:** in sandboxed sessions `$D generate --output` under -`~/.gstack/` can abort ("The operation was aborted"), while `/tmp/` works. +**Why staging:** in sandboxed sessions `$D generate --output` under `~/.gstack/` can +abort ("The operation was aborted"), while `/tmp/` works. A fresh `mktemp -d` per run +means no other session's file can be published, and the claim helper never clobbers. ### Step 3d: Results After all agents complete: -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. -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..." -5. Proceed to Step 4 (comparison board). - -**Dynamic image list for comparison board:** When proceeding to Step 4, construct the -image list from whatever variant files actually exist, not a hardcoded A/B/C list: - -```bash -setopt +o nomatch 2>/dev/null || true # zsh compat -_IMAGES=$(ls "$_DESIGN_DIR"/variant-*.png 2>/dev/null | tr '\n' ',' | sed 's/,$//') -``` - -Use `$_IMAGES` in the `$D compare --images` command. + +1. Round accounting first: this round's images are exactly the paths the DONE and FAILED + lines report (never a directory listing; older rounds stay on disk). Tell the user: + "{saved} of {N} paid images saved in ~{actual time}", listing every published path. +2. For any failures: report explicitly with the error. Do NOT silently skip. +3. If zero variants succeeded: fall back to sequential generation, running the agent + steps yourself one variant at a time and showing each as it lands. Tell the user: + "Parallel generation failed (likely rate limiting). Falling back to sequential..." + If that also saves nothing, report the failures and stop: no board. +4. Read each saved image inline (Read tool, the reported paths) so the user sees all variants at once. +5. Proceed to Step 4 with each variant's pick (the last reported path), in letter order, + as this round's board images. ## Step 4: Comparison Board + Feedback Loop @@ -733,8 +734,12 @@ Use `$_IMAGES` in the `$D compare --images` command. Create the comparison board and serve it over HTTP: + +Write this round's board images (printed paths that passed checks, in order) as a JSON array to `$_DESIGN_DIR/board-images.json` with the Write tool; board letters A, B, C follow that order. Then archive any earlier Submit so it cannot approve these images, and build the board: + ```bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve +[ -f "$_DESIGN_DIR/feedback.json" ] && mv "$_DESIGN_DIR/feedback.json" "$_DESIGN_DIR/feedback-$(date -u +%Y%m%dT%H%M%SZ).json" +$D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html" --serve ``` Creates HTML and opens the board. **Run it in the background** (host task, or `&` redirecting stdout/stderr to private files in `$_DESIGN_DIR`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. @@ -793,8 +798,8 @@ the approved variant. 1. Read `regenerateAction` from the JSON (`"different"`, `"match"`, `"more_like_B"`, `"remix"`, or custom text) 2. If `regenerateAction` is `"remix"`, read `remixSpec` (e.g. `{"layout":"A","colors":"B"}`) -3. Generate new variants with `$D iterate` or `$D variants` using updated brief -4. Create new board: `$D compare --images "..." --output "$_DESIGN_DIR/design-board.html"` +3. Generate new variants with `$D iterate` or `$D variants` using updated brief (capture the JSON and do round accounting as for the first round) +4. Rebuild with the board block above (it archives feedback.json and rewrites board-images.json), without `--serve` 5. Reload the board in the user's browser (same tab) — the URL is per-board under daemon mode, so use `` (from the `BOARD_URL:` stderr line) as the base: @@ -828,9 +833,16 @@ Is this right?" Use AskUserQuestion to verify before proceeding. -**Save the approved choice:** +**Save the approved choice.** Map the confirmed letter through this board's `board-images.json` (never the directory listing) and save it: + ```bash -echo '{"approved_variant":"","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" +_IMG=$(jq -r --arg v "" '.[($v | explode[0]) - 65] // empty' "$_DESIGN_DIR/board-images.json") +if [ -n "$_IMG" ]; then + echo '{"approved_variant":"","approved_path":"'"$(basename "$_IMG")"'","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" + echo "APPROVED_IMAGE: $_IMG" +else + echo "NO_BOARD_IMAGE: is not on this board; reselect from the board" +fi ``` ## Step 5: Feedback Confirmation @@ -854,7 +866,8 @@ Use AskUserQuestion to confirm before saving. Write `approved.json` to `$_DESIGN_DIR/` (handled by the loop above). If invoked from another skill: return the structured feedback for that skill to consume. -The calling skill reads `approved.json` and the approved variant PNG. +The calling skill reads `approved.json` and resolves the approved image with +`~/.claude/skills/gstack/bin/gstack-design-approved "$_DESIGN_DIR/approved.json"`. If standalone, offer next steps via AskUserQuestion: diff --git a/design-shotgun/SKILL.md.tmpl b/design-shotgun/SKILL.md.tmpl index ac3d1ecdd..1845aa8c7 100644 --- a/design-shotgun/SKILL.md.tmpl +++ b/design-shotgun/SKILL.md.tmpl @@ -173,14 +173,16 @@ _TASTE=$(find "$GSTACK_STATE_ROOT/projects/$SLUG/designs/" -name "approved.json" ``` If prior sessions exist, read each `approved.json` and extract patterns from the -approved variants. Merge these into the taste-profile.json-derived signal — if the +approved variants; resolve each approved image with `{{BIN_DIR}}/gstack-design-approved ` +(an error means that image is gone: skip it, never substitute another). Merge these into the taste-profile.json-derived signal — if the profile already says "user prefers Geist font" (from aggregated history), the approved.json files add the specific recent approval context. Limit to last 10 sessions. Try/catch JSON parse on each (skip corrupted files). **Updating taste profile after a design-shotgun session:** When the user picks a -variant, call `{{BIN_DIR}}/gstack-taste-update approved `. When they +variant, call `{{BIN_DIR}}/gstack-taste-update approved ` (the +`APPROVED_IMAGE` printed when approved.json is saved). When they explicitly reject a variant, call `{{BIN_DIR}}/gstack-taste-update rejected `. The CLI handles schema migration from approved.json, decay, and conflict flagging. @@ -271,58 +273,55 @@ variable that agents do NOT inherit. Substitute the resolved absolute path (from **Agent prompt template** (one per variant, substitute all `{...}` values): ``` -Generate a design variant and save it. +Generate a design variant and publish it without overwriting any image. Design binary: {absolute path to $D binary} +Claim helper: {{BIN_DIR}}/gstack-design-claim Brief: {the full variant-specific brief for this direction} -Output: /tmp/variant-{letter}.png -Final location: {_DESIGN_DIR absolute path}/variant-{letter}.png +Design dir: {_DESIGN_DIR absolute path} -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} +Steps (no set -e; never cp or mv an image): +1. Stage in a fresh per-run directory: STAGE=$(mktemp -d /tmp/gstack-variant-XXXXXXXX) +2. Generate: OUT=$({$D path} generate --brief "{brief}" --output "$STAGE/variant.png"); RC=$?; echo "$OUT"; echo "EXIT: $RC" + Read outputPath, saved and failures from the JSON. Exit 2 = nothing saved: on a rate + limit (429 or "rate limit") wait 5 seconds and retry, up to 3 retries. +3. Publish: FINAL=$({{BIN_DIR}}/gstack-design-claim "" "{Design dir}/variant-{letter}.png") + It never overwrites and prints the final (possibly bumped) path. Use FINAL from here on. + If it fails, report the error; the staged image stays at . +4. Quality check: {$D path} check --image "" --brief "{brief}" + If it fails, regenerate once (steps 2-3; the first image is kept). +5. Verify: ls -lh "" +6. Report exactly one of: + VARIANT_{letter}_DONE: {every published path, comma-separated, last one is the pick} {file size} + VARIANT_{letter}_FAILED: {error description, plus any published paths} VARIANT_{letter}_RATE_LIMITED: exhausted retries ``` -For the evolve path, replace step 1 with: +For the evolve path, replace the step 2 command with: ``` -{$D path} evolve --screenshot {_DESIGN_DIR}/current.png --brief "{brief}" --output /tmp/variant-{letter}.png +OUT=$({$D path} evolve --screenshot {_DESIGN_DIR}/current.png --brief "{brief}" --output "$STAGE/variant.png"); RC=$? ``` -**Generate to `/tmp/`, then `cp`:** in sandboxed sessions `$D generate --output` under -`~/.gstack/` can abort ("The operation was aborted"), while `/tmp/` works. +**Why staging:** in sandboxed sessions `$D generate --output` under `~/.gstack/` can +abort ("The operation was aborted"), while `/tmp/` works. A fresh `mktemp -d` per run +means no other session's file can be published, and the claim helper never clobbers. ### Step 3d: Results After all agents complete: -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. -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..." -5. Proceed to Step 4 (comparison board). - -**Dynamic image list for comparison board:** When proceeding to Step 4, construct the -image list from whatever variant files actually exist, not a hardcoded A/B/C list: - -```bash -setopt +o nomatch 2>/dev/null || true # zsh compat -_IMAGES=$(ls "$_DESIGN_DIR"/variant-*.png 2>/dev/null | tr '\n' ',' | sed 's/,$//') -``` - -Use `$_IMAGES` in the `$D compare --images` command. + +1. Round accounting first: this round's images are exactly the paths the DONE and FAILED + lines report (never a directory listing; older rounds stay on disk). Tell the user: + "{saved} of {N} paid images saved in ~{actual time}", listing every published path. +2. For any failures: report explicitly with the error. Do NOT silently skip. +3. If zero variants succeeded: fall back to sequential generation, running the agent + steps yourself one variant at a time and showing each as it lands. Tell the user: + "Parallel generation failed (likely rate limiting). Falling back to sequential..." + If that also saves nothing, report the failures and stop: no board. +4. Read each saved image inline (Read tool, the reported paths) so the user sees all variants at once. +5. Proceed to Step 4 with each variant's pick (the last reported path), in letter order, + as this round's board images. ## Step 4: Comparison Board + Feedback Loop @@ -349,7 +348,8 @@ Use AskUserQuestion to confirm before saving. Write `approved.json` to `$_DESIGN_DIR/` (handled by the loop above). If invoked from another skill: return the structured feedback for that skill to consume. -The calling skill reads `approved.json` and the approved variant PNG. +The calling skill reads `approved.json` and resolves the approved image with +`{{BIN_DIR}}/gstack-design-approved "$_DESIGN_DIR/approved.json"`. If standalone, offer next steps via AskUserQuestion: diff --git a/design/src/approval.ts b/design/src/approval.ts new file mode 100644 index 000000000..400015439 --- /dev/null +++ b/design/src/approval.ts @@ -0,0 +1,56 @@ +/** + * Resolve the image an approval record names. `approved_path` is stored + * relative to the directory holding approved.json, so a copied or relocated + * designs directory still resolves. Records without `approved_path` (written + * before never-overwrite) fall back to `variant-.png`. A + * present `approved_path` whose file is missing is an error: another image is + * never substituted. + */ + +import fs from "fs"; +import path from "path"; + +export type ApprovedImage = { ok: true; path: string; legacy: boolean } | { ok: false; error: string }; + +export interface ApprovalRecord { + approved_variant?: unknown; + approved_path?: unknown; +} + +function missing(imagePath: string): ApprovedImage { + return { ok: false, error: `approved image ${imagePath} is missing; reselect from the board` }; +} + +function readable(imagePath: string): boolean { + try { + fs.accessSync(imagePath, fs.constants.R_OK); + return fs.statSync(imagePath).isFile(); + } catch { + return false; + } +} + +/** The image path a record names, without checking that it exists. */ +export function approvedImagePath(record: ApprovalRecord, recordDir: string): { path: string; legacy: boolean } | null { + if (typeof record.approved_path === "string" && record.approved_path !== "") { + return { path: path.resolve(recordDir, record.approved_path), legacy: false }; + } + if (typeof record.approved_variant === "string" && record.approved_variant !== "") { + return { path: path.join(recordDir, `variant-${record.approved_variant}.png`), legacy: true }; + } + return null; +} + +export function resolveApprovedImage(approvedJsonPath: string): ApprovedImage { + let record: ApprovalRecord; + try { + record = JSON.parse(fs.readFileSync(approvedJsonPath, "utf-8")); + } catch (err: any) { + return { ok: false, error: `cannot read ${approvedJsonPath}: ${err.code || err.message}` }; + } + const named = approvedImagePath(record ?? {}, path.dirname(path.resolve(approvedJsonPath))); + if (!named) { + return { ok: false, error: `${approvedJsonPath} names no approved image; reselect from the board` }; + } + return readable(named.path) ? { ok: true, ...named } : missing(named.path); +} diff --git a/design/src/auth.ts b/design/src/auth.ts index 1202500a9..fc25cbcf4 100644 --- a/design/src/auth.ts +++ b/design/src/auth.ts @@ -119,19 +119,20 @@ export function saveApiKey(key: string): void { } /** - * Get API key or exit with setup instructions. + * Get API key or throw with setup instructions. */ export function requireApiKey(): string { const resolution = resolveApiKeyInfo(); if (!resolution) { - console.error("No OpenAI API key found."); - console.error(""); - console.error("Run: $D setup"); - console.error(" or save to ~/.gstack/openai.json: { \"api_key\": \"sk-...\" }"); - console.error(" or set OPENAI_API_KEY environment variable"); - console.error(""); - console.error("Get a key at: https://platform.openai.com/api-keys"); - process.exit(1); + throw new Error([ + "No OpenAI API key found.", + "", + "Run: $D setup", + " or save to ~/.gstack/openai.json: { \"api_key\": \"sk-...\" }", + " or set OPENAI_API_KEY environment variable", + "", + "Get a key at: https://platform.openai.com/api-keys", + ].join("\n")); } console.error(`Using OpenAI key from ${describeApiKeySource(resolution)}.`); if (resolution.warning) console.error(resolution.warning); diff --git a/design/src/cli.ts b/design/src/cli.ts index b266c6f38..2278b9a65 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -12,7 +12,7 @@ * 4. Print result JSON to stdout */ -import { COMMANDS } from "./commands"; +import { COMMANDS, OUTPUT_CONTRACT } from "./commands"; import { generate } from "./generate"; import { checkCommand } from "./check"; import { compare } from "./compare"; @@ -26,6 +26,7 @@ import { generateDesignToCodePrompt } from "./design-to-code"; import { serve } from "./serve"; import { gallery } from "./gallery"; import { normalizeIntFlag } from "./flag-utils"; +import { resolveImagePaths } from "./image-args"; import { daemonStatus as daemonStatusClient, ensureDaemon, @@ -33,6 +34,9 @@ import { shutdownDaemon, } from "./daemon-client"; import { spawn as nodeSpawn } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; function parseArgs(argv: string[]): { command: string; @@ -75,6 +79,7 @@ function printUsage(): void { console.log(` ${name.padEnd(12)} ${info.description}`); console.log(` ${"".padEnd(12)} ${info.usage}`); } + console.log(`\n${OUTPUT_CONTRACT}`); console.log("\nAuth: ~/.gstack/openai.json, then OPENAI_API_KEY env var"); console.log("If OPENAI_API_KEY matches a current-directory .env file, the source is reported before billing."); console.log("Setup: $D setup"); @@ -107,19 +112,18 @@ async function runSetup(): Promise { // Smoke test console.log("\nRunning smoke test (generating a simple image)..."); - try { - await generate({ - brief: "A simple blue square centered on a white background. Minimal, geometric, clean.", - output: "/tmp/gstack-design-smoke-test.png", - size: "1024x1024", - quality: "low", - }); - console.log("\nSmoke test PASSED. Design generation is working."); - } catch (err: any) { - console.error(`\nSmoke test FAILED: ${err.message}`); + const smoke = await generate({ + brief: "A simple blue square centered on a white background. Minimal, geometric, clean.", + output: path.join(fs.mkdtempSync(path.join(os.tmpdir(), "gstack-design-smoke-")), "smoke-test.png"), + size: "1024x1024", + quality: "low", + }); + if (smoke.exitCode !== 0) { + console.error(`\nSmoke test FAILED: ${smoke.failures.map(f => f.reason).join("; ")}`); console.error("Check your API key and organization verification status."); process.exit(1); } + console.log("\nSmoke test PASSED. Design generation is working."); } async function main(): Promise { @@ -133,7 +137,7 @@ async function main(): Promise { switch (command) { case "generate": - await generate({ + process.exitCode = (await generate({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, output: (flags.output as string) || "/tmp/gstack-mockup.png", @@ -141,7 +145,7 @@ async function main(): Promise { retry: normalizeIntFlag(flags.retry, { name: "retry", def: 0, min: 0 }), size: flags.size as string, quality: flags.quality as string, - }); + })).exitCode; break; case "check": @@ -149,9 +153,7 @@ async function main(): Promise { break; case "compare": { - // Parse --images as glob or multiple files - const imagesArg = flags.images as string; - const images = await resolveImagePaths(imagesArg); + const images = await resolveImagePaths(flags.images as string, flags["images-file"] as string | undefined); const outputPath = (flags.output as string) || "/tmp/gstack-design-board.html"; compare({ images, output: outputPath }); // If --serve flag is set, publish the board. @@ -195,7 +197,7 @@ async function main(): Promise { break; case "variants": - await variants({ + process.exitCode = await variants({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, // #2032: pass the RAW flag through — variants() normalizes at its @@ -209,7 +211,7 @@ async function main(): Promise { break; case "iterate": - await iterate({ + process.exitCode = await iterate({ session: flags.session as string, feedback: flags.feedback as string, output: (flags.output as string) || "/tmp/gstack-iterate.png", @@ -261,7 +263,7 @@ async function main(): Promise { } case "evolve": - await evolve({ + process.exitCode = await evolve({ screenshot: flags.screenshot as string, brief: flags.brief as string, output: (flags.output as string) || "/tmp/gstack-evolved.png", @@ -381,31 +383,6 @@ function openBrowser(url: string): void { } } -/** - * Resolve image paths from a glob pattern or comma-separated list. - */ -async function resolveImagePaths(input: string): Promise { - if (!input) { - console.error("--images is required. Provide glob pattern or comma-separated paths."); - process.exit(1); - } - - // Check if it's a glob pattern - if (input.includes("*")) { - const glob = new Bun.Glob(input); - const paths: string[] = []; - for await (const match of glob.scan({ absolute: true })) { - if (match.endsWith(".png") || match.endsWith(".jpg") || match.endsWith(".jpeg")) { - paths.push(match); - } - } - return paths.sort(); - } - - // Comma-separated or single path - return input.split(",").map(p => p.trim()); -} - // Self-execution shortcut: when invoked with --daemon-mode, this same // binary runs as the persistent design daemon instead of the CLI. Keeps // the production install to a single executable; daemon-client.ts spawns diff --git a/design/src/commands.ts b/design/src/commands.ts index 17d5811a7..76ff9ded7 100644 --- a/design/src/commands.ts +++ b/design/src/commands.ts @@ -16,17 +16,17 @@ export const COMMANDS = new Map([ ["generate", { description: "Generate a UI mockup from a design brief", - usage: "generate --brief \"...\" --output /path.png", + usage: "generate --brief \"...\" --output /path.png [--check --retry N] (never overwrites: bumps to /path-2.png; each --retry attempt is its own saved image; prints outputPath, attempts + run fields)", flags: ["--brief", "--brief-file", "--output", "--check", "--retry", "--size", "--quality"], }], ["variants", { description: "Generate N design variants from a brief", - usage: "variants --brief \"...\" --count 3 --output-dir /path/", + usage: "variants --brief \"...\" --count 3 --output-dir /path/ [--viewports desktop,tablet,mobile] (never overwrites: variant-A.png bumps to variant-A-2.png; prints paths, errors + run fields)", flags: ["--brief", "--brief-file", "--count", "--output-dir", "--size", "--quality", "--viewports"], }], ["iterate", { description: "Iterate on an existing mockup with feedback", - usage: "iterate --session /path/session.json --feedback \"...\" --output /path.png", + usage: "iterate --session /path/session.json --feedback \"...\" --output /path.png (never overwrites; prints outputPath + run fields)", flags: ["--session", "--feedback", "--output"], }], ["check", { @@ -36,8 +36,8 @@ export const COMMANDS = new Map x-2.png, x-2.png -> x-2-2.png), stderr prints +"note: exists; saved to (existing file kept)", and the JSON +reports the actual path. Read paths from the JSON, never from a fixed file name. + +Run fields printed by generate, iterate, evolve and variants (on every exit): + requested images requested saved every saved path, in order + selected outputPath or paths failures [{file, reason}] not saved + recovered [{path, reason}] private recovery copies in the temp dir + + command extra fields readers + generate outputPath sessionFile responseId checkResult design-review, design-shotgun, + attempts[{path, check}] (one per --retry try) plan-design-review + iterate outputPath sessionFile responseId iteration plan-design-review, design-consultation + evolve outputPath sourceScreenshot brief attempts design-shotgun + variants outputDir count succeeded failed paths errors office-hours, plan-design-review, + viewports (--viewports only) design-consultation + compare board HTML; --serve prints {id, url, sourceDir} all board flows (--images-file) + +Exit codes: 0 a selected result exists; 2 nothing was saved; 3 stopped after +saving at least one image but before its selected result (interrupted retry); +1 invalid flags rejected before any API call (no JSON).`; diff --git a/design/src/evolve.ts b/design/src/evolve.ts index a656054b0..69b486849 100644 --- a/design/src/evolve.ts +++ b/design/src/evolve.ts @@ -6,10 +6,10 @@ */ import fs from "fs"; -import path from "path"; import { requireApiKey } from "./auth"; import { receiptedFetch } from "./receipted-fetch"; import { imageRequestBody, modelRejectionHint, visionRequestBody } from "./models"; +import { emitResult, exitCodeFor, newAccounting, persistImage, recordOutcome, type ExitCode } from "./persist"; export interface EvolveOptions { screenshot: string; // Path to current site screenshot @@ -22,37 +22,68 @@ export interface EvolveOptions { * Sends the screenshot as context to the image model with image generation, * asking it to produce a new version incorporating the brief's changes. */ -export async function evolve(options: EvolveOptions): Promise { - const apiKey = requireApiKey(); - const screenshotData = fs.readFileSync(options.screenshot).toString("base64"); +export async function evolve(options: EvolveOptions): Promise { + const acct = newAccounting(1); + let outputPath: string | null = null; - console.error(`Evolving ${options.screenshot} with: "${options.brief}"`); - const startTime = Date.now(); + try { + const apiKey = requireApiKey(); + const screenshotData = fs.readFileSync(options.screenshot).toString("base64"); - // Use the Responses API with both a text prompt referencing the screenshot - // and the image_generation tool to produce the evolved version. - // Since we can't send reference images directly to image_generation, - // we describe the current state in detail first via vision, then generate. + console.error(`Evolving ${options.screenshot} with: "${options.brief}"`); + const startTime = Date.now(); - // Step 1: Analyze current screenshot - const analysis = await analyzeScreenshot(apiKey, screenshotData); - console.error(` Analyzed current design: ${analysis.slice(0, 100)}...`); + // Use the Responses API with both a text prompt referencing the screenshot + // and the image_generation tool to produce the evolved version. + // Since we can't send reference images directly to image_generation, + // we describe the current state in detail first via vision, then generate. - // 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"); + // Step 1: Analyze current screenshot + const analysis = await analyzeScreenshot(apiKey, screenshotData); + 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 imageData = await requestEvolvedImage(apiKey, evolvedPrompt); + const outcome = persistImage(imageData, options.output); + outputPath = recordOutcome(acct, outcome); + if (outcome.ok) { + const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); + console.error(`Generated (${elapsed}s, ${(outcome.bytes / 1024).toFixed(0)}KB) → ${outcome.path}`); + } + } catch (err: any) { + const reason = err?.message || String(err); + console.error(reason); + acct.failures.push({ file: options.output, reason }); + } + + return emitResult({ + outputPath, + sourceScreenshot: options.screenshot, + brief: options.brief, + attempts: acct.saved.map(p => ({ path: p })), + requested: acct.requested, + saved: acct.saved, + selected: outputPath, + failures: acct.failures, + recovered: acct.recovered, + }, exitCodeFor(outputPath !== null, acct.saved.length)); +} + +async function requestEvolvedImage(apiKey: string, evolvedPrompt: string): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 240_000); @@ -85,19 +116,7 @@ export async function evolve(options: EvolveOptions): Promise { if (!imageItem?.result) { throw new Error("No image data in response"); } - - fs.mkdirSync(path.dirname(options.output), { recursive: true }); - const imageBuffer = Buffer.from(imageItem.result, "base64"); - fs.writeFileSync(options.output, imageBuffer); - - const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - console.error(`Generated (${elapsed}s, ${(imageBuffer.length / 1024).toFixed(0)}KB) → ${options.output}`); - - console.log(JSON.stringify({ - outputPath: options.output, - sourceScreenshot: options.screenshot, - brief: options.brief, - }, null, 2)); + return imageItem.result; } finally { clearTimeout(timeout); } diff --git a/design/src/gallery.ts b/design/src/gallery.ts index 95675559d..5ab08a486 100644 --- a/design/src/gallery.ts +++ b/design/src/gallery.ts @@ -6,6 +6,7 @@ import fs from "fs"; import path from "path"; +import { approvedImagePath } from "./approval"; export interface GalleryOptions { designsDir: string; // ~/.gstack/projects/$SLUG/designs/ @@ -20,6 +21,32 @@ interface SessionData { variants: string[]; // paths to variant PNGs } +const VARIANT_IMAGE = [/^variant-[A-Z](-\d+)?\.png$/i, /^variant-\d+(-\d+)?\.png$/i]; + +/** + * Variant images in a session directory as chronological history, not rounds: + * names not on the current board sort by mtime then name, followed by the + * board's images in board-images.json order. Never-overwrite means one + * letter can appear in several rounds (variant-A.png, variant-A-2.png). + */ +export function listVariantImages(sessionDir: string): string[] { + const found = fs.readdirSync(sessionDir) + .filter(f => VARIANT_IMAGE.some(re => re.test(f))) + .map(f => path.join(sessionDir, f)); + + let board: string[] = []; + try { + const parsed = JSON.parse(fs.readFileSync(path.join(sessionDir, "board-images.json"), "utf-8")); + if (Array.isArray(parsed)) board = parsed.filter((p): p is string => typeof p === "string").map(p => path.resolve(sessionDir, p)); + } catch {} + + const mtime = new Map(found.map(f => [f, fs.statSync(f).mtimeMs])); + const history = found + .filter(f => !board.includes(f)) + .sort((a, b) => (mtime.get(a)! - mtime.get(b)!) || a.localeCompare(b)); + return [...history, ...board.filter(f => found.includes(f))]; +} + export function generateGalleryHtml(designsDir: string): string { const sessions: SessionData[] = []; @@ -44,16 +71,9 @@ export function generateGalleryHtml(designsDir: string): string { } } - // Find variant PNGs - const variants: string[] = []; + let variants: string[] = []; try { - const files = fs.readdirSync(sessionDir); - for (const f of files) { - if (f.match(/variant-[A-Z]\.png$/i) || f.match(/variant-\d+\.png$/i)) { - variants.push(path.join(sessionDir, f)); - } - } - variants.sort(); + variants = listVariantImages(sessionDir); } catch { // Can't read directory, skip } @@ -81,12 +101,13 @@ export function generateGalleryHtml(designsDir: string): string { sessions.sort((a, b) => b.date.localeCompare(a.date)); const sessionCards = sessions.map(session => { + const approvedTarget = session.approved ? approvedImagePath(session.approved, session.dir) : null; const variantImgs = session.variants.map((vPath, i) => { try { const imgData = fs.readFileSync(vPath).toString("base64"); const ext = path.extname(vPath).slice(1) || "png"; const label = path.basename(vPath, `.${ext}`).replace("variant-", ""); - const isApproved = session.approved?.approved_variant === label; + const isApproved = approvedTarget?.path === vPath; return `
Variant ${label} @@ -200,7 +221,7 @@ export function generateGalleryHtml(designsDir: string): string {

Design History

-
${sessions.length} exploration${sessions.length === 1 ? "" : "s"}
+
${sessions.length} exploration${sessions.length === 1 ? "" : "s"} · images oldest to newest; letters do not identify rounds
${sessionCards} diff --git a/design/src/generate.ts b/design/src/generate.ts index 49bd401d0..c7ea9a796 100644 --- a/design/src/generate.ts +++ b/design/src/generate.ts @@ -2,14 +2,13 @@ * Generate UI mockups via OpenAI Responses API with image_generation tool. */ -import fs from "fs"; -import path from "path"; import { requireApiKey } from "./auth"; import { receiptedFetch } from "./receipted-fetch"; import { imageRequestBody, modelRejectionHint } from "./models"; import { parseBrief } from "./brief"; import { createSession, sessionPath } from "./session"; import { checkMockup } from "./check"; +import { emitResult, exitCodeFor, newAccounting, persistImage, recordOutcome, type ExitCode, type RunAccounting } from "./persist"; export interface GenerateOptions { brief?: string; @@ -21,11 +20,21 @@ export interface GenerateOptions { quality?: string; } -export interface GenerateResult { - outputPath: string; +export interface GenerateAttempt { + path: string; sessionFile: string; responseId: string; + check?: { pass: boolean; issues: string }; +} + +export interface GenerateResult extends RunAccounting { + outputPath: string | null; + sessionFile: string | null; + responseId: string | null; checkResult?: { pass: boolean; issues: string }; + attempts: GenerateAttempt[]; + selected: string | null; + exitCode: ExitCode; } /** @@ -86,69 +95,80 @@ async function callImageGeneration( } /** - * Generate a single mockup from a brief. + * Generate a single mockup from a brief. Every --retry attempt is its own paid + * image and is saved under its own claimed name before its quality check. */ export async function generate(options: GenerateOptions): Promise { - const apiKey = requireApiKey(); + const acct = newAccounting(1); + const attempts: GenerateAttempt[] = []; + let selected: GenerateAttempt | null = null; - // Parse the brief - const prompt = options.briefFile - ? parseBrief(options.briefFile, true) - : parseBrief(options.brief!, false); + try { + const apiKey = requireApiKey(); + const prompt = options.briefFile + ? parseBrief(options.briefFile, true) + : parseBrief(options.brief!, false); + const size = options.size || "1536x1024"; + const quality = options.quality || "high"; + const maxRetries = options.retry ?? 0; - const size = options.size || "1536x1024"; - const quality = options.quality || "high"; - const maxRetries = options.retry ?? 0; - - let lastResult: GenerateResult | null = null; - - for (let attempt = 0; attempt <= maxRetries; attempt++) { - if (attempt > 0) { - console.error(`Retry ${attempt}/${maxRetries}...`); - } - - // Generate the image - const startTime = Date.now(); - const { responseId, imageData } = await callImageGeneration(apiKey, prompt, size, quality); - const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - - // Write to disk - const outputDir = path.dirname(options.output); - fs.mkdirSync(outputDir, { recursive: true }); - const imageBuffer = Buffer.from(imageData, "base64"); - fs.writeFileSync(options.output, imageBuffer); - - // Create session - const session = createSession(responseId, prompt, options.output); - - console.error(`Generated (${elapsed}s, ${(imageBuffer.length / 1024).toFixed(0)}KB) → ${options.output}`); - - lastResult = { - outputPath: options.output, - sessionFile: sessionPath(session.id), - responseId, - }; - - // Quality check if requested - if (options.check) { - const checkResult = await checkMockup(options.output, prompt); - lastResult.checkResult = checkResult; + for (let attempt = 0; attempt <= maxRetries; attempt++) { + if (attempt > 0) { + console.error(`Retry ${attempt}/${maxRetries}...`); + } + + const startTime = Date.now(); + const { responseId, imageData } = await callImageGeneration(apiKey, prompt, size, quality); + const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); + + const outcome = persistImage(imageData, options.output); + const savedPath = recordOutcome(acct, outcome); + if (!savedPath) break; + + const session = createSession(responseId, prompt, savedPath); + const entry: GenerateAttempt = { path: savedPath, sessionFile: sessionPath(session.id), responseId }; + attempts.push(entry); + console.error(`Generated (${elapsed}s, ${outcome.ok ? (outcome.bytes / 1024).toFixed(0) : 0}KB) → ${savedPath}`); + + if (!options.check) { + selected = entry; + break; + } - if (checkResult.pass) { + entry.check = await checkMockup(savedPath, prompt); + if (entry.check.pass) { console.error(`Quality check: PASS`); + selected = entry; break; + } + console.error(`Quality check: FAIL — ${entry.check.issues}`); + if (attempt < maxRetries) { + console.error("Will retry..."); } else { - console.error(`Quality check: FAIL — ${checkResult.issues}`); - if (attempt < maxRetries) { - console.error("Will retry..."); - } + selected = entry; } - } else { - break; } + } catch (err: any) { + const reason = err?.message || String(err); + console.error(reason); + acct.failures.push({ file: options.output, reason }); } - // Output result as JSON to stdout - console.log(JSON.stringify(lastResult, null, 2)); - return lastResult!; + const exitCode = exitCodeFor(selected !== null, acct.saved.length); + const result: GenerateResult = { + outputPath: selected?.path ?? null, + sessionFile: selected?.sessionFile ?? null, + responseId: selected?.responseId ?? null, + ...(selected?.check ? { checkResult: selected.check } : {}), + attempts, + requested: acct.requested, + saved: acct.saved, + selected: selected?.path ?? null, + failures: acct.failures, + recovered: acct.recovered, + exitCode, + }; + const { exitCode: _omit, ...printed } = result; + emitResult(printed, exitCode); + return result; } diff --git a/design/src/image-args.ts b/design/src/image-args.ts new file mode 100644 index 000000000..5662f87ff --- /dev/null +++ b/design/src/image-args.ts @@ -0,0 +1,53 @@ +/** + * Image list parsing for `$D compare`. Accepted forms, in order: + * --images-file JSON array of path strings (lossless; skills use this) + * --images '[...]' JSON array string, only when it starts with `[` and parses as string[] + * --images 'dir/*.png' glob (sorted) + * --images 'a.png,b.png' comma-separated list, or a single path + */ + +import fs from "fs"; + +function parseStringArray(text: string): string[] | null { + try { + const value = JSON.parse(text); + if (Array.isArray(value) && value.every(item => typeof item === "string")) return value; + } catch {} + return null; +} + +export async function resolveImagePaths(input: string | undefined, imagesFile?: string): Promise { + if (imagesFile) { + let text: string; + try { + text = fs.readFileSync(imagesFile, "utf-8"); + } catch (err: any) { + throw new Error(`cannot read --images-file ${imagesFile}: ${err.code || err.message}`); + } + const list = parseStringArray(text); + if (!list) throw new Error(`--images-file ${imagesFile} must contain a JSON array of image path strings`); + return list; + } + + if (!input) { + throw new Error("--images or --images-file is required. Provide a JSON array file, glob pattern or comma-separated paths."); + } + + if (input.trimStart().startsWith("[")) { + const list = parseStringArray(input); + if (list) return list; + } + + if (input.includes("*")) { + const glob = new Bun.Glob(input); + const paths: string[] = []; + for await (const match of glob.scan({ absolute: true })) { + if (match.endsWith(".png") || match.endsWith(".jpg") || match.endsWith(".jpeg")) { + paths.push(match); + } + } + return paths.sort(); + } + + return input.split(",").map(p => p.trim()); +} diff --git a/design/src/iterate.ts b/design/src/iterate.ts index 85b70d4cc..47bf196c8 100644 --- a/design/src/iterate.ts +++ b/design/src/iterate.ts @@ -6,12 +6,11 @@ * with original brief + accumulated feedback in a single prompt. */ -import fs from "fs"; -import path from "path"; import { requireApiKey } from "./auth"; import { receiptedFetch } from "./receipted-fetch"; import { imageRequestBody, modelRejectionHint } from "./models"; import { readSession, updateSession } from "./session"; +import { emitResult, exitCodeFor, newAccounting, persistImage, recordOutcome, type ExitCode } from "./persist"; export interface IterateOptions { session: string; // Path to session JSON file @@ -20,62 +19,64 @@ export interface IterateOptions { } /** - * Iterate on an existing design using session state. + * Iterate on an existing design using session state. One received image means + * one claim; a local save failure never triggers the fallback purchase. */ -export async function iterate(options: IterateOptions): Promise { - const apiKey = requireApiKey(); - const session = readSession(options.session); - - console.error(`Iterating on session ${session.id}...`); - console.error(` Previous iterations: ${session.feedbackHistory.length}`); - console.error(` Feedback: "${options.feedback}"`); - - const startTime = Date.now(); - - // Try multi-turn with previous_response_id first - let success = false; - let responseId = ""; +export async function iterate(options: IterateOptions): Promise { + const acct = newAccounting(1); + let outputPath: string | null = null; + let responseId: string | null = null; + let iteration: number | null = null; try { - const result = await callWithThreading(apiKey, session.lastResponseId, options.feedback); - responseId = result.responseId; + const apiKey = requireApiKey(); + const session = readSession(options.session); - fs.mkdirSync(path.dirname(options.output), { recursive: true }); - fs.writeFileSync(options.output, Buffer.from(result.imageData, "base64")); - success = true; + console.error(`Iterating on session ${session.id}...`); + console.error(` Previous iterations: ${session.feedbackHistory.length}`); + console.error(` Feedback: "${options.feedback}"`); + + const startTime = Date.now(); + let image: { responseId: string; imageData: string }; + try { + image = await callWithThreading(apiKey, session.lastResponseId, options.feedback); + } catch (err: any) { + console.error(` Threading failed: ${err.message}`); + console.error(" Falling back to re-generation with accumulated feedback..."); + const accumulatedPrompt = buildAccumulatedPrompt( + session.originalBrief, + [...session.feedbackHistory, options.feedback] + ); + image = await callFresh(apiKey, accumulatedPrompt); + } + + const outcome = persistImage(image.imageData, options.output); + outputPath = recordOutcome(acct, outcome); + if (outcome.ok) { + responseId = image.responseId; + const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); + console.error(`Generated (${elapsed}s, ${(outcome.bytes / 1024).toFixed(0)}KB) → ${outcome.path}`); + updateSession(session, image.responseId, options.feedback, outcome.path); + iteration = session.feedbackHistory.length + 1; + } } catch (err: any) { - console.error(` Threading failed: ${err.message}`); - console.error(" Falling back to re-generation with accumulated feedback..."); - - // Fallback: re-generate with original brief + all feedback - const accumulatedPrompt = buildAccumulatedPrompt( - session.originalBrief, - [...session.feedbackHistory, options.feedback] - ); - - const result = await callFresh(apiKey, accumulatedPrompt); - responseId = result.responseId; - - fs.mkdirSync(path.dirname(options.output), { recursive: true }); - fs.writeFileSync(options.output, Buffer.from(result.imageData, "base64")); - success = true; + const reason = err?.message || String(err); + console.error(reason); + acct.failures.push({ file: options.output, reason }); } - if (success) { - const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - const size = fs.statSync(options.output).size; - console.error(`Generated (${elapsed}s, ${(size / 1024).toFixed(0)}KB) → ${options.output}`); - - // Update session - updateSession(session, responseId, options.feedback, options.output); - - console.log(JSON.stringify({ - outputPath: options.output, - sessionFile: options.session, - responseId, - iteration: session.feedbackHistory.length + 1, - }, null, 2)); - } + return emitResult({ + outputPath, + sessionFile: options.session, + responseId, + iteration, + attempts: acct.saved.map(p => ({ path: p })), + requested: acct.requested, + saved: acct.saved, + selected: outputPath, + failures: acct.failures, + recovered: acct.recovered, + }, exitCodeFor(outputPath !== null, acct.saved.length)); } async function callWithThreading( diff --git a/design/src/persist.ts b/design/src/persist.ts new file mode 100644 index 000000000..37b16a121 --- /dev/null +++ b/design/src/persist.ts @@ -0,0 +1,196 @@ +/** + * Persistence for paid design images. Every image the API returns goes through + * persistImage exactly once: mkdir, exclusive claim (never overwrite), write, + * fsync, close. Any failure after the bytes arrived removes the claimed file and + * leaves a private recovery copy in os.tmpdir(), so a paid image is never lost + * or silently replaced. + * + * Every generating command prints the same accounting fields (see + * commands.ts for the $D contract): + * requested number of images the command set out to produce + * saved every image path written, in order + * selected outputPath (single-image commands) or paths (variants) + * failures [{file, reason}] for every image that was not saved + * recovered [{path, reason}] recovery copies written after a save failed + * Exit 0 when a selected result exists, 2 when nothing was saved, 3 when the + * run stopped after saving at least one image but before its selected result. + */ + +import crypto from "crypto"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +export const CLAIM_CAP = 999; + +export interface PersistFs { + mkdirSync(p: string, opts: { recursive: true }): unknown; + openSync(p: string, flags: string, mode?: number): number; + writeSync(fd: number, buf: Buffer, offset: number, length: number): number; + fsyncSync(fd: number): void; + closeSync(fd: number): void; + unlinkSync(p: string): void; +} + +export interface PersistDeps { + fs?: PersistFs; + tmpdir?: string; +} + +export interface Failure { file: string; reason: string } +export interface Recovery { path: string; reason: string } + +export type PersistOutcome = + | { ok: true; path: string; bytes: number } + | { ok: false; failure: Failure; recovered?: Recovery }; + +export interface RunAccounting { + requested: number; + saved: string[]; + failures: Failure[]; + recovered: Recovery[]; +} + +/** `x.png` -> `x-2.png`; `x-2.png` -> `x-2-2.png`. The suffix always appends to the requested stem. */ +export function bumpedName(requested: string, n: number): string { + const ext = path.extname(requested); + return `${requested.slice(0, requested.length - ext.length)}-${n}${ext}`; +} + +/** + * Atomically create the first free name among requested, -2 ... -999 with an + * exclusive `wx` open. Only EEXIST bumps; any other open error throws at once. + */ +export function claimOutputPath(requested: string, fsImpl: PersistFs = fs): { path: string; fd: number } { + for (let n = 1; n <= CLAIM_CAP; n++) { + const candidate = n === 1 ? requested : bumpedName(requested, n); + try { + return { path: candidate, fd: fsImpl.openSync(candidate, "wx") }; + } catch (err: any) { + if (err?.code !== "EEXIST") throw err; + } + } + const capErr = new Error(`${requested} and every name through ${path.basename(bumpedName(requested, CLAIM_CAP))} already exist`); + throw Object.assign(capErr, { code: "EEXIST" }); +} + +function writeAll(fsImpl: PersistFs, fd: number, buf: Buffer): void { + let offset = 0; + while (offset < buf.length) { + offset += fsImpl.writeSync(fd, buf, offset, buf.length - offset); + } +} + +function fixHint(code: string, dir: string): string { + switch (code) { + case "EACCES": + case "EPERM": + case "EROFS": + return `make ${dir} writable or pass a writable --output/--output-dir`; + case "ENOSPC": + case "EDQUOT": + return `free disk space on the volume holding ${dir}, then rerun`; + case "EEXIST": + return `choose a new --output name or move older images out of ${dir}`; + case "ENOENT": + case "ENOTDIR": + return `check that the --output/--output-dir path is a directory you can create`; + default: + return `pass a writable --output/--output-dir`; + } +} + +function describeError(err: any): { code: string; cause: string } { + return { code: err?.code || "ERROR", cause: err?.message || String(err) }; +} + +function writeRecoveryCopy(bytes: Buffer, fsImpl: PersistFs, tmp: string): string { + const stamp = new Date().toISOString().replace(/[-:.]/g, ""); + const recoveryPath = path.join(tmp, `gstack-design-unsaved-${stamp}-${crypto.randomBytes(4).toString("hex")}.png`); + const fd = fsImpl.openSync(recoveryPath, "wx", 0o600); + try { + writeAll(fsImpl, fd, bytes); + fsImpl.closeSync(fd); + } catch (err) { + try { fsImpl.closeSync(fd); } catch {} + try { fsImpl.unlinkSync(recoveryPath); } catch {} + throw err; + } + return recoveryPath; +} + +/** + * Save one received image under `requested` (or its first free -N name). + * Empty image data is a generation failure: no claim, no file. + */ +export function persistImage(imageData: string, requested: string, deps: PersistDeps = {}): PersistOutcome { + const fsImpl = deps.fs ?? fs; + const bytes = Buffer.from(imageData || "", "base64"); + if (bytes.length === 0) { + const reason = "API returned an empty image; nothing was saved"; + console.error(` ✗ ${path.basename(requested)}: ${reason}`); + return { ok: false, failure: { file: requested, reason } }; + } + + let claimed: { path: string; fd: number } | null = null; + try { + fsImpl.mkdirSync(path.dirname(requested), { recursive: true }); + claimed = claimOutputPath(requested, fsImpl); + writeAll(fsImpl, claimed.fd, bytes); + fsImpl.fsyncSync(claimed.fd); + fsImpl.closeSync(claimed.fd); + if (claimed.path !== requested) { + console.error(`note: ${requested} exists; saved to ${claimed.path} (existing file kept)`); + } + return { ok: true, path: claimed.path, bytes: bytes.length }; + } catch (err) { + const target = claimed?.path ?? requested; + if (claimed) { + try { fsImpl.closeSync(claimed.fd); } catch {} + try { fsImpl.unlinkSync(claimed.path); } catch {} + } + const { code, cause } = describeError(err); + const fix = fixHint(code, path.dirname(requested)); + const tmp = deps.tmpdir ?? os.tmpdir(); + let reason: string; + let recovered: Recovery | undefined; + try { + const recoveryPath = writeRecoveryCopy(bytes, fsImpl, tmp); + reason = `cannot save paid image to ${target}: ${code} (${cause}). Image bytes were received; saved a recovery copy to ${recoveryPath}. Fix: ${fix}.`; + recovered = { path: recoveryPath, reason: `${code} (${cause})` }; + } catch (recoveryErr) { + const second = describeError(recoveryErr); + reason = `cannot save paid image to ${target}: ${code} (${cause}). Image bytes were received (${bytes.length} bytes) but the recovery copy in ${tmp} also failed: ${second.code} (${second.cause}). Fix: ${fix}.`; + } + console.error(reason); + return { ok: false, failure: { file: requested, reason }, ...(recovered ? { recovered } : {}) }; + } +} + +export function newAccounting(requested: number): RunAccounting { + return { requested, saved: [], failures: [], recovered: [] }; +} + +/** Fold one persist outcome into the run's accounting; returns the saved path or null. */ +export function recordOutcome(acct: RunAccounting, outcome: PersistOutcome): string | null { + if (outcome.ok) { + acct.saved.push(outcome.path); + return outcome.path; + } + acct.failures.push(outcome.failure); + if (outcome.recovered) acct.recovered.push(outcome.recovered); + return null; +} + +export function exitCodeFor(hasSelected: boolean, savedCount: number): ExitCode { + if (hasSelected) return 0; + return savedCount === 0 ? 2 : 3; +} + +export type ExitCode = 0 | 2 | 3; + +/** Print the command's JSON on stdout and return its exit code for the CLI. */ +export function emitResult(result: Record, exitCode: ExitCode): ExitCode { + console.log(JSON.stringify(result, null, 2)); + return exitCode; +} diff --git a/design/src/session.ts b/design/src/session.ts index 01986618e..57fc2bcd4 100644 --- a/design/src/session.ts +++ b/design/src/session.ts @@ -1,9 +1,10 @@ /** * Session state management for multi-turn design iteration. - * Session files are JSON in /tmp, keyed by PID + timestamp. + * Session files are JSON in the OS temp directory, keyed by PID + timestamp. */ import fs from "fs"; +import os from "os"; import path from "path"; export interface DesignSession { @@ -27,7 +28,7 @@ export function createSessionId(): string { * Get the file path for a session. */ export function sessionPath(sessionId: string): string { - return path.join("/tmp", `design-session-${sessionId}.json`); + return path.join(os.tmpdir(), `design-session-${sessionId}.json`); } /** diff --git a/design/src/variants.ts b/design/src/variants.ts index 5042da98c..7a1e14316 100644 --- a/design/src/variants.ts +++ b/design/src/variants.ts @@ -11,6 +11,15 @@ import { receiptedFetch } from "./receipted-fetch"; import { imageRequestBody, modelRejectionHint } from "./models"; import { parseBrief } from "./brief"; import { normalizeIntFlag } from "./flag-utils"; +import { + emitResult, + exitCodeFor, + newAccounting, + persistImage, + type ExitCode, + type Recovery, + type RunAccounting, +} from "./persist"; export interface VariantsOptions { brief?: string; @@ -37,20 +46,25 @@ const STYLE_VARIATIONS = [ "Use a playful, modern style with asymmetric layout and unexpected color accents.", ]; +export interface VariantResult { + requested: string; + path: string; + success: boolean; + error?: string; + recovered?: Recovery; +} + /** - * 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. + * Request one variant image, retrying only the API call on 429. Returns the + * base64 image or the final error; nothing here touches the filesystem. */ -export async function generateVariant( +export async function requestVariantImage( apiKey: string, prompt: string, - outputPath: string, size: string, quality: string, fetchFn: typeof globalThis.fetch = globalThis.fetch, -): Promise<{ path: string; success: boolean; error?: string }> { +): Promise<{ imageData: string } | { error: string }> { const maxRetries = 3; const MAX_RETRY_AFTER_MS = 60_000; // cap honored Retry-After to bound stalls let lastError = ""; @@ -59,7 +73,7 @@ export async function generateVariant( try { body = imageRequestBody(prompt, { size, quality }); } catch (err: any) { - return { path: outputPath, success: false, error: err.message }; + return { error: err.message }; } for (let attempt = 0; attempt <= maxRetries; attempt++) { @@ -74,8 +88,9 @@ export async function generateVariant( const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 240_000); + let response: Response; try { - const response = await receiptedFetch("variants-image-request", "https://api.openai.com/v1/responses", { + response = await receiptedFetch("variants-image-request", "https://api.openai.com/v1/responses", { method: "POST", headers: { "Authorization": `Bearer ${apiKey}`, @@ -84,83 +99,174 @@ export async function generateVariant( body, signal: controller.signal, }, fetchFn); - - clearTimeout(timeout); - - if (response.status === 429) { - lastError = "Rate limited (429)"; - const retryAfter = response.headers.get("retry-after"); - if (retryAfter) { - const trimmed = retryAfter.trim(); - let waitMs: number | null = null; - if (/^\d+$/.test(trimmed)) { - // delta-seconds (RFC 7231) - waitMs = Math.min(Number.parseInt(trimmed, 10) * 1000, MAX_RETRY_AFTER_MS); - } else { - // HTTP-date (RFC 7231) - const dateMs = Date.parse(trimmed); - if (!Number.isNaN(dateMs)) { - waitMs = Math.min(Math.max(0, dateMs - Date.now()), MAX_RETRY_AFTER_MS); - } - } - if (waitMs !== null) { - if (waitMs > 0) { - await new Promise(resolve => setTimeout(resolve, waitMs)); - } - // Honored Retry-After (incl. 0 / past date "retry now") — skip the - // next iteration's leading exponential sleep so we don't double-wait. - skipLeadingDelay = true; - } - } - continue; - } - - if (!response.ok) { - const error = await response.text(); - 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: `API error (${response.status}): ${error.slice(0, 200)}${modelRejectionHint(response.status, error, "image")}` }; - } - - const data = await response.json() as any; - 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" }; - } - - fs.writeFileSync(outputPath, Buffer.from(imageItem.result, "base64")); - return { path: outputPath, success: true }; } catch (err: any) { clearTimeout(timeout); - if (err.name === "AbortError") { - return { path: outputPath, success: false, error: "Timeout (240s)" }; - } + if (err.name === "AbortError") return { error: "Timeout (240s)" }; lastError = err.message; + continue; + } + clearTimeout(timeout); + + if (response.status === 429) { + lastError = "Rate limited (429)"; + const retryAfter = response.headers.get("retry-after"); + if (retryAfter) { + const trimmed = retryAfter.trim(); + let waitMs: number | null = null; + if (/^\d+$/.test(trimmed)) { + // delta-seconds (RFC 7231) + waitMs = Math.min(Number.parseInt(trimmed, 10) * 1000, MAX_RETRY_AFTER_MS); + } else { + // HTTP-date (RFC 7231) + const dateMs = Date.parse(trimmed); + if (!Number.isNaN(dateMs)) { + waitMs = Math.min(Math.max(0, dateMs - Date.now()), MAX_RETRY_AFTER_MS); + } + } + if (waitMs !== null) { + if (waitMs > 0) { + await new Promise(resolve => setTimeout(resolve, waitMs)); + } + // Honored Retry-After (incl. 0 / past date "retry now") — skip the + // next iteration's leading exponential sleep so we don't double-wait. + skipLeadingDelay = true; + } + } + continue; + } + + if (!response.ok) { + const error = await response.text(); + if (response.status === 403 && error.includes("organization must be verified")) { + return { error: "OpenAI organization verification required. Go to https://platform.openai.com/settings/organization to verify." }; + } + return { error: `API error (${response.status}): ${error.slice(0, 200)}${modelRejectionHint(response.status, error, "image")}` }; + } + + try { + const data = await response.json() as any; + const imageItem = data.output?.find((item: any) => item.type === "image_generation_call"); + if (!imageItem?.result) return { error: "No image data in response" }; + return { imageData: imageItem.result }; + } catch (err: any) { + return { error: `Unreadable API response: ${err.message}` }; } } - return { path: outputPath, success: false, error: lastError }; + return { error: lastError }; +} + +/** + * Generate a single variant: one API request (with 429 retry), then exactly + * one persistence attempt on the received bytes. A save failure is reported + * with its path and never triggers another request. + * + * Exported for testability. Pass `fetchFn` to inject a stubbed fetch in tests; + * production code uses the global fetch by default. + */ +export async function generateVariant( + apiKey: string, + prompt: string, + outputPath: string, + size: string, + quality: string, + fetchFn: typeof globalThis.fetch = globalThis.fetch, +): Promise { + const received = await requestVariantImage(apiKey, prompt, size, quality, fetchFn); + if ("error" in received) { + return { requested: outputPath, path: outputPath, success: false, error: received.error }; + } + const outcome = persistImage(received.imageData, outputPath); + if (outcome.ok) return { requested: outputPath, path: outcome.path, success: true }; + return { + requested: outputPath, + path: outputPath, + success: false, + error: outcome.failure.reason, + ...(outcome.recovered ? { recovered: outcome.recovered } : {}), + }; +} + +interface VariantJob { + outputPath: string; + prompt: string; + size: string; + label: string; +} + +/** Launch jobs 1.5s apart, wait for all, and fold them into run accounting. */ +async function runVariantJobs(apiKey: string, quality: string, jobs: VariantJob[]): Promise<{ + acct: RunAccounting; + errors: string[]; +}> { + const promises = jobs.map((job, i) => + new Promise(resolve => setTimeout(resolve, i * 1500)).then(() => { + console.error(` Starting ${job.label}...`); + return generateVariant(apiKey, job.prompt, job.outputPath, job.size, quality); + }) + ); + const results = await Promise.allSettled(promises); + + const acct = newAccounting(jobs.length); + const errors: string[] = []; + results.forEach((result, i) => { + const requested = jobs[i].outputPath; + if (result.status === "fulfilled" && result.value.success) { + const size = fs.statSync(result.value.path).size; + console.error(` ✓ ${path.basename(result.value.path)} (${(size / 1024).toFixed(0)}KB)`); + acct.saved.push(result.value.path); + return; + } + const reason = result.status === "fulfilled" ? result.value.error || "unknown error" : (result.reason as Error).message; + console.error(` ✗ ${path.basename(requested)}: ${reason}`); + errors.push(path.basename(requested)); + acct.failures.push({ file: requested, reason }); + if (result.status === "fulfilled" && result.value.recovered) acct.recovered.push(result.value.recovered); + }); + return { acct, errors }; +} + +function emitVariantsResult(outputDir: string, acct: RunAccounting, errors: string[], extra: Record = {}): ExitCode { + return emitResult({ + outputDir, + ...extra, + count: acct.requested, + succeeded: acct.saved.length, + failed: errors.length, + paths: acct.saved, + errors, + requested: acct.requested, + saved: acct.saved, + selected: acct.saved, + failures: acct.failures, + recovered: acct.recovered, + }, exitCodeFor(acct.saved.length > 0, acct.saved.length)); } /** * Generate N variants with staggered parallel execution. */ -export async function variants(options: VariantsOptions): Promise { - const apiKey = requireApiKey(); - const baseBrief = options.briefFile - ? parseBrief(options.briefFile, true) - : parseBrief(options.brief!, false); +export async function variants(options: VariantsOptions): Promise { + let apiKey: string; + let baseBrief: string; + try { + apiKey = requireApiKey(); + baseBrief = options.briefFile + ? parseBrief(options.briefFile, true) + : parseBrief(options.brief!, false); + } catch (err: any) { + const reason = err?.message || String(err); + console.error(reason); + const acct = newAccounting(0); + acct.failures.push({ file: options.outputDir, reason }); + return emitVariantsResult(options.outputDir, acct, []); + } const quality = options.quality || "high"; - 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); - return; + return generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality); } // #2032: normalize at the consumption site so every caller (CLI or @@ -177,58 +283,22 @@ export async function variants(options: VariantsOptions): Promise { console.error(`Generating ${count} variants...`); const startTime = Date.now(); - // Staggered parallel: start each call 1.5s apart - const promises: Promise<{ path: string; success: boolean; error?: string }>[] = []; - + const jobs: VariantJob[] = []; for (let i = 0; i < count; i++) { const variation = STYLE_VARIATIONS[i] || ""; - const prompt = variation - ? `${baseBrief}\n\nStyle direction: ${variation}` - : baseBrief; - - const outputPath = path.join(options.outputDir, `variant-${String.fromCharCode(65 + i)}.png`); - - // Stagger: wait 1.5s between launches - const delay = i * 1500; - promises.push( - new Promise(resolve => setTimeout(resolve, delay)) - .then(() => { - console.error(` Starting variant ${String.fromCharCode(65 + i)}...`); - return generateVariant(apiKey, prompt, outputPath, size, quality); - }) - ); + const letter = String.fromCharCode(65 + i); + jobs.push({ + outputPath: path.join(options.outputDir, `variant-${letter}.png`), + prompt: variation ? `${baseBrief}\n\nStyle direction: ${variation}` : baseBrief, + size, + label: `variant ${letter}`, + }); } - const results = await Promise.allSettled(promises); + const { acct, errors } = await runVariantJobs(apiKey, quality, jobs); const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - - const succeeded: string[] = []; - const failed: string[] = []; - - for (const result of results) { - if (result.status === "fulfilled" && result.value.success) { - const size = fs.statSync(result.value.path).size; - console.error(` ✓ ${path.basename(result.value.path)} (${(size / 1024).toFixed(0)}KB)`); - succeeded.push(result.value.path); - } else { - const error = result.status === "fulfilled" ? result.value.error : (result.reason as Error).message; - const filePath = result.status === "fulfilled" ? result.value.path : "unknown"; - console.error(` ✗ ${path.basename(filePath)}: ${error}`); - failed.push(path.basename(filePath)); - } - } - - console.error(`\n${succeeded.length}/${count} variants generated (${elapsed}s)`); - - // Output structured result to stdout - console.log(JSON.stringify({ - outputDir: options.outputDir, - count, - succeeded: succeeded.length, - failed: failed.length, - paths: succeeded, - errors: failed, - }, null, 2)); + console.error(`\n${acct.saved.length}/${count} variants generated (${elapsed}s)`); + return emitVariantsResult(options.outputDir, acct, errors); } const VIEWPORT_CONFIGS: Record = { @@ -243,55 +313,34 @@ async function generateResponsiveVariants( outputDir: string, viewports: string, quality: string, -): Promise { +): Promise { const viewportList = viewports.split(",").map(v => v.trim().toLowerCase()); const configs = viewportList.map(v => VIEWPORT_CONFIGS[v]).filter(Boolean); if (configs.length === 0) { - console.error(`No valid viewports. Use: desktop, tablet, mobile`); - process.exit(1); + const reason = `No valid viewports. Use: desktop, tablet, mobile`; + console.error(reason); + const acct = newAccounting(viewportList.length); + acct.failures.push({ file: outputDir, reason }); + return emitVariantsResult(outputDir, acct, [], { viewports: viewportList }); } console.error(`Generating responsive variants: ${configs.map(c => c.desc).join(", ")}...`); const startTime = Date.now(); - const promises = configs.map((config, i) => { - const prompt = `${baseBrief}\n\nViewport: ${config.desc}. Adapt the layout for this screen size. ${ + const jobs: VariantJob[] = configs.map(config => ({ + outputPath: path.join(outputDir, `responsive-${config.suffix}.png`), + prompt: `${baseBrief}\n\nViewport: ${config.desc}. Adapt the layout for this screen size. ${ config.suffix === "mobile" ? "Use a single-column layout, larger touch targets, and mobile navigation patterns." : config.suffix === "tablet" ? "Use a responsive layout that works for medium screens." : "" - }`; - const outputPath = path.join(outputDir, `responsive-${config.suffix}.png`); - const delay = i * 1500; + }`, + size: config.size, + label: config.desc, + })); - return new Promise<{ path: string; success: boolean; error?: string }>(resolve => - setTimeout(resolve, delay) - ).then(() => { - console.error(` Starting ${config.desc}...`); - return generateVariant(apiKey, prompt, outputPath, config.size, quality); - }); - }); - - const results = await Promise.allSettled(promises); + const { acct, errors } = await runVariantJobs(apiKey, quality, jobs); const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - - const succeeded: string[] = []; - for (const result of results) { - if (result.status === "fulfilled" && result.value.success) { - const sz = fs.statSync(result.value.path).size; - console.error(` ✓ ${path.basename(result.value.path)} (${(sz / 1024).toFixed(0)}KB)`); - succeeded.push(result.value.path); - } else { - const error = result.status === "fulfilled" ? result.value.error : (result.reason as Error).message; - console.error(` ✗ ${error}`); - } - } - - console.error(`\n${succeeded.length}/${configs.length} responsive variants generated (${elapsed}s)`); - console.log(JSON.stringify({ - outputDir, - viewports: viewportList, - succeeded: succeeded.length, - paths: succeeded, - }, null, 2)); + console.error(`\n${acct.saved.length}/${configs.length} responsive variants generated (${elapsed}s)`); + return emitVariantsResult(outputDir, acct, errors, { viewports: viewportList }); } diff --git a/design/test/approval.test.ts b/design/test/approval.test.ts new file mode 100644 index 000000000..7a0e877e8 --- /dev/null +++ b/design/test/approval.test.ts @@ -0,0 +1,61 @@ +/** + * approved.json readers: approved_path (relative to approved.json) wins, legacy + * letter-only records still resolve, and a missing approved image is an error, + * never a substitute. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { resolveApprovedImage } from "../src/approval"; + +const BIN = path.join(import.meta.dir, "..", "..", "bin", "gstack-design-approved"); +let dir: string; +beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "design-approval-")); }); +afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + +function record(sessionDir: string, data: object): string { + fs.mkdirSync(sessionDir, { recursive: true }); + const file = path.join(sessionDir, "approved.json"); + fs.writeFileSync(file, JSON.stringify(data)); + return file; +} + +describe("resolveApprovedImage", () => { + // Value: protects=pre-upgrade letter-only records keep resolving to variant-.png; fails_when=the legacy fallback is removed; + // why_new=approved_path is new and legacy records exist on disk; seam=none + test("legacy letter-only record resolves by letter", () => { + const file = record(dir, { approved_variant: "B" }); + fs.writeFileSync(path.join(dir, "variant-B.png"), "b"); + expect(resolveApprovedImage(file)).toEqual({ ok: true, path: path.join(dir, "variant-B.png"), legacy: true }); + }); + + // Value: protects=a present-but-missing approved_path errors instead of resolving variant-A.png; fails_when=readers fall back to the letter; + // why_new=eng approval-fallback precedence; seam=none + test("missing approved_path errors and never substitutes the letter's image", () => { + const file = record(dir, { approved_variant: "A", approved_path: "variant-A-2.png" }); + fs.writeFileSync(path.join(dir, "variant-A.png"), "round one"); + const r = resolveApprovedImage(file); + expect(r).toEqual({ ok: false, error: `approved image ${path.join(dir, "variant-A-2.png")} is missing; reselect from the board` }); + const cli = spawnSync(process.execPath, [BIN, file], { encoding: "utf8", timeout: 30_000 }); + expect(cli.status).toBe(1); + expect(cli.stdout).toBe(""); + expect(cli.stderr).toContain("reselect from the board"); + }); + + // Value: protects=approved_path is relative, so a copied or relocated designs directory still resolves; fails_when=an absolute path is stored or resolved against cwd; + // why_new=DX relocation requirement; seam=none + test("relocated designs directory still resolves the approved image", () => { + const original = path.join(dir, "state", "designs", "home-20261003"); + record(original, { approved_variant: "A", approved_path: "variant-A-2.png" }); + fs.writeFileSync(path.join(original, "variant-A-2.png"), "round two"); + const moved = path.join(dir, "elsewhere", "home-20261003"); + fs.cpSync(original, moved, { recursive: true }); + fs.rmSync(path.join(dir, "state"), { recursive: true }); + const cli = spawnSync(process.execPath, [BIN, path.join(moved, "approved.json")], { encoding: "utf8", timeout: 30_000, cwd: os.tmpdir() }); + expect(cli.status, cli.stderr).toBe(0); + expect(cli.stdout.trim()).toBe(path.join(moved, "variant-A-2.png")); + }); +}); diff --git a/design/test/cli-persistence.test.ts b/design/test/cli-persistence.test.ts new file mode 100644 index 000000000..fc4589c51 --- /dev/null +++ b/design/test/cli-persistence.test.ts @@ -0,0 +1,223 @@ +/** + * $D CLI persistence contract with the image API stubbed (no spend): paid + * images are never overwritten, every saved image is reported, a local save + * failure never buys a second image, and JSON is printed on every exit. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const CLI = path.join(import.meta.dir, "..", "src", "cli.ts"); +const RUN_FIELDS = ["requested", "saved", "selected", "failures", "recovered"]; + +let dir: string; +let calls: string; +let preload: string; +const sessions: string[] = []; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "design-cli-persist-")); + calls = path.join(dir, "calls.log"); + preload = path.join(dir, "stub.ts"); + fs.writeFileSync(preload, `import { appendFileSync, existsSync, readFileSync } from "node:fs"; +const seq = (name: string) => (process.env[name] || "").split(",").filter(Boolean); +globalThis.fetch = (async (url: unknown) => { + const pathname = new URL(String(url)).pathname; + const log = process.env.CALLS!; + const prior = existsSync(log) ? readFileSync(log, "utf8").split("\\n").filter(l => l === pathname).length : 0; + appendFileSync(log, pathname + "\\n"); + if (pathname === "/v1/responses") { + const step = seq("IMAGES")[prior] ?? "ok"; + if (step === "fail") return new Response("upstream unavailable", { status: 503 }); + const result = step === "empty" ? "====" : Buffer.from("image " + prior).toString("base64"); + return Response.json({ id: "resp-" + prior, output: [{ type: "image_generation_call", result }] }); + } + const step = seq("CHECKS")[prior] ?? "PASS"; + if (step === "throw") throw Object.assign(new Error("The operation was aborted."), { name: "AbortError" }); + return Response.json({ choices: [{ message: { content: step === "PASS" ? "PASS" : "FAIL: illegible title" } }] }); +}) as typeof fetch; +`); +}); + +afterEach(() => { + for (const s of sessions.splice(0)) fs.rmSync(s, { force: true }); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +function run(args: string[], env: Record = {}) { + const r = spawnSync(process.execPath, ["--no-env-file", "--preload", preload, CLI, ...args], { + encoding: "utf8", + timeout: 60_000, + env: { + PATH: process.env.PATH!, HOME: dir, GSTACK_HOME: path.join(dir, "state"), TMPDIR: dir, + OPENAI_API_KEY: "fixture-not-a-real-key", CALLS: calls, ...env, + }, + }); + const json = JSON.parse(r.stdout); + for (const a of json.attempts ?? []) if (a.sessionFile) sessions.push(a.sessionFile); + return { status: r.status, stderr: r.stderr, json }; +} + +const apiCalls = () => fs.existsSync(calls) ? fs.readFileSync(calls, "utf8").split("\n").filter(l => l === "/v1/responses").length : 0; + +describe("never overwrite", () => { + // Value: protects=a second generate into the same --output keeps the first image and reports the bumped path with the exact notice; + // fails_when=generate writes with writeFileSync again or reports the requested path; why_new=#1529 had no CLI coverage; seam=none + test("generate bumps, prints the notice and leaves the original bytes", () => { + const out = path.join(dir, "mock.png"); + const first = run(["generate", "--brief", "dashboard", "--output", out]); + expect(first.status, first.stderr).toBe(0); + expect(first.json.outputPath).toBe(out); + const second = run(["generate", "--brief", "dashboard", "--output", out]); + const bumped = path.join(dir, "mock-2.png"); + expect(second.status, second.stderr).toBe(0); + expect(second.json.outputPath).toBe(bumped); + expect(second.json.saved).toEqual([bumped]); + expect(second.stderr).toContain(`note: ${out} exists; saved to ${bumped} (existing file kept)`); + expect(fs.readFileSync(out, "utf8")).toBe("image 0"); + expect(fs.readFileSync(bumped, "utf8")).toBe("image 1"); + for (const key of [...RUN_FIELDS, "outputPath", "attempts"]) expect(second.json).toHaveProperty(key); + }); + + // Value: protects=a second variants round keeps round one, reports variant-X-2 paths, and both branches print one key set; + // fails_when=variants overwrites or the --viewports branch drops count/errors/failures; why_new=#1529 overwrite path; seam=none + test("variants (both branches) bump per file with one shared key set", () => { + const outDir = path.join(dir, "designs"); + const r1 = run(["variants", "--brief", "dashboard", "--count", "2", "--output-dir", outDir]); + expect(r1.json.paths).toEqual([path.join(outDir, "variant-A.png"), path.join(outDir, "variant-B.png")]); + const r2 = run(["variants", "--brief", "dashboard", "--count", "2", "--output-dir", outDir]); + expect(r2.status, r2.stderr).toBe(0); + expect(r2.json.paths).toEqual([path.join(outDir, "variant-A-2.png"), path.join(outDir, "variant-B-2.png")]); + expect(r2.stderr).toContain(`note: ${path.join(outDir, "variant-A.png")} exists; saved to ${path.join(outDir, "variant-A-2.png")} (existing file kept)`); + expect(fs.readFileSync(path.join(outDir, "variant-A.png"), "utf8")).toBe("image 0"); + run(["variants", "--brief", "dashboard", "--viewports", "desktop", "--output-dir", outDir]); + const again = run(["variants", "--brief", "dashboard", "--viewports", "desktop", "--output-dir", outDir]); + expect(again.json.paths).toEqual([path.join(outDir, "responsive-desktop-2.png")]); + expect(fs.readFileSync(path.join(outDir, "responsive-desktop.png"), "utf8")).toBe("image 4"); + const keys = (o: object) => Object.keys(o).filter(k => k !== "viewports").sort(); + expect(keys(again.json)).toEqual(keys(r2.json)); + expect(again.json).toMatchObject({ count: 1, succeeded: 1, failed: 0, errors: [], failures: [], viewports: ["desktop"] }); + for (const key of [...RUN_FIELDS, "outputDir", "count", "succeeded", "failed", "paths", "errors"]) expect(r2.json).toHaveProperty(key); + }); +}); + +describe("accounting and exit codes", () => { + // Value: protects=each --retry attempt is its own saved, counted image; fails_when=retries overwrite one --output or attempts drop rejected images; + // why_new=the retry premise was wrong in #1737; seam=none + test("generate --check --retry keeps the rejected and the passing image", () => { + const out = path.join(dir, "retry.png"); + const r = run(["generate", "--brief", "dashboard", "--output", out, "--check", "--retry", "1"], { CHECKS: "FAIL,PASS" }); + expect(r.status, r.stderr).toBe(0); + const second = path.join(dir, "retry-2.png"); + expect(r.json.saved).toEqual([out, second]); + expect(r.json.attempts.map((a: any) => [a.path, a.check.pass])).toEqual([[out, false], [second, true]]); + expect(r.json.outputPath).toBe(second); + expect(fs.existsSync(out) && fs.existsSync(second)).toBe(true); + }); + + // Value: protects=an interrupted retry exits 3 with JSON listing the earlier saved attempt; fails_when=the error escapes as exit 1 without JSON; + // why_new=new exit contract; seam=none + test("interrupted retry exits 3 and still prints every saved path", () => { + const out = path.join(dir, "retry.png"); + const r = run(["generate", "--brief", "dashboard", "--output", out, "--check", "--retry", "2"], { CHECKS: "FAIL", IMAGES: "ok,fail" }); + expect(r.status).toBe(3); + expect(r.json).toMatchObject({ outputPath: null, selected: null, saved: [out], requested: 1 }); + expect(r.json.failures).toHaveLength(1); + expect(r.json.failures[0].reason).toContain("503"); + }); + + // Value: protects=a quality-check timeout after a save still prints partial JSON and exits 3; fails_when=a check exception loses the saved path; + // why_new=DX contract case; seam=none + test("quality-check timeout after a save exits 3 with partial JSON", () => { + const out = path.join(dir, "checked.png"); + const r = run(["generate", "--brief", "dashboard", "--output", out, "--check"], { CHECKS: "throw" }); + expect(r.status).toBe(3); + expect(r.json.saved).toEqual([out]); + expect(r.json.failures[0].reason).toContain("aborted"); + }); + + // Value: protects=zero saved images exit 2 with JSON naming every failure; fails_when=all-fail exits 0 or prints nothing; + // why_new=skills route on this code; seam=none + test("variants with zero successes exits 2 with JSON", () => { + const outDir = path.join(dir, "designs"); + const r = run(["variants", "--brief", "dashboard", "--count", "2", "--output-dir", outDir], { IMAGES: "fail,fail" }); + expect(r.status).toBe(2); + expect(r.json).toMatchObject({ requested: 2, saved: [], paths: [], errors: ["variant-A.png", "variant-B.png"] }); + expect(r.json.failures.map((f: any) => path.basename(f.file))).toEqual(["variant-A.png", "variant-B.png"]); + }); + + // Value: protects=a partial batch exits 0 and names the failed basename; fails_when=one failure fails the whole round; + // why_new=partial accounting is new; seam=none + test("partial variants batch exits 0", () => { + const outDir = path.join(dir, "designs"); + const r = run(["variants", "--brief", "dashboard", "--count", "2", "--output-dir", outDir], { IMAGES: "ok,fail" }); + expect(r.status).toBe(0); + expect(r.json).toMatchObject({ succeeded: 1, failed: 1, paths: [path.join(outDir, "variant-A.png")], errors: ["variant-B.png"] }); + }); + + // Value: protects=an empty image from the API is a failure, never a 0-byte PNG; fails_when=persistImage writes empty bytes; + // why_new=CEO empty-b64 warning; seam=none + test("empty image bytes exit 2 with no file", () => { + const out = path.join(dir, "empty.png"); + const r = run(["generate", "--brief", "dashboard", "--output", out], { IMAGES: "empty" }); + expect(r.status).toBe(2); + expect(fs.existsSync(out)).toBe(false); + expect(r.json.failures[0].reason).toContain("empty image"); + }); + + // Value: protects=a missing API key still prints the run JSON before exiting non-zero; fails_when=the key check exits the process directly; + // why_new=JSON on every exit path; seam=none + test("missing API key prints JSON and exits 2", () => { + const r = run(["evolve", "--screenshot", path.join(dir, "none.png"), "--brief", "calmer", "--output", path.join(dir, "e.png")], { OPENAI_API_KEY: "" }); + expect(r.status).toBe(2); + expect(r.json.failures[0].reason).toContain("No OpenAI API key found"); + for (const key of [...RUN_FIELDS, "outputPath", "attempts"]) expect(r.json).toHaveProperty(key); + }); +}); + +describe("no re-purchase on a local save failure", () => { + // Value: protects=a write failure after a paid response makes exactly one API call per variant and keeps a recovery copy; + // fails_when=the save sits inside the 429 retry loop again; why_new=Codex DX finding 1; seam=none + test("variants (both branches): one API call, recovery copy, exit 2", () => { + const blocker = path.join(dir, "blocker"); + fs.writeFileSync(blocker, "a file, not a directory"); + for (const extra of [["--count", "1"], ["--viewports", "desktop"]]) { + const before = apiCalls(); + const r = run(["variants", "--brief", "dashboard", ...extra, "--output-dir", path.join(blocker, "designs")]); + expect(r.status).toBe(2); + expect(apiCalls() - before).toBe(1); + expect(r.stderr).not.toContain("Rate limited"); + expect(r.json.recovered).toHaveLength(1); + expect(fs.existsSync(r.json.recovered[0].path)).toBe(true); + expect(r.json.failures[0].reason).toContain("cannot save paid image to"); + } + }); + + // Value: protects=iterate makes one claim per run, and a save failure never triggers the fallback purchase; + // fails_when=the write moves back inside the threaded try; why_new=eng loop restructure; seam=none + test("iterate: threaded failure claims once; save failure buys nothing more", () => { + const gen = run(["generate", "--brief", "dashboard", "--output", path.join(dir, "base.png")]); + const session = gen.json.sessionFile; + const out = path.join(dir, "refined.png"); + + const before = apiCalls(); + const fallback = run(["iterate", "--session", session, "--feedback", "bigger", "--output", out], { IMAGES: "ok,fail,ok" }); + expect(fallback.status, fallback.stderr).toBe(0); + expect(apiCalls() - before).toBe(2); + expect(fallback.json.saved).toEqual([out]); + expect(fs.readdirSync(dir).filter(f => f.startsWith("refined"))).toEqual(["refined.png"]); + + const blocker = path.join(dir, "blocker"); + fs.writeFileSync(blocker, "file"); + const mid = apiCalls(); + const failed = run(["iterate", "--session", session, "--feedback", "bigger", "--output", path.join(blocker, "x.png")]); + expect(failed.status).toBe(2); + expect(apiCalls() - mid).toBe(1); + expect(failed.stderr).not.toContain("Falling back"); + expect(failed.json.recovered).toHaveLength(1); + for (const key of [...RUN_FIELDS, "outputPath", "attempts", "sessionFile", "iteration"]) expect(failed.json).toHaveProperty(key); + }); +}); diff --git a/design/test/gallery.test.ts b/design/test/gallery.test.ts index 7eaebc618..d31230955 100644 --- a/design/test/gallery.test.ts +++ b/design/test/gallery.test.ts @@ -3,7 +3,7 @@ */ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; -import { generateGalleryHtml } from '../src/gallery'; +import { generateGalleryHtml, listVariantImages } from '../src/gallery'; import * as fs from 'fs'; import * as path from 'path'; @@ -137,3 +137,37 @@ describe('Gallery generation', () => { expect(html).toContain('data:image/png;base64,'); }); }); + +describe('Gallery with never-overwrite rounds', () => { + const setMtime = (file: string, seconds: number) => fs.utimesSync(file, seconds, seconds); + + // Value: protects=bumped names (variant-A-2.png) are listed and approved_path marks the exact image, not the letter; + // fails_when=the regex drops -N suffixes or marking falls back to the letter when approved_path exists; why_new=#1529 gallery gap; seam=none + test('lists bumped variants and marks the one approved_path names', () => { + const session = path.join(tmpDir, 'bumped', 'home-20261003'); + fs.mkdirSync(session, { recursive: true }); + createTestPng(path.join(session, 'variant-A.png')); + createTestPng(path.join(session, 'variant-A-2.png')); + fs.writeFileSync(path.join(session, 'approved.json'), JSON.stringify({ approved_variant: 'A', approved_path: 'variant-A-2.png' })); + expect(listVariantImages(session).map(f => path.basename(f)).sort()).toEqual(['variant-A-2.png', 'variant-A.png']); + const html = generateGalleryHtml(path.join(tmpDir, 'bumped')); + expect(html.match(/class="gallery-variant approved"/g)).toHaveLength(1); + expect(html).toMatch(/class="gallery-variant approved">\s*]+alt="Variant A-2"/); + expect(html).toContain('letters do not identify rounds'); + }); + + // Value: protects=history order is mtime then name, and the current board follows in board-images.json order even when files finished out of order; + // fails_when=the gallery sorts by name only or ignores board-images.json; why_new=eng gallery semantics; seam=none + test('orders history by mtime then name, then the board in board-images.json order', () => { + const session = path.join(tmpDir, 'ordered'); + fs.mkdirSync(session, { recursive: true }); + for (const f of ['variant-B.png', 'variant-A.png']) { createTestPng(path.join(session, f)); setMtime(path.join(session, f), 1_000); } + const round2 = ['variant-A-2.png', 'variant-B-2.png', 'variant-C-2.png']; + [3_000, 2_500, 2_000].forEach((t, i) => { createTestPng(path.join(session, round2[i])); setMtime(path.join(session, round2[i]), t); }); + expect(listVariantImages(session).map(f => path.basename(f))).toEqual( + ['variant-A.png', 'variant-B.png', 'variant-C-2.png', 'variant-B-2.png', 'variant-A-2.png']); + fs.writeFileSync(path.join(session, 'board-images.json'), JSON.stringify(round2.map(f => path.join(session, f)))); + expect(listVariantImages(session).map(f => path.basename(f))).toEqual( + ['variant-A.png', 'variant-B.png', 'variant-A-2.png', 'variant-B-2.png', 'variant-C-2.png']); + }); +}); diff --git a/design/test/image-args.test.ts b/design/test/image-args.test.ts new file mode 100644 index 000000000..8be15133b --- /dev/null +++ b/design/test/image-args.test.ts @@ -0,0 +1,52 @@ +/** + * `$D compare` image list input: --images-file and JSON arrays carry any path + * losslessly; comma and glob input keep working. + */ + +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { resolveImagePaths } from "../src/image-args"; + +let dir: string; +beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "design-image-args-")); }); +afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + +// Value: protects=a path containing a comma and a space reaches compare intact via --images-file; fails_when=the file is split on commas; +// why_new=Lossless path handoff is new; seam=none +test("--images-file keeps a comma+space path and builds the board", () => { + const odd = path.join(dir, "round 2, final"); + fs.mkdirSync(odd); + const images = [path.join(odd, "variant-A-2.png"), path.join(odd, "variant-B.png")]; + for (const img of images) fs.writeFileSync(img, Buffer.from("iVBORw0KGgo=", "base64")); + const list = path.join(odd, "board-images.json"); + fs.writeFileSync(list, JSON.stringify(images)); + const board = path.join(odd, "board.html"); + const r = spawnSync(process.execPath, ["--no-env-file", path.join(import.meta.dir, "..", "src", "cli.ts"), + "compare", "--images-file", list, "--output", board], { encoding: "utf8", timeout: 30_000, env: { PATH: process.env.PATH!, HOME: dir } }); + expect(r.status, r.stderr).toBe(0); + const html = fs.readFileSync(board, "utf8"); + expect(html).toContain('data-variant="A"'); + expect(html).toContain('data-variant="B"'); + expect(html).not.toContain('data-variant="C"'); +}); + +// Value: protects=JSON-array --images input and the unchanged comma form; fails_when=a `[`-prefixed literal path is misparsed or comma input changes; +// why_new=new input form; seam=none +test("--images: JSON array only when it parses; otherwise comma and literal behavior is unchanged", async () => { + expect(await resolveImagePaths('["/a, b/x.png","/c.png"]')).toEqual(["/a, b/x.png", "/c.png"]); + expect(await resolveImagePaths("[draft]/x.png")).toEqual(["[draft]/x.png"]); + expect(await resolveImagePaths("/a.png, /b.png")).toEqual(["/a.png", "/b.png"]); + expect(await resolveImagePaths("[1,2]")).toEqual(["[1", "2]"]); +}); + +// Value: protects=a malformed --images-file fails loudly instead of building an empty board; fails_when=parse errors are swallowed; +// why_new=new flag; seam=none +test("--images-file must hold a JSON array of strings", async () => { + const bad = path.join(dir, "bad.json"); + fs.writeFileSync(bad, '{"paths": []}'); + await expect(resolveImagePaths(undefined, bad)).rejects.toThrow("JSON array"); + await expect(resolveImagePaths(undefined, path.join(dir, "none.json"))).rejects.toThrow("cannot read --images-file"); +}); diff --git a/design/test/models.test.ts b/design/test/models.test.ts index a057e632f..57565c230 100644 --- a/design/test/models.test.ts +++ b/design/test/models.test.ts @@ -139,15 +139,19 @@ describe("image-generation call sites (Responses API)", () => { test("generate", async () => { stubFetch((u) => ok(u)); const r = await generate({ brief: "a login page", output: path.join(dir, "g.png") }); - fs.rmSync(r.sessionFile, { force: true }); + fs.rmSync(r.sessionFile!, { force: true }); expect(calls).toHaveLength(1); expectImageShape(calls[0]); stubFetch(() => rejected()); - await expect(generate({ brief: "x", output: path.join(dir, "g2.png") })).rejects.toThrow(`${DESIGN_MODEL_ENV}=${OVERRIDE}`); + const failed = await generate({ brief: "x", output: path.join(dir, "g2.png") }); + expect(failed.exitCode).toBe(2); + names(failed.failures[0].reason); // An override that cannot drive gpt-image-2 fails before any request. calls = []; process.env[DESIGN_MODEL_ENV] = "gpt-4o"; - await expect(generate({ brief: "x", output: path.join(dir, "g3.png") })).rejects.toThrow(DESIGN_MODEL_ENV); + const refused = await generate({ brief: "x", output: path.join(dir, "g3.png") }); + expect(refused.exitCode).toBe(2); + expect(refused.failures[0].reason).toContain(DESIGN_MODEL_ENV); expect(calls).toHaveLength(0); }); @@ -163,7 +167,7 @@ describe("image-generation call sites (Responses API)", () => { test("iterate: threaded and fresh fallback", async () => { const id = `models-test-${process.pid}-${Date.now()}`; - const session = path.join("/tmp", `design-session-${id}.json`); + const session = path.join(os.tmpdir(), `design-session-${id}.json`); const write = () => fs.writeFileSync(session, JSON.stringify({ id, lastResponseId: "resp_prev", originalBrief: "b", feedbackHistory: [], outputPaths: [], createdAt: "", updatedAt: "" })); try { write(); @@ -177,7 +181,8 @@ describe("image-generation call sites (Responses API)", () => { write(); errors = []; stubFetch(() => rejected()); - await expect(iterate({ session, feedback: "x", output: path.join(dir, "i2.png") })).rejects.toThrow(`${DESIGN_MODEL_ENV}=${OVERRIDE}`); + expect(await iterate({ session, feedback: "x", output: path.join(dir, "i2.png") })).toBe(2); + names(errors.at(-1)!); names(errors.find((e) => e.includes("Threading failed"))!); } finally { fs.rmSync(session, { force: true }); @@ -192,7 +197,8 @@ describe("image-generation call sites (Responses API)", () => { expectImageShape(calls[1]); expect(calls[1].body.input).toContain("a centered card"); stubFetch(() => rejected()); - await expect(evolve({ screenshot: path.join(dir, "shot.png"), brief: "x", output: path.join(dir, "e2.png") })).rejects.toThrow(`${DESIGN_MODEL_ENV}=${OVERRIDE}`); + expect(await evolve({ screenshot: path.join(dir, "shot.png"), brief: "x", output: path.join(dir, "e2.png") })).toBe(2); + names(errors.at(-1)!); names(errors.find((e) => e.includes("Screenshot analysis failed"))!); }); }); diff --git a/design/test/persist.test.ts b/design/test/persist.test.ts new file mode 100644 index 000000000..9cf2c6867 --- /dev/null +++ b/design/test/persist.test.ts @@ -0,0 +1,165 @@ +/** + * persistImage / claimOutputPath: a paid image is never overwritten and never lost. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { CLAIM_CAP, bumpedName, claimOutputPath, persistImage, type PersistFs } from "../src/persist"; + +const PNG = Buffer.from("fake paid image bytes"); +const B64 = PNG.toString("base64"); + +let dir: string; +let tmp: string; +let stderr: string[]; +const savedError = console.error; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "design-persist-")); + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "design-persist-tmp-")); + stderr = []; + console.error = (...a: unknown[]) => { stderr.push(a.map(String).join(" ")); }; +}); + +afterEach(() => { + console.error = savedError; + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +function failingFs(fail: { op: keyof PersistFs; code: string; when?: (p: unknown) => boolean }): PersistFs { + const wrapped: any = {}; + for (const op of ["mkdirSync", "openSync", "writeSync", "fsyncSync", "closeSync", "unlinkSync"] as const) { + wrapped[op] = (...args: any[]) => { + if (op === fail.op && (!fail.when || fail.when(args[0]))) { + throw Object.assign(new Error(`${fail.code}: simulated ${op} failure`), { code: fail.code }); + } + return (fs as any)[op](...args); + }; + } + return wrapped; +} + +describe("claimOutputPath", () => { + // Value: protects=an existing image is never truncated and the next free -N name is claimed; + // fails_when=the claim opens with "w" instead of "wx" or the bump rule changes; why_new=no claim existed before; seam=none + test("keeps existing files and bumps sequentially", () => { + const requested = path.join(dir, "x.png"); + fs.writeFileSync(requested, "round one"); + const second = claimOutputPath(requested); + fs.closeSync(second.fd); + const third = claimOutputPath(requested); + fs.closeSync(third.fd); + expect([second.path, third.path]).toEqual([path.join(dir, "x-2.png"), path.join(dir, "x-3.png")]); + expect(fs.readFileSync(requested, "utf8")).toBe("round one"); + expect(bumpedName(path.join(dir, "x-2.png"), 2)).toBe(path.join(dir, "x-2-2.png")); + }); + + // Value: protects=parallel writers (variants, shotgun subagents) each get their own file; + // fails_when=the claim is check-then-write instead of an exclusive create; why_new=concurrency was untested; seam=none + test("N concurrent processes claim N distinct files", async () => { + const requested = path.join(dir, "variant-A.png"); + const script = `import { claimOutputPath } from ${JSON.stringify(path.join(import.meta.dir, "../src/persist.ts"))}; +const c = claimOutputPath(process.argv[2]); console.log(c.path);`; + const scriptPath = path.join(tmp, "claim.ts"); + fs.writeFileSync(scriptPath, script); + const procs = Array.from({ length: 8 }, () => + Bun.spawn([process.execPath, scriptPath, requested], { stdout: "pipe", stderr: "pipe" })); + const outs = await Promise.all(procs.map(p => new Response(p.stdout).text())); + await Promise.all(procs.map(p => p.exited)); + const claimed = outs.map(o => o.trim()); + expect(new Set(claimed).size).toBe(8); + expect(fs.readdirSync(dir).length).toBe(8); + }); + + // Value: protects=a permission error fails at once instead of walking 999 names; fails_when=every open error is treated as EEXIST; + // why_new=new behavior; seam=PersistFs fault injection + test("non-EEXIST open errors do not bump", () => { + let opens = 0; + const fsImpl = failingFs({ op: "openSync", code: "EACCES", when: () => { opens++; return true; } }); + expect(() => claimOutputPath(path.join(dir, "x.png"), fsImpl)).toThrow("EACCES"); + expect(opens).toBe(1); + }); + + // Value: protects=the claim stops at -999 with an error; fails_when=the cap is removed (unbounded loop) or off by one; + // why_new=new behavior; seam=PersistFs fault injection + test("stops at the -999 cap", () => { + const fsImpl = failingFs({ op: "openSync", code: "EEXIST" }); + expect(() => claimOutputPath(path.join(dir, "x.png"), fsImpl)).toThrow(`x-${CLAIM_CAP}.png already exist`); + }); +}); + +describe("persistImage", () => { + // Value: protects=the exact bump notice and actual-path reporting scripts rely on; fails_when=the notice text or returned path drifts; + // why_new=bumping is new; seam=none + test("bumps a taken name, prints the notice and leaves the old bytes untouched", () => { + const requested = path.join(dir, "nested", "x.png"); + fs.mkdirSync(path.dirname(requested)); + fs.writeFileSync(requested, "old"); + const outcome = persistImage(B64, requested, { tmpdir: tmp }); + const actual = path.join(dir, "nested", "x-2.png"); + expect(outcome).toEqual({ ok: true, path: actual, bytes: PNG.length }); + expect(stderr).toContain(`note: ${requested} exists; saved to ${actual} (existing file kept)`); + expect(fs.readFileSync(requested, "utf8")).toBe("old"); + expect(fs.readFileSync(actual)).toEqual(PNG); + }); + + // Value: protects=an empty API image is a failure, not a 0-byte PNG; fails_when=the empty-bytes guard is removed; + // why_new=closes the CEO empty-b64 warning; seam=none + test("empty image data is a failure with no claim and no file", () => { + const outcome = persistImage("", path.join(dir, "x.png"), { tmpdir: tmp }); + expect(outcome.ok).toBe(false); + expect(fs.readdirSync(dir)).toEqual([]); + expect(fs.readdirSync(tmp)).toEqual([]); + }); + + // Value: protects=a mid-write ENOSPC removes the partial claimed file and keeps a 0600 recovery copy; fails_when=cleanup or recovery is skipped; + // why_new=new behavior; seam=PersistFs fault injection + test("post-claim write failure removes the claimed file and saves a private recovery copy", () => { + const requested = path.join(dir, "x.png"); + let writes = 0; + const fsImpl = failingFs({ op: "writeSync", code: "ENOSPC", when: () => writes++ === 0 }); + const outcome = persistImage(B64, requested, { tmpdir: tmp, fs: fsImpl }); + expect(outcome.ok).toBe(false); + if (outcome.ok) return; + expect(fs.existsSync(requested)).toBe(false); + const recovery = outcome.recovered!.path; + expect(path.dirname(recovery)).toBe(tmp); + expect(path.basename(recovery)).toMatch(/^gstack-design-unsaved-.*\.png$/); + expect(fs.readFileSync(recovery)).toEqual(PNG); + expect(fs.statSync(recovery).mode & 0o777).toBe(0o600); + expect(outcome.failure.file).toBe(requested); + expect(outcome.failure.reason).toContain(`cannot save paid image to ${requested}: ENOSPC`); + expect(outcome.failure.reason).toContain(`saved a recovery copy to ${recovery}`); + expect(outcome.failure.reason).toContain("Fix: free disk space"); + }); + + // Value: protects=a mkdir/claim failure (EACCES) still recovers the bytes with problem, cause and fix; fails_when=recovery only covers write errors; + // why_new=new behavior; seam=PersistFs fault injection + test("EACCES on the claim recovers the bytes and names the fix", () => { + const requested = path.join(dir, "x.png"); + const fsImpl = failingFs({ op: "openSync", code: "EACCES", when: p => p === requested }); + const outcome = persistImage(B64, requested, { tmpdir: tmp, fs: fsImpl }); + expect(outcome.ok).toBe(false); + if (outcome.ok) return; + expect(outcome.failure.reason).toMatch(/cannot save paid image to .*x\.png: EACCES \(.*\)\. Image bytes were received; saved a recovery copy to .*\. Fix: make .* writable/); + expect(fs.readFileSync(outcome.recovered!.path)).toEqual(PNG); + }); + + // Value: protects=when the recovery copy also fails both errors and the byte count are reported and no recovery is claimed; + // fails_when=the message claims a recovery copy that does not exist; why_new=new behavior; seam=PersistFs fault injection + test("recovery failure reports both errors and the byte count", () => { + const requested = path.join(dir, "x.png"); + const fsImpl = failingFs({ op: "openSync", code: "EROFS" }); + const outcome = persistImage(B64, requested, { tmpdir: tmp, fs: fsImpl }); + expect(outcome.ok).toBe(false); + if (outcome.ok) return; + expect(outcome.recovered).toBeUndefined(); + expect(outcome.failure.reason).toContain(`(${PNG.length} bytes)`); + expect(outcome.failure.reason).toContain("recovery copy"); + expect(outcome.failure.reason.match(/EROFS/g)!.length).toBeGreaterThanOrEqual(2); + expect(outcome.failure.reason).not.toContain("saved a recovery copy"); + }); +}); diff --git a/office-hours/SKILL.md b/office-hours/SKILL.md index 3e1a87541..d62225055 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -959,18 +959,26 @@ explore wide across diverse directions. **Step 3: Generate 3 variants** ```bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` This generates 3 style variations of the same brief (~40 seconds total). + +**Round accounting (before any board, check or inline view):** `$D` never overwrites; a taken name is bumped (for example `-2`), so use only the paths its JSON printed and carry them as literal paths into later blocks (bash blocks do not share variables). Tell the user how many of `requested` paid images were saved (`saved`) and name each `failures` entry. Route on the exit code: 0 continue with `saved`; 3 continue with `saved` and say the run stopped early; 2 nothing was saved, so report `failures` and stop: do not run `$D compare` or `$D check`. + **Step 4: Show variants inline, then open comparison board** -Show each variant to the user inline first (read the PNGs with Read tool), then +Show each saved image to the user inline first (read the printed paths with Read tool), then create and serve the comparison board: + +Write this round's board images (printed paths that passed checks, in order) as a JSON array to `$_DESIGN_DIR/board-images.json` with the Write tool; board letters A, B, C follow that order. Then archive any earlier Submit so it cannot approve these images, and build the board: + ```bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve +[ -f "$_DESIGN_DIR/feedback.json" ] && mv "$_DESIGN_DIR/feedback.json" "$_DESIGN_DIR/feedback-$(date -u +%Y%m%dT%H%M%SZ).json" +$D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html" --serve ``` This publishes the board to the design daemon, opens it in the user's default @@ -987,8 +995,8 @@ with Read and fall back to AskUserQuestion: "Which variant do you prefer? Any fe If the JSON contains `"regenerated": true`: 1. Read `regenerateAction` (or `remixSpec` for remix requests) -2. Generate new variants with `$D iterate` or `$D variants` using updated brief -3. Create new board with `$D compare` +2. Generate new variants with `$D iterate` or `$D variants` using updated brief (capture and round accounting as in Step 3) +3. Rebuild the board with Step 4's board block, without `--serve` 4. POST the new HTML to the running board. Parse the board URL from stderr (`BOARD_URL: http://127.0.0.1:N/boards//` — the daemon path) or fall back to the legacy port (`SERVE_STARTED: port=N` — only emitted under @@ -1000,11 +1008,19 @@ If `"regenerated": false`: proceed with the approved variant. **Step 6: Save approved choice** +Map the confirmed letter through this board's `board-images.json` (never the directory listing) and save it: + ```bash -echo '{"approved_variant":"","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"mockup","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" +_IMG=$(jq -r --arg v "" '.[($v | explode[0]) - 65] // empty' "$_DESIGN_DIR/board-images.json") +if [ -n "$_IMG" ]; then + echo '{"approved_variant":"","approved_path":"'"$(basename "$_IMG")"'","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"mockup","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" + echo "APPROVED_IMAGE: $_IMG" +else + echo "NO_BOARD_IMAGE: is not on this board; reselect from the board" +fi ``` -Reference the saved mockup in the design doc or plan. +Reference the printed `APPROVED_IMAGE` in the design doc or plan. ## Visual Sketch (UI ideas only) diff --git a/package.json b/package.json index 1132252e6..8c7f0e4ef 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.16", + "version": "1.91.17", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index a430512e2..01a9faa0e 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -654,13 +654,15 @@ Comparison boards are local HTML files: open them with `open file://...` on macO If `DESIGN_READY`: the design binary is available for visual mockup generation. Commands: -- `$D generate --brief "..." --output /path.png` — generate a single mockup -- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants -- `$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve` — comparison board + HTTP server +- `$D generate --brief "..." --output /path.png` — generate a single mockup (prints `outputPath`) +- `$D variants --brief "..." --count 3 --output-dir /path/` — generate N style variants (prints `paths`) +- `$D compare --images-file /path/board-images.json --output /path/board.html --serve` — comparison board + HTTP server - `$D serve --html /path/board.html` — serve comparison board and collect feedback via HTTP - `$D check --image /path.png --brief "..."` — vision quality gate - `$D iterate --session /path/session.json --feedback "..." --output /path.png` — iterate +Image commands never overwrite (a taken name gets `-2`) and always print JSON (`requested`, `saved`, `failures`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without `set -e`: `_OUT=$($D ...); _RC=$?`. + **Path rule:** Design artifacts belong in `$GSTACK_STATE_ROOT/projects/$SLUG/designs/`. Use `bin/gstack-paths` (docs/state-root.md). Keep it even if temporary; never substitute .context/, docs/designs/ or another directory. @@ -758,6 +760,7 @@ planning phase. Generating mockups during planning is the whole point. Allowed commands under this exception: - `mkdir -p "$GSTACK_STATE_ROOT/projects/$SLUG/designs/..."` - `$D generate`, `$D variants`, `$D compare`, `$D iterate`, `$D evolve`, `$D check` +- Writing `board-images.json` and `approved.json`, and archiving `feedback.json`, inside `$_DESIGN_DIR` - `open` (viewing comparison boards in the default browser) First, set up the output directory. Name it after the screen/feature being designed and today's date: @@ -778,13 +781,17 @@ fewer variants and benefits from sequential control; /design-shotgun is the para For each UI screen/section in scope, construct a design brief from the plan's description (and DESIGN.md if present) and generate variants: ```bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -After generation, run a cross-model quality check on each variant: + +**Round accounting (before any check or board):** `$D` never overwrites, so a taken name is bumped (for example `-2`); use only the printed `saved` paths. Tell the user how many of `requested` paid images were saved and name each `failures` entry. Exit 0: continue with `saved`. Exit 2: nothing was saved; report `failures` and stop: no `$D check`, no board. + +Then run a cross-model quality check on each saved path, starting with the first: ```bash -$D check --image "$_DESIGN_DIR/variant-A.png" --brief "" +$D check --image "" --brief "" ``` Flag any variants that fail the quality check. Offer to regenerate failures. @@ -798,8 +805,12 @@ feedback output. Showing mockups inline is a degraded experience. Create the comparison board and serve it over HTTP: + +Write this round's board images (printed paths that passed checks, in order) as a JSON array to `$_DESIGN_DIR/board-images.json` with the Write tool; board letters A, B, C follow that order. Then archive any earlier Submit so it cannot approve these images, and build the board: + ```bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve +[ -f "$_DESIGN_DIR/feedback.json" ] && mv "$_DESIGN_DIR/feedback.json" "$_DESIGN_DIR/feedback-$(date -u +%Y%m%dT%H%M%SZ).json" +$D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html" --serve ``` Creates HTML and opens the board. **Run it in the background** (host task, or `&` redirecting stdout/stderr to private files in `$_DESIGN_DIR`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. @@ -858,8 +869,8 @@ the approved variant. 1. Read `regenerateAction` from the JSON (`"different"`, `"match"`, `"more_like_B"`, `"remix"`, or custom text) 2. If `regenerateAction` is `"remix"`, read `remixSpec` (e.g. `{"layout":"A","colors":"B"}`) -3. Generate new variants with `$D iterate` or `$D variants` using updated brief -4. Create new board: `$D compare --images "..." --output "$_DESIGN_DIR/design-board.html"` +3. Generate new variants with `$D iterate` or `$D variants` using updated brief (capture the JSON and do round accounting as for the first round) +4. Rebuild with the board block above (it archives feedback.json and rewrites board-images.json), without `--serve` 5. Reload the board in the user's browser (same tab) — the URL is per-board under daemon mode, so use `` (from the `BOARD_URL:` stderr line) as the base: @@ -893,9 +904,16 @@ Is this right?" Use AskUserQuestion to verify before proceeding. -**Save the approved choice:** +**Save the approved choice.** Map the confirmed letter through this board's `board-images.json` (never the directory listing) and save it: + ```bash -echo '{"approved_variant":"","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" +_IMG=$(jq -r --arg v "" '.[($v | explode[0]) - 65] // empty' "$_DESIGN_DIR/board-images.json") +if [ -n "$_IMG" ]; then + echo '{"approved_variant":"","approved_path":"'"$(basename "$_IMG")"'","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" + echo "APPROVED_IMAGE: $_IMG" +else + echo "NO_BOARD_IMAGE: is not on this board; reselect from the board" +fi ``` **Do NOT use AskUserQuestion to ask which variant the user picked.** Read `feedback.json` — it already contains their preferred variant, ratings, comments, and overall feedback. Only use AskUserQuestion to confirm you understood the feedback correctly, never to re-ask what they chose. @@ -1105,11 +1123,11 @@ mkdir -p "$_DESIGN_DIR" _ROOT=$(git rev-parse --show-toplevel 2>/dev/null) D="$_ROOT/.claude/skills/gstack/design/dist/design" [ -x "$D" ] || D=~/.claude/skills/gstack/design/dist/design -"$D" generate --brief "" --output "$_DESIGN_DIR/ideal-.png" -echo "IDEAL_IMAGE: $_DESIGN_DIR/ideal-.png" +_OUT=$("$D" generate --brief "" --output "$_DESIGN_DIR/ideal-.png"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -Show the mockup to the user via the Read tool. This makes the gap between +Exit 0: show the image at the printed `outputPath` (it may be bumped, never overwritten) to the user via the Read tool. Exit 2 or 3: report `failures` and continue text-only. This makes the gap between "what the plan describes" and "what it should look like" visceral, not abstract. If the design binary is not available, skip this and continue with text-based diff --git a/plan-design-review/SKILL.md.tmpl b/plan-design-review/SKILL.md.tmpl index 023217a56..61f9c94c4 100644 --- a/plan-design-review/SKILL.md.tmpl +++ b/plan-design-review/SKILL.md.tmpl @@ -219,6 +219,7 @@ planning phase. Generating mockups during planning is the whole point. Allowed commands under this exception: - `mkdir -p "$GSTACK_STATE_ROOT/projects/$SLUG/designs/..."` - `$D generate`, `$D variants`, `$D compare`, `$D iterate`, `$D evolve`, `$D check` +- Writing `board-images.json` and `approved.json`, and archiving `feedback.json`, inside `$_DESIGN_DIR` - `open` (viewing comparison boards in the default browser) First, set up the output directory. Name it after the screen/feature being designed and today's date: @@ -239,13 +240,17 @@ fewer variants and benefits from sequential control; /design-shotgun is the para For each UI screen/section in scope, construct a design brief from the plan's description (and DESIGN.md if present) and generate variants: ```bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -After generation, run a cross-model quality check on each variant: + +**Round accounting (before any check or board):** `$D` never overwrites, so a taken name is bumped (for example `-2`); use only the printed `saved` paths. Tell the user how many of `requested` paid images were saved and name each `failures` entry. Exit 0: continue with `saved`. Exit 2: nothing was saved; report `failures` and stop: no `$D check`, no board. + +Then run a cross-model quality check on each saved path, starting with the first: ```bash -$D check --image "$_DESIGN_DIR/variant-A.png" --brief "" +$D check --image "" --brief "" ``` Flag any variants that fail the quality check. Offer to regenerate failures. @@ -303,11 +308,11 @@ mkdir -p "$_DESIGN_DIR" _ROOT=$(git rev-parse --show-toplevel 2>/dev/null) D="$_ROOT/.claude/skills/gstack/design/dist/design" [ -x "$D" ] || D=~/.claude/skills/gstack/design/dist/design -"$D" generate --brief "" --output "$_DESIGN_DIR/ideal-.png" -echo "IDEAL_IMAGE: $_DESIGN_DIR/ideal-.png" +_OUT=$("$D" generate --brief "" --output "$_DESIGN_DIR/ideal-.png"); _RC=$? +printf '%s\n' "$_OUT"; echo "EXIT: $_RC" ``` -Show the mockup to the user via the Read tool. This makes the gap between +Exit 0: show the image at the printed `outputPath` (it may be bumped, never overwritten) to the user via the Read tool. Exit 2 or 3: report `failures` and continue text-only. This makes the gap between "what the plan describes" and "what it should look like" visceral, not abstract. If the design binary is not available, skip this and continue with text-based diff --git a/plan-design-review/sections/review-sections.md b/plan-design-review/sections/review-sections.md index 67c4465bb..86525df56 100644 --- a/plan-design-review/sections/review-sections.md +++ b/plan-design-review/sections/review-sections.md @@ -216,7 +216,7 @@ FIX TO 10: Rewrite vague UI descriptions with specific alternatives: - "Hero section" → what makes this hero feel like THIS product? - "Clean, modern UI" → meaningless. Replace with actual design decisions. - "Dashboard with widgets" → what makes this NOT every other dashboard? -If visual mockups were generated in Step 0.5, evaluate them against the AI slop blacklist above. Read each mockup image using the Read tool. Does the mockup fall into generic patterns (3-column grid, centered hero, stock-photo feel)? If so, flag it and offer to regenerate with more specific direction via `$D iterate --feedback "..."`. +If visual mockups were generated in Step 0.5, evaluate them against the AI slop blacklist above. Read each mockup image using the Read tool. Does the mockup fall into generic patterns (3-column grid, centered hero, stock-photo feel)? If so, flag it and offer to regenerate with more specific direction via `$D iterate --feedback "..."`; use its printed `outputPath` for any later view, check or board. **STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY. ### Pass 5: Design System Alignment @@ -257,7 +257,7 @@ If mockups were generated in Step 0.5 and review passes changed significant desi AskUserQuestion: "The review passes changed [list major design changes]. Want me to regenerate mockups to reflect the updated plan? This ensures the visual reference matches what we're actually building." -If yes, use `$D iterate` with feedback summarizing the changes, or `$D variants` with an updated brief. Save to the same `$_DESIGN_DIR` directory. +If yes, use `$D iterate` with feedback summarizing the changes, or `$D variants` with an updated brief. Save to the same `$_DESIGN_DIR` directory; nothing is overwritten, so do round accounting and carry the printed paths into any new board (rebuild it with the board block, which rewrites `board-images.json`) or check. ## How to ask questions Follow the AskUserQuestion format from the Preamble above. Additional rules for plan design reviews: diff --git a/plan-design-review/sections/review-sections.md.tmpl b/plan-design-review/sections/review-sections.md.tmpl index 32c06a75a..ee1627124 100644 --- a/plan-design-review/sections/review-sections.md.tmpl +++ b/plan-design-review/sections/review-sections.md.tmpl @@ -79,7 +79,7 @@ FIX TO 10: Rewrite vague UI descriptions with specific alternatives: - "Hero section" → what makes this hero feel like THIS product? - "Clean, modern UI" → meaningless. Replace with actual design decisions. - "Dashboard with widgets" → what makes this NOT every other dashboard? -If visual mockups were generated in Step 0.5, evaluate them against the AI slop blacklist above. Read each mockup image using the Read tool. Does the mockup fall into generic patterns (3-column grid, centered hero, stock-photo feel)? If so, flag it and offer to regenerate with more specific direction via `$D iterate --feedback "..."`. +If visual mockups were generated in Step 0.5, evaluate them against the AI slop blacklist above. Read each mockup image using the Read tool. Does the mockup fall into generic patterns (3-column grid, centered hero, stock-photo feel)? If so, flag it and offer to regenerate with more specific direction via `$D iterate --feedback "..."`; use its printed `outputPath` for any later view, check or board. **STOP.** AskUserQuestion once per issue. Do NOT batch. Recommend + WHY. ### Pass 5: Design System Alignment @@ -120,7 +120,7 @@ If mockups were generated in Step 0.5 and review passes changed significant desi AskUserQuestion: "The review passes changed [list major design changes]. Want me to regenerate mockups to reflect the updated plan? This ensures the visual reference matches what we're actually building." -If yes, use `$D iterate` with feedback summarizing the changes, or `$D variants` with an updated brief. Save to the same `$_DESIGN_DIR` directory. +If yes, use `$D iterate` with feedback summarizing the changes, or `$D variants` with an updated brief. Save to the same `$_DESIGN_DIR` directory; nothing is overwritten, so do round accounting and carry the printed paths into any new board (rebuild it with the board block, which rewrites `board-images.json`) or check. ## How to ask questions Follow the AskUserQuestion format from the Preamble above. Additional rules for plan design reviews: diff --git a/scripts/resolvers/design.ts b/scripts/resolvers/design.ts index 075aa3d82..0f74c918c 100644 --- a/scripts/resolvers/design.ts +++ b/scripts/resolvers/design.ts @@ -1114,6 +1114,33 @@ ${slopSection} Source: [OpenAI "Designing Delightful Frontends with GPT-5.4"](https://developers.openai.com/blog/designing-delightful-frontends-with-gpt-5-4) (Mar 2026) + gstack design methodology.`; } +const DESIGN_ROUND_ACCOUNTING = ` +**Round accounting (before any board, check or inline view):** \`$D\` never overwrites; a taken name is bumped (for example \`-2\`), so use only the paths its JSON printed and carry them as literal paths into later blocks (bash blocks do not share variables). Tell the user how many of \`requested\` paid images were saved (\`saved\`) and name each \`failures\` entry. Route on the exit code: 0 continue with \`saved\`; 3 continue with \`saved\` and say the run stopped early; 2 nothing was saved, so report \`failures\` and stop: do not run \`$D compare\` or \`$D check\`.`; + +function designBoardBlock(serve: boolean): string { + return ` +Write this round's board images (printed paths that passed checks, in order) as a JSON array to \`$_DESIGN_DIR/board-images.json\` with the Write tool; board letters A, B, C follow that order. Then archive any earlier Submit so it cannot approve these images, and build the board: + +\`\`\`bash +[ -f "$_DESIGN_DIR/feedback.json" ] && mv "$_DESIGN_DIR/feedback.json" "$_DESIGN_DIR/feedback-$(date -u +%Y%m%dT%H%M%SZ).json" +$D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html"${serve ? ' --serve' : ''} +\`\`\``; +} + +function designApprovalBlock(screen: string): string { + return `Map the confirmed letter through this board's \`board-images.json\` (never the directory listing) and save it: + +\`\`\`bash +_IMG=$(jq -r --arg v "" '.[($v | explode[0]) - 65] // empty' "$_DESIGN_DIR/board-images.json") +if [ -n "$_IMG" ]; then + echo '{"approved_variant":"","approved_path":"'"$(basename "$_IMG")"'","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"${screen}","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" + echo "APPROVED_IMAGE: $_IMG" +else + echo "NO_BOARD_IMAGE: is not on this board; reselect from the board" +fi +\`\`\``; +} + export function generateDesignSetup(ctx: TemplateContext): string { return `## DESIGN SETUP (run this check BEFORE any design mockup command) @@ -1140,12 +1167,14 @@ Comparison boards are local HTML files: open them with \`open file://...\` on ma If \`DESIGN_READY\`: the design binary is available for visual mockup generation. Commands: -- \`$D generate --brief "..." --output /path.png\` — generate a single mockup -- \`$D variants --brief "..." --count 3 --output-dir /path/\` — generate N style variants -- \`$D compare --images "a.png,b.png,c.png" --output /path/board.html --serve\` — comparison board + HTTP server +- \`$D generate --brief "..." --output /path.png\` — generate a single mockup (prints \`outputPath\`) +- \`$D variants --brief "..." --count 3 --output-dir /path/\` — generate N style variants (prints \`paths\`) +- \`$D compare --images-file /path/board-images.json --output /path/board.html --serve\` — comparison board + HTTP server - \`$D serve --html /path/board.html\` — serve comparison board and collect feedback via HTTP - \`$D check --image /path.png --brief "..."\` — vision quality gate -- \`$D iterate --session /path/session.json --feedback "..." --output /path.png\` — iterate${ctx.skillName === 'design-consultation' ? ` +- \`$D iterate --session /path/session.json --feedback "..." --output /path.png\` — iterate + +Image commands never overwrite (a taken name gets \`-2\`) and always print JSON (\`requested\`, \`saved\`, \`failures\`); exit 0 ready, 2 nothing saved, 3 stopped after saving some. Capture without \`set -e\`: \`_OUT=$($D ...); _RC=$?\`.${ctx.skillName === 'design-consultation' ? ` - \`$D extract --image /absolute/path.png\` — print tokens and automatically update DESIGN.md in the current Git repository; no read-only flag \`generate\` returns \`sessionFile\`; \`iterate\` requires that existing session. \`variants\` returns \`paths\` but creates no session: regenerate with an updated brief instead.` : ''} @@ -1192,19 +1221,20 @@ explore wide across diverse directions. **Step 3: Generate 3 variants** \`\`\`bash -$D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/" +_OUT=$($D variants --brief "" --count 3 --output-dir "$_DESIGN_DIR/"); _RC=$? +printf '%s\\n' "$_OUT"; echo "EXIT: $_RC" \`\`\` This generates 3 style variations of the same brief (~40 seconds total). +${DESIGN_ROUND_ACCOUNTING} + **Step 4: Show variants inline, then open comparison board** -Show each variant to the user inline first (read the PNGs with Read tool), then +Show each saved image to the user inline first (read the printed paths with Read tool), then create and serve the comparison board: -\`\`\`bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve -\`\`\` +${designBoardBlock(true)} This publishes the board to the design daemon, opens it in the user's default browser, and exits; it does not wait for feedback. Read captured stderr for the @@ -1220,8 +1250,8 @@ with Read and fall back to AskUserQuestion: "Which variant do you prefer? Any fe If the JSON contains \`"regenerated": true\`: 1. Read \`regenerateAction\` (or \`remixSpec\` for remix requests) -2. Generate new variants with \`$D iterate\` or \`$D variants\` using updated brief -3. Create new board with \`$D compare\` +2. Generate new variants with \`$D iterate\` or \`$D variants\` using updated brief (capture and round accounting as in Step 3) +3. Rebuild the board with Step 4's board block, without \`--serve\` 4. POST the new HTML to the running board. Parse the board URL from stderr (\`BOARD_URL: http://127.0.0.1:N/boards//\` — the daemon path) or fall back to the legacy port (\`SERVE_STARTED: port=N\` — only emitted under @@ -1233,21 +1263,15 @@ If \`"regenerated": false\`: proceed with the approved variant. **Step 6: Save approved choice** -\`\`\`bash -echo '{"approved_variant":"","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"mockup","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" -\`\`\` +${designApprovalBlock('mockup')} -Reference the saved mockup in the design doc or plan.`; +Reference the printed \`APPROVED_IMAGE\` in the design doc or plan.`; } export function generateDesignShotgunLoop(ctx: TemplateContext): string { if (ctx.skillName === 'design-consultation') return `### Comparison Board + Feedback Loop -Use the successful, quality-checked paths in this example: - -\`\`\`bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve -\`\`\` +${designBoardBlock(true)} This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with \`BOARD_URL\` means the daemon is serving. Save its full \`http://127.0.0.1:N/boards//\` URL. Only legacy \`--no-daemon\` needs a host background task; \`SERVE_STARTED: port=N\` gives root URL \`http://127.0.0.1:N/\`. @@ -1268,23 +1292,21 @@ After the response, read current feedback next to the board HTML: **Board or chat:** revisions regenerate; a final choice needs summary confirmation; skip goes to Phase 6 without a mockup. Ask if no choice/detail; never infer approval from a missing file. Submit with revision notes is a revision. **Regenerate:** -1. Revise the brief, preserving unrelated constraints. Archive this round's feedback files so old Submit cannot approve new images. -2. Run \`$D variants\` with the new brief (no session). Re-run the quality check and visual self-gate on every new image. -3. Rebuild: \`$D compare --images "" --output "$_DESIGN_DIR/design-board.html"\`, without \`--serve\`. +1. Revise the brief, preserving unrelated constraints. Archive this round's feedback files so old Submit cannot approve new images (the board block does this on rebuild). +2. Run \`$D variants\` with the new brief (no session), with the same capture and round accounting. Re-run the quality check and visual self-gate on every new image (its printed path). +3. Rebuild with the board block above (it rewrites board-images.json), without \`--serve\`. 4. Reload at the saved URL (keep its per-board path; legacy uses root): \`jq -nc --arg html "$_DESIGN_DIR/design-board.html" '{html: $html}' | curl -sS -X POST "\${BOARD_URL}api/reload" -H 'Content-Type: application/json' --data-binary @-\` 5. Check reload succeeded, then AskUserQuestion at the same URL until a final choice, skip or stop. Failed generation/reload uses the fallback, not another wait. **SERVER FALLBACK:** Nonzero exit or no readiness marker: show each variant inline with Read, then AskUserQuestion: "The comparison board server failed to start. Which variant? Any changes?" Route chat feedback as above. -**After receiving feedback (any path):** summarize PREFERRED, RATINGS, YOUR NOTES, DIRECTION; AskUserQuestion "Is this right?" A confirmed final choice permits Write of \`$_DESIGN_DIR/approved.json\` with \`approved_variant\`, \`feedback\`, \`date\` (UTC), \`screen\` (the product page depicted by the chosen mockup), and \`branch\` (the current \`git branch --show-current\` result, empty if detached). Use valid JSON, never shell interpolation. This approves the image only; Q-final gates project writes.`; +**After receiving feedback (any path):** summarize PREFERRED, RATINGS, YOUR NOTES, DIRECTION; AskUserQuestion "Is this right?" A confirmed final choice permits Write of \`$_DESIGN_DIR/approved.json\` with \`approved_variant\`, \`approved_path\` (file name of that letter's entry in this board's \`board-images.json\`, never the directory listing), \`feedback\`, \`date\` (UTC), \`screen\` (the product page depicted by the chosen mockup), and \`branch\` (the current \`git branch --show-current\` result, empty if detached). Use valid JSON, never shell interpolation. This approves the image only; Q-final gates project writes.`; return `### Comparison Board + Feedback Loop Create the comparison board and serve it over HTTP: -\`\`\`bash -$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png,$_DESIGN_DIR/variant-C.png" --output "$_DESIGN_DIR/design-board.html" --serve -\`\`\` +${designBoardBlock(true)} Creates HTML and opens the board. **Run it in the background** (host task, or \`&\` redirecting stdout/stderr to private files in \`$_DESIGN_DIR\`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. @@ -1342,8 +1364,8 @@ the approved variant. 1. Read \`regenerateAction\` from the JSON (\`"different"\`, \`"match"\`, \`"more_like_B"\`, \`"remix"\`, or custom text) 2. If \`regenerateAction\` is \`"remix"\`, read \`remixSpec\` (e.g. \`{"layout":"A","colors":"B"}\`) -3. Generate new variants with \`$D iterate\` or \`$D variants\` using updated brief -4. Create new board: \`$D compare --images "..." --output "$_DESIGN_DIR/design-board.html"\` +3. Generate new variants with \`$D iterate\` or \`$D variants\` using updated brief (capture the JSON and do round accounting as for the first round) +4. Rebuild with the board block above (it archives feedback.json and rewrites board-images.json), without \`--serve\` 5. Reload the board in the user's browser (same tab) — the URL is per-board under daemon mode, so use \`\` (from the \`BOARD_URL:\` stderr line) as the base: @@ -1377,10 +1399,7 @@ Is this right?" Use AskUserQuestion to verify before proceeding. -**Save the approved choice:** -\`\`\`bash -echo '{"approved_variant":"","feedback":"","date":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","screen":"","branch":"'$(git branch --show-current 2>/dev/null)'"}' > "$_DESIGN_DIR/approved.json" -\`\`\``; +**Save the approved choice.** ${designApprovalBlock('')}`; } export function generateTasteProfile(ctx: TemplateContext): string { @@ -1410,7 +1429,7 @@ fonts [top-3], colors [top-3], layouts [top-3], aesthetics [top-3]. Bias generation toward these unless the user explicitly requests a different direction. Also avoid their strong rejections: [top-3 rejected per dimension]." -**Legacy fallback:** Glob \`$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json\` (resolve the root with gstack-paths); Read the five newest. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. +**Legacy fallback:** Glob \`$GSTACK_STATE_ROOT/projects/$SLUG/designs/**/approved.json\` (resolve the root with gstack-paths); Read the five newest. To view an approved image, resolve it with \`${ctx.paths.binDir}/gstack-design-approved \`. Use explicit feedback only, never infer fonts/colors from variant letters. No usable files: continue without a taste profile. **Conflict handling:** If the current user request contradicts a strong persistent signal (e.g., "make it playful" when taste profile strongly prefers minimal), flag diff --git a/setup-deploy/SKILL.md b/setup-deploy/SKILL.md index c5ef439b3..9ba78fe4a 100644 --- a/setup-deploy/SKILL.md +++ b/setup-deploy/SKILL.md @@ -425,7 +425,7 @@ Run the platform detection from the deploy bootstrap: # GitHub Actions deploy workflows for f in $(find .github/workflows -maxdepth 1 \( -name '*.yml' -o -name '*.yaml' \) 2>/dev/null); do - [ -f "$f" ] && grep -qiE "deploy|release|production|staging|cd" "$f" 2>/dev/null && echo "DEPLOY_WORKFLOW:$f" + [ -f "$f" ] && grep -qiE "deploy|release|production|staging" "$f" 2>/dev/null && echo "DEPLOY_WORKFLOW:$f" done # Project type @@ -447,7 +447,7 @@ If `fly.toml` detected: 3. If installed, verify: `fly status --app {app} 2>/dev/null` 4. Infer URL: `https://{app}.fly.dev` 5. Set deploy status command: `fly status --app {app}` -6. Set health check: `https://{app}.fly.dev` (or `/health` if the app has one) +6. Set health check: run `curl -s -o /dev/null -w "%{http_code}" https://{app}.fly.dev/health`; use `https://{app}.fly.dev/health` when it returns 2xx, otherwise `https://{app}.fly.dev` Ask the user to confirm the production URL. Some Fly apps use custom domains. @@ -495,6 +495,7 @@ If deploy workflows detected but no platform config: 1. Read the workflow file to understand what it does 2. Extract the deploy target (if mentioned) 3. Ask the user for the production URL +4. Set deploy status command: `gh run list --workflow {workflow file name} --limit 1 --json headSha,status,conclusion` #### Custom / Manual @@ -523,7 +524,7 @@ Use AskUserQuestion to gather the information: ### Step 4: Write configuration -Before writing, collect fields not already confirmed: merge method (squash/merge/rebase, constrained to methods allowed by repo settings), pre-merge command or none, deploy trigger, and status/health checks. Ask only for missing values, across every platform path. If the project does not deploy, set platform/URL/workflow/status/health/trigger to `none`, retain its CLI/library project type, and skip deploy verification. Show the complete proposed configuration and obtain confirmation. +Before writing, collect fields not already confirmed: merge method (squash/merge/rebase, offering only the methods `gh repo view --json squashMergeAllowed,mergeCommitAllowed,rebaseMergeAllowed` reports as allowed), pre-merge command or none, deploy trigger, and status/health checks. Ask only for missing values, across every platform path. If the project does not deploy, set platform/URL/workflow/status/health/trigger to `none`, retain its CLI/library project type, and skip deploy verification. Show the complete proposed configuration and obtain confirmation. Read CLAUDE.md (or create it). Find and replace the `## Deploy Configuration` section if it exists, or append it at the end. @@ -545,6 +546,8 @@ if it exists, or append it at the end. - Health check: {URL or command} ``` +The hooks block repeats the exact commands /land-and-deploy runs: `Deploy status` is the same value as `Deploy status command`, and `Health check` the same as `Post-deploy health check`. Write each value in both places. + ### Step 5: Verify After writing, verify the configuration works: @@ -556,7 +559,9 @@ curl -sf "{health-check-url}" -o /dev/null -w "%{http_code}" 2>/dev/null || echo 2. If a deploy status command was configured, try it: ```bash -{deploy-status-command} 2>/dev/null | head -5 || echo "COMMAND_FAILED" +_OUT=$({deploy-status-command} 2>&1); _RC=$? +printf '%s\n' "$_OUT" | head -5 +[ "$_RC" -eq 0 ] || echo "COMMAND_FAILED (exit $_RC)" ``` Report results. If anything failed, note it but don't block — the config is still diff --git a/setup-deploy/SKILL.md.tmpl b/setup-deploy/SKILL.md.tmpl index 6ac0d4092..4f3125f81 100644 --- a/setup-deploy/SKILL.md.tmpl +++ b/setup-deploy/SKILL.md.tmpl @@ -68,7 +68,7 @@ Run the platform detection from the deploy bootstrap: # GitHub Actions deploy workflows for f in $(find .github/workflows -maxdepth 1 \( -name '*.yml' -o -name '*.yaml' \) 2>/dev/null); do - [ -f "$f" ] && grep -qiE "deploy|release|production|staging|cd" "$f" 2>/dev/null && echo "DEPLOY_WORKFLOW:$f" + [ -f "$f" ] && grep -qiE "deploy|release|production|staging" "$f" 2>/dev/null && echo "DEPLOY_WORKFLOW:$f" done # Project type @@ -90,7 +90,7 @@ If `fly.toml` detected: 3. If installed, verify: `fly status --app {app} 2>/dev/null` 4. Infer URL: `https://{app}.fly.dev` 5. Set deploy status command: `fly status --app {app}` -6. Set health check: `https://{app}.fly.dev` (or `/health` if the app has one) +6. Set health check: run `curl -s -o /dev/null -w "%{http_code}" https://{app}.fly.dev/health`; use `https://{app}.fly.dev/health` when it returns 2xx, otherwise `https://{app}.fly.dev` Ask the user to confirm the production URL. Some Fly apps use custom domains. @@ -138,6 +138,7 @@ If deploy workflows detected but no platform config: 1. Read the workflow file to understand what it does 2. Extract the deploy target (if mentioned) 3. Ask the user for the production URL +4. Set deploy status command: `gh run list --workflow {workflow file name} --limit 1 --json headSha,status,conclusion` #### Custom / Manual @@ -166,7 +167,7 @@ Use AskUserQuestion to gather the information: ### Step 4: Write configuration -Before writing, collect fields not already confirmed: merge method (squash/merge/rebase, constrained to methods allowed by repo settings), pre-merge command or none, deploy trigger, and status/health checks. Ask only for missing values, across every platform path. If the project does not deploy, set platform/URL/workflow/status/health/trigger to `none`, retain its CLI/library project type, and skip deploy verification. Show the complete proposed configuration and obtain confirmation. +Before writing, collect fields not already confirmed: merge method (squash/merge/rebase, offering only the methods `gh repo view --json squashMergeAllowed,mergeCommitAllowed,rebaseMergeAllowed` reports as allowed), pre-merge command or none, deploy trigger, and status/health checks. Ask only for missing values, across every platform path. If the project does not deploy, set platform/URL/workflow/status/health/trigger to `none`, retain its CLI/library project type, and skip deploy verification. Show the complete proposed configuration and obtain confirmation. Read CLAUDE.md (or create it). Find and replace the `## Deploy Configuration` section if it exists, or append it at the end. @@ -188,6 +189,8 @@ if it exists, or append it at the end. - Health check: {URL or command} ``` +The hooks block repeats the exact commands /land-and-deploy runs: `Deploy status` is the same value as `Deploy status command`, and `Health check` the same as `Post-deploy health check`. Write each value in both places. + ### Step 5: Verify After writing, verify the configuration works: @@ -199,7 +202,9 @@ curl -sf "{health-check-url}" -o /dev/null -w "%{http_code}" 2>/dev/null || echo 2. If a deploy status command was configured, try it: ```bash -{deploy-status-command} 2>/dev/null | head -5 || echo "COMMAND_FAILED" +_OUT=$({deploy-status-command} 2>&1); _RC=$? +printf '%s\n' "$_OUT" | head -5 +[ "$_RC" -eq 0 ] || echo "COMMAND_FAILED (exit $_RC)" ``` Report results. If anything failed, note it but don't block — the config is still diff --git a/test/context-remaining-work-provenance.test.ts b/test/context-remaining-work-provenance.test.ts new file mode 100644 index 000000000..0fc423fe2 --- /dev/null +++ b/test/context-remaining-work-provenance.test.ts @@ -0,0 +1,63 @@ +/** + * Remaining Work provenance (#3004): prompt-byte contract for the shipped + * context-save and context-restore SKILL.md files. + * + * The markers are a wire format between two skills: /context-save writes them + * into a checkpoint and a later /context-restore sorts on them. Only that + * contract is pinned (marker tokens, group headings, banner, option-A order), + * not the surrounding prose. Behavior is covered by the periodic E2E case + * context-restore-provenance-order. + * + * Value: protects=save writes the five provenance markers and restore sorts them into Next steps and Verify first; fails_when=a marker, a group or the saved-order option A is dropped; why_new=no test reads Remaining Work; seam=none + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const MARKERS = ['(path run)', '(path read)', '(path assumed)', '(code read)', '(target state checked)']; + +function read(rel: string): string { + return fs.readFileSync(path.join(ROOT, rel), 'utf-8'); +} + +function between(text: string, start: string, end: string): string { + const from = text.indexOf(start); + expect(from, `missing ${start}`).toBeGreaterThanOrEqual(0); + const to = text.indexOf(end, from + start.length); + expect(to, `missing ${end} after ${start}`).toBeGreaterThan(from); + return text.slice(from, to); +} + +describe('Remaining Work provenance contract', () => { + test('context-save marks every item Open. and defines all five markers, writing steps first', () => { + const save = read('context-save/SKILL.md'); + const table = between(save, '**Remaining Work provenance.**', 'If the user provided a title'); + for (const marker of MARKERS) expect(table).toContain(`\`${marker}\``); + expect(table).toContain('`Open.`'); + expect(table).toContain('`[done]`'); + expect(table.indexOf('a writing step')).toBeLessThan(table.indexOf('names a concrete path')); + const format = between(save, '### Remaining Work', '### Notes'); + expect(format).toContain('`Open.`'); + }); + + test('context-restore sorts into Next steps and Verify first and option A walks saved order', () => { + const restore = read('context-restore/SKILL.md'); + expect(restore).not.toContain('{remaining work items}'); + const rules = between(restore, '**Sort Remaining Work by provenance.**', 'Read the chosen file'); + const verifyFirst = between(rules, 'Put an item under **Verify first**', 'Every other item'); + for (const marker of ['(path assumed)', '(code read)']) expect(verifyFirst).toContain(`\`${marker}\``); + expect(verifyFirst).toContain('reports a failure'); + expect(verifyFirst).toContain('has no marker'); + const nextSteps = between(rules, 'Every other item goes under **Next steps**', 'Verifying means'); + for (const marker of ['(path run)', '(path read)', '(target state checked)']) expect(nextSteps).toContain(`\`${marker}\``); + expect(rules).toContain( + '`This checkpoint predates provenance markers; items naming commands, paths or writes are listed under Verify first.`', + ); + const summary = between(restore, '### Remaining Work', '### Notes'); + expect(summary.indexOf('Next steps')).toBeLessThan(summary.indexOf('Verify first')); + const optionA = between(restore, 'If A,', '## If no saved contexts exist'); + expect(optionA).toContain('first Remaining Work item in saved order'); + expect(optionA).toContain('under Verify first, suggest verifying it'); + }); +}); diff --git a/test/design-approval-flow.test.ts b/test/design-approval-flow.test.ts new file mode 100644 index 000000000..ef6e740d8 --- /dev/null +++ b/test/design-approval-flow.test.ts @@ -0,0 +1,97 @@ +/** + * Stubbed two-round design flow: round two lands at bumped names, the board + * block archives round one's Submit, approval maps the letter through this + * board's board-images.json, and a fresh session (relocated directory) + * resolves the approved image. Runs the generated SKILL.md bash blocks with + * the image API stubbed (no spend). + */ + +import { afterEach, beforeEach, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SKILL = fs.readFileSync(path.join(ROOT, 'plan-design-review/SKILL.md'), 'utf8'); + +function bashBlockAfter(marker: string): string { + const at = SKILL.indexOf(marker); + expect(at).toBeGreaterThan(-1); + return SKILL.slice(at).match(/```bash\n([\s\S]*?)```/)![1]; +} + +const BOARD = bashBlockAfter(''); +const APPROVAL = SKILL.slice(SKILL.indexOf('**Save the approved choice.**')).match(/```bash\n([\s\S]*?)```/)![1]; + +let dir: string; +let designDir: string; +let env: Record; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'design-approval-flow-')); + designDir = path.join(dir, 'state', 'designs', 'home-20261003'); + const preload = path.join(dir, 'stub.ts'); + fs.writeFileSync(preload, `let n = 0; +globalThis.fetch = (async () => Response.json({ id: "r", output: [{ type: "image_generation_call", result: Buffer.from("round image " + (n++)).toString("base64") }] })) as typeof fetch; +`); + const design = path.join(dir, 'design'); + fs.writeFileSync(design, `#!/bin/sh +if [ "$1" = compare ]; then printf '%s\\n' "$*" > "$COMPARE_ARGS"; exit 0; fi +exec ${JSON.stringify(process.execPath)} --no-env-file --preload ${JSON.stringify(preload)} ${JSON.stringify(path.join(ROOT, 'design/src/cli.ts'))} "$@" +`, { mode: 0o755 }); + env = { + PATH: process.env.PATH!, HOME: dir, GSTACK_HOME: path.join(dir, 'gstack'), TMPDIR: dir, + OPENAI_API_KEY: 'fixture-not-a-real-key', D: design, _DESIGN_DIR: designDir, COMPARE_ARGS: path.join(dir, 'compare-args'), + }; +}); + +afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + +const sh = (script: string) => spawnSync('bash', ['-c', script], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }); +const variants = () => JSON.parse(sh('"$D" variants --brief "home page" --count 2 --output-dir "$_DESIGN_DIR/"').stdout); +const approve = (letter: string) => sh(APPROVAL.replaceAll('', letter).replaceAll('', 'calmer').replaceAll('', 'home')); + +// Value: protects=round two's approval comes from round two's board, never round one's stale feedback.json or the newest-file listing; +// fails_when=the board block stops archiving feedback.json or approval maps letters through the directory; why_new=eng feedback-binding; seam=none +test('round two board archives round one Submit and approval maps to the bumped image', () => { + const round1 = variants(); + fs.writeFileSync(path.join(designDir, 'board-images.json'), JSON.stringify(round1.paths)); + fs.writeFileSync(path.join(designDir, 'feedback.json'), JSON.stringify({ preferred: 'B', regenerated: false })); + + const round2 = variants(); + expect(round2.paths.map((p: string) => path.basename(p))).toEqual(['variant-A-2.png', 'variant-B-2.png']); + fs.writeFileSync(path.join(designDir, 'board-images.json'), JSON.stringify(round2.paths)); + const board = sh(BOARD); + expect(board.status, board.stderr).toBe(0); + expect(fs.existsSync(path.join(designDir, 'feedback.json'))).toBe(false); + const archived = fs.readdirSync(designDir).filter(f => /^feedback-\d{8}T\d{6}Z\.json$/.test(f)); + expect(archived).toHaveLength(1); + expect(fs.readFileSync(env.COMPARE_ARGS, 'utf8')).toContain(`--images-file ${path.join(designDir, 'board-images.json')}`); + + const approved = approve('A'); + expect(approved.status, approved.stderr).toBe(0); + expect(approved.stdout).toContain(`APPROVED_IMAGE: ${path.join(designDir, 'variant-A-2.png')}`); + const record = JSON.parse(fs.readFileSync(path.join(designDir, 'approved.json'), 'utf8')); + expect(record).toMatchObject({ approved_variant: 'A', approved_path: 'variant-A-2.png', screen: 'home', feedback: 'calmer' }); + + const missing = approve('C'); + expect(missing.stdout).toContain('NO_BOARD_IMAGE'); + expect(JSON.parse(fs.readFileSync(path.join(designDir, 'approved.json'), 'utf8')).approved_path).toBe('variant-A-2.png'); +}); + +// Value: protects=a fresh session in a relocated state root opens round two's approved image, not variant-A.png; +// fails_when=approved_path is stored absolute or readers resolve by letter; why_new=DX relocation + CEO fresh-session recovery; seam=none +test('fresh session resolves the approved bumped image after relocation', () => { + variants(); + const round2 = variants(); + fs.writeFileSync(path.join(designDir, 'board-images.json'), JSON.stringify(round2.paths)); + expect(approve('A').status).toBe(0); + const moved = path.join(dir, 'moved', 'home-20261003'); + fs.cpSync(designDir, moved, { recursive: true }); + fs.rmSync(path.join(dir, 'state'), { recursive: true }); + const r = spawnSync(process.execPath, [path.join(ROOT, 'bin/gstack-design-approved'), path.join(moved, 'approved.json')], { encoding: 'utf8', timeout: 30_000 }); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout.trim()).toBe(path.join(moved, 'variant-A-2.png')); + expect(fs.readFileSync(r.stdout.trim(), 'utf8')).toBe('round image 0'); +}); diff --git a/test/design-printed-paths.test.ts b/test/design-printed-paths.test.ts new file mode 100644 index 000000000..4546d2188 --- /dev/null +++ b/test/design-printed-paths.test.ts @@ -0,0 +1,165 @@ +/** + * Tripwire: design skills read the paths `$D` printed for this round, never a + * fixed variant name or a directory glob, because `$D` never overwrites and a + * second round lands at bumped names (variant-A-2.png). Runs on the generated + * SKILL.md files (with their sections) and the resolver sources. + */ + +import { describe, expect, test } from 'bun:test'; +import fs from 'node:fs'; +import path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '..'); + +/** The one reasoned exception, anchored on content rather than a line number. */ +const ALLOWLIST = [{ + file: 'design-html/SKILL.md', + line: '_VARIANTS=$(ls -t "$GSTACK_STATE_ROOT/projects/$SLUG/designs/"*/variant-*.png 2>/dev/null | head -1)', + reason: 'cross-session discovery probe: design-html runs no generator, it only detects that earlier rounds exist; its approved image comes from approved.json', +}]; + +const GENERATOR = /(?:\$D|"\$D")\s+(generate|variants|iterate|evolve)\b/; +const CONSUMER = /^\s*(?:\$D|"\$D")\s+(compare|check)\b/m; + +function flowFiles(): string[] { + const out: string[] = []; + for (const entry of fs.readdirSync(ROOT, { withFileTypes: true })) { + if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; + const skill = `${entry.name}/SKILL.md`; + if (fs.existsSync(path.join(ROOT, skill))) out.push(skill); + const sections = `${entry.name}/sections`; + if (fs.existsSync(path.join(ROOT, sections))) { + for (const f of fs.readdirSync(path.join(ROOT, sections))) if (f.endsWith('.md')) out.push(`${sections}/${f}`); + } + } + for (const f of fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))) if (f.endsWith('.ts')) out.push(`scripts/resolvers/${f}`); + return out; +} + +function bashLines(text: string): string[] { + return [...text.matchAll(/```bash\n([\s\S]*?)```/g)].flatMap(m => m[1].split('\n')); +} + +function argAfter(line: string, flag: string): string | null { + const m = line.match(new RegExp(`${flag}\\s+("([^"]+)"|(\\S+))`)); + return m ? (m[2] ?? m[3]) : null; +} + +/** Concrete bans only; returns one message per violation. */ +export function scanForBannedReads(file: string, text: string): string[] { + const violations: string[] = []; + const allowed = new Set(ALLOWLIST.filter(a => a.file === file).map(a => a.line.trim())); + const outputs = new Set(); + for (const line of text.split('\n')) { + if (GENERATOR.test(line)) { + const out = argAfter(line, '--output'); + if (out) outputs.add(out); + } + } + text.split('\n').forEach((line, i) => { + const where = `${file}:${i + 1}`; + if (allowed.has(line.trim())) return; + if (/variant-[A-Z]\.png/.test(line)) violations.push(`${where} fixed variant name: ${line.trim()}`); + if (/variant-\*\.png/.test(line)) violations.push(`${where} variant glob: ${line.trim()}`); + if (/variant-\{letter\}/.test(line) && !/gstack-design-claim/.test(line)) violations.push(`${where} variant-{letter} placeholder read: ${line.trim()}`); + const copy = line.match(/(?:^|[\s;&|(`])(?:cp|mv)\s+(.*)$/); + if (copy && (/variant|responsive-|outputPath/.test(copy[1]) || [...outputs].some(o => copy[1].includes(o)))) { + violations.push(`${where} cp/mv of a $D output (publish with gstack-design-claim): ${line.trim()}`); + } + const image = /\$D\s+check\b/.test(line) ? argAfter(line, '--image') : /\$D\s+verify\b/.test(line) ? argAfter(line, '--mockup') : null; + if (image && outputs.has(image)) violations.push(`${where} reads the requested --output instead of the printed path: ${line.trim()}`); + }); + return violations; +} + +const files = flowFiles(); +const read = (f: string) => fs.readFileSync(path.join(ROOT, f), 'utf8'); + +describe('design printed-paths tripwire', () => { + // Value: protects=no skill reads a fixed variant name, glob or requested --output after $D bumps a name; + // fails_when=a template reintroduces variant-A.png, variant-*.png, a cp of a $D output or a literal check path; why_new=#1529; seam=none + test('generated skills and resolvers read only printed paths', () => { + const violations = files.flatMap(f => scanForBannedReads(f, read(f))); + expect(violations).toEqual([]); + }); + + // Value: protects=the allowlist cannot rot into a blanket pass; fails_when=the probe line changes and the entry no longer anchors; + // why_new=content-anchored allowlist; seam=none + test('the allowlist entry still anchors to real content', () => { + for (const a of ALLOWLIST) { + expect(read(a.file).split('\n').map(l => l.trim())).toContain(a.line); + expect(a.reason.length).toBeGreaterThan(20); + } + }); + + // Value: protects=the scanner actually catches each banned form; fails_when=a regex is loosened until it matches nothing; + // why_new=negative controls for the tripwire; seam=none + test('fixtures with each banned form fail', () => { + const cases = [ + '$D compare --images "$_DESIGN_DIR/variant-A.png,$_DESIGN_DIR/variant-B.png"', + '_IMAGES=$(ls "$_DESIGN_DIR"/variant-*.png)', + '5. Quality check: {$D path} check --image {_DESIGN_DIR}/variant-{letter}.png', + 'cp /tmp/variant-B.png "$_DESIGN_DIR/"', + '$D generate --brief "x" --output "$DIR/target.png"\n$D verify --mockup "$DIR/target.png" --screenshot after.png', + '$D generate --brief "x" --output "$DIR/target.png"\n$D check --image "$DIR/target.png" --brief x', + ]; + for (const c of cases) expect(scanForBannedReads('fixture.md', c).length, c).toBeGreaterThan(0); + expect(scanForBannedReads('fixture.md', 'FINAL=$(gstack-design-claim "" "{dir}/variant-{letter}.png")')).toEqual([]); + }); +}); + +describe('design generating flows', () => { + const flows: Record = { + 'office-hours': ['office-hours/SKILL.md'], + 'plan-design-review': ['plan-design-review/SKILL.md'], + 'design-consultation': ['design-consultation/sections/proposal-and-preview.md'], + 'design-shotgun': ['design-shotgun/SKILL.md'], + }; + + for (const [skill, sources] of Object.entries(flows)) { + // Value: protects=round accounting (with the zero-saved stop) comes before the first board or check in each flow; + // fails_when=a template moves the board or check above the accounting marker or drops the stop; why_new=CEO accounting position; seam=none + test(`${skill}: round accounting precedes the first consumer and stops on zero saved`, () => { + const text = sources.map(read).join('\n'); + const accounting = text.indexOf(''); + const board = text.indexOf(''); + const consumer = text.search(CONSUMER); + expect(accounting).toBeGreaterThan(-1); + expect(board).toBeGreaterThan(accounting); + expect(consumer).toBeGreaterThan(accounting); + const span = text.slice(accounting, Math.min(board, consumer)); + expect(span).toMatch(/stop/i); + expect(span).toMatch(/nothing was saved|saves nothing|zero/i); + }); + } + + // Value: protects=generating steps capture JSON and the exit code instead of dying under set -e; + // fails_when=a bare `$D generate ...` line returns; why_new=eng template exit handling; seam=none + test('every generating bash line captures stdout and the exit code', () => { + const bare = files.filter(f => f.endsWith('.md')).flatMap(f => bashLines(read(f)) + .filter(l => GENERATOR.test(l) && !/=\$\(.*\);\s*_?RC=\$\?/.test(l)) + .map(l => `${f}: ${l.trim()}`)); + expect(bare).toEqual([]); + }); + + // Value: protects=every approval writer records approved_path so a bumped round resolves to the right image; + // fails_when=a writer goes back to approved_variant only; why_new=approved-image identity; seam=none + test('every approval-record writer stores approved_path', () => { + const writers = files.flatMap(f => read(f).split('\n') + .filter(l => /approved_variant/.test(l) && (/>\s*"\$_DESIGN_DIR\/approved\.json"/.test(l) || /Write of `\$_DESIGN_DIR\/approved\.json`/.test(l))) + .map(l => ({ f, l }))); + expect(writers.length).toBeGreaterThanOrEqual(3); + for (const { f, l } of writers) expect(l, f).toContain('approved_path'); + }); + + // Value: protects=shotgun subagents stage in a fresh mktemp dir and publish through the no-clobber helper; + // fails_when=the shared /tmp/variant file and cp return; why_new=eng staging requirement; seam=none + test('design-shotgun stages per run and publishes with gstack-design-claim', () => { + const shotgun = read('design-shotgun/SKILL.md'); + const prompt = shotgun.slice(shotgun.indexOf('**Agent prompt template**'), shotgun.indexOf('### Step 3d')); + expect(prompt).toContain('mktemp -d /tmp/'); + expect(prompt).toMatch(/gstack-design-claim ""/); + expect(prompt).not.toMatch(/\/tmp\/variant-/); + expect(prompt).toMatch(/every published path/); + }); +}); diff --git a/test/gstack-design-claim.test.ts b/test/gstack-design-claim.test.ts new file mode 100644 index 000000000..2c2cd3328 --- /dev/null +++ b/test/gstack-design-claim.test.ts @@ -0,0 +1,118 @@ +/** + * bin/gstack-design-claim publishes a staged paid image without overwriting. + * One table drives both claim implementations (the sh helper and $D's + * claimOutputPath) so their suffix and cap rules cannot drift apart. + */ + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { claimOutputPath } from '../design/src/persist'; + +const BIN = path.resolve(import.meta.dir, '..', 'bin', 'gstack-design-claim'); + +let dir: string; +beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'design-claim-')); }); +afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + +function stage(content = 'paid bytes'): string { + const staged = path.join(fs.mkdtempSync(path.join(dir, 'stage-')), 'variant.png'); + fs.writeFileSync(staged, content); + return staged; +} + +function claimSh(src: string, dest: string) { + return spawnSync('sh', [BIN, src, dest], { encoding: 'utf8', timeout: 30_000 }); +} + +const TABLE: Array<{ name: string; existing: string[]; requested: string; expected: string }> = [ + { name: 'free name', existing: [], requested: 'variant-A.png', expected: 'variant-A.png' }, + { name: 'first bump', existing: ['variant-A.png'], requested: 'variant-A.png', expected: 'variant-A-2.png' }, + { name: 'gap is not reused out of order', existing: ['variant-A.png', 'variant-A-2.png'], requested: 'variant-A.png', expected: 'variant-A-3.png' }, + { name: 'suffix appends to the requested stem', existing: ['x-2.png'], requested: 'x-2.png', expected: 'x-2-2.png' }, + { name: 'only the last extension splits', existing: ['shot.v1.png'], requested: 'shot.v1.png', expected: 'shot.v1-2.png' }, + { name: 'no extension', existing: ['mock'], requested: 'mock', expected: 'mock-2' }, +]; + +describe('claim rule table (sh helper and claimOutputPath agree)', () => { + for (const row of TABLE) { + // Value: protects=the sh helper and $D bump to the same -N name; fails_when=either suffix rule changes alone; + // why_new=the two implementations are new and must stay in lockstep; seam=none + test(row.name, () => { + for (const impl of ['sh', 'ts'] as const) { + const target = fs.mkdtempSync(path.join(dir, `${impl}-`)); + for (const f of row.existing) fs.writeFileSync(path.join(target, f), `existing ${f}`); + let actual: string; + if (impl === 'sh') { + const r = claimSh(stage(), path.join(target, row.requested)); + expect(r.status, r.stderr).toBe(0); + actual = r.stdout.trim(); + } else { + const c = claimOutputPath(path.join(target, row.requested)); + fs.closeSync(c.fd); + actual = c.path; + } + expect({ impl, actual }).toEqual({ impl, actual: path.join(target, row.expected) }); + for (const f of row.existing) expect(fs.readFileSync(path.join(target, f), 'utf8')).toBe(`existing ${f}`); + } + }); + } +}); + +describe('gstack-design-claim', () => { + // Value: protects=publishing copies the staged bytes, prints the final path and never clobbers; fails_when=the helper uses plain cp/mv or > without noclobber; + // why_new=new helper replacing shotgun's cp; seam=none + test('publishes bytes without clobbering and removes the staged copy', () => { + const dest = path.join(dir, 'designs', 'variant-B.png'); + fs.mkdirSync(path.dirname(dest)); + fs.writeFileSync(dest, 'round one'); + const staged = stage('round two'); + const r = claimSh(staged, dest); + expect(r.status, r.stderr).toBe(0); + const final = path.join(dir, 'designs', 'variant-B-2.png'); + expect(r.stdout).toBe(`${final}\n`); + expect(fs.readFileSync(dest, 'utf8')).toBe('round one'); + expect(fs.readFileSync(final, 'utf8')).toBe('round two'); + expect(fs.existsSync(staged)).toBe(false); + }); + + // Value: protects=the -999 cap stops with problem/cause/fix and keeps the staged image; fails_when=the cap is removed or the staged file is deleted on failure; + // why_new=new helper; seam=none + test('stops at the -999 cap and keeps the staged image', () => { + const target = path.join(dir, 'full'); + fs.mkdirSync(target); + fs.writeFileSync(path.join(target, 'x.png'), '1'); + for (let n = 2; n <= 999; n++) fs.writeFileSync(path.join(target, `x-${n}.png`), String(n)); + const staged = stage(); + const r = claimSh(staged, path.join(target, 'x.png')); + expect(r.status).not.toBe(0); + expect(r.stderr).toContain('already exist'); + expect(r.stderr).toMatch(/cause: .+\nfix: .+/); + expect(fs.existsSync(staged)).toBe(true); + expect(fs.readdirSync(target).length).toBe(999); + }); + + // Value: protects=a non-collision error (destination parent is a file) fails at once, explains itself and keeps the staged bytes; + // fails_when=errors are treated as collisions or the staged file is removed; why_new=new helper; seam=none + test('other errors exit non-zero with problem, cause and fix, keeping the staged image', () => { + const blocker = path.join(dir, 'not-a-dir'); + fs.writeFileSync(blocker, 'file'); + const staged = stage(); + const r = claimSh(staged, path.join(blocker, 'variant-A.png')); + expect(r.status).not.toBe(0); + expect(r.stderr).toMatch(/^gstack-design-claim: cannot create .*staged image kept at .*\ncause: .+\nfix: .+/); + expect(fs.readFileSync(staged, 'utf8')).toBe('paid bytes'); + }); + + // Value: protects=an empty or missing staged file is refused instead of published; fails_when=the -s guard is removed; + // why_new=new helper; seam=none + test('refuses a missing or empty staged image', () => { + const empty = stage(''); + const r = claimSh(empty, path.join(dir, 'variant-A.png')); + expect(r.status).not.toBe(0); + expect(r.stderr).toContain('missing or empty'); + expect(fs.existsSync(path.join(dir, 'variant-A.png'))).toBe(false); + }); +}); diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index d1c3d3ac7..3d11db808 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -655,6 +655,7 @@ export const E2E_TOUCHFILES: Record = { 'context-save-routing': [ 'context-save/**', 'scripts/resolvers/preamble.ts', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-save-then-restore-roundtrip': ['bin/gstack-state-root.sh', 'lib/state-root.ts', 'context-save/**', 'context-restore/**', 'bin/gstack-slug', 'bin/gstack-remote-identity.sh', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-fragment-match': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], + 'context-restore-provenance-order': ['context-save/**', 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-empty-state': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-list-delegates': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], 'context-restore-legacy-compat': [ 'context-restore/**', 'test/skill-e2e-context-skills.test.ts', 'bin/gstack-skill-start', 'test/helpers/shipped-skill-routing.ts'], @@ -1287,6 +1288,7 @@ export const E2E_TIERS: Record = { 'context-save-routing': 'periodic', // Proves /context-save routes via Skill tool 'context-save-then-restore-roundtrip': 'periodic', // Full cycle in one session 'context-restore-fragment-match': 'periodic', // /context-restore + 'context-restore-provenance-order': 'periodic', // #3004 Next steps / Verify first, 3-trial panel 'context-restore-empty-state': 'periodic', // Graceful zero-saves message 'context-restore-list-delegates': 'periodic', // /context-restore list redirect 'context-restore-legacy-compat': 'periodic', // Pre-rename files still load @@ -1664,6 +1666,7 @@ export const E2E_KINDS: Record = { 'context-save-routing': 'rule', 'context-save-then-restore-roundtrip': 'rule', 'context-restore-fragment-match': 'rule', + 'context-restore-provenance-order': 'behavior', 'context-restore-empty-state': 'rule', 'context-restore-list-delegates': 'rule', 'context-restore-legacy-compat': 'rule', @@ -1787,6 +1790,8 @@ export const E2E_KINDS: Record = { export const BEHAVIOR_WHY: Record = { 'shared-libs-opportunity-judgment': "Whether a candidate extraction is worth recommending is a judgment call; the read-only invariant stays a contract.", + 'context-restore-provenance-order': + "How the live model lays out the two Remaining Work groups can drift run to run; never executing a seeded step and never leapfrogging the unverified first item stay contracts.", 'review-design-lite': "How many of the seven design-lite checklist items the live review flags varies run to run; the fake-engine rows it must carry stay strict.", 'review-army-red-team': diff --git a/test/skill-e2e-context-skills.test.ts b/test/skill-e2e-context-skills.test.ts index 71fabe0da..31f214505 100644 --- a/test/skill-e2e-context-skills.test.ts +++ b/test/skill-e2e-context-skills.test.ts @@ -14,6 +14,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { JUDGE_MS, CAPTURE_MS } from './helpers/eval-budgets'; import { runSkillTest } from './helpers/session-runner'; +import { expectContract } from './helpers/eval-store'; import { ROOT, runId, evalsEnabled, describeIfSelected, testConcurrentIfSelected, @@ -136,6 +137,7 @@ describeIfSelected('Context Skills E2E (live-fire)', [ 'context-save-routing', 'context-save-then-restore-roundtrip', 'context-restore-fragment-match', + 'context-restore-provenance-order', 'context-restore-empty-state', 'context-restore-list-delegates', 'context-restore-legacy-compat', @@ -277,6 +279,73 @@ Do NOT use AskUserQuestion.`, try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {} }, CAPTURE_MS); + // ── 3b. Provenance (#3004): guessed and code-read steps are Verify first ── + // The checkpoint ranks an assumed step first, then a step that was run, then + // a write whose target was never inspected. Restore must show the run step + // under Next steps, the other two under Verify first, and option A must + // verify the first item instead of leapfrogging to the runnable one. + testConcurrentIfSelected('context-restore-provenance-order', async () => { + const { workDir, gstackHome, slug } = setupWorkdir('provenance'); + seedSave(gstackHome, slug, '20260404-120000-billing-migration.md', + { status: 'in-progress', branch: 'main', timestamp: '2026-04-04T12:00:00Z' }, + [ + '## Working on: billing migration', + '', + '### Summary', + 'Moving invoices to the new billing schema.', + '', + '### Remaining Work', + '1. Open. Run the migration check with SWITCH_B=1 against staging, PROV_ASSUMED_7Q. (path assumed)', + '2. Open. Run bun test test/billing.test.ts, PROV_RUN_4K. (path run) exit 0', + '3. Open. Apply scripts/backfill.sql (INSERT IGNORE into invoices), PROV_WRITE_9Z. (code read)', + '', + '### Notes', + 'None.', + '', + ].join('\n')); + + const result = await runSkillTest({ + prompt: `Run /context-restore. Invoke via the Skill tool and present its summary. Do NOT use AskUserQuestion: assume the user picks A (continue working). Do not execute or edit anything. End your reply with exactly one line \`FIRST_ACTION: \`.`, + workingDirectory: workDir, + env: { GSTACK_HOME: gstackHome }, + maxTurns: 10, + allowedTools: ['Skill', 'Bash', 'Read', 'Grep', 'Glob'], + timeout: JUDGE_MS, + testName: 'context-restore-provenance-order', + runId, + }); + + logCost('context-restore-provenance-order', result); + + const out = result.output || ''; + const base = Math.max(0, out.indexOf('Remaining Work')); + const next = out.indexOf('Next steps', base); + const verify = next >= 0 ? out.indexOf('Verify first', next) : -1; + const pos = (token: string) => out.indexOf(token, base); + const grouped = next >= 0 && verify > next + && pos('PROV_RUN_4K') > next && pos('PROV_RUN_4K') < verify + && pos('PROV_ASSUMED_7Q') > verify && pos('PROV_WRITE_9Z') > verify; + const firstAction = out.match(/FIRST_ACTION:\s*(verify|do)\s+(PROV_\w+)/i); + const ranSeeded = (result.toolCalls || []).some((tc) => tc.tool === 'Bash' + && /SWITCH_B=1|backfill\.sql|billing\.test/.test(JSON.stringify(tc.input || {}))); + const exitOk = ['success', 'error_max_turns'].includes(result.exitReason); + const name = 'context-restore provenance order'; + + expectContract(!ranSeeded, 'context-restore-provenance-order: restore executed a seeded Remaining Work command', { collector: evalCollector, name }); + expectContract(!(firstAction && firstAction[2] !== 'PROV_ASSUMED_7Q' && /do/i.test(firstAction[1])), + 'context-restore-provenance-order: option A jumped past the unverified first item to execute a later one', { collector: evalCollector, name }); + + const startsByVerifying = !!firstAction && /verify/i.test(firstAction[1]) && firstAction[2] === 'PROV_ASSUMED_7Q'; + recordE2E(evalCollector, name, 'Context Skills E2E', result, { + passed: exitOk && grouped && startsByVerifying, + }); + + expect(exitOk).toBe(true); + expect(grouped).toBe(true); + expect(startsByVerifying).toBe(true); + try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {} + }, CAPTURE_MS); + // ── 4. /context-restore with zero saves → graceful empty-state ─────── testConcurrentIfSelected('context-restore-empty-state', async () => { const { workDir, gstackHome, slug } = setupWorkdir('empty');