feat(rook): scaffold package layout with domain types, CLI, and module stubs#601
Merged
Conversation
Deploying corvus with
|
| Latest commit: |
40315f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac366e9d.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-rook-580-package-layout.corvus-42x.pages.dev |
Contributor
|
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 a new standalone Rust crate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-19 to 2026-04-19 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/rook/README.md`:
- Around line 40-53: Update the fenced code block in clients/rook/README.md (the
directory tree block that currently begins with ``` ) to include a language
specifier (use `text`) so the block starts with ```text; this resolves the MD040
lint by making the directory tree a `text`-fenced block.
- Around line 31-35: Update the Quick Start in clients/rook/README.md to mark
the cargo subcommands "doctor" and "config export" as stubs/not-implemented
instead of presenting them as functional; specifically change the entries for
the commands shown (the "doctor" and "config export" lines) to indicate "(stub -
not yet implemented)" or add a brief parenthetical note that they only print
"not yet implemented", so users won't expect working diagnostics or config
export.
- Around line 1-71: Add an explicit note about English/Spanish parity in the
README: update the top-level Corvus Rook README (e.g., the "# Corvus Rook"
header and/or "Status" section) to either state that an official Spanish
translation exists or, if none yet, include a clear "ES translation pending"
line indicating translations are planned and where to contribute; reference the
README sections like "Quick Start" or "Status" so the note is visible to users
and maintainers.
In `@clients/rook/src/domain/mod.rs`:
- Around line 1-159: Add unit tests covering serde and error conversions: write
a serde round-trip test for ProviderVendor that serializes known variants and an
Other(String) slug via serde_json and asserts deserializing yields the original
(ensure the Other case round-trips); add a test asserting SelectionStrategy
serializes to snake_case strings and deserializes back (Priority -> "priority",
RoundRobin -> "round_robin", etc.) to lock config stability; and add a smoke
test for RookError From conversions that constructs a std::io::Error and an
anyhow::Error, converts each into RookError via From (or ? operator) and asserts
they match the Io and Other variants respectively. Place these tests in a tests
module (or #[cfg(test)] mod tests) near domain/mod.rs so they run with cargo
test and reference ProviderVendor, SelectionStrategy, and RookError by name.
- Around line 43-53: The newtype ID structs (AccountId, PoolId, RouteId)
currently expose their inner Uuid as pub and are non-Copy; change each to hide
the inner field (remove pub), add Copy to the derives (e.g., #[derive(Debug,
Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]), and annotate each
with #[serde(transparent)] so the serialized wire format remains a bare UUID
string; this preserves encapsulation for future invariants and restores Uuid's
copy semantics to avoid unnecessary clones while keeping serde behavior
identical.
- Around line 57-71: ProviderVendor currently uses externally-tagged
serialization so a raw string like "mistral" fails to map to
ProviderVendor::Other(String); change the enum to be untagged by adding
#[serde(untagged)] on the ProviderVendor enum so plain string values deserialize
into Other(String) while known unit variants still map to their named cases, and
add round-trip serde tests that serialize+deserialize known vendor slugs (e.g.,
"anthropic") and unknown slugs (e.g., "mistral") to prevent regressions.
In `@clients/rook/src/main.rs`:
- Around line 47-62: The CLI match arms for unimplemented variants
(Commands::Serve, Commands::Tui, Commands::Doctor, and the Commands::Config {
action: ConfigCommands::Export } arm) currently only print a stub and exit with
success; change them to return a non-zero exit status—either by calling
std::process::exit(1) after the println! or by refactoring main to return a
Result and returning an Err for those branches—so the program signals failure
for unimplemented commands.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b42664dd-33be-4e69-a64d-73f32a958243
⛔ Files ignored due to path filters (1)
clients/rook/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
clients/rook/Cargo.tomlclients/rook/README.mdclients/rook/src/config/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/src/domain/mod.rsclients/rook/src/gateway/mod.rsclients/rook/src/lib.rsclients/rook/src/main.rsclients/rook/src/registry/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/tui/mod.rs
📜 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). (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/rook/src/tui/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/registry/mod.rsclients/rook/src/config/mod.rsclients/rook/src/lib.rsclients/rook/src/gateway/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/src/main.rsclients/rook/src/domain/mod.rs
**/*
⚙️ 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:
clients/rook/src/tui/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/registry/mod.rsclients/rook/src/config/mod.rsclients/rook/src/lib.rsclients/rook/src/gateway/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/README.mdclients/rook/src/main.rsclients/rook/Cargo.tomlclients/rook/src/domain/mod.rs
**/*.{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:
clients/rook/README.md
🧠 Learnings (10)
📚 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/rook/src/routing/mod.rsclients/rook/src/registry/mod.rsclients/rook/src/config/mod.rsclients/rook/src/lib.rsclients/rook/src/gateway/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/src/domain/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:
clients/rook/src/config/mod.rsclients/rook/src/lib.rsclients/rook/src/gateway/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/README.mdclients/rook/src/main.rsclients/rook/Cargo.toml
📚 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/rook/src/lib.rsclients/rook/src/gateway/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/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/rook/src/gateway/mod.rsclients/rook/README.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/rook/src/gateway/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/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/rook/src/gateway/mod.rsclients/rook/README.mdclients/rook/src/main.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/rook/README.mdclients/rook/src/main.rsclients/rook/Cargo.toml
📚 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 : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/rook/src/main.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/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/rook/src/main.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 : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
clients/rook/Cargo.toml
🪛 markdownlint-cli2 (0.22.0)
clients/rook/README.md
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
clients/rook/src/lib.rs (1)
1-11: LGTM.Crate root is a clean module manifest; no premature re‑exports, no glob imports. Appropriate for a scaffolding PR.
clients/rook/Cargo.toml (1)
1-8:⚠️ Potential issue | 🟡 MinorAddress cross-workspace path dependency.
Rook is indeed standalone (no parent workspace exists). However, it depends on
corvus-traitsvia a path dependency into theclients/agent-runtimeworkspace (../agent-runtime/crates/corvus-traits). This cross-workspace path dependency works but indicates a structural issue.Better approaches:
- Merge both into a single root workspace (preferred for monorepos with shared internal crates).
- Publish
corvus-traitsto a registry and use normal versioned dependencies.Simply adding
[workspace]to rook won't resolve the fundamental cross-workspace coupling.⛔ Skipped due to 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/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additionsLearnt 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
Comment on lines
+40
to
+53
| ``` | ||
| src/ | ||
| ├── main.rs CLI entrypoint (clap subcommands) | ||
| ├── lib.rs Library root / module declarations | ||
| ├── domain/ Core data model: ProviderAccount, ProviderPool, | ||
| │ ModelRoute, RoutingPolicy, RookError | ||
| ├── registry/ SQLite persistence for all domain objects | ||
| ├── routing/ Request-time account selection (Priority, RoundRobin, | ||
| │ Weighted, Failover) and health-aware fallback | ||
| ├── gateway/ OpenAI-compatible axum HTTP handlers | ||
| ├── dashboard/ Embedded admin UI + management REST API | ||
| ├── tui/ Operator terminal interface (ratatui, future) | ||
| └── config/ Rook-specific TOML/env config loading | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced block.
This trips MD040; use text for the directory tree.
Lint fix
-```
+```text
src/
├── main.rs CLI entrypoint (clap subcommands)🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/rook/README.md` around lines 40 - 53, Update the fenced code block in
clients/rook/README.md (the directory tree block that currently begins with ```
) to include a language specifier (use `text`) so the block starts with ```text;
this resolves the MD040 lint by making the directory tree a `text`-fenced block.
Comment on lines
+47
to
+62
| match cli.command { | ||
| Commands::Serve => { | ||
| println!("rook serve: not yet implemented"); | ||
| } | ||
| Commands::Tui => { | ||
| println!("rook tui: not yet implemented"); | ||
| } | ||
| Commands::Doctor => { | ||
| println!("rook doctor: not yet implemented"); | ||
| } | ||
| Commands::Config { | ||
| action: ConfigCommands::Export, | ||
| } => { | ||
| println!("rook config export: not yet implemented"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Return a non-zero error for unimplemented commands.
These branches currently exit successfully after printing a stub message, which can make scripts or health checks believe serve, doctor, or config export completed correctly.
Behavior fix
- match cli.command {
+ match cli.command {
Commands::Serve => {
- println!("rook serve: not yet implemented");
+ anyhow::bail!("rook serve: not yet implemented");
}
Commands::Tui => {
- println!("rook tui: not yet implemented");
+ anyhow::bail!("rook tui: not yet implemented");
}
Commands::Doctor => {
- println!("rook doctor: not yet implemented");
+ anyhow::bail!("rook doctor: not yet implemented");
}
Commands::Config {
action: ConfigCommands::Export,
} => {
- println!("rook config export: not yet implemented");
+ anyhow::bail!("rook config export: not yet implemented");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| match cli.command { | |
| Commands::Serve => { | |
| println!("rook serve: not yet implemented"); | |
| } | |
| Commands::Tui => { | |
| println!("rook tui: not yet implemented"); | |
| } | |
| Commands::Doctor => { | |
| println!("rook doctor: not yet implemented"); | |
| } | |
| Commands::Config { | |
| action: ConfigCommands::Export, | |
| } => { | |
| println!("rook config export: not yet implemented"); | |
| } | |
| } | |
| match cli.command { | |
| Commands::Serve => { | |
| anyhow::bail!("rook serve: not yet implemented"); | |
| } | |
| Commands::Tui => { | |
| anyhow::bail!("rook tui: not yet implemented"); | |
| } | |
| Commands::Doctor => { | |
| anyhow::bail!("rook doctor: not yet implemented"); | |
| } | |
| Commands::Config { | |
| action: ConfigCommands::Export, | |
| } => { | |
| anyhow::bail!("rook config export: not yet implemented"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/rook/src/main.rs` around lines 47 - 62, The CLI match arms for
unimplemented variants (Commands::Serve, Commands::Tui, Commands::Doctor, and
the Commands::Config { action: ConfigCommands::Export } arm) currently only
print a stub and exit with success; change them to return a non-zero exit
status—either by calling std::process::exit(1) after the println! or by
refactoring main to return a Result and returning an Err for those branches—so
the program signals failure for unimplemented commands.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 6 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
clients/rook/src/main.rs (1)
47-66: 🧹 Nitpick | 🔵 TrivialPrefer
anyhow::bail!overstd::process::exit(1).
std::process::exitshort-circuits destructors and the tokio runtime teardown — harmless today with empty handlers, but a footgun onceserve/tuiopen sockets, DB handles, or spawn tasks. Sincemainalready returnsResult<()>, returningErris both idiomatic and CI-friendly (non-zero exit preserved, and the error is printed viaanyhow).♻️ Proposed change
match cli.command { Commands::Serve => { - println!("rook serve: not yet implemented"); - std::process::exit(1); + anyhow::bail!("rook serve: not yet implemented"); } Commands::Tui => { - println!("rook tui: not yet implemented"); - std::process::exit(1); + anyhow::bail!("rook tui: not yet implemented"); } Commands::Doctor => { - println!("rook doctor: not yet implemented"); - std::process::exit(1); + anyhow::bail!("rook doctor: not yet implemented"); } Commands::Config { action: ConfigCommands::Export, } => { - println!("rook config export: not yet implemented"); - std::process::exit(1); + anyhow::bail!("rook config export: not yet implemented"); } }As per coding guidelines ("Focus on Rust idioms … unchecked panics in production paths").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/main.rs` around lines 47 - 66, Replace the std::process::exit(1) calls in the match handling of cli.command (specifically inside the Commands::Serve, Commands::Tui, Commands::Doctor, and Commands::Config { action: ConfigCommands::Export } arms) with an anyhow error return (e.g., use anyhow::bail! or return Err(anyhow!(...))) so main's Result<()> propagates the non-zero exit instead of short-circuiting destructors; remove the exit calls and invoke bail! with the same "rook ...: not yet implemented" message for each arm.
🤖 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/rook/src/domain/mod.rs`:
- Around line 92-115: The ProviderAccount struct currently uses capabilities:
Vec<String>; replace this with the typed corvus_traits::ProviderCapabilities to
avoid divergent vocabularies: import ProviderCapabilities from corvus_traits,
change the capabilities field on ProviderAccount from Vec<String> to
ProviderCapabilities, and update any code that constructs,
serializes/deserializes, or matches on ProviderAccount::capabilities (e.g.,
ModelRoute::capability_constraints) to use the struct's fields instead of string
matching; keep tags as Vec<String> unless you want to migrate them too, and
ensure serde derives/traits still compile after the type substitution.
- Around line 64-74: The enum ProviderVendor has #[serde(untagged)] incorrectly
applied to the whole enum causing unit variants to serialize as null and strings
to be parsed into Other; remove #[serde(untagged)] from the enum-level
attributes and instead apply serde attributes on the Other variant so that
Other(String) is untagged (e.g., make Other carry #[serde(untagged)] or use the
appropriate per-variant serde annotation) while keeping rename_all =
"snake_case" for the unit variants; update the enum declaration around
ProviderVendor and the Other variant accordingly and run the full test suite
(cargo test --manifest-path clients/rook/Cargo.toml) to confirm the assertions
for serialization/deserialization pass.
---
Duplicate comments:
In `@clients/rook/src/main.rs`:
- Around line 47-66: Replace the std::process::exit(1) calls in the match
handling of cli.command (specifically inside the Commands::Serve, Commands::Tui,
Commands::Doctor, and Commands::Config { action: ConfigCommands::Export } arms)
with an anyhow error return (e.g., use anyhow::bail! or return
Err(anyhow!(...))) so main's Result<()> propagates the non-zero exit instead of
short-circuiting destructors; remove the exit calls and invoke bail! with the
same "rook ...: not yet implemented" message for each arm.
🪄 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: ceddabf1-2e96-4832-8172-58c1ce86fd6f
📒 Files selected for processing (3)
clients/rook/README.mdclients/rook/src/domain/mod.rsclients/rook/src/main.rs
📜 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). (2)
- GitHub Check: Cloudflare Pages
- GitHub Check: submit-gradle
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/rook/src/main.rsclients/rook/src/domain/mod.rs
**/*
⚙️ 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:
clients/rook/src/main.rsclients/rook/README.mdclients/rook/src/domain/mod.rs
**/*.{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:
clients/rook/README.md
🧠 Learnings (7)
📓 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/**/*.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
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
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
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
📚 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/rook/src/main.rsclients/rook/README.mdclients/rook/src/domain/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/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/rook/src/main.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:
clients/rook/src/main.rsclients/rook/README.mdclients/rook/src/domain/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/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/rook/src/main.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/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/rook/src/domain/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/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/rook/src/domain/mod.rs
🪛 markdownlint-cli2 (0.22.0)
clients/rook/README.md
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 75-75: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (1)
clients/rook/README.md (1)
40-53: Addtextlanguage hint to the fenced block.MD040 still trips on line 40 — the directory-tree block is unfenced. Adding
textsilences markdownlint and keeps rendering consistent.Lint fix
-``` +```text src/ ├── main.rs CLI entrypoint (clap subcommands)
Comment on lines
+92
to
+115
| /// A configured account for a specific AI provider. | ||
| /// | ||
| /// FIXME: persist via registry | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct ProviderAccount { | ||
| /// Unique stable identifier. | ||
| pub id: AccountId, | ||
| /// Which vendor this account belongs to. | ||
| pub vendor: ProviderVendor, | ||
| /// Human-readable label shown in the dashboard and TUI. | ||
| pub display_name: String, | ||
| /// Override the vendor's default API base URL (e.g., for local proxies). | ||
| pub api_base_override: Option<String>, | ||
| /// Whether this account is eligible for routing. | ||
| pub enabled: bool, | ||
| /// Relative weight used by [`SelectionStrategy::Weighted`]. | ||
| pub weight: u32, | ||
| /// Lower value = higher priority for [`SelectionStrategy::Priority`]. | ||
| pub priority: u32, | ||
| /// Free-form labels for grouping and filtering. | ||
| pub tags: Vec<String>, | ||
| /// Model capabilities advertised by this account. | ||
| pub capabilities: Vec<String>, | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider reusing corvus_traits::ProviderCapabilities instead of Vec<String>.
capabilities: Vec<String> and tags: Vec<String> are free-form bags that will drift from the structured ProviderCapabilities already defined in clients/agent-runtime/crates/corvus-traits/src/providers.rs (fields like native_tool_calling, image_input, image_transport_forms). Since Rook depends on corvus-traits per the README, using the typed struct here prevents divergent "capability" vocabularies between the router and the runtime, and lets routing logic in ModelRoute::capability_constraints match on real fields instead of matching strings. Non-blocking for the skeleton, but worth aligning before the registry lands and freezes the schema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/rook/src/domain/mod.rs` around lines 92 - 115, The ProviderAccount
struct currently uses capabilities: Vec<String>; replace this with the typed
corvus_traits::ProviderCapabilities to avoid divergent vocabularies: import
ProviderCapabilities from corvus_traits, change the capabilities field on
ProviderAccount from Vec<String> to ProviderCapabilities, and update any code
that constructs, serializes/deserializes, or matches on
ProviderAccount::capabilities (e.g., ModelRoute::capability_constraints) to use
the struct's fields instead of string matching; keep tags as Vec<String> unless
you want to migrate them too, and ensure serde derives/traits still compile
after the type substitution.
- Add #[serde(untagged)] to the Other(String) variant so it round-trips
as a bare string instead of {"other":"…"}
- Add 5 unit tests covering unit-variant snake_case, Other bare-string
serialization, deserialization, and round-trip
Skipped: capabilities Vec<String> → ProviderCapabilities swap because
corvus_traits::ProviderCapabilities lacks Serialize/Deserialize derives;
that migration belongs in a separate issue against corvus-traits.
Skipped: process::exit(1) removal — no such calls exist in main.rs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/rook/src/domain/mod.rs`:
- Around line 68-82: ProviderVendor's #[serde(untagged)] Other(String) silently
accepts misspelled/ differently-cased known vendors; replace the auto-derived
behavior by removing #[serde(untagged)] and adding a custom Deserialize impl for
ProviderVendor that: deserializes the input as a string, normalizes it (e.g.
lowercase, map "_" -> ""), checks against the canonical vendor tokens for
OpenAi, Anthropic, Google, OpenRouter, DeepSeek and returns the matching unit
variant when normalized-equal, otherwise returns Other(original_string);
additionally detect near-misses (using a simple similarity check or normalized
equality) and return a deserialization error or a clear warning message for
close-miss cases so typos like "openai" / "OpenAI" / "open-ai" don't silently
become Other; implement this logic in the Deserialize impl for ProviderVendor.
- Around line 43-56: The newtype ID structs AccountId, PoolId, and RouteId
currently wrap a private Uuid with no public constructor or accessor; add public
constructors and accessors (e.g., an associated fn new(uuid: Uuid) -> Self, fn
generate() -> Self to mint a random Uuid, and fn as_uuid(&self) -> Uuid or
into_inner(self) -> Uuid) for each type so callers in registry/routing/gateway
can create and read the inner Uuid; implement From<Uuid> and Into<Uuid> (or
AsRef<Uuid>) and consider implementing Display/Debug wrappers if needed to help
logging—apply the same pattern to AccountId, PoolId, and RouteId (or extract
into a small macro to avoid duplication).
🪄 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: 3ee5f0e1-039e-445a-9307-9c6f0fac2753
📒 Files selected for processing (1)
clients/rook/src/domain/mod.rs
📜 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). (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/rook/src/domain/mod.rs
**/*
⚙️ 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:
clients/rook/src/domain/mod.rs
🧠 Learnings (7)
📓 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
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
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
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
📚 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/rook/src/domain/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:
clients/rook/src/domain/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/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/rook/src/domain/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/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/rook/src/domain/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/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/rook/src/domain/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/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
clients/rook/src/domain/mod.rs
3 tasks
This was referenced Apr 20, 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 issue #580 — establishes the complete
clients/rook/package layout for Corvus Rook v1, the local-first provider gateway.What's included
clients/rook/Cargo.toml— standalone Rust workspace with path deps to sharedagent-runtimecratesclients/rook/src/main.rs— CLI withclap: subcommandsserve,tui,doctor,config exportclients/rook/src/lib.rs— module declarationsclients/rook/src/domain/mod.rs— core domain types:ProviderVendor,ProviderAccount,ProviderPool,ModelRoute,RoutingPolicy,SelectionStrategy,RookErrorregistry,routing,gateway,dashboard,tui,configclients/rook/README.md— product READMEValidation
cargo check --manifest-path clients/rook/Cargo.toml— clean (0 errors, 0 warnings)Part of
Milestone: Corvus Rook v1 (#5)
Parent: Foundation (#573)
Close #580