Allow case-only file explorer renames (#2763)
* Allow file explorer case-only renames Reference design doc: docs/file-explorer-case-only-rename.md * Remove case-only rename design doc
This commit is contained in:
parent
c93759659f
commit
b24cc8087a
|
|
@ -0,0 +1,51 @@
|
|||
import { lstat } from 'fs/promises'
|
||||
import type { Stats } from 'fs'
|
||||
import { basename, dirname } from 'path'
|
||||
|
||||
function isENOENT(error: unknown): boolean {
|
||||
return (
|
||||
error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'ENOENT'
|
||||
)
|
||||
}
|
||||
|
||||
function caseFoldFileExplorerBasename(name: string): string {
|
||||
// Why: APFS may surface canonically equivalent Unicode names in different forms.
|
||||
// Node has no filesystem-native collation API here, so this is best-effort.
|
||||
return name.normalize('NFC').toLowerCase()
|
||||
}
|
||||
|
||||
function hasSameFilesystemIdentity(oldStat: Stats, newStat: Stats): boolean {
|
||||
return oldStat.dev === newStat.dev && oldStat.ino === newStat.ino
|
||||
}
|
||||
|
||||
function isCaseOnlySameParentRename(oldPath: string, newPath: string): boolean {
|
||||
const oldBasename = basename(oldPath)
|
||||
const newBasename = basename(newPath)
|
||||
return (
|
||||
dirname(oldPath) === dirname(newPath) &&
|
||||
oldBasename !== newBasename &&
|
||||
caseFoldFileExplorerBasename(oldBasename) === caseFoldFileExplorerBasename(newBasename)
|
||||
)
|
||||
}
|
||||
|
||||
export async function assertFileExplorerRenameDestinationAvailable(
|
||||
oldPath: string,
|
||||
newPath: string
|
||||
): Promise<void> {
|
||||
let newStat: Stats
|
||||
try {
|
||||
newStat = await lstat(newPath)
|
||||
} catch (error) {
|
||||
if (isENOENT(error)) {
|
||||
return
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
const oldStat = await lstat(oldPath)
|
||||
if (hasSameFilesystemIdentity(oldStat, newStat) && isCaseOnlySameParentRename(oldPath, newPath)) {
|
||||
return
|
||||
}
|
||||
|
||||
throw new Error(`A file or folder named '${basename(newPath)}' already exists in this location`)
|
||||
}
|
||||
|
|
@ -58,6 +58,10 @@ function mockRealpath(mapping: Record<string, string>) {
|
|||
})
|
||||
}
|
||||
|
||||
function mockStats(dev: number, ino: number) {
|
||||
return { dev, ino, isDirectory: () => false }
|
||||
}
|
||||
|
||||
describe('registerFilesystemMutationHandlers', () => {
|
||||
beforeEach(() => {
|
||||
handlers.clear()
|
||||
|
|
@ -162,18 +166,22 @@ describe('registerFilesystemMutationHandlers', () => {
|
|||
expect(renameMock).toHaveBeenCalledWith(oldPath, newPath)
|
||||
})
|
||||
|
||||
it('rejects rename when destination already exists', async () => {
|
||||
it('rejects rename when destination already exists as a true collision', async () => {
|
||||
const oldPath = path.resolve('/workspace/repo/old.ts')
|
||||
const resolvedNewPath = path.resolve('/workspace/repo/new.ts')
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === oldPath) {
|
||||
return mockStats(1, 10)
|
||||
}
|
||||
if (p === resolvedNewPath) {
|
||||
return { isDirectory: () => false }
|
||||
return mockStats(1, 11)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(
|
||||
handlers.get('fs:rename')!(null, {
|
||||
oldPath: path.resolve('/workspace/repo/old.ts'),
|
||||
oldPath,
|
||||
newPath: resolvedNewPath
|
||||
})
|
||||
).rejects.toThrow("A file or folder named 'new.ts' already exists in this location")
|
||||
|
|
@ -181,6 +189,55 @@ describe('registerFilesystemMutationHandlers', () => {
|
|||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('allows case-only rename when destination is the same entry in the same parent', async () => {
|
||||
const oldPath = path.resolve('/workspace/repo/README.md')
|
||||
const newPath = path.resolve('/workspace/repo/readme.md')
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === oldPath || p === newPath) {
|
||||
return mockStats(2, 20)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await handlers.get('fs:rename')!(null, { oldPath, newPath })
|
||||
|
||||
expect(renameMock).toHaveBeenCalledWith(oldPath, newPath)
|
||||
})
|
||||
|
||||
it('rejects hard-link alias rename collisions even when dev and ino match', async () => {
|
||||
const oldPath = path.resolve('/workspace/repo/README.md')
|
||||
const newPath = path.resolve('/workspace/repo/README-hardlink.md')
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === oldPath || p === newPath) {
|
||||
return mockStats(3, 30)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(handlers.get('fs:rename')!(null, { oldPath, newPath })).rejects.toThrow(
|
||||
"A file or folder named 'README-hardlink.md' already exists in this location"
|
||||
)
|
||||
|
||||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects cross-parent case-only rename collisions even when dev and ino match', async () => {
|
||||
const oldPath = path.resolve('/workspace/repo/src/README.md')
|
||||
const newPath = path.resolve('/workspace/repo/docs/readme.md')
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === oldPath || p === newPath) {
|
||||
return mockStats(4, 40)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(handlers.get('fs:rename')!(null, { oldPath, newPath })).rejects.toThrow(
|
||||
"A file or folder named 'readme.md' already exists in this location"
|
||||
)
|
||||
|
||||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects rename when parent directory escapes allowed roots', async () => {
|
||||
// Why: the parent is still canonicalized (preserveSymlink only preserves
|
||||
// the leaf). A symlinked ancestor that points outside allowed roots must
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import type { Store } from '../persistence'
|
|||
import { authorizeExternalPath, resolveAuthorizedPath, isENOENT } from './filesystem-auth'
|
||||
import { requireSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch'
|
||||
import { importExternalPathsSsh } from './filesystem-import-ssh'
|
||||
import { assertFileExplorerRenameDestinationAvailable } from '../file-explorer-rename-collision'
|
||||
|
||||
/**
|
||||
* Re-throw filesystem errors with user-friendly messages.
|
||||
|
|
@ -118,7 +119,7 @@ export function registerFilesystemMutationHandlers(store: Store): void {
|
|||
// accidentally write into a symlinked destination name.
|
||||
const oldPath = await resolveAuthorizedPath(args.oldPath, store, { preserveSymlink: true })
|
||||
const newPath = await resolveAuthorizedPath(args.newPath, store, { preserveSymlink: true })
|
||||
await assertNotExists(newPath)
|
||||
await assertFileExplorerRenameDestinationAvailable(oldPath, newPath)
|
||||
await rename(oldPath, newPath)
|
||||
}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -3,14 +3,21 @@ import type * as Fs from 'fs'
|
|||
import type * as FsPromises from 'fs/promises'
|
||||
import type * as FilesystemAuth from '../ipc/filesystem-auth'
|
||||
|
||||
const { resolveAuthorizedPathMock, statMock, subscribeParcelWatcherMock, watchMock } = vi.hoisted(
|
||||
() => ({
|
||||
resolveAuthorizedPathMock: vi.fn(),
|
||||
statMock: vi.fn(),
|
||||
subscribeParcelWatcherMock: vi.fn(),
|
||||
watchMock: vi.fn()
|
||||
})
|
||||
)
|
||||
const {
|
||||
lstatMock,
|
||||
renameMock,
|
||||
resolveAuthorizedPathMock,
|
||||
statMock,
|
||||
subscribeParcelWatcherMock,
|
||||
watchMock
|
||||
} = vi.hoisted(() => ({
|
||||
lstatMock: vi.fn(),
|
||||
renameMock: vi.fn(),
|
||||
resolveAuthorizedPathMock: vi.fn(),
|
||||
statMock: vi.fn(),
|
||||
subscribeParcelWatcherMock: vi.fn(),
|
||||
watchMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('fs', async () => {
|
||||
const actual = await vi.importActual<typeof Fs>('fs')
|
||||
|
|
@ -24,6 +31,8 @@ vi.mock('fs/promises', async () => {
|
|||
const actual = await vi.importActual<typeof FsPromises>('fs/promises')
|
||||
return {
|
||||
...actual,
|
||||
lstat: lstatMock,
|
||||
rename: renameMock,
|
||||
stat: statMock
|
||||
}
|
||||
})
|
||||
|
|
@ -48,15 +57,45 @@ vi.mock('../providers/ssh-filesystem-dispatch', () => ({
|
|||
|
||||
import { awaitRuntimeFileWatcherUnsubscribes, RuntimeFileCommands } from './orca-runtime-files'
|
||||
|
||||
function enoent(): Error {
|
||||
return Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
|
||||
}
|
||||
|
||||
function mockStats(dev: number, ino: number) {
|
||||
return { dev, ino, isDirectory: () => false }
|
||||
}
|
||||
|
||||
function createRuntimeFileCommands() {
|
||||
const store = {
|
||||
getRepo: vi.fn(() => undefined)
|
||||
}
|
||||
const commands = new RuntimeFileCommands({
|
||||
getRuntimeId: () => 'runtime-1',
|
||||
requireStore: () => store,
|
||||
resolveWorktreeSelector: vi.fn(async () => ({
|
||||
id: 'wt-1',
|
||||
repoId: 'repo-1',
|
||||
path: '/repo'
|
||||
})),
|
||||
resolveRuntimeGitTarget: vi.fn(),
|
||||
openFile: vi.fn()
|
||||
} as never)
|
||||
return { commands, store }
|
||||
}
|
||||
|
||||
describe('RuntimeFileCommands', () => {
|
||||
const originalPlatform = process.platform
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
lstatMock.mockReset()
|
||||
renameMock.mockReset()
|
||||
resolveAuthorizedPathMock.mockReset()
|
||||
statMock.mockReset()
|
||||
subscribeParcelWatcherMock.mockReset()
|
||||
watchMock.mockReset()
|
||||
lstatMock.mockRejectedValue(enoent())
|
||||
renameMock.mockResolvedValue(undefined)
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: originalPlatform
|
||||
|
|
@ -98,6 +137,84 @@ describe('RuntimeFileCommands', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('renames a runtime-local file when destination does not exist', async () => {
|
||||
const { commands } = createRuntimeFileCommands()
|
||||
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
|
||||
|
||||
await commands.renameFileExplorerPath('id:wt-1', 'old.ts', 'new.ts')
|
||||
|
||||
expect(renameMock).toHaveBeenCalledWith('/repo/old.ts', '/repo/new.ts')
|
||||
})
|
||||
|
||||
it('allows runtime-local case-only rename with IPC parity guard behavior', async () => {
|
||||
const { commands } = createRuntimeFileCommands()
|
||||
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === '/repo/README.md' || p === '/repo/readme.md') {
|
||||
return mockStats(10, 100)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await commands.renameFileExplorerPath('id:wt-1', 'README.md', 'readme.md')
|
||||
|
||||
expect(renameMock).toHaveBeenCalledWith('/repo/README.md', '/repo/readme.md')
|
||||
})
|
||||
|
||||
it('rejects runtime-local true destination collisions', async () => {
|
||||
const { commands } = createRuntimeFileCommands()
|
||||
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === '/repo/old.ts') {
|
||||
return mockStats(11, 110)
|
||||
}
|
||||
if (p === '/repo/new.ts') {
|
||||
return mockStats(11, 111)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(commands.renameFileExplorerPath('id:wt-1', 'old.ts', 'new.ts')).rejects.toThrow(
|
||||
"A file or folder named 'new.ts' already exists in this location"
|
||||
)
|
||||
|
||||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects runtime-local hard-link alias collisions', async () => {
|
||||
const { commands } = createRuntimeFileCommands()
|
||||
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === '/repo/README.md' || p === '/repo/README-hardlink.md') {
|
||||
return mockStats(12, 120)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(
|
||||
commands.renameFileExplorerPath('id:wt-1', 'README.md', 'README-hardlink.md')
|
||||
).rejects.toThrow("A file or folder named 'README-hardlink.md' already exists in this location")
|
||||
|
||||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects runtime-local cross-parent case-only collisions', async () => {
|
||||
const { commands } = createRuntimeFileCommands()
|
||||
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
|
||||
lstatMock.mockImplementation(async (p: string) => {
|
||||
if (p === '/repo/src/README.md' || p === '/repo/docs/readme.md') {
|
||||
return mockStats(13, 130)
|
||||
}
|
||||
throw enoent()
|
||||
})
|
||||
|
||||
await expect(
|
||||
commands.renameFileExplorerPath('id:wt-1', 'src/README.md', 'docs/readme.md')
|
||||
).rejects.toThrow("A file or folder named 'readme.md' already exists in this location")
|
||||
|
||||
expect(renameMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('uses a conservative Node watcher for Windows runtime file watches', async () => {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ import {
|
|||
getSshFilesystemProvider,
|
||||
SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE
|
||||
} from '../providers/ssh-filesystem-dispatch'
|
||||
import { assertFileExplorerRenameDestinationAvailable } from '../file-explorer-rename-collision'
|
||||
import { joinWorktreeRelativePath, normalizeRuntimeRelativePath } from './runtime-relative-paths'
|
||||
|
||||
const MOBILE_FILE_LIST_LIMIT = 5000
|
||||
|
|
@ -549,7 +550,7 @@ export class RuntimeFileCommands {
|
|||
const store = this.host.requireStore()
|
||||
const oldPath = await resolveAuthorizedPath(oldTarget.path, store, { preserveSymlink: true })
|
||||
const newPath = await resolveAuthorizedPath(newTarget.path, store, { preserveSymlink: true })
|
||||
await assertRuntimePathDoesNotExist(newPath)
|
||||
await assertFileExplorerRenameDestinationAvailable(oldPath, newPath)
|
||||
await rename(oldPath, newPath)
|
||||
return { ok: true }
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue