Round-2 review fixes on the budgets endpoint.
Making the route an info route also enrolled it in proxy-only error logging, which
hands the request path to every failure callback. That gate exists to stop management
endpoints leaking temporary keys, and /key/info is immune only because its key is a
query param. The path form now rejects a plaintext key and asks for the hash, since a
key in a URL also reaches access logs and span names regardless of this gate.
The reservation layer builds no counter for a non-positive cap, so reporting its
tightened operator there turned a team, tag or end user sitting at max_budget 0.0 into
"exceeded" when both layers admit the request. The tightening now needs a positive cap.
Per-model budgets are enforced per request model, not per cap, so reporting the highest
counter under a cap claimed a denial that no request would hit. Each request model that
maps onto a cap gets its own row, identified by the model whose counter it reports.
Deployment names join the candidates, because routing straight at a deployment keys the
counter on that name rather than on a model group.
The reservation note claimed more than the layer delivers: a request it cannot price up
front is gated by the read-time check alone. Reworded rather than re-implemented, since
introspection cannot know the request.
Severity is now the fallback for a code a client has not been taught yet, not a ranking:
warning means the numbers may be incomplete or misread, info means they are accurate.
Also dropped the token end-user cap plumbing, which could only ever attribute the
caller's request-scoped cap to the inspected key, and memoised is_info_route, which the
pattern change had put 25 uncached regex builds per request behind.
Each caveat now arrives as its own note with a stable code, so the table renders
one line per caveat at its own severity instead of one prose blob, and decides
whether a row is dead by code rather than by matching wording.
A row that structurally cannot trip, like a project budget whose spend is never
incremented, now says so and sorts below every row that can, since it is never
the answer to which budget stopped a request.
spend_state replaces inferring absence from a null spend. A no-counter-yet zero
keeps its $0.00 and its meter because it is genuinely zero, while a failed read
still shows Unknown with no meter. Any state this build predates is treated as
unreadable rather than drawn as a confident number.
CODE_KILLS_ROW is exhaustive over the code union, so a caveat added server-side
fails the build until it is classified, and severity is the runtime fallback for
a code that arrives from a newer server than this bundle.
`note` shipped as one prose string, so a client had to substring-match it to act
on anything, and two caveats arrived joined by "; " with no way to render them as
the list they were. Each caveat is now a `KeyBudgetNote` with a stable `code` to
branch on, an `info`/`warning` severity, and `text` that stays free to reword, and
`notes` is always a list rather than a nullable string.
Severity is the difference between a row that cannot trip, like a project budget
whose spend is never incremented, and one that needs reading, like a rolling window
or a scope the reservation layer already blocks at the limit.
The absence of a spend number was also being smuggled through the same field. It is
data quality, not a caveat, so it moved to `spend_state`: `live`, `no_counter` for a
per-model budget whose cache-only counter does not exist yet, and `unavailable` for
a read that failed. A failed read no longer renders as untouched headroom.
The endpoint has not shipped, so this costs nothing today and would be a breaking
change tomorrow.
SpendBudgetCell coerces a null spend to 0 and draws an empty meter, so a
budget whose live counter could not be read rendered as untouched headroom.
The key budgets table now branches before that: an unreadable spend reads
"Unknown" against its limit with no meter, which is the difference between
"we could not tell you" and "there is nothing to tell".
The endpoint's longest reachable note is 276 characters over three clauses,
on an end_user row where budget reservation tightened the operator and a
custom auth callable is configured. Assert the whole string renders and that
the note carries no clipping utility, so re-adding truncate fails the test
rather than silently hiding a caveat.
Regenerates schema.d.ts for the end-user docstring change, which the types sync
gate needs.
Notes now carry two clauses, e.g. "alert only, never blocks; compared against
recorded spend rather than the live counter", and the scope cell truncated them
to one clipped line behind a native tooltip. They are the most load-bearing text
in the table, so they wrap now and the column is wider to suit.
The threshold helper always read `comparison` per row, but its doc comment and
the fixtures around it asserted that team enforces ">", which is no longer true:
reservation tightens team, tag and end_user to ">=", and disabling reservation
relaxes them back. Nothing about the rendering changes, but the tests now pin
that one scope can render either operator instead of teaching a wrong default.
end_user_id was free-form and the lookup was a global find_unique with nothing tying the end
user to the key, its team or the caller. Any valid key could read any end user's alias, budget,
live spend and reset times, and a null entity_label told the caller whether an id existed at
all. /key/budgets made that reachable for every role, since a caller trivially satisfies the
key check by inspecting their own key.
End users are a proxy-global namespace, LiteLLM_EndUserTable carries no team or organization
column, so there is nothing to scope a non-admin against. Gate the parameter on proxy admin and
admin viewer, matching /customer/info and /customer/list. The gate is on the parameter, not the
route, so a non-admin reading their own key's budgets is unaffected.
Four ways the report disagreed with enforcement.
The comparison came from auth_checks alone, but reservation runs first and refuses once spend
has reached the cap, so team, tag and end_user really block at >= while the row claimed >. A
team at 300 of 300 read "ok" next to a member at 50 of 50 reading "exceeded", with nothing on
the row to explain the difference. Every scope reservation covers now reports >=, status
follows that same operator, and a row whose operator was tightened says so. Scopes reservation
does not cover keep their read-time operator, and so does everything when
disable_budget_reservation is set.
Per-model spend is counted under the request model, but the report read the model_max_budget
key. A cap on "gpt-4o" with callers sending "openai/gpt-4o" enforced against a warm counter
while the row showed no spend at all and blamed a cold cache. Probe every model that routes to
the cap and report the highest, since each counter is compared against the cap on its own.
get_request_model_budget_key now owns that matching so enforcement and the report share it.
A single malformed budget_limits or model_max_budget entry dropped every window or model in
the scope, hiding budgets that were still being enforced. Bad entries are skipped one at a
time now.
The end user cap came from the row, but reservation reads the request token's first. Mirror
that precedence, and say plainly when a custom auth callable could be setting one out of view.
is_info_route compared the request path against info_routes with a bare `in`, so an entry
carrying a path parameter could never match: the incoming route holds a resolved id, not the
`{key_id}` template. /key/{key_id}/budgets was therefore refused for the view-only, team and
customer roles that /key/info serves, and its info_routes entry matched nothing at all.
Route it through check_route_access, the way is_management_route already does. It is the only
templated entry in the list today, so no other route changes reachability.
Scopes disagree on whether hitting the limit exactly is over it. team_member
enforces >= so 50 of 50 is already denied, while team enforces > so 300 of 300
still passes. The table rendered the numbers and the server's status but not the
operator, so two rows could show identical spend and limit with opposite
statuses and nothing on screen explaining why.
Each row with a limit now states its own threshold, "Blocks at >= $300.00"
against "Blocks at > $300.00", and a soft row says "Alerts at" so the wording
never promises a block it cannot make.
entity_type was declared str, so the generated client saw a bare string
even though the resolver already carried a Litellm_EntityType and only
called .value at the boundary. Declaring the enum keeps the wire format
identical and lets a caller match a BudgetExceededError's entity against
a row without comparing loose strings.
key is populated on every 200, since a request that resolves no key 404s
before the response is built, so declaring it optional understated the
contract.
The team member budget check ran twice. common_checks calls
_check_team_member_budget behind skip_all_budget_checks, which correctly leaves
non-LLM routes alone, while a second inline copy in _user_api_key_auth_builder
was gated only on the zero-cost-model check and so fired on every route. A
member who had spent their in-team budget got a 429 from /key/info and could no
longer see which budget had stopped them.
The surviving check is a superset of the deleted one: it accepts a per-member
max_budget of 0 as an explicit disable where the inline copy required > 0, it
compares with >= where the inline copy used >, and it adds the team-level
team_member_budget_id default the inline copy never read. common_checks runs
unconditionally for every authenticated request, so nothing stops being
enforced on LLM routes.
Behaviour change: management and UI routes are no longer blocked by an
exhausted team member budget. LLM routes still are.
A BudgetExceededError names one entity, so a caller who gets a 429 still has to
read auth source to work out which of the key, its windows, its per-model caps,
its team, their membership in that team, the owning user, org, project, the
key's tags, the end user or the proxy-wide limit produced it. This returns all
of them in one call, with the live spend and reset schedule of each, including
the scopes that are left unconfigured so they can be ruled out without opening
every object.
GET /key/budgets reports the calling key. Both routes reuse
_can_user_query_key_info, so reading another key's budgets needs the same rights
as reading its info, and 404 on an unknown key matches /key/info.
The report has to agree with enforcement or it is worse than nothing, so the
resolver consumes the same UserAPIKeyAuth get_key_object hands the auth path,
reads spend through get_current_spend, and shares the limit resolution with the
checks: counter key strings now come from one spend_counter_keys module, and the
team-member, personal-budget-on-team-key and budget-org-id rules were extracted
out of auth_checks for both callers. Each row carries the operator its check
actually uses, since they differ per scope, plus a note where a budget cannot
behave the way its numbers suggest.
Renders GET /key/{key_id}/budgets as a table of every budget that can gate the
key, so a 429 that names no entity can be traced to a row without reading auth
source. Rows sort blocking-first, and a soft budget that is over reads as
"Exceeded (alert only)" against "Blocks requests", so an alert can never be
mistaken for the thing that rejected the request. Scopes with nothing configured
still get a row, rendered "Unlimited" rather than $0, which is what lets someone
rule out the org and the team without clicking into them.
The panel is deliberately not keepMounted, so the request is lazy and its cells
cannot collide with the keepMounted Overview panel.
InheritedBudgetHint keeps its client-side team/org guess on the Overview card and
the keys list, where no tab is in reach, but its tooltip now says it is not the
full list and points at this tab, so the two cannot read as competing answers.
An OpenAPI-backed MCP tool whose upstream answered 401 came back as a
successful tool result carrying the upstream's rejection as its content, so a
caller saw {"error":"invalid_token"} presented as data and the gateway recorded
the request in its own spend log as call_mcp_tool | success.
Three layers each erased the outcome. The request function returned
response.text whatever the status, _handle_local_mcp_tool caught every exception
and returned it as ordinary TextContent, and both dispatch sites then stamped
isError=False unconditionally. Fixing only the first, which is the obvious fix,
changes nothing, because the two above it still map failure onto the
success-shaped value.
The status is now classified where the response is held: a 401 becomes
MCPUpstreamAuthError so the caller is told to re-authenticate, and every other
non-2xx becomes MCPOpenApiUpstreamError, which carries the status and drops the
upstream body rather than serving it as tool content. _handle_local_mcp_tool no
longer swallows, and the call_tool arm keeps the auth error's type. Nothing new
renders these: call_mcp_tool and call_tool_rest_api already turn them into an
isError result naming the status and into a real 401 with WWW-Authenticate, and
the OpenAPI path simply never reached them.
The result is now byte-identical to the regular MCP path for the same failure.
Two or more reasoning keyword matches promoted a request straight to the
REASONING tier no matter what the weighted score said, so "hi, step by step,
pros and cons" scored 0.100 and still bought the most expensive tier.
Require the score to clear the simple_medium boundary before the override
applies. Promotion from MEDIUM or COMPLEX is unchanged; only prompts the
scorer already placed in the cheapest band stay there.
* chore(ui): upgrade the dashboard to React 19
Bumps react and react-dom from 18.3.1 to 19.2.8 with matching @types. Next 16 already required a React 19 peer, so this aligns the dashboard with what the framework expects and unblocks Base UI and shadcn work that assumes the React 19 ref model.
React 19 passes ref through as a regular prop, so the setup file's forwardRef tripwire and the ref-forwarding test's forwardRef case no longer describe real behavior; both now assert the React 19 contract instead. useRef<T>(null) now yields RefObject<T | null>, which is the one prop type MessageList had to widen.
* test(ui): wait for a Base UI select popup to open before clicking an option
The option lands in the DOM one render before the popup finishes entering, while its positioner still carries pointer-events: none, so clicking it throws. Waiting on the option's text alone was a race that React 19's flush timing loses, which is why four ToolPolicies cases went red on the bump.
chooseSelectOption in test-utils opens the trigger, finds the option by role, waits for it to stop being pointer-blocked, then clicks. It also replaces the last-match-by-text hack, which only worked because the popup happens to portal after the table.
Moves all 33 antd Alert usages across 20 dashboard files onto
src/components/shared/Alert, following the composition the rest of the
dashboard already uses: message becomes AlertTitle, description becomes
AlertDescription, showIcon becomes a lucide icon child, and closable
becomes an AlertAction ghost button.
antd type="success" has no counterpart on the shared Alert, so the two
success sites land on the default variant with a CircleCheck icon, which
is what cloudzero_export_modal and CloudZeroIntegrationSettings already
do for the same case.
LoginPage's dismissible SSO notice moves into its own SsoEnabledNotice
component in the same file: antd's closable carried its own dismiss
state, and inlining it pushed LoginPageContent past the complexity
budget.
Four files lose their last antd symbol, so their no-restricted-imports
suppressions are pruned by hand. antd import sites drop from 115 to 111
across 107 to 103 files, and the no-restricted-imports ratchet drops
from 119 to 115 over 110 to 106 files.
One test asserted antd's own ant-alert-info class; it is repointed to
the shared Alert's text-info variant class, which keeps the same
"info, not warning" check. Every other colocated test passes untouched.
* feat(auto-router): scope shadow eval jobs to multiple keys
A shadow eval job now covers a set of keys instead of exactly one, and each
key carries its own max_turns budget, so one key exhausting its budget leaves
its siblings sampling. The existing job row already is the per-key unit
(api_key_id, max_turns, stopped_at, and the one-active-per-key-and-direction
partial unique index all live on it), so multi-key is grouping rather than
schema surgery: a new group_id column ties N sibling rows written atomically
by one create_many, the API's job id becomes the group id, and pre-existing
jobs backfill group_id = id so their ids keep resolving. The sampler hot path
is untouched; its test file has a zero-line diff
Results come back pooled plus a per-key breakdown and responses list every key
with its own budget, stop state and read-time labels. The dashboard is adapted
minimally to the new shapes (the picker stays single-key and submits a one-key
list); the multi-select picker and per-key table land in the stacked UI PR
* fix(shadow_eval): derive completed from spent budgets and record operator stops
* fix(shadow_eval): stamp stops atomically and freeze counts at the stamp
The stop endpoint wrote stopped_by and stopped_at as two separate updates, so
a failure between them left a job reading stopped while its unstamped legs
kept sampling, and the retry got 400 already stopped. One UPDATE now stamps
stopped_by and every missing stopped_at together, preserving the stopped_at a
leg earned from its own budget via COALESCE
Attempt counts now exclude attempts that land after a leg's stopped_at, so an
in-flight attempt finishing just after an operator stop can never push a
legacy pre-stopped_by job over its budget and flip it from stopped to
completed at read time
* fix(shadow_eval): backfill stopped_by so legacy stops never read as completions
* chore(ui): regenerate api types for the shadow eval stop fields
* fix(shadow_eval): let the stop statement pick one winner under racing stops
Two operators can both pass the derived-status guard in the race window. The
stop UPDATE now claims only legs with stopped_by still null and the endpoint
judges by its row count, so exactly one caller ever gets the 200 and the loser
gets the same already-stopped 400 a late caller gets
* refactor(shadow_eval): make the stop statement the whole state machine
The status guard ran before the UPDATE, so a stop racing the last budgeted
attempt still claimed the job and it read stopped forever instead of
completed. The statement now claims the job only while a leg still samples
inside the window with no stop recorded, and the endpoint reads once after
writing: a racing operator, a same-instant budget spend, and a repeat stop all
get the 400 naming the status the job actually holds. The pre-write guard and
the hand-built response go away
* chore(ui): regenerate api types for the stop route description
The flat-cost rollup reads deployments only from LiteLLM_ProxyModelTable, so a PTU
deployment declared in config.yaml never accrues flat cost. Those deployments live in
llm_router.model_list as plain dicts whose id sits in model_info rather than on the entry,
so they do not satisfy the shape _parse_ptu_model reads.
Adds a frozen record in that shape and a factory that maps a router entry onto it, leaving
_parse_ptu_model byte-identical so the existing cases stand as evidence of no behaviour
change. Nothing calls the factory yet; the caller lands with the loader union.
_decode_model_info also stops handing back valid JSON that is not an object. It decoded
a list or a scalar and returned it as a mapping, so the caller read fields off it and
raised, losing the whole run rather than the one bad deployment.
* test(ui): split the vitest suite into unit, component and integration tiers
Every test file booted jsdom, including the ~1800 that assert pure functions
and never render. They now run as a separate vitest project in the node
environment, where the whole tier finishes in under four seconds.
The tiers are vitest projects rather than a naming convention, so CI can run
them as independent jobs. A .test.ts that renders React, a hook test being the
usual case, is listed explicitly and stays in the jsdom tier.
* test(ui): report per-test duration against a per-tier budget
A timeout only catches a hung test, and it has to stay generous enough to
survive a loaded runner, so it never reports the multi-second render tests that
make CI fail the moment the box is busy. Budgets are separate and far tighter:
50ms unit, 1s component, 3s integration.
The counts are laptop measurements, so the CI job is report-only for now.
Flipping it to blocking is one line once CI has published its own numbers.
* test(ui): run the tiers as separate CI jobs and stop clicking popups by text
The old job ran every file in one process, so the single slowest file set the
wall clock and a bigger box bought nothing. The tiers now run as separate jobs
with the component tier sharded four ways.
getByText and findByText match hidden nodes, so they resolve against a closed
Base UI popup whose positioner still carries pointer-events: none, and the
click lands or not depending on how far the open transition got. Two files
failed this way, one three runs in five and one every run. Querying the option
by role waits for it to be visible, and both are now stable. A lint rule keeps
the pattern from coming back.
* test(ui): give React Testing Library's async queries a CI-sized window
findBy* and waitFor run on asyncUtilTimeout, which defaults to 1000ms and is
independent of vitest's testTimeout. Raising the vitest timeout therefore did
nothing for them: a query still gave up after one second while the test had 59
seconds of budget left, which is why a loaded runner produced 'Unable to find
role=...' rather than a timeout.
UserSearchModal is the worked example. The role query it makes resolves in
249ms on a laptop and blew past 1000ms on CI, failing the run at 1494ms. Five
seconds keeps the same assertions and only widens the window a failing query
waits before reporting; a passing query still resolves the moment the element
appears.
* test(ui): calibrate the tier budgets from real CI numbers and report by default
The first CI run showed the laptop counts were badly off: component 176 local
against 326 on CI, integration 87 against 128. The maxima now come from that
run with headroom.
continue-on-error still painted the check red, which is the opposite of the
point, so the report-only decision moves into test-budgets.json as an explicit
enforce flag. The job passes and prints the counts; flipping enforce to true
makes it a gate.
* docs(ui): drop the CLAUDE.md edits from the tier split
Keeping this PR to the vitest, CI and test changes.
* ci(ui): run every tier in one job instead of eight check rows
Sharding bought nothing. Measured on the first run of this branch, the
component tier unsharded finishes in 198s while the integration tier is floored
at 384s by a single file, so integration was always the critical path and the
four component shards only added rows. One job running every project comes in
around 384s against the 426s the split jobs took.
Eight rows named things like 'component (2)' also told a reviewer nothing, on a
PR page that already carries forty checks.
The job keeps the id ui-unit-tests because guard-internal-staging requires that
exact context; renaming the jobs had silently stopped it reporting, which would
have blocked every merge on a check that no longer existed. The workflow's
display name becomes UI Tests since it runs more than unit tests.
The tier split itself is untouched: it lives in the vitest projects config, so
the unit tier still runs in node with no jsdom, and each tier keeps its own
timeout and budget.
* fix(ui): stop the type check from running the whole suite a second time
test:types was 'vitest --run --typecheck.only'. Under test.projects that flag
is ignored and the root-level typecheck block is not inherited, so the step
collected each project's normal include and ran all 8464 runtime tests instead
of type-checking. It took 542s on CI against 33s on the flat config it
replaced, and the job then ran the same suite again in the next step.
Typecheck now belongs to a project of its own, with an empty include so it
contributes no runtime tests, and the CI job runs one vitest invocation for all
four. The type tier adds about 3s to a full run and reports 'Type Errors: no
errors' rather than a suite of tests.
Verified it still catches things: breaking SortingState in DataTable.test-d.tsx
fails with 'Type number is not assignable to type string' and exit 1, and
restoring it passes.
* test(ui): scope the split down to the vitest tier projects
Removes everything from this branch that was not the tier split.
The three lint rules brought 1381 lines of grandfathered suppressions in
eslint-suppressions.json, which is 81% of the branch's added lines and
debt nobody is going to pay down. The per-test duration budget does not
scale as a CI step. Both are gone, along with the two query rewrites the
no-click-by-text rule forced: those files pass 10/10 at this base, quiet
and under load, so there was no failure behind them.
The workflow is byte-identical to the base again. It already runs
npm run test:types and then vitest related on pull requests, so PR cost
is unchanged; the split only repoints test:types at the new project.
That project is required, not optional: vitest silently ignores
--typecheck.only under test.projects, so without it the type script
collects the whole suite instead of the one typed file.
Restores the base 60s testTimeout on the unit tier. The 5s cap was not
part of the split and failed ChatShell.serverRootPath.test.ts, a 960ms
test, under load.
Moves all 58 antd Button JSX sites across 24 files onto the shadcn
Button, leaving zero antd Button importers.
Prop mapping follows what already merged rather than a new convention:
type="primary" to the default variant, a bare button to outline (the
house default), type="text" to ghost, type="link" to link,
type="dashed" to outline plus border-dashed, danger to destructive,
size="small" to sm, htmlType to type, block to w-full, the icon prop to
a child, and loading to disabled plus aria-busy. Icon-only buttons take
the matching icon-* size. Inline style props that had a direct utility
equivalent moved to className, and the opacity toggle on the create key
submit is dropped since the base cva already carries disabled:opacity-50.
Base UI's Button defaults type to "button" for native buttons, the same
default antd used, so bare buttons inside a form do not start
submitting.
guardrail_info.tsx and tag_info.tsx lose their last antd import, so
their no-restricted-imports suppressions are pruned. The other 22 files
keep other antd symbols and keep their entries.
One behavior change: the MCP transports docs link now opens in a new
tab, matching every other external docs link in the dashboard, instead
of navigating the dashboard away.
The ModelSettingsModal loading assertion moves off antd's spinner
element onto aria-busy, which is what the rest of the suite already
asserts; that markup cannot survive removing antd.
Tremor's TabList defaulted to the underlined `line` variant and its TabPanel
rendered every panel, hiding the inactive ones with a class. The shadcn
TabsList defaults to a segmented pill and Base UI's TabsPanel unmounts a
hidden panel unless it carries `keepMounted`, so the conversion waves quietly
changed both on the pages that took tremor's defaults.
Restores the underline on the nine strips whose tremor markup carried no
`variant`, leaving the ones that were explicitly `variant="solid"` as pills,
and puts `keepMounted` back on the thirteen files whose panels used to stay
mounted, so filters, scroll position and in-progress input survive a tab
switch again.
Seeding the old usage page's activity state properly comes with it: it was
cast from `{}`, so the panel crashed on `data.length` the moment it mounted
before its fetch resolved, which only stayed hidden while the panel was
unmounted until first opened.
user.type dispatches one event per character and re-renders the whole form on
each one, so a test that only needs a field to hold a value pays for every
keystroke. Where the value is all the test wants, fireEvent.change sets it in
one go.
Measured against a control on the same tree: the 100 converted files went from
654s to 619s of CPU, 5.4% cheaper, while the files nobody touched drifted 1.2%
the other way. Modest, and honest about it.
Rolled out one file at a time, running each before and after and keeping the
conversion only where the file stayed green. That rejected 35 files, all cases
where the keystrokes are the behaviour: Base UI comboboxes drive their filter
from real keyboard input and ignore a raw change event, and the same goes for
autocompletes, debounces and key handlers. Those keep user.type.
Both OpenAPI dispatch arms sourced the upstream credential only from the
deprecated global / BYOK mcp_auth_header and never from mcp_server_auth_headers,
so x-mcp-{alias}-authorization was silently dropped on spec_path servers and the
upstream API received no Authorization at all. The managed path already resolves
it through lookup_mcp_server_auth_in_headers, so the two had drifted.
_resolve_openapi_tool_auth now owns that resolution for both arms. A per-server
value is already a complete header value and is forwarded verbatim, while a BYOK
credential keeps its auth-type prefix, so the two are never conflated into
"Bearer Bearer <token>". The resolved credential is also handed to
resolve_openapi_upstream_auth, whose passthrough arm reads it through
_passthrough_token_from_mcp_auth_header and outranks the ContextVar.
server.py loses its inlined copy of the forwarded-header logic along with its
mcp_server is None guards, which are unreachable after the 503 raised above them.
Credit to the earlier analysis and approach in #33349, which this supersedes
against the current v2 credential resolver.
* refactor(ui): port the MCP server forms off antd Form onto react-hook-form
The MCP create and edit forms were the last antd `Form` graph in the
dashboard. antd's `onFinish` hands back only the fields mounted at submit
time, while react-hook-form with `shouldUnregister: false` hands back the
whole store, so a direct port would quietly widen every create and update
request.
All 14 files now bind through `MountedFormField`, whose mount registry
reproduces antd's mounted-only submit: both roots build their payload from
`projectMountedValues` instead of `getValues`. `mcpFormStore` carries the
rest of the FormInstance surface the two roots relied on, each piece
matched to what rc-field-form actually does rather than to what the API
name suggests: `setFieldsValue` deep-merges plain objects and writes an
explicit `undefined`, `resetFields` restores the seeded values rather than
clearing the key, and `onValuesChange` is rebuilt from a `watch`
subscription filtered to user input, carrying a single changed branch.
Two watches needed the mount gate moved rather than translated.
`MCPPermissionManagement` renders outside the transport gate that mounts
`auth_type`, so antd's watch read `undefined` there and mounted the
pass-through toggle; the effective auth type now arrives as a prop that
each root computes with exactly that gate. The edit root reads every watch
off the mounted projection for the same reason, which also stops the token
material in a saved server's credentials from reaching the tool preview.
`Form.List` becomes `useFieldArray` plus a `useMountedName` registration
for the list key itself, because antd registers a list as one field: a
per-user variable row keeps the `value` its scope hides, and an empty list
still submits `env_vars: []` instead of dropping the key.
* test(ui): cover the mount registry's unregister path on the real primitive
The existing MountedFormField suite drove a hand-written registry whose
register returned a no-op, so nothing exercised useMountRegistry's
ref-counting or the cleanup that React wires from useMountedName's effect
return value. A reviewer read that gap as a missing unregister.
These three cases drive the real hook through a gated tree: a key leaves
the submitted payload when its gate unmounts the field, a required field
that unmounts stops blocking submission, and a name held by two fields
survives one of them releasing it.
Verified by mutation: rewriting the effect body to discard the cleanup
turns the first two red, the second reporting the reviewer's exact
symptom, "expected [ 'server_name', 'token_url' ] to not include
'token_url'".
* test(ui): prove the permission panel's booleans reach the create payload
CreateMCPServer.integration.test.tsx mocks MCPPermissionManagement, so the
four booleans createServerPayload writes were invisible to every existing
create-side test. vi.mock is file-scoped, so rendering the real panel needs
its own file.
Four cases, each killed by a different mutation:
unbind allow_all_keys -> "sends allow_all_keys true"
unbind available_on_public_internet -> "sends the panel's defaults"
invertedSwitchControl -> switchControl -> "sends ... false when the
operator restricts"
isOAuth2 gate forced open -> "omits delegate_auth_to_upstream"
A payload assertion expecting false cannot detect an unbound field, since
Boolean(undefined) is false too, so the two cases carrying unbinding
detection are the ones asserting true. The other two are pinned by the
switch-inversion and mount-gate mutations instead.
PassThroughGuardrailsSection threads its `disabled` prop to the guardrail
selector and to all three quick-add buttons, but not to the two `TagsInput`
controls for request and response field targeting. Every other prop crossed,
so the section greys out while a user can still type field names straight
into both tag inputs.
Latent rather than live: neither call site passes `disabled` today, so the
prop is currently dead across the tree. This arms the gate for whoever passes
it first.
`TagsInput` already honours `disabled` end to end, measured rather than read:
with it set, the input carries a real disabled attribute and all three commit
paths, token separator, Enter and blur, are inert. So the fix belongs at the
call sites and the primitive needs no change.
The new test drives the control rather than asserting an attribute, since a
Base UI control can paint a disabled state while still accepting input. The
enabled cases are the liveness gate: they type the same text through the same
gesture and assert it commits, so the disabled cases cannot pass vacuously.
* refactor(ui): extract the MCP server edit save payload into a pure builder
`handleSave` built the update payload inline across 276 lines, spreading
`...restValues` straight off a mounted-only `onFinish`. That makes the payload a
function of which fields happen to be mounted, and it leaves no seam to test the
shape without rendering the whole edit form.
Move the payload construction into `editServerPayload.ts` as
`buildEditServerPayload(values, ui)`, a pure function over the submitted values
plus the nine pieces of component state the handler reads. Failures become values
rather than early returns with a toast: the six error branches are a tagged union
that `editPayloadErrorMessage` maps back to the exact strings shown today, via an
exhaustive switch. `handleSave` keeps the network call, the OAuth token
persistence and its own try/catch.
This is a move, not a rewrite. To prove that, `editServerPayload.differential.test.ts`
holds a baseline machine-extracted from the pre-refactor function body by line
range, with the failure branches converted by exact string replacement. The
generator refuses to emit unless the slice is still present verbatim in the
source, every conversion matches exactly once, no toast call survives, and a
deliberately corrupted probe still trips that check. 59 scenarios run both
implementations and compare the payload object, its key order, and its serialised
bytes, so a re-ordering that leaves values untouched is caught too.
The duplicate local `AUTH_TYPES_REQUIRING_CREDENTIALS` is dropped in favour of the
identical exported list, and `reduceStaticHeaders` is shared with the create side.
Both were verified equal before reuse.
Behaviour is unchanged. The 466 pre-existing tests in the directory pass unedited.
* refactor(ui): type the MCP edit payload builder instead of Record<string, any>
The extraction created a new public signature, so it should carry a real
contract. buildEditServerPayload now takes EditServerFormValues and returns
EditServerPayload, both declaring every field the builder actually reads and
writes, with an unknown-valued index signature for the keys the form passes
straight through. handleSave is annotated too, so antd's untyped onFinish
value is narrowed once at the boundary rather than travelling as any.
Fields that arrive from the store with their own runtime validation
(static_headers, env_vars, credentials) stay unknown rather than being given a
narrower declared type the form does not actually guarantee. Values are not
run through a parser: the payload's serialised key order is part of the
contract this module exists to hold, and rebuilding the object would reorder
it.
The credentials assignment moves from two post-hoc mutations to a single
resolved entry, which keeps the payload readonly end to end and lands the key
in the same position in all four branches.
Behaviour is unchanged. The 59 differential scenarios still match the frozen
pre-extraction body on object, Object.keys order and JSON.stringify bytes, and
the mcp-servers suite is 525/525 across all 30 files. Three tsc probes confirm
the new types have teeth: a wrong payload assignment, a misspelled field read
and an invalid value each fail the type check.
* feat(ui): add mounted-field projections for the MCP server form graph
antd's onFinish reports exactly the fields mounted at submit time, and both MCP
server payload builders spread that object straight through. react-hook-form
with shouldUnregister false hands back the whole store instead, so a port needs
the mount set written out explicitly before any JSX moves.
This adds mountedEditFieldNames / mountedCreateFieldNames as pure functions over
the form values, plus the projections that apply them, covering all 22 gates
across the graph's 89 named bindings. No JSX changes, nothing imports them yet.
Three behaviours are probed against antd 5.29.3 rather than assumed, and the
tests pin all three:
- a mounted-but-unset field is EMITTED as a key holding undefined, at the root
and inside credentials, so the projection emits rather than omits
- a Form.List row is NOT projected down to its mounted sub-fields, so env_vars
rows pass through whole; filtering them would drop per-user values
- the two roots disagree on more than StdioConfiguration: edit gates url on a
deny-list while create uses an allow-list, and create additionally gates the
whole auth section on a non-empty transport
* refactor(ui): port the create key form off antd Form onto react-hook-form
antd hands onFinish exactly the fields mounted at submit time, so a collapsed
section contributes nothing to the request while the values typed into it
survive for re-expansion. react-hook-form reaches only one of those two
behaviours per shouldUnregister setting, so the store is kept intact and
projected down to the mounted set through an explicit mount registry.
MountedFormField carries the rest of the Form.Item contract the payload
depends on: defaults taken from each field's own declaration rather than a
blanket empty value, and help text that replaces the rule message instead of
sitting beside it.
The 60-case submit differential runs unedited against the port, joined by
cases for the writers outside the submit path, mounted-set validation, Enter
to submit, and switch coercion.
* test(ui): pin the mounted-field sets by membership, not array order
Two credential assertions compared the returned array with toStrictEqual
against a literal in source order, so they failed when two names were
swapped inside the source array even though the projected payload was
unchanged. Key order is not observable in the payload, so those two
assertions rejected a refactor that changes nothing a caller can see.
Route both through the same sorted() helper the other fifteen set
assertions already use. Membership keeps its teeth: deleting any one of
the eighty emitted field names still fails the suite, while reordering
two of them now passes.
* docs(ui): state the mounted projection's static-name limit at its export
The registry counts by name and the projection emits flat keys, so a
Form.List row and its per-row sub-fields, whose names are generated at
runtime, are never in the mounted set and go missing from the payload.
That is silent and it is correct for every static field around it, so the
contract belongs where the next consumer reads it.
* refactor(ui): cut the mounted field's explanatory comments to the contract limit
The mechanism the projection uses and the reason a helped field hides its
rule message are both derivable from the code, so they belong in the pull
request rather than in two places. What survives is the one thing no reader
can derive: that a runtime-generated name is silently absent from the payload.
* refactor(ui): drop the doc comment from MountedFormField
The static-name constraint it described moves to the PR description, where
it is not a second place to keep in sync with the code.
* fix(ui): rebuild nested and list paths in the mounted-field projection
projectMountedValues emitted one flat key per registered name, so a field
registered under a dotted path produced a literal "credentials.client_id"
key instead of a nested credentials object, and a field-array row produced
"env_vars.0.name" instead of a row.
Both are silent. buildEditServerPayload destructures credentials and passes
it to buildCredentials, which returns undefined for a non-object, so every
credential drops out of the payload while the code compiles and the existing
suites pass. reduceStaticHeaders loses its rows the same way.
Split each registered name on "." and rebuild the value, treating a numeric
segment as an array index so field-array rows come back as arrays rather
than objects keyed by digits. Nested credentials and both field-array sites
are the same defect and take the same fix.
* fix(ui): make mounted-field nesting opt-in via array names
The first cut split every registered name on ".", which diverges from antd.
antd's getNamePath is toArray, so a string name is a one-element path and
"a.b" is stored as a literal flat key; only an array name nests.
check_openapi_schema registers names taken from a live /openapi.json at
runtime, so the field set is an input rather than source. A spec property
containing a dot would have silently nested under the unconditional split
and changed that payload with nothing failing.
Accept string | readonly string[]. An array nests, with a numeric segment
as an array index, which is what the credential paths and the field-array
rows already use. A string stays one literal key. The registry keys by the
joined path but projects from the original shape, so the two cannot drift.
The suppression said exercising this component requires a real antd Form.
That stopped being true once the create key form began driving it through
a bare mounted field, so the harness and the suppression go together.
Two of the auto-rotation cases were racing the Base UI select popup all
along: findByText resolves as soon as the option lands, while the popup
still carries pointer-events none until its open transition settles. The
antd harness happened to hide it, at 0 failures in 6 runs against 1 in 6
for plain state and 5 in 6 under a Controller, since each extra render is
another chance to catch the popup mid-transition. Both now take the same
PointerEventsCheckLevel.Never their sibling case already used.
* fix(ui): restore the cache control Role and Index field hints
The add_model cache control editor lost both field hints when it moved off
antd Form.Item in #37392. "LiteLLM will mark all messages of this role as
cacheable" and "(Optional) If set litellm will mark the message at this index
as cacheable" went with the Form.Item tooltip props and neither string exists
in dashboard source any more. The Index hint was the only thing telling a user
that field is optional, so this is lost information rather than styling.
Both come back as shadcn tooltips beside their labels, matching how the
surviving switch-level hint is already rendered.
Also adds the payload characterization net this graph did not have. Before
this commit the seven suites over add_model and model_add held 37 cases, no
antd module mock, and zero toStrictEqual, so nothing pinned the submit
payload. AddModelPanel.integration.test.tsx drives the real panel, the real
antd store and the real prepareModelAddRequest, and asserts the object handed
to modelCreateCall.
It pins the distinctions only a strict assertion can see: litellm_credential_name
arrives as null from its initialValue while api_key, api_base, mode and
access_groups arrive as undefined, and team_id is absent entirely until the
Team-BYOK switch mounts it. It also pins the mount gate in both directions,
since a collapsed Advanced Settings drops both its keys and anything typed
into it while re-expanding restores them, and the empty-string skip, since a
cleared api_base must vanish rather than arrive as "".
Every fixture was captured from the running component rather than written by
hand. A 12-mutation battery over the bindings, the empty-string skip, the two
required rules and an added keepMounted all go red, each run gated on having
executed the expected case count.
* refactor(ui): port the add model form off antd Form onto react-hook-form
The Add Model form graph is shared by three antd hosts, so it only moves as
one piece: AddModelPanel, LlmCredentialsPanel and CredentialModal all mount
the same children. Form and Form.Item are replaced everywhere, and every
widget inside them is left alone, so the change is the binding layer only.
antd submits the mounted fields, react-hook-form submits its whole store. A
shared mount registry keeps that difference from reaching the request: each
field registers on mount, and the panel projects the store down to the
registered names before it builds the payload. shouldUnregister would have
been the other option, but it drops a collapsed section's typed values, so
re-expanding Advanced Settings would come back empty.
The antd rules modules are reused as-is through a thin validator adapter, so
the messages stay in one place rather than being reworded per field.
Advanced Settings held a Form.useForm() instance in a component that renders
no Form, which made ten imperative calls dead. They are removed rather than
translated, and the three behaviours they looked like they drove were checked
against the antd original first: invalid LiteLLM Params still blocks submit,
the pass-through toggle still leaves LiteLLM Params empty, and turning custom
pricing off then on still keeps the typed cost.
The existing 14 case payload net runs unedited against the port.
* test(ui): pin the three add model behaviours the dead form instance looked like it drove
Advanced Settings used to hold a form instance it never rendered, and the ten
imperative calls against it were dead. The inherited payload net covered none
of the three behaviours those calls appeared to own, so removing them looked
riskier than it was. These cases characterise what the antd original actually
did, checked against it before the port.
Invalid LiteLLM Params blocks the submit, which also closes the one mutation
the inherited net could not kill: dropping the JSON rule left all 14 green.
The proxy resolves its own public origin from PROXY_BASE_URL before it
looks at anything on the request, so a developer who has that set for
their own deployment watched 65 OAuth discovery, redirect_uri, and client
registration cases fail against an origin no test ever asked for
An autouse fixture now clears it for every unit test, matching the host
AWS config isolation that already sits beside it, and the tests that do
exercise a configured public origin keep setting it in their own body
* refactor(ui): port the create key form off antd Form onto react-hook-form
antd hands onFinish exactly the fields mounted at submit time, so a collapsed
section contributes nothing to the request while the values typed into it
survive for re-expansion. react-hook-form reaches only one of those two
behaviours per shouldUnregister setting, so the store is kept intact and
projected down to the mounted set through an explicit mount registry.
MountedFormField carries the rest of the Form.Item contract the payload
depends on: defaults taken from each field's own declaration rather than a
blanket empty value, and help text that replaces the rule message instead of
sitting beside it.
The 60-case submit differential runs unedited against the port, joined by
cases for the writers outside the submit path, mounted-set validation, Enter
to submit, and switch coercion.
* docs(ui): state the mounted projection's static-name limit at its export
The registry counts by name and the projection emits flat keys, so a
Form.List row and its per-row sub-fields, whose names are generated at
runtime, are never in the mounted set and go missing from the payload.
That is silent and it is correct for every static field around it, so the
contract belongs where the next consumer reads it.
* refactor(ui): cut the mounted field's explanatory comments to the contract limit
The mechanism the projection uses and the reason a helped field hides its
rule message are both derivable from the code, so they belong in the pull
request rather than in two places. What survives is the one thing no reader
can derive: that a runtime-generated name is silently absent from the payload.
* refactor(ui): drop the doc comment from MountedFormField
The static-name constraint it described moves to the PR description, where
it is not a second place to keep in sync with the code.
