diff --git a/backend/src/routes/stacks.ts b/backend/src/routes/stacks.ts index 05d46574..9c03e2ac 100644 --- a/backend/src/routes/stacks.ts +++ b/backend/src/routes/stacks.ts @@ -2639,6 +2639,9 @@ function sendFsError( console.error('[files] %s (helper failure): %s', sanitizeForLog(fallback), sanitizeForLog(e.message)); return res.status(status).json({ error: fallback }); } + if (status === 409 && /not empty/i.test(e.message)) { + return res.status(409).json({ error: e.message, code: 'NOT_EMPTY' satisfies FsErrorCode }); + } return res.status(status).json({ error: e.message }); } console.error(`[files] ${fallback}:`, sanitizeForLog(e.message)); diff --git a/frontend/src/components/files/DeleteFileConfirm.tsx b/frontend/src/components/files/DeleteFileConfirm.tsx index 10352b0a..456fb067 100644 --- a/frontend/src/components/files/DeleteFileConfirm.tsx +++ b/frontend/src/components/files/DeleteFileConfirm.tsx @@ -6,7 +6,7 @@ import { BusyButton } from '@/components/ui/busy-button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { toast } from '@/components/ui/toast-store'; -import { deleteStackPath } from '@/lib/stackFilesApi'; +import { deleteStackPath, NotEmptyError } from '@/lib/stackFilesApi'; import type { FileEntry } from '@/lib/stackFilesApi'; interface DeleteFileConfirmProps { @@ -50,15 +50,14 @@ export function DeleteFileConfirm({ const executeDelete = async (recursive: boolean) => { setDeleting(true); try { - await deleteStackPath(stackName, relPath, recursive || undefined, rootId); + await deleteStackPath(stackName, relPath, recursive, rootId); onDeleted(); onOpenChange(false); } catch (e: unknown) { - const msg = e instanceof Error ? e.message : 'Delete failed.'; - if (!recursive && msg.toUpperCase().includes('NOT_EMPTY')) { + if (!recursive && e instanceof NotEmptyError) { setNotEmpty(true); } else { - toast.error(msg); + toast.error(e instanceof Error ? e.message : 'Delete failed.'); } } finally { setDeleting(false); diff --git a/frontend/src/components/files/StackFileExplorer.tsx b/frontend/src/components/files/StackFileExplorer.tsx index f8fe67cf..797e2b28 100644 --- a/frontend/src/components/files/StackFileExplorer.tsx +++ b/frontend/src/components/files/StackFileExplorer.tsx @@ -657,7 +657,7 @@ export function StackFileExplorer({ entry={ctxDeleteEntry} rootId={selectedRootId} onDeleted={() => { - if (ctxDeletePath === selectedPath) handleDeleted(); + if (openFileAffectedBy([ctxDeletePath])) handleDeleted(); else refresh(); setCtxDeletePath(''); setCtxDeleteEntry(null); diff --git a/frontend/src/components/files/__tests__/DeleteFileConfirm.test.tsx b/frontend/src/components/files/__tests__/DeleteFileConfirm.test.tsx new file mode 100644 index 00000000..0e77976e --- /dev/null +++ b/frontend/src/components/files/__tests__/DeleteFileConfirm.test.tsx @@ -0,0 +1,206 @@ +/** + * Coverage for DeleteFileConfirm two-step delete semantics. + * + * Deleting a non-empty directory without the recursive flag returns HTTP 409 + * with code NOT_EMPTY from the server. The dialog catches this via + * NotEmptyError and promotes the confirm button to "Delete all" so the user + * can confirm recursive deletion. Other errors surface as toasts. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +const h = vi.hoisted(() => ({ + deleteMock: vi.fn<(stack: string, path: string, recursive?: boolean, rootId?: string) => Promise>(), + toastError: vi.fn(), +})); + +// Keep the real module so NotEmptyError stays a real class for instanceof. +vi.mock('@/lib/stackFilesApi', async (orig) => ({ + ...(await orig()), + deleteStackPath: h.deleteMock, +})); + +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: h.toastError, success: vi.fn(), loading: vi.fn(() => 'id'), dismiss: vi.fn() }, +})); + +import { DeleteFileConfirm } from '../DeleteFileConfirm'; +import { NotEmptyError } from '@/lib/stackFilesApi'; +import type { FileEntry } from '@/lib/stackFilesApi'; + +const dirEntry: FileEntry = { + name: 'nonempty', + type: 'directory', + size: 0, + mtime: 0, + isProtected: false, +}; + +const fileEntry: FileEntry = { + name: 'app.conf', + type: 'file', + size: 1024, + mtime: 1700000000000, + isProtected: false, +}; + +const protectedEntry: FileEntry = { + name: 'compose.yaml', + type: 'file', + size: 2048, + mtime: 1700000000000, + isProtected: true, +}; + +function setup(entry: FileEntry = dirEntry) { + const onDeleted = vi.fn(); + const onOpenChange = vi.fn(); + render( + , + ); + return { onDeleted, onOpenChange }; +} + +beforeEach(() => { + h.deleteMock.mockReset(); + h.toastError.mockReset(); +}); + +describe('DeleteFileConfirm', () => { + it('deletes the entry and reports success', async () => { + h.deleteMock.mockResolvedValue(undefined); + const user = userEvent.setup(); + const { onDeleted, onOpenChange } = setup(fileEntry); + + await user.click(screen.getByTestId('delete-confirm-btn')); + + await waitFor(() => + expect(h.deleteMock).toHaveBeenCalledWith('my-stack', 'app.conf', false, 'stack-source'), + ); + expect(onDeleted).toHaveBeenCalled(); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it('shows the not-empty warning and retries with recursive on second click', async () => { + h.deleteMock + .mockRejectedValueOnce(new NotEmptyError('Directory is not empty')) + .mockResolvedValueOnce(undefined); + const user = userEvent.setup(); + const { onDeleted, onOpenChange } = setup(dirEntry); + + // First click: non-recursive, receives NotEmptyError. + await user.click(screen.getByTestId('delete-confirm-btn')); + + // The warning banner and "Delete all" button appear. + expect(await screen.findByText(/this folder is not empty/i)).toBeInTheDocument(); + expect(screen.getByTestId('delete-confirm-btn')).toHaveTextContent('Delete all'); + + // The first attempt was non-recursive. + expect(h.deleteMock).toHaveBeenCalledTimes(1); + expect(h.deleteMock).toHaveBeenLastCalledWith('my-stack', 'nonempty', false, 'stack-source'); + + // Second click: retries with recursive=true. + await user.click(screen.getByTestId('delete-confirm-btn')); + + await waitFor(() => expect(h.deleteMock).toHaveBeenCalledTimes(2)); + expect(h.deleteMock).toHaveBeenLastCalledWith('my-stack', 'nonempty', true, 'stack-source'); + expect(onDeleted).toHaveBeenCalled(); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it('routes an unrelated error to a toast and does not change the button label', async () => { + h.deleteMock.mockRejectedValueOnce(new Error('disk full')); + const user = userEvent.setup(); + const { onDeleted, onOpenChange } = setup(fileEntry); + + await user.click(screen.getByTestId('delete-confirm-btn')); + + await waitFor(() => expect(h.toastError).toHaveBeenCalledWith('disk full')); + expect(onDeleted).not.toHaveBeenCalled(); + expect(onOpenChange).not.toHaveBeenCalledWith(false); + // The button stays "Delete", not "Delete all". + expect(screen.getByTestId('delete-confirm-btn')).not.toHaveTextContent('Delete all'); + expect(screen.getByTestId('delete-confirm-btn')).toHaveTextContent('Delete'); + // No not-empty warning. + expect(screen.queryByText(/this folder is not empty/i)).toBeNull(); + }); + + it('toasts when a recursive retry still fails with NotEmptyError instead of re-promoting', async () => { + h.deleteMock + .mockRejectedValueOnce(new NotEmptyError('Directory is not empty')) + .mockRejectedValueOnce(new NotEmptyError('Directory is not empty')); + const user = userEvent.setup(); + const { onDeleted } = setup(dirEntry); + + // First click: non-recursive, receives NotEmptyError, promotes to "Delete all". + await user.click(screen.getByTestId('delete-confirm-btn')); + expect(await screen.findByText(/this folder is not empty/i)).toBeInTheDocument(); + + // Second click: recursive, still fails with NotEmptyError (race condition). + // The !recursive guard should route this to a toast instead of re-promoting. + await user.click(screen.getByTestId('delete-confirm-btn')); + + await waitFor(() => expect(h.toastError).toHaveBeenCalledWith('Directory is not empty')); + expect(onDeleted).not.toHaveBeenCalled(); + // The recursive guard (!recursive) correctly routes to toast instead of + // re-calling setNotEmpty. The warning stays visible (the directory is still + // not empty) and the user can retry or cancel. + expect(h.toastError).toHaveBeenCalledTimes(1); + expect(screen.getByTestId('delete-confirm-btn')).toHaveTextContent('Delete all'); + }); + + it('disables the delete button until the correct filename is typed for protected files', async () => { + h.deleteMock.mockResolvedValue(undefined); + const user = userEvent.setup(); + setup(protectedEntry); + + // Button is disabled because the confirm input is empty. + expect(screen.getByTestId('delete-confirm-btn')).toBeDisabled(); + + // Typing a wrong name keeps it disabled. + await user.type(screen.getByLabelText(/type compose.yaml to confirm/i), 'wrong'); + expect(screen.getByTestId('delete-confirm-btn')).toBeDisabled(); + + // Typing the correct name enables it. + await user.clear(screen.getByLabelText(/type compose.yaml to confirm/i)); + await user.type(screen.getByLabelText(/type compose.yaml to confirm/i), 'compose.yaml'); + expect(screen.getByTestId('delete-confirm-btn')).toBeEnabled(); + }); + + it('resets the notEmpty state when the dialog is reopened', async () => { + const user = userEvent.setup(); + const baseProps = { + open: true, + onOpenChange: vi.fn(), + stackName: 'my-stack', + relPath: 'nonempty', + entry: dirEntry, + onDeleted: vi.fn(), + }; + const { rerender } = render(); + + // Trigger the not-empty state. + h.deleteMock.mockRejectedValueOnce(new NotEmptyError('Directory is not empty')); + await user.click(screen.getByTestId('delete-confirm-btn')); + expect(await screen.findByText(/this folder is not empty/i)).toBeInTheDocument(); + + // Close and reopen: the state should reset. + rerender(); + rerender(); + + // After reopening, the warning should be gone and button should say "Delete". + await waitFor(() => + expect(screen.queryByText(/this folder is not empty/i)).toBeNull(), + ); + expect(screen.getByTestId('delete-confirm-btn')).toHaveTextContent('Delete'); + }); +}); diff --git a/frontend/src/lib/__tests__/stackFilesApi.test.ts b/frontend/src/lib/__tests__/stackFilesApi.test.ts index 6530830f..42736514 100644 --- a/frontend/src/lib/__tests__/stackFilesApi.test.ts +++ b/frontend/src/lib/__tests__/stackFilesApi.test.ts @@ -14,6 +14,8 @@ import { nextDuplicateName, createEmptyStackFile, UploadConflictError, + deleteStackPath, + NotEmptyError, } from '../stackFilesApi'; describe('isClientSafeRelPath', () => { @@ -193,3 +195,77 @@ describe('createEmptyStackFile', () => { expect(err).not.toBeInstanceOf(UploadConflictError); }); }); + +describe('deleteStackPath', () => { + function stubFetch(status: number, body?: object) { + const res = { + status, + ok: status >= 200 && status < 300, + headers: { get: () => null }, + clone() { return this; }, + json: async () => body ?? {}, + }; + const fetchMock = vi.fn().mockResolvedValue(res); + vi.stubGlobal('fetch', fetchMock); + return fetchMock; + } + + beforeEach(() => localStorage.clear()); + afterEach(() => vi.unstubAllGlobals()); + + it('resolves on success without throwing', async () => { + const fetchMock = stubFetch(204); + await expect(deleteStackPath('my-stack', 'nonempty')).resolves.toBeUndefined(); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(new URL(url, 'http://x').searchParams.get('path')).toBe('nonempty'); + expect(init).toMatchObject({ method: 'DELETE' }); + }); + + it('throws NotEmptyError when the server reports the directory is not empty', async () => { + stubFetch(409, { code: 'NOT_EMPTY', error: 'Directory is not empty' }); + await expect(deleteStackPath('my-stack', 'nonempty')).rejects.toBeInstanceOf(NotEmptyError); + }); + + it('includes the server error message in NotEmptyError', async () => { + stubFetch(409, { code: 'NOT_EMPTY', error: 'Directory is not empty' }); + const err = await deleteStackPath('my-stack', 'nonempty').catch((e) => e); + expect(err).toBeInstanceOf(NotEmptyError); + expect((err as Error).message).toBe('Directory is not empty'); + }); + + it('uses a fallback message when the server omits the error field', async () => { + stubFetch(409, { code: 'NOT_EMPTY' }); + const err = await deleteStackPath('my-stack', 'nonempty').catch((e) => e); + expect(err).toBeInstanceOf(NotEmptyError); + expect((err as Error).message).toBe('Directory is not empty.'); + }); + + it('throws a generic error (not NotEmptyError) for other 409 codes', async () => { + stubFetch(409, { code: 'PROTECTED_FILE', error: 'This is a protected stack file.' }); + const err = await deleteStackPath('my-stack', 'compose.yaml').catch((e) => e); + expect(err).toBeInstanceOf(Error); + expect(err).not.toBeInstanceOf(NotEmptyError); + expect((err as Error).message).toBe('This is a protected stack file.'); + }); + + it('passes recursive=1 in the query string when recursive is true', async () => { + const fetchMock = stubFetch(204); + await deleteStackPath('my-stack', 'nonempty', true); + const [url] = fetchMock.mock.calls[0] as [string]; + expect(new URL(url, 'http://x').searchParams.get('recursive')).toBe('1'); + }); + + it('omits the recursive query param when recursive is false', async () => { + const fetchMock = stubFetch(204); + await deleteStackPath('my-stack', 'nonempty', false); + const [url] = fetchMock.mock.calls[0] as [string]; + expect(new URL(url, 'http://x').searchParams.get('recursive')).toBeNull(); + }); + + it('includes rootId in the query string when provided', async () => { + const fetchMock = stubFetch(204); + await deleteStackPath('my-stack', 'nonempty', undefined, 'vol-abc'); + const [url] = fetchMock.mock.calls[0] as [string]; + expect(new URL(url, 'http://x').searchParams.get('rootId')).toBe('vol-abc'); + }); +}); diff --git a/frontend/src/lib/stackFilesApi.ts b/frontend/src/lib/stackFilesApi.ts index 35141ecc..bae0bdad 100644 --- a/frontend/src/lib/stackFilesApi.ts +++ b/frontend/src/lib/stackFilesApi.ts @@ -380,6 +380,20 @@ export async function writeStackFile( return { version, mtimeMs }; } +/** + * Thrown by deleteStackPath when the server refuses to delete a non-empty + * directory without the recursive flag (HTTP 409, code NOT_EMPTY). The + * DeleteFileConfirm dialog promotes its confirm button to "Delete all" on + * this signal and retries with recursive=true. + */ +export class NotEmptyError extends Error { + readonly code = 'NOT_EMPTY' as const; + constructor(message: string) { + super(message); + this.name = 'NotEmptyError'; + } +} + export async function deleteStackPath( stackName: string, relPath: string, @@ -392,6 +406,15 @@ export async function deleteStackPath( stackFilesUrl(stackName, `?path=${encodeURIComponent(relPath)}${recursiveSuffix}${rootParam(rootId)}`), { method: 'DELETE' }, ); + if (res.status === 409) { + let body: { code?: string; error?: string } = {}; + try { body = await res.clone().json(); } catch { /* ignore */ } + if (body.code === 'NOT_EMPTY') { + throw new NotEmptyError(body.error ?? 'Directory is not empty.'); + } + // PROTECTED_FILE and any other 409 fall through to the generic Error path + // so the caller surfaces the server message as a toast. + } if (!res.ok) throw new Error(await parseApiError(res)); }