fix(tui): scope MCP startup status by thread#26639
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5eae73269
ℹ️ 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: 84b8997c6c
ℹ️ 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 |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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 ready for re-review at your convenience. |
etraut-openai
left a comment
There was a problem hiding this comment.
Looks good. Code is much simpler than before!
I had codex manually test this by creating a bad MCP config.
Why
MCP startup failures from spawned subagents were rendered as global notifications, so a child thread's failure could pollute the visible parent transcript. Routing the notification to the child exposed two related replay problems: session refresh could discard the buffered event, and a newly created child
ChatWidgetdid not know the expected MCP server set, which could leave its startup spinner running after every server had settled.MCP startup diagnostics should remain visible in the thread that owns the startup without affecting other transcripts. The protocol also needs to support a future app-scoped MCP lifecycle where startup is not owned by any thread.
Reported Behavior
The originating Slack report called out that using subagents could turn MCP startup failures into a wall of yellow CLI warnings because repeated failures were not deduplicated. The intended behavior is for those diagnostics to remain visible once in the thread that owns the startup, without polluting the parent transcript.
What Changed
threadIdownership tomcpServer/startupStatus/updatedthreadIdas app-scoped without injecting it into the active chat transcriptThe owning thread still renders the detailed failure and final
MCP startup incomplete (...)summary.How to Test
Child-owned failure
smokewhose first launch succeeds and subsequent launches fail during initialization.smokewarning or incomplete-startup summary.smokefailure and oneMCP startup incomplete (failed: smoke)summary.Primary-owned failure
smokefailure and one incomplete-startup summary.Targeted tests:
just test -p codex-app-server-protocoljust test -p codex-app-server thread_start_emits_mcp_server_status_updated_notificationsjust test -p codex-tui mcp_startupBoth manual paths were exercised in tmux on commit
1b50304606. The child-owned run kept the parent warning-free across thread switches, rendered the child diagnostics once, and left no stale startup spinner.just argument-comment-lintwas attempted locally but blocked by an unrelated Bazel LLVM empty-glob failure; the corresponding CI checks passed.