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)) 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 a4708bb151..5789bbbbe4 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/CustomModelsSection.tsx @@ -713,6 +713,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 f85be293de..22d0c8b203 100644 --- a/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx +++ b/src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx @@ -27,6 +27,34 @@ 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; + +// 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 @@ -43,6 +71,21 @@ 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; +} + +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( @@ -97,6 +140,8 @@ export interface ModelCompatPopoverProps { export default function ModelCompatPopover({ t, + providerId, + modelId, effectiveModelNormalize, effectiveModelPreserveDeveloper, getUpstreamHeadersRecord, @@ -110,8 +155,8 @@ 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 [paramSaveFailed, setParamSaveFailed] = useState(false); const [valuePeekRowId, setValuePeekRowId] = useState(null); const [valueFocusRowId, setValueFocusRowId] = useState(null); const ref = useRef(null); @@ -126,6 +171,78 @@ export default function ModelCompatPopover({ const headerRowsRef = useRef([]); headerRowsRef.current = headerRows; + // 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); + // 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 = paramTargetKeyOf(providerId, 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; + // 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); + + 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; + return () => { + mountedRef.current = false; + }; + }, []); + const genHeaderRowId = () => { headerRowIdRef.current += 1; return `uh-${headerRowIdRef.current}`; @@ -168,40 +285,140 @@ export default function ModelCompatPopover({ // Load model-level block/allow from param-filters API useEffect(() => { if (!open) 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 { 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; + // 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 (draftForThisTarget()) return; const modelCfg = data?.models?.[modelId]; - setBlockText(modelCfg ? (modelCfg.block ?? []).join(", ") : ""); - setAllowText(modelCfg ? (modelCfg.allow ?? []).join(", ") : ""); + 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 (paramDraftsRef.current.size === 0) setParamSaveFailed(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, applyParamFields]); + // 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 (!paramDirty) return; - setParamSaving(true); + if (paramDraftsRef.current.size === 0 || paramSavingRef.current) return; + paramSavingRef.current = true; + if (mountedRef.current) setParamSaving(true); + + // 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 (draft dropped) but never persisted — the #8910 lost update. + for (let attempt = 0; attempt < PARAM_SAVE_MAX_ATTEMPTS; attempt += 1) { + const draft = paramDraftsRef.current.get(key); + if (!draft) return true; + try { + 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 (!wroteDraft) continue; + // 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 for this target. + return false; + }; + try { - const res = await fetch(`/api/providers/${providerId}/param-filters`); - const current = await res.json(); - const payload = buildModelParamFilterPayload(current, modelId, blockText, allowText); - await fetch(`/api/providers/${providerId}/param-filters`, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(payload), - }); - setParamDirty(false); - } catch { - // Silently ignore save error + // 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(paramDraftsRef.current.size > 0); } finally { - setParamSaving(false); + paramSavingRef.current = false; + if (mountedRef.current) setParamSaving(false); } - }, [paramDirty, blockText, allowText]); + }, []); + + // 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, paramTargetKey, saveModelParamFilters]); useEffect(() => { setValuePeekRowId(null); @@ -360,10 +577,7 @@ export default function ModelCompatPopover({ { - setBlockText(e.target.value); - setParamDirty(true); - }} + onChange={(e) => editParamDraft("block", e.target.value)} onBlur={() => saveModelParamFilters()} placeholder={t("compatBlockedParamsPlaceholder")} disabled={disabled} @@ -376,16 +590,22 @@ export default function ModelCompatPopover({ "Blocked params (stripped from requests)" )} {paramSaving && ` ● ${t("compatSaving")}`} + {paramSaveFailed && !paramSaving && ( + + ● {t("failed")} + + )}

{ - setAllowText(e.target.value); - setParamDirty(true); - }} + 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-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: [] }, + }); + }); +}); 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: [] }, + }); +}); 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: [] } }); + }); +}); 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: [] } }); + }); +}); 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..7bf4acc861 --- /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("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..bf425bb885 --- /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("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(); + + + expect(server.models).toEqual({ + "model-a": { block: ["aaa"], allow: [] }, + "model-b": { block: ["bbb"], allow: [] }, + }); + + document.body.innerHTML = ""; + vi.unstubAllGlobals(); +}); 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: [] } }); + }); +}); 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"); + }); +}); 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" })}