From dd696732c8a48728337e9f5982004feeffd7c2aa Mon Sep 17 00:00:00 2001 From: Alessandro <155005371+3clyp50@users.noreply.github.com> Date: Sat, 2 May 2026 20:51:58 +0200 Subject: [PATCH] Fix canvas markdown rename before save Route Office canvas renames through the document store so dirty or missing-on-disk Markdown sessions can be materialized at the new path without hitting the generic workdir filesystem rename endpoint. Add regression coverage for missing draft materialization, dirty markdown rename, and the custom rename hook contract. --- plugins/_office/api/office_session.py | 8 +- plugins/_office/helpers/document_store.py | 91 +++++++++++++++++++ plugins/_office/webui/office-store.js | 24 +++-- tests/test_office_canvas_setup.py | 4 + tests/test_office_document_store.py | 34 +++++++ .../modals/file-browser/file-browser-store.js | 38 ++++++-- 6 files changed, 181 insertions(+), 18 deletions(-) diff --git a/plugins/_office/api/office_session.py b/plugins/_office/api/office_session.py index 8d812bae0..7f41ea401 100644 --- a/plugins/_office/api/office_session.py +++ b/plugins/_office/api/office_session.py @@ -118,7 +118,12 @@ class OfficeSession(ApiHandler): if not path: return {"ok": False, "error": "path is required."} try: - updated = document_store.update_document_path(file_id, path, context_id=context_id) + updated = document_store.rename_document( + file_id, + path, + content=input.get("text") if "text" in input else None, + context_id=context_id, + ) except Exception as exc: return {"ok": False, "error": str(exc)} desktop = None @@ -129,6 +134,7 @@ class OfficeSession(ApiHandler): "document": _public_doc(updated), "version": document_store.item_version(updated), "desktop": desktop, + "refreshFiles": False, } def _desktop(self) -> dict: diff --git a/plugins/_office/helpers/document_store.py b/plugins/_office/helpers/document_store.py index 303714290..77155791e 100644 --- a/plugins/_office/helpers/document_store.py +++ b/plugins/_office/helpers/document_store.py @@ -303,6 +303,97 @@ def update_document_path(file_id: str, path: str | Path, context_id: str = "") - return get_document(file_id, conn=conn) +def rename_document( + file_id: str, + path: str | Path, + content: str | None = None, + context_id: str = "", +) -> dict[str, Any]: + resolved = normalize_path(path, context_id=context_id) + ext = normalize_extension(resolved.suffix.lstrip(".")) + data = None + if content is not None: + if ext != "md": + raise ValueError("Inline content can only be provided for Markdown documents.") + data = str(content or "").encode("utf-8") + if len(data) > MAX_SAVE_BYTES: + raise OverflowError("Document save exceeds maximum size") + + changed_at = now() + with connect() as conn: + doc = get_document(file_id, conn=conn) + source = Path(doc["path"]) + source_resolved = source.resolve(strict=False) + changed_path = str(source_resolved) != str(resolved) + source_exists = source.exists() + + if ext != str(doc["extension"]).lower(): + raise ValueError("Document extension cannot change during rename.") + + row = conn.execute("SELECT file_id FROM documents WHERE path = ?", (str(resolved),)).fetchone() + if row and row["file_id"] != file_id: + raise ValueError(f"Document path is already registered: {display_path(resolved)}") + if changed_path and resolved.exists(): + raise FileExistsError(f"Target already exists: {display_path(resolved)}") + if not source_exists and data is None: + raise FileNotFoundError(str(source_resolved)) + + previous = source.read_bytes() if source_exists else b"" + content_changed = data is not None and data != previous + + if changed_path and data is None: + resolved.parent.mkdir(parents=True, exist_ok=True) + source.rename(resolved) + final_data = resolved.read_bytes() + elif data is not None: + if content_changed: + _record_version(conn, file_id, source_resolved, item_version(doc), previous) + _write_atomic(resolved, data) + if changed_path and source_exists: + source.unlink(missing_ok=True) + final_data = data + else: + final_data = previous + + stat = resolved.stat() + next_version = int(doc["version"]) + 1 if content_changed else int(doc["version"]) + conn.execute( + """ + UPDATE documents + SET path=?, basename=?, extension=?, size=?, version=?, sha256=?, last_modified=?, updated_at=? + WHERE file_id=? + """, + ( + str(resolved), + resolved.name, + ext, + stat.st_size, + next_version, + sha256_bytes(final_data), + now_iso(), + changed_at, + file_id, + ), + ) + conn.execute( + "INSERT INTO events (file_id, event_type, payload, created_at) VALUES (?, ?, ?, ?)", + ( + file_id, + "renamed", + json.dumps( + { + "from": display_path(source_resolved), + "to": display_path(resolved), + "saved": content_changed, + "materialized": not source_exists, + } + ), + changed_at, + ), + ) + return get_document(file_id, conn=conn) + + def get_open_documents(limit: int = 6) -> list[dict[str, Any]]: with connect() as conn: _clear_expired_sessions(conn) diff --git a/plugins/_office/webui/office-store.js b/plugins/_office/webui/office-store.js index 2fcee590e..b5bb349a5 100644 --- a/plugins/_office/webui/office-store.js +++ b/plugins/_office/webui/office-store.js @@ -517,10 +517,6 @@ const model = { async renameActiveFile() { if (!this.session || this.isDesktopSession() || this.saving) return; - if (this.dirty || this.session.dirty) { - await this.save(); - if (this.error) return; - } const session = this.session; const path = session.path || session.document?.path || ""; @@ -545,15 +541,26 @@ const model = { if (!extension) return true; return extensionOf(newName) === extension || `Keep the .${extension} extension for this open document.`; }, - onRenamed: async ({ path: renamedPath }) => { - await this.handleActiveFileRenamed(session, renamedPath); + performRename: async ({ path: renamedPath }) => { + const payload = { + file_id: session.file_id || "", + path: renamedPath, + }; + if (this.isMarkdown(session)) { + this.syncEditorText(); + payload.text = this.session?.tab_id === session.tab_id ? this.editorText : session.text || ""; + } + return await callOffice("renamed", payload); + }, + onRenamed: async ({ path: renamedPath, response }) => { + await this.handleActiveFileRenamed(session, renamedPath, response); }, }, ); }, - async handleActiveFileRenamed(session, renamedPath) { - const response = await callOffice("renamed", { + async handleActiveFileRenamed(session, renamedPath, renameResponse = null) { + const response = renameResponse || await callOffice("renamed", { file_id: session.file_id || "", path: renamedPath, }); @@ -569,6 +576,7 @@ const model = { file_id: document.file_id || session.file_id, version: document.version || response.version || session.version, desktop: response.desktop?.desktop || session.desktop, + text: this.session?.tab_id === session.tab_id ? this.editorText : session.text, dirty: false, }; this.replaceSession(session, updated); diff --git a/tests/test_office_canvas_setup.py b/tests/test_office_canvas_setup.py index 51b83148b..ccbfb2cab 100644 --- a/tests/test_office_canvas_setup.py +++ b/tests/test_office_canvas_setup.py @@ -40,6 +40,8 @@ def test_document_canvas_uses_markdown_editor_and_official_libreoffice_desktop_f assert "desktop_save" in store assert "openRenameModal" in store assert 'callOffice("renamed"' in store + assert "performRename" in store + assert "payload.text" in store assert "handleActiveFileRenamed" in store assert "--office-zoom" not in panel assert "zoom: 1" not in store @@ -147,8 +149,10 @@ def test_office_surface_filters_tabs_to_desktop_and_markdown_without_dashboard() ).read_text(encoding="utf-8") assert "renameAfterConfirm" in file_browser_store + assert "renamePerformAction" in file_browser_store assert "renameValidateName" in file_browser_store assert "options.onRenamed" in file_browser_store + assert "options.performRename" in file_browser_store assert "options.validateName" in file_browser_store diff --git a/tests/test_office_document_store.py b/tests/test_office_document_store.py index c51ebb6b1..3b6f2d56d 100644 --- a/tests/test_office_document_store.py +++ b/tests/test_office_document_store.py @@ -277,6 +277,40 @@ def test_document_path_update_preserves_file_id_after_rename(office_state): assert document_store.get_document(doc["file_id"])["path"] == str(renamed) +def test_document_rename_materializes_missing_markdown_with_editor_text(office_state): + doc = document_store.create_document("document", "Unsaved Draft", "md", "Seed") + original = Path(doc["path"]) + original.unlink() + renamed = original.with_name("Renamed Draft.md") + + updated = document_store.rename_document( + doc["file_id"], + renamed, + content="# Renamed Draft\n\nCanvas text", + ) + + assert updated["file_id"] == doc["file_id"] + assert updated["basename"] == "Renamed Draft.md" + assert updated["path"] == str(renamed) + assert renamed.read_text(encoding="utf-8") == "# Renamed Draft\n\nCanvas text" + + +def test_document_rename_saves_dirty_markdown_and_removes_original(office_state): + doc = document_store.create_document("document", "Dirty Rename", "md", "Old") + original = Path(doc["path"]) + renamed = original.with_name("Clean Rename.md") + + updated = document_store.rename_document( + doc["file_id"], + renamed, + content="# Clean Rename\n\nFresh text", + ) + + assert updated["version"] == 2 + assert not original.exists() + assert renamed.read_text(encoding="utf-8") == "# Clean Rename\n\nFresh text" + + def test_direct_markdown_edits_refresh_open_canvas_session(office_state, monkeypatch): manager = markdown_sessions.MarkdownSessionManager() monkeypatch.setattr(markdown_sessions, "_manager", manager, raising=False) diff --git a/webui/components/modals/file-browser/file-browser-store.js b/webui/components/modals/file-browser/file-browser-store.js index 282ec1ab7..3c71f325c 100644 --- a/webui/components/modals/file-browser/file-browser-store.js +++ b/webui/components/modals/file-browser/file-browser-store.js @@ -24,6 +24,7 @@ const model = { isRenaming: false, renameError: null, renameAfterConfirm: null, + renamePerformAction: null, renameValidateName: null, openDropdownPath: null, // Track which dropdown is currently open searchQuery: "", @@ -245,6 +246,7 @@ const model = { this.isRenaming = false; this.renameError = null; this.renameAfterConfirm = null; + this.renamePerformAction = null; this.renameValidateName = null; }, @@ -364,6 +366,7 @@ const model = { this.renameMode = "rename"; this.renameError = null; this.renameAfterConfirm = typeof options.onRenamed === "function" ? options.onRenamed : null; + this.renamePerformAction = typeof options.performRename === "function" ? options.performRename : null; this.renameValidateName = typeof options.validateName === "function" ? options.validateName : null; if (typeof options.currentPath === "string" && options.currentPath) { this.browser.currentPath = options.currentPath; @@ -451,18 +454,35 @@ const model = { newName: newName, }; - const resp = await fetchApi("/rename_work_dir_file", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(payload), - }); + let data = {}; + if (this.renamePerformAction) { + data = await this.renamePerformAction({ + action: this.renameMode, + previousPath, + path: renamedPath, + name: newName, + target: this.renameTarget, + payload, + }) || {}; + if (data.error || data.ok === false) { + throw new Error(data.error || "Rename failed"); + } + } else { + const resp = await fetchApi("/rename_work_dir_file", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(payload), + }); - const data = await resp.json().catch(() => ({})); - if (!resp.ok || data.error) { - throw new Error(data.error || "Rename failed"); + data = await resp.json().catch(() => ({})); + if (!resp.ok || data.error) { + throw new Error(data.error || "Rename failed"); + } } - await this.fetchFiles(this.browser.currentPath); + if (!this.renamePerformAction || data.refreshFiles !== false) { + await this.fetchFiles(this.browser.currentPath); + } if (this.renameAfterConfirm) { await this.renameAfterConfirm({ action: this.renameMode,