feat: add provider account pools and rust coverage#240
Conversation
Deploying corvus with
|
| Latest commit: |
cd02651
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4a28a274.corvus-42x.pages.dev |
| Branch Preview URL: | https://provider-proxy.corvus-42x.pages.dev |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-18 to 2026-03-18 |
|
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 multi-account provider pooling: new config types and validation, AccountPoolProvider with round‑robin/weighted selection, per-account cooldowns and per-account provider caching, admin GET/PUT endpoints (conditional exposure), TypeScript admin types, tests, docs/specs, and Makefile rust-coverage target wired into test-coverage. Changes
Sequence DiagramssequenceDiagram
participant Client
participant ReliableProvider
participant AccountPoolProvider
participant ProviderCache as Provider\r\nCache
participant ConcreteProvider
Client->>ReliableProvider: chat_with_history(...)
ReliableProvider->>AccountPoolProvider: chat_with_history(...)
AccountPoolProvider->>AccountPoolProvider: select_account()\n(skip disabled/cooldown)
AccountPoolProvider->>ProviderCache: get_or_create_provider(account_id)
ProviderCache->>ConcreteProvider: create with account credentials
ProviderCache-->>AccountPoolProvider: Provider instance
AccountPoolProvider->>ConcreteProvider: chat_with_history(with account creds)
alt Rate-Limited (429 / Retry-After)
ConcreteProvider-->>AccountPoolProvider: Error
AccountPoolProvider->>AccountPoolProvider: mark_cooldown(account_id)
AccountPoolProvider->>AccountPoolProvider: select_account()\n(next eligible)
AccountPoolProvider->>ConcreteProvider: retry with alternate account
ConcreteProvider-->>AccountPoolProvider: Success
else Success
ConcreteProvider-->>AccountPoolProvider: Response / StreamChunk
end
AccountPoolProvider-->>ReliableProvider: Result
ReliableProvider-->>Client: Result
sequenceDiagram
participant Admin
participant Gateway as Admin\r\nGateway
participant Config as Config\r\nStore
participant Validator as Validator
Admin->>Gateway: PUT /web/admin/provider-pools {account_pools: {...}}
Gateway->>Validator: validate patch
Validator->>Validator: check provider name,\nids/api_keys present, unique ids,\npositive weights, non-empty accounts
alt Validation Fails
Validator-->>Gateway: Error
Gateway-->>Admin: 400 Bad Request
else Valid
Validator-->>Gateway: OK
Gateway->>Config: encrypt credentials & update pools
Config->>Config: persist encrypted state
Config-->>Gateway: OK
Gateway-->>Admin: 200 {pools with has_api_key flags (redacted)}
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/agent-runtime/src/providers/mod.rs (1)
657-663:⚠️ Potential issue | 🟠 MajorAvoid combining global API-key rotation with account-pool selection.
Line 662 still applies
ReliableProvider::with_api_keys(...)even when providers in the chain are pool-backed. That creates two credential selection mechanisms (reliable-layer key rotation + pool account strategy), which can bypass pool weights/cooldowns and make retry behavior inconsistent.Proposed diff
- let reliable = ReliableProvider::new( + let has_pooled_provider = providers + .iter() + .any(|(name, _)| reliability.account_pools.contains_key(name)); + + let reliable = { + let base = ReliableProvider::new( providers, reliability.provider_retries, reliability.provider_backoff_ms, - ) - .with_api_keys(reliability.api_keys.clone()) + ); + if has_pooled_provider { + base + } else { + base.with_api_keys(reliability.api_keys.clone()) + } + } .with_model_fallbacks(reliability.model_fallbacks.clone());As per coding guidelines: “
**/*: Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/providers/mod.rs` around lines 657 - 663, The ReliableProvider construction is applying global API-key rotation via with_api_keys(...) even when the provider chain contains pool-backed providers, creating conflicting credential-selection mechanisms; update the construction so you only call .with_api_keys(reliability.api_keys.clone()) when the providers list contains no pool-backed/provider-pool entries (e.g., check the providers variable for pool-backed types or a Provider::Pool variant) and otherwise skip applying with_api_keys so pool account selection/weights/cooldowns remain authoritative; keep the .with_model_fallbacks(...) call as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/providers/pool.rs`:
- Around line 203-211: AccountPoolProvider currently advertises native tool
support via capabilities()/supports_native_tools() but still uses the trait
default convert_tools(), causing native-tool providers behind the pool to
receive generic payloads; override convert_tools() on AccountPoolProvider to
delegate to the selected underlying provider’s convert_tools implementation (use
the same provider lookup used by
provider_capabilities()/supports_native_tools(), e.g. obtain the inner provider
instance and call its convert_tools(...) when it exists and/or when
native_tool_calling is true), falling back to the default behavior only if no
underlying provider or conversion is available.
- Around line 263-320: The streaming methods stream_chat_with_system and
stream_chat_with_history bypass with_account() and thus never call
mark_cooldown() on rate-limit (429) failures; wrap the provider stream so we
detect a 429 and mark the selected account on cooldown. Concretely, refactor
each method to select an account via select_account_index()/provider_for_account
(or call the existing with_account helper if available), obtain both account and
provider, then call provider.stream_chat_... but map/inspect the resulting
BoxStream to intercept StreamError::Provider (or the underlying HTTP/error code)
and call self.mark_cooldown(&account) before re-emitting the error; ensure the
stream still yields items normally when no 429 occurs. Reference:
stream_chat_with_system, stream_chat_with_history, with_account, mark_cooldown,
select_account_index, provider_for_account.
In `@clients/web/apps/dashboard/src/types/admin-config.ts`:
- Around line 40-51: The response types are too permissive: change
AdminProviderPoolsResponse so pools is required (remove the optional ? on pools)
and change AdminProviderPoolsUpdateResponse so updated and pools are required
(remove ? on updated and pools) to match the backend contract; update any places
that construct or consume AdminProviderPoolsResponse and
AdminProviderPoolsUpdateResponse accordingly (look for usages of
AdminProviderPoolsResponse, AdminProviderPoolsUpdateResponse and
AdminProviderPoolsUpdateRequest) to ensure callers handle the now-required
fields.
In `@clients/web/build.gradle.kts`:
- Around line 97-102: The registered per-app task "${appName}Install" is a no-op
because changes to the app-level package.json don't cause the shared
workspaceInstall task to rerun; update the workspaceInstall task (symbol:
workspaceInstall) to include each app's "${appDir}/package.json" as inputs so
changes to app-level package.json files invalidate and rerun workspaceInstall.
Concretely, locate the workspaceInstall task declaration and add
inputs.files(...) or inputs.file("${appDir}/package.json") for each app (using
the same appDir/appName variables used when registering "${appName}Install") so
workspaceInstall will track app-level dependency files and rebuild node_modules
when they change.
In `@Makefile`:
- Line 275: The new Makefile target rust-coverage must be added to the .PHONY
list to prevent a file of that name from shadowing the target; edit the .PHONY
declaration and include rust-coverage alongside existing phony targets so the
rust-coverage target is always executed when invoked.
- Around line 275-277: The rust-coverage Makefile target currently assumes
cargo-llvm-cov and llvm-tools-preview are installed; add preflight checks at the
top of the rust-coverage target to verify availability of the cargo-llvm-cov
binary (e.g., via `command -v cargo-llvm-cov` or similar) and that `rustup
component list` shows `llvm-tools-preview` installed, and fail with a clear
error message and non-zero exit if either is missing; update any dependent
target such as test-coverage to still rely on rust-coverage but not proceed
silently on missing tooling; also add a short prerequisites section to
clients/agent-runtime/CONTRIBUTING.md listing installation commands for
cargo-llvm-cov and llvm-tools-preview and why they are required so local devs
and CI parity are clear.
- Around line 270-271: The Makefile target test-coverage currently invokes the
Gradle task :agent-core-kmp:koverVerify which has no Kover verification rules
and thus is a no-op; either remove the :agent-core-kmp:koverVerify invocation
from the test-coverage recipe or add explicit Kover verification rules in the
agent-core-kmp module (e.g., configure kover { verify { rule {
minBound(<percent>) } } } or equivalent) so the :agent-core-kmp:koverVerify task
enforces a coverage gate; update the Makefile to only call
:agent-core-kmp:koverVerify if those verification rules are defined, otherwise
remove that task from the test-coverage line.
In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md`:
- Line 35: Update the heading text that currently reads "Decision: Rate-limit
aware selection with cooldown hints" to hyphenate the compound modifier so it
reads "Decision: Rate-limit-aware selection with cooldown hints"; locate the
header line by the exact phrase "Decision: Rate-limit aware selection with
cooldown hints" in the document and replace it with the hyphenated form.
- Around line 113-177: Replace the fenced code blocks with the repo's configured
indented code-block style for the Rust and TOML examples: convert the Rust block
containing AccountPoolStrategy, ProviderAccountConfig,
ProviderAccountPoolConfig, ReliabilityConfig and the AccountPoolProvider struct
into an indented code block, and likewise convert the TOML example
(reliability.account_pools.openrouter and its accounts) to an indented block so
MD046 is satisfied; keep the exact code and indentation consistent with
surrounding markdown examples.
In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md`:
- Around line 165-176: Update the archived design snippet for
AccountPoolProvider to match the shipped implementation: change the cache field
from HashMap<String, Box<dyn Provider>> to parking_lot::Mutex<HashMap<String,
std::sync::Arc<dyn Provider>>> (or equivalent Arc<dyn Provider> usage), add any
extra state fields present in the implementation (e.g., flags/counters used by
the runtime), document that pool admin exposure is gated by
gateway.admin_expose_provider_pools, and note that account entries with blank
api_key are rejected (i.e., required) so admin/API decisions are closed; apply
the same changes to the other snippet mentioned (lines ~200-205).
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md`:
- Around line 11-90: Add a blank line before every "#### Scenario:" heading in
this spec so each scenario heading is separated from the previous
paragraph/section; search for the token "#### Scenario:" and insert a single
empty line immediately above each occurrence to fix markdown linting and
readability (apply the same change across all occurrences in the document).
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md`:
- Around line 22-25: The fenced code blocks in verify-report.md lack language
specifiers (triggering MD040); update each triple-backtick block (including the
blocks showing "make build\nBUILD SUCCESSFUL" and "make test\nBUILD SUCCESSFUL"
and any other blocks in the 28-36 range) to include a language token such as
text (e.g., replace ``` with ```text) so tooling and syntax highlighting can
correctly recognize them.
- Line 1: Change the top-level heading in the verify-report.md file from "##
Verification Report" to a level-1 heading "# Verification Report" so the file
begins with a single top-level title (replace the existing "## Verification
Report" line with "# Verification Report").
---
Outside diff comments:
In `@clients/agent-runtime/src/providers/mod.rs`:
- Around line 657-663: The ReliableProvider construction is applying global
API-key rotation via with_api_keys(...) even when the provider chain contains
pool-backed providers, creating conflicting credential-selection mechanisms;
update the construction so you only call
.with_api_keys(reliability.api_keys.clone()) when the providers list contains no
pool-backed/provider-pool entries (e.g., check the providers variable for
pool-backed types or a Provider::Pool variant) and otherwise skip applying
with_api_keys so pool account selection/weights/cooldowns remain authoritative;
keep the .with_model_fallbacks(...) call as-is.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11182aba-c5ec-42c2-8ac3-87f5414a0ff6
📒 Files selected for processing (25)
Makefileclients/agent-runtime/src/config/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rsclients/agent-runtime/tests/admin_config_api_integration.rsclients/web/apps/dashboard/src/types/admin-config.tsclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdclients/web/build.gradle.ktsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/proposal.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/tasks.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/verify-report.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/README-superseded.txtopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/proposal.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/tasks.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdopenspec/specs/agent-runtime-providers/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/tasks.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/proposal.mdopenspec/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/verify-report.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/proposal.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/tasks.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/tasks.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdclients/agent-runtime/tests/admin_config_api_integration.rsMakefileclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/proposal.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/README-superseded.txtopenspec/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.mdclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/verify-report.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.mdclients/web/apps/dashboard/src/types/admin-config.tsclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/proposal.mdclients/web/build.gradle.ktsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdclients/agent-runtime/src/gateway/admin.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/tasks.mdclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/gateway/admin.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
clients/agent-runtime/src/providers/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider
Files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
**/*.gradle.kts
⚙️ CodeRabbit configuration file
**/*.gradle.kts: Prefer tasks.register/configureEach, avoid afterEvaluate, and preserve configuration cache.
Ensure dependencies come from version catalogs and avoid eager task realization.
Review plugin/config changes for supply-chain and reproducibility risks.
Files:
clients/web/build.gradle.kts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/tasks.mdclients/agent-runtime/tests/admin_config_api_integration.rsopenspec/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.mdclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/providers/mod.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdclients/agent-runtime/src/gateway/admin.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/tasks.mdclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/config/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
Makefileclients/agent-runtime/src/config/schema.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/config/schema.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/providers/pool.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/agent-runtime/src/config/schema.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/agent-runtime/src/config/schema.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
clients/agent-runtime/src/config/schema.rs
🪛 LanguageTool
openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md
[grammar] ~35-~35: Use a hyphen to join words.
Context: ...ing semantics. ### Decision: Rate-limit aware selection with cooldown hints **C...
(QB_NEW_EN_HYPHEN)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
[grammar] ~35-~35: Use a hyphen to join words.
Context: ...ing semantics. ### Decision: Rate-limit aware selection with cooldown hints **C...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 28-28: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 61-61: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 62-62: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 73-73: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 74-74: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 94-94: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/exploration.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/specs/agent-runtime-providers/spec.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 46-46: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 68-68: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 80-80: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 46-46: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 68-68: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 80-80: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 39-39: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 46-46: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 68-68: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 80-80: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 28-28: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 34-34: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 67-67: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 79-79: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 80-80: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md
[warning] 113-113: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 150-150: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 167-167: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/proposal.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
[warning] 113-113: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 150-150: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 167-167: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (26)
clients/agent-runtime/src/config/mod.rs (1)
4-16: LGTM!The new re-exports for
AccountPoolStrategy,ProviderAccountConfig, andProviderAccountPoolConfigare correctly added and maintain alphabetical ordering. These types are tested inschema.rsas indicated by the AI summary.clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md (2)
4-4: LGTM!Adding an explicit slug improves URL stability for this guide.
9-9: Archive URL path exists and is correct. The referenced fileopenspec/changes/archive/2026-03-16-cerebro/cerebro.mdis present in the repository.openspec/changes/archive/2026-03-18-multi-account-provider-pool/tasks.md (1)
1-46: LGTM!Task tracking is comprehensive and all items are marked complete. Note this archive is superseded by
multi-account-provider-pool-2/per the siblingREADME-superseded.txt.openspec/changes/archive/2026-03-18-multi-account-provider-pool/README-superseded.txt (1)
1-4: LGTM!Clear supersession notice pointing to the updated archive with reason documented.
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/tasks.md (1)
1-46: LGTM!Authoritative task list with clear TDD methodology (RED/GREEN cycles) and complete status tracking.
openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md (2)
63-72: Good security posture.The requirement to encrypt pooled credentials at rest and redact them in logs/diagnostics aligns with the coding guidelines for secret handling. This is correctly implemented per the relevant code snippets showing redacted Debug impl.
74-90: Secure-by-default admin exposure.The spec correctly requires deny-by-default for admin pool access. The implementation in
admin.rs(per context snippet 2) returns 403 FORBIDDEN whenadmin_expose_provider_poolsis false. As per coding guidelines: "keep default behavior secure-by-default with deny-by-default where applicable."openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/proposal.md (1)
1-69: LGTM!Comprehensive proposal with clear scope boundaries, risk mitigation strategies, and measurable success criteria. The rollback plan at lines 53-57 provides a safe escape path.
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md (1)
87-88: Valid coverage gap warning.The report correctly identifies that coverage is only reported for
modules/agent-core-kmpwhile this change primarily affectsclients/agent-runtime. Consider adding Rust coverage to CI if not already tracked elsewhere.openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/verify-report.md (1)
1-104: LGTM — Verification report is comprehensive and accurate.The report correctly documents test coverage, compliance scenarios, and design coherence. The warning about
make buildrunningagentsync applyand modifying workspace files is a good catch for CI hygiene.Minor markdown formatting issues flagged by linters (missing blank lines around headings/tables, unspecified code block languages) are cosmetic and won't affect readability.
clients/agent-runtime/tests/admin_config_api_integration.rs (4)
48-59: Clean helper function.
sample_poolis minimal and follows existing test patterns. Using hardcoded test credentials in test files is appropriate.
218-243: Good coverage for admin exposure toggle.Test correctly verifies that both GET and PUT operations return
FORBIDDENwhenadmin_expose_provider_poolsisfalse. This ensures secure-by-default behavior.
245-267: Redaction test is solid.Verifies
has_api_key: trueis present while confirming the actual secret value is absent from the response body.
269-307: Invalid patch test includes rollback verification.Good practice to assert that the original pool configuration remains intact after a rejected patch (lines 304-306). This confirms atomic/rollback semantics.
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/exploration.md (1)
1-45: Exploration document is technically accurate and well-reasoned.The analysis correctly identifies the current state, affected areas, and tradeoffs between approaches. Approach 1 (Account Pool in ReliabilityConfig) aligns with the implemented design using
AccountPoolProvider.openspec/changes/archive/2026-03-18-multi-account-provider-pool/exploration.md (1)
1-45: Duplicate exploration document — same content as the other archive path.Content is technically accurate. Consider consolidating if both paths are not needed for versioning purposes.
clients/agent-runtime/src/gateway/mod.rs (3)
1104-1107: Console output is correctly conditional.The provider-pools endpoints are only printed when
admin_expose_provider_poolsis true, which is good UX — operators won't see endpoints advertised that they haven't enabled.
1304-1309: Handler wrappers follow established patterns.Both wrappers delegate to
admin::handle_admin_*functions, consistent with other admin endpoints in this file.Also applies to: 1319-1325
1157-1160: No action required—handlers correctly enforce theadmin_expose_provider_poolsflag.Both
handle_admin_get_provider_poolsandhandle_admin_update_provider_poolscheck the flag and returnFORBIDDENwhen disabled, meeting the secure-by-default requirement with deny-by-default enforcement.openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/specs/agent-runtime-providers/spec.md (1)
1-90: Specification is well-structured and aligned with implementation.All requirements (pool configuration, round-robin selection, account-bound provider reuse, backward compatibility, secret handling, admin exposure controls) are covered by corresponding implementation and tests. Scenarios map to actual test cases.
openspec/specs/agent-runtime-providers/spec.md (1)
1-90: Canonical spec matches archive delta — requirements are complete and accurate.openspec/changes/archive/2026-03-18-multi-account-provider-pool/proposal.md (1)
1-69: Proposal is clear and complete.Intent, scope, approach, affected areas, risks, and rollback plan are all well-defined. The implementation aligns with this proposal —
AccountPoolProviderprovides the credential-aware wrapper mentioned in the risks mitigation.clients/agent-runtime/src/providers/mod.rs (1)
9-22: Pool provider registration is wired correctly.
pub mod pool;plus factory integration throughAccountPoolProviderkeeps provider registration aligned with the provider-module contract.Based on learnings: “Applies to clients/agent-runtime/src/providers/**/*.rs : Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider”.clients/agent-runtime/src/gateway/admin.rs (1)
1569-1585: Good secure-by-default gating on provider-pools endpoints.Origin check, admin auth, and
admin_expose_provider_poolsgating are all enforced before read/write access.As per coding guidelines: “Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable”.
Also applies to: 1688-1712
clients/agent-runtime/src/config/schema.rs (1)
1406-1416: Good secret hygiene for pooled credentials.Redacting
ProviderAccountConfig::api_keyand wiring pooled keys through the same encrypt/decrypt path as the other secrets keeps the new config surface off the obvious leak paths.As per coding guidelines: "Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements"
Also applies to: 2690-2700, 3189-3197
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
Makefile (1)
280-283:⚠️ Potential issue | 🟠 MajorUse a more flexible pattern to detect the
llvm-toolscomponent.The current pattern
^llvm-tools-previewsearches for a component name that may not exist. The actual installed component is typically namedllvm-tools(notllvm-tools-preview), so the grep will fail and cause unnecessary errors. Update the pattern to^llvm-toolsto match the correct component name.🔧 Proposed patch
- `@rustup` component list --installed | grep -q '^llvm-tools-preview' || { \ + `@rustup` component list --installed | grep -Eq '^llvm-tools' || { \ echo "llvm-tools-preview is required. Install with: rustup component add llvm-tools-preview" >&2; \ exit 1; \ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 280 - 283, The Makefile's rustup component check is using the too-specific grep pattern '^llvm-tools-preview' which misses the actual installed component name; update the grep pattern in the command that pipes `rustup component list --installed` (the line containing `| grep -q '...' || { ... }`) to use '^llvm-tools' (or simply 'llvm-tools' for broader matching) so the check detects the installed llvm-tools component correctly and avoids false failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/providers/pool.rs`:
- Around line 188-196: In provider_capabilities, stop falling back to a disabled
account via .or_else(|| self.accounts.first()) — instead only use an account if
it's enabled and if none exist return None/default capabilities; locate the same
pattern in the similar method around lines 210-226 and remove the fallback to
self.accounts.first(), so both provider_capabilities and the other method call
provider_for_account only with an enabled account and return the default/None
when no enabled account is found.
- Around line 97-107: select_round_robin currently iterates over all accounts
and filters with eligible.contains(...), which biases rotation when some slots
are ineligible; instead rotate directly over the eligible slice. Change
select_round_robin to: check eligible.len() > 0 (return the same anyhow error
with self.provider_name if empty), read the counter with self.index.fetch_add(1,
Ordering::Relaxed), compute selected = eligible[(start as usize) %
eligible.len()] and return that index; keep function name select_round_robin and
the same error message for the empty case.
In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md`:
- Around line 101-107: Update the admin/config response contract so pooled API
keys are never returned in plaintext: change the pool secret schema in
clients/agent-runtime/src/config/schema.rs to exclude or redact the raw api_key
from serialization (e.g., mark field non-serializable or replace with
redacted/token flag), update any admin API handlers used in
clients/agent-runtime/tests/admin_config_api_integration.rs to ensure they
return only redacted/omitted values (or a boolean/metadata) for pooled
credentials, and adjust the frontend type in
clients/web/apps/dashboard/src/types/admin-config.ts to expect no plaintext
api_key (use redacted/omitted token or boolean exposed flag). Also verify
providers/mod.rs and providers/pool.rs do not rely on admin endpoints to
retrieve plaintext keys and ensure any code paths that might serialize pooled
secrets use the new redacted representation.
- Around line 196-199: Replace the open checklist items about admin API exposure
and pooled `api_key` with explicit decisions: state that the admin config API
will not expose pool read/patch in this phase (the admin exposure is
toggle-controlled and deferred), and state that pooled accounts must provide an
explicit `api_key` (no fallback to `config.api_key`) to match the implemented
admin exposure toggle and required pooled `api_key` behavior; update the
language in design.md to remove the checkboxes and make these two assertions
authoritative.
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md`:
- Around line 7-9: The spec text is inaccurate: account pools are provider-keyed
in the config (reliability.account_pools.<provider>) and each entry contains
account fields only (credentials, api_url, weight, etc.), not a separate
provider identifier; update the sentence to state that the system MUST support
configuring provider-keyed account pools under reliability settings
(reliability.account_pools.<provider>), where each pool entry contains account
credentials and optional metadata (api_url, weight) rather than a redundant
provider identifier.
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md`:
- Around line 38-40: The coverage statement only cites modules/agent-core-kmp
(the "Source: `modules/agent-core-kmp/build/reports/kover/html/index.html`"
line) while this PR touches clients/agent-runtime and claims Rust coverage
aggregation; update the verify-report.md coverage section to either (A) include
the Rust coverage metric and its source path for clients/agent-runtime (or the
aggregated Rust coverage source you used) alongside the existing KMP source, or
(B) explicitly mark Rust coverage as unavailable/uncollected in the main
coverage line and add a short note explaining why; modify the “Coverage: 92.5% …
Source: …” line and the nearby summary text so the report accurately reflects
both JVM/KMP and Rust coverage status.
---
Duplicate comments:
In `@Makefile`:
- Around line 280-283: The Makefile's rustup component check is using the
too-specific grep pattern '^llvm-tools-preview' which misses the actual
installed component name; update the grep pattern in the command that pipes
`rustup component list --installed` (the line containing `| grep -q '...' || {
... }`) to use '^llvm-tools' (or simply 'llvm-tools' for broader matching) so
the check detects the installed llvm-tools component correctly and avoids false
failures.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b0c0da5-57ed-4d8f-846c-d57ae55fb021
📒 Files selected for processing (10)
Makefileclients/agent-runtime/CONTRIBUTING.mdclients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rsclients/web/apps/dashboard/src/types/admin-config.tsclients/web/build.gradle.ktsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdclients/agent-runtime/CONTRIBUTING.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdclients/agent-runtime/CONTRIBUTING.mdclients/web/apps/dashboard/src/types/admin-config.tsMakefileclients/agent-runtime/src/providers/mod.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/design.mdclients/web/build.gradle.kts
clients/agent-runtime/src/providers/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider
Files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
**/*.gradle.kts
⚙️ CodeRabbit configuration file
**/*.gradle.kts: Prefer tasks.register/configureEach, avoid afterEvaluate, and preserve configuration cache.
Ensure dependencies come from version catalogs and avoid eager task realization.
Review plugin/config changes for supply-chain and reproducibility risks.
Files:
clients/web/build.gradle.kts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/CONTRIBUTING.mdMakefileopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/CONTRIBUTING.mdMakefileclients/agent-runtime/src/providers/pool.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
clients/agent-runtime/CONTRIBUTING.mdMakefile
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Document agent configurations and capabilities in AGENTS.md
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/CONTRIBUTING.mdclients/agent-runtime/src/providers/pool.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/CONTRIBUTING.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/providers/mod.rsclients/agent-runtime/src/providers/pool.rs
🪛 LanguageTool
openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md
[grammar] ~35-~35: Use a hyphen to join words.
Context: ...ing semantics. ### Decision: Rate-limit aware selection with cooldown hints **C...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
[warning] 113-113: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 150-150: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 167-167: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md
[warning] 113-113: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 150-150: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 167-167: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (7)
clients/web/build.gradle.kts (1)
97-102: Good fix: app-level dependency changes now correctly invalidate workspace install.This resolves the stale
node_modulesrisk by wiring each app’spackage.jsonintoworkspaceInstallinputs while keeping task registration lazy.Also applies to: 104-106
clients/agent-runtime/CONTRIBUTING.md (1)
28-40: Coverage prerequisites doc is accurate and aligned with the new targets.The commands and output path match the Makefile changes for
rust-coverageandtest-coverage.As per coding guidelines "
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."Makefile (2)
270-273:test-coveragewiring looks correct for the new Rust+Kover reporting flow.Dependency on
rust-coverageplus explicit report paths is a good improvement for local discoverability.
390-390: Good catch addingrust-coverageto.PHONY.This prevents accidental shadowing by a same-named file and keeps the target reliably executable.
clients/web/apps/dashboard/src/types/admin-config.ts (1)
8-51: Type additions look consistent and contract-safe.The pool strategy/config/view/update shapes are explicit, and success responses correctly require
pools/updated.clients/agent-runtime/src/providers/mod.rs (1)
9-9: Pool provider wiring into factory/resilient construction looks correct.
poolis registered andAccountPoolProvideris integrated at provider-construction time with dedicated pool tests covering selected behavior.
Based on learnings: "Applies to clients/agent-runtime/src/providers/**/*.rs : ImplementProvidertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider".Also applies to: 21-21, 617-670
openspec/changes/archive/2026-03-18-multi-account-provider-pool/design.md (1)
165-188: Design section is now aligned with shipped pool behavior.The wrapper shape, cooldown behavior, admin exposure gate, and required non-empty
api_keyexpectations are documented consistently.
As per coding guidelines: "**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md (1)
198-202:⚠️ Potential issue | 🟡 MinorRename section header to reflect finalized decisions.
The bullets now read as decisions, not questions. Update the header for clarity.
📝 Proposed fix
-## Open Questions +## Decisions (Finalized) -- Admin config pool read/patch is gated by `gateway.admin_expose_provider_pools` and is - deferred by default. -- Pooled accounts require explicit `api_key` values; no fallback to `config.api_key`. +- Admin config API exposure for provider pools is controlled by + `gateway.admin_expose_provider_pools` (default: `false`). +- Pooled accounts require explicit `api_key`; omission is rejected by validation.As per coding guidelines: "
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md` around lines 198 - 202, Rename the "## Open Questions" section header to reflect that these are finalized decisions (e.g., "## Decisions" or "## Finalized Decisions") and update any nearby header anchors if necessary; the bullets under the header reference gateway.admin_expose_provider_pools and the pooled-account api_key behavior (no fallback to config.api_key), so ensure the new header clearly indicates these are decisions rather than open questions and leave the decision bullets unchanged.openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md (1)
38-42:⚠️ Potential issue | 🟠 MajorAlign the Rust coverage story across the report.
Lines 39-41 cite
coverage/agent-runtime-coverage.lcov, but Lines 89-99 still sayclients/agent-runtimehas no coverage report and that Rust coverage is not reported. Either surface the Rust metric, or reword the warning/verdict to say the LCOV artifact exists but no Rust percentage/threshold is summarized yet.As per coding guidelines "Verify technical accuracy and that docs stay aligned with code changes."Possible doc fix
-**Coverage**: 92.5% line (KMP) / threshold: 60% → ✅ Above threshold +**Coverage**: +- KMP: 92.5% line / threshold: 60% → ✅ Above threshold +- Rust: LCOV artifact generated at `coverage/agent-runtime-coverage.lcov`; percentage not summarized here -**WARNING** (should fix): -- Coverage report is scoped to `modules/agent-core-kmp`; change touches `clients/agent-runtime` which has no coverage report. +**WARNING** (should fix): +- Rust coverage percentage is not summarized here, even though `coverage/agent-runtime-coverage.lcov` was generated. -All spec scenarios are covered by passing tests; coverage threshold met for the available report, but coverage is not reported for the Rust runtime module changed in this work. +All spec scenarios are covered by passing tests; the KMP threshold is met, and a Rust LCOV artifact was generated, but this report does not yet summarize a Rust percentage/threshold.Also applies to: 89-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md` around lines 38 - 42, The report currently lists coverage/agent-runtime-coverage.lcov as a Rust LCOV artifact while later (clients/agent-runtime section) states no Rust coverage is reported; update verify-report.md so the Rust coverage story is consistent by either (A) parsing and surfacing the Rust percentage from coverage/agent-runtime-coverage.lcov and displaying it alongside the KMP metric, or (B) changing the clients/agent-runtime verdict text to note that an LCOV artifact exists but no summarized Rust percentage/threshold is computed yet; adjust the text near the Sources list and the clients/agent-runtime warning (the entries mentioning coverage/agent-runtime-coverage.lcov and clients/agent-runtime) so both places convey the same state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/providers/pool.rs`:
- Around line 431-448: The conversion path can panic for very large secs because
Duration::from_secs_f64 overflows; update secs_to_millis (and/or
parse_retry_after_seconds usage) to clamp the parsed secs to a safe maximum
before calling Duration::from_secs_f64 (e.g., define a MAX_SAFE_SECONDS constant
derived from your MAX_COOLDOWN_MS or the largest representable duration and do
secs = secs.min(MAX_SAFE_SECONDS)), or avoid Duration::from_secs_f64 entirely by
computing milliseconds as secs * 1000.0, clamping to MAX_COOLDOWN_MS, and then
converting to u64 with try_from; reference the functions secs_to_millis and
parse_retry_after_seconds when making the change.
In `@Makefile`:
- Around line 270-273: The Makefile target test-coverage advertises an
"aggregated report" but only prints two separate artifacts; either rename the
target to something accurate (e.g., test-coverage-reports) or actually emit a
single aggregated summary file and update the echoes accordingly; specifically
update the Makefile target named test-coverage and change its echo lines (the
two "📊 Report:" messages) to reflect either separate reports or add a step that
produces/prints a combined summary (for example, generate an aggregated
HTML/index or a merged lcov summary) so the command contract matches the output.
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md`:
- Around line 47-59: The compliance summary omits explicit evidence for weighted
round-robin and tightened pool validation; update the table to include concrete
test entries and adjust the summary. Add rows naming the weighted-RR test (e.g.,
clients/agent-runtime/src/providers/pool.rs >
round_robin_weighted_selects_accounts) and the tightened validation tests (e.g.,
clients/agent-runtime/src/config/schema.rs >
validate_for_runtime_rejects_pool_account_zero_weight and
clients/agent-runtime/src/config/schema.rs >
validate_for_runtime_rejects_pool_duplicate_account_id) with their pass/fail
results, or if those tests don't exist, change the final summary to reflect only
the scenarios covered; ensure the summary line (previously "9/9 scenarios
compliant") is updated to match the actual listed rows.
---
Duplicate comments:
In `@openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md`:
- Around line 198-202: Rename the "## Open Questions" section header to reflect
that these are finalized decisions (e.g., "## Decisions" or "## Finalized
Decisions") and update any nearby header anchors if necessary; the bullets under
the header reference gateway.admin_expose_provider_pools and the pooled-account
api_key behavior (no fallback to config.api_key), so ensure the new header
clearly indicates these are decisions rather than open questions and leave the
decision bullets unchanged.
In
`@openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md`:
- Around line 38-42: The report currently lists
coverage/agent-runtime-coverage.lcov as a Rust LCOV artifact while later
(clients/agent-runtime section) states no Rust coverage is reported; update
verify-report.md so the Rust coverage story is consistent by either (A) parsing
and surfacing the Rust percentage from coverage/agent-runtime-coverage.lcov and
displaying it alongside the KMP metric, or (B) changing the
clients/agent-runtime verdict text to note that an LCOV artifact exists but no
summarized Rust percentage/threshold is computed yet; adjust the text near the
Sources list and the clients/agent-runtime warning (the entries mentioning
coverage/agent-runtime-coverage.lcov and clients/agent-runtime) so both places
convey the same state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a809c35c-1776-4aa8-a916-8995295739ec
📒 Files selected for processing (5)
Makefileclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdMakefileclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
clients/agent-runtime/src/providers/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider
Files:
clients/agent-runtime/src/providers/pool.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/providers/pool.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/providers/pool.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/providers/pool.rs
🧠 Learnings (9)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdMakefileopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdclients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdMakefileclients/agent-runtime/src/providers/pool.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.mdopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
Makefile
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/providers/pool.rsopenspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/src/providers/pool.rs
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/verify-report.md
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 28-28: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 64-64: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 75-75: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 96-96: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md
[warning] 12-12: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 39-39: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 51-51: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 75-75: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 88-88: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 95-95: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 28-28: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 64-64: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 75-75: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 76-76: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 96-96: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (9)
openspec/changes/archive/2026-03-18-multi-account-provider-pool/specs/agent-runtime-providers/spec.md (1)
1-99: LGTM - Spec content aligns with implementation.The requirements and scenarios accurately reflect the implemented pool selection logic, credential handling, and admin exposure controls. The technical specification matches the behavior in
pool.rsandschema.rs.The markdown linting warnings about blank lines below headings persist from the previous review cycle. These are formatting-only concerns.
clients/agent-runtime/src/providers/pool.rs (6)
1-61: Clean module setup and data structures.The use of
parking_lot::Mutexfor low-contention synchronization andAtomicUsizefor the round-robin index is appropriate. TheWeightedStatestruct cleanly encapsulates the deficit-based weighted selection state.
97-108: Round-robin bias fix correctly applied.The selection now rotates directly over the eligible slice (
eligible[start % eligible.len()]), ensuring even distribution regardless of disabled/cooldown slots.
138-158: Per-account provider caching is correct.Provider instances are cached by account ID, preserving account boundaries. The pattern of check-then-create has a minor TOCTOU window where two threads could both create providers, but this is benign—the second insert simply overwrites, and no credentials leak across accounts.
189-219: Disabled account fallback correctly removed.
provider_capabilities()andconvert_tools()now only consider enabled accounts, preventing disabled credentials from influencing capability detection or tool conversion.
271-349: Streaming cooldown handling properly applied.Both streaming methods now wrap the underlying stream with
.map()to inspect errors and callmark_stream_cooldown, ensuring rate-limited accounts are cooled down even on streaming paths.
450-551: Solid test coverage for pool selection mechanics.Tests cover round-robin alternation, weighted distribution, cooldown skipping, deterministic single-account behavior, and account-bound cache isolation.
openspec/changes/archive/2026-03-18-multi-account-provider-pool-2/design.md (2)
1-45: Design accurately reflects implementation.The architecture decisions (provider-keyed config, wrapper pattern, rate-limit-aware selection) match the actual implementation in
pool.rsandmod.rs.
146-148: API key exposure contract clearly documented.The explicit statement that admin responses never return plaintext API keys and use
has_api_keymetadata addresses the security concern from previous review cycles.
|
1 participant
This pull request introduces support for provider account pools in the agent runtime configuration, enabling more flexible and secure management of multiple provider accounts (such as for OpenAI, Anthropic, etc.). It adds new configuration types, validation, and secret management for provider account pools, as well as updates to the gateway configuration and related tests. Additionally, the Makefile and test coverage reporting are updated to account for these changes.
Provider Account Pools and Security Enhancements:
ProviderAccountConfig,ProviderAccountPoolConfig, andAccountPoolStrategyto support pools of provider accounts, including strategies like round-robin and weighted round-robin. Each account can have its own API key, URL, weight, and enabled flag. (clients/agent-runtime/src/config/schema.rs,clients/agent-runtime/src/config/mod.rs) [1] [2]ReliabilityConfig, allowing configuration of account pools per provider. Default values and validation logic ensure pools are well-formed and secure. (clients/agent-runtime/src/config/schema.rs) [1] [2] [3]clients/agent-runtime/src/config/schema.rs) [1] [2] [3]admin_expose_provider_pools) to allow admin HTTP API access to provider pools, with a default offalse. (clients/agent-runtime/src/config/schema.rs) [1] [2] [3]Testing and Validation:
clients/agent-runtime/src/config/schema.rs) [1] [2] [3]Build and Coverage Reporting:
Makefile)Other Minor Improvements:
clients/agent-runtime/src/config/schema.rs)clients/agent-runtime/src/config/mod.rs,clients/agent-runtime/src/gateway/admin.rs) [1] [2]