fix(cli): avoid default env overrides for profiles#2119
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the build_tui_command function to prevent the CLI from unconditionally overriding TUI environment variables with global defaults, specifically for the provider, model, and base URL. This change ensures that profile-specific configurations are respected unless explicitly overridden by CLI flags. New tests have been added to verify these isolation improvements. A review comment suggests that similar conditional logic should be applied to DEEPSEEK_API_KEY and DEEPSEEK_HTTP_HEADERS to ensure full profile isolation and consistency across all runtime options.
45c7d7d to
4f5a350
Compare
|
Updated in What changed:
Kept keyring bridging intentionally, because the TUI does not re-query platform credential stores on normal config load and still needs the dispatcher-recovered secret handoff. Verification rerun:
|
4f5a350 to
d8a2f84
Compare
d2fd9b0 to
2da889e
Compare
Summary
Closes #2114
Verification
Greptile Summary
This PR fixes a bug where the CLI dispatcher-resolved defaults (model, base_url, provider, auth_mode, http_headers) were unconditionally exported as environment variables when spawning the TUI, causing them to override profile-specific settings loaded by the TUI itself. The fix narrows handoff to only explicit CLI flags and keyring-sourced secrets.
Confidence Score: 4/5
Safe to merge for the profile-override regression, but the explicit --api-key path drops provider-specific key vars for most providers.
The core fix is correct and well-tested. However, the new cli.api_key block manually enumerates only Openai, Atlascloud, and WanjieArk (partially) instead of using provider_env_vars(). Anyone who passes --provider moonshot --api-key or any of the other eight supported providers will have their provider-specific env var missing in the TUI process, which is a regression from the previous code that used provider_env_vars() for all sources.
crates/cli/src/lib.rs — specifically the cli.api_key forwarding block (lines 1512–1524)
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant CLI as codewhale CLI participant Dispatcher as Config Dispatcher participant TUI as codewhale TUI User->>CLI: deepseek --profile google CLI->>Dispatcher: resolve runtime options Dispatcher-->>CLI: ResolvedRuntimeOptions(provider, model, base_url, auth_mode, api_key) Note over CLI: OLD: forwarded all resolved values as env overrides Note over CLI: NEW: only forwards explicit CLI flags alt explicit --provider flag CLI->>TUI: DEEPSEEK_PROVIDER injected end alt explicit --model flag CLI->>TUI: DEEPSEEK_MODEL injected end alt explicit --base-url flag CLI->>TUI: DEEPSEEK_BASE_URL injected end alt api_key_source is Keyring CLI->>TUI: "API key env vars + source=keyring injected" end alt explicit --api-key flag CLI->>TUI: "API key env vars + source=cli injected" end CLI->>TUI: --profile google passed as arg TUI->>TUI: reads profile config for model, base_url, auth_modeComments Outside Diff (1)
crates/cli/src/lib.rs, line 1512-1524 (link)provider_env_vars()in the--api-keyCLI path leaves provider-specific keys unset for the TUI. Providers like Moonshot (MOONSHOT_API_KEY,KIMI_API_KEY), NvidiaNim (NVIDIA_API_KEY,NVIDIA_NIM_API_KEY), Openrouter, Novita, Fireworks, SGLang, vLLM, and Ollama are all skipped — onlyDEEPSEEK_API_KEYis forwarded. Even the WanjieArk case is incomplete:provider_env_varsreturns three vars (WANJIE_ARK_API_KEY,WANJIE_API_KEY,WANJIE_MAAS_API_KEY) but only the first is set. The keyring path usesprovider_env_vars()correctly; this path should match it.Reviews (3): Last reviewed commit: "test(cli): restore prompt flag coverage" | Re-trigger Greptile