From d82c4495a69216747687f730e6ee3508062ebfa0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 09:05:30 -0700 Subject: [PATCH] feat(review): add a combined "Copy for AI agents" block to the PR comment Mirrors CodeRabbit's combined "Prompt for AI agents" feature (per their own docs: gathers every fix prompt from a review into ONE structured instruction instead of one per finding, specifically to cut the repeated copy-paste that produced). Right after "Why this is blocked" / "Concerns raised", render one collapsible containing every blocker as a numbered plain-text instruction inside a fenced code block -- GitHub renders a copy icon on the fence for free, no custom JS needed. Uses the FULL (pre-display-truncation) blocker set, not the human- facing capped subset shown above it, since an agent benefits from complete context. Never gated by comment_verbosity: quiet -- it extends the never-gated blockers themselves, not decorative detail like Nits. --- src/review/unified-comment.ts | 16 +++++++ test/unit/unified-comment-bridge.test.ts | 23 +++++---- test/unit/unified-comment.test.ts | 60 +++++++++++++++++++++++- 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index df66fecbe6..e080ddb5cf 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -477,6 +477,18 @@ function taskList(items: string[]): string { .join("\n"); } +/** A single copy-paste-ready plain-text prompt combining every blocker into one instruction an AI coding + * agent can act on directly โ€” mirrors CodeRabbit's combined "Prompt for AI agents" feature (per their own + * docs: gathers every fix prompt from a review into ONE structured instruction instead of one per finding, + * specifically to cut the repeated copy-paste this produced before). A GitHub-rendered fenced code block + * gets its own copy icon for free โ€” no custom JS needed, just plain text inside the fence. The sole caller + * only invokes this inside its own `blockersAll.length` guard, so an empty list never reaches here. */ +function buildAiContextBlock(blockers: string[], open: boolean): string { + const items = blockers.map((line, i) => `${i + 1}. ${line}`).join("\n\n"); + const body = "```\nFix the following blocker(s) from this PR review:\n\n" + items + "\n```"; + return details("๐Ÿ“‹ Copy for AI agents", body, "paste into your coding agent", open); +} + function actionReasonBullets(reason: string): string { const reasons = reason .split(/[;\n]+/) @@ -661,6 +673,10 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi ? appendMoreFooter(bullets(blockersTrunc.shown), blockersTrunc.hiddenCount) : `_+${blockersTrunc.hiddenCount} more_`; blocks.push(`**${heading}**\n${blockersBody}`); + // The FULL (pre-display-truncation) blocker set, not blockersTrunc.shown -- an AI agent benefits from + // every blocker, not just the human-scannable capped subset shown above. Never gated by verbosity: this + // is an extension of the blockers themselves (never gated), not decorative detail like Nits. + blocks.push(buildAiContextBlock(blockersAll, collapsiblesOpen)); } // Category breakdown (#2150): a compact, deterministic one-liner of the finding mix (e.g. "2 correctness ยท diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts index 0e697eadb8..430b77d7ba 100644 --- a/test/unit/unified-comment-bridge.test.ts +++ b/test/unit/unified-comment-bridge.test.ts @@ -714,10 +714,11 @@ describe("single AI pass: the bridge RECOVERS the consensus defect, never re-der changedFiles: 4, footerMarkdown: footer, }); - // The defect title appears EXACTLY ONCE in the whole comment (the Code-review blocker bullet), never - // duplicated into the gate signal row (which only renders the conclusion-derived "Blocking" status text). + // The defect title appears in the Code-review blocker bullet AND once more inside the "Copy for AI + // agents" block (a deliberate copyable rendition) -- but never a THIRD time duplicated into the gate + // signal row (which only renders the conclusion-derived "Blocking" status text). const occurrences = body.split(defectTitle).length - 1; - expect(occurrences, "consensus defect title must appear exactly once").toBe(1); + expect(occurrences, "consensus defect title must appear exactly twice (blocker bullet + AI-context block)").toBe(2); // It is rendered under the blocked-reasons heading (the Code-review side), confirming where the one copy lives. expect(body).toMatch(/Why this is blocked|Concerns raised/); }); @@ -774,8 +775,10 @@ describe("gate blockers render in 'Why this is blocked' (FIX D1)", () => { changedFiles: 3, footerMarkdown: footer, }); - // The defect surfaces exactly once (recovered via consensusDefect; excluded from the folded gate blockers). - expect(body.split(title).length - 1).toBe(1); + // The defect surfaces as ONE blocker (recovered via consensusDefect; excluded from the folded gate + // blockers so it isn't double-counted as two separate findings) -- which then legitimately renders in + // two places: the blocker bullet and the "Copy for AI agents" block. + expect(body.split(title).length - 1).toBe(2); }); it("renders BOTH the recovered consensus defect AND a separate non-AI gate blocker", () => { @@ -836,9 +839,13 @@ describe("gate blockers render in 'Why this is blocked' (FIX D1)", () => { changedFiles: 10, footerMarkdown: footer, }); - // Exactly once: under "Why this is blocked" only, never ALSO restated under "Suggested Action". - expect(body.split(blockerTitle).length - 1).toBe(1); - expect(body).toContain("Why this is blocked"); + // Never restated under "Suggested Action" (the original #5347 bug) -- the text before "Why this is + // blocked" must not contain it. It legitimately appears twice total: once under "Why this is blocked", + // once more inside the "Copy for AI agents" block (a deliberate, separate copyable rendition). + const [beforeWhyBlocked, afterWhyBlocked] = body.split("Why this is blocked"); + expect(beforeWhyBlocked).not.toContain(blockerTitle); + expect(afterWhyBlocked).toContain(blockerTitle); + expect(body.split(blockerTitle).length - 1).toBe(2); }); it("a manual-review HOLD (no gate blockers) still shows its own top-level reason, unaffected by the #5347 fix", () => { diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index c47dd0026c..cce6b82520 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -241,7 +241,9 @@ describe("renderUnifiedReviewComment", () => { { ...base, decision: "close", blockers: ["Same issue", "same issue", "Same issue"] }, {}, ); - expect(md.match(/Same issue/gi)?.length).toBe(1); + // Deduped down to ONE unique blocker, which then legitimately renders in TWO places: the "Why this is + // blocked" bullet list, and once more inside the "Copy for AI agents" code block. + expect(md.match(/Same issue/gi)?.length).toBe(2); }); it("omits optional chrome when the host provides none", () => { @@ -481,7 +483,8 @@ describe("renderUnifiedReviewComment", () => { it("skips empty blocker lines and caps long nit lists at 12", () => { const withEmpty = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["", " ", "Real blocker"] }, {}); - expect(withEmpty.match(/Real blocker/g)?.length).toBe(1); + // The one real blocker renders in TWO places: "Why this is blocked" and the "Copy for AI agents" block. + expect(withEmpty.match(/Real blocker/g)?.length).toBe(2); const capped = renderUnifiedReviewComment({ ...base, decision: "merge", nits: Array.from({ length: 13 }, (_, i) => `Distinct nit ${i + 1}`) }, {}); expect(capped).toContain("Distinct nit 12"); expect(capped).not.toContain("Distinct nit 13"); @@ -535,6 +538,59 @@ describe("renderUnifiedReviewComment", () => { }); }); +describe("'Copy for AI agents' block", () => { + it("omits the section entirely when there are no blockers", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, {}); + expect(md).not.toContain("Copy for AI agents"); + }); + + it("renders every blocker as a numbered item inside a fenced code block", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["First issue.", "Second issue."] }, {}); + expect(md).toContain("
๐Ÿ“‹ Copy for AI agents โ€” paste into your coding agent"); + // The whole comment is wrapped in a `> ` blockquote alert, so each line below carries that prefix. + expect(md).toContain("> ```\n> Fix the following blocker(s) from this PR review:"); + expect(md).toContain("> 1. First issue."); + expect(md).toContain("> 2. Second issue."); + }); + + it("uses the FULL blocker set, not the display-truncated one, when maxFindingsCaps.blockers is set", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "close", blockers: ["Alpha", "Beta", "Gamma"], maxFindingsCaps: { blockers: 1, nits: null } }, + {}, + ); + // Human-facing bullets are capped at 1 (plus a "+N more" footer)... + expect(md).toMatch(/Alpha[\s\S]*_\+2 more_/); + // ...but the AI-context block still gets every blocker, since an agent benefits from full context. + const aiSection = md.split("๐Ÿ“‹ Copy for AI agents")[1]!; + expect(aiSection).toContain("Alpha"); + expect(aiSection).toContain("Beta"); + expect(aiSection).toContain("Gamma"); + }); + + it("is NOT dropped by comment_verbosity: quiet (it extends the never-gated blockers, not decorative detail)", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "close", blockers: ["a real blocker"] }, + { commentVerbosity: "quiet" }, + ); + expect(md).toContain("Copy for AI agents"); + }); + + it("renders pre-expanded (
) under comment_verbosity: detailed, matching every other collapsible", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "close", blockers: ["a real blocker"] }, + { commentVerbosity: "detailed" }, + ); + expect(md).toContain("
๐Ÿ“‹ Copy for AI agents"); + }); + + it("escapes angle brackets inside the fenced block, same as every other public-facing field", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["Uses ."] }, {}); + const aiSection = md.split("๐Ÿ“‹ Copy for AI agents")[1]!; + expect(aiSection).toContain("<script>alert(1)</script>"); + expect(aiSection).not.toContain("