diff --git a/client/src/api/prompts.test.ts b/client/src/api/prompts.test.ts index 7a23ebef6b..40aa964242 100644 --- a/client/src/api/prompts.test.ts +++ b/client/src/api/prompts.test.ts @@ -123,4 +123,42 @@ describe("promptsApi", () => { ); }); }); + + describe("delete", () => { + it("calls DELETE /prompts/:id with CSRF token and same-origin credentials", async () => { + mockFetch.mockResolvedValueOnce(new Response(null, { status: 204 })); + + await promptsApi.delete("prompt-abc-123"); + + expect(mockFetch).toHaveBeenCalledWith( + expect.stringContaining("/prompts/prompt-abc-123"), + expect.objectContaining({ + method: "DELETE", + headers: expect.objectContaining({ + "X-CSRF-Token": "test-csrf-token", + }), + credentials: "same-origin", // pragma: allowlist secret + }), + ); + }); + + it("throws synchronously for an empty ID", () => { + expect(() => promptsApi.delete("")).toThrow("Invalid prompt ID"); + }); + + it("throws synchronously for an ID with path traversal characters", () => { + expect(() => promptsApi.delete("../etc/passwd")).toThrow("Invalid prompt ID format"); + }); + + it("throws ApiError on 404 response", async () => { + mockFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ detail: "Prompt not found" }), { + status: 404, + headers: { "Content-Type": "application/json" }, + }), + ); + + await expect(promptsApi.delete("prompt-abc-123")).rejects.toThrow("HTTP 404"); + }); + }); }); diff --git a/client/src/api/prompts.ts b/client/src/api/prompts.ts index fa214bcdfa..0437494bf1 100644 --- a/client/src/api/prompts.ts +++ b/client/src/api/prompts.ts @@ -116,4 +116,16 @@ export const promptsApi = { const validId = validatePromptId(id); return api.put(`/prompts/${encodeURIComponent(validId)}`, { tags }); }, + + /** + * Delete a prompt by its primary-key ID. + * + * Mirrors `DELETE /prompts/{prompt_id}` (`mcpgateway/main.py`), which requires + * the `prompts.delete` permission. The ID is validated (not the name used by + * {@link promptsApi.render}) to guard against path traversal and injection. + */ + delete: (id: string): Promise => { + const validId = validatePromptId(id); + return api.delete(`/prompts/${encodeURIComponent(validId)}`); + }, }; diff --git a/client/src/components/ui/dialog.test.tsx b/client/src/components/ui/dialog.test.tsx index f425f051aa..51603b2b91 100644 --- a/client/src/components/ui/dialog.test.tsx +++ b/client/src/components/ui/dialog.test.tsx @@ -204,11 +204,33 @@ describe("Dialog Components", () => { const content = screen.getByTestId("content"); expect(content).toHaveClass("fixed"); - expect(content).toHaveClass("inset-0"); - expect(content).toHaveClass("m-auto"); + expect(content).toHaveClass("inset-x-0"); + expect(content).toHaveClass("top-1/2"); + expect(content).toHaveClass("-translate-y-1/2"); + expect(content).toHaveClass("mx-auto"); expect(content).toHaveClass("z-50"); }); + it("bounds its height to the viewport and scrolls overflowing content", () => { + // Tall dialogs must stay within the viewport so the header, top content, + // and close button remain reachable instead of extending off-screen. + render( + + + Tall dialog + Lots of content + {Array.from({ length: 100 }, (_, i) => ( +

Row {i}

+ ))} + + , + ); + + const content = screen.getByTestId("content"); + expect(content).toHaveClass("max-h-[calc(100vh-2rem)]"); + expect(content).toHaveClass("overflow-y-auto"); + }); + it("should include close button", () => { render( diff --git a/client/src/components/ui/dialog.tsx b/client/src/components/ui/dialog.tsx index ea474b6949..75179b01ad 100644 --- a/client/src/components/ui/dialog.tsx +++ b/client/src/components/ui/dialog.tsx @@ -36,7 +36,7 @@ const DialogContent = React.forwardRef< ) => ( -
+
); DialogHeader.displayName = "DialogHeader"; diff --git a/client/src/i18n/locales/en-US/prompts.json b/client/src/i18n/locales/en-US/prompts.json index c786228cf0..5dc448f4fa 100644 --- a/client/src/i18n/locales/en-US/prompts.json +++ b/client/src/i18n/locales/en-US/prompts.json @@ -81,5 +81,10 @@ "prompts.add.error.teamRequired": "Team selection is required when visibility is set to team", "prompts.add.error.argumentsInvalidJson": "Invalid JSON format", "prompts.add.error.argumentsArrayRequired": "Arguments must be a JSON array", - "prompts.tags.addError": "Failed to add tag. Please try again." + "prompts.tags.addError": "Failed to add tag. Please try again.", + "prompts.delete.confirm.title": "Delete prompt", + "prompts.delete.confirm.description": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", + "prompts.delete.confirm.button": "Delete", + "prompts.delete.success": "Prompt \"{name}\" deleted successfully", + "prompts.delete.error": "Failed to delete prompt" } diff --git a/client/src/i18n/locales/es-ES/prompts.json b/client/src/i18n/locales/es-ES/prompts.json index 433e5f91aa..aaf305e1fa 100644 --- a/client/src/i18n/locales/es-ES/prompts.json +++ b/client/src/i18n/locales/es-ES/prompts.json @@ -81,5 +81,10 @@ "prompts.add.error.teamRequired": "La selección de equipo es obligatoria cuando la visibilidad está configurada como equipo", "prompts.add.error.argumentsInvalidJson": "Formato JSON inválido", "prompts.add.error.argumentsArrayRequired": "Los argumentos deben ser un array JSON", - "prompts.tags.addError": "No se pudo añadir la etiqueta. Inténtelo de nuevo." + "prompts.tags.addError": "No se pudo añadir la etiqueta. Inténtelo de nuevo.", + "prompts.delete.confirm.title": "Eliminar prompt", + "prompts.delete.confirm.description": "¿Está seguro de que desea eliminar \"{name}\"? Esta acción no se puede deshacer.", + "prompts.delete.confirm.button": "Eliminar", + "prompts.delete.success": "Prompt \"{name}\" eliminado correctamente", + "prompts.delete.error": "Error al eliminar el prompt" } diff --git a/client/src/i18n/locales/pt-BR/prompts.json b/client/src/i18n/locales/pt-BR/prompts.json index 538ed25f80..3964838739 100644 --- a/client/src/i18n/locales/pt-BR/prompts.json +++ b/client/src/i18n/locales/pt-BR/prompts.json @@ -81,5 +81,10 @@ "prompts.add.error.teamRequired": "A seleção de equipe é obrigatória quando a visibilidade está definida como equipe", "prompts.add.error.argumentsInvalidJson": "Formato JSON inválido", "prompts.add.error.argumentsArrayRequired": "Os argumentos devem ser um array JSON", - "prompts.tags.addError": "Falha ao adicionar a tag. Tente novamente." + "prompts.tags.addError": "Falha ao adicionar a tag. Tente novamente.", + "prompts.delete.confirm.title": "Excluir prompt", + "prompts.delete.confirm.description": "Tem certeza de que deseja excluir \"{name}\"? Esta ação não pode ser desfeita.", + "prompts.delete.confirm.button": "Excluir", + "prompts.delete.success": "Prompt \"{name}\" excluído com sucesso", + "prompts.delete.error": "Falha ao excluir o prompt" } diff --git a/client/src/pages/Prompts.test.tsx b/client/src/pages/Prompts.test.tsx index ce813d343c..fbb63c52c3 100644 --- a/client/src/pages/Prompts.test.tsx +++ b/client/src/pages/Prompts.test.tsx @@ -295,6 +295,230 @@ describe("Prompts", () => { expect(screen.getByRole("menuitem", { name: "Delete" })).toBeInTheDocument(); }); + it("deletes the sole prompt in a group and closes the drawer after confirmation", async () => { + const user = userEvent.setup(); + let remaining: Prompt[] = [ + createMockPrompt({ + id: "prompt-1", + name: "summarize", + displayName: "Summarize document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + ]; + + const deleteSpy = vi.fn(() => { + remaining = remaining.filter((p) => p.id !== "prompt-1"); + return HttpResponse.json({ status: "success" }); + }); + server.use( + http.get("/prompts", () => HttpResponse.json(remaining)), + http.delete("/prompts/prompt-1", deleteSpy), + ); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument(); + }); + + // Open the group's details panel and its Definition tab. + await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" })); + await user.click(await screen.findByRole("menuitem", { name: "View details" })); + await user.click(await screen.findByRole("tab", { name: /definition/i })); + + // Trigger delete from the row overflow menu. + await user.click(await screen.findByRole("button", { name: /more options for summarize/i })); + await user.click(await screen.findByRole("menuitem", { name: "Delete" })); + + // Confirm dialog names the prompt and requires an explicit confirmation. + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText("Delete prompt")).toBeInTheDocument(); + expect( + within(dialog).getByText(/Are you sure you want to delete "Summarize document"/), + ).toBeInTheDocument(); + + await user.click(within(dialog).getByRole("button", { name: /^delete$/i })); + + await waitFor(() => { + expect(deleteSpy).toHaveBeenCalledTimes(1); + }); + + // The group's only prompt is gone, so its grid card (and the card-only + // "More options" trigger) disappears. + await waitFor(() => { + expect( + screen.queryByRole("button", { name: "More options for gh-repo-tasks" }), + ).not.toBeInTheDocument(); + }); + }); + + it("keeps the drawer and other rows when deleting one prompt from a multi-prompt group", async () => { + const user = userEvent.setup(); + let remaining: Prompt[] = [ + createMockPrompt({ + id: "prompt-1", + name: "summarize", + displayName: "Summarize document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + createMockPrompt({ + id: "prompt-2", + name: "translate", + displayName: "Translate document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + ]; + + server.use( + http.get("/prompts", () => HttpResponse.json(remaining)), + http.delete("/prompts/prompt-1", () => { + remaining = remaining.filter((p) => p.id !== "prompt-1"); + return HttpResponse.json({ status: "success" }); + }), + ); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" })); + await user.click(await screen.findByRole("menuitem", { name: "View details" })); + await user.click(await screen.findByRole("tab", { name: /definition/i })); + + await user.click(await screen.findByRole("button", { name: /more options for summarize/i })); + await user.click(await screen.findByRole("menuitem", { name: "Delete" })); + + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: /^delete$/i })); + + // The deleted row is gone but the drawer stays open with the sibling prompt. + await waitFor(() => { + expect( + screen.queryByRole("button", { name: /more options for summarize/i }), + ).not.toBeInTheDocument(); + }); + expect(screen.getByRole("button", { name: /more options for translate/i })).toBeInTheDocument(); + }); + + it("reconciles with the server and restores the row when the DELETE fails and the prompt survives", async () => { + const user = userEvent.setup(); + const prompts: Prompt[] = [ + createMockPrompt({ + id: "prompt-1", + name: "summarize", + displayName: "Summarize document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + createMockPrompt({ + id: "prompt-2", + name: "translate", + displayName: "Translate document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + ]; + + // The DELETE fails and the prompt is NOT removed server-side (a pre-commit + // failure), so the post-failure refetch still returns it and the row + // reappears. + server.use( + http.get("/prompts", () => HttpResponse.json(prompts)), + http.delete("/prompts/prompt-1", () => + HttpResponse.json({ detail: "Prompt is in use" }, { status: 409 }), + ), + ); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" })); + await user.click(await screen.findByRole("menuitem", { name: "View details" })); + await user.click(await screen.findByRole("tab", { name: /definition/i })); + + await user.click(await screen.findByRole("button", { name: /more options for summarize/i })); + await user.click(await screen.findByRole("menuitem", { name: "Delete" })); + + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: /^delete$/i })); + + // The optimistic removal is reconciled against server truth, so the row + // reappears because the prompt still exists. + await waitFor(() => { + expect( + screen.getByRole("button", { name: /more options for summarize/i }), + ).toBeInTheDocument(); + }); + }); + + it("does not resurrect a prompt when the DELETE reports an error but the row is already gone", async () => { + const user = userEvent.setup(); + // Simulates a post-commit failure: PromptService.delete_prompt() commits + // the removal, then a later step (notification/cache invalidation) fails + // and the endpoint returns an error. The server no longer has the prompt, + // so reconciling must NOT restore it. + let remaining: Prompt[] = [ + createMockPrompt({ + id: "prompt-1", + name: "summarize", + displayName: "Summarize document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + createMockPrompt({ + id: "prompt-2", + name: "translate", + displayName: "Translate document", + gatewayId: "gw-github", + gatewaySlug: "gh-repo-tasks", + }), + ]; + + server.use( + http.get("/prompts", () => HttpResponse.json(remaining)), + http.delete("/prompts/prompt-1", () => { + // Row is committed as deleted, but the request still errors out. + remaining = remaining.filter((p) => p.id !== "prompt-1"); + return HttpResponse.json({ detail: "Post-commit hook failed" }, { status: 500 }); + }), + ); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" })); + await user.click(await screen.findByRole("menuitem", { name: "View details" })); + await user.click(await screen.findByRole("tab", { name: /definition/i })); + + await user.click(await screen.findByRole("button", { name: /more options for summarize/i })); + await user.click(await screen.findByRole("menuitem", { name: "Delete" })); + + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: /^delete$/i })); + + // The drawer stays open with the surviving sibling, and the deleted prompt + // is not brought back by the failure path. + await waitFor(() => { + expect( + screen.getByRole("button", { name: /more options for translate/i }), + ).toBeInTheDocument(); + }); + expect( + screen.queryByRole("button", { name: /more options for summarize/i }), + ).not.toBeInTheDocument(); + }); + it("collapses gateway-less prompts into a single REST prompts card", async () => { const prompts: Prompt[] = [ createMockPrompt({ diff --git a/client/src/pages/Prompts.tsx b/client/src/pages/Prompts.tsx index 51fad69fea..f8de35d8ac 100644 --- a/client/src/pages/Prompts.tsx +++ b/client/src/pages/Prompts.tsx @@ -13,10 +13,12 @@ import { } from "@/components/ui/dropdown-menu"; import type { CursorPaginatedPromptsResponse, PromptRead } from "@/generated/types"; import { PromptDetailsPanel } from "@/components/prompts"; +import { ConfirmDialog } from "@/components/servers/ConfirmDialog"; import { useQuery } from "@/hooks/useQuery"; import { promptsApi } from "@/api/prompts"; import { ApiError } from "@/api/client"; -import { extractApiErrorDetail } from "@/utils/errors"; +import { extractApiErrorDetail, sanitizeError } from "@/utils/errors"; +import { parseApiError } from "@/lib/errorUtils"; import { toast } from "sonner"; import type { PromptGroup } from "@/types/prompts"; @@ -202,6 +204,8 @@ export function Prompts() { const addPromptsCardRef = useRef(null); const [showForm, setShowForm] = useState(false); const [shouldRestoreFormCloseFocus, setShouldRestoreFormCloseFocus] = useState(false); + const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); + const [promptToDelete, setPromptToDelete] = useState | null>(null); const { data: promptsData, error, @@ -271,15 +275,87 @@ export function Prompts() { await refetch(); }; - // TODO: placeholder handlers so the details-panel Definition table shows its + // TODO: placeholder handler so the details-panel Definition table shows its // row overflow menu. No behaviour yet — a follow-up PR adds PromptForm edit - // mode and a delete flow. + // mode. const handleEditPrompt = () => { // TODO: open PromptForm in edit mode for the selected prompt }; - const handleDeletePrompt = () => { - // TODO: delete the selected prompt (with confirmation) - }; + + const handleDeletePrompt = useCallback((prompt: NonNullable) => { + setPromptToDelete(prompt); + setDeleteDialogOpen(true); + }, []); + + const confirmDeletePrompt = useCallback(async () => { + if (!promptToDelete) return; + + const prompt = promptToDelete; + const name = getPromptLabel(prompt) || prompt.id; + + // Remember the open group so a failed delete can re-resolve it against + // server truth (below). We deliberately do NOT snapshot the whole list: + // reconciling with the server on failure is safer than blindly restoring. + const previousActiveGroup = activeGroup; + + const removeFromList = (list: (PromptRead | null)[]) => + list.filter((p) => p == null || p.id !== prompt.id); + setPromptsData((prev) => { + if (!prev) return prev; + return Array.isArray(prev) + ? removeFromList(prev) + : { ...prev, prompts: removeFromList(prev.prompts) }; + }); + + // Keep the open drawer's group in sync with the removal: drop the prompt + // from its list, and close the drawer once the group is empty. Recomputing + // from the current `activeGroup` (rather than checking a frozen snapshot) + // keeps sequential deletes from the same group correct. + if (activeGroup) { + const remaining = activeGroup.prompts.filter((p) => p.id !== prompt.id); + setActiveGroup(remaining.length > 0 ? { ...activeGroup, prompts: remaining } : null); + } + + setDeleteDialogOpen(false); + setPromptToDelete(null); + + try { + await promptsApi.delete(prompt.id); + toast.success(intl.formatMessage({ id: "prompts.delete.success" }, { name })); + + try { + await refetch(); + } catch (refreshErr) { + console.error("Failed to refresh prompts after deletion:", sanitizeError(refreshErr)); + } + } catch (err) { + // A DELETE error does not guarantee the prompt still exists: + // PromptService.delete_prompt() commits before running notification and + // cache invalidation, so a post-commit failure returns an error even + // though the row is already gone. Reconcile with the server instead of + // restoring a pre-delete snapshot — that would resurrect an + // already-deleted prompt, and unconditionally restoring the whole list + // could also clobber newer state from an overlapping delete. + try { + const fresh = await refetch(); + // Re-resolve the open drawer's group from server truth so the drawer + // reopens/repopulates on a genuine failure (or stays closed if the + // group is now empty). + if (previousActiveGroup) { + const freshGroups = buildPromptGroups(getPromptItems(fresh), restPromptsLabel); + setActiveGroup(freshGroups.find((g) => g.key === previousActiveGroup.key) ?? null); + } + } catch (refreshErr) { + console.error( + "Failed to reconcile prompts after failed deletion:", + sanitizeError(refreshErr), + ); + } + + // Surface the backend detail when present, otherwise the localized fallback. + toast.error(parseApiError(err, intl.formatMessage({ id: "prompts.delete.error" }))); + } + }, [promptToDelete, activeGroup, setPromptsData, refetch, intl, restPromptsLabel]); return (
@@ -335,6 +411,19 @@ export function Prompts() { onEdit={handleEditPrompt} onDelete={handleDeletePrompt} /> + +
); }