mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Support renaming cloud threads (#61182)
## What changed Handle `ThreadSetName` requests in the cloud backend through the cloud client's new `set_thread_name` method, without opening an attachment. Trim the requested name and reject empty names or thread IDs. After the server acknowledges the rename, update the matching loaded thread's name and emit `ThreadNameUpdated`. Leave the cached name unchanged and emit no notification if the request fails. ## Testing Add tests for the rename request payload, server acknowledgment and error handling, and preservation of the loaded thread's name when a rename fails. GitOrigin-RevId: eecfe17a9cfba50c9fa9091a38a36c0011ab9246
This commit is contained in:
committed by
copyberry
parent
bbd0d2b2fb
commit
c432cf0253
@@ -8,9 +8,85 @@ use pretty_assertions::assert_eq;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_bytes;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejected_rename_preserves_loaded_cloud_thread_name() -> 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 mut requests = requests::Requests {
|
||||
client: std::sync::Arc::new(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(Box::new(serde_json::from_value(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": [], "name": "Old",
|
||||
},
|
||||
"model": "test", "modelProvider": "openai", "cwd": home.path(),
|
||||
"approvalPolicy": "never", "approvalsReviewer": "user",
|
||||
"sandbox": {"type": "readOnly"},
|
||||
}))?)),
|
||||
warned_partial_history: None,
|
||||
};
|
||||
Mock::given(method("POST"))
|
||||
.and(path(
|
||||
"/oaiproto.codex_app_server_backend.ThreadService/SetThreadName",
|
||||
))
|
||||
.and(body_bytes(b"\x00\x00\x00\x00\x08\x0a\x01t\x12\x03New"))
|
||||
.respond_with(ResponseTemplate::new(503))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let (output, mut notifications) = mpsc::unbounded_channel();
|
||||
let mut replies = Replies::default();
|
||||
let read = protocol::ThreadReadParams {
|
||||
thread_id: "t".into(),
|
||||
include_turns: false,
|
||||
};
|
||||
let rejected = handle_request(
|
||||
protocol::ClientRequest::ThreadSetName {
|
||||
request_id: protocol::RequestId::Integer(1),
|
||||
params: protocol::ThreadSetNameParams {
|
||||
thread_id: "t".into(),
|
||||
name: " New ".into(),
|
||||
},
|
||||
},
|
||||
cloud::SubmissionMode::StartIfIdle,
|
||||
/*project*/ None,
|
||||
&mut requests,
|
||||
&mut replies,
|
||||
&output,
|
||||
)
|
||||
.await;
|
||||
assert!(rejected.unwrap_err().is::<cloud::MutationError>());
|
||||
assert_eq!(
|
||||
requests.read_thread(&read, /*status*/ None)?["thread"]["name"],
|
||||
"Old"
|
||||
);
|
||||
assert!(notifications.try_recv().is_err());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_archive_loaded_list_tracks_attachment_not_cached_metadata() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -72,6 +72,25 @@ impl Requests {
|
||||
}
|
||||
serde_json::to_value(page)?
|
||||
}
|
||||
Request::ThreadSetName { params, .. } => {
|
||||
let name = params.name.trim();
|
||||
self.client.set_thread_name(¶ms.thread_id, name).await?;
|
||||
if let Some(loaded) = &mut self.loaded
|
||||
&& loaded.thread.id == params.thread_id
|
||||
{
|
||||
loaded.thread.name = Some(name.to_owned());
|
||||
}
|
||||
send_notification(
|
||||
output,
|
||||
protocol::ServerNotification::ThreadNameUpdated(
|
||||
protocol::ThreadNameUpdatedNotification {
|
||||
thread_id: params.thread_id,
|
||||
thread_name: Some(name.to_owned()),
|
||||
},
|
||||
),
|
||||
);
|
||||
serde_json::to_value(protocol::ThreadSetNameResponse {})?
|
||||
}
|
||||
Request::ThreadArchive { params, .. } => {
|
||||
self.client.archive_thread(¶ms.thread_id).await?;
|
||||
return Ok(Response::Archived(params.thread_id));
|
||||
|
||||
@@ -132,6 +132,33 @@ async fn archive_addresses_the_requested_thread_and_requires_acknowledgment() {
|
||||
assert!(fixture.requests.try_recv().is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rename_sends_the_thread_and_title_and_requires_acknowledgment() {
|
||||
let mut fixture = fixture_responses(vec![
|
||||
response(vec![frame(&[])], tonic::Status::ok("")),
|
||||
response(
|
||||
vec![frame(&[])],
|
||||
tonic::Status::permission_denied("synthetic denial"),
|
||||
),
|
||||
])
|
||||
.await;
|
||||
|
||||
fixture.client.set_thread_name("t", "New").await.unwrap();
|
||||
assert_request(&mut fixture, "SetThreadName", b"\x0a\x01t\x12\x03New").await;
|
||||
|
||||
let error = fixture
|
||||
.client
|
||||
.set_thread_name("u", "Other")
|
||||
.await
|
||||
.unwrap_err();
|
||||
let MutationError::OutcomeUnknown(Error::Rpc(status)) = error else {
|
||||
panic!("expected a remote rename failure");
|
||||
};
|
||||
assert_eq!(status.code, tonic::Code::PermissionDenied as i32);
|
||||
assert_request(&mut fixture, "SetThreadName", b"\x0a\x01u\x12\x05Other").await;
|
||||
assert!(fixture.requests.try_recv().is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn activation_preserves_native_settings_and_thread_metadata() {
|
||||
// Native Thread fields encoded independently of the client's wire types.
|
||||
|
||||
@@ -298,6 +298,25 @@ impl Client {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Saves a conversation name without opening an attachment.
|
||||
pub async fn set_thread_name(&self, thread_id: &str, name: &str) -> Result<(), MutationError> {
|
||||
if thread_id.trim().is_empty() || name.trim().is_empty() {
|
||||
return Err(MutationError::InvalidRequest);
|
||||
}
|
||||
let payload = self
|
||||
.mutate(
|
||||
wire::SetThreadNameRequest {
|
||||
thread_id: thread_id.to_owned(),
|
||||
name: name.to_owned(),
|
||||
},
|
||||
"/oaiproto.codex_app_server_backend.ThreadService/SetThreadName",
|
||||
)
|
||||
.await?;
|
||||
wire::EmptyResponse::decode(payload)
|
||||
.map_err(|_| MutationError::OutcomeUnknown(Error::InvalidResponse))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Archives a conversation using the current account, without opening an attachment.
|
||||
pub async fn archive_thread(&self, thread_id: &str) -> Result<(), MutationError> {
|
||||
if thread_id.trim().is_empty() {
|
||||
|
||||
@@ -16,6 +16,14 @@ pub(crate) struct ThreadIdRequest {
|
||||
pub thread_id: String,
|
||||
}
|
||||
|
||||
#[derive(Clone, prost::Message)]
|
||||
pub(crate) struct SetThreadNameRequest {
|
||||
#[prost(string, tag = "1")]
|
||||
pub thread_id: String,
|
||||
#[prost(string, tag = "2")]
|
||||
pub name: String,
|
||||
}
|
||||
|
||||
#[derive(Clone, prost::Message)]
|
||||
pub(crate) struct UnarchiveResponse {
|
||||
#[prost(bytes = "bytes", optional, tag = "1")]
|
||||
|
||||
Reference in New Issue
Block a user