diff --git a/apps/opencode-plugin/index.ts b/apps/opencode-plugin/index.ts index 0bb75c8d..6868a076 100644 --- a/apps/opencode-plugin/index.ts +++ b/apps/opencode-plugin/index.ts @@ -40,7 +40,7 @@ import { handleArchiveCommand, type CommandDeps, } from "./commands"; -import { planDenyFeedback } from "@plannotator/shared/feedback-templates"; +import { planDenyFeedback, planApproveFeedback } from "@plannotator/shared/feedback-templates"; import { normalizeEditPermission, stripConflictingPlanModeRules, @@ -473,20 +473,7 @@ Do NOT proceed with implementation until your plan is approved.`); } } - if (result.feedback) { - return `Plan approved with notes! -${result.savedPath ? `Saved to: ${result.savedPath}` : ""} - -## Implementation Notes - -The user approved your plan but added the following notes to consider during implementation: - -${result.feedback} - -Proceed with implementation, incorporating these notes where applicable.`; - } - - return `Plan approved!${result.savedPath ? ` Saved to: ${result.savedPath}` : ""}`; + return planApproveFeedback(result.feedback, result.savedPath); } else { return planDenyFeedback(result.feedback || "", "submit_plan", { planFilePath: sourceFilePath, diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index db58ef9a..db029a33 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -34,7 +34,7 @@ import { markCompletedSteps, parseChecklist, } from "./generated/checklist.js"; -import { planDenyFeedback } from "./generated/feedback-templates.js"; +import { planDenyFeedback, DECISIONS_LOG_NOTE } from "./generated/feedback-templates.js"; import { hasMarkdownFiles } from "./generated/resolve-file.js"; import { FILE_BROWSER_EXCLUDED } from "./generated/reference-common.js"; import { openBrowser } from "./server/network.js"; @@ -666,7 +666,7 @@ export default function plannotator(pi: ExtensionAPI): void { content: [ { type: "text", - text: `Plan approved with notes! You now have full tool access (read, bash, edit, write). Execute the plan in ${planFilePath}. ${doneMsg}\n\n## Implementation Notes\n\nThe user approved your plan but added the following notes to consider during implementation:\n\n${result.feedback}\n\nProceed with implementation, incorporating these notes where applicable.`, + text: `Plan approved with notes! You now have full tool access (read, bash, edit, write). Execute the plan in ${planFilePath}. ${doneMsg}\n\n## Implementation Notes\n\nThe user approved your plan but added the following notes to consider during implementation:\n\n${result.feedback}\n\nProceed with implementation, incorporating these notes where applicable.${DECISIONS_LOG_NOTE}`, }, ], details: { approved: true, feedback: result.feedback }, @@ -677,7 +677,7 @@ export default function plannotator(pi: ExtensionAPI): void { content: [ { type: "text", - text: `Plan approved. You now have full tool access (read, bash, edit, write). Execute the plan in ${planFilePath}. ${doneMsg}`, + text: `Plan approved. You now have full tool access (read, bash, edit, write). Execute the plan in ${planFilePath}. ${doneMsg}${DECISIONS_LOG_NOTE}`, }, ], details: { approved: true }, diff --git a/packages/shared/feedback-templates.test.ts b/packages/shared/feedback-templates.test.ts index 47564a00..aa8ed64b 100644 --- a/packages/shared/feedback-templates.test.ts +++ b/packages/shared/feedback-templates.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { planDenyFeedback } from "./feedback-templates"; +import { planDenyFeedback, planApproveFeedback, DECISIONS_LOG_NOTE } from "./feedback-templates"; describe("feedback-templates", () => { /** @@ -63,3 +63,67 @@ describe("feedback-templates", () => { }); }); + +describe("context anchoring", () => { + /** + * On denial, the agent must be instructed to maintain a Decisions Log + * so that rejected approaches are documented and not re-proposed. + */ + test("plan deny includes context anchoring instructions", () => { + const result = planDenyFeedback("some feedback"); + expect(result).toContain("Decisions Log"); + expect(result).toContain("Rejected:"); + expect(result).toContain("cross-session memory"); + }); + + /** + * On approval, the agent must be reminded to reference the Decisions Log + * during implementation — closing the context anchoring loop. + */ + test("plan approve includes Decisions Log reminder", () => { + const result = planApproveFeedback(); + expect(result).toContain("Plan approved"); + expect(result).toContain("Decisions Log"); + }); + + /** + * Approval with notes must signal "with notes" in the header so the + * agent knows content follows, and include both the notes and the + * Decisions Log reminder. + */ + test("plan approve with notes includes both notes and Decisions Log reminder", () => { + const result = planApproveFeedback("Use the adapter pattern here."); + expect(result).toContain("Plan approved with notes!"); + expect(result).toContain("Implementation Notes"); + expect(result).toContain("Use the adapter pattern here."); + expect(result).toContain("Decisions Log"); + }); + + /** + * Approval without notes should NOT say "with notes". + */ + test("plan approve without notes does not include 'with notes' in header", () => { + const result = planApproveFeedback(); + expect(result).toContain("Plan approved!"); + expect(result).not.toContain("with notes"); + }); + + /** + * DECISIONS_LOG_NOTE is exported as a named constant so Pi and other + * integrations that compose their own approval messages can include it + * without duplicating the text. + */ + test("DECISIONS_LOG_NOTE is a non-empty string containing 'Decisions Log'", () => { + expect(typeof DECISIONS_LOG_NOTE).toBe("string"); + expect(DECISIONS_LOG_NOTE.length).toBeGreaterThan(0); + expect(DECISIONS_LOG_NOTE).toContain("Decisions Log"); + }); + + /** + * Approval with saved path must surface the file path. + */ + test("plan approve with savedPath includes the path", () => { + const result = planApproveFeedback(undefined, "/tmp/plans/auth.md"); + expect(result).toContain("/tmp/plans/auth.md"); + }); +}); diff --git a/packages/shared/feedback-templates.ts b/packages/shared/feedback-templates.ts index 8d6c3426..8beed2a3 100644 --- a/packages/shared/feedback-templates.ts +++ b/packages/shared/feedback-templates.ts @@ -9,6 +9,25 @@ export interface PlanDenyFeedbackOptions { planFilePath?: string; } +/** + * Appended to all approval messages so the agent knows to carry the + * Decisions Log forward into implementation. + */ +export const DECISIONS_LOG_NOTE = `\n\nIf your plan contains a \`## Decisions Log\`, keep it as a reference during implementation — it documents the rejected alternatives that shaped this design.`; + +export const planApproveFeedback = ( + notes?: string, + savedPath?: string, +): string => { + const header = `Plan approved${notes ? " with notes" : ""}!`; + const savedNote = savedPath ? `\nSaved to: ${savedPath}` : ""; + const notesSection = notes + ? `\n\n## Implementation Notes\n\nThe user approved your plan but added the following notes to consider during implementation:\n\n${notes}\n\nProceed with implementation, incorporating these notes where applicable.` + : ""; + + return `${header}${savedNote}${notesSection}${DECISIONS_LOG_NOTE}`; +}; + export const planDenyFeedback = ( feedback: string, toolName: string = "ExitPlanMode", @@ -18,5 +37,7 @@ export const planDenyFeedback = ( ? `- Your plan is saved at: ${options.planFilePath}\n You can edit this file to make targeted changes, then pass its path to ${toolName}.\n` : ""; - 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}- 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"}`; + const contextAnchoringInstructions = `\n## Context Anchoring\n\nBefore revising your plan:\n1. Add (or update) a \`## Decisions Log\` section at the bottom of the plan.\n2. For each rejected approach from this feedback, add an entry:\n - **Rejected:** [brief description of the rejected approach] **Why:** [reason from this feedback]\n3. Once added, do NOT re-propose approaches listed in the Decisions Log — it is your cross-session memory.\n`; + + 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}- Do not resubmit the same plan unchanged.\n- Do NOT change the plan title (first # heading) unless the user explicitly asks you to.\n${contextAnchoringInstructions}\n${feedback || "Plan changes requested"}`; };