mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
Support archiving and restoring cloud threads (#60635)
## What changed - Add cloud archive and unarchive requests and support listing archived threads. - Detach successfully archived threads and emit archive notifications. Report loaded cloud threads from live attachments rather than cached resume metadata, including after archive failures. - Scope resume picker archive state and transcript previews by backend and thread so archiving a cloud session preserves a local session with the same ID. - Limit cloud task archive cleanup to the selected thread, preserving child agents and their active session state, and update the confirmation text accordingly. ## Testing Add regression coverage for archive acknowledgments, archived-thread discovery and restoration, attachment state after archive failures, same-ID sessions across backends, and preservation of attached child agents. GitOrigin-RevId: bd155be85ee911375d4d326b18f9b5abe8e82b3d
This commit is contained in:
committed by
copyberry
parent
627b579219
commit
df82e2fe0a
@@ -5,6 +5,147 @@
|
||||
use super::*;
|
||||
use futures::stream;
|
||||
use pretty_assertions::assert_eq;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_archive_loaded_list_tracks_attachment_not_cached_metadata() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let home = tempfile::tempdir()?;
|
||||
let config = crate::legacy_core::config::ConfigBuilder::default()
|
||||
.codex_home(home.path().to_owned())
|
||||
.fallback_cwd(Some(home.path().to_owned()))
|
||||
.without_project_context()
|
||||
.build()
|
||||
.await?;
|
||||
let cached = Box::new(serde_json::from_value::<protocol::ThreadResumeResponse>(
|
||||
serde_json::json!({
|
||||
"thread": {
|
||||
"id": "t", "sessionId": "t", "preview": "", "ephemeral": false,
|
||||
"modelProvider": "openai", "createdAt": 0, "updatedAt": 0,
|
||||
"status": {"type": "idle"}, "cwd": home.path(), "cliVersion": "",
|
||||
"source": "cli", "turns": [],
|
||||
},
|
||||
"model": "test", "modelProvider": "openai", "cwd": home.path(),
|
||||
"approvalPolicy": "never", "approvalsReviewer": "user",
|
||||
"sandbox": {"type": "readOnly"},
|
||||
}),
|
||||
)?);
|
||||
let mut requests = requests::Requests {
|
||||
client: cloud::Client::new(
|
||||
&config.http_client_factory(),
|
||||
&server.uri(),
|
||||
cloud::Credentials::new("synthetic-bearer", "synthetic-account")?,
|
||||
)?,
|
||||
account: protocol::GetAccountResponse {
|
||||
user_id: None,
|
||||
account: None,
|
||||
requires_openai_auth: true,
|
||||
workspace_routing: None,
|
||||
},
|
||||
loaded: Some(cached.clone()),
|
||||
warned_partial_history: None,
|
||||
};
|
||||
let (output, mut notifications) = mpsc::unbounded_channel();
|
||||
let (sender, stream) = futures::channel::mpsc::unbounded();
|
||||
let mut replies = Replies::default();
|
||||
replies
|
||||
.replace("t", async { Ok(stream.boxed()) }, &output)
|
||||
.await?;
|
||||
Mock::given(method("POST"))
|
||||
.and(path(
|
||||
"/oaiproto.codex_app_server_backend.ThreadService/ArchiveThread",
|
||||
))
|
||||
.respond_with(ResponseTemplate::new(503))
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
// A rejection before shutdown must preserve both this attachment and an
|
||||
// unrelated attachment when the rejected archive targets another thread.
|
||||
for id in ["t", "other"] {
|
||||
assert!(
|
||||
handle_request(
|
||||
protocol::ClientRequest::ThreadArchive {
|
||||
request_id: protocol::RequestId::Integer(1),
|
||||
params: protocol::ThreadArchiveParams {
|
||||
thread_id: id.into(),
|
||||
},
|
||||
},
|
||||
cloud::SubmissionMode::StartIfIdle,
|
||||
/*project*/ None,
|
||||
&mut requests,
|
||||
&mut replies,
|
||||
&output,
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
let loaded = handle_request(
|
||||
protocol::ClientRequest::ThreadLoadedList {
|
||||
request_id: protocol::RequestId::Integer(2),
|
||||
params: Default::default(),
|
||||
},
|
||||
cloud::SubmissionMode::StartIfIdle,
|
||||
/*project*/ None,
|
||||
&mut requests,
|
||||
&mut replies,
|
||||
&output,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
loaded,
|
||||
serde_json::json!({"data": ["t"], "nextCursor": null})
|
||||
);
|
||||
replies.admit_turn("t", &output)?;
|
||||
assert!(notifications.try_recv().is_err());
|
||||
}
|
||||
|
||||
// Model a late archive failure winning while_listening's biased select
|
||||
// when Attach's EOF is also ready. No worker poll has observed the EOF yet.
|
||||
drop(sender);
|
||||
let failed: Result<()> = replies
|
||||
.while_listening(
|
||||
async { Err(eyre!("archive failed after shutdown")) },
|
||||
&output,
|
||||
)
|
||||
.await;
|
||||
assert!(failed.is_err());
|
||||
assert!(replies.active.is_some());
|
||||
// The first query must notice EOF itself; the second sees an already
|
||||
// observed disconnect. Neither may use the cached resume response.
|
||||
for _ in 0..2 {
|
||||
let loaded = handle_request(
|
||||
protocol::ClientRequest::ThreadLoadedList {
|
||||
request_id: protocol::RequestId::Integer(3),
|
||||
params: Default::default(),
|
||||
},
|
||||
cloud::SubmissionMode::StartIfIdle,
|
||||
/*project*/ None,
|
||||
&mut requests,
|
||||
&mut replies,
|
||||
&output,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(loaded, serde_json::json!({"data": [], "nextCursor": null}));
|
||||
assert_eq!(requests.loaded.as_ref(), Some(&cached));
|
||||
}
|
||||
let AppServerEvent::ThreadStreamDisconnected(error) = notifications.try_recv()? else {
|
||||
panic!("expected the existing stream-loss recovery notification");
|
||||
};
|
||||
assert_eq!(
|
||||
*error,
|
||||
cloud_error(
|
||||
"t",
|
||||
"Cloud stream ended. Reopen the conversation to reconnect."
|
||||
)
|
||||
);
|
||||
assert!(notifications.try_recv().is_err());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_replacement_keeps_old_thread_until_new_attachment_succeeds() -> Result<()> {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
//! Adapts native Cloud requests and the currently open conversation’s reply stream.
|
||||
//! A replacement becomes active only after its native attachment opens successfully.
|
||||
//! Loaded-thread queries report reply attachments, not retained resume metadata.
|
||||
|
||||
mod auth;
|
||||
mod events;
|
||||
@@ -252,6 +253,15 @@ async fn handle_request(
|
||||
replies: &mut Replies,
|
||||
output: &mpsc::UnboundedSender<AppServerEvent>,
|
||||
) -> Result<Value> {
|
||||
if let protocol::ClientRequest::ThreadLoadedList { .. } = &request {
|
||||
// The archive RPC can fail after closing Attach. Observe a ready EOF even
|
||||
// when the RPC result won the worker's select before the stream did.
|
||||
let _ = tokio::task::unconstrained(replies.forward_next(output)).now_or_never();
|
||||
return Ok(serde_json::json!({
|
||||
"data": replies.active.iter().map(|active| &active.thread_id).collect::<Vec<_>>(),
|
||||
"nextCursor": null,
|
||||
}));
|
||||
}
|
||||
if let protocol::ClientRequest::ThreadRead { params, .. } = &request {
|
||||
return requests.read_thread(params, replies.thread_status(¶ms.thread_id));
|
||||
}
|
||||
@@ -277,6 +287,24 @@ async fn handle_request(
|
||||
.await?
|
||||
{
|
||||
requests::Response::Value(value) => Ok(value),
|
||||
requests::Response::Archived(thread_id) => {
|
||||
replies.detach(&thread_id);
|
||||
replies.stream_losses.supersede(&thread_id);
|
||||
if requests
|
||||
.loaded
|
||||
.as_ref()
|
||||
.is_some_and(|loaded| loaded.thread.id == thread_id)
|
||||
{
|
||||
requests.loaded = None;
|
||||
}
|
||||
send_notification(
|
||||
output,
|
||||
protocol::ServerNotification::ThreadArchived(
|
||||
protocol::ThreadArchivedNotification { thread_id },
|
||||
),
|
||||
);
|
||||
Ok(serde_json::to_value(protocol::ThreadArchiveResponse {})?)
|
||||
}
|
||||
requests::Response::Attach(settings) => {
|
||||
let id = &settings.thread.id;
|
||||
let result = serde_json::to_value(&settings)?;
|
||||
|
||||
@@ -7,7 +7,7 @@ use pretty_assertions::assert_eq;
|
||||
fn ordinary_multi_backend_resume_queries_cloud_interactive_threads_on_every_page() {
|
||||
// The mixed picker has no shared cwd/provider filter. It sends conventional
|
||||
// interactive source names, even when the connection belongs to Cloud.
|
||||
for (cursor, state_db_only) in [(None, true), (Some("page-2"), false)] {
|
||||
for (cursor, state_db_only, archived) in [(None, true, false), (Some("page-2"), false, true)] {
|
||||
let params = serde_json::from_value(json!({
|
||||
"currentAccountOnly": true,
|
||||
"cursor": cursor,
|
||||
@@ -15,13 +15,14 @@ fn ordinary_multi_backend_resume_queries_cloud_interactive_threads_on_every_page
|
||||
"sortKey": "updated_at",
|
||||
"modelProviders": [],
|
||||
"sourceKinds": ["cli", "vscode"],
|
||||
"archived": false,
|
||||
"archived": archived,
|
||||
"useStateDbOnly": state_db_only,
|
||||
}))
|
||||
.unwrap();
|
||||
let query = thread_list_query(params).unwrap();
|
||||
assert_eq!(query.cursor.as_deref(), cursor);
|
||||
assert_eq!(query.limit, Some(25));
|
||||
assert_eq!(query.archived, archived);
|
||||
assert!(matches!(query.sort_key, cloud::ThreadSortKey::UpdatedAt));
|
||||
assert!(matches!(
|
||||
query.sort_direction,
|
||||
@@ -34,7 +35,6 @@ fn ordinary_multi_backend_resume_queries_cloud_interactive_threads_on_every_page
|
||||
#[test]
|
||||
fn unsupported_filters_do_not_return_unrelated_cloud_rows() {
|
||||
for filter in [
|
||||
json!({"archived": true}),
|
||||
json!({"sourceKinds": ["exec"]}),
|
||||
json!({"attachedToThreadId": "other"}),
|
||||
json!({"modelProviders": ["other"]}),
|
||||
|
||||
@@ -28,6 +28,7 @@ pub(super) struct Requests {
|
||||
pub(super) enum Response {
|
||||
Value(Value),
|
||||
Attach(Box<protocol::ThreadResumeResponse>),
|
||||
Archived(String),
|
||||
}
|
||||
|
||||
impl Requests {
|
||||
@@ -70,6 +71,24 @@ impl Requests {
|
||||
}
|
||||
serde_json::to_value(page)?
|
||||
}
|
||||
Request::ThreadArchive { params, .. } => {
|
||||
self.client.archive_thread(¶ms.thread_id).await?;
|
||||
return Ok(Response::Archived(params.thread_id));
|
||||
}
|
||||
Request::ThreadUnarchive { params, .. } => {
|
||||
let mut thread = self.client.unarchive_thread(¶ms.thread_id).await?;
|
||||
use_paginated_history(&mut thread);
|
||||
let value = json!({ "thread": thread });
|
||||
send_notification(
|
||||
output,
|
||||
protocol::ServerNotification::ThreadUnarchived(
|
||||
protocol::ThreadUnarchivedNotification {
|
||||
thread_id: params.thread_id,
|
||||
},
|
||||
),
|
||||
);
|
||||
value
|
||||
}
|
||||
Request::ThreadStart { mut params, .. } => {
|
||||
let model = params.model.take();
|
||||
let provider = params.model_provider.take();
|
||||
@@ -98,14 +117,6 @@ impl Requests {
|
||||
let settings = self.client.activate(¶ms.thread_id).await?;
|
||||
return attach_response(settings);
|
||||
}
|
||||
Request::ThreadLoadedList { .. } => {
|
||||
let ids = self
|
||||
.loaded
|
||||
.iter()
|
||||
.map(|loaded| &loaded.thread.id)
|
||||
.collect::<Vec<_>>();
|
||||
json!({"data": ids, "nextCursor": null})
|
||||
}
|
||||
Request::ThreadTurnsList { params, .. } => {
|
||||
if params.items_view != Some(protocol::TurnItemsView::NotLoaded) {
|
||||
bail!("Read cloud turn items separately with thread/items/list.");
|
||||
@@ -246,7 +257,6 @@ fn thread_list_query(params: protocol::ThreadListParams) -> Result<cloud::Thread
|
||||
// Its explicit CLI/VS Code defaults are conventional source names.
|
||||
let sources = params.source_kinds.as_deref().unwrap_or_default();
|
||||
if !matches!(sources, [] | [Cli, VsCode] | [VsCode, Cli])
|
||||
|| params.archived == Some(true)
|
||||
|| params.cwd.is_some()
|
||||
|| params.search_term.is_some()
|
||||
|| params.section_id.is_some()
|
||||
@@ -268,6 +278,7 @@ fn thread_list_query(params: protocol::ThreadListParams) -> Result<cloud::Thread
|
||||
Ok(cloud::ThreadListQuery {
|
||||
cursor: params.cursor,
|
||||
limit: params.limit,
|
||||
archived: params.archived.unwrap_or_default(),
|
||||
originators: params.originators.unwrap_or_default(),
|
||||
sort_key: match params.sort_key {
|
||||
Some(protocol::ThreadSortKey::CreatedAt) | None => cloud::ThreadSortKey::CreatedAt,
|
||||
|
||||
Reference in New Issue
Block a user