mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
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>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -183,7 +183,12 @@ if (args[0] === "sessions") {
|
||||
server.stop();
|
||||
|
||||
// Output feedback (captured by slash command)
|
||||
console.log(result.feedback || "No feedback provided.");
|
||||
if (result.approved) {
|
||||
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);
|
||||
|
||||
} else if (args[0] === "annotate") {
|
||||
|
||||
@@ -221,7 +221,11 @@ Do NOT proceed with implementation until your plan is approved.
|
||||
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 = 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 {
|
||||
await ctx.client.session.prompt({
|
||||
path: { id: sessionId },
|
||||
@@ -230,7 +234,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,
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
@@ -255,7 +255,11 @@ 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.`);
|
||||
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");
|
||||
}
|
||||
|
||||
@@ -362,7 +362,7 @@ export interface GitContext {
|
||||
export interface ReviewServerResult {
|
||||
port: number;
|
||||
url: string;
|
||||
waitForDecision: () => Promise<{ feedback: string }>;
|
||||
waitForDecision: () => Promise<{ approved: boolean; feedback: string }>;
|
||||
stop: () => void;
|
||||
}
|
||||
|
||||
@@ -428,8 +428,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;
|
||||
});
|
||||
|
||||
@@ -459,7 +459,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);
|
||||
|
||||
@@ -88,6 +88,7 @@ server.stop();
|
||||
console.log(
|
||||
JSON.stringify({
|
||||
gitRef: displayRef,
|
||||
approved: result.approved,
|
||||
feedback: result.feedback,
|
||||
annotations: result.annotations,
|
||||
}, null, 2)
|
||||
|
||||
@@ -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:*",
|
||||
},
|
||||
|
||||
@@ -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,7 +531,8 @@ const ReviewApp: React.FC = () => {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
feedback: 'LGTM - no changes requested.',
|
||||
approved: true,
|
||||
feedback: 'LGTM - no changes requested.', // unused — integrations branch on `approved` flag
|
||||
annotations: [],
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user