mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(editor): don't consume move-echo provenance in the safety-net verify (round 5)
The round-3 proactive post-commit verify ran resolveLiveMoveVerification, which consumed pendingSelfMoveEcho. On FSEvents/SSH the real destination watcher event reliably lands AFTER the fast local read, so it then found no provenance, took the immediate changed-on-disk mark (no baseline check when there is no recent self-write), and raised a false conflict banner on the just-moved dirty tab — the exact data-loss this change removes. The proactive verify is a safety net: it now releases the autosave gate but leaves the provenance, so a later destination event is still recognized as the move's own echo and content-verified. Only a real watcher event consumes the provenance. Keeping it is safe — every watcher consumer verifies by content, which is strictly safer than the immediate mark. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -285,7 +285,8 @@ describe('verifyLatchedMoveDestinations (proactive, cross-worktree path)', () =>
|
||||
expect(readFile).toHaveBeenCalledWith(expect.objectContaining({ filePath: FLOATING_FILE_PATH }))
|
||||
// Disk == baseline -> echo -> suppress + release the gate, no false banner.
|
||||
expect(setExternalMutation).not.toHaveBeenCalledWith('file-x', 'changed')
|
||||
expect(clearSelfMoveEcho).toHaveBeenCalledWith('file-x')
|
||||
expect(setPendingLiveDiskVerification).toHaveBeenCalledWith('file-x', false)
|
||||
// Safety net does NOT consume provenance (a later watcher event still needs it).
|
||||
expect(clearSelfMoveEcho).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -669,7 +669,9 @@ export function createExternalWatchEventHandler(
|
||||
normalizedAbsolutePath
|
||||
)
|
||||
if (isSelfMoveEcho) {
|
||||
scheduleSelfMoveEchoVerification(target, dirtyIds)
|
||||
// A real destination fs event confirms the echo — consume the
|
||||
// provenance so a later genuine write takes the normal path.
|
||||
scheduleSelfMoveEchoVerification(target, dirtyIds, true)
|
||||
} else {
|
||||
// An external write on a dirty tab must not vanish silently (issue
|
||||
// #7265); mark it so the editor shows the reload banner.
|
||||
@@ -810,7 +812,8 @@ type LiveMoveVerifyCandidate = {
|
||||
function resolveLiveMoveVerification(
|
||||
candidate: LiveMoveVerifyCandidate,
|
||||
diskSignature: string | null,
|
||||
connectionId: string | undefined
|
||||
connectionId: string | undefined,
|
||||
consumeProvenance: boolean
|
||||
): void {
|
||||
const { fileId, baseline, generation, operationId } = candidate
|
||||
if (liveMoveVerifyGeneration.get(fileId) !== generation) {
|
||||
@@ -832,9 +835,14 @@ function resolveLiveMoveVerification(
|
||||
) {
|
||||
return
|
||||
}
|
||||
// Consume the provenance: this echo is settled, so a later write at this path
|
||||
// takes the normal changed-on-disk path.
|
||||
state.clearSelfMoveEcho(fileId)
|
||||
// Consume the provenance only when a real watcher event drove this check: the
|
||||
// proactive post-commit verify is a safety net and must LEAVE the provenance,
|
||||
// or the destination fs event (which on FSEvents/SSH lands after the faster
|
||||
// local read) would fall through to the immediate changed-on-disk mark and
|
||||
// raise a false conflict banner. Keeping it is safe — every consumer verifies.
|
||||
if (consumeProvenance) {
|
||||
state.clearSelfMoveEcho(fileId)
|
||||
}
|
||||
const isMoveEcho = baseline !== undefined && diskSignature === baseline
|
||||
if (!isMoveEcho) {
|
||||
markFileChangedOnDisk(state, file, { connectionId, origin: 'live' })
|
||||
@@ -860,9 +868,12 @@ export function verifyLatchedMoveDestinations(
|
||||
}
|
||||
// Owner/worktree are resolved per-tab inside the read; only connectionId is
|
||||
// shared. worktreePath is unused here (each tab is read at its own filePath).
|
||||
// consumeProvenance:false — this is the safety net; leave the provenance so a
|
||||
// destination watcher event arriving after this read is still seen as an echo.
|
||||
scheduleSelfMoveEchoVerification(
|
||||
{ worktreeId: '', worktreePath, connectionId, runtimeEnvironmentId: null },
|
||||
gated
|
||||
gated,
|
||||
false
|
||||
)
|
||||
}
|
||||
|
||||
@@ -870,7 +881,11 @@ export function verifyLatchedMoveDestinations(
|
||||
// external write does not. Read the destination once and settle each dirty tab
|
||||
// by content identity. Autosave is suspended synchronously first so a write
|
||||
// landing mid-read can't be overwritten before we decide.
|
||||
function scheduleSelfMoveEchoVerification(target: WatchedTarget, fileIds: string[]): void {
|
||||
function scheduleSelfMoveEchoVerification(
|
||||
target: WatchedTarget,
|
||||
fileIds: string[],
|
||||
consumeProvenance: boolean
|
||||
): void {
|
||||
if (fileIds.length === 0) {
|
||||
return
|
||||
}
|
||||
@@ -902,9 +917,16 @@ function scheduleSelfMoveEchoVerification(target: WatchedTarget, fileIds: string
|
||||
})
|
||||
.then((result) => {
|
||||
const diskSignature = result.isBinary ? null : getDiskBaselineSignature(result.content)
|
||||
resolveLiveMoveVerification(candidate, diskSignature, target.connectionId)
|
||||
resolveLiveMoveVerification(
|
||||
candidate,
|
||||
diskSignature,
|
||||
target.connectionId,
|
||||
consumeProvenance
|
||||
)
|
||||
})
|
||||
.catch(() => resolveLiveMoveVerification(candidate, null, target.connectionId))
|
||||
.catch(() =>
|
||||
resolveLiveMoveVerification(candidate, null, target.connectionId, consumeProvenance)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import { useAppStore } from '@/store'
|
||||
import { executeOpenEditorPathMove } from './execute-open-editor-path-move'
|
||||
import { __activeEditorPathMoveCountForTests } from '@/components/editor/editor-path-move-inflight'
|
||||
import { getDiskBaselineSignature } from '@/components/editor/diff-content-signature'
|
||||
import { createExternalWatchEventHandler } from '@/hooks/useEditorExternalWatch'
|
||||
|
||||
const CONTEXT = {
|
||||
settings: null,
|
||||
@@ -100,9 +101,56 @@ describe('executeOpenEditorPathMove', () => {
|
||||
const moved = useAppStore.getState().openFiles[0]!
|
||||
// Disk matched the carried baseline: gate released, no false conflict banner.
|
||||
expect(moved.pendingLiveDiskVerification).toBeFalsy()
|
||||
expect(moved.pendingSelfMoveEcho).toBeUndefined()
|
||||
expect(moved.externalMutation).toBeUndefined()
|
||||
expect(moved.isDirty).toBe(true)
|
||||
// Safety-net verify LEAVES the provenance so a destination watcher event
|
||||
// arriving after this read is still recognized as the move's own echo.
|
||||
expect(moved.pendingSelfMoveEcho?.targetPath).toBe('/repo/sub/a.md')
|
||||
})
|
||||
|
||||
it('does not raise a false banner when the destination watcher event lands after the move', async () => {
|
||||
// Regression: the proactive verify must NOT consume the echo provenance, or
|
||||
// the real destination fs event (which on FSEvents/SSH lands after the fast
|
||||
// local read) would fall through to the immediate changed-on-disk mark.
|
||||
const DISK_CONTENT = 'the file as it exists on disk\n'
|
||||
const id = openDirtyTab()
|
||||
useAppStore.getState().setLastKnownDiskSignature(id, getDiskBaselineSignature(DISK_CONTENT))
|
||||
vi.stubGlobal('window', {
|
||||
api: {
|
||||
fs: { readFile: vi.fn().mockResolvedValue({ isBinary: false, content: DISK_CONTENT }) }
|
||||
}
|
||||
})
|
||||
|
||||
await executeOpenEditorPathMove({
|
||||
context: CONTEXT,
|
||||
fromPath: '/repo/a.md',
|
||||
toPath: '/repo/sub/a.md',
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/repo'
|
||||
})
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
|
||||
// The delayed destination create/update event finally arrives.
|
||||
const { handleFsChanged, dispose } = createExternalWatchEventHandler((worktreePath) =>
|
||||
worktreePath === '/repo'
|
||||
? {
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined,
|
||||
runtimeEnvironmentId: null
|
||||
}
|
||||
: undefined
|
||||
)
|
||||
handleFsChanged({
|
||||
worktreePath: '/repo',
|
||||
events: [{ kind: 'update', absolutePath: '/repo/sub/a.md' }]
|
||||
})
|
||||
await new Promise((resolve) => setTimeout(resolve, 120))
|
||||
|
||||
const moved = useAppStore.getState().openFiles[0]!
|
||||
expect(moved.externalMutation).toBeUndefined()
|
||||
expect(moved.isDirty).toBe(true)
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('undoes the on-disk rename when the editor rekey collides', async () => {
|
||||
|
||||
Reference in New Issue
Block a user