mirror of
https://github.com/openai/codex.git
synced 2026-10-21 07:42:01 +00:00
Fall back to summary history for read-only conversations (#45980)
## Why History paging failures can prevent the TUI from opening a conversation that is already open elsewhere, even when its prompts and final replies are available through turn summaries. ## What changed - When paginated read-only history loading fails, request the latest 100 turns from `thread/turns/list` with `itemsView: summary` and display them in chronological order. - Clear history pagination state and show a notice that intermediate messages and tool activity are unavailable. Surface the notice during startup, session selection, and command center selection. - Show a concise retry message when the command center cannot load the conversation, preserving the current conversation. ## Testing Extend read-only history tests to cover summary recovery, the 100-turn limit and ordering, the user notice, and preservation of the current conversation when both history loading methods fail. GitOrigin-RevId: c9f06ad58ed5573ce0b4578f429558bfd475830f
This commit is contained in:
@@ -491,6 +491,7 @@ impl App {
|
||||
crate::app_server_session::ResumeModelSettings::PreserveExistingThread
|
||||
}
|
||||
};
|
||||
let mut history_notice = None;
|
||||
let (resumed, read_only) = if let Some((_, started)) = started {
|
||||
(started, false)
|
||||
} else if !unloaded
|
||||
@@ -519,11 +520,16 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(thread) => (thread, true),
|
||||
Err(error) => {
|
||||
self.add_agents_overview_error(format!(
|
||||
"Failed to view task open elsewhere: {error}"
|
||||
));
|
||||
Ok((thread, notice)) => {
|
||||
history_notice = notice;
|
||||
(thread, true)
|
||||
}
|
||||
Err(_) => {
|
||||
tracing::warn!("Failed to load read-only conversation history");
|
||||
self.add_agents_overview_error(
|
||||
"Couldn't load this conversation. Please try again."
|
||||
.to_string(),
|
||||
);
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
}
|
||||
@@ -606,6 +612,10 @@ impl App {
|
||||
self.ensure_thread_channel(root_thread_id)
|
||||
.mark_external_writer();
|
||||
self.chat_widget.show_external_writer_thread();
|
||||
if let Some(notice) = history_notice {
|
||||
self.chat_widget
|
||||
.add_info_message(notice.to_string(), /*hint*/ None);
|
||||
}
|
||||
}
|
||||
let mut destination_config = self.chat_widget.config_ref().clone();
|
||||
if self.app_server_target.uses_remote_workspace() {
|
||||
|
||||
@@ -1246,13 +1246,17 @@ impl App {
|
||||
self.resume_model_settings(),
|
||||
)
|
||||
.await;
|
||||
let mut history_notice = None;
|
||||
let (resumed, read_only) = match resumed {
|
||||
Ok(resumed) => (resumed, false),
|
||||
Err(err) if crate::app_server_session::is_active_writer_error(&err) => match app_server
|
||||
.read_thread_for_viewing(&resume_config, &local_settings, target_session.thread_id)
|
||||
.await
|
||||
{
|
||||
Ok(thread) => (thread, true),
|
||||
Ok((thread, notice)) => {
|
||||
history_notice = notice;
|
||||
(thread, true)
|
||||
}
|
||||
Err(read_err) => {
|
||||
self.add_session_picker_error(format!(
|
||||
"Failed to view thread open elsewhere: {read_err}"
|
||||
@@ -1310,6 +1314,10 @@ impl App {
|
||||
self.ensure_thread_channel(resumed_thread_id)
|
||||
.mark_external_writer();
|
||||
self.chat_widget.show_external_writer_thread();
|
||||
if let Some(notice) = history_notice {
|
||||
self.chat_widget
|
||||
.add_info_message(notice.to_string(), /*hint*/ None);
|
||||
}
|
||||
}
|
||||
if self.app_server_target.uses_remote_workspace() {
|
||||
let config = self.chat_widget.config_ref();
|
||||
|
||||
@@ -378,6 +378,7 @@ impl App {
|
||||
let mut start_in_agents_overview =
|
||||
matches!(&session_selection, SessionSelection::AgentsOverview);
|
||||
let mut read_only_thread = false;
|
||||
let mut history_notice = None;
|
||||
let (mut chat_widget, initial_started_thread) = match session_selection {
|
||||
SessionSelection::StartFresh
|
||||
| SessionSelection::Exit
|
||||
@@ -493,7 +494,10 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(result) => result,
|
||||
Ok(result) => result.map(|(thread, notice)| {
|
||||
history_notice = notice;
|
||||
thread
|
||||
}),
|
||||
Err(err) => return shutdown_on_startup_error(app_server, err).await,
|
||||
}
|
||||
}
|
||||
@@ -851,6 +855,10 @@ See the Codex keymap documentation for supported actions and examples."
|
||||
if read_only_thread {
|
||||
app.ensure_thread_channel(thread_id).mark_external_writer();
|
||||
app.chat_widget.show_external_writer_thread();
|
||||
if let Some(notice) = history_notice {
|
||||
app.chat_widget
|
||||
.add_info_message(notice.to_string(), /*hint*/ None);
|
||||
}
|
||||
}
|
||||
if !read_only_thread
|
||||
&& should_prompt_for_paused_goal_after_startup_resume
|
||||
|
||||
@@ -151,6 +151,8 @@ pub(super) enum HistoryCapabilities {
|
||||
LegacyDynamicToolsAndHistory,
|
||||
ForkHydrationFails,
|
||||
ReadAfterResumeFails,
|
||||
ItemsListFails,
|
||||
ItemsAndSummaryTurnsFail,
|
||||
ThreadListFails,
|
||||
ThreadStartFails,
|
||||
ConfigReadUnsupported(i64),
|
||||
@@ -360,13 +362,21 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
id: request_id,
|
||||
result: serde_json::json!({}),
|
||||
})
|
||||
} else if history_capabilities == HistoryCapabilities::ReadAfterResumeFails
|
||||
&& request.method == "thread/read"
|
||||
&& request_sink
|
||||
.lock()
|
||||
.expect("request recorder lock")
|
||||
.iter()
|
||||
.any(|recorded| recorded.method == "thread/resume")
|
||||
} else if (matches!(
|
||||
history_capabilities,
|
||||
HistoryCapabilities::ItemsListFails
|
||||
| HistoryCapabilities::ItemsAndSummaryTurnsFail
|
||||
) && (request.method == "thread/items/list"
|
||||
|| (history_capabilities == HistoryCapabilities::ItemsAndSummaryTurnsFail
|
||||
&& request.method == "thread/turns/list"
|
||||
&& params.is_some_and(|params| params["itemsView"] == "summary"))))
|
||||
|| (history_capabilities == HistoryCapabilities::ReadAfterResumeFails
|
||||
&& request.method == "thread/read"
|
||||
&& request_sink
|
||||
.lock()
|
||||
.expect("request recorder lock")
|
||||
.iter()
|
||||
.any(|recorded| recorded.method == "thread/resume"))
|
||||
{
|
||||
JSONRPCMessage::Error(JSONRPCError {
|
||||
id: request_id,
|
||||
@@ -4480,9 +4490,12 @@ async fn external_writer_escape_preserves_snapshot_and_explicit_quits() -> Resul
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_center_read_only_open_requests_and_failure_preservation() -> Result<()> {
|
||||
for history_capabilities in [
|
||||
HistoryCapabilities::Current,
|
||||
HistoryCapabilities::ReadAfterResumeFails,
|
||||
for (history_capabilities, saved_turn_count) in [
|
||||
(HistoryCapabilities::Current, 0usize),
|
||||
(HistoryCapabilities::ReadAfterResumeFails, 0),
|
||||
(HistoryCapabilities::ItemsListFails, 6),
|
||||
(HistoryCapabilities::ItemsListFails, 101),
|
||||
(HistoryCapabilities::ItemsAndSummaryTurnsFail, 6),
|
||||
] {
|
||||
let (mut app, _codex_home) = Box::pin(make_history_test_app()).await?;
|
||||
std::fs::write(
|
||||
@@ -4497,8 +4510,72 @@ async fn command_center_read_only_open_requests_and_failure_preservation() -> Re
|
||||
)
|
||||
.map_err(std::io::Error::other)?;
|
||||
}
|
||||
let thread_id =
|
||||
create_history_rollout(&app.config, ThreadHistoryMode::Legacy, "Locked task")?;
|
||||
let history_mode = if matches!(
|
||||
history_capabilities,
|
||||
HistoryCapabilities::ItemsListFails | HistoryCapabilities::ItemsAndSummaryTurnsFail
|
||||
) {
|
||||
ThreadHistoryMode::Paginated
|
||||
} else {
|
||||
ThreadHistoryMode::Legacy
|
||||
};
|
||||
let thread_id = create_history_rollout(&app.config, history_mode, "Locked task")?;
|
||||
if history_mode == ThreadHistoryMode::Paginated {
|
||||
let path = rollout_path(
|
||||
app.config.codex_home.as_path(),
|
||||
"2026-01-02T00-00-00",
|
||||
&thread_id.to_string(),
|
||||
);
|
||||
let mut contents = std::fs::read_to_string(&path)?;
|
||||
for index in 0..saved_turn_count {
|
||||
let first_ordinal = contents.lines().count();
|
||||
let events = [
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: format!("saved-turn-{index}"),
|
||||
root_turn_id: None,
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
thread_id,
|
||||
turn_id: format!("saved-turn-{index}"),
|
||||
item: TurnItem::UserMessage(UserMessageItem::new(&[CoreUserInput::Text {
|
||||
text: "Locked task".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}])),
|
||||
started_at_ms: None,
|
||||
completed_at_ms: 0,
|
||||
}),
|
||||
EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
thread_id,
|
||||
turn_id: format!("saved-turn-{index}"),
|
||||
item: TurnItem::AgentMessage(AgentMessageItem {
|
||||
id: format!("saved-answer-{index}"),
|
||||
content: vec![AgentMessageContent::Text {
|
||||
text: "Saved final answer".to_string(),
|
||||
}],
|
||||
phase: Some(codex_protocol::models::MessagePhase::FinalAnswer),
|
||||
memory_citation: None,
|
||||
delivery: None,
|
||||
questions: None,
|
||||
}),
|
||||
started_at_ms: None,
|
||||
completed_at_ms: 0,
|
||||
}),
|
||||
];
|
||||
for (offset, event) in events.into_iter().enumerate() {
|
||||
let record = serde_json::json!({
|
||||
"timestamp": "2026-01-02T00:00:00Z",
|
||||
"ordinal": first_ordinal + offset,
|
||||
"type": "event_msg",
|
||||
"payload": event,
|
||||
});
|
||||
contents.push_str(&format!("{record}\n"));
|
||||
}
|
||||
}
|
||||
std::fs::write(path, contents)?;
|
||||
}
|
||||
let mut owner = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?;
|
||||
Box::pin(owner.resume_thread(
|
||||
&app.local_settings,
|
||||
@@ -4532,16 +4609,28 @@ async fn command_center_read_only_open_requests_and_failure_preservation() -> Re
|
||||
let before = render_bottom_popup(&app.chat_widget, /*width*/ 96);
|
||||
requests.lock().unwrap().clear();
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let (tx, mut events) = tokio::sync::mpsc::unbounded_channel();
|
||||
app.app_event_tx = AppEventSender::new(tx);
|
||||
|
||||
Box::pin(app.select_agents_overview_thread(&mut tui, &mut server, thread_id)).await?;
|
||||
assert_eq!(recorded_params(&requests, "thread/resume").len(), 1);
|
||||
assert!(recorded_params(&requests, "turn/start").is_empty());
|
||||
if history_capabilities == HistoryCapabilities::ReadAfterResumeFails {
|
||||
if matches!(
|
||||
history_capabilities,
|
||||
HistoryCapabilities::ReadAfterResumeFails
|
||||
| HistoryCapabilities::ItemsAndSummaryTurnsFail
|
||||
) {
|
||||
let error = render_bottom_popup(&app.chat_widget, /*width*/ 96);
|
||||
assert!(
|
||||
error.contains("Failed to view task open elsewhere"),
|
||||
"{error}"
|
||||
);
|
||||
insta::allow_duplicates! {
|
||||
insta::assert_snapshot!(error, @"
|
||||
Unable to complete action
|
||||
Couldn't load this conversation. Please try again.
|
||||
|
||||
› 1. Return to command center
|
||||
|
||||
Press enter to confirm or esc to go back
|
||||
");
|
||||
}
|
||||
assert_eq!(app.current_displayed_thread_id(), Some(current_id));
|
||||
assert!(recorded_params(&requests, "thread/unsubscribe").is_empty());
|
||||
app.chat_widget.handle_key_event(KeyCode::Esc.into());
|
||||
@@ -4553,6 +4642,48 @@ async fn command_center_read_only_open_requests_and_failure_preservation() -> Re
|
||||
app.thread_event_channels[&thread_id].attachment(),
|
||||
ThreadEventAttachment::ExternalWriter
|
||||
);
|
||||
let turns = app.thread_event_channels[&thread_id]
|
||||
.store
|
||||
.lock()
|
||||
.await
|
||||
.snapshot()
|
||||
.turns;
|
||||
assert!(serde_json::to_string(&turns)?.contains("Locked task"));
|
||||
if history_capabilities == HistoryCapabilities::ItemsListFails {
|
||||
let notice = std::iter::from_fn(|| events.try_recv().ok())
|
||||
.filter_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => {
|
||||
Some(lines_to_single_string(&cell.display_lines(/*width*/ 200)))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.find(|message| message.contains("Showing up to 100 recent prompts"))
|
||||
.expect("summary history notice");
|
||||
insta::allow_duplicates! {
|
||||
insta::assert_snapshot!(notice, @"• Showing up to 100 recent prompts and final replies. Intermediate messages and tool activity are unavailable.");
|
||||
}
|
||||
assert!(serde_json::to_string(&turns)?.contains("Saved final answer"));
|
||||
assert!(!recorded_params(&requests, "thread/items/list").is_empty());
|
||||
assert_eq!(
|
||||
turns.iter().map(|turn| turn.id.clone()).collect::<Vec<_>>(),
|
||||
(saved_turn_count.saturating_sub(100)..saved_turn_count)
|
||||
.map(|index| format!("saved-turn-{index}"))
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
assert_eq!(
|
||||
recorded_params(&requests, "thread/turns/list")
|
||||
.into_iter()
|
||||
.filter(|params| params["itemsView"] == "summary")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
recorded_params(&requests, "thread/read")
|
||||
.iter()
|
||||
.all(|params| !params["includeTurns"].as_bool().unwrap_or(false))
|
||||
);
|
||||
assert!(!server.has_older_history(thread_id));
|
||||
}
|
||||
}
|
||||
owner.shutdown().await?;
|
||||
server.shutdown().await?;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Resume history policy for local rollouts that may migrate between formats.
|
||||
//! Resume and read-only history loading across rollout formats.
|
||||
|
||||
use super::AppServerSession;
|
||||
use super::AppServerStartedThread;
|
||||
@@ -14,12 +14,19 @@ use crate::legacy_core::config::Config;
|
||||
use codex_app_server_client::TypedRequestError;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::SortDirection;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadResumeResponse;
|
||||
use codex_app_server_protocol::ThreadTurnsListParams;
|
||||
use codex_app_server_protocol::ThreadTurnsListResponse;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use color_eyre::eyre::Result;
|
||||
|
||||
// Bound recovery to recent messages when item paging is unavailable.
|
||||
const READ_ONLY_HISTORY_TURN_LIMIT: u32 = 100;
|
||||
|
||||
impl AppServerSession {
|
||||
/// Read a conflicting thread without taking its writer lease. This is a snapshot, not an
|
||||
/// attachment: the caller must not route turns through this session.
|
||||
@@ -28,17 +35,46 @@ impl AppServerSession {
|
||||
config: &Config,
|
||||
local_settings: &crate::local_settings::LocalSettings,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
) -> Result<(AppServerStartedThread, Option<&'static str>)> {
|
||||
let mut thread = self.thread_read(thread_id, /*include_turns*/ false).await?;
|
||||
self.hydrate_initial_thread_history(
|
||||
&mut thread,
|
||||
/*turn_cursor*/ None,
|
||||
/*item_cursor*/ None,
|
||||
Some(config),
|
||||
Some(local_settings),
|
||||
HistoryHydrationScope::Initial,
|
||||
)
|
||||
.await?;
|
||||
let mut history_notice = None;
|
||||
if let Err(error) = self
|
||||
.hydrate_initial_thread_history(
|
||||
&mut thread,
|
||||
/*turn_cursor*/ None,
|
||||
/*item_cursor*/ None,
|
||||
Some(config),
|
||||
Some(local_settings),
|
||||
HistoryHydrationScope::Initial,
|
||||
)
|
||||
.await
|
||||
{
|
||||
if thread.history_mode == ThreadHistoryMode::Legacy {
|
||||
return Err(error);
|
||||
}
|
||||
// Summary pages retain user messages and final replies without scanning
|
||||
// every tool item when the item-paging endpoint is unavailable.
|
||||
tracing::warn!("Failed to page read-only thread history; trying summary pages");
|
||||
let request_id = self.next_request_id();
|
||||
let page: ThreadTurnsListResponse = self
|
||||
.client
|
||||
.request_typed(ClientRequest::ThreadTurnsList {
|
||||
request_id,
|
||||
params: ThreadTurnsListParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
cursor: None,
|
||||
limit: Some(READ_ONLY_HISTORY_TURN_LIMIT),
|
||||
sort_direction: Some(SortDirection::Desc),
|
||||
items_view: Some(TurnItemsView::Summary),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
thread.turns = page.data.into_iter().rev().collect();
|
||||
self.history_pagination.remove(&thread_id);
|
||||
history_notice = Some(
|
||||
"Showing up to 100 recent prompts and final replies. Intermediate messages and tool activity are unavailable.",
|
||||
);
|
||||
}
|
||||
let session = thread_session_state_from_thread_response(
|
||||
&thread.id,
|
||||
crate::windows_sandbox::host_from_environments(thread.environments.as_deref()),
|
||||
@@ -61,12 +97,15 @@ impl AppServerSession {
|
||||
)
|
||||
.await
|
||||
.map_err(color_eyre::eyre::Report::msg)?;
|
||||
Ok(AppServerStartedThread {
|
||||
session,
|
||||
turns: thread.turns,
|
||||
blocks_direct_input: false,
|
||||
task_tools_available: false,
|
||||
})
|
||||
Ok((
|
||||
AppServerStartedThread {
|
||||
session,
|
||||
turns: thread.turns,
|
||||
blocks_direct_input: false,
|
||||
task_tools_available: false,
|
||||
},
|
||||
history_notice,
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn with_local_codex_home(mut self, codex_home: &AbsolutePathBuf) -> Self {
|
||||
|
||||
@@ -214,7 +214,7 @@ async fn viewing_thread_reads_history_without_resuming_it() -> Result<()> {
|
||||
)?;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
let next_request_id = app_server.next_request_id;
|
||||
let viewed = app_server
|
||||
let (viewed, notice) = app_server
|
||||
.read_thread_for_viewing(
|
||||
&config,
|
||||
&crate::local_settings::LocalSettings::from(&config),
|
||||
@@ -222,6 +222,7 @@ async fn viewing_thread_reads_history_without_resuming_it() -> Result<()> {
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(notice, None);
|
||||
assert_eq!(viewed.session.thread_id, thread_id);
|
||||
assert!(!viewed.turns.is_empty());
|
||||
assert!(!viewed.blocks_direct_input);
|
||||
|
||||
Reference in New Issue
Block a user