mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(editor): prefix-suppress the move root so late tabs under a dir move aren't flagged (round 8)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -24,6 +24,21 @@ describe('editor-path-move-inflight', () => {
|
||||
expect(__activeEditorPathMoveCountForTests()).toBe(0)
|
||||
})
|
||||
|
||||
it('prefix-matches a root so a file opened under a moving directory is suppressed', () => {
|
||||
beginEditorPathMove({
|
||||
operationId: 'op-1',
|
||||
worktreeId: 'wt-1',
|
||||
runtimeEnvironmentId: null,
|
||||
sourcePaths: ['/repo/src']
|
||||
})
|
||||
// The exact root, and a file that appeared under it mid-move, are both echoes.
|
||||
expect(isActiveMoveSourcePath('wt-1', null, '/repo/src')).toBe(true)
|
||||
expect(isActiveMoveSourcePath('wt-1', null, '/repo/src/late.ts')).toBe(true)
|
||||
// A sibling outside the root is a genuine external delete.
|
||||
expect(isActiveMoveSourcePath('wt-1', null, '/repo/srcabc.ts')).toBe(false)
|
||||
expect(isActiveMoveSourcePath('wt-1', null, '/repo/other.ts')).toBe(false)
|
||||
})
|
||||
|
||||
it('scopes by worktree and runtime owner', () => {
|
||||
beginEditorPathMove({
|
||||
operationId: 'op-1',
|
||||
|
||||
@@ -6,14 +6,16 @@ import { normalizeAbsolutePathForComparison } from '@/components/right-sidebar/f
|
||||
// no TTL to race a slow SSH rename and no cross-operation aliasing.
|
||||
//
|
||||
// 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.
|
||||
// of anything under one of its source ROOTS is the move's own echo, not an
|
||||
// external delete — don't tombstone. Roots are prefix-matched (not exact) so a
|
||||
// file opened UNDER a moving directory mid-rename is covered too. The
|
||||
// destination is handled after commit: the coordinator proactively
|
||||
// content-verifies every gated moved tab.
|
||||
|
||||
type MoveOperation = {
|
||||
worktreeId: string
|
||||
runtimeEnvironmentId: string | null
|
||||
sourcePaths: Set<string>
|
||||
sourceRoots: string[]
|
||||
}
|
||||
|
||||
const operations = new Map<string, MoveOperation>()
|
||||
@@ -26,16 +28,23 @@ function normalize(absolutePath: string): string {
|
||||
return normalizeAbsolutePathForComparison(absolutePath)
|
||||
}
|
||||
|
||||
// Both sides are normalized (separators folded, trailing slash trimmed), so a
|
||||
// single-separator prefix check is exact.
|
||||
function isInsideOrEqual(root: string, candidate: string): boolean {
|
||||
return candidate === root || candidate.startsWith(`${root}/`)
|
||||
}
|
||||
|
||||
export function beginEditorPathMove(args: {
|
||||
operationId: string
|
||||
worktreeId: string
|
||||
runtimeEnvironmentId: string | null | undefined
|
||||
/** Move roots (fromPath); a delete under any of them is suppressed. */
|
||||
sourcePaths: readonly string[]
|
||||
}): void {
|
||||
operations.set(args.operationId, {
|
||||
worktreeId: args.worktreeId,
|
||||
runtimeEnvironmentId: owner(args.runtimeEnvironmentId),
|
||||
sourcePaths: new Set(args.sourcePaths.map(normalize))
|
||||
sourceRoots: args.sourcePaths.map(normalize)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -52,11 +61,10 @@ export function isActiveMoveSourcePath(
|
||||
const normalizedPath = normalize(absolutePath)
|
||||
const scopedOwner = owner(runtimeEnvironmentId)
|
||||
for (const operation of operations.values()) {
|
||||
if (
|
||||
operation.worktreeId === worktreeId &&
|
||||
operation.runtimeEnvironmentId === scopedOwner &&
|
||||
operation.sourcePaths.has(normalizedPath)
|
||||
) {
|
||||
if (operation.worktreeId !== worktreeId || operation.runtimeEnvironmentId !== scopedOwner) {
|
||||
continue
|
||||
}
|
||||
if (operation.sourceRoots.some((root) => isInsideOrEqual(root, normalizedPath))) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,6 +86,45 @@ describe('self-move source suppression (in-flight)', () => {
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('does not tombstone a tab opened UNDER a moving directory mid-rename', () => {
|
||||
// A directory move registers the root; a file opened under it during the
|
||||
// in-flight window is not in the registered tab list but must still be
|
||||
// recognized as the move's own echo (prefix match) — no false conflict.
|
||||
const lateTab = {
|
||||
id: 'file-late',
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/repo',
|
||||
filePath: '/repo/src/late.md',
|
||||
relativePath: 'src/late.md',
|
||||
mode: 'edit' as const,
|
||||
isDirty: true
|
||||
}
|
||||
vi.mocked(useAppStore.getState).mockReturnValue({
|
||||
openFiles: [lateTab],
|
||||
setExternalMutation
|
||||
} as never)
|
||||
beginEditorPathMove({
|
||||
operationId: 'op-dir',
|
||||
worktreeId: 'wt-1',
|
||||
runtimeEnvironmentId: null,
|
||||
sourcePaths: ['/repo/src']
|
||||
})
|
||||
const { handleFsChanged, dispose } = createExternalWatchEventHandler(findTarget)
|
||||
|
||||
// The rename echo arrives as a delete+create pair for the old subtree path.
|
||||
handleFsChanged(
|
||||
payload([
|
||||
{ kind: 'delete', absolutePath: '/repo/src/late.md' },
|
||||
{ kind: 'create', absolutePath: '/repo/dst/late.md' }
|
||||
])
|
||||
)
|
||||
vi.advanceTimersByTime(200)
|
||||
|
||||
expect(setExternalMutation).not.toHaveBeenCalledWith('file-late', 'renamed')
|
||||
expect(setExternalMutation).not.toHaveBeenCalledWith('file-late', 'deleted')
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('tombstones a genuine delete once the move has settled', () => {
|
||||
beginMove()
|
||||
settleEditorPathMove('op-1')
|
||||
|
||||
@@ -11,7 +11,10 @@ import {
|
||||
} from '@/lib/remap-open-editor-tabs-for-path-change'
|
||||
import { verifyLatchedMoveDestinations } from '@/hooks/useEditorExternalWatch'
|
||||
import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab'
|
||||
import { getExecutionHostIdForWorktree } from '@/lib/worktree-runtime-owner'
|
||||
import {
|
||||
getExecutionHostIdForWorktree,
|
||||
getRuntimeEnvironmentIdForWorktree
|
||||
} from '@/lib/worktree-runtime-owner'
|
||||
|
||||
let moveOperationCounter = 0
|
||||
|
||||
@@ -47,24 +50,28 @@ export async function executeOpenEditorPathMove(args: {
|
||||
getExecutionHostIdForWorktree(moveState, f.worktreeId) === initiatingHostId
|
||||
)
|
||||
|
||||
// In-flight source suppression is scoped per (worktree, runtime owner): the
|
||||
// same path can be open and watched under more than one worktree/owner.
|
||||
// In-flight source suppression is scoped per (worktree, runtime owner) and
|
||||
// registers the move ROOT (fromPath, prefix-matched) — so a file opened UNDER a
|
||||
// moving directory mid-rename still has its own delete/rename echo suppressed,
|
||||
// even though it wasn't in `affected`. Always include the initiating scope even
|
||||
// when no tab is open there yet (an empty-affected directory move).
|
||||
const ownerSubOps: string[] = []
|
||||
const scopeKey = (f: (typeof affected)[number]): string =>
|
||||
`${f.worktreeId}::${f.runtimeEnvironmentId?.trim() || 'local'}`
|
||||
const scopes = new Map<string, typeof affected>()
|
||||
for (const f of affected) {
|
||||
;(scopes.get(scopeKey(f)) ?? scopes.set(scopeKey(f), []).get(scopeKey(f))!).push(f)
|
||||
const scopes = new Map<string, { worktreeId: string; owner: string | null }>()
|
||||
const addScope = (wtId: string, owner: string | null): void => {
|
||||
scopes.set(`${wtId}::${owner ?? 'local'}`, { worktreeId: wtId, owner })
|
||||
}
|
||||
for (const [, scopeFiles] of scopes) {
|
||||
const first = scopeFiles[0]!
|
||||
const subOperationId = `${operationId}::${scopeKey(first)}`
|
||||
addScope(worktreeId, getRuntimeEnvironmentIdForWorktree(moveState, worktreeId))
|
||||
for (const f of affected) {
|
||||
addScope(f.worktreeId, f.runtimeEnvironmentId?.trim() || null)
|
||||
}
|
||||
for (const [key, scope] of scopes) {
|
||||
const subOperationId = `${operationId}::${key}`
|
||||
ownerSubOps.push(subOperationId)
|
||||
beginEditorPathMove({
|
||||
operationId: subOperationId,
|
||||
worktreeId: first.worktreeId,
|
||||
runtimeEnvironmentId: first.runtimeEnvironmentId?.trim() || null,
|
||||
sourcePaths: scopeFiles.map((f) => f.filePath)
|
||||
worktreeId: scope.worktreeId,
|
||||
runtimeEnvironmentId: scope.owner,
|
||||
sourcePaths: [fromPath]
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user