fix(tui_app_server): fix remote subagent switching and agent names#15513
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores remote subagent hotkey switching in tui_app_server by rebuilding the in-memory agent navigation state from threads that are already loaded on the app-server, and by retrying next/previous navigation after an on-demand rescan.
Changes:
- Add logic to reconstruct a loaded subagent thread tree for a given primary thread.
- Backfill loaded subagent threads into the TUI navigation state on attach and on hotkey-navigation misses.
- Add a new
thread/loaded/listwrapper toAppServerSessionand a unit test for loaded subagent tree reconstruction.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
codex-rs/tui_app_server/src/app/loaded_threads.rs |
New helper to find loaded descendant subagent threads from a primary thread, with unit test. |
codex-rs/tui_app_server/src/app.rs |
Backfill loaded subagent threads into navigation state and retry agent navigation hotkeys after rescan. |
codex-rs/tui_app_server/src/app_server_session.rs |
Add thread_loaded_list request wrapper for the new backfill flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fcoury
force-pushed
the
fix/tui-app-server-remote-agent-switch
branch
2 times, most recently
from
b494707 to
f7c0d91
Compare
|
@codex review |
![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: f7c0d91da8
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 284a6d45c6
ℹ️ 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".
fcoury
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da1a089b7b
ℹ️ 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".
fcoury
force-pushed
the
fix/tui-app-server-remote-agent-switch
branch
from
86d1113 to
6e348f5
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e348f504b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e348f504b
ℹ️ 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".
etraut-openai
left a comment
etraut-openai
left a comment
There was a problem hiding this comment.
Looks good. Just a few questions and comments.
Fix remote subagent hotkey switching and collab agent names when using the app server. - backfill loaded subagent threads after resume/fork and on navigation fallback - hydrate collab receiver metadata before rendering tool-call history - reseed ChatWidget metadata when replacing the widget for replay - add coverage for replayed collab items keeping agent names
fcoury
force-pushed
the
fix/tui-app-server-remote-agent-switch
branch
2 times, most recently
from
863ec1a to
314fa06
Compare
Clarify the `loaded_threads` module comment so it describes the behavior generically when resuming or switching threads. This avoids implying a remote-specific client code path when the logic should be agnostic to transport.
Just addressed all your comments. Thanks @etraut-openai |
TL;DR
This PR changes the
tui_app_serverpath in the following ways:Cmd/Alt+Arrowsnavigation between agent conversationsProblem
When the TUI connects to a remote app server, collab agent tool-call items (spawn, wait, delegate, etc.) render thread UUIDs instead of human-readable agent names because the
ChatWidgetnever receives nickname/role metadata for receiver threads. Separately, keyboard next/previous agent navigation silently does nothing when the localAgentNavigationStatecache has not yet been populated with subagent threads that the remote server already knows about.Both issues share a root cause: in the remote (app-server) code path the TUI never proactively fetches thread metadata. In the local code path this metadata arrives naturally via spawn events the TUI itself orchestrates, but in the remote path those events were processed by a different client and the TUI only sees the resulting collab tool-call notifications.
Mental model
Collab agent tool-call notifications reference receiver threads by id, but carry no nickname or role. The TUI needs that metadata in two places:
ChatWidgetconvertsCollabAgentToolCallitems into history cells. Without metadata, agent status lines show raw UUIDs.AgentNavigationStatetracks known threads for the/agentpicker and keyboard cycling. Without entries for remote subagents, next/previous has nowhere to go.This change closes the gap with two complementary strategies:
receiver_thread_ids, the TUI fetches metadata (thread/read) for threads it has not yet cached before the notification is rendered.thread/loaded/list, walks the parent-child spawn tree, and registers every descendant subagent in both the navigation cache and theChatWidgetmetadata map.A new
collab_agent_metadataside-table inChatWidgetstores nickname/role keyed byThreadId, kept in sync byAppwhenever it callsupsert_agent_picker_thread. Thereplace_chat_widgethelper re-seeds this map fromAgentNavigationStateso that thread switches (which reconstruct the widget) do not lose previously discovered metadata.Non-goals
thread/readandthread/loaded/listRPCs.AgentNavigationStateorders or cycles through threads. The traversal logic is unchanged; only the population of entries is extended.Tradeoffs
hydrate_collab_agent_metadata_for_notificationissues athread/readfor each unknown receiver thread before the notification is forwarded to rendering. This adds latency on the notification path but only fires once per thread (the result is cached). The alternative -- rendering first and backfilling names later -- would cause visible flicker as UUIDs are replaced with names.backfill_loaded_subagent_threadsfetches the full loaded-thread list and walks the spawn tree even when the user may only care about one subagent. This is simple and correct but O(loaded_threads) per thread switch. For typical session sizes this is negligible; it could become a concern for sessions with hundreds of subagents.AgentNavigationState(for picker/label) andChatWidget::collab_agent_metadata(for rendering). The two are kept in sync throughupsert_agent_picker_threadandreplace_chat_widget, but there is no compile-time enforcement of this coupling.Architecture
New module:
app::loaded_threadsPure function
find_loaded_subagent_threads_for_primarythat takes a flat list ofThreadobjects and a primary thread id, then walks theSessionSource::SubAgentparent-child edges to collect all transitive descendants. Returns a sorted vec ofLoadedSubagentThread(thread_id + nickname + role). No async, no side effects -- designed for unit testing.New methods on
Appcollab_receiver_thread_idsreceiver_thread_idsfromItemStarted/ItemCompletedcollab notificationshydrate_collab_agent_metadata_for_notificationbackfill_loaded_subagent_threadsadjacent_thread_id_with_backfillreplace_chat_widgetAgentNavigationStateNew state in
ChatWidgetcollab_agent_metadata: HashMap<ThreadId, CollabAgentMetadata>-- a lookup table that rendering functions consult to attach human-readable names to collab tool-call items. Populated externally byAppviaset_collab_agent_metadata.New method on
AppServerSessionthread_loaded_list-- thin wrapper aroundClientRequest::ThreadLoadedList.Observability
tracing::warnon invalid thread ids during hydration and backfill.tracing::warnon failedthread/readorthread/loaded/listRPCs (with thread id and error).Tests
loaded_threads::tests::finds_loaded_subagent_tree_for_primary_thread-- unit test for the spawn-tree walk: verifies child and grandchild are included, unrelated threads are excluded, and metadata is carried through.app::tests::replace_chat_widget_reseeds_collab_agent_metadata_for_replay-- integration test that creates aChatWidget, replaces it viareplace_chat_widget, replays a collab wait notification, and asserts the rendered history cell contains the agent name rather than a UUID.app_server_collab_wait_items_render_history-- the existing collab wait rendering test now sets metadata before sending notifications, so the snapshot showsRobie [explorer]/Ada [reviewer]instead of raw thread ids.