mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
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
This commit is contained in:
@@ -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" } }
|
||||
|
||||
@@ -2375,15 +2375,22 @@ impl ThreadRequestProcessor {
|
||||
include_turns: bool,
|
||||
) -> Result<Option<Thread>, 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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user