Skip to content

Fix /review interrupt and TUI exit wedges#18921

Merged
etraut-openai merged 1 commit intomainfrom
etraut/fix-review-mcp-startup-interrupt
Apr 23, 2026
Merged

Fix /review interrupt and TUI exit wedges#18921
etraut-openai merged 1 commit intomainfrom
etraut/fix-review-mcp-startup-interrupt

Conversation

@etraut-openai
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai commented Apr 22, 2026

Addresses #11267

Summary

/review can be interrupted while it is still spawning the review sub-agent. That spawn path lives in codex-core and did not observe the task cancellation token until after Codex::spawn returned, so an interrupted review could keep building a child session and leave the TUI in a wedged state.

The TUI exit path also waited indefinitely for app-server thread/unsubscribe, which made Ctrl+C look broken if the app-server was already stuck. This makes interactive delegate startup cancellation-aware and bounds the TUI shutdown-first unsubscribe wait with a short UI escape-hatch timeout.

Testing

I reproed the hang using the steps in the bug report. Confirmed hang no longer exists after fix.

@etraut-openai etraut-openai requested a review from a team as a code owner April 22, 2026 01:16
@etraut-openai etraut-openai force-pushed the etraut/fix-review-mcp-startup-interrupt branch from d545af3 to fa445a3 Compare April 22, 2026 01:23
@etraut-openai etraut-openai changed the title Prevent /review MCP startup and exit wedges Fix /review interrupt and TUI exit wedges Apr 22, 2026
@etraut-openai etraut-openai enabled auto-merge (squash) April 23, 2026 20:24
@etraut-openai etraut-openai disabled auto-merge April 23, 2026 20:28
@etraut-openai etraut-openai merged commit 3f8c06e into main Apr 23, 2026
25 checks passed
@etraut-openai etraut-openai deleted the etraut/fix-review-mcp-startup-interrupt branch April 23, 2026 20:28
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants