From 4e3ff73d8970b7d0c97c95db2ebdd2aae245d6a2 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 6 May 2026 17:12:24 +0200 Subject: [PATCH] feat(coding-agent): add approve and keep context plan option Fixes #948 --- .../src/modes/interactive-mode.ts | 31 ++++--- .../src/prompts/system/plan-mode-active.md | 10 ++- .../src/prompts/system/plan-mode-approved.md | 6 ++ .../test/interactive-mode-plan-review.test.ts | 87 +++++++++++++++++++ 4 files changed, 119 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index aaaa67974..5120f75db 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -1060,7 +1060,7 @@ export class InteractiveMode implements InteractiveModeContext { async #approvePlan( planContent: string, - options: { planFilePath: string; finalPlanFilePath: string }, + options: { planFilePath: string; finalPlanFilePath: string; preserveContext?: boolean }, ): Promise { await renameApprovedPlanFile({ planFilePath: options.planFilePath, @@ -1070,14 +1070,16 @@ export class InteractiveMode implements InteractiveModeContext { }); const previousTools = this.#planModePreviousTools ?? this.session.getActiveToolNames(); await this.#exitPlanMode({ silent: true, paused: false }); - await this.handleClearCommand(); - // The new session has a fresh local:// root — persist the approved plan there - // so `local://.md` resolves correctly in the execution session. - const newLocalPath = resolveLocalUrlToPath(options.finalPlanFilePath, { - getArtifactsDir: () => this.sessionManager.getArtifactsDir(), - getSessionId: () => this.sessionManager.getSessionId(), - }); - await Bun.write(newLocalPath, planContent); + if (!options.preserveContext) { + await this.handleClearCommand(); + // The new session has a fresh local:// root — persist the approved plan there + // so `local://<title>.md` resolves correctly in the execution session. + const newLocalPath = resolveLocalUrlToPath(options.finalPlanFilePath, { + getArtifactsDir: () => this.sessionManager.getArtifactsDir(), + getSessionId: () => this.sessionManager.getSessionId(), + }); + await Bun.write(newLocalPath, planContent); + } if (previousTools.length > 0) { await this.session.setActiveToolsByName(previousTools); } @@ -1086,6 +1088,7 @@ export class InteractiveMode implements InteractiveModeContext { const planModePrompt = prompt.render(planModeApprovedPrompt, { planContent, finalPlanFilePath: options.finalPlanFilePath, + contextPreserved: options.preserveContext === true, }); await this.session.prompt(planModePrompt, { synthetic: true }); } @@ -1129,14 +1132,14 @@ export class InteractiveMode implements InteractiveModeContext { this.#renderPlanPreview(planContent); const choice = await this.showHookSelector( "Plan mode - next step", - ["Approve and execute", "Refine plan", "Stay in plan mode"], + ["Approve and execute", "Approve and keep context", "Refine plan", "Stay in plan mode"], { helpText: this.#getPlanReviewHelpText(), onExternalEditor: () => void this.#openPlanInExternalEditor(planFilePath), }, ); - if (choice === "Approve and execute") { + if (choice === "Approve and execute" || choice === "Approve and keep context") { const finalPlanFilePath = details.finalPlanFilePath || planFilePath; try { const latestPlanContent = await this.#readPlanFile(planFilePath); @@ -1144,7 +1147,11 @@ export class InteractiveMode implements InteractiveModeContext { this.showError(`Plan file not found at ${planFilePath}`); return; } - await this.#approvePlan(latestPlanContent, { planFilePath, finalPlanFilePath }); + await this.#approvePlan(latestPlanContent, { + planFilePath, + finalPlanFilePath, + preserveContext: choice === "Approve and keep context", + }); } catch (error) { this.showError( `Failed to finalize approved plan: ${error instanceof Error ? error.message : String(error)}`, diff --git a/packages/coding-agent/src/prompts/system/plan-mode-active.md b/packages/coding-agent/src/prompts/system/plan-mode-active.md index 199707952..3ef126d3f 100644 --- a/packages/coding-agent/src/prompts/system/plan-mode-active.md +++ b/packages/coding-agent/src/prompts/system/plan-mode-active.md @@ -6,7 +6,7 @@ You **MUST NOT**: - Run state-changing commands (git commit, npm install, etc.) - Make any system changes -To implement: call `{{exitToolName}}` → user approves → new session starts with full write access to execute the plan. +To implement: call `{{exitToolName}}` → user approves an execution option → full write access is restored to execute the plan. You **MUST NOT** ask the user to exit plan mode for you; you **MUST** call `{{exitToolName}}` yourself. </critical> @@ -21,7 +21,11 @@ You **MUST** create a plan at `{{planFilePath}}`. You **MUST** use `{{editToolName}}` for incremental updates; use `{{writeToolName}}` only for create/full replace. <caution> -Plan execution runs in fresh context (session cleared). You **MUST** make the plan file self-contained: include requirements, decisions, key findings, remaining todos needed to continue without prior session history. +The approval selector includes: +- **Approve and execute**: starts execution in fresh context (session cleared). +- **Approve and keep context**: starts execution in this session, preserving exploration history. + +You **MUST** still make the plan file self-contained: include requirements, decisions, key findings, and remaining todos needed to continue without prior session history. </caution> {{#if reentry}} @@ -100,7 +104,7 @@ You **MUST** ask questions throughout. You **MUST NOT** make large assumptions a <critical> Your turn ends ONLY by: 1. Using `{{askToolName}}` to gather information, OR -2. Calling `{{exitToolName}}` when ready — this triggers user approval, then a new implementation session with full tool access +2. Calling `{{exitToolName}}` when ready — this triggers user approval, then implementation with full tool access You **MUST NOT** ask plan approval via text or `{{askToolName}}`; you **MUST** use `{{exitToolName}}`. You **MUST** keep going until complete. diff --git a/packages/coding-agent/src/prompts/system/plan-mode-approved.md b/packages/coding-agent/src/prompts/system/plan-mode-approved.md index 855140ece..55a43e6c2 100644 --- a/packages/coding-agent/src/prompts/system/plan-mode-approved.md +++ b/packages/coding-agent/src/prompts/system/plan-mode-approved.md @@ -3,6 +3,12 @@ Plan approved. You **MUST** execute it now. </critical> Finalized plan artifact: `{{finalPlanFilePath}}` +{{#if contextPreserved}} +Context was preserved for execution. Use the existing conversation history when it is useful, and treat the finalized plan as the source of truth if it conflicts with earlier exploration. +{{else}} +Execution may be running in fresh context. Treat the finalized plan as the source of truth. +{{/if}} + ## Plan diff --git a/packages/coding-agent/test/interactive-mode-plan-review.test.ts b/packages/coding-agent/test/interactive-mode-plan-review.test.ts index f14359b31..30f988fd7 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -96,4 +96,91 @@ describe("InteractiveMode plan review rendering", () => { expect(mode.chatContainer.children.at(-2)).toBe(marker); expect(firstPreview!.render(120).join("\n")).toContain("Second plan"); }); + + it("offers approve-and-keep-context as a distinct plan approval path", async () => { + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nDo the thing."); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + const selector = vi.spyOn(mode, "showHookSelector").mockResolvedValue("Stay in plan mode"); + + await mode.handleExitPlanModeTool({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath: "local://APPROVED.md", + }); + + expect(selector).toHaveBeenCalledWith( + "Plan mode - next step", + ["Approve and execute", "Approve and keep context", "Refine plan", "Stay in plan mode"], + expect.any(Object), + ); + }); + + it("approves a plan without clearing the session when keeping context", async () => { + const planFilePath = "local://PLAN.md"; + const finalPlanFilePath = "local://APPROVED.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + const resolvedFinalPlanPath = resolveLocalUrlToPath(finalPlanFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nKeep context."); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and keep context"); + const clear = vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); + const prompt = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); + + await mode.handleExitPlanModeTool({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath, + }); + + expect(clear).not.toHaveBeenCalled(); + expect(await Bun.file(resolvedFinalPlanPath).text()).toBe("# Plan\n\nKeep context."); + expect(prompt).toHaveBeenCalledWith(expect.stringContaining("Context was preserved for execution."), { + synthetic: true, + }); + }); + + it("keeps the existing approve-and-execute path clearing the session", async () => { + const planFilePath = "local://PLAN.md"; + const finalPlanFilePath = "local://APPROVED.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nClear context."); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + vi.spyOn(mode, "showHookSelector").mockResolvedValue("Approve and execute"); + const clear = vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); + const prompt = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); + + await mode.handleExitPlanModeTool({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath, + }); + + expect(clear).toHaveBeenCalledTimes(1); + expect(prompt).toHaveBeenCalledWith(expect.stringContaining("Execution may be running in fresh context."), { + synthetic: true, + }); + }); });