diff --git a/.claude/rules/code-review.md b/.claude/rules/code-review.md index 11b1107991..73b1a587b0 100644 --- a/.claude/rules/code-review.md +++ b/.claude/rules/code-review.md @@ -24,7 +24,7 @@ Project-specific rules layered on top of `CLAUDE.md` for `/quality:agent-review` ## Critical File Patterns -Files that control auth, routing, Apollo setup, build config, or the CI/review pipeline. Each contributes +3 to risk score (on top of the universal defaults). +Highest-risk tier — files that control auth, routing, Apollo setup, build config, or the CI/review pipeline. Additive to the universal defaults. _(This file only sorts files into tiers; how each tier maps onto the risk axes is defined by the skill.)_ - `pages/api/auth/[...nextauth].page.ts` — NextAuth handler (OAuth callbacks, session) - `pages/api/auth/apiOauthSignIn.ts` — API OAuth sign-in flow @@ -34,7 +34,7 @@ Files that control auth, routing, Apollo setup, build config, or the CI/review p - `pages/_app.page.tsx` — app-level providers and global wiring - `next.config.{js,ts}` — build-time config, headers, rewrites, CSP - `codegen.ts`, `codegen.*.ts` — GraphQL codegen configuration -- `package.json` — dependency changes (new packages trigger `## Special Pattern Detection` below) +- `package.json` — dependency changes (supply-chain surface; the skill's universal signals also flag new/updated dependencies) - `.github/workflows/**` — CI/CD workflows - `src/lib/apollo/client.ts` — Apollo Client instantiation (auth headers, default options, link chain composition) - `src/lib/apollo/link.ts` — Apollo Link chain (dual-server routing, auth token attachment, error handling) @@ -44,7 +44,7 @@ Files that control auth, routing, Apollo setup, build config, or the CI/review p ## High-Risk File Patterns -Each contributes +2 to risk score. +Second tier — sensitive or wide-reach files below Critical. - `pages/api/Schema/**/*.{ts,graphql}` — REST-proxy resolvers, schemas, and data handlers (silent data-reshaping risk) - `pages/api/Schema/**/datahandler*.ts` — response transformation feeding the REST proxy @@ -58,7 +58,7 @@ Each contributes +2 to risk score. ## Medium-Risk File Patterns -Each contributes +1 to risk score. +Third tier — everyday feature, hook, and utility files. - `src/components/**/*.{ts,tsx}` — feature components (excluding Shared, already High-Risk) - `src/hooks/**/*.ts` — custom hooks @@ -69,7 +69,7 @@ Each contributes +1 to risk score. ## Low-Risk File Patterns -Zero points. These override or augment the universal Low-Risk defaults. +Lowest tier — files that may legitimately carry no risk. These override or augment the universal Low-Risk defaults. - `**/*.test.{ts,tsx}` — test files (content changes only; new test infrastructure should still be reviewed) - `public/locales/**/*.json` — translation content @@ -77,20 +77,6 @@ Zero points. These override or augment the universal Low-Risk defaults. - `public/fonts/**`, `public/images/**` — binary assets - `**/*.snap` — Jest snapshot files -## Special Pattern Detection - -Additional risk modifiers, added to the universal defaults. - -- **New package added to `package.json` dependencies/devDependencies:** +2 (supply-chain risk, bundle size impact) -- **Updated critical package** (`next`, `react`, `@apollo/client`, `@mui/material`, `formik`, `next-auth`, `typescript`, `graphql-codegen`): +3 -- **`yarn.lock` changed without matching `package.json` change:** +1 (likely a resolution drift or lockfile hand-edit) -- **`.graphql` file changed without verifying `yarn gql` runs cleanly:** +2 (codegen out of sync — runtime errors likely). Note: `.generated.ts` files are not committed; CI regenerates them. The check is that codegen succeeds, not that generated files appear in the PR -- **New `.graphql` file under `pages/api/Schema/` without matching resolver/dataHandler updates:** +1 -- **New file in `src/hooks/` that uses Apollo hooks without an accompanying test file:** +1 (hooks drive component behavior; untested hooks are landmines) -- **New file in `src/components/` without an accompanying `*.test.tsx`:** +1 -- **Changes to `next.config.{js,ts}` rewrites, headers, CSP, or image domains:** +2 (affects all pages and external requests) -- **Changes to `src/lib/apollo/cache.ts` `typePolicies` or `merge` functions:** +2 (can silently corrupt cached data across the app) - ## Agent Triggers Repo-specific triggers that supplement the skill's minimal universal triggers. @@ -280,6 +266,8 @@ This is where domain-specific data invariants belong. - **Pagination over `nodes`** — aggregating client-side over a single page of `nodes` silently ignores unpaginated data. Prefer server-provided totals - **Filter/search state** — when filters change, paginated data must be re-fetched from the first page, not appended to existing pages - **Optimistic updates on lists** — adding an item optimistically must place it in the correct sort position, not just at the end +- **Codegen sync** — when a `.graphql` operation or schema changes, `yarn gql` must run cleanly. `.generated.ts` files are not committed (CI regenerates them), so verify codegen _succeeds_ rather than expecting generated files in the PR; drift causes runtime type errors +- **New REST-proxy operations** — a new `.graphql` file under `pages/api/Schema/` needs a matching `resolvers.ts` / `datahandler.ts`, or it will fail at runtime ## Testing Focus Areas diff --git a/.github/workflows/ai-review-auto-approve.yml b/.github/workflows/ai-review-auto-approve.yml index ca6178a77e..467de4bf47 100644 --- a/.github/workflows/ai-review-auto-approve.yml +++ b/.github/workflows/ai-review-auto-approve.yml @@ -77,17 +77,19 @@ jobs: const risk_level = String(meta.risk_level || '').toUpperCase().trim(); const verdict = String(meta.verdict || '').toUpperCase().trim(); - if (isNaN(Number(meta.risk_score))) { - console.log(`Warning: risk_score "${meta.risk_score}" is not a number, defaulting to 0`); - } if (isNaN(Number(meta.blockers))) { console.log(`Warning: blockers "${meta.blockers}" is not a number, defaulting to 0`); } - const risk_score = Number(meta.risk_score) || 0; const blockers = Number(meta.blockers) || 0; const dismissed = Number(meta.dismissed) || 0; - console.log(`Agent review found — Risk: ${risk_level} (${risk_score}/10), Blockers: ${blockers}, Verdict: ${verdict}, Dismissed: ${dismissed}`); + + // Axis risk model. These are for display only. + const axes = meta.axes && typeof meta.axes === 'object' ? meta.axes : null; + const danger = axes && !isNaN(Number(axes.danger)) ? Number(axes.danger) : null; + const effort = axes && !isNaN(Number(axes.effort)) ? Number(axes.effort) : null; + const riskDetail = `danger ${danger ?? '?'}/6, effort ${effort ?? '?'}/6` + console.log(`Agent review found — Risk: ${risk_level} (${riskDetail}), Blockers: ${blockers}, Verdict: ${verdict}, Dismissed: ${dismissed}`); // Gate 1: Only LOW or MEDIUM risk if (!['LOW', 'MEDIUM'].includes(risk_level)) { @@ -133,7 +135,7 @@ jobs: const bodyLines = [ '## AI Review Auto-Approval', '', - `**Risk Level:** ${risk_level} (${risk_score}/10)`, + `**Risk Level:** ${risk_level} (${riskDetail})`, `**Verdict:** ${verdictDesc}`, '', 'This PR was auto-approved because:',