[bug] MCP set_budget_limits calls itself a patch but wipes the four budget fields it does not expose #501

What a user sees

Set a per-tree budget or a Loop iteration cap in the console (Usage → Budget limits), then later ask an agent over MCP to raise your daily token ceiling. The token ceiling changes — and the four settings you configured in the console silently revert to platform defaults. Nothing reports it. set_budget_limits returns the re-resolved response, so the loss is visible in the reply only if you read fields you did not ask about.

Where it is

workers/mcp/src/instance-tools/account.ts:122-151. The tool exposes two of the six stored fields and describes itself as a patch:

• :123 — "Patch your per-account AI-spend circuit breakers. Pass null for a field to inherit from the platform default."
• :133 — const input = { token_ceiling: token_ceiling ?? null, charged_micros_ceiling: charged_micros_ceiling ?? null };
• :148 — body: JSON.stringify({ tokenCeiling: input.token_ceiling, chargedMicrosCeiling: input.charged_micros_ceiling })

The four fields #477 added — perTreeCostMicros, perTreeDelegations, perTreeMaxDepth, loopMaxIterations — are never sent.

The route it calls is a full replace, not a patch. workers/api/src/routes/budget.ts:250-255:

const perTreeCostMicros  = nullOrClamp(body.perTreeCostMicros,  MAX_PER_TREE_MICROS,      "perTreeCostMicros");
const perTreeDelegations = nullOrClamp(body.perTreeDelegations, MAX_PER_TREE_DELEGATIONS, "perTreeDelegations");
const perTreeMaxDepth    = nullOrClamp(body.perTreeMaxDepth,    MAX_PER_TREE_DEPTH,       "perTreeMaxDepth");
const loopMaxIterations  = nullOrClamp(body.loopMaxIterations,  MAX_LOOP_ITERATIONS,      "loopMaxIterations");

nullOrClamp (:290-297) returns null for absent, and the upsert at :265-274 writes it: ON CONFLICT … DO UPDATE SET … per_tree_cost_micros = excluded.per_tree_cost_micros — i.e. NULL. resolveAccountCeilings (lib/delegation-budget-store.ts:158-165) then falls through to platform → env → constant, so the override is gone.

Worse case: if the account row held only per-tree overrides and the MCP call passes token_ceiling: null, charged_micros_ceiling: null, the allNull branch at budget.ts:259-263 DELETEs the row.

Mechanism

Two individually-defensible decisions compose into data loss. The route was written as a full-document PUT with "absent = inherit" semantics, which is coherent for the console (it renders all six controls and submits all six). The MCP tool was written as a partial patch, which is coherent for a model that wants to change one number. Neither is wrong alone; together the narrower client silently clears what it does not know about.

dry_run does not help — :137-141 previews exactly the two-field body, so it faithfully previews a call whose destructive part is the fields it omits.

What to do — cheapest first

1. One-line-ish fix, no schema change. Read the current row first and re-send it merged, or add the four fields to the tool's schema and forward them. Reading-then-merging is the smaller change and also fixes any future field the tool has not caught up with.
2. The structural fix. Give the route real patch semantics: distinguish "absent" from "explicit null" ("perTreeCostMicros" in body) so null means inherit and absent means leave alone. Then the tool description at account.ts:123 becomes true and the console keeps working (it sends all six either way). This is the version I would ship if only one lands, because it fixes every future partial client, but it changes documented behaviour — see risk.
3. Expose the four missing fields on set_budget_limits regardless, for [epic] AI budget & circuit-breaker limits are user-configurable via UI + MCP (today they are hardcoded constants) #478 parity.

Rejected: making the tool refuse when an account row exists (safe, but makes the tool useless in exactly the case it is for). Rejected: having MCP call a new dedicated endpoint (a second write path to the same table is how the two drift again).

Acceptance criteria

• set_budget_limits{token_ceiling: X} against a row holding loop_max_iterations = 200 leaves it at 200; GET /v1/budget/limits still resolves loopMaxIterationsTier: "account".
• set_budget_limits{token_ceiling: null} against that row clears only the token ceiling; the row is not deleted.
• A test asserts the console's six-field PUT still fully replaces (or, under fix 2, that an explicit null clears).
• The tool's schema covers every column in account_budget_limits, or a test fails when a new column is added without it.

Regression risk

Fix 2 changes PUT semantics. The console (store/console/src/pages/Usage.tsx:626) submits the full set, so it is unaffected; workers/api/src/routes/budget.test.ts covers the "all null deletes the row" behaviour and would need its intent restated as "all explicit nulls". store/openapi.yaml documents the current semantics and must move with it. Anything else calling PUT /v1/budget/limits with a partial body today is relying on the clearing behaviour — grep says nothing does.

Context: found while auditing #485/#478. Related to #477 (which added the four fields to REST + console but not to MCP).