diff --git a/src/renderer/src/components/editor/EditorAutosaveController.tsx b/src/renderer/src/components/editor/EditorAutosaveController.tsx index 0ba4358d7..d96556166 100644 --- a/src/renderer/src/components/editor/EditorAutosaveController.tsx +++ b/src/renderer/src/components/editor/EditorAutosaveController.tsx @@ -1,13 +1,21 @@ import { useEffect } from 'react' import { useAppStore } from '@/store' import { attachEditorAutosaveController } from './editor-autosave-controller' +import { attachRestoredTabConflictScan } from './editor-restored-tab-conflict-scan' export default function EditorAutosaveController(): null { useEffect(() => { // Why: autosave and quit coordination need to survive editor tab switches, // but keeping the full EditorPanel mounted while hidden widened the restart // surface too far. Keep only this narrow controller alive between mounts. - return attachEditorAutosaveController(useAppStore) + const detachAutosave = attachEditorAutosaveController(useAppStore) + // Why: restored dirty tabs must be conflict-checked app-level, before any + // panel mounts — autosave can otherwise write over an offline agent edit. + const detachConflictScan = attachRestoredTabConflictScan(useAppStore) + return () => { + detachAutosave() + detachConflictScan() + } }, []) return null diff --git a/src/renderer/src/components/editor/EditorContent.test.tsx b/src/renderer/src/components/editor/EditorContent.test.tsx index d1507dd2a..130c92abe 100644 --- a/src/renderer/src/components/editor/EditorContent.test.tsx +++ b/src/renderer/src/components/editor/EditorContent.test.tsx @@ -1,6 +1,14 @@ import { renderToStaticMarkup } from 'react-dom/server' import { afterEach, describe, expect, it, vi } from 'vitest' import type { OpenFile } from '@/store/slices/editor' + +// Why: EditorContent's mode renderers (Monaco, DiffViewer, ...) are lazy(); +// renderToStaticMarkup cannot resolve them. Stubbing them keeps the banner +// branch structure renderable so its placement is pinned by tests. +vi.mock('@/lib/lazy-with-retry', () => ({ + lazyWithRetry: () => () => null +})) + import { EditorContent, getMarkdownSourceLineOffset } from './EditorContent' function createOpenFile(overrides: Partial = {}): OpenFile { @@ -67,7 +75,7 @@ describe('EditorContent', () => { handleDirtyStateHint={vi.fn()} handleSave={vi.fn()} handleSaveForFile={vi.fn()} - reloadFileContent={vi.fn()} + reloadContent={vi.fn()} /> ) @@ -75,4 +83,137 @@ describe('EditorContent', () => { expect(html).toContain('Access denied') expect(html).not.toContain('Unable to render notebook') }) + + it('shows the changed-on-disk banner above a dirty edit tab', () => { + const activeFile = createOpenFile({ + id: '/repo/file.ts', + filePath: '/repo/file.ts', + relativePath: 'file.ts', + language: 'typescript', + isDirty: true, + externalMutation: 'changed' + }) + const html = renderToStaticMarkup( + + ) + + expect(html).toContain('role="alert"') + expect(html).toContain('changed on disk') + expect(html).toContain('Reload from Disk') + }) + + it('shows the changed-on-disk banner above a dirty unstaged diff without collapsing it', () => { + const activeFile = createOpenFile({ + id: 'diff:/repo/file.ts', + filePath: '/repo/file.ts', + relativePath: 'file.ts', + language: 'typescript', + mode: 'diff', + diffSource: 'unstaged', + isDirty: true, + externalMutation: 'changed' + }) + const html = renderToStaticMarkup( + + ) + + expect(html).toContain('role="alert"') + expect(html).toContain('changed on disk') + // Why: the diff-mode wrapper must give the viewer its height back — a + // flex-1-only wrapper collapsed the DiffViewer to 0px (found live). + expect(html).toContain('flex h-full min-h-0 flex-col') + }) + + it('shows the changed-on-disk banner on a dirty markdown diff in preview mode', () => { + const activeFile = createOpenFile({ + id: 'diff:/repo/notes.md', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + language: 'markdown', + mode: 'diff', + diffSource: 'unstaged', + isDirty: true, + externalMutation: 'changed' + }) + const html = renderToStaticMarkup( + + ) + + expect(html).toContain('role="alert"') + expect(html).toContain('changed on disk') + expect(html).toContain('Previewing the modified version of this diff') + }) }) diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index ed813e0ec..76996c0ac 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -33,6 +33,7 @@ import { import { getDiffContentSignature } from './diff-content-signature' import { translate } from '@/i18n/i18n' import { CheckRunDetailsPanel } from './CheckRunDetailsPanel' +import { ExternalFileChangeBanner } from './ExternalFileChangeBanner' const MonacoEditor = lazy(() => import('./MonacoEditor')) const DiffViewer = lazy(() => import('./DiffViewer')) @@ -139,7 +140,7 @@ export function EditorContent({ handleDirtyStateHint, handleSave, handleSaveForFile, - reloadFileContent + reloadContent }: { activeFile: OpenFile viewStateScopeId: string @@ -166,7 +167,7 @@ export function EditorContent({ handleDirtyStateHint: (dirty: boolean) => void handleSave: (content: string) => Promise handleSaveForFile: (file: OpenFile, content: string) => Promise - reloadFileContent: (file: OpenFile) => void + reloadContent: (file: OpenFile) => void }): React.JSX.Element { const editorViewStateKey = viewStateScopeId === activeFile.id @@ -505,10 +506,7 @@ export function EditorContent({ if (fc.loadError) { return (
- reloadFileContent(contentFile)} - /> + reloadContent(contentFile)} />
) } @@ -709,9 +707,7 @@ export function EditorContent({ ) } if (fc.loadError) { - return ( - reloadFileContent(activeFile)} /> - ) + return reloadContent(activeFile)} /> } if (fc.isBinary) { return ( @@ -758,9 +754,7 @@ export function EditorContent({ ) } if (fc.loadError) { - return ( - reloadFileContent(activeFile)} /> - ) + return reloadContent(activeFile)} /> } if (fc.isBinary) { if (fc.isImage) { @@ -777,8 +771,16 @@ export function EditorContent({ ) } + const externalChangeBanner = + activeFile.externalMutation === 'changed' ? ( + + ) : null if (isChangesMode) { - return ( + const changesView = ( ) + if (!externalChangeBanner) { + return changesView + } + return ( +
+ {externalChangeBanner} +
{changesView}
+
+ ) } return (
+ {externalChangeBanner} {activeFile.conflict && ( + ) : null if (isMarkdown && mdViewMode === 'preview' && dc.largeDiffRenderLimit?.limited !== true) { return (
+ {diffExternalChangeBanner}
{/* Why: a rendered markdown preview cannot express additions and deletions simultaneously, so preview mode intentionally shows the @@ -927,7 +950,7 @@ export function EditorContent({ const diffReloadNonce = activeFile.diffContentReloadNonce ?? 0 const originalModelKey = `${diffViewStateKey}:original:${getDiffContentSignature(dc.originalContent)}` const modifiedModelKey = `${diffViewStateKey}:modified:${getDiffContentSignature(dc.modifiedContent)}:${diffReloadNonce}` - return ( + const diffViewer = ( ) + // Why: editable unstaged diffs can hold unsaved edits, so they get the same + // changed-on-disk recovery banner as edit tabs; its reload refetches the + // diff body rather than plain file content. + if (activeFile.externalMutation !== 'changed') { + return diffViewer + } + return ( + // Why: h-full (not flex-1) — the diff-mode container is not a flex parent, + // so flex-1 resolves to zero height and collapses this wrapper. The inner + // div must itself be a flex column because DiffViewer's root sizes with + // flex-1 and collapses to 0px inside a block parent. +
+ {diffExternalChangeBanner} +
{diffViewer}
+
+ ) } // Why: a minimal read-only banner that shows the raw front-matter content diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index 8af1758a1..03fde8425 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -92,7 +92,7 @@ function EditorPanelInner({ activeFile.mode === 'edit' && canUseChangesModeForFile(activeFile) && editorViewMode[activeFile.id] === 'changes' - const { fileContents, diffContents, reloadFileContent } = useEditorPanelContentState({ + const { fileContents, diffContents, reloadContent } = useEditorPanelContentState({ activeFile, isChangesMode: requestedChangesMode, openFiles, @@ -377,7 +377,7 @@ function EditorPanelInner({ onDirtyStateHint={handleDirtyStateHint} onSave={handleSave} onSaveForFile={handleSaveForFile} - onReloadFileContent={reloadFileContent} + onReloadContent={reloadContent} onCloseMarkdownTableOfContents={() => setMarkdownTableOfContentsVisible(markdownDocumentStateFileId, false) } diff --git a/src/renderer/src/components/editor/EditorPanelShell.tsx b/src/renderer/src/components/editor/EditorPanelShell.tsx index 89d2a2a8a..de63cd78b 100644 --- a/src/renderer/src/components/editor/EditorPanelShell.tsx +++ b/src/renderer/src/components/editor/EditorPanelShell.tsx @@ -46,7 +46,7 @@ type EditorPanelShellProps = { onDirtyStateHint: (dirty: boolean) => void onSave: (content: string) => Promise onSaveForFile: (file: OpenFile, content: string) => Promise - onReloadFileContent: (file: OpenFile) => void + onReloadContent: (file: OpenFile) => void onCloseMarkdownTableOfContents: () => void onCloseRenameDialog: () => void onRenameConfirm: (newRelPath: string) => Promise @@ -86,7 +86,7 @@ export function EditorPanelShell({ onDirtyStateHint, onSave, onSaveForFile, - onReloadFileContent, + onReloadContent, onCloseMarkdownTableOfContents, onCloseRenameDialog, onRenameConfirm, @@ -152,7 +152,7 @@ export function EditorPanelShell({ handleDirtyStateHint={onDirtyStateHint} handleSave={onSave} handleSaveForFile={onSaveForFile} - reloadFileContent={onReloadFileContent} + reloadContent={onReloadContent} showMarkdownTableOfContents={showMarkdownTableOfContents} showMarkdownFrontmatter={markdownFrontmatterVisible} onCloseMarkdownTableOfContents={onCloseMarkdownTableOfContents} diff --git a/src/renderer/src/components/editor/ExternalFileChangeBanner.test.tsx b/src/renderer/src/components/editor/ExternalFileChangeBanner.test.tsx new file mode 100644 index 000000000..eae2909be --- /dev/null +++ b/src/renderer/src/components/editor/ExternalFileChangeBanner.test.tsx @@ -0,0 +1,231 @@ +import { renderToStaticMarkup } from 'react-dom/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' + +const toastMock = vi.hoisted(() => vi.fn()) +const readRuntimeFileContentMock = vi.hoisted(() => vi.fn()) +vi.mock('sonner', () => ({ + toast: toastMock +})) +vi.mock('@/store', () => ({ + useAppStore: { + getState: vi.fn() + } +})) +vi.mock('@/runtime/runtime-file-client', () => ({ + readRuntimeFileContent: readRuntimeFileContentMock +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + settingsForRuntimeOwner: () => null +})) +vi.mock('@/lib/connection-context', () => ({ + getConnectionIdForFile: () => undefined +})) + +import { + ExternalFileChangeBanner, + keepTabEditsOverExternalChange, + reloadTabContentFromDisk +} from './ExternalFileChangeBanner' +import { getDiskBaselineSignature } from './diff-content-signature' +import { useAppStore } from '@/store' + +const file = { + id: 'file-1', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + worktreeId: 'wt-1', + mode: 'edit', + isDirty: true, + externalMutation: 'changed' +} as OpenFile + +describe('ExternalFileChangeBanner', () => { + const clearEditorDraft = vi.fn() + const markFileDirty = vi.fn() + const setExternalMutation = vi.fn() + const setEditorDraft = vi.fn() + const setLastKnownDiskSignature = vi.fn() + + function mockStoreState(editorDrafts: Record, openFiles: OpenFile[] = [file]) { + vi.mocked(useAppStore.getState).mockReturnValue({ + clearEditorDraft, + markFileDirty, + setExternalMutation, + setEditorDraft, + setLastKnownDiskSignature, + editorDrafts, + openFiles, + settings: {} + } as never) + } + + beforeEach(() => { + vi.clearAllMocks() + readRuntimeFileContentMock.mockResolvedValue({ content: 'disk content', isBinary: false }) + mockStoreState({}) + }) + + it('renders the overwrite warning, all three actions, and an alert role', () => { + const html = renderToStaticMarkup( + + ) + expect(html).toContain('role="alert"') + expect(html).toContain('changed on disk') + expect(html).toContain('Saving will overwrite') + expect(html).toContain('Compare') + expect(html).toContain('Reload from Disk') + expect(html).toContain('Keep My Edits') + }) + + it('reload clears the draft, dirty flag, and mark before refetching content', () => { + mockStoreState({ 'file-1': 'unsaved text' }) + const calls: string[] = [] + clearEditorDraft.mockImplementation(() => calls.push('clearEditorDraft')) + markFileDirty.mockImplementation(() => calls.push('markFileDirty')) + setExternalMutation.mockImplementation(() => calls.push('setExternalMutation')) + const reloadContent = vi.fn(() => calls.push('reloadContent')) + + reloadTabContentFromDisk(file, reloadContent) + + expect(clearEditorDraft).toHaveBeenCalledWith('file-1') + expect(markFileDirty).toHaveBeenCalledWith('file-1', false) + expect(setExternalMutation).toHaveBeenCalledWith('file-1', null) + expect(reloadContent).toHaveBeenCalledWith(file) + // Why: the draft shadows loaded content (editBuffers ?? fileContents), so + // the refetch must come last or the stale unsaved text stays visible. + expect(calls).toEqual([ + 'clearEditorDraft', + 'markFileDirty', + 'setExternalMutation', + 'reloadContent' + ]) + }) + + it('reload offers an undo toast that restores the discarded draft and the conflict', () => { + mockStoreState({ 'file-1': 'discarded draft' }) + + reloadTabContentFromDisk(file, vi.fn()) + + expect(toastMock).toHaveBeenCalledTimes(1) + const options = toastMock.mock.calls[0][1] as { + action: { label: string; onClick: () => void } + } + vi.clearAllMocks() + // Why: after a real reload the tab is clean with no draft — the undo + // guard only restores over that untouched state. + mockStoreState({}, [{ ...file, isDirty: false, externalMutation: undefined } as OpenFile]) + + options.action.onClick() + + expect(setEditorDraft).toHaveBeenCalledWith('file-1', 'discarded draft') + expect(markFileDirty).toHaveBeenCalledWith('file-1', true) + // Why: disk still differs from the restored draft — the conflict (and its + // autosave suspension) must return with the edits. + expect(setExternalMutation).toHaveBeenCalledWith('file-1', 'changed') + }) + + it('undo is a no-op when the tab closed while the toast was up', () => { + mockStoreState({ 'file-1': 'discarded draft' }) + + reloadTabContentFromDisk(file, vi.fn()) + + const options = toastMock.mock.calls[0][1] as { + action: { label: string; onClick: () => void } + } + vi.clearAllMocks() + mockStoreState({}, []) + + options.action.onClick() + + expect(setEditorDraft).not.toHaveBeenCalled() + expect(markFileDirty).not.toHaveBeenCalled() + expect(setExternalMutation).not.toHaveBeenCalled() + }) + + it('undo is a no-op when the user typed or saved after the reload', () => { + mockStoreState({ 'file-1': 'discarded draft' }) + + reloadTabContentFromDisk(file, vi.fn()) + + const options = toastMock.mock.calls[0][1] as { + action: { label: string; onClick: () => void } + } + vi.clearAllMocks() + // Why: post-reload edits are newer intent than the discarded draft — + // undoing over them would be a second silent discard. + mockStoreState({ 'file-1': 'newer post-reload edits' }, [ + { ...file, isDirty: true, externalMutation: undefined } as OpenFile + ]) + + options.action.onClick() + + expect(setEditorDraft).not.toHaveBeenCalled() + expect(setExternalMutation).not.toHaveBeenCalled() + }) + + it('does not toast when there was no draft to restore', () => { + mockStoreState({}) + + reloadTabContentFromDisk(file, vi.fn()) + + expect(toastMock).not.toHaveBeenCalled() + }) + + it('undo restores the pre-reload disk signature with the draft', async () => { + // Why: the reload re-stamps the baseline to the new disk content; without + // restoring the old signature the restart scan sees disk == baseline and + // silently drops the conflict the undo just brought back. + mockStoreState({ 'file-1': 'discarded draft' }, [ + { ...file, lastKnownDiskSignature: 'pre-reload-signature' } as OpenFile + ]) + + reloadTabContentFromDisk(file, vi.fn()) + + const options = toastMock.mock.calls[0][1] as { + action: { label: string; onClick: () => void } + } + vi.clearAllMocks() + mockStoreState({}, [{ ...file, isDirty: false, externalMutation: undefined } as OpenFile]) + + options.action.onClick() + + expect(setLastKnownDiskSignature).toHaveBeenCalledWith('file-1', 'pre-reload-signature') + }) + + it('keep-my-edits clears the mark without touching the draft or dirty flag', () => { + keepTabEditsOverExternalChange(file) + + expect(setExternalMutation).toHaveBeenCalledWith('file-1', null) + expect(clearEditorDraft).not.toHaveBeenCalled() + expect(markFileDirty).not.toHaveBeenCalled() + }) + + it('keep-my-edits advances the disk baseline so the dismissal survives restart', async () => { + readRuntimeFileContentMock.mockResolvedValue({ content: 'agent content', isBinary: false }) + mockStoreState({}, [{ ...file, externalMutation: undefined } as OpenFile]) + + keepTabEditsOverExternalChange(file) + await Promise.resolve() + await Promise.resolve() + + expect(setLastKnownDiskSignature).toHaveBeenCalledWith( + 'file-1', + getDiskBaselineSignature('agent content') + ) + }) + + it('keep-my-edits does not stamp a baseline over a newer conflict', async () => { + // Why: if the file changed again before the read resolved, the fresh + // 'changed' mark owns the baseline — stamping would hide that conflict + // from the restart scan. + readRuntimeFileContentMock.mockResolvedValue({ content: 'even newer', isBinary: false }) + mockStoreState({}, [{ ...file, externalMutation: 'changed' } as OpenFile]) + + keepTabEditsOverExternalChange(file) + await Promise.resolve() + await Promise.resolve() + + expect(setLastKnownDiskSignature).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx b/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx new file mode 100644 index 000000000..d23421605 --- /dev/null +++ b/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx @@ -0,0 +1,185 @@ +import React, { useState } from 'react' +import { TriangleAlert } from 'lucide-react' +import { toast } from 'sonner' +import { Button } from '@/components/ui/button' +import { getConnectionIdForFile } from '@/lib/connection-context' +import { readRuntimeFileContent } from '@/runtime/runtime-file-client' +import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' +import { useAppStore } from '@/store' +import { translate } from '@/i18n/i18n' +import type { OpenFile } from '@/store/slices/editor' +import { ExternalFileChangeCompareDialog } from './ExternalFileChangeCompareDialog' +import { getDiskBaselineSignature } from './diff-content-signature' +import { trackExternalChangeConflictAction } from './editor-external-change-telemetry' + +// Why: when an external process (usually an agent) rewrites a file while the +// tab holds unsaved edits, the reload pipeline preserves the buffer and marks +// the tab externalMutation='changed' (issue #7265). This banner is the +// recovery path — without it the tab is silently stale until close/reopen and +// the next save clobbers the newer disk content unannounced. + +const RELOAD_UNDO_TOAST_DURATION_MS = 8_000 + +export function reloadTabContentFromDisk( + file: OpenFile, + reloadContent: (file: OpenFile) => void +): void { + const state = useAppStore.getState() + const discardedDraft = state.editorDrafts[file.id] + const discardedDiskSignature = state.openFiles.find( + (openFile) => openFile.id === file.id + )?.lastKnownDiskSignature + // Why: drop the draft before reloading — the buffer shadows loaded content + // (editBuffers ?? fileContents), so a reload alone would keep showing the + // stale unsaved text. + state.clearEditorDraft(file.id) + state.markFileDirty(file.id, false) + state.setExternalMutation(file.id, null) + reloadContent(file) + if (discardedDraft === undefined) { + return + } + // Why: on diff tabs the reload rotates the Monaco model, destroying the undo + // stack — without this toast a mistaken click is an unrecoverable discard. + toast( + translate('auto.components.editor.ExternalFileChangeBanner.5c02de9b31', 'Reloaded from disk'), + { + description: file.relativePath, + duration: RELOAD_UNDO_TOAST_DURATION_MS, + action: { + label: translate('auto.components.editor.ExternalFileChangeBanner.d1e830fa22', 'Undo'), + onClick: () => { + const current = useAppStore.getState() + const liveFile = current.openFiles.find((openFile) => openFile.id === file.id) + // Why: the tab may have closed while the toast was up; restoring a + // draft for a dead fileId would strand an orphan buffer. And if the + // user already typed after the reload (dirty), that newer work wins + // — undoing over it would be a second silent discard. isDirty is the + // signal: the editor content-sync repopulates editorDrafts with the + // reloaded content itself, so draft existence proves nothing. + if (!liveFile || liveFile.isDirty) { + return + } + current.setEditorDraft(file.id, discardedDraft) + current.markFileDirty(file.id, true) + // Why: the disk still differs from the restored draft, so the + // conflict (and its autosave suspension) must come back with it. + current.setExternalMutation(file.id, 'changed') + if (discardedDiskSignature !== undefined) { + // Why: the reload re-stamped the baseline to the new disk content; + // restoring the pre-reload signature with the draft keeps the + // restart scan re-deriving the conflict the undo just brought back. + current.setLastKnownDiskSignature(file.id, discardedDiskSignature) + } + trackExternalChangeConflictAction(file, 'undo_reload') + } + } + } + ) +} + +export function keepTabEditsOverExternalChange(file: OpenFile): void { + const state = useAppStore.getState() + state.setExternalMutation(file.id, null) + // Why: the dismissal must survive restart — without advancing the baseline + // to the current disk content, the restored-tab conflict scan re-derives + // the dismissed conflict from the stale signature on every launch. + // Best-effort: a failed read leaves the old signature, which can only + // re-surface the banner — never lose data. + void readRuntimeFileContent({ + settings: settingsForRuntimeOwner(state.settings, file.runtimeEnvironmentId), + filePath: file.filePath, + relativePath: file.relativePath, + worktreeId: file.worktreeId, + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined + }) + .then((result) => { + if (result.isBinary) { + return + } + const current = useAppStore.getState() + const liveFile = current.openFiles.find((openFile) => openFile.id === file.id) + // Why: only stamp while the dismissal still stands — a save or a newer + // conflict marked in the interim owns the baseline. + if (!liveFile || liveFile.externalMutation === 'changed') { + return + } + current.setLastKnownDiskSignature(file.id, getDiskBaselineSignature(result.content)) + }) + .catch(() => undefined) +} + +export function ExternalFileChangeBanner({ + file, + currentContent, + reloadContent +}: { + file: OpenFile + /** The tab's live buffer (draft if dirty) — what "Keep My Edits" keeps. */ + currentContent: string + /** Refetches the tab's content — file body for edit tabs, diff body for + * unstaged diff tabs. */ + reloadContent: (file: OpenFile) => void +}): React.JSX.Element { + const [compareOpen, setCompareOpen] = useState(false) + + const handleReload = (): void => { + trackExternalChangeConflictAction(file, 'reload') + reloadTabContentFromDisk(file, reloadContent) + } + const handleKeepEdits = (): void => { + trackExternalChangeConflictAction(file, 'keep') + keepTabEditsOverExternalChange(file) + } + const handleCompare = (): void => { + trackExternalChangeConflictAction(file, 'compare') + setCompareOpen(true) + } + + return ( + // Why: role=alert because the banner appears asynchronously (an agent + // rewrote the file) — screen readers must announce it unprompted. +
+
+
+ + {/* Why: wraps instead of truncating — the overwrite warning at the + end of the sentence is the part the user must not lose. */} + + {translate( + 'auto.components.editor.ExternalFileChangeBanner.7c41e90d12', + 'This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.' + )} + +
+
+ + + +
+
+ {compareOpen && ( + + )} +
+ ) +} diff --git a/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.test.tsx b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.test.tsx new file mode 100644 index 000000000..2a9060055 --- /dev/null +++ b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.test.tsx @@ -0,0 +1,142 @@ +// @vitest-environment happy-dom +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' + +const mocks = vi.hoisted(() => ({ + readRuntimeFileContent: vi.fn(), + getConnectionIdForFile: vi.fn(), + getState: vi.fn() +})) + +vi.mock('@/runtime/runtime-file-client', () => ({ + readRuntimeFileContent: mocks.readRuntimeFileContent +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + settingsForRuntimeOwner: () => null +})) +vi.mock('@/lib/connection-context', () => ({ + getConnectionIdForFile: mocks.getConnectionIdForFile +})) +vi.mock('@/store', () => ({ + useAppStore: { getState: mocks.getState } +})) +// Why: the lazy DiffViewer chunk cannot resolve under happy-dom; a stub that +// echoes its props pins the disk-left/buffer-right wiring instead. +vi.mock('@/lib/lazy-with-retry', () => ({ + lazyWithRetry: () => (props: { originalContent: string; modifiedContent: string }) => ( +
+ original:{props.originalContent}|modified:{props.modifiedContent} +
+ ) +})) + +import { ExternalFileChangeCompareDialog } from './ExternalFileChangeCompareDialog' + +const file = { + id: 'file-1', + filePath: '/repo/notes.ts', + relativePath: 'notes.ts', + worktreeId: 'wt-1', + mode: 'edit', + isDirty: true, + externalMutation: 'changed' +} as OpenFile + +describe('ExternalFileChangeCompareDialog', () => { + let root: Root | null = null + let container: HTMLElement | null = null + + beforeEach(() => { + mocks.readRuntimeFileContent.mockReset() + mocks.getConnectionIdForFile.mockReset() + mocks.getConnectionIdForFile.mockReturnValue(undefined) + mocks.getState.mockReturnValue({ settings: null }) + container = document.createElement('div') + document.body.appendChild(container) + }) + + afterEach(async () => { + await act(async () => { + root?.unmount() + }) + container?.remove() + document.body.innerHTML = '' + }) + + async function render(element: React.JSX.Element): Promise { + await act(async () => { + root = createRoot(container!) + root.render(element) + }) + } + + it('shows the disk version left and the buffer right once the read resolves', async () => { + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'disk version', isBinary: false }) + + await render( + + ) + + expect(document.body.textContent).toContain('File changed on disk') + expect(document.body.textContent).toContain('original:disk version|modified:buffer version') + expect(document.body.textContent).toContain('Reload from Disk') + expect(document.body.textContent).toContain('Keep My Edits') + }) + + it('surfaces a read failure instead of a blank comparison', async () => { + mocks.readRuntimeFileContent.mockRejectedValue(new Error('transport down')) + + await render( + + ) + + expect(document.body.textContent).toContain('Could not read the file from disk') + expect(document.body.textContent).toContain('transport down') + }) + + it('wires the footer actions and closes the dialog around them', async () => { + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'disk version', isBinary: false }) + const onReload = vi.fn() + const onKeepEdits = vi.fn() + const onOpenChange = vi.fn() + + await render( + + ) + + const buttons = Array.from(document.body.querySelectorAll('button')) + await act(async () => { + buttons.find((b) => b.textContent === 'Reload from Disk')?.click() + }) + expect(onOpenChange).toHaveBeenCalledWith(false) + expect(onReload).toHaveBeenCalledTimes(1) + + await act(async () => { + buttons.find((b) => b.textContent === 'Keep My Edits')?.click() + }) + expect(onKeepEdits).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx new file mode 100644 index 000000000..eb6611abe --- /dev/null +++ b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx @@ -0,0 +1,191 @@ +import React, { Suspense, useEffect, useState } from 'react' +import { Loader2 } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' +import { lazyWithRetry as lazy } from '@/lib/lazy-with-retry' +import { getConnectionIdForFile } from '@/lib/connection-context' +import { detectLanguage } from '@/lib/language-detect' +import { readRuntimeFileContent } from '@/runtime/runtime-file-client' +import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' +import { useAppStore } from '@/store' +import type { OpenFile } from '@/store/slices/editor' +import { translate } from '@/i18n/i18n' + +const DiffViewer = lazy(() => import('./DiffViewer')) + +type DiskReadState = + | { kind: 'loading' } + | { kind: 'error'; message: string } + | { kind: 'binary' } + | { kind: 'ready'; content: string } + +// Why: choosing between "Reload from Disk" and "Keep My Edits" blind is the +// sharpest edge of the changed-on-disk banner — this dialog shows exactly +// what each choice discards before the user commits (issue #7265 follow-up). +export function ExternalFileChangeCompareDialog({ + file, + currentContent, + open, + onOpenChange, + onReload, + onKeepEdits +}: { + file: OpenFile + /** The tab's live buffer — the unsaved edits the user would keep. */ + currentContent: string + open: boolean + onOpenChange: (open: boolean) => void + onReload: () => void + onKeepEdits: () => void +}): React.JSX.Element { + const [diskState, setDiskState] = useState({ kind: 'loading' }) + + useEffect(() => { + if (!open) { + return + } + let cancelled = false + setDiskState({ kind: 'loading' }) + // Why: read at open time — the banner can be minutes old and the agent + // may have written again since; the comparison must show current disk. + void readRuntimeFileContent({ + settings: settingsForRuntimeOwner(useAppStore.getState().settings, file.runtimeEnvironmentId), + filePath: file.filePath, + relativePath: file.relativePath, + worktreeId: file.worktreeId, + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined + }) + .then((result) => { + if (cancelled) { + return + } + setDiskState( + result.isBinary ? { kind: 'binary' } : { kind: 'ready', content: result.content } + ) + }) + .catch((err: unknown) => { + if (cancelled) { + return + } + setDiskState({ + kind: 'error', + message: err instanceof Error ? err.message : String(err) + }) + }) + return () => { + cancelled = true + } + }, [open, file.filePath, file.relativePath, file.worktreeId, file.runtimeEnvironmentId]) + + const language = detectLanguage(file.relativePath) + + return ( + + + + + {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.4b8de20a11', + 'File changed on disk' + )} + + + {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.90cc31e4d7', + 'Disk version on the left, your unsaved edits on the right.' + )} + + +
+ {diskState.kind === 'loading' ? ( +
+ + {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.8fe30ab254', + 'Reading file from disk...' + )} +
+ ) : diskState.kind === 'error' ? ( +
+ {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.e2b1cd0393', + 'Could not read the file from disk: {{value0}}', + { value0: diskState.message } + )} +
+ ) : diskState.kind === 'binary' ? ( +
+ {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.b6cf20d514', + 'The file on disk is binary — no text comparison available.' + )} +
+ ) : ( + + + {translate( + 'auto.components.editor.ExternalFileChangeCompareDialog.2c8f1e07b9', + 'Loading comparison...' + )} +
+ } + > +
+ +
+ + )} +
+ + + + + + + ) +} diff --git a/src/renderer/src/components/editor/diff-content-signature.ts b/src/renderer/src/components/editor/diff-content-signature.ts index 927d9711d..ba5bc26fc 100644 --- a/src/renderer/src/components/editor/diff-content-signature.ts +++ b/src/renderer/src/components/editor/diff-content-signature.ts @@ -9,3 +9,19 @@ export function getDiffContentSignature(content: string): string { } return (hash >>> 0).toString(16) } + +// Why: the disk baseline guards against silently overwriting external writes +// (issue #7265), where a hash collision means a missed conflict — so it gets +// two independent FNV lanes plus the length instead of the single 32-bit lane +// that suffices for cosmetic model rotation above. +export function getDiskBaselineSignature(content: string): string { + let hashA = 2166136261 + let hashB = 84696351 + for (let i = 0; i < content.length; i += 1) { + const code = content.charCodeAt(i) + hashA ^= code + hashA = Math.imul(hashA, 16777619) + hashB = Math.imul(hashB ^ code, 1099511627) + } + return `${(hashA >>> 0).toString(16)}-${(hashB >>> 0).toString(16)}-${content.length.toString(16)}` +} diff --git a/src/renderer/src/components/editor/editor-autosave-conflict-flow.test.ts b/src/renderer/src/components/editor/editor-autosave-conflict-flow.test.ts new file mode 100644 index 000000000..753836b65 --- /dev/null +++ b/src/renderer/src/components/editor/editor-autosave-conflict-flow.test.ts @@ -0,0 +1,191 @@ +// Why: the changed-on-disk conflict flow (issue #7265) — draft preservation, +// echo-aware backstop marking, autosave suspension, and baseline settling — +// in the headless autosave controller. Split from +// editor-autosave-controller.test.ts to stay under max-lines. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, requestEditorFileSave } from './editor-autosave' +import { attachEditorAutosaveController } from './editor-autosave-controller' +import { __clearSelfWriteRegistryForTests, recordSelfWrite } from './editor-self-write-registry' +import { createEditorStore, stubEditorWindow } from './editor-autosave-controller-test-fixture' + +const mocks = vi.hoisted(() => ({ + getConnectionIdForFile: vi.fn() +})) + +vi.mock('@/lib/connection-context', () => ({ + getConnectionIdForFile: mocks.getConnectionIdForFile +})) + +function openDirtyFile(store: ReturnType, draft = 'unsaved edit'): void { + store.getState().openFile({ + filePath: '/repo/file.ts', + relativePath: 'file.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setEditorDraft('/repo/file.ts', draft) + store.getState().markFileDirty('/repo/file.ts', true) +} + +function dispatchExternalChange(): void { + window.dispatchEvent( + new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { + detail: { worktreeId: 'wt-1', worktreePath: '/repo', relativePath: 'file.ts' } + }) + ) +} + +describe('editor autosave changed-on-disk conflict flow', () => { + beforeEach(() => { + vi.useFakeTimers() + mocks.getConnectionIdForFile.mockReset() + mocks.getConnectionIdForFile.mockReturnValue(undefined) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + __clearSelfWriteRegistryForTests() + }) + + it('preserves dirty drafts and marks the tab changed-on-disk on external file change', () => { + stubEditorWindow() + const store = createEditorStore() + openDirtyFile(store) + + const cleanup = attachEditorAutosaveController(store) + try { + dispatchExternalChange() + + const file = store.getState().openFiles[0] + expect(file?.isDirty).toBe(true) + expect(file?.externalMutation).toBe('changed') + expect(store.getState().editorDrafts['/repo/file.ts']).toBe('unsaved edit') + } finally { + cleanup() + } + }) + + it('clears drafts and a stale changed-on-disk mark for clean tabs on external file change', () => { + stubEditorWindow() + const store = createEditorStore() + store.getState().openFile({ + filePath: '/repo/file.ts', + relativePath: 'file.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setExternalMutation('/repo/file.ts', 'changed') + + const cleanup = attachEditorAutosaveController(store) + try { + dispatchExternalChange() + + const file = store.getState().openFiles[0] + expect(file?.isDirty).toBe(false) + expect(file?.externalMutation).toBeUndefined() + } finally { + cleanup() + } + }) + + it('does not backstop-mark a dirty tab for the echo of its own save', () => { + stubEditorWindow() + const store = createEditorStore() + openDirtyFile(store, 'typed during save') + // Why: the combined-Changes reload notification routes through the + // controller for the saved path — a fresh self-write stamp means the + // event is Orca's own echo, not an external change. + recordSelfWrite('/repo/file.ts', 'orca save') + + const cleanup = attachEditorAutosaveController(store) + try { + dispatchExternalChange() + + const file = store.getState().openFiles[0] + expect(file?.externalMutation).toBeUndefined() + expect(file?.isDirty).toBe(true) + expect(store.getState().editorDrafts['/repo/file.ts']).toBe('typed during save') + } finally { + cleanup() + } + }) + + it('suspends autosave while a tab is marked changed-on-disk and resumes when cleared', async () => { + const writeFile = stubEditorWindow() + const store = createEditorStore() + openDirtyFile(store, 'user edit') + store.getState().setExternalMutation('/repo/file.ts', 'changed') + + const cleanup = attachEditorAutosaveController(store) + try { + await vi.advanceTimersByTimeAsync(1500) + expect(writeFile).not.toHaveBeenCalled() + + // Keep My Edits clears the mark — autosave resumes and overwrites. + store.getState().setExternalMutation('/repo/file.ts', null) + await vi.advanceTimersByTimeAsync(1500) + expect(writeFile).toHaveBeenCalledWith({ + filePath: '/repo/file.ts', + content: 'user edit' + }) + } finally { + cleanup() + } + }) + + it('suspends autosave while a restored tab awaits disk baseline verification', async () => { + const writeFile = stubEditorWindow() + const store = createEditorStore() + openDirtyFile(store, 'restored draft') + // Why: mimic hydration — the scan has not yet compared disk against the + // persisted baseline, so autosave must hold off (a slow remote read must + // not lose a race to this timer). + store.setState({ + openFiles: store + .getState() + .openFiles.map((f) => + f.id === '/repo/file.ts' ? { ...f, pendingDiskBaselineVerification: true } : f + ) + } as never) + + const cleanup = attachEditorAutosaveController(store) + try { + await vi.advanceTimersByTimeAsync(1500) + expect(writeFile).not.toHaveBeenCalled() + + store.getState().clearPendingDiskBaselineVerification('/repo/file.ts') + await vi.advanceTimersByTimeAsync(1500) + expect(writeFile).toHaveBeenCalledWith({ + filePath: '/repo/file.ts', + content: 'restored draft' + }) + } finally { + cleanup() + } + }) + + it('clears the changed-on-disk mark after a successful save', async () => { + const writeFile = stubEditorWindow() + const store = createEditorStore() + openDirtyFile(store, 'user version') + store.getState().setExternalMutation('/repo/file.ts', 'changed') + + const cleanup = attachEditorAutosaveController(store) + try { + await requestEditorFileSave({ fileId: '/repo/file.ts' }) + + expect(writeFile).toHaveBeenCalledWith({ + filePath: '/repo/file.ts', + content: 'user version' + }) + const file = store.getState().openFiles[0] + expect(file?.isDirty).toBe(false) + expect(file?.externalMutation).toBeUndefined() + } finally { + cleanup() + } + }) +}) diff --git a/src/renderer/src/components/editor/editor-autosave-controller-test-fixture.ts b/src/renderer/src/components/editor/editor-autosave-controller-test-fixture.ts new file mode 100644 index 000000000..6ecf88409 --- /dev/null +++ b/src/renderer/src/components/editor/editor-autosave-controller-test-fixture.ts @@ -0,0 +1,52 @@ +// Why: shared rig for autosave-controller suites — the controller needs a +// real editor store slice plus a window stub (event target, timers, fs +// bridge), and duplicating that per test file bloats suites past max-lines. +import { vi } from 'vitest' +import { createStore, type StoreApi } from 'zustand/vanilla' +import { createEditorSlice } from '@/store/slices/editor' +import type { AppState } from '@/store' + +export type EditorWindowStub = { + addEventListener: Window['addEventListener'] + removeEventListener: Window['removeEventListener'] + dispatchEvent: Window['dispatchEvent'] + setTimeout: Window['setTimeout'] + clearTimeout: Window['clearTimeout'] + api: { + fs: { + writeFile: ReturnType + } + } +} + +/** Stubs the global window with an isolated event target and fs bridge; + * returns the writeFile mock for assertions. */ +export function stubEditorWindow(): ReturnType { + const writeFile = vi.fn().mockResolvedValue(undefined) + const eventTarget = new EventTarget() + vi.stubGlobal('window', { + addEventListener: eventTarget.addEventListener.bind(eventTarget), + removeEventListener: eventTarget.removeEventListener.bind(eventTarget), + dispatchEvent: eventTarget.dispatchEvent.bind(eventTarget), + setTimeout: globalThis.setTimeout.bind(globalThis), + clearTimeout: globalThis.clearTimeout.bind(globalThis), + api: { + fs: { + writeFile + } + } + } satisfies EditorWindowStub) + return writeFile +} + +export function createEditorStore(): StoreApi { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return createStore()((...args: any[]) => ({ + activeWorktreeId: 'wt-1', + settings: { + editorAutoSave: true, + editorAutoSaveDelayMs: 1000 + }, + ...createEditorSlice(...(args as Parameters)) + })) as unknown as StoreApi +} diff --git a/src/renderer/src/components/editor/editor-autosave-controller.ts b/src/renderer/src/components/editor/editor-autosave-controller.ts index 23de3fecd..7c6f043bd 100644 --- a/src/renderer/src/components/editor/editor-autosave-controller.ts +++ b/src/renderer/src/components/editor/editor-autosave-controller.ts @@ -16,6 +16,7 @@ import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' import { canAutoSaveOpenFile, getOpenFilesForExternalFileChange, + isAutosaveSuspendedForFile, normalizeAutoSaveDelayMs, ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, ORCA_EDITOR_FILE_SAVED_EVENT, @@ -27,8 +28,16 @@ import { type EditorSaveFileDetail, type EditorSaveQuiesceDetail } from './editor-autosave' +import { markFileChangedOnDisk } from './editor-changed-on-disk-mark' import { flushPendingEditorChange } from './editor-pending-flush' -import { clearSelfWrite, recordSelfWrite } from './editor-self-write-registry' +import { + clearSelfWrite, + hasRecentSelfWrite, + recordSelfWrite, + SELF_WRITE_REMOTE_TTL_MS +} from './editor-self-write-registry' +import { getDiskBaselineSignature } from './diff-content-signature' +import { trackExternalChangeConflictAction } from './editor-external-change-telemetry' import { autosaveSubscriberInputsEqual, getAutosaveSubscriberInputs, @@ -62,7 +71,11 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { saveGeneration.set(fileId, (saveGeneration.get(fileId) ?? 0) + 1) } - const queueSave = (file: OpenFile, fallbackContent: string): Promise => { + const queueSave = ( + file: OpenFile, + fallbackContent: string, + trigger: 'autosave' | 'user' = 'user' + ): Promise => { clearAutoSaveTimer(file.id) const queuedGeneration = saveGeneration.get(file.id) ?? 0 @@ -80,6 +93,12 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { return } + // Why: explicit user saves proceed even while suspended (the banner + // warned) and clear both suspension flags below. + if (trigger === 'autosave' && isAutosaveSuspendedForFile(liveFile)) { + return + } + const contentToSave = state.editorDrafts[file.id] ?? fallbackContent const connectionId = getConnectionIdForFile(liveFile.worktreeId, liveFile.filePath) ?? undefined @@ -91,7 +110,14 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { // round-tripping back into a setContent that jumps the cursor to the // end (and, under round-trip drift, can drop keystrokes typed in the // debounce window). See editor-self-write-registry. - recordSelfWrite(liveFile.filePath, contentToSave, liveFile.runtimeEnvironmentId) + recordSelfWrite( + liveFile.filePath, + contentToSave, + liveFile.runtimeEnvironmentId, + connectionId || liveFile.runtimeEnvironmentId?.trim() + ? SELF_WRITE_REMOTE_TTL_MS + : undefined + ) try { await writeRuntimeFile( { @@ -122,6 +148,21 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { if (!stillDirty) { nextState.clearEditorDraft(file.id) } + // Why: disk now holds contentToSave — future edits baseline on it, and + // a restore must not flag our own save as an external change. An + // explicit save also settles any pending baseline verification: the + // user chose to write, so there is nothing left to verify against. + nextState.setLastKnownDiskSignature(file.id, getDiskBaselineSignature(contentToSave)) + nextState.clearPendingDiskBaselineVerification(file.id) + // Why: the write just made disk match the buffer, resolving any + // changed-on-disk conflict in favor of the user's content. The banner + // warned before this point; keeping the mark would show a stale + // conflict for a file that no longer diverges. + const savedFile = nextState.openFiles.find((openFile) => openFile.id === file.id) + if (savedFile?.externalMutation === 'changed') { + trackExternalChangeConflictAction(savedFile, 'save_overwrite') + nextState.setExternalMutation(file.id, null) + } window.dispatchEvent( new CustomEvent(ORCA_EDITOR_FILE_SAVED_EVENT, { @@ -171,6 +212,9 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { file && file.isDirty && canAutoSaveOpenFile(file) && + // Why: suspension holds until the user picks a side via the banner + // (or saves manually) — see the queueSave guard. + !isAutosaveSuspendedForFile(file) && draft !== undefined if (!shouldKeepTimer) { clearAutoSaveTimer(fileId) @@ -184,7 +228,12 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { const autoSaveDelayMs = normalizeAutoSaveDelayMs(state.settings.editorAutoSaveDelayMs) for (const file of state.openFiles) { const draft = state.editorDrafts[file.id] - if (!file.isDirty || draft === undefined || !canAutoSaveOpenFile(file)) { + if ( + !file.isDirty || + draft === undefined || + !canAutoSaveOpenFile(file) || + isAutosaveSuspendedForFile(file) + ) { clearAutoSaveTimer(file.id) continue } @@ -198,7 +247,7 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { const timerId = window.setTimeout(() => { autoSaveTimers.delete(file.id) autoSaveScheduledContent.delete(file.id) - void queueSave(file, draft) + void queueSave(file, draft, 'autosave') }, autoSaveDelayMs) autoSaveTimers.set(file.id, timerId) } @@ -377,12 +426,35 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { return } + // Why: dirty files must keep their draft — destroying unsaved edits on an + // external write is the data-loss half of issue #7265. Mark them + // changed-on-disk instead (backstop for tabs that became dirty during the + // notify debounce; the watch hook marks the ones dirty at event time). + const reloadingFiles = matchingFiles.filter((file) => !file.isDirty) for (const file of matchingFiles) { + if (file.isDirty) { + // Why: the self-write check keeps this backstop from marking on the + // echo of Orca's own save (the combined-Changes reload notification + // routes through here and would otherwise bypass the watch hook's + // echo verification). + if (!hasRecentSelfWrite(file.filePath, file.runtimeEnvironmentId)) { + markFileChangedOnDisk(state, file, { + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined, + origin: 'live' + }) + } + continue + } clearAutoSaveTimer(file.id) bumpSaveGeneration(file.id) state.markFileDirty(file.id, false) + // Why: this file is about to reload fresh disk content, so a stale + // changed-on-disk mark (set while it was dirty) is resolved. + if (file.externalMutation === 'changed') { + state.setExternalMutation(file.id, null) + } } - state.clearEditorDrafts(matchingFiles.map((file) => file.id)) + state.clearEditorDrafts(reloadingFiles.map((file) => file.id)) } // Why: the root store subscriber fires for every terminal title/focus tick. diff --git a/src/renderer/src/components/editor/editor-autosave.ts b/src/renderer/src/components/editor/editor-autosave.ts index 09d0ae866..c36f91e8a 100644 --- a/src/renderer/src/components/editor/editor-autosave.ts +++ b/src/renderer/src/components/editor/editor-autosave.ts @@ -77,6 +77,16 @@ export function canAutoSaveOpenFile(file: OpenFile): boolean { return file.mode === 'edit' || (file.mode === 'diff' && file.diffSource === 'unstaged') } +// Why: autosave must not resolve a changed-on-disk conflict by overwriting +// the newer external content, nor write over a restored tab whose disk +// baseline is still unverified (the conflict may simply not be marked YET). +// One predicate so the save-queue gate and the timer scheduler cannot drift. +export function isAutosaveSuspendedForFile( + file: Pick +): boolean { + return file.externalMutation === 'changed' || file.pendingDiskBaselineVerification === true +} + export function normalizeAutoSaveDelayMs(value: unknown): number { // Why: settings are persisted locally and can be missing or hand-edited. // Clamp the delay at the write site so autosave never degenerates into an @@ -176,6 +186,9 @@ export function requestEditorFileClose(fileId: string): void { ) } +// CONTRACT: this event fires even when some tabs of the path are dirty — +// every consumer MUST skip dirty files per-file. Reloading a dirty tab's +// content destroys its unsaved draft (the data-loss half of issue #7265). export function notifyEditorExternalFileChange(target: EditorPathMutationTarget): void { window.dispatchEvent( new CustomEvent(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, { diff --git a/src/renderer/src/components/editor/editor-changed-on-disk-mark.ts b/src/renderer/src/components/editor/editor-changed-on-disk-mark.ts new file mode 100644 index 000000000..06e726bd2 --- /dev/null +++ b/src/renderer/src/components/editor/editor-changed-on-disk-mark.ts @@ -0,0 +1,26 @@ +import type { OpenFile } from '@/store/slices/editor' +import { canAutoSaveOpenFile } from './editor-autosave' +import { trackExternalChangeConflictShown } from './editor-external-change-telemetry' + +type ChangedOnDiskMarkState = { + setExternalMutation: (fileId: string, mutation: 'deleted' | 'renamed' | 'changed' | null) => void +} + +// Why: the changed-on-disk mark has one rule — dirty, banner-capable tab, +// telemetry only on the first surfacing — but three writers (the watch hook +// at fs-event time, the autosave controller's notification backstop, and the +// restored-tab conflict scan). Centralizing keeps the rule from drifting; +// each writer keeps its own echo/eligibility guard at the call site. +export function markFileChangedOnDisk( + state: ChangedOnDiskMarkState, + file: OpenFile, + options: { connectionId: string | undefined; origin: 'live' | 'restore' } +): void { + if (!file.isDirty || !canAutoSaveOpenFile(file)) { + return + } + if (file.externalMutation !== 'changed') { + trackExternalChangeConflictShown(file, options) + } + state.setExternalMutation(file.id, 'changed') +} diff --git a/src/renderer/src/components/editor/editor-external-change-telemetry.ts b/src/renderer/src/components/editor/editor-external-change-telemetry.ts new file mode 100644 index 000000000..f9752147c --- /dev/null +++ b/src/renderer/src/components/editor/editor-external-change-telemetry.ts @@ -0,0 +1,62 @@ +// Why: one place derives the path-free analytics shape for the changed-on-disk +// conflict flow (issue #7265), so the three marking sites and the banner +// actions cannot drift on enum values. Measures false-banner rates per +// transport and which resolution users actually pick. +import { track } from '@/lib/telemetry' +import { getConnectionIdForFile } from '@/lib/connection-context' +import type { OpenFile } from '@/store/slices/editor' + +type ConflictSurface = 'edit' | 'unstaged-diff' +type ConflictTransport = 'local' | 'ssh' | 'runtime' + +export type ExternalChangeConflictAction = + | 'reload' + | 'keep' + | 'compare' + | 'undo_reload' + | 'save_overwrite' + +function conflictSurface(file: Pick): ConflictSurface { + return file.mode === 'edit' ? 'edit' : 'unstaged-diff' +} + +export function conflictTransport( + connectionId: string | undefined, + runtimeEnvironmentId: string | null | undefined +): ConflictTransport { + if (connectionId) { + return 'ssh' + } + if (runtimeEnvironmentId?.trim()) { + return 'runtime' + } + return 'local' +} + +export function trackExternalChangeConflictShown( + file: Pick, + options: { connectionId: string | undefined; origin: 'live' | 'restore' } +): void { + track('editor_external_change_conflict_shown', { + surface: conflictSurface(file), + transport: conflictTransport(options.connectionId, file.runtimeEnvironmentId), + origin: options.origin + }) +} + +export function trackExternalChangeConflictAction( + file: Pick, + action: ExternalChangeConflictAction +): void { + track('editor_external_change_conflict_action', { + action, + surface: conflictSurface(file), + // Why: shown-vs-action cross-tabs per transport are the point of the + // metric — false-banner detection needs to see WHICH transports' banners + // users dismiss versus act on. + transport: conflictTransport( + getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined, + file.runtimeEnvironmentId + ) + }) +} diff --git a/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.test.ts b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.test.ts new file mode 100644 index 000000000..8c50887a0 --- /dev/null +++ b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.test.ts @@ -0,0 +1,273 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createStore, type StoreApi } from 'zustand/vanilla' +import { createEditorSlice } from '@/store/slices/editor' +import type { AppState } from '@/store' +import { attachRestoredTabConflictScan } from './editor-restored-tab-conflict-scan' +import { getDiskBaselineSignature } from './diff-content-signature' + +const mocks = vi.hoisted(() => ({ + readRuntimeFileContent: vi.fn(), + getConnectionIdForFile: vi.fn(), + pathExists: vi.fn() +})) + +vi.mock('@/runtime/runtime-file-client', () => ({ + readRuntimeFileContent: mocks.readRuntimeFileContent +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + settingsForRuntimeOwner: () => null +})) +vi.mock('@/lib/connection-context', () => ({ + getConnectionIdForFile: mocks.getConnectionIdForFile +})) + +function createEditorStore(): StoreApi { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return createStore()((...args: any[]) => ({ + settings: {}, + ...createEditorSlice(...(args as Parameters)) + })) as unknown as StoreApi +} + +function openRestoredDirtyTab( + store: StoreApi, + filePath: string, + baselineContent: string +): void { + store.getState().openFile({ + filePath, + relativePath: filePath.slice(1), + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setEditorDraft(filePath, 'restored draft') + store.getState().markFileDirty(filePath, true) + store.getState().setLastKnownDiskSignature(filePath, getDiskBaselineSignature(baselineContent)) + // Why: hydration flags restored dirty tabs for verification; tests mimic it. + store.setState({ + openFiles: store + .getState() + .openFiles.map((f) => + f.id === filePath ? { ...f, pendingDiskBaselineVerification: true } : f + ) + } as never) +} + +describe('attachRestoredTabConflictScan', () => { + beforeEach(() => { + vi.useFakeTimers() + mocks.readRuntimeFileContent.mockReset() + mocks.getConnectionIdForFile.mockReset() + mocks.getConnectionIdForFile.mockReturnValue(undefined) + mocks.pathExists.mockReset() + mocks.pathExists.mockResolvedValue(true) + vi.stubGlobal('window', { api: { fs: { pathExists: mocks.pathExists } } }) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('marks a restored dirty tab whose file changed while the app was closed', async () => { + mocks.readRuntimeFileContent.mockResolvedValue({ + content: 'agent rewrote this offline', + isBinary: false + }) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/file.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + expect(store.getState().openFiles[0]?.externalMutation).toBe('changed') + } finally { + detach() + } + }) + + it('leaves a restored dirty tab unmarked when disk still matches its baseline', async () => { + mocks.readRuntimeFileContent.mockResolvedValue({ + content: 'original baseline', + isBinary: false + }) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/file.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + expect(store.getState().openFiles[0]?.externalMutation).toBeUndefined() + expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(1) + } finally { + detach() + } + }) + + it('does not read files for clean tabs or tabs without a baseline', async () => { + const store = createEditorStore() + store.getState().openFile({ + filePath: '/repo/clean.ts', + relativePath: 'clean.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().openFile({ + filePath: '/repo/dirty-no-baseline.ts', + relativePath: 'dirty-no-baseline.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setEditorDraft('/repo/dirty-no-baseline.ts', 'draft') + store.getState().markFileDirty('/repo/dirty-no-baseline.ts', true) + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + expect(mocks.readRuntimeFileContent).not.toHaveBeenCalled() + } finally { + detach() + } + }) + + it('retries a failed read and marks once the file becomes readable', async () => { + // Why: SSH/runtime connections come up after launch; the first reads fail. + mocks.readRuntimeFileContent + .mockRejectedValueOnce(new Error('connection not ready')) + .mockResolvedValue({ content: 'agent rewrote this offline', isBinary: false }) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/file.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + expect(store.getState().openFiles[0]?.externalMutation).toBeUndefined() + await vi.advanceTimersByTimeAsync(2_100) + expect(store.getState().openFiles[0]?.externalMutation).toBe('changed') + } finally { + detach() + } + }) + + it('clears the pending-verification flag on both verification outcomes', async () => { + mocks.readRuntimeFileContent.mockResolvedValue({ + content: 'original baseline', + isBinary: false + }) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/match.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + // Why: the flag suspends autosave — leaving it set after a clean + // verification would strand the tab's autosave forever. + expect(store.getState().openFiles[0]?.pendingDiskBaselineVerification).toBeUndefined() + + mocks.readRuntimeFileContent.mockResolvedValue({ + content: 'agent rewrote this offline', + isBinary: false + }) + openRestoredDirtyTab(store, '/repo/mismatch.ts', 'original baseline') + await vi.advanceTimersByTimeAsync(10) + const mismatchTab = store.getState().openFiles.find((f) => f.id === '/repo/mismatch.ts') + expect(mismatchTab?.externalMutation).toBe('changed') + expect(mismatchTab?.pendingDiskBaselineVerification).toBeUndefined() + } finally { + detach() + } + }) + + it('does not re-verify live dirty tabs that were never flagged at hydration', async () => { + const store = createEditorStore() + store.getState().openFile({ + filePath: '/repo/live.ts', + relativePath: 'live.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setEditorDraft('/repo/live.ts', 'live edits') + store.getState().markFileDirty('/repo/live.ts', true) + store.getState().setLastKnownDiskSignature('/repo/live.ts', getDiskBaselineSignature('base')) + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + // Why: in-session drift is the live watcher's job; re-reading here would + // turn the scan into a poller and skew the 'restore' telemetry origin. + expect(mocks.readRuntimeFileContent).not.toHaveBeenCalled() + } finally { + detach() + } + }) + + it('resolves verification with a deleted mark when the file is definitively gone', async () => { + // Why: a file deleted while the app was closed can never verify — without + // a terminal state the retry loop keeps the tab's autosave silently + // suspended for the whole session. + mocks.readRuntimeFileContent.mockRejectedValue(new Error('ENOENT')) + mocks.pathExists.mockResolvedValue(false) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/deleted.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + const tab = store.getState().openFiles[0] + expect(tab?.pendingDiskBaselineVerification).toBeUndefined() + expect(tab?.externalMutation).toBe('deleted') + // Why: the verification is resolved — no further retries may fire. + await vi.advanceTimersByTimeAsync(60_000) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(1) + } finally { + detach() + } + }) + + it('keeps retrying with the suspension intact when the existence probe fails', async () => { + // Why: a transport that is down cannot disprove existence — lifting the + // suspension unverified would reopen the clobber window the moment the + // transport recovers. + mocks.readRuntimeFileContent.mockRejectedValue(new Error('connection not ready')) + mocks.pathExists.mockRejectedValue(new Error('connection not ready')) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/unreachable.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + await vi.advanceTimersByTimeAsync(10) + await vi.advanceTimersByTimeAsync(2_100) + const tab = store.getState().openFiles[0] + expect(tab?.pendingDiskBaselineVerification).toBe(true) + expect(tab?.externalMutation).toBeUndefined() + expect(mocks.readRuntimeFileContent.mock.calls.length).toBeGreaterThan(1) + } finally { + detach() + } + }) + + it('does not mark a tab that was saved while the read was in flight', async () => { + let resolveRead: (value: { content: string; isBinary: boolean }) => void = () => {} + mocks.readRuntimeFileContent.mockReturnValue( + new Promise((resolve) => { + resolveRead = resolve + }) + ) + const store = createEditorStore() + openRestoredDirtyTab(store, '/repo/file.ts', 'original baseline') + + const detach = attachRestoredTabConflictScan(store) + try { + store.getState().markFileDirty('/repo/file.ts', false) + resolveRead({ content: 'agent rewrote this offline', isBinary: false }) + await vi.advanceTimersByTimeAsync(10) + expect(store.getState().openFiles[0]?.externalMutation).toBeUndefined() + } finally { + detach() + } + }) +}) diff --git a/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts new file mode 100644 index 000000000..bc4029321 --- /dev/null +++ b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts @@ -0,0 +1,186 @@ +// Why: a dirty tab restored from a workspace session carries edits based on +// disk content that may have changed while the app was closed (an agent write, +// a sync tool). The in-memory changed-on-disk mark does not survive restarts, +// so without this scan a resumed autosave would silently overwrite that newer +// content (issue #7265 follow-up). The scan re-derives the conflict from +// ground truth: it reads each restored dirty tab's file and compares the disk +// signature against the persisted edit baseline. Autosave is hard-suspended +// for those tabs (pendingDiskBaselineVerification, set at hydration) until a +// verification resolves — otherwise the read would merely race the autosave +// timer and a slow remote read would lose. +import type { StoreApi } from 'zustand' +import type { AppState } from '@/store' +import type { OpenFile } from '@/store/slices/editor' +import { getConnectionIdForFile } from '@/lib/connection-context' +import { readRuntimeFileContent } from '@/runtime/runtime-file-client' +import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' +import { canAutoSaveOpenFile } from './editor-autosave' +import { getDiskBaselineSignature } from './diff-content-signature' +import { markFileChangedOnDisk } from './editor-changed-on-disk-mark' + +type AppStoreApi = Pick, 'getState' | 'subscribe'> + +// Why: SSH/runtime reads fail while the connection is still coming up after +// launch. Retry fast for the first minute, then keep probing slowly — +// giving up on a transport error would either strand the tab's autosave +// suspension or lift it unverified right as the transport comes back up. +// Only a definitive not-found (file deleted while the app was closed) ends +// the loop early; see probeFileMissing. +const VERIFY_RETRY_MS = 2_000 +const VERIFY_SLOW_RETRY_MS = 15_000 +const VERIFY_FAST_ATTEMPTS = 30 + +export function attachRestoredTabConflictScan(store: AppStoreApi): () => void { + // Why: dedupes in-flight verifications; the store's pending flag is the + // durable "needs verification" signal. + const inFlightFileIds = new Set() + const attemptsByFileId = new Map() + const retryTimers = new Set>() + let disposed = false + + // Why: distinguishes "file was deleted while the app was closed" (a + // definitive not-found) from a transport still coming up. Only local/SSH + // paths can be probed — for runtime-owned files window.api.fs would stat + // the client-local path and misreport a remote file as gone. + const probeFileMissing = async (file: OpenFile): Promise => { + const settings = settingsForRuntimeOwner(store.getState().settings, file.runtimeEnvironmentId) + if (settings?.activeRuntimeEnvironmentId?.trim()) { + return false + } + try { + const exists = await globalThis.window?.api?.fs?.pathExists?.({ + filePath: file.filePath, + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined + }) + return exists === false + } catch { + // Why: a failed probe can't disprove existence — keep retrying. + return false + } + } + + const verify = async (file: OpenFile): Promise => { + // Why: OpenFile ids are file paths — a marker left behind by an early + // exit would silently skip every future verification of a reopened + // same-path tab. Only a scheduled retry may keep the marker set. + let retryScheduled = false + try { + const state = store.getState() + const result = await readRuntimeFileContent({ + settings: settingsForRuntimeOwner(state.settings, file.runtimeEnvironmentId), + filePath: file.filePath, + relativePath: file.relativePath, + worktreeId: file.worktreeId, + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined + }) + if (disposed) { + return + } + const liveFile = store.getState().openFiles.find((f) => f.id === file.id) + if (!liveFile) { + return + } + // Why: verification resolved — lift the autosave suspension regardless + // of outcome. If a save raced the read, the save already re-baselined + // and cleared the flag itself; wasPending distinguishes that case. + const wasPending = liveFile.pendingDiskBaselineVerification === true + store.getState().clearPendingDiskBaselineVerification(file.id) + if ( + !wasPending || + result.isBinary || + !liveFile.isDirty || + liveFile.externalMutation === 'changed' + ) { + return + } + if (getDiskBaselineSignature(result.content) !== file.lastKnownDiskSignature) { + markFileChangedOnDisk(store.getState(), liveFile, { + connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined, + origin: 'restore' + }) + } + } catch { + if (disposed) { + return + } + if (await probeFileMissing(file)) { + if (disposed) { + return + } + // Why: a definitive not-found IS ground truth — there is no newer + // disk content for a save to clobber, so verification is resolved. + // Converge to the live delete affordance (tombstone mark) instead of + // retrying forever with the tab's autosave silently suspended. + const liveFile = store.getState().openFiles.find((f) => f.id === file.id) + if (!liveFile) { + return + } + const wasPending = liveFile.pendingDiskBaselineVerification === true + store.getState().clearPendingDiskBaselineVerification(file.id) + if (wasPending && liveFile.isDirty && liveFile.externalMutation !== 'changed') { + store.getState().setExternalMutation(file.id, 'deleted') + } + return + } + if (disposed) { + return + } + const attempts = (attemptsByFileId.get(file.id) ?? 0) + 1 + attemptsByFileId.set(file.id, attempts) + retryScheduled = true + const timer = setTimeout( + () => { + retryTimers.delete(timer) + inFlightFileIds.delete(file.id) + scan() + }, + attempts < VERIFY_FAST_ATTEMPTS ? VERIFY_RETRY_MS : VERIFY_SLOW_RETRY_MS + ) + retryTimers.add(timer) + } finally { + if (!retryScheduled) { + inFlightFileIds.delete(file.id) + } + } + } + + const scan = (): void => { + if (disposed) { + return + } + for (const file of store.getState().openFiles) { + if ( + !file.pendingDiskBaselineVerification || + !file.isDirty || + !file.lastKnownDiskSignature || + file.externalMutation === 'changed' || + !canAutoSaveOpenFile(file) || + inFlightFileIds.has(file.id) + ) { + continue + } + inFlightFileIds.add(file.id) + void verify(file) + } + } + + let previousOpenFiles = store.getState().openFiles + const unsubscribe = store.subscribe(() => { + const nextOpenFiles = store.getState().openFiles + if (nextOpenFiles === previousOpenFiles) { + return + } + previousOpenFiles = nextOpenFiles + scan() + }) + scan() + + return () => { + disposed = true + unsubscribe() + for (const timer of retryTimers) { + clearTimeout(timer) + } + retryTimers.clear() + } +} diff --git a/src/renderer/src/components/editor/editor-self-write-registry.test.ts b/src/renderer/src/components/editor/editor-self-write-registry.test.ts index d05a98a40..e2eacaca9 100644 --- a/src/renderer/src/components/editor/editor-self-write-registry.test.ts +++ b/src/renderer/src/components/editor/editor-self-write-registry.test.ts @@ -4,7 +4,8 @@ import { __getSelfWriteRegistrySizeForTests, clearSelfWrite, hasRecentSelfWrite, - recordSelfWrite + recordSelfWrite, + SELF_WRITE_REMOTE_TTL_MS } from './editor-self-write-registry' describe('editor self-write registry', () => { @@ -77,4 +78,15 @@ describe('editor self-write registry', () => { expect(hasRecentSelfWrite('/repo/0.md')).toBe(false) expect(hasRecentSelfWrite('/repo/259.md')).toBe(true) }) + + it('keeps remote-TTL stamps alive past the local window', () => { + // Why: SSH/runtime watcher echoes can land seconds after the write; the + // longer TTL keeps them recognized as Orca's own save. + recordSelfWrite('/repo/remote.md', 'content', 'env-1', SELF_WRITE_REMOTE_TTL_MS) + + vi.advanceTimersByTime(751) + expect(hasRecentSelfWrite('/repo/remote.md', 'env-1')).toBe(true) + vi.advanceTimersByTime(SELF_WRITE_REMOTE_TTL_MS) + expect(hasRecentSelfWrite('/repo/remote.md', 'env-1')).toBe(false) + }) }) diff --git a/src/renderer/src/components/editor/editor-self-write-registry.ts b/src/renderer/src/components/editor/editor-self-write-registry.ts index 561f4ec61..0bf0b1150 100644 --- a/src/renderer/src/components/editor/editor-self-write-registry.ts +++ b/src/renderer/src/components/editor/editor-self-write-registry.ts @@ -12,6 +12,11 @@ import { normalizeAbsolutePathForComparison } from '@/components/right-sidebar/f // path, bounded by a short TTL so a genuinely external edit that lands after // the window still gets picked up. const SELF_WRITE_TTL_MS = 750 +// Why: SSH/runtime watcher echoes travel a poll-plus-network path and can +// land seconds after the write. A local-sized TTL lets the echo arrive after +// the stamp expired, which raises a false changed-on-disk banner on remote +// tabs while typing with autosave on. +export const SELF_WRITE_REMOTE_TTL_MS = 3000 const SELF_WRITE_MAX_STAMPS = 256 export type RecentSelfWrite = { @@ -49,7 +54,8 @@ function enforceSelfWriteStampLimit(): void { export function recordSelfWrite( absolutePath: string, content?: string, - runtimeEnvironmentId?: string | null + runtimeEnvironmentId?: string | null, + ttlMs: number = SELF_WRITE_TTL_MS ): void { const now = Date.now() pruneExpiredSelfWrites(now) @@ -59,7 +65,7 @@ export function recordSelfWrite( stamps.delete(key) stamps.set(key, { content: content ?? null, - expiresAt: now + SELF_WRITE_TTL_MS + expiresAt: now + ttlMs }) enforceSelfWriteStampLimit() } diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx index 8a932eb36..15cd29adb 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelContentState.test.tsx @@ -47,6 +47,7 @@ vi.mock('@/store', () => ({ })) import { useEditorPanelContentState } from './useEditorPanelContentState' +import { getDiskBaselineSignature } from './diff-content-signature' import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT } from './editor-autosave' type Deferred = { @@ -87,6 +88,7 @@ type ProbeProps = { let latestFileContents: Record = {} let latestDiffContents: Record = {} +let latestReloadContent: (file: OpenFile) => void = () => {} const EMPTY_GIT_STATUS_BY_WORKTREE: Record = {} function HookProbe({ @@ -103,6 +105,7 @@ function HookProbe({ }) latestFileContents = state.fileContents latestDiffContents = state.diffContents + latestReloadContent = state.reloadContent return null } @@ -135,7 +138,11 @@ describe('useEditorPanelContentState', () => { mocks.isWorktreeConnectionResolved.mockReset() mocks.isWorktreeConnectionResolved.mockReturnValue(true) mocks.getState.mockReset() - mocks.getState.mockReturnValue({ settings: null }) + mocks.getState.mockReturnValue({ + settings: null, + openFiles: [], + setLastKnownDiskSignature: vi.fn() + }) }) afterEach(() => { @@ -564,4 +571,124 @@ describe('useEditorPanelContentState', () => { }) expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('fresh diff content') }) + + it('routes reloadContent for a diff tab to a forced diff refetch, not a file read', async () => { + // Why: the changed-on-disk banner's "Reload from Disk" on an unstaged + // diff tab must refetch the diff body — routing it to the file store + // would leave the visible diff stale (and vice versa for edit tabs). + const activeFile = createOpenFile({ + id: 'wt-1::diff::unstaged::file.ts', + mode: 'diff', + diffSource: 'unstaged' + }) + mocks.getRuntimeGitDiff + .mockResolvedValueOnce({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'first diff content', + originalIsBinary: false, + modifiedIsBinary: false + }) + .mockResolvedValueOnce({ + kind: 'text', + originalContent: 'old', + modifiedContent: 'reloaded diff content', + originalIsBinary: false, + modifiedIsBinary: false + }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render() + }) + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('first diff content') + ) + + await act(async () => { + latestReloadContent(activeFile) + }) + + await vi.waitFor(() => + expect(latestDiffContents[activeFile.id]?.modifiedContent).toBe('reloaded diff content') + ) + expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2) + expect(mocks.readRuntimeFileContent).not.toHaveBeenCalled() + }) + + it('routes reloadContent for an edit tab to a forced file read, not a diff refetch', async () => { + const activeFile = createOpenFile() + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old content', isBinary: false }) + .mockResolvedValueOnce({ content: 'reloaded content', isBinary: false }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render() + }) + await vi.waitFor(() => expect(latestFileContents[activeFile.id]?.content).toBe('old content')) + + await act(async () => { + latestReloadContent(activeFile) + }) + + await vi.waitFor(() => + expect(latestFileContents[activeFile.id]?.content).toBe('reloaded content') + ) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2) + expect(mocks.getRuntimeGitDiff).not.toHaveBeenCalled() + }) + + it('stamps the disk baseline when a clean tab load resolves', async () => { + const activeFile = createOpenFile() + const setLastKnownDiskSignature = vi.fn() + mocks.getState.mockReturnValue({ + settings: null, + openFiles: [activeFile], + setLastKnownDiskSignature + }) + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'disk content', isBinary: false }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render() + }) + + await vi.waitFor(() => + expect(setLastKnownDiskSignature).toHaveBeenCalledWith( + activeFile.id, + getDiskBaselineSignature('disk content') + ) + ) + }) + + it('keeps a dirty tab baseline untouched by content loads', async () => { + // Why: a dirty tab's draft still derives from the OLD content — moving + // the baseline on load would hide the conflict its restore check exists + // to catch. + const activeFile = createOpenFile({ isDirty: true }) + const setLastKnownDiskSignature = vi.fn() + mocks.getState.mockReturnValue({ + settings: null, + openFiles: [activeFile], + setLastKnownDiskSignature + }) + mocks.readRuntimeFileContent.mockResolvedValue({ content: 'disk content', isBinary: false }) + + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render() + }) + + await vi.waitFor(() => expect(latestFileContents[activeFile.id]?.content).toBe('disk content')) + expect(setLastKnownDiskSignature).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index 152b8974f..ae05de954 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -10,6 +10,7 @@ import { } from '@/lib/connection-context' import { joinPath } from '@/lib/path' import { useAppStore } from '@/store' +import { getDiskBaselineSignature } from './diff-content-signature' import { getRuntimeFileReadScope, readRuntimeFileContent } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' import { @@ -51,7 +52,26 @@ type UseEditorPanelContentStateParams = { type UseEditorPanelContentStateResult = { fileContents: Record diffContents: Record - reloadFileContent: (file: OpenFile) => void + reloadContent: (file: OpenFile) => void +} + +// Why: a clean load re-baselines what this tab's future edits are based on; a +// dirty tab keeps its baseline (its draft still derives from the older content +// the signature was taken over). Best-effort metadata — a failure here must +// not convert an already-delivered load into an error view, hence the guard. +function stampCleanTabDiskBaseline(id: string, result: FileContent): void { + if (result.isBinary || result.loadError) { + return + } + try { + const state = useAppStore.getState() + const loadedFile = state.openFiles.find((file) => file.id === id) + if (loadedFile && !loadedFile.isDirty) { + state.setLastKnownDiskSignature(id, getDiskBaselineSignature(result.content)) + } + } catch (err) { + console.warn('[editor] failed to stamp disk baseline', err) + } } function inFlightReadKey(connectionId: string | undefined, filePath: string): string { @@ -171,6 +191,7 @@ export function useEditorPanelContentState({ } delete fileLoadRetryAttemptsRef.current[id] setFileContents((prev) => ({ ...prev, [id]: result })) + stampCleanTabDiskBaseline(id, result) } catch (err) { if (fileReadGenerationRef.current[id] !== generation) { return @@ -304,8 +325,23 @@ export function useEditorPanelContentState({ [] ) - const reloadFileContent = useCallback( + // Why: the changed-on-disk banner's explicit reload on an unstaged diff tab + // must refetch the diff body, not the plain file content — one entry point + // branches on the tab mode so every consumer reloads the right store. + const reloadContent = useCallback( (file: OpenFile): void => { + if (file.mode === 'diff') { + setDiffContents((prev) => { + if (!prev[file.id]) { + return prev + } + const next = { ...prev } + delete next[file.id] + return next + }) + void loadDiffContent(file, { force: true }) + return + } delete fileLoadRetryAttemptsRef.current[file.id] setFileContents((prev) => { if (!prev[file.id]) { @@ -319,7 +355,7 @@ export function useEditorPanelContentState({ force: true }) }, - [loadFileContent] + [loadDiffContent, loadFileContent] ) useEffect(() => { @@ -507,5 +543,5 @@ export function useEditorPanelContentState({ setDiffContents ) - return { fileContents, diffContents, reloadFileContent } + return { fileContents, diffContents, reloadContent } } diff --git a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts index 3b70c27b4..19650b1cd 100644 --- a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts +++ b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts @@ -43,6 +43,12 @@ export function useEditorPanelExternalContentEvents({ return } for (const file of getOpenFilesForExternalFileChange(openFilesRef.current, detail)) { + // Why: a dirty file keeps its unsaved buffer (issue #7265) — it is + // marked changed-on-disk upstream and resolves via the editor banner, + // a save, or a later clean reload. Reloading here would clobber it. + if (file.isDirty) { + continue + } if (file.mode === 'edit' || file.mode === 'markdown-preview') { // Why: external writes must replace any in-flight pre-change read so // the tab shows the new on-disk content, not a stale dedupe result. diff --git a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx index 5b7f7bc94..7709c4f95 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelFileLoadRetry.test.tsx @@ -159,7 +159,7 @@ describe('useEditorPanelFileLoadRetry — owner-not-ready bounding (#6648)', () }) expect(loadFileContent.mock.calls.length).toBe(callsAfterTerminal) - // Retry (reloadFileContent) clears the attempt budget for a fresh start. + // Retry (reloadContent) clears the attempt budget for a fresh start. delete attemptsRef.current[file.id] expect(attemptsRef.current[file.id]).toBeUndefined() }) diff --git a/src/renderer/src/components/tab-bar/EditorFileTab.tsx b/src/renderer/src/components/tab-bar/EditorFileTab.tsx index 9de55f9c9..2ce15236f 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTab.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTab.tsx @@ -80,6 +80,11 @@ export default function EditorFileTab({ const isConflictReview = file.mode === 'conflict-review' const isCheckDetails = file.mode === 'check-details' const isMarkdownPreviewTab = file.mode === 'markdown-preview' + // Why: only deleted/renamed mean the file is gone from its path, which is + // what strikethrough conveys. 'changed' keeps a normal label — its surface + // is the changed-on-disk banner inside the editor. + const isMissingFileMutation = + file.externalMutation === 'deleted' || file.externalMutation === 'renamed' const resolvedLanguage = file.mode === 'diff' ? detectLanguage(file.relativePath) @@ -308,7 +313,7 @@ export default function EditorFileTab({ /> ) : ( { if (file.isPreview && onMakePermanent) { @@ -329,12 +334,12 @@ export default function EditorFileTab({ {tabLabel} )} - {file.externalMutation && !isRenaming && ( + {isMissingFileMutation && !isRenaming && ( {file.externalMutation} )} - {tabStatus && !isRenaming && !file.externalMutation && ( + {tabStatus && !isRenaming && !isMissingFileMutation && ( { dispose() }) + it('marks a dirty edit tab changed-on-disk instead of reloading it', () => { + const dirtyFile = { + ...fileNotes, + isDirty: true + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyFile], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyFile] as never) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + vi.advanceTimersByTime(200) + + expect(setExternalMutation).toHaveBeenCalledWith('file-notes', 'changed') + expect(notifyEditorExternalFileChange).not.toHaveBeenCalled() + dispose() + }) + + it('still reloads a clean sibling diff tab when the edit tab for the path is dirty', () => { + const dirtyFile = { + ...fileNotes, + isDirty: true + } + const cleanDiffTab = { + id: 'diff-notes', + worktreeId: 'wt-1', + worktreePath: '/repo', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + mode: 'diff' as const, + diffSource: 'unstaged' as const, + isDirty: false + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyFile, cleanDiffTab], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyFile, cleanDiffTab] as never) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + vi.advanceTimersByTime(200) + + expect(setExternalMutation).toHaveBeenCalledWith('file-notes', 'changed') + expect(notifyEditorExternalFileChange).toHaveBeenCalledWith({ + worktreeId: 'wt-1', + worktreePath: '/repo', + relativePath: 'notes.md', + runtimeEnvironmentId: null + }) + dispose() + }) + + it('does not mark a dirty tab for the echo of Orca’s own save', async () => { + const dirtyFile = { + ...fileNotes, + isDirty: true + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyFile], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyFile] as never) + const readFile = vi.fn().mockResolvedValue({ content: 'orca save', isBinary: false }) + vi.stubGlobal('window', { api: { fs: { readFile } } }) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + recordSelfWrite('/repo/notes.md', 'orca save') + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + await vi.advanceTimersByTimeAsync(100) + + expect(setExternalMutation).not.toHaveBeenCalledWith('file-notes', 'changed') + dispose() + }) + + it('marks a dirty tab when a genuine external write lands inside the self-write TTL', async () => { + const dirtyFile = { + ...fileNotes, + isDirty: true + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyFile], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyFile] as never) + const readFile = vi.fn().mockResolvedValue({ content: 'agent content', isBinary: false }) + vi.stubGlobal('window', { api: { fs: { readFile } } }) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + recordSelfWrite('/repo/notes.md', 'orca save') + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + await vi.advanceTimersByTimeAsync(100) + + expect(setExternalMutation).toHaveBeenCalledWith('file-notes', 'changed') + dispose() + }) + + it('shares one echo-verification read across a burst of watcher payloads', async () => { + const dirtyFile = { + ...fileNotes, + isDirty: true + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyFile], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyFile] as never) + const readFile = vi.fn().mockResolvedValue({ content: 'agent content', isBinary: false }) + vi.stubGlobal('window', { api: { fs: { readFile } } }) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + recordSelfWrite('/repo/notes.md', 'orca save') + // Why: SSH poll + event streams can deliver several payloads for one + // write; each verification is a full-file read, so a burst must share + // the in-flight read instead of stacking network round-trips. + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + await vi.advanceTimersByTimeAsync(100) + + expect(readFile).toHaveBeenCalledTimes(1) + expect(setExternalMutation).toHaveBeenCalledWith('file-notes', 'changed') + dispose() + }) + + it('marks a lone dirty unstaged-diff tab changed-on-disk', () => { + const dirtyDiffTab = { + id: 'diff-notes', + worktreeId: 'wt-1', + worktreePath: '/repo', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + mode: 'diff' as const, + diffSource: 'unstaged' as const, + isDirty: true + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyDiffTab], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyDiffTab] as never) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + vi.advanceTimersByTime(200) + + expect(setExternalMutation).toHaveBeenCalledWith('diff-notes', 'changed') + expect(notifyEditorExternalFileChange).not.toHaveBeenCalled() + dispose() + }) + + it('does not let an update event clear a changed-on-disk mark while the tab stays dirty', () => { + const dirtyChangedFile = { + ...fileNotes, + isDirty: true, + externalMutation: 'changed' as const + } + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [dirtyChangedFile], + setExternalMutation + } as never) + vi.mocked(getOpenFilesForExternalFileChange).mockReturnValue([dirtyChangedFile] as never) + const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget) + + handleFsChanged(payload([{ kind: 'update', absolutePath: '/repo/notes.md' }])) + vi.advanceTimersByTime(200) + + expect(setExternalMutation).not.toHaveBeenCalledWith('file-notes', null) + dispose() + }) + it('does not reload a branch-compare combined diff for working-tree changes', () => { const branchDiffTab = { id: 'wt-1::all-diffs::branch', diff --git a/src/renderer/src/hooks/useEditorExternalWatch.ts b/src/renderer/src/hooks/useEditorExternalWatch.ts index 157b91df0..9fd18202b 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.ts @@ -8,6 +8,7 @@ import { basename, joinPath } from '@/lib/path' import { getExternalFileChangeRelativePath } from '@/components/right-sidebar/useFileExplorerWatch' import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' import { + canAutoSaveOpenFile, getOpenFilesForExternalFileChange, isExternalReloadableEditorTab, isWorkingTreeCombinedDiffTab, @@ -28,6 +29,7 @@ import { type WorktreeFileChangeEventDetail } from './worktree-file-change-event' import { isGitRepoKind } from '../../../shared/repo-kind' +import { markFileChangedOnDisk } from '@/components/editor/editor-changed-on-disk-mark' // Why: atomic-write patterns (Claude Code's Edit tool, editors like vim, // VSCode) land as a short burst of `update` events — or `delete + create` on @@ -545,7 +547,10 @@ export function createExternalWatchEventHandler( // Why: if a previously-deleted file reappears at the same path (e.g. // the user ran `git checkout`), clear the tombstone so the tab returns // to its normal state and any non-dirty content gets reloaded below. - // `createOrUpdatePaths` was collected above. + // `createOrUpdatePaths` was collected above. Scoped to deleted/renamed: + // a 'changed' mark means the file was rewritten while the tab was dirty, + // so a further update event must not clear it — it resolves via reload, + // save, or the reload path below. if (createOrUpdatePaths.size > 0) { const state = useAppStore.getState() for (const file of state.openFiles) { @@ -553,7 +558,7 @@ export function createExternalWatchEventHandler( file.worktreeId === target.worktreeId && openFileRuntimeOwner(file) === target.runtimeEnvironmentId && (file.mode === 'edit' || file.mode === 'markdown-preview') && - file.externalMutation && + (file.externalMutation === 'deleted' || file.externalMutation === 'renamed') && createOrUpdatePaths.has(normalizeRuntimePathForComparison(file.filePath)) ) { state.setExternalMutation(file.id, null) @@ -637,11 +642,27 @@ export function createExternalWatchEventHandler( } continue } - if (matching.some((f) => f.isDirty)) { - if (hasCombinedDiffConsumer) { - scheduleDebouncedExternalReload(notification) + const dirtyMatches = matching.filter((f) => f.isDirty) + if (dirtyMatches.length > 0) { + // Why: an external write landing on a dirty tab must not vanish + // silently (issue #7265) — the user was left with a stale tab and a + // save that clobbered the newer disk content. Mark the tab so the + // editor shows a changed-on-disk banner with an explicit reload path. + scheduleChangedOnDiskMark( + target, + notification, + // Why: canAutoSaveOpenFile is exactly the set of tabs that can hold + // unsaved edits (edit + unstaged diff) — the tabs the banner serves. + dirtyMatches.filter((dirtyFile) => canAutoSaveOpenFile(dirtyFile)).map((f) => f.id) + ) + if (dirtyMatches.length === matching.length) { + if (hasCombinedDiffConsumer) { + scheduleDebouncedExternalReload(notification) + } + continue } - continue + // Clean sibling tabs (e.g. an unstaged diff of the same path) still + // reload below; every notification consumer skips dirty files. } const absolutePath = joinPath(notification.worktreePath, notification.relativePath) const recentSelfWrite = getRecentSelfWrite(absolutePath, target.runtimeEnvironmentId) @@ -665,6 +686,90 @@ export function createExternalWatchEventHandler( return { handleFsChanged, dispose } } +const inFlightEchoVerificationReads = new Map>() + +// Why: one save echo can arrive as a burst of watcher payloads (SSH poll + +// event streams), and each verification is a full-file read — on remote +// transports a network round-trip. Concurrent payloads for the same file +// share the in-flight read instead of stacking duplicates. +function readFileForEchoVerification(args: { + runtimeEnvironmentId: string | null | undefined + filePath: string + relativePath: string + worktreeId: string | null | undefined + connectionId: string | undefined +}): ReturnType { + const key = `${args.runtimeEnvironmentId ?? ''}::${args.connectionId ?? ''}::${args.filePath}` + let pending = inFlightEchoVerificationReads.get(key) + if (!pending) { + pending = readRuntimeFileContent({ + settings: args.runtimeEnvironmentId + ? { activeRuntimeEnvironmentId: args.runtimeEnvironmentId } + : null, + filePath: args.filePath, + relativePath: args.relativePath, + worktreeId: args.worktreeId ?? undefined, + connectionId: args.connectionId + }) + inFlightEchoVerificationReads.set(key, pending) + const release = (): void => { + if (inFlightEchoVerificationReads.get(key) === pending) { + inFlightEchoVerificationReads.delete(key) + } + } + pending.then(release, release) + } + return pending +} + +function markTabsChangedOnDisk(fileIds: string[], connectionId: string | undefined): void { + const state = useAppStore.getState() + for (const fileId of fileIds) { + const file = state.openFiles.find((f) => f.id === fileId) + // Why: echo verification resolves async — a save or reload may already + // have resolved the conflict; the helper only marks still-dirty tabs. + if (file) { + markFileChangedOnDisk(state, file, { connectionId, origin: 'live' }) + } + } +} + +function scheduleChangedOnDiskMark( + target: WatchedTarget, + notification: ExternalWatchNotification, + fileIds: string[] +): void { + if (fileIds.length === 0) { + return + } + const absolutePath = joinPath(notification.worktreePath, notification.relativePath) + const recentSelfWrite = getRecentSelfWrite(absolutePath, target.runtimeEnvironmentId) + // Why: the fs event may be the echo of Orca's own save racing keystrokes + // typed during the write. Marking on the echo would show a false "changed + // on disk" banner, so verify disk really differs from our last write. + if (!recentSelfWrite || recentSelfWrite.content === null) { + markTabsChangedOnDisk(fileIds, target.connectionId) + return + } + void readFileForEchoVerification({ + runtimeEnvironmentId: target.runtimeEnvironmentId, + filePath: absolutePath, + relativePath: notification.relativePath, + worktreeId: notification.worktreeId, + connectionId: target.connectionId + }) + .then((result) => { + if (result.isBinary || result.content !== recentSelfWrite.content) { + markTabsChangedOnDisk(fileIds, target.connectionId) + } + }) + .catch(() => { + // Why: unreadable disk state can't disprove an external change — keep + // the conflict visible rather than risk a silent overwrite. + markTabsChangedOnDisk(fileIds, target.connectionId) + }) +} + function scheduleSelfWriteAwareExternalReload( target: WatchedTarget, notification: ExternalWatchNotification, @@ -680,8 +785,8 @@ function scheduleSelfWriteAwareExternalReload( // Why: a recent self-write stamp only proves the path changed recently; an // agent can write a newer version inside the same TTL. Compare disk content // with the saved text so we suppress only the echo of Orca's own write. - void readRuntimeFileContent({ - settings: runtimeEnvironmentId ? { activeRuntimeEnvironmentId: runtimeEnvironmentId } : null, + void readFileForEchoVerification({ + runtimeEnvironmentId, filePath: file.filePath, relativePath: file.relativePath, worktreeId: file.worktreeId, @@ -706,7 +811,9 @@ function scheduleSelfWriteAwareExternalReload( function hasCleanExternalReloadTarget(notification: ExternalWatchNotification): boolean { const matching = getOpenFilesForExternalFileChange(useAppStore.getState().openFiles, notification) - return matching.length > 0 && matching.every((file) => !file.isDirty) + // Why: one clean target is enough — every notification consumer skips dirty + // files per-file, so a dirty sibling tab no longer vetoes the reload. + return matching.some((file) => !file.isDirty) } export function getOverflowExternalReloadTargets( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index d15a81c46..7a51b6f4c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -11522,6 +11522,24 @@ }, "richMarkdownLargeTextPaste": { "tooLarge": "Paste is too large." + }, + "ExternalFileChangeBanner": { + "7c41e90d12": "This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "5c02de9b31": "Reloaded from disk", + "d1e830fa22": "Undo", + "90b2ce7d43": "Compare" + }, + "ExternalFileChangeCompareDialog": { + "4b8de20a11": "File changed on disk", + "90cc31e4d7": "Disk version on the left, your unsaved edits on the right.", + "8fe30ab254": "Reading file from disk...", + "e2b1cd0393": "Could not read the file from disk: {{value0}}", + "b6cf20d514": "The file on disk is binary — no text comparison available.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "2c8f1e07b9": "Loading comparison..." } }, "diff": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 84d5a861a..5551dae29 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -11522,6 +11522,24 @@ "3e6c9a2b71": "pendiente", "4f7d0c3e88": "pasos fallidos", "5a8e1d4f23": " · " + }, + "ExternalFileChangeBanner": { + "7c41e90d12": "This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "5c02de9b31": "Reloaded from disk", + "d1e830fa22": "Undo", + "90b2ce7d43": "Compare" + }, + "ExternalFileChangeCompareDialog": { + "4b8de20a11": "File changed on disk", + "90cc31e4d7": "Disk version on the left, your unsaved edits on the right.", + "8fe30ab254": "Reading file from disk...", + "e2b1cd0393": "Could not read the file from disk: {{value0}}", + "b6cf20d514": "The file on disk is binary — no text comparison available.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "2c8f1e07b9": "Loading comparison..." } }, "diff": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 20e253a58..158ec815b 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -11522,6 +11522,24 @@ "3e6c9a2b71": "保留中", "4f7d0c3e88": "失敗したステップ", "5a8e1d4f23": " · " + }, + "ExternalFileChangeBanner": { + "7c41e90d12": "This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "5c02de9b31": "Reloaded from disk", + "d1e830fa22": "Undo", + "90b2ce7d43": "Compare" + }, + "ExternalFileChangeCompareDialog": { + "4b8de20a11": "File changed on disk", + "90cc31e4d7": "Disk version on the left, your unsaved edits on the right.", + "8fe30ab254": "Reading file from disk...", + "e2b1cd0393": "Could not read the file from disk: {{value0}}", + "b6cf20d514": "The file on disk is binary — no text comparison available.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "2c8f1e07b9": "Loading comparison..." } }, "diff": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index cd83a58fa..aba282e25 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -11522,6 +11522,24 @@ "3e6c9a2b71": "보류 중", "4f7d0c3e88": "단계 실패", "5a8e1d4f23": " · " + }, + "ExternalFileChangeBanner": { + "7c41e90d12": "This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "5c02de9b31": "Reloaded from disk", + "d1e830fa22": "Undo", + "90b2ce7d43": "Compare" + }, + "ExternalFileChangeCompareDialog": { + "4b8de20a11": "File changed on disk", + "90cc31e4d7": "Disk version on the left, your unsaved edits on the right.", + "8fe30ab254": "Reading file from disk...", + "e2b1cd0393": "Could not read the file from disk: {{value0}}", + "b6cf20d514": "The file on disk is binary — no text comparison available.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "2c8f1e07b9": "Loading comparison..." } }, "diff": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 5be008913..582ad7e30 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -11522,6 +11522,24 @@ "3e6c9a2b71": "等待中", "4f7d0c3e88": "步骤失败", "5a8e1d4f23": " · " + }, + "ExternalFileChangeBanner": { + "7c41e90d12": "This file changed on disk while you have unsaved edits. Saving will overwrite the newer disk content.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "5c02de9b31": "Reloaded from disk", + "d1e830fa22": "Undo", + "90b2ce7d43": "Compare" + }, + "ExternalFileChangeCompareDialog": { + "4b8de20a11": "File changed on disk", + "90cc31e4d7": "Disk version on the left, your unsaved edits on the right.", + "8fe30ab254": "Reading file from disk...", + "e2b1cd0393": "Could not read the file from disk: {{value0}}", + "b6cf20d514": "The file on disk is binary — no text comparison available.", + "3fa2b8d417": "Reload from Disk", + "a95d02c644": "Keep My Edits", + "2c8f1e07b9": "Loading comparison..." } }, "diff": { diff --git a/src/renderer/src/lib/workspace-session-editor-drafts.test.ts b/src/renderer/src/lib/workspace-session-editor-drafts.test.ts index 88ce7a68d..99dd4ec59 100644 --- a/src/renderer/src/lib/workspace-session-editor-drafts.test.ts +++ b/src/renderer/src/lib/workspace-session-editor-drafts.test.ts @@ -78,4 +78,49 @@ describe('workspace session editor drafts', () => { }) ]) }) + + it('persists the disk baseline signature only alongside a dirty draft', () => { + const payload = buildWorkspaceSessionPayload( + createSnapshot({ + openFiles: [ + { + id: '/tmp/dirty.md', + filePath: '/tmp/dirty.md', + relativePath: 'dirty.md', + worktreeId: 'wt-1', + language: 'markdown', + mode: 'edit', + isDirty: true, + lastKnownDiskSignature: 'abc123' + } as never, + { + id: '/tmp/clean.md', + filePath: '/tmp/clean.md', + relativePath: 'clean.md', + worktreeId: 'wt-1', + language: 'markdown', + mode: 'edit', + isDirty: false, + lastKnownDiskSignature: 'def456' + } as never + ], + editorDrafts: { + '/tmp/dirty.md': 'unsaved edits' + } + }) + ) + + expect(payload.openFilesByWorktree?.['wt-1']).toEqual([ + expect.objectContaining({ + filePath: '/tmp/dirty.md', + dirtyDraftContent: 'unsaved edits', + lastKnownDiskSignature: 'abc123' + }), + // Why: a clean tab has no draft to conflict-check on restore, and + // persisting the signature anyway would bloat every session write. + expect.not.objectContaining({ + lastKnownDiskSignature: expect.any(String) + }) + ]) + }) }) diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index 88cb8ca5e..4970840d5 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -135,7 +135,13 @@ export function buildEditorSessionData( language: f.language, isPreview: f.isPreview || undefined, runtimeEnvironmentId: f.runtimeEnvironmentId, - ...(dirtyDraftContent !== undefined ? { dirtyDraftContent } : {}) + ...(dirtyDraftContent !== undefined ? { dirtyDraftContent } : {}), + // Why: the edit baseline travels with the dirty draft so a restore can + // re-derive a changed-on-disk conflict before autosave may overwrite an + // agent write that landed while the app was closed. + ...(dirtyDraftContent !== undefined && f.lastKnownDiskSignature + ? { lastKnownDiskSignature: f.lastKnownDiskSignature } + : {}) }) const ids = editFileIdsByWorktree[f.worktreeId] ?? (editFileIdsByWorktree[f.worktreeId] = new Set()) diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 5b451cbbc..4fdd5fb76 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -251,8 +251,21 @@ export type OpenFile = { // disk while it's open, we keep the tab around so the user can still see // (and potentially save) their in-memory content. The tab surfaces this as // a strikethrough label plus a "deleted"/"renamed" suffix. Cleared if the - // file reappears on disk at its original path. - externalMutation?: 'deleted' | 'renamed' + // file reappears on disk at its original path. 'changed' means the file was + // rewritten on disk while this tab held unsaved edits (issue #7265): the + // buffer is preserved and the editor shows a changed-on-disk banner instead + // of tab strikethrough. + externalMutation?: 'deleted' | 'renamed' | 'changed' + /** Why: signature of the disk content this tab's edits are based on (last + * load or save). Persisted with dirty drafts so a restore can re-derive a + * changed-on-disk conflict from ground truth — an agent write that landed + * while the app was closed must not be clobbered by a resumed autosave. */ + lastKnownDiskSignature?: string + /** Why: set at hydration for restored dirty tabs; suspends autosave until + * the restored-tab conflict scan has compared disk against the baseline. + * Without this hard gate the scan's async read merely races the autosave + * timer, and a slow (SSH/runtime) read loses the race. Not persisted. */ + pendingDiskBaselineVerification?: boolean /** Why: diff bodies are cached in EditorPanel. Re-selecting an existing diff * tab from the tree bumps this so the panel refetches instead of reusing a * stale snapshot. */ @@ -497,7 +510,9 @@ export type EditorSlice = { setActiveFile: (fileId: string) => void reorderFiles: (fileIds: string[]) => void markFileDirty: (fileId: string, dirty: boolean) => void - setExternalMutation: (fileId: string, mutation: 'deleted' | 'renamed' | null) => void + setExternalMutation: (fileId: string, mutation: 'deleted' | 'renamed' | 'changed' | null) => void + setLastKnownDiskSignature: (fileId: string, signature: string) => void + clearPendingDiskBaselineVerification: (fileId: string) => void clearUntitled: (fileId: string) => void openDiff: ( worktreeId: string, @@ -2469,6 +2484,32 @@ export const createEditorSlice: StateCreator = (s } }), + setLastKnownDiskSignature: (fileId, signature) => + set((s) => { + const file = s.openFiles.find((f) => f.id === fileId) + if (!file || file.lastKnownDiskSignature === signature) { + return s + } + return { + openFiles: s.openFiles.map((f) => + f.id === fileId ? { ...f, lastKnownDiskSignature: signature } : f + ) + } + }), + + clearPendingDiskBaselineVerification: (fileId) => + set((s) => { + const file = s.openFiles.find((f) => f.id === fileId) + if (!file?.pendingDiskBaselineVerification) { + return s + } + return { + openFiles: s.openFiles.map((f) => + f.id === fileId ? { ...f, pendingDiskBaselineVerification: undefined } : f + ) + } + }), + clearUntitled: (fileId) => set((s) => ({ openFiles: s.openFiles.map((f) => (f.id === fileId ? { ...f, isUntitled: undefined } : f)) @@ -4300,6 +4341,15 @@ export const createEditorSlice: StateCreator = (s isDirty: pf.dirtyDraftContent !== undefined, isPreview: pf.isPreview, runtimeEnvironmentId: pf.runtimeEnvironmentId, + lastKnownDiskSignature: pf.lastKnownDiskSignature, + // Why: hard-suspends autosave until the restored-tab conflict scan + // verifies disk against the baseline — an async race would let a + // slow remote read lose to the autosave timer and clobber an + // offline agent write. + pendingDiskBaselineVerification: + pf.dirtyDraftContent !== undefined && pf.lastKnownDiskSignature !== undefined + ? true + : undefined, mode: 'edit' }) } diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 5df0c89d8..cb4169cb1 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -1364,6 +1364,25 @@ const terminalPaneSplitSchema = z }) .strict() +// Why: measures the changed-on-disk conflict flow (issue #7265) — how often +// conflicts surface per transport (false-banner detection on ssh/runtime +// echoes) and which resolution users pick. Deliberately path-free. +const editorExternalChangeConflictShownSchema = z + .object({ + surface: z.enum(['edit', 'unstaged-diff']), + transport: z.enum(['local', 'ssh', 'runtime']), + origin: z.enum(['live', 'restore']) + }) + .strict() + +const editorExternalChangeConflictActionSchema = z + .object({ + action: z.enum(['reload', 'keep', 'compare', 'undo_reload', 'save_overwrite']), + surface: z.enum(['edit', 'unstaged-diff']), + transport: z.enum(['local', 'ssh', 'runtime']) + }) + .strict() + // ── Event registry: the one record the validator consumes ─────────────── // // The validator does `eventSchemas[name].safeParse(props)`. `EventMap` is @@ -1451,6 +1470,9 @@ export const eventSchemas = { setup_guide_step_completed: setupGuideStepCompletedSchema, terminal_pane_split: terminalPaneSplitSchema, + editor_external_change_conflict_shown: editorExternalChangeConflictShownSchema, + editor_external_change_conflict_action: editorExternalChangeConflictActionSchema, + smart_sort_class_distribution: smartSortClassDistributionSchema, smart_sort_class_1_promotion: smartSortClass1PromotionSchema, smart_to_recent_switch: smartToRecentSwitchSchema diff --git a/src/shared/types.ts b/src/shared/types.ts index 92715f786..98749e101 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1022,6 +1022,9 @@ export type PersistedOpenFile = { runtimeEnvironmentId?: string | null /** Unsaved editor buffer captured for hot exit; presence restores the tab dirty. */ dirtyDraftContent?: string + /** Signature of the disk content the dirty draft is based on; lets restore + * re-derive a changed-on-disk conflict from ground truth. */ + lastKnownDiskSignature?: string } export type WorkspaceSessionState = { diff --git a/src/shared/workspace-session-schema.ts b/src/shared/workspace-session-schema.ts index 07e98996e..22657617c 100644 --- a/src/shared/workspace-session-schema.ts +++ b/src/shared/workspace-session-schema.ts @@ -161,7 +161,8 @@ const persistedOpenFileSchema = z.object({ language: z.string(), isPreview: z.boolean().optional(), runtimeEnvironmentId: z.string().nullable().optional(), - dirtyDraftContent: z.string().optional() + dirtyDraftContent: z.string().optional(), + lastKnownDiskSignature: z.string().optional() }) // ─── Browser ────────────────────────────────────────────────────────