fix(desktop): drain queued prompts for background sessions

[frizikk]

What does this PR do?

Fixes #57850.

A queued Desktop prompt could remain stranded after its source session became backgrounded: the only auto-drain observer belonged to the mounted ChatBar for the selected session. This adds a controller-scoped drainer for non-selected sessions and preserves each queued entry's runtime and stored session IDs through submission.

The rebase keeps the newer foreground session-context drift guard. Background drains deliberately bypass that foreground-only guard, resume their own stored session after a stale runtime ID, and never overwrite the foreground runtime ID or foreground busy/notification state.

Related Issues

• Fixes Desktop queued prompts in background sessions do not auto-drain until focused #57850
• Addresses [Bug]: Queued messages can fire into the wrong session when switching between concurrent sessions (Desktop only) #56390
• Related field evidence: Desktop: message queued in a busy session is delivered to an unrelated idle session #61573

#56390 describes the same queued-prompt source-session affinity failure once a drain runs. #61573 provides field evidence from Desktop v0.18.2 where a prompt queued for busy Session A was persisted and executed in unrelated idle Session B. This PR covers both mechanisms involved: explicit source runtime/stored IDs through queued submit and a controller-scoped offscreen drain loop.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• apps/desktop/src/app/session/hooks/use-background-queue-drain.ts
  • Adds a DesktopController-scoped drain for idle queues belonging to non-selected sessions.
  • Leaves the selected session's interactive queue UX to its mounted ChatBar.
  • Retries bounded transient rejections without waiting for another queue/busy-state event.
• apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts
  • Accepts explicit queued runtime/stored IDs, updates the source-session cache, and resumes the source stored session when a runtime ID is stale.
  • Preserves the current foreground session-context drift protection, session.resume desktop source payload, and guards foreground-only state mutations.
  • Documents the shared per-session _submitInFlight guard that makes transient foreground/background drainer overlap safe.
• apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts
  • Threads source runtime/stored IDs through the visible queue drain too.
• Tests cover:
  • background idle drain, selected/working-session skips, and retry;
  • source-session routing and no foreground busy-state mutation;
  • stale-runtime resume using the queued stored ID without clobbering the foreground runtime ID.

How to Test

1. Start a prompt in Session A, queue a follow-up there, then switch to Session B.
2. When A becomes idle, confirm the follow-up submits to A without focusing it or making B look busy.
3. Run:

NODE_ENV=test NODE_OPTIONS='--max-old-space-size=8192 --localstorage-file=/tmp/hermes-vitest-pr57849.json' \
  npm --workspace apps/desktop exec -- vitest run --environment jsdom \
  src/app/session/hooks/use-prompt-actions/index.test.tsx \
  src/app/session/hooks/use-background-queue-drain.test.tsx

NODE_ENV=test npm --workspace apps/desktop run typecheck

NODE_ENV=test npm --workspace apps/desktop exec -- eslint \
  src/app/session/hooks/use-background-queue-drain.ts \
  src/app/session/hooks/use-background-queue-drain.test.tsx \
  src/app/session/hooks/use-prompt-actions/submit.ts \
  src/app/session/hooks/use-prompt-actions/utils.ts \
  src/app/session/hooks/use-prompt-actions/index.test.tsx \
  src/app/session/hooks/use-prompt-actions/slash.ts \
  src/app/chat/composer/types.ts \
  src/app/chat/composer/hooks/use-composer-queue.ts \
  src/app/chat/index.tsx \
  src/app/desktop-controller.tsx

NODE_ENV=production npm --workspace apps/desktop run build

git diff --check upstream/main...HEAD

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix
• I've run the relevant automated Desktop checks for this TypeScript/React-only change (pytest tests/ -q is N/A)
• I've added regression tests for this bug fix
• I've tested on Linux (CachyOS), Node.js v25.4.0 / npm 11.7.0

Documentation & Housekeeping

• Docs updated — N/A; behavior is specified by regression tests and focused comments
• cli-config.yaml.example updated — N/A; no config keys changed
• CONTRIBUTING.md/AGENTS.md updated — N/A; no workflow change
• Cross-platform impact considered — browser/React-only code, no platform APIs added
• Tool descriptions/schemas updated — N/A

For New Skills

N/A — this PR does not add a skill.

Screenshots / Logs

Rebased onto current upstream/main (569b912d7d0931c7256e9f5fb326609e9deda377) and verified:

