Skip to content

fix(critique): validate tokenBudget and costBudgetUsd in TokenBudgetBreaker (#3666) - #3682

Closed
djm204 wants to merge 3 commits into
mainfrom
fix/issue-3666-token-budget-validation
Closed

fix(critique): validate tokenBudget and costBudgetUsd in TokenBudgetBreaker (#3666)#3682
djm204 wants to merge 3 commits into
mainfrom
fix/issue-3666-token-budget-validation

Conversation

@djm204

@djm204 djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Fixes #3666.

Description

TokenBudgetBreaker compared spend.totalTokens against config.tokenBudget without validating numeric sanity. If tokenBudget was undefined, NaN, or non-positive, comparisons failed or evaluated unpredictably.

Changes

  • TokenBudgetBreaker.check() validates config.tokenBudget and throws a ConfigurationError if !Number.isFinite(tokenBudget) or tokenBudget <= 0.
  • Validates config.costBudgetUsd (if specified) to ensure it is a non-negative number (>= 0).
  • Added comprehensive unit tests covering missing/NaN/negative configuration edge cases in token-budget.test.ts.

dmendez204 and others added 3 commits July 4, 2026 10:08
Records the verified current state (ContainerBeastExecutor already
implemented per ADR-036) and the final 7-issue backlog, plus a lesson
on verifying "doesn't exist" claims via direct file reads before
filing issues.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@djm204

djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@djm204

djm204 commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded. Main already has this PR's tokenBudget validation (merged via #3896, identical logic/message). This PR's costBudgetUsd check also has the same bug that #3843 was filed against — it uses Number.isNaN(costBudgetUsd) instead of Number.isFinite(costBudgetUsd), so it still doesn't reject Infinity. #3843 is fixed properly (using Number.isFinite) in #3886, currently open and mergeable. No remaining unique content here; closing rather than asking for a rebase.

@djm204 djm204 closed this Jul 31, 2026
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.

Missing numeric validation for TokenBudgetBreaker tokenBudget config

2 participants