feat(coding-agent): add approve and keep context plan option
Fixes #948
This commit is contained in:
@@ -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<void> {
|
||||
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://<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 (!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)}`,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user