Skip to content

Commit 54e951e

Browse files
backnotpropclaude
andauthored
fix: don't ask agent to address feedback on LGTM approval (#293)
* fix: don't ask agent to address feedback on LGTM approval (#284) 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) <noreply@anthropic.com> * fix: use explicit approved flag instead of annotations.length heuristic 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * chore: annotate unused LGTM feedback string Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f5705cd commit 54e951e

9 files changed

Lines changed: 38 additions & 14 deletions

File tree

apps/hook/commands/plannotator-review.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ allowed-tools: Bash(plannotator:*)
99

1010
## Your task
1111

12-
Address the code review feedback above. The user has reviewed your changes in the Plannotator UI and provided specific annotations and comments.
12+
If the review above contains feedback or annotations, address them. If no changes were requested, acknowledge and continue.

apps/hook/server/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,12 @@ if (args[0] === "sessions") {
183183
server.stop();
184184

185185
// Output feedback (captured by slash command)
186-
console.log(result.feedback || "No feedback provided.");
186+
if (result.approved) {
187+
console.log("Code review completed — no changes requested.");
188+
} else {
189+
console.log(result.feedback);
190+
console.log("\nThe reviewer has identified issues above. You must address all of them.");
191+
}
187192
process.exit(0);
188193

189194
} else if (args[0] === "annotate") {

apps/opencode-plugin/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,11 @@ Do NOT proceed with implementation until your plan is approved.
221221
const shouldSwitchAgent = result.agentSwitch && result.agentSwitch !== 'disabled';
222222
const targetAgent = result.agentSwitch || 'build';
223223

224-
// Send feedback to agent - it will automatically respond and address it
224+
const message = result.approved
225+
? `# Code Review\n\nCode review completed — no changes requested.`
226+
: `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`;
227+
228+
// Send feedback to agent
225229
try {
226230
await ctx.client.session.prompt({
227231
path: { id: sessionId },
@@ -230,7 +234,7 @@ Do NOT proceed with implementation until your plan is approved.
230234
parts: [
231235
{
232236
type: "text",
233-
text: `# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`,
237+
text: message,
234238
},
235239
],
236240
},

apps/pi-extension/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,11 @@ export default function plannotator(pi: ExtensionAPI): void {
255255
server.stop();
256256

257257
if (result.feedback) {
258-
pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`);
258+
if (result.approved) {
259+
pi.sendUserMessage(`# Code Review\n\nCode review completed — no changes requested.`);
260+
} else {
261+
pi.sendUserMessage(`# Code Review Feedback\n\n${result.feedback}\n\nPlease address this feedback.`);
262+
}
259263
} else {
260264
ctx.ui.notify("Code review closed (no feedback).", "info");
261265
}

apps/pi-extension/server.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ export interface GitContext {
362362
export interface ReviewServerResult {
363363
port: number;
364364
url: string;
365-
waitForDecision: () => Promise<{ feedback: string }>;
365+
waitForDecision: () => Promise<{ approved: boolean; feedback: string }>;
366366
stop: () => void;
367367
}
368368

@@ -428,8 +428,8 @@ export function startReviewServer(options: {
428428
let currentGitRef = options.gitRef;
429429
let currentDiffType: DiffType = options.diffType || "uncommitted";
430430

431-
let resolveDecision!: (result: { feedback: string }) => void;
432-
const decisionPromise = new Promise<{ feedback: string }>((r) => {
431+
let resolveDecision!: (result: { approved: boolean; feedback: string }) => void;
432+
const decisionPromise = new Promise<{ approved: boolean; feedback: string }>((r) => {
433433
resolveDecision = r;
434434
});
435435

@@ -459,7 +459,10 @@ export function startReviewServer(options: {
459459
json(res, { rawPatch: currentPatch, gitRef: currentGitRef, diffType: currentDiffType });
460460
} else if (url.pathname === "/api/feedback" && req.method === "POST") {
461461
const body = await parseBody(req);
462-
resolveDecision({ feedback: (body.feedback as string) || "" });
462+
resolveDecision({
463+
approved: (body.approved as boolean) ?? false,
464+
feedback: (body.feedback as string) || "",
465+
});
463466
json(res, { ok: true });
464467
} else {
465468
html(res, options.htmlContent);

apps/review/server/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ server.stop();
8888
console.log(
8989
JSON.stringify({
9090
gitRef: displayRef,
91+
approved: result.approved,
9192
feedback: result.feedback,
9293
annotations: result.annotations,
9394
}, null, 2)

bun.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/review-editor/App.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,7 @@ const ReviewApp: React.FC = () => {
504504
method: 'POST',
505505
headers: { 'Content-Type': 'application/json' },
506506
body: JSON.stringify({
507+
approved: false,
507508
feedback: feedbackMarkdown,
508509
annotations,
509510
...(effectiveAgent && { agentSwitch: effectiveAgent }),
@@ -530,7 +531,8 @@ const ReviewApp: React.FC = () => {
530531
method: 'POST',
531532
headers: { 'Content-Type': 'application/json' },
532533
body: JSON.stringify({
533-
feedback: 'LGTM - no changes requested.',
534+
approved: true,
535+
feedback: 'LGTM - no changes requested.', // unused — integrations branch on `approved` flag
534536
annotations: [],
535537
}),
536538
});

packages/server/review.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ export interface ReviewServerResult {
5656
url: string;
5757
/** Whether running in remote mode */
5858
isRemote: boolean;
59-
/** Wait for user feedback submission */
59+
/** Wait for user review decision */
6060
waitForDecision: () => Promise<{
61+
approved: boolean;
6162
feedback: string;
6263
annotations: unknown[];
6364
agentSwitch?: string;
@@ -101,11 +102,13 @@ export async function startReviewServer(
101102

102103
// Decision promise
103104
let resolveDecision: (result: {
105+
approved: boolean;
104106
feedback: string;
105107
annotations: unknown[];
106108
agentSwitch?: string;
107109
}) => void;
108110
const decisionPromise = new Promise<{
111+
approved: boolean;
109112
feedback: string;
110113
annotations: unknown[];
111114
agentSwitch?: string;
@@ -259,13 +262,15 @@ export async function startReviewServer(
259262
if (url.pathname === "/api/feedback" && req.method === "POST") {
260263
try {
261264
const body = (await req.json()) as {
265+
approved?: boolean;
262266
feedback: string;
263267
annotations: unknown[];
264268
agentSwitch?: string;
265269
};
266270

267271
deleteDraft(draftKey);
268272
resolveDecision({
273+
approved: body.approved ?? false,
269274
feedback: body.feedback || "",
270275
annotations: body.annotations || [],
271276
agentSwitch: body.agentSwitch,

0 commit comments

Comments
 (0)