From bd86ce9938f7b69326a7875547a91e2e8f7dbb7b Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 03:22:09 -0500 Subject: [PATCH 1/4] feat(desktop): open a tab from the sidebar "+" when a chat is loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar "+" ran startFreshSessionDraft, so it replaced whatever was on screen — ⌘N behavior. With a conversation already open (possibly mid-turn) that discards it to make room for a blank draft, which is not what a create affordance should do. The tab-strip "+" and ⌘T already stack a new tab. Route the sidebar button through the same path once a session is loaded, falling back to the fresh-draft path when the surface is empty and there is no tab worth preserving. openNewSessionTile now takes an optional cwd so the new tab stays anchored to the clicked project/worktree lane. --- apps/desktop/src/app/contrib/wiring.tsx | 12 +++++++-- .../hooks/use-session-actions/index.ts | 9 ++++--- .../session/workspace-session-target.test.ts | 25 ++++++++++++++++++- .../app/session/workspace-session-target.ts | 13 ++++++++++ 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index d00bd646c1cf..c3d975b63e81 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' @@ -484,8 +484,16 @@ export function ContribWiring({ children }: { children: ReactNode }) { // 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. + // Once a chat is loaded the button opens a TAB instead of replacing it — + // see newSessionOpensTab. const startSessionInWorkspace = useCallback( (path: null | string) => { + if (newSessionOpensTab(activeSessionIdRef.current, $selectedStoredSessionId.get())) { + void openNewSessionTile('center', { cwd: path, listed: false }) + + return + } + startWorkspaceSession({ activeSessionIdRef, followActiveSessionCwd, @@ -495,7 +503,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 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, From fc39c7ac31f3c2f835af450dc2d50ce42c171870 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 03:57:49 -0500 Subject: [PATCH 2/4] test(desktop): scope e2e transcript helpers to the active chat surface The sidebar "+" now stacks a tab instead of replacing the surface, so the prior session stays mounted and several chat surfaces can be on the page at once. Helpers that waited for the old transcript to disappear from the page timed out, and `.first()` locators / bare `document.querySelector` calls started resolving against the wrong session (CI's "resolved to 2 elements" strict-mode violation). Target the most recently mounted `[data-composer-target]` surface instead, and assert the NEW surface is empty rather than waiting for the old text to vanish. --- .../e2e/correction-session-switch.spec.ts | 82 +++++++++++++------ .../e2e/image-attachment-resume.spec.ts | 36 ++++++-- apps/desktop/e2e/warm-resume-jitter.spec.ts | 20 +++-- apps/desktop/src/app/contrib/wiring.tsx | 19 +++-- 4 files changed, 113 insertions(+), 44 deletions(-) diff --git a/apps/desktop/e2e/correction-session-switch.spec.ts b/apps/desktop/e2e/correction-session-switch.spec.ts index dd32caa61e71..0b9083807606 100644 --- a/apps/desktop/e2e/correction-session-switch.spec.ts +++ b/apps/desktop/e2e/correction-session-switch.spec.ts @@ -21,8 +21,18 @@ 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.` +// A ⌘T-style tab can put several chat surfaces on the page at once, so every +// locator here is scoped to the ACTIVE one (the most recently mounted surface) +// rather than `.first()` / a bare document query, which would silently target +// the wrong session's composer or transcript. +const SURFACE = '[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 +40,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 +53,78 @@ 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"]')) .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 }, ) } diff --git a/apps/desktop/e2e/image-attachment-resume.spec.ts b/apps/desktop/e2e/image-attachment-resume.spec.ts index cfa9829a4e2e..3c653f2e2c4f 100644 --- a/apps/desktop/e2e/image-attachment-resume.spec.ts +++ b/apps/desktop/e2e/image-attachment-resume.spec.ts @@ -93,28 +93,54 @@ function sessionRow(page: Page) { return page.locator('[data-slot="sidebar"] button').filter({ hasText: CAPTION }).first() } +// A ⌘T-style tab can put several chat surfaces on the page at once, so +// transcript queries target the ACTIVE (most recently mounted) surface rather +// than the first match, which would read the wrong session. +const SURFACE = '[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..657c503bd9ca 100644 --- a/apps/desktop/e2e/warm-resume-jitter.spec.ts +++ b/apps/desktop/e2e/warm-resume-jitter.spec.ts @@ -294,12 +294,15 @@ test('warm-route resume paints transcript exactly once (no jitter)', async ({}, .first() await newSessionButton.click() - // Wait for the new-chat empty state. + // Wait for the new-chat empty state. The "+" opens a NEW TAB beside the + // resumed session rather than replacing it, so the old transcript stays + // mounted — assert the newly-added surface is the empty one. await page.waitForFunction( (firstMsg: string) => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - if (!viewport) return false - const text = viewport.textContent ?? '' + const surfaces = document.querySelectorAll('[data-composer-target]') + const active = surfaces[surfaces.length - 1] + if (!active) return false + const text = active.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' return !text.includes(firstMsg) }, FIRST_USER_MSG, @@ -392,11 +395,14 @@ test('warm-route resume after background inference completes (no jitter)', async .locator('[data-slot="sidebar"] button[aria-label="New session"]') .first() await newSessionButton.click() + // "+" stacks a new tab, so the prior transcript stays mounted in its own + // surface — check the newly-added surface rather than the whole page. await page.waitForFunction( (prompt: string) => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') - if (!viewport) return false - return !(viewport.textContent ?? '').includes(prompt) + const surfaces = document.querySelectorAll('[data-composer-target]') + const active = surfaces[surfaces.length - 1] + if (!active) return false + return !(active.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(prompt) }, PROMPT, { timeout: 15_000 }, diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index c3d975b63e81..a182ae68c3da 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -481,14 +481,17 @@ 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. - // Once a chat is loaded the button opens a TAB instead of replacing it — - // see newSessionOpensTab. + // 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) => { - if (newSessionOpensTab(activeSessionIdRef.current, $selectedStoredSessionId.get())) { + (path: null | string, options?: { openTab?: boolean }) => { + if (options?.openTab && newSessionOpensTab(activeSessionIdRef.current, $selectedStoredSessionId.get())) { void openNewSessionTile('center', { cwd: path, listed: false }) return @@ -799,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'), From 76e9ac3915e5bea2a7920a73a2d3c5d52aa546a9 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 05:00:18 -0500 Subject: [PATCH 3/4] fix(desktop): preserve correction order in session tabs --- .../e2e/correction-session-switch.spec.ts | 19 ++- .../e2e/image-attachment-resume.spec.ts | 7 +- apps/desktop/e2e/warm-resume-jitter.spec.ts | 158 ++++++++---------- .../src/app/chat/session-tile-actions.ts | 81 ++++++--- 4 files changed, 142 insertions(+), 123 deletions(-) diff --git a/apps/desktop/e2e/correction-session-switch.spec.ts b/apps/desktop/e2e/correction-session-switch.spec.ts index 0b9083807606..dc435b1d5384 100644 --- a/apps/desktop/e2e/correction-session-switch.spec.ts +++ b/apps/desktop/e2e/correction-session-switch.spec.ts @@ -21,11 +21,9 @@ 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.` -// A ⌘T-style tab can put several chat surfaces on the page at once, so every -// locator here is scoped to the ACTIVE one (the most recently mounted surface) -// rather than `.first()` / a bare document query, which would silently target -// the wrong session's composer or transcript. -const SURFACE = '[data-composer-target]' +// 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() @@ -102,7 +100,9 @@ async function transcriptMessageOrder(page: Page): Promise { 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) @@ -150,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 3c653f2e2c4f..a4f8da68e39d 100644 --- a/apps/desktop/e2e/image-attachment-resume.spec.ts +++ b/apps/desktop/e2e/image-attachment-resume.spec.ts @@ -93,10 +93,9 @@ function sessionRow(page: Page) { return page.locator('[data-slot="sidebar"] button').filter({ hasText: CAPTION }).first() } -// A ⌘T-style tab can put several chat surfaces on the page at once, so -// transcript queries target the ACTIVE (most recently mounted) surface rather -// than the first match, which would read the wrong session. -const SURFACE = '[data-composer-target]' +// 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) diff --git a/apps/desktop/e2e/warm-resume-jitter.spec.ts b/apps/desktop/e2e/warm-resume-jitter.spec.ts index 657c503bd9ca..2469ac72e4c9 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,10 @@ 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])' /** 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. */ @@ -155,8 +161,9 @@ test.afterAll(async () => { * add/remove nodes. */ async function installRenderCounter(page: import('@playwright/test').Page): Promise { - await page.evaluate(() => { - const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + await page.evaluate((surfaceSelector: string) => { + const surfaces = document.querySelectorAll(surfaceSelector) + const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]') if (!viewport) { throw new Error('Thread viewport not found before warm resume') } @@ -224,7 +231,53 @@ async function installRenderCounter(page: import('@playwright/test').Page): Prom hasMessages = true } }, 2) - }) + }, SURFACE) +} + +/** 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. */ @@ -261,7 +314,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 paints the transcript exactly once (no jitter)', async ({}, testInfo) => { const page = fixture!.page // Wait for the sidebar to populate with our seeded session. @@ -277,61 +330,22 @@ 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. The "+" opens a NEW TAB beside the - // resumed session rather than replacing it, so the old transcript stays - // mounted — assert the newly-added surface is the empty one. - await page.waitForFunction( - (firstMsg: string) => { - const surfaces = document.querySelectorAll('[data-composer-target]') - const active = surfaces[surfaces.length - 1] - if (!active) return false - const text = active.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '' - return !text.includes(firstMsg) - }, - FIRST_USER_MSG, - { timeout: 15_000 }, - ) + // Observe the loaded transcript before "+" hides it. The old test attached + // after the switch and watched the empty draft instead of this session. + await installRenderCounter(page) + await openNewSessionTab(page, FIRST_USER_MSG) await page.waitForTimeout(500) - // Step 3: Install render counter, click back (warm resume), wait, assert. - await installRenderCounter(page) + // Step 3: Click back, settle, and assert one paint with no second reconcile. 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) const result = await readRenderCount(page) @@ -357,13 +371,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. @@ -376,37 +384,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() - // "+" stacks a new tab, so the prior transcript stays mounted in its own - // surface — check the newly-added surface rather than the whole page. - await page.waitForFunction( - (prompt: string) => { - const surfaces = document.querySelectorAll('[data-composer-target]') - const active = surfaces[surfaces.length - 1] - if (!active) return false - return !(active.querySelector('[data-slot="aui_thread-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. @@ -415,13 +401,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 From f2f5b3253138d62bce2d391216151337f9011772 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 05:11:46 -0500 Subject: [PATCH 4/4] fix(desktop): expect keep-alive tabs not to repaint on reactivation --- apps/desktop/e2e/warm-resume-jitter.spec.ts | 66 +++++++++++++++++---- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/apps/desktop/e2e/warm-resume-jitter.spec.ts b/apps/desktop/e2e/warm-resume-jitter.spec.ts index 2469ac72e4c9..dbc0fe5dd113 100644 --- a/apps/desktop/e2e/warm-resume-jitter.spec.ts +++ b/apps/desktop/e2e/warm-resume-jitter.spec.ts @@ -49,6 +49,7 @@ 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. */ @@ -160,16 +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((surfaceSelector: string) => { - const surfaces = document.querySelectorAll(surfaceSelector) - const viewport = surfaces[surfaces.length - 1]?.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 @@ -231,7 +245,7 @@ async function installRenderCounter(page: import('@playwright/test').Page): Prom hasMessages = true } }, 2) - }, SURFACE) + }, [SURFACE, ALL_SURFACES, transcriptText] as [string, string, string | undefined]) } /** Wait until the ACTIVE chat surface's transcript contains `text`. */ @@ -298,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() @@ -314,7 +352,7 @@ function assertNoJitter(result: { bursts: number; mutations: number; timeline: n ).toBe(0) } -test('tab reactivation paints the 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. @@ -335,22 +373,24 @@ test('tab reactivation paints the transcript exactly once (no jitter)', async ({ // Wait for the session to fully settle (cold-path RPC + reconciliation). await page.waitForTimeout(2_000) - // Observe the loaded transcript before "+" hides it. The old test attached - // after the switch and watched the empty draft instead of this session. - await installRenderCounter(page) + // 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: Click back, settle, and assert one paint with no second reconcile. + // Step 3: Click back and verify the same kept-alive viewport becomes active + // without rebuilding or reconciling its transcript. await sessionRow.click() 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) => {