From 58eb473baa817f8105d51b8651dd30b5d731682e Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 3 Jun 2026 21:07:33 -0500 Subject: [PATCH 01/29] fix(desktop): surface background-session clarify prompts instead of hanging clarify.request is a one-shot blocking event: the gateway turn blocks on clarify.respond. The desktop handler dropped it for any non-focused session (`if (!isActiveEvent) return`) and stored at most one request in a single global atom, so a background session that asked a clarifying question hung forever and re-focusing it could never recover (the event was already gone). - store/clarify.ts: key pending requests by runtime session id; expose the active session's request via a focus-scoped computed view (ClarifyTool is unchanged). clearClarifyRequest takes an optional session id for targeted clears, with a request-id fallback. - use-message-stream.ts: park every session's clarify (drop the isActiveEvent early return); toast when one lands for a background session since the row otherwise just keeps spinning like normal work. - clarify-tool.tsx: clear by session id so answering one chat can't wipe another's pending request. - store/clarify.test.ts: concurrent independence, focus-scoped view, targeted/stale/fallback clears. --- .../app/session/hooks/use-message-stream.ts | 22 ++++- .../components/assistant-ui/clarify-tool.tsx | 2 +- apps/desktop/src/store/clarify.test.ts | 81 +++++++++++++++++++ apps/desktop/src/store/clarify.ts | 63 ++++++++++++--- 4 files changed, 150 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/store/clarify.test.ts diff --git a/apps/desktop/src/app/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index fc0d8ec3e35..f4dbb244e51 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -798,13 +798,16 @@ export function useMessageStream({ ) } } else if (event.type === 'clarify.request') { - if (!isActiveEvent) { - return - } - // Surface the clarify tool's overlay. The Python side is blocked on // `clarify.respond`, so without this handler the agent would hang // forever (see tools/clarify_tool.py + tui_gateway/server.py:_block). + // + // Store the request for whichever session raised it — even a background + // one. clarify.request is a one-shot event; if we dropped it for an + // unfocused session, that session would block on `clarify.respond` + // indefinitely and re-focusing it could never recover (the event is + // gone). Parking it per-session lets the user answer once they switch + // over; the inline ClarifyTool reads the active session's entry. const requestId = typeof payload?.request_id === 'string' ? payload.request_id : '' const question = typeof payload?.question === 'string' ? payload.question : '' @@ -815,6 +818,17 @@ export function useMessageStream({ choices: Array.isArray(payload?.choices) ? payload!.choices!.filter(c => typeof c === 'string') : null, sessionId: sessionId ?? null }) + + // The transcript only renders the active session, so a background + // clarify is otherwise invisible (the row just keeps spinning like + // it's working). Nudge the user toward the chat that needs them. + if (!isActiveEvent) { + notify({ + kind: 'info', + title: 'Another chat needs input', + message: 'Hermes asked a question in a background session. Open it to answer.' + }) + } } } else if (event.type === 'error') { const errorMessage = payload?.message || 'Hermes reported an error' diff --git a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx index 266e554ad11..1fb652f04c2 100644 --- a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx @@ -103,7 +103,7 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) { answer }) triggerHaptic('submit') - clearClarifyRequest(matchingRequest.requestId) + clearClarifyRequest(matchingRequest.requestId, matchingRequest.sessionId) // The matching tool.complete will land shortly after, swapping this // panel for the ToolFallback view above. } catch (error) { diff --git a/apps/desktop/src/store/clarify.test.ts b/apps/desktop/src/store/clarify.test.ts new file mode 100644 index 00000000000..269004b49f1 --- /dev/null +++ b/apps/desktop/src/store/clarify.test.ts @@ -0,0 +1,81 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { + $clarifyRequest, + $clarifyRequests, + type ClarifyRequest, + clearClarifyRequest, + setClarifyRequest +} from './clarify' +import { $activeSessionId } from './session' + +function clarify(sessionId: string | null, requestId: string): ClarifyRequest { + return { + requestId, + question: `question-${requestId}`, + choices: null, + sessionId + } +} + +describe('clarify store', () => { + beforeEach(() => { + $clarifyRequests.set({}) + $activeSessionId.set(null) + }) + + afterEach(() => { + $clarifyRequests.set({}) + $activeSessionId.set(null) + }) + + it('keeps clarify requests from concurrent sessions independent', () => { + setClarifyRequest(clarify('session-a', 'req-a')) + setClarifyRequest(clarify('session-b', 'req-b')) + + expect($clarifyRequests.get()['session-a']?.requestId).toBe('req-a') + expect($clarifyRequests.get()['session-b']?.requestId).toBe('req-b') + }) + + it('exposes only the active session via the focus-scoped view', () => { + setClarifyRequest(clarify('session-a', 'req-a')) + setClarifyRequest(clarify('session-b', 'req-b')) + + $activeSessionId.set('session-a') + expect($clarifyRequest.get()?.requestId).toBe('req-a') + + $activeSessionId.set('session-b') + expect($clarifyRequest.get()?.requestId).toBe('req-b') + + $activeSessionId.set('session-c') + expect($clarifyRequest.get()).toBeNull() + }) + + it('clears only the targeted session, leaving the other pending', () => { + setClarifyRequest(clarify('session-a', 'req-a')) + setClarifyRequest(clarify('session-b', 'req-b')) + + clearClarifyRequest('req-a', 'session-a') + + expect($clarifyRequests.get()['session-a']).toBeUndefined() + expect($clarifyRequests.get()['session-b']?.requestId).toBe('req-b') + }) + + it('ignores a stale clear whose request id no longer matches', () => { + setClarifyRequest(clarify('session-a', 'req-a2')) + + clearClarifyRequest('req-a1', 'session-a') + + expect($clarifyRequests.get()['session-a']?.requestId).toBe('req-a2') + }) + + it('clears by request id across sessions when no session hint is given', () => { + setClarifyRequest(clarify('session-a', 'shared')) + setClarifyRequest(clarify('session-b', 'other')) + + clearClarifyRequest('shared') + + expect($clarifyRequests.get()['session-a']).toBeUndefined() + expect($clarifyRequests.get()['session-b']?.requestId).toBe('other') + }) +}) diff --git a/apps/desktop/src/store/clarify.ts b/apps/desktop/src/store/clarify.ts index ce90f48d8c9..14a98f15c88 100644 --- a/apps/desktop/src/store/clarify.ts +++ b/apps/desktop/src/store/clarify.ts @@ -1,4 +1,6 @@ -import { atom } from 'nanostores' +import { atom, computed } from 'nanostores' + +import { $activeSessionId } from './session' export interface ClarifyRequest { requestId: string @@ -7,26 +9,61 @@ export interface ClarifyRequest { sessionId: string | null } -// Holds the request_id (and metadata) for the most recent in-flight -// clarify call. The inline ClarifyTool component (rendered inside the -// assistant message stream) reads this to know which request_id to send -// back over `clarify.respond`. -export const $clarifyRequest = atom(null) +// Pending clarify requests keyed by the runtime session id that raised them. +// Storing per-session (instead of one shared slot) lets a *background* session +// park its clarify request while the user is looking at a different chat, then +// resolve it once they switch over — without a second concurrent clarify +// clobbering the first. A request with no session id lands under the empty key. +const keyFor = (sessionId: string | null | undefined): string => sessionId ?? '' + +export const $clarifyRequests = atom>({}) + +// The clarify request for the currently-viewed session. The inline ClarifyTool +// only ever mounts inside the active session's transcript, so it reads this +// focus-scoped view rather than reaching into the whole map. +export const $clarifyRequest = computed( + [$clarifyRequests, $activeSessionId], + (requests, activeId) => requests[keyFor(activeId)] ?? null +) export function setClarifyRequest(request: ClarifyRequest): void { - $clarifyRequest.set(request) + $clarifyRequests.set({ ...$clarifyRequests.get(), [keyFor(request.sessionId)]: request }) } -export function clearClarifyRequest(requestId?: string): void { - const current = $clarifyRequest.get() +export function clearClarifyRequest(requestId?: string, sessionId?: string | null): void { + const requests = $clarifyRequests.get() + + // Targeted clear when the caller knows the session (the common path from the + // inline ClarifyTool answering its own request). + if (sessionId !== undefined) { + const key = keyFor(sessionId) + const current = requests[key] + + if (!current || (requestId && current.requestId !== requestId)) { + return + } + + const next = { ...requests } + delete next[key] + $clarifyRequests.set(next) - if (!current) { return } - if (requestId && current.requestId !== requestId) { - return + // Fallback with no session hint: drop every entry matching the request id + // (or clear all when none is given). + const next: Record = {} + let changed = false + + for (const [key, value] of Object.entries(requests)) { + if (requestId && value.requestId !== requestId) { + next[key] = value + } else { + changed = true + } } - $clarifyRequest.set(null) + if (changed) { + $clarifyRequests.set(next) + } } From 35a750eedd7b8b669dee3c9878ab157f1e4eb010 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 3 Jun 2026 21:44:30 -0500 Subject: [PATCH 02/29] feat(desktop): persistent needs-input indicator + icon button consolidation Replace the background-clarify toast (expired on alt-tab, easy to miss) with a persistent, glowing amber "needs input" dot on the session's sidebar row, driven off a new ClientSessionState.needsInput flag mirrored into a $attentionSessionIds store. The flag is set on clarify.request and cleared the moment the turn resumes (tool.complete) or ends. Also: redesign the clarify tool UI (borderless choices, pseudo-radio dots, right-aligned checkmark, arc border, tighter padding), make Button the single source of icon-button styling (4px radius, new icon-titlebar variant, titlebar buttons rendered polymorphically via asChild, Codicons throughout), put the file-tree refresh action first, and .trim() pasted composer text. --- apps/desktop/src/app/chat/composer/index.tsx | 10 ++- .../src/app/chat/sidebar/session-row.tsx | 52 ++++++++++-- apps/desktop/src/app/right-sidebar/index.tsx | 48 ++++++----- .../app/session/hooks/use-message-stream.ts | 23 +++--- .../session/hooks/use-session-state-cache.ts | 7 +- .../src/app/shell/titlebar-controls.tsx | 48 ++++++----- apps/desktop/src/app/shell/titlebar.ts | 6 +- apps/desktop/src/app/types.ts | 3 + .../components/assistant-ui/clarify-tool.tsx | 81 ++++++++----------- apps/desktop/src/components/ui/button.tsx | 9 ++- apps/desktop/src/lib/chat-runtime.ts | 3 +- apps/desktop/src/store/session.test.ts | 30 ++++++- apps/desktop/src/store/session.ts | 41 +++++++--- apps/desktop/src/styles.css | 31 +++++++ 14 files changed, 262 insertions(+), 130 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index d19c0a2532f..6d1be390706 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -407,13 +407,19 @@ export function ChatBar({ return } - const pastedText = event.clipboardData.getData('text') + // Trim surrounding whitespace so a copy that dragged along leading/trailing + // blank lines (common when selecting from terminals, code blocks, web pages) + // doesn't dump multiline padding into the composer. Internal newlines are + // preserved — only the edges are cleaned up. + const pastedText = event.clipboardData.getData('text').trim() if (!pastedText) { + event.preventDefault() + return } - if (DATA_IMAGE_URL_RE.test(pastedText.trim())) { + if (DATA_IMAGE_URL_RE.test(pastedText)) { event.preventDefault() return diff --git a/apps/desktop/src/app/chat/sidebar/session-row.tsx b/apps/desktop/src/app/chat/sidebar/session-row.tsx index 81359a6f854..aaa623faa3f 100644 --- a/apps/desktop/src/app/chat/sidebar/session-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-row.tsx @@ -1,3 +1,4 @@ +import { useStore } from '@nanostores/react' import type * as React from 'react' import { Button } from '@/components/ui/button' @@ -6,6 +7,7 @@ import type { SessionInfo } from '@/hermes' import { sessionTitle } from '@/lib/chat-runtime' import { triggerHaptic } from '@/lib/haptics' import { cn } from '@/lib/utils' +import { $attentionSessionIds } from '@/store/session' import { SessionActionsMenu, SessionContextMenu } from './session-actions-menu' @@ -61,6 +63,10 @@ export function SidebarSessionRow({ const title = sessionTitle(session) const age = formatAge(session.last_active || session.started_at) const handleLabel = `Reorder ${title}` + // Subscribe per-row (the leaf) instead of drilling a set through the list — + // the atom is tiny and rarely non-empty. True when a clarify prompt in this + // session is waiting on the user. + const needsInput = useStore($attentionSessionIds).includes(session.id) return ( - {isWorking &&