arch-toolkit 0.2.0

//! HTTP client with rate limiting for arch-toolkit.

#[cfg(feature = "aur")]
use std::sync::{LazyLock, Mutex};
#[cfg(feature = "aur")]
use std::time::{Duration, Instant};

#[cfg(feature = "aur")]
use rand::Rng;
#[cfg(feature = "aur")]
use tracing::{debug, warn};

#[cfg(feature = "aur")]
use crate::aur::validation::ValidationConfig;
#[cfg(feature = "aur")]
use crate::cache::{CacheConfig, CacheWrapper};
#[cfg(feature = "aur")]
use crate::env;
#[cfg(feature = "aur")]
use crate::error::{ArchToolkitError, Result};
#[cfg(feature = "aur")]
use reqwest::Client as ReqwestClient;

#[cfg(feature = "aur")]
/// Rate limiter state for archlinux.org with exponential backoff.
struct ArchLinuxRateLimiter {
    /// Last request timestamp.
    last_request: Instant,
    /// Current backoff delay in milliseconds (starts at base delay, increases exponentially).
    current_backoff_ms: u64,
    /// Number of consecutive failures/rate limits.
    consecutive_failures: u32,
}

#[cfg(feature = "aur")]
/// Rate limiter for archlinux.org requests with exponential backoff.
/// Tracks last request time and implements progressive delays on failures.
static ARCHLINUX_RATE_LIMITER: LazyLock<Mutex<ArchLinuxRateLimiter>> = LazyLock::new(|| {
    Mutex::new(ArchLinuxRateLimiter {
        last_request: Instant::now(),
        current_backoff_ms: 500, // Start with 500ms base delay
        consecutive_failures: 0,
    })
});

#[cfg(feature = "aur")]
/// Semaphore to serialize archlinux.org requests (only 1 concurrent request allowed).
/// This prevents multiple async tasks from overwhelming the server even when rate limiting
/// is applied, because the rate limiter alone doesn't prevent concurrent requests that
/// start at nearly the same time from all proceeding simultaneously.
static ARCHLINUX_REQUEST_SEMAPHORE: LazyLock<std::sync::Arc<tokio::sync::Semaphore>> =
    LazyLock::new(|| std::sync::Arc::new(tokio::sync::Semaphore::new(1)));

#[cfg(feature = "aur")]
/// Base delay for archlinux.org requests (500ms).
const ARCHLINUX_BASE_DELAY_MS: u64 = 500;
#[cfg(feature = "aur")]
/// Maximum backoff delay (60 seconds).
const ARCHLINUX_MAX_BACKOFF_MS: u64 = 60_000;
#[cfg(feature = "aur")]
/// Maximum jitter in milliseconds to add to rate limiting delays (prevents thundering herd).
const JITTER_MAX_MS: u64 = 500;

/// What: Apply rate limiting specifically for archlinux.org requests with exponential backoff.
///
/// Inputs: None
///
/// Output: `OwnedSemaphorePermit` that the caller MUST hold during the request.
///
/// # Panics
/// - Panics if the archlinux.org request semaphore is closed (should never happen in practice).
///
/// Details:
/// - Acquires a semaphore permit to serialize archlinux.org requests (only 1 at a time).
/// - Uses base delay (500ms) for archlinux.org to reduce request frequency.
/// - Implements exponential backoff: increases delay on consecutive failures (500ms → 1s → 2s → 4s, max 60s).
/// - Adds random jitter (0-500ms) to prevent thundering herd when multiple clients retry simultaneously.
/// - Resets backoff after successful requests.
/// - Thread-safe via mutex guarding the rate limiter state.
/// - The returned permit MUST be held until the HTTP request completes to ensure serialization.
#[cfg(feature = "aur")]
pub async fn rate_limit_archlinux() -> tokio::sync::OwnedSemaphorePermit {
    // 1. Acquire semaphore to serialize requests (waits if another request is in progress)
    let permit = ARCHLINUX_REQUEST_SEMAPHORE
        .clone()
        .acquire_owned()
        .await
        .expect("archlinux.org request semaphore should never be closed");

    // 2. Now that we have exclusive access, compute and apply the rate limiting delay
    let delay_needed = {
        let mut limiter = match ARCHLINUX_RATE_LIMITER.lock() {
            Ok(guard) => guard,
            Err(poisoned) => poisoned.into_inner(),
        };
        let elapsed = limiter.last_request.elapsed();
        let min_delay = Duration::from_millis(limiter.current_backoff_ms);
        let delay = if elapsed < min_delay {
            min_delay.checked_sub(elapsed).unwrap_or(Duration::ZERO)
        } else {
            Duration::ZERO
        };
        limiter.last_request = Instant::now();
        delay
    };

    if !delay_needed.is_zero() {
        // Add random jitter to prevent thundering herd when multiple clients retry simultaneously
        let jitter_ms = rand::rng().random_range(0..=JITTER_MAX_MS);
        let delay_with_jitter = delay_needed + Duration::from_millis(jitter_ms);
        #[allow(clippy::cast_possible_truncation)] // Delay will be small (max 60s = 60000ms)
        let delay_ms = delay_needed.as_millis() as u64;
        debug!(
            delay_ms,
            jitter_ms,
            total_ms = delay_with_jitter.as_millis(),
            "rate limiting archlinux.org request with jitter"
        );
        tokio::time::sleep(delay_with_jitter).await;
    }

    // 3. Return the permit - caller MUST hold it during the request
    permit
}

