From dff70f712b71e369dfce0a514e14d23aa6e0d459 Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Fri, 16 Oct 2026 23:02:57 -0700 Subject: [PATCH 1/6] feat(show-dont-tell): reusable check that a PR touching rendered paths shows an image or preview URL Co-Authored-By: Claude Sonnet 5.5 --- .github/scripts/show-dont-tell.mjs | 103 ++++++++++++++++++++++++ .github/scripts/show-dont-tell.test.mjs | 61 ++++++++++++++ .github/workflows/ci.yml | 12 ++- .github/workflows/show-dont-tell.yml | 85 +++++++++++++++++++ 4 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 .github/scripts/show-dont-tell.mjs create mode 100644 .github/scripts/show-dont-tell.test.mjs create mode 100644 .github/workflows/show-dont-tell.yml diff --git a/.github/scripts/show-dont-tell.mjs b/.github/scripts/show-dont-tell.mjs new file mode 100644 index 0000000..cfb0821 --- /dev/null +++ b/.github/scripts/show-dont-tell.mjs @@ -0,0 +1,103 @@ +// 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 (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*]+/, // markdown image + /]*\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}. Without `urls`, any +// https URL not on github.com counts; with it, only URLs starting with a prefix. +export function scanBody(body, urls = []) { + const text = body || ''; + for (const re of IMAGE) { + const m = re.exec(text); + if (m) return { kind: 'image', found: m[0] }; + } + const all = text.match(/https:\/\/[^\s)<>"'\]]+/g) || []; + const hit = all.find((u) => (urls.length + ? urls.some((p) => u.startsWith(p)) + : !/^https:\/\/(?:[\w-]+\.)*github\.com(?:[/:]|$)/i.test(u))); + 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 ? `a link starting with one of ${urls.map((u) => `\`${u}\``).join(', ')}` : 'a link to a preview outside github.com'; + 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) or ${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 } = process.env; + if (!existsSync(cfgPath)) { + console.log(`### show-dont-tell\n\nNo \`${cfgPath}\` in this repository: not opted in, nothing checked.`); + return 0; + } + const cfg = parseConfig(readFileSync(cfgPath, 'utf8')); + 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()); diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs new file mode 100644 index 0000000..d6ca957 --- /dev/null +++ b/.github/scripts/show-dont-tell.test.mjs @@ -0,0 +1,61 @@ +// 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('scanBody: images count anywhere', () => { + assert.equal(scanBody('hi ![shot](https://x.test/a.png)').kind, 'image'); + assert.equal(scanBody('').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: default preview URL is any https link off github.com', () => { + assert.equal(scanBody('see https://pr-4.preview.example.com/x').kind, 'url'); + assert.equal(scanBody('see https://github.com/o/r/actions/runs/1').kind, null); + assert.equal(scanBody('see https://gist.github.com/o/1').kind, null); + assert.equal(scanBody('see http://insecure.example.com').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://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/); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba88f5a..e6f1dae 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/.github/workflows/show-dont-tell.yml b/.github/workflows/show-dont-tell.yml new file mode 100644 index 0000000..740c53c --- /dev/null +++ b/.github/workflows/show-dont-tell.yml @@ -0,0 +1,85 @@ +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: +# +# paths: # globs of rendered files; ** crosses /, * and ? do not +# - "web/**" +# - "screenshots/goldens/**" +# urls: # optional prefixes that count as a preview URL +# - https://preview.example.com/ +# +# What satisfies the check: an inline image anywhere in the description +# (`![..](..)`, ``, a github.com/user-attachments upload), or a +# preview URL -- with `urls:` absent, any https link not on github.com; with +# it, a link starting with one of the prefixes. 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 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: + - 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 }} + 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" From 07207a9ecbb37d29b331a1349e237a7955141e6f Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Fri, 16 Oct 2026 23:05:53 -0700 Subject: [PATCH 2/6] fix(show-dont-tell): read the opt-in file at the base, accept unindented lists, ignore code spans Co-Authored-By: Claude Sonnet 5.5 --- .github/scripts/show-dont-tell.mjs | 24 ++++++++++++++++++------ .github/scripts/show-dont-tell.test.mjs | 11 +++++++++++ .github/workflows/show-dont-tell.yml | 8 ++++++-- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/.github/scripts/show-dont-tell.mjs b/.github/scripts/show-dont-tell.mjs index cfb0821..ca0e213 100644 --- a/.github/scripts/show-dont-tell.mjs +++ b/.github/scripts/show-dont-tell.mjs @@ -40,7 +40,7 @@ export function parseConfig(text) { if (!line.trim()) continue; const head = /^(paths|urls):\s*$/.exec(line); if (head) { key = head[1]; continue; } - const item = /^\s+-\s+(.*)$/.exec(line); + const item = /^\s*-\s+(.*)$/.exec(line); if (item && key) out[key].push(item[1].trim().replace(/^(["'])(.*)\1$/, '$2')); else key = null; } @@ -58,8 +58,10 @@ const IMAGE = [ // https URL not on github.com counts; with it, only URLs starting with a prefix. export function scanBody(body, urls = []) { const text = body || ''; + // Code spans and fences only describe a picture; they do not show one. + const shown = text.replace(/```[\s\S]*?```|`[^`\n]*`/g, ''); for (const re of IMAGE) { - const m = re.exec(text); + const m = re.exec(shown); if (m) return { kind: 'image', found: m[0] }; } const all = text.match(/https:\/\/[^\s)<>"'\]]+/g) || []; @@ -81,12 +83,22 @@ function gh(...args) { return execFileSync('gh', args, { encoding: 'utf8', maxBu function main() { const [cfgPath] = process.argv.slice(2); - const { REPO, PR } = process.env; - if (!existsSync(cfgPath)) { - console.log(`### show-dont-tell\n\nNo \`${cfgPath}\` in this repository: not opted in, nothing checked.`); + 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(readFileSync(cfgPath, 'utf8')); + const cfg = parseConfig(text); if (!cfg.paths.length) { console.log(`### show-dont-tell\n\n\`${cfgPath}\` lists no \`paths:\`: nothing checked.`); return 0; diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs index d6ca957..ccd0e9b 100644 --- a/.github/scripts/show-dont-tell.test.mjs +++ b/.github/scripts/show-dont-tell.test.mjs @@ -28,6 +28,17 @@ test('parseConfig reads paths and urls, tolerates comments and quotes', () => { 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'); +}); + test('scanBody: images count anywhere', () => { assert.equal(scanBody('hi ![shot](https://x.test/a.png)').kind, 'image'); assert.equal(scanBody('').kind, 'image'); diff --git a/.github/workflows/show-dont-tell.yml b/.github/workflows/show-dont-tell.yml index 740c53c..92d6a53 100644 --- a/.github/workflows/show-dont-tell.yml +++ b/.github/workflows/show-dont-tell.yml @@ -18,7 +18,8 @@ name: 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: +# 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/**" @@ -33,6 +34,7 @@ name: show-dont-tell # 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`, @@ -57,7 +59,8 @@ jobs: env: SCRIPT: ${{ github.repository == 'mark-brannan/.github' && '.github/scripts' || '.show-dont-tell-engine/.github/scripts' }} steps: - - uses: actions/checkout@v7 + - if: github.repository == 'mark-brannan/.github' + uses: actions/checkout@v7 with: persist-credentials: false - if: github.repository != 'mark-brannan/.github' @@ -75,6 +78,7 @@ jobs: 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 From c34752103c76c56a9c12f5e5492cb0feac1b3709 Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Fri, 16 Oct 2026 23:16:43 -0700 Subject: [PATCH 3/6] fix(show-dont-tell): a URL quoted in a code span or fence is not shown either The code-span strip covered the image scan only; the URL scan still read the raw body, so a fenced example URL counted as a preview. Co-Authored-By: Claude Fable 5.1 --- .github/scripts/show-dont-tell.mjs | 2 +- .github/scripts/show-dont-tell.test.mjs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/scripts/show-dont-tell.mjs b/.github/scripts/show-dont-tell.mjs index ca0e213..ba6f58f 100644 --- a/.github/scripts/show-dont-tell.mjs +++ b/.github/scripts/show-dont-tell.mjs @@ -64,7 +64,7 @@ export function scanBody(body, urls = []) { const m = re.exec(shown); if (m) return { kind: 'image', found: m[0] }; } - const all = text.match(/https:\/\/[^\s)<>"'\]]+/g) || []; + const all = shown.match(/https:\/\/[^\s)<>"'\]]+/g) || []; const hit = all.find((u) => (urls.length ? urls.some((p) => u.startsWith(p)) : !/^https:\/\/(?:[\w-]+\.)*github\.com(?:[/:]|$)/i.test(u))); diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs index ccd0e9b..e091fba 100644 --- a/.github/scripts/show-dont-tell.test.mjs +++ b/.github/scripts/show-dont-tell.test.mjs @@ -37,6 +37,9 @@ test('scanBody: an image only named in a code span or fence is not shown', () => 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('urls: `https://preview.example.com/`').kind, null); + assert.equal(scanBody('```\nhttps://preview.example.com/\n```').kind, null); + assert.equal(scanBody('`code` then https://preview.example.com/').kind, 'url'); }); test('scanBody: images count anywhere', () => { From 047ba9c11ead971f66fc4050fb9f3f4f9a15db7b Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Sat, 17 Oct 2026 00:13:48 -0700 Subject: [PATCH 4/6] fix(show-dont-tell): a URL counts only when the opt-in file names its prefix Without urls:, any https link off github.com satisfied the check, and every agent-written PR body carries one: the attribution link https://claude.com/claude-code. The check would never have fired on an agent PR. Now only an image counts unless urls: names the repo's preview prefixes; the red message drops the link hint when none are named. Co-Authored-By: Claude Fable 5.1 --- .github/scripts/show-dont-tell.mjs | 15 +++++++-------- .github/scripts/show-dont-tell.test.mjs | 11 +++++++---- .github/workflows/show-dont-tell.yml | 8 +++++--- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/.github/scripts/show-dont-tell.mjs b/.github/scripts/show-dont-tell.mjs index ba6f58f..29d8b37 100644 --- a/.github/scripts/show-dont-tell.mjs +++ b/.github/scripts/show-dont-tell.mjs @@ -54,8 +54,9 @@ const IMAGE = [ /https:\/\/user-images\.githubusercontent\.com\/[^\s)"'>]+/, // legacy upload ]; -// What the body shows: {kind: 'image'|'url'|null, found}. Without `urls`, any -// https URL not on github.com counts; with it, only URLs starting with a prefix. +// 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 and fences only describe a picture; they do not show one. @@ -64,10 +65,8 @@ export function scanBody(body, urls = []) { const m = re.exec(shown); if (m) return { kind: 'image', found: m[0] }; } - const all = shown.match(/https:\/\/[^\s)<>"'\]]+/g) || []; - const hit = all.find((u) => (urls.length - ? urls.some((p) => u.startsWith(p)) - : !/^https:\/\/(?:[\w-]+\.)*github\.com(?:[/:]|$)/i.test(u))); + 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: '' }; } @@ -75,8 +74,8 @@ 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 ? `a link starting with one of ${urls.map((u) => `\`${u}\``).join(', ')}` : 'a link to a preview outside github.com'; - 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) or ${what}; the check re-runs when the description is edited.\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 }); } diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs index e091fba..5b92f6c 100644 --- a/.github/scripts/show-dont-tell.test.mjs +++ b/.github/scripts/show-dont-tell.test.mjs @@ -49,11 +49,10 @@ test('scanBody: images count anywhere', () => { assert.equal(scanBody('https://user-images.githubusercontent.com/1/a.png').kind, 'image'); }); -test('scanBody: default preview URL is any https link off github.com', () => { - assert.equal(scanBody('see https://pr-4.preview.example.com/x').kind, 'url'); +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('see https://gist.github.com/o/1').kind, null); - assert.equal(scanBody('see http://insecure.example.com').kind, null); assert.equal(scanBody('').kind, null); assert.equal(scanBody(null).kind, null); }); @@ -61,6 +60,8 @@ test('scanBody: default preview URL is any https link off github.com', () => { 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'); }); @@ -72,4 +73,6 @@ test('report: green with no match, green when shown, red when not', () => { 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\/`/); }); diff --git a/.github/workflows/show-dont-tell.yml b/.github/workflows/show-dont-tell.yml index 92d6a53..2ee8962 100644 --- a/.github/workflows/show-dont-tell.yml +++ b/.github/workflows/show-dont-tell.yml @@ -24,13 +24,15 @@ name: show-dont-tell # paths: # globs of rendered files; ** crosses /, * and ? do not # - "web/**" # - "screenshots/goldens/**" -# urls: # optional prefixes that count as a preview URL +# urls: # optional: prefixes of this repo's preview URLs # - https://preview.example.com/ # # What satisfies the check: an inline image anywhere in the description # (`![..](..)`, ``, a github.com/user-attachments upload), or a -# preview URL -- with `urls:` absent, any https link not on github.com; with -# it, a link starting with one of the prefixes. Green with "no rendered path +# 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. From 97708f2fecc265e8b299045cd221d71d665fb0cb Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Sat, 17 Oct 2026 00:14:16 -0700 Subject: [PATCH 5/6] test(show-dont-tell): the code-span URL cases pass the prefix they test against Co-Authored-By: Claude Fable 5.1 --- .github/scripts/show-dont-tell.test.mjs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs index 5b92f6c..df44c93 100644 --- a/.github/scripts/show-dont-tell.test.mjs +++ b/.github/scripts/show-dont-tell.test.mjs @@ -37,9 +37,10 @@ test('scanBody: an image only named in a code span or fence is not shown', () => 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('urls: `https://preview.example.com/`').kind, null); - assert.equal(scanBody('```\nhttps://preview.example.com/\n```').kind, null); - assert.equal(scanBody('`code` then https://preview.example.com/').kind, 'url'); + 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'); }); test('scanBody: images count anywhere', () => { From c0eae14c111dcc2a06494f1229a0ad09adbe668a Mon Sep 17 00:00:00 2001 From: "Solace (Mark) Brannan" Date: Sat, 17 Oct 2026 00:23:21 -0700 Subject: [PATCH 6/6] fix(show-dont-tell): an image or URL inside an HTML comment is not shown either A PR template's placeholder () survives into every body written over it and counted as an image. Comments are stripped with code spans and fences before the scan, with tests. Co-Authored-By: Claude Fable 5.1 --- .github/scripts/show-dont-tell.mjs | 5 +++-- .github/scripts/show-dont-tell.test.mjs | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/scripts/show-dont-tell.mjs b/.github/scripts/show-dont-tell.mjs index 29d8b37..5a6886d 100644 --- a/.github/scripts/show-dont-tell.mjs +++ b/.github/scripts/show-dont-tell.mjs @@ -59,8 +59,9 @@ const IMAGE = [ // attribution link (https://claude.com/claude-code) in every agent-written body. export function scanBody(body, urls = []) { const text = body || ''; - // Code spans and fences only describe a picture; they do not show one. - const shown = text.replace(/```[\s\S]*?```|`[^`\n]*`/g, ''); + // 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]*`|/g, ''); for (const re of IMAGE) { const m = re.exec(shown); if (m) return { kind: 'image', found: m[0] }; diff --git a/.github/scripts/show-dont-tell.test.mjs b/.github/scripts/show-dont-tell.test.mjs index df44c93..596e9ba 100644 --- a/.github/scripts/show-dont-tell.test.mjs +++ b/.github/scripts/show-dont-tell.test.mjs @@ -37,10 +37,14 @@ test('scanBody: an image only named in a code span or fence is not shown', () => 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('').kind, null); + assert.equal(scanBody('').kind, null); + assert.equal(scanBody('\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('', urls).kind, null); }); test('scanBody: images count anywhere', () => {