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] 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: [] }, + }); +});