agentguard_core/decision/

cache.rs

//! LRU decision cache with TTL and policy-version invalidation.

use crate::decision::canonical::{canonical_json, write_canonical_value};
use crate::request::AgentRequest;
use crate::ttl::Clock;
use cedar_policy::Entities;
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::Arc;
use std::time::Duration;

/// Cache key derived from a request (and the current policy version).
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CacheKey(pub [u8; 32]);

impl CacheKey {
    /// Derive a key from an agent request without additional entities.
    /// Includes `policy_version` so a policy reload invalidates all entries.
    ///
    /// Streams each field's canonical JSON through the hasher, avoiding the
    /// intermediate `String` allocations the previous implementation made.
    /// For high-throughput PDPs (thousands of decisions/sec) this matters.
    ///
    /// # Examples
    /// ```
    /// use agentguard_core::{AgentRequestBuilder, Principal, AgentAction, Resource, AgentContext};
    /// use agentguard_core::decision::cache::CacheKey;
    /// let req = AgentRequestBuilder::new(Principal::user("alice"))
    ///     .action(AgentAction::tool("send_email"))
    ///     .resource(Resource::new("Mailbox", "alice@acme"))
    ///     .context(AgentContext::new())
    ///     .build()
    ///     .unwrap();
    /// let _k = CacheKey::for_request(&req, 0);
    /// ```
    pub fn for_request(req: &AgentRequest, policy_version: u64) -> Self {
        Self::for_request_with_entities(req, &Entities::empty(), policy_version)
    }

    /// Derive a key from an agent request and the complete Cedar entity set.
    /// Entity attributes and parent relationships can change a decision even
    /// when the visible request is identical, so they are part of the cache
    /// identity and are canonicalized in deterministic order.
    pub fn for_request_with_entities(
        req: &AgentRequest,
        entities: &Entities,
        policy_version: u64,
    ) -> Self {
        let mut hasher = Sha256::new();
        // Reusable buffer for canonical-JSON serialization. Capacity
        // 256 is a heuristic: the principal/action/resource/context
        // JSON blobs fit comfortably for typical requests; a deeper
        // path or many attributes will grow the Vec automatically.
        let mut buf: Vec<u8> = Vec::with_capacity(256);

        // Hash a length-prefixed JSON encoding of each component so
        // boundaries between fields can't be ambiguous.
        // Format: 4-byte big-endian length || canonical JSON.
        // Reusing `buf` across fields avoids 3-4 small Vec allocations
        // per cache-miss (the prior implementation allocated a fresh
        // Vec for every field).
        //
        // Security: every field is mandatory. If serialization fails
        // for any reason (today it's infallible for Principal /
        // Action / Resource / Context, but the serde_json::Value API
        // is fallible), we PANIC rather than produce a key that omits
        // the field — a key collision between two distinct requests
        // would leak Allow decisions across security boundaries.
        let mut hash_value = |h: &mut Sha256, value: &serde_json::Value| {
            buf.clear();
            write_canonical_value(&mut buf, value).expect("canonical write to Vec is infallible");
            let len = (buf.len() as u32).to_be_bytes();
            sha2::Digest::update(h, len);
            sha2::Digest::update(h, &buf);
        };

        let principal = serde_json::to_value(&req.principal)
            .expect("Principal is always serializable; see principal.rs round-trip tests");
        hash_value(&mut hasher, &principal);
        let action = serde_json::to_value(&req.action)
            .expect("AgentAction is always serializable; see action.rs round-trip tests");
        hash_value(&mut hasher, &action);
        let resource = serde_json::to_value(&req.resource)
            .expect("Resource is always serializable; see resource.rs round-trip tests");
        hash_value(&mut hasher, &resource);
        // Context is already canonical; hash it directly.
        let ctx_bytes = canonical_json(&req.context)
            .expect("AgentContext is always serializable; see context.rs round-trip tests");
        let ctx_len = (ctx_bytes.len() as u32).to_be_bytes();
        sha2::Digest::update(&mut hasher, ctx_len);
        sha2::Digest::update(&mut hasher, &ctx_bytes);
        let mut entity_values: Vec<Vec<u8>> = entities
            .iter()
            .map(|entity| {
                let value = entity
                    .to_json_value()
                    .expect("Cedar entities must be serializable for cache keys");
                canonical_json(&value).expect("Cedar entity JSON must be canonicalizable")
            })
            .collect();
        entity_values.sort();
        for entity in entity_values {
            let len = (entity.len() as u32).to_be_bytes();
            sha2::Digest::update(&mut hasher, len);
            sha2::Digest::update(&mut hasher, entity);
        }
        if let Some(t) = &req.trace {
            buf.clear();
            let s = t.to_string();
            let len = (s.len() as u32).to_be_bytes();
            sha2::Digest::update(&mut hasher, len);
            sha2::Digest::update(&mut hasher, s.as_bytes());
        }
        sha2::Digest::update(&mut hasher, policy_version.to_be_bytes());

        let hash: [u8; 32] = hasher.finalize().into();
        Self(hash)
    }

