mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Reuse the TUI startup account response during bootstrap (#38649)
## Why The TUI reads the account to determine login status, then bootstrap reads the same account again during startup. ## What changed - Preserve the login-status account response and pass it to app-server bootstrap, avoiding the second account request. - Discard the prefetched response when onboarding or a resume-directory prompt allows authentication to change, when a picker replaces the app-server session, or when the model provider changes. ## Testing - Verify bootstrap reuses a prefetched account without issuing another account request and retains its account metadata. - Verify normal bootstrap still reads the account when none was prefetched. - Verify matching resume directories skip the interactive prompt. GitOrigin-RevId: 8da1b9104b9e7f048c4254399b126116798d641a
This commit is contained in:
@@ -976,7 +976,8 @@ impl App {
|
||||
));
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
Ok(crate::session_resume::ResolveCwdOutcome::Continue(Some(cwd))) => cwd,
|
||||
Ok(crate::session_resume::ResolveCwdOutcome::Continue(Some(cwd)))
|
||||
| Ok(crate::session_resume::ResolveCwdOutcome::ContinueAfterPrompt(cwd)) => cwd,
|
||||
Ok(crate::session_resume::ResolveCwdOutcome::Continue(None)) => current_cwd.clone(),
|
||||
Ok(crate::session_resume::ResolveCwdOutcome::Exit) => {
|
||||
return Ok(AppRunControl::Exit(ExitReason::UserRequested));
|
||||
|
||||
@@ -385,6 +385,20 @@ impl AppServerSession {
|
||||
pub(crate) async fn bootstrap(&mut self, config: &Config) -> Result<AppServerBootstrap> {
|
||||
let started_at = Instant::now();
|
||||
let account = self.read_account().await?;
|
||||
let mut bootstrap = self.bootstrap_with_account(config, account).await?;
|
||||
bootstrap.duration = started_at.elapsed();
|
||||
Ok(bootstrap)
|
||||
}
|
||||
|
||||
/// Bootstraps using a previously read account.
|
||||
///
|
||||
/// Callers must discard a prefetched account after authentication, server, or provider changes.
|
||||
pub(crate) async fn bootstrap_with_account(
|
||||
&mut self,
|
||||
config: &Config,
|
||||
account: GetAccountResponse,
|
||||
) -> Result<AppServerBootstrap> {
|
||||
let started_at = Instant::now();
|
||||
// `hooks/list` holds the global config queue during startup. Submit models and config
|
||||
// requirements together so an uncached model fetch can overlap both config requests.
|
||||
let model_request_id = self.next_request_id();
|
||||
@@ -2077,6 +2091,58 @@ mod tests {
|
||||
.expect("config should build")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn bootstrap_reuses_prefetched_account_without_another_account_read() -> Result<()> {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let config = build_config(&codex_home).await;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
let next_request_id = app_server.next_request_id;
|
||||
let account = GetAccountResponse {
|
||||
account: Some(Account::Chatgpt {
|
||||
email: Some("teammate@openai.com".to_string()),
|
||||
plan_type: codex_protocol::account::PlanType::Plus,
|
||||
}),
|
||||
requires_openai_auth: true,
|
||||
};
|
||||
|
||||
let bootstrap = app_server.bootstrap_with_account(&config, account).await?;
|
||||
|
||||
assert_eq!(app_server.next_request_id, next_request_id + 2);
|
||||
assert_eq!(
|
||||
(
|
||||
bootstrap.account_email.as_deref(),
|
||||
bootstrap.auth_mode,
|
||||
bootstrap.plan_type,
|
||||
bootstrap.feedback_audience,
|
||||
bootstrap.has_chatgpt_account,
|
||||
),
|
||||
(
|
||||
Some("teammate@openai.com"),
|
||||
Some(TelemetryAuthMode::Chatgpt),
|
||||
Some(codex_protocol::account::PlanType::Plus),
|
||||
FeedbackAudience::OpenAiEmployee,
|
||||
true,
|
||||
)
|
||||
);
|
||||
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn bootstrap_reads_account_when_no_prefetched_account_is_available() -> Result<()> {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let config = build_config(&codex_home).await;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
let next_request_id = app_server.next_request_id;
|
||||
|
||||
app_server.bootstrap(&config).await?;
|
||||
|
||||
assert_eq!(app_server.next_request_id, next_request_id + 3);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn rate_limit_snapshot(limit_id: &str) -> RateLimitSnapshot {
|
||||
RateLimitSnapshot {
|
||||
limit_id: Some(limit_id.to_string()),
|
||||
|
||||
@@ -34,6 +34,7 @@ pub use codex_app_server_client::RemoteAppServerEndpoint;
|
||||
use codex_app_server_protocol::Account as AppServerAccount;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ConfigWarningNotification;
|
||||
use codex_app_server_protocol::GetAccountResponse;
|
||||
use codex_app_server_protocol::Thread as AppServerThread;
|
||||
use codex_app_server_protocol::ThreadListCwdFilter;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
@@ -1077,20 +1078,18 @@ async fn run_ratatui_app(
|
||||
!uses_remote_workspace && should_show_trust_screen(&initial_config);
|
||||
#[cfg(target_os = "windows")]
|
||||
let mut trust_decision_was_made = false;
|
||||
let login_status = if workload_identity_selected {
|
||||
LoginStatus::AuthMode(AuthMode::Chatgpt)
|
||||
let startup_model_provider = initial_config.model_provider_id.clone();
|
||||
let (login_status, mut startup_account) = if workload_identity_selected {
|
||||
(LoginStatus::AuthMode(AuthMode::Chatgpt), None)
|
||||
} else if initial_config.model_provider.requires_openai_auth {
|
||||
let Some(active_app_server) = app_server.as_mut() else {
|
||||
unreachable!("app server should exist when auth is required");
|
||||
};
|
||||
let login_status = startup_draft
|
||||
.run_until(
|
||||
&mut tui,
|
||||
get_login_status(active_app_server, &initial_config),
|
||||
)
|
||||
.run_until(&mut tui, get_login_status(active_app_server))
|
||||
.await;
|
||||
match login_status {
|
||||
Ok(Ok(login_status)) => login_status,
|
||||
Ok(Ok((login_status, account))) => (login_status, Some(account)),
|
||||
Ok(Err(err)) => {
|
||||
shutdown_startup_session(app_server.take(), &mut terminal_restore_guard).await;
|
||||
return Err(err);
|
||||
@@ -1101,7 +1100,7 @@ async fn run_ratatui_app(
|
||||
}
|
||||
}
|
||||
} else {
|
||||
LoginStatus::NotAuthenticated
|
||||
(LoginStatus::NotAuthenticated, None)
|
||||
};
|
||||
let should_show_onboarding =
|
||||
should_show_onboarding(login_status, &initial_config, should_show_trust_screen_flag);
|
||||
@@ -1111,6 +1110,8 @@ async fn run_ratatui_app(
|
||||
shutdown_startup_session(app_server.take(), &mut terminal_restore_guard).await;
|
||||
return Err(err.into());
|
||||
}
|
||||
// Authentication can change while any interactive onboarding screen is open.
|
||||
startup_account = None;
|
||||
let show_login_screen = should_show_login_screen(login_status, &initial_config);
|
||||
let onboarding_result = run_onboarding_app(
|
||||
OnboardingScreenArgs {
|
||||
@@ -1434,6 +1435,11 @@ async fn run_ratatui_app(
|
||||
.await
|
||||
{
|
||||
Ok(ResolveCwdOutcome::Continue(cwd)) => cwd,
|
||||
Ok(ResolveCwdOutcome::ContinueAfterPrompt(cwd)) => {
|
||||
// Another daemon client can change authentication while this prompt is open.
|
||||
startup_account = None;
|
||||
Some(cwd)
|
||||
}
|
||||
Ok(ResolveCwdOutcome::Exit) => {
|
||||
terminal_restore_guard.restore_silently();
|
||||
session_log::log_session_end();
|
||||
@@ -1568,6 +1574,8 @@ async fn run_ratatui_app(
|
||||
.await
|
||||
{
|
||||
Ok(Ok(app_server)) => {
|
||||
// A picker can replace the server; account reads belong to their original session.
|
||||
startup_account = None;
|
||||
AppServerSession::new(app_server, app_server_target.thread_params_mode())
|
||||
.with_startup_config(&config)
|
||||
.with_remote_cwd_override(remote_cwd_override.clone())
|
||||
@@ -1595,11 +1603,19 @@ async fn run_ratatui_app(
|
||||
let bypass_hook_trust_for_startup_review = config.bypass_hook_trust && !is_persistent_resume;
|
||||
let hooks_request_handle = app_server.request_handle();
|
||||
let hooks_cwd = config.cwd.to_path_buf();
|
||||
if config.model_provider_id != startup_model_provider {
|
||||
startup_account = None;
|
||||
}
|
||||
let startup_prefetch_started_at = Instant::now();
|
||||
let startup_prefetch = startup_draft
|
||||
.run_until(&mut tui, async {
|
||||
tokio::join!(
|
||||
app_server.bootstrap(&config),
|
||||
async {
|
||||
match startup_account {
|
||||
Some(account) => app_server.bootstrap_with_account(&config, account).await,
|
||||
None => app_server.bootstrap(&config).await,
|
||||
}
|
||||
},
|
||||
load_startup_hooks_review_entry(hooks_request_handle, hooks_cwd),
|
||||
)
|
||||
})
|
||||
@@ -1737,24 +1753,17 @@ pub enum LoginStatus {
|
||||
NotAuthenticated,
|
||||
}
|
||||
|
||||
/// Determines the user's authentication mode using a lightweight account read
|
||||
/// rather than a full `bootstrap`, avoiding the model-list fetch and
|
||||
/// rate-limit round-trip that `bootstrap` would trigger.
|
||||
/// Reads the account once to determine login status and preserve the response for bootstrap.
|
||||
async fn get_login_status(
|
||||
app_server: &mut AppServerSession,
|
||||
config: &Config,
|
||||
) -> color_eyre::Result<LoginStatus> {
|
||||
if !config.model_provider.requires_openai_auth {
|
||||
return Ok(LoginStatus::NotAuthenticated);
|
||||
}
|
||||
|
||||
) -> color_eyre::Result<(LoginStatus, GetAccountResponse)> {
|
||||
let account = app_server.read_account().await?;
|
||||
Ok(match account.account {
|
||||
let login_status = match &account.account {
|
||||
Some(AppServerAccount::ApiKey {}) => LoginStatus::AuthMode(AuthMode::ApiKey),
|
||||
Some(AppServerAccount::Chatgpt { .. }) => LoginStatus::AuthMode(AuthMode::Chatgpt),
|
||||
Some(AppServerAccount::AmazonBedrock { .. }) => LoginStatus::NotAuthenticated,
|
||||
None => LoginStatus::NotAuthenticated,
|
||||
})
|
||||
Some(AppServerAccount::AmazonBedrock { .. }) | None => LoginStatus::NotAuthenticated,
|
||||
};
|
||||
Ok((login_status, account))
|
||||
}
|
||||
|
||||
async fn load_config_or_exit(
|
||||
@@ -2149,6 +2158,9 @@ mod tests {
|
||||
.await?
|
||||
{
|
||||
ResolveCwdOutcome::Continue(cwd) => cwd,
|
||||
ResolveCwdOutcome::ContinueAfterPrompt(_) => {
|
||||
panic!("configured cwd should not prompt during startup")
|
||||
}
|
||||
ResolveCwdOutcome::Exit => panic!("configured cwd should not exit startup"),
|
||||
};
|
||||
let final_config = ConfigBuilder::default()
|
||||
|
||||
@@ -51,6 +51,8 @@ struct RawRecord {
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(crate) enum ResolveCwdOutcome {
|
||||
Continue(Option<PathBuf>),
|
||||
/// An interactive prompt was shown, so previously cached authentication may be stale.
|
||||
ContinueAfterPrompt(PathBuf),
|
||||
Exit,
|
||||
}
|
||||
|
||||
@@ -149,7 +151,7 @@ pub(crate) async fn resolve_cwd_for_resume_or_fork(
|
||||
)
|
||||
.await?;
|
||||
return Ok(match selection_outcome {
|
||||
CwdPromptOutcome::Selection(selection) => ResolveCwdOutcome::Continue(Some(
|
||||
CwdPromptOutcome::Selection(selection) => ResolveCwdOutcome::ContinueAfterPrompt(
|
||||
selection
|
||||
.selected_cwd(
|
||||
cwd_context.current_cwd,
|
||||
@@ -157,7 +159,7 @@ pub(crate) async fn resolve_cwd_for_resume_or_fork(
|
||||
cwd_context.remembered_current_cwd,
|
||||
)
|
||||
.to_path_buf(),
|
||||
)),
|
||||
),
|
||||
CwdPromptOutcome::Exit => ResolveCwdOutcome::Exit,
|
||||
});
|
||||
}
|
||||
@@ -519,6 +521,54 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn matching_resume_cwd_skips_prompt_without_configured_mode() -> color_eyre::Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let thread_id = ThreadId::new();
|
||||
let rollout_path = temp_dir.path().join("rollout.jsonl");
|
||||
let config = crate::legacy_core::config::ConfigBuilder::default()
|
||||
.codex_home(temp_dir.path().to_path_buf())
|
||||
.build()
|
||||
.await?;
|
||||
let current_cwd = config.cwd.to_path_buf();
|
||||
write_rollout_lines(
|
||||
&rollout_path,
|
||||
&[rollout_line(
|
||||
"t0",
|
||||
"session_meta",
|
||||
serde_json::json!({
|
||||
"id": thread_id,
|
||||
"cwd": current_cwd,
|
||||
"originator": "test",
|
||||
"cli_version": "test",
|
||||
}),
|
||||
)],
|
||||
)?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
|
||||
let outcome = resolve_cwd_for_resume_or_fork(
|
||||
&mut tui,
|
||||
&config,
|
||||
/*state_db_ctx*/ None,
|
||||
&SessionTarget {
|
||||
path: Some(rollout_path),
|
||||
thread_id,
|
||||
history_mode: None,
|
||||
},
|
||||
CwdPromptAction::Resume,
|
||||
ResumeCwdContext {
|
||||
current_cwd: ¤t_cwd,
|
||||
remembered_current_cwd: ¤t_cwd,
|
||||
allow_remember_current: true,
|
||||
mode: None,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(outcome, ResolveCwdOutcome::Continue(Some(current_cwd)));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_session_cwd_rejects_missing_metadata() -> color_eyre::Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user