mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Respect paginated history when stopping background tasks (#40787)
## Why Stopping a task from the agents overview may need to look up its active turn. Paginated threads should use the turn history endpoint instead of requesting embedded full history. ## What changed - Read the thread without turns first to determine its history mode. - Fetch the first turn page for paginated threads when the app server supports it. - Fall back to embedded turn history for legacy threads and legacy server negotiation. ## Testing Added coverage for paginated and legacy threads, including fallback after legacy capability negotiation. GitOrigin-RevId: 44d506b19e32c6b3c27fac11fb748c23b8e47b5e
This commit is contained in:
@@ -12,6 +12,7 @@ use crate::chatwidget::ThreadInputStateRestoreMode;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::Thread;
|
||||
use codex_app_server_protocol::ThreadHistoryMode;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadLoadedListParams;
|
||||
use codex_app_server_protocol::ThreadLoadedListResponse;
|
||||
@@ -666,15 +667,34 @@ impl App {
|
||||
) {
|
||||
let active_turn_id = match self.active_turn_id_for_thread(thread_id).await {
|
||||
Some(turn_id) => Some(turn_id),
|
||||
None => match app_server
|
||||
.thread_read(thread_id, /*include_turns*/ true)
|
||||
.await
|
||||
None => match async {
|
||||
let thread = app_server
|
||||
.thread_read(thread_id, /*include_turns*/ false)
|
||||
.await?;
|
||||
let turns = match thread.history_mode {
|
||||
ThreadHistoryMode::Paginated if app_server.supports_paginated_history() => {
|
||||
app_server
|
||||
.thread_turns_page(thread_id, /*cursor*/ None)
|
||||
.await?
|
||||
.data
|
||||
}
|
||||
ThreadHistoryMode::Legacy | ThreadHistoryMode::Paginated => {
|
||||
app_server
|
||||
.thread_read(thread_id, /*include_turns*/ true)
|
||||
.await?
|
||||
.turns
|
||||
}
|
||||
};
|
||||
Ok::<_, color_eyre::Report>(
|
||||
turns
|
||||
.into_iter()
|
||||
.find(|turn| turn.status == TurnStatus::InProgress)
|
||||
.map(|turn| turn.id),
|
||||
)
|
||||
}
|
||||
.await
|
||||
{
|
||||
Ok(thread) => thread
|
||||
.turns
|
||||
.into_iter()
|
||||
.find(|turn| turn.status == TurnStatus::InProgress)
|
||||
.map(|turn| turn.id),
|
||||
Ok(turn_id) => turn_id,
|
||||
Err(error) => {
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to stop background task: {error}"));
|
||||
|
||||
@@ -2265,6 +2265,92 @@ async fn paginated_workflows_never_request_full_thread_history() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn agents_overview_stop_uses_history_mode_for_turn_lookup() -> Result<()> {
|
||||
let (mut app, _codex_home) = make_history_test_app().await?;
|
||||
let paginated_thread_id = create_history_rollout(
|
||||
&app.config,
|
||||
ThreadHistoryMode::Paginated,
|
||||
"paginated background task",
|
||||
)?;
|
||||
let cases = [
|
||||
(paginated_thread_id, vec![false], 1),
|
||||
(
|
||||
create_history_rollout(
|
||||
&app.config,
|
||||
ThreadHistoryMode::Legacy,
|
||||
"legacy background task",
|
||||
)?,
|
||||
vec![false, true],
|
||||
0,
|
||||
),
|
||||
];
|
||||
let (mut app_server, requests, proxy) = start_recording_app_server_with_history(
|
||||
&app.config,
|
||||
HistoryCapabilities::Current,
|
||||
/*blocked_thread_list*/ None,
|
||||
/*failed_thread_name*/ None,
|
||||
crate::app_server_session::ThreadParamsMode::Embedded,
|
||||
)
|
||||
.await?;
|
||||
|
||||
for (thread_id, expected_include_turns, expected_turn_page_count) in cases {
|
||||
let previous_reads = recorded_params(&requests, "thread/read");
|
||||
let previous_turn_page_count = recorded_params(&requests, "thread/turns/list").len();
|
||||
|
||||
app.stop_agents_overview_thread(&mut app_server, thread_id)
|
||||
.await;
|
||||
|
||||
let reads = recorded_params(&requests, "thread/read");
|
||||
let include_turns = reads[previous_reads.len()..]
|
||||
.iter()
|
||||
.map(|params| params["includeTurns"].as_bool().unwrap_or(false))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(include_turns, expected_include_turns);
|
||||
assert_eq!(
|
||||
recorded_params(&requests, "thread/turns/list").len() - previous_turn_page_count,
|
||||
expected_turn_page_count
|
||||
);
|
||||
}
|
||||
|
||||
app_server.shutdown().await?;
|
||||
proxy.await??;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn agents_overview_stop_uses_full_history_after_legacy_negotiation() -> Result<()> {
|
||||
let (mut app, _codex_home) = make_history_test_app().await?;
|
||||
let thread_id = create_history_rollout(
|
||||
&app.config,
|
||||
ThreadHistoryMode::Paginated,
|
||||
"paginated background task",
|
||||
)?;
|
||||
let (mut app_server, requests, proxy) = start_recording_app_server_with_history(
|
||||
&app.config,
|
||||
HistoryCapabilities::LegacyOnly,
|
||||
/*blocked_thread_list*/ None,
|
||||
/*failed_thread_name*/ None,
|
||||
crate::app_server_session::ThreadParamsMode::Embedded,
|
||||
)
|
||||
.await?;
|
||||
app_server.start_thread(&app.config).await?;
|
||||
|
||||
app.stop_agents_overview_thread(&mut app_server, thread_id)
|
||||
.await;
|
||||
|
||||
let include_turns = recorded_params(&requests, "thread/read")
|
||||
.into_iter()
|
||||
.map(|params| params["includeTurns"].as_bool().unwrap_or(false))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(include_turns, vec![false, true]);
|
||||
assert!(recorded_params(&requests, "thread/turns/list").is_empty());
|
||||
|
||||
app_server.shutdown().await?;
|
||||
proxy.await??;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_paginated_subagent_transcript_excludes_inherited_parent_history() -> Result<()> {
|
||||
let (app, codex_home) = make_history_test_app().await?;
|
||||
|
||||
@@ -652,6 +652,10 @@ impl AppServerSession {
|
||||
self.managed_new_thread_defaults.as_ref()
|
||||
}
|
||||
|
||||
pub(crate) fn supports_paginated_history(&self) -> bool {
|
||||
self.history_support == ThreadHistorySupport::Paginated
|
||||
}
|
||||
|
||||
/// Fetches the current account info without refreshing the auth token.
|
||||
///
|
||||
/// Used by both `bootstrap` (to populate the initial UI) and `get_login_status`
|
||||
|
||||
Reference in New Issue
Block a user