feat(rook): add admin API for operator management#629
Conversation
Expose stable /api endpoints for accounts, pools, routes, health, settings, and usage status so the dashboard and automation can manage Rook through supported contracts instead of internal runtime state.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughImplements a complete administrative HTTP API under Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AdminHandler as Admin Handler<br/>/api/accounts/{id}
participant Registry as RookRegistry<br/>(Services)
participant DB as SQLite DB
participant Health as Health Tracker
Client->>AdminHandler: DELETE /api/accounts/{id}
activate AdminHandler
AdminHandler->>AdminHandler: Validate account_id format
AdminHandler->>Registry: account().get(id).await
activate Registry
Registry->>DB: fetch account by id
activate DB
DB-->>Registry: account (or not found)
deactivate DB
deactivate Registry
alt Account not found
AdminHandler-->>Client: 404 not_found response
else Account found
AdminHandler->>Registry: account().is_account_referenced_by_pool(id).await
activate Registry
Registry->>DB: SELECT 1 FROM pool_members WHERE account_id=?
activate DB
DB-->>Registry: row exists? (bool)
deactivate DB
deactivate Registry
alt Referenced by pool
AdminHandler-->>Client: 409 Conflict (reference_conflict)
else Not referenced
AdminHandler->>Registry: account().delete(id).await
activate Registry
Registry->>DB: DELETE FROM accounts WHERE id=?
activate DB
DB-->>Registry: rows_affected
deactivate DB
Registry->>Health: cleanup_health_for(id)
activate Health
Health-->>Registry: ok
deactivate Health
deactivate Registry
AdminHandler-->>Client: 204 No Content
end
end
deactivate AdminHandler
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-21 to 2026-04-21 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
clients/rook/src/db/pool.rs (1)
78-105:⚠️ Potential issue | 🟠 MajorValidate
fallback_pool_idbefore inserting pools.
fallback_pool_idis persisted directly, while the provided schema shows only a nullableTEXTcolumn. That allows admin-created pools to point at non-existent fallback pools, undermining the new delete/reference-integrity semantics.Proposed guard
pub async fn insert_pool(&self, pool: &ProviderPool) -> Result<(), RookError> { let id = pool.id.to_string(); let strategy_str = strategy_to_db_str(&pool.strategy)?; let fallback = pool.fallback_pool_id.as_ref().map(|p| p.to_string()); let now = Utc::now().to_rfc3339(); // Start transaction to make pool + members insertion atomic let mut tx = self .pool() .begin() .await .map_err(|e| RookError::Registry(format!("failed to begin transaction: {e}")))?; + + if let Some(fallback_pool_id) = &pool.fallback_pool_id { + if fallback_pool_id == &pool.id { + return Err(RookError::Registry(format!( + "fallback_pool_id {} cannot reference the pool itself", + fallback_pool_id + ))); + } + + let fallback_exists = + sqlx::query("SELECT 1 FROM provider_pools WHERE id = ? LIMIT 1") + .bind(fallback_pool_id.to_string()) + .fetch_optional(&mut *tx) + .await + .map_err(|e| { + RookError::Registry(format!("failed to validate fallback_pool_id: {e}")) + })?; + + if fallback_exists.is_none() { + return Err(RookError::Registry(format!( + "fallback_pool_id {} does not exist", + fallback_pool_id + ))); + } + } // Insert pool sqlx::query(As per coding guidelines,
**/*: Security first, performance second. Validate input boundaries, auth/authz implications, and secret management.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/db/pool.rs` around lines 78 - 105, The insert_pool function currently persists pool.fallback_pool_id without checking it exists; before executing the INSERT in insert_pool, validate that if pool.fallback_pool_id.is_some() then that id exists in provider_pools (or whatever canonical provider table) and return a RookError::Registry if it does not; perform the existence check within the same transaction (use the mut tx already created) using a SELECT 1 WHERE id = ? and only proceed with the INSERT when the check succeeds (treat NULL/None as allowed), so provider_pools.fallback_pool_id cannot reference a non-existent pool and the operation remains atomic.clients/rook/src/db/route.rs (1)
180-197:⚠️ Potential issue | 🟠 MajorDelete routes with an atomic fallback-reference guard.
Line 181 checks references, then Line 197 deletes later. A concurrent insert can add
fallback_route_id = idbetween those statements, bypassing the guard. Use a transaction/conditional delete such asDELETE ... WHERE id = ? AND NOT EXISTS (...), then re-query only to classify “not found” vs “referenced”.As per coding guidelines,
**/*.rs: Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/db/route.rs` around lines 180 - 197, The current two-step check (the referencing query using referencing and id_str) then DELETE (result) is vulnerable to a race where a concurrent insert sets fallback_route_id between the checks; change to a single conditional delete inside the same DB operation: perform a DELETE FROM model_routes WHERE id = ? AND NOT EXISTS (SELECT 1 FROM model_routes WHERE fallback_route_id = ?) (using self.pool() and binding id_str twice), then inspect the affected rows to decide whether to return success, return RookError::Registry indicating "referenced by route ..." (by re-querying the referencing route only if rows_affected == 0 and the route still exists), or return not-found if the route itself does not exist—replace the separate referencing lookup and delete (the referencing variable and subsequent query("DELETE ...")) with this atomic conditional delete and the follow-up disambiguation logic.clients/rook/src/services/pool.rs (1)
188-197:⚠️ Potential issue | 🔴 CriticalMake SQLite pool updates atomic and preserve not-found semantics.
This path currently deletes first and reinserts later, so an unknown pool ID becomes a create, and a failed reinsert can leave the existing pool deleted. Add a real transactional update/upsert-with-existence-check before exposing this through admin
PUT /api/pools/{pool_id}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/services/pool.rs` around lines 188 - 197, The update(&self, pool: ProviderPool) implementation deletes then reinserts which turns unknown IDs into creates and risks data loss on failure; change update to run inside a DB transaction on self.db.pool() (begin/commit/rollback), first SELECT FOR UPDATE or query provider_pools WHERE id = ? to verify existence, return a not-found RookError if absent, then perform an UPDATE (or an upsert) of the existing row (or delete+insert inside the same transaction) and only commit if all steps succeed; reference the update function, ProviderPool type, existing sqlx::query("DELETE FROM provider_pools WHERE id = ?") usage and self.db.insert_pool to locate where to introduce the transaction and existence-check logic.clients/rook/src/server/mod.rs (1)
61-80:⚠️ Potential issue | 🟠 MajorGate the admin router before exposing mutating endpoints.
/apinow includes unauthenticated CRUD/delete/settings routes and inherits the configured bind address; withhost = "0.0.0.0", these admin endpoints become remotely reachable before#591lands. Keep/apiloopback-only or add auth middleware before nesting it.🛡️ Minimal loopback guard until admin auth exists
async fn build_app_with_registry( - _config: ServerConfig, + config: ServerConfig, registry: RookRegistry, ) -> Result<Router, RookError> { + let addr: SocketAddr = config + .socket_addr() + .parse() + .map_err(|e: std::net::AddrParseError| RookError::Config(e.to_string()))?; + if !addr.ip().is_loopback() { + return Err(RookError::Config( + "admin API requires a loopback bind until admin auth is implemented".to_string(), + )); + } + let engine = RoutingEngine::new(registry.clone());If
/v1must still support remote binds before#591, split admin/gateway listeners or protect only the nested admin router with middleware. As per coding guidelines,**/*: Security first, performance second.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/server/mod.rs` around lines 61 - 80, The admin router is being exposed unguarded in build_app_with_registry via .nest("/api", admin::build_router(registry)), which makes unauthenticated mutating endpoints reachable; either restrict the admin nest to loopback only or wrap it with auth middleware before adding it to the Router. Update build_app_with_registry to (a) apply an authentication/middleware layer around admin::build_router(registry) (e.g., using tower/axum middleware) or (b) remove the admin nest from this externally bound Router and instead serve admin::build_router(registry) on a loopback-only listener (split admin/gateway listeners), ensuring the change touches the Router construction where .nest("/api", ...) is invoked.
🤖 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/admin/handlers.rs`:
- Around line 32-69: The handlers currently leak internal error detail by
returning other.to_string() in classify_rook_error and the raw message in
classify_registry_message's default branch; change both to return a generic
transport message (e.g. use admin_error_response("internal_error", "Internal
server error") or similar) while preserving the original error text only for
logging; update classify_rook_error (the match arm for `other`) and the fallback
branch of classify_registry_message to use the generic message instead of the
raw error string.
- Around line 13-23: Handlers currently use raw Path and Json extractors (Path,
ExtractJson/Json) which let Axum return its default rejections; update those
handlers to accept rejection-aware extractors or explicitly map rejections into
the AdminErrorResponse envelope: change handler parameters from Path<T> and
Json<T> to Result<Path<T>, axum::extract::rejection::PathRejection> and
Result<ExtractJson<T>, axum::extract::rejection::JsonRejection> (or use a custom
wrapper extractor), then convert any Err(rejection) into the
AdminJson/AdminCreated/AdminEmpty error variant by constructing
(StatusCode::BAD_REQUEST, Json(AdminErrorResponse { error: ... })) so all
invalid UUIDs and malformed JSON produce the documented AdminErrorResponse;
ensure you reference and use the AdminJson and AdminErrorResponse types in the
handler return signatures.
In `@clients/rook/src/admin/mod.rs`:
- Around line 165-183: Add a new unified composition test (or extend
admin_router_preserves_health_and_usage_placeholder) that builds the composed
server via test_api_app() and build_router(registry) and then issues sequential
requests for "/api/health", "/v1/models", "/", and an "/assets/<some-file>" path
using the existing helpers request_text and request_json; assert expected
StatusCode and bodies for each response to ensure no route shadowing (verify
health returns "ok", usage/usage_json assertions remain, models endpoint returns
expected v1 models JSON, root "/" returns expected index, and "/assets/..."
returns the asset content). Use the same helper functions (test_api_app,
build_router, request_text, request_json) so the test exercises the full nested
router composition and covers the previously untested "/assets/*" route.
In `@clients/rook/src/services/account.rs`:
- Around line 149-157: The current delete method in AccountService uses two
separate DB calls (is_account_referenced_by_pool and delete_account) which
allows a race where a pool membership can be inserted between them; change this
to an atomic DB operation: implement a transactional/conditional delete in your
DB layer (e.g., a new method like delete_account_if_not_referenced or
delete_account_tx) that attempts to delete the account only if no pool
references exist and returns a discriminated result (Deleted, Referenced,
NotFound); update AccountService::delete to call that single DB operation
(replace is_account_referenced_by_pool + delete_account) and map the Returned
enum into the appropriate RookError variants so you can distinguish “not found”
from “referenced” after the atomic operation.
In `@openspec/changes/rook-590-admin-api/design.md`:
- Around line 773-788: The design currently says handle_remove_pool_member
should return 404 for removing a non-member but the implementation/verification
shows a 409 Conflict; update the design to match the implemented contract: in
the `handle_remove_pool_member` errors list, change the "unknown membership"
outcome to `409 Conflict` (or alternatively, if you prefer the 404 contract,
modify the implementation path `registry.pools().remove_member(pool_id,
account_id).await?` and associated tests to return 404 instead); ensure
references to `handle_remove_pool_member`, `registry.pools().remove_member`, and
`registry.pools().get` are consistent with the chosen behavior.
In `@openspec/changes/rook-590-admin-api/proposal.md`:
- Around line 110-114: The Settings endpoints list incorrectly advertises PATCH
/api/settings as supported; update the proposal so it reflects the MVP contract
by removing `PATCH /api/settings` from the endpoint list or explicitly marking
it as unsupported/“TBD” (i.e., leave only `GET /api/settings` and `PUT
/api/settings` or add a note like “PATCH /api/settings — unsupported in MVP”) to
match the design and tasks.
- Around line 55-57: The proposal references stale paths under
clients/agent-runtime (e.g., "clients/agent-runtime/src/gateway/admin/mod.rs",
"clients/agent-runtime/src/gateway/admin/types.rs",
"clients/agent-runtime/src/gateway/admin/handlers.rs"); update those occurrences
to the correct implementation paths under clients/rook (replace each
"clients/agent-runtime/..." with the corresponding "clients/rook/..." path) and
scan the rest of proposal.md (including the noted ranges around lines 133-137
and 177-178) to correct any other similar stale path references so the
documented implementation areas match the actual PR.
In `@openspec/changes/rook-590-admin-api/spec.md`:
- Around line 839-843: The example error object under the "code":
"resource_in_use" entry uses a numeric id ("123") which violates the spec
requiring UUID-string IDs; update the example so the details.id (and any id
shown inside the message text, e.g., "pool 123") are replaced with a UUID-shaped
string (for example a v4 UUID) and adjust the message to reference that UUID
string so the example aligns with the documented contract.
- Around line 746-755: The documented HealthAccountView JSON schema is out of
sync with the Rust transport struct used by clients/rook (see HealthAccountView
in clients/rook/src/admin/types.rs) which serializes display_name, vendor,
enabled, and is_available; update the spec to include these fields
(display_name, vendor, enabled, is_available) with their types and
nullable/optional semantics to match the implemented struct, or remove those
fields from the Rust transport view so both the spec and the HealthAccountView
implementation share one stable contract; ensure the doc example's keys,
nullability, and default values exactly mirror the serialization behavior in
HealthAccountView.
In `@openspec/changes/rook-590-admin-api/verify-report.md`:
- Around line 151-161: The verification report incorrectly shows a PASS while
two required gates failed: the formatting check (cargo fmt --manifest-path
"clients/rook/Cargo.toml" --all -- --check) and clippy (cargo clippy
--manifest-path "clients/rook/Cargo.toml" --all-targets -- -D warnings); run and
fix formatting issues raised by cargo fmt and resolve all clippy warnings in the
Rook crate (clients/rook) so cargo fmt and cargo clippy succeed, then re-run the
three gates (cargo test, cargo fmt --check, cargo clippy ...) and update the
verification state to FAILED/INCOMPLETE if any gate still fails rather than
marking as PASS.
- Around line 182-186: The implementation currently relies on Axum's default
extraction behavior for malformed path IDs, but the design requires returning
the admin `400 bad_request` envelope; change the route handling for path IDs
(where Path<...>/Uuid extraction or parse_id helper is used) to explicitly catch
UUID parse failures and convert them into the admin bad_request response (either
by implementing a custom extractor that maps parse errors to the admin error
envelope or by adding a rejection-to-response mapping in the app's
handle_rejection middleware), and add unit/integration tests asserting that
malformed path IDs produce the admin `400 bad_request` envelope rather than Axum
defaults.
---
Outside diff comments:
In `@clients/rook/src/db/pool.rs`:
- Around line 78-105: The insert_pool function currently persists
pool.fallback_pool_id without checking it exists; before executing the INSERT in
insert_pool, validate that if pool.fallback_pool_id.is_some() then that id
exists in provider_pools (or whatever canonical provider table) and return a
RookError::Registry if it does not; perform the existence check within the same
transaction (use the mut tx already created) using a SELECT 1 WHERE id = ? and
only proceed with the INSERT when the check succeeds (treat NULL/None as
allowed), so provider_pools.fallback_pool_id cannot reference a non-existent
pool and the operation remains atomic.
In `@clients/rook/src/db/route.rs`:
- Around line 180-197: The current two-step check (the referencing query using
referencing and id_str) then DELETE (result) is vulnerable to a race where a
concurrent insert sets fallback_route_id between the checks; change to a single
conditional delete inside the same DB operation: perform a DELETE FROM
model_routes WHERE id = ? AND NOT EXISTS (SELECT 1 FROM model_routes WHERE
fallback_route_id = ?) (using self.pool() and binding id_str twice), then
inspect the affected rows to decide whether to return success, return
RookError::Registry indicating "referenced by route ..." (by re-querying the
referencing route only if rows_affected == 0 and the route still exists), or
return not-found if the route itself does not exist—replace the separate
referencing lookup and delete (the referencing variable and subsequent
query("DELETE ...")) with this atomic conditional delete and the follow-up
disambiguation logic.
In `@clients/rook/src/server/mod.rs`:
- Around line 61-80: The admin router is being exposed unguarded in
build_app_with_registry via .nest("/api", admin::build_router(registry)), which
makes unauthenticated mutating endpoints reachable; either restrict the admin
nest to loopback only or wrap it with auth middleware before adding it to the
Router. Update build_app_with_registry to (a) apply an authentication/middleware
layer around admin::build_router(registry) (e.g., using tower/axum middleware)
or (b) remove the admin nest from this externally bound Router and instead serve
admin::build_router(registry) on a loopback-only listener (split admin/gateway
listeners), ensuring the change touches the Router construction where
.nest("/api", ...) is invoked.
In `@clients/rook/src/services/pool.rs`:
- Around line 188-197: The update(&self, pool: ProviderPool) implementation
deletes then reinserts which turns unknown IDs into creates and risks data loss
on failure; change update to run inside a DB transaction on self.db.pool()
(begin/commit/rollback), first SELECT FOR UPDATE or query provider_pools WHERE
id = ? to verify existence, return a not-found RookError if absent, then perform
an UPDATE (or an upsert) of the existing row (or delete+insert inside the same
transaction) and only commit if all steps succeed; reference the update
function, ProviderPool type, existing sqlx::query("DELETE FROM provider_pools
WHERE id = ?") usage and self.db.insert_pool to locate where to introduce the
transaction and existence-check logic.
🪄 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: 662a7daf-365a-444f-8bd9-4db08d7d3069
📒 Files selected for processing (30)
clients/rook/src/admin/handlers.rsclients/rook/src/admin/mod.rsclients/rook/src/admin/types.rsclients/rook/src/dashboard/mod.rsclients/rook/src/db/account.rsclients/rook/src/db/mod.rsclients/rook/src/db/pool.rsclients/rook/src/db/route.rsclients/rook/src/db/settings.rsclients/rook/src/domain/mod.rsclients/rook/src/gateway/handlers.rsclients/rook/src/gateway/mod.rsclients/rook/src/gateway/types.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/vendor.rsclients/rook/src/lib.rsclients/rook/src/registry/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/services/account.rsclients/rook/src/services/health.rsclients/rook/src/services/pool.rsclients/rook/src/services/route.rsclients/rook/src/services/settings.rsopenspec/changes/rook-590-admin-api/design.mdopenspec/changes/rook-590-admin-api/proposal.mdopenspec/changes/rook-590-admin-api/spec.mdopenspec/changes/rook-590-admin-api/state.yamlopenspec/changes/rook-590-admin-api/tasks.mdopenspec/changes/rook-590-admin-api/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). (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/lib.rsclients/rook/src/gateway/types.rsclients/rook/src/gateway/vendor.rsclients/rook/src/gateway/mod.rsclients/rook/src/services/settings.rsclients/rook/src/services/health.rsclients/rook/src/services/account.rsclients/rook/src/db/mod.rsclients/rook/src/db/account.rsclients/rook/src/registry/mod.rsclients/rook/src/server/mod.rsclients/rook/src/db/route.rsclients/rook/src/db/settings.rsclients/rook/src/dashboard/mod.rsclients/rook/src/gateway/handlers.rsclients/rook/src/db/pool.rsclients/rook/src/services/route.rsclients/rook/src/routing/mod.rsclients/rook/src/services/pool.rsclients/rook/src/domain/mod.rsclients/rook/src/admin/mod.rsclients/rook/src/gateway/upstream.rsclients/rook/src/admin/handlers.rsclients/rook/src/admin/types.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/lib.rsclients/rook/src/gateway/types.rsclients/rook/src/gateway/vendor.rsclients/rook/src/gateway/mod.rsopenspec/changes/rook-590-admin-api/state.yamlclients/rook/src/services/settings.rsclients/rook/src/services/health.rsclients/rook/src/services/account.rsclients/rook/src/db/mod.rsclients/rook/src/db/account.rsclients/rook/src/registry/mod.rsclients/rook/src/server/mod.rsclients/rook/src/db/route.rsclients/rook/src/db/settings.rsclients/rook/src/dashboard/mod.rsclients/rook/src/gateway/handlers.rsclients/rook/src/db/pool.rsopenspec/changes/rook-590-admin-api/verify-report.mdclients/rook/src/services/route.rsclients/rook/src/routing/mod.rsopenspec/changes/rook-590-admin-api/design.mdopenspec/changes/rook-590-admin-api/proposal.mdclients/rook/src/services/pool.rsopenspec/changes/rook-590-admin-api/tasks.mdclients/rook/src/domain/mod.rsopenspec/changes/rook-590-admin-api/spec.mdclients/rook/src/admin/mod.rsclients/rook/src/gateway/upstream.rsclients/rook/src/admin/handlers.rsclients/rook/src/admin/types.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:
openspec/changes/rook-590-admin-api/verify-report.mdopenspec/changes/rook-590-admin-api/design.mdopenspec/changes/rook-590-admin-api/proposal.mdopenspec/changes/rook-590-admin-api/tasks.mdopenspec/changes/rook-590-admin-api/spec.md
🧠 Learnings (6)
📚 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/gateway/vendor.rsclients/rook/src/db/mod.rsopenspec/changes/rook-590-admin-api/verify-report.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/{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/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.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/db/mod.rsclients/rook/src/db/route.rsclients/rook/src/db/pool.rsclients/rook/src/services/route.rsclients/rook/src/routing/mod.rsclients/rook/src/domain/mod.rsclients/rook/src/gateway/upstream.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/registry/mod.rsclients/rook/src/server/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/services/pool.rsclients/rook/src/domain/mod.rsclients/rook/src/admin/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/server/mod.rsclients/rook/src/gateway/upstream.rs
🪛 LanguageTool
openspec/changes/rook-590-admin-api/design.md
[style] ~245-~245: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...| clients/rook/src/services/pool.rs | Maybe modify | Only if needed to tighten not-...
(REP_MAYBE)
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... clients/rook/src/services/route.rs | Maybe modify | Only if needed to preserve sta...
(REP_MAYBE)
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ices/route.rs` | Maybe modify | Only if needed to preserve stable not-found/conflict mapp...
(REP_NEED_TO_VB)
openspec/changes/rook-590-admin-api/proposal.md
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., get, update, and delete operations. - Expose pool membership endpoints for adding an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~29-~29: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...emoving provider accounts from pools. - Expose route management endpoints for create, ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., get, update, and delete operations. - Expose health read endpoints for account-level...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... records and aggregate summary views. - Expose settings read and update endpoints. - A...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/rook-590-admin-api/spec.md
[style] ~32-~32: Consider removing “of” to be more concise
Context: ...ok. The composed server MUST preserve all of the following at the same time: - GET /ap...
(ALL_OF_THE)
🔇 Additional comments (11)
clients/rook/src/gateway/vendor.rs (1)
51-54: Looks good — test-only formatting change with no behavior impact.
These assertion reflows preserve semantics and improve readability.Also applies to: 61-64, 69-72
clients/rook/src/routing/mod.rs (2)
45-809: LGTM: Clean formatting pass.The import reorganization, struct literal alignment, argument wrapping, and test assertion formatting improve readability without changing behavior.
813-813: Nice improvement: idiomatic range check.Using
(24..=32).contains(&heavy_count)is more idiomatic Rust than the manual>=and<=comparison. Clearer intent.clients/rook/src/gateway/mod.rs (1)
14-14: No issues in module reorder.
This is a non-functional ordering change and does not affect gateway behavior.clients/rook/src/gateway/types.rs (1)
281-284: Assertion reformat is safe.
No behavioral change in error-shape validation.clients/rook/src/services/health.rs (1)
107-109: Health-service edits are behavior-preserving.
These are readability-only refactors; lock/error handling and health updates remain intact.Also applies to: 128-130, 149-151, 156-156
clients/rook/src/dashboard/mod.rs (1)
13-13: Dashboard changes look safe.
Only formatting/import normalization; no functional impact.Also applies to: 16-16, 92-95
clients/rook/src/domain/mod.rs (1)
371-390: Test-formatting updates are fine.
No behavior changes to vendor parsing or serialization coverage.Also applies to: 436-437
clients/rook/src/db/settings.rs (1)
147-155: Settings test refactor is correct.
Struct-literal initialization keeps the same assertions and improves clarity.Also applies to: 170-173, 176-179
clients/rook/src/services/settings.rs (1)
48-51: No concerns in settings-service changes.
Behavior is preserved in both in-memory save path and test setup updates.Also applies to: 103-108, 122-125, 128-131
clients/rook/src/registry/mod.rs (1)
26-27: Registry edits are non-functional and safe.
No break in service composition or settings round-trip intent.Also applies to: 114-115, 153-156
Wrap admin extractor failures in the documented error envelope, avoid leaking internal error details, make account and route deletes safer under references, validate pool fallbacks transactionally, and align Rook admin docs with the verified contract.
Deploying corvus with
|
| Latest commit: |
b20252a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://26459d05.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-rook-590-admin-api.corvus-42x.pages.dev |
1 participant
Related Issues
closes #590
Summary
/apiadmin surface for Rook backed by shared registry/services instead of internal-only management state/v1gateway and dashboard routes while preserving redacted account responses (has_api_key, never rawapi_key)Tested Information
cargo test --manifest-path "clients/rook/Cargo.toml"cargo fmt --manifest-path "clients/rook/Cargo.toml" --all -- --checkcargo clippy --manifest-path "clients/rook/Cargo.toml" --all-targets -- -D warningsrook-590-admin-api: PASSReviewer focus:
/api+ coexistence with/v1and dashboardDocumentation Impact
openspec/changes/rook-590-admin-api/proposal.mdopenspec/changes/rook-590-admin-api/spec.mdopenspec/changes/rook-590-admin-api/design.mdopenspec/changes/rook-590-admin-api/tasks.mdopenspec/changes/rook-590-admin-api/verify-report.mdBreaking Changes
None.
Checklist