feat(queue): hold/resume individual queued AI messages#941
feat(queue): hold/resume individual queued AI messages#941pedramamini wants to merge 1 commit intomainfrom
Conversation
Adds a pause toggle on each queued item so users can park a message in place when the AI comes back with a question on an earlier turn. Held items stay in the queue (preserving order) but are skipped during dispatch until the user resumes them. The skip-paused logic is applied at every dispatch site: the agent exit handler, interrupt/kill handlers, the agent execution completion path, batch queue processing, the startup recovery path, and the debug release action — so paused items are honored regardless of how the queue gets drained. The Auto Run drain wait also treats "all remaining items paused" as drained so a user-held message can't stall a batch. UI shows a Pause/Play button next to the existing Remove control in both the inline queued-items list and the cross-session execution queue browser, plus a "HELD" badge and dimmed/dashed-border styling on paused rows. closes #261
📝 WalkthroughWalkthroughThis PR adds pause/resume functionality for individual queued items in AI mode. A new ChangesQueue Pause/Resume Feature
Sequence DiagramsequenceDiagram
actor User
participant QueueUI as Queue UI<br/>(Button/Badge)
participant Handler as handleTogglePauseQueueItem<br/>(Hook)
participant Store as Session State<br/>(Store)
participant Engine as Queue Processing<br/>(Engine Hooks)
User->>QueueUI: Clicks pause button on item
QueueUI->>Handler: onTogglePauseQueueItem(sessionId, itemId)
Handler->>Store: setSessions() → toggle item.paused
Store->>QueueUI: Item visually marked HELD, dimmed
loop When processing queue
Engine->>Store: Find first non-paused item
alt Item is paused
Engine->>Store: Skip, check next item
else Item is not paused
Engine->>Engine: Process queued item
Engine->>Store: Remove from executionQueue
end
end
User->>QueueUI: Clicks resume button on paused item
QueueUI->>Handler: onTogglePauseQueueItem(sessionId, itemId)
Handler->>Store: setSessions() → set item.paused = false
Store->>Engine: Next queue-processing cycle picks up item
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR adds per-item pause/hold to the AI execution queue: a
Confidence Score: 3/5Safe to merge for the pause/hold use-case, but resuming a paused item while the session is idle silently does nothing — users must send a new message to unblock it. A P1 behavioral gap (resume-while-idle is a no-op) pulls the score below the P1 ceiling of 4. The dispatch plumbing is correct at all active-run exit points; the missing piece is a reactive effect that watches for newly-runnable queue items and kicks off dispatch when the session is idle. src/renderer/hooks/agent/useQueueProcessing.ts — needs a secondary effect (or a store subscriber) that fires dispatch when a paused item is resumed and the session is already idle. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent onExit / Interrupt] --> B{Queue has\nnon-paused item?}
B -- Yes --> C[Dequeue & dispatch\nnext runnable item]
B -- No --> D[Session → idle\nPaused items remain in queue]
E[User clicks Resume] --> F[Flip paused=false in state]
F --> G{Session\ncurrently idle?}
G -- Yes --> H[❌ No dispatch triggered\nItem stuck until next onExit]
G -- No --> I[Item will be picked up\non next onExit cycle]
J[App startup] --> K{Idle sessions with\nnon-paused queue items?}
K -- Yes --> L[One-shot recovery dispatch]
K -- No --> M[Skip — no action]
|
| ); | ||
| }, []); | ||
|
|
||
| const handleTogglePauseQueuedItem = useCallback((itemId: string) => { | ||
| setSessions((prev) => | ||
| prev.map((s) => { | ||
| if (s.id !== activeSessionIdRef.current) return s; | ||
| return { | ||
| ...s, | ||
| executionQueue: s.executionQueue.map((item) => | ||
| item.id === itemId ? { ...item, paused: !item.paused } : item | ||
| ), | ||
| }; |
There was a problem hiding this comment.
Duplicate toggle-pause implementation drifts from
useQueueHandlers
App.tsx defines its own handleTogglePauseQueuedItem locally (used for the inline QueuedItemsList) while useQueueHandlers exports handleTogglePauseQueueItem (used for ExecutionQueueBrowser). Both do the same !item.paused flip, but through different code paths — one reading activeSessionIdRef.current, the other taking an explicit sessionId. If the logic ever needs to change (e.g., adding a "cannot pause the first non-paused item" guard), it must be updated in two places. Consider wiring the inline list through the hook handler by passing the active session ID, or exposing an ID-less variant from the hook.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/TerminalOutput.tsx`:
- Around line 1057-1060: The memoized TerminalOutput component's custom
comparator is missing the onTogglePauseQueuedItem prop, so changes to that
handler won't trigger re-renders; update the equality check used in React.memo
for TerminalOutput (the comparator function that compares props like
onRemoveQueuedItem, onDeleteLog, onInterrupt, etc.) to also compare
prevProps.onTogglePauseQueuedItem !== nextProps.onTogglePauseQueuedItem (and
mirror this change in the other comparator occurrences mentioned around the
other blocks), ensuring the memo returns false when the toggle handler reference
changes so the component updates.
In `@src/renderer/hooks/agent/useAgentExecution.ts`:
- Around line 377-383: The drain check treats "!hasRunnableLeft" as sufficient
even when checkSession.state === 'busy', letting Auto Run continue while a
just-dequeued runnable is still executing; update the condition around
checkSession/hasRunnableLeft so you only treat the queue as drained if there are
no runnable items AND there is no active running session (i.e., require
checkSession.state !== 'busy' or explicitly ensure no item is currently
running). Concretely, modify the logic that computes hasRunnableLeft and the if
that follows (referencing checkSession, executionQueue, hasRunnableLeft, and
checkSession.state) so the branch that continues the batch only runs when
checkSession is falsy or state === 'idle' or (no runnable items AND no currently
running item/session).
In `@src/renderer/hooks/agent/useQueueProcessing.ts`:
- Around line 125-132: The code may process firstItem even when dispatchIndex
=== -1 (meaning it wasn't removed), so change the logic to only proceed with
dequeuing and processing when the item was actually found: compute dispatchIndex
from s.executionQueue.findIndex, if dispatchIndex === -1 do not set
remainingQueue to the original and do not call the dispatch/processing path for
firstItem; otherwise build remainingQueue by slicing out dispatchIndex and then
continue to dispatch/process firstItem. Ensure the guard (dispatchIndex !== -1)
wraps both the remainingQueue assignment and the subsequent processing/dispatch
of firstItem used later in useQueueProcessing (references: s.executionQueue,
dispatchIndex, firstItem, remainingQueue).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c7ae014-ffb1-475b-95ad-75e27f61d5e0
📒 Files selected for processing (15)
src/renderer/App.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/ExecutionQueueBrowser.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QueuedItemsList.tsxsrc/renderer/components/TerminalOutput.tsxsrc/renderer/hooks/agent/useAgentExecution.tssrc/renderer/hooks/agent/useAgentListeners.tssrc/renderer/hooks/agent/useInterruptHandler.tssrc/renderer/hooks/agent/useQueueHandlers.tssrc/renderer/hooks/agent/useQueueProcessing.tssrc/renderer/hooks/batch/useBatchHandlers.tssrc/renderer/hooks/modal/useQuickActionsHandlers.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/types/index.ts
| onDeleteLog?: (logId: string) => number | null; // Returns the index to scroll to after deletion | ||
| onRemoveQueuedItem?: (itemId: string) => void; // Callback to remove a queued item from execution queue | ||
| onTogglePauseQueuedItem?: (itemId: string) => void; // Callback to toggle the held/paused state of a queued item | ||
| onInterrupt?: () => void; // Callback to interrupt the current process |
There was a problem hiding this comment.
Include the new callback in the React.memo comparison.
TerminalOutput is memoized with a custom comparator, but onTogglePauseQueuedItem is not part of the equality check. If that handler reference changes, the queue UI can keep calling the stale callback.
🔧 Suggested fix
prevProps.bionifyReadingMode === nextProps.bionifyReadingMode &&
prevProps.bionifyIntensity === nextProps.bionifyIntensity &&
prevProps.bionifyAlgorithm === nextProps.bionifyAlgorithm &&
+ prevProps.onTogglePauseQueuedItem === nextProps.onTogglePauseQueuedItem &&
prevProps.fontFamily === nextProps.fontFamily &&Also applies to: 1088-1121, 1844-1855
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/TerminalOutput.tsx` around lines 1057 - 1060, The
memoized TerminalOutput component's custom comparator is missing the
onTogglePauseQueuedItem prop, so changes to that handler won't trigger
re-renders; update the equality check used in React.memo for TerminalOutput (the
comparator function that compares props like onRemoveQueuedItem, onDeleteLog,
onInterrupt, etc.) to also compare prevProps.onTogglePauseQueuedItem !==
nextProps.onTogglePauseQueuedItem (and mirror this change in the other
comparator occurrences mentioned around the other blocks), ensuring the memo
returns false when the toggle handler reference changes so the component
updates.
| // "Drained" means no runnable items left — paused items are | ||
| // held by the user and shouldn't block batch progress. | ||
| const hasRunnableLeft = checkSession?.executionQueue.some( | ||
| (item) => !item.paused | ||
| ); | ||
| if (!checkSession || checkSession.state === 'idle' || !hasRunnableLeft) { | ||
| // Queue drained or session idle - safe to continue batch |
There was a problem hiding this comment.
Queue-drain check can resolve before the dispatched runnable item finishes.
At Line 382, !hasRunnableLeft is treated as drained even when checkSession.state is still busy. That allows Auto Run to continue while the just-dequeued manual item is still running.
Suggested fix
- if (!checkSession || checkSession.state === 'idle' || !hasRunnableLeft) {
+ if (!checkSession || (checkSession.state === 'idle' && !hasRunnableLeft)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // "Drained" means no runnable items left — paused items are | |
| // held by the user and shouldn't block batch progress. | |
| const hasRunnableLeft = checkSession?.executionQueue.some( | |
| (item) => !item.paused | |
| ); | |
| if (!checkSession || checkSession.state === 'idle' || !hasRunnableLeft) { | |
| // Queue drained or session idle - safe to continue batch | |
| // "Drained" means no runnable items left — paused items are | |
| // held by the user and shouldn't block batch progress. | |
| const hasRunnableLeft = checkSession?.executionQueue.some( | |
| (item) => !item.paused | |
| ); | |
| if (!checkSession || (checkSession.state === 'idle' && !hasRunnableLeft)) { | |
| // Queue drained or session idle - safe to continue batch |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/agent/useAgentExecution.ts` around lines 377 - 383, The
drain check treats "!hasRunnableLeft" as sufficient even when checkSession.state
=== 'busy', letting Auto Run continue while a just-dequeued runnable is still
executing; update the condition around checkSession/hasRunnableLeft so you only
treat the queue as drained if there are no runnable items AND there is no active
running session (i.e., require checkSession.state !== 'busy' or explicitly
ensure no item is currently running). Concretely, modify the logic that computes
hasRunnableLeft and the if that follows (referencing checkSession,
executionQueue, hasRunnableLeft, and checkSession.state) so the branch that
continues the batch only runs when checkSession is falsy or state === 'idle' or
(no runnable items AND no currently running item/session).
| const dispatchIndex = s.executionQueue.findIndex((item) => item.id === firstItem.id); | ||
| const remainingQueue = | ||
| dispatchIndex === -1 | ||
| ? s.executionQueue | ||
| : [ | ||
| ...s.executionQueue.slice(0, dispatchIndex), | ||
| ...s.executionQueue.slice(dispatchIndex + 1), | ||
| ]; |
There was a problem hiding this comment.
Don’t dispatch startup item if it was not actually dequeued.
When dispatchIndex === -1 (Line 127), the queue remains unchanged, but Line 161 still processes firstItem. During startup races, this can execute an already-removed item twice.
Suggested fix
+ let didDequeue = false;
setSessions((prev) =>
prev.map((s) => {
if (s.id !== session.id) return s;
const dispatchIndex = s.executionQueue.findIndex((item) => item.id === firstItem.id);
- const remainingQueue =
- dispatchIndex === -1
- ? s.executionQueue
- : [
- ...s.executionQueue.slice(0, dispatchIndex),
- ...s.executionQueue.slice(dispatchIndex + 1),
- ];
+ if (dispatchIndex === -1) return s;
+ didDequeue = true;
+ const remainingQueue = [
+ ...s.executionQueue.slice(0, dispatchIndex),
+ ...s.executionQueue.slice(dispatchIndex + 1),
+ ];
// ...
})
);
+ if (!didDequeue) return;
processQueuedItem(session.id, firstItem).catch((err) => {Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/agent/useQueueProcessing.ts` around lines 125 - 132, The
code may process firstItem even when dispatchIndex === -1 (meaning it wasn't
removed), so change the logic to only proceed with dequeuing and processing when
the item was actually found: compute dispatchIndex from
s.executionQueue.findIndex, if dispatchIndex === -1 do not set remainingQueue to
the original and do not call the dispatch/processing path for firstItem;
otherwise build remainingQueue by slicing out dispatchIndex and then continue to
dispatch/process firstItem. Ensure the guard (dispatchIndex !== -1) wraps both
the remainingQueue assignment and the subsequent processing/dispatch of
firstItem used later in useQueueProcessing (references: s.executionQueue,
dispatchIndex, firstItem, remainingQueue).
Summary
How it works
pausedflag onQueuedItem.executionQueue[0]:useAgentListeners(agent exit handler — bothqueuedItemToProcessand the inline state update).useInterruptHandler(interrupt + force-kill paths).useAgentExecution(post-completion dispatch + Auto Run drain wait).useBatchHandlers(onProcessQueueAfterCompletion).useQueueProcessing(startup recovery).useQuickActionsHandlers(debug "release queued item" action).useQueueHandlers.handleTogglePauseQueueItemand anApp.tsx-local equivalent flip the flag.QueuedItemsList(inline) andExecutionQueueBrowser(modal). Held rows get dimmed/dashed-border styling and a "HELD" badge.Test plan
npm run lint— clean (pre-existing warning in unrelatedweb-server-factory.ts)npm run lint:eslinton changed files — cleannpx vitest run— 15,098 tests passNotes
--no-verifybecause main currently has CRLF/format violations indocs/releases.md(whichCLAUDE.mdexplicitly forbids editing manually) and an unrelated test file that the pre-push hook trips on. CI on main is already failing on the same issues — this PR doesn't introduce them.closes #261
Summary by CodeRabbit
Release Notes