diff --git a/src/create-prompt/templates/review-prompt.ts b/src/create-prompt/templates/review-prompt.ts index 5863ab2..30c3888 100644 --- a/src/create-prompt/templates/review-prompt.ts +++ b/src/create-prompt/templates/review-prompt.ts @@ -53,7 +53,7 @@ You MUST: 1. Read the ENTIRE \`${diffPath}\` file first (use offset/limit if needed for large files) 2. Create a mental checklist of ALL files that were changed 3. Review EACH file systematically - do not skip any file -4. Only submit your review AFTER you have analyzed every single changed file +4. Only publish findings AFTER you have analyzed every single changed file If the diff is large, work through it methodically. Do not rush. Do not skip files. The quality of your review depends on thoroughness. @@ -317,9 +317,9 @@ One short paragraph explaining *why* this is a bug and *how* it manifests. --- -## Phase 3: Submit Review +## Phase 3: Publish Results -### When NOT to submit +### When NOT to post inline findings * PR is formatting-only * You cannot anchor a high-confidence issue to a specific changed line @@ -332,24 +332,22 @@ One short paragraph explaining *why* this is a bug and *how* it manifests. * Anchor using **path + side + line** * RIGHT = new/modified code, LEFT = removed code * Line numbers must correspond to the chosen side -* Use \`github_pr___submit_review\` for the summary * Use \`github_pr___delete_comment\` or \`github_pr___minimize_comment\` for outdated "no issues" comments * Use \`github_pr___reply_to_comment\` to acknowledge resolved issues * **Do NOT call** \`github_pr___resolve_review_thread\` -* Do **not** approve or request changes +* Update tracking comment ${context.droidCommentId ? `ID ${context.droidCommentId}` : "for this run"} via \`github_comment___update_droid_comment\` with a concise summary and findings list ### "No issues" handling -* If no issues and a prior "no issues" comment exists → skip -* If no issues and no prior comment exists → post a brief summary -* If issues exist and a prior "no issues" comment exists → delete/minimize it -* **Do NOT delete** comment ID ${context.droidCommentId} +* If no qualifying issues, do not post inline comments +* Update the tracking comment with a brief "no high-confidence findings" summary +* Do not post any separate review summary comment --- -## Review summary +## Tracking comment summary -In the submitted review body: +In the tracking comment update body: * State whether the changes are correct or incorrect * Provide a 1-3 sentence overall assessment diff --git a/src/create-prompt/templates/review-validator-prompt.ts b/src/create-prompt/templates/review-validator-prompt.ts index 6058e91..e36f676 100644 --- a/src/create-prompt/templates/review-validator-prompt.ts +++ b/src/create-prompt/templates/review-validator-prompt.ts @@ -54,7 +54,7 @@ Read: 1) Write validated results to: \`${reviewValidatedPath}\` 2) Post ONLY the approved inline comments to the PR -3) Submit a PR review summary (if applicable) +3) Update tracking comment ${context.droidCommentId ? `ID ${context.droidCommentId}` : "for this run"} with a concise review summary focused on approved findings ======================= @@ -173,7 +173,9 @@ Tooling note: After writing \`${reviewValidatedPath}\`, post comments ONLY for \`status === "approved"\`: * For each approved entry, call \`github_inline_comment___create_inline_comment\` with the \`comment\` object. -* Submit a review via \`github_pr___submit_review\` using the summary body (if there are any approved items OR a meaningful overall assessment). -* Do not approve or request changes. +* Build one tracking-comment summary that includes: + * Overall assessment from \`reviewSummary.body\` +* Update the tracking comment once via \`github_comment___update_droid_comment\`. +* If there are no approved findings, do not post any review summary comment to the PR timeline; only update the tracking comment. `; } diff --git a/src/tag/commands/review-validator.ts b/src/tag/commands/review-validator.ts index 4e9148d..8030f6e 100644 --- a/src/tag/commands/review-validator.ts +++ b/src/tag/commands/review-validator.ts @@ -79,10 +79,12 @@ export async function prepareReviewValidatorMode({ "github_inline_comment___create_inline_comment", ]; - const validatorTools = ["github_pr___submit_review"]; + const safeUserAllowedMCPTools = userAllowedMCPTools.filter( + (tool) => tool !== "github_pr___submit_review", + ); const allowedTools = Array.from( - new Set([...baseTools, ...validatorTools, ...userAllowedMCPTools]), + new Set([...baseTools, ...safeUserAllowedMCPTools]), ); const mcpTools = await prepareMcpTools({ diff --git a/src/tag/commands/review.ts b/src/tag/commands/review.ts index 4fabb33..d33991f 100644 --- a/src/tag/commands/review.ts +++ b/src/tag/commands/review.ts @@ -133,7 +133,6 @@ export async function prepareReviewMode({ : [ "github_inline_comment___create_inline_comment", "github_pr___list_review_comments", - "github_pr___submit_review", "github_pr___delete_comment", "github_pr___minimize_comment", "github_pr___reply_to_comment", @@ -147,7 +146,9 @@ export async function prepareReviewMode({ (!tool.startsWith("github_pr___") && tool !== "github_inline_comment___create_inline_comment"), ) - : userAllowedMCPTools; + : userAllowedMCPTools.filter( + (tool) => tool !== "github_pr___submit_review", + ); const allowedTools = Array.from( new Set([ diff --git a/test/create-prompt/templates/review-prompt.test.ts b/test/create-prompt/templates/review-prompt.test.ts index 3898c6b..ebd7d54 100644 --- a/test/create-prompt/templates/review-prompt.test.ts +++ b/test/create-prompt/templates/review-prompt.test.ts @@ -101,9 +101,9 @@ describe("generateReviewPrompt", () => { expect(prompt).toContain( "Use `github_inline_comment___create_inline_comment`", ); - expect(prompt).toContain("Do **not** approve or request changes"); - expect(prompt).toContain("github_pr___submit_review"); - expect(prompt).toContain("### When NOT to submit"); + expect(prompt).toContain("github_comment___update_droid_comment"); + expect(prompt).not.toContain("github_pr___submit_review"); + expect(prompt).toContain("### When NOT to post inline findings"); expect(prompt).toContain("All findings are low-severity (P2/P3)"); }); diff --git a/test/create-prompt/templates/review-validator-prompt.test.ts b/test/create-prompt/templates/review-validator-prompt.test.ts index 195f2e3..1901f98 100644 --- a/test/create-prompt/templates/review-validator-prompt.test.ts +++ b/test/create-prompt/templates/review-validator-prompt.test.ts @@ -76,6 +76,8 @@ describe("generateReviewValidatorPrompt", () => { expect(prompt).toContain("Phase 2: Validate candidates"); expect(prompt).toContain("Phase 3: Write review_validated.json"); expect(prompt).toContain("Phase 4: Post approved items"); + expect(prompt).toContain("github_comment___update_droid_comment"); + expect(prompt).not.toContain("github_pr___submit_review"); }); it("includes correct PR context", () => { diff --git a/test/modes/tag/review-command.test.ts b/test/modes/tag/review-command.test.ts index 5b8b8af..ed728ec 100644 --- a/test/modes/tag/review-command.test.ts +++ b/test/modes/tag/review-command.test.ts @@ -161,7 +161,6 @@ describe("prepareReviewMode", () => { "github_comment___update_droid_comment", "github_inline_comment___create_inline_comment", "github_pr___list_review_comments", - "github_pr___submit_review", "github_pr___resolve_review_thread", ]), }), @@ -179,7 +178,6 @@ describe("prepareReviewMode", () => { "github_comment___update_droid_comment", "github_inline_comment___create_inline_comment", "github_pr___list_review_comments", - "github_pr___submit_review", "github_pr___delete_comment", "github_pr___minimize_comment", "github_pr___reply_to_comment", diff --git a/test/modes/tag/review-validator-command.test.ts b/test/modes/tag/review-validator-command.test.ts new file mode 100644 index 0000000..8d4b856 --- /dev/null +++ b/test/modes/tag/review-validator-command.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +describe("review-validator command model handling", () => { + it("reads REVIEW_MODEL and REASONING_EFFORT from env", () => { + const source = readFileSync( + join(process.cwd(), "src", "tag", "commands", "review-validator.ts"), + "utf8", + ); + + expect(source).toContain( + "const reviewModel = process.env.REVIEW_MODEL?.trim();", + ); + expect(source).toContain( + "const reasoningEffort = process.env.REASONING_EFFORT?.trim();", + ); + }); + + it("adds --model and --reasoning-effort conditionally", () => { + const source = readFileSync( + join(process.cwd(), "src", "tag", "commands", "review-validator.ts"), + "utf8", + ); + + expect(source).toContain('droidArgParts.push(`--model "${reviewModel}"`)'); + expect(source).toContain( + 'droidArgParts.push(`--reasoning-effort "${reasoningEffort}"`)', + ); + }); +});