From 3f207247a8857b004cccc8859619564f2a661b16 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:55:37 +0000 Subject: [PATCH] fix(design): print and alternate stylesheets are not scanned as page CSS; no cross-run dump sweep; probe-state and design-system caveats in prose - The dump inlined every linked sheet's rules as active CSS, so a print sheet's 12pt black text or an alternate theme produced tiny-text and palette findings the user never sees; disabled and alternate sheets are skipped and a media-scoped sheet is wrapped in its @media block. - The cross-run dump sweep is gone: two same-day reviews shared REPORT_DIR and one run's sweep deleted the other's dumps mid-audit. Dumps stay per run, owner-only, deleted after Phase 9 unless --keep-dom (now defined in the prose), and an interrupted run's dumps wait for the user. - Prose: design-system-* rows in DOM mode compare the page to THIS repo's DESIGN.md and apply only to the repo's own app; an empty scan JSON with exit 0 means the probe state changed since Setup (read stderr); the persist block names a missing dump instead of mislabeling it as a redaction block. Co-Authored-By: Claude Fable 5.1 --- design-review/SKILL.md | 9 +++++---- lib/dom-dump-script.ts | 8 +++++++- lib/dom-dump.js | 8 +++++++- scripts/resolvers/design.ts | 9 +++++---- test/dom-dump-hygiene.test.ts | 6 ++++++ 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/design-review/SKILL.md b/design-review/SKILL.md index 237169b3c..cd3cf61e4 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -819,7 +819,7 @@ Remember `RUN_ID` and restate it literally in later blocks (each bash block is a _DJ=$(mktemp); bun --no-env-file run $HOME/.claude/skills/gstack/bin/gstack-design-detect.ts scan --changed --format gstack --host claude > "$_DJ"; echo "DETECT_EXIT_CODE=$?"; echo "DETECT_JSON=$_DJ" ``` -DOM mode never scans source (Rule 4): Phase 3 dumps each page's rendered DOM into `$REPORT_DIR/dom/$RUN_ID/` and scans once after the last page. Exit 2 means findings; exit 1 means a target could not be scanned (note which, move on); exit 3 is a gstack bug (`DESIGN_DETECT_INTERNAL_ERROR`: report it, never retry). Each rule in the `DETECT_TOP` block becomes one `FINDING-NNN` tagged `[rule-id]` with the printed impact and its location list, never one finding per hit. A detector hit is evidence, not a verdict: confirm it in the rendered page before it counts, drop it when DESIGN.md tokens bless the value, never pad the report with advisory rows. Phase 9 recomputes the same way (DOM mode re-dumps the affected pages after reload; source mode rescans the touched files) and Phase 10 reports `Detector: N → M`. When `IMPECCABLE_SKILL: present`, end each deferred finding with the `handoff=` command the scan printed (`/impeccable typeset`, `layout`, `colorize`, `harden`, `clarify`, `polish`, `animate`, `quieter`); recommend it, never open its files. +DOM mode never scans source (Rule 4): Phase 3 dumps each page's rendered DOM into `$REPORT_DIR/dom/$RUN_ID/` and scans once after the last page. Exit 2 means findings; exit 1 means a target could not be scanned (note which, move on); exit 0 with an empty `$_DJ` means the probe state changed since Setup (read the sentinel on stderr); exit 3 is a gstack bug (`DESIGN_DETECT_INTERNAL_ERROR`: report it, never retry). Each rule in the `DETECT_TOP` block becomes one `FINDING-NNN` tagged `[rule-id]` with the printed impact and its location list, never one finding per hit. A detector hit is evidence, not a verdict: confirm it in the rendered page before it counts, drop it when DESIGN.md tokens bless the value, never pad the report with advisory rows. Phase 9 recomputes the same way (DOM mode re-dumps the affected pages after reload; source mode rescans the touched files) and Phase 10 reports `Detector: N → M`. When `IMPECCABLE_SKILL: present`, end each deferred finding with the `handoff=` command the scan printed (`/impeccable typeset`, `layout`, `colorize`, `harden`, `clarify`, `polish`, `animate`, `quieter`); recommend it, never open its files. --- @@ -1093,11 +1093,12 @@ _TMP=$(mktemp -d); _DUMP=$(cat "$HOME/.claude/skills/gstack/lib/dom-dump.js") $B js '('"$_DUMP"')()' --out "$_TMP/{page}.dom.html" --raw && echo "DUMP=$_TMP/{page}.dom.html" ``` -Persist it into this run's directory, size-capped and redaction-checked: a HIGH finding, or a redaction tool that fails to run, skips the page, not the review; MEDIUM findings (emails, PII shapes on an authenticated page) persist owner-only (mode 600) and are deleted with the rest after Phase 9. Before the first dump of a run, remove earlier runs' dumps (`find "/dom" -mindepth 1 -maxdepth 1 -type d -exec rm -rf {} + 2>/dev/null`) unless the user passed `--keep-dom` this run. Each bash block is a fresh shell: restate the report directory and run id from Setup literally. +Persist it into this run's directory, size-capped and redaction-checked: a HIGH finding, or a redaction tool that fails to run, skips the page, not the review; MEDIUM findings (emails, PII shapes on an authenticated page) persist owner-only (mode 600) and are deleted with the rest after Phase 9 (`--keep-dom`, a design-review flag, keeps them; an interrupted run's dumps stay owner-only under their run id until you delete them). Each bash block is a fresh shell: restate the report directory and run id from Setup literally. ```bash _D="/{page}.dom.html"; _REPORT=""; _RUN="" -if [ "$(wc -c < "$_D")" -gt 10485760 ]; then echo "DOM_DUMP_TOO_LARGE: {page} $(wc -c < "$_D")"; rm -f "$_D" +if [ ! -s "$_D" ]; then echo "DOM_DUMP_MISSING: {page} (the dump script wrote nothing)" +elif [ "$(wc -c < "$_D")" -gt 10485760 ]; then echo "DOM_DUMP_TOO_LARGE: {page} $(wc -c < "$_D")"; rm -f "$_D" elif $HOME/.claude/skills/gstack/bin/gstack-redact --from-file "$_D" --max-bytes 10485760 >/dev/null 2>&1; _RC=$?; [ "$_RC" -ne 0 ] && [ "$_RC" -ne 2 ]; then echo "DOM_DUMP_REDACTION_BLOCKED: {page} redact-exit=$_RC"; rm -f "$_D" else mkdir -p "$_REPORT/dom/$_RUN" && cp "$_D" "$_REPORT/dom/$_RUN/" && chmod 600 "$_REPORT/dom/$_RUN/{page}.dom.html" && rm -f "$_D" && echo "DOM_DUMP_OK: {page}"; fi ``` @@ -1108,7 +1109,7 @@ After the LAST page's dump, scan the run directory once (source mode scanned in _DJ=$(mktemp); bun --no-env-file run $HOME/.claude/skills/gstack/bin/gstack-design-detect.ts scan --format gstack --host claude "/dom/" > "$_DJ"; echo "DETECT_EXIT_CODE=$?"; echo "DETECT_JSON=$_DJ" ``` -Say once in the report: "static scan of the rendered DOM; cross-origin CSS not resolved". A DOM-mode `file:line` points into `{page}.dom.html` and is approximate (HTML findings carry line 0); the `snippet` locates the element. Confirm each hit in the rendered page, never by hunting a source line. Dumps are deleted after Phase 9 unless the user passed `--keep-dom`. +Say once in the report: "static scan of the rendered DOM; cross-origin CSS not resolved". A DOM-mode `file:line` points into `{page}.dom.html` and is approximate (HTML findings carry line 0); the `snippet` locates the element. Confirm each hit in the rendered page, never by hunting a source line. `design-system-*` rows compare the page against THIS repository's DESIGN.md: keep them only when the page is this repository's own app. An empty `$_DJ` with exit 0 means the probe state changed since Setup: read the sentinel the scan printed on stderr. Dumps are deleted after Phase 9 unless the user passed `--keep-dom`. ### Auth Detection diff --git a/lib/dom-dump-script.ts b/lib/dom-dump-script.ts index 1d103f900..83da95e1c 100644 --- a/lib/dom-dump-script.ts +++ b/lib/dom-dump-script.ts @@ -50,8 +50,14 @@ export const DOM_DUMP_SCRIPT = String.raw`() => { liveLinks.forEach((link, i) => { const sheet = link.sheet; if (!sheet) return; + if (link.disabled || (link.getAttribute("rel") || "").indexOf("alternate") !== -1) { + if (cloneLinks[i]) cloneLinks[i].remove(); // not active CSS: never scanned as page styles + return; + } try { - const text = Array.from(sheet.cssRules).map((rule) => rule.cssText).join("\n"); + let text = Array.from(sheet.cssRules).map((rule) => rule.cssText).join("\n"); + const media = sheet.media && sheet.media.mediaText; + if (media && media !== "all") text = "@media " + media + " {\n" + text + "\n}"; // a print sheet stays a print sheet inlined.push("/* gstack-dom-dump: " + (sheet.href || "link") + " */\n" + text); if (cloneLinks[i]) cloneLinks[i].remove(); } catch (err) { diff --git a/lib/dom-dump.js b/lib/dom-dump.js index 230802f3e..c7702abf9 100644 --- a/lib/dom-dump.js +++ b/lib/dom-dump.js @@ -8,8 +8,14 @@ liveLinks.forEach((link, i) => { const sheet = link.sheet; if (!sheet) return; + if (link.disabled || (link.getAttribute("rel") || "").indexOf("alternate") !== -1) { + if (cloneLinks[i]) cloneLinks[i].remove(); // not active CSS: never scanned as page styles + return; + } try { - const text = Array.from(sheet.cssRules).map((rule) => rule.cssText).join("\n"); + let text = Array.from(sheet.cssRules).map((rule) => rule.cssText).join("\n"); + const media = sheet.media && sheet.media.mediaText; + if (media && media !== "all") text = "@media " + media + " {\n" + text + "\n}"; // a print sheet stays a print sheet inlined.push("/* gstack-dom-dump: " + (sheet.href || "link") + " */\n" + text); if (cloneLinks[i]) cloneLinks[i].remove(); } catch (err) { diff --git a/scripts/resolvers/design.ts b/scripts/resolvers/design.ts index d350d4485..c8a75538e 100644 --- a/scripts/resolvers/design.ts +++ b/scripts/resolvers/design.ts @@ -239,11 +239,12 @@ _TMP=$(mktemp -d); _DUMP=$(cat "${toShellPath(ctx.paths.skillRoot)}/${DOM_DUMP_F $B js '('"$_DUMP"')()' --out "$_TMP/{page}.dom.html" --raw && echo "DUMP=$_TMP/{page}.dom.html" \`\`\` -Persist it into this run's directory, size-capped and redaction-checked: a HIGH finding, or a redaction tool that fails to run, skips the page, not the review; MEDIUM findings (emails, PII shapes on an authenticated page) persist owner-only (mode 600) and are deleted with the rest after Phase 9. Before the first dump of a run, remove earlier runs' dumps (\`find "/dom" -mindepth 1 -maxdepth 1 -type d -exec rm -rf {} + 2>/dev/null\`) unless the user passed \`--keep-dom\` this run. Each bash block is a fresh shell: restate the report directory and run id from Setup literally. +Persist it into this run's directory, size-capped and redaction-checked: a HIGH finding, or a redaction tool that fails to run, skips the page, not the review; MEDIUM findings (emails, PII shapes on an authenticated page) persist owner-only (mode 600) and are deleted with the rest after Phase 9 (\`--keep-dom\`, a design-review flag, keeps them; an interrupted run's dumps stay owner-only under their run id until you delete them). Each bash block is a fresh shell: restate the report directory and run id from Setup literally. \`\`\`bash _D="/{page}.dom.html"; _REPORT=""; _RUN="" -if [ "$(wc -c < "$_D")" -gt ${DETECT_LIMITS.domDumpBytes} ]; then echo "${SENTINEL.DOM_DUMP_TOO_LARGE}: {page} $(wc -c < "$_D")"; rm -f "$_D" +if [ ! -s "$_D" ]; then echo "${SENTINEL.DOM_DUMP_MISSING}: {page} (the dump script wrote nothing)" +elif [ "$(wc -c < "$_D")" -gt ${DETECT_LIMITS.domDumpBytes} ]; then echo "${SENTINEL.DOM_DUMP_TOO_LARGE}: {page} $(wc -c < "$_D")"; rm -f "$_D" elif ${toShellPath(ctx.paths.binDir)}/gstack-redact --from-file "$_D" --max-bytes ${DETECT_LIMITS.domDumpBytes} >/dev/null 2>&1; _RC=$?; [ "$_RC" -ne 0 ] && [ "$_RC" -ne 2 ]; then echo "${SENTINEL.DOM_DUMP_REDACTION_BLOCKED}: {page} redact-exit=$_RC"; rm -f "$_D" else mkdir -p "$_REPORT/dom/$_RUN" && cp "$_D" "$_REPORT/dom/$_RUN/" && chmod 600 "$_REPORT/dom/$_RUN/{page}.dom.html" && rm -f "$_D" && echo "${SENTINEL.DOM_DUMP_OK}: {page}"; fi \`\`\` @@ -254,7 +255,7 @@ After the LAST page's dump, scan the run directory once (source mode scanned in _DJ=$(mktemp); bun --no-env-file run ${toShellPath(ctx.paths.binDir)}/gstack-design-detect.ts scan --format gstack --host ${ctx.host} "/dom/" > "$_DJ"${DETECT_EXIT_ECHO}; echo "${SENTINEL.DETECT_JSON}=$_DJ" \`\`\` -Say once in the report: "static scan of the rendered DOM; cross-origin CSS not resolved". A DOM-mode \`file:line\` points into \`{page}.dom.html\` and is approximate (HTML findings carry line 0); the \`snippet\` locates the element. Confirm each hit in the rendered page, never by hunting a source line. Dumps are deleted after Phase 9 unless the user passed \`--keep-dom\`. +Say once in the report: "static scan of the rendered DOM; cross-origin CSS not resolved". A DOM-mode \`file:line\` points into \`{page}.dom.html\` and is approximate (HTML findings carry line 0); the \`snippet\` locates the element. Confirm each hit in the rendered page, never by hunting a source line. \`design-system-*\` rows compare the page against THIS repository's DESIGN.md: keep them only when the page is this repository's own app. An empty \`$_DJ\` with exit 0 means the probe state changed since Setup: read the sentinel the scan printed on stderr. Dumps are deleted after Phase 9 unless the user passed \`--keep-dom\`. ### Auth Detection @@ -888,7 +889,7 @@ export function generateDesignDetector(ctx: TemplateContext, args?: string[]): s _DJ=$(mktemp); ${bin} scan --changed --format gstack --host ${ctx.host} > "$_DJ"${DETECT_EXIT_ECHO}; echo "${SENTINEL.DETECT_JSON}=$_DJ" \`\`\` -DOM mode never scans source (Rule 4): Phase 3 dumps each page's rendered DOM into \`$REPORT_DIR/dom/$RUN_ID/\` and scans once after the last page. Exit 2 means findings; exit 1 means a target could not be scanned (note which, move on); exit 3 is a gstack bug (\`${SENTINEL.INTERNAL_ERROR}\`: report it, never retry). Each rule in the \`${SENTINEL.DETECT_TOP}\` block becomes one \`FINDING-NNN\` tagged \`[rule-id]\` with the printed impact and its location list, never one finding per hit. A detector hit is evidence, not a verdict: confirm it in the rendered page before it counts, drop it when DESIGN.md tokens bless the value, never pad the report with advisory rows. Phase 9 recomputes the same way (DOM mode re-dumps the affected pages after reload; source mode rescans the touched files) and Phase 10 reports \`Detector: N → M\`. When \`${SENTINEL.SKILL}: present\`, end each deferred finding with the \`handoff=\` command the scan printed (\`/impeccable ${HANDOFF_COMMANDS.join('\`, \`')}\`); recommend it, never open its files.`; +DOM mode never scans source (Rule 4): Phase 3 dumps each page's rendered DOM into \`$REPORT_DIR/dom/$RUN_ID/\` and scans once after the last page. Exit 2 means findings; exit 1 means a target could not be scanned (note which, move on); exit 0 with an empty \`$_DJ\` means the probe state changed since Setup (read the sentinel on stderr); exit 3 is a gstack bug (\`${SENTINEL.INTERNAL_ERROR}\`: report it, never retry). Each rule in the \`${SENTINEL.DETECT_TOP}\` block becomes one \`FINDING-NNN\` tagged \`[rule-id]\` with the printed impact and its location list, never one finding per hit. A detector hit is evidence, not a verdict: confirm it in the rendered page before it counts, drop it when DESIGN.md tokens bless the value, never pad the report with advisory rows. Phase 9 recomputes the same way (DOM mode re-dumps the affected pages after reload; source mode rescans the touched files) and Phase 10 reports \`Detector: N → M\`. When \`${SENTINEL.SKILL}: present\`, end each deferred finding with the \`handoff=\` command the scan printed (\`/impeccable ${HANDOFF_COMMANDS.join('\`, \`')}\`); recommend it, never open its files.`; } if (mode === 'gate') { return `### Slop Gate (bounded, never a loop) diff --git a/test/dom-dump-hygiene.test.ts b/test/dom-dump-hygiene.test.ts index 16a4f6488..f7af19c2f 100644 --- a/test/dom-dump-hygiene.test.ts +++ b/test/dom-dump-hygiene.test.ts @@ -41,10 +41,13 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM ( }); const big = 'data:image/png;base64,' + 'A'.repeat(1500); fs.writeFileSync(path.join(site, 'styles.css'), '.hero { background: linear-gradient(135deg, #6366f1, #8b5cf6); } .x { background-image: url("' + big + '"); } .y { background: url("/y.png?token=SECRETCSS") }\n'); + fs.writeFileSync(path.join(site, 'print.css'), '.p { font-size: 4px }\n'); + fs.writeFileSync(path.join(site, 'alt.css'), '.alt { color: #ff00ff }\n'); fs.writeFileSync(path.join(site, 'index.html'), ` + link @@ -109,6 +112,9 @@ describe.skipIf(!BROWSE || !POSIX || !OPTED_IN)('lib/dom-dump.js in a real DOM ( expect(html).toContain('url("/y.png")'); expect(html).not.toContain('SECRETSRCDOC'); expect(html).not.toContain('SECRETXLINK'); + expect(html).toMatch(/@media print \{[\s\S]*font-size: 4px[\s\S]*\}/); // a print sheet is scanned as print CSS, not as the page's styles + expect(html).not.toContain('#ff00ff'); // an alternate stylesheet is not active CSS + expect(html).not.toMatch(/]*alt\.css/); expect(html).toContain('srcset="/a.png 1x, /b.png 2x"'); expect(html).not.toContain('L'.repeat(40)); expect(html).toContain('data-short="ok"');