Skip to content

Commit c7f7323

Browse files
authored
🤖 fix: preserve plan file after propose_plan Start Here (#1290)
Fixes a case where clicking **Start Here** on a proposed plan deletes the plan markdown file, preventing post-compaction `plan_file_reference` attachments. - Stop requesting `deletePlanFile` when starting from a proposed plan - Include the plan path in the Start Here message (when known) - Add a unit test covering the Start Here hook inputs Validation: `make static-check`. --- <details> <summary>📋 Implementation Plan</summary> # Preserve plan file across compactions (post-compaction attachments) ## What’s happening (root cause) Compaction itself **does not delete the plan file**. - **Compaction path**: `CompactionHandler.performCompaction()` clears only `chat.jsonl` via `HistoryService.clearHistory()` and then appends a summary message. It never calls the workspace-level history helpers that clean up plan files. - `src/node/services/compactionHandler.ts` → `historyService.clearHistory()` - `src/node/services/historyService.ts` → `truncateHistory(..., 1.0)` (deletes `chat.jsonl` only) The plan markdown file is deleted by *other* “reset-like” flows: 1) **“Start Here” on a proposed plan** (most likely): - UI: `ProposePlanToolCall.tsx` calls `useStartHere(..., { deletePlanFile: true })`. - ORPC: `api.workspace.replaceChatHistory({ deletePlanFile: true })`. - Backend: `WorkspaceService.replaceHistory(..., { deletePlanFile: true })` → `deletePlanFilesForWorkspace()`. 2) **Full history clear**: `WorkspaceService.truncateHistory(workspaceId, 1.0)` also deletes the plan file. Because the proposed-plan “Start Here” flow deletes the plan file, later compactions can’t generate the `plan_file_reference` post-compaction attachment (AttachmentService reads the plan file from disk). ## Repro (current behavior) 1. Enter Plan Mode and write a plan (plan file exists under `~/.mux/plans/<project>/<workspace>.md`). 2. Call `propose_plan`. 3. Click **Start Here** in the plan tool UI. 4. Observe the plan file is removed from `~/.mux/plans/...`. 5. Later, when compaction happens, `AttachmentService.generatePlanFileReference()` returns `null` (file missing), so the post-compaction attachment does not include the plan. ## Fix options ### Option A (recommended): Don’t delete the plan file on propose_plan “Start Here” **Net LoC (product code): ~+0 to +10** 1. **Frontend:** keep the plan file on disk when the user clicks **Start Here**: - `src/browser/components/tools/ProposePlanToolCall.tsx` - Change `useStartHere(..., { deletePlanFile: true })` → omit the option (or pass `{ deletePlanFile: false }`). 2. **Frontend UX (recommended):** add a small note into the Start Here content that the plan is still stored at the plan file path. - Keep it short and conditional on `planPath` being known (so it doesn’t break legacy/edge cases). - Example copy: `*Plan file preserved at: <path>*`. 3. **Backend comment cleanup:** update/remove the comment in `WorkspaceService.replaceHistory()` that claims propose_plan Start Here deletes the plan file. 4. (Optional) Keep the backend `deletePlanFile` plumbing intact (it may still be useful for a future explicit “delete plan file” action). Why this works: post-compaction attachments read the plan file from disk (`AttachmentService.generatePlanFileReference`), and the attachment system is explicitly “mode-agnostic” (valuable in exec mode too). ### Option B: Keep deleting the plan file, but persist a “plan snapshot” for compaction **Net LoC (product code): ~+120–200** Store an immutable snapshot at Start Here time (e.g., `~/.mux/sessions/<ws>/plan-snapshot.md`), and have `AttachmentService` fall back to this snapshot when the plan file is missing. This keeps the “no lingering plan files” behavior but still preserves plan context for post-compaction attachments. <details> <summary>Details (Option B)</summary> - On Start Here (propose plan), write snapshot before deleting plan file. - Update `AttachmentService.generatePlanFileReference()` to: 1) read `getPlanFilePath()` 2) read legacy path 3) read snapshot path - Decide cleanup rules (delete snapshot on full clear, workspace delete, etc.). </details> ### Option C (defensive hardening): If plan file is missing, don’t filter plan diffs out **Net LoC (product code): ~+10–25** In `AttachmentService.generatePostCompactionAttachments()`, only filter out plan paths from `edited_files_reference` **if** a `plan_file_reference` attachment was successfully created. This doesn’t fix the Start Here case (history was replaced, so there are no plan diffs to extract), but it prevents *other* “plan file missing” scenarios from silently dropping plan context. ## Tests / validation 1. Manual: - Run through the repro steps and confirm the plan file still exists after Start Here. - Confirm the Start Here message includes the “plan file preserved at …” note (with the correct path). - Trigger compaction and verify a `plan_file_reference` post-compaction attachment is injected. 2. Automated (pick one): - Browser unit test: mount `ProposePlanToolCall` and assert `replaceChatHistory` is called with `deletePlanFile: undefined/false`, and that the `summaryMessage` content includes the plan path note. - Node unit test: verify `WorkspaceService.replaceHistory(..., { deletePlanFile: true })` deletes plan file, and `deletePlanFile: false` does not (guard against regressions). </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: openai:gpt-5.2 • Thinking: xhigh_
1 parent 07a55f2 commit c7f7323

File tree

3 files changed

+103
-5
lines changed

3 files changed

+103
-5
lines changed
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import React from "react";
2+
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
3+
import { GlobalWindow } from "happy-dom";
4+
import { cleanup, render } from "@testing-library/react";
5+
6+
import { TooltipProvider } from "../ui/tooltip";
7+
8+
import { ProposePlanToolCall } from "./ProposePlanToolCall";
9+
10+
let startHereCalls: Array<{
11+
workspaceId: string | undefined;
12+
content: string;
13+
isCompacted: boolean;
14+
options: { deletePlanFile?: boolean } | undefined;
15+
}> = [];
16+
17+
const useStartHereMock = mock(
18+
(
19+
workspaceId: string | undefined,
20+
content: string,
21+
isCompacted: boolean,
22+
options?: { deletePlanFile?: boolean }
23+
) => {
24+
startHereCalls.push({ workspaceId, content, isCompacted, options });
25+
return {
26+
openModal: () => undefined,
27+
isStartingHere: false,
28+
buttonLabel: "Start Here",
29+
buttonEmoji: "",
30+
disabled: false,
31+
modal: null,
32+
};
33+
}
34+
);
35+
36+
void mock.module("@/browser/hooks/useStartHere", () => ({
37+
useStartHere: useStartHereMock,
38+
}));
39+
40+
void mock.module("@/browser/contexts/API", () => ({
41+
useAPI: () => ({ api: null, status: "connected" as const, error: null }),
42+
}));
43+
44+
void mock.module("@/browser/hooks/useOpenInEditor", () => ({
45+
useOpenInEditor: () => () => Promise.resolve({ success: true } as const),
46+
}));
47+
48+
void mock.module("@/browser/contexts/WorkspaceContext", () => ({
49+
useWorkspaceContext: () => ({
50+
workspaceMetadata: new Map<string, { runtimeConfig?: unknown }>(),
51+
}),
52+
}));
53+
54+
describe("ProposePlanToolCall Start Here", () => {
55+
beforeEach(() => {
56+
startHereCalls = [];
57+
globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis;
58+
globalThis.document = globalThis.window.document;
59+
});
60+
61+
afterEach(() => {
62+
cleanup();
63+
globalThis.window = undefined as unknown as Window & typeof globalThis;
64+
globalThis.document = undefined as unknown as Document;
65+
});
66+
67+
test("keeps plan file on disk and includes plan path note in Start Here content", () => {
68+
const planPath = "~/.mux/plans/demo/ws-123.md";
69+
70+
render(
71+
<TooltipProvider>
72+
<ProposePlanToolCall
73+
args={{}}
74+
result={{
75+
success: true,
76+
planPath,
77+
// Old-format chat history may include planContent; this is the easiest path to
78+
// ensure the rendered Start Here message includes the full plan + the path note.
79+
planContent: "# My Plan\n\nDo the thing.",
80+
}}
81+
workspaceId="ws-123"
82+
isLatest={false}
83+
/>
84+
</TooltipProvider>
85+
);
86+
87+
expect(startHereCalls.length).toBe(1);
88+
expect(startHereCalls[0]?.options).toBeUndefined();
89+
expect(startHereCalls[0]?.isCompacted).toBe(false);
90+
91+
// The Start Here message should explicitly tell the user the plan file remains on disk.
92+
expect(startHereCalls[0]?.content).toContain("*Plan file preserved at:*");
93+
expect(startHereCalls[0]?.content).toContain(planPath);
94+
});
95+
});

src/browser/components/tools/ProposePlanToolCall.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,11 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
223223
planTitle = "Plan";
224224
}
225225

