mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-11 23:49:01 +02:00
fix(design): Aside dump script stays single-quoted; redaction gate sized to the dump cap; doctrine made consistent
- The DOM-dump Aside block was the only double-quoted `aside repl` script in
the tree (to splice the function text), which put the agent-filled <url>
inside a double-quoted bash string: a same-origin href carrying $(...) would
run in the reviewer's shell when Phase 3 opened that page. The script is
single-quoted like every other Aside script and the function text enters
through a closed-quote segment ('"$_DUMP"'); the fallback line is
`$B js '('"$_DUMP"')()'`. A free test pins that no rendered Aside script
opens with a double quote.
- The persist block capped dumps at 10 MiB but ran gstack-redact with its
1 MiB default, so every real page between the two was deleted as
DOM_DUMP_REDACTION_BLOCKED; the gate passes --max-bytes at the dump cap and
blocks on any exit other than clean (0) or MEDIUM (2), so a redaction tool
that fails to run can no longer fall through to "persist".
- Dump hygiene removes <template> and <noscript> subtrees (invisible to the
attribute walk), inline on* handlers, and the cross-origin <link> nodes
already named in the note, so the file handed to the engine references no
remote stylesheet.
- Doctrine: the Codex design-voice prompts said "2-3 intentional motions"
against the one-authored-moment rule; the overused-display heading scoped
its ban to Persuade/Experience while the catalog and hard rules ban it
everywhere; design-consultation's Important Rule 4 still said "as primary";
design-html's blacklist header is now "Never include by default" with the
mockup/DESIGN.md/user-ask override the catalog grants; the slop gate honors
Decisions Log and Do's and Don'ts blessings like /review does; the landing
"poster" line says poster in stance, not type size; the design binary's
variant dials no longer flip light/dark for variety; gstack's DESIGN.md
rows name data labels (UI labels stay the DM Sans token) and call the
skill-bar fill and hovers functional transitions.
- design-review names how the base branch is found (gh pr view, then the
repo default; never main) for the source-mode scan and the diff-aware mode.
- frontend-scope matches the config globs at the repo root only, like the
bash arm; the parity test carries nested samples.
- Cleanups: renderCatalog's stale style option, an unused import, the
identity-map bannedFontNames, the checklist header's "same entries" claim,
the catalog header's consumer list, the orphaned main() docstring, the
plan doc's IIFE bullet. design-html's skeleton ceiling is re-measured
(54,184) for the two doctrine sentences.
Tests: AUTO-FIX rendering from the catalog, the E2E slice markers checked in
the free suite, the hygiene cases for templates/noscript/handlers/remote
links, and the review E2E counting detector rows separately from the seven
checklist plants.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
ae5a5298e0
commit
982a738663
@@ -176,7 +176,7 @@ const MOCKUP_NEVER_IDS = ['kicker-above-heading', 'icon-tile-stack', 'gradient-t
|
||||
|
||||
function designHtmlNeverIds(): string[] {
|
||||
const tmpl = fs.readFileSync(path.join(ROOT, 'design-html', 'SKILL.md.tmpl'), 'utf-8');
|
||||
const start = tmpl.indexOf('**Never include (AI slop blacklist):**');
|
||||
const start = tmpl.indexOf('**Never include by default (AI slop blacklist):**');
|
||||
expect(start).toBeGreaterThan(0);
|
||||
const block = tmpl.slice(start, tmpl.indexOf('\n\n', start + 10));
|
||||
const lines = block.split('\n').filter(l => l.startsWith('- '));
|
||||
|
||||
@@ -12,7 +12,7 @@ import * as path from 'path';
|
||||
import { spawnSync } from 'child_process';
|
||||
import {
|
||||
generateDesignChecklistMd, checklistSlopEntries,
|
||||
DESIGN_CHECKLIST_HEADER, DESIGN_CHECKLIST_TITLE, DESIGN_CHECKLIST_SLOP_HEADING,
|
||||
DESIGN_CHECKLIST_HEADER, DESIGN_CHECKLIST_TITLE, DESIGN_CHECKLIST_SLOP_HEADING, autoFixEntries,
|
||||
} from '../scripts/resolvers/design-checklist';
|
||||
import { DESIGN_SLOP_CATALOG, BANNED_FONTS } from '../lib/design-catalog';
|
||||
import { AI_SLOP_BLACKLIST } from '../scripts/resolvers/constants';
|
||||
@@ -41,6 +41,17 @@ describe('review/design-checklist.md is generated', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the Classification AUTO-FIX list renders every auto-fix catalog entry with its id', () => {
|
||||
const md = generateDesignChecklistMd();
|
||||
const block = md.slice(md.indexOf('**AUTO-FIX**'), md.indexOf('**ASK**'));
|
||||
const entries = autoFixEntries();
|
||||
expect(entries.length).toBeGreaterThan(0);
|
||||
for (const e of entries) {
|
||||
expect(e.tier).toBe('auto-fix');
|
||||
expect(block).toContain(`- [${e.impeccableId}] ${e.prose}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('category 1 renders every grep-detectable slop entry and every legacy line', () => {
|
||||
const md = generateDesignChecklistMd();
|
||||
const entries = checklistSlopEntries();
|
||||
@@ -99,6 +110,28 @@ describe('gen-skill-docs writes the checklist for the Claude host only', () => {
|
||||
}
|
||||
}, 300_000);
|
||||
|
||||
test('--host claude --out-dir also renders lib/dom-dump.js; a modified out-dir copy flips --dry-run to STALE', () => {
|
||||
const out = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-assets-'));
|
||||
try {
|
||||
const claude = runGen(['--host', 'claude', '--out-dir', out]);
|
||||
expect(claude.status).toBe(0);
|
||||
expect(claude.stdout).toContain('GENERATED: lib/dom-dump.js');
|
||||
const dump = fs.readFileSync(path.join(out, 'lib', 'dom-dump.js'), 'utf-8');
|
||||
expect(dump).toBe(fs.readFileSync(path.join(ROOT, 'lib', 'dom-dump.js'), 'utf-8'));
|
||||
const fresh = runGen(['--host', 'claude', '--out-dir', out, '--dry-run']);
|
||||
expect(fresh.stdout).toContain('FRESH: lib/dom-dump.js');
|
||||
expect(fresh.stdout).toContain('FRESH: review/design-checklist.md');
|
||||
fs.appendFileSync(path.join(out, 'review', 'design-checklist.md'), '\nhand edit\n');
|
||||
fs.writeFileSync(path.join(out, 'lib', 'dom-dump.js'), '// tampered\n');
|
||||
const stale = runGen(['--host', 'claude', '--out-dir', out, '--dry-run']);
|
||||
expect(stale.stdout).toContain('STALE: review/design-checklist.md');
|
||||
expect(stale.stdout).toContain('STALE: lib/dom-dump.js');
|
||||
expect(stale.status).not.toBe(0);
|
||||
} finally {
|
||||
fs.rmSync(out, { recursive: true, force: true });
|
||||
}
|
||||
}, 300_000);
|
||||
|
||||
test('--dry-run reports the checklist FRESH', () => {
|
||||
const r = runGen(['--dry-run']);
|
||||
expect(r.stdout).toContain('FRESH: review/design-checklist.md');
|
||||
|
||||
@@ -9,7 +9,8 @@
|
||||
* existing <style> nodes), <meta content> emptied (viewport kept), query
|
||||
* strings cut from every URL attribute, script bodies emptied, linked
|
||||
* stylesheets inlined with author hex restored, cross-origin sheets named in
|
||||
* the trailing note, inlined <link> nodes removed.
|
||||
* the trailing note and removed from the markup, inlined <link> nodes removed,
|
||||
* <template> and <noscript> subtrees dropped, inline on* handlers dropped.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
@@ -48,7 +49,8 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM (
|
||||
<input value="SECRET INPUT"><textarea>SECRET TEXT</textarea>
|
||||
<a href="/page?token=SECRET">link</a>
|
||||
<img src="/img.png?sig=SECRETSIG" srcset="/a.png?s=SECRETSET 1x, /b.png?s=SECRETSET2 2x">
|
||||
<form action="/submit?csrf=SECRETCSRF"><button formaction="/alt?f=SECRETFORM">go</button></form>
|
||||
<form action="/submit?csrf=SECRETCSRF"><button formaction="/alt?f=SECRETFORM" onclick="track('SECRETHANDLER')">go</button></form>
|
||||
<template><input value="SECRET TEMPLATE"><a href="/t?x=SECRETTPL">t</a></template><noscript><img src="/px.gif?id=SECRETNOSCRIPT"></noscript>
|
||||
<style>.inline { background: url("${big}") }</style>
|
||||
<div data-long="${'L'.repeat(40)}" data-short="ok" title="${big}">x</div>
|
||||
<img src="${big}">
|
||||
@@ -79,7 +81,7 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM (
|
||||
expect(html).toContain(`<style ${DOM_DUMP_STYLE_ATTR}=""`);
|
||||
expect(html).toContain('#6366f1');
|
||||
expect(html).not.toMatch(/<link[^>]*href="styles\.css"/);
|
||||
expect(html).toMatch(/<link[^>]*cross-origin\.css/);
|
||||
expect(html).not.toMatch(/<link[^>]*cross-origin\.css/); // named in the note, removed from the markup: the engine never sees a remote stylesheet
|
||||
expect(html).toContain(`<!-- ${DOM_DUMP_NOTE_PREFIX} `);
|
||||
expect(html).toContain('cross-origin stylesheets not resolved');
|
||||
expect(html).toContain('scripts stripped: 1');
|
||||
@@ -94,6 +96,12 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM (
|
||||
expect(html).not.toContain('SECRETSET');
|
||||
expect(html).not.toContain('SECRETCSRF');
|
||||
expect(html).not.toContain('SECRETFORM');
|
||||
expect(html).not.toContain('SECRETHANDLER');
|
||||
expect(html).not.toMatch(/ onclick=/);
|
||||
expect(html).not.toContain('SECRET TEMPLATE');
|
||||
expect(html).not.toContain('SECRETTPL');
|
||||
expect(html).not.toContain('SECRETNOSCRIPT');
|
||||
expect(html).not.toMatch(/<template|<noscript/);
|
||||
expect(html).toContain('srcset="/a.png 1x, /b.png 2x"');
|
||||
expect(html).not.toContain('L'.repeat(40));
|
||||
expect(html).toContain('data-short="ok"');
|
||||
@@ -105,5 +113,5 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM (
|
||||
fs.rmSync(site, { recursive: true, force: true });
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
}, 120_000);
|
||||
}, 180_000);
|
||||
});
|
||||
|
||||
+1
-1
@@ -36,7 +36,7 @@
|
||||
"impeccable-detect-dom-sample.json": {
|
||||
"mode": "dom",
|
||||
"dump": "review-eval-design-slop.dom.html",
|
||||
"dumpedWith": "browse goto http://127.0.0.1:<port>/index.html; browse js \"($(cat lib/dom-dump.js))()\" --out <tmp>/review-eval-design-slop.dom.html --raw (the arrow function in lib/dom-dump.js, called in the page; same form the skill renders for the fallback engine)",
|
||||
"dumpedWith": "browse goto http://127.0.0.1:<port>/index.html; browse js '('\"$(cat lib/dom-dump.js)\"')()' --out <tmp>/review-eval-design-slop.dom.html --raw (the arrow function in lib/dom-dump.js, called in the page; same form the skill renders for the fallback engine)",
|
||||
"command": "impeccable detect --json review-eval-design-slop.dom.html",
|
||||
"exit": 2,
|
||||
"stderrBytes": 0,
|
||||
|
||||
@@ -32,6 +32,8 @@ const SAMPLES: Array<[string, boolean]> = [
|
||||
['public/index.html', true],
|
||||
['tailwind.config.js', true],
|
||||
['postcss.config.cjs', true],
|
||||
['src/tailwind.config.js', false], // the bash glob is matched against the whole repo-relative path: root-level configs only
|
||||
['packages/ui/postcss.config.cjs', false],
|
||||
['app/assets/stylesheets/app.css', true],
|
||||
['lib/util/components/helper.rb', true],
|
||||
['lib/server.ts', false],
|
||||
|
||||
@@ -1835,7 +1835,7 @@ describe('DESIGN_HARD_RULES resolver', () => {
|
||||
|
||||
test('design-html blacklist lines carry catalog ids', () => {
|
||||
const content = fs.readFileSync(path.join(ROOT, 'design-html', 'SKILL.md'), 'utf-8');
|
||||
expect(content).toContain('**Never include (AI slop blacklist):**');
|
||||
expect(content).toContain('**Never include by default (AI slop blacklist):**');
|
||||
expect(content).toContain('Purple/blue gradients as default <!-- ai-color-palette -->');
|
||||
expect(content).toContain('lib/design-catalog.ts');
|
||||
});
|
||||
@@ -1888,7 +1888,7 @@ describe('DESIGN_DETECTOR resolver', () => {
|
||||
expect(c).toContain('scan --changed <base> --format gstack --host claude');
|
||||
expect(c).toContain('### DOM dump (DOM mode only');
|
||||
expect(c).toContain('data-gstack-dom-css');
|
||||
expect(c).toContain('$B js "($_DUMP)()" --out "$_TMP/{page}.dom.html" --raw');
|
||||
expect(c).toContain(`$B js '('"$_DUMP"')()' --out "$_TMP/{page}.dom.html" --raw`);
|
||||
expect(c).toContain('DOM_DUMP_OK');
|
||||
expect(c).toContain('DOM_DUMP_REDACTION_BLOCKED');
|
||||
expect(c).toContain('DOM_DUMP_TOO_LARGE');
|
||||
@@ -1905,10 +1905,31 @@ describe('DESIGN_DETECTOR resolver', () => {
|
||||
expect(c).not.toMatch(/```js\n/);
|
||||
expect(c).not.toContain('document.documentElement.cloneNode');
|
||||
expect(c).toContain('_DUMP=$(cat "$HOME/.claude/skills/gstack/lib/dom-dump.js")');
|
||||
expect(c).toContain('const html = await pg.evaluate($_DUMP);');
|
||||
expect(c).toContain(`const html = await pg.evaluate('"$_DUMP"');`);
|
||||
expect(c).toContain('_TMP=$(mktemp -d); _DUMP=$(cat "$HOME/.claude/skills/gstack/lib/dom-dump.js")');
|
||||
});
|
||||
|
||||
test('every rendered Aside script is single-quoted: a page-controlled <url> is never inside a double-quoted bash string', () => {
|
||||
const files = [...fs.readdirSync(ROOT).filter(d => fs.existsSync(path.join(ROOT, d, 'SKILL.md'))).map(d => path.join(ROOT, d, 'SKILL.md')),
|
||||
...fs.readdirSync(ROOT).flatMap(d => fs.existsSync(path.join(ROOT, d, 'sections')) ? fs.readdirSync(path.join(ROOT, d, 'sections')).filter(f => f.endsWith('.md')).map(f => path.join(ROOT, d, 'sections', f)) : [])];
|
||||
expect(files.length).toBeGreaterThan(10);
|
||||
for (const f of files) {
|
||||
const c = fs.readFileSync(f, 'utf-8');
|
||||
expect(c, path.relative(ROOT, f)).not.toMatch(/^aside repl "/m);
|
||||
}
|
||||
});
|
||||
|
||||
test('the E2E fixture slice markers exist in the rendered design skills (a template rename fails here, not in paid CI)', () => {
|
||||
const dr = designReview();
|
||||
const dh = fs.readFileSync(path.join(ROOT, 'design-html', 'SKILL.md'), 'utf-8');
|
||||
for (const [a, b] of [['**Design detector (optional, deterministic):**', '**Create output directories:**'], ['**Phase 0: mechanical scan**', '## Phases 1-6'], ['### DOM dump (DOM mode only', '### Auth Detection']]) {
|
||||
expect(sliceBetween(dr, a, b).length, `${a} .. ${b}`).toBeGreaterThan(100);
|
||||
}
|
||||
for (const [a, b] of [['**Design detector (optional, deterministic):**', '## Step 0: Input Detection'], ['### Slop Gate (bounded, never a loop)', '### Verification Screenshots']]) {
|
||||
expect(sliceBetween(dh, a, b).length, `${a} .. ${b}`).toBeGreaterThan(100);
|
||||
}
|
||||
});
|
||||
|
||||
test('design-html carries the probe and the bounded slop gate', () => {
|
||||
const c = designHtml();
|
||||
expect(c).toContain('gstack-design-detect.ts probe --host claude');
|
||||
@@ -2612,6 +2633,7 @@ describe('Factory generation (--host factory)', () => {
|
||||
// ─── Parameterized host smoke tests (config-driven) ─────────
|
||||
|
||||
import { ALL_HOST_CONFIGS, getExternalHosts } from '../hosts/index';
|
||||
import { sliceBetween } from './helpers/skill-fixture';
|
||||
|
||||
describe('Parameterized host smoke tests', () => {
|
||||
// Every external host was rendered up front by the module-level
|
||||
|
||||
@@ -158,6 +158,14 @@ describe('design_detector (auto|off, rejecting validator)', () => {
|
||||
expect(get('design_detector').out).toBe('auto');
|
||||
});
|
||||
|
||||
test('list and defaults enumerate design_detector', () => {
|
||||
for (const verb of ['list', 'defaults']) {
|
||||
const r = spawnSync('bash', [CONFIG_BIN, verb], { encoding: 'utf-8', timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: STATE } });
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toMatch(/design_detector:\s+auto/);
|
||||
}
|
||||
});
|
||||
|
||||
test('set off / set auto round-trip', () => {
|
||||
spawnSync('bash', [CONFIG_BIN, 'set', 'design_detector', 'off'], { encoding: 'utf-8', timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: STATE } });
|
||||
expect(get('design_detector').out).toBe('off');
|
||||
|
||||
@@ -678,7 +678,7 @@ export const CARVE_GUARDS: Record<string, CarveGuard> = {
|
||||
gateAfterStop: undefined, // operational skill, no plan-mode gate
|
||||
},
|
||||
behavioral: 'prompt',
|
||||
maxSkeletonBytes: 54_000, // + v1.82 design detector: {{DESIGN_DETECTOR}} probe + Step 4 slop gate + catalog id tags; measured 53_592
|
||||
maxSkeletonBytes: 54_300, // measured 54,184 (2026-09-08): the Slop Gate's Decisions-Log blessing clause and the blacklist header's override sentence, both review-cycle coherence fixes
|
||||
minUnionBytes: 57_500, // Phase 4 wave 4; measured union 58,682
|
||||
mustContain: ["Don't make me think", "Users scan, they don't read", 'The Goodwill Reservoir', 'PRETEXT API CHEATSHEET', 'Pattern 3: Text around obstacles'],
|
||||
},
|
||||
|
||||
@@ -136,7 +136,7 @@ describe('DOM_DUMP_SCRIPT contract', () => {
|
||||
expect(DOM_DUMP_SCRIPT).not.toContain('${');
|
||||
expect(DOM_DUMP_SCRIPT).not.toContain('`');
|
||||
// An arrow FUNCTION, not a self-calling IIFE: Aside's pg.evaluate(fn) runs it in
|
||||
// the page; the fallback engine calls it with `$B js "($_DUMP)()"`.
|
||||
// the page; the fallback engine calls it with `$B js '('"$_DUMP"')()'`.
|
||||
expect(DOM_DUMP_SCRIPT.trim().startsWith('() => {')).toBe(true);
|
||||
expect(DOM_DUMP_SCRIPT.trim().endsWith('}')).toBe(true);
|
||||
expect(() => new Function('return ' + DOM_DUMP_SCRIPT)).not.toThrow();
|
||||
@@ -151,7 +151,7 @@ describe('DOM_DUMP_SCRIPT contract', () => {
|
||||
expect(DOM_DUMP_SCRIPT).toContain(DOM_DUMP_NOTE_PREFIX);
|
||||
for (const rule of ['querySelectorAll("script")', 'querySelectorAll("textarea")', 'value.length > 32',
|
||||
'name === "content" && el.nodeName === "META"', 'cutQuery(value)', 'value.length > 1024',
|
||||
'gstack-stripped', 'cloneLinks[i].remove()', 'querySelectorAll("style")']) {
|
||||
'gstack-stripped', 'cloneLinks[i].remove()', 'querySelectorAll("style")', 'querySelectorAll("template, noscript")', 'name.indexOf("on") === 0']) {
|
||||
expect(DOM_DUMP_SCRIPT).toContain(rule);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -871,7 +871,7 @@ Do not run npx. Do not fix anything.`,
|
||||
recordE2E(evalCollector, '/design-review detector shim (dom)', 'Design review detector shim E2E (DOM mode)', result);
|
||||
expect(result.exitReason).toBe('success');
|
||||
const bash = result.toolCalls.filter(c => c.tool === 'Bash').map(c => String(c.input?.command ?? ''));
|
||||
expect(bash.some(c => c.includes('dom-dump.js') && c.includes('--out') && c.includes('--raw'))).toBe(true); // $B js "($_DUMP)()" with the file spliced in
|
||||
expect(bash.some(c => c.includes('dom-dump.js') && c.includes('--out') && c.includes('--raw'))).toBe(true); // $B js '('"$_DUMP"')()' with the file spliced in
|
||||
expect(bash.some(c => /gstack-design-detect\.ts scan /.test(c) && c.includes('dom/run1'))).toBe(true);
|
||||
expect(bash.some(c => /gstack-design-detect\.ts scan --changed/.test(c))).toBe(false);
|
||||
const dumps = fs.readdirSync(path.join(allowed, 'dom', 'run1')).filter(f => f.endsWith('.dom.html'));
|
||||
|
||||
@@ -274,10 +274,11 @@ Important: The design checklist should catch issues like blacklisted fonts, smal
|
||||
// Issue 7: 3-column feature grid — LOW
|
||||
if (review.includes('3-column') || review.includes('three-column') || review.includes('feature grid') || review.includes('icon') || review.includes('circle')) detected++;
|
||||
// Signal 8: the mechanical pass (fake impeccable engine via IMPECCABLE_BIN) surfaced a detector row
|
||||
if (review.includes('detector') || review.includes('[ai-color-palette]') || review.includes('[low-contrast]') || review.includes('impeccable')) detected++;
|
||||
const detectorSeen = review.includes('detector') || review.includes('[ai-color-palette]') || review.includes('[low-contrast]') || review.includes('impeccable');
|
||||
|
||||
console.log(`Design review detected ${detected}/8 planted signals`);
|
||||
expect(detected).toBeGreaterThanOrEqual(4);
|
||||
console.log(`Design review detected ${detected}/7 planted checklist signals; detector rows surfaced: ${detectorSeen}`);
|
||||
expect(detected).toBeGreaterThanOrEqual(4); // the LLM-checklist bar, unchanged by the detector
|
||||
expect(detectorSeen).toBe(true); // the fake engine's rows are deterministic; the review must carry them
|
||||
}
|
||||
}, CAPTURE_MS);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user