diff --git a/crates/local-deployment/src/container.rs b/crates/local-deployment/src/container.rs index be28726a5e..877960440c 100644 --- a/crates/local-deployment/src/container.rs +++ b/crates/local-deployment/src/container.rs @@ -310,7 +310,7 @@ impl LocalContainerService { } async fn cleanup_expired_workspaces(&self) -> Result<(), DeploymentError> { - if std::env::var("DISABLE_WORKTREE_CLEANUP").is_ok() { + if utils::env::disable_flag_set("DISABLE_WORKTREE_CLEANUP") { tracing::info!( "Expired workspace cleanup is disabled via DISABLE_WORKTREE_CLEANUP environment variable" ); @@ -348,7 +348,7 @@ impl LocalContainerService { const IDLE_REAP_HOURS: i64 = 48; const ACTIVITY_GRACE_SECS: i64 = 60; - if std::env::var("DISABLE_CLI_SESSION_REAP").is_ok() { + if utils::env::disable_flag_set("DISABLE_CLI_SESSION_REAP") { return; } let sessions = match crate::pty::list_cli_tmux_sessions().await { diff --git a/crates/local-deployment/src/loop_supervisor.rs b/crates/local-deployment/src/loop_supervisor.rs index f0e097c9d8..46aaf2623e 100644 --- a/crates/local-deployment/src/loop_supervisor.rs +++ b/crates/local-deployment/src/loop_supervisor.rs @@ -358,7 +358,7 @@ pub struct LoopSupervisor; impl LoopSupervisor { pub fn spawn(db: DBService) { tokio::spawn(async move { - if std::env::var(DISABLE_ENV).is_ok() { + if utils::env::disable_flag_set(DISABLE_ENV) { tracing::info!("{DISABLE_ENV} set; loop automation supervisor disabled"); return; } diff --git a/crates/utils/src/env.rs b/crates/utils/src/env.rs index 5bc6a79dc7..044a407008 100644 --- a/crates/utils/src/env.rs +++ b/crates/utils/src/env.rs @@ -62,6 +62,40 @@ pub(crate) fn normalize_path_override(name: &str, value: Option) -> Op } } +/// Check a `DISABLE_*` style opt-out flag. +/// +/// Any value — including `0`, `false` and the empty string — disables the gated +/// behaviour. That is deliberately unchanged from the original +/// `std::env::var(..).is_ok()` gate: several of these flags guard destructive +/// cleanup, so tightening them such that `DISABLE_X=0` means "enabled" would +/// silently switch worktree deletion back on for anyone relying on the previous +/// behaviour. The surprising case is reported instead of reinterpreted. +pub fn disable_flag_set(name: &str) -> bool { + evaluate_disable_flag(name, std::env::var(name).ok()) +} + +/// Evaluate an opt-out flag without reading the environment, so the warning +/// behaviour is testable. +pub(crate) fn evaluate_disable_flag(name: &str, value: Option) -> bool { + let Some(value) = value else { + return false; + }; + + if matches!( + value.trim().to_ascii_lowercase().as_str(), + "" | "0" | "false" | "no" | "off" + ) { + tracing::warn!( + variable = name, + value = %value, + "Opt-out flag is set to a falsy-looking value but still DISABLES the \ + gated behaviour; unset the variable entirely to re-enable it" + ); + } + + true +} + fn select_env_var_with_legacy(lookup: F, new_name: &str, legacy_name: &str) -> Option where F: Fn(&str) -> Option, @@ -86,6 +120,28 @@ mod tests { ) } + #[test] + fn disable_flag_is_unset_when_variable_is_absent() { + assert!(!evaluate_disable_flag("DISABLE_X", None)); + } + + #[test] + fn disable_flag_is_set_for_a_truthy_value() { + assert!(evaluate_disable_flag("DISABLE_X", Some("1".to_string()))); + } + + /// The whole point of the helper: falsy-looking values still disable, so a + /// bug fix can never silently re-enable destructive cleanup. + #[test] + fn falsy_looking_values_still_disable() { + for value in ["0", "false", "no", "off", "", " FALSE "] { + assert!( + evaluate_disable_flag("DISABLE_X", Some(value.to_string())), + "{value:?} should still disable" + ); + } + } + #[test] fn returns_legacy_value_when_only_legacy_is_set() { assert_eq!( diff --git a/crates/workspace-manager/src/workspace_manager.rs b/crates/workspace-manager/src/workspace_manager.rs index 6545982116..d1534e60b8 100644 --- a/crates/workspace-manager/src/workspace_manager.rs +++ b/crates/workspace-manager/src/workspace_manager.rs @@ -537,7 +537,7 @@ impl WorkspaceManager { } pub async fn cleanup_orphan_workspaces(&self) { - if std::env::var("DISABLE_WORKTREE_CLEANUP").is_ok() { + if utils::env::disable_flag_set("DISABLE_WORKTREE_CLEANUP") { info!( "Orphan workspace cleanup is disabled via DISABLE_WORKTREE_CLEANUP environment variable" );