mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 17:28:23 +01:00
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>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed
1 parent
40071a4237
commit
5862765a20
20 files changed
+273
-14
No files matched your search
@@ -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<void> 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-z][a-z0-9-]*([[:space:]/>]|$)|(^|[^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
|
||||
|
||||
@@ -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')),
|
||||
|
||||
@@ -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)
|
||||
+12
-2
@@ -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
|
||||
<json|@file>`: `{ status: updated | current | blocked, scope, exception? }`,
|
||||
where `scope` names the docs roots the audit covered and `exception` is
|
||||
|
||||
@@ -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 <base>` 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',
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
// 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' } } } } } },
|
||||
},
|
||||
|
||||
@@ -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; }
|
||||
|
||||
@@ -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<string> | null } {
|
||||
export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion; tagged: ReadonlySet<string> | 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' };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+1
-1
@@ -22,7 +22,7 @@ const KEY_ORDER: Array<keyof ShipReceipt | 'docs'> = [
|
||||
'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. */
|
||||
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
@@ -120,7 +120,7 @@ proof, fixes and blocked/inconclusive/not-run coverage. Never present stale or
|
||||
unavailable results as passing.>
|
||||
|
||||
## Design Review
|
||||
<If design review ran: "Design Review (lite): N findings — M auto-fixed, K skipped. AI Slop: clean/N issues.">
|
||||
<If design review ran: "Design Review: static (no preview declared). Design-lite: N findings — M auto-fixed, K skipped. AI Slop: clean/N issues." The receipt's `design` field says the same; no rendered /design-review runs inside /ship yet.>
|
||||
<Detector: "clean" | "N findings (rule-id, rule-id)" | "not installed" | "not cached" | "off" — the state the probe printed; rule ids and counts only, finding text and snippets never reach the PR body.>
|
||||
<If no frontend files changed: "No frontend files changed — design review skipped.">
|
||||
|
||||
|
||||
@@ -111,7 +111,7 @@ proof, fixes and blocked/inconclusive/not-run coverage. Never present stale or
|
||||
unavailable results as passing.>
|
||||
|
||||
## Design Review
|
||||
<If design review ran: "Design Review (lite): N findings — M auto-fixed, K skipped. AI Slop: clean/N issues.">
|
||||
<If design review ran: "Design Review: static (no preview declared). Design-lite: N findings — M auto-fixed, K skipped. AI Slop: clean/N issues." The receipt's `design` field says the same; no rendered /design-review runs inside /ship yet.>
|
||||
<Detector: "clean" | "N findings (rule-id, rule-id)" | "not installed" | "not cached" | "off" — the state the probe printed; rule ids and counts only, finding text and snippets never reach the PR body.>
|
||||
<If no frontend files changed: "No frontend files changed — design review skipped.">
|
||||
|
||||
|
||||
@@ -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`<div class="permission-row"><span class="label">${name}</span></div>`;',
|
||||
'}',
|
||||
'export async function load(): Promise<void> {',
|
||||
' await fetch("/api/personas");',
|
||||
'}',
|
||||
'',
|
||||
].join('\n');
|
||||
function templateRepo(): { dir: string; git: (...a: string[]) => ReturnType<typeof spawnSync> } {
|
||||
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, '<span class="label">${name}</span>', '<span class="label">${name}</span><small class="note">Only owner</small>');
|
||||
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<void> 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<void>', 'Promise<Map<string, number>>');
|
||||
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 <placeholder> 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 <base> --head <sha>\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`',
|
||||
' <div class="row">',
|
||||
' ${this.label}',
|
||||
' </div>`;',
|
||||
' }',
|
||||
'}',
|
||||
'',
|
||||
].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 `<p>Hi</p>`;\n");
|
||||
expect(runScope(dir).SCOPE_FRONTEND).toBe('true');
|
||||
});
|
||||
});
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
+1
-1
@@ -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 <base>`) 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 <base>`) (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.
|
||||
|
||||
|
||||
@@ -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`<div class=\"row\">${n}</div>`;\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
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
{
|
||||
"bytes": 205,
|
||||
"why": "GST-1718: Design Review line says static (no preview declared); template UI counts as frontend"
|
||||
}
|
||||
Reference in new issue
Block a user