Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
112 changes: 112 additions & 0 deletions apps/web/tests/playwright/visual-audit-quarantine.txt
Original file line number Diff line number Diff line change
@@ -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
30 changes: 30 additions & 0 deletions scripts/quality/ci-worker-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
48 changes: 48 additions & 0 deletions scripts/quality/playwright-visual-audits.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
88 changes: 88 additions & 0 deletions scripts/quality/select-playwright-visual-audits.ts
Original file line number Diff line number Diff line change
@@ -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<string>();

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();
}
47 changes: 47 additions & 0 deletions tasks/archive/2026-08-25-repair-playwright-visual-ci.md
Original file line number Diff line number Diff line change
@@ -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.
Loading