Merge remote-tracking branch 'origin/garrytan/mvanhorn-wave-design' into garrytan/mvanhorn-fix-wave

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/);
+  });
+});

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);
+  });
+});