Reborn cost-based budgets: foundation (resources, accountant, gate, period) - #3841
Conversation
…eriod) Implements the design from #2843 in crates/ironclaw_resources, crates/ironclaw_loop_support, crates/ironclaw_reborn_config, and crates/ironclaw_agent_loop. Establishes the USD-denominated reservation contract that cascades across the tenant/user/project/agent/mission/thread hierarchy, plus first-touch seeding, calendar+rolling period rollover, graduated intervention (warn -> approval gate -> hard deny), audit-sink primitives, and progress-detection primitives. Zero = unlimited end-to-end. No separate enable flag — every deployment runs the same code path with operator-tunable limits. Phases shipped: - 0: schema (BudgetPeriod, BudgetThresholds, RequiresApproval, snapshot v2) - 1: GovernorBackedAccountant wired into ThreadBackedLoopModelPort - 2: BudgetDefaults config layering (TOML + env, 0 = unlimited preserved) - 3: BudgetApprovalGate primitive + InMemoryBudgetGateStore - 4: BackgroundKind enum on ResourceScope (contract-only) - 5: ParamHash + JSON normalization for loop-stuck detection - 6: BudgetEventSink contract (NoOp + InMemory) - 7: HARD_CAP_* invariants in ironclaw_common ~1600 lines added, ~120 lines modified across 21 files + 6 new modules. 80+ new contract tests across resources/loop_support/reborn_config/agent_loop. Production wiring of GovernorBackedAccountant into the Reborn host factory is intentionally deferred to a follow-up on this branch; the surface is in place via RebornLoopDriverHostFactory::with_model_budget_accountant. Refs: #2843 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
scope: dependencies
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a cost-based budgeting system and a progress-detection strategy for the Reborn loop. Key additions include a GovernorBackedAccountant for model call reservations, an updated ResourceGovernor supporting period-based limits (Rolling24h and Calendar), and an approval gate mechanism for threshold-crossing events. Hard budget caps are also introduced, and the snapshot schema is updated to version 2. Review feedback focuses on optimizing string normalization and character iteration to avoid unnecessary heap allocations, fixing incorrect period boundary reporting in Rolling24h account snapshots, and correcting threshold intervention logic to properly handle 100% utilization and disabled states.
| fn is_correlation_key(key: &str) -> bool { | ||
| let normalized: String = key | ||
| .chars() | ||
| .filter(|c| c.is_ascii_alphanumeric() || *c == '_') | ||
| .map(|c| c.to_ascii_lowercase()) | ||
| .collect(); | ||
| matches!( | ||
| normalized.as_str(), | ||
| "request_id" | ||
| | "requestid" | ||
| | "trace_id" | ||
| | "traceid" | ||
| | "correlation_id" | ||
| | "correlationid" | ||
| | "idempotency_key" | ||
| | "idempotencykey" | ||
| | "x_request_id" | ||
| ) | ||
| } |
There was a problem hiding this comment.
The normalization logic in is_correlation_key is inconsistent and inefficient. It collects into a String on every call. Per project convention, use to_ascii_lowercase() for case-insensitive ASCII comparisons. Additionally, consider performing cheaper validation checks on a trimmed slice before performing allocations for normalization.
fn is_correlation_key(key: &str) -> bool {
let normalized: String = key
.chars()
.filter(|c| c.is_ascii_alphanumeric())
.map(|c| c.to_ascii_lowercase())
.collect();
matches!(
normalized.as_str(),
"requestid"
| "traceid"
| "correlationid"
| "idempotencykey"
| "xrequestid"
)
}References
- For case-insensitive comparisons, use to_ascii_lowercase() as it is the project convention.
- Perform cheaper validation checks on a trimmed slice before performing more expensive operations like allocations.
There was a problem hiding this comment.
Fixed in ccc1b66. is_correlation_key now uses str::eq_ignore_ascii_case against a static slice of known variants — zero heap allocation per hash, and the matched values are still listed explicitly so the call site is obvious.
| fn replace_uuids(s: &str) -> String { | ||
| // Strict UUID v1-v5: 8-4-4-4-12 hex segments. Case-insensitive. | ||
| let mut out = String::with_capacity(s.len()); | ||
| let chars: Vec<char> = s.chars().collect(); |
There was a problem hiding this comment.
Collecting characters into a Vec<char> for potentially large strings is inefficient as it performs a heap allocation proportional to the string length. To improve performance, especially in WASM, avoid unnecessary heap allocations and use iterators directly instead of collecting into a Vec.
References
- To improve performance in WASM, avoid unnecessary heap allocations by using iterators directly instead of collecting into a Vec.
There was a problem hiding this comment.
Fixed in ccc1b66. Replaced both replace_uuids and replace_timestamps with a shared replace_ascii_pattern helper that iterates &[u8] (UUIDs and ISO-8601 are ASCII by construction) and copies multibyte codepoints via str::is_char_boundary on the non-match path — no more Vec<char> allocation per hash.
| .as_ref() | ||
| .map(|l| l.period.clone()) | ||
| .unwrap_or_default(); | ||
| let (period_start, period_end) = period_bounds(&period, now); |
There was a problem hiding this comment.
The account_snapshot reporting logic for Rolling24h periods is incorrect. It uses period_bounds(now), which returns a window starting now, whereas the actual ledger window is anchored to the time the limit was set or last rolled over. This will cause the UI to display incorrect period boundaries for accumulated usage. The snapshot should derive the start time from the stored period_anchors.
let (period_start, period_end) = match state.period_anchors.get(account) {
Some(end) if matches!(period, BudgetPeriod::Rolling24h) => (*end - chrono::Duration::hours(24), *end),
_ => period_bounds(&period, now),
};There was a problem hiding this comment.
Fixed in ccc1b66. account_snapshot_in_state now derives the window from state.period_anchors for Rolling24h (period_start = anchor - 24h) and falls back to period_bounds(now) for Calendar/PerInvocation. Regression test rolling_24h_snapshot_reports_anchored_window_not_now_window advances a FakeClock and asserts the reported window doesn't slide.
| if utilization >= thresholds.pause_at && utilization < 1.0 { | ||
| return Some(ThresholdIntervention::Approval(ResourceApprovalNeeded { | ||
| account: account.clone(), | ||
| dimension, | ||
| limit: ResourceValue::Decimal(limit), | ||
| current_usage: ResourceValue::Decimal(usage), | ||
| active_reserved: ResourceValue::Decimal(reserved), | ||
| requested: ResourceValue::Decimal(requested), | ||
| utilization, | ||
| period_end, | ||
| })); | ||
| } | ||
| if utilization >= thresholds.warn_at && utilization < thresholds.pause_at { | ||
| return Some(ThresholdIntervention::Warning(BudgetWarning { | ||
| account: account.clone(), | ||
| dimension, | ||
| utilization, | ||
| limit: ResourceValue::Decimal(limit), | ||
| period_end, | ||
| })); | ||
| } |
There was a problem hiding this comment.
The threshold intervention logic has two issues: 1) it fails to trigger an intervention when utilization is exactly 1.0 (100%), and 2) the utilization < 1.0 check is an incorrect way to implement the '1.0 disables' rule. Additionally, ensure that any accumulated metrics up to this point are propagated and reported before the operation pauses for user approval.
if thresholds.pause_at < 1.0 && utilization >= thresholds.pause_at {
return Some(ThresholdIntervention::Approval(ResourceApprovalNeeded {
account: account.clone(),
dimension,
limit: ResourceValue::Decimal(limit),
current_usage: ResourceValue::Decimal(usage),
active_reserved: ResourceValue::Decimal(reserved),
requested: ResourceValue::Decimal(requested),
utilization,
period_end,
}));
}
if thresholds.warn_at < 1.0 && utilization >= thresholds.warn_at {
return Some(ThresholdIntervention::Warning(BudgetWarning {
account: account.clone(),
dimension,
utilization,
limit: ResourceValue::Decimal(limit),
period_end,
}));
}References
- When an operation is paused for user interaction (e.g., approval), ensure that any accumulated metrics up to that point are propagated and reported before waiting.
There was a problem hiding this comment.
Partially fixed in ccc1b66. threshold_decimal now uses thresholds.pause_at < 1.0 && utilization >= thresholds.pause_at (and the same shape for warn_at), so:
- 100% utilization with
pause_at < 1.0correctly returns Approval pause_at == 1.0cleanly disables approval (the "1.0 disables" rule is now expressed on the threshold itself, not on utilization)
Also dropped the #[allow(clippy::too_many_arguments)] by moving the inputs into a ThresholdInputs<'_, Decimal> struct.
Not addressing the "report accumulated metrics before pausing for user approval" suggestion in this PR — the cascade currently returns the first intervention; emitting partial warnings before an approval requires re-shaping CascadeOutcome. Filing as follow-up.
| if utilization >= thresholds.pause_at && utilization < 1.0 { | ||
| return Some(ThresholdIntervention::Approval(ResourceApprovalNeeded { | ||
| account: account.clone(), | ||
| dimension, | ||
| limit: ResourceValue::Integer(limit), | ||
| current_usage: ResourceValue::Integer(usage), | ||
| active_reserved: ResourceValue::Integer(reserved), | ||
| requested: ResourceValue::Integer(requested), | ||
| utilization, | ||
| period_end, | ||
| })); | ||
| } | ||
| if utilization >= thresholds.warn_at && utilization < thresholds.pause_at { | ||
| return Some(ThresholdIntervention::Warning(BudgetWarning { | ||
| account: account.clone(), | ||
| dimension, | ||
| utilization, | ||
| limit: ResourceValue::Integer(limit), | ||
| period_end, | ||
| })); | ||
| } |
There was a problem hiding this comment.
Similar to the decimal threshold check, the integer threshold logic misses the 100% utilization case and incorrectly uses current utilization to determine if a threshold is disabled. Ensure that accumulated metrics are reported before the pause for user interaction.
if thresholds.pause_at < 1.0 && utilization >= thresholds.pause_at {
return Some(ThresholdIntervention::Approval(ResourceApprovalNeeded {
account: account.clone(),
dimension,
limit: ResourceValue::Integer(limit),
current_usage: ResourceValue::Integer(usage),
active_reserved: ResourceValue::Integer(reserved),
requested: ResourceValue::Integer(requested),
utilization,
period_end,
}));
}
if thresholds.warn_at < 1.0 && utilization >= thresholds.warn_at {
return Some(ThresholdIntervention::Warning(BudgetWarning {
account: account.clone(),
dimension,
utilization,
limit: ResourceValue::Integer(limit),
period_end,
}));
}References
- When an operation is paused for user interaction (e.g., approval), ensure that any accumulated metrics up to that point are propagated and reported before waiting.
There was a problem hiding this comment.
Fixed in ccc1b66. threshold_integer got the same pause_at < 1.0 && utilization >= pause_at rule and the ThresholdInputs<'_, u64> refactor — see the parallel fix to threshold_decimal. Regression test threshold_pause_fires_at_exactly_100_percent_when_pause_below_one covers the 100% case; pause_threshold_of_one_disables_approval_and_allows_under_hard_cap covers the disable case.
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Multi-agent review completed with 5 reviewer lanes: Security, Bugs, Performance/Concurrency, Tests, and Conventions.
Outcome: requesting changes.
Primary blockers:
- High: successful paid model calls reconcile zero USD, so daily USD budgets never deplete.
- High: budget reservations can leak before provider dispatch when prompt message resolution fails.
- High: post-call accounting failures are swallowed on the provider-error path.
Additional findings are inline. Test gaps worth covering with the fixes: caller-level ThreadBackedLoopModelPort budget-accountant integration, budget env override precedence/error handling, and TOML [budget] parser validation.
| // the ledger records *something* for accounting. | ||
| let chunks = response.chunks.len() as u64; | ||
| ResourceUsage { | ||
| usd: Decimal::ZERO, |
There was a problem hiding this comment.
[High][Security/Correctness] Successful calls reconcile the reservation with usd: Decimal::ZERO, so reconcile removes the estimated hold and records no USD spend. That means a user/project daily USD budget only limits concurrent in-flight calls; unlimited sequential paid calls can pass as long as each individual estimate fits. Thread provider token/cost usage into the response, or fail safe by reconciling the conservative reservation estimate when exact usage is unavailable.
There was a problem hiding this comment.
Fixed in ccc1b66. post_model_call now stores the reservation estimate alongside the id and reconciles the conservative estimate as actual USD on success — daily caps actually deplete. Threading real provider numbers is the long-term fix; this is the fail-safe path you suggested.
| return Err(budget_error.into_host_error()); | ||
| } | ||
|
|
||
| let resolved_messages = self.resolve_model_messages(prompt_grant.messages).await?; |
There was a problem hiding this comment.
[High][Performance/Correctness] The budget reservation happens before resolve_model_messages(...).await?. If message resolution, thread reads, or reference validation fails here, the function returns before post_model_call can release the reservation, leaving a stuck active hold for work that never reached the provider. Move reservation after fallible pre-dispatch resolution or add a guard that releases on every early-return path after pre_model_call.
There was a problem hiding this comment.
Fixed in ccc1b66. Moved the pre_model_call reservation block after resolve_model_messages so a resolution failure short-circuits before the hold is taken.
| outcome: ModelCallOutcome<'_>, | ||
| host_response_result: Result<LoopModelResponse, AgentLoopHostError>, | ||
| ) -> Result<LoopModelResponse, AgentLoopHostError> { | ||
| if let Err(acc_error) = accountant |
There was a problem hiding this comment.
[High][Correctness] On the provider-error path, a failed post_model_call is only logged and the original model error is returned. The LoopModelBudgetAccountant contract says durable accounting/release failures must fail closed; swallowing this hides stuck reservations and reports the wrong failure cause. Return acc_error.into_host_error() here, matching the success-path accounting behavior.
There was a problem hiding this comment.
Fixed in ccc1b66. finalize_post_model_call now returns acc_error.into_host_error() (matching success-path semantics) and emits a model_failed milestone before propagating.
| _request: &LoopModelRequest, | ||
| outcome: ModelCallOutcome<'_>, | ||
| ) -> Result<(), LoopModelGatewayError> { | ||
| let Some((_, reservation_id)) = self.in_flight.remove(&context.run_id) else { |
There was a problem hiding this comment.
[Medium][Concurrency] This removes the reservation id from in_flight before reconcile/release succeeds. If the governor returns a transient storage/accounting error, the reservation can remain active in the governor while the accountant has lost the id needed to retry or clean it up. Keep the id until the governor operation succeeds, then remove it.
There was a problem hiding this comment.
Fixed in ccc1b66. post_model_call now peeks the entry, runs reconcile/release, and only calls in_flight.remove after the governor confirms success. On failure the id stays in-flight (with a warning log naming the reservation id) so a future retry/cleanup hook has the data it needs.
| { | ||
| Ok(outcome) => { | ||
| self.in_flight | ||
| .insert(context.run_id, outcome.reservation.id); |
There was a problem hiding this comment.
[Medium][Concurrency] The in-flight map is keyed only by TurnRunId, so two overlapping model calls for the same run overwrite each other's reservation id. One hold can then be leaked, or a post-call can reconcile/release the wrong reservation. Track a per-call token through pre/post accounting, or reject a second active reservation for the same run and release the new hold immediately.
There was a problem hiding this comment.
Fixed in ccc1b66. pre_model_call now rejects a second concurrent reservation for the same run: the new hold is released immediately and the caller gets BudgetAccountingFailed. The check is guarded by a DashMap::entry race-window guard so two parallel tasks can't both insert. A regression test (pre_model_call_rejects_overlapping_reservation_for_same_run) covers it.
| // configuration. | ||
| } | ||
| _ => { | ||
| if let Err(err) = self.governor.set_limit(account.clone(), limits.clone()) { |
There was a problem hiding this comment.
[Medium][Conventions/Error Handling] account_snapshot errors fall through to set_limit, and set_limit errors are logged while the account has already been inserted into seeded. The repo error-handling rule forbids warn-and-continue after storage/read failures when it poisons cached state; this can let later calls proceed without the intended default budget and without retrying seeding. Return a Result from seeding and fail pre_model_call, or only mark seeded after a successful read/write with an explicit fallback rationale.
There was a problem hiding this comment.
Fixed in ccc1b66. install_if_unseeded now only inserts into seeded after a successful read showing an existing limit, or a successful set_limit. On snapshot or write failure the account is left unseeded so the next pre_model_call retries — no more poisoned cache state. New test seeding_retry_after_transient_failure_uses_failing_governor exercises the retry path.
| } | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] |
There was a problem hiding this comment.
[Medium][Conventions] This adds #[allow(clippy::too_many_arguments)] without the required arch-exempt annotation. .claude/rules/architecture.md requires the line above each new allow to name the missing aggregation and link the cleanup plan. Prefer a small threshold-input struct, or add the required exemption comment for both threshold helpers.
There was a problem hiding this comment.
Fixed in ccc1b66. Both threshold_decimal and threshold_integer now take a ThresholdInputs<'_, T> struct, dropping the #[allow(clippy::too_many_arguments)] annotations entirely — no arch-exempt needed because there is no longer an exemption to declare.
| } | ||
|
|
||
| /// Apply env-var overrides over the current set. | ||
| pub fn with_env(mut self) -> Result<Self, BudgetDefaultsError> { |
There was a problem hiding this comment.
[Medium][Tests] with_env is a new public env-input path, including malformed IRONCLAW_BUDGET_* parse errors and env-over-section precedence, but the added tests only cover compiled defaults, section overrides, and validation. Add a targeted test such as tests::budget::env_layer_overrides_section_and_rejects_invalid_f64.
There was a problem hiding this comment.
Fixed in ccc1b66. New test env_layer_overrides_section_and_rejects_invalid_f64 covers both: env overriding a section value, and a non-numeric env var surfacing as InvalidEnvF64. Env mutation is serialized through a process-wide Mutex so concurrent tests can't race.
| } | ||
| } | ||
| } | ||
| if let Some(budget) = &self.budget { |
There was a problem hiding this comment.
[Medium][Tests] The user-facing [budget] TOML section adds parser validation for negative USD values, threshold ranges, and pause_at < warn_at, but there is no parser-level test for this section. Add a RebornConfigFile::parse_text test covering a valid budget section plus those invalid values; BudgetDefaults unit tests do not exercise TOML deserialization or deny_unknown_fields.
There was a problem hiding this comment.
Fixed in ccc1b66. Added parses_valid_budget_section, rejects_negative_budget_usd_field, rejects_budget_threshold_out_of_range, rejects_budget_pause_below_warn, and rejects_unknown_budget_section_key parser-level tests against RebornConfigFile::parse_text.
Resolves multi-agent review feedback on the cost-based budget foundation: - Daily USD budgets now actually deplete on successful calls — reconcile records the conservative reservation estimate as actual spend until provider token usage threads through the loop layer. (review: High) - Budget reservation taken AFTER message resolution so a resolution failure cannot orphan an active hold. (review: High) - post_model_call failures on the provider-error path now propagate as host errors instead of being logged-and-swallowed. (review: High) - in_flight entry held until governor reconcile/release succeeds, so a transient storage error leaves the id available for retry/cleanup. - Per-run overlap protection: a second concurrent pre_model_call for the same run releases the new hold and surfaces an accounting error. - Seeding no longer poisons the `seeded` cache when set_limit fails; the account is marked seeded only after a successful read or write. - Threshold logic correctly fires Approval at exactly 100% utilization and uses `pause_at < 1.0` (not utilization-based) as the disable rule. - threshold_decimal/integer use a ThresholdInputs struct instead of silencing clippy::too_many_arguments. - account_snapshot reports the Rolling24h window anchored to set_limit time, not to `now` — UI now sees the ledger's actual window. - progress::is_correlation_key uses eq_ignore_ascii_case against a static slice — zero heap allocation per hash. - progress::replace_uuids / replace_timestamps iterate &[u8] for the ASCII patterns, copying multibyte chars via is_char_boundary — drops the Vec<char> allocation on every hash. - New tests cover: Rolling24h snapshot anchoring, 100% threshold, pause-at-1.0 disable, estimate-as-actual reconcile, overlap rejection, seeding retry after failure, env layer over section in BudgetDefaults, [budget] TOML parser validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…born-cost-budgets # Conflicts: # Cargo.lock # crates/ironclaw_loop_support/Cargo.toml # crates/ironclaw_loop_support/src/lib.rs
Follow-up items surfaced during review responseThese came out of the review pass + a re-review after merging the latest 1. Provider token usage threading (the proper fix for the "USD never depletes" bug)The current commit reconciles the conservative reservation estimate as recorded actual USD ( The right long-term fix is to thread real 2. Cancellation safety — reservation orphans on tokio cancelIf the
Fix shape: a 3. Filesystem-backed
|
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
* Reborn budgets: address all #3841 follow-ups end-to-end Implements every open follow-up from PR #3841 (cost-based budgets foundation), driven by the plan in `docs/plans/2026-05-22-reborn-budgets-followups.md`: - **C2 (provider tokens)**: `LoopModelResponse.usage` carries real `(input_tokens, output_tokens)` from `CompletionResponse` / `ToolCompletionResponse`; `usage_for_response` reconciles to actual USD via the cost table instead of the conservative estimate. - **D1 (cascade warnings)**: `CascadeOutcome` variants carry `Vec<BudgetWarning>` so warnings preceding a pause or hard deny reach the audit sink. `ResourceError::LimitExceeded` / `RequiresApproval` reshaped to struct variants. - **C1 (cancellation safety)**: new `LoopModelBudgetAccountant::release_in_flight` trait hook + RAII `ReservationReleaseGuard` in `HostManagedLoopModelPort::stream_model` so a cancelled future doesn't orphan its reservation. - **E1 (dead code)**: removed the never-set `budget_accountant` field on `ThreadBackedLoopModelPort`. - **Real cost table**: new `StaticModelCostTable` + `LlmModelProfilePolicy::build_cost_table()` populated from `ironclaw_llm::costs::model_cost` with `default_cost` fallback so unknown providers never silently reconcile to zero. - **B1 (filesystem gate store)**: new `FilesystemBudgetGateStore` mirroring `FilesystemResourceGovernorStore`; pending gates survive process restart. - **A1 (production wiring)**: composition builds `GovernorBackedAccountant` from the cost table + governor and threads it through `RebornLoopDriverHostFactory::with_model_budget_accountant`. - **A2 (audit / SSE projection)**: `InMemoryResourceGovernor::with_event_sink` emits `Reserved`, `Reconciled`, `Released`, `Warned`, `ApprovalRequested`, `Denied`, `LimitChanged`; composition holds an `InMemoryBudgetEventSink` ready for downstream SSE projection. - **F1 (stuck-loop normalization)**: `CapabilityCallSignature::from_call` now runs `progress::normalize_for_hash` so the existing repetition window collapses request-id / UUID / timestamp noise. Side fix: `ResourceValue` moved to adjacent serde tagging (the combination of internal tagging + `Decimal`'s `serde-with-str` representation breaks JSON serialization — rust-lang/serde#1402). Regression tests added per item — see the acceptance evidence appendix in the plan doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Reborn budgets: end-to-end test coverage via test-support feature Adds 13 e2e tests covering the budget pipeline through `build_reborn_runtime` + `send_user_message`. Required infrastructure: - **`test-support` feature** on `ironclaw_reborn_composition` exposing `BudgetTestGateway` (scripted token usage) and `RebornRuntimeInputTestExt`. Existing `model_gateway_override` field promoted from `#[cfg(test)]` to `#[cfg(any(test, feature = ...))]` with a new public `with_model_gateway_override_for_tests` setter. - **Cost-table override** on `RebornRuntimeInput` so tests can pair the gateway with a deterministic `ModelCostTable`. Without this, an override gateway dropped the cost table and the accountant never fired. - **Budget accessors** on `RebornRuntime`: `budget_resource_governor`, `budget_event_sink`, `budget_gate_store`, and `apply_resolved_budget_gate`. Test-feature gated. - **`ResourceGovernor::usage_for`** added as a default-impl trait method so tests read spend through the trait surface. - **`BudgetGateStore` wired into the accountant**: `GovernorBackedAccountant::with_gate_store(...)` opens a pending gate whenever the governor cascade returns `RequiresApproval`. The approval-required host error is unchanged; the gate is the out-of-band channel a user-facing handler resolves. Scenarios covered: | # | Test | What it asserts | |---|---|---| | F1 | `f1_happy_path_records_actual_usd_in_ledger` | Ledger depletes by provider tokens × cost table | | F2 | `f2_crossing_warn_threshold_emits_warned_event` | Warn fires alongside successful Reserved/Reconciled | | F3 | `f3_approval_with_increased_limit_unblocks_retry` | Approve → set_limit applies → retry succeeds | | F4 | `f4_cancel_keeps_budget_blocked_on_retry` | Cancel → retry still short-circuits | | F5 | `f5_expiry_marks_gate_terminal_and_keeps_budget_blocked` | Expiry → gate drops from pending list, retry still blocked | | F6 | `f6_hard_cap_denied_before_provider_call` | Estimate over cap → zero model calls, Denied event | | C1 | `c1_provider_tokens_reconcile_to_actual_usd` | Real numbers, not estimate | | C2 | `c2_unknown_model_in_cost_table_reconciles_to_zero` | Unknown profile → zero spend | | C3 | `c3_zero_cost_model_records_zero_spend` | Free model → zero USD with non-zero tokens | | D1 | `d1_agent_deny_preserves_user_warn_event` | Cascade emits both Warned and Denied | | D3 | `d3_fresh_user_without_limits_runs_without_denial` | No limit → no denial | | + | `pause_in_distinct_runs_produces_distinct_pending_gates` | Per-run gate identity | | + | `budget_test_gateway_scripted_replies_drive_per_turn_costs` | Multi-turn scripted accumulation | F7 (cancellation mid-stream) is unit-covered by `release_in_flight_drains_orphan_reservation_on_cancellation`. D2 (period rollover) is unit-covered by `rolling_24h_snapshot_reports_anchored_window_not_now_window`. B-series (background ticks) await the BackgroundKind scheduler call site (no production caller in Reborn yet). Run via `cargo test -p ironclaw_reborn_composition --features test-support`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Budget review feedback: address all 7 findings from PR #3899 review Two High and five Medium issues raised by serrrfirat's multi-agent review. **High #1 — `FilesystemBudgetGateStore` cross-tenant leakage** The store hardcoded `ResourceScope::system()` for every op, so all tenants wrote into the same `/tenants/__SYSTEM__/...` snapshot and `list_pending` would expose gates across tenants. Fix: `new(...)` now takes a `ResourceScope`; each tenant gets its own store, and the `ScopedFilesystem` mount view routes the snapshot under that tenant's path. Added `list_pending_does_not_leak_across_tenants` regression. **High #2 — accountant wired without default budget limits** Composition built `GovernorBackedAccountant` without `with_seeding_policy`, so the local-dev governor started empty and `reserve_with_outcome_in_state` skipped accounts that had no configured limit — model calls reconciled spend but never enforced a cap. Fix: `build_reborn_runtime` now loads `BudgetDefaults::compiled_defaults().with_env()` and wires `BudgetSeedingPolicy` + `with_overestimate_factor`. Renamed the D3 test to `d3_seeding_policy_installs_default_cap_on_first_touch` to prove the wiring fires. **Medium #3 — RAII guard disarmed before post_model_call await** `HostManagedLoopModelPort::stream_model` was disarming the `ReservationReleaseGuard` before awaiting `post_model_call`. A cancellation during that await dropped the future without cleanup, orphaning the reservation. Fix: disarm AFTER `post_model_call` returns. `release_in_flight` is now idempotent (peek-then-release- then-remove) so a successful post-call + subsequent guard drop is a no-op. **Medium #4 — failed release drops the retry handle** `release_in_flight` removed the in-flight entry before calling `governor.release`. A transient storage error left the reservation active in the governor with the id discarded. Fix: peek first, release, only remove on success. Errors keep the entry retained for a future retry / cleanup hook. **Medium #5 — unknown model silently reconciles to zero USD** Both `estimate_for` and `usage_for_response` fell back to `ModelCost { 0, 0, 0 }` when the cost table had no entry for the effective model. Cost-table drift would silently bypass daily caps. Fix: `GovernorBackedAccountant` carries a `default_cost` (default ~ GPT-4o pricing, ~`$0.0000025 input + $0.00001 output per token`) used for unknown models. Callers wiring `ZeroCostTable` for free / Ollama explicitly opt out of the fallback. Updated the C2 e2e test to assert the new fail-closed shape. **Medium #6 — paused dimension lost when another hard-denies** `check_thresholds_all_interventions` stored `Approval` only in the `approval` slot, so when one dimension paused and another hard-denied, the `Deny { warnings, denial }` outcome lost the pause signal. Fix: also push a warning-shaped record for the paused dimension. **Medium #7 — unbounded terminal-gate retention** The snapshot kept every gate forever; `open` / `resolve` / `get` / `list_pending` were O(total historical gates). Fix: `with_terminal_retention` (default 30 days). Every mutation prunes terminal gates whose resolution timestamp is older than the window. Added `terminal_gates_older_than_retention_are_pruned_on_next_write` regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: replace lock-poisoned expects with PoisonError::into_inner scripts/check_no_panics.py flagged five .expect("...lock poisoned") calls in the new test_support.rs. Use the same idiomatic recovery pattern the rest of the codebase uses (see InMemoryBudgetGateStore, InMemoryBudgetEventSink): on a poisoned lock, recover the inner data via PoisonError::into_inner rather than panicking. The test gateway's state is append-only logs / replies queues, so reading them through a poisoned lock is safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Finish A1 / A2 / F1 from plan + honest plan doc update The plan claimed "all nine items landed" but A1 (production wiring), A2 (SSE projection), and F1 (full progress strategy) were partials. This commit finishes the work so the plan matches reality. **A1 — production-shape accountant builder** New `ironclaw_reborn_composition::build_default_budget_accountant` public helper that wires the seeding policy + overestimate factor + gate store from `BudgetDefaults::compiled_defaults().with_env()` and returns an `Arc<dyn LoopModelBudgetAccountant>`. Production loop composers call this with their `PersistentResourceGovernor` + `FilesystemBudgetGateStore` + LLM-policy-derived cost table; the local-dev runtime in `build_reborn_runtime` now uses the same helper instead of duplicating the seeding logic inline. Unit-tier regression `seeds_compiled_default_user_cap_on_first_touch` proves the helper installs the compiled-default $5 user cap on first model call. **A2 — broadcast sink + AppEvent projection** - `ironclaw_resources::BroadcastBudgetEventSink` wraps `tokio::sync::broadcast::Sender<BudgetEvent>` with `subscribe()` / `subscriber_count()`. `CompositeBudgetEventSink` fans events to multiple sinks. - Composition fans every `BudgetEvent` to the in-memory sink (for tests) AND the broadcast sink (for SSE projection) via `CompositeBudgetEventSink`. - New `AppEvent::BudgetWarn` / `BudgetPause` / `BudgetDenied` / `BudgetLimitChanged` wire-stable variants in `ironclaw_common::event`. - `src/bridge/budget_events.rs` carries the projection: a tokio task spawned by `spawn_budget_event_projection` drains the broadcast receiver and emits the appropriate `AppEvent` via `SseManager::broadcast_for_user`. System-scoped events (no user identity) are skipped. This is the only producer of these `AppEvent` variants per `.claude/rules/gateway-events.md`. - `RebornRuntime::broadcast_budget_event_sink()` exposes the sink to the binary so the startup path subscribes. E2E test `broadcast_sink_publishes_events_to_subscribers` drives a real `send_user_message` and asserts Reserved + Reconciled lands on the broadcast. **F1 — diminishing-returns stop condition** The earlier shipped `ParamHash` normalization in `CapabilityCallSignature` strengthened the existing `recent_call_signatures`-based repetition detector. This commit adds the second half of F1: a rolling output-token window that detects "wedged" loops the repetition detector misses (model keeps responding but produces no useful output). - `LoopExecutionState.recent_output_token_counts: BoundedRing<u32, 8>` populated by the executor from `LoopModelResponse::usage`. - `BoundedRing::iter` returns `impl DoubleEndedIterator` so the strategy can scan the trailing window. - `DefaultStopConditionStrategy` gets `min_delta_tokens` (default 4) + `noprogress_window` (default 4). When the last N turns all produce ≤ min_delta_tokens of output, fire `StopKind::NoProgressDetected`. - Regression tests: `four_consecutive_low_token_turns_trigger_no_progress` proves the detector fires; `occasional_low_token_turn_does_not_trip_no_progress` proves a productive turn resets the trailing count. **Plan doc** Updated the status header from "all nine items landed" to the honest per-item shape. Acceptance evidence table expanded with the new test names. New "Review-feedback fixes layered on top" subsection documenting all 2 High + 5 Medium findings addressed during review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address thermo-nuclear review: collapse filesystem-store duplication, flatten cfg permutations, split budget accountant Five structural simplifications surfaced by the deep audit on PR #3899, plus two bug fixes from the earlier review pass: - ironclaw_resources: extract `cas_snapshot` shared infrastructure (`StorageError` + `Snapshot` traits + `CasSnapshotStore<F>` + async-runtime worker + per-path lock map) and merge `filesystem_gate_store.rs` into `filesystem_store.rs`. Deletes ~350 lines of duplicated read-modify-write + worker-thread + CAS machinery; both stores are now thin shims over the shared helper. - ironclaw_reborn_composition: flatten the 4-way cfg permutation in `build_reborn_runtime` model-gateway resolution into three flat steps (normalize override → build production gateway via cfg-gated helper → test override wins). Also drops the `unused_mut` warning. - ironclaw_reborn_composition: collapse the 3-layer test-only setter dance for `model_gateway_override` / `model_cost_table_override` into a single setter pair gated on `cfg(any(test, feature = "test-support"))`. Deletes the `RebornRuntimeInputTestExt` extension trait — integration tests now call the inherent methods directly. - ironclaw_loop_support: split the 1305-line `budget_accountant.rs` into `budget_cost_table.rs` (ModelCost/ModelCostTable trait/ZeroCostTable/ StaticModelCostTable), `budget_seeding.rs` (BudgetSeedingPolicy), and `budget_accountant.rs` (just GovernorBackedAccountant). Each module now owns one concern. - ironclaw_resources: add `impl Display for ResourceAccount` and route the hierarchical account-label rendering through it; delete the 60-line bespoke `account_label` helper from `src/bridge/budget_events.rs`. - ironclaw_common + bridge: collapse the four `AppEvent::Budget*` variants into a single typed `AppEvent::Budget(AppBudgetEvent)` with the four shapes carried inside the enum. Wire-shape stays identical (snake_case serde tag). - ironclaw_resources + ironclaw_loop_support: thread real gate id through `BudgetEvent::GateOpened { gate_id, needed, at }` (new variant) and have the accountant emit it via the broadcast event sink after store.open succeeds. The bridge now projects `BudgetEvent::GateOpened` (not `ApprovalRequested`) into `AppEvent::Budget(Pause { gate_id, ... })` so SSE consumers receive the persisted gate id rather than a fabricated zero uuid. - ironclaw_agent_loop: in the F1 token-counting path, push to `recent_output_token_counts` only when the model response carries `Some(usage)` and only on the `AssistantReply` arm (instead of `unwrap_or(0)`). Diminishing-returns detection now reflects real spend. Net delta: -461 lines (+999 / -1460). Workspace `cargo clippy` clean, `cargo test` clean on ironclaw_resources / ironclaw_loop_support / ironclaw_reborn_composition; budget_e2e + budget_approval_e2e both green. Pre-existing CI failures (`cli::tests::test_version` stack overflow, `facade_factory::production_*` RuntimeProcessPort missing) are unrelated and reproduce on the pristine branch tip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(cli): refresh insta snapshots after runtime-policy flag additions The `import`-feature variants of the help snapshots were left stale when `--deployment-mode`, `--runtime-profile`, `--yolo-disclosure` were added in cc04481 (#3243); the `_without_import` variants were updated but these were not. CI was failing the snapshot assertion under the slim PR matrix (`--features postgres,libsql,html-to-markdown,bedrock,import`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR #3899 thermo-nuclear review (TN #1, #2, #3) TN #1 — budget defaults resolved in wrong layer: - `build_default_budget_accountant` no longer reads process env; it now takes `&BudgetDefaults` as a parameter and the caller owns the config-layer precedence (compiled → section → env) plus the `validate()` call. - `RebornRuntimeInput` gains an optional `budget_defaults` field + `with_budget_defaults()` builder so the composition root passes a pre-resolved value. `build_reborn_runtime` falls back to `compiled_defaults().with_env() + validate()` when none is supplied so existing call sites keep working. TN #2 — gate-store scoping at wrong boundary: - `BudgetGateStore` trait methods (`open`, `resolve`, `expire_pending_older_than`, `get`, `list_pending`) now take `&ResourceScope` as first arg. `GovernorBackedAccountant` passes the caller's scope from `resource_scope(context)`. - `CasSnapshotStore` gains `update_with_scope` so the same store instance can route per-operation. `FilesystemBudgetGateStore` no longer takes scope at construction — one shared instance serves every tenant via the `ScopedFilesystem` mount view. - `InMemoryBudgetGateStore` ignores scope (suitable for single-tenant tests / local-dev); production multi-tenant filesystem path is correctly partitioned by `ResourceScope`. - `RebornRuntime::apply_resolved_budget_gate` now takes scope too. TN #3 — half-wired projection bridge: - Removed `src/bridge/budget_events.rs`, its `spawn_budget_event_projection` helper, the `AppEvent::Budget` variant, and the `AppBudgetEvent` type. No production caller ever subscribed the broadcast sink onto SSE and no frontend consumed the variant, so the half-wired bridge is gone pending a real owner that spawns a projection task with shutdown cancellation. - The runtime's `broadcast_budget_event_sink()` accessor stays so a future production composer can still subscribe without rebuilding the runtime. Bonus — to keep budget e2e tests working under the new libsql local- dev path that origin/reborn-integration introduced, added `PersistentResourceGovernor::with_event_sink` (parity with the `InMemoryResourceGovernor` accessor). The libsql variant of `build_local_dev_store_graph` now wires the composite sink to the persistent governor so governor-emitted `Warned`/`Denied`/`Reserved`/ `Reconciled` events reach subscribers on both feature paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire budget-event projection task into RebornRuntime Re-implements PR #3899 follow-up A2 / Thermo-Nuclear #3 with a real production owner instead of leaving the broadcast sink half-wired: - `crates/ironclaw_reborn_composition/src/budget_events.rs` (new): `BudgetEventObserver` trait + `TracingBudgetEventObserver` default observer + crate-internal `BudgetEventProjection` task that drains the runtime's broadcast `Receiver<BudgetEvent>` and forwards every event to the observer. Cancellation via `CancellationToken`; lagged subscribers logged and resumed; receiver-closed exits cleanly. - `RebornRuntimeInput::with_budget_event_observer(...)` lets production owners install a custom observer (SSE projection, WS fan-out, telemetry export). When unset, the runtime installs the tracing observer so events always surface in structured logs. - `build_reborn_runtime` always spawns the projection task at runtime construction; `RebornRuntime::shutdown` cancels it and awaits the handle so background state drains before the runtime drops. - E2E test `projection_delivers_budget_events_to_installed_observer` drives `build_reborn_runtime` with a capturing observer and asserts the observer sees `Reserved` + `Reconciled` from a real model call, testing through the caller per `.claude/rules/testing.md`. - Existing `broadcast_sink_publishes_events_to_subscribers` updated to expect the runtime's own projection task as a baseline subscriber. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn): rustfmt the merged loop_support import block The conflict resolution for the post-merge import list was not run through rustfmt; CI Formatting flagged the wrapping. No logic change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eriod) (nearai#3841) * Reborn cost-based budgets: foundation (resources, accountant, gate, period) Implements the design from nearai#2843 in crates/ironclaw_resources, crates/ironclaw_loop_support, crates/ironclaw_reborn_config, and crates/ironclaw_agent_loop. Establishes the USD-denominated reservation contract that cascades across the tenant/user/project/agent/mission/thread hierarchy, plus first-touch seeding, calendar+rolling period rollover, graduated intervention (warn -> approval gate -> hard deny), audit-sink primitives, and progress-detection primitives. Zero = unlimited end-to-end. No separate enable flag — every deployment runs the same code path with operator-tunable limits. Phases shipped: - 0: schema (BudgetPeriod, BudgetThresholds, RequiresApproval, snapshot v2) - 1: GovernorBackedAccountant wired into ThreadBackedLoopModelPort - 2: BudgetDefaults config layering (TOML + env, 0 = unlimited preserved) - 3: BudgetApprovalGate primitive + InMemoryBudgetGateStore - 4: BackgroundKind enum on ResourceScope (contract-only) - 5: ParamHash + JSON normalization for loop-stuck detection - 6: BudgetEventSink contract (NoOp + InMemory) - 7: HARD_CAP_* invariants in ironclaw_common ~1600 lines added, ~120 lines modified across 21 files + 6 new modules. 80+ new contract tests across resources/loop_support/reborn_config/agent_loop. Production wiring of GovernorBackedAccountant into the Reborn host factory is intentionally deferred to a follow-up on this branch; the surface is in place via RebornLoopDriverHostFactory::with_model_budget_accountant. Refs: nearai#2843 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR nearai#3841 review: budget reservation & threshold fixes Resolves multi-agent review feedback on the cost-based budget foundation: - Daily USD budgets now actually deplete on successful calls — reconcile records the conservative reservation estimate as actual spend until provider token usage threads through the loop layer. (review: High) - Budget reservation taken AFTER message resolution so a resolution failure cannot orphan an active hold. (review: High) - post_model_call failures on the provider-error path now propagate as host errors instead of being logged-and-swallowed. (review: High) - in_flight entry held until governor reconcile/release succeeds, so a transient storage error leaves the id available for retry/cleanup. - Per-run overlap protection: a second concurrent pre_model_call for the same run releases the new hold and surfaces an accounting error. - Seeding no longer poisons the `seeded` cache when set_limit fails; the account is marked seeded only after a successful read or write. - Threshold logic correctly fires Approval at exactly 100% utilization and uses `pause_at < 1.0` (not utilization-based) as the disable rule. - threshold_decimal/integer use a ThresholdInputs struct instead of silencing clippy::too_many_arguments. - account_snapshot reports the Rolling24h window anchored to set_limit time, not to `now` — UI now sees the ledger's actual window. - progress::is_correlation_key uses eq_ignore_ascii_case against a static slice — zero heap allocation per hash. - progress::replace_uuids / replace_timestamps iterate &[u8] for the ASCII patterns, copying multibyte chars via is_char_boundary — drops the Vec<char> allocation on every hash. - New tests cover: Rolling24h snapshot anchoring, 100% threshold, pause-at-1.0 disable, estimate-as-actual reconcile, overlap rejection, seeding retry after failure, env layer over section in BudgetDefaults, [budget] TOML parser validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#3899) * Reborn budgets: address all nearai#3841 follow-ups end-to-end Implements every open follow-up from PR nearai#3841 (cost-based budgets foundation), driven by the plan in `docs/plans/2026-05-22-reborn-budgets-followups.md`: - **C2 (provider tokens)**: `LoopModelResponse.usage` carries real `(input_tokens, output_tokens)` from `CompletionResponse` / `ToolCompletionResponse`; `usage_for_response` reconciles to actual USD via the cost table instead of the conservative estimate. - **D1 (cascade warnings)**: `CascadeOutcome` variants carry `Vec<BudgetWarning>` so warnings preceding a pause or hard deny reach the audit sink. `ResourceError::LimitExceeded` / `RequiresApproval` reshaped to struct variants. - **C1 (cancellation safety)**: new `LoopModelBudgetAccountant::release_in_flight` trait hook + RAII `ReservationReleaseGuard` in `HostManagedLoopModelPort::stream_model` so a cancelled future doesn't orphan its reservation. - **E1 (dead code)**: removed the never-set `budget_accountant` field on `ThreadBackedLoopModelPort`. - **Real cost table**: new `StaticModelCostTable` + `LlmModelProfilePolicy::build_cost_table()` populated from `ironclaw_llm::costs::model_cost` with `default_cost` fallback so unknown providers never silently reconcile to zero. - **B1 (filesystem gate store)**: new `FilesystemBudgetGateStore` mirroring `FilesystemResourceGovernorStore`; pending gates survive process restart. - **A1 (production wiring)**: composition builds `GovernorBackedAccountant` from the cost table + governor and threads it through `RebornLoopDriverHostFactory::with_model_budget_accountant`. - **A2 (audit / SSE projection)**: `InMemoryResourceGovernor::with_event_sink` emits `Reserved`, `Reconciled`, `Released`, `Warned`, `ApprovalRequested`, `Denied`, `LimitChanged`; composition holds an `InMemoryBudgetEventSink` ready for downstream SSE projection. - **F1 (stuck-loop normalization)**: `CapabilityCallSignature::from_call` now runs `progress::normalize_for_hash` so the existing repetition window collapses request-id / UUID / timestamp noise. Side fix: `ResourceValue` moved to adjacent serde tagging (the combination of internal tagging + `Decimal`'s `serde-with-str` representation breaks JSON serialization — rust-lang/serde#1402). Regression tests added per item — see the acceptance evidence appendix in the plan doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Reborn budgets: end-to-end test coverage via test-support feature Adds 13 e2e tests covering the budget pipeline through `build_reborn_runtime` + `send_user_message`. Required infrastructure: - **`test-support` feature** on `ironclaw_reborn_composition` exposing `BudgetTestGateway` (scripted token usage) and `RebornRuntimeInputTestExt`. Existing `model_gateway_override` field promoted from `#[cfg(test)]` to `#[cfg(any(test, feature = ...))]` with a new public `with_model_gateway_override_for_tests` setter. - **Cost-table override** on `RebornRuntimeInput` so tests can pair the gateway with a deterministic `ModelCostTable`. Without this, an override gateway dropped the cost table and the accountant never fired. - **Budget accessors** on `RebornRuntime`: `budget_resource_governor`, `budget_event_sink`, `budget_gate_store`, and `apply_resolved_budget_gate`. Test-feature gated. - **`ResourceGovernor::usage_for`** added as a default-impl trait method so tests read spend through the trait surface. - **`BudgetGateStore` wired into the accountant**: `GovernorBackedAccountant::with_gate_store(...)` opens a pending gate whenever the governor cascade returns `RequiresApproval`. The approval-required host error is unchanged; the gate is the out-of-band channel a user-facing handler resolves. Scenarios covered: | # | Test | What it asserts | |---|---|---| | F1 | `f1_happy_path_records_actual_usd_in_ledger` | Ledger depletes by provider tokens × cost table | | F2 | `f2_crossing_warn_threshold_emits_warned_event` | Warn fires alongside successful Reserved/Reconciled | | F3 | `f3_approval_with_increased_limit_unblocks_retry` | Approve → set_limit applies → retry succeeds | | F4 | `f4_cancel_keeps_budget_blocked_on_retry` | Cancel → retry still short-circuits | | F5 | `f5_expiry_marks_gate_terminal_and_keeps_budget_blocked` | Expiry → gate drops from pending list, retry still blocked | | F6 | `f6_hard_cap_denied_before_provider_call` | Estimate over cap → zero model calls, Denied event | | C1 | `c1_provider_tokens_reconcile_to_actual_usd` | Real numbers, not estimate | | C2 | `c2_unknown_model_in_cost_table_reconciles_to_zero` | Unknown profile → zero spend | | C3 | `c3_zero_cost_model_records_zero_spend` | Free model → zero USD with non-zero tokens | | D1 | `d1_agent_deny_preserves_user_warn_event` | Cascade emits both Warned and Denied | | D3 | `d3_fresh_user_without_limits_runs_without_denial` | No limit → no denial | | + | `pause_in_distinct_runs_produces_distinct_pending_gates` | Per-run gate identity | | + | `budget_test_gateway_scripted_replies_drive_per_turn_costs` | Multi-turn scripted accumulation | F7 (cancellation mid-stream) is unit-covered by `release_in_flight_drains_orphan_reservation_on_cancellation`. D2 (period rollover) is unit-covered by `rolling_24h_snapshot_reports_anchored_window_not_now_window`. B-series (background ticks) await the BackgroundKind scheduler call site (no production caller in Reborn yet). Run via `cargo test -p ironclaw_reborn_composition --features test-support`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Budget review feedback: address all 7 findings from PR nearai#3899 review Two High and five Medium issues raised by serrrfirat's multi-agent review. **High #1 — `FilesystemBudgetGateStore` cross-tenant leakage** The store hardcoded `ResourceScope::system()` for every op, so all tenants wrote into the same `/tenants/__SYSTEM__/...` snapshot and `list_pending` would expose gates across tenants. Fix: `new(...)` now takes a `ResourceScope`; each tenant gets its own store, and the `ScopedFilesystem` mount view routes the snapshot under that tenant's path. Added `list_pending_does_not_leak_across_tenants` regression. **High #2 — accountant wired without default budget limits** Composition built `GovernorBackedAccountant` without `with_seeding_policy`, so the local-dev governor started empty and `reserve_with_outcome_in_state` skipped accounts that had no configured limit — model calls reconciled spend but never enforced a cap. Fix: `build_reborn_runtime` now loads `BudgetDefaults::compiled_defaults().with_env()` and wires `BudgetSeedingPolicy` + `with_overestimate_factor`. Renamed the D3 test to `d3_seeding_policy_installs_default_cap_on_first_touch` to prove the wiring fires. **Medium #3 — RAII guard disarmed before post_model_call await** `HostManagedLoopModelPort::stream_model` was disarming the `ReservationReleaseGuard` before awaiting `post_model_call`. A cancellation during that await dropped the future without cleanup, orphaning the reservation. Fix: disarm AFTER `post_model_call` returns. `release_in_flight` is now idempotent (peek-then-release- then-remove) so a successful post-call + subsequent guard drop is a no-op. **Medium #4 — failed release drops the retry handle** `release_in_flight` removed the in-flight entry before calling `governor.release`. A transient storage error left the reservation active in the governor with the id discarded. Fix: peek first, release, only remove on success. Errors keep the entry retained for a future retry / cleanup hook. **Medium #5 — unknown model silently reconciles to zero USD** Both `estimate_for` and `usage_for_response` fell back to `ModelCost { 0, 0, 0 }` when the cost table had no entry for the effective model. Cost-table drift would silently bypass daily caps. Fix: `GovernorBackedAccountant` carries a `default_cost` (default ~ GPT-4o pricing, ~`$0.0000025 input + $0.00001 output per token`) used for unknown models. Callers wiring `ZeroCostTable` for free / Ollama explicitly opt out of the fallback. Updated the C2 e2e test to assert the new fail-closed shape. **Medium #6 — paused dimension lost when another hard-denies** `check_thresholds_all_interventions` stored `Approval` only in the `approval` slot, so when one dimension paused and another hard-denied, the `Deny { warnings, denial }` outcome lost the pause signal. Fix: also push a warning-shaped record for the paused dimension. **Medium #7 — unbounded terminal-gate retention** The snapshot kept every gate forever; `open` / `resolve` / `get` / `list_pending` were O(total historical gates). Fix: `with_terminal_retention` (default 30 days). Every mutation prunes terminal gates whose resolution timestamp is older than the window. Added `terminal_gates_older_than_retention_are_pruned_on_next_write` regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: replace lock-poisoned expects with PoisonError::into_inner scripts/check_no_panics.py flagged five .expect("...lock poisoned") calls in the new test_support.rs. Use the same idiomatic recovery pattern the rest of the codebase uses (see InMemoryBudgetGateStore, InMemoryBudgetEventSink): on a poisoned lock, recover the inner data via PoisonError::into_inner rather than panicking. The test gateway's state is append-only logs / replies queues, so reading them through a poisoned lock is safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Finish A1 / A2 / F1 from plan + honest plan doc update The plan claimed "all nine items landed" but A1 (production wiring), A2 (SSE projection), and F1 (full progress strategy) were partials. This commit finishes the work so the plan matches reality. **A1 — production-shape accountant builder** New `ironclaw_reborn_composition::build_default_budget_accountant` public helper that wires the seeding policy + overestimate factor + gate store from `BudgetDefaults::compiled_defaults().with_env()` and returns an `Arc<dyn LoopModelBudgetAccountant>`. Production loop composers call this with their `PersistentResourceGovernor` + `FilesystemBudgetGateStore` + LLM-policy-derived cost table; the local-dev runtime in `build_reborn_runtime` now uses the same helper instead of duplicating the seeding logic inline. Unit-tier regression `seeds_compiled_default_user_cap_on_first_touch` proves the helper installs the compiled-default $5 user cap on first model call. **A2 — broadcast sink + AppEvent projection** - `ironclaw_resources::BroadcastBudgetEventSink` wraps `tokio::sync::broadcast::Sender<BudgetEvent>` with `subscribe()` / `subscriber_count()`. `CompositeBudgetEventSink` fans events to multiple sinks. - Composition fans every `BudgetEvent` to the in-memory sink (for tests) AND the broadcast sink (for SSE projection) via `CompositeBudgetEventSink`. - New `AppEvent::BudgetWarn` / `BudgetPause` / `BudgetDenied` / `BudgetLimitChanged` wire-stable variants in `ironclaw_common::event`. - `src/bridge/budget_events.rs` carries the projection: a tokio task spawned by `spawn_budget_event_projection` drains the broadcast receiver and emits the appropriate `AppEvent` via `SseManager::broadcast_for_user`. System-scoped events (no user identity) are skipped. This is the only producer of these `AppEvent` variants per `.claude/rules/gateway-events.md`. - `RebornRuntime::broadcast_budget_event_sink()` exposes the sink to the binary so the startup path subscribes. E2E test `broadcast_sink_publishes_events_to_subscribers` drives a real `send_user_message` and asserts Reserved + Reconciled lands on the broadcast. **F1 — diminishing-returns stop condition** The earlier shipped `ParamHash` normalization in `CapabilityCallSignature` strengthened the existing `recent_call_signatures`-based repetition detector. This commit adds the second half of F1: a rolling output-token window that detects "wedged" loops the repetition detector misses (model keeps responding but produces no useful output). - `LoopExecutionState.recent_output_token_counts: BoundedRing<u32, 8>` populated by the executor from `LoopModelResponse::usage`. - `BoundedRing::iter` returns `impl DoubleEndedIterator` so the strategy can scan the trailing window. - `DefaultStopConditionStrategy` gets `min_delta_tokens` (default 4) + `noprogress_window` (default 4). When the last N turns all produce ≤ min_delta_tokens of output, fire `StopKind::NoProgressDetected`. - Regression tests: `four_consecutive_low_token_turns_trigger_no_progress` proves the detector fires; `occasional_low_token_turn_does_not_trip_no_progress` proves a productive turn resets the trailing count. **Plan doc** Updated the status header from "all nine items landed" to the honest per-item shape. Acceptance evidence table expanded with the new test names. New "Review-feedback fixes layered on top" subsection documenting all 2 High + 5 Medium findings addressed during review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address thermo-nuclear review: collapse filesystem-store duplication, flatten cfg permutations, split budget accountant Five structural simplifications surfaced by the deep audit on PR nearai#3899, plus two bug fixes from the earlier review pass: - ironclaw_resources: extract `cas_snapshot` shared infrastructure (`StorageError` + `Snapshot` traits + `CasSnapshotStore<F>` + async-runtime worker + per-path lock map) and merge `filesystem_gate_store.rs` into `filesystem_store.rs`. Deletes ~350 lines of duplicated read-modify-write + worker-thread + CAS machinery; both stores are now thin shims over the shared helper. - ironclaw_reborn_composition: flatten the 4-way cfg permutation in `build_reborn_runtime` model-gateway resolution into three flat steps (normalize override → build production gateway via cfg-gated helper → test override wins). Also drops the `unused_mut` warning. - ironclaw_reborn_composition: collapse the 3-layer test-only setter dance for `model_gateway_override` / `model_cost_table_override` into a single setter pair gated on `cfg(any(test, feature = "test-support"))`. Deletes the `RebornRuntimeInputTestExt` extension trait — integration tests now call the inherent methods directly. - ironclaw_loop_support: split the 1305-line `budget_accountant.rs` into `budget_cost_table.rs` (ModelCost/ModelCostTable trait/ZeroCostTable/ StaticModelCostTable), `budget_seeding.rs` (BudgetSeedingPolicy), and `budget_accountant.rs` (just GovernorBackedAccountant). Each module now owns one concern. - ironclaw_resources: add `impl Display for ResourceAccount` and route the hierarchical account-label rendering through it; delete the 60-line bespoke `account_label` helper from `src/bridge/budget_events.rs`. - ironclaw_common + bridge: collapse the four `AppEvent::Budget*` variants into a single typed `AppEvent::Budget(AppBudgetEvent)` with the four shapes carried inside the enum. Wire-shape stays identical (snake_case serde tag). - ironclaw_resources + ironclaw_loop_support: thread real gate id through `BudgetEvent::GateOpened { gate_id, needed, at }` (new variant) and have the accountant emit it via the broadcast event sink after store.open succeeds. The bridge now projects `BudgetEvent::GateOpened` (not `ApprovalRequested`) into `AppEvent::Budget(Pause { gate_id, ... })` so SSE consumers receive the persisted gate id rather than a fabricated zero uuid. - ironclaw_agent_loop: in the F1 token-counting path, push to `recent_output_token_counts` only when the model response carries `Some(usage)` and only on the `AssistantReply` arm (instead of `unwrap_or(0)`). Diminishing-returns detection now reflects real spend. Net delta: -461 lines (+999 / -1460). Workspace `cargo clippy` clean, `cargo test` clean on ironclaw_resources / ironclaw_loop_support / ironclaw_reborn_composition; budget_e2e + budget_approval_e2e both green. Pre-existing CI failures (`cli::tests::test_version` stack overflow, `facade_factory::production_*` RuntimeProcessPort missing) are unrelated and reproduce on the pristine branch tip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(cli): refresh insta snapshots after runtime-policy flag additions The `import`-feature variants of the help snapshots were left stale when `--deployment-mode`, `--runtime-profile`, `--yolo-disclosure` were added in e9628ed (nearai#3243); the `_without_import` variants were updated but these were not. CI was failing the snapshot assertion under the slim PR matrix (`--features postgres,libsql,html-to-markdown,bedrock,import`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address PR nearai#3899 thermo-nuclear review (TN #1, #2, #3) TN #1 — budget defaults resolved in wrong layer: - `build_default_budget_accountant` no longer reads process env; it now takes `&BudgetDefaults` as a parameter and the caller owns the config-layer precedence (compiled → section → env) plus the `validate()` call. - `RebornRuntimeInput` gains an optional `budget_defaults` field + `with_budget_defaults()` builder so the composition root passes a pre-resolved value. `build_reborn_runtime` falls back to `compiled_defaults().with_env() + validate()` when none is supplied so existing call sites keep working. TN #2 — gate-store scoping at wrong boundary: - `BudgetGateStore` trait methods (`open`, `resolve`, `expire_pending_older_than`, `get`, `list_pending`) now take `&ResourceScope` as first arg. `GovernorBackedAccountant` passes the caller's scope from `resource_scope(context)`. - `CasSnapshotStore` gains `update_with_scope` so the same store instance can route per-operation. `FilesystemBudgetGateStore` no longer takes scope at construction — one shared instance serves every tenant via the `ScopedFilesystem` mount view. - `InMemoryBudgetGateStore` ignores scope (suitable for single-tenant tests / local-dev); production multi-tenant filesystem path is correctly partitioned by `ResourceScope`. - `RebornRuntime::apply_resolved_budget_gate` now takes scope too. TN #3 — half-wired projection bridge: - Removed `src/bridge/budget_events.rs`, its `spawn_budget_event_projection` helper, the `AppEvent::Budget` variant, and the `AppBudgetEvent` type. No production caller ever subscribed the broadcast sink onto SSE and no frontend consumed the variant, so the half-wired bridge is gone pending a real owner that spawns a projection task with shutdown cancellation. - The runtime's `broadcast_budget_event_sink()` accessor stays so a future production composer can still subscribe without rebuilding the runtime. Bonus — to keep budget e2e tests working under the new libsql local- dev path that origin/reborn-integration introduced, added `PersistentResourceGovernor::with_event_sink` (parity with the `InMemoryResourceGovernor` accessor). The libsql variant of `build_local_dev_store_graph` now wires the composite sink to the persistent governor so governor-emitted `Warned`/`Denied`/`Reserved`/ `Reconciled` events reach subscribers on both feature paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire budget-event projection task into RebornRuntime Re-implements PR nearai#3899 follow-up A2 / Thermo-Nuclear #3 with a real production owner instead of leaving the broadcast sink half-wired: - `crates/ironclaw_reborn_composition/src/budget_events.rs` (new): `BudgetEventObserver` trait + `TracingBudgetEventObserver` default observer + crate-internal `BudgetEventProjection` task that drains the runtime's broadcast `Receiver<BudgetEvent>` and forwards every event to the observer. Cancellation via `CancellationToken`; lagged subscribers logged and resumed; receiver-closed exits cleanly. - `RebornRuntimeInput::with_budget_event_observer(...)` lets production owners install a custom observer (SSE projection, WS fan-out, telemetry export). When unset, the runtime installs the tracing observer so events always surface in structured logs. - `build_reborn_runtime` always spawns the projection task at runtime construction; `RebornRuntime::shutdown` cancels it and awaits the handle so background state drains before the runtime drops. - E2E test `projection_delivers_budget_events_to_installed_observer` drives `build_reborn_runtime` with a capturing observer and asserts the observer sees `Reserved` + `Reconciled` from a real model call, testing through the caller per `.claude/rules/testing.md`. - Existing `broadcast_sink_publishes_events_to_subscribers` updated to expect the runtime's own projection task as a baseline subscriber. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(reborn): rustfmt the merged loop_support import block The conflict resolution for the post-merge import list was not run through rustfmt; CI Formatting flagged the wrapping. No logic change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Summary
Implements cost-based budgeting for IronClaw Reborn per #2843. Replaces
implicit iteration-count caps with USD-denominated reservations that cascade
across
tenant → user → project → agent → mission → thread, with calendaror rolling-24h period rollover and graduated intervention (warn → pause
with approval gate → hard deny). Setting any USD limit to
0meansunlimited — no separate "disabled" flag needed; the same code path runs
in every environment.
This is the foundation PR: ledger, accountant, gate primitive, period
rules, audit-sink contract, progress-detection primitives, and config
plumbing all land here. Production wiring (host_factory injection,
gateway UI, audit-event projection) is intentionally deferred to follow-ups
on this branch.
Cross-reference
Implements the design from #2843. See
docs/plans/reborn-budgets.md(coming in a follow-up) for the rollout matrix; this PR ships the
implementation seams.
What lands
Phase 0 —
ironclaw_resourcesschema extensionBudgetPeriod(PerInvocation | Rolling24h | Calendar{tz,unit}),PeriodUnit,BudgetThresholds,ResourceApprovalNeeded,BudgetWarning,ReservationOutcome,AccountSnapshot,Clock/SystemClock/FakeClock.ResourceLimitscarriesperiodandthresholds. 0 = unlimited isbaked into
check_decimal/check_integer.ResourceError::RequiresApproval(...)variant (hardLimitExceededstill wins over approval gates).
reserve_with_outcome,reserve_with_id_and_outcome,account_snapshot. Oldreserve/reserve_with_idkeep working viadefault impls — no churn at non-budget call sites.
crates/ironclaw_resources/tests/).Phase 1 — LLM cost bridge (
ironclaw_loop_support)GovernorBackedAccountantimplementsLoopModelBudgetAccountant:reserves an estimated cost in
pre_model_call, reconciles (success)or releases (failure) in
post_model_call.ModelCostTabletrait +ZeroCostTablefor local/free models.BudgetSeedingPolicy+ first-touch seeder that installs user/projectdefaults the first time the accountant sees them, without overwriting
user-set limits.
ThreadBackedLoopModelPort::with_budget_accountant(...)builder injectsthe accountant into the existing model-port pipeline; pre/post hooks
fire on the in-flight reservation map keyed by
TurnRunId.AgentLoopHostErrorKindvariants:BudgetApprovalRequired,BudgetAccountingFailed. Wired throughtext_loop_driver.rsandmodel_gateway.rsmapping tables.LoopModelGatewayError::into_host_errorpromoted from private to
pubso accountants can convert.Phase 2 — Config defaults (
ironclaw_reborn_config)[budget]TOML section, env-var overrides(
IRONCLAW_BUDGET_USER_DAILY_USD, etc.), and a resolvedBudgetDefaultstype with full layering (compiled defaults → TOML →env). 0 = unlimited preserved end-to-end.
pause_at < warn_at,overestimate_factor < 1.0.ironclaw_architecture/tests/reborn_dependency_boundaries.rsrule thatthis crate must stay workspace-dep-free.
Phase 3 — Budget approval gate (
ironclaw_resources::gate)BudgetApprovalGate,BudgetGateId,BudgetGateStatus(
Pending | Approved | Cancelled | Expired),BudgetGateOutcome.BudgetGateStoretrait +InMemoryBudgetGateStore. Filesystem-backedstore deferred — the gate state machine is the contract.
unknown-gate, expiry, and pending-list filtering.
Phase 4 — Background-invocation scope contract (
ironclaw_host_api)BackgroundKindenum:HeartbeatTick | RoutineLightweight | RoutineStandard | MissionTick | ContainerJob | UserInitiated.per-kind ledgers (
skip-and-persistsemantics on exhaustion) withoutanother schema bump.
Phase 5 — Progress detection (
ironclaw_agent_loop::progress)ParamHashnewtype: blake3 over JCS-canonicalized, normalized JSON.correlation/request-id keys before hashing — same tool call with
different
request_idcollapses to one hash.normalization, recursive arrays, and the "long hex without dashes ≠
uuid" edge.
ironclaw_agent_loop::progressmodule re-exports the typedprimitive for downstream stuck-loop detectors.
Phase 6 — Audit sink contract (
ironclaw_resources::event)BudgetEventenum (Reserved | Reconciled | Released | Warned | Denied | ApprovalRequested | ApprovalResolved | LimitChanged).BudgetEventSinktrait +NoOpBudgetEventSink+InMemoryBudgetEventSink(records and drains for tests).scoped — the contract is in place so audit/SSE projections can land
in a follow-up without a churning trait change.
Phase 7 — Invariant backstops (
ironclaw_common)HARD_CAP_WALL_CLOCK_SECS = 86_400(24h).HARD_CAP_ITERATIONS = 10_000.HARD_CAP_BUDGET_USD = "100.00"(string form; parsed to Decimal atcall sites without forcing a workspace dep on
rust_decimalhere).MAX_WORKER_ITERATIONS = 500left in place — it's still theworker-job cap until the budget enforcer fully replaces it (Cost-based budgeting: replace iteration/time caps with USD budgets cascading user → project → mission → thread #2843
Phase D in the original plan).
Test plan
cargo fmt --all— clean.cargo clippy --workspace --all-targets --all-features— zerowarnings.
cargo test -p ironclaw_resources— 50+ tests pass (15 pre-existing,17 new contract tests for periods/thresholds/0=unlimited/gates/events).
cargo test -p ironclaw_loop_support— 81 tests pass including 12new accountant + seeding tests.
cargo test -p ironclaw_agent_loop --lib strategies::progress—7 new progress tests pass.
cargo test -p ironclaw_reborn_config— 32 tests pass including5 new
BudgetDefaultsvalidation tests.cargo test --workspace --all-features— running.ResourceError::RequiresApprovalandBudgetApprovalRequiredpropagate end-to-end through a real modelcall (needs Phase 2 host-factory injection, deferred follow-up).
Out of scope (explicit follow-ups)
GovernorBackedAccountantinto production host factory. Thefield already exists (
RebornLoopDriverHostFactory::with_model_budget_accountant);it currently defaults to
NoOpBudgetAccountant. A follow-up PR addscomposition wiring driven by
BudgetDefaults.BudgetGateStore. In-memory store ships here;durable store mirrors the existing
FilesystemResourceGovernorStorepattern.
BudgetEventinto the gateway event log.Contract is here; projection sink is the next PR.
BackgroundKindscopeexists in
ironclaw_host_api; the scheduler/runner side has noproduction call sites yet in Reborn (the
turn_runnerheartbeat islease-keepalive, not agent ticking).
ProgressStrategyas a stop-condition. TheParamHashprimitive ships here; the strategy that consumes recentstep deltas + repeated-call counts is a follow-up alongside the
composition wiring.
Migration notes
ResourceGovernor::reserve/reserve_with_idcontinue to work — the new methods are default-impl'd. No churn at
~50 production call sites.
BudgetPeriod::PerInvocationwithBudgetThresholds::DISABLED(matches pre-existing v1 behavior bit-for-bit). The first mutation
rewrites the file in v2 shape.
ResourceLimitsgainsperiod+thresholds. Existing literalsthat didn't set those fields fall back to
Default(PerInvocation +DISABLED), preserving v1 behavior.
ResourceLimitslosesEqbecauseBudgetThresholdscarriesf64.One internal
ResourceStatederivedEqwas relaxed toPartialEq;no public types changed
Eqsemantics on a path that was being used.🤖 Generated with Claude Code