fix(env): warn when a DISABLE_* flag is set to a falsy value - #48
Merged
Conversation
The `std::env::var(..).is_ok()` gates treat any value as "disable", so `DISABLE_WORKTREE_CLEANUP=0` still disables cleanup — the opposite of what setting it to 0 suggests. Route the four opt-out gates through a shared `utils::env::disable_flag_set` helper that keeps the existing permissive semantics and logs a warning when the value looks falsy (0/false/no/off/empty). The semantics are deliberately left unchanged. Two of these gates guard `cleanup_workspace()`, which kills CLI tmux sessions and deletes worktrees. Reinterpreting `DISABLE_WORKTREE_CLEANUP=0` as "enabled" would silently switch worktree deletion back on for anyone relying on the current behaviour, so the surprising value is reported rather than reinterpreted. `WSL_DISTRO_NAME`/`WSLENV` in utils::is_wsl2 are left alone: those are genuine presence checks, not opt-out flags.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The four
DISABLE_*opt-out gates usestd::env::var(..).is_ok(), which is true for any value. SoDISABLE_WORKTREE_CLEANUP=0still disables cleanup — the opposite of what setting it to0suggests. Same forfalse,no,off, and the empty string.Affected gates:
workspace_manager.rs:539DISABLE_WORKTREE_CLEANUPcontainer.rs:281DISABLE_WORKTREE_CLEANUPcontainer.rs:319DISABLE_CLI_SESSION_REAPloop_supervisor.rs:358DISABLE_LOOP_AUTOMATIONApproach: warn, don't reinterpret
The obvious fix is to parse booleans properly. This PR deliberately does not do that.
Two of these gates guard
cleanup_workspace(), which kills CLI tmux sessions, deletes worktrees, and marks them deleted in the DB. IfDISABLE_WORKTREE_CLEANUP=0started meaning "enabled", anyone currently relying on the permissive behaviour would have worktree deletion silently switched on by what looks like a routine bug fix. That is a bad thing to ship in a drive-by change.So the semantics are unchanged — any value still disables — and a falsy-looking value now produces a warning telling you to unset the variable instead. The confusion is fixed without any behavioural risk.
If you'd rather have strict boolean parsing, that's a reasonable call, but it should be a deliberate breaking change with a release note rather than folded in here.
Changes
utils::env::disable_flag_set(), following the existingnormalize_path_overridepattern in that module (pure inner fn for testability,tracing::warn!on the surprising case)0,false,no,off,""and" FALSE "all still disableWSL_DISTRO_NAME/WSLENVinutils::is_wsl2are left alone — those are genuine presence checks, not opt-out flags.Verification
cargo test -p utils --lib env— 12 passed (9 existing + 3 new)cargo check -p workspace-manager -p local-deployment— cleanProvenance
To be clear about authorship: this was written by Claude Code on the shared
ded02dev box at @dev1's request, while investigating a disk-space issue. The commit is attributed to Miguel Rasero because that is the git identity configured in that checkout and the only GitHub credential available on the box — not because Miguel wrote or reviewed it. Flagging so the history isn't misread.It came out of noticing that
DISABLE_WORKTREE_CLEANUP=1was set on the dev servers. Worth saying plainly: this fix would not have prevented that disk issue. The space was in active, registered worktrees that no cleanup path touches; the orphaned dirs totalled ~6 MB.🤖 Generated with Claude Code
https://claude.ai/code/session_01UsimweKP7yPhJFvCSGswBa