Skip to content

fix(desktop): make Stop button actually interrupt when a turn is queued#37948

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/desktop-stop-button-interrupt
Jun 3, 2026
Merged

fix(desktop): make Stop button actually interrupt when a turn is queued#37948
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/desktop-stop-button-interrupt

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

On the desktop app (reproduced on macOS), sending a message while the agent is busy queues a follow-up — but then the Stop button never actually interrupts the running turn. Clicking the arrow/Stop button after queuing keeps the agent running on the queued prompt instead of halting.

Root cause

Two cooperating bits of logic in apps/desktop/src/app/chat/composer/:

  1. submitDraft Stop branch. When busy with an empty composer (the state right after a follow-up has been queued and the composer cleared), the primary button shows the Stop square. But clicking it ran interruptAndSendNextQueued() — cancel the turn and immediately re-send the head of the queue. So Stop never stopped; it just swapped which prompt the agent was running.

  2. Auto-drain on settle. The busy: true → false effect unconditionally re-sent the next queued prompt. Even a pure onCancel() would flip busy false and the queue would auto-fire. The user could never halt while anything was queued.

Net effect: the Stop affordance lied — it rendered as a Stop square but behaved as "interrupt, then keep going."

Fix

  • Stop button (busy + empty composer) is now a pure interrupt via onCancel(). It no longer hijacks the queue.
  • An explicit interrupt latches userInterruptedRef so the busy → false auto-drain skips exactly one drain. Queued turns are preserved; the user resumes them deliberately — Cmd/Ctrl+K, Enter on an empty composer, or the per-row "send now" arrow — matching the documented affordances (Esc = cancel run, Cmd+K = send next queued).
  • The settle decision is extracted into a pure shouldAutoDrainOnSettle() helper in composer-queue.ts so the invariant is explicit and testable.
  • Removed the now-unused interruptAndSendNextQueued.

Enter while busy and the Stop button now behave identically (both interrupt), which is the intended "cancel run" semantics.

Test plan

New unit tests in composer-queue.test.ts cover shouldAutoDrainOnSettle:

  • drains the next queued prompt when a turn completes naturally
  • does not drain when the user explicitly interrupted (Stop) — regression guard
  • does not drain on empty queue
  • ignores steady busy / busy-entry / steady-idle (only a true→false transition triggers)
✓ src/store/composer-queue.test.ts (12 tests)
  Test Files  1 passed (1)
       Tests  12 passed (12)

Manual repro before/after on the desktop composer:

  1. Send a message → agent busy.
  2. Type a second message, click the button → it queues (Layers icon), composer clears.
  3. Click the Stop square.
    • Before: agent keeps running on the queued prompt.
    • After: turn halts; the queued message stays in the queue for the user to send when ready.

@rodriguez46p-ui rodriguez46p-ui left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes hourly review: I read the diff for the composer queue Stop-button regression and did not find a blocking issue. The new pure shouldAutoDrainOnSettle helper captures the important invariant: natural busy→idle completion drains queued work, explicit Stop suppresses exactly one drain while preserving the queue.\n\nI am leaving this as a comment rather than approval because the broader CI matrix was still pending when checked; the already-finished lint/security/e2e checks were passing.

@OutThisLife OutThisLife self-requested a review June 3, 2026 06:04
OutThisLife
OutThisLife previously approved these changes Jun 3, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 3, 2026
When a follow-up message is queued during a busy turn, the composer
clears and the primary button switches back to the Stop affordance. But
clicking Stop ran interruptAndSendNextQueued(), which cancelled the turn
and *immediately* re-sent the head of the queue. The auto-drain effect
(busy true to false) compounded this: any explicit cancel flipped busy
false and re-fired the queue. The net effect was that Stop appeared to
never interrupt -- the agent kept running on the queued prompt.

Fix:
- Stop button (busy + empty composer) now always performs a pure
  interrupt via onCancel(); it no longer hijacks the queue.
- An explicit interrupt latches userInterruptedRef so the busy to false
  auto-drain skips exactly one drain. Queued turns are preserved and the
  user resumes them deliberately (Cmd/Ctrl+K, Enter, or the per-row
  send-now arrow), matching the documented Esc=cancel / Cmd+K=send-next
  affordances.
- Extracted the settle decision into shouldAutoDrainOnSettle() with unit
  tests covering natural completion vs. explicit interrupt.
@kshitijk4poor kshitijk4poor force-pushed the fix/desktop-stop-button-interrupt branch from 9a69103 to a23728d Compare June 3, 2026 06:16
@kshitijk4poor kshitijk4poor enabled auto-merge June 3, 2026 06:16
@kshitijk4poor kshitijk4poor merged commit ada0457 into NousResearch:main Jun 3, 2026
20 checks passed
davidgut1982 pushed a commit to davidgut1982/hermes-agent that referenced this pull request Jun 5, 2026
…stop-button-interrupt

fix(desktop): make Stop button actually interrupt when a turn is queued
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants