From 0ab6c0342966661c69225592d8dd5fa6ded53778 Mon Sep 17 00:00:00 2001
From: Kandev Agent <agent@kandev.dev>
Date: Fri, 1 May 2026 19:02:51 +0100
Subject: [PATCH] fix: invalidate ValidatedPopover in-flight submits on close

If the user clicked Submit and dismissed the popover before the request
settled, the next time they re-opened it would inherit `loading=true` (the
disabled-while-loading submit button was stuck) and a late rejection from
the previous session could repopulate `error` in the new one.

Track the submit via a `useRef` token, bump it on close, and ignore any
result/rejection whose token no longer matches. Also reset `loading` and
`error` on close so the next open starts clean. Adds a regression test
covering both the late-resolution and late-rejection paths.

Addresses CodeRabbit review on PR #759.
---
 .../integrations/validated-popover.test.tsx   | 130 ++++++++++++++++++
 .../integrations/validated-popover.tsx        |  22 ++-
 2 files changed, 147 insertions(+), 5 deletions(-)
 create mode 100644 apps/web/components/integrations/validated-popover.test.tsx

diff --git a/apps/web/components/integrations/validated-popover.test.tsx b/apps/web/components/integrations/validated-popover.test.tsx
new file mode 100644
index 0000000000..1af519ec9b
--- /dev/null
+++ b/apps/web/components/integrations/validated-popover.test.tsx
@@ -0,0 +1,130 @@
+import { describe, it, expect, vi, afterEach } from "vitest";
+import { render, screen, fireEvent, act, waitFor, cleanup } from "@testing-library/react";
+
+afterEach(() => {
+  cleanup();
+});
+
+// Stub the Radix-backed Popover/Tooltip primitives so children render inline
+// (no portal, no measurement gymnastics) and the popover's open/close is
+// driven by ValidatedPopover's own onOpenChange callback that we surface via
+// a parent-controlled close button.
+vi.mock("@kandev/ui/popover", () => {
+  return {
+    Popover: ({
+      open,
+      onOpenChange,
+      children,
+    }: {
+      open: boolean;
+      onOpenChange: (next: boolean) => void;
+      children: React.ReactNode;
+    }) => (
+      <div data-testid="popover" data-open={open}>
+        {children}
+        <button data-testid="popover-close" type="button" onClick={() => onOpenChange(false)}>
+          close
+        </button>
+      </div>
+    ),
+    PopoverTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+    PopoverContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+  };
+});
+
+vi.mock("@kandev/ui/tooltip", () => ({
+  Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+  TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+  TooltipContent: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+}));
+
+import { ValidatedPopover } from "./validated-popover";
+
+function deferred<T>() {
+  let resolve!: (v: T) => void;
+  let reject!: (e: unknown) => void;
+  const promise = new Promise<T>((res, rej) => {
+    resolve = res;
+    reject = rej;
+  });
+  return { promise, resolve, reject };
+}
+
+function renderHarness(overrides?: { fetch?: (key: string) => Promise<string> }) {
+  const onSuccess = vi.fn();
+  const result = render(
+    <ValidatedPopover
+      triggerStyle="ghost-icon"
+      triggerIcon={<span>icon</span>}
+      triggerAriaLabel="trigger"
+      tooltip="tip"
+      headline="Headline"
+      placeholder="paste here"
+      extractKey={(raw) => (raw.trim() ? raw.trim() : null)}
+      validationHint="hint"
+      fetch={overrides?.fetch ?? (async (k: string) => k)}
+      onSuccess={onSuccess}
+      submitLabel="Submit"
+      submittingLabel="Submitting..."
+    />,
+  );
+  return { ...result, onSuccess };
+}
+
+describe("ValidatedPopover", () => {
+  it("close-while-loading resets loading state and ignores the late resolution", async () => {
+    const d = deferred<string>();
+    const fetchSpy = vi.fn(async () => d.promise);
+    const { onSuccess } = renderHarness({ fetch: fetchSpy });
+
+    // Type a key + submit
+    fireEvent.change(screen.getByPlaceholderText("paste here"), { target: { value: "KEY-1" } });
+    fireEvent.click(screen.getByRole("button", { name: "Submit" }));
+
+    // Mid-flight: submitting label visible, button disabled.
+    await waitFor(() => {
+      const submitting = screen.getByRole("button", { name: "Submitting..." }) as HTMLButtonElement;
+      expect(submitting.disabled).toBe(true);
+    });
+
+    // User dismisses the popover before fetch resolves.
+    fireEvent.click(screen.getByTestId("popover-close"));
+
+    // Loading must reset on close so the next open starts clean.
+    await waitFor(() => {
+      expect(screen.queryByRole("button", { name: "Submitting..." })).toBeNull();
+    });
+
+    // Late resolution must not call onSuccess (the submission token was
+    // invalidated when we closed) and must not bring the popover back.
+    await act(async () => {
+      d.resolve("late");
+      await Promise.resolve();
+    });
+    expect(onSuccess).not.toHaveBeenCalled();
+  });
+
+  it("close-while-loading suppresses a late rejection rather than re-populating error", async () => {
+    const d = deferred<string>();
+    const fetchSpy = vi.fn(async () => d.promise);
+    renderHarness({ fetch: fetchSpy });
+
+    fireEvent.change(screen.getByPlaceholderText("paste here"), { target: { value: "KEY-1" } });
+    fireEvent.click(screen.getByRole("button", { name: "Submit" }));
+    await waitFor(() => {
+      const submitting = screen.getByRole("button", { name: "Submitting..." }) as HTMLButtonElement;
+      expect(submitting.disabled).toBe(true);
+    });
+
+    fireEvent.click(screen.getByTestId("popover-close"));
+
+    // Reject after close — the in-flight error must not leak into UI state.
+    await act(async () => {
+      d.reject(new Error("boom"));
+      await Promise.resolve().catch(() => {});
+    });
+
+    // role=alert is the inline error <p> in ValidatedPopover; should be absent.
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+});
diff --git a/apps/web/components/integrations/validated-popover.tsx b/apps/web/components/integrations/validated-popover.tsx
index cc90ba84c1..afaf95b5f0 100644
--- a/apps/web/components/integrations/validated-popover.tsx
+++ b/apps/web/components/integrations/validated-popover.tsx
@@ -1,6 +1,6 @@
 "use client";
 
-import { useCallback, useState, type ReactNode } from "react";
+import { useCallback, useRef, useState, type ReactNode } from "react";
 import { Button } from "@kandev/ui/button";
 import { Input } from "@kandev/ui/input";
 import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
@@ -105,6 +105,10 @@ export function ValidatedPopover<T>(props: ValidatedPopoverProps<T>) {
   const [value, setValue] = useState("");
   const [loading, setLoading] = useState(false);
   const [error, setError] = useState<string | null>(null);
+  // Submission token: bumped on every submit and on close, so an in-flight
+  // request whose promise resolves after the popover was closed (or
+  // re-opened) cannot leak loading/error state into the next session.
+  const submissionRef = useRef(0);
 
   const submit = useCallback(async () => {
     const key = extractKey(value.trim());
@@ -112,25 +116,33 @@ export function ValidatedPopover<T>(props: ValidatedPopoverProps<T>) {
       setError(validationHint);
       return;
     }
+    const submission = ++submissionRef.current;
     setLoading(true);
     setError(null);
     try {
       const result = await fetch(key);
+      if (submission !== submissionRef.current) return;
       onSuccess(key, result);
       setOpen(false);
       setValue("");
     } catch (err) {
+      if (submission !== submissionRef.current) return;
       setError(err instanceof Error ? err.message : String(err));
     } finally {
-      setLoading(false);
+      if (submission === submissionRef.current) setLoading(false);
     }
   }, [value, extractKey, validationHint, fetch, onSuccess]);
 
-  // Closing discards any stale validation/error so the next open starts
-  // clean rather than rehydrating yesterday's failure.
+  // Closing invalidates any in-flight submit (so a late rejection can't
+  // re-populate `error` after the user dismissed the popover) and clears
+  // local submit state so the next open starts clean.
   const handleOpenChange = (next: boolean) => {
     setOpen(next);
-    if (!next) setError(null);
+    if (!next) {
+      submissionRef.current += 1;
+      setLoading(false);
+      setError(null);
+    }
   };
 
   return (