fix(dashboard): bind the param-filter save to the draft's own target (#8910)

saveModelParamFilters guarded on paramDirtyRef alone and read the
providerId/modelId it closed over, never the target the draft was typed
for. ModelCompatPopover is not always keyed by a stable identity
(CompatibleModelsSection keys by `${alias}:${modelId}`,
PassthroughModelsSection by the full model string, and providerId is
threaded from route/page state), so a re-render can re-point a live,
mounted popover at a different provider/model. If the old target's save
had failed or never ran, the still-dirty draft was then PUT into the NEW
target — writing a filter list under a model/provider the user never
edited and destroying that target's real config.

Replace the dirty flag / revision counter / dirty-key trio with a single
ParamFilterDraft ref that carries the provider, model and both field
values captured at edit time. The save drives its GET, PUT and payload
from that draft instead of the current props, re-reads the ref after
each await (restarting the attempt if the draft was replaced by one for
another target), and only clears it when the exact draft object it wrote
is still pending. Object identity replaces the revision counter, keeping
the existing lost-update protection.

A load no longer clears the draft or the failure indicator: a draft
pending here belongs to another target and is still owed a write to it.
An orphaned draft is therefore neither dropped nor redirected — it keeps
its own provider/model, keeps the failure marker visible, and is retried
by the next blur/close/unmount save. The cleanup effect also depends on
the target key so re-pointing the popover flushes the old draft.

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

@@ -47,6 +47,17 @@ interface ParamFilterConfigLike {
   autoLearn?: boolean;
 }
 
