mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 16:03:26 +00:00
Allow deleting files on WSL UNC paths from the file explorer
shell.trashItem cannot move a \wsl.localhost\<distro>\... UNC file to a Recycle Bin (the WSL virtual volume has none), so fs:deletePath threw and the file explorer showed an error. For WSL UNC targets, hard-delete via `rm` inside the distro through wsl.exe (a true delete that honors Linux perms). Normal local paths still go to the Recycle Bin via shell.trashItem. Fixes #6415
This commit is contained in:
@@ -41,7 +41,8 @@ const {
|
||||
cancelGenerateCommitMessageLocalMock,
|
||||
cancelGeneratePullRequestFieldsLocalMock,
|
||||
getSshFilesystemProviderMock,
|
||||
getSshGitProviderMock
|
||||
getSshGitProviderMock,
|
||||
tryDeleteWslUncPathMock
|
||||
} = vi.hoisted(() => ({
|
||||
handleMock: vi.fn(),
|
||||
showSaveDialogMock: vi.fn(),
|
||||
@@ -80,7 +81,8 @@ const {
|
||||
cancelGenerateCommitMessageLocalMock: vi.fn(),
|
||||
cancelGeneratePullRequestFieldsLocalMock: vi.fn(),
|
||||
getSshFilesystemProviderMock: vi.fn(),
|
||||
getSshGitProviderMock: vi.fn()
|
||||
getSshGitProviderMock: vi.fn(),
|
||||
tryDeleteWslUncPathMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
@@ -110,6 +112,10 @@ vi.mock('fs/promises', () => ({
|
||||
lstat: lstatMock
|
||||
}))
|
||||
|
||||
vi.mock('../wsl-unc-delete', () => ({
|
||||
tryDeleteWslUncPath: tryDeleteWslUncPathMock
|
||||
}))
|
||||
|
||||
vi.mock('../git/status', () => ({
|
||||
commitChanges: commitChangesMock,
|
||||
getStatus: getStatusMock,
|
||||
@@ -264,7 +270,8 @@ describe('registerFilesystemHandlers', () => {
|
||||
cancelGenerateCommitMessageLocalMock,
|
||||
cancelGeneratePullRequestFieldsLocalMock,
|
||||
getSshFilesystemProviderMock,
|
||||
getSshGitProviderMock
|
||||
getSshGitProviderMock,
|
||||
tryDeleteWslUncPathMock
|
||||
]) {
|
||||
mock.mockReset()
|
||||
}
|
||||
@@ -288,6 +295,8 @@ describe('registerFilesystemHandlers', () => {
|
||||
}
|
||||
])
|
||||
trashItemMock.mockResolvedValue(undefined)
|
||||
// Default: not a WSL UNC path, so deletePath falls through to shell.trashItem.
|
||||
tryDeleteWslUncPathMock.mockResolvedValue(false)
|
||||
showSaveDialogMock.mockResolvedValue({ canceled: true })
|
||||
fromWebContentsMock.mockReturnValue(null)
|
||||
getSshGitProviderMock.mockReturnValue(null)
|
||||
@@ -868,6 +877,42 @@ describe('registerFilesystemHandlers', () => {
|
||||
await handlers.get('fs:deletePath')!(null, { targetPath })
|
||||
|
||||
expect(trashItemMock).toHaveBeenCalledWith(targetPath)
|
||||
expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(targetPath, { recursive: undefined })
|
||||
})
|
||||
|
||||
// Regression for #6415: WSL UNC paths have no Recycle Bin, so shell.trashItem
|
||||
// throws. The handler must hard-delete via the distro instead of surfacing an
|
||||
// error popup.
|
||||
it('hard-deletes a WSL UNC path instead of trashing it', async () => {
|
||||
const wslUncRoot = '\\\\wsl.localhost\\Ubuntu\\home\\me\\repo'
|
||||
const targetPath = `${wslUncRoot}\\file.txt`
|
||||
registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, wslUncRoot])
|
||||
tryDeleteWslUncPathMock.mockResolvedValue(true)
|
||||
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
await handlers.get('fs:deletePath')!(null, { targetPath, recursive: true })
|
||||
|
||||
expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(targetPath, { recursive: true })
|
||||
// Critical: we must NOT call trashItem for WSL UNC paths — that is exactly
|
||||
// the call that throws and produced the user-facing error.
|
||||
expect(trashItemMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('propagates a WSL hard-delete failure instead of swallowing it', async () => {
|
||||
const wslUncRoot = '\\\\wsl.localhost\\Ubuntu\\home\\me\\repo'
|
||||
const targetPath = `${wslUncRoot}\\file.txt`
|
||||
registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, wslUncRoot])
|
||||
tryDeleteWslUncPathMock.mockRejectedValue(
|
||||
new Error('Failed to delete WSL path: Permission denied')
|
||||
)
|
||||
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
await expect(
|
||||
handlers.get('fs:deletePath')!(null, { targetPath })
|
||||
).rejects.toThrow('Failed to delete WSL path: Permission denied')
|
||||
expect(trashItemMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps non-image binaries hidden from the editor payload', async () => {
|
||||
|
||||
@@ -7,6 +7,7 @@ import { dirname, extname, join, resolve } from 'path'
|
||||
import type { ChildProcess } from 'child_process'
|
||||
import { gitExecFileAsync, wslAwareSpawn } from '../git/runner'
|
||||
import { parseWslPath, toWindowsWslPath } from '../wsl'
|
||||
import { tryDeleteWslUncPath } from '../wsl-unc-delete'
|
||||
import type { Store } from '../persistence'
|
||||
import type {
|
||||
DirEntry,
|
||||
@@ -806,6 +807,14 @@ export function registerFilesystemHandlers(
|
||||
preserveSymlink: true
|
||||
})
|
||||
|
||||
// Why: WSL UNC targets (\\wsl.localhost\<distro>\...) have no Recycle Bin,
|
||||
// so shell.trashItem throws. Hard-delete via `rm` inside the distro instead
|
||||
// (true delete, honors Linux perms). Returns false for normal local paths,
|
||||
// which still go to the Recycle Bin (issue #6415).
|
||||
if (await tryDeleteWslUncPath(targetPath, { recursive: args.recursive })) {
|
||||
return
|
||||
}
|
||||
|
||||
// Why: once auto-refresh exists, an external delete can race with a
|
||||
// UI-initiated delete. Swallowing ENOENT keeps the action idempotent
|
||||
// from the user's perspective (design §7.1).
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { execFileMock } = vi.hoisted(() => ({ execFileMock: vi.fn() }))
|
||||
|
||||
vi.mock('child_process', () => ({ execFile: execFileMock }))
|
||||
|
||||
import { tryDeleteWslUncPath } from './wsl-unc-delete'
|
||||
|
||||
async function withPlatform<T>(platform: NodeJS.Platform, run: () => Promise<T>): Promise<T> {
|
||||
const original = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
|
||||
try {
|
||||
return await run()
|
||||
} finally {
|
||||
if (original) {
|
||||
Object.defineProperty(process, 'platform', original)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('tryDeleteWslUncPath', () => {
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
execFileMock.mockImplementation((_cmd, _args, _opts, cb) => {
|
||||
cb(null, '', '')
|
||||
})
|
||||
})
|
||||
|
||||
it('returns false and never spawns for a normal local path', async () => {
|
||||
await withPlatform('win32', async () => {
|
||||
await expect(tryDeleteWslUncPath('C:\\Users\\me\\repo\\file.txt')).resolves.toBe(false)
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns false off Windows so non-Windows callers keep trashing', async () => {
|
||||
await withPlatform('darwin', async () => {
|
||||
await expect(
|
||||
tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\file.txt')
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('hard-deletes a WSL UNC file via rm inside the distro', async () => {
|
||||
await withPlatform('win32', async () => {
|
||||
await expect(
|
||||
tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\file.txt')
|
||||
).resolves.toBe(true)
|
||||
})
|
||||
|
||||
expect(execFileMock).toHaveBeenCalledTimes(1)
|
||||
const [binary, spawnArgs] = execFileMock.mock.calls[0]
|
||||
expect(binary).toBe('wsl.exe')
|
||||
expect(spawnArgs).toEqual([
|
||||
'-d',
|
||||
'Ubuntu',
|
||||
'--',
|
||||
'rm',
|
||||
'-f',
|
||||
'--',
|
||||
'/home/me/repo/file.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('passes -rf for a recursive directory delete', async () => {
|
||||
await withPlatform('win32', async () => {
|
||||
await tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\dir', {
|
||||
recursive: true
|
||||
})
|
||||
})
|
||||
|
||||
const [, spawnArgs] = execFileMock.mock.calls[0]
|
||||
expect(spawnArgs).toEqual(['-d', 'Ubuntu', '--', 'rm', '-rf', '--', '/home/me/repo/dir'])
|
||||
})
|
||||
|
||||
it('also handles the legacy \\\\wsl$ UNC prefix', async () => {
|
||||
await withPlatform('win32', async () => {
|
||||
await tryDeleteWslUncPath('\\\\wsl$\\Debian\\srv\\app.log')
|
||||
})
|
||||
|
||||
const [, spawnArgs] = execFileMock.mock.calls[0]
|
||||
expect(spawnArgs).toEqual(['-d', 'Debian', '--', 'rm', '-f', '--', '/srv/app.log'])
|
||||
})
|
||||
|
||||
it('surfaces the distro stderr when rm fails', async () => {
|
||||
execFileMock.mockImplementation((_cmd, _args, _opts, cb) => {
|
||||
cb(new Error('Command failed'), '', 'rm: cannot remove: Permission denied')
|
||||
})
|
||||
|
||||
await withPlatform('win32', async () => {
|
||||
await expect(
|
||||
tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\file.txt')
|
||||
).rejects.toThrow('Failed to delete WSL path: rm: cannot remove: Permission denied')
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,61 @@
|
||||
import { execFile } from 'child_process'
|
||||
import { parseWslPath } from './wsl'
|
||||
|
||||
/**
|
||||
* Delete a file/directory that lives on a WSL distro's Linux filesystem,
|
||||
* addressed from Windows via a \\wsl.localhost\<distro>\... (or legacy
|
||||
* \\wsl$\...) UNC path.
|
||||
*
|
||||
* Why: Electron's shell.trashItem() cannot move a WSL UNC item to a Recycle
|
||||
* Bin — the WSL virtual volume has none — so it throws and the delete fails
|
||||
* (issue #6415). For these paths we run `rm` inside the distro via wsl.exe,
|
||||
* which performs a true delete on the Linux fs and honors Linux permissions.
|
||||
*
|
||||
* Returns false when the path is not a WSL UNC path, so callers can fall back
|
||||
* to the normal local-trash behavior (we must never hard-delete a normal local
|
||||
* file that currently goes to the Recycle Bin).
|
||||
*/
|
||||
export async function tryDeleteWslUncPath(
|
||||
targetPath: string,
|
||||
options: { recursive?: boolean } = {}
|
||||
): Promise<boolean> {
|
||||
const info = parseWslPath(targetPath)
|
||||
if (!info) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Why: `rm -f` makes the delete idempotent — a missing file is success,
|
||||
// matching the ENOENT-swallowing semantics of the trash path. `--` stops
|
||||
// flag parsing so a Linux path that starts with `-` can't be read as an
|
||||
// option. `-r` is added only for directory deletes the renderer confirmed.
|
||||
const flags = options.recursive ? '-rf' : '-f'
|
||||
await execFileWsl(info.distro, ['rm', flags, '--', info.linuxPath])
|
||||
return true
|
||||
}
|
||||
|
||||
function execFileWsl(distro: string, command: string[]): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
execFile(
|
||||
'wsl.exe',
|
||||
['-d', distro, '--', ...command],
|
||||
// Why: a generous bound so deleting a large directory tree on the WSL fs
|
||||
// doesn't abort mid-delete, while still capping a wedged wsl.exe.
|
||||
{ encoding: 'utf-8', timeout: 30000 },
|
||||
(error, _stdout, stderr) => {
|
||||
if (error) {
|
||||
reject(wslDeleteError(error, stderr))
|
||||
return
|
||||
}
|
||||
resolve()
|
||||
}
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function wslDeleteError(error: Error, stderr: string): Error {
|
||||
const detail = stderr.trim()
|
||||
if (!detail) {
|
||||
return error
|
||||
}
|
||||
return new Error(`Failed to delete WSL path: ${detail}`)
|
||||
}
|
||||
Reference in New Issue
Block a user