From e4682229324b873f584a55c5c543d45964929631 Mon Sep 17 00:00:00 2001 From: "Dina Berry (She/her)" Date: Fri, 3 Apr 2026 06:29:31 -0700 Subject: [PATCH] feat(ci): automated PR readiness checks (#750) Adds a GitHub Action that posts a readiness checklist comment on every PR, giving contributors immediate feedback on what needs fixing before review. 7 checks: single commit, not draft, branch up to date, Copilot review, changeset present, no merge conflicts, CI passing. Informational only. Includes extracted script (pr-readiness.mjs), tests, and CONTRIBUTING.md update. Closes #750 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/squad-pr-readiness.yml | 46 ++ CONTRIBUTING.md | 16 + scripts/pr-readiness.mjs | 534 ++++++++++++++++ test/pr-readiness.test.ts | 781 +++++++++++++++++++++++ 4 files changed, 1377 insertions(+) create mode 100644 .github/workflows/squad-pr-readiness.yml create mode 100644 scripts/pr-readiness.mjs create mode 100644 test/pr-readiness.test.ts diff --git a/.github/workflows/squad-pr-readiness.yml b/.github/workflows/squad-pr-readiness.yml new file mode 100644 index 000000000..64b832282 --- /dev/null +++ b/.github/workflows/squad-pr-readiness.yml @@ -0,0 +1,46 @@ +name: Squad PR Readiness + +on: + pull_request_target: + branches: [dev, main, insider] + types: [opened, synchronize, reopened, edited, ready_for_review] + +# NOTE: Using pull_request_target so we get a write token even for fork PRs. +# This is safe because we never check out or execute PR code — all data +# comes from the GitHub API. +permissions: + contents: read + pull-requests: write + issues: write + statuses: read + checks: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + readiness: + runs-on: ubuntu-latest + timeout-minutes: 5 + # Skip if the PR is from a bot account (dependabot, renovate, etc.) + if: github.actor != 'dependabot[bot]' + steps: + - name: Checkout scripts + uses: actions/checkout@v4 + with: + sparse-checkout: scripts/pr-readiness.mjs + sparse-checkout-cone-mode: false + - name: Check PR readiness + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + PR_DRAFT: ${{ github.event.pull_request.draft }} + PR_LABELS: ${{ toJson(github.event.pull_request.labels) }} + REPO_OWNER: ${{ github.repository_owner }} + REPO_NAME: ${{ github.event.repository.name }} + RUN_NAME: ${{ github.workflow }} + run: node scripts/pr-readiness.mjs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e3d7ee08d..f8f792b54 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -156,6 +156,22 @@ The Co-authored-by trailer is **required** for all commits (added by Copilot CLI 5. Wait for CI checks to pass 6. Request review from the team (agents will respond via comments) +### PR Readiness Checklist + +An automated readiness check runs on every PR and posts a checklist comment. Address all items before requesting review: + +| Check | What it means | +|-------|---------------| +| **Single commit** | Squash your commits into one clean commit, or the repo will squash on merge | +| **Not in draft** | Mark your PR as "Ready for review" when it's done | +| **Branch up to date** | Rebase on latest `dev` (`git fetch upstream && git rebase upstream/dev`) | +| **Copilot review** | Wait for the Copilot reviewer bot to post its review | +| **Changeset present** | Run `npx changeset add` if you changed files in `packages/squad-sdk/src/` or `packages/squad-cli/src/` | +| **No merge conflicts** | Resolve any conflicts with the target branch | +| **CI passing** | All CI checks (build, test, lint) must be green | + +The readiness check is **informational** — it helps you self-serve before a human reviewer looks at your PR. See `.github/PR_REQUIREMENTS.md` for the full requirements spec. + ## Code Style & Conventions Squad follows strict TypeScript conventions: diff --git a/scripts/pr-readiness.mjs b/scripts/pr-readiness.mjs new file mode 100644 index 000000000..0f760bf5f --- /dev/null +++ b/scripts/pr-readiness.mjs @@ -0,0 +1,534 @@ +/** + * PR Readiness checks — pure check functions + orchestration. + * + * Each check is a pure function that returns { pass, detail }. + * The `run()` function is the orchestrator: it reads PR context from + * environment variables, calls the GitHub API via `fetchFn`, runs all + * checks, builds the checklist markdown, and upserts the PR comment. + * + * The workflow invokes this script via `node scripts/pr-readiness.mjs`. + * + * Issue: #750 + */ + +import { fileURLToPath } from 'node:url'; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +export const COMMENT_MARKER = ''; + +/** Check-run names belonging to this workflow (filtered from CI checks). */ +export const SELF_CHECK_NAMES = ['readiness', 'PR Readiness Check']; + +/** Regex for source files that require a changeset. */ +export const SOURCE_PATTERN = /^packages\/squad-(sdk|cli)\/src\//; + +// --------------------------------------------------------------------------- +// Pure check functions +// --------------------------------------------------------------------------- + +/** + * Check 1: Single commit. + * @param {number} commitCount + * @returns {{ pass: boolean, detail: string }} + */ +export function checkCommitCount(commitCount) { + return { + pass: commitCount === 1, + detail: commitCount === 1 + ? '1 commit — clean history' + : `${commitCount} commits — consider squashing before review`, + }; +} + +/** + * Check 2: Not in draft. + * @param {boolean} isDraft + * @returns {{ pass: boolean, detail: string }} + */ +export function checkDraftStatus(isDraft) { + return { + pass: !isDraft, + detail: isDraft + ? 'PR is still in draft — mark as ready for review when done' + : 'Ready for review', + }; +} + +/** + * Check 3: Branch freshness (up to date with base). + * @param {number|null} behindBy — null when comparison failed + * @param {string} baseRef + * @returns {{ pass: boolean, detail: string }} + */ +export function checkBranchFreshness(behindBy, baseRef) { + if (behindBy === null) { + return { pass: false, detail: 'Could not determine — check manually' }; + } + const upToDate = behindBy === 0; + return { + pass: upToDate, + detail: upToDate + ? `Up to date with ${baseRef}` + : `${baseRef} is ${behindBy} commit(s) ahead — rebase recommended`, + }; +} + +/** + * Check 4: Copilot review. + * @param {Array<{ user?: { login?: string }, state?: string, submitted_at?: string, created_at?: string }>} reviews + * @returns {{ pass: boolean, detail: string }} + */ +export function checkCopilotReview(reviews) { + const copilotReviews = (reviews || []).filter( + (r) => r.user && r.user.login === 'copilot-pull-request-reviewer', + ); + const latest = copilotReviews.length + ? copilotReviews.reduce((a, b) => { + const aTime = new Date(a.submitted_at || a.created_at || 0).getTime(); + const bTime = new Date(b.submitted_at || b.created_at || 0).getTime(); + return bTime > aTime ? b : a; + }) + : null; + const approved = latest && latest.state === 'APPROVED'; + return { + pass: !!approved, + detail: approved + ? 'Copilot reviewed and approved' + : latest + ? `Copilot reviewed (state: ${latest.state})` + : 'No Copilot review yet — it may still be processing', + }; +} + +/** + * Check 5: Changeset present. + * @param {Array<{ filename: string }>} files — files changed in the PR + * @param {Array<{ name: string }>} labels — PR labels + * @returns {{ pass: boolean, detail: string }} + */ +export function checkChangeset(files, labels) { + const hasChangeset = files.some( + (f) => f.filename.startsWith('.changeset/') && f.filename.endsWith('.md'), + ); + const hasChangelogEdit = files.some((f) => f.filename === 'CHANGELOG.md'); + const hasChangelogArtifact = hasChangeset || hasChangelogEdit; + const touchesSource = files.some((f) => SOURCE_PATTERN.test(f.filename)); + const hasSkipLabel = (labels || []).some((l) => l.name === 'skip-changelog'); + + let pass = hasChangelogArtifact || !touchesSource; + let detail = ''; + + if (hasChangeset) { + detail = 'Changeset file found'; + } else if (hasChangelogEdit) { + detail = 'CHANGELOG.md edit found'; + } else if (!touchesSource) { + detail = 'No source files changed — changeset not required'; + } else { + detail = + 'Missing `.changeset/*.md` or `CHANGELOG.md` edit — run `npx changeset add` (or add `skip-changelog` label)'; + } + + if (hasSkipLabel && !hasChangelogArtifact) { + pass = true; + detail = 'Changeset skipped via `skip-changelog` label'; + } + + return { pass, detail }; +} + +/** + * Check 6: No merge conflicts (mergeability). + * @param {boolean|null} mergeable — true/false/null + * @returns {{ pass: boolean, detail: string }} + */ +export function checkMergeability(mergeable) { + if (mergeable === true) { + return { pass: true, detail: 'No merge conflicts' }; + } + if (mergeable === false) { + return { pass: false, detail: 'Merge conflicts detected — resolve before review' }; + } + // null / unknown — don't penalize + return { pass: true, detail: 'Merge status unknown — GitHub is still computing' }; +} + +/** + * Check 7: Scope cleanliness — warn when PR includes `.squad/` or `docs/proposals/` files. + * Informational only (always passes); helps flag accidental scope creep. + * @param {Array<{ filename: string }>} files — files changed in the PR + * @returns {{ pass: boolean, detail: string }} + */ +export function checkScopeClean(files) { + const squadFiles = (files || []).filter((f) => f.filename.startsWith('.squad/')); + const proposalFiles = (files || []).filter((f) => f.filename.startsWith('docs/proposals/')); + const squadCount = squadFiles.length; + const proposalCount = proposalFiles.length; + + if (squadCount === 0 && proposalCount === 0) { + return { pass: true, detail: 'No .squad/ or docs/proposals/ files' }; + } + + const parts = []; + if (squadCount > 0) parts.push(`${squadCount} .squad/ file(s)`); + if (proposalCount > 0) parts.push(`${proposalCount} docs/proposals/ file(s)`); + return { + pass: true, + detail: `⚠️ PR includes ${parts.join(' and ')} — ensure these are intentional`, + }; +} + +/** + * Check 8: All Copilot review threads resolved. + * @param {Array<{ isResolved: boolean, comments: { nodes: Array<{ author: { login: string } }> } }>} threads + * @returns {{ pass: boolean, detail: string }} + */ +export function checkCopilotThreads(threads) { + const copilotThreads = (threads || []).filter( + (t) => + t.comments && + t.comments.nodes && + t.comments.nodes[0]?.author?.login === 'copilot-pull-request-reviewer', + ); + const unresolved = copilotThreads.filter((t) => !t.isResolved); + return { + pass: unresolved.length === 0, + detail: + unresolved.length === 0 + ? copilotThreads.length > 0 + ? `All ${copilotThreads.length} Copilot thread(s) resolved` + : 'No Copilot review threads' + : `${unresolved.length} unresolved Copilot thread(s) — fix and resolve before merging`, + }; +} + +/** + * Check 9: CI passing. + * @param {Array<{ name: string, conclusion: string|null, status: string }>} checkRuns + * @param {Array} statuses — combined status entries + * @returns {{ pass: boolean, detail: string }} + */ +export function checkCIStatus(checkRuns, statuses) { + const otherChecks = (checkRuns || []).filter( + (cr) => !SELF_CHECK_NAMES.includes(cr.name), + ); + const failedChecks = otherChecks.filter( + (cr) => cr.conclusion === 'failure' || cr.conclusion === 'cancelled', + ); + const pendingChecks = otherChecks.filter( + (cr) => cr.status === 'in_progress' || cr.status === 'queued', + ); + + if (failedChecks.length > 0) { + return { + pass: false, + detail: `${failedChecks.length} check(s) failing: ${failedChecks.map((c) => c.name).join(', ')}`, + }; + } + if (pendingChecks.length > 0) { + return { + pass: false, + detail: `${pendingChecks.length} check(s) still running`, + }; + } + if (otherChecks.length === 0 && (statuses || []).length === 0) { + return { pass: false, detail: 'No CI checks have run yet' }; + } + return { pass: true, detail: 'All checks passing' }; +} + +// --------------------------------------------------------------------------- +// Checklist markdown builder +// --------------------------------------------------------------------------- + +/** + * Build the PR readiness comment body. + * @param {Array<{ name: string, pass: boolean, detail: string }>} checks + * @param {string} owner + * @param {string} repo + * @param {string} baseRef + * @returns {string} + */ +export function buildChecklist(checks, owner, repo, baseRef) { + const allPass = checks.every((c) => c.pass); + const passCount = checks.filter((c) => c.pass).length; + + const status = allPass + ? '### ✅ PR is ready for review' + : `### ⚠️ ${checks.length - passCount} item(s) to address before review`; + + const rows = checks.map((c) => { + const icon = c.pass ? '✅' : '❌'; + return `| ${icon} | **${c.name}** | ${c.detail} |`; + }); + + return [ + COMMENT_MARKER, + '## 🛫 PR Readiness Check', + '', + status, + '', + '| Status | Check | Details |', + '|--------|-------|---------|', + ...rows, + '', + '---', + '*This check runs automatically on every push. Fix any ❌ items and push again.*', + `*See [CONTRIBUTING.md](https://github.com/${owner}/${repo}/blob/${baseRef}/CONTRIBUTING.md#pr-readiness-checklist) and [PR Requirements](https://github.com/${owner}/${repo}/blob/${baseRef}/.github/PR_REQUIREMENTS.md) for details.*`, + ].join('\n'); +} + +// --------------------------------------------------------------------------- +// API helpers +// --------------------------------------------------------------------------- + +/** + * Paginate a GitHub REST list endpoint. + * @param {typeof globalThis.fetch} fetchFn + * @param {string} url — initial URL (with per_page param) + * @param {Record} headers + * @returns {Promise} + */ +export async function paginate(fetchFn, url, headers) { + const items = []; + let nextUrl = url; + while (nextUrl) { + const res = await fetchFn(nextUrl, { headers }); + if (!res.ok) throw new Error(`GitHub API ${res.status}: ${nextUrl}`); + const data = await res.json(); + items.push(...(Array.isArray(data) ? data : data.check_runs || [])); + // Parse Link header for next page + const link = res.headers.get('link') || ''; + const match = link.match(/<([^>]+)>;\s*rel="next"/); + nextUrl = match ? match[1] : null; + } + return items; +} + +// --------------------------------------------------------------------------- +// Orchestration +// --------------------------------------------------------------------------- + +/** + * Full PR readiness orchestrator. Reads context from env vars, calls + * GitHub API, runs all checks, and upserts the readiness comment. + * + * Dependencies (`env` and `fetchFn`) are injectable for testing. + * + * @param {object} [opts] + * @param {Record} [opts.env] — defaults to process.env + * @param {typeof globalThis.fetch} [opts.fetchFn] — defaults to global fetch + * @returns {Promise<{ checks: Array<{name:string,pass:boolean,detail:string}>, action: string }>} + */ +export async function run({ env = process.env, fetchFn = globalThis.fetch } = {}) { + const token = env.GITHUB_TOKEN; + const prNumber = env.PR_NUMBER; + const prDraft = env.PR_DRAFT === 'true'; + const prHeadSha = env.PR_HEAD_SHA; + const prBaseRef = env.PR_BASE_REF; + const owner = env.REPO_OWNER; + const repo = env.REPO_NAME; + const runName = env.RUN_NAME || ''; + // Labels come as a JSON array string when set, or empty + const prLabelsRaw = env.PR_LABELS || '[]'; + + let prLabels = []; + try { + prLabels = JSON.parse(prLabelsRaw); + } catch { + prLabels = []; + } + + const apiHeaders = { + Authorization: `token ${token}`, + Accept: 'application/vnd.github+json', + 'User-Agent': 'squad-pr-readiness', + }; + + const apiBase = `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; + const checks = []; + + // 1. Commit count + const commits = await paginate( + fetchFn, + `${apiBase}/pulls/${prNumber}/commits?per_page=100`, + apiHeaders, + ); + checks.push({ name: 'Single commit', ...checkCommitCount(commits.length) }); + + // 2. Draft status + checks.push({ name: 'Not in draft', ...checkDraftStatus(prDraft) }); + + // 3. Branch freshness + let behindBy = null; + try { + const compRes = await fetchFn( + `${apiBase}/compare/${encodeURIComponent(prBaseRef)}...${prHeadSha}`, + { headers: apiHeaders }, + ); + if (compRes.ok) { + const comp = await compRes.json(); + behindBy = comp.behind_by; + } + } catch { + // leave behindBy as null + } + checks.push({ name: 'Branch up to date', ...checkBranchFreshness(behindBy, prBaseRef) }); + + // 4. Copilot review + const reviews = await paginate( + fetchFn, + `${apiBase}/pulls/${prNumber}/reviews?per_page=100`, + apiHeaders, + ); + checks.push({ name: 'Copilot review', ...checkCopilotReview(reviews) }); + + // 5. Changeset present + const files = await paginate( + fetchFn, + `${apiBase}/pulls/${prNumber}/files?per_page=100`, + apiHeaders, + ); + checks.push({ name: 'Changeset present', ...checkChangeset(files, prLabels) }); + + // 6. Scope cleanliness + checks.push({ name: 'Scope clean', ...checkScopeClean(files) }); + + // 7. Mergeability + let mergeable = null; + for (let attempt = 0; attempt < 2; attempt++) { + const prRes = await fetchFn(`${apiBase}/pulls/${prNumber}`, { + headers: apiHeaders, + }); + if (prRes.ok) { + const prData = await prRes.json(); + if (prData.mergeable === null && attempt === 0) { + await new Promise((r) => setTimeout(r, 3000)); + continue; + } + mergeable = prData.mergeable; + } + break; + } + checks.push({ name: 'No merge conflicts', ...checkMergeability(mergeable) }); + + // 8. Copilot review threads resolved (via GraphQL) + let reviewThreadNodes = []; + try { + const graphqlBody = JSON.stringify({ + query: `query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { + isResolved + comments(first: 1) { + nodes { + author { login } + } + } + } + } + } + } + }`, + variables: { owner, repo, number: parseInt(prNumber, 10) }, + }); + const threadsRes = await fetchFn('https://api.github.com/graphql', { + method: 'POST', + headers: { ...apiHeaders, 'Content-Type': 'application/json' }, + body: graphqlBody, + }); + if (threadsRes.ok) { + const threadsData = await threadsRes.json(); + reviewThreadNodes = + threadsData.data?.repository?.pullRequest?.reviewThreads?.nodes || []; + } + } catch { + // leave empty — will show "No Copilot review threads" + } + checks.push({ + name: 'Copilot threads resolved', + ...checkCopilotThreads(reviewThreadNodes), + }); + + // 9. CI status + let checkRuns = []; + let statusEntries = []; + try { + checkRuns = await paginate( + fetchFn, + `${apiBase}/commits/${prHeadSha}/check-runs?per_page=100`, + apiHeaders, + ); + const statusRes = await fetchFn( + `${apiBase}/commits/${prHeadSha}/status`, + { headers: apiHeaders }, + ); + if (statusRes.ok) { + const statusData = await statusRes.json(); + statusEntries = statusData.statuses || []; + } + } catch { + // leave empty + } + checks.push({ name: 'CI passing', ...checkCIStatus(checkRuns, statusEntries) }); + + // ── Build checklist and upsert comment ── + const body = buildChecklist(checks, owner, repo, prBaseRef); + + // Find existing comment + const existingComments = await paginate( + fetchFn, + `${apiBase}/issues/${prNumber}/comments?per_page=100`, + apiHeaders, + ); + const existing = existingComments.find( + (c) => c.body && c.body.includes(COMMENT_MARKER), + ); + + let action; + if (existing) { + await fetchFn(`${apiBase}/issues/comments/${existing.id}`, { + method: 'PATCH', + headers: { ...apiHeaders, 'Content-Type': 'application/json' }, + body: JSON.stringify({ body }), + }); + action = 'updated'; + } else { + await fetchFn(`${apiBase}/issues/${prNumber}/comments`, { + method: 'POST', + headers: { ...apiHeaders, 'Content-Type': 'application/json' }, + body: JSON.stringify({ body }), + }); + action = 'created'; + } + + // Log summary + const passCount = checks.filter((c) => c.pass).length; + console.log(`PR Readiness: ${passCount}/${checks.length} checks passing`); + if (passCount < checks.length) { + const failing = checks.filter((c) => !c.pass).map((c) => c.name); + console.log(`Failing: ${failing.join(', ')}`); + } + + return { checks, action }; +} + +// --------------------------------------------------------------------------- +// CLI entry point +// --------------------------------------------------------------------------- + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + run() + .then((result) => { + console.log(`Comment ${result.action}.`); + }) + .catch((err) => { + console.error('PR readiness script failed:', err); + process.exitCode = 1; + }); +} diff --git a/test/pr-readiness.test.ts b/test/pr-readiness.test.ts new file mode 100644 index 000000000..15488fc93 --- /dev/null +++ b/test/pr-readiness.test.ts @@ -0,0 +1,781 @@ +/** + * Tests for PR readiness check functions and orchestration. + * Validates each pure check function independently, the checklist builder, + * and the run() orchestrator with mocked fetchFn. + * Issue: #750, PR: #752 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + checkCommitCount, + checkDraftStatus, + checkBranchFreshness, + checkCopilotReview, + checkChangeset, + checkScopeClean, + checkMergeability, + checkCopilotThreads, + checkCIStatus, + buildChecklist, + paginate, + run, + COMMENT_MARKER, + SELF_CHECK_NAMES, + SOURCE_PATTERN, +} from '../scripts/pr-readiness.mjs'; + +// --------------------------------------------------------------------------- +// checkCommitCount +// --------------------------------------------------------------------------- + +describe('checkCommitCount', () => { + it('passes with exactly 1 commit', () => { + const result = checkCommitCount(1); + expect(result.pass).toBe(true); + expect(result.detail).toContain('1 commit'); + }); + + it('fails with 0 commits', () => { + const result = checkCommitCount(0); + expect(result.pass).toBe(false); + expect(result.detail).toContain('0 commits'); + }); + + it('fails with multiple commits and suggests squashing', () => { + const result = checkCommitCount(5); + expect(result.pass).toBe(false); + expect(result.detail).toContain('5 commits'); + expect(result.detail).toContain('squashing'); + }); +}); + +// --------------------------------------------------------------------------- +// checkDraftStatus +// --------------------------------------------------------------------------- + +describe('checkDraftStatus', () => { + it('passes when PR is not a draft', () => { + const result = checkDraftStatus(false); + expect(result.pass).toBe(true); + expect(result.detail).toBe('Ready for review'); + }); + + it('fails when PR is a draft', () => { + const result = checkDraftStatus(true); + expect(result.pass).toBe(false); + expect(result.detail).toContain('draft'); + }); +}); + +// --------------------------------------------------------------------------- +// checkBranchFreshness +// --------------------------------------------------------------------------- + +describe('checkBranchFreshness', () => { + it('passes when branch is up to date (behindBy=0)', () => { + const result = checkBranchFreshness(0, 'dev'); + expect(result.pass).toBe(true); + expect(result.detail).toContain('Up to date with dev'); + }); + + it('fails when branch is behind', () => { + const result = checkBranchFreshness(3, 'main'); + expect(result.pass).toBe(false); + expect(result.detail).toContain('3 commit(s) ahead'); + expect(result.detail).toContain('main'); + }); + + it('fails gracefully when comparison failed (null)', () => { + const result = checkBranchFreshness(null, 'dev'); + expect(result.pass).toBe(false); + expect(result.detail).toContain('Could not determine'); + }); +}); + +// --------------------------------------------------------------------------- +// checkCopilotReview +// --------------------------------------------------------------------------- + +describe('checkCopilotReview', () => { + it('passes when copilot approved', () => { + const reviews = [ + { user: { login: 'copilot-pull-request-reviewer' }, state: 'APPROVED', submitted_at: '2025-01-01T00:00:00Z' }, + ]; + const result = checkCopilotReview(reviews); + expect(result.pass).toBe(true); + expect(result.detail).toContain('approved'); + }); + + it('fails when copilot review state is CHANGES_REQUESTED', () => { + const reviews = [ + { user: { login: 'copilot-pull-request-reviewer' }, state: 'CHANGES_REQUESTED', submitted_at: '2025-01-01T00:00:00Z' }, + ]; + const result = checkCopilotReview(reviews); + expect(result.pass).toBe(false); + expect(result.detail).toContain('CHANGES_REQUESTED'); + }); + + it('fails when no copilot review exists', () => { + const result = checkCopilotReview([]); + expect(result.pass).toBe(false); + expect(result.detail).toContain('No Copilot review yet'); + }); + + it('handles null/undefined reviews array', () => { + const result = checkCopilotReview(null); + expect(result.pass).toBe(false); + }); + + it('ignores non-copilot reviews', () => { + const reviews = [ + { user: { login: 'human-reviewer' }, state: 'APPROVED', submitted_at: '2025-01-01T00:00:00Z' }, + ]; + const result = checkCopilotReview(reviews); + expect(result.pass).toBe(false); + expect(result.detail).toContain('No Copilot review yet'); + }); + + it('uses the latest copilot review when multiple exist', () => { + const reviews = [ + { user: { login: 'copilot-pull-request-reviewer' }, state: 'CHANGES_REQUESTED', submitted_at: '2025-01-01T00:00:00Z' }, + { user: { login: 'copilot-pull-request-reviewer' }, state: 'APPROVED', submitted_at: '2025-01-02T00:00:00Z' }, + ]; + const result = checkCopilotReview(reviews); + expect(result.pass).toBe(true); + }); + + it('picks the latest review even when older one is approved', () => { + const reviews = [ + { user: { login: 'copilot-pull-request-reviewer' }, state: 'APPROVED', submitted_at: '2025-01-01T00:00:00Z' }, + { user: { login: 'copilot-pull-request-reviewer' }, state: 'CHANGES_REQUESTED', submitted_at: '2025-01-02T00:00:00Z' }, + ]; + const result = checkCopilotReview(reviews); + expect(result.pass).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// checkChangeset +// --------------------------------------------------------------------------- + +describe('checkChangeset', () => { + it('passes when changeset file exists', () => { + const files = [{ filename: '.changeset/abc.md' }, { filename: 'packages/squad-sdk/src/foo.ts' }]; + const result = checkChangeset(files, []); + expect(result.pass).toBe(true); + expect(result.detail).toContain('Changeset file found'); + }); + + it('passes when CHANGELOG.md is edited', () => { + const files = [{ filename: 'CHANGELOG.md' }, { filename: 'packages/squad-cli/src/bar.ts' }]; + const result = checkChangeset(files, []); + expect(result.pass).toBe(true); + expect(result.detail).toContain('CHANGELOG.md edit found'); + }); + + it('passes when no source files changed', () => { + const files = [{ filename: 'README.md' }, { filename: '.github/workflows/ci.yml' }]; + const result = checkChangeset(files, []); + expect(result.pass).toBe(true); + expect(result.detail).toContain('changeset not required'); + }); + + it('fails when source files changed but no changeset', () => { + const files = [{ filename: 'packages/squad-sdk/src/index.ts' }]; + const result = checkChangeset(files, []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('Missing'); + }); + + it('passes with skip-changelog label', () => { + const files = [{ filename: 'packages/squad-sdk/src/index.ts' }]; + const labels = [{ name: 'skip-changelog' }]; + const result = checkChangeset(files, labels); + expect(result.pass).toBe(true); + expect(result.detail).toContain('skip-changelog'); + }); + + it('prefers changeset detail over skip-changelog when both present', () => { + const files = [{ filename: '.changeset/abc.md' }, { filename: 'packages/squad-sdk/src/x.ts' }]; + const labels = [{ name: 'skip-changelog' }]; + const result = checkChangeset(files, labels); + expect(result.pass).toBe(true); + expect(result.detail).toContain('Changeset file found'); + }); + + it('handles null labels gracefully', () => { + const files = [{ filename: 'packages/squad-sdk/src/index.ts' }]; + const result = checkChangeset(files, null); + expect(result.pass).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// checkScopeClean +// --------------------------------------------------------------------------- + +describe('checkScopeClean', () => { + it('passes with no scope files', () => { + const files = [{ filename: 'packages/squad-sdk/src/index.ts' }, { filename: 'README.md' }]; + const result = checkScopeClean(files); + expect(result.pass).toBe(true); + expect(result.detail).toContain('No .squad/ or docs/proposals/ files'); + }); + + it('warns when .squad/ files are present', () => { + const files = [{ filename: '.squad/team.md' }, { filename: 'packages/squad-sdk/src/index.ts' }]; + const result = checkScopeClean(files); + expect(result.pass).toBe(true); + expect(result.detail).toContain('1 .squad/ file(s)'); + expect(result.detail).toContain('ensure these are intentional'); + }); + + it('warns when docs/proposals/ files are present', () => { + const files = [{ filename: 'docs/proposals/my-proposal.md' }]; + const result = checkScopeClean(files); + expect(result.pass).toBe(true); + expect(result.detail).toContain('1 docs/proposals/ file(s)'); + expect(result.detail).toContain('ensure these are intentional'); + }); + + it('warns with both .squad/ and docs/proposals/ counts', () => { + const files = [ + { filename: '.squad/team.md' }, + { filename: '.squad/routing.md' }, + { filename: 'docs/proposals/pr-readiness-checks.md' }, + ]; + const result = checkScopeClean(files); + expect(result.pass).toBe(true); + expect(result.detail).toContain('2 .squad/ file(s)'); + expect(result.detail).toContain('1 docs/proposals/ file(s)'); + }); + + it('catches nested .squad/ paths', () => { + const files = [{ filename: '.squad/agents/eecom/history.md' }]; + const result = checkScopeClean(files); + expect(result.pass).toBe(true); + expect(result.detail).toContain('1 .squad/ file(s)'); + }); + + it('handles null/undefined files gracefully', () => { + const result = checkScopeClean(null); + expect(result.pass).toBe(true); + expect(result.detail).toContain('No .squad/ or docs/proposals/ files'); + }); +}); + +// --------------------------------------------------------------------------- +// checkMergeability +// --------------------------------------------------------------------------- + +describe('checkMergeability', () => { + it('passes when mergeable is true', () => { + const result = checkMergeability(true); + expect(result.pass).toBe(true); + expect(result.detail).toContain('No merge conflicts'); + }); + + it('fails when mergeable is false', () => { + const result = checkMergeability(false); + expect(result.pass).toBe(false); + expect(result.detail).toContain('Merge conflicts'); + }); + + it('passes (not penalized) when mergeable is null', () => { + const result = checkMergeability(null); + expect(result.pass).toBe(true); + expect(result.detail).toContain('unknown'); + }); +}); + +// --------------------------------------------------------------------------- +// checkCIStatus +// --------------------------------------------------------------------------- + +describe('checkCIStatus', () => { + it('passes when all checks succeed', () => { + const checks = [ + { name: 'build', conclusion: 'success', status: 'completed' }, + { name: 'lint', conclusion: 'success', status: 'completed' }, + ]; + const result = checkCIStatus(checks, [{ state: 'success' }]); + expect(result.pass).toBe(true); + expect(result.detail).toContain('All checks passing'); + }); + + it('fails when checks are failing', () => { + const checks = [ + { name: 'build', conclusion: 'failure', status: 'completed' }, + { name: 'lint', conclusion: 'success', status: 'completed' }, + ]; + const result = checkCIStatus(checks, []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('1 check(s) failing'); + expect(result.detail).toContain('build'); + }); + + it('fails when checks are pending', () => { + const checks = [ + { name: 'build', conclusion: null, status: 'in_progress' }, + ]; + const result = checkCIStatus(checks, []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('still running'); + }); + + it('fails when no checks have run', () => { + const result = checkCIStatus([], []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('No CI checks have run yet'); + }); + + it('filters out self check runs', () => { + const checks = [ + { name: 'readiness', conclusion: 'success', status: 'completed' }, + { name: 'PR Readiness Check', conclusion: 'success', status: 'completed' }, + ]; + const result = checkCIStatus(checks, []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('No CI checks have run yet'); + }); + + it('reports cancelled checks as failures', () => { + const checks = [ + { name: 'build', conclusion: 'cancelled', status: 'completed' }, + ]; + const result = checkCIStatus(checks, []); + expect(result.pass).toBe(false); + expect(result.detail).toContain('failing'); + }); + + it('handles null checkRuns/statuses gracefully', () => { + const result = checkCIStatus(null, null); + expect(result.pass).toBe(false); + expect(result.detail).toContain('No CI checks have run yet'); + }); + + it('passes when only statuses exist (no check runs)', () => { + const result = checkCIStatus([], [{ state: 'success' }]); + expect(result.pass).toBe(true); + expect(result.detail).toContain('All checks passing'); + }); +}); + +// --------------------------------------------------------------------------- +// checkCopilotThreads +// --------------------------------------------------------------------------- + +describe('checkCopilotThreads', () => { + const copilotThread = (resolved: boolean) => ({ + isResolved: resolved, + comments: { nodes: [{ author: { login: 'copilot-pull-request-reviewer' } }] }, + }); + + const humanThread = (resolved: boolean) => ({ + isResolved: resolved, + comments: { nodes: [{ author: { login: 'some-human' } }] }, + }); + + it('passes when all copilot threads are resolved', () => { + const threads = [copilotThread(true), copilotThread(true)]; + const result = checkCopilotThreads(threads); + expect(result.pass).toBe(true); + expect(result.detail).toContain('All 2 Copilot thread(s) resolved'); + }); + + it('passes when no copilot threads exist', () => { + const result = checkCopilotThreads([]); + expect(result.pass).toBe(true); + expect(result.detail).toBe('No Copilot review threads'); + }); + + it('passes with null/undefined input', () => { + expect(checkCopilotThreads(null).pass).toBe(true); + expect(checkCopilotThreads(undefined).pass).toBe(true); + }); + + it('fails when unresolved copilot threads exist', () => { + const threads = [copilotThread(true), copilotThread(false), copilotThread(false)]; + const result = checkCopilotThreads(threads); + expect(result.pass).toBe(false); + expect(result.detail).toContain('2 unresolved Copilot thread(s)'); + }); + + it('ignores non-copilot threads', () => { + const threads = [humanThread(false), copilotThread(true)]; + const result = checkCopilotThreads(threads); + expect(result.pass).toBe(true); + expect(result.detail).toContain('All 1 Copilot thread(s) resolved'); + }); + + it('handles threads with missing comment data', () => { + const threads = [ + { isResolved: false, comments: { nodes: [] } }, + { isResolved: false, comments: null }, + copilotThread(true), + ]; + const result = checkCopilotThreads(threads); + expect(result.pass).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// buildChecklist +// --------------------------------------------------------------------------- + +describe('buildChecklist', () => { + it('includes the comment marker', () => { + const checks = [{ name: 'Test', pass: true, detail: 'OK' }]; + const body = buildChecklist(checks, 'owner', 'repo', 'dev'); + expect(body).toContain(COMMENT_MARKER); + }); + + it('shows ready status when all pass', () => { + const checks = [ + { name: 'Single commit', pass: true, detail: '1 commit' }, + { name: 'Not in draft', pass: true, detail: 'Ready' }, + ]; + const body = buildChecklist(checks, 'owner', 'repo', 'dev'); + expect(body).toContain('✅ PR is ready for review'); + }); + + it('shows warning status when some fail', () => { + const checks = [ + { name: 'Single commit', pass: false, detail: '3 commits' }, + { name: 'Not in draft', pass: true, detail: 'Ready' }, + ]; + const body = buildChecklist(checks, 'owner', 'repo', 'dev'); + expect(body).toContain('⚠️ 1 item(s) to address'); + }); + + it('includes links to CONTRIBUTING.md and PR_REQUIREMENTS.md', () => { + const checks = [{ name: 'Test', pass: true, detail: 'OK' }]; + const body = buildChecklist(checks, 'myorg', 'myrepo', 'main'); + expect(body).toContain('https://github.com/myorg/myrepo/blob/main/CONTRIBUTING.md'); + expect(body).toContain('https://github.com/myorg/myrepo/blob/main/.github/PR_REQUIREMENTS.md'); + }); + + it('renders a table row for each check', () => { + const checks = [ + { name: 'A', pass: true, detail: 'OK' }, + { name: 'B', pass: false, detail: 'Bad' }, + ]; + const body = buildChecklist(checks, 'o', 'r', 'dev'); + expect(body).toContain('| ✅ | **A** | OK |'); + expect(body).toContain('| ❌ | **B** | Bad |'); + }); +}); + +// --------------------------------------------------------------------------- +// paginate +// --------------------------------------------------------------------------- + +describe('paginate', () => { + it('collects items from a single page', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => [{ id: 1 }, { id: 2 }], + headers: new Map([['link', '']]), + }); + const items = await paginate(mockFetch, 'https://api.example.com/items', {}); + expect(items).toHaveLength(2); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('follows pagination links', async () => { + const mockFetch = vi.fn() + .mockResolvedValueOnce({ + ok: true, + json: async () => [{ id: 1 }], + headers: new Map([['link', '; rel="next"']]), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => [{ id: 2 }], + headers: new Map([['link', '']]), + }); + const items = await paginate(mockFetch, 'https://api.example.com/items', {}); + expect(items).toHaveLength(2); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('throws on non-ok response', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: false, + status: 404, + headers: new Map(), + }); + await expect(paginate(mockFetch, 'https://api.example.com/items', {})).rejects.toThrow('404'); + }); + + it('extracts check_runs from wrapped response', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ check_runs: [{ id: 1 }, { id: 2 }] }), + headers: new Map([['link', '']]), + }); + const items = await paginate(mockFetch, 'https://api.example.com/check-runs', {}); + expect(items).toHaveLength(2); + }); +}); + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +describe('constants', () => { + it('SELF_CHECK_NAMES contains expected values', () => { + expect(SELF_CHECK_NAMES).toContain('readiness'); + expect(SELF_CHECK_NAMES).toContain('PR Readiness Check'); + }); + + it('SOURCE_PATTERN matches SDK source files', () => { + expect(SOURCE_PATTERN.test('packages/squad-sdk/src/index.ts')).toBe(true); + }); + + it('SOURCE_PATTERN matches CLI source files', () => { + expect(SOURCE_PATTERN.test('packages/squad-cli/src/cli.ts')).toBe(true); + }); + + it('SOURCE_PATTERN does not match non-source files', () => { + expect(SOURCE_PATTERN.test('README.md')).toBe(false); + expect(SOURCE_PATTERN.test('.github/workflows/ci.yml')).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// run() orchestration +// --------------------------------------------------------------------------- + +describe('run()', () => { + const baseEnv = { + GITHUB_TOKEN: 'test-token', + PR_NUMBER: '42', + PR_DRAFT: 'false', + PR_HEAD_SHA: 'abc123', + PR_BASE_REF: 'dev', + REPO_OWNER: 'testorg', + REPO_NAME: 'testrepo', + RUN_NAME: 'Squad PR Readiness', + PR_LABELS: '[]', + }; + + function createMockFetch(overrides = {}) { + const defaults = { + commits: [{ sha: 'abc123' }], + compare: { behind_by: 0 }, + reviews: [{ user: { login: 'copilot-pull-request-reviewer' }, state: 'APPROVED', submitted_at: '2025-01-01T00:00:00Z' }], + files: [{ filename: '.changeset/feat.md' }], + pr: { mergeable: true }, + checkRuns: { check_runs: [{ name: 'build', conclusion: 'success', status: 'completed' }] }, + status: { statuses: [{ state: 'success' }] }, + comments: [], + reviewThreads: { + data: { + repository: { + pullRequest: { + reviewThreads: { + nodes: [{ + isResolved: true, + comments: { nodes: [{ author: { login: 'copilot-pull-request-reviewer' } }] }, + }], + }, + }, + }, + }, + }, + }; + const data = { ...defaults, ...overrides }; + + return vi.fn().mockImplementation(async (url, opts) => { + const headers = new Map([['link', '']]); + const ok = (json) => ({ ok: true, json: async () => json, headers }); + + // GraphQL endpoint for review threads + if (url === 'https://api.github.com/graphql') return ok(data.reviewThreads); + + if (url.includes('/commits?')) return ok(data.commits); + if (url.includes('/compare/')) return ok(data.compare); + if (url.includes('/reviews?')) return ok(data.reviews); + if (url.includes('/files?')) return ok(data.files); + if (url.includes('/check-runs?')) return ok(data.checkRuns); + if (url.includes('/status')) return ok(data.status); + if (url.includes('/comments?')) return ok(data.comments); + if (url.match(/\/pulls\/\d+$/)) return ok(data.pr); + + // POST/PATCH for comment upsert + if (url.includes('/comments')) return ok({ id: 999 }); + + return ok({}); + }); + } + + it('creates a comment when none exists (all checks pass)', async () => { + const mockFetch = createMockFetch(); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + expect(result.action).toBe('created'); + expect(result.checks).toHaveLength(9); + expect(result.checks.every((c) => c.pass)).toBe(true); + + // Verify POST was called for comment creation (not PATCH) + const postCalls = mockFetch.mock.calls.filter( + ([url, opts]) => opts && opts.method === 'POST' && url.includes('/issues/'), + ); + expect(postCalls.length).toBe(1); + expect(postCalls[0][0]).toContain('/issues/42/comments'); + }); + + it('updates an existing comment', async () => { + const mockFetch = createMockFetch({ + comments: [{ id: 123, body: `${COMMENT_MARKER}\nold body` }], + }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + expect(result.action).toBe('updated'); + const patchCalls = mockFetch.mock.calls.filter( + ([url, opts]) => opts && opts.method === 'PATCH', + ); + expect(patchCalls.length).toBe(1); + expect(patchCalls[0][0]).toContain('/issues/comments/123'); + }); + + it('marks draft PRs as failing', async () => { + const mockFetch = createMockFetch(); + const env = { ...baseEnv, PR_DRAFT: 'true' }; + const result = await run({ env, fetchFn: mockFetch }); + + const draftCheck = result.checks.find((c) => c.name === 'Not in draft'); + expect(draftCheck.pass).toBe(false); + }); + + it('handles multiple commits', async () => { + const mockFetch = createMockFetch({ + commits: [{ sha: '1' }, { sha: '2' }, { sha: '3' }], + }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + const commitCheck = result.checks.find((c) => c.name === 'Single commit'); + expect(commitCheck.pass).toBe(false); + expect(commitCheck.detail).toContain('3 commits'); + }); + + it('handles branch behind base', async () => { + const mockFetch = createMockFetch({ compare: { behind_by: 5 } }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + const branchCheck = result.checks.find((c) => c.name === 'Branch up to date'); + expect(branchCheck.pass).toBe(false); + expect(branchCheck.detail).toContain('5 commit(s) ahead'); + }); + + it('handles failed branch comparison gracefully', async () => { + const mockFetch = createMockFetch(); + mockFetch.mockImplementation(async (url) => { + const headers = new Map([['link', '']]); + const ok = (json) => ({ ok: true, json: async () => json, headers }); + if (url === 'https://api.github.com/graphql') return ok({ data: { repository: { pullRequest: { reviewThreads: { nodes: [] } } } } }); + if (url.includes('/compare/')) return { ok: false, status: 404, headers }; + if (url.includes('/commits?')) return ok([{ sha: '1' }]); + if (url.includes('/reviews?')) return ok([]); + if (url.includes('/files?')) return ok([]); + if (url.includes('/check-runs?')) return ok({ check_runs: [] }); + if (url.includes('/status')) return ok({ statuses: [] }); + if (url.includes('/comments?')) return ok([]); + if (url.match(/\/pulls\/\d+$/)) return ok({ mergeable: true }); + if (url.includes('/comments')) return ok({ id: 1 }); + return ok({}); + }); + + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + const branchCheck = result.checks.find((c) => c.name === 'Branch up to date'); + expect(branchCheck.pass).toBe(false); + expect(branchCheck.detail).toContain('Could not determine'); + }); + + it('passes PR labels through for changeset check', async () => { + const mockFetch = createMockFetch({ + files: [{ filename: 'packages/squad-sdk/src/foo.ts' }], + }); + const env = { + ...baseEnv, + PR_LABELS: JSON.stringify([{ name: 'skip-changelog' }]), + }; + const result = await run({ env, fetchFn: mockFetch }); + + const changesetCheck = result.checks.find((c) => c.name === 'Changeset present'); + expect(changesetCheck.pass).toBe(true); + expect(changesetCheck.detail).toContain('skip-changelog'); + }); + + it('handles invalid PR_LABELS JSON gracefully', async () => { + const mockFetch = createMockFetch({ + files: [{ filename: 'packages/squad-sdk/src/foo.ts' }], + }); + const env = { ...baseEnv, PR_LABELS: 'not-json' }; + const result = await run({ env, fetchFn: mockFetch }); + + const changesetCheck = result.checks.find((c) => c.name === 'Changeset present'); + expect(changesetCheck).toBeDefined(); + }); + + it('handles merge conflict detection', async () => { + const mockFetch = createMockFetch({ pr: { mergeable: false } }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + const mergeCheck = result.checks.find((c) => c.name === 'No merge conflicts'); + expect(mergeCheck.pass).toBe(false); + expect(mergeCheck.detail).toContain('Merge conflicts'); + }); + + it('handles CI failures', async () => { + const mockFetch = createMockFetch({ + checkRuns: { check_runs: [{ name: 'build', conclusion: 'failure', status: 'completed' }] }, + }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + const ciCheck = result.checks.find((c) => c.name === 'CI passing'); + expect(ciCheck.pass).toBe(false); + expect(ciCheck.detail).toContain('failing'); + }); + + it('handles empty reviews (no copilot review)', async () => { + const mockFetch = createMockFetch({ reviews: [] }); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + + const copilotCheck = result.checks.find((c) => c.name === 'Copilot review'); + expect(copilotCheck.pass).toBe(false); + expect(copilotCheck.detail).toContain('No Copilot review yet'); + }); + + it('always produces 9 checks', async () => { + const mockFetch = createMockFetch(); + const result = await run({ env: baseEnv, fetchFn: mockFetch }); + expect(result.checks).toHaveLength(9); + + const names = result.checks.map((c) => c.name); + expect(names).toEqual([ + 'Single commit', + 'Not in draft', + 'Branch up to date', + 'Copilot review', + 'Changeset present', + 'Scope clean', + 'No merge conflicts', + 'Copilot threads resolved', + 'CI passing', + ]); + }); + + it('includes comment marker in upserted body', async () => { + const mockFetch = createMockFetch(); + await run({ env: baseEnv, fetchFn: mockFetch }); + + const postCall = mockFetch.mock.calls.find( + ([url, opts]) => opts && opts.method === 'POST' && url.includes('/comments'), + ); + expect(postCall).toBeDefined(); + const body = JSON.parse(postCall[1].body).body; + expect(body).toContain(COMMENT_MARKER); + }); +});