fix: address review findings (#2220)

This commit is contained in:
Jinjing
2026-05-17 22:34:09 -07:00
committed by GitHub
parent 84e9ee964b
commit cfe39bcd8e
12 changed files with 241 additions and 28 deletions
@@ -93,6 +93,24 @@ describe('SshFilesystemProvider', () => {
})
})
describe('getTempDir', () => {
it('reads and caches the remote temp directory from the relay', async () => {
mux.request.mockResolvedValue('/var/folders/remote')
await expect(provider.getTempDir()).resolves.toBe('/var/folders/remote')
await expect(provider.getTempDir()).resolves.toBe('/var/folders/remote')
expect(mux.request).toHaveBeenCalledTimes(1)
expect(mux.request).toHaveBeenCalledWith('fs.tempDir', {})
})
it('falls back to /tmp when connected to an older relay', async () => {
mux.request.mockRejectedValue(Object.assign(new Error('Method not found'), { code: -32601 }))
await expect(provider.getTempDir()).resolves.toBe('/tmp')
})
})
describe('writeFileBase64', () => {
it('writes decoded bytes through SFTP', async () => {
const written: Buffer[] = []
@@ -26,6 +26,7 @@ export class SshFilesystemProvider implements IFilesystemProvider {
// multiplexer. Without this, notification callbacks keep firing after
// the provider is torn down on disconnect, routing events to stale state.
private unsubscribeNotifications: (() => void) | null = null
private tempDirPromise: Promise<string> | null = null
// Why: relays from a previous build may not implement fs.readFileStream.
// We log the fallback once per session at warn level so users on stale
// relays get diagnosed quickly without per-read log spam.
@@ -92,6 +93,20 @@ export class SshFilesystemProvider implements IFilesystemProvider {
}
}
async getTempDir(): Promise<string> {
this.tempDirPromise ??= this.mux.request('fs.tempDir', {}).then(
(result) => result as string,
(err) => {
this.tempDirPromise = null
if (isMethodNotFoundError(err)) {
return '/tmp'
}
throw err
}
)
return this.tempDirPromise
}
async writeFile(filePath: string, content: string): Promise<void> {
await this.mux.request('fs.writeFile', { filePath, content })
}
+1
View File
@@ -121,6 +121,7 @@ export type FileReadResult = {
export type IFilesystemProvider = {
readDir(dirPath: string): Promise<DirEntry[]>
readFile(filePath: string): Promise<FileReadResult>
getTempDir?(): Promise<string>
writeFile(filePath: string, content: string): Promise<void>
writeFileBase64(filePath: string, contentBase64: string): Promise<void>
writeFileBase64Chunk(filePath: string, contentBase64: string, append: boolean): Promise<void>
+103 -3
View File
@@ -1,21 +1,37 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const {
removeHandlerMock,
handleMock,
fsWriteFileMock,
clipboardReadTextMock,
clipboardWriteTextMock,
clipboardReadImageMock,
clipboardWriteImageMock,
nativeImageCreateFromBufferMock
nativeImageCreateFromBufferMock,
randomUUIDMock,
getSshFilesystemProviderMock
} = vi.hoisted(() => ({
removeHandlerMock: vi.fn(),
handleMock: vi.fn(),
fsWriteFileMock: vi.fn(),
clipboardReadTextMock: vi.fn(),
clipboardWriteTextMock: vi.fn(),
clipboardReadImageMock: vi.fn(),
clipboardWriteImageMock: vi.fn(),
nativeImageCreateFromBufferMock: vi.fn()
nativeImageCreateFromBufferMock: vi.fn(),
randomUUIDMock: vi.fn(() => '00000000-0000-4000-8000-000000000000'),
getSshFilesystemProviderMock: vi.fn()
}))
vi.mock('node:fs/promises', () => ({
default: {
writeFile: fsWriteFileMock
}
}))
vi.mock('node:crypto', () => ({
randomUUID: randomUUIDMock
}))
vi.mock('electron', () => ({
@@ -37,6 +53,10 @@ vi.mock('electron', () => ({
}
}))
vi.mock('../providers/ssh-filesystem-dispatch', () => ({
getSshFilesystemProvider: getSshFilesystemProviderMock
}))
import { registerClipboardHandlers } from './clipboard-ipc-handlers'
function getRegisteredHandlers(): Map<string, (...args: unknown[]) => unknown> {
@@ -52,13 +72,22 @@ function getRegisteredHandlers(): Map<string, (...args: unknown[]) => unknown> {
describe('registerClipboardHandlers', () => {
beforeEach(() => {
vi.spyOn(Date, 'now').mockReturnValue(1760000000000)
removeHandlerMock.mockReset()
handleMock.mockReset()
fsWriteFileMock.mockReset()
clipboardReadTextMock.mockReset()
clipboardWriteTextMock.mockReset()
clipboardReadImageMock.mockReset()
clipboardWriteImageMock.mockReset()
nativeImageCreateFromBufferMock.mockReset()
randomUUIDMock.mockReset()
randomUUIDMock.mockReturnValue('00000000-0000-4000-8000-000000000000')
getSshFilesystemProviderMock.mockReset()
})
afterEach(() => {
vi.restoreAllMocks()
})
it('registers normal and selection text clipboard IPC handlers', () => {
@@ -90,4 +119,75 @@ describe('registerClipboardHandlers', () => {
expect(removeHandlerMock).toHaveBeenCalledWith('clipboard:writeImage')
expect(removeHandlerMock).toHaveBeenCalledWith('clipboard:saveImageAsTempFile')
})
it('saves clipboard images to a local temp file when no connection is provided', async () => {
const png = Buffer.from([0, 1, 2, 3])
clipboardReadImageMock.mockReturnValue({
isEmpty: () => false,
toPNG: () => png
})
registerClipboardHandlers()
const handlers = getRegisteredHandlers()
await expect(handlers.get('clipboard:saveImageAsTempFile')?.({}, undefined)).resolves.toBe(
'/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png'
)
expect(fsWriteFileMock).toHaveBeenCalledWith(
'/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png',
png
)
expect(getSshFilesystemProviderMock).not.toHaveBeenCalled()
})
it('uploads clipboard images to the SSH host when a connection is provided', async () => {
const png = Buffer.from([0, 1, 2, 3])
const writeFileBase64 = vi.fn().mockResolvedValue(undefined)
const getTempDir = vi.fn().mockResolvedValue('/var/tmp')
clipboardReadImageMock.mockReturnValue({
isEmpty: () => false,
toPNG: () => png
})
getSshFilesystemProviderMock.mockReturnValue({ getTempDir, writeFileBase64 })
registerClipboardHandlers()
const handlers = getRegisteredHandlers()
await expect(
handlers.get('clipboard:saveImageAsTempFile')?.({}, { connectionId: 'ssh-1' })
).resolves.toBe('/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png')
expect(getSshFilesystemProviderMock).toHaveBeenCalledWith('ssh-1')
expect(getTempDir).toHaveBeenCalled()
expect(writeFileBase64).toHaveBeenCalledWith(
'/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png',
png.toString('base64')
)
expect(fsWriteFileMock).not.toHaveBeenCalled()
})
it('uses Windows path joining for Windows SSH temp directories', async () => {
const png = Buffer.from([0, 1, 2, 3])
const writeFileBase64 = vi.fn().mockResolvedValue(undefined)
clipboardReadImageMock.mockReturnValue({
isEmpty: () => false,
toPNG: () => png
})
getSshFilesystemProviderMock.mockReturnValue({
getTempDir: vi.fn().mockResolvedValue('C:\\Users\\alice\\AppData\\Local\\Temp'),
writeFileBase64
})
registerClipboardHandlers()
const handlers = getRegisteredHandlers()
await expect(
handlers.get('clipboard:saveImageAsTempFile')?.({}, { connectionId: 'ssh-1' })
).resolves.toBe(
'C:\\Users\\alice\\AppData\\Local\\Temp\\orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png'
)
expect(writeFileBase64).toHaveBeenCalledWith(
'C:\\Users\\alice\\AppData\\Local\\Temp\\orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png',
png.toString('base64')
)
})
})
+49 -8
View File
@@ -1,7 +1,47 @@
import fs from 'node:fs/promises'
import path from 'node:path'
import { randomUUID } from 'node:crypto'
import { app, clipboard, ipcMain, nativeImage } from 'electron'
import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch'
import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path'
type SaveClipboardImageAsTempFileArgs = {
connectionId?: string | null
}
const REMOTE_CLIPBOARD_IMAGE_TEMP_DIR = '/tmp'
function joinRemotePath(basePath: string, fileName: string): string {
if (isWindowsAbsolutePathLike(basePath)) {
return path.win32.join(basePath, fileName)
}
return path.posix.join(basePath, fileName)
}
async function saveClipboardImageBufferAsTempFile(
buffer: Buffer,
args?: SaveClipboardImageAsTempFileArgs
): Promise<string> {
const fileName = `orca-paste-${Date.now()}-${randomUUID()}.png`
if (args?.connectionId) {
const provider = getSshFilesystemProvider(args.connectionId)
if (!provider) {
throw new Error(`No filesystem provider for connection "${args.connectionId}"`)
}
const remoteTempDir = (await provider.getTempDir?.()) ?? REMOTE_CLIPBOARD_IMAGE_TEMP_DIR
const remotePath = joinRemotePath(remoteTempDir, fileName)
// Why: SSH terminal agents run on the remote host, so the pasted path must
// name a remote file. The provider's base64 path writes binary bytes via SFTP.
await provider.writeFileBase64(remotePath, buffer.toString('base64'))
return remotePath
}
const tempPath = path.join(app.getPath('temp'), fileName)
await fs.writeFile(tempPath, buffer)
return tempPath
}
export function registerClipboardHandlers(): void {
ipcMain.removeHandler('clipboard:readText')
@@ -16,15 +56,16 @@ export function registerClipboardHandlers(): void {
// Why: terminals need to detect clipboard images to support tools like Claude
// Code that accept image input via paste. Writes the clipboard image to a
// temp file and returns the path, or null if the clipboard has no image.
ipcMain.handle('clipboard:saveImageAsTempFile', async () => {
const image = clipboard.readImage()
if (image.isEmpty()) {
return null
ipcMain.handle(
'clipboard:saveImageAsTempFile',
async (_event, args?: SaveClipboardImageAsTempFileArgs) => {
const image = clipboard.readImage()
if (image.isEmpty()) {
return null
}
return saveClipboardImageBufferAsTempFile(image.toPNG(), args)
}
const tempPath = path.join(app.getPath('temp'), `orca-paste-${Date.now()}.png`)
await fs.writeFile(tempPath, image.toPNG())
return tempPath
})
)
ipcMain.handle('clipboard:writeText', (_event, text: string) => clipboard.writeText(text))
ipcMain.handle('clipboard:writeSelectionText', (_event, text: string) =>
clipboard.writeText(text, 'selection')
+3 -1
View File
@@ -1660,7 +1660,9 @@ export type PreloadApi = {
onTerminalZoom: (callback: (direction: 'in' | 'out' | 'reset') => void) => () => void
readClipboardText: () => Promise<string>
readSelectionClipboardText: () => Promise<string>
saveClipboardImageAsTempFile: () => Promise<string | null>
saveClipboardImageAsTempFile: (args?: {
connectionId?: string | null
}) => Promise<string | null>
writeClipboardText: (text: string) => Promise<void>
writeSelectionClipboardText: (text: string) => Promise<void>
writeClipboardImage: (dataUrl: string) => Promise<void>
+3 -2
View File
@@ -2471,8 +2471,9 @@ const api = {
readClipboardText: (): Promise<string> => ipcRenderer.invoke('clipboard:readText'),
readSelectionClipboardText: (): Promise<string> =>
ipcRenderer.invoke('clipboard:readSelectionText'),
saveClipboardImageAsTempFile: (): Promise<string | null> =>
ipcRenderer.invoke('clipboard:saveImageAsTempFile'),
saveClipboardImageAsTempFile: (args?: {
connectionId?: string | null
}): Promise<string | null> => ipcRenderer.invoke('clipboard:saveImageAsTempFile', args),
writeClipboardText: (text: string): Promise<void> =>
ipcRenderer.invoke('clipboard:writeText', text),
writeSelectionClipboardText: (text: string): Promise<void> =>
+5
View File
@@ -124,6 +124,7 @@ describe('FsHandler', () => {
const methods = Array.from(dispatcher._requestHandlers.keys())
expect(methods).toContain('fs.readDir')
expect(methods).toContain('fs.readFile')
expect(methods).toContain('fs.tempDir')
expect(methods).toContain('fs.writeFile')
expect(methods).toContain('fs.stat')
expect(methods).toContain('fs.deletePath')
@@ -142,6 +143,10 @@ describe('FsHandler', () => {
expect(notifMethods).toContain('fs.unwatch')
})
it('tempDir returns the relay host temp directory', async () => {
await expect(dispatcher.callRequest('fs.tempDir')).resolves.toBe(tmpdir())
})
it('readDir returns sorted entries with directories first', async () => {
mkdirSync(path.join(tmpDir, 'subdir'))
writeFileSync(path.join(tmpDir, 'file.txt'), 'hello')
+6
View File
@@ -2,6 +2,7 @@
path expansion, file IO, search, streaming reads, Space scans, and watch lifecycle state. */
import { readdir, writeFile, stat, lstat, mkdir, rename, cp, rm, realpath } from 'fs/promises'
import { execFile } from 'child_process'
import { tmpdir } from 'os'
import { join } from 'path'
import type { RelayDispatcher, RequestContext } from './dispatcher'
import type { RelayContext } from './context'
@@ -64,6 +65,7 @@ export class FsHandler {
this.dispatcher.onRequest('fs.readDir', (p) => this.readDir(p))
this.dispatcher.onRequest('fs.readFile', (p) => this.readFile(p))
this.dispatcher.onRequest('fs.readFileStream', (p, c) => this.readFileStream(p, c))
this.dispatcher.onRequest('fs.tempDir', () => this.tempDir())
this.dispatcher.onRequest('fs.writeFile', (p) => this.writeFile(p))
this.dispatcher.onRequest('fs.stat', (p) => this.stat(p))
this.dispatcher.onRequest('fs.deletePath', (p) => this.deletePath(p))
@@ -110,6 +112,10 @@ export class FsHandler {
return readRelayFileStreamMetadata(filePath, this.dispatcher, this.streamRegistry, ctx)
}
private async tempDir(): Promise<string> {
return tmpdir()
}
private cancelStream(params: Record<string, unknown>): void {
const streamId = params.streamId as number | undefined
if (typeof streamId === 'number') {
@@ -83,6 +83,11 @@ type TerminalPaneProps = {
onCloseTab: () => void
}
function formatClipboardImagePasteError(error: unknown): string {
const detail = error instanceof Error ? error.message : String(error)
return `Image paste failed: ${detail}`
}
export default function TerminalPane({
tabId,
worktreeId,
@@ -896,12 +901,18 @@ export default function TerminalPane({
// Why: clipboard has no text — check for an image. This is the
// image-only clipboard case (e.g. screenshot) where Chromium's paste
// event would never fire on a textarea. We save the image to a temp
// file and paste the path so the terminal process can access it.
return window.api.ui.saveClipboardImageAsTempFile().then((filePath) => {
if (filePath) {
pane.terminal.paste(filePath)
}
})
// file owned by the terminal host and paste that path.
const connectionId = getConnectionId(worktreeId) ?? null
return window.api.ui
.saveClipboardImageAsTempFile({ connectionId })
.then((filePath) => {
if (filePath) {
pane.terminal.paste(filePath)
}
})
.catch((error: unknown) => {
setTerminalError(formatClipboardImagePasteError(error))
})
})
.catch(() => {
/* ignore clipboard failures */
@@ -965,7 +976,7 @@ export default function TerminalPane({
container.removeEventListener('keydown', onKeyPaste, { capture: true })
container.removeEventListener('paste', onPaste, { capture: true })
}
}, [isActive])
}, [isActive, worktreeId])
// Why: a click inside the terminal container is a deliberate interaction
// with the pane — dismiss the bell indicator for this tab and worktree
@@ -1166,10 +1177,12 @@ export default function TerminalPane({
managerRef,
paneTransportsRef,
paneCwdRef,
worktreeId,
fallbackCwd: cwd ?? '',
toggleExpandPane,
onRequestClosePane: handleRequestClosePane,
onSetTitle: handleStartRename,
onPasteError: setTerminalError,
rightClickToPaste
})
@@ -1,6 +1,7 @@
import { useEffect, useRef, useState } from 'react'
import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager'
import type { PtyTransport } from './pty-transport'
import { getConnectionId } from '@/lib/connection-context'
import { resolveSplitCwd, type PaneCwdMap } from './resolve-split-cwd'
import type { TerminalQuickCommand } from '../../../../shared/types'
import { sendTerminalQuickCommandToPane } from './terminal-quick-command-dispatch'
@@ -12,10 +13,12 @@ type UseTerminalPaneContextMenuDeps = {
managerRef: React.RefObject<PaneManager | null>
paneTransportsRef: React.RefObject<Map<number, PtyTransport>>
paneCwdRef: React.RefObject<PaneCwdMap>
worktreeId: string
fallbackCwd: string
toggleExpandPane: (paneId: number) => void
onRequestClosePane: (paneId: number) => void
onSetTitle: (paneId: number) => void
onPasteError: (message: string) => void
rightClickToPaste: boolean
}
@@ -43,10 +46,12 @@ export function useTerminalPaneContextMenu({
managerRef,
paneTransportsRef,
paneCwdRef,
worktreeId,
fallbackCwd,
toggleExpandPane,
onRequestClosePane,
onSetTitle,
onPasteError,
rightClickToPaste
}: UseTerminalPaneContextMenuDeps): TerminalMenuState {
const contextPaneIdRef = useRef<number | null>(null)
@@ -107,12 +112,17 @@ export function useTerminalPaneContextMenu({
pane.terminal.focus()
return
}
// Why: clipboard has no text — check for an image (e.g. screenshot).
// Saves the image to a temp file and pastes the path so CLI tools like
// Claude Code can access it, consistent with the keyboard paste path.
const filePath = await window.api.ui.saveClipboardImageAsTempFile()
if (filePath) {
pane.terminal.paste(filePath)
// Why: clipboard has no text — check for an image (e.g. screenshot) and
// save it on the same host as this terminal before pasting the file path.
try {
const connectionId = getConnectionId(worktreeId) ?? null
const filePath = await window.api.ui.saveClipboardImageAsTempFile({ connectionId })
if (filePath) {
pane.terminal.paste(filePath)
}
} catch (error) {
const detail = error instanceof Error ? error.message : String(error)
onPasteError(`Image paste failed: ${detail}`)
}
// Why: Radix returns focus to the menu trigger (the pane container) on
// close, but xterm.js only accepts input when its own helper textarea is
+2 -1
View File
@@ -779,7 +779,8 @@ function createWebUiApi(): NonNullable<Partial<PreloadApi>['ui']> {
readClipboardText: () => navigator.clipboard?.readText?.() ?? Promise.resolve(''),
readSelectionClipboardText: () =>
Promise.reject(new Error('Selection clipboard is unavailable in the web client')),
saveClipboardImageAsTempFile: () => Promise.resolve(null),
saveClipboardImageAsTempFile: (_args?: { connectionId?: string | null }) =>
Promise.resolve(null),
writeClipboardText: (text) => navigator.clipboard?.writeText?.(text) ?? Promise.resolve(),
writeSelectionClipboardText: () =>
Promise.reject(new Error('Selection clipboard is unavailable in the web client')),