refactor(desktop): setup, install and probe code: shared helpers, honest docs

No behavior change.

- ProbeOutcome::version replaces two copies of the same match in setup.rs,
  and classify_response reads the status JSON with serde_json's indexing
  (same answer for every body, a missing key is Null).
- local_server::is_env_name is the one test for a variable name, used by
  the login shell's environment and by ~/.codeman/.env.
- setup.rs has basename() for its two command-line scans; the install
  scan no longer formats the script prefix once per word.
- agents.rs: refused() names the statuses that end the polling, and
  answer() takes the body it already has instead of a closure.
- Small simplifications in gui_install.rs (Run::matches, follow, the
  download error text) and a let-else in local_server::launch.
- Docs that no longer matched the code: probe.rs said the probe never runs
  in a loop, but the This Mac tab re-asks while it waits for an install
  (every 3 s) or for a starting server (every second); local_server.rs
  covered only Start; FIND_TIMEOUT and exec named one caller of several.

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

packages/desktop/src/agents.rs

@@ -115,19 +115,22 @@ fn session_cookie(header: &str) -> Option<&str> {
     (name == COOKIE_NAME && !value.is_empty()).then_some(pair)
 }
 
+/// A refused login: Codeman counts each against the login rate limit, so the
+/// polling ends.
+fn refused(status: u16) -> bool {
+    matches!(status, 401 | 403 | 429)
+}
+
 /// What a status code says about asking again.
