bindings_nodes: fix parent variable redeclaration (rename to parentVal)

cmd/verstak-gui/bindings_nodes.go

@@ -228,18 +228,24 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 	isFolderLike := n.Type != nodes.TypeNote && n.Type != nodes.TypeFile
 
 	if isFolderLike {
-		// Folder-like node: rename physical directory
+		return a.renameFolderNode(nodeID, n, newTitle, seg)
+	}
+	return a.renameNoteFileNode(nodeID, n, newTitle, seg)
+}
+
+// renameFolderNode renames a folder-like node atomically: FS first, then DB transaction.
+func (a *App) renameFolderNode(nodeID string, n *nodes.Node, newTitle, seg string) error {
 	if n.FsPath == "" {
 		return fmt.Errorf("cannot rename node %s (%s): fs_path is empty", n.ID, n.Title)
 	}
 
 	oldFsPath := n.FsPath
 	oldPhysPath := filepath.Join(a.vault, oldFsPath)
+	oldTitle := n.Title
 
 	parentFsPath := ""
 	if n.ParentID != nil {
-			p, err := a.nodes.GetActive(*n.ParentID)
+		if p, err := a.nodes.GetActive(*n.ParentID); err == nil {
-			if err == nil {
 			parentFsPath = p.FsPath
 		}
 	}
@@ -256,7 +262,19 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 		return fmt.Errorf("path safety: %w", err)
 	}
 
-		oldTitle := n.Title
+	// Pre-collect all descendant fs_path updates (before transaction, to avoid deadlock).
+	type pathUp struct{ id, path string }
+	var updates []pathUp
+	var walk func(string, string)
+	walk = func(id, p string) {
+		updates = append(updates, pathUp{id, p})
+		children, _ := a.nodes.ListChildren(id, false)
+		for _, c := range children {
+			cseg := templates.SafeDisplayNameToPathSegment(c.Title)
+			walk(c.ID, filepath.Join(p, cseg))
+		}
+	}
+	walk(nodeID, newFsPath)
 
 	// Check source exists before filesystem rename
 	if _, err := os.Stat(oldPhysPath); err != nil {
@@ -266,19 +284,35 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 		return fmt.Errorf("rename folder: %w", err)
 	}
 
-		// Update DB only after successful filesystem rename
+	// Atomic DB transaction: title + own fs_path + all descendant fs_paths
-		if err := a.nodes.UpdateTitle(nodeID, newTitle); err != nil {
+	nowT := time.Now().UTC().Format(time.RFC3339)
+	slug := nodes.Slugify(newTitle)
+
+	tx, err := a.db.Begin()
+	if err != nil {
+		_ = os.Rename(newPhysPath, oldPhysPath)
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	if _, err := tx.Exec(
+		`UPDATE nodes SET title=?, slug=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+		newTitle, slug, nowT, nodeID); err != nil {
 		_ = os.Rename(newPhysPath, oldPhysPath)
 		return err
 	}
-		if err := a.nodes.UpdateFsPath(nodeID, newFsPath); err != nil {
+	for _, u := range updates {
-			_ = a.nodes.UpdateTitle(nodeID, oldTitle)
+		if _, err := tx.Exec(
+			`UPDATE nodes SET fs_path=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+			u.path, nowT, u.id); err != nil {
 			_ = os.Rename(newPhysPath, oldPhysPath)
 			return err
 		}
-		if err := a.nodes.UpdateFsPathRecursive(nodeID, newFsPath); err != nil {
+	}
+
+	if err := tx.Commit(); err != nil {
 		_ = os.Rename(newPhysPath, oldPhysPath)
-			return err
+		return fmt.Errorf("commit tx: %w", err)
 	}
 
 	pid := ""
@@ -292,10 +326,11 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 		"updated_at": time.Now().UTC().Format(time.RFC3339),
 	})
 	return nil
-	}
+}
 
-	// Note/file node: rename the physical file, update file record
+// renameNoteFileNode renames a note or file node atomically: FS first, then DB transaction.
-	// Collect file records first to avoid connection deadlock (SetMaxOpenConns=1)
+func (a *App) renameNoteFileNode(nodeID string, n *nodes.Node, newTitle, seg string) error {
+	// Collect file records before any mutations (avoid deadlock with SetMaxOpenConns=1).
 	type fileRec struct {
 		id, path, filename string
 	}
@@ -312,13 +347,13 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 		rows.Close()
 	}
 
