diff --git a/.changeset/fn-7451-workflow-settings-save-race.md b/.changeset/fn-7451-workflow-settings-save-race.md new file mode 100644 index 0000000000..2f546a0a8f --- /dev/null +++ b/.changeset/fn-7451-workflow-settings-save-race.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Preserve workflow setting edits made while a values save is still in flight. +category: fix +dev: WorkflowSettingsPanel and Project Models workflow lane saves now clear only snapshot-matching pending keys. diff --git a/packages/dashboard/app/__tests__/settings-sections.test.tsx b/packages/dashboard/app/__tests__/settings-sections.test.tsx index 6bc0f14891..215c8f685b 100644 --- a/packages/dashboard/app/__tests__/settings-sections.test.tsx +++ b/packages/dashboard/app/__tests__/settings-sections.test.tsx @@ -9,8 +9,8 @@ * component-test conventions in settings-primitives.test.tsx. */ import { useState } from "react"; -import { describe, it, expect, vi, afterEach } from "vitest"; -import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, fireEvent, cleanup, waitFor, act } from "@testing-library/react"; import * as jestDomMatchers from "@testing-library/jest-dom/matchers"; import { AppearanceSection } from "../components/settings/sections/AppearanceSection"; @@ -23,7 +23,8 @@ import { PromptsSection } from "../components/settings/sections/PromptsSection"; import { SecretsSection } from "../components/settings/sections/SecretsSection"; import { WorktreesSection } from "../components/settings/sections/WorktreesSection"; import type { SettingsFormState } from "../components/settings/sections/context"; -import { fetchWorkflow, fetchWorkflowSettingValues } from "../api"; +import { fetchWorkflow, fetchWorkflowSettingValues, updateWorkflowSettingValues } from "../api"; +import type { WorkflowSettingValuesPayload } from "../api"; vi.mock("../components/AgentPromptsManager", () => ({ AgentPromptsManager: () =>
, @@ -41,17 +42,32 @@ vi.mock("../api", async (importOriginal) => { fetchProjectDefaultWorkflow: vi.fn(async () => ({ workflowId: null })), setProjectDefaultWorkflow: vi.fn(async () => ({ workflowId: null })), fetchGlobalSettings: vi.fn(async () => ({})), + updateWorkflowSettingValues: vi.fn(async () => ({ stored: {}, effective: {}, orphaned: [] })), }; }); vi.mock("../components/CustomModelDropdown", () => ({ - CustomModelDropdown: ({ id, label, menuWidth = "trigger" }: { id?: string; label: string; menuWidth?: "trigger" | "readable" }) => ( - ), })); expect.extend(jestDomMatchers); +beforeEach(() => { + vi.mocked(fetchWorkflow).mockReset(); + vi.mocked(fetchWorkflowSettingValues).mockReset(); + vi.mocked(updateWorkflowSettingValues).mockReset(); + vi.mocked(fetchWorkflow).mockResolvedValue({ id: "builtin:coding", name: "Coding", ir: {} } as never); + vi.mocked(fetchWorkflowSettingValues).mockResolvedValue({ stored: {}, effective: {}, orphaned: [] }); + vi.mocked(updateWorkflowSettingValues).mockResolvedValue({ stored: {}, effective: {}, orphaned: [] }); +}); afterEach(() => cleanup()); const emptyForm = {} as SettingsFormState; @@ -345,6 +361,89 @@ describe("ProjectModelsSection", () => { expect(await screen.findByTestId("mock-model-dropdown-workflow-planning-model")).toHaveAttribute("data-menu-width", "readable"); }); + it("preserves workflow lane edits made while the registered saver is in flight", async () => { + let saver: (() => Promise) | null = null; + let resolveSave!: (value: WorkflowSettingValuesPayload) => void; + vi.mocked(fetchWorkflow).mockResolvedValueOnce({ + id: "builtin:coding", + name: "Coding", + ir: { + settings: [ + { id: "planningProvider", name: "Planning Provider", type: "string" }, + { id: "planningModelId", name: "Planning Model", type: "string" }, + { id: "executionProvider", name: "Execution Provider", type: "string" }, + { id: "executionModelId", name: "Execution Model", type: "string" }, + ], + }, + } as never); + vi.mocked(fetchWorkflowSettingValues).mockResolvedValueOnce({ stored: {}, effective: {}, orphaned: [] }); + vi.mocked(updateWorkflowSettingValues) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveSave = resolve; + }), + ) + .mockResolvedValueOnce({ + stored: { executionProvider: "anthropic", executionModelId: "claude-sonnet-4-5" }, + effective: { executionProvider: "anthropic", executionModelId: "claude-sonnet-4-5" }, + orphaned: [], + }); + + render( + { + saver = next; + }} + />, + ); + + const planning = await screen.findByTestId("mock-model-dropdown-workflow-planning-model"); + const execution = await screen.findByTestId("mock-model-dropdown-workflow-execution-model"); + fireEvent.click(planning); + await waitFor(() => expect(planning).toHaveAttribute("data-value", "anthropic/claude-sonnet-4-5")); + + const firstSave = saver!(); + await waitFor(() => expect(updateWorkflowSettingValues).toHaveBeenCalledTimes(1)); + expect(updateWorkflowSettingValues).toHaveBeenNthCalledWith( + 1, + "builtin:coding", + { planningProvider: "anthropic", planningModelId: "claude-sonnet-4-5" }, + "project-1", + ); + + fireEvent.click(execution); + await waitFor(() => expect(execution).toHaveAttribute("data-value", "anthropic/claude-sonnet-4-5")); + await act(async () => { + resolveSave({ + stored: { planningProvider: "anthropic", planningModelId: "claude-sonnet-4-5" }, + effective: { planningProvider: "anthropic", planningModelId: "claude-sonnet-4-5" }, + orphaned: [], + }); + await firstSave; + }); + + expect(execution).toHaveAttribute("data-value", "anthropic/claude-sonnet-4-5"); + await act(async () => { + await saver!(); + }); + await waitFor(() => expect(updateWorkflowSettingValues).toHaveBeenCalledTimes(2)); + expect(updateWorkflowSettingValues).toHaveBeenNthCalledWith( + 2, + "builtin:coding", + { executionProvider: "anthropic", executionModelId: "claude-sonnet-4-5" }, + "project-1", + ); + }); + it("renders PR prompt guidance textareas and emits edits through setForm", () => { function ProjectModelsHost() { const [form, setFormState] = useState({ diff --git a/packages/dashboard/app/components/WorkflowSettingsPanel.tsx b/packages/dashboard/app/components/WorkflowSettingsPanel.tsx index 7034aff014..a2172d7d51 100644 --- a/packages/dashboard/app/components/WorkflowSettingsPanel.tsx +++ b/packages/dashboard/app/components/WorkflowSettingsPanel.tsx @@ -492,6 +492,14 @@ function rawValueDisplay(value: unknown): string { } } +function workflowSettingPendingValueEquals(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((value, index) => Object.is(value, b[index])); + } + return false; +} + export interface WorkflowModelLanePair { id: string; providerId: string; @@ -712,14 +720,21 @@ function ValuesTab({ const save = useCallback(async () => { if (!dirty) return; - const savedKeys = new Set(Object.keys(pending)); + const pendingSnapshot = { ...pending }; + const savedKeys = Object.keys(pendingSnapshot); setSaving(true); try { - const res = await updateWorkflowSettingValues(workflowId, pending, boundProjectId); + const res = await updateWorkflowSettingValues(workflowId, pendingSnapshot, boundProjectId); setPayload(res); setPending((prev) => { const next = { ...prev }; - for (const k of savedKeys) delete next[k]; + /* + * FNXC:WorkflowSettingsSaveRace 2026-07-02-13:28: + * Values remain editable during an async save, so success may only clear keys whose current pending value still equals this request's snapshot. New same-key edits, clear-to-default nulls, and model-lane pair edits must remain queued for the next save. + */ + for (const k of savedKeys) { + if (workflowSettingPendingValueEquals(prev[k], pendingSnapshot[k])) delete next[k]; + } return next; }); setRejections({}); diff --git a/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx index 1c38318505..ef1cbdc204 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx @@ -82,6 +82,9 @@ const modelResponse = { }; beforeEach(() => { + mockFetchModels.mockReset(); + mockFetchValues.mockReset(); + mockUpdateValues.mockReset(); mockFetchModels.mockResolvedValue(modelResponse); mockFetchValues.mockResolvedValue(payload()); mockUpdateValues.mockResolvedValue(payload()); @@ -89,7 +92,6 @@ beforeEach(() => { afterEach(() => { cleanup(); - vi.clearAllMocks(); }); describe("WorkflowSettingsPanel — Definitions tab", () => { @@ -272,6 +274,81 @@ describe("WorkflowSettingsPanel — Values tab", () => { expect(mockUpdateValues).toHaveBeenNthCalledWith(2, "wf-1", { label: "mid-flight-edit" }, "proj-1"); }); + it("preserves a newer same-key edit when an older save resolves", async () => { + mockFetchValues.mockResolvedValue(payload({ effective: { "timeout-ms": 1000, "new-sessions": false, label: "server" } })); + let resolveSave!: (value: WorkflowSettingValuesPayload) => void; + mockUpdateValues + .mockReturnValueOnce( + new Promise((resolve) => { + resolveSave = resolve; + }), + ) + .mockResolvedValueOnce( + payload({ + stored: { "timeout-ms": 7000 }, + effective: { "timeout-ms": 7000, "new-sessions": false, label: "server" }, + }), + ); + + render(); + openValues(); + await waitFor(() => expect(mockFetchValues).toHaveBeenCalled()); + + fireEvent.change(screen.getByLabelText("Timeout"), { target: { value: "5000" } }); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(1)); + expect(mockUpdateValues).toHaveBeenNthCalledWith(1, "wf-1", { "timeout-ms": 5000 }, "proj-1"); + + fireEvent.change(screen.getByLabelText("Timeout"), { target: { value: "7000" } }); + await act(async () => { + resolveSave(payload({ stored: { "timeout-ms": 5000 }, effective: { "timeout-ms": 5000, "new-sessions": false, label: "server" } })); + }); + + expect(screen.getByLabelText("Timeout")).toHaveValue(7000); + await waitFor(() => expect(screen.getByTestId("wf-settings-save-values")).not.toBeDisabled()); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(2)); + expect(mockUpdateValues).toHaveBeenNthCalledWith(2, "wf-1", { "timeout-ms": 7000 }, "proj-1"); + }); + + it("preserves mid-flight clear-to-default edits after an unrelated save resolves", async () => { + mockFetchValues.mockResolvedValue( + payload({ + stored: { label: "custom" }, + effective: { "timeout-ms": 1000, "new-sessions": false, label: "custom" }, + }), + ); + let resolveSave!: (value: WorkflowSettingValuesPayload) => void; + mockUpdateValues + .mockReturnValueOnce( + new Promise((resolve) => { + resolveSave = resolve; + }), + ) + .mockResolvedValueOnce(payload({ stored: {}, effective: { "timeout-ms": 5000, "new-sessions": false } })); + + render(); + openValues(); + await waitFor(() => expect(screen.getByLabelText("Label")).toHaveValue("custom")); + await waitFor(() => expect(screen.getByTestId("wf-settings-customized-label")).toBeInTheDocument()); + + fireEvent.change(screen.getByLabelText("Timeout"), { target: { value: "5000" } }); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(1)); + + const labelRow = screen.getByTestId("wf-settings-value-label"); + fireEvent.click(within(labelRow).getByRole("button")); + await act(async () => { + resolveSave(payload({ stored: { "timeout-ms": 5000, label: "custom" }, effective: { "timeout-ms": 5000, "new-sessions": false, label: "custom" } })); + }); + + expect(screen.getByLabelText("Label")).toHaveValue(""); + await waitFor(() => expect(screen.getByTestId("wf-settings-save-values")).not.toBeDisabled()); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(2)); + expect(mockUpdateValues).toHaveBeenNthCalledWith(2, "wf-1", { label: null }, "proj-1"); + }); + it("renders a per-field rejection on the matching row and keeps other edits applied", async () => { mockFetchValues.mockResolvedValue(payload({ effective: { "timeout-ms": 1000, "new-sessions": false } })); mockUpdateValues.mockRejectedValueOnce( @@ -520,6 +597,48 @@ describe("WorkflowSettingsPanel — Values tab", () => { ); }); + it("preserves mid-flight model-lane pair edits after an unrelated save resolves", async () => { + mockFetchValues.mockResolvedValue(payload({ effective: { planningProvider: "", planningModelId: "", customModelProvider: "old" } })); + let resolveSave!: (value: WorkflowSettingValuesPayload) => void; + mockUpdateValues + .mockReturnValueOnce( + new Promise((resolve) => { + resolveSave = resolve; + }), + ) + .mockResolvedValueOnce( + payload({ + stored: { planningProvider: "anthropic", planningModelId: "claude-sonnet", customModelProvider: "saved" }, + effective: { planningProvider: "anthropic", planningModelId: "claude-sonnet", customModelProvider: "saved" }, + }), + ); + + render(); + await waitFor(() => expect(mockFetchModels).toHaveBeenCalled()); + + fireEvent.change(screen.getByLabelText("Custom model provider"), { target: { value: "saved" } }); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(1)); + expect(mockUpdateValues).toHaveBeenNthCalledWith(1, "wf-1", { customModelProvider: "saved" }, "proj-1"); + + await openPlanningDropdown(); + fireEvent.click(await screen.findByRole("option", { name: /Claude Sonnet/i })); + await act(async () => { + resolveSave(payload({ stored: { customModelProvider: "saved" }, effective: { customModelProvider: "saved" } })); + }); + + expect(screen.getByLabelText("Plan/Triage Model")).toHaveTextContent("Claude Sonnet"); + await waitFor(() => expect(screen.getByTestId("wf-settings-save-values")).not.toBeDisabled()); + fireEvent.click(screen.getByTestId("wf-settings-save-values")); + await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledTimes(2)); + expect(mockUpdateValues).toHaveBeenNthCalledWith( + 2, + "wf-1", + { planningProvider: "anthropic", planningModelId: "claude-sonnet" }, + "proj-1", + ); + }); + it("shows inherited/default dropdown state for undefined values without a customized badge", async () => { render(); await waitFor(() => expect(mockFetchValues).toHaveBeenCalledWith("wf-1", "proj-1")); diff --git a/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx b/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx index bec4af0ebc..f627fd8918 100644 --- a/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx +++ b/packages/dashboard/app/components/settings/sections/ProjectModelsSection.tsx @@ -80,6 +80,14 @@ function modelPairValue(values: Record, pair: WorkflowModelPair ? `${provider}/${modelId}` : ""; } +function workflowPendingValueEquals(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) + return true; + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((value, index) => Object.is(value, b[index])); + } + return false; +} export interface ProjectModelsSectionModelProps { modelLanes: ModelLane[]; getLaneStatus: (lane: ModelLane) => LaneStatus; @@ -208,10 +216,23 @@ export function ProjectModelsSection({ scopeBanner, form, setForm, models, proje const saveWorkflowLanes = useCallback(async () => { if (!projectId || !workflowDirty) return; + const pendingSnapshot = { ...workflowPending }; + const savedKeys = Object.keys(pendingSnapshot); try { - const payload = await updateWorkflowSettingValues(workflowId, workflowPending, projectId); + const payload = await updateWorkflowSettingValues(workflowId, pendingSnapshot, projectId); setWorkflowPayload(payload); - setWorkflowPending({}); + setWorkflowPending((current) => { + const next = { ...current }; + /* + * FNXC:ProjectModelsWorkflowLanes 2026-07-02-13:30: + * The registered Settings saver can resolve after workflow lane dropdowns changed again. Clear only snapshot-matching keys so Project Models follows the same no-lost-pending-edits invariant as Workflow Settings Values. + */ + for (const key of savedKeys) { + if (workflowPendingValueEquals(current[key], pendingSnapshot[key])) + delete next[key]; + } + return next; + }); setWorkflowRejections({}); } catch (err) {