226-
// Format: Title as H1 + plan content for "Start Here" functionality
227-
const startHereContent = `# ${planTitle}\n\n${planContent}`;
226+
// Format: Title as H1 + plan content for "Start Here" functionality.
227+
// Note: we intentionally preserve the plan file on disk when starting here so it can be
228+
// referenced later (e.g., via post-compaction attachments).
229+
const planPathNote = planPath ? `\n\n---\n\n*Plan file preserved at:* \`${planPath}\`` : "";
230+
const startHereContent = `# ${planTitle}\n\n${planContent}${planPathNote}`;
228231
const {
229232
openModal,
230233
buttonLabel,
@@ -233,8 +236,7 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
233236
} = useStartHere(
234237
workspaceId,
235238
startHereContent,
236-
false, // Plans are never already compacted
237-
{ deletePlanFile: true }
239+
false // Plans are never already compacted
238240
);
239241

240242
// Copy to clipboard with feedback

src/node/services/workspaceService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1921,7 +1921,8 @@ export class WorkspaceService extends EventEmitter {
19211921
}
19221922

19231923
// Optional cleanup: delete plan file when caller explicitly requests it.
1924-
// Used by propose_plan "Start Here" to prevent stale plans from lingering on disk.
1924+
// Note: the propose_plan UI keeps the plan file on disk; this flag is reserved for
1925+
// explicit reset flows and backwards compatibility.
19251926
if (options?.deletePlanFile === true) {
19261927
const metadata = await this.getInfo(workspaceId);
19271928
if (metadata) {

0 commit comments

Comments
 (0)