Editor tab state is written to its buffer record, not copied into it afterwards (phase 1a) (#31251)

* refactor(editor): make the editor buffer record the source of truth for tab buffer state

Writers of the six buffer fields read and write the record, with the OpenFile
fields written as a mirror in the same store update. The store middleware no
longer copies tab values into the record; it only keeps records keyed by open
tabs (seeded from the tab on create, dropped on close) and, in dev and test
builds, asserts every mirror field equals its record. Host-mirrored web tabs
adopt the host's published values explicitly in the session sync patch.

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor(editor): check the buffer invariant in e2e builds and tidy the record helpers

Co-Authored-By: Claude <noreply@anthropic.com>

* test(e2e): give session-sync fixtures the now-required editorBuffers

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Neil
2026-10-20 16:01:30 -07:00
committed by GitHub
co-authored by Claude
parent f812c67a05
commit ebc8b06057
24 changed files with 133 additions and 138 deletions
@@ -151,13 +151,7 @@ describe('editor autosave changed-on-disk conflict flow', () => {
// 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)
store.getState().setPendingDiskBaselineVerification('/repo/file.ts', true)
const cleanup = attachEditorAutosaveController(withLoadedBaselines(store))
try {
@@ -47,13 +47,7 @@ function openRestoredDirtyTab(
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)
store.getState().setPendingDiskBaselineVerification(filePath, true)
}
describe('attachRestoredTabConflictScan', () => {
@@ -141,7 +141,8 @@ describe('floating titlebar close actions', () => {
it('routes a dirty last reference through the shared save confirmation', () => {
useAppStore.setState({
unifiedTabsByWorktree: { [worktreeId]: [editor] },
openFiles: [makeOpenFile({ id: sharedFileId, worktreeId, isDirty: true })]
openFiles: [makeOpenFile({ id: sharedFileId, worktreeId, isDirty: true })],
editorBuffers: {}
})
actions().closeFloatingItemConfirmed(editor.id)
@@ -64,7 +64,8 @@ function requestTerminalClose(): { land: () => void } {
function floatingNote({ isDirty, isPinned }: { isDirty: boolean; isPinned: boolean }): Tab {
useAppStore.setState({
openFiles: [makeOpenFile({ id: 'note', worktreeId: FLOATING_TERMINAL_WORKTREE_ID, isDirty })]
openFiles: [makeOpenFile({ id: 'note', worktreeId: FLOATING_TERMINAL_WORKTREE_ID, isDirty })],
editorBuffers: {}
})
return { ...terminalTab, id: 'unified-note', entityId: 'note', contentType: 'editor', isPinned }
}
@@ -40,6 +40,7 @@ function emptyState(overrides: Partial<WebSessionTabsSyncState>): WebSessionTabs
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -112,6 +112,7 @@ function createSnapshot(): WebSessionTabsSyncState {
}
},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: { [TERMINAL_ID]: ['pty-1'] },
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: { [WORKTREE_ID]: [TERMINAL_ID, STRUCTURED_ID] },
@@ -102,6 +102,7 @@ function hydratedState(): WebSessionTabsSyncState {
}
},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: { [TERMINAL_ID]: ['pty-1'] },
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: { [WORKTREE_ID]: [TERMINAL_ID, CLAUDE_TAB, CODEX_TAB] },
@@ -54,6 +54,7 @@ function makeState(overrides: Partial<WebSessionTabsSyncState> = {}): WebSession
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -60,6 +60,7 @@ function baseState(overrides: Partial<WebSessionTabsSyncState>): WebSessionTabsS
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -30,7 +30,7 @@ describe('web session structured tab focus', () => {
it('keeps the exact active structured tab across a host snapshot', () => {
const first = structuredTab('session-1', 0)
const second = structuredTab('session-2', 1)
const state = {
const state: WebSessionTabsSyncState = {
activeBrowserTabId: null,
activeBrowserTabIdByWorktree: {},
activeFileId: null,
@@ -58,6 +58,7 @@ describe('web session structured tab focus', () => {
},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: { [WORKTREE_ID]: [first.id, second.id] },
@@ -66,7 +67,7 @@ describe('web session structured tab focus', () => {
unifiedTabsByWorktree: { [WORKTREE_ID]: [first, second] },
unreadTerminalTabs: {},
sortEpoch: 0
} as WebSessionTabsSyncState
}
const snapshot: RuntimeMobileSessionTabsResult = {
worktree: WORKTREE_ID,
publicationEpoch: 'epoch-1',
@@ -70,6 +70,7 @@ function makeState(overrides: Partial<WebSessionTabsSyncState> = {}): WebSession
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -59,6 +59,7 @@ export function makeState(
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -7,6 +7,7 @@ import {
buildRetractedMirroredTabSweepPatch
} from './agent-status-primitives'
import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence'
import { adoptMirroredEditorBuffers } from '../../store/slices/editor/buffers/editor-buffer-record'
type FinalPatchContext = ReturnType<typeof applyActiveStateUpdates>
@@ -95,8 +96,12 @@ export function buildWebSessionTabsFinalPatch(
...agentStatusPatch,
...retractedTabSweepPatch,
...remirroredClosedTabLiftPatch,
// Why no editorBuffers: the store's buffer middleware adopts the host's mirrored tab state.
...(nextOpenFiles !== state.openFiles ? { openFiles: nextOpenFiles } : {}),
...(nextOpenFiles !== state.openFiles
? {
openFiles: nextOpenFiles,
editorBuffers: adoptMirroredEditorBuffers(state.editorBuffers, nextOpenFiles)
}
: {}),
...(nextTabsByWorktree !== state.tabsByWorktree ? { tabsByWorktree: nextTabsByWorktree } : {}),
...(nextBrowserTabsByWorktree !== state.browserTabsByWorktree
? { browserTabsByWorktree: nextBrowserTabsByWorktree }
@@ -235,6 +235,7 @@ export type WebSessionTabsSyncState = Pick<
| 'browserPagesByWorkspace'
| 'browserCertificateFailuresByPageId'
| 'browserTabsByWorktree'
| 'editorBuffers'
| 'groupsByWorktree'
| 'layoutByWorktree'
| 'openFiles'
@@ -5,7 +5,10 @@ import {
assertEditorBufferInvariant,
withEditorBufferRecords
} from './editor-buffer-record-lifecycle'
import type { EditorBufferRecords } from './slices/editor/buffers/editor-buffer-record'
import {
adoptMirroredEditorBuffers,
type EditorBufferRecords
} from './slices/editor/buffers/editor-buffer-record'
import type { OpenFile } from './slices/editor/types/open-file'
import { folderWorkspaceKey } from '../../../shared/workspace-scope'
@@ -206,18 +209,25 @@ describe('editor buffer records', () => {
})
})
it("adopts a host republish of an open tab's buffer state, as remote session sync writes it", () => {
it("adopts a host republish of a mirrored tab's buffer state, as remote session sync writes it", () => {
const id = openEditTab()
useAppStore.setState((state) => ({
openFiles: state.openFiles.map((file) =>
file.id === id ? { ...file, isDirty: true, lastKnownDiskSignature: 'host' } : file
)
}))
const republish = (mirroredFromRuntimeSession: true | undefined): void =>
useAppStore.setState((state) => {
const openFiles = state.openFiles.map((file) =>
file.id === id ? { ...file, isDirty: !file.isDirty, mirroredFromRuntimeSession } : file
)
return {
openFiles,
editorBuffers: adoptMirroredEditorBuffers(state.editorBuffers, openFiles)
}
})
expect(useAppStore.getState().editorBuffers[id]).toMatchObject({
isDirty: true,
lastKnownDiskSignature: 'host'
})
// Why: only host-owned tabs adopt; a local tab's buffer state is written through its record.
expect(() => republish(undefined)).toThrow(/"isDirty" of/)
republish(true)
expect(useAppStore.getState().editorBuffers[id]?.isDirty).toBe(true)
expect(fileOf(id)?.isDirty).toBe(true)
})
it('rejects a slice write that changes a mirror field without its record', () => {
@@ -1,46 +1,28 @@
import type { StateCreator } from 'zustand'
import type { OpenFile } from './slices/editor/types/open-file'
import {
editorBufferMirrorMismatch,
editorBufferRecordFromFile,
reconcileEditorBufferMembership,
type EditorBufferRecords
type EditorBufferState
} from './slices/editor/buffers/editor-buffer-record'
type EditorBufferState = {
openFiles: OpenFile[]
editorBuffers: EditorBufferRecords
}
/**
* Ties editor buffer records to `openFiles` membership inside the same write, so open, hydrate,
* close and teardown never need their own record bookkeeping. Slice writes must route the six
* buffer fields through the record; dev and test builds throw when a tab mirror drifts from it.
* Ties editor buffer records to `openFiles` membership inside the same write: a new tab's record
* is seeded from the tab and a closed tab's record is dropped. Field writes go through the record
* (`writeEditorBuffer`); with `checkInvariant` a write that drifts a tab mirror from it throws.
*/
function reconcileEditorBuffers<TState extends EditorBufferState>(
state: TState,
write: TState | Partial<TState>,
adoptMirrorDrift: boolean
checkInvariant: boolean
): TState | Partial<TState> {
const openFiles = write.openFiles ?? state.openFiles
const records = write.editorBuffers ?? state.editorBuffers
if (openFiles === state.openFiles && records === state.editorBuffers) {
return write
}
let next = reconcileEditorBufferMembership(records, openFiles)
for (const file of openFiles) {
const field = editorBufferMirrorMismatch(file, next[file.id])
if (field === null) {
continue
}
// Why adopt outside dev: a missed writer must keep main's behaviour, not revert the tab.
if (!adoptMirrorDrift && import.meta.env.DEV) {
throw new Error(
`Editor buffer mirror "${field}" of ${file.id} was written without its record`
)
}
next = next === records ? { ...records } : next
next[file.id] = editorBufferRecordFromFile(file)
const next = reconcileEditorBufferMembership(records, openFiles)
if (checkInvariant) {
assertEditorBufferInvariant({ openFiles, editorBuffers: next })
}
return next === records ? write : { ...write, editorBuffers: next }
}
@@ -60,38 +42,30 @@ export function assertEditorBufferInvariant(state: EditorBufferState): void {
}
export function withEditorBufferRecords<TState extends EditorBufferState>(
createState: StateCreator<TState, [], []>
createState: StateCreator<TState, [], []>,
checkInvariant: boolean = import.meta.env.DEV
): StateCreator<TState, [], []> {
return (set, get, api) => {
// Why external writes adopt: `useAppStore.setState` is the fixture/seed path, like hydrate.
const wrap = (write: typeof set, adoptMirrorDrift: boolean): typeof set => {
const wrap = (write: typeof set): typeof set => {
const wrapped = (
partial: TState | Partial<TState> | ((state: TState) => TState | Partial<TState>),
replace?: boolean
): void => {
const before = get()
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: forwards the caller's own partial/replace pair; with replace=true the updater still yields a full state.
;(write as (next: (state: TState) => TState | Partial<TState>, replace?: boolean) => void)(
(state) =>
reconcileEditorBuffers(
state,
typeof partial === 'function' ? partial(state) : partial,
adoptMirrorDrift
checkInvariant
),
replace
)
const after = get()
if (
import.meta.env.DEV &&
(after.openFiles !== before.openFiles || after.editorBuffers !== before.editorBuffers)
) {
assertEditorBufferInvariant(after)
}
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: one implementation serves both zustand setState overloads.
return wrapped as typeof set
}
api.setState = wrap(api.setState, true)
return createState(wrap(set, false), get, api)
api.setState = wrap(api.setState)
return createState(wrap(set), get, api)
}
}
+3 -1
View File
@@ -78,6 +78,7 @@ const withDevelopmentStoreProbes = (createState: StateCreator<AppState, [], []>)
: createState
// Why buffer records outermost: the probes must see each caller's own partial and frame.
// Why e2e checks the buffer invariant too: a writer that skips the record must fail there, not drift.
export const useAppStore = create<AppState>()(
withEditorBufferRecords(
withDevelopmentStoreProbes(
@@ -140,7 +141,8 @@ export const useAppStore = create<AppState>()(
...createWorkspaceAttentionSlice(...a)
}
})
)
),
import.meta.env.DEV || e2eConfig.exposeStore
)
)
@@ -205,11 +205,7 @@ describe('rekeyOpenFilesForPathChange', () => {
const oldId = useAppStore.getState().openFiles[0]!.id
// A dirty tab that already conflicts with disk: autosave is suspended by the
// banner; gating it would strand the gate (verification skips a 'changed' tab).
useAppStore.setState((s) => ({
openFiles: s.openFiles.map((f) =>
f.id === oldId ? { ...f, externalMutation: 'changed' as const } : f
)
}))
useAppStore.getState().setExternalMutation(oldId, 'changed')
useAppStore.getState().rekeyOpenFilesForPathChange({
rekeys: [rekeyFor(oldId, '/repo/sub/a.md', 'sub/a.md')],
@@ -8,7 +8,11 @@ import {
previewEditorTabsOf,
recordPromotedEditorTabsInLayoutOwner
} from '../tabs/editor-tab-layout-owner'
import { writeEditorBuffer } from '../buffers/editor-buffer-record'
import {
editorBufferOf,
updateEditorBuffer,
writeEditorBuffer
} from '../buffers/editor-buffer-record'
import { getDiskBaselineSignature } from '../../../../../../shared/disk-baseline-signature'
function withoutKey<T>(record: Record<string, T>, key: string): Record<string, T> {
@@ -85,6 +89,7 @@ export function createOpenFileMutations(
if (file.readOnly === true || file.csvPreviewOnly === true) {
return s
}
const buffer = editorBufferOf(s.editorBuffers, file)
const needsPreviewClear = dirty && file.isPreview
// Why: a clean tab holds no draft of the disk text; a kept one could later restore stale
// text into a shared model. A draft that differs from disk (whitespace a Markdown view
@@ -93,9 +98,9 @@ export function createOpenFileMutations(
const dropDraft =
!dirty &&
draft !== undefined &&
(file.lastKnownDiskSignature === undefined ||
getDiskBaselineSignature(draft) === file.lastKnownDiskSignature)
if (file.isDirty === dirty && !needsPreviewClear && !dropDraft) {
(buffer.lastKnownDiskSignature === undefined ||
getDiskBaselineSignature(draft) === buffer.lastKnownDiskSignature)
if (buffer.isDirty === dirty && !needsPreviewClear && !dropDraft) {
return s
}
if (ORCA_LAYOUT_OWNER_IS_MAIN && needsPreviewClear) {
@@ -132,55 +137,23 @@ export function createOpenFileMutations(
},
setExternalMutation: (fileId, mutation) =>
set((s) => {
const file = s.openFiles.find((f) => f.id === fileId)
if (!file) {
return s
}
const next = mutation ?? undefined
if (file.externalMutation === next) {
return s
}
return writeEditorBuffer(s, fileId, { externalMutation: next })
}),
set((s) => updateEditorBuffer(s, fileId, { externalMutation: mutation ?? undefined })),
setLastKnownDiskSignature: (fileId, signature) =>
set((s) => {
const file = s.openFiles.find((f) => f.id === fileId)
if (!file || file.lastKnownDiskSignature === signature) {
return s
}
return writeEditorBuffer(s, fileId, { lastKnownDiskSignature: signature })
}),
set((s) => updateEditorBuffer(s, fileId, { lastKnownDiskSignature: signature })),
clearPendingDiskBaselineVerification: (fileId) =>
set((s) => {
const file = s.openFiles.find((f) => f.id === fileId)
if (!file?.pendingDiskBaselineVerification) {
return s
}
return writeEditorBuffer(s, fileId, { pendingDiskBaselineVerification: undefined })
}),
get().setPendingDiskBaselineVerification(fileId, false),
setPendingDiskBaselineVerification: (fileId, value) =>
set((s) => {
const file = s.openFiles.find((f) => f.id === fileId)
const next = value || undefined
if (!file || file.pendingDiskBaselineVerification === next) {
return s
}
return writeEditorBuffer(s, fileId, { pendingDiskBaselineVerification: next })
}),
set((s) =>
updateEditorBuffer(s, fileId, { pendingDiskBaselineVerification: value || undefined })
),
setPendingLiveDiskVerification: (fileId, value) =>
set((s) => {
const file = s.openFiles.find((f) => f.id === fileId)
const next = value || undefined
if (!file || file.pendingLiveDiskVerification === next) {
return s
}
return writeEditorBuffer(s, fileId, { pendingLiveDiskVerification: next })
}),
set((s) =>
updateEditorBuffer(s, fileId, { pendingLiveDiskVerification: value || undefined })
),
clearSelfMoveEcho: (fileId) =>
set((s) => {
@@ -4,7 +4,7 @@ import type { RestoredEditorOwnerResult } from '../types/restored-editor-owner'
import { buildRestoredEditorOwnerTransition } from './restored-editor-owner-transition'
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../../shared/layout-owner-switch'
import { recordRestoredEditorMoveInLayoutOwner } from '../tabs/editor-tab-layout-owner'
import { writeEditorBuffer } from '../buffers/editor-buffer-record'
import { updateEditorBuffer } from '../buffers/editor-buffer-record'
export function createRestoredEditorOwner(
set: EditorSet,
@@ -14,13 +14,9 @@ export function createRestoredEditorOwner(
setRestoredEditorOwnerMigrationPending: (fileId, pending) => {
let changed = false
set((s) => {
const file = s.openFiles.find((candidate) => candidate.id === fileId)
const next = pending || undefined
if (!file || file.pendingOwnerMigration === next) {
return s
}
changed = true
return writeEditorBuffer(s, fileId, { pendingOwnerMigration: next })
const next = updateEditorBuffer(s, fileId, { pendingOwnerMigration: pending || undefined })
changed = next !== s
return next
})
return changed
},
@@ -16,6 +16,8 @@ export type EditorBufferRecord = Pick<
export type EditorBufferRecords = Record<string, EditorBufferRecord>
export type EditorBufferState = { openFiles: OpenFile[]; editorBuffers: EditorBufferRecords }
export const EDITOR_BUFFER_FIELDS = [
'isDirty',
'lastKnownDiskSignature',
@@ -48,8 +50,7 @@ export function editorBufferMirrorMismatch(
return EDITOR_BUFFER_FIELDS.find((field) => !Object.is(file[field], record[field])) ?? null
}
/** Patches `file`'s record; callers spread the same patch onto the tab in the same update. */
export function patchEditorBuffer(
function patchEditorBuffer(
records: EditorBufferRecords,
file: OpenFile,
patch: Partial<EditorBufferRecord>
@@ -62,11 +63,11 @@ export function patchEditorBuffer(
/** Writes `patch` to one file's record and its tab mirror in one update. */
export function writeEditorBuffer(
state: { openFiles: OpenFile[]; editorBuffers: EditorBufferRecords },
state: EditorBufferState,
fileId: string,
patch: Partial<EditorBufferRecord>,
tabPatch?: Partial<OpenFile>
): { openFiles: OpenFile[]; editorBuffers: EditorBufferRecords } {
): EditorBufferState {
let editorBuffers = state.editorBuffers
const openFiles = state.openFiles.map((file) => {
if (file.id !== fileId) {
@@ -78,6 +79,44 @@ export function writeEditorBuffer(
return { openFiles, editorBuffers }
}
/** Writes `patch` through the file's record; returns `state` itself when the record already holds it. */
export function updateEditorBuffer<TState extends EditorBufferState>(
state: TState,
fileId: string,
patch: Partial<EditorBufferRecord>
): TState | EditorBufferState {
const file = state.openFiles.find((candidate) => candidate.id === fileId)
if (!file) {
return state
}
const record = editorBufferOf(state.editorBuffers, file)
const changes = EDITOR_BUFFER_FIELDS.some(
(field) => Object.hasOwn(patch, field) && !Object.is(record[field], patch[field])
)
return changes ? writeEditorBuffer(state, fileId, patch) : state
}
/** A host-mirrored tab carries the host's buffer state, so its record adopts it in the same write. */
export function adoptMirroredEditorBuffers(
records: EditorBufferRecords,
openFiles: readonly OpenFile[]
): EditorBufferRecords {
let next = records
for (const file of openFiles) {
const record = records[file.id]
if (
file.mirroredFromRuntimeSession !== true ||
!record ||
editorBufferMirrorMismatch(file, record) === null
) {
continue
}
next = next === records ? { ...records } : next
next[file.id] = editorBufferRecordFromFile(file)
}
return next
}
/** Moves each migrated tab's record to its new id in the same update that re-keys the tab. */
export function rekeyEditorBuffers(
records: EditorBufferRecords,
@@ -61,18 +61,17 @@ test('keeps same-path host models and undo history separate', async ({
},
{ selection: 'none' }
)
store.getState().markFileDirty(localId, true)
store.getState().markFileDirty(remoteId, true)
store.setState((state) => ({
openFiles: state.openFiles.map((entry) =>
entry.id === remoteId
? {
...entry,
isDirty: true,
runtimeEnvironmentId: undefined,
operationProvenance: provenance('ssh:fixture-target', null)
}
: entry.id === localId
? { ...entry, isDirty: true }
: entry
: entry
),
editorDrafts: {
...state.editorDrafts,
@@ -43,6 +43,7 @@ function makeViewerState(): WebSessionTabsSyncState {
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},
@@ -66,6 +66,7 @@ function makeViewerState(): WebSessionTabsSyncState {
groupsByWorktree: {},
layoutByWorktree: {},
openFiles: [],
editorBuffers: {},
ptyIdsByTabId: {},
remoteBrowserPageHandlesByPageId: {},
tabBarOrderByWorktree: {},