feat(show-dont-tell): require an image or preview URL when rendered paths change

.github/scripts/show-dont-tell.mjs

@@ -0,0 +1,115 @@
+// show-dont-tell: a pull request that touches a rendered path must show it.
+// Node built-ins only. Used by workflows/show-dont-tell.yml.
+//
+//   node show-dont-tell.mjs <config-file>   (env: GH_TOKEN, REPO, PR)
+//
+// Exits 1 (red) when a changed file matches `paths:` and the PR body carries
+// neither an image nor a preview URL; 0 otherwise. Writes markdown to stdout.
+import { execFileSync } from 'node:child_process';
+import { existsSync, readFileSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+
+// Glob -> RegExp. `**` crosses `/`, `*` and `?` do not; `**/` may match nothing.
+export function globToRegExp(glob) {
+  let re = '';
+  for (let i = 0; i < glob.length; i++) {
+    const c = glob[i];
+    if (c === '*') {
+      if (glob[i + 1] === '*') {
+        i++;
+        if (glob[i + 1] === '/') { i++; re += '(?:.*/)?'; } else re += '.*';
+      } else re += '[^/]*';
+    } else if (c === '?') re += '[^/]';
+    else re += c.replace(/[.+^${}()|[\]\\]/g, '\\$&');
+  }
+  return new RegExp(`^${re}$`);
+}
+
+export function matchPaths(files, globs) {
+  const res = globs.map(globToRegExp);
+  return files.filter((f) => res.some((r) => r.test(f)));
+}
+
+// The opt-in file: top-level `paths:` and optional `urls:`, each a "- item"
+// list. Comments (#) and quotes are tolerated; nothing else is YAML.
+export function parseConfig(text) {
+  const out = { paths: [], urls: [] };
+  let key = null;
+  for (const raw of text.split(/\r?\n/)) {
+    const line = raw.replace(/(^|\s)#.*$/, '').trimEnd();
+    if (!line.trim()) continue;
+    const head = /^(paths|urls):\s*$/.exec(line);
+    if (head) { key = head[1]; continue; }
+    const item = /^\s*-\s+(.*)$/.exec(line);
+    if (item && key) out[key].push(item[1].trim().replace(/^(["'])(.*)\1$/, '$2'));
+    else key = null;
+  }
+  return out;
+}
+
+const IMAGE = [
+  /!\[[^\]]*\]\(\s*<?[^)\s>]+/,                              // markdown image
+  /<img\b[^>]*\bsrc\s*=/i,                                    // html image
+  /https:\/\/github\.com\/user-attachments\/(?:assets|files)\/[\w-]+/, // dragged-in upload
+  /https:\/\/user-images\.githubusercontent\.com\/[^\s)"'>]+/, // legacy upload
+];
+
+// What the body shows: {kind: 'image'|'url'|null, found}. A URL counts only
+// when `urls` names its prefix: "any https link" was satisfied by the agent
+// attribution link (https://claude.com/claude-code) in every agent-written body.
+export function scanBody(body, urls = []) {
+  const text = body || '';
+  // Code spans, fences and HTML comments (a template's "drop a screenshot
+  // here" hint) only describe a picture; they do not show one.
+  const shown = text.replace(/```[\s\S]*?```|`[^`\n]*`|<!--[\s\S]*?-->/g, '');
+  for (const re of IMAGE) {
+    const m = re.exec(shown);
+    if (m) return { kind: 'image', found: m[0] };
+  }
+  const all = urls.length ? shown.match(/https:\/\/[^\s)<>"'\]]+/g) || [] : [];
+  const hit = all.find((u) => urls.some((p) => u.startsWith(p)));
+  return hit ? { kind: 'url', found: hit } : { kind: null, found: '' };
+}
+
+export function report({ matched, shown, urls }) {
+  if (!matched.length) return { ok: true, md: '### show-dont-tell\n\nNo rendered path touched: nothing to show.\n' };
+  const list = matched.slice(0, 20).map((f) => `- \`${f}\``).join('\n') + (matched.length > 20 ? `\n- ... and ${matched.length - 20} more` : '');
+  if (shown.kind) return { ok: true, md: `### show-dont-tell\n\nRendered paths touched (${matched.length}) and the body shows ${shown.kind === 'image' ? 'an image' : 'a preview URL'}: \`${shown.found}\`\n\n${list}\n` };
+  const what = urls.length ? ` or a link starting with one of ${urls.map((u) => `\`${u}\``).join(', ')}` : '';
+  return { ok: false, md: `### show-dont-tell: red\n\nThis pull request touches rendered paths:\n\n${list}\n\nIts description carries no picture. Edit the description to add an inline image (\`![what it shows](url)\`, or drag a screenshot in)${what}; the check re-runs when the description is edited.\n` };
+}
+
+function gh(...args) { return execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 1 << 28 }); }
+
+function main() {
+  const [cfgPath] = process.argv.slice(2);
+  const { REPO, PR, BASE_SHA } = process.env;
+  // The opt-in file is read at the PR's base, so a PR cannot exempt itself by
+  // deleting or emptying it; a change to it takes effect once it merges.
+  let text = null;
+  if (BASE_SHA) {
+    try {
+      text = gh('api', `repos/${REPO}/contents/${cfgPath}?ref=${BASE_SHA}`, '-H', 'Accept: application/vnd.github.raw');
+    } catch (e) {
+      if (!/HTTP 404/.test(String(e.stderr || e.message))) throw e;
+    }
+  } else if (existsSync(cfgPath)) text = readFileSync(cfgPath, 'utf8');
+  if (text === null) {
+    console.log(`### show-dont-tell\n\nNo \`${cfgPath}\` in this repository${BASE_SHA ? ' at the base commit' : ''}: not opted in, nothing checked.`);
+    return 0;
+  }
+  const cfg = parseConfig(text);
+  if (!cfg.paths.length) {
+    console.log(`### show-dont-tell\n\n\`${cfgPath}\` lists no \`paths:\`: nothing checked.`);
+    return 0;
+  }
+  const files = gh('api', `repos/${REPO}/pulls/${PR}/files`, '--paginate', '--jq', '.[] | .filename, (.previous_filename // empty)').split('\n').filter(Boolean);
+  const matched = matchPaths(files, cfg.paths);
+  const body = matched.length ? gh('api', `repos/${REPO}/pulls/${PR}`, '--jq', '.body // ""') : '';
+  const r = report({ matched, shown: scanBody(body, cfg.urls), urls: cfg.urls });
+  console.log(r.md);
+  if (!r.ok) console.error('::error::show-dont-tell: rendered paths touched, but the PR description has no image or preview URL.');
+  return r.ok ? 0 : 1;
+}
+
+if (process.argv[1] === fileURLToPath(import.meta.url)) process.exit(main());