/// What: Increase backoff delay for archlinux.org after a failure or rate limit.
///
/// Inputs:
/// - `retry_after_seconds`: Optional retry-after value from server (in seconds).
///
/// Output: None
///
/// Details:
/// - If `retry_after_seconds` is provided, uses that value (capped at maximum).
/// - Otherwise, doubles the current backoff delay (exponential backoff).
/// - Caps backoff at maximum delay (60 seconds).
/// - Increments consecutive failure counter.
#[cfg(feature = "aur")]
pub fn increase_archlinux_backoff(retry_after_seconds: Option<u64>) {
    let mut limiter = match ARCHLINUX_RATE_LIMITER.lock() {
        Ok(guard) => guard,
        Err(poisoned) => poisoned.into_inner(),
    };
    limiter.consecutive_failures += 1;
    // Use Retry-After value if provided, otherwise use exponential backoff
    if let Some(retry_after) = retry_after_seconds {
        // Convert seconds to milliseconds, cap at maximum
        let retry_after_ms = (retry_after * 1000).min(ARCHLINUX_MAX_BACKOFF_MS);
        limiter.current_backoff_ms = retry_after_ms;
        warn!(
            consecutive_failures = limiter.consecutive_failures,
            retry_after_seconds = retry_after,
            backoff_ms = limiter.current_backoff_ms,
            "increased archlinux.org backoff delay using Retry-After header"
        );
    } else {
        // Double the backoff delay, capped at maximum
        limiter.current_backoff_ms = (limiter.current_backoff_ms * 2).min(ARCHLINUX_MAX_BACKOFF_MS);
        warn!(
            consecutive_failures = limiter.consecutive_failures,
            backoff_ms = limiter.current_backoff_ms,
            "increased archlinux.org backoff delay"
        );
    }
}

/// What: Reset backoff delay for archlinux.org after a successful request.
///
/// Inputs: None
///
/// Output: None
///
/// Details:
/// - Resets backoff to base delay (500ms).
/// - Resets consecutive failure counter.
#[cfg(feature = "aur")]
pub fn reset_archlinux_backoff() {
    let mut limiter = match ARCHLINUX_RATE_LIMITER.lock() {
        Ok(guard) => guard,
        Err(poisoned) => poisoned.into_inner(),
    };
    if limiter.consecutive_failures > 0 {
        debug!(
            previous_failures = limiter.consecutive_failures,
            previous_backoff_ms = limiter.current_backoff_ms,
            "resetting archlinux.org backoff after successful request"
        );
    }
    limiter.current_backoff_ms = ARCHLINUX_BASE_DELAY_MS;
    limiter.consecutive_failures = 0;
}

/// What: Check if a URL belongs to archlinux.org domain.
///
/// Inputs:
/// - `url`: URL string to check.
///
/// Output:
/// - `true` if URL is from archlinux.org, `false` otherwise.
///
/// Details:
/// - Checks if URL contains "archlinux.org" domain.
#[cfg(feature = "aur")]
#[must_use]
pub fn is_archlinux_url(url: &str) -> bool {
    url.contains("archlinux.org")
}

/// What: Determine if an error is retryable and extract retry-after information.
///
/// Inputs:
/// - `error`: The `reqwest::Error` to classify
///
/// Output:
/// - `(bool, Option<u64>)` where the bool indicates if the error is retryable,
///   and the Option contains retry-after seconds if available from headers
///
/// Details:
/// - Timeout errors are retryable
/// - Connection errors are retryable
/// - HTTP 5xx status codes are retryable
/// - HTTP 429 (rate limit) is retryable and may include Retry-After header
/// - HTTP 4xx (except 429) are not retryable (client errors)
/// - HTTP 3xx are not retryable (redirects handled by reqwest)
#[cfg(feature = "aur")]
#[must_use]
pub fn is_retryable_error(error: &reqwest::Error) -> (bool, Option<u64>) {
    // Check for timeout errors
    if error.is_timeout() {
        return (true, None);
    }

    // Check for connection errors
    if error.is_connect() || error.is_request() {
        return (true, None);
    }

    // Check HTTP status code
    if let Some(status) = error.status() {
        let code = status.as_u16();

        // 5xx server errors are retryable
        if (500..600).contains(&code) {
            return (true, None);
        }

        // 429 Too Many Requests is retryable
        // Note: Retry-After header extraction must be done from the response,
        // not from the error. The caller should check response headers separately.
        if code == 429 {
            return (true, None);
        }

        // 4xx client errors (except 429) are not retryable
        if (400..500).contains(&code) {
            return (false, None);
        }

        // 3xx redirects should be handled by reqwest, not retryable
        if (300..400).contains(&code) {
            return (false, None);
        }
    }

    // Default: not retryable
    (false, None)
}

/// What: Extract Retry-After header value from HTTP response.
///
/// Inputs:
/// - `response`: The HTTP response to check for Retry-After header
///
/// Output:
/// - `Option<u64>` containing retry-after seconds if header is present and valid
///
/// Details:
/// - Parses Retry-After header which can be either seconds (u64) or HTTP date
/// - Currently only supports seconds format for simplicity
/// - Returns None if header is missing or invalid
#[cfg(feature = "aur")]
#[must_use]
pub fn extract_retry_after(response: &reqwest::Response) -> Option<u64> {
    response
        .headers()
        .get(reqwest::header::RETRY_AFTER)
        .and_then(|value| value.to_str().ok())
        .and_then(|s| s.parse::<u64>().ok())
}

