From a00b95abe03bcae2a5db6ca11a7c18212f337154 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Thu, 3 Sep 2026 12:56:47 -0400 Subject: [PATCH] Report available environments when default is missing When `fabro run` or `fabro create` omits `--environment` and the server has no environment named `default`, the CLI previously failed with only "could not retrieve environment `default`". It now lists the server's environment catalog in the error so the user can pass an explicit `--environment ` or create the missing `default` entry. Explicit `--environment` lookups keep their existing not-found message. Adds `Client::list_environments` to fabro-client for the catalog read. Co-Authored-By: Claude Fable 5.1 --- lib/apps/fabro-cli/src/commands/run/create.rs | 50 ++++++++++++---- lib/apps/fabro-cli/tests/it/cmd/create.rs | 58 +++++++++++++++++++ lib/foundation/fabro-client/src/client.rs | 35 +++++++++++ 3 files changed, 132 insertions(+), 11 deletions(-) diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 184c19816..f9d0fb037 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -2,7 +2,7 @@ use std::path::Path; use anyhow::{Context as _, anyhow, bail}; use fabro_config::project; -use fabro_environment::DEFAULT_ENVIRONMENT_ID; +use fabro_environment::{DEFAULT_ENVIRONMENT_ID, Environment}; use fabro_types::settings::run::EnvironmentProvider; use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget}; use fabro_util::terminal::Styles; @@ -61,10 +61,6 @@ pub(crate) async fn create_run( } let client = ctx.server().await?; - let environment_id = args - .environment - .as_deref() - .unwrap_or(DEFAULT_ENVIRONMENT_ID); let (parent_id, environment) = tokio::try_join!( async { match args.parent.as_deref() { @@ -74,12 +70,7 @@ pub(crate) async fn create_run( None => Ok(None), } }, - async { - client - .retrieve_environment(environment_id) - .await - .with_context(|| format!("could not retrieve environment `{environment_id}`")) - }, + resolve_run_environment(client.as_ref(), args.environment.as_deref()), )?; let (target, dirty_worktree) = run_target_for_environment(environment.settings.provider, &canonical_cwd)?; @@ -118,6 +109,43 @@ pub(crate) async fn create_run( }) } +/// Retrieves the run environment, defaulting to `default` when `--environment` +/// is omitted. A missing `default` is reported with the catalog so the user +/// can pick an explicit environment or create the missing one. +async fn resolve_run_environment( + client: &fabro_client::Client, + explicit_id: Option<&str>, +) -> anyhow::Result { + let id = explicit_id.unwrap_or(DEFAULT_ENVIRONMENT_ID); + match client.retrieve_environment(id).await { + Ok(environment) => Ok(environment), + Err(error) if explicit_id.is_none() && fabro_client::is_not_found_error(&error) => { + Err(missing_default_environment_error(client).await) + } + Err(error) => Err(error).with_context(|| format!("could not retrieve environment `{id}`")), + } +} + +async fn missing_default_environment_error(client: &fabro_client::Client) -> anyhow::Error { + let catalog_hint = match client.list_environments().await { + Ok(environments) if environments.is_empty() => { + " The server has no environments configured.".to_string() + } + Ok(environments) => { + let ids = environments + .iter() + .map(|environment| format!("`{}`", environment.id)) + .collect::>() + .join(", "); + format!(" Available environments: {ids}.") + } + Err(_) => String::new(), + }; + anyhow!( + "environment `{DEFAULT_ENVIRONMENT_ID}` not found on the server; pass `--environment ` or create an environment named `{DEFAULT_ENVIRONMENT_ID}`.{catalog_hint}" + ) +} + fn warn_untransmitted_settings( ctx: &CommandContext, styles: &Styles, diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index d8125f278..0818a83b6 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -155,6 +155,64 @@ fn create_uses_explicit_server_target_and_prints_remote_run_id() { assert_eq!(output_stdout(&output).trim(), run_id.as_str()); } +#[test] +fn create_reports_available_environments_when_default_is_missing() { + let context = test_context!(); + let server = MockServer::start(); + let default_mock = server.mock(|when, then| { + when.method("GET").path("/api/v1/environments/default"); + then.status(404) + .header("content-type", "application/json") + .json_body(json!({ + "errors": [{ + "status": "404", + "title": "Not Found", + "detail": "environment `default` not found", + "code": "environment_not_found" + }] + })); + }); + let list_mock = server.mock(|when, then| { + when.method("GET").path("/api/v1/environments"); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ + "data": [ + environment_json("production", "docker"), + environment_json("staging", "daytona"), + ], + "meta": { "total": 2 } + })); + }); + let create_mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(500); + }); + + let output = context + .create_cmd() + .args([ + "--server", + &format!("{}/api/v1", server.base_url()), + "--dry-run", + fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!(!output.status.success(), "command should fail"); + default_mock.assert(); + list_mock.assert(); + create_mock.assert_calls(0); + let stderr = output_stderr(&output); + assert!( + stderr.contains( + "environment `default` not found on the server; pass `--environment ` or create an environment named `default`. Available environments: `production`, `staging`." + ), + "unexpected stderr:\n{stderr}" + ); +} + #[test] fn create_defers_provider_validation_to_the_server() { let context = test_context!(); diff --git a/lib/foundation/fabro-client/src/client.rs b/lib/foundation/fabro-client/src/client.rs index 9ed8e88c1..add17b464 100644 --- a/lib/foundation/fabro-client/src/client.rs +++ b/lib/foundation/fabro-client/src/client.rs @@ -712,6 +712,14 @@ impl Client { Ok(response.into_inner()) } + /// Lists the canonical server-managed environment catalog. + pub async fn list_environments(&self) -> Result> { + let response = self + .send_api(|client| async move { client.list_environments().send().await }) + .await?; + Ok(response.into_inner().data) + } + /// Registers one workflow version and verifies the server assigned the /// content-derived id, so a mismatched response fails loudly here rather /// than being trusted downstream. @@ -2460,6 +2468,33 @@ mod tests { assert_eq!(environment.settings.provider, EnvironmentProvider::Local); } + #[tokio::test] + async fn list_environments_returns_the_canonical_catalog() { + let server = MockServer::start_async().await; + let mock = server + .mock_async(|when, then| { + when.method(GET).path("/api/v1/environments"); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ + "data": [environment_json("production", "daytona")], + "meta": { "total": 1 } + })); + }) + .await; + let client = Client::new_no_proxy(&server.url("")).unwrap(); + + let environments = client.list_environments().await.unwrap(); + + mock.assert_async().await; + assert_eq!(environments.len(), 1); + assert_eq!(environments[0].id.as_str(), "production"); + assert_eq!( + environments[0].settings.provider, + EnvironmentProvider::Daytona + ); + } + #[tokio::test] async fn retrieve_environment_preserves_api_failure_metadata() { let server = MockServer::start_async().await;