mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Show onboarding when Codex home lacks authentication state (#38644)
## Why Routine state such as history, logs, sessions, or temporary files does not mean that the default account can authenticate. Treating any non-pristine Codex home as configured can show the composer before onboarding is complete. ## What changed - Base the startup decision on authentication-relevant state instead of requiring an empty Codex home. Existing credentials, configuration, workload identity, managed configuration, or a running local daemon continue to keep the composer visible. - Treat an unreadable or ambiguous home conservatively and keep the composer visible. - Allow onboarding when the legacy `--search` flag is the only configuration override. ## Testing Expanded startup preflight and draft tests to cover existing home state, credential sources, workload identity markers, daemon state, ambiguous paths, and search-only overrides. GitOrigin-RevId: 76837e3fd6aa714f2a7f72d191088a3faae23515
This commit is contained in:
@@ -518,7 +518,27 @@ async fn startup_draft_waits_for_onboarding_before_accepting_input() {
|
||||
]
|
||||
.into_iter(),
|
||||
);
|
||||
pump.initial_screen = StartupDraftInitialScreen::Onboarding;
|
||||
let codex_home = tempfile::tempdir().expect("create an existing custom Codex home");
|
||||
std::fs::write(codex_home.path().join("history.jsonl"), "")
|
||||
.expect("create existing startup history");
|
||||
let system_config_path = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(
|
||||
codex_home.path().join("system.toml"),
|
||||
)
|
||||
.expect("resolve missing system configuration");
|
||||
let search_override = [("web_search".to_string(), toml::Value::String("live".into()))];
|
||||
pump.initial_screen =
|
||||
if crate::startup_preflight::has_only_search_config_override(&search_override)
|
||||
&& crate::startup_preflight::should_delay_startup_composer_for_first_login(
|
||||
codex_home.path(),
|
||||
Ok(system_config_path),
|
||||
|| Ok(false),
|
||||
|name| (name == "CODEX_HOME").then(|| codex_home.path().as_os_str().to_os_string()),
|
||||
)
|
||||
{
|
||||
StartupDraftInitialScreen::Onboarding
|
||||
} else {
|
||||
StartupDraftInitialScreen::Composer
|
||||
};
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("create test terminal");
|
||||
pump.show_initial_screen(&mut tui)
|
||||
.expect("keep the composer hidden until onboarding finishes");
|
||||
|
||||
@@ -142,17 +142,23 @@ pub(super) async fn run_main_inner(
|
||||
strict_config,
|
||||
cli.bypass_hook_trust,
|
||||
);
|
||||
let search_only_config_override = !workload_identity_selected
|
||||
&& cli.web_search
|
||||
&& startup_preflight::has_only_search_config_override(&cli_kv_overrides)
|
||||
&& loader_overrides_are_default(&launch_loader_overrides)
|
||||
&& !strict_config
|
||||
&& !cli.bypass_hook_trust;
|
||||
let initial_screen = if cli.resume_picker || cli.fork_picker {
|
||||
startup_draft::StartupDraftInitialScreen::SessionPicker
|
||||
} else if !cli.oss
|
||||
&& explicit_remote_endpoint.is_none()
|
||||
&& reuse_implicit_local_daemon
|
||||
&& (reuse_implicit_local_daemon || search_only_config_override)
|
||||
&& launch_loader_overrides.packaged_defaults_path.is_none()
|
||||
&& startup_preflight::should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
codex_config::loader::system_config_toml_file(),
|
||||
|| codex_config::loader::has_local_managed_configuration(&codex_home),
|
||||
|name| std::env::var(name).ok(),
|
||||
|name| std::env::var_os(name),
|
||||
)
|
||||
{
|
||||
startup_draft::StartupDraftInitialScreen::Onboarding
|
||||
|
||||
@@ -1,22 +1,36 @@
|
||||
//! Conservative first-install checks that run before the provisional composer appears.
|
||||
//! Conservative authentication checks that run before the provisional composer appears.
|
||||
//!
|
||||
//! Any existing user, system, daemon, or authentication state keeps the composer visible.
|
||||
//! Existing configuration, daemon, or authentication state keeps the composer visible.
|
||||
|
||||
use std::ffi::OsString;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR;
|
||||
use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
/// Hide the composer only on a first installation without user or machine-wide configuration.
|
||||
/// Recognize the single configuration override synthesized by the legacy `--search` flag.
|
||||
pub(super) fn has_only_search_config_override(cli_kv_overrides: &[(String, toml::Value)]) -> bool {
|
||||
matches!(
|
||||
cli_kv_overrides,
|
||||
[(key, toml::Value::String(value))] if key == "web_search" && value == "live"
|
||||
)
|
||||
}
|
||||
|
||||
/// Hide the composer when the default file-backed account cannot already be authenticated.
|
||||
pub(super) fn should_delay_startup_composer_for_first_login(
|
||||
codex_home: &Path,
|
||||
system_config_path: io::Result<AbsolutePathBuf>,
|
||||
managed_configuration: impl FnOnce() -> io::Result<bool>,
|
||||
environment_variable: impl Fn(&str) -> Option<String>,
|
||||
environment_variable: impl Fn(&str) -> Option<OsString>,
|
||||
) -> bool {
|
||||
if environment_variable("CODEX_HOME").is_some_and(|value| !value.is_empty())
|
||||
|| environment_variable(codex_login::CODEX_ACCESS_TOKEN_ENV_VAR)
|
||||
.is_some_and(|credential| !credential.trim().is_empty())
|
||||
if environment_variable(codex_login::CODEX_ACCESS_TOKEN_ENV_VAR).is_some_and(|credential| {
|
||||
credential
|
||||
.to_str()
|
||||
.is_some_and(|value| !value.trim().is_empty())
|
||||
}) || environment_variable(OPENAI_FEDERATION_RULE_ID_ENV_VAR).is_some()
|
||||
|| environment_variable(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR).is_some()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
@@ -28,40 +42,31 @@ pub(super) fn should_delay_startup_composer_for_first_login(
|
||||
return false;
|
||||
}
|
||||
|
||||
let pristine_home = match codex_home.try_exists() {
|
||||
Ok(false) => true,
|
||||
Err(_) => false,
|
||||
Ok(true) => {
|
||||
let Ok(mut entries) = std::fs::read_dir(codex_home) else {
|
||||
return false;
|
||||
};
|
||||
let Some(Ok(temporary_root)) = entries.next() else {
|
||||
return false;
|
||||
};
|
||||
if temporary_root.file_name() != "tmp"
|
||||
|| !temporary_root
|
||||
.file_type()
|
||||
.is_ok_and(|file_type| file_type.is_dir())
|
||||
|| entries.next().is_some()
|
||||
{
|
||||
return false;
|
||||
match std::fs::metadata(codex_home) {
|
||||
Ok(metadata) if !metadata.is_dir() => return false,
|
||||
Ok(_) => {
|
||||
for state_file in ["auth.json", "config.toml", "environments.toml"] {
|
||||
if !matches!(codex_home.join(state_file).try_exists(), Ok(false)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
let Ok(mut entries) = std::fs::read_dir(temporary_root.path()) else {
|
||||
let Ok(daemon_socket) =
|
||||
codex_app_server_client::app_server_control_socket_path(codex_home)
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
let Some(Ok(arg0_root)) = entries.next() else {
|
||||
if !matches!(daemon_socket.as_path().try_exists(), Ok(false)) {
|
||||
return false;
|
||||
};
|
||||
arg0_root.file_name() == "arg0"
|
||||
&& arg0_root
|
||||
.file_type()
|
||||
.is_ok_and(|file_type| file_type.is_dir())
|
||||
&& entries.next().is_none()
|
||||
}
|
||||
}
|
||||
};
|
||||
Err(error)
|
||||
if error.kind() == io::ErrorKind::NotFound
|
||||
&& matches!(codex_home.try_exists(), Ok(false)) => {}
|
||||
Err(_) => return false,
|
||||
}
|
||||
|
||||
pristine_home && matches!(managed_configuration(), Ok(false))
|
||||
matches!(managed_configuration(), Ok(false))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use tempfile::TempDir;
|
||||
|
||||
use super::has_only_search_config_override;
|
||||
use super::should_delay_startup_composer_for_first_login;
|
||||
|
||||
#[test]
|
||||
fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<()> {
|
||||
fn startup_delays_composer_for_homes_without_authentication_state() -> std::io::Result<()> {
|
||||
let temporary_directory = TempDir::new()?;
|
||||
let codex_home = temporary_directory.path().join("codex-home");
|
||||
let system_config_path =
|
||||
@@ -20,7 +21,7 @@ fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|name| (name == codex_login::CODEX_ACCESS_TOKEN_ENV_VAR).then(|| " ".to_string()),
|
||||
|name| (name == codex_login::CODEX_ACCESS_TOKEN_ENV_VAR).then(|| " ".into()),
|
||||
));
|
||||
|
||||
std::fs::create_dir_all(codex_home.join("tmp").join("arg0"))?;
|
||||
@@ -43,19 +44,17 @@ fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<
|
||||
|_| None,
|
||||
));
|
||||
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|name| (name == "CODEX_HOME").then(|| "/custom/home".to_string()),
|
||||
|name| (name == "CODEX_HOME").then(|| "/custom/home".into()),
|
||||
));
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|name| {
|
||||
(name == codex_login::CODEX_ACCESS_TOKEN_ENV_VAR).then(|| "access-token".to_string())
|
||||
},
|
||||
|name| { (name == codex_login::CODEX_ACCESS_TOKEN_ENV_VAR).then(|| "access-token".into()) },
|
||||
));
|
||||
for disabled_credential in [
|
||||
codex_login::OPENAI_API_KEY_ENV_VAR,
|
||||
@@ -65,17 +64,48 @@ fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|name| (name == disabled_credential).then(|| "disabled-key".to_string()),
|
||||
|name| (name == disabled_credential).then(|| "disabled-key".into()),
|
||||
));
|
||||
}
|
||||
|
||||
for existing_state in ["auth.json", "config.toml", "history.jsonl", "sessions"] {
|
||||
for existing_state in ["history.jsonl", "log", "sessions"] {
|
||||
let state_path = codex_home.join(existing_state);
|
||||
if existing_state == "history.jsonl" {
|
||||
std::fs::write(&state_path, "")?;
|
||||
} else {
|
||||
std::fs::create_dir(&state_path)?;
|
||||
}
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|_| None,
|
||||
));
|
||||
}
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|name| (name == "CODEX_HOME").then(|| "/custom/home".into()),
|
||||
));
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| panic!("process credentials should not probe managed configuration"),
|
||||
|name| match name {
|
||||
"CODEX_HOME" => Some("/custom/home".into()),
|
||||
codex_login::CODEX_ACCESS_TOKEN_ENV_VAR => Some("access-token".into()),
|
||||
_ => None,
|
||||
},
|
||||
));
|
||||
|
||||
for existing_state in ["auth.json", "config.toml", "environments.toml"] {
|
||||
let state_path = codex_home.join(existing_state);
|
||||
std::fs::write(&state_path, "")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|| panic!("configured homes should not probe managed configuration"),
|
||||
|_| None,
|
||||
));
|
||||
std::fs::remove_file(state_path)?;
|
||||
@@ -83,11 +113,17 @@ fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<
|
||||
|
||||
let daemon_directory = codex_home.join("app-server-control");
|
||||
std::fs::create_dir(&daemon_directory)?;
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|_| None,
|
||||
));
|
||||
std::fs::write(daemon_directory.join("app-server-control.sock"), "")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| panic!("existing homes should not probe managed configuration"),
|
||||
|| panic!("daemon-owned homes should not probe managed configuration"),
|
||||
|_| None,
|
||||
));
|
||||
std::fs::remove_file(daemon_directory.join("app-server-control.sock"))?;
|
||||
@@ -95,7 +131,7 @@ fn startup_delays_composer_only_for_pristine_default_homes() -> std::io::Result<
|
||||
|
||||
let additional_temporary_state = codex_home.join("tmp").join("other");
|
||||
std::fs::write(&additional_temporary_state, "")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
@@ -121,23 +157,105 @@ fn startup_keeps_composer_when_home_state_cannot_be_confirmed() -> std::io::Resu
|
||||
AbsolutePathBuf::from_absolute_path(temporary_directory.path().join("system.toml"))?;
|
||||
|
||||
std::fs::create_dir(&codex_home)?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| Ok(false),
|
||||
|_| None,
|
||||
));
|
||||
|
||||
std::fs::write(codex_home.join("tmp"), "not a directory")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
assert!(should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path),
|
||||
|| Ok(false),
|
||||
|_| None,
|
||||
));
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
let system_config_path =
|
||||
AbsolutePathBuf::from_absolute_path(temporary_directory.path().join("system.toml"))?;
|
||||
let daemon_directory = codex_home.join("app-server-control");
|
||||
std::fs::write(&daemon_directory, "not a directory")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| panic!("ambiguous daemon state should not probe managed configuration"),
|
||||
|_| None,
|
||||
));
|
||||
std::fs::remove_file(&daemon_directory)?;
|
||||
|
||||
let blocking_parent = temporary_directory.path().join("blocking-home-parent");
|
||||
std::fs::write(&blocking_parent, "not a directory")?;
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&blocking_parent.join("codex-home"),
|
||||
Ok(system_config_path),
|
||||
|| panic!("ambiguous home state should not probe managed configuration"),
|
||||
|_| None,
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_keeps_composer_for_workload_identity_markers() -> std::io::Result<()> {
|
||||
let temporary_directory = TempDir::new()?;
|
||||
let codex_home = temporary_directory.path().join("codex-home");
|
||||
let system_config_path =
|
||||
AbsolutePathBuf::from_absolute_path(temporary_directory.path().join("system.toml"))?;
|
||||
|
||||
for marker in [
|
||||
codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR,
|
||||
codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR,
|
||||
] {
|
||||
for value in ["", "configured"] {
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path.clone()),
|
||||
|| panic!("workload identity should not probe managed configuration"),
|
||||
|name| (name == marker).then(|| value.into()),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
|
||||
assert!(!should_delay_startup_composer_for_first_login(
|
||||
&codex_home,
|
||||
Ok(system_config_path),
|
||||
|| panic!("workload identity should not probe managed configuration"),
|
||||
|name| {
|
||||
(name == codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR)
|
||||
.then(|| std::ffi::OsString::from_vec(vec![b'/', 0xff]))
|
||||
},
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_only_accepts_the_synthetic_search_override() {
|
||||
let search = (
|
||||
"web_search".to_string(),
|
||||
toml::Value::String("live".to_string()),
|
||||
);
|
||||
|
||||
assert!(!has_only_search_config_override(&[]));
|
||||
assert!(has_only_search_config_override(std::slice::from_ref(
|
||||
&search
|
||||
)));
|
||||
assert!(!has_only_search_config_override(&[(
|
||||
"web_search".to_string(),
|
||||
toml::Value::String("cached".to_string()),
|
||||
)]));
|
||||
assert!(!has_only_search_config_override(&[(
|
||||
"model_provider".to_string(),
|
||||
toml::Value::String("live".to_string()),
|
||||
)]));
|
||||
assert!(!has_only_search_config_override(&[
|
||||
search,
|
||||
(
|
||||
"model".to_string(),
|
||||
toml::Value::String("custom".to_string())
|
||||
),
|
||||
]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_keeps_composer_when_system_configuration_is_possible() -> std::io::Result<()> {
|
||||
let temporary_directory = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user