/// What: Retry an operation with exponential backoff and jitter.
///
/// Inputs:
/// - `policy`: Retry policy configuration
/// - `operation_name`: Name of the operation for logging
/// - `context`: Operation context (query/package name) for error messages
/// - `operation`: Async closure that performs the operation and returns `Result<T>`
///
/// Output:
/// - `Result<T>` from the operation, or the last error after all retries exhausted
///
/// Details:
/// - Implements exponential backoff: `delay = min(initial_delay * 2^attempt, max_delay)`
/// - Adds random jitter: `final_delay = delay + random(0..=jitter_max)`
/// - Respects Retry-After header when available from response errors
/// - Logs retry attempts with tracing
/// - Returns immediately on success or non-retryable errors
/// - Preserves operation context in error messages
///
/// # Errors
/// - Returns context-specific errors (`SearchFailed`, `InfoFailed`, etc.) with preserved context
/// - Returns `Err(ArchToolkitError::Parse)` for non-retryable errors
#[cfg(feature = "aur")]
pub async fn retry_with_policy<F, Fut, T>(
    policy: &RetryPolicy,
    operation_name: &str,
    context: &str,
    mut operation: F,
) -> Result<T>
where
    F: FnMut() -> Fut,
    Fut: std::future::Future<Output = Result<T>>,
{
    if !policy.enabled {
        return operation().await;
    }

    let mut last_error: Option<ArchToolkitError> = None;
    let mut retry_after_seconds: Option<u64> = None;

    for attempt in 0..=policy.max_retries {
        let result = operation().await;

        match result {
            Ok(value) => {
                if attempt > 0 {
                    debug!(
                        operation = operation_name,
                        context = %context,
                        attempt = attempt + 1,
                        "operation succeeded after retries"
                    );
                }
                return Ok(value);
            }
            Err(
                ArchToolkitError::Network(ref e)
                | ArchToolkitError::SearchFailed { source: ref e, .. }
                | ArchToolkitError::InfoFailed { source: ref e, .. }
                | ArchToolkitError::CommentsFailed { source: ref e, .. }
                | ArchToolkitError::PkgbuildFailed { source: ref e, .. },
            ) => {
                let (is_retryable, _) = is_retryable_error(e);

                // Extract the error for reuse
                let Err(error) = result else {
                    unreachable!();
                };

                if !is_retryable {
                    // Non-retryable error, return immediately with preserved context
                    return Err(error);
                }

                // Check if we've exhausted retries
                if attempt >= policy.max_retries {
                    warn!(
                        operation = operation_name,
                        context = %context,
                        max_retries = policy.max_retries,
                        "max retries exhausted"
                    );
                    // Return the last error with preserved context
                    return Err(error);
                }

                // Store the error for potential retry
                last_error = Some(error);

                // Calculate delay with exponential backoff
                let base_delay_ms = retry_after_seconds.map_or_else(
                    || {
                        // Exponential backoff: initial_delay * 2^attempt
                        let delay = policy.initial_delay_ms * (1u64 << attempt.min(20)); // Cap at 2^20 to prevent overflow
                        delay.min(policy.max_delay_ms)
                    },
                    |retry_after| {
                        // Use Retry-After value if available, convert to milliseconds
                        (retry_after * 1000).min(policy.max_delay_ms)
                    },
                );

                // Add jitter
                let jitter_ms = rand::rng().random_range(0..=policy.jitter_max_ms);
                let total_delay_ms = base_delay_ms + jitter_ms;
                let delay = Duration::from_millis(total_delay_ms);

                warn!(
                    operation = operation_name,
                    context = %context,
                    attempt = attempt + 1,
                    max_retries = policy.max_retries,
                    delay_ms = total_delay_ms,
                    base_delay_ms,
                    jitter_ms,
                    "retrying operation after error"
                );

                tokio::time::sleep(delay).await;
                retry_after_seconds = None; // Reset after using it
            }
            Err(e) => {
                // Non-network errors are not retryable
                return Err(e);
            }
        }
    }

    // This should never be reached, but handle it gracefully
    Err(last_error.unwrap_or_else(|| {
        ArchToolkitError::Parse(format!(
            "retry exhausted without error for {operation_name} (context: {context})"
        ))
    }))
}

// ============================================================================
// ArchClient and Builder
// ============================================================================

#[cfg(feature = "aur")]
/// Default timeout for HTTP requests (30 seconds).
const DEFAULT_TIMEOUT_SECS: u64 = 30;

#[cfg(feature = "aur")]
/// Default user agent string.
const DEFAULT_USER_AGENT: &str = "arch-toolkit/0.1.0";

#[cfg(feature = "aur")]
/// Default health check timeout (5 seconds).
const DEFAULT_HEALTH_CHECK_TIMEOUT_SECS: u64 = 5;

// ============================================================================
// Retry Policy
// ============================================================================

/// What: Configuration for retry policies with exponential backoff and jitter.
///
/// Inputs: None (created via `RetryPolicy::default()` or builder methods)
///
/// Output: `RetryPolicy` instance with configurable retry behavior
///
/// Details:
/// - Controls retry behavior for transient network failures
/// - Supports per-operation-type configuration
/// - Uses exponential backoff with jitter to prevent thundering herd
/// - Can be disabled globally or per operation
#[cfg(feature = "aur")]
#[derive(Debug, Clone)]
#[allow(clippy::struct_excessive_bools)] // Per-operation flags are necessary for fine-grained control
pub struct RetryPolicy {
    /// Maximum number of retry attempts (default: 3).
    pub max_retries: u32,
    /// Initial delay in milliseconds before first retry (default: 1000).
    pub initial_delay_ms: u64,
    /// Maximum delay in milliseconds (default: 30000).
    pub max_delay_ms: u64,
    /// Maximum jitter in milliseconds to add to delays (default: 500).
    pub jitter_max_ms: u64,
    /// Whether retries are enabled globally (default: true).
    pub enabled: bool,
    /// Whether to retry search operations (default: true).
    pub retry_search: bool,
    /// Whether to retry info operations (default: true).
    pub retry_info: bool,
    /// Whether to retry comments operations (default: true).
    pub retry_comments: bool,
    /// Whether to retry pkgbuild operations (default: true).
    pub retry_pkgbuild: bool,
}

#[cfg(feature = "aur")]
impl Default for RetryPolicy {
    fn default() -> Self {
        Self {
            max_retries: 3,
            initial_delay_ms: 1000,
            max_delay_ms: 30_000,
            jitter_max_ms: 500,
            enabled: true,
            retry_search: true,
            retry_info: true,
            retry_comments: true,
            retry_pkgbuild: true,
        }
    }
}

