Skip to content

fix(kernel): pre-dispatch provider budget gate prevents streaming token leak#4801

Closed
DaBlitzStein wants to merge 5 commits into
librefang:mainfrom
DaBlitzStein:fix/provider-budget-precheck
Closed

fix(kernel): pre-dispatch provider budget gate prevents streaming token leak#4801
DaBlitzStein wants to merge 5 commits into
librefang:mainfrom
DaBlitzStein:fix/provider-budget-precheck

Conversation

@DaBlitzStein

Copy link
Copy Markdown
Contributor

Summary

Closes #4800

  • Add check_provider_budget() pre-dispatch in send_message_full_with_upstream — after the existing check_quota_and_reserve gate, with proper USD + token reservation rollback on rejection
  • Add check_provider_budget() pre-dispatch in send_message_ephemeral — after the suspended-agent check

Both paths previously checked provider budgets only post-call via check_all_and_record(). Streaming responses that had already consumed tokens could not be rejected in time, causing providers to overshoot their hourly token budgets. Observed on a production instance where Moonshot's 250k hourly budget was exceeded to 317k — 16,924 tokens leaked through a single streaming call after the budget was already exhausted.

1 file changed, 27 insertions. No new types, no new crates — wires the existing MeteringEngine::check_provider_budget (which has unit test coverage) into the two dispatch entry points.

Supersedes the approach in #4757 (closed: BudgetGatedDriver was never instantiated). This PR takes the minimal path: direct calls at the dispatch sites instead of a wrapper type.

Test plan

  • cargo check --workspace --lib — zero errors
  • cargo clippy --workspace --all-targets -- -D warnings — zero warnings
  • cargo test -p librefang-kernel-metering — 28/28 passed
  • cargo test -p librefang-kernel — 831/831 passed (1 pre-existing failure unrelated: reload_config_with_invalid_toml_preserves_live_config)

@github-actions github-actions Bot added area/kernel Core kernel (scheduling, RBAC, workflows) size/S 10-49 lines changed labels May 8, 2026
…aste

Both send_message_full and send_message_ephemeral checked provider
budgets only post-call via check_all_and_record. Streaming responses
that had already consumed tokens could not be rejected in time,
causing providers to overshoot their hourly token budgets by the full
cost of the leaked call.

Wire check_provider_budget before the LLM dispatch in both paths.
On the main dispatch path, roll back the USD and token reservations
on rejection so no phantom holds accumulate in the in-memory ledger.
@DaBlitzStein
DaBlitzStein force-pushed the fix/provider-budget-precheck branch from f65d97f to 93fb234 Compare May 8, 2026 10:22
@houko

houko commented May 9, 2026

Copy link
Copy Markdown
Contributor

Closing — two issues surfaced in review that make this PR's claim ("ready, all checks passed locally") not hold:

1. Compile bug (will fail CI's Test / Unit (lib+bin) lane). Both new sites call self.metering.check_provider_budget(...), but self.metering is a MeteringSubsystem (crates/librefang-kernel/src/kernel/mod.rs:751) which has no check_provider_budget method and no Deref impl. The method only exists on MeteringEngine (crates/librefang-kernel-metering/src/lib.rs:515). The correct call is self.metering.engine.check_provider_budget(provider, pb), matching the existing self.metering.engine.check_all_and_record(…) pattern at messaging.rs:652.

2. The streaming dispatch path is not modified. The PR description's reproduction is a streaming overshoot ("16,924 tokens leaked through a single streaming call"), but neither of the two sites this PR touches sits on the streaming dispatch chain. All send_message_streaming* public entries funnel into send_message_streaming_with_sender_and_opts (messaging.rs:1661), which has its own check_quota_and_reserve and a spawned task — it never goes through send_message_full_with_upstream. So even after fixing (1), the production case in the description would still leak.

Beyond those two: putting the pre-check before the suspended branch in send_message_full_with_upstream makes us pay 4 SQL queries to skip a suspended agent; and the bc = self.budget_config() snapshot clones the whole BudgetConfig (HashMap and all) on every dispatch, when an ArcSwap::load() would do (the metering subsystem comment at subsystems/metering.rs:36-42 already calls this out as an LLM-hot-path read).

Suggestion: this is a sign the wrapper-driver layer (#4757's original direction) was probably the right one — there is exactly one chokepoint at the driver call where pre-check belongs, and it covers streaming and non-streaming uniformly. The "minimal patch" approach has to find every dispatch entry and the streaming one was missed.

Thanks for picking this up; happy to look at a v2 that either (a) covers all three entries (send_message_full_with_upstream, send_message_ephemeral, send_message_streaming_with_sender_and_opts) with the corrected self.metering.engine.… call, or (b) reinstates the BudgetGatedDriver wrapper at the driver layer so there's one place to gate.

Issue #4800 stays open.

@houko houko closed this May 9, 2026
DaBlitzStein added a commit to DaBlitzStein/librefang that referenced this pull request May 9, 2026
Cover send_message_full_with_upstream, send_message_ephemeral, and
send_message_streaming_with_sender_and_opts with check_provider_budget
before the LLM round-trip. Uses self.metering.engine (not the
subsystem wrapper) and budget_config.load() to avoid cloning. Checks
run after the suspended-agent gate to skip SQL for no-ops.

Addresses librefang#4800 review feedback on closed librefang#4801.
@DaBlitzStein
DaBlitzStein deleted the fix/provider-budget-precheck branch May 9, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kernel Core kernel (scheduling, RBAC, workflows) size/S 10-49 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(kernel): provider budget checked post-call only — streaming leaks tokens past exhausted budgets

2 participants