-
Notifications
You must be signed in to change notification settings - Fork 41
🤖 fix: allow interrupt during stream startup #1536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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: df524048ba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
df52404 to
22cac8f
Compare
|
@codex review Addressed the startup-interrupt race by honoring already-aborted signals before atomic stream creation, plus a unit test that aborts during temp-dir creation to ensure we never emit stream-start / start streaming. |
There was a problem hiding this 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: bfe9be15f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review Refactored stream startup abort handling to be race-free by construction:
|
There was a problem hiding this 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: 0fe42157f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!appendResult.success) { | ||
| return Err({ type: "unknown", raw: appendResult.error }); | ||
| } | ||
|
|
||
| if (abortSignal?.aborted) { | ||
| return Ok(undefined); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean up mock history when abort after append
In mock mode, play() now checks abortSignal after appending the assistant placeholder to history and returns early when aborted. If the user cancels during startup (after appendToHistory resolves but before scheduleEvents), the history ends up with an empty assistant message and no stream-abort/cleanup for that message ID, so reloads or retry logic can treat the response as completed even though nothing streamed. Consider moving the abort check before the history append or emitting a stream-abort/removing the assistant entry when this abort path is hit.
Useful? React with 👍 / 👎.
What
Allow interrupt (Esc / Ctrl+C) to cancel a message while the UI is still in "Starting..." (i.e., before
stream-start).Why
Previously, interrupts were gated on
canInterrupt(active StreamManager stream). If a send was blocked during init/runtime setup, there was no active stream to stop, so the UI could get stuck in "Starting..." with no way to cancel.How
pendingStreamStartTimeas interruptible for keybind interception.pendingStreamStartTimeonstream-abort(abort can arrive beforestream-start).AIService.streamMessage().stopStream()aborts pending startup and emits a syntheticstream-abortwhen cancelled before StreamManager starts.Tests
make static-checktests/ipc/interruptStream.starting.test.ts(requiresTEST_INTEGRATION=1+ANTHROPIC_API_KEY).Generated with
mux• Model:openai:gpt-5.2• Thinking:xhigh• Cost:$11.21