-fn answer(status: u16, body: impl FnOnce() -> Option<String>) -> Poll {
+fn answer(status: u16, body: Option<&str>) -> Poll {
     match status {
-        200 => body()
-            .as_deref()
-            .and_then(count_working)
-            .map_or(Poll::Failed, |working| {
-                Poll::Seen(Activity {
-                    working: working > 0,
-                    ..Activity::default()
-                })
-            }),
-        401 | 403 | 429 => Poll::Refused,
+        200 => body.and_then(count_working).map_or(Poll::Failed, |working| {
+            Poll::Seen(Activity {
+                working: working > 0,
+                ..Activity::default()
+            })
+        }),
+        s if refused(s) => Poll::Refused,
         _ => Poll::Failed,
     }
 }
@@ -192,7 +195,7 @@ impl Poller {
         let Some((status, body)) = self.ask(&url) else {
             return Poll::Failed;
         };
-        let mut activity = match answer(status, || body) {
+        let mut activity = match answer(status, body.as_deref()) {
             Poll::Seen(activity) => activity,
             other => return other,
         };
@@ -203,7 +206,7 @@ impl Poller {
                         self.alerts = alerts;
                     }
                 }
-                Some((401 | 403 | 429, _)) => return Poll::Refused,
+                Some((s, _)) if refused(s) => return Poll::Refused,
                 // A server from before the list has no alerts to report.
                 Some((404, _)) => self.alerts = (false, false),
                 _ => {}
@@ -264,17 +267,17 @@ mod tests {
     fn a_body_that_is_not_the_list_is_no_answer() {
         assert_eq!(count_working("<html>"), None);
         assert_eq!(count_working(r#"{"success":false,"error":"nope"}"#), None);
-        assert_eq!(answer(200, || Some("<html>".into())), Poll::Failed);
-        assert_eq!(answer(200, || None), Poll::Failed);
+        assert_eq!(answer(200, Some("<html>")), Poll::Failed);
+        assert_eq!(answer(200, None), Poll::Failed);
     }
 
     #[test]
     fn only_a_login_refusal_ends_the_polling() {
         for status in [401, 403, 429] {
-            assert_eq!(answer(status, || None), Poll::Refused, "{status}");
+            assert_eq!(answer(status, None), Poll::Refused, "{status}");
         }
         for status in [301, 404, 500, 502, 503] {
-            assert_eq!(answer(status, || None), Poll::Failed, "{status}");
+            assert_eq!(answer(status, None), Poll::Failed, "{status}");
         }
     }
 

packages/desktop/src/gui_install.rs

@@ -100,8 +100,8 @@ impl Run {
     /// not one: it has no `--progress`.
     pub fn matches(self, command: &str) -> bool {
         let suffix = format!("/{}/{SCRIPT_FILE}", self.folder());
-        let mut words = command.split_whitespace();
-        words.any(|w| w.ends_with(&suffix)) && command.split_whitespace().any(|w| w == "--progress")
+        command.split_whitespace().any(|w| w.ends_with(&suffix))
+            && command.split_whitespace().any(|w| w == "--progress")
     }
 
     /// A process of this run, if it is running.
@@ -269,11 +269,12 @@ fn download_installer(script: &Path) -> Result<(), String> {
     if out.code == Some(0) && script.is_file() {
         return Ok(());
     }
+    const FAILED: &str = "Could not download the installer. Check the internet connection, then try again.";
     let said = out.stderr.trim();
     Err(if said.is_empty() {
-        "Could not download the installer. Check the internet connection, then try again.".to_string()
+        FAILED.to_string()
     } else {
-        format!("Could not download the installer. Check the internet connection, then try again.\n{said}")
+        format!("{FAILED}\n{said}")
     })
 }
 
@@ -571,9 +572,8 @@ pub fn follow(run: Run, stale: impl Fn() -> bool, send: impl Fn(Value)) {
             return;
         }
         let running = run.process().is_some();
-        let log = progress.feed(&read_more(&mut file, &path, &mut offset));
+        let mut log = progress.feed(&read_more(&mut file, &path, &mut offset));
         if !running || progress.done {
-            let mut log = log;
             log.extend(progress.finish());
             let cancelled = CANCELLED.load(Ordering::SeqCst);
             if progress.done {

packages/desktop/src/local_server.rs

@@ -1,4 +1,7 @@
 //! "Start Codeman on this Mac": `codeman web -d` through the user's login shell.
+//! The same login-shell machinery answers whether Codeman is installed
+//! (`installed`) and hands its environment to the graphical install
+//! (`login_environment`); `exec_bounded` is the crate's bounded process runner.
 //!
 //! A double-clicked app gets launchd's bare PATH (`/usr/bin:/bin:...`), which
 //! finds neither `codeman` nor `node`, and the server inherits whatever
@@ -143,7 +146,8 @@ fn pump<R: Read + Send + 'static>(pipe: Option<R>, stream: usize, tx: mpsc::Send
     });
 }
 
-/// `exec_bounded` for a start.
+/// `exec_bounded` with the start's timeout message. The installed check and
+/// `login_environment` run through it too, and drop its error.
 fn exec(cmd: Command, deadline: Instant) -> Result<Output, String> {
     exec_bounded(
         cmd,
@@ -248,13 +252,16 @@ fn parse_env(output: &str) -> Option<Vec<(String, String)>> {
     None
 }
 
+/// A portable variable name: a letter or `_`, then letters, digits or `_`.
+pub fn is_env_name(name: &str) -> bool {
+    let mut chars = name.chars();
+    chars.next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_')
+        && chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
+}
+
 fn env_entry(line: &str) -> Option<(&str, &str)> {
     let (name, value) = line.split_once('=')?;
-    let mut chars = name.chars();
-    let first = chars.next()?;
-    let portable =
-        (first.is_ascii_alphabetic() || first == '_') && chars.all(|c| c.is_ascii_alphanumeric() || c == '_');
-    portable.then_some((name, value))
+    is_env_name(name).then_some((name, value))
 }
 
 /// Ask `shell` for its environment with `flags`. The outer error ends the
@@ -339,16 +346,13 @@ fn run_in_shell(
 /// 127 means "no codeman here" and moves on to the next way of asking; the
 /// interactive profile is asked only when `interactive` allows it.
 fn launch(script: &str, shell: &Path, args: &[String], deadline: Instant, interactive: bool) -> Result<Run, String> {
-    let attempts = match flavor(shell) {
-        Flavor::Posix => {
-            let result = run_in_shell(script, shell, false, args, deadline)?;
-            if result.code != Some(127) || !interactive {
-                return Ok(result);
-            }
-            // Not on the login PATH; try the interactive profile too.
-            return run_in_shell(script, shell, true, args, deadline);
+    let Flavor::Capture(attempts) = flavor(shell) else {
+        let result = run_in_shell(script, shell, false, args, deadline)?;
+        if result.code != Some(127) || !interactive {
+            return Ok(result);
         }
-        Flavor::Capture(attempts) => attempts,
+        // Not on the login PATH; try the interactive profile too.
+        return run_in_shell(script, shell, true, args, deadline);
     };
     let mut last_run = None;
     let mut capture_error = None;
@@ -386,8 +390,8 @@ const FIND_SCRIPT: &str = r#"if command -v codeman >/dev/null 2>&1; then exit 0;
 if [ -f "$HOME/.codeman/app/dist/index.js" ] && command -v node >/dev/null 2>&1; then exit 0; fi
 exit 127"#;
 
-/// How long the This Mac tab waits for the login shell to answer whether
-/// Codeman is installed.
+/// How long one look at the login shell may take: whether Codeman is
+/// installed (the This Mac tab), or its environment for the graphical install.
 const FIND_TIMEOUT: Duration = Duration::from_secs(15);
 
 /// `installed` with the shell given, so tests need not touch `$SHELL`.
@@ -453,7 +457,7 @@ mod tests {
             })
             .output
         };
-        // stdout that ended without a newline used to be glued to stderr.
+        // stdout that ends without a newline: stderr still starts a line of its own.
         assert_eq!(
             said("Starting Codeman", "Error: port 3000 is in use\n"),
             "Starting Codeman\nError: port 3000 is in use"

packages/desktop/src/probe.rs

@@ -2,8 +2,10 @@
 //!
 //! One `GET <server>/api/status`, five seconds at most, run off the UI thread.
 //! The answer is classified into something the connect page can explain. It is
-//! asked once per user action, never in a loop: Codeman counts a request
-//! without valid credentials against the login rate limit (10 per IP).
+//! asked on a user action, or by one of the This Mac tab's short waits (for an
+//! install to bring Codeman up, for a server that is starting) that end at the
+//! first answer, never as a standing poll: Codeman counts a request without
+//! valid credentials against the login rate limit (10 per IP).
 
 use serde::Serialize;
 use std::time::Duration;
@@ -82,6 +84,16 @@ pub enum ProbeOutcome {
     },
 }
 
+impl ProbeOutcome {
+    /// The server's version, when it answered as Codeman.
+    pub fn version(&self) -> Option<String> {
+        match self {
+            ProbeOutcome::Ok { version } => Some(version.clone()),
+            _ => None,
+        }
+    }
+}
+
 /// Classify an HTTP answer to `<server>/api/status`. `request` is the URL that
 /// was asked, `location` and `retry_after` are those response headers.
 pub fn classify_response(
@@ -94,12 +106,10 @@ pub fn classify_response(
 ) -> ProbeOutcome {
     match status {
         200 => {
-            let parsed: Option<serde_json::Value> = serde_json::from_str(body).ok();
-            let version = parsed.as_ref().and_then(|v| {
-                (v.get("success")? == &serde_json::Value::Bool(true))
-                    .then(|| v.get("data")?.get("version")?.as_str().map(str::to_string))
-                    .flatten()
-            });
+            let version = serde_json::from_str::<serde_json::Value>(body)
+                .ok()
+                .filter(|v| v["success"] == true)
+                .and_then(|v| v["data"]["version"].as_str().map(str::to_string));
             match version {
                 Some(version) => ProbeOutcome::Ok { version },
                 None => ProbeOutcome::NotCodeman { status },
@@ -742,8 +752,8 @@ mod tests {
     }
 
     /// A Codeman that sends its status headers and then stalls (a blocked
-    /// event loop, a lossy link) did not answer in time. It is not "something
-    /// else on this port", which is what an empty body read as before.
+    /// event loop, a lossy link) did not answer in time: a Timeout, not
+    /// NotCodeman ("something else on this port").
     #[test]
     fn a_body_that_stalls_is_a_timeout() {
         use std::io::{Read, Write};

packages/desktop/src/setup.rs

@@ -182,10 +182,7 @@ pub fn parse_env_file(text: &str) -> HashMap<String, String> {
         let Some((name, value)) = line.split_once('=') else {
             continue;
         };
-        let mut chars = name.chars();
-        let portable = chars.next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_')
-            && chars.all(|c| c.is_ascii_alphanumeric() || c == '_');
-        if !portable {
+        if !local_server::is_env_name(name) {
             continue;
         }
         let mut value = value.trim();
@@ -335,12 +332,17 @@ pub fn is_codeman_server(command: &str) -> bool {
     server_args(command).is_some()
 }
 
+/// The last path component of a command-line word.
+fn basename(word: &str) -> &str {
+    word.rsplit('/').next().unwrap_or(word)
+}
+
 /// What follows `web` on a Codeman server's command line, or None for any
 /// other command.
 fn server_args(command: &str) -> Option<Vec<&str>> {
     let words: Vec<&str> = command.split_whitespace().collect();
     let is_entry = |w: &str| {
-        let name = w.rsplit('/').next().unwrap_or(w);
+        let name = basename(w);
         w.ends_with("dist/index.js") || w.ends_with("src/index.ts") || name == "codeman" || name == "aicodeman"
     };
     // Any entry word that `web` follows: an earlier word may only look like
@@ -359,14 +361,14 @@ fn server_args(command: &str) -> Option<Vec<&str>> {
 /// the port it asked for once the installer runs.
 pub fn install_in_progress(command: &str) -> Option<Option<u16>> {
     let words: Vec<&str> = command.split_whitespace().collect();
-    let name = |w: &&str| w.rsplit('/').next().unwrap_or(w).to_string();
+    let script = format!("{LOCAL_SCRIPT_NAME}-");
     if words.iter().any(|w| {
-        let n = name(w);
-        n.starts_with(&format!("{LOCAL_SCRIPT_NAME}-")) && n.ends_with(".command")
+        let n = basename(w);
+        n.starts_with(&script) && n.ends_with(".command")
     }) {
         return Some(None);
     }
-    if words.iter().any(|w| name(w).starts_with("codeman-install.")) || gui_install::Run::Codeman.matches(command) {
+    if words.iter().any(|w| basename(w).starts_with("codeman-install.")) || gui_install::Run::Codeman.matches(command) {
         let port = words
             .iter()
             .position(|w| *w == "--port")
@@ -539,10 +541,7 @@ fn found_server(url: &Url, answer: &ProbeOutcome, default: &Url) -> FoundServer
     };
     FoundServer {
         url: url.to_string(),
-        version: match answer {
-            ProbeOutcome::Ok { version } => Some(version.clone()),
-            _ => None,
-        },
+        version: answer.version(),
         state,
         is_default: url == default,
     }
@@ -702,11 +701,10 @@ pub fn inspect(depth: Depth) -> LocalStatus {
     };
     let answer = probe::probe(&default, auth.as_deref());
 
-    // Every address one of this user's Codeman servers listens at. The one at
-    // the default address has just answered; the others are asked now, once
-    // each. With nothing at the default address, the first of them that
-    // answers is what the tab opens, as it always was, so each gets the
-    // probe's usual time; otherwise they are only listed, and get less.
+    // The other addresses this user's Codeman servers listen at, asked once
+    // each. With nothing at the default address, the first that answers is
+    // what the tab opens (`shown`), so each gets the probe's usual time;
+    // otherwise they are only listed, and get less.
     let patience = if answer == ProbeOutcome::Refused {
         probe::TIMEOUT
     } else {
@@ -742,10 +740,7 @@ pub fn inspect(depth: Depth) -> LocalStatus {
             Depth::Interactive => local_server::installed(true),
         };
     let state = local_state(&outcome, !servers.is_empty(), !listening.is_empty(), installed);
-    let version = match &outcome {
-        ProbeOutcome::Ok { version } => Some(version.clone()),
-        _ => None,
-    };
+    let version = outcome.version();
     LocalStatus {
         url: url.to_string(),
         state,