clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 14:45:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): configure syntax theme from final config, not initial

Browse Source 
`set_theme_override` was called with `initial_config.tui_theme` before
onboarding/resume/fork could reload config. Because the theme state
lives in a `OnceLock`, it was permanently locked to the initial value.

Move the call to after the last possible config reload so the theme
and its warning always reflect the final session config.
This commit is contained in:
Felipe Coury
2026-02-09 01:10:48 -03:00
parent d32e05d4e5
commit 976dd6831e
2 changed files with 39 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
codex-rs/tui/src/lib.rs
View File

@@ -441,15 +441,6 @@ async fn run_ratatui_app(
) -> color_eyre::Result<AppExitInfo> {
color_eyre::install()?;
// Configure syntax highlighting theme before any rendering can occur.
// Surface resolution failures as a startup warning (⚠ banner in chat).
// Keep the warning separate so we can re-inject it after onboarding
// reloads config (which would otherwise discard it).
let theme_warning = crate::render::highlight::set_theme_override(
initial_config.tui_theme.clone(),
find_codex_home().ok(),
);
tooltips::announcement::prewarm();
// Forward panic reports through tracing so they appear in the UI status
@@ -716,10 +707,13 @@ async fn run_ratatui_app(
_ => config,
};
// Inject theme warning after the last possible config reload (onboarding
// at line ~520 and resume/fork just above can both replace the config,
// discarding any previously pushed warnings).
if let Some(w) = theme_warning {
// Configure syntax highlighting theme from the final config — onboarding
// and resume/fork can both reload config with a different tui_theme, so
// this must happen after the last possible reload.
if let Some(w) = crate::render::highlight::set_theme_override(
config.tui_theme.clone(),
find_codex_home().ok(),
) {
config.startup_warnings.push(w);
}
@@ -1204,43 +1198,48 @@ trust_level = "untrusted"
Ok(())
}
/// Regression: theme warning must survive config reloads.
/// Regression: theme must be configured from the *final* config.
///
/// `run_ratatui_app` reloads config twice — once during onboarding and
/// once on session resume/fork. Each reload produces a fresh `Config`
/// with an empty `startup_warnings`. The theme warning (captured before
/// any reload) must be injected *after the last reload* so it is never
/// discarded. This test exercises that exact pattern.
/// `run_ratatui_app` can reload config during onboarding and again
/// during session resume/fork. The syntax theme override (stored in
/// a `OnceLock`) must use the final config's `tui_theme`, not the
/// initial one — otherwise users resuming a thread in a project with
/// a different theme get the wrong highlighting.
///
/// We verify the invariant indirectly: `validate_theme_name` (the
/// pure validation core of `set_theme_override`) must be called with
/// the *final* config's theme, and its warning must land in the
/// final config's `startup_warnings`.
#[tokio::test]
async fn theme_warning_survives_config_reload() -> std::io::Result<()> {
async fn theme_warning_uses_final_config() -> std::io::Result<()> {
use crate::render::highlight::validate_theme_name;
let temp_dir = TempDir::new()?;
// Capture a theme warning before any config reload (mirrors the real
// call to `set_theme_override` at the top of `run_ratatui_app`).
let theme_warning: Option<String> = Some("unknown syntax theme \"bogus\"".into());
// initial_config has a valid theme — no warning.
let initial_config = build_config(&temp_dir).await?;
assert!(initial_config.tui_theme.is_none());
// First config — stands in for `initial_config` after onboarding.
// Simulate resume/fork reload: the final config has an invalid theme.
let mut config = build_config(&temp_dir).await?;
config.startup_warnings.push("other warning".into());
config.tui_theme = Some("bogus-theme".into());
// Simulate the resume/fork config reload: a fresh config with no
// startup_warnings (just like `load_config_or_exit_with_fallback_cwd`
// returns).
let mut config = build_config(&temp_dir).await?;
assert!(
config.startup_warnings.is_empty(),
"freshly loaded config should have no warnings"
);
// Re-inject after the last reload — the pattern used in run_ratatui_app.
if let Some(w) = theme_warning {
// Theme override must use the final config (not initial_config).
// This mirrors the real call site in run_ratatui_app.
if let Some(w) =
validate_theme_name(config.tui_theme.as_deref(), Some(temp_dir.path()))
{
config.startup_warnings.push(w);
}
assert_eq!(config.startup_warnings.len(), 1);
assert_eq!(
config.startup_warnings.len(),
1,
"warning from final config's invalid theme should be present"
);
assert!(
config.startup_warnings[0].contains("bogus"),
"theme warning should be present after config reload"
config.startup_warnings[0].contains("bogus-theme"),
"warning should reference the final config's theme name"
);
Ok(())
}

2
codex-rs/tui/src/render/highlight.rs
View File

@@ -46,7 +46,7 @@ pub(crate) fn set_theme_override(
/// Check whether a theme name resolves to a bundled theme or a custom
/// `.tmTheme` file. Returns a user-facing warning when it does not.
fn validate_theme_name(name: Option<&str>, codex_home: Option<&Path>) -> Option<String> {
pub(crate) fn validate_theme_name(name: Option<&str>, codex_home: Option<&Path>) -> Option<String> {
let name = name?;
// Bundled themes always resolve.
if parse_theme_name(name).is_some() {