Skip to content

Rename CLAUDE_MAX_BUDGET_USD to BUDGET_CEILING, separate ceiling from default#305

Merged
dcellison merged 2 commits intomainfrom
fix/budget-ceiling-rename
Apr 15, 2026
Merged

Rename CLAUDE_MAX_BUDGET_USD to BUDGET_CEILING, separate ceiling from default#305
dcellison merged 2 commits intomainfrom
fix/budget-ceiling-rename

Conversation

@dcellison
Copy link
Copy Markdown
Owner

Summary

  • Rename CLAUDE_MAX_BUDGET_USD to BUDGET_CEILING across config, bot, pool, sessions, install, docs, and tests
  • Separate ceiling enforcement from per-user defaults: the ceiling always comes from the global BUDGET_CEILING env var, while max_budget in users.yaml is only a default
  • Remove the "Budget ceiling" line from /settings output
  • Add backward-compat parsing so existing .env files with the old key name still work on upgrade

Details

Previously, CLAUDE_MAX_BUDGET_USD did double duty as both the global default budget and the ceiling. Per-user max_budget in users.yaml also conflated default and ceiling. When a user changed their max_budget, the displayed ceiling changed with it, defeating the purpose.

Now the ceiling is always config.budget_ceiling (the global env var). Per-user max_budget is only a baseline default. A user with max_budget: 15 in users.yaml can /settings budget 40 if the global ceiling is $50.

Backward compat: BUDGET_CEILING > CLAUDE_MAX_BUDGET_USD > 10.0. install.py reads from either key on upgrade, writes the new key, and removes the old one.

Test plan

  • All 1686 tests pass
  • Ruff lint and format clean
  • Backward compat tests: old env var still works, new var takes precedence when both are set
  • New test: budget above yaml default but under global ceiling succeeds
  • Ceiling enforcement tests updated for global-only behavior

Fixes #304

@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

PR Review: fix/budget-ceiling-rename

Overall this is a clean, well-scoped rename with correct backward-compat logic. One notable bug in the install path.


Warning: install.py - BUDGET_CEILING not written for users with users.yaml

File: install.py, around the block at line ~702 in the diff.

env.pop("CLAUDE_MAX_BUDGET_USD", None)
if not users_yaml_exists:
    env["DEFAULT_MODEL"] = model
    env["CLAUDE_TIMEOUT_SECONDS"] = timeout
    env["BUDGET_CEILING"] = budget   # <-- inside the users_yaml guard

BUDGET_CEILING is a global ceiling, not a deprecated per-user var - but it's being written inside the same if not users_yaml_exists: block that guards legacy single-user vars. When users_yaml_exists=True, running make config will:

  1. Read budget from BUDGET_CEILING or CLAUDE_MAX_BUDGET_USD (correct)
  2. Pop CLAUDE_MAX_BUDGET_USD from the env (correct)
  3. Never write BUDGET_CEILING (bug)

Any installation that has users.yaml and runs make config silently loses its budget ceiling config. The old key is deleted; the new key is never created. The backward-compat fallback in load_config() continues to work until make config is run, at which point the protection disappears.

Fix: Move env["BUDGET_CEILING"] = budget outside the if not users_yaml_exists: guard. Only DEFAULT_MODEL, CLAUDE_TIMEOUT_SECONDS, etc. are per-user legacy vars - BUDGET_CEILING is global and should always be written.


Suggestion: Users can no longer see their ceiling before hitting it

The removal of the ceiling_line from /settings output means users are surprised when rejected ("Budget cannot exceed $X.XX") with no prior indication of X. Previously the ceiling was visible proactively. Consider restoring a ceiling line sourced from config.budget_ceiling (now that the source is always global, it's simpler to display).


Suggestion: Redundant comment repeated 4-5 times

# budget_ceiling doubles as the fallback default for unconfigured users appears in bot.py (lines ~712, ~804, ~1522), pool.py (~158), and sessions.py (~726). One canonical comment in config.py's docstring is sufficient; inline repetition adds noise without adding clarity.


Suggestion: Misleading deprecation warning message in config.py

"CLAUDE_MAX_BUDGET_USD": "Renamed to BUDGET_CEILING. Set per-user 'max_budget' in users.yaml or use /settings budget",

The second clause ("Set per-user 'max_budget' in users.yaml") conflates the ceiling with the per-user default - which is exactly the conflation this PR is fixing. The message should clarify that max_budget in users.yaml is now a default, not a ceiling replacement.


No security issues, no auth bypass vectors. The core logic (ceiling always from config.budget_ceiling, backward-compat precedence in load_config(), test coverage for both old and new var names) is correct.

@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

PR Review: fix/budget-ceiling-rename

Prior review issues

[FIXED] install.py - BUDGET_CEILING now written outside the users.yaml guard.
The block now reads:

env.pop("CLAUDE_MODEL", None)
env.pop("CLAUDE_MAX_BUDGET_USD", None)
env["BUDGET_CEILING"] = budget   # outside the guard - correct
if not users_yaml_exists:
    env["DEFAULT_MODEL"] = model
    env["CLAUDE_TIMEOUT_SECONDS"] = timeout

The critical bug is resolved.

[FIXED] Deprecation warning message in config.py is now clear: "Renamed to BUDGET_CEILING (global ceiling). Per-user defaults go in users.yaml 'max_budget'". No longer conflates ceiling with per-user default.

The two suggestions (ceiling visibility in /settings, redundant comments) were not addressed - noted, moving on.


New findings

Warning: install.py - non-interactive path writes BUDGET_CEILING= when neither key exists

install.py:464:

budget = existing_env.get("BUDGET_CEILING", existing_env.get("CLAUDE_MAX_BUDGET_USD", ""))

If an existing .env has neither key, budget = "", and then unconditionally:

env["BUDGET_CEILING"] = budget  # writes BUDGET_CEILING= (empty)

load_config() handles empty strings correctly (falls through to 10.0), so this doesn't break anything - but it writes a BUDGET_CEILING= line to the .env file that looks like it's configured when it isn't. The DEFAULT_MODEL and CLAUDE_TIMEOUT_SECONDS paths have the same pre-existing pattern, so this is not a regression. Worth a follow-up if budget: guard on the write but not a blocker.


Everything else looks correct

  • Backward-compat precedence (BUDGET_CEILING > CLAUDE_MAX_BUDGET_USD > 10.0) in load_config() is correct.
  • Ceiling enforcement in both /settings budget and /workspace config budget handlers now consistently reads config.budget_ceiling only.
  • The new test test_budget_above_yaml_default_but_under_ceiling_allowed covers the exact scenario this PR is fixing.
  • The backward-compat tests (test_old_budget_env_var_backward_compat, test_new_budget_env_var_takes_precedence) are precise and sufficient.
  • No auth bypass or security issues. Config field rename is complete across bot/pool/sessions/tests.

Ready to merge once the minor install.py edge case is decided on.

@dcellison dcellison merged commit 543d3c0 into main Apr 15, 2026
1 check passed
@dcellison dcellison deleted the fix/budget-ceiling-rename branch April 15, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate budget ceiling from per-user default

2 participants