Replace move_task Option params with Placement enum
State-mismatch validation and auto-reorder on state change.

move_task now takes &Task and Placement (Before/After/Between)
instead of id-based Option<i64> pairs. The enum eliminates
invalid states at the type level — no more (None, None) case.

State changes via edit auto-reposition: moving up (toward done)
appends to the target group, moving down (toward icebox) prepends.

Assisted-by: Claude Opus 4.6 via pi
change onlntuqutqxszwmyrykzwmxznxwnrzzl
commit 6f1d5752b2051c8e6e33a552b932333907293405
author Alpha Chen <alpha@kejadlen.dev>
date
parent ospzysnk
diff --git a/src/bin/ranger/commands/task.rs b/src/bin/ranger/commands/task.rs
index 7c37a50..f863c9d 100644
--- a/src/bin/ranger/commands/task.rs
+++ b/src/bin/ranger/commands/task.rs
@@ -1,12 +1,13 @@
 use clap::{Args, Subcommand};
-use color_eyre::eyre::Result;
+use color_eyre::eyre::{Result, bail};
 use ranger::db::{SqliteConnection, SqlitePool};
 use ranger::models::{State, Task};
 use ranger::ops;
+use ranger::ops::task::Placement;
 
 use crate::output;
 
-/// Positioning flags shared by create and move.
+/// Positioning flags shared by create, edit, and move.
 #[derive(Args)]
 pub struct PositionArgs {
     /// Place before this task key
@@ -18,18 +19,39 @@ pub struct PositionArgs {
 }
 
 impl PositionArgs {
-    async fn resolve(self, conn: &mut SqliteConnection) -> Result<(Option<i64>, Option<i64>)> {
-        let before_id = if let Some(k) = &self.before {
-            Some(ops::task::get_by_key_prefix(conn, k).await?.id)
-        } else {
-            None
-        };
-        let after_id = if let Some(k) = &self.after {
-            Some(ops::task::get_by_key_prefix(conn, k).await?.id)
-        } else {
-            None
-        };
-        Ok((before_id, after_id))
+    async fn resolve(self, conn: &mut SqliteConnection) -> Result<Option<PositionAnchors>> {
+        match (self.before, self.after) {
+            (None, None) => Ok(None),
+            (Some(b), None) => {
+                let before = ops::task::get_by_key_prefix(conn, &b).await?;
+                Ok(Some(PositionAnchors::Before(before)))
+            }
+            (None, Some(a)) => {
+                let after = ops::task::get_by_key_prefix(conn, &a).await?;
+                Ok(Some(PositionAnchors::After(after)))
+            }
+            (Some(b), Some(a)) => {
+                let before = ops::task::get_by_key_prefix(conn, &b).await?;
+                let after = ops::task::get_by_key_prefix(conn, &a).await?;
+                Ok(Some(PositionAnchors::Between { before, after }))
+            }
+        }
+    }
+}
+
+enum PositionAnchors {
+    Before(Task),
+    After(Task),
+    Between { before: Task, after: Task },
+}
+
+impl PositionAnchors {
+    fn as_placement(&self) -> Placement<'_> {
+        match self {
+            PositionAnchors::Before(t) => Placement::Before(t),
+            PositionAnchors::After(t) => Placement::After(t),
+            PositionAnchors::Between { before, after } => Placement::Between { before, after },
+        }
     }
 }
 
@@ -127,7 +149,7 @@ pub async fn run(pool: &SqlitePool, command: TaskCommands, json: bool) -> Result
             } else {
                 None
             };
-            let (before_id, after_id) = position.resolve(&mut tx).await?;
+            let anchors = position.resolve(&mut tx).await?;
             let state = state.map(|s| s.parse::<State>()).transpose()?;
 
             let task = ops::task::create(
@@ -142,8 +164,8 @@ pub async fn run(pool: &SqlitePool, command: TaskCommands, json: bool) -> Result
             )
             .await?;
 