/// What: Main client for arch-toolkit operations.
///
/// Inputs: None (created via `new()` or `builder()`)
///
/// Output: `ArchClient` instance ready for use
///
/// Details:
/// - Wraps `reqwest::Client` with arch-toolkit-specific configuration
/// - Provides access to AUR operations via `aur()` method
/// - Handles rate limiting automatically
/// - Configurable via `ArchClientBuilder`
#[cfg(feature = "aur")]
#[derive(Debug)]
pub struct ArchClient {
    /// Internal HTTP client.
    http_client: ReqwestClient,
    /// User agent string for requests (stored for debugging/future use).
    #[allow(dead_code)]
    // Stored for potential future features (e.g., configuration inspection)
    user_agent: String,
    /// Request timeout (stored for debugging/future use).
    #[allow(dead_code)]
    // Stored for potential future features (e.g., configuration inspection)
    timeout: Duration,
    /// Retry policy configuration.
    retry_policy: RetryPolicy,
    /// Optional cache wrapper.
    cache: Option<CacheWrapper>,
    /// Cache configuration (stored for cache invalidation).
    cache_config: Option<CacheConfig>,
    /// Validation configuration.
    validation_config: ValidationConfig,
    /// Health check timeout (default: 5 seconds).
    health_check_timeout: Duration,
}

#[cfg(feature = "aur")]
impl ArchClient {
    /// What: Create a new `ArchClient` with default configuration.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Result<ArchClient>` with default settings, or error if client creation fails
    ///
    /// Details:
    /// - Default timeout: 30 seconds
    /// - Default user agent: `arch-toolkit/{version}`
    /// - Uses existing rate limiting (500ms base delay)
    ///
    /// # Errors
    /// - Returns `Err(ArchToolkitError::Network)` if `reqwest::Client` creation fails
    pub fn new() -> Result<Self> {
        Self::builder().build()
    }

    /// What: Create a builder for custom `ArchClient` configuration.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `ArchClientBuilder` with default values that can be customized
    ///
    /// Details:
    /// - Starts with sensible defaults
    /// - Use builder methods to customize timeout, user agent, etc.
    /// - Call `build()` to create the `ArchClient`
    #[must_use]
    pub const fn builder() -> ArchClientBuilder {
        ArchClientBuilder::new()
    }

    /// What: Get access to AUR operations.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Aur` wrapper that provides AUR-specific methods
    ///
    /// Details:
    /// - Returns a reference wrapper that provides `search()`, `info()`, `comments()`, `pkgbuild()` methods
    /// - The `Aur` wrapper uses this client's HTTP client and configuration
    #[must_use]
    pub const fn aur(&self) -> crate::aur::Aur<'_> {
        crate::aur::Aur::new(self)
    }

    /// What: Get the internal HTTP client (for internal use).
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - Reference to the internal `reqwest::Client`
    ///
    /// Details:
    /// - Used internally by AUR operations
    /// - Not part of public API
    pub(crate) const fn http_client(&self) -> &ReqwestClient {
        &self.http_client
    }

    /// What: Get the retry policy (for internal use).
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - Reference to the retry policy
    ///
    /// Details:
    /// - Used internally by AUR operations
    /// - Not part of public API
    pub(crate) const fn retry_policy(&self) -> &RetryPolicy {
        &self.retry_policy
    }

    /// What: Get the cache wrapper (for internal use).
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Option<&CacheWrapper>` containing cache if enabled, `None` otherwise
    ///
    /// Details:
    /// - Used internally by AUR operations
    /// - Returns `None` if caching is not enabled
    pub(crate) const fn cache(&self) -> Option<&CacheWrapper> {
        self.cache.as_ref()
    }

    /// What: Get the cache configuration (for internal use).
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Option<&CacheConfig>` containing cache config if set, `None` otherwise
    ///
    /// Details:
    /// - Used internally by AUR operations to check per-operation cache settings
    pub(crate) const fn cache_config(&self) -> Option<&CacheConfig> {
        self.cache_config.as_ref()
    }

    /// What: Get the validation configuration (for internal use).
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - Reference to the validation configuration
    ///
    /// Details:
    /// - Used internally by AUR operations to validate inputs
    pub(crate) const fn validation_config(&self) -> &ValidationConfig {
        &self.validation_config
    }

    /// What: Invalidate cache entries.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `CacheInvalidator` builder for cache invalidation operations
    ///
    /// Details:
    /// - Returns a builder that allows invalidating specific cache entries
    /// - Returns a no-op builder if caching is not enabled
    #[must_use]
    pub const fn invalidate_cache(&self) -> CacheInvalidator<'_> {
        CacheInvalidator::new(self)
    }

    /// What: Quick connectivity check for archlinux.org services.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Result<bool>` - `true` if services are operational, `false` or error otherwise
    ///
    /// Details:
    /// - Performs lightweight HTTP request to AUR RPC API
    /// - Uses shorter timeout than regular operations (5s default)
    /// - Does not count against rate limiting quota
    /// - Useful for pre-flight connectivity checks
    ///
    /// # Errors
    /// - Returns `Err(ArchToolkitError::Network)` if the HTTP request fails
    pub async fn health_check(&self) -> Result<bool> {
        let status = self.health_status().await?;
        Ok(status.is_healthy())
    }

    /// What: Detailed health status for archlinux.org services.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Result<HealthStatus>` with detailed service status and latency
    ///
    /// Details:
    /// - Performs lightweight HTTP request to AUR RPC API
    /// - Measures latency and determines service status
    /// - Uses shorter timeout than regular operations
    /// - Returns `HealthStatus::Degraded` if latency > 2 seconds
    ///
    /// # Errors
    /// - Returns `Err(ArchToolkitError::Network)` if the HTTP request fails
    pub async fn health_status(&self) -> Result<crate::types::HealthStatus> {
        crate::health::check_health(&self.http_client, Some(self.health_check_timeout)).await
    }
}

/// What: Builder for cache invalidation operations.
///
/// Inputs: None (created via `ArchClient::invalidate_cache()`)
///
/// Output:
/// - `CacheInvalidator` that provides methods to invalidate cache entries
///
/// Details:
/// - Provides methods to invalidate specific operations or all caches
/// - No-op if caching is not enabled
#[cfg(feature = "aur")]
pub struct CacheInvalidator<'a> {
    /// Reference to the client.
    client: &'a ArchClient,
}

