diff --git a/server/src/config.rs b/server/src/config.rs index 4544cbd..6870817 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -212,34 +212,36 @@ impl Config { self.setups.iter().find(|setup| setup.name == name) } - /// What a fresh install starts with: this machine, offering the echo - /// driver to prove the pipe and the Claude CLI to be useful. + /// This machine, offering whatever was found on it. /// - /// Echo runs in-process, so it belongs to the setup with no ssh -- - /// there is nothing for a transport to wrap, and offering it on a - /// remote machine would be a choice that changes nothing. - pub fn seed() -> SetupConfig { + /// The providers are passed in rather than written here because they + /// have to be *discovered*: a hardcoded list is a claim about what is + /// installed, and this one was wrong -- every fresh install asserted a + /// `claude-cli` provider whether or not `claude` existed, which on a + /// machine without it is a spawn option that cannot work and a + /// statement the server never checked. Providers are discovered by + /// asking the machine, here exactly as for any other setup. + pub fn seed(providers: Vec) -> SetupConfig { SetupConfig { id: LOCAL_SETUP_ID.to_string(), name: LOCAL_SETUP.to_string(), ssh: None, - providers: vec![ - ProviderConfig { - name: ECHO_PROVIDER.to_string(), - kind: DriverKind::Echo, - command: None, - models: Vec::new(), - }, - ProviderConfig { - name: "claude-cli".to_string(), - kind: DriverKind::ClaudeCli, - command: None, - models: ["fable", "opus", "sonnet", "haiku"] - .iter() - .map(|m| (*m).to_string()) - .collect(), - }, - ], + providers, + } + } + + /// The one provider that needs no discovery, and the floor to fall + /// back to when discovery itself fails. + /// + /// Echo runs in-process, so it exists exactly where this server does + /// and nowhere else -- there is nothing to probe for, and offering it + /// on a remote machine would be a choice that changes nothing. + pub fn echo_provider() -> ProviderConfig { + ProviderConfig { + name: ECHO_PROVIDER.to_string(), + kind: DriverKind::Echo, + command: None, + models: Vec::new(), } } @@ -315,7 +317,15 @@ mod tests { sha256: "ab".repeat(32), }], setups: vec![ - Config::seed(), + Config::seed(vec![ + Config::echo_provider(), + ProviderConfig { + name: "claude-cli".to_string(), + kind: DriverKind::ClaudeCli, + command: Some("/usr/bin/claude".to_string()), + models: Vec::new(), + }, + ]), SetupConfig { id: "vm".to_string(), name: "the vm".to_string(), @@ -395,21 +405,44 @@ mod tests { } #[test] - /// What a fresh install can do before anybody configures anything: - /// echo to prove the pipe, and the Claude CLI to be useful. Both are - /// on the machine the server runs on, and echo belongs there because - /// it runs in-process -- there is no transport for it to cross. - fn the_seed_setup_is_this_machine_and_can_spawn_something() { - let seed = Config::seed(); + /// The seed is this machine and nothing more: a name, no ssh, and + /// exactly the providers it was handed. + /// + /// It used to assert a `claude-cli` provider here, which is what made + /// the bug look correct -- the test agreed with the code that every + /// machine has `claude`, because both were written from the same + /// assumption. What a machine has is discovered, so the only thing + /// this can check is that the seed does not invent anything. + fn the_seed_is_this_machine_and_claims_only_what_it_was_given() { + let seed = Config::seed(vec![Config::echo_provider()]); assert_eq!(seed.name, LOCAL_SETUP); assert!(seed.ssh.is_none()); assert_eq!( seed.provider(ECHO_PROVIDER).expect("echo").kind, DriverKind::Echo ); + assert!( + seed.provider("claude-cli").is_none(), + "the seed must not assert a provider nobody looked for", + ); + + // And it carries through whatever discovery did find. + let discovered = Config::seed(vec![ + Config::echo_provider(), + ProviderConfig { + name: "claude-cli".to_string(), + kind: DriverKind::ClaudeCli, + command: Some("/usr/bin/claude".to_string()), + models: Vec::new(), + }, + ]); assert_eq!( - seed.provider("claude-cli").expect("claude").kind, - DriverKind::ClaudeCli, + discovered + .provider("claude-cli") + .expect("found") + .command + .as_deref(), + Some("/usr/bin/claude"), ); } } diff --git a/server/src/main.rs b/server/src/main.rs index 29df202..15f4162 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -109,6 +109,11 @@ async fn main() -> Result<()> { SessionManager::new(config_path.clone(), data_dir, models_dir.clone()) .with_context(|| format!("failed to load {}", config_path.display()))?, ); + // After construction rather than inside it: seeding asks this machine + // what it has, which is I/O, and a constructor that quietly runs a + // subprocess is a surprise to every caller including the tests. + manager.seed_setup().await?; + tracing::info!("config: {}", config_path.display()); tracing::info!("models: {}", models_dir.display()); for setup in manager.setups() { diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index c2ad10e..df79a53 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -241,29 +241,54 @@ impl SessionManager { models_dir, inner: RwLock::new(Inner { config, live }), }; - manager.seed_setup()?; Ok(manager) } - /// Writes a starting `claude-cli` provider into a config that has none, - /// so a fresh install has something to spawn and a worked example of - /// the schema to edit. Runs local by default -- a session is given a - /// host when the CLI lives elsewhere, which is a per-session choice. - /// Gives a fresh install something to spawn. Only ever fires when - /// there are no setups at all -- deleting the last one is a choice, - /// not a state to be repaired. - fn seed_setup(&self) -> Result<()> { + /// Writes this machine into a config that has no setups, with the + /// providers actually found on it. + /// + /// Discovered rather than assumed. Until 2026-08-28 this wrote a + /// `claude-cli` provider unconditionally, so a fresh install on a + /// machine without `claude` -- which is every machine but the dev VM + /// -- offered a spawn option that could not work, and said so with the + /// same confidence as a provider that had been checked for. Providers + /// are discovered by asking the machine, and the local machine is not + /// an exception to that. + /// + /// A discovery that fails seeds only `echo`, which is true wherever + /// this server runs, and says so in the log. Seeding the hardcoded + /// list on failure would be the original bug with an extra step, and + /// seeding nothing would leave a fresh install with nothing to prove + /// the pipe with. + pub async fn seed_setup(&self) -> Result<()> { + if !self.inner.read().unwrap().config.setups.is_empty() { + return Ok(()); + } + let providers = match crate::setups::discover(&transport::Transport::Here).await { + Ok(found) => found, + Err(err) => { + tracing::warn!( + "couldn't ask this machine what it has ({err}); seeding {} only -- \ + re-probe the setup from the app once that is fixed", + crate::config::ECHO_PROVIDER + ); + vec![Config::echo_provider()] + } + }; + let names: Vec<&str> = providers.iter().map(|p| p.name.as_str()).collect(); + let mut inner = self.inner.write().unwrap(); if !inner.config.setups.is_empty() { return Ok(()); } let mut candidate = inner.config.clone(); - candidate.setups.push(Config::seed()); + candidate.setups.push(Config::seed(providers.clone())); candidate.save(&self.config_path)?; inner.config = candidate; tracing::info!( - "no setups configured -- added \"{}\"", - crate::config::LOCAL_SETUP + "no setups configured -- added \"{}\" with: {}", + crate::config::LOCAL_SETUP, + names.join(", ") ); Ok(()) } @@ -763,11 +788,28 @@ mod tests { .await } + /// Writes this machine into `config_path` with echo and nothing else. + /// + /// Explicit rather than letting the manager seed itself: seeding now + /// asks the machine what it has, so a test that relied on it would + /// pass or fail depending on whether `claude` happens to be installed + /// on whoever is running it. Echo is the only provider that is true + /// everywhere, and the only one these tests need. + fn seed_echo_only(config_path: &std::path::Path) { + Config { + setups: vec![Config::seed(vec![Config::echo_provider()])], + ..Config::default() + } + .save(config_path) + .expect("seed config"); + } + #[tokio::test] async fn spawn_message_and_delete_round_trip() { let dir = tempfile::tempdir().expect("tempdir"); let config_path = dir.path().join("config.ron"); let data_dir = dir.path().join("sessions"); + seed_echo_only(&config_path); let manager = SessionManager::new( config_path.clone(), data_dir.clone(), @@ -828,6 +870,7 @@ mod tests { #[tokio::test] async fn questions_round_trip_through_answer() { let dir = tempfile::tempdir().expect("tempdir"); + seed_echo_only(&dir.path().join("config.ron")); let manager = SessionManager::new( dir.path().join("config.ron"), dir.path().join("sessions"), @@ -869,6 +912,7 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let config_path = dir.path().join("config.ron"); let data_dir = dir.path().join("sessions"); + seed_echo_only(&config_path); let manager = SessionManager::new( config_path.clone(),