diff --git a/changelog.d/fixes/provider-node-null-quota-reset.md b/changelog.d/fixes/provider-node-null-quota-reset.md new file mode 100644 index 0000000000..a8bdd9aeae --- /dev/null +++ b/changelog.d/fixes/provider-node-null-quota-reset.md @@ -0,0 +1 @@ +- **fix(validation):** Provider node edits no longer fail with a generic "Invalid request" when the optional daily-quota reset fields are left blank. The dashboard sends `dailyQuotaResetTimezone` and `dailyQuotaResetHour` as `null`, and only the hour accepted it. ([#13066](https://github.com/diegosouzapw/OmniRoute/issues/13066)) diff --git a/src/shared/validation/schemas/provider.ts b/src/shared/validation/schemas/provider.ts index 5d845f1653..e6f324891b 100644 --- a/src/shared/validation/schemas/provider.ts +++ b/src/shared/validation/schemas/provider.ts @@ -35,10 +35,17 @@ import { isValidProviderIconUrl } from "@/shared/validation/iconUrl"; export { validateProviderSpecificData }; +// Nullable as well as optional, to match dailyQuotaResetHourSchema below. The +// dashboard sends both fields as null when they are left blank, and the two +// schemas disagreeing about that meant an edit touching neither of them still +// failed validation on this one (#13066). The storage layer already coerces to +// null (`data.dailyQuotaResetTimezone || null` in db/providers/nodes.ts), so +// accepting null here changes nothing downstream. const dailyQuotaResetTimezoneSchema = z .string() .trim() .optional() + .nullable() .or(z.literal("")) .refine((value) => !value || isValidIanaTimeZone(value), { message: "Unknown IANA timezone", @@ -519,9 +526,7 @@ export const updateProviderConnectionSchema = z errorCode: z.union([z.string(), z.null()]).optional(), rateLimitedUntil: z.union([z.string(), z.null()]).optional(), lastTested: z.union([z.string(), z.null()]).optional(), - healthCheckInterval: z - .union([z.null(), z.coerce.number().int().min(0).max(1440)]) - .optional(), + healthCheckInterval: z.union([z.null(), z.coerce.number().int().min(0).max(1440)]).optional(), group: z.union([z.string().max(100), z.null()]).optional(), maxConcurrent: z.union([z.null(), z.coerce.number().int().min(0)]).optional(), // Per-window quota cutoffs. Map keys are window names (e.g. "window5h", diff --git a/tests/unit/provider-node-null-quota-reset-13066.test.ts b/tests/unit/provider-node-null-quota-reset-13066.test.ts new file mode 100644 index 0000000000..75e931a7c3 --- /dev/null +++ b/tests/unit/provider-node-null-quota-reset-13066.test.ts @@ -0,0 +1,89 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + createProviderNodeSchema, + updateProviderNodeSchema, +} from "../../src/shared/validation/schemas/provider.ts"; + +// Regression for #13066: saving an edit to a custom OpenAI-compatible node failed +// with a generic "Invalid request" whenever the optional daily-quota reset fields +// were left blank. The dashboard sends both as `null`, and the two schemas +// disagreed about that: `dailyQuotaResetHour` was `.optional().nullable()`, while +// `dailyQuotaResetTimezone` was only `.optional()`. So `null` passed for the hour +// and was rejected for the timezone, and the whole PUT 400'd on a field the user +// had not touched. The failure surfaced while changing the API type, which made +// it look as though changing the API type was broken. +// +// The storage layer has always coerced these to null (`data.dailyQuotaResetTimezone +// || null` in db/providers/nodes.ts), so accepting null costs nothing downstream. + +const base = { + name: "My node", + prefix: "mynode", + apiType: "chat" as const, + baseUrl: "https://example.invalid/v1", +}; + +test("update accepts a null timezone alongside a null hour (#13066)", () => { + const result = updateProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetTimezone: null, + dailyQuotaResetHour: null, + }); + assert.equal(result.success, true, JSON.stringify(result.error?.issues)); +}); + +test("create accepts the same null pair (#13066)", () => { + const result = createProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetTimezone: null, + dailyQuotaResetHour: null, + }); + assert.equal(result.success, true, JSON.stringify(result.error?.issues)); +}); + +test("a null timezone is accepted on its own, not only beside a null hour", () => { + // The two fields are independent; the pairing above is just what the dashboard + // happens to send. A fix that only tolerated the pair would still reject this. + const result = updateProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetTimezone: null, + dailyQuotaResetHour: 3, + }); + assert.equal(result.success, true, JSON.stringify(result.error?.issues)); +}); + +test("the fields stay optional and blank-string still passes", () => { + assert.equal(updateProviderNodeSchema.safeParse({ ...base }).success, true); + assert.equal( + updateProviderNodeSchema.safeParse({ ...base, dailyQuotaResetTimezone: "" }).success, + true + ); +}); + +test("a real timezone still round-trips", () => { + const result = updateProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetTimezone: "Asia/Ho_Chi_Minh", + dailyQuotaResetHour: 0, + }); + assert.equal(result.success, true, JSON.stringify(result.error?.issues)); +}); + +test("an unknown timezone is still rejected", () => { + // Accepting null must not widen the field into accepting anything: the IANA + // check is the reason this schema exists. + const result = updateProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetTimezone: "Mars/Olympus_Mons", + }); + assert.equal(result.success, false); +}); + +test("an out-of-range hour is still rejected", () => { + const result = updateProviderNodeSchema.safeParse({ + ...base, + dailyQuotaResetHour: 24, + }); + assert.equal(result.success, false); +});