From a5d083896fd2aaea4cdb166b6b4005ae45be2380 Mon Sep 17 00:00:00 2001 From: VascoSch92 Date: Fri, 29 May 2026 14:10:35 +0200 Subject: [PATCH] fix(settings): validate condenser Max Size and block invalid values --- .../sdk-settings/schema-field.test.tsx | 121 +++++++++++++++++- .../settings/sdk-settings/schema-field.tsx | 73 +++++++++++ src/i18n/translation.json | 34 +++++ src/utils/sdk-settings-field-metadata.test.ts | 8 ++ src/utils/sdk-settings-field-metadata.ts | 9 ++ 5 files changed, 244 insertions(+), 1 deletion(-) diff --git a/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx b/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx index 6bfc181b0..1afb2def4 100644 --- a/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx +++ b/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx @@ -1,6 +1,9 @@ import { render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; -import { SchemaField } from "#/components/features/settings/sdk-settings/schema-field"; +import { + SchemaField, + getNumericFieldError, +} from "#/components/features/settings/sdk-settings/schema-field"; import { SettingsFieldSchema } from "#/types/settings"; vi.mock("react-i18next", () => ({ @@ -67,4 +70,120 @@ describe("SchemaField", () => { expect(screen.getByText("Top P")).toBeInTheDocument(); expect(screen.getByText("Controls nucleus sampling.")).toBeInTheDocument(); }); + + describe("numeric validation", () => { + function buildIntegerField(): SettingsFieldSchema { + return buildField({ + key: "condenser.max_size", + label: "Max Size", + section: "condenser", + section_label: "Condenser", + value_type: "integer", + default: 240, + }); + } + + const errorTestId = "sdk-settings-condenser.max_size-error"; + + it("shows an error when the value is below the backend minimum", () => { + // condenser.max_size must be >= 20; 10 fails validation on save. + render( + {}} + />, + ); + + expect(screen.getByTestId(errorTestId)).toHaveTextContent( + "SCHEMA$ERROR$MIN_VALUE", + ); + }); + + it("shows an error when an integer field receives a float value", () => { + render( + {}} + />, + ); + + expect(screen.getByTestId(errorTestId)).toHaveTextContent( + "SCHEMA$ERROR$WHOLE_NUMBER", + ); + }); + + it("shows no error for a valid non-negative whole number", () => { + render( + {}} + />, + ); + + expect(screen.queryByTestId(errorTestId)).not.toBeInTheDocument(); + }); + + it("shows no error while the field is empty", () => { + render( + {}} + />, + ); + + expect(screen.queryByTestId(errorTestId)).not.toBeInTheDocument(); + }); + }); +}); + +describe("getNumericFieldError", () => { + function integerField(): SettingsFieldSchema { + return { + key: "condenser.max_size", + label: "Max Size", + section: "condenser", + section_label: "Condenser", + value_type: "integer", + choices: [], + depends_on: [], + prominence: "major", + secret: false, + required: false, + }; + } + + it("flags non-numeric input (e.g. a typed letter) as a whole-number error", () => { + // Browsers surface unparseable number-input entries as an empty value, so + // the native bad-input flag is what tells us the user typed a letter. + expect(getNumericFieldError(integerField(), "", { min: 20 }, true)).toEqual( + { + key: "SCHEMA$ERROR$WHOLE_NUMBER", + }, + ); + }); + + it("flags a float on an integer field as a whole-number error", () => { + expect(getNumericFieldError(integerField(), "0.9", { min: 20 })).toEqual({ + key: "SCHEMA$ERROR$WHOLE_NUMBER", + }); + }); + + it("flags a value below the minimum", () => { + expect(getNumericFieldError(integerField(), "10", { min: 20 })).toEqual({ + key: "SCHEMA$ERROR$MIN_VALUE", + options: { min: 20 }, + }); + }); + + it("returns null for a valid whole number at or above the minimum", () => { + expect(getNumericFieldError(integerField(), "240", { min: 20 })).toBeNull(); + }); }); diff --git a/src/components/features/settings/sdk-settings/schema-field.tsx b/src/components/features/settings/sdk-settings/schema-field.tsx index f69e0b8df..84100b836 100644 --- a/src/components/features/settings/sdk-settings/schema-field.tsx +++ b/src/components/features/settings/sdk-settings/schema-field.tsx @@ -4,6 +4,7 @@ import { OptionalTag } from "#/components/features/settings/optional-tag"; import { SettingsDropdownInput } from "#/components/features/settings/settings-dropdown-input"; import { SettingsInput } from "#/components/features/settings/settings-input"; import { SettingsSwitch } from "#/components/features/settings/settings-switch"; +import { I18nKey } from "#/i18n/declaration"; import { SettingsFieldSchema } from "#/types/settings"; import { HelpLink } from "#/ui/help-link"; import { Typography } from "#/ui/typography"; @@ -12,6 +13,7 @@ import { resolveSchemaChoiceLabel, resolveSchemaFieldDescription, resolveSchemaFieldLabel, + type SettingsFieldConstraints, } from "#/utils/sdk-settings-field-metadata"; import { cn } from "#/utils/utils"; import { @@ -80,6 +82,49 @@ function isUrlField(field: SettingsFieldSchema): boolean { return field.key.endsWith("url") || field.key.endsWith("_url"); } +function isNumericField(field: SettingsFieldSchema): boolean { + return field.value_type === "integer" || field.value_type === "number"; +} + +/** + * Live validation message for numeric inputs: returns an I18nKey (with optional + * interpolation values) for the field's value, or null when it is valid/empty. + * Mirrors the native min/step constraints so users get immediate red feedback + * instead of an unclear error only when they try to save. + * + * `hasBadInput` reflects the native `` bad-input state: + * browsers report unparseable entries (e.g. typed letters) as an empty value, + * so this flag is the only signal that the user typed something non-numeric. + */ +export function getNumericFieldError( + field: SettingsFieldSchema, + value: string | boolean, + constraints: SettingsFieldConstraints | undefined, + hasBadInput = false, +): { key: I18nKey; options?: Record } | null { + if (!isNumericField(field) || typeof value !== "string") { + return null; + } + if (field.value_type === "integer" && hasBadInput) { + return { key: I18nKey.SCHEMA$ERROR$WHOLE_NUMBER }; + } + const trimmed = value.trim(); + if (trimmed === "") { + return null; + } + const parsed = Number(trimmed); + if (field.value_type === "integer" && !Number.isInteger(parsed)) { + return { key: I18nKey.SCHEMA$ERROR$WHOLE_NUMBER }; + } + if (constraints?.min !== undefined && parsed < constraints.min) { + return { + key: I18nKey.SCHEMA$ERROR$MIN_VALUE, + options: { min: constraints.min }, + }; + } + return null; +} + function getInputType( field: SettingsFieldSchema, ): React.HTMLInputTypeAttribute { @@ -107,8 +152,32 @@ export function SchemaField({ onChange: (value: string | boolean) => void; }) { const { t } = useTranslation("openhands"); + const numericInputRef = React.useRef(null); + const [hasBadNumericInput, setHasBadNumericInput] = React.useState(false); const label = resolveSchemaFieldLabel(t, field.key, field.label); const constraints = getSettingsFieldConstraints(field.key); + const numeric = isNumericField(field); + const numericError = getNumericFieldError( + field, + value, + constraints, + hasBadNumericInput, + ); + + // Track the native bad-input state of number inputs. We listen to the raw + // `input` event rather than React's `onChange` because React skips onChange + // when `node.value` is unchanged — and a number input reports an empty value + // for unparseable entries (e.g. a typed letter), so onChange never fires when + // a letter is typed into an empty field. + React.useEffect(() => { + const input = numericInputRef.current; + if (!numeric || !input) { + return undefined; + } + const syncBadInput = () => setHasBadNumericInput(input.validity.badInput); + input.addEventListener("input", syncBadInput); + return () => input.removeEventListener("input", syncBadInput); + }, [numeric]); if (isBooleanField(field)) { return ( @@ -186,6 +255,7 @@ export function SchemaField({ return (
diff --git a/src/i18n/translation.json b/src/i18n/translation.json index 53ad6c2ac..47648a483 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -2243,6 +2243,40 @@ "uk": "Увімкнути конденсацію пам'яті", "ca": "Habilitar condensació de memòria" }, + "SCHEMA$ERROR$WHOLE_NUMBER": { + "en": "Please enter a whole number.", + "ja": "整数を入力してください。", + "zh-CN": "请输入整数。", + "zh-TW": "請輸入整數。", + "ko-KR": "정수를 입력하세요.", + "no": "Vennligst skriv inn et heltall.", + "it": "Inserisci un numero intero.", + "pt": "Insira um número inteiro.", + "es": "Introduce un número entero.", + "ar": "الرجاء إدخال رقم صحيح.", + "fr": "Veuillez saisir un nombre entier.", + "tr": "Lütfen bir tam sayı girin.", + "de": "Bitte geben Sie eine ganze Zahl ein.", + "uk": "Будь ласка, введіть ціле число.", + "ca": "Introduïu un nombre enter." + }, + "SCHEMA$ERROR$MIN_VALUE": { + "en": "Value must be at least {{min}}.", + "ja": "値は {{min}} 以上である必要があります。", + "zh-CN": "值必须至少为 {{min}}。", + "zh-TW": "值必須至少為 {{min}}。", + "ko-KR": "값은 {{min}} 이상이어야 합니다.", + "no": "Verdien må være minst {{min}}.", + "it": "Il valore deve essere almeno {{min}}.", + "pt": "O valor deve ser pelo menos {{min}}.", + "es": "El valor debe ser al menos {{min}}.", + "ar": "يجب أن تكون القيمة {{min}} على الأقل.", + "fr": "La valeur doit être d'au moins {{min}}.", + "tr": "Değer en az {{min}} olmalıdır.", + "de": "Der Wert muss mindestens {{min}} betragen.", + "uk": "Значення має бути не менше {{min}}.", + "ca": "El valor ha de ser com a mínim {{min}}." + }, "SCHEMA$CONDENSER$MAX_SIZE$DESCRIPTION": { "en": "Maximum number of events kept before the condenser runs.", "ja": "コンデンサーが実行される前に保持されるイベントの最大数。", diff --git a/src/utils/sdk-settings-field-metadata.test.ts b/src/utils/sdk-settings-field-metadata.test.ts index bf8f39d31..6798d7b6d 100644 --- a/src/utils/sdk-settings-field-metadata.test.ts +++ b/src/utils/sdk-settings-field-metadata.test.ts @@ -246,6 +246,14 @@ describe("getSettingsFieldConstraints", () => { }); }); + it("clamps condenser.max_size at the backend minimum of 20", () => { + const constraints = getSettingsFieldConstraints("condenser.max_size"); + expect(constraints).toEqual({ + min: 20, + step: 1, + }); + }); + it("returns undefined for unknown fields", () => { const constraints = getSettingsFieldConstraints("unknown.field"); expect(constraints).toBeUndefined(); diff --git a/src/utils/sdk-settings-field-metadata.ts b/src/utils/sdk-settings-field-metadata.ts index 66d84e124..0fc327529 100644 --- a/src/utils/sdk-settings-field-metadata.ts +++ b/src/utils/sdk-settings-field-metadata.ts @@ -52,6 +52,15 @@ const FIELD_METADATA: Record = { step: 0.1, }, }, + // Backend enforces `CondenserSettings.max_size` with `ge=20` (no upper + // bound); values below 20 fail validation on save. Keep this floor in sync + // with the agent-server SDK constraint. + "condenser.max_size": { + constraints: { + min: 20, + step: 1, + }, + }, }; export function getSettingsFieldConstraints(fieldKey: string) {