diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index f4469e74eda..2894f5afb48 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -372,8 +372,10 @@ const { blocks: [] as unknown[], messages: [] as unknown[], queuedPromptHoldHistory: [] as boolean[], + queuedPromptStreamingState: 'idle', chatEditorRenderCount: 0, latestChatEditorProps: null as ChatEditorTestProps | null, + onChatEditorLayout: null as ((props: ChatEditorTestProps) => void) | null, latestToastHostElevated: false, latestStatusBarTasks: null as DaemonSessionMonitorTaskStatus[] | null, latestStatusBarOnOpenTasks: null as (() => void) | null, @@ -600,10 +602,14 @@ vi.mock('./hooks/useAnimationFrameValue', () => ({ })); vi.mock('./hooks/useQueuedPrompts', () => ({ - useQueuedPrompts: (args: { holdQueuedPromptsLocally?: boolean }) => { + useQueuedPrompts: (args: { + holdQueuedPromptsLocally?: boolean; + streamingState: string; + }) => { testState.queuedPromptHoldHistory.push( args.holdQueuedPromptsLocally === true, ); + testState.queuedPromptStreamingState = args.streamingState; return { queuedPrompts: [], queuedTexts, @@ -663,6 +669,9 @@ vi.mock('./components/ChatEditor', async () => { ), ); }, [onAttachmentsChange]); + React.useLayoutEffect(() => { + testState.onChatEditorLayout?.(props); + }); React.useImperativeHandle(ref, () => ({ clear: () => { testState.prompt = ''; @@ -4785,8 +4794,10 @@ beforeEach(() => { testState.blocks = []; testState.messages = []; testState.queuedPromptHoldHistory = []; + testState.queuedPromptStreamingState = 'idle'; testState.chatEditorRenderCount = 0; testState.latestChatEditorProps = null; + testState.onChatEditorLayout = null; testState.latestToastHostElevated = false; testState.latestStatusBarTasks = null; testState.latestStatusBarOnOpenTasks = null; @@ -11007,16 +11018,65 @@ describe('App session callbacks', () => { expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); expect(onToast).toHaveBeenCalledWith( 'error', - "Slash commands can't be queued while a turn is running.", + 'Slash commands are unavailable while a Goal owns the session or its state is loading.', ); }); - it('holds a composer prompt while Goal state is still hydrating', async () => { - // The session load clears `loadingTranscript` before its `goal()` fetch - // resolves, so the composer is writable while the Goal state is unknown. - // The queue-hold gate already fails closed on that state; a direct submit - // must too, or a prompt typed in that window is sent straight into a Goal - // the client has not learned about yet. + it.each([ + [ + 'hidden command', + '/internal deploy', + { hiddenSlashCommands: ['internal'] }, + ], + ['fast model command', '/model --fast qwen-max', {}], + ['skill command', '/skills deployer', {}], + ['delegated rename', '/rename --auto', {}], + ])( + 'refuses an enqueue-style %s while a Goal owns the session', + async (_label, command, props) => { + const onToast = vi.fn(); + mockConnection.goalState = activeGoalSnapshot('keep working'); + renderApp({ ...props, onToast }); + await flush(); + + let accepted: boolean | undefined; + await act(async () => { + accepted = testState.latestChatEditorProps?.onSubmit(command); + await flush(); + }); + + expect(accepted).toBe(false); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + 'Slash commands are unavailable while a Goal owns the session or its state is loading.', + ); + }, + ); + + it.each(['idle', 'responding'] as const)( + 'does not enqueue a forwarded command while Goal state is hydrating (%s)', + async (streamingState) => { + mockConnection.goalState = undefined; + testState.streamingState = streamingState; + renderApp(); + await flush(); + + let accepted: boolean | undefined; + await act(async () => { + accepted = + testState.latestChatEditorProps?.onSubmit('/deploy production'); + await flush(); + }); + + expect(accepted).toBe(false); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + }, + ); + + it('sends an idle composer prompt while Goal state is still hydrating', async () => { mockConnection.goalState = undefined; renderApp(); await flush(); @@ -11029,20 +11089,127 @@ describe('App session callbacks', () => { await flush(); }); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'hello during hydration', + expect.objectContaining({ retry: undefined }), + ); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + expect(accepted).toBe(true); + }); + + it('inserts a hydrating composer prompt while the session is active', async () => { + mockConnection.goalState = undefined; + testState.streamingState = 'responding'; + renderApp(); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('hello during active turn'); + await flush(); + }); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); expect(rawEnqueuePrompt).toHaveBeenCalledTimes(1); - expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('hello during hydration'); - expect(accepted).toBe(true); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe( + 'hello during active turn', + ); }); - it('holds queued prompts on the first render of an active Goal', async () => { + it('inserts a prompt before the first stream event reaches the client', async () => { + testState.sessionHasActivePrompt = true; + const { rerender } = renderApp(); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('hello before first token'); + await flush(); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).toHaveBeenCalledTimes(1); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe( + 'hello before first token', + ); + expect(testState.queuedPromptStreamingState).toBe('responding'); + + mockSessionActions.sendPrompt.mockClear(); + rawEnqueuePrompt.mockClear(); + testState.sessionHasActivePrompt = false; + rerender(); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('hello after completion'); + await flush(); + }); + + expect(testState.queuedPromptStreamingState).toBe('idle'); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledTimes(1); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + }); + + it('enqueues a forwarded command while the session is active and Goal is idle', async () => { + testState.streamingState = 'responding'; + renderApp(); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('/deploy production'); + await flush(); + }); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).toHaveBeenCalledTimes(1); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('/deploy production'); + }); + + it('inserts a prompt when the session becomes active before effects run', async () => { + mockConnection.goalState = undefined; + const { rerender } = renderApp(); + await flush(); + + let submitted = false; + testState.onChatEditorLayout = (props) => { + if (!props.isRunning || submitted) return; + submitted = true; + props.onSubmit('hello during active transition'); + }; + testState.streamingState = 'responding'; + + rerender({}); + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(rawEnqueuePrompt).toHaveBeenCalledTimes(1); + expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe( + 'hello during active transition', + ); + }); + + it('sends an idle composer prompt when an active Goal is known', async () => { + mockConnection.goalState = activeGoalSnapshot(); + renderApp(); + await flush(); + + await act(async () => { + testState.latestChatEditorProps?.onSubmit('hello during active Goal'); + await flush(); + }); + + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'hello during active Goal', + expect.objectContaining({ retry: undefined }), + ); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); + }); + + it('does not locally hold prompts for an active Goal', async () => { mockConnection.goalState = activeGoalSnapshot(); renderApp(); await flush(); expect(testState.queuedPromptHoldHistory.length).toBeGreaterThan(0); - expect(testState.queuedPromptHoldHistory).not.toContain(false); + expect(testState.queuedPromptHoldHistory).not.toContain(true); }); it('restores the Goal snapshot when the same session learns its workspace', async () => { @@ -13176,8 +13343,9 @@ describe('App session callbacks', () => { ); }); - it('allows manual retry after a model stream interrupted turn error', async () => { + it('allows manual retry after a model stream interrupted turn error with an active Goal', async () => { const retrySend = deferred(); + mockConnection.goalState = activeGoalSnapshot('keep working'); const { container, rerender } = renderApp(); await flush(); @@ -13204,6 +13372,19 @@ describe('App session callbacks', () => { expect(container.querySelector('[data-testid="retry"]')).not.toBeNull(); + const staleRetry = testState.latestMessageListProps?.onRetryClick; + act(() => { + testState.sessionHasActivePrompt = true; + rerender(); + }); + expect(container.querySelector('[data-testid="retry"]')).toBeNull(); + act(() => staleRetry?.()); + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + act(() => { + testState.sessionHasActivePrompt = false; + rerender(); + }); + mockSessionActions.sendPrompt.mockImplementationOnce( () => retrySend.promise, ); @@ -17966,6 +18147,68 @@ describe('App session callbacks', () => { expect(settingsReload).toHaveBeenCalled(); }); + it.each([ + ['fast-model selection', ['open-fast-model', 'model-select']], + ['settings language change', ['change-language-workspace']], + ])( + 'shows the Goal-specific rejection for a blocked %s', + async (_label, actions) => { + const onToast = vi.fn(); + mockConnection.goalState = activeGoalSnapshot('keep working'); + const { container } = renderApp({ onToast }); + await flush(); + testState.prompt = '/settings'; + await clickSubmit(container); + await flush(); + + for (const action of actions) { + await act(async () => { + container + .querySelector(`[data-testid="${action}"]`) + ?.click(); + await Promise.resolve(); + }); + } + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + 'Slash commands are unavailable while a Goal owns the session or its state is loading.', + ); + }, + ); + + it.each([ + ['fast-model selection', ['open-fast-model', 'model-select']], + ['settings language change', ['change-language-workspace']], + ])( + 'blocks %s before the first stream event reaches the client', + async (_label, actions) => { + const onToast = vi.fn(); + testState.sessionHasActivePrompt = true; + const { container } = renderApp({ onToast }); + await flush(); + testState.prompt = '/settings'; + await clickSubmit(container); + await flush(); + + for (const action of actions) { + await act(async () => { + container + .querySelector(`[data-testid="${action}"]`) + ?.click(); + await Promise.resolve(); + }); + } + + expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); + expect(onToast).toHaveBeenCalledWith( + 'error', + "Slash commands can't be queued while a turn is running.", + ); + }, + ); + it('clears model selection busy state after a same-session reattach', async () => { const selection = deferred(); mockSessionActions.setModel.mockReturnValueOnce(selection.promise); @@ -18629,8 +18872,9 @@ describe('App prompt send failure retry', () => { warn.mockRestore(); }); - it('marks the failed message and retries its original payload without a duplicate', async () => { + it('marks and retries a failed prompt while an active Goal is known', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); + mockConnection.goalState = activeGoalSnapshot('keep working'); const firstSend = deferred(); mockSessionActions.sendPrompt.mockImplementationOnce(() => { testState.blocks = [{ id: 'u1', kind: 'user' }]; @@ -18646,7 +18890,7 @@ describe('App prompt send failure retry', () => { }, ] as DaemonInputAnnotation[]; const images = [{ data: 'aGVsbG8=', media_type: 'image/png' }]; - renderApp(); + const { rerender } = renderApp(); await flush(); act(() => { @@ -18671,6 +18915,21 @@ describe('App prompt send failure retry', () => { ?.textContent, ).toBe('u1'); + const staleRetry = testState.latestMessageListProps?.onRetryFailedPrompt; + act(() => { + testState.sessionHasActivePrompt = true; + rerender(); + }); + expect( + document.querySelector('[data-testid="failed-prompt-retry"]'), + ).toBeNull(); + act(() => staleRetry?.()); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledTimes(1); + act(() => { + testState.sessionHasActivePrompt = false; + rerender(); + }); + await act(async () => { document .querySelector('[data-testid="failed-prompt-retry"]') @@ -20312,10 +20571,7 @@ describe('App /goal command', () => { it('installs the allocated-session Goal before its re-sync resolves', async () => { // `workspaceActions.controlGoal` does not write `connection.goalState`, so - // without installing the create response the state stays goal-less for a - // whole round trip (up to the action timeout if the GET stalls): the hold - // gate reads false and a prompt typed in that window bypasses the Goal - // queue entirely. + // install the create response without waiting for the follow-up GET. const active = activeGoalSnapshot('first objective'); mockConnection.sessionId = undefined; mockSessionActions.createSession.mockResolvedValueOnce({ @@ -20346,7 +20602,7 @@ describe('App /goal command', () => { await flush(); expect(mockConnection.goalState).toBe(active); - expect(testState.queuedPromptHoldHistory.at(-1)).toBe(true); + expect(testState.queuedPromptHoldHistory.at(-1)).toBe(false); // The App's own snapshot drives the strip; asserting only the connection // state would re-read what this test's mock wrote. expect( @@ -20356,14 +20612,17 @@ describe('App /goal command', () => { rawEnqueuePrompt.mockClear(); mockSessionActions.sendPrompt.mockClear(); - testState.prompt = 'bypass me'; + testState.prompt = 'continue normally'; await act(async () => { - testState.latestChatEditorProps?.onSubmit('bypass me'); + testState.latestChatEditorProps?.onSubmit('continue normally'); await flush(); }); - expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled(); - expect(rawEnqueuePrompt.mock.calls[0]?.[0]).toBe('bypass me'); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'continue normally', + expect.objectContaining({ retry: undefined }), + ); + expect(rawEnqueuePrompt).not.toHaveBeenCalled(); }); it('keeps a session-less /goal control in the composer', async () => { diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index bcb34225b98..719c69252b3 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -2124,6 +2124,8 @@ export function App({ connection.workspaceCwd, connection.sessionId, ); + const sessionHasActivePromptRef = useRef(sessionHasActivePrompt); + sessionHasActivePromptRef.current = sessionHasActivePrompt; const refreshWorkspaceCapabilities = workspace.refreshCapabilities; const workspaces = useMemo(() => { const capabilityWorkspaces = workspace.capabilities?.workspaces ?? []; @@ -4225,6 +4227,10 @@ export function App({ const [isStartingNewSessionSuggestion, setIsStartingNewSessionSuggestion] = useState(false); const streamingState = useStreamingState(); + const queuedPromptStreamingState = + streamingState === 'idle' && sessionHasActivePrompt + ? 'responding' + : streamingState; const failedPromptRetryIsCurrent = Boolean( failedPromptRetry && retryOwnerMatchesCurrent( @@ -4253,9 +4259,7 @@ export function App({ (!failedPromptRetry.admitted || failedPromptRetry.settled), ); const streamingStateRef = useRef(streamingState); - useEffect(() => { - streamingStateRef.current = streamingState; - }, [streamingState]); + streamingStateRef.current = streamingState; // Cleared in three places: the session-switch effect, the drain loop, and // handleCancel. Bumping drainGenerationRef at each clear site also cancels // any in-flight inline ! command whose ensureSessionForPrompt is resolving. @@ -5386,8 +5390,8 @@ export function App({ /** * Whether a local action must be held back because a Goal owns the session. * Reads the latest connection through the ref so callers get the gate as of - * call time; the fail-closed hydration convention lives in the shared - * predicate, which every Goal gate in the client shares. + * call time. Commands and run guards fail closed during Goal hydration; + * ordinary chat submissions do not consult this gate. */ const isGoalGateBlocked = useCallback( () => isGoalGateBlockedFor(connectionRef.current), @@ -6143,7 +6147,8 @@ export function App({ if ( sessionWriteBlockedRef.current || promptPreparationOwnerRef.current || - isGoalGateBlocked() + streamingStateRef.current !== 'idle' || + sessionHasActivePromptRef.current ) { return; } @@ -6280,7 +6285,6 @@ export function App({ t, updateFailedPrompt, updateUnknownPromptAdmission, - isGoalGateBlocked, ]); const canMutateMidTurn = connection.capabilities?.features.includes( @@ -6311,11 +6315,7 @@ export function App({ canQueryMidTurn, canInjectMidTurnMedia, workspaceFileActions: artifactWorkspaceActions, - streamingState, - holdQueuedPromptsLocally: - connection.sessionId !== undefined && - (connection.goalState === undefined || - connection.goalState.goal?.status === 'active'), + streamingState: queuedPromptStreamingState, sessionActions, store, editorRef, @@ -6676,10 +6676,17 @@ export function App({ [store, resumeChatBottomFollow], ); - const blockLocalCommandDuringTurn = useCallback((): false => { - pushToast('error', t('queue.commandBlocked')); + const blockCommand = useCallback((): false => { + pushToast( + 'error', + t( + isGoalGateBlocked() + ? 'queue.commandGoalBlocked' + : 'queue.commandBlocked', + ), + ); return false; - }, [pushToast, t]); + }, [isGoalGateBlocked, pushToast, t]); const handleThemeChange = useCallback( (nextTheme: WebShellTheme) => { @@ -7097,9 +7104,13 @@ export function App({ reloadWorkspaceSettings(), ]); }; - if (streamingStateRef.current !== 'idle' || isGoalGateBlocked()) { + if ( + streamingStateRef.current !== 'idle' || + sessionHasActivePromptRef.current || + isGoalGateBlocked() + ) { handleLanguageChange(previousLanguage); - blockLocalCommandDuringTurn(); + blockCommand(); return; } sendPrompt(command, undefined, undefined, { ownerRef: owner }) @@ -7111,7 +7122,7 @@ export function App({ }); }, [ - blockLocalCommandDuringTurn, + blockCommand, handleLanguageChange, reloadWorkspaceSettings, reportError, @@ -8717,10 +8728,7 @@ export function App({ }); // The workspace-scoped control does not write `connection.goalState` // the way `sessionActions.controlGoal` does, so install the create - // response directly. Until it lands, `holdQueuedPromptsLocally` reads - // false and the sync effect re-derives the local snapshot to null — a - // prompt typed in that window would go straight to the daemon instead - // of the Goal queue, and no Goal strip would render. + // response directly to keep the Goal strip and controls authoritative. sessionActions.applyGoalSnapshot(sessionId, response.snapshot); if ( !connectionRef.current.sessionId || @@ -8933,8 +8941,22 @@ export function App({ pushToast('warning', t('editor.connectionDisconnected')); return false; } - const promptBlocked = - streamingStateRef.current !== 'idle' || isGoalGateBlocked(); + const goalBlocked = isGoalGateBlocked(); + const sessionActive = + streamingStateRef.current !== 'idle' || + sessionHasActivePromptRef.current; + const commandBlocked = sessionActive || goalBlocked; + const enqueueBlockedCommand = (commandText: string) => { + if (goalBlocked) return blockCommand(); + return enqueuePrompt( + commandText, + images, + files, + undefined, + commitComposerAccepted, + metadata?.inputAnnotations, + ); + }; const submitPromptFromEditor = ( promptText: string, promptImages: PromptImage[] | undefined, @@ -9054,15 +9076,8 @@ export function App({ if (match) { const cmd = match[1]; if (hiddenCommands.has(normalizeHiddenCommand(cmd))) { - if (promptBlocked) { - return enqueuePrompt( - text, - images, - files, - undefined, - commitComposerAccepted, - metadata?.inputAnnotations, - ); + if (commandBlocked) { + return enqueueBlockedCommand(text); } return submitPromptFromEditor( text, @@ -9188,7 +9203,7 @@ export function App({ // (turn in flight, or a Goal owning the session) refuse the // command instead of switching the UI alone — the language // picker treats the identical condition the same way. - if (promptBlocked) return blockLocalCommandDuringTurn(); + if (commandBlocked) return blockCommand(); handleLanguageChange(nextLanguage); { const deferComposerCommit = @@ -9251,13 +9266,13 @@ export function App({ return true; } if (cmd === 'branch') { - if (promptBlocked) return blockLocalCommandDuringTurn(); + if (commandBlocked) return blockCommand(); const branchName = text.slice(match[0].length).trim(); branchCurrentSession(branchName || undefined); return true; } if (cmd === 'fork') { - if (promptBlocked) return blockLocalCommandDuringTurn(); + if (commandBlocked) return blockCommand(); if (!requireActiveSessionForLocalCommand()) return false; const directive = text.slice(match[0].length).trim(); if (!directive) { @@ -9298,15 +9313,8 @@ export function App({ return true; } if (modelArg.startsWith('--fast ')) { - if (promptBlocked) { - return enqueuePrompt( - text, - images, - files, - undefined, - commitComposerAccepted, - metadata?.inputAnnotations, - ); + if (commandBlocked) { + return enqueueBlockedCommand(text); } return submitPromptFromEditor( text, @@ -9365,7 +9373,7 @@ export function App({ return true; } if (cmd === 'plan') { - if (promptBlocked) return blockLocalCommandDuringTurn(); + if (commandBlocked) return blockCommand(); const prompt = text.slice(match[0].length).trim(); if (!connectionRef.current.sessionId) { setPendingMode('plan'); @@ -9456,15 +9464,8 @@ export function App({ openPanel('skills'); } else { const skillPrompt = `/${skillArg}`; - if (promptBlocked) { - return enqueuePrompt( - skillPrompt, - images, - files, - undefined, - commitComposerAccepted, - metadata?.inputAnnotations, - ); + if (commandBlocked) { + return enqueueBlockedCommand(skillPrompt); } return submitPromptFromEditor( skillPrompt, @@ -9575,7 +9576,7 @@ export function App({ if (subCommand === 'install') { // Install echoes into the transcript (and its error/usage replies // do too); block it mid-turn so it can't split the active turn. - if (promptBlocked) return blockLocalCommandDuringTurn(); + if (commandBlocked) return blockCommand(); const tokens = args.slice('install'.length).trim().split(/\s+/); let source = ''; let ref: string | undefined; @@ -9686,15 +9687,8 @@ export function App({ if (cmd === 'rename') { const renameArg = parseRenameArgument(text.slice(match[0].length)); if (renameArg.type === 'auto' || renameArg.type === 'delegate') { - if (promptBlocked) { - return enqueuePrompt( - text, - images, - files, - undefined, - commitComposerAccepted, - metadata?.inputAnnotations, - ); + if (commandBlocked) { + return enqueueBlockedCommand(text); } return submitPromptFromEditor( text, @@ -9918,15 +9912,8 @@ export function App({ } } // Forward slash commands as prompts - if (promptBlocked) { - return enqueuePrompt( - text, - images, - files, - undefined, - commitComposerAccepted, - metadata?.inputAnnotations, - ); + if (commandBlocked) { + return enqueueBlockedCommand(text); } return submitPromptFromEditor( text, @@ -9999,7 +9986,7 @@ export function App({ }); return !needsSession; } else { - if (promptBlocked) { + if (sessionActive) { return enqueuePrompt( text, images, @@ -10047,7 +10034,7 @@ export function App({ handleThemeChange, handleSetMode, handleLanguageChange, - blockLocalCommandDuringTurn, + blockCommand, createSideTask, sideTasksAvailable, openEnvironmentTasksPanel, @@ -10198,17 +10185,14 @@ export function App({ ); const handleRetry = useCallback(() => { - if ( - sessionWriteBlockedRef.current || - promptPreparationOwnerRef.current || - isGoalGateBlocked() - ) { + if (sessionWriteBlockedRef.current || promptPreparationOwnerRef.current) { return; } if ( showRetryHintRef.current && connected && streamingStateRef.current === 'idle' && + !sessionHasActivePromptRef.current && retryableTurnErrorIdRef.current && retryableTurnErrorIdentityRef.current && connectionRef.current.sessionId && @@ -10398,7 +10382,6 @@ export function App({ store, t, updateUnknownPromptAdmission, - isGoalGateBlocked, ]); useEffect(() => { @@ -10591,6 +10574,7 @@ export function App({ const showCurrentRetryHint = Boolean( showRetryHint && !isPreparingPrompt && + !sessionHasActivePrompt && retryableTurnErrorIdentity && matchesTurnErrorIdentity( getRetryableTurnError(blocks), @@ -10792,8 +10776,12 @@ export function App({ const handleFastModelSelect = useCallback( (modelId: string) => { - if (streamingState !== 'idle' || isGoalGateBlocked()) { - blockLocalCommandDuringTurn(); + if ( + streamingState !== 'idle' || + sessionHasActivePromptRef.current || + isGoalGateBlocked() + ) { + blockCommand(); return; } // Model IDs from the picker arrive as bare model IDs (baseModelId), not @@ -10840,7 +10828,7 @@ export function App({ }); }, [ - blockLocalCommandDuringTurn, + blockCommand, closePanel, sendPrompt, streamingState, @@ -12457,7 +12445,9 @@ export function App({ showRetryHint={showCurrentRetryHint} onRetryClick={handleRetry} failedPromptMessageId={ - isPreparingPrompt + isPreparingPrompt || + streamingState !== 'idle' || + sessionHasActivePrompt ? undefined : visibleFailedPromptBlock?.id } @@ -12777,7 +12767,9 @@ export function App({ prompts={queuedPrompts} t={t} canMutateMidTurn={canMutateMidTurn} - canInsertMidTurn={streamingState !== 'idle'} + canInsertMidTurn={ + queuedPromptStreamingState !== 'idle' + } onDelete={removeQueuedPrompt} onInsert={insertQueuedPrompt} onEdit={editQueuedPrompt} diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 990df99a2b1..0ba95df00c4 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -32,6 +32,7 @@ let connectionState: any; let streamingStateValue: string; let pendingPermission: any; let sessionHasActivePromptValue: boolean; +let queuedPromptStreamingState: string | undefined; let latestOnSubmit: | (( text: string, @@ -141,15 +142,18 @@ vi.mock('../session-catalog/session-catalog-hooks', () => ({ })); vi.mock('../hooks/useQueuedPrompts', () => ({ - useQueuedPrompts: () => ({ - queuedPrompts: queuedPromptsMock, - queuedTexts: queuedTextsMock, - enqueuePrompt, - removeQueuedPrompt, - editQueuedPrompt, - editLastQueuedPrompt, - clearQueuedPrompts, - }), + useQueuedPrompts: (args: { streamingState: string }) => { + queuedPromptStreamingState = args.streamingState; + return { + queuedPrompts: queuedPromptsMock, + queuedTexts: queuedTextsMock, + enqueuePrompt, + removeQueuedPrompt, + editQueuedPrompt, + editLastQueuedPrompt, + clearQueuedPrompts, + }; + }, })); let messagesState: any[]; @@ -393,10 +397,8 @@ beforeEach(() => { workspaceCwd: '/w', loadingTranscript: false, catchingUp: false, - // A loaded session always carries a Goal snapshot (the load falls back to - // an idle one when the fetch fails), and the Goal gates fail CLOSED on an - // absent one — leaving it out here would model a session that is still - // hydrating, not a Goal-less one. + // A loaded session normally carries a Goal snapshot; tests that exercise + // the hydration window set it back to undefined. goalState: { v: 2, activity: 'idle', goal: null }, }; streamingStateValue = 'idle'; @@ -406,6 +408,7 @@ beforeEach(() => { latestChatEditorProps = undefined; renderRealChatEditor = false; sessionHasActivePromptValue = false; + queuedPromptStreamingState = undefined; latestComposerCoreOptions.current = null; latestFollowupAccept = undefined; latestMonitorDetailsOnOpen = undefined; @@ -608,9 +611,8 @@ describe('ChatPane', () => { }); it('offers Insert only while a turn is running', () => { - // Between two Goal turns streaming is idle while the hold keeps queued - // prompts visible. `insertQueuedPrompt` no-ops at idle, so the affordance - // has to disappear with it rather than render a button that does nothing. + // `insertQueuedPrompt` no-ops at idle, so the affordance has to disappear + // with it rather than render a button that does nothing. queuedPromptsMock = [{ id: 1, text: 'held while the Goal runs' } as never]; connectionState.goalState = { v: 2, @@ -633,6 +635,14 @@ describe('ChatPane', () => { expect(testid('pane-queue')?.dataset['canInsertMidTurn']).toBe('false'); act(() => { + sessionHasActivePromptValue = true; + rerender(); + }); + + expect(testid('pane-queue')?.dataset['canInsertMidTurn']).toBe('true'); + + act(() => { + sessionHasActivePromptValue = false; streamingStateValue = 'responding'; rerender(); }); @@ -1619,13 +1629,23 @@ describe('ChatPane', () => { expect(enqueuePrompt).not.toHaveBeenCalled(); }); - it('holds an idle prompt while the Goal state is still hydrating', () => { - // The session load clears `loadingTranscript` before its `goal()` fetch - // resolves, so the composer is writable with no snapshot yet. The daemon - // has no server-side prompt gate for an active Goal, so a direct send in - // that window bypasses the Goal queue outright — fail closed, exactly as - // the local hold does. + it('sends an idle prompt while the Goal state is still hydrating', () => { + connectionState = { ...connectionState, goalState: undefined }; + render(); + + act(() => + testid('pane-submit')!.dispatchEvent( + new MouseEvent('click', { bubbles: true }), + ), + ); + + expect(sendPrompt).toHaveBeenCalledTimes(1); + expect(enqueuePrompt).not.toHaveBeenCalled(); + }); + + it('inserts a hydrating prompt while the session is active', () => { connectionState = { ...connectionState, goalState: undefined }; + streamingStateValue = 'responding'; render(); act(() => @@ -1636,14 +1656,27 @@ describe('ChatPane', () => { expect(sendPrompt).not.toHaveBeenCalled(); expect(enqueuePrompt).toHaveBeenCalled(); + expect(queuedPromptStreamingState).toBe('responding'); + }); - // ...and the gate reopens once the snapshot lands Goal-less — the window - // is a hold, not a lock. + it('inserts a prompt before the first stream event reaches the pane', () => { + sessionHasActivePromptValue = true; + render(); + + act(() => + testid('pane-submit')!.dispatchEvent( + new MouseEvent('click', { bubbles: true }), + ), + ); + + expect(sendPrompt).not.toHaveBeenCalled(); + expect(enqueuePrompt).toHaveBeenCalled(); + expect(queuedPromptStreamingState).toBe('responding'); + + sendPrompt.mockClear(); + enqueuePrompt.mockClear(); act(() => { - connectionState = { - ...connectionState, - goalState: { v: 2, activity: 'idle', goal: null }, - }; + sessionHasActivePromptValue = false; rerender(); }); act(() => @@ -1652,9 +1685,66 @@ describe('ChatPane', () => { ), ); + expect(queuedPromptStreamingState).toBe('idle'); expect(sendPrompt).toHaveBeenCalledTimes(1); + expect(enqueuePrompt).not.toHaveBeenCalled(); }); + it('sends an idle prompt when an active Goal is known', () => { + connectionState.goalState = { + v: 2, + activity: 'idle', + goal: { + goalId: 'goal-1', + revision: 1, + objective: 'ship it', + status: 'active', + evidenceCursor: { recordId: 'record-1' }, + turnCount: 1, + activeTimeMs: 10, + createdAt: 1, + updatedAt: 1, + }, + }; + render(); + + act(() => + testid('pane-submit')!.dispatchEvent( + new MouseEvent('click', { bubbles: true }), + ), + ); + + expect(sendPrompt).toHaveBeenCalledTimes(1); + expect(enqueuePrompt).not.toHaveBeenCalled(); + + sendPrompt.mockClear(); + let accepted: boolean | undefined; + act(() => { + accepted = latestOnSubmit!('/deploy production'); + }); + expect(accepted).toBe(false); + expect(sendPrompt).not.toHaveBeenCalled(); + expect(enqueuePrompt).not.toHaveBeenCalled(); + }); + + it.each(['idle', 'responding'] as const)( + 'blocks a forwarded slash command while Goal state is hydrating (%s)', + (streamingState) => { + streamingStateValue = streamingState; + connectionState = { ...connectionState, goalState: undefined }; + render(); + + let accepted: boolean | undefined; + act(() => { + accepted = latestOnSubmit!('/deploy production'); + }); + + expect(accepted).toBe(false); + expect(sendPrompt).not.toHaveBeenCalled(); + expect(enqueuePrompt).not.toHaveBeenCalled(); + }, + ); + it('lets the host handle a slash command', () => { const onSlashCommand = vi.fn(() => true); render({ onSlashCommand }); @@ -1689,6 +1779,27 @@ describe('ChatPane', () => { }); }); + it('queues a forwarded slash command while the pane is running', () => { + streamingStateValue = 'responding'; + const onSlashCommand = vi.fn(); + render({ onSlashCommand }); + + act(() => { + latestOnSubmit!('/deploy staging'); + }); + + expect(onSlashCommand).toHaveBeenCalledTimes(1); + expect(sendPrompt).not.toHaveBeenCalled(); + expect(enqueuePrompt).toHaveBeenCalledWith( + '/deploy staging', + undefined, + undefined, + undefined, + undefined, + expect.any(Function), + ); + }); + it('lets the host handle a slash command while the pane is disconnected', () => { connectionState.status = 'disconnected'; const onSlashCommand = vi.fn(() => true); diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index 10ea9925b47..1275983e36f 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -266,11 +266,17 @@ export function ChatPane({ workspaceCwd ?? connection.workspaceCwd, connection.sessionId, ); + const sessionHasActivePromptRef = useRef(sessionHasActivePrompt); + sessionHasActivePromptRef.current = sessionHasActivePrompt; const { blocks, blockChangeSummary } = useAnimationFrameTranscriptSnapshot(); const messages = useMessagesFromBlocks(t, blocks, blockChangeSummary); const transcriptHistory = useTranscriptHistory(); const store = useTranscriptStore(); const streamingState = useStreamingState(); + const queuedPromptStreamingState = + streamingState === 'idle' && sessionHasActivePrompt + ? 'responding' + : streamingState; const [goalControlBusy, setGoalControlBusy] = useState(false); const goalControlOpSeqRef = useRef(0); const goalControlOwnerRef = useRef< @@ -602,8 +608,7 @@ export function ChatPane({ canQueryMidTurn, canInjectMidTurnMedia, workspaceFileActions: attachmentWorkspaceTarget?.actions, - streamingState, - holdQueuedPromptsLocally: isGoalGateBlocked(connection), + streamingState: queuedPromptStreamingState, sessionActions: actions, store, editorRef, @@ -797,16 +802,16 @@ export function ChatPane({ onFirstPromptAdmitted(trimmed); } }; - // Fail CLOSED on a hydrating `goalState`, exactly as the local hold - // above does: the load makes the composer writable before `goal()` - // resolves, and the daemon has no server-side prompt gate for an active - // Goal, so a direct send in that window bypasses the Goal queue. - if ( - streamingStateRef.current === 'idle' && - !isGoalGateBlocked({ + const commandBlockedByGoal = + trimmed.startsWith('/') && + isGoalGateBlocked({ sessionId: connection.sessionId, goalState: connection.goalState, - }) + }); + if (commandBlockedByGoal) return false; + if ( + streamingStateRef.current === 'idle' && + !sessionHasActivePromptRef.current ) { const admissionOwner = admissionOwnerRef.current; let admissionStarted = false; @@ -880,9 +885,6 @@ export function ChatPane({ admissionPayloadLocked, catalogOwnerCwd, clearFollowup, - // The whole snapshot, not just the status: the gate distinguishes an - // absent (hydrating) snapshot from a Goal-less one, and both read as an - // undefined status. connection.goalState, connection.sessionId, connection.status, @@ -1349,7 +1351,7 @@ export function ChatPane({ prompts={queuedPrompts} t={t} canMutateMidTurn={canMutateMidTurn} - canInsertMidTurn={streamingState !== 'idle'} + canInsertMidTurn={queuedPromptStreamingState !== 'idle'} onDelete={removeQueuedPrompt} onInsert={insertQueuedPrompt} onEdit={editQueuedPrompt} diff --git a/packages/web-shell/client/e2e/web-shell.goal.spec.ts b/packages/web-shell/client/e2e/web-shell.goal.spec.ts index 00dfacfb9cc..f93ba382fb9 100644 --- a/packages/web-shell/client/e2e/web-shell.goal.spec.ts +++ b/packages/web-shell/client/e2e/web-shell.goal.spec.ts @@ -45,7 +45,7 @@ test('creates a Goal directly from a new task before any chat', async ({ .toBe(0); }); -test('runs the canonical Goal and explicit queue interaction chain @smoke', async ({ +test('runs the canonical Goal and active-turn queue interaction chain @smoke', async ({ page, }, testInfo) => { const scenario = createWebShellDaemonScenario({ @@ -90,32 +90,27 @@ test('runs the canonical Goal and explicit queue interaction chain @smoke', asyn await strip.getByRole('button', { name: 'Resume goal' }).click(); await expect(strip).toContainText('In progress'); - await submitComposer(page, 'stay queued until I choose'); - const queue = page.locator('[data-web-shell-queued-prompts]'); - await expect(queue).toContainText('stay queued until I choose'); - expect(daemon.promptRequests()).toHaveLength(0); - expect(midTurnRequests(daemon)).toHaveLength(0); - const [queueWidth, goalWidth] = await Promise.all([ - queue.evaluate((element) => element.getBoundingClientRect().width), - strip.evaluate((element) => element.getBoundingClientRect().width), - ]); - expect(Math.abs(queueWidth - goalWidth)).toBeLessThan(1); - await capture(page, testInfo, '02-goal-with-local-queue.png'); - await daemon.sendEvent( assistantTextEvent('Goal turn running', { id: 2, sessionId: scenario.sessionId, }), ); - await queue.getByRole('button', { name: 'Insert' }).click(); + await submitComposer(page, 'stay queued until I choose'); + const queue = page.locator('[data-web-shell-queued-prompts]'); + await expect(queue).toContainText('stay queued until I choose'); + expect(daemon.promptRequests()).toHaveLength(0); await expect.poll(() => midTurnRequests(daemon).length).toBe(1); expect(midTurnRequests(daemon)[0]?.body).toMatchObject({ message: 'stay queued until I choose', }); + const [queueWidth, goalWidth] = await Promise.all([ + queue.evaluate((element) => element.getBoundingClientRect().width), + strip.evaluate((element) => element.getBoundingClientRect().width), + ]); + expect(Math.abs(queueWidth - goalWidth)).toBeLessThan(1); await expect(queue).toContainText('Queued...'); - expect(daemon.promptRequests()).toHaveLength(0); - await capture(page, testInfo, '03-explicitly-inserted.png'); + await capture(page, testInfo, '02-inserted-during-active-turn.png'); await daemon.sendEvent( turnCompleteEvent('goal-turn-1', { id: 3, @@ -123,11 +118,10 @@ test('runs the canonical Goal and explicit queue interaction chain @smoke', asyn }), ); - await submitComposer(page, 'run only after the goal pauses'); - await expect(queue).toContainText('run only after the goal pauses'); - expect(daemon.promptRequests()).toHaveLength(0); - await strip.getByRole('button', { name: 'Pause goal' }).click(); + await submitComposer(page, 'run while the goal stays active'); await expect.poll(() => daemon.promptRequests().length).toBe(1); + expect(midTurnRequests(daemon)).toHaveLength(1); + await expect(queue).not.toContainText('run while the goal stays active'); let confirmationOpened = false; page.on('dialog', async (dialog) => { @@ -140,7 +134,7 @@ test('runs the canonical Goal and explicit queue interaction chain @smoke', asyn expect(goalControlRequests(daemon).at(-1)?.body).toMatchObject({ action: 'clear', }); - await capture(page, testInfo, '04-goal-cleared.png'); + await capture(page, testInfo, '03-goal-cleared.png'); }); async function installScenario( diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 6ab8069fb9b..75ae44aff95 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -1437,6 +1437,8 @@ const EN: Messages = { 'The prompt may already be running. Continue editing only if you accept the risk of sending it twice.', 'queue.commandBlocked': "Slash commands can't be queued while a turn is running.", + 'queue.commandGoalBlocked': + 'Slash commands are unavailable while a Goal owns the session or its state is loading.', 'queue.shellQueued': 'Shell command queued — it will run after the current turn finishes.', 'queue.shellDropped': (v) => { @@ -4437,6 +4439,8 @@ const ZH: Messages = { 'queue.continueEditingConfirm': '这条消息可能已经在执行。只有在接受重复发送风险时才继续编辑。', 'queue.commandBlocked': '当前回合运行时,Slash 命令不能进入排队。', + 'queue.commandGoalBlocked': + 'Goal 正在占用会话或状态仍在加载,暂时无法执行 Slash 命令。', 'queue.shellQueued': 'Shell 命令已排队,将在当前回合结束后执行。', 'queue.shellDropped': (v) => `${v?.count ?? 0} 条排队的 Shell 命令将不会执行。`, diff --git a/packages/web-shell/client/utils/goalGate.ts b/packages/web-shell/client/utils/goalGate.ts index 174a4b35b55..f039c5f9428 100644 --- a/packages/web-shell/client/utils/goalGate.ts +++ b/packages/web-shell/client/utils/goalGate.ts @@ -22,12 +22,11 @@ export interface GoalGateConnection { * Fails CLOSED while `goalState` is still hydrating: the session load clears * `loadingTranscript` (making the composer writable) before its `goal()` fetch * resolves, so an unknown Goal state on a real session has to read as "a Goal - * may be active". The daemon has no server-side prompt gate for an active Goal, - * so a submit inside that window would bypass the Goal queue outright. + * may be active". Commands and automatic runs must not start against that + * unknown ownership state. * - * Every Goal gate in the client goes through here — the composer submit path, - * the local queue hold, and the manual/bound run guards — so none of them can - * drift into failing open on its own. + * Command and automatic/manual run guards go through here. Ordinary chat + * submissions are routed only by the session's streaming state. */ export function isGoalGateBlocked(connection: GoalGateConnection): boolean { return (