Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/review/unified-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]+/)
Expand Down Expand Up @@ -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 Β·
Expand Down
23 changes: 15 additions & 8 deletions test/unit/unified-comment-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
60 changes: 58 additions & 2 deletions test/unit/unified-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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("<details><summary><b>πŸ“‹ Copy for AI agents</b> β€” paste into your coding agent</summary>");
// 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 (<details open>) under comment_verbosity: detailed, matching every other collapsible", () => {
const md = renderUnifiedReviewComment(
{ ...base, decision: "close", blockers: ["a real blocker"] },
{ commentVerbosity: "detailed" },
);
expect(md).toContain("<details open><summary><b>πŸ“‹ Copy for AI agents</b>");
});

it("escapes angle brackets inside the fenced block, same as every other public-facing field", () => {
const md = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["Uses <script>alert(1)</script>."] }, {});
const aiSection = md.split("πŸ“‹ Copy for AI agents")[1]!;
expect(aiSection).toContain("&lt;script&gt;alert(1)&lt;/script&gt;");
expect(aiSection).not.toContain("<script>");
});
});

function reviewNote(rec: ReviewRecommendation, extra: Partial<ReviewNotes> = {}): DualReviewNote {
return {
model: "test-model",
Expand Down