Add renaming slave folders to the master's spelling

The "N folders are named differently" warning now offers to rename the
slave folders to match the master, marked as the recommended first step
after a scan (also linked from the dashboard card).

- A preview lists each rename with the differing characters highlighted;
  renames are selectable, and conflicts (the slave already has a folder
  with the master's name, symlinks, the quarantine) are skipped with a
  reason.
- Only renames the server computes itself are applied; a request can
  only narrow them down, never name arbitrary paths. Only slave folders
  are renamed.
- Case-only renames go through a temporary name so they also work on
  case-insensitive filesystems; a rename interrupted by a crash is
  completed on startup (the folder is never removed).
- Renaming everything repeats until folders revealed inside renamed ones
  (an album under a renamed artist) are fixed too, then rescans.
- Each rename is an operation in History that can be undone (which also
  rescans). Dry run records renames as planned.

Also fixes the Review list not refreshing after a job that started and
finished between two status polls, and logs the new path of operations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/api/server.go

@@ -87,6 +87,8 @@ func (s *Server) Handler() http.Handler {
 				r.Get("/groups", s.handleGroups)
 				r.Post("/bulk/preview", s.handleBulkPreview)
 				r.Post("/bulk", s.handleBulk)
+				r.Get("/renames", s.handleRenamePreview)
+				r.Post("/renames", s.handleRenames)
 				r.Post("/link-identical", s.handleLinkIdentical)
 				r.Get("/tree", s.handleTree)
 				r.Post("/build", s.handleBuild)
@@ -634,3 +636,39 @@ func (s *Server) handleBulk(w http.ResponseWriter, r *http.Request) {
 		writeJSON(w, 202, j)
 	}
 }
+
+// ---- folder renames ----
+
+func (s *Server) handleRenamePreview(w http.ResponseWriter, r *http.Request) {
+	id, _ := idParam(r)
+	items, err := s.eng.PreviewRenames(r.Context(), id)
+	if err != nil {
+		opErr(w, err)
+		return
+	}
+	writeJSON(w, 200, map[string]any{"items": items, "dry_run": s.st.DryRun(r.Context())})
+}
+
+func (s *Server) handleRenames(w http.ResponseWriter, r *http.Request) {
+	id, _ := idParam(r)
+	var body struct {
+		// Only restricts the renames to these slave folders; empty renames all.
+		Only []string `json:"only"`
+	}
+	if !readJSON(w, r, &body) {
+		return
+	}
+	for _, p := range body.Only {
+		if _, err := fsops.CleanRel(p); err != nil {
+			writeErr(w, 400, err)
+			return
+		}
+	}
+	j, err := s.eng.StartRenames(id, body.Only)
+	if err != nil {
+		opErr(w, err)
+		return
+	}
+	s.log.Info("folder renames started", "pair", id, "selected", len(body.Only))
+	writeJSON(w, 202, j)
+}

internal/engine/engine.go

@@ -316,6 +316,11 @@ func layoutWarnings(l *libs) []store.Warning {
 			Detail: "Use Build to pick songs from the master library to hardlink into it."}}
 	}
 	if w := renamedWarning(renamedFolders(l)); w != nil {
+		for _, r := range planRenames(l) {
+			if r.Ready {
+				w.Fixable++
+			}
+		}
 		return []store.Warning{*w}
 	}
 	return nil

internal/engine/layout.go