    pub fn as_hex(&self) -> String {
        hex::encode(self.0)
    }
}

/// A cached decision record.
///
/// The `effect` string is `"allow"` or `"deny"`. `cached_at_policy_version`
/// is the policy version that was active when this entry was inserted;
/// `get` ignores entries whose version doesn't match the current
/// `policy_version`, achieving invalidation on policy reload.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct CachedDecision {
    pub effect: String,
    pub policies: Vec<String>,
    pub reasons: Vec<String>,
    pub cached_at_policy_version: u64,
}

impl CachedDecision {
    /// Construct an `Allow` decision with the current policy version.
    pub fn allow() -> Self {
        Self {
            effect: "allow".into(),
            policies: vec![],
            reasons: vec![],
            cached_at_policy_version: 0,
        }
    }

    /// Construct a `Deny` decision with the current policy version.
    pub fn deny() -> Self {
        Self {
            effect: "deny".into(),
            policies: vec![],
            reasons: vec![],
            cached_at_policy_version: 0,
        }
    }
}

/// Configuration for [`DecisionCache`].
#[derive(Debug, Clone)]
pub struct CacheConfig {
    /// Maximum number of entries.
    pub capacity: usize,
    /// TTL for "allow" decisions.
    pub allow_ttl: Duration,
    /// TTL for "deny" decisions. Conservative (shorter) because deny flips are
    /// security-sensitive.
    pub deny_ttl: Duration,
    /// Whether to cache deny decisions at all.
    pub cache_denies: bool,
}

impl Default for CacheConfig {
    fn default() -> Self {
        Self {
            capacity: 10_000,
            allow_ttl: Duration::from_secs(60),
            deny_ttl: Duration::from_secs(5),
            cache_denies: true,
        }
    }
}

/// LRU + TTL decision cache.
///
/// Backed by a simple LRU map. Thread-safe via [`parking_lot::RwLock`]:
/// reads (the common case on a hot PDP) take a shared lock and run
/// concurrently; only `put()` (cache fill) takes the exclusive lock.
///
/// Note: a cache miss can still trigger a "thundering herd" where many
/// concurrent readers all see `None` for a popular key and all run the
/// underlying authorization. A single-flight pattern (per-key `OnceCell`)
/// would close this, but is left for a future enhancement.
pub struct DecisionCache {
    config: CacheConfig,
    clock: Arc<dyn Clock>,
    policy_version: AtomicU64,
    inner: parking_lot::RwLock<lru::LruCache<CacheKey, (CachedDecision, std::time::Instant)>>,
    hits: AtomicU64,
    misses: AtomicU64,
    evictions: AtomicU64,
}

impl DecisionCache {
    /// Construct a cache after validating its public configuration.
    ///
    /// Use this at application boundaries so invalid capacities produce a
    /// recoverable configuration error instead of a panic.
    pub fn try_new(
        config: CacheConfig,
        clock: Arc<dyn Clock>,
    ) -> std::result::Result<Self, String> {
        let capacity = std::num::NonZeroUsize::new(config.capacity)
            .ok_or_else(|| "cache capacity must be greater than zero".to_owned())?;
        if std::time::Instant::now()
            .checked_add(config.allow_ttl)
            .is_none()
        {
            return Err("allow cache TTL exceeds the monotonic clock range".to_owned());
        }
        if std::time::Instant::now()
            .checked_add(config.deny_ttl)
            .is_none()
        {
            return Err("deny cache TTL exceeds the monotonic clock range".to_owned());
        }
        Ok(Self {
            config,
            clock,
            policy_version: AtomicU64::new(0),
            inner: parking_lot::RwLock::new(lru::LruCache::new(capacity)),
            hits: AtomicU64::new(0),
            misses: AtomicU64::new(0),
            evictions: AtomicU64::new(0),
        })
    }

