From e047a9adffe3649be2832aff5cabd3ca5b010307 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 23 Oct 2026 16:10:25 -0400 Subject: [PATCH 1/3] 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)); From 6ba50e08077db1eda2aa24577648a3d671558c6e Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 23 Oct 2026 16:32:19 -0400 Subject: [PATCH 2/3] Keep installer tests focused on behavioral contracts --- lib/apps/fabro-server/tests/it/api/install.rs | 10 ---------- lib/components/fabro-install/src/lib.rs | 10 ++-------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/apps/fabro-server/tests/it/api/install.rs b/lib/apps/fabro-server/tests/it/api/install.rs index c958a7d36..778b95403 100644 --- a/lib/apps/fabro-server/tests/it/api/install.rs +++ b/lib/apps/fabro-server/tests/it/api/install.rs @@ -956,9 +956,6 @@ 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\"")); - 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; @@ -2609,10 +2606,6 @@ async fn sandbox_switching_from_daytona_to_docker_drops_saved_key() { ) .await; - let settings = std::fs::read_to_string(&config_path).unwrap(); - 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"); @@ -2724,9 +2717,6 @@ async fn daytona_install_finish_writes_settings_and_vault_secret() { .await; let settings = std::fs::read_to_string(&config_path).unwrap(); - 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 1c956155d..785a4401e 100644 --- a/lib/components/fabro-install/src/lib.rs +++ b/lib/components/fabro-install/src/lib.rs @@ -1324,7 +1324,6 @@ stale = "remove-me" .and_then(toml::Value::as_str), Some("/srv/fabro/objects") ); - assert!(server.get("slatedb").is_none()); assert!(plan.writes.is_empty()); assert_eq!(plan.removals.len(), 2); } @@ -1353,7 +1352,6 @@ stale = "remove-me" .and_then(toml::Value::as_str), Some("artifacts") ); - assert!(server.get("slatedb").is_none()); assert!(plan.writes.is_empty()); } @@ -1392,26 +1390,22 @@ stale = "remove-me" } #[test] - fn write_sandbox_settings_records_docker_policy_without_run_settings() { + fn write_sandbox_settings_records_docker_policy() { let mut doc = toml::Value::Table(toml::Table::default()); write_sandbox_settings(&mut doc, InstallSandboxSelection::Docker, true) .expect("docker sandbox selection should succeed"); - 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)); assert_eq!(sandbox_provider_enabled(&doc, "daytona"), Some(false)); } #[test] - fn write_sandbox_settings_records_daytona_policy_without_run_settings() { + fn write_sandbox_settings_records_daytona_policy() { let mut doc = toml::Value::Table(toml::Table::default()); write_sandbox_settings(&mut doc, InstallSandboxSelection::Daytona, true) .expect("daytona sandbox selection should succeed"); - 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)); assert_eq!(sandbox_provider_enabled(&doc, "daytona"), Some(true)); From cd9b0be39340e4f2cd035eb43a884afbb50a1152 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 23 Oct 2026 17:20:56 -0400 Subject: [PATCH 3/3] Remove unnecessary installer warning regression test --- lib/apps/fabro-cli/tests/it/cmd/create.rs | 52 ----------------------- 1 file changed, 52 deletions(-) diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index 1d10bd302..2b4ee82df 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -6,7 +6,6 @@ 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; @@ -1582,57 +1581,6 @@ 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!();