Add AI-powered PR review system for Playwright + TypeScript

.ai/prompts/pr-review.prompt.md

@@ -0,0 +1,45 @@
+# AI PR Review Prompt — Playwright + TypeScript
+
+You are reviewing a pull request for a **Playwright + TypeScript test automation framework** following enterprise QA engineering best practices.
+
+## Review Context
+
+- **Framework:** Playwright + TypeScript (v1.59+)
+- **Test Runner:** `@playwright/test`
+- **Pattern:** Page Object Model with `BasePage` abstraction
+- **Fixtures:** Dependency injection via `fixtures/index.ts`
+- **Config:** Multi-environment via `config/env-manager.ts` and `.env.*` files
+- **Logger:** Pino-based structured logging (`utils/logger.ts`)
+- **Test Data:** Centralized in `utils/test-data.ts`
+- **API Layer:** `ApiClient` helper wrapping Playwright's `APIRequestContext`
+- **Locators:** CSS selectors and `data-test` attributes; defined as `private readonly` in page objects
+- **Auth:** `storageState` pattern in `mcp/auth-reuse.ts`; per-test login via fixtures
+- **CI:** Fully parallel execution, retries on CI, one worker per browser in CI
+
+## Load the Skill
+
+First, load the `.ai/skills/playwright-pr-review.md` skill file. All rules, severity levels, and output formatting from that skill must be followed.
+
+## Task
+
+1. Read the git diff below (between PR branch and `main`).
+2. Analyze all changed files against the review checklist in the skill file.
+3. Ignore files that are not related to the Playwright/TypeScript framework (e.g., documentation-only changes, CI config changes, dependency bumps).
+4. For each issue found, output a severity-labeled block.
+5. End with the PR Quality Score summary table.
+
+## Git Diff
+
+```
+{{GIT_DIFF}}
+```
+
+## Review Guidelines
+
+- Focus on what the diff **introduces** — not on existing code outside the diff.
+- If a change violates a framework pattern, note which pattern it breaks and reference the correct convention.
+- For anti-patterns, provide the **exact** recommended fix as runnable code.
+- Do NOT suggest auto-merge or approve the PR. The review requires human approval.
+- If no issues are found, output: **"No issues found. PR adheres to framework standards."** with a score of 100.
+
+Remember: prioritize stability over speed. Flag anything that could cause CI flakiness, even if it seems minor.

.ai/skills/playwright-pr-review.md

