From 71e1441257f330a557344384f5db78e023226052 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 16 Apr 2026 11:25:06 -0400 Subject: [PATCH] chore: Refactor PR classification + add tests (#2129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Refactor for readability + add tests - Allow small agent-branch PRs to reach Tier 2 (< 50 prod lines, ≤ 3 files) - Add cross-layer detection to keep multi-package changes at Tier 3+ - Raise Tier 2 ceiling from 100 → 150 production lines - Exclude test and changeset files from line counts used for tier decisions - Extract pure classification logic into .github/scripts/ for testability - Gate classify job on tests passing in pr-triage.yml - Only CI changes to main.yaml and release.yaml are tier 4 - Test changes in critical paths don't count toward tiering decisions --- .../__tests__/pr-triage-classify.test.js | 569 ++++++++++++++++++ .github/scripts/pr-triage-classify.js | 257 ++++++++ .github/scripts/pr-triage.js | 123 ++++ .github/workflows/pr-triage.yml | 284 +-------- Makefile | 4 + knip.json | 2 +- 6 files changed, 977 insertions(+), 262 deletions(-) create mode 100644 .github/scripts/__tests__/pr-triage-classify.test.js create mode 100644 .github/scripts/pr-triage-classify.js create mode 100644 .github/scripts/pr-triage.js diff --git a/.github/scripts/__tests__/pr-triage-classify.test.js b/.github/scripts/__tests__/pr-triage-classify.test.js new file mode 100644 index 00000000..8b03d3af --- /dev/null +++ b/.github/scripts/__tests__/pr-triage-classify.test.js @@ -0,0 +1,569 @@ +'use strict'; + +// Tests for the pure classification functions in pr-triage-classify.js. +// Uses Node's built-in test runner (no extra dependencies required). +// Run with: node --test .github/scripts/__tests__/pr-triage-classify.test.js + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + isTestFile, isTrivialFile, isCriticalFile, + computeSignals, determineTier, buildTierComment, +} = require('../pr-triage-classify'); + +// ── Test helpers ───────────────────────────────────────────────────────────── + +/** Minimal PR object matching the shape returned by the GitHub API */ +function makePR(login, ref) { + return { user: { login }, head: { ref } }; +} + +/** Minimal file entry matching the shape returned by pulls.listFiles */ +function makeFile(filename, additions = 10, deletions = 5) { + return { filename, additions, deletions }; +} + +/** Classify a PR end-to-end from raw inputs (the common test path) */ +function classify(login, ref, files) { + return determineTier(computeSignals(makePR(login, ref), files)); +} + +// ── File classification helpers ────────────────────────────────────────────── + +describe('isTestFile', () => { + it('matches __tests__ directory', () => { + assert.ok(isTestFile('packages/api/src/__tests__/foo.test.ts')); + assert.ok(isTestFile('packages/app/src/components/__tests__/Foo.test.tsx')); + }); + + it('matches .test.* and .spec.* extensions', () => { + assert.ok(isTestFile('packages/app/src/Foo.test.tsx')); + assert.ok(isTestFile('packages/app/src/Foo.spec.js')); + assert.ok(isTestFile('packages/api/src/bar.test.ts')); + }); + + it('matches packages/app/tests/ prefix', () => { + assert.ok(isTestFile('packages/app/tests/e2e/navigation.ts')); + }); + + it('does not match regular source files', () => { + assert.ok(!isTestFile('packages/api/src/routers/foo.ts')); + assert.ok(!isTestFile('packages/app/src/App.tsx')); + }); +}); + +describe('isTrivialFile', () => { + it('matches docs and images', () => { + assert.ok(isTrivialFile('README.md')); + assert.ok(isTrivialFile('docs/setup.txt')); + assert.ok(isTrivialFile('assets/logo.png')); + assert.ok(isTrivialFile('assets/icon.svg')); + }); + + it('matches lock files and yarn config', () => { + assert.ok(isTrivialFile('yarn.lock')); + assert.ok(isTrivialFile('package-lock.json')); + assert.ok(isTrivialFile('.yarnrc.yml')); + }); + + it('matches .changeset/ files', () => { + assert.ok(isTrivialFile('.changeset/some-change.md')); + assert.ok(isTrivialFile('.changeset/fancy-bears-dance.md')); + }); + + it('matches .env.example and .github/images/', () => { + assert.ok(isTrivialFile('.env.example')); + assert.ok(isTrivialFile('.github/images/screenshot.png')); + }); + + it('matches .github/scripts/ files', () => { + assert.ok(isTrivialFile('.github/scripts/pr-triage.js')); + assert.ok(isTrivialFile('.github/scripts/pr-triage-classify.js')); + }); + + it('matches .github/workflows/ files', () => { + assert.ok(isTrivialFile('.github/workflows/pr-triage.yml')); + assert.ok(isTrivialFile('.github/workflows/knip.yml')); + // main.yml and release.yml are also trivial per isTrivialFile, but they are + // caught first by isCriticalFile in computeSignals, so they still → Tier 4 + assert.ok(isTrivialFile('.github/workflows/main.yml')); + }); + + it('does not match production source files', () => { + assert.ok(!isTrivialFile('packages/app/src/App.tsx')); + assert.ok(!isTrivialFile('packages/api/src/routers/logs.ts')); + assert.ok(!isTrivialFile('Makefile')); + assert.ok(!isTrivialFile('knip.json')); + }); +}); + +describe('isCriticalFile', () => { + it('matches auth middleware', () => { + assert.ok(isCriticalFile('packages/api/src/middleware/auth.ts')); + assert.ok(isCriticalFile('packages/api/src/middleware/auth/index.ts')); + }); + + it('matches sensitive API routes', () => { + assert.ok(isCriticalFile('packages/api/src/routers/api/me.ts')); + assert.ok(isCriticalFile('packages/api/src/routers/api/team.ts')); + assert.ok(isCriticalFile('packages/api/src/routers/external-api/logs.ts')); + }); + + it('matches core data models', () => { + assert.ok(isCriticalFile('packages/api/src/models/user.ts')); + assert.ok(isCriticalFile('packages/api/src/models/team.ts')); + assert.ok(isCriticalFile('packages/api/src/models/teamInvite.ts')); + }); + + it('matches config, tasks, otel, clickhouse, and core CI workflows', () => { + assert.ok(isCriticalFile('packages/api/src/config.ts')); + assert.ok(isCriticalFile('packages/api/src/tasks/alertChecker.ts')); + assert.ok(isCriticalFile('packages/otel-collector/config.yaml')); + assert.ok(isCriticalFile('docker/clickhouse/config.xml')); + assert.ok(isCriticalFile('.github/workflows/main.yml')); + assert.ok(isCriticalFile('.github/workflows/release.yml')); + }); + + it('does NOT flag non-core workflow files as critical', () => { + assert.ok(!isCriticalFile('.github/workflows/pr-triage.yml')); + assert.ok(!isCriticalFile('.github/workflows/knip.yml')); + assert.ok(!isCriticalFile('.github/workflows/claude.yml')); + }); + + it('matches docker/hyperdx/', () => { + assert.ok(isCriticalFile('docker/hyperdx/Dockerfile')); + }); + + it('does NOT match non-critical API models', () => { + assert.ok(!isCriticalFile('packages/api/src/models/alert.ts')); + assert.ok(!isCriticalFile('packages/api/src/models/dashboard.ts')); + }); + + it('does NOT match regular app and API files', () => { + assert.ok(!isCriticalFile('packages/app/src/App.tsx')); + assert.ok(!isCriticalFile('packages/api/src/routers/logs.ts')); + }); + + // Note: isCriticalFile DOES return true for test files under critical paths + // (e.g. packages/api/src/tasks/tests/util.test.ts). The exclusion happens in + // computeSignals, which filters test files out before building criticalFiles. + it('returns true for test files under critical paths (exclusion is in computeSignals)', () => { + assert.ok(isCriticalFile('packages/api/src/tasks/tests/util.test.ts')); + }); +}); + +// ── computeSignals ─────────────────────────────────────────────────────────── + +describe('computeSignals', () => { + it('separates prod, test, and trivial file line counts', () => { + const pr = makePR('alice', 'feature/foo'); + const files = [ + makeFile('packages/app/src/Foo.tsx', 20, 5), // prod: 25 lines + makeFile('packages/app/src/__tests__/Foo.test.tsx', 50, 0), // test: 50 lines + makeFile('README.md', 2, 1), // trivial: excluded + ]; + const s = computeSignals(pr, files); + assert.equal(s.prodFiles.length, 1); + assert.equal(s.prodLines, 25); + assert.equal(s.testLines, 50); + }); + + it('excludes changeset files from prod counts', () => { + const pr = makePR('alice', 'feature/foo'); + const files = [ + makeFile('packages/app/src/Foo.tsx', 20, 5), + makeFile('.changeset/witty-foxes-run.md', 5, 0), // trivial + ]; + const s = computeSignals(pr, files); + assert.equal(s.prodFiles.length, 1); + assert.equal(s.prodLines, 25); + }); + + it('detects agent branches by prefix', () => { + for (const prefix of ['claude/', 'agent/', 'ai/']) { + const s = computeSignals(makePR('alice', `${prefix}fix-thing`), []); + assert.ok(s.isAgentBranch, `expected isAgentBranch for prefix "${prefix}"`); + } + assert.ok(!computeSignals(makePR('alice', 'feature/normal'), []).isAgentBranch); + }); + + it('detects bot authors', () => { + assert.ok(computeSignals(makePR('dependabot[bot]', 'dependabot/npm/foo'), []).isBotAuthor); + assert.ok(!computeSignals(makePR('alice', 'feature/foo'), []).isBotAuthor); + }); + + it('sets allFilesTrivial when every file is trivial', () => { + const files = [makeFile('README.md'), makeFile('yarn.lock')]; + assert.ok(computeSignals(makePR('alice', 'docs/update'), files).allFilesTrivial); + }); + + it('does not set allFilesTrivial for mixed files', () => { + const files = [makeFile('README.md'), makeFile('packages/app/src/Foo.tsx')]; + assert.ok(!computeSignals(makePR('alice', 'feat/foo'), files).allFilesTrivial); + }); + + it('detects cross-layer changes (frontend + backend)', () => { + const files = [ + makeFile('packages/app/src/NewFeature.tsx'), // frontend + makeFile('packages/api/src/services/newFeature.ts'), // backend (not models/routers) + ]; + const s = computeSignals(makePR('alice', 'feat/new'), files); + assert.ok(s.isCrossLayer); + assert.ok(s.touchesFrontend); + assert.ok(s.touchesBackend); + }); + + it('detects cross-layer changes (backend + shared-utils)', () => { + const files = [ + makeFile('packages/api/src/services/foo.ts'), + makeFile('packages/common-utils/src/queryParser.ts'), + ]; + const s = computeSignals(makePR('alice', 'feat/foo'), files); + assert.ok(s.isCrossLayer); + assert.ok(s.touchesSharedUtils); + }); + + it('does not flag single-package changes as cross-layer', () => { + const files = [ + makeFile('packages/app/src/Foo.tsx'), + makeFile('packages/app/src/Bar.tsx'), + ]; + assert.ok(!computeSignals(makePR('alice', 'feat/foo'), files).isCrossLayer); + }); + + it('blocks agent branch from Tier 2 when prod lines exceed threshold', () => { + // 60 prod lines > AGENT_TIER2_MAX_LINES (50) + const s = computeSignals(makePR('alice', 'claude/feature'), [ + makeFile('packages/app/src/Foo.tsx', 60, 0), + ]); + assert.ok(s.agentBlocksTier2); + }); + + it('blocks agent branch from Tier 2 when prod file count exceeds threshold', () => { + // 5 prod files > AGENT_TIER2_MAX_PROD_FILES (3) + const files = Array.from({ length: 5 }, (_, i) => + makeFile(`packages/app/src/File${i}.tsx`, 5, 2) + ); + const s = computeSignals(makePR('alice', 'claude/feature'), files); + assert.ok(s.agentBlocksTier2); + }); + + it('does NOT block agent branch when change is small and focused', () => { + // 16 prod lines, 1 prod file — well under both thresholds + const s = computeSignals(makePR('mikeshi', 'claude/fix-mobile-nav'), [ + makeFile('packages/app/src/AppNav.tsx', 11, 5), + ]); + assert.ok(!s.agentBlocksTier2); + }); +}); + +// ── determineTier ──────────────────────────────────────────────────────────── + +describe('determineTier', () => { + describe('Tier 1', () => { + it('bot author', () => { + assert.equal(classify('dependabot[bot]', 'dependabot/npm/foo', [ + makeFile('package.json', 5, 3), + ]), 1); + }); + + // package.json is not in TIER1_PATTERNS (it's a production file), but bot + // author short-circuits to Tier 1 before the trivial-file check fires. + it('bot author with package.json (non-trivial file) is still Tier 1', () => { + assert.equal(classify('dependabot[bot]', 'dependabot/npm/lodash', [ + makeFile('package.json', 5, 3), + makeFile('packages/api/package.json', 2, 2), + ]), 1); + }); + + it('all trivial files (docs + lock)', () => { + assert.equal(classify('alice', 'docs/update-readme', [ + makeFile('README.md', 10, 2), + makeFile('docs/setup.md', 5, 0), + makeFile('yarn.lock', 100, 80), + ]), 1); + }); + + it('changeset-only PR', () => { + assert.equal(classify('alice', 'release/v2.1', [ + makeFile('.changeset/witty-foxes-run.md', 4, 0), + ]), 1); + }); + }); + + describe('Tier 4', () => { + it('touches auth middleware', () => { + assert.equal(classify('alice', 'fix/auth-bug', [ + makeFile('packages/api/src/middleware/auth.ts', 20, 5), + ]), 4); + }); + + it('touches ClickHouse docker config', () => { + assert.equal(classify('alice', 'infra/clickhouse-update', [ + makeFile('docker/clickhouse/config.xml', 10, 2), + ]), 4); + }); + + it('touches main.yml or release.yml', () => { + assert.equal(classify('alice', 'ci/add-step', [ + makeFile('.github/workflows/main.yml', 15, 3), + ]), 4); + assert.equal(classify('alice', 'ci/release-fix', [ + makeFile('.github/workflows/release.yml', 8, 2), + ]), 4); + }); + + it('non-critical workflow-only changes are Tier 1 (workflow files are trivial)', () => { + assert.equal(classify('alice', 'ci/add-triage-step', [ + makeFile('.github/workflows/pr-triage.yml', 10, 2), + ]), 1); + }); + + it('does NOT flag test files under critical paths as Tier 4', () => { + // e.g. packages/api/src/tasks/tests/util.test.ts should not be critical + assert.equal(classify('alice', 'feat/alert-tests', [ + makeFile('packages/api/src/tasks/tests/util.test.ts', 40, 0), + makeFile('packages/api/src/tasks/checkAlerts/tests/checkAlerts.test.ts', 80, 0), + ]), 2); + }); + + it('touches core user/team models', () => { + assert.equal(classify('alice', 'feat/user-fields', [ + makeFile('packages/api/src/models/user.ts', 10, 2), + ]), 4); + }); + + it('escalates Tier 3 human branch past 1000 prod lines', () => { + assert.equal(classify('alice', 'feat/huge-refactor', [ + makeFile('packages/app/src/BigComponent.tsx', 600, 450), // 1050 lines + ]), 4); + }); + + it('escalates Tier 3 agent branch past 400 prod lines (stricter threshold)', () => { + assert.equal(classify('alice', 'claude/large-feature', [ + makeFile('packages/app/src/BigFeature.tsx', 300, 120), // 420 lines + ]), 4); + }); + }); + + describe('Tier 2', () => { + it('small single-layer frontend change', () => { + assert.equal(classify('alice', 'fix/button-style', [ + makeFile('packages/app/src/components/Button.tsx', 20, 10), + ]), 2); + }); + + it('small single-layer backend change (not models/routers)', () => { + assert.equal(classify('alice', 'fix/service-bug', [ + makeFile('packages/api/src/services/logs.ts', 30, 15), + ]), 2); + }); + + it('agent branch small enough to qualify (PR #1431 pattern: 1 file, 16 lines)', () => { + assert.equal(classify('mikeshi', 'claude/fix-mobile-nav', [ + makeFile('packages/app/src/AppNav.tsx', 11, 5), + ]), 2); + }); + + it('agent branch exactly at file limit (3 prod files, small lines)', () => { + const files = Array.from({ length: 3 }, (_, i) => + makeFile(`packages/app/src/File${i}.tsx`, 10, 5) + ); + assert.equal(classify('alice', 'claude/small-multi', files), 2); + }); + + it('human branch at 149 prod lines (just under threshold)', () => { + assert.equal(classify('alice', 'fix/component', [ + makeFile('packages/app/src/Foo.tsx', 100, 49), // 149 lines + ]), 2); + }); + + it('agent branch at exactly 49 prod lines qualifies for Tier 2', () => { + assert.equal(classify('alice', 'claude/fix', [ + makeFile('packages/app/src/Foo.tsx', 49, 0), + ]), 2); + }); + }); + + describe('Tier 3', () => { + it('cross-layer change (frontend + backend)', () => { + assert.equal(classify('alice', 'feat/new-feature', [ + makeFile('packages/app/src/NewFeature.tsx', 30, 5), + makeFile('packages/api/src/services/newFeature.ts', 40, 10), + ]), 3); + }); + + it('touches API routes (non-critical)', () => { + assert.equal(classify('alice', 'feat/new-route', [ + makeFile('packages/api/src/routers/logs.ts', 30, 5), + ]), 3); + }); + + it('touches API models (non-critical)', () => { + assert.equal(classify('alice', 'feat/model-field', [ + makeFile('packages/api/src/models/alert.ts', 20, 3), + ]), 3); + }); + + it('agent branch at exactly 50 prod lines is blocked from Tier 2', () => { + assert.equal(classify('alice', 'claude/feature', [ + makeFile('packages/app/src/Foo.tsx', 50, 0), // exactly AGENT_TIER2_MAX_LINES — >= blocks it + ]), 3); + }); + + it('agent branch over prod-line threshold (60 > 50) → Tier 3, not Tier 2', () => { + assert.equal(classify('alice', 'claude/medium-feature', [ + makeFile('packages/app/src/Foo.tsx', 60, 0), + ]), 3); + }); + + it('agent branch over file count threshold (4 files) → Tier 3', () => { + const files = Array.from({ length: 4 }, (_, i) => + makeFile(`packages/app/src/File${i}.tsx`, 10, 5) + ); + assert.equal(classify('alice', 'claude/big-feature', files), 3); + }); + + it('does NOT escalate agent branch at exactly 400 lines (threshold is exclusive)', () => { + // prodLines > threshold, not >=, so 400 stays at Tier 3 + assert.equal(classify('alice', 'claude/medium-large', [ + makeFile('packages/app/src/Feature.tsx', 200, 200), // exactly 400 + ]), 3); + }); + + it('large test additions with small prod change stay Tier 3 (PR #2122 pattern)', () => { + // Alert threshold PR: 1300 total adds but ~1100 are tests + const files = [ + makeFile('packages/api/src/services/checkAlerts.ts', 180, 70), // prod: 250 lines + makeFile('packages/api/src/__tests__/checkAlerts.test.ts', 1100, 0), // test: excluded + ]; + // 250 prod lines > TIER2_MAX_LINES (150) → Tier 3, not Tier 4 + assert.equal(classify('alice', 'feat/alert-thresholds', files), 3); + }); + + it('human branch at exactly 150 prod lines is Tier 3, not Tier 2', () => { + assert.equal(classify('alice', 'fix/component', [ + makeFile('packages/app/src/Foo.tsx', 100, 50), // exactly TIER2_MAX_LINES — < is exclusive + ]), 3); + }); + + it('does NOT escalate human branch at exactly 1000 prod lines', () => { + assert.equal(classify('alice', 'feat/medium-large', [ + makeFile('packages/app/src/Feature.tsx', 500, 500), // exactly 1000 + ]), 3); + }); + }); +}); + +// ── buildTierComment ───────────────────────────────────────────────────────── + +describe('buildTierComment', () => { + /** Build a signal object with sensible defaults, overrideable per test */ + function makeSignals(overrides = {}) { + return { + author: 'alice', + branchName: 'feature/foo', + prodFiles: [makeFile('packages/app/src/Foo.tsx')], + prodLines: 50, + testLines: 0, + criticalFiles: [], + isAgentBranch: false, + isBotAuthor: false, + allFilesTrivial: false, + touchesApiModels: false, + touchesFrontend: true, + touchesBackend: false, + touchesSharedUtils: false, + isCrossLayer: false, + agentBlocksTier2: false, + ...overrides, + }; + } + + it('always includes the pr-triage sentinel marker', () => { + assert.ok(buildTierComment(2, makeSignals()).includes('')); + }); + + it('includes the correct headline for each tier', () => { + assert.ok(buildTierComment(1, makeSignals()).includes('Tier 1')); + assert.ok(buildTierComment(2, makeSignals()).includes('Tier 2')); + assert.ok(buildTierComment(3, makeSignals()).includes('Tier 3')); + assert.ok(buildTierComment(4, makeSignals()).includes('Tier 4')); + }); + + it('includes override instructions with the correct tier label', () => { + const body = buildTierComment(3, makeSignals()); + assert.ok(body.includes('review/tier-3')); + assert.ok(body.includes('Manual overrides are preserved')); + }); + + it('lists critical files when present', () => { + const signals = makeSignals({ + criticalFiles: [makeFile('packages/api/src/middleware/auth.ts')], + }); + const body = buildTierComment(4, signals); + assert.ok(body.includes('Critical-path files')); + assert.ok(body.includes('auth.ts')); + }); + + it('explains cross-layer trigger with which layers are involved', () => { + const signals = makeSignals({ + isCrossLayer: true, + touchesFrontend: true, + touchesBackend: true, + touchesSharedUtils: false, + }); + const body = buildTierComment(3, signals); + assert.ok(body.includes('Cross-layer change')); + assert.ok(body.includes('packages/app')); + assert.ok(body.includes('packages/api')); + }); + + it('explains API model/route trigger', () => { + const body = buildTierComment(3, makeSignals({ touchesApiModels: true })); + assert.ok(body.includes('API routes or data models')); + }); + + it('explains agent branch bump to Tier 3', () => { + const signals = makeSignals({ + isAgentBranch: true, + agentBlocksTier2: true, + branchName: 'claude/big-feature', + prodLines: 80, + prodFiles: Array.from({ length: 5 }, (_, i) => makeFile(`packages/app/src/File${i}.tsx`)), + }); + const body = buildTierComment(3, signals); + assert.ok(body.includes('bumped to Tier 3')); + }); + + it('notes when agent branch is small enough for Tier 2', () => { + const signals = makeSignals({ + isAgentBranch: true, + agentBlocksTier2: false, + branchName: 'claude/tiny-fix', + }); + const body = buildTierComment(2, signals); + assert.ok(body.includes('small enough to qualify for Tier 2')); + }); + + it('shows test line count in stats when non-zero', () => { + const body = buildTierComment(2, makeSignals({ testLines: 200 })); + assert.ok(body.includes('200 in test files')); + }); + + it('omits test line note when testLines is 0', () => { + const body = buildTierComment(2, makeSignals({ testLines: 0 })); + assert.ok(!body.includes('test files')); + }); + + it('includes a catch-all trigger for standard Tier 3 PRs with no specific signals', () => { + const body = buildTierComment(3, makeSignals()); + assert.ok(body.includes('Standard feature/fix')); + }); + + it('includes bot-author trigger for Tier 1 bot PRs', () => { + const body = buildTierComment(1, makeSignals({ isBotAuthor: true, author: 'dependabot[bot]' })); + assert.ok(body.includes('Bot author')); + }); +}); diff --git a/.github/scripts/pr-triage-classify.js b/.github/scripts/pr-triage-classify.js new file mode 100644 index 00000000..5d501901 --- /dev/null +++ b/.github/scripts/pr-triage-classify.js @@ -0,0 +1,257 @@ +'use strict'; + +// ── File classification patterns ───────────────────────────────────────────── +const TIER4_PATTERNS = [ + /^packages\/api\/src\/middleware\/auth/, + /^packages\/api\/src\/routers\/api\/me\./, + /^packages\/api\/src\/routers\/api\/team\./, + /^packages\/api\/src\/routers\/external-api\//, + /^packages\/api\/src\/models\/(user|team|teamInvite)\./, + /^packages\/api\/src\/config\./, + /^packages\/api\/src\/tasks\//, + /^packages\/otel-collector\//, + /^docker\/otel-collector\//, + /^docker\/clickhouse\//, + /^docker\/hyperdx\//, + /^\.github\/workflows\/(main|release)\.yml$/, +]; + +const TIER1_PATTERNS = [ + /\.(md|txt|png|jpg|jpeg|gif|svg|ico)$/i, + /^yarn\.lock$/, + /^package-lock\.json$/, + /^\.yarnrc\.yml$/, + /^\.github\/images\//, + /^\.env\.example$/, + /^\.changeset\//, // version-bump config files; no functional code + /^\.github\/scripts\//, // GitHub Actions scripts; not application code + /^\.github\/workflows\//, // workflow files (main.yml/release.yml still caught by TIER4_PATTERNS) +]; + +const TEST_FILE_PATTERNS = [ + /\/__tests__\//, + /\.test\.[jt]sx?$/, + /\.spec\.[jt]sx?$/, + /^packages\/app\/tests\//, +]; + +// ── Thresholds (all line counts exclude test and trivial files) ─────────────── +const TIER2_MAX_LINES = 150; // max prod lines eligible for Tier 2 +const TIER4_ESCALATION_HUMAN = 1000; // Tier 3 → 4 for human branches +const TIER4_ESCALATION_AGENT = 400; // Tier 3 → 4 for agent branches (stricter) + +// Agent branches can reach Tier 2 only for very small, focused changes +const AGENT_TIER2_MAX_LINES = 50; +const AGENT_TIER2_MAX_PROD_FILES = 3; + +// ── Other constants ────────────────────────────────────────────────────────── +const BOT_AUTHORS = ['dependabot', 'dependabot[bot]']; +const AGENT_BRANCH_PREFIXES = ['claude/', 'agent/', 'ai/']; + +const TIER_LABELS = { + 1: { name: 'review/tier-1', color: '0E8A16', description: 'Trivial — auto-merge candidate once CI passes' }, + 2: { name: 'review/tier-2', color: '1D76DB', description: 'Low risk — AI review + quick human skim' }, + 3: { name: 'review/tier-3', color: 'E4E669', description: 'Standard — full human review required' }, + 4: { name: 'review/tier-4', color: 'B60205', description: 'Critical — deep review + domain expert sign-off' }, +}; + +const TIER_INFO = { + 1: { + emoji: '🟢', + headline: 'Tier 1 — Trivial', + detail: 'Docs, images, lock files, or a dependency bump. No functional code changes detected.', + process: 'Auto-merge once CI passes. No human review required.', + sla: 'Resolves automatically.', + }, + 2: { + emoji: '🔵', + headline: 'Tier 2 — Low Risk', + detail: 'Small, isolated change with no API route or data model modifications.', + process: 'AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.', + sla: 'Resolve within 4 business hours.', + }, + 3: { + emoji: '🟡', + headline: 'Tier 3 — Standard', + detail: 'Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.', + process: 'Full human review — logic, architecture, edge cases.', + sla: 'First-pass feedback within 1 business day.', + }, + 4: { + emoji: '🔴', + headline: 'Tier 4 — Critical', + detail: 'Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.', + process: 'Deep review from a domain expert. Synchronous walkthrough may be required.', + sla: 'Schedule synchronous review within 2 business days.', + }, +}; + +// ── File classification helpers ────────────────────────────────────────────── +const isTestFile = f => TEST_FILE_PATTERNS.some(p => p.test(f)); +const isTrivialFile = f => TIER1_PATTERNS.some(p => p.test(f)); +const isCriticalFile = f => TIER4_PATTERNS.some(p => p.test(f)); + +// ── Signal computation ─────────────────────────────────────────────────────── +// Returns a flat object of all facts needed for tier determination and comment +// generation. All derived from PR metadata + file list — no GitHub API calls. +// +// @param {object} pr - GitHub PR object: { user: { login }, head: { ref } } +// @param {Array} filesRes - GitHub files array: [{ filename, additions, deletions }] +function computeSignals(pr, filesRes) { + const author = pr.user.login; + const branchName = pr.head.ref; + + const testFiles = filesRes.filter(f => isTestFile(f.filename)); + const prodFiles = filesRes.filter(f => !isTestFile(f.filename) && !isTrivialFile(f.filename)); + const criticalFiles = filesRes.filter(f => isCriticalFile(f.filename) && !isTestFile(f.filename)); + + const prodLines = prodFiles.reduce((sum, f) => sum + f.additions + f.deletions, 0); + const testLines = testFiles.reduce((sum, f) => sum + f.additions + f.deletions, 0); + + const isAgentBranch = AGENT_BRANCH_PREFIXES.some(p => branchName.startsWith(p)); + const isBotAuthor = BOT_AUTHORS.includes(author); + const allFilesTrivial = filesRes.length > 0 && filesRes.every(f => isTrivialFile(f.filename)); + + // Blocks Tier 2 — API models and routes carry implicit cross-cutting risk + const touchesApiModels = prodFiles.some(f => + f.filename.startsWith('packages/api/src/models/') || + f.filename.startsWith('packages/api/src/routers/') + ); + + // Cross-layer: production changes spanning multiple monorepo packages + const touchesFrontend = prodFiles.some(f => f.filename.startsWith('packages/app/')); + const touchesBackend = prodFiles.some(f => f.filename.startsWith('packages/api/')); + const touchesSharedUtils = prodFiles.some(f => f.filename.startsWith('packages/common-utils/')); + const isCrossLayer = [touchesFrontend, touchesBackend, touchesSharedUtils].filter(Boolean).length >= 2; + + // Agent branches can reach Tier 2 only when the change is very small and focused + const agentBlocksTier2 = isAgentBranch && + (prodLines >= AGENT_TIER2_MAX_LINES || prodFiles.length > AGENT_TIER2_MAX_PROD_FILES); + + return { + author, branchName, + prodFiles, prodLines, testLines, criticalFiles, + isAgentBranch, isBotAuthor, allFilesTrivial, + touchesApiModels, touchesFrontend, touchesBackend, touchesSharedUtils, + isCrossLayer, agentBlocksTier2, + }; +} + +// ── Tier determination ─────────────────────────────────────────────────────── +// @param {object} signals - output of computeSignals() +// @returns {number} tier - 1 | 2 | 3 | 4 +function determineTier(signals) { + const { + criticalFiles, isBotAuthor, allFilesTrivial, + prodLines, touchesApiModels, isCrossLayer, agentBlocksTier2, isAgentBranch, + } = signals; + + // Tier 4: touches critical infrastructure (auth, config, pipeline, CI/CD) + if (criticalFiles.length > 0) return 4; + + // Tier 1: bot-authored or only docs/images/lock files changed + if (isBotAuthor || allFilesTrivial) return 1; + + // Tier 2: small, isolated, single-layer change + // Agent branches qualify when the change is very small and focused + // (agentBlocksTier2 is false when under AGENT_TIER2_MAX_LINES / MAX_PROD_FILES) + const qualifiesForTier2 = + prodLines < TIER2_MAX_LINES && + !touchesApiModels && + !isCrossLayer && + !agentBlocksTier2; + if (qualifiesForTier2) return 2; + + // Tier 3: everything else — escalate very large diffs to Tier 4 + const sizeThreshold = isAgentBranch ? TIER4_ESCALATION_AGENT : TIER4_ESCALATION_HUMAN; + return prodLines > sizeThreshold ? 4 : 3; +} + +// ── Comment generation ─────────────────────────────────────────────────────── +// @param {number} tier - 1 | 2 | 3 | 4 +// @param {object} signals - output of computeSignals() +// @returns {string} - Markdown comment body +function buildTierComment(tier, signals) { + const { + author, branchName, + prodFiles, prodLines, testLines, criticalFiles, + isAgentBranch, isBotAuthor, allFilesTrivial, + touchesApiModels, touchesFrontend, touchesBackend, touchesSharedUtils, + isCrossLayer, agentBlocksTier2, + } = signals; + + const info = TIER_INFO[tier]; + const sizeThreshold = isAgentBranch ? TIER4_ESCALATION_AGENT : TIER4_ESCALATION_HUMAN; + + // Primary triggers — the specific reasons this tier was assigned + const triggers = []; + if (criticalFiles.length > 0) { + triggers.push(`**Critical-path files** (${criticalFiles.length}):\n${criticalFiles.map(f => ` - \`${f.filename}\``).join('\n')}`); + } + if (tier === 4 && prodLines > sizeThreshold && criticalFiles.length === 0) { + triggers.push(`**Large diff**: ${prodLines} production lines changed (threshold: ${sizeThreshold})`); + } + if (isBotAuthor) triggers.push(`**Bot author**: \`${author}\``); + if (allFilesTrivial && !isBotAuthor) triggers.push('**All files are docs / images / lock files**'); + if (isCrossLayer) { + const layers = [ + touchesFrontend && 'frontend (`packages/app`)', + touchesBackend && 'backend (`packages/api`)', + touchesSharedUtils && 'shared utils (`packages/common-utils`)', + ].filter(Boolean); + triggers.push(`**Cross-layer change**: touches ${layers.join(' + ')}`); + } + if (touchesApiModels && criticalFiles.length === 0) { + triggers.push('**Touches API routes or data models** — hidden complexity risk'); + } + if (isAgentBranch && agentBlocksTier2 && tier <= 3) { + triggers.push(`**Agent-generated branch** (\`${branchName}\`) with ${prodLines} prod lines across ${prodFiles.length} files — bumped to Tier 3 for mandatory human review`); + } + if (triggers.length === 0) { + triggers.push('**Standard feature/fix** — introduces new logic or modifies core functionality'); + } + + // Informational context — didn't drive the tier on their own + const contextSignals = []; + if (isAgentBranch && !agentBlocksTier2 && tier === 2) { + contextSignals.push(`agent branch (\`${branchName}\`) — change small enough to qualify for Tier 2`); + } else if (isAgentBranch && tier === 4) { + contextSignals.push(`agent branch (\`${branchName}\`)`); + } + + const triggerSection = `\n**Why this tier:**\n${triggers.map(t => `- ${t}`).join('\n')}`; + const contextSection = contextSignals.length > 0 + ? `\n**Additional context:** ${contextSignals.join(', ')}` + : ''; + + return [ + '', + `## ${info.emoji} ${info.headline}`, + '', + info.detail, + triggerSection, + contextSection, + '', + `**Review process**: ${info.process}`, + `**SLA**: ${info.sla}`, + '', + '
Stats', + '', + `- Production files changed: ${prodFiles.length}`, + `- Production lines changed: ${prodLines}${testLines > 0 ? ` (+ ${testLines} in test files, excluded from tier calculation)` : ''}`, + `- Branch: \`${branchName}\``, + `- Author: ${author}`, + '', + '
', + '', + `> To override this classification, remove the \`${TIER_LABELS[tier].name}\` label and apply a different \`review/tier-*\` label. Manual overrides are preserved on subsequent pushes.`, + ].join('\n'); +} + +module.exports = { + // Constants needed by the orchestration script + TIER_LABELS, TIER_INFO, + // Pure functions + isTestFile, isTrivialFile, isCriticalFile, + computeSignals, determineTier, buildTierComment, +}; diff --git a/.github/scripts/pr-triage.js b/.github/scripts/pr-triage.js new file mode 100644 index 00000000..ac570a5a --- /dev/null +++ b/.github/scripts/pr-triage.js @@ -0,0 +1,123 @@ +'use strict'; + +// Entry point for actions/github-script@v7 via script-path. +// Pure classification logic lives in pr-triage-classify.js so it can be +// unit-tested without GitHub API machinery. + +const { + TIER_LABELS, + computeSignals, determineTier, buildTierComment, +} = require('./pr-triage-classify'); + +module.exports = async ({ github, context }) => { + const owner = context.repo.owner; + const repo = context.repo.repo; + + // ── Determine which PRs to process ────────────────────────────────────── + let prNumbers; + if (context.eventName === 'workflow_dispatch') { + // Use context.payload.inputs to avoid script-injection via template interpolation + const input = (context.payload.inputs?.pr_number ?? '').trim(); + if (input !== '') { + const num = Number(input); + if (!Number.isInteger(num) || num <= 0) { + throw new Error(`Invalid PR number: "${input}"`); + } + prNumbers = [num]; + } else { + const openPRs = await github.paginate( + github.rest.pulls.list, + { owner, repo, state: 'open', per_page: 100 } + ); + prNumbers = openPRs.map(pr => pr.number); + console.log(`Bulk triage: found ${prNumbers.length} open PRs`); + } + } else { + prNumbers = [context.payload.pull_request.number]; + } + + // ── Ensure tier labels exist (once, before the loop) ──────────────────── + const repoLabels = await github.paginate( + github.rest.issues.listLabelsForRepo, + { owner, repo, per_page: 100 } + ); + const repoLabelNames = new Set(repoLabels.map(l => l.name)); + for (const label of Object.values(TIER_LABELS)) { + if (!repoLabelNames.has(label.name)) { + await github.rest.issues.createLabel({ owner, repo, ...label }); + repoLabelNames.add(label.name); + } + } + + // ── Classify a single PR ───────────────────────────────────────────────── + async function classifyPR(prNumber) { + const filesRes = await github.paginate( + github.rest.pulls.listFiles, + { owner, repo, pull_number: prNumber, per_page: 100 } + ); + const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); + const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ owner, repo, issue_number: prNumber }); + const currentLabelNames = new Set(currentLabels.map(l => l.name)); + + // Skip drafts (bulk mode; PR events already filter these via the job condition) + if (pr.draft) { + console.log(`PR #${prNumber}: skipping draft`); + return; + } + + // Respect manual tier overrides — don't overwrite labels applied by humans + const existingTierLabel = currentLabels.find(l => l.name.startsWith('review/tier-')); + if (existingTierLabel) { + const events = await github.paginate( + github.rest.issues.listEvents, + { owner, repo, issue_number: prNumber, per_page: 100 } + ); + const lastLabelEvent = events + .filter(e => e.event === 'labeled' && e.label?.name === existingTierLabel.name) + .pop(); + if (lastLabelEvent && lastLabelEvent.actor.type !== 'Bot') { + console.log(`PR #${prNumber}: tier manually set to ${existingTierLabel.name} by ${lastLabelEvent.actor.login} — skipping`); + return; + } + } + + const signals = computeSignals(pr, filesRes); + const tier = determineTier(signals); + const body = buildTierComment(tier, signals); + + // Apply the tier label (remove any stale tier label first) + for (const label of currentLabels) { + if (label.name.startsWith('review/tier-') && label.name !== TIER_LABELS[tier].name) { + await github.rest.issues.removeLabel({ owner, repo, issue_number: prNumber, name: label.name }); + } + } + if (!currentLabelNames.has(TIER_LABELS[tier].name)) { + await github.rest.issues.addLabels({ owner, repo, issue_number: prNumber, labels: [TIER_LABELS[tier].name] }); + } + + // Post or update the triage comment + const comments = await github.paginate( + github.rest.issues.listComments, + { owner, repo, issue_number: prNumber, per_page: 100 } + ); + const existingComment = comments.find( + c => c.user.login === 'github-actions[bot]' && c.body.includes('') + ); + if (existingComment) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existingComment.id, body }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body }); + } + + console.log(`PR #${prNumber}: Tier ${tier} (${signals.prodLines} prod lines, ${signals.prodFiles.length} prod files, ${signals.testLines} test lines)`); + } + + // ── Process all target PRs ─────────────────────────────────────────────── + for (const prNumber of prNumbers) { + try { + await classifyPR(prNumber); + } catch (err) { + console.error(`PR #${prNumber}: classification failed — ${err.message}`); + } + } +}; diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 0b885c42..1ed6dd72 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -2,6 +2,7 @@ name: PR Triage on: pull_request: + branches: [main] types: [opened, synchronize, reopened, ready_for_review] workflow_dispatch: inputs: @@ -16,278 +17,39 @@ permissions: pull-requests: write issues: write +concurrency: + group: + ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id + }} + cancel-in-progress: true + jobs: + test: + name: Test triage logic + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + - run: node --test .github/scripts/__tests__/pr-triage-classify.test.js + classify: name: Classify PR risk tier + needs: test runs-on: ubuntu-24.04 + timeout-minutes: 8 # For pull_request events skip drafts; workflow_dispatch always runs if: ${{ github.event_name == 'workflow_dispatch' || !github.event.pull_request.draft }} steps: + - uses: actions/checkout@v4 - name: Classify and label PR(s) uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const owner = context.repo.owner; - const repo = context.repo.repo; - - // ── Determine which PRs to process ────────────────────────────── - let prNumbers; - if (context.eventName === 'workflow_dispatch') { - // Use context.payload.inputs to avoid script-injection via template interpolation - const input = (context.payload.inputs?.pr_number ?? '').trim(); - if (input && input !== '') { - prNumbers = [Number(input)]; - } else { - const openPRs = await github.paginate( - github.rest.pulls.list, - { owner, repo, state: 'open', per_page: 100 } - ); - prNumbers = openPRs.map(pr => pr.number); - console.log(`Bulk triage: found ${prNumbers.length} open PRs`); - } - } else { - prNumbers = [context.payload.pull_request.number]; - } - - // ── Shared constants ───────────────────────────────────────────── - const TIER4_PATTERNS = [ - /^packages\/api\/src\/middleware\/auth/, - /^packages\/api\/src\/routers\/api\/me\./, - /^packages\/api\/src\/routers\/api\/team\./, - /^packages\/api\/src\/routers\/external-api\//, - /^packages\/api\/src\/models\/(user|team|teamInvite)\./, - /^packages\/api\/src\/config\./, - /^packages\/api\/src\/tasks\//, - /^packages\/otel-collector\//, - /^docker\/otel-collector\//, - /^docker\/clickhouse\//, - /^\.github\/workflows\//, - ]; - - const TIER1_PATTERNS = [ - /\.(md|txt|png|jpg|jpeg|gif|svg|ico)$/i, - /^yarn\.lock$/, - /^package-lock\.json$/, - /^\.yarnrc\.yml$/, - /^\.github\/images\//, - /^\.env\.example$/, - ]; - - const BOT_AUTHORS = ['dependabot', 'dependabot[bot]']; - const AGENT_BRANCH_PREFIXES = ['claude/', 'agent/', 'ai/']; - - const TIER_LABELS = { - 1: { name: 'review/tier-1', color: '0E8A16', description: 'Trivial — auto-merge candidate once CI passes' }, - 2: { name: 'review/tier-2', color: '1D76DB', description: 'Low risk — AI review + quick human skim' }, - 3: { name: 'review/tier-3', color: 'E4E669', description: 'Standard — full human review required' }, - 4: { name: 'review/tier-4', color: 'B60205', description: 'Critical — deep review + domain expert sign-off' }, - }; - - const TIER_INFO = { - 1: { - emoji: '🟢', - headline: 'Tier 1 — Trivial', - detail: 'Docs, images, lock files, or a dependency bump. No functional code changes detected.', - process: 'Auto-merge once CI passes. No human review required.', - sla: 'Resolves automatically.', - }, - 2: { - emoji: '🔵', - headline: 'Tier 2 — Low Risk', - detail: 'Small, isolated change with no API route or data model modifications.', - process: 'AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.', - sla: 'Resolve within 4 business hours.', - }, - 3: { - emoji: '🟡', - headline: 'Tier 3 — Standard', - detail: 'Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.', - process: 'Full human review — logic, architecture, edge cases.', - sla: 'First-pass feedback within 1 business day.', - }, - 4: { - emoji: '🔴', - headline: 'Tier 4 — Critical', - detail: 'Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.', - process: 'Deep review from a domain expert. Synchronous walkthrough may be required.', - sla: 'Schedule synchronous review within 2 business days.', - }, - }; - - // ── Ensure tier labels exist (once, before the loop) ───────────── - const repoLabels = await github.paginate( - github.rest.issues.listLabelsForRepo, - { owner, repo, per_page: 100 } - ); - const repoLabelNames = new Set(repoLabels.map(l => l.name)); - for (const label of Object.values(TIER_LABELS)) { - if (!repoLabelNames.has(label.name)) { - await github.rest.issues.createLabel({ owner, repo, ...label }); - repoLabelNames.add(label.name); - } - } - - // ── Classify a single PR ───────────────────────────────────────── - async function classifyPR(prNumber) { - // Fetch changed files - const filesRes = await github.paginate( - github.rest.pulls.listFiles, - { owner, repo, pull_number: prNumber, per_page: 100 } - ); - const files = filesRes.map(f => f.filename); - const TEST_FILE_PATTERNS = [ - /\/__tests__\//, - /\.test\.[jt]sx?$/, - /\.spec\.[jt]sx?$/, - /^packages\/app\/tests\//, - ]; - const isTestFile = f => TEST_FILE_PATTERNS.some(p => p.test(f)); - const nonTestFiles = filesRes.filter(f => !isTestFile(f.filename)); - const testLines = filesRes.filter(f => isTestFile(f.filename)).reduce((sum, f) => sum + f.additions + f.deletions, 0); - const linesChanged = nonTestFiles.reduce((sum, f) => sum + f.additions + f.deletions, 0); - - // Fetch PR metadata - const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); - const author = pr.user.login; - const branchName = pr.head.ref; - - // Skip drafts when running in bulk mode - if (pr.draft) { - console.log(`Skipping PR #${prNumber}: draft`); - return; - } - - // Check for manual tier override — if a human last applied the label, respect it - const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ owner, repo, issue_number: prNumber }); - const existingTierLabel = currentLabels.find(l => l.name.startsWith('review/tier-')); - if (existingTierLabel) { - const events = await github.paginate( - github.rest.issues.listEvents, - { owner, repo, issue_number: prNumber, per_page: 100 } - ); - const lastLabelEvent = events - .filter(e => e.event === 'labeled' && e.label?.name === existingTierLabel.name) - .pop(); - if (lastLabelEvent && lastLabelEvent.actor.type !== 'Bot') { - console.log(`PR #${prNumber}: tier manually set to ${existingTierLabel.name} by ${lastLabelEvent.actor.login} — skipping`); - return; - } - } - - // Classify - const isTier4 = files.some(f => TIER4_PATTERNS.some(p => p.test(f))); - const isTrivialAuthor = BOT_AUTHORS.includes(author); - const allFilesTrivial = files.length > 0 && files.every(f => TIER1_PATTERNS.some(p => p.test(f))); - const isTier1 = isTrivialAuthor || allFilesTrivial; - const isAgentBranch = AGENT_BRANCH_PREFIXES.some(p => branchName.startsWith(p)); - const touchesApiModels = files.some(f => - f.startsWith('packages/api/src/models/') || f.startsWith('packages/api/src/routers/') - ); - const isSmallDiff = linesChanged < 100; - // Agent branches are bumped to Tier 3 regardless of size to ensure human review - const isTier2 = !isTier4 && !isTier1 && isSmallDiff && !touchesApiModels && !isAgentBranch; - - let tier; - if (isTier4) tier = 4; - else if (isTier1) tier = 1; - else if (isTier2) tier = 2; - else tier = 3; - - // Escalate very large non-critical PRs to Tier 4. Agent branches use a lower - // threshold (400 lines) since they warrant deeper scrutiny. Human-authored PRs - // are only escalated for truly massive diffs (1000+ lines); Tier 3 already - // requires full human review, so smaller large PRs don't need the extra urgency. - const sizeThreshold = isAgentBranch ? 400 : 1000; - if (tier === 3 && linesChanged > sizeThreshold) tier = 4; - - // Apply label - for (const existing of currentLabels) { - if (existing.name.startsWith('review/tier-') && existing.name !== TIER_LABELS[tier].name) { - await github.rest.issues.removeLabel({ owner, repo, issue_number: prNumber, name: existing.name }); - } - } - if (!currentLabels.find(l => l.name === TIER_LABELS[tier].name)) { - await github.rest.issues.addLabels({ owner, repo, issue_number: prNumber, labels: [TIER_LABELS[tier].name] }); - } - - // Build comment body - const info = TIER_INFO[tier]; - - // Primary triggers — what actually determined (or escalated) the tier - const triggers = []; - const criticalFiles = files.filter(f => TIER4_PATTERNS.some(p => p.test(f))); - if (criticalFiles.length > 0) { - triggers.push(`**Critical-path files** (${criticalFiles.length}):\n${criticalFiles.map(f => ` - \`${f}\``).join('\n')}`); - } - if (tier === 4 && linesChanged > sizeThreshold && criticalFiles.length === 0) { - triggers.push(`**Large diff**: ${linesChanged} lines changed (threshold: ${sizeThreshold})`); - } - if (isTrivialAuthor) triggers.push(`**Bot author**: \`${author}\``); - if (allFilesTrivial && !isTrivialAuthor) triggers.push('**All files are docs / images / lock files**'); - // Agent branch prevents Tier 2 — it's a cause for Tier 3, not just context - if (isAgentBranch && tier <= 3) triggers.push(`**Agent-generated branch** (\`${branchName}\`) — bumped to Tier 3 for mandatory human review`); - // Catch-all for Tier 3 PRs that don't match any specific trigger above - if (triggers.length === 0 && tier === 3) triggers.push('**Standard feature/fix** — introduces new logic or modifies core functionality'); - - // Informational signals — didn't drive the tier by themselves - const contextSignals = []; - if (isAgentBranch && tier === 4) contextSignals.push(`agent branch (\`${branchName}\`)`); - if (touchesApiModels && criticalFiles.length === 0) contextSignals.push('touches API routes or data models'); - if (linesChanged > sizeThreshold && criticalFiles.length > 0) contextSignals.push(`large diff (${linesChanged} lines)`); - - const triggerSection = triggers.length > 0 - ? `\n**Why this tier:**\n${triggers.map(t => `- ${t}`).join('\n')}` - : ''; - const contextSection = contextSignals.length > 0 - ? `\n**Additional context:** ${contextSignals.join(', ')}` - : ''; - - const body = [ - '', - `## ${info.emoji} ${info.headline}`, - '', - info.detail, - triggerSection, - contextSection, - '', - `**Review process**: ${info.process}`, - `**SLA**: ${info.sla}`, - '', - `
Stats`, - '', - `- Files changed: ${files.length}`, - `- Lines changed: ${linesChanged}${testLines > 0 ? ` (+ ${testLines} in test files, excluded from tier calculation)` : ''}`, - `- Branch: \`${branchName}\``, - `- Author: ${author}`, - '', - '
', - '', - `> To override this classification, remove the \`${TIER_LABELS[tier].name}\` label and apply a different \`review/tier-*\` label. Manual overrides are preserved on subsequent pushes.`, - ].join('\n'); - - // Post or update the single triage comment - const comments = await github.paginate( - github.rest.issues.listComments, - { owner, repo, issue_number: prNumber, per_page: 100 } - ); - const existing = comments.find(c => c.user.login === 'github-actions[bot]' && c.body.includes('')); - if (existing) { - await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); - } else { - await github.rest.issues.createComment({ owner, repo, issue_number: prNumber, body }); - } - - console.log(`PR #${prNumber}: Tier ${tier} (${linesChanged} lines, ${files.length} files)`); - } - - // ── Process all target PRs ─────────────────────────────────────── - for (const prNumber of prNumbers) { - try { - await classifyPR(prNumber); - } catch (err) { - console.error(`PR #${prNumber}: classification failed — ${err.message}`); - } - } + const path = require('path'); + const run = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pr-triage.js')); + await run({ github, context }); diff --git a/Makefile b/Makefile index 258fb04d..413696d3 100644 --- a/Makefile +++ b/Makefile @@ -187,6 +187,10 @@ dev-unit: ci-unit: npx nx run-many -t ci:unit +.PHONY: ci-triage +ci-triage: + node --test .github/scripts/__tests__/pr-triage-classify.test.js + # --------------------------------------------------------------------------- # E2E tests — port isolation is handled by scripts/test-e2e.sh # --------------------------------------------------------------------------- diff --git a/knip.json b/knip.json index 9bb451db..1337d9a0 100644 --- a/knip.json +++ b/knip.json @@ -27,7 +27,7 @@ "project": ["src/**/*.ts"] } }, - "ignore": ["scripts/dev-portal/**"], + "ignore": ["scripts/dev-portal/**", ".github/scripts/**"], "ignoreBinaries": ["make", "migrate", "playwright"], "ignoreDependencies": [ "@dotenvx/dotenvx",