From 9949245d1d2b4a39a6f1841922322f767fa146ad Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 2 Aug 2026 20:20:52 +0000 Subject: [PATCH] Preserve SQLite thread metadata during goal mutations (#36632) ## Why Setting or clearing a thread goal could reconcile an already indexed rollout and overwrite SQLite-only thread metadata, including the thread preview. ## What changed Skip rollout reconciliation when SQLite already references the same existing rollout and its session metadata matches the requested thread. Continue reconciling when the SQLite row is missing or the stored rollout is invalid so goal mutations can still repair thread metadata. ## Testing Add coverage that verifies goal set and clear preserve SQLite previews, and that goal set restores a deleted SQLite thread row. GitOrigin-RevId: cf19cef98559fc67182ac7430e4e50ed2eed83f3 --- .../thread_goal_processor.rs | 13 ++ .../tests/suite/v2/thread_resume.rs | 132 ++++++++++++++++++ 2 files changed, 145 insertions(+) diff --git a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs index 8c285dbd1f..81cb9bf0be 100644 --- a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs @@ -323,6 +323,19 @@ impl ThreadGoalRequestProcessor { })? .ok_or_else(|| invalid_request(format!("thread not found: {thread_id}")))?, }; + + if let Ok(Some(metadata)) = state_db.get_thread(thread_id).await + && codex_rollout::plain_rollout_path(metadata.rollout_path.as_path()) + == codex_rollout::plain_rollout_path(rollout_path.as_path()) + && let Some(existing_path) = + codex_rollout::existing_rollout_path(metadata.rollout_path.as_path()).await + && codex_rollout::read_session_meta_line(existing_path.as_path()) + .await + .is_ok_and(|session_meta| session_meta.meta.id == thread_id) + { + return Ok(()); + } + reconcile_rollout( Some(state_db), rollout_path.as_path(), diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 17a768b73c..e2a21a4ccc 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -1012,6 +1012,138 @@ async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_goal_mutations_preserve_authoritative_sqlite_metadata() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + mock_responses_config(&server.uri()) + .enable_feature(Feature::Goals) + .write(codex_home.path())?; + let thread_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Rollout preview", + Some("mock_provider"), + /*git_info*/ None, + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let state_db = StateRuntime::init( + codex_state::SqliteConfig::new_for_testing(codex_home.path().abs()), + "mock_provider".into(), + ) + .await?; + let thread_id = ThreadId::from_string(&thread_id)?; + let mut metadata = state_db + .get_thread(thread_id) + .await? + .expect("thread metadata should exist"); + metadata.preview = Some("SQLite preview before goal set".to_string()); + state_db.upsert_thread(&metadata).await?; + + let goal_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread_id.to_string(), + "objective": "preserve SQLite metadata", + "status": "paused", + })), + ) + .await?; + let _: ThreadGoalSetResponse = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(goal_id)).await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("thread/goal/updated"), + ) + .await??; + + let mut metadata = state_db + .get_thread(thread_id) + .await? + .expect("thread metadata should survive goal set"); + assert_eq!( + metadata.preview.as_deref(), + Some("SQLite preview before goal set") + ); + metadata.preview = Some("SQLite preview before goal clear".to_string()); + state_db.upsert_thread(&metadata).await?; + + let clear_id = mcp + .send_raw_request( + "thread/goal/clear", + Some(json!({ + "threadId": thread_id.to_string(), + })), + ) + .await?; + let cleared: ThreadGoalClearResponse = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(clear_id)).await??; + assert!(cleared.cleared); + let metadata = state_db + .get_thread(thread_id) + .await? + .expect("thread metadata should survive goal clear"); + assert_eq!( + metadata.preview.as_deref(), + Some("SQLite preview before goal clear") + ); + + Ok(()) +} + +#[tokio::test] +async fn thread_goal_set_repairs_missing_sqlite_metadata() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + mock_responses_config(&server.uri()) + .enable_feature(Feature::Goals) + .write(codex_home.path())?; + let thread_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Rollout preview", + Some("mock_provider"), + /*git_info*/ None, + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let state_db = StateRuntime::init( + codex_state::SqliteConfig::new_for_testing(codex_home.path().abs()), + "mock_provider".into(), + ) + .await?; + let thread_id = ThreadId::from_string(&thread_id)?; + state_db.delete_thread(thread_id).await?; + + let goal_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread_id.to_string(), + "objective": "repair missing SQLite metadata", + "status": "paused", + })), + ) + .await?; + let _: ThreadGoalSetResponse = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(goal_id)).await??; + assert!(state_db.get_thread(thread_id).await?.is_some()); + + Ok(()) +} + #[tokio::test] async fn goal_first_live_thread_appears_in_state_db_thread_list() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await;