+// An unsaved param-filter draft, bound to the provider/model it was typed for. The save path
+// writes through THIS target instead of the props the callback happens to close over, so a draft
+// can never be persisted under a provider/model the user never edited (#8910).
+interface ParamFilterDraft {
+  key: string;
+  providerId: string;
+  modelId: string;
+  block: string;
+  allow: string;
+}
+
 // Builds the PUT body for the model-level block/allow save. Extracted so the
 // caller's async handler stays simple — this is pure payload-shaping logic.
 function buildModelParamFilterPayload(
@@ -132,30 +143,40 @@ export default function ModelCompatPopover({
   const headerRowsRef = useRef<HeaderDraftRow[]>([]);
   headerRowsRef.current = headerRows;
 
-  // Param-filter drafts are mirrored into refs so the close/unmount save path reads the
+  // Param-filter drafts are mirrored into a ref so the close/unmount save path reads the
   // latest typed values instead of the values captured when the handler was created (#8910).
-  const paramDirtyRef = useRef(false);
   const paramSavingRef = useRef(false);
-  // Monotonic draft revision — bumped on every edit so an in-flight save can tell whether the
-  // draft it snapshotted is still the newest one (#8910 lost update).
-  const paramRevRef = useRef(0);
-  // Provider/model the currently displayed draft belongs to. Recorded when the draft is marked
-  // dirty — never derived from a completed load — so an unsaved draft is protected even when no
-  // load ever succeeded for that target (slow or failing initial GET, #8910).
-  const paramDirtyKeyRef = useRef<string | null>(null);
+  // The single unsaved draft, together with the provider/model it was typed for. Non-null means
+  // "dirty". Recorded when the draft is edited — never derived from a completed load — so an
+  // unsaved draft is protected even when no load ever succeeded for that target, and every write
+  // lands on the draft's own target rather than whatever the popover currently points at (#8910).
+  const paramDraftRef = useRef<ParamFilterDraft | null>(null);
+
+  const paramTargetKey = `${providerId}\u0000${modelId}`;
+  const paramTargetRef = useRef<{ key: string; providerId: string; modelId: string }>({
+    key: paramTargetKey,
+    providerId,
+    modelId,
+  });
+  paramTargetRef.current = { key: paramTargetKey, providerId, modelId };
+
+  // Mirrors of the displayed text, so an edit can snapshot both fields synchronously.
   const blockTextRef = useRef("");
   const allowTextRef = useRef("");
   blockTextRef.current = blockText;
   allowTextRef.current = allowText;
 
-  const paramTargetKey = `${providerId}\u0000${modelId}`;
-  const paramTargetKeyRef = useRef(paramTargetKey);
-  paramTargetKeyRef.current = paramTargetKey;
-
+  // Every edit replaces the draft with a fresh object bound to the CURRENT target. Object
+  // identity doubles as the draft revision an in-flight save compares against (#8910).
   const markParamDraftDirty = useCallback(() => {
-    paramDirtyRef.current = true;
-    paramDirtyKeyRef.current = paramTargetKeyRef.current;
-    paramRevRef.current += 1;
+    const target = paramTargetRef.current;
+    paramDraftRef.current = {
+      key: target.key,
+      providerId: target.providerId,
+      modelId: target.modelId,
+      block: blockTextRef.current,
+      allow: allowTextRef.current,
+    };
   }, []);
   const mountedRef = useRef(true);
   useEffect(() => {
@@ -211,8 +232,7 @@ export default function ModelCompatPopover({
     // A draft that is still dirty for this exact provider/model was never persisted (failed or
     // exhausted save). Reloading server state here would silently revert it — the very complaint
     // behind #8910 — so keep the draft on screen and let the user retry instead.
-    const draftIsDirtyForThisTarget = () =>
-      paramDirtyRef.current && paramDirtyKeyRef.current === draftKey;
+    const draftIsDirtyForThisTarget = () => paramDraftRef.current?.key === draftKey;
     if (draftIsDirtyForThisTarget()) return;
     let cancelled = false;
     (async () => {
@@ -227,11 +247,10 @@ export default function ModelCompatPopover({
         const modelCfg = data?.models?.[modelId];
         setBlockText(modelCfg ? (modelCfg.block ?? []).join(", ") : "");
         setAllowText(modelCfg ? (modelCfg.allow ?? []).join(", ") : "");
-        // Only the freshly loaded server state is clean — a failed load must not
-        // discard drafts the user already typed (#8910).
-        paramDirtyRef.current = false;
-        paramDirtyKeyRef.current = null;
-        setParamSaveFailed(false);
+        // A load never clears the draft: any draft still pending here belongs to a DIFFERENT
+        // target and is still owed a write to that target (#8910). The failure indicator is
+        // only cleared once nothing is left unsaved anywhere.
+        if (!paramDraftRef.current) setParamSaveFailed(false);
       } catch {
         // Keep whatever the user has in the fields (and its dirty flag) on load failure.
       }
@@ -242,8 +261,12 @@ export default function ModelCompatPopover({
     // Reload only when opening or when the popover targets a different provider/model.
   }, [open, providerId, modelId]);
 
+  // The save always writes through the draft's OWN provider/model — never the props this
+  // callback happens to be bound to — so a draft typed for target A can never be persisted under
+  // a target B the user never edited (#8910). The draft is re-read after every await for the same
+  // reason the load effect re-checks its guard: the target can change while the save is in flight.
   const saveModelParamFilters = useCallback(async () => {
-    if (!paramDirtyRef.current || paramSavingRef.current) return;
+    if (!paramDraftRef.current || paramSavingRef.current) return;
     paramSavingRef.current = true;
     if (mountedRef.current) setParamSaving(true);
     try {
@@ -251,26 +274,29 @@ export default function ModelCompatPopover({
       // before the PUT resolves, so a keystroke landing in that window would otherwise be
       // acknowledged (dirty cleared) but never persisted — the #8910 lost update.
       for (let attempt = 0; attempt < PARAM_SAVE_MAX_ATTEMPTS; attempt += 1) {
-        const rev = paramRevRef.current;
-        const res = await fetch(`/api/providers/${providerId}/param-filters`);
+        const draft = paramDraftRef.current;
+        if (!draft) return;
+        const res = await fetch(`/api/providers/${draft.providerId}/param-filters`);
         if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`);
         const current = await res.json();
+        // The fetched config belongs to draft.providerId; if the draft was replaced by one for
+        // another provider/model while the GET was in flight, restart with a matching GET.
+        if (paramDraftRef.current?.key !== draft.key) continue;
         const payload = buildModelParamFilterPayload(
           current,
-          modelId,
-          blockTextRef.current,
-          allowTextRef.current
+          draft.modelId,
+          draft.block,
+          draft.allow
         );
-        const putRes = await fetch(`/api/providers/${providerId}/param-filters`, {
+        const putRes = await fetch(`/api/providers/${draft.providerId}/param-filters`, {
           method: "PUT",
           headers: { "Content-Type": "application/json" },
           body: JSON.stringify(payload),
         });
         if (!putRes.ok) throw new Error(`param-filters PUT failed: ${putRes.status}`);
-        // Only the revision that was actually written may clear the dirty flag.
-        if (paramRevRef.current === rev) {
-          paramDirtyRef.current = false;
-          paramDirtyKeyRef.current = null;
+        // Only the exact draft that was written may clear the dirty flag.
+        if (paramDraftRef.current === draft) {
+          paramDraftRef.current = null;
           if (mountedRef.current) setParamSaveFailed(false);
           return;
         }
@@ -279,22 +305,25 @@ export default function ModelCompatPopover({
       // kept on reopen and the next blur/close retries it.
       if (mountedRef.current) setParamSaveFailed(true);
     } catch {
-      // Save failed — drafts and dirty state are intentionally preserved, and the failure is
-      // surfaced in the panel instead of being silently swallowed.
+      // Save failed — the draft (and the target it belongs to) is intentionally preserved, and
+      // the failure is surfaced in the panel instead of being silently swallowed. An orphaned
+      // draft whose target is no longer displayed is neither dropped nor redirected: it keeps its
+      // own provider/model and is retried by the next blur/close/unmount save.
       if (mountedRef.current) setParamSaveFailed(true);
     } finally {
       paramSavingRef.current = false;
       if (mountedRef.current) setParamSaving(false);
     }
-  }, [providerId, modelId]);
+  }, []);
 
-  // Persist pending param-filter drafts when the popover closes or unmounts (#8910).
+  // Persist pending param-filter drafts when the popover closes, unmounts, or is re-pointed at a
+  // different provider/model (#8910).
   useEffect(() => {
     if (!open) return;
     return () => {
       void saveModelParamFilters();
     };
-  }, [open, saveModelParamFilters]);
+  }, [open, paramTargetKey, saveModelParamFilters]);
 
   useEffect(() => {
     setValuePeekRowId(null);

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

@@ -0,0 +1,192 @@
+// @vitest-environment jsdom
+// Regression coverage for the cross-target write defect found while fixing #8910.
+//
+// ModelCompatPopover instances are not always keyed by a stable identity (CompatibleModelsSection
+// keys by `${alias}:${modelId}`, PassthroughModelsSection by the full model string, and providerId
+// is threaded from route/page state), so a re-render can re-point a LIVE, mounted popover at a
+// different provider/model. When the draft for the old target failed to save, the save callback —
+// now bound to the new target — used to PUT the old draft into the new target's config,
+// destructively overwriting a model/provider the user never edited.
+//
+// Contract asserted here: a write always lands on the provider/model the draft was typed for, and
+// an orphaned draft whose target is no longer displayed is preserved (retried later) rather than
+// silently dropped or redirected.
+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(rounds = 40) {
+  await act(async () => {
+    for (let i = 0; i < rounds; i += 1) await Promise.resolve();
+  });
+}
+
+function blockInput() {
+  return document.querySelector(
+    'input[placeholder="compatBlockedParamsPlaceholder"]'
+  ) as HTMLInputElement | null;
+}
+
+function renderPopover(props: { providerId: string; modelId: string }) {
+  act(() => {
+    root.render(
+      <ModelCompatPopover
+        t={(key) => key}
+        providerId={props.providerId}
+        modelId={props.modelId}
+        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 cross-target writes (#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(() => {
+    try {
+      act(() => root.unmount());
+    } catch {
+      // already unmounted by the test
+    }
+    document.body.innerHTML = "";
+    vi.unstubAllGlobals();
+  });
+
+  it("never writes a draft typed for one model under a different model", async () => {
+    const server: ParamFilterState = {
+      block: [],
+      allow: [],
+      models: { "model-b": { block: ["bval"], allow: [] } },
+      autoLearn: false,
+    };
+    const puts: { url: string; models?: Record<string, unknown> }[] = [];
+    let getFails = true;
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = String(input);
+        if (init?.method === "PUT") {
+          const body = JSON.parse(String(init.body));
+          puts.push({ url, models: body.models });
+          server.models = body.models ?? {};
+          return { ok: true, json: async () => ({ success: true }) } as Response;
+        }
+        if (getFails) return { ok: false, status: 503, json: async () => ({}) } as Response;
+        return { ok: true, json: async () => structuredClone(server) } as Response;
+      })
+    );
+
+    renderPopover({ providerId: "openai", modelId: "model-a" });
+    await openPopover();
+    // The load failed, so the fields are empty; the user types a draft for model-a.
+    await act(async () => setInputValue(blockInput()!, "aaa"));
+
+    // A re-render re-points this live popover at model-b while model-a's draft is still dirty.
+    // The close-time save for model-a runs here and fails (its GET is still 503).
+    renderPopover({ providerId: "openai", modelId: "model-b" });
+    await flushEffects();
+
+    // The network recovers and the popover closes: the retried save must target model-a.
+    getFails = false;
+    await closePopoverByOutsideClick();
+
+    // model-b's real server config is untouched...
+    expect(server.models["model-b"]).toEqual({ block: ["bval"], allow: [] });
+    // ...and the orphaned model-a draft is not silently dropped either — it lands on model-a.
+    expect(server.models["model-a"]).toEqual({ block: ["aaa"], allow: [] });
+    expect(puts.every((p) => p.url.includes("/openai/"))).toBe(true);
+  });
+
+  it("never writes a draft typed for one provider under a different provider", async () => {
+    const byProvider: Record<string, ParamFilterState> = {
+      alpha: { block: [], allow: [], models: {}, autoLearn: false },
+      beta: {
+        block: [],
+        allow: [],
+        models: { "gpt-test": { block: ["betaval"], allow: [] } },
+        autoLearn: false,
+      },
+    };
+    const puts: { providerId: string; models?: Record<string, unknown> }[] = [];
+    let getFails = true;
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = String(input);
+        const providerId = url.match(/providers\/([^/]+)\//)![1];
+        if (init?.method === "PUT") {
+          const body = JSON.parse(String(init.body));
+          puts.push({ providerId, models: body.models });
+          byProvider[providerId].models = body.models ?? {};
+          return { ok: true, json: async () => ({ success: true }) } as Response;
+        }
+        if (getFails) return { ok: false, status: 503, json: async () => ({}) } as Response;
+        return { ok: true, json: async () => structuredClone(byProvider[providerId]) } as Response;
+      })
+    );
+
+    renderPopover({ providerId: "alpha", modelId: "gpt-test" });
+    await openPopover();
+    await act(async () => setInputValue(blockInput()!, "alpha-only"));
+
+    // Re-point the live popover at provider beta while alpha's draft is dirty and its save fails.
+    renderPopover({ providerId: "beta", modelId: "gpt-test" });
+    await flushEffects();
+
+    getFails = false;
+    await closePopoverByOutsideClick();
+
+    // beta must receive no write at all; its stored config survives intact.
+    expect(puts.filter((p) => p.providerId === "beta")).toEqual([]);
+    expect(byProvider.beta.models).toEqual({ "gpt-test": { block: ["betaval"], allow: [] } });
+    // The alpha draft is preserved and eventually persisted under alpha.
+    expect(byProvider.alpha.models).toEqual({ "gpt-test": { block: ["alpha-only"], allow: [] } });
+  });
+});