fix(composer): name the attachments a drop could not add, in one toast

This commit is contained in:
Jinjing
2026-09-14 13:31:47 -07:00
parent fdcb903f30
commit 0077f83256
12 changed files with 444 additions and 29 deletions
@@ -0,0 +1,87 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { toastError } = vi.hoisted(() => ({ toastError: vi.fn() }))
vi.mock('sonner', () => ({ toast: { error: toastError } }))
import { showComposerDropFailureToast } from './composer-drop-failure-toast'
import type { ComposerDropSkipReason } from './composer-drop-upload-result'
// Why a Record: a fifth ComposerDropSkipReason fails to compile here, the way it does in the
// copy switch — a plain annotated array would leave this test green.
const SKIP_REASON_COPY: Record<ComposerDropSkipReason, string> = {
missing: 'No longer at its original path.',
symlink: 'Symbolic links cannot be attached.',
'permission-denied': 'Permission denied.',
unsupported: 'Unsupported file type.'
}
function lastToast(): { title: string; description?: string } {
const call = toastError.mock.calls.at(-1)
return {
title: String(call?.[0]),
description: (call?.[1] as { description?: string })?.description
}
}
describe('showComposerDropFailureToast', () => {
beforeEach(() => {
toastError.mockClear()
})
it('stays neutral about the gesture, and pluralises like its namespace siblings', () => {
showComposerDropFailureToast({ skippedOrFailed: 1, total: 1 })
expect(lastToast().title).toBe('1 of 1 item could not be attached.')
showComposerDropFailureToast({ skippedOrFailed: 2, total: 5 })
expect(lastToast().title).toBe('2 of 5 items could not be attached.')
})
it("turns the import client's skip enum into copy instead of leaking the token", () => {
const seen = new Map<string, string | undefined>()
for (const reason of Object.keys(SKIP_REASON_COPY) as ComposerDropSkipReason[]) {
showComposerDropFailureToast({
skippedOrFailed: 1,
total: 3,
uniformFailure: { status: 'skipped', reason }
})
seen.set(reason, lastToast().description)
}
expect(Object.fromEntries(seen)).toEqual(SKIP_REASON_COPY)
})
it('passes a free-form failure reason straight through', () => {
showComposerDropFailureToast({
skippedOrFailed: 2,
total: 4,
uniformFailure: { status: 'failed', reason: 'EACCES: permission denied' }
})
expect(lastToast().description).toBe('EACCES: permission denied')
})
it('shows no description when nothing explained the failure', () => {
showComposerDropFailureToast({ skippedOrFailed: 1, total: 2 })
expect(lastToast().description).toBeUndefined()
})
it('unwraps and clamps a host-minted failure reason before it reaches the row', () => {
// Why: this reason is free text from the execution host — it can arrive IPC-wrapped and
// multi-line, and a toast description is a single compact row.
showComposerDropFailureToast({
skippedOrFailed: 1,
total: 2,
uniformFailure: {
status: 'failed',
reason:
"Error invoking remote method 'runtime:call': Error: EACCES: permission denied\nat Object.upload"
}
})
expect(lastToast().description).toBe('EACCES: permission denied')
})
it('gives no reason at all when the batch failed for differing reasons', () => {
// Why: a description under an aggregate count reads as the explanation for the whole count.
showComposerDropFailureToast({ skippedOrFailed: 3, total: 6 })
expect(lastToast().title).toBe('3 of 6 items could not be attached.')
expect(lastToast().description).toBeUndefined()
})
})
@@ -0,0 +1,62 @@
import { toast } from 'sonner'
import { translate } from '@/i18n/i18n'
import { readIpcErrorMessage } from '@/lib/ipc-error'
import type { ComposerDropFailure } from './composer-drop-upload-result'
/**
* Copy for one drop failure. The skip arm has no `default:` on purpose — a new member of the closed
* enum must fail the build here rather than reach a user as a raw token. An unrecognised reason from
* a newer host falls off the end and degrades to no detail, which is the right behaviour per
* `docs/reference/remote-wire-compatibility.md`.
*/
function skipReasonText(failure: ComposerDropFailure): string | undefined {
if (failure.status === 'failed') {
// Why route free text through the IPC reader: this reason is minted host-side, can arrive
// wrapped by `ipcRenderer.invoke`, and can be multi-line — a toast description is one row.
return failure.reason ? readIpcErrorMessage(new Error(failure.reason)) : undefined
}
switch (failure.reason) {
case undefined:
return undefined
case 'missing':
return translate(
'auto.hooks.useComposerState.attachSkipMissing',
'No longer at its original path.'
)
case 'symlink':
return translate(
'auto.hooks.useComposerState.attachSkipSymlink',
'Symbolic links cannot be attached.'
)
case 'permission-denied':
return translate(
'auto.hooks.useComposerState.attachSkipPermissionDenied',
'Permission denied.'
)
case 'unsupported':
return translate(
'auto.hooks.useComposerState.attachSkipUnsupported',
'Unsupported file type.'
)
}
}
/** Reports one gesture-neutral summary for a partially applied attachment batch. */
export function showComposerDropFailureToast({
skippedOrFailed,
total,
uniformFailure
}: {
skippedOrFailed: number
total: number
uniformFailure?: ComposerDropFailure
}): void {
toast.error(
translate(
'auto.hooks.useComposerState.dropPartiallyAttached',
'{{value0}} of {{value1}} item{{value2}} could not be attached.',
{ value0: skippedOrFailed, value1: total, value2: total === 1 ? '' : 's' }
),
{ description: uniformFailure ? skipReasonText(uniformFailure) : undefined }
)
}
@@ -10,17 +10,27 @@ describe('composer drop upload result', () => {
const results: ComposerDropUploadImportResult[] = [
{ status: 'imported', kind: 'file', destPath: '/repo/.orca/drops/file.txt' },
{ status: 'imported', kind: 'directory', destPath: '/repo/.orca/drops/folder' },
{ status: 'skipped' },
{ status: 'failed' }
{ status: 'skipped', reason: 'permission-denied' },
{ status: 'failed', reason: 'disk full' }
]
expect(collectComposerDropUploadResult(results)).toEqual({
filePaths: ['/repo/.orca/drops/file.txt'],
folderPaths: ['/repo/.orca/drops/folder'],
skippedOrFailed: 2
skippedOrFailed: 2,
// Why undefined: the two non-imported entries disagree, so no single reason explains the count.
uniformFailure: undefined
})
})
it('reports no first failure when every path imported', () => {
expect(
collectComposerDropUploadResult([
{ status: 'imported', kind: 'file', destPath: '/repo/.orca/drops/file.txt' }
]).uniformFailure
).toBeUndefined()
})
it('suppresses failed-upload reporting after a composer loses drop ownership', () => {
const uploadResult = { skippedOrFailed: 1 }
@@ -1,29 +1,62 @@
/**
* Hand-copied from `ImportSkipReason` (`src/main/ipc/filesystem-import-result-types.ts`); the
* renderer cannot import that path under `config/tsconfig.web.json`. Nothing type-links the two, so
* adding a member there will NOT fail the build here — an unrecognised reason simply renders no
* detail, which is the correct degradation per `docs/reference/remote-wire-compatibility.md`.
*/
export type ComposerDropSkipReason = 'missing' | 'symlink' | 'permission-denied' | 'unsupported'
export type ComposerDropUploadImportResult =
| {
status: 'imported'
destPath: string
kind: 'file' | 'directory'
}
// Why split: the runtime import client reports a closed enum for a skip and free text for a
// failure. Collapsing them would erase the union and let an unmapped token reach the UI.
| {
status: 'skipped' | 'failed'
status: 'skipped'
reason: ComposerDropSkipReason
}
| {
status: 'failed'
reason?: string
}
export type ComposerDropUploadResult = {
filePaths: string[]
folderPaths: string[]
skippedOrFailed: number
/**
* Set only when EVERY non-imported entry agrees on status and reason. A description sitting under
* an aggregate count reads as the explanation for all of it, so a mixed batch gets no reason
* rather than one item's reason presented as the whole story.
*/
uniformFailure?: ComposerDropFailure
}
export type ComposerDropFailure = Exclude<ComposerDropUploadImportResult, { status: 'imported' }>
export function collectComposerDropUploadResult(
results: readonly ComposerDropUploadImportResult[]
): ComposerDropUploadResult {
const filePaths: string[] = []
const folderPaths: string[] = []
let skippedOrFailed = 0
let uniformFailure: ComposerDropFailure | undefined
let failureVaries = false
for (const result of results) {
if (result.status !== 'imported') {
skippedOrFailed += 1
if (!uniformFailure) {
uniformFailure = result
} else if (
uniformFailure.status !== result.status ||
uniformFailure.reason !== result.reason
) {
failureVaries = true
}
continue
}
if (result.kind === 'directory') {
@@ -33,7 +66,12 @@ export function collectComposerDropUploadResult(
}
}
return { filePaths, folderPaths, skippedOrFailed }
return {
filePaths,
folderPaths,
skippedOrFailed,
uniformFailure: failureVaries ? undefined : uniformFailure
}
}
export function shouldReportComposerDropUploadFailure(
@@ -0,0 +1,194 @@
// @vitest-environment happy-dom
import { act, renderHook } from '@testing-library/react'
import { createRef } from 'react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({ toastError: vi.fn(), importExternalPaths: vi.fn() }))
vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } }))
vi.mock('@/store', () => ({
useAppStore: Object.assign(() => undefined, { getState: () => ({}) })
}))
vi.mock('@/runtime/runtime-file-client', () => ({
importExternalPathsToRuntime: (...args: unknown[]) => mocks.importExternalPaths(...args)
}))
vi.mock('./composer-drop-listener', () => ({ useComposerDropListener: vi.fn() }))
import { useAttachmentDropState } from './attachment-drop-state'
const FAILING_PATHS = new Set(['/drop/bad-1.png', '/drop/bad-2.png', '/drop/bad-3.png'])
function dropPaths(count: number): string[] {
return [
...FAILING_PATHS,
...Array.from({ length: count - FAILING_PATHS.size }, (_, index) => `/drop/ok-${index}.png`)
]
}
function installFsApi(): void {
Object.assign(window, {
api: {
fs: {
authorizeExternalPath: vi.fn(async () => {}),
stat: vi.fn(async ({ filePath }: { filePath: string }) => {
if (FAILING_PATHS.has(filePath)) {
throw new Error(
"Error invoking remote method 'fs:stat': Error: ENOENT: no such file or directory"
)
}
return { isDirectory: false }
})
}
}
})
}
function renderDropState(setAttachmentPaths: (updater: (current: string[]) => string[]) => void) {
return renderHook(() =>
useAttachmentDropState({
agentPromptRef: createRef<string>() as never,
cancelPromptCaretFrame: () => {},
connectionId: null,
promptCaretFrameRef: { current: null },
promptTextareaRef: createRef<HTMLTextAreaElement>(),
selectedRepoPath: '/repo',
selectedRepoSettings: null,
setAgentPrompt: () => {},
setAttachmentPaths: setAttachmentPaths as never
})
)
}
describe('local composer drop failures', () => {
beforeEach(() => {
vi.clearAllMocks()
installFsApi()
})
it('reports partially skipped paths in ONE aggregated toast and still attaches the rest', async () => {
const attached: string[] = []
const { result } = renderDropState((updater) => {
attached.push(...updater([]))
})
await act(async () => {
await result.current.applyLocalComposerDrop(dropPaths(12))
})
expect(mocks.toastError).toHaveBeenCalledTimes(1)
const [title, options] = mocks.toastError.mock.calls[0] as [string, { description?: string }]
expect(title).toBe('3 of 12 items could not be attached.')
// Why the translated copy: a local drop classifies ENOENT as a skip, so the user gets the
// sentence rather than the errno. All three failures agree, so the reason explains the count.
expect(options.description).toBe('No longer at its original path.')
expect(attached).toHaveLength(9)
expect(attached).not.toContain('/drop/bad-1.png')
})
it('stays silent when every dropped path attaches', async () => {
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.applyLocalComposerDrop(['/drop/ok-0.png', '/drop/ok-1.png'])
})
expect(mocks.toastError).not.toHaveBeenCalled()
})
it('says nothing once the composer that owned the drop is gone', async () => {
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.applyLocalComposerDrop(dropPaths(12), () => false)
})
expect(mocks.toastError).not.toHaveBeenCalled()
})
})
// Why: the upload branch returns early unless a runtime environment or connection is resolved.
const RUNTIME_SETTINGS = { activeRuntimeEnvironmentId: 'env-1' } as never
describe('composer upload failures', () => {
beforeEach(() => {
vi.clearAllMocks()
installFsApi()
})
it('aggregates a mixed runtime import into ONE toast, and withholds a reason that is not shared', async () => {
mocks.importExternalPaths.mockResolvedValue({
results: [
{
sourcePath: '/a.png',
status: 'imported',
destPath: '/repo/.orca/drops/a.png',
kind: 'file',
renamed: false
},
{ sourcePath: '/b.png', status: 'skipped', reason: 'permission-denied' },
{ sourcePath: '/c.png', status: 'failed', reason: 'disk full' }
]
})
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.uploadComposerPaths(
['/a.png', '/b.png', '/c.png'],
RUNTIME_SETTINGS,
null,
'/repo'
)
})
expect(mocks.toastError).toHaveBeenCalledTimes(1)
const [title, options] = mocks.toastError.mock.calls[0] as [string, { description?: string }]
expect(title).toBe('2 of 3 items could not be attached.')
// Why no description: one was skipped for permission, the other failed for disk space — a
// single reason printed under the count would read as the explanation for both.
expect(options.description).toBeUndefined()
})
it('stays silent when every uploaded path imports', async () => {
mocks.importExternalPaths.mockResolvedValue({
results: [
{
sourcePath: '/a.png',
status: 'imported',
destPath: '/repo/.orca/drops/a.png',
kind: 'file',
renamed: false
}
]
})
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.uploadComposerPaths(['/a.png'], RUNTIME_SETTINGS, null, '/repo')
})
expect(mocks.toastError).not.toHaveBeenCalled()
})
it('does give the shared reason when every uploaded path failed the same way', async () => {
mocks.importExternalPaths.mockResolvedValue({
results: [
{ sourcePath: '/b.png', status: 'skipped', reason: 'permission-denied' },
{ sourcePath: '/c.png', status: 'skipped', reason: 'permission-denied' }
]
})
const { result } = renderDropState(() => {})
await act(async () => {
await result.current.uploadComposerPaths(
['/b.png', '/c.png'],
RUNTIME_SETTINGS,
null,
'/repo'
)
})
const [, options] = mocks.toastError.mock.calls[0] as [string, { description?: string }]
expect(options.description).toBe('Permission denied.')
})
})
@@ -20,13 +20,29 @@ import { joinPath } from '@/lib/path'
import { captureDirectSshMutationExpectation } from '@/lib/ssh-mutation-expectation'
import { useAppStore } from '@/store'
import { importExternalPathsToRuntime } from '@/runtime/runtime-file-client'
import { readIpcErrorMessage } from '@/lib/ipc-error'
import { showComposerDropFailureToast } from '../composer-drop-failure-toast'
import {
collectComposerDropUploadResult,
shouldReportComposerDropUploadFailure
shouldReportComposerDropUploadFailure,
type ComposerDropUploadImportResult
} from '../composer-drop-upload-result'
import { applyComposerNativeFileDrop } from '../composer-native-file-drop'
import { useComposerDropListener } from './composer-drop-listener'
// Why map errno here: a local drop never reaches the runtime importer that classifies skips, so
// without this the translated "no longer at its original path" copy would be unreachable for anyone
// on a plain local workspace.
function localDropFailure(detail: string | undefined): ComposerDropUploadImportResult {
if (detail?.startsWith('ENOENT')) {
return { status: 'skipped', reason: 'missing' }
}
if (/^(EACCES|EPERM)/.test(detail ?? '')) {
return { status: 'skipped', reason: 'permission-denied' }
}
return { status: 'failed', reason: detail }
}
export function useAttachmentDropState(input: AttachmentDropStateInput) {
const {
agentPromptRef,
@@ -166,12 +182,11 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
)
const uploadResult = collectComposerDropUploadResult(results)
if (shouldReportComposerDropUploadFailure(uploadResult, canReportFailure)) {
toast.error(
translate(
'auto.hooks.useComposerState.a9ff236145',
'Some attachments could not be uploaded.'
)
)
showComposerDropFailureToast({
skippedOrFailed: uploadResult.skippedOrFailed,
total: sourcePaths.length,
uniformFailure: uploadResult.uniformFailure
})
}
return { filePaths: uploadResult.filePaths, folderPaths: uploadResult.folderPaths }
},
@@ -199,27 +214,37 @@ export function useAttachmentDropState(input: AttachmentDropStateInput) {
const applyLocalComposerDrop = useCallback(
async (paths: string[], canApply: () => boolean = () => true): Promise<void> => {
const fileAttachments: string[] = []
const folderPaths: string[] = []
const results: ComposerDropUploadImportResult[] = []
for (const filePath of paths) {
try {
await window.api.fs.authorizeExternalPath({ targetPath: filePath })
const stat = await window.api.fs.stat({ filePath })
if (stat.isDirectory) {
folderPaths.push(filePath)
} else {
fileAttachments.push(filePath)
}
} catch {
// Skip paths we cannot authorize or stat.
results.push({
status: 'imported',
destPath: filePath,
kind: stat.isDirectory ? 'directory' : 'file'
})
} catch (error) {
// Why classify here: these are the only producers on a local workspace, so without this
// the translated skip copy would be unreachable for anyone without a runtime host.
results.push(localDropFailure(readIpcErrorMessage(error)))
}
}
if (!canApply()) {
return
}
addComposerAttachments(fileAttachments)
insertComposerFolderPaths(folderPaths)
const dropResult = collectComposerDropUploadResult(results)
addComposerAttachments(dropResult.filePaths)
insertComposerFolderPaths(dropResult.folderPaths)
// Why: the drop-ownership gate already ran above, so only the count is left to check.
if (dropResult.skippedOrFailed > 0) {
showComposerDropFailureToast({
skippedOrFailed: dropResult.skippedOrFailed,
total: paths.length,
uniformFailure: dropResult.uniformFailure
})
}
},
[addComposerAttachments, insertComposerFolderPaths]
)
+5 -1
View File
@@ -997,7 +997,11 @@
"folderWorkspaceCreateFailedMessage": "The folder workspace could not be created. Check the error details above, then try again.",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base.",
"a9ff236145": "Some attachments could not be uploaded."
"dropPartiallyAttached": "{{value0}} of {{value1}} item{{value2}} could not be attached.",
"attachSkipMissing": "No longer at its original path.",
"attachSkipSymlink": "Symbolic links cannot be attached.",
"attachSkipPermissionDenied": "Permission denied.",
"attachSkipUnsupported": "Unsupported file type."
},
"useGlobalFileDrop": {
"38c9f034ff": "Failed to upload dropped files.",
-1
View File
@@ -713,7 +713,6 @@
"useComposerState": {
"7eb3f44ff7": "El agente seleccionado está deshabilitado. Elige un agente habilitado antes de crear.",
"b2ead86962": "No se pudo resolver la base del PR.",
"a9ff236145": "Algunos archivos adjuntos no se pudieron cargar.",
"3db83fc58a": "No hay ninguna ruta de proyecto remoto disponible para los archivos adjuntos.",
"ba6cb77082": "No se pudo conectar al proyecto.",
"chooseOrAddProjectBeforeWorkspace": "Elige o agrega un proyecto antes de crear un espacio de trabajo.",
-1
View File
@@ -835,7 +835,6 @@
"useComposerState": {
"7eb3f44ff7": "L'agent sélectionné est désactivé. Choisissez un agent activé avant de créer.",
"b2ead86962": "Échec de la résolution de la base de la PR.",
"a9ff236145": "Certaines pièces jointes n'ont pas pu être envoyées.",
"3db83fc58a": "Aucun chemin de projet n'est disponible sur cet hôte pour les pièces jointes.",
"ba6cb77082": "Échec de la connexion au projet.",
"chooseOrAddProjectBeforeWorkspace": "Choisissez ou ajoutez un projet avant de créer un espace de travail.",
-1
View File
@@ -713,7 +713,6 @@
"useComposerState": {
"7eb3f44ff7": "選択した Agent は無効です。作成する前に、有効な Agent を選択してください。",
"b2ead86962": "PR ベースを解決できませんでした。",
"a9ff236145": "一部の添付ファイルをアップロードできませんでした。",
"3db83fc58a": "このホスト上に、添付に使用できるプロジェクトパスがありません。",
"ba6cb77082": "プロジェクトへの接続に失敗しました。",
"chooseOrAddProjectBeforeWorkspace": "ワークスペースを作成する前に、プロジェクトを選択または追加してください。",
-1
View File
@@ -716,7 +716,6 @@
"useComposerState": {
"7eb3f44ff7": "선택한 agent가 비활성화되었습니다. 생성하기 전에 활성화된 agent를 선택하세요.",
"b2ead86962": "PR 기반을 해결하지 못했습니다.",
"a9ff236145": "일부 첨부파일을 업로드할 수 없습니다.",
"3db83fc58a": "이 호스트에 첨부 파일에 사용할 수 있는 프로젝트 경로가 없습니다.",
"ba6cb77082": "프로젝트에 연결하지 못했습니다.",
"chooseOrAddProjectBeforeWorkspace": "워크스페이스를 만들기 전에 프로젝트를 선택하거나 추가하세요.",
-1
View File
@@ -716,7 +716,6 @@
"useComposerState": {
"7eb3f44ff7": "所选智能体已禁用。创建之前选择启用的智能体。",
"b2ead86962": "无法解析 PR 基础引用。",
"a9ff236145": "部分附件无法上传。",
"3db83fc58a": "没有可用于附件的远程项目路径。",
"ba6cb77082": "无法连接到项目。",
"chooseOrAddProjectBeforeWorkspace": "创建工作区前,请选择或添加项目。",