From 20158200641eb53921f6022ab2b2d87313dfbea5 Mon Sep 17 00:00:00 2001 From: Tom Wiltzius Date: Fri, 15 May 2026 13:58:00 -0700 Subject: [PATCH 1/3] codex: soften sqlite metadata sync failures --- .../src/local/update_thread_metadata.rs | 80 ++++++++++++++++++- 1 file changed, 76 insertions(+), 4 deletions(-) diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index c09ca80f977d..16f9f79e06d6 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -14,6 +14,7 @@ use codex_rollout::find_archived_thread_path_by_id_str; use codex_rollout::find_thread_path_by_id_str; use codex_rollout::read_session_meta_line; use codex_state::ThreadMetadataBuilder; +use tracing::warn; use super::LocalThreadStore; use super::helpers::git_info_from_parts; @@ -51,8 +52,15 @@ pub(super) async fn update_thread_metadata( } let needs_rollout_compat = needs_rollout_compatibility_update(&patch); - let updated = - apply_metadata_update(store, thread_id, patch.clone(), params.include_archived).await?; + let require_sqlite_write = sqlite_write_failure_should_block(&patch); + let updated = apply_metadata_update( + store, + thread_id, + patch.clone(), + params.include_archived, + require_sqlite_write, + ) + .await?; if !needs_rollout_compat { return Ok(updated); } @@ -169,6 +177,7 @@ async fn apply_metadata_update( thread_id: ThreadId, patch: ThreadMetadataPatch, include_archived: bool, + require_sqlite_write: bool, ) -> ThreadStoreResult { let live_rollout_path = live_writer::rollout_path(store, thread_id).await.ok(); let mut rollout_path = patch.rollout_path.clone().or(live_rollout_path); @@ -316,9 +325,19 @@ async fn apply_metadata_update( }; match (state_db.is_some(), sqlite_write_result) { (true, Ok(())) => {} - (true, Err(err)) => return Err(err), + (true, Err(err)) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { + return Err(err); + } + (true, Err(err)) => { + warn!("state db update_thread_metadata failed for {thread_id}: {err}"); + } (false, Ok(())) => {} - (false, Err(err)) => return Err(err), + (false, Err(err)) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { + return Err(err); + } + (false, Err(err)) => { + warn!("state db update_thread_metadata failed for {thread_id}: {err}"); + } } read_thread::read_thread( @@ -342,6 +361,19 @@ fn needs_rollout_compatibility_update(patch: &ThreadMetadataPatch) -> bool { !has_observed_metadata_facts(patch) } +fn sqlite_write_failure_should_block(patch: &ThreadMetadataPatch) -> bool { + // Before live metadata sync moved above the rollout writer, SQLite sync failures for + // transcript-derived metadata, thread names, and memory-mode indexing were log-only. Keep that + // failure isolation so a corrupted optional state DB does not make JSONL transcript durability + // look broken. Explicit git-only updates still require SQLite because partial git patches need + // the existing SQLite value to preserve unspecified fields. + patch.git_info.is_some() && !has_observed_metadata_facts(patch) +} + +fn sqlite_write_error_is_best_effort(err: &ThreadStoreError) -> bool { + matches!(err, ThreadStoreError::Internal { .. }) +} + fn has_observed_metadata_facts(patch: &ThreadMetadataPatch) -> bool { patch.rollout_path.is_some() || patch.preview.is_some() @@ -1136,6 +1168,46 @@ mod tests { assert_eq!(memory_mode.as_deref(), Some("disabled")); } + #[test] + fn sqlite_failures_are_best_effort_for_legacy_rollout_compat_updates() { + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + name: Some(Some("User chosen name".to_string())), + ..Default::default() + })); + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + memory_mode: Some(ThreadMemoryMode::Disabled), + ..Default::default() + })); + } + + #[test] + fn sqlite_failures_are_best_effort_for_observed_metadata_updates() { + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + updated_at: Some(Utc::now()), + ..Default::default() + })); + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + preview: Some("Observed preview".to_string()), + git_info: Some(GitInfoPatch { + branch: Some(Some("main".to_string())), + ..Default::default() + }), + memory_mode: Some(ThreadMemoryMode::Enabled), + ..Default::default() + })); + } + + #[test] + fn sqlite_failures_still_block_for_explicit_git_only_updates() { + assert!(sqlite_write_failure_should_block(&ThreadMetadataPatch { + git_info: Some(GitInfoPatch { + branch: Some(Some("main".to_string())), + ..Default::default() + }), + ..Default::default() + })); + } + #[tokio::test] async fn metadata_patch_applies_title_over_existing_name() { let home = TempDir::new().expect("temp dir"); From 3511e6cafe2b419d40136d770c7e827d1ab73bd5 Mon Sep 17 00:00:00 2001 From: Tom Wiltzius Date: Fri, 15 May 2026 14:16:49 -0700 Subject: [PATCH 2/3] codex: address PR review feedback (#22899) --- .../src/local/update_thread_metadata.rs | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index 16f9f79e06d6..852c5ea868fa 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -90,7 +90,13 @@ pub(super) async fn update_thread_metadata( .await; if let Some(name) = name { - apply_thread_name(store, thread_id, name.unwrap_or_default()).await?; + apply_thread_name( + store, + thread_id, + name.unwrap_or_default(), + require_sqlite_write, + ) + .await?; } let resolved_git_info = match git_info { @@ -498,18 +504,31 @@ async fn apply_thread_name( store: &LocalThreadStore, thread_id: ThreadId, name: String, + require_sqlite_write: bool, ) -> ThreadStoreResult<()> { if let Some(state_db) = store.state_db().await { - let updated = state_db + let title_update = state_db .update_thread_title(thread_id, &name) .await .map_err(|err| ThreadStoreError::Internal { message: format!("failed to set thread name: {err}"), - })?; - if !updated { - return Err(ThreadStoreError::Internal { - message: format!("thread metadata unavailable before name update: {thread_id}"), }); + match title_update { + Ok(true) => {} + Ok(false) if require_sqlite_write => { + return Err(ThreadStoreError::Internal { + message: format!("thread metadata unavailable before name update: {thread_id}"), + }); + } + Ok(false) => { + warn!("state db thread metadata unavailable before name update for {thread_id}"); + } + Err(err) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { + return Err(err); + } + Err(err) => { + warn!("state db update_thread_title failed for {thread_id}: {err}"); + } } } From 7c258ff3d1ba32e23d7f9e88f6fe274e496b4fd6 Mon Sep 17 00:00:00 2001 From: Tom Wiltzius Date: Fri, 15 May 2026 14:28:43 -0700 Subject: [PATCH 3/3] codex: keep explicit name sqlite failures strict --- .../src/local/update_thread_metadata.rs | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index 852c5ea868fa..16f9f79e06d6 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -90,13 +90,7 @@ pub(super) async fn update_thread_metadata( .await; if let Some(name) = name { - apply_thread_name( - store, - thread_id, - name.unwrap_or_default(), - require_sqlite_write, - ) - .await?; + apply_thread_name(store, thread_id, name.unwrap_or_default()).await?; } let resolved_git_info = match git_info { @@ -504,31 +498,18 @@ async fn apply_thread_name( store: &LocalThreadStore, thread_id: ThreadId, name: String, - require_sqlite_write: bool, ) -> ThreadStoreResult<()> { if let Some(state_db) = store.state_db().await { - let title_update = state_db + let updated = state_db .update_thread_title(thread_id, &name) .await .map_err(|err| ThreadStoreError::Internal { message: format!("failed to set thread name: {err}"), + })?; + if !updated { + return Err(ThreadStoreError::Internal { + message: format!("thread metadata unavailable before name update: {thread_id}"), }); - match title_update { - Ok(true) => {} - Ok(false) if require_sqlite_write => { - return Err(ThreadStoreError::Internal { - message: format!("thread metadata unavailable before name update: {thread_id}"), - }); - } - Ok(false) => { - warn!("state db thread metadata unavailable before name update for {thread_id}"); - } - Err(err) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { - return Err(err); - } - Err(err) => { - warn!("state db update_thread_title failed for {thread_id}: {err}"); - } } }