chore: retire the DEEPSEEK_YOLO env alias (#5443) - #5841
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9d89ac5a-c02f-4d0f-97ef-4a3785660072) |
|
Claude finished @Hmbown's task in 1m 30s —— View job Review: retire the
|
There was a problem hiding this comment.
🟡 Changes recommended
The read-path deprecation warning mechanism described in #5443 isn’t implemented for DEEPSEEK_YOLO, and the new env-mutation tests don’t restore env vars on panic, risking cross-test contamination.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR retires the write path for the legacy DEEPSEEK_YOLO environment variable, ensuring --yolo only emits the canonical CODEWHALE_YOLO while keeping DEEPSEEK_YOLO readable as a fallback alias through 0.9.x. It also updates rebrand documentation and adds regression coverage for alias precedence.
Changes:
- Stop exporting
DEEPSEEK_YOLOfrom the CLI when--yolois used; export onlyCODEWHALE_YOLO. - Document
DEEPSEEK_YOLOas deprecated (EN/ID) and clarify precedence (CODEWHALE_YOLOwins when both are set). - Add tests in config/TUI/CLI layers to validate canonical-only writes and alias precedence.
File summaries
| File | Description |
|---|---|
| docs/REBRAND.md | Documents DEEPSEEK_YOLO deprecation and canonical CODEWHALE_YOLO usage (EN). |
| docs/id/REBRAND.md | Same deprecation/precedence documentation in Indonesian. |
| crates/tui/src/lib.rs | Updates inline launcher documentation to reference CODEWHALE_YOLO forwarding. |
| crates/tui/src/config/tests.rs | Adds TUI config test for yolo env alias precedence. |
| crates/tui/src/config.rs | Adds inline docs clarifying DEEPSEEK_YOLO as a deprecated alias read path. |
| crates/config/src/tests.rs | Adds config-layer test for yolo env alias precedence. |
| crates/config/src/lib.rs | Clarifies alias semantics in comments and narrows invalid-value warning wording. |
| crates/cli/src/lib.rs | Stops writing DEEPSEEK_YOLO; adds CLI regression test ensuring only canonical env is written. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // `DEEPSEEK_YOLO` is a read-only deprecated alias of | ||
| // `CODEWHALE_YOLO` so existing scripts keep working; when both are | ||
| // set `CODEWHALE_YOLO` wins. The alias is removed in 0.10 per | ||
| // issue #5443 — do not write it anywhere. | ||
| yolo: std::env::var("CODEWHALE_YOLO") | ||
| .or_else(|_| std::env::var("DEEPSEEK_YOLO")) |
| let codewhale_prev = env::var_os("CODEWHALE_YOLO"); | ||
| let deepseek_prev = env::var_os("DEEPSEEK_YOLO"); | ||
| let config = ConfigToml::default(); | ||
|
|
| // `DEEPSEEK_YOLO` is a read-only deprecated alias of `CODEWHALE_YOLO` | ||
| // (removable in 0.10 per issue #5443); `CODEWHALE_YOLO` wins when both | ||
| // are set. | ||
| if let Ok(value) = std::env::var("CODEWHALE_YOLO").or_else(|_| std::env::var("DEEPSEEK_YOLO")) { | ||
| config.yolo = Some(value == "1" || value.eq_ignore_ascii_case("true")); | ||
| } |
| let codewhale_prev = env::var_os("CODEWHALE_YOLO"); | ||
| let deepseek_prev = env::var_os("DEEPSEEK_YOLO"); | ||
| let mut config = Config::default(); | ||
|
|
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
PR stops emitting DEEPSEEK_YOLO when --yolo is used while preserving read fallback. Adds tests for canonical write and alias precedence. One TUI test scenario is not isolated and could pass even if the alias stopped working.
Findings
- [WARNING] TUI yolo alias test reuses mutated Config, second scenario not isolated (
crates/tui/src/config/tests.rs:1452)
The test creates oneconfigand mutates it with the firstapply_env_overrides, leavingconfig.yolo = Some(true). The second scenario removesCODEWHALE_YOLOand setsDEEPSEEK_YOLO, but if the alias were not read,apply_env_overrideswould not overwrite the existingconfig.yolo, so the assertion would still pass. This makes the test unable to catch regressions whereDEEPSEEK_YOLOis no longer honored. ResetconfigtoConfig::default()before the second scenario (or use a freshConfig).
Suggestions
-
crates/tui/src/config/tests.rs:1452— Reset config before testing the deprecated alias so the assertion does not pass because of state left by the previous case.// Reset config so this scenario doesn't inherit yolo=true from the previous case. config = Config::default(); // Only the deprecated alias is set: it must keep working through 0.9.x.
Assessment
PR is well scoped and the core change is correct. The tests largely cover the intended behavior, but the TUI alias-precedence test needs a config reset between scenarios to avoid a false positive.
Advisory review by Codewhale (codewhale review --pr 5841 --post, head 949a00e58ce6b9ed43c87a2d5d70c36737cc22a4). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| "CODEWHALE_YOLO=true must enable the yolo posture" | ||
| ); | ||
|
|
||
| // Only the deprecated alias is set: it must keep working through 0.9.x. |
There was a problem hiding this comment.
[WARNING] TUI yolo alias test reuses mutated Config, second scenario not isolated
The test creates one config and mutates it with the first apply_env_overrides, leaving config.yolo = Some(true). The second scenario removes CODEWHALE_YOLO and sets DEEPSEEK_YOLO, but if the alias were not read, apply_env_overrides would not overwrite the existing config.yolo, so the assertion would still pass. This makes the test unable to catch regressions where DEEPSEEK_YOLO is no longer honored. Reset config to Config::default() before the second scenario (or use a fresh Config).
| "CODEWHALE_YOLO=true must enable the yolo posture" | ||
| ); | ||
|
|
||
| // Only the deprecated alias is set: it must keep working through 0.9.x. |
There was a problem hiding this comment.
Reset config before testing the deprecated alias so the assertion does not pass because of state left by the previous case.
| // Only the deprecated alias is set: it must keep working through 0.9.x. | |
| // Reset config so this scenario doesn't inherit yolo=true from the previous case. | |
| config = Config::default(); | |
| // Only the deprecated alias is set: it must keep working through 0.9.x. |
Closes #5443. Single commit off current tip. Gates: fmt clean, dead-code budget PASS at 425. Full matrix via CI.
Note
Low Risk
Behavior change is limited to no longer writing a legacy env var; existing scripts that set DEEPSEEK_YOLO still work on read paths.
Overview
Stops emitting
DEEPSEEK_YOLOwhen--yolois used; the CLI now sets onlyCODEWHALE_YOLO. ReadingDEEPSEEK_YOLOas a fallback alias is unchanged through 0.9.x, withCODEWHALE_YOLOwinning when both are set; removal is planned for 0.10 (#5443).Adds regression tests in the CLI, config, and TUI layers for “write canonical only” and alias precedence. Config/TUI code gains inline docs; invalid-value warnings mention only
CODEWHALE_YOLO. REBRAND docs (EN/ID) document the deprecation.Reviewed by Cursor Bugbot for commit 949a00e. Bugbot is set up for automated code reviews on this repo. Configure here.