@@ -100,8 +100,8 @@ func renamedWarning(pairs [][2]string) *store.Warning {
 	if len(pairs) == 0 {
 		return nil
 	}
-	w := &store.Warning{Total: len(pairs),
-		Detail: "Songs inside these folders can only match by path once the names agree. Rename one side so they match."}
+	w := &store.Warning{Code: store.WarnRenamedFolders, Total: len(pairs),
+		Detail: "Songs inside these folders can only match by path once the names agree. Renaming the slave folders to the master's spelling is the recommended first step after a scan."}
 	if len(pairs) == 1 {
 		w.Title = "1 folder is named differently in the two libraries"
 	} else {

internal/engine/ops.go

@@ -180,7 +180,7 @@ func (e *Engine) finish(ctx context.Context, o *store.Operation, err error) erro
 		e.log.Warn("operation failed", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "err", err)
 	} else {
 		o.Status = store.OpDone
-		e.log.Info("operation done", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "master", o.RelMaster)
+		e.log.Info("operation done", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "master", o.RelMaster, "new", o.RelNew)
 	}
 	if ferr := e.st.FinishOp(ctx, o.ID, o.Status, o.Error); ferr != nil && err == nil {
 		return ferr
@@ -625,6 +625,17 @@ func (e *Engine) Undo(ctx context.Context, opID int64) error {
 	if err != nil {
 		return err
 	}
+	if o.Action == store.ActRename {
+		if err := undoRename(l, o); err != nil {
+			return err
+		}
+		if err := e.st.MarkUndone(ctx, o.ID); err != nil {
+			return err
+		}
+		e.log.Info("folder rename undone", "op", o.ID, "folder", o.RelNew)
+		_, _ = e.StartScan(o.PairID, "undo") // paths changed; ErrBusy just means a job already runs
+		return nil
+	}
 	res := func(r fsops.Root, rel string) string {
 		p, e := r.Resolve(rel)
 		if e != nil && err == nil {
@@ -837,6 +848,13 @@ func (e *Engine) Recover(ctx context.Context) {
 		return
 	}
 	for _, o := range ops {
+		if o.Action == store.ActRename {
+			if p, err := e.st.Pair(ctx, o.PairID); err == nil {
+				recoverRename(p, o, e.log)
+			}
+			_ = e.st.FinishOp(ctx, o.ID, store.OpInterrupted, "interrupted by shutdown; rescan to verify state")
+			continue
+		}
 		if p, err := e.st.Pair(ctx, o.PairID); err == nil {
 			for _, root := range []string{p.SlaveRoot, p.MasterRoot} {
 				for _, rel := range []string{o.RelSlave, o.RelNew, o.RelMaster} {

internal/engine/rename.go

@@ -0,0 +1,233 @@
+package engine
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/hex"
+	"errors"
+	"fmt"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/cdnutter/playlink/internal/fsops"
+	"github.com/cdnutter/playlink/internal/store"
+)
+
+// Folder renames fix the "named differently" warning: slave folders that are
+// the same as a master folder under another spelling ("City Of Evil" vs
+// "City of Evil", "AC-DC" vs "AC_DC") are renamed to the master's spelling so
+// their songs match by path. Only the slave library is ever renamed.
+
+// FolderRename is one proposed rename of a slave folder.
+type FolderRename struct {
+	Slave  string `json:"slave"`  // current slave folder (relative)
+	Master string `json:"master"` // the master's spelling = the new slave name
+	Ready  bool   `json:"ready"`
+	Reason string `json:"reason,omitempty"` // why it cannot be renamed
+}
+
+// maxRenamePasses bounds the loop that picks up folders revealed by renaming
+// their parent (an artist rename can expose album folders that also differ).
+const maxRenamePasses = 3
+
+// planRenames lists the renames needed right now, marking conflicts.
+func planRenames(l *libs) []FolderRename {
+	var out []FolderRename
+	for _, p := range renamedFolders(l) {
+		r := FolderRename{Slave: p[0], Master: p[1], Ready: true}
+		r.Reason = checkRename(l, r.Slave, r.Master)
+		r.Ready = r.Reason == ""
+		out = append(out, r)
+	}
+	return out
+}
+
+// checkRename returns why a slave folder cannot be renamed, or "".
+func checkRename(l *libs, from, to string) string {
+	src, err := l.slave.Resolve(from)
+	if err != nil {
+		return err.Error()
+	}
+	dst, err := l.slave.Resolve(to)
+	if err != nil {
+		return err.Error()
+	}
+	fi, err := os.Lstat(src)
+	switch {
+	case err != nil:
+		return "folder no longer exists (rescan)"
+	case fi.Mode()&os.ModeSymlink != 0:
+		return "is a symlink"
+	case !fi.IsDir():
+		return "is not a folder"
+	case src == l.quarantine || fsops.Within(src, l.quarantine):
+		return "contains the quarantine folder"
+	}
+	if filepath.Dir(from) != filepath.Dir(to) {
+		return "rename would move the folder"
+	}
+	// A case-only rename "exists" on case-insensitive filesystems; that is fine.
+	if di, err := os.Lstat(dst); err == nil && !(strings.EqualFold(from, to) && os.SameFile(fi, di)) {
+		return fmt.Sprintf("the slave already has a folder named %q; merge the two by hand", filepath.Base(to))
+	}
+	return ""
+}
+
+// PreviewRenames lists the folder renames that would be made.
+func (e *Engine) PreviewRenames(ctx context.Context, pairID int64) ([]FolderRename, error) {
+	l, err := e.loadPair(ctx, pairID)
+	if err != nil {
+		return nil, err
+	}
+	out := planRenames(l)
+	if out == nil {
+		out = []FolderRename{}
+	}
+	return out, nil
+}
+
+// StartRenames renames slave folders to the master's spelling, then rescans.
+// With only set, just those renames are made (by slave path); they must be
+// among the renames the server computes itself, so a request can never name
+// arbitrary paths. Later passes (folders revealed inside renamed ones) only
+// run when renaming everything.
+func (e *Engine) StartRenames(pairID int64, only []string) (*Job, error) {
+	return e.startJob(pairID, "rename-folders", func(ctx context.Context, j *Job) error {
+		l, err := e.loadPair(ctx, pairID)
+		if err != nil {
+			return err
+		}
+		want := map[string]bool{}
+		for _, s := range only {
+			want[filepath.Clean(s)] = true
+		}
+		done := map[string]bool{}
+		for pass := 0; pass < maxRenamePasses; pass++ {
+			var todo []FolderRename
+			for _, r := range planRenames(l) {
+				if done[r.Slave] || (len(want) > 0 && !want[r.Slave]) {
+					continue
+				}
+				todo = append(todo, r)
+			}
+			if len(todo) == 0 {
+				break
+			}
+			j.phase(fmt.Sprintf("Renaming slave folders (pass %d)", pass+1), int64(len(todo)))
+			for _, r := range todo {
+				if ctx.Err() != nil {
+					return ctx.Err()
+				}
+				done[r.Slave] = true
+				if !r.Ready {
+					j.skip(r.Slave + ": " + r.Reason)
+				} else if err := e.renameOne(ctx, j, l, r); err != nil {
+					j.skip(r.Slave + ": " + err.Error())
+				}
+				j.tick()
+			}
+			if len(want) > 0 || e.st.DryRun(ctx) {
+				break // a dry run renames nothing, so there is nothing new to find
+			}
+		}
+		if e.st.DryRun(ctx) {
+			j.update(func(j *Job) { j.Message = "Dry run: renames recorded as planned in Activity" })
+			return nil
+		}
+		// Paths changed, so the stored matches are stale.
+		return e.scan(ctx, j, pairID, "rename")
+	})
+}
+
+func (e *Engine) renameOne(ctx context.Context, j *Job, l *libs, r FolderRename) error {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	if why := checkRename(l, r.Slave, r.Master); why != "" { // re-check under the lock
+		return errors.New(why)
+	}
+	src, _ := l.slave.Resolve(r.Slave)
+	dst, _ := l.slave.Resolve(r.Master)
+	o := store.Operation{PairID: l.pair.ID, Action: store.ActRename, RelSlave: r.Slave, RelNew: r.Master, Trigger: "rename"}
+	if err := e.begin(ctx, &o); err != nil {
+		return err
+	}
+	if o.DryRun {
+		j.count("planned")
+		return nil
+	}
+	err := renameDir(src, dst)
+	if err = e.finish(ctx, &o, err); err != nil {
+		return err
+	}
+	j.count("renamed")
+	return nil
+}
+
+// renameDir renames a folder. Case-only renames go through a temporary name
+// so they also work on case-insensitive filesystems. The destination must not
+// exist (checked by the caller under the engine lock).
+func renameDir(src, dst string) error {
+	if strings.EqualFold(src, dst) {
+		var b [6]byte
+		if _, err := rand.Read(b[:]); err != nil {
+			return err
+		}
+		tmp := filepath.Join(filepath.Dir(src), fsops.TmpPrefix+"dir-"+hex.EncodeToString(b[:]))
+		if err := os.Rename(src, tmp); err != nil {
+			return err
+		}
+		if err := os.Rename(tmp, dst); err != nil {
+			_ = os.Rename(tmp, src) // put it back
+			return err
+		}
+	} else if err := os.Rename(src, dst); err != nil {
+		return err
+	}
+	return fsops.SyncDir(filepath.Dir(dst))
+}
+
+// undoRename renames a folder back to its old name.
+func undoRename(l *libs, o store.Operation) error {
+	cur, err := l.slave.Resolve(o.RelNew)
+	if err != nil {
+		return err
+	}
+	old, err := l.slave.Resolve(o.RelSlave)
+	if err != nil {
+		return err
+	}
+	fi, err := os.Lstat(cur)
+	if err != nil || !fi.IsDir() {
+		return fmt.Errorf("the folder %q no longer exists", o.RelNew)
+	}
+	if di, err := os.Lstat(old); err == nil && !(strings.EqualFold(o.RelNew, o.RelSlave) && os.SameFile(fi, di)) {
+		return fmt.Errorf("%q exists again: %w", o.RelSlave, fsops.ErrExists)
+	}
+	return renameDir(cur, old)
+}
+
+// recoverRename finishes a case-only folder rename cut short by a crash: the
+// folder was moved to a temporary name and never reached its new name. The
+// folder holds user data, so it is renamed, never removed.
+func recoverRename(p store.Pair, o store.Operation, log *slog.Logger) {
+	dir := filepath.Join(p.SlaveRoot, filepath.Dir(o.RelSlave))
+	from := filepath.Join(p.SlaveRoot, o.RelSlave)
+	to := filepath.Join(p.SlaveRoot, o.RelNew)
+	if _, err := os.Lstat(from); err == nil {
+		return // never moved
+	}
+	if _, err := os.Lstat(to); err == nil {
+		return // completed
+	}
+	ents, _ := os.ReadDir(dir)
+	for _, en := range ents {
+		if en.IsDir() && strings.HasPrefix(en.Name(), fsops.TmpPrefix+"dir-") {
+			if err := os.Rename(filepath.Join(dir, en.Name()), to); err == nil {
+				log.Warn("finished interrupted folder rename", "op", o.ID, "folder", o.RelNew)
+			}
+			return
+		}
+	}
+}

internal/engine/rename_test.go

@@ -0,0 +1,163 @@
+package engine
+
+import (
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/cdnutter/playlink/internal/store"
+)
+
+func renameLibrary(t *testing.T, dryRun bool) *env {
+	ev := setup(t, dryRun)
+	ev.write(ev.master, "AC_DC/Back in Black/01 Hells Bells.flac", "hb")
+	ev.write(ev.slave, "AC-DC/Back In Black/01 Hells Bells.flac", "hb")
+	ev.write(ev.master, "Avenged Sevenfold/City of Evil/03 Bat Country.flac", "bc")
+	ev.write(ev.slave, "Avenged Sevenfold/City Of Evil/03 Bat Country.flac", "bc")
+	ev.scan()
+	return ev
+}
+
+func TestRenameFoldersToMasterSpelling(t *testing.T) {
+	ev := renameLibrary(t, false)
+	plan, err := ev.e.PreviewRenames(ctx, ev.pair.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(plan) != 2 || !plan[0].Ready || !plan[1].Ready {
+		t.Fatalf("want 2 ready renames, got %+v", plan)
+	}
+	if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil {
+		t.Fatal(err)
+	}
+	j := ev.wait()
+	// The artist rename reveals "Back In Black" inside it: a second pass fixes it.
+	if j.Results["renamed"] != 3 {
+		t.Fatalf("want 3 renames over two passes, got %+v %v", j.Results, j.Skipped)
+	}
+	for _, rel := range []string{"AC_DC/Back in Black/01 Hells Bells.flac", "Avenged Sevenfold/City of Evil/03 Bat Country.flac"} {
+		if ev.read(filepath.Join(ev.slave, rel)) == "" {
+			t.Fatalf("%s missing after rename", rel)
+		}
+		// The job rescans, so the songs now match by path.
+		if m := ev.match(rel); m.Status != store.StatusIdentical || m.Kind != store.KindPath {
+			t.Fatalf("%s should match by path after the rename, got %s/%s", rel, m.Status, m.Kind)
+		}
+	}
+	p, _ := ev.st.Pair(ctx, ev.pair.ID)
+	for _, w := range p.LastScanWarnings {
+		if w.Code == store.WarnRenamedFolders {
+			t.Fatalf("renamed-folders warning should be gone, got %+v", w)
+		}
+	}
+	if ev.read(filepath.Join(ev.master, "AC_DC/Back in Black/01 Hells Bells.flac")) != "hb" {
+		t.Fatal("the master library must not change")
+	}
+
+	// Undo the artist rename from History.
+	ops, _, _ := ev.st.Ops(ctx, store.OpFilter{PairID: ev.pair.ID})
+	var artist store.Operation
+	for _, o := range ops {
+		if o.Action == store.ActRename && o.RelSlave == "AC-DC" {
+			artist = o
+		}
+	}
+	if artist.ID == 0 || artist.RelNew != "AC_DC" {
+		t.Fatalf("rename not recorded in history: %+v", ops)
+	}
+	if err := ev.e.Undo(ctx, artist.ID); err != nil {
+		t.Fatal(err)
+	}
+	ev.wait() // undo starts a rescan
+	if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC/Back in Black/01 Hells Bells.flac")); err != nil {
+		t.Fatalf("undo should rename the folder back: %v", err)
+	}
+}
+
+func TestRenameConflictIsSkipped(t *testing.T) {
+	ev := setup(t, false)
+	ev.write(ev.master, "Band/Album Name/01 a.flac", "a")
+	ev.write(ev.slave, "Band/Album name/01 a.flac", "a")
+	ev.write(ev.slave, "Band/Album Name/02 b.flac", "b") // the master spelling already exists too
+	ev.scan()
+	plan, _ := ev.e.PreviewRenames(ctx, ev.pair.ID)
+	if len(plan) != 1 || plan[0].Ready || plan[0].Reason == "" {
+		t.Fatalf("want one conflicting rename, got %+v", plan)
+	}
+	if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil {
+		t.Fatal(err)
+	}
+	j := ev.wait()
+	if j.Results["renamed"] != 0 || j.Results["skipped"] != 1 {
+		t.Fatalf("conflict must be skipped, got %+v", j.Results)
+	}
+	if ev.read(filepath.Join(ev.slave, "Band/Album name/01 a.flac")) != "a" || ev.read(filepath.Join(ev.slave, "Band/Album Name/02 b.flac")) != "b" {
+		t.Fatal("nothing may change on a conflict")
+	}
+}
+
+func TestRenameOnlySelectedAndNeverArbitraryPaths(t *testing.T) {
+	ev := renameLibrary(t, false)
+	ev.write(ev.slave, "Unrelated/x.flac", "x")
+	if _, err := ev.e.StartRenames(ev.pair.ID, []string{"Avenged Sevenfold/City Of Evil", "Unrelated"}); err != nil {
+		t.Fatal(err)
+	}
+	j := ev.wait()
+	if j.Results["renamed"] != 1 {
+		t.Fatalf("only the selected, computed rename may run, got %+v", j.Results)
+	}
+	if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC")); err != nil {
+		t.Fatal("unselected folders must not be renamed")
+	}
+	if _, err := os.Stat(filepath.Join(ev.slave, "Unrelated/x.flac")); err != nil {
+		t.Fatal("a folder that is not a computed rename must never be touched")
+	}
+}
+
+func TestRenameDryRun(t *testing.T) {
+	ev := renameLibrary(t, true)
+	if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil {
+		t.Fatal(err)
+	}
+	j := ev.wait()
+	if j.Results["planned"] != 2 {
+		t.Fatalf("dry run should plan 2 renames, got %+v", j.Results)
+	}
+	if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC")); err != nil {
+		t.Fatal("dry run must not rename anything")
+	}
+}
+
+func TestRecoverInterruptedCaseRename(t *testing.T) {
+	root := t.TempDir()
+	os.MkdirAll(filepath.Join(root, "Band", ".playlink-tmp-dir-abc123"), 0o755)
+	os.WriteFile(filepath.Join(root, "Band", ".playlink-tmp-dir-abc123", "song.flac"), []byte("data"), 0o644)
+	p := store.Pair{SlaveRoot: root}
+	o := store.Operation{ID: 1, Action: store.ActRename, RelSlave: "Band/City Of Evil", RelNew: "Band/City of Evil"}
+	recoverRename(p, o, slog.New(slog.NewTextHandler(io.Discard, nil)))
+	if b, err := os.ReadFile(filepath.Join(root, "Band", "City of Evil", "song.flac")); err != nil || string(b) != "data" {
+		t.Fatalf("interrupted rename should be completed, got %v", err)
+	}
+}
+
+func TestRenameWarningCountsFixable(t *testing.T) {
+	ev := setup(t, false)
+	ev.write(ev.master, "Band/Album Name/01 a.flac", "a")
+	ev.write(ev.slave, "Band/Album name/01 a.flac", "a")
+	ev.write(ev.slave, "Band/Album Name/02 b.flac", "b") // conflict
+	ev.write(ev.master, "Other/Record/01 c.flac", "c")
+	ev.write(ev.slave, "Other/record/01 c.flac", "c") // renameable
+	ev.scan()
+	p, _ := ev.st.Pair(ctx, ev.pair.ID)
+	for _, w := range p.LastScanWarnings {
+		if w.Code == store.WarnRenamedFolders {
+			if w.Total != 2 || w.Fixable != 1 {
+				t.Fatalf("want 2 renamed folders, 1 fixable; got total=%d fixable=%d", w.Total, w.Fixable)
+			}
+			return
+		}
+	}
+	t.Fatal("renamed-folders warning missing")
+}

internal/store/models.go

@@ -93,15 +93,22 @@ func (s *Store) PruneSessions(ctx context.Context) {
 
 // ---- warnings ----
 
+// Warning codes.
+const WarnRenamedFolders = "renamed_folders"
+
 // Warning is a scan or validation notice, structured so the UI can lay it
 // out (a heading, an explanation and a table of affected folders) rather
 // than print one long sentence.
 type Warning struct {
+	// Code identifies warnings the UI offers a fix for.
+	Code   string        `json:"code,omitempty"`
 	Title  string        `json:"title"`
 	Detail string        `json:"detail,omitempty"`
 	Items  []WarningItem `json:"items,omitempty"`
 	// Total is the full number of items when Items was truncated.
 	Total int `json:"total,omitempty"`
+	// Fixable is how many items the offered fix can handle automatically.
+	Fixable int `json:"fixable,omitempty"`
 }
 
 // WarningItem pairs a slave path with the master path it corresponds to.
@@ -645,6 +652,7 @@ const (
 	ActPromote = "promote" // master replaced by slave's file, then linked
 	ActBuild   = "build"   // master file linked into the slave (fresh library)
 	ActRemove  = "remove"  // slave hardlink removed (master copy kept)
+	ActRename  = "rename"  // slave folder renamed to the master's spelling
 )
 
 // Operation statuses.