From e047a9adffe3649be2832aff5cabd3ca5b010307 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 23 Oct 2026 16:10:25 -0400 Subject: [PATCH] Stop generating ignored run settings during browser installation --- lib/apps/fabro-cli/tests/it/cmd/create.rs | 52 +++++++++++++++++++ lib/apps/fabro-server/tests/it/api/install.rs | 29 ++++------- lib/components/fabro-install/src/lib.rs | 31 +++-------- 3 files changed, 68 insertions(+), 44 deletions(-) diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index 2b4ee82df..1d10bd302 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -6,6 +6,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; +use fabro_install::InstallSandboxSelection; use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; use httpmock::{HttpMockResponse, Mock, MockServer}; use insta::assert_snapshot; @@ -1581,6 +1582,57 @@ fn create_registers_package_before_surfacing_server_admission_rejection() { assert!(!stderr.contains("Validation failed"), "{stderr}"); } +#[test] +fn create_uses_server_environment_without_warning_for_installer_settings() { + for (selection, provider) in [ + (InstallSandboxSelection::Docker, "docker"), + (InstallSandboxSelection::Daytona, "daytona"), + ] { + for explicit_environment in [None, Some("custom")] { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + let environment_id = explicit_environment.unwrap_or("default"); + let environment_mock = mock_environment(&server, environment_id, provider); + let version_mock = mock_workflow_version_registrations(&server); + let requests = Arc::new(Mutex::new(Vec::new())); + let create_mock = mock_intent_create(&server, &run_id, Arc::clone(&requests)); + let project = tempfile::tempdir().unwrap(); + let workflow = write_workflow(project.path(), "workflow", "InstalledSettings"); + let mut settings = toml::Value::Table(toml::Table::default()); + fabro_install::write_sandbox_settings(&mut settings, selection, true).unwrap(); + context.write_home( + ".fabro/settings.toml", + toml::to_string_pretty(&settings).unwrap(), + ); + + let mut command = context.create_cmd(); + command.current_dir(project.path()).args([ + "--server", + &format!("{}/api/v1", server.base_url()), + workflow.to_str().unwrap(), + ]); + if let Some(environment_id) = explicit_environment { + command.args(["--environment", environment_id]); + } + let output = command.output().unwrap(); + + assert!(output.status.success(), "{}", output_stderr(&output)); + environment_mock.assert(); + version_mock.assert(); + create_mock.assert_calls(1); + assert_eq!( + requests.lock().unwrap()[0]["environment_id"], + environment_id + ); + assert_eq!(output_stdout(&output).trim(), run_id.as_str()); + insta::allow_duplicates! { + assert_snapshot!(output_stderr(&output), @""); + } + } + } +} + #[test] fn create_does_not_warn_for_client_only_settings() { let context = test_context!(); diff --git a/lib/apps/fabro-server/tests/it/api/install.rs b/lib/apps/fabro-server/tests/it/api/install.rs index 995976580..c958a7d36 100644 --- a/lib/apps/fabro-server/tests/it/api/install.rs +++ b/lib/apps/fabro-server/tests/it/api/install.rs @@ -956,14 +956,9 @@ async fn token_install_finish_persists_settings_env_and_vault() { let settings = std::fs::read_to_string(&config_path).unwrap(); assert!(settings.contains("https://fabro.example.com")); assert!(settings.contains("strategy = \"token\"")); - assert!( - settings.contains("[run.environment]"), - "settings.toml should contain [run.environment]" - ); - assert!( - !settings.contains("[environments"), - "settings.toml should not contain environment catalog entries" - ); + let document: toml::Value = toml::from_str(&settings).unwrap(); + assert!(document.get("run").is_none()); + assert!(document.get("environments").is_none()); assert_sandbox_provider_policy(&settings, true, true, false); assert_no_legacy_environment_dir(&temp_dir); let default_environment = seeded_default_environment(&temp_dir).await; @@ -2615,10 +2610,9 @@ async fn sandbox_switching_from_daytona_to_docker_drops_saved_key() { .await; let settings = std::fs::read_to_string(&config_path).unwrap(); - assert!( - settings.contains("[run.environment]"), - "settings.toml should select the default environment" - ); + let document: toml::Value = toml::from_str(&settings).unwrap(); + assert!(document.get("run").is_none()); + assert!(document.get("environments").is_none()); assert_no_legacy_environment_dir(&temp_dir); let default_environment = seeded_default_environment(&temp_dir).await; assert_eq!(default_environment.settings.provider.to_string(), "docker"); @@ -2730,14 +2724,9 @@ async fn daytona_install_finish_writes_settings_and_vault_secret() { .await; let settings = std::fs::read_to_string(&config_path).unwrap(); - assert!( - settings.contains("[run.environment]"), - "settings.toml should contain [run.environment]" - ); - assert!( - !settings.contains("[environments"), - "settings.toml should not contain environment catalog entries" - ); + let document: toml::Value = toml::from_str(&settings).unwrap(); + assert!(document.get("run").is_none()); + assert!(document.get("environments").is_none()); assert_sandbox_provider_policy(&settings, true, false, true); assert_no_legacy_environment_dir(&temp_dir); let default_environment = seeded_default_environment(&temp_dir).await; diff --git a/lib/components/fabro-install/src/lib.rs b/lib/components/fabro-install/src/lib.rs index b73070020..1c956155d 100644 --- a/lib/components/fabro-install/src/lib.rs +++ b/lib/components/fabro-install/src/lib.rs @@ -498,10 +498,9 @@ pub fn write_sandbox_settings( allow_local: bool, ) -> Result<()> { let root = root_table_mut(doc)?; - let run = ensure_table(root, "run")?; - let environment = ensure_table(run, "environment")?; - environment.insert("id".to_string(), toml::Value::String("default".to_string())); - + // Install seeds `default` in the server catalog separately. Run creation + // already selects it when no environment is supplied; writing a [run] table + // here would make the CLI warn about settings it does not transmit. let server = ensure_table(root, "server")?; write_sandbox_provider_policy(server, selection, allow_local)?; Ok(()) @@ -1393,20 +1392,12 @@ stale = "remove-me" } #[test] - fn write_sandbox_settings_records_run_default_without_environment_catalog() { + fn write_sandbox_settings_records_docker_policy_without_run_settings() { let mut doc = toml::Value::Table(toml::Table::default()); write_sandbox_settings(&mut doc, InstallSandboxSelection::Docker, true) .expect("docker sandbox selection should succeed"); - assert_eq!( - doc.get("run") - .and_then(toml::Value::as_table) - .and_then(|run| run.get("environment")) - .and_then(toml::Value::as_table) - .and_then(|env| env.get("id")) - .and_then(toml::Value::as_str), - Some("default") - ); + assert!(doc.get("run").is_none()); assert!(doc.get("environments").is_none()); assert_eq!(sandbox_provider_enabled(&doc, "local"), Some(true)); assert_eq!(sandbox_provider_enabled(&doc, "docker"), Some(true)); @@ -1414,20 +1405,12 @@ stale = "remove-me" } #[test] - fn write_sandbox_settings_records_daytona_policy_without_environment_catalog() { + fn write_sandbox_settings_records_daytona_policy_without_run_settings() { let mut doc = toml::Value::Table(toml::Table::default()); write_sandbox_settings(&mut doc, InstallSandboxSelection::Daytona, true) .expect("daytona sandbox selection should succeed"); - assert_eq!( - doc.get("run") - .and_then(toml::Value::as_table) - .and_then(|run| run.get("environment")) - .and_then(toml::Value::as_table) - .and_then(|env| env.get("id")) - .and_then(toml::Value::as_str), - Some("default") - ); + assert!(doc.get("run").is_none()); assert!(doc.get("environments").is_none()); assert_eq!(sandbox_provider_enabled(&doc, "local"), Some(true)); assert_eq!(sandbox_provider_enabled(&doc, "docker"), Some(false));