From e87e2b495bcf8aa1950e2bb24cc95bfdc6fd473a Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 4 Aug 2026 23:08:34 +0000 Subject: [PATCH] Support `includeTurns` reads for paginated threads (#36993) ## Why Clients using `thread/read` with `includeTurns: true` need the legacy full-history view even when a thread stores its history in paginated form. ## What changed - Reconstruct full projected turns from paginated history for both stored and loaded threads. - Keep `thread/turns/list` and `thread/items/list` as the preferred incremental history APIs. ## Testing - Cover `thread/read(includeTurns=true)` before and after a paginated thread is loaded. GitOrigin-RevId: 6efdc04e5ed3cad2ff2078d8103933082c7921db --- codex-rs/app-server/README.md | 3 +- .../request_processors/thread_processor.rs | 46 ++++++++++++------ .../app-server/tests/suite/v2/thread_read.rs | 48 +++++++++++-------- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 9a01e92b9e..dbe090e54b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -535,7 +535,8 @@ Later, after the idle unload timeout: Use `thread/read` to fetch a stored thread by id without resuming it. Pass `includeTurns` when you want thread history loaded into `thread.turns`. The returned thread includes `parentThreadId`, `agentNickname`, and `agentRole` for subagent threads when available. -Paginated threads support metadata-only reads; `includeTurns: true` is unsupported for them. +Paginated threads can also use `includeTurns: true`, but clients should prefer +`thread/turns/list` and `thread/items/list` for incremental history loading. ```json { "method": "thread/read", "id": 22, "params": { "threadId": "thr_123" } } diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index b296a4e33d..2164737c21 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -2375,15 +2375,22 @@ impl ThreadRequestProcessor { include_turns: bool, ) -> Result, ThreadReadViewError> { let fallback_provider = self.config.model_provider_id.as_str(); - if include_turns - && self + if include_turns { + let Some(stored_thread) = self .read_stored_thread_for_read(thread_id, /*include_history*/ false) .await? - .is_some_and(|thread| matches!(thread.history_mode, ThreadHistoryMode::Paginated)) - { - return Err(ThreadReadViewError::InvalidRequest( - "paginated threads do not support thread/read(includeTurns=true)".to_string(), - )); + else { + return Ok(None); + }; + if matches!(stored_thread.history_mode, ThreadHistoryMode::Paginated) { + let (mut thread, _) = + thread_from_stored_thread(stored_thread, fallback_provider, &self.config.cwd); + thread.turns = self + .paginated_thread_full_turns(thread_id) + .await + .map_err(ThreadReadViewError::JsonRpc)?; + return Ok(Some(thread)); + } } let Some(stored_thread) = self .read_stored_thread_for_read(thread_id, /*include_history*/ include_turns) @@ -2448,11 +2455,6 @@ impl ThreadRequestProcessor { "ephemeral threads do not support includeTurns".to_string(), )); } - if include_turns && matches!(config_snapshot.history_mode, ThreadHistoryMode::Paginated) { - return Err(ThreadReadViewError::InvalidRequest( - "paginated threads do not support thread/read(includeTurns=true)".to_string(), - )); - } let fallback_thread = build_thread_from_loaded_snapshot(thread_id, &config_snapshot, loaded_thread); let mut thread = if let Some(mut thread) = persisted_thread { @@ -2481,6 +2483,20 @@ impl ThreadRequestProcessor { self.attach_thread_name(thread_id, thread).await; if include_turns { + if matches!( + thread.history_mode, + codex_app_server_protocol::ThreadHistoryMode::Paginated + ) { + self.thread_store + .persist_thread(thread_id) + .await + .map_err(|err| thread_read_history_load_error(thread_id, err))?; + thread.turns = self + .paginated_thread_full_turns(thread_id) + .await + .map_err(ThreadReadViewError::JsonRpc)?; + return Ok(()); + } let history = loaded_thread .load_history(/*include_archived*/ true) .await @@ -2746,8 +2762,8 @@ impl ThreadRequestProcessor { } } - // Older clients omit `excludeTurns` and expect full `thread.turns` on resume. - // Remove this slow path once all clients use paginated resume bootstrap. + // Older clients expect full `thread.turns` from resume and `thread/read(includeTurns=true)`. + // Keep this slow compatibility path until all clients page history directly. async fn paginated_thread_full_turns( &self, thread_id: ThreadId, @@ -4904,6 +4920,7 @@ enum ThreadReadViewError { InvalidRequest(String), Unsupported(&'static str), Internal(String), + JsonRpc(JSONRPCErrorError), } fn thread_read_view_error(err: ThreadReadViewError) -> JSONRPCErrorError { @@ -4913,6 +4930,7 @@ fn thread_read_view_error(err: ThreadReadViewError) -> JSONRPCErrorError { unsupported_thread_store_operation(operation) } ThreadReadViewError::Internal(message) => internal_error(message), + ThreadReadViewError::JsonRpc(error) => error, } } diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 8ad3dc0214..226643eeea 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -213,8 +213,7 @@ async fn thread_read_can_include_turns() -> Result<()> { } #[tokio::test] -async fn paginated_stored_thread_routes_projected_turns_and_rejects_legacy_history_paths() --> Result<()> { +async fn paginated_stored_thread_routes_projected_turns() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; MockResponsesConfig::new(&server.uri()).write(codex_home.path())?; @@ -270,23 +269,6 @@ async fn paginated_stored_thread_routes_projected_turns_and_rejects_legacy_histo .expect("thread/list should include paginated thread"); assert_eq!(listed.history_mode, ThreadHistoryMode::Paginated); - let read_id = mcp - .send_thread_read_request(ThreadReadParams { - thread_id: conversation_id.clone(), - include_turns: true, - }) - .await?; - let read_err: JSONRPCError = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(read_id)), - ) - .await??; - assert_eq!(read_err.error.code, -32600); - assert_eq!( - read_err.error.message, - "paginated threads do not support thread/read(includeTurns=true)" - ); - let turns_list_id = mcp .send_thread_turns_list_request(ThreadTurnsListParams { thread_id: conversation_id.clone(), @@ -1512,7 +1494,7 @@ async fn thread_turns_list_rejects_unmaterialized_loaded_thread() -> Result<()> } #[tokio::test] -async fn paginated_history_lists_use_projected_turns_and_items() -> Result<()> { +async fn paginated_history_lists_and_legacy_reads_use_projected_turns_and_items() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; MockResponsesConfig::new(&server.uri()).write(codex_home.path())?; @@ -1663,6 +1645,17 @@ async fn paginated_history_lists_use_projected_turns_and_items() -> Result<()> { }; let expected_full_turns = vec![expected_turn_1_full.clone(), expected_turn_2_full.clone()]; + let read_id = mcp + .send_thread_read_request(ThreadReadParams { + thread_id: thread_id.to_string(), + include_turns: true, + }) + .await?; + let ThreadReadResponse { + thread: unloaded_thread, + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(read_id)).await??; + assert_eq!(unloaded_thread.turns, expected_full_turns); + let legacy_resume_id = mcp .send_thread_resume_request(ThreadResumeParams { thread_id: thread_id.to_string(), @@ -1915,6 +1908,21 @@ async fn paginated_history_lists_use_projected_turns_and_items() -> Result<()> { ) .await??; + let read_id = mcp + .send_thread_read_request(ThreadReadParams { + thread_id: thread_id.to_string(), + include_turns: true, + }) + .await?; + let ThreadReadResponse { + thread: loaded_thread, + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(read_id)).await??; + assert_eq!(&loaded_thread.turns[..2], expected_full_turns); + assert_eq!( + turn_user_texts(&loaded_thread.turns), + vec!["continue after legacy resume"] + ); + Ok(()) }