mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(runtime): keep a client-dirty mirrored file dirty across a host republish (#21393)
The host publishes only its own store's isDirty and never learns about client edits, so rebuilding a mirrored OpenFile from the snapshot cleared the client's flag while editorDrafts still held the draft. The tab strip then closed the tab with no unsaved-changes prompt and closeFile deleted the draft; the external-change reload guards would reload over it too. Keep the client's flag when the client's file is dirty and it holds a draft; with no draft the host's flag still wins so a host-side save does not strand the tab as dirty. A host-side save never clears a client draft. Fixes #21392
This commit is contained in:
@@ -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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -117,7 +117,8 @@ export function prepareWebSessionTabsSnapshotBrowser(
|
||||
hostGroupIdByTabId,
|
||||
targetGroupId,
|
||||
mirroredTerminalTabEntries.length + mirroredBrowserTabs.length,
|
||||
now
|
||||
now,
|
||||
(fileId) => state.editorDrafts?.[fileId] !== undefined
|
||||
)
|
||||
const mirroredAgentTabs = buildMirroredAgentTabs(
|
||||
snapshot,
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -103,7 +103,8 @@ export function buildMirroredEditorTabs(
|
||||
hostGroupIdByTabId: ReadonlyMap<string, string>,
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user