-	// Collect rename operations without modifying anything yet
+	// Compute new filenames and paths (no FS modifications yet).
 	type renameOp struct {
 		id          string
-		oldFilename string
 		oldAbs      string
+		newAbs      string
 		newFilename string
-		newRelPath  string
+		newPath     string
 	}
 	var renameOps []renameOp
 	for _, r := range records {
@@ -337,10 +372,10 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 		} else {
 			newFilename = newTitle
 		}
-		newRelPath := filepath.Join(dir, newFilename)
+		newPath := filepath.Join(dir, newFilename)
-		newAbs := filepath.Join(a.vault, newRelPath)
+		newAbs := filepath.Join(a.vault, newPath)
 
-		// Check for collision — generate unique name if needed
+		// Check for collision
 		if _, err := os.Stat(newAbs); err == nil {
 			ext := filepath.Ext(newFilename)
 			base := strings.TrimSuffix(newFilename, ext)
@@ -348,62 +383,79 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 			if baseSeg == "" {
 				baseSeg = "renamed"
 			}
-			for n := 2; ; n++ {
+			for i := 2; ; i++ {
-				candidate := fmt.Sprintf("%s (%d)%s", baseSeg, n, ext)
+				candidate := fmt.Sprintf("%s (%d)%s", baseSeg, i, ext)
 				candidatePath := filepath.Join(dir, candidate)
 				if _, err := os.Stat(filepath.Join(a.vault, candidatePath)); os.IsNotExist(err) {
 					newFilename = candidate
-					newRelPath = candidatePath
+					newPath = candidatePath
-					newAbs = filepath.Join(a.vault, newRelPath)
+					newAbs = filepath.Join(a.vault, newPath)
 					break
 				}
 			}
 		}
 
-		// Check source file exists
 		if _, err := os.Stat(oldAbs); err != nil {
 			return fmt.Errorf("source file not found for rename: %w", err)
 		}
-
 		renameOps = append(renameOps, renameOp{
-			id:          r.id,
+			id: r.id, oldAbs: oldAbs, newAbs: newAbs,
-			oldFilename: r.filename,
+			newFilename: newFilename, newPath: newPath,
-			oldAbs:      oldAbs,
-			newFilename: newFilename,
-			newRelPath:  newRelPath,
 		})
 	}
 
 	// Perform all physical renames first
