-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: process queued messages after context condensation #8528
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
Conversation
- Added processQueuedMessages() call after condenseContext completes - Ensures messages sent during condensation are not lost - Added comprehensive tests for the fix Fixes #8526
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.
Reviewing my own code feels like arguing with the mirror about whose bugs these are while we both know it's me.
Review SummaryThis PR addresses issue #8526 by adding a call to AnalysisAfter reviewing the implementation, the existing concern about only processing a single queued message is not accurate. The implementation correctly processes all queued messages through a natural loop:
TestingThe PR includes comprehensive tests that verify:
RecommendationThis PR is ready to merge. The fix correctly addresses the reported issue without introducing new problems. |
|
Starting my review of your PR—comments incoming soon! 🚀 |
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.
Code Review Summary
After thorough analysis of the implementation, I can confirm this PR correctly fixes issue #8526.
Analysis of Existing Concern
The existing review comment suggested that processQueuedMessages() might only process a single queued message. However, after deep code analysis, I found this concern is not accurate:
How the implementation works:
processQueuedMessages()dequeues one message and callssubmitUserMessage()- This triggers the webview to send the message back to the task
- The
ask()method (lines 843-863 in Task.ts) has built-in logic that checksisMessageQueuedand automatically processes queued messages - This creates a natural loop that continues until all queued messages are processed
This pattern is used consistently throughout the codebase (in applyDiffTool.ts, writeToFileTool.ts, etc.), confirming it's the intended design.
Review Findings
✅ No new issues found
✅ Implementation correctly fixes the reported issue
✅ Comprehensive tests included
✅ Code follows existing patterns
Recommendation: This PR should be approved and merged.
Note: Unable to formally approve due to bot account limitations (cannot approve own PR), but the code review is complete and positive.
Description
This PR fixes an issue where messages sent during manual context condensation were not being processed after condensation completed, causing them to get stuck in the queue.
Problem
When users manually condense context and simultaneously attempt to send a message, the message gets queued but is never dispatched after condensation completes. This forces users to re-enter their message.
Solution
Added a call to
processQueuedMessages()after context condensation completes inClineProvider.condenseTaskContext(). This ensures any messages that were queued during the condensation process are properly processed and sent.Changes
src/core/webview/ClineProvider.tsto process queued messages after condensationsrc/core/webview/__tests__/ClineProvider.spec.tsTesting
Related Issue
Fixes #8477
Type of Change
Checklist
Important
Fixes message queue processing issue in
ClineProvider.condenseTaskContext()by addingprocessQueuedMessages()call post-condensation.ClineProvider.condenseTaskContext().processQueuedMessages()aftercondenseContext()to ensure queued messages are processed.ClineProvider.spec.tsto verify processing of queued messages post-condensation.This description was created by
for f49fd8e. You can customize this summary. It will automatically update as commits are pushed.