fix(dashboard): persist model param-filter edits on popover close (#8910)

changelog.d/fixes/9013-model-param-filter-save.md

@@ -0,0 +1 @@
+- **fix(dashboard):** model-level allowed/blocked param edits now persist when the compatibility popover is closed by clicking outside, and a failed save no longer clears the edit or reports success ([#9013](https://github.com/diegosouzapw/OmniRoute/pull/9013))

config/quality/dashboard-typecheck-baseline.json

@@ -127,12 +127,6 @@
   "src/app/(dashboard)/dashboard/providers/[id]/components/ConnectionsListPanel.tsx": {
     "TS2322": 2
   },
-  "src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx": {
-    "TS2739": 1
-  },
-  "src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx": {
-    "TS2304": 5
-  },
   "src/app/(dashboard)/dashboard/providers/[id]/components/ProviderModalsPanel.tsx": {
     "TS2322": 3,
     "TS2739": 1,
@@ -147,9 +141,6 @@
   "src/app/(dashboard)/dashboard/providers/[id]/components/ProviderPlaygroundPanel.tsx": {
     "TS2503": 1
   },
-  "src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/phase1d.test.tsx": {
-    "TS2739": 2
-  },
   "src/app/(dashboard)/dashboard/providers/[id]/components/modals/EditConnectionModal.tsx": {
     "TS2322": 1
   },

src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx

@@ -695,6 +695,8 @@ export default function CustomModelsSection({
                   </button>
                   <ModelCompatPopover
                     t={t}
+                    providerId={providerId}
+                    modelId={model.id!}
                     effectiveModelNormalize={(p) =>
                       effectiveNormalizeForProtocol(model.id!, p, customMap, overrideMap)
                     }

src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-concurrency.test.tsx

@@ -0,0 +1,208 @@
+// @vitest-environment jsdom
+// Regression coverage for the concurrency defects found while fixing #8910:
+//   1. an edit landing after the PUT payload snapshot but before the PUT resolves must still
+//      be persisted (lost update);
+//   2. a save that failed must not be silently reverted on reopen, and the failure must be
+//      visible in the panel.
+import React, { act } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import ModelCompatPopover from "../ModelCompatPopover";
+
+vi.mock("next-intl", () => ({
+  useTranslations: () => (key: string) => key,
+}));
+
+interface ParamFilterState {
+  block: string[];
+  allow: string[];
+  models?: Record<string, { block: string[]; allow: string[] }>;
+  autoLearn: boolean;
+}
+
+let container: HTMLDivElement;
+let root: Root;
+
+function setInputValue(input: HTMLInputElement, value: string) {
+  const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value")?.set;
+  setter?.call(input, value);
+  input.dispatchEvent(new Event("input", { bubbles: true }));
+}
+
+async function flushEffects() {
+  await act(async () => {
+    for (let i = 0; i < 8; i += 1) await Promise.resolve();
+  });
+}
+
+function blockInput() {
+  return document.querySelector(
+    'input[placeholder="compatBlockedParamsPlaceholder"]'
+  ) as HTMLInputElement | null;
+}
+
+function renderPopover() {
+  act(() => {
+    root.render(
+      <ModelCompatPopover
+        t={(key) => key}
+        providerId="openai"
+        modelId="gpt-test"
+        effectiveModelNormalize={() => false}
+        effectiveModelPreserveDeveloper={() => true}
+        getUpstreamHeadersRecord={() => ({})}
+        onCompatPatch={vi.fn()}
+      />
+    );
+  });
+}
+
+async function openPopover() {
+  const trigger = container.querySelector("button") as HTMLButtonElement;
+  await act(async () => trigger.click());
+  await flushEffects();
+}
+
+async function closePopoverByOutsideClick() {
+  await act(async () => {
+    document.body.dispatchEvent(new MouseEvent("mousedown", { bubbles: true }));
+  });
+  await flushEffects();
+}
+
+describe("ModelCompatPopover param-filter save concurrency (#8910)", () => {
+  beforeEach(() => {
+    (
+      globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }
+    ).IS_REACT_ACT_ENVIRONMENT = true;
+    container = document.createElement("div");
+    document.body.appendChild(container);
+    root = createRoot(container);
+  });
+
+  afterEach(() => {
+    act(() => root.unmount());
+    document.body.innerHTML = "";
+    vi.unstubAllGlobals();
+  });
+
+  it("persists an edit typed after the payload snapshot but before the PUT resolves", async () => {
+    let serverState: ParamFilterState = { block: [], allow: [], models: {}, autoLearn: false };
+    let releasePut: (() => void) | null = null;
+    let holdPut = false;
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
+        if (init?.method === "PUT") {
+          serverState = JSON.parse(String(init.body)) as ParamFilterState;
+          if (holdPut) {
+            await new Promise<void>((resolve) => {
+              releasePut = resolve;
+            });
+          }
+          return { ok: true, json: async () => ({ success: true }) } as Response;
+        }
+        return { ok: true, json: async () => structuredClone(serverState) } as Response;
+      })
+    );
+
+    renderPopover();
+    await openPopover();
+
+    await act(async () => setInputValue(blockInput()!, "temperature"));
+    holdPut = true;
+    // Blur starts the save: GET resolves, payload is snapshotted, PUT is issued and held open.
+    await act(async () => {
+      blockInput()!.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
+    });
+    await flushEffects();
+    expect(releasePut).not.toBeNull();
+
+    // The user keeps typing while that PUT is still in flight, then closes the popover.
+    await act(async () => setInputValue(blockInput()!, "temperature, seed"));
+    await closePopoverByOutsideClick();
+
+    holdPut = false;
+    await act(async () => {
+      releasePut?.();
+      await Promise.resolve();
+    });
+    await flushEffects();
+
+    expect(serverState.models).toEqual({
+      "gpt-test": { block: ["temperature", "seed"], allow: [] },
+    });
+  });
+
+  it("keeps the draft and surfaces the failure when the save fails, instead of reverting on reopen", async () => {
+    const serverState: ParamFilterState = { block: [], allow: [], models: {}, autoLearn: false };
+    let putAttempts = 0;
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
+        if (init?.method === "PUT") {
+          putAttempts += 1;
+          return { ok: false, status: 500, json: async () => ({}) } as Response;
+        }
+        return { ok: true, json: async () => structuredClone(serverState) } as Response;
+      })
+    );
+
+    renderPopover();
+    await openPopover();
+
+    await act(async () => setInputValue(blockInput()!, "temperature"));
+    await closePopoverByOutsideClick();
+    expect(putAttempts).toBe(1);
+
+    // Reopening must NOT clobber the unsaved draft with server state (#8910 complaint class).
+    await openPopover();
+    expect(blockInput()!.value).toBe("temperature");
+    // The failure is visible to the user rather than silently swallowed.
+    expect(document.querySelector('[role="alert"]')?.textContent).toContain("failed");
+
+    // ...and the retained draft is actually retryable through the normal close path.
+    await closePopoverByOutsideClick();
+    expect(putAttempts).toBe(2);
+  });
+
+  it("clears the failure indicator once a later save succeeds", async () => {
+    let serverState: ParamFilterState = { block: [], allow: [], models: {}, autoLearn: false };
+    let failNextPut = true;
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
+        if (init?.method === "PUT") {
+          if (failNextPut) return { ok: false, status: 500, json: async () => ({}) } as Response;
+          serverState = JSON.parse(String(init.body)) as ParamFilterState;
+          return { ok: true, json: async () => ({ success: true }) } as Response;
+        }
+        return { ok: true, json: async () => structuredClone(serverState) } as Response;
+      })
+    );
+
+    renderPopover();
+    await openPopover();
+    await act(async () => setInputValue(blockInput()!, "temperature"));
+    await act(async () => {
+      blockInput()!.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
+    });
+    await flushEffects();
+    expect(document.querySelector('[role="alert"]')).not.toBeNull();
+
+    failNextPut = false;
+    await act(async () => setInputValue(blockInput()!, "temperature, seed"));
+    await act(async () => {
+      blockInput()!.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
+    });
+    await flushEffects();
+
+    expect(document.querySelector('[role="alert"]')).toBeNull();
+    expect(serverState.models).toEqual({
+      "gpt-test": { block: ["temperature", "seed"], allow: [] },
+    });
+  });
+});

