fix(editor): preserve failed save result

This commit is contained in:
Brennan Benson
2026-07-20 20:11:38 -07:00
parent b852929766
commit 2ccdd55466
12 changed files with 123 additions and 77 deletions
@@ -34,7 +34,7 @@ export function ChangesModeView({
viewStateScopeId: string
diffViewStateKey: string
onContentChange: (content: string) => void
onSave: (content: string) => Promise<void>
onSave: (content: string) => Promise<boolean>
}): React.JSX.Element {
if (!dc) {
return (
@@ -43,7 +43,7 @@ const IpynbViewer = lazy(() => import('./IpynbViewer'))
// Why: module-level for a stable no-op identity so read-only tabs don't rebuild callbacks each render.
const noopEditorContentChange = (_content: string): void => {}
const noopEditorSave = (_content: string): void => {}
const noopEditorSave = async (_content: string): Promise<boolean> => false
export function getMarkdownSourceLineOffset(frontMatterRaw: string): number {
let offset = 0
@@ -164,8 +164,8 @@ export function EditorContent({
handleContentChange: (content: string) => void
handleContentChangeForFile: (file: OpenFile, content: string) => void
handleDirtyStateHint: (dirty: boolean) => void
handleSave: (content: string) => Promise<void>
handleSaveForFile: (file: OpenFile, content: string) => Promise<void>
handleSave: (content: string) => Promise<boolean>
handleSaveForFile: (file: OpenFile, content: string) => Promise<boolean>
reloadContent: (file: OpenFile) => void
}): React.JSX.Element {
const editorViewStateKey =
@@ -376,7 +376,7 @@ export function EditorContent({
: handleContentChange
const onSaveWithFm = fm
? (body: string): Promise<void> => md.mdSave(prependFrontMatter(fm.raw, body))
? (body: string): Promise<boolean> => md.mdSave(prependFrontMatter(fm.raw, body))
: md.mdSave
return (
@@ -6,7 +6,6 @@ import { openFilePreviewToSide } from '@/lib/file-preview'
import { getEditorHeaderCopyState } from './editor-header'
import { isLocalPathOpenBlocked, showLocalPathOpenBlockedToast } from '@/lib/local-path-open-guard'
import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client'
import { requestEditorFileSave } from './editor-autosave'
import { exportActiveMarkdownToPdf } from './export-active-markdown'
import type { EditorToggleValue } from './EditorViewToggle'
import { EditorPanelShell } from './EditorPanelShell'
@@ -24,7 +23,7 @@ import {
selectEditorPanelGitStatusEntries
} from './editor-panel-git-entry-selector'
import { createEditorPanelDraftSelector } from './editor-panel-draft-selector'
import { notifyEditorSaveFailure } from './editor-save-failure-notice'
import { attemptEditorFileSave } from './editor-file-save-attempt'
function EditorPanelInner({
activeFileId: activeFileIdProp,
@@ -174,9 +173,9 @@ function EditorPanelInner({
)
const handleSaveForFile = useCallback(
async (file: typeof activeFile, content: string) => {
async (file: typeof activeFile, content: string): Promise<boolean> => {
if (!file) {
return
return false
}
const saveTargetFile =
file.mode === 'markdown-preview'
@@ -186,24 +185,20 @@ function EditorPanelInner({
) ?? null)
: file
if (!saveTargetFile) {
return
return false
}
if (saveTargetFile.isUntitled) {
requestRenameForFile(saveTargetFile.id)
return
}
try {
await requestEditorFileSave({ fileId: saveTargetFile.id, fallbackContent: content })
} catch (error) {
notifyEditorSaveFailure(error)
return false
}
return attemptEditorFileSave({ fileId: saveTargetFile.id, fallbackContent: content })
},
[openFiles, requestRenameForFile]
)
const handleSave = useCallback(
async (content: string) => {
await handleSaveForFile(activeFile, content)
async (content: string): Promise<boolean> => {
return handleSaveForFile(activeFile, content)
},
[activeFile, handleSaveForFile]
)
@@ -44,8 +44,8 @@ type EditorPanelShellProps = {
onContentChange: (content: string) => void
onContentChangeForFile: (file: OpenFile, content: string) => void
onDirtyStateHint: (dirty: boolean) => void
onSave: (content: string) => Promise<void>
onSaveForFile: (file: OpenFile, content: string) => Promise<void>
onSave: (content: string) => Promise<boolean>
onSaveForFile: (file: OpenFile, content: string) => Promise<boolean>
onReloadContent: (file: OpenFile) => void
onCloseMarkdownTableOfContents: () => void
onCloseRenameDialog: () => void
@@ -80,7 +80,7 @@ type IpynbViewerProps = {
scrollCacheKey: string
onContentChange: (content: string) => void
onDirtyStateHint: (dirty: boolean) => void
onSave: (content: string) => Promise<void>
onSave: (content: string) => Promise<boolean>
}
const NOTEBOOK_SOURCE_COMMIT_DELAY_MS = 400
@@ -843,7 +843,10 @@ export default function IpynbViewer({
setRunError(null)
setRunningCellIndex(index)
try {
await onSave(latestContent)
const didSave = await onSave(latestContent)
if (!didSave) {
return
}
const result = await window.api.notebook.runPythonCell({
filePath,
code: cell.source,
@@ -0,0 +1,42 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { requestEditorFileSave, toastError } = vi.hoisted(() => ({
requestEditorFileSave: vi.fn(),
toastError: vi.fn()
}))
vi.mock('./editor-autosave', () => ({ requestEditorFileSave }))
vi.mock('sonner', () => ({ toast: { error: toastError } }))
// Return the English fallback so the assertion is stable without initializing i18n.
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
import { attemptEditorFileSave } from './editor-file-save-attempt'
describe('attemptEditorFileSave', () => {
beforeEach(() => {
requestEditorFileSave.mockReset()
toastError.mockClear()
})
afterEach(() => {
vi.restoreAllMocks()
})
it('reports success only after the save request resolves', async () => {
requestEditorFileSave.mockResolvedValue(undefined)
await expect(attemptEditorFileSave({ fileId: 'file-1' })).resolves.toBe(true)
expect(toastError).not.toHaveBeenCalled()
})
it('surfaces a failed save and reports failure to dependent actions (STA-2027)', async () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
const cause = new Error('disk full')
requestEditorFileSave.mockRejectedValue(cause)
await expect(attemptEditorFileSave({ fileId: 'file-1' })).resolves.toBe(false)
expect(toastError).toHaveBeenCalledTimes(1)
expect(toastError).toHaveBeenCalledWith('Failed to save the file. Please try again.')
expect(consoleError).toHaveBeenCalledWith('[editor] file save failed', cause)
})
})
@@ -0,0 +1,20 @@
import { toast } from 'sonner'
import { translate } from '@/i18n/i18n'
import { requestEditorFileSave, type EditorSaveFileTarget } from './editor-autosave'
export async function attemptEditorFileSave(target: EditorSaveFileTarget): Promise<boolean> {
try {
await requestEditorFileSave(target)
return true
} catch (error) {
// Why: shortcut handlers need a non-throwing result, while dependent actions must not treat a rejected write as success.
console.error('[editor] file save failed', error)
toast.error(
translate(
'auto.components.editor.editor.save.failure.notice.8c59ce5075',
'Failed to save the file. Please try again.'
)
)
return false
}
}
@@ -1,36 +0,0 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { toastError } = vi.hoisted(() => ({ toastError: vi.fn() }))
vi.mock('sonner', () => ({ toast: { error: toastError } }))
// Return the English fallback so the assertion is stable without initializing i18n.
vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
import { notifyEditorSaveFailure } from './editor-save-failure-notice'
describe('notifyEditorSaveFailure', () => {
beforeEach(() => {
toastError.mockClear()
})
afterEach(() => {
vi.restoreAllMocks()
})
it('surfaces a failed save as a toast instead of swallowing it (STA-2027)', () => {
// Why: the regression was silent data loss — a save rejection must reach the user, not vanish.
vi.spyOn(console, 'error').mockImplementation(() => {})
notifyEditorSaveFailure(new Error('disk full'))
expect(toastError).toHaveBeenCalledTimes(1)
expect(toastError).toHaveBeenCalledWith('Failed to save the file. Please try again.')
})
it('logs the underlying error for diagnosis', () => {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
const cause = new Error('EPERM')
notifyEditorSaveFailure(cause)
expect(consoleError).toHaveBeenCalledWith('[editor] file save failed', cause)
})
})
@@ -1,15 +0,0 @@
import { toast } from 'sonner'
import { translate } from '@/i18n/i18n'
// Why: requestEditorFileSave rejects on a dropped write, but callers used to swallow it — turning a
// failed save into silent data loss (the edit stays dirty in the buffer with no signal it never
// persisted, STA-2027). Surface it so Cmd/Ctrl+S can't look successful while dropping the save.
export function notifyEditorSaveFailure(error: unknown): void {
console.error('[editor] file save failed', error)
toast.error(
translate(
'auto.components.editor.editor.save.failure.notice.8c59ce5075',
'Failed to save the file. Please try again.'
)
)
}
@@ -8,7 +8,7 @@ type UseEditorCmdSaveRequestParams = {
activeFile: OpenFile | null
openFiles: OpenFile[]
fileContents: Record<string, FileContent>
handleSave: (content: string) => Promise<void>
handleSave: (content: string) => Promise<boolean>
}
export function useEditorCmdSaveRequest({
@@ -0,0 +1,23 @@
import { describe, expect, it, vi } from 'vitest'
import { saveMarkdownAndRefreshDocuments } from './useMarkdownDocuments'
describe('saveMarkdownAndRefreshDocuments', () => {
it('does not refresh the document index after a failed write', async () => {
const save = vi.fn().mockResolvedValue(false)
const refresh = vi.fn().mockResolvedValue(undefined)
await expect(saveMarkdownAndRefreshDocuments('draft', save, refresh)).resolves.toBe(false)
expect(save).toHaveBeenCalledWith('draft')
expect(refresh).not.toHaveBeenCalled()
})
it('refreshes the document index once after a successful write', async () => {
const save = vi.fn().mockResolvedValue(true)
const refresh = vi.fn().mockResolvedValue(undefined)
await expect(saveMarkdownAndRefreshDocuments('draft', save, refresh)).resolves.toBe(true)
expect(refresh).toHaveBeenCalledTimes(1)
})
})
@@ -17,6 +17,19 @@ type OpenMarkdownDocumentOptions = {
anchor?: string | null
}
export async function saveMarkdownAndRefreshDocuments(
content: string,
save: (content: string) => Promise<boolean>,
refresh: () => Promise<void>
): Promise<boolean> {
const didSave = await save(content)
if (!didSave) {
return false
}
await refresh()
return true
}
type UseMarkdownDocumentsResult = {
markdownDocuments: MarkdownDocument[]
openMarkdownDocument: (
@@ -31,14 +44,14 @@ type UseMarkdownDocumentsResult = {
options?: OpenMarkdownDocumentOptions
) => Promise<void>
}
mdSave: (content: string) => Promise<void>
mdSave: (content: string) => Promise<boolean>
}
export function useMarkdownDocuments(
activeFile: OpenFile,
isMarkdown: boolean,
viewMode: MarkdownViewMode,
onSave: (content: string) => Promise<void>
onSave: (content: string) => Promise<boolean>
): UseMarkdownDocumentsResult {
const worktreeId = activeFile.worktreeId
// Why: PTY activity replaces worktree metadata; only a routing-path change
@@ -179,7 +192,8 @@ export function useMarkdownDocuments(
)
const mdSave = useCallback(
(content: string) => onSave(content).then(() => refreshMarkdownDocuments(true)),
(content: string) =>
saveMarkdownAndRefreshDocuments(content, onSave, () => refreshMarkdownDocuments(true)),
[onSave, refreshMarkdownDocuments]
)