From 6e3d35cf84726a5f8bb5497a46e732c1c1eee89c Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 14:18:09 -0700 Subject: [PATCH 01/10] Prevent duplicate editor records from surviving document close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapt the document identity, persistence dedupe, and orphan boundaries from #21124. Preserve current workspace-selection behavior, scope closes when legacy IDs repeat across workspaces, and use the captured file owner for mirrored closes. Co-authored-by: 김영민 --- src/renderer/src/hooks/ipc-tab-switch.test.ts | 24 + .../lib/workspace-session-editor-records.ts | 165 ++++++ .../src/lib/workspace-session.test.ts | 243 ++++++++- src/renderer/src/lib/workspace-session.ts | 63 +-- .../runtime/close-mirrored-editor-tab.test.ts | 68 +++ .../src/runtime/close-mirrored-editor-tab.ts | 54 +- .../editor-close-file-sibling-sweep.test.ts | 490 ++++++++++++++++++ .../editor/actions/close-file-action.ts | 271 +++++----- .../editor/actions/hydrate-editor-session.ts | 40 +- .../editor/actions/open-file-mutations.ts | 9 + .../file-ids/editor-document-identity.test.ts | 111 ++++ .../file-ids/editor-document-identity.ts | 44 ++ .../slices/editor/file-ids/editor-file-ids.ts | 42 +- .../file-ids/hydrated-editor-file-ids.ts | 3 +- .../file-ids/orphan-editor-file-ids.test.ts | 105 ++++ .../editor/file-ids/orphan-editor-file-ids.ts | 72 +++ ...reconciliation-orphan-editor-files.test.ts | 211 ++++++++ .../store/slices/tabs/tabs-reconciliation.ts | 15 + .../store/slices/tabs/tabs-session-actions.ts | 70 ++- 19 files changed, 1892 insertions(+), 208 deletions(-) create mode 100644 src/renderer/src/lib/workspace-session-editor-records.ts create mode 100644 src/renderer/src/store/slices/editor-close-file-sibling-sweep.test.ts create mode 100644 src/renderer/src/store/slices/editor/file-ids/editor-document-identity.test.ts create mode 100644 src/renderer/src/store/slices/editor/file-ids/editor-document-identity.ts create mode 100644 src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.test.ts create mode 100644 src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.ts create mode 100644 src/renderer/src/store/slices/tabs/tabs-reconciliation-orphan-editor-files.test.ts diff --git a/src/renderer/src/hooks/ipc-tab-switch.test.ts b/src/renderer/src/hooks/ipc-tab-switch.test.ts index d5d07642a1c8..1df0af838f78 100644 --- a/src/renderer/src/hooks/ipc-tab-switch.test.ts +++ b/src/renderer/src/hooks/ipc-tab-switch.test.ts @@ -464,6 +464,30 @@ describe('handleSwitchRecentTab', () => { expect(store.setActiveTabType).toHaveBeenCalledWith('browser', 'wt-1') }) + it('skips an MRU entry naming a tab that no longer renders', () => { + const store = makeStore('editor') + store.activeFileId = 'file-c' + store.groupsByWorktree = { + 'wt-1': [ + { + id: 'group-1', + activeTabId: 'tab-c', + tabOrder: ['tab-a', 'tab-c'], + recentTabIds: ['tab-a', 'tab-pruned', 'tab-c'] + } + ] + } + getStateMock.mockReturnValue(store) + getActiveTabNavOrderMock.mockReturnValue([ + { type: 'editor', id: 'file-a', tabId: 'tab-a' }, + { type: 'editor', id: 'file-c', tabId: 'tab-c' } + ]) + + expect(handleSwitchRecentTab()).toBe(true) + expect(store.setActiveFile).toHaveBeenCalledWith('file-a') + expect(store.activateTab).toHaveBeenCalledWith('tab-a') + }) + it('returns false when the MRU stack has no previous visible tab', () => { const store = makeStore('terminal') store.groupsByWorktree = { diff --git a/src/renderer/src/lib/workspace-session-editor-records.ts b/src/renderer/src/lib/workspace-session-editor-records.ts new file mode 100644 index 000000000000..5451751f54dc --- /dev/null +++ b/src/renderer/src/lib/workspace-session-editor-records.ts @@ -0,0 +1,165 @@ +import type { PersistedOpenFile } from '../../../shared/workspace-session-state-types' +import type { OpenFile } from '../store/slices/editor' +import { editorDocumentIdentityKey } from '../store/slices/editor/file-ids/editor-document-identity' + +export type PersistedEditorFileRecords = { + openFilesByWorktree: Record + /** Ids of the live OpenFiles that own a persisted record, per worktree. */ + editFileIdsByWorktree: Record> + /** Merged-away OpenFile id → the id whose record replaced it. */ + survivingFileIdByMergedId: Map +} + +function toPersistedOpenFile( + file: OpenFile, + editorDrafts: Record +): PersistedOpenFile { + // Why: never persist a dirty draft for a read-only tab — restoring one would reintroduce writable/hot-exit state for an agent transcript. + const dirtyDraftContent = + file.isDirty && file.readOnly !== true ? editorDrafts[file.id] : undefined + return { + filePath: file.filePath, + relativePath: file.relativePath, + worktreeId: file.worktreeId, + language: file.language, + isPreview: file.isPreview || undefined, + runtimeEnvironmentId: file.runtimeEnvironmentId, + externalSshTargetId: file.externalSshTargetId, + // Why: persist readOnly only when true; absence is the writable default on restore. + ...(file.readOnly === true ? { readOnly: true } : {}), + ...(file.readOnly === true && file.liveTail === true ? { liveTail: true } : {}), + ...(dirtyDraftContent !== undefined ? { dirtyDraftContent } : {}), + // Why: baseline travels with the draft so restore can detect a changed-on-disk conflict before autosave clobbers an offline agent write. + ...(dirtyDraftContent !== undefined && file.lastKnownDiskSignature + ? { lastKnownDiskSignature: file.lastKnownDiskSignature } + : {}) + } +} + +/** A record already written to the worktree's row list, and the live OpenFile that owns it. */ +type KeptRecord = { record: PersistedOpenFile; fileId: string; position: number } + +/** Two unsaved buffers that disagree: merging would silently destroy one of them. */ +function hasDivergentDraft(left: PersistedOpenFile, right: PersistedOpenFile): boolean { + return ( + left.dirtyDraftContent !== undefined && + right.dirtyDraftContent !== undefined && + left.dirtyDraftContent !== right.dirtyDraftContent + ) +} + +/** + * Why: merging drops the candidate's row, so a baseline it alone carries would be lost. The drafts + * are the same text, so the candidate's baseline describes the kept record's document too. + */ +function withCarriedDiskBaseline( + kept: PersistedOpenFile, + candidate: PersistedOpenFile +): PersistedOpenFile | null { + if ( + kept.dirtyDraftContent === undefined || + kept.dirtyDraftContent !== candidate.dirtyDraftContent || + kept.lastKnownDiskSignature !== undefined || + candidate.lastKnownDiskSignature === undefined + ) { + return null + } + return { ...kept, lastKnownDiskSignature: candidate.lastKnownDiskSignature } +} + +function winsOverKeptRecord( + candidate: { record: PersistedOpenFile; fileId: string }, + kept: { record: PersistedOpenFile; fileId: string }, + activeFileId: string | null | undefined +): boolean { + if (kept.record.dirtyDraftContent !== undefined) { + return false + } + if (candidate.record.dirtyDraftContent !== undefined) { + return true + } + // Why before the active-file rule: a preview record restores a tab the next single click replaces. + if (kept.record.isPreview !== candidate.record.isPreview) { + return kept.record.isPreview === true + } + return candidate.fileId === activeFileId && kept.fileId !== activeFileId +} + +function resolveSurvivingFileId(merged: Map, fileId: string): string { + let current = fileId + const seen = new Set([fileId]) + let next = merged.get(current) + while (next !== undefined && !seen.has(next)) { + seen.add(next) + current = next + next = merged.get(current) + } + return current +} + +/** Serialize the edit-mode documents, collapsing records that cannot be told apart on restore. */ +export function buildPersistedEditorFileRecords( + openFiles: readonly OpenFile[], + editorDrafts: Record, + activeFileIdByWorktree: Record +): PersistedEditorFileRecords { + const openFilesByWorktree: Record = {} + const editFileIdsByWorktree: Record> = {} + const survivingFileIdByMergedId = new Map() + // Why a list per identity: divergent drafts stay apart as separate rows, and a later duplicate + // must merge into whichever of them it agrees with — tracking only the first stacks a new row + // for every repeat of the second draft. + const keptVariantsByIdentity = new Map() + + for (const file of openFiles) { + if (file.mode !== 'edit') { + continue + } + const records = + openFilesByWorktree[file.worktreeId] ?? (openFilesByWorktree[file.worktreeId] = []) + const fileIds = + editFileIdsByWorktree[file.worktreeId] ?? (editFileIdsByWorktree[file.worktreeId] = new Set()) + const record = toPersistedOpenFile(file, editorDrafts) + const identity = editorDocumentIdentityKey(file) + const variants = keptVariantsByIdentity.get(identity) ?? [] + if (variants.length === 0) { + keptVariantsByIdentity.set(identity, variants) + } + const kept = variants.find((variant) => !hasDivergentDraft(variant.record, record)) + if (!kept) { + records.push(record) + fileIds.add(file.id) + variants.push({ record, fileId: file.id, position: records.length - 1 }) + continue + } + if ( + !winsOverKeptRecord( + { record, fileId: file.id }, + kept, + activeFileIdByWorktree[file.worktreeId] + ) + ) { + const withBaseline = withCarriedDiskBaseline(kept.record, record) + if (withBaseline) { + records[kept.position] = withBaseline + kept.record = withBaseline + } + survivingFileIdByMergedId.set(file.id, kept.fileId) + continue + } + records[kept.position] = record + fileIds.delete(kept.fileId) + fileIds.add(file.id) + survivingFileIdByMergedId.set(kept.fileId, file.id) + kept.record = record + kept.fileId = file.id + } + + for (const mergedId of survivingFileIdByMergedId.keys()) { + survivingFileIdByMergedId.set( + mergedId, + resolveSurvivingFileId(survivingFileIdByMergedId, mergedId) + ) + } + return { openFilesByWorktree, editFileIdsByWorktree, survivingFileIdByMergedId } +} diff --git a/src/renderer/src/lib/workspace-session.test.ts b/src/renderer/src/lib/workspace-session.test.ts index e38660bffcc1..974c5e6acac6 100644 --- a/src/renderer/src/lib/workspace-session.test.ts +++ b/src/renderer/src/lib/workspace-session.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' -import { buildWorkspaceSessionPayload } from './workspace-session' +import { buildEditorSessionData, buildWorkspaceSessionPayload } from './workspace-session' import type { AppState } from '../store' +import type { OpenFile } from '../store/slices/editor' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' function createSnapshot(overrides: Partial = {}): AppState { @@ -508,3 +509,243 @@ describe('buildWorkspaceSessionPayload', () => { expect(payload.activeTabTypeByWorktree).toEqual({ 'wt-2': 'terminal' }) }) }) + +const WORKTREE = 'wt-dup' +const FILE = '/tmp/dup/app.ts' + +function editOpenFile(id: string, overrides: Partial = {}): OpenFile { + const base: OpenFile = { + id, + filePath: FILE, + relativePath: 'app.ts', + worktreeId: WORKTREE, + language: 'typescript', + mode: 'edit', + isDirty: false, + isPreview: false, + runtimeEnvironmentId: null + } + return { ...base, ...overrides } +} + +function buildEditorSession( + openFiles: OpenFile[], + overrides: { + editorDrafts?: Record + markdownFrontmatterVisible?: Record + activeFileIdByWorktree?: Record + } = {} +) { + const session = buildEditorSessionData( + openFiles, + overrides.editorDrafts ?? {}, + overrides.markdownFrontmatterVisible ?? {}, + overrides.activeFileIdByWorktree ?? {}, + {} + ) + return { + openFilesByWorktree: session.openFilesByWorktree ?? {}, + activeFileIdByWorktree: session.activeFileIdByWorktree ?? {}, + markdownFrontmatterVisible: session.markdownFrontmatterVisible ?? {} + } +} + +describe('buildEditorSessionData duplicate collapse', () => { + it('writes one record for live files that share worktree, owner and path', () => { + const session = buildEditorSession([ + editOpenFile(FILE), + editOpenFile(`editor:a:${FILE}`), + editOpenFile(`editor:b:${FILE}`) + ]) + + expect(session.openFilesByWorktree[WORKTREE]).toHaveLength(1) + }) + + it('keeps the dirty record and its disk baseline over a clean duplicate', () => { + const session = buildEditorSession( + [ + editOpenFile(FILE), + editOpenFile(`editor:dirty:${FILE}`, { + isDirty: true, + lastKnownDiskSignature: 'sig-7' + }) + ], + { editorDrafts: { [`editor:dirty:${FILE}`]: 'unsaved text' } } + ) + + expect(session.openFilesByWorktree[WORKTREE]).toEqual([ + expect.objectContaining({ + dirtyDraftContent: 'unsaved text', + lastKnownDiskSignature: 'sig-7' + }) + ]) + }) + + it('prefers the active file over a clean duplicate', () => { + const session = buildEditorSession( + [editOpenFile(FILE, { isPreview: true }), editOpenFile(`editor:b:${FILE}`)], + { activeFileIdByWorktree: { [WORKTREE]: `editor:b:${FILE}` } } + ) + + expect(session.openFilesByWorktree[WORKTREE]).toEqual([ + expect.objectContaining({ isPreview: undefined }) + ]) + expect(session.activeFileIdByWorktree[WORKTREE]).toBe(`editor:b:${FILE}`) + }) + + it('remaps the active file id onto the record that outranked it', () => { + const session = buildEditorSession( + [editOpenFile(FILE, { isDirty: true }), editOpenFile(`editor:b:${FILE}`)], + { + editorDrafts: { [FILE]: 'unsaved text' }, + activeFileIdByWorktree: { [WORKTREE]: `editor:b:${FILE}` } + } + ) + + expect(session.openFilesByWorktree[WORKTREE]).toHaveLength(1) + expect(session.activeFileIdByWorktree[WORKTREE]).toBe(FILE) + }) + + it('never merges a read-only live-tail log with a writable record for the same path', () => { + const session = buildEditorSession( + [ + editOpenFile(FILE, { readOnly: true, liveTail: true }), + editOpenFile(`editor:writable:${FILE}`, { isDirty: true }) + ], + { editorDrafts: { [`editor:writable:${FILE}`]: 'unsaved text' } } + ) + + const [logRecord, writableRecord] = session.openFilesByWorktree[WORKTREE] + expect(logRecord).toEqual(expect.objectContaining({ readOnly: true, liveTail: true })) + expect(logRecord).not.toHaveProperty('dirtyDraftContent') + expect(writableRecord).toEqual(expect.objectContaining({ dirtyDraftContent: 'unsaved text' })) + expect(writableRecord).not.toHaveProperty('readOnly') + }) + + it('prefers a non-preview record over the active preview record', () => { + const previewFirst = buildEditorSession( + [editOpenFile(FILE, { isPreview: true }), editOpenFile(`editor:b:${FILE}`)], + { activeFileIdByWorktree: { [WORKTREE]: FILE } } + ) + const previewSecond = buildEditorSession( + [editOpenFile(FILE), editOpenFile(`editor:b:${FILE}`, { isPreview: true })], + { activeFileIdByWorktree: { [WORKTREE]: `editor:b:${FILE}` } } + ) + + expect(previewFirst.openFilesByWorktree[WORKTREE]).toEqual([ + expect.objectContaining({ isPreview: undefined }) + ]) + expect(previewSecond.openFilesByWorktree[WORKTREE]).toEqual([ + expect.objectContaining({ isPreview: undefined }) + ]) + }) + + it('keeps records apart when only the runtime owner or the ssh target differs', () => { + const session = buildEditorSession([ + editOpenFile(FILE), + editOpenFile(`editor:env-a:${FILE}`, { runtimeEnvironmentId: 'env-a' }), + editOpenFile(`editor:ssh:${FILE}`, { externalSshTargetId: 'ssh-target' }) + ]) + + expect(session.openFilesByWorktree[WORKTREE]).toHaveLength(3) + }) + + it('keeps both records when two duplicates hold divergent unsaved drafts', () => { + const session = buildEditorSession( + [editOpenFile(FILE, { isDirty: true }), editOpenFile(`editor:b:${FILE}`, { isDirty: true })], + { editorDrafts: { [FILE]: 'left text', [`editor:b:${FILE}`]: 'right text' } } + ) + + expect(session.openFilesByWorktree[WORKTREE].map((record) => record.dirtyDraftContent)).toEqual( + ['left text', 'right text'] + ) + }) + + it('carries a merged front-matter override onto the surviving record', () => { + const session = buildEditorSession([editOpenFile(FILE), editOpenFile(`editor:b:${FILE}`)], { + markdownFrontmatterVisible: { [`editor:b:${FILE}`]: false } + }) + + expect(session.markdownFrontmatterVisible).toEqual({ [FILE]: false }) + }) + + it('merges a repeated draft into the variant it agrees with', () => { + const ids = [FILE, `editor:b:${FILE}`, `editor:c:${FILE}`] + const session = buildEditorSession( + ids.map((id) => editOpenFile(id, { isDirty: true })), + { + editorDrafts: { + [ids[0]]: 'left text', + [ids[1]]: 'right text', + [ids[2]]: 'right text' + } + } + ) + + expect(session.openFilesByWorktree[WORKTREE].map((record) => record.dirtyDraftContent)).toEqual( + ['left text', 'right text'] + ) + }) + + it('carries the merged duplicate disk baseline onto the equal-draft survivor', () => { + const ids = [FILE, `editor:b:${FILE}`] + const session = buildEditorSession( + [ + editOpenFile(ids[0], { isDirty: true }), + editOpenFile(ids[1], { isDirty: true, lastKnownDiskSignature: 'sig' }) + ], + { editorDrafts: { [ids[0]]: 'same text', [ids[1]]: 'same text' } } + ) + + expect(session.openFilesByWorktree[WORKTREE]).toEqual([ + expect.objectContaining({ dirtyDraftContent: 'same text', lastKnownDiskSignature: 'sig' }) + ]) + }) + + it('remaps the active file id onto the variant that absorbed it', () => { + const ids = [FILE, `editor:b:${FILE}`, `editor:c:${FILE}`] + const session = buildEditorSession( + ids.map((id) => editOpenFile(id, { isDirty: true })), + { + editorDrafts: { + [ids[0]]: 'left text', + [ids[1]]: 'right text', + [ids[2]]: 'right text' + }, + activeFileIdByWorktree: { [WORKTREE]: ids[2] } + } + ) + + expect(session.activeFileIdByWorktree[WORKTREE]).toBe(ids[1]) + }) + + it('merges clean duplicates on both sides of a drafted record', () => { + const ids = [FILE, `editor:b:${FILE}`, `editor:c:${FILE}`] + const session = buildEditorSession( + [editOpenFile(ids[0]), editOpenFile(ids[1], { isDirty: true }), editOpenFile(ids[2])], + { editorDrafts: { [ids[1]]: 'right text' } } + ) + + expect(session.openFilesByWorktree[WORKTREE].map((record) => record.dirtyDraftContent)).toEqual( + ['right text'] + ) + }) + + it('keeps two records when a draft repeats after a divergent one', () => { + const ids = [FILE, `editor:b:${FILE}`, `editor:c:${FILE}`] + const session = buildEditorSession( + ids.map((id) => editOpenFile(id, { isDirty: true })), + { + editorDrafts: { + [ids[0]]: 'left text', + [ids[1]]: 'right text', + [ids[2]]: 'left text' + } + } + ) + + expect(session.openFilesByWorktree[WORKTREE].map((record) => record.dirtyDraftContent)).toEqual( + ['left text', 'right text'] + ) + }) +}) diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index 4bf57f6d82fc..b536f01df883 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -1,8 +1,6 @@ import type { WorkspaceVisibleTabType } from '../../../shared/tab-types' -import type { - PersistedOpenFile, - WorkspaceSessionState -} from '../../../shared/workspace-session-state-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { buildPersistedEditorFileRecords } from './workspace-session-editor-records' import { pruneLocalTerminalScrollbackBuffers } from '../../../shared/workspace-session-terminal-buffers' import { normalizeBrowserHistoryEntries } from '../../../shared/workspace-session-browser-history' import { normalizeWorkspaceDocHistoryEntries } from '../../../shared/workspace-doc-history' @@ -125,42 +123,21 @@ export function buildEditorSessionData( | 'activeTabTypeByWorktree' | 'markdownFrontmatterVisible' > { - const editFiles = openFiles.filter((f) => f.mode === 'edit') - const byWorktree: Record = {} - const editFileIdsByWorktree: Record> = {} - for (const f of editFiles) { - const arr = byWorktree[f.worktreeId] ?? (byWorktree[f.worktreeId] = []) - // Why: never persist a dirty draft for a read-only tab — restoring one would reintroduce writable/hot-exit state for an agent transcript. - const dirtyDraftContent = f.isDirty && f.readOnly !== true ? editorDrafts[f.id] : undefined - arr.push({ - filePath: f.filePath, - relativePath: f.relativePath, - worktreeId: f.worktreeId, - language: f.language, - isPreview: f.isPreview || undefined, - runtimeEnvironmentId: f.runtimeEnvironmentId, - externalSshTargetId: f.externalSshTargetId, - // Why: persist readOnly only when true; absence is the writable default on restore. - ...(f.readOnly === true ? { readOnly: true } : {}), - ...(f.readOnly === true && f.liveTail === true ? { liveTail: true } : {}), - ...(dirtyDraftContent !== undefined ? { dirtyDraftContent } : {}), - // Why: baseline travels with the draft so restore can detect a changed-on-disk conflict before autosave clobbers an offline agent write. - ...(dirtyDraftContent !== undefined && f.lastKnownDiskSignature - ? { lastKnownDiskSignature: f.lastKnownDiskSignature } - : {}) - }) - const ids = - editFileIdsByWorktree[f.worktreeId] ?? (editFileIdsByWorktree[f.worktreeId] = new Set()) - ids.add(f.id) - } + const { + openFilesByWorktree: byWorktree, + editFileIdsByWorktree, + survivingFileIdByMergedId + } = buildPersistedEditorFileRecords(openFiles, editorDrafts, activeFileIdByWorktree) const activeFileEntries: [string, string][] = [] for (const [worktreeId, fileId] of Object.entries(activeFileIdByWorktree)) { if (!fileId) { continue } - if (editFileIdsByWorktree[worktreeId]?.has(fileId)) { - activeFileEntries.push([worktreeId, fileId]) + // Why: the active tab can be the one whose record merged away — follow it to the survivor. + const survivingFileId = survivingFileIdByMergedId.get(fileId) ?? fileId + if (editFileIdsByWorktree[worktreeId]?.has(survivingFileId)) { + activeFileEntries.push([worktreeId, survivingFileId]) } } const persistedActiveFileIdByWorktree = Object.fromEntries(activeFileEntries) as Record< @@ -185,11 +162,21 @@ export function buildEditorSessionData( > const allEditFileIds = new Set(Object.values(editFileIdsByWorktree).flatMap((ids) => [...ids])) // Why: preserve the value so per-file hide overrides survive restart (map only carries `false`; visible is the default). - const persistedMarkdownFrontmatterVisible = Object.fromEntries( - Object.entries(markdownFrontmatterVisible ?? {}).filter(([fileId]) => - allEditFileIds.has(fileId) - ) + // A merged-away id carries its override onto the surviving record unless that record set its own. + const frontmatterEntries = Object.entries(markdownFrontmatterVisible ?? {}).flatMap( + ([fileId, visible]): [string, boolean][] => { + if (allEditFileIds.has(fileId)) { + return [[fileId, visible]] + } + const survivingFileId = survivingFileIdByMergedId.get(fileId) + return survivingFileId && + allEditFileIds.has(survivingFileId) && + markdownFrontmatterVisible?.[survivingFileId] === undefined + ? [[survivingFileId, visible]] + : [] + } ) + const persistedMarkdownFrontmatterVisible = Object.fromEntries(frontmatterEntries) return { openFilesByWorktree: byWorktree, diff --git a/src/renderer/src/runtime/close-mirrored-editor-tab.test.ts b/src/renderer/src/runtime/close-mirrored-editor-tab.test.ts index b6bd926e1f5c..6ca7188aa6c8 100644 --- a/src/renderer/src/runtime/close-mirrored-editor-tab.test.ts +++ b/src/renderer/src/runtime/close-mirrored-editor-tab.test.ts @@ -21,6 +21,22 @@ import { resetWebSessionCloseIntentForTests } from './web-session-close-intent' import { toHostSessionTabId } from '../../../shared/terminal-surface-id' +import type { Tab } from '../../../shared/tab-types' + +function mirroredEditorTab(id: string): Tab { + return { + id, + entityId: 'file-1', + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'editor', + label: id, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} function buildState(overrides: Partial = {}): MirroredEditorCloseState { return { @@ -40,6 +56,24 @@ describe('notifyHostOfMirroredEditorClose', () => { resetWebSessionCloseIntentForTests() }) + it('uses the file owner when workspace focus points at another server', async () => { + const state = buildState() + state.openFiles = state.openFiles.map((file) => ({ + ...file, + runtimeEnvironmentId: 'owning-host' + })) + notifyHostOfMirroredEditorClose(state, 'wt-1', 'file-1') + await vi.waitFor(() => + expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({ + worktreeId: 'wt-1', + tabId: 'host-tab-1', + environmentId: 'owning-host', + reason: 'user' + }) + ) + expect(getRuntimeEnvironmentIdForWorktreeMock).not.toHaveBeenCalled() + }) + it('records the host close intent SYNCHRONOUSLY (before the async close resolves)', () => { const now = Date.now() // No await: the intent must be pending the instant the call returns, so a @@ -71,6 +105,40 @@ describe('notifyHostOfMirroredEditorClose', () => { }) }) + it('closes every mirrored tab that renders the file, not just the first', async () => { + const now = Date.now() + const state = buildState({ + unifiedTabsByWorktree: { + 'wt-1': [mirroredEditorTab('host-tab-1'), mirroredEditorTab('host-tab-2')] + } + }) + + const handled = notifyHostOfMirroredEditorClose(state, 'wt-1', 'file-1') + + expect(handled).toBe(true) + for (const tabId of ['host-tab-1', 'host-tab-2']) { + expect( + isWebSessionCloseIntentPending( + { environmentId: 'env-1' }, + 'wt-1', + toHostSessionTabId(tabId), + now + ) + ).toBe(true) + } + await vi.waitFor(() => { + expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledTimes(2) + }) + for (const tabId of ['host-tab-1', 'host-tab-2']) { + expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({ + worktreeId: 'wt-1', + tabId, + environmentId: 'env-1', + reason: 'user' + }) + } + }) + it('records and sends a close for every host tab linked to the document', async () => { const state = buildState() const original = state.unifiedTabsByWorktree['wt-1'][0] diff --git a/src/renderer/src/runtime/close-mirrored-editor-tab.ts b/src/renderer/src/runtime/close-mirrored-editor-tab.ts index f8b0e8c54d67..82fcd41a600f 100644 --- a/src/renderer/src/runtime/close-mirrored-editor-tab.ts +++ b/src/renderer/src/runtime/close-mirrored-editor-tab.ts @@ -3,6 +3,11 @@ import { type WorktreeRuntimeOwnerState } from '@/lib/worktree-runtime-owner' import { recordWebSessionCloseIntent } from './web-session-close-intent' +import { parseExecutionHostId } from '../../../shared/execution-host' +import { + getOpenFileExecutionHostId, + hasOpenFileExecutionHostEvidence +} from '@/lib/unified-tab-host-ownership' import { toHostSessionTabId } from '../../../shared/terminal-surface-id' import type { OpenFile } from '@/store/slices/editor' import type { Tab } from '../../../shared/tab-types' @@ -23,48 +28,53 @@ export function notifyHostOfMirroredEditorClose( if (!worktreeId) { return false } - const file = state.openFiles.find((candidate) => candidate.id === fileId) + const file = state.openFiles.find( + (candidate) => candidate.id === fileId && candidate.worktreeId === worktreeId + ) if (!file?.mirroredFromRuntimeSession) { return false } - // Only legacy records need catalog inference; snapshots capture their authoritative runtime owner. + const capturedHost = parseExecutionHostId(getOpenFileExecutionHostId(file)) const runtimeEnvironmentId = - file.runtimeEnvironmentId === undefined - ? getRuntimeEnvironmentIdForWorktree(state, worktreeId) - : file.runtimeEnvironmentId + file.runtimeEnvironmentId !== undefined || hasOpenFileExecutionHostEvidence(file) + ? capturedHost?.kind === 'runtime' + ? capturedHost.environmentId + : null + : getRuntimeEnvironmentIdForWorktree(state, worktreeId) if (!runtimeEnvironmentId?.trim()) { return false } // A mirrored unified tab carries the host's tab id as `id` and the local file id as `entityId`; the host close RPC resolves by id. + // Why every match, not the first: a split view mirrors one file under several tabs, and a tab left + // un-notified keeps the host's copy open, which re-mirrors the file on the next snapshot. const unifiedTabs = (state.unifiedTabsByWorktree[worktreeId] ?? []).filter( (tab) => tab.contentType === 'editor' && tab.entityId === fileId ) if (unifiedTabs.length === 0) { return false } - // Record the close intent SYNCHRONOUSLY so a host snapshot landing before the dynamic import below resolves can't - // flash the old-path tab back. closeWebRuntimeSessionTab re-records it idempotently. - for (const tab of unifiedTabs) { + const closedAt = Date.now() + for (const unifiedTab of unifiedTabs) { + // Record the close intent SYNCHRONOUSLY so a host snapshot landing before the dynamic import below resolves can't + // flash the old-path tab back. closeWebRuntimeSessionTab re-records it idempotently. recordWebSessionCloseIntent( { environmentId: runtimeEnvironmentId }, worktreeId, - toHostSessionTabId(tab.id), - Date.now() + toHostSessionTabId(unifiedTab.id), + closedAt ) } // Dynamic import: this helper is imported by the editor slice during store creation, so importing // web-runtime-session eagerly imports the store back and trips cyclic init in full-suite import order. - void import('./web-runtime-session').then(({ closeWebRuntimeSessionTab }) => - Promise.all( - unifiedTabs.map((tab) => - closeWebRuntimeSessionTab({ - worktreeId, - tabId: tab.id, - environmentId: runtimeEnvironmentId, - reason: 'user' - }) - ) - ) - ) + void import('./web-runtime-session').then(({ closeWebRuntimeSessionTab }) => { + for (const unifiedTab of unifiedTabs) { + closeWebRuntimeSessionTab({ + worktreeId, + tabId: unifiedTab.id, + environmentId: runtimeEnvironmentId, + reason: 'user' + }) + } + }) return true } diff --git a/src/renderer/src/store/slices/editor-close-file-sibling-sweep.test.ts b/src/renderer/src/store/slices/editor-close-file-sibling-sweep.test.ts new file mode 100644 index 000000000000..404217c7de0f --- /dev/null +++ b/src/renderer/src/store/slices/editor-close-file-sibling-sweep.test.ts @@ -0,0 +1,490 @@ +import type { StoreApi } from 'zustand/vanilla' +import { describe, expect, it, vi } from 'vitest' +import { createEditorTabsStore } from './editor-slice-test-harness' +import type { AppState } from '../types' +import type { Tab, TabGroup } from '../../../../shared/tab-types' +import type { OpenFile } from './editor' + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), info: vi.fn(), success: vi.fn() } })) + +const { notifyHostOfMirroredEditorCloseMock } = vi.hoisted(() => ({ + notifyHostOfMirroredEditorCloseMock: vi.fn() +})) +vi.mock('@/runtime/close-mirrored-editor-tab', () => ({ + notifyHostOfMirroredEditorClose: (...args: unknown[]) => + notifyHostOfMirroredEditorCloseMock(...args) +})) + +const WORKTREE_ID = 'wt-1' +const FILE_PATH = '/repo/app.ts' +const GROUP_ID = 'group-1' + +function openFile(id: string, overrides: Partial = {}): OpenFile { + const base: OpenFile = { + id, + filePath: FILE_PATH, + relativePath: 'app.ts', + worktreeId: WORKTREE_ID, + language: 'typescript', + mode: 'edit', + isDirty: false, + isPreview: false, + runtimeEnvironmentId: null + } + return { ...base, ...overrides } +} + +function editorTab(id: string, entityId: string, groupId = GROUP_ID): Tab { + return { + id, + entityId, + groupId, + worktreeId: WORKTREE_ID, + contentType: 'editor', + label: 'app.ts', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +function group(id: string, tabIds: string[]): TabGroup { + return { + id, + worktreeId: WORKTREE_ID, + activeTabId: tabIds[0] ?? null, + tabOrder: tabIds, + recentTabIds: tabIds.toReversed() + } +} + +function seed(store: StoreApi, openFiles: OpenFile[], tabs: Tab[], groups: TabGroup[]) { + store.setState({ + openFiles, + activeFileId: openFiles[0]?.id ?? null, + activeFileIdByWorktree: { [WORKTREE_ID]: openFiles[0]?.id ?? null }, + unifiedTabsByWorktree: { [WORKTREE_ID]: tabs }, + groupsByWorktree: { [WORKTREE_ID]: groups }, + activeGroupIdByWorktree: { [WORKTREE_ID]: groups[0]?.id ?? GROUP_ID }, + layoutByWorktree: { + [WORKTREE_ID]: { type: 'leaf' as const, groupId: groups[0]?.id ?? GROUP_ID } + }, + tabBarOrderByWorktree: { [WORKTREE_ID]: openFiles.map((file) => file.id) } + }) +} + +describe('closeFile duplicate-record sweep', () => { + it('preserves another workspace record and metadata when it reuses the same bare file ID', () => { + const store = createEditorTabsStore() + const foreignWorktree = 'folder:foreign' + const local = openFile(FILE_PATH) + const foreign = openFile(FILE_PATH, { + worktreeId: foreignWorktree, + filePath: '/foreign/app.ts' + }) + seed( + store, + [local, foreign], + [editorTab('local-tab', FILE_PATH)], + [group(GROUP_ID, ['local-tab'])] + ) + const foreignTab = { ...editorTab('foreign-tab', FILE_PATH), worktreeId: foreignWorktree } + store.setState({ + editorDrafts: { [FILE_PATH]: 'foreign pending draft' }, + unifiedTabsByWorktree: { + ...store.getState().unifiedTabsByWorktree, + [foreignWorktree]: [foreignTab] + }, + activeFileIdByWorktree: { [WORKTREE_ID]: FILE_PATH, [foreignWorktree]: FILE_PATH } + }) + store.getState().closeFile(FILE_PATH) + expect(store.getState().openFiles).toEqual([foreign]) + expect(store.getState().unifiedTabsByWorktree[foreignWorktree]).toEqual([foreignTab]) + expect(store.getState().editorDrafts[FILE_PATH]).toBe('foreign pending draft') + expect(store.getState().activeFileIdByWorktree[foreignWorktree]).toBe(FILE_PATH) + }) + + it('closes every duplicate record of the document and all of their tabs', () => { + const store = createEditorTabsStore() + const ids = [FILE_PATH, 'editor:dup-a', 'editor:dup-b'] + seed( + store, + ids.map((id) => openFile(id)), + ids.map((id) => editorTab(`tab:${id}`, id)), + [ + group( + GROUP_ID, + ids.map((id) => `tab:${id}`) + ) + ] + ) + + store.getState().closeFile('editor:dup-a') + + expect(store.getState().openFiles).toEqual([]) + expect(store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + const remainingGroup = store.getState().groupsByWorktree[WORKTREE_ID]?.[0] + expect(remainingGroup?.tabOrder ?? []).toEqual([]) + expect(remainingGroup?.recentTabIds ?? []).toEqual([]) + expect(store.getState().tabBarOrderByWorktree[WORKTREE_ID]).toEqual([]) + expect(store.getState().activeFileIdByWorktree[WORKTREE_ID]).toBeNull() + }) + + it('pushes exactly one reopen snapshot for the swept document', () => { + const store = createEditorTabsStore() + const ids = [FILE_PATH, 'editor:dup-a', 'editor:dup-b'] + seed( + store, + ids.map((id) => openFile(id)), + ids.map((id) => editorTab(`tab:${id}`, id)), + [ + group( + GROUP_ID, + ids.map((id) => `tab:${id}`) + ) + ] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().recentlyClosedEditorTabsByWorktree[WORKTREE_ID]).toHaveLength(1) + expect(store.getState().recentlyClosedTabKindsByWorktree[WORKTREE_ID]).toEqual(['editor']) + expect(store.getState().reopenClosedEditorTab(WORKTREE_ID)).toBe(true) + expect(store.getState().openFiles).toHaveLength(1) + // Why: three tab closes must not leave two more reopen presses backed by nothing. + expect(store.getState().reopenClosedEditorTab(WORKTREE_ID)).toBe(false) + }) + + it('tells the host about a mirrored duplicate swept under another id', () => { + const store = createEditorTabsStore() + notifyHostOfMirroredEditorCloseMock.mockClear() + const files = [ + openFile(FILE_PATH), + openFile('editor:mirrored', { mirroredFromRuntimeSession: true }) + ] + seed( + store, + files, + files.map((file) => editorTab(`tab:${file.id}`, file.id)), + [ + group( + GROUP_ID, + files.map((file) => `tab:${file.id}`) + ) + ] + ) + + store.getState().closeFile(FILE_PATH) + + expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledWith( + expect.anything(), + WORKTREE_ID, + 'editor:mirrored' + ) + }) + + it('keeps a duplicate that holds unsaved work instead of discarding it', () => { + const store = createEditorTabsStore() + const files = [openFile(FILE_PATH), openFile('editor:dirty', { isDirty: true })] + seed( + store, + files, + files.map((file) => editorTab(`tab:${file.id}`, file.id)), + [ + group( + GROUP_ID, + files.map((file) => `tab:${file.id}`) + ) + ] + ) + store.setState({ editorDrafts: { 'editor:dirty': 'unsaved text' } }) + + store.getState().closeFile(FILE_PATH) + + // The save/discard prompt only asked about the named id, so the rival buffer stays open. + expect(store.getState().openFiles.map((file) => file.id)).toEqual(['editor:dirty']) + expect(store.getState().editorDrafts['editor:dirty']).toBe('unsaved text') + expect( + (store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.entityId) + ).toEqual(['editor:dirty']) + }) + + it('never sweeps another diff variant of the same path', () => { + const store = createEditorTabsStore() + const files = [ + openFile('diff:unstaged', { mode: 'diff', diffSource: 'unstaged' }), + openFile('diff:staged', { mode: 'diff', diffSource: 'staged' }), + openFile('diff:combined-all', { mode: 'diff', diffSource: 'combined-all' }), + openFile('diff:combined-branch', { mode: 'diff', diffSource: 'combined-branch' }) + ] + seed( + store, + files, + files.map((file) => editorTab(`tab:${file.id}`, file.id)), + [ + group( + GROUP_ID, + files.map((file) => `tab:${file.id}`) + ) + ] + ) + + store.getState().closeFile('diff:unstaged') + store.getState().closeFile('diff:combined-all') + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + 'diff:staged', + 'diff:combined-branch' + ]) + }) + + it('leaves the same path open under a different owner or mode', () => { + const store = createEditorTabsStore() + const files = [ + openFile(FILE_PATH), + openFile('editor:runtime', { runtimeEnvironmentId: 'env-a' }), + openFile('editor:ssh', { externalSshTargetId: 'ssh-target' }), + openFile('editor:diff', { mode: 'diff' }) + ] + seed( + store, + files, + files.map((file) => editorTab(`tab:${file.id}`, file.id)), + [ + group( + GROUP_ID, + files.map((file) => `tab:${file.id}`) + ) + ] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + 'editor:runtime', + 'editor:ssh', + 'editor:diff' + ]) + }) + + it('refuses to reselect a swept document', () => { + const store = createEditorTabsStore() + const ids = [FILE_PATH, 'editor:dup-a'] + seed( + store, + ids.map((id) => openFile(id)), + ids.map((id) => editorTab(`tab:${id}`, id)), + [ + group( + GROUP_ID, + ids.map((id) => `tab:${id}`) + ) + ] + ) + + store.getState().closeFile(FILE_PATH) + store.getState().setActiveFile('editor:dup-a') + + expect(store.getState().activeFileId).toBeNull() + expect(store.getState().activeFileIdByWorktree[WORKTREE_ID]).toBeNull() + + // Same guard for an id the store never knew. + store.getState().setActiveFile('/repo/never-opened.ts') + + expect(store.getState().activeFileId).toBeNull() + }) + + it('keeps a split view intact until the document itself closes', () => { + const store = createEditorTabsStore() + seed( + store, + [openFile(FILE_PATH)], + [ + editorTab('tab:left', FILE_PATH, 'group-left'), + editorTab('tab:right', FILE_PATH, 'group-right') + ], + [group('group-left', ['tab:left']), group('group-right', ['tab:right'])] + ) + + store.getState().closeUnifiedTab('tab:left') + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([FILE_PATH]) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles).toEqual([]) + expect(store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + }) + + it('leaves another worktree whose tab reuses the closed file id untouched', () => { + const store = createEditorTabsStore() + const otherWorktreeId = 'wt-2' + const otherGroupId = 'group-2' + seed( + store, + [openFile(FILE_PATH)], + [editorTab(`tab:${FILE_PATH}`, FILE_PATH)], + [group(GROUP_ID, [`tab:${FILE_PATH}`])] + ) + store.setState({ + unifiedTabsByWorktree: { + ...store.getState().unifiedTabsByWorktree, + // Why the shared entity id: an unoccupied editor id is just the file path, so two workspaces + // can name the same tab entity — the scan must not reach across on that coincidence. + [otherWorktreeId]: [ + { + ...editorTab('tab:wt-2', FILE_PATH, otherGroupId), + worktreeId: otherWorktreeId + } + ] + }, + groupsByWorktree: { + ...store.getState().groupsByWorktree, + [otherWorktreeId]: [ + { + id: otherGroupId, + worktreeId: otherWorktreeId, + activeTabId: 'tab:wt-2', + tabOrder: ['tab:wt-2'], + recentTabIds: ['tab:wt-2'] + } + ] + }, + layoutByWorktree: { + ...store.getState().layoutByWorktree, + [otherWorktreeId]: { type: 'leaf' as const, groupId: otherGroupId } + } + }) + const otherTabsBefore = store.getState().unifiedTabsByWorktree[otherWorktreeId] + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().unifiedTabsByWorktree[otherWorktreeId]).toBe(otherTabsBefore) + expect(store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + }) + + it('selects the closed file`s neighbour instead of skipping past the swept siblings', () => { + const store = createEditorTabsStore() + const files = [ + openFile('editor:x', { filePath: '/repo/x.ts', relativePath: 'x.ts' }), + openFile('editor:dup-a'), + openFile('editor:dup-b'), + openFile('editor:y', { filePath: '/repo/y.ts', relativePath: 'y.ts' }), + openFile('editor:z', { filePath: '/repo/z.ts', relativePath: 'z.ts' }) + ] + // Why no tab model: with one hydrated, the unified close path re-derives the selection from the + // group's active tab and closeFile's own reselect never shows. + store.setState({ + openFiles: files, + activeFileId: 'editor:dup-b', + activeFileIdByWorktree: { [WORKTREE_ID]: 'editor:dup-b' }, + tabBarOrderByWorktree: { [WORKTREE_ID]: files.map((file) => file.id) } + }) + + store.getState().closeFile('editor:dup-b') + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + 'editor:x', + 'editor:y', + 'editor:z' + ]) + // dup-a is swept with dup-b, so the survivor that took their place is y — not z. + expect(store.getState().activeFileIdByWorktree[WORKTREE_ID]).toBe('editor:y') + expect(store.getState().activeFileId).toBe('editor:y') + }) + + it('never sweeps a read-only live-tail log along with the writable tab for its path', () => { + const store = createEditorTabsStore() + const logId = 'editor:log' + seed( + store, + [openFile(FILE_PATH), openFile(logId, { readOnly: true, liveTail: true })], + [editorTab(`tab:${FILE_PATH}`, FILE_PATH), editorTab(`tab:${logId}`, logId)], + [group(GROUP_ID, [`tab:${FILE_PATH}`, `tab:${logId}`])] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([logId]) + expect( + (store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.entityId) + ).toEqual([logId]) + }) + + it('never sweeps the writable tab along with the read-only log for its path', () => { + const store = createEditorTabsStore() + const logId = 'editor:log' + seed( + store, + [openFile(FILE_PATH), openFile(logId, { readOnly: true, liveTail: true })], + [editorTab(`tab:${FILE_PATH}`, FILE_PATH), editorTab(`tab:${logId}`, logId)], + [group(GROUP_ID, [`tab:${FILE_PATH}`, `tab:${logId}`])] + ) + + store.getState().closeFile(logId) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([FILE_PATH]) + expect( + (store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.entityId) + ).toEqual([FILE_PATH]) + }) + + it('keeps a clean duplicate whose tab is pinned', () => { + const store = createEditorTabsStore() + const pinnedId = 'editor:dup-pinned' + seed( + store, + [openFile(FILE_PATH), openFile(pinnedId)], + [ + editorTab(`tab:${FILE_PATH}`, FILE_PATH), + { ...editorTab(`tab:${pinnedId}`, pinnedId), isPinned: true } + ], + [group(GROUP_ID, [`tab:${FILE_PATH}`, `tab:${pinnedId}`])] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([pinnedId]) + expect( + (store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.entityId) + ).toEqual([pinnedId]) + }) + + it('sweeps the same clean duplicate once its tab is unpinned', () => { + const store = createEditorTabsStore() + const siblingId = 'editor:dup-pinned' + seed( + store, + [openFile(FILE_PATH), openFile(siblingId)], + [editorTab(`tab:${FILE_PATH}`, FILE_PATH), editorTab(`tab:${siblingId}`, siblingId)], + [group(GROUP_ID, [`tab:${FILE_PATH}`, `tab:${siblingId}`])] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles).toEqual([]) + expect(store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + }) + + it('still closes the named record when its own tab is pinned', () => { + const store = createEditorTabsStore() + const siblingId = 'editor:dup-pinned' + seed( + store, + [openFile(FILE_PATH), openFile(siblingId)], + [ + { ...editorTab(`tab:${FILE_PATH}`, FILE_PATH), isPinned: true }, + { ...editorTab(`tab:${siblingId}`, siblingId), isPinned: true } + ], + [group(GROUP_ID, [`tab:${FILE_PATH}`, `tab:${siblingId}`])] + ) + + store.getState().closeFile(FILE_PATH) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([siblingId]) + expect( + (store.getState().unifiedTabsByWorktree[WORKTREE_ID] ?? []).map((tab) => tab.entityId) + ).toEqual([siblingId]) + }) +}) diff --git a/src/renderer/src/store/slices/editor/actions/close-file-action.ts b/src/renderer/src/store/slices/editor/actions/close-file-action.ts index 0d7799d8e797..12da12b4b8bb 100644 --- a/src/renderer/src/store/slices/editor/actions/close-file-action.ts +++ b/src/renderer/src/store/slices/editor/actions/close-file-action.ts @@ -2,70 +2,18 @@ import type { EditorGet, EditorSet } from '../types/editor-set-get' import type { EditorSlice } from '../types/editor-slice' import { getRecentlyClosedTabPosition, pushRecentlyClosedTabKind } from '../../recently-closed-tabs' import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab' -import { - type ClosedEditorTabSnapshot, - MAX_RECENT_CLOSED_EDITOR_TABS, - type OpenFile -} from '../types/open-file' +import { MAX_RECENT_CLOSED_EDITOR_TABS, type OpenFile } from '../types/open-file' import { removeMarkdownVisibilityKeys } from '../tabs/workspace-editor-item' +import { isEditorTabContentType } from '../tabs/editor-tab-content-type' +import { + collectSameDocumentOpenFileIds, + mayShareEditorBackingFile +} from '../file-ids/editor-file-ids' import { deleteUntouchedUntitledFile, shouldDeleteUntouchedUntitledFile } from '../tabs/untitled-file-cleanup' import { unifiedTabsKeepWorktreeSelected } from './unified-tabs-keep-worktree-selected' -import { isSameEditorOwner, mayShareEditorBackingFile } from '../file-ids/editor-file-ids' - -function isSameDocumentOwner(candidate: OpenFile, closedFile: OpenFile): boolean { - const expectedRoute = closedFile.operationProvenance?.generation.route - const actualRoute = candidate.operationProvenance?.generation.route - return ( - isSameEditorOwner(candidate, closedFile.worktreeId, closedFile.runtimeEnvironmentId) && - candidate.filePath === closedFile.filePath && - candidate.externalSshTargetId === closedFile.externalSshTargetId && - actualRoute?.executionHostId === expectedRoute?.executionHostId && - actualRoute?.runtimeEnvironmentId === expectedRoute?.runtimeEnvironmentId - ) -} - -function isSameOwnedDocument(candidate: OpenFile, closedFile: OpenFile): boolean { - const expected = closedFile.operationProvenance - const actual = candidate.operationProvenance - const expectedGeneration = expected?.generation - const actualGeneration = actual?.generation - return ( - isSameDocumentOwner(candidate, closedFile) && - !closedFile.externalSshTargetId && - !candidate.externalSshTargetId && - actual?.ownershipProjection === expected?.ownershipProjection && - actual?.expectedSshConnectionGeneration === expected?.expectedSshConnectionGeneration && - actualGeneration?.runtimeConnectionGeneration === - expectedGeneration?.runtimeConnectionGeneration && - actualGeneration?.runtimePairingRevision === expectedGeneration?.runtimePairingRevision && - actualGeneration?.runtimeSshGeneration === expectedGeneration?.runtimeSshGeneration && - actualGeneration?.nestedSshGeneration === expectedGeneration?.nestedSshGeneration && - actualGeneration?.directSshGeneration === expectedGeneration?.directSshGeneration - ) -} - -function findAdjacentOpenFileId( - files: readonly OpenFile[], - closedFileIds: ReadonlySet, - preferredFileId: string | null -): string | null { - const preferredIndex = files.findIndex((file) => file.id === preferredFileId) - const nextFile = files - .slice(Math.max(preferredIndex + 1, 0)) - .find((file) => !closedFileIds.has(file.id)) - if (nextFile) { - return nextFile.id - } - return ( - files - .slice(0, Math.max(preferredIndex, 0)) - .toReversed() - .find((file) => !closedFileIds.has(file.id))?.id ?? null - ) -} export function createCloseFileAction( set: EditorSet, @@ -73,75 +21,101 @@ export function createCloseFileAction( ): Pick { return { closeFile: (fileId) => { + // One snapshot keeps bulk-close decisions consistent without repeated store reads. + const preCloseState = get() // Why: capture untitled+dirty state before set() mutates the store, so cleanup of throwaway untitled files can decide after removal. - const preClose = get().openFiles.find((f) => f.id === fileId) - // Why: stale same-owner records survive tab close; retain other owners and pending drafts. - const fileIdsToClose = new Set( + const preClose = preCloseState.openFiles.find((f) => f.id === fileId) + + // Why: also check editorDrafts — isDirty is set by a debounced callback, so a draft can exist before isDirty flushes; a draft means the user typed something. + const hasUnsavedWork = (file: OpenFile): boolean => + file.isDirty === true || preCloseState.editorDrafts[file.id] !== undefined + const documentFileIds = preClose + ? collectSameDocumentOpenFileIds(preCloseState.openFiles, preClose, preCloseState) + : new Set() + const documentFiles = preCloseState.openFiles.filter( + (file) => file.worktreeId === preClose?.worktreeId && documentFileIds.has(file.id) + ) + // Only the named tab was explicitly closed; pinned siblings survive the sweep. + const pinnedSiblingIds = new Set( preClose - ? get() - .openFiles.filter( - (file) => - file.id === fileId || - (preClose.mode === 'edit' && - file.mode === 'edit' && - preClose.readOnly !== true && - file.readOnly !== true && - isSameOwnedDocument(file, preClose) && - !file.isDirty && - !(file.id in get().editorDrafts)) - ) - .map((file) => file.id) + ? (preCloseState.unifiedTabsByWorktree?.[preClose.worktreeId] ?? []) + .filter((tab) => tab.isPinned === true && isEditorTabContentType(tab.contentType)) + .map((tab) => tab.entityId) + : [] + ) + // Save/discard confirmation covers only the named record, never a sibling's draft. + const keptSiblingIds = new Set( + documentFiles + .filter( + (file) => file.id !== fileId && (hasUnsavedWork(file) || pinnedSiblingIds.has(file.id)) + ) + .map((f) => f.id) + ) + const siblingIds = new Set( + documentFiles.length > 0 + ? documentFiles.filter((file) => !keptSiblingIds.has(file.id)).map((file) => file.id) : [fileId] ) - const unifiedTabIdsToClose = Object.values(get().unifiedTabsByWorktree ?? {}).flatMap( - (tabs) => - tabs - .filter( - (entry) => - fileIdsToClose.has(entry.entityId) && - (entry.contentType === 'editor' || - entry.contentType === 'diff' || - entry.contentType === 'conflict-review' || - entry.contentType === 'check-details') - ) - .map((entry) => entry.id) + const closedTabOrderIds = new Set([ + ...siblingIds, + ...(preCloseState.unifiedTabsByWorktree?.[preClose?.worktreeId ?? ''] ?? []) + .filter((tab) => siblingIds.has(tab.entityId) && isEditorTabContentType(tab.contentType)) + .map((tab) => tab.id) + ]) + const sweptUnsavedWork = documentFiles.some( + (file) => siblingIds.has(file.id) && hasUnsavedWork(file) ) - const closedTabOrderIds = new Set([...fileIdsToClose, ...unifiedTabIdsToClose]) - const preCloseFiles = get().openFiles.filter((file) => fileIdsToClose.has(file.id)) - // Why: also check editorDrafts — isDirty is set by a debounced callback, so a draft can exist before isDirty flushes; a draft means the user typed something. - const hasDraft = fileId in get().editorDrafts + // Why the kept-sibling guard: a surviving duplicate still points at the untitled placeholder on disk. const shouldDeleteFromDisk = preClose !== undefined && - shouldDeleteUntouchedUntitledFile(preClose, hasDraft) && - !get().openFiles.some( - (file) => !fileIdsToClose.has(file.id) && mayShareEditorBackingFile(file, preClose) + keptSiblingIds.size === 0 && + shouldDeleteUntouchedUntitledFile(preClose, sweptUnsavedWork) && + !preCloseState.openFiles.some( + (file) => + (file.worktreeId !== preClose.worktreeId || !siblingIds.has(file.id)) && + mayShareEditorBackingFile(file, preClose) ) // Why: mirrored tabs are host-owned, so the host must close its copy or its next snapshot re-mirrors the file and the tab reopens. - for (const file of preCloseFiles) { - notifyHostOfMirroredEditorClose(get(), file.worktreeId, file.id) + // Why per sibling: the notifier resolves the mirror from the id it is given, so a mirrored duplicate swept under another id is never reported. + notifyHostOfMirroredEditorClose(preCloseState, preClose?.worktreeId, fileId) + for (const siblingId of siblingIds) { + if (siblingId !== fileId) { + notifyHostOfMirroredEditorClose(preCloseState, preClose?.worktreeId, siblingId) + } } set((s) => { - const closedFile = s.openFiles.find((f) => f.id === fileId) - const newFiles = s.openFiles.filter((f) => !fileIdsToClose.has(f.id)) + const closedFile = s.openFiles.find( + (file) => file.id === fileId && file.worktreeId === preClose?.worktreeId + ) + const closingWorktreeId = closedFile?.worktreeId ?? s.activeWorktreeId + const newFiles = s.openFiles.filter( + (file) => file.worktreeId !== closingWorktreeId || !siblingIds.has(file.id) + ) + const survivingIds = new Set(newFiles.map((file) => file.id)) const newEditorDrafts = { ...s.editorDrafts } const newMarkdownViewMode = { ...s.markdownViewMode } const newMarkdownRichModeSizeOverride = { ...s.markdownRichModeSizeOverride } const newEditorViewMode = { ...s.editorViewMode } + // Why: editorCursorLine is keyed by fileId and grows unbounded across a long session without cleanup on close. const newEditorCursorLine = { ...s.editorCursorLine } - for (const id of fileIdsToClose) { - delete newEditorDrafts[id] - delete newMarkdownViewMode[id] - delete newMarkdownRichModeSizeOverride[id] - delete newEditorViewMode[id] - // Why: editorCursorLine is keyed by fileId and grows unbounded across a long session without cleanup on close. - delete newEditorCursorLine[id] - } - const markdownVisibilityKeys = new Set(fileIdsToClose) - for (const file of preCloseFiles) { - if (file.markdownPreviewSourceFileId) { - markdownVisibilityKeys.add(file.markdownPreviewSourceFileId) + const markdownVisibilityKeys = new Set() + for (const siblingId of siblingIds) { + if (survivingIds.has(siblingId)) { + continue + } + delete newEditorDrafts[siblingId] + delete newMarkdownViewMode[siblingId] + delete newMarkdownRichModeSizeOverride[siblingId] + delete newEditorViewMode[siblingId] + delete newEditorCursorLine[siblingId] + markdownVisibilityKeys.add(siblingId) + const sourceFileId = s.openFiles.find( + (f) => f.id === siblingId + )?.markdownPreviewSourceFileId + if (sourceFileId) { + markdownVisibilityKeys.add(sourceFileId) } } const visibilityKeysToRemove = [...markdownVisibilityKeys].filter( @@ -158,28 +132,47 @@ export function createCloseFileAction( : s.markdownTableOfContentsVisible let newActiveId = s.activeFileId const newActiveFileIdByWorktree = { ...s.activeFileIdByWorktree } + const activeWasClosed = + s.activeWorktreeId === closingWorktreeId && + s.activeFileId !== null && + siblingIds.has(s.activeFileId) - if (s.activeFileId && fileIdsToClose.has(s.activeFileId)) { + if (activeWasClosed) { const worktreeId = closedFile?.worktreeId ?? s.activeWorktreeId - const worktreeFiles = worktreeId - ? s.openFiles.filter((file) => file.worktreeId === worktreeId) + const preSweepFiles = worktreeId + ? s.openFiles.filter((f) => f.worktreeId === worktreeId) : s.openFiles - newActiveId = findAdjacentOpenFileId(worktreeFiles, fileIdsToClose, s.activeFileId) + const worktreeFiles = worktreeId + ? newFiles.filter((f) => f.worktreeId === worktreeId) + : [] + if (worktreeFiles.length === 0) { + newActiveId = null + } else { + const closedPreSweepIdx = preSweepFiles.findIndex((f) => f.id === fileId) + const closedWorktreeIdx = + closedPreSweepIdx === -1 + ? 0 + : preSweepFiles.slice(0, closedPreSweepIdx).filter((f) => !siblingIds.has(f.id)) + .length + newActiveId = + worktreeFiles[Math.min(Math.max(closedWorktreeIdx, 0), worktreeFiles.length - 1)].id + } + if (worktreeId) { + newActiveFileIdByWorktree[worktreeId] = newActiveId + } } - - const closedWorktreeId = closedFile?.worktreeId - if (closedWorktreeId) { - const worktreeFiles = s.openFiles.filter((file) => file.worktreeId === closedWorktreeId) - const worktreeActiveFileId = s.activeFileIdByWorktree[closedWorktreeId] - if (worktreeActiveFileId && fileIdsToClose.has(worktreeActiveFileId)) { - newActiveFileIdByWorktree[closedWorktreeId] = findAdjacentOpenFileId( - worktreeFiles, - fileIdsToClose, - worktreeActiveFileId - ) - } else if (s.activeFileId && fileIdsToClose.has(s.activeFileId)) { - newActiveFileIdByWorktree[closedWorktreeId] = newActiveId + // Why: a swept sibling must not stay named as some workspace's active file. + const reselectedWorktreeId = activeWasClosed ? closedFile?.worktreeId : undefined + for (const [wId, activeId] of Object.entries(newActiveFileIdByWorktree)) { + if ( + wId !== closingWorktreeId || + wId === reselectedWorktreeId || + !activeId || + !siblingIds.has(activeId) + ) { + continue } + newActiveFileIdByWorktree[wId] = newFiles.find((f) => f.worktreeId === wId)?.id ?? null } // Why: editors share a mixed tab strip with browser tabs; closing the last editor should reveal a browser tab before falling back to a terminal. @@ -213,10 +206,7 @@ export function createCloseFileAction( // Structured chats have no legacy terminal row to keep their workspace selected. const hasRemainingUnifiedTabs = activeWorktreeId !== null && - unifiedTabsKeepWorktreeSelected( - s.unifiedTabsByWorktree?.[activeWorktreeId], - fileIdsToClose - ) + unifiedTabsKeepWorktreeSelected(s.unifiedTabsByWorktree?.[activeWorktreeId], siblingIds) const shouldDeactivateWorktree = activeWorktreeId !== null && remainingForWorktree.length === 0 && @@ -258,7 +248,7 @@ export function createCloseFileAction( ...s.recentlyClosedEditorTabsByWorktree, [wtRecent]: [ { - ...(snap as ClosedEditorTabSnapshot), + ...snap, reopenId: fileId, ...(position ? { position } : {}) }, @@ -295,7 +285,7 @@ export function createCloseFileAction( tabBarOrderByWorktree: nextTabBarOrderByWorktree, pendingEditorReveal: null, pendingEditorFocusRequest: - s.pendingEditorFocusRequest && fileIdsToClose.has(s.pendingEditorFocusRequest.fileId) + s.pendingEditorFocusRequest && siblingIds.has(s.pendingEditorFocusRequest.fileId) ? null : s.pendingEditorFocusRequest, recentlyClosedEditorTabsByWorktree: nextRecentlyClosed, @@ -309,8 +299,21 @@ export function createCloseFileAction( } // Why: route editor/diff closes through the unified close path (MRU + visual-neighbor fallback) so they match terminal/browser tab-close behavior. - for (const unifiedTabId of unifiedTabIdsToClose) { - get().closeUnifiedTab(unifiedTabId) + // Why collected first: each close rewrites the tab maps this scan would otherwise be reading. + // Other workspaces may reuse the same file ID and must retain their tabs. + const tabsByWorktree = get().unifiedTabsByWorktree ?? {} + const scannedTabLists = preClose + ? [tabsByWorktree[preClose.worktreeId] ?? []] + : Object.values(tabsByWorktree) + const closableTabIds = scannedTabLists.flatMap((tabs) => + tabs + .filter( + (entry) => siblingIds.has(entry.entityId) && isEditorTabContentType(entry.contentType) + ) + .map((entry) => entry.id) + ) + for (const tabId of closableTabIds) { + get().closeUnifiedTab(tabId) } } } diff --git a/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts b/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts index 7508ca8cfb25..ee12fb3de584 100644 --- a/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts +++ b/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts @@ -16,6 +16,10 @@ import { LegacyHydratedEditorFileIndex, shouldHydrateWithOwnedEditorFileId } from '../file-ids/hydrated-editor-file-ids' +import { + collectHydratedOrphanEditorFileIds, + isOrphanEditorFile +} from '../file-ids/orphan-editor-file-ids' export function createHydrateEditorSession( set: EditorSet, @@ -138,21 +142,49 @@ export function createHydrateEditorSession( // Why: transient diff/conflict surfaces aren't restored, so clear a stale "editor" marker and fall back to terminal. const nextActiveTabType = nextActiveFileId || activeTabType !== 'editor' ? activeTabType : 'terminal' + const migratedTabsAndGroups = migrateHydratedEditorTabsAndGroups( + s, + editorFileIdMigrationsByWorktree + ) + // `?? {}` because an editor-only store (tests, partial slices) has no tab map at all. + const nextTabsByWorktree = + migratedTabsAndGroups.unifiedTabsByWorktree ?? s.unifiedTabsByWorktree ?? {} + const orphanFileIdsByWorktree = collectHydratedOrphanEditorFileIds( + openFiles, + nextTabsByWorktree, + filteredActiveFileIdByWorktree, + editorDrafts + ) + const survivingFiles = + orphanFileIdsByWorktree.size > 0 + ? openFiles.filter((file) => !isOrphanEditorFile(orphanFileIdsByWorktree, file)) + : openFiles + // Why by surviving id, not by orphan id: drafts and front-matter keys are keyed by id alone, + // and an id orphaned in one worktree can still name a live document in another. + const survivingFileIds = new Set(survivingFiles.map((file) => file.id)) + const survivingIds = new Set( + [...usedOpenFileIds].filter((fileId) => survivingFileIds.has(fileId)) + ) const markdownFrontmatterVisible = resolveHydratedEditorFrontmatter( persistedMarkdownFrontmatterVisible, - usedOpenFileIds, + survivingIds, editorFileIdMigrationsByWorktree ) return { - openFiles, - editorDrafts, + openFiles: survivingFiles, + editorDrafts: + survivingFiles.length === openFiles.length + ? editorDrafts + : Object.fromEntries( + Object.entries(editorDrafts).filter(([fileId]) => survivingFileIds.has(fileId)) + ), markdownFrontmatterVisible, activeFileId: nextActiveFileId, activeFileIdByWorktree: filteredActiveFileIdByWorktree, activeTabType: nextActiveTabType, activeTabTypeByWorktree: filteredActiveTabTypeByWorktree, - ...migrateHydratedEditorTabsAndGroups(s, editorFileIdMigrationsByWorktree) + ...migratedTabsAndGroups } }) } diff --git a/src/renderer/src/store/slices/editor/actions/open-file-mutations.ts b/src/renderer/src/store/slices/editor/actions/open-file-mutations.ts index fb9bded1db00..79152fb9ce6b 100644 --- a/src/renderer/src/store/slices/editor/actions/open-file-mutations.ts +++ b/src/renderer/src/store/slices/editor/actions/open-file-mutations.ts @@ -21,6 +21,15 @@ export function createOpenFileMutations( > { return { setActiveFile: (fileId, targetWorktreeId) => { + if ( + !get().openFiles.some( + (file) => + file.id === fileId && (!targetWorktreeId || file.worktreeId === targetWorktreeId) + ) + ) { + console.warn(`[editor] ignoring selection without an open document: ${fileId}`) + return + } set((s) => { const file = s.openFiles.find((f) => f.id === fileId) const worktreeId = file?.worktreeId diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.test.ts b/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.test.ts new file mode 100644 index 000000000000..0188ce2a608d --- /dev/null +++ b/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest' +import { buildPersistedEditorFileRecords } from '@/lib/workspace-session-editor-records' +import type { OpenFile } from '../types/open-file' +import { editorDocumentIdentityKey } from './editor-document-identity' +import { collectSameDocumentOpenFileIds } from './editor-file-ids' + +const WORKTREE_ID = 'repo-1::/workspace' +const FILE_PATH = '/workspace/app.ts' + +function openFile(id: string, overrides: Partial = {}): OpenFile { + const base: OpenFile = { + id, + filePath: FILE_PATH, + relativePath: 'app.ts', + worktreeId: WORKTREE_ID, + language: 'typescript', + mode: 'edit', + isDirty: false, + isPreview: false, + runtimeEnvironmentId: null + } + return { ...base, ...overrides } +} + +const documentVariants: { + name: string + left: Partial + right: Partial + sameDocument: boolean +}[] = [ + { name: 'two records with identical fields', left: {}, right: {}, sameDocument: true }, + { + name: 'a blank external ssh target beside an absent one', + left: {}, + right: { externalSshTargetId: ' ' }, + sameDocument: true + }, + { + name: 'a blank runtime owner beside a null one', + left: {}, + right: { runtimeEnvironmentId: ' ' }, + sameDocument: true + }, + { + name: 'a null owner beside a runtime owner', + left: {}, + right: { runtimeEnvironmentId: 'env-a' }, + sameDocument: false + }, + { + name: 'a writable record beside a read-only one', + left: {}, + right: { readOnly: true }, + sameDocument: false + }, + { + name: 'a read-only log beside its live-tail twin', + left: { readOnly: true }, + right: { readOnly: true, liveTail: true }, + sameDocument: false + }, + { + name: 'a writable record beside one flagged live-tail', + left: {}, + right: { liveTail: true }, + sameDocument: true + }, + { + name: 'an ssh-pinned record beside a worktree-local one', + left: {}, + right: { externalSshTargetId: 'ssh-target' }, + sameDocument: false + }, + { + name: 'two different paths', + left: {}, + right: { filePath: '/workspace/other.ts' }, + sameDocument: false + }, + { + name: 'two different worktrees', + left: {}, + right: { worktreeId: 'repo-1::/other' }, + sameDocument: false + } +] + +describe('editor document identity', () => { + for (const variant of documentVariants) { + it(`treats ${variant.name} the same way in the key, the session write and the close sweep`, () => { + const left = openFile('left', variant.left) + const right = openFile('right', variant.right) + + const sameKey = editorDocumentIdentityKey(left) === editorDocumentIdentityKey(right) + const { openFilesByWorktree } = buildPersistedEditorFileRecords([left, right], {}, {}) + const sweptIds = collectSameDocumentOpenFileIds([left, right], left) + + expect(sameKey).toBe(variant.sameDocument) + expect(Object.values(openFilesByWorktree).flat()).toHaveLength(variant.sameDocument ? 1 : 2) + expect(sweptIds.has(right.id)).toBe(variant.sameDocument) + }) + } + + it('groups a record under a substituted owner exactly as under its own', () => { + const file = openFile('left') + + expect(editorDocumentIdentityKey(file, 'env-a')).toBe( + editorDocumentIdentityKey(openFile('right', { runtimeEnvironmentId: 'env-a' })) + ) + }) +}) diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.ts b/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.ts new file mode 100644 index 000000000000..3db6204efdcb --- /dev/null +++ b/src/renderer/src/store/slices/editor/file-ids/editor-document-identity.ts @@ -0,0 +1,44 @@ +/** The execution owner half of a document's identity: an unset or blank runtime means local. */ +export function runtimeOwnerKey(runtimeEnvironmentId: string | null | undefined): string | null { + return runtimeEnvironmentId?.trim() || null +} + +/** + * The fields that decide whether two editor records are the same document. Declared structurally + * so a live `OpenFile` and a `PersistedOpenFile` — whose `runtimeEnvironmentId` optionality + * differs — both satisfy it without a cast. + */ +export type EditorDocumentIdentityFields = { + worktreeId: string + filePath: string + runtimeEnvironmentId?: string | null + externalSshTargetId?: string | null + readOnly?: boolean + liveTail?: boolean +} + +/** + * Identity of one edit document: one path, one owner. Persisted records carry no id, so two + * records sharing this key serialize to indistinguishable rows that restore as one document — + * the write merges them, a close sweeps all of them, and the restore heal collapses them. + * + * `owner` defaults to the record's own runtime owner; the restore heal passes the route owner it + * is normalizing onto so pre- and post-heal owners group together. + */ +export function editorDocumentIdentityKey( + file: EditorDocumentIdentityFields, + owner: string | null = runtimeOwnerKey(file.runtimeEnvironmentId) +): string { + return JSON.stringify([ + file.worktreeId, + owner, + file.externalSshTargetId?.trim() || null, + file.filePath, + // Why: a read-only log tab and a writable tab on one path are different documents — merging + // them would restore the log writable, carrying a hot-exit draft it must never have. + file.readOnly === true, + // Why gated on readOnly: the writer persists liveTail only for a read-only row, and an + // identity finer than the row it serializes to leaves a duplicate restore cannot tell apart. + file.readOnly === true && file.liveTail === true + ]) +} diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts index 39bfa7a5f7fd..52dbe61b10de 100644 --- a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts @@ -3,13 +3,10 @@ import type { EditorSlice } from '../types/editor-slice' import type { DiffSource, EditorOpenTargetOptions, OpenFile } from '../types/open-file' import { toSshExecutionHostId } from '../../../../../../shared/execution-host' import { areLocalWindowsWslPathAliases } from '../../../../../../shared/cross-platform-path' +import { editorDocumentIdentityKey, runtimeOwnerKey } from './editor-document-identity' import { getConnectionIdForFileFromState } from '@/lib/connection-owner-resolution' import { isLocalWindowsDesktopClient } from '@/lib/desktop-window-chrome' -export function runtimeOwnerKey(runtimeEnvironmentId: string | null | undefined): string | null { - return runtimeEnvironmentId?.trim() || null -} - export function isSameEditorOwner( file: Pick, worktreeId: string, @@ -126,6 +123,43 @@ export function getReusableOpenFileModes(mode: OpenFile['mode']): readonly OpenF return [mode] } +/** + * Every OpenFile that is the same edit document: one path, one owner. More than one record for + * that identity is corruption, but until it is gone a close has to sweep all of them or the + * survivors restore the tab on the next session write. + * + * Edit mode only: a path's diff variants all share `mode: 'diff'` and, for combined diffs, the + * worktree path, so their identity lives in `diffSource` — and only edit records persist, so only + * they can be duplicated by the session writer. + */ +export function collectSameDocumentOpenFileIds( + openFiles: readonly OpenFile[], + file: Pick< + OpenFile, + | 'id' + | 'filePath' + | 'worktreeId' + | 'runtimeEnvironmentId' + | 'externalSshTargetId' + | 'readOnly' + | 'liveTail' + | 'mode' + > +): Set { + const fileIds = new Set([file.id]) + if (file.mode !== 'edit') { + return fileIds + } + const identity = editorDocumentIdentityKey(file) + const modes = getReusableOpenFileModes(file.mode) + for (const candidate of openFiles) { + if (matchesEditorMode(candidate, modes) && editorDocumentIdentityKey(candidate) === identity) { + fileIds.add(candidate.id) + } + } + return fileIds +} + export function resolveEditorFileIdForOwner( state: Pick, filePath: string, diff --git a/src/renderer/src/store/slices/editor/file-ids/hydrated-editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/hydrated-editor-file-ids.ts index 8bb47c3bfd8a..ff5b9fbbe8e6 100644 --- a/src/renderer/src/store/slices/editor/file-ids/hydrated-editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/hydrated-editor-file-ids.ts @@ -4,7 +4,8 @@ import type { PersistedOpenFile } from '../../../../../../shared/workspace-sessi import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../../../shared/constants' import type { OpenFile } from '../types/open-file' import { isEditorTabContentType } from '../tabs/editor-tab-content-type' -import { buildOwnedEditorFileId, runtimeOwnerKey } from './editor-file-ids' +import { buildOwnedEditorFileId } from './editor-file-ids' +import { runtimeOwnerKey } from './editor-document-identity' export function shouldHydrateWithOwnedEditorFileId( worktreeId: string, diff --git a/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.test.ts b/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.test.ts new file mode 100644 index 000000000000..7d15bf73a8ea --- /dev/null +++ b/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from 'vitest' +import type { Tab } from '../../../../../../shared/tab-types' +import { collectHydratedOrphanEditorFileIds } from './orphan-editor-file-ids' + +const WORKTREE_ID = 'repo-1::/workspace' +const OTHER_WORKTREE_ID = 'repo-2::/other-workspace' +const TABBED_FILE_ID = '/workspace/tabbed.ts' +const ORPHAN_FILE_ID = '/workspace/orphan.ts' + +function editorTab(entityId: string, worktreeId = WORKTREE_ID): Tab { + return { + id: `tab:${entityId}`, + entityId, + groupId: 'group-1', + worktreeId, + contentType: 'editor', + label: entityId, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +function orphansFor( + orphanFileIdsByWorktree: Map>, + worktreeId = WORKTREE_ID +): string[] { + return [...(orphanFileIdsByWorktree.get(worktreeId) ?? [])] +} + +function collectOrphans( + orphan: { isDirty?: boolean }, + editorDrafts: Record +): string[] { + return orphansFor( + collectHydratedOrphanEditorFileIds( + [ + { id: TABBED_FILE_ID, worktreeId: WORKTREE_ID, isDirty: false }, + { id: ORPHAN_FILE_ID, worktreeId: WORKTREE_ID, isDirty: orphan.isDirty ?? false } + ], + { [WORKTREE_ID]: [editorTab(TABBED_FILE_ID)] }, + { [WORKTREE_ID]: TABBED_FILE_ID }, + editorDrafts + ) + ) +} + +describe('collectHydratedOrphanEditorFileIds', () => { + it('reports a clean document no editor tab references', () => { + expect(collectOrphans({}, {})).toEqual([ORPHAN_FILE_ID]) + }) + + it('keeps an orphan whose unsaved draft has not flushed into isDirty yet', () => { + expect(collectOrphans({}, { [ORPHAN_FILE_ID]: 'typed but not flushed' })).toEqual([]) + }) + + it('keeps an orphan that already flushed its dirty flag', () => { + expect(collectOrphans({ isDirty: true }, {})).toEqual([]) + }) + + it('skips a worktree missing from the hydrated tab map', () => { + expect( + collectHydratedOrphanEditorFileIds( + [{ id: ORPHAN_FILE_ID, worktreeId: WORKTREE_ID, isDirty: false }], + {}, + {}, + {} + ).size + ).toBe(0) + }) + + it('prunes a worktree whose hydrated tab list is present but empty', () => { + expect( + orphansFor( + collectHydratedOrphanEditorFileIds( + [{ id: ORPHAN_FILE_ID, worktreeId: WORKTREE_ID, isDirty: false }], + { [WORKTREE_ID]: [] }, + {}, + {} + ) + ) + ).toEqual([ORPHAN_FILE_ID]) + }) + + it('reports an orphan only under the worktree it is orphaned in', () => { + // Why the shared id: an unowned editor id is the bare file path, so the same id names a live + // document in the other worktree. + const orphanFileIdsByWorktree = collectHydratedOrphanEditorFileIds( + [ + { id: ORPHAN_FILE_ID, worktreeId: WORKTREE_ID, isDirty: false }, + { id: ORPHAN_FILE_ID, worktreeId: OTHER_WORKTREE_ID, isDirty: false } + ], + { + [WORKTREE_ID]: [], + [OTHER_WORKTREE_ID]: [editorTab(ORPHAN_FILE_ID, OTHER_WORKTREE_ID)] + }, + {}, + {} + ) + + expect(orphansFor(orphanFileIdsByWorktree)).toEqual([ORPHAN_FILE_ID]) + expect(orphansFor(orphanFileIdsByWorktree, OTHER_WORKTREE_ID)).toEqual([]) + }) +}) diff --git a/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.ts new file mode 100644 index 000000000000..da1d495c48ba --- /dev/null +++ b/src/renderer/src/store/slices/editor/file-ids/orphan-editor-file-ids.ts @@ -0,0 +1,72 @@ +import type { Tab } from '../../../../../../shared/tab-types' +import type { OpenFile } from '../types/open-file' +import { isEditorTabContentType } from '../tabs/editor-tab-content-type' + +/** + * Open documents with no editor-family tab left. They render nowhere, but the active-surface + * fallback still selects one, which is how a closed file comes back on the next restart. + */ +export function collectOrphanEditorFileIds( + editorFileIds: ReadonlySet, + tabs: readonly Tab[], + activeFileId: string | null | undefined +): string[] { + if (editorFileIds.size === 0) { + return [] + } + const tabbedEntityIds = new Set( + tabs.filter((tab) => isEditorTabContentType(tab.contentType)).map((tab) => tab.entityId) + ) + return [...editorFileIds].filter( + (fileId) => !tabbedEntityIds.has(fileId) && fileId !== activeFileId + ) +} + +/** + * Orphans per worktree. Why not one flat set: an unowned editor id is the bare file path, so the + * same id can name a live document in another worktree — a flat set sweeps that one too. + */ +export function collectHydratedOrphanEditorFileIds( + openFiles: readonly Pick[], + tabsByWorktree: Record, + activeFileIdByWorktree: Record, + editorDrafts: Record +): Map> { + const fileIdsByWorktree = new Map>() + for (const file of openFiles) { + // Why: an unsaved buffer must survive the sweep; only a clean document is disposable chrome. + // Why the draft check: a draft is unsaved work whether or not the caller has flushed isDirty, + // so the collector must never depend on that flag alone. + if (file.isDirty === true || editorDrafts[file.id] !== undefined) { + continue + } + const fileIds = fileIdsByWorktree.get(file.worktreeId) + if (fileIds) { + fileIds.add(file.id) + continue + } + fileIdsByWorktree.set(file.worktreeId, new Set([file.id])) + } + const orphanFileIdsByWorktree = new Map>() + for (const [worktreeId, fileIds] of fileIdsByWorktree) { + const tabs = tabsByWorktree[worktreeId] + // Why: missing = unknown, skip; empty = known, prune — a hydrated but empty tab list is + // positive evidence that no editor tab renders those documents. + if (!tabs) { + continue + } + const orphans = collectOrphanEditorFileIds(fileIds, tabs, activeFileIdByWorktree[worktreeId]) + if (orphans.length > 0) { + orphanFileIdsByWorktree.set(worktreeId, new Set(orphans)) + } + } + return orphanFileIdsByWorktree +} + +/** Membership test for a worktree-scoped orphan map, so callers never compare ids alone. */ +export function isOrphanEditorFile( + orphanFileIdsByWorktree: ReadonlyMap>, + file: Pick +): boolean { + return orphanFileIdsByWorktree.get(file.worktreeId)?.has(file.id) === true +} diff --git a/src/renderer/src/store/slices/tabs/tabs-reconciliation-orphan-editor-files.test.ts b/src/renderer/src/store/slices/tabs/tabs-reconciliation-orphan-editor-files.test.ts new file mode 100644 index 000000000000..b6fc8e488e5d --- /dev/null +++ b/src/renderer/src/store/slices/tabs/tabs-reconciliation-orphan-editor-files.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it, vi } from 'vitest' +import type * as AgentStatusModule from '@/lib/agent-status' +import type { Tab } from '../../../../../shared/tab-types' +import type { OpenFile } from '../editor' +import { createTabsSliceMockApi } from '../tabs-slice-test-harness' +import { createTestStore } from '../store-test-helpers' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/lib/agent-status', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) } +}) + +createTabsSliceMockApi() + +const WORKTREE_ID = 'repo-1::/workspace' +const GROUP_ID = 'group-1' +const OTHER_WORKTREE_ID = 'repo-2::/other-workspace' +const OTHER_GROUP_ID = 'group-2' +const TABBED_FILE_ID = '/workspace/tabbed.ts' +const ORPHAN_FILE_ID = '/workspace/orphan.ts' + +function openFile(id: string, overrides: Partial = {}): OpenFile { + const base: OpenFile = { + id, + filePath: id, + relativePath: id.slice('/workspace/'.length), + worktreeId: WORKTREE_ID, + language: 'typescript', + mode: 'edit', + isDirty: false, + isPreview: false + } + return { ...base, ...overrides } +} + +function editorTab(entityId: string): Tab { + return { + id: `tab:${entityId}`, + entityId, + groupId: GROUP_ID, + worktreeId: WORKTREE_ID, + contentType: 'editor', + label: entityId, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +function prepareStore(openFiles: OpenFile[], activeFileId: string | null) { + const store = createTestStore() + store.setState({ + openFiles, + activeFileIdByWorktree: { [WORKTREE_ID]: activeFileId }, + unifiedTabsByWorktree: { [WORKTREE_ID]: [editorTab(TABBED_FILE_ID)] }, + groupsByWorktree: { + [WORKTREE_ID]: [ + { + id: GROUP_ID, + worktreeId: WORKTREE_ID, + activeTabId: `tab:${TABBED_FILE_ID}`, + tabOrder: [`tab:${TABBED_FILE_ID}`], + recentTabIds: [`tab:${TABBED_FILE_ID}`] + } + ] + }, + activeGroupIdByWorktree: { [WORKTREE_ID]: GROUP_ID }, + layoutByWorktree: { [WORKTREE_ID]: { type: 'leaf' as const, groupId: GROUP_ID } } + }) + return store +} + +describe('hydrated reconciliation orphan editor sweep', () => { + it('drops an open document no tab references', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([TABBED_FILE_ID]) + }) + + it('keeps the workspace active document even when it lost its tab', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], ORPHAN_FILE_ID) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + TABBED_FILE_ID, + ORPHAN_FILE_ID + ]) + }) + + it('keeps an orphan that still holds an unsaved buffer', () => { + const store = prepareStore( + [openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID, { isDirty: true })], + TABBED_FILE_ID + ) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + TABBED_FILE_ID, + ORPHAN_FILE_ID + ]) + }) + + it('keeps an orphan whose unsaved draft has not flushed into isDirty yet', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + store.setState({ editorDrafts: { [ORPHAN_FILE_ID]: 'typed but not flushed' } }) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([ + TABBED_FILE_ID, + ORPHAN_FILE_ID + ]) + }) + + it('drops a swept document from the tab strip order', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + store.setState({ + tabBarOrderByWorktree: { [WORKTREE_ID]: [TABBED_FILE_ID, ORPHAN_FILE_ID] } + }) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().tabBarOrderByWorktree[WORKTREE_ID]).toEqual([TABBED_FILE_ID]) + }) + + it('leaves openFiles identical when nothing is orphaned', () => { + const store = prepareStore([openFile(TABBED_FILE_ID)], TABBED_FILE_ID) + const before = store.getState().openFiles + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles).toBe(before) + }) + + it('never sweeps a workspace whose tab model was never hydrated', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + store.setState({ unifiedTabsByWorktree: {} }) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles).toHaveLength(2) + }) + + it('leaves the same id alone in a workspace that still renders it', () => { + // Why the shared id: an unowned editor id is the bare file path, so one id names a live + // document in the other workspace while it is orphaned here. + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + store.setState({ + openFiles: [ + openFile(TABBED_FILE_ID), + openFile(ORPHAN_FILE_ID), + openFile(ORPHAN_FILE_ID, { worktreeId: OTHER_WORKTREE_ID }) + ], + unifiedTabsByWorktree: { + [WORKTREE_ID]: [editorTab(TABBED_FILE_ID)], + [OTHER_WORKTREE_ID]: [ + { ...editorTab(ORPHAN_FILE_ID), worktreeId: OTHER_WORKTREE_ID, groupId: OTHER_GROUP_ID } + ] + }, + groupsByWorktree: { + ...store.getState().groupsByWorktree, + [OTHER_WORKTREE_ID]: [ + { + id: OTHER_GROUP_ID, + worktreeId: OTHER_WORKTREE_ID, + activeTabId: `tab:${ORPHAN_FILE_ID}`, + tabOrder: [`tab:${ORPHAN_FILE_ID}`], + recentTabIds: [`tab:${ORPHAN_FILE_ID}`] + } + ] + }, + activeGroupIdByWorktree: { + ...store.getState().activeGroupIdByWorktree, + [OTHER_WORKTREE_ID]: OTHER_GROUP_ID + }, + layoutByWorktree: { + ...store.getState().layoutByWorktree, + [OTHER_WORKTREE_ID]: { type: 'leaf' as const, groupId: OTHER_GROUP_ID } + }, + tabBarOrderByWorktree: { + [WORKTREE_ID]: [TABBED_FILE_ID, ORPHAN_FILE_ID], + [OTHER_WORKTREE_ID]: [ORPHAN_FILE_ID] + } + }) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID, OTHER_WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => [file.worktreeId, file.id])).toEqual([ + [WORKTREE_ID, TABBED_FILE_ID], + [OTHER_WORKTREE_ID, ORPHAN_FILE_ID] + ]) + expect(store.getState().tabBarOrderByWorktree[OTHER_WORKTREE_ID]).toEqual([ORPHAN_FILE_ID]) + }) + + it('sweeps a workspace whose hydrated tab list is present but empty', () => { + const store = prepareStore([openFile(TABBED_FILE_ID), openFile(ORPHAN_FILE_ID)], TABBED_FILE_ID) + // Why empty rather than absent: the tab model is known and renders no editor, so both documents + // are unreachable — only the workspace's active file is held back by the selection guard. + store.setState({ unifiedTabsByWorktree: { [WORKTREE_ID]: [] } }) + + store.getState().reconcileWorktreeTabModels([WORKTREE_ID]) + + expect(store.getState().openFiles.map((file) => file.id)).toEqual([TABBED_FILE_ID]) + }) +}) diff --git a/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts b/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts index d5a005f537e0..371f6d12e55d 100644 --- a/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts +++ b/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts @@ -12,11 +12,17 @@ import { writeBatchedWorkspaceRecordEntry, type WorktreeTabModelReconciliationBatch } from './tabs-reconciliation-batch' +import { collectOrphanEditorFileIds } from '../editor/file-ids/orphan-editor-file-ids' export type WorktreeTabModelReconciliation = { patch: Partial renderableTabCount: number activeRenderableTabId: string | null + /** + * Open documents this workspace has no tab for. Reported rather than pruned here: the batch + * fold indexes `openFiles` once and stays valid only while reconciliation never writes it. + */ + orphanEditorFileIds: readonly string[] } /** @@ -259,6 +265,15 @@ export function projectWorktreeTabModelReconciliation( return { patch, renderableTabCount: validTabs.length, + // Why the guard: a workspace whose tab model was never hydrated proves nothing about orphans. + orphanEditorFileIds: + worktreeId in state.unifiedTabsByWorktree + ? collectOrphanEditorFileIds( + liveEditorIds, + validTabs, + state.activeFileIdByWorktree[worktreeId] + ) + : [], activeRenderableTabId: nextGroups.find((group) => group.id === nextActiveGroupId)?.activeTabId ?? nextGroups.find((group) => group.activeTabId !== null)?.activeTabId ?? diff --git a/src/renderer/src/store/slices/tabs/tabs-session-actions.ts b/src/renderer/src/store/slices/tabs/tabs-session-actions.ts index e33f1f533cdc..145823cdc910 100644 --- a/src/renderer/src/store/slices/tabs/tabs-session-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-session-actions.ts @@ -36,17 +36,76 @@ function projectWorktreeTabModelReconciliations( // Private working copy so batch-owned maps can be written in place. const working = { ...state } const merged: Partial = {} + // Why per worktree: an unowned editor id is the bare file path, so the same id can name a live + // document in another workspace — a flat set of orphans would sweep that one too. + const orphanEditorFileIdsByWorktree = new Map>() for (const worktreeId of worktreeIds) { - const { patch } = projectWorktreeTabModelReconciliation(working, worktreeId, batch) - if (Object.keys(patch).length === 0) { + const reconciliation = projectWorktreeTabModelReconciliation(working, worktreeId, batch) + if (reconciliation.orphanEditorFileIds.length > 0) { + orphanEditorFileIdsByWorktree.set(worktreeId, new Set(reconciliation.orphanEditorFileIds)) + } + if (Object.keys(reconciliation.patch).length === 0) { continue } - Object.assign(merged, patch) - Object.assign(working, patch) + Object.assign(merged, reconciliation.patch) + Object.assign(working, reconciliation.patch) + } + // Why only here: `openFiles` is written once the whole fold is projected, so the batch's + // one-shot editor index stays valid — and an unsaved buffer is never swept. + if (orphanEditorFileIdsByWorktree.size > 0) { + // Why the draft check: isDirty is set by a debounced callback, so a just-typed buffer can hold + // a draft before the flag flushes — sweeping it would discard the user's text. + const sweptFileIdsByWorktree = new Map>() + for (const file of state.openFiles) { + if ( + file.isDirty === true || + state.editorDrafts[file.id] !== undefined || + orphanEditorFileIdsByWorktree.get(file.worktreeId)?.has(file.id) !== true + ) { + continue + } + const swept = sweptFileIdsByWorktree.get(file.worktreeId) + if (swept) { + swept.add(file.id) + continue + } + sweptFileIdsByWorktree.set(file.worktreeId, new Set([file.id])) + } + if (sweptFileIdsByWorktree.size > 0) { + merged.openFiles = state.openFiles.filter( + (file) => sweptFileIdsByWorktree.get(file.worktreeId)?.has(file.id) !== true + ) + const tabBarOrder = pruneTabBarOrderEntries( + working.tabBarOrderByWorktree ?? state.tabBarOrderByWorktree, + sweptFileIdsByWorktree + ) + if (tabBarOrder) { + merged.tabBarOrderByWorktree = tabBarOrder + } + } } return merged } +/** Why: a swept id left in the strip order still shifts positions on the next reconcile. */ +function pruneTabBarOrderEntries( + tabBarOrderByWorktree: AppState['tabBarOrderByWorktree'], + sweptFileIdsByWorktree: ReadonlyMap> +): AppState['tabBarOrderByWorktree'] | null { + if (!tabBarOrderByWorktree) { + return null + } + let changed = false + const next: AppState['tabBarOrderByWorktree'] = {} + for (const [worktreeId, order] of Object.entries(tabBarOrderByWorktree)) { + const sweptFileIds = sweptFileIdsByWorktree.get(worktreeId) + const pruned = sweptFileIds ? order.filter((entryId) => !sweptFileIds.has(entryId)) : order + changed = changed || pruned.length !== order.length + next[worktreeId] = pruned.length === order.length ? order : pruned + } + return changed ? next : null +} + export function createTabsSessionActions( set: TabsSliceSet, get: TabsSliceGet @@ -65,6 +124,9 @@ export function createTabsSessionActions( } }, + // Why no orphan prune here: this runs from setActiveWorktree and empty-group checks, which can + // observe an in-flight open (the OpenFile is published before its tab), so pruning would delete + // a live document — only the batch fold at the hydration boundary sees a settled tab model. reconcileWorktreeTabModel: (worktreeId) => { const reconciliation = projectWorktreeTabModelReconciliation(get(), worktreeId) if (Object.keys(reconciliation.patch).length > 0) { From 0e4ebc6216de980e5e897534f9679befafa97a65 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 15:00:34 -0700 Subject: [PATCH 02/10] Respect captured execution owners in duplicate editor close sweeps --- .../closed-editor-model-lifetime.test.ts | 20 ++++++++++++---- .../editor-model-host-isolation.test.ts | 1 + .../monaco-cursor-line-lifetime.test.ts | 3 +-- .../slices/editor/file-ids/editor-file-ids.ts | 24 +++++++++---------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/renderer/src/components/editor/closed-editor-model-lifetime.test.ts b/src/renderer/src/components/editor/closed-editor-model-lifetime.test.ts index c1d65f922744..778f2dff68c6 100644 --- a/src/renderer/src/components/editor/closed-editor-model-lifetime.test.ts +++ b/src/renderer/src/components/editor/closed-editor-model-lifetime.test.ts @@ -35,16 +35,28 @@ it('keeps a live model and releases only its closed sibling', async () => { expect(live.model.isDisposed()).toBe(false) }) -it('preserves a shared URI until its final file owner closes', async () => { +it('releases a shared URI when all clean duplicate records close', async () => { const { store, attach, add } = createModelLifetimeFixture() const { file, model } = add('shared-a') - store.setState({ openFiles: [file, { ...file, id: 'shared-b', readOnly: true }] }) + store.setState({ openFiles: [file, { ...file, id: 'shared-b' }] }) + attach() + store.getState().closeFile(file.id) + await Promise.resolve() + expect(store.getState().openFiles).toHaveLength(0) + expect(model.isDisposed()).toBe(true) +}) + +it('keeps a dirty shared-model duplicate alive until its own close', async () => { + const { store, attach, add } = createModelLifetimeFixture() + const { file, model } = add('shared-dirty-a') + const sibling = { ...file, id: 'shared-dirty-b', isDirty: true } + store.setState({ openFiles: [file, sibling], editorDrafts: { [sibling.id]: 'unsaved text' } }) attach() store.getState().closeFile(file.id) await Promise.resolve() - expect(store.getState().openFiles.map((openFile) => openFile.id)).toEqual(['shared-b']) expect(model.isDisposed()).toBe(false) - store.getState().closeFile('shared-b') + expect(store.getState().editorDrafts[sibling.id]).toBe('unsaved text') + store.getState().closeFile(sibling.id) await Promise.resolve() expect(model.isDisposed()).toBe(true) }) 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 index 6fa6f5d829eb..d79fe6712cff 100644 --- a/src/renderer/src/components/editor/editor-model-host-isolation.test.ts +++ b/src/renderer/src/components/editor/editor-model-host-isolation.test.ts @@ -258,6 +258,7 @@ describe('same-path models on different execution hosts', () => { } const detach = attachModelLifetimeView(local.model) edit(runtime.model, 'runtime independent edit') + store.setState({ editorDrafts: { [runtime.file.id]: 'runtime independent edit' } }) attach() store.setState({ openFiles: store diff --git a/src/renderer/src/components/editor/monaco-cursor-line-lifetime.test.ts b/src/renderer/src/components/editor/monaco-cursor-line-lifetime.test.ts index c4ec59c1311a..ada59a920c23 100644 --- a/src/renderer/src/components/editor/monaco-cursor-line-lifetime.test.ts +++ b/src/renderer/src/components/editor/monaco-cursor-line-lifetime.test.ts @@ -41,7 +41,6 @@ function startTracking( describe('Monaco cursor-line ownership lifetime', () => { it('releases positions after 250 distinct owned editor tabs close', () => { const store = createEditorStore() - store.setState({ unifiedTabsByWorktree: {} }) for (let index = 0; index < 250; index += 1) { const file = makeFile(`editor:wt-1:local:file-${index}`, `/repo/file-${index}.ts`) store.setState({ openFiles: [file] }) @@ -62,7 +61,7 @@ describe('Monaco cursor-line ownership lifetime', () => { ...makeFile('editor:wt-1:remote-a:file.ts', '/repo/file.ts'), runtimeEnvironmentId: 'remote-a' } - store.setState({ openFiles: [local, remote], unifiedTabsByWorktree: {} }) + store.setState({ openFiles: [local, remote] }) const localTracking = startTracking(store, local, 12) const remoteTracking = startTracking(store, remote, 35) diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts index 52dbe61b10de..ad85e68b9858 100644 --- a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts @@ -5,6 +5,8 @@ import { toSshExecutionHostId } from '../../../../../../shared/execution-host' import { areLocalWindowsWslPathAliases } from '../../../../../../shared/cross-platform-path' import { editorDocumentIdentityKey, runtimeOwnerKey } from './editor-document-identity' import { getConnectionIdForFileFromState } from '@/lib/connection-owner-resolution' +import { getEditorModelOwnerKey } from '@/components/editor/editor-model-owner' +import type { WorktreeOperationRouteState } from '@/lib/worktree-operation-route' import { isLocalWindowsDesktopClient } from '@/lib/desktop-window-chrome' export function isSameEditorOwner( @@ -134,26 +136,22 @@ export function getReusableOpenFileModes(mode: OpenFile['mode']): readonly OpenF */ export function collectSameDocumentOpenFileIds( openFiles: readonly OpenFile[], - file: Pick< - OpenFile, - | 'id' - | 'filePath' - | 'worktreeId' - | 'runtimeEnvironmentId' - | 'externalSshTargetId' - | 'readOnly' - | 'liveTail' - | 'mode' - > + file: OpenFile, + state?: WorktreeOperationRouteState ): Set { const fileIds = new Set([file.id]) if (file.mode !== 'edit') { return fileIds } - const identity = editorDocumentIdentityKey(file) + const closeIdentity = (candidate: OpenFile): string => + JSON.stringify([ + editorDocumentIdentityKey(candidate), + state ? getEditorModelOwnerKey(candidate, state) : null + ]) + const identity = closeIdentity(file) const modes = getReusableOpenFileModes(file.mode) for (const candidate of openFiles) { - if (matchesEditorMode(candidate, modes) && editorDocumentIdentityKey(candidate) === identity) { + if (matchesEditorMode(candidate, modes) && closeIdentity(candidate) === identity) { fileIds.add(candidate.id) } } From 041fae7a77705ca9bbbbb0f834e1e6e12d524387 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 15:39:44 -0700 Subject: [PATCH 03/10] Preserve captured editor owners through session serialization --- .../editor-model-host-isolation.test.ts | 54 +++++++++++++++++++ .../src/lib/editor-file-operation-owner.ts | 19 +++++++ .../lib/workspace-session-editor-records.ts | 6 +-- .../slices/editor/file-ids/editor-file-ids.ts | 3 +- 4 files changed, 78 insertions(+), 4 deletions(-) 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 index d79fe6712cff..08e944bdb2da 100644 --- a/src/renderer/src/components/editor/editor-model-host-isolation.test.ts +++ b/src/renderer/src/components/editor/editor-model-host-isolation.test.ts @@ -11,6 +11,7 @@ import { resetModelLifetimeFixtures } from './editor-model-lifetime-fixture' import { getEditorModelOwnerKey } from './editor-model-owner' +import { buildPersistedEditorFileRecords } from '@/lib/workspace-session-editor-records' import { toEditorModelUri } from './editor-model-uri' import { beginProgrammaticContentSync, @@ -78,6 +79,59 @@ afterEach(() => { }) describe('same-path models on different execution hosts', () => { + it('uses captured ownership over stale fields for persistence and clean duplicate close', () => { + const { owners, store, attach } = createHostModels() + const remote = owners[3]! + const stale = { + ...remote.file, + id: 'stale-fields', + runtimeEnvironmentId: 'wrong-hub', + externalSshTargetId: 'wrong-target' + } + store.setState({ openFiles: [remote.file, stale] }) + const records = Object.values( + buildPersistedEditorFileRecords([remote.file, stale], {}, {}).openFilesByWorktree + ).flat() + expect(records).toHaveLength(1) + expect(records[0]).toMatchObject({ + runtimeEnvironmentId: 'hub-a', + externalSshTargetId: 'target' + }) + attach() + store.getState().closeFile(remote.file.id) + expect(store.getState().openFiles).toHaveLength(0) + }) + + it.each(['clean', 'same-draft', 'different-drafts'] as const)( + 'persists all captured owners of same-path %s documents', + (kind) => { + const { owners } = createHostModels() + const files = owners.map((owner) => ({ ...owner.file, isDirty: kind !== 'clean' })) + const drafts = Object.fromEntries( + files.map((file, index) => [ + file.id, + kind === 'same-draft' ? 'identical unsaved text' : `draft ${index}` + ]) + ) + const records = Object.values( + buildPersistedEditorFileRecords(files, drafts, {}).openFilesByWorktree + ).flat() + expect(records).toHaveLength(HOSTS.length) + expect( + records.map((record) => [record.runtimeEnvironmentId, record.externalSshTargetId ?? null]) + ).toEqual([ + [null, null], + [null, 'target'], + ['hub-a', null], + ['hub-a', 'target'], + ['hub-b', 'target'] + ]) + expect(records.map((record) => record.dirtyDraftContent)).toEqual( + files.map((file) => (kind === 'clean' ? undefined : drafts[file.id])) + ) + } + ) + 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) => { diff --git a/src/renderer/src/lib/editor-file-operation-owner.ts b/src/renderer/src/lib/editor-file-operation-owner.ts index a1805976d51a..f578e00e64b0 100644 --- a/src/renderer/src/lib/editor-file-operation-owner.ts +++ b/src/renderer/src/lib/editor-file-operation-owner.ts @@ -1,3 +1,4 @@ +import type { OpenFile } from '@/store/slices/editor' import { parseExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { parseWorkspaceKey } from '../../../shared/workspace-scope' @@ -21,6 +22,24 @@ export type EditorFileOperationProvenance = { expectedSshConnectionGeneration?: number } +export function getPersistedEditorOwnerFields( + file: Pick +): Pick { + const captured = file.operationProvenance?.generation.route + const host = parseExecutionHostId(captured?.executionHostId) + if (!captured || !host) { + return { + runtimeEnvironmentId: file.runtimeEnvironmentId, + externalSshTargetId: file.externalSshTargetId + } + } + // A restart must retain the owner that opened the document, even after catalog replacement. + return { + runtimeEnvironmentId: captured.runtimeEnvironmentId, + externalSshTargetId: host.kind === 'ssh' ? host.targetId : undefined + } +} + type EditorOwnerState = Pick< AppState, | 'settings' diff --git a/src/renderer/src/lib/workspace-session-editor-records.ts b/src/renderer/src/lib/workspace-session-editor-records.ts index 5451751f54dc..ea1e9c8257bc 100644 --- a/src/renderer/src/lib/workspace-session-editor-records.ts +++ b/src/renderer/src/lib/workspace-session-editor-records.ts @@ -1,3 +1,4 @@ +import { getPersistedEditorOwnerFields } from './editor-file-operation-owner' import type { PersistedOpenFile } from '../../../shared/workspace-session-state-types' import type { OpenFile } from '../store/slices/editor' import { editorDocumentIdentityKey } from '../store/slices/editor/file-ids/editor-document-identity' @@ -23,8 +24,7 @@ function toPersistedOpenFile( worktreeId: file.worktreeId, language: file.language, isPreview: file.isPreview || undefined, - runtimeEnvironmentId: file.runtimeEnvironmentId, - externalSshTargetId: file.externalSshTargetId, + ...getPersistedEditorOwnerFields(file), // Why: persist readOnly only when true; absence is the writable default on restore. ...(file.readOnly === true ? { readOnly: true } : {}), ...(file.readOnly === true && file.liveTail === true ? { liveTail: true } : {}), @@ -120,7 +120,7 @@ export function buildPersistedEditorFileRecords( const fileIds = editFileIdsByWorktree[file.worktreeId] ?? (editFileIdsByWorktree[file.worktreeId] = new Set()) const record = toPersistedOpenFile(file, editorDrafts) - const identity = editorDocumentIdentityKey(file) + const identity = editorDocumentIdentityKey(record) const variants = keptVariantsByIdentity.get(identity) ?? [] if (variants.length === 0) { keptVariantsByIdentity.set(identity, variants) diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts index ad85e68b9858..1105acb9095a 100644 --- a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts @@ -5,6 +5,7 @@ import { toSshExecutionHostId } from '../../../../../../shared/execution-host' import { areLocalWindowsWslPathAliases } from '../../../../../../shared/cross-platform-path' import { editorDocumentIdentityKey, runtimeOwnerKey } from './editor-document-identity' import { getConnectionIdForFileFromState } from '@/lib/connection-owner-resolution' +import { getPersistedEditorOwnerFields } from '@/lib/editor-file-operation-owner' import { getEditorModelOwnerKey } from '@/components/editor/editor-model-owner' import type { WorktreeOperationRouteState } from '@/lib/worktree-operation-route' import { isLocalWindowsDesktopClient } from '@/lib/desktop-window-chrome' @@ -145,7 +146,7 @@ export function collectSameDocumentOpenFileIds( } const closeIdentity = (candidate: OpenFile): string => JSON.stringify([ - editorDocumentIdentityKey(candidate), + editorDocumentIdentityKey({ ...candidate, ...getPersistedEditorOwnerFields(candidate) }), state ? getEditorModelOwnerKey(candidate, state) : null ]) const identity = closeIdentity(file) From 37494ab0413200286fe545ccdafdcbaf4de65687 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 15:50:49 -0700 Subject: [PATCH 04/10] test: route new orchestration SQLite fixtures through Node --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9098a6f566db..cec9f43adefd 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -1,5 +1,7 @@ // Database opening and lifecycle fixtures exercise Node SQLite behavior. export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ + 'src/main/runtime/orchestration/orchestration-party-location.test.ts', + 'src/main/runtime/runtime-orchestration-sender-names.test.ts', 'src/main/ai-vault-search/session-search-file-write.test.ts', 'src/main/ai-vault-search/session-search-live-transcript.test.ts', 'src/main/ai-vault/session-scanner-devin-contention.test.ts', From aeaf432c00d380ee4476705ea484e128ca16f3a9 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 16:23:21 -0700 Subject: [PATCH 05/10] Keep restored nested SSH editor reads and writes on their recorded owners --- .../editor/ExternalFileChangeBanner.tsx | 1 + .../ExternalFileChangeCompareDialog.tsx | 1 + .../editor-restored-tab-conflict-scan.ts | 1 + .../editor/rich-markdown-image-context.ts | 3 +- .../use-markdown-preview-source-foundation.ts | 4 +- .../editor/useEditorPanelFileContentLoader.ts | 1 + ...editor-external-watch-disk-verification.ts | 11 +- ...-external-watch-owner-verification.test.ts | 69 +++++++ .../lib/editor-file-operation-owner.test.ts | 189 +++++++++++------- .../src/lib/editor-file-operation-owner.ts | 4 +- .../src/runtime/mobile-markdown-bridge.ts | 1 + .../src/runtime/runtime-file-client-types.ts | 2 + .../src/runtime/runtime-file-client.test.ts | 40 ++++ .../src/runtime/runtime-file-range-client.ts | 7 +- .../src/runtime/runtime-file-read-client.ts | 18 +- .../src/runtime/runtime-file-routing.ts | 8 +- 16 files changed, 275 insertions(+), 85 deletions(-) create mode 100644 src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts diff --git a/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx b/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx index 71a11eaa7757..28946d17a182 100644 --- a/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx +++ b/src/renderer/src/components/editor/ExternalFileChangeBanner.tsx @@ -94,6 +94,7 @@ export function keepTabEditsOverExternalChange(file: OpenFile): void { worktreeId: file.worktreeId, connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined, expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId, access: editorTabFileAccess(state, file) }) .then((result) => { diff --git a/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx index 26cd9fbe95f7..29dd1bcbb31e 100644 --- a/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx +++ b/src/renderer/src/components/editor/ExternalFileChangeCompareDialog.tsx @@ -64,6 +64,7 @@ export function ExternalFileChangeCompareDialog({ worktreeId: file.worktreeId, connectionId: getConnectionIdForFile(file.worktreeId, file.filePath) ?? undefined, expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId, access }) .then((result) => { diff --git a/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts index 72d44520981e..79ac96415cda 100644 --- a/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts +++ b/src/renderer/src/components/editor/editor-restored-tab-conflict-scan.ts @@ -71,6 +71,7 @@ export function attachRestoredTabConflictScan(store: AppStoreApi): () => void { worktreeId: file.worktreeId, connectionId: getFileConnectionId(file), expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId, access: editorTabFileAccess(state, file) }) if (disposed) { diff --git a/src/renderer/src/components/editor/rich-markdown-image-context.ts b/src/renderer/src/components/editor/rich-markdown-image-context.ts index b01d2236b453..56e575080251 100644 --- a/src/renderer/src/components/editor/rich-markdown-image-context.ts +++ b/src/renderer/src/components/editor/rich-markdown-image-context.ts @@ -76,7 +76,8 @@ export function createRichMarkdownImageResolverContext({ worktreeId, worktreePath: worktreeRoot, connectionId: getConnectionId(worktreeId), - expectedExternalSshTargetId: externalSshTargetId + expectedExternalSshTargetId: externalSshTargetId, + expectedRuntimeEnvironmentId: runtimeEnvironmentId } : undefined } diff --git a/src/renderer/src/components/editor/use-markdown-preview-source-foundation.ts b/src/renderer/src/components/editor/use-markdown-preview-source-foundation.ts index 433bd6c4a31e..02f85fb90d6f 100644 --- a/src/renderer/src/components/editor/use-markdown-preview-source-foundation.ts +++ b/src/renderer/src/components/editor/use-markdown-preview-source-foundation.ts @@ -133,13 +133,15 @@ export function useMarkdownPreviewSourceFoundation({ worktreeId: sourceRoutingWorktreeId, worktreePath: worktreeRoot, connectionId: sourceConnectionId, - expectedExternalSshTargetId: sourceOpenFile?.externalSshTargetId + expectedExternalSshTargetId: sourceOpenFile?.externalSshTargetId, + expectedRuntimeEnvironmentId: sourceOpenFile?.runtimeEnvironmentId } : undefined, [ settings, sourceConnectionId, sourceOpenFile?.externalSshTargetId, + sourceOpenFile?.runtimeEnvironmentId, resolvedSourceRuntimeEnvironmentId, sourceRoutingWorktreeId, worktreeRoot diff --git a/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts b/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts index 10011d251a85..64324d50056a 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts +++ b/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts @@ -211,6 +211,7 @@ export function useEditorPanelFileContentLoader({ worktreeId: readWorktreeId, connectionId: readConnectionId, expectedExternalSshTargetId: restoredOpenFile?.externalSshTargetId, + expectedRuntimeEnvironmentId: restoredOpenFile?.runtimeEnvironmentId, includeLocalLogMetadata: isLiveTailLogTab, access }, diff --git a/src/renderer/src/hooks/editor-external-watch-disk-verification.ts b/src/renderer/src/hooks/editor-external-watch-disk-verification.ts index 96caaf8c84a1..6e6653686c4b 100644 --- a/src/renderer/src/hooks/editor-external-watch-disk-verification.ts +++ b/src/renderer/src/hooks/editor-external-watch-disk-verification.ts @@ -55,11 +55,13 @@ function readFileForEchoVerification(args: { worktreeId: string | null | undefined connectionId: string | undefined expectedExternalSshTargetId?: string + expectedRuntimeEnvironmentId?: string | null }): ReturnType { const key = [ args.runtimeEnvironmentId ?? '', args.connectionId ?? '', args.expectedExternalSshTargetId ?? '', + args.expectedRuntimeEnvironmentId ?? '', args.filePath ].join('::') let pending = inFlightEchoVerificationReads.get(key) @@ -72,7 +74,8 @@ function readFileForEchoVerification(args: { relativePath: args.relativePath, worktreeId: args.worktreeId ?? undefined, connectionId: args.connectionId, - expectedExternalSshTargetId: args.expectedExternalSshTargetId + expectedExternalSshTargetId: args.expectedExternalSshTargetId, + expectedRuntimeEnvironmentId: args.expectedRuntimeEnvironmentId }) inFlightEchoVerificationReads.set(key, pending) const release = (): void => { @@ -226,7 +229,8 @@ export function scheduleEditorSelfMoveEchoVerification( relativePath: file.relativePath, worktreeId: file.worktreeId, connectionId: target.connectionId, - expectedExternalSshTargetId: file.externalSshTargetId + expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId }) .then((result) => { const diskSignature = result.isBinary ? null : getDiskBaselineSignature(result.content) @@ -261,7 +265,8 @@ export function scheduleSelfWriteAwareEditorExternalReload( relativePath: file.relativePath, worktreeId: file.worktreeId, connectionId: target.connectionId, - expectedExternalSshTargetId: file.externalSshTargetId + expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId }) .then((result) => { if ( diff --git a/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts b/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts new file mode 100644 index 000000000000..3aff34fa7d9e --- /dev/null +++ b/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts @@ -0,0 +1,69 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import type { OpenFile } from '@/store/slices/editor' +import { readRuntimeFileContent } from '@/runtime/runtime-file-client' +import { scheduleSelfWriteAwareEditorExternalReload } from './editor-external-watch-disk-verification' + +vi.mock('@/runtime/runtime-file-client', () => ({ readRuntimeFileContent: vi.fn() })) + +const target = { + worktreeId: 'repo::/repo', + worktreePath: '/repo', + connectionId: 'ssh-1', + runtimeEnvironmentId: 'hub-a' +} +const notification = { ...target, relativePath: 'file.txt' } + +function file(runtimeEnvironmentId: string | null): OpenFile { + return { + id: '/repo/file.txt', + filePath: '/repo/file.txt', + relativePath: 'file.txt', + worktreeId: target.worktreeId, + language: 'plaintext', + isDirty: false, + mode: 'edit', + runtimeEnvironmentId, + externalSshTargetId: 'ssh-1' + } +} + +beforeEach(() => { + vi.mocked(readRuntimeFileContent).mockReset() + useAppStore.setState({ openFiles: [] }) +}) + +describe('external editor verification ownership', () => { + it.each([false, true])( + 'shares reads only for the same recorded owner (%s)', + async (different) => { + const releases: (() => void)[] = [] + vi.mocked(readRuntimeFileContent).mockImplementation( + () => + new Promise((resolve) => { + releases.push(() => resolve({ content: 'saved', isBinary: false })) + }) + ) + try { + for (const runtime of ['hub-a', different ? null : 'hub-a']) { + scheduleSelfWriteAwareEditorExternalReload(target, notification, file(runtime), { + content: 'saved' + }) + } + expect(readRuntimeFileContent).toHaveBeenCalledTimes(different ? 2 : 1) + if (different) { + expect(readRuntimeFileContent).toHaveBeenLastCalledWith( + expect.objectContaining({ + settings: { activeRuntimeEnvironmentId: 'hub-a' }, + expectedExternalSshTargetId: 'ssh-1', + expectedRuntimeEnvironmentId: null + }) + ) + } + } finally { + releases.forEach((release) => release()) + await Promise.resolve() + } + } + ) +}) diff --git a/src/renderer/src/lib/editor-file-operation-owner.test.ts b/src/renderer/src/lib/editor-file-operation-owner.test.ts index 43e257419db4..e04cee5c2862 100644 --- a/src/renderer/src/lib/editor-file-operation-owner.test.ts +++ b/src/renderer/src/lib/editor-file-operation-owner.test.ts @@ -3,7 +3,8 @@ import { useAppStore } from '@/store' import { assertEditorFileOperationCurrent, captureEditorFileOperationProvenance, - getEditorFileOperationContext + getEditorFileOperationContext, + getPersistedEditorOwnerFields } from './editor-file-operation-owner' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' @@ -122,78 +123,124 @@ describe('editor file operation owner', () => { ).toThrow('Reopen the file') }) - it('captures and rejects replacement nested SSH connection generations', () => { - useAppStore.setState({ - worktreesByRepo: { - repo: [ - { - id: worktreeId, - repoId: 'repo', - path: '/remote/repo', - hostId: 'ssh:private', - runtimeOwnerEnvironmentId: 'hub-a' - } as never - ] - }, - sshStateByEnvironment: new Map([ - [ - 'hub-a', - { - targetsHydrated: true, - targetGenerations: new Map(), - targetLabels: new Map([['private', 'private']]), - removedTargetLabels: new Map(), - connectionStates: new Map([ - [ - 'private', - { - targetId: 'private', - status: 'connected', - error: null, - reconnectAttempt: 0, - connectionGeneration: 7 - } - ] - ]) - } - ] - ]) - }) - const provenance = captureEditorFileOperationProvenance( - useAppStore.getState(), - worktreeId, - undefined, - false - ) - expect(provenance.expectedSshConnectionGeneration).toBe(7) + it.each([false, true])( + 'keeps nested SSH operations owner-bound before and after serialization (%s)', + (persisted) => { + useAppStore.setState({ + worktreesByRepo: { + repo: [ + { + id: worktreeId, + repoId: 'repo', + path: '/remote/repo', + hostId: 'ssh:private', + runtimeOwnerEnvironmentId: 'hub-a' + } as never + ] + }, + sshStateByEnvironment: new Map([ + [ + 'hub-a', + { + targetsHydrated: true, + targetGenerations: new Map(), + targetLabels: new Map([['private', 'private']]), + removedTargetLabels: new Map(), + connectionStates: new Map([ + [ + 'private', + { + targetId: 'private', + status: 'connected', + error: null, + reconnectAttempt: 0, + connectionGeneration: 7 + } + ] + ]) + } + ] + ]) + }) + const provenance = captureEditorFileOperationProvenance( + useAppStore.getState(), + worktreeId, + undefined, + false + ) + expect(provenance.expectedSshConnectionGeneration).toBe(7) - useAppStore.setState((state) => ({ - sshStateByEnvironment: new Map([ - [ - 'hub-a', - { - ...state.sshStateByEnvironment.get('hub-a')!, - connectionStates: new Map([ - [ - 'private', - { - targetId: 'private', - status: 'connected', - error: null, - reconnectAttempt: 0, - connectionGeneration: 8 - } - ] - ]) - } - ] - ]) - })) + const file = { worktreeId, runtimeEnvironmentId: 'hub-a', operationProvenance: provenance } + const ownerFields = persisted ? getPersistedEditorOwnerFields(file) : {} + const context = getEditorFileOperationContext( + useAppStore.getState(), + { ...file, ...ownerFields }, + '/remote/repo' + ) + expect(context).toMatchObject({ + expectedExecutionHostId: 'ssh:private', + expectedSshTargetId: 'private', + expectedSshConnectionGeneration: 7 + }) + expect(context.settings?.activeRuntimeEnvironmentId).toBe('hub-a') + expect(context.connectionId).toBeUndefined() + if (persisted) { + expect( + getEditorFileOperationContext( + useAppStore.getState(), + { worktreeId, ...ownerFields }, + '/remote/repo' + ) + ).toMatchObject({ + settings: expect.objectContaining({ activeRuntimeEnvironmentId: 'hub-a' }), + expectedExecutionHostId: 'ssh:private', + expectedSshTargetId: 'private', + expectedSshConnectionGeneration: 7 + }) + } + expect(() => + getEditorFileOperationContext( + useAppStore.getState(), + { ...file, externalSshTargetId: 'private', runtimeEnvironmentId: null }, + '/remote/repo' + ) + ).toThrow('Reopen the file') + expect(() => + getEditorFileOperationContext( + useAppStore.getState(), + { ...file, externalSshTargetId: 'another-target' }, + '/remote/repo' + ) + ).toThrow('Reopen the file') - expect(() => - assertEditorFileOperationCurrent(useAppStore.getState(), worktreeId, provenance) - ).toThrow('Reopen the file') - }) + useAppStore.setState((state) => ({ + sshStateByEnvironment: new Map([ + [ + 'hub-a', + { + ...state.sshStateByEnvironment.get('hub-a')!, + connectionStates: new Map([ + [ + 'private', + { + targetId: 'private', + status: 'connected', + error: null, + reconnectAttempt: 0, + connectionGeneration: 8 + } + ] + ]) + } + ] + ]) + })) + + expect(() => + assertEditorFileOperationCurrent(useAppStore.getState(), worktreeId, provenance) + ).toThrow('Reopen the file') + } + ) it('rejects a restored external SSH file after its target changes', () => { useAppStore.setState({ diff --git a/src/renderer/src/lib/editor-file-operation-owner.ts b/src/renderer/src/lib/editor-file-operation-owner.ts index f578e00e64b0..c01591c11cc7 100644 --- a/src/renderer/src/lib/editor-file-operation-owner.ts +++ b/src/renderer/src/lib/editor-file-operation-owner.ts @@ -193,6 +193,7 @@ export function getEditorFileOperationContext( expectedSshTargetId?: string expectedSshConnectionGeneration?: number expectedExecutionHostId: 'local' | `ssh:${string}` + expectedRuntimeEnvironmentId: string | null } { const provenance = file.operationProvenance ?? @@ -221,7 +222,7 @@ export function getEditorFileOperationContext( if ( externalSshTargetId && (host.kind !== 'ssh' || - route.runtimeEnvironmentId !== null || + (file.runtimeEnvironmentId?.trim() || null) !== route.runtimeEnvironmentId || host.targetId !== externalSshTargetId) ) { throw new Error(OWNER_CHANGED_MESSAGE) @@ -235,6 +236,7 @@ export function getEditorFileOperationContext( worktreeId: file.worktreeId, worktreePath: resolvedWorktreePath, expectedExecutionHostId: host.kind === 'ssh' ? host.id : 'local', + expectedRuntimeEnvironmentId: route.runtimeEnvironmentId, ...(route.runtimeEnvironmentId === null && host?.kind === 'ssh' ? { connectionId: host.targetId } : {}), diff --git a/src/renderer/src/runtime/mobile-markdown-bridge.ts b/src/renderer/src/runtime/mobile-markdown-bridge.ts index ef26415f9b10..bc9c8e50fc80 100644 --- a/src/renderer/src/runtime/mobile-markdown-bridge.ts +++ b/src/renderer/src/runtime/mobile-markdown-bridge.ts @@ -251,6 +251,7 @@ async function readFileContent(file: OpenFile): Promise { worktreeId: file.worktreeId, connectionId, expectedExternalSshTargetId: file.externalSshTargetId, + expectedRuntimeEnvironmentId: file.runtimeEnvironmentId, access: editorTabFileAccess(state, file) }) if (result.isBinary) { diff --git a/src/renderer/src/runtime/runtime-file-client-types.ts b/src/renderer/src/runtime/runtime-file-client-types.ts index 87f84d9aa220..def46b4b1825 100644 --- a/src/renderer/src/runtime/runtime-file-client-types.ts +++ b/src/renderer/src/runtime/runtime-file-client-types.ts @@ -17,6 +17,7 @@ export type RuntimeFileReadArgs = { worktreeId?: string connectionId?: string expectedExternalSshTargetId?: string + expectedRuntimeEnvironmentId?: string | null includeLocalLogMetadata?: boolean /** File access of the local fallback read; remote reads stay root-relative. */ access?: LocalFileAccess @@ -31,6 +32,7 @@ export type RuntimeFileOperationArgs = { expectedSshTargetId?: string expectedSshConnectionGeneration?: number expectedExternalSshTargetId?: string + expectedRuntimeEnvironmentId?: string | null } export type RuntimeFileDownloadResult = diff --git a/src/renderer/src/runtime/runtime-file-client.test.ts b/src/renderer/src/runtime/runtime-file-client.test.ts index 7f2cd70ca5cd..b8491553a2b6 100644 --- a/src/renderer/src/runtime/runtime-file-client.test.ts +++ b/src/renderer/src/runtime/runtime-file-client.test.ts @@ -21,6 +21,46 @@ import { installRuntimeFileClientEnvironment() describe('runtime file client', () => { + it('reads a restored nested SSH file only through its recorded paired host', async () => { + runtimeEnvironmentCall.mockResolvedValue({ + ok: true, + result: { content: 'nested owner', isBinary: false } + }) + const args = { + settings: { activeRuntimeEnvironmentId: 'hub-a' }, + filePath: '/repo/file.txt', + relativePath: 'file.txt', + worktreeId: 'wt-1', + connectionId: 'ssh-1', + expectedExternalSshTargetId: 'ssh-1', + expectedRuntimeEnvironmentId: 'hub-a' + } + await expect(readRuntimeFileContent(args)).resolves.toMatchObject({ content: 'nested owner' }) + await expect( + readRuntimeFilePreview( + { + ...args, + worktreePath: '/repo', + connectionId: undefined, + expectedSshTargetId: 'ssh-1' + }, + args.filePath + ) + ).resolves.toMatchObject({ content: 'nested owner' }) + expect(fsReadFile).not.toHaveBeenCalled() + runtimeEnvironmentCall.mockClear() + for (const wrongOwner of [ + { settings: { activeRuntimeEnvironmentId: 'hub-b' } }, + { settings: { activeRuntimeEnvironmentId: null } }, + { connectionId: 'ssh-2' } + ]) { + await expect(readRuntimeFileContent({ ...args, ...wrongOwner })).rejects.toThrow( + 'workspace host changes' + ) + } + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + it('uses local filesystem reads when no remote runtime is active', async () => { const localResult: RuntimeReadableFileContent = { content: 'hello', isBinary: false } fsReadFile.mockResolvedValue(localResult) diff --git a/src/renderer/src/runtime/runtime-file-range-client.ts b/src/renderer/src/runtime/runtime-file-range-client.ts index bc04820d45a1..a4fafc02aca6 100644 --- a/src/renderer/src/runtime/runtime-file-range-client.ts +++ b/src/renderer/src/runtime/runtime-file-range-client.ts @@ -6,7 +6,12 @@ import { callRuntimeRpc, getActiveRuntimeTarget, RuntimeRpcCallError } from './r import { toRuntimeWorktreeSelector } from './runtime-worktree-selector' function rangeReadTarget(args: RuntimeFileReadArgs) { - assertExternalSshReadOwnership(args.settings, args.connectionId, args.expectedExternalSshTargetId) + assertExternalSshReadOwnership( + args.settings, + args.connectionId, + args.expectedExternalSshTargetId, + args.expectedRuntimeEnvironmentId + ) const target = getActiveRuntimeTarget(args.settings) if (target.kind !== 'environment') { return null diff --git a/src/renderer/src/runtime/runtime-file-read-client.ts b/src/renderer/src/runtime/runtime-file-read-client.ts index 3cd9943fa40e..baa84d1975a1 100644 --- a/src/renderer/src/runtime/runtime-file-read-client.ts +++ b/src/renderer/src/runtime/runtime-file-read-client.ts @@ -41,10 +41,16 @@ export async function readRuntimeFileContent({ worktreeId, connectionId, expectedExternalSshTargetId, + expectedRuntimeEnvironmentId, includeLocalLogMetadata, access }: RuntimeFileReadArgs): Promise { - assertExternalSshReadOwnership(settings, connectionId, expectedExternalSshTargetId) + assertExternalSshReadOwnership( + settings, + connectionId, + expectedExternalSshTargetId, + expectedRuntimeEnvironmentId + ) const target = getActiveRuntimeTarget(settings) if (target.kind !== 'environment' || !worktreeId) { return window.api.fs.readFile({ @@ -96,8 +102,9 @@ export async function readRuntimeFilePreview( ): Promise { assertExternalSshReadOwnership( context.settings, - context.connectionId, - context.expectedExternalSshTargetId + context.connectionId ?? context.expectedSshTargetId, + context.expectedExternalSshTargetId, + context.expectedRuntimeEnvironmentId ) const remoteArgs = getRemoteFileArgs(context, filePath) const runtimeTarget = getActiveRuntimeTarget(context.settings) @@ -140,8 +147,9 @@ export async function downloadRuntimeFile( ): Promise { assertExternalSshReadOwnership( context.settings, - context.connectionId, - context.expectedExternalSshTargetId + context.connectionId ?? context.expectedSshTargetId, + context.expectedExternalSshTargetId, + context.expectedRuntimeEnvironmentId ) const remoteArgs = getRemoteFileArgs(context, filePath) if (!remoteArgs) { diff --git a/src/renderer/src/runtime/runtime-file-routing.ts b/src/renderer/src/runtime/runtime-file-routing.ts index e08b7f493208..4fab479eb79f 100644 --- a/src/renderer/src/runtime/runtime-file-routing.ts +++ b/src/renderer/src/runtime/runtime-file-routing.ts @@ -11,12 +11,16 @@ import type { RuntimeFileOperationArgs } from './runtime-file-client-types' export function assertExternalSshReadOwnership( settings: Pick | null | undefined, connectionId: string | undefined, - expectedExternalSshTargetId: string | undefined + expectedExternalSshTargetId: string | undefined, + expectedRuntimeEnvironmentId?: string | null ): void { const expectedTargetId = expectedExternalSshTargetId?.trim() + const runtime = getActiveRuntimeTarget(settings) + const runtimeEnvironmentId = runtime.kind === 'environment' ? runtime.environmentId : null if ( expectedTargetId && - (getActiveRuntimeTarget(settings).kind === 'environment' || connectionId !== expectedTargetId) + (runtimeEnvironmentId !== (expectedRuntimeEnvironmentId?.trim() || null) || + connectionId !== expectedTargetId) ) { throw new Error('External SSH files are not available after the workspace host changes.') } From 84b271a3ba4500a36fd83fda01deff2c47e1872e Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 17:44:44 -0700 Subject: [PATCH 06/10] Use upstream SQLite runtime registrations without duplicates --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index cec9f43adefd..9098a6f566db 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -1,7 +1,5 @@ // Database opening and lifecycle fixtures exercise Node SQLite behavior. export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ - 'src/main/runtime/orchestration/orchestration-party-location.test.ts', - 'src/main/runtime/runtime-orchestration-sender-names.test.ts', 'src/main/ai-vault-search/session-search-file-write.test.ts', 'src/main/ai-vault-search/session-search-live-transcript.test.ts', 'src/main/ai-vault/session-scanner-devin-contention.test.ts', From f9d2fe21f0e7fe3693bac90d3d3d7ef04d384ee7 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 17:43:38 -0700 Subject: [PATCH 07/10] Scope editor verification reads to their workspace selectors --- ...editor-external-watch-disk-verification.ts | 14 +++++----- ...-external-watch-owner-verification.test.ts | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/hooks/editor-external-watch-disk-verification.ts b/src/renderer/src/hooks/editor-external-watch-disk-verification.ts index 6e6653686c4b..f00495285cff 100644 --- a/src/renderer/src/hooks/editor-external-watch-disk-verification.ts +++ b/src/renderer/src/hooks/editor-external-watch-disk-verification.ts @@ -57,13 +57,15 @@ function readFileForEchoVerification(args: { expectedExternalSshTargetId?: string expectedRuntimeEnvironmentId?: string | null }): ReturnType { - const key = [ - args.runtimeEnvironmentId ?? '', - args.connectionId ?? '', - args.expectedExternalSshTargetId ?? '', - args.expectedRuntimeEnvironmentId ?? '', + const key = JSON.stringify([ + args.runtimeEnvironmentId ?? null, + args.connectionId ?? null, + args.expectedExternalSshTargetId ?? null, + args.expectedRuntimeEnvironmentId ?? null, + args.worktreeId ?? null, + args.relativePath, args.filePath - ].join('::') + ]) let pending = inFlightEchoVerificationReads.get(key) if (!pending) { pending = readRuntimeFileContent({ diff --git a/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts b/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts index 3aff34fa7d9e..10cf4d5c8a0f 100644 --- a/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts +++ b/src/renderer/src/hooks/editor-external-watch-owner-verification.test.ts @@ -34,6 +34,33 @@ beforeEach(() => { }) describe('external editor verification ownership', () => { + it('keeps verification reads separate when workspace selectors differ', async () => { + const releases: (() => void)[] = [] + vi.mocked(readRuntimeFileContent).mockImplementation( + () => + new Promise((resolve) => { + releases.push(() => resolve({ content: 'saved', isBinary: false })) + }) + ) + try { + for (const worktreeId of [target.worktreeId, 'other-repo::/repo']) { + scheduleSelfWriteAwareEditorExternalReload( + target, + notification, + { ...file('hub-a'), worktreeId }, + { content: 'saved' } + ) + } + expect(readRuntimeFileContent).toHaveBeenCalledTimes(2) + expect(readRuntimeFileContent).toHaveBeenLastCalledWith( + expect.objectContaining({ worktreeId: 'other-repo::/repo' }) + ) + } finally { + releases.forEach((release) => release()) + await Promise.resolve() + } + }) + it.each([false, true])( 'shares reads only for the same recorded owner (%s)', async (different) => { From 4a805b5c70c19e9a0f822dea021a9f0463536980 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 18:20:38 -0700 Subject: [PATCH 08/10] Run the upstream agent launch database fixture in Node --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9098a6f566db..e837c26efb27 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -34,6 +34,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/claude-structured-session-integration.test.ts', 'src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts', 'src/main/runtime/exit-provenance-audit.test.ts', + 'src/main/runtime/rpc/methods/agent-launch-instant-tab.test.ts', 'src/main/runtime/orchestration-cli-subprocess.test.ts', 'src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts', 'src/main/runtime/orchestration-mailbox-crash-recovery.test.ts', From bd2e132c7ae5e55696e4f22e841e4b4991c22013 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 18:20:42 -0700 Subject: [PATCH 09/10] Preserve contradictory owner records until explicit document close --- .../components/editor/editor-model-host-isolation.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 index 08e944bdb2da..1ed5d4385631 100644 --- a/src/renderer/src/components/editor/editor-model-host-isolation.test.ts +++ b/src/renderer/src/components/editor/editor-model-host-isolation.test.ts @@ -79,7 +79,7 @@ afterEach(() => { }) describe('same-path models on different execution hosts', () => { - it('uses captured ownership over stale fields for persistence and clean duplicate close', () => { + it('persists captured ownership while preserving contradictory records until explicitly closed', async () => { const { owners, store, attach } = createHostModels() const remote = owners[3]! const stale = { @@ -99,7 +99,13 @@ describe('same-path models on different execution hosts', () => { }) attach() store.getState().closeFile(remote.file.id) + await Promise.resolve() + expect(store.getState().openFiles).toEqual([stale]) + expect(remote.model.isDisposed()).toBe(false) + store.getState().closeFile(stale.id) + await Promise.resolve() expect(store.getState().openFiles).toHaveLength(0) + expect(remote.model.isDisposed()).toBe(true) }) it.each(['clean', 'same-draft', 'different-drafts'] as const)( From 4979062c7e61e93339e2763d48a1ac0234165e78 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 7 Oct 2026 13:08:31 -0700 Subject: [PATCH 10/10] Preserve captured connection generations in rebased editor close identity --- .../slices/editor/file-ids/editor-file-ids.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts index 1105acb9095a..746b0261a8d5 100644 --- a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts @@ -144,11 +144,24 @@ export function collectSameDocumentOpenFileIds( if (file.mode !== 'edit') { return fileIds } - const closeIdentity = (candidate: OpenFile): string => - JSON.stringify([ + const closeIdentity = (candidate: OpenFile): string => { + const provenance = candidate.operationProvenance + const generation = provenance?.generation + return JSON.stringify([ + editorDocumentIdentityKey(candidate), editorDocumentIdentityKey({ ...candidate, ...getPersistedEditorOwnerFields(candidate) }), - state ? getEditorModelOwnerKey(candidate, state) : null + state ? getEditorModelOwnerKey(candidate, state) : null, + provenance?.ownershipProjection, + provenance?.expectedSshConnectionGeneration, + generation?.route.executionHostId, + generation?.route.runtimeEnvironmentId, + generation?.runtimeConnectionGeneration, + generation?.runtimePairingRevision, + generation?.runtimeSshGeneration, + generation?.nestedSshGeneration, + generation?.directSshGeneration ]) + } const identity = closeIdentity(file) const modes = getReusableOpenFileModes(file.mode) for (const candidate of openFiles) {