.github/scripts/show-dont-tell.test.mjs

@@ -0,0 +1,83 @@
+// Tests for show-dont-tell.mjs. Run: node --test .github/scripts/show-dont-tell.test.mjs
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+import { globToRegExp, matchPaths, parseConfig, report, scanBody } from './show-dont-tell.mjs';
+
+test('globs: ** crosses directories, * and ? do not', () => {
+  const m = (g, p) => globToRegExp(g).test(p);
+  assert.ok(m('ui/**', 'ui/a/b.tsx'));
+  assert.ok(m('**/golden/**', 'x/y/golden/a.png'));
+  assert.ok(m('**/golden/**', 'golden/a.png'));
+  assert.ok(m('src/*.css', 'src/a.css'));
+  assert.ok(!m('src/*.css', 'src/deep/a.css'));
+  assert.ok(m('a?.md', 'ab.md'));
+  assert.ok(!m('a?.md', 'a/.md'));
+  assert.ok(!m('ui/**', 'other/ui/a.tsx'));
+  assert.ok(m('a.b', 'a.b'));
+  assert.ok(!m('a.b', 'axb'));
+});
+
+test('matchPaths returns only matching files', () => {
+  assert.deepEqual(matchPaths(['ui/a.tsx', 'README.md', 'docs/x.md'], ['ui/**', 'docs/*.md']), ['ui/a.tsx', 'docs/x.md']);
+  assert.deepEqual(matchPaths(['README.md'], ['ui/**']), []);
+});
+
+test('parseConfig reads paths and urls, tolerates comments and quotes', () => {
+  const c = parseConfig('# rendered\npaths:\n  - "ui/**"   # the app\n  - docs/*.md\nurls:\n  - https://preview.example.com/\n');
+  assert.deepEqual(c, { paths: ['ui/**', 'docs/*.md'], urls: ['https://preview.example.com/'] });
+  assert.deepEqual(parseConfig(''), { paths: [], urls: [] });
+});
+
+test('parseConfig accepts unindented list items', () => {
+  assert.deepEqual(parseConfig('paths:\n- web/**\n'), { paths: ['web/**'], urls: [] });
+});
+
+test('scanBody: an image only named in a code span or fence is not shown', () => {
+  assert.equal(scanBody('Satisfied by an inline image (`![](..)`)').kind, null);
+  assert.equal(scanBody('add `![](url)`').kind, null);
+  assert.equal(scanBody('```\n![x](y.png)\n```').kind, null);
+  assert.equal(scanBody('`code` then ![x](y.png)').kind, 'image');
+  assert.equal(scanBody('<!-- drop a screenshot here: ![what it shows](url) -->').kind, null);
+  assert.equal(scanBody('<!-- <img src="a.png"> -->').kind, null);
+  assert.equal(scanBody('<!-- hint -->\n![x](y.png)').kind, 'image');
+  const urls = ['https://preview.example.com/'];
+  assert.equal(scanBody('urls: `https://preview.example.com/`', urls).kind, null);
+  assert.equal(scanBody('```\nhttps://preview.example.com/\n```', urls).kind, null);
+  assert.equal(scanBody('`code` then https://preview.example.com/', urls).kind, 'url');
+  assert.equal(scanBody('<!-- https://preview.example.com/ -->', urls).kind, null);
+});
+
+test('scanBody: images count anywhere', () => {
+  assert.equal(scanBody('hi ![shot](https://x.test/a.png)').kind, 'image');
+  assert.equal(scanBody('<img width="3" src="a.png">').kind, 'image');
+  assert.equal(scanBody('https://github.com/user-attachments/assets/1234-abcd').kind, 'image');
+  assert.equal(scanBody('https://user-images.githubusercontent.com/1/a.png').kind, 'image');
+});
+
+test('scanBody: without urls, no link counts, the agent attribution link least of all', () => {
+  assert.equal(scanBody('see https://pr-4.preview.example.com/x').kind, null);
+  assert.equal(scanBody('🤖 Generated with [Claude Code](https://claude.com/claude-code)').kind, null);
+  assert.equal(scanBody('see https://github.com/o/r/actions/runs/1').kind, null);
+  assert.equal(scanBody('').kind, null);
+  assert.equal(scanBody(null).kind, null);
+});
+
+test('scanBody: with urls, only those prefixes count', () => {
+  const urls = ['https://preview.example.com/'];
+  assert.equal(scanBody('https://other.example.org/x', urls).kind, null);
+  assert.equal(scanBody('https://claude.com/claude-code', urls).kind, null);
+  assert.equal(scanBody('see http://preview.example.com/pr/4', urls).kind, null);
+  assert.equal(scanBody('https://preview.example.com/pr/4', urls).kind, 'url');
+});
+
+test('report: green with no match, green when shown, red when not', () => {
+  assert.ok(report({ matched: [], shown: { kind: null }, urls: [] }).ok);
+  assert.match(report({ matched: [], shown: { kind: null }, urls: [] }).md, /No rendered path touched/);
+  assert.ok(report({ matched: ['ui/a'], shown: { kind: 'image', found: 'x' }, urls: [] }).ok);
+  const red = report({ matched: ['ui/a'], shown: { kind: null, found: '' }, urls: [] });
+  assert.ok(!red.ok);
+  assert.match(red.md, /`ui\/a`/);
+  assert.match(red.md, /inline image/);
+  assert.doesNotMatch(red.md, /link starting with/);
+  assert.match(report({ matched: ['ui/a'], shown: { kind: null, found: '' }, urls: ['https://p.example/'] }).md, /link starting with one of `https:\/\/p\.example\/`/);
+});

