diff --git a/packages/github-scan/src/github-scan/engine/commands/poll.ts b/packages/github-scan/src/github-scan/engine/commands/poll.ts index 259b12b..5457854 100644 --- a/packages/github-scan/src/github-scan/engine/commands/poll.ts +++ b/packages/github-scan/src/github-scan/engine/commands/poll.ts @@ -26,32 +26,21 @@ * surface it here. */ -import { - existsSync, - mkdirSync, - readdirSync, - readFileSync, - rmSync, -} from "node:fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { appendActivityEvent } from "../runtime/activity-log.js"; import { classifyGitHubScanStatus } from "../runtime/classifier.js"; import { loadGitHubScanConfig } from "../runtime/config.js"; -import { GhClient, GhExecError } from "../runtime/gh.js"; +import { GhClient, GhExecError, splitConcatenatedJsonArrays } from "../runtime/gh.js"; + +export { splitConcatenatedJsonArrays }; import { resolveGitHubScanPaths } from "../runtime/paths.js"; import { RepoFilter } from "../runtime/repo-filter.js"; import { updateInbox } from "../runtime/store.js"; import { shouldTrackReason } from "../runtime/task-kind.js"; -import { - parseAllowRepoArg, - requireExplicitRepoFilter, -} from "../runtime/allow-repo.js"; -import { - type GhState, - type Inbox, - type InboxEntry, -} from "../runtime/types.js"; +import { parseAllowRepoArg, requireExplicitRepoFilter } from "../runtime/allow-repo.js"; +import { type GhState, type Inbox, type InboxEntry } from "../runtime/types.js"; export interface PollIO { stdout: (line: string) => void; @@ -175,8 +164,7 @@ export function parseNotifications( const lastActor = item.subject?.latest_comment_url ?? url ?? ""; const updatedAt = item.updated_at ?? ""; const unread = Boolean(item.unread); - const number = - typeof url === "string" ? extractTrailingNumber(url) : null; + const number = typeof url === "string" ? extractTrailingNumber(url) : null; const htmlUrl = htmlUrlFor(host, repo, subjectType, number); seenIds.add(id); entries.push({ @@ -267,13 +255,9 @@ function parseLabelResponse( if (typeof number !== "number") continue; const rawState = node.state; const gh_state: GhState | null = - rawState === "OPEN" || rawState === "CLOSED" || rawState === "MERGED" - ? rawState - : null; + rawState === "OPEN" || rawState === "CLOSED" || rawState === "MERGED" ? rawState : null; const labelNodes = node.labels?.nodes ?? []; - const labels = labelNodes - .map((n) => n?.name) - .filter((n): n is string => typeof n === "string"); + const labels = labelNodes.map((n) => n?.name).filter((n): n is string => typeof n === "string"); rows.push({ number, gh_state, labels }); } return rows; @@ -287,11 +271,7 @@ function parseLabelResponse( * Mutates `entries` in place. Returns a warning string if any repo's * enrichment failed; otherwise `null`. */ -export function enrichWithLabels( - entries: InboxEntry[], - gh: GhClient, - host: string, -): string | null { +export function enrichWithLabels(entries: InboxEntry[], gh: GhClient, host: string): string | null { const byRepo = new Map>(); for (const entry of entries) { if (entry.number === null) continue; @@ -316,10 +296,7 @@ export function enrichWithLabels( const sorted = [...items].sort((a, b) => a.number - b.number); const deduped: Array<{ number: number; isPR: boolean }> = []; for (const item of sorted) { - if ( - deduped.length === 0 || - deduped[deduped.length - 1].number !== item.number - ) { + if (deduped.length === 0 || deduped[deduped.length - 1].number !== item.number) { deduped.push(item); } } @@ -327,14 +304,7 @@ export function enrichWithLabels( for (let i = 0; i < deduped.length; i += LABEL_BATCH_SIZE) { const batch = deduped.slice(i, i + LABEL_BATCH_SIZE); const query = buildLabelQuery(owner, name, batch); - const result = gh.run([ - "api", - "graphql", - "-H", - `GH-Host: ${host}`, - "-f", - `query=${query}`, - ]); + const result = gh.run(["api", "graphql", "-H", `GH-Host: ${host}`, "-f", `query=${query}`]); if (result.status !== 0) { warnings.push(`GraphQL label enrichment for ${repo} failed`); continue; @@ -385,10 +355,7 @@ interface DiffEvent { * - `transition` event when `github_scan_status` changed, EXCEPT * `new → done` (auto-close/merge noise; spec 3 §8) */ -export function diffEvents( - old: Inbox | null, - next: readonly InboxEntry[], -): DiffEvent[] { +export function diffEvents(old: Inbox | null, next: readonly InboxEntry[]): DiffEvent[] { const prevStatuses = new Map(); if (old) { for (const entry of old.notifications) { @@ -415,11 +382,7 @@ export function diffEvents( } /** Remove claim directories whose `claimed_at` is older than `timeoutSecs`. */ -function cleanupExpiredClaims( - claimsDir: string, - timeoutSecs: number, - now: () => Date, -): void { +function cleanupExpiredClaims(claimsDir: string, timeoutSecs: number, now: () => Date): void { if (!existsSync(claimsDir)) return; let dirs: string[]; try { @@ -433,9 +396,7 @@ function cleanupExpiredClaims( if (!existsSync(marker)) continue; try { const contents = readFileSync(marker, "utf-8").trim(); - const m = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})Z$/u.exec( - contents, - ); + const m = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})Z$/u.exec(contents); let claimedMs: number | null = null; if (m) { claimedMs = Date.UTC( @@ -461,56 +422,6 @@ function cleanupExpiredClaims( } } -/** - * Split `gh api --paginate` stdout into individual JSON array pages. - * - * `gh` concatenates paginated arrays as `[...][...][...]` with no - * separator. We walk the string and track bracket depth to carve out each - * top-level array. - */ -export function splitConcatenatedJsonArrays(raw: string): string[] { - const pages: string[] = []; - let depth = 0; - let start = -1; - let inString = false; - let escape = false; - for (let i = 0; i < raw.length; i += 1) { - const ch = raw[i]; - if (inString) { - if (escape) { - escape = false; - } else if (ch === "\\") { - escape = true; - } else if (ch === '"') { - inString = false; - } - continue; - } - if (ch === '"') { - inString = true; - continue; - } - if (ch === "[") { - if (depth === 0) start = i; - depth += 1; - continue; - } - if (ch === "]") { - depth -= 1; - if (depth === 0 && start >= 0) { - pages.push(raw.slice(start, i + 1)); - start = -1; - } - } - } - if (pages.length === 0 && raw.trim().length > 0) { - // Not a recognizable array stream — return the raw text as a single - // page; the parser will drop it if malformed. - pages.push(raw); - } - return pages; -} - /** * Entry point for `first-tree github scan poll`. * @@ -521,10 +432,7 @@ export function splitConcatenatedJsonArrays(raw: string): string[] { * schema validation failure on the existing inbox). */ // oxlint-disable-next-line complexity -export async function runPoll( - argv: readonly string[], - deps: PollDeps = {}, -): Promise { +export async function runPoll(argv: readonly string[], deps: PollDeps = {}): Promise { if (argv[0] === "--help" || argv[0] === "-h" || argv[0] === "help") { const io = deps.io ?? DEFAULT_IO; io.stdout("Usage: first-tree github scan poll"); @@ -541,9 +449,7 @@ export async function runPoll( const paths = deps.paths ?? resolveGitHubScanPaths(); const repoFilter = (() => { const allowRepo = parseAllowRepoArg(argv); - return allowRepo?.trim() - ? requireExplicitRepoFilter(allowRepo) - : RepoFilter.empty(); + return allowRepo?.trim() ? requireExplicitRepoFilter(allowRepo) : RepoFilter.empty(); })(); const gh = deps.gh ?? new GhClient(); const now = deps.now ?? (() => new Date()); @@ -556,9 +462,7 @@ export async function runPoll( const authCheck = gh.run(["auth", "status"]); if (authCheck.status !== 0) { const firstLine = authCheck.stderr.split("\n")[0]?.trim() ?? ""; - io.stderr( - `ERROR: gh not authenticated (run \`gh auth login\`). ${firstLine}`.trim(), - ); + io.stderr(`ERROR: gh not authenticated (run \`gh auth login\`). ${firstLine}`.trim()); return 1; } diff --git a/packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts b/packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts index fa2f68f..a87b1cf 100644 --- a/packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts +++ b/packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts @@ -138,7 +138,18 @@ export function shouldAutoRevertHuman(input: AutoRevertInput): boolean { return false; } -function parseCommentsPage(stdout: string): IssueComment[] | null { +/** + * Parse a single GitHub comment-like REST page into our `IssueComment` + * shape. Shared by the issue-comment, PR-review-body, and PR-inline-review- + * comment fetchers — all three expose the same `user.login` / `body` / + * created-at fields, modulo one quirk: PR review bodies expose the + * timestamp as `submitted_at` rather than `created_at`. Pass the field + * name in `tsField` to handle that case (issue #366). + */ +function parseCommentsPage( + stdout: string, + tsField: "created_at" | "submitted_at" = "created_at", +): IssueComment[] | null { try { const parsed = JSON.parse(stdout) as unknown; if (!Array.isArray(parsed)) return null; @@ -149,10 +160,11 @@ function parseCommentsPage(stdout: string): IssueComment[] | null { user?: { login?: unknown } | null; body?: unknown; created_at?: unknown; + submitted_at?: unknown; }; const login = r.user?.login; const body = r.body; - const createdAt = r.created_at; + const createdAt = tsField === "submitted_at" ? r.submitted_at : r.created_at; if (typeof login !== "string") return null; if (typeof createdAt !== "string") return null; return { @@ -167,6 +179,93 @@ function parseCommentsPage(stdout: string): IssueComment[] | null { } } +/** + * Fetch PR *review bodies* via `GET /repos/{r}/pulls/{n}/reviews` + * (issue #366). + * + * A "review" is the top-level submission a reviewer makes when they + * click APPROVE / REQUEST CHANGES / COMMENT in the PR review UI. The + * `body` field holds the review's text; an APPROVE-with-no-text review + * has an empty body and is correctly filtered by the empty-body guard. + * + * Pagination: walks pages forward (the reviews endpoint does not + * support `direction=desc`, returns reviews oldest-first). No early-exit + * — a recent qualifying review can land on the last page since the + * timeline is oldest-first. Hard cap of {@link AUTO_REVERT_MAX_PAGES} + * pages with a `console.warn` if the cap engages. + * + * Returns `null` on error so the caller can degrade gracefully — a + * failed PR-review fetch should NEVER strip the label. + */ +export function fetchPrReviews(gh: GhClient, repo: string, number: number): IssueComment[] | null { + const all: IssueComment[] = []; + for (let page = 1; page <= AUTO_REVERT_MAX_PAGES; page++) { + const result = gh.run([ + "api", + `/repos/${repo}/pulls/${number}/reviews?per_page=${AUTO_REVERT_PAGE_SIZE}&page=${page}`, + "-H", + "X-GitHub-Api-Version: 2022-11-28", + ]); + if (result.status !== 0) return null; + const pageReviews = parseCommentsPage(result.stdout, "submitted_at"); + if (pageReviews === null) return null; + + if (pageReviews.length === 0) return all; + all.push(...pageReviews); + + if (pageReviews.length < AUTO_REVERT_PAGE_SIZE) return all; + + if (page === AUTO_REVERT_MAX_PAGES) { + console.warn( + `auto-revert: PR reviews fetch hit ${AUTO_REVERT_MAX_PAGES}-page cap for ${repo}#${number}; older reviews may be ignored`, + ); + } + } + return all; +} + +/** + * Fetch PR *inline review comments* via + * `GET /repos/{r}/pulls/{n}/comments` (the line-by-line file comments + * in the diff view, not the conversation tab). Issue #366. + * + * Pagination: same forward-walking pattern as {@link fetchPrReviews}, + * with the same {@link AUTO_REVERT_MAX_PAGES} cap and warning. + * + * Returns `null` on error so the caller can degrade gracefully — a + * failed fetch should NEVER strip the label. + */ +export function fetchPrReviewComments( + gh: GhClient, + repo: string, + number: number, +): IssueComment[] | null { + const all: IssueComment[] = []; + for (let page = 1; page <= AUTO_REVERT_MAX_PAGES; page++) { + const result = gh.run([ + "api", + `/repos/${repo}/pulls/${number}/comments?per_page=${AUTO_REVERT_PAGE_SIZE}&page=${page}`, + "-H", + "X-GitHub-Api-Version: 2022-11-28", + ]); + if (result.status !== 0) return null; + const pageComments = parseCommentsPage(result.stdout); + if (pageComments === null) return null; + + if (pageComments.length === 0) return all; + all.push(...pageComments); + + if (pageComments.length < AUTO_REVERT_PAGE_SIZE) return all; + + if (page === AUTO_REVERT_MAX_PAGES) { + console.warn( + `auto-revert: PR review comments fetch hit ${AUTO_REVERT_MAX_PAGES}-page cap for ${repo}#${number}; older comments may be ignored`, + ); + } + } + return all; +} + /** * Fetch comments for an issue/PR (number) via the REST API. * @@ -397,13 +496,23 @@ export function fetchHumanLabelAppliedAt( export interface AutoRevertDeps { gh: GhClient; agentLogin: string; - /** Test seam: override comment-fetch (default uses `fetchIssueComments`). */ + /** Test seam: override issue-comment fetch (default uses `fetchIssueComments`). */ fetchComments?: ( gh: GhClient, repo: string, number: number, labelAppliedAt?: string, ) => IssueComment[] | null; + /** + * Test seam: override PR review-body fetch (default uses `fetchPrReviews`). + * Only invoked for entries whose `type === "PullRequest"`. Issue #366. + */ + fetchPrReviews?: (gh: GhClient, repo: string, number: number) => IssueComment[] | null; + /** + * Test seam: override PR inline review-comment fetch (default uses + * `fetchPrReviewComments`). Only invoked for PR entries. Issue #366. + */ + fetchPrReviewComments?: (gh: GhClient, repo: string, number: number) => IssueComment[] | null; /** Test seam: override timeline-fetch. */ fetchLabelAppliedAt?: (gh: GhClient, repo: string, number: number) => string | null; /** Test seam: override PR-commits fetch (default uses `fetchPrCommits`). Issue #383. */ @@ -439,6 +548,8 @@ export function autoRevertHumanLabels( deps: AutoRevertDeps, ): AutoRevertOutcome { const fetchComments = deps.fetchComments ?? fetchIssueComments; + const fetchReviews = deps.fetchPrReviews ?? fetchPrReviews; + const fetchReviewComments = deps.fetchPrReviewComments ?? fetchPrReviewComments; const fetchLabelAppliedAt = deps.fetchLabelAppliedAt ?? fetchHumanLabelAppliedAt; const fetchCommits = deps.fetchCommits ?? fetchPrCommits; @@ -457,23 +568,47 @@ export function autoRevertHumanLabels( continue; } - const comments = fetchComments(deps.gh, entry.repo, entry.number, labelAppliedAt); - if (!comments) { + // Always scan issue comments (works for both Issues and PRs — PRs + // are issues for the conversation API). + const issueComments = fetchComments(deps.gh, entry.repo, entry.number, labelAppliedAt); + if (!issueComments) { warnings.push(`auto-revert: comment fetch failed for ${entry.repo}#${entry.number}`); continue; } + // Issue #366: for PR entries, also scan PR review bodies and PR + // inline review comments. A failed sub-fetch is treated as a soft + // warning so a successful issue-comment fetch still gets a chance + // to fire. + // // Issue #383: PR entries also qualify on author push. Issues do not - // have commits, so we skip the fetch entirely for non-PRs to avoid - // a wasted (404) round-trip. + // have commits, so we skip the commits fetch entirely for non-PRs + // to avoid a wasted (404) round-trip. + let comments: IssueComment[] = [...issueComments]; let commits: readonly PrCommit[] = []; if (entry.type === "PullRequest") { - const fetched = fetchCommits(deps.gh, entry.repo, entry.number); - if (!fetched) { + const reviews = fetchReviews(deps.gh, entry.repo, entry.number); + if (reviews === null) { + warnings.push(`auto-revert: PR reviews fetch failed for ${entry.repo}#${entry.number}`); + } else { + comments = comments.concat(reviews); + } + + const reviewComments = fetchReviewComments(deps.gh, entry.repo, entry.number); + if (reviewComments === null) { + warnings.push( + `auto-revert: PR review comments fetch failed for ${entry.repo}#${entry.number}`, + ); + } else { + comments = comments.concat(reviewComments); + } + + const fetchedCommits = fetchCommits(deps.gh, entry.repo, entry.number); + if (!fetchedCommits) { warnings.push(`auto-revert: PR commits fetch failed for ${entry.repo}#${entry.number}`); continue; } - commits = fetched; + commits = fetchedCommits; } const decide = shouldAutoRevertHuman({ diff --git a/packages/github-scan/src/github-scan/engine/runtime/gh.ts b/packages/github-scan/src/github-scan/engine/runtime/gh.ts index 77c4c83..7ec4736 100644 --- a/packages/github-scan/src/github-scan/engine/runtime/gh.ts +++ b/packages/github-scan/src/github-scan/engine/runtime/gh.ts @@ -185,3 +185,53 @@ export class GhClient { ]); } } + +/** + * Split `gh api --paginate` stdout into individual JSON array pages. + * + * `gh` concatenates paginated arrays as `[...][...][...]` with no + * separator. We walk the string and track bracket depth to carve out each + * top-level array. + */ +export function splitConcatenatedJsonArrays(raw: string): string[] { + const pages: string[] = []; + let depth = 0; + let start = -1; + let inString = false; + let escape = false; + for (let i = 0; i < raw.length; i += 1) { + const ch = raw[i]; + if (inString) { + if (escape) { + escape = false; + } else if (ch === "\\") { + escape = true; + } else if (ch === '"') { + inString = false; + } + continue; + } + if (ch === '"') { + inString = true; + continue; + } + if (ch === "[") { + if (depth === 0) start = i; + depth += 1; + continue; + } + if (ch === "]") { + depth -= 1; + if (depth === 0 && start >= 0) { + pages.push(raw.slice(start, i + 1)); + start = -1; + } + } + } + if (pages.length === 0 && raw.trim().length > 0) { + // Not a recognizable array stream — return the raw text as a single + // page; the parser will drop it if malformed. + pages.push(raw); + } + return pages; +} diff --git a/packages/github-scan/tests/github-scan/github-scan-auto-revert.test.ts b/packages/github-scan/tests/github-scan/github-scan-auto-revert.test.ts index 3287269..63e52c0 100644 --- a/packages/github-scan/tests/github-scan/github-scan-auto-revert.test.ts +++ b/packages/github-scan/tests/github-scan/github-scan-auto-revert.test.ts @@ -26,6 +26,8 @@ import { fetchHumanLabelAppliedAt, fetchIssueComments, fetchPrCommits, + fetchPrReviewComments, + fetchPrReviews, shouldAutoRevertHuman, type IssueComment, type PrCommit, @@ -758,6 +760,8 @@ describe("autoRevertHumanLabels — commit-driven revert (issue #383)", () => { agentLogin: AGENT, fetchLabelAppliedAt: () => LABEL_TS, fetchComments: () => [], + fetchPrReviews: () => [], + fetchPrReviewComments: () => [], fetchCommits: () => [{ author: "alice", committedAt: "2026-04-30T11:00:00Z" }], }); @@ -799,6 +803,8 @@ describe("autoRevertHumanLabels — commit-driven revert (issue #383)", () => { agentLogin: AGENT, fetchLabelAppliedAt: () => LABEL_TS, fetchComments: () => [], + fetchPrReviews: () => [], + fetchPrReviewComments: () => [], fetchCommits: () => [{ author: AGENT, committedAt: "2026-04-30T11:00:00Z" }], }); @@ -817,6 +823,8 @@ describe("autoRevertHumanLabels — commit-driven revert (issue #383)", () => { agentLogin: AGENT, fetchLabelAppliedAt: () => LABEL_TS, fetchComments: () => [], + fetchPrReviews: () => [], + fetchPrReviewComments: () => [], fetchCommits: () => null, }); @@ -918,3 +926,231 @@ describe("fetchPrCommits — pagination (issue #383)", () => { expect(fetchPrCommits(gh, "o/r", 1)).toBeNull(); }); }); + +/** + * Issue #366: auto-revert must also inspect PR review bodies + * (`/pulls/{n}/reviews`) and PR inline review comments + * (`/pulls/{n}/comments`) for PR entries — that's the dominant reply + * surface for code review and was previously invisible. + */ +describe("autoRevertHumanLabels — PR review surfaces (issue #366)", () => { + function makePrEntry(overrides: Partial = {}): InboxEntry { + return makeEntry({ + type: "PullRequest", + url: "https://api.github.com/repos/agent-team-foundation/first-tree/pulls/42", + html_url: "https://github.com/agent-team-foundation/first-tree/pull/42", + ...overrides, + }); + } + + const QUALIFYING = "Please proceed with this approach — confirmed via #366 e2e test."; + + it("PR review body by non-agent, after label, triggers revert", () => { + const removeLabel = vi.fn().mockReturnValue(true); + const stubGh = { removeLabel } as unknown as GhClient; + const entry = makePrEntry(); + + const out = autoRevertHumanLabels([entry], { + gh: stubGh, + agentLogin: AGENT, + fetchLabelAppliedAt: () => LABEL_TS, + fetchComments: () => [], + fetchPrReviews: () => [ + { author: "alice", body: QUALIFYING, createdAt: "2026-04-30T11:00:00Z" }, + ], + fetchPrReviewComments: () => [], + fetchCommits: () => [], + }); + + expect(removeLabel).toHaveBeenCalledTimes(1); + expect(entry.labels).not.toContain("github-scan:human"); + expect(out.reverted).toEqual(["n-1"]); + }); + + it("PR inline review comment by non-agent, after label, triggers revert", () => { + const removeLabel = vi.fn().mockReturnValue(true); + const stubGh = { removeLabel } as unknown as GhClient; + const entry = makePrEntry(); + + const out = autoRevertHumanLabels([entry], { + gh: stubGh, + agentLogin: AGENT, + fetchLabelAppliedAt: () => LABEL_TS, + fetchComments: () => [], + fetchPrReviews: () => [], + fetchPrReviewComments: () => [ + { + author: "alice", + body: "On line 42: this branch needs a guard for the null case before merging.", + createdAt: "2026-04-30T11:05:00Z", + }, + ], + fetchCommits: () => [], + }); + + expect(removeLabel).toHaveBeenCalledTimes(1); + expect(entry.labels).not.toContain("github-scan:human"); + expect(out.reverted).toEqual(["n-1"]); + }); + + it("PR review body by the agent itself does NOT trigger revert", () => { + const removeLabel = vi.fn().mockReturnValue(true); + const stubGh = { removeLabel } as unknown as GhClient; + const entry = makePrEntry(); + + autoRevertHumanLabels([entry], { + gh: stubGh, + agentLogin: AGENT, + fetchLabelAppliedAt: () => LABEL_TS, + fetchComments: () => [], + fetchPrReviews: () => [ + { author: AGENT, body: QUALIFYING, createdAt: "2026-04-30T11:00:00Z" }, + ], + fetchPrReviewComments: () => [], + fetchCommits: () => [], + }); + + expect(removeLabel).not.toHaveBeenCalled(); + expect(entry.labels).toContain("github-scan:human"); + }); + + it("Issue entries do NOT fetch PR review surfaces (skipped for non-PR)", () => { + const removeLabel = vi.fn().mockReturnValue(true); + const stubGh = { removeLabel } as unknown as GhClient; + const entry = makeEntry({ type: "Issue" }); + const reviewSpy = vi.fn(() => []); + const reviewCommentSpy = vi.fn(() => []); + const commitSpy = vi.fn(() => []); + + autoRevertHumanLabels([entry], { + gh: stubGh, + agentLogin: AGENT, + fetchLabelAppliedAt: () => LABEL_TS, + fetchComments: () => [], + fetchPrReviews: reviewSpy, + fetchPrReviewComments: reviewCommentSpy, + fetchCommits: commitSpy, + }); + + expect(reviewSpy).not.toHaveBeenCalled(); + expect(reviewCommentSpy).not.toHaveBeenCalled(); + expect(commitSpy).not.toHaveBeenCalled(); + }); + + it("PR reviews fetch failure: warns but still allows revert via other surfaces", () => { + const removeLabel = vi.fn().mockReturnValue(true); + const stubGh = { removeLabel } as unknown as GhClient; + const entry = makePrEntry(); + + const out = autoRevertHumanLabels([entry], { + gh: stubGh, + agentLogin: AGENT, + fetchLabelAppliedAt: () => LABEL_TS, + fetchComments: () => [ + { author: "alice", body: QUALIFYING, createdAt: "2026-04-30T11:00:00Z" }, + ], + fetchPrReviews: () => null, + fetchPrReviewComments: () => [], + fetchCommits: () => [], + }); + + expect(removeLabel).toHaveBeenCalledTimes(1); + expect(out.reverted).toEqual(["n-1"]); + expect(out.warnings.some((w) => w.includes("PR reviews fetch failed"))).toBe(true); + }); +}); + +describe("fetchPrReviews / fetchPrReviewComments — pagination (issue #366)", () => { + function reviewRaw(login: string, body: string, submittedAt: string): unknown { + return { user: { login }, body, submitted_at: submittedAt }; + } + function commentRaw(login: string, body: string, createdAt: string): unknown { + return { user: { login }, body, created_at: createdAt }; + } + + function makeRunStub(pages: readonly unknown[][]) { + const state = { calls: 0, requestedPages: [] as number[] }; + const run = (args: readonly string[]): GhExecResult => { + state.calls++; + const url = args[1] ?? ""; + const match = /[?&]page=(\d+)/.exec(url); + const page = match ? Number.parseInt(match[1]!, 10) : 1; + state.requestedPages.push(page); + const body = pages[page - 1] ?? []; + return { status: 0, stdout: JSON.stringify(body), stderr: "" }; + }; + return { run, state }; + } + + it("fetchPrReviews: review on page 2 is found (forward pagination, no early-exit)", () => { + const fullPage1 = Array.from({ length: AUTO_REVERT_PAGE_SIZE }, (_, i) => + reviewRaw("u", `early review ${i}`, `2026-04-01T00:${String(i % 60).padStart(2, "0")}:00Z`), + ); + const shortPage2 = [reviewRaw("alice", "Looks good, please proceed.", "2026-04-30T11:00:00Z")]; + const stub = makeRunStub([fullPage1, shortPage2]); + const gh = { run: stub.run } as unknown as GhClient; + + const out = fetchPrReviews(gh, "o/r", 42); + expect(out).not.toBeNull(); + expect(out!.length).toBe(AUTO_REVERT_PAGE_SIZE + 1); + expect(stub.state.requestedPages).toEqual([1, 2]); + // The qualifying review was correctly decoded from `submitted_at`. + expect(out![out!.length - 1]).toEqual({ + author: "alice", + body: "Looks good, please proceed.", + createdAt: "2026-04-30T11:00:00Z", + }); + }); + + it("fetchPrReviewComments: comment on page 2 is found", () => { + const fullPage1 = Array.from({ length: AUTO_REVERT_PAGE_SIZE }, (_, i) => + commentRaw("u", `early ${i}`, `2026-04-01T00:${String(i % 60).padStart(2, "0")}:00Z`), + ); + const shortPage2 = [ + commentRaw("alice", "On line 42: please add a null guard here.", "2026-04-30T11:05:00Z"), + ]; + const stub = makeRunStub([fullPage1, shortPage2]); + const gh = { run: stub.run } as unknown as GhClient; + + const out = fetchPrReviewComments(gh, "o/r", 42); + expect(out).not.toBeNull(); + expect(out!.length).toBe(AUTO_REVERT_PAGE_SIZE + 1); + expect(stub.state.requestedPages).toEqual([1, 2]); + }); + + it("fetchPrReviews: hard-cap engages on > MAX_PAGES full pages and emits a console.warn", () => { + const fullPage = Array.from({ length: AUTO_REVERT_PAGE_SIZE }, (_, i) => + reviewRaw("u", `r${i}`, "2026-04-01T00:00:00Z"), + ); + let calls = 0; + const gh = { + run: () => { + calls++; + return { status: 0, stdout: JSON.stringify(fullPage), stderr: "" }; + }, + } as unknown as GhClient; + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + fetchPrReviews(gh, "o/r", 1); + expect(calls).toBe(AUTO_REVERT_MAX_PAGES); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0]![0]).toMatch(/cap/); + } finally { + warnSpy.mockRestore(); + } + }); + + it("fetchPrReviews / fetchPrReviewComments: non-zero gh exit returns null", () => { + const gh = { + run: () => ({ status: 1, stdout: "", stderr: "boom" }), + } as unknown as GhClient; + expect(fetchPrReviews(gh, "o/r", 1)).toBeNull(); + expect(fetchPrReviewComments(gh, "o/r", 1)).toBeNull(); + }); + + // Note: timeline pagination for `fetchHumanLabelAppliedAt` is covered + // by the dedicated `fetchHumanLabelAppliedAt — pagination (issue #365)` + // describe block above (forward-pagination + AUTO_REVERT_MAX_PAGES cap), + // including the page-2 regression case bingran-you flagged on PR #369. +});