Merge pull request #2811 from ESHAYAT102/feat/server-delete-confirmation

🤖 Agent: read the diff at 84390f6dc: Delete server in its settings: refused unless the server is in the list, not installed as a service, and its supervisor lock (`.horizon/lock`) can be taken; the folder goes to the system trash, never `rm`. Unit tests cover a live lock, a non-server folder and a failing trash. verify and launcher green; no harness reaches it. Queued for the merger, which waits for this head's parity run.

launcher/src/servers.rs

@@ -68,6 +68,48 @@ pub fn forget(dir: &Path) -> Result<(), String> {
     save_list(&all)
 }
 
+/// Removes a registered, stopped server, retaining its files in the system trash.
+pub fn delete(dir: &Path) -> Result<(), String> {
+    if !list().iter().any(|entry| entry.dir == dir) {
+        return Err("This server is no longer in the launcher’s list.".into());
+    }
+    if service_installed(dir) {
+        return Err("Turn off running in the background before deleting this server.".into());
+    }
+    trash_stopped_server(dir, crate::desktop_seams::to_trash)?;
+    forget(dir).map_err(|e| {
+        format!("The server was moved to the trash, but its list could not be saved: {e}")
+    })
+}
+
+fn trash_stopped_server(
+    dir: &Path,
+    trash: impl FnOnce(&Path) -> Result<(), String>,
+) -> Result<(), String> {
+    if dir
+        .symlink_metadata()
+        .map_err(|e| e.to_string())?
+        .file_type()
+        .is_symlink()
+        || !dir.join("server.toml").is_file()
+    {
+        return Err("This is not a server folder.".into());
+    }
+    let state = dir.join(".horizon");
+    fs::create_dir_all(&state).map_err(|e| e.to_string())?;
+    // The supervisor holds this same lock throughout starting, running and stopping.
+    // Hold it through the move so a stale UI/API status cannot delete a live server.
+    let lock = fs::File::options()
+        .create(true)
+        .truncate(false)
+        .write(true)
+        .open(state.join("lock"))
+        .map_err(|e| e.to_string())?;
+    lock.try_lock()
+        .map_err(|_| "Stop the server before deleting it.".to_string())?;
+    trash(dir).map_err(|e| format!("Could not move the server to the trash: {e}"))
+}
+
 /// A folder name for a server name: lower case letters, digits and dashes.
 pub fn slug(name: &str) -> String {
     let mut out = String::new();

launcher/src/servers/tests.rs

@@ -226,3 +226,54 @@ fn the_api_sends_the_token() {
     // Nothing answers: stopped.
     assert_eq!(status(Path::new("/nonexistent")).phase, "stopped");
 }
+
+#[test]
+fn deleting_a_stopped_server_moves_its_world_and_backups_to_trash() {
+    let root = scratch("delete-stopped");
+    let dir = root.join("server");
+    let trash = root.join("trash");
+    fs::create_dir_all(dir.join("world/region")).unwrap();
+    fs::create_dir_all(dir.join("backups")).unwrap();
+    fs::write(dir.join("server.toml"), "[server]\n").unwrap();
+    fs::write(dir.join("world/region/region.mca"), b"world").unwrap();
+    fs::write(dir.join("backups/backup.zip"), b"backup").unwrap();
+    trash_stopped_server(&dir, |path| crate::desktop_seams::to_folder(path, &trash)).unwrap();
+    assert!(!dir.exists());
+    assert_eq!(
+        fs::read(trash.join("server/world/region/region.mca")).unwrap(),
+        b"world"
+    );
+    assert_eq!(
+        fs::read(trash.join("server/backups/backup.zip")).unwrap(),
+        b"backup"
+    );
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[test]
+fn deleting_refuses_a_live_supervisor_even_when_its_api_is_unavailable() {
+    let root = scratch("delete-running");
+    fs::create_dir_all(root.join(".horizon")).unwrap();
+    fs::write(root.join("server.toml"), "[server]\n").unwrap();
+    let lock = fs::File::create(root.join(".horizon/lock")).unwrap();
+    lock.try_lock().unwrap();
+    let result = trash_stopped_server(&root, |_| panic!("must not move a running server"));
+    assert!(result.unwrap_err().contains("Stop the server"));
+    assert!(root.join("server.toml").exists());
+    drop(lock);
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[test]
+fn deleting_refuses_an_unrecognized_folder_and_keeps_files_on_trash_failure() {
+    let root = scratch("delete-invalid");
+    assert!(trash_stopped_server(&root, |_| panic!("not a server")).is_err());
+    fs::write(root.join("server.toml"), "[server]\n").unwrap();
+    assert!(
+        trash_stopped_server(&root, |_| Err("trash unavailable".into()))
+            .unwrap_err()
+            .contains("trash unavailable")
+    );
+    assert!(root.join("server.toml").exists());
+    fs::remove_dir_all(root).unwrap();
+}

launcher/src/servers_page.rs

@@ -46,10 +46,11 @@ pub(super) enum Field {
     LaunchEnv,
     /// Settings' new offline name (`switch_page.rs`).
     OfflineName,
+    DeleteServer,
 }
 
 impl Field {
-    const ALL: [Field; 18] = [
+    const ALL: [Field; 19] = [
         Field::Name,
         Field::Seed,
         Field::Players,
@@ -68,6 +69,7 @@ impl Field {
         Field::Wrapper,
         Field::LaunchEnv,
         Field::OfflineName,
+        Field::DeleteServer,
     ];
 }
 
@@ -110,6 +112,10 @@ pub(super) enum Event {
     Line(String),
     Done(Result<String, Problem>),
     Created(Result<Entry, Problem>),
+    Deleted {
+        dir: PathBuf,
+        result: Result<(), String>,
+    },
     Details {
         dir: PathBuf,
         config: Option<Value>,
@@ -177,6 +183,7 @@ pub(super) struct Servers {
     restart_needed: BTreeSet<String>,
     backups: Vec<Backup>,
     confirm_restore: Option<String>,
+    confirm_delete: bool,
     service: Option<bool>,
     update: Option<Value>,
     /// `update --check` is under way.
@@ -225,6 +232,7 @@ impl Servers {
             restart_needed: BTreeSet::new(),
             backups: Vec::new(),
             confirm_restore: None,
+            confirm_delete: false,
             service: None,
             update: None,
             checking: false,
@@ -492,6 +500,8 @@ impl Launcher {
         s.problem = None;
         s.note = None;
         s.confirm_restore = None;
+        s.confirm_delete = false;
+        s.texts.remove(&Field::DeleteServer);
         s.restart_needed.clear();
         s.texts.remove(&Field::Motd);
         s.texts.remove(&Field::Port);
@@ -640,6 +650,24 @@ impl Launcher {
                     s.service = Some(service);
                 }
             }
+            Event::Deleted { dir, result } => {
+                s.busy = None;
+                s.confirm_delete = false;
+                s.list = servers::list();
+                s.statuses.remove(&dir);
+                STARTED_HERE
+                    .lock()
+                    .unwrap()
+                    .retain(|started| started != &dir);
+                if !s.list.iter().any(|entry| entry.dir == dir) {
+                    s.open = None;
+                    self.side_servers = s.list.clone();
+                }
+                match result {
+                    Ok(()) => s.note = Some("Server moved to the trash.".into()),
+                    Err(e) => s.problem = Some(Problem::plain(e)),
+                }
+            }
             Event::Applied(result) => {
                 s.applying = false;
                 match result {
@@ -989,6 +1017,7 @@ impl Launcher {
                     }
                     Key::Enter => this.field_enter(f),
                     Key::Escape => {
+                        this.servers.confirm_delete = false;
                         if this.servers.create.is_some() && !this.servers.creating {
                             this.servers.create = None;
                         }
@@ -1013,6 +1042,10 @@ impl Launcher {
 
     /// Escape closes the Create dialog. Whether it did.
     pub(super) fn servers_escape(&mut self) -> bool {
+        if self.page == Page::Servers && self.servers.confirm_delete {
+            self.servers.confirm_delete = false;
+            return true;
+        }
         if self.page != Page::Servers || self.servers.create.is_none() || self.servers.creating {
             return false;
         }
@@ -1044,7 +1077,10 @@ impl Launcher {
             Field::Port => self.save_port(),
             Field::Token => self.save_token(),
             Field::Email => self.invite_friend(),
-            Field::InstanceName | Field::InstanceGroup | Field::ConsoleSearch => {}
+            Field::InstanceName
+            | Field::InstanceGroup
+            | Field::ConsoleSearch
+            | Field::DeleteServer => {}
             Field::PreLaunch | Field::Wrapper | Field::LaunchEnv => self.save_launch(),
             Field::OfflineName => self.offline_name_enter(),
             Field::ServerName | Field::ServerAddress => self.add_server(),
@@ -2304,7 +2340,8 @@ impl Launcher {
                 },
                 accounts_switch,
             ))
-            .child(self.background_row(cx));
+            .child(self.background_row(cx))
+            .child(self.delete_server_section(status, cx));
         div()
             .flex()
             .flex_col()
@@ -2316,6 +2353,119 @@ impl Launcher {
             .child(form)
     }
 
+    fn delete_server_section(&mut self, status: &Status, cx: &mut Context<Self>) -> Div {
+        let t = self.theme;
+        let name = self
+            .servers
+            .list
+            .iter()
+            .find(|entry| Some(&entry.dir) == self.servers.open.as_ref())
+            .map(|entry| entry.name.clone())
+            .unwrap_or_default();
+        let ready = !status.running()
+            && self.servers.busy.is_none()
+            && !self.servers.applying
+            && self.servers.queued.is_empty()
+            && self.servers.service == Some(false);
+        let hint = if status.running() {
+            Some("Stop the server before deleting it.")
+        } else if self.servers.service == Some(true) {
+            Some("Turn off running in the background before deleting it.")
+        } else if !ready {
+            Some("Wait for the current operation to finish.")
+        } else {
+            None
+        };
+        let mut controls = div()
+            .flex()
+            .flex_col()
+            .items_end()
+            .flex_none()
+            .w(px(260.))
+            .gap(px(12.));
+        if self.servers.confirm_delete {
+            let matches = self.servers.value(Field::DeleteServer) == name && !name.is_empty();
+            let field = self.field(Field::DeleteServer, "Server name", 225., cx);
+            let confirm = button(
+                "server-delete-confirm",
+                "Move to trash",
+                Kind::Secondary,
+                &t,
+            )
+            .text_color(t.danger);
+            let confirm = if ready && matches {
+                confirm.on_click(cx.listener(|this, _: &ClickEvent, _, cx| {
+                    if let Some(dir) = this.servers.open.clone() {
+                        this.servers.busy = Some("Moving server to trash".into());
+                        let tx = this.servers.tx.clone();
+                        std::thread::spawn(move || {
+                            let result = servers::delete(&dir);
+                            let _ = tx.send(Event::Deleted { dir, result });
+                        });
+                    }
+                    cx.notify();
+                }))
+            } else {
+                disabled(confirm)
+            };
+            controls = controls
+                .child(field)
+                .child(text(theme::CAPTION).text_color(t.muted).child(
+                    gpui::StyledText::new(format!("Type {name} to confirm.")).with_highlights([(
+                        5..5 + name.len(),
+                        gpui::HighlightStyle {
+                            font_weight: Some(FontWeight::BOLD),
+                            ..Default::default()
+                        },
+                    )]),
+                ))
+                .child(
+                    div()
+                        .flex()
+                        .gap(px(8.))
+                        .child(
+                            button("server-delete-cancel", "Cancel", Kind::Ghost, &t).on_click(
+                                cx.listener(|this, _: &ClickEvent, _, cx| {
+                                    this.servers.confirm_delete = false;
+                                    cx.notify();
+                                }),
+                            ),
+                        )
+                        .child(confirm),
+                );
+        } else {
+            let delete =
+                button("server-delete", "Delete server…", Kind::Secondary, &t).text_color(t.danger);
+            let delete = if ready {
+                delete.on_click(cx.listener(|this, _: &ClickEvent, _, cx| {
+                    this.servers.confirm_delete = true;
+                    this.servers.text(Field::DeleteServer).clear();
+                    this.focus_handle(Field::DeleteServer, cx);
+                    this.servers.focus_soon = Some(Field::DeleteServer);
+                    cx.notify();
+                }))
+            } else {
+                disabled(delete)
+            };
+            controls = controls.child(delete);
+        }
+        if let Some(hint) = hint {
+            controls = controls.child(
+                text(theme::CAPTION)
+                    .text_color(t.muted)
+                    .w(px(260.))
+                    .text_align(gpui::TextAlign::Right)
+                    .child(hint),
+            );
+        }
+        self.setting_row(
+            "Delete server",
+            "Remove this server from Horizon and move its worlds, settings and backups to the trash.".into(),
+            controls.into_any_element(),
+        )
+        .items_start()
+    }
+
     fn background_row(&mut self, cx: &mut Context<Self>) -> Div {
         let t = self.theme;
         let on = self.servers.service.unwrap_or(false);