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.

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('<!-- design:board -->');
+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<string, string>;
+
+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('<VARIANT>', letter).replaceAll('<FEEDBACK>', 'calmer').replaceAll('<SCREEN>', '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');
+});

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 = path.join(entry.name, 'SKILL.md');
+    if (fs.existsSync(path.join(ROOT, skill))) out.push(skill);
+    const sections = path.join(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(path.join(sections, f));
+    }
+  }
+  for (const f of fs.readdirSync(path.join(ROOT, 'scripts/resolvers'))) if (f.endsWith('.ts')) out.push(path.join('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<string>();
+  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 "<outputPath>" "{dir}/variant-{letter}.png")')).toEqual([]);
+  });
+});
+
+describe('design generating flows', () => {
+  const flows: Record<string, string[]> = {
+    '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('<!-- design:round-accounting -->');
+      const board = text.indexOf('<!-- design:board -->');
+      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 "<outputPath>"/);
+    expect(prompt).not.toMatch(/\/tmp\/variant-/);
+    expect(prompt).toMatch(/every published path/);
+  });
+});