From 6382115de906f610c8119d1a60bb06a3433ec926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Sat, 9 May 2026 21:51:34 +0800 Subject: [PATCH] fix: keep chat tabs from losing scroll context Chat sessions need predictable navigation when users switch tabs or read history during streaming. This preserves each session's scroll position, defaults fresh sessions to the latest message, and adds a compact jump-to-latest affordance when auto-follow is paused. The PR gate also exposed that provider-scoped scheduled tasks must force the sdk-cli entrypoint when launched through the sidecar, so the same change records that runtime contract and its regression assertion. Constraint: Desktop chat should not force-scroll while the user is reading older messages Constraint: Scheduled task provider env must not inherit stale parent model runtime values Rejected: Persist scroll positions in localStorage | session scroll is transient UI state and should not survive app restarts Confidence: high Scope-risk: moderate Directive: Do not remove the sdk-cli entrypoint marker from provider-scoped cron tasks without rerunning the sidecar launcher regression Tested: cd desktop && bun run test -- MessageList.test.tsx Tested: cd desktop && bun run lint Tested: bun test src/server/__tests__/cron-scheduler-launcher.test.ts src/utils/__tests__/themeWords.test.ts Tested: bun run check:desktop Tested: bun run check:server Tested: bun run check:native Not-tested: bun run verify still fails on pre-existing agent-utils global coverage baseline below threshold Not-tested: Chrome extension E2E blocked by Codex Chrome Extension communication timeout after plugin diagnostics passed --- .../src/components/chat/MessageList.test.tsx | 185 +++++++++++++ desktop/src/components/chat/MessageList.tsx | 256 ++++++++++++------ desktop/src/i18n/locales/en.ts | 1 + desktop/src/i18n/locales/zh.ts | 1 + .../__tests__/cron-scheduler-launcher.test.ts | 1 + src/server/services/cronScheduler.ts | 5 +- src/utils/__tests__/themeWords.test.ts | 42 +++ 7 files changed, 409 insertions(+), 82 deletions(-) create mode 100644 src/utils/__tests__/themeWords.test.ts diff --git a/desktop/src/components/chat/MessageList.test.tsx b/desktop/src/components/chat/MessageList.test.tsx index b1e75d32..0b1ee1a0 100644 --- a/desktop/src/components/chat/MessageList.test.tsx +++ b/desktop/src/components/chat/MessageList.test.tsx @@ -537,6 +537,191 @@ describe('MessageList nested tool calls', () => { expect(scrollIntoView).toHaveBeenCalled() }) + it('restores a session scroll position when switching back to a tab', async () => { + const scrollIntoView = vi.fn() + Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { + configurable: true, + value: scrollIntoView, + }) + + useTabStore.setState({ + activeTabId: 'session-a', + tabs: [ + { sessionId: 'session-a', title: 'A', type: 'session' as const, status: 'idle' }, + { sessionId: 'session-b', title: 'B', type: 'session' as const, status: 'idle' }, + ], + }) + useChatStore.setState({ + sessions: { + 'session-a': makeSessionState({ + messages: [ + { id: 'a-user', type: 'user_text', content: 'A prompt', timestamp: 1 }, + { id: 'a-assistant', type: 'assistant_text', content: 'A response', timestamp: 2 }, + ], + }), + 'session-b': makeSessionState({ + messages: [ + { id: 'b-user', type: 'user_text', content: 'B prompt', timestamp: 1 }, + { id: 'b-assistant', type: 'assistant_text', content: 'B response', timestamp: 2 }, + ], + }), + }, + }) + + const { container } = render() + const scroller = container.querySelector('.overflow-y-auto') as HTMLDivElement + let scrollTop = 180 + Object.defineProperty(scroller, 'scrollHeight', { configurable: true, value: 1200 }) + Object.defineProperty(scroller, 'clientHeight', { configurable: true, value: 400 }) + Object.defineProperty(scroller, 'scrollTop', { + configurable: true, + get: () => scrollTop, + set: (value) => { + scrollTop = value + }, + }) + + fireEvent.scroll(scroller) + expect(screen.getByRole('button', { name: 'Latest' })).toBeTruthy() + + act(() => { + useTabStore.setState({ activeTabId: 'session-b' }) + }) + await waitFor(() => { + expect(screen.getByText('B response')).toBeTruthy() + }) + + scrollTop = 760 + fireEvent.scroll(scroller) + + act(() => { + useTabStore.setState({ activeTabId: 'session-a' }) + }) + await waitFor(() => { + expect(screen.getByText('A response')).toBeTruthy() + }) + + expect(scrollTop).toBe(180) + expect(screen.getByRole('button', { name: 'Latest' })).toBeTruthy() + }) + + it('scrolls new sessions to the latest message instead of inheriting another tab position', async () => { + const scrollIntoView = vi.fn() + Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { + configurable: true, + value: scrollIntoView, + }) + + useTabStore.setState({ + activeTabId: 'session-a', + tabs: [ + { sessionId: 'session-a', title: 'A', type: 'session' as const, status: 'idle' }, + { sessionId: 'session-fresh', title: 'Fresh', type: 'session' as const, status: 'idle' }, + ], + }) + useChatStore.setState({ + sessions: { + 'session-a': makeSessionState({ + messages: [ + { id: 'a-user', type: 'user_text', content: 'A prompt', timestamp: 1 }, + { id: 'a-assistant', type: 'assistant_text', content: 'A response', timestamp: 2 }, + ], + }), + 'session-fresh': makeSessionState({ + messages: [ + { id: 'fresh-user', type: 'user_text', content: 'Fresh prompt', timestamp: 1 }, + { id: 'fresh-assistant', type: 'assistant_text', content: 'Fresh latest response', timestamp: 2 }, + ], + }), + }, + }) + + const { container } = render() + const scroller = container.querySelector('.overflow-y-auto') as HTMLDivElement + Object.defineProperty(scroller, 'scrollHeight', { configurable: true, value: 1200 }) + Object.defineProperty(scroller, 'clientHeight', { configurable: true, value: 400 }) + Object.defineProperty(scroller, 'scrollTop', { + configurable: true, + value: 150, + writable: true, + }) + + fireEvent.scroll(scroller) + scrollIntoView.mockClear() + + act(() => { + useTabStore.setState({ activeTabId: 'session-fresh' }) + }) + + await waitFor(() => { + expect(screen.getByText('Fresh latest response')).toBeTruthy() + expect(scrollIntoView).toHaveBeenCalledWith({ behavior: 'auto', block: 'end' }) + }) + }) + + it('shows a latest button when reading history and resumes following after clicking it', async () => { + const scrollIntoView = vi.fn() + Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { + configurable: true, + value: scrollIntoView, + }) + + useChatStore.setState({ + sessions: { + [ACTIVE_TAB]: makeSessionState({ + chatState: 'streaming', + messages: [ + { + id: 'user-1', + type: 'user_text', + content: '历史消息', + timestamp: 1, + }, + ], + streamingText: 'streaming', + }), + }, + }) + + const { container } = render() + const scroller = container.querySelector('.overflow-y-auto') as HTMLDivElement + let scrollTop = 120 + Object.defineProperty(scroller, 'scrollHeight', { configurable: true, value: 1000 }) + Object.defineProperty(scroller, 'clientHeight', { configurable: true, value: 400 }) + Object.defineProperty(scroller, 'scrollTop', { + configurable: true, + get: () => scrollTop, + set: (value) => { + scrollTop = value + }, + }) + + scrollIntoView.mockClear() + fireEvent.scroll(scroller) + fireEvent.click(screen.getByRole('button', { name: 'Latest' })) + + expect(scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth', block: 'end' }) + expect(screen.queryByRole('button', { name: 'Latest' })).toBeNull() + + scrollIntoView.mockClear() + act(() => { + useChatStore.setState((state) => ({ + sessions: { + ...state.sessions, + [ACTIVE_TAB]: { + ...state.sessions[ACTIVE_TAB]!, + streamingText: 'streaming after jump', + }, + }, + })) + }) + + await waitFor(() => { + expect(screen.getByText('streaming after jump')).toBeTruthy() + }) + expect(scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth', block: 'end' }) + }) + it('keeps user actions anchored to the right bubble and assistant actions to the left bubble', () => { useChatStore.setState({ sessions: { diff --git a/desktop/src/components/chat/MessageList.tsx b/desktop/src/components/chat/MessageList.tsx index ce9f1f1a..79b01834 100644 --- a/desktop/src/components/chat/MessageList.tsx +++ b/desktop/src/components/chat/MessageList.tsx @@ -1,4 +1,5 @@ -import { useRef, useEffect, useMemo, memo, useState, useCallback } from 'react' +import { useRef, useEffect, useMemo, memo, useState, useCallback, useLayoutEffect } from 'react' +import { ArrowDown } from 'lucide-react' import { ApiError } from '../../api/client' import { sessionsApi, type SessionTurnCheckpoint } from '../../api/sessions' import { useChatStore } from '../../stores/chatStore' @@ -246,6 +247,28 @@ type MessageListProps = { } const AUTO_SCROLL_BOTTOM_THRESHOLD_PX = 48 +const MAX_SCROLL_SNAPSHOTS = 100 +const CHAT_SCROLL_AREA_CLASS = [ + 'chat-scroll-area', + '[scrollbar-width:auto]', + '[scrollbar-color:color-mix(in_srgb,var(--color-outline)_72%,transparent)_transparent]', + '[&::-webkit-scrollbar]:w-2.5', + '[&::-webkit-scrollbar-track]:bg-transparent', + '[&::-webkit-scrollbar-thumb]:rounded-full', + '[&::-webkit-scrollbar-thumb]:border-[3px]', + '[&::-webkit-scrollbar-thumb]:border-transparent', + '[&::-webkit-scrollbar-thumb]:bg-[color-mix(in_srgb,var(--color-outline)_74%,transparent)]', + '[&::-webkit-scrollbar-thumb]:bg-clip-content', + '[&::-webkit-scrollbar-thumb:hover]:border-2', + '[&::-webkit-scrollbar-thumb:hover]:bg-[color-mix(in_srgb,var(--color-outline)_90%,transparent)]', +].join(' ') + +type SessionScrollSnapshot = { + scrollTop: number + wasAtBottom: boolean +} + +const sessionScrollSnapshots = new Map() function isNearScrollBottom(element: HTMLElement) { return ( @@ -254,6 +277,24 @@ function isNearScrollBottom(element: HTMLElement) { ) } +function rememberSessionScroll(sessionId: string, element: HTMLElement) { + if (sessionScrollSnapshots.size >= MAX_SCROLL_SNAPSHOTS && !sessionScrollSnapshots.has(sessionId)) { + const oldestSessionId = sessionScrollSnapshots.keys().next().value + if (oldestSessionId) { + sessionScrollSnapshots.delete(oldestSessionId) + } + } + + sessionScrollSnapshots.set(sessionId, { + scrollTop: element.scrollTop, + wasAtBottom: isNearScrollBottom(element), + }) +} + +function clampScrollTop(element: HTMLElement, scrollTop: number) { + return Math.max(0, Math.min(scrollTop, element.scrollHeight - element.clientHeight)) +} + export function MessageList({ sessionId, compact = false }: MessageListProps = {}) { const activeTabId = useTabStore((s) => s.activeTabId) const resolvedSessionId = sessionId ?? activeTabId @@ -283,23 +324,61 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = { const [isLoadingTurnChangeCards, setIsLoadingTurnChangeCards] = useState(false) const [rewindingTurnId, setRewindingTurnId] = useState(null) const [turnUndoConfirmTargetId, setTurnUndoConfirmTargetId] = useState(null) + const [showJumpToLatest, setShowJumpToLatest] = useState(false) + + const scrollToBottom = useCallback((behavior: ScrollBehavior) => { + shouldAutoScrollRef.current = true + bottomRef.current?.scrollIntoView?.({ behavior, block: 'end' }) + const container = scrollContainerRef.current + if (container && resolvedSessionId) { + sessionScrollSnapshots.set(resolvedSessionId, { + scrollTop: Math.max(0, container.scrollHeight - container.clientHeight), + wasAtBottom: true, + }) + } + setShowJumpToLatest(false) + }, [resolvedSessionId]) const updateAutoScrollState = useCallback(() => { const container = scrollContainerRef.current if (!container) return - shouldAutoScrollRef.current = isNearScrollBottom(container) - }, []) + const isAtBottom = isNearScrollBottom(container) + shouldAutoScrollRef.current = isAtBottom + setShowJumpToLatest(!isAtBottom) + + if (resolvedSessionId) { + rememberSessionScroll(resolvedSessionId, container) + } + }, [resolvedSessionId]) + + useLayoutEffect(() => { + if (lastSessionIdRef.current !== resolvedSessionId) { + const snapshot = resolvedSessionId ? sessionScrollSnapshots.get(resolvedSessionId) : undefined + shouldAutoScrollRef.current = snapshot?.wasAtBottom ?? true + lastSessionIdRef.current = resolvedSessionId + + const container = scrollContainerRef.current + if (container && snapshot && !snapshot.wasAtBottom) { + container.scrollTop = clampScrollTop(container, snapshot.scrollTop) + setShowJumpToLatest(true) + } else { + scrollToBottom('auto') + } + } + }, [resolvedSessionId, scrollToBottom]) useEffect(() => { - if (lastSessionIdRef.current !== resolvedSessionId) { - shouldAutoScrollRef.current = true - lastSessionIdRef.current = resolvedSessionId + if (!shouldAutoScrollRef.current) { + setShowJumpToLatest(true) + return } - if (!shouldAutoScrollRef.current) return + scrollToBottom('smooth') + }, [messages.length, resolvedSessionId, scrollToBottom, streamingText]) - bottomRef.current?.scrollIntoView?.({ behavior: 'smooth' }) - }, [messages.length, resolvedSessionId, streamingText]) + const handleJumpToLatest = useCallback(() => { + scrollToBottom('smooth') + }, [scrollToBottom]) const { toolResultMap, childToolCallsByParent, renderItems } = useMemo( () => buildRenderModel(messages), @@ -446,84 +525,99 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = { ]) return ( -
-
- {renderItems.map((item, index) => { - const cardsForItem = turnCardsByRenderIndex.get(index) ?? [] +
+
+
+ {renderItems.map((item, index) => { + const cardsForItem = turnCardsByRenderIndex.get(index) ?? [] - return ( -
- {item.kind === 'tool_group' ? ( - !toolResultMap.has(tc.toolUseId)) - } - /> - ) : ( - { - const result = toolResultMap.get(item.message.toolUseId) - return result ? { content: result.content, isError: result.isError } : null - })() - : null - } - /> - )} + return ( +
+ {item.kind === 'tool_group' ? ( + !toolResultMap.has(tc.toolUseId)) + } + /> + ) : ( + { + const result = toolResultMap.get(item.message.toolUseId) + return result ? { content: result.content, isError: result.isError } : null + })() + : null + } + /> + )} - {resolvedSessionId && cardsForItem.map((card) => ( - { - setTurnUndoConfirmTargetId(card.target.messageId) - }} - /> - ))} + {resolvedSessionId && cardsForItem.map((card) => ( + { + setTurnUndoConfirmTargetId(card.target.messageId) + }} + /> + ))} +
+ ) + })} + + {streamingText.trim() && ( + + )} + + {/* Show StreamingIndicator when: + - tool_executing: tool is running + - thinking but no active ThinkingBlock yet: the gap between + sending a message and receiving the first thinking delta */} + {(chatState === 'tool_executing' || (chatState === 'thinking' && !activeThinkingId)) && ( + + )} + + {!isLoadingTurnChangeCards && turnChangeCards.length === 0 && turnChangeLoadError && ( +
+ {turnChangeLoadError}
- ) - })} + )} - {streamingText.trim() && ( - - )} - - {/* Show StreamingIndicator when: - - tool_executing: tool is running - - thinking but no active ThinkingBlock yet: the gap between - sending a message and receiving the first thinking delta */} - {(chatState === 'tool_executing' || (chatState === 'thinking' && !activeThinkingId)) && ( - - )} - - {!isLoadingTurnChangeCards && turnChangeCards.length === 0 && turnChangeLoadError && ( -
- {turnChangeLoadError} -
- )} - -
+
+
+ {showJumpToLatest && ( + + )} + { diff --git a/desktop/src/i18n/locales/en.ts b/desktop/src/i18n/locales/en.ts index 45920002..cbc57c6a 100644 --- a/desktop/src/i18n/locales/en.ts +++ b/desktop/src/i18n/locales/en.ts @@ -823,6 +823,7 @@ export const en = { 'chat.select': 'select', 'chat.dismiss': 'dismiss', 'chat.stopTitle': 'Stop generation (Cmd+.)', + 'chat.jumpToLatest': 'Latest', 'chat.rewindSuccessWithCode': 'Rewound {count} messages and restored tracked files.', 'chat.rewindSuccessConversationOnly': 'Rewound {count} messages. No file checkpoint was available for this turn.', 'chat.turnChangesTitle': '{count} files changed', diff --git a/desktop/src/i18n/locales/zh.ts b/desktop/src/i18n/locales/zh.ts index 801f69ae..0925cb5b 100644 --- a/desktop/src/i18n/locales/zh.ts +++ b/desktop/src/i18n/locales/zh.ts @@ -825,6 +825,7 @@ export const zh: Record = { 'chat.select': '选择', 'chat.dismiss': '关闭', 'chat.stopTitle': '停止生成 (Cmd+.)', + 'chat.jumpToLatest': '回到最新', 'chat.rewindSuccessWithCode': '已回滚 {count} 条消息，并恢复相关文件。', 'chat.rewindSuccessConversationOnly': '已回滚 {count} 条消息。这一轮没有可用的文件检查点。', 'chat.turnChangesTitle': '{count} 个文件已更改', diff --git a/src/server/__tests__/cron-scheduler-launcher.test.ts b/src/server/__tests__/cron-scheduler-launcher.test.ts index 8e9eee04..d7da04c5 100644 --- a/src/server/__tests__/cron-scheduler-launcher.test.ts +++ b/src/server/__tests__/cron-scheduler-launcher.test.ts @@ -295,6 +295,7 @@ describe('cron scheduler launcher resolution', () => { ) expect(env.ANTHROPIC_API_KEY).toBe('proxy-managed') expect(env.ANTHROPIC_MODEL).toBe('provider-fast') + expect(env.ANTHROPIC_MODEL).not.toBe('stale-parent-model') expect(env.CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST).toBe('1') expect(env.CLAUDE_CODE_ENTRYPOINT).toBe('sdk-cli') }) diff --git a/src/server/services/cronScheduler.ts b/src/server/services/cronScheduler.ts index e7d9bf0f..9cb9fc78 100644 --- a/src/server/services/cronScheduler.ts +++ b/src/server/services/cronScheduler.ts @@ -666,7 +666,10 @@ export class CronScheduler { PWD: workDir, CC_HAHA_SKIP_DOTENV: '1', ...(explicitProviderEnv - ? { CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST: '1' } + ? { + CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST: '1', + CLAUDE_CODE_ENTRYPOINT: 'sdk-cli', + } : {}), ...(explicitProviderEnv ?? {}), ...(this.shouldMarkManagedOAuth(task.providerId) diff --git a/src/utils/__tests__/themeWords.test.ts b/src/utils/__tests__/themeWords.test.ts new file mode 100644 index 00000000..d58fa754 --- /dev/null +++ b/src/utils/__tests__/themeWords.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, test } from 'bun:test' + +import { + getTheme, + THEME_NAMES, + themeColorToAnsi, + type ThemeName, +} from '../theme.js' +import { generateShortWordSlug, generateWordSlug } from '../words.js' + +describe('theme utilities', () => { + test('resolves every supported theme to a complete palette', () => { + for (const themeName of THEME_NAMES) { + const theme = getTheme(themeName) + + expect(theme.text).toBeTruthy() + expect(theme.inverseText).toBeTruthy() + expect(theme.success).toBeTruthy() + expect(theme.error).toBeTruthy() + expect(theme.claude).toBeTruthy() + } + }) + + test('falls back to the dark theme for unknown concrete theme names', () => { + const darkTheme = getTheme('dark') + const unknownTheme = getTheme('unknown' as ThemeName) + + expect(unknownTheme).toBe(darkTheme) + }) + + test('converts rgb theme colors to ansi escapes', () => { + expect(themeColorToAnsi('rgb(12, 34, 56)')).toEqual(expect.any(String)) + expect(themeColorToAnsi('not-a-color')).toBe('\x1b[35m') + }) +}) + +describe('word slug utilities', () => { + test('generates hyphenated slugs with the expected part count', () => { + expect(generateWordSlug().split('-')).toHaveLength(3) + expect(generateShortWordSlug().split('-')).toHaveLength(2) + }) +})