diff --git a/src/renderer/src/runtime/web-session-tabs-sync-editor-tabs.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-editor-tabs.test.ts index c978ac47e1b..0820a71164a 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync-editor-tabs.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync-editor-tabs.test.ts @@ -410,4 +410,109 @@ describe('applyWebSessionTabsSnapshot', () => { expect(patch.activeTabType).toBeUndefined() expect(patch.activeTabTypeByWorktree).toBeUndefined() }) + + describe('client dirtiness across a host republish (#21392)', () => { + const notesPath = '/repo/NOTES.md' + const mirroredNotes = (isDirty: boolean): OpenFile => ({ + id: notesPath, + filePath: notesPath, + relativePath: 'NOTES.md', + worktreeId: WT, + language: 'markdown', + isDirty, + runtimeEnvironmentId: ENV, + mode: 'edit', + mirroredFromRuntimeSession: true + }) + const notesUnifiedTab: Tab = { + id: 'host-notes-unified', + entityId: notesPath, + groupId: 'host-group-1', + worktreeId: WT, + contentType: 'editor', + label: 'NOTES.md', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: NOW - 10, + isPreview: false, + isPinned: false + } + // The host republishes the same tab with its own store's flag: not dirty. + const hostCleanSnapshot = () => + makeSnapshot( + [ + { + type: 'markdown', + id: 'host-notes-unified', + title: 'NOTES.md', + filePath: notesPath, + relativePath: 'NOTES.md', + language: 'markdown', + mode: 'edit', + isDirty: false, + isActive: true, + sourceFileId: notesPath, + sourceFilePath: notesPath, + sourceRelativePath: 'NOTES.md', + documentVersion: `file:${notesPath}`, + color: null, + isPinned: false + } + ], + { activeTabId: 'host-notes-unified', activeTabType: 'markdown' } + ) + + it('keeps a client-dirty mirrored file dirty when the host republishes isDirty: false', () => { + // Why: the host never learns about client edits, so its flag would otherwise erase the + // client's, and the tab strip would close the tab with no prompt while the draft lives. + const patch = applyWebSessionTabsSnapshot( + makeState({ + openFiles: [mirroredNotes(true)], + editorDrafts: { [notesPath]: '# unsaved client edits' }, + unifiedTabsByWorktree: { [WT]: [notesUnifiedTab] } + }), + hostCleanSnapshot(), + ENV, + NOW + ) + + // No open-file change means the dirty flag survived exactly as it was. + expect(patch.openFiles).toBeUndefined() + }) + + it('follows a host-side save when the client holds no draft', () => { + // Why: a dirty flag with no client draft came from an earlier host snapshot; keeping it + // would strand the tab as dirty after the host saved. + const patch = applyWebSessionTabsSnapshot( + makeState({ + openFiles: [mirroredNotes(true)], + editorDrafts: {}, + unifiedTabsByWorktree: { [WT]: [notesUnifiedTab] } + }), + hostCleanSnapshot(), + ENV, + NOW + ) + + expect(patch.openFiles).toMatchObject([{ id: notesPath, isDirty: false }]) + }) + + it('does not invent dirtiness from a draft the client already reverted', () => { + // Why: a lingering draft with isDirty false means the user typed and undid; the tab is + // clean and must not start prompting on close. + const patch = applyWebSessionTabsSnapshot( + makeState({ + openFiles: [mirroredNotes(false)], + editorDrafts: { [notesPath]: 'same as disk' }, + unifiedTabsByWorktree: { [WT]: [notesUnifiedTab] } + }), + hostCleanSnapshot(), + ENV, + NOW + ) + + expect(patch.openFiles).toBeUndefined() + }) + }) }) diff --git a/src/renderer/src/runtime/web-session-tabs-sync-mirrored-draft-close.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-mirrored-draft-close.test.ts new file mode 100644 index 00000000000..3091d9370cc --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync-mirrored-draft-close.test.ts @@ -0,0 +1,133 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Tab } from '../../../shared/tab-types' +import type { OpenFile } from '../store/slices/editor' + +const closeWebRuntimeSessionTabMock = vi.fn(async (_args: unknown) => 'applied' as const) + +vi.mock('./web-runtime-session', () => ({ + closeWebRuntimeSessionTab: (args: unknown) => closeWebRuntimeSessionTabMock(args) +})) + +import { useAppStore } from '../store' +import { createWorkspaceTabCloseCommands } from '@/components/tab-group/workspace-tab-close-commands' +import { ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT } from '@/components/editor/editor-autosave' +import { applyWebSessionTabsSnapshot } from './web-session-tabs-sync' +import { + ENV, + NOW, + WT, + makeSnapshot, + resetWebSessionTabsSyncTestState +} from './web-session-tabs-sync-test-harness' + +const notesPath = '/repo/NOTES.md' +const clientDraft = '# unsaved client edits' + +const notesUnifiedTab: Tab = { + id: 'host-notes-unified', + entityId: notesPath, + groupId: 'host-group-1', + worktreeId: WT, + contentType: 'editor', + label: 'NOTES.md', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: NOW - 10, + isPreview: false, + isPinned: false +} + +// A host-mirrored tab the user has edited on this client: dirty, with a draft recorded. +const clientDirtyMirroredNotes: OpenFile = { + id: notesPath, + filePath: notesPath, + relativePath: 'NOTES.md', + worktreeId: WT, + language: 'markdown', + isDirty: true, + runtimeEnvironmentId: ENV, + mode: 'edit', + mirroredFromRuntimeSession: true +} + +// The host republishes the same tab; its own store has no unsaved edits. +function hostCleanRepublish() { + return makeSnapshot( + [ + { + type: 'markdown', + id: notesUnifiedTab.id, + title: 'NOTES.md', + filePath: notesPath, + relativePath: 'NOTES.md', + language: 'markdown', + mode: 'edit', + isDirty: false, + isActive: true, + sourceFileId: notesPath, + sourceFilePath: notesPath, + sourceRelativePath: 'NOTES.md', + documentVersion: `file:${notesPath}`, + color: null, + isPinned: false + } + ], + { activeTabId: notesUnifiedTab.id, activeTabType: 'markdown' } + ) +} + +describe('tab-strip close of a client-dirty mirrored file after a host republish (#21392)', () => { + const initialState = useAppStore.getState() + const closeRequests: string[] = [] + const onCloseRequest = (event: Event): void => { + if (event instanceof CustomEvent) { + closeRequests.push(String(event.detail?.fileId)) + } + } + + beforeEach(() => { + resetWebSessionTabsSyncTestState() + closeWebRuntimeSessionTabMock.mockClear() + closeRequests.length = 0 + window.addEventListener(ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, onCloseRequest) + useAppStore.setState({ + ...initialState, + activeWorktreeId: WT, + openFiles: [clientDirtyMirroredNotes], + editorDrafts: { [notesPath]: clientDraft }, + unifiedTabsByWorktree: { [WT]: [notesUnifiedTab] } + }) + }) + + afterEach(() => { + window.removeEventListener(ORCA_EDITOR_REQUEST_FILE_CLOSE_EVENT, onCloseRequest) + useAppStore.setState(initialState, true) + }) + + it('routes the close to the unsaved-changes prompt instead of discarding the draft', () => { + // Why: this is the user-visible property. #21363 lost a draft on a transient error; this + // path loses one on an ordinary Cmd+W / tab X unless the client's dirty flag survives the + // host's republish, because the tab strip gates its prompt on that flag alone. + const patch = applyWebSessionTabsSnapshot( + useAppStore.getState(), + hostCleanRepublish(), + ENV, + NOW + ) + useAppStore.setState(patch) + + createWorkspaceTabCloseCommands({ worktreeId: WT, groupTabs: [notesUnifiedTab] }).closeItem( + notesUnifiedTab.id + ) + + // Prompted, not closed: the request went to the save/discard queue and nothing was lost. + expect(closeRequests).toEqual([notesPath]) + const state = useAppStore.getState() + expect(state.openFiles.some((file) => file.id === notesPath)).toBe(true) + expect(state.editorDrafts[notesPath]).toBe(clientDraft) + expect(closeWebRuntimeSessionTabMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts index 6752cdcd8e1..4dcfd7ae44d 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts @@ -117,7 +117,8 @@ export function prepareWebSessionTabsSnapshotBrowser( hostGroupIdByTabId, targetGroupId, mirroredTerminalTabEntries.length + mirroredBrowserTabs.length, - now + now, + (fileId) => state.editorDrafts?.[fileId] !== undefined ) const mirroredAgentTabs = buildMirroredAgentTabs( snapshot, diff --git a/src/renderer/src/runtime/web-session-tabs-sync/state.ts b/src/renderer/src/runtime/web-session-tabs-sync/state.ts index f44d4a3d18c..8db00f5a238 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/state.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/state.ts @@ -202,6 +202,9 @@ export type WebSessionTabsSyncState = Pick< | 'activityClearedAtByPaneKey' | 'agentLaunchConfigByPaneKey' | 'automaticAgentResumeClaimsByTabId' + // Why: a client draft is the evidence that a mirrored file's dirty flag is the client's + // own and must survive a host republish (#21392); absent here, the host flag wins. + | 'editorDrafts' | 'migrationUnsupportedByPtyId' | 'manuallyUnreadTurnsByPaneKey' | 'paneForegroundAgentByPaneKey' diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tab-builders.ts b/src/renderer/src/runtime/web-session-tabs-sync/tab-builders.ts index 94234af1276..bc1dc6d83b1 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tab-builders.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tab-builders.ts @@ -103,7 +103,8 @@ export function buildMirroredEditorTabs( hostGroupIdByTabId: ReadonlyMap, fallbackGroupId: string, sortOffset: number, - now: number + now: number, + hasLocalDraft: (fileId: string) => boolean ): MirroredEditorTab[] { return snapshot.tabs.filter(isReadyEditorTab).map((tab, index) => { const fileId = localEditorFileId(tab) @@ -111,6 +112,12 @@ export function buildMirroredEditorTabs( const existingUnifiedTab = existingTabIndex.getEditorUnifiedTab(fileId, tab.id) const sourceFileId = editorSourceFileId(tab) const groupId = hostGroupIdByTabId.get(tab.id) ?? fallbackGroupId + // Why: the host publishes only its own store's flag and never learns of client edits, so + // taking it verbatim would clear a client-dirty tab and the tab strip would then close it + // with no unsaved-changes prompt while the draft still exists (#21392). A local draft is + // the evidence the flag is the client's own; a dirty flag with no draft came from an + // earlier snapshot and must keep following the host, e.g. after a host-side save. + const keepsClientDirty = existingFile?.isDirty === true && hasLocalDraft(fileId) const file: OpenFile = { ...existingFile, id: fileId, @@ -118,7 +125,7 @@ export function buildMirroredEditorTabs( relativePath: tab.relativePath, worktreeId: snapshot.worktree, language: tab.language, - isDirty: tab.isDirty, + isDirty: tab.isDirty || keepsClientDirty, runtimeEnvironmentId: environmentId, mode: tab.type === 'markdown' ? tab.mode : 'edit', markdownPreviewSourceFileId: sourceFileId,