mirror of
https://github.com/openai/codex.git
synced 2026-05-02 02:17:22 +00:00
79359fb5e7
## TL;DR This PR changes the `tui_app_server` _path_ in the following ways: - add missing feature to show agent names (shows only UUIDs today) - add `Cmd/Alt+Arrows` navigation between agent conversations ## Problem 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 `ChatWidget` never receives nickname/role metadata for receiver threads. Separately, keyboard next/previous agent navigation silently does nothing when the local `AgentNavigationState` cache 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: 1. **Rendering** -- `ChatWidget` converts `CollabAgentToolCall` items into history cells. Without metadata, agent status lines show raw UUIDs. 2. **Navigation** -- `AgentNavigationState` tracks known threads for the `/agent` picker and keyboard cycling. Without entries for remote subagents, next/previous has nowhere to go. This change closes the gap with two complementary strategies: - **Eager hydration**: when any notification carries `receiver_thread_ids`, the TUI fetches metadata (`thread/read`) for threads it has not yet cached before the notification is rendered. - **Backfill on thread switch**: when the user resumes, forks, or starts a new app-server thread, the TUI fetches the full `thread/loaded/list`, walks the parent-child spawn tree, and registers every descendant subagent in both the navigation cache and the `ChatWidget` metadata map. A new `collab_agent_metadata` side-table in `ChatWidget` stores nickname/role keyed by `ThreadId`, kept in sync by `App` whenever it calls `upsert_agent_picker_thread`. The `replace_chat_widget` helper re-seeds this map from `AgentNavigationState` so that thread switches (which reconstruct the widget) do not lose previously discovered metadata. ## Non-goals - This change does not alter the local (non-app-server) collab code path. That path already receives metadata via spawn events and is unaffected. - No new protocol messages are introduced. The change uses existing `thread/read` and `thread/loaded/list` RPCs. - No changes to how `AgentNavigationState` orders or cycles through threads. The traversal logic is unchanged; only the population of entries is extended. ## Tradeoffs - **Extra RPCs on notification path**: `hydrate_collab_agent_metadata_for_notification` issues a `thread/read` for 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 fetches all loaded threads**: `backfill_loaded_subagent_threads` fetches 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. - **Metadata duplication**: agent nickname/role is now stored in both `AgentNavigationState` (for picker/label) and `ChatWidget::collab_agent_metadata` (for rendering). The two are kept in sync through `upsert_agent_picker_thread` and `replace_chat_widget`, but there is no compile-time enforcement of this coupling. ## Architecture ### New module: `app::loaded_threads` Pure function `find_loaded_subagent_threads_for_primary` that takes a flat list of `Thread` objects and a primary thread id, then walks the `SessionSource::SubAgent` parent-child edges to collect all transitive descendants. Returns a sorted vec of `LoadedSubagentThread` (thread_id + nickname + role). No async, no side effects -- designed for unit testing. ### New methods on `App` | Method | Purpose | |--------|---------| | `collab_receiver_thread_ids` | Extracts `receiver_thread_ids` from `ItemStarted` / `ItemCompleted` collab notifications | | `hydrate_collab_agent_metadata_for_notification` | Fetches and caches metadata for unknown receiver threads before a notification is rendered | | `backfill_loaded_subagent_threads` | Bulk-fetches all loaded threads and registers descendants of the primary thread | | `adjacent_thread_id_with_backfill` | Attempts navigation, falls back to backfill if the cache has no adjacent entry | | `replace_chat_widget` | Replaces the widget and re-seeds its metadata map from `AgentNavigationState` | ### New state in `ChatWidget` `collab_agent_metadata: HashMap<ThreadId, CollabAgentMetadata>` -- a lookup table that rendering functions consult to attach human-readable names to collab tool-call items. Populated externally by `App` via `set_collab_agent_metadata`. ### New method on `AppServerSession` `thread_loaded_list` -- thin wrapper around `ClientRequest::ThreadLoadedList`. ## Observability - `tracing::warn` on invalid thread ids during hydration and backfill. - `tracing::warn` on failed `thread/read` or `thread/loaded/list` RPCs (with thread id and error). - No new metrics or feature flags. ## 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 a `ChatWidget`, replaces it via `replace_chat_widget`, replays a collab wait notification, and asserts the rendered history cell contains the agent name rather than a UUID. - **Updated snapshot** `app_server_collab_wait_items_render_history` -- the existing collab wait rendering test now sets metadata before sending notifications, so the snapshot shows `Robie [explorer]` / `Ada [reviewer]` instead of raw thread ids. --------- Co-authored-by: Eric Traut <etraut@openai.com>