fix(api): normalise stored root paths

#243 normalised the incoming path but compared it against the value read
raw from the database, so a root stored with a trailing separator never
compared equal. Every edit of it -- a policy change included -- took the
relocation branch, where each planned destination is its own source and
the pre-check refuses. That root could not be edited at all.

`update` now normalises both sides, and hands `relocate_root` the
normalised stored value. `path_is_free` normalises the stored side in SQL
and `create` goes through it too, so `/mnt/x` and `/mnt/x/` cannot be two
roots for one directory -- the unique index compares raw strings and
cannot see that.

Migration 0031 strips the separator from rows already written. It skips
any row whose stripped form another row would also hold, rather than
tripping the unique index: a migration that cannot apply stops the daemon
booting, which is worse than two roots naming one directory.

Also from the same review: `undo` recorded only the leaf directory, so a
failed move into `/mnt/media-v2/tv/kids` left `tv` behind. It now records
every level `create_dir_all` materialised, deepest first, and still never
touches one that was already on disk.

Refs #244.

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

crates/arr-db/migrations/0031_normalise_root_paths.sql

@@ -0,0 +1,34 @@
+-- Issue #244. A root's path could be stored with a trailing separator until
+-- #243 normalised the incoming value. `roots::update` normalises the payload
+-- and compares it against the stored value, so such a row never compares
+-- equal: every edit, a policy change included, takes the relocation branch,
+-- and there every planned destination is its own source. Normalising the
+-- payload alone fixed the half that cannot bite; this is the other half.
+--
+-- `rtrim` strips every trailing separator at once, so '/mnt/x//' normalises
+-- in one pass. A bare '/' rtrims to the empty string and is put back, which
+-- is what `normalize_path` in arr-api does.
+--
+-- Guarded, because `roots.path` is UNIQUE and a migration that cannot apply
+-- stops the daemon booting -- worse than the bug it fixes. A row is
+-- normalised only when no other row shares its normalised path: neither a
+-- row already holding the stripped value, nor another trailing-separator row
+-- that would strip to the same thing. Every row in such a group is left
+-- exactly as it is. That leaves two roots naming one directory, which is a
+-- settings mistake for the operator to resolve by hand, not a reason to
+-- refuse to boot.
+--
+-- This cannot introduce a collision either. An updated row's new value is a
+-- normalised path no other row normalises to, and a row left alone whose raw
+-- path equalled that value would have had the same normalised path, which is
+-- the case the guard excludes.
+UPDATE roots
+SET path = CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END
+WHERE path <> CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END
+  AND NOT EXISTS (
+      SELECT 1
+      FROM roots AS other
+      WHERE other.id <> roots.id
+        AND CASE WHEN rtrim(other.path, '/') = '' THEN '/' ELSE rtrim(other.path, '/') END
+            = CASE WHEN rtrim(roots.path, '/') = '' THEN '/' ELSE rtrim(roots.path, '/') END
+  );

crates/arr-db/src/lib.rs

@@ -347,6 +347,93 @@ mod tests {
         assert_eq!(renamed_from_imported, "available");
     }
 
+    /// The four seeded roots, up to but not including migration 0031, with
+    /// the given legacy paths written straight into the rows.
+    async fn roots_before_normalisation(paths: &[(i64, &str)]) -> (tempfile::TempDir, Db) {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let db = Db::connect(dir.path().join("arr.db"))
+            .await
+            .expect("connect");
+        MIGRATOR
+            .run_to(30, db.pool())
+            .await
+            .expect("migrations before #244");
+        for (id, path) in paths {
+            sqlx::query("UPDATE roots SET path = ? WHERE id = ?")
+                .bind(path)
+                .bind(id)
+                .execute(db.pool())
+                .await
+                .expect("legacy path, valid under the unique index");
+        }
+        (dir, db)
+    }
+
+    async fn root_path(db: &Db, id: i64) -> String {
+        sqlx::query_scalar("SELECT path FROM roots WHERE id = ?")
+            .bind(id)
+            .fetch_one(db.pool())
+            .await
+            .expect("root path")
+    }
+
+    /// #244: a root's path could be stored with a trailing separator until
+    /// #243 normalised the incoming value, and `roots::update` compares a
+    /// normalised payload against the stored value — so such a row could
+    /// never be edited again. Migration 0031 strips the separator, and skips
+    /// a row whose stripped form another row already holds rather than
+    /// tripping the unique index and refusing to apply.
+    #[tokio::test]
+    async fn root_paths_are_normalised_but_never_onto_a_path_in_use() {
+        let (_dir, db) = roots_before_normalisation(&[
+            (1, "/mnt/media/movies/main/"),
+            (2, "/mnt/collide"),
+            (3, "/mnt/collide/"),
+            (4, "/mnt/media/tv/kids///"),
+        ])
+        .await;
+
+        db.migrate()
+            .await
+            .expect("0031 applies with a collision present");
+
+        assert_eq!(root_path(&db, 1).await, "/mnt/media/movies/main");
+        assert_eq!(
+            root_path(&db, 4).await,
+            "/mnt/media/tv/kids",
+            "every trailing separator goes in one pass"
+        );
+        assert_eq!(root_path(&db, 2).await, "/mnt/collide");
+        assert_eq!(
+            root_path(&db, 3).await,
+            "/mnt/collide/",
+            "left exactly as it is: normalising it would collide with root 2"
+        );
+    }
+
+    /// #244: the harder half of the same guard. Two rows that strip to the
+    /// same path, *neither* of which already holds the stripped value, still
+    /// have to be left alone — normalising them would collide with each
+    /// other, and a migration that cannot apply stops the daemon booting.
+    #[tokio::test]
+    async fn two_rows_that_would_collide_with_each_other_stop_nothing() {
+        let (_dir, db) =
+            roots_before_normalisation(&[(1, "/mnt/one/"), (2, "/mnt/dup/"), (3, "/mnt/dup//")])
+                .await;
+
+        db.migrate()
+            .await
+            .expect("0031 applies with a mutually colliding pair present");
+
+        assert_eq!(root_path(&db, 2).await, "/mnt/dup/");
+        assert_eq!(root_path(&db, 3).await, "/mnt/dup//");
+        assert_eq!(
+            root_path(&db, 1).await,
+            "/mnt/one",
+            "the rest of the table is still normalised"
+        );
+    }
+
     /// #155: the table-rebuild migrations (0007, 0014, 0021) run outside a
     /// transaction so `PRAGMA foreign_keys = OFF` holds and dropping the old
     /// tables does not cascade-delete `movie_releases`/`episode_releases`.