From ae3d026ad062607f9bdf775a9ea99bdc687bed4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 00:48:39 +0800 Subject: [PATCH 1/9] test: reproduce model param filter close persistence --- .../modelCompatPopover-param-filters.test.tsx | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filters.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filters.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filters.test.tsx new file mode 100644 index 0000000000..1349af1925 --- /dev/null +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filters.test.tsx @@ -0,0 +1,146 @@ +// @vitest-environment jsdom +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; + 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 () => { + await Promise.resolve(); + }); +} + +async function openPopover() { + const trigger = container.querySelector("button") as HTMLButtonElement; + await act(async () => trigger.click()); + await flushEffects(); +} + +describe("ModelCompatPopover model param filters (#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 the latest block and allow drafts when an outside mousedown closes the popover", async () => { + const apiPath = "/api/providers/openai/param-filters"; + let serverState: ParamFilterState = { + block: ["provider-block"], + allow: ["provider-allow"], + models: { + "other-model": { block: ["keep-block"], allow: ["keep-allow"] }, + }, + autoLearn: true, + }; + const putBodies: unknown[] = []; + const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + if (String(input) !== apiPath) throw new Error(`Unexpected param-filter URL: ${input}`); + if (init?.method === "PUT") { + const body = JSON.parse(String(init.body)) as ParamFilterState; + putBodies.push(body); + serverState = body; + } + return { + ok: true, + json: async () => structuredClone(serverState), + } as Response; + }); + vi.stubGlobal("fetch", fetchMock); + + act(() => { + root.render( + key} + providerId="openai" + modelId="gpt-test" + effectiveModelNormalize={() => false} + effectiveModelPreserveDeveloper={() => true} + getUpstreamHeadersRecord={() => ({})} + onCompatPatch={vi.fn()} + /> + ); + }); + + await openPopover(); + const blockInput = document.querySelector( + 'input[placeholder="compatBlockedParamsPlaceholder"]' + ) as HTMLInputElement; + const allowInput = document.querySelector( + 'input[placeholder="compatAllowedParamsPlaceholder"]' + ) as HTMLInputElement; + + await act(async () => { + setInputValue(blockInput, "temperature, top_p"); + setInputValue(allowInput, "tools, response_format"); + }); + + await act(async () => { + document.body.dispatchEvent(new MouseEvent("mousedown", { bubbles: true })); + }); + await flushEffects(); + + expect( + document.querySelector('input[placeholder="compatBlockedParamsPlaceholder"]') + ).toBeNull(); + expect(putBodies).toEqual([ + { + block: ["provider-block"], + allow: ["provider-allow"], + models: { + "other-model": { block: ["keep-block"], allow: ["keep-allow"] }, + "gpt-test": { + block: ["temperature", "top_p"], + allow: ["tools", "response_format"], + }, + }, + autoLearn: true, + }, + ]); + + await openPopover(); + expect( + ( + document.querySelector( + 'input[placeholder="compatBlockedParamsPlaceholder"]' + ) as HTMLInputElement + ).value + ).toBe("temperature, top_p"); + expect( + ( + document.querySelector( + 'input[placeholder="compatAllowedParamsPlaceholder"]' + ) as HTMLInputElement + ).value + ).toBe("tools, response_format"); + }); +}); From 152a3e13f76d7c7fed7457923665826ce9f771f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 03:14:41 +0800 Subject: [PATCH 2/9] fix(dashboard): persist model param filters on popover close (#8910) ModelCompatPopover declared providerId/modelId in its props type but never destructured them, so both param-filter fetches referenced undefined identifiers (TS2304, frozen in the dashboard-typecheck baseline) and threw into a silent catch. CustomModelsSection also never passed the two props. - Destructure providerId/modelId; pass them from CustomModelsSection. - Save pending block/allow drafts when the popover closes or unmounts, so an outside mousedown no longer discards them. - Read drafts from refs at save time and guard concurrent saves, avoiding stale-closure payloads and duplicate PUTs. - Keep dirty state and drafts on non-OK/failed GET or PUT instead of silently clearing them; skip state updates after unmount. - Provider-level block/allow, autoLearn, and other model entries are preserved; an empty block+allow still removes only the selected model entry. - Ratchet the three now-clean dashboard-typecheck baseline entries. Compat-toggle and upstream-header paths are unchanged. --- .../quality/dashboard-typecheck-baseline.json | 9 --- .../[id]/components/CustomModelsSection.tsx | 2 + .../[id]/components/ModelCompatPopover.tsx | 75 +++++++++++++++---- .../components/__tests__/phase1d.test.tsx | 4 + 4 files changed, 66 insertions(+), 24 deletions(-) diff --git a/config/quality/dashboard-typecheck-baseline.json b/config/quality/dashboard-typecheck-baseline.json index 18cd279581..b97762d325 100644 --- a/config/quality/dashboard-typecheck-baseline.json +++ b/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 }, diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx index dd2e842159..d464bef8ef 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx @@ -695,6 +695,8 @@ export default function CustomModelsSection({ effectiveNormalizeForProtocol(model.id!, p, customMap, overrideMap) } diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index 44c0708325..3e8b4e9088 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -96,6 +96,8 @@ export interface ModelCompatPopoverProps { export default function ModelCompatPopover({ t, + providerId, + modelId, effectiveModelNormalize, effectiveModelPreserveDeveloper, getUpstreamHeadersRecord, @@ -109,7 +111,6 @@ export default function ModelCompatPopover({ const [headerRows, setHeaderRows] = useState([]); const [blockText, setBlockText] = useState(""); const [allowText, setAllowText] = useState(""); - const [paramDirty, setParamDirty] = useState(false); const [paramSaving, setParamSaving] = useState(false); const [valuePeekRowId, setValuePeekRowId] = useState(null); const [valueFocusRowId, setValueFocusRowId] = useState(null); @@ -125,6 +126,22 @@ export default function ModelCompatPopover({ const headerRowsRef = useRef([]); headerRowsRef.current = headerRows; + // Param-filter drafts are mirrored into refs 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); + const blockTextRef = useRef(""); + const allowTextRef = useRef(""); + blockTextRef.current = blockText; + allowTextRef.current = allowText; + const mountedRef = useRef(true); + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + }; + }, []); + const genHeaderRowId = () => { headerRowIdRef.current += 1; return `uh-${headerRowIdRef.current}`; @@ -167,40 +184,66 @@ export default function ModelCompatPopover({ // Load model-level block/allow from param-filters API useEffect(() => { if (!open) return; + let cancelled = false; (async () => { try { const res = await fetch(`/api/providers/${providerId}/param-filters`); + if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`); const data = await res.json(); + if (cancelled || !mountedRef.current) return; 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; } catch { - setBlockText(""); - setAllowText(""); + // Keep whatever the user has in the fields (and its dirty flag) on load failure. } - setParamDirty(false); })(); - }, [open]); + return () => { + cancelled = true; + }; + // Reload only when opening or when the popover targets a different provider/model. + }, [open, providerId, modelId]); const saveModelParamFilters = useCallback(async () => { - if (!paramDirty) return; - setParamSaving(true); + if (!paramDirtyRef.current || paramSavingRef.current) return; + paramSavingRef.current = true; + if (mountedRef.current) setParamSaving(true); try { const res = await fetch(`/api/providers/${providerId}/param-filters`); + if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`); const current = await res.json(); - const payload = buildModelParamFilterPayload(current, modelId, blockText, allowText); - await fetch(`/api/providers/${providerId}/param-filters`, { + const payload = buildModelParamFilterPayload( + current, + modelId, + blockTextRef.current, + allowTextRef.current + ); + const putRes = await fetch(`/api/providers/${providerId}/param-filters`, { method: "PUT", headers: { "Content-Type": "application/json" }, body: JSON.stringify(payload), }); - setParamDirty(false); + if (!putRes.ok) throw new Error(`param-filters PUT failed: ${putRes.status}`); + // Stay dirty unless the write actually succeeded, so a later close can retry. + paramDirtyRef.current = false; } catch { - // Silently ignore save error + // Save failed — drafts and dirty state are intentionally preserved. } finally { - setParamSaving(false); + paramSavingRef.current = false; + if (mountedRef.current) setParamSaving(false); } - }, [paramDirty, blockText, allowText]); + }, [providerId, modelId]); + + // Persist pending param-filter drafts when the popover closes or unmounts (#8910). + useEffect(() => { + if (!open) return; + return () => { + void saveModelParamFilters(); + }; + }, [open, saveModelParamFilters]); useEffect(() => { setValuePeekRowId(null); @@ -361,7 +404,8 @@ export default function ModelCompatPopover({ value={blockText} onChange={(e) => { setBlockText(e.target.value); - setParamDirty(true); + blockTextRef.current = e.target.value; + paramDirtyRef.current = true; }} onBlur={() => saveModelParamFilters()} placeholder={t("compatBlockedParamsPlaceholder")} @@ -379,7 +423,8 @@ export default function ModelCompatPopover({ value={allowText} onChange={(e) => { setAllowText(e.target.value); - setParamDirty(true); + allowTextRef.current = e.target.value; + paramDirtyRef.current = true; }} onBlur={() => saveModelParamFilters()} placeholder={t("compatAllowedParamsPlaceholder")} diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/phase1d.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/phase1d.test.tsx index 577b292c4b..d559b8d23b 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/phase1d.test.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/phase1d.test.tsx @@ -177,6 +177,8 @@ describe("phase-1d extractions (#3501)", () => { const c = renderComponent( k} + providerId="openai" + modelId="gpt-test" effectiveModelNormalize={() => false} effectiveModelPreserveDeveloper={() => true} getUpstreamHeadersRecord={() => ({})} @@ -190,6 +192,8 @@ describe("phase-1d extractions (#3501)", () => { const c = renderComponent( k} + providerId="openai" + modelId="gpt-test" effectiveModelNormalize={() => true} effectiveModelPreserveDeveloper={() => false} getUpstreamHeadersRecord={() => ({ "X-Custom": "value" })} From 59d5f43d7b4463fa73b24b45a795596109ecfabb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 03:43:52 +0800 Subject: [PATCH 3/9] fix(dashboard): avoid lost update and surface failed param-filter saves (#8910) The close-time save could clear the dirty flag for a payload snapshotted before the PUT resolved, silently discarding any keystroke that landed in that window. Track a monotonic draft revision and only acknowledge the revision that was actually written, re-running the save (bounded) otherwise. A failed save previously stayed dirty to 'retry on a later close', but reopening the popover reloaded server state and silently reverted the draft. Keep a dirty draft for the same provider/model on reopen and show a failure marker next to the saving indicator instead. --- .../[id]/components/ModelCompatPopover.tsx | 88 ++++++-- ...tPopover-param-filter-concurrency.test.tsx | 208 ++++++++++++++++++ 2 files changed, 276 insertions(+), 20 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-concurrency.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index 3e8b4e9088..daafc791eb 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -26,6 +26,11 @@ function recordToHeaderRows(rec: Record, genId: () => string): H return entries.map(([name, value]) => ({ id: genId(), name, value })); } +// Bounded re-run budget for the model param-filter save: if the draft changed while the PUT was +// in flight, the save repeats with the newer draft instead of clearing the dirty flag on a +// payload that no longer matches what the user typed (#8910). +const PARAM_SAVE_MAX_ATTEMPTS = 3; + function parseCommaList(text: string): string[] { return text ? text @@ -112,6 +117,7 @@ export default function ModelCompatPopover({ const [blockText, setBlockText] = useState(""); const [allowText, setAllowText] = useState(""); const [paramSaving, setParamSaving] = useState(false); + const [paramSaveFailed, setParamSaveFailed] = useState(false); const [valuePeekRowId, setValuePeekRowId] = useState(null); const [valueFocusRowId, setValueFocusRowId] = useState(null); const ref = useRef(null); @@ -130,10 +136,21 @@ export default function ModelCompatPopover({ // 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 was loaded for, so a reopen of the same target + // does not clobber a draft that is still pending a (previously failed) save. + const paramLoadedKeyRef = useRef(null); const blockTextRef = useRef(""); const allowTextRef = useRef(""); blockTextRef.current = blockText; allowTextRef.current = allowText; + + const markParamDraftDirty = useCallback(() => { + paramDirtyRef.current = true; + paramRevRef.current += 1; + }, []); const mountedRef = useRef(true); useEffect(() => { mountedRef.current = true; @@ -184,6 +201,11 @@ export default function ModelCompatPopover({ // Load model-level block/allow from param-filters API useEffect(() => { if (!open) return; + const draftKey = `${providerId}\u0000${modelId}`; + // 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. + if (paramDirtyRef.current && paramLoadedKeyRef.current === draftKey) return; let cancelled = false; (async () => { try { @@ -197,6 +219,8 @@ export default function ModelCompatPopover({ // Only the freshly loaded server state is clean — a failed load must not // discard drafts the user already typed (#8910). paramDirtyRef.current = false; + paramLoadedKeyRef.current = draftKey; + setParamSaveFailed(false); } catch { // Keep whatever the user has in the fields (and its dirty flag) on load failure. } @@ -212,25 +236,40 @@ export default function ModelCompatPopover({ paramSavingRef.current = true; if (mountedRef.current) setParamSaving(true); try { - const res = await fetch(`/api/providers/${providerId}/param-filters`); - if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`); - const current = await res.json(); - const payload = buildModelParamFilterPayload( - current, - modelId, - blockTextRef.current, - allowTextRef.current - ); - const putRes = await fetch(`/api/providers/${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}`); - // Stay dirty unless the write actually succeeded, so a later close can retry. - paramDirtyRef.current = false; + // Re-run while the draft changed under the in-flight write: the payload is snapshotted + // 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`); + if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`); + const current = await res.json(); + const payload = buildModelParamFilterPayload( + current, + modelId, + blockTextRef.current, + allowTextRef.current + ); + const putRes = await fetch(`/api/providers/${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; + if (mountedRef.current) setParamSaveFailed(false); + return; + } + } + // Budget exhausted (the user is still typing): stay dirty and flag it, so the draft is + // 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. + // Save failed — drafts and dirty state are intentionally preserved, and the failure is + // surfaced in the panel instead of being silently swallowed. + if (mountedRef.current) setParamSaveFailed(true); } finally { paramSavingRef.current = false; if (mountedRef.current) setParamSaving(false); @@ -405,7 +444,7 @@ export default function ModelCompatPopover({ onChange={(e) => { setBlockText(e.target.value); blockTextRef.current = e.target.value; - paramDirtyRef.current = true; + markParamDraftDirty(); }} onBlur={() => saveModelParamFilters()} placeholder={t("compatBlockedParamsPlaceholder")} @@ -415,6 +454,15 @@ export default function ModelCompatPopover({