-            if before_id.is_some() || after_id.is_some() {
-                ops::task::move_task(&mut tx, task.id, before_id, after_id).await?;
+            if let Some(ref anchors) = anchors {
+                ops::task::move_task(&mut tx, &task, anchors.as_placement()).await?;
             }
 
             if let Some(tags) = &tag {
@@ -228,10 +250,9 @@ pub async fn run(pool: &SqlitePool, command: TaskCommands, json: bool) -> Result
         } => {
             let mut conn = pool.acquire().await?;
             let state = state.map(|s| s.parse::<State>()).transpose()?;
+            let anchors = position.resolve(&mut conn).await?;
 
             let task = ops::task::get_by_key_prefix(&mut conn, &key).await?;
-            let (before_id, after_id) = position.resolve(&mut conn).await?;
-
             let updated = ops::task::edit(
                 &mut conn,
                 task.id,
@@ -241,8 +262,8 @@ pub async fn run(pool: &SqlitePool, command: TaskCommands, json: bool) -> Result
             )
             .await?;
 
-            if before_id.is_some() || after_id.is_some() {
-                ops::task::move_task(&mut conn, updated.id, before_id, after_id).await?;
+            if let Some(ref anchors) = anchors {
+                ops::task::move_task(&mut conn, &updated, anchors.as_placement()).await?;
             }
 
             output::print(&updated, json, print_task);
@@ -250,10 +271,15 @@ pub async fn run(pool: &SqlitePool, command: TaskCommands, json: bool) -> Result
         TaskCommands::Move { key, position } => {
             let mut conn = pool.acquire().await?;
             let task = ops::task::get_by_key_prefix(&mut conn, &key).await?;
-            let (before_id, after_id) = position.resolve(&mut conn).await?;
+            let anchors = position.resolve(&mut conn).await?;
 
-            ops::task::move_task(&mut conn, task.id, before_id, after_id).await?;
-            println!("Moved {} {}", &task.key[..8], task.title);
+            match anchors {
+                Some(anchors) => {
+                    ops::task::move_task(&mut conn, &task, anchors.as_placement()).await?;
+                    println!("Moved {} {}", &task.key[..8], task.title);
+                }
+                None => bail!("--before or --after is required"),
+            }
         }
         TaskCommands::Delete { key } => {
             let mut conn = pool.acquire().await?;
diff --git a/src/error.rs b/src/error.rs
index 8df5d6f..17ad532 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -4,6 +4,11 @@ pub enum RangerError {
     KeyNotFound(String),
     #[error("ambiguous prefix '{0}' matches multiple keys")]
     AmbiguousPrefix(String),
+    #[error("can't move {task_state} task relative to {anchor_state} task")]
+    StateMismatch {
+        task_state: String,
+        anchor_state: String,
+    },
     #[error("database error: {0}")]
     Db(#[from] sqlx::Error),
     #[error("migration error: {0}")]
diff --git a/src/ops/task.rs b/src/ops/task.rs
index 546a2ad..2d336b0 100644
--- a/src/ops/task.rs
+++ b/src/ops/task.rs
@@ -145,7 +145,7 @@ pub async fn edit(
             .await?;
 
         if old_state != *new_state {
-            reposition_for_state_change(&mut *conn, task_id, &old_state, new_state).await?;
+            reorder(&mut *conn, task_id, &old_state, new_state).await?;
         }
     }
 
@@ -157,11 +157,60 @@ pub async fn edit(
     Ok(task)
 }
 
-/// Reposition a task when its state changes.
+pub enum Placement<'a> {
+    Before(&'a Task),
+    After(&'a Task),
+    Between { after: &'a Task, before: &'a Task },
+}
+
+impl<'a> Placement<'a> {
+    pub fn anchors(&self) -> impl Iterator<Item = &Task> {
+        match self {
+            Placement::Before(t) | Placement::After(t) => vec![*t],
+            Placement::Between { after, before } => vec![*after, *before],
+        }
+        .into_iter()
+    }
+}
+
+pub async fn move_task(
+    conn: &mut SqliteConnection,
+    task: &Task,
+    placement: Placement<'_>,
+) -> Result<(), RangerError> {
+    for anchor in placement.anchors() {
+        if anchor.state != task.state {
+            return Err(RangerError::StateMismatch {
+                task_state: task.state.to_string(),
+                anchor_state: anchor.state.to_string(),
+            });
+        }
+    }
+
+    let new_pos = match placement {
+        Placement::After(anchor) => {
+            let next =
+                next_position_after(&mut *conn, task.backlog_id, task.id, &anchor.position).await?;
+            position::between(&anchor.position, next.as_deref().unwrap_or(""))
+        }
+        Placement::Before(anchor) => {
+            let prev = prev_position_before(&mut *conn, task.backlog_id, task.id, &anchor.position)
+                .await?;
+            position::between(prev.as_deref().unwrap_or(""), &anchor.position)
+        }
+        Placement::Between { after, before } => {
+            position::between(&after.position, &before.position)
+        }
+    };
+
+    set_position(&mut *conn, task.id, &new_pos).await
+}
+
+/// Reorder a task's position when its state changes.
 ///
 /// Moving up (toward done): place at end of target state group.
 /// Moving down (toward icebox): place at beginning of target state group.
-async fn reposition_for_state_change(
+async fn reorder(
     conn: &mut SqliteConnection,
     task_id: i64,
     old_state: &State,
@@ -175,177 +224,154 @@ async fn reposition_for_state_change(
     let moving_up = new_state.rank() > old_state.rank();
 
     let new_pos = if moving_up {
-        // End of target state group: after the last task with this state
-        let last_in_state: Option<String> = sqlx::query_scalar(
-            "SELECT position FROM tasks \
-             WHERE backlog_id = ? AND state = ? AND id != ? \
-             ORDER BY position DESC LIMIT 1",
-        )
-        .bind(backlog_id)
-        .bind(new_state.as_str())
-        .bind(task_id)
-        .fetch_optional(&mut *conn)
-        .await?;
+        let last_in_state =
+            last_position_in_state(&mut *conn, backlog_id, task_id, new_state).await?;
 
         match last_in_state {
             Some(last) => {
-                let next: Option<String> = sqlx::query_scalar(
-                    "SELECT position FROM tasks \
-                     WHERE backlog_id = ? AND id != ? AND position > ? \
-                     ORDER BY position ASC LIMIT 1",
-                )
-                .bind(backlog_id)
-                .bind(task_id)
-                .bind(&last)
-                .fetch_optional(&mut *conn)
-                .await?;
+                let next = next_position_after(&mut *conn, backlog_id, task_id, &last).await?;
                 position::between(&last, next.as_deref().unwrap_or(""))
             }
             None => {
-                // No other tasks in this state — place at end of backlog
-                let last_pos: Option<String> = sqlx::query_scalar(
-                    "SELECT position FROM tasks \
-                     WHERE backlog_id = ? AND id != ? \
-                     ORDER BY position DESC LIMIT 1",
-                )
-                .bind(backlog_id)
-                .bind(task_id)
-                .fetch_optional(&mut *conn)
-                .await?;
-                position::between(last_pos.as_deref().unwrap_or(""), "")
+                let last = last_position(&mut *conn, backlog_id, task_id).await?;
+                position::between(last.as_deref().unwrap_or(""), "")
             }
         }
     } else {
-        // Beginning of target state group: before the first task with this state
-        let first_in_state: Option<String> = sqlx::query_scalar(
-            "SELECT position FROM tasks \
-             WHERE backlog_id = ? AND state = ? AND id != ? \
-             ORDER BY position ASC LIMIT 1",
-        )
-        .bind(backlog_id)
-        .bind(new_state.as_str())
-        .bind(task_id)
-        .fetch_optional(&mut *conn)
-        .await?;
+        let first_in_state =
+            first_position_in_state(&mut *conn, backlog_id, task_id, new_state).await?;
 
         match first_in_state {
             Some(first) => {
-                let prev: Option<String> = sqlx::query_scalar(
-                    "SELECT position FROM tasks \
-                     WHERE backlog_id = ? AND id != ? AND position < ? \
-                     ORDER BY position DESC LIMIT 1",
-                )
-                .bind(backlog_id)
-                .bind(task_id)
-                .bind(&first)
-                .fetch_optional(&mut *conn)
-                .await?;
+                let prev = prev_position_before(&mut *conn, backlog_id, task_id, &first).await?;
                 position::between(prev.as_deref().unwrap_or(""), &first)
             }
             None => {
-                // No other tasks in this state — place at beginning of backlog
-                let first_pos: Option<String> = sqlx::query_scalar(
-                    "SELECT position FROM tasks \
-                     WHERE backlog_id = ? AND id != ? \
-                     ORDER BY position ASC LIMIT 1",
-                )
-                .bind(backlog_id)
-                .bind(task_id)
-                .fetch_optional(&mut *conn)
-                .await?;
-                position::between("", first_pos.as_deref().unwrap_or(""))
+                let first = first_position(&mut *conn, backlog_id, task_id).await?;
+                position::between("", first.as_deref().unwrap_or(""))
             }
         }
     };
 
-    sqlx::query("UPDATE tasks SET position = ? WHERE id = ?")
-        .bind(&new_pos)
-        .bind(task_id)
-        .execute(&mut *conn)
-        .await?;
+    set_position(&mut *conn, task_id, &new_pos).await
+}
 
-    Ok(())
+// -- Position query helpers --
+
+async fn last_position(
+    conn: &mut SqliteConnection,
+    backlog_id: i64,
+    exclude_task_id: i64,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND id != ? \
+         ORDER BY position DESC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(exclude_task_id)
+    .fetch_optional(&mut *conn)
+    .await?)
 }
 
-pub async fn move_task(
+async fn first_position(
     conn: &mut SqliteConnection,
-    task_id: i64,
-    before_task_id: Option<i64>,
-    after_task_id: Option<i64>,
-) -> Result<(), RangerError> {
-    // Look up the task's backlog_id so position queries are scoped correctly
-    let backlog_id: i64 = sqlx::query_scalar("SELECT backlog_id FROM tasks WHERE id = ?")
-        .bind(task_id)
-        .fetch_one(&mut *conn)
-        .await?;
+    backlog_id: i64,
+    exclude_task_id: i64,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND id != ? \
+         ORDER BY position ASC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(exclude_task_id)
+    .fetch_optional(&mut *conn)
+    .await?)
+}
 
-    let upper: Option<String> = if let Some(id) = before_task_id {
-        sqlx::query_scalar("SELECT position FROM tasks WHERE id = ? AND backlog_id = ?")
-            .bind(id)
-            .bind(backlog_id)
-            .fetch_optional(&mut *conn)
-            .await?
-    } else {
-        None
-    };
+async fn next_position_after(
+    conn: &mut SqliteConnection,
+    backlog_id: i64,
+    exclude_task_id: i64,
+    pos: &str,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND id != ? AND position > ? \
+         ORDER BY position ASC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(exclude_task_id)
+    .bind(pos)
+    .fetch_optional(&mut *conn)
+    .await?)
+}
 
-    let lower: Option<String> = if let Some(id) = after_task_id {
-        sqlx::query_scalar("SELECT position FROM tasks WHERE id = ? AND backlog_id = ?")
-            .bind(id)
-            .bind(backlog_id)
-            .fetch_optional(&mut *conn)
-            .await?
-    } else {
-        None
-    };
+async fn prev_position_before(
+    conn: &mut SqliteConnection,
+    backlog_id: i64,
+    exclude_task_id: i64,
+    pos: &str,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND id != ? AND position < ? \
+         ORDER BY position DESC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(exclude_task_id)
+    .bind(pos)
+    .fetch_optional(&mut *conn)
+    .await?)
+}
 
-    let new_pos = match (lower.as_deref(), upper.as_deref()) {
-        (None, None) => {
-            let last_pos: Option<String> = sqlx::query_scalar(
-                "SELECT position FROM tasks \
-                 WHERE backlog_id = ? \
-                 ORDER BY position DESC LIMIT 1",
-            )
-            .bind(backlog_id)
-            .fetch_optional(&mut *conn)
-            .await?;
-            position::between(last_pos.as_deref().unwrap_or(""), "")
-        }
-        (Some(low), None) => {
-            let next: Option<String> = sqlx::query_scalar(
-                "SELECT position FROM tasks \
-                 WHERE backlog_id = ? AND id != ? AND position > ? \
-                 ORDER BY position ASC LIMIT 1",
-            )
-            .bind(backlog_id)
-            .bind(task_id)
-            .bind(low)
-            .fetch_optional(&mut *conn)
-            .await?;
-            position::between(low, next.as_deref().unwrap_or(""))
-        }
-        (None, Some(up)) => {
-            let prev: Option<String> = sqlx::query_scalar(
-                "SELECT position FROM tasks \
-                 WHERE backlog_id = ? AND id != ? AND position < ? \
-                 ORDER BY position DESC LIMIT 1",
-            )
-            .bind(backlog_id)
-            .bind(task_id)
-            .bind(up)
-            .fetch_optional(&mut *conn)
-            .await?;
-            position::between(prev.as_deref().unwrap_or(""), up)
-        }
-        (Some(low), Some(up)) => position::between(low, up),
-    };
+async fn last_position_in_state(
+    conn: &mut SqliteConnection,
+    backlog_id: i64,
+    exclude_task_id: i64,
+    state: &State,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND state = ? AND id != ? \
+         ORDER BY position DESC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(state.as_str())
+    .bind(exclude_task_id)
+    .fetch_optional(&mut *conn)
+    .await?)
+}
 
+async fn first_position_in_state(
+    conn: &mut SqliteConnection,
+    backlog_id: i64,
+    exclude_task_id: i64,
+    state: &State,
+) -> Result<Option<String>, RangerError> {
+    Ok(sqlx::query_scalar(
+        "SELECT position FROM tasks \
+         WHERE backlog_id = ? AND state = ? AND id != ? \
+         ORDER BY position ASC LIMIT 1",
+    )
+    .bind(backlog_id)
+    .bind(state.as_str())
+    .bind(exclude_task_id)
+    .fetch_optional(&mut *conn)
+    .await?)
+}
+
+async fn set_position(
+    conn: &mut SqliteConnection,
+    task_id: i64,
+    pos: &str,
+) -> Result<(), RangerError> {
     sqlx::query("UPDATE tasks SET position = ? WHERE id = ?")
-        .bind(&new_pos)
+        .bind(pos)
         .bind(task_id)
         .execute(&mut *conn)
         .await?;
-
     Ok(())
 }
 
@@ -540,14 +566,14 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn move_task_before() {
+    async fn delete_task() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
-        let t1 = create(
+        let task = create(
             &mut conn,
             CreateTask {
-                title: "First",
+                title: "Delete me",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -556,10 +582,25 @@ mod tests {
         )
         .await
         .unwrap();
-        let t2 = create(
+
+        delete(&mut conn, task.id).await.unwrap();
+
+        let result = get_by_key_prefix(&mut conn, &task.key).await;
+        assert!(result.is_err());
+
+        let tasks = list(&mut conn, bl.id, None).await.unwrap();
+        assert_eq!(tasks.len(), 0);
+    }
+
+    #[tokio::test]
+    async fn get_by_id_returns_task() {
+        let pool = test_pool().await;
+        let mut conn = pool.acquire().await.unwrap();
+        let bl = backlog::create(&mut conn, "Test").await.unwrap();
+        let task = create(
             &mut conn,
             CreateTask {
-                title: "Second",
+                title: "Find by id",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -568,10 +609,43 @@ mod tests {
         )
         .await
         .unwrap();
-        let t3 = create(
+
+        let found = get_by_id(&mut conn, task.id).await.unwrap();
+        assert_eq!(found.title, "Find by id");
+    }
+
+    #[tokio::test]
+    async fn get_by_key_prefix_ambiguous() {
+        let pool = test_pool().await;
+        let mut conn = pool.acquire().await.unwrap();
+        let bl = backlog::create(&mut conn, "Test").await.unwrap();
+
+        // Insert two tasks with keys that share a prefix, bypassing
+        // the random key generator
+        sqlx::query("INSERT INTO tasks (key, backlog_id, title, state, position) VALUES ('kkkkaaaa', ?, 'First', 'icebox', 'a')")
+            .bind(bl.id)
+            .execute(&mut *conn)
+            .await
+            .unwrap();
+        sqlx::query("INSERT INTO tasks (key, backlog_id, title, state, position) VALUES ('kkkkbbbb', ?, 'Second', 'icebox', 'b')")
+            .bind(bl.id)
+            .execute(&mut *conn)
+            .await
+            .unwrap();
+
+        let result = get_by_key_prefix(&mut conn, "kkkk").await;
+        assert!(result.is_err());
+    }
+
+    #[tokio::test]
+    async fn edit_title_only() {
+        let pool = test_pool().await;
+        let mut conn = pool.acquire().await.unwrap();
+        let bl = backlog::create(&mut conn, "Test").await.unwrap();
+        let task = create(
             &mut conn,
             CreateTask {
-                title: "Third",
+                title: "Original",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -581,19 +655,15 @@ mod tests {
         .await
         .unwrap();
 
-        // Move t3 before t1 — should produce order: t3, t1, t2
-        move_task(&mut conn, t3.id, Some(t1.id), None)
+        let updated = edit(&mut conn, task.id, Some("New title"), None, None)
             .await
             .unwrap();
-
-        let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t3.id, "t3 should be first");
-        assert_eq!(tasks[1].id, t1.id, "t1 should be second");
-        assert_eq!(tasks[2].id, t2.id, "t2 should be third");
+        assert_eq!(updated.title, "New title");
+        assert_eq!(updated.state, State::Icebox);
     }
 
     #[tokio::test]
-    async fn move_task_after() {
+    async fn move_task_to_end() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
@@ -621,39 +691,26 @@ mod tests {
         )
         .await
         .unwrap();
-        let t3 = create(
-            &mut conn,
-            CreateTask {
-                title: "Third",
-                backlog_id: bl.id,
-                state: None,
-                parent_id: None,
-                description: None,
-            },
-        )
-        .await
-        .unwrap();
 
-        // Move t1 after t3 — should produce order: t2, t3, t1
-        move_task(&mut conn, t1.id, None, Some(t3.id))
+        // Move first task after second
+        move_task(&mut conn, &t1, Placement::After(&t2))
             .await
             .unwrap();
 
         let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t2.id, "t2 should be first");
-        assert_eq!(tasks[1].id, t3.id, "t3 should be second");
-        assert_eq!(tasks[2].id, t1.id, "t1 should be third");
+        assert_eq!(tasks[0].id, t2.id);
+        assert_eq!(tasks[1].id, t1.id);
     }
 
     #[tokio::test]
-    async fn move_task_after_into_middle() {
+    async fn move_task_before() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
         let t1 = create(
             &mut conn,
             CreateTask {
-                title: "First",
+                title: "A",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -665,7 +722,7 @@ mod tests {
         let t2 = create(
             &mut conn,
             CreateTask {
-                title: "Second",
+                title: "B",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -677,7 +734,7 @@ mod tests {
         let t3 = create(
             &mut conn,
             CreateTask {
-                title: "Third",
+                title: "C",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -687,26 +744,25 @@ mod tests {
         .await
         .unwrap();
 
-        // Move t3 after t1 (but before t2) — should produce order: t1, t3, t2
-        move_task(&mut conn, t3.id, None, Some(t1.id))
+        move_task(&mut conn, &t3, Placement::Before(&t1))
             .await
             .unwrap();
 
         let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t1.id, "t1 should be first");
-        assert_eq!(tasks[1].id, t3.id, "t3 should be second (after t1)");
-        assert_eq!(tasks[2].id, t2.id, "t2 should be third");
+        assert_eq!(tasks[0].id, t3.id);
+        assert_eq!(tasks[1].id, t1.id);
+        assert_eq!(tasks[2].id, t2.id);
     }
 
     #[tokio::test]
-    async fn move_task_before_from_middle() {
+    async fn move_task_after() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
         let t1 = create(
             &mut conn,
             CreateTask {
-                title: "First",
+                title: "A",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -718,7 +774,7 @@ mod tests {
         let t2 = create(
             &mut conn,
             CreateTask {
-                title: "Second",
+                title: "B",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -730,7 +786,7 @@ mod tests {
         let t3 = create(
             &mut conn,
             CreateTask {
-                title: "Third",
+                title: "C",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -740,15 +796,14 @@ mod tests {
         .await
         .unwrap();
 
-        // Move t1 before t3 (but after t2) — should produce order: t2, t1, t3
-        move_task(&mut conn, t1.id, Some(t3.id), None)
+        move_task(&mut conn, &t1, Placement::After(&t3))
             .await
             .unwrap();
 
         let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t2.id, "t2 should be first");
-        assert_eq!(tasks[1].id, t1.id, "t1 should be second (before t3)");
-        assert_eq!(tasks[2].id, t3.id, "t3 should be third");
+        assert_eq!(tasks[0].id, t2.id);
+        assert_eq!(tasks[1].id, t3.id);
+        assert_eq!(tasks[2].id, t1.id);
     }
 
     #[tokio::test]
@@ -759,7 +814,7 @@ mod tests {
         let t1 = create(
             &mut conn,
             CreateTask {
-                title: "First",
+                title: "A",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -771,7 +826,7 @@ mod tests {
         let t2 = create(
             &mut conn,
             CreateTask {
-                title: "Second",
+                title: "B",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -783,7 +838,7 @@ mod tests {
         let t3 = create(
             &mut conn,
             CreateTask {
-                title: "Third",
+                title: "C",
                 backlog_id: bl.id,
                 state: None,
                 parent_id: None,
@@ -793,100 +848,46 @@ mod tests {
         .await
         .unwrap();
 
-        // Move t3 after t1 and before t2 — should produce order: t1, t3, t2
-        move_task(&mut conn, t3.id, Some(t2.id), Some(t1.id))
-            .await
-            .unwrap();
-
-        let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t1.id, "t1 should be first");
-        assert_eq!(tasks[1].id, t3.id, "t3 should be second");
-        assert_eq!(tasks[2].id, t2.id, "t2 should be third");
-    }
-
-    #[tokio::test]
-    async fn delete_task() {
-        let pool = test_pool().await;
-        let mut conn = pool.acquire().await.unwrap();
-        let bl = backlog::create(&mut conn, "Test").await.unwrap();
-        let task = create(
+        move_task(
             &mut conn,
-            CreateTask {
-                title: "Delete me",
-                backlog_id: bl.id,
-                state: None,
-                parent_id: None,
-                description: None,
+            &t3,
+            Placement::Between {
+                after: &t1,
+                before: &t2,
             },
         )
         .await
         .unwrap();
 
-        delete(&mut conn, task.id).await.unwrap();
-
-        let result = get_by_key_prefix(&mut conn, &task.key).await;
-        assert!(result.is_err());
-
         let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks.len(), 0);
+        assert_eq!(tasks[0].id, t1.id);
+        assert_eq!(tasks[1].id, t3.id);
+        assert_eq!(tasks[2].id, t2.id);
     }
 
     #[tokio::test]
-    async fn get_by_id_returns_task() {
+    async fn move_task_rejects_cross_state() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
-        let task = create(
+        let queued = create(
             &mut conn,
             CreateTask {
-                title: "Find by id",
+                title: "Q",
                 backlog_id: bl.id,
-                state: None,
+                state: Some(State::Queued),
                 parent_id: None,
                 description: None,
             },
         )
         .await
         .unwrap();
-
-        let found = get_by_id(&mut conn, task.id).await.unwrap();
-        assert_eq!(found.title, "Find by id");
-    }
-
-    #[tokio::test]
-    async fn get_by_key_prefix_ambiguous() {
-        let pool = test_pool().await;
-        let mut conn = pool.acquire().await.unwrap();
-        let bl = backlog::create(&mut conn, "Test").await.unwrap();
-
-        // Insert two tasks with keys that share a prefix, bypassing
-        // the random key generator
-        sqlx::query("INSERT INTO tasks (key, backlog_id, title, state, position) VALUES ('kkkkaaaa', ?, 'First', 'icebox', 'a')")
-            .bind(bl.id)
-            .execute(&mut *conn)
-            .await
-            .unwrap();
-        sqlx::query("INSERT INTO tasks (key, backlog_id, title, state, position) VALUES ('kkkkbbbb', ?, 'Second', 'icebox', 'b')")
-            .bind(bl.id)
-            .execute(&mut *conn)
-            .await
-            .unwrap();
-
-        let result = get_by_key_prefix(&mut conn, "kkkk").await;
-        assert!(result.is_err());
-    }
-
-    #[tokio::test]
-    async fn edit_title_only() {
-        let pool = test_pool().await;
-        let mut conn = pool.acquire().await.unwrap();
-        let bl = backlog::create(&mut conn, "Test").await.unwrap();
-        let task = create(
+        let done = create(
             &mut conn,
             CreateTask {
-                title: "Original",
+                title: "D",
                 backlog_id: bl.id,
-                state: None,
+                state: Some(State::Done),
                 parent_id: None,
                 description: None,
             },
@@ -894,49 +895,11 @@ mod tests {
         .await
         .unwrap();
 
-        let updated = edit(&mut conn, task.id, Some("New title"), None, None)
+        let err = move_task(&mut conn, &queued, Placement::Before(&done))
             .await
-            .unwrap();
-        assert_eq!(updated.title, "New title");
-        assert_eq!(updated.state, State::Icebox);
-    }
-
-    #[tokio::test]
-    async fn move_task_to_end() {
-        let pool = test_pool().await;
-        let mut conn = pool.acquire().await.unwrap();
-        let bl = backlog::create(&mut conn, "Test").await.unwrap();
-        let t1 = create(
-            &mut conn,
-            CreateTask {
-                title: "First",
-                backlog_id: bl.id,
-                state: None,
-                parent_id: None,
-                description: None,
-            },
-        )
-        .await
-        .unwrap();
-        let t2 = create(
-            &mut conn,
-            CreateTask {
-                title: "Second",
-                backlog_id: bl.id,
-                state: None,
-                parent_id: None,
-                description: None,
-            },
-        )
-        .await
-        .unwrap();
-
-        // Move first task to end (no before/after)
-        move_task(&mut conn, t1.id, None, None).await.unwrap();
-
-        let tasks = list(&mut conn, bl.id, None).await.unwrap();
-        assert_eq!(tasks[0].id, t2.id);
-        assert_eq!(tasks[1].id, t1.id);
+            .unwrap_err();
+        assert!(err.to_string().contains("queued"));
+        assert!(err.to_string().contains("done"));
     }
 
     #[tokio::test]
@@ -1060,7 +1023,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn state_change_same_state_does_not_reposition() {
+    async fn state_change_same_state_does_not_reorder() {
         let pool = test_pool().await;
         let mut conn = pool.acquire().await.unwrap();
         let bl = backlog::create(&mut conn, "Test").await.unwrap();
diff --git a/tests/cli.rs b/tests/cli.rs
index 491ea14..61bd1a5 100644
--- a/tests/cli.rs
+++ b/tests/cli.rs
@@ -110,65 +110,76 @@ fn full_workflow() {
     assert_eq!(detail["tags"][0]["name"], "urgent");
     assert_eq!(detail["blockers"].as_array().unwrap().len(), 1);
 
-    // Create a third task and use edit --before to reposition it
+    // Create two queued tasks and use edit --before to reposition within the same state
     let output = ranger(db_path)
         .args(["task", "create", "Third task", "--state", "queued"])
         .output()
         .unwrap();
     assert!(output.status.success());
 
+    let output = ranger(db_path)
+        .args(["task", "create", "Fourth task", "--state", "queued"])
+        .output()
+        .unwrap();
+    assert!(output.status.success());
+
     let output = ranger(db_path)
         .args(["task", "list", "--json", "--state", "queued"])
         .output()
         .unwrap();
-    let tasks_before_move: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
-    let tasks_before_move = tasks_before_move.as_array().unwrap();
-    // Third task should be after Second task (both queued, but Second was icebox — actually
-    // let's just get the keys and verify edit --before works)
-    let t3_key = tasks_before_move
+    let queued_tasks: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
+    let queued_tasks = queued_tasks.as_array().unwrap();
+    let t3_key = queued_tasks
         .iter()
         .find(|t| t["title"] == "Third task")
         .unwrap()["key"]
         .as_str()
         .unwrap()
         .to_string();
+    let t4_key = queued_tasks
+        .iter()
+        .find(|t| t["title"] == "Fourth task")
+        .unwrap()["key"]
+        .as_str()
+        .unwrap()
+        .to_string();
 
-    // Edit Third task: change title AND reposition before First task
+    // Edit Fourth task: change title AND reposition before Third task
     let output = ranger(db_path)
         .args([
             "task",
             "edit",
-            &t3_key[..4],
+            &t4_key[..4],
             "--title",
-            "Third task (edited)",
+            "Fourth task (edited)",
             "--before",
-            &t1_key[..4],
+            &t3_key[..4],
         ])
         .output()
         .unwrap();
     assert!(output.status.success());
     let stdout = String::from_utf8(output.stdout).unwrap();
-    assert!(stdout.contains("Third task (edited)"));
+    assert!(stdout.contains("Fourth task (edited)"));
 
-    // Verify ordering: Third should now be before First
+    // Verify ordering within queued: Fourth should now be before Third
     let output = ranger(db_path)
-        .args(["task", "list", "--json"])
+        .args(["task", "list", "--json", "--state", "queued"])
         .output()
         .unwrap();
-    let tasks_after_move: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
-    let tasks_after_move = tasks_after_move.as_array().unwrap();
-    let titles: Vec<&str> = tasks_after_move
+    let queued_after: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
+    let queued_after = queued_after.as_array().unwrap();
+    let titles: Vec<&str> = queued_after
         .iter()
         .map(|t| t["title"].as_str().unwrap())
         .collect();
-    let third_pos = titles
+    let fourth_pos = titles
         .iter()
-        .position(|t| *t == "Third task (edited)")
+        .position(|t| *t == "Fourth task (edited)")
         .unwrap();
-    let first_pos = titles.iter().position(|t| *t == "First task").unwrap();
+    let third_pos = titles.iter().position(|t| *t == "Third task").unwrap();
     assert!(
-        third_pos < first_pos,
-        "Third task should be before First task after edit --before, got: {:?}",
+        fourth_pos < third_pos,
+        "Fourth should be before Third after edit --before, got: {:?}",
         titles
     );
 
@@ -186,5 +197,5 @@ fn full_workflow() {
         .unwrap();
     assert!(output.status.success());
     let tasks: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
-    assert_eq!(tasks.as_array().unwrap().len(), 2);
+    assert_eq!(tasks.as_array().unwrap().len(), 3);
 }