From 5862765a20cd751b73e54fe85ffebd9e4da01e30 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 22:39:20 -0800 Subject: [PATCH] feat(diff-scope): template UI in .ts/.js counts as frontend; receipt records the static design pass (GST-1718) (#4112) * feat(diff-scope): template UI in .ts/.js counts as frontend; receipt records the static design pass (GST-1718) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: regenerate ship goldens for the static design-review line (GST-1718) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Review-Finding: p1/specialist-batched#1 reader=specialist Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: count edits inside tagged templates and spaced template tags as frontend Review-Finding: v1/outside-voice#1 reader=outside-voice Review-Finding: v1/adversarial-native#1 reader=adversarial-native Review-Finding: v1/outside-voice#2 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: one set comparison per template file instead of a grep per changed line Review-Finding: p2/red-team#2 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): GST-1718 fragment covers edits inside tagged templates Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(ship-policy): design record wording matches the in-template rule Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-diff-scope | 93 +++++++++++++ bin/gstack-ship-receipt | 1 + .../capy-gst-1718-template-ui-scope.md | 1 + docs/ship-policy.md | 14 +- lib/frontend-scope.ts | 3 + lib/headless-artifacts.ts | 3 + lib/ship-preflight.ts | 2 +- lib/ship-receipt-depth.ts | 4 +- lib/ship-receipt.ts | 2 +- ship/SKILL.md | 2 +- ship/SKILL.md.tmpl | 2 +- ship/sections/pr-body.md | 2 +- ship/sections/pr-body.md.tmpl | 2 +- test/diff-scope.test.ts | 131 ++++++++++++++++++ test/fixtures/golden/agy-ship-SKILL.md | 2 +- test/fixtures/golden/claude-ship-SKILL.md | 2 +- test/fixtures/golden/codex-ship-SKILL.md | 2 +- test/fixtures/golden/factory-ship-SKILL.md | 2 +- test/ship-receipt-depth.test.ts | 13 ++ .../ship/gst-1718-template-ui-scope.json | 4 + 20 files changed, 273 insertions(+), 14 deletions(-) create mode 100644 changelog.d/capy-gst-1718-template-ui-scope.md create mode 100644 test/size-ledger/ship/gst-1718-template-ui-scope.json diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index afdddbb06..8c63dda07 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -349,6 +349,99 @@ for f in ${FILES_LIST[@]+"${FILES_LIST[@]}"}; do fi done +# Template UI (GST-1718): a screen written as a tagged template inside a .ts or +# .js file (Lit, lit-html, htm, uhtml) is frontend when a changed line in that +# file carries markup. qm#3186 rendered its persona page this way and read +# SCOPE_FRONTEND=false, so /ship's design checks skipped it silently. This arm +# sets FRONTEND only; the backend arm above already counted the file, so a Lit +# edit reads both, as .dart does. lib/frontend-scope.ts stays path-only (the +# design detector scans paths). A template file holds a tagged template at the +# head or the base (a deleted screen counts). A changed line counts when it sits +# inside a tagged template on its side of the diff (CSS declarations and ${} +# bindings included), or carries markup: a tagged template, an element tag not +# preceded by an identifier (Promise is not markup), class= or style=; +# comment lines never count as markup. Paths are NUL-delimited, as in _collect. +# Every git call tolerates failure under set -e. +TEMPLATE_TAG_RE='(^|[^A-Za-z0-9_$.])(html|css|svg)[[:space:]]*`' +MARKUP_LINE_RE="${TEMPLATE_TAG_RE}|(^|[^A-Za-z0-9_$])]|$)|(^|[^A-Za-z0-9_-])(class|style)=" +if [ "$FRONTEND" = false ]; then + TPL_CANDIDATES=() + for f in "${FILES_LIST[@]}"; do + case "$f" in + *.test.*|*.spec.*|test/*|tests/*|spec/*|__tests__/*|cypress/*|e2e/*) continue ;; + *.ts|*.js|*.mjs|*.cjs|*.mts|*.cts) [[ "$f" =~ $TEST_PATH_RE ]] || TPL_CANDIDATES+=("$f") ;; + esac + done + if [ "${#TPL_CANDIDATES[@]}" -gt 0 ]; then + _root=$(git rev-parse --show-toplevel 2>/dev/null || pwd) + _g() { git --literal-pathspecs -C "$_root" "$@" 2>/dev/null || true; } + _tpl_lines() { + awk ' + function top() { return sp ? st[sp] : "" } + { + line = $0; n = length(line); hit = (tags > 0); i = 1 + while (i <= n) { + c = substr(line, i, 1); t = top() + if (blk) { if (substr(line, i, 2) == "*/") { blk = 0; i += 2 } else i++; continue } + if (t == "T" || t == "U") { + if (c == "\\") { i += 2; continue } + if (c == "`") { if (t == "T") tags--; sp--; i++; continue } + if (substr(line, i, 2) == "${") { st[++sp] = "E"; dep[sp] = 0; i += 2; continue } + i++; continue + } + if (c == "\"" || c == "'\''") { q = c; i++; while (i <= n && substr(line, i, 1) != q) { if (substr(line, i, 1) == "\\") i++; i++ } i++; continue } + if (substr(line, i, 2) == "//") break + if (substr(line, i, 2) == "/*") { blk = 1; i += 2; continue } + if (c == "{" && t == "E") { dep[sp]++; i++; continue } + if (c == "}" && t == "E") { if (dep[sp] == 0) sp--; else dep[sp]--; i++; continue } + if (c == "`") { + if (substr(line, 1, i - 1) ~ /(^|[^A-Za-z0-9_$.])(html|css|svg)[ \t]*$/) { st[++sp] = "T"; tags++; hit = 1 } else st[++sp] = "U" + i++; continue + } + i++ + } + if (hit) print NR + }' + } + _hunk_lines() { + awk '/^@@ /{ split($2, o, ","); split($3, a, ","); os = substr(o[1], 2) + 0; oc = (o[2] == "") ? 1 : o[2] + 0; ns = substr(a[1], 2) + 0; nc = (a[2] == "") ? 1 : a[2] + 0 + for (k = 0; k < oc; k++) print "o " os + k; for (k = 0; k < nc; k++) print "n " ns + k }' + } + _mb=$(git merge-base "$BASE_SHA" "${HEAD_REF:-HEAD}" 2>/dev/null || echo "$BASE_SHA") + TPL_FILES=() + if [ -n "$HEAD_REF" ]; then + while IFS= read -r -d '' f; do [ -n "$f" ] && TPL_FILES+=("${f#"$HEAD_REF":}"); done < <(_g grep -l -z -E -e "$TEMPLATE_TAG_RE" "$HEAD_REF" -- "${TPL_CANDIDATES[@]}") + else + while IFS= read -r -d '' f; do [ -n "$f" ] && TPL_FILES+=("$f"); done < <(_g grep -l -z -E --untracked -e "$TEMPLATE_TAG_RE" -- "${TPL_CANDIDATES[@]}") + fi + while IFS= read -r -d '' f; do [ -n "$f" ] && TPL_FILES+=("${f#"$_mb":}"); done < <(_g grep -l -z -E -e "$TEMPLATE_TAG_RE" "$_mb" -- "${TPL_CANDIDATES[@]}") + if [ "${#TPL_FILES[@]}" -gt 0 ]; then + if [ -n "$HEAD_REF" ]; then + _d=$(_g diff -U0 "$BASE_SHA...$HEAD_REF" -- "${TPL_FILES[@]}") + else + _d=$(_g diff -U0 "$_mb" -- "${TPL_FILES[@]}") + [ -n "$(_g ls-files --others --exclude-standard -- "${TPL_FILES[@]}")" ] && FRONTEND=true + fi + _changed=$(grep -E '^[+-]' <<<"$_d" | grep -v -E '^(\+\+\+|---) (a/|b/|/dev/null)' | grep -v -E '^[+-][[:space:]]*(//|/\*|\*)' || true) + if [ "$FRONTEND" = false ] && [ -n "$_changed" ] && grep -q -E -e "$MARKUP_LINE_RE" <<<"$_changed"; then FRONTEND=true; fi + for f in "${TPL_FILES[@]}"; do + [ "$FRONTEND" = true ] && break + _old=$(_g show "$_mb:$f" | _tpl_lines) + if [ -n "$HEAD_REF" ]; then + _fd=$(_g diff -U0 "$BASE_SHA...$HEAD_REF" -- "$f") + _new=$(_g show "$HEAD_REF:$f" | _tpl_lines) + else + _fd=$(_g diff -U0 "$_mb" -- "$f") + _new= + if [ -f "$_root/$f" ]; then _new=$(_tpl_lines < "$_root/$f"); fi + fi + if awk 'FNR == NR { s[$0] = 1; next } ($0 in s) { f = 1; exit } END { exit !f }' \ + <(sed 's/^/o /' <<<"$_old"; sed 's/^/n /' <<<"$_new") <(_hunk_lines <<<"$_fd"); then FRONTEND=true; fi + done + fi + fi +fi + _print_flags # Changed files but ZERO category matches (#2526): a classifier bug, an diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index e782202e8..3c190a097 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -332,6 +332,7 @@ function cmdWrite(rest: string[], json: boolean): number { ...pregate, issues: issuesFile ? receiptIssuesFrom(issuesFile) : undefined, approval: approval.kind === 'ok' ? approval.approval : undefined, review_loop: reviewLoop, ...(docs?.ok ? { docs: docs.docs } : {}), ...product, gstack_version: installedGstackVersion(), review_depth: reviewDepth, review_loop_exception: loopException?.ok ? loopException.exception : undefined, ruled_notes: ruledNotes.length ? ruledNotes : undefined, mechanical: mechanical.length ? mechanical : undefined, review_targeted: targeted?.ok ? targeted.targeted : undefined, + design: review.frontend ? { mode: 'static', reason: 'no preview declared' } : undefined, base_red: pause ? receiptBaseRed(pause) : undefined, review_exclude: receiptExclude(review.exclude), no_stop: noStopArg(flag(rest, '--no-stop')), diff --git a/changelog.d/capy-gst-1718-template-ui-scope.md b/changelog.d/capy-gst-1718-template-ui-scope.md new file mode 100644 index 000000000..d0eb07320 --- /dev/null +++ b/changelog.d/capy-gst-1718-template-ui-scope.md @@ -0,0 +1 @@ +- **/ship now notices a screen written as a Lit or htm template inside a .ts or .js file.** `gstack-diff-scope` counted only CSS, .tsx, views and similar files as frontend, so a pull request that rendered its UI from tagged `html`` templates in plain TypeScript read `SCOPE_FRONTEND=false`, and /ship skipped its design-lite check and the design reviewer without saying so. qm#3186 shipped three visible spacing problems that way. A changed non-test .ts/.js file that holds a tagged `html`, `css` or `svg` template now counts as frontend when a changed line sits inside one of those templates (a CSS declaration or a `${}` binding counts) or carries markup. Logic-only edits, comments and TypeScript generics don't count. SCOPE_BACKEND is unchanged, so those files still get the backend reviewers. Replayed on the last 100 merged gstack PRs and the last 100 merged qm PRs, no scope flag was lost, and only SCOPE_FRONTEND gained (qm only). On a rendered-UI diff the ship receipt now records `design: { mode: static, reason: no preview declared }`, and the PR body's Design Review line says the same: no rendered /design-review runs inside /ship yet. (GST-1718) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index f161f4b80..2b7fc5094 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1264,8 +1264,8 @@ written by the pr-body section of `/ship` after the body is composed. The ```` ```gstack-ship-receipt ```` block is the handoff record: identifiers first (`pr`, `head`, `version`, `base`), then the evidence (`gate`, `ci`, `head_tree`, `product_patch_id`, `gstack_version`, `review_depth`, `pregate`, -`issues`, `approval`, `docs`, `review_loop`, `review_loop_exception`, `carry`, -`no_stop`), validated +`issues`, `approval`, `docs`, `review_loop`, `review_loop_exception`, `design`, +`carry`, `no_stop`), validated against the `ship-receipt` schema (`gstack-artifact schema ship-receipt`) and read back by `gstack-ship-receipt read`, `result`, `carry` and the queue. @@ -1281,6 +1281,16 @@ and `read`, `result`, `gstack-ship-queue ready`, `train build` and `verify but no version source records no `version` at all (the audit still says what it looked for); pass `--version` to name one. +**`design`.** On a rendered-UI diff (`gstack-diff-scope` reports +`SCOPE_FRONTEND=true` for the committed range, which counts CSS, views, +components and, since GST-1718, Lit or htm tagged templates in .ts/.js files +whose changed lines sit inside a tagged template or carry markup), write records +`{ "mode": "static", "reason": "no preview declared" }`: /ship ran only the +static design-lite check, and the PR body's `## Design Review` line says the +same. It is advisory and nothing refuses on it. Other diffs record no +`design` field, and receipts written before it stay valid. A carry copies it +unchanged. + **`docs`.** Step 14.5's outcome, from `gstack-ship-receipt write --docs `: `{ status: updated | current | blocked, scope, exception? }`, where `scope` names the docs roots the audit covered and `exception` is diff --git a/lib/frontend-scope.ts b/lib/frontend-scope.ts index 880741fba..19e1905d6 100644 --- a/lib/frontend-scope.ts +++ b/lib/frontend-scope.ts @@ -5,6 +5,9 @@ // SCOPE_FRONTEND); test/frontend-scope.test.ts pins the two against the same // sample paths so they cannot drift. bin/gstack-design-detect.ts uses this to // derive `scan --changed ` targets without consuming a shell-split list. +// The bash script's template-UI content arm (GST-1718: Lit or htm templates in +// .ts/.js) reads file contents and is not mirrored here; the detector scans +// paths, so its targets stay path-only. const EXTENSIONS = new Set([ '.css', '.scss', '.less', '.sass', '.pcss', diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index f96667533..559896d0c 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -109,6 +109,8 @@ export interface ShipReceipt { thread?: string; tree?: string; gate_reuse?: string; policy?: string; queue_mode?: string; preregistration?: string; history?: string; // C-pregate (C8): the pre-gate's verdict against this head and its remote obligations (publication requires their receipts). pregate?: string; requires_remote?: string[]; + /** GST-1718: the design pass a rendered-UI diff got; phase 1 records only the static design-lite check, with why no rendered pass ran. */ + design?: { mode: 'static'; reason: string }; /** Approval classes (lib/approval-citations.ts): source, status, one record per declared class with hits, report-only built-in hits. */ approval?: Record; // C-pregate (C6): issue links the body carries, by intent. @@ -276,6 +278,7 @@ export const ARTIFACT_SCHEMAS = { review_loop_exception: { type: 'object', required: ['accepted_by', 'date', 'via', 'reason'], properties: { accepted_by: str, date: str, via: str, reason: str, fix_check: { type: 'object', required: ['head', 'findings'], properties: { head: str, findings: { type: 'integer', minimum: 0 }, files: strList } }, second_pass: { type: 'object', required: ['head', 'deferred'], properties: { head: str, deferred: strList } } } }, ruled_notes: { type: 'array', items: { type: 'object', required: ['finding', 'head', 'by', 'reason', 'at', 'approval'], properties: { finding: str, head: str, by: str, reason: str, at: str, approval: str } } }, mechanical: strList, + design: { type: 'object', required: ['mode', 'reason'], properties: { mode: { enum: ['static'] }, reason: str } }, no_stop: { type: 'array', items: { type: 'object', required: ['class', 'reason', 'evidence'], properties: { class: { enum: [...NO_STOP_CLASSES] }, reason: str, evidence: str } } }, issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } }, }, diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index c54d4a226..d9489e2af 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -113,7 +113,7 @@ function integrate(cwd: string, base: string, run: string | undefined, r: Report const scope = bin(cwd, 'gstack-diff-scope', [`origin/${base}`], 60_000).stdout; const scopeError = /^SCOPE_ERROR=(?!unmatched)(\S+)/m.exec(scope)?.[1]; if (scopeError) r.add(`SCOPE: unknown (gstack-diff-scope origin/${base}: SCOPE_ERROR=${scopeError})`); - else r.add(`SCOPE: frontend=${/SCOPE_FRONTEND=true/.test(scope)}${/SCOPE_FRONTEND=true/.test(scope) ? ' (Step 9 runs the design lite check; mention /design-review when no design review exists)' : ''}`); + else r.add(`SCOPE: frontend=${/SCOPE_FRONTEND=true/.test(scope)}${/SCOPE_FRONTEND=true/.test(scope) ? ' (Step 9 runs the static design lite check only, no preview declared; mention /design-review when no design review exists)' : ''}`); if (!merge(cwd, base, r)) return false; const marker = run && shipRunMarker(cwd); try { if (marker) fs.writeFileSync(marker, `${run}\n`); } catch (e) { r.stop(`cannot record /ship run ${run} in ${marker} (${firstLine((e as Error).message)}): the base is merged, but a later full preflight could merge again under this run's reviews; fix that, then rerun`); return false; } diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 6aec39a34..7fc3ceedc 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -104,7 +104,7 @@ export function receiptDepthAtHead(repoRoot: string, base: string, binDir: strin * receiptDepthAtHead plus the exclusion the head's review read under: the depth classifies excluded paths as docs (GST-508). * `declared` (write --declare-risk, GST-1129) joins the detected risk classes and only raises; the record then keeps both lists. */ -export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion; tagged: ReadonlySet | null } { +export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion; tagged: ReadonlySet | null; frontend: boolean } { const branch = base.replace(/^origin\//, ''); const baseSha = baseCommit(repoRoot, `origin/${branch}`); const baseRev = baseSha.ok ? baseSha.out : branch; @@ -124,7 +124,7 @@ export function receiptReviewAtHead(repoRoot: string, base: string, binDir: stri } const exclude = exclusionAt(repoRoot, ref, sha, head, c, paths, 'gstack-ship-receipt'); const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged, ...riskPathsOf(review), declared, ...(exclude.state === 'active' ? { excluded: exclude.excluded } : {}) }); - return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review), ...(d.declared.length ? { declared: d.declared, detected: d.detected } : {}) }, exclude, tagged: paths }; + return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review), ...(d.declared.length ? { declared: d.declared, detected: d.detected } : {}) }, exclude, tagged: paths, frontend: scope.SCOPE_FRONTEND === 'true' }; } /** diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 92fdd409b..0bfb37440 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -22,7 +22,7 @@ const KEY_ORDER: Array = [ 'schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'ci_carried', 'spend_usd', 'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history', 'head_tree', 'product_patch_id', 'gstack_version', 'review_depth', 'pregate', 'requires_remote', 'issues', 'approval', 'docs', 'review_loop', - 'review_loop_exception', 'ruled_notes', 'mechanical', 'review_targeted', 'review_exclude', 'carry', 'base_red', 'flaky_exception', 'no_stop', + 'review_loop_exception', 'ruled_notes', 'mechanical', 'review_targeted', 'design', 'review_exclude', 'carry', 'base_red', 'flaky_exception', 'no_stop', ]; /** Keys in the fixed order so identifiers lead; undefined values are dropped. */ diff --git a/ship/SKILL.md b/ship/SKILL.md index 688f0b977..cbf02c15f 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -424,7 +424,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index c96726c0d..1b3e755c9 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -154,7 +154,7 @@ act on each `STOP:` and `NEXT:` line. An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index fcb16d98b..64795eb78 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -120,7 +120,7 @@ proof, fixes and blocked/inconclusive/not-run coverage. Never present stale or unavailable results as passing.> ## Design Review - + diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index f6853c840..625d8f7bd 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -111,7 +111,7 @@ proof, fixes and blocked/inconclusive/not-run coverage. Never present stale or unavailable results as passing.> ## Design Review - + diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 8d1e12650..b8fb6aa5b 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -542,3 +542,134 @@ describe('uncommitted work is visible (#2299)', () => { expect(vars.SCOPE_DOCS).toBe('true'); // pre-fix: git's octal quoting defeated *.md }); }); + +describe('gstack-diff-scope template UI (GST-1718)', () => { + const LIT_BASE = [ + "import { html } from 'lit';", + 'export function rowTpl(name: string) {', + ' return html`
${name}
`;', + '}', + 'export async function load(): Promise {', + ' await fetch("/api/personas");', + '}', + '', + ].join('\n'); + function templateRepo(): { dir: string; git: (...a: string[]) => ReturnType } { + const dir = createRepo([]); + const git = (...a: string[]) => spawnSync('git', a, { cwd: dir, stdio: 'pipe', timeout: 5000 }); + git('checkout', '-q', 'main'); + mkdirSync(join(dir, 'plugins', 'web-ui', 'src'), { recursive: true }); + mkdirSync(join(dir, 'src', 'personas'), { recursive: true }); + writeFileSync(join(dir, 'plugins', 'web-ui', 'src', 'personas.ts'), LIT_BASE); + writeFileSync(join(dir, 'src', 'personas', 'persona-service.ts'), 'export const svc = 1;\n'); + git('add', '.'); git('commit', '-qm', 'base screens'); + git('checkout', '-q', '-b', 'topic'); + return { dir, git }; + } + const edit = (dir: string, from: string, to: string) => { + const p = join(dir, 'plugins', 'web-ui', 'src', 'personas.ts'); + writeFileSync(p, readFileSync(p, 'utf8').replace(from, to)); + }; + + // Value: protects=a screen written as a Lit template in a .ts file reads as frontend, so /ship's design-lite check and + // the design specialist run (qm#3186 read SCOPE_FRONTEND=false and both skipped silently); fails_when=the content arm + // is missing, or it clears SCOPE_BACKEND for the same file; seam=a synthetic Lit file at qm#3186's path, committed and + // read through --head and through the working tree; no_claim=no qm source is used. + test('a markup edit in a Lit template file is frontend and still backend, on --head and in the working tree', () => { + const { dir, git } = templateRepo(); + edit(dir, '${name}', '${name}Only owner'); + expect(runScope(dir).SCOPE_FRONTEND).toBe('true'); + writeFileSync(join(dir, 'src', 'personas', 'persona-service.ts'), 'export const svc = 2;\n'); + git('add', '.'); git('commit', '-qm', 'persona access'); + const head = git('rev-parse', 'HEAD').stdout.toString().trim(); + const out = spawnSync('bash', [SCRIPT, 'main', '--head', head], { cwd: dir, stdio: 'pipe', timeout: 5000 }).stdout.toString(); + expect(out).toContain('SCOPE_FRONTEND=true'); + expect(out).toContain('SCOPE_BACKEND=true'); + }); + + // Value: protects=logic edits in a file that happens to hold templates, and TypeScript generics, stay backend-only, so + // the design reviewers do not fire on every touch of a UI module; fails_when=the arm counts any changed line in a + // template file, or reads Promise as a tag; seam=none + test('a logic-only or generic-only edit in a template file is not frontend', () => { + const { dir } = templateRepo(); + edit(dir, 'await fetch("/api/personas");', 'await fetch("/api/personas?v=2");'); + expect(runScope(dir).SCOPE_FRONTEND).toBe('false'); + edit(dir, 'Promise', 'Promise>'); + const vars = runScope(dir); + expect(vars.SCOPE_FRONTEND).toBe('false'); + expect(vars.SCOPE_BACKEND).toBe('true'); + }); + + // Value: protects=test fixtures that render templates do not turn on the design reviewers, while a brand-new or a + // deleted screen does; fails_when=test paths are scanned, untracked files are ignored, or a deleted template file + // reads as no change; seam=none + test('a template test file is not frontend; an untracked or deleted template file is', () => { + const t = templateRepo(); + mkdirSync(join(t.dir, 'plugins', 'web-ui', 'test'), { recursive: true }); + writeFileSync(join(t.dir, 'plugins', 'web-ui', 'test', 'personas-page.test.ts'), LIT_BASE); + writeFileSync(join(t.dir, 'plugins', 'web-ui', 'src', 'row.test.ts'), LIT_BASE); + expect(runScope(t.dir).SCOPE_FRONTEND).toBe('false'); + writeFileSync(join(t.dir, 'plugins', 'web-ui', 'src', 'picker.ts'), LIT_BASE); + expect(runScope(t.dir).SCOPE_FRONTEND).toBe('true'); + const d = templateRepo(); + d.git('rm', '-q', 'plugins/web-ui/src/personas.ts'); + d.git('commit', '-qm', 'drop screen'); + expect(runScope(d.dir).SCOPE_FRONTEND).toBe('true'); + }); + + // Value: protects=a Lit screen with a non-ASCII file name still reads as frontend (git grep octal-quotes such paths + // unless -z, and the quoted path matched no pathspec), and a in a comment does not; fails_when=the + // template arm reads git grep without -z, or counts comment lines as markup; seam=none + test('a non-ASCII template file is frontend on --head and in the working tree; a comment placeholder is not', () => { + const { dir, git } = templateRepo(); + edit(dir, 'export async function load', '// usage: load --head \nexport async function load'); + expect(runScope(dir).SCOPE_FRONTEND).toBe('false'); + git('checkout', '-q', '.'); + writeFileSync(join(dir, 'plugins', 'web-ui', 'src', 'café.ts'), LIT_BASE); + expect(runScope(dir).SCOPE_FRONTEND).toBe('true'); + git('add', '.'); git('commit', '-qm', 'cafe screen'); + const head = git('rev-parse', 'HEAD').stdout.toString().trim(); + const out = spawnSync('bash', [SCRIPT, 'main', '--head', head], { cwd: dir, stdio: 'pipe', timeout: 5000 }).stdout.toString(); + expect(out).toContain('SCOPE_FRONTEND=true'); + }); + + // Value: protects=a spacing or binding edit inside a multiline Lit template (no tag, element or class= on the changed + // line) still reads as frontend, as does a screen tagged with whitespace before the backtick; fails_when=the arm + // tests changed lines alone instead of their place inside a tagged template, or the tag regex requires the backtick + // right after html/css/svg; seam=none + test('a CSS-only or binding-only edit inside a multiline template is frontend; so is html `...` with a space', () => { + const { dir, git } = templateRepo(); + git('checkout', '-q', 'main'); + const card = join(dir, 'plugins', 'web-ui', 'src', 'card.ts'); + writeFileSync(card, [ + "import { LitElement, css, html } from 'lit';", + 'export class Card extends LitElement {', + ' static styles = css`', + ' .row { padding: 4px; }', + ' `;', + ' render() {', + ' return html`', + '
', + ' ${this.label}', + '
`;', + ' }', + '}', + '', + ].join('\n')); + git('add', '.'); git('commit', '-qm', 'card'); + git('checkout', '-q', '-B', 'topic', 'main'); + const swap = (from: string, to: string) => writeFileSync(card, readFileSync(card, 'utf8').replace(from, to)); + swap('padding: 4px;', 'padding: 12px;'); + expect(runScope(dir).SCOPE_FRONTEND).toBe('true'); + git('commit', '-qam', 'spacing'); + const head = git('rev-parse', 'HEAD').stdout.toString().trim(); + const out = spawnSync('bash', [SCRIPT, 'main', '--head', head], { cwd: dir, stdio: 'pipe', timeout: 5000 }).stdout.toString(); + expect(out).toContain('SCOPE_FRONTEND=true'); + git('reset', '-q', '--hard', 'HEAD~1'); + swap('${this.label}', '${this.title}'); + expect(runScope(dir).SCOPE_FRONTEND).toBe('true'); + git('checkout', '-q', '.'); + writeFileSync(join(dir, 'plugins', 'web-ui', 'src', 'row.ts'), "import { html } from 'lit';\nexport const row = html `

Hi

`;\n"); + expect(runScope(dir).SCOPE_FRONTEND).toBe('true'); + }); +}); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index fa4786bfe..473ed05b7 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -425,7 +425,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 688f0b977..cbf02c15f 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -424,7 +424,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index d1ba0868e..ce97503b8 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -424,7 +424,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index babfb52cb..dce37aa13 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -426,7 +426,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from -`gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." +`gstack-diff-scope `) (Lit or htm templates in .ts/.js count) with no design review: mention "Design Review: static (no preview declared) — Step 9 runs the lite check only; consider /design-review on a served page for a visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/ship-receipt-depth.test.ts b/test/ship-receipt-depth.test.ts index bd9403a7d..67ff214c7 100644 --- a/test/ship-receipt-depth.test.ts +++ b/test/ship-receipt-depth.test.ts @@ -52,6 +52,7 @@ describe('write records the review depth at the checked-out head', () => { expect(r.review_depth).toMatchObject({ depth: 'light', class: 'code', base, registry: base, gate_scan: { tagged: 0, head } }); expect(r.review_depth!.runtime_lines).toBeGreaterThanOrEqual(5); expect(r.review_depth!.runtime_lines).toBeLessThanOrEqual(50); + expect(r.design).toBeUndefined(); const other = path.join(repo.root, 'other.md'); const bad = writeReceipt(repo, other, ['--head', older]); expect(bad.status).toBe(EXIT.fail); @@ -60,6 +61,18 @@ describe('write records the review depth at the checked-out head', () => { expect(fs.existsSync(other)).toBe(false); }, 60_000); + // Value: protects=a rendered-UI diff's receipt says only the static design check ran and why (GST-1718), from the same + // diff-scope the depth reads; fails_when=write records no design field on a frontend range, or records one on a + // backend-only range (test above); seam=none + test('a template-UI diff records design: static (no preview declared)', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, { 'src/ui/row.ts': "import { html } from 'lit';\nexport const row = (n: string) => html`
${n}
`;\n" }); + const body = path.join(repo.root, 'body.md'); + const ok = writeReceipt(repo, body); + expect(ok.status).toBe(EXIT.ok); + expect(receiptIn(body).design).toEqual({ mode: 'static', reason: 'no preview declared' }); + }, 60_000); + // Value: protects=a receipt names the gstack release that wrote it, never a newer policy key its tree carries, so a // version check against a receipt cannot be passed by an older build (GST-1366); fails_when=write records the // key-inflated version policy reading uses, or policy reading stops seeing the newest key; seam=none diff --git a/test/size-ledger/ship/gst-1718-template-ui-scope.json b/test/size-ledger/ship/gst-1718-template-ui-scope.json new file mode 100644 index 000000000..e9904c1be --- /dev/null +++ b/test/size-ledger/ship/gst-1718-template-ui-scope.json @@ -0,0 +1,4 @@ +{ + "bytes": 205, + "why": "GST-1718: Design Review line says static (no preview declared); template UI counts as frontend" +}