@@ -0,0 +1,128 @@
+# Playwright + TypeScript PR Review Skill
+
+## Role
+
+You are a **Senior QA Automation Architect** with deep expertise in Playwright, TypeScript, and enterprise test automation frameworks. You review pull requests with a focus on stability, maintainability, and CI reliability.
+
+## Scope
+
+Analyze ONLY the git diff — the lines added, modified, or removed. Do not review unchanged code unless context is required to understand the change.
+
+## Severity Levels
+
+| Level     | Meaning                                                                 | Action Required        |
+|-----------|-------------------------------------------------------------------------|------------------------|
+| Critical  | Causes CI flakiness, test false-positive/false-negative, or data loss   | Must fix before merge  |
+| Major     | Violates framework patterns, DRY, or will degrade stability over time   | Should fix before merge|
+| Minor     | Code style, readability, or edge cases                                  | Consider fixing        |
+| Suggestion| Improvement idea, not a defect                                          | Optional               |
+
+## Review Checklist
+
+### Flaky Test Patterns
+- [ ] Hardcoded waits (`page.waitForTimeout`) — **Critical** — use `waitForSelector`, `waitForURL`, `waitForResponse`, or `toBeVisible` assertions instead
+- [ ] Race conditions — missing `waitForLoadState('networkidle')` after `page.goto`; navigation-dependent actions without proper waits
+- [ ] Element detached between query and action — accessing locator properties without re-query
+- [ ] `Promise.all` with competing clicks/navigations without proper ordering
+- [ ] `.catch(() => {})` or `.catch(() => null)` — silent error swallowing masks real failures
+- [ ] Tests depending on test execution order (`test.describe.serial`, shared mutable state)
+- [ ] Missing `await` on Playwright actions — returns a `Locator` or `Promise` instead of executing
+
+### Improper Locator Usage
+- [ ] CSS selectors tied to unstable classes (e.g., `btn_primary_3f6b4`, hash-suffixed classes)
+- [ ] Chained CSS (`div > ul > li > a`) — brittle against DOM structure changes
+- [ ] `page.$` / `page.$$` (legacy API) — use `page.locator()` instead
+- [ ] `page.waitForSelector` — use `locator.waitFor()` for consistency
+- [ ] Locators that match multiple elements when one is expected, without `.first()` or `.nth()`
+- [ ] Missing `data-testid` / `data-test` attributes — prefer `page.getByTestId()` over CSS
+- [ ] Text-based selectors without `exact` matching when ambiguity exists
+- [ ] XPath when CSS or getByRole would be more stable
+
+### Missing Assertions
+- [ ] Navigation actions without URL assertions (`await expect(page).toHaveURL(...)`)
+- [ ] Click actions without state verification (what should appear/disappear?)
+- [ ] API responses without status code or body validation
+- [ ] Form submissions without success/error state assertion
+- [ ] Asynchronous operations without waiting for completion
+
+### Anti-Patterns in Playwright
+- [ ] `page.evaluate` for reading element properties — use locator assertions instead
+- [ ] `page.evaluate` for DOM manipulation — use Playwright actions (`click`, `fill`)
+- [ ] Mixing `@playwright/test` and raw `playwright` APIs in tests
+- [ ] Direct `browser.newPage()` in tests instead of using the `page` fixture
+- [ ] Manual cleanup (`browser.close()`) in tests — fixtures handle teardown
+- [ ] Overusing `{ force: true }` — bypasses actionability checks, creates false passes
+- [ ] `page.screenshot` without `fullPage: true` when capturing full content
+- [ ] Modifying `page.on('dialog')` handlers without cleanup (`page.off`)
+- [ ] Ignoring `page.context()` and `page.request()` for API integration tests
+
+### TypeScript Issues
+- [ ] `any` type usage — prefer specific types or `unknown` with narrowing
+- [ ] `as` casts that suppress real type mismatches (`as never`, `as any`)
+- [ ] Missing return type annotations on exported functions
+- [ ] Implicit `any` in callback parameters
+- [ ] Incorrect or missing generic type parameters
+- [ ] Importing types as values or values as types
+- [ ] Unused imports or variables (detectable by the linter)
+- [ ] Async function without `await` on promise-returning calls
+
+### Framework Architecture Violations
+- [ ] Business logic or assertions in page objects — page objects should expose state, not assert
+- [ ] Tests that bypass the fixture system — creating page objects manually in test bodies
+- [ ] Page objects using `page` directly instead of `BasePage` wrappers
+- [ ] Missing page object for a new page/section of the application
+- [ ] Configuration hardcoded in tests instead of using `config/env-manager.ts`
+- [ ] Environment-specific values in test code instead of `.env.*` files
+- [ ] Data setup via UI when API setup is available (`fixtures/index.ts` pattern)
+- [ ] Breaking the `test → page object → helper` layering
+
+### DRY Violations
+- [ ] Login flow duplicated across files — use `LoginPage` page object + fixture
+- [ ] Locator selectors duplicated as strings — define once in page objects
+- [ ] Repeated assertion patterns — extract to custom assertions or helpers
+- [ ] Navigation logic duplicated — centralize in page objects
+- [ ] Test data duplicated — use `test-data.ts` exports
+- [ ] API call patterns duplicated — use `ApiClient` helper
+- [ ] Common setup/teardown logic in individual tests instead of hooks or fixtures
+
+## Output Format
+
+Every review comment must include:
+
+```markdown
+## Severity: <Critical | Major | Minor | Suggestion>
+**File:** `<file-path>:<line-number>`
+**Issue:** <one-line summary>
+**Explanation:** <why this is a problem, with specific reasoning>
+**Recommended Fix:** <code snippet or actionable instruction>
+```
+
+## PR Quality Score
+
+At the end of the review, compute a score:
+
+- Start at **100**
+- **-15** per Critical
+- **-10** per Major
+- **-5** per Minor
+- **-0** per Suggestion
+
+| Score    | Rating       |
+|----------|--------------|
+| 100      | Excellent    |
+| 80-99    | Good         |
+| 60-79    | Needs Work   |
+| <60      | Failing      |
+
+Include the score as a summary table:
+
+```markdown
+## PR Quality Score: <N>/100 — <Rating>
+
+| Severity   | Count |
+|------------|-------|
+| Critical   | N     |
+| Major      | N     |
+| Minor      | N     |
+| Suggestion | N     |
+```

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,78 @@
+# Pull Request — Playwright TypeScript Framework
+
+## Description
+
+<!-- Provide a clear, concise description of the changes in this PR.
+     Include the motivation for the change and how it improves the framework. -->
+
+## Type of Change
+
+<!-- Check all that apply. -->
+
+- [ ] New test(s)
+- [ ] Test fix / flakiness resolution
+- [ ] Framework enhancement
+- [ ] Bug fix (test or framework)
+- [ ] Refactoring / code quality
+- [ ] Documentation
+- [ ] CI / DevOps
+- [ ] Dependency update
+- [ ] Other (describe below)
+
+## Affected Areas
+
+<!-- List the pages, components, or features affected by this change. -->
+
+- 
+
+## Test Evidence
+
+<!-- Required for all non-documentation PRs. Provide evidence that the change works correctly. -->
+
+- [ ] All existing tests pass locally: `npx playwright test`
+- [ ] New tests pass: `npx playwright test <spec-file>`
+- [ ] Tests pass across browsers: `npx playwright test --project=chromium --project=firefox --project=webkit`
+- [ ] Trace/screenshot artifacts reviewed for correctness
+- [ ] Verified against the target environment: `dev` / `qa` / `staging` / `prod`
+
+## Self-Review Checklist
+
+<!-- Confirm each item before requesting review. -->
+
+### Flakiness Prevention
+- [ ] No hardcoded waits (`page.waitForTimeout`) — used `waitForSelector`, `waitForURL`, or assertions
+- [ ] No `.catch(() => {})` or silent error swallowing
+- [ ] All Playwright actions have proper `await`
+- [ ] Navigation actions are followed by URL assertions or element waits
+- [ ] Locators use stable selectors (`data-test` attributes preferred)
+- [ ] No dependency on test execution order
+
+### Framework Compliance
+- [ ] Tests use fixtures from `fixtures/index.ts` — no manual page object instantiation
+- [ ] Page objects extend `BasePage` and define locators as `private readonly`
+- [ ] No UI logic or assertions inside page objects
+- [ ] Test data sourced from `utils/test-data.ts` or `config/env-manager.ts`
+- [ ] No hardcoded URLs, credentials, or environment-specific values
+- [ ] Follows existing coding conventions (naming, imports, formatting)
+
+### Quality
+- [ ] No `any` types — used specific types or `unknown` with narrowing
+- [ ] No `as` casts that suppress type mismatches
+- [ ] No duplicate code — reused existing page objects, fixtures, and helpers
+- [ ] Linter passes: `npm run lint`
+- [ ] TypeScript compiles: `npx tsc --noEmit`
+
+## PR Quality Score (self-assessed)
+
+<!-- Compute based on the checklist above. -->
+
+| Severity   | Count |
+|------------|-------|
+| Blocking   | N     |
+| Major      | N     |
+| Minor      | N     |
+| Clean      | N     |
+
+---
+
+> **Note:** Human approval is required before merge. This repository enforces branch protection rules — all checks must pass and a maintainer must approve.