• Focused Vitest: 2 files, 52 tests passed.
• Full Desktop UI suite: 1,286 passed, 2 failed. The untouched upstream/main baseline also fails (1,279 passed, 2 failed): both reproduce the existing fallback-model.test.ts thousands-separator mismatch, while the other failure varies between localStorage-sensitive tests. The PR run's terminals.test.ts failure passes in isolation (7/7).
• TypeScript typecheck: passed.
• Changed-file ESLint: 0 errors, 1 warning (react-hooks/exhaustive-deps in the upstream act() wrapper; the same warning exists on untouched upstream/main).
• Desktop production build: passed; assert-dist-built confirmed dist/index.html and assets.
• git diff --check upstream/main...HEAD: passed.
• Mutation sanity check: replacing the queued stored-session recovery with the selected foreground stored ID makes the switched-session regression fail (expected stored-db-xyz789, received stored-foreground). The mutation was restored and the regression re-passed before push.

[frizikk]

@alt-glitch For extra context: this problem is very frustrating in day-to-day use. Queued prompts getting stranded in background sessions forces manual switching/checking and noticeably slows down the workflow.

[falkoro]

Community review — read the full diff. The core of this is strong: the root-cause diagnosis is correct (the only auto-drain effect lives inside the mounted ChatBar, so a backgrounded session's queue is simply never observed), and lifting the drain to a DesktopController-scope hook with explicit sessionId/storedSessionId threading is the right shape — it also closes the adjacent wrong-session hazard for good. The test coverage is meaningful, especially the offscreen-drain-must-not-flip-foreground-busy assertion.

A few things to tighten, mostly against CONTRIBUTING.md:

1. PR template. The body has good Summary/Root cause/Fix/Tests sections, but skips the repo's template — Type of Change and the checklist (guide read, duplicate search, only-related-changes, platform tested). At the current PR volume that checklist is how maintainers triage; adding it makes this easier to salvage/merge.

2. Unrelated formatting churn. Four hunks look like a different Prettier config rather than the fix: the import reflow in desktop-controller.tsx, the parenthesization change in sessionMessagesSignature, the GatewayRequest type reflow in utils.ts, and the render(...) reflow in index.test.tsx. Dropping those keeps the diff review-tight per the "only changes related to this fix" rule.

3. Small code nit in submit.ts: two back-to-back identical if (targetIsCurrentView) blocks (busy-set, then clearNotifications()) — can merge into one.

4. Re-entrancy across the two drainers: useBackgroundQueueDrain and the ChatBar's useComposerQueue guard with separate drainingSessionIdsRefs, so a session switch mid-drain can briefly have both hooks eligible for the same session (background hook skips the newly selected session; the ChatBar now owns it while the old drain is still in flight). The per-session _submitInFlight lock in submitText does catch the double-submit — worth a one-line comment saying that lock is what makes the dual-drainer overlap safe, so a future refactor doesn't remove it innocently.

None of these are blockers from where I sit — the fix itself looks correct and well-tested. Fixing #1/#2 would mostly make it merge faster.

[frizikk]

@falkoro thanks for the review — addressed the feedback in the latest push:

• moved the PR body onto the repo template and filled out the Type of Change/checklist sections
• dropped the unrelated formatting-only hunks
• merged the duplicate targetIsCurrentView block in submit.ts
• added a comment documenting that the per-session _submitInFlight lock is what makes brief foreground/background drainer overlap safe during session switches

I also rebased onto current main to clear the merge conflict and re-ran the focused checks: targeted Vitest (43 passed), typecheck, changed-file ESLint, desktop build, and git diff --check. Details are in the PR body.

[yingliang-zhang]

Great work on the background drainer — this covers the offscreen-drain gap that #56444 doesn't address.

One thing to watch for in the session.resume retry path inside submit.ts: when a background queued drain hits a "session not found" error and falls back to session.resume, it currently reads selectedStoredSessionIdRef.current — which points to the foreground session if the user switched. This re-registers the wrong session.

The fix is to accept targetStoredId in SubmitTextOptions (threaded from the queued entry's sourceStoredId) and use it for the resume:

const storedIdForResume = options?.targetStoredId || selectedStoredSessionIdRef.current

Also, after a successful resume, the current code unconditionally sets activeSessionIdRef.current = recoveredId. For a background drain, this clobbers the foreground session's runtime id. Guard it:

if (!options?.targetRuntimeId || options.targetRuntimeId === activeSessionIdRef.current) {
  activeSessionIdRef.current = recoveredId
}

These two changes make the background drain fully safe when the user has switched to a different session.

[falkoro]

Re-reviewed the updated push — every point from my earlier review is addressed, verified against the new head:

• ✅ PR template — Type of Change, Checklist, and How to Test all filled in
• ✅ Formatting churn dropped — desktop-controller.tsx is now just the +8 hook wiring, utils.ts just the SubmitTextOptions addition, the sessionMessagesSignature/GatewayRequest/test-render reflows are gone; the diff is review-tight
• ✅ Duplicate targetIsCurrentView blocks merged — busy-set + clearNotifications() now live in one block in submit.ts
• ✅ Dual-drainer safety documented — the comment at the _submitInFlight lock now says exactly why the foreground/background drainer overlap during session switches is safe, so a future refactor won't remove it innocently

The core design was already sound (controller-scope drain + explicit sessionId/storedSessionId threading); with the diff tightened and the invariant documented, this looks ready from a community perspective — LGTM. (GitHub won't let a non-collaborator file a formal approval on this repo, so consider this the approve.) CI on the new push is still pending workflow approval, but the changes themselves check out. Nice turnaround on the feedback.

[frizikk]

@yingliang-zhang thanks — agreed this is the important safety case.

I re-checked the current implementation while resolving the latest main conflict. The queued-drain context is already threaded through under the existing option names:

• SubmitTextOptions.storedSessionId = the queued/background stored id
• SubmitTextOptions.sessionId = the queued/background runtime id

The retry path now resolves the stored id with:

const recoverStoredSessionId = targetStoredSessionId ?? selectedStoredSessionIdRef.current

and only updates the foreground runtime ref when the target is still the current view:

if (targetIsCurrentView) {
  activeSessionIdRef.current = recoveredId
}

I added a regression test for the exact switched-session case you called out:

background queue resume uses the queued stored id and leaves foreground runtime selected

It covers: background queued drain hits session not found, resumes the queued stored id (not the foreground selected id), retries with the recovered runtime id, and leaves the foreground runtime id untouched.

Rebased onto latest main and re-ran:

Vitest focused: 2 files passed, 47 tests passed
Typecheck: passed
Changed-file ESLint: passed
Desktop production build: passed
Git diff check: passed

The PR is now mergeable from GitHub's perspective; only review/policy gates remain.

[frizikk]

@falkoro thanks for the second pass and the detailed LGTM.

Small follow-up after this review: main moved, so I rebased the PR to clear the conflict and preserved the tightened diff/invariant comments you checked here. I also added one extra regression test after the later stored/runtime-id review note:

background queue resume uses the queued stored id and leaves foreground runtime selected

That test covers the switched-session edge case where a background queued submit hits session not found, resumes the queued stored id instead of the foreground selected id, retries on the recovered runtime id, and leaves the foreground runtime ref untouched.

Re-ran after the rebase:

Vitest focused: 2 files passed, 47 tests passed
Typecheck: passed
Changed-file ESLint: passed
Desktop production build: passed
Git diff check: passed

Current PR head is 48241048d7e5e79e8e867acc0c39f07273fc4826; GitHub reports it mergeable now, with only review/policy gates remaining.

[frizikk]

Rebased this PR onto current main (af250d849); new head: ec36940ab4f0.

Kept the newer foreground session-context drift guard while preserving queued/background routing:

• background resumes use the queued stored ID, not the selected foreground ID;
• recovered background runtime IDs do not overwrite the foreground runtime ref;
• session.resume retains source: "desktop".

Re-ran: focused Vitest (2 files / 51 tests), Desktop typecheck, changed-file ESLint, production build, and git diff --check. The PR body has the commands and mutation-sanity evidence. CI is now running.

[frizikk]

Rebased onto current main (569b912d7d09); new head: 7f2ad884f091.

Conflict resolution preserved both sides of the current test contract: upstream's React act() wrappers and this PR's observable foreground runtime ref. The production code applied cleanly.

Re-ran after the rebase:

• focused Vitest: 2 files / 52 tests passed;
• Desktop typecheck: passed;
• changed-file ESLint: 0 errors (1 warning already present on main);
• production build: passed;
• git diff --check: passed;
• mutation sanity check failed on the wrong foreground stored ID as expected, then the restored regression passed.

I also ran the full Desktop UI suite. It remains red on unrelated baseline/flaky tests; exact PR/base evidence is documented in the updated PR body.

[teknium1]

Thanks for the focused background-queue fix. The premise remains present on current main: the sole auto-drain effect is in apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts:309-316, while the only mounted ChatBar is bound to the selected session in apps/desktop/src/app/chat/index.tsx:524-550. An idle queue from a previously selected session is therefore not observed.

The PR's controller-scoped drain and explicit queued runtime/stored ID threading address that gap without publishing background work into the foreground. This fits the Desktop isolation contract: apps/desktop/src/app/session/hooks/use-session-state-cache.ts:257-286 maintains per-session state and only syncs the active runtime ID to the shared view. The included tests also cover the stale-runtime recovery and foreground-runtime preservation case raised in prior review.

No blocking issues found. Current main has no apps/desktop commits after this PR's base, so salvage should be mechanical.

Automated hermes-sweeper review.

[frizikk]

Added the missing issue linkage after re-checking the current release and upstream reports:

• [Bug]: Queued messages can fire into the wrong session when switching between concurrent sessions (Desktop only) #56390 is the same queued-prompt source-session affinity failure.
• Desktop: message queued in a busy session is delivered to an unrelated idle session #61573 is concrete v0.18.2 field evidence: a prompt queued for busy Session A was persisted and executed in unrelated idle Session B.

The current regression coverage in this PR exercises both the source-binding invariant (a fromQueue drain sends to its queued session even after the active session changes) and the offscreen-drain gap. I updated the PR body to mark #56390 as addressed and #61573 as a likely fix rather than claiming reporter verification that has not happened yet.

[frizikk]

@teknium1 could you please reassess the priority/severity of this Desktop session-routing class?

I agree that #57850's narrow symptom — a background queue remaining stranded until the session is focused — can be viewed as P3 on its own. But this PR also addresses the materially more serious source-session routing failure reported in #56390 and evidenced in #61573:

• a prompt composed/queued for busy Session A can be persisted and executed in unrelated idle Session B;
• the wrong session accepts it as an in-context instruction and can act with full tool access;
• the intended session silently loses the prompt;
• Desktop gives no indication that the delivery target changed.

#61573 contains concrete v0.18.2 agent.log + state.db evidence of exactly that persistence misrouting. An approval-shaped follow-up such as “yes, go ahead” landing in another session is a real command-execution risk, not a cosmetic queue problem.

Under the repository's labels (P1 = major feature broken, no workaround, P2 = degraded but workaround exists, P3 = cosmetic), I think this class warrants P1 consideration, or at minimum P2 plus sweeper:risk-message-delivery. The only reliable workaround is effectively to avoid concurrent Desktop session workflow / queued follow-ups or use another surface, which undermines the feature this UI is meant to provide.

The separate ordinary-submit A/B/B bug in #64789 / #65328 strengthens the overall Desktop session-isolation concern, but I am not claiming this PR fixes that distinct foreground path. This request is specifically about the queued cross-session persistence behavior covered here.

[OutThisLife]

Reviewed (cc from the Desktop cross-session thread). The fix direction is right — binding queued entries to their originating runtime/stored session IDs + a controller-scoped offscreen drainer is exactly what the "prompt queued for A executes in B" reports (#57850, #56390, #61573) need.

The blocker is a rebase collision, not the design. Since this branch's base, main grew a profile-aware routed-resume subsystem in this very function (useSubmitPrompt) that your rewrite predates:

• New SubmitPromptDeps: getRoutedStoredSessionId, getRuntimeIdForStoredSession, resumeStoredSession
• New control-flow branch: routedSessionNeedsResume → resumeStoredSession() + cross-wired runtime/stored validation (the Desktop async delegation completions can revive an old session while follow-up prompts create a new session #55578 "don't fork a contextless chat on profile swap" guard)

Your version restructured the same routing block around targetIsCurrentView() / targetStoredSessionId / options.fromQueue / activeSessionId and doesn't carry those deps or that branch — so the 3 conflicts in submit.ts aren't mechanical: they're two independent rewrites of the most bleed-sensitive routing path. A blind merge risks either dropping the profile-aware resume (reintroducing #55578) or mis-composing it with the background drain (reintroducing the very A→B bleed this fixes).

This one needs your intent on how the background-drain path should compose with the routed-resume path — could you rebase onto current main and reconcile the two in useSubmitPrompt (keep main's routedSessionNeedsResume foreground recovery; layer your targetIsCurrentView guards + fromQueue background path on top)? The other 9 files auto-merge cleanly; it's only this function + its caller wiring (use-prompt-actions/index) that need your hand. Happy to re-review the moment it's rebased.

(For the thread: #62408 terminal-cwd is merged; the bg-completion routing piece is being worked in the #54785/#63317/#42731 cluster.)

[OutThisLife]

Update: went ahead and did the rebase myself in #66001 rather than bounce it back to you. The submit.ts collision with main's routed-resume subsystem (#55578) is reconciled by composing both — your targetIsCurrentView() foreground guards + background drainer on top of main's profile-aware resumeStoredSession path, with an explicit queued runtime id bypassing the routed force-null so the drain keeps its originating session. Your commits + authorship preserved.

typecheck clean; your 57 behavioral tests pass unchanged (source-session routing / no foreground mutation / stale-runtime resume), plus 221 across session-hooks + composer. Will close this once #66001 merges. Thanks @frizikk — solid fix, just needed to catch up to main.

[OutThisLife]

Landed via #66001 (merged) — your commits preserved with Co-authored-by. Closing as superseded. Thanks @frizikk.