fix(ui-rewrite): bound dialog height and reconcile failed prompt deletes

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

Author: Marek Dano

client/src/components/ui/dialog.test.tsx

@@ -211,6 +211,26 @@ describe("Dialog Components", () => {
       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(
+        <Dialog open={true}>
+          <DialogContent data-testid="content">
+            <DialogTitle>Tall dialog</DialogTitle>
+            <DialogDescription>Lots of content</DialogDescription>
+            {Array.from({ length: 100 }, (_, i) => (
+              <p key={i}>Row {i}</p>
+            ))}
+          </DialogContent>
+        </Dialog>,
+      );
+
+      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(
         <Dialog open={true}>

client/src/components/ui/dialog.tsx

@@ -36,7 +36,7 @@ const DialogContent = React.forwardRef<
     <DialogPrimitive.Content
       ref={ref}
       className={cn(
-        "fixed inset-x-0 top-1/2 z-50 mx-auto grid w-full max-w-lg -translate-y-1/2 gap-5 border bg-popover p-7 shadow-lg duration-150 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 sm:rounded-lg",
+        "fixed inset-x-0 top-1/2 z-50 mx-auto grid max-h-[calc(100vh-2rem)] w-full max-w-lg -translate-y-1/2 gap-5 overflow-y-auto border bg-popover p-7 shadow-lg duration-150 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 sm:rounded-lg",
         className,
       )}
       {...props}

client/src/pages/Prompts.test.tsx

@@ -405,7 +405,7 @@ describe("Prompts", () => {
     expect(screen.getByRole("button", { name: /more options for translate/i })).toBeInTheDocument();
   });
 
-  it("restores the deleted prompt row when the DELETE request fails", async () => {
+  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({
@@ -424,6 +424,9 @@ describe("Prompts", () => {
       }),
     ];
 
+    // 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", () =>
@@ -447,14 +450,75 @@ describe("Prompts", () => {
     const dialog = await screen.findByRole("dialog");
     await user.click(within(dialog).getByRole("button", { name: /^delete$/i }));
 
-    // The optimistic removal is rolled back, so the row reappears.
+    // 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(<Prompts />);
+
+    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({

client/src/pages/Prompts.tsx

@@ -293,9 +293,9 @@ export function Prompts() {
     const prompt = promptToDelete;
     const name = getPromptLabel(prompt) || prompt.id;
 
-    // Snapshot the list and the open group so we can roll back the optimistic
-    // removal if the DELETE fails, mirroring the Tools page delete flow.
-    const previousData = promptsData;
+    // 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)[]) =>
@@ -329,14 +329,33 @@ export function Prompts() {
         console.error("Failed to refresh prompts after deletion:", sanitizeError(refreshErr));
       }
     } catch (err) {
-      // Restore the pre-delete list and reopen the group on failure.
-      setPromptsData(() => previousData);
-      setActiveGroup(previousActiveGroup);
+      // 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, promptsData, activeGroup, setPromptsData, refetch, intl]);
+  }, [promptToDelete, activeGroup, setPromptsData, refetch, intl, restPromptsLabel]);
 
   return (
     <div className="p-6">