    /// Construct a cache with an already validated configuration.
    ///
    /// # Panics
    /// Panics when the capacity is zero or a TTL exceeds the monotonic clock
    /// range. Use [`Self::try_new`] when values come from application
    /// configuration or another fallible input.
    pub fn new(config: CacheConfig, clock: Arc<dyn Clock>) -> Self {
        Self::try_new(config, clock).expect("DecisionCache configuration must be valid")
    }

    /// A disabled cache (every call is a miss).
    pub fn disabled(clock: Arc<dyn Clock>) -> Self {
        let c = CacheConfig {
            capacity: 1,
            ..CacheConfig::default()
        };
        let cap = std::num::NonZeroUsize::new(c.capacity).unwrap();
        Self {
            config: c,
            clock,
            policy_version: AtomicU64::new(0),
            inner: parking_lot::RwLock::new(lru::LruCache::new(cap)),
            hits: AtomicU64::new(0),
            misses: AtomicU64::new(0),
            evictions: AtomicU64::new(0),
        }
    }

    pub fn policy_version(&self) -> u64 {
        self.policy_version.load(Ordering::Relaxed)
    }

    /// Bump the policy version. All cache entries are invalidated on the
    /// next `get()` because the stored `cached_at_policy_version` no longer
    /// matches.
    pub fn invalidate_all(&self) {
        self.policy_version.fetch_add(1, Ordering::Relaxed);
    }

    /// Convenience: set the policy version to a specific value.
    pub fn set_policy_version(&self, v: u64) {
        self.policy_version.store(v, Ordering::Relaxed);
    }

    pub fn get(&self, key: &CacheKey) -> Option<CachedDecision> {
        let now = self.clock.now();
        let policy_version = self.policy_version();
        // Read path takes a SHARED lock. Many concurrent readers can
        // coexist. Stale / expired entries are NOT evicted under the
        // read lock — instead, a separate [`sweep_stale`] call (run
        // from the policy watcher task) drains them so concurrent
        // readers do not serialize on the writer after a policy
        // reload.
        let guard = self.inner.read();
        if let Some((cached, expires_at)) = guard.peek(key).cloned() {
            if cached.cached_at_policy_version != policy_version {
                self.misses.fetch_add(1, Ordering::Relaxed);
                return None;
            }
            if now >= expires_at {
                self.misses.fetch_add(1, Ordering::Relaxed);
                return None;
            }
            // Touch for LRU recency (requires mut access; skip here
            // because we hold a shared lock — readers don't update
            // recency, which means hot reads can be evicted before
            // cold reads. Acceptable trade-off for the throughput
            // gain: the cache capacity is sized for working set, and
            // eviction is "approximately LRU" which is good enough for
            // a PDP).
            self.hits.fetch_add(1, Ordering::Relaxed);
            return Some(cached);
        }
        self.misses.fetch_add(1, Ordering::Relaxed);
        None
    }

    /// Drain stale (policy-version mismatch) and expired entries
    /// from the cache. Cheap when nothing is stale: O(1) under the
    /// fast path. Designed to be called from a background task or
    /// after [`Self::invalidate_all`].
    ///
    /// Returns the number of entries dropped. Used by tests to assert
    /// the eviction worked.
    pub fn sweep_stale(&self) -> usize {
        let now = self.clock.now();
        let policy_version = self.policy_version();
        let mut guard = self.inner.write();
        let before = guard.len();
        let stale: Vec<CacheKey> = guard
            .iter()
            .filter_map(|(k, (cached, expires_at))| {
                if cached.cached_at_policy_version != policy_version || now >= *expires_at {
                    Some(k.clone())
                } else {
                    None
                }
            })
            .collect();
        for k in &stale {
            guard.pop(k);
        }
        before.saturating_sub(guard.len())
    }

