Skip to content

Commit 908ea9c

Browse files
backnotpropclaude
andauthored
refactor: shared feedback templates + preserve plan title on deny (#296) (#298)
* refactor: shared feedback templates across all integrations The deny/feedback prompts sent to LLM agents were duplicated as inline string templates in hook, opencode-plugin, and pi-extension — each with different tone and framing. The hook's directive style (from #224) was the most effective at getting agents to address feedback. This extracts all feedback text into @plannotator/shared/feedback-templates and has every integration import from the single source of truth. Closes #215 follow-up (propagates fix to OpenCode and Pi). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: rewrite feedback template tests around contracts not implementation Tests now verify: cross-integration consistency, verbatim feedback preservation, empty input handling, and that approved messages don't contain directive language. Wording can change freely without breaking tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: instruct agent to preserve plan title on resubmission (#296) Version history slugs are derived from the plan's first # heading. When the agent renames the heading after a deny, the version chain breaks and the user loses diffs. The deny template now tells the agent not to change the title unless explicitly asked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: improve plan deny preamble readability Break the dense single-paragraph preamble into structured sections: verdict, directive, and rules list. Easier for agents to parse. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add missing @plannotator/shared workspace dependency Hook and OpenCode plugin imported from @plannotator/shared/feedback-templates without declaring it as a dependency. Worked locally but failed in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove code review and annotate from shared templates Scope-crept into code review/annotate feedback which introduced a double heading regression and dropped integration-specific strings. Reverts those paths to their original inline strings; shared module now only covers plan deny feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: restore Pi plan file hint and vendor template for source installs Add optional planFilePath to planDenyFeedback so Pi can tell the agent to read the plan file before editing. Check in a vendored copy of the template so Pi source installs work without running build:pi first. 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 6cde395 commit 908ea9c

12 files changed

Lines changed: 124 additions & 21 deletions

File tree

apps/hook/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"dependencies": {
1212
"@plannotator/editor": "workspace:*",
1313
"@plannotator/server": "workspace:*",
14+
"@plannotator/shared": "workspace:*",
1415
"@plannotator/ui": "workspace:*",
1516
"react": "^19.2.3",
1617
"react-dom": "^19.2.3",

apps/hook/server/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import { resolveMarkdownFile } from "@plannotator/server/resolve-file";
4949
import { registerSession, unregisterSession, listSessions } from "@plannotator/server/sessions";
5050
import { openBrowser } from "@plannotator/server/browser";
5151
import { detectProjectName } from "@plannotator/server/project";
52+
import { planDenyFeedback } from "@plannotator/shared/feedback-templates";
5253
import path from "path";
5354

5455
// Embed the built HTML at compile time
@@ -369,7 +370,7 @@ if (args[0] === "sessions") {
369370
hookEventName: "PermissionRequest",
370371
decision: {
371372
behavior: "deny",
372-
message: `YOUR PLAN WAS NOT APPROVED. You MUST revise the plan to address ALL of the feedback below before calling ExitPlanMode again. Do not resubmit the same plan — use the Edit tool to make targeted changes to the plan file first.\n\n${result.feedback || "Plan changes requested"}`,
373+
message: planDenyFeedback(result.feedback || "", "ExitPlanMode"),
373374
},
374375
},
375376
})

apps/opencode-plugin/index.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
import { getGitContext, runGitDiff } from "@plannotator/server/git";
3030
import { writeRemoteShareLink } from "@plannotator/server/share-url";
3131
import { resolveMarkdownFile } from "@plannotator/server/resolve-file";
32+
import { planDenyFeedback } from "@plannotator/shared/feedback-templates";
3233

3334
// @ts-ignore - Bun import attribute for text
3435
import indexHtml from "./plannotator.html" with { type: "text" };
@@ -439,18 +440,7 @@ Proceed with implementation, incorporating these notes where applicable.`;
439440
Plan Summary: ${args.summary}
440441
${result.savedPath ? `Saved to: ${result.savedPath}` : ""}`;
441442
} else {
442-
return `Plan needs revision.
443-
${result.savedPath ? `\nSaved to: ${result.savedPath}` : ""}
444-
445-
The user has requested changes to your plan. Please review their feedback below and revise your plan accordingly.
446-
447-
## User Feedback
448-
449-
${result.feedback}
450-
451-
---
452-
453-
Please revise your plan based on this feedback and call \`submit_plan\` again when ready.`;
443+
return planDenyFeedback(result.feedback || "", "submit_plan");
454444
}
455445
},
456446
}),

apps/opencode-plugin/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
"@opencode-ai/plugin": "^1.1.10"
3939
},
4040
"devDependencies": {
41-
"@plannotator/server": "workspace:*"
41+
"@plannotator/server": "workspace:*",
42+
"@plannotator/shared": "workspace:*"
4243
},
4344
"peerDependencies": {
4445
"bun": ">=1.0.0"
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Vendored copy of packages/shared/feedback-templates.ts for source installs.
3+
* Keep this file in sync with the shared source via `bun run build:pi`.
4+
*/
5+
6+
export interface PlanDenyFeedbackOptions {
7+
planFilePath?: string;
8+
}
9+
10+
export const planDenyFeedback = (
11+
feedback: string,
12+
toolName: string = "ExitPlanMode",
13+
options?: PlanDenyFeedbackOptions,
14+
): string => {
15+
const planFileRule = options?.planFilePath
16+
? `- Read ${options.planFilePath} to see the current plan before editing it.\n`
17+
: "";
18+
19+
return `YOUR PLAN WAS NOT APPROVED.\n\nYou MUST revise the plan to address ALL of the feedback below before calling ${toolName} again.\n\nRules:\n${planFileRule}- Use the Edit tool to make targeted changes to the plan — do not resubmit the same plan unchanged.\n- Do NOT change the plan title (first # heading) unless the user explicitly asks you to.\n\n${feedback || "Plan changes requested"}`;
20+
};

apps/pi-extension/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
runGitDiff,
3636
openBrowser,
3737
} from "./server.js";
38+
import { planDenyFeedback } from "./feedback-templates.js";
3839

