From 8a21df18acbe73c63d06747d0ab359288bf84276 Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 22 Jul 2026 22:04:57 -0400 Subject: [PATCH] fix(desktop): place steer messages before redirected replies (#69739) * test(desktop): reproduce steer transcript placement * fix(desktop): place steer messages before redirected replies --- .../e2e/correction-session-switch.spec.ts | 39 +++++++- apps/desktop/e2e/queue-turn-boundary.spec.ts | 49 +++++++++++ .../session/hooks/use-prompt-actions/index.ts | 88 +++++++++++++++---- 3 files changed, 155 insertions(+), 21 deletions(-) diff --git a/apps/desktop/e2e/correction-session-switch.spec.ts b/apps/desktop/e2e/correction-session-switch.spec.ts index 9dc71dfa7912..dd32caa61e71 100644 --- a/apps/desktop/e2e/correction-session-switch.spec.ts +++ b/apps/desktop/e2e/correction-session-switch.spec.ts @@ -29,6 +29,17 @@ async function send(page: Page, text: string): Promise { await page.keyboard.press('Enter') } +async function steer(page: Page, text: string): Promise { + const composer = page.locator('[contenteditable="true"]').first() + const primary = page.locator('[data-slot="composer-root"] button[type="submit"]') + + await composer.waitFor({ state: 'visible', timeout: 15_000 }) + await composer.click() + await composer.type(text, { delay: 5 }) + await expect(primary).toHaveAttribute('aria-label', /Steer/) + await primary.click() +} + async function waitForTranscriptText(page: Page, text: string): Promise { await page.waitForFunction( (expected: string) => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), @@ -64,6 +75,17 @@ async function transcriptTextOrder(page: Page): Promise { }) } +async function transcriptMessageOrder(page: Page): Promise { + return page.evaluate(() => { + const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + if (!viewport) return [] + + return Array.from(viewport.querySelectorAll('[data-role="user"], [data-role="assistant"]')) + .map(message => message.textContent?.trim() ?? '') + .filter(Boolean) + }) +} + async function openFreshDraft(page: Page, priorSessionText: string): Promise { await page.locator('[data-slot="sidebar"] button[aria-label="New session"]').first().click() await page.waitForFunction( @@ -97,6 +119,16 @@ function relevantOrder(messages: string[]): string[] { return messages.filter(message => message.includes(ORIGINAL_PROMPT) || message.includes(CORRECTION)) } +function steerTurnOrder(messages: string[]): string[] { + return messages.flatMap(message => { + if (message.includes(ORIGINAL_PROMPT)) return [ORIGINAL_PROMPT] + if (message.includes(CORRECTION)) return [CORRECTION] + if (message.includes(CORRECTED_REPLY)) return [CORRECTED_REPLY] + + return [] + }) +} + test.describe('correction session switch', () => { let fixture: MockBackendFixture | null = null @@ -125,9 +157,9 @@ test.describe('correction session switch', () => { await waitForTranscriptText(page, TOOL_STARTED) await waitForTranscriptText(page, ORIGINAL_PROMPT) - // The historical session redirected while a foreground terminal task was - // running. Enter records the accepted correction at the next tool boundary. - await send(page, CORRECTION) + // The historical session redirects while a foreground terminal task is + // running. Use the visible Steer action to cover the real composer path. + await steer(page, CORRECTION) await waitForTranscriptText(page, CORRECTION) const orderBeforeSwitch = relevantOrder(await transcriptTextOrder(page)) @@ -148,6 +180,7 @@ test.describe('correction session switch', () => { expect(await textNodeOccurrences(page, CORRECTION)).toBe(1) await waitForTranscriptText(page, CORRECTED_REPLY) + expect(steerTurnOrder(await transcriptMessageOrder(page))).toEqual([ORIGINAL_PROMPT, CORRECTION, CORRECTED_REPLY]) }) test('keeps an inference-time correction visible through a warm session switch', async ({}, testInfo: TestInfo) => { diff --git a/apps/desktop/e2e/queue-turn-boundary.spec.ts b/apps/desktop/e2e/queue-turn-boundary.spec.ts index 834a5fe949e4..27cffd7a625a 100644 --- a/apps/desktop/e2e/queue-turn-boundary.spec.ts +++ b/apps/desktop/e2e/queue-turn-boundary.spec.ts @@ -14,6 +14,7 @@ import { MOCK_REPLY } from './mock-server' const ACTIVE_PROMPT = 'E2E_QUEUE_TURN_BOUNDARY_ACTIVE' const QUEUED_PROMPT = 'E2E_QUEUE_TURN_BOUNDARY_QUEUED' +const STEER_PROMPT = 'E2E_STEER_TURN_BOUNDARY_CORRECTION' async function send(page: Page, text: string): Promise { const composer = page.locator('[contenteditable="true"]').first() @@ -23,6 +24,37 @@ async function send(page: Page, text: string): Promise { await page.keyboard.press('Enter') } +async function steer(page: Page, text: string): Promise { + const composer = page.locator('[contenteditable="true"]').first() + const primary = page.locator('[data-slot="composer-root"] button[type="submit"]') + + await composer.click() + await composer.type(text, { delay: 5 }) + await expect(primary).toHaveAttribute('aria-label', /Steer/) + await primary.click() +} + +async function transcriptMessageOrder(page: Page): Promise { + return page.evaluate(() => { + const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + if (!viewport) return [] + + return Array.from(viewport.querySelectorAll('[data-role="user"], [data-role="assistant"]')) + .map(message => message.textContent?.trim() ?? '') + .filter(Boolean) + }) +} + +function steerTurnOrder(messages: string[]): string[] { + return messages.flatMap(message => { + if (message.includes(ACTIVE_PROMPT)) return [ACTIVE_PROMPT] + if (message.includes(STEER_PROMPT)) return [STEER_PROMPT] + if (message.includes(MOCK_REPLY)) return [MOCK_REPLY] + + return [] + }) +} + test.describe('queued prompt turn boundary', () => { let fixture: MockBackendFixture | null = null @@ -66,4 +98,21 @@ test.describe('queued prompt turn boundary', () => { ) await expect.poll(() => mock.receivedPrompts.filter(prompt => prompt === QUEUED_PROMPT)).toHaveLength(1) }) + + test('places a steer prompt before the reply it redirects', async () => { + const { mock, page } = fixture! + + await send(page, ACTIVE_PROMPT) + await mock.waitForHeldStream() + await steer(page, STEER_PROMPT) + mock.releaseHeldStream() + + await page.waitForFunction( + expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + MOCK_REPLY, + { timeout: 60_000 } + ) + + expect(steerTurnOrder(await transcriptMessageOrder(page))).toEqual([ACTIVE_PROMPT, STEER_PROMPT, MOCK_REPLY]) + }) }) \ No newline at end of file diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 788305889763..a3327594568f 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -220,7 +220,13 @@ export function usePromptActions({ const copy = t.desktop const appendSessionTextMessage = useCallback( - (sessionId: string, role: ChatMessage['role'], text: string, storedSessionId?: string | null) => { + ( + sessionId: string, + role: ChatMessage['role'], + text: string, + storedSessionId?: string | null, + options: { insertBeforeActiveReply?: boolean } = {} + ) => { // Strip ANSI: slash-command output from the backend worker carries SGR // color codes (e.g. "Unknown command" in red). The ESC byte is invisible // in the chat panel, so without this the `[1;31m…[0m` payload leaks as @@ -231,21 +237,33 @@ export function usePromptActions({ return } + const messageId = `${role}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` + updateSessionState( sessionId, - state => ({ - ...state, - messages: [ - ...state.messages, - { - id: `${role}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, - role, - parts: [textPart(body)] - } - ] - }), + state => { + const message: ChatMessage = { + id: messageId, + role, + parts: [textPart(body)] + } + const streamIndex = options.insertBeforeActiveReply && state.streamId + ? state.messages.findIndex(candidate => candidate.id === state.streamId) + : -1 + const lastAssistantIndex = options.insertBeforeActiveReply + ? state.messages.map(candidate => candidate.role).lastIndexOf('assistant') + : -1 + const insertionIndex = streamIndex >= 0 ? streamIndex : lastAssistantIndex + const messages = insertionIndex >= 0 + ? [...state.messages.slice(0, insertionIndex), message, ...state.messages.slice(insertionIndex)] + : [...state.messages, message] + + return { ...state, messages } + }, storedSessionId ?? selectedStoredSessionIdRef.current ) + + return messageId }, [selectedStoredSessionIdRef, updateSessionState] ) @@ -620,15 +638,49 @@ export function usePromptActions({ // message after the interrupted checkpoint, matching the durable core // transcript rather than a system note that changes role after reload. const send = async (id: string): Promise => { - const result = await requestGateway('session.redirect', { session_id: id, text }) + // Redirect aborts the model request, so the completion event can race + // its RPC response. Insert before the live reply *before* awaiting the + // gateway; appending after the response leaves the correction below a + // reply that the redirect has already replaced. + const messageId = appendSessionTextMessage(id, 'user', text, undefined, { insertBeforeActiveReply: true }) + const discardOptimisticMessage = () => + updateSessionState(id, state => ({ + ...state, + messages: state.messages.filter(message => message.id !== messageId) + })) + const moveOptimisticMessageToEnd = () => + updateSessionState(id, state => { + const message = state.messages.find(candidate => candidate.id === messageId) - if (result?.status === 'redirected' || result?.status === 'queued') { - triggerHaptic('submit') - appendSessionTextMessage(id, 'user', text) + return message + ? { ...state, messages: [...state.messages.filter(candidate => candidate.id !== messageId), message] } + : state + }) - return true + try { + const result = await requestGateway('session.redirect', { session_id: id, text }) + + if (result?.status === 'redirected') { + triggerHaptic('submit') + + return true + } + + if (result?.status === 'queued') { + // Build-window redirects become the next turn, not part of the + // active reply, so retain the optimistic row at the tail. + moveOptimisticMessageToEnd() + triggerHaptic('submit') + + return true + } + } catch (err) { + discardOptimisticMessage() + throw err } + discardOptimisticMessage() + return false } @@ -661,7 +713,7 @@ export function usePromptActions({ return false }, - [activeSessionId, activeSessionIdRef, appendSessionTextMessage, requestGateway, selectedStoredSessionIdRef] + [activeSessionId, activeSessionIdRef, appendSessionTextMessage, requestGateway, selectedStoredSessionIdRef, updateSessionState] ) const reloadFromMessage = useCallback(