    pub fn put(&self, key: CacheKey, decision: CachedDecision) {
        let now = self.clock.now();
        let ttl = match decision.effect.as_str() {
            "allow" => self.config.allow_ttl,
            "deny" if self.config.cache_denies => self.config.deny_ttl,
            _ => return,
        };
        // Fill takes an EXCLUSIVE lock. This is rare relative to reads.
        let mut guard = self.inner.write();
        let prev = guard.push(key.clone(), (decision, now + ttl));
        // `lru::LruCache::push` returns `Some((old_k, old_v))` when the
        // key already existed (the old value is returned on refresh)
        // OR when a new key caused an LRU eviction (the evicted key
        // is returned). Distinguish by comparing the returned key
        // against the one we just pushed: an eviction has a different
        // key, a refresh returns the same one.
        if let Some((evicted_key, _)) = prev {
            if evicted_key != key {
                self.evictions.fetch_add(1, Ordering::Relaxed);
            }
        }
    }

    pub fn stats(&self) -> CacheStats {
        CacheStats {
            hits: self.hits.load(Ordering::Relaxed),
            misses: self.misses.load(Ordering::Relaxed),
            evictions: self.evictions.load(Ordering::Relaxed),
            size: self.inner.read().len(),
            policy_version: self.policy_version(),
        }
    }

    /// Build a `CacheConfig` from the `AGENTGUARD_CACHE_*` environment
    /// variables.
    ///
    /// Environment knobs (humantime where applicable):
    ///
    /// - `AGENTGUARD_CACHE_TTL` — sets `allow_ttl`. Does NOT touch
    ///   `deny_ttl` (use `AGENTGUARD_DENY_CACHE_TTL` for that).
    /// - `AGENTGUARD_DENY_CACHE_TTL` — sets `deny_ttl` independently.
    ///   Falls back to the default when unset.
    /// - `AGENTGUARD_CACHE_CAPACITY` — sets the LRU capacity (positive
    ///   integer).
    ///
    /// Falls back to defaults when unset or unparseable.
    #[deprecated(note = "use try_config_from_env to reject malformed settings")]
    pub fn config_from_env() -> CacheConfig {
        Self::config_from_overrides(
            std::env::var("AGENTGUARD_CACHE_TTL").ok(),
            std::env::var("AGENTGUARD_DENY_CACHE_TTL").ok(),
            std::env::var("AGENTGUARD_CACHE_CAPACITY").ok(),
        )
    }

    /// Parse cache settings from the process environment, rejecting malformed
    /// values rather than silently changing the requested operating limits.
    pub fn try_config_from_env() -> std::result::Result<CacheConfig, String> {
        fn read(name: &str) -> std::result::Result<Option<String>, String> {
            match std::env::var(name) {
                Ok(value) => Ok(Some(value)),
                Err(std::env::VarError::NotPresent) => Ok(None),
                Err(std::env::VarError::NotUnicode(_)) => {
                    Err(format!("{name} must be valid Unicode"))
                }
            }
        }
        Self::try_config_from_overrides(
            read("AGENTGUARD_CACHE_TTL")?.as_deref(),
            read("AGENTGUARD_DENY_CACHE_TTL")?.as_deref(),
            read("AGENTGUARD_CACHE_CAPACITY")?.as_deref(),
        )
    }

    /// Parse optional environment-style cache overrides strictly.
    pub fn try_config_from_overrides(
        cache_ttl: Option<&str>,
        deny_ttl: Option<&str>,
        capacity: Option<&str>,
    ) -> std::result::Result<CacheConfig, String> {
        let mut cfg = CacheConfig::default();
        if let Some(ttl) = cache_ttl {
            cfg.allow_ttl = crate::ttl::parse_duration(ttl)
                .map_err(|error| format!("AGENTGUARD_CACHE_TTL: {error}"))?;
        }
        if let Some(ttl) = deny_ttl {
            cfg.deny_ttl = crate::ttl::parse_duration(ttl)
                .map_err(|error| format!("AGENTGUARD_DENY_CACHE_TTL: {error}"))?;
        }
        if let Some(capacity) = capacity {
            cfg.capacity = capacity
                .parse::<usize>()
                .ok()
                .filter(|capacity| *capacity > 0)
                .ok_or_else(|| {
                    "AGENTGUARD_CACHE_CAPACITY must be a positive integer when set".to_owned()
                })?;
        }
        Ok(cfg)
    }

