🤖 fix: delete plan file on Start Here + read-only exec (#1116)

### What
- When clicking **Start Here** on a `propose_plan` tool call, we now
delete the plan file on disk (new + legacy paths) and clear plan file
tracking state.
- The plan file is now **readable in exec mode** via `file_read` (we
always pass `planFilePath`, and plan path detection is mode-agnostic).
- The plan file is now explicitly **read-only outside plan mode**,
including for SSH runtimes.

### Tests
- `make static-check`
- `bun test src/node/services/tools/file_read.test.ts
src/node/services/tools/file_edit_operation.test.ts
src/node/runtime/SSHRuntime.test.ts`

---

<details>
<summary>📋 Implementation Plan</summary>

# 🤖 Plan: Delete plan file on “Start Here” + make plan file readable
(read-only) in Exec mode

## Problem
1. **Plan file is left behind after using “Start Here” on a
`propose_plan` tool call.**
- The chat history is replaced, but
`~/.mux/plans/<project>/<workspace>.md` remains on disk, confusing later
sessions (stale/duplicate plan).
2. **Switching from Plan → Exec currently blocks reading the plan
file.**
- `file_read` only exempts the plan file from `cwd`-restriction in
**plan mode**; in exec mode the plan file path resolves outside the
workspace and is rejected.

## Goals
- When the user clicks **Start Here** on a `propose_plan` tool call,
delete the plan file from disk (new + legacy paths) and clear plan file
tracking state.
- In **exec mode**, allow the agent to **read** the plan file via
`file_read`, but **never write/modify** it (treat as read-only outside
plan mode).
- Preserve existing safety: agents still can’t read arbitrary files
outside the workspace; only the plan file is exempt.
- Support **local + SSH runtimes**.

## Non-goals
- Changing where plan files are stored or the plan file naming scheme.
- Auto-deleting the plan file on every “Start Here” action (only when
invoked from `propose_plan`).

---

## Recommended approach (A): Add `deletePlanFile` option to
`workspace.replaceChatHistory`
**Net LoC (product code): ~120–180**

### Why this approach
- Single round-trip: “Start Here” + plan cleanup happens in one backend
call.
- Keeps deletion scoped to the UI that knows the intent (`propose_plan`
Start Here).
- Avoids fragile heuristics (e.g., inferring intent from message ID
prefix).

### Implementation steps

#### 1) Allow plan file *read* in exec mode (but keep edits blocked)
1. **Pass `planFilePath` in tool config in all modes**
   - File: `src/node/services/aiService.ts`
   - Change tool config from:
     - `planFilePath: mode === "plan" ? planFilePath : undefined`
     - to `planFilePath: planFilePath` (always set)
- Rationale: tools need to know the canonical plan path even in exec
mode.

2. **Refactor plan path detection helper(s)**
   - File: `src/node/services/tools/fileCommon.ts`
- Add a helper that checks whether `targetPath` equals the configured
plan file path **regardless of mode**, using `runtime.resolvePath` for
tilde/absolute equivalence.
   - Keep the existing plan-mode semantics available for edit tools.
   - Suggested shape:
- `isConfiguredPlanFilePath(targetPath, config)` → ignores
`config.mode`, requires `config.planFilePath`.
- `isPlanFilePathInPlanMode(targetPath, config)` → `config.mode ===
"plan" && isConfiguredPlanFilePath(...)`.

3. **Update `file_read` to allow reading the plan file in any mode**
   - File: `src/node/services/tools/file_read.ts`
   - Replace the current exemption check:
- `if (!(await isPlanFilePath(filePath, config))) {
validatePathInCwd(...) }`
   - With:
- `if (!(await isConfiguredPlanFilePath(filePath, config))) {
validatePathInCwd(...) }`

4. **(Defensive) Make plan file explicitly read-only outside plan mode**
   - Files:
     - `src/node/services/tools/file_edit_operation.ts`
     - `src/node/services/tools/file_edit_insert.ts`
   - Add an early check:
- If `isConfiguredPlanFilePath(filePath, config)` and `config.mode !==
"plan"`, return a clear error like:
       - `Plan file is read-only outside plan mode: <path>`
- This prevents future regressions where someone might accidentally add
a read-exemption to edits too.

#### 2) Delete the plan file when “Start Here” is used on `propose_plan`
5. **Extend ORPC schema for `replaceChatHistory`**
   - File: `src/common/orpc/schemas/api.ts`
   - Add an optional flag:
     - `deletePlanFile: z.boolean().optional()`
   - Keep default behavior unchanged when omitted.

6. **Plumb the flag through the ORPC router**
   - File: `src/node/orpc/router.ts`
   - Pass the flag to the workspace service:
- `replaceHistory(workspaceId, summaryMessage, { deletePlanFile })`.

7. **Implement deletion in `WorkspaceService.replaceHistory`**
   - File: `src/node/services/workspaceService.ts`
   - Update signature to accept an optional options object.
   - If `deletePlanFile === true`:
     - Delete both:
       - `getPlanFilePath(metadata.name, metadata.projectName)`
       - `getLegacyPlanFilePath(workspaceId)`
- Use the existing local/SSH-safe deletion technique already used in
`truncateHistory`:
- `rm -f <quotedNewPath> <quotedLegacyPath>` via `runtime.exec(...)`
     - Clear plan file tracking state:
       - `this.sessions.get(workspaceId)?.clearFileState()`
- **Refactor**: extract the duplicated plan deletion logic currently
embedded in `truncateHistory` into a private helper (e.g.,
`deletePlanFilesForWorkspace(...)`) and call it from both places.

8. **Update frontend “Start Here” call site for plans**
   - Files:
     - `src/browser/hooks/useStartHere.ts`
     - `src/browser/components/tools/ProposePlanToolCall.tsx`
- Extend `useStartHere(...)` to accept optional `replaceChatHistory`
options.
   - In `ProposePlanToolCall`, pass `{ deletePlanFile: true }`.
- Keep `AssistantMessage` usage unchanged (no flag), so ordinary “Start
Here” does not delete the plan.

---

## Alternative approach (B): Add a separate `workspace.deletePlanFile`
endpoint
**Net LoC (product code): ~140–220**

### Summary
- Keep `replaceChatHistory` unchanged.
- Add a new endpoint that deletes plan files (new + legacy) + clears
file tracking state.
- `ProposePlanToolCall` would call `replaceChatHistory(...)` and then
`deletePlanFile(...)`.

### Pros / cons
- **Pros**: avoids touching an existing API signature.
- **Cons**: two round-trips; harder to make the behavior feel atomic;
more UI error-handling complexity.

---

## Test plan

### Unit tests
1. **`file_read` can read plan file in exec mode**
   - File: `src/node/services/tools/file_read.test.ts`
   - Create a temp plan file outside `cwd`.
   - Configure tool with `mode: "exec"`, `planFilePath: <planPath>`.
- Assert `file_read` succeeds for the plan file and still rejects other
outside-cwd paths.

2. **Plan file is not editable outside plan mode**
   - Files:
     - `src/node/services/tools/file_edit_operation.test.ts`
     - (optionally) `src/node/services/tools/file_edit_insert.test.ts`
- With `mode: "exec"` and `planFilePath` set, attempt to edit plan file
path.
   - Expect the explicit “read-only outside plan mode” error.

### Integration tests (IPC)
3. **`replaceChatHistory(deletePlanFile: true)` deletes plan file**
- Add a test near existing plan tests (e.g.,
`tests/ipc/planCommands.test.ts` or a new
`tests/ipc/startHerePlanCleanup.test.ts`).
- Create workspace, create plan file at the expected location, call
`replaceChatHistory` with the flag.
   - Assert:
     - Plan file no longer exists.
     - Subsequent `getPlanContent` returns `success: false`.

---

## Manual verification checklist
- Plan mode: create/edit plan file, call `propose_plan`.
- Click **Start Here** on the plan tool call:
  - Chat history is replaced.
  - Plan file is deleted from `~/.mux/plans/<project>/<workspace>.md`.
- Switch to exec mode without using Start Here:
  - `file_read` can read the plan file.
  - `file_edit_*` attempting to modify the plan file fails (read-only).

---

## Rollout / compatibility notes
- This change is backward compatible: the new `deletePlanFile` flag is
optional.
- Existing “Start Here” on normal assistant messages is unaffected.
- Plan file deletion includes both new and legacy paths to handle
migrated workspaces.

</details>

---
_Generated with `mux`_

Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

src/browser/components/tools/ProposePlanToolCall.tsx

@@ -223,7 +223,8 @@ export const ProposePlanToolCall: React.FC<ProposePlanToolCallProps> = (props) =
   } = useStartHere(
     workspaceId,
     startHereContent,
-    false // Plans are never already compacted
+    false, // Plans are never already compacted
+    { deletePlanFile: true }
   );
 
   // Copy to clipboard with feedback

