From f17b0828a09d5cdc0a2d1ff759b321df5ce4fa6a 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: Sun, 31 May 2026 00:01:04 +0800 Subject: [PATCH] feat(desktop): change-card row opens the workspace diff instead of an inline diff Co-Authored-By: Claude Opus 4.8 (1M context) --- .../chat/CurrentTurnChangeCard.test.tsx | 77 +++++++--- .../components/chat/CurrentTurnChangeCard.tsx | 135 ++++-------------- .../src/components/chat/MessageList.test.tsx | 77 +++++----- desktop/src/components/chat/MessageList.tsx | 1 - desktop/src/i18n/locales/en.ts | 5 +- desktop/src/i18n/locales/zh.ts | 5 +- 6 files changed, 125 insertions(+), 175 deletions(-) diff --git a/desktop/src/components/chat/CurrentTurnChangeCard.test.tsx b/desktop/src/components/chat/CurrentTurnChangeCard.test.tsx index 34b640ce..3dacbd69 100644 --- a/desktop/src/components/chat/CurrentTurnChangeCard.test.tsx +++ b/desktop/src/components/chat/CurrentTurnChangeCard.test.tsx @@ -14,13 +14,6 @@ const { openPreviewSpy, browserOpenSpy, openTargetSpy, ensureTargetsMock } = vi. return { openPreviewSpy, browserOpenSpy, openTargetSpy, ensureTargetsMock } }) -// Mock sessionsApi so diff calls don't run -vi.mock('../../api/sessions', () => ({ - sessionsApi: { - getTurnCheckpointDiff: vi.fn().mockResolvedValue({ state: 'ok', diff: '--- a\n+++ b\n@@ -0,0 +1 @@\n+hello' }), - }, -})) - // Mock openTargetStore vi.mock('../../stores/openTargetStore', () => ({ useOpenTargetStore: Object.assign( @@ -120,7 +113,6 @@ function renderCard(filesChanged: string[]) { return render( { }) }) +describe('CurrentTurnChangeCard – row opens the workspace diff', () => { + beforeEach(() => { + vi.clearAllMocks() + ensureTargetsMock.mockResolvedValue(undefined) + openPreviewSpy.mockResolvedValue(undefined) + }) + + it('clicking a file row calls openPreview(sessionId, displayPath, "diff")', () => { + renderCard(['/w/proj/src/main.ts']) + const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ }) + fireEvent.click(row) + // displayPath is the workDir-relative path (matches the workspace file tree) + expect(openPreviewSpy).toHaveBeenCalledWith('s1', 'src/main.ts', 'diff') + }) + + it('passes the workDir-relative displayPath (not the absolute path) to openPreview', () => { + renderCard(['/w/proj/README.md']) + const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ }) + fireEvent.click(row) + expect(openPreviewSpy).toHaveBeenCalledWith('s1', 'README.md', 'diff') + }) + + it('does NOT render an inline diff surface after clicking a row', () => { + renderCard(['/w/proj/src/main.ts']) + const row = screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ }) + fireEvent.click(row) + // No inline diff is rendered inside the card anymore — the diff opens in the + // right-side workspace panel instead. + expect(screen.queryByText('chat.turnChangesDiffLoading')).not.toBeInTheDocument() + expect(screen.queryByText('chat.turnChangesDiffUnavailable')).not.toBeInTheDocument() + // The CodeMirror diff surface (.cm-editor) is never mounted in the card. + expect(document.querySelector('.cm-editor')).toBeNull() + }) + + it('each file row exposes a single "open in workspace" button (no expand/collapse toggle)', () => { + renderCard(['/w/proj/README.md', '/w/proj/src/index.ts']) + expect(screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toHaveLength(2) + }) +}) + describe('CurrentTurnChangeCard – open-with buttons', () => { beforeEach(() => { vi.clearAllMocks() @@ -178,11 +210,11 @@ describe('CurrentTurnChangeCard – open-with buttons', () => { expect(buttons).toHaveLength(2) }) - it('does NOT render an "open-with" button for a source file (diff toggle stays)', () => { + it('does NOT render an "open-with" button for a source file (row still opens workspace)', () => { renderCard(['/w/proj/src/main.ts']) expect(screen.queryAllByRole('button', { name: 'openWith.title' })).toHaveLength(0) - // source files keep their inline diff toggle — only the open-with pill is dropped - expect(screen.getByRole('button', { name: /turnChangesShowDiffAria/ })).toBeInTheDocument() + // source files keep their workspace-open row — only the open-with pill is dropped + expect(screen.getByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toBeInTheDocument() }) it('mixed turn: only previewable rows (md/html) get the open-with button, not .ts', () => { @@ -240,15 +272,16 @@ describe('CurrentTurnChangeCard – open-with buttons', () => { expect(ensureTargetsMock).toHaveBeenCalledTimes(1) }) - it('diff-toggle button still works (not nested button regression)', async () => { + it('open-with button does not also trigger the row workspace-open (stopPropagation)', async () => { renderCard(['/w/proj/README.md']) - // The diff toggle has aria-label from the i18n key + path - const diffBtn = screen.getByRole('button', { name: /turnChangesShowDiffAria/ }) - expect(diffBtn).toBeInTheDocument() - // Clicking should not throw + const [openWithBtn] = screen.getAllByRole('button', { name: 'openWith.title' }) + await act(async () => { - fireEvent.click(diffBtn) + fireEvent.click(openWithBtn!) }) + + // The diff open (3rd arg 'diff') must not have fired from clicking the pill. + expect(openPreviewSpy).not.toHaveBeenCalledWith('s1', 'README.md', 'diff') }) }) @@ -265,15 +298,15 @@ describe('CurrentTurnChangeCard – collapse long file lists', () => { it('does NOT render a show-more toggle with ≤5 files', () => { renderCard(makeFiles(5)) - expect(screen.getAllByRole('button', { name: /turnChangesShowDiffAria/ })).toHaveLength(5) + expect(screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toHaveLength(5) expect(screen.queryByText('chat.turnChangesShowMore')).not.toBeInTheDocument() expect(screen.queryByText('chat.turnChangesShowLess')).not.toBeInTheDocument() }) it('with 8 files shows only 5 rows + a "show more" toggle (remaining = 3)', () => { renderCard(makeFiles(8)) - // only the first 5 diff-toggle rows are rendered - expect(screen.getAllByRole('button', { name: /turnChangesShowDiffAria/ })).toHaveLength(5) + // only the first 5 workspace-open rows are rendered + expect(screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toHaveLength(5) // the show-more toggle is present (identity-mock key). The real key carries the // remaining count via '{count}'; with the placeholder-bearing real string this // renders as "再显示 3 个文件" (8 - COLLAPSED_COUNT(5) = 3). @@ -287,13 +320,13 @@ describe('CurrentTurnChangeCard – collapse long file lists', () => { const showMore = screen.getByText('chat.turnChangesShowMore') fireEvent.click(showMore) - expect(screen.getAllByRole('button', { name: /turnChangesShowDiffAria/ })).toHaveLength(8) + expect(screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toHaveLength(8) const showLess = screen.getByText('chat.turnChangesShowLess') expect(showLess).toBeInTheDocument() expect(screen.queryByText('chat.turnChangesShowMore')).not.toBeInTheDocument() fireEvent.click(showLess) - expect(screen.getAllByRole('button', { name: /turnChangesShowDiffAria/ })).toHaveLength(5) + expect(screen.getAllByRole('button', { name: /turnChangesOpenInWorkspaceAria/ })).toHaveLength(5) expect(screen.getByText('chat.turnChangesShowMore')).toBeInTheDocument() }) }) diff --git a/desktop/src/components/chat/CurrentTurnChangeCard.tsx b/desktop/src/components/chat/CurrentTurnChangeCard.tsx index 5c2f251a..5f4722db 100644 --- a/desktop/src/components/chat/CurrentTurnChangeCard.tsx +++ b/desktop/src/components/chat/CurrentTurnChangeCard.tsx @@ -1,9 +1,8 @@ import { useCallback, useMemo, useState } from 'react' import type { MouseEvent as ReactMouseEvent } from 'react' import { ChevronDown, ChevronUp } from 'lucide-react' -import { sessionsApi, type SessionTurnCheckpoint } from '../../api/sessions' +import type { SessionTurnCheckpoint } from '../../api/sessions' import { useTranslation, type TranslationKey } from '../../i18n' -import { WorkspaceDiffSurface } from '../workspace/WorkspaceCodeSurface' import { OpenWithMenu } from '../common/OpenWithMenu' import { buildOpenWithItems, describeFileType, isPreviewableChangedFile, type OpenWithItem } from '../../lib/openWithItems' import { openWithContextForWorkspaceFile } from '../../lib/openWithContextForHref' @@ -12,15 +11,8 @@ import { useOpenTargetStore } from '../../stores/openTargetStore' import { useBrowserPanelStore } from '../../stores/browserPanelStore' import { useWorkspacePanelStore } from '../../stores/workspacePanelStore' -type DiffPreviewState = { - loading: boolean - diff?: string - error?: string -} - type CurrentTurnChangeCardProps = { sessionId: string - targetUserMessageId: string checkpoint: SessionTurnCheckpoint workDir: string | null error: string | null @@ -38,7 +30,6 @@ const COLLAPSED_COUNT = 5 export function CurrentTurnChangeCard({ sessionId, - targetUserMessageId, checkpoint, workDir, error, @@ -47,8 +38,6 @@ export function CurrentTurnChangeCard({ onUndo, }: CurrentTurnChangeCardProps) { const t = useTranslation() - const [expandedPath, setExpandedPath] = useState(null) - const [diffByPath, setDiffByPath] = useState>({}) const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect } | null>(null) const [showAllFiles, setShowAllFiles] = useState(false) @@ -65,49 +54,12 @@ export function CurrentTurnChangeCard({ ? files.slice(0, COLLAPSED_COUNT) : files - const toggleDiff = useCallback((fileEntry: ChangedFileEntry) => { - const nextExpandedPath = expandedPath === fileEntry.apiPath ? null : fileEntry.apiPath - setExpandedPath(nextExpandedPath) - if (!nextExpandedPath || diffByPath[fileEntry.apiPath]?.diff || diffByPath[fileEntry.apiPath]?.loading) { - return - } - - setDiffByPath((current) => ({ - ...current, - [fileEntry.apiPath]: { loading: true }, - })) - - void sessionsApi - .getTurnCheckpointDiff( - sessionId, - targetUserMessageId, - fileEntry.apiPath, - checkpoint.target.userMessageIndex, - ) - .then((result) => { - setDiffByPath((current) => ({ - ...current, - [fileEntry.apiPath]: { - loading: false, - diff: result.state === 'ok' ? result.diff || '' : undefined, - error: result.state === 'ok' - ? undefined - : result.error || t('chat.turnChangesDiffUnavailable'), - }, - })) - }) - .catch((diffError) => { - setDiffByPath((current) => ({ - ...current, - [fileEntry.apiPath]: { - loading: false, - error: diffError instanceof Error - ? diffError.message - : String(diffError), - }, - })) - }) - }, [diffByPath, expandedPath, sessionId, t, targetUserMessageId]) + const openDiffInWorkspace = useCallback((fileEntry: ChangedFileEntry) => { + // Jump to the right-side workspace and open a diff tab. We pass the workDir-relative + // path (same format the workspace file tree passes to openPreview), so the diff tab + // is keyed/fetched identically to the tree-driven one. + void useWorkspacePanelStore.getState().openPreview(sessionId, fileEntry.displayPath, 'diff') + }, [sessionId]) const handleOpenWith = useCallback((event: ReactMouseEvent, fileEntry: ChangedFileEntry) => { event.stopPropagation() @@ -180,66 +132,35 @@ export function CurrentTurnChangeCard({
{visibleFiles.map((fileEntry) => { - const isExpanded = expandedPath === fileEntry.apiPath - const diffState = diffByPath[fileEntry.apiPath] const fileName = fileEntry.displayPath.split('/').pop() || fileEntry.displayPath const typeInfo = describeFileType(fileEntry.displayPath) const previewable = isPreviewableChangedFile(fileEntry.displayPath) return ( -
-
+
+ + {previewable && ( - {previewable && ( - - )} -
- - {isExpanded && ( -
- {diffState?.loading ? ( -
- {t('chat.turnChangesDiffLoading')} -
- ) : diffState?.error ? ( -
- {diffState.error} -
- ) : diffState?.diff ? ( - - ) : ( -
- {t('chat.turnChangesDiffUnavailable')} -
- )} -
)}
) diff --git a/desktop/src/components/chat/MessageList.test.tsx b/desktop/src/components/chat/MessageList.test.tsx index e59eb8a0..6ed8a784 100644 --- a/desktop/src/components/chat/MessageList.test.tsx +++ b/desktop/src/components/chat/MessageList.test.tsx @@ -5,6 +5,7 @@ import { relativizeWorkspacePath } from './CurrentTurnChangeCard' import { sessionsApi } from '../../api/sessions' import { useChatStore } from '../../stores/chatStore' import { useWorkspaceChatContextStore } from '../../stores/workspaceChatContextStore' +import { useWorkspacePanelStore } from '../../stores/workspacePanelStore' import { useSettingsStore } from '../../stores/settingsStore' import { useSessionStore } from '../../stores/sessionStore' import { useTabStore } from '../../stores/tabStore' @@ -110,6 +111,9 @@ describe('MessageList nested tool calls', () => { useSessionStore.setState({ sessions: [], activeSessionId: null, isLoading: false, error: null }) useChatStore.setState({ sessions: { [ACTIVE_TAB]: makeSessionState() } }) useWorkspaceChatContextStore.setState(useWorkspaceChatContextStore.getInitialState(), true) + // The workspace panel store is a shared singleton; reset it so preview tabs opened by + // one test (clicking a change-card row) don't dedupe/leak into the next test. + useWorkspacePanelStore.setState(useWorkspacePanelStore.getInitialState(), true) vi.spyOn(sessionsApi, 'getTurnCheckpoints').mockImplementation( () => new Promise(() => {}), ) @@ -3020,7 +3024,7 @@ describe('MessageList nested tool calls', () => { expect(screen.queryByText('third.ts')).toBeNull() }) - it('expands a historical turn diff through the turn checkpoint diff API', async () => { + it('opens the workspace diff (working-tree) when a historical turn change row is clicked', async () => { vi.spyOn(sessionsApi, 'getTurnCheckpoints').mockResolvedValue({ checkpoints: [ { @@ -3051,16 +3055,12 @@ describe('MessageList nested tool calls', () => { }, ], }) - vi.spyOn(sessionsApi, 'getWorkspaceDiff').mockResolvedValue({ - state: 'ok', - path: 'src/first.ts', - diff: 'diff --session a/src/first.ts b/src/first.ts\n-old\n+new', - }) - vi.spyOn(sessionsApi, 'getTurnCheckpointDiff').mockResolvedValue({ + const getWorkspaceDiff = vi.spyOn(sessionsApi, 'getWorkspaceDiff').mockResolvedValue({ state: 'ok', path: 'src/first.ts', diff: 'diff --session a/src/first.ts b/src/first.ts\n-old\n+new', }) + const getTurnCheckpointDiff = vi.spyOn(sessionsApi, 'getTurnCheckpointDiff') useChatStore.setState({ sessions: { @@ -3097,20 +3097,21 @@ describe('MessageList nested tool calls', () => { render() - fireEvent.click(await screen.findByRole('button', { name: 'Show diff for src/first.ts' })) + // Clicking the row no longer expands an inline diff inside the card — it jumps to + // the right-side workspace and opens a diff tab (via workspacePanelStore.openPreview, + // which fetches the *current working-tree* diff through getWorkspaceDiff). + fireEvent.click(await screen.findByRole('button', { name: 'Open src/first.ts in workspace' })) - const diffSurface = await screen.findByTestId('workspace-code') - expect(diffSurface.textContent).toContain('+new') - expect(sessionsApi.getTurnCheckpointDiff).toHaveBeenCalledWith( - ACTIVE_TAB, - 'user-1', - 'src/first.ts', - 0, - ) - expect(sessionsApi.getWorkspaceDiff).not.toHaveBeenCalled() + await waitFor(() => { + expect(getWorkspaceDiff).toHaveBeenCalledWith(ACTIVE_TAB, 'src/first.ts') + }) + // The turn-snapshot diff endpoint is no longer used by the card. + expect(getTurnCheckpointDiff).not.toHaveBeenCalled() + // No inline diff surface is mounted inside the transcript anymore. + expect(screen.queryByTestId('workspace-code')).toBeNull() }) - it('keeps checkpoint paths bound to the original turn cwd when expanding historical diffs', async () => { + it('opens the workspace diff with the turn-relativized path (working-tree, not the turn snapshot)', async () => { vi.spyOn(sessionsApi, 'getWorkspaceStatus').mockResolvedValue({ state: 'ok', workDir: '/tmp/current-project', @@ -3137,11 +3138,12 @@ describe('MessageList nested tool calls', () => { }, ], }) - vi.spyOn(sessionsApi, 'getTurnCheckpointDiff').mockResolvedValue({ + const getWorkspaceDiff = vi.spyOn(sessionsApi, 'getWorkspaceDiff').mockResolvedValue({ state: 'ok', - path: '/tmp/old-project/src/first.ts', + path: 'src/first.ts', diff: 'diff --git a/src/first.ts b/src/first.ts\n-old\n+new', }) + const getTurnCheckpointDiff = vi.spyOn(sessionsApi, 'getTurnCheckpointDiff') useChatStore.setState({ sessions: { @@ -3166,15 +3168,17 @@ describe('MessageList nested tool calls', () => { render() - fireEvent.click(await screen.findByRole('button', { name: 'Show diff for src/first.ts' })) + // The checkpoint's absolute path (under the turn's original cwd /tmp/old-project) is + // relativized to 'src/first.ts' for display. Clicking the row opens the right-side + // workspace diff for that relative path. Caveat (intended): the workspace diff is the + // current working-tree diff, NOT the historical turn snapshot — so the turn cwd is no + // longer carried through, and getTurnCheckpointDiff is not called. + fireEvent.click(await screen.findByRole('button', { name: 'Open src/first.ts in workspace' })) - await screen.findByTestId('workspace-code') - expect(sessionsApi.getTurnCheckpointDiff).toHaveBeenCalledWith( - ACTIVE_TAB, - 'user-1', - '/tmp/old-project/src/first.ts', - 0, - ) + await waitFor(() => { + expect(getWorkspaceDiff).toHaveBeenCalledWith(ACTIVE_TAB, 'src/first.ts') + }) + expect(getTurnCheckpointDiff).not.toHaveBeenCalled() }) it('relativizes Windows checkpoint paths against the turn workdir', () => { @@ -3202,7 +3206,7 @@ describe('MessageList nested tool calls', () => { }, ], }) - vi.spyOn(sessionsApi, 'getTurnCheckpointDiff').mockResolvedValue({ + const getWorkspaceDiff = vi.spyOn(sessionsApi, 'getWorkspaceDiff').mockResolvedValue({ state: 'ok', path: 'src/live.ts', diff: 'diff --session a/src/live.ts b/src/live.ts\n+live', @@ -3231,15 +3235,14 @@ describe('MessageList nested tool calls', () => { render() + // The card only renders if the transcript checkpoint (id 'transcript-user-1') was + // matched to the local message ('local-user-temp-id') by userMessageIndex. expect(await screen.findByText('live.ts')).toBeTruthy() - fireEvent.click(screen.getByRole('button', { name: 'Show diff for src/live.ts' })) - await screen.findByTestId('workspace-code') - expect(sessionsApi.getTurnCheckpointDiff).toHaveBeenCalledWith( - ACTIVE_TAB, - 'transcript-user-1', - 'src/live.ts', - 0, - ) + // Clicking the row jumps to the right-side workspace diff for the relativized path. + fireEvent.click(screen.getByRole('button', { name: 'Open src/live.ts in workspace' })) + await waitFor(() => { + expect(getWorkspaceDiff).toHaveBeenCalledWith(ACTIVE_TAB, 'src/live.ts') + }) }) it('keeps turn change cards anchored when the only response item is filtered from rendering', async () => { diff --git a/desktop/src/components/chat/MessageList.tsx b/desktop/src/components/chat/MessageList.tsx index 9d1efe71..d42670d3 100644 --- a/desktop/src/components/chat/MessageList.tsx +++ b/desktop/src/components/chat/MessageList.tsx @@ -1798,7 +1798,6 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = { = { 'chat.turnChangesHistoricalConfirmBody': '这会把会话回滚到这一轮之前，并恢复该检查点对应的文件变更。', 'chat.turnChangesLatestConfirmUndo': '撤销当前轮次', 'chat.turnChangesHistoricalConfirmUndo': '回滚到这一轮之前', - 'chat.turnChangesShowDiffAria': '查看 {path} 的 diff', - 'chat.turnChangesHideDiffAria': '收起 {path} 的 diff', - 'chat.turnChangesDiffLoading': '正在加载 diff...', - 'chat.turnChangesDiffUnavailable': 'Diff 不可用。', + 'chat.turnChangesOpenInWorkspaceAria': '在工作区打开 {path}', 'chat.turnChangesShowMore': '再显示 {count} 个文件', 'chat.turnChangesShowLess': '收起',