Plugins: binary editors keep unsaved edits across renames, reloads and twin tabs (#35698)

* fix(plugins): keep binary editor edits across renames, reloads and twin tabs

- Renaming a binary file with unsaved plugin-editor edits gave its tab a new
  id, and the bytes store took the old id's disappearance as a close and
  dropped the edit. The bytes now move with the tab as the rename lands.
- Tabs of one file share one dirty flag, so they now share one bytes draft;
  before, saving one tab marked both clean and dropped the other's edit.
- A reload that reads the same bytes keeps the document, so the editor is not
  sent a new version that turns an edit in flight into a conflict.

* fix(plugins): finish a twin-tab bytes save through any tab still open

A tab that closed mid-save left its twin with the old hash, and a twin
adopting a shared draft no longer lets its own older read replace it.

* fix(plugins): hand a closed tab's bytes draft to its open twin; drop stale reads

A twin that had not shown the file yet lost the draft when the tab holding it
closed, and a read that started before an edit and save could replace the
newer saved bytes. Boxes now count edits and saves; a read only lands if none
happened since it started.

src/renderer/src/components/editor/PluginBytesCustomEditorSurface.test.tsx

@@ -20,7 +20,9 @@ vi.mock('@/runtime/runtime-paged-file-read', () => ({
 const store = vi.hoisted(() => ({
   markFileDirty: vi.fn(),
   openFiles: [{ id: 'book', isDirty: false }],
-  listeners: new Set<(state: unknown) => void>()
+  editorBufferIdOf: {},
+  editorBufferViews: {},
+  listeners: new Set<(state: unknown, previous: unknown) => void>()
 }))
 vi.mock('@/store', () => ({
   useAppStore: {
@@ -75,7 +77,7 @@ beforeEach(() => {
 afterEach(async () => {
   // Closing the tab drops its bytes document.
   for (const listener of store.listeners) {
-    listener({ openFiles: [] })
+    listener({ openFiles: [] }, store)
   }
   await act(async () => root.unmount())
   container.remove()

src/renderer/src/components/editor/PluginBytesCustomEditorSurface.tsx

@@ -14,6 +14,7 @@ import PluginCustomEditorFrame from './PluginCustomEditorFrame'
 import { ExternalFileChangeBanner } from './ExternalFileChangeBanner'
 import {
   editorBytesDocument,
+  editorBytesEdits,
   hasEditorBytesDraft,
   loadEditorBytes,
   releaseEditorBytes,
@@ -99,6 +100,7 @@ export default function PluginBytesCustomEditorSurface({
       return
     }
     let cancelled = false
+    const editsAtRead = editorBytesEdits(fileId)
     readRuntimeFileBytes(source, CUSTOM_EDITOR_DOCUMENT_MAX_BYTES)
       .then(async (read) => {
         if (cancelled) {
@@ -107,7 +109,7 @@ export default function PluginBytesCustomEditorSurface({
         if (read.kind === 'ok') {
           const sha256 = await sha256Hex(read.bytes)
           if (!cancelled) {
-            loadEditorBytes(fileId, { bytes: read.bytes, fileType }, sha256)
+            loadEditorBytes(fileId, { bytes: read.bytes, fileType }, sha256, editsAtRead)
             setState({ status: 'ok' })
           }
         } else if (read.kind === 'too_large') {

src/renderer/src/components/editor/editor-bytes-document-boxes.ts

@@ -0,0 +1,35 @@
+import type { CustomEditorBinaryDocument } from './plugin-custom-editor-host'
+
+/**
+ * A binary custom editor's bytes last read or saved, and the unsaved edit. Kept apart from the
+ * text drafts so a text save can never write bytes as text; not kept across restarts.
+ */
+export type BytesDocument = {
+  saved: CustomEditorBinaryDocument
+  savedSha256: string
+  draft: CustomEditorBinaryDocument | null
+}
+
+// Why a shared box: tabs of one file share one dirty flag, so they must share one draft too.
+// `edits` counts edits and saves, so a read that started before one can be told apart.
+export type BytesDocumentBox = { current: BytesDocument; edits: number }
+
+/** Per tab id. Its own module so the store's rename can move entries without importing the store. */
+export const bytesDocumentBoxes = new Map<string, BytesDocumentBox>()
+
+/** A rename gives tabs new ids; their bytes, unsaved edits included, move with them. */
+export function rekeyEditorBytesDocuments(
+  rekeys: readonly { oldFileId: string; newFileId: string }[]
+): void {
+  const moved = rekeys.flatMap(({ oldFileId, newFileId }) => {
+    const box = bytesDocumentBoxes.get(oldFileId)
+    return box ? [{ oldFileId, newFileId, box }] : []
+  })
+  // Why two passes: a swap of two ids must not delete the entry it just wrote.
+  for (const { oldFileId } of moved) {
+    bytesDocumentBoxes.delete(oldFileId)
+  }
+  for (const { newFileId, box } of moved) {
+    bytesDocumentBoxes.set(newFileId, box)
+  }
+}

src/renderer/src/components/editor/editor-bytes-documents.test.ts

@@ -8,8 +8,12 @@ const state = vi.hoisted(() => {
     { id: 'book', isDirty: false }
   ]
   const tab = (id: string) => openFiles.find((file) => file.id === id)
+  const editorBufferIdOf: Record<string, string> = {}
+  const editorBufferViews: Record<string, string[]> = {}
   return {
     openFiles,
+    editorBufferIdOf,
+    editorBufferViews,
     markFileDirty: vi.fn((id: string, dirty: boolean) => {
       const found = tab(id)
       if (found) {
@@ -24,7 +28,19 @@ const state = vi.hoisted(() => {
     })
   }
 })
-vi.mock('@/store', () => ({ useAppStore: { subscribe: () => () => {}, getState: () => state } }))
+const storeListener = vi.hoisted(() => {
+  const holder: { current: ((next: unknown, prev: unknown) => void) | null } = { current: null }
+  return holder
+})
+vi.mock('@/store', () => ({
+  useAppStore: {
+    subscribe: (listener: (next: unknown, prev: unknown) => void) => {
+      storeListener.current = listener
+      return () => {}
+    },
+    getState: () => state
+  }
+}))
 const writeRuntimeFileBytes = vi.fn()
 vi.mock('@/runtime/runtime-file-bytes-write', () => ({
   writeRuntimeFileBytes: (...args: unknown[]) => writeRuntimeFileBytes(...args)
@@ -32,13 +48,14 @@ vi.mock('@/runtime/runtime-file-bytes-write', () => ({
 
 import {
   editorBytesDocument,
+  editorBytesEdits,
   loadEditorBytes,
   releaseEditorBytes,
   retainEditorBytes,
   saveEditorBytesDraft,
-  setEditorBytesDraft,
-  sha256Hex
+  setEditorBytesDraft
 } from './editor-bytes-documents'
+import { bytesDocumentBoxes, rekeyEditorBytesDocuments } from './editor-bytes-document-boxes'
 
 // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the save reads only openFiles, markFileDirty and setExternalMutation.
 const getState = () => state as unknown as AppState
@@ -49,12 +66,14 @@ const save = () => saveEditorBytesDraft(getState, file, context, 'user', () => f
 const loaded = new Uint8Array([0x50, 0x4b, 1])
 const edited = { bytes: new Uint8Array([0x50, 0x4b, 2, 0xff]), fileType: 'xlsx' }
 const hex = (bytes: Uint8Array) => createHash('sha256').update(bytes).digest('hex')
+const load = (id: string, bytes: Uint8Array<ArrayBuffer>, editsAtRead = editorBytesEdits(id)) =>
+  loadEditorBytes(id, { bytes, fileType: 'xlsx' }, hex(bytes), editsAtRead)
 
 beforeEach(async () => {
   writeRuntimeFileBytes.mockReset()
   state.openFiles[0].isDirty = false
   state.openFiles[0].externalMutation = undefined
-  loadEditorBytes('book', { bytes: loaded, fileType: 'xlsx' }, await sha256Hex(loaded))
+  load('book', loaded)
 })
 
 describe('binary custom editor saves', () => {
@@ -112,7 +131,7 @@ describe('binary custom editor views', () => {
   it('keeps an edit guarded by the bytes it started from when a later read lands', async () => {
     setEditorBytesDraft('book', edited)
     const newer = new Uint8Array([9])
-    loadEditorBytes('book', { bytes: newer, fileType: 'xlsx' }, await sha256Hex(newer))
+    load('book', newer)
     writeRuntimeFileBytes.mockResolvedValue({ ok: true })
     await save()
     expect(writeRuntimeFileBytes.mock.calls[0]?.[4]).toEqual({
@@ -121,3 +140,62 @@ describe('binary custom editor views', () => {
     })
   })
 })
+
+describe('binary custom editor reloads', () => {
+  it('keeps the same document when a reload reads unchanged bytes', async () => {
+    const before = editorBytesDocument('book')
+    const reread = new Uint8Array(loaded)
+    load('book', reread)
+    // A new object would resend the document and turn the frame's in-flight edit into a conflict.
+    expect(editorBytesDocument('book')).toBe(before)
+  })
+})
+
+describe('binary custom editor tabs', () => {
+  it('ignores a read that started before an edit and save it does not include', async () => {
+    const editsAtRead = editorBytesEdits('book')
+    setEditorBytesDraft('book', edited)
+    writeRuntimeFileBytes.mockResolvedValue({ ok: true })
+    await save()
+    load('book', loaded, editsAtRead)
+    expect(editorBytesDocument('book')).toBe(edited)
+  })
+
+  it('moves an unsaved edit with its tab when the file is renamed', () => {
+    setEditorBytesDraft('book', edited)
+    rekeyEditorBytesDocuments([{ oldFileId: 'book', newFileId: 'renamed' }])
+    expect(editorBytesDocument('renamed')).toBe(edited)
+    expect(editorBytesDocument('book')).toBeNull()
+    rekeyEditorBytesDocuments([{ oldFileId: 'renamed', newFileId: 'book' }])
+  })
+
+  it('shares one draft between tabs of one file, which share one dirty flag', async () => {
+    state.editorBufferIdOf = { book: 'record', twin: 'record' }
+    state.editorBufferViews = { record: ['book', 'twin'] }
+    state.openFiles.push({ id: 'twin', isDirty: false })
+    load('twin', new Uint8Array(loaded))
+    setEditorBytesDraft('twin', edited)
+    expect(editorBytesDocument('book')).toBe(edited)
+    // The saving tab closes mid-write; its twin still ends up clean against the written bytes.
+    writeRuntimeFileBytes.mockImplementation(async () => {
+      bytesDocumentBoxes.delete('book')
+      return { ok: true }
+    })
+    await save()
+    expect(state.markFileDirty).toHaveBeenLastCalledWith('twin', false, { written: true })
+    bytesDocumentBoxes.set('book', bytesDocumentBoxes.get('twin')!)
+
+    // The tab holding a draft closes before its twin has shown it: the twin inherits the draft.
+    setEditorBytesDraft('book', edited)
+    bytesDocumentBoxes.delete('twin')
+    const previous = { ...state, openFiles: [...state.openFiles] }
+    state.openFiles.splice(0, 1)
+    storeListener.current?.(state, previous)
+    expect(editorBytesDocument('twin')).toBe(edited)
+    state.openFiles.unshift({ id: 'book', isDirty: false })
+    bytesDocumentBoxes.set('book', bytesDocumentBoxes.get('twin')!)
+    state.openFiles.pop()
+    state.editorBufferIdOf = {}
+    state.editorBufferViews = {}
+  })
+})

src/renderer/src/components/editor/editor-bytes-documents.ts

@@ -13,19 +13,9 @@ import {
 } from './editor-self-write-registry'
 import { markEditorSaveRefusal } from './editor-save-rebaseline'
 import { EditorSaveDiscardedError } from './editor-discard-epoch'
+import { editorBufferSiblingViewIds } from '@/store/slices/editor/buffers/editor-buffer-views'
+import { bytesDocumentBoxes } from './editor-bytes-document-boxes'
 
-/**
- * A binary custom editor's document per tab: the bytes last read or saved, and the unsaved edit.
- * Kept here, not in the text drafts, so a text save can never write them as text. Unsaved bytes
- * live only while the tab is dirty and open; they are not kept across restarts.
- */
-type BytesDocument = {
-  saved: CustomEditorBinaryDocument
-  savedSha256: string
-  draft: CustomEditorBinaryDocument | null
-}
-
-const documents = new Map<string, BytesDocument>()
 const listeners = new Set<() => void>()
 let unsubscribeStore: (() => void) | null = null
 
@@ -36,19 +26,27 @@ function notify(): void {
 }
 
 /** Drops a draft once its tab is clean (saved, reverted, discarded) and the entry once it closes. */
-function dropSettledDrafts(state: AppState): void {
+function dropSettledDrafts(state: AppState, previous: AppState): void {
   let changed = false
-  for (const [fileId, document] of documents) {
+  const isOpen = (id: string): boolean => state.openFiles.some((file) => file.id === id)
+  for (const [fileId, box] of bytesDocumentBoxes) {
     const tab = state.openFiles.find((file) => file.id === fileId)
     if (!tab) {
-      documents.delete(fileId)
+      bytesDocumentBoxes.delete(fileId)
+      // Why: a twin still open keeps the record dirty, so it inherits the draft even if not yet shown.
+      const heir = box.current.draft
+        ? editorBufferSiblingViewIds(previous, fileId).find((id) => id !== fileId && isOpen(id))
+        : undefined
+      if (heir !== undefined && !bytesDocumentBoxes.has(heir)) {
+        bytesDocumentBoxes.set(heir, box)
+      }
       changed = true
-    } else if (document.draft && !tab.isDirty) {
-      documents.set(fileId, { ...document, draft: null })
+    } else if (box.current.draft && !tab.isDirty) {
+      box.current = { ...box.current, draft: null }
       changed = true
     }
   }
-  if (documents.size === 0) {
+  if (bytesDocumentBoxes.size === 0) {
     unsubscribeStore?.()
     unsubscribeStore = null
   }
@@ -69,7 +67,7 @@ export function subscribeEditorBytes(listener: () => void): () => void {
 
 /** The document the editor shows: the unsaved edit, else the bytes on disk. */
 export function editorBytesDocument(fileId: string): CustomEditorBinaryDocument | null {
-  const document = documents.get(fileId)
+  const document = bytesDocumentBoxes.get(fileId)?.current
   return document ? (document.draft ?? document.saved) : null
 }
 
@@ -77,21 +75,46 @@ export function editorBytesDocument(fileId: string): CustomEditorBinaryDocument
 export function loadEditorBytes(
   fileId: string,
   saved: CustomEditorBinaryDocument,
-  savedSha256: string
+  savedSha256: string,
+  /** `editorBytesEdits` when the read started. */
+  editsAtRead: number | undefined
 ): void {
-  const existing = documents.get(fileId)
-  // Why keep the draft's baseline: a later read must not vouch for bytes the edit never saw.
-  documents.set(fileId, existing?.draft ? existing : { saved, savedSha256, draft: null })
   unsubscribeStore ??= useAppStore.subscribe(dropSettledDrafts)
-  notify()
+  const own = bytesDocumentBoxes.get(fileId)
+  if (!own) {
+    const siblings = editorBufferSiblingViewIds(useAppStore.getState(), fileId)
+    const shared = siblings.map((id) => bytesDocumentBoxes.get(id)).find(Boolean)
+    // Why adopt as is: the twin's box is newer than a read that may have started before its save.
+    bytesDocumentBoxes.set(
+      fileId,
+      shared ?? { current: { saved, savedSha256, draft: null }, edits: 0 }
+    )
+    notify()
+    return
+  }
+  const existing = own.current
+  // Why: an edit or save since the read started is newer than what the read returns.
+  if (own.edits !== editsAtRead) {
+    return
+  }
+  // Why keep the draft's baseline: a later read must not vouch for bytes the edit never saw.
+  // Why keep unchanged bytes: a new object resends the document, so an edit in flight conflicts.
+  if (
+    !existing.draft &&
+    (existing.savedSha256 !== savedSha256 || existing.saved.fileType !== saved.fileType)
+  ) {
+    own.current = { saved, savedSha256, draft: null }
+    notify()
+  }
 }
 
 export function setEditorBytesDraft(fileId: string, draft: CustomEditorBinaryDocument): void {
-  const document = documents.get(fileId)
-  if (!document) {
+  const box = bytesDocumentBoxes.get(fileId)
+  if (!box) {
     return
   }
-  documents.set(fileId, { ...document, draft })
+  box.current = { ...box.current, draft }
+  box.edits += 1
   useAppStore.getState().markFileDirty(fileId, true)
   notify()
 }
@@ -111,14 +134,18 @@ export function releaseEditorBytes(fileId: string): void {
     return
   }
   mountedViews.delete(fileId)
-  if (documents.get(fileId)?.draft === null) {
-    documents.delete(fileId)
+  if (bytesDocumentBoxes.get(fileId)?.current.draft === null) {
+    bytesDocumentBoxes.delete(fileId)
     notify()
   }
 }
 
+export function editorBytesEdits(fileId: string): number | undefined {
+  return bytesDocumentBoxes.get(fileId)?.edits
+}
+
 export function hasEditorBytesDraft(fileId: string): boolean {
-  return Boolean(documents.get(fileId)?.draft)
+  return Boolean(bytesDocumentBoxes.get(fileId)?.current.draft)
 }
 
 /** Writes the tab's unsaved bytes, guarded by the hash of the bytes the edit started from. */
@@ -130,9 +157,10 @@ export async function saveEditorBytesDraft(
   /** A Don't Save since the request; checked at the last moment before the write. */
   discarded: () => boolean
 ): Promise<EditorSaveResult> {
-  const document = documents.get(file.id)
+  const box = bytesDocumentBoxes.get(file.id)
+  const document = box?.current
   const draft = document?.draft
-  if (!document || !draft) {
+  if (!box || !document || !draft) {
     return 'written'
   }
   const remote = Boolean(context.connectionId || file.runtimeEnvironmentId?.trim())
@@ -172,15 +200,17 @@ export async function saveEditorBytesDraft(
     return 'blocked-stale'
   }
   const savedSha256 = await sha256Hex(draft.bytes)
-  const current = documents.get(file.id)
-  if (current) {
+  // Why any holder: the saving tab may have closed while a twin of the same file stays open.
+  const holder = [...bytesDocumentBoxes].find(([, held]) => held === box)?.[0]
+  if (holder !== undefined) {
     // An edit made while the write ran stays unsaved, now against the bytes just written.
-    const newer = current.draft !== draft ? current.draft : null
-    documents.set(file.id, { saved: draft, savedSha256, draft: newer })
+    const newer = box.current.draft !== draft ? box.current.draft : null
+    box.current = { saved: draft, savedSha256, draft: newer }
+    box.edits += 1
     const state = getState()
-    state.markFileDirty(file.id, newer !== null, { written: true })
-    if (state.openFiles.find((tab) => tab.id === file.id)?.externalMutation) {
-      state.setExternalMutation(file.id, null)
+    state.markFileDirty(holder, newer !== null, { written: true })
+    if (state.openFiles.find((tab) => tab.id === holder)?.externalMutation) {
+      state.setExternalMutation(holder, null)
     }
     notify()
   }

src/renderer/src/components/editor/editor-panel-render-model.ts

@@ -140,7 +140,7 @@ export function getEditorPanelRenderModel({
   // Read-only tabs keep raw source, as for every other rich view.
   const textEditorEligible =
     customEditorContentReady && !rawReadOnly && inlineFileContent?.isBinary !== true
-  // Bytes editors are read-only, so they also view read-only tabs; they need a pageable route.
+  // Bytes editors honor read-only themselves, so they open read-only tabs too; they need a pageable route.
   const bytesEditorEligible =
     customEditorContentReady && inlineFileContent?.byteSource !== undefined
   const eligibleCustomEditors = customEditors.filter((editor) =>

src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts

@@ -7,6 +7,7 @@ import { rekeyEditorBuffers } from '../buffers/editor-buffer-record'
 import { migrateHydratedEditorTabsAndGroups } from '../file-ids/hydrated-editor-file-ids'
 import { renameEditorFilesThroughLayoutOwner } from '../tabs/editor-file-rename-through-layout-owner'
 import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../../shared/layout-owner-switch'
+import { rekeyEditorBytesDocuments } from '@/components/editor/editor-bytes-document-boxes'
 
 // The store change is pure, so a layout-owner build applies it in the frame that shows main's rename.
 export type RekeyOpenFilesPatch =
@@ -168,6 +169,7 @@ export function createRekeyOpenFilesAction(
           result = next
           return s
         }
+        rekeyEditorBytesDocuments(rekeys)
         return next.patch
       })
       return result

src/renderer/src/store/slices/editor/tabs/editor-file-rename-through-layout-owner.ts

@@ -15,6 +15,7 @@ import type { OpenFilePathRekey, RekeyOpenFilesResult } from '../types/open-file
 import { isEditorTabContentType } from './editor-tab-content-type'
 import { restorableEditorFile } from './editor-tab-layout-owner'
 import { layoutEditorViewOfOpenFile } from './open-file-layout-view'
+import { rekeyEditorBytesDocuments } from '@/components/editor/editor-bytes-document-boxes'
 
 /** Main's renames: an edit-mode file's record moves from its old path; a view takes the new one. */
 function renamesOf(
@@ -81,7 +82,12 @@ export function renameEditorFilesThroughLayoutOwner(
   const after = new Map((planned.patch.openFiles ?? []).map((file) => [file.id, file]))
   const rekeyHere = (own: readonly OpenFilePathRekey[]) => (s: AppState) => {
     const next = rekeyOpenFilesPatch(s, own, moveOperationId)
-    return next.ok ? next.patch : {}
+    if (!next.ok) {
+      return {}
+    }
+    // Why here: this runs as the rename lands, so the old tab's close never frees its bytes.
+    rekeyEditorBytesDocuments(own)
+    return next.patch
   }
   // Each file's own workspace: the same path can be open in more than one.
   const byWorktree = new Map<string, OpenFilePathRekey[]>()