From 767869879ffcca4b5ef4b1af4303367e6cf0ba51 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 06:18:44 +0000 Subject: [PATCH 1/2] feat(gates): narrow the browser gate to the specs a change can actually break MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm run verify:ui` is the most expensive run in this repository — 646 Chromium tests, ~25 minutes — and CI repeats it wholesale: `Production UI critical` and the three `Production UI` shards are guarded on `ui_changed`, so any change touching a browser surface gets the full suite on GitHub whether or not it ran locally first. Measured on two consecutive changes 2026-09-01/02: 25.0m and 20.5m locally, while the focused selection for the same diffs took 37s and 6.1s and reached the same verdict. `gate-receipts.mjs` removed local-versus-local duplication and `gate-arbiter.mjs` weighs local-versus-CI for the static gates, but neither can touch this one. The arbiter's lever is DEFERRAL, and `ui` is in `NEVER_DEFER_CLASSES` — correctly, because pushing a UI change with no browser evidence at all is not a bet this repository takes. So the lever here is a different one: run the part of the suite the diff can break. Narrowing is strictly safer than deferring; something always runs. `scripts/browser-test-plan.mjs` picks one of four levels — `none` (no browser surface changed, and CI skips Production UI for that scope too), `changed` (an edited spec, run complete rather than grepped), `focused` (changed UI source attributed to its owning specs), `full`. Attribution is evidence, not a hand-maintained table that rots: a spec owns a changed file when both contain the same literal — a `data-testid` the source renders and the spec asks for, or a route the source defines and the spec navigates to. Component names are deliberately not matched, because a spec never names a component, so a match there would be a comment. It fails closed, the opposite of the arbiter's fail-open contract: an unattributable UI file, a shared foundation, an unclassifiable browser-lane path, or a deleted file all escalate on their own. The arbiter's bug costs a redundant run; this one's would cost an unrun journey. `isBrowserLanePath` restates `uiPatterns` from `ci-change-scope.mjs` because that script answers for a CHANGE, not a FILE — without a per-file answer every doc in a mixed change looks unclassified and forces the full suite, which is nearly every real change. A restated rule drifts, so it is not left to inspection: the test cross-checks the predicate against the real classifier path by path, and a `ui_changed` this mirror does not recognise escalates rather than narrows. The arbiter now names the browser gates as narrowed rather than answering "nothing to weigh", which read as "no saving available" for the gate with the largest saving on offer. `.claude/hooks/testing-policy.sh` states the policy to cloud sessions at SessionStart, including the reporting rules that keep it honest — a narrowed run is reported as "focused browser proof, full suite left to CI", never as `verify:ui` passing. CI is untouched. GitHub runs exactly what it ran before, and no required check is weakened to save local time. Verification: `npm run test` 948 files, 12104 passed | 1 skipped; `npm run lint` and `npm run typecheck` exit 0; `check:gate-manifest`, `check:ci-scope`, `check:verification-plan`, `check:skills`, `check:knip`, `docs:check-links`, `docs:check-scripts` and `docs:check-inventory` all pass. `npm run plan:browser` on this commit's own files returns level `none` and CI agrees, so no browser run was needed — the tool's first use is its own change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NkKHznd5E5KLnKbowRTLd8 --- .claude/hooks/testing-policy.sh | 74 ++++ .claude/settings.json | 12 + AGENTS.md | 2 +- docs/agents/verification-gates.md | 55 +++ docs/scripts-index.md | 2 +- package.json | 2 + scripts/browser-test-plan.mjs | 556 ++++++++++++++++++++++++++++++ scripts/gate-arbiter.mjs | 26 ++ tests/browser-test-plan.test.ts | 362 +++++++++++++++++++ 9 files changed, 1089 insertions(+), 2 deletions(-) create mode 100755 .claude/hooks/testing-policy.sh create mode 100644 scripts/browser-test-plan.mjs create mode 100644 tests/browser-test-plan.test.ts diff --git a/.claude/hooks/testing-policy.sh b/.claude/hooks/testing-policy.sh new file mode 100755 index 0000000000..25b7b33e9b --- /dev/null +++ b/.claude/hooks/testing-policy.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# SessionStart hook — state the verification-cost policy for cloud sessions. +# +# A cloud session starts with no memory of what the last one learned the hard +# way, and the most expensive lesson in this repository is about the browser +# gate: `npm run verify:ui` is ~25 minutes, CI repeats it wholesale on every +# change that touches a browser surface, and a session that does not know this +# will spend those 25 minutes buying a verdict GitHub is about to reach anyway. +# Measured 2026-09-02 on two consecutive changes: 25.0m and 20.5m locally, while +# the focused selection for the same diffs took 37s and 6.1s and agreed. +# +# So this hook says, once per session and in the model's own context, which gate +# to reach for first. It is scoped to cloud sessions because that is where the +# ask came from and where the session has no local history to draw on; a +# workstation session already has the docs, the receipts store, and the operator. +# +# Contract: READ-ONLY and unfailable. Emits nothing but `additionalContext` on +# stdout, exits 0 on every path including a malformed payload, and makes no +# decision — a session that ignores it simply runs the wider gate, which is the +# conservative outcome. SessionStart hook stdout is injected into context; +# stderr is not, which is why every line below goes to stdout. +set -uo pipefail + +# Cloud sessions only. `session-start.sh` uses the same gate. +[ "${CLAUDE_CODE_REMOTE:-}" = "true" ] || exit 0 + +# Drain stdin so the caller never blocks on an unread pipe. +cat >/dev/null 2>&1 || true + +root="${CLAUDE_PROJECT_DIR:-}" +[ -z "$root" ] && root="$(git rev-parse --show-toplevel 2>/dev/null || true)" +[ -z "$root" ] && root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." 2>/dev/null && pwd || true)" +# Without the planner there is no advice to give, and inventing some would be +# worse than silence. +[ -f "$root/scripts/browser-test-plan.mjs" ] || exit 0 + +read -r -d '' context <<'POLICY' || true +[testing-policy] Verification cost policy for this cloud session. + +CI re-runs the browser suite on every change that touches a browser surface, so a +full local `npm run verify:ui` (~25 min, 646 tests) buys a verdict GitHub is about +to reach. Do not spend it by default. + +Before any browser run, ask the planner which part of the suite the diff can break: + + npm run plan:browser # dry run: the level, the specs, and why + npm run plan:browser -- --run # execute the plan it printed + +It fails closed: shared foundations, an unattributable UI file, or unknown scope +all escalate to the full suite on their own. You do not have to judge that. + +For the static gates the arbiter already answers the same question: + + npm run arbiter -- # RUN / DEFER / PROVEN, with its evidence + +Reporting rules, which the cost saving depends on: +- A narrowed run is NOT the full gate. Say "focused browser proof at level , + full suite left to CI" — never "verify:ui passed". +- A deferred gate is not a passed gate; say "deferred to CI". +- Paste the decisive line of real output. Exit 0 alone is not proof. + +Unchanged by any of this: GitHub remains the authoritative merge gate and runs +exactly what it ran before. Never weaken a required check to save local time. +POLICY + +# The context is a JSON string field, so the only characters that must be escaped +# are backslash and double quote; the heredoc above contains neither, and this +# substitution keeps that true if it ever does. Newlines become \n. +escaped="${context//\\/\\\\}" +escaped="${escaped//\"/\\\"}" +escaped="${escaped//$'\n'/\\n}" + +printf '{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"%s"}}\n' "$escaped" +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index 3acfcbdfd0..903477516d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -97,6 +97,13 @@ "Bash(npm run verify:pr-local)", "Bash(npm run verify:pr-local --*)", "Bash(npm run verify:phone-chrome)", + "Bash(npm run plan:browser)", + "Bash(npm run plan:browser --*)", + "Bash(npm run arbiter)", + "Bash(npm run arbiter --*)", + "Bash(npm run arbiter:status)", + "Bash(npm run receipts)", + "Bash(npm run check:browser-test-plan)", "Bash(npm run ensure)", "Bash(npm run skills)", "Bash(npm run docs:check-index)", @@ -305,6 +312,11 @@ "type": "command", "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/issues-surface.sh\"", "timeout": 30 + }, + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/testing-policy.sh\"", + "timeout": 15 } ] } diff --git a/AGENTS.md b/AGENTS.md index 1209da30ec..f1000dfaf9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,7 +116,7 @@ For the verification principle, the tier table, and the rest of the gate-selecti ## Do not pay twice for the verdict GitHub is about to reach -For the rule against re-deriving a verdict GitHub is about to reach, and the gate arbiter's inputs and non-negotiable boundaries, see [`docs/agents/verification-gates.md`](docs/agents/verification-gates.md). +For the rule against re-deriving a verdict GitHub is about to reach, the gate arbiter's inputs and non-negotiable boundaries, and the browser-gate planner that narrows `verify:ui` to the specs a diff can actually break (`npm run plan:browser`), see [`docs/agents/verification-gates.md`](docs/agents/verification-gates.md). diff --git a/docs/agents/verification-gates.md b/docs/agents/verification-gates.md index 5242a6df75..ac8e5a79f4 100644 --- a/docs/agents/verification-gates.md +++ b/docs/agents/verification-gates.md @@ -99,3 +99,58 @@ twice. The smallest-correct-gate rule above still decides which gate is right; t only decides whether that gate has anything left to tell you before you push. + +## The browser gate is narrowed, not deferred + +`npm run verify:ui` is the most expensive run here — 646 Chromium tests, ~25 minutes — +and CI repeats it wholesale: `Production UI critical` and the three `Production UI` +shards are guarded on `ui_changed`, so any change touching a browser surface gets the +full suite on GitHub whether or not it ran locally first. + +The arbiter above cannot help with it. Its lever is deferral, and `ui` is in +`NEVER_DEFER_CLASSES` — pushing a UI change with **no** browser evidence is not a bet +this repository takes, and that stays true. The lever here is a different one: run the +part of the suite the diff can actually break. + +```bash +npm run plan:browser # dry run: the level it chose, the specs, and why +npm run plan:browser -- --run # execute that plan +npm run plan:browser -- --full # force the whole suite +``` + +Four levels, and only the middle two are a saving: + +| Level | When | What runs | +| --------- | -------------------------------------------------------- | -------------------------------------------------------------------------------------- | +| `none` | no browser surface changed | nothing — CI skips Production UI for this scope too | +| `changed` | a `ui-*.spec.ts` changed | those specs, **complete** — never grepped, since the diff rewrote their own assertions | +| `focused` | changed UI source attributable to specs | the owning specs, complete | +| `full` | shared foundation, unattributable file, or unknown scope | `verify:ui` | + +**Attribution is evidence, not a table.** A spec owns a changed file when both contain +the same literal — a `data-testid` the source renders and the spec asks for, or a route +the source defines and the spec navigates to. Component names are deliberately not +matched: a spec never names a component, so a match there would be a comment, and a +comment is not proof a journey covers the code. + +**It fails closed, the opposite of the arbiter.** A file with no owning spec, a shared +foundation (`globals.css`, any shared style root, the Playwright config, the runner, the +shell/chrome coordinator set), a browser-lane path it cannot classify, or a deleted +file all escalate to the full suite on their own. The arbiter's bug costs a redundant +run; this one's would cost an unrun journey, so the defaults point the other way. + +Non-negotiable, and the reason the saving is allowed at all: + +- **A narrowed run is not the UI gate.** Report it as "focused browser proof at level + ``, full suite left to CI" — never "verify:ui passed". `check:browser-test-plan` + and `tests/browser-test-plan.test.ts` pin that wording in the runner itself. +- **CI is untouched.** The planner advises local work only; GitHub runs exactly what it + ran before, and no required check may be weakened to save local time. +- **Dry run by default.** `--run` executes; nothing happens without it. +- **When CI will NOT repeat it** — a browser-lane change on a scope where the Production + UI jobs are skipped — the planner says so and tells you to consider `--full`, because + there the local run is the only browser evidence there will be. + +Cloud sessions are told this at SessionStart by `.claude/hooks/testing-policy.sh`, which +is read-only, exits 0 on every path, and states the reporting rules alongside the +commands. diff --git a/docs/scripts-index.md b/docs/scripts-index.md index b141994388..97e1430392 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (283 files) and the `package.json` script surface (284 entries), +Curated map of `scripts/` (283 files) and the `package.json` script surface (286 entries), grouped by purpose. This is orientation, not an exhaustive per-file listing — the authoritative command list is `package.json`, and `npm run docs:check-scripts` verifies every `npm run ` referenced in docs resolves to a real script. `npm run docs:update` refreshes the exact counts above. diff --git a/package.json b/package.json index fa3799198c..e502f4f48f 100644 --- a/package.json +++ b/package.json @@ -86,6 +86,8 @@ "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:upload-limit-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:verification-plan && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:skills && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run check:ledger-write-discipline && npm run check:pr-mergeability && npm run sitemap:check && npm run check:repo-awareness-snapshot && npm run docs:check-index && npm run docs:check-inventory && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:mha-act-sections && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:phone-chrome": "node scripts/verify-phone-chrome.mjs", + "plan:browser": "node scripts/browser-test-plan.mjs", + "check:browser-test-plan": "node scripts/browser-test-plan.mjs --self-test", "audit:final-merge": "node scripts/final-merge-audit.mjs", "audit:merge-loss": "node scripts/audit-merge-loss.mjs --self-test && node scripts/audit-merge-loss.mjs", "verify:ui": "npm run check:runtime && npm run check:installed-lock-parity && npm run test:e2e:pr", diff --git a/scripts/browser-test-plan.mjs b/scripts/browser-test-plan.mjs new file mode 100644 index 0000000000..3d1bad63a6 --- /dev/null +++ b/scripts/browser-test-plan.mjs @@ -0,0 +1,556 @@ +#!/usr/bin/env node +/** + * browser-test-plan.mjs — choose the smallest browser gate that still covers a change. + * + * `gate-receipts.mjs` removed local-versus-local duplication. `gate-arbiter.mjs` + * weighs the local-versus-CI duplication for the static gates. Neither touches the + * single most expensive run in this repository: + * + * npm run verify:ui -> 646 Chromium tests, ~25 minutes + * + * CI repeats it wholesale. `Production UI critical` and the three `Production UI` + * shards are guarded by `needs.changes.outputs.ui_changed == 'true'`, so for any + * change that touches a browser surface the full suite runs on GitHub whether or + * not it ran locally first. Measured on this repository 2026-09-02: two + * consecutive changes ran the full local gate (25.0m and 20.5m), and between them + * the focused selection for the same diffs took 37s and 6.1s and reached the same + * verdict. + * + * The arbiter cannot answer this one. Its lever is DEFERRAL — run the gate or hand + * it to CI — and `ui` is in `NEVER_DEFER_CLASSES`, correctly: a UI change with no + * browser evidence at all before a push is not a bet this repository takes. So the + * lever here is a different one: + * + * not "run it or skip it", but "run the part of it that can actually fail" + * + * Narrowing is strictly safer than deferring. Something always runs locally; what + * changes is how much of the suite that cannot be affected by the diff is dragged + * along with it. + * + * The levels, cheapest first. Each is chosen only when it can be justified from the + * tree, never from a hand-maintained ownership table that rots in silence: + * + * none No browser surface changed. `ui_changed` is false, so CI skips its + * Production UI jobs too — nothing is left unrun by running nothing. + * changed A `ui-*.spec.ts` file changed: run those specs COMPLETE. An edited + * spec is evidence about itself, and a grep inside it would be the + * author marking their own work. + * focused Changed UI source, attributed to the specs that exercise it by an + * identifier both files contain — a `data-testid` the source renders, + * or a route the source defines and the spec navigates to. + * full Shared foundations changed, or a changed UI file could not be + * attributed to any spec. Both are fail-closed: an unattributable + * change is treated exactly as CI treats unknown scope. + * + * Boundaries, all of them the conservative direction: + * + * - **Fail closed to `full`.** Every uncertainty — an unreadable file, a changed UI + * source with no owning spec, an unrecognised path — escalates. A bug here costs + * a full local run, never a missed one. That is the opposite of the arbiter's + * fail-open contract, and deliberately so: the arbiter's failure mode is a + * redundant run, this one's would be an unrun journey. + * - **CI is never advised by this file.** It plans local work only. GitHub keeps + * running exactly what it runs today. + * - **A narrowed run is not a full run.** The plan says which level it chose and + * what it is leaving to CI, and must be reported that way — "focused browser + * proof, full suite left to CI", never "the UI gate passed". + * - **Dry-run by default**, like every other planner in `docs/productivity-workflows.md`. + * `--run` executes; nothing happens without it. + * + * CLI: + * node scripts/browser-test-plan.mjs plan for the diff vs origin/main + * node scripts/browser-test-plan.mjs --files a.tsx,b.ts plan for an explicit file list + * node scripts/browser-test-plan.mjs --diff HEAD~1 plan against another base + * node scripts/browser-test-plan.mjs --json machine-readable plan + * node scripts/browser-test-plan.mjs --run execute the planned stages + * node scripts/browser-test-plan.mjs --full force the full suite + * node scripts/browser-test-plan.mjs --self-test offline contract self-test + */ +import { execFileSync, spawnSync } from "node:child_process"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { deriveCiCoverage } from "./gate-arbiter.mjs"; + +const projectRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); + +const normalize = (file) => + String(file ?? "") + .trim() + .replaceAll("\\", "/") + .replace(/^\.\//, ""); + +/** + * Files whose blast radius is the whole suite. + * + * Not a list of "important" files — a list of files whose effect cannot be + * attributed to any subset of journeys. A design token, the global stylesheet, the + * Playwright config, the runner, or a shared fixture changes what EVERY spec + * renders or how every spec runs, so narrowing on them would be narrowing on + * nothing. Kept in step with `sharedFoundation` in `phone-chrome-plan.mjs`, which + * makes the same call for the phone-chrome subset. + */ +export const SHARED_FOUNDATION_PATTERNS = [ + /^src\/app\/globals\.css$/, + /^src\/styles\//, + /^src\/app\/layout\.tsx$/, + /^playwright(?:\..*)?\.config\.ts$/, + /^tests\/helpers\//, + /^tests\/playwright-.*\.ts$/, + /^scripts\/(?:run-playwright|playwright-base-url|playwright-browser-preflight|playwright-pr-shards)\.mjs$/, + /^src\/components\/ClinicalDashboard\.tsx$/, + /^src\/components\/clinical-dashboard\/(?:global-search-shell|master-search-header|mobile-composer-reserve|phone-footer-layer-portal|scroll-surface|use-active-scroll-owner|use-dashboard-chrome-coordinator|use-hide-on-scroll|use-phone-overlay-chrome-reserve)\.(?:ts|tsx)$/, + /^src\/lib\/app-modes\.ts$/, +]; + +/** A Playwright spec this planner can select. Mirrors `uiPatterns` in `ci-change-scope.mjs`. */ +export const BROWSER_SPEC_PATTERN = /^tests\/(?:ui-.*|answer-progress-ui-smoke)\.spec\.ts$/; + +export function isSharedFoundation(file) { + return SHARED_FOUNDATION_PATTERNS.some((pattern) => pattern.test(file)); +} + +/** + * Is this file in CI's browser lane at all? + * + * A mirror of `uiPatterns` in `ci-change-scope.mjs`, and the one piece of that file's + * routing this planner has to restate rather than shell out for: `ci-change-scope.mjs` + * answers for a CHANGE, not for a FILE, so a mixed change reports one `ui_changed` for + * the whole set. Without a per-file answer, every documentation or config file in a + * mixed change looks like an unclassified browser file and forces the full suite — + * which is nearly every real change, and would make the planner useless. + * + * Restating a rule is a drift risk, so it is not left to inspection: + * `tests/browser-test-plan.test.ts` cross-checks this predicate against + * `ci-change-scope.mjs` itself, path by path, and fails when the two disagree. + */ +export const BROWSER_LANE_PATTERNS = [ + /^data\//, + /^public\//, + /^src\/(?:app|components|styles)\//, + /^\.github\/actions\/setup-ui-e2e\//, + /^tests\/(?:ui-.*|answer-progress-ui-smoke)\.spec\.ts$/, + /^tests\/playwright-.*\.ts$/, + /^tests\/helpers\/.*\.ts$/, + /^tests\/__screenshots__\//, + /^playwright(?:\..*)?\.config\.ts$/, + /^scripts\/(?:run-playwright|playwright-base-url|playwright-browser-preflight|playwright-pr-shards|check-playwright-browser-revision)\.(?:mjs|ts)$/, + /^scripts\/(?:run|check)-lighthouse-budget\.mjs$/, + /^lighthouse-budget\.json$/, + /^src\/lib\/(?:app-modes|app-mode-icons|search-route-ownership|ui-copy|mode-home-composer|mode-secondary-navigation|category-identity(?:-icons)?|brand-mark|brand-image|search-command-surface|search-navigation-context|search-scope-filter-chips|search-shell-props|document-flow-routes|document-viewer-navigation|differentials-navigation|therapy-compass-navigation|therapies)\.tsx?$/, +]; + +export function isBrowserLanePath(file) { + if (file === "src/app/api" || file.startsWith("src/app/api/")) return false; + return BROWSER_LANE_PATTERNS.some((pattern) => pattern.test(file)); +} + +/** + * Files that can carry an identifier a spec names — routes and components. + * + * Nothing else is attributable, and that is the point rather than a limitation: a + * `src/lib` module, a public asset or a screenshot baseline renders no `data-testid` + * of its own, so no honest attribution exists for it. Such a file either sits + * outside the browser lane entirely (and is ignored) or sits inside it and cannot be + * classified (and escalates to the full suite). API handlers are excluded for the + * same reason `ci-change-scope.mjs` excludes them from `ui_changed`: they are not + * browser journeys. + */ +export function isRenderingSource(file) { + if (file === "src/app/api" || file.startsWith("src/app/api/")) return false; + return /^src\/(?:app|components)\/.+\.(?:tsx|ts|css)$/.test(file); +} + +/** + * The identifiers a changed source file and a spec can both name. + * + * Deliberately only two kinds, both of them literal strings that appear verbatim + * on each side of the tap: + * + * - `data-testid="x"` — what a spec reaches with `getByTestId("x")`. + * - a route segment from `src/app/**\/page.tsx` — what a spec navigates to. + * + * Component names are NOT used. A spec never names a component, so matching on one + * would mean matching a comment, and a comment is not evidence that a journey + * covers the code. Rejecting that is what keeps a `focused` plan honest: an + * attribution either rests on a string the browser actually sees, or the file is + * unattributable and the plan escalates to `full`. + * + * @param {string} file repository-relative path + * @param {string} contents + * @returns {{ testIds: string[], routes: string[] }} + */ +export function ownershipKeys(file, contents) { + const testIds = [ + ...new Set( + [...String(contents ?? "").matchAll(/data-testid=(?:"([^"]+)"|\{"([^"]+)"\}|'([^']+)')/g)] + .map((match) => match[1] ?? match[2] ?? match[3]) + .filter(Boolean), + ), + ].sort(); + + const routes = []; + const routeMatch = file.match(/^src\/app\/(.+)\/page\.tsx$/); + if (routeMatch) { + const segments = routeMatch[1] + .split("/") + // Route groups `(search-app)` are organisational and absent from the URL; + // a dynamic `[slug]` cannot be matched against a literal in a spec. + .filter((segment) => !/^\(.*\)$/.test(segment)); + if (!segments.some((segment) => /^\[.*\]$/.test(segment))) routes.push(`/${segments.join("/")}`); + } + + return { testIds, routes }; +} + +/** + * Which specs reference any of these identifiers. + * + * A plain substring test, because that is exactly how the identifier is written in + * both files. A quoted `"answer-feedback-trigger"` in a spec is the spec asking the + * browser for the element the source renders under that name; nothing subtler is + * needed, and anything subtler would be guessing. + * + * @param {{ testIds: string[], routes: string[] }} keys + * @param {Map} specSources spec path -> contents + * @returns {string[]} spec paths, sorted + */ +export function specsReferencing(keys, specSources) { + const needles = [...keys.testIds.map((id) => `"${id}"`), ...keys.routes.map((route) => `"${route}"`)]; + if (needles.length === 0) return []; + const owners = []; + for (const [spec, contents] of specSources) { + if (needles.some((needle) => contents.includes(needle))) owners.push(spec); + } + return owners.sort(); +} + +/** + * Build the plan. + * + * Pure: every input is passed in, so the self-test and `tests/browser-test-plan.test.ts` + * drive it without a worktree, a build, or a browser. + * + * @param {object} input + * @param {string[]} input.files changed files, repository-relative + * @param {{ ui_changed?: boolean }} input.scope output of `ci-change-scope.mjs --json` + * @param {Map} input.specSources every browser spec -> its contents + * @param {Map} input.sourceSources changed UI source files -> their contents + * @param {"auto" | "full"} [input.mode] + */ +export function browserTestPlan({ files, scope, specSources, sourceSources, mode = "auto" }) { + const normalized = [...new Set((files ?? []).map(normalize).filter(Boolean))].sort(); + const uiChanged = Boolean(scope?.ui_changed); + + const changedSpecs = normalized.filter((file) => BROWSER_SPEC_PATTERN.test(file)); + const foundation = normalized.filter((file) => isSharedFoundation(file)); + // The files that have to be attributed to a journey: rendering sources that are + // neither a spec nor a foundation. Anything else in the change is either outside + // the browser lane (ignored) or inside it and unclassifiable (handled last, by + // escalating). + const attributable = normalized.filter( + (file) => !BROWSER_SPEC_PATTERN.test(file) && !isSharedFoundation(file) && isRenderingSource(file), + ); + const classified = new Set([...changedSpecs, ...foundation, ...attributable]); + // Only files that are themselves in the browser lane can be an unclassified + // browser file. A doc or a config file riding along in a mixed change is simply + // not this planner's business, and treating it as unknown scope would escalate + // nearly every real change. + const unclassifiedBrowserFiles = normalized.filter((file) => isBrowserLanePath(file) && !classified.has(file)); + // `ui_changed` is true but nothing here is recognised as a browser-lane path: + // the mirror above has drifted from `ci-change-scope.mjs`. Fail closed on the + // drift rather than narrow on a rule that no longer matches CI's. + const laneMirrorDrifted = uiChanged && !normalized.some((file) => isBrowserLanePath(file)); + + const attribution = []; + const unattributed = []; + for (const file of attributable) { + const keys = ownershipKeys(file, sourceSources.get(file) ?? ""); + const owners = specsReferencing(keys, specSources); + if (owners.length === 0) unattributed.push(file); + else attribution.push({ file, keys, owners }); + } + + const forced = mode === "full"; + let level; + const reasons = []; + + if (!uiChanged && !forced) { + level = "none"; + reasons.push("No browser surface changed, so CI skips its Production UI jobs for this scope too."); + } else if (forced) { + level = "full"; + reasons.push("The full suite was requested explicitly."); + } else if (foundation.length > 0) { + level = "full"; + reasons.push( + `Shared foundations changed (${foundation.join(", ")}); their effect cannot be attributed to a subset of journeys.`, + ); + } else if (unattributed.length > 0) { + level = "full"; + reasons.push( + `No spec references anything rendered by ${unattributed.join(", ")}; an unattributable UI change fails closed to the full suite.`, + ); + } else if (laneMirrorDrifted) { + level = "full"; + reasons.push( + "CI puts this change in the browser lane but no changed path matches BROWSER_LANE_PATTERNS; the mirror of ci-change-scope has drifted, so the full suite runs.", + ); + } else if (unclassifiedBrowserFiles.length > 0) { + // `ui_changed` is true because of a file this planner cannot attribute — a + // public asset, a screenshot baseline, a runner script outside the foundation + // list. Unknown scope is heavy scope, exactly as CI routes it. + level = "full"; + reasons.push( + `${unclassifiedBrowserFiles.join(", ")} puts this change in the browser lane but cannot be attributed to a journey; unknown scope runs the full suite.`, + ); + } else if (attribution.length > 0) { + level = "focused"; + reasons.push("Every changed UI file is exercised by a spec that names something it renders."); + } else if (changedSpecs.length > 0) { + level = "changed"; + reasons.push("Only browser specs changed; each one runs complete."); + } else { + level = "full"; + reasons.push("The change could not be classified; unknown scope runs the full suite."); + } + + // A changed spec always runs in full, at every level below `full` — it is the one + // file whose own assertions the diff rewrote. + const specsToRun = [...new Set([...changedSpecs, ...attribution.flatMap((entry) => entry.owners)])].sort(); + + const stages = []; + if (level === "full") { + stages.push({ + id: "full-ui", + label: "full Chromium UI suite", + command: { executable: "npm", args: ["run", "verify:ui"] }, + }); + } else if (level !== "none") { + stages.push({ + id: "browser-owners", + label: `complete browser specs owning the change (${specsToRun.length})`, + command: { executable: "node", args: ["scripts/run-playwright.mjs", ...specsToRun, "--project=chromium"] }, + }); + } + + return { + files: normalized, + level, + stages, + specs: specsToRun, + attribution, + unattributed, + foundation, + unclassifiedBrowserFiles, + laneMirrorDrifted, + uiChanged, + reasons, + }; +} + +export function renderCommand(command) { + return [command.executable, ...command.args.map((arg) => (/\s|\|/.test(arg) ? JSON.stringify(arg) : arg))].join(" "); +} + +/* ------------------------------------------------------------------ * + * CLI * + * ------------------------------------------------------------------ */ + +function git(args) { + return execFileSync("git", args, { cwd: projectRoot, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); +} + +function changedFilesFromGit(base) { + const tracked = git(["diff", "--name-only", `${base}...HEAD`]).split("\n"); + const working = git(["diff", "--name-only", "HEAD"]).split("\n"); + const staged = git(["diff", "--name-only", "--cached"]).split("\n"); + const untracked = git(["ls-files", "--others", "--exclude-standard"]).split("\n"); + return [...tracked, ...working, ...staged, ...untracked].map(normalize).filter(Boolean); +} + +function readChangeScope(files) { + // `--files ` as two arguments: `getArgValue` in that script reads the NEXT + // argv entry, so the `--files=a,b` spelling is silently ignored and + // `resolveChangedFiles` falls through to the local working tree — classifying a + // different change from the one being planned. Verified against its parser rather + // than assumed; the scope this returns decides whether the plan may narrow at all. + const output = execFileSync( + process.execPath, + [path.join(projectRoot, "scripts", "ci-change-scope.mjs"), "--json", "--files", files.join(",")], + { cwd: projectRoot, encoding: "utf8", maxBuffer: 32 * 1024 * 1024 }, + ); + const scope = JSON.parse(output); + // Fail closed on a scope that did not classify what was asked: an empty list, or + // a list that came back different, means the fallback fired. + const asked = [...new Set(files)].sort().join("|"); + const answered = [...new Set(scope.files ?? [])].sort().join("|"); + if (files.length > 0 && asked !== answered) { + throw new Error( + `ci-change-scope classified a different file set than requested (asked ${asked || ""}, answered ${answered || ""})`, + ); + } + return scope; +} + +function readAllSpecs() { + const testsDir = path.join(projectRoot, "tests"); + const specs = new Map(); + for (const entry of readdirSync(testsDir)) { + const relative = `tests/${entry}`; + if (!BROWSER_SPEC_PATTERN.test(relative)) continue; + specs.set(relative, readFileSync(path.join(testsDir, entry), "utf8")); + } + return specs; +} + +function readSources(files) { + const sources = new Map(); + for (const file of files) { + if (BROWSER_SPEC_PATTERN.test(file)) continue; + const absolute = path.join(projectRoot, file); + // A deleted file cannot be attributed and must not be silently dropped: leaving + // it out of `sourceSources` keeps it out of `attributable`, which would let a + // deletion narrow the plan. Recording it empty makes it unattributable instead, + // and unattributable escalates to the full suite. + sources.set(file, existsSync(absolute) ? readFileSync(absolute, "utf8") : ""); + } + return sources; +} + +function selfTest() { + const specSources = new Map([ + ["tests/ui-smoke.spec.ts", 'getByTestId("answer-feedback-trigger"); gotoApp(page, "/documents");'], + ["tests/ui-tools.spec.ts", 'getByTestId("tools-launcher");'], + ]); + const assert = (name, condition) => { + if (!condition) throw new Error(`browser-test-plan self-test failed: ${name}`); + }; + + const attributed = browserTestPlan({ + files: ["src/components/clinical-dashboard/evidence-panels.tsx"], + scope: { ui_changed: true }, + specSources, + sourceSources: new Map([ + ["src/components/clinical-dashboard/evidence-panels.tsx", '