src/browser/hooks/useStartHere.ts

@@ -12,11 +12,13 @@ import { useAPI } from "@/browser/contexts/API";
  * @param workspaceId - Current workspace ID (required for operation)
  * @param content - Content to use as the new conversation starting point
  * @param isCompacted - Whether the message is already compacted (disables button if true)
+ * @param options - Optional behavior flags for this Start Here action
  */
 export function useStartHere(
   workspaceId: string | undefined,
   content: string,
-  isCompacted = false
+  isCompacted = false,
+  options?: { deletePlanFile?: boolean }
 ) {
   const { api } = useAPI();
   const [isModalOpen, setIsModalOpen] = useState(false);
@@ -52,6 +54,7 @@ export function useStartHere(
       const result = await api.workspace.replaceChatHistory({
         workspaceId,
         summaryMessage,
+        deletePlanFile: options?.deletePlanFile,
       });
 
       if (!result.success) {

src/common/orpc/schemas/api.ts

@@ -272,6 +272,8 @@ export const workspace = {
     input: z.object({
       workspaceId: z.string(),
       summaryMessage: MuxMessageSchema,
+      /** When true, delete the plan file (new + legacy paths) and clear plan tracking state. */
+      deletePlanFile: z.boolean().optional(),
     }),
     output: ResultSchema(z.void(), z.string()),
   },

src/node/orpc/router.ts

@@ -409,7 +409,8 @@ export const router = (authToken?: string) => {
         .handler(async ({ context, input }) => {
           const result = await context.workspaceService.replaceHistory(
             input.workspaceId,
-            input.summaryMessage
+            input.summaryMessage,
+            { deletePlanFile: input.deletePlanFile }
           );
           if (!result.success) {
             return { success: false, error: result.error };

src/node/runtime/SSHRuntime.ts

@@ -416,10 +416,15 @@ export class SSHRuntime implements Runtime {
     };
   }
   async resolvePath(filePath: string): Promise<string> {
-    // Use shell to expand tildes on remote system
-    // Bash will expand ~ automatically when we echo the unquoted variable
-    // This works with BusyBox (doesn't require GNU coreutils)
-    const command = `bash -c 'p=${shescape.quote(filePath)}; echo $p'`;
+    // Expand tilde on the remote system.
+    // IMPORTANT: This must not single-quote a "~" path directly, because quoted tildes won't expand.
+    // We reuse expandTildeForSSH() to produce a "$HOME"-based, bash-safe expression.
+    //
+    // Note: This does not attempt to canonicalize relative paths (no filesystem access).
+    // It only ensures ~ is expanded so callers can compare against absolute paths.
+    const script = `echo ${expandTildeForSSH(filePath)}`;
+    const command = `bash -c ${shescape.quote(script)}`;
+
     // Use 10 second timeout for path resolution to allow for slower SSH connections
     return this.execSSHCommand(command, 10000);
   }

src/node/services/aiService.ts

@@ -1154,9 +1154,11 @@ export class AIService extends EventEmitter {
           ),
           runtimeTempDir,
           backgroundProcessManager: this.backgroundProcessManager,
-          // Plan mode configuration for path enforcement
+          // Plan/exec mode configuration for plan file access.
+          // - read: plan file is readable in all modes (useful context)
+          // - write: enforced by file_edit_* tools (plan file is read-only outside plan mode)
           mode: mode as UIMode | undefined,
-          planFilePath: mode === "plan" ? planFilePath : undefined,
+          planFilePath,
           workspaceId,
           // External edit detection callback
           recordFileState,

src/node/services/tools/fileCommon.ts

@@ -30,22 +30,25 @@ export function generateDiff(filePath: string, oldContent: string, newContent: s
 }
 
 /**
- * Check if a file path is the plan file in plan mode.
+ * Check if a file path is the configured plan file (any mode).
  * Uses runtime.resolvePath to properly expand tildes for comparison.
  *
+ * Why mode-agnostic: the plan file is useful context in both plan + exec modes,
+ * but should only be writable in plan mode.
+ *
  * @param targetPath - The path being accessed (may contain ~ or be absolute)
- * @param config - Tool configuration containing mode and planFilePath
- * @returns true if this is the plan file in plan mode
+ * @param config - Tool configuration containing planFilePath
+ * @returns true if this is the configured plan file
  */
 export async function isPlanFilePath(
   targetPath: string,
   config: ToolConfiguration
 ): Promise<boolean> {
-  if (config.mode !== "plan" || !config.planFilePath) {
+  if (!config.planFilePath) {
     return false;
   }
-  // Resolve both paths to absolute form for proper comparison
-  // This handles cases where one path uses ~ and the other is fully expanded
+  // Resolve both paths to absolute form for proper comparison.
+  // This handles cases where one path uses ~ and the other is fully expanded.
   const [resolvedTarget, resolvedPlan] = await Promise.all([
     config.runtime.resolvePath(targetPath),
     config.runtime.resolvePath(config.planFilePath),

src/node/services/tools/file_edit_insert.ts

@@ -64,6 +64,14 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
         );
         file_path = correctedPath;
 
+        // Plan file is always read-only outside plan mode.
+        // This is especially important for SSH runtimes, where cwd validation is intentionally skipped.
+        if ((await isPlanFilePath(file_path, config)) && config.mode !== "plan") {
+          return {
+            success: false,
+            error: `Plan file is read-only outside plan mode: ${file_path}`,
+          };
+        }
         const resolvedPath = config.runtime.normalizePath(file_path, config.cwd);
 
         // Plan mode restriction: only allow editing/creating the plan file

src/node/services/tools/file_edit_operation.test.ts

@@ -216,6 +216,36 @@ describe("executeFileEditOperation plan mode enforcement", () => {
     expect(await fs.readFile(testFile, "utf-8")).toBe("const x = 2;\n");
   });
 
+  test("should block editing the plan file outside plan mode (integration)", async () => {
+    using tempDir = new TestTempDir("exec-plan-readonly-test");
+
+    const planPath = path.join(tempDir.path, "plan.md");
+    await fs.writeFile(planPath, "# Plan\n");
+
+    const workspaceCwd = path.join(tempDir.path, "workspace");
+    await fs.mkdir(workspaceCwd);
+
+    const result = await executeFileEditOperation({
+      config: {
+        cwd: workspaceCwd,
+        runtime: new LocalRuntime(workspaceCwd),
+        runtimeTempDir: tempDir.path,
+        mode: "exec",
+        planFilePath: planPath,
+      },
+      filePath: planPath,
+      operation: () => ({ success: true, newContent: "# Updated\n", metadata: {} }),
+    });
+
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error).toContain("read-only outside plan mode");
+    }
+
+    // Verify file was not modified
+    expect(await fs.readFile(planPath, "utf-8")).toBe("# Plan\n");
+  });
+
   test("should handle relative path to plan file in plan mode", async () => {
     // When user provides a relative path that resolves to the plan file,
     // it should still be allowed

src/node/services/tools/file_edit_operation.ts

@@ -56,6 +56,15 @@ export async function executeFileEditOperation<TMetadata>({
     // This ensures path resolution uses runtime-specific semantics instead of Node.js path module
     const resolvedPath = config.runtime.normalizePath(filePath, config.cwd);
 
+    // Plan file is always read-only outside plan mode.
+    // This is especially important for SSH runtimes, where cwd validation is intentionally skipped.
+    if ((await isPlanFilePath(filePath, config)) && config.mode !== "plan") {
+      return {
+        success: false,
+        error: `Plan file is read-only outside plan mode: ${filePath}`,
+      };
+    }
+
     // Plan mode restriction: only allow editing the plan file
     if (config.mode === "plan" && config.planFilePath) {
       if (!(await isPlanFilePath(filePath, config))) {