From 5c0fa15d1d5bbe48f85f21e98bb6e91e9b9bcd85 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 24 May 2026 14:33:21 -0500 Subject: fix(org-roam): guard move-branch-to-roam against data loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cj/move-org-branch-to-roam cut the subtree from the source buffer before writing the new roam file, so a failure in the demote/format/write/db-sync steps left the subtree gone from the source and not persisted anywhere — a destructive operation with no rollback. Reordered so the node file is written and verified on disk before org-cut-subtree runs; a failed write now aborts with the source untouched. Added a no-clobber guard (refuse an existing target file) and a confirmation prompt for large subtrees (>= cj/move-org-branch-confirm-lines, 30) or buffers with unsaved changes. The source buffer is deliberately left modified and undoable rather than auto-saved, so the move stays reversible. New test drives the write-failure-preserves-source invariant via an unwritable roam dir; the existing creates-roam-file test gained the confirm mock. --- modules/org-roam-config.el | 65 ++++++++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 22 deletions(-) (limited to 'modules') diff --git a/modules/org-roam-config.el b/modules/org-roam-config.el index da4b0657..c6e8977f 100644 --- a/modules/org-roam-config.el +++ b/modules/org-roam-config.el @@ -278,12 +278,23 @@ Returns the complete file content as a string." "#+FILETAGS: Topic\n\n" content)) +(defconst cj/move-org-branch-confirm-lines 30 + "Subtree size (in lines) at or above which `cj/move-org-branch-to-roam' confirms. +Cutting a large subtree by accident is jarring even though the move is +undoable, so the command asks first past this threshold.") + (defun cj/move-org-branch-to-roam () "Move the org subtree at point to a new org-roam node. The node filename will be timestamp-based with the heading name. The heading becomes the node title, and the entire subtree is demoted to level 1. If the heading contains a link, extract the description for the -title." +title. + +Safety: the new node file is written and verified on disk *before* the +subtree is cut from the source, so a failed write or db-sync can't lose +the subtree. The source buffer is left modified (not auto-saved) so the +cut stays undoable. A confirmation prompt guards large subtrees (see +`cj/move-org-branch-confirm-lines') and buffers with unsaved changes." (interactive) ;; Lazy load org and org-roam when needed (require 'org) @@ -304,29 +315,39 @@ title." (filename (format "%s-%s.org" timestamp title-slug)) (filepath (expand-file-name filename org-roam-directory)) ;; Generate a unique ID for the node - (node-id (org-id-new)) - ;; Store the subtree in a temporary buffer - subtree-content) - - ;; Copy the subtree content - (org-copy-subtree) - (setq subtree-content (current-kill 0)) + (node-id (org-id-new))) - ;; Now cut it to remove from original buffer - (org-cut-subtree) + ;; Refuse to overwrite an existing node file (timestamp collision, re-run). + (when (file-exists-p filepath) + (user-error "Roam node file already exists: %s" filepath)) - ;; Process the subtree to demote it to level 1 - (setq subtree-content (cj/--demote-org-subtree subtree-content current-level 1)) - - ;; Create the new org-roam file - (with-temp-file filepath - (insert (cj/--format-roam-node title node-id subtree-content))) - - ;; Sync the org-roam database - (org-roam-db-sync) - - ;; Message to user - (message "'%s' added as an org-roam node." title))) + ;; Copy the subtree (non-destructive) and build the node text up front. + (org-copy-subtree) + (let* ((subtree-content (cj/--demote-org-subtree (current-kill 0) current-level 1)) + (node-text (cj/--format-roam-node title node-id subtree-content)) + (line-count (length (split-string subtree-content "\n" t)))) + + ;; Confirm before the destructive cut for a large subtree or a buffer + ;; with unsaved changes, where recovery is murkier. + (when (or (>= line-count cj/move-org-branch-confirm-lines) + (buffer-modified-p)) + (unless (yes-or-no-p + (format "Move subtree \"%s\" (%d lines) to a new roam node? " + title line-count)) + (user-error "Aborted"))) + + ;; Write the new file BEFORE removing the source, and verify it landed, + ;; so a write failure can't lose the subtree. + (with-temp-file filepath + (insert node-text)) + (unless (file-exists-p filepath) + (error "Failed to create roam node file: %s" filepath)) + + ;; The subtree is safely on disk -- now cut it. The buffer is left + ;; modified and undoable (not auto-saved), so the move is reversible. + (org-cut-subtree) + (org-roam-db-sync) + (message "'%s' moved to a new org-roam node (%s)." title filename)))) ;; TASK: Need to decide keybindings before implementation and testing ;; (use-package consult-org-roam -- cgit v1.2.3