mirror of
https://github.com/openai/codex.git
synced 2026-11-09 08:40:33 +00:00
Default durable threads to paginated history (#40677)
## What changed When `thread/start` omits `historyMode`, select `paginated` for durable threads whose store supports `thread/turns/list` and `thread/items/list`. Continue to default ephemeral threads and threads backed by stores without those APIs to `legacy`. Explicit `legacy` and `paginated` selections keep their existing behavior. ## Testing - Verify durable threads use paginated history with the local store. - Verify ephemeral threads and stores without history-list support use legacy history. GitOrigin-RevId: c94b6a3b522049d541da33c41618c4142370f1a7
This commit is contained in:
1 parent
d132b69219
commit
17c4c47dfa
11 files changed
+58
-6
No files matched your search
@@ -14,6 +14,7 @@ use app_test_support::write_chatgpt_auth;
|
||||
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
|
||||
use codex_app_server_protocol::LoginAccountResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
use codex_app_server_protocol::ThreadResumeResponse;
|
||||
use codex_app_server_protocol::ThreadRollbackParams;
|
||||
@@ -147,6 +148,7 @@ async fn git_attribution_follows_authenticated_workspace_policy() -> Result<()>
|
||||
"chatgpt_base_url".to_string(),
|
||||
json!(format!("{}/backend-api", settings_server.uri())),
|
||||
)])),
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::StrictReviewRequiredNotification;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
use codex_app_server_protocol::ThreadResumeResponse;
|
||||
use codex_app_server_protocol::ThreadRollbackParams;
|
||||
@@ -601,6 +602,8 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
.start_thread(ThreadStartParams {
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(requested_reviewer),
|
||||
history_mode: matches!(lifecycle, ThreadLifecycle::RootRollback)
|
||||
.then_some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -27,6 +27,7 @@ use codex_app_server_protocol::ProjectUpdateResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
use codex_app_server_protocol::ThreadListResponse;
|
||||
use codex_app_server_protocol::ThreadMetadataGitInfoUpdateParams;
|
||||
@@ -855,6 +856,7 @@ async fn assigned_forks_inherit_projects_for_persistent_and_ephemeral_children()
|
||||
let started = server
|
||||
.start_thread(ThreadStartParams {
|
||||
project_id: Some(project.project.id.clone()),
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -168,6 +168,25 @@ async fn thread_section_operations_without_sqlite_return_method_not_found() -> R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_defaults_to_legacy_without_history_list_support() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let store_id = Uuid::new_v4().to_string();
|
||||
create_config_toml_with_thread_store(codex_home.path(), &server.uri(), &store_id)?;
|
||||
|
||||
let _in_memory_store = InMemoryThreadStoreId { store_id };
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
|
||||
let ThreadStartResponse { thread, .. } = mcp.start_thread(ThreadStartParams::default()).await?;
|
||||
|
||||
assert_eq!(thread.history_mode, ThreadHistoryMode::Legacy);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_rejects_paginated_history_without_list_support() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -365,7 +365,19 @@ async fn review_start_with_detached_delivery_returns_new_thread_id() -> Result<(
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let thread_id = start_default_thread(&mut mcp).await?;
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("thread/started"),
|
||||
)
|
||||
.await??;
|
||||
let thread_id = thread.id;
|
||||
materialize_thread_rollout(&mut mcp, &thread_id).await?;
|
||||
let ReviewStartResponse {
|
||||
turn,
|
||||
|
||||
@@ -1444,6 +1444,7 @@ async fn thread_read_include_turns_rejects_unmaterialized_loaded_thread() -> Res
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -142,6 +142,7 @@ async fn thread_rollback_drops_last_turns_and_persists_to_rollout() -> Result<()
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SortDirection;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
@@ -60,7 +61,10 @@ async fn thread_shell_command_history_responses_exclude_persisted_command_execut
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.request(|request_id| ClientRequest::ThreadStart {
|
||||
request_id,
|
||||
params: ThreadStartParams::default(),
|
||||
params: ThreadStartParams {
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let (shell_command, expected_output) = current_shell_output_command("hello from bang")?;
|
||||
@@ -228,7 +232,10 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> {
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.request(|request_id| ClientRequest::ThreadStart {
|
||||
request_id,
|
||||
params: ThreadStartParams::default(),
|
||||
params: ThreadStartParams {
|
||||
history_mode: Some(ThreadHistoryMode::Legacy),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let (shell_command, expected_output) = current_shell_output_command("active turn bang")?;
|
||||
|
||||
@@ -408,8 +408,8 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> {
|
||||
);
|
||||
assert_eq!(
|
||||
thread_json.get("historyMode").and_then(Value::as_str),
|
||||
Some("legacy"),
|
||||
"new threads should serialize `historyMode: legacy`"
|
||||
Some("paginated"),
|
||||
"durable threads should default to `historyMode: paginated` when supported"
|
||||
);
|
||||
assert_eq!(
|
||||
thread_json.get("threadSource").and_then(Value::as_str),
|
||||
@@ -1097,6 +1097,7 @@ async fn thread_start_ephemeral_remains_pathless() -> Result<()> {
|
||||
thread.ephemeral,
|
||||
"ephemeral threads should be marked explicitly"
|
||||
);
|
||||
assert_eq!(thread.history_mode, ThreadHistoryMode::Legacy);
|
||||
assert_eq!(
|
||||
thread.path, None,
|
||||
"ephemeral threads should not expose a path"
|
||||
|
||||
Reference in new issue
Block a user