diff --git a/CHANGELOG.md b/CHANGELOG.md index ad46477413..0ddcc1d655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### 🔧 Bug Fixes - **fix(routing):** combo model substitution no longer forwards a client `thinking:{type:"disabled"}` to a target model that rejects it — when a combo/route swaps the upstream model (e.g. `claude-opus-4-8` → `claude-fable-5`), OmniRoute now strips the now-invalid `thinking.type:"disabled"` for models flagged `rejectsThinkingDisabled` (Fable 5 defaults to adaptive and rejects it), preventing the upstream 400 that silently broke Claude Code's internal title/name-generation calls. Models that accept `disabled` (opus/sonnet) are untouched. ([#3554](https://github.com/diegosouzapw/OmniRoute/issues/3554)) +- **fix(usage):** the budget dashboard can now save a budget with some limit fields left empty and clear all limits — `setBudgetSchema` used `.positive()` (rejecting the `0` the form sends for blank fields) plus a superRefine requiring at least one limit `> 0`, so saving with one field filled 400'd and clearing all limits was impossible. Limits now accept `0` (= "no limit for this period"; enforcement only kicks in above 0) and the cross-field minimum was removed; negatives are still rejected. ([#3537](https://github.com/diegosouzapw/OmniRoute/issues/3537)) --- diff --git a/src/shared/validation/schemas.ts b/src/shared/validation/schemas.ts index 9579aaa1ea..edeaf5aac4 100644 --- a/src/shared/validation/schemas.ts +++ b/src/shared/validation/schemas.ts @@ -913,15 +913,13 @@ export const v1CountTokensSchema = z export const setBudgetSchema = z .object({ apiKeyId: z.string().trim().min(1, "apiKeyId is required"), - dailyLimitUsd: z.coerce.number().positive("dailyLimitUsd must be greater than zero").optional(), - weeklyLimitUsd: z.coerce - .number() - .positive("weeklyLimitUsd must be greater than zero") - .optional(), - monthlyLimitUsd: z.coerce - .number() - .positive("monthlyLimitUsd must be greater than zero") - .optional(), + // #3537: a limit of 0 means "no limit for this period" (checkBudget only enforces when + // activeLimitUsd > 0). The dashboard sends 0 for unfilled fields, so 0 must be accepted — + // `.positive()` (rejects 0) used to 400 any save that left a field blank. Negatives are + // still rejected by `.min(0)`. + dailyLimitUsd: z.coerce.number().min(0, "dailyLimitUsd must be zero or greater").optional(), + weeklyLimitUsd: z.coerce.number().min(0, "weeklyLimitUsd must be zero or greater").optional(), + monthlyLimitUsd: z.coerce.number().min(0, "monthlyLimitUsd must be zero or greater").optional(), warningThreshold: z.coerce.number().min(0).max(1).optional(), resetInterval: z.enum(["daily", "weekly", "monthly"]).optional(), resetTime: z @@ -929,19 +927,10 @@ export const setBudgetSchema = z .trim() .regex(/^\d{2}:\d{2}$/, "resetTime must be in HH:MM format") .optional(), - }) - .superRefine((value, ctx) => { - const hasAnyLimit = [value.dailyLimitUsd, value.weeklyLimitUsd, value.monthlyLimitUsd].some( - (entry) => typeof entry === "number" && Number.isFinite(entry) && entry > 0 - ); - if (!hasAnyLimit) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: "At least one budget limit must be provided", - path: ["dailyLimitUsd"], - }); - } }); +// #3537: the previous superRefine required at least one limit > 0, which made it impossible to +// clear all limits (save 0/0/0). Setting all limits to 0 is a valid "disable enforcement" +// operation, so no cross-field minimum is imposed. export const setTokenLimitSchema = z .object({ diff --git a/tests/unit/budget-schema-empty-limits-3537.test.ts b/tests/unit/budget-schema-empty-limits-3537.test.ts new file mode 100644 index 0000000000..4ac839e676 --- /dev/null +++ b/tests/unit/budget-schema-empty-limits-3537.test.ts @@ -0,0 +1,44 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { setBudgetSchema } from "../../src/shared/validation/schemas.ts"; + +// Regression for #3537: the budget dashboard sends 0 for unfilled limit fields, but +// `setBudgetSchema` used `.positive()` (rejects 0) + a superRefine requiring at least one +// limit > 0. Result: saving a budget with only one field filled 400'd, and clearing all +// limits was impossible. A limit of 0 means "no limit for this period" (checkBudget only +// enforces when activeLimitUsd > 0), so 0/all-zero must be accepted. + +test("#3537 saving one limit with the others left at 0 succeeds (Bug 1)", () => { + const r = setBudgetSchema.safeParse({ + apiKeyId: "key-1", + dailyLimitUsd: 5, + weeklyLimitUsd: 0, + monthlyLimitUsd: 0, + }); + assert.equal(r.success, true, r.success ? "" : JSON.stringify(r.error?.issues)); +}); + +test("#3537 clearing all limits to 0 succeeds (Bug 2 — disables enforcement)", () => { + const r = setBudgetSchema.safeParse({ + apiKeyId: "key-1", + dailyLimitUsd: 0, + weeklyLimitUsd: 0, + monthlyLimitUsd: 0, + }); + assert.equal(r.success, true, r.success ? "" : JSON.stringify(r.error?.issues)); +}); + +test("#3537 a budget with no limit fields (only apiKeyId) is accepted as no-limit", () => { + const r = setBudgetSchema.safeParse({ apiKeyId: "key-1" }); + assert.equal(r.success, true, r.success ? "" : JSON.stringify(r.error?.issues)); +}); + +test("#3537 negative limits are still rejected", () => { + const r = setBudgetSchema.safeParse({ apiKeyId: "key-1", dailyLimitUsd: -3 }); + assert.equal(r.success, false); +}); + +test("#3537 a normal positive limit still validates (regression)", () => { + const r = setBudgetSchema.safeParse({ apiKeyId: "key-1", monthlyLimitUsd: 500 }); + assert.equal(r.success, true, r.success ? "" : JSON.stringify(r.error?.issues)); +});