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..7447530683 100644 --- a/docs/agents/verification-gates.md +++ b/docs/agents/verification-gates.md @@ -99,3 +99,72 @@ 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. + +**The project is part of the selection.** `chromium` grep-inverts `@mockup` and +`chromium-mockups` collects only those, so a mockup spec run under `--project=chromium` +collects nothing and "passes" having executed no test. The planner reads both +`testMatch` patterns out of `playwright.config.ts` (rather than copying them, which +would drift) and routes each selected spec accordingly — `tests/ui-tools.spec.ts` holds +both kinds and gets both projects. A spec neither project collects escalates to the full +suite rather than producing a command that matches nothing. + +**Coverage that rests on an unreadable precondition is reported as conditional.** +`ui-critical-fast` is guarded on `github.event.pull_request.draft != true`, which no +worktree can evaluate. On a draft PR CI skips the very job that makes narrowing safe, so +the planner prints those assumptions with the verdict instead of stating flatly that CI +will repeat the run. + +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..df5ff81f7c 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/` (284 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..c10e5656b6 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "test:coverage": "node scripts/run-vitest.mjs run --coverage", "test:coverage:node": "node scripts/run-vitest.mjs run --project=node --coverage", "test:coverage:ui": "node scripts/run-vitest.mjs run --project=jsdom --coverage", - "test:ci-workflows": "node scripts/run-vitest.mjs run tests/ci-cache-safety.test.ts tests/authenticated-live-workflow.test.ts tests/codex-autofix-workflow.test.ts tests/codex-run-pr-operator-workflow.test.ts tests/eval-canary-workflow.test.ts tests/live-drift-workflow.test.ts tests/ops-digest.test.ts tests/container-ci-contract.test.ts tests/test-runner-safety.test.ts tests/installed-lock-parity.test.ts tests/railway-config.test.ts tests/ingestion-autopilot.test.ts tests/ingestion-autopilot-workflow.test.ts tests/check-lighthouse-budget.test.ts tests/live-web-vitals-inputs.test.ts tests/offline-release-profile.test.ts", + "test:ci-workflows": "node scripts/run-vitest.mjs run tests/ci-cache-safety.test.ts tests/authenticated-live-workflow.test.ts tests/browser-test-plan.test.ts tests/codex-autofix-workflow.test.ts tests/codex-run-pr-operator-workflow.test.ts tests/eval-canary-workflow.test.ts tests/live-drift-workflow.test.ts tests/ops-digest.test.ts tests/container-ci-contract.test.ts tests/test-runner-safety.test.ts tests/installed-lock-parity.test.ts tests/railway-config.test.ts tests/ingestion-autopilot.test.ts tests/ingestion-autopilot-workflow.test.ts tests/check-lighthouse-budget.test.ts tests/live-web-vitals-inputs.test.ts tests/offline-release-profile.test.ts", "test:cc-guards": "node scripts/run-vitest.mjs run --reporter=dot tests/caring-contacts-plan-draft.dom.test.tsx tests/caring-contacts-plan-patient-detail.test.ts tests/caring-contacts-plan-activation.test.ts tests/caring-contacts-plan-wizard.dom.test.tsx tests/caring-contacts-schedule.test.ts tests/caring-contacts-schedule-view.test.ts tests/caring-contacts-schedule-route.test.ts tests/caring-contacts-schedule-screen.dom.test.tsx tests/caring-contacts-schedule-page.dom.test.tsx tests/caring-contacts-clock.test.ts tests/caring-contacts-new-plan-page.dom.test.tsx tests/caring-contacts-explained-automation.dom.test.tsx tests/caring-contacts-workspace-shell.dom.test.tsx tests/caring-contacts-patients-directory.dom.test.tsx tests/caring-contacts-patient-overview.dom.test.tsx tests/caring-contacts-patients-page.dom.test.tsx tests/caring-contacts-domain-isolation.test.ts tests/caring-contacts-interface-vocabulary.test.ts tests/caring-contacts-retention.test.ts tests/caring-contacts-repository.test.ts tests/caring-contacts-overlay-definitions.test.ts tests/caring-contacts-overlay-trigger-inventory.test.ts tests/caring-contacts-workspace-screens.test.ts tests/route-reachability.test.ts tests/design-system-adoption.test.ts tests/caring-contacts-contact-time-adjustment.dom.test.tsx tests/caring-contacts-contact-route.test.ts tests/caring-contacts-overlay-trigger.dom.test.tsx tests/caring-contacts-overlay-host.dom.test.tsx tests/source-control-bytes.test.ts tests/caring-contacts-demo-seed.test.ts tests/caring-contacts-pathway-versions.test.ts tests/caring-contacts-templates-library.dom.test.tsx tests/caring-contacts-templates-page.dom.test.tsx tests/caring-contacts-template-detail.dom.test.tsx tests/caring-contacts-template-detail-page.dom.test.tsx tests/caring-contacts-reporting.test.ts tests/caring-contacts-guidance-reports-pages.dom.test.tsx tests/caring-contacts-team-workload.test.ts tests/caring-contacts-team-route.test.ts tests/caring-contacts-team-roster.dom.test.tsx tests/caring-contacts-team-page.dom.test.tsx", "test:e2e": "node scripts/run-playwright.mjs", "test:e2e:all": "node scripts/run-playwright.mjs", @@ -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..395c0d6921 --- /dev/null +++ b/scripts/browser-test-plan.mjs @@ -0,0 +1,702 @@ +#!/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$/; + +/** + * The two `testMatch` patterns from `playwright.config.ts`, read from that file + * rather than copied into this one. + * + * `chromium` carries `grepInvert: /@mockup/` and `chromium-mockups` carries + * `grep: /@mockup/`, so a project is not a stylistic choice: selecting a + * mockup-only spec under `--project=chromium` collects nothing and the run + * "passes" having executed no test at all. `tests/ui-tools.spec.ts` matches BOTH + * patterns — it holds production journeys and `@mockup` ones — so a mixed spec + * needs both projects. + * + * Extracted at runtime, because a copy of a regex in two files is a copy that + * drifts. If the extraction fails the caller runs both projects, which is the + * conservative direction: too many tests, never none. + * + * @param {(path: string, encoding: string) => string} [readFile] + * @returns {{ production: RegExp | null, mockup: RegExp | null }} + */ +export function playwrightSpecPatterns(readFile = readFileSync) { + const extract = (source, name) => { + // One regex literal, delimiters included, and nothing beyond it. A greedy + // `/.*/ ` runs from the first slash in the file to the last and yields a + // pattern that matches nothing — which reads as "no project collects this + // spec" and escalated every spec-only change to the full suite. The literal + // may sit on the line after the `=`, contains no newline, and escapes its own + // slashes, so those are exactly the three things this allows. + const match = source.match(new RegExp(`const\\s+${name}\\s*=\\s*(/(?:[^/\\\\\\n]|\\\\.)+/)\\s*;`)); + if (!match) return null; + try { + return new RegExp(match[1].slice(1, -1)); + } catch { + return null; + } + }; + try { + const source = readFile(path.join(projectRoot, "playwright.config.ts"), "utf8"); + return { production: extract(source, "productionSpecPattern"), mockup: extract(source, "mockupSpecPattern") }; + } catch { + return { production: null, mockup: null }; + } +} + +/** + * The `--project` flags that will actually collect `specs`. + * + * `unroutable` is the fail-closed half: a spec neither project collects cannot be + * run by any selection, so the plan must escalate rather than emit a command that + * silently matches nothing. + * + * @param {string[]} specs + * @param {{ production: RegExp | null, mockup: RegExp | null }} patterns + * @returns {{ projects: string[], unroutable: string[] }} + */ +export function projectsForSpecs(specs, patterns) { + // Without both patterns there is nothing to route on; run both projects rather + // than guess which one a spec belongs to. + if (!patterns.production || !patterns.mockup) { + return { projects: ["chromium", "chromium-mockups"], unroutable: [] }; + } + const projects = new Set(); + const unroutable = []; + for (const spec of specs) { + const production = patterns.production.test(spec); + const mockup = patterns.mockup.test(spec); + if (production) projects.add("chromium"); + if (mockup) projects.add("chromium-mockups"); + if (!production && !mockup) unroutable.push(spec); + } + return { projects: [...projects].sort(), unroutable }; +} + +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] + * @param {{ production: RegExp | null, mockup: RegExp | null }} [input.specPatterns] + */ +export function browserTestPlan({ + files, + scope, + specSources, + sourceSources, + mode = "auto", + specPatterns = { production: null, mockup: null }, +}) { + 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(); + + // A selection is only real if some project collects it. `chromium` excludes + // `@mockup` and `chromium-mockups` collects only those, so the project is part + // of the selection, not a default. + const routing = projectsForSpecs(specsToRun, specPatterns); + if (level !== "full" && level !== "none" && routing.unroutable.length > 0) { + level = "full"; + reasons.push( + `No Playwright project collects ${routing.unroutable.join(", ")}, so a focused command would run nothing; the full suite runs instead.`, + ); + } + + 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, + ...routing.projects.map((project) => `--project=${project}`), + ], + }, + }); + } + + return { + files: normalized, + level, + stages, + specs: specsToRun, + attribution, + unattributed, + foundation, + projects: routing.projects, + unroutableSpecs: routing.unroutable, + unclassifiedBrowserFiles, + laneMirrorDrifted, + uiChanged, + reasons, + }; +} + +export function renderCommand(command) { + return [command.executable, ...command.args.map((arg) => (/\s|\|/.test(arg) ? JSON.stringify(arg) : arg))].join(" "); +} + +/* ------------------------------------------------------------------ * + * CLI * + * ------------------------------------------------------------------ */ + +/** + * A value flag written either way: `--files a,b` or `--files=a,b`. + * + * Accepting only the `=` spelling is not a cosmetic gap. The documented syntax is + * the two-token form, and a parser that ignores it does not error — it falls + * through to the `origin/main` diff and plans, or with `--run` EXECUTES, a + * different change from the one asked about. That is the same failure this file + * already guards against when calling `ci-change-scope.mjs`, and it was reported + * here as a P2 on PR #2553. + * + * @param {string[]} argv + * @param {string} name + * @returns {string | undefined} + */ +export function flagValue(argv, name) { + const inline = argv.find((argument) => argument.startsWith(`${name}=`)); + if (inline) return inline.slice(name.length + 1); + const index = argv.indexOf(name); + if (index < 0) return undefined; + const next = argv[index + 1]; + // `--files --run` is a missing value, not a value of "--run". + return next && !next.startsWith("--") ? next : undefined; +} + +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", '