diff --git a/base-action/src/index.ts b/base-action/src/index.ts index 2623880..860884a 100644 --- a/base-action/src/index.ts +++ b/base-action/src/index.ts @@ -29,8 +29,7 @@ async function run() { mcpTools: process.env.INPUT_MCP_TOOLS, systemPrompt: process.env.INPUT_SYSTEM_PROMPT, appendSystemPrompt: process.env.INPUT_APPEND_SYSTEM_PROMPT, - pathToDroidExecutable: - process.env.INPUT_PATH_TO_DROID_EXECUTABLE, + pathToDroidExecutable: process.env.INPUT_PATH_TO_DROID_EXECUTABLE, showFullOutput: process.env.INPUT_SHOW_FULL_OUTPUT, }); } catch (error) { diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index b2ea5af..068b276 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -108,15 +108,12 @@ export function prepareContext( commonFields.droidBranch = droidBranch; } - const eventData = buildEventData( - context, - { - commentId, - commentBody, - baseBranch, - droidBranch, - }, - ); + const eventData = buildEventData(context, { + commentId, + commentBody, + baseBranch, + droidBranch, + }); const result: PreparedContext = { ...commonFields, @@ -282,9 +279,7 @@ function buildEventData( } } -export type PromptGenerator = ( - context: PreparedContext, -) => string; +export type PromptGenerator = (context: PreparedContext) => string; export type PromptCreationOptions = { githubContext: ParsedGitHubContext; diff --git a/src/create-prompt/templates/review-prompt.ts b/src/create-prompt/templates/review-prompt.ts index 418d264..f7a82e1 100644 --- a/src/create-prompt/templates/review-prompt.ts +++ b/src/create-prompt/templates/review-prompt.ts @@ -99,7 +99,7 @@ Your review comments should be: Output Format: Structure each inline comment as: -**[P0-P3] Clear title (≤ 80 chars, imperative mood)** +**[P0/P1] Clear title (≤ 80 chars, imperative mood)** (blank line) Explanation of why this is a problem (1 paragraph max). @@ -131,6 +131,65 @@ Commenting rules: - For low confidence findings, ask a question; for medium/high confidence, state the issue concretely. - Only include explicit code suggestions when you are absolutely certain the replacement is correct and safe. +Output: +1. Analyze the PR to generate: + - A concise 1-2 sentence summary of what the PR does + - 3-5 key changes extracted from the diff + - The most important files changed (up to 5-7 files) + +2. Write findings to \`code-review-results.json\` with this structure: +\`\`\`json +{ + "type": "code", + "summary": "Brief 1-2 sentence description of what this PR does", + "keyChanges": [ + "Added new authentication flow", + "Refactored database queries for performance", + "Fixed validation bug in user input" + ], + "importantFiles": [ + { "path": "src/auth/login.ts", "description": "New OAuth implementation" }, + { "path": "src/db/queries.ts", "description": "Query optimization" } + ], + "findings": [ + { + "id": "CR-001", + "type": "bug|issue|suggestion", + "severity": "high|medium|low", + "file": "path/to/file.ts", + "line": 45, + "side": "RIGHT", + "description": "Brief description of the issue", + "suggestion": "Optional code fix" + } + ] +} +\`\`\` + +3. Update the tracking comment with a summary using \`github_comment___update_droid_comment\`: +\`\`\`markdown +## Code review completed + +### Summary +{Brief 1-2 sentence description of what this PR does} + +### Key Changes +- {Change 1} +- {Change 2} +- {Change 3} + +### Important Files Changed +- \`path/to/file1.ts\` - {Brief description of changes} +- \`path/to/file2.ts\` - {Brief description of changes} + +### Review Findings +| ID | Type | Severity | File | Description | +|----|------|----------|------|-------------| +| CR-001 | Bug | high | auth.ts:45 | Null pointer exception | + +*Inline comments will be posted after all reviews complete.* +\`\`\` + Submission: - Do not submit inline comments when: - the PR appears formatting-only, or diff --git a/src/github/context.ts b/src/github/context.ts index 15592f3..ee0e888 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -157,7 +157,10 @@ export function parseGitHubContext(): GitHubContext { securityBlockOnHigh: process.env.SECURITY_BLOCK_ON_HIGH === "true", securityNotifyTeam: process.env.SECURITY_NOTIFY_TEAM ?? "", securityScanSchedule: process.env.SECURITY_SCAN_SCHEDULE === "true", - securityScanDays: Math.max(1, parseInt(process.env.SECURITY_SCAN_DAYS ?? "7", 10) || 7), + securityScanDays: Math.max( + 1, + parseInt(process.env.SECURITY_SCAN_DAYS ?? "7", 10) || 7, + ), }, }; diff --git a/src/github/utils/command-parser.test.ts b/src/github/utils/command-parser.test.ts index 602a13b..8d30c0f 100644 --- a/src/github/utils/command-parser.test.ts +++ b/src/github/utils/command-parser.test.ts @@ -79,26 +79,16 @@ describe("Command Parser", () => { expect(result?.raw).toBe("@droid review"); }); - it("should detect @droid review security (combined)", () => { + it("should parse @droid review security as review", () => { const result = parseDroidCommand("@droid review security"); - expect(result?.command).toBe("review-security"); - expect(result?.raw).toBe("@droid review security"); + expect(result?.command).toBe("review"); + expect(result?.raw).toBe("@droid review"); }); - it("should detect @droid security review (combined)", () => { + it("should parse @droid security review as security", () => { const result = parseDroidCommand("@droid security review"); - expect(result?.command).toBe("review-security"); - expect(result?.raw).toBe("@droid security review"); - }); - - it("should be case insensitive for combined commands", () => { - const result = parseDroidCommand("@DROID REVIEW SECURITY"); - expect(result?.command).toBe("review-security"); - }); - - it("should prioritize combined over individual review", () => { - const result = parseDroidCommand("@droid review security please"); - expect(result?.command).toBe("review-security"); + expect(result?.command).toBe("security"); + expect(result?.raw).toBe("@droid security"); }); it("should detect @droid security", () => { diff --git a/src/github/utils/command-parser.ts b/src/github/utils/command-parser.ts index 182cac5..dcf15fd 100644 --- a/src/github/utils/command-parser.ts +++ b/src/github/utils/command-parser.ts @@ -8,7 +8,6 @@ export type DroidCommand = | "fill" | "review" | "security" - | "review-security" | "security-full" | "default"; @@ -39,19 +38,6 @@ export function parseDroidCommand(text: string): ParsedCommand | null { }; } - // Check for @droid review security OR @droid security review (both reviews) - // Must check before individual review/security to avoid false matches - const combinedMatch = text.match( - /@droid\s+(?:review\s+security|security\s+review)/i, - ); - if (combinedMatch) { - return { - command: "review-security", - raw: combinedMatch[0], - location: "body", // Will be set by caller - }; - } - // Check for @droid review command (case insensitive) const reviewMatch = text.match(/@droid\s+review/i); if (reviewMatch) { diff --git a/test/create-prompt/templates/review-prompt.test.ts b/test/create-prompt/templates/review-prompt.test.ts index 3833ded..f0ec52f 100644 --- a/test/create-prompt/templates/review-prompt.test.ts +++ b/test/create-prompt/templates/review-prompt.test.ts @@ -29,41 +29,73 @@ describe("generateReviewPrompt", () => { expect(prompt).toContain("Objectives:"); expect(prompt).toContain("Re-check existing review comments"); + expect(prompt).toContain("Review the PR diff"); expect(prompt).toContain("git merge-base"); expect(prompt).toContain("git diff"); + expect(prompt).toContain("gh pr diff 42 --repo test-owner/test-repo"); expect(prompt).toContain( - "gh pr diff 42 --repo test-owner/test-repo", + "gh api repos/test-owner/test-repo/pulls/42/files", ); - expect(prompt).toContain("gh api repos/test-owner/test-repo/pulls/42/files"); expect(prompt).toContain("github_inline_comment___create_inline_comment"); - expect(prompt).toContain("github_pr___resolve_review_thread"); - expect(prompt).toContain("every substantive comment must be inline on the changed line"); + expect(prompt).toContain("every substantive comment must be inline"); + expect(prompt).toContain("Thread resolution rule (CRITICAL)"); + expect(prompt).toContain("NEVER resolve review threads"); }); it("emphasizes accuracy gates and bug detection guidelines", () => { const prompt = generateReviewPrompt(createBaseContext()); expect(prompt).toContain("How Many Findings to Return:"); - expect(prompt).toContain("Output all findings that the original author would fix"); + expect(prompt).toContain( + "Output all findings that the original author would fix", + ); expect(prompt).toContain("Key Guidelines for Bug Detection:"); expect(prompt).toContain("Priority Levels:"); expect(prompt).toContain("[P0]"); + expect(prompt).toContain("Do not escalate style/formatting into P0/P1"); expect(prompt).toContain("Never raise purely stylistic"); - expect(prompt).toContain("Never repeat or re-raise an issue previously highlighted"); + expect(prompt).toContain( + "Never repeat or re-raise an issue previously highlighted", + ); }); - it("describes submission guidance", () => { + it("describes MCP tools and diff side selection", () => { const prompt = generateReviewPrompt(createBaseContext()); - expect(prompt).toContain("Prefer github_inline_comment___create_inline_comment"); - expect(prompt).toContain("gh api repos/test-owner/test-repo/pulls/42/reviews"); - expect(prompt).toContain("Do not approve or request changes"); + expect(prompt).toContain("Preferred MCP tools"); + expect(prompt).toContain("github_inline_comment___create_inline_comment"); expect(prompt).toContain("github_pr___submit_review"); + expect(prompt).toContain("github_pr___delete_comment"); expect(prompt).toContain("github_pr___resolve_review_thread"); - expect(prompt).toContain("skip submitting another comment to avoid redundancy"); - expect(prompt).toContain("Do not submit inline comments"); + expect(prompt).toContain("Diff Side Selection (CRITICAL)"); + expect(prompt).toContain('side="RIGHT"'); + expect(prompt).toContain('side="LEFT"'); + }); + + it("describes JSON output format with summary", () => { + const prompt = generateReviewPrompt(createBaseContext()); + + expect(prompt).toContain("code-review-results.json"); + expect(prompt).toContain("github_comment___update_droid_comment"); + expect(prompt).toContain( + "Inline comments will be posted after all reviews complete", + ); + expect(prompt).toContain("### Summary"); + expect(prompt).toContain("### Key Changes"); + expect(prompt).toContain("### Important Files Changed"); + expect(prompt).toContain("### Review Findings"); + }); + + it("describes submission guidance", () => { + const prompt = generateReviewPrompt(createBaseContext()); + + expect(prompt).toContain("Submission:"); + expect(prompt).toContain("Do not submit inline comments when"); + expect(prompt).toContain("all findings are low-severity (P2/P3)"); expect(prompt).toContain( - "Do not escalate style/formatting into P0/P1 just to justify leaving an inline comment", + "gh api repos/test-owner/test-repo/pulls/42/reviews", ); + expect(prompt).toContain("Do not approve or request changes"); + expect(prompt).toContain("submit a comment-only review"); }); }); diff --git a/test/integration/review-flow.test.ts b/test/integration/review-flow.test.ts index 61bbf6e..9eb1795 100644 --- a/test/integration/review-flow.test.ts +++ b/test/integration/review-flow.test.ts @@ -1,11 +1,4 @@ -import { - afterEach, - beforeEach, - describe, - expect, - it, - spyOn, -} from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import path from "node:path"; import os from "node:os"; import { mkdtemp, readFile, rm } from "node:fs/promises"; @@ -32,8 +25,6 @@ describe("review command integration", () => { process.env.RUNNER_TEMP = tmpDir; process.env.DROID_ARGS = ""; - - createCommentSpy = spyOn( createInitial, "createInitialComment", @@ -95,27 +86,28 @@ describe("review command integration", () => { } as any, }); - const octokit = { - rest: {}, - graphql: () => Promise.resolve({ - repository: { - pullRequest: { - baseRefName: "main", - headRefName: "feature/review", - headRefOid: "def456", - } - } - }) + const octokit = { + rest: {}, + graphql: () => + Promise.resolve({ + repository: { + pullRequest: { + baseRefName: "main", + headRefName: "feature/review", + headRefOid: "def456", + }, + }, + }), } as any; graphqlSpy = spyOn(octokit, "graphql").mockResolvedValue({ repository: { pullRequest: { baseRefName: "main", - headRefName: "feature/review", + headRefName: "feature/review", headRefOid: "def456", - } - } + }, + }, }); const result = await prepareTagExecution({ @@ -147,20 +139,25 @@ describe("review command integration", () => { expect(prompt).toContain("You are performing an automated code review"); expect(prompt).toContain("github_inline_comment___create_inline_comment"); expect(prompt).toContain("How Many Findings to Return:"); - expect(prompt).toContain("Output all findings that the original author would fix"); + expect(prompt).toContain( + "Output all findings that the original author would fix", + ); expect(prompt).toContain("Key Guidelines for Bug Detection:"); expect(prompt).toContain("Priority Levels:"); - expect(prompt).toContain("gh pr view 7 --repo test-owner/test-repo --json comments,reviews"); - expect(prompt).toContain("every substantive comment must be inline on the changed line"); - expect(prompt).toContain("github_pr___resolve_review_thread"); + expect(prompt).toContain( + "gh pr view 7 --repo test-owner/test-repo --json comments,reviews", + ); + expect(prompt).toContain( + "every substantive comment must be inline on the changed line", + ); + expect(prompt).toContain("code-review-results.json"); + expect(prompt).toContain("github_pr___submit_review"); const droidArgsCall = setOutputSpy.mock.calls.find( (call: unknown[]) => call[0] === "droid_args", ) as [string, string] | undefined; - expect(droidArgsCall?.[1]).toContain( - "github_pr___list_review_comments", - ); + expect(droidArgsCall?.[1]).toContain("github_pr___list_review_comments"); expect(droidArgsCall?.[1]).toContain("github_pr___submit_review"); expect(droidArgsCall?.[1]).toContain( "github_inline_comment___create_inline_comment", diff --git a/test/modes/tag/review-command.test.ts b/test/modes/tag/review-command.test.ts index e31db2c..d09be4a 100644 --- a/test/modes/tag/review-command.test.ts +++ b/test/modes/tag/review-command.test.ts @@ -260,7 +260,9 @@ describe("prepareReviewMode", () => { const droidArgsCall = setOutputSpy.mock.calls.find( (call: unknown[]) => call[0] === "droid_args", ) as [string, string] | undefined; - expect(droidArgsCall?.[1]).toContain('--model "claude-sonnet-4-5-20250929"'); + expect(droidArgsCall?.[1]).toContain( + '--model "claude-sonnet-4-5-20250929"', + ); }); it("does not add --model flag when REVIEW_MODEL is empty", async () => {