From a753affffe99b5a2cf50ab238a8f43d36ca56a59 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 15:27:30 -0700 Subject: [PATCH] Isolate code editor text and Undo history by execution host (#25174) * Isolate code editor text and undo history by execution host * Verify editor owner resolution and Windows model path isolation * Respect native model line endings in content sync history coverage * Preserve selection and scroll when editor ownership resolves * Verify owner synchronization against a reachable editor change callback --- .../MonacoEditor.content-owner.test.tsx | 126 ++++++- .../MonacoEditor.owner-view-state.test.tsx | 161 +++++++++ .../src/components/editor/MonacoEditor.tsx | 18 +- .../editor/closed-editor-tab-controller.ts | 77 +++-- .../editor/closed-editor-tab-disposal.ts | 6 +- ...closed-editor-view-state-retention.test.ts | 5 +- .../editor-model-host-isolation.test.ts | 324 ++++++++++++++++++ .../editor/editor-model-owner.test.ts | 151 ++++++++ .../components/editor/editor-model-owner.ts | 39 +++ .../editor/editor-model-uri.test.ts | 12 + .../src/components/editor/editor-model-uri.ts | 8 +- .../monaco-content-sync.undo-history.test.ts | 2 +- .../editor/monaco-editor-mount-params.ts | 1 + .../editor/monaco-programmatic-sync.test.ts | 30 +- .../editor/monaco-programmatic-sync.ts | 32 +- .../editor/use-monaco-content-sync-bridge.ts | 23 +- .../editor/use-monaco-editor-mount.ts | 8 +- tests/e2e/editor-model-host-isolation.spec.ts | 110 ++++++ 18 files changed, 1070 insertions(+), 63 deletions(-) create mode 100644 src/renderer/src/components/editor/MonacoEditor.owner-view-state.test.tsx create mode 100644 src/renderer/src/components/editor/editor-model-host-isolation.test.ts create mode 100644 src/renderer/src/components/editor/editor-model-owner.test.ts create mode 100644 src/renderer/src/components/editor/editor-model-owner.ts create mode 100644 tests/e2e/editor-model-host-isolation.spec.ts diff --git a/src/renderer/src/components/editor/MonacoEditor.content-owner.test.tsx b/src/renderer/src/components/editor/MonacoEditor.content-owner.test.tsx index 790752921e2..68c8740eda6 100644 --- a/src/renderer/src/components/editor/MonacoEditor.content-owner.test.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.content-owner.test.tsx @@ -1,12 +1,39 @@ // @vitest-environment happy-dom import { cleanup, render } from '@testing-library/react' +import { useEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { buildOwnedEditorFileId } from '@/store/slices/editor/file-ids/editor-file-ids' +import type { OpenFile } from '@/store/slices/editor' +import type { WorktreeOperationRouteState } from '@/lib/worktree-operation-route' -const editorProps = vi.hoisted(() => ({ current: null as Record | null })) +const editorProps = vi.hoisted(() => { + const state: { + current: Record | null + files: OpenFile[] + ownerCatalog: WorktreeOperationRouteState + mounts: unknown[] + unmounts: unknown[] + } = { + current: null, + files: [], + ownerCatalog: { + worktreesByRepo: { local: [{ id: 'local-worktree', repoId: 'local', hostId: 'local' }] } + }, + mounts: [], + unmounts: [] + } + return state +}) vi.mock('@monaco-editor/react', () => ({ - default: (props: Record) => { + default: function MockEditor(props: Record) { editorProps.current = props + useEffect(() => { + editorProps.mounts.push(props.path) + return () => { + editorProps.unmounts.push(props.path) + } + }, [props.path]) return null }, loader: { config: vi.fn() } @@ -16,6 +43,8 @@ vi.mock('@/store', () => ({ selector({ settings: { theme: 'dark', terminalFontSize: 13, terminalFontFamily: 'monospace' }, editorFontZoomLevel: 0, + openFiles: editorProps.files, + ...editorProps.ownerCatalog, setPendingEditorReveal: vi.fn(), setEditorCursorLine: vi.fn(), addDiffComment: vi.fn(), @@ -38,6 +67,12 @@ import MonacoEditor from './MonacoEditor' afterEach(() => { cleanup() editorProps.current = null + editorProps.files = [] + editorProps.ownerCatalog = { + worktreesByRepo: { local: [{ id: 'local-worktree', repoId: 'local', hostId: 'local' }] } + } + editorProps.mounts = [] + editorProps.unmounts = [] }) describe('MonacoEditor content ownership', () => { @@ -59,4 +94,91 @@ describe('MonacoEditor content ownership', () => { expect(editorProps.current?.defaultValue).toBe('initial content') expect(editorProps.current).not.toHaveProperty('value') }) + + it('isolates same-path files on different hosts while sharing split panes', () => { + const filePath = '/srv/repo/file.ts' + const ownedId = buildOwnedEditorFileId(filePath, 'ssh-worktree', 'remote-runtime') + editorProps.files = [ + { + id: filePath, + filePath, + relativePath: 'file.ts', + worktreeId: 'local-worktree', + mode: 'edit', + language: 'typescript', + isDirty: false + }, + { + id: ownedId, + filePath, + relativePath: 'file.ts', + worktreeId: 'ssh-worktree', + runtimeEnvironmentId: 'remote-runtime', + mode: 'edit', + language: 'typescript', + isDirty: false + } + ] + const props = { + filePath, + relativePath: 'file.ts', + language: 'typescript', + onContentChange: vi.fn(), + onSave: vi.fn() + } + const local = render( + + ) + const localModel = editorProps.current?.path + local.unmount() + const remote = render( + + ) + const remoteModel = editorProps.current?.path + expect(remoteModel).not.toBe(localModel) + remote.unmount() + render() + expect(editorProps.current?.path).toBe(remoteModel) + }) + + it('keeps the current draft when restored ownership resolves and remounts the widget', () => { + const file: OpenFile = { + id: 'restored-file', + filePath: '/repo/restored.ts', + relativePath: 'restored.ts', + worktreeId: 'restored-worktree', + mode: 'edit', + language: 'typescript', + isDirty: true + } + editorProps.files = [file] + editorProps.ownerCatalog = {} + const onContentChange = vi.fn() + const props = { + fileId: file.id, + filePath: file.filePath, + viewStateKey: 'pane:restored-file', + relativePath: file.relativePath, + language: file.language, + onContentChange, + onSave: vi.fn() + } + const rendered = render() + const unresolvedModel = editorProps.current?.path + rendered.rerender() + expect(editorProps.mounts).toEqual([unresolvedModel]) + + editorProps.ownerCatalog = { + worktreesByRepo: { + repo: [{ id: file.worktreeId, repoId: 'repo', hostId: 'local' }] + } + } + rendered.rerender() + + const resolvedModel = editorProps.current?.path + expect(resolvedModel).not.toBe(unresolvedModel) + expect(editorProps.current?.defaultValue).toBe('latest dirty draft') + expect(editorProps.mounts).toEqual([unresolvedModel, resolvedModel]) + expect(editorProps.unmounts).toEqual([unresolvedModel]) + }) }) diff --git a/src/renderer/src/components/editor/MonacoEditor.owner-view-state.test.tsx b/src/renderer/src/components/editor/MonacoEditor.owner-view-state.test.tsx new file mode 100644 index 00000000000..f8e3605ab78 --- /dev/null +++ b/src/renderer/src/components/editor/MonacoEditor.owner-view-state.test.tsx @@ -0,0 +1,161 @@ +// @vitest-environment happy-dom +import { act, cleanup, render, waitFor } from '@testing-library/react' +import * as monaco from 'monaco-editor/esm/vs/editor/editor.api.js' +import { afterEach, expect, it, vi } from 'vitest' +import type { AppState } from '@/store/types' +import { createTestStore, makeWorktree } from '@/store/slices/store-test-helpers' +import type { OpenFile } from '@/store/slices/editor' +import { editorSelectionCache, scrollTopCache } from '@/lib/scroll-cache' +import { getDefaultSettings } from '../../../../shared/constants' + +const testState = vi.hoisted(() => { + const state: { store: ReturnType | null } = { store: null } + return state +}) + +vi.mock('@/store', () => { + const getStore = () => { + if (!testState.store) { + throw new Error('Missing test store') + } + return testState.store + } + return { + useAppStore: Object.assign((selector: (state: AppState) => unknown) => getStore()(selector), { + getState: () => getStore().getState() + }) + } +}) +vi.mock('@/lib/monaco-setup', async () => { + const { loader } = await import('@monaco-editor/react') + loader.config({ monaco }) + return {} +}) +vi.mock('./useContextualCopySetup', () => ({ + useContextualCopySetup: () => ({ setupCopy: vi.fn(), toastNode: null }) +})) +vi.mock('../diff-comments/useDiffCommentDecorator', () => ({ + useDiffCommentDecorator: vi.fn() +})) +vi.mock('sonner', () => ({ + toast: { info: vi.fn(), error: vi.fn(), success: vi.fn() } +})) + +import MonacoEditor from './MonacoEditor' + +const originalGetContext = HTMLCanvasElement.prototype.getContext +let creationListener: monaco.IDisposable | null = null + +afterEach(() => { + cleanup() + creationListener?.dispose() + creationListener = null + for (const model of monaco.editor.getModels()) { + model.dispose() + } + editorSelectionCache.clear() + scrollTopCache.clear() + Reflect.set(HTMLCanvasElement.prototype, 'getContext', originalGetContext) + testState.store = null +}) + +it('preserves a draft, selected range and scroll when the same tab gains a known owner', async () => { + // Only text metrics are needed; this test does not inspect canvas painting. + Reflect.set(HTMLCanvasElement.prototype, 'getContext', () => ({ + webkitBackingStorePixelRatio: 1, + measureText: (value: string) => ({ width: value.length * 8 }), + fillRect: () => {}, + clearRect: () => {}, + fillText: () => {}, + setTransform: () => {}, + save: () => {}, + restore: () => {}, + beginPath: () => {}, + moveTo: () => {}, + lineTo: () => {}, + stroke: () => {}, + createImageData: (width: number, height: number) => ({ + data: new Uint8ClampedArray(width * height * 4) + }), + getImageData: (width: number, height: number) => ({ + data: new Uint8ClampedArray(width * height * 4) + }), + putImageData: () => {} + })) + const store = createTestStore() + testState.store = store + const file: OpenFile = { + id: 'restored-file', + worktreeId: 'pending-workspace', + filePath: '/repo/restored.txt', + relativePath: 'restored.txt', + mode: 'edit', + language: 'plaintext', + isDirty: true + } + store.setState({ + openFiles: [file], + repos: [], + worktreesByRepo: {}, + detectedWorktreesByRepo: {}, + settings: { ...getDefaultSettings('/fixture'), theme: 'dark', editorWordWrap: false } + }) + const widgets: monaco.editor.ICodeEditor[] = [] + creationListener = monaco.editor.onDidCreateEditor((instance) => widgets.push(instance)) + const props = { + fileId: file.id, + filePath: file.filePath, + viewStateKey: 'pane:restored-file', + relativePath: file.relativePath, + language: file.language, + content: Array.from({ length: 300 }, (_, index) => `line ${index + 1}`).join('\n'), + onContentChange: vi.fn(), + onSave: vi.fn() + } + const rendered = render() + await waitFor(() => expect(widgets).toHaveLength(1)) + const before = widgets[0] + if (!before) { + throw new Error('Missing initial editor') + } + const originalModel = before.getModel() + if (!originalModel) { + throw new Error('Missing original model') + } + const draft = `${props.content}\nunsaved draft` + rendered.rerender() + expect(originalModel.getValue()).toBe(draft) + expect(originalModel.canUndo()).toBe(true) + before.layout({ width: 600, height: 300 }) + const selection = new monaco.Selection(80, 2, 82, 5) + before.setSelection(selection) + before.setScrollTop(1500) + expect(before.getScrollTop()).toBe(1500) + + const openFiles = store.getState().openFiles + act(() => { + store.setState({ + worktreesByRepo: { + repo: [ + makeWorktree({ id: file.worktreeId, repoId: 'repo', path: '/repo', hostId: 'local' }) + ] + } + }) + }) + expect(store.getState().openFiles).toBe(openFiles) + await waitFor(() => expect(widgets).toHaveLength(2)) + const after = widgets[1] + if (!after) { + throw new Error('Missing resolved editor') + } + expect(after.getModel()).not.toBe(originalModel) + expect(after.getModel()?.getValue()).toBe(draft) + expect(after.getModel()?.canUndo()).toBe(false) + await waitFor(() => expect(after.getSelection()?.toString()).toBe(selection.toString())) + expect(after.getScrollTop()).toBe(1500) + expect(props.onContentChange).not.toHaveBeenCalled() + act(() => { + after.executeEdits('user-edit', [{ range: new monaco.Range(301, 14, 301, 14), text: '!' }]) + }) + expect(props.onContentChange).toHaveBeenCalledExactlyOnceWith(`${draft}!`) +}) diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index 17f659607a3..3f1f27423cd 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -13,6 +13,7 @@ import { isLinuxUserAgent } from '../terminal-pane/pane-helpers' import { MAX_TOKENIZATION_LINE_LENGTH } from '@/lib/monaco-languages/monarch-embed-entry-budget' import { buildFileEditorWordWrapOptions } from './file-editor-word-wrap-options' import { toEditorModelUri } from './editor-model-uri' +import { getEditorModelOwnerKey } from './editor-model-owner' import { getMonacoAutoHeightForContent, isMonacoAutoHeightCapped } from './monaco-auto-height' import { monacoFindOptions } from './monaco-find-options' import { useMonacoRevealScheduler } from './use-monaco-reveal-scheduler' @@ -97,7 +98,16 @@ export default function MonacoEditor({ ) const editorFontFamily = resolveEditorFontFamily(settings) const editorWordWrap = settings?.editorWordWrap - const modelUri = useMemo(() => toEditorModelUri(filePath), [filePath]) + const modelOwnerKey = useAppStore((state) => { + const file = state.openFiles?.find((entry) => entry.id === fileId) + return file + ? getEditorModelOwnerKey(file, state) + : JSON.stringify([null, `unresolved:${fileId}`]) + }) + const modelUri = useMemo( + () => toEditorModelUri(filePath, modelOwnerKey), + [filePath, modelOwnerKey] + ) const estimatedAutoHeight = useMemo(() => { if (!autoHeight) { return null @@ -127,7 +137,7 @@ export default function MonacoEditor({ content, contentRef, contentSyncModeRef, - filePath, + modelKey: modelUri, onContentChange }) const annotations = useMonacoMarkdownAnnotations({ @@ -154,7 +164,7 @@ export default function MonacoEditor({ unregisterFileSearchSelectionRef.current?.() unregisterFileSearchSelectionRef.current = null } - }, [cancelScheduledReveal, clearTransientRevealHighlight, viewStateKey]) + }, [cancelScheduledReveal, clearTransientRevealHighlight, modelUri, viewStateKey]) // Update editor options when settings change useEffect(() => { @@ -183,6 +193,7 @@ export default function MonacoEditor({ const handleMount = useMonacoEditorMount({ fileId, filePath, + modelOwnerKey, viewStateKey, viewStateId, worktreeId, @@ -232,6 +243,7 @@ export default function MonacoEditor({ onSubmitMarkdownComment={annotations.handleSubmitMarkdownComment} /> , 'getState' | 'subscribe'> +type EditorStore = Pick< + StoreApi, + 'getState' | 'subscribe' +> type RetainedModel = { files: Map detach: { dispose(): void } @@ -14,7 +19,7 @@ type RetainedModel = { } function ownerKey(file: ClosedEditorTab): string { - return JSON.stringify([file.id, file.mode, file.filePath]) + return JSON.stringify([file.id, file.mode, file.filePath, file.modelOwnerKey]) } export function attachClosedEditorTabCleanup( @@ -22,7 +27,8 @@ export function attachClosedEditorTabCleanup( bridge = editorModelRegistry ): () => void { let registry = bridge.get() - let previousFiles = store.getState().openFiles + let previousState = store.getState() + let previousFiles = previousState.openFiles const pendingFiles = new Map() const candidateModels = new Set() const retainedModels = new Map() @@ -96,24 +102,26 @@ export function attachClosedEditorTabCleanup( return } const flushGeneration = generation - let checkedOpenFiles: OpenFile[] | null = null + let checkedState: ReturnType | null = null let openIds = new Set() let openEditUris = new Set() const stillOwned = (file: ClosedEditorTab): boolean => { - const openFiles = store.getState().openFiles - if (openFiles !== checkedOpenFiles) { - checkedOpenFiles = openFiles + const currentState = store.getState() + const openFiles = currentState.openFiles + if (currentState !== checkedState) { + checkedState = currentState openIds = new Set(openFiles.map((openFile) => openFile.id)) openEditUris = new Set( openFiles .filter((openFile) => openFile.mode === 'edit') - .map((openFile) => toEditorModelUri(openFile.filePath)) + .map((openFile) => + toEditorModelUri(openFile.filePath, getEditorModelOwnerKey(openFile, currentState)) + ) ) } - return ( - openIds.has(file.id) || - (file.mode === 'edit' && openEditUris.has(toEditorModelUri(file.filePath))) - ) + return file.mode === 'edit' + ? openEditUris.has(toEditorModelUri(file.filePath, file.modelOwnerKey)) + : openIds.has(file.id) } const disposeCaptured = ( files: ClosedEditorTab[], @@ -181,23 +189,52 @@ export function attachClosedEditorTabCleanup( } const unsubscribe = store.subscribe(() => { - const openFiles = store.getState().openFiles - if (openFiles === previousFiles) { + const currentState = store.getState() + const openFiles = currentState.openFiles + const previous = previousFiles + const closedState = previousState + previousFiles = openFiles + previousState = currentState + if ( + openFiles === previous && + currentState.repos === closedState.repos && + currentState.worktreesByRepo === closedState.worktreesByRepo && + currentState.detectedWorktreesByRepo === closedState.detectedWorktreesByRepo && + currentState.folderWorkspaces === closedState.folderWorkspaces && + currentState.projectGroups === closedState.projectGroups && + currentState.restoredRuntimeHostIdByWorkspaceSessionKey === + closedState.restoredRuntimeHostIdByWorkspaceSessionKey && + currentState.runtimeEnvironmentCatalogHydrated === + closedState.runtimeEnvironmentCatalogHydrated && + currentState.removedRuntimeEnvironmentIds === closedState.removedRuntimeEnvironmentIds && + currentState.runtimeEnvironments === closedState.runtimeEnvironments && + currentState.settings === closedState.settings + ) { return } - const previous = previousFiles - previousFiles = openFiles - const liveIds = new Set(openFiles.map((file) => file.id)) + const liveFiles = new Map(openFiles.map((file) => [file.id, file])) let removed = false let removedDiff = false for (const file of previous) { - if (!liveIds.has(file.id)) { - const descriptor = { id: file.id, mode: file.mode, filePath: file.filePath } + const priorOwner = getEditorModelOwnerKey(file, closedState) + const liveFile = liveFiles.get(file.id) + if ( + !liveFile || + liveFile.mode !== file.mode || + liveFile.filePath !== file.filePath || + (file.mode === 'edit' && getEditorModelOwnerKey(liveFile, currentState) !== priorOwner) + ) { + const descriptor = { + id: file.id, + mode: file.mode, + filePath: file.filePath, + modelOwnerKey: priorOwner + } pendingFiles.set(ownerKey(descriptor), descriptor) removed = true if (file.mode === 'edit' && registry) { const model = registry.editor.getModel( - registry.Uri.parse(toEditorModelUri(file.filePath)) + registry.Uri.parse(toEditorModelUri(file.filePath, descriptor.modelOwnerKey)) ) if (model) { candidateModels.add(model) diff --git a/src/renderer/src/components/editor/closed-editor-tab-disposal.ts b/src/renderer/src/components/editor/closed-editor-tab-disposal.ts index 750c5e4317e..cd961c898b5 100644 --- a/src/renderer/src/components/editor/closed-editor-tab-disposal.ts +++ b/src/renderer/src/components/editor/closed-editor-tab-disposal.ts @@ -17,7 +17,9 @@ import { } from './closed-editor-tab-cache-sweep' import { toEditorModelUri } from './editor-model-uri' -export type ClosedEditorTab = Pick +export type ClosedEditorTab = Pick & { + modelOwnerKey?: string +} // One registry sweep avoids quadratic close-all work. export function disposeClosedEditorModels( @@ -37,7 +39,7 @@ export function disposeClosedEditorModels( } if (closedFile.mode === 'edit') { const model = monacoRegistry.editor.getModel( - monacoRegistry.Uri.parse(toEditorModelUri(closedFile.filePath)) + monacoRegistry.Uri.parse(toEditorModelUri(closedFile.filePath, closedFile.modelOwnerKey)) ) if (model?.isAttachedToEditor()) { onAttachedModel?.(model, closedFile) diff --git a/src/renderer/src/components/editor/closed-editor-view-state-retention.test.ts b/src/renderer/src/components/editor/closed-editor-view-state-retention.test.ts index 1b88f0ccbb6..206ff5fdf77 100644 --- a/src/renderer/src/components/editor/closed-editor-view-state-retention.test.ts +++ b/src/renderer/src/components/editor/closed-editor-view-state-retention.test.ts @@ -42,7 +42,10 @@ function open(path: string, text = 'original') { 'plaintext', monaco.Uri.parse(toEditorModelUri(path)) ) - const store = createStore(() => ({ openFiles: [file] })) + const store = createStore(() => ({ + openFiles: [file], + worktreesByRepo: { fixture: [{ id: 'fixture', repoId: 'fixture', hostId: 'local' as const }] } + })) const bridge = createEditorModelRegistry() disposeOwners.push(bridge.register(monaco), attachClosedEditorTabCleanup(store, bridge)) return { diff --git a/src/renderer/src/components/editor/editor-model-host-isolation.test.ts b/src/renderer/src/components/editor/editor-model-host-isolation.test.ts new file mode 100644 index 00000000000..c37491b0836 --- /dev/null +++ b/src/renderer/src/components/editor/editor-model-host-isolation.test.ts @@ -0,0 +1,324 @@ +// @vitest-environment happy-dom +import * as monaco from 'monaco-editor/esm/vs/editor/editor.api.js' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import type { WorktreeOperationRoute } from '@/lib/worktree-operation-route' +import { + attachModelLifetimeView, + createModelLifetimeFixture, + modelLifetimeFile, + modelLifetimeTextModel, + resetModelLifetimeFixtures +} from './editor-model-lifetime-fixture' +import { getEditorModelOwnerKey } from './editor-model-owner' +import { toEditorModelUri } from './editor-model-uri' +import { + beginProgrammaticContentSync, + endProgrammaticContentSync, + resetProgrammaticContentSyncForTests, + shouldIgnoreMonacoContentChange +} from './monaco-programmatic-sync' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), error: vi.fn(), success: vi.fn() } })) + +const FILE_PATH = '/fixture/workspace/same-path.txt' +const HOSTS: readonly WorktreeOperationRoute[] = [ + { executionHostId: 'local', runtimeEnvironmentId: null }, + { executionHostId: 'ssh:target', runtimeEnvironmentId: null }, + { executionHostId: 'runtime:hub-a', runtimeEnvironmentId: 'hub-a' }, + { executionHostId: 'ssh:target', runtimeEnvironmentId: 'hub-a' }, + { executionHostId: 'ssh:target', runtimeEnvironmentId: 'hub-b' } +] + +function file(id: string, route: WorktreeOperationRoute, filePath = FILE_PATH): OpenFile { + return { + ...modelLifetimeFile(id), + filePath, + relativePath: 'same-path.txt', + operationProvenance: { + ownershipProjection: 'explicit', + generation: { + route, + runtimeConnectionGeneration: null, + runtimePairingRevision: undefined, + runtimeSshGeneration: null, + nestedSshGeneration: null, + directSshGeneration: null + } + } + } +} + +function createHostModels(filePath = FILE_PATH) { + const fixture = createModelLifetimeFixture() + const owners = HOSTS.map((route, index) => { + const ownedFile = file(`host-${index}`, route, filePath) + const modelKey = toEditorModelUri( + ownedFile.filePath, + getEditorModelOwnerKey(ownedFile, fixture.store.getState()) + ) + const initial = `text owned by ${route.executionHostId} through ${route.runtimeEnvironmentId}` + const model = modelLifetimeTextModel(modelKey, initial) + return { file: ownedFile, model, modelKey, initial } + }) + fixture.store.setState({ openFiles: owners.map((owner) => owner.file) }) + return { ...fixture, owners } +} + +function edit(model: monaco.editor.ITextModel, text: string) { + model.pushStackElement() + model.pushEditOperations([], [{ range: model.getFullModelRange(), text }], () => null) + model.pushStackElement() +} + +afterEach(() => { + resetProgrammaticContentSyncForTests() + resetModelLifetimeFixtures() +}) + +describe('same-path models on different execution hosts', () => { + it.each([FILE_PATH, 'C:\\fixture\\workspace\\same-path.txt', '\\\\server\\share\\same-path.txt'])( + 'keeps host text and undo histories independent for %s', + async (filePath) => { + const { owners } = createHostModels(filePath) + expect(new Set(owners.map((owner) => owner.modelKey)).size).toBe(HOSTS.length) + expect(new Set(owners.map((owner) => owner.model.uri.fsPath))).toEqual( + new Set([monaco.Uri.file(filePath).fsPath]) + ) + for (const [index, owner] of owners.entries()) { + expect(monaco.editor.getModel(monaco.Uri.parse(owner.modelKey))).toBe(owner.model) + edit(owner.model, `edited host ${index}`) + expect(owner.model.canUndo()).toBe(true) + expect(owners.map((candidate) => candidate.model.getValue())).toEqual( + owners.map((candidate, candidateIndex) => + candidateIndex <= index ? `edited host ${candidateIndex}` : candidate.initial + ) + ) + } + for (const [index, owner] of owners.entries()) { + await owner.model.undo() + expect(owner.model.canRedo()).toBe(true) + expect(owners.map((candidate) => candidate.model.getValue())).toEqual( + owners.map((candidate, candidateIndex) => + candidateIndex <= index ? candidate.initial : `edited host ${candidateIndex}` + ) + ) + } + await owners[1]!.model.redo() + expect(owners.map((owner) => owner.model.getValue())).toEqual( + owners.map((owner, index) => (index === 1 ? 'edited host 1' : owner.initial)) + ) + } + ) + + it('suppresses watcher echoes in shared local panes without hiding an SSH user edit', () => { + const { owners } = createHostModels() + const local = owners[0]! + const ssh = owners[1]! + const accepted: string[] = [] + const listeners = [local, local, ssh].map((owner, index) => + owner.model.onDidChangeContent(() => { + if ( + !shouldIgnoreMonacoContentChange({ + modelKey: owner.modelKey, + isApplyingProgrammaticContent: false + }) + ) { + accepted.push(`pane-${index}:${owner.model.getValue()}`) + } + }) + ) + beginProgrammaticContentSync(local.modelKey) + try { + local.model.setValue('local watcher update') + expect(accepted).toEqual([]) + edit(ssh.model, 'SSH user edit') + expect(accepted).toEqual(['pane-2:SSH user edit']) + expect(local.model.getValue()).toBe('local watcher update') + expect(owners.slice(2).map((owner) => owner.model.getValue())).toEqual( + owners.slice(2).map((owner) => owner.initial) + ) + } finally { + endProgrammaticContentSync(local.modelKey) + listeners.forEach((listener) => listener.dispose()) + } + }) + + it('closes only the selected host model while siblings retain their text and undo', async () => { + const { owners, store, attach } = createHostModels() + owners.forEach((owner, index) => edit(owner.model, `edited host ${index}`)) + attach() + const enumeration = vi.spyOn(monaco.editor, 'getModels') + for (const [index, owner] of owners.entries()) { + store.getState().closeFile(owner.file.id) + await Promise.resolve() + expect(owner.model.isDisposed()).toBe(true) + for (const [siblingIndex, sibling] of owners.slice(index + 1).entries()) { + expect(sibling.model.isDisposed()).toBe(false) + expect(sibling.model.getValue()).toBe(`edited host ${index + siblingIndex + 1}`) + expect(sibling.model.canUndo()).toBe(true) + } + } + expect(enumeration).not.toHaveBeenCalled() + expect(store.getState().openFiles).toHaveLength(0) + }) + + it.each([FILE_PATH, 'C:\\fixture\\workspace\\same-path.txt', '\\\\server\\share\\same-path.txt'])( + 'restores independent closed-tab undo for remote owners of %s', + async (filePath) => { + const { owners, store, attach } = createHostModels(filePath) + const remoteOwners = [owners[1], owners[4]] + attach() + const closed = remoteOwners.map((owner, index) => { + if (!owner) { + throw new Error('Missing remote owner') + } + edit(owner.model, `saved remote draft ${index}`) + return { ...owner, saved: owner.model.getValue() } + }) + for (const owner of closed) { + store.getState().closeFile(owner.file.id) + } + await Promise.resolve() + expect(closed.every((owner) => owner.model.isDisposed())).toBe(true) + store.setState({ + openFiles: [...store.getState().openFiles, ...closed.map((owner) => owner.file)] + }) + const reopened = closed.map((owner) => modelLifetimeTextModel(owner.modelKey, owner.saved)) + expect(reopened.every((model) => model.canUndo())).toBe(true) + for (const [index, model] of reopened.entries()) { + await model.undo() + expect(reopened.map((candidate) => candidate.getValue())).toEqual( + closed.map((owner, ownerIndex) => (ownerIndex <= index ? owner.initial : owner.saved)) + ) + } + for (const owner of owners.filter( + (candidate) => !closed.some((entry) => entry.file.id === candidate.file.id) + )) { + expect(owner.model.isDisposed()).toBe(false) + expect(owner.model.getValue()).toBe(owner.initial) + } + } + ) + + it('releases a closed SSH model after its view detaches without touching a live local model', async () => { + const { owners, store, attach } = createHostModels() + const local = owners[0]! + const ssh = owners[1]! + const detach = attachModelLifetimeView(ssh.model) + attach() + store.getState().closeFile(ssh.file.id) + await Promise.resolve() + expect(ssh.model.isDisposed()).toBe(false) + detach() + expect(ssh.model.isDisposed()).toBe(false) + await Promise.resolve() + expect(ssh.model.isDisposed()).toBe(true) + expect(local.model.isDisposed()).toBe(false) + edit(local.model, 'local still editable') + await local.model.undo() + expect(local.model.getValue()).toBe(local.initial) + }) + + it('preserves local same-path sharing until the final local tab closes', async () => { + const { owners, store, attach } = createHostModels() + const local = owners[0]! + const sharedLocal = { + ...local.file, + id: 'local-other-workspace', + worktreeId: 'another-workspace' + } + expect(toEditorModelUri(FILE_PATH, getEditorModelOwnerKey(sharedLocal, store.getState()))).toBe( + local.modelKey + ) + store.setState({ openFiles: [...store.getState().openFiles, sharedLocal] }) + attach() + edit(local.model, 'shared local edit') + store.getState().closeFile(local.file.id) + await Promise.resolve() + expect(local.model.isDisposed()).toBe(false) + await local.model.undo() + expect(local.model.getValue()).toBe(local.initial) + store.getState().closeFile(sharedLocal.id) + await Promise.resolve() + expect(local.model.isDisposed()).toBe(true) + expect(owners.slice(1).every((owner) => !owner.model.isDisposed())).toBe(true) + }) + + it('releases an attached old owner when the same tab moves to a live runtime model', async () => { + const { owners, store, attach } = createHostModels() + const local = owners[0]! + const runtime = owners[2]! + const detach = attachModelLifetimeView(local.model) + edit(runtime.model, 'runtime independent edit') + attach() + store.setState({ + openFiles: store + .getState() + .openFiles.map((opened) => + opened.id === local.file.id + ? { ...opened, operationProvenance: runtime.file.operationProvenance } + : opened + ) + }) + await Promise.resolve() + expect(local.model.isDisposed()).toBe(false) + detach() + await Promise.resolve() + expect(local.model.isDisposed()).toBe(true) + expect(runtime.model.isDisposed()).toBe(false) + expect(runtime.model.getValue()).toBe('runtime independent edit') + store.getState().closeFile(local.file.id) + await Promise.resolve() + expect(runtime.model.isDisposed()).toBe(false) + await runtime.model.undo() + expect(runtime.model.getValue()).toBe(runtime.initial) + }) + + it('releases the prior owner when an uncaptured tab changes host without changing the tab array', async () => { + const { owners, store, attach } = createHostModels() + const local = owners[0]! + const ssh = owners[1]! + const legacyFile = { ...local.file, operationProvenance: undefined } + store.setState({ openFiles: [legacyFile, ...owners.slice(1).map((owner) => owner.file)] }) + expect(getEditorModelOwnerKey(legacyFile, store.getState())).toBe('') + const openFiles = store.getState().openFiles + const detach = attachModelLifetimeView(local.model) + attach() + store.setState({ + worktreesByRepo: Object.fromEntries( + Object.entries(store.getState().worktreesByRepo).map(([repoId, worktrees]) => [ + repoId, + worktrees.map((worktree) => ({ ...worktree, hostId: 'ssh:target' as const })) + ]) + ) + }) + expect(store.getState().openFiles).toBe(openFiles) + expect(toEditorModelUri(FILE_PATH, getEditorModelOwnerKey(legacyFile, store.getState()))).toBe( + ssh.modelKey + ) + await Promise.resolve() + expect(local.model.isDisposed()).toBe(false) + detach() + await Promise.resolve() + expect(local.model.isDisposed()).toBe(true) + expect(ssh.model.isDisposed()).toBe(false) + expect(ssh.model.getValue()).toBe(ssh.initial) + }) + + it('uses prior host ownership when a catalog and its uncaptured tab disappear together', async () => { + const { owners, store, attach } = createHostModels() + const local = owners[0]! + const legacyFile = { ...local.file, operationProvenance: undefined } + store.setState({ openFiles: [legacyFile, ...owners.slice(1).map((owner) => owner.file)] }) + expect(getEditorModelOwnerKey(legacyFile, store.getState())).toBe('') + attach() + store.setState({ openFiles: owners.slice(1).map((owner) => owner.file), worktreesByRepo: {} }) + await Promise.resolve() + expect(local.model.isDisposed()).toBe(true) + for (const owner of owners.slice(1)) { + expect(owner.model.isDisposed()).toBe(false) + expect(owner.model.getValue()).toBe(owner.initial) + } + }) +}) diff --git a/src/renderer/src/components/editor/editor-model-owner.test.ts b/src/renderer/src/components/editor/editor-model-owner.test.ts new file mode 100644 index 00000000000..3106939277d --- /dev/null +++ b/src/renderer/src/components/editor/editor-model-owner.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import type { + WorktreeOperationRoute, + WorktreeOperationRouteState +} from '@/lib/worktree-operation-route' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' +import { getEditorModelOwnerKey } from './editor-model-owner' + +function file(overrides: Partial = {}): OpenFile { + return { + id: 'file', + filePath: '/srv/repo/file.ts', + relativePath: 'file.ts', + worktreeId: 'repo::/srv/repo', + language: 'typescript', + mode: 'edit', + isDirty: false, + ...overrides + } +} + +function captured(route: WorktreeOperationRoute): OpenFile['operationProvenance'] { + return { + ownershipProjection: 'explicit', + generation: { + route, + runtimeConnectionGeneration: null, + runtimePairingRevision: undefined, + runtimeSshGeneration: null, + nestedSshGeneration: null, + directSshGeneration: null + } + } +} + +const localState: WorktreeOperationRouteState = { + worktreesByRepo: { repo: [{ id: 'repo::/srv/repo', repoId: 'repo', hostId: 'local' }] } +} + +describe('editor model ownership', () => { + it('shares local physical files across workspace and tab identities', () => { + expect(getEditorModelOwnerKey(file(), localState)).toBe('') + expect( + getEditorModelOwnerKey( + file({ + id: 'other', + worktreeId: 'other', + operationProvenance: captured({ executionHostId: 'local', runtimeEnvironmentId: null }) + }), + {} + ) + ).toBe('') + }) + + it('keeps captured ownership when the focused host changes', () => { + const opened = file({ + operationProvenance: captured({ + executionHostId: 'ssh:target', + runtimeEnvironmentId: 'hub-a' + }) + }) + const key = getEditorModelOwnerKey(opened, {}) + expect( + getEditorModelOwnerKey(opened, { + ...localState, + activeWorktreeId: opened.worktreeId, + activeWorkspaceExecutionHostId: 'runtime:hub-b' + }) + ).toBe(key) + expect(key).toBe('["hub-a","ssh:target"]') + }) + + it('keeps distinct paired transports to the same target separate', () => { + const keys = ['hub-a', 'hub-b'].map((runtimeEnvironmentId) => + getEditorModelOwnerKey( + file({ + operationProvenance: captured({ executionHostId: 'ssh:target', runtimeEnvironmentId }) + }), + {} + ) + ) + expect(keys[0]).not.toBe(keys[1]) + expect(keys).not.toContain( + getEditorModelOwnerKey( + file({ externalSshTargetId: 'target', runtimeEnvironmentId: null }), + {} + ) + ) + }) + + it('uses the tab runtime hint when no owner catalog is present', () => { + expect(getEditorModelOwnerKey(file({ runtimeEnvironmentId: 'hub-a' }), {})).toBe( + '["hub-a","runtime:hub-a"]' + ) + expect(getEditorModelOwnerKey(file({ runtimeEnvironmentId: 'hub-b' }), {})).toBe( + '["hub-b","runtime:hub-b"]' + ) + }) + + it('honors an explicit local transport hint instead of the focused runtime', () => { + const opened = file({ runtimeEnvironmentId: null, externalSshTargetId: 'target' }) + const state: WorktreeOperationRouteState = { + worktreesByRepo: { + repo: [ + { + id: opened.worktreeId, + repoId: 'repo', + hostId: 'ssh:target', + runtimeOwnerEnvironmentId: 'hub-a' + } + ] + } + } + expect(getEditorModelOwnerKey(opened, state)).toBe('[null,"ssh:target"]') + }) + + it('does not adopt a new active host for a legacy local tab', () => { + const opened = file() + expect( + getEditorModelOwnerKey(opened, { + ...localState, + activeWorktreeId: opened.worktreeId, + activeWorkspaceExecutionHostId: 'ssh:new-target' + }) + ).toBe('') + }) + + it('keeps unresolved owners separate from local files', () => { + expect(getEditorModelOwnerKey(file(), {})).not.toBe('') + expect(getEditorModelOwnerKey(file(), {})).not.toBe( + getEditorModelOwnerKey(file({ worktreeId: 'another-owner' }), {}) + ) + }) + + it('supports local and remote folder workspaces', () => { + const opened = file({ worktreeId: folderWorkspaceKey('folder') }) + expect( + getEditorModelOwnerKey(opened, { + folderWorkspaces: [{ id: 'folder', projectGroupId: 'group', executionHostId: 'local' }] + }) + ).toBe('') + expect( + getEditorModelOwnerKey(opened, { + folderWorkspaces: [ + { id: 'folder', projectGroupId: 'group', executionHostId: 'runtime:hub-a' } + ] + }) + ).toBe('["hub-a","runtime:hub-a"]') + }) +}) diff --git a/src/renderer/src/components/editor/editor-model-owner.ts b/src/renderer/src/components/editor/editor-model-owner.ts new file mode 100644 index 00000000000..b8764943ca1 --- /dev/null +++ b/src/renderer/src/components/editor/editor-model-owner.ts @@ -0,0 +1,39 @@ +import type { OpenFile } from '@/store/slices/editor' +import { + resolveWorktreeOperationRoute, + type WorktreeOperationRouteState +} from '@/lib/worktree-operation-route' +import { toRuntimeExecutionHostId, toSshExecutionHostId } from '../../../../shared/execution-host' + +export function getEditorModelOwnerKey(file: OpenFile, state: WorktreeOperationRouteState): string { + const captured = file.operationProvenance?.generation.route + const route = + captured ?? + resolveWorktreeOperationRoute( + state.activeWorktreeId === file.worktreeId ? { ...state, activeWorktreeId: null } : state, + file.worktreeId + ) + const environmentId = captured + ? captured.runtimeEnvironmentId + : file.runtimeEnvironmentId !== undefined + ? file.runtimeEnvironmentId?.trim() || null + : (route?.runtimeEnvironmentId ?? null) + const hostId = captured + ? captured.executionHostId + : file.externalSshTargetId + ? toSshExecutionHostId(file.externalSshTargetId) + : route && + (file.runtimeEnvironmentId === undefined || + (file.runtimeEnvironmentId || null) === route.runtimeEnvironmentId) + ? route.executionHostId + : environmentId + ? toRuntimeExecutionHostId(environmentId) + : null + if (hostId === 'local' && !environmentId) { + return '' + } + // Unresolved owners must not borrow another host's retained text. + return JSON.stringify( + hostId ? [environmentId, hostId] : [environmentId, 'unresolved', file.worktreeId, file.id] + ) +} diff --git a/src/renderer/src/components/editor/editor-model-uri.test.ts b/src/renderer/src/components/editor/editor-model-uri.test.ts index ab36097ca20..4fc1828b791 100644 --- a/src/renderer/src/components/editor/editor-model-uri.test.ts +++ b/src/renderer/src/components/editor/editor-model-uri.test.ts @@ -37,4 +37,16 @@ describe('toEditorModelUri', () => { it('keeps distinct POSIX paths distinct', () => { expect(toEditorModelUri('/repo/a b.ts')).not.toBe(toEditorModelUri('/repo/a%20b.ts')) }) + + it.each(paths)('isolates owners while preserving the filesystem path for %s', (path) => { + const local = URI.parse(toEditorModelUri(path)) + const remoteUri = toEditorModelUri(path, '["hub-a","ssh:target#1"]') + const remote = URI.parse(remoteUri) + expect(remote.scheme).toBe('file') + expect(remote.fsPath).toBe(local.fsPath) + expect(remote.toString()).not.toBe(local.toString()) + expect(remote.toString()).toBe(remoteUri) + expect(toEditorModelUri(remoteUri)).toBe(remoteUri) + expect(toEditorModelUri(path, '["hub-b","ssh:target#1"]')).not.toBe(remoteUri) + }) }) diff --git a/src/renderer/src/components/editor/editor-model-uri.ts b/src/renderer/src/components/editor/editor-model-uri.ts index 1953a2ab757..6d467bd2549 100644 --- a/src/renderer/src/components/editor/editor-model-uri.ts +++ b/src/renderer/src/components/editor/editor-model-uri.ts @@ -12,6 +12,10 @@ const FILE_SCHEME = /^file:/i * re-parses to itself, so a consumer that can only take a string (the `path` prop of * `@monaco-editor/react`, which calls `Uri.parse` internally) lands on the same key. */ -export function toEditorModelUri(filePath: string): string { - return URI.file(FILE_SCHEME.test(filePath) ? URI.parse(filePath).fsPath : filePath).toString() +export function toEditorModelUri(filePath: string, ownerKey = ''): string { + const parsed = FILE_SCHEME.test(filePath) ? URI.parse(filePath) : URI.file(filePath) + // Equal paths on different owners must not share text or undo history. + return URI.file(parsed.fsPath) + .with({ fragment: ownerKey || parsed.fragment }) + .toString() } diff --git a/src/renderer/src/components/editor/monaco-content-sync.undo-history.test.ts b/src/renderer/src/components/editor/monaco-content-sync.undo-history.test.ts index 380a44ec8d4..0f8a4069d36 100644 --- a/src/renderer/src/components/editor/monaco-content-sync.undo-history.test.ts +++ b/src/renderer/src/components/editor/monaco-content-sync.undo-history.test.ts @@ -35,7 +35,7 @@ describe('Monaco external-content undo history', () => { syncContentUpdate(editorInstance, 'first line\nappended', 'read-only-live-tail') - expect(model.getValue()).toBe('first line\nappended') + expect(model.getValue()).toBe(['first line', 'appended'].join(model.getEOL())) expect(model.canUndo()).toBe(false) }) diff --git a/src/renderer/src/components/editor/monaco-editor-mount-params.ts b/src/renderer/src/components/editor/monaco-editor-mount-params.ts index 5e29ffd16c9..38f7cc6d289 100644 --- a/src/renderer/src/components/editor/monaco-editor-mount-params.ts +++ b/src/renderer/src/components/editor/monaco-editor-mount-params.ts @@ -14,6 +14,7 @@ export type MonacoEditorPropsRef = MutableRefObject<{ }> export type MonacoEditorMountParams = { + modelOwnerKey?: string fileId: string filePath: string viewStateKey: string diff --git a/src/renderer/src/components/editor/monaco-programmatic-sync.test.ts b/src/renderer/src/components/editor/monaco-programmatic-sync.test.ts index ae7cc67d06b..e61b68a23cb 100644 --- a/src/renderer/src/components/editor/monaco-programmatic-sync.test.ts +++ b/src/renderer/src/components/editor/monaco-programmatic-sync.test.ts @@ -12,25 +12,25 @@ afterEach(() => { describe('shouldIgnoreMonacoContentChange', () => { it('ignores echoed shared-model changes in the sibling split pane', () => { - const filePath = '/repo/seed.spec.ts' + const modelKey = '/repo/seed.spec.ts' - beginProgrammaticContentSync(filePath) + beginProgrammaticContentSync(modelKey) try { expect( shouldIgnoreMonacoContentChange({ - filePath, + modelKey, isApplyingProgrammaticContent: false }) ).toBe(true) } finally { - endProgrammaticContentSync(filePath) + endProgrammaticContentSync(modelKey) } }) it('ignores local programmatic sync even without a sibling pane', () => { expect( shouldIgnoreMonacoContentChange({ - filePath: '/repo/seed.spec.ts', + modelKey: '/repo/seed.spec.ts', isApplyingProgrammaticContent: true }) ).toBe(true) @@ -39,9 +39,27 @@ describe('shouldIgnoreMonacoContentChange', () => { it('does not ignore a real user edit once programmatic sync is finished', () => { expect( shouldIgnoreMonacoContentChange({ - filePath: '/repo/seed.spec.ts', + modelKey: '/repo/seed.spec.ts', isApplyingProgrammaticContent: false }) ).toBe(false) }) + + it('keeps nested sync suppression scoped to one model owner', () => { + const local = 'file:///repo/file.ts' + const remote = `${local}#remote-owner` + beginProgrammaticContentSync(local) + beginProgrammaticContentSync(local) + endProgrammaticContentSync(local) + expect( + shouldIgnoreMonacoContentChange({ modelKey: local, isApplyingProgrammaticContent: false }) + ).toBe(true) + expect( + shouldIgnoreMonacoContentChange({ modelKey: remote, isApplyingProgrammaticContent: false }) + ).toBe(false) + endProgrammaticContentSync(local) + expect( + shouldIgnoreMonacoContentChange({ modelKey: local, isApplyingProgrammaticContent: false }) + ).toBe(false) + }) }) diff --git a/src/renderer/src/components/editor/monaco-programmatic-sync.ts b/src/renderer/src/components/editor/monaco-programmatic-sync.ts index 465186df847..1cf28a21fff 100644 --- a/src/renderer/src/components/editor/monaco-programmatic-sync.ts +++ b/src/renderer/src/components/editor/monaco-programmatic-sync.ts @@ -1,37 +1,37 @@ -const programmaticContentSyncDepthByFilePath = new Map() +const programmaticContentSyncDepthByModelKey = new Map() -export function beginProgrammaticContentSync(filePath: string): void { - programmaticContentSyncDepthByFilePath.set( - filePath, - (programmaticContentSyncDepthByFilePath.get(filePath) ?? 0) + 1 +export function beginProgrammaticContentSync(modelKey: string): void { + programmaticContentSyncDepthByModelKey.set( + modelKey, + (programmaticContentSyncDepthByModelKey.get(modelKey) ?? 0) + 1 ) } -export function endProgrammaticContentSync(filePath: string): void { - const depth = programmaticContentSyncDepthByFilePath.get(filePath) ?? 0 +export function endProgrammaticContentSync(modelKey: string): void { + const depth = programmaticContentSyncDepthByModelKey.get(modelKey) ?? 0 if (depth <= 1) { - programmaticContentSyncDepthByFilePath.delete(filePath) + programmaticContentSyncDepthByModelKey.delete(modelKey) return } - programmaticContentSyncDepthByFilePath.set(filePath, depth - 1) + programmaticContentSyncDepthByModelKey.set(modelKey, depth - 1) } -export function isProgrammaticContentSyncInFlight(filePath: string): boolean { - return (programmaticContentSyncDepthByFilePath.get(filePath) ?? 0) > 0 +export function isProgrammaticContentSyncInFlight(modelKey: string): boolean { + return (programmaticContentSyncDepthByModelKey.get(modelKey) ?? 0) > 0 } export function shouldIgnoreMonacoContentChange(args: { - filePath: string + modelKey: string isApplyingProgrammaticContent: boolean }): boolean { - const { filePath, isApplyingProgrammaticContent } = args + const { modelKey, isApplyingProgrammaticContent } = args - // Why: split panes can share one retained Monaco model by file path. If any + // Why: split panes can share one retained model. If any // pane is currently reconciling prop content into that shared model, every // pane sees the echoed change event and must treat it as programmatic. - return isApplyingProgrammaticContent || isProgrammaticContentSyncInFlight(filePath) + return isApplyingProgrammaticContent || isProgrammaticContentSyncInFlight(modelKey) } export function resetProgrammaticContentSyncForTests(): void { - programmaticContentSyncDepthByFilePath.clear() + programmaticContentSyncDepthByModelKey.clear() } diff --git a/src/renderer/src/components/editor/use-monaco-content-sync-bridge.ts b/src/renderer/src/components/editor/use-monaco-content-sync-bridge.ts index 6a55d9a1ada..6fd4e63a089 100644 --- a/src/renderer/src/components/editor/use-monaco-content-sync-bridge.ts +++ b/src/renderer/src/components/editor/use-monaco-content-sync-bridge.ts @@ -24,10 +24,10 @@ export function useMonacoContentSyncBridge(params: { content: string contentRef: MutableRefObject contentSyncModeRef: MutableRefObject - filePath: string + modelKey: string onContentChange: (content: string) => void }): MonacoContentSyncBridge { - const { editorRef, content, contentRef, contentSyncModeRef, filePath, onContentChange } = params + const { editorRef, content, contentRef, contentSyncModeRef, modelKey, onContentChange } = params const lastSyncedContentRef = useRef(content) @@ -38,6 +38,9 @@ export function useMonacoContentSyncBridge(params: { const handleChange = useCallback( (value: string | undefined) => { if (value !== undefined) { + if (editorRef.current?.getModel()?.uri.toString() !== modelKey) { + return + } // Why: split panes share one retained model, so a sibling must ignore the echoed programmatic-sync onChange or it marks the file dirty. if (isApplyingLargePasteRef.current) { lastSyncedContentRef.current = value @@ -45,7 +48,7 @@ export function useMonacoContentSyncBridge(params: { } if ( shouldIgnoreMonacoContentChange({ - filePath, + modelKey, isApplyingProgrammaticContent: isApplyingProgrammaticContentRef.current }) ) { @@ -55,25 +58,29 @@ export function useMonacoContentSyncBridge(params: { onContentChange(value) } }, - [filePath, onContentChange] + [editorRef, modelKey, onContentChange] ) // Why: sync the model on external `content` drift; useLayoutEffect lands the overwrite before paint so no stale text flashes. On-mount handled in handleMount. useLayoutEffect(() => { const ed = editorRef.current - if (!ed || lastSyncedContentRef.current === content) { + if ( + !ed || + ed.getModel()?.uri.toString() !== modelKey || + lastSyncedContentRef.current === content + ) { return } - beginProgrammaticContentSync(filePath) + beginProgrammaticContentSync(modelKey) isApplyingProgrammaticContentRef.current = true try { syncContentUpdate(ed, content, contentSyncModeRef.current) lastSyncedContentRef.current = content } finally { isApplyingProgrammaticContentRef.current = false - endProgrammaticContentSync(filePath) + endProgrammaticContentSync(modelKey) } - }, [content, contentSyncModeRef, editorRef, filePath]) + }, [content, contentSyncModeRef, editorRef, modelKey]) return { contentRef, diff --git a/src/renderer/src/components/editor/use-monaco-editor-mount.ts b/src/renderer/src/components/editor/use-monaco-editor-mount.ts index da97dc6ec32..3e355ffcefd 100644 --- a/src/renderer/src/components/editor/use-monaco-editor-mount.ts +++ b/src/renderer/src/components/editor/use-monaco-editor-mount.ts @@ -3,6 +3,7 @@ import type { OnMount } from '@monaco-editor/react' import { useAppStore } from '@/store' import { registerFileSearchSelectedTextProvider } from '@/lib/file-search-selection' import { syncContentOnMount } from './monaco-content-sync' +import { toEditorModelUri } from './editor-model-uri' import { beginProgrammaticContentSync, endProgrammaticContentSync @@ -24,6 +25,7 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount { const { fileId, filePath, + modelOwnerKey, viewStateKey, viewStateId, worktreeId, @@ -103,7 +105,8 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount { updateMarkdownCompletionDocuments() // Why: see contentRef — reconcile the retained model to the current prop before user interaction (surfaces edits made while unmounted). - beginProgrammaticContentSync(filePath) + const modelKey = toEditorModelUri(filePath, modelOwnerKey) + beginProgrammaticContentSync(modelKey) isApplyingProgrammaticContentRef.current = true try { const didSyncOnMount = syncContentOnMount( @@ -116,7 +119,7 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount { } } finally { isApplyingProgrammaticContentRef.current = false - endProgrammaticContentSync(filePath) + endProgrammaticContentSync(modelKey) } setupCopy(editorInstance, monaco, filePath, propsRef) @@ -219,6 +222,7 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount { setupCopy, fileId, filePath, + modelOwnerKey, setEditorCursorLine, updateMarkdownCompletionDocuments, viewStateKey, diff --git a/tests/e2e/editor-model-host-isolation.spec.ts b/tests/e2e/editor-model-host-isolation.spec.ts new file mode 100644 index 00000000000..5c201a0ba54 --- /dev/null +++ b/tests/e2e/editor-model-host-isolation.spec.ts @@ -0,0 +1,110 @@ +import { randomUUID } from 'node:crypto' +import { rm, writeFile } from 'node:fs/promises' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { getActiveWorktreeContext } from './helpers/markdown-editor-fixture' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { buildOwnedEditorFileId } from '../../src/renderer/src/store/slices/editor/file-ids/editor-file-ids' + +test('keeps same-path host models and undo history separate', async ({ + orcaPage, + registerPostElectronShutdownCleanup +}, testInfo) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const context = await getActiveWorktreeContext(orcaPage) + const filePath = path.join(context.rootPath, `host-model-${randomUUID()}.txt`) + await writeFile(filePath, 'LOCAL_MARKER', 'utf8') + registerPostElectronShutdownCleanup(() => rm(filePath, { force: true })) + const remoteId = buildOwnedEditorFileId(filePath, context.worktreeId, 'host-model-fixture') + + // Controlled owner records exercise renderer identity; file transport remains local. + await orcaPage.evaluate( + ({ filePath, remoteId, worktreeId }) => { + const store = window.__store + if (!store) { + throw new Error('Editor store unavailable') + } + const base = { + filePath, + relativePath: filePath.split(/[\\/]/).pop() ?? filePath, + worktreeId, + language: 'plaintext', + mode: 'edit' as const, + isDirty: true + } + const generation = { + runtimeConnectionGeneration: null, + runtimePairingRevision: undefined, + runtimeSshGeneration: null, + nestedSshGeneration: null, + directSshGeneration: null + } + store.setState((state) => ({ + openFiles: [ + { + ...base, + id: filePath, + operationProvenance: { + ownershipProjection: 'explicit', + generation: { + ...generation, + route: { executionHostId: 'local', runtimeEnvironmentId: null } + } + } + }, + { + ...base, + id: remoteId, + operationProvenance: { + ownershipProjection: 'explicit', + generation: { + ...generation, + route: { executionHostId: 'ssh:fixture-target', runtimeEnvironmentId: null } + } + } + } + ], + editorDrafts: { + ...state.editorDrafts, + [filePath]: 'LOCAL_MARKER', + [remoteId]: 'REMOTE_MARKER' + }, + activeFileId: filePath, + activeFileIdByWorktree: { ...state.activeFileIdByWorktree, [worktreeId]: filePath } + })) + const state = store.getState() + const localTab = state.createUnifiedTab(worktreeId, 'editor', { + entityId: filePath, + label: 'Local file' + }) + state.createUnifiedTab(worktreeId, 'editor', { entityId: remoteId, label: 'Remote file' }) + store.getState().activateTab(localTab.id) + }, + { filePath, remoteId, worktreeId: context.worktreeId } + ) + + const editor = orcaPage.locator('.monaco-editor').first() + await expect(editor).toBeVisible({ timeout: 25_000 }) + const valueTail = () => orcaPage.evaluate(() => window.__monacoEditorE2E?.snapshot().valueTail) + await expect.poll(valueTail).toBe('LOCAL_MARKER') + await editor.click() + await orcaPage.keyboard.press('ControlOrMeta+End') + await orcaPage.keyboard.type('!') + await expect.poll(valueTail).toBe('LOCAL_MARKER!') + await orcaPage.evaluate((id) => window.__store?.getState().setActiveFile(id), remoteId) + await expect.poll(valueTail).toBe('REMOTE_MARKER') + await orcaPage.screenshot({ path: testInfo.outputPath('remote-before-undo.png') }) + await editor.click() + await orcaPage.keyboard.press('ControlOrMeta+z') + await orcaPage.screenshot({ path: testInfo.outputPath('remote-after-undo.png') }) + await expect.poll(valueTail).toBe('REMOTE_MARKER') + await orcaPage.keyboard.press('ControlOrMeta+End') + await orcaPage.keyboard.type('?') + await expect.poll(valueTail).toBe('REMOTE_MARKER?') + await orcaPage.keyboard.press('ControlOrMeta+z') + await expect.poll(valueTail).toBe('REMOTE_MARKER') + await orcaPage.evaluate((id) => window.__store?.getState().setActiveFile(id), filePath) + await expect.poll(valueTail).toBe('LOCAL_MARKER!') + await orcaPage.screenshot({ path: testInfo.outputPath('local-retained-edit.png') }) +})