diff --git a/src/renderer/src/components/editor/editor-path-move-inflight.test.ts b/src/renderer/src/components/editor/editor-path-move-inflight.test.ts index d3975a547e2..1844cba2cde 100644 --- a/src/renderer/src/components/editor/editor-path-move-inflight.test.ts +++ b/src/renderer/src/components/editor/editor-path-move-inflight.test.ts @@ -4,7 +4,6 @@ import { __clearEditorPathMovesForTests, beginEditorPathMove, isActiveMoveSourcePath, - noteEditorPathMoveDestinationEvent, settleEditorPathMove } from './editor-path-move-inflight' @@ -16,8 +15,7 @@ describe('editor-path-move-inflight', () => { operationId: 'op-1', worktreeId: 'wt-1', runtimeEnvironmentId: null, - sourcePaths: ['/repo/a.md'], - targetPaths: ['/repo/sub/a.md'] + sourcePaths: ['/repo/a.md'] }) expect(isActiveMoveSourcePath('wt-1', null, '/repo/a.md')).toBe(true) @@ -31,44 +29,25 @@ describe('editor-path-move-inflight', () => { operationId: 'op-1', worktreeId: 'wt-1', runtimeEnvironmentId: 'env-1', - sourcePaths: ['/repo/a.md'], - targetPaths: [] + sourcePaths: ['/repo/a.md'] }) expect(isActiveMoveSourcePath('wt-1', 'env-1', '/repo/a.md')).toBe(true) expect(isActiveMoveSourcePath('wt-1', null, '/repo/a.md')).toBe(false) expect(isActiveMoveSourcePath('wt-2', 'env-1', '/repo/a.md')).toBe(false) }) - it('latches destination events and returns them on settle', () => { - beginEditorPathMove({ - operationId: 'op-1', - worktreeId: 'wt-1', - runtimeEnvironmentId: null, - sourcePaths: ['/repo/a.md'], - targetPaths: ['/repo/sub/a.md'] - }) - // A destination event that arrives before the rekey is latched, not lost. - expect(noteEditorPathMoveDestinationEvent('wt-1', null, '/repo/sub/a.md')).toBe(true) - expect(noteEditorPathMoveDestinationEvent('wt-1', null, '/repo/other.md')).toBe(false) - - const latched = settleEditorPathMove('op-1') - expect(latched).toEqual(['/repo/sub/a.md'.toLowerCase()]) - }) - it('keeps concurrent operations independent (settling one leaves the other)', () => { beginEditorPathMove({ operationId: 'op-1', worktreeId: 'wt-1', runtimeEnvironmentId: null, - sourcePaths: ['/repo/a.md'], - targetPaths: [] + sourcePaths: ['/repo/a.md'] }) beginEditorPathMove({ operationId: 'op-2', worktreeId: 'wt-1', runtimeEnvironmentId: null, - sourcePaths: ['/repo/b.md'], - targetPaths: [] + sourcePaths: ['/repo/b.md'] }) settleEditorPathMove('op-1') diff --git a/src/renderer/src/components/editor/editor-path-move-inflight.ts b/src/renderer/src/components/editor/editor-path-move-inflight.ts index b6ba3b4d322..7ba7b3a1762 100644 --- a/src/renderer/src/components/editor/editor-path-move-inflight.ts +++ b/src/renderer/src/components/editor/editor-path-move-inflight.ts @@ -5,20 +5,15 @@ import { normalizeAbsolutePathForComparison } from '@/components/right-sidebar/f // for the exact duration of its filesystem rename + atomic rekey, so there is // no TTL to race a slow SSH rename and no cross-operation aliasing. // -// Source side: while an operation is live, a watcher delete of one of its exact -// source paths is the move's own echo, not an external delete — don't tombstone. -// Destination side: a create/update at a target path may be the move's echo but -// may also be a concurrent external write; the operation only LATCHES that such -// an event was seen (before the rekey installs the tab) so the coordinator can -// route it into content verification after commit — the latch never suppresses. +// Only the source side needs this: while an operation is live, a watcher delete +// of one of its exact source paths is the move's own echo, not an external +// delete — don't tombstone. The destination is handled after commit: the +// coordinator proactively content-verifies every gated moved tab. type MoveOperation = { - operationId: string worktreeId: string runtimeEnvironmentId: string | null sourcePaths: Set - targetPaths: Set - destinationSeen: Set } const operations = new Map() @@ -36,38 +31,16 @@ export function beginEditorPathMove(args: { worktreeId: string runtimeEnvironmentId: string | null | undefined sourcePaths: readonly string[] - targetPaths: readonly string[] }): void { operations.set(args.operationId, { - operationId: args.operationId, worktreeId: args.worktreeId, runtimeEnvironmentId: owner(args.runtimeEnvironmentId), - sourcePaths: new Set(args.sourcePaths.map(normalize)), - targetPaths: new Set(args.targetPaths.map(normalize)), - destinationSeen: new Set() + sourcePaths: new Set(args.sourcePaths.map(normalize)) }) } -/** Removes the operation and returns the target paths whose destination event - * was latched while in flight, so the coordinator can verify them post-rekey. */ -export function settleEditorPathMove(operationId: string): string[] { - const operation = operations.get(operationId) +export function settleEditorPathMove(operationId: string): void { operations.delete(operationId) - return operation ? Array.from(operation.destinationSeen) : [] -} - -function matches( - operation: MoveOperation, - worktreeId: string, - runtimeEnvironmentId: string | null, - normalizedPath: string, - paths: Set -): boolean { - return ( - operation.worktreeId === worktreeId && - operation.runtimeEnvironmentId === runtimeEnvironmentId && - paths.has(normalizedPath) - ) } /** True when this delete is the source side of a live Orca-owned move. */ @@ -79,33 +52,17 @@ export function isActiveMoveSourcePath( const normalizedPath = normalize(absolutePath) const scopedOwner = owner(runtimeEnvironmentId) for (const operation of operations.values()) { - if (matches(operation, worktreeId, scopedOwner, normalizedPath, operation.sourcePaths)) { + if ( + operation.worktreeId === worktreeId && + operation.runtimeEnvironmentId === scopedOwner && + operation.sourcePaths.has(normalizedPath) + ) { return true } } return false } -/** Latch that a destination event was observed for a live move (used when the - * event arrives before the atomic rekey installs the tab). Returns true if it - * belonged to a live move. Never suppresses on its own. */ -export function noteEditorPathMoveDestinationEvent( - worktreeId: string, - runtimeEnvironmentId: string | null | undefined, - absolutePath: string -): boolean { - const normalizedPath = normalize(absolutePath) - const scopedOwner = owner(runtimeEnvironmentId) - let latched = false - for (const operation of operations.values()) { - if (matches(operation, worktreeId, scopedOwner, normalizedPath, operation.targetPaths)) { - operation.destinationSeen.add(normalizedPath) - latched = true - } - } - return latched -} - export function __clearEditorPathMovesForTests(): void { operations.clear() } diff --git a/src/renderer/src/hooks/useEditorExternalWatch-self-move.test.ts b/src/renderer/src/hooks/useEditorExternalWatch-self-move.test.ts index ea96c401175..a5ae6de8505 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch-self-move.test.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch-self-move.test.ts @@ -67,8 +67,7 @@ describe('self-move source suppression (in-flight)', () => { operationId: 'op-1', worktreeId: 'wt-1', runtimeEnvironmentId: null, - sourcePaths: ['/repo/notes.md'], - targetPaths: ['/repo/subdir/notes.md'] + sourcePaths: ['/repo/notes.md'] }) } diff --git a/src/renderer/src/hooks/useEditorExternalWatch.ts b/src/renderer/src/hooks/useEditorExternalWatch.ts index 1be31a23b14..addd8cb444b 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.ts @@ -19,10 +19,7 @@ import { getRecentSelfWrite, type RecentSelfWrite } from '@/components/editor/editor-self-write-registry' -import { - isActiveMoveSourcePath, - noteEditorPathMoveDestinationEvent -} from '@/components/editor/editor-path-move-inflight' +import { isActiveMoveSourcePath } from '@/components/editor/editor-path-move-inflight' import type { FsChangedPayload } from '../../../shared/types' import { findWorktreeById } from '@/store/slices/worktree-helpers' import type { OpenFile } from '@/store/slices/editor' @@ -647,14 +644,6 @@ export function createExternalWatchEventHandler( const absolutePath = joinPath(notification.worktreePath, notification.relativePath) const matching = getOpenFilesForExternalFileChange(openFilesSnapshot, notification) if (matching.length === 0) { - // Latch a destination event that arrived before the rekey installed the - // tab (drag-drop's refresh-before-remap window), so the coordinator can - // content-verify it post-rekey instead of dropping it. - noteEditorPathMoveDestinationEvent( - target.worktreeId, - target.runtimeEnvironmentId, - absolutePath - ) // Why: notify the combined-diff tab so its section reloads. Its own // dirty/section guards make a blanket reload safe, and there is no // in-memory editor content to clobber, so self-write suppression is diff --git a/src/renderer/src/lib/execute-open-editor-path-move.test.ts b/src/renderer/src/lib/execute-open-editor-path-move.test.ts index d7b489ab751..91770f80aeb 100644 --- a/src/renderer/src/lib/execute-open-editor-path-move.test.ts +++ b/src/renderer/src/lib/execute-open-editor-path-move.test.ts @@ -15,6 +15,7 @@ vi.mock('@/components/editor/editor-autosave', async (importOriginal) => { 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' const CONTEXT = { settings: null, @@ -77,6 +78,33 @@ describe('executeOpenEditorPathMove', () => { expect(__activeEditorPathMoveCountForTests()).toBe(0) }) + it('resolves the verify gate proactively when no destination watcher event arrives', async () => { + 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)) + + 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) + }) + it('undoes the on-disk rename when the editor rekey collides', async () => { openDirtyTab() // A distinct live session already occupies the destination id. diff --git a/src/renderer/src/lib/execute-open-editor-path-move.ts b/src/renderer/src/lib/execute-open-editor-path-move.ts index 45cf1b824ad..764a0124c58 100644 --- a/src/renderer/src/lib/execute-open-editor-path-move.ts +++ b/src/renderer/src/lib/execute-open-editor-path-move.ts @@ -10,7 +10,6 @@ import { remapOpenEditorTabsForPathChange } from '@/lib/remap-open-editor-tabs-for-path-change' import { verifyLatchedMoveDestinations } from '@/hooks/useEditorExternalWatch' -import { normalizeAbsolutePathForComparison } from '@/components/right-sidebar/file-explorer-paths' import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab' let moveOperationCounter = 0 @@ -40,7 +39,6 @@ export async function executeOpenEditorPathMove(args: { const affected = useAppStore .getState() .openFiles.filter((f) => isPathInsideOrEqual(fromPath, f.filePath)) - const newPathOf = (filePath: string): string => toPath + filePath.slice(fromPath.length) // In-flight source suppression is scoped per (worktree, runtime owner): the // same path can be open and watched under more than one worktree/owner. @@ -59,8 +57,7 @@ export async function executeOpenEditorPathMove(args: { operationId: subOperationId, worktreeId: first.worktreeId, runtimeEnvironmentId: first.runtimeEnvironmentId?.trim() || null, - sourcePaths: scopeFiles.map((f) => f.filePath), - targetPaths: scopeFiles.map((f) => newPathOf(f.filePath)) + sourcePaths: scopeFiles.map((f) => f.filePath) }) } @@ -122,21 +119,21 @@ export async function executeOpenEditorPathMove(args: { notifyHostOfMirroredEditorClose(mirrorState, file.worktreeId, file.id) } - const latchedTargets = new Set() for (const subOperationId of ownerSubOps) { - for (const target of settleEditorPathMove(subOperationId)) { - latchedTargets.add(target) - } + settleEditorPathMove(subOperationId) } - if (latchedTargets.size > 0) { - const latchedTabIds = useAppStore - .getState() - .openFiles.filter( - (f) => - f.pendingSelfMoveEcho && - latchedTargets.has(normalizeAbsolutePathForComparison(f.pendingSelfMoveEcho.targetPath)) - ) - .map((f) => f.id) - verifyLatchedMoveDestinations(worktreePath, context.connectionId, latchedTabIds) + + // Drive verification for EVERY tab the rekey gated — not just paths whose + // watcher event was latched. The rename has completed, so disk is authoritative + // now; verifying proactively guarantees the autosave gate resolves even if the + // destination watcher event never arrives (watcher down / dropped / coalesced). + // A move echo reads disk == baseline (suppress); a genuine write differs + // (banner). A later watcher event finds the provenance consumed and no-ops. + const gatedTabIds = useAppStore + .getState() + .openFiles.filter((f) => f.pendingSelfMoveEcho?.operationId === operationId) + .map((f) => f.id) + if (gatedTabIds.length > 0) { + verifyLatchedMoveDestinations(worktreePath, context.connectionId, gatedTabIds) } }