mirror of
https://github.com/openai/codex.git
synced 2026-11-12 06:40:35 +00:00
TUI config cleanup: oss_provider (#24254)
## Summary Manual provider selection during `codex --oss` startup was still persisting `oss_provider` through the legacy local `config.toml` writer. That bypasses the app-server-owned config mutation path used by the TUI, so this routes the write through the app server config API instead. The net behavior is intentionally narrow: only an interactive picker selection is persisted. Auto-detected single-running-provider startup and explicit `--local-provider` startup remain ephemeral, so merely having one backend running does not make that provider sticky for future runs. ## What Changed - Removed the TUI picker’s direct dependency on `set_default_oss_provider`. - Had `oss_selection` report whether the returned provider came from the interactive picker. - Carried only manually selected providers into startup persistence. - Wrote `oss_provider` via `config/batchWrite` once the app server session is available. - Logged a warning and continued startup if the app-server config write fails. ## Verification Manually smoke-tested the real `codex-tui` binary with a temporary `CODEX_HOME`, pseudo-terminal input, and a fake LM Studio HTTP server: - Interactive picker selection persisted `oss_provider = "lmstudio"`. - Non-picker `--local-provider lmstudio` startup did not persist `oss_provider`.
This commit is contained in:
1 parent
5fb5e47767
commit
f05fd0e661
3 files changed
+43
-14
No files matched your search
@@ -122,6 +122,10 @@ pub(crate) fn build_memory_settings_edits(
|
|||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn build_oss_provider_edit(provider: &str) -> ConfigEdit {
|
||||||
|
replace_config_value("oss_provider", serde_json::json!(provider))
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) async fn write_config_batch(
|
pub(crate) async fn write_config_batch(
|
||||||
request_handle: AppServerRequestHandle,
|
request_handle: AppServerRequestHandle,
|
||||||
edits: Vec<ConfigEdit>,
|
edits: Vec<ConfigEdit>,
|
||||||
|
|||||||
+21
-1
@@ -1002,6 +1002,7 @@ pub async fn run_main(
|
|||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
|
let mut manually_selected_oss_provider = None;
|
||||||
let model_provider_override = if cli.oss {
|
let model_provider_override = if cli.oss {
|
||||||
let resolved = resolve_oss_provider(cli.oss_provider.as_deref(), &config_toml);
|
let resolved = resolve_oss_provider(cli.oss_provider.as_deref(), &config_toml);
|
||||||
|
|
||||||
@@ -1009,12 +1010,16 @@ pub async fn run_main(
|
|||||||
Some(provider)
|
Some(provider)
|
||||||
} else {
|
} else {
|
||||||
// No provider configured, prompt the user
|
// No provider configured, prompt the user
|
||||||
let provider = oss_selection::select_oss_provider(&codex_home).await?;
|
let selection = oss_selection::select_oss_provider().await?;
|
||||||
|
let provider = selection.provider;
|
||||||
if provider == "__CANCELLED__" {
|
if provider == "__CANCELLED__" {
|
||||||
return Err(std::io::Error::other(
|
return Err(std::io::Error::other(
|
||||||
"OSS provider selection was cancelled by user",
|
"OSS provider selection was cancelled by user",
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
if selection.manually_selected {
|
||||||
|
manually_selected_oss_provider = Some(provider.clone());
|
||||||
|
}
|
||||||
Some(provider)
|
Some(provider)
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
@@ -1256,6 +1261,7 @@ pub async fn run_main(
|
|||||||
app_server_target,
|
app_server_target,
|
||||||
remote_cwd_override,
|
remote_cwd_override,
|
||||||
config,
|
config,
|
||||||
|
manually_selected_oss_provider,
|
||||||
overrides,
|
overrides,
|
||||||
cli_kv_overrides,
|
cli_kv_overrides,
|
||||||
cloud_requirements,
|
cloud_requirements,
|
||||||
@@ -1277,6 +1283,7 @@ async fn run_ratatui_app(
|
|||||||
app_server_target: AppServerTarget,
|
app_server_target: AppServerTarget,
|
||||||
remote_cwd_override: Option<PathBuf>,
|
remote_cwd_override: Option<PathBuf>,
|
||||||
initial_config: Config,
|
initial_config: Config,
|
||||||
|
manually_selected_oss_provider: Option<String>,
|
||||||
overrides: ConfigOverrides,
|
overrides: ConfigOverrides,
|
||||||
cli_kv_overrides: Vec<(String, toml::Value)>,
|
cli_kv_overrides: Vec<(String, toml::Value)>,
|
||||||
mut cloud_requirements: CloudRequirementsLoader,
|
mut cloud_requirements: CloudRequirementsLoader,
|
||||||
@@ -1356,6 +1363,19 @@ async fn run_ratatui_app(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
.with_remote_cwd_override(remote_cwd_override.clone());
|
.with_remote_cwd_override(remote_cwd_override.clone());
|
||||||
|
if let Some(provider) = manually_selected_oss_provider.as_deref()
|
||||||
|
&& let Err(err) = config_update::write_config_batch(
|
||||||
|
app_server_session.request_handle(),
|
||||||
|
vec![config_update::build_oss_provider_edit(provider)],
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
warn!(
|
||||||
|
%err,
|
||||||
|
provider,
|
||||||
|
"Failed to persist selected OSS provider preference"
|
||||||
|
);
|
||||||
|
}
|
||||||
let mut app_server = Some(app_server_session);
|
let mut app_server = Some(app_server_session);
|
||||||
|
|
||||||
let should_show_trust_screen_flag =
|
let should_show_trust_screen_flag =
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ use std::sync::LazyLock;
|
|||||||
use crate::key_hint;
|
use crate::key_hint;
|
||||||
use crate::key_hint::KeyBinding;
|
use crate::key_hint::KeyBinding;
|
||||||
use crate::key_hint::KeyBindingListExt;
|
use crate::key_hint::KeyBindingListExt;
|
||||||
use crate::legacy_core::config::set_default_oss_provider;
|
|
||||||
use codex_model_provider_info::DEFAULT_LMSTUDIO_PORT;
|
use codex_model_provider_info::DEFAULT_LMSTUDIO_PORT;
|
||||||
use codex_model_provider_info::DEFAULT_OLLAMA_PORT;
|
use codex_model_provider_info::DEFAULT_OLLAMA_PORT;
|
||||||
use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
|
use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
|
||||||
@@ -309,7 +308,12 @@ fn get_status_symbol_and_color(status: &ProviderStatus) -> (&'static str, Color)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub async fn select_oss_provider(codex_home: &std::path::Path) -> io::Result<String> {
|
pub(crate) struct OssProviderSelection {
|
||||||
|
pub(crate) provider: String,
|
||||||
|
pub(crate) manually_selected: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
pub async fn select_oss_provider() -> io::Result<OssProviderSelection> {
|
||||||
// Check provider statuses first
|
// Check provider statuses first
|
||||||
let lmstudio_status = check_lmstudio_status().await;
|
let lmstudio_status = check_lmstudio_status().await;
|
||||||
let ollama_status = check_ollama_status().await;
|
let ollama_status = check_ollama_status().await;
|
||||||
@@ -318,11 +322,17 @@ pub async fn select_oss_provider(codex_home: &std::path::Path) -> io::Result<Str
|
|||||||
match (&lmstudio_status, &ollama_status) {
|
match (&lmstudio_status, &ollama_status) {
|
||||||
(ProviderStatus::Running, ProviderStatus::NotRunning) => {
|
(ProviderStatus::Running, ProviderStatus::NotRunning) => {
|
||||||
let provider = LMSTUDIO_OSS_PROVIDER_ID.to_string();
|
let provider = LMSTUDIO_OSS_PROVIDER_ID.to_string();
|
||||||
return Ok(provider);
|
return Ok(OssProviderSelection {
|
||||||
|
provider,
|
||||||
|
manually_selected: false,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
(ProviderStatus::NotRunning, ProviderStatus::Running) => {
|
(ProviderStatus::NotRunning, ProviderStatus::Running) => {
|
||||||
let provider = OLLAMA_OSS_PROVIDER_ID.to_string();
|
let provider = OLLAMA_OSS_PROVIDER_ID.to_string();
|
||||||
return Ok(provider);
|
return Ok(OssProviderSelection {
|
||||||
|
provider,
|
||||||
|
manually_selected: false,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
_ => {
|
_ => {
|
||||||
// Both running or both not running - show UI
|
// Both running or both not running - show UI
|
||||||
@@ -346,21 +356,16 @@ pub async fn select_oss_provider(codex_home: &std::path::Path) -> io::Result<Str
|
|||||||
if let Event::Key(key_event) = event::read()?
|
if let Event::Key(key_event) = event::read()?
|
||||||
&& let Some(selection) = widget.handle_key_event(key_event)
|
&& let Some(selection) = widget.handle_key_event(key_event)
|
||||||
{
|
{
|
||||||
break Ok(selection);
|
break Ok(OssProviderSelection {
|
||||||
|
provider: selection,
|
||||||
|
manually_selected: true,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
disable_raw_mode()?;
|
disable_raw_mode()?;
|
||||||
execute!(terminal.backend_mut(), LeaveAlternateScreen)?;
|
execute!(terminal.backend_mut(), LeaveAlternateScreen)?;
|
||||||
|
|
||||||
// If the user manually selected an OSS provider, we save it as the
|
|
||||||
// default one to use later.
|
|
||||||
if let Ok(ref provider) = result
|
|
||||||
&& let Err(e) = set_default_oss_provider(codex_home, provider)
|
|
||||||
{
|
|
||||||
tracing::warn!("Failed to save OSS provider preference: {e}");
|
|
||||||
}
|
|
||||||
|
|
||||||
result
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user