diff --git a/apps/desktop/e2e/correction-session-switch.spec.ts b/apps/desktop/e2e/correction-session-switch.spec.ts index dd32caa61e71..dc435b1d5384 100644 --- a/apps/desktop/e2e/correction-session-switch.spec.ts +++ b/apps/desktop/e2e/correction-session-switch.spec.ts @@ -21,8 +21,16 @@ const INFERENCE_SWITCH_TRIGGER = 'E2E_INFERENCE_SWITCH_TRIGGER' const INFERENCE_PROMPT = `${INFERENCE_SWITCH_TRIGGER}: original inference prompt must remain singular.` const INFERENCE_CORRECTION = `${INFERENCE_SWITCH_TRIGGER}: correction sent while inference is live.` +// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the +// renderer's keep-alive visibility policy instead of relying on DOM order. +const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])' + +function activeSurface(page: Page) { + return page.locator(SURFACE).last() +} + async function send(page: Page, text: string): Promise { - const composer = page.locator('[contenteditable="true"]').first() + const composer = activeSurface(page).locator('[contenteditable="true"]').first() await composer.waitFor({ state: 'visible', timeout: 15_000 }) await composer.click() await composer.type(text, { delay: 5 }) @@ -30,8 +38,9 @@ async function send(page: Page, text: string): Promise { } 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"]') + const surface = activeSurface(page) + const composer = surface.locator('[contenteditable="true"]').first() + const primary = surface.locator('[data-slot="composer-root"] button[type="submit"]') await composer.waitFor({ state: 'visible', timeout: 15_000 }) await composer.click() @@ -42,55 +51,80 @@ async function steer(page: Page, text: string): Promise { async function waitForTranscriptText(page: Page, text: string): Promise { await page.waitForFunction( - (expected: string) => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), - text, + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const active = surfaces[surfaces.length - 1] + + return (active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected) + }, + [text, SURFACE] as [string, string], { timeout: 30_000 }, ) } async function textNodeOccurrences(page: Page, text: string): Promise { - return page.evaluate((expected: string) => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - if (!viewport) return 0 + return page.evaluate( + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]') + if (!viewport) return 0 - const walker = document.createTreeWalker(viewport, NodeFilter.SHOW_TEXT) - let count = 0 - while (walker.nextNode()) { - if (walker.currentNode.textContent?.includes(expected)) { - count += 1 + const walker = document.createTreeWalker(viewport, NodeFilter.SHOW_TEXT) + let count = 0 + while (walker.nextNode()) { + if (walker.currentNode.textContent?.includes(expected)) { + count += 1 + } } - } - return count - }, text) + return count + }, + [text, SURFACE] as [string, string], + ) } async function transcriptTextOrder(page: Page): Promise { - return page.evaluate(() => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + return page.evaluate((surfaceSelector: string) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]') if (!viewport) return [] return Array.from(viewport.querySelectorAll('[data-role="message"], [data-message-id]')) .map(message => message.textContent?.trim() ?? '') .filter(Boolean) - }) + }, SURFACE) } async function transcriptMessageOrder(page: Page): Promise { - return page.evaluate(() => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + return page.evaluate((surfaceSelector: string) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]') if (!viewport) return [] - return Array.from(viewport.querySelectorAll('[data-role="user"], [data-role="assistant"]')) + return Array.from( + viewport.querySelectorAll('[data-role="user"], [data-role="assistant"], [data-role="system"]'), + ) .map(message => message.textContent?.trim() ?? '') .filter(Boolean) - }) + }, SURFACE) } +/** + * The sidebar "+" opens a NEW TAB beside the current chat rather than + * replacing it, so the prior session stays mounted in its own surface. Wait + * for the newly-mounted surface to show an empty transcript instead of waiting + * for the old text to disappear from the page (it never will). + */ async function openFreshDraft(page: Page, priorSessionText: string): Promise { await page.locator('[data-slot="sidebar"] button[aria-label="New session"]').first().click() await page.waitForFunction( - (priorText: string) => !(document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(priorText), - priorSessionText, + ([priorText, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const active = surfaces[surfaces.length - 1] + const transcript = active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' + + return surfaces.length > 0 && !transcript.includes(priorText) + }, + [priorSessionText, SURFACE] as [string, string], { timeout: 15_000 }, ) } @@ -116,7 +150,12 @@ async function reopenInferenceSession(page: Page): Promise { } function relevantOrder(messages: string[]): string[] { - return messages.filter(message => message.includes(ORIGINAL_PROMPT) || message.includes(CORRECTION)) + return messages.flatMap(message => { + if (message.includes(ORIGINAL_PROMPT)) return [ORIGINAL_PROMPT] + if (message.includes(CORRECTION)) return [CORRECTION] + + return [] + }) } function steerTurnOrder(messages: string[]): string[] { diff --git a/apps/desktop/e2e/image-attachment-resume.spec.ts b/apps/desktop/e2e/image-attachment-resume.spec.ts index cfa9829a4e2e..a4f8da68e39d 100644 --- a/apps/desktop/e2e/image-attachment-resume.spec.ts +++ b/apps/desktop/e2e/image-attachment-resume.spec.ts @@ -93,28 +93,53 @@ function sessionRow(page: Page) { return page.locator('[data-slot="sidebar"] button').filter({ hasText: CAPTION }).first() } +// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the +// renderer's keep-alive visibility policy instead of relying on DOM order. +const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])' + +function activeViewportText(surfaceSelector: string): string { + const surfaces = document.querySelectorAll(surfaceSelector) + + return surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' +} + async function openSeededSession(page: Page): Promise { const row = sessionRow(page) await row.waitFor({ state: 'visible', timeout: 60_000 }) await row.click() await page.waitForFunction( - expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), - CAPTION, + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const text = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' + + return text.includes(expected) + }, + [CAPTION, SURFACE] as [string, string], { timeout: 30_000 }, ) } +/** + * The sidebar "+" opens a NEW TAB beside the current chat instead of replacing + * it, so the seeded session stays mounted in its own surface. Assert the new + * surface is empty rather than waiting for the old caption to leave the page. + */ async function openNewSession(page: Page): Promise { await page.locator('[data-slot="sidebar"] button[aria-label="New session"]').first().click() await page.waitForFunction( - expected => !(document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), - CAPTION, + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const text = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' + + return surfaces.length > 0 && !text.includes(expected) + }, + [CAPTION, SURFACE] as [string, string], { timeout: 15_000 }, ) } async function transcriptText(page: Page): Promise { - return page.evaluate(() => document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '') + return page.evaluate(activeViewportText, SURFACE) } async function assertRendersThumbnail(page: Page, label: string): Promise { diff --git a/apps/desktop/e2e/warm-resume-jitter.spec.ts b/apps/desktop/e2e/warm-resume-jitter.spec.ts index 72e71087d690..dbc0fe5dd113 100644 --- a/apps/desktop/e2e/warm-resume-jitter.spec.ts +++ b/apps/desktop/e2e/warm-resume-jitter.spec.ts @@ -24,6 +24,8 @@ * MutationObserver burst), but `$messages` was still set twice. * * The test passes when bursts === 1 AND reconciles === 0. + * The sidebar "+" keeps the session warm in another tab. Its reactivation + * follows the same contract: one additive paint and zero reconciles. * * Prerequisite: `npm run build` must have been run so dist/ exists. */ @@ -43,6 +45,11 @@ import { startMockServer } from './mock-server' import { RealSessionBuilder } from './real-session-builder' const SESSION_TITLE = 'E2E Warm Resume Jitter Test' + +// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the +// renderer's keep-alive visibility policy instead of relying on DOM order. +const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])' +const ALL_SURFACES = '[data-composer-target]' /** 32 messages (16 user/assistant pairs) — enough DOM churn for detection. */ const MESSAGE_COUNT = 32 /** Seeded PRNG so the generated content is deterministic across runs. */ @@ -154,15 +161,29 @@ test.afterAll(async () => { * after the initial paint, catching key-based reconciles that don't * add/remove nodes. */ -async function installRenderCounter(page: import('@playwright/test').Page): Promise { - await page.evaluate(() => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') +async function installRenderCounter( + page: import('@playwright/test').Page, + transcriptText?: string, +): Promise { + await page.evaluate(([visibleSelector, allSelector, expected]: [string, string, string | undefined]) => { + const surfaces = [...document.querySelectorAll(expected ? allSelector : visibleSelector)] + const surface = expected + ? surfaces.find(candidate => + (candidate.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + ) + : surfaces.at(-1) + const viewport = surface?.querySelector('[data-slot="aui_thread-viewport"]') if (!viewport) { throw new Error('Thread viewport not found before warm resume') } const state = { bursts: 0, mutations: 0, timeline: [] as number[], stopped: false, reconciles: 0 } - ;(window as unknown as { __RENDER_COUNT__: typeof state }).__RENDER_COUNT__ = state + const debugWindow = window as unknown as { + __RENDER_COUNT__: typeof state + __RENDER_VIEWPORT__: Element + } + debugWindow.__RENDER_COUNT__ = state + debugWindow.__RENDER_VIEWPORT__ = viewport let currentBatch = 0 let flushTimer: ReturnType | null = null @@ -224,7 +245,53 @@ async function installRenderCounter(page: import('@playwright/test').Page): Prom hasMessages = true } }, 2) - }) + }, [SURFACE, ALL_SURFACES, transcriptText] as [string, string, string | undefined]) +} + +/** Wait until the ACTIVE chat surface's transcript contains `text`. */ +async function waitForActiveTranscriptText( + page: import('@playwright/test').Page, + text: string, + timeout = 30_000, +): Promise { + await page.waitForFunction( + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const active = surfaces[surfaces.length - 1] + + return (active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected) + }, + [text, SURFACE] as [string, string], + { timeout }, + ) +} + +async function waitForActiveTranscriptWithoutText( + page: import('@playwright/test').Page, + text: string, +): Promise { + await page.waitForFunction( + ([expected, surfaceSelector]: [string, string]) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const active = surfaces[surfaces.length - 1] + + return !(active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected) + }, + [text, SURFACE] as [string, string], + { timeout: 15_000 }, + ) +} + +/** Replace the primary surface with a draft while retaining its warm cache. */ +async function openFreshDraft(page: import('@playwright/test').Page, priorText: string): Promise { + await page.keyboard.press(process.platform === 'darwin' ? 'Meta+N' : 'Control+N') + await waitForActiveTranscriptWithoutText(page, priorText) +} + +/** Stack an empty tab while leaving the current transcript mounted and warm. */ +async function openNewSessionTab(page: import('@playwright/test').Page, priorText: string): Promise { + await page.locator('[data-slot="sidebar"] button[aria-label="New session"]').first().click() + await waitForActiveTranscriptWithoutText(page, priorText) } /** Stop the render counter and return the recorded burst/reconcile counts. */ @@ -245,6 +312,30 @@ async function readRenderCount(page: import('@playwright/test').Page): Promise<{ }) } +async function observedViewportIsActive(page: import('@playwright/test').Page): Promise { + return page.evaluate((surfaceSelector: string) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const activeViewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]') + const observedViewport = (window as unknown as { __RENDER_VIEWPORT__?: Element }).__RENDER_VIEWPORT__ + + return activeViewport === observedViewport + }, SURFACE) +} + +/** A kept-alive tab must become visible without rebuilding its transcript. */ +function assertNoRepaint(result: { bursts: number; mutations: number; timeline: number[]; reconciles: number } | null): void { + expect(result, 'MutationObserver should have recorded render data').toBeTruthy() + expect( + result!.bursts, + `Expected no additive render bursts for a kept-alive tab, but got ${result!.bursts}. ` + + `Mutation timeline: ${JSON.stringify(result!.timeline)}.`, + ).toBe(0) + expect( + result!.reconciles, + `Expected no transcript reconciles for a kept-alive tab, but got ${result!.reconciles}.`, + ).toBe(0) +} + /** Assert the render counter shows exactly one paint with no re-renders. */ function assertNoJitter(result: { bursts: number; mutations: number; timeline: number[]; reconciles: number } | null): void { expect(result, 'MutationObserver should have recorded render data').toBeTruthy() @@ -261,7 +352,7 @@ function assertNoJitter(result: { bursts: number; mutations: number; timeline: n ).toBe(0) } -test('warm-route resume paints transcript exactly once (no jitter)', async ({}, testInfo) => { +test('tab reactivation preserves the mounted transcript without repainting', async ({}, testInfo) => { const page = fixture!.page // Wait for the sidebar to populate with our seeded session. @@ -277,63 +368,29 @@ test('warm-route resume paints transcript exactly once (no jitter)', async ({}, // Wait for the transcript to appear — the first user message text confirms // the cold-path prefetch painted. - await page.waitForFunction( - (text: string) => - document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ?? - false, - FIRST_USER_MSG, - { timeout: 30_000 }, - ) + await waitForActiveTranscriptText(page, FIRST_USER_MSG) // Wait for the session to fully settle (cold-path RPC + reconciliation). await page.waitForTimeout(2_000) - // Step 2: Navigate away to a new chat — this does NOT evict the warm cache. - const newSessionButton = page - .locator('[data-slot="sidebar"] button[aria-label="New session"]') - .first() - await newSessionButton.click() - - // Wait for the new-chat empty state. - await page.waitForFunction( - (firstMsg: string) => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - if (!viewport) return false - const text = viewport.textContent ?? '' - return !text.includes(firstMsg) - }, - FIRST_USER_MSG, - { timeout: 15_000 }, - ) - + // Stack a new tab, then observe the seeded transcript while it is hidden. + // Installing after the switch isolates reactivation from mutations caused + // while the new tab was being created. + await openNewSessionTab(page, FIRST_USER_MSG) await page.waitForTimeout(500) + await installRenderCounter(page, FIRST_USER_MSG) - // Step 3: Install render counter, click back (warm resume), wait, assert. - await installRenderCounter(page) + // Step 3: Click back and verify the same kept-alive viewport becomes active + // without rebuilding or reconciling its transcript. await sessionRow.click() - await page.waitForFunction( - (text: string) => - document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ?? - false, - FIRST_USER_MSG, - { timeout: 30_000 }, - ) - - // Wait for at least 1 burst, then settle. - await page.waitForFunction( - () => { - const w = window as unknown as { __RENDER_COUNT__?: { bursts: number } } - return Boolean(w.__RENDER_COUNT__ && w.__RENDER_COUNT__.bursts > 0) - }, - undefined, - { timeout: 10_000 }, - ) + await waitForActiveTranscriptText(page, FIRST_USER_MSG) await page.waitForTimeout(2_000) + expect(await observedViewportIsActive(page), 'Reactivation should reveal the observed kept-alive viewport').toBe(true) const result = await readRenderCount(page) await page.screenshot({ path: testInfo.outputPath('warm-resume-idle.png') }) - assertNoJitter(result) + assertNoRepaint(result) }) test('warm-route resume after background inference completes (no jitter)', async ({}, testInfo) => { @@ -354,13 +411,7 @@ test('warm-route resume after background inference completes (no jitter)', async // Step 1: Cold resume — populate the warm cache. await sessionRow.click() - await page.waitForFunction( - (text: string) => - document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ?? - false, - FIRST_USER_MSG, - { timeout: 30_000 }, - ) + await waitForActiveTranscriptText(page, FIRST_USER_MSG) await page.waitForTimeout(2_000) // Step 2: Send a message — triggers inference via the mock server. @@ -373,34 +424,15 @@ test('warm-route resume after background inference completes (no jitter)', async // Wait for the mock response to appear in the transcript, confirming // the turn completed and message.complete fired (which updates the warm // cache via updateSessionState). - await page.waitForFunction( - () => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - return viewport?.textContent?.includes('mock inference server') ?? false - }, - undefined, - { timeout: 60_000 }, - ) + await waitForActiveTranscriptText(page, 'mock inference server', 60_000) // Extra settle for message.complete → updateSessionState → cache write. await page.waitForTimeout(2_000) // Verify the prompt was received by the mock server. expect(mock.receivedPrompts).toContain(PROMPT) - // Step 3: Navigate away — the warm cache retains the updated messages. - const newSessionButton = page - .locator('[data-slot="sidebar"] button[aria-label="New session"]') - .first() - await newSessionButton.click() - await page.waitForFunction( - (prompt: string) => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - if (!viewport) return false - return !(viewport.textContent ?? '').includes(prompt) - }, - PROMPT, - { timeout: 15_000 }, - ) + // Step 3: Replace the primary chat; the warm cache retains the updated messages. + await openFreshDraft(page, PROMPT) await page.waitForTimeout(500) // Step 4: Install render counter, click back (warm resume), wait, assert. @@ -409,13 +441,7 @@ test('warm-route resume after background inference completes (no jitter)', async // Wait for the transcript to reappear — the warm cache should already // have the completed turn (updated by message.complete events). - await page.waitForFunction( - (text: string) => - document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ?? - false, - FIRST_USER_MSG, - { timeout: 30_000 }, - ) + await waitForActiveTranscriptText(page, FIRST_USER_MSG) // Wait for at least 1 burst, then settle. await page.waitForFunction( diff --git a/apps/desktop/src/app/chat/session-tile-actions.ts b/apps/desktop/src/app/chat/session-tile-actions.ts index 0a6db9733508..a189af6f6479 100644 --- a/apps/desktop/src/app/chat/session-tile-actions.ts +++ b/apps/desktop/src/app/chat/session-tile-actions.ts @@ -236,23 +236,6 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses [listTileSession, scope.attachments.$attachments, submitPromptText] ) - const appendSystemNote = useCallback( - (text: string) => { - update(state => ({ - ...state, - messages: [ - ...state.messages, - { - id: `system-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, - role: 'system', - parts: [textPart(text)] - } - ] - })) - }, - [update] - ) - const cancelRun = useCallback(async () => { const sessionId = runtimeIdRef.current @@ -283,30 +266,82 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses const steerPrompt = useCallback( async (rawText: string): Promise => { const text = rawText.trim() + const sessionId = runtimeIdRef.current - if (!text) { + if (!text || !sessionId) { return false } + const messageId = `user-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` + const mutate = (updater: (state: ClientSessionState) => ClientSessionState) => + sessionTileDelegate()?.updateSession(sessionId, updater) + + // Match the primary composer: insert the correction before the active + // reply before awaiting the redirect RPC, whose completion can race us. + mutate(state => { + const message = { + id: messageId, + role: 'user' as const, + parts: [textPart(text)] + } + const streamIndex = state.streamId + ? state.messages.findIndex(candidate => candidate.id === state.streamId) + : -1 + const lastAssistantIndex = state.messages.map(candidate => candidate.role).lastIndexOf('assistant') + 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 } + }) + + const discardOptimisticMessage = () => + mutate(state => ({ + ...state, + messages: state.messages.filter(message => message.id !== messageId) + })) + + const moveOptimisticMessageToEnd = () => + mutate(state => { + const message = state.messages.find(candidate => candidate.id === messageId) + + return message + ? { ...state, messages: [...state.messages.filter(candidate => candidate.id !== messageId), message] } + : state + }) + try { - const result = await requestGateway<{ status?: string }>('session.steer', { - session_id: runtimeIdRef.current, + const result = await requestGateway<{ status?: string }>('session.redirect', { + session_id: sessionId, text }) - if (result?.status === 'queued') { + if (result?.status === 'redirected') { + triggerHaptic('submit') + + return true + } + + if (result?.status === 'queued') { + moveOptimisticMessageToEnd() triggerHaptic('submit') - appendSystemNote(`steer:${text}`) return true } } catch { + discardOptimisticMessage() // Swallow — the caller queues the text so nothing is lost. + + return false } + discardOptimisticMessage() + return false }, - [appendSystemNote, requestGateway] + [requestGateway] ) // Rewind primitive (interrupt-first for live turns, busy-retry) — shared with diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index d00bd646c1cf..a182ae68c3da 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -89,7 +89,7 @@ import { useRouteResume } from '../session/hooks/use-route-resume' import { useSessionActions } from '../session/hooks/use-session-actions' import { useSessionListActions } from '../session/hooks/use-session-list-actions' import { useSessionStateCache } from '../session/hooks/use-session-state-cache' -import { startWorkspaceSession } from '../session/workspace-session-target' +import { newSessionOpensTab, startWorkspaceSession } from '../session/workspace-session-target' import { useOverlayRouting } from '../shell/hooks/use-overlay-routing' import { useWindowControlsOverlayWidth } from '../shell/hooks/use-window-controls-overlay-width' import { titlebarControlsPosition } from '../shell/titlebar' @@ -481,11 +481,22 @@ export function ContribWiring({ children }: { children: ReactNode }) { void refreshActiveProfile() }, [activeGatewayProfile, refreshCurrentModel, refreshHermesConfig]) - // New session anchored to a workspace (sidebar "+" on a project/worktree). - // Seeds cwd + branch from the clicked workspace; an explicit worktree path - // also drills the sidebar into that project so the new lane is visible. + // New session anchored to a workspace. Seeds cwd + branch from the clicked + // workspace; an explicit worktree path also drills the sidebar into that + // project so the new lane is visible. + // + // `openTab` is the sidebar "+" behavior: once a chat is loaded, stack a new + // tab instead of replacing it (see newSessionOpensTab). The composer's + // "branch off into a new worktree" flow keeps the fresh-draft path — it + // prefills the MAIN composer right after, so it has to own that surface. const startSessionInWorkspace = useCallback( - (path: null | string) => { + (path: null | string, options?: { openTab?: boolean }) => { + if (options?.openTab && newSessionOpensTab(activeSessionIdRef.current, $selectedStoredSessionId.get())) { + void openNewSessionTile('center', { cwd: path, listed: false }) + + return + } + startWorkspaceSession({ activeSessionIdRef, followActiveSessionCwd, @@ -495,7 +506,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { startFreshSessionDraft }) }, - [activeSessionIdRef, requestGateway, startFreshSessionDraft] + [activeSessionIdRef, openNewSessionTile, requestGateway, startFreshSessionDraft] ) // Composer "branch off into a new worktree": open a fresh session anchored @@ -791,7 +802,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { navigate(CRON_ROUTE) }, onNavigate: selectSidebarItem, - onNewSessionInWorkspace: startSessionInWorkspace, + onNewSessionInWorkspace: path => startSessionInWorkspace(path, { openTab: true }), onNewSessionSplit: dir => void openNewSessionTile(dir), onPasteClipboardImage: opts => composer.pasteClipboardImage(opts), onPickFiles: () => void composer.pickContextPaths('file'), diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 2ef5dfe9aadf..b0202e722a4d 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -476,13 +476,14 @@ export function useSessionActions({ * list (Cursor-style draft tab); it surfaces on the next refresh once the * first message persists a turn. "Open in split" keeps the listed behavior. */ const openNewSessionTile = useCallback( - async (dir: TileDock = 'right', options?: { listed?: boolean }) => { + async (dir: TileDock = 'right', options?: { cwd?: null | string; listed?: boolean }) => { const listed = options?.listed ?? true try { - // Fresh tile → the resolved new-session cwd (project/default), not the - // primary composer's live cwd. - const params = await desktopSessionCreateParams(resolveNewSessionCwd().trim()) + // Fresh tile → the caller's workspace when one was named (the sidebar + // "+" on a project/worktree lane), else the resolved new-session cwd + // (project/default) — never the primary composer's live cwd. + const params = await desktopSessionCreateParams((options?.cwd || resolveNewSessionCwd()).trim()) const created = await requestGateway('session.create', params) const stored = created.stored_session_id diff --git a/apps/desktop/src/app/session/workspace-session-target.test.ts b/apps/desktop/src/app/session/workspace-session-target.test.ts index 3a83f605a321..614227585c05 100644 --- a/apps/desktop/src/app/session/workspace-session-target.test.ts +++ b/apps/desktop/src/app/session/workspace-session-target.test.ts @@ -9,7 +9,7 @@ import { setNewChatWorkspaceTarget } from '@/store/session' -import { startWorkspaceSession } from './workspace-session-target' +import { newSessionOpensTab, startWorkspaceSession } from './workspace-session-target' function deferred() { let resolve!: (value: T) => void @@ -21,6 +21,29 @@ function deferred() { return { promise, resolve } } +/** + * The sidebar "+" is a create affordance, not a discard one: with a chat + * already loaded it must stack a tab (⌘T) rather than replace the surface + * (⌘N), which would throw away a conversation that may still be mid-turn. + */ +describe('newSessionOpensTab', () => { + it('opens a tab once a conversation is on screen', () => { + expect(newSessionOpensTab('runtime-a', 'stored-a')).toBe(true) + }) + + it('opens a tab for a live runtime whose stored id has not landed yet', () => { + expect(newSessionOpensTab('runtime-a', null)).toBe(true) + }) + + it('opens a tab for a selected session still resuming into a runtime', () => { + expect(newSessionOpensTab(null, 'stored-a')).toBe(true) + }) + + it('replaces the empty surface when nothing is open', () => { + expect(newSessionOpensTab(null, null)).toBe(false) + }) +}) + describe('startWorkspaceSession', () => { afterEach(() => { setCurrentBranch('') diff --git a/apps/desktop/src/app/session/workspace-session-target.ts b/apps/desktop/src/app/session/workspace-session-target.ts index da5028502a11..9a87d95c0095 100644 --- a/apps/desktop/src/app/session/workspace-session-target.ts +++ b/apps/desktop/src/app/session/workspace-session-target.ts @@ -17,6 +17,19 @@ interface WorkspaceSessionOptions { startFreshSessionDraft: (options?: { workspaceTarget: string }) => void } +/** + * Should the sidebar "+" open a NEW TAB rather than replace what's on screen? + * + * "+" is a create affordance, never a discard one. Once a conversation is + * loaded, replacing it with a blank draft would throw away a chat that may + * still be mid-turn, so the button stacks a tab beside it (⌘T) instead of + * taking over the surface (⌘N). With nothing open there is no tab worth + * preserving and the cheaper fresh-draft path applies. + */ +export function newSessionOpensTab(activeSessionId: null | string, selectedStoredSessionId: null | string): boolean { + return Boolean(activeSessionId || selectedStoredSessionId) +} + export function startWorkspaceSession({ activeSessionIdRef, followActiveSessionCwd: followCwd = followActiveSessionCwd,