    /// Same as [`Self::config_from_env`] but accepts explicit
    /// overrides. Used by tests so they don't have to mutate
    /// `std::env` from `unsafe` blocks (the crate forbids unsafe).
    pub fn config_from_overrides(
        cache_ttl: Option<String>,
        deny_ttl: Option<String>,
        capacity: Option<String>,
    ) -> CacheConfig {
        let mut cfg = CacheConfig::default();
        if let Some(ttl) = cache_ttl {
            match crate::ttl::parse_duration(&ttl) {
                Ok(d) => cfg.allow_ttl = d,
                Err(_) => tracing::warn!(
                    ttl = %ttl,
                    "AGENTGUARD_CACHE_TTL is not a valid duration; using default"
                ),
            }
        }
        if let Some(ttl) = deny_ttl {
            match crate::ttl::parse_duration(&ttl) {
                Ok(d) => cfg.deny_ttl = d,
                Err(_) => tracing::warn!(
                    ttl = %ttl,
                    "AGENTGUARD_DENY_CACHE_TTL is not a valid duration; using default"
                ),
            }
        }
        if let Some(cap) = capacity {
            if let Ok(n) = cap.parse::<usize>() {
                if n > 0 {
                    cfg.capacity = n;
                }
            }
        }
        cfg
    }
}

#[derive(Debug, Clone, Copy)]
pub struct CacheStats {
    pub hits: u64,
    pub misses: u64,
    pub evictions: u64,
    pub size: usize,
    pub policy_version: u64,
}

impl CacheStats {
    pub fn hit_rate(&self) -> f64 {
        let total = self.hits + self.misses;
        if total == 0 {
            0.0
        } else {
            self.hits as f64 / total as f64
        }
    }
}