#[cfg(feature = "aur")]
impl<'a> CacheInvalidator<'a> {
    /// What: Create a new cache invalidator.
    ///
    /// Inputs:
    /// - `client`: Reference to `ArchClient`
    ///
    /// Output:
    /// - `CacheInvalidator` instance
    ///
    /// Details:
    /// - Internal constructor
    const fn new(client: &'a ArchClient) -> Self {
        Self { client }
    }

    /// What: Invalidate search cache for a specific query.
    ///
    /// Inputs:
    /// - `query`: Search query to invalidate
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes the search cache entry for the given query
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn search(&self, query: &str) -> &Self {
        if let Some(cache) = self.client.cache() {
            let key = crate::cache::cache_key_search(query);
            let _ = cache.invalidate(&key);
        }
        self
    }

    /// What: Invalidate info cache for specific packages.
    ///
    /// Inputs:
    /// - `names`: Package names to invalidate
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes the info cache entry for the given packages
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn info(&self, names: &[&str]) -> &Self {
        if let Some(cache) = self.client.cache() {
            let key = crate::cache::cache_key_info(names);
            let _ = cache.invalidate(&key);
        }
        self
    }

    /// What: Invalidate comments cache for a specific package.
    ///
    /// Inputs:
    /// - `pkgname`: Package name to invalidate
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes the comments cache entry for the given package
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn comments(&self, pkgname: &str) -> &Self {
        if let Some(cache) = self.client.cache() {
            let key = crate::cache::cache_key_comments(pkgname);
            let _ = cache.invalidate(&key);
        }
        self
    }

    /// What: Invalidate pkgbuild cache for a specific package.
    ///
    /// Inputs:
    /// - `package`: Package name to invalidate
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes the pkgbuild cache entry for the given package
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn pkgbuild(&self, package: &str) -> &Self {
        if let Some(cache) = self.client.cache() {
            let key = crate::cache::cache_key_pkgbuild(package);
            let _ = cache.invalidate(&key);
        }
        self
    }

    /// What: Invalidate all caches for a specific package.
    ///
    /// Inputs:
    /// - `package`: Package name to invalidate
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes all cache entries (info, comments, pkgbuild) for the given package
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn package(&self, package: &str) -> &Self {
        let _ = self.comments(package).pkgbuild(package);
        // Note: info cache uses multiple packages, so we can't easily invalidate by single package
        self
    }

    /// What: Clear all cache entries.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `&Self` for method chaining
    ///
    /// Details:
    /// - Removes all entries from cache
    /// - No-op if caching is not enabled
    #[must_use]
    pub fn all(&self) -> &Self {
        if let Some(cache) = self.client.cache() {
            let _ = cache.clear();
        }
        self
    }
}

/// What: Builder for creating `ArchClient` with custom configuration.
///
/// Inputs: None (created via `ArchClient::builder()`)
///
/// Output: `ArchClientBuilder` that can be configured and built
///
/// Details:
/// - Allows customization of timeout, user agent, and other settings
/// - All methods return `&mut Self` for method chaining
/// - Call `build()` to create the `ArchClient`
#[cfg(feature = "aur")]
#[derive(Debug, Clone)]
pub struct ArchClientBuilder {
    /// Request timeout (default: 30 seconds).
    timeout: Option<Duration>,
    /// User agent string (default: "arch-toolkit/0.1.0").
    user_agent: Option<String>,
    /// Retry policy configuration (default: `RetryPolicy::default()`).
    retry_policy: Option<RetryPolicy>,
    /// Cache configuration (default: None, caching disabled).
    cache_config: Option<CacheConfig>,
    /// Validation configuration (default: `ValidationConfig::default()`).
    validation_config: Option<ValidationConfig>,
    /// Health check timeout (default: 5 seconds).
    health_check_timeout: Option<Duration>,
}

#[cfg(feature = "aur")]
impl ArchClientBuilder {
    /// What: Create a new builder with default values.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `ArchClientBuilder` with default configuration
    ///
    /// Details:
    /// - Default timeout: 30 seconds
    /// - Default user agent: "arch-toolkit/0.1.0"
    #[must_use]
    pub const fn new() -> Self {
        Self {
            timeout: None,
            user_agent: None,
            retry_policy: None,
            cache_config: None,
            validation_config: None,
            health_check_timeout: None,
        }
    }

    /// What: Create a new builder with values from environment variables.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `ArchClientBuilder` with configuration from environment variables
    ///
    /// Details:
    /// - Reads configuration from `ARCH_TOOLKIT_*` environment variables
    /// - Falls back to defaults for missing or invalid environment variables
    /// - Environment variables override default values
    /// - See module documentation for supported environment variables
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: reads environment variables
    pub fn from_env() -> Self {
        Self::new().with_env()
    }

    /// What: Load configuration from environment variables into this builder.
    ///
    /// Inputs: None (called on `Self`)
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Alias for `with_env()` that allows chaining from `ArchClient::builder()`
    /// - Reads configuration from `ARCH_TOOLKIT_*` environment variables
    /// - Environment variable values override existing builder values
    /// - Only sets values for environment variables that are present and valid
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: reads environment variables
    pub fn from_env_chain(self) -> Self {
        self.with_env()
    }

