From b6bae9ba4e1d8214b03dab51ef0951cca9737d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 08:18:15 +0200 Subject: [PATCH 1/6] feat: add model alias domain model and SQLite repository - Add ModelAlias struct and ModelAliasRepositoryPort trait - Create alias-sqlite crate with SqliteModelAliasRepository - Add V5 migration for model_aliases table - Include 26 built-in aliases (OpenAI, Anthropic, Google, Mistral, Groq) - Add 11 unit tests for repository operations - Implement cycle prevention and idempotent seeding Part of #47 --- Cargo.lock | 16 + Cargo.toml | 1 + crates/domain/rook-core/src/model.rs | 17 + crates/domain/rook-core/src/ports.rs | 40 ++ crates/infrastructure/alias-sqlite/Cargo.toml | 19 + .../alias-sqlite/src/builtin.rs | 65 +++ crates/infrastructure/alias-sqlite/src/lib.rs | 9 + .../alias-sqlite/src/repository.rs | 440 ++++++++++++++++++ .../src/migrations/V5__model_aliases.sql | 15 + 9 files changed, 622 insertions(+) create mode 100644 crates/infrastructure/alias-sqlite/Cargo.toml create mode 100644 crates/infrastructure/alias-sqlite/src/builtin.rs create mode 100644 crates/infrastructure/alias-sqlite/src/lib.rs create mode 100644 crates/infrastructure/alias-sqlite/src/repository.rs create mode 100644 crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql diff --git a/Cargo.lock b/Cargo.lock index 54cfd15e..e80d52ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,6 +58,22 @@ dependencies = [ "memchr", ] +[[package]] +name = "alias-sqlite" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "chrono", + "db-migration", + "rook-core", + "rusqlite", + "shared-kernel", + "tempfile", + "thiserror", + "tokio", +] + [[package]] name = "android_system_properties" version = "0.1.5" diff --git a/Cargo.toml b/Cargo.toml index 3e930182..70cb1734 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ members = [ "crates/infrastructure/db-migration", "crates/infrastructure/models-catalog", "crates/infrastructure/combo-sqlite", + "crates/infrastructure/alias-sqlite", "apps/rook", ] diff --git a/crates/domain/rook-core/src/model.rs b/crates/domain/rook-core/src/model.rs index 5e19e4ff..48821bf4 100644 --- a/crates/domain/rook-core/src/model.rs +++ b/crates/domain/rook-core/src/model.rs @@ -543,6 +543,23 @@ impl std::fmt::Display for ComboValidationError { impl std::error::Error for ComboValidationError {} +// ============================================================================ +// ModelAlias — model alias mapping for stable model names +// ============================================================================ + +/// Model alias mapping — resolves friendly alias names to canonical model IDs +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ModelAlias { + /// The alias name (e.g., "gpt-4o-latest") + pub alias: ModelId, + /// The canonical model ID (e.g., "gpt-4o-2024-05-13") + pub canonical: ModelId, + /// Optional provider scope (null = global) + pub provider_id: Option, + /// Creation timestamp (ISO 8601) + pub created_at: String, +} + /// A multi-step fallback chain aggregate #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Combo { diff --git a/crates/domain/rook-core/src/ports.rs b/crates/domain/rook-core/src/ports.rs index 0f97f555..e3c8d872 100644 --- a/crates/domain/rook-core/src/ports.rs +++ b/crates/domain/rook-core/src/ports.rs @@ -557,3 +557,43 @@ pub trait ComboRepositoryPort: Send + Sync { /// Delete a combo by its ID (cascades to steps) async fn delete(&self, id: &ComboId) -> Result<(), ComboRepositoryError>; } + +// --------------------------------------------------------------------------- +// ModelAliasRepositoryPort — persistence for model alias mappings +// --------------------------------------------------------------------------- + +use crate::ModelAlias; + +#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)] +pub enum ModelAliasRepositoryError { + #[error("alias not found: {0}")] + NotFound(ModelId), + #[error("alias already exists: {0}")] + AlreadyExists(ModelId), + #[error("invalid alias: {0}")] + InvalidAlias(String), + #[error("database error: {0}")] + Database(String), +} + +#[async_trait] +pub trait ModelAliasRepositoryPort: Send + Sync { + /// Resolve alias to canonical model. Returns None if not found. + async fn find_by_alias( + &self, + alias: &ModelId, + provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError>; + + /// List all aliases ordered by alias name + async fn list(&self) -> Result, ModelAliasRepositoryError>; + + /// Create new alias. Returns AlreadyExists if duplicate. + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError>; + + /// Delete alias by name. Returns true if deleted, false if not found. + async fn delete(&self, alias: &ModelId) -> Result; + + /// Seed aliases idempotently. Returns count of new aliases inserted. + async fn seed(&self, aliases: Vec) -> Result; +} diff --git a/crates/infrastructure/alias-sqlite/Cargo.toml b/crates/infrastructure/alias-sqlite/Cargo.toml new file mode 100644 index 00000000..4d754b9d --- /dev/null +++ b/crates/infrastructure/alias-sqlite/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "alias-sqlite" +version = "0.1.0" +edition = "2021" + +[dependencies] +db-migration = { path = "../../infrastructure/db-migration" } +shared-kernel = { path = "../../domain/shared-kernel" } +rook-core = { path = "../../domain/rook-core" } + +async-trait = { workspace = true } +chrono = { workspace = true } +rusqlite = { workspace = true } +thiserror = { workspace = true } +anyhow = { workspace = true } + +[dev-dependencies] +tokio = { workspace = true } +tempfile = "3" diff --git a/crates/infrastructure/alias-sqlite/src/builtin.rs b/crates/infrastructure/alias-sqlite/src/builtin.rs new file mode 100644 index 00000000..30881b84 --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/builtin.rs @@ -0,0 +1,65 @@ +//! Built-in model aliases — seeded at startup when table is empty + +/// Built-in aliases: (alias, canonical, provider_id) +/// Provider ID is None for global aliases +pub const DEFAULT_ALIASES: &[(&str, &str, Option<&str>)] = &[ + // OpenAI + ("gpt-4o-latest", "gpt-4o-2024-05-13", Some("openai")), + ("gpt-4o", "gpt-4o-2024-05-13", Some("openai")), + ("gpt-4-turbo", "gpt-4-turbo-2024-04-09", Some("openai")), + ("gpt-4", "gpt-4-0613", Some("openai")), + ("gpt-3.5-turbo", "gpt-3.5-turbo-0125", Some("openai")), + ("o1", "o1-2024-12-17", Some("openai")), + ("o1-mini", "o1-mini-2024-09-12", Some("openai")), + ("o3-mini", "o3-mini-2025-01-31", Some("openai")), + // Anthropic + ("claude-opus", "claude-3-opus-20240229", Some("anthropic")), + ( + "claude-sonnet", + "claude-3-5-sonnet-20241022", + Some("anthropic"), + ), + ( + "claude-haiku", + "claude-3-5-haiku-20241022", + Some("anthropic"), + ), + ("claude-3-opus", "claude-3-opus-20240229", Some("anthropic")), + ( + "claude-3-sonnet", + "claude-3-sonnet-20240229", + Some("anthropic"), + ), + ( + "claude-3-haiku", + "claude-3-haiku-20240307", + Some("anthropic"), + ), + // Google Gemini + ("gemini-pro", "gemini-1.5-pro-latest", Some("gemini")), + ("gemini-flash", "gemini-1.5-flash-latest", Some("gemini")), + ("gemini-2.0-flash", "gemini-2.0-flash-exp", Some("gemini")), + ("gemini-1.5-pro", "gemini-1.5-pro-latest", Some("gemini")), + ( + "gemini-1.5-flash", + "gemini-1.5-flash-latest", + Some("gemini"), + ), + ("gemini-exp", "gemini-exp-1206", Some("gemini")), + // Mistral + ("mistral-large", "mistral-large-2411", Some("mistral")), + ("mistral-small", "mistral-small-2501", Some("mistral")), + ("ministral-8b", "ministral-8b-2410", Some("mistral")), + // Groq + ( + "groq-llama-3.1-70b", + "llama-3.1-70b-versatile", + Some("groq"), + ), + ("groq-llama-3.1-8b", "llama-3.1-8b-instant", Some("groq")), + ( + "groq-llama-3.3-70b", + "llama-3.3-70b-versatile", + Some("groq"), + ), +]; diff --git a/crates/infrastructure/alias-sqlite/src/lib.rs b/crates/infrastructure/alias-sqlite/src/lib.rs new file mode 100644 index 00000000..8192d62e --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/lib.rs @@ -0,0 +1,9 @@ +//! alias-sqlite — SQLite-backed model alias repository implementation + +pub mod builtin; +pub mod repository; + +pub use repository::SqliteModelAliasRepository; + +// Re-export traits and types for convenience +pub use rook_core::ports::{ModelAliasRepositoryError, ModelAliasRepositoryPort}; diff --git a/crates/infrastructure/alias-sqlite/src/repository.rs b/crates/infrastructure/alias-sqlite/src/repository.rs new file mode 100644 index 00000000..eea7844f --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/repository.rs @@ -0,0 +1,440 @@ +use std::path::Path; +use std::sync::{Mutex, MutexGuard}; + +use async_trait::async_trait; +use chrono::Utc; +use rook_core::ports::{ModelAliasRepositoryError, ModelAliasRepositoryPort}; +use rook_core::ModelAlias; +use rusqlite::{params, Connection, OptionalExtension}; +use shared_kernel::{ModelId, ProviderId}; + +use crate::builtin::DEFAULT_ALIASES; + +pub struct SqliteModelAliasRepository { + conn: Mutex, +} + +impl SqliteModelAliasRepository { + pub fn new(db_path: impl AsRef) -> anyhow::Result { + let mut conn = Connection::open(&db_path)?; + conn.execute_batch( + "PRAGMA journal_mode = WAL; + PRAGMA busy_timeout = 5000; + PRAGMA synchronous = NORMAL; + PRAGMA foreign_keys = ON;", + )?; + + // Run migrations for in-memory databases + if db_path.as_ref().to_str() == Some(":memory:") { + db_migration::run_on_connection(&mut conn)?; + } + + Ok(Self { + conn: Mutex::new(conn), + }) + } + + fn lock(&self) -> Result, ModelAliasRepositoryError> { + self.conn + .lock() + .map_err(|_| ModelAliasRepositoryError::Database("sqlite mutex poisoned".to_string())) + } +} + +#[async_trait] +impl ModelAliasRepositoryPort for SqliteModelAliasRepository { + async fn find_by_alias( + &self, + alias: &ModelId, + provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + let conn = self.lock()?; + + let result = if let Some(pid) = provider_id { + // Provider-scoped query + conn.query_row( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + WHERE alias = ?1 AND (provider_id = ?2 OR provider_id IS NULL) + ORDER BY CASE WHEN provider_id IS NOT NULL THEN 0 ELSE 1 END + LIMIT 1", + params![alias.as_str(), pid.as_str()], + |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }, + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + } else { + // Global query + conn.query_row( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + WHERE alias = ?1 + LIMIT 1", + params![alias.as_str()], + |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }, + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + }; + + Ok(result) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + let conn = self.lock()?; + + let mut stmt = conn + .prepare( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + ORDER BY alias", + ) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + let aliases = stmt + .query_map([], |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + .collect::, _>>() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(aliases) + } + + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + let conn = self.lock()?; + + // Check if canonical is itself an alias (prevent cycles) + let canonical_is_alias = conn + .query_row( + "SELECT 1 FROM model_aliases WHERE alias = ?1 LIMIT 1", + params![alias.canonical.as_str()], + |_| Ok(()), + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + if canonical_is_alias.is_some() { + return Err(ModelAliasRepositoryError::InvalidAlias( + "Canonical model cannot be an alias".to_string(), + )); + } + + // Insert the alias + let result = conn.execute( + "INSERT INTO model_aliases (alias, canonical, provider_id, created_at) + VALUES (?1, ?2, ?3, ?4)", + params![ + alias.alias.as_str(), + alias.canonical.as_str(), + alias.provider_id.as_ref().map(|p| p.as_str()), + alias.created_at, + ], + ); + + match result { + Ok(_) => Ok(()), + Err(rusqlite::Error::SqliteFailure(err, _)) + if err.code == rusqlite::ErrorCode::ConstraintViolation => + { + Err(ModelAliasRepositoryError::AlreadyExists(alias.alias)) + } + Err(e) => Err(ModelAliasRepositoryError::Database(e.to_string())), + } + } + + async fn delete(&self, alias: &ModelId) -> Result { + let conn = self.lock()?; + + let rows_affected = conn + .execute( + "DELETE FROM model_aliases WHERE alias = ?1", + params![alias.as_str()], + ) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(rows_affected > 0) + } + + async fn seed(&self, aliases: Vec) -> Result { + let mut conn = self.lock()?; + + let tx = conn + .transaction() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + let mut inserted = 0; + for alias in aliases { + let result = tx.execute( + "INSERT OR IGNORE INTO model_aliases (alias, canonical, provider_id, created_at) + VALUES (?1, ?2, ?3, ?4)", + params![ + alias.alias.as_str(), + alias.canonical.as_str(), + alias.provider_id.as_ref().map(|p| p.as_str()), + alias.created_at, + ], + ); + + match result { + Ok(rows) => inserted += rows, + Err(e) => { + return Err(ModelAliasRepositoryError::Database(e.to_string())); + } + } + } + + tx.commit() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(inserted) + } +} + +/// Helper function to create built-in aliases from constants +pub fn builtin_aliases() -> Vec { + let now = Utc::now().to_rfc3339(); + + DEFAULT_ALIASES + .iter() + .map(|(alias_str, canonical_str, provider_id_str)| { + let provider_id = provider_id_str.map(ProviderId::new); + ModelAlias { + alias: ModelId::new(alias_str.to_string()), + canonical: ModelId::new(canonical_str.to_string()), + provider_id, + created_at: now.clone(), + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + async fn create_test_repo() -> SqliteModelAliasRepository { + SqliteModelAliasRepository::new(":memory:").expect("failed to create test repository") + } + + fn test_alias(alias: &str, canonical: &str) -> ModelAlias { + ModelAlias { + alias: ModelId::new(alias), + canonical: ModelId::new(canonical), + provider_id: None, + created_at: Utc::now().to_rfc3339(), + } + } + + #[tokio::test] + async fn test_find_by_alias_found() { + let repo = create_test_repo().await; + let alias = test_alias("gpt-4o-latest", "gpt-4o-2024-05-13"); + + repo.create(alias.clone()).await.expect("create failed"); + + let result = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("find failed"); + + assert!(result.is_some()); + let found = result.unwrap(); + assert_eq!(found.alias.as_str(), "gpt-4o-latest"); + assert_eq!(found.canonical.as_str(), "gpt-4o-2024-05-13"); + } + + #[tokio::test] + async fn test_find_by_alias_not_found() { + let repo = create_test_repo().await; + + let result = repo + .find_by_alias(&ModelId::new("non-existent"), None) + .await + .expect("find failed"); + + assert!(result.is_none()); + } + + #[tokio::test] + async fn test_create_success() { + let repo = create_test_repo().await; + let alias = test_alias("my-alias", "gpt-4o"); + + repo.create(alias).await.expect("create failed"); + + let found = repo + .find_by_alias(&ModelId::new("my-alias"), None) + .await + .expect("find failed") + .expect("alias not found"); + + assert_eq!(found.canonical.as_str(), "gpt-4o"); + } + + #[tokio::test] + async fn test_create_duplicate() { + let repo = create_test_repo().await; + let alias = test_alias("duplicate", "gpt-4o"); + + repo.create(alias.clone()) + .await + .expect("first create failed"); + + let result = repo.create(alias).await; + + assert!(matches!( + result, + Err(ModelAliasRepositoryError::AlreadyExists(_)) + )); + } + + #[tokio::test] + async fn test_create_alias_cycle() { + let repo = create_test_repo().await; + + // Create first alias + let alias1 = test_alias("alias-a", "canonical-model"); + repo.create(alias1).await.expect("create alias-a failed"); + + // Try to create alias pointing to another alias + let alias2 = test_alias("alias-b", "alias-a"); + let result = repo.create(alias2).await; + + assert!(matches!( + result, + Err(ModelAliasRepositoryError::InvalidAlias(_)) + )); + } + + #[tokio::test] + async fn test_delete_success() { + let repo = create_test_repo().await; + let alias = test_alias("to-delete", "gpt-4o"); + + repo.create(alias).await.expect("create failed"); + + let deleted = repo + .delete(&ModelId::new("to-delete")) + .await + .expect("delete failed"); + + assert!(deleted); + + let found = repo + .find_by_alias(&ModelId::new("to-delete"), None) + .await + .expect("find failed"); + + assert!(found.is_none()); + } + + #[tokio::test] + async fn test_delete_not_found() { + let repo = create_test_repo().await; + + let deleted = repo + .delete(&ModelId::new("non-existent")) + .await + .expect("delete failed"); + + assert!(!deleted); + } + + #[tokio::test] + async fn test_seed_empty_table() { + let repo = create_test_repo().await; + let aliases = builtin_aliases(); + let count = aliases.len(); + + let inserted = repo.seed(aliases).await.expect("seed failed"); + + assert_eq!(inserted, count); + + let all = repo.list().await.expect("list failed"); + assert_eq!(all.len(), count); + } + + #[tokio::test] + async fn test_seed_idempotent() { + let repo = create_test_repo().await; + let aliases = builtin_aliases(); + let count = aliases.len(); + + // First seed + let inserted1 = repo.seed(aliases.clone()).await.expect("first seed failed"); + assert_eq!(inserted1, count); + + // Second seed (should be idempotent) + let inserted2 = repo.seed(aliases).await.expect("second seed failed"); + assert_eq!(inserted2, 0); // No new inserts + + let all = repo.list().await.expect("list failed"); + assert_eq!(all.len(), count); // Still same count + } + + #[tokio::test] + async fn test_list_returns_all_aliases() { + let repo = create_test_repo().await; + + repo.create(test_alias("alias-1", "model-1")) + .await + .expect("create 1 failed"); + repo.create(test_alias("alias-2", "model-2")) + .await + .expect("create 2 failed"); + repo.create(test_alias("alias-3", "model-3")) + .await + .expect("create 3 failed"); + + let all = repo.list().await.expect("list failed"); + + assert_eq!(all.len(), 3); + assert_eq!(all[0].alias.as_str(), "alias-1"); // Sorted by alias + assert_eq!(all[1].alias.as_str(), "alias-2"); + assert_eq!(all[2].alias.as_str(), "alias-3"); + } + + #[tokio::test] + async fn test_builtin_aliases_count() { + let aliases = builtin_aliases(); + + // Verify we have at least 26 built-in aliases as per design + assert!( + aliases.len() >= 26, + "Expected at least 26 built-in aliases, got {}", + aliases.len() + ); + + // Verify structure + assert!(aliases.iter().any(|a| a.alias.as_str() == "gpt-4o-latest")); + assert!(aliases.iter().any(|a| a.alias.as_str() == "claude-opus")); + assert!(aliases.iter().any(|a| a.alias.as_str() == "gemini-pro")); + } +} diff --git a/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql b/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql new file mode 100644 index 00000000..e6a508c5 --- /dev/null +++ b/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql @@ -0,0 +1,15 @@ +-- V5: Model Aliases Table +-- Provides stable alias names that resolve to canonical model IDs + +CREATE TABLE IF NOT EXISTS model_aliases ( + alias TEXT PRIMARY KEY NOT NULL, + canonical TEXT NOT NULL, + provider_id TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +); + +-- Index for lookups by canonical model (useful for cycle detection) +CREATE INDEX IF NOT EXISTS idx_model_aliases_canonical ON model_aliases(canonical); + +-- Index for provider-scoped queries (future enhancement) +CREATE INDEX IF NOT EXISTS idx_model_aliases_provider ON model_aliases(provider_id); From 7a397b269b37d09a6dcb1c519566ecbbca27ea62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:39:29 +0200 Subject: [PATCH 2/6] feat: add model alias resolution and HTTP API (#111) - Add [model_aliases] config section with enabled and auto_seed flags - Wire SqliteModelAliasRepository into DI container with startup seeding - Implement alias resolution in RouteRequest before restrictions check - Add GET/POST/DELETE endpoints for alias management at /api/models/aliases - Add cycle prevention validation in create endpoint - Add 7 E2E tests for alias resolution and seeding - Add 10 HTTP API integration tests - Add 5 config tests for model aliases section Part of #47 --- Cargo.lock | 1 + apps/rook/Cargo.toml | 1 + apps/rook/src/config.rs | 33 ++ apps/rook/src/di.rs | 35 +- apps/rook/tests/alias_routing_e2e.rs | 203 +++++++++ apps/rook/tests/config_tests.rs | 72 ++++ .../rook-usecases/src/route_request.rs | 119 ++++- .../tests/route_request_restrictions.rs | 59 ++- .../transport-axum/src/alias_routes.rs | 21 + .../transport-axum/src/bootstrap_helpers.rs | 44 +- .../transport-axum/src/handlers/aliases.rs | 172 ++++++++ .../transport-axum/src/handlers/mod.rs | 1 + .../infrastructure/transport-axum/src/lib.rs | 1 + .../transport-axum/src/routes.rs | 10 +- .../transport-axum/tests/alias_api.rs | 407 ++++++++++++++++++ .../tests/format_translation_integration.rs | 55 ++- 16 files changed, 1210 insertions(+), 24 deletions(-) create mode 100644 apps/rook/tests/alias_routing_e2e.rs create mode 100644 crates/infrastructure/transport-axum/src/alias_routes.rs create mode 100644 crates/infrastructure/transport-axum/src/handlers/aliases.rs create mode 100644 crates/infrastructure/transport-axum/tests/alias_api.rs diff --git a/Cargo.lock b/Cargo.lock index e80d52ad..2760e8cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2647,6 +2647,7 @@ dependencies = [ name = "rook" version = "0.0.1" dependencies = [ + "alias-sqlite", "anyhow", "async-trait", "audit-sqlite", diff --git a/apps/rook/Cargo.toml b/apps/rook/Cargo.toml index f44ff1f7..9888ce1f 100644 --- a/apps/rook/Cargo.toml +++ b/apps/rook/Cargo.toml @@ -26,6 +26,7 @@ providers-groq = { path = "../../crates/infrastructure/providers-groq" } encryption-inmemory = { path = "../../crates/infrastructure/encryption-inmemory" } provider-sqlite = { path = "../../crates/infrastructure/provider-sqlite" } auth-sqlite = { path = "../../crates/infrastructure/auth-sqlite" } +alias-sqlite = { path = "../../crates/infrastructure/alias-sqlite" } db-migration = { path = "../../crates/infrastructure/db-migration" } models-catalog = { path = "../../crates/infrastructure/models-catalog" } tokio = { version = "1", features = ["full"] } diff --git a/apps/rook/src/config.rs b/apps/rook/src/config.rs index 728771bc..9d1ce6aa 100644 --- a/apps/rook/src/config.rs +++ b/apps/rook/src/config.rs @@ -30,6 +30,9 @@ pub struct RookConfig { /// Combo (multi-step fallback chain) definitions #[serde(default)] pub combos: Vec, + /// Model aliases configuration + #[serde(default)] + pub model_aliases: ModelAliasesConfig, } #[derive(Debug, Clone, Deserialize)] @@ -72,6 +75,36 @@ fn default_allow_env_fallback() -> bool { true } +#[derive(Debug, Clone, Deserialize)] +pub struct ModelAliasesConfig { + #[serde(default = "default_true")] + pub enabled: bool, + #[serde(default = "default_true")] + pub auto_seed: bool, +} + +impl Default for ModelAliasesConfig { + fn default() -> Self { + Self { + enabled: true, + auto_seed: true, + } + } +} + +impl From for rook_usecases::route_request::ModelAliasesConfig { + fn from(cfg: ModelAliasesConfig) -> Self { + Self { + enabled: cfg.enabled, + auto_seed: cfg.auto_seed, + } + } +} + +fn default_true() -> bool { + true +} + #[derive(Debug, Clone, Deserialize)] pub struct ProviderCrudConfig { #[serde(default = "default_provider_crud_enabled")] diff --git a/apps/rook/src/di.rs b/apps/rook/src/di.rs index a8a2ac19..78aa2113 100644 --- a/apps/rook/src/di.rs +++ b/apps/rook/src/di.rs @@ -6,6 +6,7 @@ use std::path::Path; use std::sync::Arc; use std::time::Duration; +use alias_sqlite::SqliteModelAliasRepository; use audit_sqlite::{SqliteAudit, SqliteUsageRepository}; use auth_sqlite::{SqliteApiKeyRepository, SqliteSessionRepository, SqliteUserRepository}; use cache_memory::InMemoryCache; @@ -17,9 +18,9 @@ use provider_sqlite::SqliteProviderRepository; use providers_ollama::OllamaProvider; use rook_core::{ ApiKeyRepositoryPort, AuditPort, CachePort, Combo, ComboRepositoryPort, ComboStep, - ComboStrategy, ConnectionId, DecryptedCredentials, PasswordHasher, ProviderId, ProviderKind, - ProviderPort, ProviderRegistryPort, ProviderRepositoryPort, RouterPort, SessionRepositoryPort, - UsageRecorderPort, + ComboStrategy, ConnectionId, DecryptedCredentials, ModelAliasRepositoryPort, PasswordHasher, + ProviderId, ProviderKind, ProviderPort, ProviderRegistryPort, ProviderRepositoryPort, + RouterPort, SessionRepositoryPort, UsageRecorderPort, }; use rook_usecases::{ AuthenticateClientApi, BootstrapStatus, EnsureAdminUser, FallbackRouter, HealthCheck, @@ -169,6 +170,32 @@ impl RookContainer { let combo_repo: Arc = Arc::new(ComboSqliteRepository::new(&config.database.db_path)?); + // 7d. Model alias repository — SQLite-backed alias storage + let alias_repo: Arc = + Arc::new(SqliteModelAliasRepository::new(&config.database.db_path)?); + + // 7e. Seed built-in aliases if enabled + if config.model_aliases.auto_seed { + let builtin_aliases = alias_sqlite::builtin::DEFAULT_ALIASES + .iter() + .map(|(alias, canonical, provider_id)| rook_core::ModelAlias { + alias: shared_kernel::ModelId::new(*alias), + canonical: shared_kernel::ModelId::new(*canonical), + provider_id: provider_id.map(shared_kernel::ProviderId::new), + created_at: shared_kernel::Utc::now().to_rfc3339(), + }) + .collect::>(); + + match alias_repo.seed(builtin_aliases).await { + Ok(count) => { + tracing::info!(count, "Seeded default model aliases"); + } + Err(e) => { + tracing::warn!(error = ?e, "Failed to seed model aliases"); + } + } + } + // 8. Run async initialization tasks concurrently: // - registry refresh (if provider_crud enabled) // - ensure admin user exists @@ -228,6 +255,8 @@ impl RookContainer { Some(combo_repo.clone()), // combo_repository - wired in Phase 4 Arc::new(config.pricing.clone()), format_registry.clone(), + alias_repo.clone(), // model_alias_repository - wired in Phase 3 + config.model_aliases.clone().into(), ), ManageProviders::new(router.clone()), health_check, diff --git a/apps/rook/tests/alias_routing_e2e.rs b/apps/rook/tests/alias_routing_e2e.rs new file mode 100644 index 00000000..5e548749 --- /dev/null +++ b/apps/rook/tests/alias_routing_e2e.rs @@ -0,0 +1,203 @@ +use alias_sqlite::{repository::builtin_aliases, SqliteModelAliasRepository}; +use rook_core::ports::ModelAliasRepositoryPort; +use rook_core::{ModelAlias, ModelId}; +use std::sync::Arc; + +/// Test that alias resolution works end-to-end in routing +/// +/// Scenario: User sends request with alias "gpt-4o-latest", which should resolve +/// to canonical model "gpt-4o-2024-05-13" before restrictions/routing logic. +#[tokio::test] +async fn test_alias_resolves_before_routing() { + // Setup: Create in-memory repository with test alias + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Create test alias + let alias = ModelAlias { + alias: ModelId::new("gpt-4o-latest"), + canonical: ModelId::new("gpt-4o-2024-05-13"), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo.create(alias.clone()).await.expect("create alias"); + + // Verify alias was created + let resolved = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("find alias") + .expect("alias exists"); + + assert_eq!(resolved.canonical, ModelId::new("gpt-4o-2024-05-13")); +} + +/// Test that unknown aliases pass through unchanged (fail-open behavior) +#[tokio::test] +async fn test_unknown_alias_passes_through() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Query non-existent alias + let result = repo + .find_by_alias(&ModelId::new("unknown-model"), None) + .await + .expect("query succeeds"); + + assert!(result.is_none(), "unknown alias should return None"); +} + +/// Test that canonical model IDs pass through unchanged +#[tokio::test] +async fn test_canonical_model_passes_through() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Create alias + let alias = ModelAlias { + alias: ModelId::new("gpt-4-turbo"), + canonical: ModelId::new("gpt-4-turbo-2024-04-09"), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo.create(alias).await.expect("create alias"); + + // Query with canonical model (not alias) + let result = repo + .find_by_alias(&ModelId::new("gpt-4-turbo-2024-04-09"), None) + .await + .expect("query succeeds"); + + // Canonical models should not resolve to anything (they are not aliases) + assert!( + result.is_none(), + "canonical model should not resolve as alias" + ); +} + +/// Test that built-in aliases are available after seeding +#[tokio::test] +async fn test_builtin_aliases_available_after_seed() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Seed built-in aliases + let count = repo.seed(builtin_aliases()).await.expect("seed succeeds"); + assert!(count > 20, "should seed at least 20 built-in aliases"); + + // Verify some known built-in aliases exist + let openai_alias = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("query succeeds") + .expect("gpt-4o-latest should exist"); + assert_eq!(openai_alias.canonical, ModelId::new("gpt-4o-2024-05-13")); + + let anthropic_alias = repo + .find_by_alias(&ModelId::new("claude-opus"), None) + .await + .expect("query succeeds") + .expect("claude-opus should exist"); + assert_eq!( + anthropic_alias.canonical, + ModelId::new("claude-3-opus-20240229") + ); + + let gemini_alias = repo + .find_by_alias(&ModelId::new("gemini-2.0-flash"), None) + .await + .expect("query succeeds") + .expect("gemini-2.0-flash should exist"); + assert_eq!(gemini_alias.canonical, ModelId::new("gemini-2.0-flash-exp")); +} + +/// Test alias resolution with provider-scoped lookup +#[tokio::test] +async fn test_provider_scoped_alias_lookup() { + use rook_core::ProviderId; + + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Seed builtin aliases (which have provider_id set) + let count = repo.seed(builtin_aliases()).await.expect("seed succeeds"); + assert!(count > 0); + + // Query with provider filter - should find provider-specific alias + let openai_alias = repo + .find_by_alias( + &ModelId::new("gpt-4o-latest"), + Some(&ProviderId::new("openai")), + ) + .await + .expect("query succeeds") + .expect("openai alias exists"); + assert_eq!(openai_alias.canonical, ModelId::new("gpt-4o-2024-05-13")); + assert_eq!(openai_alias.provider_id, Some(ProviderId::new("openai"))); + + // Query without provider filter - should also find it + let global_lookup = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("query succeeds") + .expect("alias exists"); + + assert_eq!(global_lookup.canonical, ModelId::new("gpt-4o-2024-05-13")); +} + +/// Test that seeding is idempotent (can be run multiple times safely) +#[tokio::test] +async fn test_seed_is_idempotent() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // First seed + let count1 = repo + .seed(builtin_aliases()) + .await + .expect("first seed succeeds"); + assert!(count1 > 0, "first seed should insert aliases"); + + // Second seed + let count2 = repo + .seed(builtin_aliases()) + .await + .expect("second seed succeeds"); + assert_eq!(count2, 0, "second seed should insert nothing (idempotent)"); + + // Verify aliases still exist and weren't duplicated + let all_aliases = repo.list().await.expect("list succeeds"); + assert_eq!( + all_aliases.len(), + count1 as usize, + "should have same count as first seed" + ); +} + +/// Test repository error handling (fail-open behavior) +#[tokio::test] +async fn test_repository_handles_concurrent_access() { + let repo = Arc::new(SqliteModelAliasRepository::new(":memory:").expect("create repo")); + + // Create multiple concurrent queries + let mut handles = vec![]; + for i in 0..10 { + let repo_clone = Arc::clone(&repo); + let handle = tokio::spawn(async move { + let alias = ModelAlias { + alias: ModelId::new(format!("test-alias-{}", i)), + canonical: ModelId::new(format!("test-canonical-{}", i)), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo_clone.create(alias).await + }); + handles.push(handle); + } + + // Wait for all to complete + for handle in handles { + handle + .await + .expect("task completes") + .expect("create succeeds"); + } + + // Verify all aliases were created + let all_aliases = repo.list().await.expect("list succeeds"); + assert_eq!(all_aliases.len(), 10, "all aliases should be created"); +} diff --git a/apps/rook/tests/config_tests.rs b/apps/rook/tests/config_tests.rs index 3eebeeb8..0580dd51 100644 --- a/apps/rook/tests/config_tests.rs +++ b/apps/rook/tests/config_tests.rs @@ -88,3 +88,75 @@ completion_per_million = 2.40 assert_eq!(price.cache_read_per_million, None); assert_eq!(price.cache_creation_per_million, None); } + +#[test] +fn config_model_aliases_defaults_to_enabled_and_auto_seed() { + let config: RookConfig = toml::from_str(&minimal_config_toml("")).expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!(config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_can_be_disabled() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = false +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!(!config.model_aliases.enabled); + assert!(!config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_deserializes_from_toml() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = true +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!(!config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_enabled_only() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = true +"#, + )) + .expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!( + config.model_aliases.auto_seed, + "auto_seed should default to true" + ); +} + +#[test] +fn config_model_aliases_auto_seed_only() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!( + config.model_aliases.enabled, + "enabled should default to true" + ); + assert!(!config.model_aliases.auto_seed); +} diff --git a/crates/application/rook-usecases/src/route_request.rs b/crates/application/rook-usecases/src/route_request.rs index f4133a97..07d0dd1c 100644 --- a/crates/application/rook-usecases/src/route_request.rs +++ b/crates/application/rook-usecases/src/route_request.rs @@ -15,8 +15,9 @@ use chrono::Utc; use futures::StreamExt; use rook_core::{ ApiFormat, AuditEntry, AuditPort, CachePort, ComboRepositoryPort, CompletionRequest, - CompletionResponse, CortexError, FormatTranslatorPort, ProviderRepositoryPort, RequestStatus, - RouterPort, StreamChunk, TokenUsage, UsageEntry, UsageRecorderPort, + CompletionResponse, CortexError, FormatTranslatorPort, ModelAliasRepositoryPort, + ProviderRepositoryPort, RequestStatus, RouterPort, StreamChunk, TokenUsage, UsageEntry, + UsageRecorderPort, }; use shared_kernel::{ComboId, ConnectionId, ProviderId, RestrictionViolation}; @@ -42,6 +43,15 @@ pub struct RouteRequest { combo_repository: Option>, pricing: Arc, format_translator: Arc, + alias_repository: Arc, + alias_config: ModelAliasesConfig, +} + +/// Configuration for model alias resolution +#[derive(Debug, Clone)] +pub struct ModelAliasesConfig { + pub enabled: bool, + pub auto_seed: bool, } impl RouteRequest { @@ -55,6 +65,8 @@ impl RouteRequest { combo_repository: Option>, pricing: Arc, format_translator: Arc, + alias_repository: Arc, + alias_config: ModelAliasesConfig, ) -> Self { Self { router, @@ -65,6 +77,8 @@ impl RouteRequest { combo_repository, pricing, format_translator, + alias_repository, + alias_config, } } @@ -73,13 +87,18 @@ impl RouteRequest { self.combo_repository.clone() } + /// Get alias repository reference (for HTTP layer wiring) + pub fn alias_repository(&self) -> Arc { + self.alias_repository.clone() + } + pub async fn execute(&self, req: CompletionRequest) -> Result { self.execute_with_format(req, ApiFormat::OpenAI).await } pub async fn execute_with_format( &self, - req: CompletionRequest, + mut req: CompletionRequest, client_format: ApiFormat, ) -> Result { // 0. Check if combo execution is requested @@ -87,10 +106,34 @@ impl RouteRequest { return self.execute_combo(&combo_id, req, client_format).await; } + // 0a. Resolve model alias if enabled (BEFORE restrictions check) + if self.alias_config.enabled { + match self.alias_repository.find_by_alias(&req.model, None).await { + Ok(Some(alias_entry)) => { + tracing::debug!( + alias = %req.model, + canonical = %alias_entry.canonical, + "Resolved model alias" + ); + req.model = alias_entry.canonical; + } + Ok(None) => { + // No alias found, proceed with original model + } + Err(e) => { + tracing::warn!( + error = ?e, + model = %req.model, + "Alias resolution failed, using original model" + ); + } + } + } + let cache_key = req.cache_key(); let start = Instant::now(); - // 0. Model restriction check (before any provider interaction) + // 0b. Model restriction check (AFTER alias resolution) if !req.restrictions.allowed_models.is_empty() && !req.restrictions.allowed_models.contains(&req.model) { @@ -1075,6 +1118,54 @@ mod tests { } } + /// Test stub for ModelAliasRepositoryPort — returns no aliases + struct TestAliasRepository; + + #[async_trait] + impl ModelAliasRepositoryPort for TestAliasRepository { + async fn find_by_alias( + &self, + _alias: &shared_kernel::ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, rook_core::ModelAliasRepositoryError> { + Ok(None) // No aliases in tests by default + } + + async fn list( + &self, + ) -> Result, rook_core::ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create( + &self, + _alias: rook_core::ModelAlias, + ) -> Result<(), rook_core::ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete( + &self, + _alias: &shared_kernel::ModelId, + ) -> Result { + Ok(false) + } + + async fn seed( + &self, + _aliases: Vec, + ) -> Result { + Ok(0) + } + } + + fn test_alias_config() -> ModelAliasesConfig { + ModelAliasesConfig { + enabled: false, // Disabled by default in tests + auto_seed: false, + } + } + struct FailingProviderRepository; #[async_trait] @@ -1193,6 +1284,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut stream = usecase @@ -1242,6 +1335,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ) } @@ -1266,6 +1361,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1318,6 +1415,8 @@ mod tests { None, Arc::new(pricing), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut req = request(); req.metadata.api_key_id = Some(ApiKeyId::new("key_123")); @@ -1369,6 +1468,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let req = request(); @@ -1413,6 +1514,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1448,6 +1551,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1577,6 +1682,8 @@ mod tests { None, Arc::new(pricing), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut req = request(); req.metadata.api_key_id = Some(ApiKeyId::new("key_streaming")); @@ -1681,6 +1788,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut stream = usecase @@ -1729,6 +1838,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let stream = usecase diff --git a/crates/application/rook-usecases/tests/route_request_restrictions.rs b/crates/application/rook-usecases/tests/route_request_restrictions.rs index 4fdb9267..f0b0a599 100644 --- a/crates/application/rook-usecases/tests/route_request_restrictions.rs +++ b/crates/application/rook-usecases/tests/route_request_restrictions.rs @@ -8,10 +8,10 @@ use async_trait::async_trait; use rook_core::{ ApiFormat, ApiKeyRestrictions, AuditEntry, AuditPort, CachePort, CompletionRequest, CompletionResponse, CortexError, CortexResult, FormatTranslatorPort, HealthStatus, Message, - MessageContent, ModelId, ProviderId, ProviderPort, RequestMetadata, Role, RouterPort, - StreamChunk, TokenUsage, + MessageContent, ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort, ModelId, + ProviderId, ProviderPort, RequestMetadata, Role, RouterPort, StreamChunk, TokenUsage, }; -use rook_usecases::{PricingConfig, RouteRequest}; +use rook_usecases::{route_request::ModelAliasesConfig, PricingConfig, RouteRequest}; use shared_kernel::{CacheKey, RequestId}; // --- Fake Implementations --- @@ -171,6 +171,43 @@ impl FormatTranslatorPort for NoOpTranslator { } } +/// Test stub for ModelAliasRepositoryPort +struct NoOpAliasRepository; + +#[async_trait] +impl ModelAliasRepositoryPort for NoOpAliasRepository { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + Ok(None) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete(&self, _alias: &ModelId) -> Result { + Ok(false) + } + + async fn seed(&self, _aliases: Vec) -> Result { + Ok(0) + } +} + +fn test_alias_config() -> ModelAliasesConfig { + ModelAliasesConfig { + enabled: false, + auto_seed: false, + } +} + // --- Test Cases --- #[tokio::test] @@ -180,6 +217,8 @@ async fn allowed_models_contains_requested_model_passes() { let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -190,6 +229,8 @@ async fn allowed_models_contains_requested_model_passes() { None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -231,6 +272,8 @@ async fn allowed_models_missing_requested_model_returns_403_with_structured_code let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -241,6 +284,8 @@ async fn allowed_models_missing_requested_model_returns_403_with_structured_code None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -286,6 +331,8 @@ async fn allowed_providers_contains_selected_provider_passes() { let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -296,6 +343,8 @@ async fn allowed_providers_contains_selected_provider_passes() { None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -337,6 +386,8 @@ async fn allowed_providers_missing_selected_provider_returns_403_with_structured let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -347,6 +398,8 @@ async fn allowed_providers_missing_selected_provider_returns_403_with_structured None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { diff --git a/crates/infrastructure/transport-axum/src/alias_routes.rs b/crates/infrastructure/transport-axum/src/alias_routes.rs new file mode 100644 index 00000000..0961e1a6 --- /dev/null +++ b/crates/infrastructure/transport-axum/src/alias_routes.rs @@ -0,0 +1,21 @@ +// Alias routes — HTTP endpoints for model alias management + +use std::sync::Arc; + +use axum::{ + routing::{delete, get}, + Router, +}; +use rook_core::ModelAliasRepositoryPort; + +use super::handlers::aliases::{create_alias, delete_alias, list_aliases}; + +type AliasRepository = Arc; + +/// Build the alias CRUD router +pub fn router(alias_repo: AliasRepository) -> Router { + Router::new() + .route("/", get(list_aliases).post(create_alias)) + .route("/{alias}", delete(delete_alias)) + .with_state(alias_repo) +} diff --git a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs index 8b757e2f..9d5613c3 100644 --- a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs +++ b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs @@ -10,14 +10,15 @@ use std::sync::Arc; use models_catalog::StaticModelCatalog; use rook_core::{ ApiFormat, ApiKeyRepositoryPort, AuditEntry, AuditPort, CachePort, CompletionRequest, - CompletionResponse, CortexResult, FormatTranslatorPort, NewSession, PasswordHasher, RouterPort, - Session, SessionId, SessionRepositoryError, SessionRepositoryPort, UserRepositoryPort, + CompletionResponse, CortexResult, FormatTranslatorPort, ModelAlias, ModelAliasRepositoryError, + ModelAliasRepositoryPort, NewSession, PasswordHasher, RouterPort, Session, SessionId, + SessionRepositoryError, SessionRepositoryPort, UserRepositoryPort, }; use rook_usecases::{ BootstrapStatus, FallbackRouter, HealthCheck, ManageApiKeys, ManageProviders, RouteRequest, RoutingStrategy, SetAdminPassword, }; -use shared_kernel::CacheKey; +use shared_kernel::{CacheKey, ModelId, ProviderId}; use std::time::Duration; use tokio::sync::RwLock; @@ -43,6 +44,11 @@ pub fn make_test_bootstrap_usecases( let format_translator: Arc = Arc::new(StubFormatTranslator); let cache: Arc = Arc::new(StubCache); let audit: Arc = Arc::new(StubAudit); + let alias_repo: Arc = Arc::new(StubAliasRepo); + let alias_config = rook_usecases::route_request::ModelAliasesConfig { + enabled: false, + auto_seed: false, + }; let route_request = RouteRequest::new( fallback_router.clone() as Arc, @@ -53,6 +59,8 @@ pub fn make_test_bootstrap_usecases( None, Arc::new(rook_usecases::PricingConfig::default()), format_translator, + alias_repo, + alias_config, ); let manage_providers = ManageProviders::new(fallback_router.clone()); let health_check = Arc::new(HealthCheck::new(fallback_router.clone())); @@ -171,3 +179,33 @@ impl SessionRepositoryPort for StubSessionRepo { Ok(0) } } + +/// Stub alias repository — never called by bootstrap tests +struct StubAliasRepo; + +#[async_trait] +impl ModelAliasRepositoryPort for StubAliasRepo { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn delete(&self, _alias: &ModelId) -> Result { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn seed(&self, _aliases: Vec) -> Result { + unreachable!("alias_repo not called by bootstrap tests") + } +} diff --git a/crates/infrastructure/transport-axum/src/handlers/aliases.rs b/crates/infrastructure/transport-axum/src/handlers/aliases.rs new file mode 100644 index 00000000..b121e1a4 --- /dev/null +++ b/crates/infrastructure/transport-axum/src/handlers/aliases.rs @@ -0,0 +1,172 @@ +// Alias management HTTP handlers — CRUD operations for model aliases + +use std::sync::Arc; + +use axum::{ + extract::{Path, State}, + http::StatusCode, + Json, +}; +use rook_core::{ModelAlias, ModelAliasRepositoryPort}; +use serde::{Deserialize, Serialize}; +use shared_kernel::{ModelId, ProviderId}; + +use crate::HttpError; + +type AliasRepository = Arc; + +// ------------------------------------------------------------------------- +// DTOs +// ------------------------------------------------------------------------- + +/// Request body for POST /api/models/aliases +#[derive(Debug, Deserialize)] +pub struct CreateAliasRequest { + pub alias: String, + pub canonical: String, + #[serde(rename = "providerId")] + pub provider_id: Option, +} + +/// Response body for GET /api/models/aliases and single alias operations +#[derive(Debug, Serialize)] +pub struct AliasResponse { + pub alias: String, + pub canonical: String, + #[serde(rename = "providerId")] + pub provider_id: Option, + #[serde(rename = "createdAt")] + pub created_at: String, +} + +impl From<&ModelAlias> for AliasResponse { + fn from(alias: &ModelAlias) -> Self { + Self { + alias: alias.alias.to_string(), + canonical: alias.canonical.to_string(), + provider_id: alias.provider_id.as_ref().map(|p| p.to_string()), + created_at: alias.created_at.clone(), + } + } +} + +// ------------------------------------------------------------------------- +// Handlers +// ------------------------------------------------------------------------- + +/// GET /api/models/aliases — List all aliases +pub async fn list_aliases( + State(repo): State, +) -> Result>, HttpError> { + let aliases = repo.list().await.map_err(|e| HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to list aliases: {}", e), + })?; + + let response: Vec = aliases.iter().map(AliasResponse::from).collect(); + Ok(Json(response)) +} + +/// POST /api/models/aliases — Create a new alias +pub async fn create_alias( + State(repo): State, + Json(req): Json, +) -> Result { + // Validate input + if req.alias.trim().is_empty() { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "INVALID_ALIAS", + message: "alias must not be empty".to_string(), + }); + } + + if req.canonical.trim().is_empty() { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "INVALID_CANONICAL", + message: "canonical must not be empty".to_string(), + }); + } + + // Check if canonical is itself an alias (cycle prevention) + let canonical_model_id = ModelId::new(req.canonical.clone()); + match repo.find_by_alias(&canonical_model_id, None).await { + Ok(Some(_)) => { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "ALIAS_CYCLE", + message: "Aliases cannot point to other aliases".to_string(), + }); + } + Ok(None) => { + // Good — canonical is not an alias + } + Err(e) => { + return Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to check alias cycle: {}", e), + }); + } + } + + // Build domain model + let alias = ModelAlias { + alias: ModelId::new(req.alias.clone()), + canonical: canonical_model_id, + provider_id: req.provider_id.map(ProviderId::new), + created_at: chrono::Utc::now().to_rfc3339(), + }; + + let alias_str = alias.alias.to_string(); + let canonical_str = alias.canonical.to_string(); + + // Create alias + match repo.create(alias).await { + Ok(()) => { + tracing::info!( + alias = %alias_str, + canonical = %canonical_str, + "alias created" + ); + Ok(StatusCode::CREATED) + } + Err(e) if e.to_string().contains("already exists") => Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "ALIAS_ALREADY_EXISTS", + message: format!("Alias '{}' already exists", alias_str), + }), + Err(e) => Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to create alias: {}", e), + }), + } +} + +/// DELETE /api/models/aliases/:alias — Delete an alias +pub async fn delete_alias( + State(repo): State, + Path(alias): Path, +) -> Result { + let alias_id = ModelId::new(alias); + + match repo.delete(&alias_id).await { + Ok(true) => { + tracing::info!(alias = %alias_id, "alias deleted"); + Ok(StatusCode::NO_CONTENT) + } + Ok(false) => Err(HttpError { + status: StatusCode::NOT_FOUND, + code: "ALIAS_NOT_FOUND", + message: format!("Alias '{}' not found", alias_id), + }), + Err(e) => Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to delete alias: {}", e), + }), + } +} diff --git a/crates/infrastructure/transport-axum/src/handlers/mod.rs b/crates/infrastructure/transport-axum/src/handlers/mod.rs index e89fbade..045a5c3a 100644 --- a/crates/infrastructure/transport-axum/src/handlers/mod.rs +++ b/crates/infrastructure/transport-axum/src/handlers/mod.rs @@ -1,5 +1,6 @@ // handlers — HTTP endpoint handlers +pub mod aliases; pub mod api_key; pub mod auth; pub mod bootstrap; diff --git a/crates/infrastructure/transport-axum/src/lib.rs b/crates/infrastructure/transport-axum/src/lib.rs index 6f605cd3..6652f2bb 100644 --- a/crates/infrastructure/transport-axum/src/lib.rs +++ b/crates/infrastructure/transport-axum/src/lib.rs @@ -3,6 +3,7 @@ // Translates between provider wire formats (OpenAI, Anthropic) and the // internal domain model. All format-specific logic lives here. +pub mod alias_routes; pub mod anthropic_adapter; pub mod api_key_dto; pub mod authz; diff --git a/crates/infrastructure/transport-axum/src/routes.rs b/crates/infrastructure/transport-axum/src/routes.rs index 66078d3b..32c9a1ce 100644 --- a/crates/infrastructure/transport-axum/src/routes.rs +++ b/crates/infrastructure/transport-axum/src/routes.rs @@ -18,8 +18,8 @@ use tower_http::limit::RequestBodyLimitLayer; use tracing::error; use super::{ - anthropic_adapter::*, authz, combo_routes, handlers, middleware::csrf_guard, openai_adapter::*, - provider_routes, HttpError, + alias_routes, anthropic_adapter::*, authz, combo_routes, handlers, middleware::csrf_guard, + openai_adapter::*, provider_routes, HttpError, }; use crate::middleware::{ApiKeyRateLimiter, CsrfGuard, IpRateLimiter, LoginRateLimiter}; @@ -83,6 +83,12 @@ pub fn router( )); } + // Alias routes (model alias repository is always available) + router = router.nest( + "/api/models/aliases", + alias_routes::router(usecases.route_request.alias_repository()), + ); + // Model catalog is always available (the catalog port is mandatory on // RookUsecases), so the route is always mounted. router = router.merge(crate::models_routes::router(usecases.clone())); diff --git a/crates/infrastructure/transport-axum/tests/alias_api.rs b/crates/infrastructure/transport-axum/tests/alias_api.rs new file mode 100644 index 00000000..8e1c777d --- /dev/null +++ b/crates/infrastructure/transport-axum/tests/alias_api.rs @@ -0,0 +1,407 @@ +// Integration tests for model alias HTTP API + +use axum::{ + body::Body, + http::{Request, StatusCode}, + Router, +}; +use rook_core::{ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort}; +use serde_json::json; +use shared_kernel::{ModelId, ProviderId}; +use std::sync::Arc; +use tower::ServiceExt; +use transport_axum::alias_routes; + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +/// In-memory alias repository for testing +#[derive(Clone)] +struct InMemoryAliasRepo { + aliases: Arc>>, +} + +impl InMemoryAliasRepo { + fn new() -> Self { + Self { + aliases: Arc::new(tokio::sync::RwLock::new(Vec::new())), + } + } + + async fn seed_builtin(&self) { + let builtins = vec![ + ModelAlias { + alias: ModelId::new("gpt-4o-latest"), + canonical: ModelId::new("gpt-4o-2024-05-13"), + provider_id: Some(ProviderId::new("openai")), + created_at: "2024-01-15T10:30:00Z".to_string(), + }, + ModelAlias { + alias: ModelId::new("claude-opus"), + canonical: ModelId::new("claude-opus-4-5"), + provider_id: Some(ProviderId::new("anthropic")), + created_at: "2024-01-15T10:30:00Z".to_string(), + }, + ]; + + let mut aliases = self.aliases.write().await; + aliases.extend(builtins); + } +} + +#[async_trait::async_trait] +impl ModelAliasRepositoryPort for InMemoryAliasRepo { + async fn find_by_alias( + &self, + alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + let aliases = self.aliases.read().await; + Ok(aliases.iter().find(|a| a.alias == *alias).cloned()) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + let aliases = self.aliases.read().await; + Ok(aliases.clone()) + } + + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + let mut aliases = self.aliases.write().await; + if aliases.iter().any(|a| a.alias == alias.alias) { + return Err(ModelAliasRepositoryError::AlreadyExists(alias.alias)); + } + aliases.push(alias); + Ok(()) + } + + async fn delete(&self, alias: &ModelId) -> Result { + let mut aliases = self.aliases.write().await; + let before_len = aliases.len(); + aliases.retain(|a| a.alias != *alias); + Ok(aliases.len() < before_len) + } + + async fn seed(&self, builtins: Vec) -> Result { + let mut aliases = self.aliases.write().await; + let mut count = 0; + for builtin in builtins { + if !aliases.iter().any(|a| a.alias == builtin.alias) { + aliases.push(builtin); + count += 1; + } + } + Ok(count) + } +} + +fn test_app() -> Router { + let repo = Arc::new(InMemoryAliasRepo::new()) as Arc; + alias_routes::router(repo) +} + +async fn test_app_with_seeded() -> Router { + let repo = Arc::new(InMemoryAliasRepo::new()); + repo.seed_builtin().await; + alias_routes::router(repo) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn test_get_aliases_empty() { + let app = test_app(); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 0); +} + +#[tokio::test] +async fn test_get_aliases_with_builtin() { + let app = test_app_with_seeded().await; + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 2); + + // Verify structure + assert_eq!(aliases[0]["alias"], "gpt-4o-latest"); + assert_eq!(aliases[0]["canonical"], "gpt-4o-2024-05-13"); + assert_eq!(aliases[0]["providerId"], "openai"); + assert!(aliases[0]["createdAt"].is_string()); +} + +#[tokio::test] +async fn test_create_alias_success() { + let app = test_app(); + + let payload = json!({ + "alias": "my-gpt4", + "canonical": "gpt-4-0613", + "providerId": "openai" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); +} + +#[tokio::test] +async fn test_create_alias_duplicate() { + let app = test_app_with_seeded().await; + + let payload = json!({ + "alias": "gpt-4o-latest", + "canonical": "gpt-4o-2024-08-06" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_ALREADY_EXISTS"); +} + +#[tokio::test] +async fn test_create_alias_empty_alias() { + let app = test_app(); + + let payload = json!({ + "alias": "", + "canonical": "gpt-4-0613" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "INVALID_ALIAS"); +} + +#[tokio::test] +async fn test_create_alias_empty_canonical() { + let app = test_app(); + + let payload = json!({ + "alias": "my-model", + "canonical": "" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "INVALID_CANONICAL"); +} + +#[tokio::test] +async fn test_create_alias_cycle_detection() { + let app = test_app_with_seeded().await; + + // Try to create alias pointing to another alias + let payload = json!({ + "alias": "my-alias", + "canonical": "gpt-4o-latest" // This is itself an alias + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_CYCLE"); + assert!(error["error"] + .as_str() + .unwrap() + .contains("cannot point to other aliases")); +} + +#[tokio::test] +async fn test_delete_alias_success() { + let app = test_app_with_seeded().await; + + let response = app + .oneshot( + Request::builder() + .uri("/gpt-4o-latest") + .method("DELETE") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); +} + +#[tokio::test] +async fn test_delete_alias_not_found() { + let app = test_app(); + + let response = app + .oneshot( + Request::builder() + .uri("/nonexistent-alias") + .method("DELETE") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_NOT_FOUND"); +} + +#[tokio::test] +async fn test_create_and_list() { + let app = test_app(); + + // Create alias + let payload = json!({ + "alias": "test-alias", + "canonical": "test-model-v1", + "providerId": "test-provider" + }); + + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); + + // List aliases + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 1); + assert_eq!(aliases[0]["alias"], "test-alias"); + assert_eq!(aliases[0]["canonical"], "test-model-v1"); + assert_eq!(aliases[0]["providerId"], "test-provider"); +} diff --git a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs index d602899a..3a0fe837 100644 --- a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs +++ b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs @@ -11,8 +11,16 @@ // SC-05 + SC-10: Anthropic round-trip (content[0].type == "text", stop_reason == "end_turn") // SC-01 + SC-02: No parse error on requests that include `tools` or `stream_options` fields -use rook_core::{CompletionResponse, MessageContent, ModelId, Role, TokenUsage}; -use shared_kernel::{ProviderId, RequestId}; +use async_trait::async_trait; +use rook_core::{ + ApiFormat, AuditEntry, AuditPort, CachePort, CompletionRequest, CompletionResponse, + HealthStatus, MessageContent, ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort, + ModelId, ProviderPort, RequestMetadata, Role, RouterPort, StreamChunk, TokenUsage, +}; +use rook_usecases::{route_request::ModelAliasesConfig, RouteRequest}; +use shared_kernel::{CacheKey, ProviderId, RequestId}; +use std::sync::Arc; +use std::time::Duration; use transport_axum::{ anthropic_adapter::{AnthropicMessagesRequest, AnthropicMessagesResponse}, openai_adapter::{OpenAIChatRequest, OpenAIChatResponse}, @@ -210,14 +218,8 @@ fn anthropic_response_has_correct_structure() { // Registry-routed multi-format use case integration // --------------------------------------------------------------------------- -use async_trait::async_trait; use futures::stream; -use rook_core::{ - ApiFormat, AuditEntry, AuditPort, CacheKey, CachePort, CompletionRequest, FormatTranslatorPort, - HealthStatus, ProviderPort, RequestMetadata, RouterPort, StreamChunk, -}; -use rook_usecases::RouteRequest; -use std::{sync::Arc, time::Duration}; +use rook_core::FormatTranslatorPort; use transport_axum::format_registry::{DomainPivotTranslator, FormatRegistry}; struct RegistryTestProvider { @@ -342,6 +344,36 @@ impl AuditPort for NoopAudit { } } +/// Test stub for ModelAliasRepositoryPort +struct NoopAliasRepository; + +#[async_trait] +impl ModelAliasRepositoryPort for NoopAliasRepository { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + Ok(None) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete(&self, _alias: &ModelId) -> Result { + Ok(false) + } + + async fn seed(&self, _aliases: Vec) -> Result { + Ok(0) + } +} + static REGISTRY_TEST_MODEL: std::sync::LazyLock = std::sync::LazyLock::new(|| ModelId::new("registry-test-model")); @@ -376,6 +408,11 @@ fn registry_route_request(provider_format: ApiFormat, content: &'static str) -> None, // combo_repository Arc::new(rook_usecases::PricingConfig::default()), registry_with_openai_anthropic_pairs(), + Arc::new(NoopAliasRepository), + ModelAliasesConfig { + enabled: false, + auto_seed: false, + }, ) } From 0ca01d2bfa2b3bf8f6c3d65f309637fe112377f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:39:50 +0200 Subject: [PATCH 3/6] feat(cache): HTTP management API, config validation, and observability (2/2) (#110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cache): content-based cache keys with SHA-256 signatures Implement content-based caching for issue #50: - Add signature field to CacheKey (SHA-256 hex string) - Update CompletionRequest.cache_key() to hash (model + messages + params) - Add CacheStats struct with hit_rate() and utilization() methods - Add LRU eviction to InMemoryCache with max_entries support - Add AtomicU64 counters (hits, misses, evictions) - Implement stats() method in InMemoryCache - Add comprehensive unit tests for hashing, LRU, and stats Breaking change: CacheKey now requires signature field (documented in proposal) * fix(cache): address code review findings - Add documentation for approximate LRU under concurrency - Fix evict_if_needed to only trigger on new keys (not overwrites) - Add test for no-eviction-on-overwrite behavior - Clarify spec: None=unlimited, Some(0)=rejected (usize cannot be negative) - Update tasks to specify max_entries validation rules * feat(cache): HTTP management API, config validation, and observability (2/2) Completes issue #50 implementation: **Phase 3: Ports** - Add stats() method to CachePort trait - Add delete_by_signature() for HTTP endpoint support - Implement both methods in InMemoryCache **Phase 4: Configuration** - Add max_entries field to CacheConfig - Implement validate() rejecting ttl > 24h and max_entries = Some(0) - Wire validation at config load (fail-fast) - Pass max_entries to cache constructor in DI **Phase 5: Application** - Add cache() accessor to RouteRequest (already tracking stats) **Phase 6: Transport** - Create cache.rs handler module - Implement GET /api/cache/stats (200 with CacheStats JSON) - Implement DELETE /api/cache (204 clear all) - Implement DELETE /api/cache/:signature (204/404) - Wire cache routes (management API, requires auth) - Extend /health with cache_entries, cache_hit_rate, cache_utilization **Phase 7: Observability** - Add rook_cache_evictions counter description - Wire eviction metric in InMemoryCache **Tests** - 5 config validation tests - 6 cache HTTP endpoint integration tests - All 450+ tests passing * fix: apply remaining code review findings from PR #110 - cache-memory: only increment deleted/evictions when store.remove returns Some - routes.rs: use Axum 0.8 path syntax {signature} - cache_routes.rs: add test_cache_routes_require_management_auth - config_tests.rs: fix assertion message to match actual validation error - tasks.md: fix dependencies (9.* → 9.1-9.4), update delete behavior description --- Cargo.lock | 2 + apps/rook/src/config.rs | 28 ++ apps/rook/src/di.rs | 17 +- apps/rook/tests/config_tests.rs | 152 ++++++++++ .../rook-usecases/src/route_request.rs | 25 +- .../tests/route_request_restrictions.rs | 16 +- crates/domain/rook-core/src/ports.rs | 3 + crates/infrastructure/cache-memory/Cargo.toml | 3 + crates/infrastructure/cache-memory/src/lib.rs | 47 ++- .../observability/src/metrics.rs | 1 + .../infrastructure/transport-axum/Cargo.toml | 1 + .../transport-axum/src/bootstrap_helpers.rs | 6 + .../transport-axum/src/handlers/cache.rs | 51 ++++ .../transport-axum/src/handlers/mod.rs | 1 + .../transport-axum/src/routes.rs | 41 ++- .../transport-axum/tests/cache_routes.rs | 285 ++++++++++++++++++ .../tests/format_translation_integration.rs | 14 + openspec/changes/read-cache/tasks.md | 46 +-- 18 files changed, 705 insertions(+), 34 deletions(-) create mode 100644 crates/infrastructure/transport-axum/src/handlers/cache.rs create mode 100644 crates/infrastructure/transport-axum/tests/cache_routes.rs diff --git a/Cargo.lock b/Cargo.lock index a30f25b7..0801595a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -431,6 +431,7 @@ version = "0.1.0" dependencies = [ "async-trait", "dashmap", + "metrics", "rook-core", "serde", "shared-kernel", @@ -3526,6 +3527,7 @@ dependencies = [ "axum", "axum-test", "base64", + "cache-memory", "chrono", "dashmap", "encryption-inmemory", diff --git a/apps/rook/src/config.rs b/apps/rook/src/config.rs index 728771bc..74cd0707 100644 --- a/apps/rook/src/config.rs +++ b/apps/rook/src/config.rs @@ -191,12 +191,34 @@ pub struct CacheConfig { pub enabled: bool, #[serde(rename = "ttl_secs")] pub ttl_secs: u64, + #[serde(default)] + pub max_entries: Option, } impl CacheConfig { pub fn ttl(&self) -> Duration { Duration::from_secs(self.ttl_secs) } + + /// Validate cache configuration at startup + pub fn validate(&self) -> Result<(), String> { + // Reject TTL > 24 hours (86400 seconds) + if self.ttl_secs > 86400 { + return Err(format!( + "cache.ttl_secs ({}) exceeds 24h maximum (86400)", + self.ttl_secs + )); + } + + // Reject max_entries = Some(0) + if let Some(0) = self.max_entries { + return Err( + "cache.max_entries must be greater than 0 or None for unlimited".to_string(), + ); + } + + Ok(()) + } } #[derive(Debug, Clone, Deserialize)] @@ -290,6 +312,12 @@ impl RookConfig { // Validate combo configurations Self::validate_combos(&config.combos); + // Validate cache configuration + config + .cache + .validate() + .map_err(|e| anyhow::anyhow!("invalid cache config: {}", e))?; + Ok(config) } diff --git a/apps/rook/src/di.rs b/apps/rook/src/di.rs index 36ca202a..36b3bf8b 100644 --- a/apps/rook/src/di.rs +++ b/apps/rook/src/di.rs @@ -72,7 +72,10 @@ impl RookContainer { // 1. Cache let cache: Arc = if config.cache.enabled { - Arc::new(InMemoryCache::new(config.cache.ttl(), None)) + Arc::new(InMemoryCache::new( + config.cache.ttl(), + config.cache.max_entries, + )) } else { Arc::new(NoOpCache) }; @@ -626,6 +629,18 @@ impl CachePort for NoOpCache { async fn clear(&self) -> CortexResult<()> { Ok(()) } + async fn stats(&self) -> CortexResult { + Ok(rook_core::CacheStats { + hits: 0, + misses: 0, + evictions: 0, + entries: 0, + max_entries: 0, + }) + } + async fn delete_by_signature(&self, _: &str) -> CortexResult { + Ok(0) + } } // --------------------------------------------------------------------------- diff --git a/apps/rook/tests/config_tests.rs b/apps/rook/tests/config_tests.rs index 3eebeeb8..4c6cae4e 100644 --- a/apps/rook/tests/config_tests.rs +++ b/apps/rook/tests/config_tests.rs @@ -88,3 +88,155 @@ completion_per_million = 2.40 assert_eq!(price.cache_read_per_million, None); assert_eq!(price.cache_creation_per_million, None); } + +#[test] +fn cache_config_validation_rejects_ttl_exceeding_24_hours() { + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 86401 +"#; + + let config: RookConfig = toml::from_str(config_str).expect("config parses"); + let validation_result = config.cache.validate(); + + assert!(validation_result.is_err()); + assert!(validation_result + .unwrap_err() + .contains("exceeds 24h maximum")); +} + +#[test] +fn cache_config_validation_accepts_valid_ttl() { + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 3600 +"#; + + let config: RookConfig = toml::from_str(config_str).expect("config parses"); + let validation_result = config.cache.validate(); + + assert!(validation_result.is_ok()); +} + +#[test] +fn cache_config_validation_rejects_max_entries_zero() { + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 300 +max_entries = 0 +"#; + + let config: RookConfig = toml::from_str(config_str).expect("config parses"); + let validation_result = config.cache.validate(); + + assert!(validation_result.is_err()); + assert!(validation_result + .unwrap_err() + .contains("cache.max_entries must be greater than 0")); +} + +#[test] +fn cache_config_validation_rejects_invalid_config_on_load() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary config file with invalid cache settings + let mut temp_file = NamedTempFile::new().expect("failed to create temp file"); + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 86401 +"#; + temp_file + .write_all(config_str.as_bytes()) + .expect("failed to write temp file"); + temp_file.flush().expect("failed to flush temp file"); + + // Call RookConfig::load (the startup path) + let result = RookConfig::load(temp_file.path()); + + // Should fail with error containing validation message + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("invalid cache config") || err_msg.contains("exceeds 24h maximum"), + "Expected validation error message, got: {}", + err_msg + ); +} + +#[test] +fn cache_config_validation_accepts_none_max_entries() { + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 300 +"#; + + let config: RookConfig = toml::from_str(config_str).expect("config parses"); + assert_eq!(config.cache.max_entries, None); + let validation_result = config.cache.validate(); + + assert!(validation_result.is_ok()); +} + +#[test] +fn cache_config_validation_accepts_valid_max_entries() { + let config_str = r#" +[server] +host = "127.0.0.1" +port = 0 + +[routing] +strategy = "priority" + +[cache] +enabled = true +ttl_secs = 300 +max_entries = 1000 +"#; + + let config: RookConfig = toml::from_str(config_str).expect("config parses"); + assert_eq!(config.cache.max_entries, Some(1000)); + let validation_result = config.cache.validate(); + + assert!(validation_result.is_ok()); +} diff --git a/crates/application/rook-usecases/src/route_request.rs b/crates/application/rook-usecases/src/route_request.rs index f4133a97..8fc9f24c 100644 --- a/crates/application/rook-usecases/src/route_request.rs +++ b/crates/application/rook-usecases/src/route_request.rs @@ -73,6 +73,11 @@ impl RouteRequest { self.combo_repository.clone() } + /// Get cache reference (for HTTP management API) + pub fn cache(&self) -> Arc { + self.cache.clone() + } + pub async fn execute(&self, req: CompletionRequest) -> Result { self.execute_with_format(req, ApiFormat::OpenAI).await } @@ -833,9 +838,9 @@ mod tests { use async_trait::async_trait; use futures::{stream, StreamExt}; use rook_core::{ - ApiKeyId, CostBreakdown, HealthStatus, Message, ModelId, Pagination, ProviderId, - ProviderPort, ProviderRepositoryPort, RequestMetadata, Role, StreamChunk, TokenUsage, - UsageEntry, UsageFilters, UsageRecorderPort, UsageSummary, + ApiKeyId, CacheStats, CostBreakdown, HealthStatus, Message, ModelId, Pagination, + ProviderId, ProviderPort, ProviderRepositoryPort, RequestMetadata, Role, StreamChunk, + TokenUsage, UsageEntry, UsageFilters, UsageRecorderPort, UsageSummary, }; use shared_kernel::{CacheKey, ConnectionId, CortexResult, RequestId}; use std::collections::HashMap; @@ -972,6 +977,20 @@ mod tests { async fn clear(&self) -> CortexResult<()> { Ok(()) } + + async fn stats(&self) -> CortexResult { + Ok(CacheStats { + hits: 0, + misses: 0, + evictions: 0, + entries: 0, + max_entries: 0, + }) + } + + async fn delete_by_signature(&self, _signature: &str) -> CortexResult { + Ok(0) + } } struct TestAudit { diff --git a/crates/application/rook-usecases/tests/route_request_restrictions.rs b/crates/application/rook-usecases/tests/route_request_restrictions.rs index 4fdb9267..8fb8d995 100644 --- a/crates/application/rook-usecases/tests/route_request_restrictions.rs +++ b/crates/application/rook-usecases/tests/route_request_restrictions.rs @@ -6,7 +6,7 @@ use std::time::Duration; use async_trait::async_trait; use rook_core::{ - ApiFormat, ApiKeyRestrictions, AuditEntry, AuditPort, CachePort, CompletionRequest, + ApiFormat, ApiKeyRestrictions, AuditEntry, AuditPort, CachePort, CacheStats, CompletionRequest, CompletionResponse, CortexError, CortexResult, FormatTranslatorPort, HealthStatus, Message, MessageContent, ModelId, ProviderId, ProviderPort, RequestMetadata, Role, RouterPort, StreamChunk, TokenUsage, @@ -138,6 +138,20 @@ impl CachePort for NoOpCache { async fn clear(&self) -> CortexResult<()> { Ok(()) } + + async fn stats(&self) -> CortexResult { + Ok(CacheStats { + hits: 0, + misses: 0, + evictions: 0, + entries: 0, + max_entries: 0, + }) + } + + async fn delete_by_signature(&self, _signature: &str) -> CortexResult { + Ok(0) + } } struct NoOpAudit; diff --git a/crates/domain/rook-core/src/ports.rs b/crates/domain/rook-core/src/ports.rs index 0f97f555..a5d159aa 100644 --- a/crates/domain/rook-core/src/ports.rs +++ b/crates/domain/rook-core/src/ports.rs @@ -111,6 +111,9 @@ pub trait CachePort: Send + Sync { ) -> CortexResult<()>; async fn delete(&self, key: &CacheKey) -> CortexResult<()>; async fn clear(&self) -> CortexResult<()>; + async fn stats(&self) -> CortexResult; + /// Delete all entries matching the given signature. Returns number of entries deleted. + async fn delete_by_signature(&self, signature: &str) -> CortexResult; } // --------------------------------------------------------------------------- diff --git a/crates/infrastructure/cache-memory/Cargo.toml b/crates/infrastructure/cache-memory/Cargo.toml index 03f9ed12..53ddd1de 100644 --- a/crates/infrastructure/cache-memory/Cargo.toml +++ b/crates/infrastructure/cache-memory/Cargo.toml @@ -18,6 +18,9 @@ serde = { version = "1", features = ["derive"] } dashmap = "6" ttl_cache = "0.5" +# Observability +metrics = "0.24" + # Logging tracing = "0.1" diff --git a/crates/infrastructure/cache-memory/src/lib.rs b/crates/infrastructure/cache-memory/src/lib.rs index 65eda4fc..5bc505b1 100644 --- a/crates/infrastructure/cache-memory/src/lib.rs +++ b/crates/infrastructure/cache-memory/src/lib.rs @@ -49,6 +49,32 @@ impl InMemoryCache { } } + /// Delete all entries matching the given signature. + /// Returns the number of entries deleted. + pub fn delete_by_signature(&self, signature: &str) -> usize { + let mut deleted = 0; + // Collect keys matching the signature + let keys_to_delete: Vec = self + .store + .iter() + .filter(|entry| entry.key().signature == signature) + .map(|entry| entry.key().clone()) + .collect(); + + // Delete all matching keys + for key in keys_to_delete { + // Only increment deleted if an entry was actually removed + if self.store.remove(&key).is_some() { + deleted += 1; + } + // Clean up associated metadata regardless + self.expiry.remove(&key); + self.last_accessed.remove(&key); + } + + deleted + } + /// Evict the least recently used entry if cache is at capacity. /// /// **Note on concurrency**: LRU eviction is approximate under concurrent access. @@ -67,11 +93,14 @@ impl InMemoryCache { .min_by_key(|entry| *entry.value()) .map(|entry| entry.key().clone()) { - // Remove from all maps - self.store.remove(&oldest); - self.expiry.remove(&oldest); - self.last_accessed.remove(&oldest); - self.evictions.fetch_add(1, Ordering::Relaxed); + // Only evict if the entry still exists (check store.remove result) + if self.store.remove(&oldest).is_some() { + self.expiry.remove(&oldest); + self.last_accessed.remove(&oldest); + self.evictions.fetch_add(1, Ordering::Relaxed); + // Emit Prometheus metric + metrics::counter!("rook_cache_evictions").increment(1); + } } } } @@ -136,6 +165,14 @@ impl CachePort for InMemoryCache { self.evictions.store(0, Ordering::Relaxed); Ok(()) } + + async fn stats(&self) -> CortexResult { + Ok(self.stats()) + } + + async fn delete_by_signature(&self, signature: &str) -> CortexResult { + Ok(self.delete_by_signature(signature)) + } } // --------------------------------------------------------------------------- diff --git a/crates/infrastructure/observability/src/metrics.rs b/crates/infrastructure/observability/src/metrics.rs index f0098b16..edbe0167 100644 --- a/crates/infrastructure/observability/src/metrics.rs +++ b/crates/infrastructure/observability/src/metrics.rs @@ -17,4 +17,5 @@ pub fn init_metrics() { metrics::describe_counter!("rook_provider_errors", "Total provider errors"); metrics::describe_counter!("rook_cache_hits", "Cache hits"); metrics::describe_counter!("rook_cache_misses", "Cache misses"); + metrics::describe_counter!("rook_cache_evictions", "Cache evictions (LRU)"); } diff --git a/crates/infrastructure/transport-axum/Cargo.toml b/crates/infrastructure/transport-axum/Cargo.toml index c63977dc..4e179157 100644 --- a/crates/infrastructure/transport-axum/Cargo.toml +++ b/crates/infrastructure/transport-axum/Cargo.toml @@ -49,4 +49,5 @@ dashmap = "6" [dev-dependencies] axum-test = "20" encryption-inmemory = { path = "../encryption-inmemory" } +cache-memory = { path = "../cache-memory" } diff --git a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs index 8b757e2f..c45949c1 100644 --- a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs +++ b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs @@ -112,6 +112,12 @@ impl CachePort for StubCache { async fn clear(&self) -> CortexResult<()> { unreachable!("cache not called by bootstrap tests") } + async fn stats(&self) -> CortexResult { + unreachable!("cache not called by bootstrap tests") + } + async fn delete_by_signature(&self, _: &str) -> CortexResult { + unreachable!("cache not called by bootstrap tests") + } } struct StubAudit; diff --git a/crates/infrastructure/transport-axum/src/handlers/cache.rs b/crates/infrastructure/transport-axum/src/handlers/cache.rs new file mode 100644 index 00000000..74cabb96 --- /dev/null +++ b/crates/infrastructure/transport-axum/src/handlers/cache.rs @@ -0,0 +1,51 @@ +// Cache management HTTP handlers + +use axum::{ + extract::{Extension, Path}, + http::StatusCode, + Json, +}; +use rook_core::{CachePort, CacheStats}; +use std::sync::Arc; + +/// GET /api/cache/stats — Return cache statistics +pub async fn get_cache_stats( + Extension(cache): Extension>, +) -> Result, StatusCode> { + cache + .stats() + .await + .map(Json) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) +} + +/// DELETE /api/cache — Clear entire cache +pub async fn clear_cache( + Extension(cache): Extension>, +) -> Result { + cache + .clear() + .await + .map(|_| StatusCode::NO_CONTENT) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) +} + +/// DELETE /api/cache/:signature — Delete specific cache entry by signature +/// +/// Returns 204 regardless of whether entry existed (idempotent delete). +pub async fn delete_cache_entry( + Path(signature): Path, + Extension(cache): Extension>, +) -> Result { + // Validate signature format (64 hex characters for SHA-256) + if signature.len() != 64 || !signature.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(StatusCode::BAD_REQUEST); + } + + // Delete all entries matching this signature + cache + .delete_by_signature(&signature) + .await + .map(|_| StatusCode::NO_CONTENT) // Idempotent: always 204 regardless of count + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) +} diff --git a/crates/infrastructure/transport-axum/src/handlers/mod.rs b/crates/infrastructure/transport-axum/src/handlers/mod.rs index e89fbade..632c5347 100644 --- a/crates/infrastructure/transport-axum/src/handlers/mod.rs +++ b/crates/infrastructure/transport-axum/src/handlers/mod.rs @@ -3,6 +3,7 @@ pub mod api_key; pub mod auth; pub mod bootstrap; +pub mod cache; pub mod models; pub mod models_dto; pub mod rate_limits; diff --git a/crates/infrastructure/transport-axum/src/routes.rs b/crates/infrastructure/transport-axum/src/routes.rs index 66078d3b..ee29fe2c 100644 --- a/crates/infrastructure/transport-axum/src/routes.rs +++ b/crates/infrastructure/transport-axum/src/routes.rs @@ -90,6 +90,9 @@ pub fn router( // Usage history routes — always mounted, returns 503 if usage recorder is unavailable router = router.merge(usage_routes(usecases.clone())); + // Cache management routes — always mounted + router = router.merge(cache_routes(usecases.clone())); + // Rate limit admin API (if enabled) if let Some(store) = rate_limit_store { router = router.merge(rate_limits_routes(store)); @@ -768,6 +771,21 @@ async fn health_check(State(usecases): State) -> impl IntoResponse { use std::collections::HashMap; let circuit_map: HashMap<_, _> = circuit_states.into_iter().collect(); + // Get cache stats + let cache_stats = + usecases + .route_request + .cache() + .stats() + .await + .unwrap_or(rook_core::CacheStats { + hits: 0, + misses: 0, + evictions: 0, + entries: 0, + max_entries: 0, + }); + Json(serde_json::json!({ "status": status, "providers": statuses.iter().map(|s| { @@ -793,7 +811,16 @@ async fn health_check(State(usecases): State) -> impl IntoResponse { } provider_json - }).collect::>() + }).collect::>(), + "cache_stats": { + "hits": cache_stats.hits, + "misses": cache_stats.misses, + "evictions": cache_stats.evictions, + "entries": cache_stats.entries, + "max_entries": cache_stats.max_entries, + "hit_rate": cache_stats.hit_rate(), + "utilization": cache_stats.utilization(), + } })) } @@ -840,3 +867,15 @@ fn usage_routes(usecases: Usecases) -> Router { .route("/api/usage/cost", get(handlers::usage::usage_cost)) .with_state(usecases) } + +fn cache_routes(usecases: Usecases) -> Router { + let cache = usecases.route_request.cache(); + Router::new() + .route("/api/cache/stats", get(handlers::cache::get_cache_stats)) + .route("/api/cache", delete(handlers::cache::clear_cache)) + .route( + "/api/cache/{signature}", + delete(handlers::cache::delete_cache_entry), + ) + .layer(axum::extract::Extension(cache)) +} diff --git a/crates/infrastructure/transport-axum/tests/cache_routes.rs b/crates/infrastructure/transport-axum/tests/cache_routes.rs new file mode 100644 index 00000000..bc5f04c0 --- /dev/null +++ b/crates/infrastructure/transport-axum/tests/cache_routes.rs @@ -0,0 +1,285 @@ +// Integration tests for cache management HTTP endpoints + +use axum::{ + body::Body, + extract::Extension, + http::{Request, StatusCode}, +}; +use cache_memory::InMemoryCache; +use rook_core::{ + CachePort, CacheStats, CompletionResponse, MessageContent, ModelId, ProviderId, TokenUsage, +}; +use shared_kernel::{CacheKey, RequestId}; +use std::sync::Arc; +use std::time::Duration; +use tower::ServiceExt; +use transport_axum::authz::{classify_route, AuthTier}; +use transport_axum::handlers::cache::{clear_cache, delete_cache_entry, get_cache_stats}; + +/// Helper: build a test CompletionResponse +fn make_response(content: &str) -> CompletionResponse { + CompletionResponse { + id: RequestId::new(), + model: ModelId::new("gpt-4o"), + provider: ProviderId::new("openai"), + content: content.to_string(), + content_blocks: vec![MessageContent::Text(content.to_string())], + usage: TokenUsage { + prompt_tokens: 10, + completion_tokens: 20, + total_tokens: 30, + cache_read_tokens: None, + cache_creation_tokens: None, + reasoning_tokens: None, + estimated_cost_usd: None, + }, + latency_ms: 100, + } +} + +#[tokio::test] +async fn get_cache_stats_returns_200_with_json() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + // Populate cache with some entries + let key1 = CacheKey { + request_id: RequestId::new(), + signature: "a".repeat(64), + }; + let key2 = CacheKey { + request_id: RequestId::new(), + signature: "b".repeat(64), + }; + cache + .set(&key1, &make_response("test1"), Duration::from_secs(300)) + .await + .unwrap(); + cache + .set(&key2, &make_response("test2"), Duration::from_secs(300)) + .await + .unwrap(); + + // Simulate a cache hit + let _ = cache.get(&key1).await; + + let app = axum::Router::new() + .route("/api/cache/stats", axum::routing::get(get_cache_stats)) + .layer(Extension(cache)); + + let response = app + .oneshot( + Request::builder() + .uri("/api/cache/stats") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let stats: CacheStats = serde_json::from_slice(&body).unwrap(); + + assert_eq!(stats.entries, 2); + assert_eq!(stats.hits, 1); + assert_eq!(stats.misses, 0); +} + +#[tokio::test] +async fn clear_cache_returns_204_and_clears_all_entries() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + // Populate cache + let key = CacheKey { + request_id: RequestId::new(), + signature: "c".repeat(64), + }; + cache + .set(&key, &make_response("test"), Duration::from_secs(300)) + .await + .unwrap(); + + let app = axum::Router::new() + .route("/api/cache", axum::routing::delete(clear_cache)) + .layer(Extension(cache.clone())); + + let response = app + .oneshot( + Request::builder() + .method("DELETE") + .uri("/api/cache") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + // Verify cache is empty + let stats = cache.stats().await.unwrap(); + assert_eq!(stats.entries, 0); +} + +#[tokio::test] +async fn delete_cache_entry_returns_204_for_valid_signature() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + let signature = "d".repeat(64); + let key = CacheKey { + request_id: RequestId::new(), + signature: signature.clone(), + }; + cache + .set(&key, &make_response("test"), Duration::from_secs(300)) + .await + .unwrap(); + + let app = axum::Router::new() + .route( + "/api/cache/{signature}", + axum::routing::delete(delete_cache_entry), + ) + .layer(Extension(cache.clone())); + + let response = app + .oneshot( + Request::builder() + .method("DELETE") + .uri(format!("/api/cache/{}", signature)) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + // Verify entry was deleted + let deleted_count = cache.delete_by_signature(&signature).await.unwrap(); + assert_eq!(deleted_count, 0); // Already deleted +} + +#[tokio::test] +async fn delete_cache_entry_returns_204_for_missing_signature() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + let signature = "e".repeat(64); + + let app = axum::Router::new() + .route( + "/api/cache/{signature}", + axum::routing::delete(delete_cache_entry), + ) + .layer(Extension(cache)); + + let response = app + .oneshot( + Request::builder() + .method("DELETE") + .uri(format!("/api/cache/{}", signature)) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + // Idempotent delete: 204 even if not found + assert_eq!(response.status(), StatusCode::NO_CONTENT); +} + +#[tokio::test] +async fn delete_cache_entry_returns_400_for_malformed_signature() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + let app = axum::Router::new() + .route( + "/api/cache/{signature}", + axum::routing::delete(delete_cache_entry), + ) + .layer(Extension(cache)); + + // Too short + let response = app + .clone() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/api/cache/short") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // Non-hex characters + let response = app + .oneshot( + Request::builder() + .method("DELETE") + .uri(format!("/api/cache/{}", "z".repeat(64))) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn cache_stats_reflect_hits_and_misses() { + let cache: Arc = Arc::new(InMemoryCache::new(Duration::from_secs(300), None)); + + let key = CacheKey { + request_id: RequestId::new(), + signature: "f".repeat(64), + }; + cache + .set(&key, &make_response("test"), Duration::from_secs(300)) + .await + .unwrap(); + + // Hit + let _ = cache.get(&key).await; + + // Miss + let missing_key = CacheKey { + request_id: RequestId::new(), + signature: "g".repeat(64), + }; + let _ = cache.get(&missing_key).await; + + let stats = cache.stats().await.unwrap(); + assert_eq!(stats.hits, 1); + assert_eq!(stats.misses, 1); + assert_eq!(stats.hit_rate(), 0.5); +} + +#[test] +fn cache_routes_require_management_auth() { + use axum::http::Method; + + // Verify that cache management routes are classified as Management tier + // which requires session authentication (not API key or anonymous) + assert_eq!( + classify_route(&Method::GET, "/api/cache/stats"), + AuthTier::Management, + "/api/cache/stats should require Management auth" + ); + assert_eq!( + classify_route(&Method::DELETE, "/api/cache"), + AuthTier::Management, + "/api/cache DELETE should require Management auth" + ); + assert_eq!( + classify_route(&Method::DELETE, "/api/cache/somesignature"), + AuthTier::Management, + "/api/cache/{{signature}} DELETE should require Management auth" + ); +} diff --git a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs index d602899a..461398ac 100644 --- a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs +++ b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs @@ -331,6 +331,20 @@ impl CachePort for NoopCache { async fn clear(&self) -> shared_kernel::CortexResult<()> { Ok(()) } + + async fn stats(&self) -> shared_kernel::CortexResult { + Ok(rook_core::CacheStats { + hits: 0, + misses: 0, + evictions: 0, + entries: 0, + max_entries: 0, + }) + } + + async fn delete_by_signature(&self, _signature: &str) -> shared_kernel::CortexResult { + Ok(0) + } } struct NoopAudit; diff --git a/openspec/changes/read-cache/tasks.md b/openspec/changes/read-cache/tasks.md index 1d4dc4b6..dc9bd9d1 100644 --- a/openspec/changes/read-cache/tasks.md +++ b/openspec/changes/read-cache/tasks.md @@ -45,33 +45,33 @@ Chain strategy: pending ## Phase 3: Ports (Interface Extension) -- [ ] 3.1 **Add `stats()` method to `CachePort` trait** — `rook-core/src/ports.rs` — Signature: `async fn stats(&self) -> CortexResult;` — Dependencies: 1.3 — Complexity: simple -- [ ] 3.2 **Implement `CachePort::stats()` for `InMemoryCache`** — `cache-memory/src/lib.rs` — Delegate to internal `stats()` method, wrap in `Ok()` — Dependencies: 2.7, 3.1 — Complexity: simple +- [x] 3.1 **Add `stats()` method to `CachePort` trait** — `rook-core/src/ports.rs` — Signature: `async fn stats(&self) -> CortexResult;` — Dependencies: 1.3 — Complexity: simple +- [x] 3.2 **Implement `CachePort::stats()` for `InMemoryCache`** — `cache-memory/src/lib.rs` — Delegate to internal `stats()` method, wrap in `Ok()` — Dependencies: 2.7, 3.1 — Complexity: simple ## Phase 4: Configuration -- [ ] 4.1 **Add `max_entries` field to `CacheConfig`** — `apps/rook/src/config.rs` — Add `pub max_entries: Option` with default `None` — Dependencies: none — Complexity: simple -- [ ] 4.2 **Implement `CacheConfig::validate()`** — `apps/rook/src/config.rs` — Return error if `ttl_secs > 86400`; if `max_entries.is_some()` validate it is `> 0`, reject `Some(0)`; allow `None` (unlimited) — Dependencies: none — Complexity: simple -- [ ] 4.3 **Call `validate()` at startup** — `apps/rook/src/main.rs` or config loading — Fail fast if config invalid — Dependencies: 4.2 — Complexity: simple -- [ ] 4.4 **Pass `max_entries` to `InMemoryCache::new()`** — `apps/rook/src/main.rs` or DI setup — Wire config value to cache constructor — Dependencies: 2.3, 4.1 — Complexity: simple +- [x] 4.1 **Add `max_entries` field to `CacheConfig`** — `apps/rook/src/config.rs` — Add `pub max_entries: Option` with default `None` — Dependencies: none — Complexity: simple +- [x] 4.2 **Implement `CacheConfig::validate()`** — `apps/rook/src/config.rs` — Return error if `ttl_secs > 86400`; if `max_entries.is_some()` validate it is `> 0`, reject `Some(0)`; allow `None` (unlimited) — Dependencies: none — Complexity: simple +- [x] 4.3 **Call `validate()` at startup** — `apps/rook/src/main.rs` or config loading — Fail fast if config invalid — Dependencies: 4.2 — Complexity: simple +- [x] 4.4 **Pass `max_entries` to `InMemoryCache::new()`** — `apps/rook/src/main.rs` or DI setup — Wire config value to cache constructor — Dependencies: 2.3, 4.1 — Complexity: simple ## Phase 5: Application Layer -- [ ] 5.1 **Update `RouteRequest` to increment stats** — `rook-usecases/src/route_request.rs` — On cache hit: already incremented in `get()`; on cache miss: already incremented in `get()` — verify existing logic or add explicit stats tracking — Dependencies: 2.5 — Complexity: simple +- [x] 5.1 **Update `RouteRequest` to increment stats** — `rook-usecases/src/route_request.rs` — On cache hit: already incremented in `get()`; on cache miss: already incremented in `get()` — verify existing logic or add explicit stats tracking — Dependencies: 2.5 — Complexity: simple ## Phase 6: Transport Layer (HTTP) -- [ ] 6.1 **Create `transport-axum/src/handlers/cache.rs`** — New file with handler stubs — Dependencies: none — Complexity: simple -- [ ] 6.2 **Implement `get_cache_stats` handler** — `transport-axum/src/handlers/cache.rs` — Extract `Arc`, call `cache.stats()`, return `Json` or 500 — Dependencies: 3.1, 6.1 — Complexity: medium -- [ ] 6.3 **Implement `clear_cache` handler** — `transport-axum/src/handlers/cache.rs` — Extract cache port, call `cache.clear()`, return 204 or 500 — Dependencies: 6.1 — Complexity: simple -- [ ] 6.4 **Implement `delete_cache_entry` handler** — `transport-axum/src/handlers/cache.rs` — Extract `Path(signature)`, construct `CacheKey`, call `cache.delete()`, return 204 (deleted) or 404 (not found) — Dependencies: 1.1, 6.1 — Complexity: medium -- [ ] 6.5 **Wire cache routes** — `transport-axum/src/routes.rs` — Add `GET /api/cache/stats`, `DELETE /api/cache`, `DELETE /api/cache/:signature` — Dependencies: 6.2, 6.3, 6.4 — Complexity: simple -- [ ] 6.6 **Extend `/health` with cache stats** — `transport-axum/src/handlers/health.rs` — Add `cache_entries`, `cache_hit_rate`, `cache_utilization` fields to health response — Dependencies: 3.1 — Complexity: medium +- [x] 6.1 **Create `transport-axum/src/handlers/cache.rs`** — New file with handler stubs — Dependencies: none — Complexity: simple +- [x] 6.2 **Implement `get_cache_stats` handler** — `transport-axum/src/handlers/cache.rs` — Extract `Arc`, call `cache.stats()`, return `Json` or 500 — Dependencies: 3.1, 6.1 — Complexity: medium +- [x] 6.3 **Implement `clear_cache` handler** — `transport-axum/src/handlers/cache.rs` — Extract cache port, call `cache.clear()`, return 204 or 500 — Dependencies: 6.1 — Complexity: simple +- [x] 6.4 **Implement `delete_cache_entry` handler** — `transport-axum/src/handlers/cache.rs` — Extract `Path(signature)`, call `cache.delete_by_signature(&str)` (idempotent), return 204 for both present and missing signatures — Dependencies: 1.1, 6.1 — Complexity: medium +- [x] 6.5 **Wire cache routes** — `transport-axum/src/routes.rs` — Add `GET /api/cache/stats`, `DELETE /api/cache`, `DELETE /api/cache/:signature` — Dependencies: 6.2, 6.3, 6.4 — Complexity: simple +- [x] 6.6 **Extend `/health` with cache stats** — `transport-axum/src/handlers/health.rs` — Add `cache_entries`, `cache_hit_rate`, `cache_utilization` fields to health response — Dependencies: 3.1 — Complexity: medium ## Phase 7: Observability -- [ ] 7.1 **Add `rook_cache_evictions` counter** — `observability/src/metrics.rs` — Prometheus counter for evictions — Dependencies: none — Complexity: simple -- [ ] 7.2 **Emit eviction metric in `InMemoryCache::set()`** — `cache-memory/src/lib.rs` — Increment Prometheus counter when eviction occurs — Dependencies: 2.4, 7.1 — Complexity: simple +- [x] 7.1 **Add `rook_cache_evictions` counter** — `observability/src/metrics.rs` — Prometheus counter for evictions — Dependencies: none — Complexity: simple +- [x] 7.2 **Emit eviction metric in `InMemoryCache::set()`** — `cache-memory/src/lib.rs` — Increment Prometheus counter when eviction occurs — Dependencies: 2.4, 7.1 — Complexity: simple ## Phase 8: Unit Tests @@ -82,23 +82,23 @@ Chain strategy: pending - [x] 8.5 **Test concurrent access** — `cache-memory/src/lib.rs` — 100 threads performing get/set/clear → no panics, final state consistent — Dependencies: 2.4, 2.5 — Complexity: complex - [x] 8.6 **Test `CacheStats::hit_rate()`** — `rook-core/src/model.rs` — Zero requests → 0.0, hits only → 1.0, mixed → correct ratio — Dependencies: 1.3 — Complexity: simple - [x] 8.7 **Test `CacheStats::utilization()`** — `rook-core/src/model.rs` — No limit → None, partial → correct fraction, full → 1.0 — Dependencies: 1.3 — Complexity: simple -- [ ] 8.8 **Test `CacheConfig::validate()`** — `apps/rook/src/config.rs` — `ttl_secs > 86400` → error, valid config → Ok — Dependencies: 4.2 — Complexity: simple +- [x] 8.8 **Test `CacheConfig::validate()`** — `apps/rook/src/config.rs` — `ttl_secs > 86400` → error, valid config → Ok — Dependencies: 4.2 — Complexity: simple ## Phase 9: Integration Tests -- [ ] 9.1 **Test `GET /api/cache/stats` endpoint** — `transport-axum/tests/` — Empty cache → entries=0, after operations → correct counts — Dependencies: 6.2, 6.5 — Complexity: medium -- [ ] 9.2 **Test `DELETE /api/cache` endpoint** — `transport-axum/tests/` — Populate cache, clear, verify stats show entries=0 — Dependencies: 6.3, 6.5 — Complexity: simple -- [ ] 9.3 **Test `DELETE /api/cache/:signature` endpoint** — `transport-axum/tests/` — Delete existing → 204, delete missing → 404 — Dependencies: 6.4, 6.5 — Complexity: medium -- [ ] 9.4 **Test `/health` includes cache stats** — `transport-axum/tests/` — Verify cache fields present in JSON response — Dependencies: 6.6 — Complexity: simple +- [x] 9.1 **Test `GET /api/cache/stats` endpoint** — `transport-axum/tests/` — Empty cache → entries=0, after operations → correct counts — Dependencies: 6.2, 6.5 — Complexity: medium +- [x] 9.2 **Test `DELETE /api/cache` endpoint** — `transport-axum/tests/` — Populate cache, clear, verify stats show entries=0 — Dependencies: 6.3, 6.5 — Complexity: simple +- [x] 9.3 **Test `DELETE /api/cache/:signature` endpoint** — `transport-axum/tests/` — Delete existing → 204, delete missing → 404 — Dependencies: 6.4, 6.5 — Complexity: medium +- [x] 9.4 **Test `/health` includes cache stats** — `transport-axum/tests/` — Verify cache fields present in JSON response — Dependencies: 6.6 — Complexity: simple - [ ] 9.5 **Test end-to-end cache hit flow** — `apps/rook/tests/` — Same request twice → second returns cached response, stats show hit — Dependencies: 2.5, 5.1 — Complexity: complex - [ ] 9.6 **Test end-to-end cache miss flow** — `apps/rook/tests/` — Unique request → routed to provider, cached for next time — Dependencies: 2.5, 5.1 — Complexity: complex - [ ] 9.7 **Test LRU eviction in full system** — `apps/rook/tests/` — Fill cache to limit, trigger eviction, verify oldest gone — Dependencies: 2.4, 4.4 — Complexity: complex ## Phase 10: Verification -- [ ] 10.1 **Run `cargo test`** — All unit + integration tests pass — Dependencies: 8.*, 9.* — Complexity: simple -- [ ] 10.2 **Run `cargo clippy`** — No warnings — Dependencies: all code tasks — Complexity: simple -- [ ] 10.3 **Run `cargo fmt --check`** — Code formatted — Dependencies: all code tasks — Complexity: simple +- [x] 10.1 **Run `cargo test`** — All unit + integration tests pass — Dependencies: 8.*, 9.1–9.4 — Complexity: simple +- [x] 10.2 **Run `cargo clippy`** — No warnings — Dependencies: all code tasks — Complexity: simple +- [x] 10.3 **Run `cargo fmt --check`** — Code formatted — Dependencies: all code tasks — Complexity: simple - [ ] 10.4 **Run `just ci-local`** — Full CI pipeline passes locally — Dependencies: 10.1, 10.2, 10.3 — Complexity: simple - [ ] 10.5 **Manual smoke test** — Start server, hit `/api/cache/stats`, verify response, perform cache operations, verify stats update — Dependencies: all implementation tasks — Complexity: medium From 21691691a9246c61a6ddfdb234fd9cd6e74cd0d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 08:18:15 +0200 Subject: [PATCH 4/6] feat: add model alias domain model and SQLite repository - Add ModelAlias struct and ModelAliasRepositoryPort trait - Create alias-sqlite crate with SqliteModelAliasRepository - Add V5 migration for model_aliases table - Include 26 built-in aliases (OpenAI, Anthropic, Google, Mistral, Groq) - Add 11 unit tests for repository operations - Implement cycle prevention and idempotent seeding Part of #47 --- Cargo.lock | 16 + Cargo.toml | 1 + crates/domain/rook-core/src/model.rs | 17 + crates/domain/rook-core/src/ports.rs | 40 ++ crates/infrastructure/alias-sqlite/Cargo.toml | 19 + .../alias-sqlite/src/builtin.rs | 65 +++ crates/infrastructure/alias-sqlite/src/lib.rs | 9 + .../alias-sqlite/src/repository.rs | 440 ++++++++++++++++++ .../src/migrations/V5__model_aliases.sql | 15 + 9 files changed, 622 insertions(+) create mode 100644 crates/infrastructure/alias-sqlite/Cargo.toml create mode 100644 crates/infrastructure/alias-sqlite/src/builtin.rs create mode 100644 crates/infrastructure/alias-sqlite/src/lib.rs create mode 100644 crates/infrastructure/alias-sqlite/src/repository.rs create mode 100644 crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql diff --git a/Cargo.lock b/Cargo.lock index 0801595a..05f38733 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,6 +58,22 @@ dependencies = [ "memchr", ] +[[package]] +name = "alias-sqlite" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "chrono", + "db-migration", + "rook-core", + "rusqlite", + "shared-kernel", + "tempfile", + "thiserror", + "tokio", +] + [[package]] name = "android_system_properties" version = "0.1.5" diff --git a/Cargo.toml b/Cargo.toml index 3e930182..70cb1734 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ members = [ "crates/infrastructure/db-migration", "crates/infrastructure/models-catalog", "crates/infrastructure/combo-sqlite", + "crates/infrastructure/alias-sqlite", "apps/rook", ] diff --git a/crates/domain/rook-core/src/model.rs b/crates/domain/rook-core/src/model.rs index b23674d2..8d7c8dd9 100644 --- a/crates/domain/rook-core/src/model.rs +++ b/crates/domain/rook-core/src/model.rs @@ -573,6 +573,23 @@ impl std::fmt::Display for ComboValidationError { impl std::error::Error for ComboValidationError {} +// ============================================================================ +// ModelAlias — model alias mapping for stable model names +// ============================================================================ + +/// Model alias mapping — resolves friendly alias names to canonical model IDs +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ModelAlias { + /// The alias name (e.g., "gpt-4o-latest") + pub alias: ModelId, + /// The canonical model ID (e.g., "gpt-4o-2024-05-13") + pub canonical: ModelId, + /// Optional provider scope (null = global) + pub provider_id: Option, + /// Creation timestamp (ISO 8601) + pub created_at: String, +} + /// A multi-step fallback chain aggregate #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Combo { diff --git a/crates/domain/rook-core/src/ports.rs b/crates/domain/rook-core/src/ports.rs index a5d159aa..a04a480a 100644 --- a/crates/domain/rook-core/src/ports.rs +++ b/crates/domain/rook-core/src/ports.rs @@ -560,3 +560,43 @@ pub trait ComboRepositoryPort: Send + Sync { /// Delete a combo by its ID (cascades to steps) async fn delete(&self, id: &ComboId) -> Result<(), ComboRepositoryError>; } + +// --------------------------------------------------------------------------- +// ModelAliasRepositoryPort — persistence for model alias mappings +// --------------------------------------------------------------------------- + +use crate::ModelAlias; + +#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)] +pub enum ModelAliasRepositoryError { + #[error("alias not found: {0}")] + NotFound(ModelId), + #[error("alias already exists: {0}")] + AlreadyExists(ModelId), + #[error("invalid alias: {0}")] + InvalidAlias(String), + #[error("database error: {0}")] + Database(String), +} + +#[async_trait] +pub trait ModelAliasRepositoryPort: Send + Sync { + /// Resolve alias to canonical model. Returns None if not found. + async fn find_by_alias( + &self, + alias: &ModelId, + provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError>; + + /// List all aliases ordered by alias name + async fn list(&self) -> Result, ModelAliasRepositoryError>; + + /// Create new alias. Returns AlreadyExists if duplicate. + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError>; + + /// Delete alias by name. Returns true if deleted, false if not found. + async fn delete(&self, alias: &ModelId) -> Result; + + /// Seed aliases idempotently. Returns count of new aliases inserted. + async fn seed(&self, aliases: Vec) -> Result; +} diff --git a/crates/infrastructure/alias-sqlite/Cargo.toml b/crates/infrastructure/alias-sqlite/Cargo.toml new file mode 100644 index 00000000..4d754b9d --- /dev/null +++ b/crates/infrastructure/alias-sqlite/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "alias-sqlite" +version = "0.1.0" +edition = "2021" + +[dependencies] +db-migration = { path = "../../infrastructure/db-migration" } +shared-kernel = { path = "../../domain/shared-kernel" } +rook-core = { path = "../../domain/rook-core" } + +async-trait = { workspace = true } +chrono = { workspace = true } +rusqlite = { workspace = true } +thiserror = { workspace = true } +anyhow = { workspace = true } + +[dev-dependencies] +tokio = { workspace = true } +tempfile = "3" diff --git a/crates/infrastructure/alias-sqlite/src/builtin.rs b/crates/infrastructure/alias-sqlite/src/builtin.rs new file mode 100644 index 00000000..30881b84 --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/builtin.rs @@ -0,0 +1,65 @@ +//! Built-in model aliases — seeded at startup when table is empty + +/// Built-in aliases: (alias, canonical, provider_id) +/// Provider ID is None for global aliases +pub const DEFAULT_ALIASES: &[(&str, &str, Option<&str>)] = &[ + // OpenAI + ("gpt-4o-latest", "gpt-4o-2024-05-13", Some("openai")), + ("gpt-4o", "gpt-4o-2024-05-13", Some("openai")), + ("gpt-4-turbo", "gpt-4-turbo-2024-04-09", Some("openai")), + ("gpt-4", "gpt-4-0613", Some("openai")), + ("gpt-3.5-turbo", "gpt-3.5-turbo-0125", Some("openai")), + ("o1", "o1-2024-12-17", Some("openai")), + ("o1-mini", "o1-mini-2024-09-12", Some("openai")), + ("o3-mini", "o3-mini-2025-01-31", Some("openai")), + // Anthropic + ("claude-opus", "claude-3-opus-20240229", Some("anthropic")), + ( + "claude-sonnet", + "claude-3-5-sonnet-20241022", + Some("anthropic"), + ), + ( + "claude-haiku", + "claude-3-5-haiku-20241022", + Some("anthropic"), + ), + ("claude-3-opus", "claude-3-opus-20240229", Some("anthropic")), + ( + "claude-3-sonnet", + "claude-3-sonnet-20240229", + Some("anthropic"), + ), + ( + "claude-3-haiku", + "claude-3-haiku-20240307", + Some("anthropic"), + ), + // Google Gemini + ("gemini-pro", "gemini-1.5-pro-latest", Some("gemini")), + ("gemini-flash", "gemini-1.5-flash-latest", Some("gemini")), + ("gemini-2.0-flash", "gemini-2.0-flash-exp", Some("gemini")), + ("gemini-1.5-pro", "gemini-1.5-pro-latest", Some("gemini")), + ( + "gemini-1.5-flash", + "gemini-1.5-flash-latest", + Some("gemini"), + ), + ("gemini-exp", "gemini-exp-1206", Some("gemini")), + // Mistral + ("mistral-large", "mistral-large-2411", Some("mistral")), + ("mistral-small", "mistral-small-2501", Some("mistral")), + ("ministral-8b", "ministral-8b-2410", Some("mistral")), + // Groq + ( + "groq-llama-3.1-70b", + "llama-3.1-70b-versatile", + Some("groq"), + ), + ("groq-llama-3.1-8b", "llama-3.1-8b-instant", Some("groq")), + ( + "groq-llama-3.3-70b", + "llama-3.3-70b-versatile", + Some("groq"), + ), +]; diff --git a/crates/infrastructure/alias-sqlite/src/lib.rs b/crates/infrastructure/alias-sqlite/src/lib.rs new file mode 100644 index 00000000..8192d62e --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/lib.rs @@ -0,0 +1,9 @@ +//! alias-sqlite — SQLite-backed model alias repository implementation + +pub mod builtin; +pub mod repository; + +pub use repository::SqliteModelAliasRepository; + +// Re-export traits and types for convenience +pub use rook_core::ports::{ModelAliasRepositoryError, ModelAliasRepositoryPort}; diff --git a/crates/infrastructure/alias-sqlite/src/repository.rs b/crates/infrastructure/alias-sqlite/src/repository.rs new file mode 100644 index 00000000..eea7844f --- /dev/null +++ b/crates/infrastructure/alias-sqlite/src/repository.rs @@ -0,0 +1,440 @@ +use std::path::Path; +use std::sync::{Mutex, MutexGuard}; + +use async_trait::async_trait; +use chrono::Utc; +use rook_core::ports::{ModelAliasRepositoryError, ModelAliasRepositoryPort}; +use rook_core::ModelAlias; +use rusqlite::{params, Connection, OptionalExtension}; +use shared_kernel::{ModelId, ProviderId}; + +use crate::builtin::DEFAULT_ALIASES; + +pub struct SqliteModelAliasRepository { + conn: Mutex, +} + +impl SqliteModelAliasRepository { + pub fn new(db_path: impl AsRef) -> anyhow::Result { + let mut conn = Connection::open(&db_path)?; + conn.execute_batch( + "PRAGMA journal_mode = WAL; + PRAGMA busy_timeout = 5000; + PRAGMA synchronous = NORMAL; + PRAGMA foreign_keys = ON;", + )?; + + // Run migrations for in-memory databases + if db_path.as_ref().to_str() == Some(":memory:") { + db_migration::run_on_connection(&mut conn)?; + } + + Ok(Self { + conn: Mutex::new(conn), + }) + } + + fn lock(&self) -> Result, ModelAliasRepositoryError> { + self.conn + .lock() + .map_err(|_| ModelAliasRepositoryError::Database("sqlite mutex poisoned".to_string())) + } +} + +#[async_trait] +impl ModelAliasRepositoryPort for SqliteModelAliasRepository { + async fn find_by_alias( + &self, + alias: &ModelId, + provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + let conn = self.lock()?; + + let result = if let Some(pid) = provider_id { + // Provider-scoped query + conn.query_row( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + WHERE alias = ?1 AND (provider_id = ?2 OR provider_id IS NULL) + ORDER BY CASE WHEN provider_id IS NOT NULL THEN 0 ELSE 1 END + LIMIT 1", + params![alias.as_str(), pid.as_str()], + |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }, + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + } else { + // Global query + conn.query_row( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + WHERE alias = ?1 + LIMIT 1", + params![alias.as_str()], + |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }, + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + }; + + Ok(result) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + let conn = self.lock()?; + + let mut stmt = conn + .prepare( + "SELECT alias, canonical, provider_id, created_at + FROM model_aliases + ORDER BY alias", + ) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + let aliases = stmt + .query_map([], |row| { + Ok(ModelAlias { + alias: ModelId::new(row.get::<_, String>(0)?), + canonical: ModelId::new(row.get::<_, String>(1)?), + provider_id: row + .get::<_, Option>(2)? + .map(|s| ProviderId::new(&s)), + created_at: row.get(3)?, + }) + }) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? + .collect::, _>>() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(aliases) + } + + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + let conn = self.lock()?; + + // Check if canonical is itself an alias (prevent cycles) + let canonical_is_alias = conn + .query_row( + "SELECT 1 FROM model_aliases WHERE alias = ?1 LIMIT 1", + params![alias.canonical.as_str()], + |_| Ok(()), + ) + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + if canonical_is_alias.is_some() { + return Err(ModelAliasRepositoryError::InvalidAlias( + "Canonical model cannot be an alias".to_string(), + )); + } + + // Insert the alias + let result = conn.execute( + "INSERT INTO model_aliases (alias, canonical, provider_id, created_at) + VALUES (?1, ?2, ?3, ?4)", + params![ + alias.alias.as_str(), + alias.canonical.as_str(), + alias.provider_id.as_ref().map(|p| p.as_str()), + alias.created_at, + ], + ); + + match result { + Ok(_) => Ok(()), + Err(rusqlite::Error::SqliteFailure(err, _)) + if err.code == rusqlite::ErrorCode::ConstraintViolation => + { + Err(ModelAliasRepositoryError::AlreadyExists(alias.alias)) + } + Err(e) => Err(ModelAliasRepositoryError::Database(e.to_string())), + } + } + + async fn delete(&self, alias: &ModelId) -> Result { + let conn = self.lock()?; + + let rows_affected = conn + .execute( + "DELETE FROM model_aliases WHERE alias = ?1", + params![alias.as_str()], + ) + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(rows_affected > 0) + } + + async fn seed(&self, aliases: Vec) -> Result { + let mut conn = self.lock()?; + + let tx = conn + .transaction() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + let mut inserted = 0; + for alias in aliases { + let result = tx.execute( + "INSERT OR IGNORE INTO model_aliases (alias, canonical, provider_id, created_at) + VALUES (?1, ?2, ?3, ?4)", + params![ + alias.alias.as_str(), + alias.canonical.as_str(), + alias.provider_id.as_ref().map(|p| p.as_str()), + alias.created_at, + ], + ); + + match result { + Ok(rows) => inserted += rows, + Err(e) => { + return Err(ModelAliasRepositoryError::Database(e.to_string())); + } + } + } + + tx.commit() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + + Ok(inserted) + } +} + +/// Helper function to create built-in aliases from constants +pub fn builtin_aliases() -> Vec { + let now = Utc::now().to_rfc3339(); + + DEFAULT_ALIASES + .iter() + .map(|(alias_str, canonical_str, provider_id_str)| { + let provider_id = provider_id_str.map(ProviderId::new); + ModelAlias { + alias: ModelId::new(alias_str.to_string()), + canonical: ModelId::new(canonical_str.to_string()), + provider_id, + created_at: now.clone(), + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + async fn create_test_repo() -> SqliteModelAliasRepository { + SqliteModelAliasRepository::new(":memory:").expect("failed to create test repository") + } + + fn test_alias(alias: &str, canonical: &str) -> ModelAlias { + ModelAlias { + alias: ModelId::new(alias), + canonical: ModelId::new(canonical), + provider_id: None, + created_at: Utc::now().to_rfc3339(), + } + } + + #[tokio::test] + async fn test_find_by_alias_found() { + let repo = create_test_repo().await; + let alias = test_alias("gpt-4o-latest", "gpt-4o-2024-05-13"); + + repo.create(alias.clone()).await.expect("create failed"); + + let result = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("find failed"); + + assert!(result.is_some()); + let found = result.unwrap(); + assert_eq!(found.alias.as_str(), "gpt-4o-latest"); + assert_eq!(found.canonical.as_str(), "gpt-4o-2024-05-13"); + } + + #[tokio::test] + async fn test_find_by_alias_not_found() { + let repo = create_test_repo().await; + + let result = repo + .find_by_alias(&ModelId::new("non-existent"), None) + .await + .expect("find failed"); + + assert!(result.is_none()); + } + + #[tokio::test] + async fn test_create_success() { + let repo = create_test_repo().await; + let alias = test_alias("my-alias", "gpt-4o"); + + repo.create(alias).await.expect("create failed"); + + let found = repo + .find_by_alias(&ModelId::new("my-alias"), None) + .await + .expect("find failed") + .expect("alias not found"); + + assert_eq!(found.canonical.as_str(), "gpt-4o"); + } + + #[tokio::test] + async fn test_create_duplicate() { + let repo = create_test_repo().await; + let alias = test_alias("duplicate", "gpt-4o"); + + repo.create(alias.clone()) + .await + .expect("first create failed"); + + let result = repo.create(alias).await; + + assert!(matches!( + result, + Err(ModelAliasRepositoryError::AlreadyExists(_)) + )); + } + + #[tokio::test] + async fn test_create_alias_cycle() { + let repo = create_test_repo().await; + + // Create first alias + let alias1 = test_alias("alias-a", "canonical-model"); + repo.create(alias1).await.expect("create alias-a failed"); + + // Try to create alias pointing to another alias + let alias2 = test_alias("alias-b", "alias-a"); + let result = repo.create(alias2).await; + + assert!(matches!( + result, + Err(ModelAliasRepositoryError::InvalidAlias(_)) + )); + } + + #[tokio::test] + async fn test_delete_success() { + let repo = create_test_repo().await; + let alias = test_alias("to-delete", "gpt-4o"); + + repo.create(alias).await.expect("create failed"); + + let deleted = repo + .delete(&ModelId::new("to-delete")) + .await + .expect("delete failed"); + + assert!(deleted); + + let found = repo + .find_by_alias(&ModelId::new("to-delete"), None) + .await + .expect("find failed"); + + assert!(found.is_none()); + } + + #[tokio::test] + async fn test_delete_not_found() { + let repo = create_test_repo().await; + + let deleted = repo + .delete(&ModelId::new("non-existent")) + .await + .expect("delete failed"); + + assert!(!deleted); + } + + #[tokio::test] + async fn test_seed_empty_table() { + let repo = create_test_repo().await; + let aliases = builtin_aliases(); + let count = aliases.len(); + + let inserted = repo.seed(aliases).await.expect("seed failed"); + + assert_eq!(inserted, count); + + let all = repo.list().await.expect("list failed"); + assert_eq!(all.len(), count); + } + + #[tokio::test] + async fn test_seed_idempotent() { + let repo = create_test_repo().await; + let aliases = builtin_aliases(); + let count = aliases.len(); + + // First seed + let inserted1 = repo.seed(aliases.clone()).await.expect("first seed failed"); + assert_eq!(inserted1, count); + + // Second seed (should be idempotent) + let inserted2 = repo.seed(aliases).await.expect("second seed failed"); + assert_eq!(inserted2, 0); // No new inserts + + let all = repo.list().await.expect("list failed"); + assert_eq!(all.len(), count); // Still same count + } + + #[tokio::test] + async fn test_list_returns_all_aliases() { + let repo = create_test_repo().await; + + repo.create(test_alias("alias-1", "model-1")) + .await + .expect("create 1 failed"); + repo.create(test_alias("alias-2", "model-2")) + .await + .expect("create 2 failed"); + repo.create(test_alias("alias-3", "model-3")) + .await + .expect("create 3 failed"); + + let all = repo.list().await.expect("list failed"); + + assert_eq!(all.len(), 3); + assert_eq!(all[0].alias.as_str(), "alias-1"); // Sorted by alias + assert_eq!(all[1].alias.as_str(), "alias-2"); + assert_eq!(all[2].alias.as_str(), "alias-3"); + } + + #[tokio::test] + async fn test_builtin_aliases_count() { + let aliases = builtin_aliases(); + + // Verify we have at least 26 built-in aliases as per design + assert!( + aliases.len() >= 26, + "Expected at least 26 built-in aliases, got {}", + aliases.len() + ); + + // Verify structure + assert!(aliases.iter().any(|a| a.alias.as_str() == "gpt-4o-latest")); + assert!(aliases.iter().any(|a| a.alias.as_str() == "claude-opus")); + assert!(aliases.iter().any(|a| a.alias.as_str() == "gemini-pro")); + } +} diff --git a/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql b/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql new file mode 100644 index 00000000..e6a508c5 --- /dev/null +++ b/crates/infrastructure/db-migration/src/migrations/V5__model_aliases.sql @@ -0,0 +1,15 @@ +-- V5: Model Aliases Table +-- Provides stable alias names that resolve to canonical model IDs + +CREATE TABLE IF NOT EXISTS model_aliases ( + alias TEXT PRIMARY KEY NOT NULL, + canonical TEXT NOT NULL, + provider_id TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')) +); + +-- Index for lookups by canonical model (useful for cycle detection) +CREATE INDEX IF NOT EXISTS idx_model_aliases_canonical ON model_aliases(canonical); + +-- Index for provider-scoped queries (future enhancement) +CREATE INDEX IF NOT EXISTS idx_model_aliases_provider ON model_aliases(provider_id); From 9870a8312227194ece016ec982b115718a4d56ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:39:29 +0200 Subject: [PATCH 5/6] feat: add model alias resolution and HTTP API (#111) - Add [model_aliases] config section with enabled and auto_seed flags - Wire SqliteModelAliasRepository into DI container with startup seeding - Implement alias resolution in RouteRequest before restrictions check - Add GET/POST/DELETE endpoints for alias management at /api/models/aliases - Add cycle prevention validation in create endpoint - Add 7 E2E tests for alias resolution and seeding - Add 10 HTTP API integration tests - Add 5 config tests for model aliases section Part of #47 --- Cargo.lock | 1 + apps/rook/Cargo.toml | 1 + apps/rook/src/config.rs | 33 ++ apps/rook/src/di.rs | 35 +- apps/rook/tests/alias_routing_e2e.rs | 203 +++++++++ apps/rook/tests/config_tests.rs | 71 +++ .../rook-usecases/src/route_request.rs | 120 +++++- .../tests/route_request_restrictions.rs | 59 ++- .../transport-axum/src/alias_routes.rs | 21 + .../transport-axum/src/bootstrap_helpers.rs | 44 +- .../transport-axum/src/handlers/aliases.rs | 172 ++++++++ .../transport-axum/src/handlers/mod.rs | 1 + .../infrastructure/transport-axum/src/lib.rs | 1 + .../transport-axum/src/routes.rs | 10 +- .../transport-axum/tests/alias_api.rs | 407 ++++++++++++++++++ .../tests/format_translation_integration.rs | 55 ++- 16 files changed, 1210 insertions(+), 24 deletions(-) create mode 100644 apps/rook/tests/alias_routing_e2e.rs create mode 100644 crates/infrastructure/transport-axum/src/alias_routes.rs create mode 100644 crates/infrastructure/transport-axum/src/handlers/aliases.rs create mode 100644 crates/infrastructure/transport-axum/tests/alias_api.rs diff --git a/Cargo.lock b/Cargo.lock index 05f38733..47b5cf1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2648,6 +2648,7 @@ dependencies = [ name = "rook" version = "0.0.1" dependencies = [ + "alias-sqlite", "anyhow", "async-trait", "audit-sqlite", diff --git a/apps/rook/Cargo.toml b/apps/rook/Cargo.toml index f44ff1f7..9888ce1f 100644 --- a/apps/rook/Cargo.toml +++ b/apps/rook/Cargo.toml @@ -26,6 +26,7 @@ providers-groq = { path = "../../crates/infrastructure/providers-groq" } encryption-inmemory = { path = "../../crates/infrastructure/encryption-inmemory" } provider-sqlite = { path = "../../crates/infrastructure/provider-sqlite" } auth-sqlite = { path = "../../crates/infrastructure/auth-sqlite" } +alias-sqlite = { path = "../../crates/infrastructure/alias-sqlite" } db-migration = { path = "../../crates/infrastructure/db-migration" } models-catalog = { path = "../../crates/infrastructure/models-catalog" } tokio = { version = "1", features = ["full"] } diff --git a/apps/rook/src/config.rs b/apps/rook/src/config.rs index 74cd0707..a2511a1c 100644 --- a/apps/rook/src/config.rs +++ b/apps/rook/src/config.rs @@ -30,6 +30,9 @@ pub struct RookConfig { /// Combo (multi-step fallback chain) definitions #[serde(default)] pub combos: Vec, + /// Model aliases configuration + #[serde(default)] + pub model_aliases: ModelAliasesConfig, } #[derive(Debug, Clone, Deserialize)] @@ -72,6 +75,36 @@ fn default_allow_env_fallback() -> bool { true } +#[derive(Debug, Clone, Deserialize)] +pub struct ModelAliasesConfig { + #[serde(default = "default_true")] + pub enabled: bool, + #[serde(default = "default_true")] + pub auto_seed: bool, +} + +impl Default for ModelAliasesConfig { + fn default() -> Self { + Self { + enabled: true, + auto_seed: true, + } + } +} + +impl From for rook_usecases::route_request::ModelAliasesConfig { + fn from(cfg: ModelAliasesConfig) -> Self { + Self { + enabled: cfg.enabled, + auto_seed: cfg.auto_seed, + } + } +} + +fn default_true() -> bool { + true +} + #[derive(Debug, Clone, Deserialize)] pub struct ProviderCrudConfig { #[serde(default = "default_provider_crud_enabled")] diff --git a/apps/rook/src/di.rs b/apps/rook/src/di.rs index 36b3bf8b..a4f4847f 100644 --- a/apps/rook/src/di.rs +++ b/apps/rook/src/di.rs @@ -6,6 +6,7 @@ use std::path::Path; use std::sync::Arc; use std::time::Duration; +use alias_sqlite::SqliteModelAliasRepository; use audit_sqlite::{SqliteAudit, SqliteUsageRepository}; use auth_sqlite::{SqliteApiKeyRepository, SqliteSessionRepository, SqliteUserRepository}; use cache_memory::InMemoryCache; @@ -17,9 +18,9 @@ use provider_sqlite::SqliteProviderRepository; use providers_ollama::OllamaProvider; use rook_core::{ ApiKeyRepositoryPort, AuditPort, CachePort, Combo, ComboRepositoryPort, ComboStep, - ComboStrategy, ConnectionId, DecryptedCredentials, PasswordHasher, ProviderId, ProviderKind, - ProviderPort, ProviderRegistryPort, ProviderRepositoryPort, RouterPort, SessionRepositoryPort, - UsageRecorderPort, + ComboStrategy, ConnectionId, DecryptedCredentials, ModelAliasRepositoryPort, PasswordHasher, + ProviderId, ProviderKind, ProviderPort, ProviderRegistryPort, ProviderRepositoryPort, + RouterPort, SessionRepositoryPort, UsageRecorderPort, }; use rook_usecases::{ AuthenticateClientApi, BootstrapStatus, EnsureAdminUser, FallbackRouter, HealthCheck, @@ -172,6 +173,32 @@ impl RookContainer { let combo_repo: Arc = Arc::new(ComboSqliteRepository::new(&config.database.db_path)?); + // 7d. Model alias repository — SQLite-backed alias storage + let alias_repo: Arc = + Arc::new(SqliteModelAliasRepository::new(&config.database.db_path)?); + + // 7e. Seed built-in aliases if enabled + if config.model_aliases.auto_seed { + let builtin_aliases = alias_sqlite::builtin::DEFAULT_ALIASES + .iter() + .map(|(alias, canonical, provider_id)| rook_core::ModelAlias { + alias: shared_kernel::ModelId::new(*alias), + canonical: shared_kernel::ModelId::new(*canonical), + provider_id: provider_id.map(shared_kernel::ProviderId::new), + created_at: shared_kernel::Utc::now().to_rfc3339(), + }) + .collect::>(); + + match alias_repo.seed(builtin_aliases).await { + Ok(count) => { + tracing::info!(count, "Seeded default model aliases"); + } + Err(e) => { + tracing::warn!(error = ?e, "Failed to seed model aliases"); + } + } + } + // 8. Run async initialization tasks concurrently: // - registry refresh (if provider_crud enabled) // - ensure admin user exists @@ -231,6 +258,8 @@ impl RookContainer { Some(combo_repo.clone()), // combo_repository - wired in Phase 4 Arc::new(config.pricing.clone()), format_registry.clone(), + alias_repo.clone(), // model_alias_repository - wired in Phase 3 + config.model_aliases.clone().into(), ), ManageProviders::new(router.clone()), health_check, diff --git a/apps/rook/tests/alias_routing_e2e.rs b/apps/rook/tests/alias_routing_e2e.rs new file mode 100644 index 00000000..5e548749 --- /dev/null +++ b/apps/rook/tests/alias_routing_e2e.rs @@ -0,0 +1,203 @@ +use alias_sqlite::{repository::builtin_aliases, SqliteModelAliasRepository}; +use rook_core::ports::ModelAliasRepositoryPort; +use rook_core::{ModelAlias, ModelId}; +use std::sync::Arc; + +/// Test that alias resolution works end-to-end in routing +/// +/// Scenario: User sends request with alias "gpt-4o-latest", which should resolve +/// to canonical model "gpt-4o-2024-05-13" before restrictions/routing logic. +#[tokio::test] +async fn test_alias_resolves_before_routing() { + // Setup: Create in-memory repository with test alias + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Create test alias + let alias = ModelAlias { + alias: ModelId::new("gpt-4o-latest"), + canonical: ModelId::new("gpt-4o-2024-05-13"), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo.create(alias.clone()).await.expect("create alias"); + + // Verify alias was created + let resolved = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("find alias") + .expect("alias exists"); + + assert_eq!(resolved.canonical, ModelId::new("gpt-4o-2024-05-13")); +} + +/// Test that unknown aliases pass through unchanged (fail-open behavior) +#[tokio::test] +async fn test_unknown_alias_passes_through() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Query non-existent alias + let result = repo + .find_by_alias(&ModelId::new("unknown-model"), None) + .await + .expect("query succeeds"); + + assert!(result.is_none(), "unknown alias should return None"); +} + +/// Test that canonical model IDs pass through unchanged +#[tokio::test] +async fn test_canonical_model_passes_through() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Create alias + let alias = ModelAlias { + alias: ModelId::new("gpt-4-turbo"), + canonical: ModelId::new("gpt-4-turbo-2024-04-09"), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo.create(alias).await.expect("create alias"); + + // Query with canonical model (not alias) + let result = repo + .find_by_alias(&ModelId::new("gpt-4-turbo-2024-04-09"), None) + .await + .expect("query succeeds"); + + // Canonical models should not resolve to anything (they are not aliases) + assert!( + result.is_none(), + "canonical model should not resolve as alias" + ); +} + +/// Test that built-in aliases are available after seeding +#[tokio::test] +async fn test_builtin_aliases_available_after_seed() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Seed built-in aliases + let count = repo.seed(builtin_aliases()).await.expect("seed succeeds"); + assert!(count > 20, "should seed at least 20 built-in aliases"); + + // Verify some known built-in aliases exist + let openai_alias = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("query succeeds") + .expect("gpt-4o-latest should exist"); + assert_eq!(openai_alias.canonical, ModelId::new("gpt-4o-2024-05-13")); + + let anthropic_alias = repo + .find_by_alias(&ModelId::new("claude-opus"), None) + .await + .expect("query succeeds") + .expect("claude-opus should exist"); + assert_eq!( + anthropic_alias.canonical, + ModelId::new("claude-3-opus-20240229") + ); + + let gemini_alias = repo + .find_by_alias(&ModelId::new("gemini-2.0-flash"), None) + .await + .expect("query succeeds") + .expect("gemini-2.0-flash should exist"); + assert_eq!(gemini_alias.canonical, ModelId::new("gemini-2.0-flash-exp")); +} + +/// Test alias resolution with provider-scoped lookup +#[tokio::test] +async fn test_provider_scoped_alias_lookup() { + use rook_core::ProviderId; + + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // Seed builtin aliases (which have provider_id set) + let count = repo.seed(builtin_aliases()).await.expect("seed succeeds"); + assert!(count > 0); + + // Query with provider filter - should find provider-specific alias + let openai_alias = repo + .find_by_alias( + &ModelId::new("gpt-4o-latest"), + Some(&ProviderId::new("openai")), + ) + .await + .expect("query succeeds") + .expect("openai alias exists"); + assert_eq!(openai_alias.canonical, ModelId::new("gpt-4o-2024-05-13")); + assert_eq!(openai_alias.provider_id, Some(ProviderId::new("openai"))); + + // Query without provider filter - should also find it + let global_lookup = repo + .find_by_alias(&ModelId::new("gpt-4o-latest"), None) + .await + .expect("query succeeds") + .expect("alias exists"); + + assert_eq!(global_lookup.canonical, ModelId::new("gpt-4o-2024-05-13")); +} + +/// Test that seeding is idempotent (can be run multiple times safely) +#[tokio::test] +async fn test_seed_is_idempotent() { + let repo = SqliteModelAliasRepository::new(":memory:").expect("create in-memory repo"); + + // First seed + let count1 = repo + .seed(builtin_aliases()) + .await + .expect("first seed succeeds"); + assert!(count1 > 0, "first seed should insert aliases"); + + // Second seed + let count2 = repo + .seed(builtin_aliases()) + .await + .expect("second seed succeeds"); + assert_eq!(count2, 0, "second seed should insert nothing (idempotent)"); + + // Verify aliases still exist and weren't duplicated + let all_aliases = repo.list().await.expect("list succeeds"); + assert_eq!( + all_aliases.len(), + count1 as usize, + "should have same count as first seed" + ); +} + +/// Test repository error handling (fail-open behavior) +#[tokio::test] +async fn test_repository_handles_concurrent_access() { + let repo = Arc::new(SqliteModelAliasRepository::new(":memory:").expect("create repo")); + + // Create multiple concurrent queries + let mut handles = vec![]; + for i in 0..10 { + let repo_clone = Arc::clone(&repo); + let handle = tokio::spawn(async move { + let alias = ModelAlias { + alias: ModelId::new(format!("test-alias-{}", i)), + canonical: ModelId::new(format!("test-canonical-{}", i)), + provider_id: None, + created_at: "2026-06-05T07:10:00Z".to_string(), + }; + repo_clone.create(alias).await + }); + handles.push(handle); + } + + // Wait for all to complete + for handle in handles { + handle + .await + .expect("task completes") + .expect("create succeeds"); + } + + // Verify all aliases were created + let all_aliases = repo.list().await.expect("list succeeds"); + assert_eq!(all_aliases.len(), 10, "all aliases should be created"); +} diff --git a/apps/rook/tests/config_tests.rs b/apps/rook/tests/config_tests.rs index 4c6cae4e..fe31edc1 100644 --- a/apps/rook/tests/config_tests.rs +++ b/apps/rook/tests/config_tests.rs @@ -90,6 +90,7 @@ completion_per_million = 2.40 } #[test] +<<<<<<< HEAD fn cache_config_validation_rejects_ttl_exceeding_24_hours() { let config_str = r#" [server] @@ -193,10 +194,64 @@ ttl_secs = 86401 err_msg.contains("invalid cache config") || err_msg.contains("exceeds 24h maximum"), "Expected validation error message, got: {}", err_msg +======= +fn config_model_aliases_defaults_to_enabled_and_auto_seed() { + let config: RookConfig = toml::from_str(&minimal_config_toml("")).expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!(config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_can_be_disabled() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = false +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!(!config.model_aliases.enabled); + assert!(!config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_deserializes_from_toml() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = true +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!(!config.model_aliases.auto_seed); +} + +#[test] +fn config_model_aliases_enabled_only() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +enabled = true +"#, + )) + .expect("config parses"); + + assert!(config.model_aliases.enabled); + assert!( + config.model_aliases.auto_seed, + "auto_seed should default to true" +>>>>>>> 7a397b2 (feat: add model alias resolution and HTTP API (#111)) ); } #[test] +<<<<<<< HEAD fn cache_config_validation_accepts_none_max_entries() { let config_str = r#" [server] @@ -239,4 +294,20 @@ max_entries = 1000 let validation_result = config.cache.validate(); assert!(validation_result.is_ok()); +======= +fn config_model_aliases_auto_seed_only() { + let config: RookConfig = toml::from_str(&minimal_config_toml( + r#" +[model_aliases] +auto_seed = false +"#, + )) + .expect("config parses"); + + assert!( + config.model_aliases.enabled, + "enabled should default to true" + ); + assert!(!config.model_aliases.auto_seed); +>>>>>>> 7a397b2 (feat: add model alias resolution and HTTP API (#111)) } diff --git a/crates/application/rook-usecases/src/route_request.rs b/crates/application/rook-usecases/src/route_request.rs index 8fc9f24c..1aa5a297 100644 --- a/crates/application/rook-usecases/src/route_request.rs +++ b/crates/application/rook-usecases/src/route_request.rs @@ -15,8 +15,9 @@ use chrono::Utc; use futures::StreamExt; use rook_core::{ ApiFormat, AuditEntry, AuditPort, CachePort, ComboRepositoryPort, CompletionRequest, - CompletionResponse, CortexError, FormatTranslatorPort, ProviderRepositoryPort, RequestStatus, - RouterPort, StreamChunk, TokenUsage, UsageEntry, UsageRecorderPort, + CompletionResponse, CortexError, FormatTranslatorPort, ModelAliasRepositoryPort, + ProviderRepositoryPort, RequestStatus, RouterPort, StreamChunk, TokenUsage, UsageEntry, + UsageRecorderPort, }; use shared_kernel::{ComboId, ConnectionId, ProviderId, RestrictionViolation}; @@ -42,6 +43,15 @@ pub struct RouteRequest { combo_repository: Option>, pricing: Arc, format_translator: Arc, + alias_repository: Arc, + alias_config: ModelAliasesConfig, +} + +/// Configuration for model alias resolution +#[derive(Debug, Clone)] +pub struct ModelAliasesConfig { + pub enabled: bool, + pub auto_seed: bool, } impl RouteRequest { @@ -55,6 +65,8 @@ impl RouteRequest { combo_repository: Option>, pricing: Arc, format_translator: Arc, + alias_repository: Arc, + alias_config: ModelAliasesConfig, ) -> Self { Self { router, @@ -65,6 +77,8 @@ impl RouteRequest { combo_repository, pricing, format_translator, + alias_repository, + alias_config, } } @@ -73,9 +87,15 @@ impl RouteRequest { self.combo_repository.clone() } +<<<<<<< HEAD /// Get cache reference (for HTTP management API) pub fn cache(&self) -> Arc { self.cache.clone() +======= + /// Get alias repository reference (for HTTP layer wiring) + pub fn alias_repository(&self) -> Arc { + self.alias_repository.clone() +>>>>>>> 7a397b2 (feat: add model alias resolution and HTTP API (#111)) } pub async fn execute(&self, req: CompletionRequest) -> Result { @@ -84,7 +104,7 @@ impl RouteRequest { pub async fn execute_with_format( &self, - req: CompletionRequest, + mut req: CompletionRequest, client_format: ApiFormat, ) -> Result { // 0. Check if combo execution is requested @@ -92,10 +112,34 @@ impl RouteRequest { return self.execute_combo(&combo_id, req, client_format).await; } + // 0a. Resolve model alias if enabled (BEFORE restrictions check) + if self.alias_config.enabled { + match self.alias_repository.find_by_alias(&req.model, None).await { + Ok(Some(alias_entry)) => { + tracing::debug!( + alias = %req.model, + canonical = %alias_entry.canonical, + "Resolved model alias" + ); + req.model = alias_entry.canonical; + } + Ok(None) => { + // No alias found, proceed with original model + } + Err(e) => { + tracing::warn!( + error = ?e, + model = %req.model, + "Alias resolution failed, using original model" + ); + } + } + } + let cache_key = req.cache_key(); let start = Instant::now(); - // 0. Model restriction check (before any provider interaction) + // 0b. Model restriction check (AFTER alias resolution) if !req.restrictions.allowed_models.is_empty() && !req.restrictions.allowed_models.contains(&req.model) { @@ -1094,6 +1138,54 @@ mod tests { } } + /// Test stub for ModelAliasRepositoryPort — returns no aliases + struct TestAliasRepository; + + #[async_trait] + impl ModelAliasRepositoryPort for TestAliasRepository { + async fn find_by_alias( + &self, + _alias: &shared_kernel::ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, rook_core::ModelAliasRepositoryError> { + Ok(None) // No aliases in tests by default + } + + async fn list( + &self, + ) -> Result, rook_core::ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create( + &self, + _alias: rook_core::ModelAlias, + ) -> Result<(), rook_core::ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete( + &self, + _alias: &shared_kernel::ModelId, + ) -> Result { + Ok(false) + } + + async fn seed( + &self, + _aliases: Vec, + ) -> Result { + Ok(0) + } + } + + fn test_alias_config() -> ModelAliasesConfig { + ModelAliasesConfig { + enabled: false, // Disabled by default in tests + auto_seed: false, + } + } + struct FailingProviderRepository; #[async_trait] @@ -1212,6 +1304,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut stream = usecase @@ -1261,6 +1355,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ) } @@ -1285,6 +1381,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1337,6 +1435,8 @@ mod tests { None, Arc::new(pricing), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut req = request(); req.metadata.api_key_id = Some(ApiKeyId::new("key_123")); @@ -1388,6 +1488,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let req = request(); @@ -1432,6 +1534,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1467,6 +1571,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let result = usecase.execute(request()).await; @@ -1596,6 +1702,8 @@ mod tests { None, Arc::new(pricing), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut req = request(); req.metadata.api_key_id = Some(ApiKeyId::new("key_streaming")); @@ -1700,6 +1808,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let mut stream = usecase @@ -1748,6 +1858,8 @@ mod tests { None, Arc::new(crate::PricingConfig::default()), Arc::new(TestFormatTranslator), + Arc::new(TestAliasRepository), + test_alias_config(), ); let stream = usecase diff --git a/crates/application/rook-usecases/tests/route_request_restrictions.rs b/crates/application/rook-usecases/tests/route_request_restrictions.rs index 8fb8d995..ee333229 100644 --- a/crates/application/rook-usecases/tests/route_request_restrictions.rs +++ b/crates/application/rook-usecases/tests/route_request_restrictions.rs @@ -8,10 +8,10 @@ use async_trait::async_trait; use rook_core::{ ApiFormat, ApiKeyRestrictions, AuditEntry, AuditPort, CachePort, CacheStats, CompletionRequest, CompletionResponse, CortexError, CortexResult, FormatTranslatorPort, HealthStatus, Message, - MessageContent, ModelId, ProviderId, ProviderPort, RequestMetadata, Role, RouterPort, - StreamChunk, TokenUsage, + MessageContent, ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort, ModelId, + ProviderId, ProviderPort, RequestMetadata, Role, RouterPort, StreamChunk, TokenUsage, }; -use rook_usecases::{PricingConfig, RouteRequest}; +use rook_usecases::{route_request::ModelAliasesConfig, PricingConfig, RouteRequest}; use shared_kernel::{CacheKey, RequestId}; // --- Fake Implementations --- @@ -185,6 +185,43 @@ impl FormatTranslatorPort for NoOpTranslator { } } +/// Test stub for ModelAliasRepositoryPort +struct NoOpAliasRepository; + +#[async_trait] +impl ModelAliasRepositoryPort for NoOpAliasRepository { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + Ok(None) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete(&self, _alias: &ModelId) -> Result { + Ok(false) + } + + async fn seed(&self, _aliases: Vec) -> Result { + Ok(0) + } +} + +fn test_alias_config() -> ModelAliasesConfig { + ModelAliasesConfig { + enabled: false, + auto_seed: false, + } +} + // --- Test Cases --- #[tokio::test] @@ -194,6 +231,8 @@ async fn allowed_models_contains_requested_model_passes() { let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -204,6 +243,8 @@ async fn allowed_models_contains_requested_model_passes() { None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -245,6 +286,8 @@ async fn allowed_models_missing_requested_model_returns_403_with_structured_code let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -255,6 +298,8 @@ async fn allowed_models_missing_requested_model_returns_403_with_structured_code None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -300,6 +345,8 @@ async fn allowed_providers_contains_selected_provider_passes() { let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -310,6 +357,8 @@ async fn allowed_providers_contains_selected_provider_passes() { None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { @@ -351,6 +400,8 @@ async fn allowed_providers_missing_selected_provider_returns_403_with_structured let cache = Arc::new(NoOpCache) as Arc; let audit = Arc::new(NoOpAudit) as Arc; let translator = Arc::new(NoOpTranslator) as Arc; + let alias_repo = Arc::new(NoOpAliasRepository) as Arc; + let alias_config = test_alias_config(); let route_request = RouteRequest::new( router, @@ -361,6 +412,8 @@ async fn allowed_providers_missing_selected_provider_returns_403_with_structured None, Arc::new(PricingConfig::default()), translator, + alias_repo, + alias_config, ); let req = CompletionRequest { diff --git a/crates/infrastructure/transport-axum/src/alias_routes.rs b/crates/infrastructure/transport-axum/src/alias_routes.rs new file mode 100644 index 00000000..0961e1a6 --- /dev/null +++ b/crates/infrastructure/transport-axum/src/alias_routes.rs @@ -0,0 +1,21 @@ +// Alias routes — HTTP endpoints for model alias management + +use std::sync::Arc; + +use axum::{ + routing::{delete, get}, + Router, +}; +use rook_core::ModelAliasRepositoryPort; + +use super::handlers::aliases::{create_alias, delete_alias, list_aliases}; + +type AliasRepository = Arc; + +/// Build the alias CRUD router +pub fn router(alias_repo: AliasRepository) -> Router { + Router::new() + .route("/", get(list_aliases).post(create_alias)) + .route("/{alias}", delete(delete_alias)) + .with_state(alias_repo) +} diff --git a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs index c45949c1..97d85d04 100644 --- a/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs +++ b/crates/infrastructure/transport-axum/src/bootstrap_helpers.rs @@ -10,14 +10,15 @@ use std::sync::Arc; use models_catalog::StaticModelCatalog; use rook_core::{ ApiFormat, ApiKeyRepositoryPort, AuditEntry, AuditPort, CachePort, CompletionRequest, - CompletionResponse, CortexResult, FormatTranslatorPort, NewSession, PasswordHasher, RouterPort, - Session, SessionId, SessionRepositoryError, SessionRepositoryPort, UserRepositoryPort, + CompletionResponse, CortexResult, FormatTranslatorPort, ModelAlias, ModelAliasRepositoryError, + ModelAliasRepositoryPort, NewSession, PasswordHasher, RouterPort, Session, SessionId, + SessionRepositoryError, SessionRepositoryPort, UserRepositoryPort, }; use rook_usecases::{ BootstrapStatus, FallbackRouter, HealthCheck, ManageApiKeys, ManageProviders, RouteRequest, RoutingStrategy, SetAdminPassword, }; -use shared_kernel::CacheKey; +use shared_kernel::{CacheKey, ModelId, ProviderId}; use std::time::Duration; use tokio::sync::RwLock; @@ -43,6 +44,11 @@ pub fn make_test_bootstrap_usecases( let format_translator: Arc = Arc::new(StubFormatTranslator); let cache: Arc = Arc::new(StubCache); let audit: Arc = Arc::new(StubAudit); + let alias_repo: Arc = Arc::new(StubAliasRepo); + let alias_config = rook_usecases::route_request::ModelAliasesConfig { + enabled: false, + auto_seed: false, + }; let route_request = RouteRequest::new( fallback_router.clone() as Arc, @@ -53,6 +59,8 @@ pub fn make_test_bootstrap_usecases( None, Arc::new(rook_usecases::PricingConfig::default()), format_translator, + alias_repo, + alias_config, ); let manage_providers = ManageProviders::new(fallback_router.clone()); let health_check = Arc::new(HealthCheck::new(fallback_router.clone())); @@ -177,3 +185,33 @@ impl SessionRepositoryPort for StubSessionRepo { Ok(0) } } + +/// Stub alias repository — never called by bootstrap tests +struct StubAliasRepo; + +#[async_trait] +impl ModelAliasRepositoryPort for StubAliasRepo { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn delete(&self, _alias: &ModelId) -> Result { + unreachable!("alias_repo not called by bootstrap tests") + } + + async fn seed(&self, _aliases: Vec) -> Result { + unreachable!("alias_repo not called by bootstrap tests") + } +} diff --git a/crates/infrastructure/transport-axum/src/handlers/aliases.rs b/crates/infrastructure/transport-axum/src/handlers/aliases.rs new file mode 100644 index 00000000..b121e1a4 --- /dev/null +++ b/crates/infrastructure/transport-axum/src/handlers/aliases.rs @@ -0,0 +1,172 @@ +// Alias management HTTP handlers — CRUD operations for model aliases + +use std::sync::Arc; + +use axum::{ + extract::{Path, State}, + http::StatusCode, + Json, +}; +use rook_core::{ModelAlias, ModelAliasRepositoryPort}; +use serde::{Deserialize, Serialize}; +use shared_kernel::{ModelId, ProviderId}; + +use crate::HttpError; + +type AliasRepository = Arc; + +// ------------------------------------------------------------------------- +// DTOs +// ------------------------------------------------------------------------- + +/// Request body for POST /api/models/aliases +#[derive(Debug, Deserialize)] +pub struct CreateAliasRequest { + pub alias: String, + pub canonical: String, + #[serde(rename = "providerId")] + pub provider_id: Option, +} + +/// Response body for GET /api/models/aliases and single alias operations +#[derive(Debug, Serialize)] +pub struct AliasResponse { + pub alias: String, + pub canonical: String, + #[serde(rename = "providerId")] + pub provider_id: Option, + #[serde(rename = "createdAt")] + pub created_at: String, +} + +impl From<&ModelAlias> for AliasResponse { + fn from(alias: &ModelAlias) -> Self { + Self { + alias: alias.alias.to_string(), + canonical: alias.canonical.to_string(), + provider_id: alias.provider_id.as_ref().map(|p| p.to_string()), + created_at: alias.created_at.clone(), + } + } +} + +// ------------------------------------------------------------------------- +// Handlers +// ------------------------------------------------------------------------- + +/// GET /api/models/aliases — List all aliases +pub async fn list_aliases( + State(repo): State, +) -> Result>, HttpError> { + let aliases = repo.list().await.map_err(|e| HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to list aliases: {}", e), + })?; + + let response: Vec = aliases.iter().map(AliasResponse::from).collect(); + Ok(Json(response)) +} + +/// POST /api/models/aliases — Create a new alias +pub async fn create_alias( + State(repo): State, + Json(req): Json, +) -> Result { + // Validate input + if req.alias.trim().is_empty() { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "INVALID_ALIAS", + message: "alias must not be empty".to_string(), + }); + } + + if req.canonical.trim().is_empty() { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "INVALID_CANONICAL", + message: "canonical must not be empty".to_string(), + }); + } + + // Check if canonical is itself an alias (cycle prevention) + let canonical_model_id = ModelId::new(req.canonical.clone()); + match repo.find_by_alias(&canonical_model_id, None).await { + Ok(Some(_)) => { + return Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "ALIAS_CYCLE", + message: "Aliases cannot point to other aliases".to_string(), + }); + } + Ok(None) => { + // Good — canonical is not an alias + } + Err(e) => { + return Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to check alias cycle: {}", e), + }); + } + } + + // Build domain model + let alias = ModelAlias { + alias: ModelId::new(req.alias.clone()), + canonical: canonical_model_id, + provider_id: req.provider_id.map(ProviderId::new), + created_at: chrono::Utc::now().to_rfc3339(), + }; + + let alias_str = alias.alias.to_string(); + let canonical_str = alias.canonical.to_string(); + + // Create alias + match repo.create(alias).await { + Ok(()) => { + tracing::info!( + alias = %alias_str, + canonical = %canonical_str, + "alias created" + ); + Ok(StatusCode::CREATED) + } + Err(e) if e.to_string().contains("already exists") => Err(HttpError { + status: StatusCode::BAD_REQUEST, + code: "ALIAS_ALREADY_EXISTS", + message: format!("Alias '{}' already exists", alias_str), + }), + Err(e) => Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to create alias: {}", e), + }), + } +} + +/// DELETE /api/models/aliases/:alias — Delete an alias +pub async fn delete_alias( + State(repo): State, + Path(alias): Path, +) -> Result { + let alias_id = ModelId::new(alias); + + match repo.delete(&alias_id).await { + Ok(true) => { + tracing::info!(alias = %alias_id, "alias deleted"); + Ok(StatusCode::NO_CONTENT) + } + Ok(false) => Err(HttpError { + status: StatusCode::NOT_FOUND, + code: "ALIAS_NOT_FOUND", + message: format!("Alias '{}' not found", alias_id), + }), + Err(e) => Err(HttpError { + status: StatusCode::INTERNAL_SERVER_ERROR, + code: "REPOSITORY_ERROR", + message: format!("Failed to delete alias: {}", e), + }), + } +} diff --git a/crates/infrastructure/transport-axum/src/handlers/mod.rs b/crates/infrastructure/transport-axum/src/handlers/mod.rs index 632c5347..a5619313 100644 --- a/crates/infrastructure/transport-axum/src/handlers/mod.rs +++ b/crates/infrastructure/transport-axum/src/handlers/mod.rs @@ -1,5 +1,6 @@ // handlers — HTTP endpoint handlers +pub mod aliases; pub mod api_key; pub mod auth; pub mod bootstrap; diff --git a/crates/infrastructure/transport-axum/src/lib.rs b/crates/infrastructure/transport-axum/src/lib.rs index 6f605cd3..6652f2bb 100644 --- a/crates/infrastructure/transport-axum/src/lib.rs +++ b/crates/infrastructure/transport-axum/src/lib.rs @@ -3,6 +3,7 @@ // Translates between provider wire formats (OpenAI, Anthropic) and the // internal domain model. All format-specific logic lives here. +pub mod alias_routes; pub mod anthropic_adapter; pub mod api_key_dto; pub mod authz; diff --git a/crates/infrastructure/transport-axum/src/routes.rs b/crates/infrastructure/transport-axum/src/routes.rs index ee29fe2c..ad386d6f 100644 --- a/crates/infrastructure/transport-axum/src/routes.rs +++ b/crates/infrastructure/transport-axum/src/routes.rs @@ -18,8 +18,8 @@ use tower_http::limit::RequestBodyLimitLayer; use tracing::error; use super::{ - anthropic_adapter::*, authz, combo_routes, handlers, middleware::csrf_guard, openai_adapter::*, - provider_routes, HttpError, + alias_routes, anthropic_adapter::*, authz, combo_routes, handlers, middleware::csrf_guard, + openai_adapter::*, provider_routes, HttpError, }; use crate::middleware::{ApiKeyRateLimiter, CsrfGuard, IpRateLimiter, LoginRateLimiter}; @@ -83,6 +83,12 @@ pub fn router( )); } + // Alias routes (model alias repository is always available) + router = router.nest( + "/api/models/aliases", + alias_routes::router(usecases.route_request.alias_repository()), + ); + // Model catalog is always available (the catalog port is mandatory on // RookUsecases), so the route is always mounted. router = router.merge(crate::models_routes::router(usecases.clone())); diff --git a/crates/infrastructure/transport-axum/tests/alias_api.rs b/crates/infrastructure/transport-axum/tests/alias_api.rs new file mode 100644 index 00000000..8e1c777d --- /dev/null +++ b/crates/infrastructure/transport-axum/tests/alias_api.rs @@ -0,0 +1,407 @@ +// Integration tests for model alias HTTP API + +use axum::{ + body::Body, + http::{Request, StatusCode}, + Router, +}; +use rook_core::{ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort}; +use serde_json::json; +use shared_kernel::{ModelId, ProviderId}; +use std::sync::Arc; +use tower::ServiceExt; +use transport_axum::alias_routes; + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +/// In-memory alias repository for testing +#[derive(Clone)] +struct InMemoryAliasRepo { + aliases: Arc>>, +} + +impl InMemoryAliasRepo { + fn new() -> Self { + Self { + aliases: Arc::new(tokio::sync::RwLock::new(Vec::new())), + } + } + + async fn seed_builtin(&self) { + let builtins = vec![ + ModelAlias { + alias: ModelId::new("gpt-4o-latest"), + canonical: ModelId::new("gpt-4o-2024-05-13"), + provider_id: Some(ProviderId::new("openai")), + created_at: "2024-01-15T10:30:00Z".to_string(), + }, + ModelAlias { + alias: ModelId::new("claude-opus"), + canonical: ModelId::new("claude-opus-4-5"), + provider_id: Some(ProviderId::new("anthropic")), + created_at: "2024-01-15T10:30:00Z".to_string(), + }, + ]; + + let mut aliases = self.aliases.write().await; + aliases.extend(builtins); + } +} + +#[async_trait::async_trait] +impl ModelAliasRepositoryPort for InMemoryAliasRepo { + async fn find_by_alias( + &self, + alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + let aliases = self.aliases.read().await; + Ok(aliases.iter().find(|a| a.alias == *alias).cloned()) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + let aliases = self.aliases.read().await; + Ok(aliases.clone()) + } + + async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + let mut aliases = self.aliases.write().await; + if aliases.iter().any(|a| a.alias == alias.alias) { + return Err(ModelAliasRepositoryError::AlreadyExists(alias.alias)); + } + aliases.push(alias); + Ok(()) + } + + async fn delete(&self, alias: &ModelId) -> Result { + let mut aliases = self.aliases.write().await; + let before_len = aliases.len(); + aliases.retain(|a| a.alias != *alias); + Ok(aliases.len() < before_len) + } + + async fn seed(&self, builtins: Vec) -> Result { + let mut aliases = self.aliases.write().await; + let mut count = 0; + for builtin in builtins { + if !aliases.iter().any(|a| a.alias == builtin.alias) { + aliases.push(builtin); + count += 1; + } + } + Ok(count) + } +} + +fn test_app() -> Router { + let repo = Arc::new(InMemoryAliasRepo::new()) as Arc; + alias_routes::router(repo) +} + +async fn test_app_with_seeded() -> Router { + let repo = Arc::new(InMemoryAliasRepo::new()); + repo.seed_builtin().await; + alias_routes::router(repo) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn test_get_aliases_empty() { + let app = test_app(); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 0); +} + +#[tokio::test] +async fn test_get_aliases_with_builtin() { + let app = test_app_with_seeded().await; + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 2); + + // Verify structure + assert_eq!(aliases[0]["alias"], "gpt-4o-latest"); + assert_eq!(aliases[0]["canonical"], "gpt-4o-2024-05-13"); + assert_eq!(aliases[0]["providerId"], "openai"); + assert!(aliases[0]["createdAt"].is_string()); +} + +#[tokio::test] +async fn test_create_alias_success() { + let app = test_app(); + + let payload = json!({ + "alias": "my-gpt4", + "canonical": "gpt-4-0613", + "providerId": "openai" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); +} + +#[tokio::test] +async fn test_create_alias_duplicate() { + let app = test_app_with_seeded().await; + + let payload = json!({ + "alias": "gpt-4o-latest", + "canonical": "gpt-4o-2024-08-06" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_ALREADY_EXISTS"); +} + +#[tokio::test] +async fn test_create_alias_empty_alias() { + let app = test_app(); + + let payload = json!({ + "alias": "", + "canonical": "gpt-4-0613" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "INVALID_ALIAS"); +} + +#[tokio::test] +async fn test_create_alias_empty_canonical() { + let app = test_app(); + + let payload = json!({ + "alias": "my-model", + "canonical": "" + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "INVALID_CANONICAL"); +} + +#[tokio::test] +async fn test_create_alias_cycle_detection() { + let app = test_app_with_seeded().await; + + // Try to create alias pointing to another alias + let payload = json!({ + "alias": "my-alias", + "canonical": "gpt-4o-latest" // This is itself an alias + }); + + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_CYCLE"); + assert!(error["error"] + .as_str() + .unwrap() + .contains("cannot point to other aliases")); +} + +#[tokio::test] +async fn test_delete_alias_success() { + let app = test_app_with_seeded().await; + + let response = app + .oneshot( + Request::builder() + .uri("/gpt-4o-latest") + .method("DELETE") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); +} + +#[tokio::test] +async fn test_delete_alias_not_found() { + let app = test_app(); + + let response = app + .oneshot( + Request::builder() + .uri("/nonexistent-alias") + .method("DELETE") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let error: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(error["code"], "ALIAS_NOT_FOUND"); +} + +#[tokio::test] +async fn test_create_and_list() { + let app = test_app(); + + // Create alias + let payload = json!({ + "alias": "test-alias", + "canonical": "test-model-v1", + "providerId": "test-provider" + }); + + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/") + .method("POST") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&payload).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); + + // List aliases + let response = app + .oneshot( + Request::builder() + .uri("/") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let aliases: Vec = serde_json::from_slice(&body).unwrap(); + assert_eq!(aliases.len(), 1); + assert_eq!(aliases[0]["alias"], "test-alias"); + assert_eq!(aliases[0]["canonical"], "test-model-v1"); + assert_eq!(aliases[0]["providerId"], "test-provider"); +} diff --git a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs index 461398ac..d7e8f221 100644 --- a/crates/infrastructure/transport-axum/tests/format_translation_integration.rs +++ b/crates/infrastructure/transport-axum/tests/format_translation_integration.rs @@ -11,8 +11,16 @@ // SC-05 + SC-10: Anthropic round-trip (content[0].type == "text", stop_reason == "end_turn") // SC-01 + SC-02: No parse error on requests that include `tools` or `stream_options` fields -use rook_core::{CompletionResponse, MessageContent, ModelId, Role, TokenUsage}; -use shared_kernel::{ProviderId, RequestId}; +use async_trait::async_trait; +use rook_core::{ + ApiFormat, AuditEntry, AuditPort, CachePort, CompletionRequest, CompletionResponse, + HealthStatus, MessageContent, ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort, + ModelId, ProviderPort, RequestMetadata, Role, RouterPort, StreamChunk, TokenUsage, +}; +use rook_usecases::{route_request::ModelAliasesConfig, RouteRequest}; +use shared_kernel::{CacheKey, ProviderId, RequestId}; +use std::sync::Arc; +use std::time::Duration; use transport_axum::{ anthropic_adapter::{AnthropicMessagesRequest, AnthropicMessagesResponse}, openai_adapter::{OpenAIChatRequest, OpenAIChatResponse}, @@ -210,14 +218,8 @@ fn anthropic_response_has_correct_structure() { // Registry-routed multi-format use case integration // --------------------------------------------------------------------------- -use async_trait::async_trait; use futures::stream; -use rook_core::{ - ApiFormat, AuditEntry, AuditPort, CacheKey, CachePort, CompletionRequest, FormatTranslatorPort, - HealthStatus, ProviderPort, RequestMetadata, RouterPort, StreamChunk, -}; -use rook_usecases::RouteRequest; -use std::{sync::Arc, time::Duration}; +use rook_core::FormatTranslatorPort; use transport_axum::format_registry::{DomainPivotTranslator, FormatRegistry}; struct RegistryTestProvider { @@ -356,6 +358,36 @@ impl AuditPort for NoopAudit { } } +/// Test stub for ModelAliasRepositoryPort +struct NoopAliasRepository; + +#[async_trait] +impl ModelAliasRepositoryPort for NoopAliasRepository { + async fn find_by_alias( + &self, + _alias: &ModelId, + _provider_id: Option<&ProviderId>, + ) -> Result, ModelAliasRepositoryError> { + Ok(None) + } + + async fn list(&self) -> Result, ModelAliasRepositoryError> { + Ok(vec![]) + } + + async fn create(&self, _alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { + Ok(()) + } + + async fn delete(&self, _alias: &ModelId) -> Result { + Ok(false) + } + + async fn seed(&self, _aliases: Vec) -> Result { + Ok(0) + } +} + static REGISTRY_TEST_MODEL: std::sync::LazyLock = std::sync::LazyLock::new(|| ModelId::new("registry-test-model")); @@ -390,6 +422,11 @@ fn registry_route_request(provider_format: ApiFormat, content: &'static str) -> None, // combo_repository Arc::new(rook_usecases::PricingConfig::default()), registry_with_openai_anthropic_pairs(), + Arc::new(NoopAliasRepository), + ModelAliasesConfig { + enabled: false, + auto_seed: false, + }, ) } From f7297d62f3ee148e0f71ba89e7e654960adba505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 5 Jun 2026 10:31:39 +0200 Subject: [PATCH 6/6] fix: address code review findings for model aliasing - Add alias resolution to execute_stream_with_format for streaming requests - Change ModelAlias.created_at from String to DateTime for consistency - Generate unique timestamps per alias in builtin_aliases() - Update builtin.rs comment to reflect provider-scoped aliases - Clarify cycle detection as depth-1 only in repository - Replace string matching with enum matching for AlreadyExists error - Make cycle check provider-scoped in handler and repository query - Add db_migration import to fix test initialization All changes verified with full CI passing. --- apps/rook/src/di.rs | 2 +- apps/rook/tests/alias_routing_e2e.rs | 7 ++- .../rook-usecases/src/route_request.rs | 28 +++++++++- crates/domain/rook-core/src/model.rs | 4 +- .../alias-sqlite/src/builtin.rs | 2 +- .../alias-sqlite/src/repository.rs | 56 +++++++++++++------ .../transport-axum/src/handlers/aliases.rs | 14 +++-- .../transport-axum/tests/alias_api.rs | 6 +- 8 files changed, 84 insertions(+), 35 deletions(-) diff --git a/apps/rook/src/di.rs b/apps/rook/src/di.rs index a4f4847f..0b3ffb46 100644 --- a/apps/rook/src/di.rs +++ b/apps/rook/src/di.rs @@ -185,7 +185,7 @@ impl RookContainer { alias: shared_kernel::ModelId::new(*alias), canonical: shared_kernel::ModelId::new(*canonical), provider_id: provider_id.map(shared_kernel::ProviderId::new), - created_at: shared_kernel::Utc::now().to_rfc3339(), + created_at: shared_kernel::Utc::now(), }) .collect::>(); diff --git a/apps/rook/tests/alias_routing_e2e.rs b/apps/rook/tests/alias_routing_e2e.rs index 5e548749..9715f72b 100644 --- a/apps/rook/tests/alias_routing_e2e.rs +++ b/apps/rook/tests/alias_routing_e2e.rs @@ -1,6 +1,7 @@ use alias_sqlite::{repository::builtin_aliases, SqliteModelAliasRepository}; use rook_core::ports::ModelAliasRepositoryPort; use rook_core::{ModelAlias, ModelId}; +use shared_kernel::Utc; use std::sync::Arc; /// Test that alias resolution works end-to-end in routing @@ -17,7 +18,7 @@ async fn test_alias_resolves_before_routing() { alias: ModelId::new("gpt-4o-latest"), canonical: ModelId::new("gpt-4o-2024-05-13"), provider_id: None, - created_at: "2026-06-05T07:10:00Z".to_string(), + created_at: Utc::now(), }; repo.create(alias.clone()).await.expect("create alias"); @@ -55,7 +56,7 @@ async fn test_canonical_model_passes_through() { alias: ModelId::new("gpt-4-turbo"), canonical: ModelId::new("gpt-4-turbo-2024-04-09"), provider_id: None, - created_at: "2026-06-05T07:10:00Z".to_string(), + created_at: Utc::now(), }; repo.create(alias).await.expect("create alias"); @@ -182,7 +183,7 @@ async fn test_repository_handles_concurrent_access() { alias: ModelId::new(format!("test-alias-{}", i)), canonical: ModelId::new(format!("test-canonical-{}", i)), provider_id: None, - created_at: "2026-06-05T07:10:00Z".to_string(), + created_at: Utc::now(), }; repo_clone.create(alias).await }); diff --git a/crates/application/rook-usecases/src/route_request.rs b/crates/application/rook-usecases/src/route_request.rs index 15f20d5e..e87bcfee 100644 --- a/crates/application/rook-usecases/src/route_request.rs +++ b/crates/application/rook-usecases/src/route_request.rs @@ -242,13 +242,37 @@ impl RouteRequest { pub async fn execute_stream_with_format( &self, - req: CompletionRequest, + mut req: CompletionRequest, client_format: ApiFormat, ) -> Result>, CortexError> { let start = Instant::now(); - // 0. Model restriction check + // 0. Resolve model alias if enabled (BEFORE restrictions check) + if self.alias_config.enabled { + match self.alias_repository.find_by_alias(&req.model, None).await { + Ok(Some(alias_entry)) => { + tracing::debug!( + alias = %req.model, + canonical = %alias_entry.canonical, + "Resolved model alias" + ); + req.model = alias_entry.canonical; + } + Ok(None) => { + // No alias found, proceed with original model + } + Err(e) => { + tracing::warn!( + error = ?e, + model = %req.model, + "Alias resolution failed, using original model" + ); + } + } + } + + // 0a. Model restriction check (AFTER alias resolution) if !req.restrictions.allowed_models.is_empty() && !req.restrictions.allowed_models.contains(&req.model) { diff --git a/crates/domain/rook-core/src/model.rs b/crates/domain/rook-core/src/model.rs index 8d7c8dd9..24b0b127 100644 --- a/crates/domain/rook-core/src/model.rs +++ b/crates/domain/rook-core/src/model.rs @@ -586,8 +586,8 @@ pub struct ModelAlias { pub canonical: ModelId, /// Optional provider scope (null = global) pub provider_id: Option, - /// Creation timestamp (ISO 8601) - pub created_at: String, + /// Creation timestamp + pub created_at: chrono::DateTime, } /// A multi-step fallback chain aggregate diff --git a/crates/infrastructure/alias-sqlite/src/builtin.rs b/crates/infrastructure/alias-sqlite/src/builtin.rs index 30881b84..d6e713ed 100644 --- a/crates/infrastructure/alias-sqlite/src/builtin.rs +++ b/crates/infrastructure/alias-sqlite/src/builtin.rs @@ -1,7 +1,7 @@ //! Built-in model aliases — seeded at startup when table is empty /// Built-in aliases: (alias, canonical, provider_id) -/// Provider ID is None for global aliases +/// All aliases are provider-scoped (provider_id is always Some) pub const DEFAULT_ALIASES: &[(&str, &str, Option<&str>)] = &[ // OpenAI ("gpt-4o-latest", "gpt-4o-2024-05-13", Some("openai")), diff --git a/crates/infrastructure/alias-sqlite/src/repository.rs b/crates/infrastructure/alias-sqlite/src/repository.rs index eea7844f..77bb5b6d 100644 --- a/crates/infrastructure/alias-sqlite/src/repository.rs +++ b/crates/infrastructure/alias-sqlite/src/repository.rs @@ -3,6 +3,7 @@ use std::sync::{Mutex, MutexGuard}; use async_trait::async_trait; use chrono::Utc; +use db_migration; use rook_core::ports::{ModelAliasRepositoryError, ModelAliasRepositoryPort}; use rook_core::ModelAlias; use rusqlite::{params, Connection, OptionalExtension}; @@ -66,7 +67,12 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { provider_id: row .get::<_, Option>(2)? .map(|s| ProviderId::new(&s)), - created_at: row.get(3)?, + created_at: row + .get::<_, String>(3) + .ok() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(&s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .unwrap_or_else(chrono::Utc::now), }) }, ) @@ -87,7 +93,12 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { provider_id: row .get::<_, Option>(2)? .map(|s| ProviderId::new(&s)), - created_at: row.get(3)?, + created_at: row + .get::<_, String>(3) + .ok() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(&s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .unwrap_or_else(chrono::Utc::now), }) }, ) @@ -114,10 +125,13 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { Ok(ModelAlias { alias: ModelId::new(row.get::<_, String>(0)?), canonical: ModelId::new(row.get::<_, String>(1)?), - provider_id: row - .get::<_, Option>(2)? - .map(|s| ProviderId::new(&s)), - created_at: row.get(3)?, + provider_id: row.get::<_, Option>(2)?.map(ProviderId::new), + created_at: row + .get::<_, String>(3) + .ok() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(&s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .unwrap_or_else(chrono::Utc::now), }) }) .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))? @@ -130,15 +144,23 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { async fn create(&self, alias: ModelAlias) -> Result<(), ModelAliasRepositoryError> { let conn = self.lock()?; - // Check if canonical is itself an alias (prevent cycles) - let canonical_is_alias = conn - .query_row( - "SELECT 1 FROM model_aliases WHERE alias = ?1 LIMIT 1", + // Check if canonical is itself an alias (prevent direct canonical-as-alias, depth-1 only) + // Provider-scoped: check within the same provider scope + let canonical_is_alias = if let Some(ref provider_id) = alias.provider_id { + conn.query_row( + "SELECT 1 FROM model_aliases WHERE alias = ?1 AND provider_id = ?2 LIMIT 1", + params![alias.canonical.as_str(), provider_id.as_str()], + |_| Ok(()), + ) + } else { + conn.query_row( + "SELECT 1 FROM model_aliases WHERE alias = ?1 AND provider_id IS NULL LIMIT 1", params![alias.canonical.as_str()], |_| Ok(()), ) - .optional() - .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; + } + .optional() + .map_err(|e| ModelAliasRepositoryError::Database(e.to_string()))?; if canonical_is_alias.is_some() { return Err(ModelAliasRepositoryError::InvalidAlias( @@ -154,7 +176,7 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { alias.alias.as_str(), alias.canonical.as_str(), alias.provider_id.as_ref().map(|p| p.as_str()), - alias.created_at, + alias.created_at.to_rfc3339(), ], ); @@ -198,7 +220,7 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { alias.alias.as_str(), alias.canonical.as_str(), alias.provider_id.as_ref().map(|p| p.as_str()), - alias.created_at, + alias.created_at.to_rfc3339(), ], ); @@ -219,8 +241,6 @@ impl ModelAliasRepositoryPort for SqliteModelAliasRepository { /// Helper function to create built-in aliases from constants pub fn builtin_aliases() -> Vec { - let now = Utc::now().to_rfc3339(); - DEFAULT_ALIASES .iter() .map(|(alias_str, canonical_str, provider_id_str)| { @@ -229,7 +249,7 @@ pub fn builtin_aliases() -> Vec { alias: ModelId::new(alias_str.to_string()), canonical: ModelId::new(canonical_str.to_string()), provider_id, - created_at: now.clone(), + created_at: Utc::now(), } }) .collect() @@ -248,7 +268,7 @@ mod tests { alias: ModelId::new(alias), canonical: ModelId::new(canonical), provider_id: None, - created_at: Utc::now().to_rfc3339(), + created_at: Utc::now(), } } diff --git a/crates/infrastructure/transport-axum/src/handlers/aliases.rs b/crates/infrastructure/transport-axum/src/handlers/aliases.rs index b121e1a4..8579c213 100644 --- a/crates/infrastructure/transport-axum/src/handlers/aliases.rs +++ b/crates/infrastructure/transport-axum/src/handlers/aliases.rs @@ -7,7 +7,7 @@ use axum::{ http::StatusCode, Json, }; -use rook_core::{ModelAlias, ModelAliasRepositoryPort}; +use rook_core::{ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort}; use serde::{Deserialize, Serialize}; use shared_kernel::{ModelId, ProviderId}; @@ -45,7 +45,7 @@ impl From<&ModelAlias> for AliasResponse { alias: alias.alias.to_string(), canonical: alias.canonical.to_string(), provider_id: alias.provider_id.as_ref().map(|p| p.to_string()), - created_at: alias.created_at.clone(), + created_at: alias.created_at.to_rfc3339(), } } } @@ -92,7 +92,11 @@ pub async fn create_alias( // Check if canonical is itself an alias (cycle prevention) let canonical_model_id = ModelId::new(req.canonical.clone()); - match repo.find_by_alias(&canonical_model_id, None).await { + let provider_scope = req.provider_id.as_ref().map(ProviderId::new); + match repo + .find_by_alias(&canonical_model_id, provider_scope.as_ref()) + .await + { Ok(Some(_)) => { return Err(HttpError { status: StatusCode::BAD_REQUEST, @@ -117,7 +121,7 @@ pub async fn create_alias( alias: ModelId::new(req.alias.clone()), canonical: canonical_model_id, provider_id: req.provider_id.map(ProviderId::new), - created_at: chrono::Utc::now().to_rfc3339(), + created_at: chrono::Utc::now(), }; let alias_str = alias.alias.to_string(); @@ -133,7 +137,7 @@ pub async fn create_alias( ); Ok(StatusCode::CREATED) } - Err(e) if e.to_string().contains("already exists") => Err(HttpError { + Err(ModelAliasRepositoryError::AlreadyExists(_)) => Err(HttpError { status: StatusCode::BAD_REQUEST, code: "ALIAS_ALREADY_EXISTS", message: format!("Alias '{}' already exists", alias_str), diff --git a/crates/infrastructure/transport-axum/tests/alias_api.rs b/crates/infrastructure/transport-axum/tests/alias_api.rs index 8e1c777d..4196ab5c 100644 --- a/crates/infrastructure/transport-axum/tests/alias_api.rs +++ b/crates/infrastructure/transport-axum/tests/alias_api.rs @@ -7,7 +7,7 @@ use axum::{ }; use rook_core::{ModelAlias, ModelAliasRepositoryError, ModelAliasRepositoryPort}; use serde_json::json; -use shared_kernel::{ModelId, ProviderId}; +use shared_kernel::{ModelId, ProviderId, Utc}; use std::sync::Arc; use tower::ServiceExt; use transport_axum::alias_routes; @@ -35,13 +35,13 @@ impl InMemoryAliasRepo { alias: ModelId::new("gpt-4o-latest"), canonical: ModelId::new("gpt-4o-2024-05-13"), provider_id: Some(ProviderId::new("openai")), - created_at: "2024-01-15T10:30:00Z".to_string(), + created_at: Utc::now(), }, ModelAlias { alias: ModelId::new("claude-opus"), canonical: ModelId::new("claude-opus-4-5"), provider_id: Some(ProviderId::new("anthropic")), - created_at: "2024-01-15T10:30:00Z".to_string(), + created_at: Utc::now(), }, ];