diff --git a/action/index.cjs b/action/index.cjs index 0779c91..f199b2b 100644 --- a/action/index.cjs +++ b/action/index.cjs @@ -147027,15 +147027,176 @@ var __importDefault = (this && this.__importDefault) || function (mod) { return (mod && mod.__esModule) ? mod : { "default": mod }; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.robot = void 0; +exports.robot = exports.getChangedFiles = exports.createReviewBody = exports.createInlineReviewComment = exports.isHunkHeaderInPatch = exports.getReviewCommentLocation = void 0; const minimatch_1 = __nccwpck_require__(4501); const chat_js_1 = __nccwpck_require__(85365); const loglevel_1 = __importDefault(__nccwpck_require__(78063)); const OPENAI_API_KEY = 'OPENAI_API_KEY'; +const COMPARE_FILES_LIMIT = 300; +const REVIEW_MARKER = ''; const MAX_PATCH_COUNT = process.env.MAX_PATCH_LENGTH ? +process.env.MAX_PATCH_LENGTH : Infinity; +const parseHunkHeader = (hunkHeader) => { + if (!hunkHeader) { + return null; + } + const match = hunkHeader.trim().match(/^@@\s+-\s*(\d+)(?:,(\d+))?\s+\+\s*(\d+)(?:,(\d+))?\s+@@/); + if (!match) { + return null; + } + const [, oldStartValue, oldCountValue, newStartValue, newCountValue] = match; + const oldStart = Number(oldStartValue); + const oldCount = oldCountValue === undefined ? 1 : Number(oldCountValue); + const newStart = Number(newStartValue); + const newCount = newCountValue === undefined ? 1 : Number(newCountValue); + return { oldStart, oldCount, newStart, newCount }; +}; +const getReviewCommentLocation = (hunkHeader) => { + const range = parseHunkHeader(hunkHeader); + if (!range) { + return null; + } + const { oldStart, oldCount, newStart, newCount } = range; + if (newCount > 0) { + return { + line: newStart + newCount - 1, + side: 'RIGHT', + }; + } + if (oldCount > 0) { + return { + line: oldStart + oldCount - 1, + side: 'LEFT', + }; + } + return null; +}; +exports.getReviewCommentLocation = getReviewCommentLocation; +const isHunkHeaderInPatch = (hunkHeader, patch) => { + const expectedRange = parseHunkHeader(hunkHeader); + if (!expectedRange) { + return false; + } + return patch.split('\n').some((line) => { + if (!line.startsWith('@@')) { + return false; + } + const actualRange = parseHunkHeader(line); + return (actualRange?.oldStart === expectedRange.oldStart && + actualRange.oldCount === expectedRange.oldCount && + actualRange.newStart === expectedRange.newStart && + actualRange.newCount === expectedRange.newCount); + }); +}; +exports.isHunkHeaderInPatch = isHunkHeaderInPatch; +const createInlineReviewComment = (path, body, hunkHeader, patch) => { + const location = (0, exports.getReviewCommentLocation)(hunkHeader); + if (!location || !(0, exports.isHunkHeaderInPatch)(hunkHeader, patch)) { + return null; + } + return { + path, + body, + line: location.line, + side: location.side, + }; +}; +exports.createInlineReviewComment = createInlineReviewComment; +const createReviewBody = (hasInlineComments, bodyComments) => { + const heading = hasInlineComments || bodyComments.length + ? 'Code review by ChatGPT' + : 'LGTM 👍'; + return [heading, ...bodyComments, REVIEW_MARKER].join('\n\n'); +}; +exports.createReviewBody = createReviewBody; +const getChangedFiles = async (context) => { + const repo = context.repo(); + const pullRequestFiles = await context.octokit.paginate(context.octokit.pulls.listFiles, { + owner: repo.owner, + repo: repo.repo, + pull_number: context.pullRequest().pull_number, + per_page: 100, + }); + if (context.payload.action !== 'synchronize') { + return pullRequestFiles; + } + let comparisonBase; + try { + const reviews = await context.octokit.paginate(context.octokit.pulls.listReviews, { + owner: repo.owner, + repo: repo.repo, + pull_number: context.pullRequest().pull_number, + per_page: 100, + }); + const botReview = reviews + .slice() + .reverse() + .find((r) => r.user?.type === 'Bot' && + r.body?.includes(REVIEW_MARKER)); + if (botReview) { + if (!botReview.commit_id) { + return pullRequestFiles; + } + comparisonBase = botReview.commit_id; + } + else { + const hasLegacyReview = reviews.some((r) => r.user?.type === 'Bot' && + r.body && + (r.body.startsWith('Code review by ChatGPT') || + r.body.startsWith('LGTM'))); + if (hasLegacyReview) { + return pullRequestFiles; + } + } + } + catch (err) { + loglevel_1.default.debug('failed to detect previous bot review', err); + return pullRequestFiles; + } + if (!comparisonBase) { + comparisonBase = context.payload.before; + } + if (!comparisonBase) { + return pullRequestFiles; + } + try { + const { data } = await context.octokit.repos.compareCommits({ + owner: repo.owner, + repo: repo.repo, + base: comparisonBase, + head: context.payload.pull_request.head.sha, + }); + if (data.status === 'identical') { + return []; + } + if (data.status === 'ahead') { + if (!data.files) { + loglevel_1.default.debug('commit comparison omitted files; using the full pull request diff'); + return pullRequestFiles; + } + const comparisonFiles = data.files; + if (comparisonFiles.length >= COMPARE_FILES_LIMIT) { + loglevel_1.default.debug('commit comparison reached the GitHub file limit; using the full pull request diff'); + return pullRequestFiles; + } + const changedFilenames = new Set(comparisonFiles.map((file) => file.filename)); + return pullRequestFiles.filter((file) => changedFilenames.has(file.filename)); + } + loglevel_1.default.debug(`commit comparison from ${comparisonBase} is ${data.status}; using the full pull request diff`); + } + catch (err) { + loglevel_1.default.debug(`failed to compare commits from ${comparisonBase}`, err); + } + return pullRequestFiles; +}; +exports.getChangedFiles = getChangedFiles; const robot = (app) => { + if (process.env.GITHUB_ACTIONS === 'true') { + app.onError(() => { + process.exitCode = 1; + }); + } const loadChat = async (context) => { if (process.env.USE_GITHUB_MODELS === 'true' && process.env.GITHUB_TOKEN) { return new chat_js_1.Chat(process.env.GITHUB_TOKEN); @@ -147086,61 +147247,7 @@ const robot = (app) => { loglevel_1.default.info('no target label attached'); return 'no target label attached'; } - const data = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: context.payload.pull_request.base.sha, - head: context.payload.pull_request.head.sha, - }); - let { files: changedFiles, commits } = data.data; - if (context.payload.action === 'synchronize') { - // Try to detect the last commit we reviewed (by looking for our previous review) - try { - const reviewsResp = await context.octokit.pulls.listReviews({ - owner: repo.owner, - repo: repo.repo, - pull_number: context.pullRequest().pull_number, - }); - const reviews = reviewsResp.data || []; - // Find the most recent review created by this bot (we mark our reviews with a body) - const botReview = reviews - .slice() - .reverse() - .find((r) => r.body && (r.body.startsWith('Code review by ChatGPT') || r.body.startsWith('LGTM'))); - if (botReview?.commit_id) { - const { data: { files, commits: newCommits }, } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: botReview.commit_id, - head: context.payload.pull_request.head.sha, - }); - changedFiles = files; - commits = newCommits; - } - else if (commits.length >= 2) { - // fallback: compare last two commits in the PR - const { data: { files }, } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: commits[commits.length - 2].sha, - head: commits[commits.length - 1].sha, - }); - changedFiles = files; - } - } - catch (err) { - loglevel_1.default.debug('failed to detect previous bot review, falling back', err); - if (commits.length >= 2) { - const { data: { files }, } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: commits[commits.length - 2].sha, - head: commits[commits.length - 1].sha, - }); - changedFiles = files; - } - } - } + let changedFiles = await (0, exports.getChangedFiles)(context); loglevel_1.default.debug('changedFiles:', changedFiles); loglevel_1.default.debug; const ignoreList = (process.env.IGNORE || process.env.ignore || '') @@ -147173,6 +147280,7 @@ const robot = (app) => { } console.time('gpt cost'); const ress = []; + const bodyComments = []; for (let i = 0; i < changedFiles.length; i++) { const file = changedFiles[i]; const patch = file.patch || ''; @@ -147189,26 +147297,14 @@ const robot = (app) => { const reviews = Array.isArray(res) ? res : [res]; for (const review of reviews) { if (!review.lgtm && !!review.review_comment) { - let line; - let side = 'RIGHT'; - // Extract line number from hunk header if available - if (review.hunk_header) { - const c = review.hunk_header.match(/\+\s*(\d+),(\d+)/); - if (c) { - const [, start, count] = c; - line = Number(start) + Number(count) - 1; - } - else { - loglevel_1.default.error(`Failed to parse hunk header: ${review.hunk_header}`); - continue; - } + const inlineComment = (0, exports.createInlineReviewComment)(file.filename, review.review_comment, review.hunk_header, patch); + if (!inlineComment) { + const filename = file.filename.replace(/`/g, '\\`'); + bodyComments.push(`**File:** \`${filename}\`\n\n${review.review_comment}`); + loglevel_1.default.error(`Failed to locate inline review comment: ${review.hunk_header || 'missing hunk header'}`); + continue; } - ress.push({ - path: file.filename, - body: review.review_comment, - line: line, - side: side, - }); + ress.push(inlineComment); } } } @@ -147222,7 +147318,7 @@ const robot = (app) => { repo: repo.repo, owner: repo.owner, pull_number: context.pullRequest().pull_number, - body: ress.length ? "Code review by ChatGPT" : "LGTM 👍", + body: (0, exports.createReviewBody)(ress.length > 0, bodyComments), event: 'COMMENT', commit_id: context.payload.pull_request.head.sha, comments: ress, diff --git a/action/src/bot.d.ts b/action/src/bot.d.ts index a458589..8fe8f84 100644 --- a/action/src/bot.d.ts +++ b/action/src/bot.d.ts @@ -1,2 +1,32 @@ -import { Probot } from 'probot'; +import { Context, Probot } from 'probot'; +type PullRequestContext = Context<'pull_request.opened' | 'pull_request.synchronize'>; +export declare const getReviewCommentLocation: (hunkHeader?: string) => { + line: number; + side: "RIGHT"; +} | { + line: number; + side: "LEFT"; +} | null; +export declare const isHunkHeaderInPatch: (hunkHeader: string | undefined, patch: string) => boolean; +export declare const createInlineReviewComment: (path: string, body: string, hunkHeader: string | undefined, patch: string) => { + path: string; + body: string; + line: number; + side: "RIGHT" | "LEFT"; +} | null; +export declare const createReviewBody: (hasInlineComments: boolean, bodyComments: string[]) => string; +export declare const getChangedFiles: (context: PullRequestContext) => Promise<{ + sha: string; + filename: string; + status: "added" | "removed" | "modified" | "renamed" | "copied" | "changed" | "unchanged"; + additions: number; + deletions: number; + changes: number; + blob_url: string; + raw_url: string; + contents_url: string; + patch?: string | undefined; + previous_filename?: string | undefined; +}[]>; export declare const robot: (app: Probot) => void; +export {}; diff --git a/jest.config.js b/jest.config.cjs similarity index 81% rename from jest.config.js rename to jest.config.cjs index 9ee81b0..0ab9764 100644 --- a/jest.config.js +++ b/jest.config.cjs @@ -5,5 +5,8 @@ module.exports = { }, testRegex: "(/__tests__/.*|\\.(test|spec))\\.[tj]sx?$", moduleFileExtensions: ["ts", "tsx", "js", "jsx", "json", "node"], + moduleNameMapper: { + "^(\\.{1,2}/.*)\\.js$": "$1", + }, testEnvironment: "node", }; diff --git a/src/bot.ts b/src/bot.ts index 71eb783..489e85e 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -5,11 +5,234 @@ import { Chat } from './chat.js'; import log from 'loglevel'; const OPENAI_API_KEY = 'OPENAI_API_KEY'; +const COMPARE_FILES_LIMIT = 300; +const REVIEW_MARKER = ''; const MAX_PATCH_COUNT = process.env.MAX_PATCH_LENGTH ? +process.env.MAX_PATCH_LENGTH : Infinity; +type PullRequestContext = Context< + 'pull_request.opened' | 'pull_request.synchronize' +>; + +const parseHunkHeader = (hunkHeader?: string) => { + if (!hunkHeader) { + return null; + } + + const match = hunkHeader.trim().match( + /^@@\s+-\s*(\d+)(?:,(\d+))?\s+\+\s*(\d+)(?:,(\d+))?\s+@@/ + ); + if (!match) { + return null; + } + + const [, oldStartValue, oldCountValue, newStartValue, newCountValue] = match; + const oldStart = Number(oldStartValue); + const oldCount = oldCountValue === undefined ? 1 : Number(oldCountValue); + const newStart = Number(newStartValue); + const newCount = newCountValue === undefined ? 1 : Number(newCountValue); + + return { oldStart, oldCount, newStart, newCount }; +}; + +export const getReviewCommentLocation = (hunkHeader?: string) => { + const range = parseHunkHeader(hunkHeader); + if (!range) { + return null; + } + + const { oldStart, oldCount, newStart, newCount } = range; + + if (newCount > 0) { + return { + line: newStart + newCount - 1, + side: 'RIGHT' as const, + }; + } + + if (oldCount > 0) { + return { + line: oldStart + oldCount - 1, + side: 'LEFT' as const, + }; + } + + return null; +}; + +export const isHunkHeaderInPatch = ( + hunkHeader: string | undefined, + patch: string +) => { + const expectedRange = parseHunkHeader(hunkHeader); + if (!expectedRange) { + return false; + } + + return patch.split('\n').some((line) => { + if (!line.startsWith('@@')) { + return false; + } + const actualRange = parseHunkHeader(line); + return ( + actualRange?.oldStart === expectedRange.oldStart && + actualRange.oldCount === expectedRange.oldCount && + actualRange.newStart === expectedRange.newStart && + actualRange.newCount === expectedRange.newCount + ); + }); +}; + +export const createInlineReviewComment = ( + path: string, + body: string, + hunkHeader: string | undefined, + patch: string +) => { + const location = getReviewCommentLocation(hunkHeader); + if (!location || !isHunkHeaderInPatch(hunkHeader, patch)) { + return null; + } + + return { + path, + body, + line: location.line, + side: location.side, + }; +}; + +export const createReviewBody = ( + hasInlineComments: boolean, + bodyComments: string[] +) => { + const heading = + hasInlineComments || bodyComments.length + ? 'Code review by ChatGPT' + : 'LGTM 👍'; + return [heading, ...bodyComments, REVIEW_MARKER].join('\n\n'); +}; + +export const getChangedFiles = async (context: PullRequestContext) => { + const repo = context.repo(); + const pullRequestFiles = await context.octokit.paginate( + context.octokit.pulls.listFiles, + { + owner: repo.owner, + repo: repo.repo, + pull_number: context.pullRequest().pull_number, + per_page: 100, + } + ); + + if (context.payload.action !== 'synchronize') { + return pullRequestFiles; + } + + let comparisonBase: string | undefined; + + try { + const reviews = await context.octokit.paginate( + context.octokit.pulls.listReviews, + { + owner: repo.owner, + repo: repo.repo, + pull_number: context.pullRequest().pull_number, + per_page: 100, + } + ); + const botReview = reviews + .slice() + .reverse() + .find((r) => + r.user?.type === 'Bot' && + r.body?.includes(REVIEW_MARKER) + ); + + if (botReview) { + if (!botReview.commit_id) { + return pullRequestFiles; + } + comparisonBase = botReview.commit_id; + } else { + const hasLegacyReview = reviews.some( + (r) => + r.user?.type === 'Bot' && + r.body && + (r.body.startsWith('Code review by ChatGPT') || + r.body.startsWith('LGTM')) + ); + if (hasLegacyReview) { + return pullRequestFiles; + } + } + } catch (err) { + log.debug('failed to detect previous bot review', err); + return pullRequestFiles; + } + + if (!comparisonBase) { + comparisonBase = context.payload.before; + } + + if (!comparisonBase) { + return pullRequestFiles; + } + + try { + const { data } = await context.octokit.repos.compareCommits({ + owner: repo.owner, + repo: repo.repo, + base: comparisonBase, + head: context.payload.pull_request.head.sha, + }); + + if (data.status === 'identical') { + return []; + } + + if (data.status === 'ahead') { + if (!data.files) { + log.debug( + 'commit comparison omitted files; using the full pull request diff' + ); + return pullRequestFiles; + } + + const comparisonFiles = data.files; + if (comparisonFiles.length >= COMPARE_FILES_LIMIT) { + log.debug( + 'commit comparison reached the GitHub file limit; using the full pull request diff' + ); + return pullRequestFiles; + } + + const changedFilenames = new Set( + comparisonFiles.map((file) => file.filename) + ); + return pullRequestFiles.filter((file) => + changedFilenames.has(file.filename) + ); + } + + log.debug( + `commit comparison from ${comparisonBase} is ${data.status}; using the full pull request diff` + ); + } catch (err) { + log.debug(`failed to compare commits from ${comparisonBase}`, err); + } + + return pullRequestFiles; +}; + export const robot = (app: Probot) => { + if (process.env.GITHUB_ACTIONS === 'true') { + app.onError(() => { + process.exitCode = 1; + }); + } + const loadChat = async (context: Context) => { if (process.env.USE_GITHUB_MODELS === 'true' && process.env.GITHUB_TOKEN) { return new Chat(process.env.GITHUB_TOKEN); @@ -80,72 +303,7 @@ export const robot = (app: Probot) => { return 'no target label attached'; } - const data = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: context.payload.pull_request.base.sha, - head: context.payload.pull_request.head.sha, - }); - - let { files: changedFiles } = data.data; - - if (context.payload.action === 'synchronize') { - // Try to detect the last commit we reviewed (by looking for our previous review) - try { - const reviewsResp = await context.octokit.pulls.listReviews({ - owner: repo.owner, - repo: repo.repo, - pull_number: context.pullRequest().pull_number, - }); - - const reviews = reviewsResp.data || []; - // Find the most recent review created by this bot (we mark our reviews with a body) - const botReview = reviews - .slice() - .reverse() - .find((r) => r.body && (r.body.startsWith('Code review by ChatGPT') || r.body.startsWith('LGTM'))); - - if (botReview?.commit_id) { - const { - data: { files }, - } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: botReview.commit_id, - head: context.payload.pull_request.head.sha, - }); - - changedFiles = files; - } else if (context.payload.before) { - // Use the 'before' SHA from the synchronize event to capture - // all commits in the push, not just the last one. - const { - data: { files }, - } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: context.payload.before, - head: context.payload.pull_request.head.sha, - }); - - changedFiles = files; - } - } catch (err) { - log.debug('failed to detect previous bot review, falling back', err); - if (context.payload.before) { - const { - data: { files }, - } = await context.octokit.repos.compareCommits({ - owner: repo.owner, - repo: repo.repo, - base: context.payload.before, - head: context.payload.pull_request.head.sha, - }); - - changedFiles = files; - } - } - } + let changedFiles = await getChangedFiles(context); log.debug('changedFiles:', changedFiles); log.debug @@ -189,6 +347,7 @@ export const robot = (app: Probot) => { console.time('gpt cost'); const ress = []; + const bodyComments = []; for (let i = 0; i < changedFiles.length; i++) { const file = changedFiles[i]; @@ -211,28 +370,24 @@ export const robot = (app: Probot) => { for (const review of reviews) { if (!review.lgtm && !!review.review_comment) { - let line: number | undefined; - let side: 'LEFT' | 'RIGHT' = 'RIGHT'; - - // Extract line number from hunk header if available - if (review.hunk_header) { - const c = review.hunk_header.match(/\+\s*(\d+),(\d+)/); - if (c) { - const [, start, count] = c; - line = Number(start) + Number(count) - 1; - } - else { - log.error(`Failed to parse hunk header: ${review.hunk_header}`); - continue; - } + const inlineComment = createInlineReviewComment( + file.filename, + review.review_comment, + review.hunk_header, + patch + ); + if (!inlineComment) { + const filename = file.filename.replace(/`/g, '\\`'); + bodyComments.push( + `**File:** \`${filename}\`\n\n${review.review_comment}` + ); + log.error( + `Failed to locate inline review comment: ${review.hunk_header || 'missing hunk header'}` + ); + continue; } - ress.push({ - path: file.filename, - body: review.review_comment, - line: line, - side: side, - }) + ress.push(inlineComment) } } } catch (e) { @@ -245,7 +400,7 @@ export const robot = (app: Probot) => { repo: repo.repo, owner: repo.owner, pull_number: context.pullRequest().pull_number, - body: ress.length ? "Code review by ChatGPT" : "LGTM 👍", + body: createReviewBody(ress.length > 0, bodyComments), event: 'COMMENT', commit_id: context.payload.pull_request.head.sha, comments: ress, @@ -279,4 +434,4 @@ const matchPatterns = (patterns: string[], path: string) => { } } }) -} \ No newline at end of file +} diff --git a/test/index.test.ts b/test/index.test.ts index d3a6e55..5bee4ce 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -1,74 +1,549 @@ -// You can import your modules -// import index from '../src/index' - -import nock from "nock"; -// Requiring our app implementation -import myProbotApp from "../src"; -import { Probot, ProbotOctokit } from "probot"; -// Requiring our fixtures -import payload from "./fixtures/issues.opened.json"; -const issueCreatedBody = { body: "Thanks for opening this issue!" }; -const fs = require("fs"); -const path = require("path"); - -const privateKey = fs.readFileSync( - path.join(__dirname, "fixtures/mock-cert.pem"), - "utf-8" -); - -describe("My Probot app", () => { - let probot: any; - - beforeEach(() => { - nock.disableNetConnect(); - probot = new Probot({ - appId: 123, - privateKey, - // disable request throttling and retries for testing - Octokit: ProbotOctokit.defaults({ - retry: { enabled: false }, - throttle: { enabled: false }, +import { + createInlineReviewComment, + createReviewBody, + getChangedFiles, + getReviewCommentLocation, + isHunkHeaderInPatch, + robot, +} from '../src/bot'; + +const pullRequestFiles = [ + { + filename: 'src/current.ts', + status: 'modified', + contents_url: 'https://api.github.com/repos/owner/repo/contents/src/current.ts', + patch: 'current pull request patch', + }, + { + filename: 'src/incremental.ts', + status: 'modified', + contents_url: 'https://api.github.com/repos/owner/repo/contents/src/incremental.ts', + patch: 'incremental pull request patch', + }, +]; + +const incrementalFiles = [ + { + filename: 'src/incremental.ts', + status: 'modified', + contents_url: 'https://api.github.com/repos/owner/repo/contents/src/incremental.ts', + patch: 'commit comparison patch', + }, +]; + +const unrelatedFiles = [ + { + filename: 'README.md', + status: 'modified', + contents_url: 'https://api.github.com/repos/owner/repo/contents/README.md', + patch: 'unrelated comparison patch', + }, +]; + +const patch = [ + '@@ -10,2 +10,3 @@ function example() {', + ' unchanged', + '-old', + '+new', + '+added', + '@@ -20,3 +21,0 @@ function removed() {', + '-one', + '-two', + '-three', +].join('\n'); + +const createBotReview = ( + body = 'Code review by ChatGPT', + commitId: string | null = 'reviewed-head' +) => ({ + body: `${body}\n\n`, + commit_id: commitId, + user: { type: 'Bot' }, +}); + +const createContext = ({ + action = 'synchronize', + before = 'previous-head', + reviews = [], + comparisons = [], + reviewsError, +}: { + action?: 'opened' | 'synchronize'; + before?: string; + reviews?: Array<{ + body: string; + commit_id?: string | null; + user?: { type: string }; + }>; + comparisons?: Array< + | { + status: 'ahead' | 'behind' | 'diverged' | 'identical'; + files?: typeof pullRequestFiles; + } + | Error + >; + reviewsError?: Error; +} = {}) => { + const compareCommits = jest.fn(); + const listFiles = jest.fn(); + const listReviews = jest.fn(); + const paginate = jest.fn((method) => { + if (method === listFiles) { + return Promise.resolve(pullRequestFiles); + } + if (method === listReviews) { + if (reviewsError) { + return Promise.reject(reviewsError); + } + return Promise.resolve(reviews); + } + return Promise.reject(new Error('unexpected pagination method')); + }); + + for (const comparison of comparisons) { + if (comparison instanceof Error) { + compareCommits.mockRejectedValueOnce(comparison); + } else { + compareCommits.mockResolvedValueOnce({ data: comparison }); + } + } + + return { + context: { + repo: () => ({ owner: 'owner', repo: 'repo' }), + pullRequest: () => ({ + owner: 'owner', + repo: 'repo', + pull_number: 123, }), + payload: { + action, + before, + pull_request: { + head: { sha: 'current-head' }, + }, + }, + octokit: { + paginate, + pulls: { + listFiles, + listReviews, + }, + repos: { + compareCommits, + }, + }, + } as any, + compareCommits, + listFiles, + listReviews, + paginate, + }; +}; + +describe('getChangedFiles', () => { + test('uses the current pull request files when the pull request is opened', async () => { + const { context, compareCommits } = createContext({ action: 'opened' }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(context.octokit.pulls.listReviews).not.toHaveBeenCalled(); + expect(compareCommits).not.toHaveBeenCalled(); + }); + + test('uses current pull request patches for files in an incremental diff', async () => { + const { context, compareCommits } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'ahead', + files: incrementalFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual([ + pullRequestFiles[1], + ]); + expect(compareCommits).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + base: 'reviewed-head', + head: 'current-head', + }); + }); + + test('ignores matching review text from a human reviewer', async () => { + const { context, compareCommits } = createContext({ + reviews: [ + { + body: 'LGTM 👍', + commit_id: 'human-reviewed-head', + user: { type: 'User' }, + }, + ], + comparisons: [ + { + status: 'ahead', + files: incrementalFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual([ + pullRequestFiles[1], + ]); + expect(compareCommits).toHaveBeenCalledTimes(1); + expect(compareCommits).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + base: 'previous-head', + head: 'current-head', }); - // Load our app into probot - probot.load(myProbotApp); }); - test("creates a comment when an issue is opened", async () => { - const mock = nock("https://api.github.com") - // Test that we correctly return a test token - .post("/app/installations/2/access_tokens") - .reply(200, { - token: "test", - permissions: { - issues: "write", + test('uses the full pull request for a legacy bot review without the marker', async () => { + const { context, compareCommits } = createContext({ + reviews: [ + { + body: 'Code review by ChatGPT', + commit_id: 'legacy-reviewed-head', + user: { type: 'Bot' }, }, - }) + ], + }); - // Test that a comment is posted - .post("/repos/hiimbex/testing-things/issues/1/comments", (body: any) => { - expect(body).toMatchObject(issueCreatedBody); - return true; - }) - .reply(200); + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(compareCommits).not.toHaveBeenCalled(); + }); - // Receive a webhook event - await probot.receive({ name: "issues", payload }); + test('uses the full pull request when a trusted review has no commit', async () => { + const { context, compareCommits } = createContext({ + reviews: [createBotReview('Code review by ChatGPT', null)], + }); - expect(mock.pendingMocks()).toStrictEqual([]); + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(compareCommits).not.toHaveBeenCalled(); }); - afterEach(() => { - nock.cleanAll(); - nock.enableNetConnect(); + test('uses the full pull request when review history cannot be listed', async () => { + const { context, compareCommits } = createContext({ + reviewsError: new Error('reviews unavailable'), + }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(compareCommits).not.toHaveBeenCalled(); + }); + + test('excludes files that are only present in an ahead comparison', async () => { + const { context } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'ahead', + files: unrelatedFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual([]); + }); + + test('falls back when a comparison reaches the GitHub file limit', async () => { + const comparisonFiles = Array.from({ length: 300 }, (_, index) => ({ + filename: `src/file-${index}.ts`, + status: 'modified', + contents_url: `https://api.github.com/repos/owner/repo/contents/src/file-${index}.ts`, + patch: 'commit comparison patch', + })); + const { context } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'ahead', + files: comparisonFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + }); + + test('falls back when an ahead comparison omits files', async () => { + const { context } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'ahead', + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + }); + + test('returns no files for an identical comparison', async () => { + const { context } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'identical', + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual([]); + }); + + test('falls back to the current pull request files after diverged comparisons', async () => { + const { context, compareCommits } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'diverged', + files: unrelatedFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(compareCommits).toHaveBeenCalledTimes(1); + expect(compareCommits).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + base: 'reviewed-head', + head: 'current-head', + }); + }); + + test('uses the synchronize before SHA when no prior bot review exists', async () => { + const { context, compareCommits } = createContext({ + comparisons: [ + { + status: 'ahead', + files: incrementalFiles, + }, + ], + }); + + await expect(getChangedFiles(context)).resolves.toEqual([ + pullRequestFiles[1], + ]); + expect(compareCommits).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + base: 'previous-head', + head: 'current-head', + }); + }); + + test('uses the full pull request when the trusted review comparison fails', async () => { + const { context, compareCommits } = createContext({ + reviews: [createBotReview('LGTM 👍')], + comparisons: [new Error('reviewed commit is unavailable')], + }); + + await expect(getChangedFiles(context)).resolves.toEqual(pullRequestFiles); + expect(compareCommits).toHaveBeenCalledTimes(1); + expect(compareCommits).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + base: 'reviewed-head', + head: 'current-head', + }); + }); + + test('paginates pull request reviews before selecting the latest one', async () => { + const { context, listReviews, paginate } = createContext({ + reviews: [createBotReview()], + comparisons: [ + { + status: 'ahead', + files: incrementalFiles, + }, + ], + }); + + await getChangedFiles(context); + + expect(paginate).toHaveBeenCalledWith(listReviews, { + owner: 'owner', + repo: 'repo', + pull_number: 123, + per_page: 100, + }); }); }); -// For more information about testing with Jest see: -// https://facebook.github.io/jest/ +describe('getReviewCommentLocation', () => { + test('uses the last right-side line for a multi-line hunk', () => { + expect(getReviewCommentLocation('@@ -10,5 +10,7 @@')).toEqual({ + line: 16, + side: 'RIGHT', + }); + }); -// For more information about using TypeScript in your tests, Jest recommends: -// https://github.com/kulshekhar/ts-jest + test('defaults omitted hunk counts to one line', () => { + expect(getReviewCommentLocation('@@ -10 +12 @@ function name')).toEqual({ + line: 12, + side: 'RIGHT', + }); + }); -// For more information about testing with Nock see: -// https://github.com/nock/nock + test('uses the left side for a deletion-only hunk', () => { + expect(getReviewCommentLocation('@@ -10,3 +10,0 @@')).toEqual({ + line: 12, + side: 'LEFT', + }); + }); + + test('rejects malformed hunk headers', () => { + expect(getReviewCommentLocation('not a hunk header')).toBeNull(); + }); + + test('rejects a missing hunk header', () => { + expect(getReviewCommentLocation()).toBeNull(); + }); +}); + +describe('isHunkHeaderInPatch', () => { + test('accepts a hunk range present in the current patch', () => { + expect( + isHunkHeaderInPatch('@@ -10,2 +10,3 @@ function example() {', patch) + ).toBe(true); + }); + + test('normalizes omitted single-line counts', () => { + expect( + isHunkHeaderInPatch('@@ -30 +30 @@', '@@ -30,1 +30,1 @@') + ).toBe(true); + }); + + test('rejects a valid hunk range absent from the current patch', () => { + expect(isHunkHeaderInPatch('@@ -100,2 +100,3 @@', patch)).toBe(false); + }); + + test('does not treat a context line as a hunk header', () => { + expect( + isHunkHeaderInPatch( + '@@ -100,2 +100,3 @@', + ' @@ -100,2 +100,3 @@ example text' + ) + ).toBe(false); + }); + + test('rejects a malformed hunk header', () => { + expect(isHunkHeaderInPatch('not a hunk header', patch)).toBe(false); + }); +}); + +describe('createInlineReviewComment', () => { + test('creates a right-side comment for a hunk in the current patch', () => { + expect( + createInlineReviewComment( + 'src/file.ts', + 'Finding', + '@@ -10,2 +10,3 @@ function example() {', + patch + ) + ).toEqual({ + path: 'src/file.ts', + body: 'Finding', + line: 12, + side: 'RIGHT', + }); + }); + + test('creates a left-side comment for a deletion hunk in the patch', () => { + expect( + createInlineReviewComment( + 'src/file.ts', + 'Finding', + '@@ -20,3 +21,0 @@ function removed() {', + patch + ) + ).toEqual({ + path: 'src/file.ts', + body: 'Finding', + line: 22, + side: 'LEFT', + }); + }); + + test('rejects a hallucinated hunk range', () => { + expect( + createInlineReviewComment( + 'src/file.ts', + 'Finding', + '@@ -100,2 +100,3 @@', + patch + ) + ).toBeNull(); + }); + + test('rejects a missing hunk header', () => { + expect( + createInlineReviewComment('src/file.ts', 'Finding', undefined, patch) + ).toBeNull(); + }); +}); + +describe('createReviewBody', () => { + test('marks a review with inline comments as a ChatGPT review', () => { + expect(createReviewBody(true, [])).toBe( + 'Code review by ChatGPT\n\n' + ); + }); + + test('keeps unpositioned findings in the review body', () => { + expect(createReviewBody(false, ['**File:** `src/file.ts`\n\nFinding'])).toBe( + 'Code review by ChatGPT\n\n**File:** `src/file.ts`\n\nFinding\n\n' + ); + }); + + test('marks a review with no findings as LGTM', () => { + expect(createReviewBody(false, [])).toBe( + 'LGTM 👍\n\n' + ); + }); +}); + +describe('robot', () => { + const githubActions = process.env.GITHUB_ACTIONS; + + afterEach(() => { + if (githubActions === undefined) { + delete process.env.GITHUB_ACTIONS; + } else { + process.env.GITHUB_ACTIONS = githubActions; + } + process.exitCode = undefined; + }); + + test('marks the GitHub Action as failed when a webhook handler fails', () => { + process.env.GITHUB_ACTIONS = 'true'; + const app = { + on: jest.fn(), + onError: jest.fn(), + }; + + robot(app as any); + + expect(app.onError).toHaveBeenCalledTimes(1); + const errorHandler = app.onError.mock.calls[0][0]; + errorHandler(); + expect(process.exitCode).toBe(1); + }); + + test('does not change process error handling outside GitHub Actions', () => { + delete process.env.GITHUB_ACTIONS; + const app = { + on: jest.fn(), + onError: jest.fn(), + }; + + robot(app as any); + + expect(app.onError).not.toHaveBeenCalled(); + expect(process.exitCode).toBeUndefined(); + }); +});