From 9df06018efbbb27471fdcfd1a2f73746daf11087 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Tue, 4 Aug 2026 15:44:02 +1200 Subject: [PATCH 1/4] fix: harden detection layer and rate limit refill clock --- CHANGELOG.md | 1 + src/detection/loop_detector.rs | 99 +++++++++++++++++++++++-- src/middleware/unknown_tool.rs | 55 ++++++++++++-- src/stream/rate_limit.rs | 129 ++++++++++++++++++++++++++------- src/tool/shield.rs | 31 +++++++- 5 files changed, 274 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31ac349..9aa6462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. ### Fixed +- Detection-layer long-tail correctness (six items): `check_file_reads` now normalizes the query under the recorded op's real tool name (was: empty string) and matches by bidirectional path containment (was: exact equality); `LoopDetector::new` clamps `window_size == 0` to 1 with a warning; `find_best_match` rejects zero-score candidates even at threshold 0.0 and breaks ties lexicographically; `ToolShield::with_thresholds` swaps inverted warn/block pairs with a warning. All non-breaking. - `SessionConfig::compact_threshold` now clamps into its documented `0..=100` range on the validating construction paths (`Default`, `with_compact_threshold`, and `Deserialize`). Previously only the `with_compact_threshold` builder clamped; deserialized configs could carry an out-of-range value (e.g. `200` from disk), which the compaction subsystem would then interpret as "never compact." A single canonical clamp method plus a field-level serde deserialize helper now enforce the range silently. `Default` was already in range (80); its clamp call is defensive. Direct public struct-literal construction (`SessionConfig { compact_threshold: 200, .. }`) still bypasses normalization — the field is `pub`, and callers using that form are responsible for honoring the documented range. Non-breaking (silent clamp; no signature changes). - Corrected the `ParallelMode::Parallel` doc, which falsely claimed detection/observer side-effects "are not thread-safe" and fire "once on the final result only" in parallel mode. They are thread-safe (`DetectionManager` and the observer registry use `Mutex`/immutable-`Vec` interiors; `ToolHealthRegistry` uses atomics) and fire on every retry attempt in both modes, exactly as the code already does. No behavior change; the code matched the corrected doc all along. - Fixed code-level doc contradictions: the `ApiClient` trait example showed `request: StreamRequest` (by-value) instead of `&StreamRequest` (matches the real trait), and `BareLoop::machine` was described as an "empty placeholder" rather than the real "empty machine (no history, no pending messages)". diff --git a/src/detection/loop_detector.rs b/src/detection/loop_detector.rs index 8a6ad2b..1311dd8 100644 --- a/src/detection/loop_detector.rs +++ b/src/detection/loop_detector.rs @@ -1098,7 +1098,13 @@ impl LoopDetector { /// let config = LoopDetectorConfig { window_size: 100, ..Default::default() }; /// let detector = LoopDetector::new(config, Arc::new(NoOpToolSignature)); /// ``` - pub fn new(config: LoopDetectorConfig, signature: Arc) -> Self { + pub fn new(mut config: LoopDetectorConfig, signature: Arc) -> Self { + if config.window_size == 0 { + tracing::warn!( + "LoopDetectorConfig.window_size was 0; clamping to 1 (the smallest sensible window)" + ); + config.window_size = 1; + } Self { operations: Mutex::new(VecDeque::with_capacity(config.window_size)), config, @@ -1580,13 +1586,16 @@ impl LoopDetector { }; let sig = &self.signature; - let normalized_input = sig.normalize_param_for_comparison("", file_path); let read_count = ops .iter() .filter(|o| { - sig.is_file_read_tool(&o.tool) - && sig.normalize_param_for_comparison(&o.tool, &o.primary_param) - == normalized_input + if !sig.is_file_read_tool(&o.tool) { + return false; + } + let normalized_op = sig.normalize_param_for_comparison(&o.tool, &o.primary_param); + let normalized_query = sig.normalize_param_for_comparison(&o.tool, file_path); + normalized_op.contains(normalized_query.as_str()) + || normalized_query.contains(normalized_op.as_str()) }) .count(); @@ -2539,4 +2548,84 @@ mod tests { "Should contain tool signature suggestion: {msg}" ); } + + /// Signature that flags the `Read` tool as a file-read tool and prefixes + /// the primary parameter with the tool name during comparison, so tests + /// can verify which tool name `check_file_reads` passes to + /// `normalize_param_for_comparison`. + struct ToolNameNormalizingSig; + + impl ToolSignature for ToolNameNormalizingSig { + fn is_file_read_tool(&self, tool: &str) -> bool { + tool == "read" + } + + fn normalize_param_for_comparison(&self, tool: &str, param: &str) -> String { + format!("{tool}:{param}") + } + } + + #[test] + fn check_file_reads_matches_containment_both_directions() { + // Use the shared TestToolSignature which treats "Read" as a file-read + // tool and leaves params unchanged under normalization. Lower the + // threshold so a single recorded read trips the check. + let config = LoopDetectorConfig { + max_same_file_reads: 1, + ..Default::default() + }; + let mk = || LoopDetector::new(config.clone(), Arc::new(TestToolSignature)); + + // Forward direction: op path "/a/b/c" contains query "/a/b". + let detector = mk(); + detector.record(Operation::new("Read", "/a/b/c")); + assert!( + detector.check_file_reads("/a/b"), + "op path containing the query path should match" + ); + + // Reverse direction: query "/a/b/c" contains op path "/a". + let detector2 = mk(); + detector2.record(Operation::new("Read", "/a")); + assert!( + detector2.check_file_reads("/a/b/c"), + "query path containing the op path should match" + ); + } + + #[test] + fn check_file_reads_uses_real_tool_name() { + // ToolNameNormalizingSig prepends the tool name during normalization, + // so the match only succeeds if check_file_reads passes the recorded + // op's tool name ("read") rather than an empty string. + let detector = LoopDetector::new( + LoopDetectorConfig { + max_same_file_reads: 1, + ..Default::default() + }, + Arc::new(ToolNameNormalizingSig), + ); + + detector.record(Operation::new("read", "/file")); + // normalize_param_for_comparison("read", "/file") == "read:/file" + // and the query is normalized with the same tool name, so the + // containment check only holds if both use "read". + assert!( + detector.check_file_reads("/file"), + "check_file_reads must normalize under the recorded op's tool name" + ); + } + + #[test] + fn window_size_zero_clamps_to_one() { + let config = LoopDetectorConfig { + window_size: 0, + ..Default::default() + }; + let detector = LoopDetector::new(config, Arc::new(TestToolSignature)); + assert_eq!( + detector.config.window_size, 1, + "a window_size of 0 must be clamped to 1" + ); + } } diff --git a/src/middleware/unknown_tool.rs b/src/middleware/unknown_tool.rs index 9429e2f..1455e62 100644 --- a/src/middleware/unknown_tool.rs +++ b/src/middleware/unknown_tool.rs @@ -148,9 +148,11 @@ impl UnknownToolMiddleware { /// Find the best matching tool name from a list, given a threshold. /// - /// Returns the name with the highest similarity score that meets or - /// exceeds `threshold`. Returns `None` when no candidate scores high - /// enough. + /// Returns the name with the highest similarity score that exceeds zero + /// and meets or exceeds `threshold`. Ties (equal scores) are broken + /// lexicographically — the smaller name wins — so the result is + /// deterministic regardless of candidate order. Returns `None` when no + /// candidate scores high enough. fn find_best_match_inner<'a>( requested: &str, available: &[&'a str], @@ -159,10 +161,18 @@ impl UnknownToolMiddleware { let mut best: Option<(&'a str, f64)> = None; for &name in available { let score = Self::similarity(requested, name); - if score >= threshold { + if score > 0.0 && score >= threshold { match best { - Some((_, best_score)) if score <= best_score => {} - _ => best = Some((name, score)), + None => best = Some((name, score)), + Some((_, best_score)) if score > best_score => { + best = Some((name, score)); + } + Some((best_name, best_score)) + if (score - best_score).abs() < f64::EPSILON && name < best_name => + { + best = Some((name, score)); + } + _ => {} } } } @@ -385,6 +395,39 @@ mod tests { ); } + #[test] + fn find_best_match_threshold_zero_rejects_zero_score() { + let available = ["bbb"]; + assert!( + UnknownToolMiddleware::find_best_match_inner("xyz", &available, 0.0).is_none(), + "a zero-similarity candidate must be rejected even at threshold 0.0" + ); + } + + #[test] + fn find_best_match_ties_break_lexicographically() { + let forward = ["toll", "toil"]; + let (suggestion, score) = + UnknownToolMiddleware::find_best_match_inner("tool", &forward, 0.0).unwrap(); + assert_eq!( + suggestion, "toil", + "tie should break to the lexicographically smaller name" + ); + assert!( + (score - 0.8).abs() < 1e-6, + "expected tied score 0.8, got {score}" + ); + + // Reversed input order must yield the same winner. + let reversed = ["toil", "toll"]; + let (suggestion_rev, _) = + UnknownToolMiddleware::find_best_match_inner("tool", &reversed, 0.0).unwrap(); + assert_eq!( + suggestion_rev, "toil", + "tie-break must be order-independent" + ); + } + use crate::message::ToolContent; use crate::tool::ToolDispatchResult; use std::time::Duration; diff --git a/src/stream/rate_limit.rs b/src/stream/rate_limit.rs index 564e86b..ffd1988 100644 --- a/src/stream/rate_limit.rs +++ b/src/stream/rate_limit.rs @@ -1,9 +1,8 @@ //! Proactive client-side rate limiting (token bucket). //! -//! A [`TokenBucket`] is a continuous-fill bucket: it allows a burst equal to its -//! capacity, then refills at `capacity / 60` tokens per second. [`RateLimiter`] -//! holds one bucket per provider identity (`base_url`) so distinct providers get -//! independent budgets. +//! A [`TokenBucket`] allows a burst equal to its capacity, then refills at +//! `capacity / 60` tokens per second. [`RateLimiter`] holds one bucket per +//! provider identity (`base_url`) so distinct providers get independent budgets. //! //! This is the proactive complement to the reactive 429 handling in //! `stream::handler`: the bucket gates a request *before* it fires, @@ -143,18 +142,9 @@ impl TokenBucket { self.take_at(Instant::now()) } - /// Tokens available at a known instant (after a lazy refill). Non-consuming. + /// Tokens available at a known instant (after a lazy refill). /// - /// # Example - /// - /// ```rust - /// use loopctl::stream::rate_limit::TokenBucket; - /// - /// let bucket = TokenBucket::new(10); - /// // A fresh bucket is full. - /// assert!((bucket.available() - 10.0).abs() < 0.5); - /// ``` - #[must_use] + /// Advances `last_refill` to `at`, banking the elapsed refill. pub fn available_at(&self, at: Instant) -> f64 { let mut state = self .state @@ -163,12 +153,7 @@ impl TokenBucket { elapsed_refill(&mut state, at, self.capacity, self.refill_per_sec) } - /// Tokens available right now (after a lazy refill). Non-consuming. - /// - /// Thin wrapper around [`available_at`](Self::available_at) that - /// plugs in `Instant::now()`. Useful for observability — reporting - /// the current budget without consuming a token. - #[must_use] + /// Tokens available right now (after a lazy refill). pub fn available(&self) -> f64 { self.available_at(Instant::now()) } @@ -176,18 +161,34 @@ impl TokenBucket { /// Apply the elapsed-time refill to `state` in place and return the new token count. /// -/// Caps at `capacity` so a long idle does not overflow. Leaves `last_refill` at -/// `at` so the next caller only accounts for the gap since this call. +/// Caps at `capacity` so a long idle does not overflow. When the bucket fills +/// before `at` (the elapsed time exceeded what was needed to reach capacity), +/// `last_refill` advances only to the fill point — not to `at`. This prevents +/// a caller that passes a far-future instant from freezing the bucket: the +/// excess time beyond capacity is irrelevant, so the clock ignores it. fn elapsed_refill(state: &mut BucketState, at: Instant, capacity: f64, refill_per_sec: f64) -> f64 { let elapsed = at.saturating_duration_since(state.last_refill); if elapsed.is_zero() { return state.tokens; } + if refill_per_sec <= 0.0 { + return state.tokens; + } let added = elapsed.as_secs_f64() * refill_per_sec; - let topped = (state.tokens + added).min(capacity); - state.tokens = topped; - state.last_refill = at; - topped + let raw = state.tokens + added; + if raw >= capacity { + let needed = capacity - state.tokens; + let secs_to_fill = needed / refill_per_sec; + state.last_refill = state + .last_refill + .checked_add(Duration::from_secs_f64(secs_to_fill)) + .unwrap_or(at); + state.tokens = capacity; + } else { + state.tokens = raw; + state.last_refill = at; + } + state.tokens } /// One [`TokenBucket`] per distinct provider identity (`base_url`). @@ -385,4 +386,78 @@ mod tests { let result = bucket.take(); assert!(result.is_err(), "empty bucket should return Err"); } + + #[test] + fn future_instant_breaks_rate_limit() { + let bucket = TokenBucket::new(10); + let now = Instant::now(); + let one_hour_later = now + Duration::from_hours(1); + let one_min_later = now + Duration::from_mins(1); + + // Drain all 10 tokens at t=now. + for _ in 0..10 { + bucket.take_at(now).expect("burst tokens"); + } + + // Poison: take_at with a future instant refills and sets + // last_refill = one_hour_later. The bucket now thinks time is + // 1 hour ahead of reality. + let _poison = bucket.take_at(one_hour_later); + + // Drain any remaining tokens at the future instant. + for _ in 0..9 { + let _drain = bucket.take_at(one_hour_later); + } + + // Now 1 minute has passed in real time. Normally 1 min of refill + // would restore tokens. After the fix, take_at should succeed + // because the bucket must not allow a future instant to freeze + // the refill clock. + let result = bucket.take_at(one_min_later); + assert!( + result.is_ok(), + "take_at must succeed — 1 min of refill should restore a token; the future-instant call must not freeze the bucket" + ); + } + + #[test] + fn past_instant_does_not_refund_or_corrupt() { + // A take_at called with an instant earlier than last_refill must not + // fabricate tokens from negative elapsed time, and must not rewind the + // clock. The probe uses a small forward step so the partial refill is + // stable (a far-future step would re-fill on every call). + let bucket = TokenBucket::new(5); + let t0 = Instant::now(); + // 6s ≈ 0.5 token at capacity 5 (refill 5/60 per second). + let t1 = t0 + Duration::from_secs(6); + + // Drain at t0. + for _ in 0..5 { + bucket.take_at(t0).expect("drain while full"); + } + assert!(bucket.take_at(t0).is_err(), "drained at t0"); + + // Advance to t1: 6s of refill ≈ 0.5 token — still under one, take fails. + // last_refill is now t1, tokens ≈ 0.5. + assert!( + bucket.take_at(t1).is_err(), + "partial refill under one token" + ); + + // Probe: take at a past instant (t0 < last_refill = t1). + // Correct: saturating_duration_since returns ZERO → early return, no + // token granted, last_refill untouched. + assert!( + bucket.take_at(t0).is_err(), + "past-instant take must not refund tokens from negative elapsed time" + ); + + // Integrity: take_at(t1) again must observe the same 0.5-token state. + // A bug that rewound last_refill to t0 would make this see a fresh 6s + // gap (0.5 + 0.5 = 1.0 token) and succeed. + assert!( + bucket.take_at(t1).is_err(), + "last_refill must not rewind — bucket state unchanged by the past-instant probe" + ); + } } diff --git a/src/tool/shield.rs b/src/tool/shield.rs index 34cf438..1493db8 100644 --- a/src/tool/shield.rs +++ b/src/tool/shield.rs @@ -593,11 +593,20 @@ impl UnixShield { /// /// Both values are clamped to `[0.0, 1.0]` so an out-of-range /// configuration cannot produce a shield that never warns or never - /// blocks. The shield does not enforce `block >= warn`; passing an - /// inverted pair will produce surprising decisions, so callers - /// should validate their own inputs. + /// blocks. If `block < warn` (an inverted pair), the two values are + /// swapped and a warning is logged so the bands stay ordered. #[must_use] pub fn with_thresholds(mut self, warn: f32, block: f32) -> Self { + let (warn, block) = if block < warn { + tracing::warn!( + warn, + block, + "ToolShield block threshold < warn threshold; swapping to keep bands ordered" + ); + (block, warn) + } else { + (warn, block) + }; self.warn_threshold = warn.clamp(0.0, 1.0); self.block_threshold = block.clamp(0.0, 1.0); self @@ -1273,4 +1282,20 @@ mod tests { let block = SafetyDecision::block("test".into(), "cat"); assert!(block.is_blocked()); } + + #[test] + fn with_thresholds_swaps_inverted_pair() { + // Pass an inverted pair (warn > block). with_thresholds should swap + // them so the bands stay ordered: warn_threshold becomes the smaller + // value (0.4) and block_threshold becomes the larger (0.7). + let shield = UnixShield::new().with_thresholds(0.7, 0.4); + assert!( + (shield.warn_threshold - 0.4).abs() < f32::EPSILON, + "inverted pair should be swapped so warn is the smaller value" + ); + assert!( + (shield.block_threshold - 0.7).abs() < f32::EPSILON, + "inverted pair should be swapped so block is the larger value" + ); + } } From 0bd9289639c45bede24a555ec9833807a8896002 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Tue, 4 Aug 2026 16:31:59 +1200 Subject: [PATCH 2/4] fix: harden detection, rate-limit docs, and shield thresholds --- src/detection/loop_detector.rs | 19 ++++++++++++++ src/stream/rate_limit.rs | 6 ++++- src/tool/shield.rs | 48 +++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/detection/loop_detector.rs b/src/detection/loop_detector.rs index 1311dd8..42c3816 100644 --- a/src/detection/loop_detector.rs +++ b/src/detection/loop_detector.rs @@ -1594,6 +1594,11 @@ impl LoopDetector { } let normalized_op = sig.normalize_param_for_comparison(&o.tool, &o.primary_param); let normalized_query = sig.normalize_param_for_comparison(&o.tool, file_path); + + if normalized_op.is_empty() || normalized_query.is_empty() { + return false; + } + normalized_op.contains(normalized_query.as_str()) || normalized_query.contains(normalized_op.as_str()) }) @@ -2616,6 +2621,20 @@ mod tests { ); } + #[test] + fn check_file_reads_ignores_empty_primary_param() { + let config = LoopDetectorConfig { + max_same_file_reads: 1, + ..Default::default() + }; + let detector = LoopDetector::new(config, Arc::new(TestToolSignature)); + detector.record(Operation::new("Read", "")); + assert!( + !detector.check_file_reads("/etc/passwd"), + "an empty primary param must not match every query" + ); + } + #[test] fn window_size_zero_clamps_to_one() { let config = LoopDetectorConfig { diff --git a/src/stream/rate_limit.rs b/src/stream/rate_limit.rs index ffd1988..0465cd9 100644 --- a/src/stream/rate_limit.rs +++ b/src/stream/rate_limit.rs @@ -144,7 +144,11 @@ impl TokenBucket { /// Tokens available at a known instant (after a lazy refill). /// - /// Advances `last_refill` to `at`, banking the elapsed refill. + /// Applies the elapsed-time refill since the last call and returns the + /// resulting token count. When the refill reaches capacity before `at`, + /// `last_refill` advances only to the fill instant (the moment capacity + /// was hit), not to `at` — the excess time beyond capacity is irrelevant + /// and is discarded so a far-future `at` cannot freeze the refill clock. pub fn available_at(&self, at: Instant) -> f64 { let mut state = self .state diff --git a/src/tool/shield.rs b/src/tool/shield.rs index 1493db8..c16e59d 100644 --- a/src/tool/shield.rs +++ b/src/tool/shield.rs @@ -594,9 +594,27 @@ impl UnixShield { /// Both values are clamped to `[0.0, 1.0]` so an out-of-range /// configuration cannot produce a shield that never warns or never /// blocks. If `block < warn` (an inverted pair), the two values are - /// swapped and a warning is logged so the bands stay ordered. + /// swapped and a warning is logged so the bands stay ordered. A + /// non-finite value (NaN or infinity) is rejected and replaced with + /// that band's default — a stored NaN would silently disable the + /// band because every `score >= NaN` comparison is false. #[must_use] pub fn with_thresholds(mut self, warn: f32, block: f32) -> Self { + let warn = if warn.is_finite() { + warn + } else { + tracing::warn!(warn, "ToolShield warn threshold not finite; using default"); + self.warn_threshold + }; + let block = if block.is_finite() { + block + } else { + tracing::warn!( + block, + "ToolShield block threshold not finite; using default" + ); + self.block_threshold + }; let (warn, block) = if block < warn { tracing::warn!( warn, @@ -1298,4 +1316,32 @@ mod tests { "inverted pair should be swapped so block is the larger value" ); } + + #[test] + fn with_thresholds_rejects_non_finite() { + // NaN must not be stored: `score >= NaN` is always false, so a NaN + // threshold would silently disable the band. Infinity would be clamped + // to 1.0 by clamp(), but rejecting it up-front keeps the policy uniform. + // Both fall back to the band's default (warn 0.4, block 0.7). + let shield = UnixShield::new().with_thresholds(f32::NAN, f32::INFINITY); + assert!( + (shield.warn_threshold - 0.4).abs() < f32::EPSILON, + "NaN warn should fall back to the default" + ); + assert!( + (shield.block_threshold - 0.7).abs() < f32::EPSILON, + "infinite block should fall back to the default" + ); + + // A finite pair still routes through the normal clamp/swap path. + let shield = UnixShield::new().with_thresholds(0.2, 0.9); + assert!( + (shield.warn_threshold - 0.2).abs() < f32::EPSILON, + "finite warn should be stored (after clamp)" + ); + assert!( + (shield.block_threshold - 0.9).abs() < f32::EPSILON, + "finite block should be stored (after clamp)" + ); + } } From ad0f2eb253ce3738a950891cb1f58e927d881df4 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Tue, 4 Aug 2026 16:45:22 +1200 Subject: [PATCH 3/4] chore: 0.2.1 release prep --- CHANGELOG.md | 6 +++++- Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aa6462..b45e331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0. ## [Unreleased] +## [0.2.1] - 2026-08-04 + ### Fixed -- Detection-layer long-tail correctness (six items): `check_file_reads` now normalizes the query under the recorded op's real tool name (was: empty string) and matches by bidirectional path containment (was: exact equality); `LoopDetector::new` clamps `window_size == 0` to 1 with a warning; `find_best_match` rejects zero-score candidates even at threshold 0.0 and breaks ties lexicographically; `ToolShield::with_thresholds` swaps inverted warn/block pairs with a warning. All non-breaking. +- Detection-layer long-tail correctness (six items): `check_file_reads` now normalizes the query under the recorded op's real tool name (was: empty string) and matches by bidirectional path containment (was: exact equality), and rejects empty normalized params (an empty string is a substring of every query, which would inflate the read count); `LoopDetector::new` clamps `window_size == 0` to 1 with a warning; `find_best_match` rejects zero-score candidates even at threshold 0.0 and breaks ties lexicographically; `ToolShield::with_thresholds` swaps inverted warn/block pairs with a warning and rejects non-finite (NaN/inf) values, falling back to the band's default — a stored NaN would silently disable the band because every `score >= NaN` comparison is false. All non-breaking. - `SessionConfig::compact_threshold` now clamps into its documented `0..=100` range on the validating construction paths (`Default`, `with_compact_threshold`, and `Deserialize`). Previously only the `with_compact_threshold` builder clamped; deserialized configs could carry an out-of-range value (e.g. `200` from disk), which the compaction subsystem would then interpret as "never compact." A single canonical clamp method plus a field-level serde deserialize helper now enforce the range silently. `Default` was already in range (80); its clamp call is defensive. Direct public struct-literal construction (`SessionConfig { compact_threshold: 200, .. }`) still bypasses normalization — the field is `pub`, and callers using that form are responsible for honoring the documented range. Non-breaking (silent clamp; no signature changes). +- `TokenBucket` refill clock no longer jumps to a future instant. The shared `elapsed_refill` helper advances `last_refill` only to the fill point (the instant capacity is reached) instead of to `at`, so a caller that passes a far-future instant — directly or via `take`/`acquire`/`available` — can no longer freeze the refill clock on the production rate-limit path. Non-breaking (callers passing correct `Instant::now()` values see no change). - Corrected the `ParallelMode::Parallel` doc, which falsely claimed detection/observer side-effects "are not thread-safe" and fire "once on the final result only" in parallel mode. They are thread-safe (`DetectionManager` and the observer registry use `Mutex`/immutable-`Vec` interiors; `ToolHealthRegistry` uses atomics) and fire on every retry attempt in both modes, exactly as the code already does. No behavior change; the code matched the corrected doc all along. - Fixed code-level doc contradictions: the `ApiClient` trait example showed `request: StreamRequest` (by-value) instead of `&StreamRequest` (matches the real trait), and `BareLoop::machine` was described as an "empty placeholder" rather than the real "empty machine (no history, no pending messages)". - Reconciled the planning docs (ROADMAP, CONTEXT, ARCHITECTURE, README, DEPENDENCIES, DCH-DESIGN, the v0.2.0 release file) to the shipped 0.2.0 reality: status Planned→Shipped, `compact_threshold` u16→u8, `Loop::process_turn` soft-deprecated→removed, `LoopRuntime`/`LoopConfig`/`SessionResult`/`run_session` → their shipped replacements (`managers`/`SessionConfig`+`RunConfig`/`Run`+`Session`/`run`), MSRV 1.85→1.94, doctest count 303→286. Added a staleness banner to `LOOPCTL-DESIGN.md`. @@ -119,5 +122,6 @@ Initial crates.io release. - Built-in testing utilities for writing LLM loop tests - Example CLIs: hello, REPL, echo tool, and multi-provider chat +[0.2.1]: https://github.com/dch-labs/loopctl/releases/tag/v0.2.1 [0.2.0]: https://github.com/dch-labs/loopctl/releases/tag/v0.2.0 [0.1.0]: https://github.com/dch-labs/loopctl/releases/tag/v0.1.0 diff --git a/Cargo.toml b/Cargo.toml index c4e7a24..9d58394 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "loopctl" -version = "0.2.0" +version = "0.2.1" edition = "2024" license = "MIT OR Apache-2.0" description = "A trait-based framework for building agent loops with pluggable LLM clients, tools, and memory" From 4ad0ae8735f0263f7b5b4b7afc176fdd9de09b91 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Tue, 4 Aug 2026 17:02:13 +1200 Subject: [PATCH 4/4] chore: default warn/block thresholds for tool shield --- src/tool/shield.rs | 53 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/src/tool/shield.rs b/src/tool/shield.rs index c16e59d..16a9598 100644 --- a/src/tool/shield.rs +++ b/src/tool/shield.rs @@ -573,6 +573,12 @@ pub struct UnixShield { combination_rules: Vec, } +/// Default warn threshold: an aggregate score at or above this produces a warn. +const DEFAULT_WARN_THRESHOLD: f32 = 0.4; + +/// Default block threshold: an aggregate score at or above this produces a block. +const DEFAULT_BLOCK_THRESHOLD: f32 = 0.7; + impl UnixShield { /// Create a shield with default Unix shell patterns and thresholds. /// @@ -581,8 +587,8 @@ impl UnixShield { #[must_use] pub fn new() -> Self { Self { - warn_threshold: 0.4, - block_threshold: 0.7, + warn_threshold: DEFAULT_WARN_THRESHOLD, + block_threshold: DEFAULT_BLOCK_THRESHOLD, turn_history: Mutex::new(Vec::new()), patterns: Self::unix_patterns(), combination_rules: Self::unix_combination_rules(), @@ -604,7 +610,7 @@ impl UnixShield { warn } else { tracing::warn!(warn, "ToolShield warn threshold not finite; using default"); - self.warn_threshold + DEFAULT_WARN_THRESHOLD }; let block = if block.is_finite() { block @@ -613,7 +619,7 @@ impl UnixShield { block, "ToolShield block threshold not finite; using default" ); - self.block_threshold + DEFAULT_BLOCK_THRESHOLD }; let (warn, block) = if block < warn { tracing::warn!( @@ -982,8 +988,8 @@ impl UnixShieldBuilder { #[must_use] pub fn new() -> Self { Self { - warn_threshold: 0.4, - block_threshold: 0.7, + warn_threshold: DEFAULT_WARN_THRESHOLD, + block_threshold: DEFAULT_BLOCK_THRESHOLD, patterns: UnixShield::unix_patterns(), combination_rules: UnixShield::unix_combination_rules(), } @@ -996,8 +1002,8 @@ impl UnixShieldBuilder { #[must_use] pub fn blank() -> Self { Self { - warn_threshold: 0.4, - block_threshold: 0.7, + warn_threshold: DEFAULT_WARN_THRESHOLD, + block_threshold: DEFAULT_BLOCK_THRESHOLD, patterns: HashMap::new(), combination_rules: Vec::new(), } @@ -1344,4 +1350,35 @@ mod tests { "finite block should be stored (after clamp)" ); } + + #[test] + fn with_thresholds_chained_non_finite_restores_defaults() { + // After a finite customisation, a non-finite value must reset the band + // to the documented defaults (0.4 / 0.7), not inherit the previously + // stored custom value. + let shield = UnixShield::new() + .with_thresholds(0.1, 0.2) + .with_thresholds(f32::NAN, f32::INFINITY); + assert!( + (shield.warn_threshold - 0.4).abs() < f32::EPSILON, + "chained NaN warn should restore the default 0.4, not inherit 0.1" + ); + assert!( + (shield.block_threshold - 0.7).abs() < f32::EPSILON, + "chained infinite block should restore the default 0.7, not inherit 0.2" + ); + + // One bad band, one finite band: only the bad one resets. + let shield = UnixShield::new() + .with_thresholds(0.1, 0.2) + .with_thresholds(f32::NAN, 0.9); + assert!( + (shield.warn_threshold - 0.4).abs() < f32::EPSILON, + "NaN warn should reset to default while finite block is honoured" + ); + assert!( + (shield.block_threshold - 0.9).abs() < f32::EPSILON, + "finite block should be honoured alongside a NaN warn" + ); + } }