feat(cli): ask which provider on first run instead of pinning xAI - #577
Conversation
Silent bootstrap and run_setup always chose xAI (endpoint + OAuth) when no env key was present. Ask once interactively (or honour --provider / AGENT_CODE_PROVIDER), write that provider's defaults, and remove the dead multi-step legacy wizard with its invalid theme ids.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89b718b902
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| " {} SuperGrok / X Premium device sign-in…\n", | ||
| "→".dark_cyan().bold() | ||
| " {}", | ||
| "(Or paste a key into config.toml under [api] — env is preferred.)".dark_grey() |
There was a problem hiding this comment.
This newly tells first-run users to paste a credential into config.toml, directly violating the repository's security requirement that API keys remain in environment variables only. Remove this fallback and only direct users to the appropriate provider environment variable.
AGENTS.md reference: AGENTS.md:L108-L112
Useful? React with 👍 / 👎.
| let result = SetupResult { | ||
| api_key: String::new(), | ||
| auth_mode: "api_key".into(), |
There was a problem hiding this comment.
When a keyless first-run user chooses Ollama, this empty key is omitted from the saved config and run_setup_wizard does not seed AGENT_CODE_API_KEY; after reloading, main.rs lines 682–690 therefore returns “API key required” before constructing the local provider. The advertised Ollama option can never launch unless the user independently supplies a dummy key, so the API-key readiness check needs to recognize this keyless local configuration.
Useful? React with 👍 / 👎.
|
@codex review |
Stop telling users to paste API keys into config.toml. Persist the local "ollama" api_key placeholder so has_key passes after Ollama setup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89b718b902
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| theme: "auto".into(), | ||
| permission_mode: "accept_edits".into(), | ||
| }; | ||
| write_config(&result); |
There was a problem hiding this comment.
When an existing user config has no currently usable credential, selecting any API-key provider reaches this unconditional write, which replaces the entire config.toml with only the reconstructed API, permissions, and UI tables. This drops unrelated MCP, hook, and security settings; notably, it can remove security.disable_bypass_permissions, silently weakening later permission enforcement. Update the API fields while preserving existing tables instead of truncating the user config.
AGENTS.md reference: AGENTS.md:L95-L98
Useful? React with 👍 / 👎.
| let result = SetupResult { | ||
| api_key: String::new(), | ||
| auth_mode: "api_key".into(), |
There was a problem hiding this comment.
When the user selects Ollama with no environment key, this empty API key is omitted from the rendered config, but the caller reloads it with auth_mode = api_key and main.rs then returns API key required before constructing the local provider. Consequently, the newly advertised no-key Ollama option never reaches the TUI; preserve the local sentinel or exempt the local provider from the API-key gate.
Useful? React with 👍 / 👎.
| let pinned = cli_provider.trim(); | ||
| if !pinned.is_empty() && !pinned.eq_ignore_ascii_case("auto") { |
There was a problem hiding this comment.
When a script sets only AGENT_CODE_PROVIDER, cli_provider still arrives as auto: the Cli::provider argument in main.rs has no env = "AGENT_CODE_PROVIDER" binding and no other code reads that variable. The process therefore opens the interactive selector and may hang instead of using the promised pinned provider; bind the environment variable while retaining explicit --provider precedence.
AGENTS.md reference: AGENTS.md:L124-L129
Useful? React with 👍 / 👎.
1 participant
Summary
--provider/AGENT_CODE_PROVIDERpins the choice for scripts (autostill asks).run_setup_wizard_legacyand invalid theme ids (midnight,daybreak, …).Test plan
default_setup_does_not_pin_a_vendormap_cli_provider_aliases,api_key_defaults_cover_main_providers