diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..3c3629e --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +node_modules diff --git a/AGENTS.md b/AGENTS.md index b007e62..55e54ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,7 +36,8 @@ src/ github/ notifications.ts # listNotifications() — paginated /notifications API pr.ts # parsePullRequest(), subjectUrlToWebUrl() - reviews.ts # postGithubReview() — createReview with inline comments + diff.ts # getCommentableLines() — RIGHT-side lines that accept comments + reviews.ts # postGithubReview() — validate vs diff, then createReview review/ process.ts # processReviewRequest() — orchestrates full review flow @@ -61,7 +62,7 @@ cli/notifications.ts **Approve vs request changes:** `critical` or `high` findings → `REQUEST_CHANGES`; otherwise `APPROVE`. -**Inline comments:** findings with `path` + `line` become review comments (`side: RIGHT`). Unanchored findings go in the review body. If GitHub rejects inline comments (422), falls back to summary-only. +**Inline comments:** findings with `path` + `line` become review comments (`side: RIGHT`). The agent reviews the whole repo, so it can cite lines outside the diff — but GitHub only accepts RIGHT-side comments on lines present in the PR diff, and one bad anchor 422s the *entire* inline batch. So before posting, `github/diff.ts` → `getCommentableLines()` parses the PR diff hunks and `reviews.ts` filters comments against it: anchorable lines post inline, the rest are demoted into the review body (`appendCommentsToBody`). Findings without `path`/`line` go in the body too. A body-only 422 fallback remains as a last resort. ## Key extension points diff --git a/src/cli/notifications.ts b/src/cli/notifications.ts index cbb6394..42f90b7 100644 --- a/src/cli/notifications.ts +++ b/src/cli/notifications.ts @@ -1,5 +1,8 @@ import { RequestError } from "@octokit/request-error"; -import { listNotifications } from "../github/notifications.js"; +import { + listNotifications, + markNotificationDone, +} from "../github/notifications.js"; import { parsePullRequest, subjectUrlToWebUrl } from "../github/pr.js"; import { processReviewRequest } from "../review/process.js"; @@ -43,7 +46,10 @@ try { console.log(` ${subjectUrlToWebUrl(n.subject.url)}`); } console.log(); - await processReviewRequest(n, { githubToken: token, cursorApiKey }); + const ok = await processReviewRequest(n, { githubToken: token, cursorApiKey }); + if (ok) { + await markNotificationDone(token, n.id); + } } console.log(`${notifications.length} notification(s)`); diff --git a/src/github/diff.ts b/src/github/diff.ts new file mode 100644 index 0000000..e525201 --- /dev/null +++ b/src/github/diff.ts @@ -0,0 +1,59 @@ +import * as github from "@actions/github"; +import type { PullRequestRef } from "../types.js"; + +type Octokit = ReturnType; + +/** + * RIGHT-side line numbers GitHub will accept a review comment on, per file. + * A line is commentable if it appears in the PR diff as an added (`+`) or + * context (` `) line — deleted lines live on the LEFT side and are excluded. + */ +export type CommentableLines = Map>; + +export async function getCommentableLines( + octokit: Octokit, + pr: Pick, +): Promise { + const files = await octokit.paginate(octokit.rest.pulls.listFiles, { + owner: pr.owner, + repo: pr.repo, + pull_number: pr.prNumber, + per_page: 100, + }); + + const map: CommentableLines = new Map(); + for (const file of files) { + if (!file.patch) continue; + map.set(file.filename, rightSideLinesFromPatch(file.patch)); + } + return map; +} + +/** Parse a unified-diff patch and collect RIGHT-side (new file) line numbers. */ +function rightSideLinesFromPatch(patch: string): Set { + const lines = new Set(); + let newLine = 0; + + for (const raw of patch.split("\n")) { + const hunk = raw.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/); + if (hunk) { + newLine = Number(hunk[1]); + continue; + } + + if (raw.startsWith("\\")) continue; // "\ No newline at end of file" + + if (raw.startsWith("+")) { + lines.add(newLine); + newLine++; + } else if (raw.startsWith("-")) { + // deletion: LEFT side only, does not advance the new-file cursor + } else { + // context line: present on the RIGHT side and commentable + lines.add(newLine); + newLine++; + } + } + + return lines; +} diff --git a/src/github/notifications.ts b/src/github/notifications.ts index d67205f..36bee80 100644 --- a/src/github/notifications.ts +++ b/src/github/notifications.ts @@ -13,3 +13,14 @@ export async function listNotifications( ); return { login: user.login, notifications }; } + +/** Mark a thread as done, removing it from the inbox so it won't be re-processed. */ +export async function markNotificationDone( + token: string, + threadId: string, +): Promise { + const octokit = github.getOctokit(token); + await octokit.rest.activity.markThreadAsDone({ + thread_id: Number(threadId), + }); +} diff --git a/src/github/reviews.ts b/src/github/reviews.ts index 08c43b0..19ef4d8 100644 --- a/src/github/reviews.ts +++ b/src/github/reviews.ts @@ -5,6 +5,9 @@ import { buildGithubReview, } from "../review/payload.js"; import type { PullRequestRef } from "../types.js"; +import { getCommentableLines } from "./diff.js"; + +type ReviewComment = ReturnType["comments"][number]; export async function postGithubReview( githubToken: string, @@ -18,37 +21,69 @@ export async function postGithubReview( pull_number: pr.prNumber, }); + // Only lines that actually appear in the PR diff are commentable; anything + // else 422s and would poison the whole inline batch. Split accordingly. + const commentable = await getCommentableLines(octokit, pr); + const { anchored, demoted } = splitByCommentable(review.comments, commentable); + + if (demoted.length > 0) { + console.error( + `${demoted.length} comment(s) not on the diff — moved to review body`, + ); + } + + const body = + demoted.length > 0 ? appendCommentsToBody(review.body, demoted) : review.body; + const baseParams = { owner: pr.owner, repo: pr.repo, pull_number: pr.prNumber, commit_id: pull.head.sha, event: review.event, - body: review.body, + body, }; try { const { data } = await octokit.rest.pulls.createReview({ ...baseParams, - comments: review.comments.length > 0 ? review.comments : undefined, + comments: anchored.length > 0 ? anchored : undefined, }); - console.log(`Posted ${review.event} review: ${data.html_url}`); + console.log( + `Posted ${review.event} review (${anchored.length} inline): ${data.html_url}`, + ); return; } catch (err) { - if ( - !(err instanceof RequestError) || - err.status !== 422 || - review.comments.length === 0 - ) { + if (!(err instanceof RequestError) || err.status !== 422 || anchored.length === 0) { throw err; } - console.error("Inline comments rejected, posting summary only..."); - const body = appendCommentsToBody(review.body, review.comments); + // Safety net: validation should make this unreachable, but if GitHub still + // rejects the inline batch, fall back to a body-only review rather than lose + // the findings entirely. + console.error("Inline batch still rejected, posting body-only..."); const { data } = await octokit.rest.pulls.createReview({ ...baseParams, - body, + body: appendCommentsToBody(body, anchored), }); console.log(`Posted ${review.event} review (no inline): ${data.html_url}`); } } + +function splitByCommentable( + comments: ReviewComment[], + commentable: Map>, +): { anchored: ReviewComment[]; demoted: ReviewComment[] } { + const anchored: ReviewComment[] = []; + const demoted: ReviewComment[] = []; + + for (const comment of comments) { + if (commentable.get(comment.path)?.has(comment.line)) { + anchored.push(comment); + } else { + demoted.push(comment); + } + } + + return { anchored, demoted }; +} diff --git a/src/review/payload.ts b/src/review/payload.ts index c58f650..e79d950 100644 --- a/src/review/payload.ts +++ b/src/review/payload.ts @@ -3,9 +3,6 @@ export const SEVERITIES = [ "high", "medium", "low", - "warning", - "suggestion", - "info", ] as const; export type Severity = (typeof SEVERITIES)[number]; diff --git a/src/review/process.ts b/src/review/process.ts index c50288e..0ced02b 100644 --- a/src/review/process.ts +++ b/src/review/process.ts @@ -7,14 +7,15 @@ import type { NotificationThread } from "../types.js"; import { runAgentReview } from "./agent.js"; import { buildGithubReview } from "./payload.js"; +/** Returns true when the PR was reviewed and posted successfully. */ export async function processReviewRequest( notification: NotificationThread, options: { githubToken: string; cursorApiKey: string }, -): Promise { +): Promise { const pr = parsePullRequest(notification); if (!pr) { console.error("Skipping: not a pull request notification"); - return; + return false; } let workDir: string | undefined; @@ -36,23 +37,20 @@ export async function processReviewRequest( options.cursorApiKey, ); - console.log('payload=', payload); - return; - const githubReview = buildGithubReview(payload); console.log(`Verdict: ${githubReview.event}`); - console.log(`${githubReview.comments.length} inline comment(s)\n`); - console.log(githubReview.body); + console.log(`${githubReview.comments.length} inline comment(s)`); await postGithubReview(options.githubToken, pr, githubReview); + return true; } catch (err) { if (err instanceof CursorAgentError) { console.error(`Review startup failed: ${err.message}`); - return; + return false; } if (err instanceof Error) { console.error(err.message); - return; + return false; } throw err; } finally {