Refuse a start when the port is held on the other loopback

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### 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
+readiness check that follows is answered by a server it never started: everything reads green and
+none of it is yours. That readiness check asks both loopback addresses on purpose, since a process
+binds whichever one its runtime resolved `localhost` to — Node picks `::1`, Bun picks `127.0.0.1` —
+so an answer at either counts. The refusal in front of it asked only `127.0.0.1`, which meant a port
+held on `::1` alone was reported free and the start went ahead into it. Both addresses are asked
+now, so the two agree on what "in use" means and the person is told which port is taken and what
+OpenBot wanted it for.
 ### A request for a secret no longer follows a Bot into tomorrow's conversations
 
 An unanswered ask to take the wheel stops being shown after ten minutes, because control belongs to a

desktop/src-tauri/src/stack.rs

@@ -366,12 +366,7 @@ pub fn services_that_exited(engine: &Address, root: &Path) -> Vec<(String, Strin
 /// outside.
 pub fn port_already_taken(ports: &[(&'static str, u16)]) -> Option<String> {
     for (name, port) in ports {
-        if std::net::TcpStream::connect_timeout(
-            &std::net::SocketAddr::from(([127, 0, 0, 1], *port)),
-            std::time::Duration::from_millis(300),
-        )
-        .is_ok()
-        {
+        if held_on_a_loopback(*port) {
             return Some(format!(
                 "Something is already listening on port {port}, which OpenBot uses for the {name}. \
                  Stop it, or change the port, and start again."
@@ -381,6 +376,28 @@ pub fn port_already_taken(ports: &[(&'static str, u16)]) -> Option<String> {
     None
 }
 
+/// Whether anything is listening on `port`, at either address a loopback service can be bound to.
+///
+/// Both, for the reason `LOOPBACKS` below already records: a process binds whichever loopback its
+/// runtime resolved `localhost` to, and binding one and not the other is normal rather than broken.
+/// Asking only `127.0.0.1` therefore called a port free that the readiness check would then accept a
+/// stranger's answer on, which is the exact outcome the check above exists to prevent. A refused
+/// connection comes back at once on both addresses, so the second question costs nothing on a port
+/// nobody holds.
+fn held_on_a_loopback(port: u16) -> bool {
+    const LOOPBACK_ADDRESSES: [std::net::IpAddr; 2] = [
+        std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST),
+        std::net::IpAddr::V6(std::net::Ipv6Addr::LOCALHOST),
+    ];
+    LOOPBACK_ADDRESSES.iter().any(|address| {
+        std::net::TcpStream::connect_timeout(
+            &std::net::SocketAddr::new(*address, port),
+            std::time::Duration::from_millis(300),
+        )
+        .is_ok()
+    })
+}
+
 /// Wait until the API answers, or say why it never did.
 ///
 /// Spawning is not starting. Each of these three can exit in the first second for a reason that has
@@ -654,6 +671,23 @@ mod tests {
         );
     }
 
+    #[test]
+    fn a_port_held_on_the_other_loopback_is_still_held() {
+        // The readiness check accepts an answer at either loopback, for the reason `LOOPBACKS`
+        // gives: a process binds whichever one its runtime resolved `localhost` to. A guard that
+        // asks only 127.0.0.1 therefore calls a port free that `answering_at` would then accept a
+        // stranger's answer on, which is the one outcome this check exists to prevent.
+        let listener = std::net::TcpListener::bind("[::1]:0").unwrap();
+        let port = listener.local_addr().unwrap().port();
+
+        let problem = port_already_taken(&[("app", port)]).expect("a held port is a problem");
+        assert!(problem.contains(&port.to_string()), "{problem}");
+        assert!(
+            problem.contains("app"),
+            "must say what it is for: {problem}"
+        );
+    }
+
     #[test]
     fn migrate_is_not_raised_as_a_service() {
         // Raised alongside the others it exits immediately, and Compose reports a service that will