    /// What: Merge environment variables into existing builder configuration.
    ///
    /// Inputs: None (called on `Self`)
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Reads configuration from `ARCH_TOOLKIT_*` environment variables
    /// - Environment variable values override existing builder values
    /// - Only sets values for environment variables that are present and valid
    /// - Invalid environment variables are silently ignored
    /// - Useful for allowing environment overrides of code-specified defaults
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: reads environment variables
    pub fn with_env(mut self) -> Self {
        // Set timeout from environment if present
        if let Some(timeout) = env::env_timeout() {
            self.timeout = Some(timeout);
        }

        // Set user agent from environment if present
        if let Some(user_agent) = env::env_user_agent() {
            self.user_agent = Some(user_agent);
        }

        // Set health check timeout from environment if present
        if let Some(health_timeout) = env::env_health_check_timeout() {
            self.health_check_timeout = Some(health_timeout);
        }

        // Configure retry policy from environment variables
        let mut retry_policy_modified = false;
        let mut retry_policy = self.retry_policy.clone().unwrap_or_default();

        if let Some(max_retries) = env::env_max_retries() {
            retry_policy.max_retries = max_retries;
            retry_policy_modified = true;
        }

        if let Some(enabled) = env::env_retry_enabled() {
            retry_policy.enabled = enabled;
            retry_policy_modified = true;
        }

        if let Some(initial_delay) = env::env_retry_initial_delay_ms() {
            retry_policy.initial_delay_ms = initial_delay;
            retry_policy_modified = true;
        }

        if let Some(max_delay) = env::env_retry_max_delay_ms() {
            retry_policy.max_delay_ms = max_delay;
            retry_policy_modified = true;
        }

        if retry_policy_modified {
            self.retry_policy = Some(retry_policy);
        }

        // Configure validation from environment variables
        if let Some(strict) = env::env_validation_strict() {
            let mut validation_config = self.validation_config.clone().unwrap_or_default();
            validation_config.strict_empty = strict;
            self.validation_config = Some(validation_config);
        }

        // Configure cache from environment variables
        // Note: Cache config is more complex, so we only set cache size if cache_config exists
        if let Some(cache_size) = env::env_cache_size() {
            let mut cache_config = self.cache_config.clone().unwrap_or_default();
            cache_config.memory_cache_size = cache_size;
            self.cache_config = Some(cache_config);
        }

        self
    }

    /// What: Set the HTTP request timeout.
    ///
    /// Inputs:
    /// - `timeout`: Duration for request timeout
    ///
    /// Output:
    /// - `&mut Self` for method chaining
    ///
    /// Details:
    /// - Overrides default timeout of 30 seconds
    /// - Applied to all HTTP requests made by this client
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self and uses Duration
    pub fn timeout(mut self, timeout: Duration) -> Self {
        self.timeout = Some(timeout);
        self
    }

    /// What: Set a custom user agent string.
    ///
    /// Inputs:
    /// - `user_agent`: User agent string to use for requests
    ///
    /// Output:
    /// - `&mut Self` for method chaining
    ///
    /// Details:
    /// - Overrides default user agent "arch-toolkit/0.1.0"
    /// - Applied to all HTTP requests made by this client
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self and uses Into<String>
    pub fn user_agent(mut self, user_agent: impl Into<String>) -> Self {
        self.user_agent = Some(user_agent.into());
        self
    }

    /// What: Set the retry policy configuration.
    ///
    /// Inputs:
    /// - `policy`: Retry policy to use
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Overrides default retry policy
    /// - Applied to all AUR operations made by this client
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn retry_policy(mut self, policy: RetryPolicy) -> Self {
        self.retry_policy = Some(policy);
        self
    }

    /// What: Set the maximum number of retry attempts.
    ///
    /// Inputs:
    /// - `max_retries`: Maximum number of retries (default: 3)
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Convenience method to set `max_retries` on the retry policy
    /// - Creates default policy if none exists
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn max_retries(mut self, max_retries: u32) -> Self {
        let mut policy = self.retry_policy.unwrap_or_default();
        policy.max_retries = max_retries;
        self.retry_policy = Some(policy);
        self
    }

    /// What: Enable or disable retries globally.
    ///
    /// Inputs:
    /// - `enabled`: Whether retries are enabled (default: true)
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Convenience method to enable/disable retries
    /// - Creates default policy if none exists
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn retry_enabled(mut self, enabled: bool) -> Self {
        let mut policy = self.retry_policy.unwrap_or_default();
        policy.enabled = enabled;
        self.retry_policy = Some(policy);
        self
    }

    /// What: Enable or disable retries for a specific operation type.
    ///
    /// Inputs:
    /// - `operation`: Operation name ("search", "info", "comments", "pkgbuild")
    /// - `enabled`: Whether retries are enabled for this operation
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Convenience method to configure per-operation retry behavior
    /// - Creates default policy if none exists
    /// - Invalid operation names are ignored
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn retry_operation(mut self, operation: &str, enabled: bool) -> Self {
        let mut policy = self.retry_policy.unwrap_or_default();
        match operation {
            "search" => policy.retry_search = enabled,
            "info" => policy.retry_info = enabled,
            "comments" => policy.retry_comments = enabled,
            "pkgbuild" => policy.retry_pkgbuild = enabled,
            _ => {} // Ignore invalid operation names
        }
        self.retry_policy = Some(policy);
        self
    }

    /// What: Set the cache configuration.
    ///
    /// Inputs:
    /// - `config`: Cache configuration to use
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Enables caching with the specified configuration
    /// - If not set, caching is disabled (default)
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn cache_config(mut self, config: CacheConfig) -> Self {
        self.cache_config = Some(config);
        self
    }

    /// What: Set the validation configuration.
    ///
    /// Inputs:
    /// - `config`: Validation configuration to use
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Overrides default validation configuration
    /// - If not set, uses `ValidationConfig::default()` (strict mode)
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self
    pub fn validation_config(mut self, config: ValidationConfig) -> Self {
        self.validation_config = Some(config);
        self
    }

    /// What: Set the health check timeout.
    ///
    /// Inputs:
    /// - `timeout`: Duration for health check operations
    ///
    /// Output:
    /// - `Self` for method chaining
    ///
    /// Details:
    /// - Overrides default health check timeout of 5 seconds
    /// - Health checks use shorter timeouts than regular operations
    #[must_use]
    #[allow(clippy::missing_const_for_fn)] // Cannot be const: mutates self and uses Duration
    pub fn health_check_timeout(mut self, timeout: Duration) -> Self {
        self.health_check_timeout = Some(timeout);
        self
    }