.github/workflows/ci.yml

@@ -98,6 +98,16 @@ jobs:
       - run: npm ci --prefix .github/scripts/golden-diff --no-audit --no-fund
       - run: node --test .github/scripts/golden-diff.test.mjs
 
+  # The show-dont-tell script's suite (glob matcher, body scanner); no
+  # network, node built-ins only; seconds.
+  show-dont-tell-tests:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+      - run: node --test .github/scripts/show-dont-tell.test.mjs
+
   # The review-metrics suite; parsing and classing only, no network; seconds.
   review-metrics-tests:
     runs-on: ubuntu-latest
@@ -110,7 +120,7 @@ jobs:
   # The single required status check. `always()` lets it evaluate when a
   # dependency failed or was skipped instead of inheriting the skip.
   ci-gate:
-    needs: [nonempty, secret-scan, complexity-tests, duplication-tests, golden-diff-tests, review-metrics-tests]
+    needs: [nonempty, secret-scan, complexity-tests, duplication-tests, golden-diff-tests, show-dont-tell-tests, review-metrics-tests]
     if: always()
     permissions: {}
     uses: ./.github/workflows/ci-gate.yml

.github/workflows/show-dont-tell.yml

@@ -0,0 +1,91 @@
+name: show-dont-tell
+
+# A pull request that touches a rendered path must show it: an image or a
+# preview URL in its description, else this check is red. Why: a change to
+# what people see is judged by looking, and a description that only tells
+# ("tweaked the header spacing") makes the reviewer rebuild the page to find
+# out. Opt-in per repository; a repository without the file below passes
+# with a note.
+#
+# Caller, in the repository's own workflow (the `edited` type is what makes
+# a description edit re-run the check, so keep it):
+#
+#     on:
+#       pull_request:
+#         types: [opened, edited, synchronize, reopened]
+#     jobs:
+#       show-dont-tell:
+#         permissions: { contents: read, pull-requests: read }
+#         uses: mark-brannan/.github/.github/workflows/show-dont-tell.yml@main
+#
+# Opt-in file, `.github/show-dont-tell.yml` in the calling repository, read
+# at the PR's base commit (a PR cannot exempt itself by editing it):
+#
+#     paths:               # globs of rendered files; ** crosses /, * and ? do not
+#       - "web/**"
+#       - "screenshots/goldens/**"
+#     urls:                # optional: prefixes of this repo's preview URLs
+#       - https://preview.example.com/
+#
+# What satisfies the check: an inline image anywhere in the description
+# (`![..](..)`, `<img src=..>`, a github.com/user-attachments upload), or a
+# link starting with one of the `urls:` prefixes. With `urls:` absent only an
+# image counts: "any https link" was met by the agent attribution link
+# (https://claude.com/claude-code) in every agent-written body, so the check
+# would never have fired on one. Green with "no rendered path
+# touched" when no changed file matches. The description is read live from
+# the API, not the event payload, so an edit is judged on its new text.
+# Renamed files match on both old and new name. Needs only read scopes.
+# (The caller checks nothing out; the job reads the file through the API.)
+#
+# The script is .github/scripts/show-dont-tell.mjs in this repository (node
+# built-ins only, so the job is seconds); it is fetched from `engine-ref`,
+# main by default, as golden-diff.yml does, so a caller can try a branch.
+
+on:
+  workflow_call:
+    inputs:
+      config:
+        description: "Path of the opt-in file in the calling repository"
+        type: string
+        default: ".github/show-dont-tell.yml"
+      engine-ref:
+        description: "Ref of mark-brannan/.github to fetch the script from; override to try a branch from a caller before it lands"
+        type: string
+        default: main
+
+jobs:
+  show-dont-tell:
+    runs-on: ubuntu-latest
+    if: github.event_name == 'pull_request'
+    env:
+      SCRIPT: ${{ github.repository == 'mark-brannan/.github' && '.github/scripts' || '.show-dont-tell-engine/.github/scripts' }}
+    steps:
+      - if: github.repository == 'mark-brannan/.github'
+        uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+      - if: github.repository != 'mark-brannan/.github'
+        uses: actions/checkout@v7
+        with:
+          repository: mark-brannan/.github
+          ref: ${{ inputs.engine-ref }}
+          sparse-checkout: |
+            /.github/scripts/show-dont-tell.mjs
+          sparse-checkout-cone-mode: false
+          path: .show-dont-tell-engine
+          persist-credentials: false
+      - name: The description shows what the rendered paths changed
+        env:
+          GH_TOKEN: ${{ github.token }}
+          REPO: ${{ github.repository }}
+          PR: ${{ github.event.pull_request.number }}
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          CONFIG: ${{ inputs.config }}
+        run: |
+          set -euo pipefail
+          rc=0
+          node "$SCRIPT/show-dont-tell.mjs" "$CONFIG" > "$RUNNER_TEMP/sdt.md" || rc=$?
+          cat "$RUNNER_TEMP/sdt.md" >> "$GITHUB_STEP_SUMMARY"
+          cat "$RUNNER_TEMP/sdt.md"
+          exit "$rc"