diff --git a/.sandcastle/issue-source.ts b/.sandcastle/issue-source.ts index 5175a67..6b33674 100644 --- a/.sandcastle/issue-source.ts +++ b/.sandcastle/issue-source.ts @@ -12,6 +12,20 @@ import { READY_LABEL, type Issue, type ReviewOutput } from "./reduce.ts"; const exec = promisify(execFile); +/** + * Render a structured review verdict as plain markdown, for postPrReview's + * final fallback (a plain PR comment) when GitHub's formal review API is + * unavailable (self-review restriction — see postPrReview). + */ +export function formatReviewAsComment(output: ReviewOutput): string { + const header = `**AI review: ${output.verdict}**\n\n${output.summary}`; + if (output.comments.length === 0) return header; + const commentsBlock = output.comments + .map((c) => `**${c.path}:${c.line}** — ${c.body}`) + .join("\n\n"); + return `${header}\n\n---\n${commentsBlock}`; +} + /** Parse "Blocked by #N" references from an issue body (one or many). */ export function parseBlockedBy(body: string): number[] { const idx = body.search(/blocked\s+by\b/i); @@ -228,7 +242,16 @@ export class IssueSource { /** * Post a REQUEST_CHANGES review on the PR for the given issue. * Uses the GitHub API to attach the structured verdict + filtered inline comments. - * Best-effort: if inline-comment posting fails, falls back to a plain PR comment. + * Best-effort, two-stage fallback: + * 1. inline comments may fail when lines are no longer in the diff — retry + * as a plain top-level review (no inline comments). + * 2. GitHub rejects REQUEST_CHANGES reviews from a PR's OWN AUTHOR + * ("Review Can not request changes on your own pull request", HTTP 422) + * — the normal case here, since this orchestrator's GH_TOKEN both opens + * the PR and posts its AI review. A plain PR comment carries the same + * feedback without that restriction, so it is the final fallback rather + * than letting the verdict go unposted (confirmed live: both the inline + * and body-only review attempts hit the identical 422 in that case). */ async postPrReview(issueId: number, output: ReviewOutput): Promise { const { stdout: prJson } = await this.gh([ @@ -254,16 +277,23 @@ export class IssueSource { ["api", `repos/{owner}/{repo}/pulls/${prInfo.number}/reviews`, "--method", "POST", "--input", "-"], JSON.stringify(reviewBody), ); + return; } catch (err) { - // Inline comments may fail when lines are no longer in the diff; fall back to a - // plain top-level review without inline comments so the verdict is still visible. console.warn(`[issue-source] inline review post failed — falling back to body-only review:`, err); + } + + try { const bodyOnly = { ...reviewBody, comments: [] }; await this.ghWithInput( ["api", `repos/{owner}/{repo}/pulls/${prInfo.number}/reviews`, "--method", "POST", "--input", "-"], JSON.stringify(bodyOnly), ); + return; + } catch (err) { + console.warn(`[issue-source] body-only review post also failed — falling back to a plain PR comment:`, err); } + + await this.comment(issueId, formatReviewAsComment(output)); } /** Open a PR for an already-pushed head branch. Returns the PR URL. */ diff --git a/.sandcastle/reduce.test.ts b/.sandcastle/reduce.test.ts index 64a6709..711e49e 100644 --- a/.sandcastle/reduce.test.ts +++ b/.sandcastle/reduce.test.ts @@ -9,7 +9,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { reduce, READY_LABEL, IN_PROGRESS_LABEL, IN_REVIEW_LABEL, reviewVerdict, reconcileFromLabels, type State, type CiStatus, type Pr, type ReviewOutput } from "./reduce.ts"; -import { parseBlockedBy } from "./issue-source.ts"; +import { parseBlockedBy, formatReviewAsComment } from "./issue-source.ts"; import { sweepOrphanedSandboxes, ensureSandboxNetwork, parseConcurrency, withRetry, resetAgentBranch, refreshBase, validateSignature, classifyDelivery, parseSmeeEvent, parseOrchEnv, resolveCredentials, resolveRunMode, resolveDockerHost } from "./main.ts"; import { createHmac } from "node:crypto"; import { SANDBOX_LABEL, PROJECT_LABEL_KEY, deriveProject } from "./sandbox-runner.ts"; @@ -105,6 +105,40 @@ test("parseBlockedBy: stops at next markdown section", () => { ); }); +// ─── formatReviewAsComment ─────────────────────────────────────────────────── + +test("formatReviewAsComment: no comments — just verdict and summary", () => { + const rendered = formatReviewAsComment({ verdict: "pass", summary: "Looks good.", comments: [] }); + assert.equal(rendered, "**AI review: pass**\n\nLooks good."); +}); + +test("formatReviewAsComment: includes each comment with path:line", () => { + const rendered = formatReviewAsComment({ + verdict: "changes-requested", + summary: "One issue found.", + comments: [{ path: "src/foo.ts", line: 42, body: "This is wrong." }], + }); + assert.equal( + rendered, + "**AI review: changes-requested**\n\nOne issue found.\n\n---\n**src/foo.ts:42** — This is wrong.", + ); +}); + +test("formatReviewAsComment: multiple comments joined with blank lines", () => { + const rendered = formatReviewAsComment({ + verdict: "changes-requested", + summary: "Two issues.", + comments: [ + { path: "a.ts", line: 1, body: "first" }, + { path: "b.ts", line: 2, body: "second" }, + ], + }); + assert.equal( + rendered, + "**AI review: changes-requested**\n\nTwo issues.\n\n---\n**a.ts:1** — first\n\n**b.ts:2** — second", + ); +}); + // ─── Dependency blocking (isReady + Tick) ──────────────────────────────────── test("issue with no blockers (blockedBy: []) behaves as before", () => {