From 857c16985910da5cbbc4b5ded6bcd7116287151f Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 11:59:27 +0200 Subject: [PATCH 1/6] feat(account): rotate on quota exhaustion (task 25) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD §6.4 / PRD-v2 §P1.6. New `AccountRotator` application service detects quota exhaustion (HTTP 429 or `traffic_left` below threshold via `is_quota_signal`), pulls the offending account out of rotation for a hoster-specific cooldown via `mark_exhausted`, and asks the existing `AccountSelector` for the next best candidate via `next_account` returning a `NextAccountOutcome` enum that distinguishes `Picked(Account)`, `NoneAvailable`, and `AllExhausted { next_eligible_at_ms }` so callers can decide between using a credential, falling back to the free path, or stalling the download in `Waiting` until the earliest cooldown expires. `NextAccountOutcome::error_message(service_name)` returns the PRD-frozen wording ("All accounts exhausted for {service}" / "No account available for {service}") so the UI / log copy stays uniform across hosters. Cooldown state lives in an in-memory `Mutex>` (NOT persisted in SQLite — a restart wipes the cooldown, which is the desired behaviour for the 5-15 minute hoster reset window). A poisoned mutex surfaces as `AppError::Validation` to mirror `AccountSelector::pick_round_robin`'s contract. Expired entries are pruned lazily so no background sweeper is needed. `record_traffic_refresh(account_id, traffic_left, threshold)` clears the marker only when the upstream confirms `traffic_left >= threshold` — a `None` or below-threshold observation leaves the marker in place so a hoster without a traffic counter cannot silently undo every `mark_exhausted`. Selector gains `select_best_excluding(service, strategy, exclude_ids)`; `select_best` is now a thin wrapper. New `DomainEvent::AccountExhausted { id, service_name, exhausted_until_ms }` forwarded by the Tauri bridge as `account-exhausted`. New transient `Account::exhausted_until` field reset to `None` by `Account::reconstruct` so the rotator's map remains the single source of truth across SQLite roundtrips. Twenty-two unit tests cover the four acceptance criteria plus edge cases: zero-TTL no-op, deadline-exclusive equality, cross-service deadline isolation, `None`-traffic refresh keeps cooldown, `404` / `500` ignored by `is_quota_signal`, idempotent `clear_exhausted`, lazy cooldown expiry surfaces an account back into rotation. Total suite: 1172 lib tests + 1 integration test green. --- CHANGELOG.md | 1 + .../src/adapters/driven/event/tauri_bridge.rs | 12 + .../driven/logging/download_log_bridge.rs | 3 +- src-tauri/src/application/command_bus.rs | 16 +- .../application/services/account_rotator.rs | 753 ++++++++++++++++++ .../application/services/account_selector.rs | 119 ++- src-tauri/src/application/services/mod.rs | 2 + src-tauri/src/domain/event.rs | 12 + src-tauri/src/domain/model/account.rs | 88 ++ 9 files changed, 1002 insertions(+), 4 deletions(-) create mode 100644 src-tauri/src/application/services/account_rotator.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e91cbd..3c1dd18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Account rotation on quota** (PRD §6.4, PRD-v2 §P1.6, task 25): new `AccountRotator` application service detects quota exhaustion (HTTP `429` or `traffic_left` below a caller-supplied threshold via `is_quota_signal`), pulls the offending account out of rotation for a hoster-specific cooldown via `mark_exhausted(account_id, service_name, ttl_secs)`, and asks the existing `AccountSelector` for the next best candidate via `next_account(service, strategy) -> NextAccountOutcome`. The outcome enum distinguishes three caller-actionable states: `Picked(Account)` (use the credential), `NoneAvailable` (no enabled / non-expired account configured — fall back to the free path or surface a UI hint), and `AllExhausted { next_eligible_at_ms }` (every eligible account is on cooldown — stall the download in `Waiting` until the earliest deadline so the scheduler can retry without busy-looping). `NextAccountOutcome::error_message(service_name)` returns the PRD §6.4 standard wording (`"All accounts exhausted for {service}"` / `"No account available for {service}"`) so callers attaching the error to `Download.error` stay uniform across hosters. Cooldown lifecycle: `record_traffic_refresh(account_id, traffic_left, threshold)` clears the marker only when the upstream confirms `traffic_left >= threshold` (a `None` observation or below-threshold value leaves the marker in place so a hoster without a traffic counter cannot silently undo every `mark_exhausted`); `clear_exhausted(account_id)` is the explicit reset path, idempotent for unknown ids; expired entries are pruned lazily on the next `next_account` call so no background sweeper is needed. The exhaustion map sits behind a `std::sync::Mutex` in `AccountRotator` (intentionally NOT persisted in SQLite — a process restart wipes the cooldown, which is the desired behaviour for the 5-to-15-minute hoster reset window); a poisoned mutex surfaces as `AppError::Validation("exhausted accounts mutex poisoned")` so callers can distinguish "no candidate" from "internal state corrupted", matching `AccountSelector::pick_round_robin`'s contract. The `AllExhausted` deadline restricts its scan to accounts that actually belong to the queried service so a parallel-service entry cannot leak its cooldown into an unrelated answer. New `AccountSelector::select_best_excluding(service, strategy, exclude_ids)` extends the existing `select_best` with an exclude list (no caching, no behaviour change for empty `exclude`); the prior signature is now a thin wrapper. New `DomainEvent::AccountExhausted { id, service_name, exhausted_until_ms }` forwarded by the Tauri bridge as `account-exhausted` (camelCase `exhaustedUntilMs`). New transient `Account::exhausted_until: Option` field with `mark_exhausted` / `clear_exhausted` / `is_exhausted(now_ms)` / `exhausted_until()` methods — the field is reset to `None` by `Account::reconstruct` so the rotator's in-memory map remains the single source of truth even though SQLite roundtrips drop the marker. New `CommandBus::with_account_rotator` / `account_rotator()` builder & accessor wires the rotator alongside the existing `AccountSelector`. Twenty-two new unit tests cover the four acceptance criteria (`429 → next account`, `all exhausted → AllExhausted with earliest deadline`, `traffic-refresh clears cooldown when above threshold`, full rotator + selector-exclude integration), plus edge cases: zero-TTL no-op, deadline-exclusive equality, cross-service deadline isolation, `None`-traffic refresh keeps cooldown, `404` / `500` ignored by `is_quota_signal`, threshold-equality below-but-not-above, idempotent `clear_exhausted`, lazy cooldown expiry surfaces an account back into rotation. Unblocks task 38 (vortex-mod-1fichier free + premium) which is the first hoster to wire the rotation flow. - **Account auto-selection** (PRD §6.4, PRD-v2 §P1.5, task 24): new `AccountSelector` application service picks the best `Account` per service for the live `AppConfig::account_selection_strategy`. Three strategies: `BestTraffic` (default, ranks `enabled → not expired → most traffic_left → most recent last_validated → smallest id` with `Unlimited` traffic ranking above any finite value), `RoundRobin` (per-service cursor over enabled non-expired candidates ordered by id; a poisoned cursor mutex now surfaces as `AppError::Validation("round-robin cursor mutex poisoned")` so it stays distinguishable from "no eligible account"), and `Manual` (fallback alias of `BestTraffic` until pinning UI lands). The selector reads `AccountRepository::list_by_service` on every call instead of caching: the previous event-driven invalidation could read stale rows when `select_best` landed between `bus.publish(AccountUpdated)` and the spawned `TokioEventBus` subscriber firing. New `CommandBus::resolve_account_for(service_name)` exposes the selector to download / link-grabber flows; failures from `ConfigStore::get_config()` propagate via `?` instead of being swallowed by a default-strategy fallback. New `DomainEvent::NoAccountAvailable { service_name }` (emitted when no candidate passes the filter) and `DomainEvent::AccountSelected { id, service_name, strategy }` (emitted whenever a pick is made), both forwarded by the Tauri bridge as `no-account-available` / `account-selected`. New `account_selection_strategy` field on `AppConfig` / `ConfigPatch` / `apply_patch` plus the matching IPC and TOML serialisation paths (snake_case `"best_traffic" | "round_robin" | "manual"`). The IPC layer rejects unknown strategy values: `ConfigPatchDto` → `ConfigPatch` is `TryFrom` and `settings_update` surfaces `invalid account selection strategy: …` instead of silently ignoring a typo. The TOML store mirrors the rule: `ConfigDto` → `AppConfig` is also `TryFrom`, so a hand-edited `config.toml` carrying an unknown strategy value now fails fast with `StorageError("invalid config: …")` instead of silently coercing to `best_traffic`. Backward compat is preserved: a legacy `config.toml` written before this field existed deserializes the missing key as the empty string via `#[serde(default)]`, and that empty case is treated as `BestTraffic` so an upgrade does not break startup. Eighteen unit tests cover the four acceptance criteria (3-account scenario, all-expired surface, comparative ranking table, round-robin alternance), repo-fresh selection, poisoned-cursor surfacing, IPC rejection of unknown strategies, TOML-store rejection of unknown persisted strategies, legacy-config (missing strategy field) backward compat, and config-error propagation. Unblocks task 25 (rotation auto sur quota). - **Accounts view** (PRD §6.4, PRD-v2 §P1.4, task 23): full Accounts management UI replacing the previous `PlaceholderView`. Header tabs (`All` / `Debrid` / `Premium` / `Free`) drive a category filter on top of the SQLite-backed `account_list` query, with the `(filter, all)` count rendered next to each label. Each row exposes the service, username, account type, derived status badge (`Active` / `Expired` / `Disabled` / `Unverified`), an aria-labelled traffic progress bar (used / total formatted via `formatBytes`), `valid_until` and `last_validated` columns, an enable/disable `Switch`, an inline `Validate` button, and a kebab menu with `Edit` / `Delete`. The new `AddAccountDialog` validates non-empty service / username / password before submission. `EditAccountDialog` posts a partial `AccountPatch` (skips fields that did not change so the keyring rotation only fires when the password field is filled). The `Delete` action honours the existing `settings.confirm_delete` toggle: when enabled it pops the new `DeleteAccountDialog` (translated description naming the row), otherwise it deletes immediately. `ImportAccountsDialog` calls `tauri-plugin-dialog`'s file-pick to anchor the encrypted bundle path, prompts for the passphrase, then calls `account_import` and invalidates the list cache so freshly-imported rows appear without a manual refresh; `ExportAccountsDialog` requires the user to confirm the passphrase, opens the native `save` dialog for the destination, and reports the row count via toast. Nine new Tauri IPC commands wire the existing `CommandBus` / `QueryBus` handlers (tasks 21, 22) to the frontend: `account_add`, `account_update`, `account_delete`, `account_validate`, `account_export`, `account_import`, `account_list`, `account_get`, `account_traffic_get`, all registered in `invoke_handler!` and re-exported from `lib.rs`. The runtime now wires `SqliteAccountRepo` to both buses and provides the `KeyringAccountStore` + `AesGcmPbkdf2Codec` adapters to the `CommandBus`. Adds `useAccountsQuery` (TanStack Query, 30 s `staleTime`) and `accountQueries` cache key factory. New i18n namespace `accounts.*` covers titles, status badges, dialog copy and toast messages in `en.json` + `fr.json`. 13 Vitest tests cover render, empty state, category filter, add → IPC → toast flow, delete → confirm → IPC, export trigger disabled when no accounts, export with passphrase, import with file picker. `AccountValidator` is intentionally not wired in this commit — `account_validate` returns the configured `Validation` error until the first hoster plugin lands (task 38), letting the UI render the failure toast without crashing. The "volume per account" stat from the requirements list is deferred until `history` gains an `account_id` column. - **Accounts queries** (PRD §6.4, PRD-v2 §P1.3, task 22): three CQRS query handlers (`list_accounts`, `get_account`, `get_account_traffic`) wired through the `QueryBus` builder via a new `with_account_repo` setter. New read models `AccountViewDto` and `AccountTrafficDto` (`#[serde(rename_all = "camelCase")]`) expose every persisted field — `id`, `service_name`, `username`, `account_type`, `enabled`, `traffic_left`, `traffic_total`, `valid_until`, `last_validated`, `created_at`, `credential_ref` — and never carry a password or raw credential field, by construction. `AccountFilter { service_name?, account_type?, enabled? }` AND-combines filters: `service_name` is delegated to the repo's `list_by_service` for SQL-level pruning, while `account_type` and `enabled` filter in memory. `get_account_traffic` returns the persisted counters; the upstream-refresh path is the existing `account_validate` command (task 21), keeping queries side-effect free per the project CQRS rule. 21 new unit tests against an `InMemoryAccountRepoForQueries` fixture cover filter combinations, missing-id 404s, missing-repo validation errors, camelCase serialization shape, and explicit "no password field" assertions on `serde_json::to_value` output. Unblocks task 23 (Vue Accounts). diff --git a/src-tauri/src/adapters/driven/event/tauri_bridge.rs b/src-tauri/src/adapters/driven/event/tauri_bridge.rs index 457719b..40ffc1c 100644 --- a/src-tauri/src/adapters/driven/event/tauri_bridge.rs +++ b/src-tauri/src/adapters/driven/event/tauri_bridge.rs @@ -70,6 +70,7 @@ fn event_name(event: &DomainEvent) -> &'static str { DomainEvent::AccountsExported { .. } => "accounts-exported", DomainEvent::NoAccountAvailable { .. } => "no-account-available", DomainEvent::AccountSelected { .. } => "account-selected", + DomainEvent::AccountExhausted { .. } => "account-exhausted", } } @@ -212,6 +213,17 @@ fn event_payload(event: &DomainEvent) -> serde_json::Value { "strategy": strategy, }) } + DomainEvent::AccountExhausted { + id, + service_name, + exhausted_until_ms, + } => { + json!({ + "id": id.as_str(), + "serviceName": service_name, + "exhaustedUntilMs": exhausted_until_ms, + }) + } } } diff --git a/src-tauri/src/adapters/driven/logging/download_log_bridge.rs b/src-tauri/src/adapters/driven/logging/download_log_bridge.rs index 6fa65bf..955007e 100644 --- a/src-tauri/src/adapters/driven/logging/download_log_bridge.rs +++ b/src-tauri/src/adapters/driven/logging/download_log_bridge.rs @@ -139,7 +139,8 @@ fn record_download_event(store: &DownloadLogStore, event: &DomainEvent) { | DomainEvent::AccountsImported { .. } | DomainEvent::AccountsExported { .. } | DomainEvent::NoAccountAvailable { .. } - | DomainEvent::AccountSelected { .. } => {} + | DomainEvent::AccountSelected { .. } + | DomainEvent::AccountExhausted { .. } => {} } } diff --git a/src-tauri/src/application/command_bus.rs b/src-tauri/src/application/command_bus.rs index aada26c..99cf8ac 100644 --- a/src-tauri/src/application/command_bus.rs +++ b/src-tauri/src/application/command_bus.rs @@ -5,7 +5,7 @@ use std::sync::Arc; -use crate::application::services::AccountSelector; +use crate::application::services::{AccountRotator, AccountSelector}; use crate::domain::ports::driven::{ AccountCredentialStore, AccountRepository, AccountValidator, ArchiveExtractor, ChecksumComputer, ClipboardObserver, ConfigStore, CredentialStore, DownloadEngine, @@ -38,6 +38,7 @@ pub struct CommandBus { account_credential_store: Option>, account_validator: Option>, account_selector: Option>, + account_rotator: Option>, passphrase_codec: Option>, /// Serializes queue-position allocation across handlers. Without this, /// two concurrent move-to-top/move-to-bottom/start-download calls can @@ -83,6 +84,7 @@ impl CommandBus { account_credential_store: None, account_validator: None, account_selector: None, + account_rotator: None, passphrase_codec: None, queue_position_lock: tokio::sync::Mutex::new(()), } @@ -116,6 +118,14 @@ impl CommandBus { self } + /// Builder-style setter for the quota-aware [`AccountRotator`]. + /// Optional — when omitted, callers fall back to the plain + /// `account_selector()` (no rotation, no exhaustion tracking). + pub fn with_account_rotator(mut self, rotator: Arc) -> Self { + self.account_rotator = Some(rotator); + self + } + /// Builder-style setter for the passphrase codec used by the /// import / export commands. pub fn with_passphrase_codec(mut self, codec: Arc) -> Self { @@ -139,6 +149,10 @@ impl CommandBus { self.account_selector.as_deref() } + pub fn account_rotator(&self) -> Option<&AccountRotator> { + self.account_rotator.as_deref() + } + pub fn passphrase_codec(&self) -> Option<&dyn PassphraseCodec> { self.passphrase_codec.as_deref() } diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs new file mode 100644 index 0000000..436d219 --- /dev/null +++ b/src-tauri/src/application/services/account_rotator.rs @@ -0,0 +1,753 @@ +//! `AccountRotator` — quota-aware account rotation. +//! +//! PRD §6.4 ("Rotation si quota atteint") — when a hoster signals +//! quota exhaustion (HTTP 429, `traffic_left` below threshold, …) the +//! rotator pulls the offending account out of the rotation for a +//! cooldown window, asks the [`AccountSelector`] for the next best +//! candidate, and emits a `DomainEvent::AccountExhausted` so the UI +//! can warn the user. +//! +//! The exhaustion state is held entirely in memory: the SQLite-backed +//! [`Account`] aggregate intentionally does not persist +//! `exhausted_until` (a fresh `Account::reconstruct` always returns +//! `exhausted_until == None`). Storing it in a process-local map means +//! a restart wipes the cooldown — that is the desired behaviour for a +//! 5-to-15 minute window. Persisting it would need a new SQLite column +//! plus a purge job, neither of which buys correctness when the +//! upstream hoster will simply re-send the same 429. +//! +//! Concurrency: the map sits behind a `std::sync::Mutex`. Every public +//! method that takes the lock surfaces a poisoned mutex as +//! `AppError::Validation` instead of folding it into `Ok(None)`, so a +//! caller can distinguish "nothing eligible" from "internal state +//! corrupted" — same contract as `AccountSelector::pick_round_robin`. + +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +use crate::application::error::AppError; +use crate::application::services::AccountSelector; +use crate::domain::event::DomainEvent; +use crate::domain::model::account::{Account, AccountId, AccountSelectionStrategy}; +use crate::domain::ports::driven::AccountRepository; +use crate::domain::ports::driven::clock::Clock; +use crate::domain::ports::driven::event_bus::EventBus; + +/// Outcome of [`AccountRotator::next_account`]. +/// +/// Distinguishes the three states callers must react to differently: +/// pick a credential, fall back to the free / unauthenticated path, or +/// stall the download in `Waiting` until the cooldown expires. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum NextAccountOutcome { + /// The rotator picked a non-exhausted account. + Picked(Account), + /// The service has zero registered accounts (or all are + /// disabled / expired). Callers should fall back to the free + /// hoster path or surface a `NoAccountConfigured` UI hint. + NoneAvailable, + /// At least one account exists but every eligible candidate is + /// currently exhausted. Callers should mark the download + /// `Waiting` until `next_eligible_at_ms` (Unix epoch ms — the + /// earliest cooldown deadline among the exhausted set) so the + /// scheduler can retry without busy-looping. + AllExhausted { next_eligible_at_ms: u64 }, +} + +impl NextAccountOutcome { + /// Standard human-readable message a caller can attach to the + /// `Download.error` field when the outcome is not `Picked`. Returns + /// `None` for `Picked` (the caller has a credential, no error to + /// report). The wording is frozen by PRD §6.4 so the UI / log / + /// notification copy stays consistent across hosters. + pub fn error_message(&self, service_name: &str) -> Option { + match self { + Self::Picked(_) => None, + Self::NoneAvailable => Some(format!("No account available for {service_name}")), + Self::AllExhausted { .. } => Some(format!("All accounts exhausted for {service_name}")), + } + } +} + +/// Central account rotation service. Wraps an [`AccountSelector`] and +/// adds an in-memory cooldown map keyed by [`AccountId`]. +pub struct AccountRotator { + selector: Arc, + repo: Arc, + event_bus: Arc, + clock: Arc, + /// `account_id → cooldown deadline (Unix epoch ms)`. An entry whose + /// deadline is `<= now_ms` is considered expired and pruned on the + /// next read. This avoids needing a background sweeper. + exhausted: Mutex>, +} + +impl AccountRotator { + pub fn new( + selector: Arc, + repo: Arc, + event_bus: Arc, + clock: Arc, + ) -> Arc { + Arc::new(Self { + selector, + repo, + event_bus, + clock, + exhausted: Mutex::new(HashMap::new()), + }) + } + + /// Pick the next eligible account for `service_name`, skipping any + /// account whose cooldown is still active. + /// + /// Distinguishes "zero accounts" (`NoneAvailable`) from "all + /// exhausted" (`AllExhausted { next_eligible_at_ms }`) so the + /// caller can decide whether to fall back to a free path or stall + /// the download in `Waiting` until the cooldown expires. + pub fn next_account( + &self, + service_name: &str, + strategy: AccountSelectionStrategy, + ) -> Result { + let now_ms = self.now_ms(); + let exhausted_ids = self.snapshot_exhausted(now_ms)?; + let picked = self + .selector + .select_best_excluding(service_name, strategy, &exhausted_ids)?; + if let Some(account) = picked { + return Ok(NextAccountOutcome::Picked(account)); + } + // No pick. Decide between NoneAvailable and AllExhausted by + // looking at the repo directly: if there's at least one + // enabled, non-expired account for this service, the rotation + // is what is blocking the caller, not the absence of credentials. + let candidates = self.repo.list_by_service(service_name)?; + let live: Vec<&Account> = candidates + .iter() + .filter(|a| a.is_enabled() && !a.is_expired(now_ms)) + .collect(); + if live.is_empty() { + return Ok(NextAccountOutcome::NoneAvailable); + } + let next_eligible_at_ms = self.earliest_deadline_for_service(&live, now_ms)?; + Ok(NextAccountOutcome::AllExhausted { + next_eligible_at_ms, + }) + } + + /// Mark `account_id` as quota-exhausted for `ttl_secs` seconds. + /// Callers pass a hoster-specific cooldown (typical range: a few + /// hundred seconds for free plans, longer for daily caps). Emits + /// [`DomainEvent::AccountExhausted`]. + pub fn mark_exhausted( + &self, + account_id: &AccountId, + service_name: &str, + ttl_secs: u64, + ) -> Result<(), AppError> { + let now_ms = self.now_ms(); + let until_ms = now_ms.saturating_add(ttl_secs.saturating_mul(1_000)); + { + let mut guard = self.lock_exhausted()?; + guard.insert(account_id.clone(), until_ms); + } + self.event_bus.publish(DomainEvent::AccountExhausted { + id: account_id.clone(), + service_name: service_name.to_string(), + exhausted_until_ms: until_ms, + }); + Ok(()) + } + + /// Drop any cooldown entry for `account_id` regardless of its + /// remaining TTL. Idempotent — calling on an unknown id is a + /// no-op. + pub fn clear_exhausted(&self, account_id: &AccountId) -> Result<(), AppError> { + let mut guard = self.lock_exhausted()?; + guard.remove(account_id); + Ok(()) + } + + /// `true` when `account_id` has an active cooldown at the current + /// clock reading. Expired entries are NOT pruned by this call — + /// pruning happens lazily inside `next_account` / + /// `snapshot_exhausted`. The check is read-only by design so it can + /// be called from log paths without surprising state changes. + pub fn is_exhausted(&self, account_id: &AccountId) -> Result { + let now_ms = self.now_ms(); + let guard = self.lock_exhausted()?; + Ok(guard + .get(account_id) + .is_some_and(|deadline| now_ms < *deadline)) + } + + /// Hoster-agnostic quota signal. Returns `true` when an HTTP + /// response should be treated as quota exhaustion: + /// + /// * `http_status == 429` (Too Many Requests — the unambiguous + /// quota signal) + /// * `traffic_left.is_some()` AND below `threshold_bytes` (the + /// remaining quota dropped under the configured floor) + /// + /// Hoster-specific patterns (e.g. body string `"daily limit"`) + /// belong in the plugin layer; the rotator stays generic. + pub fn is_quota_signal( + http_status: u16, + traffic_left: Option, + threshold_bytes: u64, + ) -> bool { + if http_status == 429 { + return true; + } + matches!(traffic_left, Some(left) if left < threshold_bytes) + } + + /// Reconcile a freshly observed `traffic_left` against the + /// exhaustion map. When the upstream confirms `traffic_left` is at + /// or above `threshold_bytes`, drop the cooldown so the next + /// `next_account` call can pick the account again. When the + /// observation is below the threshold OR `None` (unknown), the + /// cooldown is left untouched — `mark_exhausted` is the canonical + /// way to extend it. + pub fn record_traffic_refresh( + &self, + account_id: &AccountId, + traffic_left: Option, + threshold_bytes: u64, + ) -> Result<(), AppError> { + let confirms_available = matches!(traffic_left, Some(left) if left >= threshold_bytes); + if !confirms_available { + return Ok(()); + } + self.clear_exhausted(account_id) + } + + fn snapshot_exhausted(&self, now_ms: u64) -> Result, AppError> { + let mut guard = self.lock_exhausted()?; + guard.retain(|_, deadline| now_ms < *deadline); + Ok(guard.keys().cloned().collect()) + } + + fn earliest_deadline_for_service( + &self, + live_candidates: &[&Account], + now_ms: u64, + ) -> Result { + // Restrict the deadline scan to accounts that actually belong + // to the queried service so a parallel-service entry cannot + // leak its cooldown into an unrelated `AllExhausted` answer. + let guard = self.lock_exhausted()?; + let next = live_candidates + .iter() + .filter_map(|acc| guard.get(acc.id()).copied()) + .filter(|deadline| now_ms < *deadline) + .min() + .unwrap_or(now_ms); + Ok(next) + } + + fn lock_exhausted( + &self, + ) -> Result>, AppError> { + self.exhausted + .lock() + .map_err(|_| AppError::Validation("exhausted accounts mutex poisoned".to_string())) + } + + fn now_ms(&self) -> u64 { + self.clock.now_unix_secs().saturating_mul(1_000) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::application::services::AccountSelector; + use crate::domain::error::DomainError; + use crate::domain::model::account::{Account, AccountType}; + use crate::domain::ports::driven::AccountRepository; + use std::sync::Mutex as StdMutex; + + // --- Inline mocks (mirroring account_selector tests) --- + + struct InMemoryRepo { + accounts: StdMutex>, + } + + impl InMemoryRepo { + fn new(accounts: Vec) -> Self { + Self { + accounts: StdMutex::new(accounts), + } + } + } + + impl AccountRepository for InMemoryRepo { + fn find_by_id(&self, id: &AccountId) -> Result, DomainError> { + Ok(self + .accounts + .lock() + .unwrap() + .iter() + .find(|a| a.id() == id) + .cloned()) + } + + fn save(&self, account: &Account) -> Result<(), DomainError> { + let mut guard = self.accounts.lock().unwrap(); + if let Some(existing) = guard.iter_mut().find(|a| a.id() == account.id()) { + *existing = account.clone(); + } else { + guard.push(account.clone()); + } + Ok(()) + } + + fn list(&self) -> Result, DomainError> { + Ok(self.accounts.lock().unwrap().clone()) + } + + fn list_by_service(&self, service_name: &str) -> Result, DomainError> { + Ok(self + .accounts + .lock() + .unwrap() + .iter() + .filter(|a| a.service_name() == service_name) + .cloned() + .collect()) + } + + fn delete(&self, id: &AccountId) -> Result<(), DomainError> { + self.accounts.lock().unwrap().retain(|a| a.id() != id); + Ok(()) + } + } + + type EventSubscriber = Box; + + struct CollectingBus { + events: StdMutex>, + subscribers: StdMutex>, + } + + impl CollectingBus { + fn new() -> Self { + Self { + events: StdMutex::new(Vec::new()), + subscribers: StdMutex::new(Vec::new()), + } + } + + fn events(&self) -> Vec { + self.events.lock().unwrap().clone() + } + } + + impl EventBus for CollectingBus { + fn publish(&self, event: DomainEvent) { + self.events.lock().unwrap().push(event.clone()); + for handler in self.subscribers.lock().unwrap().iter() { + handler(&event); + } + } + + fn subscribe(&self, handler: Box) { + self.subscribers.lock().unwrap().push(handler); + } + } + + /// Mutable clock that tests advance manually so we never rely on + /// `std::time::Instant` (which would couple tests to wall-clock). + struct TestClock { + now_secs: StdMutex, + } + + impl TestClock { + fn new(now_secs: u64) -> Arc { + Arc::new(Self { + now_secs: StdMutex::new(now_secs), + }) + } + + fn advance_secs(&self, delta: u64) { + let mut g = self.now_secs.lock().unwrap(); + *g = g.saturating_add(delta); + } + } + + impl Clock for TestClock { + fn now_unix_secs(&self) -> u64 { + *self.now_secs.lock().unwrap() + } + } + + fn account(id: &str, service: &str, traffic_left: Option, enabled: bool) -> Account { + Account::reconstruct( + AccountId::new(id), + service.to_string(), + format!("user-{id}"), + AccountType::Premium, + enabled, + traffic_left, + None, + // Far in the future so `is_expired` never fires in these + // tests — exhaustion logic is the focus, not expiry. + Some(u64::MAX), + Some(0), + 0, + ) + } + + fn build_rotator( + accounts: Vec, + clock_secs: u64, + ) -> (Arc, Arc, Arc) { + let repo: Arc = Arc::new(InMemoryRepo::new(accounts)); + let bus = Arc::new(CollectingBus::new()); + let clock = TestClock::new(clock_secs); + let selector = AccountSelector::new(repo.clone(), bus.clone(), clock.clone()); + let rotator = AccountRotator::new(selector, repo, bus.clone(), clock.clone()); + (rotator, bus, clock) + } + + // --- AC #1: 429 → rotation vers 2ème account visible --- + #[test] + fn test_mark_exhausted_routes_next_account_to_remaining_candidate() { + let a = account("a", "Uploaded", Some(50_000_000_000), true); + let b = account("b", "Uploaded", Some(40_000_000_000), true); + let (rotator, bus, _clock) = build_rotator(vec![a, b], 1_700_000_000); + + let first = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match first { + NextAccountOutcome::Picked(acc) => assert_eq!(acc.id().as_str(), "a"), + other => panic!("expected Picked(a), got {other:?}"), + } + + rotator + .mark_exhausted(&AccountId::new("a"), "Uploaded", 600) + .unwrap(); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + let second = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match second { + NextAccountOutcome::Picked(acc) => assert_eq!(acc.id().as_str(), "b"), + other => panic!("expected Picked(b), got {other:?}"), + } + + let events = bus.events(); + assert!(events.iter().any(|e| matches!( + e, + DomainEvent::AccountExhausted { id, service_name, exhausted_until_ms: _ } + if id.as_str() == "a" && service_name == "Uploaded" + ))); + } + + // --- AC #2: tous accounts 429 → AllExhausted --- + #[test] + fn test_next_account_returns_all_exhausted_when_every_candidate_is_marked() { + let a = account("a", "Uploaded", Some(50), true); + let b = account("b", "Uploaded", Some(40), true); + let (rotator, _bus, _clock) = build_rotator(vec![a, b], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "Uploaded", 600) + .unwrap(); + rotator + .mark_exhausted(&AccountId::new("b"), "Uploaded", 1200) + .unwrap(); + + let outcome = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match outcome { + NextAccountOutcome::AllExhausted { + next_eligible_at_ms, + } => { + let now_ms = 1_700_000_000_u64.saturating_mul(1_000); + let earliest = now_ms.saturating_add(600 * 1_000); + assert_eq!( + next_eligible_at_ms, earliest, + "must report the EARLIEST cooldown deadline" + ); + } + other => panic!("expected AllExhausted, got {other:?}"), + } + } + + #[test] + fn test_next_account_returns_none_available_when_service_has_no_account() { + let (rotator, _bus, _clock) = build_rotator(vec![], 1_700_000_000); + let outcome = rotator + .next_account("UnknownService", AccountSelectionStrategy::BestTraffic) + .unwrap(); + assert_eq!(outcome, NextAccountOutcome::NoneAvailable); + } + + #[test] + fn test_next_account_returns_none_available_when_only_disabled_accounts() { + let a = account("a", "Uploaded", Some(50), false); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + let outcome = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + assert_eq!(outcome, NextAccountOutcome::NoneAvailable); + } + + // --- AC #3: reset après refresh confirme dispo --- + #[test] + fn test_record_traffic_refresh_clears_cooldown_when_confirms_available() { + let a = account("a", "Uploaded", Some(50), true); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "Uploaded", 600) + .unwrap(); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + // Refresh observes plenty of traffic available → clear. + rotator + .record_traffic_refresh(&AccountId::new("a"), Some(50_000_000), 1_000) + .unwrap(); + assert!(!rotator.is_exhausted(&AccountId::new("a")).unwrap()); + } + + #[test] + fn test_record_traffic_refresh_keeps_cooldown_when_below_threshold() { + let a = account("a", "Uploaded", Some(50), true); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "Uploaded", 600) + .unwrap(); + // Refresh observes traffic STILL below threshold → keep + // cooldown, do not flip-flop. + rotator + .record_traffic_refresh(&AccountId::new("a"), Some(500), 1_000) + .unwrap(); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + } + + #[test] + fn test_record_traffic_refresh_keeps_cooldown_when_traffic_unknown() { + // `traffic_left == None` is the "unknown" case (e.g. hoster + // does not expose a counter). The rotator must NOT clear the + // cooldown on a None observation — that would silently undo + // every `mark_exhausted` for hosters with no traffic API. + let a = account("a", "S", None, true); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "S", 600) + .unwrap(); + rotator + .record_traffic_refresh(&AccountId::new("a"), None, 1_000) + .unwrap(); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + } + + #[test] + fn test_clear_exhausted_drops_cooldown_explicitly() { + let a = account("a", "S", Some(50), true); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + rotator + .mark_exhausted(&AccountId::new("a"), "S", 600) + .unwrap(); + rotator.clear_exhausted(&AccountId::new("a")).unwrap(); + assert!(!rotator.is_exhausted(&AccountId::new("a")).unwrap()); + } + + #[test] + fn test_clear_exhausted_is_noop_for_unknown_id() { + let (rotator, _bus, _clock) = build_rotator(vec![], 1_700_000_000); + rotator + .clear_exhausted(&AccountId::new("ghost")) + .expect("clearing an unknown id is a no-op, not an error"); + } + + #[test] + fn test_cooldown_expires_after_ttl_so_account_picks_back_up() { + let a = account("a", "S", Some(50), true); + let (rotator, _bus, clock) = build_rotator(vec![a], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "S", 60) + .unwrap(); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + clock.advance_secs(61); + assert!(!rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + let outcome = rotator + .next_account("S", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match outcome { + NextAccountOutcome::Picked(acc) => assert_eq!(acc.id().as_str(), "a"), + other => panic!("expected Picked(a) after cooldown, got {other:?}"), + } + } + + #[test] + fn test_is_quota_signal_detects_429_regardless_of_traffic() { + assert!(AccountRotator::is_quota_signal(429, None, 1_000)); + assert!(AccountRotator::is_quota_signal(429, Some(u64::MAX), 1_000)); + } + + #[test] + fn test_is_quota_signal_detects_traffic_below_threshold() { + assert!(AccountRotator::is_quota_signal(200, Some(500), 1_000)); + assert!(AccountRotator::is_quota_signal(200, Some(0), 1_000)); + } + + #[test] + fn test_is_quota_signal_ignores_normal_responses_above_threshold() { + assert!(!AccountRotator::is_quota_signal(200, Some(2_000), 1_000)); + assert!(!AccountRotator::is_quota_signal(200, None, 1_000)); + assert!(!AccountRotator::is_quota_signal(404, None, 1_000)); + assert!(!AccountRotator::is_quota_signal(500, Some(50_000), 1_000)); + } + + #[test] + fn test_is_quota_signal_threshold_is_exclusive_at_equality() { + // Equal traffic vs threshold should NOT trip the signal — + // matches the "below threshold" copy in PRD §6.4. This freezes + // the rule so a future change cannot quietly invert it. + assert!(!AccountRotator::is_quota_signal(200, Some(1_000), 1_000)); + } + + /// Integration-flavour test: simulate the full quota detection → + /// rotation flow as the calling plugin would orchestrate it. + /// `is_quota_signal` decides exhaustion → `mark_exhausted` → + /// `next_account` returns the second candidate. Primary is sized + /// with more traffic so `BestTraffic` picks it first. + #[test] + fn test_quota_detection_to_rotation_full_flow() { + let a = account("primary", "Uploaded", Some(50_000_000), true); + let b = account("backup", "Uploaded", Some(500), true); + let (rotator, bus, _clock) = build_rotator(vec![a, b], 1_700_000_000); + + // Step 1: caller picks the primary (more traffic wins). + let first = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + let primary = match first { + NextAccountOutcome::Picked(acc) => acc, + other => panic!("expected Picked(primary), got {other:?}"), + }; + assert_eq!(primary.id().as_str(), "primary"); + + // Step 2: hoster responds 429 → caller checks the heuristic. + // Pass `Some(0)` to mimic a real "exhausted on the wire" case; + // the 429 alone is enough but exercising the traffic branch + // makes the assertion robust against future rule changes. + let exhausted = AccountRotator::is_quota_signal(429, Some(0), 1_000); + assert!(exhausted); + + // Step 3: caller marks it exhausted with a hoster-supplied TTL. + rotator + .mark_exhausted(primary.id(), primary.service_name(), 300) + .unwrap(); + + // Step 4: rotator now picks the backup. + let second = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match second { + NextAccountOutcome::Picked(acc) => assert_eq!(acc.id().as_str(), "backup"), + other => panic!("expected Picked(backup), got {other:?}"), + } + + let event_count = bus + .events() + .iter() + .filter(|e| matches!(e, DomainEvent::AccountExhausted { .. })) + .count(); + assert_eq!( + event_count, 1, + "exactly one AccountExhausted should have been emitted" + ); + } + + #[test] + fn test_mark_exhausted_handles_zero_ttl_gracefully() { + // A zero TTL is degenerate but not a bug; the rotator must not + // panic. The deadline equals `now_ms`, which `is_exhausted` + // treats as "just elapsed" (deadline is exclusive). + let a = account("a", "S", Some(50), true); + let (rotator, _bus, _clock) = build_rotator(vec![a], 1_700_000_000); + + rotator + .mark_exhausted(&AccountId::new("a"), "S", 0) + .unwrap(); + assert!( + !rotator.is_exhausted(&AccountId::new("a")).unwrap(), + "ttl=0 means the cooldown has already expired at now" + ); + } + + /// PRD §6.4 freezes the human-facing message format. Callers that + /// translate `AllExhausted` into a `Download.error` rely on this + /// exact wording so the UI / notification text stays uniform across + /// hosters. + #[test] + fn test_outcome_error_message_uses_prd_wording() { + let outcome = NextAccountOutcome::AllExhausted { + next_eligible_at_ms: 1_700_000_000_000, + }; + assert_eq!( + outcome.error_message("Uploaded"), + Some("All accounts exhausted for Uploaded".to_string()), + ); + + let none = NextAccountOutcome::NoneAvailable; + assert_eq!( + none.error_message("Mediafire"), + Some("No account available for Mediafire".to_string()), + ); + + let a = account("a", "S", Some(1), true); + let picked = NextAccountOutcome::Picked(a); + assert_eq!( + picked.error_message("S"), + None, + "Picked is the success path — no error message" + ); + } + + #[test] + fn test_all_exhausted_deadline_uses_only_this_services_accounts() { + let primary = account("a", "Uploaded", Some(50), true); + let cross_service = account("b", "Mediafire", Some(50), true); + let (rotator, _bus, _clock) = build_rotator(vec![primary, cross_service], 1_700_000_000); + + // Mark BOTH exhausted but with very different deadlines. + rotator + .mark_exhausted(&AccountId::new("a"), "Uploaded", 100) + .unwrap(); + rotator + .mark_exhausted(&AccountId::new("b"), "Mediafire", 99_999) + .unwrap(); + + let outcome = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + match outcome { + NextAccountOutcome::AllExhausted { + next_eligible_at_ms, + } => { + let now_ms = 1_700_000_000_u64 * 1_000; + assert_eq!( + next_eligible_at_ms, + now_ms + 100 * 1_000, + "Mediafire's longer cooldown must NOT leak into Uploaded's deadline" + ); + } + other => panic!("expected AllExhausted, got {other:?}"), + } + } +} diff --git a/src-tauri/src/application/services/account_selector.rs b/src-tauri/src/application/services/account_selector.rs index 4b75bff..180a69f 100644 --- a/src-tauri/src/application/services/account_selector.rs +++ b/src-tauri/src/application/services/account_selector.rs @@ -31,7 +31,7 @@ use std::sync::{Arc, Mutex}; use crate::application::error::AppError; use crate::domain::event::DomainEvent; -use crate::domain::model::account::{Account, AccountSelectionStrategy}; +use crate::domain::model::account::{Account, AccountId, AccountSelectionStrategy}; use crate::domain::ports::driven::AccountRepository; use crate::domain::ports::driven::clock::Clock; use crate::domain::ports::driven::event_bus::EventBus; @@ -73,12 +73,32 @@ impl AccountSelector { &self, service_name: &str, strategy: AccountSelectionStrategy, + ) -> Result, AppError> { + self.select_best_excluding(service_name, strategy, &[]) + } + + /// Same contract as `select_best` but skips any account whose id is + /// listed in `exclude`. Used by `AccountRotator` to filter out + /// quota-exhausted accounts without persisting transient state in + /// the repository. + /// + /// Emits `NoAccountAvailable` only when the *post-exclude* eligible + /// set is empty — that mirrors the caller-facing semantics: from + /// the rotator's point of view "all eligible accounts are + /// exhausted" is operationally equivalent to "no account left". + pub fn select_best_excluding( + &self, + service_name: &str, + strategy: AccountSelectionStrategy, + exclude: &[AccountId], ) -> Result, AppError> { let candidates = self.repo.list_by_service(service_name)?; let now_ms = self.now_ms(); let eligible: Vec<&Account> = candidates .iter() - .filter(|a| a.is_enabled() && !a.is_expired(now_ms)) + .filter(|a| { + a.is_enabled() && !a.is_expired(now_ms) && !exclude.iter().any(|id| id == a.id()) + }) .collect(); if eligible.is_empty() { self.event_bus.publish(DomainEvent::NoAccountAvailable { @@ -615,6 +635,101 @@ mod tests { assert!(r2.is_none(), "case-mismatched service name has no rows"); } + // --- Acceptance criterion: rotation-friendly exclude list --- + #[test] + fn test_select_best_excluding_skips_listed_ids_and_picks_next_best() { + let now_ms = 2_000_000_000_000; + let now_secs = now_ms / 1_000; + let a = account("a", "S", Some(50), Some(now_ms + 1), Some(now_ms - 1), true); + let b = account("b", "S", Some(40), Some(now_ms + 1), Some(now_ms - 1), true); + let c = account("c", "S", Some(30), Some(now_ms + 1), Some(now_ms - 1), true); + + let (selector, _bus) = build_selector(vec![a, b, c], now_secs); + + let p1 = selector + .select_best_excluding("S", AccountSelectionStrategy::BestTraffic, &[]) + .unwrap() + .unwrap(); + assert_eq!(p1.id().as_str(), "a", "no exclusion → top traffic wins"); + + let p2 = selector + .select_best_excluding( + "S", + AccountSelectionStrategy::BestTraffic, + &[AccountId::new("a")], + ) + .unwrap() + .unwrap(); + assert_eq!(p2.id().as_str(), "b"); + + let p3 = selector + .select_best_excluding( + "S", + AccountSelectionStrategy::BestTraffic, + &[AccountId::new("a"), AccountId::new("b")], + ) + .unwrap() + .unwrap(); + assert_eq!(p3.id().as_str(), "c"); + } + + #[test] + fn test_select_best_excluding_emits_no_account_when_all_excluded() { + let now_ms = 2_000_000_000_000; + let now_secs = now_ms / 1_000; + let a = account("a", "S", Some(50), Some(now_ms + 1), None, true); + let b = account("b", "S", Some(40), Some(now_ms + 1), None, true); + + let (selector, bus) = build_selector(vec![a, b], now_secs); + + let chosen = selector + .select_best_excluding( + "S", + AccountSelectionStrategy::BestTraffic, + &[AccountId::new("a"), AccountId::new("b")], + ) + .unwrap(); + assert!(chosen.is_none()); + + let events = bus.events(); + assert!(events.iter().any(|e| matches!( + e, + DomainEvent::NoAccountAvailable { service_name } if service_name == "S" + ))); + assert!( + !events + .iter() + .any(|e| matches!(e, DomainEvent::AccountSelected { .. })), + "must NOT emit AccountSelected when nothing was picked" + ); + } + + #[test] + fn test_select_best_excluding_round_robin_skips_excluded_and_alternates() { + let now_ms = 2_000_000_000_000; + let now_secs = now_ms / 1_000; + let a = account("acc-1", "S", Some(100), Some(now_ms + 1), None, true); + let b = account("acc-2", "S", Some(100), Some(now_ms + 1), None, true); + let c = account("acc-3", "S", Some(100), Some(now_ms + 1), None, true); + + let (selector, _bus) = build_selector(vec![a, b, c], now_secs); + + let exclude = vec![AccountId::new("acc-2")]; + let mut picked = Vec::new(); + for _ in 0..4 { + let chosen = selector + .select_best_excluding("S", AccountSelectionStrategy::RoundRobin, &exclude) + .unwrap() + .unwrap(); + picked.push(chosen.id().as_str().to_string()); + } + assert_eq!( + picked, + vec!["acc-1", "acc-3", "acc-1", "acc-3"], + "round-robin must alternate over the un-excluded subset" + ); + } + /// CodeRabbit / cubic-flagged regression: a poisoned `rr_cursor` /// mutex used to fold into `Ok(None)` because `lock().ok()?` swallowed /// the `PoisonError`. The contract reserves `Ok(None)` for diff --git a/src-tauri/src/application/services/mod.rs b/src-tauri/src/application/services/mod.rs index 947469c..9e94dcb 100644 --- a/src-tauri/src/application/services/mod.rs +++ b/src-tauri/src/application/services/mod.rs @@ -1,3 +1,4 @@ +pub mod account_rotator; pub mod account_selector; pub mod checksum_validator; pub mod engine_config_bridge; @@ -6,6 +7,7 @@ pub mod queue_config_bridge; pub mod queue_manager; pub mod startup_recovery; +pub use account_rotator::AccountRotator; pub use account_selector::AccountSelector; pub use checksum_validator::{ChecksumOutcome, ChecksumValidatorService}; pub use engine_config_bridge::subscribe_engine_to_config; diff --git a/src-tauri/src/domain/event.rs b/src-tauri/src/domain/event.rs index 9ef33f4..e6480e3 100644 --- a/src-tauri/src/domain/event.rs +++ b/src-tauri/src/domain/event.rs @@ -291,6 +291,18 @@ pub enum DomainEvent { /// One of `"best_traffic"`, `"round_robin"`, `"manual"`. strategy: String, }, + /// Emitted by `AccountRotator::mark_exhausted` when a hoster signals + /// quota exhaustion (HTTP 429, low `traffic_left`, …) so the account + /// is taken out of the rotation until the cooldown expires or the + /// next traffic refresh confirms availability. Carries `service_name` + /// so the UI can group notifications per hoster. + AccountExhausted { + id: AccountId, + service_name: String, + /// Unix epoch milliseconds — deadline after which the rotator + /// will consider the account eligible again. + exhausted_until_ms: u64, + }, } #[cfg(test)] diff --git a/src-tauri/src/domain/model/account.rs b/src-tauri/src/domain/model/account.rs index f786ab9..06aac74 100644 --- a/src-tauri/src/domain/model/account.rs +++ b/src-tauri/src/domain/model/account.rs @@ -115,6 +115,12 @@ pub struct Account { valid_until: Option, last_validated: Option, created_at: u64, + /// Transient quota-exhaustion deadline (Unix epoch ms). Set by the + /// `AccountRotator` when the upstream signals quota exhaustion + /// (HTTP 429, traffic below threshold, …) and cleared when a + /// fresh traffic refresh confirms the account is usable again. + /// NOT persisted in SQLite — always `None` after `reconstruct`. + exhausted_until: Option, } impl Account { @@ -136,6 +142,7 @@ impl Account { valid_until: None, last_validated: None, created_at, + exhausted_until: None, } } @@ -163,6 +170,7 @@ impl Account { valid_until, last_validated, created_at, + exhausted_until: None, } } @@ -208,6 +216,35 @@ impl Account { } } + /// Mark this account as quota-exhausted until `until_ms` (Unix epoch + /// ms). Transient — never persisted in SQLite. + pub fn mark_exhausted(&mut self, until_ms: u64) { + self.exhausted_until = Some(until_ms); + } + + /// Drop any pending quota-exhaustion marker, regardless of the + /// remaining cooldown. + pub fn clear_exhausted(&mut self) { + self.exhausted_until = None; + } + + /// Active quota-exhaustion deadline (Unix epoch ms) when set, else + /// `None`. The marker is informational; expiration is decided by + /// `is_exhausted(now)`. + pub fn exhausted_until(&self) -> Option { + self.exhausted_until + } + + /// `true` when the exhaustion marker is active at `now` (Unix epoch + /// ms). Mirrors `is_expired`: the deadline is exclusive — exactly + /// at `now == until` the cooldown is considered just elapsed. + pub fn is_exhausted(&self, now: u64) -> bool { + match self.exhausted_until { + Some(until) => now < until, + None => false, + } + } + /// Reference used to look up the credential in the system keyring. /// Format: `keyring://{service_name}/{username}`. Both segments are /// percent-encoded so reserved characters (`/`, `?`, `#`, `@`...) cannot @@ -491,4 +528,55 @@ mod tests { assert_eq!(acc.last_validated(), Some(101)); assert_eq!(acc.created_at(), 42); } + + #[test] + fn test_account_new_has_no_exhaustion_marker() { + let acc = make_account(); + assert!(acc.exhausted_until().is_none()); + assert!(!acc.is_exhausted(0)); + assert!(!acc.is_exhausted(u64::MAX)); + } + + #[test] + fn test_account_reconstruct_resets_exhausted_marker_to_none() { + // Transient state must NOT survive a reload from SQLite — the + // rotator owns the lifetime in memory. + let acc = Account::reconstruct( + AccountId::new("k"), + "Host".to_string(), + "u".to_string(), + AccountType::Premium, + true, + None, + None, + None, + None, + 0, + ); + assert!(acc.exhausted_until().is_none()); + } + + #[test] + fn test_mark_exhausted_records_deadline_and_flips_is_exhausted() { + let mut acc = make_account(); + acc.mark_exhausted(1_000); + assert_eq!(acc.exhausted_until(), Some(1_000)); + assert!(acc.is_exhausted(0)); + assert!(acc.is_exhausted(999)); + assert!( + !acc.is_exhausted(1_000), + "deadline is exclusive — at exact equality cooldown is over" + ); + assert!(!acc.is_exhausted(1_001)); + } + + #[test] + fn test_clear_exhausted_drops_marker_regardless_of_clock() { + let mut acc = make_account(); + acc.mark_exhausted(u64::MAX); + assert!(acc.is_exhausted(0)); + acc.clear_exhausted(); + assert!(acc.exhausted_until().is_none()); + assert!(!acc.is_exhausted(0)); + } } From 2ceeeffdbf8a97ec3095f25eaf1537337200d4c3 Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:14:34 +0200 Subject: [PATCH 2/6] fix(account): suppress NoAccountAvailable on exclusion + race-check rotator pick - Selector: only emit NoAccountAvailable when pre-exclude eligible set is empty. Rotator-driven exclusion of cooled-down accounts maps to AllExhausted upstream and must not collapse into "no account configured". - Rotator: re-check picked AccountId under lock and retry with id added to exclude list when a parallel mark_exhausted lands in the snapshot/select gap. Both findings raised by Codex + CodeRabbit on PR #129. Tests added/updated. --- .../application/services/account_rotator.rs | 29 +++++++-- .../application/services/account_selector.rs | 63 +++++++++++++++---- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs index 436d219..0822347 100644 --- a/src-tauri/src/application/services/account_rotator.rs +++ b/src-tauri/src/application/services/account_rotator.rs @@ -111,12 +111,29 @@ impl AccountRotator { strategy: AccountSelectionStrategy, ) -> Result { let now_ms = self.now_ms(); - let exhausted_ids = self.snapshot_exhausted(now_ms)?; - let picked = self - .selector - .select_best_excluding(service_name, strategy, &exhausted_ids)?; - if let Some(account) = picked { - return Ok(NextAccountOutcome::Picked(account)); + let mut exhausted_ids = self.snapshot_exhausted(now_ms)?; + // Linearise with concurrent `mark_exhausted`: snapshot is taken + // before the selector runs, so a parallel exhaustion landing in + // the gap could otherwise leak a stale account back to the + // caller. Re-check under the lock after each pick and retry + // with the offending id added to the exclude list. + loop { + let picked = + self.selector + .select_best_excluding(service_name, strategy, &exhausted_ids)?; + let Some(account) = picked else { + break; + }; + let still_available = { + let guard = self.lock_exhausted()?; + guard + .get(account.id()) + .is_none_or(|deadline| now_ms >= *deadline) + }; + if still_available { + return Ok(NextAccountOutcome::Picked(account)); + } + exhausted_ids.push(account.id().clone()); } // No pick. Decide between NoneAvailable and AllExhausted by // looking at the repo directly: if there's at least one diff --git a/src-tauri/src/application/services/account_selector.rs b/src-tauri/src/application/services/account_selector.rs index 180a69f..f0da8c6 100644 --- a/src-tauri/src/application/services/account_selector.rs +++ b/src-tauri/src/application/services/account_selector.rs @@ -94,16 +94,25 @@ impl AccountSelector { ) -> Result, AppError> { let candidates = self.repo.list_by_service(service_name)?; let now_ms = self.now_ms(); - let eligible: Vec<&Account> = candidates + let base_eligible: Vec<&Account> = candidates .iter() - .filter(|a| { - a.is_enabled() && !a.is_expired(now_ms) && !exclude.iter().any(|id| id == a.id()) - }) + .filter(|a| a.is_enabled() && !a.is_expired(now_ms)) + .collect(); + let eligible: Vec<&Account> = base_eligible + .iter() + .copied() + .filter(|a| !exclude.iter().any(|id| id == a.id())) .collect(); if eligible.is_empty() { - self.event_bus.publish(DomainEvent::NoAccountAvailable { - service_name: service_name.to_string(), - }); + // Only emit NoAccountAvailable when the pre-exclude set is + // empty: rotator-driven exclusion of cooled-down accounts + // is reported as AllExhausted upstream and must not be + // collapsed into "no account configured". + if base_eligible.is_empty() { + self.event_bus.publish(DomainEvent::NoAccountAvailable { + service_name: service_name.to_string(), + }); + } return Ok(None); } let chosen = match strategy { @@ -674,7 +683,12 @@ mod tests { } #[test] - fn test_select_best_excluding_emits_no_account_when_all_excluded() { + fn test_select_best_excluding_does_not_emit_no_account_when_only_exclusion_empties_set() { + // Pre-exclude eligible set is non-empty (a + b enabled, + // non-expired). Excluding both still yields Ok(None) but the + // event must NOT fire — that signal is reserved for "no + // configured/eligible account at all". Rotator path + // re-classifies as `AllExhausted`. let now_ms = 2_000_000_000_000; let now_secs = now_ms / 1_000; let a = account("a", "S", Some(50), Some(now_ms + 1), None, true); @@ -692,10 +706,13 @@ mod tests { assert!(chosen.is_none()); let events = bus.events(); - assert!(events.iter().any(|e| matches!( - e, - DomainEvent::NoAccountAvailable { service_name } if service_name == "S" - ))); + assert!( + !events.iter().any(|e| matches!( + e, + DomainEvent::NoAccountAvailable { service_name } if service_name == "S" + )), + "must NOT emit NoAccountAvailable when only exclusion emptied the set" + ); assert!( !events .iter() @@ -704,6 +721,28 @@ mod tests { ); } + #[test] + fn test_select_best_excluding_emits_no_account_when_pre_exclude_set_is_empty() { + // No enabled, non-expired account exists at all. Even with an + // empty exclude list the selector must signal NoAccountAvailable. + let now_ms = 2_000_000_000_000; + let now_secs = now_ms / 1_000; + let disabled = account("d", "S", Some(50), Some(now_ms + 1), None, false); + + let (selector, bus) = build_selector(vec![disabled], now_secs); + + let chosen = selector + .select_best_excluding("S", AccountSelectionStrategy::BestTraffic, &[]) + .unwrap(); + assert!(chosen.is_none()); + + let events = bus.events(); + assert!(events.iter().any(|e| matches!( + e, + DomainEvent::NoAccountAvailable { service_name } if service_name == "S" + ))); + } + #[test] fn test_select_best_excluding_round_robin_skips_excluded_and_alternates() { let now_ms = 2_000_000_000_000; From 92d640c806a65e8a26c39091a94a85bdd04fcffa Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:25:10 +0200 Subject: [PATCH 3/6] fix(account): emit AccountSelected only on committed rotator pick Selector gains `select_best_excluding_quiet` (no AccountSelected emission). Rotator's retry loop calls the quiet variant and emits AccountSelected itself once a pick survives the under-lock recheck. Without this, a probe discarded by a parallel `mark_exhausted` would have leaked an `AccountSelected` event for an account never returned to the caller, polluting UI/telemetry. Found by Codex on PR #129. Tests added: - selector quiet variant must not publish AccountSelected - rotator emits exactly one AccountSelected per Picked outcome - rotator emits zero AccountSelected on NoneAvailable --- .../application/services/account_rotator.rs | 63 +++++++++++++++++- .../application/services/account_selector.rs | 64 ++++++++++++++++--- 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs index 0822347..9daee09 100644 --- a/src-tauri/src/application/services/account_rotator.rs +++ b/src-tauri/src/application/services/account_rotator.rs @@ -118,9 +118,11 @@ impl AccountRotator { // caller. Re-check under the lock after each pick and retry // with the offending id added to the exclude list. loop { - let picked = - self.selector - .select_best_excluding(service_name, strategy, &exhausted_ids)?; + let picked = self.selector.select_best_excluding_quiet( + service_name, + strategy, + &exhausted_ids, + )?; let Some(account) = picked else { break; }; @@ -131,6 +133,15 @@ impl AccountRotator { .is_none_or(|deadline| now_ms >= *deadline) }; if still_available { + // Emit AccountSelected only on the committed pick, not + // on probes that lose the race to a parallel + // `mark_exhausted`. Otherwise UI/telemetry would see + // "selected" for an account never returned to the caller. + self.event_bus.publish(DomainEvent::AccountSelected { + id: account.id().clone(), + service_name: service_name.to_string(), + strategy: strategy.to_string(), + }); return Ok(NextAccountOutcome::Picked(account)); } exhausted_ids.push(account.id().clone()); @@ -767,4 +778,50 @@ mod tests { other => panic!("expected AllExhausted, got {other:?}"), } } + + #[test] + fn test_next_account_emits_account_selected_exactly_once_on_picked() { + // Regression: rotator now drives `AccountSelected` emission + // itself (selector probes go via `_quiet`). The contract is + // "one Picked outcome = one AccountSelected event" — never + // zero, never several. + let a = account("only", "Uploaded", Some(1_000), true); + let (rotator, bus, _clock) = build_rotator(vec![a], 1_700_000_000); + + let outcome = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + assert!(matches!(outcome, NextAccountOutcome::Picked(_))); + + let selected_count = bus + .events() + .iter() + .filter(|e| matches!(e, DomainEvent::AccountSelected { .. })) + .count(); + assert_eq!( + selected_count, 1, + "rotator must emit exactly one AccountSelected per Picked outcome" + ); + } + + #[test] + fn test_next_account_does_not_emit_account_selected_on_none() { + // No accounts configured. Path returns NoneAvailable and + // must not produce an AccountSelected event (it would only + // be possible via the selector's old emission point, which + // moved into the rotator's commit branch). + let (rotator, bus, _clock) = build_rotator(vec![], 1_700_000_000); + + let outcome = rotator + .next_account("Uploaded", AccountSelectionStrategy::BestTraffic) + .unwrap(); + assert!(matches!(outcome, NextAccountOutcome::NoneAvailable)); + + let selected_count = bus + .events() + .iter() + .filter(|e| matches!(e, DomainEvent::AccountSelected { .. })) + .count(); + assert_eq!(selected_count, 0); + } } diff --git a/src-tauri/src/application/services/account_selector.rs b/src-tauri/src/application/services/account_selector.rs index f0da8c6..bc95717 100644 --- a/src-tauri/src/application/services/account_selector.rs +++ b/src-tauri/src/application/services/account_selector.rs @@ -91,6 +91,34 @@ impl AccountSelector { service_name: &str, strategy: AccountSelectionStrategy, exclude: &[AccountId], + ) -> Result, AppError> { + let account = self.select_best_excluding_quiet(service_name, strategy, exclude)?; + if let Some(ref acc) = account { + self.event_bus.publish(DomainEvent::AccountSelected { + id: acc.id().clone(), + service_name: service_name.to_string(), + strategy: strategy.to_string(), + }); + } + Ok(account) + } + + /// Same selection logic as [`Self::select_best_excluding`] but never + /// publishes [`DomainEvent::AccountSelected`]. Reserved for callers + /// (e.g. [`crate::application::services::AccountRotator`]) that + /// probe the selector inside a retry loop and only want the event + /// emitted once a pick is actually committed — otherwise discarded + /// probes leak misleading "selected" signals to UI/telemetry. + /// + /// `NoAccountAvailable` is still emitted under the same rules as + /// the public variant (pre-exclude eligible set empty), since that + /// signal is independent of whether a pick survives downstream + /// post-checks. + pub fn select_best_excluding_quiet( + &self, + service_name: &str, + strategy: AccountSelectionStrategy, + exclude: &[AccountId], ) -> Result, AppError> { let candidates = self.repo.list_by_service(service_name)?; let now_ms = self.now_ms(); @@ -123,15 +151,7 @@ impl AccountSelector { self.pick_round_robin(service_name, &eligible)? } }; - let account = chosen.cloned(); - if let Some(ref acc) = account { - self.event_bus.publish(DomainEvent::AccountSelected { - id: acc.id().clone(), - service_name: service_name.to_string(), - strategy: strategy.to_string(), - }); - } - Ok(account) + Ok(chosen.cloned()) } /// Returns the next round-robin candidate, or `None` when `eligible` @@ -743,6 +763,32 @@ mod tests { ))); } + #[test] + fn test_select_best_excluding_quiet_does_not_emit_account_selected() { + // The _quiet variant exists for the rotator's retry probes: + // it must return the same account as the public variant but + // never publish AccountSelected. Discarded probes would + // otherwise leak misleading "selected" telemetry. + let now_ms = 2_000_000_000_000; + let now_secs = now_ms / 1_000; + let a = account("a", "S", Some(50), Some(now_ms + 1), None, true); + + let (selector, bus) = build_selector(vec![a], now_secs); + + let chosen = selector + .select_best_excluding_quiet("S", AccountSelectionStrategy::BestTraffic, &[]) + .unwrap(); + assert!(chosen.is_some()); + + let events = bus.events(); + assert!( + !events + .iter() + .any(|e| matches!(e, DomainEvent::AccountSelected { .. })), + "_quiet variant must not emit AccountSelected" + ); + } + #[test] fn test_select_best_excluding_round_robin_skips_excluded_and_alternates() { let now_ms = 2_000_000_000_000; From 7f8df6462bfb012ab81db6eec1b2353c3874a878 Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:01:41 +0200 Subject: [PATCH 4/6] fix(account): re-snapshot cooldown map before classifying AllExhausted If a parallel `clear_exhausted` / `record_traffic_refresh` lands while `next_account` is probing the selector, the original snapshot becomes stale. The loop now re-snapshots when the selector exhausts options and retries with the fresh exclude list whenever any baseline id was cleared. Bounded by the initial snapshot size so termination is guaranteed. Without this, a freshly-cleared account would be reported as `AllExhausted` even though it became selectable mid-call, sending callers down a needless waiting path. Found by Codex on PR #129. --- .../application/services/account_rotator.rs | 76 +++++++++++-------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs index 9daee09..c995273 100644 --- a/src-tauri/src/application/services/account_rotator.rs +++ b/src-tauri/src/application/services/account_rotator.rs @@ -111,45 +111,61 @@ impl AccountRotator { strategy: AccountSelectionStrategy, ) -> Result { let now_ms = self.now_ms(); - let mut exhausted_ids = self.snapshot_exhausted(now_ms)?; - // Linearise with concurrent `mark_exhausted`: snapshot is taken - // before the selector runs, so a parallel exhaustion landing in - // the gap could otherwise leak a stale account back to the - // caller. Re-check under the lock after each pick and retry - // with the offending id added to the exclude list. + let mut snapshot_baseline = self.snapshot_exhausted(now_ms)?; + let mut exhausted_ids = snapshot_baseline.clone(); + // Linearise with concurrent `mark_exhausted` / `clear_exhausted`: + // - On every pick, re-check the chosen id under the lock and + // retry with that id added to the exclude list when a + // parallel `mark_exhausted` landed in the gap. + // - When the selector exhausts options, re-snapshot the + // cooldown map and retry once more if a parallel clear (via + // `clear_exhausted` or `record_traffic_refresh`) freed an id + // that was in our snapshot baseline. Otherwise we'd return + // `AllExhausted` while a live account is in fact selectable. loop { let picked = self.selector.select_best_excluding_quiet( service_name, strategy, &exhausted_ids, )?; - let Some(account) = picked else { + if let Some(account) = picked { + let still_available = { + let guard = self.lock_exhausted()?; + guard + .get(account.id()) + .is_none_or(|deadline| now_ms >= *deadline) + }; + if still_available { + // Emit AccountSelected only on the committed pick, not + // on probes that lose the race to a parallel + // `mark_exhausted`. Otherwise UI/telemetry would see + // "selected" for an account never returned to the caller. + self.event_bus.publish(DomainEvent::AccountSelected { + id: account.id().clone(), + service_name: service_name.to_string(), + strategy: strategy.to_string(), + }); + return Ok(NextAccountOutcome::Picked(account)); + } + exhausted_ids.push(account.id().clone()); + continue; + } + // No pick under the current exclude list. Re-snapshot the + // cooldown map: if a parallel clear removed any id we were + // previously excluding, the selector may now find a live + // account. + let fresh = self.snapshot_exhausted(now_ms)?; + let any_cleared = snapshot_baseline.iter().any(|id| !fresh.contains(id)); + if !any_cleared { break; - }; - let still_available = { - let guard = self.lock_exhausted()?; - guard - .get(account.id()) - .is_none_or(|deadline| now_ms >= *deadline) - }; - if still_available { - // Emit AccountSelected only on the committed pick, not - // on probes that lose the race to a parallel - // `mark_exhausted`. Otherwise UI/telemetry would see - // "selected" for an account never returned to the caller. - self.event_bus.publish(DomainEvent::AccountSelected { - id: account.id().clone(), - service_name: service_name.to_string(), - strategy: strategy.to_string(), - }); - return Ok(NextAccountOutcome::Picked(account)); } - exhausted_ids.push(account.id().clone()); + snapshot_baseline = fresh.clone(); + exhausted_ids = fresh; } - // No pick. Decide between NoneAvailable and AllExhausted by - // looking at the repo directly: if there's at least one - // enabled, non-expired account for this service, the rotation - // is what is blocking the caller, not the absence of credentials. + // No pick after stable re-snapshot. Decide between NoneAvailable + // and AllExhausted by looking at the repo directly: if there's + // at least one enabled, non-expired account for this service, + // the rotation is the blocker, not the absence of credentials. let candidates = self.repo.list_by_service(service_name)?; let live: Vec<&Account> = candidates .iter() From 80ffd11585af554844d124c55e71c7bb788c4477 Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:15:15 +0200 Subject: [PATCH 5/6] fix(account): include race-pushed ids in stale-snapshot recheck `any_cleared` previously only diffed against the initial `snapshot_baseline`. IDs appended to `exhausted_ids` during the mark_exhausted race-retry path were ignored, so a parallel clear of one of those IDs would silently fall through to `AllExhausted` while a live account was actually selectable. Compare against the full current exclude list instead. Drop the separate `snapshot_baseline` since `exhausted_ids` already tracks both the initial snapshot and race-pushed entries. Found by Codex + CodeRabbit on PR #129. --- .../application/services/account_rotator.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs index c995273..6978d85 100644 --- a/src-tauri/src/application/services/account_rotator.rs +++ b/src-tauri/src/application/services/account_rotator.rs @@ -111,16 +111,16 @@ impl AccountRotator { strategy: AccountSelectionStrategy, ) -> Result { let now_ms = self.now_ms(); - let mut snapshot_baseline = self.snapshot_exhausted(now_ms)?; - let mut exhausted_ids = snapshot_baseline.clone(); + let mut exhausted_ids = self.snapshot_exhausted(now_ms)?; // Linearise with concurrent `mark_exhausted` / `clear_exhausted`: // - On every pick, re-check the chosen id under the lock and // retry with that id added to the exclude list when a // parallel `mark_exhausted` landed in the gap. // - When the selector exhausts options, re-snapshot the - // cooldown map and retry once more if a parallel clear (via - // `clear_exhausted` or `record_traffic_refresh`) freed an id - // that was in our snapshot baseline. Otherwise we'd return + // cooldown map and retry once more if any id from the full + // current exclude list (initial snapshot ∪ race-pushed ids) + // has since been cleared via `clear_exhausted` or + // `record_traffic_refresh`. Otherwise we'd return // `AllExhausted` while a live account is in fact selectable. loop { let picked = self.selector.select_best_excluding_quiet( @@ -151,15 +151,13 @@ impl AccountRotator { continue; } // No pick under the current exclude list. Re-snapshot the - // cooldown map: if a parallel clear removed any id we were - // previously excluding, the selector may now find a live - // account. + // cooldown map and retry if any id we were previously + // excluding (including race-pushed ones) has been cleared. let fresh = self.snapshot_exhausted(now_ms)?; - let any_cleared = snapshot_baseline.iter().any(|id| !fresh.contains(id)); + let any_cleared = exhausted_ids.iter().any(|id| !fresh.contains(id)); if !any_cleared { break; } - snapshot_baseline = fresh.clone(); exhausted_ids = fresh; } // No pick after stable re-snapshot. Decide between NoneAvailable From 2aa717b6860f447d1b95ef15423eed909d65381d Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:31:01 +0200 Subject: [PATCH 6/6] fix(account): preserve longer cooldown when re-marking exhausted `mark_exhausted` previously overwrote the existing deadline. A short retry-driven TTL (e.g. 60s) landing on top of a daily-cap cooldown (e.g. 600s) would shrink the window and put the account back into rotation early. Keep `max(existing, proposed)` and publish that committed value in `AccountExhausted` so subscribers see the active deadline, not a phantom shorter one. Found by CodeRabbit on PR #129. Test `test_mark_exhausted_keeps_existing_longer_deadline` pins the behaviour through clock advance. --- .../application/services/account_rotator.rs | 69 +++++++++++++++++-- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/application/services/account_rotator.rs b/src-tauri/src/application/services/account_rotator.rs index 6978d85..f46911c 100644 --- a/src-tauri/src/application/services/account_rotator.rs +++ b/src-tauri/src/application/services/account_rotator.rs @@ -181,7 +181,13 @@ impl AccountRotator { /// Mark `account_id` as quota-exhausted for `ttl_secs` seconds. /// Callers pass a hoster-specific cooldown (typical range: a few /// hundred seconds for free plans, longer for daily caps). Emits - /// [`DomainEvent::AccountExhausted`]. + /// [`DomainEvent::AccountExhausted`] carrying the committed deadline. + /// + /// If a cooldown entry already exists and its deadline is further + /// in the future than the proposed one, the existing deadline + /// wins. This prevents a short retry-driven TTL from accidentally + /// shortening a longer daily-cap cooldown set by a previous + /// signal. pub fn mark_exhausted( &self, account_id: &AccountId, @@ -189,15 +195,20 @@ impl AccountRotator { ttl_secs: u64, ) -> Result<(), AppError> { let now_ms = self.now_ms(); - let until_ms = now_ms.saturating_add(ttl_secs.saturating_mul(1_000)); - { + let proposed = now_ms.saturating_add(ttl_secs.saturating_mul(1_000)); + let committed = { let mut guard = self.lock_exhausted()?; - guard.insert(account_id.clone(), until_ms); - } + let final_deadline = match guard.get(account_id) { + Some(existing) if *existing > proposed => *existing, + _ => proposed, + }; + guard.insert(account_id.clone(), final_deadline); + final_deadline + }; self.event_bus.publish(DomainEvent::AccountExhausted { id: account_id.clone(), service_name: service_name.to_string(), - exhausted_until_ms: until_ms, + exhausted_until_ms: committed, }); Ok(()) } @@ -732,6 +743,52 @@ mod tests { ); } + #[test] + fn test_mark_exhausted_keeps_existing_longer_deadline() { + // A long cooldown (daily cap) followed by a short retry signal + // must not shrink the active cooldown. The committed deadline + // wins, and the AccountExhausted event publishes it verbatim + // so subscribers don't see a phantom shorter window. + let a = account("a", "S", Some(50), true); + let (rotator, bus, clock) = build_rotator(vec![a], 1_700_000_000); + let now_ms = 1_700_000_000_u64 * 1_000; + + rotator + .mark_exhausted(&AccountId::new("a"), "S", 600) + .unwrap(); + let long_deadline = now_ms + 600 * 1_000; + + rotator + .mark_exhausted(&AccountId::new("a"), "S", 60) + .unwrap(); + + // The shorter retry signal would expire after 60s. Advance + // past that and confirm the cooldown is still active — + // proving the longer (600s) deadline stuck. + clock.advance_secs(120); + assert!(rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + // Advance past the long deadline; cooldown finally clears. + clock.advance_secs(600); + assert!(!rotator.is_exhausted(&AccountId::new("a")).unwrap()); + + let payloads: Vec = bus + .events() + .iter() + .filter_map(|e| match e { + DomainEvent::AccountExhausted { + exhausted_until_ms, .. + } => Some(*exhausted_until_ms), + _ => None, + }) + .collect(); + assert_eq!( + payloads, + vec![long_deadline, long_deadline], + "second AccountExhausted must republish the still-active longer deadline, not the shorter proposed one" + ); + } + /// PRD §6.4 freezes the human-facing message format. Callers that /// translate `AllExhausted` into a `Download.error` rely on this /// exact wording so the UI / notification text stays uniform across