diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 685fd92d8..995e69495 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -389,29 +389,27 @@ jobs: working-directory: apps/web run: mkdir -p ../../.codex/tmp/playwright-screenshots + - name: Select non-quarantined Playwright visual audits + id: visual-audits + run: | + pnpm exec tsx scripts/quality/select-playwright-visual-audits.ts \ + > /tmp/playwright-visual-audits.txt + echo "Selected Playwright visual audit specs:" + sed 's/^/ - /' /tmp/playwright-visual-audits.txt + - name: Run Playwright visual audit tests id: playwright - continue-on-error: true working-directory: apps/web run: | - find tests/playwright \ - -name '*audit*.spec.ts' \ - ! -name 'staging-*.spec.ts' \ - -print0 \ - | xargs -0 npx playwright test \ - --project='iPhone 14 (390x844)' \ - --reporter=list + xargs npx playwright test \ + --project='iPhone 14 (390x844)' \ + --reporter=list \ + < /tmp/playwright-visual-audits.txt env: CI: true - - name: Fail if Playwright timed out - if: steps.playwright.outcome == 'failure' - run: | - echo "::warning::Playwright visual audit tests had failures. See uploaded screenshots for details." - echo "Visual audit failures are informational — individual test fixes tracked separately." - - name: Upload screenshots on failure - if: failure() || steps.playwright.outcome == 'failure' + if: failure() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: playwright-screenshots diff --git a/apps/web/tests/playwright/visual-audit-quarantine.txt b/apps/web/tests/playwright/visual-audit-quarantine.txt new file mode 100644 index 000000000..4c8e0da66 --- /dev/null +++ b/apps/web/tests/playwright/visual-audit-quarantine.txt @@ -0,0 +1,112 @@ +# Playwright visual audit quarantine +# +# Context: SAM idea 01M0QED2G61RWM8Z3TPFCHEQXS measured the old CI job at +# ~34 minutes, with ~93% of worker time spent in known-red audit timeouts while +# the job still concluded success. This quarantine makes the remaining healthy +# visual audits blocking without deleting the stale corpus. +# +# Each entry is a basename under apps/web/tests/playwright. Remove an entry only +# after the spec passes in CI mode under the Playwright visual job project. +# +# Follow-up for repairing/retiring these stale specs: +# - tasks/backlog/2026-07-17-stale-playwright-audit-specs.md +# +admin-ai-proxy-audit.spec.ts +admin-analytics-audit.spec.ts +admin-chrome-theme-audit.spec.ts +admin-errors-audit.spec.ts +admin-trials-audit.spec.ts +admin-users-signup-approval-audit.spec.ts +agent-context-audit.spec.ts +agent-crash-report-audit.spec.ts +agent-guided-connect-audit.spec.ts +agent-settings-audit.spec.ts +ai-usage-audit.spec.ts +auth-flows-audit.spec.ts +chat-dom-bound-audit.spec.ts +chat-file-viewer-audit.spec.ts +chat-layout-scroll-audit.spec.ts +chat-light-mode-overlay-audit.spec.ts +chat-scroll-button-audit.spec.ts +chat-timeline-drawer-audit.spec.ts +chats-audit.spec.ts +chrome-theme-slice-audit.spec.ts +command-palette-audit.spec.ts +comments-navigation-audit.spec.ts +completion-dock-audit.spec.ts +compute-usage-audit.spec.ts +connections-ui-audit.spec.ts +credential-health-audit.spec.ts +debug-diagnosis-audit.spec.ts +deployment-control-surface-audit.spec.ts +deployment-settings-audit.spec.ts +deployment-volumes-audit.spec.ts +deployments-empty-state-audit.spec.ts +file-preview-modal-audit.spec.ts +focus-mode-audit.spec.ts +fork-retry-new-chat-audit.spec.ts +frontend-cache-audit.spec.ts +gcp-credential-audit.spec.ts +github-reauth-prompt-audit.spec.ts +global-audio-player-audit.spec.ts +idea-detail-audit.spec.ts +instant-runtime-recovery-audit.spec.ts +knowledge-ui-audit.spec.ts +landing-audit.spec.ts +lazy-route-chunks-audit.spec.ts +library-file-comments-audit.spec.ts +library-ui-audit.spec.ts +light-mode-admin-audit.spec.ts +light-mode-lists-audit.spec.ts +light-mode-settings-cluster-audit.spec.ts +light-mode-slice-b-audit.spec.ts +light-mode-tools-audit.spec.ts +markdown-chat-rendering-audit.spec.ts +mcp-servers-audit.spec.ts +message-comments-audit.spec.ts +nav-toggle-audit.spec.ts +nested-session-sidebar-audit.spec.ts +notification-tabs-audit.spec.ts +onboarding-inline-audit.spec.ts +onboarding-project-selector-audit.spec.ts +onboarding-wizard-theme-audit.spec.ts +platform-config-audit.spec.ts +portal-overlay-audit.spec.ts +profile-form-dialog-audit.spec.ts +profiles-ui-audit.spec.ts +project-agent-chat-audit.spec.ts +project-chat-composer-audit.spec.ts +project-chat-document-card-audit.spec.ts +project-chat-recoverable-error-audit.spec.ts +project-chat-streaming-poll-audit.spec.ts +project-chat-system-context-audit.spec.ts +project-chat-tool-call-audit.spec.ts +project-members-audit.spec.ts +project-onboarding-wizard-audit.spec.ts +project-settings-subpages-audit.spec.ts +provisioning-downgrade-audit.spec.ts +recent-chats-dropdown-audit.spec.ts +report-issue-audit.spec.ts +repository-access-audit.spec.ts +sam-prototype-audit.spec.ts +scaling-settings-audit.spec.ts +session-header-agent-info-audit.spec.ts +settings-credentials-audit.spec.ts +settings-digitalocean-credential-audit.spec.ts +settings-hetzner-validation-audit.spec.ts +settings-infomaniak-credential-audit.spec.ts +settings-notifications-audit.spec.ts +settings-upcloud-credential-audit.spec.ts +settings-vultr-credential-audit.spec.ts +shared-session-ux-audit.spec.ts +skills-ui-audit.spec.ts +slice-e-theme-audit.spec.ts +terminal-logout-cleanup-audit.spec.ts +theme-foundation-audit.spec.ts +theme-switcher-audit.spec.ts +trial-chat-gate-audit.spec.ts +trial-ui-audit.spec.ts +triggers-ui-audit.spec.ts +uncovered-pages-audit.spec.ts +wake-progress-audit.spec.ts +workspace-node-chrome-light-audit.spec.ts diff --git a/scripts/quality/ci-worker-suite.test.ts b/scripts/quality/ci-worker-suite.test.ts index 50334fc0a..a39ae2f6b 100644 --- a/scripts/quality/ci-worker-suite.test.ts +++ b/scripts/quality/ci-worker-suite.test.ts @@ -61,3 +61,33 @@ describe('CI Worker and Durable Object suite wiring', () => { ); }); }); + +describe('CI Playwright visual audit wiring', () => { + function expectBlockingPlaywrightVisualJob(workflow: string): void { + const job = jobBlock(workflow, 'playwright-visual'); + const selectionStep = stepBlock(job, 'Select non-quarantined Playwright visual audits'); + const runStep = stepBlock(job, 'Run Playwright visual audit tests'); + + expect(job).toContain("if: github.event_name == 'pull_request' && needs.changes.outputs.web-ui == 'true'"); + expect(selectionStep).toContain('pnpm exec tsx scripts/quality/select-playwright-visual-audits.ts'); + expect(runStep).toContain('xargs npx playwright test'); + expect(runStep).toContain("--project='iPhone 14 (390x844)'"); + expect(runStep).not.toContain('continue-on-error'); + expect(job).not.toContain('Visual audit failures are informational'); + expect(job).not.toContain('Fail if Playwright timed out'); + expect(job).toContain('if: failure()'); + } + + it('runs selected Playwright visual audits as a blocking PR-only web-ui gate', () => { + expectBlockingPlaywrightVisualJob(readCiWorkflow()); + }); + + it('fails if Playwright is made warn-only again', () => { + const warnOnlyWorkflow = readCiWorkflow().replace( + ' working-directory: apps/web\n run: |\n xargs npx playwright test', + ' continue-on-error: true\n working-directory: apps/web\n run: |\n xargs npx playwright test' + ); + + expect(() => expectBlockingPlaywrightVisualJob(warnOnlyWorkflow)).toThrow(); + }); +}); diff --git a/scripts/quality/playwright-visual-audits.test.ts b/scripts/quality/playwright-visual-audits.test.ts new file mode 100644 index 000000000..efe4c7024 --- /dev/null +++ b/scripts/quality/playwright-visual-audits.test.ts @@ -0,0 +1,48 @@ +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { selectPlaywrightVisualAudits } from './select-playwright-visual-audits'; + +function createFixture(files: string[], quarantine: string): string { + const root = mkdtempSync(path.join(tmpdir(), 'sam-playwright-visual-')); + const testsDir = path.join(root, 'apps/web/tests/playwright'); + mkdirSync(testsDir, { recursive: true }); + for (const file of files) { + writeFileSync(path.join(testsDir, file), 'test fixture\n'); + } + writeFileSync(path.join(testsDir, 'visual-audit-quarantine.txt'), quarantine); + return root; +} + +describe('Playwright visual audit selector', () => { + it('selects non-staging audit specs that are not quarantined', () => { + const root = createFixture( + [ + 'failure-card-audit.spec.ts', + 'legacy-audit.spec.ts', + 'staging-real-env-audit.spec.ts', + 'component.spec.ts', + ], + 'legacy-audit.spec.ts\n' + ); + + expect(selectPlaywrightVisualAudits(root).selected).toEqual([ + 'tests/playwright/failure-card-audit.spec.ts', + ]); + }); + + it('fails closed when quarantine references a missing file', () => { + const root = createFixture(['failure-card-audit.spec.ts'], 'missing-audit.spec.ts\n'); + + expect(() => selectPlaywrightVisualAudits(root)).toThrow('file does not exist'); + }); + + it('fails closed when every visual audit is quarantined', () => { + const root = createFixture(['failure-card-audit.spec.ts'], 'failure-card-audit.spec.ts\n'); + + expect(() => selectPlaywrightVisualAudits(root)).toThrow('selection is empty'); + }); +}); diff --git a/scripts/quality/select-playwright-visual-audits.ts b/scripts/quality/select-playwright-visual-audits.ts new file mode 100644 index 000000000..b40c9943a --- /dev/null +++ b/scripts/quality/select-playwright-visual-audits.ts @@ -0,0 +1,88 @@ +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const DEFAULT_WEB_DIR = 'apps/web'; +const PLAYWRIGHT_DIR = 'tests/playwright'; +const QUARANTINE_FILE = 'visual-audit-quarantine.txt'; + +export interface PlaywrightVisualAuditSelection { + selected: string[]; + quarantined: string[]; +} + +function repoRootFromCurrentFile(): string { + return path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); +} + +function compareAlphabetically(left: string, right: string): number { + return left.localeCompare(right); +} + +function parseArgs(argv: string[]): { webDir: string } { + const webDirArg = argv.find((arg) => arg.startsWith('--web-dir=')); + return { webDir: webDirArg?.slice('--web-dir='.length) || DEFAULT_WEB_DIR }; +} + +function readQuarantine(quarantinePath: string): string[] { + const contents = readFileSync(quarantinePath, 'utf8'); + return contents + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith('#')); +} + +function assertValidQuarantineEntry(entry: string, testsDir: string): void { + if (entry.includes('/') || entry.includes('\\')) { + throw new Error(`Invalid Playwright quarantine entry "${entry}": use a basename only.`); + } + if (!entry.endsWith('audit.spec.ts')) { + throw new Error(`Invalid Playwright quarantine entry "${entry}": expected *audit.spec.ts.`); + } + if (entry.startsWith('staging-')) { + throw new Error(`Invalid Playwright quarantine entry "${entry}": staging specs are never part of this CI job.`); + } + + const absolutePath = path.join(testsDir, entry); + if (!existsSync(absolutePath) || !statSync(absolutePath).isFile()) { + throw new Error(`Invalid Playwright quarantine entry "${entry}": file does not exist.`); + } +} + +export function selectPlaywrightVisualAudits(repoRoot: string, webDir = DEFAULT_WEB_DIR): PlaywrightVisualAuditSelection { + const testsDir = path.join(repoRoot, webDir, PLAYWRIGHT_DIR); + const quarantinePath = path.join(testsDir, QUARANTINE_FILE); + const quarantined = readQuarantine(quarantinePath); + const quarantineSet = new Set(); + + for (const entry of quarantined) { + if (quarantineSet.has(entry)) { + throw new Error(`Duplicate Playwright quarantine entry "${entry}".`); + } + assertValidQuarantineEntry(entry, testsDir); + quarantineSet.add(entry); + } + + const selected = readdirSync(testsDir) + .filter((fileName) => fileName.endsWith('audit.spec.ts')) + .filter((fileName) => !fileName.startsWith('staging-')) + .filter((fileName) => !quarantineSet.has(fileName)) + .sort(compareAlphabetically) + .map((fileName) => path.posix.join(PLAYWRIGHT_DIR, fileName)); + + if (selected.length === 0) { + throw new Error('Playwright visual audit selection is empty. Repair at least one audit before running CI.'); + } + + return { selected, quarantined: [...quarantineSet].sort(compareAlphabetically) }; +} + +function main(): void { + const { webDir } = parseArgs(process.argv.slice(2)); + const { selected } = selectPlaywrightVisualAudits(repoRootFromCurrentFile(), webDir); + process.stdout.write(`${selected.join('\n')}\n`); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main(); +} diff --git a/tasks/archive/2026-08-25-repair-playwright-visual-ci.md b/tasks/archive/2026-08-25-repair-playwright-visual-ci.md new file mode 100644 index 000000000..1ab6e0d42 --- /dev/null +++ b/tasks/archive/2026-08-25-repair-playwright-visual-ci.md @@ -0,0 +1,47 @@ +# Repair Playwright visual CI job + +## Problem + +The `Playwright Visual Tests` CI job currently runs the visual audit corpus on UI PRs, but it is not a real gate: + +- it runs every non-staging `*audit*.spec.ts` file; +- many audit specs are stale and time out on current `main`; +- the job is marked `continue-on-error`; +- the follow-up failure step only emits a warning. + +The result is bad in both directions: UI PRs pay roughly 34 minutes of wall-clock time, but the job still concludes success and does not protect the merge. + +## Research findings + +- The August CI/CD performance analysis is stored as SAM idea `01M0QED2G61RWM8Z3TPFCHEQXS`. +- That analysis found the Playwright job spent roughly 93% of worker time in timeouts and still returned success. +- It recommended quarantining the stale/red visual audit specs and making the remaining healthy suite blocking. +- The workflow is currently path-gated to PRs touching `web-ui`, which is correct and should be preserved. +- Existing quality tests in `scripts/quality/` parse `.github/workflows/ci.yml` and are the right place to add regression coverage for workflow wiring. +- Older backlog `tasks/backlog/2026-07-17-stale-playwright-audit-specs.md` documents examples of stale audit specs, but the CI repair should not wait for every stale spec to be fixed. + +## Implementation checklist + +- [x] Add an explicit quarantine file for stale Playwright audit specs. +- [x] Add a deterministic Playwright audit selector script that: + - includes non-staging `*audit*.spec.ts` files; + - excludes quarantined specs; + - fails closed if the quarantine references missing files, non-audit files, or staging specs; + - prints newline-delimited spec paths suitable for `xargs`. +- [x] Update `.github/workflows/ci.yml` so `playwright-visual`: + - keeps PR + `web-ui` path gating; + - runs only selected, non-quarantined audit specs; + - removes `continue-on-error`; + - removes the warning-only fake failure step; + - uploads screenshots on normal failure. +- [x] Add quality tests covering the selector and workflow wiring. +- [x] Run targeted validation, then the repository quality suite in proportion to risk. +- [x] Archive this task after validation. + +## Acceptance criteria + +- [x] CI no longer contains a warn-only Playwright visual gate. +- [x] The healthy Playwright visual audit subset is blocking. +- [x] Quarantined specs are explicit, reviewable, and have a tracked follow-up. +- [x] Workflow regression tests fail if someone reintroduces `continue-on-error` or bypasses the selector. +- [x] The implementation preserves the existing PR + `web-ui` path gate.