mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
add turn items view to app-server turns (#21063)
## Why `Turn.items` currently overloads an empty array to mean either that no items exist or that the server intentionally did not load them for this response. That ambiguity blocks future lazy-loading work where clients need to distinguish unloaded, summary, and fully hydrated turn payloads. ## What changed - add a new `TurnItemsView` enum with `notLoaded`, `summary`, and `full` variants - add required `itemsView` metadata to app-server `Turn` payloads - mark reconstructed persisted history as `full` and live shell-style turn payloads as `notLoaded` - keep current `thread/turns/list` behavior unchanged and document that it still returns `full` turns today - regenerate the JSON and TypeScript protocol fixtures ## Verification - `just write-app-server-schema` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-app-server thread_read_can_include_turns` - `cargo test -p codex-app-server thread_turns_list_can_page_backward_and_forward` - `cargo test -p codex-app-server thread_resume_rejects_history_when_thread_is_running` - `just fix -p codex-app-server-protocol` - `just fix -p codex-app-server` - `just fmt`
This commit is contained in:
@@ -75,6 +75,7 @@ use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnDiffUpdatedNotification;
|
||||
use codex_app_server_protocol::TurnError;
|
||||
use codex_app_server_protocol::TurnInterruptResponse;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_app_server_protocol::TurnPlanStep;
|
||||
use codex_app_server_protocol::TurnPlanUpdatedNotification;
|
||||
use codex_app_server_protocol::TurnStartedNotification;
|
||||
@@ -157,15 +158,19 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.await;
|
||||
let turn = {
|
||||
let state = thread_state.lock().await;
|
||||
state.active_turn_snapshot().unwrap_or_else(|| Turn {
|
||||
let mut turn = state.active_turn_snapshot().unwrap_or_else(|| Turn {
|
||||
id: payload.turn_id.clone(),
|
||||
items: Vec::new(),
|
||||
items_view: TurnItemsView::NotLoaded,
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
started_at: payload.started_at,
|
||||
completed_at: None,
|
||||
duration_ms: None,
|
||||
})
|
||||
});
|
||||
turn.items.clear();
|
||||
turn.items_view = TurnItemsView::NotLoaded;
|
||||
turn
|
||||
};
|
||||
let notification = TurnStartedNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
@@ -1305,6 +1310,7 @@ async fn emit_turn_completed_with_status(
|
||||
turn: Turn {
|
||||
id: event_turn_id,
|
||||
items: vec![],
|
||||
items_view: TurnItemsView::NotLoaded,
|
||||
error: turn_completion_metadata.error,
|
||||
status: turn_completion_metadata.status,
|
||||
started_at: turn_completion_metadata.started_at,
|
||||
@@ -3198,6 +3204,91 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_started_omits_active_snapshot_items() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config = load_default_config_for_test(&codex_home).await;
|
||||
let thread_manager = Arc::new(
|
||||
codex_core::test_support::thread_manager_with_models_provider_and_home(
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
config.model_provider.clone(),
|
||||
config.codex_home.to_path_buf(),
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
),
|
||||
);
|
||||
let codex_core::NewThread {
|
||||
thread_id: conversation_id,
|
||||
thread: conversation,
|
||||
..
|
||||
} = thread_manager.start_thread(config.clone()).await?;
|
||||
let thread_state = new_thread_state();
|
||||
{
|
||||
let mut state = thread_state.lock().await;
|
||||
state.track_current_turn_event(
|
||||
"turn-1",
|
||||
&EventMsg::TurnStarted(codex_protocol::protocol::TurnStartedEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at: Some(42),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
);
|
||||
state.track_current_turn_event(
|
||||
"turn-1",
|
||||
&EventMsg::UserMessage(codex_protocol::protocol::UserMessageEvent {
|
||||
message: "already tracked".to_string(),
|
||||
images: None,
|
||||
local_images: Vec::new(),
|
||||
text_elements: Vec::new(),
|
||||
}),
|
||||
);
|
||||
}
|
||||
let thread_watch_manager = ThreadWatchManager::new();
|
||||
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(
|
||||
tx,
|
||||
codex_analytics::AnalyticsEventsClient::disabled(),
|
||||
));
|
||||
let outgoing = ThreadScopedOutgoingMessageSender::new(
|
||||
outgoing,
|
||||
vec![ConnectionId(1)],
|
||||
conversation_id,
|
||||
);
|
||||
|
||||
apply_bespoke_event_handling(
|
||||
Event {
|
||||
id: "turn-1".to_string(),
|
||||
msg: EventMsg::TurnStarted(codex_protocol::protocol::TurnStartedEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at: Some(42),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
},
|
||||
conversation_id,
|
||||
conversation,
|
||||
thread_manager,
|
||||
/*analytics_events_client*/ None,
|
||||
outgoing,
|
||||
thread_state,
|
||||
thread_watch_manager,
|
||||
Arc::new(tokio::sync::Semaphore::new(/*permits*/ 1)),
|
||||
"test-provider".to_string(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let msg = recv_broadcast_message(&mut rx).await?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnStarted(n)) => {
|
||||
assert_eq!(n.turn.id, "turn-1");
|
||||
assert_eq!(n.turn.items_view, TurnItemsView::NotLoaded);
|
||||
assert!(n.turn.items.is_empty());
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_turn_complete_emits_completed_without_error() -> Result<()> {
|
||||
let conversation_id = ThreadId::new();
|
||||
@@ -3245,6 +3336,8 @@ mod tests {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_turn_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Completed);
|
||||
assert_eq!(n.turn.items_view, TurnItemsView::NotLoaded);
|
||||
assert!(n.turn.items.is_empty());
|
||||
assert_eq!(n.turn.error, None);
|
||||
assert_eq!(n.turn.started_at, Some(42));
|
||||
assert_eq!(n.turn.completed_at, Some(TEST_TURN_COMPLETED_AT));
|
||||
|
||||
@@ -732,6 +732,7 @@ mod tests {
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -961,6 +962,7 @@ mod tests {
|
||||
turn: Turn {
|
||||
id: "turn-1".to_string(),
|
||||
items: Vec::new(),
|
||||
items_view: TurnItemsView::NotLoaded,
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: None,
|
||||
|
||||
@@ -222,6 +222,7 @@ use codex_app_server_protocol::TurnEnvironmentParams;
|
||||
use codex_app_server_protocol::TurnError;
|
||||
use codex_app_server_protocol::TurnInterruptParams;
|
||||
use codex_app_server_protocol::TurnInterruptResponse;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
|
||||
@@ -52,6 +52,7 @@ mod thread_processor_behavior_tests {
|
||||
use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_app_server_protocol::ServerRequestPayload;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ToolRequestUserInputParams;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::LoaderOverrides;
|
||||
@@ -205,6 +206,7 @@ mod thread_processor_behavior_tests {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}],
|
||||
items_view: TurnItemsView::Full,
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
started_at: None,
|
||||
|
||||
@@ -502,6 +502,7 @@ impl TurnRequestProcessor {
|
||||
let turn = Turn {
|
||||
id: turn_id,
|
||||
items: vec![],
|
||||
items_view: TurnItemsView::NotLoaded,
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
started_at: None,
|
||||
@@ -807,6 +808,7 @@ impl TurnRequestProcessor {
|
||||
Turn {
|
||||
id: turn_id,
|
||||
items,
|
||||
items_view: TurnItemsView::NotLoaded,
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
started_at: None,
|
||||
@@ -981,7 +983,7 @@ impl TurnRequestProcessor {
|
||||
request_id,
|
||||
parent_thread,
|
||||
review_request,
|
||||
display_text.as_str(),
|
||||
&display_text,
|
||||
thread_id,
|
||||
)
|
||||
.await?;
|
||||
@@ -992,7 +994,7 @@ impl TurnRequestProcessor {
|
||||
parent_thread_id,
|
||||
parent_thread,
|
||||
review_request,
|
||||
display_text.as_str(),
|
||||
&display_text,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user