🤖 Fix retry barrier flash on message send #415
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
ammar-agent
force-pushed
the
interrupt-flash
branch
from
8100ef1 to
78e0daf
Compare
When sending a message, the retry barrier would briefly appear before the stream-start event arrived. This happened because: 1. User message added to history immediately 2. hasInterruptedStream() returns true for trailing user messages 3. canInterrupt is still false (no active stream yet) 4. Retry barrier renders briefly until stream starts Fixed by ignoring very recent user messages (<2s) in hasInterruptedStream(). This prevents the flash during normal send flow while still catching truly interrupted streams after app restarts or slow model responses. Updated tests to verify the time-based behavior.
The initial fix prevented the flash but introduced a regression: if a send truly hangs (no stream-start event arrives), the retry barrier would never appear because React doesn't re-render just because time passes. Fixed by adding a timer that forces a re-render 2.1s after a user message is sent. This ensures the retry barrier appears even if the stream never starts, while still preventing the flash during normal operation.
React hooks must be called in the same order on every render, so they cannot be placed after conditional returns. Moved the timer useEffect before the early return and adjusted it to safely check workspaceState.
The original fix used time-based hacks (checking message age, forced
re-renders with timers) to prevent retry barrier flash. This was janky
and semantically incorrect.
New approach:
- Track pendingStreamStart flag in StreamingMessageAggregator
- Set true when user message is received
- Set false when stream-start arrives
- 30s timeout if stream never starts (safety net)
- hasInterruptedStream() checks this flag instead of message age
Benefits:
- Semantically correct: explicitly tracks "waiting for stream-start"
- No frontend timer hacks - state updates drive UI naturally
- Cleaner: removed ~24 LoC of hacky code, added ~60 LoC of proper state
- The 30s timeout is meaningful (truly stuck) vs arbitrary ("hide for 2s")
Changes:
- StreamingMessageAggregator: +25 LoC (flag, timer, handlers)
- WorkspaceStore: +3 LoC (expose via state)
- retryEligibility: -14 LoC (removed time checks, added flag param)
- AIView: -27 LoC (removed timer hack)
- Tests: Updated to use flag instead of timestamps
Net: +40 LoC but much cleaner architecture.
Replace boolean flag with timestamp to gracefully handle stale pending states. Instead of a 30s timeout with timer cleanup, check if timestamp is recent (<3s) when evaluating retry eligibility. Benefits: - No timer management/cleanup needed - UI naturally re-evaluates on re-render - More graceful - shows barrier after 3s if stream truly hung - Handles edge cases (app restart, etc.) better Changes: - StreamingMessageAggregator: Store timestamp instead of bool, remove timer - retryEligibility: Check if timestamp < 3s old to prevent flash - WorkspaceStore: Expose pendingStreamStartTime instead of bool - AIView/useResumeManager: Pass timestamp to hasInterruptedStream - Tests: Updated to verify timestamp-based logic (11 tests passing)
Compute showRetryBarrier once before early return, use for both keybinds and UI rendering. Eliminates duplicate hasInterruptedStream call.
ammar-agent
force-pushed
the
interrupt-flash
branch
from
de47795 to
bf93fa9
Compare
ammario
approved these changes
Oct 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes the retry barrier flash that appeared briefly when sending a message, before the stream-start event arrived.
Problem
When sending a message, the retry barrier would briefly flash before disappearing:
hasInterruptedStream()returns true for trailing user messagescanInterruptis still false (no active stream yet)Solution
Track
pendingStreamStartflag explicitly in the aggregator:truewhen user message is receivedfalsewhenstream-startevent arriveshasInterruptedStream()checks this flag to prevent flashThis is semantically correct: we explicitly track the window between "user message added" and "stream-start received" rather than inferring it from message timestamps.
Changes
pendingStreamStartflag with 30s timeoutpendingStreamStartparameterBenefits
Generated with
cmux