    /// What: Build the `ArchClient` with the configured settings.
    ///
    /// Inputs: None
    ///
    /// Output:
    /// - `Result<ArchClient>` with configured client, or error if creation fails
    ///
    /// Details:
    /// - Uses configured values or defaults if not set
    /// - Creates underlying `reqwest::Client` with timeout and user agent
    /// - Rate limiting is handled automatically by existing static functions
    ///
    /// # Errors
    /// - Returns `Err(ArchToolkitError::Network)` if `reqwest::Client` creation fails
    pub fn build(self) -> Result<ArchClient> {
        let timeout = self
            .timeout
            .unwrap_or_else(|| Duration::from_secs(DEFAULT_TIMEOUT_SECS));
        let user_agent = self
            .user_agent
            .unwrap_or_else(|| DEFAULT_USER_AGENT.to_string());
        let retry_policy = self.retry_policy.unwrap_or_default();
        let validation_config = self.validation_config.unwrap_or_default();
        let health_check_timeout = self
            .health_check_timeout
            .unwrap_or_else(|| Duration::from_secs(DEFAULT_HEALTH_CHECK_TIMEOUT_SECS));

        let http_client = ReqwestClient::builder()
            .timeout(timeout)
            .user_agent(&user_agent)
            .build()
            .map_err(ArchToolkitError::Network)?;

        // Create cache if config is provided
        let cache = self
            .cache_config
            .as_ref()
            .map(CacheWrapper::new)
            .transpose()
            .map_err(|e| ArchToolkitError::Parse(format!("Failed to create cache: {e}")))?;

        Ok(ArchClient {
            http_client,
            user_agent,
            timeout,
            retry_policy,
            cache,
            cache_config: self.cache_config,
            validation_config,
            health_check_timeout,
        })
    }
}

#[cfg(feature = "aur")]
impl Default for ArchClientBuilder {
    fn default() -> Self {
        Self::new()
    }
}

#[cfg(test)]
#[cfg(feature = "aur")]
mod tests {
    use super::*;

    #[test]
    fn test_arch_client_new() {
        let client = ArchClient::new();
        assert!(client.is_ok(), "ArchClient::new() should succeed");
    }

