fix(editor): disk-checked save follow-ups: repeatable stale reloads, Markdown whitespace, Undo of Reload (#31113)

* fix(editor): follow-ups to disk-checked saves: repeatable stale reloads, Markdown whitespace, Undo of Reload

- A stale cached copy can reload again once the disk baseline moves or a mount sees fresh text.
- A clean Markdown tab keeps a trailing-whitespace-only draft across a reload of the same text.
- Undo of Reload re-reads the file's tabs, so typing in a tab that joined after the reload wins.

* fix(editor): Undo of Reload skips tabs that left the model; keep whitespace drafts only when disk is unchanged
This commit is contained in:
Neil
2026-10-20 05:00:00 -07:00
committed by GitHub
parent c2b3364445
commit 0848dbfcc4
7 changed files with 158 additions and 18 deletions
@@ -74,12 +74,15 @@ export function reloadTabContentFromDisk(
// signal: the editor content-sync repopulates editorDrafts with the
// reloaded content itself, so draft existence proves nothing.
// Why every sibling: they share the reloaded text, so typing in any of them is newer work.
const liveSiblings = siblings.flatMap(
(tab) => current.openFiles.find((openFile) => openFile.id === tab.id) ?? []
)
if (!liveFile || liveSiblings.some((tab) => tab.isDirty)) {
// Why re-read: a tab that joined the model since the reload (another workspace) counts too.
const modelTabs = liveFile ? editorModelSiblingTabs(current, liveFile) : []
if (!liveFile || modelTabs.some((tab) => tab.isDirty)) {
return
}
// Why: a reloaded tab that has since left the model holds its own text; leave it alone.
const liveSiblings = modelTabs.filter((tab) =>
siblings.some((reloaded) => reloaded.id === tab.id)
)
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
@@ -1,4 +1,6 @@
import { useAppStore } from '@/store'
import { useAppStore, type AppState } from '@/store'
import type { OpenFile } from '@/store/slices/editor'
import { isEditorContentUnchanged } from './editor-content-dirty-state'
import { getDiskBaselineSignature } from './diff-content-signature'
import type { FileContent } from './editor-panel-content-types'
import {
@@ -9,6 +11,22 @@ import {
import { setModelExternalMutation } from './editor-save-rebaseline'
import { notifyEditorFileCacheStale } from './editor-autosave'
/** `tab` is the pre-load snapshot, so its baseline is the version the draft was typed over. */
function clearDraftUnlessWhitespaceOnly(state: AppState, tab: OpenFile, diskContent: string): void {
const draft = state.editorDrafts[tab.id]
if (
draft !== undefined &&
draft !== diskContent &&
// Why: if disk moved at all (even whitespace), keeping the draft would undo an outside change.
tab.lastKnownDiskSignature === getDiskBaselineSignature(diskContent) &&
tab.language === 'markdown' &&
isEditorContentUnchanged(draft, diskContent, true)
) {
return
}
state.clearEditorDraft(tab.id)
}
// 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
@@ -36,10 +54,11 @@ export function stampCleanTabDiskBaseline(id: string, result: FileContent): void
}
if (loadedFile) {
const moved = setEditorDiskBaseline(state, id, getDiskBaselineSignature(result.content))
// Why: the model is clean, so any draft left on its tabs is stale text from an older version.
state.clearEditorDraft(id)
// Why: the model is clean, so any draft left on its tabs is stale text from an older version,
// except a Markdown draft that differs from this disk text only in trailing whitespace.
clearDraftUnlessWhitespaceOnly(state, loadedFile, result.content)
for (const tab of moved) {
state.clearEditorDraft(tab.id)
clearDraftUnlessWhitespaceOnly(state, tab, result.content)
notifyEditorFileCacheStale(tab.id)
}
}
@@ -7,6 +7,7 @@ import { __clearSelfWriteRegistryForTests } from './editor-self-write-registry'
import { createEditorStore } from './editor-autosave-controller-test-fixture'
import { reloadTabContentFromDisk } from './ExternalFileChangeBanner'
import { mayPushTabTextIntoModel } from './editor-stale-mount-guard'
import { stampCleanTabDiskBaseline } from './editor-clean-tab-disk-baseline'
import { rollBackFailedMobileSave } from '@/runtime/mobile-markdown-save-rollback'
import { getDiskBaselineSignature } from '../../../../shared/disk-baseline-signature'
import { GUARDED_PATH, stubGuardedDisk } from './editor-save-guarded-disk-fixture'
@@ -19,14 +20,14 @@ vi.mock('sonner', () => ({ toast: Object.assign(vi.fn(), { error: vi.fn() }) }))
vi.mock('@/lib/connection-context', () => ({ getConnectionIdForFile: () => undefined }))
vi.mock('@/store', () => ({ useAppStore: { getState: () => storeRef.current?.getState() } }))
function openCleanTab(disk: string) {
function openCleanTab(disk: string, language = 'typescript') {
const store = createEditorStore()
storeRef.current = store
store.getState().openFile({
filePath: GUARDED_PATH,
relativePath: 'file.ts',
worktreeId: 'wt-1',
language: 'typescript',
language,
mode: 'edit'
})
const id = store.getState().openFiles[0].id
@@ -112,3 +113,21 @@ it("never restores a clean tab's leftover draft into the model", () => {
store.getState().markFileDirty(id, true)
expect(mayPushTabTextIntoModel(id)).toBe(true)
})
it("a reload keeps a clean Markdown tab's trailing-whitespace draft, but not one over older text", () => {
const { store, id } = openCleanTab('# notes', 'markdown')
store.getState().setEditorDraft(id, '# notes\n\n')
stampCleanTabDiskBaseline(id, { content: '# notes', isBinary: false })
expect(store.getState().editorDrafts[id]).toBe('# notes\n\n')
stampCleanTabDiskBaseline(id, { content: '# newer notes', isBinary: false })
expect(store.getState().editorDrafts[id]).toBeUndefined()
})
it('a reload drops a whitespace-only Markdown draft when disk moved, even by whitespace alone', () => {
const { store, id } = openCleanTab('# notes', 'markdown')
store.getState().setEditorDraft(id, '# notes\n\n')
// An outside write added a trailing newline; keeping the draft would undo it.
stampCleanTabDiskBaseline(id, { content: '# notes\n', isBinary: false })
expect(store.getState().editorDrafts[id]).toBeUndefined()
})
@@ -339,6 +339,48 @@ describe('two tabs sharing one model', () => {
expect(tab(b.id)?.externalMutation).toBeUndefined()
})
it('Undo of Reload yields to typing in a workspace tab that joined the model after the reload', () => {
const { store, a, b, edit } = openSharedTabs('A')
store.getState().setEditorViewMode(b.id, 'changes')
edit(a.id, 'mine')
const toastMock = vi.mocked(toast)
toastMock.mockClear()
reloadTabContentFromDisk(store.getState().openFiles[0], vi.fn())
// The other workspace's tab switches to the code view, joining the model, and types.
store.getState().setEditorViewMode(b.id, 'edit')
edit(b.id, 'typed in the joined tab')
const undo: unknown = toastMock.mock.calls[0]?.[1]?.action
expect(isClickAction(undo)).toBe(true)
if (isClickAction(undo)) {
undo.onClick()
}
expect(store.getState().editorDrafts[a.id]).toBeUndefined()
expect(store.getState().editorDrafts[b.id]).toBe('typed in the joined tab')
})
it('Undo of Reload leaves alone a reloaded tab that left the model and typed', () => {
const { store, a, b, edit, tab } = openSharedTabs('A')
edit(a.id, 'mine')
const toastMock = vi.mocked(toast)
toastMock.mockClear()
reloadTabContentFromDisk(store.getState().openFiles[0], vi.fn())
// The other workspace's tab switches to the Changes view, leaving the model, and types.
store.getState().setEditorViewMode(b.id, 'changes')
edit(b.id, 'typed in the Changes view')
const undo: unknown = toastMock.mock.calls[0]?.[1]?.action
expect(isClickAction(undo)).toBe(true)
if (isClickAction(undo)) {
undo.onClick()
}
expect(store.getState().editorDrafts[a.id]).toBe('mine')
expect(store.getState().editorDrafts[b.id]).toBe('typed in the Changes view')
expect(tab(b.id)?.externalMutation).toBeUndefined()
})
it('never reaches a Changes-view tab through Keep or Reload', async () => {
stubGuardedDisk({ [GUARDED_PATH]: 'B' })
const { store, a, b, edit, tab } = openSharedTabs('A')
@@ -17,18 +17,25 @@ export function isStaleCachedEditorText(fileId: string, content: string): boolea
return baseline !== undefined && getDiskBaselineSignature(content) !== baseline
}
// Why keyed by baseline too: once the baseline moves, the same stale text must be able to reload again.
const requestedReloads = new Map<string, string>()
/** Drops a stale cache so it reloads from disk, once per cached text: a reload that lands the same text can't loop. */
/** Drops a stale cache so it reloads from disk, once per cached text and baseline: a reload that lands the same text can't loop. */
export function requestStaleCacheReload(fileId: string, content: string): void {
const signature = getDiskBaselineSignature(content)
if (requestedReloads.get(fileId) === signature) {
const file = useAppStore.getState()?.openFiles?.find((openFile) => openFile.id === fileId)
const stamp = `${file?.lastKnownDiskSignature ?? ''}\n${getDiskBaselineSignature(content)}`
if (requestedReloads.get(fileId) === stamp) {
return
}
requestedReloads.set(fileId, signature)
requestedReloads.set(fileId, stamp)
notifyEditorFileCacheStale(fileId)
}
/** A mount found the tab's text fresh, so an earlier reload settled; a later stale copy may reload again. */
export function settleStaleCacheReload(fileId: string): void {
requestedReloads.delete(fileId)
}
/**
* The one rule for pushing a tab's text into its live model: a clean tab's text (disk-derived, or a
* leftover draft) never enters a dirty model. Reload and Overwrite clear the model's edits first.
@@ -9,6 +9,7 @@ import type { MonacoEditorMountParams } from './monaco-editor-mount-params'
import { useMonacoEditorMount } from './use-monaco-editor-mount'
const filePath = '/repo/file.ts'
const disk = vi.hoisted(() => ({ text: 'B' }))
vi.mock('@/store', () => ({
useAppStore: {
@@ -22,7 +23,7 @@ vi.mock('@/store', () => ({
id: 'tab-b',
filePath: '/repo/file.ts',
mode: 'edit',
lastKnownDiskSignature: getDiskBaselineSignature('B')
lastKnownDiskSignature: getDiskBaselineSignature(disk.text)
}
]
})
@@ -39,9 +40,12 @@ vi.mock('./monaco-editor-input-bindings', () => ({
installMonacoEditorInputBindings: () => ({ disposeInputBindings: vi.fn() })
}))
afterEach(cleanup)
afterEach(() => {
cleanup()
disk.text = 'B'
})
it('mounts the live model over a stale retained cache, editable, and reloads a model-less stale cache once', () => {
function setupMount() {
const params: MonacoEditorMountParams = {
fileId: 'tab-b',
filePath,
@@ -96,6 +100,7 @@ it('mounts the live model over a stale retained cache, editable, and reloads a m
const modelText = { value: 'B' }
const model = {
getValue: () => modelText.value,
getEOL: () => '\n',
pushEditOperations: vi.fn(),
getFullModelRange: vi.fn()
}
@@ -117,6 +122,12 @@ it('mounts the live model over a stale retained cache, editable, and reloads a m
const editorInstance = mockEditor as unknown as editor.IStandaloneCodeEditor
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The mocked mount integrations do not access Monaco.
const monaco = {} as Parameters<OnMount>[1]
return { params, modelText, model, updateOptions, executeEdits, editorInstance, monaco }
}
it('mounts the live model over a stale retained cache, editable, and reloads a model-less stale cache once', () => {
const { params, modelText, model, updateOptions, executeEdits, editorInstance, monaco } =
setupMount()
const dropped: string[] = []
const onStale = (event: Event): void => {
if (event instanceof CustomEvent) {
@@ -146,3 +157,38 @@ it('mounts the live model over a stale retained cache, editable, and reloads a m
window.removeEventListener(ORCA_EDITOR_FILE_CACHE_STALE_EVENT, onStale)
}
})
it('reloads the same stale text again once the baseline moves or a fresh mount settles the reload', () => {
const { params, modelText, editorInstance, monaco } = setupMount()
const dropped: string[] = []
const onStale = (event: Event): void => {
if (event instanceof CustomEvent) {
dropped.push(event.detail.fileId)
}
}
window.addEventListener(ORCA_EDITOR_FILE_CACHE_STALE_EVENT, onStale)
const view = renderHook(() => useMonacoEditorMount(params))
const mountWith = (text: string): void => {
params.contentSync.contentRef.current = text
modelText.value = text
view.result.current(editorInstance, monaco)
}
try {
mountWith('A2 (stale cache)')
mountWith('A2 (stale cache)')
expect(dropped).toHaveLength(1)
// Another write moved the baseline: the same stale text must reload again.
disk.text = 'C'
mountWith('A2 (stale cache)')
expect(dropped).toHaveLength(2)
// The reload landed fresh text; stale text returning later reloads again.
mountWith('C')
expect(dropped).toHaveLength(2)
mountWith('A2 (stale cache)')
expect(dropped).toHaveLength(3)
} finally {
window.removeEventListener(ORCA_EDITOR_FILE_CACHE_STALE_EVENT, onStale)
}
})
@@ -22,7 +22,8 @@ import {
import {
isStaleCachedEditorText,
mayPushTabTextIntoModel,
requestStaleCacheReload
requestStaleCacheReload,
settleStaleCacheReload
} from './editor-stale-mount-guard'
// Why: binds a freshly created Monaco instance to the app; reads everything through refs so the callback identity stays stable across renders.
@@ -112,6 +113,9 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount {
// Why: see contentRef — reconcile the retained model to the current prop before user interaction (surfaces edits made while unmounted).
const modelKey = toEditorModelUri(filePath, modelOwnerKey)
const staleCache = isStaleCachedEditorText(fileIdRef.current, contentRef.current)
if (!staleCache) {
settleStaleCacheReload(fileIdRef.current)
}
const liveTextDiffers = editorInstance.getModel()?.getValue() !== contentRef.current
if (
(staleCache && liveTextDiffers) ||