mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
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 <id>` 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 <noreply@anthropic.com>
This commit is contained in:
parent
f52f2a1edb
commit
a00b95abe0
3 changed files with 132 additions and 11 deletions
|
|
@ -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<Environment> {
|
||||
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::<Vec<_>>()
|
||||
.join(", ");
|
||||
format!(" Available environments: {ids}.")
|
||||
}
|
||||
Err(_) => String::new(),
|
||||
};
|
||||
anyhow!(
|
||||
"environment `{DEFAULT_ENVIRONMENT_ID}` not found on the server; pass `--environment <id>` or create an environment named `{DEFAULT_ENVIRONMENT_ID}`.{catalog_hint}"
|
||||
)
|
||||
}
|
||||
|
||||
fn warn_untransmitted_settings(
|
||||
ctx: &CommandContext,
|
||||
styles: &Styles,
|
||||
|
|
|
|||
|
|
@ -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 <id>` 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!();
|
||||
|
|
|
|||
|
|
@ -712,6 +712,14 @@ impl Client {
|
|||
Ok(response.into_inner())
|
||||
}
|
||||
|
||||
/// Lists the canonical server-managed environment catalog.
|
||||
pub async fn list_environments(&self) -> Result<Vec<types::Environment>> {
|
||||
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;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue