Discover this machine's providers instead of asserting them
A fresh install wrote a `claude-cli` provider into the local setup unconditionally. Nothing looked for `claude`; the list was hardcoded in `Config::seed`, so on any machine without it -- which is every machine but the dev VM -- the phone was offered a provider that cannot spawn, stated with exactly the confidence of one that had been checked. Discovery already existed and was already right: `setups::discover` probes with `command -v` over the transport, includes echo for the local one because it runs in-process, and records the resolved path rather than the bare name. Only the local setup skipped it, which is the one place the answer felt obvious enough not to ask. So `seed` now takes the providers it is given, and seeding asks this machine the same question it asks any other. It moved out of `SessionManager::new` into an awaited step in main, because asking is I/O and a constructor that quietly spawns a subprocess surprises every caller. A discovery that fails seeds `echo` alone and says so, since echo is true wherever this server runs -- falling back to the hardcoded list would be the same bug with an extra step. The test that covered this agreed with the bug, because both were written from the same assumption: it asserted the seed contains `claude-cli`. It now asserts the opposite -- that the seed invents nothing -- and the session tests seed echo explicitly rather than relying on a constructor that would make them pass or fail on whether `claude` happens to be installed on whoever runs them. Verified by running a server on a PATH holding only `sh`: it seeds `echo` alone. With claude and llama-server present it finds both. 34 tests.
This commit is contained in:
1 parent
3cf6925d90
commit
4370c467ca
3 files changed
+126
-44
No files matched your search
+65
-32
@@ -212,34 +212,36 @@ impl Config {
|
|||||||
self.setups.iter().find(|setup| setup.name == name)
|
self.setups.iter().find(|setup| setup.name == name)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// What a fresh install starts with: this machine, offering the echo
|
/// This machine, offering whatever was found on it.
|
||||||
/// driver to prove the pipe and the Claude CLI to be useful.
|
|
||||||
///
|
///
|
||||||
/// Echo runs in-process, so it belongs to the setup with no ssh --
|
/// The providers are passed in rather than written here because they
|
||||||
/// there is nothing for a transport to wrap, and offering it on a
|
/// have to be *discovered*: a hardcoded list is a claim about what is
|
||||||
/// remote machine would be a choice that changes nothing.
|
/// installed, and this one was wrong -- every fresh install asserted a
|
||||||
pub fn seed() -> SetupConfig {
|
/// `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<ProviderConfig>) -> SetupConfig {
|
||||||
SetupConfig {
|
SetupConfig {
|
||||||
id: LOCAL_SETUP_ID.to_string(),
|
id: LOCAL_SETUP_ID.to_string(),
|
||||||
name: LOCAL_SETUP.to_string(),
|
name: LOCAL_SETUP.to_string(),
|
||||||
ssh: None,
|
ssh: None,
|
||||||
providers: vec![
|
providers,
|
||||||
ProviderConfig {
|
}
|
||||||
name: ECHO_PROVIDER.to_string(),
|
}
|
||||||
kind: DriverKind::Echo,
|
|
||||||
command: None,
|
/// The one provider that needs no discovery, and the floor to fall
|
||||||
models: Vec::new(),
|
/// back to when discovery itself fails.
|
||||||
},
|
///
|
||||||
ProviderConfig {
|
/// Echo runs in-process, so it exists exactly where this server does
|
||||||
name: "claude-cli".to_string(),
|
/// and nowhere else -- there is nothing to probe for, and offering it
|
||||||
kind: DriverKind::ClaudeCli,
|
/// on a remote machine would be a choice that changes nothing.
|
||||||
command: None,
|
pub fn echo_provider() -> ProviderConfig {
|
||||||
models: ["fable", "opus", "sonnet", "haiku"]
|
ProviderConfig {
|
||||||
.iter()
|
name: ECHO_PROVIDER.to_string(),
|
||||||
.map(|m| (*m).to_string())
|
kind: DriverKind::Echo,
|
||||||
.collect(),
|
command: None,
|
||||||
},
|
models: Vec::new(),
|
||||||
],
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -315,7 +317,15 @@ mod tests {
|
|||||||
sha256: "ab".repeat(32),
|
sha256: "ab".repeat(32),
|
||||||
}],
|
}],
|
||||||
setups: vec![
|
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 {
|
SetupConfig {
|
||||||
id: "vm".to_string(),
|
id: "vm".to_string(),
|
||||||
name: "the vm".to_string(),
|
name: "the vm".to_string(),
|
||||||
@@ -395,21 +405,44 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
/// What a fresh install can do before anybody configures anything:
|
/// The seed is this machine and nothing more: a name, no ssh, and
|
||||||
/// echo to prove the pipe, and the Claude CLI to be useful. Both are
|
/// exactly the providers it was handed.
|
||||||
/// on the machine the server runs on, and echo belongs there because
|
///
|
||||||
/// it runs in-process -- there is no transport for it to cross.
|
/// It used to assert a `claude-cli` provider here, which is what made
|
||||||
fn the_seed_setup_is_this_machine_and_can_spawn_something() {
|
/// the bug look correct -- the test agreed with the code that every
|
||||||
let seed = Config::seed();
|
/// 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_eq!(seed.name, LOCAL_SETUP);
|
||||||
assert!(seed.ssh.is_none());
|
assert!(seed.ssh.is_none());
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
seed.provider(ECHO_PROVIDER).expect("echo").kind,
|
seed.provider(ECHO_PROVIDER).expect("echo").kind,
|
||||||
DriverKind::Echo
|
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!(
|
assert_eq!(
|
||||||
seed.provider("claude-cli").expect("claude").kind,
|
discovered
|
||||||
DriverKind::ClaudeCli,
|
.provider("claude-cli")
|
||||||
|
.expect("found")
|
||||||
|
.command
|
||||||
|
.as_deref(),
|
||||||
|
Some("/usr/bin/claude"),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -109,6 +109,11 @@ async fn main() -> Result<()> {
|
|||||||
SessionManager::new(config_path.clone(), data_dir, models_dir.clone())
|
SessionManager::new(config_path.clone(), data_dir, models_dir.clone())
|
||||||
.with_context(|| format!("failed to load {}", config_path.display()))?,
|
.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!("config: {}", config_path.display());
|
||||||
tracing::info!("models: {}", models_dir.display());
|
tracing::info!("models: {}", models_dir.display());
|
||||||
for setup in manager.setups() {
|
for setup in manager.setups() {
|
||||||
|
|||||||
+56
-12
@@ -241,29 +241,54 @@ impl SessionManager {
|
|||||||
models_dir,
|
models_dir,
|
||||||
inner: RwLock::new(Inner { config, live }),
|
inner: RwLock::new(Inner { config, live }),
|
||||||
};
|
};
|
||||||
manager.seed_setup()?;
|
|
||||||
Ok(manager)
|
Ok(manager)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Writes a starting `claude-cli` provider into a config that has none,
|
/// Writes this machine into a config that has no setups, with the
|
||||||
/// so a fresh install has something to spawn and a worked example of
|
/// providers actually found on it.
|
||||||
/// 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.
|
/// Discovered rather than assumed. Until 2026-08-28 this wrote a
|
||||||
/// Gives a fresh install something to spawn. Only ever fires when
|
/// `claude-cli` provider unconditionally, so a fresh install on a
|
||||||
/// there are no setups at all -- deleting the last one is a choice,
|
/// machine without `claude` -- which is every machine but the dev VM
|
||||||
/// not a state to be repaired.
|
/// -- offered a spawn option that could not work, and said so with the
|
||||||
fn seed_setup(&self) -> Result<()> {
|
/// 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();
|
let mut inner = self.inner.write().unwrap();
|
||||||
if !inner.config.setups.is_empty() {
|
if !inner.config.setups.is_empty() {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
let mut candidate = inner.config.clone();
|
let mut candidate = inner.config.clone();
|
||||||
candidate.setups.push(Config::seed());
|
candidate.setups.push(Config::seed(providers.clone()));
|
||||||
candidate.save(&self.config_path)?;
|
candidate.save(&self.config_path)?;
|
||||||
inner.config = candidate;
|
inner.config = candidate;
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
"no setups configured -- added \"{}\"",
|
"no setups configured -- added \"{}\" with: {}",
|
||||||
crate::config::LOCAL_SETUP
|
crate::config::LOCAL_SETUP,
|
||||||
|
names.join(", ")
|
||||||
);
|
);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -763,11 +788,28 @@ mod tests {
|
|||||||
.await
|
.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]
|
#[tokio::test]
|
||||||
async fn spawn_message_and_delete_round_trip() {
|
async fn spawn_message_and_delete_round_trip() {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
let config_path = dir.path().join("config.ron");
|
let config_path = dir.path().join("config.ron");
|
||||||
let data_dir = dir.path().join("sessions");
|
let data_dir = dir.path().join("sessions");
|
||||||
|
seed_echo_only(&config_path);
|
||||||
let manager = SessionManager::new(
|
let manager = SessionManager::new(
|
||||||
config_path.clone(),
|
config_path.clone(),
|
||||||
data_dir.clone(),
|
data_dir.clone(),
|
||||||
@@ -828,6 +870,7 @@ mod tests {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn questions_round_trip_through_answer() {
|
async fn questions_round_trip_through_answer() {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
seed_echo_only(&dir.path().join("config.ron"));
|
||||||
let manager = SessionManager::new(
|
let manager = SessionManager::new(
|
||||||
dir.path().join("config.ron"),
|
dir.path().join("config.ron"),
|
||||||
dir.path().join("sessions"),
|
dir.path().join("sessions"),
|
||||||
@@ -869,6 +912,7 @@ mod tests {
|
|||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
let config_path = dir.path().join("config.ron");
|
let config_path = dir.path().join("config.ron");
|
||||||
let data_dir = dir.path().join("sessions");
|
let data_dir = dir.path().join("sessions");
|
||||||
|
seed_echo_only(&config_path);
|
||||||
|
|
||||||
let manager = SessionManager::new(
|
let manager = SessionManager::new(
|
||||||
config_path.clone(),
|
config_path.clone(),
|
||||||
|
|||||||
Reference in new issue
Block a user