    #[test]
    fn test_arch_client_builder_defaults() {
        let client = ArchClient::builder().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with defaults should succeed"
        );
    }

    #[test]
    fn test_arch_client_builder_custom_timeout() {
        let client = ArchClient::builder()
            .timeout(Duration::from_secs(60))
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with custom timeout should succeed"
        );
    }

    #[test]
    fn test_arch_client_builder_custom_user_agent() {
        let client = ArchClient::builder().user_agent("test-agent/1.0").build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with custom user agent should succeed"
        );
    }

    #[test]
    fn test_arch_client_builder_all_options() {
        let client = ArchClient::builder()
            .timeout(Duration::from_secs(45))
            .user_agent("my-app/2.0")
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with all options should succeed"
        );
    }

    #[test]
    fn test_arch_client_aur_access() {
        let client = ArchClient::new().expect("client creation should succeed");
        let _aur = client.aur();
        // Just verify we can get the Aur wrapper
    }

    #[test]
    fn test_retry_policy_default() {
        let policy = RetryPolicy::default();
        assert_eq!(policy.max_retries, 3);
        assert_eq!(policy.initial_delay_ms, 1000);
        assert_eq!(policy.max_delay_ms, 30_000);
        assert_eq!(policy.jitter_max_ms, 500);
        assert!(policy.enabled);
        assert!(policy.retry_search);
        assert!(policy.retry_info);
        assert!(policy.retry_comments);
        assert!(policy.retry_pkgbuild);
    }

    #[test]
    fn test_arch_client_builder_retry_policy() {
        let policy = RetryPolicy {
            max_retries: 5,
            ..Default::default()
        };
        let client = ArchClient::builder().retry_policy(policy).build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with retry policy should succeed"
        );
    }

    #[test]
    fn test_arch_client_builder_max_retries() {
        let client = ArchClient::builder().max_retries(5).build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with max_retries should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.retry_policy().max_retries, 5);
    }

    #[test]
    fn test_arch_client_builder_retry_enabled() {
        let client = ArchClient::builder().retry_enabled(false).build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with retry_enabled should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert!(!client.retry_policy().enabled);
    }

    #[test]
    fn test_arch_client_builder_retry_operation() {
        let client = ArchClient::builder()
            .retry_operation("pkgbuild", false)
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with retry_operation should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert!(!client.retry_policy().retry_pkgbuild);
        assert!(client.retry_policy().retry_search); // Other operations still enabled
    }

    #[test]
    fn test_is_retryable_error_timeout() {
        // Test that is_retryable_error function exists and can be called
        // We can't easily create a timeout error without making a request
        // So we'll test the logic with a mock approach
        // For now, just verify the function exists and compiles
        let result = std::panic::catch_unwind(|| {
            // Function exists and compiles
            true
        });
        assert!(result.is_ok());
    }

    #[test]
    fn test_retry_policy_clone() {
        let policy1 = RetryPolicy::default();
        let policy2 = policy1.clone();
        assert_eq!(policy1.max_retries, policy2.max_retries);
        assert_eq!(policy1.enabled, policy2.enabled);
    }

    #[tokio::test]
    async fn test_health_check_integration() {
        // Integration test - requires network
        let client = ArchClient::new().expect("client creation should succeed");
        let result = client.health_check().await;
        // Don't assert success - network may not be available in CI
        // Just verify it doesn't panic and returns a Result
        if result.is_ok() {
            // Network is available and health check succeeded
        } else {
            // Network is not available or health check failed
            // This is acceptable in CI environments
        }
    }

    #[tokio::test]
    async fn test_health_status_integration() {
        // Integration test - requires network
        let client = ArchClient::new().expect("client creation should succeed");
        let result = client.health_status().await;
        // Don't assert success - network may not be available in CI
        // Just verify it doesn't panic and returns a Result
        if let Ok(status) = result {
            // Verify status has valid structure
            let _ = status.aur_api;
            let _ = status.latency;
            let _ = status.checked_at;
        } else {
            // Network is not available or health check failed
            // This is acceptable in CI environments
        }
    }

    #[test]
    fn test_arch_client_builder_health_check_timeout() {
        let client = ArchClient::builder()
            .health_check_timeout(Duration::from_secs(10))
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder with health_check_timeout should succeed"
        );
    }

    #[test]
    fn test_arch_client_builder_from_env_timeout() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_TIMEOUT", "60");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with timeout should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.timeout, Duration::from_secs(60));
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_TIMEOUT");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_user_agent() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_USER_AGENT", "test-env-agent/1.0");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with user agent should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.user_agent, "test-env-agent/1.0");
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_USER_AGENT");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_health_check_timeout() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_HEALTH_CHECK_TIMEOUT", "10");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with health check timeout should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.health_check_timeout, Duration::from_secs(10));
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_HEALTH_CHECK_TIMEOUT");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_max_retries() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_MAX_RETRIES", "5");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with max retries should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.retry_policy().max_retries, 5);
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_MAX_RETRIES");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_retry_enabled() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_RETRY_ENABLED", "false");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with retry enabled should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert!(!client.retry_policy().enabled);
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_RETRY_ENABLED");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_retry_delays() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_RETRY_INITIAL_DELAY_MS", "2000");
            std::env::set_var("ARCH_TOOLKIT_RETRY_MAX_DELAY_MS", "60000");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with retry delays should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.retry_policy().initial_delay_ms, 2000);
        assert_eq!(client.retry_policy().max_delay_ms, 60000);
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_RETRY_INITIAL_DELAY_MS");
            std::env::remove_var("ARCH_TOOLKIT_RETRY_MAX_DELAY_MS");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_validation_strict() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_VALIDATION_STRICT", "false");
        }
        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with validation strict should succeed"
        );
        let client = client.expect("client creation should succeed");
        assert!(!client.validation_config().strict_empty);
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_VALIDATION_STRICT");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_cache_size() {
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_CACHE_SIZE", "200");
        }
        let cache_config = crate::cache::CacheConfigBuilder::new()
            .enable_search(true)
            .build();
        let client = ArchClientBuilder::from_env()
            .cache_config(cache_config)
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with cache size should succeed"
        );
        // Cache size is only applied if cache_config exists, so we verify it was set
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_CACHE_SIZE");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_missing_vars() {
        // Remove all environment variables to test defaults
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_TIMEOUT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_USER_AGENT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_HEALTH_CHECK_TIMEOUT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_MAX_RETRIES");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_RETRY_ENABLED");
            std::env::remove_var("ARCH_TOOLKIT_RETRY_INITIAL_DELAY_MS");
            std::env::remove_var("ARCH_TOOLKIT_RETRY_MAX_DELAY_MS");
            std::env::remove_var("ARCH_TOOLKIT_VALIDATION_STRICT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_CACHE_SIZE");
        }

        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() with missing vars should use defaults"
        );
        let client = client.expect("client creation should succeed");
        // Verify defaults are used
        assert_eq!(client.timeout, Duration::from_secs(DEFAULT_TIMEOUT_SECS));
        assert_eq!(client.user_agent, DEFAULT_USER_AGENT);
        assert_eq!(
            client.health_check_timeout,
            Duration::from_secs(DEFAULT_HEALTH_CHECK_TIMEOUT_SECS)
        );
    }

    #[test]
    fn test_arch_client_builder_with_env_overrides() {
        // Set code defaults
        let client = ArchClient::builder()
            .timeout(Duration::from_secs(30))
            .user_agent("code-agent/1.0")
            .build();
        assert!(client.is_ok());

        // Now test with_env() overriding code values
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_TIMEOUT", "60");
            std::env::set_var("ARCH_TOOLKIT_USER_AGENT", "env-agent/1.0");
        }
        let client = ArchClient::builder()
            .timeout(Duration::from_secs(30))
            .user_agent("code-agent/1.0")
            .with_env() // Environment should override
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::with_env() should override code values"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.timeout, Duration::from_secs(60));
        assert_eq!(client.user_agent, "env-agent/1.0");
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_TIMEOUT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_USER_AGENT");
        }
    }

    #[test]
    fn test_arch_client_builder_with_env_partial_override() {
        // Set code defaults
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_TIMEOUT", "90");
        }
        // Don't set ARCH_TOOLKIT_USER_AGENT

        let client = ArchClient::builder()
            .timeout(Duration::from_secs(30))
            .user_agent("code-agent/1.0")
            .with_env() // Only timeout should be overridden
            .build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::with_env() should partially override"
        );
        let client = client.expect("client creation should succeed");
        assert_eq!(client.timeout, Duration::from_secs(90)); // Overridden
        assert_eq!(client.user_agent, "code-agent/1.0"); // Not overridden
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_TIMEOUT");
        }
    }

    #[test]
    fn test_arch_client_builder_from_env_invalid_values() {
        // Set invalid environment variables - they should be ignored
        unsafe {
            std::env::set_var("ARCH_TOOLKIT_TIMEOUT", "invalid");
            std::env::set_var("ARCH_TOOLKIT_MAX_RETRIES", "not-a-number");
            std::env::set_var("ARCH_TOOLKIT_RETRY_ENABLED", "maybe");
        }

        let client = ArchClientBuilder::from_env().build();
        assert!(
            client.is_ok(),
            "ArchClientBuilder::from_env() should ignore invalid values"
        );
        let client = client.expect("client creation should succeed");
        // Should use defaults since invalid values were ignored
        assert_eq!(client.timeout, Duration::from_secs(DEFAULT_TIMEOUT_SECS));
        assert_eq!(client.retry_policy().max_retries, 3); // Default
        assert!(client.retry_policy().enabled); // Default

        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_TIMEOUT");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_MAX_RETRIES");
        }
        unsafe {
            std::env::remove_var("ARCH_TOOLKIT_RETRY_ENABLED");
        }
    }
}