{t("compatBlockedParamsHint") ?? "Blocked params (stripped from requests)"} {paramSaving && ` ● ${t("compatSaving")}`} + {paramSaveFailed && !paramSaving && ( + + ● {t("failed")} + + )}

@@ -424,7 +472,7 @@ export default function ModelCompatPopover({ onChange={(e) => { setAllowText(e.target.value); allowTextRef.current = e.target.value; - paramDirtyRef.current = true; + markParamDraftDirty(); }} onBlur={() => saveModelParamFilters()} placeholder={t("compatAllowedParamsPlaceholder")} diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-concurrency.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-concurrency.test.tsx new file mode 100644 index 0000000000..713c374895 --- /dev/null +++ b/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; + 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( + 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((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: [] }, + }); + }); +}); From 53066a592a1121f550b66f41e90938ddf04f9b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 05:49:32 +0800 Subject: [PATCH 4/9] fix(dashboard): protect dirty param-filter drafts from load-effect clobber (#8910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retained-draft guard in the param-filters load effect required paramLoadedKeyRef to match the current target, but that ref was only assigned after a successful GET. Any draft typed before a successful load for that target was therefore unguarded, and the clean-slate write overwrote both the text and the dirty flag: - a draft typed while the INITIAL load GET was still in flight was overwritten and its dirty flag cleared, so the close-path save became a no-op and the keystrokes vanished with no feedback; - after a FAILED initial load, the retained draft was destroyed by the next successful reopen load — the exact moment the user reopens to retry — and the failure indicator was cleared as if the save had succeeded. Track the target on the dirty flag itself (paramDirtyKeyRef, set when the draft is marked dirty) instead of deriving it from a completed load, and re-check the guard after the GET await so a load result never overwrites text, clears dirty, or clears the failure indicator for a draft that is not on the server. --- .../[id]/components/ModelCompatPopover.tsx | 22 ++- ...Popover-param-filter-load-clobber.test.tsx | 186 ++++++++++++++++++ 2 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-load-clobber.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index daafc791eb..c4dabf44b7 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -139,16 +139,22 @@ export default function ModelCompatPopover({ // 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 was loaded for, so a reopen of the same target - // does not clobber a draft that is still pending a (previously failed) save. - const paramLoadedKeyRef = useRef(null); + // 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(null); const blockTextRef = useRef(""); const allowTextRef = useRef(""); blockTextRef.current = blockText; allowTextRef.current = allowText; + const paramTargetKey = `${providerId}\u0000${modelId}`; + const paramTargetKeyRef = useRef(paramTargetKey); + paramTargetKeyRef.current = paramTargetKey; + const markParamDraftDirty = useCallback(() => { paramDirtyRef.current = true; + paramDirtyKeyRef.current = paramTargetKeyRef.current; paramRevRef.current += 1; }, []); const mountedRef = useRef(true); @@ -205,7 +211,9 @@ 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. - if (paramDirtyRef.current && paramLoadedKeyRef.current === draftKey) return; + const draftIsDirtyForThisTarget = () => + paramDirtyRef.current && paramDirtyKeyRef.current === draftKey; + if (draftIsDirtyForThisTarget()) return; let cancelled = false; (async () => { try { @@ -213,13 +221,16 @@ export default function ModelCompatPopover({ if (!res.ok) throw new Error(`param-filters GET failed: ${res.status}`); const data = await res.json(); if (cancelled || !mountedRef.current) return; + // Re-check after the await: the user may have typed while the GET was in flight, and a + // load result must never overwrite (or acknowledge) a draft that is not on the server. + if (draftIsDirtyForThisTarget()) return; 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; - paramLoadedKeyRef.current = draftKey; + paramDirtyKeyRef.current = null; setParamSaveFailed(false); } catch { // Keep whatever the user has in the fields (and its dirty flag) on load failure. @@ -259,6 +270,7 @@ export default function ModelCompatPopover({ // Only the revision that was actually written may clear the dirty flag. if (paramRevRef.current === rev) { paramDirtyRef.current = false; + paramDirtyKeyRef.current = null; if (mountedRef.current) setParamSaveFailed(false); return; } diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-load-clobber.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-load-clobber.test.tsx new file mode 100644 index 0000000000..02210ea687 --- /dev/null +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-load-clobber.test.tsx @@ -0,0 +1,186 @@ +// @vitest-environment jsdom +// Regression coverage for the load-effect clobber defects found while fixing #8910: +// 1. a draft typed while the initial load GET is still in flight must survive the load +// result and still be persisted on close (otherwise keystrokes vanish silently); +// 2. after a failed initial load, the retained draft must not be destroyed by the next +// successful reopen load — that reopen is the user's retry. +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; + 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 = 12) { + 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() { + act(() => { + root.render( + 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 load clobber (#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("keeps and saves a draft typed while the initial load GET is still in flight", async () => { + const serverState: ParamFilterState = { + block: [], + allow: [], + models: { "gpt-test": { block: ["old"], allow: [] } }, + autoLearn: false, + }; + const puts: ParamFilterState[] = []; + let releaseGet: (() => void) | null = null; + let heldFirstGet = false; + + vi.stubGlobal( + "fetch", + vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + puts.push(JSON.parse(String(init.body)) as ParamFilterState); + return { ok: true, json: async () => ({ success: true }) } as Response; + } + if (!heldFirstGet) { + heldFirstGet = true; + await new Promise((resolve) => { + releaseGet = resolve; + }); + } + return { ok: true, json: async () => structuredClone(serverState) } as Response; + }) + ); + + renderPopover(); + const trigger = container.querySelector("button") as HTMLButtonElement; + await act(async () => trigger.click()); + await flushEffects(); + + // The field is already rendered while the load GET is still pending — the user types into it. + await act(async () => setInputValue(blockInput()!, "temperature")); + await act(async () => { + releaseGet?.(); + await Promise.resolve(); + }); + await flushEffects(); + + // The load result must not overwrite the dirty draft. + expect(blockInput()!.value).toBe("temperature"); + + await closePopoverByOutsideClick(); + + expect(puts.length).toBe(1); + expect(puts[0]?.models).toEqual({ "gpt-test": { block: ["temperature"], allow: [] } }); + }); + + it("does not clobber a retained draft with the successful reopen load after a failed initial load", async () => { + const serverState: ParamFilterState = { + block: [], + allow: [], + models: { "gpt-test": { block: ["serverval"], allow: [] } }, + autoLearn: false, + }; + const puts: ParamFilterState[] = []; + let failGet = true; + + vi.stubGlobal( + "fetch", + vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + puts.push(JSON.parse(String(init.body)) as ParamFilterState); + return { ok: true, json: async () => ({ success: true }) } as Response; + } + if (failGet) return { ok: false, status: 503, json: async () => ({}) } as Response; + return { ok: true, json: async () => structuredClone(serverState) } as Response; + }) + ); + + renderPopover(); + // First open: the load GET fails, so nothing is loaded and the field stays empty. + await openPopover(); + expect(blockInput()!.value).toBe(""); + + await act(async () => setInputValue(blockInput()!, "temperature")); + // Close: the save's own GET still fails, so the draft is retained and flagged as failed. + await closePopoverByOutsideClick(); + expect(puts.length).toBe(0); + + // The network recovers and the user reopens the popover to retry the save. + failGet = false; + await openPopover(); + expect(blockInput()!.value).toBe("temperature"); + // The unsaved-state indicator must still be visible — nothing was persisted. + expect(document.querySelector('[role="alert"]')).not.toBeNull(); + + await closePopoverByOutsideClick(); + expect(puts.length).toBe(1); + expect(puts[0]?.models).toEqual({ "gpt-test": { block: ["temperature"], allow: [] } }); + }); +}); From ecd1114894a879b2791799f7f249bd9875fe73a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 06:23:25 +0800 Subject: [PATCH 5/9] fix(dashboard): bind the param-filter save to the draft's own target (#8910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../[id]/components/ModelCompatPopover.tsx | 107 ++++++---- ...Popover-param-filter-cross-target.test.tsx | 192 ++++++++++++++++++ 2 files changed, 260 insertions(+), 39 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-target.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index c4dabf44b7..a293b24f06 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/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([]); 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(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(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); diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-target.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-target.test.tsx new file mode 100644 index 0000000000..fd0bfc575f --- /dev/null +++ b/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; + 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( + 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 }[] = []; + 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 = { + alpha: { block: [], allow: [], models: {}, autoLearn: false }, + beta: { + block: [], + allow: [], + models: { "gpt-test": { block: ["betaval"], allow: [] } }, + autoLearn: false, + }, + }; + const puts: { providerId: string; models?: Record }[] = []; + 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: [] } }); + }); +}); From e26fc0a774418cd3c52f3ef5b4de08f8eb799e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 07:13:52 +0800 Subject: [PATCH 6/9] fix(dashboard): keep param-filter fields and drafts bound to their own target (#8910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two remaining defects of the #8910 silent-data-loss family, both reached through the re-point path of a live ModelCompatPopover. 1. The inputs render blockText/allowText, whose only writer was the load effect — and that effect early-returned whenever a draft was dirty for the target. So re-pointing A -> B -> A left B's server values on screen under A, and the next keystroke snapshotted them into A's draft, persisting B's content into A's entry. The fields are now a function of the target: on return to a target with a pending draft the draft is restored into the inputs, and on a target with no draft the previous target's values are cleared instead of being left behind. An edit also no longer trusts the counterpart field unless the values on screen belong to the target being edited. 2. The pending draft lived in a single slot that every edit overwrote, so typing into a newly pointed target destroyed the previous target's unsaved work while the new target's successful save cleared the failure indicator — a green UI over data that was never written. Drafts are now keyed by provider/model; the save drains every pending draft against its own target, and the indicator reflects unsaved work across all targets rather than the last write. Regression tests: modelCompatPopover-param-filter-target-repoint.test.tsx (3 cases, RED at ecd111489, GREEN here). Scope limited to this component. --- .../[id]/components/ModelCompatPopover.tsx | 207 ++++++++------ ...pover-param-filter-target-repoint.test.tsx | 261 ++++++++++++++++++ 2 files changed, 388 insertions(+), 80 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-target-repoint.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index a293b24f06..4454f0c9e3 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -58,6 +58,10 @@ interface ParamFilterDraft { allow: string; } +function paramTargetKeyOf(providerId: string, modelId: string): string { + return `${providerId}\u0000${modelId}`; +} + // 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( @@ -146,13 +150,17 @@ export default function ModelCompatPopover({ // 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 paramSavingRef = useRef(false); - // 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(null); + // Every unsaved draft, keyed by the provider/model it was typed for. A per-target map (rather + // than a single slot) is required because a live popover can be re-pointed at another target + // while a draft is still unsaved: with one slot the next keystroke on the new target destroyed + // the previous target's unsaved work, and the new target's successful save then cleared the + // failure indicator — a green UI over data that was never written, i.e. exactly the silent + // data loss reported in #8910. Drafts are recorded when edited — never derived from a completed + // load — so an unsaved draft survives even when no load ever succeeded for that target, and + // every write lands on the draft's own target rather than whatever the popover now points at. + const paramDraftsRef = useRef>(new Map()); - const paramTargetKey = `${providerId}\u0000${modelId}`; + const paramTargetKey = paramTargetKeyOf(providerId, modelId); const paramTargetRef = useRef<{ key: string; providerId: string; modelId: string }>({ key: paramTargetKey, providerId, @@ -165,19 +173,44 @@ export default function ModelCompatPopover({ const allowTextRef = useRef(""); blockTextRef.current = blockText; allowTextRef.current = allowText; + // Which target the values currently in the fields belong to. Guards the invariant that + // blockTextRef/allowTextRef never hold content belonging to a target other than the one being + // displayed — the desync that let one model's server values be saved under another (#8910). + const fieldsTargetKeyRef = useRef(null); - // 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(() => { - const target = paramTargetRef.current; - paramDraftRef.current = { - key: target.key, - providerId: target.providerId, - modelId: target.modelId, - block: blockTextRef.current, - allow: allowTextRef.current, - }; + const applyParamFields = useCallback((targetKey: string, block: string, allow: string) => { + fieldsTargetKeyRef.current = targetKey; + blockTextRef.current = block; + allowTextRef.current = allow; + setBlockText(block); + setAllowText(allow); }, []); + + // Every edit rewrites the draft for the CURRENT target. The counterpart field is only trusted + // when the values on screen belong to this target; otherwise it is taken from this target's own + // pending draft (or empty), so another target's value can never be captured into this draft and + // then persisted here (#8910). Object identity doubles as the draft revision an in-flight save + // compares against. + const editParamDraft = useCallback( + (field: "block" | "allow", value: string) => { + const target = paramTargetRef.current; + const fieldsOwned = fieldsTargetKeyRef.current === target.key; + const pending = paramDraftsRef.current.get(target.key); + const block = + field === "block" ? value : fieldsOwned ? blockTextRef.current : (pending?.block ?? ""); + const allow = + field === "allow" ? value : fieldsOwned ? allowTextRef.current : (pending?.allow ?? ""); + applyParamFields(target.key, block, allow); + paramDraftsRef.current.set(target.key, { + key: target.key, + providerId: target.providerId, + modelId: target.modelId, + block, + allow, + }); + }, + [applyParamFields] + ); const mountedRef = useRef(true); useEffect(() => { mountedRef.current = true; @@ -228,12 +261,20 @@ export default function ModelCompatPopover({ // Load model-level block/allow from param-filters API useEffect(() => { if (!open) return; - const draftKey = `${providerId}\u0000${modelId}`; - // 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 = () => paramDraftRef.current?.key === draftKey; - if (draftIsDirtyForThisTarget()) return; + const draftKey = paramTargetKeyOf(providerId, modelId); + const draftForThisTarget = () => paramDraftsRef.current.get(draftKey); + // The fields must always show THIS target's content, and nothing else may ever be read back + // out of them. A draft that is still pending for this exact provider/model was never persisted + // (failed or exhausted save): restore it into the inputs instead of loading server state over + // it, because reloading here would silently revert it — the very complaint behind #8910. + const pending = draftForThisTarget(); + if (pending) { + applyParamFields(draftKey, pending.block, pending.allow); + return; + } + // No draft for this target: drop whatever the previously displayed target left on screen so + // the fields can never present (or contribute) another target's values. + if (fieldsTargetKeyRef.current !== draftKey) applyParamFields(draftKey, "", ""); let cancelled = false; (async () => { try { @@ -243,14 +284,17 @@ export default function ModelCompatPopover({ if (cancelled || !mountedRef.current) return; // Re-check after the await: the user may have typed while the GET was in flight, and a // load result must never overwrite (or acknowledge) a draft that is not on the server. - if (draftIsDirtyForThisTarget()) return; + if (draftForThisTarget()) return; const modelCfg = data?.models?.[modelId]; - setBlockText(modelCfg ? (modelCfg.block ?? []).join(", ") : ""); - setAllowText(modelCfg ? (modelCfg.allow ?? []).join(", ") : ""); - // A load never clears the draft: any draft still pending here belongs to a DIFFERENT + applyParamFields( + draftKey, + modelCfg ? (modelCfg.block ?? []).join(", ") : "", + modelCfg ? (modelCfg.allow ?? []).join(", ") : "" + ); + // A load never clears a 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); + if (paramDraftsRef.current.size === 0) setParamSaveFailed(false); } catch { // Keep whatever the user has in the fields (and its dirty flag) on load failure. } @@ -259,57 +303,68 @@ export default function ModelCompatPopover({ cancelled = true; }; // Reload only when opening or when the popover targets a different provider/model. - }, [open, providerId, modelId]); + }, [open, providerId, modelId, applyParamFields]); - // 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. + // Drains EVERY pending draft, each written through its 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, and re-pointing the popover cannot destroy target A's unsaved + // work (#8910). Each draft is re-read after every await for the same reason the load effect + // re-checks its guard: the user can keep typing while a write is in flight. const saveModelParamFilters = useCallback(async () => { - if (!paramDraftRef.current || paramSavingRef.current) return; + if (paramDraftsRef.current.size === 0 || paramSavingRef.current) return; paramSavingRef.current = true; if (mountedRef.current) setParamSaving(true); - try { + + // Returns true once nothing is owed for this target any more. + const saveDraftForTarget = async (key: string): Promise => { // Re-run while the draft changed under the in-flight write: the payload is snapshotted // before the PUT resolves, so a keystroke landing in that window would otherwise be - // acknowledged (dirty cleared) but never persisted — the #8910 lost update. + // acknowledged (draft dropped) but never persisted — the #8910 lost update. for (let attempt = 0; attempt < PARAM_SAVE_MAX_ATTEMPTS; attempt += 1) { - 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, - draft.modelId, - draft.block, - draft.allow - ); - 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 exact draft that was written may clear the dirty flag. - if (paramDraftRef.current === draft) { - paramDraftRef.current = null; - if (mountedRef.current) setParamSaveFailed(false); - return; + const draft = paramDraftsRef.current.get(key); + if (!draft) return true; + try { + 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 a newer + // one for the same target while the GET was in flight, restart with a fresh read. + if (paramDraftsRef.current.get(key) !== draft) continue; + const payload = buildModelParamFilterPayload( + current, + draft.modelId, + draft.block, + draft.allow + ); + 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 exact draft that was written may be discarded. + if (paramDraftsRef.current.get(key) === draft) { + paramDraftsRef.current.delete(key); + return true; + } + } catch { + // Save failed — the draft (and the target it belongs to) is intentionally preserved so + // the next blur/close/unmount save retries it against its own provider/model. + return false; } } - // Budget exhausted (the user is still typing): stay dirty and flag it, so the draft is - // kept on reopen and the next blur/close retries it. - if (mountedRef.current) setParamSaveFailed(true); - } catch { - // 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); + // Budget exhausted (the user is still typing): stay dirty for this target. + return false; + }; + + try { + const failures: string[] = []; + for (const key of Array.from(paramDraftsRef.current.keys())) { + if (!(await saveDraftForTarget(key))) failures.push(key); + } + // The indicator tracks unsaved work across ALL targets: a successful write for the target + // now on screen must not signal "saved" while another target's draft is still owed a write. + if (mountedRef.current) setParamSaveFailed(failures.length > 0); } finally { paramSavingRef.current = false; if (mountedRef.current) setParamSaving(false); @@ -482,11 +537,7 @@ export default function ModelCompatPopover({ { - setBlockText(e.target.value); - blockTextRef.current = e.target.value; - markParamDraftDirty(); - }} + onChange={(e) => editParamDraft("block", e.target.value)} onBlur={() => saveModelParamFilters()} placeholder={t("compatBlockedParamsPlaceholder")} disabled={disabled} @@ -510,11 +561,7 @@ export default function ModelCompatPopover({ { - setAllowText(e.target.value); - allowTextRef.current = e.target.value; - markParamDraftDirty(); - }} + onChange={(e) => editParamDraft("allow", e.target.value)} onBlur={() => saveModelParamFilters()} placeholder={t("compatAllowedParamsPlaceholder")} disabled={disabled} diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-target-repoint.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-target-repoint.test.tsx new file mode 100644 index 0000000000..e6db451f3d --- /dev/null +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-target-repoint.test.tsx @@ -0,0 +1,261 @@ +// @vitest-environment jsdom +// Regression coverage for the two target-re-point defects found while fixing #8910. +// +// A ModelCompatPopover instance is not always keyed by a stable identity, so a re-render can +// re-point a LIVE, still-open popover at a different provider/model while a draft for the previous +// target is unsaved. Two failures followed from that: +// +// 1. (C10) The inputs are driven by blockText/allowText, which used to be written only by the +// load effect — and that effect early-returned whenever a draft was dirty. Re-pointing +// A -> B -> A therefore left B's server values on screen under A, and the next keystroke +// snapshotted them into A's draft, persisting B's content into A's entry. +// 2. (C11) The pending draft lived in a single slot that every edit overwrote, so typing into +// the new target destroyed the previous target's unsaved work, and the new target's +// successful save cleared the failure indicator — a green UI over data never written. +// +// Contract asserted here: the fields always show the displayed target's own content (its pending +// draft when it has one), never another target's; and every target's unsaved draft survives until +// it is actually persisted to its own provider/model. +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; + 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(modelId: string) { + act(() => { + root.render( + key} + providerId="openai" + modelId={modelId} + effectiveModelNormalize={() => false} + effectiveModelPreserveDeveloper={() => true} + getUpstreamHeadersRecord={() => ({})} + onCompatPatch={vi.fn()} + /> + ); + }); +} + +// React 19 delegates onBlur through focusout — a bare "blur" event does not reach the handler. +async function blurBlockInput() { + await act(async () => { + blockInput()!.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + }); + await flushEffects(); +} + +describe("ModelCompatPopover param-filter target re-point (#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("restores the dirty draft into the fields on return, instead of showing the other model's values", async () => { + const server: ParamFilterState = { + block: [], + allow: [], + models: { "model-b": { block: ["secret-b"], allow: [] } }, + autoLearn: false, + }; + let putFails = true; + + vi.stubGlobal( + "fetch", + vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + if (putFails) return { ok: false, status: 500, json: async () => ({}) } as Response; + const body = JSON.parse(String(init.body)); + server.models = body.models ?? {}; + return { ok: true, json: async () => ({ success: true }) } as Response; + } + return { ok: true, json: async () => structuredClone(server) } as Response; + }) + ); + + renderPopover("model-a"); + const trigger = container.querySelector("button") as HTMLButtonElement; + await act(async () => trigger.click()); + await flushEffects(); + + // model-a has no server entry, so the field loads empty; the user types a draft whose save fails. + await act(async () => setInputValue(blockInput()!, "aaa")); + await blurBlockInput(); + expect(blockInput()!.value).toBe("aaa"); + + // The live popover is re-pointed at model-b, which does have a server value... + renderPopover("model-b"); + await flushEffects(); + expect(blockInput()!.value).toBe("secret-b"); + + // ...and back to model-a, whose draft is still unsaved: the field must show model-a's draft. + renderPopover("model-a"); + await flushEffects(); + expect(blockInput()!.value).toBe("aaa"); + + // The user appends to what they can see and closes; the write must stay inside model-a. + putFails = false; + await act(async () => setInputValue(blockInput()!, `${blockInput()!.value}, extra`)); + await act(async () => { + document.body.dispatchEvent(new MouseEvent("mousedown", { bubbles: true })); + }); + await flushEffects(); + + expect(server.models["model-a"]).toEqual({ block: ["aaa", "extra"], allow: [] }); + expect(JSON.stringify(server.models["model-a"])).not.toContain("secret-b"); + expect(server.models["model-b"]).toEqual({ block: ["secret-b"], allow: [] }); + }); + + it("keeps one model's unsaved draft alive while the user edits another model", async () => { + const server: ParamFilterState = { block: [], allow: [], models: {}, autoLearn: false }; + let putFails = true; + + vi.stubGlobal( + "fetch", + vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + if (putFails) return { ok: false, status: 500, json: async () => ({}) } as Response; + const body = JSON.parse(String(init.body)); + server.models = body.models ?? {}; + return { ok: true, json: async () => ({ success: true }) } as Response; + } + return { ok: true, json: async () => structuredClone(server) } as Response; + }) + ); + + renderPopover("model-a"); + const trigger = container.querySelector("button") as HTMLButtonElement; + await act(async () => trigger.click()); + await flushEffects(); + + await act(async () => setInputValue(blockInput()!, "aaa")); + await blurBlockInput(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain("failed"); + + // Re-pointed at model-b; the network recovers and the user edits model-b. + renderPopover("model-b"); + await flushEffects(); + putFails = false; + await act(async () => setInputValue(blockInput()!, "bbb")); + await blurBlockInput(); + + // model-a's draft must not have been destroyed by the model-b edit: both are persisted... + expect(server.models["model-a"]).toEqual({ block: ["aaa"], allow: [] }); + expect(server.models["model-b"]).toEqual({ block: ["bbb"], allow: [] }); + // ...and with nothing left unsaved anywhere the failure indicator is finally cleared. + expect(document.querySelector('[role="alert"]')).toBeNull(); + }); + + it("does not report success while another target's draft is still unsaved", async () => { + const byProvider: Record = { + alpha: { block: [], allow: [], models: {}, autoLearn: false }, + beta: { block: [], allow: [], models: {}, autoLearn: false }, + }; + let failing = new Set(["alpha"]); + + 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") { + if (failing.has(providerId)) { + return { ok: false, status: 503, json: async () => ({}) } as Response; + } + const body = JSON.parse(String(init.body)); + byProvider[providerId].models = body.models ?? {}; + return { ok: true, json: async () => ({ success: true }) } as Response; + } + return { ok: true, json: async () => structuredClone(byProvider[providerId]) } as Response; + }) + ); + + const renderFor = (providerId: string) => { + act(() => { + root.render( + key} + providerId={providerId} + modelId="gpt-test" + effectiveModelNormalize={() => false} + effectiveModelPreserveDeveloper={() => true} + getUpstreamHeadersRecord={() => ({})} + onCompatPatch={vi.fn()} + /> + ); + }); + }; + + renderFor("alpha"); + const trigger = container.querySelector("button") as HTMLButtonElement; + await act(async () => trigger.click()); + await flushEffects(); + await act(async () => setInputValue(blockInput()!, "alpha-draft")); + await blurBlockInput(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain("failed"); + + // beta saves fine, but alpha is still broken: the indicator must stay up. + renderFor("beta"); + await flushEffects(); + await act(async () => setInputValue(blockInput()!, "beta-draft")); + await blurBlockInput(); + + expect(byProvider.beta.models).toEqual({ "gpt-test": { block: ["beta-draft"], allow: [] } }); + expect(byProvider.alpha.models).toEqual({}); + expect(document.querySelector('[role="alert"]')?.textContent).toContain("failed"); + + // Once alpha recovers, the preserved draft is written to alpha and the indicator clears. + failing = new Set(); + await act(async () => { + document.body.dispatchEvent(new MouseEvent("mousedown", { bubbles: true })); + }); + await flushEffects(); + expect(byProvider.alpha.models).toEqual({ "gpt-test": { block: ["alpha-draft"], allow: [] } }); + }); +}); From 4f97f84deaeb1c0aed84663537ea60860b8853bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Thu, 30 Jul 2026 23:58:05 +0800 Subject: [PATCH 7/9] fix(dashboard): drain midflight param-filter drafts (#8910) --- .../[id]/components/ModelCompatPopover.tsx | 20 +++- ...param-filter-midflight-new-target.test.tsx | 109 +++++++++++++++++ ...er-param-filter-midflight-unmount.test.tsx | 110 ++++++++++++++++++ 3 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-new-target.test.tsx create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-unmount.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index 4454f0c9e3..a53e772db4 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -358,13 +358,25 @@ export default function ModelCompatPopover({ }; try { - const failures: string[] = []; - for (const key of Array.from(paramDraftsRef.current.keys())) { - if (!(await saveDraftForTarget(key))) failures.push(key); + // Do not snapshot the keys once: a second target can become dirty while an earlier target's + // PUT is in flight. Keep selecting live drafts until only revisions that already failed in + // this drain remain. Remember the failed object (not just its key), so a newer edit for that + // target that lands while another request is pending still gets one save attempt. + const failedDrafts = new Map(); + while (true) { + const next = Array.from(paramDraftsRef.current.entries()).find( + ([key, draft]) => failedDrafts.get(key) !== draft + ); + if (!next) break; + const [key] = next; + if (!(await saveDraftForTarget(key))) { + const failedDraft = paramDraftsRef.current.get(key); + if (failedDraft) failedDrafts.set(key, failedDraft); + } } // The indicator tracks unsaved work across ALL targets: a successful write for the target // now on screen must not signal "saved" while another target's draft is still owed a write. - if (mountedRef.current) setParamSaveFailed(failures.length > 0); + if (mountedRef.current) setParamSaveFailed(paramDraftsRef.current.size > 0); } finally { paramSavingRef.current = false; if (mountedRef.current) setParamSaving(false); diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-new-target.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-new-target.test.tsx new file mode 100644 index 0000000000..8a00bfb563 --- /dev/null +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-new-target.test.tsx @@ -0,0 +1,109 @@ +// @vitest-environment jsdom +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { expect, it, vi } from "vitest"; +import ModelCompatPopover from "../ModelCompatPopover"; + +vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key })); + +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(); + }); +} + +it("O5 preserves a new target edited while the older target save is in flight", async () => { + ( + globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + let server = { + block: [] as string[], + allow: [] as string[], + models: {} as Record, + autoLearn: false, + }; + let releaseFirstPut: (() => void) | null = null; + 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 typeof server; + if (putCount === 1) { + await new Promise((resolve) => { + releaseFirstPut = resolve; + }); + } + server = body; + return { ok: true, json: async () => ({ success: true }) } as Response; + } + return { ok: true, json: async () => structuredClone(server) } as Response; + }) + ); + + const renderFor = (modelId: string) => { + act(() => { + root.render( + key} + providerId="openai" + modelId={modelId} + effectiveModelNormalize={() => false} + effectiveModelPreserveDeveloper={() => true} + getUpstreamHeadersRecord={() => ({})} + onCompatPatch={vi.fn()} + /> + ); + }); + }; + const blockInput = () => + document.querySelector( + 'input[placeholder="compatBlockedParamsPlaceholder"]' + ) as HTMLInputElement; + const blurBlock = async () => { + await act(async () => { + blockInput().dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + }); + await flushEffects(); + }; + + renderFor("model-a"); + await act(async () => (container.querySelector("button") as HTMLButtonElement).click()); + await flushEffects(); + await act(async () => setInputValue(blockInput(), "aaa")); + await blurBlock(); + expect(releaseFirstPut).not.toBeNull(); + + renderFor("model-b"); + await flushEffects(); + expect(blockInput().value).toBe(""); + await act(async () => setInputValue(blockInput(), "bbb")); + await blurBlock(); + + await act(async () => { + releaseFirstPut?.(); + await Promise.resolve(); + }); + await flushEffects(); + + expect(server.models).toEqual({ + "model-a": { block: ["aaa"], allow: [] }, + "model-b": { block: ["bbb"], allow: [] }, + }); + expect(document.querySelector('[role="alert"]')).toBeNull(); + + act(() => root.unmount()); + document.body.innerHTML = ""; + vi.unstubAllGlobals(); +}); diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-unmount.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-unmount.test.tsx new file mode 100644 index 0000000000..c3d372ead4 --- /dev/null +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-midflight-unmount.test.tsx @@ -0,0 +1,110 @@ +// @vitest-environment jsdom +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { expect, it, vi } from "vitest"; +import ModelCompatPopover from "../ModelCompatPopover"; + +vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key })); + +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(); + }); +} + +it("O5 does not lose the new target when unmounted during the older target save", async () => { + ( + globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + let server = { + block: [] as string[], + allow: [] as string[], + models: {} as Record, + autoLearn: false, + }; + let releaseFirstPut: (() => void) | null = null; + 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 typeof server; + if (putCount === 1) { + await new Promise((resolve) => { + releaseFirstPut = resolve; + }); + } + server = body; + return { ok: true, json: async () => ({ success: true }) } as Response; + } + return { ok: true, json: async () => structuredClone(server) } as Response; + }) + ); + + const renderFor = (modelId: string) => { + act(() => { + root.render( + key} + providerId="openai" + modelId={modelId} + effectiveModelNormalize={() => false} + effectiveModelPreserveDeveloper={() => true} + getUpstreamHeadersRecord={() => ({})} + onCompatPatch={vi.fn()} + /> + ); + }); + }; + const blockInput = () => + document.querySelector( + 'input[placeholder="compatBlockedParamsPlaceholder"]' + ) as HTMLInputElement; + const blurBlock = async () => { + await act(async () => { + blockInput().dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + }); + await flushEffects(); + }; + + renderFor("model-a"); + await act(async () => (container.querySelector("button") as HTMLButtonElement).click()); + await flushEffects(); + await act(async () => setInputValue(blockInput(), "aaa")); + await blurBlock(); + expect(releaseFirstPut).not.toBeNull(); + + renderFor("model-b"); + await flushEffects(); + await act(async () => setInputValue(blockInput(), "bbb")); + await blurBlock(); + + // Every close/unmount save is rejected while model-a owns paramSavingRef. The active save took + // its key snapshot before model-b existed, so model-b has no later save scheduled. + act(() => root.unmount()); + await act(async () => { + releaseFirstPut?.(); + await Promise.resolve(); + }); + await flushEffects(); + console.log("O5 server after unmount:", JSON.stringify(server.models), "PUTs:", putCount); + + expect(server.models).toEqual({ + "model-a": { block: ["aaa"], allow: [] }, + "model-b": { block: ["bbb"], allow: [] }, + }); + + document.body.innerHTML = ""; + vi.unstubAllGlobals(); +}); From dcddbe11cdf3964ee6289c0e1057aab8b41d33cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Fri, 31 Jul 2026 02:26:10 +0800 Subject: [PATCH 8/9] fix(dashboard): serialize cross-row param-filter saves (#8910) --- .../[id]/components/ModelCompatPopover.tsx | 61 +++++--- ...pover-param-filter-cross-instance.test.tsx | 137 ++++++++++++++++++ 2 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-instance.test.tsx diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx index a53e772db4..770df5441f 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -31,6 +31,29 @@ function recordToHeaderRows(rec: Record, genId: () => string): H // payload that no longer matches what the user typed (#8910). const PARAM_SAVE_MAX_ATTEMPTS = 3; +// Param filters are stored as one document per provider. Model rows render independent popover +// instances, so their GET -> whole-document PUT transactions must share a provider-level queue; +// instance-local saving refs cannot prevent sibling rows from overwriting each other's updates. +const paramFilterSaveQueues = new Map>(); + +async function serializeProviderParamFilterSave( + providerId: string, + save: () => Promise +): Promise { + const previous = paramFilterSaveQueues.get(providerId) ?? Promise.resolve(); + const result = previous.then(save); + const tail = result.then( + () => undefined, + () => undefined + ); + paramFilterSaveQueues.set(providerId, tail); + try { + return await result; + } finally { + if (paramFilterSaveQueues.get(providerId) === tail) paramFilterSaveQueues.delete(providerId); + } +} + function parseCommaList(text: string): string[] { return text ? text @@ -324,24 +347,28 @@ export default function ModelCompatPopover({ const draft = paramDraftsRef.current.get(key); if (!draft) return true; try { - 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 a newer - // one for the same target while the GET was in flight, restart with a fresh read. - if (paramDraftsRef.current.get(key) !== draft) continue; - const payload = buildModelParamFilterPayload( - current, - draft.modelId, - draft.block, - draft.allow - ); - const putRes = await fetch(`/api/providers/${draft.providerId}/param-filters`, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(payload), + const wroteDraft = await serializeProviderParamFilterSave(draft.providerId, async () => { + 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 a newer + // one while the GET (or this instance's queue wait) was in flight, restart fresh. + if (paramDraftsRef.current.get(key) !== draft) return false; + const payload = buildModelParamFilterPayload( + current, + draft.modelId, + draft.block, + draft.allow + ); + 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}`); + return true; }); - if (!putRes.ok) throw new Error(`param-filters PUT failed: ${putRes.status}`); + if (!wroteDraft) continue; // Only the exact draft that was written may be discarded. if (paramDraftsRef.current.get(key) === draft) { paramDraftsRef.current.delete(key); diff --git a/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-instance.test.tsx b/src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-param-filter-cross-instance.test.tsx new file mode 100644 index 0000000000..08cd6556e2 --- /dev/null +++ b/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; + 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((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( +
+
+ +
+
+ +
+
+ ); + }); + + 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: [] }, + }); +}); From b21f2d91e7d746502cc638338aa41f5fe9d33ea4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Fri, 31 Jul 2026 04:29:17 +0800 Subject: [PATCH 9/9] docs(changelog): add fragment for #9013 --- changelog.d/fixes/9013-model-param-filter-save.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/fixes/9013-model-param-filter-save.md diff --git a/changelog.d/fixes/9013-model-param-filter-save.md b/changelog.d/fixes/9013-model-param-filter-save.md new file mode 100644 index 0000000000..d81d7ab179 --- /dev/null +++ b/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))