/// Test helper: convert a request + policy_version into a cache key.
#[cfg(test)]
fn cache_key_for(req: &AgentRequest, policy_version: u64) -> CacheKey {
    CacheKey::for_request(req, policy_version)
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::request::AgentRequestBuilder;
    use crate::ttl::MockClock;
    use crate::{AgentAction, AgentContext, Principal, Resource};
    use std::sync::Arc;

    fn req() -> AgentRequest {
        AgentRequestBuilder::new(Principal::user("alice"))
            .action(AgentAction::tool("send_email"))
            .resource(Resource::new("Mailbox", "alice@acme"))
            .context(AgentContext::new().with_arg("to", "[email protected]"))
            .build()
            .unwrap()
    }

    fn req_with_marker(marker: u64) -> AgentRequest {
        let principal_id = format!("alice-{marker}");
        let resource_id = format!("alice-{marker}@acme");
        AgentRequestBuilder::new(Principal::user(principal_id))
            .action(AgentAction::tool("send_email"))
            .resource(Resource::new("Mailbox", resource_id))
            .context(AgentContext::new().with_arg("to", "[email protected]"))
            .build()
            .unwrap()
    }

    #[test]
    fn try_new_rejects_zero_capacity_without_panicking() {
        let clock: Arc<dyn Clock> = Arc::new(MockClock::new());
        let config = CacheConfig {
            capacity: 0,
            ..CacheConfig::default()
        };
        let error = DecisionCache::try_new(config, clock)
            .err()
            .expect("zero capacity should return a configuration error");
        assert_eq!(error, "cache capacity must be greater than zero");
    }

    #[test]
    fn try_new_rejects_ttl_outside_monotonic_clock_range() {
        let clock: Arc<dyn Clock> = Arc::new(MockClock::new());
        let config = CacheConfig {
            allow_ttl: Duration::MAX,
            ..CacheConfig::default()
        };
        let error = DecisionCache::try_new(config, clock)
            .err()
            .expect("unrepresentable TTL should return a configuration error");
        assert_eq!(error, "allow cache TTL exceeds the monotonic clock range");
    }

    #[test]
    fn cache_miss_then_hit() {
        let clock = Arc::new(MockClock::new());
        let cache = DecisionCache::new(CacheConfig::default(), clock.clone());
        let key = CacheKey::for_request(&req(), 0);
        assert!(cache.get(&key).is_none());
        cache.put(key.clone(), CachedDecision::allow());
        let got = cache.get(&key).unwrap();
        assert_eq!(got.effect, "allow");
        assert_eq!(cache.stats().hits, 1);
        assert_eq!(cache.stats().misses, 1);
    }

    #[test]
    fn entity_attributes_change_cache_identity() {
        let entity = cedar_policy::Entity::from_json_str(
            r#"{"uid":{"type":"User","id":"alice"},"attrs":{"admin":true},"parents":[]}"#,
            None,
        )
        .unwrap();
        let entities = cedar_policy::Entities::from_entities([entity], None).unwrap();
        assert_ne!(
            CacheKey::for_request(&req(), 0),
            CacheKey::for_request_with_entities(&req(), &entities, 0)
        );
    }

    #[test]
    #[allow(clippy::field_reassign_with_default)]
    fn expire_after_ttl() {
        let clock = Arc::new(MockClock::new());
        let mut cfg = CacheConfig::default();
        cfg.allow_ttl = Duration::from_secs(5);
        let cache = DecisionCache::new(cfg, clock.clone());
        let key = CacheKey::for_request(&req(), 0);
        cache.put(key.clone(), CachedDecision::allow());
        clock.advance_unix(Duration::from_secs(10));
        assert!(cache.get(&key).is_none(), "should have expired");
    }

    #[test]
    fn invalidate_on_policy_version_bump() {
        let clock = Arc::new(MockClock::new());
        let cache = DecisionCache::new(CacheConfig::default(), clock.clone());
        let key = CacheKey::for_request(&req(), 0);
        cache.put(key.clone(), CachedDecision::allow());
        assert!(cache.get(&key).is_some());
        cache.invalidate_all();
        assert!(cache.get(&key).is_none(), "policy bump should invalidate");
    }

    #[test]
    fn deny_cached_when_enabled() {
        let clock = Arc::new(MockClock::new());
        let cache = DecisionCache::new(CacheConfig::default(), clock.clone());
        let key = CacheKey::for_request(&req(), 0);
        cache.put(key.clone(), CachedDecision::deny());
        assert_eq!(cache.get(&key).unwrap().effect, "deny");
    }

    #[test]
    #[allow(clippy::field_reassign_with_default)]
    fn deny_not_cached_when_disabled() {
        let clock = Arc::new(MockClock::new());
        let mut cfg = CacheConfig::default();
        cfg.cache_denies = false;
        let cache = DecisionCache::new(cfg, clock.clone());
        let key = CacheKey::for_request(&req(), 0);
        cache.put(key.clone(), CachedDecision::deny());
        assert!(cache.get(&key).is_none(), "denies not cached");
    }

    #[test]
    fn hit_rate_calculation() {
        let s = CacheStats {
            hits: 7,
            misses: 3,
            evictions: 0,
            size: 5,
            policy_version: 0,
        };
        assert!((s.hit_rate() - 0.7).abs() < 1e-9);
    }

    #[test]
    fn concurrent_reads_dont_block_writers() {
        use std::sync::Arc;
        use std::sync::Barrier;
        use std::thread;

        let clock = Arc::new(MockClock::new());
        let cache = Arc::new(DecisionCache::new(CacheConfig::default(), clock));
        let key = cache_key_for(&req(), 0);

        // Seed the cache so reads hit.
        cache.put(key.clone(), CachedDecision::allow());

        // ponytail: deterministic Barrier replaces the previous
        // thread::yield_now() (which was timing-sensitive). Each
        // reader runs a small, bounded number of gets so progress
        // is guaranteed on any scheduler.
        const N_READERS: usize = 4;
        const LOOPS_PER_READER: usize = 50;
        let ready = Arc::new(Barrier::new(N_READERS + 1));
        let mut handles = Vec::new();
        for _ in 0..N_READERS {
            let cache = Arc::clone(&cache);
            let key = key.clone();
            let ready = Arc::clone(&ready);
            handles.push(thread::spawn(move || {
                ready.wait();
                for _ in 0..LOOPS_PER_READER {
                    let _ = cache.get(&key);
                }
            }));
        }

        // Wait for all readers to be in their loop before writing.
        ready.wait();
        for v in 1..=5 {
            cache.set_policy_version(v);
            cache.put(key.clone(), CachedDecision::allow());
        }
        for h in handles {
            h.join().unwrap();
        }
        // The test's purpose is to confirm no deadlock. h.join().unwrap()
        // returning for all 4 readers proves the RwLock works correctly
        // under concurrent reader/writer traffic. We intentionally
        // don't assert on hits/misses counts — those are racy and
        // a small number of policy_version bumps can race every
        // reader to a miss.
    }

    #[test]
    fn eviction_counter_increments() {
        let clock = Arc::new(MockClock::new());
        let cfg = CacheConfig {
            capacity: 2,
            ..CacheConfig::default()
        };
        let cache = DecisionCache::new(cfg, clock.clone());

        for i in 0..4u64 {
            let key = CacheKey::for_request(&req_with_marker(i), i);
            cache.put(key, CachedDecision::allow());
        }

        assert!(
            cache.stats().evictions >= 2,
            "evictions counter should reflect LRU churn, got {}",
            cache.stats().evictions
        );
    }

    #[test]
    fn concurrent_eviction_throughput() {
        // Stress test for the read-then-evict contention path: many
        // readers simultaneously observe stale entries after a
        // policy reload. With the old implementation they serialized
        // on the writer; the new implementation defers eviction to
        // sweep_stale and lets readers return None without taking a
        // write lock.
        use std::sync::Barrier;
        let clock = Arc::new(MockClock::new());
        let cache = Arc::new(DecisionCache::new(CacheConfig::default(), clock.clone()));

        let key = CacheKey::for_request(&req(), 0);
        cache.put(key.clone(), CachedDecision::allow());
        cache.invalidate_all();

        let barrier = Arc::new(Barrier::new(16));
        let mut handles = Vec::new();
        for _ in 0..16 {
            let cache = cache.clone();
            let key = key.clone();
            let barrier = barrier.clone();
            handles.push(std::thread::spawn(move || {
                barrier.wait();
                for _ in 0..1000 {
                    assert!(cache.get(&key).is_none());
                }
            }));
        }
        for h in handles {
            h.join().unwrap();
        }
        // Drain any stragglers via the new background sweep.
        let drained = cache.sweep_stale();
        assert!(
            drained >= 1,
            "sweep_stale must have evicted the stale entry, got {drained}"
        );
    }

    #[test]
    fn config_from_overrides_sets_both_independently() {
        // AGENTGUARD_CACHE_TTL should set allow_ttl only;
        // AGENTGUARD_DENY_CACHE_TTL sets deny_ttl independently.
        // Neither silently caps the other.
        let cfg = DecisionCache::config_from_overrides(
            Some("600s".to_string()),
            Some("2s".to_string()),
            None,
        );
        assert_eq!(cfg.allow_ttl, Duration::from_secs(600));
        assert_eq!(cfg.deny_ttl, Duration::from_secs(2));
    }

    #[test]
    fn strict_config_parser_rejects_invalid_ttls_and_capacity() {
        assert!(
            DecisionCache::try_config_from_overrides(Some("soon"), None, None)
                .unwrap_err()
                .contains("AGENTGUARD_CACHE_TTL")
        );
        assert!(
            DecisionCache::try_config_from_overrides(None, Some("soon"), None)
                .unwrap_err()
                .contains("AGENTGUARD_DENY_CACHE_TTL")
        );
        for capacity in ["0", "-1", "many"] {
            assert!(DecisionCache::try_config_from_overrides(None, None, Some(capacity)).is_err());
        }
    }

    #[test]
    fn strict_config_parser_applies_explicit_overrides() {
        let cfg =
            DecisionCache::try_config_from_overrides(Some("600s"), Some("2s"), Some("42")).unwrap();
        assert_eq!(cfg.allow_ttl, Duration::from_secs(600));
        assert_eq!(cfg.deny_ttl, Duration::from_secs(2));
        assert_eq!(cfg.capacity, 42);
    }
}