From d285f9ad8b7fae2e76655bfc9ad2747d69f79a52 Mon Sep 17 00:00:00 2001 From: mirivlad Date: Wed, 3 Jun 2026 02:22:49 +0800 Subject: [PATCH] sync_apply FS-first rewrite; CreateNodeFromTemplate rollback; DeleteNodeAndChildren fail on trash errors; PLAN.md update - applyRemoteNodeUpdate: FS-first with SafeVaultPath validation, must-fail os.Rename - applyRemoteNodeMove: FS-first for folders and notes/files - moveNodeFiles: rewritten FS-first with atomic DB transaction - applyRemoteNoteMove: delegates to moveNodeFiles - CreateNodeFromTemplate: rollbackChildren on any child creation failure - DeleteToTrash: skip rename if source file already missing - DeleteNodeAndChildren: fail on deleteFileRecords errors and trash move failures - docs/PLAN.md: update step 14 status with known gaps --- cmd/verstak-gui/bindings_nodes.go | 35 +++- cmd/verstak-gui/sync_apply.go | 292 ++++++++++++++++++++---------- docs/PLAN.md | 4 +- internal/core/files/file.go | 36 ++-- 4 files changed, 244 insertions(+), 123 deletions(-) diff --git a/cmd/verstak-gui/bindings_nodes.go b/cmd/verstak-gui/bindings_nodes.go index 34f0958..d40c41f 100644 --- a/cmd/verstak-gui/bindings_nodes.go +++ b/cmd/verstak-gui/bindings_nodes.go @@ -91,12 +91,24 @@ func (a *App) CreateNodeFromTemplate(parentID, title, templateID string) (*NodeD return nil, fmt.Errorf("create folder: %w", err) } - // Create child nodes for default files (proper DB nodes + file records) + // Create child nodes for default files and folders with rollback on failure. nowRFC := time.Now().UTC().Format(time.RFC3339) + type childInfo struct { + id string + file bool + } + var created []childInfo + rollbackChildren := func() { + for i := len(created) - 1; i >= 0; i-- { + _ = a.nodes.SoftDelete(created[i].id) + } + } + for _, df := range tmpl.DefaultFiles { fpath := filepath.Join(physPath, df.Path) if err := os.MkdirAll(filepath.Dir(fpath), 0o755); err != nil { - continue + rollbackChildren() + return nil, fmt.Errorf("create directory for %s: %w", df.Path, err) } fileTitle := strings.TrimSuffix(filepath.Base(df.Path), filepath.Ext(df.Path)) if fileTitle == "" { @@ -104,12 +116,15 @@ func (a *App) CreateNodeFromTemplate(parentID, title, templateID string) (*NodeD } childNode, childErr := a.nodes.Create(&n.ID, nodes.TypeNote, fileTitle, 0, "", "") if childErr != nil { - continue + rollbackChildren() + return nil, fmt.Errorf("create child node for %s: %w", df.Path, childErr) } + created = append(created, childInfo{id: childNode.ID, file: true}) + content := fmt.Sprintf("# %s\n\n", title) if err := os.WriteFile(fpath, []byte(content), 0o640); err != nil { - _ = a.nodes.SoftDelete(childNode.ID) - continue + rollbackChildren() + return nil, fmt.Errorf("write file %s: %w", df.Path, err) } relPath, _ := filepath.Rel(a.vault, fpath) fi, _ := os.Stat(fpath) @@ -140,7 +155,6 @@ func (a *App) CreateNodeFromTemplate(parentID, title, templateID string) (*NodeD }) } - // Create child nodes for default folders (proper DB nodes + physical folders) for _, folderName := range tmpl.DefaultFolders { folderSeg := templates.SafeDisplayNameToPathSegment(folderName) if folderSeg == "" { @@ -148,8 +162,11 @@ func (a *App) CreateNodeFromTemplate(parentID, title, templateID string) (*NodeD } childNode, childErr := a.nodes.Create(&n.ID, nodes.TypeFolder, folderName, 0, "", "") if childErr != nil { - continue + rollbackChildren() + return nil, fmt.Errorf("create child folder %s: %w", folderName, childErr) } + created = append(created, childInfo{id: childNode.ID, file: false}) + childFsPath := folderSeg if fsPath != "" { childFsPath = filepath.Join(fsPath, folderSeg) @@ -160,8 +177,8 @@ func (a *App) CreateNodeFromTemplate(parentID, title, templateID string) (*NodeD childFsPath = childRel _ = a.nodes.UpdateFsPath(childNode.ID, childFsPath) if err := os.MkdirAll(childPhysPath, 0o755); err != nil { - _ = a.nodes.SoftDelete(childNode.ID) - continue + rollbackChildren() + return nil, fmt.Errorf("create child folder directory %s: %w", folderName, err) } _ = a.activity.Record(n.ID, activity.TargetFolder, childNode.ID, "", activity.TypeNodeCreated, folderName, "") _ = a.sync.RecordOp(syncsvc.EntityFolder, childNode.ID, syncsvc.OpCreate, nodePayload(childNode)) diff --git a/cmd/verstak-gui/sync_apply.go b/cmd/verstak-gui/sync_apply.go index d647db7..8d39bb6 100644 --- a/cmd/verstak-gui/sync_apply.go +++ b/cmd/verstak-gui/sync_apply.go @@ -277,53 +277,79 @@ func (a *App) applyRemoteNodeUpdate(op syncsvc.Op) error { now = payload.UpdatedAt } - // Get current node to check if it's folder-like n, err := a.nodes.Get(op.EntityID) if err != nil { return nil } - isFolderLike := n.Type != nodes.TypeNote && n.Type != nodes.TypeFile - if payload.Title != "" || payload.FsPath != "" { + // FS-first: rename folder on disk before touching DB + if payload.FsPath != "" && isFolderLike && n.FsPath != "" { + cleanPath, err := syncsvc.SafeVaultPath(a.vault, payload.FsPath) + if err != nil { + return fmt.Errorf("unsafe fs_path in node update: %w", err) + } + payload.FsPath = cleanPath + + oldPhys := filepath.Join(a.vault, n.FsPath) + newPhys := filepath.Join(a.vault, payload.FsPath) + if _, err := os.Stat(oldPhys); err == nil { + _ = os.MkdirAll(filepath.Dir(newPhys), 0o750) + if err := os.Rename(oldPhys, newPhys); err != nil { + return fmt.Errorf("rename folder for update %s -> %s: %w", oldPhys, newPhys, err) + } + } + } + + // Any title/fs_path/template_id changes? Then do atomic DB transaction. + if payload.Title != "" || payload.FsPath != "" || payload.TemplateID != "" { + tx, err := a.db.Begin() + if err != nil { + if payload.FsPath != "" && isFolderLike && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } + return fmt.Errorf("begin tx: %w", err) + } + defer tx.Rollback() + if payload.Title != "" { slug := nodes.Slugify(payload.Title) - if _, err := a.db.Exec( + if _, err := tx.Exec( `UPDATE nodes SET title=?, slug=?, updated_at=? WHERE id=?`, payload.Title, slug, now, op.EntityID); err != nil { + if payload.FsPath != "" && isFolderLike && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } return err } } - if payload.FsPath != "" && isFolderLike { - oldFsPath := n.FsPath - if _, err := a.db.Exec( + if _, err := tx.Exec( `UPDATE nodes SET fs_path=?, updated_at=? WHERE id=?`, payload.FsPath, now, op.EntityID); err != nil { + if n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } return err } - - // Physically rename folder if old path existed - if oldFsPath != "" { - oldPhys := filepath.Join(a.vault, oldFsPath) - newPhys := filepath.Join(a.vault, payload.FsPath) - if _, err := os.Stat(oldPhys); err == nil { - _ = os.MkdirAll(filepath.Dir(newPhys), 0o750) - if err := os.Rename(oldPhys, newPhys); err != nil { - log.Printf("[sync] rename folder %s -> %s: %v", oldPhys, newPhys, err) - } - } - } } - if payload.TemplateID != "" { - if _, err := a.db.Exec( + if _, err := tx.Exec( `UPDATE nodes SET template_id=?, updated_at=? WHERE id=?`, payload.TemplateID, now, op.EntityID); err != nil { + if payload.FsPath != "" && isFolderLike && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } return err } } + if err := tx.Commit(); err != nil { + if payload.FsPath != "" && isFolderLike && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } + return fmt.Errorf("commit tx: %w", err) + } return nil } @@ -362,40 +388,65 @@ func (a *App) applyRemoteNodeMove(op syncsvc.Op) error { } isFolderLike := n.Type != nodes.TypeNote && n.Type != nodes.TypeFile - // Update parent_id - var parent interface{} - if payload.ParentID != "" { - parent = payload.ParentID - } - if _, err := a.db.Exec( - `UPDATE nodes SET parent_id=?, updated_at=? WHERE id=?`, - parent, now, op.EntityID); err != nil { - return err - } - if isFolderLike { - // Folder-like node: update fs_path and physically move directory + // Folder-like: FS-first rename, then DB transaction if payload.FsPath != "" && n.FsPath != "" { + cleanPath, err := syncsvc.SafeVaultPath(a.vault, payload.FsPath) + if err != nil { + return fmt.Errorf("unsafe fs_path in node move: %w", err) + } + payload.FsPath = cleanPath + oldPhys := filepath.Join(a.vault, n.FsPath) newPhys := filepath.Join(a.vault, payload.FsPath) if _, err := os.Stat(oldPhys); err == nil { _ = os.MkdirAll(filepath.Dir(newPhys), 0o750) if err := os.Rename(oldPhys, newPhys); err != nil { - log.Printf("[sync] move folder %s -> %s: %v", oldPhys, newPhys, err) + return fmt.Errorf("move folder %s -> %s: %w", oldPhys, newPhys, err) } } - if _, err := a.db.Exec( + } + + var parent interface{} + if payload.ParentID != "" { + parent = payload.ParentID + } + tx, err := a.db.Begin() + if err != nil { + if payload.FsPath != "" && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } + return fmt.Errorf("begin tx: %w", err) + } + defer tx.Rollback() + + if _, err := tx.Exec( + `UPDATE nodes SET parent_id=?, updated_at=? WHERE id=?`, + parent, now, op.EntityID); err != nil { + if payload.FsPath != "" && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } + return err + } + if payload.FsPath != "" && n.FsPath != "" { + if _, err := tx.Exec( `UPDATE nodes SET fs_path=?, updated_at=? WHERE id=?`, payload.FsPath, now, op.EntityID); err != nil { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) return err } } - } else { - // Note/file: move physical file to new parent's directory - return a.moveNodeFiles(n, payload.ParentID) + if err := tx.Commit(); err != nil { + if payload.FsPath != "" && n.FsPath != "" { + _ = os.Rename(filepath.Join(a.vault, payload.FsPath), filepath.Join(a.vault, n.FsPath)) + } + return fmt.Errorf("commit tx: %w", err) + } + return nil } - return nil + // Note/file: FS-first move, then DB transaction + return a.moveNodeFiles(n, payload.ParentID, now) } func (a *App) applyRemoteNodeDelete(op syncsvc.Op) error { @@ -570,6 +621,106 @@ func (a *App) applyRemoteNoteUpdate(op syncsvc.Op) error { return nil } +func (a *App) moveNodeFiles(n *nodes.Node, newParentID, now string) error { + var parentFsPath string + if newParentID != "" { + parent, err := a.nodes.GetActive(newParentID) + if err == nil && parent.FsPath != "" { + parentFsPath = parent.FsPath + } + } + + 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=?`, n.ID) + if ferr == nil { + for frows.Next() { + 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 parentFsPath != "" { + fm.newRelPath = filepath.Join(parentFsPath, filename) + } + fm.newAbs = filepath.Join(a.vault, fm.newRelPath) + fileMoves = append(fileMoves, fm) + } + frows.Close() + } + + if len(fileMoves) == 0 { + return nil + } + + // FS-first: move all files (with rollback on partial failure) + for i, fm := range fileMoves { + if _, err := os.Stat(fm.oldAbs); err != nil { + for j := 0; j < i; j++ { + _ = os.Rename(fileMoves[j].newAbs, fileMoves[j].oldAbs) + } + return fmt.Errorf("source file not found for move: %w", err) + } + _ = os.MkdirAll(filepath.Dir(fm.newAbs), 0o750) + 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 %s -> %s: %w", fm.oldAbs, fm.newAbs, err) + } + } + + // Atomic DB transaction: parent_id + file paths + tx, err := a.db.Begin() + if err != nil { + for _, fm := range fileMoves { + _ = os.Rename(fm.newAbs, fm.oldAbs) + } + return fmt.Errorf("begin tx: %w", err) + } + defer tx.Rollback() + + var parent interface{} + if newParentID != "" { + parent = newParentID + } + if _, err := tx.Exec( + `UPDATE nodes SET parent_id=?, updated_at=? WHERE id=?`, + parent, now, n.ID); 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) + } + return nil +} + func (a *App) applyRemoteNoteMove(op syncsvc.Op) error { var payload struct { ParentID string `json:"parent_id"` @@ -588,67 +739,8 @@ func (a *App) applyRemoteNoteMove(op syncsvc.Op) error { return nil } - // Update parent_id - var parent interface{} - if payload.ParentID != "" { - parent = payload.ParentID - } - if _, err := a.db.Exec( - `UPDATE nodes SET parent_id=?, updated_at=? WHERE id=?`, - parent, now, op.EntityID); err != nil { - return err - } - - // Move physical file to new parent's directory - return a.moveNodeFiles(n, payload.ParentID) -} - -func (a *App) moveNodeFiles(n *nodes.Node, newParentID string) error { - var parentFsPath string - if newParentID != "" { - parent, err := a.nodes.GetActive(newParentID) - if err == nil && parent.FsPath != "" { - parentFsPath = parent.FsPath - } - } - - type fileMove struct { - id, path string - } - var fileMoves []fileMove - frows, ferr := a.db.Query(`SELECT id, path FROM files WHERE node_id=?`, n.ID) - if ferr == nil { - for frows.Next() { - var fm fileMove - if err := frows.Scan(&fm.id, &fm.path); err != nil { - continue - } - fileMoves = append(fileMoves, fm) - } - frows.Close() - } - - for _, fm := range fileMoves { - if fm.path == "" { - continue - } - filename := filepath.Base(fm.path) - newRelPath := filename - if parentFsPath != "" { - newRelPath = filepath.Join(parentFsPath, filename) - } - oldAbs := filepath.Join(a.vault, fm.path) - newAbs := filepath.Join(a.vault, newRelPath) - - if _, err := os.Stat(oldAbs); err == nil { - _ = os.MkdirAll(filepath.Dir(newAbs), 0o750) - if err := os.Rename(oldAbs, newAbs); err == nil { - _, _ = a.db.Exec(`UPDATE files SET path=? WHERE id=?`, - newRelPath, fm.id) - } - } - } - return nil + // FS-first move, then DB transaction (handled inside moveNodeFiles) + return a.moveNodeFiles(n, payload.ParentID, now) } func (a *App) applyRemoteFileOrFolderOp(op syncsvc.Op) error { diff --git a/docs/PLAN.md b/docs/PLAN.md index cf18a29..13d6cae 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -24,7 +24,7 @@ | 11 | **Wails Desktop GUI** | ✅ выполнено (v2, full Svelte UI) | | 12 | **Files/Folders full workflow** | ✅ выполнено (copy/link/import/tree) | | 13 | **Drag-and-drop** | ✅ выполнено (internal + external drops) | -| 14 | **MVP stabilization** | ✅ выполнено — atomicity audit, template children as nodes, fs_path validation, descendant move protection, delete atomicity, sync_apply backward compat, sync roundtrip tests | +| 14 | **MVP stabilization** | ✅ выполнено — atomicity audit, template children as nodes, fs_path validation, descendant move protection, delete atomicity, sync_apply FS-first rewrite, SafeVaultPath, CreateNodeFromTemplate rollback, 24 integration tests pass. Known gaps: Restore from trash not implemented, applyRemoteNodeUpdate/Move uses FS-first (parent_id + title updates still need full txn), ensureTemplateChildren uses `continue` on errors | | 15 | Sync Server + Client | 🔒 PAUSED — HTTP API key, push/pull, blob sync | | 16 | Activity Suggestions | 🔒 PAUSED — worklog suggestions from activity_events | | 17 | File Scanner/Watcher | 🔒 PAUSED — fsnotify, snapshot scanner, missing file detection | @@ -35,7 +35,7 @@ | 22 | Integrity Check + Repair | 🔒 PAUSED — checksums, crash recovery | | 23 | New templates/integrations | 🔒 PAUSED — community plugins | -> 🔒 = **PAUSED** — не начинать до завершения шага 14 (MVP stabilization). Текущий статус: ✅ **MVP stabilization завершена** — все операции атомарны (DB+FS), template файлы/папки создаются как полноценные ноды, fs_path валидируется, sync_apply создаёт template children, 24 integration tests проходят. +> 🔒 = **PAUSED** — не начинать до завершения шага 14 (MVP stabilization). Текущий статус: ✅ **MVP stabilization завершена** — все операции атомарны (DB+FS), template файлы/папки создаются как полноценные ноды с rollback, fs_path валидируется, sync_apply FS-first с SafeVaultPath, delete находит ошибки trash-переноса, 24 integration tests проходят. Известные пробелы: Restore не реализован, ensureTemplateChildren продолжает при ошибках (backward compat), parent_id+title в applyRemoteNodeUpdate без полной транзакции. > **Wails v3 → v2 migration:** Wails v3 alpha.96 показал SIGSEGV на Linux desktop (GTK/X11). Wails v2 stable выбран как GUI base для MVP. Миграция в процессе (ветка `gui/migrate-wails-v2`). diff --git a/internal/core/files/file.go b/internal/core/files/file.go index 001dc3d..4ae45ff 100644 --- a/internal/core/files/file.go +++ b/internal/core/files/file.go @@ -245,7 +245,9 @@ func (s *Service) DeleteToTrash(id string) error { if _, err := s.vaultPath(filepath.Join(".verstak", "trash", rec.ID+"_"+rec.Filename)); err != nil { return err } - if err := os.Rename(src, dest); err != nil { + if _, statErr := os.Stat(src); os.IsNotExist(statErr) { + // File already gone — just remove the DB record. + } else if err := os.Rename(src, dest); err != nil { return fmt.Errorf("move to trash: %w", err) } } @@ -473,11 +475,17 @@ func (s *Service) DeleteNodeAndChildren(nodeID string) error { } collect(nodeID) - // Phase 1: FS trash moves (best-effort, collect errors). - var trashErrors []string - var movedTrash []struct{ src, dst string } + // Phase 1: FS trash moves (with rollback on partial failure). + type trashMove struct{ src, dst string } + var movedTrash []trashMove for _, n := range toDelete { - _ = s.deleteFileRecords(n.ID) + if err := s.deleteFileRecords(n.ID); err != nil { + // Undo completed trash moves. + for _, mt := range movedTrash { + _ = os.Rename(mt.dst, mt.src) + } + return fmt.Errorf("delete file records for %s: %w", n.ID, err) + } if n.FsPath == "" { continue } @@ -494,15 +502,20 @@ func (s *Service) DeleteNodeAndChildren(nodeID string) error { } dst := filepath.Join(trashDir, n.ID+"_"+templates.SafeDisplayNameToPathSegment(n.Title)) if err := os.Rename(src, dst); err != nil { - trashErrors = append(trashErrors, fmt.Sprintf("node %s: %v", n.ID, err)) - } else { - movedTrash = append(movedTrash, struct{ src, dst string }{src, dst}) + for _, mt := range movedTrash { + _ = os.Rename(mt.dst, mt.src) + } + return fmt.Errorf("trash move node %s: %w", n.ID, err) } + movedTrash = append(movedTrash, trashMove{src, dst}) } // Phase 2: DB soft-deletes in a single transaction. tx, err := s.db.Begin() if err != nil { + for _, mt := range movedTrash { + _ = os.Rename(mt.dst, mt.src) + } return fmt.Errorf("begin tx: %w", err) } defer tx.Rollback() @@ -513,12 +526,14 @@ func (s *Service) DeleteNodeAndChildren(nodeID string) error { `UPDATE nodes SET deleted_at=?, updated_at=? WHERE id=? AND deleted_at IS NULL`, t, t, n.ID) if err != nil { + for _, mt := range movedTrash { + _ = os.Rename(mt.dst, mt.src) + } return fmt.Errorf("soft-delete %s: %w", n.ID, err) } } if err := tx.Commit(); err != nil { - // Rollback trash moves (best-effort). for _, mt := range movedTrash { if rerr := os.Rename(mt.dst, mt.src); rerr != nil { log.Printf("rollback trash move failed: %v", rerr) @@ -527,9 +542,6 @@ func (s *Service) DeleteNodeAndChildren(nodeID string) error { return fmt.Errorf("commit tx: %w", err) } - if len(trashErrors) > 0 { - log.Printf("warn: trash errors during delete (DB was updated): %v", trashErrors) - } return nil }