Fix tui_app_server ghost subagent entries in /agent#16110
Fix tui_app_server ghost subagent entries in /agent#16110etraut-openai merged 11 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 906535dea3
ℹ️ 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".
3f280d7 to
aa15c4a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa15c4a770
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8b7562925
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88726fda93
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e46b3e1f5b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68b7fce408
ℹ️ 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".
| async fn open_agent_picker(&mut self, app_server: &mut AppServerSession) { | ||
| let thread_ids: Vec<ThreadId> = self.thread_event_channels.keys().cloned().collect(); | ||
| let mut thread_ids = self.agent_navigation.tracked_thread_ids(); | ||
| for thread_id in self.thread_event_channels.keys().copied() { |
There was a problem hiding this comment.
No deep reason. ThreadId is Copy, and I just wanted owned ids here rather than iterating &ThreadId, so copied() was the shortest equivalent to for &thread_id in .... Happy to switch to the pattern form if you think it reads better.
| ); | ||
| } | ||
| } | ||
| continue; |
There was a problem hiding this comment.
Maybe I'm misreading, but does the continue matter? Also, if these can all be done in parallel, should a JoinSet be used?
There was a problem hiding this comment.
You're not misreading: the continue is redundant here now because there is no remaining per-iteration work after the refresh call. I left it in from an earlier shape of the loop. I don't think JoinSet buys much for this path because each refresh mutates local picker state and shares one mutable AppServerSession, so parallelizing would require a two-phase refactor (gather remote results first, then apply them back to self in order). Given the small number of agent rows, I kept it sequential.
| fn can_fallback_from_include_turns_error(err: &color_eyre::Report) -> bool { | ||
| err.chain().any(|cause| { | ||
| let message = cause.to_string(); | ||
| message.contains("includeTurns is unavailable before first user message") |
There was a problem hiding this comment.
Can/should these be static strings? This heuristic seems potentially brittle?
There was a problem hiding this comment.
They can be hoisted to named constants locally, but that would only improve readability, not the underlying brittleness. The real issue is that thread/read currently communicates these cases only via error text rather than a typed status/error code, so this helper has to key off the server message. I kept the string matching centralized in one helper to limit the blast radius, but I agree a typed app-server distinction would be the real fix.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cfb9f8267
ℹ️ 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".
Fixes #16092
The app-server-backed TUI could accumulate ghost subagent entries in
/agentafter resume/backfill flows. Some of those rows were no longer live according to the backend, but still appeared selectable in the picker and could open as blank threads.Cause
Unlike the legacy tui behavior, tui_app_server was creating local picker/replay state for subagents discovered through metadata refresh and loaded-thread backfill, even when no real local session or transcript had been attached. That let stale ids survive in the picker as if they were replayable threads.
Fix
Stop creating empty local thread channels during subagent metadata hydration and loaded-thread backfill.
When opening /agent, prune metadata-only entries that thread/read reports as terminally unavailable.
When selecting a discovered subagent that is still live but not yet locally attached, materialize a real local session on demand from thread/read instead of falling back to an empty replay state.