From e5fcfc4707998a7d0dae5922526861e987d4b172 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sun, 31 May 2026 10:44:02 +0000 Subject: [PATCH] fix: register pendingSave around workspace.save and save_as writes (Closes #982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handleWorkspaceSave and handleWorkspaceSaveAs in the MCP v2 bridge wrote directly via writeTextFile() without registerPendingSave / clearPendingSave coordination. The external file watcher couldn't tell these were our own writes and would prompt the user to reload, potentially discarding the AI's save or interleaved edits. Apply the same register → write → clear (with try/finally) pattern that issue #975 added to document.write, and extend the workspace tests to assert both handlers register and clear the pending save, including when writeTextFile rejects. --- .../mcpBridge/v2/__tests__/workspace.test.ts | 115 ++++++++++++++++++ src/hooks/mcpBridge/v2/workspace.ts | 21 +++- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/src/hooks/mcpBridge/v2/__tests__/workspace.test.ts b/src/hooks/mcpBridge/v2/__tests__/workspace.test.ts index 0783a6ef..177a0142 100644 --- a/src/hooks/mcpBridge/v2/__tests__/workspace.test.ts +++ b/src/hooks/mcpBridge/v2/__tests__/workspace.test.ts @@ -40,6 +40,15 @@ vi.mock("@tauri-apps/plugin-fs", () => ({ writeTextFile: (path: string, content: string) => writeMock(path, content), })); +const registerPendingSaveMock = vi.fn(() => 1); +const clearPendingSaveMock = vi.fn(); +vi.mock("@/utils/pendingSaves", () => ({ + registerPendingSave: (path: string, content: string) => + registerPendingSaveMock(path, content), + clearPendingSave: (path: string, token?: number) => + clearPendingSaveMock(path, token), +})); + import { respond } from "../../utils"; import { handleWorkspaceOpen, @@ -203,6 +212,9 @@ describe("vmark.workspace.save / save_as", () => { beforeEach(() => { vi.clearAllMocks(); resetStores(); + writeMock.mockReset().mockResolvedValue(undefined); + registerPendingSaveMock.mockReset().mockReturnValue(1); + clearPendingSaveMock.mockReset(); }); it("save writes the doc content to its existing filePath", async () => { @@ -271,6 +283,109 @@ describe("vmark.workspace.save / save_as", () => { useDocumentStore.getState().documents["t-a"].filePath, ).toBe("/tmp/new.md"); }); + + it("save registers and clears pending save around writeTextFile to suppress the external-change dialog", async () => { + useTabStore.setState({ + tabs: { + main: [ + { + id: "t-ps", + filePath: "/tmp/notes.md", + title: "notes", + isPinned: false, + }, + ], + }, + activeTabId: { main: "t-ps" }, + untitledCounter: 0, + closedTabs: {}, + }); + useDocumentStore.getState().initDocument("t-ps", "hi", "/tmp/notes.md"); + useDocumentStore.getState().setContent("t-ps", "updated"); + + await handleWorkspaceSave("req-ps", {}); + + expect(registerPendingSaveMock).toHaveBeenCalledWith("/tmp/notes.md", "updated"); + expect(clearPendingSaveMock).toHaveBeenCalledWith("/tmp/notes.md", 1); + const registerOrder = registerPendingSaveMock.mock.invocationCallOrder[0]; + const writeOrder = writeMock.mock.invocationCallOrder[0]; + const clearOrder = clearPendingSaveMock.mock.invocationCallOrder[0]; + expect(registerOrder).toBeLessThan(writeOrder); + expect(writeOrder).toBeLessThan(clearOrder); + }); + + it("save clears pending save even when writeTextFile rejects", async () => { + useTabStore.setState({ + tabs: { + main: [ + { + id: "t-ps-fail", + filePath: "/readonly/notes.md", + title: "notes", + isPinned: false, + }, + ], + }, + activeTabId: { main: "t-ps-fail" }, + untitledCounter: 0, + closedTabs: {}, + }); + useDocumentStore.getState().initDocument("t-ps-fail", "x", "/readonly/notes.md"); + writeMock.mockRejectedValueOnce(new Error("EACCES")); + + await handleWorkspaceSave("req-ps-fail", {}); + + expect(registerPendingSaveMock).toHaveBeenCalledWith("/readonly/notes.md", "x"); + expect(clearPendingSaveMock).toHaveBeenCalledWith("/readonly/notes.md", 1); + }); + + it("save_as registers and clears pending save around writeTextFile to suppress the external-change dialog", async () => { + useTabStore.setState({ + tabs: { + main: [{ id: "t-as", filePath: null, title: "u", isPinned: false }], + }, + activeTabId: { main: "t-as" }, + untitledCounter: 0, + closedTabs: {}, + }); + useDocumentStore.getState().initDocument("t-as", "hello", null); + + await handleWorkspaceSaveAs("req-as", { + tabId: "t-as", + filePath: "/tmp/new.md", + }); + + expect(registerPendingSaveMock).toHaveBeenCalledWith("/tmp/new.md", "hello"); + expect(clearPendingSaveMock).toHaveBeenCalledWith("/tmp/new.md", 1); + const registerOrder = registerPendingSaveMock.mock.invocationCallOrder[0]; + const writeOrder = writeMock.mock.invocationCallOrder[0]; + const clearOrder = clearPendingSaveMock.mock.invocationCallOrder[0]; + expect(registerOrder).toBeLessThan(writeOrder); + expect(writeOrder).toBeLessThan(clearOrder); + }); + + it("save_as clears pending save even when writeTextFile rejects", async () => { + useTabStore.setState({ + tabs: { + main: [{ id: "t-as-fail", filePath: null, title: "u", isPinned: false }], + }, + activeTabId: { main: "t-as-fail" }, + untitledCounter: 0, + closedTabs: {}, + }); + useDocumentStore.getState().initDocument("t-as-fail", "hello", null); + writeMock.mockRejectedValueOnce(new Error("EACCES")); + + await expect( + handleWorkspaceSaveAs("req-as-fail", { + tabId: "t-as-fail", + filePath: "/readonly/new.md", + }), + ).resolves.toBeUndefined(); + + expect(registerPendingSaveMock).toHaveBeenCalledWith("/readonly/new.md", "hello"); + expect(clearPendingSaveMock).toHaveBeenCalledWith("/readonly/new.md", 1); + }); }); describe("vmark.workspace.focus_window", () => { diff --git a/src/hooks/mcpBridge/v2/workspace.ts b/src/hooks/mcpBridge/v2/workspace.ts index f9f9aa8d..58dbe397 100644 --- a/src/hooks/mcpBridge/v2/workspace.ts +++ b/src/hooks/mcpBridge/v2/workspace.ts @@ -31,6 +31,7 @@ import { useTabStore } from "@/stores/tabStore"; import { useDocumentStore } from "@/stores/documentStore"; import { useRevisionStore } from "@/stores/documentStore"; import { getFileName } from "@/utils/paths"; +import { registerPendingSave, clearPendingSave } from "@/utils/pendingSaves"; import { getCurrentWindowLabel } from "@/utils/workspaceStorage"; import { respond } from "../utils"; import { wrapHandler } from "./wrapHandler"; @@ -163,10 +164,15 @@ export async function handleWorkspaceSave( await structuredError(id, resolved); return; } - await writeTextFile(resolved.filePath, resolved.content); - useDocumentStore - .getState() - .markSaved(resolved.tabId, resolved.content); + const saveToken = registerPendingSave(resolved.filePath, resolved.content); + try { + await writeTextFile(resolved.filePath, resolved.content); + useDocumentStore + .getState() + .markSaved(resolved.tabId, resolved.content); + } finally { + clearPendingSave(resolved.filePath, saveToken); + } const revision = useRevisionStore.getState().getRevision(resolved.tabId); await respond({ id, @@ -233,7 +239,12 @@ export async function handleWorkspaceSaveAs( }); return; } - await writeTextFile(filePath, doc.content); + const saveToken = registerPendingSave(filePath, doc.content); + try { + await writeTextFile(filePath, doc.content); + } finally { + clearPendingSave(filePath, saveToken); + } tabState.updateTabPath(tabId, filePath); tabState.updateTabTitle(tabId, getFileName(filePath) || "Untitled"); docState.setFilePath(tabId, filePath);