feat(account): auto-select best account per service#128
Merged
Conversation
Adds the `AccountSelector` application service so multiple accounts for
the same hoster / debrid service can be ranked and dispatched
deterministically. Three strategies are honoured from the live
`AppConfig::account_selection_strategy`:
- `BestTraffic` (default): rank `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, surviving cache invalidation so a hot
reload of the candidate set does not reset rotation.
- `Manual`: alias of `BestTraffic` until pinning UI lands. Frozen
by a regression test so a future change is deliberate.
Cache: per-service candidate snapshot is dropped in bulk on every
account-touching event (`AccountAdded` / `AccountUpdated` /
`AccountDeleted` / `AccountValidated` / `AccountValidationFailed` /
`AccountsImported`).
Domain events: new `NoAccountAvailable { service_name }` (emitted when
no candidate passes the filter, paired with `Ok(None)`) and
`AccountSelected { id, service_name, strategy }` (emitted on every
pick), both forwarded by the Tauri bridge as `no-account-available`
and `account-selected`.
CommandBus exposes `resolve_account_for(service_name)` so download /
link-grabber flows have a single entry point. `resolve_links` now
flags the planned hook in a comment but stops short of calling it
per-URL to avoid emitting `AccountSelected` once per probed link.
Config: new `account_selection_strategy` field on `AppConfig`,
`ConfigPatch` and `apply_patch`, plus the matching IPC and TOML
serialisation paths (snake_case `"best_traffic" | "round_robin" |
"manual"`).
Twelve unit tests cover the four PRD §6.4 acceptance criteria
(3-account scenario, all-expired surface, comparative ranking table,
round-robin alternance) plus cache invalidation and case-mismatch
behaviour. Unblocks task 25 (rotation auto sur quota).
Refs: PRD §6.4, PRD-v2 §P1.5
github-actions
Bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an AccountSelector service and AccountSelectionStrategy; threads selection through CommandBus; persists/parses strategy via IPC/TOML with strict validation and legacy compatibility; emits Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant CommandBus as CommandBus
participant ConfigStore as ConfigStore
participant AccountSelector as AccountSelector
participant AccountRepo as AccountRepository
participant EventBus as EventBus
participant TauriBridge as TauriEventBridge
Client->>CommandBus: resolve_account_for(service)
CommandBus->>ConfigStore: get_config()
ConfigStore-->>CommandBus: AppConfig (strategy)
CommandBus->>AccountSelector: select_best(service, strategy)
AccountSelector->>AccountRepo: list_by_service(service)
AccountRepo-->>AccountSelector: accounts
AccountSelector->>AccountSelector: filter & rank/select
alt Account Selected
AccountSelector->>EventBus: publish AccountSelected(id, service, strategy)
AccountSelector-->>CommandBus: Some(Account)
else No Eligible Account
AccountSelector->>EventBus: publish NoAccountAvailable(service)
AccountSelector-->>CommandBus: None
end
EventBus->>TauriBridge: forward event
TauriBridge->>Client: JSON payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e29ba8537a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src-tauri/src/domain/event.rs (1)
284-293: UseAccountSelectionStrategyhere instead of a rawString.This is a closed set already modeled in the domain. Keeping
strategyasStringmakes the event payload weaker than the source model and lets future publishers emit invalid values without compiler help. Prefer the enum here and stringify only at the adapter boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/domain/event.rs` around lines 284 - 293, The AccountSelected event currently uses a raw String for strategy; change the payload to use the enum AccountSelectionStrategy instead (replace the field signature strategy: String with strategy: AccountSelectionStrategy in the AccountSelected variant) and update any code that constructs this event (e.g., AccountSelector::select_best) to pass the enum value; keep string serialization only at the adapter/transport boundary by converting AccountSelectionStrategy to/from a string where events are serialized/deserialized so existing external formats remain unchanged.src-tauri/src/adapters/driven/event/tauri_bridge.rs (1)
71-72: Add regression coverage for the new account event bridge contract.These mappings are hand-written and frontend-facing, but the test module does not currently assert
"no-account-available"/"account-selected"or theserviceName/strategypayload keys. A small bridge test here would lock down the IPC contract you just introduced.Also applies to: 201-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/event/tauri_bridge.rs` around lines 71 - 72, The new IPC mapping for DomainEvent::NoAccountAvailable => "no-account-available" and DomainEvent::AccountSelected => "account-selected" needs unit tests in the module's test suite to lock the bridge contract: add tests that construct those DomainEvent variants, run them through the same bridge/serialization path used by the code that emits IPC events (the match/mapper that yields the event name string), assert the emitted event name equals "no-account-available" / "account-selected", and assert the serialized payload contains the frontend-facing keys "serviceName" and "strategy"; apply the same assertions for the other similar mappings in the same mapping block (the entries around the other event arms).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driving/tauri_ipc.rs`:
- Around line 1012-1015: The current conversion of account_selection_strategy
silently maps parse failures to None by using as_deref().and_then(|s|
s.parse().ok()); change this to a fallible parse that returns an explicit
validation error on unknown strings so settings_update can reject bad payloads.
Specifically, replace the .and_then(... .parse().ok()) flow in the
account_selection_strategy handling with a parse call that returns Result and
propagate or map its Err into a validation error (so settings_update returns an
error response) and update settings_update to handle that error path and return
a clear validation message for invalid accountSelectionStrategy values.
In `@src-tauri/src/application/command_bus.rs`:
- Around line 295-299: Currently the code swallows ConfigStore::get_config()
errors by falling back to AccountSelectionStrategy::DEFAULT; change to propagate
failures instead by replacing the map(...).unwrap_or(...) with a direct ?-based
extraction: e.g. let strategy =
self.config_store.get_config()?.account_selection_strategy; update the enclosing
function signatures to return Result (or propagate the error type) and adjust
callers to handle the propagated error so config read failures are surfaced
instead of silently using AccountSelectionStrategy::DEFAULT.
In `@src-tauri/src/application/services/account_selector.rs`:
- Around line 83-87: The current code calls guard.cache.clear() which evicts all
cached service entries; instead, locate the specific service key for the touched
account (the identifier used as the cache key in account_selector.rs) and remove
only that entry from the cache (e.g., use guard.cache.remove(&service_key) or
guard.cache.retain to drop just that key) inside the same weak_state.upgrade()
&& state.lock() block so only the affected service's cache is invalidated rather
than the whole cache.
---
Nitpick comments:
In `@src-tauri/src/adapters/driven/event/tauri_bridge.rs`:
- Around line 71-72: The new IPC mapping for DomainEvent::NoAccountAvailable =>
"no-account-available" and DomainEvent::AccountSelected => "account-selected"
needs unit tests in the module's test suite to lock the bridge contract: add
tests that construct those DomainEvent variants, run them through the same
bridge/serialization path used by the code that emits IPC events (the
match/mapper that yields the event name string), assert the emitted event name
equals "no-account-available" / "account-selected", and assert the serialized
payload contains the frontend-facing keys "serviceName" and "strategy"; apply
the same assertions for the other similar mappings in the same mapping block
(the entries around the other event arms).
In `@src-tauri/src/domain/event.rs`:
- Around line 284-293: The AccountSelected event currently uses a raw String for
strategy; change the payload to use the enum AccountSelectionStrategy instead
(replace the field signature strategy: String with strategy:
AccountSelectionStrategy in the AccountSelected variant) and update any code
that constructs this event (e.g., AccountSelector::select_best) to pass the enum
value; keep string serialization only at the adapter/transport boundary by
converting AccountSelectionStrategy to/from a string where events are
serialized/deserialized so existing external formats remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbbd2dbf-8f81-4599-8e9f-003aa075af58
📒 Files selected for processing (12)
CHANGELOG.mdsrc-tauri/src/adapters/driven/config/toml_config_store.rssrc-tauri/src/adapters/driven/event/tauri_bridge.rssrc-tauri/src/adapters/driven/logging/download_log_bridge.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/command_bus.rssrc-tauri/src/application/commands/resolve_links.rssrc-tauri/src/application/services/account_selector.rssrc-tauri/src/application/services/mod.rssrc-tauri/src/domain/event.rssrc-tauri/src/domain/model/account.rssrc-tauri/src/domain/model/config.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/adapters/driving/tauri_ipc.rs">
<violation number="1" location="src-tauri/src/adapters/driving/tauri_ipc.rs:1015">
P2: Invalid `account_selection_strategy` values are silently ignored instead of being rejected.</violation>
</file>
<file name="src-tauri/src/application/command_bus.rs">
<violation number="1" location="src-tauri/src/application/command_bus.rs:299">
P2: Do not swallow `get_config()` errors in `resolve_account_for`; falling back to default can silently apply the wrong selection strategy.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…pe cache eviction Addresses three review findings on PR #128: - `ConfigPatchDto` -> `ConfigPatch` is now `TryFrom` so an unknown `accountSelectionStrategy` value comes back as a validation error instead of being silently dropped to `None`. `settings_update` surfaces the parse failure to the caller. `ConfigPatchDto` now derives `Default` so tests can construct a single-field patch. - `CommandBus::resolve_account_for` propagates `ConfigStore::get_config()` failures via `?` instead of swallowing them with `unwrap_or(DEFAULT)`, so a corrupt or unreadable config no longer pretends `BestTraffic` when the user asked for `RoundRobin`/`Manual`. - `AccountSelector` cache invalidation is now scoped per service: `AccountAdded` evicts the slot named by its `service_name`, `AccountUpdated` / `AccountDeleted` / `AccountValidated` / `AccountValidationFailed` evict the slot whose snapshot already contained the affected `id`, and `AccountsImported` keeps the bulk clear. An event for service A no longer warms-up service B on its next pick. Six new tests pin the contract: `test_config_patch_dto_rejects_unknown_account_selection_strategy`, `test_config_patch_dto_accepts_known_account_selection_strategy`, `test_config_patch_dto_passes_through_when_strategy_is_none`, `test_resolve_account_for_propagates_config_store_failure`, `test_account_event_invalidates_only_touched_service`, `test_account_added_invalidates_only_target_service_cache`, `test_accounts_imported_invalidates_every_cache_slot`. `cargo test --lib` 1147 passed. `cargo clippy --lib -- -D warnings` clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8849b5c594
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driving/tauri_ipc.rs`:
- Line 963: Update the doc comment to reference the correct conversion trait:
replace the mention of ConfigPatch::from(ConfigPatchDto) with
ConfigPatch::try_from(ConfigPatchDto) (or mention TryFrom/try_from on
ConfigPatch for converting from ConfigPatchDto) so it accurately reflects that
unknown values are rejected via TryFrom rather than From; ensure the comment
names the types ConfigPatch and ConfigPatchDto and the TryFrom conversion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8077251b-5a9a-40fc-bb23-c48165c70839
📒 Files selected for processing (4)
CHANGELOG.mdsrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/command_bus.rssrc-tauri/src/application/services/account_selector.rs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
…x doc Addresses three follow-up review findings on PR #128: - `AccountSelector` no longer caches `list_by_service` results. The earlier event-driven invalidation raced with `TokioEventBus` — `bus.publish(AccountUpdated)` returns immediately while the subscriber dispatches on a `tokio::spawn`'d task, so a `select_best` call landing between publish and dispatch could still observe stale rows. SQLite reads are cheap for the row counts in scope (≤ a few dozen accounts per service), so the cache was trading correctness for negligible savings. The round-robin cursor stays — it is the only state the selector needs to keep. - `From<ConfigDto> for AppConfig` is now `TryFrom<…>` returning `DomainError`. A hand-edited `config.toml` carrying an unknown `account_selection_strategy` previously coerced to `best_traffic` via `unwrap_or(DEFAULT)`, masking corruption. The TOML store now surfaces the failure as `StorageError("invalid config: …")` so the runtime refuses to start with an invalid persisted strategy. - `ConfigPatchDto::account_selection_strategy` doc string now points at `ConfigPatch::try_from(ConfigPatchDto)` to match the actual contract introduced in 8849b5c. Six cache-mechanics tests in `account_selector.rs` collapsed into two that pin the new contract: `test_select_best_always_reflects_current_repo_state` and `test_select_best_is_case_sensitive_on_service_name`. New TOML store tests `test_get_config_rejects_unknown_persisted_account_selection_strategy` and `test_get_config_accepts_known_persisted_account_selection_strategy` pin the strict-parse path. `cargo test --lib` 1145 passed. `cargo clippy --lib -- -D warnings` clean. `cargo fmt --check` clean.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/application/services/account_selector.rs">
<violation number="1" location="src-tauri/src/application/services/account_selector.rs:112">
P2: Round-robin selection silently drops to `None` on mutex poison, which can return no account even when eligible accounts exist.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/application/services/account_selector.rs`:
- Around line 112-116: Change pick_round_robin to return
Result<Option<&Account>, AppError> instead of Option<&Account>: on lock failure
of self.rr_cursor (i.e., Mutex::lock()), convert the poisoned-lock error into
AppError::Validation("Round-robin cursor poisoned") and return Err(...) so
callers can distinguish real failures; keep the existing round-robin logic
(entry, cursor increment, selection) and wrap the final Some(pick) in
Ok(Some(pick)) and return Ok(None) when sorted.is_empty(). Then update the
caller in select_best to call pick_round_robin()? and propagate the Result (use
?), so a poisoned mutex yields Err(AppError::Validation(...)) instead of
silently returning Ok(None) and suppressing NoAccountAvailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe74b824-1448-44c0-b87c-77049636aed9
📒 Files selected for processing (4)
CHANGELOG.mdsrc-tauri/src/adapters/driven/config/toml_config_store.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/services/account_selector.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src-tauri/src/adapters/driven/config/toml_config_store.rs
- CHANGELOG.md
- src-tauri/src/adapters/driving/tauri_ipc.rs
Both CodeRabbit and cubic flagged that `pick_round_robin` swallowed mutex poisoning by writing `self.rr_cursor.lock().ok()?`, which folds the `PoisonError` into the `Option` short-circuit and returns `Ok(None)` from `select_best`. That value is reserved for "no eligible accounts" — when used for a poisoned cursor, callers cannot distinguish zero-candidate from internal corruption and the `NoAccountAvailable` event would not fire as the contract documents. `pick_round_robin` now returns `Result<Option<&Account>, AppError>` and maps `Mutex::lock()` failures to `AppError::Validation( "round-robin cursor mutex poisoned")`. `select_best` propagates with `?`. Regression test `test_pick_round_robin_returns_err_on_poisoned_cursor` deliberately poisons the cursor from a worker thread and asserts the `Validation` variant. `cargo test --lib` 1146 passed. `cargo clippy --lib -- -D warnings` clean. `cargo fmt --check` clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ae03739aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P1 finding: an existing `config.toml` written before task 24
introduced `account_selection_strategy` has no key for it.
`#[serde(default)]` on `ConfigDto` causes the missing field to
deserialize as `String::default()` (empty), and the strict `parse()`
in `TryFrom<ConfigDto> for AppConfig` would then reject every legacy
file at startup.
Treat the empty-string case as `AccountSelectionStrategy::DEFAULT` so
upgraded users keep working while non-empty typo / corruption values
still surface as `StorageError("invalid config: …")`. Regression test
`test_get_config_accepts_legacy_config_without_strategy_field` writes
a TOML carrying only `api_key` and asserts the loaded `AppConfig`
hydrates `BestTraffic`.
`cargo test --lib` 1147 passed. `cargo clippy --lib -- -D warnings`
clean. `cargo fmt --check` clean.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src-tauri/src/adapters/driven/config/toml_config_store.rs (2)
257-260: Consider trimming persisted strategy before validation.Line 257 and Line 260 currently treat
" round_robin "as invalid. Trimming before empty-check/parse improves resilience for hand-edited TOML without weakening unknown-value rejection.♻️ Suggested adjustment
- let account_selection_strategy: AccountSelectionStrategy = - if d.account_selection_strategy.is_empty() { + let raw_strategy = d.account_selection_strategy.trim(); + let account_selection_strategy: AccountSelectionStrategy = + if raw_strategy.is_empty() { AccountSelectionStrategy::DEFAULT } else { - d.account_selection_strategy.parse()? + raw_strategy.parse()? };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/config/toml_config_store.rs` around lines 257 - 260, The persisted account selection strategy value d.account_selection_strategy should be trimmed before validation so values like " round_robin " are accepted; update the logic around d.account_selection_strategy (used with AccountSelectionStrategy::DEFAULT and .parse()) to call .trim() (or equivalent) before checking is_empty() and before invoking parse(), ensuring you still propagate parse errors for unknown values.
663-679: Add one explicit empty-string regression test.This test validates a missing key path; the empty-string fallback branch at Line 257 is still unproven. Add a focused case with
account_selection_strategy = ""to lock in the intended legacy behavior.🧪 Suggested test case
+ #[test] + fn test_get_config_accepts_empty_strategy_string_as_default() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write( + &path, + "api_key = \"legacy-key\"\naccount_selection_strategy = \"\"\n", + ) + .unwrap(); + + let store = TomlConfigStore::new(path, None, Some("default-key".to_string())); + let config = store.get_config().expect("empty strategy should load as default"); + assert_eq!(config.account_selection_strategy, AccountSelectionStrategy::DEFAULT); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/config/toml_config_store.rs` around lines 663 - 679, Add a new unit test alongside test_get_config_accepts_legacy_config_without_strategy_field that writes a TOML file containing account_selection_strategy = "" and verifies TomlConfigStore::new(...).get_config() returns AccountSelectionStrategy::DEFAULT; this explicitly exercises the empty-string fallback branch (the legacy hydration path) so the empty-string value is treated the same as a missing field. Use the same tempfile setup pattern, call TomlConfigStore::new(path, None, Some("default-key".to_string())), call get_config() and assert config.account_selection_strategy == AccountSelectionStrategy::DEFAULT.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src-tauri/src/adapters/driven/config/toml_config_store.rs`:
- Around line 257-260: The persisted account selection strategy value
d.account_selection_strategy should be trimmed before validation so values like
" round_robin " are accepted; update the logic around
d.account_selection_strategy (used with AccountSelectionStrategy::DEFAULT and
.parse()) to call .trim() (or equivalent) before checking is_empty() and before
invoking parse(), ensuring you still propagate parse errors for unknown values.
- Around line 663-679: Add a new unit test alongside
test_get_config_accepts_legacy_config_without_strategy_field that writes a TOML
file containing account_selection_strategy = "" and verifies
TomlConfigStore::new(...).get_config() returns
AccountSelectionStrategy::DEFAULT; this explicitly exercises the empty-string
fallback branch (the legacy hydration path) so the empty-string value is treated
the same as a missing field. Use the same tempfile setup pattern, call
TomlConfigStore::new(path, None, Some("default-key".to_string())), call
get_config() and assert config.account_selection_strategy ==
AccountSelectionStrategy::DEFAULT.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e2216f9-2a21-4853-80df-fb796fb1472e
📒 Files selected for processing (2)
CHANGELOG.mdsrc-tauri/src/adapters/driven/config/toml_config_store.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
This was referenced Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
AccountSelectorapplication service to rank multiple accounts for the same hoster/debrid service deterministically. Supports three configurable strategies (BestTraffic, RoundRobin, Manual) read from live config. Unblocks task 25 (rotation auto sur quota).Why
Task 24 from PRD §6.4: plugins need deterministic account dispatch to handle quota rotation and fallback chains. BestTraffic ranks by traffic availability with tie-breakers for reproducibility. RoundRobin distributes load across eligible accounts. Manual reserved for future pinning UI.
Changes
Add
AccountSelectorservice insrc-tauri/src/application/services/account_selector.rs(~685 lines)BestTraffic(enabled → non-expired → most traffic → most recent validation → smallest id),RoundRobin(per-service cursor),Manual(alias of BestTraffic + regression test)NoAccountAvailable { service_name }andAccountSelected { id, service_name, strategy }Extend domain models
AccountSelectionStrategyenum indomain/model/account.rswith Display/FromStr (snake_case serialization)account_selection_strategyfield onAppConfigandConfigPatchwithapply_patch()supportUpdate adapters
no-account-availableandaccount-selectedevents to frontendconfig.toml(snake_case:best_traffic,round_robin,manual)account_selection_strategyfield (camelCase) for settings/patch endpointsExtend CommandBus with
account_selector()accessor andresolve_account_for(service_name)convenience methodFlag planned hook in
resolve_links.rs(comment only; actual per-URL calls deferred to avoid emittingAccountSelectedonce per probed link)Update CHANGELOG.md with full feature description
Testing
✅ All 12 unit tests pass covering four PRD acceptance criteria:
None+NoAccountAvailableevent✅ Comprehensive edge cases: cache invalidation, case-mismatch behavior, tie-breaker correctness, config strategy reading
✅ Integration: config patch apply, event emission, Tauri IPC serialization
Command output:
Related Issues
Notes for Reviewer
Unlimitedenum variant ranks above anyFinite(u64)via Ord derivation, ensuring unlimited plans always winresolve_account_for()reads live strategy from AppConfig each call, honoring runtime changes without restartChecklist
Summary by cubic
Auto-select the best account per service with a configurable strategy. Deterministic selection from live repo state, a single entry point for plugins/downloads, and backward-compatible config loading; implements PRD §6.4 (Linear task 24).
New Features
AccountSelectorwith three strategies:BestTraffic(default),RoundRobin(per-service cursor), andManual(alias ofBestTrafficfor now).NoAccountAvailableandAccountSelected; forwarded via Tauri asno-account-availableandaccount-selected.CommandBus::resolve_account_for(service_name)uses the liveAppConfigstrategy.account_selection_strategyonAppConfig/ConfigPatch, persisted to TOML and exposed via IPC ("best_traffic" | "round_robin" | "manual").Bug Fixes
RoundRobin: a poisoned cursor now surfaces as a validation error instead of returningNone.account_selection_strategyin IPC and TOML;settings_updatesurfaces errors. Legacyconfig.tomlwithout the field now loads with the default (BestTraffic), while unknown non-empty values fail fast.resolve_account_forpropagatesConfigStore::get_config()failures; tests cover repo-fresh selection, case sensitivity, validation, poisoned-cursor handling, and legacy-config compatibility.Written for commit 85a7f56. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Behavior