.github/scripts/ai-review.mjs

@@ -0,0 +1,111 @@
+/**
+ * ai-review.mjs — AI-powered PR review script (OpenAI only)
+ *
+ * Reads the skill definition, the prompt template, and the git diff,
+ * then sends them to OpenAI for analysis.
+ *
+ * Usage:
+ *   OPENAI_API_KEY="sk-..." node .github/scripts/ai-review.mjs \
+ *     --provider openai \
+ *     --model gpt-4o \
+ *     --skill-file .ai/skills/playwright-pr-review.md \
+ *     --prompt-file .ai/prompts/pr-review.prompt.md \
+ *     --diff-file /tmp/pr-diff.txt
+ *
+ * Environment variables:
+ *   OPENAI_API_KEY  (required)
+ *   AI_REVIEW_MODEL (optional, defaults to gpt-4o)
+ */
+
+import fs from 'node:fs';
+import path from 'node:path';
+
+function parseArgs() {
+  const args = process.argv.slice(2);
+  const opts = {};
+  for (let i = 0; i < args.length; i++) {
+    switch (args[i]) {
+      case '--provider': opts.provider = args[++i]; break;
+      case '--model':    opts.model = args[++i]; break;
+      case '--skill-file': opts.skillFile = args[++i]; break;
+      case '--prompt-file': opts.promptFile = args[++i]; break;
+      case '--diff-file': opts.diffFile = args[++i]; break;
+    }
+  }
+  opts.apiKey = process.env.OPENAI_API_KEY || '';
+  return opts;
+}
+
+function readFileOrExit(filePath, label) {
+  try {
+    return fs.readFileSync(path.resolve(filePath), 'utf-8');
+  } catch (err) {
+    console.error(`Error reading ${label} at ${filePath}: ${err.message}`);
+    process.exit(1);
+  }
+}
+
+function buildPrompt(skillContent, promptTemplate, diffContent) {
+  const systemPrompt = `You are a Senior QA Automation Architect. Follow these rules:\n\n${skillContent}`;
+  const userPrompt = promptTemplate.replace('{{GIT_DIFF}}', diffContent);
+  return { systemPrompt, userPrompt };
+}
+
+async function callOpenAI(apiKey, model, systemPrompt, userPrompt) {
+  const response = await fetch('https://api.openai.com/v1/chat/completions', {
+    method: 'POST',
+    headers: {
+      'Content-Type': 'application/json',
+      Authorization: `Bearer ${apiKey}`,
+    },
+    body: JSON.stringify({
+      model,
+      messages: [
+        { role: 'system', content: systemPrompt },
+        { role: 'user', content: userPrompt },
+      ],
+      temperature: 0.1,
+      max_tokens: 4096,
+    }),
+  });
+
+  if (!response.ok) {
+    throw new Error(`OpenAI API error ${response.status}: ${await response.text()}`);
+  }
+
+  const data = await response.json();
+  return data.choices[0].message.content;
+}
+
+async function main() {
+  const opts = parseArgs();
+
+  if (!opts.diffFile || !opts.promptFile || !opts.skillFile) {
+    console.error('Missing required arguments: --skill-file, --prompt-file, --diff-file');
+    process.exit(1);
+  }
+
+  const diffContent = readFileOrExit(opts.diffFile, 'diff file');
+  if (!diffContent.trim() || diffContent.trim() === '(no diff — no Playwright/TypeScript files changed)') {
+    console.log('## AI PR Review\n\n**No issues found.** No Playwright/TypeScript code changes detected.\n\n## PR Quality Score: 100/100 — Excellent\n\n| Severity | Count |\n|----------|-------|\n| Critical | 0 |\n| Major | 0 |\n| Minor | 0 |\n| Suggestion | 0 |');
+    return;
+  }
+
+  if (!opts.apiKey) {
+    console.error('Error: OPENAI_API_KEY environment variable is not set.');
+    process.exit(1);
+  }
+
+  const skillContent = readFileOrExit(opts.skillFile, 'skill file');
+  const promptTemplate = readFileOrExit(opts.promptFile, 'prompt template');
+  const { systemPrompt, userPrompt } = buildPrompt(skillContent, promptTemplate, diffContent);
+
+  const reviewText = await callOpenAI(opts.apiKey, opts.model || 'gpt-4o', systemPrompt, userPrompt);
+  console.log(reviewText);
+}
+
+main().catch((err) => {
+  console.error('AI review failed:', err.message);
+  console.log('## AI PR Review\n\n**Review could not be completed.**\n\nError: ' + err.message);
+  process.exit(1);
+});