3940
// Load HTML at runtime (jiti doesn't support import attributes)
4041
const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -445,7 +446,7 @@ export default function plannotator(pi: ExtensionAPI): void {
445446
content: [
446447
{
447448
type: "text",
448-
text: `Plan not approved.\n\nUser feedback: ${feedbackText}\n\nRevise the plan:\n1. Read ${planFilePath} to see the current plan.\n2. Use the edit tool to make targeted changes addressing the feedback above — do not rewrite the entire file.\n3. Call exit_plan_mode again when ready.`,
449+
text: planDenyFeedback(feedbackText, "exit_plan_mode", { planFilePath }),
449450
},
450451
],
451452
details: { approved: false, feedback: feedbackText },

apps/pi-extension/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@
2424
"utils.ts",
2525
"README.md",
2626
"plannotator.html",
27-
"review-editor.html"
27+
"review-editor.html",
28+
"feedback-templates.ts"
2829
],
2930
"scripts": {
30-
"build": "cp ../hook/dist/index.html plannotator.html && cp ../hook/dist/review.html review-editor.html",
31+
"build": "cp ../hook/dist/index.html plannotator.html && cp ../hook/dist/review.html review-editor.html && cp ../../packages/shared/feedback-templates.ts feedback-templates.ts",
3132
"prepublishOnly": "cd ../.. && bun run build:pi"
3233
},
3334
"peerDependencies": {

bun.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, test, expect } from "bun:test";
2+
import { planDenyFeedback } from "./feedback-templates";
3+
4+
describe("feedback-templates", () => {
5+
/**
6+
* The whole point of this module: all three integrations (hook, opencode, pi)
7+
* produce identical output except for the tool name. If this test fails,
8+
* the templates have diverged — which is what we're trying to prevent.
9+
*/
10+
test("plan deny is identical across integrations (modulo tool name)", () => {
11+
const normalize = (s: string) =>
12+
s.replace(/ExitPlanMode|submit_plan|exit_plan_mode/g, "TOOL");
13+
14+
const feedback = "## 1. Remove auth section\n> Not needed anymore.";
15+
const hook = normalize(planDenyFeedback(feedback, "ExitPlanMode"));
16+
const opencode = normalize(planDenyFeedback(feedback, "submit_plan"));
17+
const pi = normalize(planDenyFeedback(feedback, "exit_plan_mode"));
18+
19+
expect(hook).toBe(opencode);
20+
expect(opencode).toBe(pi);
21+
});
22+
23+
/**
24+
* The deny template must embed the user's feedback verbatim — no truncation,
25+
* no escaping, no wrapping. The agent needs the raw annotation output.
26+
*/
27+
test("plan deny preserves feedback content verbatim", () => {
28+
const feedback = "## 1. Change auth\n**From:**\n```\nold code\n```\n**To:**\n```\nnew code\n```";
29+
const result = planDenyFeedback(feedback);
30+
expect(result).toContain(feedback);
31+
});
32+
33+
/**
34+
* Empty feedback should not produce a broken message — the agent needs
35+
* something actionable even if the user didn't write annotations.
36+
*/
37+
test("plan deny handles empty feedback gracefully", () => {
38+
const result = planDenyFeedback("");
39+
expect(result.length).toBeGreaterThan(50);
40+
expect(result).toBe(result.trimEnd());
41+
});
42+
43+
/**
44+
* Version history is keyed by the plan's first # heading + date.
45+
* If the agent renames the heading on resubmission, the version chain breaks
46+
* and the user loses diffs (#296). The deny template must instruct the agent
47+
* to preserve the title.
48+
*/
49+
test("plan deny instructs agent to preserve plan title", () => {
50+
const result = planDenyFeedback("feedback");
51+
expect(result.toLowerCase()).toContain("title");
52+
expect(result.toLowerCase()).toContain("heading");
53+
});
54+
55+
test("plan deny can include a plan file hint for file-based integrations", () => {
56+
const result = planDenyFeedback("feedback", "exit_plan_mode", {
57+
planFilePath: "plans/auth.md",
58+
});
59+
60+
expect(result).toContain("Read plans/auth.md to see the current plan before editing it.");
61+
expect(result).toContain("exit_plan_mode");
62+
});
63+
});
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* Shared feedback templates for all agent integrations.
3+
*
4+
* The plan deny template was tuned in #224 / commit 3dca977 to use strong
5+
* directive framing — Claude was ignoring softer phrasing.
6+
*/
7+
8+
export interface PlanDenyFeedbackOptions {
9+
planFilePath?: string;
10+
}
11+
12+
export const planDenyFeedback = (
13+
feedback: string,
14+
toolName: string = "ExitPlanMode",
15+
options?: PlanDenyFeedbackOptions,
16+
): string => {
17+
const planFileRule = options?.planFilePath
18+
? `- Read ${options.planFilePath} to see the current plan before editing it.\n`
19+
: "";
20+
21+
return `YOUR PLAN WAS NOT APPROVED.\n\nYou MUST revise the plan to address ALL of the feedback below before calling ${toolName} again.\n\nRules:\n${planFileRule}- Use the Edit tool to make targeted changes to the plan — do not resubmit the same plan unchanged.\n- Do NOT change the plan title (first # heading) unless the user explicitly asks you to.\n\n${feedback || "Plan changes requested"}`;
22+
};

0 commit comments

Comments
 (0)