-
Notifications
You must be signed in to change notification settings - Fork 7.2k
feat: add auto refresh on thread listeners #9105
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
# Conflicts: # codex-rs/app-server/tests/suite/send_message.rs # codex-rs/core/src/thread_manager.rs
codex-rs/app-server/tests/suite/codex_message_processor_flow.rs
Outdated
Show resolved
Hide resolved
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 969e384b15
ℹ️ 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".
| } | ||
|
|
||
| /// Ensure we have a listener for thread_id | ||
| pub(crate) async fn ensure_thread_listener(&mut self, thread_id: ThreadId) { |
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.
It's a bit of a smell that this is called "ensure" if all it does is warn on error. Perhaps we could better comment under what circumstances attach_conversation_listener fails?
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.
In theory in should never fail in this scenario but will do
This PR is in the scope of multi-agent work.
An agent (=thread) can now spawn other agents. Those other agents are not attached to any clients. We need a way to make sure that the clients are aware of the new threads to look at (for approval for example). This PR adds a channel to the
ThreadManagerthat pushes the ID of those newly created agents such that the client (here the app-server) can also subscribe to those ones.