-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Conversation naming #8991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation naming #8991
Changes from all commits
0a6d2d5
bfc7a3e
27bbc02
41e7e02
9f6aac5
4d6e184
3fd2ccc
bbab77f
a0f9f68
eebd0a2
32d4278
aa4d062
3186b17
8d57df0
d9c675b
1e568e6
b385eb8
2da5bf2
e62459e
b8de956
9de9d3e
61a0477
00ec8d1
4c32273
c12882c
cb22c11
8456127
b4c12d6
9369962
66dac4a
ebf5168
34648ea
842431f
e572fa4
7b7592e
7dce954
f943f6a
701b69b
b27b049
f6e40d5
8458bee
51b32f6
b859ba1
4aa8c6a
35e4a0d
84324c0
2eb7459
00c061f
99e0835
5d90138
38b9d8b
e71c628
6899728
e021ed2
112e5b6
e7abfec
9998ca8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,8 @@ use codex_app_server_protocol::ThreadReadResponse; | |
| use codex_app_server_protocol::ThreadResumeParams; | ||
| use codex_app_server_protocol::ThreadResumeResponse; | ||
| use codex_app_server_protocol::ThreadRollbackParams; | ||
| use codex_app_server_protocol::ThreadSetNameParams; | ||
| use codex_app_server_protocol::ThreadSetNameResponse; | ||
| use codex_app_server_protocol::ThreadSortKey; | ||
| use codex_app_server_protocol::ThreadSourceKind; | ||
| use codex_app_server_protocol::ThreadStartParams; | ||
|
|
@@ -416,6 +418,9 @@ impl CodexMessageProcessor { | |
| ClientRequest::ThreadArchive { request_id, params } => { | ||
| self.thread_archive(request_id, params).await; | ||
| } | ||
| ClientRequest::ThreadSetName { request_id, params } => { | ||
| self.thread_set_name(request_id, params).await; | ||
| } | ||
| ClientRequest::ThreadUnarchive { request_id, params } => { | ||
| self.thread_unarchive(request_id, params).await; | ||
| } | ||
|
|
@@ -1780,6 +1785,36 @@ impl CodexMessageProcessor { | |
| } | ||
| } | ||
|
|
||
| async fn thread_set_name(&self, request_id: RequestId, params: ThreadSetNameParams) { | ||
| let ThreadSetNameParams { thread_id, name } = params; | ||
| let Some(name) = codex_core::util::normalize_thread_name(&name) else { | ||
| self.send_invalid_request_error( | ||
| request_id, | ||
| "thread name must not be empty".to_string(), | ||
| ) | ||
| .await; | ||
| return; | ||
| }; | ||
|
|
||
| let (_, thread) = match self.load_thread(&thread_id).await { | ||
| Ok(v) => v, | ||
| Err(error) => { | ||
| self.outgoing.send_error(request_id, error).await; | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| if let Err(err) = thread.submit(Op::SetThreadName { name }).await { | ||
| self.send_internal_error(request_id, format!("failed to set thread name: {err}")) | ||
| .await; | ||
| return; | ||
| } | ||
|
|
||
| self.outgoing | ||
| .send_response(request_id, ThreadSetNameResponse {}) | ||
| .await; | ||
|
Comment on lines
+1813
to
+1815
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Useful? React with 👍 / 👎.
Comment on lines
+1813
to
+1815
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The app-server replies success immediately after enqueuing Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| async fn thread_unarchive(&mut self, request_id: RequestId, params: ThreadUnarchiveParams) { | ||
| // TODO(jif) mostly rewrite this using sqlite after phase 1 | ||
| let thread_id = match ThreadId::from_string(¶ms.thread_id) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handler sends a success response immediately after enqueueing
Op::SetThreadName, but the core implementation can still fail later (e.g., persistence disabled or a write error when appending to the session index), in which case it emits anErrorevent and never sendsThreadNameUpdated(seecodex-rs/core/src/codex.rsaround lines 2953–2979). That means JSON-RPC clients that rely on the response will think the rename succeeded even though it didn’t persist, and resume-by-name will silently fail. Consider waiting for theThreadNameUpdated/error event (similar tothread_rollback) or pre-checking persistence and returning an error response.Useful? React with 👍 / 👎.