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] 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(); +});