feat(rook): add OpenAI-compatible gateway endpoints#627
Conversation
Expose /v1/chat/completions and /v1/models so OpenAI-style clients can route through Rook with account-aware upstream proxying. Persist provider API keys locally and archive the completed gateway spec for future iteration.
Deploying corvus with
|
| Latest commit: |
5ddeb41
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bd3c7b46.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-rook-589-gateway-api.corvus-42x.pages.dev |
|
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 24 minutes and 47 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 (14)
📝 WalkthroughWalkthroughThis PR implements an OpenAI-compatible gateway for Rook, adding database support for per-account API keys, upstream HTTP proxying with vendor-specific auth, and two new endpoints ( Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as Handler<br/>(chat_completions)
participant Engine as RoutingEngine
participant Upstream as Upstream<br/>Provider
participant Health as Health<br/>Tracker
Client->>Handler: POST /v1/chat/completions<br/>(model, messages, stream=false)
Handler->>Handler: Parse request<br/>Reject if stream=true
Handler->>Engine: resolve(model)
Engine-->>Handler: ProviderAccount<br/>(with api_key)
Handler->>Upstream: POST {vendor_url}/v1/chat<br/>(auth header, raw JSON body)
alt Upstream Success (2xx)
Upstream-->>Handler: 200 + response body
Handler->>Health: mark_success(account)
Handler-->>Client: 200 + response
else Upstream Error
Upstream-->>Handler: 4xx/5xx or timeout
Handler->>Health: mark_failure(account,<br/>cooldown=60s)
Handler->>Handler: Map to GatewayErrorResponse<br/>(502/504 with code)
Handler-->>Client: Error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PR spans multiple systems (database, HTTP proxying, vendor-specific logic, error mapping) with substantial new code, complex error handling paths, and security-sensitive auth header construction requiring careful verification across all control flows. 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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
clients/rook/src/main.rs (1)
61-67:⚠️ Potential issue | 🟡 Minor
db_pathis unreachable from the CLI.
ServerConfig.db_pathexists and branches in-memory vs on-disk inbuild_app, butServehard-codesNone, so operators cannot override./rook.dbor select:memory:without code changes. Consider threading a--db-patharg through, or removing the field until it's exposed.Proposed CLI wiring
Serve { /// Host address to bind to #[arg(long, default_value = "127.0.0.1")] host: String, /// TCP port to listen on #[arg(long, default_value_t = 4141)] port: u16, /// Enable the operator TUI alongside the HTTP server #[arg(long, default_value_t = false)] tui: bool, + + /// Path to the SQLite database file (use ":memory:" for ephemeral) + #[arg(long)] + db_path: Option<String>, }, @@ - Commands::Serve { host, port, tui } => { + Commands::Serve { host, port, tui, db_path } => { let config = ServerConfig { host, port, enable_tui: tui, - db_path: None, + db_path, };🤖 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 61 - 67, ServerConfig.db_path is hard-coded to None in the Commands::Serve arm so the CLI cannot choose an on-disk or :memory: DB; update the CLI wiring to surface a --db-path option on the Serve command and thread that value into ServerConfig (i.e., add a db_path field to the Serve variant and populate ServerConfig.db_path from it in the Serve match arm), or if you prefer to defer DB selection remove the ServerConfig.db_path field and its branches in build_app to avoid dead config.clients/rook/src/domain/mod.rs (1)
236-259:⚠️ Potential issue | 🟠 MajorSecret leak via
Debug:api_keywill be printed by any{:?}ofProviderAccount.
ProviderAccountderivesDebugand now containsapi_key: Option<String>. Anywhere this struct (or anything wrapping it — pools, routing decisions, error contexts,tracingspans/fields) is formatted with{:?}, the raw key lands in logs. Given the PR explicitly addstracing/structured logging as a follow-up NFR, this will almost certainly end up in log aggregation. "Plaintext at rest until#591" is fine; "plaintext in stdout" is not.As per coding guidelines: "Security first... secret management" and "Prioritize... secret handling."
🛡️ Suggested fix — redact in Debug
Either drop the auto-derive and implement
Debugmanually, or wrap the secret:-#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct ProviderAccount { ... pub api_key: Option<String>, ... } + +impl fmt::Debug for ProviderAccount { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProviderAccount") + .field("id", &self.id) + .field("vendor", &self.vendor) + .field("display_name", &self.display_name) + .field("api_base_override", &self.api_base_override) + .field("api_key", &self.api_key.as_ref().map(|_| "***redacted***")) + .field("enabled", &self.enabled) + .field("weight", &self.weight) + .field("priority", &self.priority) + .field("tags", &self.tags) + .field("capabilities", &self.capabilities) + .finish() + } +}Also worth auditing
Serializeusage — ifProviderAccountis ever returned over an admin API, you'll want a separate DTO withoutapi_key(or with it write-only).🤖 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 236 - 259, ProviderAccount currently derives Debug and contains api_key, which causes secrets to appear in logs; replace the auto-derived Debug for ProviderAccount with a manual impl that redacts api_key (e.g., print "<redacted>" or omit the field) or wrap api_key in a secret type that implements Debug safely, and likewise ensure Serialize does not expose the secret by either removing api_key from the public DTO/impl or implementing custom Serialize/Deserialize to prevent dumping the key; update references to ProviderAccount (pools, routing, error contexts, tracing spans) to use the redacted/debug-safe representation.clients/rook/src/server/mod.rs (1)
137-161: 🧹 Nitpick | 🔵 TrivialAdd a test asserting
db_pathdefault resolves to./rook.db.
server_config_default_valuesdoesn't assertdb_pathisNone, and no test covers theunwrap_or("./rook.db")fallback path inbuild_app. A regression that flips the default would slip through.fn server_config_default_values() { let cfg = ServerConfig::default(); assert_eq!(cfg.host, "127.0.0.1"); assert_eq!(cfg.port, 4141); assert!(!cfg.enable_tui); + assert!(cfg.db_path.is_none()); }🤖 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 137 - 161, Add an assertion in the server_config_default_values test to verify ServerConfig::default() sets db_path to None and add a new test (or extend server_config_default_values) that simulates build_app's fallback behavior by asserting that when db_path is None the effective DB path used by build_app (or the unwrap_or("./rook.db") logic) resolves to "./rook.db"; reference the ServerConfig struct, the server_config_default_values test, the db_path field, and the build_app unwrap_or("./rook.db") fallback so CI will catch regressions flipping the default db path.
🤖 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/migrations/0003_account_api_key.sql`:
- Around line 1-3: The ProviderAccount struct currently derives Debug and will
leak api_key; replace the derived Debug on ProviderAccount with a custom impl
that redacts the api_key field (e.g., print "<redacted>" for api_key) similar to
the ProviderAccountConfig example in clients/agent-runtime/src/config/schema.rs;
ensure you keep Clone/Serialize/Deserialize intact. Also, when creating the
rook.db file, ensure its permissions are set to 0o600 on creation and add a
comment on the ProviderAccount struct and the migration noting that api_key is
stored plaintext for M1 with a link to issue `#591`. Finally, audit logging sites
to never pass the full ProviderAccount to logging macros—log only safe fields
(e.g., account_id and vendor) or formatted summaries that omit api_key.
In `@clients/rook/src/db/mod.rs`:
- Around line 167-196: The migration logic for version_0003 (and analogous
0001/0002 blocks) performs DDL (MIGRATION_SQL_0003 via sqlx::raw_sql) and then
separately inserts into schema_migrations
(sqlx::query(...).bind(version_0003).bind(&now).execute(pool)), which can desync
if the process crashes between statements; wrap the DDL and the bookkeeping
INSERT in a single database transaction so they commit or roll back together:
begin a transaction from the same pool, execute MIGRATION_SQL_0003 and then the
INSERT into schema_migrations using that transaction handle instead of pool, and
finally commit the transaction (or rollback on error); consider extracting this
pattern into a helper function used by version_0001/version_0002/version_0003 to
avoid duplication.
In `@clients/rook/src/gateway/handlers.rs`:
- Around line 56-80: The health updates are being awaited before returning the
upstream response; instead, spawn them to run concurrently so the gateway
returns immediately. In both branches (success path using
state.registry.health().mark_success and error path using
state.registry.health().mark_failure), wrap the async health call in a
background task (e.g., tokio::spawn) and move/clone any needed handles
(state.registry.clone() or health handle, and decision.account.id plus
FAILURE_COOLDOWN_SECS) into that task, then return the Response or
map_upstream_error result without awaiting the spawned task.
- Around line 255-283: The test helper mock_upstream currently sets the
process-wide ROOK_TEST_UPSTREAM_URL which causes races; modify mock_upstream to
stop calling std::env::set_var and instead return the server URL (e.g., change
its return from tokio::task::JoinHandle<()> to something like (JoinHandle<()>,
String) or a struct containing the handle and url), and update callers such as
chat_completions_happy_path_returns_upstream_body to capture that returned URL
and pass it into test_app or whatever configuration expects the upstream instead
of reading ROOK_TEST_UPSTREAM_URL from the environment; ensure the
TcpListener::local_addr-derived url is the returned value and that no global env
var writes remain in mock_upstream.
In `@clients/rook/src/gateway/mod.rs`:
- Around line 12-37: The router lacks auth and a request-body size limit: update
build_router to include authentication middleware for /v1/* (or document that
binding to loopback in ServerConfig::default is required before exposing the
port) and add a request-size limiting layer for POST /chat/completions
(affecting handlers::handle_chat_completions) such as axum's DefaultBodyLimit or
a RequestBodyLimitLayer to guard against raw-body proxy memory exhaustion;
reference GatewayState when wiring middleware so the auth layer can access
registry/engine/client as needed.
In `@clients/rook/src/gateway/upstream.rs`:
- Around line 44-50: Add a structured warning when an account lacks API
credentials instead of silently sending unauthenticated requests: in
upstream.rs, inside the block handling account.api_key (the if let Some(api_key)
= account.api_key.as_deref() { ... } construct), add an else branch that emits a
warning (e.g., tracing::warn! or process_logger.warn) including only non-secret
metadata such as account identifier and vendor (reference account.vendor and
account.id or equivalent), ensuring you do NOT log api_key or any secret value;
keep the existing vendor::auth_header(...) and request.header(...) logic
unchanged.
In `@clients/rook/src/gateway/vendor.rs`:
- Around line 5-8: The current mapping in vendor.rs returns native base URLs
(ProviderVendor::OpenAi, ::Anthropic, ::Google, ::OpenRouter) but upstream.rs
always appends "/v1/chat/completions", so incompatible vendors will fail; update
the function that returns base URLs to either return None for vendors that are
not OpenAI-compatible by default (e.g., ProviderVendor::Anthropic,
ProviderVendor::Google) or implement vendor-specific path handling by returning
fully compatible base+path strings (or a tuple of base and path) for each
ProviderVendor, and ensure upstream.rs uses those vendor-specific values instead
of always appending "/v1/chat/completions".
In `@clients/rook/src/server/mod.rs`:
- Around line 59-82: The startup code in build_app currently treats the literal
db_path value ":memory:" as a sentinel to open an in-memory DB
(RookRegistry::open_in_memory), which leaks a test-only concern into production
and risks accidental volatile DBs; change the design so ServerConfig does not
use a string sentinel: either introduce an explicit enum (e.g.,
DatabaseSource::OnDisk(String) | DatabaseSource::InMemory) on ServerConfig and
switch on that to call RookRegistry::open or RookRegistry::open_in_memory, or
keep ServerConfig.db_path as Option<String> for CLI parsing but add a test-only
constructor like build_app_with_registry(registry, config) (or a dedicated
RookRegistry::from_test helper) and have build_app always call
RookRegistry::open(db_path) for production paths; update build_app to remove the
db_path == ":memory:" comparison and ensure tests use the new constructor to
avoid repeated migrations and accidental production usage.
In `@openspec/changes/archive/2026-04-21-rook-589-gateway-api/design.md`:
- Around line 539-553: Update the design doc to match the implemented behavior:
when decision.account.api_key (variable api_key in the flow) is missing or
empty, the gateway should proceed by omitting auth and proxying the request
rather than returning a 502 via error_response with "missing_api_key"; change
the described flow and the test matrix entries that assert 502/missing_api_key
to instead expect the no-auth/proxy behavior, and scan all markdown docs (glob
**/*.{md,mdx}) to ensure the wording and examples reference the updated behavior
and not the old 502 response.
- Around line 332-386: default_base_url currently returns an OpenAI URL for
ProviderVendor::Other but the shipped contract requires unknown vendors to yield
no default; change default_base_url to return Option<&'static str> and make
ProviderVendor::Other return None, then update effective_base_url(account:
&ProviderAccount) to first check account.api_base_override (as_deref), trim
trailing slashes and return that if present, otherwise call
default_base_url(&account.vendor).map(|s| s.trim_end_matches('/').to_string())
and handle the None case (decide and document whether to return an empty string
or propagate an error) so behavior matches the spec that Other has no implicit
OpenAI endpoint unless api_base_override is set.
In `@openspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.md`:
- Around line 24-26: The proposal incorrectly claims the new
ProviderAccount.api_key column is "encrypted at rest" while the risk table and
implementation indicate M1 stores keys in plaintext; update the text so the
"api_key" field explicitly states that M1 stores keys in plaintext (encryption
deferred to later migrations), and make the risk table entry for M1 and the
summary paragraph for "api_key on ProviderAccount" consistent; search for
"ProviderAccount.api_key", "M1", and the risk table section (also referenced
around the existing 169-176 notes) and revise all occurrences in the .md/.mdx
docs to state plaintext storage for M1 and clearly note that encryption-at-rest
will be introduced in a future migration.
In `@openspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.md`:
- Around line 579-624: The spec currently implies build_router(state) mounts
absolute /v1/... routes; change R12 to state that gateway::build_router(state)
MUST expose only relative routes (e.g., POST /chat/completions and GET /models)
and accept GatewayState (Clone) as before, and change R13 to require server::run
to own the /v1 prefix: create RookRegistry, RoutingEngine, reqwest::Client (30s
timeout), construct GatewayState, call gateway::build_router(state) and then
mount it under Router::nest("/v1", ...) or Router::route_layer to ensure the
server owns the /v1 path segment so there is no ambiguity between router and
server routing.
In `@openspec/changes/archive/2026-04-21-rook-589-gateway-api/verify-report.md`:
- Around line 122-133: The health feedback currently awaits
mark_success(account_id) / mark_failure(account_id, 60) inline which can add
latency; change the implementation in the gateway handler that calls
mark_success and mark_failure to offload the write into a background task (e.g.
use tokio::spawn) capturing account_id so the HTTP response is returned without
awaiting the health-write; ensure errors from the spawned task are logged but do
not affect the client response and keep the existing happy-path test semantics
by adjusting tests to allow eventual consistency if needed.
In `@openspec/specs/gateway/spec.md`:
- Around line 579-624: Update the spec to match the implemented router contract:
change R12 so gateway::build_router(state) returns a Router that serves POST
/chat/completions and GET /models (i.e., routes are relative, without the /v1
prefix) and keep GatewayState requirements (registry: RookRegistry, engine:
RoutingEngine, client: reqwest::Client, Clone); then amend R13 to state that
server::run MUST call gateway::build_router(state) and mount it via
Router::nest("/v1", gateway::build_router(state)) (creating RookRegistry,
RoutingEngine, and a reqwest::Client with 30s timeout beforehand) so the
top-level server retains /v1 as the namespace.
---
Outside diff comments:
In `@clients/rook/src/domain/mod.rs`:
- Around line 236-259: ProviderAccount currently derives Debug and contains
api_key, which causes secrets to appear in logs; replace the auto-derived Debug
for ProviderAccount with a manual impl that redacts api_key (e.g., print
"<redacted>" or omit the field) or wrap api_key in a secret type that implements
Debug safely, and likewise ensure Serialize does not expose the secret by either
removing api_key from the public DTO/impl or implementing custom
Serialize/Deserialize to prevent dumping the key; update references to
ProviderAccount (pools, routing, error contexts, tracing spans) to use the
redacted/debug-safe representation.
In `@clients/rook/src/main.rs`:
- Around line 61-67: ServerConfig.db_path is hard-coded to None in the
Commands::Serve arm so the CLI cannot choose an on-disk or :memory: DB; update
the CLI wiring to surface a --db-path option on the Serve command and thread
that value into ServerConfig (i.e., add a db_path field to the Serve variant and
populate ServerConfig.db_path from it in the Serve match arm), or if you prefer
to defer DB selection remove the ServerConfig.db_path field and its branches in
build_app to avoid dead config.
In `@clients/rook/src/server/mod.rs`:
- Around line 137-161: Add an assertion in the server_config_default_values test
to verify ServerConfig::default() sets db_path to None and add a new test (or
extend server_config_default_values) that simulates build_app's fallback
behavior by asserting that when db_path is None the effective DB path used by
build_app (or the unwrap_or("./rook.db") logic) resolves to "./rook.db";
reference the ServerConfig struct, the server_config_default_values test, the
db_path field, and the build_app unwrap_or("./rook.db") fallback so CI will
catch regressions flipping the default db path.
🪄 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: 35f0fd45-8204-4baf-8615-4ba6da817d01
⛔ Files ignored due to path filters (1)
clients/rook/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
clients/rook/Cargo.tomlclients/rook/migrations/0003_account_api_key.sqlclients/rook/src/db/account.rsclients/rook/src/db/mod.rsclients/rook/src/db/pool.rsclients/rook/src/db/route.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/main.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/services/account.rsopenspec/changes/archive/2026-04-21-rook-589-gateway-api/design.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/state.yamlopenspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/verify-report.mdopenspec/specs/gateway/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: report / Contributor Quality Report
- GitHub Check: pr-checks
- GitHub Check: sonar
- 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/routing/mod.rsclients/rook/src/db/route.rsclients/rook/src/services/account.rsclients/rook/src/db/pool.rsclients/rook/src/main.rsclients/rook/src/domain/mod.rsclients/rook/src/db/account.rsclients/rook/src/gateway/mod.rsclients/rook/src/server/mod.rsclients/rook/src/gateway/vendor.rsclients/rook/src/gateway/handlers.rsclients/rook/src/db/mod.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/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/routing/mod.rsclients/rook/src/db/route.rsclients/rook/src/services/account.rsclients/rook/src/db/pool.rsclients/rook/src/main.rsclients/rook/migrations/0003_account_api_key.sqlclients/rook/src/domain/mod.rsopenspec/changes/archive/2026-04-21-rook-589-gateway-api/state.yamlclients/rook/src/db/account.rsclients/rook/src/gateway/mod.rsopenspec/changes/archive/2026-04-21-rook-589-gateway-api/verify-report.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.mdclients/rook/src/server/mod.rsclients/rook/Cargo.tomlopenspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.mdclients/rook/src/gateway/vendor.rsopenspec/specs/gateway/spec.mdclients/rook/src/gateway/handlers.rsclients/rook/src/db/mod.rsclients/rook/src/gateway/upstream.rsopenspec/changes/archive/2026-04-21-rook-589-gateway-api/design.mdclients/rook/src/gateway/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/archive/2026-04-21-rook-589-gateway-api/verify-report.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.mdopenspec/specs/gateway/spec.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/design.md
🧠 Learnings (11)
📚 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/db/route.rsclients/rook/src/db/pool.rsclients/rook/src/domain/mod.rsclients/rook/src/db/account.rsclients/rook/src/gateway/vendor.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.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: 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:
openspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.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:
clients/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/**/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
📚 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/Cargo.tomlclients/rook/src/gateway/types.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/Cargo.tomlclients/rook/src/gateway/types.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/Cargo.tomlopenspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.mdopenspec/specs/gateway/spec.mdclients/rook/src/gateway/handlers.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/types.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/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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/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/gateway/types.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.md
[grammar] ~175-~175: Ensure spelling is correct
Context: ...reasonable default timeout (30s) on the reqwest client. Gateway returns 504 on timeout....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~214-~214: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...returns 503. - /v1/chat/completions handler (upstream error): mock returns 500, v...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.md
[style] ~514-~514: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._failure(account_id, cooldown_secs). - On upstream HTTP 5xx response: call healt...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~515-~515: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._failure(account_id, cooldown_secs). - On connection error: call health.mark_fai...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~622-~622: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...utingEnginebacked by the registry. 3. Create areqwest::Client` with a default time...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/specs/gateway/spec.md
[style] ~514-~514: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._failure(account_id, cooldown_secs). - On upstream HTTP 5xx response: call healt...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~515-~515: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._failure(account_id, cooldown_secs). - On connection error: call health.mark_fai...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~622-~622: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...utingEnginebacked by the registry. 3. Create areqwest::Client` with a default time...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-21-rook-589-gateway-api/design.md
[style] ~1077-~1077: To form a complete sentence, be sure to include a subject.
Context: ... for M1 — hardcoded 30s is reasonable. Can be made configurable as a fast follow-u...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-21-rook-589-gateway-api/verify-report.md
[warning] 113-113: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.md
[warning] 33-33: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 56-56: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 82-82: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 108-108: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 115-115: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 186-186: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 227-227: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 276-276: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 314-314: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 351-351: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 387-387: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 421-421: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 454-454: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 483-483: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 552-552: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 595-595: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 629-629: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 698-698: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 744-744: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 775-775: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 869-869: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 898-898: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 954-954: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 995-995: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-04-21-rook-589-gateway-api/design.md
[warning] 68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 143-143: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 689-689: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 930-930: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
clients/rook/src/gateway/types.rs (1)
12-28: LGTM — flatten + raw-body pass-through is consistent.Since
handle_chat_completionsforwardsbody: Bytesupstream verbatim, the#[serde(flatten)] extramap is effectively only used for deserialization tolerance and optional inspection. No data is lost on the wire. Tests cover the shape well.clients/rook/src/routing/mod.rs (1)
416-431: LGTM.Test fixtures correctly track the new
api_key: Option<String>field.clients/rook/src/services/account.rs (1)
163-163: LGTM.Fixture aligned with new domain shape.
clients/rook/src/db/route.rs (1)
223-223: LGTM.clients/rook/src/db/pool.rs (1)
312-324: LGTM.clients/rook/Cargo.toml (1)
27-28: Dependencies justified for the upstream proxy.
reqwestwithrustls-tls+jsonandbytesare consistent withgateway/upstream.rsusage.default-features = falsekeeps the surface lean.clients/rook/src/db/account.rs (1)
53-95: LGTM.api_keycolumn plumbed correctly throughrow_to_account,insert_account,get_account, andlist_accounts; round-trip tests cover bothSomeandNone. Just note theDebugleak raised ondomain/mod.rsaffects any{:?}formatting of rows fetched here.openspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.md (1)
1-1082: Docs LGTM. Task breakdown matches the implemented code; markdownlint warnings (MD031/MD029/MD040) are cosmetic in an archived artifact and can be ignored.
Redact ProviderAccount secrets, make migrations transactional, remove test-only DB sentinel behavior, limit gateway request bodies, and align tests/docs with the current gateway contract and review findings.
|
1 participant
Related Issues
closes #589
Summary
POST /v1/chat/completionsandGET /v1/modelsapi_keyvalues locally and archive the completed gateway OpenSpec artifactsTested Information
cargo test --manifest-path "clients/rook/Cargo.toml" gateway::cargo test --manifest-path "clients/rook/Cargo.toml" server::tests::cargo test --manifest-path "clients/rook/Cargo.toml" open_in_memory_applies_account_api_key_migrationcargo test --manifest-path "clients/rook/Cargo.toml" insert_and_get_account_round_trips_api_keycargo test --manifest-path "clients/rook/Cargo.toml" chat_completion_request_deserializes_typed_optional_fieldscargo test --manifest-path "clients/rook/Cargo.toml" proxy_chat_completion_without_api_key_still_forwards_requestReviewer focus:
/v1alongside/api/healthand dashboard assetsDocumentation Impact
openspec/specs/gateway/spec.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/proposal.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/spec.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/design.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-589-gateway-api/verify-report.mdBreaking Changes
None.
Checklist