src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-instance.test.tsx

@@ -0,0 +1,137 @@
+// @vitest-environment jsdom
+// Regression coverage for #8910: sibling model-row popovers for the same provider must serialize
+// their whole-document param-filter updates so one successful save cannot erase the other.
+import React, { act } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, expect, it, vi } from "vitest";
+import ModelCompatPopover from "../ModelCompatPopover";
+
+vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key }));
+
+type ParamFilterState = {
+  block: string[];
+  allow: string[];
+  models?: Record<string, { block: string[]; allow: string[] }>;
+  autoLearn: boolean;
+};
+
+let container: HTMLDivElement;
+let root: Root;
+let releaseFirstPut: (() => void) | null;
+
+function setInputValue(input: HTMLInputElement, value: string) {
+  const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value")?.set;
+  setter?.call(input, value);
+  input.dispatchEvent(new Event("input", { bubbles: true }));
+}
+
+async function flushEffects(rounds = 60) {
+  await act(async () => {
+    for (let i = 0; i < rounds; i += 1) await Promise.resolve();
+  });
+}
+
+beforeEach(() => {
+  (
+    globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }
+  ).IS_REACT_ACT_ENVIRONMENT = true;
+  container = document.createElement("div");
+  document.body.appendChild(container);
+  root = createRoot(container);
+  releaseFirstPut = null;
+});
+
+afterEach(async () => {
+  await act(async () => {
+    releaseFirstPut?.();
+    await Promise.resolve();
+  });
+  act(() => root.unmount());
+  document.body.innerHTML = "";
+  vi.unstubAllGlobals();
+});
+
+it("preserves both model updates when sibling popovers save the same provider concurrently", async () => {
+  let server: ParamFilterState = { block: [], allow: [], models: {}, autoLearn: false };
+  let putCount = 0;
+
+  vi.stubGlobal(
+    "fetch",
+    vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
+      if (init?.method === "PUT") {
+        putCount += 1;
+        const body = JSON.parse(String(init.body)) as ParamFilterState;
+        if (putCount === 1) {
+          await new Promise<void>((resolve) => {
+            releaseFirstPut = resolve;
+          });
+        }
+        server = { ...body, models: body.models ?? {} };
+        return { ok: true, json: async () => ({ success: true }) } as Response;
+      }
+      return { ok: true, json: async () => structuredClone(server) } as Response;
+    })
+  );
+
+  const props = (modelId: string) => ({
+    t: (key: string) => key,
+    providerId: "openai",
+    modelId,
+    effectiveModelNormalize: () => false,
+    effectiveModelPreserveDeveloper: () => true,
+    getUpstreamHeadersRecord: () => ({}),
+    onCompatPatch: vi.fn(),
+  });
+
+  act(() => {
+    root.render(
+      <div>
+        <div id="model-a">
+          <ModelCompatPopover {...props("model-a")} />
+        </div>
+        <div id="model-b">
+          <ModelCompatPopover {...props("model-b")} />
+        </div>
+      </div>
+    );
+  });
+
+  const triggerA = container.querySelector("#model-a button") as HTMLButtonElement;
+  const triggerB = container.querySelector("#model-b button") as HTMLButtonElement;
+  const blockInput = () =>
+    document.querySelector(
+      'input[placeholder="compatBlockedParamsPlaceholder"]'
+    ) as HTMLInputElement;
+
+  await act(async () => triggerA.click());
+  await flushEffects();
+  await act(async () => setInputValue(blockInput(), "aaa"));
+  await act(async () => {
+    blockInput().dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
+  });
+  await flushEffects();
+  expect(releaseFirstPut).not.toBeNull();
+
+  await act(async () => {
+    triggerB.dispatchEvent(new MouseEvent("mousedown", { bubbles: true }));
+  });
+  await flushEffects();
+  await act(async () => triggerB.click());
+  await flushEffects();
+  await act(async () => setInputValue(blockInput(), "bbb"));
+  await act(async () => {
+    blockInput().dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
+  });
+  await flushEffects();
+
+  await act(async () => {
+    releaseFirstPut?.();
+    await Promise.resolve();
+  });
+  await flushEffects();
+
+  expect(server.models).toEqual({
+    "model-a": { block: ["aaa"], allow: [] },
+    "model-b": { block: ["bbb"], allow: [] },
+  });
+});