Skip to content

fix(desktop): require manual send for restored queue entries - #66240

Open
AlMcClaw-ctrl wants to merge 1 commit into
NousResearch:mainfrom
AlMcClaw-ctrl:fix/desktop-restored-queue-manual-send
Open

fix(desktop): require manual send for restored queue entries#66240
AlMcClaw-ctrl wants to merge 1 commit into
NousResearch:mainfrom
AlMcClaw-ctrl:fix/desktop-restored-queue-manual-send

Conversation

@AlMcClaw-ctrl

Copy link
Copy Markdown

Summary

  • mark composer queue entries restored from local storage as requiring an explicit manual send
  • block both visible-session and background auto-drainers from submitting restored entries
  • preserve normal auto-drain behavior for entries queued during the current Desktop process
  • keep restored entries available in the queue UI so users can review, send, edit, or delete them

Problem

Desktop persists queued prompts, including attachments, in hermes.desktop.composerQueue.v1. On a later app process, load() rehydrated those entries without distinguishing them from prompts queued during the current run. Both auto-drain paths only checked whether the target session was idle and the queue was non-empty.

That allowed an old unsent prompt, including its image attachment, to be submitted automatically after a later restart/reconnect/session rebind. In the reproduced case, the queue entry was almost one month old and had never been explicitly sent by the user.

A composer queue is an immediate sequencing mechanism, not a delayed-job scheduler. Process-restored entries should therefore require renewed user intent.

Fix

load() tags persisted entries with requiresManualSend: true. canAutoDrainQueuedPrompt() gates both:

  • useComposerQueue for the mounted/visible session
  • useBackgroundQueueDrain for offscreen sessions

Manual sendQueuedNow() remains unchanged, so restored prompts are not lost and can still be explicitly submitted.

This is complementary to #56444. That PR preserves source-session affinity for active queue drains; this change prevents a persisted entry from automatically draining at all after a new Desktop process starts.

Tests

  • NODE_OPTIONS=--no-experimental-webstorage npx vitest run --project ui src/store/composer-queue.test.ts src/app/session/hooks/use-background-queue-drain.test.tsx
    • 22 tests passed
  • NODE_OPTIONS=--no-experimental-webstorage npm run typecheck
  • NODE_OPTIONS=--no-experimental-webstorage npm run lint
    • 0 errors, 10 pre-existing warnings
  • clean production build and unpacked macOS package
  • installed package verified with clean build stamp for this commit

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #56444: both protect queued desktop sends, but #56444 preserves source-session affinity while this PR requires renewed user intent for entries restored after a new process.

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Fix prevents auto-draining queue entries restored from a previous Desktop process. The new requiresManualSend flag correctly distinguishes fresh entries from persisted ones, and canAutoDrainQueuedPrompt gates the drain loop. Good test coverage.


Reviewed by Hermes Agent

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved (LGTM)

Overview

Requires manual send for restored queue entries in desktop. +68/0.

Security

  • No hardcoded secrets or credentials

Code Quality

  • Clean UX fix for queue restoration

Looks Good

  • Well-scoped fix

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused queue-safety fix. The premise remains reproducible on current main: apps/desktop/src/store/composer-queue.ts:22-25 restores persisted queue entries without a process-origin distinction, while the visible and background drainers submit the queue head at apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts:275-295 and apps/desktop/src/app/session/hooks/use-background-queue-drain.ts:156-171.

The PR gates both of those drain paths and retains explicit sendQueuedNow() behavior. I found no blocking correctness or design-fit issue in the proposed scope. Its base remains an ancestor of current main; the only relevant intervening change is the working-session-state import refactor in a75a8eda72a82a0e56972ff8084dd6ff5723a52f, so salvage should be mechanical.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants