Merge branch 'main' into fix/sync-form-parameters-race

The useSyncFormParameters hook now respects Formik's touched state.
When a field has been edited by the user but the change hasn't
round-tripped through the WebSocket yet, the hook preserves the
local value instead of overwriting it with the server's stale
response.

This addresses the root cause that #22556 noted as residual:
"5 residual failures are a separate pre-existing race in
fillParameters where user input is overwritten during the 500ms
debounce window."

Refs #22028
Refs #20257
Refs #22556
Closes #22860
Closes #22876
Refs #22875

site/e2e/helpers.ts

@@ -219,12 +219,7 @@ export const verifyParameters = async (
 					case "number":
 						{
 							const parameterField = parameterLabel.locator("input").first();
-							// Dynamic parameters can hydrate after initial render with
-							// stale or empty values. Retry with a longer timeout to
-							// allow the page to settle.
-							await expect(parameterField).toHaveValue(buildParameter.value, {
-								timeout: 15_000,
-							});
+							await expect(parameterField).toHaveValue(buildParameter.value);
 						}
 						break;
 					default:
@@ -1059,22 +1054,7 @@ const fillParameters = async (
 			case "number":
 				{
 					const parameterField = parameterLabel.locator("input");
-					// Dynamic parameters can hydrate after initial render and
-					// overwrite an early fill. Re-apply until the desired value
-					// is stable.
-					for (let attempt = 0; attempt < 3; attempt++) {
-						await parameterField.fill(buildParameter.value);
-						try {
-							await expect(parameterField).toHaveValue(buildParameter.value, {
-								timeout: 1000,
-							});
-							break;
-						} catch (error) {
-							if (attempt === 2) {
-								throw error;
-							}
-						}
-					}
+					await parameterField.fill(buildParameter.value);
 				}
 				break;
 			default:

site/src/modules/hooks/useSyncFormParameters.test.ts

@@ -0,0 +1,160 @@
+import { renderHook } from "@testing-library/react";
+import type { PreviewParameter } from "api/typesGenerated";
+import { useSyncFormParameters } from "./useSyncFormParameters";
+
+function makeParam(
+	name: string,
+	value: string,
+	valid = true,
+): PreviewParameter {
+	return {
+		name,
+		display_name: name,
+		description: "",
+		type: "string",
+		form_type: "input",
+		styling: {},
+		mutable: true,
+		default_value: { value: "", valid: false },
+		icon: "",
+		options: [],
+		validations: [],
+		required: false,
+		order: 0,
+		ephemeral: false,
+		value: { value, valid },
+		diagnostics: [],
+	};
+}
+
+describe("useSyncFormParameters", () => {
+	it("syncs untouched fields from server", () => {
+		const setFieldValue = vi.fn();
+		const parameters = [makeParam("region", "us-east")];
+		const formValues = [{ name: "region", value: "us-west" }];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: {},
+				setFieldValue,
+			}),
+		);
+
+		expect(setFieldValue).toHaveBeenCalledWith("rich_parameter_values", [
+			{ name: "region", value: "us-east" },
+		]);
+	});
+
+	it("preserves touched fields when form value differs from server", () => {
+		const setFieldValue = vi.fn();
+		// Server still has old value "false", but user toggled to "true".
+		const parameters = [makeParam("use_gpu", "false")];
+		const formValues = [{ name: "use_gpu", value: "true" }];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: { use_gpu: true },
+				setFieldValue,
+			}),
+		);
+
+		// The hook should not overwrite the user's value.
+		expect(setFieldValue).not.toHaveBeenCalled();
+	});
+
+	it("syncs touched fields when server matches form value", () => {
+		const setFieldValue = vi.fn();
+		// Server has echoed back the user's value.
+		const parameters = [
+			makeParam("use_gpu", "true"),
+			makeParam("region", "eu-west"),
+		];
+		const formValues = [
+			{ name: "use_gpu", value: "true" },
+			{ name: "region", value: "us-east" },
+		];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: { use_gpu: true },
+				setFieldValue,
+			}),
+		);
+
+		// region is untouched and differs, so it should sync.
+		// use_gpu matches server, so it syncs normally too.
+		expect(setFieldValue).toHaveBeenCalledWith("rich_parameter_values", [
+			{ name: "use_gpu", value: "true" },
+			{ name: "region", value: "eu-west" },
+		]);
+	});
+
+	it("adds new parameters from server", () => {
+		const setFieldValue = vi.fn();
+		const parameters = [
+			makeParam("region", "us-east"),
+			makeParam("instance_type", "t3.large"),
+		];
+		const formValues = [{ name: "region", value: "us-east" }];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: {},
+				setFieldValue,
+			}),
+		);
+
+		expect(setFieldValue).toHaveBeenCalledWith("rich_parameter_values", [
+			{ name: "region", value: "us-east" },
+			{ name: "instance_type", value: "t3.large" },
+		]);
+	});
+
+	it("removes parameters no longer in server response", () => {
+		const setFieldValue = vi.fn();
+		// Server only has "region", but form still has "old_param".
+		const parameters = [makeParam("region", "us-east")];
+		const formValues = [
+			{ name: "region", value: "us-east" },
+			{ name: "old_param", value: "stale" },
+		];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: {},
+				setFieldValue,
+			}),
+		);
+
+		expect(setFieldValue).toHaveBeenCalledWith("rich_parameter_values", [
+			{ name: "region", value: "us-east" },
+		]);
+	});
+
+	it("does not call setFieldValue when nothing changed", () => {
+		const setFieldValue = vi.fn();
+		const parameters = [makeParam("region", "us-east")];
+		const formValues = [{ name: "region", value: "us-east" }];
+
+		renderHook(() =>
+			useSyncFormParameters({
+				parameters,
+				formValues,
+				touched: {},
+				setFieldValue,
+			}),
+		);
+
+		expect(setFieldValue).not.toHaveBeenCalled();
+	});
+});