-	for _, rop := range renameOps {
+	successCount := 0
-		newAbs := filepath.Join(a.vault, rop.newRelPath)
+	for i, rop := range renameOps {
-		if err := os.Rename(rop.oldAbs, newAbs); err != nil {
+		if err := os.Rename(rop.oldAbs, rop.newAbs); err != nil {
-			// Rollback completed renames
+			// Rollback completed FS renames
-			for _, prev := range renameOps {
+			for j := 0; j < i; j++ {
-				if prev.id == rop.id {
+				_ = os.Rename(renameOps[j].newAbs, renameOps[j].oldAbs)
-					break
-				}
-				_ = os.Rename(filepath.Join(a.vault, prev.newRelPath), prev.oldAbs)
 			}
 			return fmt.Errorf("rename file: %w", err)
 		}
+		successCount++
 	}
 
-	// All renames succeeded — update DB
+	// Atomic DB transaction: title + all file record updates
-	if err := a.nodes.UpdateTitle(nodeID, newTitle); err != nil {
+	nowT := time.Now().UTC().Format(time.RFC3339)
-		// Rollback filesystem
+	slug := nodes.Slugify(newTitle)
-		for _, rop := range renameOps {
+
-			_ = os.Rename(filepath.Join(a.vault, rop.newRelPath), rop.oldAbs)
+	tx, err := a.db.Begin()
+	if err != nil {
+		for _, rop := range renameOps[:successCount] {
+			_ = os.Rename(rop.newAbs, rop.oldAbs)
+		}
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	if _, err := tx.Exec(
+		`UPDATE nodes SET title=?, slug=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+		newTitle, slug, nowT, nodeID); err != nil {
+		for _, rop := range renameOps[:successCount] {
+			_ = os.Rename(rop.newAbs, rop.oldAbs)
 		}
 		return err
 	}
 	for _, rop := range renameOps {
-		if _, err := a.db.Exec(`UPDATE files SET filename=?, path=? WHERE id=?`,
+		if _, err := tx.Exec(
-			rop.newFilename, rop.newRelPath, rop.id); err != nil {
+			`UPDATE files SET filename=?, path=? WHERE id=?`,
+			rop.newFilename, rop.newPath, rop.id); err != nil {
+			for _, rop2 := range renameOps[:successCount] {
+				_ = os.Rename(rop2.newAbs, rop2.oldAbs)
+			}
 			return err
 		}
 	}
 
+	if err := tx.Commit(); err != nil {
+		for _, rop := range renameOps[:successCount] {
+			_ = os.Rename(rop.newAbs, rop.oldAbs)
+		}
+		return fmt.Errorf("commit tx: %w", err)
+	}
+
 	pid := ""
 	if n.ParentID != nil {
 		pid = *n.ParentID
@@ -423,7 +475,7 @@ func (a *App) RenameNode(nodeID, newTitle string) error {
 	_ = a.activity.Record(pid, targetType, nodeID, "", evType, newTitle, "")
 	_ = a.sync.RecordOp(syncEntity, nodeID, syncsvc.OpUpdate, map[string]interface{}{
 		"title":      newTitle,
-		"updated_at": time.Now().UTC().Format(time.RFC3339),
+		"updated_at": nowT,
 	})
 	return nil
 }
@@ -472,35 +524,36 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
 		}
 	}
 
-	// Resolve name conflicts first
+	// Compute new title (name conflict resolution) but don't commit yet.
 	nodeTitle := node.Title
+	titleChanged := false
 	if parent != nil {
 		destChildren, _ := a.nodes.ListChildren(newParentID, false)
 		for i := range destChildren {
-			if destChildren[i].Title == nodeTitle {
+			if destChildren[i].Title == nodeTitle && destChildren[i].ID != nodeID {
 				nodeTitle = fmt.Sprintf("%s (%d)", nodeTitle, 2)
+				titleChanged = true
 				break
 			}
 		}
 	}
 
-	// Update title if changed
-	if nodeTitle != node.Title {
-		if err := a.nodes.UpdateTitle(nodeID, nodeTitle); err != nil {
-			return err
-		}
-		node.Title = nodeTitle
-	}
-
 	if isFolderLike {
-		// Folder-like node: move physical directory
+		return a.moveFolderNode(nodeID, node, parent, newParentID, nodeTitle, titleChanged)
+	}
+	return a.moveNoteFileNode(nodeID, node, parent, newParentID, nodeTitle, titleChanged)
+}
+
+// moveFolderNode moves a folder-like node atomically: FS first, then DB transaction.
+func (a *App) moveFolderNode(nodeID string, node *nodes.Node, parent *nodes.Node, newParentID, nodeTitle string, titleChanged bool) error {
 	if node.FsPath == "" {
 		return fmt.Errorf("cannot move node %s (%s): fs_path is empty", node.ID, node.Title)
 	}
 
-		seg := templates.SafeDisplayNameToPathSegment(node.Title)
+	oldFsPath := node.FsPath
-		oldPhysPath := filepath.Join(a.vault, node.FsPath)
+	oldPhysPath := filepath.Join(a.vault, oldFsPath)
 
+	seg := templates.SafeDisplayNameToPathSegment(nodeTitle)
 	newFsPath := seg
 	if parent != nil && parent.FsPath != "" {
 		newFsPath = filepath.Join(parent.FsPath, seg)
@@ -514,6 +567,20 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
 		return fmt.Errorf("path safety: %w", err)
 	}
 
+	// Pre-collect all descendant fs_path updates (before transaction).
+	type pathUp struct{ id, path string }
+	var updates []pathUp
+	var walk func(string, string)
+	walk = func(id, p string) {
+		updates = append(updates, pathUp{id, p})
+		children, _ := a.nodes.ListChildren(id, false)
+		for _, c := range children {
+			cseg := templates.SafeDisplayNameToPathSegment(c.Title)
+			walk(c.ID, filepath.Join(p, cseg))
+		}
+	}
+	walk(nodeID, newFsPath)
+
 	// Check source exists and do filesystem rename first
 	if _, err := os.Stat(oldPhysPath); err != nil {
 		return fmt.Errorf("source folder not found: %w", err)
@@ -522,82 +589,188 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
 		return fmt.Errorf("move folder: %w", err)
 	}
 
-		// Update DB only after successful filesystem rename
+	// Atomic DB transaction: title + parent_id + all fs_paths
-		if newParentID == "" {
+	nowT := time.Now().UTC().Format(time.RFC3339)
-			if err := a.nodes.Move(nodeID, nil, 0); err != nil {
+	tx, err := a.db.Begin()
+	if err != nil {
 		_ = os.Rename(newPhysPath, oldPhysPath)
-				return err
+		return fmt.Errorf("begin tx: %w", err)
 	}
-		} else {
+	defer tx.Rollback()
-			if err := a.nodes.Move(nodeID, &newParentID, 0); err != nil {
+
+	if titleChanged {
+		slug := nodes.Slugify(nodeTitle)
+		if _, err := tx.Exec(
+			`UPDATE nodes SET title=?, slug=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+			nodeTitle, slug, nowT, nodeID); err != nil {
 			_ = os.Rename(newPhysPath, oldPhysPath)
 			return err
 		}
 	}
-		if err := a.nodes.UpdateFsPath(nodeID, newFsPath); err != nil {
+
-			_ = os.Rename(newPhysPath, oldPhysPath)
+	var parentVal interface{}
-			return err
+	if newParentID != "" {
+		parentVal = newParentID
 	}
-		if err := a.nodes.UpdateFsPathRecursive(nodeID, newFsPath); err != nil {
+	if _, err := tx.Exec(
+		`UPDATE nodes SET parent_id=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+		parentVal, nowT, nodeID); err != nil {
 		_ = os.Rename(newPhysPath, oldPhysPath)
 		return err
 	}
 
-		node.FsPath = newFsPath
+	// Touch new parent
+	if newParentID != "" {
+		_, _ = tx.Exec(`UPDATE nodes SET updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`, nowT, newParentID)
+	}
+
+	// Touch old parent if different
+	if node.ParentID != nil && *node.ParentID != newParentID {
+		_, _ = tx.Exec(`UPDATE nodes SET updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`, nowT, *node.ParentID)
+	}
+
+	for _, u := range updates {
+		if _, err := tx.Exec(
+			`UPDATE nodes SET fs_path=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+			u.path, nowT, u.id); err != nil {
+			_ = os.Rename(newPhysPath, oldPhysPath)
+			return err
+		}
+	}
+
+	if err := tx.Commit(); err != nil {
+		_ = os.Rename(newPhysPath, oldPhysPath)
+		return fmt.Errorf("commit tx: %w", err)
+	}
+
+	// Update node.ParentID for activity/sync below
+	if newParentID == "" {
+		node.ParentID = nil
 	} else {
-		// Note/file node: move physical file first, then update DB
+		node.ParentID = &newParentID
-		var fileMoves []fileMoveInfo
+	}
+	node.FsPath = newFsPath
+
+	pid := ""
+	if node.ParentID != nil {
+		pid = *node.ParentID
+	}
+	_ = a.activity.Record(pid, activity.TargetFolder, nodeID, "", activity.TypeFolderMoved, nodeTitle, `{"to":"`+newParentID+`"}`)
+	_ = a.sync.RecordOp(syncsvc.EntityFolder, nodeID, syncsvc.OpMove, map[string]interface{}{
+		"parent_id":  newParentID,
+		"fs_path":    newFsPath,
+		"updated_at": nowT,
+	})
+	return nil
+}
+
+// moveNoteFileNode moves a note/file atomically: FS first, then DB transaction.
+func (a *App) moveNoteFileNode(nodeID string, node *nodes.Node, parent *nodes.Node, newParentID, nodeTitle string, titleChanged bool) error {
+	// Collect file records before any mutations.
+	type fileMove struct {
+		id          string
+		oldPath     string
+		oldAbs      string
+		newRelPath  string
+		newAbs      string
+	}
+	var fileMoves []fileMove
 	frows, ferr := a.db.Query(`SELECT id, path FROM files WHERE node_id=?`, nodeID)
 	if ferr == nil {
 		for frows.Next() {
-				var fm fileMoveInfo
+			var fm fileMove
 			if err := frows.Scan(&fm.id, &fm.oldPath); err != nil {
 				continue
 			}
 			if fm.oldPath == "" {
 				continue
 			}
+			fm.oldAbs = filepath.Join(a.vault, fm.oldPath)
 			filename := filepath.Base(fm.oldPath)
 			fm.newRelPath = filename
 			if parent != nil && parent.FsPath != "" {
 				fm.newRelPath = filepath.Join(parent.FsPath, filename)
 			}
+			fm.newAbs = filepath.Join(a.vault, fm.newRelPath)
 			fileMoves = append(fileMoves, fm)
 		}
 		frows.Close()
 	}
 
-		// Perform filesystem moves first
+	// Perform filesystem moves first (with rollback on partial failure).
-		for _, fm := range fileMoves {
+	for i, fm := range fileMoves {
-			oldAbs := filepath.Join(a.vault, fm.oldPath)
+		if _, err := os.Stat(fm.oldAbs); err != nil {
-			newAbs := filepath.Join(a.vault, fm.newRelPath)
+			for j := 0; j < i; j++ {
-			if _, err := os.Stat(oldAbs); err != nil {
+				_ = os.Rename(fileMoves[j].newAbs, fileMoves[j].oldAbs)
+			}
 			return fmt.Errorf("source file not found for move: %w", err)
 		}
-			_ = os.MkdirAll(filepath.Dir(newAbs), 0o750)
+		_ = os.MkdirAll(filepath.Dir(fm.newAbs), 0o750)
-			if err := os.Rename(oldAbs, newAbs); err != nil {
+		if err := os.Rename(fm.oldAbs, fm.newAbs); err != nil {
+			for j := 0; j < i; j++ {
+				_ = os.Rename(fileMoves[j].newAbs, fileMoves[j].oldAbs)
+			}
 			return fmt.Errorf("move file: %w", err)
 		}
 	}
 
-		// Update DB only after successful filesystem renames
+	// Atomic DB transaction: title + parent_id + file paths
-		if newParentID == "" {
+	nowT := time.Now().UTC().Format(time.RFC3339)
-			if err := a.nodes.Move(nodeID, nil, 0); err != nil {
+	tx, err := a.db.Begin()
-				_ = a.rollbackFileMoves(fileMoves)
+	if err != nil {
-				return err
-			}
-		} else {
-			if err := a.nodes.Move(nodeID, &newParentID, 0); err != nil {
-				_ = a.rollbackFileMoves(fileMoves)
-				return err
-			}
-		}
 		for _, fm := range fileMoves {
-			if _, err := a.db.Exec(`UPDATE files SET path=? WHERE id=?`,
+			_ = os.Rename(fm.newAbs, fm.oldAbs)
-				fm.newRelPath, fm.id); err != nil {
+		}
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	if titleChanged {
+		slug := nodes.Slugify(nodeTitle)
+		if _, err := tx.Exec(
+			`UPDATE nodes SET title=?, slug=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+			nodeTitle, slug, nowT, nodeID); err != nil {
+			for _, fm := range fileMoves {
+				_ = os.Rename(fm.newAbs, fm.oldAbs)
+			}
 			return err
 		}
 	}
+
+	var parentVal interface{}
+	if newParentID != "" {
+		parentVal = newParentID
+	}
+	if _, err := tx.Exec(
+		`UPDATE nodes SET parent_id=?, updated_at=?, revision=revision+1 WHERE id=? AND deleted_at IS NULL`,
+		parentVal, nowT, nodeID); err != nil {
+		for _, fm := range fileMoves {
+			_ = os.Rename(fm.newAbs, fm.oldAbs)
+		}
+		return err
+	}
+
+	for _, fm := range fileMoves {
+		if _, err := tx.Exec(`UPDATE files SET path=? WHERE id=?`,
+			fm.newRelPath, fm.id); err != nil {
+			for _, fm2 := range fileMoves {
+				_ = os.Rename(fm2.newAbs, fm2.oldAbs)
+			}
+			return err
+		}
+	}
+
+	if err := tx.Commit(); err != nil {
+		for _, fm := range fileMoves {
+			_ = os.Rename(fm.newAbs, fm.oldAbs)
+		}
+		return fmt.Errorf("commit tx: %w", err)
+	}
+
+	if newParentID == "" {
+		node.ParentID = nil
+	} else {
+		node.ParentID = &newParentID
 	}
 
 	pid := ""
@@ -607,11 +780,7 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
 	var targetType string
 	var evType string
 	var syncEntity string
-	if isFolderLike {
+	if node.Type == nodes.TypeNote {
-		targetType = activity.TargetFolder
-		evType = activity.TypeFolderMoved
-		syncEntity = syncsvc.EntityFolder
-	} else if node.Type == nodes.TypeNote {
 		targetType = activity.TargetNote
 		evType = activity.TypeNoteUpdated
 		syncEntity = syncsvc.EntityNote
@@ -620,15 +789,11 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
 		evType = activity.TypeFileMoved
 		syncEntity = syncsvc.EntityFile
 	}
-	_ = a.activity.Record(pid, targetType, nodeID, "", evType, node.Title, `{"to":"`+newParentID+`"}`)
+	_ = a.activity.Record(pid, targetType, nodeID, "", evType, nodeTitle, `{"to":"`+newParentID+`"}`)
-	opPayload := map[string]interface{}{
+	_ = a.sync.RecordOp(syncEntity, nodeID, syncsvc.OpMove, map[string]interface{}{
 		"parent_id":  newParentID,
-		"updated_at": time.Now().UTC().Format(time.RFC3339),
+		"updated_at": nowT,
-	}
+	})
-	if isFolderLike && node.FsPath != "" {
-		opPayload["fs_path"] = node.FsPath
-	}
-	_ = a.sync.RecordOp(syncEntity, nodeID, syncsvc.OpMove, opPayload)
 	return nil
 }