mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
tui: recover local state db startup failures (#22734)
## Why #22580 made app-server startup fail when the local SQLite state database cannot be initialized. Embedded/local TUI startup still continued on the permissive path, which left the CLI inconsistent and could hide a real startup problem behind unrelated UI. This brings local TUI startup onto the same fail-closed behavior while keeping recovery humane for the two failure modes we are seeing in practice: damaged database files and startup stalls caused by another process holding the database write lock. ## What changed - Embedded TUI startup now uses `state_db::try_init(...)` and returns a typed `LocalStateDbStartupError` that preserves the affected database path plus the underlying failure detail. - CLI startup handles that failure before entering the interactive TUI: - lock-contention failures tell users to quit other Codex processes and try again - failures consistent with a broken local database offer a safe repair that backs up Codex-owned SQLite files, rebuilds local database files, and retries startup once - declined or unsuccessful repairs print concise guidance plus technical details - Shared startup error plumbing lives in `tui/src/startup_error.rs`, while CLI recovery policy and focused recovery tests live in `cli/src/state_db_recovery.rs`. ## Verification - `cargo test -p codex-tui embedded_state_db_failure_is_typed_for_cli_recovery` - `cargo test -p codex-cli state_db_recovery` - Manually held an exclusive SQLite lock on `state_5.sqlite` and confirmed the CLI shows lock-specific guidance without offering repair. - Manually exercised the repair path with a deliberately invalid `sqlite_home` and confirmed it backs up the blocking path and resumes startup.
This commit is contained in:
@@ -5,7 +5,7 @@ use codex_utils_cli::ApprovalModeCliArg;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::SharedCliOptions;
|
||||
|
||||
#[derive(Parser, Debug)]
|
||||
#[derive(Parser, Clone, Debug)]
|
||||
#[command(version)]
|
||||
pub struct Cli {
|
||||
/// Optional user prompt to start the session.
|
||||
@@ -89,7 +89,7 @@ impl std::ops::DerefMut for Cli {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct TuiSharedCliOptions(SharedCliOptions);
|
||||
|
||||
impl TuiSharedCliOptions {
|
||||
|
||||
+52
-6
@@ -14,6 +14,7 @@ use crate::legacy_core::format_exec_policy_error_with_source;
|
||||
use crate::legacy_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use crate::session_resume::ResolveCwdOutcome;
|
||||
use crate::session_resume::resolve_cwd_for_resume_or_fork;
|
||||
pub use crate::startup_error::LocalStateDbStartupError;
|
||||
use additional_dirs::add_dir_warning_message;
|
||||
use app::App;
|
||||
pub use app::AppExitInfo;
|
||||
@@ -167,6 +168,7 @@ mod session_state;
|
||||
mod shimmer;
|
||||
mod skills_helpers;
|
||||
mod slash_command;
|
||||
mod startup_error;
|
||||
mod startup_hooks_review;
|
||||
mod status;
|
||||
mod status_indicator_widget;
|
||||
@@ -312,6 +314,21 @@ pub(crate) enum AppServerTarget {
|
||||
Remote { endpoint: RemoteAppServerEndpoint },
|
||||
}
|
||||
|
||||
async fn init_state_db_for_app_server_target(
|
||||
config: &Config,
|
||||
app_server_target: &AppServerTarget,
|
||||
) -> std::io::Result<Option<StateDbHandle>> {
|
||||
match app_server_target {
|
||||
AppServerTarget::Embedded => state_db::try_init(config).await.map(Some).map_err(|err| {
|
||||
std::io::Error::other(LocalStateDbStartupError::new(
|
||||
codex_state::state_db_path(config.sqlite_home.as_path()),
|
||||
err.to_string(),
|
||||
))
|
||||
}),
|
||||
AppServerTarget::Remote { .. } => Ok(state_db::get_state_db(config).await),
|
||||
}
|
||||
}
|
||||
|
||||
fn remote_addr_has_explicit_port(addr: &str, parsed: &Url) -> bool {
|
||||
let Some(host) = parsed.host_str() else {
|
||||
return false;
|
||||
@@ -509,7 +526,7 @@ pub(crate) async fn start_app_server_for_picker(
|
||||
pub(crate) async fn start_embedded_app_server_for_picker(
|
||||
config: &Config,
|
||||
) -> color_eyre::Result<AppServerSession> {
|
||||
let state_db = state_db::init(config).await;
|
||||
let state_db = init_state_db_for_app_server_target(config, &AppServerTarget::Embedded).await?;
|
||||
start_app_server_for_picker(
|
||||
config,
|
||||
&AppServerTarget::Embedded,
|
||||
@@ -989,10 +1006,7 @@ pub async fn run_main(
|
||||
otel.as_ref(),
|
||||
otel_originator.as_str(),
|
||||
);
|
||||
let state_db = match &app_server_target {
|
||||
AppServerTarget::Embedded => state_db::init(&config).await,
|
||||
AppServerTarget::Remote { .. } => state_db::get_state_db(&config).await,
|
||||
};
|
||||
let state_db = init_state_db_for_app_server_target(&config, &app_server_target).await?;
|
||||
|
||||
let effective_toml = config.config_layer_stack.effective_config();
|
||||
match effective_toml.try_into() {
|
||||
@@ -1823,7 +1837,8 @@ mod tests {
|
||||
async fn start_test_embedded_app_server(
|
||||
config: Config,
|
||||
) -> color_eyre::Result<InProcessAppServerClient> {
|
||||
let state_db = state_db::init(&config).await;
|
||||
let state_db =
|
||||
init_state_db_for_app_server_target(&config, &AppServerTarget::Embedded).await?;
|
||||
start_embedded_app_server(
|
||||
Arg0DispatchPaths::default(),
|
||||
config,
|
||||
@@ -2416,6 +2431,37 @@ mod tests {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn embedded_state_db_failure_is_typed_for_cli_recovery() -> color_eyre::Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let mut config = build_config(&temp_dir).await?;
|
||||
let occupied_sqlite_home = temp_dir.path().join("sqlite-home");
|
||||
std::fs::write(&occupied_sqlite_home, "occupied")?;
|
||||
config.sqlite_home = occupied_sqlite_home.clone();
|
||||
|
||||
let err =
|
||||
match init_state_db_for_app_server_target(&config, &AppServerTarget::Embedded).await {
|
||||
Ok(_) => panic!("embedded startup should surface state db init failures"),
|
||||
Err(err) => err,
|
||||
};
|
||||
let startup_error = err
|
||||
.get_ref()
|
||||
.and_then(|err| err.downcast_ref::<LocalStateDbStartupError>())
|
||||
.expect("state db startup failure should retain its typed context");
|
||||
|
||||
assert_eq!(
|
||||
startup_error.state_db_path(),
|
||||
codex_state::state_db_path(occupied_sqlite_home.as_path()).as_path()
|
||||
);
|
||||
assert!(
|
||||
startup_error
|
||||
.detail()
|
||||
.contains("failed to initialize state runtime"),
|
||||
"startup error should preserve the underlying state db failure"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn windows_shows_trust_prompt_with_sandbox() -> std::io::Result<()> {
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[error(
|
||||
"failed to initialize sqlite state db at {}: {detail}",
|
||||
state_db_path.display()
|
||||
)]
|
||||
pub struct LocalStateDbStartupError {
|
||||
state_db_path: PathBuf,
|
||||
detail: String,
|
||||
}
|
||||
|
||||
impl LocalStateDbStartupError {
|
||||
pub fn new(state_db_path: PathBuf, detail: String) -> Self {
|
||||
Self {
|
||||
state_db_path,
|
||||
detail,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn state_db_path(&self) -> &Path {
|
||||
self.state_db_path.as_path()
|
||||
}
|
||||
|
||||
pub fn detail(&self) -> &str {
|
||||
self.detail.as_str()
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user