fix: pause queued sends when response is required#429
fix: pause queued sends when response is required#429Dimillian merged 8 commits intoDimillian:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes queued-message auto-flush advancing a thread while the UI is waiting for a required user response (plan accept/changes follow-up or request-user-input prompt), with a user-facing pause hint and a configurable setting.
Changes:
- Add
pauseQueuedMessagesWhenResponseRequiredapp setting (defaulttrue) with a Settings toggle (frontend + Tauri types/defaults). - Gate
useQueuedSend’s auto-flush effect behind aqueueFlushPausedflag derived from active thread state. - Display a “Paused …” hint in the composer queue UI when queued sends are blocked.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/types.ts |
Adds the new app setting to AppSettings. |
src/features/settings/hooks/useAppSettings.ts |
Sets the default for the new app setting (true). |
src/features/settings/components/sections/SettingsFeaturesSection.tsx |
Adds a Settings toggle to control the new behavior. |
src-tauri/src/types.rs |
Adds/serializes the setting in Tauri-side AppSettings with a default and test assertion. |
src/features/threads/hooks/useQueuedSend.ts |
Introduces queueFlushPaused option and blocks auto-flush when set. |
src/features/threads/hooks/useQueuedSend.test.tsx |
Adds coverage ensuring queued messages don’t flush while paused. |
src/App.tsx |
Computes pause conditions from active items/user-input requests; passes pause flag + pause reason into composer pipeline. |
src/features/app/hooks/useComposerController.ts |
Threads queueFlushPaused through to useQueuedSend. |
src/features/layout/hooks/layoutNodes/types.ts |
Extends layout node options with queuePausedReason. |
src/features/layout/hooks/layoutNodes/buildPrimaryNodes.tsx |
Passes pause reason into Composer. |
src/features/composer/components/Composer.tsx |
Accepts and forwards queuePausedReason to the queue UI. |
src/features/composer/components/ComposerQueue.tsx |
Renders the pause hint text when provided. |
src/styles/composer.css |
Styles the pause hint text. |
src/features/settings/components/SettingsView.test.tsx |
Updates test settings fixture to include the new field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/App.tsx
Outdated
| const isPlanReadyAwaitingResponse = useMemo(() => { | ||
| if (!activeThreadId) { | ||
| return false; | ||
| } | ||
|
|
||
| let planIndex = -1; | ||
| for (let index = activeItems.length - 1; index >= 0; index -= 1) { | ||
| const item = activeItems[index]; | ||
| if (item.kind === "tool" && item.toolType === "plan") { | ||
| planIndex = index; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (planIndex < 0) { | ||
| return false; | ||
| } | ||
|
|
||
| const planItem = activeItems[planIndex]; | ||
| if (planItem.kind !== "tool" || planItem.toolType !== "plan") { | ||
| return false; | ||
| } | ||
|
|
||
| if (!(planItem.output ?? "").trim()) { | ||
| return false; | ||
| } | ||
|
|
||
| const planTone = toolStatusTone(planItem, false); | ||
| if (planTone === "failed") { | ||
| return false; | ||
| } | ||
|
|
||
| if (isProcessing && planTone !== "completed") { | ||
| return false; | ||
| } | ||
|
|
||
| for (let index = planIndex + 1; index < activeItems.length; index += 1) { | ||
| const item = activeItems[index]; | ||
| if (item.kind === "message" && item.role === "user") { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| }, [activeItems, activeThreadId, isProcessing]); |
There was a problem hiding this comment.
The logic for determining whether a plan is “ready and awaiting user response” duplicates the plan-followup gating logic already implemented in src/features/messages/components/Messages.tsx (the planFollowup useMemo). Keeping two separate implementations in sync is error-prone and can cause the queue-pause behavior to drift from the UI’s “Plan ready” follow-up behavior. Consider extracting this into a shared helper (e.g., in messageRenderUtils or a threads/messages util) and reusing it in both places (or deriving the queue pause state from the same source used to decide whether the follow-up is shown).
| Pause queued messages when response required | ||
| </div> | ||
| <div className="settings-toggle-subtitle"> | ||
| Keep queued messages paused while Codex is waiting for plan approval/changes |
There was a problem hiding this comment.
UI copy: “Pause queued messages when response required” is grammatically incomplete. Consider changing it to “Pause queued messages when a response is required” (and optionally align the subtitle wording similarly) for clarity.
| Pause queued messages when response required | |
| </div> | |
| <div className="settings-toggle-subtitle"> | |
| Keep queued messages paused while Codex is waiting for plan approval/changes | |
| Pause queued messages when a response is required | |
| </div> | |
| <div className="settings-toggle-subtitle"> | |
| Keep queued messages paused while Codex is waiting for plan approval, changes, |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ 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". |
Co-authored-by: Thomas Ricouard <ricouard77@gmail.com>
2 participants
Fixes #428.
What
pauseQueuedMessagesWhenResponseRequired(default: true) + Settings toggle.How to test
Checks