site/src/modules/hooks/useSyncFormParameters.ts

@@ -5,6 +5,7 @@ import { useEffect, useRef } from "react";
 type UseSyncFormParametersProps = {
 	parameters: readonly PreviewParameter[];
 	formValues: readonly TypesGen.WorkspaceBuildParameter[];
+	touched: Record<string, unknown>;
 	setFieldValue: (
 		field: string,
 		value: TypesGen.WorkspaceBuildParameter[],
@@ -14,19 +15,26 @@ type UseSyncFormParametersProps = {
 export function useSyncFormParameters({
 	parameters,
 	formValues,
+	touched,
 	setFieldValue,
 }: UseSyncFormParametersProps) {
 	// Form values only needs to be updated when parameters change
 	// Keep track of form values in a ref to avoid unnecessary updates to rich_parameter_values
 	const formValuesRef = useRef(formValues);
+	const touchedRef = useRef(touched);
 
 	useEffect(() => {
 		formValuesRef.current = formValues;
 	}, [formValues]);
 
+	useEffect(() => {
+		touchedRef.current = touched;
+	}, [touched]);
+
 	useEffect(() => {
 		if (!parameters) return;
 		const currentFormValues = formValuesRef.current;
+		const currentTouched = touchedRef.current;
 		const currentFormValuesMap = new Map(
 			currentFormValues.map((value) => [value.name, value.value]),
 		);
@@ -45,9 +53,24 @@ export function useSyncFormParameters({
 				}
 			}
 
+			const serverValue = param.value.value;
+			const existingValue = currentFormValuesMap.get(param.name);
+
+			// If the user has edited this field and the server hasn't
+			// echoed back the user's value yet, preserve the local
+			// value. This prevents stale WS responses from clobbering
+			// user input that hasn't round-tripped yet.
+			if (
+				currentTouched[param.name] &&
+				existingValue !== undefined &&
+				existingValue !== serverValue
+			) {
+				return { name: param.name, value: existingValue };
+			}
+
 			return {
 				name: param.name,
-				value: param.value.valid ? param.value.value : "",
+				value: serverValue,
 			};
 		});
 

site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx

@@ -345,6 +345,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
 	useSyncFormParameters({
 		parameters,
 		formValues: form.values.rich_parameter_values ?? [],
+		touched: form.touched,
 		setFieldValue: form.setFieldValue,
 	});
 

site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx

@@ -101,6 +101,7 @@ export const WorkspaceParametersPageViewExperimental: FC<
 	useSyncFormParameters({
 		parameters,
 		formValues: form.values.rich_parameter_values ?? [],
+		touched: form.touched,
 		setFieldValue: form.setFieldValue,
 	});