diff --git a/apps/desktop/src/app/chat/composer/focus.ts b/apps/desktop/src/app/chat/composer/focus.ts index 4ceefb5d3aef..6e450c10f895 100644 --- a/apps/desktop/src/app/chat/composer/focus.ts +++ b/apps/desktop/src/app/chat/composer/focus.ts @@ -67,6 +67,8 @@ const cssEscape = (value: string): string => { } interface SubmitDetail { + /** Unique mounted composer surface captured at click time. */ + surfaceId: string target: ComposerTarget text: string /** `hidden` types the persisted user row so no bubble renders — the @@ -155,6 +157,38 @@ const dispatch = (name: string, detail: T) => { window.setTimeout(() => window.dispatchEvent(new CustomEvent(name, { detail })), 0) } +/** Submit is the one bus mutation that must preserve the chat visible at click + * time. Deferring it lets a parent click handler/tab reveal switch the active + * keep-alive pane before subscribers run, so the task is dropped or claimed by + * another composer. Other bus events intentionally defer for focus restoration. + */ +const dispatchNow = (name: string, detail: T) => { + if (typeof window !== 'undefined') { + window.dispatchEvent(new CustomEvent(name, { detail })) + } +} + +/** Unique identity for the visible composer surface addressed by a submit. */ +const getVisibleComposerSurfaceId = (target: ComposerTarget): string | null => { + if (typeof document === 'undefined') { + return null + } + + const surface = queryVisible(`[data-composer-target="${cssEscape(target)}"]`) + + return surface?.dataset.composerSurfaceId || null +} + +const composerSurfaceIsVisible = (target: ComposerTarget, surfaceId: string): boolean => { + if (typeof document === 'undefined') { + return false + } + + return queryAllVisible(`[data-composer-target="${cssEscape(target)}"]`).some( + surface => surface.dataset.composerSurfaceId === surfaceId + ) +} + const subscribe = (name: string, handler: (detail: T) => void) => { if (typeof window === 'undefined') { return () => undefined @@ -264,17 +298,36 @@ export const onComposerInsertRefsRequest = (handler: (detail: InsertRefsDetail) * the agent a task without the user round-tripping through the input. */ export const requestComposerSubmit = ( text: string, - { target = 'active', displayKind }: { target?: ComposerTarget | 'active'; displayKind?: 'hidden' } = {} -) => { + { + displayKind, + surfaceId: requestedSurfaceId, + target = 'active' + }: { displayKind?: 'hidden'; surfaceId?: null | string; target?: ComposerTarget | 'active' } = {} +): boolean => { const trimmed = text.trim() - if (trimmed) { - dispatch(SUBMIT_EVENT, { - target: resolve(target), - text: trimmed, - ...(displayKind ? { displayKind } : {}) - }) + if (!trimmed) { + return false + } + + const resolvedTarget = resolve(target) + const surfaceId = + requestedSurfaceId === undefined ? getVisibleComposerSurfaceId(resolvedTarget) : requestedSurfaceId + + // Fail closed: without an exact visible surface identity, broadcasting a + // submit could make more than one keep-alive/new-chat composer claim it. + if (!surfaceId || (requestedSurfaceId !== undefined && !composerSurfaceIsVisible(resolvedTarget, surfaceId))) { + return false } + + dispatchNow(SUBMIT_EVENT, { + surfaceId, + target: resolvedTarget, + text: trimmed, + ...(displayKind ? { displayKind } : {}) + }) + + return true } export const onComposerSubmitRequest = (handler: (detail: SubmitDetail) => void) => diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx index e2eb17419de2..c278356d05a7 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx @@ -1,6 +1,8 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { type Dispatch, type PropsWithChildren, type SetStateAction, useLayoutEffect, useState } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { PaneVisibleContext } from '@/components/pane-shell/pane-visibility' import { $clarifyRequests } from '@/store/clarify' import type { ComposerAttachment } from '@/store/composer' import { $gateway } from '@/store/gateway' @@ -12,23 +14,41 @@ import { setSudoRequest } from '@/store/prompts' +import { type ComposerTarget, requestComposerSubmit } from '../focus' +import { ComposerScopeProvider, ComposerSurfaceProvider, MAIN_COMPOSER_SCOPE } from '../scope' + import { useComposerSubmit } from './use-composer-submit' interface SubmitHarnessOptions { attachments?: ComposerAttachment[] busy?: boolean compacting?: boolean + inputDisabled?: boolean + scopeTarget?: ComposerTarget + sessionKey?: string | null + submitOnHide?: boolean + surfaceId?: string | null text?: string + visible?: boolean } +let surfaceSequence = 0 + function renderSubmitHook({ attachments = [], busy = false, compacting = false, - text = '' + inputDisabled = false, + scopeTarget = 'main', + sessionKey = 'stored-session', + submitOnHide = false, + surfaceId, + text = '', + visible = true }: SubmitHarnessOptions = {}) { + const resolvedSurfaceId = surfaceId === undefined ? `test-surface-${++surfaceSequence}` : surfaceId const draftRef = { current: text } - const editor = document.createElement('div') + const editor = window.document.createElement('div') editor.dataset.slot = 'composer-rich-input' editor.textContent = text const editorRef = { current: editor } @@ -36,43 +56,213 @@ function renderSubmitHook({ const onSteer = vi.fn(async () => true) const onSubmit = vi.fn(async () => true) const queueCurrentDraft = vi.fn(() => true) + let updatePaneVisible: Dispatch> | undefined const clearDraft = vi.fn(() => { draftRef.current = '' editorRef.current!.textContent = '' }) - const hook = renderHook(() => - useComposerSubmit({ - activeQueueSessionKey: 'stored-session', - activeQueueSessionKeyRef: { current: 'stored-session' }, - attachments, - busy, - compacting, - clearDraft, - disabled: false, - draftRef, - drainNextQueued: vi.fn(async () => false), - editorRef, - exitQueuedEdit: vi.fn(() => false), - focusInput: vi.fn(), - inputDisabled: false, - loadIntoComposer: vi.fn(), - onCancel, - onSteer, - onSubmit, - queueCurrentDraft, - queueEdit: null, - queuedPrompts: [], - sessionId: 'runtime-session', - setComposerText: vi.fn(), - stashAt: vi.fn() - }) + const Wrapper = ({ children }: PropsWithChildren) => { + const [paneVisible, setPaneVisible] = useState(visible) + updatePaneVisible = setPaneVisible + + useLayoutEffect(() => { + if (submitOnHide && !paneVisible) { + requestComposerSubmit('ship while hiding', { target: scopeTarget }) + } + }, [paneVisible]) + + return ( + + + +
+ {children} +
+ + + + ) + } + + const hook = renderHook( + () => + useComposerSubmit({ + activeQueueSessionKey: sessionKey, + activeQueueSessionKeyRef: { current: sessionKey }, + attachments, + busy, + compacting, + clearDraft, + disabled: false, + draftRef, + drainNextQueued: vi.fn(async () => false), + editorRef, + exitQueuedEdit: vi.fn(() => false), + focusInput: vi.fn(), + inputDisabled, + loadIntoComposer: vi.fn(), + onCancel, + onSteer, + onSubmit, + queueCurrentDraft, + queueEdit: null, + queuedPrompts: [], + sessionId: 'runtime-session', + setComposerText: vi.fn(), + stashAt: vi.fn() + }), + { wrapper: Wrapper } ) - return { clearDraft, hook, onCancel, onSteer, onSubmit, queueCurrentDraft } + return { + clearDraft, + hook, + onCancel, + onSteer, + onSubmit, + queueCurrentDraft, + composerSurfaceId: resolvedSurfaceId, + setPaneVisible(nextVisible: boolean) { + if (!updatePaneVisible) { + throw new Error('Pane visibility setter was not initialized') + } + + updatePaneVisible(nextVisible) + } + } } +describe('useComposerSubmit external request routing', () => { + afterEach(() => { + cleanup() + vi.restoreAllMocks() + }) + + it('does not fan out a main ship across keep-alives or other projects', async () => { + const visibleMain = renderSubmitHook({ sessionKey: 'session-a' }) + const hiddenMain = renderSubmitHook({ sessionKey: 'session-b', visible: false }) + const visibleTile = renderSubmitHook({ scopeTarget: 'tile:project-b', sessionKey: 'tile-session' }) + const hiddenTile = renderSubmitHook({ + scopeTarget: 'tile:project-c', + sessionKey: 'other-tile', + visible: false + }) + + expect(requestComposerSubmit('ship this branch', { target: 'main' })).toBe(true) + + await waitFor(() => + expect(visibleMain.onSubmit).toHaveBeenCalledWith('ship this branch', { + composerScope: 'session-a' + }) + ) + expect(visibleMain.onSubmit).toHaveBeenCalledTimes(1) + expect(hiddenMain.onSubmit).not.toHaveBeenCalled() + expect(visibleTile.onSubmit).not.toHaveBeenCalled() + expect(hiddenTile.onSubmit).not.toHaveBeenCalled() + }) + + it('routes a tile-targeted submit to that tile only', async () => { + const main = renderSubmitHook({ sessionKey: 'main-session' }) + const tile = renderSubmitHook({ scopeTarget: 'tile:project-b', sessionKey: 'tile-session' }) + + expect(requestComposerSubmit('ship project B', { target: 'tile:project-b' })).toBe(true) + + await waitFor(() => + expect(tile.onSubmit).toHaveBeenCalledWith('ship project B', { + composerScope: 'tile-session' + }) + ) + expect(main.onSubmit).not.toHaveBeenCalled() + }) + + it('uses the captured surface id when two visible composers share a target', async () => { + const first = renderSubmitHook({ sessionKey: 'session-first' }) + const second = renderSubmitHook({ sessionKey: 'session-second' }) + + requestComposerSubmit('ship exactly one session', { surfaceId: second.composerSurfaceId, target: 'main' }) + + await waitFor(() => + expect(second.onSubmit).toHaveBeenCalledWith('ship exactly one session', { + composerScope: 'session-second' + }) + ) + expect(first.onSubmit).not.toHaveBeenCalled() + }) + + it('submits to the session visible at click time even when the same click switches tabs', async () => { + const hiddenA = renderSubmitHook({ sessionKey: 'session-a', visible: false }) + const visibleB = renderSubmitHook({ sessionKey: 'session-b' }) + + act(() => { + requestComposerSubmit('ship session B', { target: 'main' }) + visibleB.setPaneVisible(false) + hiddenA.setPaneVisible(true) + }) + + await waitFor(() => + expect(visibleB.onSubmit).toHaveBeenCalledWith('ship session B', { + composerScope: 'session-b' + }) + ) + expect(hiddenA.onSubmit).not.toHaveBeenCalled() + }) + + it('does not fan out when visible composers do not have queue session keys yet', async () => { + const firstNewSession = renderSubmitHook({ sessionKey: null }) + const secondNewSession = renderSubmitHook({ sessionKey: null }) + + act(() => { + requestComposerSubmit('ship the visible new session', { target: 'main' }) + }) + + await waitFor(() => expect(firstNewSession.onSubmit).toHaveBeenCalledTimes(1)) + expect(secondNewSession.onSubmit).not.toHaveBeenCalled() + }) + + it('fails closed when the visible composer has no surface identity', () => { + const unidentified = renderSubmitHook({ surfaceId: null }) + + expect(requestComposerSubmit('do not broadcast this', { target: 'main' })).toBe(false) + expect(unidentified.onSubmit).not.toHaveBeenCalled() + }) + + it('fails closed when a pinned origin surface is no longer visible', () => { + const hidden = renderSubmitHook({ sessionKey: 'hidden-origin', visible: false }) + const visible = renderSubmitHook({ sessionKey: 'other-visible' }) + + expect( + requestComposerSubmit('do not send to a stale origin', { + surfaceId: hidden.composerSurfaceId, + target: 'main' + }) + ).toBe(false) + expect(hidden.onSubmit).not.toHaveBeenCalled() + expect(visible.onSubmit).not.toHaveBeenCalled() + }) + + it('does not submit through a composer whose pane is hidden during the request', () => { + const main = renderSubmitHook({ submitOnHide: true }) + + act(() => main.setPaneVisible(false)) + + expect(main.onSubmit).not.toHaveBeenCalled() + }) + + it('does not submit through a disabled composer', () => { + const disabled = renderSubmitHook({ inputDisabled: true }) + + requestComposerSubmit('do not send this', { target: 'main' }) + + expect(disabled.onSubmit).not.toHaveBeenCalled() + }) +}) + describe('useComposerSubmit busy-turn routing', () => { afterEach(() => { cleanup() diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts index 7cfa31a13e11..9435881b43dd 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts @@ -1,5 +1,6 @@ -import { type RefObject, useEffect, useRef } from 'react' +import { type RefObject, useLayoutEffect, useRef } from 'react' +import { usePaneVisible } from '@/components/pane-shell/pane-visibility' import { SLASH_COMMAND_RE } from '@/lib/chat-runtime' import { triggerHaptic } from '@/lib/haptics' import { hasClarifyRequest, skipClarifyRequest } from '@/store/clarify' @@ -13,7 +14,7 @@ import { cloneAttachments, type QueueEditState } from '../composer-utils' import { onComposerSubmitRequest } from '../focus' import { pathifyRefs } from '../path-refs' import { composerPlainText } from '../rich-editor' -import { useComposerScope } from '../scope' +import { useComposerScope, useComposerSurfaceId } from '../scope' import type { ChatBarProps } from '../types' interface UseComposerSubmitArgs { @@ -76,7 +77,9 @@ export function useComposerSubmit({ setComposerText, stashAt }: UseComposerSubmitArgs) { + const paneVisible = usePaneVisible() const scope = useComposerScope() + const surfaceId = useComposerSurfaceId() // Shared send primitive: fire onSubmit, and if the gateway rejects (accepted // === false) or throws, re-load + re-stash the draft so the words survive. @@ -103,19 +106,26 @@ export function useComposerSubmit({ } // External "submit this prompt" requests (e.g. the review pane's agent-ship - // button) route through the same send path. A ref keeps the listener stable - // while always calling the latest dispatchSubmit closure. + // button) route through the same send path. Match both the composer target + // and the exact visible surface captured at click time — every tile stays + // mounted, and a session can be rendered in more than one pane. const dispatchSubmitRef = useRef(dispatchSubmit) dispatchSubmitRef.current = dispatchSubmit - useEffect( + useLayoutEffect( () => - onComposerSubmitRequest(({ target, text, displayKind }) => { - if (target === 'main' && !inputDisabled) { + onComposerSubmitRequest(({ surfaceId: requestedSurfaceId, target, text, displayKind }) => { + if ( + target === scope.target && + surfaceId !== null && + requestedSurfaceId === surfaceId && + paneVisible && + !inputDisabled + ) { dispatchSubmitRef.current(text, undefined, displayKind) } }), - [inputDisabled] + [inputDisabled, paneVisible, scope.target, surfaceId] ) const submitDraft = () => { diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index c61f81f1fc2c..763f62f13ed7 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -1278,7 +1278,7 @@ export function ChatBar({ // A tile's rail reviews ITS worktree: pin the pane's scope to // this surface's cwd. Main keeps the classic follow-the- // active-session scope (null). - onOpen={() => toggleReview(scope.target === 'main' ? null : (cwd ?? null))} + onOpen={() => toggleReview(scope.target === 'main' ? null : (cwd ?? null), scope.target)} onOpenWorktree={openInWorktree} onSwitchBranch={handleSwitchBranch} repoPath={cwd} diff --git a/apps/desktop/src/app/chat/composer/scope.tsx b/apps/desktop/src/app/chat/composer/scope.tsx index e7e7aff880ef..a14b53ca42f3 100644 --- a/apps/desktop/src/app/chat/composer/scope.tsx +++ b/apps/desktop/src/app/chat/composer/scope.tsx @@ -43,3 +43,15 @@ const ComposerScopeContext = createContext(MAIN_COMPOSER_SCOPE) export const ComposerScopeProvider = ComposerScopeContext.Provider export const useComposerScope = (): ComposerScope => useContext(ComposerScopeContext) + +/** + * Unique identity for one mounted ChatView/composer pair. Session ids cannot + * fill this role: a fresh chat has no id yet, and the same stored session can + * be rendered in more than one layout pane. External submit requests pin this + * surface id at click time so exactly one composer can claim the task. + */ +const ComposerSurfaceContext = createContext(null) + +export const ComposerSurfaceProvider = ComposerSurfaceContext.Provider + +export const useComposerSurfaceId = (): string | null => useContext(ComposerSurfaceContext) diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index 13ed225186d9..f3e72d7378e3 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -3,7 +3,7 @@ import { useStore } from '@nanostores/react' import { useQuery } from '@tanstack/react-query' import type { ReadableAtom } from 'nanostores' import type * as React from 'react' -import { memo, Suspense, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { memo, Suspense, useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' import { useLocation } from 'react-router' import type { SubmitTextOptions } from '@/app/session/hooks/use-prompt-actions/utils' @@ -56,7 +56,7 @@ import { ChatSwapOverlay } from './chat-swap-overlay' import { ChatBar, ChatBarFallback } from './composer' import { requestComposerInsert } from './composer/focus' import { droppedFileInlineRefs } from './composer/inline-refs' -import { useComposerScope } from './composer/scope' +import { ComposerSurfaceProvider, useComposerScope, useComposerSurfaceId } from './composer/scope' import type { ChatBarState } from './composer/types' import { type DroppedFile, partitionDroppedFiles } from './hooks/use-composer-actions' import { type DragKind, useFileDropZone } from './hooks/use-file-drop-zone' @@ -320,7 +320,17 @@ function ChatRuntimeBoundary({ // Memoized: the tile caller (session-tile.tsx) and the contrib surface re-render // on idle ticks unrelated to the chat; with stable callback props (hoisted to // useCallback at the call sites) memo() lets the whole chat shell skip those. -export const ChatView = memo(function ChatView({ +export const ChatView = memo(function ChatView(props: ChatViewProps) { + const composerSurfaceId = useId() + + return ( + + + + ) +}) + +const ChatViewContent = memo(function ChatViewContent({ className, gateway, modelMenuContent, @@ -355,6 +365,7 @@ export const ChatView = memo(function ChatView({ // atoms) or a tile's session slice — same component either way. const view = useSessionView() const composerScope = useComposerScope() + const composerSurfaceId = useComposerSurfaceId() const isPrimary = view.kind === 'primary' const activeSessionId = useStore(view.$runtimeId) const storedId = useStore(view.$storedId) @@ -558,6 +569,7 @@ export const ChatView = memo(function ChatView({ className )} data-chat-surface="" + data-composer-surface-id={composerSurfaceId} data-composer-target={composerScope.target} data-session-anchor={sessionAnchor} > diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index 5c65fd3f19ca..f5d400a2043d 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -59,7 +59,14 @@ import { SIDEBAR_MAX_WIDTH } from '@/store/layout' import { runExportProfileFlow, runImportProfileFlow } from '@/store/profile-share' -import { $reviewOpen, closeReview, openReview, REVIEW_PANE_ID } from '@/store/review' +import { + $reviewOpen, + $reviewScopeCwd, + $reviewScopeTarget, + closeReview, + openReview, + REVIEW_PANE_ID +} from '@/store/review' import { $currentCwd, $selectedStoredSessionId, $sessions, $yoloActive, sessionMatchesStoredId } from '@/store/session' import { watchSessionPins } from '@/store/session-pin-sync' import { watchUnreadWriteGuard } from '@/store/session-unread-remote' @@ -580,7 +587,7 @@ bindPaneVisibility( 'review', computed([$reviewOpen, $hasWorkspace], (open, workspace) => open && workspace), closeReview, - openReview + () => openReview($reviewScopeCwd.get(), $reviewScopeTarget.get()) ) // ⌃` / statusbar toggle — the terminal COLLAPSES to a rail (tab stays), not // hides; PTYs stay alive while collapsed (see PersistentTerminal). diff --git a/apps/desktop/src/app/right-sidebar/review/ship-bar.tsx b/apps/desktop/src/app/right-sidebar/review/ship-bar.tsx index 2f85a9ade439..3d0e03325fda 100644 --- a/apps/desktop/src/app/right-sidebar/review/ship-bar.tsx +++ b/apps/desktop/src/app/right-sidebar/review/ship-bar.tsx @@ -15,6 +15,7 @@ import { $reviewCommitDefault, $reviewCommitMsgBusy, $reviewFiles, + $reviewScopeTarget, $reviewShipBusy, $reviewShipInfo, cancelCommitMessage, @@ -35,6 +36,7 @@ export function ReviewShipBar() { const c = t.statusStack.coding const files = useStore($reviewFiles) const ship = useStore($reviewShipInfo) + const scopeTarget = useStore($reviewScopeTarget) const busy = useStore($reviewShipBusy) const generating = useStore($reviewCommitMsgBusy) const commitDefault = useStore($reviewCommitDefault) @@ -129,7 +131,11 @@ export function ReviewShipBar() {