From e57e7745bf098fd0c0f7ac96ecf55a1eaebcf154 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 14 Mar 2026 14:55:13 -0700 Subject: [PATCH 1/4] fix: don't ask agent to address feedback on LGTM approval (#284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the reviewer approves with no annotations, send a neutral "Code review completed — no changes requested." message instead of the contradictory "LGTM" + "Please address this feedback." Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/hook/commands/plannotator-review.md | 2 +- apps/hook/server/index.ts | 7 ++++++- apps/opencode-plugin/index.ts | 10 ++++++++-- apps/pi-extension/index.ts | 7 ++++++- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/apps/hook/commands/plannotator-review.md b/apps/hook/commands/plannotator-review.md index a8d8dd692..433fa1328 100644 --- a/apps/hook/commands/plannotator-review.md +++ b/apps/hook/commands/plannotator-review.md @@ -9,4 +9,4 @@ allowed-tools: Bash(plannotator:*) ## Your task -Address the code review feedback above. The user has reviewed your changes in the Plannotator UI and provided specific annotations and comments. +If the review above contains feedback or annotations, address them. If no changes were requested, acknowledge and continue. diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 98b87db09..6acf822e9 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -183,7 +183,12 @@ if (args[0] === "sessions") { server.stop(); // Output feedback (captured by slash command) - console.log(result.feedback || "No feedback provided."); + const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; + if (hasAnnotations) { + console.log(result.feedback); + } else { + console.log("Code review completed — no changes requested."); + } process.exit(0); } else if (args[0] === "annotate") { diff --git a/apps/opencode-plugin/index.ts b/apps/opencode-plugin/index.ts index 949caadf1..6a9371042 100644 --- a/apps/opencode-plugin/index.ts +++ b/apps/opencode-plugin/index.ts @@ -217,11 +217,17 @@ Do NOT proceed with implementation until your plan is approved. // Only try to send feedback if we have a valid session ID if (sessionId) { + const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; + // Check agent switch setting (defaults to 'build' if not set) const shouldSwitchAgent = result.agentSwitch && result.agentSwitch !== 'disabled'; const targetAgent = result.agentSwitch || 'build'; - // Send feedback to agent - it will automatically respond and address it + const message = hasAnnotations + ? `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.` + : `# Code Review\n\nCode review completed — no changes requested.`; + + // Send feedback to agent try { await ctx.client.session.prompt({ path: { id: sessionId }, @@ -230,7 +236,7 @@ Do NOT proceed with implementation until your plan is approved. parts: [ { type: "text", - text: `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`, + text: message, }, ], }, diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 1c8e724bd..8c06ce53f 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -255,7 +255,12 @@ export default function plannotator(pi: ExtensionAPI): void { server.stop(); if (result.feedback) { - pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`); + const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; + if (hasAnnotations) { + pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`); + } else { + pi.sendUserMessage(`# Code Review\n\nCode review completed — no changes requested.`); + } } else { ctx.ui.notify("Code review closed (no feedback).", "info"); } From 2635564d514a69b0047ee7f2eaa332fc3b72b20d Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 14 Mar 2026 17:38:11 -0700 Subject: [PATCH 2/4] fix: use explicit approved flag instead of annotations.length heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix inferred LGTM from an empty annotations array, but VS Code editor annotations are carried in feedbackMarkdown without populating the annotations array — causing real review comments to be misclassified as approvals. Thread an explicit `approved` boolean from the UI through the review server to all three integrations. Closes #284 Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/hook/server/index.ts | 7 +++---- apps/opencode-plugin/index.ts | 8 +++----- apps/pi-extension/index.ts | 7 +++---- apps/pi-extension/server.ts | 11 +++++++---- apps/review/server/index.ts | 1 + bun.lock | 6 +++--- packages/review-editor/App.tsx | 2 ++ packages/server/review.ts | 7 ++++++- 8 files changed, 28 insertions(+), 21 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 6acf822e9..4d966f104 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -183,11 +183,10 @@ if (args[0] === "sessions") { server.stop(); // Output feedback (captured by slash command) - const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; - if (hasAnnotations) { - console.log(result.feedback); - } else { + if (result.approved) { console.log("Code review completed — no changes requested."); + } else { + console.log(result.feedback); } process.exit(0); diff --git a/apps/opencode-plugin/index.ts b/apps/opencode-plugin/index.ts index 6a9371042..1a43854b7 100644 --- a/apps/opencode-plugin/index.ts +++ b/apps/opencode-plugin/index.ts @@ -217,15 +217,13 @@ Do NOT proceed with implementation until your plan is approved. // Only try to send feedback if we have a valid session ID if (sessionId) { - const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; - // Check agent switch setting (defaults to 'build' if not set) const shouldSwitchAgent = result.agentSwitch && result.agentSwitch !== 'disabled'; const targetAgent = result.agentSwitch || 'build'; - const message = hasAnnotations - ? `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.` - : `# Code Review\n\nCode review completed — no changes requested.`; + const message = result.approved + ? `# Code Review\n\nCode review completed — no changes requested.` + : `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`; // Send feedback to agent try { diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 8c06ce53f..ce46de777 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -255,11 +255,10 @@ export default function plannotator(pi: ExtensionAPI): void { server.stop(); if (result.feedback) { - const hasAnnotations = Array.isArray(result.annotations) && result.annotations.length > 0; - if (hasAnnotations) { - pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`); - } else { + if (result.approved) { pi.sendUserMessage(`# Code Review\n\nCode review completed — no changes requested.`); + } else { + pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`); } } else { ctx.ui.notify("Code review closed (no feedback).", "info"); diff --git a/apps/pi-extension/server.ts b/apps/pi-extension/server.ts index 75795a2b1..bf8ff3d69 100644 --- a/apps/pi-extension/server.ts +++ b/apps/pi-extension/server.ts @@ -349,7 +349,7 @@ export interface GitContext { export interface ReviewServerResult { port: number; url: string; - waitForDecision: () => Promise<{ feedback: string }>; + waitForDecision: () => Promise<{ approved: boolean; feedback: string }>; stop: () => void; } @@ -415,8 +415,8 @@ export function startReviewServer(options: { let currentGitRef = options.gitRef; let currentDiffType: DiffType = options.diffType || "uncommitted"; - let resolveDecision!: (result: { feedback: string }) => void; - const decisionPromise = new Promise<{ feedback: string }>((r) => { + let resolveDecision!: (result: { approved: boolean; feedback: string }) => void; + const decisionPromise = new Promise<{ approved: boolean; feedback: string }>((r) => { resolveDecision = r; }); @@ -446,7 +446,10 @@ export function startReviewServer(options: { json(res, { rawPatch: currentPatch, gitRef: currentGitRef, diffType: currentDiffType }); } else if (url.pathname === "/api/feedback" && req.method === "POST") { const body = await parseBody(req); - resolveDecision({ feedback: (body.feedback as string) || "" }); + resolveDecision({ + approved: (body.approved as boolean) ?? false, + feedback: (body.feedback as string) || "", + }); json(res, { ok: true }); } else { html(res, options.htmlContent); diff --git a/apps/review/server/index.ts b/apps/review/server/index.ts index 2c786bcab..1c27b2b0d 100644 --- a/apps/review/server/index.ts +++ b/apps/review/server/index.ts @@ -88,6 +88,7 @@ server.stop(); console.log( JSON.stringify({ gitRef: displayRef, + approved: result.approved, feedback: result.feedback, annotations: result.annotations, }, null, 2) diff --git a/bun.lock b/bun.lock index 858d55ef8..8cadb9c43 100644 --- a/bun.lock +++ b/bun.lock @@ -50,7 +50,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.11.4", + "version": "0.12.0", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -71,7 +71,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.11.4", + "version": "0.12.0", "peerDependencies": { "@mariozechner/pi-coding-agent": ">=0.53.0", }, @@ -151,7 +151,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.11.4", + "version": "0.12.0", "dependencies": { "@plannotator/shared": "workspace:*", }, diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index ab7bad579..2c8959b67 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -504,6 +504,7 @@ const ReviewApp: React.FC = () => { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ + approved: false, feedback: feedbackMarkdown, annotations, ...(effectiveAgent && { agentSwitch: effectiveAgent }), @@ -530,6 +531,7 @@ const ReviewApp: React.FC = () => { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ + approved: true, feedback: 'LGTM - no changes requested.', annotations: [], }), diff --git a/packages/server/review.ts b/packages/server/review.ts index bb16c9f91..7ed4e9e8a 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -56,8 +56,9 @@ export interface ReviewServerResult { url: string; /** Whether running in remote mode */ isRemote: boolean; - /** Wait for user feedback submission */ + /** Wait for user review decision */ waitForDecision: () => Promise<{ + approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; @@ -101,11 +102,13 @@ export async function startReviewServer( // Decision promise let resolveDecision: (result: { + approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; }) => void; const decisionPromise = new Promise<{ + approved: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; @@ -259,6 +262,7 @@ export async function startReviewServer( if (url.pathname === "/api/feedback" && req.method === "POST") { try { const body = (await req.json()) as { + approved?: boolean; feedback: string; annotations: unknown[]; agentSwitch?: string; @@ -266,6 +270,7 @@ export async function startReviewServer( deleteDraft(draftKey); resolveDecision({ + approved: body.approved ?? false, feedback: body.feedback || "", annotations: body.annotations || [], agentSwitch: body.agentSwitch, From 8102bb6ce848315acfd104adf0e9e106ac096bd9 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 14 Mar 2026 17:51:07 -0700 Subject: [PATCH 3/4] fix: append assertive instruction to review feedback output When the reviewer submits actual feedback, append "The reviewer has identified issues above. You must address all of them." so the agent treats annotations with urgency rather than soft-acknowledging them. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/hook/server/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 4d966f104..0fb5c81a8 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -187,6 +187,7 @@ if (args[0] === "sessions") { console.log("Code review completed — no changes requested."); } else { console.log(result.feedback); + console.log("\nThe reviewer has identified issues above. You must address all of them."); } process.exit(0); From e7f90ef31fbe4d17bd7ce2dc2e1656f039ab1a4a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 14 Mar 2026 17:55:56 -0700 Subject: [PATCH 4/4] chore: annotate unused LGTM feedback string Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/review-editor/App.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 2c8959b67..929c7e070 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -532,7 +532,7 @@ const ReviewApp: React.FC = () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ approved: true, - feedback: 'LGTM - no changes requested.', + feedback: 'LGTM - no changes requested.', // unused — integrations branch on `approved` flag annotations: [], }), });