diff --git a/CHANGELOG.md b/CHANGELOG.md index 422dfffe9..8bf4d73e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A deployment directory pasted with a stray space goes where it says + +The desktop setup screen asks where OpenBot should live, enables Start once that box is not blank +after trimming, and then sent the untrimmed string — the same trap the API URL, the gateway URL, the +intelligence key and the model key were taken out of, and this is the one of the five that is a +place on disk rather than a credential. A path copied with the space the selection picked up, or +with the newline a copied line carries, was used whole. A trailing space made a second directory +beside the one everything else means: the tray's Stop and the next launch both ask for the default +path, which has no space in it, so a person was left with a deployment nothing on screen could +reach. A leading space was worse, because a path starting with a space does not start with a +separator — it stopped being absolute, and the deployment was laid out relative to wherever the +window happened to be running from. The path is trimmed at both ends now. Spaces inside it are part +of a directory's name and are left alone. ### The desktop app notices a busy port whichever loopback holds it The shell refuses to start when something already holds port 3001 or 3010, because otherwise the diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index f555e2cff..155de41d9 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -120,7 +120,7 @@ async fn start_stack( api_key: String, openai_api_key: String, ) -> Result<(), String> { - let root = PathBuf::from(root); + let root = stack::root_from(&root); // The installer does not carry the deployment; it fetches one. Skipped when the recorded // version already matches, so a restart is not a download. @@ -285,7 +285,7 @@ async fn start_stack( /// of everything their Bot had logged into. #[tauri::command] fn stop_stack(app: tauri::AppHandle, root: String) -> Result<(), String> { - stop_everything(&app, &PathBuf::from(&root)) + stop_everything(&app, &stack::root_from(&root)) } /// Take the whole stack down: the host processes, anything left over, and the containers. @@ -395,7 +395,7 @@ fn show_setup(app: tauri::AppHandle) -> Result<(), String> { /// an answer on the port says one is running now. #[tauri::command] fn already_running(root: String) -> bool { - let root = PathBuf::from(&root); + let root = stack::root_from(&root); if deployment::installed(&root).is_none() { return false; } diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index b5a5041b2..e6ce611fa 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -580,6 +580,24 @@ pub fn default_root() -> PathBuf { dirs_home().join("OpenBot") } +/// The deployment directory somebody typed, as a path. +/// +/// Trimmed, the way the four settings entered beside it on the same screen already are. That screen +/// enables Start on `root.trim() !== ""` and then sends the untrimmed string, so a path pasted with +/// the space the selection picked up, or with the newline a copied line carries, arrives here whole +/// -- and this is the one of the five values that is not a credential but a place on disk. +/// +/// A trailing space makes a second directory beside the one everything else means: the tray's Stop +/// and the next launch both ask `default_root`, which has no space in it, so a person is left with +/// a deployment nothing on screen can reach. A leading one is worse, because a path that begins +/// with a space does not begin with a separator: it stops being absolute, and the whole deployment +/// is laid out relative to wherever the window happens to be running from. +/// +/// Only the ends. A space inside a path is part of a directory's name and stays where it is. +pub fn root_from(typed: &str) -> PathBuf { + PathBuf::from(typed.trim()) +} + fn dirs_home() -> PathBuf { std::env::var("HOME") .or_else(|_| std::env::var("USERPROFILE")) @@ -651,6 +669,50 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + #[test] + fn a_deployment_directory_pasted_with_a_stray_space_is_the_one_it_names() { + // The fifth value on the setup screen that trimming missed. The screen enables Start on + // `root.trim() !== ""` and then sends the untrimmed string, which is exactly what the API + // URL, the gateway URL, the intelligence key and the model key were rescued from. + // + // A trailing space is a second directory beside the one everything else means: the tray's + // Stop and the next launch both ask `default_root`, which has no space in it. A leading one + // is worse, because a path that begins with a space does not begin with a separator: the + // whole deployment stops being absolute and lands under wherever the window is running + // from. + assert_eq!( + root_from(" /home/me/OpenBot "), + PathBuf::from("/home/me/OpenBot") + ); + assert_eq!( + root_from("/home/me/OpenBot\n"), + PathBuf::from("/home/me/OpenBot") + ); + assert!( + root_from(" /home/me/OpenBot").has_root(), + "a leading space turned an absolute path into a relative one" + ); + } + + #[test] + fn a_space_inside_the_path_is_part_of_the_path() { + // Only the ends. "Documents and Settings" is a directory, and a person whose home has a + // space in it must still be able to say where OpenBot lives. + assert_eq!( + root_from("/home/me/My Files/OpenBot"), + PathBuf::from("/home/me/My Files/OpenBot") + ); + assert_eq!( + root_from(r"C:\Users\me\Open Bot"), + PathBuf::from(r"C:\Users\me\Open Bot") + ); + // And an ordinary path is handed back exactly as it was. + assert_eq!( + root_from("/home/me/OpenBot"), + PathBuf::from("/home/me/OpenBot") + ); + } + #[test] fn a_port_nobody_holds_is_not_reported_as_taken() { // 0 is never listening; this asserts the check does not invent a problem.