app-server: add v2 APIs for TUI #10298
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccd0e29421
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub(crate) fn process_client_error(&mut self, id: RequestId, error: JSONRPCErrorError) { | ||
| tracing::error!("<- error: {:?}", JSONRPCError { id, error }); | ||
| } |
There was a problem hiding this comment.
In the new typed in-process path (spawn_in_memory_typed), an AppServerClientMessage::Error is routed to process_client_error, but that method only logs and never calls outgoing.notify_client_error. When the server has an outstanding request waiting on a oneshot (created in OutgoingMessageSender::send_request_with_id), a client-side error response will leave the callback registered forever, so the awaiting task never resolves and the request map leaks until shutdown. This only occurs for typed clients that send the Error variant, but in that case it will hang any caller awaiting the response.
Useful? React with 👍 / 👎.
| params: v2::ThreadCompactParams, | ||
| response: v2::ThreadCompactResponse, | ||
| }, | ||
| ThreadShutdown => "thread/shutdown" { |
There was a problem hiding this comment.
ooc can we reuse turn/interrupt ?
This is the piece of the original #10192 that adds the APIs we need to add to app server to support the TUI.
Perhaps I should wait until #10231 is in so I can update these with the new
#[experimental]annotation?