Skip to content

Fix tui_app_server ghost subagent entries in /agent#16110

Merged
etraut-openai merged 11 commits intomainfrom
etraut/fix-tuiappserver-regression
Mar 29, 2026
Merged

Fix tui_app_server ghost subagent entries in /agent#16110
etraut-openai merged 11 commits intomainfrom
etraut/fix-tuiappserver-regression

Conversation

@etraut-openai
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai commented Mar 28, 2026

Fixes #16092

The app-server-backed TUI could accumulate ghost subagent entries in /agent after 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.

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

@etraut-openai etraut-openai force-pushed the etraut/fix-tuiappserver-regression branch from 3f280d7 to aa15c4a Compare March 28, 2026 17:28
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why copied()?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm misreading, but does the continue matter? Also, if these can all be done in parallel, should a JoinSet be used?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should these be static strings? This heuristic seems potentially brittle?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 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".

@etraut-openai etraut-openai requested a review from bolinfest March 29, 2026 00:13
@etraut-openai etraut-openai merged commit 38e648c into main Mar 29, 2026
34 of 38 checks passed
@etraut-openai etraut-openai deleted the etraut/fix-tuiappserver-regression branch March 29, 2026 18:19
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants